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 70e0270b8..128c256e0 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 @@ -58,7 +58,7 @@ public class ConstraintUpdateManager (ConstraintUpdateManager.class); public boolean orderDirty() { - return false; + return true; } protected PreparedStatementManager newPreparedStatementManager diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/RowManagerImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/RowManagerImpl.java index f05ce4aec..47a0b5136 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/RowManagerImpl.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/RowManagerImpl.java @@ -22,7 +22,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -39,17 +39,17 @@ import org.apache.openjpa.util.InternalException; public class RowManagerImpl implements RowManager { - private Map _inserts = null; - private Map _updates = null; - private Map _deletes = null; - private Collection _secondaryUpdates = null; - private Collection _secondaryDeletes = null; - private Collection _allRowUpdates = null; - private Collection _allRowDeletes = null; + private Map _inserts = null; + private Map _updates = null; + private Map _deletes = null; + private Collection _secondaryUpdates = null; + private Collection _secondaryDeletes = null; + private Collection _allRowUpdates = null; + private Collection _allRowDeletes = null; // we maintain a list of the order of all primary rows if the user // wants to be able to fetch them in order - private final List _primaryOrder; + private final List _primaryOrder; // track whether we're dealing with any auto-inc columns private boolean _auto = false; @@ -66,7 +66,7 @@ public class RowManagerImpl * @param order whether to keep track of the order in which rows are added */ public RowManagerImpl(boolean order) { - _primaryOrder = (order) ? new ArrayList() : null; + _primaryOrder = (order) ? new ArrayList() : null; } /** @@ -80,61 +80,98 @@ public class RowManagerImpl * Return the ordered primary rows. Only available if ordering requested * on construction. */ - public List getOrdered() { - return (_primaryOrder == null) ? Collections.EMPTY_LIST : _primaryOrder; + public List getOrdered() { + if(_primaryOrder == null ) { + return Collections.emptyList(); + } + else { + return _primaryOrder; + } } /** * Return all inserted primary rows. */ - public Collection getInserts() { - return (_inserts == null) ? Collections.EMPTY_LIST : _inserts.values(); + public Collection getInserts() { + if(_inserts == null ) { + return Collections.emptyList(); + } + else { + return _inserts.values(); + } } /** * Return all updated primary rows. */ - public Collection getUpdates() { - return (_updates == null) ? Collections.EMPTY_LIST : _updates.values(); + public Collection getUpdates() { + if(_updates == null ){ + return Collections.emptyList(); + } + else { + return _updates.values(); + } } /** * Return all deleted primary rows. */ - public Collection getDeletes() { - return (_deletes == null) ? Collections.EMPTY_LIST : _deletes.values(); + public Collection getDeletes() { + if(_deletes == null) { + return Collections.emptyList(); + } + else { + return _deletes.values(); + } } /** * Return all inserted and updated secondary rows. */ - public Collection getSecondaryUpdates() { - return (_secondaryUpdates == null) ? Collections.EMPTY_LIST - : _secondaryUpdates; + public Collection getSecondaryUpdates() { + if(_secondaryUpdates == null) { + return Collections.emptyList(); + } + else { + return _secondaryUpdates; + } } /** * Return all deleted secondary rows. */ - public Collection getSecondaryDeletes() { - return (_secondaryDeletes == null) ? Collections.EMPTY_LIST - : _secondaryDeletes; + public Collection getSecondaryDeletes() { + if(_secondaryDeletes == null) { + return Collections.emptyList(); + } + else { + return _secondaryDeletes; + } } /** * Return any 'all row' updates. */ - public Collection getAllRowUpdates() { - return (_allRowUpdates == null) ? Collections.EMPTY_LIST - : _allRowUpdates; + public Collection getAllRowUpdates() { + if(_allRowUpdates == null) { + return Collections.emptyList(); + } + else { + return _allRowUpdates; + } } /** * Return any 'all row' deletes. */ - public Collection getAllRowDeletes() { - return (_allRowDeletes == null) ? Collections.EMPTY_LIST - : _allRowDeletes; + public Collection getAllRowDeletes() { + if(_allRowDeletes == null) { + return Collections.emptyList(); + } + else { + return _allRowDeletes; + } + } public Row getSecondaryRow(Table table, int action) { @@ -149,12 +186,12 @@ public class RowManagerImpl SecondaryRow srow = (SecondaryRow) row; if (srow.getAction() == Row.ACTION_DELETE) { if (_secondaryDeletes == null) - _secondaryDeletes = new ArrayList(); - _secondaryDeletes.add(srow.clone()); + _secondaryDeletes = new ArrayList(); + _secondaryDeletes.add((SecondaryRow) srow.clone()); } else { if (_secondaryUpdates == null) - _secondaryUpdates = new ArrayList(); - _secondaryUpdates.add(srow.clone()); + _secondaryUpdates = new ArrayList(); + _secondaryUpdates.add((SecondaryRow) srow.clone()); } } @@ -169,12 +206,12 @@ public class RowManagerImpl switch (row.getAction()) { case Row.ACTION_UPDATE: if (_allRowUpdates == null) - _allRowUpdates = new ArrayList(); + _allRowUpdates = new ArrayList(); _allRowUpdates.add(row); break; case Row.ACTION_DELETE: if (_allRowDeletes == null) - _allRowDeletes = new ArrayList(); + _allRowDeletes = new ArrayList(); _allRowDeletes.add(row); break; default: @@ -192,25 +229,25 @@ public class RowManagerImpl && _row != null && _row.getAction() == action) return _row; - Map map; + Map map; if (action == Row.ACTION_DELETE) { if (_deletes == null && create) - _deletes = new HashMap(); + _deletes = new LinkedHashMap(); map = _deletes; } else if (action == Row.ACTION_INSERT) { if (_inserts == null && create) - _inserts = new HashMap(); + _inserts = new LinkedHashMap(); map = _inserts; } else { if (_updates == null && create) - _updates = new HashMap(); + _updates = new LinkedHashMap(); map = _updates; } if (map == null) return null; _key = new Key(table, sm); - _row = (PrimaryRow) map.get(_key); + _row = map.get(_key); if (_row == null && create) { _row = new PrimaryRow(table, action, sm); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Employee.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Employee.java new file mode 100644 index 000000000..c2fd9b852 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Employee.java @@ -0,0 +1,83 @@ +/* + * 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 java.util.Collection; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.Table; +import javax.persistence.Version; + +@Entity +// Try not to collide with other Employee entities +@Table(name = "PER_JDBC_KERN_EMP") +public class Employee { + + @Id + private int id; + + @Version + private int version; + + private String firstName; + private String lastName; + + @OneToMany(mappedBy = "employee", cascade = { CascadeType.MERGE, + CascadeType.PERSIST }) + private Collection tasks; + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } + + public Collection getTasks() { + return tasks; + } + + public void setTasks(Collection tasks) { + this.tasks = tasks; + } + + public int getVersion() { + return version; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Story.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Story.java new file mode 100644 index 000000000..d2ed6bb9b --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Story.java @@ -0,0 +1,60 @@ +/* + * 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.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Table; +import javax.persistence.Version; + +@Entity +@Table(name = "PER_JDBC_KERN_STORY") // try not to collide +public class Story { + @Id + private int id; + + @Version + private int version; + + @ManyToOne(cascade = { CascadeType.MERGE, CascadeType.PERSIST }) + private Task task; + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public Task getTask() { + return task; + } + + public void setTask(Task task) { + this.task = task; + } + + public int getVersion() { + return version; + } + +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Task.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Task.java new file mode 100644 index 000000000..d3aef7c7e --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/Task.java @@ -0,0 +1,75 @@ +/* + * 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 java.util.Collection; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.Table; +import javax.persistence.Version; + +@Entity +@Table(name = "PER_JDBC_KERN_TASK") +// try not to collide +public class Task { + @Id + private int id; + + @Version + private int version; + + @OneToMany(mappedBy = "task", cascade = { CascadeType.MERGE, + CascadeType.PERSIST }) + private Collection stories; + + @ManyToOne(cascade = { CascadeType.MERGE, CascadeType.PERSIST }) + private Employee employee; + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public Collection getStories() { + return stories; + } + + public void setStories(Collection stories) { + this.stories = stories; + } + + public Employee getEmployee() { + return employee; + } + + public void setEmployee(Employee employee) { + this.employee = employee; + } + + public int getVersion() { + return version; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestInsertOrder.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestInsertOrder.java new file mode 100644 index 000000000..6e86aa208 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestInsertOrder.java @@ -0,0 +1,115 @@ +/* + * 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 java.util.ArrayList; +import java.util.Collection; + +import javax.persistence.EntityManager; + +import org.apache.openjpa.persistence.test.SQLListenerTestCase; + +/** + * Test that insert order is preserved when using the ConstraintUpdateManager + * for entities which are not annotated with ForeignKey constraints. + */ +public class TestInsertOrder extends SQLListenerTestCase { + private String empTableName; + private String taskTableName; + private String storyTableName; + + public void setUp() { + setUp(Employee.class, Task.class, Story.class); + empTableName = getMapping(Employee.class).getTable().getFullName(); + taskTableName = getMapping(Task.class).getTable().getFullName(); + storyTableName = getMapping(Story.class).getTable().getFullName(); + } + + /** + *

Persist an Employee entity and allow the cascade to insert the children. + * The inserts should be executed in this order, Employee, Task, Story. + *

+ * + *

+ * Originally this test would pass in some scenarios. I believe the order + * relied on the hashcode of the underlying entities. + *

+ */ + public void testCascadePersist() { + Employee e = newTree(10); + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist(e); + em.getTransaction().commit(); + em.close(); + + assertSQLOrder("INSERT INTO " + empTableName + ".*", "INSERT INTO " + + taskTableName + ".*", "INSERT INTO " + storyTableName + ".*"); + } + + /** + * Merge an Employee entity and allow the cascade to insert the children. + * The inserts should be executed in this order, Employee, Task, Story. + */ + public void testCascadeMerge() { + Employee e = newTree(11); + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.merge(e); + em.getTransaction().commit(); + em.close(); + + assertSQLOrder("INSERT INTO " + empTableName + ".*", "INSERT INTO " + + taskTableName + ".*", "INSERT INTO " + storyTableName + ".*"); + } + + + /** + * Helper to create a tree of entities + * + * @param id + * ID for the entities. + * @return an unmanaged Employee instance with the appropriate relationships + * set. + */ + private Employee newTree(int id) { + Employee e = new Employee(); + e.setId(id); + + Task t = new Task(); + t.setId(id); + + Story s = new Story(); + s.setId(id); + + Collection tasks = new ArrayList(); + tasks.add(t); + + Collection stories = new ArrayList(); + stories.add(s); + + e.setTasks(tasks); + t.setEmployee(e); + + t.setStories(stories); + s.setTask(t); + + return e; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java index 13d38d560..95998dad1 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java @@ -21,7 +21,6 @@ package org.apache.openjpa.persistence.test; import java.util.Arrays; import java.util.List; import java.util.ArrayList; -import java.util.Map; import org.apache.openjpa.lib.jdbc.AbstractJDBCListener; import org.apache.openjpa.lib.jdbc.JDBCEvent; @@ -34,7 +33,7 @@ import org.apache.openjpa.lib.jdbc.JDBCListener; */ public abstract class SQLListenerTestCase extends SingleEMFTestCase { - + private static String _nl = System.getProperty("line.separator"); protected List sql = new ArrayList(); protected int sqlCount; @@ -130,4 +129,28 @@ public abstract class SQLListenerTestCase } } } + + public void assertSQLOrder(String... expected) { + int hits = 0; + + for (String executedSQL : sql) { + if (executedSQL.matches(expected[hits])) { + hits++; + } + } + + if (hits != expected.length) { + StringBuilder sb = new StringBuilder(); + sb.append("Did not find SQL in expected order : ").append(_nl); + for (String s : expected) { + sb.append(s).append(_nl); + } + + sb.append("SQL Statements issued : "); + for (String s : sql) { + sb.append(s).append(_nl); + } + fail(sb.toString()); + } + } }