db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig Russell <Craig.Russ...@Sun.COM>
Subject Re: Patch for JDO-58
Date Wed, 01 Jun 2005 16:39:49 GMT
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!

Mime
View raw message