diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java b/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java index 5e802109c..f71add065 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java @@ -423,7 +423,7 @@ public class DataCacheStoreManager public Collection loadAll(Collection sms, PCState state, int load, FetchConfiguration fetch, Object edata) { - if (isLocking(fetch)) + if (load == StoreManager.FORCE_LOAD_REFRESH || isLocking(fetch)) return super.loadAll(sms, state, load, fetch, edata); Map unloaded = null; diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java index 2119b47f9..f99ab2423 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java @@ -2720,21 +2720,17 @@ public class BrokerImpl } public void refreshAll(Collection objs, OpCallbacks call) { - if (objs.isEmpty()) + if (objs == null || objs.isEmpty()) return; beginOperation(true); try { assertNontransactionalRead(); - for (Iterator itr = objs.iterator(); itr.hasNext();) + for (Iterator itr = objs.iterator(); itr.hasNext();) { gatherCascadeRefresh(itr.next(), call); - if (_operating.isEmpty()) - return; - if (_operating.size() == 1) - refreshInternal(_operating.iterator().next(), call); - else - refreshInternal(_operating, call); + } + refreshInternal(_operating, call); } finally { endOperation(); } @@ -2749,12 +2745,7 @@ public class BrokerImpl assertNontransactionalRead(); gatherCascadeRefresh(obj, call); - if (_operating.isEmpty()) - return; - if (_operating.size() == 1) - refreshInternal(_operating.iterator().next(), call); - else - refreshInternal(_operating, call); + refreshInternal(_operating, call); } finally { endOperation(); } @@ -2786,10 +2777,12 @@ public class BrokerImpl * cascade-refresh relations from the user-given instances. */ protected void refreshInternal(Collection objs, OpCallbacks call) { + if (objs == null || objs.isEmpty()) + return; List exceps = null; try { // collect instances that need a refresh - Collection load = null; + Collection load = new ArrayList(objs.size()); StateManagerImpl sm; Object obj; for (Iterator itr = objs.iterator(); itr.hasNext();) { @@ -2804,11 +2797,9 @@ public class BrokerImpl continue; if (sm != null) { - if (sm.isDetached()) + if (sm.isDetached()) { throw newDetachedException(obj, "refresh"); - else if (sm.beforeRefresh(true)) { - if (load == null) - load = new ArrayList(objs.size()); + } else if (sm.beforeRefresh(true)) { load.add(sm); } } else if (assertPersistenceCapable(obj).pcIsDetached() @@ -2820,7 +2811,7 @@ public class BrokerImpl } // refresh all - if (load != null) { + if (!load.isEmpty()) { Collection failed = _store.loadAll(load, null, StoreManager.FORCE_LOAD_REFRESH, _fc, null); if (failed != null && !failed.isEmpty()) @@ -2862,38 +2853,9 @@ public class BrokerImpl throwNestedExceptions(exceps, false); } - /** - * Optimization for single-object refresh. - */ - protected void refreshInternal(Object obj, OpCallbacks call) { - try { - StateManagerImpl sm = getStateManagerImpl(obj, true); - if ((processArgument(OpCallbacks.OP_REFRESH, obj, sm, call) - & OpCallbacks.ACT_RUN) == 0) - return; - - if (sm != null) { - if (sm.isDetached()) - throw newDetachedException(obj, "refresh"); - else if (sm.beforeRefresh(false)) { - sm.load(_fc, StateManagerImpl.LOAD_FGS, null, null, false); - sm.afterRefresh(); - } - fireLifecycleEvent(sm.getManagedInstance(), null, - sm.getMetaData(), LifecycleEvent.AFTER_REFRESH); - } else if (assertPersistenceCapable(obj).pcIsDetached() - == Boolean.TRUE) - throw newDetachedException(obj, "refresh"); - } catch (OpenJPAException ke) { - throw ke; - } catch (RuntimeException re) { - throw new GeneralException(re); - } - } - public void retrieveAll(Collection objs, boolean dfgOnly, OpCallbacks call) { - if (objs.isEmpty()) + if (objs == null || objs.isEmpty()) return; if (objs.size() == 1) { retrieve(objs.iterator().next(), dfgOnly, call); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheBehavesIdentical.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheBehavesIdentical.java index 6efa85f3b..ac530366d 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheBehavesIdentical.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheBehavesIdentical.java @@ -265,9 +265,28 @@ public class TestDataCacheBehavesIdentical extends AbstractTestCase { * @param lock lock to be used * @param makeDirtyBeforeRefresh flags if the instance be dirtied before * refresh() + * @param expected The expected marker i.e. where the state is refreshed + * from. This should be always MARKER_DATABASE. + * a) whether DataCache is active + * b) whether current Lock is stronger than NOLOCK + * c) whether the object to be refreshed is dirty + * + * The following truth table enumerates the possibilities + * + * Use Cache? Lock? Dirty? Target + * Y Y Y Database + * Y N Y Data Cache + * Y Y N Data Cache + * Y N N Data Cache + * + * N Y Y Database + * N N Y Database + * N Y N Object Cache + * N N N Object Cache + */ public void verifyRefresh(boolean useDataCache, LockModeType lock, - boolean makeDirtyBeforeRefresh) { + boolean makeDirtyBeforeRefresh, String expected) { OpenJPAEntityManagerFactorySPI emf = (useDataCache) ? emfWithDataCache : emfWithoutDataCache; @@ -303,7 +322,6 @@ public class TestDataCacheBehavesIdentical extends AbstractTestCase { } em.refresh(pc); - String expected = getExpectedMarker(useDataCache, lock, makeDirtyBeforeRefresh); assertEquals(expected, pc.getName()); em.getTransaction().commit(); } @@ -337,47 +355,48 @@ public class TestDataCacheBehavesIdentical extends AbstractTestCase { String getExpectedMarker(boolean useDataCache, LockModeType lock, boolean makeDirtyBeforeRefresh) { if (useDataCache) { - return (lock != null && makeDirtyBeforeRefresh) - ? MARKER_DATABASE : MARKER_DATACACHE; +// return (lock != null && makeDirtyBeforeRefresh) + return (lock != null) ? MARKER_DATABASE : MARKER_DATACACHE; } else { - return (makeDirtyBeforeRefresh) ? MARKER_DATABASE : MARKER_CACHE; +// return (makeDirtyBeforeRefresh) ? MARKER_DATABASE : MARKER_CACHE; + return MARKER_DATABASE; } } - public void testDirtyRefreshWithNoLockHitsDataCache() { - verifyRefresh(WITH_DATACACHE, NOLOCK, DIRTY); + public void testDirtyRefreshWithNoLockHitsDatabase() { + verifyRefresh(WITH_DATACACHE, NOLOCK, DIRTY, MARKER_DATABASE); } - public void testCleanRefreshWithNoLockHitsDataCache() { - verifyRefresh(WITH_DATACACHE, NOLOCK, !DIRTY); + public void testCleanRefreshWithNoLockHitsDatabase() { + verifyRefresh(WITH_DATACACHE, NOLOCK, !DIRTY, MARKER_DATABASE); } public void testDirtyRefreshWithReadLockHitsDatabase() { - verifyRefresh(WITH_DATACACHE, LockModeType.READ, DIRTY); + verifyRefresh(WITH_DATACACHE, LockModeType.READ, DIRTY, MARKER_DATABASE); } - public void testCleanRefreshWithReadLockHitsDataCache() { - verifyRefresh(WITH_DATACACHE, LockModeType.READ, !DIRTY); + public void testCleanRefreshWithReadLockHitsDatabase() { + verifyRefresh(WITH_DATACACHE, LockModeType.READ, !DIRTY, MARKER_DATABASE); } public void testDirtyRefreshWithWriteLockHitsDatabase() { - verifyRefresh(WITH_DATACACHE, LockModeType.WRITE, DIRTY); + verifyRefresh(WITH_DATACACHE, LockModeType.WRITE, DIRTY, MARKER_DATABASE); } public void testCleanRefreshWithWriteLockHitsDatabase() { - verifyRefresh(WITH_DATACACHE, LockModeType.WRITE, !DIRTY); + verifyRefresh(WITH_DATACACHE, LockModeType.WRITE, !DIRTY, MARKER_DATABASE); } public void testDirtyRefreshWithoutDataCacheAlwaysHitsDatabase() { - verifyRefresh(!WITH_DATACACHE, NOLOCK, DIRTY); - verifyRefresh(!WITH_DATACACHE, LockModeType.READ, DIRTY); - verifyRefresh(!WITH_DATACACHE, LockModeType.WRITE, DIRTY); + verifyRefresh(!WITH_DATACACHE, NOLOCK, DIRTY, MARKER_DATABASE); + verifyRefresh(!WITH_DATACACHE, LockModeType.READ, DIRTY, MARKER_DATABASE); + verifyRefresh(!WITH_DATACACHE, LockModeType.WRITE, DIRTY, MARKER_DATABASE); } - public void testCleanRefreshWithoutDataCacheNeverHitsDatabase() { - verifyRefresh(!WITH_DATACACHE, NOLOCK, !DIRTY); - verifyRefresh(!WITH_DATACACHE, LockModeType.READ, !DIRTY); - verifyRefresh(!WITH_DATACACHE, LockModeType.WRITE, !DIRTY); + public void testCleanRefreshWithoutDataCacheAlwaysHitsDatabase() { + verifyRefresh(!WITH_DATACACHE, NOLOCK, !DIRTY, MARKER_DATABASE); + verifyRefresh(!WITH_DATACACHE, LockModeType.READ, !DIRTY, MARKER_DATABASE); + verifyRefresh(!WITH_DATACACHE, LockModeType.WRITE, !DIRTY, MARKER_DATABASE); } /** @@ -434,9 +453,12 @@ public class TestDataCacheBehavesIdentical extends AbstractTestCase { } else { fail("expected EntityNotFoundException for PObject:" + oid); } - } catch (EntityNotFoundException ex) { - if (lock != null) { - // we are good + } catch (Exception ex) { + if (ex instanceof EntityNotFoundException || + ex instanceof org.apache.openjpa.persistence.EntityNotFoundException) { + if (lock != null) { + // we are good + } } } finally { em.getTransaction().rollback();