diff --git a/server-spi/src/main/java/org/keycloak/storage/CacheableStorageProviderModel.java b/server-spi/src/main/java/org/keycloak/storage/CacheableStorageProviderModel.java index dd740f234c..3fb8384de3 100644 --- a/server-spi/src/main/java/org/keycloak/storage/CacheableStorageProviderModel.java +++ b/server-spi/src/main/java/org/keycloak/storage/CacheableStorageProviderModel.java @@ -197,11 +197,8 @@ public class CacheableStorageProviderModel extends PrioritizedComponentModel { invalidate = true; } } else if (policy == CacheableStorageProviderModel.CachePolicy.EVICT_DAILY) { - long dailyTimeout = dailyTimeout(getEvictionHour(), getEvictionMinute()); - dailyTimeout = dailyTimeout - (24 * 60 * 60 * 1000); - //String timeout = DateFormat.getDateTimeInstance(DateFormat.FULL, DateFormat.FULL).format(new Date(dailyTimeout)); - //String stamp = DateFormat.getDateTimeInstance(DateFormat.FULL, DateFormat.FULL).format(new Date(cached.getCacheTimestamp())); - if (cached.getCacheTimestamp() <= dailyTimeout) { + long dailyBoundary = dailyEvictionBoundary(getEvictionHour(), getEvictionMinute()); + if (cached.getCacheTimestamp() <= dailyBoundary) { invalidate = true; } } else if (policy == CacheableStorageProviderModel.CachePolicy.EVICT_WEEKLY) { @@ -231,7 +228,20 @@ public class CacheableStorageProviderModel extends PrioritizedComponentModel { int add = (24 * 60 * 60 * 1000); cal.add(Calendar.MILLISECOND, add); } else { - cal.add(Calendar.MILLISECOND, (int)(cal2.getTimeInMillis() - cal.getTimeInMillis())); + cal = cal2; + } + return cal.getTimeInMillis(); + } + + public static long dailyEvictionBoundary(int hour, int minute) { + Calendar cal = Calendar.getInstance(); + cal.setTimeInMillis(Time.currentTimeMillis()); + cal.set(Calendar.HOUR_OF_DAY, hour); + cal.set(Calendar.MINUTE, minute); + if (cal.getTimeInMillis() > Time.currentTimeMillis()) { + // if daily evict for today hasn't happened yet set boundary + // to yesterday's time of eviction + cal.add(Calendar.DAY_OF_YEAR, -1); } return cal.getTimeInMillis(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index 5b1a38f1d7..f762715fb0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -29,6 +29,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Calendar; import java.util.Collections; import java.util.List; import java.util.Map; @@ -386,6 +387,35 @@ public abstract class AbstractKeycloakTest { userResource.update(userRepresentation); } + /** + * Sets time of day by calculating time offset and using setTimeOffset() to set it. + * + * @param hour hour of day + * @param minute minute + * @param second second + */ + public void setTimeOfDay(int hour, int minute, int second) { + setTimeOfDay(hour, minute, second, 0); + } + + /** + * Sets time of day by calculating time offset and using setTimeOffset() to set it. + * + * @param hour hour of day + * @param minute minute + * @param second second + * @param addSeconds additional seconds to add to offset time + */ + public void setTimeOfDay(int hour, int minute, int second, int addSeconds) { + Calendar now = Calendar.getInstance(); + now.set(Calendar.HOUR_OF_DAY, hour); + now.set(Calendar.MINUTE, minute); + now.set(Calendar.SECOND, second); + int offset = (int) ((now.getTime().getTime() - System.currentTimeMillis()) / 1000); + + setTimeOffset(offset + addSeconds); + } + /** * Sets time offset in seconds that will be added to Time.currentTime() and Time.currentTimeMillis() both for client and server. * diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java index 138ddcde0a..54e611cf8e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java @@ -371,42 +371,160 @@ public class UserStorageTest extends AbstractAuthTest { .addPackages(true, "org.keycloak.testsuite"); } - @Test - public void testDailyEviction() { - ApiUtil.findUserByUsername(testRealmResource(), "thor"); - - // set eviction to 1 hour from now - Calendar eviction = Calendar.getInstance(); - eviction.add(Calendar.HOUR, 1); + private void setDailyEvictionTime(int hour, int minutes) { + if (hour < 0 || hour > 23) { + throw new IllegalArgumentException("hour == " + hour); + } + if (minutes < 0 || minutes > 59) { + throw new IllegalArgumentException("minutes == " + minutes); + } ComponentRepresentation propProviderRW = testRealmResource().components().component(propProviderRWId).toRepresentation(); propProviderRW.getConfig().putSingle(CACHE_POLICY, CachePolicy.EVICT_DAILY.name()); - propProviderRW.getConfig().putSingle(EVICTION_HOUR, Integer.toString(eviction.get(HOUR_OF_DAY))); - propProviderRW.getConfig().putSingle(EVICTION_MINUTE, Integer.toString(eviction.get(MINUTE))); + propProviderRW.getConfig().putSingle(EVICTION_HOUR, String.valueOf(hour)); + propProviderRW.getConfig().putSingle(EVICTION_MINUTE, String.valueOf(minutes)); testRealmResource().components().component(propProviderRWId).update(propProviderRW); + } - // now + + /** + * Test daily eviction behaviour + */ + @Test + public void testDailyEviction() { + + // We need to test both cases: eviction the same day, and eviction the next day + // Simplest is to take full control of the clock + + // set clock to 23:30 of current day + setTimeOfDay(23, 30, 0); + + // test same day eviction behaviour + // set eviction at 23:45 + setDailyEvictionTime(23, 45); + + // there are users in cache already from before-test import + // and they didn't use any time offset clock so they may have timestamps in the 'future' + + // let's clear cache testingClient.server().run(session -> { - RealmModel realm = session.realms().getRealmByName("test"); - UserModel user = session.users().getUserByUsername("thor", realm); + session.userCache().clear(); }); - // run twice to make sure its in cache. - testingClient.server().run(session -> { - RealmModel realm = session.realms().getRealmByName("test"); - UserModel user = session.users().getUserByUsername("thor", realm); - System.out.println("User class: " + user.getClass()); - Assert.assertTrue(user instanceof CachedUserModel); // should still be cached - }); - - setTimeOffset(2 * 60 * 60); // 2 hours in future testingClient.server().run(session -> { RealmModel realm = session.realms().getRealmByName("test"); UserModel user = session.users().getUserByUsername("thor", realm); - System.out.println("User class: " + user.getClass()); - Assert.assertFalse(user instanceof CachedUserModel); // should be evicted + Assert.assertTrue(user instanceof CachedUserModel); // should be newly cached }); + + setTimeOfDay(23, 40, 0); + + // lookup user again - make sure it's returned from cache + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache + }); + + + setTimeOfDay(23, 50, 0); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertFalse(user instanceof CachedUserModel); // should have been invalidated + }); + + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should have been newly cached + }); + + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache + }); + + + setTimeOfDay(23, 55, 0); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache + }); + + + // at 00:30 + // it's next day now. the daily eviction time is now in the future + setTimeOfDay(0, 30, 0, 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache - it's still good for almost the whole day + }); + + + // at 23:30 next day + setTimeOfDay(23, 30, 0, 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache - it's still good until 23:45 + }); + + // at 23:50 + setTimeOfDay(23, 50, 0, 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertFalse(user instanceof CachedUserModel); // should be invalidated + }); + + setTimeOfDay(23, 55, 0, 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be newly cached + }); + + + setTimeOfDay(23, 40, 0, 2 * 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache + }); + + setTimeOfDay(23, 50, 0, 2 * 24 * 60 * 60); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertFalse(user instanceof CachedUserModel); // should be invalidated + }); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be newly cached + }); + + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("thor", realm); + Assert.assertTrue(user instanceof CachedUserModel); // should be returned from cache + }); } @Test