From 853fb319ba81e2ad589ac6a7ce47b45de0a2749d Mon Sep 17 00:00:00 2001 From: Patrick Linskey Date: Sat, 10 Mar 2007 17:15:49 +0000 Subject: [PATCH] OPENJPA-35: fixed bulk update / bulk delete logic to properly clear out the data cache as well as the query cache. We could probably change the logic to remove the query cache mutations, since the data cache clear should automatically clear out the query cache as needed. Also changed the test framework a bit to allow for easier test harness creation without using SingleEMTest, which required providing access to the open brokers from AbstractBrokerFactory. git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@516750 13f79535-47bb-0310-9956-ffa450edef68 --- .../datacache/QueryCacheStoreQuery.java | 23 ++-- .../openjpa/kernel/AbstractBrokerFactory.java | 8 ++ .../datacache/TestBulkJPQLAndDataCache.java | 91 +++++++++++++++ .../persistence/test/SingleEMFTest.java | 106 ++++++++++++++++++ .../persistence/test/SingleEMTest.java | 79 ++----------- 5 files changed, 228 insertions(+), 79 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkJPQLAndDataCache.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMFTest.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java b/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java index ccf0699ac..908d461aa 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java @@ -19,7 +19,6 @@ import java.io.ObjectStreamException; import java.io.Serializable; import java.util.AbstractList; import java.util.ArrayList; -import java.util.Arrays; import java.util.BitSet; import java.util.Collections; import java.util.Date; @@ -112,7 +111,7 @@ public class QueryCacheStoreQuery if (fetch.getReadLockLevel() > LockLevels.LOCK_NONE) return null; - // get the cached oids + // get the cached data QueryResult res = _cache.get(qk); if (res == null) return null; @@ -123,7 +122,7 @@ public class QueryCacheStoreQuery if (projs == 0) { // make sure the data cache contains the oids for the query result; // if it doesn't, then using the result could be slower than not - // using it becauseo of the individual by-oid lookups + // using it because of the individual by-oid lookups ClassMetaData meta = _repos.getMetaData(getContext(). getCandidateType(), _sctx.getClassLoader(), true); BitSet idxs = meta.getDataCache().containsAll(res); @@ -313,7 +312,7 @@ public class QueryCacheStoreQuery * (such as deletes or updates) are performed so that the * cache remains up-to-date. */ - private void clearAccesssPath(StoreQuery q) { + private void clearAccessPath(StoreQuery q) { if (q == null) return; @@ -321,20 +320,26 @@ public class QueryCacheStoreQuery if (cmd == null || cmd.length == 0) return; - Class[] classes = new Class[cmd.length]; + List classes = new ArrayList(cmd.length); for (int i = 0; i < cmd.length; i++) - classes[i] = cmd[i].getDescribedType(); + classes.add(cmd[i].getDescribedType()); + // evict from the query cache QueryCacheStoreQuery cq = (QueryCacheStoreQuery) q; cq.getCache().onTypesChanged(new TypesChangedEvent - (q.getContext(), Arrays.asList(classes))); + (q.getContext(), classes)); + + // evict from the data cache + for (int i = 0; i < cmd.length; i++) + cmd[i].getDataCache().removeAll( + cmd[i].getDescribedType(), true); } public Number executeDelete(StoreQuery q, Object[] params) { try { return _ex.executeDelete(unwrap(q), params); } finally { - clearAccesssPath(q); + clearAccessPath(q); } } @@ -342,7 +347,7 @@ public class QueryCacheStoreQuery try { return _ex.executeUpdate(unwrap(q), params); } finally { - clearAccesssPath(q); + clearAccessPath(q); } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java index ec058932c..d7f515d53 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java @@ -645,6 +645,14 @@ public abstract class AbstractBrokerFactory } } + /** + * Returns a set of all the open brokers associated with this factory. The + * returned set is unmodifiable, and may contain null references. + */ + public Collection getOpenBrokers() { + return Collections.unmodifiableCollection(_brokers); + } + /** * Simple synchronization listener to remove completed transactions * from our cache. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkJPQLAndDataCache.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkJPQLAndDataCache.java new file mode 100644 index 000000000..eb9ac2e86 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkJPQLAndDataCache.java @@ -0,0 +1,91 @@ +package org.apache.openjpa.persistence.datacache; + +import java.util.List; +import java.util.Map; + +import org.apache.openjpa.persistence.OpenJPAEntityManager; +import org.apache.openjpa.persistence.OpenJPAPersistence; +import org.apache.openjpa.persistence.simple.AllFieldTypes; +import org.apache.openjpa.persistence.test.SingleEMFTest; + +public class TestBulkJPQLAndDataCache + extends SingleEMFTest { + + private Object oid; + + public TestBulkJPQLAndDataCache() { + super(AllFieldTypes.class); + } + + @Override + protected boolean clearDatabaseInSetUp() { + return true; + } + + protected void setEMFProps(Map props) { + super.setEMFProps(props); + props.put("openjpa.DataCache", "true"); + props.put("openjpa.RemoteCommitProvider", "sjvm"); + } + + @Override + public void setUp() throws Exception { + super.setUp(); + + OpenJPAEntityManager em = + OpenJPAPersistence.cast(emf.createEntityManager()); + em.getTransaction().begin(); + AllFieldTypes pc = new AllFieldTypes(); + pc.setStringField("DeleteMe"); + em.persist(pc); + oid = em.getObjectId(pc); + em.getTransaction().commit(); + em.close(); + } + + public void testBulkDelete() { + OpenJPAEntityManager em = + OpenJPAPersistence.cast(emf.createEntityManager()); + + em.getTransaction().begin(); + List result = em.createQuery("SELECT o FROM AllFieldTypes o") + .getResultList(); + assertEquals(1, result.size()); + em.createQuery("DELETE FROM AllFieldTypes o").executeUpdate(); + em.getTransaction().commit(); + em.close(); + + em = OpenJPAPersistence.cast(emf.createEntityManager()); + + // this assumes that we invalidate the cache, rather than update it + // according to the bulk rule. + assertFalse(OpenJPAPersistence.cast(emf).getStoreCache() + .contains(AllFieldTypes.class, oid)); + + assertNull(em.find(AllFieldTypes.class, oid)); + em.close(); + } + + public void testBulkUpdate() { + OpenJPAEntityManager em = + OpenJPAPersistence.cast(emf.createEntityManager()); + + em.getTransaction().begin(); + List result = em.createQuery("SELECT o FROM AllFieldTypes o " + + "WHERE o.intField = 0").getResultList(); + assertEquals(1, result.size()); + em.createQuery("UPDATE AllFieldTypes o SET o.intField = 10") + .executeUpdate(); + em.getTransaction().commit(); + em.close(); + + em = OpenJPAPersistence.cast(emf.createEntityManager()); + + // this assumes that we invalidate the cache, rather than update it + // according to the bulk rule. + assertFalse(OpenJPAPersistence.cast(emf).getStoreCache() + .contains(AllFieldTypes.class, oid)); + + em.close(); + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMFTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMFTest.java new file mode 100644 index 000000000..030779736 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMFTest.java @@ -0,0 +1,106 @@ +package org.apache.openjpa.persistence.test; + +import java.util.Map; +import java.util.HashMap; +import java.util.Iterator; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Persistence; +import javax.persistence.EntityManager; + +import junit.framework.TestCase; +import org.apache.openjpa.persistence.OpenJPAEntityManagerFactory; +import org.apache.openjpa.persistence.OpenJPAPersistence; +import org.apache.openjpa.kernel.AbstractBrokerFactory; +import org.apache.openjpa.kernel.Broker; + +public abstract class SingleEMFTest extends TestCase { + + protected EntityManagerFactory emf; + protected Class[] classes; + + public SingleEMFTest(Class... classes) { + this.classes = classes; + } + + /** + * Can be overridden to return a list of classes that will be used + * for this test. + */ + protected Class[] getClasses() { + return classes; + } + + /** + * Modify the properties that are used to create the EntityManagerFactory. + * By default, this will set up the MetaDataFactory with the + * persistent classes for this test case. This method can be overridden + * to add more properties to the map. + */ + protected void setEMFProps(Map props) { + // if we have specified a list of persistent classes to examine, + // then set it in the MetaDataFactory so that our automatic + // schema generation will work. + Class[] pclasses = getClasses(); + if (pclasses != null) { + StringBuilder str = new StringBuilder(); + for (Class c : pclasses) + str.append(str.length() > 0 ? ";" : "").append(c.getName()); + + props.put("openjpa.MetaDataFactory", "jpa(Types=" + str + ")"); + } + + if (clearDatabaseInSetUp()) { + props.put("openjpa.jdbc.SynchronizeMappings", + "buildSchema(ForeignKeys=true," + + "SchemaAction='add,deleteTableContents')"); + } + } + + protected boolean clearDatabaseInSetUp() { + return false; + } + + public EntityManagerFactory emf() { + return emf; + } + + public boolean closeEMF() { + if (emf == null) + return false; + + if (!emf.isOpen()) + return false; + + for (Iterator iter = ((AbstractBrokerFactory) OpenJPAPersistence + .toBrokerFactory(emf)).getOpenBrokers().iterator(); + iter.hasNext(); ) { + Broker b = (Broker) iter.next(); + if (b != null && !b.isClosed()) { + EntityManager em = OpenJPAPersistence.toEntityManager(b); + if (em.getTransaction().isActive()) + em.getTransaction().rollback(); + em.close(); + } + } + + emf.close(); + return !emf.isOpen(); + } + + @Override + public void setUp() throws Exception { + super.setUp(); + Map props = new HashMap(System.getProperties()); + setEMFProps(props); + emf = Persistence.createEntityManagerFactory("test", props); + + if (clearDatabaseInSetUp()) // get an EM to trigger schema manipulations + emf.createEntityManager().close(); + } + + @Override + public void tearDown() throws Exception { + closeEMF(); + super.tearDown(); + } +} \ No newline at end of file diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java index 30c0dce7d..406d93a66 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java @@ -19,84 +19,31 @@ import java.util.*; import javax.persistence.*; -import junit.framework.TestCase; -import junit.textui.TestRunner; -import org.apache.openjpa.persistence.*; - /** * A base test case that can be used to easily test scenarios where there * is only a single EntityManager at any given time. * * @author Marc Prud'hommeaux */ -public abstract class SingleEMTest extends TestCase { +public abstract class SingleEMTest extends SingleEMFTest { - protected EntityManagerFactory emf; protected EntityManager em; - protected Class[] classes; public SingleEMTest(Class... classes) { - this.classes = classes; + super(classes); } - /** - * Can be overridden to return a list of classes that will be used - * for this test. - */ - protected Class[] getClasses() { - return classes; - } - - /** - * Modify the properties that are used to create the EntityManagerFactory. - * By default, this will set up the MetaDataFactory with the - * persistent classes for this test case. This method can be overridden - * to add more properties to the map. - */ - protected void setEMFProps(Map props) { - // if we have specified a list of persistent classes to examine, - // then set it in the MetaDataFactory so that our automatic - // schema generation will work. - Class[] pclasses = getClasses(); - if (pclasses != null) { - StringBuilder str = new StringBuilder(); - for (Class c : pclasses) - str.append(str.length() > 0 ? ";" : "").append(c.getName()); - - props.put("openjpa.MetaDataFactory", "jpa(Types=" + str + ")"); - } - - if (clearDatabaseInSetUp()) { - props.put("openjpa.jdbc.SynchronizeMappings", - "buildSchema(ForeignKeys=true," + - "SchemaAction='add,deleteTableContents')"); - } - } - - protected boolean clearDatabaseInSetUp() { - return false; - } - - public void setUp() throws Exception { - Map props = new HashMap(System.getProperties()); - setEMFProps(props); - emf = Persistence.createEntityManagerFactory("test", props); - } - - /** + /** * Rolls back the current transaction and closes the EntityManager. */ + @Override public void tearDown() throws Exception { rollback(); close(); - closeEMF(); + super.tearDown(); } - public EntityManagerFactory emf() { - return emf; - } - - /** + /** * Returns the current EntityManager, creating one from the * EntityManagerFactory if it doesn't already exist. */ @@ -164,17 +111,10 @@ public abstract class SingleEMTest extends TestCase { return !em.isOpen(); } + @Override public boolean closeEMF() { - if (emf == null) - return false; - close(); - - if (!emf.isOpen()) - return false; - - emf.close(); - return !emf.isOpen(); + return super.closeEMF(); } /** @@ -283,5 +223,4 @@ public abstract class SingleEMTest extends TestCase { return total; } -} - +} \ No newline at end of file