From 7a1de0f67b243cafdd8be8cb9c6c153516ef89ff Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Tue, 27 Nov 2018 11:35:02 +0100 Subject: [PATCH 01/38] add the interface StoreFactory and refactor storeFactories --- .../repository/AbstractRepositoryHandler.java | 10 ++- .../InitialRepositoryLocationResolver.java | 7 +- .../RepositoryLocationResolver.java | 22 ++++++ .../sonia/scm/store/BlobStoreFactory.java | 12 +-- .../store/ConfigurationEntryStoreFactory.java | 23 ++---- .../scm/store/ConfigurationStoreFactory.java | 19 +---- .../sonia/scm/store/DataStoreFactory.java | 15 +--- .../java/sonia/scm/store/StoreFactory.java | 6 ++ .../java/sonia/scm/store/StoreParameters.java | 67 +++++++++++++++++ .../java/sonia/scm/group/xml/XmlGroupDAO.java | 9 ++- .../scm/repository/xml/XmlRepositoryDAO.java | 16 ++-- .../scm/store/FileBasedStoreFactory.java | 49 +++++++++---- .../sonia/scm/store/FileBlobStoreFactory.java | 18 ++--- .../JAXBConfigurationEntryStoreFactory.java | 20 ++++- .../scm/store/JAXBConfigurationStore.java | 2 +- .../store/JAXBConfigurationStoreFactory.java | 73 ++++++++++++++----- .../sonia/scm/store/JAXBDataStoreFactory.java | 55 +++----------- .../java/sonia/scm/user/xml/XmlUserDAO.java | 6 +- .../sonia/scm/store/FileBlobStoreTest.java | 2 +- .../JAXBConfigurationEntryStoreTest.java | 35 ++++----- .../scm/store/JAXBConfigurationStoreTest.java | 4 +- .../sonia/scm/store/JAXBDataStoreTest.java | 25 ++++++- .../scm/web/lfs/LfsBlobStoreFactory.java | 10 ++- .../repository/GitRepositoryHandlerTest.java | 3 + .../scm/web/lfs/LfsBlobStoreFactoryTest.java | 14 +++- .../repository/HgRepositoryHandlerTest.java | 3 + .../scm/repository/SvnRepositoryHandler.java | 1 + .../repository/SvnRepositoryHandlerTest.java | 2 +- .../main/java/sonia/scm/AbstractTestBase.java | 55 +++++++------- .../main/java/sonia/scm/ManagerTestBase.java | 7 +- .../sonia/scm/store/BlobStoreTestBase.java | 29 ++++---- .../ConfigurationEntryStoreTestBase.java | 32 ++++---- .../sonia/scm/store/DataStoreTestBase.java | 41 ++++++++--- .../InMemoryConfigurationStoreFactory.java | 3 +- .../scm/store/KeyValueStoreTestBase.java | 17 +++-- .../java/sonia/scm/store/StoreTestBase.java | 19 +++-- .../scm/security/DefaultSecuritySystem.java | 7 +- .../sonia/scm/security/SecureKeyResolver.java | 7 +- .../resources/AutoCompleteResourceTest.java | 2 +- .../DefaultRepositoryManagerTest.java | 4 +- .../scm/security/SecureKeyResolverTest.java | 23 +++--- .../scm/user/DefaultUserManagerTest.java | 2 +- 42 files changed, 485 insertions(+), 291 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/store/StoreFactory.java create mode 100644 scm-core/src/main/java/sonia/scm/store/StoreParameters.java diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java index 42c8f22a0f..d494858494 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java @@ -48,6 +48,7 @@ import java.io.File; import java.io.IOException; import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; +import sonia.scm.store.StoreParameters; /** @@ -72,9 +73,12 @@ public abstract class AbstractRepositoryHandler * * @param storeFactory */ - protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) - { - this.store = storeFactory.getStore(getConfigClass(), getType().getName()); + protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) { + this.store = storeFactory.getStore(new StoreParameters() + .withType(getConfigClass()) + .withName(getType().getName()) + .build() + ); } //~--- get methods ---------------------------------------------------------- diff --git a/scm-core/src/main/java/sonia/scm/repository/InitialRepositoryLocationResolver.java b/scm-core/src/main/java/sonia/scm/repository/InitialRepositoryLocationResolver.java index 51aba9bc25..a579df4e21 100644 --- a/scm-core/src/main/java/sonia/scm/repository/InitialRepositoryLocationResolver.java +++ b/scm-core/src/main/java/sonia/scm/repository/InitialRepositoryLocationResolver.java @@ -6,6 +6,7 @@ import sonia.scm.io.FileSystem; import javax.inject.Inject; import java.io.File; import java.io.IOException; +import java.text.MessageFormat; /** * @@ -44,7 +45,11 @@ public final class InitialRepositoryLocationResolver { return new File(context.getBaseDirectory(), REPOSITORIES_DIRECTORY); } - public File createDirectory(Repository repository) throws IOException { + File getContextBaseDirectory() { + return context.getBaseDirectory(); + } + + public File createDirectory(Repository repository) throws IOException { File initialRepoFolder = getDirectory(repository); fileSystem.create(initialRepoFolder); return initialRepoFolder; diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java index 5515c3eb88..64d38fbe6d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryLocationResolver.java @@ -24,6 +24,10 @@ import static sonia.scm.repository.InitialRepositoryLocationResolver.REPOSITORIE @Singleton public class RepositoryLocationResolver { + private static final String REPOSITORIES_STORES_DIRECTORY = "stores"; + private static final String REPOSITORIES_CONFIG_DIRECTORY = "config"; + private static final String GLOBAL_STORE_BASE_DIRECTORY = "var"; + private RepositoryDAO repositoryDAO; private InitialRepositoryLocationResolver initialRepositoryLocationResolver; @@ -70,4 +74,22 @@ public class RepositoryLocationResolver { public File getInitialNativeDirectory(Repository repository) { return new File (getInitialDirectory(repository), REPOSITORIES_NATIVE_DIRECTORY); } + + /** + * Get the store directory of a specific repository + * @param repository + * @return the store directory of a specific repository + */ + public File getStoresDirectory(Repository repository) throws IOException{ + return new File (getRepositoryDirectory(repository), REPOSITORIES_STORES_DIRECTORY); + } + + public File getConfigDirectory(Repository repository) throws IOException { + return new File (getRepositoryDirectory(repository), REPOSITORIES_CONFIG_DIRECTORY); + } + + public File getGlobalStoreDirectory() { + return new File(initialRepositoryLocationResolver.getContextBaseDirectory(), + GLOBAL_STORE_BASE_DIRECTORY.concat(File.separator)); + } } diff --git a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java index 941b3923d1..7d0462d371 100644 --- a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java @@ -42,16 +42,6 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.BlobStore */ -public interface BlobStoreFactory { +public interface BlobStoreFactory extends StoreFactory { - /** - * Returns a {@link BlobStore} with the given name, if the {@link BlobStore} - * with the given name does not exists the factory will create a new one. - * - * - * @param name name of the {@link BlobStore} - * - * @return {@link BlobStore} with the given name - */ - public BlobStore getBlobStore(String name); } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index 7cfebd69c1..4a1202e28c 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -34,10 +34,10 @@ package sonia.scm.store; /** * The ConfigurationEntryStoreFactory can be used to create new or get existing - * {@link ConfigurationEntryStore}s. Note: the default implementation - * uses the same location as the {@link StoreFactory}, so be sure that the - * store names are unique for all {@link ConfigurationEntryStore}s and - * {@link Store}s. + * {@link ConfigurationEntryStore}s. + * + * Note: the default implementation uses the same location as the {@link ConfigurationStoreFactory}, so be sure that the + * store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationStore}s. * * @author Sebastian Sdorra * @since 1.31 @@ -45,18 +45,5 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.ConfigurationEntryStore */ -public interface ConfigurationEntryStoreFactory -{ - - /** - * Get an existing {@link ConfigurationEntryStore} or create a new one. - * - * - * @param type type of the store objects - * @param name name of the store - * @param type of the store objects - * - * @return {@link ConfigurationEntryStore} with given name and type - */ - public ConfigurationEntryStore getStore(Class type, String name); +public interface ConfigurationEntryStoreFactory extends StoreFactory { } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index d9a97de98d..6cf9541139 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -37,23 +37,12 @@ package sonia.scm.store; * The ConfigurationStoreFactory can be used to create new or get existing * {@link ConfigurationStore} objects. * + * Note: the default implementation uses the same location as the {@link ConfigurationEntryStoreFactory}, so be sure that the + * store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationStore}s. + * * @author Sebastian Sdorra * * @apiviz.landmark * @apiviz.uses sonia.scm.store.ConfigurationStore */ -public interface ConfigurationStoreFactory -{ - - /** - * Get an existing {@link ConfigurationStore} or create a new one. - * - * - * @param type type of the store objects - * @param name name of the store - * @param type of the store objects - * - * @return {@link ConfigurationStore} of the given type and name - */ - public ConfigurationStore getStore(Class type, String name); -} +public interface ConfigurationStoreFactory extends StoreFactory{} diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index caed974ee4..c1e171a74b 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -42,17 +42,4 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.DataStore */ -public interface DataStoreFactory { - - /** - * Get an existing {@link DataStore} or create a new one. - * - * - * @param type type of the store objects - * @param name name of the store - * @param type of the store objects - * - * @return {@link DataStore} with given name and type - */ - public DataStore getStore(Class type, String name); -} +public interface DataStoreFactory extends StoreFactory{} diff --git a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java new file mode 100644 index 0000000000..a40e8cfeba --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java @@ -0,0 +1,6 @@ +package sonia.scm.store; + +public interface StoreFactory { + + STORE getStore(final StoreParameters storeParameters); +} diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java new file mode 100644 index 0000000000..b625004d42 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -0,0 +1,67 @@ +package sonia.scm.store; + +import sonia.scm.repository.Repository; + +/** + * The fields of the {@link StoreParameters} are used from the {@link StoreFactory} to create a store. + * + * @author Mohamed Karray + * @since 2.0.0 + */ +public class StoreParameters { + + private Class type; + private String name; + private Repository repository; + + public Class getType() { + return type; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public WithType withType(Class type){ + return new WithType(type); + } + + public class WithType { + + private WithType(Class type) { + StoreParameters.this.type = type; + } + + public WithTypeAndName withName(String name){ + return new WithTypeAndName(name); + } + + } + public class WithTypeAndName { + + private WithTypeAndName(String name) { + StoreParameters.this.name = name; + } + + public WithTypeNameAndRepository forRepository(Repository repository){ + return new WithTypeNameAndRepository(repository); + } + public StoreParameters build(){ + return StoreParameters.this; + } + } + + public class WithTypeNameAndRepository { + + private WithTypeNameAndRepository(Repository repository) { + StoreParameters.this.repository = repository; + } + public StoreParameters build(){ + return StoreParameters.this; + } + } +} diff --git a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java index 577d732317..2b416cf53a 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java @@ -39,6 +39,7 @@ import com.google.inject.Singleton; import sonia.scm.group.Group; import sonia.scm.group.GroupDAO; +import sonia.scm.store.StoreParameters; import sonia.scm.xml.AbstractXmlDAO; import sonia.scm.store.ConfigurationStoreFactory; @@ -64,9 +65,11 @@ public class XmlGroupDAO extends AbstractXmlDAO * @param storeFactory */ @Inject - public XmlGroupDAO(ConfigurationStoreFactory storeFactory) - { - super(storeFactory.getStore(XmlGroupDatabase.class, STORE_NAME)); + public XmlGroupDAO(ConfigurationStoreFactory storeFactory) { + super(storeFactory.getStore(new StoreParameters() + .withType(XmlGroupDatabase.class) + .withName(STORE_NAME) + .build())); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java index eadf277dd3..cdeb1adfa4 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java @@ -36,14 +36,18 @@ package sonia.scm.repository.xml; import com.google.inject.Inject; import com.google.inject.Singleton; +import sonia.scm.SCMContextProvider; import sonia.scm.repository.InitialRepositoryLocationResolver; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.PathBasedRepositoryDAO; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPathNotFoundException; -import sonia.scm.store.ConfigurationStoreFactory; +import sonia.scm.store.JAXBConfigurationStore; +import sonia.scm.store.StoreConstants; +import sonia.scm.util.IOUtil; import sonia.scm.xml.AbstractXmlDAO; +import java.io.File; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collection; @@ -65,14 +69,10 @@ public class XmlRepositoryDAO //~--- constructors --------------------------------------------------------- - /** - * Constructs ... - * - * @param storeFactory - */ @Inject - public XmlRepositoryDAO(ConfigurationStoreFactory storeFactory, InitialRepositoryLocationResolver initialRepositoryLocationResolver) { - super(storeFactory.getStore(XmlRepositoryDatabase.class, STORE_NAME)); + public XmlRepositoryDAO(InitialRepositoryLocationResolver initialRepositoryLocationResolver, SCMContextProvider context) { + super(new JAXBConfigurationStore<>(XmlRepositoryDatabase.class, new File(context.getBaseDirectory(), StoreConstants.CONFIG_DIRECTORY_NAME + File.separator + STORE_NAME + StoreConstants.FILE_EXTENSION))); + IOUtil.mkdirs(new File(context.getBaseDirectory(), StoreConstants.CONFIG_DIRECTORY_NAME + File.separator + STORE_NAME + StoreConstants.FILE_EXTENSION)); this.initialRepositoryLocationResolver = initialRepositoryLocationResolver; } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java index 5bfc4f34b9..f9d7f49e48 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java @@ -31,18 +31,23 @@ package sonia.scm.store; //~--- non-JDK imports -------------------------------------------------------- + import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import sonia.scm.SCMContextProvider; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.util.IOUtil; -//~--- JDK imports ------------------------------------------------------------ import java.io.File; +import java.io.IOException; +import java.text.MessageFormat; + +//~--- JDK imports ------------------------------------------------------------ /** * Abstract store factory for file based stores. - * + * * @author Sebastian Sdorra */ public abstract class FileBasedStoreFactory { @@ -51,18 +56,14 @@ public abstract class FileBasedStoreFactory { * the logger for FileBasedStoreFactory */ private static final Logger LOG = LoggerFactory.getLogger(FileBasedStoreFactory.class); - - private static final String BASE_DIRECTORY = "var"; - - private final SCMContextProvider context; + private RepositoryLocationResolver repositoryLocationResolver; private final String dataDirectoryName; private File dataDirectory; - protected FileBasedStoreFactory(SCMContextProvider context, - String dataDirectoryName) { - this.context = context; + protected FileBasedStoreFactory(RepositoryLocationResolver repositoryLocationResolver, String dataDirectoryName) { + this.repositoryLocationResolver = repositoryLocationResolver; this.dataDirectoryName = dataDirectoryName; } @@ -76,9 +77,8 @@ public abstract class FileBasedStoreFactory { */ protected File getDirectory(String name) { if (dataDirectory == null) { - dataDirectory = new File(context.getBaseDirectory(), - BASE_DIRECTORY.concat(File.separator).concat(dataDirectoryName)); - LOG.debug("create data directory {}", dataDirectory); + dataDirectory = new File(repositoryLocationResolver.getGlobalStoreDirectory(), dataDirectoryName); + LOG.debug("get data directory {}", dataDirectory); } File storeDirectory = new File(dataDirectory, name); @@ -86,4 +86,25 @@ public abstract class FileBasedStoreFactory { return storeDirectory; } + /** + * Returns data directory for given name. + * + * @param name name of data directory + * + * @return data directory + */ + protected File getDirectory(String name, Repository repository) { + if (dataDirectory == null) { + try { + dataDirectory = new File(repositoryLocationResolver.getStoresDirectory(repository), dataDirectoryName); + } catch (IOException e) { + throw new InternalRepositoryException(repository, MessageFormat.format("Error on getting the store directory {0} of the repository {1}", dataDirectory.getAbsolutePath(), repository.getNamespaceAndName()), e); + } + LOG.debug("create data directory {}", dataDirectory); + } + File storeDirectory = new File(dataDirectory, name); + IOUtil.mkdirs(storeDirectory); + return storeDirectory; + } + } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileBlobStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileBlobStoreFactory.java index 8cc5b34ac2..4440cfc908 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileBlobStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileBlobStoreFactory.java @@ -37,7 +37,7 @@ import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.SCMContextProvider; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.security.KeyGenerator; /** @@ -60,21 +60,21 @@ public class FileBlobStoreFactory extends FileBasedStoreFactory implements BlobS /** * Constructs a new instance. * - * @param context scm context + * @param repositoryLocationResolver * @param keyGenerator key generator */ @Inject - public FileBlobStoreFactory(SCMContextProvider context, - KeyGenerator keyGenerator) { - super(context, DIRECTORY_NAME); + public FileBlobStoreFactory(RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator) { + super(repositoryLocationResolver, DIRECTORY_NAME); this.keyGenerator = keyGenerator; } @Override - public BlobStore getBlobStore(String name) { - LOG.debug("create new blob with name {}", name); - - return new FileBlobStore(keyGenerator, getDirectory(name)); + public BlobStore getStore(StoreParameters storeParameters) { + if (storeParameters.getRepository() != null) { + return new FileBlobStore(keyGenerator, getDirectory(storeParameters.getName(), storeParameters.getRepository())); + } + return new FileBlobStore(keyGenerator, getDirectory(storeParameters.getName())); } } 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 261c36f8e4..e8f4028f50 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 @@ -42,6 +42,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.SCMContextProvider; +import sonia.scm.repository.Repository; import sonia.scm.security.KeyGenerator; import sonia.scm.util.IOUtil; @@ -91,18 +92,16 @@ public class JAXBConfigurationEntryStoreFactory * * @param type * @param name - * @param * * @return */ - @Override - public ConfigurationEntryStore getStore(Class type, String name) + private ConfigurationEntryStore getStore(Class type, String name) { logger.debug("create new configuration store for type {} with name {}", type, name); //J- - return new JAXBConfigurationEntryStore( + return new JAXBConfigurationEntryStore( new File(directory,name.concat(StoreConstants.FILE_EXTENSION)), keyGenerator, type @@ -117,4 +116,17 @@ public class JAXBConfigurationEntryStoreFactory /** Field description */ private KeyGenerator keyGenerator; + + @Override + public ConfigurationEntryStore getStore(StoreParameters storeParameters) { + if (storeParameters.getRepository() != null){ + return getStore(storeParameters.getType(),storeParameters.getName(),storeParameters.getRepository()); + } + return getStore(storeParameters.getType(),storeParameters.getName()); + } + + private ConfigurationEntryStore getStore(Class type, String name, Repository repository) { + return null; + } + } diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java index 4a87ea57f6..ac1477d7ea 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/JAXBConfigurationStore.java @@ -61,7 +61,7 @@ public class JAXBConfigurationStore extends AbstractStore { private JAXBContext context; - JAXBConfigurationStore(Class type, File configFile) { + public JAXBConfigurationStore(Class type, File configFile) { this.type = type; try { 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 705b0c1177..8b6ad81756 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 @@ -32,17 +32,18 @@ package sonia.scm.store; import com.google.inject.Inject; import com.google.inject.Singleton; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import sonia.scm.SCMContextProvider; +import sonia.scm.repository.InternalRepositoryException; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.util.IOUtil; import java.io.File; +import java.io.IOException; /** - * JAXB implementation of {@link ConfigurationStoreFactory}. + * JAXB implementation of {@link StoreFactory}. * * @author Sebastian Sdorra */ @@ -54,33 +55,65 @@ public class JAXBConfigurationStoreFactory implements ConfigurationStoreFactory */ private static final Logger LOG = LoggerFactory.getLogger(JAXBConfigurationStoreFactory.class); - private final File configDirectory; + private RepositoryLocationResolver repositoryLocationResolver; /** * Constructs a new instance. * - * @param context scm context + * @param repositoryLocationResolver Resolver to get the repository Directory */ @Inject - public JAXBConfigurationStoreFactory(SCMContextProvider context) { - configDirectory = new File(context.getBaseDirectory(), StoreConstants.CONFIG_DIRECTORY_NAME); - IOUtil.mkdirs(configDirectory); + public JAXBConfigurationStoreFactory(RepositoryLocationResolver repositoryLocationResolver) { + this.repositoryLocationResolver = repositoryLocationResolver; } - @Override - public JAXBConfigurationStore getStore(Class type, String name) { - if (configDirectory == null) { - throw new IllegalStateException("store factory is not initialized"); - } + /** + * Get or create the global config directory. + * + * @return the global config directory. + */ + private File getGlobalConfigDirectory() { + File baseDirectory = repositoryLocationResolver.getInitialBaseDirectory(); + File configDirectory = new File(baseDirectory, StoreConstants.CONFIG_DIRECTORY_NAME); + return getOrCreateFile(configDirectory); + } + /** + * Get or create the repository specific config directory. + * + * @return the repository specific config directory. + */ + private File getRepositoryConfigDirectory(Repository repository) { + File baseDirectory = null; + try { + baseDirectory = repositoryLocationResolver.getConfigDirectory(repository); + } catch (IOException e) { + e.printStackTrace(); + } + File configDirectory = new File(baseDirectory, StoreConstants.CONFIG_DIRECTORY_NAME); + return getOrCreateFile(configDirectory); + } + + private File getOrCreateFile(File directory) { + if (!directory.exists()) { + IOUtil.mkdirs(directory); + } + return directory; + } + + private JAXBConfigurationStore getStore(Class type, String name, File configDirectory) { File configFile = new File(configDirectory, name.concat(StoreConstants.FILE_EXTENSION)); - - if (LOG.isDebugEnabled()) { - LOG.debug("create store for {} at {}", type.getName(), - configFile.getPath()); - } - + LOG.debug("create store for {} at {}", type.getName(), configFile.getPath()); return new JAXBConfigurationStore<>(type, configFile); } + @Override + public JAXBConfigurationStore getStore(StoreParameters storeParameters) { + try { + return getStore(storeParameters.getType(), storeParameters.getName(),repositoryLocationResolver.getRepositoryDirectory(storeParameters.getRepository())); + } catch (IOException e) { + + throw new InternalRepositoryException(storeParameters.getRepository(),"Error on getting the store of the repository"+ storeParameters.getRepository().getNamespaceAndName()); + } + } } 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 732b8c675b..f5199d8bd2 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 @@ -40,7 +40,7 @@ import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.SCMContextProvider; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.security.KeyGenerator; /** @@ -52,54 +52,23 @@ public class JAXBDataStoreFactory extends FileBasedStoreFactory implements DataStoreFactory { - /** Field description */ + private static final Logger logger = LoggerFactory.getLogger(JAXBDataStoreFactory.class); private static final String DIRECTORY_NAME = "data"; + private KeyGenerator keyGenerator; - /** - * the logger for JAXBDataStoreFactory - */ - private static final Logger logger = - LoggerFactory.getLogger(JAXBDataStoreFactory.class); - - //~--- constructors --------------------------------------------------------- - - /** - * Constructs ... - * - * - * @param context - * @param keyGenerator - */ @Inject - public JAXBDataStoreFactory(SCMContextProvider context, - KeyGenerator keyGenerator) - { - super(context, DIRECTORY_NAME); + public JAXBDataStoreFactory(RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator) { + super(repositoryLocationResolver, DIRECTORY_NAME); this.keyGenerator = keyGenerator; } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param type - * @param name - * @param - * - * @return - */ @Override - public DataStore getStore(Class type, String name) - { - logger.debug("create new store for type {} with name {}", type, name); - - return new JAXBDataStore<>(keyGenerator, type, getDirectory(name)); + @SuppressWarnings("unchecked") + public DataStore getStore(StoreParameters storeParameters) { + logger.debug("create new store for type {} with name {}", storeParameters.getType(), storeParameters.getName()); + if (storeParameters.getRepository() != null) { + return new JAXBDataStore(keyGenerator, storeParameters.getType(), getDirectory(storeParameters.getName(), storeParameters.getRepository())); + } + return new JAXBDataStore(keyGenerator, storeParameters.getType(), getDirectory(storeParameters.getName())); } - - //~--- fields --------------------------------------------------------------- - - /** Field description */ - private KeyGenerator keyGenerator; } diff --git a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java index 1bfd877f44..5f84aeb3a8 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java @@ -37,6 +37,7 @@ package sonia.scm.user.xml; import com.google.inject.Inject; import com.google.inject.Singleton; +import sonia.scm.store.StoreParameters; import sonia.scm.user.User; import sonia.scm.user.UserDAO; import sonia.scm.xml.AbstractXmlDAO; @@ -65,7 +66,10 @@ public class XmlUserDAO extends AbstractXmlDAO @Inject public XmlUserDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.getStore(XmlUserDatabase.class, STORE_NAME)); + super(storeFactory.getStore(new StoreParameters() + .withType(XmlUserDatabase.class) + .withName(STORE_NAME) + .build())); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java index cae872538d..1df829588a 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java @@ -52,6 +52,6 @@ public class FileBlobStoreTest extends BlobStoreTestBase @Override protected BlobStoreFactory createBlobStoreFactory() { - return new FileBlobStoreFactory(contextProvider, new UUIDKeyGenerator()); + return new FileBlobStoreFactory(repositoryLocationResolver, new UUIDKeyGenerator()); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java index d0f17fc313..f520ce88e8 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java @@ -37,25 +37,22 @@ package sonia.scm.store; import com.google.common.io.Closeables; import com.google.common.io.Resources; - import org.junit.Test; - import sonia.scm.security.AssignedPermission; import sonia.scm.security.UUIDKeyGenerator; -import static org.junit.Assert.*; - -//~--- JDK imports ------------------------------------------------------------ - import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; - import java.net.URL; - import java.util.UUID; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -132,13 +129,13 @@ public class JAXBConfigurationEntryStoreTest public void testStoreAndLoad() throws IOException { String name = UUID.randomUUID().toString(); - ConfigurationEntryStore store = - createPermissionStore(RESOURCE_FIXED, name); + ConfigurationEntryStore store = createPermissionStore(RESOURCE_FIXED, name); store.put("a45", new AssignedPermission("tuser4", "repository:create")); - store = - createConfigurationStoreFactory().getStore(AssignedPermission.class, - name); + store = createConfigurationStoreFactory().getStore(new StoreParameters() + .withType(AssignedPermission.class) + .withName(name) + .build()); AssignedPermission ap = store.get("a45"); @@ -154,10 +151,9 @@ public class JAXBConfigurationEntryStoreTest * @return */ @Override - protected ConfigurationEntryStoreFactory createConfigurationStoreFactory() + protected ConfigurationEntryStoreFactory createConfigurationStoreFactory() { - return new JAXBConfigurationEntryStoreFactory(new UUIDKeyGenerator(), - contextProvider); + return new JAXBConfigurationEntryStoreFactory(new UUIDKeyGenerator(), contextProvider); } /** @@ -225,8 +221,9 @@ public class JAXBConfigurationEntryStoreTest } copy(resource, name); - - return createConfigurationStoreFactory().getStore(AssignedPermission.class, - name); + return createConfigurationStoreFactory().getStore(new StoreParameters() + .withType(AssignedPermission.class) + .withName(name) + .build()); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java index 4151a6ca20..4f3b99efe0 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java @@ -34,7 +34,7 @@ package sonia.scm.store; /** * Unit tests for {@link JAXBConfigurationStore}. - * + * * @author Sebastian Sdorra */ public class JAXBConfigurationStoreTest extends StoreTestBase { @@ -42,6 +42,6 @@ public class JAXBConfigurationStoreTest extends StoreTestBase { @Override protected ConfigurationStoreFactory createStoreFactory() { - return new JAXBConfigurationStoreFactory(contextProvider); + return new JAXBConfigurationStoreFactory(repositoryLocationResolver); } } 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 9834a48916..e050772e9e 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 @@ -34,14 +34,14 @@ package sonia.scm.store; //~--- non-JDK imports -------------------------------------------------------- +import sonia.scm.repository.Repository; import sonia.scm.security.UUIDKeyGenerator; /** * * @author Sebastian Sdorra */ -public class JAXBDataStoreTest extends DataStoreTestBase -{ +public class JAXBDataStoreTest extends DataStoreTestBase { /** * Method description @@ -52,6 +52,25 @@ public class JAXBDataStoreTest extends DataStoreTestBase @Override protected DataStoreFactory createDataStoreFactory() { - return new JAXBDataStoreFactory(contextProvider, new UUIDKeyGenerator()); + return new JAXBDataStoreFactory(repositoryLocationResolver, new UUIDKeyGenerator()); + } + + @Override + protected DataStore getDataStore(Class type, Repository repository) { + StoreParameters params = new StoreParameters() + .withType(type) + .withName("test") + .forRepository(repository) + .build(); + return createDataStoreFactory().getStore(params); + } + + @Override + protected DataStore getDataStore(Class type) { + StoreParameters params = new StoreParameters() + .withType(type) + .withName("test") + .build(); + return createDataStoreFactory().getStore(params); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java index eebd6b8f2b..3e4ba67e0f 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java @@ -35,8 +35,10 @@ package sonia.scm.web.lfs; import com.google.inject.Inject; import com.google.inject.Singleton; import sonia.scm.repository.Repository; +import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; import sonia.scm.store.BlobStoreFactory; +import sonia.scm.store.StoreParameters; /** * Creates {@link BlobStore} objects to store lfs objects. @@ -74,7 +76,13 @@ public class LfsBlobStoreFactory { * * @return blob store for the corresponding scm repository */ + @SuppressWarnings("unchecked") public BlobStore getLfsBlobStore(Repository repository) { - return blobStoreFactory.getBlobStore(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX); + return blobStoreFactory.getStore(new StoreParameters() + .withType(Blob.class) + .withName(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX) + .forRepository(repository) + .build() + ); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index ace6756cad..3e2734f45e 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -39,9 +39,12 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.io.DefaultFileSystem; import sonia.scm.schedule.Scheduler; +import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; +import sonia.scm.store.StoreFactory; import java.io.File; +import java.io.IOException; import java.nio.file.Path; import static org.junit.Assert.assertEquals; diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java index 991e2655f7..8cc36a2ec4 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java @@ -40,7 +40,8 @@ import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.repository.Repository; import sonia.scm.store.BlobStoreFactory; -import static org.mockito.Matchers.matches; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -59,12 +60,17 @@ public class LfsBlobStoreFactoryTest { private LfsBlobStoreFactory lfsBlobStoreFactory; @Test - public void getBlobStore() throws Exception { - lfsBlobStoreFactory.getLfsBlobStore(new Repository("the-id", "GIT", "space", "the-name")); + public void getBlobStore() { + Repository repository = new Repository("the-id", "GIT", "space", "the-name"); + lfsBlobStoreFactory.getLfsBlobStore(repository); // just make sure the right parameter is passed, as properly validating the return value is nearly impossible with // the return value (and should not be part of this test) - verify(blobStoreFactory).getBlobStore(matches("the-id-git-lfs")); + verify(blobStoreFactory).getStore(argThat(blobStoreParameters -> { + assertThat(blobStoreParameters.getName()).isEqualTo("the-id-git-lfs"); + assertThat(blobStoreParameters.getRepository()).isEqualTo(repository); + return true; + })); // make sure there have been no further usages of the factory verifyNoMoreInteractions(blobStoreFactory); diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java index 408f7de24d..76a3266558 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java @@ -39,9 +39,12 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.io.DefaultFileSystem; +import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; +import sonia.scm.store.StoreFactory; import java.io.File; +import java.io.IOException; import java.nio.file.Path; import static org.junit.Assert.assertEquals; diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java index 35fd59f715..d13de30599 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java @@ -51,6 +51,7 @@ import sonia.scm.logging.SVNKitLogger; import sonia.scm.plugin.Extension; import sonia.scm.repository.spi.HookEventFacade; import sonia.scm.repository.spi.SvnRepositoryServiceProvider; +import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; import sonia.scm.util.Util; diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java index 3c644056ca..bfb2aac896 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java @@ -115,7 +115,7 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { - when(factory.getStore(any(), any())).thenReturn(store); + when(factory.getStore(any())).thenReturn(store); SvnRepositoryHandler repositoryHandler = new SvnRepositoryHandler(factory, new DefaultFileSystem(), facade, repositoryLocationResolver); diff --git a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java index 13cde0391e..f92c406c22 100644 --- a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java +++ b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java @@ -46,10 +46,15 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; +import sonia.scm.io.DefaultFileSystem; +import sonia.scm.repository.InitialRepositoryLocationResolver; +import sonia.scm.repository.RepositoryDAO; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.util.IOUtil; import sonia.scm.util.MockUtil; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; //~--- JDK imports ------------------------------------------------------------ @@ -66,10 +71,29 @@ import java.util.logging.Logger; public class AbstractTestBase { - /** Field description */ private static ThreadState subjectThreadState; - //~--- methods -------------------------------------------------------------- + protected SCMContextProvider contextProvider; + + private File tempDirectory; + + protected DefaultFileSystem fileSystem; + + protected RepositoryDAO repositoryDAO = mock(RepositoryDAO.class); + protected RepositoryLocationResolver repositoryLocationResolver; + + @Before + public void setUpTest() throws Exception + { + tempDirectory = new File(System.getProperty("java.io.tmpdir"), + UUID.randomUUID().toString()); + assertTrue(tempDirectory.mkdirs()); + contextProvider = MockUtil.getSCMContextProvider(tempDirectory); + fileSystem = new DefaultFileSystem(); + InitialRepositoryLocationResolver initialRepoLocationResolver = new InitialRepositoryLocationResolver(contextProvider,fileSystem); + repositoryLocationResolver = new RepositoryLocationResolver(repositoryDAO, initialRepoLocationResolver); + postSetUp(); + } /** * Method description @@ -165,25 +189,6 @@ public class AbstractTestBase } } - //~--- set methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @throws Exception - */ - @Before - public void setUpTest() throws Exception - { - tempDirectory = new File(System.getProperty("java.io.tmpdir"), - UUID.randomUUID().toString()); - assertTrue(tempDirectory.mkdirs()); - contextProvider = MockUtil.getSCMContextProvider(tempDirectory); - postSetUp(); - } - - //~--- methods -------------------------------------------------------------- /** * Clears Shiro's thread state, ensuring the thread remains clean for @@ -249,12 +254,4 @@ public class AbstractTestBase subjectThreadState = createThreadState(subject); subjectThreadState.bind(); } - - //~--- fields --------------------------------------------------------------- - - /** Field description */ - protected SCMContextProvider contextProvider; - - /** Field description */ - private File tempDirectory; } diff --git a/scm-test/src/main/java/sonia/scm/ManagerTestBase.java b/scm-test/src/main/java/sonia/scm/ManagerTestBase.java index eda3182638..65f696067b 100644 --- a/scm-test/src/main/java/sonia/scm/ManagerTestBase.java +++ b/scm-test/src/main/java/sonia/scm/ManagerTestBase.java @@ -37,10 +37,13 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.rules.TemporaryFolder; +import sonia.scm.repository.RepositoryLocationResolver; import sonia.scm.util.MockUtil; import java.io.IOException; +import static org.mockito.Mockito.mock; + /** * * @author Sebastian Sdorra @@ -54,12 +57,14 @@ public abstract class ManagerTestBase public TemporaryFolder tempFolder = new TemporaryFolder(); protected SCMContextProvider contextProvider; - + protected RepositoryLocationResolver locationResolver; + protected Manager manager; @Before public void setUp() throws IOException { contextProvider = MockUtil.getSCMContextProvider(tempFolder.newFolder()); + locationResolver = mock(RepositoryLocationResolver.class); manager = createManager(); manager.init(contextProvider); } diff --git a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java index 48504feaf2..e30fa4de33 100644 --- a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java @@ -35,22 +35,24 @@ package sonia.scm.store; //~--- non-JDK imports -------------------------------------------------------- import com.google.common.io.ByteStreams; - import org.junit.Before; import org.junit.Test; - import sonia.scm.AbstractTestBase; - -import static org.junit.Assert.*; - -//~--- JDK imports ------------------------------------------------------------ +import sonia.scm.repository.RepositoryTestData; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; - import java.util.List; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -58,12 +60,6 @@ import java.util.List; public abstract class BlobStoreTestBase extends AbstractTestBase { - /** - * Method description - * - * - * @return - */ protected abstract BlobStoreFactory createBlobStoreFactory(); /** @@ -73,7 +69,12 @@ public abstract class BlobStoreTestBase extends AbstractTestBase @Before public void createBlobStore() { - store = createBlobStoreFactory().getBlobStore("test"); + store = createBlobStoreFactory().getStore(new StoreParameters() + .withType(Blob.class) + .withName("test") + .forRepository(RepositoryTestData.createHeartOfGold()) + .build() + ); store.clear(); } diff --git a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java index 8d3a63717a..55c92b2376 100644 --- a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java @@ -32,12 +32,13 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + /** * * @author Sebastian Sdorra */ -public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestBase -{ +public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestBase { /** * Method description @@ -48,17 +49,22 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB protected abstract ConfigurationEntryStoreFactory createConfigurationStoreFactory(); //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @return - */ @Override - protected ConfigurationEntryStore getDataStore() - { - return createConfigurationStoreFactory().getStore(StoreObject.class, - "test"); + protected ConfigurationEntryStore getDataStore(Class type) { + StoreParameters params = new StoreParameters() + .withType(type) + .withName("test") + .build(); + return this.createConfigurationStoreFactory().getStore(params); + } + + @Override + protected ConfigurationEntryStore getDataStore(Class type, Repository repository) { + StoreParameters params = new StoreParameters() + .withType(type) + .withName("test") + .forRepository(repository) + .build(); + return this.createConfigurationStoreFactory().getStore(params); } } diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index 3129d3a339..839baaa616 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -33,6 +33,13 @@ package sonia.scm.store; +import org.junit.Test; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryTestData; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + /** * * @author Sebastian Sdorra @@ -48,17 +55,33 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase */ protected abstract DataStoreFactory createDataStoreFactory(); + + protected StoreParameters getStoreParametersWithRepository(Repository repository) { + return new StoreParameters() + .withType(StoreObject.class) + .withName("test") + .forRepository(repository) + .build(); + } //~--- get methods ---------------------------------------------------------- - /** - * Method description - * - * - * @return - */ - @Override - protected DataStore getDataStore() + + + + @Test + // TODO + public void shouldStoreRepositorySpecificData() { - return createDataStoreFactory().getStore(StoreObject.class, "test"); + StoreFactory dataStoreFactory = createDataStoreFactory(); + StoreObject obj = new StoreObject("test-1"); + Repository repository = RepositoryTestData.createHeartOfGold(); + + DataStore store = dataStoreFactory.getStore(getStoreParametersWithRepository(repository)); + + String id = store.put(obj); + + assertNotNull(id); + + assertEquals(obj, store.get(id)); } } diff --git a/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java b/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java index d5e9474ff5..021b7bda02 100644 --- a/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java +++ b/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java @@ -43,8 +43,7 @@ package sonia.scm.store; public class InMemoryConfigurationStoreFactory implements ConfigurationStoreFactory { @Override - public ConfigurationStore getStore(Class type, String name) - { + public ConfigurationStore getStore(StoreParameters storeParameters) { return new InMemoryConfigurationStore<>(); } } diff --git a/scm-test/src/main/java/sonia/scm/store/KeyValueStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/KeyValueStoreTestBase.java index 0abad4f558..b1cdf8431e 100644 --- a/scm-test/src/main/java/sonia/scm/store/KeyValueStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/KeyValueStoreTestBase.java @@ -38,6 +38,8 @@ import org.junit.Before; import org.junit.Test; import sonia.scm.AbstractTestBase; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryTestData; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -56,13 +58,19 @@ import java.util.Map; public abstract class KeyValueStoreTestBase extends AbstractTestBase { + private Repository repository = RepositoryTestData.createHeartOfGold(); + private DataStore store; + private DataStore repoStore; + /** * Method description * * * @return */ - protected abstract DataStore getDataStore(); + protected abstract DataStore getDataStore(Class type , Repository repository); + protected abstract DataStore getDataStore(Class type ); + //~--- methods -------------------------------------------------------------- @@ -73,8 +81,10 @@ public abstract class KeyValueStoreTestBase extends AbstractTestBase @Before public void before() { - store = getDataStore(); + store = getDataStore(StoreObject.class); + repoStore = getDataStore(StoreObject.class, repository); store.clear(); + repoStore.clear(); } /** @@ -215,8 +225,5 @@ public abstract class KeyValueStoreTestBase extends AbstractTestBase assertNull(store.get("2")); } - //~--- fields --------------------------------------------------------------- - /** Field description */ - private DataStore store; } diff --git a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java index c39efa3ffe..fd9afaff08 100644 --- a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java @@ -35,10 +35,12 @@ package sonia.scm.store; //~--- non-JDK imports -------------------------------------------------------- import org.junit.Test; - import sonia.scm.AbstractTestBase; +import sonia.scm.repository.Repository; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; //~--- JDK imports ------------------------------------------------------------ @@ -58,6 +60,13 @@ public abstract class StoreTestBase extends AbstractTestBase */ protected abstract ConfigurationStoreFactory createStoreFactory(); + protected StoreParameters getStoreParameters() { + return new StoreParameters() + .withType(StoreObject.class) + .withName("test") + .build(); + } + /** * Method description * @@ -65,8 +74,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testGet() { - ConfigurationStore store = createStoreFactory().getStore(StoreObject.class, - "test"); + ConfigurationStore store = createStoreFactory().getStore(getStoreParameters()); assertNotNull(store); @@ -82,8 +90,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testSet() { - ConfigurationStore store = createStoreFactory().getStore(StoreObject.class, - "test"); + ConfigurationStore store = createStoreFactory().getStore(getStoreParameters()); assertNotNull(store); diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java index d958dcf41f..afbe3e00d3 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java @@ -55,6 +55,7 @@ import sonia.scm.event.ScmEventBus; import sonia.scm.group.GroupEvent; import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; +import sonia.scm.store.StoreParameters; import sonia.scm.user.UserEvent; import sonia.scm.util.ClassLoaders; @@ -108,9 +109,13 @@ public class DefaultSecuritySystem implements SecuritySystem * @param storeFactory */ @Inject + @SuppressWarnings("unchecked") public DefaultSecuritySystem(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory.getStore(AssignedPermission.class, NAME); + store = storeFactory.getStore(new StoreParameters() + .withType(AssignedPermission.class) + .withName(NAME) + .build()); readAvailablePermissions(); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java index c7c594d5e3..31556afef4 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java @@ -45,6 +45,7 @@ import org.slf4j.LoggerFactory; import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; +import sonia.scm.store.StoreParameters; import static com.google.common.base.Preconditions.*; @@ -87,9 +88,13 @@ public class SecureKeyResolver extends SigningKeyResolverAdapter * @param storeFactory store factory */ @Inject + @SuppressWarnings("unchecked") public SecureKeyResolver(ConfigurationEntryStoreFactory storeFactory) { - this.store = storeFactory.getStore(SecureKey.class, STORE_NAME); + store = storeFactory.getStore(new StoreParameters() + .withType(SecureKey.class) + .withName(STORE_NAME) + .build()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java index 2cab41bfbf..1dc93522a1 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java @@ -66,7 +66,7 @@ public class AutoCompleteResourceTest { ConfigurationStore storeConfig = mock(ConfigurationStore.class); xmlDB = mock(XmlDatabase.class); when(storeConfig.get()).thenReturn(xmlDB); - when(storeFactory.getStore(any(), any())).thenReturn(storeConfig); + when(storeFactory.getStore(any())).thenReturn(storeConfig); XmlUserDAO userDao = new XmlUserDAO(storeFactory); this.userDao = spy(userDao); XmlGroupDAO groupDAO = new XmlGroupDAO(storeFactory); diff --git a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java index ab7e3aafd9..3e2a77ff0f 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java @@ -422,10 +422,10 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { private DefaultRepositoryManager createRepositoryManager(boolean archiveEnabled, KeyGenerator keyGenerator) { DefaultFileSystem fileSystem = new DefaultFileSystem(); Set handlerSet = new HashSet<>(); - ConfigurationStoreFactory factory = new JAXBConfigurationStoreFactory(contextProvider); InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(contextProvider, fileSystem); - XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(factory, initialRepositoryLocationResolver); + XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(initialRepositoryLocationResolver, contextProvider); RepositoryLocationResolver repositoryLocationResolver = new RepositoryLocationResolver(repositoryDAO, initialRepositoryLocationResolver); + ConfigurationStoreFactory factory = new JAXBConfigurationStoreFactory(repositoryLocationResolver); handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver)); handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver) { @Override diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java index 8d708c4677..81b237de2a 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java @@ -36,20 +36,21 @@ package sonia.scm.security; //~--- non-JDK imports -------------------------------------------------------- import io.jsonwebtoken.Jwts; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; - import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; - import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; -import static org.junit.Assert.*; - -import static org.mockito.Mockito.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @@ -122,11 +123,13 @@ public class SecureKeyResolverTest @Before public void setUp() { - ConfigurationEntryStoreFactory factory = - mock(ConfigurationEntryStoreFactory.class); + ConfigurationEntryStoreFactory factory = mock(ConfigurationEntryStoreFactory.class); - when(factory.getStore(SecureKey.class, - SecureKeyResolver.STORE_NAME)).thenReturn(store); + when(factory.getStore(argThat(storeParameters -> { + assertThat(storeParameters.getName()).isEqualTo(SecureKeyResolver.STORE_NAME); + assertThat(storeParameters.getType()).isEqualTo(SecureKey.class); + return true; + }))).thenReturn(store); resolver = new SecureKeyResolver(factory); } diff --git a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java index 1614bf790a..ebfe47a343 100644 --- a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java @@ -182,6 +182,6 @@ public class DefaultUserManagerTest extends UserManagerTestBase //~--- methods -------------------------------------------------------------- private XmlUserDAO createXmlUserDAO() { - return new XmlUserDAO(new JAXBConfigurationStoreFactory(contextProvider)); + return new XmlUserDAO(new JAXBConfigurationStoreFactory(locationResolver)); } } From 0664303854c3d5fa55a47d0b1e58a08eea0e09ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 08:01:02 +0100 Subject: [PATCH 02/38] Use new JWT library --- scm-webapp/pom.xml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 4dfb749690..4503274adb 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -72,8 +72,18 @@ io.jsonwebtoken - jjwt - 0.4 + jjwt-impl + 0.10.5 + + + io.jsonwebtoken + jjwt-api + 0.10.5 + + + io.jsonwebtoken + jjwt-jackson + 0.10.5 From c85c0229c14a7be61b92e678a270fa51e5de47a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 08:01:25 +0100 Subject: [PATCH 03/38] First steps for JWT refresh --- .../java/sonia/scm/security/AccessToken.java | 46 ++++++---- .../scm/security/AccessTokenBuilder.java | 12 ++- .../sonia/scm/security/JwtAccessToken.java | 14 ++- .../scm/security/JwtAccessTokenBuilder.java | 71 +++++++++------ .../JwtAccessTokenRefreshStrategy.java | 5 ++ .../scm/security/JwtAccessTokenRefresher.java | 53 ++++++++++++ .../security/JwtAccessTokenRefresherTest.java | 86 +++++++++++++++++++ .../resources/sonia/scm/repository/shiro.ini | 2 + 8 files changed, 241 insertions(+), 48 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java create mode 100644 scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java create mode 100644 scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java diff --git a/scm-core/src/main/java/sonia/scm/security/AccessToken.java b/scm-core/src/main/java/sonia/scm/security/AccessToken.java index 714b09eff8..ec448ad235 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessToken.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessToken.java @@ -31,6 +31,7 @@ package sonia.scm.security; import java.util.Date; +import java.util.Map; import java.util.Optional; /** @@ -38,70 +39,77 @@ import java.util.Optional; * be issued from a restful webservice endpoint by providing credentials. After the token was issued, the token must be * send along with every request. The token should be send in its compact representation as bearer authorization header * or as cookie. - * + * * @author Sebastian Sdorra * @since 2.0.0 */ public interface AccessToken { - + /** * Returns unique id of the access token. - * + * * @return unique id */ String getId(); - + /** * Returns name of subject which identifies the principal. - * + * * @return name of subject */ String getSubject(); - + /** * Returns optional issuer. The issuer identifies the principal that issued the token. - * + * * @return optional issuer */ Optional getIssuer(); - + /** * Returns time at which the token was issued. - * + * * @return time at which the token was issued */ Date getIssuedAt(); - + /** * Returns the expiration time of token. - * + * * @return expiration time */ Date getExpiration(); - + + Date getRefreshExpiration(); + /** - * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this + * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this * token. For example we could issue a token which can only be used to read a single repository. for more informations * please have a look at {@link Scope}. - * + * * @return scope of token. */ Scope getScope(); - + /** * Returns an optional value of a custom token field. - * + * * @param type of field * @param key key of token field - * + * * @return optional value of custom field */ Optional getCustom(String key); - + /** * Returns compact representation of token. - * + * * @return compact representation */ String compact(); + + /** + * Returns read only map of all claim keys with their values. + */ + Map getClaims(); } diff --git a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java index dd7986c22a..5e36ba468f 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java @@ -74,11 +74,21 @@ public interface AccessTokenBuilder { * Sets the expiration for the token. * * @param count expiration count - * @param unit expirtation unit + * @param unit expiration unit * * @return {@code this} */ AccessTokenBuilder expiresIn(long count, TimeUnit unit); + + /** + * Sets the time how long this token may be refreshed. Set this to 0 (zero) to disable automatic refresh. + * + * @param count Time unit count. If set to 0, automatic refresh is disabled. + * @param unit time unit + * + * @return {@code this} + */ + AccessTokenBuilder refreshableFor(long count, TimeUnit unit); /** * Reduces the permissions of the token by providing a scope. diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 46f4c68e74..35013e4fec 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -31,7 +31,10 @@ package sonia.scm.security; import io.jsonwebtoken.Claims; + +import java.util.Collections; import java.util.Date; +import java.util.Map; import java.util.Optional; /** @@ -75,6 +78,11 @@ public final class JwtAccessToken implements AccessToken { return claims.getExpiration(); } + @Override + public Date getRefreshExpiration() { + return claims.get("scm-manager.refreshableUntil", Date.class); + } + @Override public Scope getScope() { return Scopes.fromClaims(claims); @@ -90,5 +98,9 @@ public final class JwtAccessToken implements AccessToken { public String compact() { return compact; } - + + @Override + public Map getClaims() { + return Collections.unmodifiableMap(claims); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index ece96e2954..1207b252e4 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -39,7 +39,6 @@ import io.jsonwebtoken.SignatureAlgorithm; import java.util.Date; import java.util.HashMap; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; @@ -48,7 +47,7 @@ import org.slf4j.LoggerFactory; /** * Jwt implementation of {@link AccessTokenBuilder}. - * + * * @author Sebastian Sdorra * @since 2.0.0 */ @@ -58,18 +57,20 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { * the logger for JwtAccessTokenBuilder */ private static final Logger LOG = LoggerFactory.getLogger(JwtAccessTokenBuilder.class); - - private final KeyGenerator keyGenerator; - private final SecureKeyResolver keyResolver; - + + private final KeyGenerator keyGenerator; + private final SecureKeyResolver keyResolver; + private String subject; private String issuer; - private long expiresIn = 60l; - private TimeUnit expiresInUnit = TimeUnit.MINUTES; + private long expiresIn = 1; + private TimeUnit expiresInUnit = TimeUnit.HOURS; + private long refreshableFor = 12; + private TimeUnit refreshableForUnit = TimeUnit.HOURS; private Scope scope = Scope.empty(); - + private final Map custom = Maps.newHashMap(); - + JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; @@ -81,7 +82,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { this.subject = subject; return this; } - + @Override public JwtAccessTokenBuilder custom(String key, Object value) { Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed"); @@ -92,11 +93,11 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessTokenBuilder scope(Scope scope) { - Preconditions.checkArgument(scope != null, "scope can not be null"); + Preconditions.checkArgument(scope != null, "scope cannot be null"); this.scope = scope; return this; } - + @Override public JwtAccessTokenBuilder issuer(String issuer) { Preconditions.checkArgument(!Strings.isNullOrEmpty(issuer), "null or empty value not allowed"); @@ -106,15 +107,26 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessTokenBuilder expiresIn(long count, TimeUnit unit) { - Preconditions.checkArgument(count > 0, "expires in must be greater than 0"); - Preconditions.checkArgument(unit != null, "unit can not be null"); - + Preconditions.checkArgument(count > 0, "count must be greater than 0"); + Preconditions.checkArgument(unit != null, "unit cannot be null"); + this.expiresIn = count; this.expiresInUnit = unit; - + return this; } - + + @Override + public JwtAccessTokenBuilder refreshableFor(long count, TimeUnit unit) { + Preconditions.checkArgument(count >= 0, "count must be greater or equal to 0"); + Preconditions.checkArgument(unit != null, "unit cannot be null"); + + this.refreshableFor = count; + this.refreshableForUnit = unit; + + return this; + } + private String getSubject(){ if (subject == null) { Subject currentSubject = SecurityUtils.getSubject(); @@ -130,35 +142,40 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { String id = keyGenerator.createKey(); String sub = getSubject(); - + LOG.trace("create new token {} for user {}", id, subject); SecureKey key = keyResolver.getSecureKey(sub); - + Map customClaims = new HashMap<>(custom); - + // add scope to custom claims Scopes.toClaims(customClaims, scope); - + Date now = new Date(); long expiration = expiresInUnit.toMillis(expiresIn); - + Claims claims = Jwts.claims(customClaims) .setSubject(sub) .setId(id) .setIssuedAt(now) .setExpiration(new Date(now.getTime() + expiration)); - + + if (refreshableFor > 0) { + long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); + claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime() / 1000); + } + if ( issuer != null ) { claims.setIssuer(issuer); } - + // sign token and create compact version String compact = Jwts.builder() .setClaims(claims) .signWith(SignatureAlgorithm.HS256, key.getBytes()) .compact(); - + return new JwtAccessToken(claims, compact); } - + } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..47d6a09285 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -0,0 +1,5 @@ +package sonia.scm.security; + +public interface JwtAccessTokenRefreshStrategy { + boolean shouldBeRefreshed(JwtAccessToken oldToken); +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java new file mode 100644 index 0000000000..cb26d1f010 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -0,0 +1,53 @@ +package sonia.scm.security; + +import java.time.Instant; +import java.util.Date; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +public class JwtAccessTokenRefresher { + + private final JwtAccessTokenBuilderFactory builderFactory; + private final JwtAccessTokenRefreshStrategy refreshStrategy; + + public JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy) { + this.builderFactory = builderFactory; + this.refreshStrategy = refreshStrategy; + } + + public Optional refresh(JwtAccessToken oldToken) { + JwtAccessTokenBuilder builder = builderFactory.create(); + Map claims = oldToken.getClaims(); + claims.forEach(builder::custom); + + if (canBeRefreshed(oldToken) && shouldBeRefreshed(oldToken)) { + builder.expiresIn(1, TimeUnit.HOURS); +// builder.custom("scm-manager.parentTokenId") + return Optional.of(builder.build()); + } else { + return Optional.empty(); + } + } + + private boolean canBeRefreshed(JwtAccessToken oldToken) { + return tokenIsValid(oldToken) || tokenCanBeRefreshed(oldToken); + } + + private boolean shouldBeRefreshed(JwtAccessToken oldToken) { + return refreshStrategy.shouldBeRefreshed(oldToken); + } + + private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { + Date refreshExpiration = oldToken.getRefreshExpiration(); + return refreshExpiration != null && isBeforeNow(refreshExpiration); + } + + private boolean tokenIsValid(JwtAccessToken oldToken) { + return isBeforeNow(oldToken.getExpiration()); + } + + private boolean isBeforeNow(Date expiration) { + return expiration.toInstant().isBefore(Instant.now()); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java new file mode 100644 index 0000000000..1464bfeade --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -0,0 +1,86 @@ +package sonia.scm.security; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.assertj.core.api.Assertions; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Collections; +import java.util.Optional; +import java.util.Random; +import java.util.concurrent.TimeUnit; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@SubjectAware( + username = "user", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +@RunWith(MockitoJUnitRunner.class) +public class JwtAccessTokenRefresherTest { + + @Rule + public ShiroRule shiro = new ShiroRule(); + + @Mock + private SecureKeyResolver keyResolver; + @Mock + private JwtAccessTokenRefreshStrategy refreshStrategy; + private JwtAccessTokenBuilderFactory builderFactory; + private JwtAccessTokenRefresher refresher; + private JwtAccessTokenBuilder tokenBuilder; + + @Before + public void initKeyResolver() { + byte[] bytes = new byte[256]; + new Random().nextBytes(bytes); + SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); + when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + + builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy); + tokenBuilder = builderFactory.create(); + } + + @Test + public void shouldNotRefreshTokenWithDisabledRefresh() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(0, TimeUnit.MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(10, TimeUnit.MINUTES) + .build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldRefreshTokenWithEnabledRefresh() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(1, TimeUnit.MINUTES) + .build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isNotEmpty(); + } +} diff --git a/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini b/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini index 9a39a2d46c..500325faf3 100644 --- a/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini +++ b/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini @@ -4,6 +4,7 @@ dent = secret, creator, heartOfGold, puzzle42 unpriv = secret crato = secret, creator community = secret, oss +user = secret, user [roles] admin = * @@ -11,3 +12,4 @@ creator = repository:create heartOfGold = "repository:read,modify,delete:hof" puzzle42 = "repository:read,write:p42" oss = "repository:pull" +user = * From 171f4e2f0793e56377cf979280085477f8d0ef1a Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 29 Nov 2018 16:59:04 +0100 Subject: [PATCH 04/38] merge --- .../src/main/java/sonia/scm/store/FileBasedStoreFactory.java | 3 +-- .../test/java/sonia/scm/web/HgHookCallbackServletTest.java | 4 ++-- scm-test/src/main/java/sonia/scm/AbstractTestBase.java | 4 ++-- scm-test/src/main/java/sonia/scm/ManagerTestBase.java | 4 ++-- .../sonia/scm/repository/DefaultRepositoryManagerTest.java | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java index 4b526f178a..23a4348d88 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java @@ -91,10 +91,9 @@ public abstract class FileBasedStoreFactory { * @return the store directory of a specific repository */ private File getStoreDirectory(Store store, Repository repository) { - return new File (repositoryLocationResolver.getRepositoryDirectory(repository), store.getRepositoryStoreDirectory()); + return new File (repositoryLocationResolver.getPath(repository.getId()).toFile(), store.getRepositoryStoreDirectory()); } - /** * Get the global store directory * @param store the type of the store diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgHookCallbackServletTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgHookCallbackServletTest.java index 1355b54139..7d74024630 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgHookCallbackServletTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgHookCallbackServletTest.java @@ -13,7 +13,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static sonia.scm.web.HgHookCallbackServlet.PARAM_REPOSITORYPATH; +import static sonia.scm.web.HgHookCallbackServlet.PARAM_REPOSITORYID; public class HgHookCallbackServletTest { @@ -27,7 +27,7 @@ public class HgHookCallbackServletTest { when(request.getContextPath()).thenReturn("http://example.com/scm"); when(request.getRequestURI()).thenReturn("http://example.com/scm/hook/hg/pretxnchangegroup"); String path = "/tmp/hg/12345"; - when(request.getParameter(PARAM_REPOSITORYPATH)).thenReturn(path); + when(request.getParameter(PARAM_REPOSITORYID)).thenReturn(path); servlet.doPost(request, response); diff --git a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java index 3cab59b6d6..040b347e4a 100644 --- a/scm-test/src/main/java/sonia/scm/AbstractTestBase.java +++ b/scm-test/src/main/java/sonia/scm/AbstractTestBase.java @@ -90,8 +90,8 @@ public class AbstractTestBase assertTrue(tempDirectory.mkdirs()); contextProvider = MockUtil.getSCMContextProvider(tempDirectory); fileSystem = new DefaultFileSystem(); - InitialRepositoryLocationResolver initialRepoLocationResolver = new InitialRepositoryLocationResolver(contextProvider); - repositoryLocationResolver = new RepositoryLocationResolver(repositoryDAO, initialRepoLocationResolver); + InitialRepositoryLocationResolver initialRepoLocationResolver = new InitialRepositoryLocationResolver(); + repositoryLocationResolver = new RepositoryLocationResolver(contextProvider, repositoryDAO, initialRepoLocationResolver); postSetUp(); } diff --git a/scm-test/src/main/java/sonia/scm/ManagerTestBase.java b/scm-test/src/main/java/sonia/scm/ManagerTestBase.java index 1b1f21ba09..823e88c9fc 100644 --- a/scm-test/src/main/java/sonia/scm/ManagerTestBase.java +++ b/scm-test/src/main/java/sonia/scm/ManagerTestBase.java @@ -72,9 +72,9 @@ public abstract class ManagerTestBase temp = tempFolder.newFolder(); } contextProvider = MockUtil.getSCMContextProvider(temp); - InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(contextProvider); + InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(); RepositoryDAO repoDao = mock(RepositoryDAO.class); - locationResolver = new RepositoryLocationResolver(repoDao ,initialRepositoryLocationResolver); + locationResolver = new RepositoryLocationResolver(contextProvider, repoDao ,initialRepositoryLocationResolver); manager = createManager(); manager.init(contextProvider); } diff --git a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java index 141a7f8527..772ea38775 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java @@ -433,10 +433,10 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { private DefaultRepositoryManager createRepositoryManager(boolean archiveEnabled, KeyGenerator keyGenerator) { DefaultFileSystem fileSystem = new DefaultFileSystem(); Set handlerSet = new HashSet<>(); - ConfigurationStoreFactory factory = new JAXBConfigurationStoreFactory(contextProvider); InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(); XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(contextProvider, initialRepositoryLocationResolver, fileSystem); RepositoryLocationResolver repositoryLocationResolver = new RepositoryLocationResolver(contextProvider, repositoryDAO, initialRepositoryLocationResolver); + ConfigurationStoreFactory factory = new JAXBConfigurationStoreFactory(contextProvider, repositoryLocationResolver); handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver)); handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver) { @Override From 0b1edaab08081972a9e56779fe1cc53847bbe0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 17:04:38 +0100 Subject: [PATCH 05/38] Fix time computations --- .../scm/security/JwtAccessTokenBuilder.java | 2 +- .../scm/security/JwtAccessTokenRefresher.java | 20 +++++-- .../security/JwtAccessTokenRefresherTest.java | 58 +++++++++++++++---- 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index 1207b252e4..8e5de9c283 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -162,7 +162,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime() / 1000); + claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime()); } if ( issuer != null ) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index cb26d1f010..16370209b6 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -1,6 +1,7 @@ package sonia.scm.security; -import java.time.Instant; +import javax.inject.Inject; +import java.time.Clock; import java.util.Date; import java.util.Map; import java.util.Optional; @@ -10,10 +11,17 @@ public class JwtAccessTokenRefresher { private final JwtAccessTokenBuilderFactory builderFactory; private final JwtAccessTokenRefreshStrategy refreshStrategy; + private final Clock clock; + @Inject public JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy) { + this(builderFactory, refreshStrategy, Clock.systemDefaultZone()); + } + + JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy, Clock clock) { this.builderFactory = builderFactory; this.refreshStrategy = refreshStrategy; + this.clock = clock; } public Optional refresh(JwtAccessToken oldToken) { @@ -31,7 +39,7 @@ public class JwtAccessTokenRefresher { } private boolean canBeRefreshed(JwtAccessToken oldToken) { - return tokenIsValid(oldToken) || tokenCanBeRefreshed(oldToken); + return tokenIsValid(oldToken) && tokenCanBeRefreshed(oldToken); } private boolean shouldBeRefreshed(JwtAccessToken oldToken) { @@ -40,14 +48,14 @@ public class JwtAccessTokenRefresher { private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { Date refreshExpiration = oldToken.getRefreshExpiration(); - return refreshExpiration != null && isBeforeNow(refreshExpiration); + return refreshExpiration != null && isAfterNow(refreshExpiration); } private boolean tokenIsValid(JwtAccessToken oldToken) { - return isBeforeNow(oldToken.getExpiration()); + return isAfterNow(oldToken.getExpiration()); } - private boolean isBeforeNow(Date expiration) { - return expiration.toInstant().isBefore(Instant.now()); + private boolean isAfterNow(Date expiration) { + return expiration.toInstant().isAfter(clock.instant()); } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 1464bfeade..7c7a8c68cf 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -2,7 +2,6 @@ package sonia.scm.security; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; -import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -10,11 +9,15 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.time.Clock; +import java.time.Instant; import java.util.Collections; import java.util.Optional; import java.util.Random; -import java.util.concurrent.TimeUnit; +import static java.time.Duration.ofMinutes; +import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -33,7 +36,9 @@ public class JwtAccessTokenRefresherTest { private SecureKeyResolver keyResolver; @Mock private JwtAccessTokenRefreshStrategy refreshStrategy; - private JwtAccessTokenBuilderFactory builderFactory; + @Mock + private Clock clock; + private JwtAccessTokenRefresher refresher; private JwtAccessTokenBuilder tokenBuilder; @@ -44,43 +49,74 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); tokenBuilder = builderFactory.create(); + when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); + when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); } @Test public void shouldNotRefreshTokenWithDisabledRefresh() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(0, TimeUnit.MINUTES) + .refreshableFor(0, MINUTES) .build(); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isEmpty(); + assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenTokenExpired() { + Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(oneMinuteAgo); + JwtAccessToken oldToken = tokenBuilder + .expiresIn(1, MINUTES) + .refreshableFor(5, MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenRefreshExpired() { + Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(oneMinuteAgo); + JwtAccessToken oldToken = tokenBuilder + .expiresIn(5, MINUTES) + .refreshableFor(1, MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + assertThat(refreshedToken).isEmpty(); } @Test public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(10, TimeUnit.MINUTES) + .refreshableFor(10, MINUTES) .build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isEmpty(); + assertThat(refreshedToken).isEmpty(); } @Test public void shouldRefreshTokenWithEnabledRefresh() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(1, TimeUnit.MINUTES) + .expiresIn(1, MINUTES) + .refreshableFor(1, MINUTES) .build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isNotEmpty(); + assertThat(refreshedToken).isNotEmpty(); } } From 2adcbe5d990141d655bacd865351b8fd50741128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 09:22:02 +0100 Subject: [PATCH 06/38] Set parent token id --- .../java/sonia/scm/security/AccessToken.java | 2 +- .../sonia/scm/security/JwtAccessToken.java | 10 ++++-- .../scm/security/JwtAccessTokenBuilder.java | 13 ++++++- .../scm/security/JwtAccessTokenRefresher.java | 15 ++++++-- .../security/JwtAccessTokenRefresherTest.java | 34 +++++++++---------- 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/security/AccessToken.java b/scm-core/src/main/java/sonia/scm/security/AccessToken.java index ec448ad235..c2a5f4b747 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessToken.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessToken.java @@ -80,7 +80,7 @@ public interface AccessToken { */ Date getExpiration(); - Date getRefreshExpiration(); + Optional getRefreshExpiration(); /** * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 35013e4fec..e6cfb8c957 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -37,6 +37,8 @@ import java.util.Date; import java.util.Map; import java.util.Optional; +import static java.util.Optional.ofNullable; + /** * Jwt implementation of {@link AccessToken}. * @@ -44,7 +46,9 @@ import java.util.Optional; * @since 2.0.0 */ public final class JwtAccessToken implements AccessToken { - + + public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshableUntil"; + public static final String PARENT_TOKEN_ID_CLAIM_KEY = "scm-manager.parentTokenId"; private final Claims claims; private final String compact; @@ -79,8 +83,8 @@ public final class JwtAccessToken implements AccessToken { } @Override - public Date getRefreshExpiration() { - return claims.get("scm-manager.refreshableUntil", Date.class); + public Optional getRefreshExpiration() { + return ofNullable(claims.get(REFRESHABLE_UNTIL_CLAIM_KEY, Date.class)); } @Override diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index 8e5de9c283..b6b293f525 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -67,6 +67,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private TimeUnit expiresInUnit = TimeUnit.HOURS; private long refreshableFor = 12; private TimeUnit refreshableForUnit = TimeUnit.HOURS; + private String parentKeyId; private Scope scope = Scope.empty(); private final Map custom = Maps.newHashMap(); @@ -127,6 +128,11 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { return this; } + public JwtAccessTokenBuilder parentKey(String parentKeyId) { + this.parentKeyId = parentKeyId; + return this; + } + private String getSubject(){ if (subject == null) { Subject currentSubject = SecurityUtils.getSubject(); @@ -162,7 +168,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime()); + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.getTime() + refreshExpiration).getTime()); + } + if (parentKeyId == null) { + claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); + } else { + claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, parentKeyId); } if ( issuer != null ) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index 16370209b6..f3f3d032fc 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -1,5 +1,8 @@ package sonia.scm.security; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.inject.Inject; import java.time.Clock; import java.util.Date; @@ -9,6 +12,8 @@ import java.util.concurrent.TimeUnit; public class JwtAccessTokenRefresher { + private static final Logger log = LoggerFactory.getLogger(JwtAccessTokenRefresher.class); + private final JwtAccessTokenBuilderFactory builderFactory; private final JwtAccessTokenRefreshStrategy refreshStrategy; private final Clock clock; @@ -30,8 +35,13 @@ public class JwtAccessTokenRefresher { claims.forEach(builder::custom); if (canBeRefreshed(oldToken) && shouldBeRefreshed(oldToken)) { + Optional parentTokenId = oldToken.getCustom("scm-manager.parentTokenId"); + if (!parentTokenId.isPresent()) { + log.warn("no parent token id found in token; could not refresh"); + return Optional.empty(); + } builder.expiresIn(1, TimeUnit.HOURS); -// builder.custom("scm-manager.parentTokenId") + builder.parentKey(parentTokenId.get().toString()); return Optional.of(builder.build()); } else { return Optional.empty(); @@ -47,8 +57,7 @@ public class JwtAccessTokenRefresher { } private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { - Date refreshExpiration = oldToken.getRefreshExpiration(); - return refreshExpiration != null && isAfterNow(refreshExpiration); + return oldToken.getRefreshExpiration().map(this::isAfterNow).orElse(false); } private boolean tokenIsValid(JwtAccessToken oldToken) { diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 7c7a8c68cf..06daf87fcb 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -39,6 +39,8 @@ public class JwtAccessTokenRefresherTest { @Mock private Clock clock; + private KeyGenerator keyGenerator = () -> "key"; + private JwtAccessTokenRefresher refresher; private JwtAccessTokenBuilder tokenBuilder; @@ -49,11 +51,16 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet()); refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); tokenBuilder = builderFactory.create(); when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); + + // set default expiration values + tokenBuilder + .expiresIn(5, MINUTES) + .refreshableFor(10, MINUTES); } @Test @@ -69,12 +76,9 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenTokenExpired() { - Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(oneMinuteAgo); - JwtAccessToken oldToken = tokenBuilder - .expiresIn(1, MINUTES) - .refreshableFor(5, MINUTES) - .build(); + Instant afterNormalExpiration = Instant.now().plus(ofMinutes(6)); + when(clock.instant()).thenReturn(afterNormalExpiration); + JwtAccessToken oldToken = tokenBuilder.build(); Optional refreshedToken = refresher.refresh(oldToken); @@ -83,10 +87,9 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenRefreshExpired() { - Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(oneMinuteAgo); + Instant afterRefreshExpiration = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(afterRefreshExpiration); JwtAccessToken oldToken = tokenBuilder - .expiresIn(5, MINUTES) .refreshableFor(1, MINUTES) .build(); @@ -97,9 +100,7 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { - JwtAccessToken oldToken = tokenBuilder - .refreshableFor(10, MINUTES) - .build(); + JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); Optional refreshedToken = refresher.refresh(oldToken); @@ -109,14 +110,13 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldRefreshTokenWithEnabledRefresh() { - JwtAccessToken oldToken = tokenBuilder - .expiresIn(1, MINUTES) - .refreshableFor(1, MINUTES) - .build(); + JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); Optional refreshedToken = refresher.refresh(oldToken); assertThat(refreshedToken).isNotEmpty(); + assertThat(refreshedToken.get().getClaims()) + .containsEntry(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, "key"); } } From 0f6b9ba891b4c3d0cf84c60ac318fb732fbe1532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 09:43:13 +0100 Subject: [PATCH 07/38] Inject clocks for tests --- .../sonia/scm/security/JwtAccessToken.java | 4 +++ .../scm/security/JwtAccessTokenBuilder.java | 16 ++++++---- .../JwtAccessTokenBuilderFactory.java | 13 ++++++-- .../security/JwtAccessTokenRefresherTest.java | 30 +++++++++++-------- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index e6cfb8c957..7832a463a3 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -87,6 +87,10 @@ public final class JwtAccessToken implements AccessToken { return ofNullable(claims.get(REFRESHABLE_UNTIL_CLAIM_KEY, Date.class)); } + public Optional getParentKey() { + return ofNullable(claims.get(PARENT_TOKEN_ID_CLAIM_KEY).toString()); + } + @Override public Scope getScope() { return Scopes.fromClaims(claims); diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index b6b293f525..a261c9a303 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -36,6 +36,9 @@ import com.google.common.collect.Maps; import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; + +import java.time.Clock; +import java.time.Instant; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -60,6 +63,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private final KeyGenerator keyGenerator; private final SecureKeyResolver keyResolver; + private final Clock clock; private String subject; private String issuer; @@ -72,9 +76,10 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private final Map custom = Maps.newHashMap(); - JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) { + JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Clock clock) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; + this.clock = clock; } @Override @@ -157,18 +162,19 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { // add scope to custom claims Scopes.toClaims(customClaims, scope); - Date now = new Date(); + Instant now = clock.instant(); long expiration = expiresInUnit.toMillis(expiresIn); Claims claims = Jwts.claims(customClaims) .setSubject(sub) .setId(id) - .setIssuedAt(now) - .setExpiration(new Date(now.getTime() + expiration)); + .setIssuedAt(Date.from(now)) + .setExpiration(new Date(now.toEpochMilli() + expiration)); + if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.getTime() + refreshExpiration).getTime()); + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + refreshExpiration).getTime()); } if (parentKeyId == null) { claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java index 63c4ea981c..a5704b0e82 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java @@ -30,6 +30,7 @@ */ package sonia.scm.security; +import java.time.Clock; import java.util.Set; import javax.inject.Inject; import sonia.scm.plugin.Extension; @@ -46,19 +47,25 @@ public final class JwtAccessTokenBuilderFactory implements AccessTokenBuilderFac private final KeyGenerator keyGenerator; private final SecureKeyResolver keyResolver; private final Set enrichers; + private final Clock clock; @Inject public JwtAccessTokenBuilderFactory( - KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers - ) { + KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers) { + this(keyGenerator, keyResolver, enrichers, Clock.systemDefaultZone()); + } + + JwtAccessTokenBuilderFactory( + KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers, Clock clock) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; this.enrichers = enrichers; + this.clock = clock; } @Override public JwtAccessTokenBuilder create() { - JwtAccessTokenBuilder builder = new JwtAccessTokenBuilder(keyGenerator, keyResolver); + JwtAccessTokenBuilder builder = new JwtAccessTokenBuilder(keyGenerator, keyResolver, clock); // enrich access token builder enrichers.forEach((enricher) -> { diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 06daf87fcb..88a402841d 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -29,6 +29,9 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class JwtAccessTokenRefresherTest { + private static final Instant NOW = Instant.now(); + private static final Instant TOKEN_CREATION = NOW.minus(ofMinutes(1)); + @Rule public ShiroRule shiro = new ShiroRule(); @@ -37,7 +40,9 @@ public class JwtAccessTokenRefresherTest { @Mock private JwtAccessTokenRefreshStrategy refreshStrategy; @Mock - private Clock clock; + private Clock refreshClock; + @Mock + private Clock creationClock; private KeyGenerator keyGenerator = () -> "key"; @@ -51,10 +56,11 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet()); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, refreshClock); tokenBuilder = builderFactory.create(); - when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); + when(creationClock.instant()).thenReturn(TOKEN_CREATION); + when(refreshClock.instant()).thenReturn(NOW); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); // set default expiration values @@ -76,8 +82,8 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenTokenExpired() { - Instant afterNormalExpiration = Instant.now().plus(ofMinutes(6)); - when(clock.instant()).thenReturn(afterNormalExpiration); + Instant afterNormalExpiration = NOW.plus(ofMinutes(6)); + when(refreshClock.instant()).thenReturn(afterNormalExpiration); JwtAccessToken oldToken = tokenBuilder.build(); Optional refreshedToken = refresher.refresh(oldToken); @@ -88,7 +94,7 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenRefreshExpired() { Instant afterRefreshExpiration = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(afterRefreshExpiration); + when(refreshClock.instant()).thenReturn(afterRefreshExpiration); JwtAccessToken oldToken = tokenBuilder .refreshableFor(1, MINUTES) .build(); @@ -109,14 +115,14 @@ public class JwtAccessTokenRefresherTest { } @Test - public void shouldRefreshTokenWithEnabledRefresh() { + public void shouldRefreshTokenWithCorrectClaims() { JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); - Optional refreshedToken = refresher.refresh(oldToken); + Optional refreshedTokenResult = refresher.refresh(oldToken); - assertThat(refreshedToken).isNotEmpty(); - assertThat(refreshedToken.get().getClaims()) - .containsEntry(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, "key"); + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getParentKey()).get().isEqualTo("key"); } } From 46f94730838997814988d627b4de39dba3bc0c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:05:43 +0100 Subject: [PATCH 08/38] Compute new expiration from old expiration --- .../scm/security/JwtAccessTokenRefresher.java | 6 +++- .../security/JwtAccessTokenRefresherTest.java | 31 ++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index f3f3d032fc..f219262626 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -40,7 +40,7 @@ public class JwtAccessTokenRefresher { log.warn("no parent token id found in token; could not refresh"); return Optional.empty(); } - builder.expiresIn(1, TimeUnit.HOURS); + builder.expiresIn(computeOldExpirationInMillis(oldToken), TimeUnit.MILLISECONDS); builder.parentKey(parentTokenId.get().toString()); return Optional.of(builder.build()); } else { @@ -48,6 +48,10 @@ public class JwtAccessTokenRefresher { } } + private long computeOldExpirationInMillis(JwtAccessToken oldToken) { + return oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); + } + private boolean canBeRefreshed(JwtAccessToken oldToken) { return tokenIsValid(oldToken) && tokenCanBeRefreshed(oldToken); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 88a402841d..92c6bb98e9 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -9,16 +9,21 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.sql.Date; import java.time.Clock; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Optional; import java.util.Random; +import static java.time.Duration.ofHours; import static java.time.Duration.ofMinutes; +import static java.time.temporal.ChronoUnit.SECONDS; import static java.util.concurrent.TimeUnit.MINUTES; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @SubjectAware( @@ -29,7 +34,7 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class JwtAccessTokenRefresherTest { - private static final Instant NOW = Instant.now(); + private static final Instant NOW = Instant.now().truncatedTo(SECONDS); private static final Instant TOKEN_CREATION = NOW.minus(ofMinutes(1)); @Rule @@ -41,8 +46,6 @@ public class JwtAccessTokenRefresherTest { private JwtAccessTokenRefreshStrategy refreshStrategy; @Mock private Clock refreshClock; - @Mock - private Clock creationClock; private KeyGenerator keyGenerator = () -> "key"; @@ -56,10 +59,12 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, refreshClock); - tokenBuilder = builderFactory.create(); + Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); + tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + + JwtAccessTokenBuilderFactory refreshBuilderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), refreshClock); + refresher = new JwtAccessTokenRefresher(refreshBuilderFactory, refreshStrategy, refreshClock); when(refreshClock.instant()).thenReturn(NOW); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); @@ -115,7 +120,7 @@ public class JwtAccessTokenRefresherTest { } @Test - public void shouldRefreshTokenWithCorrectClaims() { + public void shouldRefreshTokenWithParentId() { JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); @@ -125,4 +130,16 @@ public class JwtAccessTokenRefresherTest { JwtAccessToken refreshedToken = refreshedTokenResult.get(); assertThat(refreshedToken.getParentKey()).get().isEqualTo("key"); } + + @Test + public void shouldRefreshTokenWithSameExpiration() { + JwtAccessToken oldToken = tokenBuilder.build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedTokenResult = refresher.refresh(oldToken); + + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getExpiration()).isEqualTo(Date.from(NOW.plus(ofMinutes(5)))); + } } From e8672bbeff45f7054defbfc4b1c494714e01f311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:15:12 +0100 Subject: [PATCH 09/38] Keep refresh expiration --- .../main/java/sonia/scm/security/JwtAccessToken.java | 2 +- .../sonia/scm/security/JwtAccessTokenBuilder.java | 9 +++++++++ .../sonia/scm/security/JwtAccessTokenRefresher.java | 3 ++- .../scm/security/JwtAccessTokenRefresherTest.java | 12 ++++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 7832a463a3..8fb5929188 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -47,7 +47,7 @@ import static java.util.Optional.ofNullable; */ public final class JwtAccessToken implements AccessToken { - public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshableUntil"; + public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshExpiration"; public static final String PARENT_TOKEN_ID_CLAIM_KEY = "scm-manager.parentTokenId"; private final Claims claims; private final String compact; diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index a261c9a303..66db720125 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -71,6 +71,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private TimeUnit expiresInUnit = TimeUnit.HOURS; private long refreshableFor = 12; private TimeUnit refreshableForUnit = TimeUnit.HOURS; + private Instant refreshExpiration; private String parentKeyId; private Scope scope = Scope.empty(); @@ -133,6 +134,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { return this; } + JwtAccessTokenBuilder refreshExpiration(Instant refreshExpiration) { + this.refreshExpiration = refreshExpiration; + this.refreshableFor = 0; + return this; + } + public JwtAccessTokenBuilder parentKey(String parentKeyId) { this.parentKeyId = parentKeyId; return this; @@ -175,6 +182,8 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + refreshExpiration).getTime()); + } else if (refreshExpiration != null) { + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, Date.from(refreshExpiration)); } if (parentKeyId == null) { claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index f219262626..5efc01a096 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -29,7 +29,7 @@ public class JwtAccessTokenRefresher { this.clock = clock; } - public Optional refresh(JwtAccessToken oldToken) { + Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); claims.forEach(builder::custom); @@ -42,6 +42,7 @@ public class JwtAccessTokenRefresher { } builder.expiresIn(computeOldExpirationInMillis(oldToken), TimeUnit.MILLISECONDS); builder.parentKey(parentTokenId.get().toString()); + builder.refreshExpiration(oldToken.getRefreshExpiration().get().toInstant()); return Optional.of(builder.build()); } else { return Optional.empty(); diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 92c6bb98e9..83f528092c 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -142,4 +142,16 @@ public class JwtAccessTokenRefresherTest { JwtAccessToken refreshedToken = refreshedTokenResult.get(); assertThat(refreshedToken.getExpiration()).isEqualTo(Date.from(NOW.plus(ofMinutes(5)))); } + + @Test + public void shouldRefreshTokenWithSameRefreshExpiration() { + JwtAccessToken oldToken = tokenBuilder.build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedTokenResult = refresher.refresh(oldToken); + + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getRefreshExpiration()).get().isEqualTo(Date.from(TOKEN_CREATION.plus(ofMinutes(10)))); + } } From 2e092b36cf38ce99dd543da5b969e2b2379ae00d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:20:12 +0100 Subject: [PATCH 10/38] Suppress warning --- .../main/java/sonia/scm/security/JwtAccessTokenRefresher.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index 5efc01a096..ebcf88b346 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -29,6 +29,8 @@ public class JwtAccessTokenRefresher { this.clock = clock; } + @SuppressWarnings("squid:S3655") // the refresh expiration cannot be null at the time building the new token, because + // we checked this before in tokenCanBeRefreshed Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); From 176d121aa0f169d8cd92a33ebdd92a6452b7f06e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:29:08 +0100 Subject: [PATCH 11/38] Adapt tests to new version of jjwt --- .../src/test/java/sonia/scm/security/BearerRealmTest.java | 2 +- .../java/sonia/scm/security/JwtAccessTokenResolverTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index e6061e61a1..d223e271a6 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -272,7 +272,7 @@ private String createCompactToken(String subject, SecureKey key) { .thenReturn( new SecretKeySpec( key.getBytes(), - SignatureAlgorithm.HS256.getValue() + SignatureAlgorithm.HS256.getJcaName() ) ); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java index 689fc4bb35..cd7e6475aa 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java @@ -230,7 +230,7 @@ public class JwtAccessTokenResolverTest { .thenReturn( new SecretKeySpec( key.getBytes(), - SignatureAlgorithm.HS256.getValue() + SignatureAlgorithm.HS256.getJcaName() ) ); } From 205ca42e09a7bcb49d8699e5a086f0430822899b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:18:37 +0100 Subject: [PATCH 12/38] Introduce simple refresh strategy --- ...rcentageJwtAccessTokenRefreshStrategy.java | 25 +++++++ .../security/JwtAccessTokenRefresherTest.java | 2 - ...tageJwtAccessTokenRefreshStrategyTest.java | 70 +++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java create mode 100644 scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..2d5c67c7b6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java @@ -0,0 +1,25 @@ +package sonia.scm.security; + +import java.time.Clock; + +public class PercentageJwtAccessTokenRefreshStrategy implements JwtAccessTokenRefreshStrategy { + + private final Clock clock; + private final float refreshPercentage; + + public PercentageJwtAccessTokenRefreshStrategy(float refreshPercentage) { + this(Clock.systemDefaultZone(), refreshPercentage); + } + + PercentageJwtAccessTokenRefreshStrategy(Clock clock, float refreshPercentage) { + this.clock = clock; + this.refreshPercentage = refreshPercentage; + } + + @Override + public boolean shouldBeRefreshed(JwtAccessToken oldToken) { + long liveSpan = oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); + long age = clock.instant().toEpochMilli() - oldToken.getIssuedAt().getTime(); + return age/liveSpan > refreshPercentage; + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 83f528092c..cd902fb0a8 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -12,12 +12,10 @@ import org.mockito.junit.MockitoJUnitRunner; import java.sql.Date; import java.time.Clock; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Optional; import java.util.Random; -import static java.time.Duration.ofHours; import static java.time.Duration.ofMinutes; import static java.time.temporal.ChronoUnit.SECONDS; import static java.util.concurrent.TimeUnit.MINUTES; diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java new file mode 100644 index 0000000000..d2c684d4a0 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -0,0 +1,70 @@ +package sonia.scm.security; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.time.Clock; +import java.time.Instant; +import java.util.Collections; +import java.util.Random; + +import static java.time.temporal.ChronoUnit.MINUTES; +import static java.time.temporal.ChronoUnit.SECONDS; +import static java.util.concurrent.TimeUnit.HOURS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@SubjectAware( + username = "user", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +public class PercentageJwtAccessTokenRefreshStrategyTest { + + private static final Instant TOKEN_CREATION = Instant.now().truncatedTo(SECONDS); + + @Rule + public ShiroRule shiro = new ShiroRule(); + + private KeyGenerator keyGenerator = () -> "key"; + + private Clock refreshClock = mock(Clock.class); + + private JwtAccessTokenBuilder tokenBuilder; + private PercentageJwtAccessTokenRefreshStrategy refreshStrategy; + + @Before + public void initToken() { + SecureKeyResolver keyResolver = mock(SecureKeyResolver.class); + byte[] bytes = new byte[256]; + new Random().nextBytes(bytes); + SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); + when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + + Clock creationClock = mock(Clock.class); + when(creationClock.instant()).thenReturn(TOKEN_CREATION); + + tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + tokenBuilder + .refreshableFor(1, HOURS); + + refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); + } + + @Test + public void shouldNotRefreshWhenTokenIsYoung() { + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(1, MINUTES)); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + } + + @Test + public void shouldRefreshWhenTokenIsOld() { + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(31, MINUTES)); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + } +} From f7fc81b62660982be91242e55a69a760248af90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:26:23 +0100 Subject: [PATCH 13/38] Remove redundant key generation in tests --- .../test/java/sonia/scm/security/BearerRealmTest.java | 11 +---------- .../sonia/scm/security/JwtAccessTokenBuilderTest.java | 9 +-------- .../scm/security/JwtAccessTokenRefresherTest.java | 7 ++----- .../scm/security/JwtAccessTokenResolverTest.java | 8 ++------ .../PercentageJwtAccessTokenRefreshStrategyTest.java | 7 ++----- .../java/sonia/scm/security/SecureKeyTestUtil.java | 11 +++++++++++ 6 files changed, 19 insertions(+), 34 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index d223e271a6..26dfcb2099 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -61,7 +61,6 @@ import sonia.scm.user.UserDAO; import sonia.scm.user.UserTestData; import javax.crypto.spec.SecretKeySpec; -import java.security.SecureRandom; import java.util.Date; import java.util.Set; @@ -71,6 +70,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; /** * Unit tests for {@link BearerRealm}. @@ -256,12 +256,6 @@ private String createCompactToken(String subject, SecureKey key) { .compact(); } - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - random.nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - private void resolveKey(SecureKey key) { when( keyResolver.resolveSigningKey( @@ -279,9 +273,6 @@ private String createCompactToken(String subject, SecureKey key) { //~--- fields --------------------------------------------------------------- - /** Field description */ - private final SecureRandom random = new SecureRandom(); - @InjectMocks private DAORealmHelperFactory helperFactory; diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java index 6dda005019..c005e7d381 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -44,7 +44,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import java.util.Random; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -56,6 +55,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; /** * Unit test for {@link JwtAccessTokenBuilder}. @@ -162,11 +162,4 @@ public class JwtAccessTokenBuilderTest { assertEquals("b", token.getCustom("a").get()); assertEquals("[\"repo:*\"]", token.getScope().toString()); } - - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - new Random().nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index cd902fb0a8..774677cde3 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -14,7 +14,6 @@ import java.time.Clock; import java.time.Instant; import java.util.Collections; import java.util.Optional; -import java.util.Random; import static java.time.Duration.ofMinutes; import static java.time.temporal.ChronoUnit.SECONDS; @@ -23,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; @SubjectAware( username = "user", @@ -52,10 +52,7 @@ public class JwtAccessTokenRefresherTest { @Before public void initKeyResolver() { - byte[] bytes = new byte[256]; - new Random().nextBytes(bytes); - SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); - when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + when(keyResolver.getSecureKey(any())).thenReturn(createSecureKey()); Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java index cd7e6475aa..d4341f104e 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java @@ -56,6 +56,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import static org.mockito.Mockito.*; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; + import org.mockito.junit.MockitoJUnitRunner; /** @@ -214,12 +216,6 @@ public class JwtAccessTokenResolverTest { .compact(); } - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - random.nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - private void resolveKey(SecureKey key) { when( keyResolver.resolveSigningKey( diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index d2c684d4a0..e35823e445 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -9,7 +9,6 @@ import org.junit.Test; import java.time.Clock; import java.time.Instant; import java.util.Collections; -import java.util.Random; import static java.time.temporal.ChronoUnit.MINUTES; import static java.time.temporal.ChronoUnit.SECONDS; @@ -18,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; @SubjectAware( username = "user", @@ -41,10 +41,7 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { @Before public void initToken() { SecureKeyResolver keyResolver = mock(SecureKeyResolver.class); - byte[] bytes = new byte[256]; - new Random().nextBytes(bytes); - SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); - when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + when(keyResolver.getSecureKey(any())).thenReturn(createSecureKey()); Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java new file mode 100644 index 0000000000..3b9c95fd17 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java @@ -0,0 +1,11 @@ +package sonia.scm.security; + +import java.security.SecureRandom; + +public class SecureKeyTestUtil { + public static SecureKey createSecureKey() { + byte[] bytes = new byte[32]; + new SecureRandom().nextBytes(bytes); + return new SecureKey(bytes, System.currentTimeMillis()); + } +} From 57753e4de05b027417df5e5adb72842a4870f984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:35:20 +0100 Subject: [PATCH 14/38] Add default refresh strategy --- .../security/DefaultJwtAccessTokenRefreshStrategy.java | 10 ++++++++++ .../scm/security/JwtAccessTokenRefreshStrategy.java | 3 +++ .../PercentageJwtAccessTokenRefreshStrategyTest.java | 5 ++--- 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..266a327d44 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java @@ -0,0 +1,10 @@ +package sonia.scm.security; + +import sonia.scm.plugin.Extension; + +@Extension +public class DefaultJwtAccessTokenRefreshStrategy extends PercentageJwtAccessTokenRefreshStrategy { + public DefaultJwtAccessTokenRefreshStrategy() { + super(0.5F); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java index 47d6a09285..f7f030d1f6 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -1,5 +1,8 @@ package sonia.scm.security; +import sonia.scm.plugin.ExtensionPoint; + +@ExtensionPoint public interface JwtAccessTokenRefreshStrategy { boolean shouldBeRefreshed(JwtAccessToken oldToken); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index e35823e445..2e1fa44a76 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -47,15 +47,14 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { when(creationClock.instant()).thenReturn(TOKEN_CREATION); tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); - tokenBuilder - .refreshableFor(1, HOURS); + tokenBuilder.refreshableFor(1, HOURS); refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); } @Test public void shouldNotRefreshWhenTokenIsYoung() { - when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(1, MINUTES)); + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(29, MINUTES)); assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); } From 043be9f47b672d790ce2564c92aaacd0410ff0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 12:55:48 +0100 Subject: [PATCH 15/38] Add filter for token refresh --- .../sonia/scm/web/security/TokenRefreshFilter.java | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java new file mode 100644 index 0000000000..18980311e6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -0,0 +1,11 @@ +package sonia.scm.web.security; + +import sonia.scm.Priority; +import sonia.scm.filter.Filters; +import sonia.scm.filter.WebElement; + +@Priority(Filters.PRIORITY_POST_AUTHENTICATION) +@WebElement(value = Filters.PATTERN_RESTAPI, + morePatterns = { Filters.PATTERN_DEBUG }) +public class TokenRefreshFilter { +} From aec5520e57404697625c7e1a49ecca3f4fd9bdb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 16:57:04 +0100 Subject: [PATCH 16/38] Implement simple JWT refresh filter --- .../main/java/sonia/scm/ScmServletModule.java | 4 ++ .../scm/security/JwtAccessTokenRefresher.java | 2 +- .../scm/web/security/TokenRefreshFilter.java | 57 ++++++++++++++++++- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 90764a7e00..7cbdc906b8 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -83,8 +83,10 @@ import sonia.scm.security.AuthorizationChangedEventProducer; import sonia.scm.security.CipherHandler; import sonia.scm.security.CipherUtil; import sonia.scm.security.ConfigurableLoginAttemptHandler; +import sonia.scm.security.DefaultJwtAccessTokenRefreshStrategy; import sonia.scm.security.DefaultKeyGenerator; import sonia.scm.security.DefaultSecuritySystem; +import sonia.scm.security.JwtAccessTokenRefreshStrategy; import sonia.scm.security.KeyGenerator; import sonia.scm.security.LoginAttemptHandler; import sonia.scm.security.SecuritySystem; @@ -319,6 +321,8 @@ public class ScmServletModule extends ServletModule // bind(LastModifiedUpdateListener.class); bind(PushStateDispatcher.class).toProvider(PushStateDispatcherProvider.class); + + bind(JwtAccessTokenRefreshStrategy.class).to(DefaultJwtAccessTokenRefreshStrategy.class); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index ebcf88b346..6db01c904f 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -31,7 +31,7 @@ public class JwtAccessTokenRefresher { @SuppressWarnings("squid:S3655") // the refresh expiration cannot be null at the time building the new token, because // we checked this before in tokenCanBeRefreshed - Optional refresh(JwtAccessToken oldToken) { + public Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); claims.forEach(builder::custom); diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java index 18980311e6..827aaafa6d 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -1,11 +1,66 @@ package sonia.scm.web.security; +import org.apache.shiro.authc.AuthenticationToken; import sonia.scm.Priority; import sonia.scm.filter.Filters; import sonia.scm.filter.WebElement; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenCookieIssuer; +import sonia.scm.security.AccessTokenResolver; +import sonia.scm.security.BearerToken; +import sonia.scm.security.JwtAccessToken; +import sonia.scm.security.JwtAccessTokenRefresher; +import sonia.scm.web.WebTokenGenerator; +import sonia.scm.web.filter.HttpFilter; + +import javax.inject.Inject; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Set; @Priority(Filters.PRIORITY_POST_AUTHENTICATION) @WebElement(value = Filters.PATTERN_RESTAPI, morePatterns = { Filters.PATTERN_DEBUG }) -public class TokenRefreshFilter { +public class TokenRefreshFilter extends HttpFilter { + + private final Set tokenGenerators; + private final AccessTokenCookieIssuer cookieIssuer; + private final JwtAccessTokenRefresher refresher; + private final AccessTokenResolver resolver; + private final AccessTokenCookieIssuer issuer; + + @Inject + public TokenRefreshFilter(Set tokenGenerators, AccessTokenCookieIssuer cookieIssuer, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { + this.tokenGenerators = tokenGenerators; + this.cookieIssuer = cookieIssuer; + this.refresher = refresher; + this.resolver = resolver; + this.issuer = issuer; + } + + @Override + protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + AuthenticationToken token = createToken(request); + if (token != null && token instanceof BearerToken) { + AccessToken accessToken = resolver.resolve((BearerToken) token); + if (accessToken instanceof JwtAccessToken) { + refresher.refresh((JwtAccessToken) accessToken) + .ifPresent(jwtAccessToken -> issuer.authenticate(request, response, jwtAccessToken)); + } + } + chain.doFilter(request, response); + } + + private AuthenticationToken createToken(HttpServletRequest request) { + for (WebTokenGenerator generator : tokenGenerators) { + AuthenticationToken token = generator.createToken(request); + if (token != null) { + return token; + } + } + return null; + } } From 58268f88db70f9dc0bf55f68852d25402e4216be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 17:19:59 +0100 Subject: [PATCH 17/38] Fix refresh strategy --- .../scm/security/PercentageJwtAccessTokenRefreshStrategy.java | 2 +- .../security/PercentageJwtAccessTokenRefreshStrategyTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java index 2d5c67c7b6..c78654c389 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java @@ -20,6 +20,6 @@ public class PercentageJwtAccessTokenRefreshStrategy implements JwtAccessTokenRe public boolean shouldBeRefreshed(JwtAccessToken oldToken) { long liveSpan = oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); long age = clock.instant().toEpochMilli() - oldToken.getIssuedAt().getTime(); - return age/liveSpan > refreshPercentage; + return (float)age/liveSpan > refreshPercentage; } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index 2e1fa44a76..122c1b5381 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -47,6 +47,7 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { when(creationClock.instant()).thenReturn(TOKEN_CREATION); tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + tokenBuilder.expiresIn(1, HOURS); tokenBuilder.refreshableFor(1, HOURS); refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); @@ -61,6 +62,6 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { @Test public void shouldRefreshWhenTokenIsOld() { when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(31, MINUTES)); - assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isTrue(); } } From 80ce5af12a7b11eed614ea942a0a979185db5930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 17:25:53 +0100 Subject: [PATCH 18/38] Log token refresh --- .../sonia/scm/web/security/TokenRefreshFilter.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java index 827aaafa6d..6177b0251b 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -1,6 +1,8 @@ package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationToken; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.Priority; import sonia.scm.filter.Filters; import sonia.scm.filter.WebElement; @@ -26,6 +28,8 @@ import java.util.Set; morePatterns = { Filters.PATTERN_DEBUG }) public class TokenRefreshFilter extends HttpFilter { + private static final Logger LOG = LoggerFactory.getLogger(TokenRefreshFilter.class); + private final Set tokenGenerators; private final AccessTokenCookieIssuer cookieIssuer; private final JwtAccessTokenRefresher refresher; @@ -48,12 +52,17 @@ public class TokenRefreshFilter extends HttpFilter { AccessToken accessToken = resolver.resolve((BearerToken) token); if (accessToken instanceof JwtAccessToken) { refresher.refresh((JwtAccessToken) accessToken) - .ifPresent(jwtAccessToken -> issuer.authenticate(request, response, jwtAccessToken)); + .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); } } chain.doFilter(request, response); } + private void refreshToken(HttpServletRequest request, HttpServletResponse response, JwtAccessToken jwtAccessToken) { + LOG.debug("refreshing authentication token"); + issuer.authenticate(request, response, jwtAccessToken); + } + private AuthenticationToken createToken(HttpServletRequest request) { for (WebTokenGenerator generator : tokenGenerators) { AuthenticationToken token = generator.createToken(request); From e30d32f1cdf29a9c75c80716567eb6a9b247eb1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Sat, 1 Dec 2018 20:46:05 +0100 Subject: [PATCH 19/38] Test things --- .../scm/web/security/TokenRefreshFilter.java | 43 +++---- .../web/security/TokenRefreshFilterTest.java | 107 ++++++++++++++++++ 2 files changed, 130 insertions(+), 20 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java index 6177b0251b..f85c0fbbbd 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -21,8 +21,12 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Optional; import java.util.Set; +import static java.util.Optional.empty; +import static java.util.Optional.of; + @Priority(Filters.PRIORITY_POST_AUTHENTICATION) @WebElement(value = Filters.PATTERN_RESTAPI, morePatterns = { Filters.PATTERN_DEBUG }) @@ -31,15 +35,13 @@ public class TokenRefreshFilter extends HttpFilter { private static final Logger LOG = LoggerFactory.getLogger(TokenRefreshFilter.class); private final Set tokenGenerators; - private final AccessTokenCookieIssuer cookieIssuer; private final JwtAccessTokenRefresher refresher; private final AccessTokenResolver resolver; private final AccessTokenCookieIssuer issuer; @Inject - public TokenRefreshFilter(Set tokenGenerators, AccessTokenCookieIssuer cookieIssuer, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { + public TokenRefreshFilter(Set tokenGenerators, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { this.tokenGenerators = tokenGenerators; - this.cookieIssuer = cookieIssuer; this.refresher = refresher; this.resolver = resolver; this.issuer = issuer; @@ -47,29 +49,30 @@ public class TokenRefreshFilter extends HttpFilter { @Override protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - AuthenticationToken token = createToken(request); - if (token != null && token instanceof BearerToken) { - AccessToken accessToken = resolver.resolve((BearerToken) token); - if (accessToken instanceof JwtAccessToken) { - refresher.refresh((JwtAccessToken) accessToken) - .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); + extractToken(request).ifPresent(token -> examineToken(request, response, token)); + chain.doFilter(request, response); + } + + private Optional extractToken(HttpServletRequest request) { + for (WebTokenGenerator generator : tokenGenerators) { + AuthenticationToken token = generator.createToken(request); + if (token instanceof BearerToken) { + return of((BearerToken) token); } } - chain.doFilter(request, response); + return empty(); + } + + private void examineToken(HttpServletRequest request, HttpServletResponse response, BearerToken token) { + AccessToken accessToken = resolver.resolve(token); + if (accessToken instanceof JwtAccessToken) { + refresher.refresh((JwtAccessToken) accessToken) + .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); + } } private void refreshToken(HttpServletRequest request, HttpServletResponse response, JwtAccessToken jwtAccessToken) { LOG.debug("refreshing authentication token"); issuer.authenticate(request, response, jwtAccessToken); } - - private AuthenticationToken createToken(HttpServletRequest request) { - for (WebTokenGenerator generator : tokenGenerators) { - AuthenticationToken token = generator.createToken(request); - if (token != null) { - return token; - } - } - return null; - } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java new file mode 100644 index 0000000000..945d8cf0d2 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java @@ -0,0 +1,107 @@ +package sonia.scm.web.security; + +import org.apache.shiro.authc.AuthenticationToken; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.security.AccessTokenCookieIssuer; +import sonia.scm.security.AccessTokenResolver; +import sonia.scm.security.BearerToken; +import sonia.scm.security.JwtAccessToken; +import sonia.scm.security.JwtAccessTokenRefresher; +import sonia.scm.web.WebTokenGenerator; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Set; + +import static java.util.Collections.singleton; +import static java.util.Optional.of; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith({MockitoExtension.class}) +class TokenRefreshFilterTest { + + @Mock + private Set tokenGenerators; + @Mock + private WebTokenGenerator tokenGenerator; + @Mock + private JwtAccessTokenRefresher refresher; + @Mock + private AccessTokenResolver resolver; + @Mock + private AccessTokenCookieIssuer issuer; + + @InjectMocks + private TokenRefreshFilter filter; + + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + @Mock + private FilterChain filterChain; + + @BeforeEach + void initGenerators() { + when(tokenGenerators.iterator()).thenReturn(singleton(tokenGenerator).iterator()); + } + + @Test + void shouldContinueChain() throws IOException, ServletException { + filter.doFilter(request, response, filterChain); + + verify(filterChain).doFilter(request, response); + verify(issuer, never()).authenticate(any(), any(), any()); + } + + @Test + void shouldNotRefreshNonBearerToken() throws IOException, ServletException { + AuthenticationToken token = mock(AuthenticationToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + + filter.doFilter(request, response, filterChain); + + verify(issuer, never()).authenticate(any(), any(), any()); + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldNotRefreshNonJwtToken() throws IOException, ServletException { + BearerToken token = mock(BearerToken.class); + JwtAccessToken jwtToken = mock(JwtAccessToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + when(resolver.resolve(token)).thenReturn(jwtToken); + + filter.doFilter(request, response, filterChain); + + verify(issuer, never()).authenticate(any(), any(), any()); + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldRefreshIfRefreshable() throws IOException, ServletException { + BearerToken token = mock(BearerToken.class); + JwtAccessToken jwtToken = mock(JwtAccessToken.class); + JwtAccessToken newJwtToken = mock(JwtAccessToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + when(resolver.resolve(token)).thenReturn(jwtToken); + when(refresher.refresh(jwtToken)).thenReturn(of(newJwtToken)); + + filter.doFilter(request, response, filterChain); + + verify(issuer).authenticate(request, response, newJwtToken); + verify(filterChain).doFilter(request, response); + } +} From 581e6a9bffb9f228ba66c073992cb044c3881bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 08:20:41 +0100 Subject: [PATCH 20/38] Fix extension point injection --- scm-webapp/src/main/java/sonia/scm/ScmServletModule.java | 2 -- .../java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 7cbdc906b8..9555ad66b5 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -321,8 +321,6 @@ public class ScmServletModule extends ServletModule // bind(LastModifiedUpdateListener.class); bind(PushStateDispatcher.class).toProvider(PushStateDispatcherProvider.class); - - bind(JwtAccessTokenRefreshStrategy.class).to(DefaultJwtAccessTokenRefreshStrategy.class); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java index f7f030d1f6..9135a0e099 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -2,7 +2,7 @@ package sonia.scm.security; import sonia.scm.plugin.ExtensionPoint; -@ExtensionPoint +@ExtensionPoint(multi = false) public interface JwtAccessTokenRefreshStrategy { boolean shouldBeRefreshed(JwtAccessToken oldToken); } From 3638d3520fa6d187c3edb8205f05388b45ac4f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 11:28:03 +0100 Subject: [PATCH 21/38] Use static method for new StoreParameters instance --- .../repository/AbstractRepositoryHandler.java | 11 ++++++----- .../java/sonia/scm/store/StoreParameters.java | 4 ++-- .../java/sonia/scm/group/xml/XmlGroupDAO.java | 11 ++++++----- .../sonia/scm/store/FileBasedStoreFactory.java | 6 +++--- .../java/sonia/scm/user/xml/XmlUserDAO.java | 14 +++++++------- .../java/sonia/scm/store/FileBlobStoreTest.java | 11 ++++++----- .../store/JAXBConfigurationEntryStoreTest.java | 17 +++++++++-------- .../scm/store/JAXBConfigurationStoreTest.java | 11 ++++++----- .../java/sonia/scm/store/JAXBDataStoreTest.java | 17 ++++++----------- .../sonia/scm/web/lfs/LfsBlobStoreFactory.java | 14 +++++++------- .../java/sonia/scm/store/BlobStoreTestBase.java | 11 ++++++----- .../store/ConfigurationEntryStoreTestBase.java | 8 ++++---- .../java/sonia/scm/store/DataStoreTestBase.java | 6 +++--- .../java/sonia/scm/store/StoreTestBase.java | 13 +++---------- .../scm/security/DefaultSecuritySystem.java | 10 ++++++---- .../sonia/scm/security/SecureKeyResolver.java | 10 +++++----- 16 files changed, 85 insertions(+), 89 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java index 567d2a6e84..e22b51789c 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java @@ -48,7 +48,8 @@ import java.io.File; import java.io.IOException; import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; -import sonia.scm.store.StoreParameters; + +import static sonia.scm.store.StoreParameters.forType; /** @@ -74,10 +75,10 @@ public abstract class AbstractRepositoryHandler * @param storeFactory */ protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) { - this.store = storeFactory.getStore(new StoreParameters() - .withType(getConfigClass()) - .withName(getType().getName()) - .build() + this.store = storeFactory.getStore( + forType(getConfigClass()) + .withName(getType().getName()) + .build() ); } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index b625004d42..7d3aca8aba 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -26,8 +26,8 @@ public class StoreParameters { return repository; } - public WithType withType(Class type){ - return new WithType(type); + public static WithType forType(Class type){ + return new StoreParameters().new WithType(type); } public class WithType { diff --git a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java index 2b416cf53a..5236f9d7e7 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java @@ -39,11 +39,12 @@ import com.google.inject.Singleton; import sonia.scm.group.Group; import sonia.scm.group.GroupDAO; -import sonia.scm.store.StoreParameters; import sonia.scm.xml.AbstractXmlDAO; import sonia.scm.store.ConfigurationStoreFactory; +import static sonia.scm.store.StoreParameters.forType; + /** * * @author Sebastian Sdorra @@ -66,10 +67,10 @@ public class XmlGroupDAO extends AbstractXmlDAO */ @Inject public XmlGroupDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.getStore(new StoreParameters() - .withType(XmlGroupDatabase.class) - .withName(STORE_NAME) - .build())); + super(storeFactory.getStore( + forType(XmlGroupDatabase.class) + .withName(STORE_NAME) + .build())); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java index 23a4348d88..dfb7bd5d38 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java @@ -73,10 +73,10 @@ public abstract class FileBasedStoreFactory { protected File getStoreLocation(String name, Class type, Repository repository) { if (storeDirectory == null) { if (repository != null) { - LOG.debug("create store with type :{}, name:{} and repository {}", type, name, repository.getNamespaceAndName()); + LOG.debug("create store with type: {}, name: {} and repository: {}", type, name, repository.getNamespaceAndName()); storeDirectory = this.getStoreDirectory(store, repository); } else { - LOG.debug("create store with type :{} and name:{} ", type, name); + LOG.debug("create store with type: {} and name: {} ", type, name); storeDirectory = this.getStoreDirectory(store); } IOUtil.mkdirs(storeDirectory); @@ -91,7 +91,7 @@ public abstract class FileBasedStoreFactory { * @return the store directory of a specific repository */ private File getStoreDirectory(Store store, Repository repository) { - return new File (repositoryLocationResolver.getPath(repository.getId()).toFile(), store.getRepositoryStoreDirectory()); + return new File(repositoryLocationResolver.getPath(repository.getId()).toFile(), store.getRepositoryStoreDirectory()); } /** diff --git a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java index 5f84aeb3a8..7f580c9945 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java @@ -36,12 +36,12 @@ package sonia.scm.user.xml; import com.google.inject.Inject; import com.google.inject.Singleton; - -import sonia.scm.store.StoreParameters; +import sonia.scm.store.ConfigurationStoreFactory; import sonia.scm.user.User; import sonia.scm.user.UserDAO; import sonia.scm.xml.AbstractXmlDAO; -import sonia.scm.store.ConfigurationStoreFactory; + +import static sonia.scm.store.StoreParameters.forType; /** * @@ -66,10 +66,10 @@ public class XmlUserDAO extends AbstractXmlDAO @Inject public XmlUserDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.getStore(new StoreParameters() - .withType(XmlUserDatabase.class) - .withName(STORE_NAME) - .build())); + super(storeFactory.getStore( + forType(XmlUserDatabase.class) + .withName(STORE_NAME) + .build())); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java index 51d2d63a5e..ef694c23ac 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java @@ -42,6 +42,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertNotNull; +import static sonia.scm.store.StoreParameters.forType; /** * @@ -65,11 +66,11 @@ public class FileBlobStoreTest extends BlobStoreTestBase @Test @SuppressWarnings("unchecked") public void shouldStoreAndLoadInRepository() { - BlobStore store = createBlobStoreFactory().getStore(new StoreParameters() - .withType(StoreObject.class) - .withName("test") - .forRepository(new Repository("id", "git", "ns", "n")) - .build()); + BlobStore store = createBlobStoreFactory().getStore( + forType(StoreObject.class) + .withName("test") + .forRepository(new Repository("id", "git", "ns", "n")) + .build()); Blob createdBlob = store.create("abc"); List storedBlobs = store.getAll(); diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java index 81a8f4c340..408318777e 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java @@ -50,6 +50,7 @@ import java.util.UUID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -132,10 +133,10 @@ public class JAXBConfigurationEntryStoreTest ConfigurationEntryStore store = createPermissionStore(RESOURCE_FIXED, name); store.put("a45", new AssignedPermission("tuser4", "repository:create")); - store = createConfigurationStoreFactory().getStore(new StoreParameters() - .withType(AssignedPermission.class) - .withName(name) - .build()); + store = createConfigurationStoreFactory().getStore( + forType(AssignedPermission.class) + .withName(name) + .build()); AssignedPermission ap = store.get("a45"); @@ -231,9 +232,9 @@ public class JAXBConfigurationEntryStoreTest } copy(resource, name); - return createConfigurationStoreFactory().getStore(new StoreParameters() - .withType(AssignedPermission.class) - .withName(name) - .build()); + return createConfigurationStoreFactory().getStore( + forType(AssignedPermission.class) + .withName(name) + .build()); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java index ba2527b4ba..ace6cb39c4 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java @@ -39,6 +39,7 @@ import java.io.IOException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static sonia.scm.store.StoreParameters.forType; /** * Unit tests for {@link JAXBConfigurationStore}. @@ -58,11 +59,11 @@ public class JAXBConfigurationStoreTest extends StoreTestBase { @SuppressWarnings("unchecked") public void shouldStoreAndLoadInRepository() throws IOException { - ConfigurationStore store = createStoreFactory().getStore(new StoreParameters() - .withType(StoreObject.class) - .withName("test") - .forRepository(new Repository("id", "git", "ns", "n")) - .build()); + ConfigurationStore store = createStoreFactory().getStore( + forType(StoreObject.class) + .withName("test") + .forRepository(new Repository("id", "git", "ns", "n")) + .build()); store.set(new StoreObject("value")); StoreObject storeObject = store.get(); 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 f42a0cd242..bd4ccedb66 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 @@ -38,10 +38,9 @@ import org.junit.Test; import sonia.scm.repository.Repository; import sonia.scm.security.UUIDKeyGenerator; -import java.io.IOException; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static sonia.scm.store.StoreParameters.forType; /** * @@ -63,25 +62,21 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStore getDataStore(Class type, Repository repository) { - StoreParameters params = new StoreParameters() - .withType(type) + return createDataStoreFactory().getStore(forType(type) .withName("test") .forRepository(repository) - .build(); - return createDataStoreFactory().getStore(params); + .build()); } @Override protected DataStore getDataStore(Class type) { - StoreParameters params = new StoreParameters() - .withType(type) + return createDataStoreFactory().getStore(forType(type) .withName("test") - .build(); - return createDataStoreFactory().getStore(params); + .build()); } @Test - public void shouldStoreAndLoadInRepository() throws IOException + public void shouldStoreAndLoadInRepository() { repoStore.put("abc", new StoreObject("abc_value")); StoreObject storeObject = repoStore.get("abc"); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java index 3e4ba67e0f..a03868505f 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java @@ -35,10 +35,10 @@ package sonia.scm.web.lfs; import com.google.inject.Inject; import com.google.inject.Singleton; import sonia.scm.repository.Repository; -import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; import sonia.scm.store.BlobStoreFactory; -import sonia.scm.store.StoreParameters; + +import static sonia.scm.store.StoreParameters.forType; /** * Creates {@link BlobStore} objects to store lfs objects. @@ -78,11 +78,11 @@ public class LfsBlobStoreFactory { */ @SuppressWarnings("unchecked") public BlobStore getLfsBlobStore(Repository repository) { - return blobStoreFactory.getStore(new StoreParameters() - .withType(Blob.class) - .withName(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX) - .forRepository(repository) - .build() + return blobStoreFactory.getStore( + forType(String.class) + .withName(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX) + .forRepository(repository) + .build() ); } } diff --git a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java index e30fa4de33..3c029bc781 100644 --- a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java @@ -50,6 +50,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -69,11 +70,11 @@ public abstract class BlobStoreTestBase extends AbstractTestBase @Before public void createBlobStore() { - store = createBlobStoreFactory().getStore(new StoreParameters() - .withType(Blob.class) - .withName("test") - .forRepository(RepositoryTestData.createHeartOfGold()) - .build() + store = createBlobStoreFactory().getStore( + forType(Blob.class) + .withName("test") + .forRepository(RepositoryTestData.createHeartOfGold()) + .build() ); store.clear(); } diff --git a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java index 9a99a78f69..df08a98f60 100644 --- a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java @@ -34,6 +34,8 @@ package sonia.scm.store; import sonia.scm.repository.Repository; +import static sonia.scm.store.StoreParameters.forType; + /** * * @author Sebastian Sdorra @@ -51,8 +53,7 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB //~--- get methods ---------------------------------------------------------- @Override protected ConfigurationEntryStore getDataStore(Class type) { - StoreParameters params = new StoreParameters() - .withType(type) + StoreParameters params = forType(type) .withName(storeName) .build(); return this.createConfigurationStoreFactory().getStore(params); @@ -60,8 +61,7 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB @Override protected ConfigurationEntryStore getDataStore(Class type, Repository repository) { - StoreParameters params = new StoreParameters() - .withType(type) + StoreParameters params = forType(type) .withName(repoStoreName) .forRepository(repository) .build(); diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index 839baaa616..b1d1b49ebb 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -39,6 +39,7 @@ import sonia.scm.repository.RepositoryTestData; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static sonia.scm.store.StoreParameters.forType; /** * @@ -56,9 +57,8 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase protected abstract DataStoreFactory createDataStoreFactory(); - protected StoreParameters getStoreParametersWithRepository(Repository repository) { - return new StoreParameters() - .withType(StoreObject.class) + protected StoreParameters getStoreParametersWithRepository(Repository repository) { + return forType(StoreObject.class) .withName("test") .forRepository(repository) .build(); diff --git a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java index fd9afaff08..5bd665582a 100644 --- a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java @@ -36,11 +36,11 @@ package sonia.scm.store; import org.junit.Test; import sonia.scm.AbstractTestBase; -import sonia.scm.repository.Repository; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -60,13 +60,6 @@ public abstract class StoreTestBase extends AbstractTestBase */ protected abstract ConfigurationStoreFactory createStoreFactory(); - protected StoreParameters getStoreParameters() { - return new StoreParameters() - .withType(StoreObject.class) - .withName("test") - .build(); - } - /** * Method description * @@ -74,7 +67,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testGet() { - ConfigurationStore store = createStoreFactory().getStore(getStoreParameters()); + ConfigurationStore store = createStoreFactory().getStore(forType(StoreObject.class).withName("test").build()); assertNotNull(store); @@ -90,7 +83,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testSet() { - ConfigurationStore store = createStoreFactory().getStore(getStoreParameters()); + ConfigurationStore store = createStoreFactory().getStore(forType(StoreObject.class).withName("test").build()); assertNotNull(store); diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java index afbe3e00d3..9f2bb86365 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java @@ -77,6 +77,8 @@ import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; +import static sonia.scm.store.StoreParameters.forType; + /** * TODO add events * @@ -112,10 +114,10 @@ public class DefaultSecuritySystem implements SecuritySystem @SuppressWarnings("unchecked") public DefaultSecuritySystem(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory.getStore(new StoreParameters() - .withType(AssignedPermission.class) - .withName(NAME) - .build()); + store = storeFactory.getStore( + forType(AssignedPermission.class) + .withName(NAME) + .build()); readAvailablePermissions(); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java index 31556afef4..72a7751515 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java @@ -45,9 +45,9 @@ import org.slf4j.LoggerFactory; import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; -import sonia.scm.store.StoreParameters; import static com.google.common.base.Preconditions.*; +import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -91,10 +91,10 @@ public class SecureKeyResolver extends SigningKeyResolverAdapter @SuppressWarnings("unchecked") public SecureKeyResolver(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory.getStore(new StoreParameters() - .withType(SecureKey.class) - .withName(STORE_NAME) - .build()); + store = storeFactory.getStore( + forType(SecureKey.class) + .withName(STORE_NAME) + .build()); } //~--- methods -------------------------------------------------------------- From 44d99f55f206c3c09b4633ce6388a20a972178d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 12:28:35 +0100 Subject: [PATCH 22/38] Do no longer expose StoreParameters --- .../repository/AbstractRepositoryHandler.java | 9 +++---- .../java/sonia/scm/store/StoreFactory.java | 6 ++++- .../java/sonia/scm/store/StoreParameters.java | 26 ++++++++++--------- .../java/sonia/scm/group/xml/XmlGroupDAO.java | 6 ++--- .../java/sonia/scm/user/xml/XmlUserDAO.java | 7 ++--- .../sonia/scm/store/FileBlobStoreTest.java | 5 ++-- .../JAXBConfigurationEntryStoreTest.java | 9 +++---- .../scm/store/JAXBConfigurationStoreTest.java | 9 +++---- .../sonia/scm/store/JAXBDataStoreTest.java | 9 +++---- .../scm/web/lfs/LfsBlobStoreFactory.java | 8 ++---- .../repository/GitRepositoryHandlerTest.java | 7 +++++ .../scm/web/lfs/LfsBlobStoreFactoryTest.java | 7 +++-- .../repository/HgRepositoryHandlerTest.java | 8 ++++++ .../repository/SvnRepositoryHandlerTest.java | 15 +++++++---- .../sonia/scm/store/BlobStoreTestBase.java | 6 ++--- .../ConfigurationEntryStoreTestBase.java | 14 ++++------ .../sonia/scm/store/DataStoreTestBase.java | 12 +++------ .../java/sonia/scm/store/StoreTestBase.java | 5 ++-- .../scm/security/DefaultSecuritySystem.java | 7 ++--- .../sonia/scm/security/SecureKeyResolver.java | 5 ++-- .../resources/AutoCompleteResourceTest.java | 1 + .../scm/security/SecureKeyResolverTest.java | 2 ++ 22 files changed, 91 insertions(+), 92 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java index e22b51789c..d5a056e16f 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java @@ -49,8 +49,6 @@ import java.io.IOException; import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; -import static sonia.scm.store.StoreParameters.forType; - /** * @@ -75,11 +73,10 @@ public abstract class AbstractRepositoryHandler * @param storeFactory */ protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) { - this.store = storeFactory.getStore( + this.store = storeFactory. forType(getConfigClass()) - .withName(getType().getName()) - .build() - ); + .withName(getType().getName()) + .build(); } //~--- get methods ---------------------------------------------------------- diff --git a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java index a40e8cfeba..adf432d4e2 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java @@ -2,5 +2,9 @@ package sonia.scm.store; public interface StoreFactory { - STORE getStore(final StoreParameters storeParameters); + STORE getStore(final StoreParameters storeParameters); + + default StoreParameters.WithType forType(Class type) { + return new StoreParameters<>(this).new WithType(type); + } } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index 7d3aca8aba..8abda1c86d 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -8,12 +8,18 @@ import sonia.scm.repository.Repository; * @author Mohamed Karray * @since 2.0.0 */ -public class StoreParameters { +public class StoreParameters { private Class type; private String name; private Repository repository; + private final StoreFactory factory; + + StoreParameters(StoreFactory factory) { + this.factory = factory; + } + public Class getType() { return type; } @@ -26,17 +32,13 @@ public class StoreParameters { return repository; } - public static WithType forType(Class type){ - return new StoreParameters().new WithType(type); - } - public class WithType { - private WithType(Class type) { + WithType(Class type) { StoreParameters.this.type = type; } - public WithTypeAndName withName(String name){ + public StoreParameters.WithTypeAndName withName(String name){ return new WithTypeAndName(name); } @@ -47,11 +49,11 @@ public class StoreParameters { StoreParameters.this.name = name; } - public WithTypeNameAndRepository forRepository(Repository repository){ + public StoreParameters.WithTypeNameAndRepository forRepository(Repository repository){ return new WithTypeNameAndRepository(repository); } - public StoreParameters build(){ - return StoreParameters.this; + public STORE build(){ + return factory.getStore(StoreParameters.this); } } @@ -60,8 +62,8 @@ public class StoreParameters { private WithTypeNameAndRepository(Repository repository) { StoreParameters.this.repository = repository; } - public StoreParameters build(){ - return StoreParameters.this; + public STORE build(){ + return factory.getStore(StoreParameters.this); } } } diff --git a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java index 5236f9d7e7..0afc34e9c2 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java @@ -43,8 +43,6 @@ import sonia.scm.xml.AbstractXmlDAO; import sonia.scm.store.ConfigurationStoreFactory; -import static sonia.scm.store.StoreParameters.forType; - /** * * @author Sebastian Sdorra @@ -67,10 +65,10 @@ public class XmlGroupDAO extends AbstractXmlDAO */ @Inject public XmlGroupDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.getStore( + super(storeFactory. forType(XmlGroupDatabase.class) .withName(STORE_NAME) - .build())); + .build()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java index 7f580c9945..2b97afc3ce 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java @@ -41,8 +41,6 @@ import sonia.scm.user.User; import sonia.scm.user.UserDAO; import sonia.scm.xml.AbstractXmlDAO; -import static sonia.scm.store.StoreParameters.forType; - /** * * @author Sebastian Sdorra @@ -66,10 +64,9 @@ public class XmlUserDAO extends AbstractXmlDAO @Inject public XmlUserDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.getStore( - forType(XmlUserDatabase.class) + super(storeFactory.forType(XmlUserDatabase.class) .withName(STORE_NAME) - .build())); + .build()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java index ef694c23ac..5f78eb75a3 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java @@ -42,7 +42,6 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertNotNull; -import static sonia.scm.store.StoreParameters.forType; /** * @@ -66,11 +65,11 @@ public class FileBlobStoreTest extends BlobStoreTestBase @Test @SuppressWarnings("unchecked") public void shouldStoreAndLoadInRepository() { - BlobStore store = createBlobStoreFactory().getStore( + BlobStore store = createBlobStoreFactory(). forType(StoreObject.class) .withName("test") .forRepository(new Repository("id", "git", "ns", "n")) - .build()); + .build(); Blob createdBlob = store.create("abc"); List storedBlobs = store.getAll(); diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java index 408318777e..a5c8049096 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java @@ -50,7 +50,6 @@ import java.util.UUID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -133,10 +132,10 @@ public class JAXBConfigurationEntryStoreTest ConfigurationEntryStore store = createPermissionStore(RESOURCE_FIXED, name); store.put("a45", new AssignedPermission("tuser4", "repository:create")); - store = createConfigurationStoreFactory().getStore( + store = createConfigurationStoreFactory(). forType(AssignedPermission.class) .withName(name) - .build()); + .build(); AssignedPermission ap = store.get("a45"); @@ -232,9 +231,9 @@ public class JAXBConfigurationEntryStoreTest } copy(resource, name); - return createConfigurationStoreFactory().getStore( + return createConfigurationStoreFactory(). forType(AssignedPermission.class) .withName(name) - .build()); + .build(); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java index ace6cb39c4..4760f2b0fc 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java @@ -35,11 +35,8 @@ package sonia.scm.store; import org.junit.Test; import sonia.scm.repository.Repository; -import java.io.IOException; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static sonia.scm.store.StoreParameters.forType; /** * Unit tests for {@link JAXBConfigurationStore}. @@ -57,13 +54,13 @@ public class JAXBConfigurationStoreTest extends StoreTestBase { @Test @SuppressWarnings("unchecked") - public void shouldStoreAndLoadInRepository() throws IOException + public void shouldStoreAndLoadInRepository() { - ConfigurationStore store = createStoreFactory().getStore( + ConfigurationStore store = createStoreFactory(). forType(StoreObject.class) .withName("test") .forRepository(new Repository("id", "git", "ns", "n")) - .build()); + .build(); store.set(new StoreObject("value")); StoreObject storeObject = store.get(); 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 bd4ccedb66..e3df72c2e7 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 @@ -40,7 +40,6 @@ import sonia.scm.security.UUIDKeyGenerator; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static sonia.scm.store.StoreParameters.forType; /** * @@ -62,17 +61,17 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStore getDataStore(Class type, Repository repository) { - return createDataStoreFactory().getStore(forType(type) + return createDataStoreFactory().forType(type) .withName("test") .forRepository(repository) - .build()); + .build(); } @Override protected DataStore getDataStore(Class type) { - return createDataStoreFactory().getStore(forType(type) + return createDataStoreFactory().forType(type) .withName("test") - .build()); + .build(); } @Test diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java index a03868505f..59fff8cd2f 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java @@ -38,8 +38,6 @@ import sonia.scm.repository.Repository; import sonia.scm.store.BlobStore; import sonia.scm.store.BlobStoreFactory; -import static sonia.scm.store.StoreParameters.forType; - /** * Creates {@link BlobStore} objects to store lfs objects. * @@ -78,11 +76,9 @@ public class LfsBlobStoreFactory { */ @SuppressWarnings("unchecked") public BlobStore getLfsBlobStore(Repository repository) { - return blobStoreFactory.getStore( - forType(String.class) + return blobStoreFactory.forType(String.class) .withName(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX) .forRepository(repository) - .build() - ); + .build(); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index d3eca1d6e0..ed25dc2f6b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -33,6 +33,7 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -44,6 +45,8 @@ import java.io.File; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; //~--- JDK imports ------------------------------------------------------------ @@ -81,6 +84,10 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { assertTrue(refs.isDirectory()); } + @Before + public void initFactory() { + when(factory.forType(any())).thenCallRealMethod(); + } @Override protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java index 8cc36a2ec4..23077a9f5f 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java @@ -41,9 +41,11 @@ import sonia.scm.repository.Repository; import sonia.scm.store.BlobStoreFactory; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; /** * Unit tests for {@link LfsBlobStoreFactory}. @@ -61,6 +63,7 @@ public class LfsBlobStoreFactoryTest { @Test public void getBlobStore() { + when(blobStoreFactory.forType(any())).thenCallRealMethod(); Repository repository = new Repository("the-id", "GIT", "space", "the-name"); lfsBlobStoreFactory.getLfsBlobStore(repository); @@ -73,7 +76,7 @@ public class LfsBlobStoreFactoryTest { })); // make sure there have been no further usages of the factory - verifyNoMoreInteractions(blobStoreFactory); + verify(blobStoreFactory, times(1)).getStore(any()); } } diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java index 7a13c06eb2..9f351ba9e1 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java @@ -34,6 +34,7 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -44,6 +45,8 @@ import java.io.File; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; //~--- JDK imports ------------------------------------------------------------ @@ -67,6 +70,11 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { assertTrue(hgDirectory.isDirectory()); } + @Before + public void initFactory() { + when(factory.forType(any())).thenCallRealMethod(); + } + @Override protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, RepositoryLocationResolver locationResolver, File directory) { HgRepositoryHandler handler = new HgRepositoryHandler(factory, new HgContextProvider(), locationResolver); diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java index 04e8cf5c06..4b7f6486b4 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java @@ -32,6 +32,7 @@ package sonia.scm.repository; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -42,12 +43,14 @@ import sonia.scm.store.ConfigurationStore; import sonia.scm.store.ConfigurationStoreFactory; import java.io.File; +import java.io.IOException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; //~--- JDK imports ------------------------------------------------------------ @@ -55,15 +58,11 @@ import static org.mockito.Mockito.when; * * @author Sebastian Sdorra */ -@RunWith(MockitoJUnitRunner.class) public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Mock private ConfigurationStoreFactory factory; - @Mock - private ConfigurationStore store; - @Mock private com.google.inject.Provider repositoryManagerProvider; @@ -71,6 +70,12 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { private HookEventFacade facade = new HookEventFacade(repositoryManagerProvider, hookContextFactory); + @Override + protected void postSetUp() throws IOException, RepositoryPathNotFoundException { + initMocks(this); + super.postSetUp(); + } + @Override protected void checkDirectory(File directory) { File format = new File(directory, "format"); @@ -102,7 +107,7 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { - when(factory.getStore(any())).thenReturn(store); + when(factory.forType(any())).thenCallRealMethod(); SvnRepositoryHandler repositoryHandler = new SvnRepositoryHandler(factory, facade, locationResolver); diff --git a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java index 3c029bc781..c9355da37d 100644 --- a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java @@ -50,7 +50,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -70,12 +69,11 @@ public abstract class BlobStoreTestBase extends AbstractTestBase @Before public void createBlobStore() { - store = createBlobStoreFactory().getStore( + store = createBlobStoreFactory(). forType(Blob.class) .withName("test") .forRepository(RepositoryTestData.createHeartOfGold()) - .build() - ); + .build(); store.clear(); } diff --git a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java index df08a98f60..961ad92a70 100644 --- a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java @@ -34,8 +34,6 @@ package sonia.scm.store; import sonia.scm.repository.Repository; -import static sonia.scm.store.StoreParameters.forType; - /** * * @author Sebastian Sdorra @@ -53,18 +51,16 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB //~--- get methods ---------------------------------------------------------- @Override protected ConfigurationEntryStore getDataStore(Class type) { - StoreParameters params = forType(type) + return this.createConfigurationStoreFactory().forType(type) .withName(storeName) .build(); - return this.createConfigurationStoreFactory().getStore(params); } @Override protected ConfigurationEntryStore getDataStore(Class type, Repository repository) { - StoreParameters params = forType(type) - .withName(repoStoreName) - .forRepository(repository) - .build(); - return this.createConfigurationStoreFactory().getStore(params); + return this.createConfigurationStoreFactory().forType(type) + .withName(repoStoreName) + .forRepository(repository) + .build(); } } diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index b1d1b49ebb..71525a8352 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -39,7 +39,6 @@ import sonia.scm.repository.RepositoryTestData; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static sonia.scm.store.StoreParameters.forType; /** * @@ -57,12 +56,6 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase protected abstract DataStoreFactory createDataStoreFactory(); - protected StoreParameters getStoreParametersWithRepository(Repository repository) { - return forType(StoreObject.class) - .withName("test") - .forRepository(repository) - .build(); - } //~--- get methods ---------------------------------------------------------- @@ -76,7 +69,10 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase StoreObject obj = new StoreObject("test-1"); Repository repository = RepositoryTestData.createHeartOfGold(); - DataStore store = dataStoreFactory.getStore(getStoreParametersWithRepository(repository)); + DataStore store = dataStoreFactory.forType(StoreObject.class) + .withName("test") + .forRepository(repository) + .build(); String id = store.put(obj); diff --git a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java index 5bd665582a..7e03e51bdc 100644 --- a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java @@ -40,7 +40,6 @@ import sonia.scm.AbstractTestBase; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -67,7 +66,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testGet() { - ConfigurationStore store = createStoreFactory().getStore(forType(StoreObject.class).withName("test").build()); + ConfigurationStore store = createStoreFactory().forType(StoreObject.class).withName("test").build(); assertNotNull(store); @@ -83,7 +82,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testSet() { - ConfigurationStore store = createStoreFactory().getStore(forType(StoreObject.class).withName("test").build()); + ConfigurationStore store = createStoreFactory().forType(StoreObject.class).withName("test").build(); assertNotNull(store); diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java index 9f2bb86365..7a8c3e2afd 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java @@ -55,7 +55,6 @@ import sonia.scm.event.ScmEventBus; import sonia.scm.group.GroupEvent; import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; -import sonia.scm.store.StoreParameters; import sonia.scm.user.UserEvent; import sonia.scm.util.ClassLoaders; @@ -77,8 +76,6 @@ import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; -import static sonia.scm.store.StoreParameters.forType; - /** * TODO add events * @@ -114,10 +111,10 @@ public class DefaultSecuritySystem implements SecuritySystem @SuppressWarnings("unchecked") public DefaultSecuritySystem(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory.getStore( + store = storeFactory. forType(AssignedPermission.class) .withName(NAME) - .build()); + .build(); readAvailablePermissions(); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java index 72a7751515..d4a2331088 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java @@ -47,7 +47,6 @@ import sonia.scm.store.ConfigurationEntryStore; import sonia.scm.store.ConfigurationEntryStoreFactory; import static com.google.common.base.Preconditions.*; -import static sonia.scm.store.StoreParameters.forType; //~--- JDK imports ------------------------------------------------------------ @@ -91,10 +90,10 @@ public class SecureKeyResolver extends SigningKeyResolverAdapter @SuppressWarnings("unchecked") public SecureKeyResolver(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory.getStore( + store = storeFactory. forType(SecureKey.class) .withName(STORE_NAME) - .build()); + .build(); } //~--- methods -------------------------------------------------------------- diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java index 1dc93522a1..604d3a70e2 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java @@ -67,6 +67,7 @@ public class AutoCompleteResourceTest { xmlDB = mock(XmlDatabase.class); when(storeConfig.get()).thenReturn(xmlDB); when(storeFactory.getStore(any())).thenReturn(storeConfig); + when(storeFactory.forType(any())).thenCallRealMethod(); XmlUserDAO userDao = new XmlUserDAO(storeFactory); this.userDao = spy(userDao); XmlGroupDAO groupDAO = new XmlGroupDAO(storeFactory); diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java index 81b237de2a..8be2b4c831 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java @@ -48,6 +48,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -125,6 +126,7 @@ public class SecureKeyResolverTest { ConfigurationEntryStoreFactory factory = mock(ConfigurationEntryStoreFactory.class); + when(factory.forType(any())).thenCallRealMethod(); when(factory.getStore(argThat(storeParameters -> { assertThat(storeParameters.getName()).isEqualTo(SecureKeyResolver.STORE_NAME); assertThat(storeParameters.getType()).isEqualTo(SecureKey.class); From 923cd75ff16e66fc3a2fe4c202bfc964c390178a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 15:58:46 +0100 Subject: [PATCH 23/38] Use interface for StoreParameters --- .../java/sonia/scm/store/StoreFactory.java | 68 ++++++++++++++++++- .../java/sonia/scm/store/StoreParameters.java | 59 ++-------------- 2 files changed, 69 insertions(+), 58 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java index adf432d4e2..aae11bb201 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java @@ -1,10 +1,72 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + public interface StoreFactory { - STORE getStore(final StoreParameters storeParameters); + STORE getStore(final StoreParameters storeParameters); - default StoreParameters.WithType forType(Class type) { - return new StoreParameters<>(this).new WithType(type); + default FloatingStoreParameters.WithType forType(Class type) { + return new FloatingStoreParameters<>(this).new WithType(type); + } +} + +final class FloatingStoreParameters implements StoreParameters { + + private Class type; + private String name; + private Repository repository; + + private final StoreFactory factory; + + FloatingStoreParameters(StoreFactory factory) { + this.factory = factory; + } + + public Class getType() { + return type; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public class WithType { + + WithType(Class type) { + FloatingStoreParameters.this.type = type; + } + + public FloatingStoreParameters.WithTypeAndName withName(String name){ + return new WithTypeAndName(name); + } + + } + public class WithTypeAndName { + + private WithTypeAndName(String name) { + FloatingStoreParameters.this.name = name; + } + + public FloatingStoreParameters.WithTypeNameAndRepository forRepository(Repository repository){ + return new WithTypeNameAndRepository(repository); + } + public STORE build(){ + return factory.getStore(FloatingStoreParameters.this); + } + } + + public class WithTypeNameAndRepository { + + private WithTypeNameAndRepository(Repository repository) { + FloatingStoreParameters.this.repository = repository; + } + public STORE build(){ + return factory.getStore(FloatingStoreParameters.this); + } } } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index 8abda1c86d..c1bb2473ea 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -8,62 +8,11 @@ import sonia.scm.repository.Repository; * @author Mohamed Karray * @since 2.0.0 */ -public class StoreParameters { +public interface StoreParameters{ - private Class type; - private String name; - private Repository repository; + Class getType(); - private final StoreFactory factory; + String getName(); - StoreParameters(StoreFactory factory) { - this.factory = factory; - } - - public Class getType() { - return type; - } - - public String getName() { - return name; - } - - public Repository getRepository() { - return repository; - } - - public class WithType { - - WithType(Class type) { - StoreParameters.this.type = type; - } - - public StoreParameters.WithTypeAndName withName(String name){ - return new WithTypeAndName(name); - } - - } - public class WithTypeAndName { - - private WithTypeAndName(String name) { - StoreParameters.this.name = name; - } - - public StoreParameters.WithTypeNameAndRepository forRepository(Repository repository){ - return new WithTypeNameAndRepository(repository); - } - public STORE build(){ - return factory.getStore(StoreParameters.this); - } - } - - public class WithTypeNameAndRepository { - - private WithTypeNameAndRepository(Repository repository) { - StoreParameters.this.repository = repository; - } - public STORE build(){ - return factory.getStore(StoreParameters.this); - } - } + Repository getRepository(); } From 33f32161646ee44fe6e9f0719dad9230d8ea9351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 16:30:19 +0100 Subject: [PATCH 24/38] Make type optional --- .../repository/AbstractRepositoryHandler.java | 4 +-- .../java/sonia/scm/store/StoreFactory.java | 34 ++++++------------- .../java/sonia/scm/group/xml/XmlGroupDAO.java | 8 ++--- .../java/sonia/scm/user/xml/XmlUserDAO.java | 7 ++-- .../sonia/scm/store/FileBlobStoreTest.java | 12 +++---- .../JAXBConfigurationEntryStoreTest.java | 16 ++++----- .../scm/store/JAXBConfigurationStoreTest.java | 10 +++--- .../sonia/scm/store/JAXBDataStoreTest.java | 6 ++-- .../scm/web/lfs/LfsBlobStoreFactory.java | 2 +- .../repository/GitRepositoryHandlerTest.java | 2 +- .../scm/web/lfs/LfsBlobStoreFactoryTest.java | 2 +- .../repository/HgRepositoryHandlerTest.java | 2 +- .../repository/SvnRepositoryHandlerTest.java | 2 +- .../sonia/scm/store/BlobStoreTestBase.java | 9 +++-- .../ConfigurationEntryStoreTestBase.java | 6 ++-- .../sonia/scm/store/DataStoreTestBase.java | 3 +- .../java/sonia/scm/store/StoreTestBase.java | 4 +-- .../scm/security/DefaultSecuritySystem.java | 8 ++--- .../sonia/scm/security/SecureKeyResolver.java | 8 ++--- .../resources/AutoCompleteResourceTest.java | 2 +- .../scm/security/SecureKeyResolverTest.java | 2 +- 21 files changed, 70 insertions(+), 79 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java index d5a056e16f..776bd99548 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java @@ -73,9 +73,9 @@ public abstract class AbstractRepositoryHandler * @param storeFactory */ protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) { - this.store = storeFactory. - forType(getConfigClass()) + this.store = storeFactory .withName(getType().getName()) + .withType(getConfigClass()) .build(); } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java index aae11bb201..012c334983 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java @@ -6,8 +6,8 @@ public interface StoreFactory { STORE getStore(final StoreParameters storeParameters); - default FloatingStoreParameters.WithType forType(Class type) { - return new FloatingStoreParameters<>(this).new WithType(type); + default FloatingStoreParameters.Builder withName(String name) { + return new FloatingStoreParameters<>(this).new Builder(name); } } @@ -35,36 +35,22 @@ final class FloatingStoreParameters implements StoreParameters { return repository; } - public class WithType { + public class Builder { - WithType(Class type) { - FloatingStoreParameters.this.type = type; - } - - public FloatingStoreParameters.WithTypeAndName withName(String name){ - return new WithTypeAndName(name); - } - - } - public class WithTypeAndName { - - private WithTypeAndName(String name) { + Builder(String name) { FloatingStoreParameters.this.name = name; } - public FloatingStoreParameters.WithTypeNameAndRepository forRepository(Repository repository){ - return new WithTypeNameAndRepository(repository); + public FloatingStoreParameters.Builder withType(Class type) { + FloatingStoreParameters.this.type = type; + return this; } - public STORE build(){ - return factory.getStore(FloatingStoreParameters.this); - } - } - public class WithTypeNameAndRepository { - - private WithTypeNameAndRepository(Repository repository) { + public FloatingStoreParameters.Builder forRepository(Repository repository) { FloatingStoreParameters.this.repository = repository; + return this; } + public STORE build(){ return factory.getStore(FloatingStoreParameters.this); } diff --git a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java index 0afc34e9c2..02ea2bd248 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java @@ -65,10 +65,10 @@ public class XmlGroupDAO extends AbstractXmlDAO */ @Inject public XmlGroupDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory. - forType(XmlGroupDatabase.class) - .withName(STORE_NAME) - .build()); + super(storeFactory + .withName(STORE_NAME) + .withType(XmlGroupDatabase.class) + .build()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java index 2b97afc3ce..ca6d393a28 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java @@ -64,9 +64,10 @@ public class XmlUserDAO extends AbstractXmlDAO @Inject public XmlUserDAO(ConfigurationStoreFactory storeFactory) { - super(storeFactory.forType(XmlUserDatabase.class) - .withName(STORE_NAME) - .build()); + super(storeFactory + .withName(STORE_NAME) + .withType(XmlUserDatabase.class) + .build()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java index 5f78eb75a3..e58afe5e86 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java @@ -65,11 +65,11 @@ public class FileBlobStoreTest extends BlobStoreTestBase @Test @SuppressWarnings("unchecked") public void shouldStoreAndLoadInRepository() { - BlobStore store = createBlobStoreFactory(). - forType(StoreObject.class) - .withName("test") - .forRepository(new Repository("id", "git", "ns", "n")) - .build(); + BlobStore store = createBlobStoreFactory() + .withName("test") + .withType(StoreObject.class) + .forRepository(new Repository("id", "git", "ns", "n")) + .build(); Blob createdBlob = store.create("abc"); List storedBlobs = store.getAll(); @@ -78,6 +78,6 @@ public class FileBlobStoreTest extends BlobStoreTestBase assertThat(storedBlobs) .isNotNull() .hasSize(1) - .usingElementComparatorOnFields("id").containsExactly(createdBlob); + .usingElementComparatorOnFields("id").containsExactly(createdBlob); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java index a5c8049096..e130b0d205 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java @@ -132,10 +132,10 @@ public class JAXBConfigurationEntryStoreTest ConfigurationEntryStore store = createPermissionStore(RESOURCE_FIXED, name); store.put("a45", new AssignedPermission("tuser4", "repository:create")); - store = createConfigurationStoreFactory(). - forType(AssignedPermission.class) - .withName(name) - .build(); + store = createConfigurationStoreFactory() + .withName(name) + .withType(AssignedPermission.class) + .build(); AssignedPermission ap = store.get("a45"); @@ -231,9 +231,9 @@ public class JAXBConfigurationEntryStoreTest } copy(resource, name); - return createConfigurationStoreFactory(). - forType(AssignedPermission.class) - .withName(name) - .build(); + return createConfigurationStoreFactory() + .withName(name) + .withType(AssignedPermission.class) + .build(); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java index 4760f2b0fc..4e3dc29faa 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java @@ -56,11 +56,11 @@ public class JAXBConfigurationStoreTest extends StoreTestBase { @SuppressWarnings("unchecked") public void shouldStoreAndLoadInRepository() { - ConfigurationStore store = createStoreFactory(). - forType(StoreObject.class) - .withName("test") - .forRepository(new Repository("id", "git", "ns", "n")) - .build(); + ConfigurationStore store = createStoreFactory() + .withName("test") + .withType(StoreObject.class) + .forRepository(new Repository("id", "git", "ns", "n")) + .build(); store.set(new StoreObject("value")); StoreObject storeObject = store.get(); 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 e3df72c2e7..b06f06fb9c 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 @@ -61,16 +61,18 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStore getDataStore(Class type, Repository repository) { - return createDataStoreFactory().forType(type) + return createDataStoreFactory() .withName("test") + .withType(type) .forRepository(repository) .build(); } @Override protected DataStore getDataStore(Class type) { - return createDataStoreFactory().forType(type) + return createDataStoreFactory() .withName("test") + .withType(type) .build(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java index 59fff8cd2f..84733b9ea3 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsBlobStoreFactory.java @@ -76,7 +76,7 @@ public class LfsBlobStoreFactory { */ @SuppressWarnings("unchecked") public BlobStore getLfsBlobStore(Repository repository) { - return blobStoreFactory.forType(String.class) + return blobStoreFactory .withName(repository.getId() + GIT_LFS_REPOSITORY_POSTFIX) .forRepository(repository) .build(); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index ed25dc2f6b..bc4c702320 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -86,7 +86,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Before public void initFactory() { - when(factory.forType(any())).thenCallRealMethod(); + when(factory.withName(any())).thenCallRealMethod(); } @Override diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java index 23077a9f5f..93eadf8935 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LfsBlobStoreFactoryTest.java @@ -63,7 +63,7 @@ public class LfsBlobStoreFactoryTest { @Test public void getBlobStore() { - when(blobStoreFactory.forType(any())).thenCallRealMethod(); + when(blobStoreFactory.withName(any())).thenCallRealMethod(); Repository repository = new Repository("the-id", "GIT", "space", "the-name"); lfsBlobStoreFactory.getLfsBlobStore(repository); diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java index 9f351ba9e1..7e534e3453 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java @@ -72,7 +72,7 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Before public void initFactory() { - when(factory.forType(any())).thenCallRealMethod(); + when(factory.withName(any())).thenCallRealMethod(); } @Override diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java index 4b7f6486b4..8f366b2ec3 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java @@ -107,7 +107,7 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { - when(factory.forType(any())).thenCallRealMethod(); + when(factory.withName(any())).thenCallRealMethod(); SvnRepositoryHandler repositoryHandler = new SvnRepositoryHandler(factory, facade, locationResolver); diff --git a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java index c9355da37d..f3f252053d 100644 --- a/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/BlobStoreTestBase.java @@ -69,11 +69,10 @@ public abstract class BlobStoreTestBase extends AbstractTestBase @Before public void createBlobStore() { - store = createBlobStoreFactory(). - forType(Blob.class) - .withName("test") - .forRepository(RepositoryTestData.createHeartOfGold()) - .build(); + store = createBlobStoreFactory() + .withName("test") + .forRepository(RepositoryTestData.createHeartOfGold()) + .build(); store.clear(); } diff --git a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java index 961ad92a70..8ed7e33eb8 100644 --- a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java @@ -51,15 +51,17 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB //~--- get methods ---------------------------------------------------------- @Override protected ConfigurationEntryStore getDataStore(Class type) { - return this.createConfigurationStoreFactory().forType(type) + return this.createConfigurationStoreFactory() .withName(storeName) + .withType(type) .build(); } @Override protected ConfigurationEntryStore getDataStore(Class type, Repository repository) { - return this.createConfigurationStoreFactory().forType(type) + return this.createConfigurationStoreFactory() .withName(repoStoreName) + .withType(type) .forRepository(repository) .build(); } diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index 71525a8352..ce00290ee5 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -69,8 +69,9 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase StoreObject obj = new StoreObject("test-1"); Repository repository = RepositoryTestData.createHeartOfGold(); - DataStore store = dataStoreFactory.forType(StoreObject.class) + DataStore store = dataStoreFactory .withName("test") + .withType(StoreObject.class) .forRepository(repository) .build(); diff --git a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java index 7e03e51bdc..41acb043a7 100644 --- a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java @@ -66,7 +66,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testGet() { - ConfigurationStore store = createStoreFactory().forType(StoreObject.class).withName("test").build(); + ConfigurationStore store = createStoreFactory().withName("test").withType(StoreObject.class).build(); assertNotNull(store); @@ -82,7 +82,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testSet() { - ConfigurationStore store = createStoreFactory().forType(StoreObject.class).withName("test").build(); + ConfigurationStore store = createStoreFactory().withName("test").withType(StoreObject.class).build(); assertNotNull(store); diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java index 7a8c3e2afd..1a1b1c2985 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java @@ -111,10 +111,10 @@ public class DefaultSecuritySystem implements SecuritySystem @SuppressWarnings("unchecked") public DefaultSecuritySystem(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory. - forType(AssignedPermission.class) - .withName(NAME) - .build(); + store = storeFactory + .withName(NAME) + .withType(AssignedPermission.class) + .build(); readAvailablePermissions(); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java index d4a2331088..700870db7c 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java @@ -90,10 +90,10 @@ public class SecureKeyResolver extends SigningKeyResolverAdapter @SuppressWarnings("unchecked") public SecureKeyResolver(ConfigurationEntryStoreFactory storeFactory) { - store = storeFactory. - forType(SecureKey.class) - .withName(STORE_NAME) - .build(); + store = storeFactory + .withName(STORE_NAME) + .withType(SecureKey.class) + .build(); } //~--- methods -------------------------------------------------------------- diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java index 604d3a70e2..435997633b 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java @@ -67,7 +67,7 @@ public class AutoCompleteResourceTest { xmlDB = mock(XmlDatabase.class); when(storeConfig.get()).thenReturn(xmlDB); when(storeFactory.getStore(any())).thenReturn(storeConfig); - when(storeFactory.forType(any())).thenCallRealMethod(); + when(storeFactory.withName(any())).thenCallRealMethod(); XmlUserDAO userDao = new XmlUserDAO(storeFactory); this.userDao = spy(userDao); XmlGroupDAO groupDAO = new XmlGroupDAO(storeFactory); diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java index 8be2b4c831..7609fd69a9 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java @@ -126,7 +126,7 @@ public class SecureKeyResolverTest { ConfigurationEntryStoreFactory factory = mock(ConfigurationEntryStoreFactory.class); - when(factory.forType(any())).thenCallRealMethod(); + when(factory.withName(any())).thenCallRealMethod(); when(factory.getStore(argThat(storeParameters -> { assertThat(storeParameters.getName()).isEqualTo(SecureKeyResolver.STORE_NAME); assertThat(storeParameters.getType()).isEqualTo(SecureKey.class); From 3021bea65af5c3d8a0a5cbbc825ae8ff323e473b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 08:56:39 +0100 Subject: [PATCH 25/38] Multiply floating store factories for type safety --- .../repository/AbstractRepositoryHandler.java | 2 +- .../sonia/scm/store/BlobStoreFactory.java | 49 ++++++++++++++- .../store/ConfigurationEntryStoreFactory.java | 58 +++++++++++++++++- .../scm/store/ConfigurationStoreFactory.java | 59 ++++++++++++++++++- .../sonia/scm/store/DataStoreFactory.java | 59 ++++++++++++++++++- .../java/sonia/scm/store/StoreFactory.java | 58 ------------------ .../java/sonia/scm/store/StoreParameters.java | 4 +- .../java/sonia/scm/group/xml/XmlGroupDAO.java | 2 +- .../java/sonia/scm/user/xml/XmlUserDAO.java | 2 +- .../sonia/scm/store/FileBlobStoreTest.java | 1 - .../JAXBConfigurationEntryStoreTest.java | 4 +- .../scm/store/JAXBConfigurationStoreTest.java | 2 +- .../sonia/scm/store/JAXBDataStoreTest.java | 4 +- .../repository/GitRepositoryHandlerTest.java | 2 +- .../repository/HgRepositoryHandlerTest.java | 2 +- .../repository/SvnRepositoryHandlerTest.java | 2 +- .../ConfigurationEntryStoreTestBase.java | 4 +- .../sonia/scm/store/DataStoreTestBase.java | 4 +- .../java/sonia/scm/store/StoreTestBase.java | 4 +- .../scm/security/DefaultSecuritySystem.java | 2 +- .../sonia/scm/security/SecureKeyResolver.java | 2 +- .../resources/AutoCompleteResourceTest.java | 2 +- .../scm/security/SecureKeyResolverTest.java | 4 +- 23 files changed, 245 insertions(+), 87 deletions(-) delete mode 100644 scm-core/src/main/java/sonia/scm/store/StoreFactory.java diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java index 776bd99548..a3e8a1da73 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractRepositoryHandler.java @@ -74,8 +74,8 @@ public abstract class AbstractRepositoryHandler */ protected AbstractRepositoryHandler(ConfigurationStoreFactory storeFactory) { this.store = storeFactory - .withName(getType().getName()) .withType(getConfigClass()) + .withName(getType().getName()) .build(); } diff --git a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java index 7d0462d371..0ed0c07869 100644 --- a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java @@ -32,6 +32,8 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + /** * The BlobStoreFactory can be used to create new or get existing * {@link BlobStore}s. @@ -42,6 +44,51 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.BlobStore */ -public interface BlobStoreFactory extends StoreFactory { +public interface BlobStoreFactory { + BlobStore getStore(final StoreParameters storeParameters); + + default FloatingStoreParameters.Builder withName(String name) { + return new FloatingStoreParameters(this).new Builder(name); + } +} + +final class FloatingStoreParameters implements StoreParameters { + + private String name; + private Repository repository; + + private final BlobStoreFactory factory; + + FloatingStoreParameters(BlobStoreFactory factory) { + this.factory = factory; + } + + public Class getType() { + return null; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public class Builder { + + Builder(String name) { + FloatingStoreParameters.this.name = name; + } + + public FloatingStoreParameters.Builder forRepository(Repository repository) { + FloatingStoreParameters.this.repository = repository; + return this; + } + + public BlobStore build(){ + return factory.getStore(FloatingStoreParameters.this); + } + } } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index 4a1202e28c..945d3b9742 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -32,6 +32,8 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + /** * The ConfigurationEntryStoreFactory can be used to create new or get existing * {@link ConfigurationEntryStore}s. @@ -45,5 +47,59 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.ConfigurationEntryStore */ -public interface ConfigurationEntryStoreFactory extends StoreFactory { +public interface ConfigurationEntryStoreFactory { + ConfigurationEntryStore getStore(final StoreParameters storeParameters); + + default TypedFloatingConfigurationEntryStoreParameters.Builder withType(Class type) { + return new TypedFloatingConfigurationEntryStoreParameters(this).new Builder(type); + } +} + +final class TypedFloatingConfigurationEntryStoreParameters implements StoreParameters { + + private Class type; + private String name; + private Repository repository; + + private final ConfigurationEntryStoreFactory factory; + + TypedFloatingConfigurationEntryStoreParameters(ConfigurationEntryStoreFactory factory) { + this.factory = factory; + } + + public Class getType() { + return type; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public class Builder { + + Builder(Class type) { + TypedFloatingConfigurationEntryStoreParameters.this.type = type; + } + + public OptionalRepositoryBuilder withName(String name) { + TypedFloatingConfigurationEntryStoreParameters.this.name = name; + return new OptionalRepositoryBuilder(); + } + } + + public class OptionalRepositoryBuilder { + + public OptionalRepositoryBuilder forRepository(Repository repository) { + TypedFloatingConfigurationEntryStoreParameters.this.repository = repository; + return this; + } + + public ConfigurationEntryStore build(){ + return factory.getStore(TypedFloatingConfigurationEntryStoreParameters.this); + } + } } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 6cf9541139..030be50f9e 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -33,6 +33,8 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + /** * The ConfigurationStoreFactory can be used to create new or get existing * {@link ConfigurationStore} objects. @@ -45,4 +47,59 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.ConfigurationStore */ -public interface ConfigurationStoreFactory extends StoreFactory{} +public interface ConfigurationStoreFactory { + ConfigurationStore getStore(final StoreParameters storeParameters); + + default TypedFloatingConfigurationStoreParameters.Builder withType(Class type) { + return new TypedFloatingConfigurationStoreParameters(this).new Builder(type); + } +} + +final class TypedFloatingConfigurationStoreParameters implements StoreParameters { + + private Class type; + private String name; + private Repository repository; + + private final ConfigurationStoreFactory factory; + + TypedFloatingConfigurationStoreParameters(ConfigurationStoreFactory factory) { + this.factory = factory; + } + + public Class getType() { + return type; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public class Builder { + + Builder(Class type) { + TypedFloatingConfigurationStoreParameters.this.type = type; + } + + public OptionalRepositoryBuilder withName(String name) { + TypedFloatingConfigurationStoreParameters.this.name = name; + return new OptionalRepositoryBuilder(); + } + } + + public class OptionalRepositoryBuilder { + + public OptionalRepositoryBuilder forRepository(Repository repository) { + TypedFloatingConfigurationStoreParameters.this.repository = repository; + return this; + } + + public ConfigurationStore build(){ + return factory.getStore(TypedFloatingConfigurationStoreParameters.this); + } + } +} diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index c1e171a74b..ff324a9af4 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -32,6 +32,8 @@ package sonia.scm.store; +import sonia.scm.repository.Repository; + /** * The DataStoreFactory can be used to create new or get existing * {@link DataStore}s. @@ -42,4 +44,59 @@ package sonia.scm.store; * @apiviz.landmark * @apiviz.uses sonia.scm.store.DataStore */ -public interface DataStoreFactory extends StoreFactory{} +public interface DataStoreFactory { + DataStore getStore(final StoreParameters storeParameters); + + default TypedFloatingDataStoreParameters.Builder withType(Class type) { + return new TypedFloatingDataStoreParameters(this).new Builder(type); + } +} + +final class TypedFloatingDataStoreParameters implements StoreParameters { + + private Class type; + private String name; + private Repository repository; + + private final DataStoreFactory factory; + + TypedFloatingDataStoreParameters(DataStoreFactory factory) { + this.factory = factory; + } + + public Class getType() { + return type; + } + + public String getName() { + return name; + } + + public Repository getRepository() { + return repository; + } + + public class Builder { + + Builder(Class type) { + TypedFloatingDataStoreParameters.this.type = type; + } + + public OptionalRepositoryBuilder withName(String name) { + TypedFloatingDataStoreParameters.this.name = name; + return new OptionalRepositoryBuilder(); + } + } + + public class OptionalRepositoryBuilder { + + public OptionalRepositoryBuilder forRepository(Repository repository) { + TypedFloatingDataStoreParameters.this.repository = repository; + return this; + } + + public DataStore build(){ + return factory.getStore(TypedFloatingDataStoreParameters.this); + } + } +} diff --git a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java b/scm-core/src/main/java/sonia/scm/store/StoreFactory.java deleted file mode 100644 index 012c334983..0000000000 --- a/scm-core/src/main/java/sonia/scm/store/StoreFactory.java +++ /dev/null @@ -1,58 +0,0 @@ -package sonia.scm.store; - -import sonia.scm.repository.Repository; - -public interface StoreFactory { - - STORE getStore(final StoreParameters storeParameters); - - default FloatingStoreParameters.Builder withName(String name) { - return new FloatingStoreParameters<>(this).new Builder(name); - } -} - -final class FloatingStoreParameters implements StoreParameters { - - private Class type; - private String name; - private Repository repository; - - private final StoreFactory factory; - - FloatingStoreParameters(StoreFactory factory) { - this.factory = factory; - } - - public Class getType() { - return type; - } - - public String getName() { - return name; - } - - public Repository getRepository() { - return repository; - } - - public class Builder { - - Builder(String name) { - FloatingStoreParameters.this.name = name; - } - - public FloatingStoreParameters.Builder withType(Class type) { - FloatingStoreParameters.this.type = type; - return this; - } - - public FloatingStoreParameters.Builder forRepository(Repository repository) { - FloatingStoreParameters.this.repository = repository; - return this; - } - - public STORE build(){ - return factory.getStore(FloatingStoreParameters.this); - } - } -} diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index c1bb2473ea..c46611ac40 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -8,9 +8,9 @@ import sonia.scm.repository.Repository; * @author Mohamed Karray * @since 2.0.0 */ -public interface StoreParameters{ +public interface StoreParameters { - Class getType(); + Class getType(); String getName(); diff --git a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java index 02ea2bd248..d6b65b41bd 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDAO.java @@ -66,8 +66,8 @@ public class XmlGroupDAO extends AbstractXmlDAO @Inject public XmlGroupDAO(ConfigurationStoreFactory storeFactory) { super(storeFactory - .withName(STORE_NAME) .withType(XmlGroupDatabase.class) + .withName(STORE_NAME) .build()); } diff --git a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java index ca6d393a28..ea7f18fbba 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDAO.java @@ -65,8 +65,8 @@ public class XmlUserDAO extends AbstractXmlDAO public XmlUserDAO(ConfigurationStoreFactory storeFactory) { super(storeFactory - .withName(STORE_NAME) .withType(XmlUserDatabase.class) + .withName(STORE_NAME) .build()); } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java index e58afe5e86..3ec16baa57 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/FileBlobStoreTest.java @@ -67,7 +67,6 @@ public class FileBlobStoreTest extends BlobStoreTestBase public void shouldStoreAndLoadInRepository() { BlobStore store = createBlobStoreFactory() .withName("test") - .withType(StoreObject.class) .forRepository(new Repository("id", "git", "ns", "n")) .build(); diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java index e130b0d205..ae84f9d768 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationEntryStoreTest.java @@ -133,8 +133,8 @@ public class JAXBConfigurationEntryStoreTest store.put("a45", new AssignedPermission("tuser4", "repository:create")); store = createConfigurationStoreFactory() - .withName(name) .withType(AssignedPermission.class) + .withName(name) .build(); AssignedPermission ap = store.get("a45"); @@ -232,8 +232,8 @@ public class JAXBConfigurationEntryStoreTest copy(resource, name); return createConfigurationStoreFactory() - .withName(name) .withType(AssignedPermission.class) + .withName(name) .build(); } } diff --git a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java index 4e3dc29faa..802f193340 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/store/JAXBConfigurationStoreTest.java @@ -57,8 +57,8 @@ public class JAXBConfigurationStoreTest extends StoreTestBase { public void shouldStoreAndLoadInRepository() { ConfigurationStore store = createStoreFactory() - .withName("test") .withType(StoreObject.class) + .withName("test") .forRepository(new Repository("id", "git", "ns", "n")) .build(); 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 b06f06fb9c..04d86aa625 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 @@ -62,8 +62,8 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStore getDataStore(Class type, Repository repository) { return createDataStoreFactory() - .withName("test") .withType(type) + .withName("test") .forRepository(repository) .build(); } @@ -71,8 +71,8 @@ public class JAXBDataStoreTest extends DataStoreTestBase { @Override protected DataStore getDataStore(Class type) { return createDataStoreFactory() - .withName("test") .withType(type) + .withName("test") .build(); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java index bc4c702320..dbd67a7f8e 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryHandlerTest.java @@ -86,7 +86,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Before public void initFactory() { - when(factory.withName(any())).thenCallRealMethod(); + when(factory.withType(any())).thenCallRealMethod(); } @Override diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java index 7e534e3453..c45d9ab358 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java @@ -72,7 +72,7 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Before public void initFactory() { - when(factory.withName(any())).thenCallRealMethod(); + when(factory.withType(any())).thenCallRealMethod(); } @Override diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java index 8f366b2ec3..7b22e15c94 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java @@ -107,7 +107,7 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { - when(factory.withName(any())).thenCallRealMethod(); + when(factory.withType(any())).thenCallRealMethod(); SvnRepositoryHandler repositoryHandler = new SvnRepositoryHandler(factory, facade, locationResolver); diff --git a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java index 8ed7e33eb8..140bd54e65 100644 --- a/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/ConfigurationEntryStoreTestBase.java @@ -52,16 +52,16 @@ public abstract class ConfigurationEntryStoreTestBase extends KeyValueStoreTestB @Override protected ConfigurationEntryStore getDataStore(Class type) { return this.createConfigurationStoreFactory() - .withName(storeName) .withType(type) + .withName(storeName) .build(); } @Override protected ConfigurationEntryStore getDataStore(Class type, Repository repository) { return this.createConfigurationStoreFactory() - .withName(repoStoreName) .withType(type) + .withName(repoStoreName) .forRepository(repository) .build(); } diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index ce00290ee5..f109e55119 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -65,13 +65,13 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase // TODO public void shouldStoreRepositorySpecificData() { - StoreFactory dataStoreFactory = createDataStoreFactory(); + DataStoreFactory dataStoreFactory = createDataStoreFactory(); StoreObject obj = new StoreObject("test-1"); Repository repository = RepositoryTestData.createHeartOfGold(); DataStore store = dataStoreFactory - .withName("test") .withType(StoreObject.class) + .withName("test") .forRepository(repository) .build(); diff --git a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java index 41acb043a7..ef806c79f8 100644 --- a/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/StoreTestBase.java @@ -66,7 +66,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testGet() { - ConfigurationStore store = createStoreFactory().withName("test").withType(StoreObject.class).build(); + ConfigurationStore store = createStoreFactory().withType(StoreObject.class).withName("test").build(); assertNotNull(store); @@ -82,7 +82,7 @@ public abstract class StoreTestBase extends AbstractTestBase @Test public void testSet() { - ConfigurationStore store = createStoreFactory().withName("test").withType(StoreObject.class).build(); + ConfigurationStore store = createStoreFactory().withType(StoreObject.class).withName("test").build(); assertNotNull(store); diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java index 1a1b1c2985..e93d4de597 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java @@ -112,8 +112,8 @@ public class DefaultSecuritySystem implements SecuritySystem public DefaultSecuritySystem(ConfigurationEntryStoreFactory storeFactory) { store = storeFactory - .withName(NAME) .withType(AssignedPermission.class) + .withName(NAME) .build(); readAvailablePermissions(); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java index 700870db7c..a369db66bd 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/security/SecureKeyResolver.java @@ -91,8 +91,8 @@ public class SecureKeyResolver extends SigningKeyResolverAdapter public SecureKeyResolver(ConfigurationEntryStoreFactory storeFactory) { store = storeFactory - .withName(STORE_NAME) .withType(SecureKey.class) + .withName(STORE_NAME) .build(); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java index 435997633b..d392aefe4e 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AutoCompleteResourceTest.java @@ -67,7 +67,7 @@ public class AutoCompleteResourceTest { xmlDB = mock(XmlDatabase.class); when(storeConfig.get()).thenReturn(xmlDB); when(storeFactory.getStore(any())).thenReturn(storeConfig); - when(storeFactory.withName(any())).thenCallRealMethod(); + when(storeFactory.withType(any())).thenCallRealMethod(); XmlUserDAO userDao = new XmlUserDAO(storeFactory); this.userDao = spy(userDao); XmlGroupDAO groupDAO = new XmlGroupDAO(storeFactory); diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java index 7609fd69a9..c4f281537e 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyResolverTest.java @@ -126,8 +126,8 @@ public class SecureKeyResolverTest { ConfigurationEntryStoreFactory factory = mock(ConfigurationEntryStoreFactory.class); - when(factory.withName(any())).thenCallRealMethod(); - when(factory.getStore(argThat(storeParameters -> { + when(factory.withType(any())).thenCallRealMethod(); + when(factory.getStore(argThat(storeParameters -> { assertThat(storeParameters.getName()).isEqualTo(SecureKeyResolver.STORE_NAME); assertThat(storeParameters.getType()).isEqualTo(SecureKey.class); return true; From 8952e755df923d205267cc33b8ea5e4019bf4442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 09:23:12 +0100 Subject: [PATCH 26/38] Split up store parameters into typed and untyped --- .../sonia/scm/store/BlobStoreFactory.java | 6 ++---- .../store/ConfigurationEntryStoreFactory.java | 7 +++++-- .../scm/store/ConfigurationStoreFactory.java | 7 +++++-- .../sonia/scm/store/DataStoreFactory.java | 7 +++++-- .../java/sonia/scm/store/StoreParameters.java | 6 ++---- .../sonia/scm/store/TypedStoreParameters.java | 19 +++++++++++++++++++ .../scm/store/FileBasedStoreFactory.java | 4 ++++ .../JAXBConfigurationEntryStoreFactory.java | 5 ++--- .../store/JAXBConfigurationStoreFactory.java | 7 +++---- .../sonia/scm/store/JAXBDataStoreFactory.java | 6 ++---- .../InMemoryConfigurationStoreFactory.java | 2 +- 11 files changed, 50 insertions(+), 26 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/store/TypedStoreParameters.java diff --git a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java index 0ed0c07869..e86f62a136 100644 --- a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java @@ -64,14 +64,12 @@ final class FloatingStoreParameters implements StoreParameters { this.factory = factory; } - public Class getType() { - return null; - } - + @Override public String getName() { return name; } + @Override public Repository getRepository() { return repository; } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index 945d3b9742..9b5486bc22 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -48,14 +48,14 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.ConfigurationEntryStore */ public interface ConfigurationEntryStoreFactory { - ConfigurationEntryStore getStore(final StoreParameters storeParameters); + ConfigurationEntryStore getStore(final TypedStoreParameters storeParameters); default TypedFloatingConfigurationEntryStoreParameters.Builder withType(Class type) { return new TypedFloatingConfigurationEntryStoreParameters(this).new Builder(type); } } -final class TypedFloatingConfigurationEntryStoreParameters implements StoreParameters { +final class TypedFloatingConfigurationEntryStoreParameters implements TypedStoreParameters { private Class type; private String name; @@ -67,14 +67,17 @@ final class TypedFloatingConfigurationEntryStoreParameters implements StorePa this.factory = factory; } + @Override public Class getType() { return type; } + @Override public String getName() { return name; } + @Override public Repository getRepository() { return repository; } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 030be50f9e..7b96ea2b17 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -48,14 +48,14 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.ConfigurationStore */ public interface ConfigurationStoreFactory { - ConfigurationStore getStore(final StoreParameters storeParameters); + ConfigurationStore getStore(final TypedStoreParameters storeParameters); default TypedFloatingConfigurationStoreParameters.Builder withType(Class type) { return new TypedFloatingConfigurationStoreParameters(this).new Builder(type); } } -final class TypedFloatingConfigurationStoreParameters implements StoreParameters { +final class TypedFloatingConfigurationStoreParameters implements TypedStoreParameters { private Class type; private String name; @@ -67,14 +67,17 @@ final class TypedFloatingConfigurationStoreParameters implements StoreParamet this.factory = factory; } + @Override public Class getType() { return type; } + @Override public String getName() { return name; } + @Override public Repository getRepository() { return repository; } diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index ff324a9af4..ba1c9cc6c2 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -45,14 +45,14 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.DataStore */ public interface DataStoreFactory { - DataStore getStore(final StoreParameters storeParameters); + DataStore getStore(final TypedStoreParameters storeParameters); default TypedFloatingDataStoreParameters.Builder withType(Class type) { return new TypedFloatingDataStoreParameters(this).new Builder(type); } } -final class TypedFloatingDataStoreParameters implements StoreParameters { +final class TypedFloatingDataStoreParameters implements TypedStoreParameters { private Class type; private String name; @@ -64,14 +64,17 @@ final class TypedFloatingDataStoreParameters implements StoreParameters { this.factory = factory; } + @Override public Class getType() { return type; } + @Override public String getName() { return name; } + @Override public Repository getRepository() { return repository; } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index c46611ac40..da8ee4c916 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java @@ -3,14 +3,12 @@ package sonia.scm.store; import sonia.scm.repository.Repository; /** - * The fields of the {@link StoreParameters} are used from the {@link StoreFactory} to create a store. + * The fields of the {@link StoreParameters} are used from the {@link BlobStoreFactory} to create a store. * * @author Mohamed Karray * @since 2.0.0 */ -public interface StoreParameters { - - Class getType(); +public interface StoreParameters { String getName(); diff --git a/scm-core/src/main/java/sonia/scm/store/TypedStoreParameters.java b/scm-core/src/main/java/sonia/scm/store/TypedStoreParameters.java new file mode 100644 index 0000000000..116bccac41 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/store/TypedStoreParameters.java @@ -0,0 +1,19 @@ +package sonia.scm.store; + +import sonia.scm.repository.Repository; + +/** + * The fields of the {@link TypedStoreParameters} are used from the {@link ConfigurationStoreFactory}, + * {@link ConfigurationEntryStoreFactory} and {@link DataStoreFactory} to create a type safe store. + * + * @author Mohamed Karray + * @since 2.0.0 + */ +public interface TypedStoreParameters { + + Class getType(); + + String getName(); + + Repository getRepository(); +} diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java index dfb7bd5d38..099ab53baa 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/FileBasedStoreFactory.java @@ -67,6 +67,10 @@ public abstract class FileBasedStoreFactory { } protected File getStoreLocation(StoreParameters storeParameters) { + return getStoreLocation(storeParameters.getName(), null, storeParameters.getRepository()); + } + + protected File getStoreLocation(TypedStoreParameters storeParameters) { return getStoreLocation(storeParameters.getName(), storeParameters.getType(), storeParameters.getRepository()); } 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 bd18743d01..96403140ef 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 @@ -58,9 +58,8 @@ public class JAXBConfigurationEntryStoreFactory extends FileBasedStoreFactory } @Override - @SuppressWarnings("unchecked") - public ConfigurationEntryStore getStore(StoreParameters storeParameters) { - return new JAXBConfigurationEntryStore(getStoreLocation(storeParameters.getName().concat(StoreConstants.FILE_EXTENSION), storeParameters.getType(), storeParameters.getRepository()), keyGenerator, storeParameters.getType()); + public ConfigurationEntryStore getStore(TypedStoreParameters storeParameters) { + return new JAXBConfigurationEntryStore<>(getStoreLocation(storeParameters.getName().concat(StoreConstants.FILE_EXTENSION), storeParameters.getType(), storeParameters.getRepository()), keyGenerator, 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 028695c0e7..bb68ab93dc 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 @@ -36,7 +36,7 @@ import sonia.scm.SCMContextProvider; import sonia.scm.repository.RepositoryLocationResolver; /** - * JAXB implementation of {@link StoreFactory}. + * JAXB implementation of {@link ConfigurationStoreFactory}. * * @author Sebastian Sdorra */ @@ -54,8 +54,7 @@ public class JAXBConfigurationStoreFactory extends FileBasedStoreFactory impleme } @Override - @SuppressWarnings("unchecked") - public JAXBConfigurationStore getStore(StoreParameters storeParameters) { - return new JAXBConfigurationStore(storeParameters.getType(), getStoreLocation(storeParameters.getName().concat(StoreConstants.FILE_EXTENSION), storeParameters.getType(), storeParameters.getRepository())); + public JAXBConfigurationStore getStore(TypedStoreParameters storeParameters) { + return new JAXBConfigurationStore<>(storeParameters.getType(), getStoreLocation(storeParameters.getName().concat(StoreConstants.FILE_EXTENSION), storeParameters.getType(), storeParameters.getRepository())); } } 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 b3f6bdfe17..5b5c00a298 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 @@ -55,7 +55,6 @@ import java.io.File; public class JAXBDataStoreFactory extends FileBasedStoreFactory implements DataStoreFactory { - private static final Logger logger = LoggerFactory.getLogger(JAXBDataStoreFactory.class); private KeyGenerator keyGenerator; @Inject @@ -65,10 +64,9 @@ public class JAXBDataStoreFactory extends FileBasedStoreFactory } @Override - @SuppressWarnings("unchecked") - public DataStore getStore(StoreParameters storeParameters) { + public DataStore getStore(TypedStoreParameters storeParameters) { File storeLocation = getStoreLocation(storeParameters); IOUtil.mkdirs(storeLocation); - return new JAXBDataStore(keyGenerator, storeParameters.getType(), storeLocation); + return new JAXBDataStore<>(keyGenerator, storeParameters.getType(), storeLocation); } } diff --git a/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java b/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java index 021b7bda02..2c5641bfd1 100644 --- a/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java +++ b/scm-test/src/main/java/sonia/scm/store/InMemoryConfigurationStoreFactory.java @@ -43,7 +43,7 @@ package sonia.scm.store; public class InMemoryConfigurationStoreFactory implements ConfigurationStoreFactory { @Override - public ConfigurationStore getStore(StoreParameters storeParameters) { + public ConfigurationStore getStore(TypedStoreParameters storeParameters) { return new InMemoryConfigurationStore<>(); } } From c39d9bdc48a2706951feb4efac10e14fa788235e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 11:38:09 +0100 Subject: [PATCH 27/38] Remove redundant code --- .../store/ConfigurationEntryStoreFactory.java | 30 ++++------------ .../scm/store/ConfigurationStoreFactory.java | 30 ++++------------ .../sonia/scm/store/DataStoreFactory.java | 30 ++++------------ .../scm/store/TypedStoreParametersImpl.java | 36 +++++++++++++++++++ 4 files changed, 54 insertions(+), 72 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/store/TypedStoreParametersImpl.java diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index 9b5486bc22..b77eb2c0f1 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -55,41 +55,23 @@ public interface ConfigurationEntryStoreFactory { } } -final class TypedFloatingConfigurationEntryStoreParameters implements TypedStoreParameters { - - private Class type; - private String name; - private Repository repository; +final class TypedFloatingConfigurationEntryStoreParameters { + private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); private final ConfigurationEntryStoreFactory factory; TypedFloatingConfigurationEntryStoreParameters(ConfigurationEntryStoreFactory factory) { this.factory = factory; } - @Override - public Class getType() { - return type; - } - - @Override - public String getName() { - return name; - } - - @Override - public Repository getRepository() { - return repository; - } - public class Builder { Builder(Class type) { - TypedFloatingConfigurationEntryStoreParameters.this.type = type; + parameters.setType(type); } public OptionalRepositoryBuilder withName(String name) { - TypedFloatingConfigurationEntryStoreParameters.this.name = name; + parameters.setName(name); return new OptionalRepositoryBuilder(); } } @@ -97,12 +79,12 @@ final class TypedFloatingConfigurationEntryStoreParameters implements TypedSt public class OptionalRepositoryBuilder { public OptionalRepositoryBuilder forRepository(Repository repository) { - TypedFloatingConfigurationEntryStoreParameters.this.repository = repository; + parameters.setRepository(repository); return this; } public ConfigurationEntryStore build(){ - return factory.getStore(TypedFloatingConfigurationEntryStoreParameters.this); + return factory.getStore(parameters); } } } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 7b96ea2b17..55a6baf83d 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -55,41 +55,23 @@ public interface ConfigurationStoreFactory { } } -final class TypedFloatingConfigurationStoreParameters implements TypedStoreParameters { - - private Class type; - private String name; - private Repository repository; +final class TypedFloatingConfigurationStoreParameters { + private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); private final ConfigurationStoreFactory factory; TypedFloatingConfigurationStoreParameters(ConfigurationStoreFactory factory) { this.factory = factory; } - @Override - public Class getType() { - return type; - } - - @Override - public String getName() { - return name; - } - - @Override - public Repository getRepository() { - return repository; - } - public class Builder { Builder(Class type) { - TypedFloatingConfigurationStoreParameters.this.type = type; + parameters.setType(type); } public OptionalRepositoryBuilder withName(String name) { - TypedFloatingConfigurationStoreParameters.this.name = name; + parameters.setName(name); return new OptionalRepositoryBuilder(); } } @@ -97,12 +79,12 @@ final class TypedFloatingConfigurationStoreParameters implements TypedStorePa public class OptionalRepositoryBuilder { public OptionalRepositoryBuilder forRepository(Repository repository) { - TypedFloatingConfigurationStoreParameters.this.repository = repository; + parameters.setRepository(repository); return this; } public ConfigurationStore build(){ - return factory.getStore(TypedFloatingConfigurationStoreParameters.this); + return factory.getStore(parameters); } } } diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index ba1c9cc6c2..bb2f0a37ad 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -52,41 +52,23 @@ public interface DataStoreFactory { } } -final class TypedFloatingDataStoreParameters implements TypedStoreParameters { - - private Class type; - private String name; - private Repository repository; +final class TypedFloatingDataStoreParameters { + private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); private final DataStoreFactory factory; TypedFloatingDataStoreParameters(DataStoreFactory factory) { this.factory = factory; } - @Override - public Class getType() { - return type; - } - - @Override - public String getName() { - return name; - } - - @Override - public Repository getRepository() { - return repository; - } - public class Builder { Builder(Class type) { - TypedFloatingDataStoreParameters.this.type = type; + parameters.setType(type); } public OptionalRepositoryBuilder withName(String name) { - TypedFloatingDataStoreParameters.this.name = name; + parameters.setName(name); return new OptionalRepositoryBuilder(); } } @@ -94,12 +76,12 @@ final class TypedFloatingDataStoreParameters implements TypedStoreParameters< public class OptionalRepositoryBuilder { public OptionalRepositoryBuilder forRepository(Repository repository) { - TypedFloatingDataStoreParameters.this.repository = repository; + parameters.setRepository(repository); return this; } public DataStore build(){ - return factory.getStore(TypedFloatingDataStoreParameters.this); + return factory.getStore(parameters); } } } diff --git a/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersImpl.java b/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersImpl.java new file mode 100644 index 0000000000..50ce6a496b --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/store/TypedStoreParametersImpl.java @@ -0,0 +1,36 @@ +package sonia.scm.store; + +import sonia.scm.repository.Repository; + +class TypedStoreParametersImpl implements TypedStoreParameters { + private Class type; + private String name; + private Repository repository; + + @Override + public Class getType() { + return type; + } + + void setType(Class type) { + this.type = type; + } + + @Override + public String getName() { + return name; + } + + void setName(String name) { + this.name = name; + } + + @Override + public Repository getRepository() { + return repository; + } + + void setRepository(Repository repository) { + this.repository = repository; + } +} From afae02c8e6486184f3ea51375cbc42ce4b702522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 12:56:39 +0100 Subject: [PATCH 28/38] Add JavaDoc --- .../sonia/scm/store/BlobStoreFactory.java | 40 +++++++++++++- .../store/ConfigurationEntryStoreFactory.java | 55 +++++++++++++++++-- .../scm/store/ConfigurationStoreFactory.java | 55 +++++++++++++++++-- .../sonia/scm/store/DataStoreFactory.java | 49 ++++++++++++++++- 4 files changed, 185 insertions(+), 14 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java index e86f62a136..cf58fc43c7 100644 --- a/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/BlobStoreFactory.java @@ -35,8 +35,22 @@ package sonia.scm.store; import sonia.scm.repository.Repository; /** - * The BlobStoreFactory can be used to create new or get existing - * {@link BlobStore}s. + * The BlobStoreFactory can be used to create a new or get an existing {@link BlobStore}s. + *
+ * You can either create a global {@link BlobStore} or a {@link BlobStore} for a specific repository. To create a global + * {@link BlobStore} call: + * 
+ *     blobStoreFactory
+ *       .withName("name")
+ *       .build();
+ *
+ * To create a {@link BlobStore} for a specific repository call: + * 
+ *     blobStoreFactory
+ *       .withName("name")
+ *       .forRepository(repository)
+ *       .build();
+ *
* * @author Sebastian Sdorra * @since 1.23 @@ -46,8 +60,20 @@ import sonia.scm.repository.Repository; */ public interface BlobStoreFactory { + /** + * Creates a new or gets an existing {@link BlobStore}. Instead of calling this method you should use the floating API + * from {@link #withName(String)}. + * + * @param storeParameters The parameters for the blob store. + * @return A new or an existing {@link BlobStore} for the given parameters. + */ BlobStore getStore(final StoreParameters storeParameters); + /** + * Use this to create a new or get an existing {@link BlobStore} with a floating API. + * @param name The name for the {@link BlobStore}. + * @return Floating API to either specify a repository or directly build a global {@link BlobStore}. + */ default FloatingStoreParameters.Builder withName(String name) { return new FloatingStoreParameters(this).new Builder(name); } @@ -80,11 +106,21 @@ final class FloatingStoreParameters implements StoreParameters { FloatingStoreParameters.this.name = name; } + /** + * Use this to create or get a {@link BlobStore} for a specific repository. This step is optional. If you want to + * have a global {@link BlobStore}, omit this. + * @param repository The optional repository for the {@link BlobStore}. + * @return Floating API to finish the call. + */ public FloatingStoreParameters.Builder forRepository(Repository repository) { FloatingStoreParameters.this.repository = repository; return this; } + /** + * Creates or gets the {@link BlobStore} with the given name and (if specified) the given repository. If no + * repository is given, the {@link BlobStore} will be global. + */ public BlobStore build(){ return factory.getStore(FloatingStoreParameters.this); } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index b77eb2c0f1..80f9cb3df9 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -35,12 +35,28 @@ package sonia.scm.store; import sonia.scm.repository.Repository; /** - * The ConfigurationEntryStoreFactory can be used to create new or get existing - * {@link ConfigurationEntryStore}s. + * The ConfigurationEntryStoreFactory can be used to create new or get existing {@link ConfigurationEntryStore}s. + *
+ * Note: the default implementation uses the same location as the {@link ConfigurationStoreFactory}, so be sure + * that the store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationEntryStore}s. + *
+ * You can either create a global {@link ConfigurationEntryStore} or a {@link ConfigurationEntryStore} for a specific + * repository. To create a global {@link ConfigurationEntryStore} call: + * 
+ *     configurationEntryStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .build();
+ *
+ * To create a {@link ConfigurationEntryStore} for a specific repository call: + * 
+ *     configurationEntryStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .forRepository(repository)
+ *       .build();
+ *
* - * Note: the default implementation uses the same location as the {@link ConfigurationStoreFactory}, so be sure that the - * store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationStore}s. - * * @author Sebastian Sdorra * @since 1.31 * @@ -48,8 +64,22 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.ConfigurationEntryStore */ public interface ConfigurationEntryStoreFactory { + + /** + * Creates a new or gets an existing {@link ConfigurationEntryStore}. Instead of calling this method you should use + * the floating API from {@link #withType(Class)}. + * + * @param storeParameters The parameters for the {@link ConfigurationEntryStore}. + * @return A new or an existing {@link ConfigurationEntryStore} for the given parameters. + */ ConfigurationEntryStore getStore(final TypedStoreParameters storeParameters); + /** + * Use this to create a new or get an existing {@link ConfigurationEntryStore} with a floating API. + * @param type The type for the {@link ConfigurationEntryStore}. + * @return Floating API to set the name and either specify a repository or directly build a global + * {@link ConfigurationEntryStore}. + */ default TypedFloatingConfigurationEntryStoreParameters.Builder withType(Class type) { return new TypedFloatingConfigurationEntryStoreParameters(this).new Builder(type); } @@ -70,6 +100,11 @@ final class TypedFloatingConfigurationEntryStoreParameters { parameters.setType(type); } + /** + * Use this to set the name for the {@link ConfigurationEntryStore}. + * @param name The name for the {@link ConfigurationEntryStore}. + * @return Floating API to either specify a repository or directly build a global {@link ConfigurationEntryStore}. + */ public OptionalRepositoryBuilder withName(String name) { parameters.setName(name); return new OptionalRepositoryBuilder(); @@ -78,11 +113,21 @@ final class TypedFloatingConfigurationEntryStoreParameters { public class OptionalRepositoryBuilder { + /** + * Use this to create or get a {@link ConfigurationEntryStore} for a specific repository. This step is optional. If + * you want to have a global {@link ConfigurationEntryStore}, omit this. + * @param repository The optional repository for the {@link ConfigurationEntryStore}. + * @return Floating API to finish the call. + */ public OptionalRepositoryBuilder forRepository(Repository repository) { parameters.setRepository(repository); return this; } + /** + * Creates or gets the {@link ConfigurationEntryStore} with the given name and (if specified) the given repository. + * If no repository is given, the {@link ConfigurationEntryStore} will be global. + */ public ConfigurationEntryStore build(){ return factory.getStore(parameters); } diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 55a6baf83d..6624f307e7 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -36,11 +36,27 @@ package sonia.scm.store; import sonia.scm.repository.Repository; /** - * The ConfigurationStoreFactory can be used to create new or get existing - * {@link ConfigurationStore} objects. - * - * Note: the default implementation uses the same location as the {@link ConfigurationEntryStoreFactory}, so be sure that the - * store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationStore}s. + * The ConfigurationStoreFactory can be used to create new or get existing {@link ConfigurationStore} objects. + *
+ * Note: the default implementation uses the same location as the {@link ConfigurationEntryStoreFactory}, so be + * sure that the store names are unique for all {@link ConfigurationEntryStore}s and {@link ConfigurationStore}s. + *
+ * You can either create a global {@link ConfigurationStore} or a {@link ConfigurationStore} for a specific repository. + * To create a global {@link ConfigurationStore} call: + * 
+ *     configurationStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .build();
+ *
+ * To create a {@link ConfigurationStore} for a specific repository call: + * 
+ *     configurationStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .forRepository(repository)
+ *       .build();
+ *
* * @author Sebastian Sdorra * @@ -48,8 +64,22 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.ConfigurationStore */ public interface ConfigurationStoreFactory { + + /** + * Creates a new or gets an existing {@link ConfigurationStore}. Instead of calling this method you should use the + * floating API from {@link #withType(Class)}. + * + * @param storeParameters The parameters for the {@link ConfigurationStore}. + * @return A new or an existing {@link ConfigurationStore} for the given parameters. + */ ConfigurationStore getStore(final TypedStoreParameters storeParameters); + /** + * Use this to create a new or get an existing {@link ConfigurationStore} with a floating API. + * @param type The type for the {@link ConfigurationStore}. + * @return Floating API to set the name and either specify a repository or directly build a global + * {@link ConfigurationStore}. + */ default TypedFloatingConfigurationStoreParameters.Builder withType(Class type) { return new TypedFloatingConfigurationStoreParameters(this).new Builder(type); } @@ -70,6 +100,11 @@ final class TypedFloatingConfigurationStoreParameters { parameters.setType(type); } + /** + * Use this to set the name for the {@link ConfigurationStore}. + * @param name The name for the {@link ConfigurationStore}. + * @return Floating API to either specify a repository or directly build a global {@link ConfigurationStore}. + */ public OptionalRepositoryBuilder withName(String name) { parameters.setName(name); return new OptionalRepositoryBuilder(); @@ -78,11 +113,21 @@ final class TypedFloatingConfigurationStoreParameters { public class OptionalRepositoryBuilder { + /** + * Use this to create or get a {@link ConfigurationStore} for a specific repository. This step is optional. If you + * want to have a global {@link ConfigurationStore}, omit this. + * @param repository The optional repository for the {@link ConfigurationStore}. + * @return Floating API to finish the call. + */ public OptionalRepositoryBuilder forRepository(Repository repository) { parameters.setRepository(repository); return this; } + /** + * Creates or gets the {@link ConfigurationStore} with the given name and (if specified) the given repository. If no + * repository is given, the {@link ConfigurationStore} will be global. + */ public ConfigurationStore build(){ return factory.getStore(parameters); } diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index bb2f0a37ad..564c339d3d 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -35,8 +35,24 @@ package sonia.scm.store; import sonia.scm.repository.Repository; /** - * The DataStoreFactory can be used to create new or get existing - * {@link DataStore}s. + * The DataStoreFactory can be used to create new or get existing {@link DataStore}s. + *
+ * You can either create a global {@link DataStore} or a {@link DataStore} for a specific repository. + * To create a global {@link DataStore} call: + * 
+ *     dataStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .build();
+ *
+ * To create a {@link DataStore} for a specific repository call: + * 
+ *     dataStoreFactory
+ *       .withType(PersistedType.class)
+ *       .withName("name")
+ *       .forRepository(repository)
+ *       .build();
+ *
* * @author Sebastian Sdorra * @since 1.23 @@ -45,8 +61,22 @@ import sonia.scm.repository.Repository; * @apiviz.uses sonia.scm.store.DataStore */ public interface DataStoreFactory { + + /** + * Creates a new or gets an existing {@link DataStore}. Instead of calling this method you should use the + * floating API from {@link #withType(Class)}. + * + * @param storeParameters The parameters for the {@link DataStore}. + * @return A new or an existing {@link DataStore} for the given parameters. + */ DataStore getStore(final TypedStoreParameters storeParameters); + /** + * Use this to create a new or get an existing {@link DataStore} with a floating API. + * @param type The type for the {@link DataStore}. + * @return Floating API to set the name and either specify a repository or directly build a global + * {@link DataStore}. + */ default TypedFloatingDataStoreParameters.Builder withType(Class type) { return new TypedFloatingDataStoreParameters(this).new Builder(type); } @@ -67,6 +97,11 @@ final class TypedFloatingDataStoreParameters { parameters.setType(type); } + /** + * Use this to set the name for the {@link DataStore}. + * @param name The name for the {@link DataStore}. + * @return Floating API to either specify a repository or directly build a global {@link DataStore}. + */ public OptionalRepositoryBuilder withName(String name) { parameters.setName(name); return new OptionalRepositoryBuilder(); @@ -75,11 +110,21 @@ final class TypedFloatingDataStoreParameters { public class OptionalRepositoryBuilder { + /** + * Use this to create or get a {@link DataStore} for a specific repository. This step is optional. If you + * want to have a global {@link DataStore}, omit this. + * @param repository The optional repository for the {@link DataStore}. + * @return Floating API to finish the call. + */ public OptionalRepositoryBuilder forRepository(Repository repository) { parameters.setRepository(repository); return this; } + /** + * Creates or gets the {@link DataStore} with the given name and (if specified) the given repository. If no + * repository is given, the {@link DataStore} will be global. + */ public DataStore build(){ return factory.getStore(parameters); } From ba71621aae83616bb34bf896289aac1096ce7d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 13:01:17 +0100 Subject: [PATCH 29/38] Suppress Sonar issue Found no way to extract these internal classes into external classes without loosing type information, so that the created stores were no longer type safe. --- .../java/sonia/scm/store/ConfigurationEntryStoreFactory.java | 1 + .../src/main/java/sonia/scm/store/ConfigurationStoreFactory.java | 1 + scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java | 1 + 3 files changed, 3 insertions(+) diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index 80f9cb3df9..a47afaa176 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -85,6 +85,7 @@ public interface ConfigurationEntryStoreFactory { } } +@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingConfigurationEntryStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 6624f307e7..568fb1f1de 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -85,6 +85,7 @@ public interface ConfigurationStoreFactory { } } +@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingConfigurationStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index 564c339d3d..8326fa40a4 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -82,6 +82,7 @@ public interface DataStoreFactory { } } +@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingDataStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); From 6806a3bf6f7dd1d298c25c6cc3b69fc2fab863e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 13:23:03 +0100 Subject: [PATCH 30/38] Remove old TODO --- scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java index f109e55119..39ce021715 100644 --- a/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java +++ b/scm-test/src/main/java/sonia/scm/store/DataStoreTestBase.java @@ -62,7 +62,6 @@ public abstract class DataStoreTestBase extends KeyValueStoreTestBase @Test - // TODO public void shouldStoreRepositorySpecificData() { DataStoreFactory dataStoreFactory = createDataStoreFactory(); From 59e90c0204491fa9b47cffeb369be2bcc061a4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 15:52:02 +0100 Subject: [PATCH 31/38] Suppress sonar using maven property --- Jenkinsfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index e317ed77d4..f69069d72c 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -107,7 +107,8 @@ void analyzeWith(Maven mvn) { "-Dsonar.pullrequest.key=${env.CHANGE_ID} " + "-Dsonar.pullrequest.provider=bitbucketcloud " + "-Dsonar.pullrequest.bitbucketcloud.owner=sdorra " + - "-Dsonar.pullrequest.bitbucketcloud.repository=scm-manager " + "-Dsonar.pullrequest.bitbucketcloud.repository=scm-manager " + + "-Dsonar.cpd.exclusions=**/*StoreFactory.java,**/*UserPassword.js " } else { mvnArgs += " -Dsonar.branch.name=${env.BRANCH_NAME} " if (!isMainBranch()) { From 99173992bf768c619ca08aba5f1b9117723b383f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 16:33:55 +0100 Subject: [PATCH 32/38] Suppress sonar using maven property --- scm-core/pom.xml | 4 ++++ .../java/sonia/scm/store/ConfigurationEntryStoreFactory.java | 1 - .../main/java/sonia/scm/store/ConfigurationStoreFactory.java | 1 - scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java | 1 - 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/scm-core/pom.xml b/scm-core/pom.xml index 3c90fec779..c327569a9b 100644 --- a/scm-core/pom.xml +++ b/scm-core/pom.xml @@ -221,5 +221,9 @@ + + + **/*StoreFactory.java + diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java index a47afaa176..80f9cb3df9 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationEntryStoreFactory.java @@ -85,7 +85,6 @@ public interface ConfigurationEntryStoreFactory { } } -@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingConfigurationEntryStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); diff --git a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java index 568fb1f1de..6624f307e7 100644 --- a/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/ConfigurationStoreFactory.java @@ -85,7 +85,6 @@ public interface ConfigurationStoreFactory { } } -@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingConfigurationStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); diff --git a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java index 8326fa40a4..564c339d3d 100644 --- a/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java +++ b/scm-core/src/main/java/sonia/scm/store/DataStoreFactory.java @@ -82,7 +82,6 @@ public interface DataStoreFactory { } } -@SuppressWarnings("common-java:DuplicatedBlocks") final class TypedFloatingDataStoreParameters { private final TypedStoreParametersImpl parameters = new TypedStoreParametersImpl<>(); From f0665663ec79aa2284cdcbfd0ef9f0714a7f3851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 16:45:21 +0100 Subject: [PATCH 33/38] Use property for jjwt version --- scm-webapp/pom.xml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 4503274adb..01f45cb54e 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -73,17 +73,17 @@ io.jsonwebtoken jjwt-impl - 0.10.5 + ${jjwt.version} io.jsonwebtoken jjwt-api - 0.10.5 + ${jjwt.version} io.jsonwebtoken jjwt-jackson - 0.10.5 + ${jjwt.version} @@ -550,6 +550,7 @@ DEVELOPMENT target/scm-it default + 0.10.5 2.53.1 1.0 0.8.17 From 3b32511430f36b4687f5e56e9398ee4296007c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 16:57:45 +0100 Subject: [PATCH 34/38] Move sonar exculsion to parent pom --- pom.xml | 4 ++++ scm-core/pom.xml | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index f8bb7c5727..f0affcb5bb 100644 --- a/pom.xml +++ b/pom.xml @@ -810,6 +810,10 @@ SCM-BSD 1.2.0.Final + + + **/*StoreFactory.java + diff --git a/scm-core/pom.xml b/scm-core/pom.xml index c327569a9b..66859a12ee 100644 --- a/scm-core/pom.xml +++ b/scm-core/pom.xml @@ -222,8 +222,4 @@ - - **/*StoreFactory.java - - From 1e71d02ff9fc567e50d7fe5864251ef3b92c2eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 17:55:27 +0000 Subject: [PATCH 35/38] Close branch feature/store_for_repositories_v2 From 3ce57bea72dc5de0ec57e814ca63888c2cf4cb99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 18:57:43 +0100 Subject: [PATCH 36/38] Suppress accepted sonar issue --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f0affcb5bb..ca69d9b0a0 100644 --- a/pom.xml +++ b/pom.xml @@ -812,7 +812,7 @@ 1.2.0.Final - **/*StoreFactory.java + **/*StoreFactory.java,**/*UserPassword.js From 553d5648361503a4b8db66f686167f80d48d0d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 5 Dec 2018 09:27:31 +0100 Subject: [PATCH 37/38] Document sonar exceptions --- pom.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pom.xml b/pom.xml index ca69d9b0a0..e04a47ef41 100644 --- a/pom.xml +++ b/pom.xml @@ -812,6 +812,9 @@ 1.2.0.Final + + + **/*StoreFactory.java,**/*UserPassword.js From 8aeb6333377e977036e0e1917d18f89fbf307914 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Dec 2018 09:14:16 +0000 Subject: [PATCH 38/38] Close branch feature/jwt_refresh