castor
  1. castor
  2. CASTOR-2565

Handling of nill element is not consistent against different types

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Component/s: XML, XML code generator
    • Labels:
      None
    • Environment:
      Java :D
    • Testcase included:
      yes
    • Number of attachments :
      4

      Description

      I am experiencing problems with nillable elements. The behaviour of Castor is not consistent between xs:string and xs:integer elements. While xs:string elements are defaulted to null, as expected, xs:integer defaults to 0 even if the element is declared as nillable and nill.

      I haven't been able to track down the issue in the code, but I'm attaching a simple test case showing my issue there.

        Activity

        Hide
        Denis Cabasson added a comment -

        Find 5 files attached :

        • castorTest.xsd : sample schema
        • castorbuilder.properties : my castor configuration (isn't really significant since the issue is the same with primitive and java 4)
        • generator.xml : ant build script used to create the java classes
        • castor-text.xml : a valid xml file according to the schema
        • CastorTest.java : test case for reading the XML file

        For elem1, the output is correctly null, while for elem2 is output is 0 while I would expect it to be null.

        Show
        Denis Cabasson added a comment - Find 5 files attached : castorTest.xsd : sample schema castorbuilder.properties : my castor configuration (isn't really significant since the issue is the same with primitive and java 4) generator.xml : ant build script used to create the java classes castor-text.xml : a valid xml file according to the schema CastorTest.java : test case for reading the XML file For elem1, the output is correctly null, while for elem2 is output is 0 while I would expect it to be null.
        Hide
        Werner Guttmann added a comment -

        Before looking at this in more detail, let me just highlight a few things:

        a) in normal mode, an <xs:integer>-typed artefact is mapped to a Java property of type

        private long propertyName;
        

        I gues it becomes quite obvious that a scalar data type canno thave a value of null.

        b) It is possible to instruct Castor to use the corresponding Object-types for scalar values, i.e. java.lnag.Long instead of long. Have you tried that ? If not, can you please do so and report back your findings ?

        Show
        Werner Guttmann added a comment - Before looking at this in more detail, let me just highlight a few things: a) in normal mode, an <xs:integer>-typed artefact is mapped to a Java property of type private long propertyName; I gues it becomes quite obvious that a scalar data type canno thave a value of null . b) It is possible to instruct Castor to use the corresponding Object-types for scalar values, i.e. java.lnag.Long instead of long . Have you tried that ? If not, can you please do so and report back your findings ?
        Hide
        Denis Cabasson added a comment -

        Hi Werner,

        I sure did have a look into this option. If you have a closer look at the attached test case, you will find that I'm indeed using the primitivetowrapper option.

        In the case where the mapping is done to primitives, I obviously don't expect the primitives to be null. I would expect the corresponding has***() method to return false, which is not the case. Castor behaves consitently and handles the nill integers values as 0 in every respect (even the validation is triggered, which is not in accordance with the definition of nillable elements), be there primitives or wrappers.

        So I'm pretty confident I'm really reporting a bug. Maybe you could have a look at my attached "test case". If you have direction on how to provide you with a better packaged test-case, I would be happy to provide it to you.

        Thanks for your help!

        Show
        Denis Cabasson added a comment - Hi Werner, I sure did have a look into this option. If you have a closer look at the attached test case, you will find that I'm indeed using the primitivetowrapper option. In the case where the mapping is done to primitives, I obviously don't expect the primitives to be null . I would expect the corresponding has***() method to return false , which is not the case. Castor behaves consitently and handles the nill integers values as 0 in every respect (even the validation is triggered, which is not in accordance with the definition of nillable elements), be there primitives or wrappers. So I'm pretty confident I'm really reporting a bug. Maybe you could have a look at my attached "test case". If you have direction on how to provide you with a better packaged test-case, I would be happy to provide it to you. Thanks for your help!
        Hide
        Werner Guttmann added a comment -

        Thanks, Denis, for taking the time to explain this in detail. Just to make one thing clear(er): I am not asking because I don't trust your ability to detect/reporrt a bug, I am just trying to manage my (unpaid) time as committer. I will have a closer look.

        Show
        Werner Guttmann added a comment - Thanks, Denis, for taking the time to explain this in detail. Just to make one thing clear(er): I am not asking because I don't trust your ability to detect/reporrt a bug, I am just trying to manage my (unpaid) time as committer. I will have a closer look.
        Hide
        Werner Guttmann added a comment -

        Just one (initial) remark.. Yes, there seems to be a bug.But your sample XML has a minor bug, in that it's xsi:nil and not xsi:nill. Castor's behaviour changes slightly, but still causes problems with the <xs:integer> typed element. I guess I will need to debug a bit more ....

        Show
        Werner Guttmann added a comment - Just one (initial) remark.. Yes, there seems to be a bug.But your sample XML has a minor bug, in that it's xsi:nil and not xsi:nill . Castor's behaviour changes slightly, but still causes problems with the <xs:integer> typed element. I guess I will need to debug a bit more ....
        Hide
        Werner Guttmann added a comment - - edited

        Let me ask you a few questions. When you send

        <elem2 xsI.nil="true"/>

        as part of your payload, here's what I'd expect:

        a) primitive: value = 0; hasValue= false
        b) object: value = null

        Correct ?

        Show
        Werner Guttmann added a comment - - edited Let me ask you a few questions. When you send <elem2 xsI.nil="true"/> as part of your payload, here's what I'd expect: a) primitive: value = 0 ; hasValue= false b) object: value = null Correct ?
        Hide
        Denis Cabasson added a comment -

        Sorry for the typo in the XML file. And thanks for your help. Having gone through the stages of being a committer, I know what your are talking of. That's why I'm trying to provide you with a test case as clear as possible.

        Please find attached a mavenized test case, which will certainly be easier to re-launch. The only glitch is that the stable version of the castor-maven-plugin is using castor 1.1.2.1 instead of 1.2. You can probably switch to a snapshot version of the plugin.

        I have created 2 test methods, underlying the issues I find with a nillable integer.

        So you are correct in your assumptions, I would expect elem 2 to be null or to have hasElem2() equals to false (if not using the wrappers).

        Along the same lines, you will see that castor is trying to validate the value of elem2. I have created an elem3, which is a nillable integer as elem2, but with a type not allowing 0. When I include a nil elem3, I get a marshalling exception due to the value 0 not being allowed for elem3.

        I guess that pretty much as far as I can get in describing my issue. I woudl be happy to help you track this issue in your code, but I haven't been able to identify the place where you are handling nillable elements in the code. I have seen the property begin set correctly on the XMLFieldDescriptorImpl, but I don't know where you actually check that property when unmarshalling. I went thourgh the handler but didn't find the relevant place.

        Thanks for your help and your time!

        Show
        Denis Cabasson added a comment - Sorry for the typo in the XML file. And thanks for your help. Having gone through the stages of being a committer, I know what your are talking of. That's why I'm trying to provide you with a test case as clear as possible. Please find attached a mavenized test case, which will certainly be easier to re-launch. The only glitch is that the stable version of the castor-maven-plugin is using castor 1.1.2.1 instead of 1.2. You can probably switch to a snapshot version of the plugin. I have created 2 test methods, underlying the issues I find with a nillable integer. So you are correct in your assumptions, I would expect elem 2 to be null or to have hasElem2() equals to false (if not using the wrappers). Along the same lines, you will see that castor is trying to validate the value of elem2. I have created an elem3, which is a nillable integer as elem2, but with a type not allowing 0. When I include a nil elem3, I get a marshalling exception due to the value 0 not being allowed for elem3. I guess that pretty much as far as I can get in describing my issue. I woudl be happy to help you track this issue in your code, but I haven't been able to identify the place where you are handling nillable elements in the code. I have seen the property begin set correctly on the XMLFieldDescriptorImpl, but I don't know where you actually check that property when unmarshalling. I went thourgh the handler but didn't find the relevant place. Thanks for your help and your time!
        Hide
        Denis Cabasson added a comment - - edited

        Just to make sure our understanding of nillable elements is the same, here is the XML Schema Specifications I'm referring to :

        Validation Rule: Element Locally Valid (Element)
        For an element information item to be locally - valid- with respect to an element declaration all of the following must be true:
        1 The declaration must not be - absent- .
        2 Its {abstract} must be false.
        3 The appropriate case among the following must be true:
        3.1 If {nillable} is false, then there must be no attribute information item among the element information item's [attributes] whose [namespace name] is identical to http://www.w3.org/2001/XMLSchema-instance and whose [local name] is nil.
        3.2 If {nillable} is true and there is such an attribute information item and its - actual value- is true , then all of the following must be true:
        3.2.1 The element information item must have no character or element information item [children].
        3.2.2 There must be no fixed {value constraint}.
        4 If there is an attribute information item among the element information item's [attributes] whose [namespace name] is identical to http://www.w3.org/2001/XMLSchema-instance and whose [local name] is type, then all of the following must be true:
        4.1 The - normalized value- of that attribute information item must be - valid- with respect to the built-in QName simple type, as defined by String Valid (3.14.4);
        4.2 The - local name- and - namespace name- (as defined in QName Interpretation (3.15.3)), of the - actual value- of that attribute information item must resolve to a type definition, as defined in QName resolution (Instance) (3.15.4) – [Definition:] call this type definition the local type definition;
        4.3 The - local type definition- must be validly derived from the {type definition} given the union of the {disallowed substitutions} and the {type definition}'s {prohibited substitutions}, as defined in Type Derivation OK (Complex) (3.4.6) (if it is a complex type definition), or given {disallowed substitutions} as defined in Type Derivation OK (Simple) (3.14.6) (if it is a simple type definition).
        [Definition:] The phrase actual type definition occurs below. If the above three clauses are satisfied, this should be understood as referring to the - local type definition- , otherwise to the {type definition}.
        5 The appropriate case among the following must be true:
        5.1 If the declaration has a {value constraint}, the item has neither element nor character [children] and clause 3.2 has not applied, then all of the following must be true:
        5.1.1 If the - actual type definition- is a - local type definition- then the canonical lexical representation of the {value constraint} value must be a valid default for the - actual type definition- as defined in Element Default Valid (Immediate) (3.3.6).
        5.1.2 The element information item with the canonical lexical representation of the {value constraint} value used as its - normalized value- must be - valid- with respect to the - actual type definition- as defined by Element Locally Valid (Type) (3.3.4).
        5.2 If the declaration has no {value constraint} or the item has either element or character [children] or clause 3.2 has applied, then all of the following must be true:
        5.2.1 The element information item must be - valid- with respect to the - actual type definition- as defined by Element Locally Valid (Type) (3.3.4).
        5.2.2 If there is a fixed {value constraint} and clause 3.2 has not applied, all of the following must be true:
        5.2.2.1 The element information item must have no element information item [children].
        5.2.2.2 The appropriate case among the following must be true:
        5.2.2.2.1 If the {content type} of the - actual type definition- is mixed, then the - initial value- of the item must match the canonical lexical representation of the {value constraint} value.
        5.2.2.2.2 If the {content type} of the - actual type definition- is a simple type definition, then the - actual value- of the item must match the canonical lexical representation of the {value constraint} value.
        6 The element information item must be - valid- with respect to each of the {identity-constraint definitions} as per Identity-constraint Satisfied (3.11.4).
        7 If the element information item is the - validation root- , it must be - valid- per Validation Root Valid (ID/IDREF) (3.3.4).

        Case 3.2 (nil element) is stopping any validation according to my understanding of those specifications.

        Show
        Denis Cabasson added a comment - - edited Just to make sure our understanding of nillable elements is the same, here is the XML Schema Specifications I'm referring to : Validation Rule: Element Locally Valid (Element) For an element information item to be locally - valid- with respect to an element declaration all of the following must be true: 1 The declaration must not be - absent- . 2 Its {abstract} must be false. 3 The appropriate case among the following must be true: 3.1 If {nillable} is false, then there must be no attribute information item among the element information item's [attributes] whose [namespace name] is identical to http://www.w3.org/2001/XMLSchema-instance and whose [local name] is nil. 3.2 If {nillable} is true and there is such an attribute information item and its - actual value- is true , then all of the following must be true: 3.2.1 The element information item must have no character or element information item [children] . 3.2.2 There must be no fixed {value constraint}. 4 If there is an attribute information item among the element information item's [attributes] whose [namespace name] is identical to http://www.w3.org/2001/XMLSchema-instance and whose [local name] is type, then all of the following must be true: 4.1 The - normalized value- of that attribute information item must be - valid- with respect to the built-in QName simple type, as defined by String Valid (3.14.4); 4.2 The - local name- and - namespace name- (as defined in QName Interpretation (3.15.3)), of the - actual value- of that attribute information item must resolve to a type definition, as defined in QName resolution (Instance) (3.15.4) – [Definition:] call this type definition the local type definition; 4.3 The - local type definition- must be validly derived from the {type definition} given the union of the {disallowed substitutions} and the {type definition}'s {prohibited substitutions}, as defined in Type Derivation OK (Complex) (3.4.6) (if it is a complex type definition), or given {disallowed substitutions} as defined in Type Derivation OK (Simple) (3.14.6) (if it is a simple type definition). [Definition:] The phrase actual type definition occurs below. If the above three clauses are satisfied, this should be understood as referring to the - local type definition- , otherwise to the {type definition}. 5 The appropriate case among the following must be true: 5.1 If the declaration has a {value constraint}, the item has neither element nor character [children] and clause 3.2 has not applied, then all of the following must be true: 5.1.1 If the - actual type definition- is a - local type definition- then the canonical lexical representation of the {value constraint} value must be a valid default for the - actual type definition- as defined in Element Default Valid (Immediate) (3.3.6). 5.1.2 The element information item with the canonical lexical representation of the {value constraint} value used as its - normalized value- must be - valid- with respect to the - actual type definition- as defined by Element Locally Valid (Type) (3.3.4). 5.2 If the declaration has no {value constraint} or the item has either element or character [children] or clause 3.2 has applied, then all of the following must be true: 5.2.1 The element information item must be - valid- with respect to the - actual type definition- as defined by Element Locally Valid (Type) (3.3.4). 5.2.2 If there is a fixed {value constraint} and clause 3.2 has not applied, all of the following must be true: 5.2.2.1 The element information item must have no element information item [children] . 5.2.2.2 The appropriate case among the following must be true: 5.2.2.2.1 If the {content type} of the - actual type definition- is mixed, then the - initial value- of the item must match the canonical lexical representation of the {value constraint} value. 5.2.2.2.2 If the {content type} of the - actual type definition- is a simple type definition, then the - actual value- of the item must match the canonical lexical representation of the {value constraint} value. 6 The element information item must be - valid- with respect to each of the {identity-constraint definitions} as per Identity-constraint Satisfied (3.11.4). 7 If the element information item is the - validation root- , it must be - valid- per Validation Root Valid (ID/IDREF) (3.3.4). Case 3.2 (nil element) is stopping any validation according to my understanding of those specifications.
        Hide
        Werner Guttmann added a comment -

        Initial (and partial) patch for review. This patch addresses the main problem area (i..e invalid handling of nils in the context of non-strings), but does not address the validation issue.

        Show
        Werner Guttmann added a comment - Initial (and partial) patch for review. This patch addresses the main problem area (i..e invalid handling of nils in the context of non-strings), but does not address the validation issue.
        Hide
        Werner Guttmann added a comment -

        Actually, this patch seems to be fixing all three issues. If I don't hear anything back from you (about anything I might have missed), I will commit as is.

        Show
        Werner Guttmann added a comment - Actually, this patch seems to be fixing all three issues. If I don't hear anything back from you (about anything I might have missed), I will commit as is.
        Hide
        Werner Guttmann added a comment -

        Actually, just noticed that I have not tested all possible combinations yet. But as I am about to add additional tests to the CTF test suite, I shall find out soon.

        Show
        Werner Guttmann added a comment - Actually, just noticed that I have not tested all possible combinations yet. But as I am about to add additional tests to the CTF test suite, I shall find out soon.
        Hide
        Werner Guttmann added a comment -

        Final patch for review, incl. new tests for the CTF suite.

        Show
        Werner Guttmann added a comment - Final patch for review, incl. new tests for the CTF suite.
        Hide
        Denis Cabasson added a comment -

        Thanks Werner for your very quick reaction and the speed with which you have provided a patch for that.

        I will test it thouroughly on Monday, using it with my real world schema and XML file and let you know how it is working for me.

        Thanks, and keep up the good work!

        Show
        Denis Cabasson added a comment - Thanks Werner for your very quick reaction and the speed with which you have provided a patch for that. I will test it thouroughly on Monday, using it with my real world schema and XML file and let you know how it is working for me. Thanks, and keep up the good work!
        Hide
        Denis Cabasson added a comment -

        Reviewed the patch, and applied it, built a 1.3rc2-SNAPSHOT version. It is now working as expected in regards to nil integer handling.

        Thanks for your work, Werner!

        Show
        Denis Cabasson added a comment - Reviewed the patch, and applied it, built a 1.3rc2-SNAPSHOT version. It is now working as expected in regards to nil integer handling. Thanks for your work, Werner!
        Hide
        Werner Guttmann added a comment -

        Thanks, Denis, for the feedback. I shall commit the changes later on today.

        Show
        Werner Guttmann added a comment - Thanks, Denis, for the feedback. I shall commit the changes later on today.
        Hide
        Werner Guttmann added a comment -

        I still need to commit the patch, as a result of which I will mark the issue as resolved. Once we have made this patch available as part of a AG release, please feel free to mark the issue as closed.

        Show
        Werner Guttmann added a comment - I still need to commit the patch, as a result of which I will mark the issue as resolved. Once we have made this patch available as part of a AG release, please feel free to mark the issue as closed.
        Hide
        Werner Guttmann added a comment -

        fyi, just uploaded a new snapshot release of 1.3rc2 with this patch included.

        Show
        Werner Guttmann added a comment - fyi, just uploaded a new snapshot release of 1.3rc2 with this patch included.

          People

          • Assignee:
            Werner Guttmann
            Reporter:
            Denis Cabasson
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: