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
#495)

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 Dec 14, 2021
1 parent 5c1d56e commit f502ac3
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 1 deletion.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Update ServiceResaleItem record fields/record refs for 2021.2. `cost_estimate_type`, `item_options_list`, `matrix_type`, `out_of_stock_behavior`, `overall_quantity_pricing_type`, `presentation_item_list`, `site_category_list`, `sitemap_priority`, `translations_list`, `vsoe_deferral`, `vsoe_permit_discount`, `vsoe_sop_group` were all removed as fields as the are not simple fields, they require special classes. (#500)

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

## 0.8.10
Expand Down
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 f502ac3

Please sign in to comment.