Hi Erik,Thanks for your efforts here in debugging the TCK. We had anticipated the issues you found, but were unable to test our implementation of the comparison methods without an implementation of the interfaces that weren't also implementations of the classes.
Michael and I are working on a patch and a JIRA issue. The patch will modify the compare(Object, Object) methods of the top level classes and fix the deepCompareFields for Department and FullTimeEmployee. In addition, some of the methods in EqualityHelper need to be changed as well.
The objective is to avoid adding Comparable or Comparator methods to the interfaces, as there is no JDO-specified requirement to implement these for persistent interfaces.
Michael,Erik also found this in EqualityHelper at line 378. This incorrect code should be removed, as Department.class is not assignable from IDepartmentImpl.class.
if (!me.getClass().isAssignableFrom(other.getClass())) {
logUnequal(me, other, where + msgIncompatibleTypes);
return false;
}
Craig
On Apr 6, 2006, at 7:48 AM, Erik Bengtson wrote:
Hi I have a patch here for some issues. It may be useful or not for you.There are other issues that I'm working on.Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Department.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Department.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Department.java (workingcopy) @@ -246,7 +246,7 @@ */ public boolean deepCompareFields(Object other, EqualityHelper helper) { - Department otherDept = (Department)other; + IDepartment otherDept = (IDepartment)other; String where = "Department<" + deptid + ">"; returnhelper.equals(deptid, otherDept.getDeptid(), where + ".deptid") &@@ -294,7 +294,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Department)o1).compareTo(o2); + IDepartment other = (IDepartment)o2; + long otherId = other.getDeptid(); + return (((IDepartment)o1).getDeptid() < otherId ? -1 : (((IDepartment)o1).getDeptid() == otherId ? 0 : 1)); } /** Index:D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ CompanyFactoryAbstractImpl.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ CompanyFactoryAbstractImpl.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ CompanyFactoryAbstractImpl.java (workingcopy) @@ -132,6 +132,7 @@public IFullTimeEmployee newFullTimeEmployee(long personid, String first, String last, String middle, java.util.Date born, java.util.Date hired, doublesal) { IFullTimeEmployee result = newFullTimeEmployee();if (debug) logger.debug("newFullTimeEmployee returned" + result);+ logger.warn("newFullTimeEmployee returned. born" + born); result.setPersonid(personid); result.setFirstname(first); result.setLastname(last); @@ -145,6 +146,7 @@public IFullTimeEmployee newFullTimeEmployee(long personid, String first, String last, String middle, java.util.Date born, IAddress addr, java.util.Datehired, double sal) { IFullTimeEmployee result = newFullTimeEmployee();if (debug) logger.debug("newFullTimeEmployee returned" + result);+ logger.warn("newFullTimeEmployee returned. born" + born); result.setPersonid(personid); result.setFirstname(first); result.setLastname(last); @@ -178,6 +180,7 @@public IPartTimeEmployee newPartTimeEmployee(long personid, String first, String last, String middle, java.util.Date born, java.util.Date hired, doublewage) { IPartTimeEmployee result = newPartTimeEmployee();if (debug) logger.debug("newPartTimeEmployee returned" + result);+ logger.warn("newPartTimeEmployee returned. born" + born); result.setPersonid(personid); result.setFirstname(first); result.setLastname(last); @@ -191,6 +194,7 @@public IPartTimeEmployee newPartTimeEmployee(long personid, String first, String last, String middle, java.util.Date born, IAddress addr, java.util.Datehired, double wage) { IPartTimeEmployee result = newPartTimeEmployee();if (debug) logger.debug("newPartTimeEmployee returned" + result);+ logger.warn("newPartTimeEmployee returned. born" + born); result.setPersonid(personid); result.setFirstname(first); result.setLastname(last);Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Project.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Project.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Project.java (workingcopy) @@ -233,7 +233,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Project)o1).compareTo(o2); + IProject other = (IProject)o2; + long otherId = other.getProjid(); + return (((IProject)o1).getProjid() < otherId ? -1 : (((IProject)o1).getProjid() == otherId ? 0 : 1)); } /**Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Person.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Person.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Person.java (workingcopy) @@ -293,7 +293,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Person)o1).compareTo(o2); + IPerson other = (IPerson)o2; + long otherId = other.getPersonid(); + return (((IPerson)o1).getPersonid() < otherId ? -1 : (((IPerson)o1).getPersonid() == otherId ? 0 : 1)); } /**Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Company.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Company.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Company.java (workingcopy) @@ -233,7 +233,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Company)o1).compareTo(o2); + ICompany other = (ICompany)o2; + long otherId = other.getCompanyid(); + return (((ICompany)o1).getCompanyid() < otherId ? -1 : (((ICompany)o1).getCompanyid() == otherId ? 0 : 1)); } /** Index:D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ FullTimeEmployee.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ FullTimeEmployee.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ FullTimeEmployee.java (workingcopy) @@ -117,7 +117,7 @@ */ public boolean deepCompareFields(Object other, EqualityHelper helper) { - FullTimeEmployee otherEmp = (FullTimeEmployee)other; + IFullTimeEmployee otherEmp = (IFullTimeEmployee)other; String where = "FullTimeEmployee<" + getPersonid() + ">"; return super.deepCompareFields(otherEmp, helper) & helper.closeEnough(salary, otherEmp.getSalary(), where + ".salary");Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Address.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Address.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Address.java (workingcopy) @@ -209,7 +209,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Address)o1).compareTo(o2); + IAddress other = (IAddress)o2; + long otherId = other.getAddrid(); + return (((IAddress)o1).getAddrid() < otherId ? -1 : (((IAddress)o1).getAddrid() == otherId ? 0 : 1)); } /**Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Insurance.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Insurance.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ Insurance.java (workingcopy) @@ -153,7 +153,9 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Insurance)o1).compareTo(o2); + IInsurance other = (IInsurance)o2; + long otherId = other.getInsid(); + return (((IInsurance)o1).getInsid() < otherId ? -1 : (((IInsurance)o1).getInsid() == otherId ? 0 : 1)); } /**Index: D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/util/ EqualityHelper.java=================================================================== ---D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/util/ EqualityHelper.java (revision391636) +++D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/util/ EqualityHelper.java (workingcopy) @@ -375,10 +375,12 @@ logUnequal(me, other, where + msgOtherNull); return false; }+ /* no need to check since the deepCompareFields with cast to theexpected type if (!me.getClass().isAssignableFrom(other.getClass())) { logUnequal(me, other, where + msgIncompatibleTypes); return false; } + */ if (isProcessed(me)) return true; markProcessed(me); Quoting Craig L Russell <[EMAIL PROTECTED]>:Hi Michael, I haven't started on this. If possible, could you file a JIRA and attach your patch to it. Without Erik's changes, I can't test it either. Erik, Is there a way you can make the JPOX jar file available? Thanks, Craig On Apr 6, 2006, at 7:34 AM, Michael Bouschen wrote:Hi Craig, I think this issue appears at two different places: (1) Method compare(Object o1, Object o2) in all company classes delegates to the compareTo method and casts the first argument to the class instead of the interface: public int compare(Object o1, Object o2) { return ((Insurance)o1).compareTo(o2); } This requires adding the compareTo methods to the interfaces. (2) Method deepCompareFields in Department and FullTimeEmployee cast the argument to the class instead of the interface: public boolean deepCompareFields(Object other, EqualityHelper helper) { Department otherDept = (Department)other; I understood you already started to fix this, correct? Anyway, you find my changed attached. They compile, but I could not test it. Regards MichaelHi Erik, Great news. If you are this far, then you have succeeded in storing and retrieving instances to the database. I'll have a fix for you shortly. Craig On Apr 6, 2006, at 4:06 AM, Erik Bengtson wrote:Craig, The "null" issue is over. It was a reachability issue where JPOX nullify all fields before deleting from the database. Now there is another issue in the CompletenessTest.test method where it falls the expected object tree back to the concrete classes. It compares a concrete object tree to the tree of objects generated by JPOX. The code has a cast to Department and invoke to compareTo, and this code will not work for interfaces. [java] 1) test(org.apache.jdo.tck.mapping.CompletenessTest) java.lang.ClassCa stException: org.apache.jdo.tck.pc.company.IDepartmentImpl [java] at org.apache.jdo.tck.pc.company.Department.compare (Department.j ava:297) [java] at java.util.Arrays.mergeSort(Arrays.java:1284) [java] at java.util.Arrays.sort(Arrays.java:1223)[java] at java.util.Collections.sort(Collections.java: 159)[java] at org.apache.jdo.tck.util.EqualityHelper.deepEquals (EqualityHel per.java:477) [java] at org.apache.jdo.tck.pc.company.Company.deepCompareFields(Compa ny.java:224) [java] at org.apache.jdo.tck.mapping.CompletenessTest.test (Completeness Test.java:108)Craig Russell Architect, Sun Java Enterprise System http://java.sun.com/products/ jdo 408 276-5638 mailto:[EMAIL PROTECTED] P.S. A good JDO? O, Gasp!-- Michael Bouschen [EMAIL PROTECTED] Engineering GmbH mailto:[EMAIL PROTECTED] http://www.tech.spree.de/ Tel.:++49/30/235 520-33 Buelowstr. 66 Fax.:++49/30/2175 2012 D-10783 Berlin Index: src/java/org/apache/jdo/tck/pc/company/IInsurance.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/IInsurance.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/IInsurance.java (working copy) @@ -30,4 +30,6 @@ void setInsid(long insid); void setCarrier(String carrier); void setEmployee(IEmployee employee); + + public int compareTo(Object o); } Index: src/java/org/apache/jdo/tck/pc/company/Department.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Department.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/Department.java (working copy) @@ -246,7 +246,7 @@ */ public boolean deepCompareFields(Object other, EqualityHelper helper) { - Department otherDept = (Department)other; + IDepartment otherDept = (IDepartment)other; String where = "Department<" + deptid + ">"; return helper.equals(deptid, otherDept.getDeptid(), where + ".deptid") & @@ -294,7 +294,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Department)o1).compareTo(o2); + return ((IDepartment)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/Company.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Company.java (revision 391925)+++ src/java/org/apache/jdo/tck/pc/company/Company.java (working copy)@@ -233,7 +233,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Company)o1).compareTo(o2); + return ((ICompany)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/Person.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Person.java (revision 391925)+++ src/java/org/apache/jdo/tck/pc/company/Person.java (working copy)@@ -293,7 +293,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Person)o1).compareTo(o2); + return ((IPerson)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/Project.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Project.java (revision 391925)+++ src/java/org/apache/jdo/tck/pc/company/Project.java (working copy)@@ -233,7 +233,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Project)o1).compareTo(o2); + return ((IProject)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/IDepartment.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/IDepartment.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/IDepartment.java (working copy) @@ -39,4 +39,5 @@ void setEmployees(Set employees); void setFundedEmps(Set employees); + public int compareTo(Object o); } Index: src/java/org/apache/jdo/tck/pc/company/ICompany.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/ICompany.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/ICompany.java (working copy) @@ -37,4 +37,6 @@ void setDepartments(Set depts); void setFounded(Date date); void setName(String string); + + public int compareTo(Object o); } Index: src/java/org/apache/jdo/tck/pc/company/IPerson.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/IPerson.java (revision 391925)+++ src/java/org/apache/jdo/tck/pc/company/IPerson.java (working copy)@@ -42,4 +42,5 @@ void setBirthdate(Date birthdate); void setPhoneNumbers(Map phoneNumbers); + public int compareTo(Object o); } Index: src/java/org/apache/jdo/tck/pc/company/IProject.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/IProject.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/IProject.java (working copy) @@ -36,5 +36,6 @@ void setBudget(BigDecimal budget); void setReviewers(Set reviewers); void setMembers(Set employees); - + + public int compareTo(Object o); } Index: src/java/org/apache/jdo/tck/pc/company/FullTimeEmployee.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/FullTimeEmployee.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/FullTimeEmployee.java (working copy) @@ -117,7 +117,7 @@ */ public boolean deepCompareFields(Object other, EqualityHelper helper) { - FullTimeEmployee otherEmp = (FullTimeEmployee)other; + IFullTimeEmployee otherEmp = (IFullTimeEmployee)other; String where = "FullTimeEmployee<" + getPersonid() + ">"; return super.deepCompareFields(otherEmp, helper) & helper.closeEnough(salary, otherEmp.getSalary(), where + ".salary"); Index: src/java/org/apache/jdo/tck/pc/company/Address.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Address.java (revision 391925)+++ src/java/org/apache/jdo/tck/pc/company/Address.java (working copy)@@ -209,7 +209,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Address)o1).compareTo(o2); + return ((IAddress)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/Insurance.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/Insurance.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/Insurance.java (working copy) @@ -153,7 +153,7 @@ * Compare two instances. This is a method in Comparator. */ public int compare(Object o1, Object o2) { - return ((Insurance)o1).compareTo(o2); + return ((IInsurance)o1).compareTo(o2); } /** Index: src/java/org/apache/jdo/tck/pc/company/IAddress.java =================================================================== --- src/java/org/apache/jdo/tck/pc/company/IAddress.java (revision 391925) +++ src/java/org/apache/jdo/tck/pc/company/IAddress.java (working copy) @@ -36,4 +36,6 @@ void setState(String state); void setZipcode(String zipcode); void setCountry(String country); + + public int compareTo(Object o); }Craig RussellArchitect, Sun Java Enterprise System http://java.sun.com/products/ jdo408 276-5638 mailto:[EMAIL PROTECTED] P.S. A good JDO? O, Gasp!
Craig Russell Architect, Sun Java Enterprise System http://java.sun.com/products/jdo 408 276-5638 mailto:[EMAIL PROTECTED] P.S. A good JDO? O, Gasp!
smime.p7s
Description: S/MIME cryptographic signature
