Attachment#url's second argument should be a hash, but can be a boole…

…an for backwards' sake

lib/paperclip.rb

@@ -387,10 +387,9 @@ def validates_attachment_thumbnails name, options = {}
     # * +unless+: Same as +if+ but validates if lambda or method returns false.
     def validates_attachment_presence name, options = {}
       message = options[:message] || :empty
-      validates_presence_of :"#{name}_file_name",
-                            :message   => message,
-                            :if        => options[:if],
-                            :unless    => options[:unless]
+      validates_each :"#{name}_file_name" do |record, attr, value|
+        record.errors.add(name, message) if attr.blank?
+      end
     end
 
     # Places ActiveRecord-style validations on the content type of the file

lib/paperclip/attachment.rb

@@ -128,12 +128,13 @@ def assign uploaded_file
     # grained security.  This is not recommended if you don't need the
     # security, however, for performance reasons. Set use_timestamp to false
     # if you want to stop the attachment update time appended to the url
-    def url(style_name = default_style, use_timestamp = @options.use_timestamp)
-      default_url = @options.default_url.is_a?(Proc) ? @options.default_url.call(self) : @options.default_url
-      url = original_filename.nil? ? interpolate(default_url, style_name) : interpolate(@options.url, style_name)
+    def url(style_name = default_style, options = {})
+      options = handle_url_options(options)
+      url = interpolate(most_appropriate_url, style_name)
 
-      url << (url.include?("?") ? "&" : "?") + updated_at.to_s if use_timestamp && updated_at
-      url.respond_to?(:escape) ? url.escape : URI.escape(url)
+      url = url_timestamp(url) if options[:timestamp]
+      url = escape_url(url)    if options[:escape]
+      url
     end
 
     # Returns the path of the attachment as defined by the :path option. If the
@@ -320,6 +321,44 @@ def instance_read(attr)
 
     private
 
+    def handle_url_options(options)
+      timestamp = extract_timestamp(options)
+      options = {} if options == true || options == false
+      options[:timestamp] = timestamp
+      options[:escape] = true if options[:escape].nil?
+      options
+    end
+
+    def extract_timestamp(options)
+      possibilities = [((options == true || options == false) ? options : nil),
+                       (options.respond_to?(:[]) ? options[:timestamp] : nil),
+                       @options.use_timestamp]
+      possibilities.find{|n| !n.nil? }
+    end
+
+    def default_url
+      return @options.default_url.call(self) if @options.default_url.is_a?(Proc)
+      @options.default_url
+    end
+
+    def most_appropriate_url
+      if original_filename.nil?
+        default_url
+      else
+        @options.url
+      end
+    end
+
+    def url_timestamp(url)
+      return url unless updated_at
+      delimiter_char = url.include?("?") ? "&" : "?"
+      "#{url}#{delimiter_char}#{updated_at.to_s}"
+    end
+
+    def escape_url(url)
+      url.respond_to?(:escape) ? url.escape : URI.escape(url)
+    end
+
     def ensure_required_accessors! #:nodoc:
       %w(file_name).each do |field|
         unless @instance.respond_to?("#{name}_#{field}") && @instance.respond_to?("#{name}_#{field}=")

test/attachment_test.rb

@@ -160,6 +160,36 @@ class AttachmentTest < Test::Unit::TestCase
     end
   end
 
+  context "An attachment" do
+    setup do
+      @file = StringIO.new("...")
+    end
+
+    context "using default time zone" do
+      setup do
+        rebuild_model :url => "X"
+        @dummy = Dummy.new
+        @dummy.avatar = @file
+      end
+
+      should "generate a url with a timestamp when passing true" do
+        assert_equal "X?#{@dummy.avatar_updated_at.to_i.to_s}", @dummy.avatar.url(:style, true)
+      end
+
+      should "not generate a url with a timestamp when passing false" do
+        assert_equal "X", @dummy.avatar.url(:style, false)
+      end
+
+      should "generate a url with a timestamp when setting a timestamp option" do
+        assert_equal "X?#{@dummy.avatar_updated_at.to_i.to_s}", @dummy.avatar.url(:style, :timestamp => true)
+      end
+
+      should "not generate a url with a timestamp when setting a timestamp option to false" do
+        assert_equal "X", @dummy.avatar.url(:style, :timestamp => false)
+      end
+    end
+  end
+
   context "An attachment with :hash interpolations" do
     setup do
       @file = StringIO.new("...")