Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable endpoint as a configuration option to bypass NS WSDL issues #473

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

davidlaprade
Copy link
Contributor

@davidlaprade davidlaprade commented Mar 1, 2021

Background

This is related to #445, which I believe first raised this issue.

Recent versions of the NS WSDL contain an incorrect endpoint reference. This seems to have begun on version 2020_1. If you go to https://<acount_id>.suitetalk.api.netsuite.com/wsdl/v2020_1_0/netsuite.wsdl you can see at the very bottom:

image

When requests are made to this endpoint the following error is returned:

In this account, you must use account-specific domains with this SOAP web services endpoint. You can use the SOAP getDataCenterUrls operation to obtain the correct domain. Or, go to Setup > Company > Company Information in the NetSuite UI. Your domains are listed on the Company URLs tab.

Solution

To fix, I added a configuration option to set the WSDL endpoint:

NetSuite.configure do
  endpoint "https://123456.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1"
end

# or, equivalently
NetSuite::Configuration.endpoint = "https://123456.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1"

# or, to future-proof it
NetSuite::Configuration.endpoint = "#{wsdl_domain}/services/NetSuitePort_#{api_verson}"

This option is passed through to the Savon::Client at the time of client instantiation. Savon then reads the endpoint from its global config when it is building the request:

  def build_request(builder)
      @locals[:soap_action] ||= soap_action
      @globals[:endpoint] ||= endpoint

      request = SOAPRequest.new(@globals).build(
        :soap_action => soap_action,
        :cookies     => @locals[:cookies],
        :headers     => @locals[:headers]
      )

      request.url = endpoint

Note that the endpoint configuration is optional, and Savon ignores nil endpoint configs in favor of the endpoint defined on the WSDL document:

    def endpoint
      @globals[:endpoint] || @wsdl.endpoint.tap do |url|
        if @globals[:host]
          host_url = URI.parse(@globals[:host])
          url.host = host_url.host
          url.port = host_url.port
        end
      end
    end

So this should be backwards compatible.

Testing

I added automated tests. But the nature of the change makes it hard to really test without deeply mocking private methods of Savon classes. So the best tests are probably manual.

By loading these changes into a console I was able to get a record on the 2020_1 version of the NS API:

irb(main):484:0> NetSuite::Configuration.endpoint = "https://1234567.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1"
=> "https://1234567.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1"
irb(main):485:0> cp = NetSuite::Records::CustomerPayment.get(external_id: "xxxxxxxxxx")


I, [2021-03-01T21:36:44.220119 #4]  INFO -- : SOAP request: https://1234567.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1
I, [2021-03-01T21:36:44.220236 #4]  INFO -- : SOAPAction: "get", Content-Type: text/xml;charset=UTF-8, Content-Length: 1293
D, [2021-03-01T21:36:44.221013 #4] DEBUG -- : <?xml version="1.0" encoding="UTF-8"?>
<env:Envelope xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:platformMsgs="urn:messages_2020_1.platform.webservices.netsuite.com" xmlns:env="http://schemas.xmlsoap.org/soap/envelope/" xmlns:platformCore="urn:core_2020_1.platform.webservices.netsuite.com">
  <env:Header>
    <platformMsgs:tokenPassport>
      <platformCore:account>1234567</platformCore:account>
      <platformCore:consumerKey>***FILTERED***</platformCore:consumerKey>
      <platformCore:token>***FILTERED***</platformCore:token>
      <platformCore:nonce>xxxxxxxxxxxx</platformCore:nonce>
      <platformCore:timestamp>1614634604</platformCore:timestamp>
      <platformCore:signature algorithm="HMAC-SHA256">xxxxxxxxxx</platformCore:signature>
    </platformMsgs:tokenPassport>
    <platformMsgs:preferences>
      <platformMsgs:ignoreReadOnlyFields>true</platformMsgs:ignoreReadOnlyFields>
    </platformMsgs:preferences>
  </env:Header>
  ...

Note that the request is sent to the desired endpoint: https://1234567.suitetalk.api.netsuite.com/services/NetSuitePort_2020_1

@davidlaprade
Copy link
Contributor Author

@iloveitaly obviously zero rush, but when you get a chance :)

@iloveitaly
Copy link
Member

Looks great! Thanks.

In another PR, it would be great to add this snippet to the configuration:

NetSuite::Configuration.endpoint = "#{wsdl_domain}/services/NetSuitePort_#{api_verson}"

So users don't need to construct this themselves. Maybe it's something set if the API version is 2020 or greater and if the endpoint value isn't set?

@iloveitaly iloveitaly merged commit 41afd80 into NetSweet:master Mar 4, 2021
@gordysc
Copy link

gordysc commented May 12, 2021

@iloveitaly are we planning on including this in a released version soon?

@iloveitaly
Copy link
Member

@gordysc done! Just pushed 0.8.7

@cgunther
Copy link
Contributor

In the future-proof example:

NetSuite::Configuration.endpoint = "#{wsdl_domain}/services/NetSuitePort_#{api_verson}"

That needs a protocol, right?

NetSuite::Configuration.endpoint = "https://#{wsdl_domain}/services/NetSuitePort_#{api_verson}"

It seems wsdl_domain expects a hostname/domain (without a protocol), but endpoint seems to need that protocol. This tripped me up in upgrading to 0.8.7 and ditching the hacks to properly fix/set endpoint.

I can open up a PR to fix the docs once we confirm that's the issue, not something on my side.

@iloveitaly
Copy link
Member

@cgunther ah, yes, that was a mistake on my part! If could submit a PR to fix that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants