Fixed some bugs, refactored file iterator, changed :thumbnails to :styles

git-svn-id: https://svn.thoughtbot.com/plugins/paperclip/trunk@256 7bbfaf0e-4d1d-0410-9690-a8bb5f8ef2aa

lib/paperclip.rb

@@ -79,8 +79,7 @@ module Paperclip
     #   is interpolated just as the url is. The default value is "/:class/:attachment/missing_:style.png"
     #     has_attached_file :avatar, :missing_url => "/images/default_:style_avatar.png"
     #     User.new.avatar_url(:small) # => "/images/default_small_avatar.png"
-    # * +attachment_type+: If this is set to :image (which it is, by default), Paperclip will attempt to make
-    #   thumbnails if they are specified.
+    # * +content_type+: The valid content types that this attachment may be.
     # * +thumbnails+: A hash of thumbnail styles and their geometries. You can find more about geometry strings
     #   at the ImageMagick website (http://www.imagemagick.org/script/command-line-options.php#resize). Paperclip
     #   also adds the "#" option (e.g. "50x50#"), which will resize the image to fit maximally inside

lib/paperclip/attachment.rb

@@ -24,6 +24,7 @@ module Paperclip
       self.extend(storage_module)
     end
     
+    # Sets the file managed by this instance. It also creates the thumbnails if the attachment is an image.
     def assign uploaded_file
       return destroy if uploaded_file.nil?
       return unless is_a_file? uploaded_file
@@ -34,37 +35,45 @@ module Paperclip
       self[:original]         = uploaded_file.read
       @dirty                  = true
 
-      if definition.attachment_type == :image
-        make_thumbnails_from(self[:original])
+      if definition.content_type == :image
+        convert( self[:original] )
       end
     end
 
-    def [](style)
+    def [](style) #:nodoc:
       @files[style]
     end
 
-    def []=(style, data)
+    def []=(style, data) #:nodoc:
       @files[style] = data
     end
 
-    def clear_files
+    def clear_files #:nodoc:
       @files = {}
       definition.styles.each{|style, geo| self[style] = nil }
       @dirty = false
     end
 
-    def for_attached_files
+    # Iterates over the files that are stored in memory and hands them to the
+    # supplied block. If no assignment has happened since either the object
+    # was instantiated or the last time it was saved, +nil+ will be passed as
+    # the data argument.
+    def each_unsaved
       @files.each do |style, data|
-        yield style, data
+        yield( style, data ) if data
       end
     end
     
+    def styles
+      @files.keys
+    end
+
+    # Returns true if the attachment has been assigned and not saved.
     def dirty?
       @dirty
     end
 
-    # Validations
-
+    # Runs any validations that have been defined on the attachment.
     def valid?
       definition.validations.each do |validation, constraints|
         send("validate_#{validation}", *constraints)
@@ -72,8 +81,8 @@ module Paperclip
       errors.uniq!.empty?
     end
 
-    # ActiveRecord Callbacks
-
+    # Writes (or deletes, if +nil+) the attachment. This is called automatically
+    # when the active record is saved; you do not need to call this yourself.
     def save
       write_attachment  if dirty?
       delete_attachment if @delete_on_save
@@ -81,6 +90,8 @@ module Paperclip
       clear_files
     end
 
+    # Queues up the attachment for destruction, but does not actually delete.
+    # The attachment will be deleted when the record is saved.
     def destroy(complain = false)
       returning true do
         @delete_on_save         = true
@@ -92,10 +103,16 @@ module Paperclip
       end
     end
 
+    # Immediately destroys the attachment. Typically called as an ActiveRecord
+    # callback on destroy. You shold not need to call this.
     def destroy!
       delete_attachment if definition.delete_on_destroy
     end
 
+    # Returns the public-facing URL of the attachment. If this record has not
+    # been saved or does not have an attachment, this method will return the
+    # "missing" url, which can be used to supply a default. This is what should
+    # be supplied to the +image_tag+ helper.
     def url style = nil
       style ||= definition.default_style
       pattern = if original_filename && instance.id
@@ -106,30 +123,37 @@ module Paperclip
       interpolate( style, pattern )
     end
 
+    # Returns the data contained by the attachment of a particular style. This
+    # should be used if you need to restrict permissions internally to the app.
     def read style = nil
       style ||= definition.default_style
       self[style] ? self[style] : read_attachment(style)  
     end
 
+    # Sets errors if there must be an attachment but isn't.
     def validate_existence *constraints
       definition.styles.keys.each do |style|
         errors << "requires a valid #{style} file." unless attachment_exists?(style)
       end
     end
 
+    # Sets errors if the file does not meet the file size constraints.
     def validate_size *constraints
       errors << "file too large. Must be under #{constraints.last} bytes." if original_file_size > constraints.last
       errors << "file too small. Must be over #{constraints.first} bytes." if original_file_size <= constraints.first
     end
 
+    # Returns true if all the files exist.
     def exists?(style)
       style ||= definition.default_style
       attachment_exists?(style)
     end
 
-    def make_thumbnails_from data
+    # Generates the thumbnails from the data supplied. Following this call, the data will
+    # be available from for_attached_files.
+    def convert data
       begin
-        definition.thumbnails.each do |style, geometry|
+        definition.styles.each do |style, geometry|
           self[style] = Thumbnail.make(geometry, data)
         end
       rescue PaperclipError => e
@@ -139,6 +163,13 @@ module Paperclip
       end
     end
 
+    # Returns a hash of procs that will perform the various interpolations for
+    # the path, url, and missing_url attachment options. The procs are used as
+    # arguments to gsub!, so the used will be replaced with the return value
+    # of the proc. You can add to this list by assigning to the hash:
+    #   Paperclip::Attachment.interpolations[:content_type] = lambda{|style, attachment| attachment.content_type }
+    #   attchment.interpolate("none", ":content_type")
+    #   # => "image/jpeg"
     def self.interpolations
       @interpolations ||= {
         :rails_root => lambda{|style, atch| RAILS_ROOT },
@@ -152,22 +183,29 @@ module Paperclip
       }
     end
 
+    # Searches for patterns in +source+ string supplied and replaces them with values
+    # returned by the procs in the interpolations hash.
     def interpolate style, source
       returning source.dup do |s|
         Attachment.interpolations.each do |key, proc|
-          s.gsub!(/:#{key}/){ proc.call(style, self) }
+          s.gsub!(/:#{key}/) do
+            proc.call(style, self) rescue ":#{key}"
+          end
         end
       end
     end
 
+    # Sets the *_file_name column on the activerecord for this attachment
     def original_filename= new_name
       instance["#{name}_file_name"] = @original_filename = new_name
     end
     
+    # Sets the *_content_type column on the activerecord for this attachment
     def content_type= new_type
       instance["#{name}_content_type"] = @content_type = new_type
     end
 
+    # Sets the *_file_size column on the activerecord for this attachment
     def original_file_size= new_size
       instance["#{name}_file_size"] = @original_file_size = new_size
     end
@@ -178,13 +216,13 @@ module Paperclip
 
     protected
 
-    def is_a_file? data
+    def is_a_file? data #:nodoc:
       [:content_type, :original_filename, :read].map do |meth|
         data.respond_to? meth
       end.all?
     end
 
-    def sanitize_filename filename
+    def sanitize_filename filename #:nodoc:
       File.basename(filename).gsub(/[^\w\.\_]/,'_')
     end
   end

lib/paperclip/attachment_definition.rb

@@ -10,8 +10,8 @@ module Paperclip
         :path               => ":rails_root/public/:class/:attachment/:id/:style_:filename",
         :url                => "/:class/:attachment/:id/:style_:filename",
         :missing_url        => "/:class/:attachment/:style_missing.png",
-        :attachment_type    => :image,
-        :thumbnails         => {},
+        :content_type       => :image,
+        :styles             => {},
         :delete_on_destroy  => true,
         :default_style      => :original
       }
@@ -26,15 +26,10 @@ module Paperclip
       @name
     end
 
-    # A hash of all styles of the attachment. Essentially all the thumbnails
-    # plus the original.
+    # A hash of all styles of the attachment, plus :original. If :original is specified
+    # in the styles option, it will be overwritten.
     def styles
-      @styles ||= thumbnails.merge(:original => nil)
-    end
-
-    # A hash of all defined thumbnails for this attachment.
-    def thumbnails
-      @thumbnails ||= @options[:thumbnails] || {}
+      @styles ||= @options[:styles].merge(:original => nil)
     end
 
     # A convenience method to insert validation options into the options hash

lib/paperclip/storage/filesystem.rb

@@ -3,10 +3,10 @@ module Paperclip
     module Filesystem
       def write_attachment
         ensure_directories
-        for_attached_files do |style, data|
+        each_unsaved do |style, data|
           File.open( file_name(style), "w" ) do |file|
             file.rewind
-            file.write(data) if data
+            file.write(data)
           end
         end
       end
@@ -16,7 +16,7 @@ module Paperclip
       end
 
       def delete_attachment complain = false
-        for_attached_files do |style, data|
+        styles.each do |style|
           file_path = file_name(style)
           begin
             FileUtils.rm file_path if file_path
@@ -36,7 +36,7 @@ module Paperclip
       end
 
       def ensure_directories
-        for_attached_files do |style, file|
+        each_unsaved do |style, file|
           dirname = File.dirname( file_name(style) )
           FileUtils.mkdir_p dirname
         end

lib/paperclip/storage/s3.rb

@@ -44,10 +44,6 @@ module Paperclip
         interpolate( style, definition.url )
       end
 
-      def attachment_exists? style = nil
-        AWS::S3::S3Object.exists?( file_name(style), bucket )
-      end
-
       def bucket
         definition.bucket
       end
@@ -61,9 +57,20 @@ module Paperclip
         end
       end
       
+      def stream style = nil, &block
+        AWS::S3::S3Object.stream( file_name(style), bucket, &block )
+      end
+      
+      # These four methods are the primary interface for the storage module.
+      # Everything above this is support for these methods.
+      
+      def attachment_exists? style = nil
+        AWS::S3::S3Object.exists?( file_name(style), bucket )
+      end
+      
       def write_attachment
         ensure_bucket
-        for_attached_files do |style, data|
+        each_unsaved do |style, data|
           AWS::S3::S3Object.store( file_name(style), data, bucket, :access => definition.s3_access || :public_read )
         end
       end
@@ -72,12 +79,8 @@ module Paperclip
         AWS::S3::S3Object.value( file_name(style), bucket )
       end
 
-      def stream style = nil, &block
-        AWS::S3::S3Object.stream( file_name(style), bucket, &block )
-      end
-
       def delete_attachment complain = false
-        for_attached_files do |style|
+        styles.each do |style, data|
           AWS::S3::S3Object.delete( file_name(style), bucket )
         end
       end

test/test_attachment.rb

@@ -96,7 +96,7 @@ class TestAttachment < Test::Unit::TestCase
 
     context "with an image with thumbnails attached to :image and saved" do
       setup do
-        assert Foo.has_attached_file(:image, :thumbnails => {:small => "16x16", :medium => "100x100", :large => "250x250", :square => "32x32#"})
+        assert Foo.has_attached_file(:image, :styles => {:small => "16x16", :medium => "100x100", :large => "250x250", :square => "32x32#"})
         @foo = Foo.new
         @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "test_image.jpg"))
         assert_nothing_raised{ @foo.image = @file }
@@ -135,7 +135,7 @@ class TestAttachment < Test::Unit::TestCase
 
     context "with an invalid image with a square thumbnail attached to :image" do
       setup do
-        assert Foo.has_attached_file(:image, :thumbnails => {:square => "32x32#"})
+        assert Foo.has_attached_file(:image, :styles => {:square => "32x32#"})
         assert Foo.validates_attached_file(:image)
         @foo = Foo.new
         @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "test_invalid_image.jpg"))
@@ -151,7 +151,7 @@ class TestAttachment < Test::Unit::TestCase
     
     context "with an invalid image attached to :image" do
       setup do
-        assert Foo.has_attached_file(:image, :thumbnails => {:sorta_square => "32x32"})
+        assert Foo.has_attached_file(:image, :styles => {:sorta_square => "32x32"})
         assert Foo.validates_attached_file(:image)
         @foo = Foo.new
         @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "test_invalid_image.jpg"))

test/test_attachment_definition.rb

@@ -25,7 +25,7 @@ class TestAttachmentDefinition < Test::Unit::TestCase
         :path => "/home/stuff/place",
         :url => "/attachments/:attachment/:name",
         :custom_definition => :boogie!,
-        :thumbnails => {:thumb => "100x100", :large => "300x300>"},
+        :styles => {:thumb => "100x100", :large => "300x300>"},
         :validates_existance => true,
         :validates_size => [0, 2048]
       }
@@ -38,13 +38,13 @@ class TestAttachmentDefinition < Test::Unit::TestCase
     end
     
     should "be able to read options using attribute readers" do
-      @options.keys.each do |key|
+      (@options.keys - [:styles]).each do |key|
         assert_equal @options[key], @def.send(key)
       end
     end
     
-    should "return styles as thumbnails plus the original" do
-      assert( (@def.thumbnails.keys + [:original]).map(&:to_s).sort == @def.styles.keys.map(&:to_s).sort )
+    should "return styles as the styles option plus the original" do
+      assert_equal( (@options[:styles].keys + [:original]).map(&:to_s).sort.uniq, @def.styles.keys.map(&:to_s).sort )
     end
     
     should "return all validations when sent :validations" do