Permalink
Browse files

delegate to the @tempfile instance variable

  • Loading branch information...
tenderlove committed Oct 4, 2010
1 parent f9734f2 commit 2a3022db7f2ddc0fc0e678ea757f97902c5f5c01
Showing with 23 additions and 13 deletions.
  1. +5 −13 actionpack/lib/action_dispatch/http/upload.rb
  2. +18 −0 actionpack/test/dispatch/uploaded_file_test.rb
@@ -2,27 +2,19 @@
module ActionDispatch
module Http
- class UploadedFile < Tempfile

This comment has been minimized.

Show comment
Hide comment
@iain

iain Nov 16, 2010

Contributor

You broke Paperclip by doing this: https://github.com/thoughtbot/paperclip/issues/issue/346

Just letting you know. Don't know who's responsible or where it should be fixed :)

@iain

iain Nov 16, 2010

Contributor

You broke Paperclip by doing this: https://github.com/thoughtbot/paperclip/issues/issue/346

Just letting you know. Don't know who's responsible or where it should be fixed :)

This comment has been minimized.

Show comment
Hide comment
@tenderlove

tenderlove Nov 16, 2010

Member

It's fixed in paperclip here:

thoughtbot/paperclip@ef7233d

@tenderlove

tenderlove Nov 16, 2010

Member

It's fixed in paperclip here:

thoughtbot/paperclip@ef7233d

This comment has been minimized.

Show comment
Hide comment
@iain

iain Nov 16, 2010

Contributor

odd, it doesn't work for me (using paperclip straight out of github via bundler).
I'll dig around some more. Thanks for your quick response anyway!

@iain

iain Nov 16, 2010

Contributor

odd, it doesn't work for me (using paperclip straight out of github via bundler).
I'll dig around some more. Thanks for your quick response anyway!

+ class UploadedFile
attr_accessor :original_filename, :content_type, :tempfile, :headers
def initialize(hash)
@original_filename = hash[:filename]
@content_type = hash[:type]
@headers = hash[:head]
-
- # To the untrained eye, this may appear as insanity. Given the alternatives,
- # such as busting the method cache on every request or breaking backwards
- # compatibility with is_a?(Tempfile), this solution is the best available
- # option.
- #
- # TODO: Deprecate is_a?(Tempfile) and define a real API for this parameter
- tempfile = hash[:tempfile]
- tempfile.instance_variables.each do |ivar|
- instance_variable_set(ivar, tempfile.instance_variable_get(ivar))
- end
+ @tempfile = hash[:tempfile]
end
- alias local_path path
+ def method_missing(name, *args, &block)
+ @tempfile.send(name, *args, &block)
+ end
end
module Upload
@@ -21,5 +21,23 @@ def test_tempfile
uf = Http::UploadedFile.new(:tempfile => 'foo')
assert_equal 'foo', uf.tempfile
end
+
+ def test_delegates_to_tempfile
+ tf = Class.new { def tenderlove; 'thunderhorse' end }
+ uf = Http::UploadedFile.new(:tempfile => tf.new)
+ assert_equal 'thunderhorse', uf.tenderlove
+ end
+
+ def test_delegates_to_tempfile_with_params
+ tf = Class.new { def tenderlove *args; args end }
+ uf = Http::UploadedFile.new(:tempfile => tf.new)
+ assert_equal %w{ thunder horse }, uf.tenderlove(*%w{ thunder horse })
+ end
+
+ def test_delegates_to_tempfile_with_block
+ tf = Class.new { def tenderlove; yield end }
+ uf = Http::UploadedFile.new(:tempfile => tf.new)
+ assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' })
+ end
end
end

3 comments on commit 2a3022d

@chad

This comment has been minimized.

Show comment
Hide comment
@chad

chad Oct 5, 2010

Contributor

Seems like the existence of @tempfile is an invariant here and should be checked or otherwise required in the interface. Does it ever make sense to have an UploadedFile with no @tempfile? I'm guessing not, because method_missing would result in weird nil errors. At least maybe super unless @tempfile?

Contributor

chad replied Oct 5, 2010

Seems like the existence of @tempfile is an invariant here and should be checked or otherwise required in the interface. Does it ever make sense to have an UploadedFile with no @tempfile? I'm guessing not, because method_missing would result in weird nil errors. At least maybe super unless @tempfile?

@tenderlove

This comment has been minimized.

Show comment
Hide comment
@tenderlove

tenderlove Oct 5, 2010

Member

@chad fisted here 8a97470
and here 3370ad0

Member

tenderlove replied Oct 5, 2010

@chad fisted here 8a97470
and here 3370ad0

@peter

This comment has been minimized.

Show comment
Hide comment
@peter

peter Nov 18, 2010

The file upload changes between Rails 3.0.1 and Rails 3.0.3 are backwards incompatible, see: http://marklunds.com/articles/one/433

The file upload changes between Rails 3.0.1 and Rails 3.0.3 are backwards incompatible, see: http://marklunds.com/articles/one/433

Please sign in to comment.