OPENJPA-1736: Mappings with foreign keys as identity fields sometimes not resolved correctly

back-out changes in 2.0.x branch.
must be pre-approved before committing changes to 2.0.x.

git-svn-id: https://svn.apache.org/repos/asf/openjpa/branches/2.0.x@988277 13f79535-47bb-0310-9956-ffa450edef68

openjpa-kernel/src/main/java/org/apache/openjpa/conf/Compatibility.java

@@ -64,6 +64,7 @@ public class Compatibility {
     private boolean _superclassDiscriminatorStrategyByDefault = true;
     private boolean _isAbstractMappingUniDirectional = false;
     private boolean _isNonDefaultMappingAllowed = false;
+    private boolean _reorderMetaDataResolution = true;
     private boolean _reloadOnDetach = false;
     private boolean _ignoreDetachedStateFieldForProxySerialization = false;
     
@@ -507,6 +508,26 @@ public class Compatibility {
     public boolean isNonDefaultMappingAllowed() {
         return _isNonDefaultMappingAllowed;
     }
+    
+    /**
+     * Whether OpenJPA should reorder entities in MetaDataRepository.processBuffer() to ensure that the metadata for 
+     * entities with foreign keys in their identity are processed after the entities it depends on.
+     * 
+     * @return true if the reordering should be performed, false if not.
+     */
+    public boolean getReorderMetaDataResolution() {
+        return _reorderMetaDataResolution;
+    }
+    
+    /**
+     * Whether OpenJPA should reorder entities in MetaDataRepository.processBuffer() to ensure that the metadata for 
+     * entities with foreign keys in their identity are processed after the entities it depends on.
+     * 
+     * @param reorderProcessBuffer true if the reordering should be performed, false if not.
+     */
+    public void setReorderMetaDataResolution(boolean reorderProcessBuffer) {
+        _reorderMetaDataResolution = reorderProcessBuffer;
+    }
 
     /**
      * Whether OpenJPA should attempt to load fields when the DetachState

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

@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.meta;
+
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.ListIterator;
+import java.io.Serializable;
+
+public class InheritanceOrderedMetaDataList
+    implements Serializable {
+
+    private MetaDataInheritanceComparator _comp
+        = new MetaDataInheritanceComparator();
+    private LinkedList<ClassMetaData> buffer = new LinkedList<ClassMetaData>();
+
+    public boolean add(ClassMetaData meta) {
+        if (meta == null || buffer.contains(meta))
+            return false;
+        for (ListIterator<ClassMetaData> itr = buffer.listIterator();
+            itr.hasNext();) {
+            int ord = _comp.compare(meta, itr.next());
+            if (ord > 0)
+                continue;
+            if (ord == 0)
+                return false;
+            itr.previous();
+            itr.add(meta);
+            return true;
+        }
+        buffer.add(meta);
+        return true;
+    }
+
+    public boolean remove(ClassMetaData meta) {
+        return buffer.remove(meta);
+    }
+
+    public ClassMetaData peek() {
+        return buffer.peek();
+    }
+    
+    public int size() {
+        return buffer.size();
+    }
+    
+    public Iterator<ClassMetaData> iterator() {
+        return buffer.iterator();
+    }
+    
+    public boolean isEmpty() {
+        return buffer.isEmpty();
+    }
+    
+    public void clear() {
+        buffer.clear();
+    }
+}

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

@@ -141,8 +141,8 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     private final Collection<Class<?>> _registered = new HashSet<Class<?>>();
 
     // set of metadatas we're in the process of resolving
-    private final List<ClassMetaData> _resolving = new ArrayList<ClassMetaData>();
+    private final InheritanceOrderedMetaDataList _resolving = new InheritanceOrderedMetaDataList();
-    private final List<ClassMetaData> _mapping = new ArrayList<ClassMetaData>();
+    private final InheritanceOrderedMetaDataList _mapping = new InheritanceOrderedMetaDataList();
     private final List<RuntimeException> _errs = new LinkedList<RuntimeException>();
 
     // system listeners
@@ -152,6 +152,8 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     protected boolean _preloadComplete = false;
     protected boolean _locking = true;
     private static final String PRELOAD_STR = "Preload";
+    
+    private boolean _reorderMetaDataResolution = false;
 
     /**
      * Default constructor. Configure via {@link Configurable}.
@@ -768,28 +770,22 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     /**
      * Process the given metadata and the associated buffer.
      */
-    private List<ClassMetaData> processBuffer(ClassMetaData meta, List<ClassMetaData> buffer, int mode) {
+    private List<ClassMetaData> processBuffer(ClassMetaData meta, InheritanceOrderedMetaDataList buffer, int mode) {
-        // add the metadata to the buffer unless an instance for the same entity
-        // is already there
-        for (ClassMetaData cmd : buffer)
-            if (cmd.getDescribedType().equals(meta.getDescribedType()))
-                return null;
-
         // if we're already processing a metadata, just buffer this one; when
         // the initial metadata finishes processing, we traverse the buffer
         // and process all the others that were introduced during reentrant
         // calls
-        buffer.add(meta);
+        if (!buffer.add(meta) || buffer.size() != 1)
-        if (buffer.size() != 1)
             return null;
 
         // continually pop a metadata and process it until we run out; note
         // that each processing call might place more metas in the buffer as
-        // one class tries to access metadata for another
+        // one class tries to access metadata for another; also note that the
+        // buffer orders itself from least to most derived
         ClassMetaData buffered;
         List<ClassMetaData> processed = new ArrayList<ClassMetaData>(5);
         while (!buffer.isEmpty()) {
-            buffered = buffer.get(0);
+            buffered = buffer.peek();
             try {
                 buffered.resolve(mode);
                 processed.add(buffered);
@@ -812,6 +808,11 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
             }
         }
         
+        // Check if process buffer reordering for PCTypes that have relationships to other PCTypes in their identity 
+        // should be performed.
+        if (_reorderMetaDataResolution) {
+            processed = resolveFKInPKDependenciesOrdering(processed);
+        }
         return processed;
     }
 
@@ -1843,6 +1844,7 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     public void setConfiguration(Configuration conf) {
         _conf = (OpenJPAConfiguration) conf;
         _log = _conf.getLog(OpenJPAConfiguration.LOG_METADATA);
+        _reorderMetaDataResolution = _conf.getCompatibilityInstance().getReorderMetaDataResolution();
     }
 
     public void startConfiguration() {
@@ -2411,6 +2413,196 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     public XMLFieldMetaData newXMLFieldMetaData(Class<?> type, String name) {
         return new XMLFieldMetaData(type, name);
     }
+    
+    /**
+     * Analyzes the list of ClassMetaData in the supplied list for any which has foreign keys to other ClassMetaData 
+     * instances in its identity (in other words, PCTypes which have primary keys that are foreign keys to other
+     * tables), and returns a list arranged so that a ClassMetaData that depends on another ClassMetaData appears
+     * after it in the list.
+     *
+     * @param cmdList - List of ClassMetaData to examine
+     * @return - List of ClassMetaData, with ClassMetaData dependees moved after the last identified dependent 
+     *           ClassMetaData, if any move is necessary.
+     */
+    private List<ClassMetaData> resolveFKInPKDependenciesOrdering(List<ClassMetaData> cmdList) {
+        HashMap<ClassMetaData, CMDDependencyNode> nodeMap = new HashMap<ClassMetaData, CMDDependencyNode>();
+        HashSet<CMDDependencyNode> nodesWithDependenciesSet = new HashSet<CMDDependencyNode>();
+        ArrayList<CMDDependencyNode> nodeList = new ArrayList<CMDDependencyNode>(cmdList.size());
+        
+        // Initial analysis of ClassMetaData objects -- Populate the linked list with objects in the same order of 
+        // appearance in the original list. Identify CMDs whose identities have a FK to another CMD, and catalog that 
+        // dependency.
+        for (ClassMetaData cmd : cmdList) {
+            // Add this node to the list
+            CMDDependencyNode node = nodeMap.get(cmd);
+            if (node == null) {
+                node = new CMDDependencyNode(cmd);
+                nodeMap.put(cmd, node);
+            }
+            nodeList.add(node);
+            
+            // Examine its primary key fields, flag any references to another PCType that is defined in cmdList as a 
+            // dependency
+            FieldMetaData[] fmdArr = cmd.getPrimaryKeyFields();
+            for (FieldMetaData fmd : fmdArr) {
+                ValueMetaData vmd = fmd.getValue();
+                if (vmd.isTypePC()) {
+                    ClassMetaData targetCMD = vmd.getDeclaredTypeMetaData();
+
+                    // Only process entries which are in the cmdList, as we don't want to be adding anything new.
+                    if (!cmdList.contains(targetCMD)) {
+                        continue;
+                    }
+
+                    // Register the dependency
+                    CMDDependencyNode targetNode = null;
+                    if ((targetNode = nodeMap.get(targetCMD)) == null) {
+                        targetNode = new CMDDependencyNode(targetCMD);
+                        nodeMap.put(targetCMD, targetNode);
+                    }
+                    node.registerDependentNode(targetNode);
+                    nodesWithDependenciesSet.add(node);
+                }
+            }
+        }
+        
+        // Analysis is complete. For each CMD that has an identity foreign key dependency on another CMD, ensure that it
+        // appears later in the list then the CMD it is dependent on. If it appears earlier, move it immediately after 
+        // the CMD. If there are multiple CMDs the identity is dependent on, move it after the last dependency in
+        // the linked list.
+        for (CMDDependencyNode node : nodesWithDependenciesSet) {
+            // Check if there is a cycle (dependencies or subdependencies that create a cycle in the graph. If one is 
+            // detected, then this algorithm cannot be used to reorder the CMD list.  Emit a warning, and return the 
+            // original list.
+            if (node.checkForCycle()) {
+                if (_log.isWarnEnabled()) {
+                    _log.warn(_loc.get("cmd-discover-cycle", node.getCmd().getResourceName()));
+                }
+                return cmdList;
+            }
+ 
+            int nodeIndex = nodeList.indexOf(node);
+            Set<CMDDependencyNode> dependencies = node.getDependsOnSet();       
+            
+            // If the current node has a dependency that appears later in the list, then this node needs
+            // to be moved to the point immediately after that dependency.
+            CMDDependencyNode moveAfter = null;
+            int moveAfterIndex = -1;
+            for (CMDDependencyNode depNode : dependencies) {               
+                int dependencyIndex = nodeList.indexOf(depNode);
+                if ((nodeIndex < dependencyIndex) && (moveAfterIndex < dependencyIndex)) {
+                    moveAfter = depNode;
+                    moveAfterIndex = dependencyIndex;
+                }
+            }
+            if (moveAfter != null) {
+                nodeList.remove(nodeIndex);
+                nodeList.add(nodeList.indexOf(moveAfter) + 1, node);
+            }      
+        }
+        
+        // Sorting is complete, build the return list.  Clear the dependsOnSet for the GC.
+        ArrayList<ClassMetaData> returnList = new ArrayList<ClassMetaData>();
+        for (CMDDependencyNode current : nodeList) {
+            returnList.add(current.getCmd());
+            current.getDependsOnSet().clear();
+        }
+        
+        return returnList;
+    }
+
+
+    /**
+     * Linked list node class for managing any foreign keys in the identity of a ClassMetaData instance.
+     * 
+     */
+    private class CMDDependencyNode {
+        private ClassMetaData cmd;
+
+        // Marker for quick determination if this node has dependencies
+        private boolean hasDependencies = false;
+
+        // List of ClassMetaData objects this ClassMetaData depends on
+        private HashSet<CMDDependencyNode> dependsOnSet = new HashSet<CMDDependencyNode>();
+
+        /**
+         * Inner class constructor
+         */
+        CMDDependencyNode(ClassMetaData cmd) {
+            this.cmd = cmd;
+        }
+
+        /**
+         * Returns the ClassMetaData instance referenced by this node.
+         */
+        public ClassMetaData getCmd() {
+            return cmd;
+        }
+
+        /**
+         * 
+         * @return true if this node's ClassMetaData has a FK in its identity that refers to another ClassMetaData; 
+         *         false if it does not.
+         */
+        public boolean getHasDependencies() {
+            return hasDependencies;
+        }
+
+        /**
+         * Registers a ClassMetaData modelled by a CMDDependencyNode as a dependency of this ClassMetaData.
+         * 
+         */
+        public void registerDependentNode(CMDDependencyNode node) {
+            getDependsOnSet().add(node);
+            hasDependencies = true;
+        }
+
+        /**
+         * Returns a Set containing all of the CMDDependencyNode instances that this node has a FK in identity 
+         * dependency on.
+         * 
+         */
+        public Set<CMDDependencyNode> getDependsOnSet() {
+            return dependsOnSet;
+        }
+
+        /**
+         * Checks all dependencies, and sub-dependencies, for any cycles in the dependency graph.
+         * 
+         * @return true if a cycle was discovered, false if not.
+         */
+        public boolean checkForCycle() {
+            java.util.Stack<CMDDependencyNode> visitStack = new java.util.Stack<CMDDependencyNode>();
+            return internalCheckForCycle(visitStack);
+        }
+
+        /**
+         * Internal implementation of the cycle detection.
+         * 
+         * @param visitStack
+         * @return true if a cycle is detected, false if no cycle was detected.
+         */
+        private boolean internalCheckForCycle(java.util.Stack<CMDDependencyNode> visitStack) {
+            if (visitStack.contains(this)) {
+                return true;
+            }
+            visitStack.push(this);
+
+            try {
+                for (CMDDependencyNode node : dependsOnSet) {
+                    if (node.getHasDependencies()) {
+                        if (node.internalCheckForCycle(visitStack) == true) {
+                            return true;
+                        }
+                    }
+                }
+            } finally {
+                visitStack.pop();
+            }
+            
+            return false;
+        }
+    }
 
     public static boolean needsPreload(Options o) {
         if (o.getBooleanProperty(PRELOAD_STR) == true) {

openjpa-kernel/src/main/resources/org/apache/openjpa/meta/localizer.properties

@@ -348,6 +348,9 @@ repos-preload-none: No persistent metadata found for loading during initializati
 repos-preloading: Following metadata are being loaded during initialization by "{0}": {1}. 
 repos-preload-error: Unexpected error during early loading during initialization. \
 	See nested stacktrace for details. 	  
+cmd-discover-cycle: A cycle was detected while resolving the identity \
+    references for type "{0}".  The original process buffer ordering \
+    will be used.
 repos-initializeEager-none: No persistent metadata found for loading during initialization. \
     The persistent classes must be listed in persistence unit configuration to be loaded during initialization.
 repos-initializeEager-found: The following classes are being preloaded "{0}".	   

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity/TestEntityAsIdentityFields.java

@@ -28,7 +28,9 @@ import org.apache.openjpa.persistence.test.SingleEMFTestCase;
 
 public class TestEntityAsIdentityFields extends SingleEMFTestCase {    
     public void setUp() {
-        setUp(Account.class, AccountGroup.class, Person.class);
+        setUp(
+                Account.class, AccountGroup.class, Person.class,
+                "openjpa.Compatibility", "reorderMetaDataResolution=true");
     }
     
     /**

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity2/Attendance.java

@@ -1,65 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.    
- */
-package org.apache.openjpa.persistence.identity.entityasidentity2;
-
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import javax.persistence.IdClass;
-import javax.persistence.ManyToOne;
-import javax.persistence.Table;
-
-@Entity
-@Table(name = "EAI2Attendance")
-@IdClass(Attendance.AttendanceId.class)
-public class Attendance {
-
-    public static class AttendanceId {
-
-        private int student;
-        private int course;
-
-        public AttendanceId() {}
-
-        public AttendanceId(int studentId, int courseId) {
-            this.student = studentId;
-            this.course = courseId;
-        }
-
-        public String toString() {
-            return student + ":" + course;
-        }
-
-        public int hashCode() {
-            return (17 + student) * 37 + course;
-        }
-
-        public boolean equals(Object other) {
-            return this == other
-                || other instanceof AttendanceId
-                    && student == ((AttendanceId) other).student
-                    && course == ((AttendanceId) other).course;
-        }
-    }
-
-    @Id @ManyToOne
-    Student student;
-
-    @Id @ManyToOne
-    Course course;
-}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity2/Course.java

@@ -1,37 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.    
- */
-package org.apache.openjpa.persistence.identity.entityasidentity2;
-
-import java.util.Collection;
-
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import javax.persistence.OneToMany;
-import javax.persistence.Table;
-
-@Entity
-@Table(name = "EAI2Course")
-public class Course {
-
-    @Id
-    int id;
-
-    @OneToMany(mappedBy = "course")
-    Collection<Attendance> attendances;
-}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity2/Person.java

@@ -1,31 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.    
- */
-package org.apache.openjpa.persistence.identity.entityasidentity2;
-
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import javax.persistence.Table;
-
-@Entity
-@Table(name = "EAI2Person")
-public class Person {
-
-    @Id
-    int id;
-}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity2/Student.java

@@ -1,38 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.    
- */
-package org.apache.openjpa.persistence.identity.entityasidentity2;
-
-import java.util.Collection;
-
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import javax.persistence.OneToMany;
-import javax.persistence.OneToOne;
-import javax.persistence.Table;
-
-@Entity
-@Table(name = "EAI2Student")
-public class Student {
-
-    @Id @OneToOne
-    Person person;
-
-    @OneToMany(mappedBy = "student")
-    Collection<Attendance> attendances;
-}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/identity/entityasidentity2/TestEntityAsIdentityFields2.java

@@ -1,40 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.    
- */
-package org.apache.openjpa.persistence.identity.entityasidentity2;
-
-import javax.persistence.EntityManager;
-import javax.persistence.Query;
-
-import org.apache.openjpa.persistence.test.SingleEMFTestCase;
-
-public class TestEntityAsIdentityFields2 extends SingleEMFTestCase {
-
-    public void setUp() {
-        setUp(Attendance.class, Course.class, Person.class, Student.class);
-    }
-
-    public void testEntityAsIdentityField001() {
-        EntityManager em = emf.createEntityManager();
-
-        Query query = em.createQuery("select p from Person p");
-        query.getResultList();
-        
-        em.close();
-    }
-}