Author: bstansberry(a)jboss.com Date: 2008-10-13 10:02:29 -0400 (Mon, 13 Oct 2008) New Revision: 6909 Modified: core/branches/2.2.X/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java core/branches/2.2.X/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java core/branches/2.2.X/src/test/java/org/jboss/cache/loader/UnnecessaryLoadingTest.java core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/CacheLoaderTest.java core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/PassivationTest.java Log: [JBCACHE-1423] Load data from cache loader before doing put(Fqn, Map) Modified: core/branches/2.2.X/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java 2008-10-13 13:32:12 UTC (rev 6908) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java 2008-10-13 14:02:29 UTC (rev 6909) @@ -93,7 +93,7 @@ { if (command.getFqn() != null) { - loadIfNeeded(ctx, command.getFqn(), null, false, true, false, ctx.getTransactionEntry(), false, false, false); + loadIfNeeded(ctx, command.getFqn(), null, true, true, false, ctx.getTransactionEntry(), false, false, false); } return invokeNextInterceptor(ctx, command); } Modified: core/branches/2.2.X/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java =================================================================== --- core/branches/2.2.X/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2008-10-13 13:32:12 UTC (rev 6908) +++ core/branches/2.2.X/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2008-10-13 14:02:29 UTC (rev 6909) @@ -816,7 +816,27 @@ } + public void testPutDataMap() throws Exception + { + Fqn f = Fqn.fromString("/a"); + assert !cache.exists(f); + assert !loader.exists(f); + Map<Object, Object> input = new HashMap(); + input.put("one", "one"); + input.put("two", "two"); + cache.put(f, input); + cache.evict(f); + Object three = "three"; // avoid some weird generics complaint + cache.put(f, Collections.singletonMap(three, three)); + Map data = cache.getRoot().getChild(f).getData(); + assertEquals("incorrect # of entries", 3, data.size()); + assertEquals("Has key 'one", "one", data.get("one")); + assertEquals("Has key 'two", "two", data.get("two")); + assertEquals("Has key 'three", "three", data.get("three")); + + } + public void testRemoveData() { String key = "/x/y/z/"; Modified: core/branches/2.2.X/src/test/java/org/jboss/cache/loader/UnnecessaryLoadingTest.java =================================================================== --- core/branches/2.2.X/src/test/java/org/jboss/cache/loader/UnnecessaryLoadingTest.java 2008-10-13 13:32:12 UTC (rev 6908) +++ core/branches/2.2.X/src/test/java/org/jboss/cache/loader/UnnecessaryLoadingTest.java 2008-10-13 14:02:29 UTC (rev 6909) @@ -5,6 +5,7 @@ import org.jboss.cache.DefaultCacheFactory; import org.jboss.cache.Fqn; import org.jboss.cache.Node; +import org.jboss.cache.NodeSPI; import org.jboss.cache.config.CacheLoaderConfig; import static org.testng.AssertJUnit.*; import org.testng.annotations.AfterMethod; @@ -89,7 +90,8 @@ protected void assertDataNotLoaded(Fqn f) { - assertFalse("Data should not be loaded for node " + f, cache.peek(f, false).isDataLoaded()); + NodeSPI n = cache.peek(f, true); + assertFalse("Data should not be loaded for node " + f, n != null && n.isDataLoaded()); } @@ -185,14 +187,17 @@ replay(mockCacheLoader); cache.put(child, m); - assertDataNotLoaded(child); + // FIXME Changes for JBCACHE-1423 require changing a number of asserts + // below from "assertDataNotLoaded" to "assertDataLoaded". This test + // should be revisited to confirm it still tests what it was intended to + assertDataLoaded(child); // should load child data // mockCacheLoader.expects(once()).method("get").with(eq(child)); cache.get(child, "foo"); - assertDataNotLoaded(child); + assertDataLoaded(child); cache.get(child, "foo"); - assertDataNotLoaded(child); + assertDataLoaded(child); cache.get(child, "foo2"); // does not exist, will trigger a load assertDataLoaded(child); @@ -332,15 +337,19 @@ // now evict cache.evict(parent, false); + assertDataNotLoaded(parent); + // should not load cache.put(parent, m); + // FIXME Changes for JBCACHE-1423 require changing a number of asserts + // below from "assertDataNotLoaded" to "assertDataLoaded". This test + // should be revisited to confirm it still tests what it was intended to + assertDataLoaded(parent); - assertDataNotLoaded(parent); - // now a get for an existing key should not trigger a load! assertEquals("v2", cache.get(parent, "k2")); - assertDataNotLoaded(parent); + assertDataLoaded(parent); // but going a get for a nonexistent key should! assertEquals(v, cache.get(parent, k)); Modified: core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/CacheLoaderTest.java =================================================================== --- core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/CacheLoaderTest.java 2008-10-13 13:32:12 UTC (rev 6908) +++ core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/CacheLoaderTest.java 2008-10-13 14:02:29 UTC (rev 6909) @@ -30,7 +30,7 @@ assertNotNull("CacheStoreInterceptor not found.", store); // verify cache loader statistics for entries loaded into cache - int miss = 0; + int miss = 5; int load = 0; int stores = 5; assertEquals("CacheLoaderLoads count error: ", load, loader.getCacheLoaderLoads()); @@ -40,8 +40,7 @@ // now try retrieving a valid attribute and an invalid attribute assertNotNull("Retrieval error: expected to retrieve " + CAPITAL + " for " + AUSTRIA, cache.get(AUSTRIA, CAPITAL)); assertNull("Retrieval error: did not expect to retrieve " + AREA + " for " + AUSTRIA, cache.get(AUSTRIA, AREA)); - load++; // since we did a get on a non-existent key which triggered a load - + // verify statistics after retrieving entries - misses should still be same since nodes were already loaded assertEquals("CacheLoaderLoads count error: ", load, loader.getCacheLoaderLoads()); assertEquals("CacheLoaderMisses count error: ", miss, loader.getCacheLoaderMisses()); @@ -92,6 +91,7 @@ // add a new node - this should cause a store stores++; + miss++; cache.put(POLAND, new HashMap<String, Object>()); assertEquals("CacheLoaderLoads count error: ", load, loader.getCacheLoaderLoads()); assertEquals("CacheLoaderMisses count error: ", miss, loader.getCacheLoaderMisses()); @@ -100,7 +100,6 @@ // add two attributes - this should cause two stores // plus one load as we're doing put(k, v) stores += 2; - load++; cache.put(POLAND, CAPITAL, "Warsaw"); cache.put(POLAND, CURRENCY, "Zloty"); assertEquals("CacheLoaderLoads count error: ", load, loader.getCacheLoaderLoads()); Modified: core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/PassivationTest.java =================================================================== --- core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/PassivationTest.java 2008-10-13 13:32:12 UTC (rev 6908) +++ core/branches/2.2.X/src/test/java/org/jboss/cache/mgmt/PassivationTest.java 2008-10-13 14:02:29 UTC (rev 6909) @@ -37,7 +37,7 @@ assertNotNull("PassivationInterceptor not found.", pass); System.out.println("count of misses " + act.getCacheLoaderMisses()); - int miss = 0; + int miss = 5; int activations = 0; // was 5 in 1.3 (one per attribute) // now just Europe/Albania and Europe/Hungary were loaded