Did some APIish stuff.

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

Rakefile

@@ -3,7 +3,7 @@ require 'rake/testtask'
 require 'rake/rdoctask'
 
 desc 'Default: run unit tests.'
-task :default => :test
+task :default => [:clean, :test]
 
 desc 'Test the paperclip plugin.'
 Rake::TestTask.new(:test) do |t|

generators/paperclip/templates/paperclip_migration.rb

@@ -3,7 +3,7 @@ class <%= migration_name %> < ActiveRecord::Migration
 <% attachments.each do |attachment| -%>
     add_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_file_name, :string
     add_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_content_type, :string
-    add_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_size, :integer
+    add_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_file_size, :integer
 <% end -%>
   end
 
@@ -11,7 +11,7 @@ class <%= migration_name %> < ActiveRecord::Migration
 <% attachments.each do |attachment| -%>
     remove_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_file_name
     remove_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_content_type
-    remove_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_size
+    remove_column :<%= class_name.underscore.camelize.tableize %>, :<%= attachment %>_file_size
 <% end -%>
   end
 end

lib/paperclip.rb

@@ -133,11 +133,11 @@ module Thoughtbot #:nodoc:
       #
       # == Model Requirements
       # For any given attachment _foo_, the model the attachment is in needs to have both a +foo_file_name+
-      # and +foo_content_type+ column, as a type of +string+. The +foo_file_name+ column contains only the name
+      # and +foo_content_type+ column, as a type of +string+, and a +foo_file_size+ column as type +integer+.
+      # The +foo_file_name+ column contains only the name
       # of the file and none of the path information. However, the +foo_file_name+ column accessor is overwritten
       # by the one (defined above) which returns the full path to whichever style thumbnail is passed in.
-      # In a pinch, you can either use +read_attribute+ or the plain +foo+ accessor, which returns the database's
-      # +foo_file_name+ column.
+      # To access the name as stored in the database, you can use Attachment#original_filename.
       #
       # == Event Triggers
       # When an attachment is set by using he setter (+model.attachment=+), the thumbnails are created and held in
@@ -154,7 +154,10 @@ module Thoughtbot #:nodoc:
         class << self
           attr_accessor :attachment_definitions
         end
+        
         include InstanceMethods
+        after_save :save_attachments
+        before_destroy :destroy_attachments
         
         validates_each(*attachment_names) do |record, attr, value|
           value.errors.each{|e| record.errors.add(attr, e) unless record.errors.on(attr) && record.errors.on(attr).include?(e) }
@@ -191,18 +194,6 @@ module Thoughtbot #:nodoc:
           define_method "destroy_#{name}" do |*args|
             attachment_for(name).queue_destroy(args.first)
           end
-
-          define_method "#{name}_after_save" do
-            attachment_for(name).save
-          end
-          private :"#{name}_after_save"
-          after_save :"#{name}_after_save"
-          
-          define_method "#{name}_before_destroy" do
-            attachment_for(name).destroy
-          end
-          private :"#{name}_before_destroy"
-          before_destroy :"#{name}_before_destroy"
         end
       end
       
@@ -221,6 +212,18 @@ module Thoughtbot #:nodoc:
         end
         alias_method_chain :after_initialize, :paperclip
         
+        def save_attachments
+          @attachments.each do |name, attachment|
+            attachment.save
+          end
+        end
+        
+        def destroy_attachments
+          @attachments.each do |name, attachment|
+            attachment.destroy
+          end
+        end
+        
         def attachment_for name
           @attachments[name]
         end
@@ -230,17 +233,27 @@ module Thoughtbot #:nodoc:
         @attachment_definitions.keys
       end
 
+      # Paperclip always validates whether or not file creation was successful, but does not validate
+      # the presence or size of the file unless told. You can specify validations either in the
+      # has_attached_file call or with a separate validates_attached_file call, with a syntax similar
+      # to has_attached_file. If no options are given, the existence of the file is validated.
+      #
+      #   validates_attached_file :avatar, :existence => true, :size => 0..(500.kilobytes)
       def validates_attached_file *attachment_names
+        options = attachment_names.pop if attachment_names.last.is_a? Hash
+        options ||= { :existence => true }
         attachment_names.each do |name|
-          @attachment_definitions[name].validate :existence
+          options.each do |key, value|
+            @attachment_definitions[name].validate key, value
+          end
         end
       end
       
       def whine_about_columns_for name #:nodoc:
-        [ "#{name}_file_name", "#{name}_content_type", "#{name}_size" ].each do |column|
+        [ "#{name}_file_name", "#{name}_content_type", "#{name}_file_size" ].each do |column|
           unless column_names.include?(column)
             raise PaperclipError, "Class #{self.name} does not have all of the necessary columns to have an attachment named #{name}. " + 
-                                  "(#{name}_file_name, #{name}_content_type, and #{name}_size)"
+                                  "(#{name}_file_name, #{name}_content_type, and #{name}_file_size)"
           end
         end
       end
@@ -265,12 +278,20 @@ module Thoughtbot #:nodoc:
     # * delete_attachment: Delete the files and thumbnails from the storage medium. Should return true or false
     #   depending on success.
     #
-    # When writing files, the @files variable will hold a hash of style names and their data. If @files is nil,
-    # then no new data has been assigned to the attachment and you should not perform any work.
-    #
-    # You will also have access to @definition, which is the AttachmentDefintion object for the attachment. The
-    # methods in your module will be mixed into an Attachment instance, so you have full access to the
-    # Attachment itself.
+    # When writing a storage system, your code will be mixed into the Attachment that the file represents. You
+    # will therefore have access to all of the methods available to Attachments. Some methods of note are:
+    # * definition: Returns the AttachmentDefintion object created by has_attached_file. Useful for getting
+    #   style definitions and flags that you want to set. You should open the AttachmentDefinition class to
+    #   add getters for any options you want to be able to set.
+    # * instance: Returns the ActiveRecord object that the Attachment is attached to. Can be used to obtain ids.
+    # * original_filename: Returns the original_filename, which is the same as the attachment_file_name column
+    #   in the database. Should be nil if there is no attachment.
+    # * original_file_size: Returns the size of the original file, as passed to Attachment#assign.
+    # * interpolate: Given a style and a pattern, this will interpolate variables like :rails_root, :name,
+    #   and :id. See documentation for has_attached_file for more info on interpolation.
+    # * for_attached_files: Iterates over the collection of files for this attachment, passing the style name
+    #   and the data to the block. Will not call the block if the data is nil.
+    # * dirty?: Returns true if a new file has been assigned with Attachment#assign, false otherwise.
     #
     # == Validations
     # Storage systems provide their own validations, since the manner of checking the status of them is usually

lib/paperclip/storage/filesystem.rb

@@ -8,31 +8,28 @@ module Thoughtbot
       module Filesystem
 
         def file_name style = nil
-          style ||= @definition.default_style
+          style ||= definition.default_style
           pattern = if original_filename && instance.id
-            File.join(@definition.path_prefix, @definition.path)
+            File.join(definition.path_prefix, definition.path)
           else
-            @definition.missing_file_name
+            definition.missing_file_name
           end
           interpolate( style, pattern )
         end
 
         def url style = nil
-          style ||= @definition.default_style
+          style ||= definition.default_style
           pattern = if original_filename && instance.id
-            [@definition.url_prefix, @definition.url || @definition.path].compact.join("/")
+            [definition.url_prefix, definition.url || definition.path].compact.join("/")
           else
-            @definition.missing_url
+            definition.missing_url
           end
           interpolate( style, pattern )
         end
 
         def write_attachment
-          return if @files.blank?
           ensure_directories
-          @files.each do |style, data|
-            data.rewind
-            data = data.read
+          for_attached_files do |style, data|
             File.open( file_name(style), "w" ) do |file|
               file.rewind
               file.write(data)
@@ -41,7 +38,7 @@ module Thoughtbot
         end
 
         def delete_attachment complain = false
-          @definition.styles.keys.each do |style|
+          definition.styles.keys.each do |style|
             file_path = file_name(style)
             begin
               FileUtils.rm file_path if file_path
@@ -51,13 +48,14 @@ module Thoughtbot
           end
         end
         
+        def file_exists?(style)
+          style ||= definition.default_style
+          dirty? ? file_for(style) : File.exists?( file_name(style) )
+        end
+        
         def validate_existence *constraints
-          @definition.styles.keys.each do |style|
-            if @dirty
-              errors << "requires a valid #{style} file." unless @files && @files[style]
-            else
-              errors << "requires a valid #{style} file." unless File.exists?( file_name(style) )
-            end
+          definition.styles.keys.each do |style|
+            errors << "requires a valid #{style} file." unless file_exists?(style)
           end
         end
         
@@ -69,7 +67,7 @@ module Thoughtbot
         private
 
         def ensure_directories
-          @files.each do |style, file|
+          for_attached_files do |style, file|
             dirname = File.dirname( file_name(style) )
             FileUtils.mkdir_p dirname
           end

lib/paperclip/storage/s3.rb

 module Thoughtbot
   module Paperclip
     
-    module ClassMethods #:nodoc:
+    module ClassMethods
       def has_attached_file_with_s3 *attachment_names
         has_attached_file_without_s3 *attachment_names
 
@@ -16,7 +16,7 @@ module Thoughtbot
           secret_key = Thoughtbot::Paperclip.options[:s3_secret_access_key]
         end
 
-        if @definition.storage_module == Thoughtbot::Paperclip::Storage::S3
+        if definition.storage_module == Thoughtbot::Paperclip::Storage::S3
           require 'aws/s3'
           AWS::S3::Base.establish_connection!(
             :access_key_id     => access_key,
@@ -79,11 +79,11 @@ module Thoughtbot
       # * s3_persistent: Maintains an HTTP connection to the Amazon service if possible.
       module S3
         def file_name style = nil
-          style ||= @definition.default_style
+          style ||= definition.default_style
           pattern = if original_filename && instance.id
-            @definition.url
+            definition.url
           else
-            @definition.missing_url
+            definition.missing_url
           end
           interpolate( style, pattern )
         end
@@ -93,16 +93,14 @@ module Thoughtbot
         end
 
         def write_attachment
-          return if @files.blank?
           bucket = ensure_bucket
-          @files.each do |style, data|
-            data.rewind
-            AWS::S3::S3Object.store( file_name(style), data, bucket, :access => @definition.s3_access || :public_read )
+          for_attached_files do |style, data|
+            AWS::S3::S3Object.store( file_name(style), data, bucket, :access => definition.s3_access || :public_read )
           end
         end
 
         def delete_attachment complain = false
-          (attachment[:thumbnails].keys + [:original]).each do |style|
+          for_attached_files do |style, data|
             begin
               AWS::S3::S3Object.delete( file_name(style), bucket )
             rescue AWS::S3::ResponseError => error
@@ -111,13 +109,14 @@ module Thoughtbot
           end
         end
         
+        def file_exists?(style)
+          style ||= definition.default_style
+          dirty? ? file_for(style) : AWS::S3::S3Object.exists?( file_name(style), bucket )
+        end
+        
         def validate_existence *constraints
-          @definition.styles.keys.each do |style|
-            if @dirty
-              errors << "requires a valid #{style} file." unless @files && @files[style]
-            else
-              errors << "requires a valid #{style} file." unless AWS::S3::S3Object.exists?( file_name(style), bucket )
-            end
+          definition.styles.keys.each do |style|
+            errors << "requires a valid #{style} file." unless file_exists?(style)
           end
         end
         
@@ -129,7 +128,7 @@ module Thoughtbot
         private
         
         def bucket
-          interpolate(nil, @definition.url_prefix)
+          interpolate(nil, definition.url_prefix)
         end
 
         def ensure_bucket

test/models.rb

@@ -2,28 +2,28 @@ begin
   ActiveRecord::Base.connection.create_table :foos, :force => true do |table|
     table.column :image_file_name, :string
     table.column :image_content_type, :string
-    table.column :image_size, :integer
+    table.column :image_file_size, :integer
   end
   ActiveRecord::Base.connection.create_table :bars, :force => true do |table|
     table.column :document_file_name, :string
     table.column :document_content_type, :string
-    table.column :document_size, :integer
+    table.column :document_file_size, :integer
   end
   ActiveRecord::Base.connection.create_table :non_standards, :force => true do |table|
     table.column :resume_file_name, :string
     table.column :resume_content_type, :string
-    table.column :resume_size, :integer
+    table.column :resume_file_size, :integer
     table.column :avatar_file_name, :string
     table.column :avatar_content_type, :string
-    table.column :avatar_size, :integer
+    table.column :avatar_file_size, :integer
   end
   ActiveRecord::Base.connection.create_table :ess_threes, :force => true do |table|
     table.column :resume_file_name, :string
     table.column :resume_content_type, :string
-    table.column :resume_size, :integer
+    table.column :resume_file_size, :integer
     table.column :avatar_file_name, :string
     table.column :avatar_content_type, :string
-    table.column :avatar_size, :integer
+    table.column :avatar_file_size, :integer
   end
   ActiveRecord::Base.connection.create_table :negatives, :force => true do |table|
     table.column :this_is_the_wrong_name_file_name, :string

test/paperclip_non_standard_test.rb

@@ -53,9 +53,9 @@ class PaperclipNonStandardTest < Test::Unit::TestCase
   def test_should_delete_files_on_destroy
     assert @ns.save
     assert File.exists?( @ns.resume_file_name ), @ns.resume_file_name
-    assert File.exists?( @ns.avatar_file_name(:original) ), @ns.avatar_file_name(:original)
-    assert File.exists?( @ns.avatar_file_name(:bigger) )
-    assert File.exists?( @ns.avatar_file_name(:cropped) )
+    [:original, :bigger, :cropped].each do |style|
+      assert File.exists?( @ns.avatar_file_name(style) ), @ns.avatar_file_name(style)
+    end
 
     resume_file_name  = @ns.resume_file_name
     avatar_file_names = [:original, :bigger, :cropped].map{|style| @ns.avatar_file_name(style) }