Commit 716f1596 by Simon Coffey Committed by Jon Yurek

Check attachment paths for duplicates, not URLs

As per discussion in issue #660, attachments using alias interpolations
for the attachment URL mistakenly get flagged as being duplicates,
although the interpolation expands to a unique location.

This commit (partially) addresses the issue by checking the attachment path
instead. However, the problem will remain for users who use
interpolations other than :class to distinguish attachment paths for
different models.
parent 8d28a1e8
...@@ -179,7 +179,7 @@ module Paperclip ...@@ -179,7 +179,7 @@ module Paperclip
attachment_definitions[name] = Paperclip::AttachmentOptions.new(options) attachment_definitions[name] = Paperclip::AttachmentOptions.new(options)
Paperclip.classes_with_attachments << self.name Paperclip.classes_with_attachments << self.name
Paperclip.check_for_url_clash(name,attachment_definitions[name][:url],self.name) Paperclip.check_for_path_clash(name,attachment_definitions[name][:path],self.name)
after_save :save_attached_files after_save :save_attached_files
before_destroy :prepare_for_destroy before_destroy :prepare_for_destroy
......
...@@ -49,13 +49,13 @@ module Paperclip ...@@ -49,13 +49,13 @@ module Paperclip
end end
end end
def check_for_url_clash(name,url,klass) def check_for_path_clash(name,path,klass)
@names_url ||= {} @names_path ||= {}
default_url = url || Attachment.default_options[:url] default_path = path || Attachment.default_options[:path]
if @names_url[name] && @names_url[name][:url] == default_url && @names_url[name][:class] != klass && @names_url[name][:url] !~ /:class/ if @names_path[name] && @names_path[name][:path] == default_path && @names_path[name][:class] != klass && @names_path[name][:path] !~ /:class/
log("Duplicate URL for #{name} with #{default_url}. This will clash with attachment defined in #{@names_url[name][:class]} class") log("Duplicate path for #{name} with #{default_path}. This will clash with attachment defined in #{@names_path[name][:class]} class")
end end
@names_url[name] = {:url => default_url, :class => klass} @names_path[name] = {:path => default_path, :class => klass}
end end
def reset_duplicate_clash_check! def reset_duplicate_clash_check!
......
...@@ -114,24 +114,34 @@ class PaperclipTest < Test::Unit::TestCase ...@@ -114,24 +114,34 @@ class PaperclipTest < Test::Unit::TestCase
end end
end end
should "generate warning if attachment is redefined with the same url string" do should "generate warning if attachment is redefined with the same path string" do
expected_log_msg = "Duplicate URL for blah with /system/:id/:style/:filename. This will clash with attachment defined in Dummy class" expected_log_msg = "Duplicate path for blah with /system/:id/:style/:filename. This will clash with attachment defined in Dummy class"
Paperclip.expects(:log).with(expected_log_msg) Paperclip.expects(:log).with(expected_log_msg)
Dummy.class_eval do Dummy.class_eval do
has_attached_file :blah, :url => '/system/:id/:style/:filename' has_attached_file :blah, :path => '/system/:id/:style/:filename'
end end
Dummy2.class_eval do Dummy2.class_eval do
has_attached_file :blah, :url => '/system/:id/:style/:filename' has_attached_file :blah, :path => '/system/:id/:style/:filename'
end end
end end
should "not generate warning if attachment is redifined with the same url string but has :class in it" do should "not generate warning if attachment is redefined with the same path string but has :class in it" do
Paperclip.expects(:log).never Paperclip.expects(:log).never
Dummy.class_eval do Dummy.class_eval do
has_attached_file :blah, :url => "/system/:class/:attachment/:id/:style/:filename" has_attached_file :blah, :path => "/system/:class/:attachment/:id/:style/:filename"
end end
Dummy2.class_eval do Dummy2.class_eval do
has_attached_file :blah, :url => "/system/:class/:attachment/:id/:style/:filename" has_attached_file :blah, :path => "/system/:class/:attachment/:id/:style/:filename"
end
end
should "not generate warning if attachment is defined with the same URL string" do
Paperclip.expects(:log).never
Dummy.class_eval do
has_attached_file :blah, :path => "/system/:class/:attachment/:id/:style/:filename", :url => ":s3_alias_url"
end
Dummy2.class_eval do
has_attached_file :blah, :path => "/system/:class/:attachment/:id/:style/:filename", :url => ":s3_alias_url"
end end
end end
end end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment