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

NPE in ClassDiscoverer::handleXmlElement #42

Closed
Warpten opened this issue Jul 13, 2023 · 4 comments · Fixed by #43
Closed

NPE in ClassDiscoverer::handleXmlElement #42

Warpten opened this issue Jul 13, 2023 · 4 comments · Fixed by #43

Comments

@Warpten
Copy link

Warpten commented Jul 13, 2023

ClassDiscoverer#handleXmlElement fails with an NPE on type when processing the following XSD file. This is (I think) because the generated Java classes (at least, using the codehaus plugin) do not have a type associated with the XmlElement annotation for the base type:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
    <xs:element name="DerivedA" type="DerivedA"/>
    <xs:element name="BaseType" type="BaseType"/>

    <xs:element name="TypeList" type="TypeList"/>

    <xs:element name="DerivedD" type="DerivedD" />

    <xs:complexType name="TypeList">
        <xs:sequence>
            <xs:choice minOccurs="0" maxOccurs="unbounded">
                <xs:element ref="BaseType"/>
                <xs:element ref="DerivedA"/>
                <xs:element ref="DerivedD"/>
            </xs:choice>
        </xs:sequence>
    </xs:complexType>

    <xs:complexType name="BaseType">
        <xs:sequence>
            <xs:element name="localisations" minOccurs="0">
                <xs:complexType>
                    <xs:sequence>
                        <xs:element name="localisations" type="xs:string" nillable="true" minOccurs="0" maxOccurs="unbounded"/>
                    </xs:sequence>
                </xs:complexType>
            </xs:element>
        </xs:sequence>
    </xs:complexType>

    <xs:complexType name="AbstractBase">
        <xs:complexContent>
            <xs:extension base="PersistingOject">
                <xs:sequence>
                    <xs:element name="label" type="xs:string" minOccurs="0" />
                    <xs:element name="internalId" type="xs:long" minOccurs="0" />
                </xs:sequence>
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>

    <xs:complexType name="PersistingOject">
        <xs:sequence>
            <xs:element name="id" type="xs:string" minOccurs="0"/>
        </xs:sequence>
    </xs:complexType>

    <xs:complexType name="DerivedA">
        <xs:complexContent>
            <xs:extension base="BaseType">
                <xs:sequence>
                    <xs:element name="category" type="xs:int" minOccurs="0"/>
                </xs:sequence>
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>

    <xs:complexType name="DerivedD">
        <xs:complexContent>
            <xs:extension base="BaseType">
                <xs:sequence>
                    <xs:element name="nodeId" type="xs:string" minOccurs="0"/>
                </xs:sequence>
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>
</xs:schema>
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(name = "TypeList", propOrder = {
    "baseTypeOrDerivedAOrDerivedD"
})
public class TypeList {

    @XmlElements({
        @XmlElement(name = "BaseType"), // <-- !
        @XmlElement(name = "DerivedA", type = DerivedA.class),
        @XmlElement(name = "DerivedD", type = DerivedD.class)
    })
    protected List<BaseType> baseTypeOrDerivedAOrDerivedD;

    // ...
}

I just ended up checking for nullity and that fixed it (obviously), but I'm not sure that that's the correct fix.

java.lang.NullPointerException: Cannot invoke "com.sun.codemodel.JAnnotationValue.generate(com.sun.codemodel.JFormatter)" because "type" is null
	at com.massfords.jaxb.codegen.ClassDiscoverer.handleXmlElement(ClassDiscoverer.java:114)
	at com.massfords.jaxb.codegen.ClassDiscoverer.lambda$parseXmlAnnotations$6(ClassDiscoverer.java:95)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at com.massfords.jaxb.codegen.ClassDiscoverer.parseXmlAnnotations(ClassDiscoverer.java:95)
	at com.massfords.jaxb.codegen.ClassDiscoverer.lambda$discoverDirectClasses$0(ClassDiscoverer.java:62)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at com.massfords.jaxb.codegen.ClassDiscoverer.lambda$discoverDirectClasses$1(ClassDiscoverer.java:51)
	at java.base/java.util.LinkedHashMap$LinkedValues.forEach(LinkedHashMap.java:647)
	at com.massfords.jaxb.codegen.ClassDiscoverer.discoverDirectClasses(ClassDiscoverer.java:48)
	at com.massfords.jaxb.VisitorPlugin.run(VisitorPlugin.java:159)
	at com.sun.tools.xjc.model.Model.generateCode(Model.java:262)
	at com.sun.tools.xjc.Driver.run(Driver.java:359)
	at org.codehaus.mojo.jaxb2.javageneration.AbstractJavaGeneratorMojo.performExecution(AbstractJavaGeneratorMojo.java:476)
	at org.codehaus.mojo.jaxb2.AbstractJaxbMojo.execute(AbstractJaxbMojo.java:337)
	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81)
	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:957)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:289)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:193)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)

I can't modify the .xsd files as they are exposed and generated by a service I consume.

@Warpten
Copy link
Author

Warpten commented Jul 13, 2023

Looking at this again, if the type attribute is missing from an annotation, it's expected to be derived from the property type itself. Now, I don't know codemodel that much (and am having trouble debugging it to inspect it in greater details), but I assume this should rather be fixed like so:

            jfv.annotations().stream()
                    .filter(jau -> Utils.isXmlElements(jau.getAnnotationClass()))
                    .map(jau -> (JAnnotationArrayMember) jau.getAnnotationMembers().get("value"))
                    .flatMap(value -> value.annotations().stream())
                    .forEach(anno -> {
                        JAnnotationValue type = anno.getAnnotationMembers().get("type");
                        if (type != null) {
                            handleXmlElement(outline, directClasses, type);
                        } else {
                            if (jfv.type().elementType() != null) { // Adapt this for List<T>. This only works for T[].
                                handleXmlElement(outline, directClasses, jfv.type().elementType());
                            }
                        }
                    });

And changing the final argument to handleXmlElement to a JGenerable

@massfords
Copy link
Owner

@Warpten thanks for reporting this. Can you provide a full example so I can add it as a test case?

I'm wondering if the else should invoke the addIfDirectClass helper like so:

JClass elementType = (JClass) jfv.type().elementType();
List<JClass> typeParameters = elementType.getTypeParameters();
if (typeParameters != null && typeParameters.get(0) != null) {
   addIfDirectClass(directClasses, typeParameters.get(0));
} else {
   addIfDirectClass(directClasses, elementType);
}

It would help to have some schemas and different settings to test with.

@Warpten
Copy link
Author

Warpten commented Jul 18, 2023

Sorry, I forgot about this.

A test case should be as simple as reusing the codehaus unit test with the sample XSD file I provided (it's self sufficient). Alternatively, I think this triggers as soon as the base type in an inheritance layout is prsent in a <xs:choice>'s possible value types:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
    <xs:element name="DerivedA" type="DerivedA"/>
    <xs:element name="BaseType" type="BaseType"/>

    <xs:element name="TypeList" type="TypeList"/>

    <xs:element name="DerivedD" type="DerivedD" />

    <xs:complexType name="TypeList">
        <xs:sequence>
            <xs:choice minOccurs="0" maxOccurs="unbounded">
                <xs:element ref="DerivedA"/>
                <xs:element ref="BaseType"/>
                <xs:element ref="DerivedD"/>
            </xs:choice>
        </xs:sequence>
    </xs:complexType>

    <xs:complexType name="BaseType" />

    <xs:complexType name="DerivedA">
        <xs:complexContent>
            <xs:extension base="BaseType">
                <xs:sequence />
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>

    <xs:complexType name="DerivedD">
        <xs:complexContent>
            <xs:extension base="BaseType">
                <xs:sequence />
            </xs:extension>
        </xs:complexContent>
    </xs:complexType>
</xs:schema>

This is enough to trigger the NPE when used with the basic-codehaus test.

As far as fixing this goes, I also tried this approach:

            jfv.annotations().stream()
                    .filter(jau -> Utils.isXmlElements(jau.getAnnotationClass()))
                    .map(jau -> (JAnnotationArrayMember) jau.getAnnotationMembers().get("value"))
                    .flatMap(value -> value.annotations().stream())
                    .forEach(anno -> {
                        JAnnotationValue type = anno.getAnnotationMembers().get("type");
                        if (type != null) {
                            handleXmlElement(outline, directClasses, type);
                        } else {
                            JClass jcs = (JClass) jfv.type();
                            if (Objects.equals(outline.getCodeModel().ref(List.class), jcs.erasure())) {
                                JClass elementType = jcs.getTypeParameters().get(0);
                                handleXmlElement(outline, directClasses, elementType);
                            } else {
                               handleXmlElement(outline, directClasses, jcs);
                            }
                        }
                    });

But I don't know that it is correct.

@massfords
Copy link
Owner

@Warpten I added your schema as a test case and took the simple approach of checking for the type and then handling an array or collection. Thanks for the report and suggestions.

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 a pull request may close this issue.

2 participants