Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add support for custom Paperclip mappings #1271

Merged
merged 1 commit into from

6 participants

@Robovirtuoso

Paperclip supports custom interpolations in images' paths. In order to
ease transition into CarrierWave, support the same functionality with the
Paperclip::Compatibility module.

The public api is similar to Paperclip's

class CustomUploader < CarrierWave::Uploader::Base
  include CarrierWave::Compatibility::Paperclip

  interpolate :some_method do |custom, style|
    custom.model.some_method
  end
end

This is accomplished by appending the desired functionality into
CarrierWave::Compatibility::Paperclip#mappings

In addition, we removed some double stated dependencies (timecop & nokogiri)
and resolved a problem with bundling dependencies for mime-types.

carrierwave.gemspec
@@ -37,6 +37,4 @@ Gem::Specification.new do |s|
s.add_development_dependency "fog", ">= 1.3.1"
s.add_development_dependency "mini_magick", ">= 3.6.0"
s.add_development_dependency "rmagick"
- s.add_development_dependency "nokogiri", "~> 1.5.10" # 1.6 requires ruby > 1.8.7
- s.add_development_dependency "timecop", "0.6.1" # 0.6.2 requires ruby > 1.8.7
@taavo Collaborator
taavo added a note

These two lines, as indicated in the comment, are required for 1.8.7 compatibility

@bensie Owner
bensie added a note

Please don't modify the gemspec, these are required for Ruby 1.8.7.

@bensie Owner
bensie added a note

Beat me @taavo!!

@taavo Collaborator
taavo added a note

Hah! Btw I just opened a question on stack overflow re mime-types killing bundler: http://stackoverflow.com/questions/20107243/travis-error-bundler-sometimes-cant-find-compatible-versions. I'm open minded to reverting to => 1.16, as in the above, just so we can use travis again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stephenprater

Embrace the future people. :wink:

@bensie
Owner

@stephenprater If we hit a roadblock where we can't continue without dropping support for 1.8, we'll certainly embrace the future. But CarrierWave supports Rails 3, which supports 1.8.

@Robovirtuoso

Tests are failing because Rails master depends on arel 5.0 which has not been released yet.

carrierwave.gemspec
@@ -26,7 +26,7 @@ Gem::Specification.new do |s|
s.add_dependency "activesupport", ">= 3.2.0"
s.add_dependency "activemodel", ">= 3.2.0"
s.add_dependency "json", ">= 1.7"
- s.add_dependency "mime-types", ">= 1.25"
+ s.add_dependency "mime-types", ">= 1.16"
@taavo Collaborator
taavo added a note

I agree with this change, in this PR or otherwise. Although our gemspec used to work as is, something changed and bundle install now goes into an infinite loop or gives the same error travis has been reporting. I'd assumed this was just travis, but it happens for me locally.

@bensie Owner
bensie added a note

Ok, but let's put this in a separate PR since it'll probably come up again in the future and could really use its own description.

@taavo Collaborator
taavo added a note

I committed this line and the spec_helper line below separately, because I was really tired of watching travis explode. @Robovirtuoso, can you rebase off master? Also: Thank you for fixing our build for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bensie
Owner

@Robovirtuoso No problem, I'll fix the issues with Rails master -- that happens pretty much anytime a point release is near.

@stephenprater

We got a little further into this and realized that while the test case worked that in real code you'd have a problem with multiple versions - introduced a test case to cover that aspect of it and changed the implementation a little bit.

Should be good now.

@stephenprater

One other thing we noticed was that in Paperclip's custom interpolations the second method is 'style' - like :thumb or whatever - but here's it's actually the base file name. Do we want to mimic the Paperclip implementation here or leave it as is?

@Robovirtuoso

The latest commit also rebased this branch against master

@thiagofm
Owner

Looking good! I think it should mimic the paperclip implementation.

Please don't forget to squash the commits and add a proper author(with a linked github account). :+1:

@Robovirtuoso

The block variables in the .interpolate block now mimic that of Paperclip's implementation, commits have been squashed and an author with a link to a github was added.

@Robovirtuoso Robovirtuoso Add support for custom Paperclip mappings
Paperclip supports custom interpolations in images' paths. In order to
ease transition into CarrierWave, support the same functionality with the
`Paperclip::Compatibility` module.

The public api is similar to Paperclip's

```ruby
class CustomUploader < CarrierWave::Uploader::Base
  include CarrierWave::Compatibility::Paperclip

  interpolate :some_method do |custom, style|
    custom.model.some_method
  end
end
```

This is accomplished by appending the desired functionality into
`CarrierWave::Compatibility::Paperclip#mappings`
dcb65a9
@Robovirtuoso

Changing the block variables broke some of the pre-built interpolators that did not have test coverage. Test coverage was added and interpolators were fixed.

@bensie bensie merged commit cdcab9b into carrierwaveuploader:master
@bquorning bquorning commented on the diff
spec/compatibility/paperclip_spec.rb
@@ -47,6 +55,86 @@ module Rails; end unless defined?(Rails)
@uploader.stub!(:paperclip_path).and_return("/foo/:id_partition/bar")
@uploader.store_path("monkey.png").should == "/foo/000/000/023/bar"

Since Ruby 1.8 doesn't have ordered hashes, this particular spec example occasionally fails on 1.8.7. That's because sometimes the mapping for :id is used instead of the mapping for id_partition, causing the #store_path to return "/foo/23_partition/bar". Whoops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 26, 2013
  1. @Robovirtuoso

    Add support for custom Paperclip mappings

    Robovirtuoso authored Acumen Workstation committed
    Paperclip supports custom interpolations in images' paths. In order to
    ease transition into CarrierWave, support the same functionality with the
    `Paperclip::Compatibility` module.
    
    The public api is similar to Paperclip's
    
    ```ruby
    class CustomUploader < CarrierWave::Uploader::Base
      include CarrierWave::Compatibility::Paperclip
    
      interpolate :some_method do |custom, style|
        custom.model.some_method
      end
    end
    ```
    
    This is accomplished by appending the desired functionality into
    `CarrierWave::Compatibility::Paperclip#mappings`
This page is out of date. Refresh to see the latest.
View
48 lib/carrierwave/compatibility/paperclip.rb
@@ -46,11 +46,31 @@ module Compatibility
# THE SOFTWARE.
#
module Paperclip
+ extend ActiveSupport::Concern
+
+ DEFAULT_MAPPINGS = {
+ :rails_root => lambda{|u, f| Rails.root.to_s },
+ :rails_env => lambda{|u, f| Rails.env },
+ :id_partition => lambda{|u, f| ("%09d" % u.model.id).scan(/\d{3}/).join("/")},
+ :id => lambda{|u, f| u.model.id },
+ :attachment => lambda{|u, f| u.mounted_as.to_s.downcase.pluralize },
+ :style => lambda{|u, f| u.paperclip_style },
+ :basename => lambda{|u, f| u.filename.gsub(/#{File.extname(u.filename)}$/, "") },
+ :extension => lambda{|u, d| File.extname(u.filename).gsub(/^\.+/, "")},
+ :class => lambda{|u, f| u.model.class.name.underscore.pluralize}
+ }
+
+ included do
+ attr_accessor :filename
+ class_attribute :mappings
+ self.mappings ||= DEFAULT_MAPPINGS.dup
+ end
def store_path(for_file=filename)
path = paperclip_path
+ self.filename = for_file
path ||= File.join(*[store_dir, paperclip_style.to_s, for_file].compact)
- interpolate_paperclip_path(path, for_file)
+ interpolate_paperclip_path(path)
end
def store_dir
@@ -68,28 +88,18 @@ def paperclip_style
version_name || paperclip_default_style
end
- private
-
- def interpolate_paperclip_path(path, filename)
- mappings.inject(path) do |agg, pair|
- agg.gsub(":#{pair[0]}") { pair[1].call(self, filename).to_s }
+ module ClassMethods
+ def interpolate(sym, &block)
+ mappings[sym] = block
end
end
- def mappings
- [
- [:rails_root , lambda{|u, f| Rails.root }],
- [:rails_env , lambda{|u, f| Rails.env }],
- [:class , lambda{|u, f| u.model.class.name.underscore.pluralize}],
- [:id_partition , lambda{|u, f| ("%09d" % u.model.id).scan(/\d{3}/).join("/")}],
- [:id , lambda{|u, f| u.model.id }],
- [:attachment , lambda{|u, f| u.mounted_as.to_s.downcase.pluralize }],
- [:style , lambda{|u, f| u.paperclip_style }],
- [:basename , lambda{|u, f| f.gsub(/#{File.extname(f)}$/, "") }],
- [:extension , lambda{|u, f| File.extname(f).gsub(/^\.+/, "")}]
- ]
+ private
+ def interpolate_paperclip_path(path)
+ mappings.each_pair.inject(path) do |agg, pair|
+ agg.gsub(":#{pair[0]}") { pair[1].call(self, self.paperclip_style).to_s }
+ end
end
-
end # Paperclip
end # Compatibility
end # CarrierWave
View
88 spec/compatibility/paperclip_spec.rb
@@ -13,9 +13,17 @@ module Rails; end unless defined?(Rails)
Rails.stub(:env).and_return('test')
@uploader_class = Class.new(CarrierWave::Uploader::Base) do
include CarrierWave::Compatibility::Paperclip
+
+ version :thumb
+ version :list
+
end
+
@model = mock('a model')
@model.stub!(:id).and_return(23)
+ @model.stub!(:ook).and_return('eek')
+ @model.stub!(:money).and_return('monkey.png')
+
@uploader = @uploader_class.new(@model, :monkey)
end
@@ -47,6 +55,86 @@ module Rails; end unless defined?(Rails)
@uploader.stub!(:paperclip_path).and_return("/foo/:id_partition/bar")
@uploader.store_path("monkey.png").should == "/foo/000/000/023/bar"

Since Ruby 1.8 doesn't have ordered hashes, this particular spec example occasionally fails on 1.8.7. That's because sometimes the mapping for :id is used instead of the mapping for id_partition, causing the #store_path to return "/foo/23_partition/bar". Whoops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
+
+ it "should interpolate the basename" do
+ @uploader.stub!(:paperclip_path).and_return("/foo/:basename/bar")
+ @uploader.store_path("monkey.png").should == "/foo/monkey/bar"
+ end
+
+ it "should interpolate the extension" do
+ @uploader.stub!(:paperclip_path).and_return("/foo/:extension/bar")
+ @uploader.store_path("monkey.png").should == "/foo/png/bar"
+ end
+
end
+ describe '.interpolate' do
+ before do
+ @uploader_class.interpolate :ook do |custom, style|
+ custom.model.ook
+ end
+
+
+ @uploader_class.interpolate :aak do |model, style|
+ style
+ end
+ end
+
+ it 'should allow you to add custom interpolations' do
+ @uploader.stub!(:paperclip_path).and_return("/foo/:id/:ook")
+ @uploader.store_path("monkey.png").should == '/foo/23/eek'
+ end
+
+ it 'mimics paperclips arguments' do
+ @uploader.stub!(:paperclip_path).and_return("/foo/:aak")
+ @uploader.store_path("monkey.png").should == '/foo/original'
+ end
+
+ context 'when multiple uploaders include the compatibility module' do
+ before do
+ @uploader_class_other = Class.new(CarrierWave::Uploader::Base) do
+ include CarrierWave::Compatibility::Paperclip
+
+ version :thumb
+ version :list
+ end
+
+ @uploader = @uploader_class_other.new(@model, :monkey)
+ end
+
+ it 'should not share custom interpolations' do
+ @uploader.stub!(:paperclip_path).and_return("/foo/:id/:ook")
+ @uploader.store_path('monkey.jpg').should == '/foo/23/:ook'
+ end
+
+ end
+
+ context 'when there are multiple versions' do
+ before do
+ @complex_uploader_class = Class.new(CarrierWave::Uploader::Base) do
+ include CarrierWave::Compatibility::Paperclip
+
+ interpolate :ook do |model, style|
+ 'eek'
+ end
+
+ version :thumb
+ version :list
+
+ def paperclip_path
+ "#{public_path}/foo/:ook/:id/:style"
+ end
+ end
+
+ @uploader = @complex_uploader_class.new(@model, :monkey)
+ end
+
+ it 'should interpolate for all versions correctly' do
+ @file = File.open(file_path('test.jpg'))
+ @uploader.store!(@file)
+ @uploader.thumb.path.should == "#{public_path}/foo/eek/23/thumb"
+ @uploader.list.path.should == "#{public_path}/foo/eek/23/list"
+ end
+ end
+ end
end
Something went wrong with that request. Please try again.