db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig L Russell <Craig.Russ...@Sun.COM>
Subject Re: TCK completeness interface
Date Thu, 06 Apr 2006 18:16:58 GMT
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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/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,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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> CompanyFactoryAbstractImpl.java	(working
> copy)
> @@ -132,6 +132,7 @@
>      public IFullTimeEmployee newFullTimeEmployee(long personid,  
> String first,
> String last, String middle, java.util.Date born, java.util.Date  
> hired, 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);
> @@ -145,6 +146,7 @@
>      public IFullTimeEmployee newFullTimeEmployee(long personid,  
> String first,
> String last, String middle, java.util.Date born, IAddress addr,  
> java.util.Date
> hired, 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, 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);
> @@ -191,6 +194,7 @@
>      public IPartTimeEmployee newPartTimeEmployee(long personid,  
> String first,
> String last, String middle, java.util.Date born, IAddress addr,  
> java.util.Date
> hired, 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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> Project.java	(working
> copy)
> @@ -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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> Person.java	(working
> copy)
> @@ -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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> Company.java	(working
> copy)
> @@ -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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/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: 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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> Address.java	(working
> copy)
> @@ -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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/pc/company/ 
> Insurance.java	(working
> copy)
> @@ -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	(revision
> 391636)
> +++
> D:/jdo/trunk/tck20/src/java/org/apache/jdo/tck/util/ 
> EqualityHelper.java	(working
> copy)
> @@ -375,10 +375,12 @@
>              logUnequal(me, other, where + msgOtherNull);
>              return false;
>          }
> +        /* no need to check since the deepCompareFields with cast  
> to the
> expected 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 <Craig.Russell@Sun.COM>:
>
>> 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 Michael
>>>
>>>> Hi 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:Craig.Russell@sun.com
>>>> P.S. A good JDO? O, Gasp!
>>>
>>>
>>> --
>>> Michael Bouschen		Tech@Spree Engineering GmbH
>>> mailto:mbo.tech@spree.de	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 Russell
>> Architect, Sun Java Enterprise System http://java.sun.com/products/ 
>> jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!
>>
>>
>
>
>

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Mime
View raw message