Fixes relating to multiple same-typed embedded fields loading eager relations,

and deep vertical inheritance hierarchies where the base class's primary key 
is auto-assigned on insert.



git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@453449 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java

@@ -931,7 +931,7 @@ public class JDBCStoreManager
                 && sel.eagerClone(fms[i], jtype, false, 1) != null)
                 continue;
 
-            boolean hasJoin = fetch.hasJoin(fms[i].getFullName());
+            boolean hasJoin = fetch.hasJoin(fms[i].getFullName(false));
 
             // if the field declares a preferred select mode of join or does not
             // have a preferred mode and we're doing a by-id lookup, try

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/PropertiesReverseCustomizer.java

@@ -156,7 +156,7 @@ public class PropertiesReverseCustomizer
     }
 
     public void customize(FieldMapping field) {
-        String type = getProperty(field.getFullName() + ".type");
+        String type = getProperty(field.getFullName(false) + ".type");
         if (type != null)
             field.setDeclaredType(Strings.toClass(type, null));
     }
@@ -195,7 +195,7 @@ public class PropertiesReverseCustomizer
     }
 
     public String getInitialValue(FieldMapping field) {
-        return getProperty(field.getFullName() + ".value");
+        return getProperty(field.getFullName(false) + ".value");
     }
 
     public String getDeclaration(FieldMapping field) {

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/ForeignKey.java

@@ -16,6 +16,7 @@
 package org.apache.openjpa.jdbc.schema;
 
 import java.sql.DatabaseMetaData;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -161,24 +162,59 @@ public class ForeignKey
     }
 
     /**
-     * Whether the primary key columns of this key are auto-incrementing.
+     * Whether the primary key columns of this key are auto-incrementing, or
+     * whether they themselves are members of a foreign key who's primary key
+     * is auto-incrementing (recursing to arbitrary depth).
      */
     public boolean isPrimaryKeyAutoAssigned() {
-        if (_autoAssign == null) {
+        if (_autoAssign != null)
-            Column[] cols = getPrimaryKeyColumns();
+            return _autoAssign.booleanValue();
-            if (cols.length == 0)
+        return isPrimaryKeyAutoAssigned(new ArrayList(3));
-                return false;
+    }
 
-            boolean auto = false;
+    /**
-            for (int i = 0; i < cols.length; i++) {
+     * Helper to calculate whether this foreign key depends on auto-assigned 
-                if (cols[i].isAutoAssigned()) {
+     * columns.  Recurses appropriately if the primary key columns this key
-                    auto = true;
+     * joins to are themselves members of a foreign key that is dependent on
-                    break;
+     * auto-assigned columns.  Caches calculated auto-assign value as a side 
+     * effect.
+     *
+     * @param seen track seen foreign keys to prevent infinite recursion in
+     * the case of foreign key cycles
+     */
+    private boolean isPrimaryKeyAutoAssigned(List seen) {
+        if (_autoAssign != null) 
+            return _autoAssign.booleanValue();
+
+        Column[] cols = getPrimaryKeyColumns();
+        if (cols.length == 0) {
+            _autoAssign = Boolean.FALSE;
+            return false;
+        }
+
+        for (int i = 0; i < cols.length; i++) {
+            if (cols[i].isAutoAssigned()) {
+                _autoAssign = Boolean.TRUE;
+                return true;
+            }
+        }
+
+        ForeignKey[] fks = _pkTable.getForeignKeys();
+        seen.add(this);
+        for (int i = 0; i < cols.length; i++) {
+            for (int j = 0; j < fks.length; j++) {
+                if (fks[j].getPrimaryKeyColumn(cols[i]) == null)
+                    continue;
+                if (!seen.contains(fks[j])
+                    && fks[j].isPrimaryKeyAutoAssigned(seen)) {
+                    _autoAssign = Boolean.TRUE;
+                    return true;
                 }
             }
-            _autoAssign = (auto) ? Boolean.TRUE : Boolean.FALSE;
         }
-        return _autoAssign.booleanValue();
+
+        _autoAssign = Boolean.FALSE;
+        return false;
     }
 
     /**
@@ -498,6 +534,8 @@ public class ForeignKey
         // force re-cache
         _locals = null;
         _pks = null;
+        if (_autoAssign == Boolean.FALSE)
+            _autoAssign = null;
     }
 
     /**
@@ -583,6 +621,8 @@ public class ForeignKey
         if ((_joins == null || _joins.isEmpty())
             && (_constsPK == null || _constsPK.isEmpty()))
             _pkTable = null;
+        if (remd && _autoAssign == Boolean.TRUE)
+            _autoAssign = null;
         return remd;
     }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java

@@ -524,7 +524,7 @@ public class FetchConfigurationImpl
         FetchConfigurationImpl clone = newInstance(_state);
         clone._parent = this;
         clone._availableDepth = reduce(_availableDepth);
-        clone._fromField = fm.getFullName();
+        clone._fromField = fm.getFullName(false);
         clone._fromType = type;
         clone._availableRecursion = getAvailableRecursionDepth(fm, type, true);
         return clone;
@@ -537,7 +537,7 @@ public class FetchConfigurationImpl
         if ((fmd.isInDefaultFetchGroup() 
             && hasFetchGroup(FetchGroup.NAME_DEFAULT))
             || hasFetchGroup(FetchGroup.NAME_ALL)
-            || hasField(fmd.getFullName()))
+            || hasField(fmd.getFullName(false)))
             return true;
         String[] fgs = fmd.getCustomFetchGroups();
         for (int i = 0; i < fgs.length; i++)

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java

@@ -442,12 +442,12 @@ class JPQLExpressionBuilder
         JPQLNode[] outers = root().findChildrenByID(JJTOUTERFETCHJOIN);
         for (int i = 0; outers != null && i < outers.length; i++)
             (joins == null ? joins = new TreeSet() : joins).
-                add(getPath(onlyChild(outers[i])).last().getFullName());
+                add(getPath(onlyChild(outers[i])).last().getFullName(false));
 
         JPQLNode[] inners = root().findChildrenByID(JJTINNERFETCHJOIN);
         for (int i = 0; inners != null && i < inners.length; i++)
             (joins == null ? joins = new TreeSet() : joins).
-                add(getPath(onlyChild(inners[i])).last().getFullName());
+                add(getPath(onlyChild(inners[i])).last().getFullName(false));
 
         if (joins != null)
             exps.fetchPaths = (String[]) joins.

openjpa-kernel/src/main/java/org/apache/openjpa/meta/ClassMetaData.java

@@ -2327,7 +2327,8 @@ public class ClassMetaData
             FieldMetaData f2 = (FieldMetaData) o2;
             if (f1.getListingIndex() == f2.getListingIndex()) {
                 if (f1.getIndex() == f2.getIndex())
-                    return f1.getFullName ().compareTo (f2.getFullName ());
+                    return f1.getFullName(false).compareTo
+                        (f2.getFullName(false));
 				if (f1.getIndex () == -1)
 					return 1;
 				if (f2.getIndex () == -1)

openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java

@@ -127,6 +127,7 @@ public class FieldMetaData
     private Class _dec = null;
     private ClassMetaData _decMeta = null;
     private String _fullName = null;
+    private String _embedFullName = null;
     private int _resMode = MODE_NONE;
 
     // load/store info
@@ -275,6 +276,7 @@ public class FieldMetaData
         _dec = cls;
         _decMeta = null;
         _fullName = null;
+        _embedFullName = null;
     }
 
     /**
@@ -297,12 +299,20 @@ public class FieldMetaData
     }
 
     /**
-     * The field name, qualified by the owning class.
+     * The field name, qualified by the owning class and optionally the
+     * embedding owner's name (if any).
      */
-    public String getFullName() {
+    public String getFullName(boolean embedOwner) {
         if (_fullName == null)
             _fullName = getDeclaringType().getName() + "." + _name;
-        return _fullName;
+        if (embedOwner && _embedFullName == null) {
+            if (_owner.getEmbeddingMetaData() == null)
+                _embedFullName = _fullName;
+            else
+                _embedFullName = _owner.getEmbeddingMetaData().
+                    getFieldMetaData().getFullName(true) + "." + _fullName;
+        }
+        return (embedOwner) ? _embedFullName : _fullName;
     }
 
     /**
@@ -1433,7 +1443,7 @@ public class FieldMetaData
     }
 
     public int hashCode() {
-        return getFullName().hashCode();
+        return getFullName(true).hashCode();
     }
 
     public boolean equals(Object other) {
@@ -1441,18 +1451,19 @@ public class FieldMetaData
             return true;
         if (!(other instanceof FieldMetaData))
             return false;
-        return getFullName().equals(((FieldMetaData) other).getFullName());
+        return getFullName(true).equals(((FieldMetaData) other).
+            getFullName(true));
     }
 
     public int compareTo(Object other) {
         if (other == null)
             return 1;
-        return getFullName().compareTo(((FieldMetaData) other).
+        return getFullName(true).compareTo(((FieldMetaData) other).
-            getFullName());
+            getFullName(true));
     }
 
     public String toString() {
-        return getFullName();
+        return getFullName(true);
     }
 
     ////////////////////////

openjpa-kernel/src/main/java/org/apache/openjpa/meta/ValueMetaDataImpl.java

@@ -316,7 +316,7 @@ public class ValueMetaDataImpl
     }
 
     public String toString() {
-        String ret = _owner.getFullName();
+        String ret = _owner.getFullName(true);
         if (this == _owner.getKey())
             return ret + "<key:" + _decType + ">";
         if (this == _owner.getElement()) {

openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java

@@ -179,17 +179,21 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
         return new LoggingConnection(conn);
     }
 
+    /**
+     * Include SQL in exception.
+     */
     private SQLException wrap(SQLException sqle, Statement stmnt) {
         if (sqle instanceof ReportingSQLException)
             return (ReportingSQLException) sqle;
-
         return new ReportingSQLException(sqle, stmnt);
     }
 
+    /**
+     * Include SQL in exception.
+     */
     private SQLException wrap(SQLException sqle, String sql) {
         if (sqle instanceof ReportingSQLException)
             return (ReportingSQLException) sqle;
-
         return new ReportingSQLException(sqle, sql);
     }
 
@@ -202,6 +206,9 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
         public void handleWarning(SQLWarning warning) throws SQLException;
     }
 
+    /**
+     * Logging connection.
+     */
     private class LoggingConnection extends DelegatingConnection {
 
         public LoggingConnection(Connection conn) throws SQLException {
@@ -242,7 +249,6 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
 
         public void commit() throws SQLException {
             long start = System.currentTimeMillis();
-
             try {
                 super.commit();
             } finally {
@@ -254,7 +260,6 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
 
         public void rollback() throws SQLException {
             long start = System.currentTimeMillis();
-
             try {
                 super.rollback();
             } finally {
@@ -266,7 +271,6 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
 
         public void close() throws SQLException {
             long start = System.currentTimeMillis();
-
             try {
                 super.close();
             } finally {
@@ -397,6 +401,22 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
             return new LoggingDatabaseMetaData(super.getMetaData(false));
         }
 
+        /**
+         * Log time elapsed since given start.
+         */
+        private void logTime(long startTime) throws SQLException {
+            if (_logs.isSQLEnabled())
+                _logs.logSQL("spent", startTime, this); 
+        }
+
+        /**
+         * Log time elapsed since given start.
+         */
+        private void logSQL(Statement stmnt) throws SQLException {
+            if (_logs.isSQLEnabled())
+                _logs.logSQL("executing " + stmnt, this); 
+        }
+
         /**
          * Handle any {@link SQLWarning}s on the current {@link Connection}.
          *
@@ -451,7 +471,7 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
          *
          * @param warning the warning to handle
          */
-        void handleSQLWarning(SQLWarning warning) throws SQLException {
+        private void handleSQLWarning(SQLWarning warning) throws SQLException {
             if (warning == null)
                 return;
             if (_warningAction == WARN_IGNORE)
@@ -490,6 +510,9 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
             }
         }
 
+        /**
+         * Metadata wrapper that logs actions.
+         */
         private class LoggingDatabaseMetaData
             extends DelegatingDatabaseMetaData {
 
@@ -698,57 +721,49 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
 
             public void cancel() throws SQLException {
                 if (_logs.isJDBCEnabled())
-                    _logs.logJDBC("cancel " + this + ": " + _sql,
+                    _logs.logJDBC("cancel " + this, LoggingConnection.this);
-                        LoggingConnection.this);
-
                 super.cancel();
             }
 
             protected ResultSet executeQuery(String sql, boolean wrap)
                 throws SQLException {
-                long start = System.currentTimeMillis();
-
                 _sql = sql;
+                logSQL(this);
+                long start = System.currentTimeMillis();
                 try {
                     return super.executeQuery(sql, wrap);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingStatement.this);
                 } finally {
-                    if (_logs.isSQLEnabled())
+                    logTime(start);
-                        _logs.logSQL("executing " + this, start,
-                            LoggingConnection.this);
                     handleSQLWarning(LoggingStatement.this);
                 }
             }
 
             public int executeUpdate(String sql) throws SQLException {
-                long start = System.currentTimeMillis();
-
                 _sql = sql;
+                logSQL(this);
+                long start = System.currentTimeMillis();
                 try {
                     return super.executeUpdate(sql);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingStatement.this);
                 } finally {
-                    if (_logs.isSQLEnabled())
+                    logTime(start);
-                        _logs.logSQL("executing " + this, start,
-                            LoggingConnection.this);
                     handleSQLWarning(LoggingStatement.this);
                 }
             }
 
             public boolean execute(String sql) throws SQLException {
-                long start = System.currentTimeMillis();
-
                 _sql = sql;
+                logSQL(this);
+                long start = System.currentTimeMillis();
                 try {
                     return super.execute(sql);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingStatement.this);
                 } finally {
-                    if (_logs.isSQLEnabled())
+                    logTime(start);
-                        _logs.logSQL("executing " + this, start,
-                            LoggingConnection.this);
                     handleSQLWarning(LoggingStatement.this);
                 }
             }
@@ -775,78 +790,78 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
 
             protected ResultSet executeQuery(String sql, boolean wrap)
                 throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.executeQuery(sql, wrap);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             public int executeUpdate(String sql) throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.executeUpdate(sql);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             public boolean execute(String sql) throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.execute(sql);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             protected ResultSet executeQuery(boolean wrap) throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.executeQuery(wrap);
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             public int executeUpdate() throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.executeUpdate();
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             public int[] executeBatch() throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.executeBatch();
                 } catch (SQLException se) {
@@ -884,21 +899,21 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
                     }
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing batch", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
             }
 
             public boolean execute() throws SQLException {
+                logSQL(this);
                 long start = System.currentTimeMillis();
-
                 try {
                     return super.execute();
                 } catch (SQLException se) {
                     throw wrap(se, LoggingPreparedStatement.this);
                 } finally {
-                    log("executing", start);
+                    logTime(start);
                     clearLogParameters(true);
                     handleSQLWarning(LoggingPreparedStatement.this);
                 }
@@ -1024,8 +1039,9 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
             }
 
             public void addBatch() throws SQLException {
+                if (_logs.isSQLEnabled())
+                    _logs.logSQL("batching " + this, LoggingConnection.this);
                 long start = System.currentTimeMillis();
-
                 try {
                     super.addBatch();
                     if (shouldTrackParameters()) {
@@ -1040,7 +1056,7 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
                     }
                 }
                 finally {
-                    log("batching", start);
+                    logTime(start);
                 }
             }
 
@@ -1124,12 +1140,6 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
                 super.appendInfo(buf);
             }
 
-            private void log(String msg, long startTime) throws SQLException {
-                if (_logs.isSQLEnabled())
-                    _logs.logSQL(msg + " " + this, startTime,
-                        LoggingConnection.this);
-            }
-
             private void clearLogParameters(boolean batch) {
                 if (_params != null)
                     _params.clear();
@@ -1192,6 +1202,9 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
             }
         }
 
+        /**
+         * Warning-handling result set.
+         */
         private class LoggingResultSet extends DelegatingResultSet {
 
             public LoggingResultSet(ResultSet rs, Statement stmnt) {

openjpa-lib/src/main/java/org/apache/openjpa/lib/util/SimpleRegex.java

@@ -78,6 +78,13 @@ public class SimpleRegex {
                 if (!mobile && targetPos != target.length() - len)
                     return false;
 
+                // In anycase, the remaining length of the target must be
+                // at least as long as the remaining length of the expression.
+                // (We check now to avoid sending a negative start pos to
+                // indexOf)
+                if (target.length() < len)
+                    return false;
+
                 // Match the end of the target to the remainder of the
                 // expression
                 int match = indexOf(target, target.length() - len, exprPos,

openjpa-project/src/doc/manual/ref_guide_runtime.xml

@@ -928,15 +928,16 @@ mag.setPageCount(300);
 oem.setSavepoint("pages");
 
 mag.setPrice(mag.getPageCount() * pricePerPage);
-// we decide to release pages since price depends on pages.
+// we decide to release "pages"...
 oem.releaseSavepoint("pages");
+// ... and set a new savepoint which includes all changes
 oem.setSavepoint("price");
 
 mag.setPrice(testPrice);
-...
-
 // we determine the test price is not good
 oem.rollbackToSavepoint("price");
+// had we chosen to not release "pages", we might have rolled back to
+// "pages" instead
 
 // the price is now restored to mag.getPageCount() * pricePerPage
 oem.getTransaction().commit();