JDO
  1. JDO
  2. JDO-453

Create a test for illegal arguments for PersistenceManager.newInstance

    Details

      Description

      New test cases are needed for illegal arguments of newInstance.

      One strategy is to define classes in a new package along with metadata, but no mapping (since these are error classes). A new test should try to instantiate the classes using the newInstance method and verify that JDOUserException is thrown.

      Classes to test include:

      Abstract class with abstract method not declared as a property (missing metadata for the method)
      Interface with method not declared as a property (missing metadata for the method)
      Concrete class with missing public no-args constructor

      1. JDO-453.patch
        57 kB
        Michelle Caisse

        Issue Links

          Activity

          Hide
          Michelle Caisse added a comment -

          Merged changes to tck2-legacy. Completed: At revision: 609039

          Show
          Michelle Caisse added a comment - Merged changes to tck2-legacy. Completed: At revision: 609039
          Hide
          Michelle Caisse added a comment -

          Need to update tck2-legacy

          Show
          Michelle Caisse added a comment - Need to update tck2-legacy
          Hide
          Craig L Russell added a comment -

          I didn't notice until just now that the spec is inconsistent.

          In 12.6.6,
          an abstract class that is declared in the metadata as persistence-capable, in which all abstract methods are persistent properties, or
          an interface that is declared in the metadata as persistence-capable, in which all methods are persistent properties, or...
          If the parameter does not satisfy the above requirements, JDOUserException is thrown.

          But in 18.3,
          Persistent properties declared in the interface are defined as those that have both a get and a set method or both an is and a set method, named according to the JavaBeans naming conventions, and of a type supported as a persistent type.
          The implementing class will provide a suitable implementation for all property access methods and will throw JDOUserException for all other methods of the interface.

          We need to clean up one or the other.

          Show
          Craig L Russell added a comment - I didn't notice until just now that the spec is inconsistent. In 12.6.6, an abstract class that is declared in the metadata as persistence-capable, in which all abstract methods are persistent properties, or an interface that is declared in the metadata as persistence-capable, in which all methods are persistent properties, or... If the parameter does not satisfy the above requirements, JDOUserException is thrown. But in 18.3, Persistent properties declared in the interface are defined as those that have both a get and a set method or both an is and a set method, named according to the JavaBeans naming conventions, and of a type supported as a persistent type. The implementing class will provide a suitable implementation for all property access methods and will throw JDOUserException for all other methods of the interface. We need to clean up one or the other.
          Hide
          Michelle Caisse added a comment -

          Complete with Revision: 604247

          Show
          Michelle Caisse added a comment - Complete with Revision: 604247
          Hide
          Michelle Caisse added a comment -

          Per discussion on JDO-559, I will remove tests in NewInstanceBadMapping and test for impl throwing JDOUserException with a persistent interface having a method that is not a property and an abstract class having an abstract method that is not a property.

          Show
          Michelle Caisse added a comment - Per discussion on JDO-559 , I will remove tests in NewInstanceBadMapping and test for impl throwing JDOUserException with a persistent interface having a method that is not a property and an abstract class having an abstract method that is not a property.
          Hide
          Craig L Russell added a comment -

          Some minor formatting issues and typos in comments. Otherwise, good to go.

          Show
          Craig L Russell added a comment - Some minor formatting issues and typos in comments. Otherwise, good to go.
          Hide
          Michelle Caisse added a comment -

          Previous patches lacked test newInstanceBadMapping.java and the associated mapping files org/apache/jdo/tck.pc/newInstance/package-standard1.orm

          Show
          Michelle Caisse added a comment - Previous patches lacked test newInstanceBadMapping.java and the associated mapping files org/apache/jdo/tck.pc/newInstance/package-standard1.orm
          Hide
          Craig L Russell added a comment -

          Just one comment. This code in AAddress_bad seems to be superfluous:
          + /** This is the JDO-required no-args constructor. The TCK relies on
          + * this constructor for testing PersistenceManager.newInstance(PCClass).
          + */
          + public Address_bad(String street, String city, String state, String zipcode)

          { + this.street = street; + this.city = city; + this.state = state; + this.zipcode = zipcode; + }

          +
          + // pm.newInstance(this) throws JDOUserException with non-public constructor
          + private Address_bad() {}
          +
          I think all you need is

          + // pm.newInstance(this) throws JDOUserException with non-public no-args constructor
          + private Address_bad() {}
          +

          Show
          Craig L Russell added a comment - Just one comment. This code in AAddress_bad seems to be superfluous: + /** This is the JDO-required no-args constructor. The TCK relies on + * this constructor for testing PersistenceManager.newInstance(PCClass). + */ + public Address_bad(String street, String city, String state, String zipcode) { + this.street = street; + this.city = city; + this.state = state; + this.zipcode = zipcode; + } + + // pm.newInstance(this) throws JDOUserException with non-public constructor + private Address_bad() {} + I think all you need is + // pm.newInstance(this) throws JDOUserException with non-public no-args constructor + private Address_bad() {} +
          Hide
          Michelle Caisse added a comment -

          New patch containing *_bad.java

          Show
          Michelle Caisse added a comment - New patch containing *_bad.java
          Hide
          Craig L Russell added a comment -

          I couldn't find the *_bad.java classes in the attachment. I'm expecting that the comments in the class and mapping will further explain why they are not valid classes for the newInstance method.

          Show
          Craig L Russell added a comment - I couldn't find the *_bad.java classes in the attachment. I'm expecting that the comments in the class and mapping will further explain why they are not valid classes for the newInstance method.
          Hide
          Michelle Caisse added a comment -

          Please review.

          Show
          Michelle Caisse added a comment - Please review.
          Hide
          Michelle Caisse added a comment -

          Some simple positive tests with supporting pc classes and metadata was checked in (revision: 526946). There are issues for which other JIRAs will be filed.
          ).

          Show
          Michelle Caisse added a comment - Some simple positive tests with supporting pc classes and metadata was checked in (revision: 526946). There are issues for which other JIRAs will be filed. ).
          Hide
          Michelle Caisse added a comment -

          From the spec (assertion numbers need to be added):

          Object newInstance(Class persistenceCapable);

          A12.6.6-1 [The parameter must be one of the following:

          • an abstract class that is declared in the metadata as persistence-capable, in which all abstract methods are declared as persistent properties, or
          • an interface that is declared in the metadata as persistence-capable, in which all methods are declared as persistent properties, or
          • a concrete class that is declared in the metadata as persistence-capable. In this case, the concrete class must declare a public no-args constructor.

          If the parameter does not satisfy the above requirements, JDOUserException is thrown.]
          ...
          A12.6.6-2 [In order for the newInstance method to be used, the parameter interface must be completely mapped. For relational implementations, the interface must be mapped to a table and all persistent properties must be mapped to columns. Additionally, interfaces that are the targets of all relationships from persistent properties must also be mapped. Otherwise, JDOUserException is thrown by the newInstance method.]

          Show
          Michelle Caisse added a comment - From the spec (assertion numbers need to be added): Object newInstance(Class persistenceCapable); A12.6.6-1 [The parameter must be one of the following: an abstract class that is declared in the metadata as persistence-capable, in which all abstract methods are declared as persistent properties, or an interface that is declared in the metadata as persistence-capable, in which all methods are declared as persistent properties, or a concrete class that is declared in the metadata as persistence-capable. In this case, the concrete class must declare a public no-args constructor. If the parameter does not satisfy the above requirements, JDOUserException is thrown.] ... A12.6.6-2 [In order for the newInstance method to be used, the parameter interface must be completely mapped. For relational implementations, the interface must be mapped to a table and all persistent properties must be mapped to columns. Additionally, interfaces that are the targets of all relationships from persistent properties must also be mapped. Otherwise, JDOUserException is thrown by the newInstance method.]

            People

            • Assignee:
              Michelle Caisse
              Reporter:
              Craig L Russell
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development