Here is a patch that addresses a few issues, but they are all related. 
This fixes the FIXME problem in the Object.vm file that caused scarab to
recursively loop.  The problem was fixed by adding boolean flags to
objects that are associated by a foreign key.  With this patch, only
setting the object will flag it to be saved, and not getting it.
Example:
class 'A' contains a foreign key reference to class 'B' such that A
contains the methods setB and getB.  Calling setB with an object of type
B will flag Torque to save the object the next time A is saved, but this
will not be true for an Object of type B aquired from A.getB. 

The error occurred before because getB would also save the object
aquirred when A was saved.  When this operation was performed within a
save, infinite looping followed.

I think this makes intuitive since based on the fact that when calling a
set method a user is indicating a change in the object and a
relationship between the objects.  Certainly in practice we have found
this functionality very useful. 
  
In this patch the alreadyInSave flag has been replaced with a
transaction key.  A transaction key uniquely identifies a transaction. 
The use of the transaction key serves the purpose of alreadyInSave, but
it will also keep the save method from being called more than once
during a given transaction. This becomes an issue with the new set
method feature.

Also in this patch is the initSave method which serves the purpose of
the preSave.  It has some nice properties over overidding the save
method. First, it is guaranteed to be called once per object per save
transaction. Also, it is safe to call other save(DBConnection)  methods
without the risk of infinite looping. 

This patch contain the new class org.apache.torque.pool.TransactionKey

This patch has been tested with scarab.

Log Message:

Fixed infinite looping problem caused by saving associated om objects
linked by foreign key to an om object that is being saved.  Now, only
associated objects linked by passing them to the set method of the root
object are saved.  TransactionKey added to prevent the save operation
from occurring more than one time on an OM object for a given
transaction. 

Thanks Much,
Byron





? transkey.patch.txt
? temp.txt
Index: src/java/org/apache/torque/pool/DBConnection.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-turbine-torque/src/java/org/apache/torque/pool/DBConnection.java,v
retrieving revision 1.5
diff -u -r1.5 DBConnection.java
--- src/java/org/apache/torque/pool/DBConnection.java   2001/11/08 23:52:47     1.5
+++ src/java/org/apache/torque/pool/DBConnection.java   2001/12/04 08:32:23
@@ -101,9 +101,15 @@
      * is null, then this class uses the classic Connection object to manage
      * connection. Else, the PooledConnection object is used.
      */
-     private PooledConnection pooledConnection = null;
+    private PooledConnection pooledConnection = null;
 
     /**
+     * Key that uniquely defines a transaction that this connection is
+     * currently executing.
+     */
+    private TransactionKey transactionKey = null;
+  
+    /**
      * The URL of this connection.
      */
     private String url = null;
@@ -430,6 +436,11 @@
             e.printStackTrace();
             category.error(e);
         }
+        finally
+        {
+            transactionKey = null;
+        }
+        
     }
 
     /**
@@ -446,6 +457,10 @@
             e.printStackTrace();
             category.error(e);
         }
+        finally
+        {
+            transactionKey = null;
+        }
     }
 
     /**
@@ -458,6 +473,8 @@
         try
         {
             connection.setAutoCommit(b);
+            // Create a new transaction key to identify this transaction.
+            transactionKey = new TransactionKey();
         }
         catch(SQLException e)
         {
@@ -595,6 +612,18 @@
         {
             //just ignore
         }
+    }
+
+    /**
+     * Get the TransactionKey that uniquely defined the transaction
+     * that this connection is currenty executing.
+     * @return The TransactionKey object associated with the current
+     * transaction, or null if this connection is not currently executing
+     * a transaction.
+     */
+    public TransactionKey getTransactionKey()
+    {
+        return transactionKey;
     }
 
     /*
Index: src/java/org/apache/torque/pool/TransactionKey.java
===================================================================
RCS file: TransactionKey.java
diff -N TransactionKey.java
--- /dev/null   Tue Dec  4 00:31:04 2001
+++ src/java/org/apache/torque/pool/TransactionKey.java Tue Dec  4 00:32:23 2001
@@ -0,0 +1,127 @@
+package org.apache.torque.pool;
+
+/* ====================================================================
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2001 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. The end-user documentation included with the redistribution,
+ *    if any, must include the following acknowledgment:
+ *       "This product includes software developed by the
+ *        Apache Software Foundation (http://www.apache.org/)."
+ *    Alternately, this acknowledgment may appear in the software itself,
+ *    if and wherever such third-party acknowledgments normally appear.
+ *
+ * 4. The names "Apache" and "Apache Software Foundation" and
+ *    "Apache Turbine" must not be used to endorse or promote products
+ *    derived from this software without prior written permission. For
+ *    written permission, please contact [EMAIL PROTECTED]
+ *
+ * 5. Products derived from this software may not be called "Apache",
+ *    "Apache Turbine", nor may "Apache" appear in their name, without
+ *    prior written permission of the Apache Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+
+/**
+ * The TransactionKey is used to uniquely Identify a Torque
+ * transaction. Each TransactionKey object instantiated will not equal
+ * any other TransactionKey object.  Each time Torque begins a new
+ * transaction it cretes an instance of this class and
+ * associates it to the transaction through <code>DBConnection</code>.  
+ * <br><br>
+ * Currently we implement the id through an int value.  This doesn't
+ * guarantee that every TransactionKey created will be unique since
+ * the int value can rollover. However, for our purposes it's close
+ * enough.  Another implementation would be to simply do a "this ==
+ * transKey" comparison of this object to transKey in the equals
+ * method.  Undecided which is best.
+ */
+public class TransactionKey
+{
+    /**
+     * keycounter keeps track of the next value that will be used to
+     * give the next transaction a unique key.
+     */
+    private static int keycounter = 0;
+
+    /**
+     * The unique key value of this object.
+     */
+    private int key;
+
+    /**
+     * Constructs a new TransactionKey object that provides a unique
+     * key value.  Each TransactionKey object created will produce a
+     * key value unique from any other TransactionKey instantiated.
+     * This constructor is not thread safe so it should be wrapped in
+     * a synchronized block.
+     */
+    public TransactionKey()
+    {
+        keycounter++;
+        key = keycounter;
+    }
+
+    /**
+     * Key properties provides the unique value of this
+     * TransactionKey.  This method has package scope since we really
+     * only want the equals method to have access to it.  We don't
+     * want to expose the object or type used to store the value of
+     * the key.
+     * @return the unique value.
+     */
+    int getKey()
+    {
+        return key;
+    }
+
+    /**
+     * Compares this transaction object to transKey.
+     * @param transKey Transaction object to compare to.
+     * @return true if this TransactionKey is associated with the same
+     * transaction as the transKey object. Returns false otherwise.
+     */
+    public boolean equals(TransactionKey transKey)
+    {
+        if (transKey == null)
+        {
+            return false;
+        }
+        
+        return key == transKey.getKey();
+    }
+    
+  
+}
Index: src/templates/om/Object.vm
===================================================================
RCS file: /home/cvspublic/jakarta-turbine-torque/src/templates/om/Object.vm,v
retrieving revision 1.16
diff -u -r1.16 Object.vm
--- src/templates/om/Object.vm  2001/11/13 01:17:37     1.16
+++ src/templates/om/Object.vm  2001/12/04 08:32:24
@@ -19,6 +19,7 @@
 import org.apache.torque.util.BasePeer;
 import org.apache.torque.util.Criteria;
 import org.apache.torque.pool.DBConnection;
+import org.apache.torque.pool.TransactionKey;
 #if ($addSaveMethod)
 import org.apache.commons.util.ObjectUtils;
 #end
@@ -128,19 +129,19 @@
                 v = null;
             }
           #end 
-
         #if ($complexObjectModel)
           #if ($col.isForeignKey())
             #set ( $tblFK = $table.Database.getTable($col.RelatedTableName) )
             #set ( $colFK = $tblFK.getColumn($col.RelatedColumnName) )
             #if ($col.isMultipleFK() || $col.RelatedTableName.equals($table.Name))
-              #set ( $varName = "a${tblFK.JavaName}RelatedBy$col.JavaName" )
+              #set ( $varName = "${tblFK.JavaName}RelatedBy$col.JavaName" )
             #else
-              #set ( $varName = "a$tblFK.JavaName" )
+              #set ( $varName = "$tblFK.JavaName" )
             #end
-            $varName = null;
-          #end
 
+            a$varName = null;
+            save$varName = false;
+          #end
           #foreach ($fk in $col.Referrers)
             #set ( $fkColName = $fk.ForeignLocalMapping.get($col.Name) )
             #set ( $tblFK = $fk.Table )
@@ -164,25 +165,25 @@
             #end
           #end 
         #end
-
 #if ($addSaveMethod)
-
  #if ( ($cjtype == "int") || ($cjtype == "long") || ($cjtype == "boolean") 
     || ($cjtype == "short") || ($cjtype == "float") || ($cjtype == "double")
     || ($cjtype == "char") || ($cjtype == "byte") )
-        if (this.$clo != v)
-        {
+
+            if (this.$clo != v)
+            {
  #else
-        if ( !ObjectUtils.equals(this.$clo, v) )
-        {
+
+            if ( !ObjectUtils.equals(this.$clo, v) )
+            {
  #end
-            this.$clo = v;
-            setModified(true);
-        }
+               this.$clo = v;
+                setModified(true);
+            }
 #else
-          this.$clo = v;
+            this.$clo = v;
 #end
-     }
+        }
 
 ##if ($complexObjectModel)
   #if ($col.isPrimaryKey() || $col.isForeignKey())
@@ -204,6 +205,7 @@
 #if ($complexObjectModel)
  #set($pVars = [])  ## Array of object set method names for later reference.
  #set($aVars = [])  ## Array of object field names for later reference.
+ #set($saveVars = [])  ## field save name, determines if we should save field.
  #foreach ($fk in $table.ForeignKeys)
 
    #set ( $tblFK = $table.Database.getTable($fk.ForeignTableName) )
@@ -223,10 +225,15 @@
 
    #set ( $pVarName = "$className$relCol" )
    #set ( $varName = "a$pVarName" )
+   #set ( $saveName = "save$pVarName" )
    #set ( $retVal = $pVars.add($pVarName) )
    #set ( $retVal = $aVars.add($varName) )
+   #set ( $retVal = $saveVars.add($saveName) )
     private $className $varName;
 
+    // Used to determine if we should save $varName when this object is saved.
+    private boolean $saveName = false;
+
     /**
      * Declares an association between this object and a $className object
      *
@@ -238,9 +245,21 @@
     #set ( $column = $table.getColumn($columnName) )
     #set ( $colFKName = $fk.LocalForeignMapping.get($columnName) )
     #set ( $colFK = $tblFK.getColumn($colFKName) )
-        set${column.JavaName}(v.get${colFK.JavaName}());
+    #set ( $cjtype = $column.javaNative)
+
+        if (v == null)
+        {
+          set${column.JavaName}(($cjtype)null);
+        }
+        else
+        {
+          set${column.JavaName}(v.get${colFK.JavaName}());
+        }
    #end
+
         $varName = v;
+        // If the user explicitly sets $varName then save it.
+        $saveName = v != null;
     }
 
    #set ( $and = "" )
@@ -834,13 +853,31 @@
       
  #end
     }
-
   #if ($complexObjectModel)
-    /** flag to prevent endless save loop, if this object is referenced
-        by another object which falls in this transaction. */
-    private boolean alreadyInSave = false;
-  #end
+
     /**
+     * Uniquely identifies a transaction this object is involved
+     * with, We use this variable to prevent infinite looping and also
+     * prevent the save method being called for this object multiple
+     * times. transKey is null when this object is not involved in a
+     * transaction.
+     */
+    private TransactionKey transKey = null;
+
+    /**
+     * initSave is called once before this object, or an associated
+     * object, is saved to the database.  This method is intended to
+     * be overridden by a derived class to do any operations necessary
+     * before this object is saved.
+     * @param dbCon Connection object that is used for this save
+     * operation, and encapsulates the current transaction.
+     */
+    public void initSave(DBConnection dbCon) throws Exception
+    {
+    }
+   #end
+
+    /**
      * Stores the object in the database.  If the object is new,
      * it inserts it; otherwise an update is performed.  This method
      * is meant to be used as part of a transaction, otherwise use
@@ -849,35 +886,46 @@
      */
     public void save(DBConnection dbCon) throws Exception
     {
-  #if ($complexObjectModel)
-      if (!alreadyInSave)
-      {
-        alreadyInSave = true;
+    #if ($complexObjectModel)
 
-#* FIXME! the following code can cause an infinite loop, needs more thought
-shows the infinite loop: System.out.println("Entering save for " + this);
-       #if ($pVars.size() != 0)
-
-        // We call the save method on the following object(s) if they
-        // were passed to this object by their coresponding set
-        // method.  This object relates to these object(s) by a
-        // foreign key reference.  If the object(s) being saved were
-        // new to the database, an insert was performed, then they may
-        // have a new PrimaryKey.  We call the coresponding set method
-        // for the given object(s) to set this object's Id reference
-        // to this new Primary key so that it will be saved.
-      #foreach ($aVarName in $aVars)
-        #set($i = $velocityCount - 1)
-        if ($aVarName != null)
-        {   
-            ${aVarName}.save(dbCon);
-            set$pVars.get($i)($aVarName);
-        }   
-      #end
-    #end
-*#
+        if (!ObjectUtils.equals(transKey, dbCon.getTransactionKey()))
+        {
+            // The transaction associated with the connection passed
+            // to this save method is not the same as the last
+            // transaction stored in transKey.  This tells us that we
+            // are not recursing back into this method, and that we
+            // are not trying to save this object more than once
+            // during the same transaction.  Save this transaction so
+            // it may be compared to subsequent calls.
+            transKey = dbCon.getTransactionKey();
+
+            // Perform any user defined operations before we save.
+            initSave(dbCon);
+            #if ($pVars.size() != 0)
+
+            // We call the save method on the following object(s) if they
+            // were passed to this object by their coresponding set
+            // method.  This object relates to these object(s) by a
+            // foreign key reference.  If the object(s) being saved were
+            // new to the database, an insert was performed, then they may
+            // have a new PrimaryKey.  We call the coresponding set method
+            // for the given object(s) to set this object's Id reference
+            // to this new Primary key so that it will be saved.
+            #foreach ($aVarName in $aVars)
+            #set($i = $velocityCount - 1)
+
+            if ($saveVars.get($i))
+            {   
+                ${aVarName}.save(dbCon);
+                set$pVars.get($i)($aVarName);
+            }   
+            #end
+            #end
+    #else
 
-  #end
+        // Perform any user defined operations before we save.
+        initSave(dbCon);
+    #end
 
         // If this object has been modified, then save it to the database.
         if (isModified())
@@ -924,7 +972,7 @@
   #end      
  #end
   #if ($complexObjectModel)
-        alreadyInSave = false;
+
       }
   #end      
     }

--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to