Skip to content

Commit

Permalink
Merge pull request geoserver#418 from groldan/bug/cache_configuration
Browse files Browse the repository at this point in the history
Validate cache availability before creating caching catalog and config facades
  • Loading branch information
groldan authored Jan 29, 2024
2 parents 3bd5751 + f061e44 commit 5ca2491
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 25 deletions.
2 changes: 1 addition & 1 deletion config
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,17 @@ private void evictFromResourcePool(InfoEvent event) {

info.ifPresentOrElse(
object -> {
log.debug("Evict cache entry for {}({}) upon {}", infoType, id, event);
log.info(
"Evicting ResourcePool cache entry for {}({}) upon {}",
infoType,
id,
event);
ResourcePool resourcePool = rawCatalog.getResourcePool();
CacheClearingListener cleaner = new CacheClearingListener(resourcePool);
object.accept(cleaner);
}, //
() ->
log.info(
log.debug(
"{}({}) not found, unable to clean up its ResourcePool cache entry",
infoType,
id));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.geoserver.catalog.plugin.forwarding.ForwardingExtendedCatalogFacade;
import org.springframework.cache.Cache;
import org.springframework.cache.Cache.ValueWrapper;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.CacheConfig;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.CachePut;
Expand All @@ -38,9 +37,9 @@ class CachingCatalogFacadeImpl extends ForwardingExtendedCatalogFacade
implements CachingCatalogFacade {
private Cache idCache;

public CachingCatalogFacadeImpl(ExtendedCatalogFacade facade, CacheManager cacheManager) {
public CachingCatalogFacadeImpl(@NonNull ExtendedCatalogFacade facade, @NonNull Cache cache) {
super(facade);
idCache = cacheManager.getCache(CachingCatalogFacade.CACHE_NAME);
this.idCache = cache;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.geoserver.config.plugin.forwarding.ForwardingGeoServerFacade;
import org.springframework.cache.Cache;
import org.springframework.cache.Cache.ValueWrapper;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.CacheConfig;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.Cacheable;
Expand All @@ -31,7 +30,7 @@
class CachingGeoServerFacadeImpl extends ForwardingGeoServerFacade
implements CachingGeoServerFacade {

private Cache cache;
private final @NonNull Cache cache;

@Override
public Optional<GeoServerInfo> evictGlobal() {
Expand Down Expand Up @@ -109,9 +108,9 @@ static <T extends ServiceInfo> T cachePut(@NonNull Cache cache, @NonNull T servi
return service;
}

public CachingGeoServerFacadeImpl(GeoServerFacade facade, CacheManager cacheManager) {
public CachingGeoServerFacadeImpl(@NonNull GeoServerFacade facade, @NonNull Cache cache) {
super(facade);
cache = cacheManager.getCache(CACHE_NAME);
this.cache = cache;
}

@Cacheable(key = "'" + GEOSERVERINFO_KEY + "'", unless = "#result == null")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
import org.geoserver.config.GeoServerFacade;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.cache.Cache;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.EnableCaching;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import java.util.Objects;

/**
* Enables caching at the {@link CatalogFacade} and {@link GeoServerFacade} level instead of at the
* {@link Catalog} and {@link GeoServer} level, which would be the natural choice, in order not to
Expand Down Expand Up @@ -47,13 +50,24 @@ CachingCatalogFacade cachingCatalogFacade(
} else {
facade = new CatalogFacadeExtensionAdapter(rawCatalogFacade);
}
return new CachingCatalogFacadeImpl(facade, cacheManager);
Cache cache = getCache(cacheManager, CachingCatalogFacade.CACHE_NAME);
return new CachingCatalogFacadeImpl(facade, cache);
}

@Bean
CachingGeoServerFacade cachingGeoServerFacade(
@Qualifier("geoserverFacade") GeoServerFacade rawGeoServerFacade,
CacheManager cacheManager) {
return new CachingGeoServerFacadeImpl(rawGeoServerFacade, cacheManager);
Cache cache = getCache(cacheManager, CachingGeoServerFacade.CACHE_NAME);
return new CachingGeoServerFacadeImpl(rawGeoServerFacade, cache);
}

private Cache getCache(CacheManager cacheManager, String cacheName) {
Cache cache = cacheManager.getCache(cacheName);
Objects.requireNonNull(
cache,
"CacheManager %s returned null cache for cache name %s"
.formatted(cacheManager.getClass().getName(), cacheName));
return cache;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
*/
package org.geoserver.cloud.catalog.cache;

import lombok.Value;

import org.geoserver.catalog.WorkspaceInfo;
import org.geoserver.config.ServiceInfo;

import java.io.Serializable;

/** */
@Value
class ServiceInfoKey {
private String firstIdentifier;
private String secondIdentifier;
record ServiceInfoKey(String key, String qualifier) implements Serializable {

public static ServiceInfoKey byId(String id) {
return new ServiceInfoKey(id, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ private void evictEntry(InfoEvent event, BooleanSupplier evictor) {
evt -> {
boolean evicted = evictor.getAsBoolean();
if (evicted) {
log.debug("Evicted cache entry {}", evt);
log.debug("Evicted Catalog cache entry due to {}", evt);
} else {
log.trace("Remote event resulted in no cache eviction: {}", evt);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class RemoteEventCacheEvictorTest {
* Spring-cache for configuration Info objects, named after {@link
* CachingGeoServerFacade#CACHE_NAME}
*/
private Cache configCache;
private org.springframework.cache.Cache configCache;

private @Autowired @Qualifier("catalog") Catalog catalog;
private @Autowired @Qualifier("geoServer") GeoServerImpl geoServer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ UpdateSequence defaultUpdateSequence(GeoServer geoserver) {
}

@Bean(name = {"rawCatalog"})
public Catalog rawCatalog(@Qualifier("catalogFacade") CatalogFacade facade) {
Catalog rawCatalog(@Qualifier("catalogFacade") CatalogFacade facade) {
return new CatalogPlugin(facade);
}

@Bean(name = {"catalog"})
public Catalog catalog(@Qualifier("rawCatalog") Catalog raw) {
Catalog catalog(@Qualifier("rawCatalog") Catalog raw) {
return raw;
}

@Bean(name = "geoServer")
public GeoServer geoServer(
GeoServer geoServer(
@Qualifier("catalog") Catalog catalog,
@Qualifier("geoserverFacade") GeoServerFacade facade) {

Expand All @@ -59,12 +59,12 @@ public GeoServer geoServer(
}

@Bean(name = "catalogFacade")
public CatalogFacade catalogFacade() {
CatalogFacade catalogFacade() {
return new DefaultMemoryCatalogFacade();
}

@Bean(name = "geoserverFacade")
public GeoServerFacade geoserverFacade() {
GeoServerFacade geoserverFacade() {
return new RepositoryGeoServerFacadeImpl();
}
}

0 comments on commit 5ca2491

Please sign in to comment.