Modello

Modello XPP3 Reader does not honor <required>true</required> in model.

Details

  • Type: Bug Bug
  • Status: Reopened Reopened
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.0-alpha-4
  • Fix Version/s: None
  • Component/s: modello-plugin-xpp3
  • Labels:
    None
  • Complexity:
    Intermediate
  • Patch Submitted:
    Yes
  • Number of attachments :
    1

Description

Xpp3ReaderGenerator does not generate the appropriate tests for <required>true</required> in the model.

Issue Links

is depended upon by

Bug - A problem which impairs or prevents the functions of the product. MODELLO-225 "required" field descriptor not honoured

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.
relates to

Bug - A problem which impairs or prevents the functions of the product. MODELLO-176 Strict mode does not detect problems when data fields are encoded as XML attributes

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Joakim Erdfelt added a comment -

Attached: MODELLO-44-required-support-in-xpp3.patch

Added abiltity for Generated Reader to honor <required>true</required> from the model when running under strict mode.

Show
Joakim Erdfelt added a comment - Attached: MODELLO-44 -required-support-in-xpp3.patch Added abiltity for Generated Reader to honor <required>true</required> from the model when running under strict mode.
Hide
Permalink
Brett Porter added a comment -

rolled back r741 (which can be used instead of the patches to re-apply).

Problems:

It turns out the model is largely incorrectly specified (saying <required> when it is not, eg plugin >version). The patch also had a "bug" where if <defaultValue> was set it would still choke. I think in the model, required and defaultValue should just be mutually exclusive.

Then... there is inheritence to consider. Really, we need to split the required check out and perform it after inheritence is applied.

Show
Brett Porter added a comment - rolled back r741 (which can be used instead of the patches to re-apply). Problems: It turns out the model is largely incorrectly specified (saying <required> when it is not, eg plugin >version). The patch also had a "bug" where if <defaultValue> was set it would still choke. I think in the model, required and defaultValue should just be mutually exclusive. Then... there is inheritence to consider. Really, we need to split the required check out and perform it after inheritence is applied.
Hide
Permalink
Torben S. Giesselmann added a comment -

Hmmm ... AFAICT the <required> feature is completely disabled right now. The method private String getRequiredAttributeValue( String s, String attribute, XmlPullParser parser, boolean strict) is not referenced at all in the generated {{Xpp3Reader}}s.

What's the status on this issue? Any plans to change that? Couldn't we give the plugin an option whether to honor <required> constraints? That wouldn't break any existing (erroneous) models.

Show
Torben S. Giesselmann added a comment - Hmmm ... AFAICT the <required> feature is completely disabled right now. The method private String getRequiredAttributeValue( String s, String attribute, XmlPullParser parser, boolean strict) is not referenced at all in the generated {{Xpp3Reader}}s. What's the status on this issue? Any plans to change that? Couldn't we give the plugin an option whether to honor <required> constraints? That wouldn't break any existing (erroneous) models.
Hide
Permalink
Dennis Lundberg added a comment -

Having a configuration option to turn on strict handling of required attributes is the way to go. As Torben said, this option will be turned off by default, preserving the currect behavior.

Show
Dennis Lundberg added a comment - Having a configuration option to turn on strict handling of required attributes is the way to go. As Torben said, this option will be turned off by default, preserving the currect behavior.

People

  • Assignee:
    Unassigned
    Reporter:
    Joakim Erdfelt
Vote (1)
Watch (1)

Dates

  • Created:
    Updated: