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

Fix for circular descriptions #1101

Merged
merged 1 commit into from
Jan 13, 2020
Merged

Fix for circular descriptions #1101

merged 1 commit into from
Jan 13, 2020

Conversation

mastodon0
Copy link
Contributor

There seems to be an edge case when using refs in wsdls like this:

      <xsd:element name="ExampleContent" type="v1:ExampleContent"/>
      <xsd:element name="ExampleRequest" type="v1:ExampleRequestType"/>

      <xsd:complexType name="ExampleContent">
        <xsd:sequence>
          <xsd:element name="MyID" type="xsd:string" />
        </xsd:sequence>
      </xsd:complexType>

      <xsd:complexType name="ExampleRequestType">
        <xsd:sequence>
          <xsd:element ref="v1:ExampleContent"/>
        </xsd:sequence>
      </xsd:complexType>

The reference to ExampleContent would cause a circular description because there is an element and type of the same name.

@coveralls
Copy link

coveralls commented Jan 10, 2020

Coverage Status

Coverage decreased (-0.2%) to 94.826% when pulling 9ceed91 on mastodon0:ref-type into af93a6f on vpulim:master.

@jsdevel
Copy link
Collaborator

jsdevel commented Jan 10, 2020

thanks @mastodon0 . can you add more test cases to keep coverage up?

* edge case when ref element and type have the same name
* increased test coverage for elements.ts
@mastodon0
Copy link
Contributor Author

I could add an additional test but it would be unrelated to the fixed bug.
This looks like a problem with coveralls since it doesn't list any file that got decreased coverage.

@jsdevel
Copy link
Collaborator

jsdevel commented Jan 13, 2020

yea, it does seem like something is up. thanks!

@jsdevel jsdevel merged commit 322bdf1 into vpulim:master Jan 13, 2020
@jsdevel
Copy link
Collaborator

jsdevel commented Jan 13, 2020

actually, @mastodon0 could it be with this fix that we now have dead code elsewhere?

@mastodon0
Copy link
Contributor Author

I looked into that and doubt it. I know it sounds like the classic "it works on my machine" but running npm run cover has higher coverage after the commit than before.

@jsdevel
Copy link
Collaborator

jsdevel commented Jan 16, 2020

ok. thanks for checking @mastodon0

Danail-Irinkov pushed a commit to Danail-Irinkov/node-soap that referenced this pull request Jan 19, 2020
* edge case when ref element and type have the same name
* increased test coverage for elements.ts
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