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

xsd:elements defined under a xsd:choice should be correctly represented in the java class #558

Open
Prudhvi-itron opened this issue Aug 7, 2024 · 6 comments
Labels

Comments

@Prudhvi-itron
Copy link

Prudhvi-itron commented Aug 7, 2024

Given an xsd sample:

 <xsd:complexType name="Data">
        <xsd:choice>
            <xsd:element name="null-data" type="NULL" />
            <xsd:element name="array" type="SequenceOfData" />
            <xsd:element name="structure" type="SequenceOfData" />
            <xsd:element name="boolean" type="Boolean" />
            <xsd:element name="bit-string" type="BitString" />
            <xsd:element name="double-long" type="Integer32" />
            <xsd:element name="double-long-unsigned" type="Unsigned32" />
            <xsd:element name="octet-string" type="HexBinary" />
            <xsd:element name="visible-string" type="VisibleString" />
            <xsd:element name="utf8-string" type="Utf8String" />
            <xsd:element name="bcd" type="Integer8" />
      </xsd:choice>
 </xsd:complexType>

This generates a class with setters for each property. That means, we can initialize multiple fields in once object instance of Data. This should be restricted. Ideally, it should have maybe create multiple constructors which accepts only one field.
Like:

public Data(Null null-data)
{this.null-data=null-data;}
public Data(SeqeunceOfData array)
{this.array=array;}

These constructors created for each field would have the correct representation and restricts user to only initialize the Data object with one field.

Data.txt

@laurentschoelens
Copy link
Collaborator

Hi @Prudhvi-itron

I understand quite well what you're talking about but we can't produce this from XJC.
In your example, you have the following :

            <xsd:element name="array" type="SequenceOfData" />
            <xsd:element name="structure" type="SequenceOfData" />

That would lead to creating :

public Data(SequenceOfData array) {
    this.array = array;
}

public Data(SequenceOfData structure) {
    this.structure = structure;
}

Which is invalid java code (same code signature of constructor).
I don't think there is a proper solution with this.

Maybe a new plugin that would insert some code in setters which will assert that each fields of the class is null before setting the actual data ?

@Prudhvi-itron
Copy link
Author

Hi @laurentschoelens , I opened this issue because we are given an standard xsd, we want to create java classes as part of our internal library. Multiple projects will use this library and we don't want the users to have any knowledge on the xsd itself. I want the generated Java classes to be self-sufficient. As of now, the generated classes have no restriction on the number of fields the user can initialize. Ideally, the choice element says that only one of the field should be present inside the object. If I'm not being clear, there is an article on the issue . The only other way we have is to take the created object and validate it against the schema. But it would be great to restrict the users to not let them initialize other fields at all.

Okay, from your above comment, I agree that this scenario would be a problem

public Data(SequenceOfData array) {
    this.array = array;
}

public Data(SequenceOfData structure) {
    this.structure = structure;
}

What if the plugin could generate Builder class with methods like

 public Data withNullData(final String nullData) {
            this.nullData = nullData;
            return build();
        }

        public Data withArray(final SequenceOfData array) {
            this.array = array;
            return build();
        }

        public Data withStructure(final SequenceOfData structure) {
            this.structure = structure;
            return build();
        }

Or as you said, the setters should have the restriction
Maybe a new plugin that would insert some code in setters which will assert that each fields of the class is null before setting the actual data

@mattrpav
Copy link
Collaborator

mattrpav commented Aug 7, 2024

A couple thoughts to consider:

  1. There are unsolvable gaps between XML data and Java objects
  2. Some use cases may wish to have multiple-setters for good reason -- a UI wizard that allows a user to switch items within the choice. You would want to set the new one before removing/null-setting the old one to ensure there is always at least one set.
  3. Choice is a mix of single object and collection (choice supports maxOccurs)
  4. Using constructor-only approach means changing the value of the choice requires throwing away the full object vs a reference update.

choice is an odd paradigm. Not sure the benefit of using it outweighs all the language pain points. Using anyType or extension of types are more readily rationalized into programming languages.

A helper model class (possibly using a JAXB Adapter) may be another option to consider. Create a new class that enforces the desired behavior.

@Prudhvi-itron
Copy link
Author

Prudhvi-itron commented Aug 8, 2024

Hi @mattrpav ,
Choice is a mix of single object and collection (choice supports maxOccurs)
Yes, but the plugin is correctly representing when we have maxOccurs but not when we don't have maxOccurs.

Having a maxOccurs along with choice in the xsd

<xsd:complexType name="Key-Info">
       <xsd:choice maxOccurs="4">
           <xsd:element name="identified-key" type="Identified-Key" />
           <xsd:element name="wrapped-key" type="Wrapped-Key" />
           <xsd:element name="agreed-key" type="Agreed-Key" />
       </xsd:choice>
   </xsd:complexType>

Generates:

public class KeyInfo {
    @XmlElements({
        @XmlElement(name = "identified-key", type = IdentifiedKey.class),
        @XmlElement(name = "wrapped-key", type = WrappedKey.class),
        @XmlElement(name = "agreed-key", type = AgreedKey.class)
    })
    @Generated(value = "com.sun.tools.xjc.Driver", comments = "JAXB RI v2.3.7", date = "2024-08-08T10:50:32+05:30")
    protected List<Object> identifiedKeyOrWrappedKeyOrAgreedKey;

Having something like this in the xsd

  <xsd:complexType name="Key-Info">
        <xsd:choice>
            <xsd:element name="identified-key" type="Identified-Key" />
            <xsd:element name="wrapped-key" type="Wrapped-Key" />
            <xsd:element name="agreed-key" type="Agreed-Key" />
        </xsd:choice>
    </xsd:complexType>

Generates:

public class KeyInfo {
    @XmlElement(name = "identified-key")
    @Generated(value = "com.sun.tools.xjc.Driver", comments = "JAXB RI v2.3.7", date = "2024-08-08T10:44:13+05:30")
    protected IdentifiedKey identifiedKey;
    @XmlElement(name = "wrapped-key")
    @Generated(value = "com.sun.tools.xjc.Driver", comments = "JAXB RI v2.3.7", date = "2024-08-08T10:44:13+05:30")
    protected WrappedKey wrappedKey;
    @XmlElement(name = "agreed-key")
    @Generated(value = "com.sun.tools.xjc.Driver", comments = "JAXB RI v2.3.7", date = "2024-08-08T10:44:13+05:30")
    protected AgreedKey agreedKey;

I think the plugin can follow similar approach? Perhaps a builder pattern which will only initialize the Object with one property (Builder can reset the other properties to null and only retain the last property provided to it).

@laurentschoelens
Copy link
Collaborator

I've taken back your example (with less elements inside xsd:choice tag) :

  <xsd:complexType name="Key-Info">
    <xsd:sequence>
      <xsd:choice>
        <xsd:element name="identified-key" type="xsd:int" />
        <xsd:element name="wrapped-key" type="xsd:string" />
      </xsd:choice>
      <xsd:choice>
        <xsd:element name="another-identified-key" type="xsd:int" />
        <xsd:element name="another-wrapped-key" type="xsd:string" />
      </xsd:choice>
      <xsd:element name="mandatory-key" type="xsd:string" />
    </xsd:sequence>
  </xsd:complexType>

This generates the following :

public class KeyInfo {

    @XmlElement(name = "identified-key")
    protected Integer identifiedKey;
    @XmlElement(name = "wrapped-key")
    protected String wrappedKey;
    @XmlElement(name = "another-identified-key")
    protected Integer anotherIdentifiedKey;
    @XmlElement(name = "another-wrapped-key")
    protected String anotherWrappedKey;
    @XmlElement(name = "mandatory-key", required = true)
    protected String mandatoryKey;

    // ...
}

Having such thing make it more complex to create a builder class (since you will need the mandatoryKey and one of each choice tag, and the "reset setter approach" would be quite complex too.

For the moment, I don't see a simple solution to handle each use case that would be implemented in a new XJC plugin.

@Prudhvi-itron
Copy link
Author

Hi @laurentschoelens, yeah, I could see this would be very complex.

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

3 participants