diff --git a/src/catalog/cache/src/main/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacade.java b/src/catalog/cache/src/main/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacade.java index aaf2c206f..44f379482 100644 --- a/src/catalog/cache/src/main/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacade.java +++ b/src/catalog/cache/src/main/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacade.java @@ -7,29 +7,13 @@ import lombok.NonNull; import lombok.extern.slf4j.Slf4j; -import org.geoserver.catalog.Info; import org.geoserver.catalog.WorkspaceInfo; -import org.geoserver.catalog.impl.ResolvingProxy; import org.geoserver.cloud.event.UpdateSequenceEvent; -import org.geoserver.cloud.event.catalog.CatalogInfoRemoved; -import org.geoserver.cloud.event.config.GeoServerInfoModified; -import org.geoserver.cloud.event.config.GeoServerInfoSet; -import org.geoserver.cloud.event.config.LoggingInfoModified; -import org.geoserver.cloud.event.config.LoggingInfoSet; -import org.geoserver.cloud.event.config.ServiceAdded; -import org.geoserver.cloud.event.config.ServiceModified; -import org.geoserver.cloud.event.config.ServiceRemoved; -import org.geoserver.cloud.event.config.SettingsAdded; -import org.geoserver.cloud.event.config.SettingsModified; -import org.geoserver.cloud.event.config.SettingsRemoved; -import org.geoserver.cloud.event.info.ConfigInfoType; -import org.geoserver.cloud.event.info.InfoEvent; import org.geoserver.config.GeoServerFacade; import org.geoserver.config.GeoServerInfo; import org.geoserver.config.LoggingInfo; import org.geoserver.config.ServiceInfo; import org.geoserver.config.SettingsInfo; -import org.geoserver.config.impl.SettingsInfoImpl; import org.geoserver.config.plugin.forwarding.ForwardingGeoServerFacade; import org.springframework.cache.Cache; import org.springframework.cache.Cache.ValueWrapper; @@ -37,11 +21,8 @@ import org.springframework.cache.annotation.CacheConfig; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; -import org.springframework.cache.annotation.Caching; import org.springframework.context.event.EventListener; -import java.util.Optional; - import javax.annotation.Nullable; /** */ @@ -70,165 +51,20 @@ public CachingGeoServerFacade(@NonNull GeoServerFacade facade, @NonNull Cache ca ////// Event handling /////// - /** Evicts the {@link GeoServerInfo global config} upon any update sequence modifying event. */ - @EventListener(classes = UpdateSequenceEvent.class) - void onUpdateSequenceEvent(UpdateSequenceEvent event) { - evictGlobal() - .ifPresent( - evicted -> - log.debug( - "evicted global config with updatesequence {} upon event carrying update sequence {}", - evicted.getUpdateSequence(), - event.getUpdateSequence())); - } - - @EventListener(classes = GeoServerInfoModified.class) - void onGeoServerInfoModifyEvent(GeoServerInfoModified event) { - evictConfigEntry(event); - } - - @EventListener(classes = LoggingInfoModified.class) - void onLoggingInfoModifyEvent(LoggingInfoModified event) { - evictConfigEntry(event); - } - - @EventListener(classes = ServiceAdded.class) - void onServiceInfoAddEvent(ServiceAdded event) { - evictConfigEntry(event); - } - - @EventListener(classes = ServiceModified.class) - void onServiceInfoModifyEvent(ServiceModified event) { - evictConfigEntry(event); - } - - @EventListener(classes = ServiceRemoved.class) - void onServiceInfoRemoveEvent(ServiceRemoved event) { - evictConfigEntry(event); - } - - @EventListener(classes = SettingsAdded.class) - void onSettingsInfoAddEvent(SettingsAdded event) { - evictConfigEntry(event); - } - - @EventListener(classes = SettingsModified.class) - void onSettingsInfoModifyEvent(SettingsModified event) { - evictConfigEntry(event); - } - - @EventListener(classes = SettingsRemoved.class) - void onSettingsInfoRemoveEvent(SettingsRemoved event) { - evictConfigEntry(event); - } - - @EventListener(classes = GeoServerInfoSet.class) - void onGeoServerInfoSetEvent(GeoServerInfoSet event) { - evictConfigEntry(event); - } - - @EventListener(classes = LoggingInfoSet.class) - void onLoggingInfoSetEvent(LoggingInfoSet event) { - evictConfigEntry(event); - } - /** - * Can't know which {@link ServiceInfo} are attached to the workspace when its removed, so it - * invalidates all cache entries, possibly asynchronously, as per {@link Cache#clear()} + * Clears the whole config cache upon any {@link UpdateSequenceEvent}. * - *

As opposed to the other event handler methods, this one evicts upon both {@link - * InfoEvent#isRemote() remote} and {@link InfoEvent#isLocal() local} events. + *

{@link UpdateSequenceEvent} is the root event for the ones that change something in the + * catalog or the configuration. */ - @EventListener(classes = CatalogInfoRemoved.class) - void onWorkspaceRemoved(CatalogInfoRemoved event) { - if (event.getObjectType() == ConfigInfoType.WORKSPACE) { - cache.clear(); - } - } - - public void evictConfigEntry(InfoEvent event) { - if (!event.isRemote()) return; - - String objectId = event.getObjectId(); - ConfigInfoType infoType = event.getObjectType(); - Info info; - if (event instanceof SettingsModified se) { - info = mockSettings(se.getObjectId(), se.getWorkspaceId()); - } else if (event instanceof SettingsRemoved se) { - info = mockSettings(se.getObjectId(), se.getWorkspaceId()); - } else { - info = ResolvingProxy.create(objectId, infoType.getType()); - } - - if (evict(info)) { - log.debug("Evicted config cache entry due to {}", event); - } else { - log.trace("Remote event resulted in no cache eviction: {}", event); + @EventListener(classes = UpdateSequenceEvent.class) + void onUpdateSequenceEvent(UpdateSequenceEvent event) { + if (event.isRemote()) { + evictAll(); } } - private SettingsInfo mockSettings(@NonNull String objectId, String workspaceId) { - var s = new SettingsInfoImpl(); - s.setId(objectId); - s.setWorkspace(ResolvingProxy.create(workspaceId, WorkspaceInfo.class)); - return s; - } - ////// Cache manipulation functions /////// - - Optional evictGlobal() { - Optional ret = - Optional.ofNullable(cache.get(GEOSERVERINFO_KEY)) - .map(ValueWrapper::get) - .map(GeoServerInfo.class::cast); - cache.evict(GEOSERVERINFO_KEY); - return ret; - } - - boolean evict(Info info) { - log.debug("Evict cache entry for {}", info.getId()); - if (info instanceof GeoServerInfo) { - return evictGlobal().isPresent(); - } - if (info instanceof LoggingInfo) { - return cache.evictIfPresent(LOGGINGINFO_KEY); - } - if (info instanceof SettingsInfo settings) { - return evict(settings); - } - if (info instanceof ServiceInfo service) { - return evict(service); - } - return false; - } - - private boolean evict(SettingsInfo settings) { - String id = settings.getId(); - boolean evicted = cache.evictIfPresent(id); - WorkspaceInfo workspace = settings.getWorkspace(); - if (workspace != null) { - Object wsKey = CachingGeoServerFacade.settingsKey(workspace); - evicted |= cache.evictIfPresent(wsKey); - } - return evicted; - } - - private boolean evict(ServiceInfo service) { - Object idKey = CachingGeoServerFacade.serviceByIdKey(service.getId()); - ValueWrapper cachedValue = cache.get(idKey); - if (cachedValue == null) return false; - ServiceInfo cached = (ServiceInfo) cachedValue.get(); - cache.evict(idKey); - if (cached != null) { - WorkspaceInfo ws = cached.getWorkspace(); - Object nameKey = CachingGeoServerFacade.serviceByNameKey(ws, cached.getName()); - Object typeKey = CachingGeoServerFacade.serviceByTypeKey(ws, cached.getClass()); - cache.evict(nameKey); - cache.evict(typeKey); - } - return true; - } - T cachePutIncludeNull( @NonNull Object key, @NonNull Cache cache, T service) { @@ -263,13 +99,12 @@ public GeoServerInfo getGlobal() { } @Override - @CacheEvict(key = "'" + GEOSERVERINFO_KEY + "'", beforeInvocation = true) + @CacheEvict(allEntries = true) public void setGlobal(GeoServerInfo global) { super.setGlobal(global); } @Override - @CacheEvict(key = "'" + GEOSERVERINFO_KEY + "'", beforeInvocation = true) public void save(GeoServerInfo geoServer) { super.save(geoServer); } @@ -292,21 +127,19 @@ public SettingsInfo getSettings(WorkspaceInfo workspace) { } @Override - @Caching( - evict = { - @CacheEvict(key = "#settings.id", beforeInvocation = true), - @CacheEvict(key = "'settings@' + #settings.workspace.id", beforeInvocation = true) - }) + @CacheEvict(allEntries = true) + public void add(SettingsInfo settings) { + super.add(settings); + } + + @Override + @CacheEvict(allEntries = true) public void save(SettingsInfo settings) { super.save(settings); } @Override - @Caching( - evict = { - @CacheEvict(key = "#settings.id", beforeInvocation = true), - @CacheEvict(key = "'settings@' + #settings.workspace.id", beforeInvocation = true) - }) + @CacheEvict(allEntries = true) public void remove(SettingsInfo settings) { super.remove(settings); } @@ -318,26 +151,32 @@ public LoggingInfo getLogging() { } @Override - @CacheEvict(key = "'" + LOGGINGINFO_KEY + "'", beforeInvocation = true) + @CacheEvict(allEntries = true) public void setLogging(LoggingInfo logging) { super.setLogging(logging); } @Override - @CacheEvict(key = "'" + LOGGINGINFO_KEY + "'", beforeInvocation = true) + @CacheEvict(allEntries = true) public void save(LoggingInfo logging) { super.save(logging); } @Override + @CacheEvict(allEntries = true) + public void add(ServiceInfo service) { + super.add(service); + } + + @Override + @CacheEvict(allEntries = true) public void remove(ServiceInfo service) { - evict(service); super.remove(service); } @Override + @CacheEvict(allEntries = true) public void save(ServiceInfo service) { - evict(service); super.save(service); } diff --git a/src/catalog/cache/src/test/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacadeTest.java b/src/catalog/cache/src/test/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacadeTest.java index 9d162f0ac..86a5419b9 100644 --- a/src/catalog/cache/src/test/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacadeTest.java +++ b/src/catalog/cache/src/test/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacadeTest.java @@ -11,11 +11,9 @@ import static org.geoserver.cloud.event.info.ConfigInfoType.SERVICE; import static org.geoserver.cloud.event.info.ConfigInfoType.SETTINGS; import static org.geoserver.cloud.event.info.ConfigInfoType.WORKSPACE; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -24,7 +22,6 @@ import org.geoserver.catalog.Info; import org.geoserver.catalog.WorkspaceInfo; -import org.geoserver.catalog.impl.ResolvingProxy; import org.geoserver.catalog.plugin.CatalogPlugin; import org.geoserver.catalog.plugin.ExtendedCatalogFacade; import org.geoserver.cloud.autoconfigure.catalog.event.LocalCatalogEventsAutoConfiguration; @@ -182,7 +179,7 @@ void onGeoServerInfoModifyEvent() { assertThat(cache.get(GEOSERVERINFO_KEY)).isNotNull(); GeoServerInfoModified event = event(GeoServerInfoModified.class, "global", GEOSERVER); - caching.onGeoServerInfoModifyEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(GEOSERVERINFO_KEY)).isNull(); } @@ -192,7 +189,7 @@ void onGeoServerInfoSetEvent() { assertThat(cache.get(GEOSERVERINFO_KEY)).isNotNull(); GeoServerInfoSet event = event(GeoServerInfoSet.class, "global", GEOSERVER); - caching.onGeoServerInfoSetEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(GEOSERVERINFO_KEY)).isNull(); } @@ -202,7 +199,7 @@ void onLoggingInfoModifyEvent() { assertThat(cache.get(LOGGINGINFO_KEY)).isNotNull(); LoggingInfoModified event = event(LoggingInfoModified.class, "logging", LOGGING); - caching.onLoggingInfoModifyEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(LOGGINGINFO_KEY)).isNull(); } @@ -212,7 +209,7 @@ void onLoggingInfoSetEvent() { assertThat(cache.get(LOGGINGINFO_KEY)).isNotNull(); LoggingInfoSet event = event(LoggingInfoSet.class, "logging", LOGGING); - caching.onLoggingInfoSetEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(LOGGINGINFO_KEY)).isNull(); } @@ -228,7 +225,7 @@ void onSettingsInfoModifyEvent() { final String wsid = workspace.getId(); when(event.getWorkspaceId()).thenReturn(wsid); - caching.onSettingsInfoModifyEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(idKey)).isNull(); assertThat(cache.get(wsKey)).isNull(); } @@ -245,7 +242,7 @@ void onSettingsInfoRemoveEvent() { final String wsid = workspace.getId(); when(event.getWorkspaceId()).thenReturn(wsid); - caching.onSettingsInfoRemoveEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(idKey)).isNull(); assertThat(cache.get(wsKey)).isNull(); } @@ -269,7 +266,7 @@ void onServiceInfoModifyEvent() { assertThat(cache.get(nameKey)).isNotNull(); assertThat(cache.get(typeKey)).isNotNull(); - caching.onServiceInfoModifyEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(idKey)).isNull(); assertThat(cache.get(nameKey)).isNull(); @@ -281,7 +278,7 @@ void onServiceInfoModifyEvent() { assertThat(cache.get(nameKey)).isNotNull(); assertThat(cache.get(typeKey)).isNotNull(); - caching.onServiceInfoModifyEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(idKey)).isNull(); assertThat(cache.get(nameKey)).isNull(); @@ -312,7 +309,7 @@ void onServiceInfoRemoveEvent() { assertThat(cache.get(nameKey)).isNotNull(); assertThat(cache.get(typeKey)).isNotNull(); - caching.onServiceInfoRemoveEvent(event); + caching.onUpdateSequenceEvent(event); assertThat(cache.get(idKey)).isNull(); assertThat(cache.get(nameKey)).isNull(); @@ -338,7 +335,7 @@ void onWorkspaceRemoved() { // not a workspace removed event, shall not CatalogInfoRemoved event = event(CatalogInfoRemoved.class, "fakeid", NAMESPACE); - caching.onWorkspaceRemoved(event); + caching.onUpdateSequenceEvent(event); await().during(Duration.ofMillis(500)) .then() @@ -350,7 +347,7 @@ void onWorkspaceRemoved() { // this listener shall work with both local and remote events event = event(CatalogInfoRemoved.class, ws.getId(), WORKSPACE); when(event.isRemote()).thenReturn(false); - caching.onWorkspaceRemoved(event); + caching.onUpdateSequenceEvent(event); await().atMost(Duration.ofMillis(500)) .untilAsserted(() -> assertThat(nativeCache.asMap()).isEmpty()); @@ -360,7 +357,7 @@ void onWorkspaceRemoved() { caching.getService(ws, ServiceInfo.class); assertThat(nativeCache.estimatedSize()).isGreaterThan(3); when(event.isRemote()).thenReturn(true); - caching.onWorkspaceRemoved(event); + caching.onUpdateSequenceEvent(event); await().atMost(Duration.ofMillis(500)) .untilAsserted(() -> assertThat(nativeCache.asMap()).isEmpty()); } @@ -596,73 +593,6 @@ void testGetServiceByNameAndWorkspaceAndType() { assertNotNull(cache.get(typeKey)); } - /** {@link CachingGeoServerFacade#evict(Info)} manual eviction aid */ - @Test - void testEvict_GeoServerInfo() { - GeoServerInfo gsProxy = ResolvingProxy.create("someid", GeoServerInfo.class); - assertFalse(caching.evict(gsProxy)); - cache.put(GEOSERVERINFO_KEY, global); - assertTrue(caching.evict(gsProxy)); - assertFalse(caching.evict(gsProxy)); - assertNull(cache.get(GEOSERVERINFO_KEY)); - } - - /** {@link CachingGeoServerFacade#evict(Info)} manual eviction aid */ - @Test - void testEvict_LoggingInfo() { - LoggingInfo loggingProxy = ResolvingProxy.create(settings.getId(), LoggingInfo.class); - assertFalse(caching.evict(loggingProxy)); - cache.put(LOGGINGINFO_KEY, logging); - assertTrue(caching.evict(loggingProxy)); - assertFalse(caching.evict(loggingProxy)); - assertNull(cache.get(LOGGINGINFO_KEY)); - } - - /** {@link CachingGeoServerFacade#evict(Info)} manual eviction aid */ - @Test - void testEvict_SettingsInfo() { - assertNotNull(settings.getWorkspace()); - Object wsKey = CachingGeoServerFacade.settingsKey(settings.getWorkspace()); - final String id = settings.getId(); - - assertFalse(caching.evict(settings)); - - cache.put(id, settings); - cache.put(wsKey, settings); - - assertNotNull(cache.get(id)); - assertNotNull(cache.get(wsKey)); - - assertTrue(caching.evict(settings)); - assertFalse(caching.evict(settings)); - - assertNull(cache.get(id)); - assertNull(cache.get(wsKey)); - } - - /** {@link CachingGeoServerFacade#evict(Info)} manual eviction aid */ - @Test - void testEvict_ServiceInfo() { - TestService1 service = service1; - ServiceInfoKey idKey = ServiceInfoKey.byId(service.getId()); - ServiceInfoKey nameKey = ServiceInfoKey.byName(service.getWorkspace(), service.getName()); - ServiceInfoKey typeKey = ServiceInfoKey.byType(service.getWorkspace(), service.getClass()); - - ServiceInfo serviceProxy = ResolvingProxy.create(service.getId(), TestService1.class); - assertFalse(caching.evict(serviceProxy)); - - cache.put(idKey, service); - cache.put(nameKey, service); - cache.put(typeKey, service); - - assertTrue(caching.evict(serviceProxy)); - assertFalse(caching.evict(serviceProxy)); - - assertNull(cache.get(idKey)); - assertNull(cache.get(nameKey)); - assertNull(cache.get(typeKey)); - } - private void assertSameTimesN(T info, Supplier query, int times) { assertSameTimesN(info, id -> query.get(), times); }