Skip to content

Commit

Permalink
Fix "undefined method `[]' for #<Nori::StringIOFile>" when adding File
Browse files Browse the repository at this point in the history
When calling `File#add`, the response XML sets a `type="file"` attribute
on the `baseRef` element, where "file" corresponds to the NetSuite
record. This then causes Nori (the XML parser used by Savon, the SOAP
client) to interpret that element as representing a base64 encoded file,
so it tries to get fancy about how it parses that element into a hash by
returning an instance of it's `StringIOFile` class:
https://github.com/savonrb/nori/blob/f59f8e18b0e53285c87d75a635058745e66b0bfb/lib/nori/xml_utility_node.rb#L131-L136

Either NetSuite's doing something non-standard with it's use of the
`type` attribute that Nori is trying to enforce, or Nori is
over-aggressive in trying to typecast to aid the developer.

The end result was that when we tried to extract the `internal_id` from
the response, the `body` was actually an instance of `StringIOFile`, not
a hash:
https://github.com/NetSweet/netsuite/blob/f0e46a076d0e7cb2abd5e9001ccbfd4bbb3d35c3/lib/netsuite/actions/add.rb#L80

To work around this, as I don't see a way to disable such behavior in
Savon/Nori, if we detect the `baseRef` element was parsed to
`StringIOFile`, we'll then take the raw XML and parse out the `baseRef`
ourselves, returning a hash with the `internal_id` as we'd exect from
non-`File` responses.

I'm not thrilled with this solution. If we ever needed something else
from the `baseRef` element, this effectively drops all other attributes
for file records. It also introduces explicit references to `Nori` and
`Nokogiri`, both of which are dependencies of Savon, but I'm not sure if
that means this gem should list them as explicit dependencies to guard
against Savon replacing them in a future update. Listing them as
dependencies would then require keeping their version constraints in
sync with Savon most likely.

I believe this answers a question from #481:
#481 (comment)

However the fix in #481 solves it by not trying to extract the
`internal_id`, which would create a problem if someone wanted to add a
file then save the ID in their own database for future use.
  • Loading branch information
cgunther committed Oct 20, 2021
1 parent f0e46a0 commit 6abf4ba
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 3 deletions.
4 changes: 2 additions & 2 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

### Fixed

*
* Fix "undefined method `[]` for #<Nori::StringIOFile>" when adding File (#495)
*

## 0.8.9
Expand All @@ -29,4 +29,4 @@
* Fix accessing custom field values returned in advanced search results (#480)
* Fixing bug where single-selection custom multi select fields would incorrectly be parsed 3377c971d0cb727d81f4b4bc6e30edfbdfaccfd1
* Fixed some field definitions on serialized assembly item
* Properly extract external_id from advanced search results (#478)
* Properly extract external_id from advanced search results (#478)
6 changes: 5 additions & 1 deletion lib/netsuite/actions/add.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ def success?
end

def response_body
@response_body ||= response_hash[:base_ref]
@response_body ||= if response_hash[:base_ref].is_a?(Nori::StringIOFile)
{ :@internal_id => Nokogiri::XML(@response.to_s).remove_namespaces!.at_xpath('//baseRef')[:internalId] }
else
response_hash[:base_ref]
end
end

def response_errors
Expand Down
36 changes: 36 additions & 0 deletions spec/netsuite/actions/add_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,40 @@
end
end

context 'File' do
let(:file) do
NetSuite::Records::File.new(name: 'foo.pdf', content: 'abc123')
end

context 'when successful' do
before do
savon.expects(:add).with(:message => {
'platformMsgs:record' => {
:content! => {
'fileCabinet:name' => 'foo.pdf',
'fileCabinet:content' => 'abc123',
},
'@xsi:type' => 'fileCabinet:File'
},
}).returns(File.read('spec/support/fixtures/add/add_file.xml'))
end

it 'makes a valid request to the NetSuite API' do
NetSuite::Actions::Add.call([file])
end

it 'returns a valid Response object' do
response = NetSuite::Actions::Add.call([file])
expect(response).to be_kind_of(NetSuite::Response)
expect(response).to be_success
end

it 'properly extracts internal ID from response' do
file.add

expect(file.internal_id).to eq('23556')
end
end
end

end
20 changes: 20 additions & 0 deletions spec/support/fixtures/add/add_file.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<soapenv:Header>
<platformMsgs:documentInfo xmlns:platformMsgs="urn:messages_2020_2.platform.webservices.netsuite.com">
<platformMsgs:nsId>REDACTED</platformMsgs:nsId>
</platformMsgs:documentInfo>
</soapenv:Header>
<soapenv:Body>
<addResponse xmlns="urn:messages_2020_2.platform.webservices.netsuite.com">
<writeResponse>
<platformCore:status xmlns:platformCore="urn:core_2020_2.platform.webservices.netsuite.com" isSuccess="true">
<platformCore:statusDetail>
<platformCore:afterSubmitFailed>false</platformCore:afterSubmitFailed>
</platformCore:statusDetail>
</platformCore:status>
<baseRef xmlns:platformCore="urn:core_2020_2.platform.webservices.netsuite.com" internalId="23556" type="file" xsi:type="platformCore:RecordRef"/>
</writeResponse>
</addResponse>
</soapenv:Body>
</soapenv:Envelope>

0 comments on commit 6abf4ba

Please sign in to comment.