You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jdo-dev@db.apache.org by Michael Watzek <mw...@spree.de> on 2005/06/01 15:33:29 UTC

Patch for JDO-58

Hi,

please find the patch for JDO-58 attached. It includes the two changes
discussed in the mail thread JDO-58:

1) if (pmf!=null && pmf.isClosed())
     pmf = null;

2) check for any instances or classes to be removed before we get a PMF
in tearDown.

Additionally, the patch includes some minor changes:

1) A final modifier for method "tearDown".

2) Methods "deleteAndUnregisterPCInstances" and
"deleteAndUnregisterPCClasses" have been renamed to
"deleteRemoveTearDownInstances" and "deleteRemoveTearDownClasses".

3) Method closePMF calls "pmf.isClosed()" before closing the PMF.

I ran the TCK with this patch. This result is:

Tests run: 349,  Failures: 14,  Errors: 81

Please note that this TCK test run includes the patch for JDO-1. With 
patch for JDO-1 the two enhancer tests fail because they are 
(unintentionally) still in the list of test cases to be executed. Thus, 
if we exclude thw two from the list, then we have 12 Failues.

Regards,
Michael
-- 
-------------------------------------------------------------------
Michael Watzek                  Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de        Buelowstr. 66
Tel.:  ++49/30/235 520 36       10783 Berlin - Germany
Fax.:  ++49/30/217 520 12       http://www.spree.de/
-------------------------------------------------------------------


Re: Patch for JDO-58

Posted by Michael Watzek <mw...@spree.de>.
Hi Craig,

> Hi Michael,
> 
> Just one comment. I'd change the name of
> 
> deleteRemoveTearDownInstances to deleteTearDownInstances and
> deleteRemoveTearDownClasses to deleteTearDownClasses.
Ok.

Michael
> 
> Craig
> 
> On Jun 1, 2005, at 6:33 AM, Michael Watzek wrote:
> 
>> Hi,
>>
>> please find the patch for JDO-58 attached. It includes the two changes
>> discussed in the mail thread JDO-58:
>>
>> 1) if (pmf!=null && pmf.isClosed())
>>     pmf = null;
>>
>> 2) check for any instances or classes to be removed before we get a PMF
>> in tearDown.
>>
>> Additionally, the patch includes some minor changes:
>>
>> 1) A final modifier for method "tearDown".
>>
>> 2) Methods "deleteAndUnregisterPCInstances" and
>> "deleteAndUnregisterPCClasses" have been renamed to
>> "deleteRemoveTearDownInstances" and "deleteRemoveTearDownClasses".
>>
>> 3) Method closePMF calls "pmf.isClosed()" before closing the PMF.
>>
>> I ran the TCK with this patch. This result is:
>>
>> Tests run: 349,  Failures: 14,  Errors: 81
>>
>> Please note that this TCK test run includes the patch for JDO-1. With 
>> patch for JDO-1 the two enhancer tests fail because they are 
>> (unintentionally) still in the list of test cases to be executed. 
>> Thus, if we exclude thw two from the list, then we have 12 Failues.
>>
>> Regards,
>> Michael
>> -- 
>> -------------------------------------------------------------------
>> Michael Watzek                  Tech@Spree Engineering GmbH
>> mailto:mwa.tech@spree.de        Buelowstr. 66
>> Tel.:  ++49/30/235 520 36       10783 Berlin - Germany
>> Fax.:  ++49/30/217 520 12       http://www.spree.de/
>> -------------------------------------------------------------------
>>
>> Index: test/java/org/apache/jdo/tck/JDO_Test.java
>> ===================================================================
>> --- test/java/org/apache/jdo/tck/JDO_Test.java    (revision 179370)
>> +++ test/java/org/apache/jdo/tck/JDO_Test.java    (working copy)
>> @@ -225,7 +225,7 @@
>>       * Otherwise that exception is logged using fatal log level.
>>       * All other exceptions are logged using fatal log level, always.
>>       */
>> -    protected void tearDown() {
>> +    protected final void tearDown() {
>>          try {
>>              cleanupPM();
>>          }
>> @@ -233,6 +233,9 @@
>>              setTearDownThrowable("cleanupPM", t);
>>          }
>>
>> +        if (pmf != null && pmf.isClosed())
>> +            pmf = null;
>> +
>>          try {
>>              localTearDown();
>>          }
>> @@ -263,8 +266,8 @@
>>       * that they have allocated in method <code>localSetUp</code>.
>>       */
>>      protected void localTearDown() {
>> -        deleteAndUnregisterPCInstances();
>> -        deleteAndUnregisterPCClasses();
>> +        deleteRemoveTearDownInstances();
>> +        deleteRemoveTearDownClasses();
>>      }
>>
>>      protected void addTearDownObjectId(Object oid) {
>> @@ -292,50 +295,65 @@
>>      }
>>
>>      /**
>> -     * Deletes and unregistres all registered pc instances.
>> +     * Deletes and removes tear down instances.
>> +     * If there are no tear down instances,
>> +     * the this method is a noop.
>> +     * Otherwise, tear down instances are deleted
>> +     * exactly in the order they have been added.
>> +     * Tear down instances are deleted in a separate transaction.
>>       */
>> -    protected void deleteAndUnregisterPCInstances() {
>> -        getPM();
>> -        try {
>> -            this.pm.currentTransaction().begin();
>> -            for (Iterator i = this.oids.iterator(); i.hasNext(); ) {
>> -                Object pc;
>> -                try {
>> -                    pc = this.pm.getObjectById(i.next(), true);
>> +    protected void deleteRemoveTearDownInstances() {
>> +        if (this.oids.size() > 0) {
>> +            getPM();
>> +            try {
>> +                this.pm.currentTransaction().begin();
>> +                for (Iterator i = this.oids.iterator(); i.hasNext(); ) {
>> +                    Object pc;
>> +                    try {
>> +                        pc = this.pm.getObjectById(i.next(), true);
>> +                    }
>> +                    catch (JDOObjectNotFoundException e) {
>> +                        pc = null;
>> +                    }
>> +                    // we only delete those persistent instances
>> +                    // which have not been deleted by tests already.
>> +                    if (pc != null) {
>> +                        this.pm.deletePersistent(pc);
>> +                    }
>>                  }
>> -                catch (JDOObjectNotFoundException e) {
>> -                    pc = null;
>> -                }
>> -                // we only delete those persistent instances
>> -                // which have not been deleted by tests already.
>> -                if (pc != null) {
>> -                    this.pm.deletePersistent(pc);
>> -                }
>> +                this.pm.currentTransaction().commit();
>>              }
>> -            this.pm.currentTransaction().commit();
>> +            finally {
>> +                this.oids.clear();
>> +                cleanupPM();
>> +            }
>>          }
>> -        finally {
>> -            this.oids.clear();
>> -            cleanupPM();
>> -        }
>>      }
>>
>>      /**
>> -     * Deletes extents of all registered pc instances and unregisters 
>> all pc classes.
>> +     * Deletes and removes tear down classes.
>> +     * If there are no tear down classes,
>> +     * the this method is a noop.
>> +     * Otherwise, tear down classes are deleted
>> +     * exactly in the order they have been added.
>> +     * Tear down classes are deleted in a separate transaction.
>> +     * Deleting a tear down class means to delete the extent.
>>       */
>> -    protected void deleteAndUnregisterPCClasses() {
>> -        getPM();
>> -        try {
>> -            this.pm.currentTransaction().begin();
>> -            for (Iterator i = this.pcClasses.iterator(); i.hasNext(); 
>> ) {
>> -                this.pm.deletePersistentAll(getAllObjects(this.pm, 
>> (Class)i.next()));
>> +    protected void deleteRemoveTearDownClasses() {
>> +        if (this.pcClasses.size() > 0) {
>> +            getPM();
>> +            try {
>> +                this.pm.currentTransaction().begin();
>> +                for (Iterator i = this.pcClasses.iterator(); 
>> i.hasNext(); ) {
>> +                    
>> this.pm.deletePersistentAll(getAllObjects(this.pm, (Class)i.next()));
>> +                }
>> +                this.pm.currentTransaction().commit();
>>              }
>> -            this.pm.currentTransaction().commit();
>> +            finally {
>> +                this.pcClasses.clear();
>> +                cleanupPM();
>> +            }
>>          }
>> -        finally {
>> -            this.pcClasses.clear();
>> -            cleanupPM();
>> -        }
>>      }
>>
>>      /** */
>> @@ -351,6 +369,8 @@
>>      /**
>>       * Get the <code>PersistenceManagerFactory</code> instance
>>       * for the implementation under test.
>> +     * @return field <code>pmf</code> if it is not <code>null</code>,
>> +     * else sets field <code>pmf</code> to a new instance and returns 
>> that instance.
>>       */
>>      protected PersistenceManagerFactory getPMF()
>>      {
>> @@ -401,12 +421,12 @@
>>      }
>>
>>      /** Closes the pmf stored in this instance. */
>> -    protected void closePMF()
>> -    {
>> +    protected void closePMF() {
>>          JDOException failure = null;
>>          while (pmf != null) {
>>              try {
>> -                pmf.close();
>> +                if (!pmf.isClosed())
>> +                    pmf.close();
>>                  pmf = null;
>>              }
>>              catch (JDOException ex) {
>>
> 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 Watzek                  Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de        Buelowstr. 66
Tel.:  ++49/30/235 520 36       10783 Berlin - Germany
Fax.:  ++49/30/217 520 12       http://www.spree.de/
-------------------------------------------------------------------

Re: Patch for JDO-58

Posted by Craig Russell <Cr...@Sun.COM>.
Hi Michael,

Just one comment. I'd change the name of

deleteRemoveTearDownInstances to deleteTearDownInstances and
deleteRemoveTearDownClasses to deleteTearDownClasses.

Craig

On Jun 1, 2005, at 6:33 AM, Michael Watzek wrote:

> Hi,
>
> please find the patch for JDO-58 attached. It includes the two changes
> discussed in the mail thread JDO-58:
>
> 1) if (pmf!=null && pmf.isClosed())
>     pmf = null;
>
> 2) check for any instances or classes to be removed before we get a PMF
> in tearDown.
>
> Additionally, the patch includes some minor changes:
>
> 1) A final modifier for method "tearDown".
>
> 2) Methods "deleteAndUnregisterPCInstances" and
> "deleteAndUnregisterPCClasses" have been renamed to
> "deleteRemoveTearDownInstances" and "deleteRemoveTearDownClasses".
>
> 3) Method closePMF calls "pmf.isClosed()" before closing the PMF.
>
> I ran the TCK with this patch. This result is:
>
> Tests run: 349,  Failures: 14,  Errors: 81
>
> Please note that this TCK test run includes the patch for JDO-1. With 
> patch for JDO-1 the two enhancer tests fail because they are 
> (unintentionally) still in the list of test cases to be executed. 
> Thus, if we exclude thw two from the list, then we have 12 Failues.
>
> Regards,
> Michael
> -- 
> -------------------------------------------------------------------
> Michael Watzek                  Tech@Spree Engineering GmbH
> mailto:mwa.tech@spree.de        Buelowstr. 66
> Tel.:  ++49/30/235 520 36       10783 Berlin - Germany
> Fax.:  ++49/30/217 520 12       http://www.spree.de/
> -------------------------------------------------------------------
>
> Index: test/java/org/apache/jdo/tck/JDO_Test.java
> ===================================================================
> --- test/java/org/apache/jdo/tck/JDO_Test.java	(revision 179370)
> +++ test/java/org/apache/jdo/tck/JDO_Test.java	(working copy)
> @@ -225,7 +225,7 @@
>       * Otherwise that exception is logged using fatal log level.
>       * All other exceptions are logged using fatal log level, always.
>       */
> -    protected void tearDown() {
> +    protected final void tearDown() {
>          try {
>              cleanupPM();
>          }
> @@ -233,6 +233,9 @@
>              setTearDownThrowable("cleanupPM", t);
>          }
>
> +        if (pmf != null && pmf.isClosed())
> +            pmf = null;
> +
>          try {
>              localTearDown();
>          }
> @@ -263,8 +266,8 @@
>       * that they have allocated in method <code>localSetUp</code>.
>       */
>      protected void localTearDown() {
> -        deleteAndUnregisterPCInstances();
> -        deleteAndUnregisterPCClasses();
> +        deleteRemoveTearDownInstances();
> +        deleteRemoveTearDownClasses();
>      }
>
>      protected void addTearDownObjectId(Object oid) {
> @@ -292,50 +295,65 @@
>      }
>
>      /**
> -     * Deletes and unregistres all registered pc instances.
> +     * Deletes and removes tear down instances.
> +     * If there are no tear down instances,
> +     * the this method is a noop.
> +     * Otherwise, tear down instances are deleted
> +     * exactly in the order they have been added.
> +     * Tear down instances are deleted in a separate transaction.
>       */
> -    protected void deleteAndUnregisterPCInstances() {
> -        getPM();
> -        try {
> -            this.pm.currentTransaction().begin();
> -            for (Iterator i = this.oids.iterator(); i.hasNext(); ) {
> -                Object pc;
> -                try {
> -                    pc = this.pm.getObjectById(i.next(), true);
> +    protected void deleteRemoveTearDownInstances() {
> +        if (this.oids.size() > 0) {
> +            getPM();
> +            try {
> +                this.pm.currentTransaction().begin();
> +                for (Iterator i = this.oids.iterator(); i.hasNext(); 
> ) {
> +                    Object pc;
> +                    try {
> +                        pc = this.pm.getObjectById(i.next(), true);
> +                    }
> +                    catch (JDOObjectNotFoundException e) {
> +                        pc = null;
> +                    }
> +                    // we only delete those persistent instances
> +                    // which have not been deleted by tests already.
> +                    if (pc != null) {
> +                        this.pm.deletePersistent(pc);
> +                    }
>                  }
> -                catch (JDOObjectNotFoundException e) {
> -                    pc = null;
> -                }
> -                // we only delete those persistent instances
> -                // which have not been deleted by tests already.
> -                if (pc != null) {
> -                    this.pm.deletePersistent(pc);
> -                }
> +                this.pm.currentTransaction().commit();
>              }
> -            this.pm.currentTransaction().commit();
> +            finally {
> +                this.oids.clear();
> +                cleanupPM();
> +            }
>          }
> -        finally {
> -            this.oids.clear();
> -            cleanupPM();
> -        }
>      }
>
>      /**
> -     * Deletes extents of all registered pc instances and unregisters 
> all pc classes.
> +     * Deletes and removes tear down classes.
> +     * If there are no tear down classes,
> +     * the this method is a noop.
> +     * Otherwise, tear down classes are deleted
> +     * exactly in the order they have been added.
> +     * Tear down classes are deleted in a separate transaction.
> +     * Deleting a tear down class means to delete the extent.
>       */
> -    protected void deleteAndUnregisterPCClasses() {
> -        getPM();
> -        try {
> -            this.pm.currentTransaction().begin();
> -            for (Iterator i = this.pcClasses.iterator(); i.hasNext(); 
> ) {
> -                this.pm.deletePersistentAll(getAllObjects(this.pm, 
> (Class)i.next()));
> +    protected void deleteRemoveTearDownClasses() {
> +        if (this.pcClasses.size() > 0) {
> +            getPM();
> +            try {
> +                this.pm.currentTransaction().begin();
> +                for (Iterator i = this.pcClasses.iterator(); 
> i.hasNext(); ) {
> +                    
> this.pm.deletePersistentAll(getAllObjects(this.pm, (Class)i.next()));
> +                }
> +                this.pm.currentTransaction().commit();
>              }
> -            this.pm.currentTransaction().commit();
> +            finally {
> +                this.pcClasses.clear();
> +                cleanupPM();
> +            }
>          }
> -        finally {
> -            this.pcClasses.clear();
> -            cleanupPM();
> -        }
>      }
>
>      /** */
> @@ -351,6 +369,8 @@
>      /**
>       * Get the <code>PersistenceManagerFactory</code> instance
>       * for the implementation under test.
> +     * @return field <code>pmf</code> if it is not <code>null</code>,
> +     * else sets field <code>pmf</code> to a new instance and returns 
> that instance.
>       */
>      protected PersistenceManagerFactory getPMF()
>      {
> @@ -401,12 +421,12 @@
>      }
>
>      /** Closes the pmf stored in this instance. */
> -    protected void closePMF()
> -    {
> +    protected void closePMF() {
>          JDOException failure = null;
>          while (pmf != null) {
>              try {
> -                pmf.close();
> +                if (!pmf.isClosed())
> +                    pmf.close();
>                  pmf = null;
>              }
>              catch (JDOException ex) {
>
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!

Re: Patch for JDO-58

Posted by Michelle Caisse <Mi...@Sun.COM>.
Thanks, Michael.  I have checked in this patch.  For the record, I get 
15 failures and 82 errors, using yesterday's build of JPOX run with 
datastore identity.

-- Michelle

Michael Watzek wrote:

> Hi,
>
> please find the patch for JDO-58 attached. It includes the two changes
> discussed in the mail thread JDO-58:
>
> 1) if (pmf!=null && pmf.isClosed())
>     pmf = null;
>
> 2) check for any instances or classes to be removed before we get a PMF
> in tearDown.
>
> Additionally, the patch includes some minor changes:
>
> 1) A final modifier for method "tearDown".
>
> 2) Methods "deleteAndUnregisterPCInstances" and
> "deleteAndUnregisterPCClasses" have been renamed to
> "deleteRemoveTearDownInstances" and "deleteRemoveTearDownClasses".
>
> 3) Method closePMF calls "pmf.isClosed()" before closing the PMF.
>
> I ran the TCK with this patch. This result is:
>
> Tests run: 349,  Failures: 14,  Errors: 81
>
> Please note that this TCK test run includes the patch for JDO-1. With 
> patch for JDO-1 the two enhancer tests fail because they are 
> (unintentionally) still in the list of test cases to be executed. 
> Thus, if we exclude thw two from the list, then we have 12 Failues.
>
> Regards,
> Michael