Skip to content

Commit

Permalink
Reverted the nil assignment, as the issue in Rails/Rack was fixed. Al…
Browse files Browse the repository at this point in the history
…so removed a lot of logging.
  • Loading branch information
Jon Yurek committed Mar 17, 2009
1 parent d75a5db commit a1b39cc
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 36 deletions.
2 changes: 1 addition & 1 deletion lib/paperclip.rb
Expand Up @@ -43,7 +43,7 @@
# documentation for Paperclip::ClassMethods for more useful information.
module Paperclip

VERSION = "2.2.7"
VERSION = "2.2.8"

class << self
# Provides configurability to Paperclip. There are a number of options available, such as:
Expand Down
18 changes: 2 additions & 16 deletions lib/paperclip/attachment.rb
Expand Up @@ -49,8 +49,6 @@ def initialize name, instance, options = {}

normalize_style_definition
initialize_storage

log("Paperclip attachment #{name} on #{instance.class} initialized.")
end

# What gets called when you call instance.attachment = File. It clears
Expand All @@ -62,9 +60,6 @@ def initialize name, instance, options = {}
# If the file that is assigned is not valid, the processing (i.e.
# thumbnailing, etc) will NOT be run.
def assign uploaded_file
# This is because of changes in Rails 2.3 that cause blank fields to send nil
return nil if uploaded_file.nil?

%w(file_name).each do |field|
unless @instance.class.column_names.include?("#{name}_#{field}")
raise PaperclipError.new("#{@instance.class} model does not have required column '#{name}_#{field}'")
Expand All @@ -77,16 +72,12 @@ def assign uploaded_file
end

return nil unless valid_assignment?(uploaded_file)
log("Assigning #{uploaded_file.inspect} to #{name}")

uploaded_file.binmode if uploaded_file.respond_to? :binmode
queue_existing_for_delete
@errors = {}
@validation_errors = nil
self.clear

return nil if uploaded_file.nil?

log("Writing attributes for #{name}")
@queued_for_write[:original] = uploaded_file.to_tempfile
instance_write(:file_name, uploaded_file.original_filename.strip.gsub(/[^\w\d\.\-]+/, '_'))
instance_write(:content_type, uploaded_file.content_type.to_s.strip)
Expand Down Expand Up @@ -149,13 +140,11 @@ def dirty?
# the instance's errors and returns false, cancelling the save.
def save
if valid?
log("Saving files for #{name}")
flush_deletes
flush_writes
@dirty = false
true
else
log("Errors on #{name}. Not saving.")
flush_errors
false
end
Expand Down Expand Up @@ -293,7 +282,7 @@ def logging? #:nodoc:
end

def valid_assignment? file #:nodoc:
file.respond_to?(:original_filename) && file.respond_to?(:content_type)
file.nil? || (file.respond_to?(:original_filename) && file.respond_to?(:content_type))
end

def validate #:nodoc:
Expand Down Expand Up @@ -370,12 +359,10 @@ def callback which #:nodoc:
end

def post_process_styles
log("Post-processing #{name}")
@styles.each do |name, args|
begin
raise RuntimeError.new("Style #{name} has no processors defined.") if args[:processors].blank?
@queued_for_write[name] = args[:processors].inject(@queued_for_write[:original]) do |file, processor|
log("Processing #{name} #{file} in the #{processor} processor.")
Paperclip.processor(processor).make(file, args, self)
end
rescue PaperclipError => e
Expand All @@ -397,7 +384,6 @@ def interpolate pattern, style = default_style #:nodoc:

def queue_existing_for_delete #:nodoc:
return unless file?
log("Queueing the existing files for #{name} for deletion.")
@queued_for_delete += [:original, *@styles.keys].uniq.map do |style|
path(style) if exists?(style)
end.compact
Expand Down
13 changes: 4 additions & 9 deletions lib/paperclip/storage.rb
Expand Up @@ -36,22 +36,20 @@ def to_file style = default_style
alias_method :to_io, :to_file

def flush_writes #:nodoc:
logger.info("[paperclip] Writing files for #{name}")
@queued_for_write.each do |style, file|
file.close
FileUtils.mkdir_p(File.dirname(path(style)))
logger.info("[paperclip] -> #{path(style)}")
logger.info("[paperclip] saving #{path(style)}")
FileUtils.mv(file.path, path(style))
FileUtils.chmod(0644, path(style))
end
@queued_for_write = {}
end

def flush_deletes #:nodoc:
logger.info("[paperclip] Deleting files for #{name}")
@queued_for_delete.each do |path|
begin
logger.info("[paperclip] -> #{path}")
logger.info("[paperclip] deleting #{path}")
FileUtils.rm(path) if File.exist?(path)
rescue Errno::ENOENT => e
# ignore file-not-found, let everything else pass
Expand Down Expand Up @@ -151,7 +149,6 @@ def self.extended base
base.class.interpolations[:s3_domain_url] = lambda do |attachment, style|
"#{attachment.s3_protocol}://#{attachment.bucket_name}.s3.amazonaws.com/#{attachment.path(style).gsub(%r{^/}, "")}"
end
ActiveRecord::Base.logger.info("[paperclip] S3 Storage Initalized.")
end

def s3
Expand Down Expand Up @@ -193,10 +190,9 @@ def to_file style = default_style
alias_method :to_io, :to_file

def flush_writes #:nodoc:
logger.info("[paperclip] Writing files for #{name}")
@queued_for_write.each do |style, file|
begin
logger.info("[paperclip] -> #{path(style)}")
logger.info("[paperclip] saving #{path(style)}")
key = s3_bucket.key(path(style))
key.data = file
key.put(nil, @s3_permissions, {'Content-type' => instance_read(:content_type)}.merge(@s3_headers))
Expand All @@ -208,10 +204,9 @@ def flush_writes #:nodoc:
end

def flush_deletes #:nodoc:
logger.info("[paperclip] Writing files for #{name}")
@queued_for_delete.each do |path|
begin
logger.info("[paperclip] -> #{path}")
logger.info("[paperclip] deleting #{path}")
if file = s3_bucket.key(path)
file.delete
end
Expand Down
12 changes: 6 additions & 6 deletions test/attachment_test.rb
Expand Up @@ -599,14 +599,14 @@ def do_after_all; end
end
end

should "not delete the files saving in a deprecated manner" do
@attachment.expects(:instance_write).with(:file_name, nil).never
@attachment.expects(:instance_write).with(:content_type, nil).never
@attachment.expects(:instance_write).with(:file_size, nil).never
@attachment.expects(:instance_write).with(:updated_at, nil).never
should "delete the files after assigning nil" do
@attachment.expects(:instance_write).with(:file_name, nil)
@attachment.expects(:instance_write).with(:content_type, nil)
@attachment.expects(:instance_write).with(:file_size, nil)
@attachment.expects(:instance_write).with(:updated_at, nil)
@attachment.assign nil
@attachment.save
@existing_names.each{|f| assert File.exists?(f) }
@existing_names.each{|f| assert ! File.exists?(f) }
end

should "delete the files when you call #clear and #save" do
Expand Down
4 changes: 0 additions & 4 deletions test/paperclip_test.rb
Expand Up @@ -91,8 +91,6 @@ class PaperclipTest < Test::Unit::TestCase
end

should "not assign the avatar on mass-set" do
@dummy.logger.expects(:debug)

@dummy.attributes = { :other => "I'm set!",
:avatar => @file }

Expand All @@ -101,8 +99,6 @@ class PaperclipTest < Test::Unit::TestCase
end

should "still allow assigment on normal set" do
@dummy.logger.expects(:debug).times(0)

@dummy.other = "I'm set!"
@dummy.avatar = @file

Expand Down

0 comments on commit a1b39cc

Please sign in to comment.