treblereel / mapper-xml

j2cl/gwt compatible XML marshallers
Apache License 2.0
3 stars 3 forks source link

Different behaviour of (un)marshaller vs JAXB reference implementation #84

Closed manstis closed 3 years ago

manstis commented 3 years ago

I was trialling the XML (un)marshallers generated by the @XMLMapper annotation.

The issues below do not exist when using the JAXB reference implementation in a normal JRE.

You can clone/fetch https://github.com/manstis/marshaller to see the behaviours:

The issues are:

Deserialisation

The deserialisation does not work correctly for @JAXBElementRefs. It can be made to work with the inclusion of the @XmlUnwrappedCollection annotation on collections however cases that are not collections fail to de-serialise. Furthermore use of the of XmlUnwrappedCollection should not be necessary (as it is not needed by the JAXB reference implementation).

Model class

public class Attribute {
  ...
    @XmlElementRefs({
        @XmlElementRef(name = "SimplePredicate", namespace = "http://www.dmg.org/PMML-4_4", type = SimplePredicate.class, required = false),
        @XmlElementRef(name = "CompoundPredicate", namespace = "http://www.dmg.org/PMML-4_4", type = CompoundPredicate.class, required = false),
        @XmlElementRef(name = "SimpleSetPredicate", namespace = "http://www.dmg.org/PMML-4_4", type = SimpleSetPredicate.class, required = false),
        @XmlElementRef(name = "True", namespace = "http://www.dmg.org/PMML-4_4", type = True.class, required = false),
        @XmlElementRef(name = "False", namespace = "http://www.dmg.org/PMML-4_4", type = False.class, required = false)
    })
    protected ICompoundPredicate predicate;
  ...
}

Generated code (Compare this to how it works for collections with the @XmlUnwrappedCollection annotation. Deserialisation fails as the generated code fails to find a deserialiser for SimplePredicate as only a deserialiser for predicate has been registered).

public class AttributeBeanXMLDeserializerImpl extends AbstractBeanXMLDeserializer<Attribute> {
  ...
map.put("predicate", new BeanPropertyDeserializer<Attribute, com.anstis.pmml.model.ICompoundPredicate>() {

            @Override()
            protected XMLDeserializer<?> newDeserializer(org.treblereel.gwt.xml.mapper.api.stream.XMLReader reader) {
                return new Function<String, XMLDeserializer<com.anstis.pmml.model.ICompoundPredicate>>() {

                    @Override()
                    public XMLDeserializer apply(String value) {
                        if ("True".equals(value))
                            return new com.anstis.pmml.model.TrueBeanXMLDeserializerImpl();
                        if ("CompoundPredicate".equals(value))
                            return new com.anstis.pmml.model.CompoundPredicateBeanXMLDeserializerImpl();
                        if ("False".equals(value))
                            return new com.anstis.pmml.model.FalseBeanXMLDeserializerImpl();
                        if ("SimplePredicate".equals(value))
                            return new com.anstis.pmml.model.SimplePredicateBeanXMLDeserializerImpl();
                        if ("SimpleSetPredicate".equals(value))
                            return new com.anstis.pmml.model.SimpleSetPredicateBeanXMLDeserializerImpl();
                        throw new Error(value);
                    }
                }.apply(xsiTagChooser.apply(reader));
            }
   ...
        });
  ...
}

XML

<Attribute partialScore="-9">
  <SimplePredicate field="department" operator="isMissing"/>
</Attribute>

Serialisation

Generation of the serialisers does not seem to consider use of @JAXBElementRefs at all and the resulting XML does not have element names matching the sub-class names listed in the separate @JAXBElementRef annotations. For example an Attribute POJO (from above) with ICompoundPredicate predicate property equal to an instance of SimplePredicate should have the XML from serialisation as <SimplePredicate ..../> (also as above).

treblereel commented 3 years ago

sorry, missed this ... @manstis thank you for your report, i ll try to fix it asap

manstis commented 3 years ago

I believe the interpretation of XMLElementRefs is incorrect.

The "Ant Task Example" in the JAXB documentation shows that the property name itself (in this example tasks) does not become a container for the list of child XML elements (iAddressListRef in MapperXML's documentation).

<target>
  <jar>
   ....
  </jar>
  <javac>
   ....
  </javac>
</target>

vs

<iAddressListRef>
  <_Address1>
    <address>AAAAA</address>
  </_Address1>
</iAddressListRef>

I am investigating how best to fix.

treblereel commented 3 years ago

@manstis correct me if i am wrong, looks like the fix is just processing the collection, annotated with @XMLElementRefs as XmlUnwrappedCollection ?

so pitty, can't use XmlElementRef because we need to know all subtypes of annotated field to generate (de)marshallers, so only XmlElementRefs left

class Target {
        @XmlElementRef
         List<Task> tasks;
     }
manstis commented 3 years ago

Well, it all depends whether mapper-xml is meant to be a client-side implementation of the JAXB marshallers.

To be honest I was thinking to propose the removal of @XmlUnwrappedCollection as it's not needed by the JAXB reference implementation that runs in the JRE... unless there are insurmountable problems represented client-side; e.g. in order to drop @XmlUnwrappedCollection we'd need reflection to be available. IDK.

I can easily add @XmlUnwrappedCollection when generating the POJOs if we must but I was hoping the serialiser/deserialiser generated with mapper-xml would be able to work only with the JAXB annotations.

Can you explain why @XmlUnwrappedCollection came into being?

manstis commented 3 years ago

I do have another problem where an attempt to deserialise a property is being made by the wrong deserializer.

If run my webapp and click the "Load XML" button the following error is thrown:

Unknown property 'SimplePredicate' in (de)serializer com.anstis.pmml.model.CharacteristicBeanXMLDeserializer

The chain of deserialisers is:

**XML**
... ... ``` - `CharacteristicsBeanXMLDeserializerImpl` handles the `Characteristic` list: **Deserialiser** ``` map.put("Characteristic", new BeanPropertyDeserializer>() { @Override() protected XMLDeserializer newDeserializer(org.treblereel.gwt.xml.mapper.api.stream.XMLReader reader) { return ListXMLDeserializer.newInstance(new Function>() { @Override() public XMLDeserializer apply(String value) { return new com.anstis.pmml.model.CharacteristicBeanXMLDeserializerImpl(); } }); } ``` **POJO** ``` @XmlElement(name = "Characteristic", required = true) protected List characteristic; ``` **XML** ``` ``` - `CharacteristicBeanXMLDeserializerImpl` handles individual `Characteristic`: **Deserialiser** ``` map.put("Attribute", new BeanPropertyDeserializer>() { @Override() protected XMLDeserializer newDeserializer(org.treblereel.gwt.xml.mapper.api.stream.XMLReader reader) { return ListXMLDeserializer.newInstance(new Function>() { @Override() public XMLDeserializer apply(String value) { return new com.anstis.pmml.model.AttributeBeanXMLDeserializerImpl(); } }); } ``` **POJO** ``` @XmlElement(name = "Attribute", required = true) protected List attribute; ``` **XML** ``` ``` - The `SimplePredicate` should be handled by the `Attribute` deserialiser: **Deserialiser** ``` map.put("predicate", new BeanPropertyDeserializer() { @Override() protected XMLDeserializer newDeserializer(org.treblereel.gwt.xml.mapper.api.stream.XMLReader reader) { return new Function>() { @Override() public XMLDeserializer apply(String value) { if ("True".equals(value)) return new com.anstis.pmml.model.TrueBeanXMLDeserializerImpl(); if ("CompoundPredicate".equals(value)) return new com.anstis.pmml.model.CompoundPredicateBeanXMLDeserializerImpl(); if ("False".equals(value)) return new com.anstis.pmml.model.FalseBeanXMLDeserializerImpl(); if ("SimplePredicate".equals(value)) return new com.anstis.pmml.model.SimplePredicateBeanXMLDeserializerImpl(); if ("SimpleSetPredicate".equals(value)) return new com.anstis.pmml.model.SimpleSetPredicateBeanXMLDeserializerImpl(); throw new Error(value); } }.apply(xsiTagChooser.apply(reader)); } ``` **POJO** ``` protected List extension; @XmlElementRefs({ @XmlElementRef(name = "SimplePredicate", namespace = "http://www.dmg.org/PMML-4_4", type = SimplePredicate.class, required = false), @XmlElementRef(name = "CompoundPredicate", namespace = "http://www.dmg.org/PMML-4_4", type = CompoundPredicate.class, required = false), @XmlElementRef(name = "SimpleSetPredicate", namespace = "http://www.dmg.org/PMML-4_4", type = SimpleSetPredicate.class, required = false), @XmlElementRef(name = "True", namespace = "http://www.dmg.org/PMML-4_4", type = True.class, required = false), @XmlElementRef(name = "False", namespace = "http://www.dmg.org/PMML-4_4", type = False.class, required = false) }) protected ICompoundPredicate predicate; ``` **XML** ``` ``` As you can see; I have a `@XmlElementRefs` on a NON-collection field. IDK if this is the bug (support for this scenario).
treblereel commented 3 years ago

@manstis thanks you for your detailed report, i am working on it right now.

treblereel commented 3 years ago

@manstis i hope now it's fixed https://github.com/treblereel/mapper-xml/pull/85

the only thing isn't really clear for me, is namespace processing, but looks like you don't need it

treblereel commented 3 years ago

@XmlElements is also must be fixed ...

manstis commented 3 years ago

@treblereel I checked out your branch and built locally.

Unfortunately my "PMML" model still presents the Unknown property 'SimplePredicate' in (de)serializer com.anstis.pmml.model.CharacteristicBeanXMLDeserializerImpl error I reported. Did you try my use case?

tiagobento commented 3 years ago

Mirrored this on https://issues.redhat.com/browse/KOGITO-5457. Please continue the discussions here.

treblereel commented 3 years ago

@manstis ok, one moment plz ...

treblereel commented 3 years ago

@manstis hmm, no exception

  @Test
  public void test1() throws XMLStreamException {

    Attribute attribute = new Attribute();
    SimplePredicate simplePredicate = new SimplePredicate();

    attribute.setPredicate(simplePredicate);
    String xml = mapper.write(attribute);

    System.out.println("RESULT \n" + xml);

    assertEquals(xml, mapper.write(mapper.read(mapper.write(attribute))));
  }

with branch @XmlElementRefs_fix

manstis commented 3 years ago

If I run the equivalent in the JRE using the JAXB reference implementation, it works fine:

Attribute attribute = new Attribute();
SimplePredicate simplePredicate = new SimplePredicate();
attribute.setPredicate(simplePredicate);

JAXBContext context = JAXBContext.newInstance(Attribute.class);
context.createMarshaller().marshal(attribute, System.out);

This therefore remains an issue with the mapper-xml generated (JAXB) unmarshaller :-(

manstis commented 3 years ago

@manstis hmm, no exception

  @Test
  public void test1() throws XMLStreamException {

    Attribute attribute = new Attribute();
    SimplePredicate simplePredicate = new SimplePredicate();

    attribute.setPredicate(simplePredicate);
    String xml = mapper.write(attribute);

    System.out.println("RESULT \n" + xml);

    assertEquals(xml, mapper.write(mapper.read(mapper.write(attribute))));
  }

with branch @XmlElementRefs_fix

BTW, it's the unmarshalling that is failing; not the marshalling. See the JAXB RI equivalent here.

treblereel commented 3 years ago

@manstis here is marshalling -> unmarshalling->marshalling -> unmarshalling

it doesn't throw an exception

  Attribute_XMLMapperImpl mapper = Attribute_XMLMapperImpl.INSTANCE;

  assertNotNull(
        mapper.read(
            "<?xml version='1.0' encoding='UTF-8'?><Attribute><Extension/><SimplePredicate><Extension/></SimplePredicate></Attribute>"));

ps: it's not a master, it's branch @XmlElementRefs_fix

manstis commented 3 years ago

Yes, I know it's on the branch @treblereel.

Can you try the whole PMML file I am using and not just the Attribute snippet? The issue seems to be when unmarshalling a collection of Attributes within a Characteristic.

treblereel commented 3 years ago

yeap, sure

manstis commented 3 years ago

@treblereel FYI, I've re-checked I'm using the correct mapper-xml branch locally and that the unmarshalling error remains.

treblereel commented 3 years ago

@manstis i have added PMMLTest.java based on your pmml beans, no exception :(

May i ask you to attach your xml ?

manstis commented 3 years ago

@treblereel Sure, I appreciate your time investigating. It is available here.

manstis commented 3 years ago

It unmarshals fine using JAXB's Reference Implementation.

https://github.com/manstis/marshaller/blob/main/marshaller-jre/src/main/java/com/anstis/pmml/jaxb/Main.java#L24

manstis commented 3 years ago

@treblereel The XML produced from the generated marshallers is wrong:

<?xml version='1.0' encoding='UTF-8'?>
<PMML>
<AnomalyDetectionModel>
  <Extension/>
</AnomalyDetectionModel>
<Scorecard baselineMethod="other" reasonCodeAlgorithm="pointsBelow">
  <Extension/>
  <Characteristics>
    <Extension/>
  <Characteristic>
    <Characteristic>
      <Extension/>
      <Attribute>
        <Attribute reasonCode="code -539804736" partialScore="0.7535918206821989">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
        <Attribute reasonCode="code -2034742762" partialScore="0.6081749931167616">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
        <Attribute reasonCode="code -310489214" partialScore="0.37911382999971766">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
        <Attribute reasonCode="code 1002600342" partialScore="0.4530774516551558">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
        <Attribute reasonCode="code 2000510915" partialScore="0.8483799343127182">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
        <Attribute reasonCode="code 1357614927" partialScore="0.6452062432246841">
          <Extension/>
          <SimplePredicate>
            <Extension/>
          </SimplePredicate>
        </Attribute>
      </Attribute>
    </Characteristic>
  </Characteristic>
</Characteristics>
</Scorecard>
<Extension/>
</PMML>

Please note the double Characteristic and Attribute elements:

  <Characteristic>
    <Characteristic>
      <Extension/>
      <Attribute>
        <Attribute reasonCode="code -539804736" partialScore="0.7535918206821989">
    ...
treblereel commented 3 years ago

@manstis you were right, my collection (de)ser is wrong, so i reimplemented it to be more jaxb compatible.

so remarshalled https://raw.githubusercontent.com/manstis/marshaller/main/marshaller-jre/src/main/resources/jaxb/pmml.xml

is:

<?xml version='1.0' encoding='UTF-8'?>
<PMML version="4.4">
    <Header copyright="www.dmg.org" description="Sample scorecard">
        <Timestamp/>
    </Header>
    <DataDictionary>
        <DataField optype="categorical" dataType="string" isCyclic="0" name="department"/>
        <DataField optype="continuous" dataType="integer" isCyclic="0" name="age"/>
        <DataField optype="continuous" dataType="double" isCyclic="0" name="income"/>
        <DataField optype="continuous" dataType="double" isCyclic="0" name="overallScore"/>
    </DataDictionary>
    <Scorecard functionName="regression" baselineMethod="other" initialScore="0.0" modelName="SampleScorecard" useReasonCodes="true" reasonCodeAlgorithm="pointsBelow">
        <MiningSchema>
            <MiningField invalidValueTreatment="asMissing" name="department" outliers="asIs" usageType="active"/>
            <MiningField invalidValueTreatment="asMissing" name="age" outliers="asIs" usageType="active"/>
            <MiningField invalidValueTreatment="asMissing" name="income" outliers="asIs" usageType="active"/>
            <MiningField invalidValueTreatment="returnInvalid" name="overallScore" outliers="asIs" usageType="predicted"/>
        </MiningSchema>
        <Output>
            <OutputField ruleFeature="consequent" dataType="double" isMultiValued="0" rankBasis="confidence" optype="continuous" feature="predictedValue" name="Final Score" rank="1" rankOrder="descending" algorithm="exclusiveRecommendation"/>
            <OutputField ruleFeature="consequent" dataType="string" isMultiValued="0" rankBasis="confidence" optype="categorical" feature="reasonCode" name="Reason Code 1" rank="1" rankOrder="descending" algorithm="exclusiveRecommendation"/>
            <OutputField ruleFeature="consequent" dataType="string" isMultiValued="0" rankBasis="confidence" optype="categorical" feature="reasonCode" name="Reason Code 2" rank="2" rankOrder="descending" algorithm="exclusiveRecommendation"/>
            <OutputField ruleFeature="consequent" dataType="string" isMultiValued="0" rankBasis="confidence" optype="categorical" feature="reasonCode" name="Reason Code 3" rank="3" rankOrder="descending" algorithm="exclusiveRecommendation"/>
        </Output>
        <Characteristics>
            <Characteristic baselineScore="19.0" name="departmentScore" reasonCode="RC1">
                <Attribute partialScore="-9.0">
                    <SimplePredicate field="department" operator="isMissing"/>
                </Attribute>
                <Attribute partialScore="19.0">
                    <SimplePredicate field="department" value="marketing" operator="equal"/>
                </Attribute>
                <Attribute partialScore="3.0">
                    <SimplePredicate field="department" value="engineering" operator="equal"/>
                </Attribute>
                <Attribute partialScore="6.0">
                    <SimplePredicate field="department" value="business" operator="equal"/>
                </Attribute>
            </Characteristic>
            <Characteristic baselineScore="18.0" name="ageScore" reasonCode="RC2">
                <Attribute partialScore="-1.0">
                    <SimplePredicate field="age" operator="isMissing"/>
                </Attribute>
                <Attribute partialScore="-3.0">
                    <SimplePredicate field="age" value="18" operator="lessOrEqual"/>
                </Attribute>
                <Attribute partialScore="0.0">
                    <CompoundPredicate booleanOperator="and">
                        <SimplePredicate field="age" value="18" operator="greaterThan"/>
                        <SimplePredicate field="age" value="29" operator="lessOrEqual"/>
                    </CompoundPredicate>
                </Attribute>
                <Attribute partialScore="12.0">
                    <CompoundPredicate booleanOperator="and">
                        <SimplePredicate field="age" value="29" operator="greaterThan"/>
                        <SimplePredicate field="age" value="39" operator="lessOrEqual"/>
                    </CompoundPredicate>
                </Attribute>
                <Attribute partialScore="18.0">
                    <SimplePredicate field="age" value="39" operator="greaterThan"/>
                </Attribute>
            </Characteristic>
            <Characteristic baselineScore="10.0" name="incomeScore" reasonCode="RC3">
                <Attribute partialScore="5.0">
                    <SimplePredicate field="income" operator="isMissing"/>
                </Attribute>
                <Attribute partialScore="26.0">
                    <SimplePredicate field="income" value="1000" operator="lessOrEqual"/>
                </Attribute>
                <Attribute partialScore="5.0">
                    <CompoundPredicate booleanOperator="and">
                        <SimplePredicate field="income" value="1000" operator="greaterThan"/>
                        <SimplePredicate field="income" value="2500" operator="lessOrEqual"/>
                    </CompoundPredicate>
                </Attribute>
                <Attribute partialScore="-3.0">
                    <SimplePredicate field="income" value="2500" operator="greaterThan"/>
                </Attribute>
            </Characteristic>
        </Characteristics>
    </Scorecard>
</PMML>
treblereel commented 3 years ago

and yeah, @XmlUnwrappedCollection is totally useless and must me eliminated

manstis commented 3 years ago

Hi @treblereel I can confirm your changes in #85 resolve the issues I've reported. Many thanks.

I have some other issues now but I will investigate what may be causing them (if I marshal the classes to XML I am getting quite a few attributes set that were not in the original XML). IDK whether it's an unmarshalling or marshalling issue at the moment; or even something else. I'll investigate and let your know.

For now, please feel free to merge #85, resolve this issue and close https://issues.redhat.com/browse/KOGITO-5457.