From 1f1d429752fd3ace6bc62f1aabea83e56e5818e4 Mon Sep 17 00:00:00 2001 From: KimmingLau <294001791@qq.com> Date: Wed, 28 Feb 2024 11:50:59 +0800 Subject: Improved fix for #285 1. Write SAME_BYTES to cacheMap when woven bytes are null 2. Fix TODO in SimpleCache::getAndInitialize, using Optional to help indicate cache hit for unwoven class 3. Improve test coverage (cache miss, cache hit for unwoven class) Relates to #285. Co-authored-by: Alexander Kriegisch Signed-off-by: KimmingLau <294001791@qq.com> Signed-off-by: Alexander Kriegisch --- .../aspectj/weaver/tools/cache/SimpleCache.java | 17 ++-- .../weaver/tools/cache/SimpleClassCacheTest.java | 95 ++++++++++++++-------- 2 files changed, 67 insertions(+), 45 deletions(-) (limited to 'weaver') diff --git a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java b/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java index 31102c96f..d94e9ce08 100644 --- a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java +++ b/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java @@ -15,6 +15,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.zip.CRC32; import org.aspectj.weaver.Dump; @@ -37,7 +38,7 @@ import org.aspectj.weaver.tools.TraceFactory; public class SimpleCache { private static final String SAME_BYTES_STRING = "IDEM"; - private static final byte[] SAME_BYTES = SAME_BYTES_STRING.getBytes(); + static final byte[] SAME_BYTES = SAME_BYTES_STRING.getBytes(); private Map cacheMap; private boolean enabled = false; @@ -64,7 +65,7 @@ public class SimpleCache { } } - public byte[] getAndInitialize(String classname, byte[] bytes, + public Optional getAndInitialize(String classname, byte[] bytes, ClassLoader loader, ProtectionDomain protectionDomain) { if (!enabled) { return null; @@ -72,14 +73,12 @@ public class SimpleCache { byte[] res = get(classname, bytes); if (Arrays.equals(SAME_BYTES, res)) { - return null; - } else { - if (res != null) { - initializeClass(classname, res, loader, protectionDomain); - } - return res; + return Optional.empty(); + } else if (res != null) { + initializeClass(classname, res, loader, protectionDomain); + return Optional.of(res); } - + return null; } private byte[] get(String classname, byte bytes[]) { diff --git a/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleClassCacheTest.java b/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleClassCacheTest.java index c8cbc8245..e74b634a6 100644 --- a/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleClassCacheTest.java +++ b/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleClassCacheTest.java @@ -12,60 +12,83 @@ package org.aspectj.weaver.tools.cache; -import java.io.File; - import junit.framework.TestCase; -/** - */ -public class SimpleClassCacheTest extends TestCase { - byte[] FAKE_BYTES_V1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - byte[] FAKE_BYTES_V2 = {1, 1, 2, 3, 4, 5, 6, 7, 8, 9}; +import java.io.File; - byte[] FAKE_WOVEN_BYTES_V1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9,10}; - byte[] FAKE_WOVEN_BYTES_V2 = {1, 1, 2, 3, 4, 5, 6, 7, 8, 9,10}; +public class SimpleClassCacheTest extends TestCase { + byte[] FAKE_BYTES_V1 = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; + byte[] FAKE_BYTES_V2 = { 1, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; + byte[] FAKE_WOVEN_BYTES_V1 = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; + byte[] FAKE_WOVEN_BYTES_V2 = { 1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; - private SimpleCache createCache() throws Exception { - return new SimpleCache(System.getProperty("java.io.tmpdir"),true); + private SimpleCache createCache() { + return new SimpleCache(System.getProperty("java.io.tmpdir"), true); } - - public void testCache() throws Exception { + public void testCache() { String classA = "com.generated.A"; SimpleCache cache = createCache(); - cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1); + // Returned woven bytes are the original ones + byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null); + assertNotNull(result); + for (int i = 0; i < result.length; i++) + assertEquals( + "Cached version byte[" + i + "] should be equal to the original woven class", + result[i], FAKE_WOVEN_BYTES_V1[i] + ); + + // Class is properly backed up + File f = new File(System.getProperty("java.io.tmpdir") + File.separator + "com.generated.A-1164760902"); + assertTrue( + "Class should be backed up with CRC 1164760902", + f.exists() + ); + } + + public void testDifferentVersionCache() { + String classA = "com.generated.A"; + SimpleCache cache = createCache(); + cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1); + cache.put(classA, FAKE_BYTES_V2, FAKE_WOVEN_BYTES_V2); - // Test the returned woven bytes are the original one - byte result[] = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null); - for(int i = 0; i < result.length; i ++){ - assertEquals("Cached version byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V1[i]); - } + // Returned woven bytes are the original ones for v1 + byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null); + assertNotNull(result); + for (int i = 0; i < result.length; i++) + assertEquals( + "Cached version v1 byte[" + i + "] should be equal to the original woven class", + result[i], FAKE_WOVEN_BYTES_V1[i] + ); + + // Returned woven bytes are the original ones for v2 + result = cache.getAndInitialize(classA, FAKE_BYTES_V2, null, null).orElse(null); + assertNotNull(result); + for (int i = 0; i < result.length; i++) + assertEquals( + "Cached version v2 byte[" + i + "] should be equal to the original woven class", + result[i], FAKE_WOVEN_BYTES_V2[i] + ); + } - // Assure the class is properly backed up in the backing folder - File f = new File (System.getProperty("java.io.tmpdir") + File.separator + "com.generated.A-1164760902"); - assertTrue("Class should be backed up to backing folder, with te CRC:1164760902 ",f.exists()); + public void testCacheMiss() { + String classA = "com.generated.A"; + SimpleCache cache = createCache(); + // Woven bytes not found in cache + assertNull(cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null)); } - public void testDifferentVersionCache() throws Exception { + public void testCacheHitUnwoven() { String classA = "com.generated.A"; SimpleCache cache = createCache(); - cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1); - cache.put(classA, FAKE_BYTES_V2, FAKE_WOVEN_BYTES_V2); + cache.put(classA, FAKE_BYTES_V1, SimpleCache.SAME_BYTES); - // Test the returned woven bytes are the original one for v1 - byte result[] = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null); - for(int i = 0; i < result.length; i ++){ - assertEquals("Cached version v1 byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V1[i]); - } - - // Test the returned woven bytes are the original one for v2 - result = cache.getAndInitialize(classA, FAKE_BYTES_V2, null, null); - for(int i = 0; i < result.length; i ++){ - assertEquals("Cached version v2 byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V2[i]); - } + // Returned woven bytes are null, indicating an unwoven class + byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null); + assertNull(result); } } -- cgit v1.2.3