diff --git a/security/providers/common/src/main/java/io/helidon/security/providers/common/EvictableCacheImpl.java b/security/providers/common/src/main/java/io/helidon/security/providers/common/EvictableCacheImpl.java index 0db2477d5e9..e549a2006dd 100644 --- a/security/providers/common/src/main/java/io/helidon/security/providers/common/EvictableCacheImpl.java +++ b/security/providers/common/src/main/java/io/helidon/security/providers/common/EvictableCacheImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2021 Oracle and/or its affiliates. + * Copyright (c) 2018, 2023 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package io.helidon.security.providers.common; import java.util.Optional; @@ -29,15 +30,16 @@ * Default implementation of {@link EvictableCache}. */ class EvictableCacheImpl implements EvictableCache { + /** + * An implementation that does no caching. + */ + static final EvictableCache NO_CACHE = new EvictableCache<>() { }; + /** * Number of threads in the scheduled thread pool to evict records. */ private static final int EVICT_THREAD_COUNT = 1; private static final ScheduledThreadPoolExecutor EXECUTOR; - /** - * An implementation that does no caching. - */ - static final EvictableCache NO_CACHE = new EvictableCache() { }; static { ThreadFactory jf = new ThreadFactory() { @@ -79,7 +81,7 @@ public Thread newThread(Runnable r) { @Override public Optional remove(K key) { CacheRecord removed = cacheMap.remove(key); - if (null == removed) { + if (removed == null) { return Optional.empty(); } @@ -88,7 +90,10 @@ public Optional remove(K key) { @Override public Optional get(K key) { - return getRecord(key).flatMap(this::validate).map(CacheRecord::getValue); + return getRecord(key) + .flatMap(this::validate) + .map(this::accessed) + .map(CacheRecord::getValue); } @Override @@ -113,7 +118,7 @@ public void close() { void evict() { cacheMap.forEachKey(evictParallelismThreshold, key -> cacheMap.compute(key, (key1, cacheRecord) -> { - if ((null == cacheRecord) || evictor.apply(cacheRecord.getKey(), cacheRecord.getValue())) { + if ((cacheRecord == null) || evictor.apply(cacheRecord.getKey(), cacheRecord.getValue())) { return null; } else { if (cacheRecord.isValid(cacheTimeoutNanos, overallTimeoutNanos)) { @@ -125,6 +130,11 @@ void evict() { })); } + private CacheRecord accessed(CacheRecord record) { + record.accessed(); + return record; + } + private Optional> validate(CacheRecord record) { if (record.isValid(cacheTimeoutNanos, overallTimeoutNanos) && !evictor.apply(record.getKey(), record.getValue())) { return Optional.of(record); @@ -135,9 +145,8 @@ private Optional> validate(CacheRecord record) { private Optional doComputeValue(K key, Supplier> valueSupplier) { CacheRecord record = cacheMap.compute(key, (s, cacheRecord) -> { - if ((null != cacheRecord) && cacheRecord.isValid(cacheTimeoutNanos, overallTimeoutNanos)) { - cacheRecord.accessed(); - return cacheRecord; + if ((cacheRecord != null) && cacheRecord.isValid(cacheTimeoutNanos, overallTimeoutNanos)) { + return accessed(cacheRecord); } if (cacheMap.size() >= cacheMaxSize) { @@ -149,7 +158,7 @@ private Optional doComputeValue(K key, Supplier> valueSupplier) { .orElse(null); }); - if (null == record) { + if (record == null) { return Optional.empty(); } else { return Optional.of(record.value); diff --git a/security/providers/common/src/test/java/io/helidon/security/providers/common/EvictableCacheTest.java b/security/providers/common/src/test/java/io/helidon/security/providers/common/EvictableCacheTest.java index 03d4ee941cf..099e5c135df 100644 --- a/security/providers/common/src/test/java/io/helidon/security/providers/common/EvictableCacheTest.java +++ b/security/providers/common/src/test/java/io/helidon/security/providers/common/EvictableCacheTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2021 Oracle and/or its affiliates. + * Copyright (c) 2018, 2023 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -63,6 +63,27 @@ void testEviction() throws InterruptedException { cache.close(); } + @Test + void testGetUpdatesAccessed() throws InterruptedException { + EvictableCache cache = EvictableCache.builder() + .evictSchedule(100, 100, TimeUnit.MILLISECONDS) + .timeout(50, TimeUnit.MILLISECONDS) + .build(); + + assertThat(cache.computeValue("one", () -> Optional.of("1")), is(Optional.of("1"))); + + for (int i = 0; i < 10; i++) { + assertThat(cache.get("one"), is(Optional.of("1"))); + // this should amount to more than 200 millis, which would time out if the record is not marked as accessed + Thread.sleep(20); + } + // now let's make sure it times out + TimeUnit.MILLISECONDS.sleep(200); + assertThat(cache.get("one"), is(EMPTY)); + + cache.close(); + } + @Test void testOverallTimeout() throws InterruptedException { EvictableCache cache = EvictableCache.builder()