From ac4408acb36c5081dfed923512f57dfc88c71f34 Mon Sep 17 00:00:00 2001 From: Luke Griffiths Date: Fri, 3 Feb 2012 10:58:44 -0500 Subject: [PATCH 1/5] added ctags file 'tags' to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a604d9d80..fe1f103b5 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ capybara*.html *SPIKE* *emfile.lock +tags From 5f8140d1d2ca5f8cf133ee96dd7ee958dfd36ce7 Mon Sep 17 00:00:00 2001 From: Luke Griffiths Date: Fri, 3 Feb 2012 14:33:33 -0500 Subject: [PATCH 2/5] passing AttachmentOptions instance instead of hash to Attachment.new --- lib/paperclip.rb | 3 ++- lib/paperclip/attachment_options.rb | 19 +++++++++++++++ test/attachment_options_test.rb | 34 +++++++++++++++++++++++++++ test/helper.rb | 9 +++++-- test/mocks/mock_attachment_options.rb | 5 ++++ test/paperclip_test.rb | 7 ++++++ 6 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 lib/paperclip/attachment_options.rb create mode 100644 test/attachment_options_test.rb create mode 100644 test/mocks/mock_attachment_options.rb diff --git a/lib/paperclip.rb b/lib/paperclip.rb index db46ffa6c..5db1db1aa 100644 --- a/lib/paperclip.rb +++ b/lib/paperclip.rb @@ -37,6 +37,7 @@ require 'paperclip/interpolations' require 'paperclip/style' require 'paperclip/attachment' +require 'paperclip/attachment_options' require 'paperclip/storage' require 'paperclip/callback_compatibility' require 'paperclip/missing_attachment_styles' @@ -325,7 +326,7 @@ def has_attached_file name, options = {} self.attachment_definitions = self.attachment_definitions.dup end - attachment_definitions[name] = {:validations => []}.merge(options) + attachment_definitions[name] = Paperclip::AttachmentOptions.new(options) Paperclip.classes_with_attachments << self.name Paperclip.check_for_url_clash(name,attachment_definitions[name][:url],self.name) diff --git a/lib/paperclip/attachment_options.rb b/lib/paperclip/attachment_options.rb new file mode 100644 index 000000000..4bad0de9e --- /dev/null +++ b/lib/paperclip/attachment_options.rb @@ -0,0 +1,19 @@ +module Paperclip + class AttachmentOptions + def initialize(options) + @options = {:validations => []}.merge(options) + end + + def [](key) + @options[key] + end + + def []=(key, value) + @options[key] = value + end + + def to_hash + @options + end + end +end diff --git a/test/attachment_options_test.rb b/test/attachment_options_test.rb new file mode 100644 index 000000000..0df4cb19f --- /dev/null +++ b/test/attachment_options_test.rb @@ -0,0 +1,34 @@ +require './test/helper' + +class AttachmentOptionsTest < Test::Unit::TestCase + should "exist" do + Paperclip::AttachmentOptions + end + + should "add a default empty validations" do + options = {:arbi => :trary} + expected = {:validations => []}.merge(options) + actual = Paperclip::AttachmentOptions.new(options).to_hash + assert_equal expected, actual + end + + should "respond to []" do + Paperclip::AttachmentOptions.new({})[:foo] + end + + should "deliver the specified options through []" do + intended_options = {:specific_key => "specific value"} + attachment_options = Paperclip::AttachmentOptions.new(intended_options) + assert_equal "specific value", attachment_options[:specific_key] + end + + should "respond to []=" do + Paperclip::AttachmentOptions.new({})[:foo] = "bar" + end + + should "remember options set with []=" do + attachment_options = Paperclip::AttachmentOptions.new({}) + attachment_options[:foo] = "bar" + assert_equal "bar", attachment_options[:foo] + end +end diff --git a/test/helper.rb b/test/helper.rb index 8a8e353c9..b92ee80a7 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -54,10 +54,15 @@ def setup ActiveRecord::Base.establish_connection(config['test']) Paperclip.options[:logger] = ActiveRecord::Base.logger -Dir[File.join(File.dirname(__FILE__), 'support','*')].each do |f| - require f +def require_everything_in_directory(directory_name) + Dir[File.join(File.dirname(__FILE__), directory_name, '*')].each do |f| + require f + end end +require_everything_in_directory('support') +require_everything_in_directory('mocks') + def reset_class class_name ActiveRecord::Base.send(:include, Paperclip::Glue) Object.send(:remove_const, class_name) rescue nil diff --git a/test/mocks/mock_attachment_options.rb b/test/mocks/mock_attachment_options.rb new file mode 100644 index 000000000..0e7e3636c --- /dev/null +++ b/test/mocks/mock_attachment_options.rb @@ -0,0 +1,5 @@ +class MockAttachmentOptions + def [](key) + nil + end +end diff --git a/test/paperclip_test.rb b/test/paperclip_test.rb index c11357c76..ead0a9012 100644 --- a/test/paperclip_test.rb +++ b/test/paperclip_test.rb @@ -98,6 +98,13 @@ class Dummy2 < ActiveRecord::Base end end + context "An ActiveRecord model responding to has_attached_file" do + should "pass the options to Paperclip::AttachmentOptions.new" do + Paperclip::AttachmentOptions.expects(:new).with({"test" => "hash"}).returns(MockAttachmentOptions.new) + rebuild_model "test" => "hash" + end + end + context "An ActiveRecord model with an 'avatar' attachment" do setup do rebuild_model :path => "tmp/:class/omg/:style.:extension" From 675dd9a374376ed273695b2e621d3df5366e3b3c Mon Sep 17 00:00:00 2001 From: Luke Griffiths Date: Fri, 3 Feb 2012 16:18:43 -0500 Subject: [PATCH 3/5] AttachmentOptions now inherits from Hash --- lib/paperclip/attachment_options.rb | 19 +++++-------------- test/attachment_options_test.rb | 11 +++++++++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/paperclip/attachment_options.rb b/lib/paperclip/attachment_options.rb index 4bad0de9e..2d05ff84a 100644 --- a/lib/paperclip/attachment_options.rb +++ b/lib/paperclip/attachment_options.rb @@ -1,19 +1,10 @@ module Paperclip - class AttachmentOptions + class AttachmentOptions < Hash def initialize(options) - @options = {:validations => []}.merge(options) - end - - def [](key) - @options[key] - end - - def []=(key, value) - @options[key] = value - end - - def to_hash - @options + options = {:validations => []}.merge(options) + options.each do |k, v| + self.[]=(k, v) + end end end end diff --git a/test/attachment_options_test.rb b/test/attachment_options_test.rb index 0df4cb19f..205ae334a 100644 --- a/test/attachment_options_test.rb +++ b/test/attachment_options_test.rb @@ -5,6 +5,11 @@ class AttachmentOptionsTest < Test::Unit::TestCase Paperclip::AttachmentOptions end + should "be a Hash" do + attachment_options = Paperclip::AttachmentOptions.new({}) + assert attachment_options.is_a?(Hash), "attachment_options is not a Hash" + end + should "add a default empty validations" do options = {:arbi => :trary} expected = {:validations => []}.merge(options) @@ -12,6 +17,12 @@ class AttachmentOptionsTest < Test::Unit::TestCase assert_equal expected, actual end + should "not override validations if passed to initializer" do + options = {:validations => "something"} + attachment_options = Paperclip::AttachmentOptions.new(options) + assert_equal "something", attachment_options[:validations] + end + should "respond to []" do Paperclip::AttachmentOptions.new({})[:foo] end From 1f989aef35e11268f357b2d3a2c5e0e3c0de8379 Mon Sep 17 00:00:00 2001 From: Luke Griffiths Date: Fri, 3 Feb 2012 17:03:55 -0500 Subject: [PATCH 4/5] responding to code review comments --- test/attachment_options_test.rb | 11 +++-------- test/paperclip_test.rb | 7 ------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/test/attachment_options_test.rb b/test/attachment_options_test.rb index 205ae334a..1ac8ea757 100644 --- a/test/attachment_options_test.rb +++ b/test/attachment_options_test.rb @@ -1,13 +1,8 @@ require './test/helper' class AttachmentOptionsTest < Test::Unit::TestCase - should "exist" do - Paperclip::AttachmentOptions - end - should "be a Hash" do - attachment_options = Paperclip::AttachmentOptions.new({}) - assert attachment_options.is_a?(Hash), "attachment_options is not a Hash" + assert_kind_of Hash, Paperclip::AttachmentOptions.new({}) end should "add a default empty validations" do @@ -24,7 +19,7 @@ class AttachmentOptionsTest < Test::Unit::TestCase end should "respond to []" do - Paperclip::AttachmentOptions.new({})[:foo] + assert Paperclip::AttachmentOptions.new({}).respond_to?(:[]) end should "deliver the specified options through []" do @@ -34,7 +29,7 @@ class AttachmentOptionsTest < Test::Unit::TestCase end should "respond to []=" do - Paperclip::AttachmentOptions.new({})[:foo] = "bar" + assert Paperclip::AttachmentOptions.new({}).respond_to?(:[]=) end should "remember options set with []=" do diff --git a/test/paperclip_test.rb b/test/paperclip_test.rb index ead0a9012..c11357c76 100644 --- a/test/paperclip_test.rb +++ b/test/paperclip_test.rb @@ -98,13 +98,6 @@ class Dummy2 < ActiveRecord::Base end end - context "An ActiveRecord model responding to has_attached_file" do - should "pass the options to Paperclip::AttachmentOptions.new" do - Paperclip::AttachmentOptions.expects(:new).with({"test" => "hash"}).returns(MockAttachmentOptions.new) - rebuild_model "test" => "hash" - end - end - context "An ActiveRecord model with an 'avatar' attachment" do setup do rebuild_model :path => "tmp/:class/omg/:style.:extension" From 9eda1bbec7704f28f4c54a840de64c63adda43f9 Mon Sep 17 00:00:00 2001 From: Luke Griffiths Date: Fri, 3 Feb 2012 17:08:06 -0500 Subject: [PATCH 5/5] removed unused test/mocks directory and contents --- test/helper.rb | 1 - test/mocks/mock_attachment_options.rb | 5 ----- 2 files changed, 6 deletions(-) delete mode 100644 test/mocks/mock_attachment_options.rb diff --git a/test/helper.rb b/test/helper.rb index b92ee80a7..8ed11886b 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -61,7 +61,6 @@ def require_everything_in_directory(directory_name) end require_everything_in_directory('support') -require_everything_in_directory('mocks') def reset_class class_name ActiveRecord::Base.send(:include, Paperclip::Glue) diff --git a/test/mocks/mock_attachment_options.rb b/test/mocks/mock_attachment_options.rb deleted file mode 100644 index 0e7e3636c..000000000 --- a/test/mocks/mock_attachment_options.rb +++ /dev/null @@ -1,5 +0,0 @@ -class MockAttachmentOptions - def [](key) - nil - end -end