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

Order of xsd:element and xsd:group not respected #53

Closed
SimonCockx opened this issue Feb 15, 2024 · 5 comments
Closed

Order of xsd:element and xsd:group not respected #53

SimonCockx opened this issue Feb 15, 2024 · 5 comments

Comments

@SimonCockx
Copy link
Contributor

Describe the bug
When parsed, the order of xsd:elements and xsd:groups in a sequence are not respected.

E.g.,

    <xsd:sequence>
          <xsd:element name="elem1" type="xsd:decimal" />
          <xsd:group ref="myGroup" />
          <xsd:element name="elem2" type="Currency" />
    </xsd:sequence>

When running sequence.getElements() on the parsed result, this returns a list of three NamedConcreteElements, but in the wrong order:

  • element named elem1
  • element named elem2
  • group named myGroup

I assume it is due to a bug in the following method defined in the XsdMultipleElements class:

    public void replaceUnsolvedElements(NamedConcreteElement elementWrapper) {
        if (elementWrapper.getElement() instanceof XsdElement){
            super.replaceUnsolvedElements(elementWrapper);
        }

        if (elementWrapper.getElement() instanceof XsdGroup){
            elements.add(elementWrapper);

            this.elements.removeIf(element ->
               element instanceof UnsolvedReference && compareReference(elementWrapper, (UnsolvedReference) element)
            );
        }
    }

Here groups are added to the end of the list while resolving references.

Expected behavior
I would expect the order to be retained.

Library Version
1.2.4

@SimonCockx
Copy link
Contributor Author

SimonCockx commented Feb 15, 2024

@lcduarte I've raised PR #54 where I remove the faulty method. Is there any good reason why replaceUnsolvedElements was overridden?

@SimonCockx
Copy link
Contributor Author

FYI, when cloning master and running a mvn clean install the test testSchemaGetMethods is failing for me. It seems there is a broken test on master.

@lcduarte
Copy link
Member

Hello @SimonCockx

Thanks for using the library and for raising the issue. I've checked the PR and I have my doubts about removing the method entirely, I'll need to review the history of the changes to refresh my memory, but if it passes the tests its most likely correct.

Regarding the failing test, it's weird, I've ran the tests locally on Tuesday and everything was fine, but I had the same issue in the past cloning the repo.

I'll try to check this issue either tomorrow or saturday.

lcduarte added a commit that referenced this issue Feb 16, 2024
@lcduarte
Copy link
Member

It seems that code was some remnants from ancient times 😄

Regarding the error with the test, I didn't quite understood the root cause, but it was related with some order issues while parsing the schemas. I adjusted the test and shouldn't happen again.

I've approved your PR and released version 1.2.6 with the fix. Thanks for your contribution!

@SimonCockx
Copy link
Contributor Author

Thank you for the quick response and reviewing!

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

No branches or pull requests

2 participants