From 1cb0ef6fc1c409d2955a1f9c6996e0c7ae7e984a Mon Sep 17 00:00:00 2001 From: Adam Milligan Date: Sun, 18 Sep 2016 14:10:50 -0400 Subject: [PATCH 1/3] Conform to standard #respond_to? usage --- lib/rack/test/uploaded_file.rb | 6 +++--- spec/rack/test/uploaded_file_spec.rb | 14 +++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/rack/test/uploaded_file.rb b/lib/rack/test/uploaded_file.rb index 7ee09b29..d44ab0e1 100644 --- a/lib/rack/test/uploaded_file.rb +++ b/lib/rack/test/uploaded_file.rb @@ -40,11 +40,11 @@ def path alias local_path path def method_missing(method_name, *args, &block) #:nodoc: - @tempfile.__send__(method_name, *args, &block) + tempfile.public_send(method_name, *args, &block) end - def respond_to?(method_name, include_private = false) #:nodoc: - @tempfile.respond_to?(method_name, include_private) || super + def respond_to_missing?(method_name, include_private = false) #:nodoc: + tempfile.respond_to?(method_name, include_private) || super end def self.finalize(file) diff --git a/spec/rack/test/uploaded_file_spec.rb b/spec/rack/test/uploaded_file_spec.rb index 420fe9c2..d8557966 100644 --- a/spec/rack/test/uploaded_file_spec.rb +++ b/spec/rack/test/uploaded_file_spec.rb @@ -14,17 +14,9 @@ def test_file_path it 'responds to things that Tempfile responds to' do uploaded_file = Rack::Test::UploadedFile.new(test_file_path) - expect(uploaded_file).to respond_to(:close) - expect(uploaded_file).to respond_to(:close!) - expect(uploaded_file).to respond_to(:delete) - expect(uploaded_file).to respond_to(:length) - expect(uploaded_file).to respond_to(:open) - expect(uploaded_file).to respond_to(:path) - expect(uploaded_file).to respond_to(:size) - expect(uploaded_file).to respond_to(:unlink) - expect(uploaded_file).to respond_to(:read) - expect(uploaded_file).to respond_to(:original_filename) - expect(uploaded_file).to respond_to(:tempfile) # Allows calls to params[:file].tempfile + Tempfile.public_instance_methods(false).each do |method| + expect(uploaded_file).to respond_to(method) + end end it "creates Tempfiles with original file's extension" do From 1c6028fa1c738d270253a6ad759e72648ff94cab Mon Sep 17 00:00:00 2001 From: Adam Milligan Date: Sun, 18 Sep 2016 14:12:15 -0400 Subject: [PATCH 2/3] Loosen fake UploadedFile coupling to file system The initializer now accepts an IO object, which allows callers to instantiate fake upload objects with in-memory content. --- lib/rack/test/uploaded_file.rb | 39 +++++++++++++++++++--------- spec/rack/test/uploaded_file_spec.rb | 23 ++++++++++++++++ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/lib/rack/test/uploaded_file.rb b/lib/rack/test/uploaded_file.rb index d44ab0e1..13a9245a 100644 --- a/lib/rack/test/uploaded_file.rb +++ b/lib/rack/test/uploaded_file.rb @@ -18,23 +18,18 @@ class UploadedFile # The content type of the "uploaded" file attr_accessor :content_type - def initialize(path, content_type = 'text/plain', binary = false) - raise "#{path} file does not exist" unless ::File.exist?(path) - + def initialize(content, content_type = 'text/plain', binary = false, original_filename: nil) + if content.respond_to?(:read) + initialize_from_io(content, original_filename) + else + initialize_from_file_path(content) + end @content_type = content_type - @original_filename = ::File.basename(path) - - @tempfile = Tempfile.new([@original_filename, ::File.extname(path)]) - @tempfile.set_encoding(Encoding::BINARY) if @tempfile.respond_to?(:set_encoding) @tempfile.binmode if binary - - ObjectSpace.define_finalizer(self, self.class.finalize(@tempfile)) - - FileUtils.copy_file(path, @tempfile.path) end def path - @tempfile.path + tempfile.path end alias local_path path @@ -55,6 +50,26 @@ def self.actually_finalize(file) file.close file.unlink end + + private + + def initialize_from_io(io, original_filename) + @tempfile = io + @original_filename = original_filename || raise(ArgumentError, 'Missing `original_filename` for IO') + end + + def initialize_from_file_path(path) + raise "#{path} file does not exist" unless ::File.exist?(path) + + @original_filename = ::File.basename(path) + + @tempfile = Tempfile.new([@original_filename, ::File.extname(path)]) + @tempfile.set_encoding(Encoding::BINARY) if @tempfile.respond_to?(:set_encoding) + + ObjectSpace.define_finalizer(self, self.class.finalize(@tempfile)) + + FileUtils.copy_file(path, @tempfile.path) + end end end end diff --git a/spec/rack/test/uploaded_file_spec.rb b/spec/rack/test/uploaded_file_spec.rb index d8557966..7428f745 100644 --- a/spec/rack/test/uploaded_file_spec.rb +++ b/spec/rack/test/uploaded_file_spec.rb @@ -43,4 +43,27 @@ def test_file_path end end end + + describe '#initialize' do + subject { -> { uploaded_file } } + let(:uploaded_file) { described_class.new(io, original_filename: original_filename) } + + context 'with an IO object' do + let(:io) { StringIO.new('I am content') } + + context 'with an original filename' do + let(:original_filename) { 'content.txt' } + + it 'sets the specified filename' do + subject.call + uploaded_file.original_filename.should == original_filename + end + end + + context 'without an original filename' do + let(:original_filename) { nil } + it { should raise_error(ArgumentError) } + end + end + end end From 9c3d18fa6fdeb29a3eb51bafc079b77e4ebeefed Mon Sep 17 00:00:00 2001 From: Adam Milligan Date: Sun, 18 Sep 2016 14:12:15 -0400 Subject: [PATCH 3/3] Loosen multipart content coupling to file system --- lib/rack/test/utils.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/rack/test/utils.rb b/lib/rack/test/utils.rb index 076a7825..5a9d8872 100644 --- a/lib/rack/test/utils.rb +++ b/lib/rack/test/utils.rb @@ -128,17 +128,15 @@ def build_primitive_part(parameter_name, value) module_function :build_primitive_part def build_file_part(parameter_name, uploaded_file) - ::File.open(uploaded_file.path, 'rb') do |physical_file| - physical_file.set_encoding(Encoding::BINARY) if physical_file.respond_to?(:set_encoding) - <<-EOF + uploaded_file.set_encoding(Encoding::BINARY) if uploaded_file.respond_to?(:set_encoding) + <<-EOF --#{MULTIPART_BOUNDARY}\r Content-Disposition: form-data; name="#{parameter_name}"; filename="#{escape(uploaded_file.original_filename)}"\r Content-Type: #{uploaded_file.content_type}\r -Content-Length: #{::File.stat(uploaded_file.path).size}\r +Content-Length: #{uploaded_file.size}\r \r -#{physical_file.read}\r +#{uploaded_file.read}\r EOF - end end module_function :build_file_part end