Refactor the looping of attachments in the rake task

gemfiles/rails2.gemfile

@@ -7,4 +7,5 @@ gem "rake"
 gem "sqlite3-ruby", "~>1.3.0"
 gem "shoulda"
 gem "mocha"
-gem "aws-s3", {:require=>"aws/s3"}
+gem "aws-s3", {:require=>"aws/s3"}
+gem "appraisal"

gemfiles/rails2.gemfile.lock

@@ -11,6 +11,9 @@ GEM
     activeresource (2.3.10)
       activesupport (= 2.3.10)
     activesupport (2.3.10)
+    appraisal (0.1)
+      bundler
+      rake
     aws-s3 (0.6.2)
       builder
       mime-types
@@ -43,6 +46,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  appraisal
   aws-s3
   mocha
   rails (~> 2.3.0)

gemfiles/rails3.gemfile

@@ -2,9 +2,10 @@
 
 source "http://rubygems.org"
 gem "ruby-debug"
-gem "rails", "~>3.0.0"
+gem "rails", ">=3.0.3"
 gem "rake"
 gem "sqlite3-ruby", "~>1.3.0"
 gem "shoulda"
 gem "mocha"
-gem "aws-s3", {:require=>"aws/s3"}
+gem "aws-s3", {:require=>"aws/s3"}
+gem "appraisal"

gemfiles/rails3.gemfile.lock

@@ -28,6 +28,9 @@ GEM
       activemodel (= 3.0.3)
       activesupport (= 3.0.3)
     activesupport (3.0.3)
+    appraisal (0.1)
+      bundler
+      rake
     arel (2.0.4)
     aws-s3 (0.6.2)
       builder
@@ -84,9 +87,10 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  appraisal
   aws-s3
   mocha
-  rails (~> 3.0.0)
+  rails (>= 3.0.3)
   rake
   ruby-debug
   shoulda

lib/paperclip.rb

@@ -113,6 +113,12 @@ def processor name #:nodoc:
       processor
     end
 
+    def each_instance_with_attachment(klass, name)
+      Object.const_get(klass).all.each do |instance|
+        yield(instance) if instance.send(:"#{name}?")
+      end
+    end
+
     # Log a paperclip-specific line. Uses ActiveRecord::Base.logger
     # by default. Set Paperclip.options[:log] to false to turn off.
     def log message

lib/tasks/paperclip.rake

@@ -1,7 +1,6 @@
 def obtain_class
   class_name = ENV['CLASS'] || ENV['class']
   raise "Must specify CLASS" unless class_name
-  @klass = Object.const_get(class_name)
 end
 
 def obtain_attachments
@@ -14,25 +13,6 @@ def obtain_attachments
   end
 end
 
-def for_all_attachments
-  klass = obtain_class
-  names = obtain_attachments
-  ids   = klass.connection.select_values(klass.send(:construct_finder_sql, :select => 'id'))
-
-  ids.each do |id|
-    instance = klass.find(id)
-    names.each do |name|
-      result = if instance.send("#{ name }?")
-                 yield(instance, name)
-               else
-                 true
-               end
-      print result ? "." : "x"; $stdout.flush
-    end
-  end
-  puts " Done."
-end
-
 namespace :paperclip do
   desc "Refreshes both metadata and thumbnails."
   task :refresh => ["paperclip:refresh:metadata", "paperclip:refresh:thumbnails"]
@@ -41,38 +21,49 @@ namespace :paperclip do
     desc "Regenerates thumbnails for a given CLASS (and optional ATTACHMENT)."
     task :thumbnails => :environment do
       errors = []
-      for_all_attachments do |instance, name|
-        result = instance.send(name).reprocess!
-        errors << [instance.id, instance.errors] unless instance.errors.blank?
-        result
+      names = obtain_attachments
+      klass = obtain_class
+      names.each do |name|
+        Paperclip.each_instance_with_attachment(klass, name) do |instance|
+          result = instance.send(name).reprocess!
+          errors << [instance.id, instance.errors] unless instance.errors.blank?
+        end
       end
       errors.each{|e| puts "#{e.first}: #{e.last.full_messages.inspect}" }
     end
 
     desc "Regenerates content_type/size metadata for a given CLASS (and optional ATTACHMENT)."
     task :metadata => :environment do
-      for_all_attachments do |instance, name|
-        if file = instance.send(name).to_file
-          instance.send("#{name}_file_name=", instance.send("#{name}_file_name").strip)
-          instance.send("#{name}_content_type=", file.content_type.strip)
-          instance.send("#{name}_file_size=", file.size) if instance.respond_to?("#{name}_file_size")
-          instance.save(false)
-        else
-          true
+      names = obtain_attachments
+      klass = obtain_class
+      names.each do |name|
+        Paperclip.each_instance_with_attachment(klass, name) do |instance|
+          if file = instance.send(name).to_file
+            instance.send("#{name}_file_name=", instance.send("#{name}_file_name").strip)
+            instance.send("#{name}_content_type=", file.content_type.strip)
+            instance.send("#{name}_file_size=", file.size) if instance.respond_to?("#{name}_file_size")
+            instance.save(false)
+          else
+            true
+          end
         end
       end
     end
   end
 
   desc "Cleans out invalid attachments. Useful after you've added new validations."
   task :clean => :environment do
-    for_all_attachments do |instance, name|
-      instance.send(name).send(:validate)
-      if instance.send(name).valid?
-        true
-      else
-        instance.send("#{name}=", nil)
-        instance.save
+    names = obtain_attachments
+    klass = obtain_class
+    names.each do |name|
+      Paperclip.each_instance_with_attachment(klass, name) do |instance|
+        instance.send(name).send(:validate)
+        if instance.send(name).valid?
+          true
+        else
+          instance.send("#{name}=", nil)
+          instance.save
+        end
       end
     end
   end

test/paperclip_test.rb

@@ -43,6 +43,23 @@ class PaperclipTest < Test::Unit::TestCase
     end
   end
 
+  context "Paperclip.each_instance_with_attachment" do
+    setup do
+      @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb')
+      d1 = Dummy.create(:avatar => @file)
+      d2 = Dummy.create
+      d3 = Dummy.create(:avatar => @file)
+      @expected = [d1, d3]
+    end
+    should "yield every instance of a model that has an attachment" do
+      actual = []
+      Paperclip.each_instance_with_attachment("Dummy", "avatar") do |instance|
+        actual << instance
+      end
+      assert_same_elements @expected, actual
+    end
+  end
+
   should "raise when sent #processor and the name of a class that exists but isn't a subclass of Processor" do
     assert_raises(Paperclip::PaperclipError){ Paperclip.processor(:attachment) }
   end