diff --git a/core/src/main/java/org/eclipsefoundation/core/service/impl/QuarkusCachingService.java b/core/src/main/java/org/eclipsefoundation/core/service/impl/QuarkusCachingService.java index 4229cbd2c5d4015ed4aa82fa95c1442a237df45f..c1f404078fbed308afbe9969efae6c55432a1656 100644 --- a/core/src/main/java/org/eclipsefoundation/core/service/impl/QuarkusCachingService.java +++ b/core/src/main/java/org/eclipsefoundation/core/service/impl/QuarkusCachingService.java @@ -53,7 +53,7 @@ import io.quarkus.cache.CaffeineCache; @ApplicationScoped public class QuarkusCachingService implements CachingService { private static final Logger LOGGER = LoggerFactory.getLogger(QuarkusCachingService.class); - + @ConfigProperty(name = MicroprofilePropertyNames.CACHE_TTL_MAX_SECONDS, defaultValue = "900") long ttlWrite; @@ -67,13 +67,8 @@ public class QuarkusCachingService implements CachingService { @Override public <T> Optional<T> get(String id, MultivaluedMap<String, String> params, Class<?> rawType, Callable<? extends T> callable) { Objects.requireNonNull(callable); - - ParameterizedCacheKey cacheKey = ParameterizedCacheKey - .builder() - .setClazz(rawType) - .setId(id) - .setParams(params != null ? params : new MultivaluedMapImpl<>()) - .build(); + // create the cache key for the entry, cloning the map for key integrity + ParameterizedCacheKey cacheKey = ParameterizedCacheKey.builder().setClazz(rawType).setId(id).setParams(cloneMap(params)).build(); LOGGER.debug("Retrieving cache value for '{}'", cacheKey); return Optional.ofNullable(get(cacheKey, callable)); } @@ -167,7 +162,7 @@ public class QuarkusCachingService implements CachingService { } @CacheResult(cacheName = "default") - public <T> T get(@CacheKey ParameterizedCacheKey cacheKey, Callable<? extends T> callable) { + <T> T get(@CacheKey ParameterizedCacheKey cacheKey, Callable<? extends T> callable) { // attempt to get the result for the cache entry T out = null; try { @@ -189,4 +184,18 @@ public class QuarkusCachingService implements CachingService { return System.currentTimeMillis() + getMaxAge(); } + /** + * To prevent modification of maps/cache key entries post retrieval, we should be referencing copies of the maps rather + * than the direct passed reference for safety. + * + * @return the copied map, or an empty map if null + */ + private MultivaluedMap<String, String> cloneMap(MultivaluedMap<String, String> paramMap) { + MultivaluedMap<String, String> out = new MultivaluedMapImpl<>(); + if (paramMap != null) { + paramMap.entrySet().forEach(e -> out.addAll(e.getKey(), e.getValue())); + } + return out; + } + } diff --git a/core/src/test/java/org/eclipsefoundation/core/service/impl/DefaultQuarkusCachingServiceTest.java b/core/src/test/java/org/eclipsefoundation/core/service/impl/DefaultQuarkusCachingServiceTest.java index 2caf9b8f1cf031ef3ce44d24132e1eda0efc57b9..222704d624ccfa12ddfca3165053a3cf8e8cb634 100644 --- a/core/src/test/java/org/eclipsefoundation/core/service/impl/DefaultQuarkusCachingServiceTest.java +++ b/core/src/test/java/org/eclipsefoundation/core/service/impl/DefaultQuarkusCachingServiceTest.java @@ -250,6 +250,86 @@ class DefaultQuarkusCachingServiceTest { "Unknown key should not have been returned by getCacheKey, but was returned"); } + /* + * fuzzyRemove(id, type) + */ + + @Test + void fuzzyRemove_success() { + // generate a random key + String id = UUID.randomUUID().toString(); + // create empty map for initial post + MultivaluedMap<String, String> params = new MultivaluedMapImpl<>(); + // check that we have no match for ID before proceeding + Assertions.assertFalse(lookupCacheKeyUsingMainCache(id, Optional.empty()), "Expected random UUID key to be empty, but found value"); + // request + put the data in cache and check it's there + svc.get(id, params, String.class, () -> "sample"); + Assertions + .assertTrue(lookupCacheKeyUsingMainCache(id, Optional.empty()), + "Expected new cache value to have a key present in cache key set with empty map"); + + // update the map and create another entry + params = new MultivaluedMapImpl<>(); + params.add(DefaultUrlParameterNames.IDS_PARAMETER_NAME, id); + Assertions + .assertFalse(lookupCacheKeyUsingMainCache(id, Optional.of(params)), + "Expected random UUID key to be empty, but found value"); + // request + put the data in cache and check it's there + svc.get(id, params, String.class, () -> "sample"); + Assertions + .assertTrue(lookupCacheKeyUsingMainCache(id, Optional.of(params)), + "Expected new cache value to have a key present in cache key set with empty map"); + + // do the fuzzy remove operation + svc.fuzzyRemove(id, String.class); + + // check that both of the values are gone + Assertions + .assertFalse(lookupCacheKeyUsingMainCache(id, Optional.empty()), + "Expected random UUID key to be empty after fuzzy remove, but found value"); + Assertions + .assertFalse(lookupCacheKeyUsingMainCache(id, Optional.of(params)), + "Expected random UUID key + params to be empty after fuzzy remove, but found value"); + } + + @Test + void fuzzyRemove_success_onlyRemovesForSameType() { + // generate a random key + String id = UUID.randomUUID().toString(); + // create empty map for initial post + MultivaluedMap<String, String> params = new MultivaluedMapImpl<>(); + // check that we have no match for ID before proceeding + Assertions + .assertFalse(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(String.class), cache), + "Expected random UUID key to be empty, but found value"); + // request + put the data in cache and check it's there + svc.get(id, params, String.class, () -> "sample"); + Assertions + .assertTrue(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(String.class), cache), + "Expected new cache value to have a key present in cache key set with empty map"); + + // create another entry with the same ID for a different type + Assertions + .assertFalse(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(Integer.class), cache), + "Expected random UUID key to be empty, but found value"); + // request + put the data in cache and check it's there + svc.get(id, params, Integer.class, () -> 1); + Assertions + .assertTrue(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(Integer.class), cache), + "Expected new cache value to have a key present in cache key set with empty map"); + + // do the fuzzy remove operation + svc.fuzzyRemove(id, String.class); + + // check that only the targeted type is removed + Assertions + .assertFalse(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(String.class), cache), + "Expected random UUID key to be empty after fuzzy remove, but found value"); + Assertions + .assertTrue(lookupCacheKeyUsingRawCache(id, Optional.of(params), Optional.of(Integer.class), cache), + "Expected random UUID key + params to be empty after fuzzy remove, but found value"); + } + /* * remove(key) tests */