Merge pull request #2056 from dgynn/pr_interpolations_tuning_v4.3

Cache interpolator methods and reduce memory allocations - rebased on v4.3

lib/paperclip/attachment.rb

@@ -68,7 +68,8 @@ module Paperclip
     # +url_generator+ - the object used to generate URLs, using the interpolator. Defaults to Paperclip::UrlGenerator
     # +url_generator+ - the object used to generate URLs, using the interpolator. Defaults to Paperclip::UrlGenerator
     # +escape_url+ - Perform URI escaping to URLs. Defaults to true
     # +escape_url+ - Perform URI escaping to URLs. Defaults to true
     def initialize(name, instance, options = {})
     def initialize(name, instance, options = {})
-      @name              = name
+      @name              = name.to_sym
+      @name_string       = name.to_s
       @instance          = instance
       @instance          = instance
 
 
       options = self.class.default_options.deep_merge(options)
       options = self.class.default_options.deep_merge(options)
@@ -365,7 +366,7 @@ module Paperclip
     # instance_write(:file_name, "me.jpg") will write "me.jpg" to the instance's
     # instance_write(:file_name, "me.jpg") will write "me.jpg" to the instance's
     # "avatar_file_name" field (assuming the attachment is called avatar).
     # "avatar_file_name" field (assuming the attachment is called avatar).
     def instance_write(attr, value)
     def instance_write(attr, value)
-      setter = :"#{name}_#{attr}="
+      setter = :"#{@name_string}_#{attr}="
       if instance.respond_to?(setter)
       if instance.respond_to?(setter)
         instance.send(setter, value)
         instance.send(setter, value)
       end
       end
@@ -374,7 +375,7 @@ module Paperclip
     # Reads the attachment-specific attribute on the instance. See instance_write
     # Reads the attachment-specific attribute on the instance. See instance_write
     # for more details.
     # for more details.
     def instance_read(attr)
     def instance_read(attr)
-      getter = :"#{name}_#{attr}"
+      getter = :"#{@name_string}_#{attr}"
       if instance.respond_to?(getter)
       if instance.respond_to?(getter)
         instance.send(getter)
         instance.send(getter)
       end
       end
@@ -402,8 +403,8 @@ module Paperclip
 
 
     def ensure_required_accessors! #:nodoc:
     def ensure_required_accessors! #:nodoc:
       %w(file_name).each do |field|
       %w(file_name).each do |field|
-        unless @instance.respond_to?("#{name}_#{field}") && @instance.respond_to?("#{name}_#{field}=")
-          raise Paperclip::Error.new("#{@instance.class} model missing required attr_accessor for '#{name}_#{field}'")
+        unless @instance.respond_to?("#{@name_string}_#{field}") && @instance.respond_to?("#{@name_string}_#{field}=")
+          raise Paperclip::Error.new("#{@instance.class} model missing required attr_accessor for '#{@name_string}_#{field}'")
         end
         end
       end
       end
     end
     end

lib/paperclip/interpolations.rb

@@ -10,6 +10,7 @@ module Paperclip
     # and is not intended for normal use.
     # and is not intended for normal use.
     def self.[]= name, block
     def self.[]= name, block
       define_method(name, &block)
       define_method(name, &block)
+      @interpolators_cache = nil
     end
     end
 
 
     # Hash access of interpolations. Included only for compatibility,
     # Hash access of interpolations. Included only for compatibility,
@@ -20,7 +21,7 @@ module Paperclip
 
 
     # Returns a sorted list of all interpolations.
     # Returns a sorted list of all interpolations.
     def self.all
     def self.all
-      self.instance_methods(false).sort
+      self.instance_methods(false).sort!
     end
     end
 
 
     # Perform the actual interpolation. Takes the pattern to interpolate
     # Perform the actual interpolation. Takes the pattern to interpolate
@@ -29,11 +30,15 @@ module Paperclip
     # an interpolation pattern for Paperclip to use.
     # an interpolation pattern for Paperclip to use.
     def self.interpolate pattern, *args
     def self.interpolate pattern, *args
       pattern = args.first.instance.send(pattern) if pattern.kind_of? Symbol
       pattern = args.first.instance.send(pattern) if pattern.kind_of? Symbol
-      all.reverse.inject(pattern) do |result, tag|
-        result.gsub(/:#{tag}/) do |match|
-          send( tag, *args )
-        end
+      result = pattern.dup
+      interpolators_cache.each do |method, token|
+        result.gsub!(token) { send(method, *args) } if result.include?(token)
       end
       end
+      result
+    end
+
+    def self.interpolators_cache
+      @interpolators_cache ||= all.reverse!.map! { |method| [method, ":#{method}"] }
     end
     end
 
 
     def self.plural_cache
     def self.plural_cache
@@ -42,7 +47,7 @@ module Paperclip
 
 
     # Returns the filename, the same way as ":basename.:extension" would.
     # Returns the filename, the same way as ":basename.:extension" would.
     def filename attachment, style_name
     def filename attachment, style_name
-      [ basename(attachment, style_name), extension(attachment, style_name) ].reject(&:blank?).join(".")
+      [ basename(attachment, style_name), extension(attachment, style_name) ].delete_if(&:empty?).join(".".freeze)
     end
     end
 
 
     # Returns the interpolated URL. Will raise an error if the url itself
     # Returns the interpolated URL. Will raise an error if the url itself
@@ -85,12 +90,12 @@ module Paperclip
     # all class names. Calling #class will return the expected class.
     # all class names. Calling #class will return the expected class.
     def class attachment = nil, style_name = nil
     def class attachment = nil, style_name = nil
       return super() if attachment.nil? && style_name.nil?
       return super() if attachment.nil? && style_name.nil?
-      plural_cache.underscore_and_pluralize(attachment.instance.class.to_s)
+      plural_cache.underscore_and_pluralize_class(attachment.instance.class)
     end
     end
 
 
     # Returns the basename of the file. e.g. "file" for "file.jpg"
     # Returns the basename of the file. e.g. "file" for "file.jpg"
     def basename attachment, style_name
     def basename attachment, style_name
-      attachment.original_filename.gsub(/#{Regexp.escape(File.extname(attachment.original_filename))}\Z/, "")
+      File.basename(attachment.original_filename, ".*".freeze)
     end
     end
 
 
     # Returns the extension of the file. e.g. "jpg" for "file.jpg"
     # Returns the extension of the file. e.g. "jpg" for "file.jpg"
@@ -98,7 +103,7 @@ module Paperclip
     # of the actual extension.
     # of the actual extension.
     def extension attachment, style_name
     def extension attachment, style_name
       ((style = attachment.styles[style_name.to_s.to_sym]) && style[:format]) ||
       ((style = attachment.styles[style_name.to_s.to_sym]) && style[:format]) ||
-        File.extname(attachment.original_filename).gsub(/\A\.+/, "")
+        File.extname(attachment.original_filename).sub(/\A\.+/, "".freeze)
     end
     end
 
 
     # Returns the dot+extension of the file. e.g. ".jpg" for "file.jpg"
     # Returns the dot+extension of the file. e.g. ".jpg" for "file.jpg"
@@ -106,7 +111,7 @@ module Paperclip
     # of the actual extension. If the extension is empty, no dot is added.
     # of the actual extension. If the extension is empty, no dot is added.
     def dotextension attachment, style_name
     def dotextension attachment, style_name
       ext = extension(attachment, style_name)
       ext = extension(attachment, style_name)
-      ext.empty? ? "" : ".#{ext}"
+      ext.empty? ? ext : ".#{ext}"
     end
     end
 
 
     # Returns an extension based on the content type. e.g. "jpeg" for
     # Returns an extension based on the content type. e.g. "jpeg" for
@@ -170,9 +175,9 @@ module Paperclip
     def id_partition attachment, style_name
     def id_partition attachment, style_name
       case id = attachment.instance.id
       case id = attachment.instance.id
       when Integer
       when Integer
-        ("%09d" % id).scan(/\d{3}/).join("/")
+        ("%09d".freeze % id).scan(/\d{3}/).join("/".freeze)
       when String
       when String
-        id.scan(/.{3}/).first(3).join("/")
+        id.scan(/.{3}/).first(3).join("/".freeze)
       else
       else
         nil
         nil
       end
       end
@@ -181,7 +186,7 @@ module Paperclip
     # Returns the pluralized form of the attachment name. e.g.
     # Returns the pluralized form of the attachment name. e.g.
     # "avatars" for an attachment of :avatar
     # "avatars" for an attachment of :avatar
     def attachment attachment, style_name
     def attachment attachment, style_name
-      plural_cache.pluralize(attachment.name.to_s.downcase)
+      plural_cache.pluralize_symbol(attachment.name)
     end
     end
 
 
     # Returns the style, or the default style if nil is supplied.
     # Returns the style, or the default style if nil is supplied.

lib/paperclip/interpolations/plural_cache.rb

@@ -2,15 +2,16 @@ module Paperclip
   module Interpolations
   module Interpolations
     class PluralCache
     class PluralCache
       def initialize
       def initialize
-        @cache = {}
+        @symbol_cache = {}.compare_by_identity
+        @klass_cache = {}.compare_by_identity
       end
       end
 
 
-      def pluralize(word)
-        @cache[word] ||= word.pluralize
+      def pluralize_symbol(symbol)
+        @symbol_cache[symbol] ||= symbol.to_s.downcase.pluralize
       end
       end
 
 
-      def underscore_and_pluralize(word)
-        @cache[word] ||= word.underscore.pluralize
+      def underscore_and_pluralize_class(klass)
+        @klass_cache[klass] ||= klass.name.underscore.pluralize
       end
       end
     end
     end
   end
   end

lib/paperclip/rails_environment.rb

@@ -15,7 +15,7 @@ module Paperclip
     private
     private
 
 
     def rails_exists?
     def rails_exists?
-      Object.const_defined?("Rails")
+      Object.const_defined?(:Rails)
     end
     end
 
 
     def rails_environment_exists?
     def rails_environment_exists?

lib/paperclip/storage/s3.rb

@@ -141,7 +141,7 @@ module Paperclip
             Proc.new do |style, attachment|
             Proc.new do |style, attachment|
               permission  = (@s3_permissions[style.to_s.to_sym] || @s3_permissions[:default])
               permission  = (@s3_permissions[style.to_s.to_sym] || @s3_permissions[:default])
               permission  = permission.call(attachment, style) if permission.respond_to?(:call)
               permission  = permission.call(attachment, style) if permission.respond_to?(:call)
-              (permission == :public_read) ? 'http' : 'https'
+              (permission == :public_read) ? 'http'.freeze : 'https'.freeze
             end
             end
           @s3_metadata = @options[:s3_metadata] || {}
           @s3_metadata = @options[:s3_metadata] || {}
           @s3_headers = {}
           @s3_headers = {}
@@ -157,9 +157,9 @@ module Paperclip
             @s3_server_side_encryption = @options[:s3_server_side_encryption]
             @s3_server_side_encryption = @options[:s3_server_side_encryption]
           end
           end
 
 
-          unless @options[:url].to_s.match(/\A:s3.*url\Z/) || @options[:url] == ":asset_host"
-            @options[:path] = path_option.gsub(/:url/, @options[:url]).gsub(/\A:rails_root\/public\/system/, '')
-            @options[:url]  = ":s3_path_url"
+          unless @options[:url].to_s.match(/\A:s3.*url\Z/) || @options[:url] == ":asset_host".freeze
+            @options[:path] = path_option.gsub(/:url/, @options[:url]).sub(/\A:rails_root\/public\/system/, "".freeze)
+            @options[:url]  = ":s3_path_url".freeze
           end
           end
           @options[:url] = @options[:url].inspect if @options[:url].is_a?(Symbol)
           @options[:url] = @options[:url].inspect if @options[:url].is_a?(Symbol)
 
 
@@ -167,16 +167,16 @@ module Paperclip
         end
         end
 
 
         Paperclip.interpolates(:s3_alias_url) do |attachment, style|
         Paperclip.interpolates(:s3_alias_url) do |attachment, style|
-          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_alias}/#{attachment.path(style).gsub(%r{\A/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_alias}/#{attachment.path(style).sub(%r{\A/}, "".freeze)}"
         end unless Paperclip::Interpolations.respond_to? :s3_alias_url
         end unless Paperclip::Interpolations.respond_to? :s3_alias_url
         Paperclip.interpolates(:s3_path_url) do |attachment, style|
         Paperclip.interpolates(:s3_path_url) do |attachment, style|
-          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_name}/#{attachment.bucket_name}/#{attachment.path(style).gsub(%r{\A/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_name}/#{attachment.bucket_name}/#{attachment.path(style).sub(%r{\A/}, "".freeze)}"
         end unless Paperclip::Interpolations.respond_to? :s3_path_url
         end unless Paperclip::Interpolations.respond_to? :s3_path_url
         Paperclip.interpolates(:s3_domain_url) do |attachment, style|
         Paperclip.interpolates(:s3_domain_url) do |attachment, style|
-          "#{attachment.s3_protocol(style, true)}//#{attachment.bucket_name}.#{attachment.s3_host_name}/#{attachment.path(style).gsub(%r{\A/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.bucket_name}.#{attachment.s3_host_name}/#{attachment.path(style).sub(%r{\A/}, "".freeze)}"
         end unless Paperclip::Interpolations.respond_to? :s3_domain_url
         end unless Paperclip::Interpolations.respond_to? :s3_domain_url
         Paperclip.interpolates(:asset_host) do |attachment, style|
         Paperclip.interpolates(:asset_host) do |attachment, style|
-          "#{attachment.path(style).gsub(%r{\A/}, "")}"
+          "#{attachment.path(style).sub(%r{\A/}, "".freeze)}"
         end unless Paperclip::Interpolations.respond_to? :asset_host
         end unless Paperclip::Interpolations.respond_to? :asset_host
       end
       end
 
 
@@ -197,7 +197,7 @@ module Paperclip
         host_name = @options[:s3_host_name]
         host_name = @options[:s3_host_name]
         host_name = host_name.call(self) if host_name.is_a?(Proc)
         host_name = host_name.call(self) if host_name.is_a?(Proc)
 
 
-        host_name || s3_credentials[:s3_host_name] || "s3.amazonaws.com"
+        host_name || s3_credentials[:s3_host_name] || "s3.amazonaws.com".freeze
       end
       end
 
 
       def s3_host_alias
       def s3_host_alias
@@ -286,7 +286,7 @@ module Paperclip
       end
       end
 
 
       def parse_credentials creds
       def parse_credentials creds
-        creds = creds.respond_to?('call') ? creds.call(self) : creds
+        creds = creds.respond_to?(:call) ? creds.call(self) : creds
         creds = find_credentials(creds).stringify_keys
         creds = find_credentials(creds).stringify_keys
         (creds[RailsEnvironment.get] || creds).symbolize_keys
         (creds[RailsEnvironment.get] || creds).symbolize_keys
       end
       end

paperclip.gemspec

@@ -34,12 +34,13 @@ Gem::Specification.new do |s|
   s.add_development_dependency('aws-sdk', '>= 1.5.7', "<= 2.0")
   s.add_development_dependency('aws-sdk', '>= 1.5.7', "<= 2.0")
   s.add_development_dependency('bourne')
   s.add_development_dependency('bourne')
   s.add_development_dependency('cucumber', '~> 1.3.18')
   s.add_development_dependency('cucumber', '~> 1.3.18')
-  s.add_development_dependency('aruba')
+  s.add_development_dependency('aruba', '~> 0.9.0')
   s.add_development_dependency('nokogiri')
   s.add_development_dependency('nokogiri')
   # Ruby version < 1.9.3 can't install capybara > 2.0.3.
   # Ruby version < 1.9.3 can't install capybara > 2.0.3.
   s.add_development_dependency('capybara')
   s.add_development_dependency('capybara')
   s.add_development_dependency('bundler')
   s.add_development_dependency('bundler')
-  s.add_development_dependency('fog', '~> 1.0')
+  s.add_development_dependency('fog-aws')
+  s.add_development_dependency('fog-local')
   s.add_development_dependency('launchy')
   s.add_development_dependency('launchy')
   s.add_development_dependency('rake')
   s.add_development_dependency('rake')
   s.add_development_dependency('fakeweb')
   s.add_development_dependency('fakeweb')

spec/paperclip/interpolations_spec.rb

@@ -24,15 +24,16 @@ describe Paperclip::Interpolations do
   end
   end
 
 
   it "returns the class of the instance" do
   it "returns the class of the instance" do
+    class Thing ; end
     attachment = mock
     attachment = mock
     attachment.expects(:instance).returns(attachment)
     attachment.expects(:instance).returns(attachment)
-    attachment.expects(:class).returns("Thing")
+    attachment.expects(:class).returns(Thing)
     assert_equal "things", Paperclip::Interpolations.class(attachment, :style)
     assert_equal "things", Paperclip::Interpolations.class(attachment, :style)
   end
   end
 
 
   it "returns the basename of the file" do
   it "returns the basename of the file" do
     attachment = mock
     attachment = mock
-    attachment.expects(:original_filename).returns("one.jpg").times(2)
+    attachment.expects(:original_filename).returns("one.jpg").times(1)
     assert_equal "one", Paperclip::Interpolations.basename(attachment, :style)
     assert_equal "one", Paperclip::Interpolations.basename(attachment, :style)
   end
   end
 
 
@@ -187,14 +188,14 @@ describe Paperclip::Interpolations do
   it "returns the filename as basename.extension" do
   it "returns the filename as basename.extension" do
     attachment = mock
     attachment = mock
     attachment.expects(:styles).returns({})
     attachment.expects(:styles).returns({})
-    attachment.expects(:original_filename).returns("one.jpg").times(3)
+    attachment.expects(:original_filename).returns("one.jpg").times(2)
     assert_equal "one.jpg", Paperclip::Interpolations.filename(attachment, :style)
     assert_equal "one.jpg", Paperclip::Interpolations.filename(attachment, :style)
   end
   end
 
 
   it "returns the filename as basename.extension when format supplied" do
   it "returns the filename as basename.extension when format supplied" do
     attachment = mock
     attachment = mock
     attachment.expects(:styles).returns({style: {format: :png}})
     attachment.expects(:styles).returns({style: {format: :png}})
-    attachment.expects(:original_filename).returns("one.jpg").times(2)
+    attachment.expects(:original_filename).returns("one.jpg").times(1)
     assert_equal "one.png", Paperclip::Interpolations.filename(attachment, :style)
     assert_equal "one.png", Paperclip::Interpolations.filename(attachment, :style)
   end
   end
 
 
@@ -249,4 +250,13 @@ describe Paperclip::Interpolations do
     value = Paperclip::Interpolations.interpolate(":notreal/:id/:attachment", :attachment, :style)
     value = Paperclip::Interpolations.interpolate(":notreal/:id/:attachment", :attachment, :style)
     assert_equal ":notreal/1234/attachments", value
     assert_equal ":notreal/1234/attachments", value
   end
   end
+
+  it "handles question marks" do
+    Paperclip.interpolates :foo? do
+      "bar"
+    end
+    Paperclip::Interpolations.expects(:fool).never
+    value = Paperclip::Interpolations.interpolate(":fo/:foo?")
+    assert_equal ":fo/bar", value
+  end
 end
 end

spec/paperclip/plural_cache_spec.rb

@@ -3,34 +3,35 @@ require 'spec_helper'
 describe 'Plural cache' do
 describe 'Plural cache' do
   it 'caches pluralizations' do
   it 'caches pluralizations' do
     cache = Paperclip::Interpolations::PluralCache.new
     cache = Paperclip::Interpolations::PluralCache.new
-    word = "box"
+    symbol = :box
 
 
-    word.expects(:pluralize).returns("boxes").once
-
-    cache.pluralize(word)
-    cache.pluralize(word)
+    first = cache.pluralize_symbol(symbol)
+    second = cache.pluralize_symbol(symbol)
+    expect(first).to equal(second)
   end
   end
 
 
   it 'caches pluralizations and underscores' do
   it 'caches pluralizations and underscores' do
+    class BigBox ; end
     cache = Paperclip::Interpolations::PluralCache.new
     cache = Paperclip::Interpolations::PluralCache.new
-    word = "BigBox"
-
-    word.expects(:pluralize).returns(word).once
-    word.expects(:underscore).returns(word).once
+    klass = BigBox
 
 
-    cache.underscore_and_pluralize(word)
-    cache.underscore_and_pluralize(word)
+    first = cache.underscore_and_pluralize_class(klass)
+    second = cache.underscore_and_pluralize_class(klass)
+    expect(first).to equal(second)
   end
   end
 
 
   it 'pluralizes words' do
   it 'pluralizes words' do
     cache = Paperclip::Interpolations::PluralCache.new
     cache = Paperclip::Interpolations::PluralCache.new
-    word = "box"
-    assert_equal "boxes", cache.pluralize(word)
+    symbol = :box
+
+    expect(cache.pluralize_symbol(symbol)).to eq("boxes")
   end
   end
 
 
-  it 'pluralizes and underscore words' do
+  it 'pluralizes and underscore class names' do
+    class BigBox ; end
     cache = Paperclip::Interpolations::PluralCache.new
     cache = Paperclip::Interpolations::PluralCache.new
-    word = "BigBox"
-    assert_equal "big_boxes", cache.underscore_and_pluralize(word)
+    klass = BigBox
+
+    expect(cache.underscore_and_pluralize_class(klass)).to eq("big_boxes")
   end
   end
 end
 end

spec/paperclip/storage/fog_spec.rb

 require 'spec_helper'
 require 'spec_helper'
-require 'fog'
+require 'fog/aws'
+require 'fog/local'
 require 'timecop'
 require 'timecop'
 
 
 describe Paperclip::Storage::Fog do
 describe Paperclip::Storage::Fog do