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

Improve deserialization of inline simpleType declarations #1015

Merged
merged 1 commit into from
Aug 6, 2018

Conversation

kouak
Copy link
Contributor

@kouak kouak commented Jul 21, 2018

I encountered an issue deserialization of elements declared in such way :

<xs:element name="nonICAOAerodromeOfDeparture" minOccurs="0" maxOccurs="1">
  <xs:simpleType>
    <xs:restriction base="xs:boolean">
    </xs:restriction>
  </xs:simpleType>
</xs:element>

I'm no WSDL/XSD expert but I believe this is equivalent to this syntax :

<xs:element name="nonICAOAerodromeOfDeparture" minOccurs="0" maxOccurs="1" type="xs:boolean">
</xs:element>

While node-soap correctly deserializes the latter to a javascript boolean, the former is deserialized to a string (i.e "true" or "false").

Furthermore, the lack of type attribute on the xs:element declaration makes it impossible to use customDeserializer to properly handle such fields.

This PR fixes both aspects by:

  • Allowing xs:element to have a xs:simpleType child
  • Computing xs:element type if missing from child xs:simpleType's name attribute, or restriction's base attribute

This PR was inspired by #949, and aims to achieve the same effect, without breaking existing tests and preserving the ability to use customDeserializer if needed.

See attached test case for the complete behaviour.

NB: This PR includes ES6 syntax and was tested with node v6.5.0 and node v8.9.3 locally.

@coveralls
Copy link

coveralls commented Jul 21, 2018

Coverage Status

Coverage increased (+0.01%) to 93.28% when pulling e3a5c65 on kouak:master into 7da88e1 on vpulim:master.

@herom
Copy link
Contributor

herom commented Jul 29, 2018

LGTM, what about you @jsdevel ?

@herom
Copy link
Contributor

herom commented Aug 6, 2018

thanks a lot for your contribution @kouak , we appreciate it a lot 👍 🚀

@herom herom merged commit c9026a2 into vpulim:master Aug 6, 2018
@kouak
Copy link
Contributor Author

kouak commented Aug 16, 2018

@herom Any chance to get this released ?

We're currently wrapping a large SOAP API in a JS lib which uses those weird definitions. Having this sooner rather than later would save us the trouble to refactor call sites later on.

If this is not much of a hassle for the maintainers, this would be greatly appreciated !

@herom
Copy link
Contributor

herom commented Aug 16, 2018

@kouak I now we've been a lazy with releasing new versions, sorry for that - I will take a look at it tomorrow as I'm a bit tight on schedule 👍

@kouak
Copy link
Contributor Author

kouak commented Aug 20, 2018

@herom: Thanks a lot for the quick release !

@herom
Copy link
Contributor

herom commented Aug 20, 2018

You're welcome - although I wouldn't call my "response time" quick 😸 - sorry for that.

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.

3 participants