castor
  1. castor
  2. CASTOR-716

SourceGenerator doesn't correctly generate the .equals() method for base64binary

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.5.3, 1.0, 1.0.1, 1.0.2, 1.0.3
    • Fix Version/s: 1.0.4
    • Component/s: XML code generator
    • Labels:
      None
    • Environment:
      Operating System: other
      Platform: PC
    • Bugzilla Id:
      1575
    • Testcase included:
      yes
    • Number of attachments :
      4

      Description

      When generating source from a schema that has the type "xsd:base64Binary" the
      equals method generated by the SourceGenerator generates a xxx.equals( yyy )
      where xxx and yyy are of type byte[], from the "xsd:base64Binary" schema type.

      This will always be false, it should be Arrays.equals( xxx, yyy ), or something
      similar.

      Test case details:
      1) SourceGenerator command: java cp .;castor-0.9.5.3.jar;castor-0.9.5.3
      xml.jar;xml-apis.jar;xercesImpl.jar org.exolab.castor.builder.SourceGenerator -
      i "D:\backup\d_drive\Portals\WSRP\castor-bug.xsd" -types j2 -nomarshall

      Attachments to be added:
      -Example schema
      -castorbuilder.properties

      1. castor-716.diff
        14 kB
        Edward Kuns
      2. castor-bug-1575.xsd
        0.7 kB
        derek.sayeau
      3. castor-bug-1575-2.xsd
        0.4 kB
        derek.sayeau
      4. castorbuilder.properties
        2 kB
        derek.sayeau

        Issue Links

          Activity

          Hide
          derek.sayeau added a comment -

          Created an attachment (id=421)
          Example schema.

          Show
          derek.sayeau added a comment - Created an attachment (id=421) Example schema.
          Hide
          derek.sayeau added a comment -

          Created an attachment (id=422)
          Build properties file.

          Show
          derek.sayeau added a comment - Created an attachment (id=422) Build properties file.
          Hide
          Keith Visco added a comment -

          Hi Derek,

          Thanks for the bug report, I'm marking the target milestone as 0.9.5.4.

          Show
          Keith Visco added a comment - Hi Derek, Thanks for the bug report, I'm marking the target milestone as 0.9.5.4.
          Hide
          derek.sayeau added a comment -

          The generated equals also does not work when there is an element that contains
          an <xsd:any/>. This is because the equals() is not implemented on AnyNode.

          Even though the generated signature is setAnyObject( Object ), the unmarshaller
          always passes an "AnyNode" object, so two identical XML files will not
          be "equal" once unmarshalled into objects.

          Attaching an additional example schema.

          Show
          derek.sayeau added a comment - The generated equals also does not work when there is an element that contains an <xsd:any/>. This is because the equals() is not implemented on AnyNode. Even though the generated signature is setAnyObject( Object ), the unmarshaller always passes an "AnyNode" object, so two identical XML files will not be "equal" once unmarshalled into objects. Attaching an additional example schema.
          Hide
          derek.sayeau added a comment -

          Created an attachment (id=424)
          Example schema for <xsd:any/> problem.

          Show
          derek.sayeau added a comment - Created an attachment (id=424) Example schema for <xsd:any/> problem.
          Hide
          Keith Visco added a comment -

          Changing to 0.9.5.5 (or 0.9.6)

          Show
          Keith Visco added a comment - Changing to 0.9.5.5 (or 0.9.6)
          Hide
          Werner Guttmann added a comment -

          When working no this, can we please split this into two separate sub-tasks ?

          Show
          Werner Guttmann added a comment - When working no this, can we please split this into two separate sub-tasks ?
          Hide
          Edward Kuns added a comment -

          You want the Anynode piece to be in a separate subtask? Or are you speaking about a different separation into two sub-tasks?

          Show
          Edward Kuns added a comment - You want the Anynode piece to be in a separate subtask? Or are you speaking about a different separation into two sub-tasks?
          Hide
          Werner Guttmann added a comment -

          Yes, correct.

          Show
          Werner Guttmann added a comment - Yes, correct.
          Hide
          Edward Kuns added a comment -

          Attaching a patch that includes a test csae. This fixes equals() for base64binary.
          Ready for review.

          Show
          Edward Kuns added a comment - Attaching a patch that includes a test csae. This fixes equals() for base64binary. Ready for review.
          Hide
          Edward Kuns added a comment -

          Any thoughts on this fix?

          Show
          Edward Kuns added a comment - Any thoughts on this fix?
          Hide
          Werner Guttmann added a comment -

          Why not ping Ralf on this one ?

          Show
          Werner Guttmann added a comment - Why not ping Ralf on this one ?
          Hide
          Ralf Joachim added a comment -

          Eddie, wouldn't it be better to add a boolean isArray() method to JType and call that one instead of type.toString().endsWith("[]") everywhere?

          Show
          Ralf Joachim added a comment - Eddie, wouldn't it be better to add a boolean isArray() method to JType and call that one instead of type.toString().endsWith("[]") everywhere?
          Hide
          Edward Kuns added a comment -

          Fix, including a new regression test case, has been commited.

          Show
          Edward Kuns added a comment - Fix, including a new regression test case, has been commited.
          Hide
          Werner Guttmann added a comment -

          Ralf, a few weks ago, I removed isArray() and isCollection() methods from JType and introduced two sub-types of JType, namely JCollection and JArray. So let's please not re-introduce isArray() methods .. .

          Show
          Werner Guttmann added a comment - Ralf, a few weks ago, I removed isArray() and isCollection() methods from JType and introduced two sub-types of JType, namely JCollection and JArray. So let's please not re-introduce isArray() methods .. .
          Hide
          Werner Guttmann added a comment -

          Iow, why not change ....

          type.toString().endsWith("[]") e

          to

          type instanceof JArray

          rather than introducing a dummy isArray() method on JType (after we decided to move all collection and/or array related logic to subclasses of JType.

          Show
          Werner Guttmann added a comment - Iow, why not change .... type.toString().endsWith("[]") e to type instanceof JArray rather than introducing a dummy isArray() method on JType (after we decided to move all collection and/or array related logic to subclasses of JType.
          Hide
          Ralf Joachim added a comment -

          As you know the javasource class hierarchy is quite similare to the hierarchy of the classes of java language. The JType class representes Class of java language which offers the isArray() and isCollection() methods. If these methods would have be present in JType, nowone would have come to the idea to test for array by using type.toString().endsWith("[]") . Having said that it is reasonable to have the JCollection and JArray classes you introduced but in my opinon you should have implemented isArray() and isCollection() like:

          public boolean isArray()

          { return (this instanceof JArray); }

          public boolean isCollection()

          { return (this instanceof JCollection); }

          This will allow to test with type.isArray() while decoupling for the underlaying implementation. As you can see at above comments and the current implementation it does not seam to be that obvious to test if JType represents an array by (type instanceof JArray).

          Just my 0.02 cents.

          Show
          Ralf Joachim added a comment - As you know the javasource class hierarchy is quite similare to the hierarchy of the classes of java language. The JType class representes Class of java language which offers the isArray() and isCollection() methods. If these methods would have be present in JType, nowone would have come to the idea to test for array by using type.toString().endsWith("[]") . Having said that it is reasonable to have the JCollection and JArray classes you introduced but in my opinon you should have implemented isArray() and isCollection() like: public boolean isArray() { return (this instanceof JArray); } public boolean isCollection() { return (this instanceof JCollection); } This will allow to test with type.isArray() while decoupling for the underlaying implementation. As you can see at above comments and the current implementation it does not seam to be that obvious to test if JType represents an array by (type instanceof JArray). Just my 0.02 cents.

            People

            • Assignee:
              Edward Kuns
              Reporter:
              derek.sayeau
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: