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

Possible issue with generated DepthFirstSearchTraverserImpl? #6

Closed
marcparmet opened this issue Jun 27, 2015 · 10 comments
Closed

Possible issue with generated DepthFirstSearchTraverserImpl? #6

marcparmet opened this issue Jun 27, 2015 · 10 comments
Labels

Comments

@marcparmet
Copy link
Contributor

The DFS traversal that was generated by your tool gave me a method like this:

public void traverse(HTMLTextType aBean, Visitor<?, E> aVisitor)
    throws E
{
    for (Serializable bean: aBean.getContent()) {
        if (bean instanceof Visitable) {
            ((Visitable) bean).accept(aVisitor);
        }
    }
}

and I had to patch it (using a subclass) to get the traverser to visit everything:

static class PatchedDepthFirstTraverserImpl<E extends Throwable> extends DepthFirstTraverserImpl<E> {
    public void traverse(HTMLTextType aBean, Visitor<?, E> aVisitor) throws E {
        for (Serializable bean: aBean.getContent()) {
            if (bean instanceof JAXBElement<?>) {
                Object v = ((JAXBElement<?>) bean).getValue();
                if (v instanceof Visitable)
                    ((Visitable) v).accept(aVisitor);
            }
            else if (bean instanceof Visitable)
                ((Visitable) bean).accept(aVisitor);
        }
    }
}

The instanceof JAXBElement test is the new code. What does your experience suggest why this is necessary? Thanks. Great tool.

@massfords
Copy link
Owner

See the README for a link to the JAXB tutorial on JAXBElements. In short, there are tweaks to your schema that you can make to avoid JAXBElements. Of course, changing the schema you're working with is not going to work for everybody. There are some tests in place that show how the plugin correctly handles JAXBElements but it seems like you're either working with an older version or have found a bug.

I'm happy to accept a pull request. If you don't want to do the patch yourself, you should provide me with a sample schema that illustrates the problem. If your schema is proprietary, try to create a small example in a dummy namespace with just enough of the elements to recreate the problem so I can debug it.

@marcparmet
Copy link
Contributor Author

Well, modifying the schema isn't really practical -- it's from a third party -- and we're using version 2.2 of your tool. The schema is also big. It's imsqti_v2p1.xsd if you're interested. So this appears to be a bug and I'll try to whittle the schema down to a simpler use-case for you.

@marcparmet
Copy link
Contributor Author

I have a simple, stripped use-case for you. Let me know what you find.

I'll point out one thing. Here's the XSD. The mixed="true" attribute is
critical for producing the problem. I'm guessing the objects that are
created in this case are something you did not anticipate.

<?xml version = "1.0" encoding = "UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">

  <xs:element name="body" type="Body.Type"/>
  <xs:element name="p" type="P.Type"/>

  <xs:complexType name="Body.Type" mixed="false">
    <xs:sequence>
      <xs:choice minOccurs = "0" maxOccurs = "unbounded">
        <xs:element ref="p" />
      </xs:choice>
    </xs:sequence>
  </xs:complexType>

  <!-- The attribute mixed="true" is critical for reproducing the bad behavior. -->
  <xs:complexType name="P.Type" mixed="true">
    <xs:sequence>
      <xs:choice minOccurs = "0" maxOccurs = "unbounded">
        <xs:element ref="p" />
      </xs:choice>
    </xs:sequence>
  </xs:complexType>

</xs:schema>

@marcparmet
Copy link
Contributor Author

You can find the use-case project here for a short time.

@massfords
Copy link
Owner

I'll be able to look at this today. Thanks for the sample project and small schema.

@massfords
Copy link
Owner

Try with the latest snapshot. I'll push to maven central in a few days.

@massfords massfords added the bug label Jul 4, 2015
@marcparmet
Copy link
Contributor Author

This looks good. I can remove my patch and my JUnit tests all pass.

The generated code, while correct, gives me hundreds of warnings in Eclipse because it's using plain JAXBElement instead of JAXBElement<?>. I sent you a pull request that fixes this.

@marcparmet
Copy link
Contributor Author

Would you please push 2.3 to Maven Central? That's either with or without my pull request. Thanks.

@massfords
Copy link
Owner

will look at this today

On Thu, Jul 9, 2015 at 10:14 AM, Marc Parmet notifications@github.com
wrote:

Would you please push 2.3 to Maven Central? That's either with or without
my pull request. Thanks.


Reply to this email directly or view it on GitHub
#6 (comment)
.

@massfords
Copy link
Owner

done

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

No branches or pull requests

2 participants