Permalink
Browse files

[#940] Raise error when style defined & not in path

* Add StyleTokenNotFound in Errors
* Adjust tests to make sure :style present where not currently defined
  • Loading branch information...
Joel Oliveira
Joel Oliveira committed Jun 29, 2012
1 parent 029fe02 commit a5d6189de324796ad4e79477d043a09e86c47e06
Showing with 37 additions and 11 deletions.
  1. +12 −0 lib/paperclip/attachment.rb
  2. +5 −0 lib/paperclip/errors.rb
  3. +2 −2 test/attachment_test.rb
  4. +1 −1 test/storage/s3_test.rb
  5. +17 −8 test/style_test.rb
@@ -77,6 +77,7 @@ def initialize(name, instance, options = {})
@source_file_options = options[:source_file_options]
@whiny = options[:whiny]
+ initialize_path
initialize_storage
end
@@ -358,6 +359,17 @@ def valid_assignment? file #:nodoc:
file.nil? || (file.respond_to?(:original_filename) && file.respond_to?(:content_type))
end
+ def initialize_path
+ if style_not_in_custom_path?
+ raise(Paperclip::Errors::StyleTokenNotFound)
+ end
+ end
+
+ def style_not_in_custom_path?
+ @options[:path] != Paperclip::Attachment.default_options[:path] &&
+ @options[:styles].present? && !@options[:path].include?(':style')
+ end
+
def initialize_storage #:nodoc:
storage_class_name = @options[:storage].to_s.downcase.camelize
begin
View
@@ -9,6 +9,11 @@ module Errors
class StorageMethodNotFound < Paperclip::Error
end
+ # Will be thrown when there are custom styles and those styles aren't
+ # included in the path
+ class StyleTokenNotFound < Paperclip::Error
+ end
+
# Will be thrown when a command or executable is not found.
class CommandNotFoundError < Paperclip::Error
end
View
@@ -309,15 +309,15 @@ class AttachmentTest < Test::Unit::TestCase
context "An attachment with a default style and an extension interpolation" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => { :default => ["100x100", :png] },
:default_style => :default
@file = StringIO.new("...")
@file.stubs(:original_filename).returns("file.jpg")
end
should "return the right extension for the path" do
@attachment.assign(@file)
- assert_equal "file.png", @attachment.path
+ assert_equal "default_file.png", @attachment.path
end
end
View
@@ -228,7 +228,7 @@ def teardown
rebuild_model :styles => { :large => ['500x500#', :jpg] },
:storage => :s3,
:bucket => "bucket",
- :path => ":attachment/:basename.:extension",
+ :path => ":attachment/:style/:basename.:extension",
:s3_credentials => {
'access_key_id' => "12345",
'secret_access_key' => "54321"
View
@@ -5,7 +5,7 @@ class StyleTest < Test::Unit::TestCase
context "A style rule" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":basename_:style.:extension",
:styles => { :foo => {:geometry => "100x100#", :format => :png} },
:whiny => true
@style = @attachment.styles[:foo]
@@ -33,9 +33,18 @@ class StyleTest < Test::Unit::TestCase
end
end
+ context "A path not containing a style rule" do
+ should "generate a warning when a :style token isn't found" do
+ assert_raises(Paperclip::Errors::StyleTokenNotFound) do
+ @attachment = attachment :path => ":basename_:extension",
+ :styles => { :foo => {:geometry => "100x100#", :format => :png} }
+ end
+ end
+ end
+
context "A style rule with properties supplied as procs" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:whiny_thumbnails => true,
:processors => lambda {|a| [:test]},
:styles => {
@@ -64,7 +73,7 @@ class StyleTest < Test::Unit::TestCase
styles[:aslist] = ["100x100", :png]
styles[:ashash] = {:geometry => "100x100", :format => :png}
styles[:asstring] = "100x100"
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => styles
end
should "have the right number of styles" do
@@ -97,7 +106,7 @@ class StyleTest < Test::Unit::TestCase
context "An attachment with :convert_options" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => {:thumb => "100x100", :large => "400x400"},
:convert_options => {:all => "-do_stuff", :thumb => "-thumbnailize"}
@style = @attachment.styles[:thumb]
@@ -117,7 +126,7 @@ class StyleTest < Test::Unit::TestCase
context "An attachment with :source_file_options" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => {:thumb => "100x100", :large => "400x400"},
:source_file_options => {:all => "-density 400", :thumb => "-depth 8"}
@style = @attachment.styles[:thumb]
@@ -137,7 +146,7 @@ class StyleTest < Test::Unit::TestCase
context "A style rule with its own :processors" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => {
:foo => {
:geometry => "100x100#",
@@ -162,7 +171,7 @@ class StyleTest < Test::Unit::TestCase
context "A style rule with :processors supplied as procs" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => {
:foo => {
:geometry => "100x100#",
@@ -184,7 +193,7 @@ class StyleTest < Test::Unit::TestCase
context "An attachment with :convert_options and :source_file_options in :styles" do
setup do
- @attachment = attachment :path => ":basename.:extension",
+ @attachment = attachment :path => ":style_:basename.:extension",
:styles => {
:thumb => "100x100",
:large => {:geometry => "400x400",

3 comments on commit a5d6189

This seems to be saying that whenever you define a custom interpolation style, you have to use it? I have like 20 different models with paperclip attachments, not all of them use the same styles & this is breaking my code.

Argh, confusion between interpolations and styles (usually thumbnail vs. original) is frustrating too

Contributor

sikachu replied Jul 14, 2012

Right ... this would means if you have multiple styles, you need to have :style in the :path. I think this is a dumbproof so you won't save two styles into the same path.

Anyhow, I'll push another fix with a configuration to ignore the path check. Stay tuned.

Aah, I see. I've been using a custom interpolation that solved this:

Paperclip.interpolates :my_style do |attachment, style_name|
style_name ||= attachment.default_style
style_name == :original ? '' : "#{style_name == 'big' ? '' : '-'}#{style_name}"
end

Please sign in to comment.