From cc54e2ce6bcea522cb17dc17a5cdedcb8b5c0c48 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 28 Jun 2023 12:38:15 +0200 Subject: [PATCH] Caches for internal stores and files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds optional caches for configuration stores and backing data files for data stores. These stores can be enabled using the system properties `scm.storeCache.enabled=true` and `scm.cache.dataFileCache.enabled=true`. In addition, this adds the possibility to overwrite cache configurations from the guice cache (see file `gcache.xml`) with system properties. The maximum size of the external group cache for example can be overwritten with the system property `scm.cache.externalGroups.maximumSize`. Co-authored-by: René Pfeuffer --- gradle/changelog/store_cache.yaml | 2 + .../store/TypedStoreParametersBuilder.java | 2 + .../java/sonia/scm/store/CopyOnWrite.java | 5 + .../java/sonia/scm/store/DataFileCache.java | 142 +++++++++++ .../JAXBConfigurationEntryStoreFactory.java | 12 +- .../store/JAXBConfigurationStoreFactory.java | 8 + .../java/sonia/scm/store/JAXBDataStore.java | 35 +-- .../sonia/scm/store/JAXBDataStoreFactory.java | 23 +- .../main/java/sonia/scm/store/StoreCache.java | 57 +++++ .../sonia/scm/store/TypedStoreContext.java | 4 + .../sonia/scm/store/DataFileCacheTest.java | 141 +++++++++++ .../sonia/scm/store/JAXBDataStoreTest.java | 11 +- .../scm/cache/GuavaCacheConfiguration.java | 221 ++++++------------ .../cache/GuavaNamedCacheConfiguration.java | 129 ++++++++-- .../lifecycle/modules/BootstrapModule.java | 13 ++ .../lifecycle/modules/ScmServletModule.java | 12 +- .../src/main/resources/config/gcache.xml | 8 + .../GuavaNamedCacheConfigurationTest.java | 86 +++++++ 18 files changed, 706 insertions(+), 205 deletions(-) create mode 100644 gradle/changelog/store_cache.yaml create mode 100644 scm-dao-xml/src/main/java/sonia/scm/store/DataFileCache.java create mode 100644 scm-dao-xml/src/main/java/sonia/scm/store/StoreCache.java create mode 100644 scm-dao-xml/src/test/java/sonia/scm/store/DataFileCacheTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/cache/GuavaNamedCacheConfigurationTest.java diff --git a/gradle/changelog/store_cache.yaml b/gradle/changelog/store_cache.yaml new file mode 100644 index 0000000000..b06ac5f643 --- /dev/null +++ b/gradle/changelog/store_cache.yaml @@ -0,0 +1,2 @@ +- type: added + description: Optional caching for stores and data files diff --git a/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersBuilder.java b/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersBuilder.java index c4602f1656..15e90516fd 100644 --- a/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersBuilder.java +++ b/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersBuilder.java @@ -26,6 +26,7 @@ package sonia.scm.store; import com.google.common.collect.ImmutableSet; import lombok.AccessLevel; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.Setter; @@ -67,6 +68,7 @@ public final class TypedStoreParametersBuilder { @Getter @Setter(AccessLevel.PRIVATE) @RequiredArgsConstructor + @EqualsAndHashCode(exclude = {"classLoader", "adapters"}) private static class TypedStoreParametersImpl implements TypedStoreParameters { private final Class type; private String name; diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java index 567a518fdc..12ef73e667 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java @@ -55,11 +55,16 @@ public final class CopyOnWrite { } public static void withTemporaryFile(FileWriter writer, Path targetFile) { + withTemporaryFile(writer, targetFile, () -> {}); + } + + public static void withTemporaryFile(FileWriter writer, Path targetFile, Runnable onSuccess) { validateInput(targetFile); execute(() -> { Path temporaryFile = createTemporaryFile(targetFile); executeCallback(writer, targetFile, temporaryFile); replaceOriginalFile(targetFile, temporaryFile); + onSuccess.run(); }).withLockedFileForWrite(targetFile); } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/DataFileCache.java b/scm-dao-xml/src/main/java/sonia/scm/store/DataFileCache.java new file mode 100644 index 0000000000..9b31f51140 --- /dev/null +++ b/scm-dao-xml/src/main/java/sonia/scm/store/DataFileCache.java @@ -0,0 +1,142 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.store; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.cache.Cache; +import sonia.scm.cache.CacheManager; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.File; +import java.util.function.Supplier; + +@Singleton +public class DataFileCache { + + private static final String CACHE_NAME = "sonia.cache.dataFileCache"; + private static final String ENABLE_CACHE_PROPERTY_NAME = "scm.cache.dataFileCache.enabled"; + + private static final Logger LOG = LoggerFactory.getLogger(DataFileCache.class); + + private static final NoDataFileCacheInstance NO_CACHE = new NoDataFileCacheInstance(); + + private final Cache cache; + private final boolean cacheEnabled; + + @Inject + DataFileCache(CacheManager cacheManager) { + this(cacheManager.getCache(CACHE_NAME), Boolean.getBoolean(ENABLE_CACHE_PROPERTY_NAME)); + } + + @VisibleForTesting + DataFileCache(Cache cache, boolean cacheEnabled) { + this.cache = cache; + this.cacheEnabled = cacheEnabled; + } + + DataFileCacheInstance instanceFor(Class type) { + if (cacheEnabled) { + return new GCacheDataFileCacheInstance(type); + } else { + return NO_CACHE; + } + } + + @Override + public String toString() { + long size = cache.keys().stream().map(File::length).reduce(0L, Long::sum); + return String.format("data file cache, %s entries, %s bytes cached", cache.size(), size); + } + + interface DataFileCacheInstance { + void put(File file, T item); + + T get(File file, Supplier reader); + + void remove(File file); + } + + private static class NoDataFileCacheInstance implements DataFileCacheInstance { + + @Override + public void put(File file, T item) { + // nothing to do without cache + } + + @Override + public T get(File file, Supplier reader) { + return reader.get(); + } + + @Override + public void remove(File file) { + // nothing to do + } + } + + private class GCacheDataFileCacheInstance implements DataFileCacheInstance { + + private final Class type; + + GCacheDataFileCacheInstance(Class type) { + this.type = type; + } + + public void put(File file, T item) { + LOG.trace("put '{}' in {}", file, DataFileCache.this); + if (item != null) { + cache.put(file, item); + } + } + + public T get(File file, Supplier reader) { + LOG.trace("get of '{}' from {}", file, DataFileCache.this); + File absoluteFile = file.getAbsoluteFile(); + if (cache.contains(absoluteFile)) { + T t = (T) cache.get(absoluteFile); + if (t == null || type.isAssignableFrom(t.getClass())) { + return t; + } else { + LOG.info("discarding cached entry with wrong type (expected: {}, got {})", type, t.getClass()); + cache.remove(file); + } + } + + T item = reader.get(); + if (item != null) { + cache.put(absoluteFile, item); + } + return item; + } + + public void remove(File file) { + LOG.trace("remove of '{}' from {}", file, DataFileCache.this); + cache.remove(file); + } + } +} diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStoreFactory.java index 6bef72f00c..16c21c6ca3 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationEntryStoreFactory.java @@ -24,8 +24,6 @@ package sonia.scm.store; -//~--- non-JDK imports -------------------------------------------------------- - import com.google.inject.Inject; import com.google.inject.Singleton; import sonia.scm.SCMContextProvider; @@ -33,8 +31,6 @@ import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.repository.RepositoryReadOnlyChecker; import sonia.scm.security.KeyGenerator; -//~--- JDK imports ------------------------------------------------------------ - /** * @author Sebastian Sdorra */ @@ -44,14 +40,22 @@ public class JAXBConfigurationEntryStoreFactory extends FileBasedStoreFactory private final KeyGenerator keyGenerator; + private final StoreCache> storeCache; + @Inject public JAXBConfigurationEntryStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker) { super(contextProvider, repositoryLocationResolver, Store.CONFIG, readOnlyChecker); this.keyGenerator = keyGenerator; + this.storeCache = new StoreCache<>(this::createStore); } @Override + @SuppressWarnings("unchecked") public ConfigurationEntryStore getStore(TypedStoreParameters storeParameters) { + return (ConfigurationEntryStore) storeCache.getStore(storeParameters); + } + + private ConfigurationEntryStore createStore(TypedStoreParameters storeParameters) { return new JAXBConfigurationEntryStore<>( getStoreLocation(storeParameters.getName().concat(StoreConstants.FILE_EXTENSION), storeParameters.getType(), diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStoreFactory.java index 64d7043943..59611e2c30 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStoreFactory.java @@ -42,6 +42,8 @@ public class JAXBConfigurationStoreFactory extends FileBasedStoreFactory impleme private final Set decoratorFactories; + private final StoreCache> storeCache; + /** * Constructs a new instance. * @@ -51,10 +53,16 @@ public class JAXBConfigurationStoreFactory extends FileBasedStoreFactory impleme public JAXBConfigurationStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, RepositoryReadOnlyChecker readOnlyChecker, Set decoratorFactories) { super(contextProvider, repositoryLocationResolver, Store.CONFIG, readOnlyChecker); this.decoratorFactories = decoratorFactories; + this.storeCache = new StoreCache<>(this::createStore); } @Override + @SuppressWarnings("unchecked") public ConfigurationStore getStore(TypedStoreParameters storeParameters) { + return (ConfigurationStore) storeCache.getStore(storeParameters); + } + + private ConfigurationStore createStore(TypedStoreParameters storeParameters) { TypedStoreContext context = TypedStoreContext.of(storeParameters); ConfigurationStore store = new JAXBConfigurationStore<>( diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java index bed9c430ba..5a60e26b16 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStore.java @@ -40,9 +40,8 @@ import static sonia.scm.store.CopyOnWrite.compute; /** * Jaxb implementation of {@link DataStore}. * - * @author Sebastian Sdorra - * * @param type of stored data. + * @author Sebastian Sdorra */ public class JAXBDataStore extends FileBasedStore implements DataStore { @@ -53,10 +52,12 @@ public class JAXBDataStore extends FileBasedStore implements DataStore private final KeyGenerator keyGenerator; private final TypedStoreContext context; + private final DataFileCache.DataFileCacheInstance cache; - JAXBDataStore(KeyGenerator keyGenerator, TypedStoreContext context, File directory, boolean readOnly) { + JAXBDataStore(KeyGenerator keyGenerator, TypedStoreContext context, File directory, boolean readOnly, DataFileCache.DataFileCacheInstance cache) { super(directory, StoreConstants.FILE_EXTENSION, readOnly); this.keyGenerator = keyGenerator; + this.cache = cache; this.directory = directory; this.context = context; } @@ -75,10 +76,10 @@ public class JAXBDataStore extends FileBasedStore implements DataStore marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE); CopyOnWrite.withTemporaryFile( temp -> marshaller.marshal(item, temp.toFile()), - file.toPath() + file.toPath(), + () -> cache.put(file, item) ); - } - catch (JAXBException ex) { + } catch (JAXBException ex) { throw new StoreException("could not write object with id ".concat(id), ex); } @@ -106,14 +107,22 @@ public class JAXBDataStore extends FileBasedStore implements DataStore return builder.build(); } + @Override + protected void remove(File file) { + cache.remove(file); + super.remove(file); + } + @Override protected T read(File file) { - return compute(() -> { - if (file.exists()) { - LOG.trace("try to read {}", file); - return context.unmarshall(file); - } - return null; - }).withLockedFileForRead(file); + return cache.get(file, () -> + compute(() -> { + if (file.exists()) { + LOG.trace("try to read {}", file); + return context.unmarshall(file); + } + return null; + }).withLockedFileForRead(file) + ); } } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStoreFactory.java index d15a04d3d3..67d64a80f1 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBDataStoreFactory.java @@ -24,8 +24,6 @@ package sonia.scm.store; -//~--- non-JDK imports -------------------------------------------------------- - import com.google.inject.Inject; import com.google.inject.Singleton; import sonia.scm.SCMContextProvider; @@ -46,16 +44,33 @@ public class JAXBDataStoreFactory extends FileBasedStoreFactory private final KeyGenerator keyGenerator; + private final StoreCache> storeCache; + + private final DataFileCache dataFileCache; + @Inject - public JAXBDataStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker) { + public JAXBDataStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker, DataFileCache dataFileCache) { super(contextProvider, repositoryLocationResolver, Store.DATA, readOnlyChecker); this.keyGenerator = keyGenerator; + this.dataFileCache = dataFileCache; + this.storeCache = new StoreCache<>(this::createStore); } @Override + @SuppressWarnings("unchecked") public DataStore getStore(TypedStoreParameters storeParameters) { + return (DataStore) storeCache.getStore(storeParameters); + } + + private DataStore createStore(TypedStoreParameters storeParameters) { File storeLocation = getStoreLocation(storeParameters); IOUtil.mkdirs(storeLocation); - return new JAXBDataStore<>(keyGenerator, TypedStoreContext.of(storeParameters), storeLocation, mustBeReadOnly(storeParameters)); + return new JAXBDataStore<>( + keyGenerator, + TypedStoreContext.of(storeParameters), + storeLocation, + mustBeReadOnly(storeParameters), + dataFileCache.instanceFor(storeParameters.getType()) + ); } } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/StoreCache.java b/scm-dao-xml/src/main/java/sonia/scm/store/StoreCache.java new file mode 100644 index 0000000000..a8d921ca2d --- /dev/null +++ b/scm-dao-xml/src/main/java/sonia/scm/store/StoreCache.java @@ -0,0 +1,57 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.store; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +import static java.util.Collections.synchronizedMap; + +class StoreCache { + + private static final Logger LOG = LoggerFactory.getLogger(StoreCache.class); + + public static final String ENABLE_STORE_CACHE_PROPERTY = "scm.storeCache.enabled"; + + private final Function, S> cachingStoreCreator; + + StoreCache(Function, S> storeCreator) { + if (Boolean.getBoolean(ENABLE_STORE_CACHE_PROPERTY)) { + LOG.info("store cache enabled"); + Map, S> storeCache = synchronizedMap(new HashMap<>()); + cachingStoreCreator = storeParameters -> storeCache.computeIfAbsent(storeParameters, storeCreator); + } else { + cachingStoreCreator = storeCreator; + } + } + + S getStore(TypedStoreParameters storeParameters) { + return cachingStoreCreator.apply(storeParameters); + } +} diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java b/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java index f3dde1026c..2d5aaae858 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java @@ -86,6 +86,10 @@ final class TypedStoreContext { withClassLoader(consumer, unmarshaller); } + Class getType() { + return parameters.getType(); + } + private void withClassLoader(ThrowingConsumer consumer, C consume) { ClassLoader contextClassLoader = null; Optional classLoader = parameters.getClassLoader(); diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/DataFileCacheTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/DataFileCacheTest.java new file mode 100644 index 0000000000..964e043e05 --- /dev/null +++ b/scm-dao-xml/src/test/java/sonia/scm/store/DataFileCacheTest.java @@ -0,0 +1,141 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.store; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.cache.MapCache; + +import java.io.File; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.in; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DataFileCacheTest { + + @Nested + class WithActivatedCache { + + private final MapCache backingCache = new MapCache<>(); + private final DataFileCache dataFileCache = new DataFileCache(backingCache, true); + + @Test + void shouldReturnCachedData() { + File file = new File("/some.string"); + backingCache.put(file, "some string"); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + Object result = instance.get(file, () -> { + throw new RuntimeException("should not be read"); + }); + + assertThat(result).isSameAs("some string"); + } + + @Test + void shouldReadDataIfNotCached() { + File file = new File("/some.string"); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + Object result = instance.get(file, () -> "some string"); + + assertThat(result).isSameAs("some string"); + assertThat(backingCache.get(file)).isSameAs("some string"); + } + + @Test + void shouldReadDataAnewIfOfDifferentType() { + File file = new File("/some.string"); + backingCache.put(file, 42); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + Object result = instance.get(file, () -> "some string"); + + assertThat(result).isSameAs("some string"); + assertThat(backingCache.get(file)).isSameAs("some string"); + } + + @Test + void shouldRemoveOutdatedDataIfOfDifferentType() { + File file = new File("/some.string"); + backingCache.put(file, 42); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + Object result = instance.get(file, () -> null); + + assertThat(result).isNull(); + assertThat(backingCache.get(file)).isNull(); + } + + @Test + void shouldCacheNewData() { + File file = new File("/some.string"); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + instance.put(file, "some string"); + + assertThat(backingCache.get(file)).isSameAs("some string"); + } + + @Test + void shouldRemoveDataFromCache() { + File file = new File("/some.string"); + backingCache.put(file, "some string"); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + instance.remove(file); + + assertThat(backingCache.get(file)).isNull(); + } + } + + @Nested + class WithDeactivatedCache { + + private final DataFileCache dataFileCache = new DataFileCache(null, false); + + @Test + void shouldReadData() { + File file = new File("/some.string"); + + DataFileCache.DataFileCacheInstance instance = dataFileCache.instanceFor(String.class); + + Object result = instance.get(file, () -> "some string"); + + assertThat(result).isSameAs("some string"); + } + } +} diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java index f762502b82..ced640a628 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBDataStoreTest.java @@ -24,9 +24,8 @@ package sonia.scm.store; -//~--- non-JDK imports -------------------------------------------------------- - import org.junit.Test; +import sonia.scm.cache.MapCache; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryReadOnlyChecker; import sonia.scm.security.UUIDKeyGenerator; @@ -47,7 +46,13 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStoreFactory createDataStoreFactory() { - return new JAXBDataStoreFactory(contextProvider, repositoryLocationResolver, new UUIDKeyGenerator(), readOnlyChecker); + return new JAXBDataStoreFactory( + contextProvider, + repositoryLocationResolver, + new UUIDKeyGenerator(), + readOnlyChecker, + new DataFileCache(null, false) + ); } @Override diff --git a/scm-webapp/src/main/java/sonia/scm/cache/GuavaCacheConfiguration.java b/scm-webapp/src/main/java/sonia/scm/cache/GuavaCacheConfiguration.java index 428e9d0668..5724e58d12 100644 --- a/scm-webapp/src/main/java/sonia/scm/cache/GuavaCacheConfiguration.java +++ b/scm-webapp/src/main/java/sonia/scm/cache/GuavaCacheConfiguration.java @@ -21,10 +21,8 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - -package sonia.scm.cache; -//~--- non-JDK imports -------------------------------------------------------- +package sonia.scm.cache; import com.google.common.base.MoreObjects; @@ -35,215 +33,138 @@ import javax.xml.bind.annotation.XmlRootElement; import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import java.io.Serializable; -//~--- JDK imports ------------------------------------------------------------ - -/** - * - * @author Sebastian Sdorra - */ @XmlRootElement(name = "cache") -@XmlAccessorType(XmlAccessType.FIELD) -public class GuavaCacheConfiguration implements Serializable -{ +@XmlAccessorType(XmlAccessType.PROPERTY) +public class GuavaCacheConfiguration implements Serializable { - /** Field description */ private static final long serialVersionUID = -8734373158089010603L; - //~--- methods -------------------------------------------------------------- + private Integer concurrencyLevel; + private CopyStrategy copyStrategy; + private Long expireAfterAccess; + private Long expireAfterWrite; + private Integer initialCapacity; + private Long maximumSize; + private Long maximumWeight; + private Boolean recordStats; + private Boolean softValues; + private Boolean weakKeys; + private Boolean weakValues; - /** - * Method description - * - * - * @return - */ @Override - public String toString() - { - //J- + public String toString() { return MoreObjects.toStringHelper(this) - .add("concurrencyLevel", concurrencyLevel) - .add("copyStrategy", copyStrategy) - .add("expireAfterAccess", expireAfterAccess) - .add("expireAfterWrite", expireAfterWrite) - .add("initialCapacity", initialCapacity) - .add("maximumSize", maximumSize) - .add("maximumWeight", maximumWeight) - .add("recordStats", recordStats) - .add("softValues", softValues) - .add("weakKeys", weakKeys) - .add("weakValues", weakValues) - .omitNullValues().toString(); - //J+ + .add("concurrencyLevel", concurrencyLevel) + .add("copyStrategy", copyStrategy) + .add("expireAfterAccess", expireAfterAccess) + .add("expireAfterWrite", expireAfterWrite) + .add("initialCapacity", initialCapacity) + .add("maximumSize", maximumSize) + .add("maximumWeight", maximumWeight) + .add("recordStats", recordStats) + .add("softValues", softValues) + .add("weakKeys", weakKeys) + .add("weakValues", weakValues) + .omitNullValues().toString(); } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @return - */ - public Integer getConcurrencyLevel() - { + public Integer getConcurrencyLevel() { return concurrencyLevel; } - /** - * Method description - * - * - * @return - */ - public CopyStrategy getCopyStrategy() - { + public CopyStrategy getCopyStrategy() { return copyStrategy; } - /** - * Method description - * - * - * @return - */ - public Long getExpireAfterAccess() - { + public Long getExpireAfterAccess() { return expireAfterAccess; } - /** - * Method description - * - * - * @return - */ - public Long getExpireAfterWrite() - { + public Long getExpireAfterWrite() { return expireAfterWrite; } - /** - * Method description - * - * - * @return - */ - public Integer getInitialCapacity() - { + public Integer getInitialCapacity() { return initialCapacity; } - /** - * Method description - * - * - * @return - */ - public Long getMaximumSize() - { + public Long getMaximumSize() { return maximumSize; } - /** - * Method description - * - * - * @return - */ - public Long getMaximumWeight() - { + public Long getMaximumWeight() { return maximumWeight; } - /** - * Method description - * - * - * @return - */ - public Boolean getRecordStats() - { + public Boolean getRecordStats() { return recordStats; } - /** - * Method description - * - * - * @return - */ - public Boolean getSoftValues() - { + public Boolean getSoftValues() { return softValues; } - /** - * Method description - * - * - * @return - */ - public Boolean getWeakKeys() - { + public Boolean getWeakKeys() { return weakKeys; } - /** - * Method description - * - * - * @return - */ - public Boolean getWeakValues() - { + public Boolean getWeakValues() { return weakValues; } - //~--- fields --------------------------------------------------------------- - - /** Field description */ @XmlAttribute - private Integer concurrencyLevel; + void setConcurrencyLevel(Integer concurrencyLevel) { + this.concurrencyLevel = concurrencyLevel; + } - /** Field description */ @XmlAttribute @XmlJavaTypeAdapter(XmlCopyStrategyAdapter.class) - private CopyStrategy copyStrategy; + void setCopyStrategy(CopyStrategy copyStrategy) { + this.copyStrategy = copyStrategy; + } - /** Field description */ @XmlAttribute - private Long expireAfterAccess; + void setExpireAfterAccess(Long expireAfterAccess) { + this.expireAfterAccess = expireAfterAccess; + } - /** Field description */ @XmlAttribute - private Long expireAfterWrite; + void setExpireAfterWrite(Long expireAfterWrite) { + this.expireAfterWrite = expireAfterWrite; + } - /** Field description */ @XmlAttribute - private Integer initialCapacity; + void setInitialCapacity(Integer initialCapacity) { + this.initialCapacity = initialCapacity; + } - /** Field description */ @XmlAttribute - private Long maximumSize; + void setMaximumSize(Long maximumSize) { + this.maximumSize = maximumSize; + } - /** Field description */ @XmlAttribute - private Long maximumWeight; + void setMaximumWeight(Long maximumWeight) { + this.maximumWeight = maximumWeight; + } - /** Field description */ @XmlAttribute - private Boolean recordStats; + void setRecordStats(Boolean recordStats) { + this.recordStats = recordStats; + } - /** Field description */ @XmlAttribute - private Boolean softValues; + void setSoftValues(Boolean softValues) { + this.softValues = softValues; + } - /** Field description */ @XmlAttribute - private Boolean weakKeys; + void setWeakKeys(Boolean weakKeys) { + this.weakKeys = weakKeys; + } - /** Field description */ @XmlAttribute - private Boolean weakValues; + void setWeakValues(Boolean weakValues) { + this.weakValues = weakValues; + } } diff --git a/scm-webapp/src/main/java/sonia/scm/cache/GuavaNamedCacheConfiguration.java b/scm-webapp/src/main/java/sonia/scm/cache/GuavaNamedCacheConfiguration.java index 9e22cc06df..396dbe00c4 100644 --- a/scm-webapp/src/main/java/sonia/scm/cache/GuavaNamedCacheConfiguration.java +++ b/scm-webapp/src/main/java/sonia/scm/cache/GuavaNamedCacheConfiguration.java @@ -21,44 +21,133 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.cache; //~--- JDK imports ------------------------------------------------------------ +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlRootElement; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Function; + +import static java.util.Optional.empty; +import static java.util.Optional.of; -/** - * - * @author Sebastian Sdorra - */ @XmlRootElement(name = "cache") -@XmlAccessorType(XmlAccessType.FIELD) -public class GuavaNamedCacheConfiguration extends GuavaCacheConfiguration -{ +@XmlAccessorType(XmlAccessType.PROPERTY) +public class GuavaNamedCacheConfiguration extends GuavaCacheConfiguration { - /** Field description */ private static final long serialVersionUID = -624795324874828475L; + private static final Logger LOG = LoggerFactory.getLogger(GuavaNamedCacheConfiguration.class); - //~--- get methods ---------------------------------------------------------- + private String name; - /** - * Method description - * - * - * @return - */ public String getName() { return name; } - //~--- fields --------------------------------------------------------------- - - /** Field description */ @XmlAttribute - private String name; + private void setName(String name) { + this.name = name; + } + + @Override + void setConcurrencyLevel(Integer concurrencyLevel) { + setIntegerValue("concurrencyLevel", concurrencyLevel, super::setConcurrencyLevel); + } + + @Override + void setCopyStrategy(CopyStrategy copyStrategy) { + setValue("copyStrategy", copyStrategy, super::setCopyStrategy, propertyName -> of(System.getProperty(propertyName)).map(CopyStrategy::valueOf).orElse(null)); + } + + @Override + void setExpireAfterAccess(Long expireAfterAccess) { + setLongValue("expireAfterAccess", expireAfterAccess, super::setExpireAfterAccess); + } + + @Override + void setExpireAfterWrite(Long expireAfterWrite) { + setLongValue("expireAfterWrite", expireAfterWrite, super::setExpireAfterWrite); + } + + @Override + void setInitialCapacity(Integer initialCapacity) { + setIntegerValue("initialCapacity", initialCapacity, super::setInitialCapacity); + } + + @Override + void setMaximumSize(Long maximumSize) { + setLongValue("maximumSize", maximumSize, super::setMaximumSize); + } + + @Override + void setMaximumWeight(Long maximumWeight) { + setLongValue("maximumWeight", maximumWeight, super::setMaximumWeight); + } + + @Override + void setRecordStats(Boolean recordStats) { + setBooleanValue("recordStats", recordStats, super::setRecordStats); + } + + @Override + void setSoftValues(Boolean softValues) { + setBooleanValue("softValues", softValues, super::setSoftValues); + } + + @Override + void setWeakKeys(Boolean weakKeys) { + setBooleanValue("weakKeys", weakKeys, super::setWeakKeys); + } + + @Override + void setWeakValues(Boolean weakValues) { + setBooleanValue("weakValues", weakValues, super::setWeakValues); + } + + private void setIntegerValue(String propertyName, Integer originalValue, Consumer setter) { + setValue(propertyName, originalValue, setter, Integer::getInteger); + } + + private void setLongValue(String propertyName, Long originalValue, Consumer setter) { + setValue(propertyName, originalValue, setter, Long::getLong); + } + + private void setBooleanValue(String propertyName, Boolean originalValue, Consumer setter) { + setValue(propertyName, originalValue, setter, Boolean::getBoolean); + } + + private void setValue(String propertyName, T originalValue, Consumer setter, Function systemPropertyReader) { + setter.accept(originalValue); + createPropertyName(propertyName) + .map(systemPropertyReader) + .ifPresent(value -> { + logOverwrite(propertyName, originalValue, value); + setter.accept(value); + }); + } + + private void logOverwrite(String propertyName, Object originalValue, Object overwrittenValue) { + LOG.debug("overwrite {} of cache '{}' with system property value {} (original value: {})", propertyName, name, overwrittenValue, originalValue); + } + + private Optional createPropertyName(String fieldName) { + if (name == null) { + LOG.warn("failed to overwrite cache configuration with system properties, because name has not been set yet"); + return empty(); + } + if (name.startsWith("sonia.cache.")) { + return of("scm.cache." + name.substring("sonia.cache.".length()) + "." + fieldName); + } + return empty(); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java index 411a79968a..07fe9d2443 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java @@ -28,14 +28,19 @@ import com.google.inject.AbstractModule; import com.google.inject.TypeLiteral; import com.google.inject.multibindings.Multibinder; import com.google.inject.throwingproviders.ThrowingProviderBinder; +import io.micrometer.core.instrument.MeterRegistry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.SCMContext; import sonia.scm.SCMContextProvider; +import sonia.scm.cache.CacheManager; +import sonia.scm.cache.GuavaCacheManager; import sonia.scm.io.DefaultFileSystem; import sonia.scm.io.FileSystem; import sonia.scm.lifecycle.DefaultRestarter; import sonia.scm.lifecycle.Restarter; +import sonia.scm.metrics.MeterRegistryProvider; +import sonia.scm.metrics.MonitoringSystem; import sonia.scm.plugin.PluginLoader; import sonia.scm.repository.DefaultRepositoryExportingCheck; import sonia.scm.repository.EventDrivenRepositoryArchiveCheck; @@ -121,6 +126,14 @@ public class BootstrapModule extends AbstractModule { bind(RepositoryUpdateIterator.class, FileRepositoryUpdateIterator.class); bind(StoreUpdateStepUtilFactory.class, FileStoreUpdateStepUtilFactory.class); bind(new TypeLiteral>() {}).to(new TypeLiteral() {}); + + // bind metrics + bind(MeterRegistry.class).toProvider(MeterRegistryProvider.class).asEagerSingleton(); + Multibinder.newSetBinder(binder(), MonitoringSystem.class); + + // bind cache + bind(CacheManager.class, GuavaCacheManager.class); + bind(org.apache.shiro.cache.CacheManager.class, GuavaCacheManager.class); } private void bind(Class clazz, Class defaultImplementation) { diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmServletModule.java index 52d3ee8845..604b04c775 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/ScmServletModule.java @@ -30,7 +30,6 @@ import com.google.inject.multibindings.Multibinder; import com.google.inject.servlet.RequestScoped; import com.google.inject.servlet.ServletModule; import com.google.inject.throwingproviders.ThrowingProviderBinder; -import io.micrometer.core.instrument.MeterRegistry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.Default; @@ -39,14 +38,13 @@ import sonia.scm.PushStateDispatcher; import sonia.scm.PushStateDispatcherProvider; import sonia.scm.RootURL; import sonia.scm.Undecorated; +import sonia.scm.admin.ScmConfigurationStore; import sonia.scm.api.rest.ObjectMapperProvider; import sonia.scm.api.v2.resources.BranchLinkProvider; import sonia.scm.api.v2.resources.DefaultBranchLinkProvider; import sonia.scm.api.v2.resources.DefaultRepositoryLinkProvider; import sonia.scm.api.v2.resources.RepositoryLinkProvider; import sonia.scm.auditlog.AuditLogConfigurationStoreDecoratorFactory; -import sonia.scm.cache.CacheManager; -import sonia.scm.cache.GuavaCacheManager; import sonia.scm.config.ScmConfiguration; import sonia.scm.event.ScmEventBus; import sonia.scm.group.DefaultGroupCollector; @@ -65,7 +63,6 @@ import sonia.scm.initialization.InitializationCookieIssuer; import sonia.scm.initialization.InitializationFinisher; import sonia.scm.io.ContentTypeResolver; import sonia.scm.io.DefaultContentTypeResolver; -import sonia.scm.metrics.MeterRegistryProvider; import sonia.scm.migration.MigrationDAO; import sonia.scm.net.SSLContextProvider; import sonia.scm.net.TrustManagerProvider; @@ -134,7 +131,6 @@ import sonia.scm.user.UserManager; import sonia.scm.user.UserManagerProvider; import sonia.scm.user.xml.XmlUserDAO; import sonia.scm.util.DebugServlet; -import sonia.scm.admin.ScmConfigurationStore; import sonia.scm.web.UserAgentParser; import sonia.scm.web.cgi.CGIExecutorFactory; import sonia.scm.web.cgi.DefaultCGIExecutorFactory; @@ -200,8 +196,6 @@ class ScmServletModule extends ServletModule { // bind extensions pluginLoader.getExtensionProcessor().processAutoBindExtensions(binder()); - // bind metrics - bind(MeterRegistry.class).toProvider(MeterRegistryProvider.class).asEagerSingleton(); // bind security stuff bind(LoginAttemptHandler.class).to(ConfigurableLoginAttemptHandler.class); @@ -210,10 +204,6 @@ class ScmServletModule extends ServletModule { bind(SecuritySystem.class).to(DefaultSecuritySystem.class); bind(AdministrationContext.class, DefaultAdministrationContext.class); - // bind cache - bind(CacheManager.class, GuavaCacheManager.class); - bind(org.apache.shiro.cache.CacheManager.class, GuavaCacheManager.class); - // bind dao bind(GroupDAO.class, XmlGroupDAO.class); bind(UserDAO.class, XmlUserDAO.class); diff --git a/scm-webapp/src/main/resources/config/gcache.xml b/scm-webapp/src/main/resources/config/gcache.xml index bfab7a7367..76cfc92700 100644 --- a/scm-webapp/src/main/resources/config/gcache.xml +++ b/scm-webapp/src/main/resources/config/gcache.xml @@ -163,4 +163,12 @@ Search cache for repository changesets maximumSize="500" /> + + + diff --git a/scm-webapp/src/test/java/sonia/scm/cache/GuavaNamedCacheConfigurationTest.java b/scm-webapp/src/test/java/sonia/scm/cache/GuavaNamedCacheConfigurationTest.java new file mode 100644 index 0000000000..95e62d4027 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/cache/GuavaNamedCacheConfigurationTest.java @@ -0,0 +1,86 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.cache; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.xml.bind.JAXBContext; +import javax.xml.bind.JAXBException; +import java.io.StringReader; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(MockitoExtension.class) +class GuavaNamedCacheConfigurationTest { + + public static final String CACHE_CONFIGURATION = + "" + + " " + + ""; + + @Test + void shouldTakeValueFromConfigurationFile() throws JAXBException { + GuavaCacheManagerConfiguration configuration = readConfiguration(); + + assertThat(configuration.getCaches().get(0).getName()).isEqualTo("sonia.cache.externalGroups"); + assertThat(configuration.getCaches().get(0).getMaximumSize()).isEqualTo(1000); + } + + @Nested + class WithProperty { + + @BeforeEach + void setProperty() { + System.setProperty("scm.cache.externalGroups.maximumSize", "42"); + } + + @AfterEach + void removeProperty() { + System.clearProperty("scm.cache.externalGroups.maximumSize"); + } + + @Test + void shouldTakeValueFromPropertyIfPresent() throws JAXBException { + GuavaCacheManagerConfiguration configuration = readConfiguration(); + + assertThat(configuration.getCaches().get(0).getMaximumSize()).isEqualTo(42); + } + } + + private static GuavaCacheManagerConfiguration readConfiguration() throws JAXBException { + JAXBContext context = JAXBContext.newInstance(GuavaCacheManagerConfiguration.class); + return (GuavaCacheManagerConfiguration) context + .createUnmarshaller() + .unmarshal(new StringReader(CACHE_CONFIGURATION)); + } +}