From b8bb8404a9c1da1dd22ac8b414f7496ee033acbd Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 17 Jul 2007 23:56:45 +0000 Subject: [PATCH] OPENJPA-235 break-nullable-patch contributed by Markus Fuchs git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@557089 13f79535-47bb-0310-9956-ffa450edef68 --- .../jdbc/kernel/ConstraintUpdateManager.java | 265 +++++++++++++----- .../openjpa/jdbc/schema/ForeignKey.java | 13 + .../openjpa/jdbc/kernel/localizer.properties | 10 +- .../openjpa/lib/graph/DepthFirstAnalysis.java | 155 +++++++++- .../org/apache/openjpa/lib/graph/Edge.java | 33 +++ .../openjpa/lib/graph/localizer.properties | 22 ++ .../lib/graph/TestDepthFirstAnalysis.java | 76 ++++- .../apache/openjpa/jdbc/kernel/EntityB.java | 2 +- .../apache/openjpa/jdbc/kernel/EntityC.java | 2 +- .../apache/openjpa/jdbc/kernel/EntityD.java | 30 ++ .../apache/openjpa/jdbc/kernel/EntityE.java | 76 +++++ .../kernel/TestNoForeignKeyViolation.java | 82 +++++- 12 files changed, 680 insertions(+), 86 deletions(-) create mode 100644 openjpa-lib/src/main/resources/org/apache/openjpa/lib/graph/localizer.properties create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityE.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ConstraintUpdateManager.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ConstraintUpdateManager.java index 7b385a927..8fed879bb 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ConstraintUpdateManager.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ConstraintUpdateManager.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; +import java.util.List; import java.util.Map; import org.apache.openjpa.jdbc.meta.ClassMapping; @@ -43,6 +44,7 @@ import org.apache.openjpa.lib.graph.Graph; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.util.InternalException; import org.apache.openjpa.util.OpenJPAException; +import org.apache.openjpa.util.UserException; /** *

Standard update manager, capable of foreign key constraint evaluation.

@@ -163,7 +165,7 @@ public class ConstraintUpdateManager row2 = getInsertRow(insertMap, rowMgr, row); if (row2 != null) { ignoreUpdates = false; - graphs[1] = addEdge(graphs[1], row, (PrimaryRow) row2, null); + graphs[1] = addEdge(graphs[1], (PrimaryRow) row2, row, null); } // now check this row's fks against other deletes @@ -180,7 +182,7 @@ public class ConstraintUpdateManager row2 = rowMgr.getRow(fks[j].getPrimaryKeyTable(), Row.ACTION_DELETE, fkVal, false); if (row2 != null && row2.isValid() && row2 != row) - graphs[1] = addEdge(graphs[1], row, (PrimaryRow) row2, + graphs[1] = addEdge(graphs[1], (PrimaryRow) row2, row, fks[j]); } } @@ -249,7 +251,7 @@ public class ConstraintUpdateManager Row.ACTION_INSERT, row.getForeignKeySet(fks[j]), false); if (row2 != null && row2.isValid() && (row2 != row || fks[j].isDeferred() || fks[j].isLogical())) - graph = addEdge(graph, (PrimaryRow) row2, row, fks[j]); + graph = addEdge(graph, row, (PrimaryRow) row2, fks[j]); } // see if there are any relation id columns dependent on @@ -263,7 +265,7 @@ public class ConstraintUpdateManager row2 = rowMgr.getRow(getBaseTable(sm), Row.ACTION_INSERT, sm, false); if (row2 != null && row2.isValid()) - graph = addEdge(graph, (PrimaryRow) row2, row, cols[j]); + graph = addEdge(graph, row, (PrimaryRow) row2, cols[j]); } } return graph; @@ -318,80 +320,205 @@ public class ConstraintUpdateManager return; DepthFirstAnalysis dfa = newDepthFirstAnalysis(graph, autoAssign); - Collection nodes = dfa.getSortedNodes(); - Collection backs = dfa.getEdges(Edge.TYPE_BACK); + Collection insertUpdates = new LinkedList(); + Collection deleteUpdates = new LinkedList(); + boolean recalculate; - // handle circular constraints: - // - if deleted row A has a ciricular fk to deleted row B, then use an - // update statement to null A's fk to B + // Handle circular constraints: + // - if deleted row A has a ciricular fk to deleted row B, + // then use an update statement to null A's fk to B before flushing, + // and then flush // - if inserted row A has a circular fk to updated/inserted row B, - // then null the fk in the B row object, and after flushing, use - // an update to set the fk to back to A - Collection insertUpdates = null; - Collection deleteUpdates = null; - PrimaryRow row; - RowImpl update; - Edge edge; - ForeignKey fk; - Column col; - for (Iterator itr = backs.iterator(); itr.hasNext();) { - edge = (Edge) itr.next(); - if (edge.getUserObject() == null) - throw new InternalException(_loc.get("del-ins-cycle")); + // then null the fk in the B row object, then flush, + // and after flushing, use an update to set the fk back to A + // Depending on where circular dependencies are broken, the + // topological order of the graph nodes has to be re-calculated. + recalculate = resolveCycles(graph, dfa.getEdges(Edge.TYPE_BACK), + deleteUpdates, insertUpdates); + recalculate |= resolveCycles(graph, dfa.getEdges(Edge.TYPE_FORWARD), + deleteUpdates, insertUpdates); - // use a primary row update to prevent setting pk and fk values - // until after flush, to get latest auto-increment values - row = (PrimaryRow) edge.getTo(); - if (row.getAction() == Row.ACTION_DELETE) { - // copy where conditions into new update that nulls the fk - row = (PrimaryRow) edge.getFrom(); - update = new PrimaryRow(row.getTable(), Row.ACTION_UPDATE,null); - row.copyInto(update, true); - if (edge.getUserObject() instanceof ForeignKey) { - fk = (ForeignKey) edge.getUserObject(); - update.setForeignKey(fk, row.getForeignKeyIO(fk), null); - } else - update.setNull((Column) edge.getUserObject()); - - if (deleteUpdates == null) - deleteUpdates = new LinkedList(); - deleteUpdates.add(update); - } else { - // copy where conditions into new update that sets the fk - update = new PrimaryRow(row.getTable(), Row.ACTION_UPDATE,null); - if (row.getAction() == Row.ACTION_INSERT) { - if (row.getPrimaryKey() == null) - throw new InternalException(_loc.get("ref-cycle")); - update.wherePrimaryKey(row.getPrimaryKey()); - } else - row.copyInto(update, true); - if (edge.getUserObject() instanceof ForeignKey) { - fk = (ForeignKey) edge.getUserObject(); - update.setForeignKey(fk, row.getForeignKeyIO(fk), - row.getForeignKeySet(fk)); - row.clearForeignKey(fk); - } else { - col = (Column) edge.getUserObject(); - update.setRelationId(col, row.getRelationIdSet(col), - row.getRelationIdCallback(col)); - row.clearRelationId(col); - } - - if (insertUpdates == null) - insertUpdates = new LinkedList(); - insertUpdates.add(update); - } + if (recalculate) { + dfa = recalculateDepthFirstAnalysis(graph, autoAssign); } // flush delete updates to null fks, then all rows in order, then // the insert updates to set circular fk values - if (deleteUpdates != null) - flush(deleteUpdates, psMgr); + flush(deleteUpdates, psMgr); + Collection nodes = dfa.getSortedNodes(); for (Iterator itr = nodes.iterator(); itr.hasNext();) psMgr.flush((RowImpl) itr.next()); - if (insertUpdates != null) - flush(insertUpdates, psMgr); - } + flush(insertUpdates, psMgr); + } + + /** + * Break a circular dependency caused by delete operations. + * If deleted row A has a ciricular fk to deleted row B, then use an update + * statement to null A's fk to B before deleting B, then delete A. + * @param edge Edge in the dependency graph corresponding to a foreign key + * constraint. This dependency is broken by nullifying the foreign key. + * @param deleteUpdates Collection of update statements that are executed + * before the delete operations are flushed + */ + private void addDeleteUpdate(Edge edge, Collection deleteUpdates) + throws SQLException { + PrimaryRow row; + RowImpl update; + ForeignKey fk; + + // copy where conditions into new update that nulls the fk + row = (PrimaryRow) edge.getTo(); + update = new PrimaryRow(row.getTable(), Row.ACTION_UPDATE, null); + row.copyInto(update, true); + if (edge.getUserObject() instanceof ForeignKey) { + fk = (ForeignKey) edge.getUserObject(); + update.setForeignKey(fk, row.getForeignKeyIO(fk), null); + } else + update.setNull((Column) edge.getUserObject()); + + deleteUpdates.add(update); + } + + /** + * Break a circular dependency caused by insert operations. + * If inserted row A has a circular fk to updated/inserted row B, + * then null the fk in the B row object, then flush, + * and after flushing, use an update to set the fk back to A. + * @param row Row to be flushed + * @param edge Edge in the dependency graph corresponding to a foreign key + * constraint. This dependency is broken by nullifying the foreign key. + * @param insertUpdates Collection of update statements that are executed + * after the insert/update operations are flushed + */ + private void addInsertUpdate(PrimaryRow row, Edge edge, + Collection insertUpdates) throws SQLException { + RowImpl update; + ForeignKey fk; + Column col; + + // copy where conditions into new update that sets the fk + update = new PrimaryRow(row.getTable(), Row.ACTION_UPDATE, null); + if (row.getAction() == Row.ACTION_INSERT) { + if (row.getPrimaryKey() == null) + throw new InternalException(_loc.get("ref-cycle")); + update.wherePrimaryKey(row.getPrimaryKey()); + } else { + // Row.ACTION_UPDATE + row.copyInto(update, true); + } + if (edge.getUserObject() instanceof ForeignKey) { + fk = (ForeignKey) edge.getUserObject(); + update.setForeignKey(fk, row.getForeignKeyIO(fk), + row.getForeignKeySet(fk)); + row.clearForeignKey(fk); + } else { + col = (Column) edge.getUserObject(); + update.setRelationId(col, row.getRelationIdSet(col), + row.getRelationIdCallback(col)); + row.clearRelationId(col); + } + + insertUpdates.add(update); + } + + /** + * Finds a nullable foreign key by walking the dependency cycle. + * Circular dependencies can be broken at this point. + * @param cycle Cycle in the dependency graph. + * @return Edge corresponding to a nullable foreign key. + */ + private Edge findBreakableLink(List cycle) { + Edge breakableLink = null; + for (Iterator iter = cycle.iterator(); iter.hasNext(); ) { + Edge edge = (Edge) iter.next(); + Object userObject = edge.getUserObject(); + if (userObject instanceof ForeignKey) { + if (!((ForeignKey) userObject).hasNotNullColumns()) { + breakableLink = edge; + break; + } + } else if (userObject instanceof Column) { + if (!((Column) userObject).isNotNull()) { + breakableLink = edge; + break; + } + } + } + return breakableLink; + } + + /** + * Re-calculates the DepthFirstSearch analysis of the graph + * after some of the edges have been removed. Ensures + * that the dependency graph is cycle free. + * @param graph The graph of statements to be walked + * @param autoAssign Whether any of the rows in the graph have any + * auto-assign constraints + */ + private DepthFirstAnalysis recalculateDepthFirstAnalysis(Graph graph, + boolean autoAssign) { + DepthFirstAnalysis dfa; + // clear previous traversal data + graph.clearTraversal(); + dfa = newDepthFirstAnalysis(graph, autoAssign); + // make sure that the graph is non-cyclic now + assert (dfa.hasNoCycles()): _loc.get("graph-not-cycle-free"); + return dfa; + } + + /** + * Resolve circular dependencies by identifying and breaking + * a nullable foreign key. + * @param graph Dependency graph. + * @param edges Collection of edges. Each edge indicates a possible + * circular dependency + * @param deleteUpdates Collection of update operations (nullifying + * foreign keys) to be filled. These updates will be executed before + * the rows in the dependency graph are flushed + * @param insertUpdates CCollection of update operations (nullifying + * foreign keys) to be filled. These updates will be executed after + * the rows in the dependency graph are flushed + * @return Depending on where circular dependencies are broken, the + * topological order of the graph nodes has to be re-calculated. + */ + private boolean resolveCycles(Graph graph, Collection edges, + Collection deleteUpdates, Collection insertUpdates) + throws SQLException { + boolean recalculate = false; + for (Iterator itr = edges.iterator(); itr.hasNext();) { + Edge edge = (Edge) itr.next(); + List cycle = edge.getCycle(); + + if (cycle != null) { + // find a nullable foreign key + Edge breakableLink = findBreakableLink(cycle); + if (breakableLink == null) { + throw new UserException(_loc.get("no-nullable-fk")); + } + + // topologic node order must be re-calculated, if the + // breakable link is different from the edge where + // the circular dependency was originally detected + if (edge != breakableLink) { + recalculate = true; + } + + if (!breakableLink.isRemovedFromGraph()) { + + // use a primary row update to prevent setting pk and fk values + // until after flush, to get latest auto-increment values + PrimaryRow row = (PrimaryRow) breakableLink.getFrom(); + if (row.getAction() == Row.ACTION_DELETE) { + addDeleteUpdate(breakableLink, deleteUpdates); + } else { + addInsertUpdate(row, breakableLink, insertUpdates); + } + graph.removeEdge(breakableLink); + } + } + } + return recalculate; + } /** * Create a new {@link DepthFirstAnalysis} suitable for the given graph diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/ForeignKey.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/ForeignKey.java index 0baf0e773..40df305ea 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/ForeignKey.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/ForeignKey.java @@ -701,6 +701,19 @@ public class ForeignKey && match(getPrimaryKeyColumns(), fkPKCols); } + /** + * Checks for non-nullable local columns. + */ + public boolean hasNotNullColumns() { + Column[] columns = getColumns(); + for (int j = 0; j < columns.length; j++) { + if (columns[j].isNotNull()) { + return true; + } + } + return false; + } + private static boolean match(Column[] cols, Column[] fkCols) { if (cols.length != fkCols.length) return false; diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties index 3cb715d72..dd0f87b75 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties @@ -98,7 +98,15 @@ native-seq-usage: Usage: java org.apache.openjpa.jdbc.kernel.NativeJDBCSeq\n\ \t[-properties/-p ]\n\ \t[- ]* bad-level: Invalid isolation level. Valid levels are -1, \ - Connection.TRANSACTION_NONE, Connection.TRANSACTION_READ_UNCOMMITTED, \ + Connection.TRANSACTION_NONE, Connection.TRANSACTION_READ_UNCOMMITTED, \ Connection.TRANSACTION_READ_COMMITTED, \ Connection.TRANSACTION_REPEATABLE_READ, or \ Connection.TRANSACTION_SERIALIZABLE. Specified value: {0}. +no-nullable-fk: No nullable foreign key found to resolve circular flush\n\ + dependency. During flush processing, changes to instances, new\n\ + instances, and deleted instances must be processed in a specific sequence\n\ + to avoid foreign key constraint violations. The changes required in this\n\ + transaction cannot be reordered because none of the foreign key constraints\n\ + is nullable (optional). +graph-not-cycle-free: A circular flush dependency has been found after all \ + circular dependencies should have been resolved. diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/DepthFirstAnalysis.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/DepthFirstAnalysis.java index 99a8fa642..ae7647450 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/DepthFirstAnalysis.java +++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/DepthFirstAnalysis.java @@ -18,6 +18,8 @@ */ package org.apache.openjpa.lib.graph; +import org.apache.openjpa.lib.util.Localizer; + import java.util.AbstractList; import java.util.ArrayList; import java.util.Arrays; @@ -26,6 +28,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -42,6 +45,9 @@ import java.util.Map; */ public class DepthFirstAnalysis { + private static final Localizer _loc = Localizer.forPackage + (DepthFirstAnalysis.class); + private final Graph _graph; private final Map _nodeInfo = new HashMap(); private Comparator _comp; @@ -65,14 +71,15 @@ public class DepthFirstAnalysis { node = itr.next(); info = (NodeInfo) _nodeInfo.get(node); if (info.color == NodeInfo.COLOR_WHITE) - visit(graph, node, info, 0); + visit(graph, node, info, 0, new LinkedList()); } } /** * Visit a node. See Introduction to Algorithms book for details. */ - private int visit(Graph graph, Object node, NodeInfo info, int time) { + private int visit(Graph graph, Object node, NodeInfo info, int time, + List path) { // discover node info.color = NodeInfo.COLOR_GRAY; @@ -89,14 +96,24 @@ public class DepthFirstAnalysis { otherInfo = (NodeInfo) _nodeInfo.get(other); if (otherInfo.color == NodeInfo.COLOR_WHITE) { // undiscovered node; recurse into it - childTime = visit(graph, other, otherInfo, time); + path.add(edge); + childTime = visit(graph, other, otherInfo, time, path); + path.remove(edge); edge.setType(Edge.TYPE_TREE); } else if (otherInfo.color == NodeInfo.COLOR_GRAY) { childTime = -1; edge.setType(Edge.TYPE_BACK); + // calculate the cycle including this edge + edge.setCycle(cycleForBackEdge(edge, path)); } else { childTime = otherInfo.finished; edge.setType(Edge.TYPE_FORWARD); + // find the cycle including this edge + List cycle = new LinkedList(); + cycle.add(edge); + if (cycleForForwardEdge(graph, other, node, cycle)) { + edge.setCycle(cycle); + } } maxChildTime = Math.max(maxChildTime, childTime); } @@ -134,7 +151,7 @@ public class DepthFirstAnalysis { /** * Return all edges of the given type. This method can be used to * discover all edges that cause cycles in the graph by passing it - * the {@link #EDGE_BACK} edge type. + * the {@link Edge#TYPE_BACK} or {@link Edge#TYPE_FORWARD} edge type. */ public Collection getEdges(int type) { Collection typed = null; @@ -166,6 +183,132 @@ public class DepthFirstAnalysis { return info.finished; } + /** + * Returns a list of graph edges forming a cycle. The cycle begins + * with a type {@link Edge#TYPE_BACK} edge. + * @param backEdge "Starting" edge of the cycle + * @param path Continuous list of graph edges, may be null + * @param pos Index of the first edge in path continuing the cycle + * @return Cycle starting with a type {@link Edge#TYPE_BACK} edge + */ + private List buildCycle(Edge backEdge, List path, int pos) { + int length = path != null ? path.size() - pos : 0; + List cycle = new ArrayList(length + 1); + cycle.add(0, backEdge); + for (int i = 0; i < length; i++) { + cycle.add(i + 1, path.get(pos + i)); + } + return cycle; + } + + /** + * Computes the list of edges forming a cycle. The cycle always exists for + * a type {@link Edge#TYPE_BACK} edge. This method should only be called + * for type {@link Edge#TYPE_BACK} edges. + * @param edge Edge where the cycle was detected + * @param path Path consisting of edges to the edge's starting node + * @return Cycle starting with a type {@link Edge#TYPE_BACK} edge + */ + private List cycleForBackEdge(Edge edge, List path) { + if (edge.getType() != Edge.TYPE_BACK) { + return null; + } + + List cycle; + int pos = 0; + if (path != null && edge.getFrom() != edge.getTo()) { + // Not a single edge loop + pos = findNodeInPath(edge.getTo(), path); + assert (pos >= 0): _loc.get("node-not-on-path", edge, edge.getTo()); + } else { + assert (edge.getFrom() == edge.getTo()): + _loc.get("edge-no-loop", edge).getMessage(); + path = null; + } + cycle = buildCycle(edge, path, pos); + assert (cycle != null): _loc.get("cycle-null", edge).getMessage(); + return cycle; + } + + /** + * Computes the cycle of edges including node cycleTo. The cycle must not + * necessarily exist. This method should only be called for type + * {@link Edge#TYPE_FORWARD} edges. + * @param graph Graph + * @param node Current node + * @param cycleTo End node for loop + * @param path Path from loop end node to current node + * @return True if a cycle has been found. The cycle will be contained in + * the path parameter. + */ + private boolean cycleForForwardEdge(Graph graph, Object node, + Object cycleTo, List path) { + boolean found = false; + Collection edges = graph.getEdgesFrom(node); + for (Iterator itr = edges.iterator(); !found && itr.hasNext();) { + Edge edge = (Edge) itr.next(); + Object other = edge.getOther(node); + // Single edge loops are ignored + if (node != other) { + if (other == cycleTo) { + // Cycle complete + path.add(edge); + found = true; + } else if (!path.contains(edge)){ + // Walk this edge + path.add(edge); + found = cycleForForwardEdge(graph, other, cycleTo, path); + if (!found) { + // Remove edge again + path.remove(edge); + } + } + } + } + return found; + } + + /** + * Finds the position of the edge starting from a particular node in the + * continuous list of edges. + * @param node Node on the cycle. + * @param path Continuous list of graph edges. + * @return Edge index if found, -1 otherwise. + */ + private int findNodeInPath(Object node, List path) { + int pos = -1; + if (path != null) { + for (int i = 0; i < path.size(); i++) { + if (((Edge)path.get(i)).getFrom() == node) { + pos = i; + } + } + } + return pos; + } + + /** + * Test, if the analysis didn't find cycles. + */ + public boolean hasNoCycles() { + // a) there must not be any back edges + if (!getEdges(Edge.TYPE_BACK).isEmpty()) { + return false; + } + // b) there might be forward edges + // make sure these don't indicate cycles + Collection edges = getEdges(Edge.TYPE_FORWARD); + if (!edges.isEmpty()) { + for (Iterator itr = edges.iterator(); itr.hasNext();) { + Edge edge = (Edge) itr.next(); + if (edge.getCycle() != null) { + return false; + } + } + } + return true; + } + /** * Comparator for toplogically sorting entries in the node info map. */ @@ -184,8 +327,8 @@ public class DepthFirstAnalysis { NodeInfo n1 = (NodeInfo) e1.getValue(); NodeInfo n2 = (NodeInfo) e2.getValue(); - // reverse finished order - int ret = n2.finished - n1.finished; + // sort by finished order + int ret = n1.finished - n2.finished; if (ret == 0 && _subComp != null) ret = _subComp.compare(e1.getKey(), e2.getKey()); return ret; diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/Edge.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/Edge.java index 314afcdd0..5d5b1a7fe 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/Edge.java +++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/graph/Edge.java @@ -18,6 +18,8 @@ */ package org.apache.openjpa.lib.graph; +import java.util.List; + /** *

A graph edge. Includes the from and to nodes, an arbitrary user object, * and a weight. Edges can be either directed or undirected.

@@ -52,6 +54,8 @@ public class Edge { private int _type = 0; private double _weight = 0; private Object _userObj = null; + private List _cycle = null; + private boolean _removedFromGraph = false; /** * Constructor. @@ -175,11 +179,40 @@ public class Edge { _type = type; } + /** + * List of edges forming a cycle. Only set for TYPE_BACK and TYPE_FORWARD edges. + */ + public List getCycle() { + return _cycle; + } + + /** + * List of edges forming a cycle. Only set for TYPE_BACK and TYPE_FORWARD edges. + */ + public void setCycle(List cycle) { + _cycle = cycle; + } + + /** + * Returns if this edge is (still) part of the graph. + */ + public boolean isRemovedFromGraph() { + return _removedFromGraph; + } + + /** + * Mark this edge as removed from the graph. + */ + public void setRemovedFromGraph() { + this._removedFromGraph = true; + } + /** * Clear traversal info. */ public void clearTraversal() { _type = 0; + _cycle = null; } public String toString() { diff --git a/openjpa-lib/src/main/resources/org/apache/openjpa/lib/graph/localizer.properties b/openjpa-lib/src/main/resources/org/apache/openjpa/lib/graph/localizer.properties new file mode 100644 index 000000000..ed10c2082 --- /dev/null +++ b/openjpa-lib/src/main/resources/org/apache/openjpa/lib/graph/localizer.properties @@ -0,0 +1,22 @@ +# 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. + +cycle-null: The cycle must not be null for edge {0}! +edge-no-loop: Edge {0} must be a single node loop, if the depth first search \ + path is null! +node-not-on-path: Could not find node {1} in the depth first search path \ + leading to edge {0}! diff --git a/openjpa-lib/src/test/java/org/apache/openjpa/lib/graph/TestDepthFirstAnalysis.java b/openjpa-lib/src/test/java/org/apache/openjpa/lib/graph/TestDepthFirstAnalysis.java index 33fdc99ff..d16170b5a 100644 --- a/openjpa-lib/src/test/java/org/apache/openjpa/lib/graph/TestDepthFirstAnalysis.java +++ b/openjpa-lib/src/test/java/org/apache/openjpa/lib/graph/TestDepthFirstAnalysis.java @@ -20,6 +20,7 @@ package org.apache.openjpa.lib.graph; import java.util.Collection; import java.util.Iterator; +import java.util.List; import org.apache.openjpa.lib.test.AbstractTestCase; @@ -34,6 +35,10 @@ public class TestDepthFirstAnalysis private DepthFirstAnalysis _dfa = null; public void setUp() { + setUpGraph1(); + } + + public void setUpGraph1() { Graph graph = new Graph(); Object node1 = new Object(); Object node2 = new Object(); @@ -51,15 +56,37 @@ public class TestDepthFirstAnalysis _dfa = new DepthFirstAnalysis(graph); } + public void setUpGraph2() { + Graph graph = new Graph(); + Integer node1 = new Integer(5); + Integer node2 = new Integer(4); + Integer node3 = new Integer(3); + Integer node4 = new Integer(2); + Integer node5 = new Integer(1); + graph.addNode(node1); + graph.addNode(node2); + graph.addNode(node3); + graph.addNode(node4); + graph.addNode(node5); + graph.addEdge(new Edge(node1, node2, true)); + graph.addEdge(new Edge(node2, node3, true)); + graph.addEdge(new Edge(node3, node3, true)); + graph.addEdge(new Edge(node3, node4, true)); + graph.addEdge(new Edge(node4, node1, true)); + graph.addEdge(new Edge(node4, node2, true)); + graph.addEdge(new Edge(node5, node2, true)); + _dfa = new DepthFirstAnalysis(graph); + } + public void testNodeSorting() { Collection nodes = _dfa.getSortedNodes(); assertEquals(4, nodes.size()); - int time = Integer.MAX_VALUE; + int time = 0; Object node; for (Iterator itr = nodes.iterator(); itr.hasNext();) { node = itr.next(); - assertTrue(time >= _dfa.getFinishedTime(node)); + assertTrue(time <= _dfa.getFinishedTime(node)); time = _dfa.getFinishedTime(node); } } @@ -74,6 +101,51 @@ public class TestDepthFirstAnalysis || edge1.getTo() == edge1.getFrom()); } + public void testBackEdges() { + setUpGraph2(); + Collection edges = _dfa.getEdges(Edge.TYPE_BACK); + assertEquals(2, edges.size()); + Iterator itr = edges.iterator(); + Edge edge0 = (Edge) itr.next(); + Edge edge1 = (Edge) itr.next(); + if (edge0.getTo() == edge0.getFrom()) { + assertTrue(edge0.getCycle() != null && edge0.getCycle().size() == 1); + List cycle = edge1.getCycle(); + assertTrue(cycle != null && cycle.size() == 4); + assertTrue(((Edge)cycle.get(0)).getFrom() == ((Edge)cycle.get(3)).getTo()); + } else if (edge1.getTo() == edge1.getFrom()) { + assertTrue(edge1.getCycle() != null && edge1.getCycle().size() == 1); + assertTrue(edge1 == edge1.getCycle()); + List cycle = edge0.getCycle(); + assertTrue(cycle != null && cycle.size() == 4); + assertTrue(((Edge)cycle.get(0)).getFrom() == ((Edge)cycle.get(3)).getTo()); + } else { + // should not happen + assertFalse(true); + } + } + + public void testForwardEdges() { + setUpGraph2(); + Collection edges = _dfa.getEdges(Edge.TYPE_FORWARD); + assertEquals(2, edges.size()); + Iterator itr = edges.iterator(); + Edge edge0 = (Edge) itr.next(); + Edge edge1 = (Edge) itr.next(); + if (edge0.getCycle() == null) { + List cycle = edge1.getCycle(); + assertTrue(cycle != null && cycle.size() == 3); + assertTrue(((Edge)cycle.get(0)).getFrom() == ((Edge)cycle.get(2)).getTo()); + } else if (edge1.getCycle() == null) { + List cycle = edge0.getCycle(); + assertTrue(cycle != null && cycle.size() == 3); + assertTrue(((Edge)cycle.get(0)).getFrom() == ((Edge)cycle.get(2)).getTo()); + } else { + // should not happen + assertFalse(true); + } + } + public static void main(String[] args) { main(TestDepthFirstAnalysis.class); } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityB.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityB.java index c978a0278..a4087f2b0 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityB.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityB.java @@ -40,7 +40,7 @@ public class EntityB { private String name; - @OneToOne(cascade = CascadeType.ALL) + @OneToOne(cascade = CascadeType.ALL, optional = false) @JoinColumn(name = "entityc_id", referencedColumnName = "entityc_id", nullable = false) @ForeignKey diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityC.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityC.java index 3c1a9cfbe..03c85b4f3 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityC.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityC.java @@ -41,7 +41,7 @@ public class EntityC { private String name; - @OneToOne(cascade = CascadeType.ALL) + @OneToOne(cascade = CascadeType.ALL, optional = false) @JoinColumn(name = "entityd_id", referencedColumnName = "entityd_id", nullable = false) @ForeignKey diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityD.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityD.java index 4f5c7e488..d321dd2e0 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityD.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityD.java @@ -18,12 +18,16 @@ */ package org.apache.openjpa.jdbc.kernel; +import org.apache.openjpa.persistence.jdbc.ForeignKey; + import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; import javax.persistence.Id; import javax.persistence.Version; +import javax.persistence.OneToOne; +import javax.persistence.JoinColumn; @Entity public class EntityD { @@ -35,6 +39,16 @@ public class EntityD { private String name; + @OneToOne + @JoinColumn(name = "entitya_id", referencedColumnName = "entitya_id") + @ForeignKey + private EntityA entityA; + + @OneToOne + @JoinColumn(name = "entityb_id", referencedColumnName = "entityb_id") + @ForeignKey + private EntityB entityB; + @Version private Integer optLock; @@ -49,6 +63,22 @@ public class EntityD { return id; } + public EntityA getEntityA() { + return this.entityA; + } + + public void setEntityA(EntityA entityA) { + this.entityA = entityA; + } + + public EntityB getEntityB() { + return entityB; + } + + public void setEntityB(EntityB entityB) { + this.entityB = entityB; + } + public String getName() { return this.name; } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityE.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityE.java new file mode 100644 index 000000000..df5b83026 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/EntityE.java @@ -0,0 +1,76 @@ +/* + * 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.jdbc.kernel; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.OneToOne; +import javax.persistence.Version; + +import org.apache.openjpa.persistence.jdbc.ForeignKey; + +@Entity +public class EntityE { + + @Id + @Column(name = "entitye_id", nullable = false) + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Integer id; + + private String name; + + @OneToOne + @JoinColumn(name = "entityb_id", referencedColumnName = "entityb_id") + @ForeignKey + private EntityB entityB; + + @Version + private Integer optLock; + + public EntityE() { + } + + public void setId(Integer id) { + this.id = id; + } + + public Integer getId() { + return id; + } + + public EntityB getEntityB() { + return this.entityB; + } + + public void setEntityB(EntityB entityB) { + this.entityB = entityB; + } + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestNoForeignKeyViolation.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestNoForeignKeyViolation.java index 7d2a5574a..956e569e5 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestNoForeignKeyViolation.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestNoForeignKeyViolation.java @@ -34,15 +34,20 @@ public class TestNoForeignKeyViolation private EntityA entityA; private EntityB entityB; private EntityC entityC; + private EntityD entityD; public void setUp() { - setUp(EntityA.class, EntityB.class, EntityC.class, EntityD.class); + setUp(EntityA.class, EntityB.class, EntityC.class, EntityD.class, EntityE.class); + createTestData(); + } + + private void createTestData() { entityA = new EntityA(); - entityC = new EntityC(); - EntityD entityD = new EntityD(); - entityA.setName("entityA"); entityB = new EntityB(); + entityC = new EntityC(); + entityD = new EntityD(); + entityA.setName("entityA"); entityB.setName("entityB"); entityC.setName("entityC"); entityD.setName("entityD"); @@ -52,7 +57,6 @@ public class TestNoForeignKeyViolation } public void testSqlOrder() { - EntityManager em = emf.createEntityManager(); try { em.getTransaction().begin(); @@ -70,7 +74,7 @@ public class TestNoForeignKeyViolation EntityC newEntityC = new EntityC(); newEntityC.setName("newEntityC"); newEntityD = new EntityD(); - newEntityD.setName("newEntityD"); + newEntityD.setName("newNewEntityD"); newEntityC.setEntityD(newEntityD); entityB.setEntityC(newEntityC); @@ -84,4 +88,70 @@ public class TestNoForeignKeyViolation em.close(); } } + + public void testSimpleCycle() { + EntityManager em = emf.createEntityManager(); + try { + em.getTransaction().begin(); + entityD.setEntityA(entityA); + em.persist(entityA); + em.getTransaction().commit(); + } + finally { + if (em.getTransaction().isActive()) + em.getTransaction().rollback(); + em.close(); + } + } + + public void testComplexCycle() { + EntityManager em = emf.createEntityManager(); + try { + EntityE entityE = new EntityE(); + entityE.setName("entityE"); + entityE.setEntityB(entityB); + + em.getTransaction().begin(); + em.persist(entityE); + entityD.setEntityA(entityA); + em.persist(entityA); + em.getTransaction().commit(); + + em.getTransaction().begin(); + em.remove(entityE); + em.remove(entityA); + em.getTransaction().commit(); + } + finally { + if (em.getTransaction().isActive()) + em.getTransaction().rollback(); + em.close(); + } + } + + public void testComplexTwoCycles() { + EntityManager em = emf.createEntityManager(); + try { + EntityE entityE = new EntityE(); + entityE.setName("entityE"); + entityE.setEntityB(entityB); + + em.getTransaction().begin(); + em.persist(entityE); + entityD.setEntityA(entityA); + entityD.setEntityB(entityB); + em.persist(entityA); + em.getTransaction().commit(); + + em.getTransaction().begin(); + em.remove(entityE); + em.remove(entityA); + em.getTransaction().commit(); + } + finally { + if (em.getTransaction().isActive()) + em.getTransaction().rollback(); + em.close(); + } + } }