Skip to content

Commit

Permalink
Patches Rubydora::RestApiClient to not read file-like object
Browse files Browse the repository at this point in the history
When adding or modifying datastream content.

Note: Previously, rubydora read the entire file content into a string
and passed the string to rest-client, so the file remained open.
Since rest-client closes the file, the changed methods alter the state
of passed in file/io objects.
  • Loading branch information
dchandekstark committed Jan 31, 2017
1 parent cf81f19 commit 890aba2
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ gemspec

gem 'jruby-openssl', :platform => :jruby
gem 'activesupport', '< 5' if RUBY_VERSION < '2.2.2'
gem 'rake', '< 12'
6 changes: 2 additions & 4 deletions lib/rubydora/rest_api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,8 @@ def add_datastream(options = {})
file = query_options.delete(:content)
content_type = query_options.delete(:content_type) || query_options[:mimeType] || file_content_type(file)
run_hook :before_add_datastream, :pid => pid, :dsid => dsid, :file => file, :options => options
str = file.respond_to?(:read) ? file.read : file
file.rewind if file.respond_to?(:rewind)
ProfileParser.parse_datastream_profile(client[datastream_url(pid, dsid, query_options)].post(str, :content_type => content_type.to_s, :multipart => true))
ProfileParser.parse_datastream_profile(client[datastream_url(pid, dsid, query_options)].post(file, :content_type => content_type.to_s, :multipart => true))
rescue Exception => exception
rescue_with_handler(exception) || raise
end
Expand All @@ -361,9 +360,8 @@ def modify_datastream(options = {})
end

run_hook :before_modify_datastream, :pid => pid, :dsid => dsid, :file => file, :content_type => content_type, :options => options
str = file.respond_to?(:read) ? file.read : file
file.rewind if file.respond_to?(:rewind)
ProfileParser.parse_datastream_profile(client[datastream_url(pid, dsid, query_options)].put(str, rest_client_options))
ProfileParser.parse_datastream_profile(client[datastream_url(pid, dsid, query_options)].put(file, rest_client_options))

rescue Exception => exception
rescue_with_handler(exception) || raise
Expand Down
10 changes: 5 additions & 5 deletions spec/lib/rest_api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,11 @@ class MockRepository
@mock_repository.add_datastream :pid => 'mypid', :dsid => 'aaa'
end
describe "when a file is passed" do
let!(:file) { StringIO.new('test', 'r') } # StringIO is a good stand it for a real File (it has read, rewind and close)
it "should rewind the file" do
let!(:file) { StringIO.new('test', 'r') } # StringIO is a good stand in for a real File (it has read, rewind and close)
it "closes the file" do
RestClient::Request.any_instance.should_receive(:transmit) #stub transmit so that Request.execute can close the file we pass
@mock_repository.add_datastream :pid => 'mypid', :dsid => 'aaa', :content=>file
lambda {file.read}.should_not raise_error
file.should be_closed
end
describe "and mimeType is not provided" do
describe "and file responds to :content_type" do
Expand Down Expand Up @@ -282,10 +282,10 @@ class MockRepository
end
describe "when a file is passed" do
let!(:file) { StringIO.new('test', 'r') } # StringIO is a good stand it for a real File (it has read, rewind and close)
it "should rewind the file" do
it "closes the file" do
RestClient::Request.any_instance.should_receive(:transmit) #stub transmit so that Request.execute can close the file we pass
@mock_repository.modify_datastream :pid => 'mypid', :dsid => 'aaa', :content=>file
lambda {file.read}.should_not raise_error
file.should be_closed
end
describe "and mimeType is not provided" do
describe "and file responds to :content_type" do
Expand Down

0 comments on commit 890aba2

Please sign in to comment.