Merge branch 'stable-3.12' * stable-3.12: Fix rollback implementation Change-Id: I60efdbe7726025136decdef792f7543aace16829
diff --git a/bindings.md b/bindings.md index c936c5d..72b9190 100644 --- a/bindings.md +++ b/bindings.md
@@ -34,13 +34,6 @@ factory(BatchRefUpdateValidator.Factory.class); bind(SharedRefDbConfiguration.class).toInstance(cfg.getSharedRefDbConfiguration()); bind(GitRepositoryManager.class).to(SharedRefDbGitRepositoryManager.class); - if (cfg.getSharedRefDbConfiguration().getSharedRefDb().getEnforcementRules().isEmpty()) { - bind(SharedRefEnforcement.class).to(DefaultSharedRefEnforcement.class).in(Scopes.SINGLETON); - } else { - bind(SharedRefEnforcement.class) - .to(CustomSharedRefEnforcementByProject.class) - .in(Scopes.SINGLETON); - } DynamicSet.bind(binder(), ExceptionHook.class).to(SharedRefDbExceptionHook.class); } }
diff --git a/config.md b/config.md index 21064be..312e84a 100644 --- a/config.md +++ b/config.md
@@ -11,28 +11,39 @@ : Enable the use of a global refdb Defaults: true -```ref-database.enforcementRules.<policy>``` -: Level of consistency enforcement across sites on a project:refs basis. - Supports two values for enforcing the policy on multiple projects or refs. - If the project or ref is omitted, apply the policy to all projects or all refs. +```ref-database.storeMutableRefs``` +: Specifies which projects should have mutable refs stored. An asterisk can be + used to match all projects. Excludes draft comments, immutable refs, and + cache-automerge refs. An asterisk can be used to match all projects. - The <policy> can have one of the following values: + Defaults: No rules = All projects store mutable refs. - 1. REQUIRED - Throw an exception if a git ref-update is processed against - a local ref not yet in sync with the global refdb. - The user transaction is cancelled. + Details: An asterisk can be used to match all projects. Storage rules are + evaluated in the following order: project-specific settings (storeNoRefs, then + storeMutableRefs, then storeAllRefs), followed by global settings (using * as + a wildcard) in the same order. Each project can only be in one ref storage + category. - 2. IGNORED - Ignore any validation against the global refdb. +```ref-database.storeAllRefs``` +: Specifies which projects should have all refs stored, including refs which + are excluded by default (draft comments, immutable refs, and cache-automerge + refs). See ```ref-database.storeMutableRefs``` for more details. - *Example:* - ``` - [ref-database "enforcementRules"] - IGNORED = AProject:/refs/heads/feature - ``` + Details: An asterisk can be used to match all projects. Storage rules are + evaluated in the following order: project-specific settings (storeNoRefs, then + storeMutableRefs, then storeAllRefs), followed by global settings (using * as + a wildcard) in the same order. - Ignore the alignment with the global refdb for AProject on refs/heads/feature. +```ref-database.storeNoRefs``` +: Specifies which projects should not be stored in the global-refdb. No refs + from these projects will be stored. An asterisk can be used to match all + projects. If a project is in both storeNoRefs and storeAllRefs, it will not + be stored; the order of processing is storeNoRefs then storeAllRefs. - Defaults: No rules = All projects are REQUIRED to be consistent on all refs. + Details: An asterisk can be used to match all projects. Storage rules are + evaluated in the following order: project-specific settings (storeNoRefs, then + storeMutableRefs, then storeAllRefs), followed by global settings (using * as + a wildcard) in the same order. ```projects.pattern``` : Specifies which projects should be validated against the global refdb.
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java index 4a28687..0e26e1f 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
@@ -14,11 +14,9 @@ package com.gerritforge.gerrit.globalrefdb.validation; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.CustomSharedRefEnforcementByProject; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.OutOfSyncException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; +import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.Policy; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.inject.Inject; @@ -55,9 +53,7 @@ * * @param sharedRefDb an instance of the global refdb to check for out-of-sync refs. * @param validationMetrics to update validation results, such as split-brains. - * @param refEnforcement Specific ref enforcements for this project. Either a {@link - * CustomSharedRefEnforcementByProject} when custom policies are provided via configuration * - * file or a {@link DefaultSharedRefEnforcement} for defaults. + * @param refEnforcement Specific ref enforcements for this project. * @param projectsFilter filter to match whether the project being updated should be validated * against global refdb * @param projectName the name of the project being updated. @@ -94,7 +90,7 @@ * <ul> * <li>The project being updated is a global project ({@link * RefUpdateValidator#isGlobalProject(String)} - * <li>The enforcement policy for the project being updated is {@link EnforcePolicy#IGNORED} + * <li>The enforcement policy for the project being updated is {@link Policy#EXCLUDE} * </ul> * * @param batchRefUpdate batchRefUpdate object @@ -109,8 +105,7 @@ NoParameterVoidFunction batchRefUpdateFunction, OneParameterVoidFunction<List<ReceiveCommand>> batchRefUpdateRollbackFunction) throws IOException { - if (refEnforcement.getPolicy(projectName) == EnforcePolicy.IGNORED - || !isGlobalProject(projectName)) { + if (refEnforcement.getPolicy(projectName) == Policy.EXCLUDE || !isGlobalProject(projectName)) { batchRefUpdateFunction.invoke(); return; } @@ -120,7 +115,7 @@ } catch (IOException e) { logger.atWarning().withCause(e).log( "Failed to execute Batch Update on project %s", projectName); - if (refEnforcement.getPolicy(projectName) == EnforcePolicy.REQUIRED) { + if (refEnforcement.getPolicy(projectName) == Policy.INCLUDE) { throw e; } }
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java index 895a7f4..4009c6e 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -17,12 +17,10 @@ import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError; import com.gerritforge.gerrit.globalrefdb.RefDbLockException; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.CustomSharedRefEnforcementByProject; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.OutOfSyncException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedDbSplitBrainException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; +import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.Policy; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; @@ -89,9 +87,7 @@ * * @param sharedRefDb an instance of the global refdb to check for out-of-sync refs. * @param validationMetrics to update validation results, such as split-brains. - * @param refEnforcement Specific ref enforcements for this project. Either a {@link - * CustomSharedRefEnforcementByProject} when custom policies are provided via configuration - * file or a {@link DefaultSharedRefEnforcement} for defaults. + * @param refEnforcement Whether or not a given ref should be stored * @param projectsFilter filter to match whether the project being updated should be validated * against global refdb * @param projectName the name of the project being updated. @@ -131,14 +127,14 @@ * RefUpdateValidator#isRefToBeIgnored(String)}) * <li>The project being updated is a global project ({@link * RefUpdateValidator#isGlobalProject(String)} - * <li>The enforcement policy for the project being updated is {@link EnforcePolicy#IGNORED} + * <li>The enforcement policy for the project being updated is {@link Policy#EXCLUDE} * </ul> * * @param refUpdate the refUpdate command * @param refUpdateFunction the refUpdate function to execute after validation * @param rollbackFunction function to invoke when the ref-update needs to be rolled back * @return the result of the update, or "null" in case a split brain was detected but the policy - * enforcement was not REQUIRED + * enforcement was not INCLUDE * @throws IOException Execution of ref update failed */ public RefUpdate.Result executeRefUpdate( @@ -148,7 +144,7 @@ throws IOException { if (isRefToBeIgnored(refUpdate.getName()) || !isGlobalProject(projectName) - || refEnforcement.getPolicy(projectName) == EnforcePolicy.IGNORED) { + || refEnforcement.getPolicy(projectName) == Policy.EXCLUDE) { return refUpdateFunction.invoke(); } @@ -162,12 +158,11 @@ return isRefToBeIgnored; } - private <T extends Throwable> void softFailBasedOnEnforcement(T e, EnforcePolicy policy) - throws T { + private <T extends Throwable> void softFailBasedOnEnforcement(T e, Policy policy) throws T { logger.atWarning().withCause(e).log( "Failure while running with policy enforcement %s. Error message: %s", policy, e.getMessage()); - if (policy == EnforcePolicy.REQUIRED) { + if (policy == Policy.INCLUDE) { throw e; } } @@ -215,9 +210,11 @@ protected void updateSharedDbOrThrowExceptionFor(RefUpdateSnapshot refSnapshot) throws IOException { // We are not checking refs that should be ignored - final EnforcePolicy refEnforcementPolicy = + final Policy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refSnapshot.getName()); - if (refEnforcementPolicy == EnforcePolicy.IGNORED) return; + if (refEnforcementPolicy == Policy.EXCLUDE) { + return; + } boolean succeeded; try { @@ -266,8 +263,8 @@ RefUpdateSnapshot refUpdateSnapshot, CloseableSet<AutoCloseable> locks) throws GlobalRefDbLockException, OutOfSyncException, IOException { String refName = refUpdateSnapshot.getName(); - EnforcePolicy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refName); - if (refEnforcementPolicy == EnforcePolicy.IGNORED) { + Policy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refName); + if (refEnforcementPolicy == Policy.EXCLUDE) { return refUpdateSnapshot; }
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java index 4b7f987..c255491 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
@@ -17,13 +17,11 @@ import static com.google.common.base.Suppliers.memoize; import static com.google.common.base.Suppliers.ofInstance; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; +import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration.Projects; +import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration.SharedRefDatabase; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Multimap; -import com.google.common.collect.MultimapBuilder; import java.io.IOException; import java.util.List; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -53,7 +51,17 @@ public SharedRefDbConfiguration(Config config, String pluginName) { Supplier<Config> lazyCfg = lazyLoad(config); projects = memoize(() -> new Projects(lazyCfg)); - sharedRefDb = memoize(() -> new SharedRefDatabase(lazyCfg)); + sharedRefDb = + memoize( + () -> { + try { + return new SharedRefDatabase(lazyCfg); + } catch (ConfigInvalidException e) { + log.error("Invalid configuration for shared refdb", e); + throw new RuntimeException( + "Failed to initialize SharedRefDatabase due to invalid configuration", e); + } + }); this.pluginName = pluginName; } @@ -99,29 +107,32 @@ /** * Represents the global refdb configuration, which is computed by reading the 'ref-database' - * section from the configuration file of this library's consumers. It allows to specify whether - * it is enabled, specific {@link SharedRefEnforcement}s and to tune other parameters that define - * specific behaviours of the global refdb. + * section from the configuration file of this library's consumers. It allows specifying whether + * it is enabled, storage rules, and to tune other parameters that define specific behaviours of + * the global refdb. */ public static class SharedRefDatabase { public static final String SECTION = "ref-database"; public static final String ENABLE_KEY = "enabled"; - public static final String SUBSECTION_ENFORCEMENT_RULES = "enforcementRules"; + public static final String STORE_ALL_REFS_KEY = "storeAllRefs"; + public static final String STORE_MUTABLE_REFS_KEY = "storeMutableRefs"; + public static final String STORE_NO_REFS_KEY = "storeNoRefs"; public static final String IGNORED_REFS_PREFIXES = "ignoredRefsPrefixes"; + public static final String PROJECT = "project"; private final boolean enabled; - private final Multimap<EnforcePolicy, String> enforcementRules; private final ImmutableSet<String> ignoredRefsPrefixes; + private final ImmutableSet<String> storeAllRefs; + private final ImmutableSet<String> storeMutableRefs; + private final ImmutableSet<String> storeNoRefs; - private SharedRefDatabase(Supplier<Config> cfg) { + private SharedRefDatabase(Supplier<Config> cfg) throws ConfigInvalidException { enabled = getBoolean(cfg, SECTION, null, ENABLE_KEY, false); - enforcementRules = MultimapBuilder.hashKeys().arrayListValues().build(); - for (EnforcePolicy policy : EnforcePolicy.values()) { - enforcementRules.putAll( - policy, getList(cfg, SECTION, SUBSECTION_ENFORCEMENT_RULES, policy.name())); - } - ignoredRefsPrefixes = ImmutableSet.copyOf(getList(cfg, SECTION, null, IGNORED_REFS_PREFIXES)); + storeAllRefs = getSet(cfg, SECTION, STORE_ALL_REFS_KEY, PROJECT); + storeMutableRefs = getSet(cfg, SECTION, STORE_MUTABLE_REFS_KEY, PROJECT); + storeNoRefs = getSet(cfg, SECTION, STORE_NO_REFS_KEY, PROJECT); + validateNoRefStorageOverlap(storeAllRefs, storeMutableRefs, storeNoRefs); } /** @@ -134,25 +145,30 @@ } /** - * Getter for the map of {@link EnforcePolicy} to a specific "project:refs". Each entry can be - * either be {@link SharedRefEnforcement.EnforcePolicy#IGNORED} or {@link - * SharedRefEnforcement.EnforcePolicy#REQUIRED} and it represents the level of consistency - * enforcements for that specific "project:refs". If the project or ref is omitted, apply the - * policy to all projects or all refs. + * Returns the set of projects to store all refs for in the global-refdb * - * <p>The projec/ref will not be validated against the global refdb if it one to be ignored by - * default ({@link SharedRefEnforcement#isRefToBeIgnoredBySharedRefDb(String)} or if it has been - * configured so, for example: - * - * <pre> - * [ref-database "enforcementRules"] - * IGNORED = AProject:/refs/heads/feature - * </pre> - * - * @return Map of "project:refs" policies + * @return set of projects to store all refs for */ - public Multimap<EnforcePolicy, String> getEnforcementRules() { - return enforcementRules; + public ImmutableSet<String> getStoreAllRefs() { + return storeAllRefs; + } + + /** + * Returns the set of projects to store mutable refs for in the global-refdb + * + * @return set of projects to store only mutable refs for + */ + public ImmutableSet<String> getStoreMutableRefs() { + return storeMutableRefs; + } + + /** + * Returns the set of projects to not store in the global-refdb + * + * @return set of projects to not store refs for + */ + public ImmutableSet<String> getStoreNoRefs() { + return storeNoRefs; } /** @@ -165,10 +181,57 @@ return ignoredRefsPrefixes; } - private List<String> getList( + private ImmutableList<String> getList( Supplier<Config> cfg, String section, String subsection, String name) { return ImmutableList.copyOf(cfg.get().getStringList(section, subsection, name)); } + + private ImmutableSet<String> getSet( + Supplier<Config> cfg, String section, String subsection, String name) { + return ImmutableSet.copyOf(cfg.get().getStringList(section, subsection, name)); + } + + /** + * Validates that no project is configured in more than one ref storage category. + * + * @throws ConfigInvalidException if any project appears in multiple categories + */ + private void validateNoRefStorageOverlap( + ImmutableSet<String> storeAllRefs, + ImmutableSet<String> storeMutableRefs, + ImmutableSet<String> storeNoRefs) + throws ConfigInvalidException { + + for (String project : storeAllRefs) { + if (storeMutableRefs.contains(project)) { + throw new ConfigInvalidException( + String.format( + "Project '%s' appears in both storeAllRefs and storeMutableRefs. Each project can" + + " only be in one ref storage category (storeMutableRefs, storeAllRefs, or" + + " storeNoRefs).", + project)); + } + if (storeNoRefs.contains(project)) { + throw new ConfigInvalidException( + String.format( + "Project '%s' appears in both storeAllRefs and storeNoRefs. Each project can only" + + " be in one ref storage category (storeMutableRefs, storeAllRefs, or" + + " storeNoRefs).", + project)); + } + } + + for (String project : storeMutableRefs) { + if (storeNoRefs.contains(project)) { + throw new ConfigInvalidException( + String.format( + "Project '%s' appears in both storeMutableRefs and storeNoRefs. Each project can" + + " only be in one ref storage category (storeMutableRefs, storeAllRefs, or" + + " storeNoRefs).", + project)); + } + } + } } /**
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProject.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProject.java deleted file mode 100644 index 394aa58..0000000 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProject.java +++ /dev/null
@@ -1,141 +0,0 @@ -// Copyright (C) 2020 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; - -import static com.google.common.base.Suppliers.memoize; - -import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; -import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration; -import com.google.common.base.MoreObjects; -import com.google.common.base.Splitter; -import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableMap; -import com.google.inject.Inject; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; - -/** - * Implementation of the {@link SharedRefEnforcement} interface which derives project and - * project/ref enforcement policy from the configuration of the libModule consuming this library - */ -public class CustomSharedRefEnforcementByProject implements SharedRefEnforcement { - private static final String ALL = ".*"; - - private final Supplier<Map<String, Map<String, EnforcePolicy>>> predefEnforcements; - private final Boolean draftCommentEventsEnabled; - - /** - * Constructs a {@code CustomSharedRefEnforcementByProject} with the values specified in the - * configuration of the libModule consuming this library - * - * @param config the libModule configuration - */ - @Inject - public CustomSharedRefEnforcementByProject( - SharedRefDbConfiguration config, - DraftCommentEventsEnabledProvider draftCommentEventsEnabledProvider) { - this.predefEnforcements = memoize(() -> parseDryRunEnforcementsToMap(config)); - this.draftCommentEventsEnabled = draftCommentEventsEnabledProvider.get(); - } - - private static Map<String, Map<String, EnforcePolicy>> parseDryRunEnforcementsToMap( - SharedRefDbConfiguration config) { - Map<String, Map<String, EnforcePolicy>> enforcementMap = new HashMap<>(); - - for (Map.Entry<EnforcePolicy, String> enforcementEntry : - config.getSharedRefDb().getEnforcementRules().entries()) { - parseEnforcementEntry(enforcementMap, enforcementEntry); - } - - return enforcementMap; - } - - private static void parseEnforcementEntry( - Map<String, Map<String, EnforcePolicy>> enforcementMap, - Map.Entry<EnforcePolicy, String> enforcementEntry) { - Iterator<String> projectAndRef = Splitter.on(':').split(enforcementEntry.getValue()).iterator(); - EnforcePolicy enforcementPolicy = enforcementEntry.getKey(); - - if (projectAndRef.hasNext()) { - String projectName = emptyToAll(projectAndRef.next()); - String refName = emptyToAll(projectAndRef.hasNext() ? projectAndRef.next() : ALL); - - Map<String, EnforcePolicy> existingOrDefaultRef = - enforcementMap.getOrDefault(projectName, new HashMap<>()); - - existingOrDefaultRef.put(refName, enforcementPolicy); - - enforcementMap.put(projectName, existingOrDefaultRef); - } - } - - private static String emptyToAll(String value) { - return value.trim().isEmpty() ? ALL : value; - } - - /** - * The enforcement policy for 'refName' in 'projectName' as computed from the libModule's - * configuration file. - * - * <p>By default all projects are REQUIRED to be consistent on all refs. - * - * @param projectName project to be enforced - * @param refName ref name to be enforced - * @return the enforcement policy for this project/ref - */ - @Override - public EnforcePolicy getPolicy(String projectName, String refName) { - if (isRefToBeIgnoredBySharedRefDb(refName)) { - return EnforcePolicy.IGNORED; - } - - return getRefEnforcePolicy(projectName, refName); - } - - private EnforcePolicy getRefEnforcePolicy(String projectName, String refName) { - Map<String, EnforcePolicy> orDefault = - predefEnforcements - .get() - .getOrDefault( - projectName, predefEnforcements.get().getOrDefault(ALL, ImmutableMap.of())); - - return MoreObjects.firstNonNull( - orDefault.getOrDefault(refName, orDefault.get(ALL)), EnforcePolicy.REQUIRED); - } - - /** - * The enforcement policy for 'projectName' as computed from the libModule's configuration file. - * - * <p>By default all projects are REQUIRED to be consistent on all refs. - * - * @param projectName the name of the project to get the policy for - * @return the enforcement policy for the project - */ - @Override - public EnforcePolicy getPolicy(String projectName) { - Map<String, EnforcePolicy> policiesForProject = - predefEnforcements - .get() - .getOrDefault( - projectName, predefEnforcements.get().getOrDefault(ALL, ImmutableMap.of())); - return policiesForProject.getOrDefault(ALL, EnforcePolicy.REQUIRED); - } - - @Override - public Boolean isDraftCommentEventsEnabled() { - return draftCommentEventsEnabled; - } -}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcement.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcement.java deleted file mode 100644 index eabd83b..0000000 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcement.java +++ /dev/null
@@ -1,75 +0,0 @@ -// Copyright (C) 2020 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; - -import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; -import com.google.common.annotations.VisibleForTesting; -import com.google.inject.Inject; - -/** - * Default implementation of {@link SharedRefEnforcement}. This class provides the default - * project/ref enforcement rules when no more specific rules have been configured for the libModule - * consuming this library. - */ -public class DefaultSharedRefEnforcement implements SharedRefEnforcement { - - private final Boolean enableDraftCommentEvents; - - @Inject - public DefaultSharedRefEnforcement( - DraftCommentEventsEnabledProvider draftCommentEventsEnabledProvider) { - this.enableDraftCommentEvents = draftCommentEventsEnabledProvider.get(); - } - - @VisibleForTesting - public DefaultSharedRefEnforcement(boolean enableDraftCommentEvents) { - this.enableDraftCommentEvents = enableDraftCommentEvents; - } - - @VisibleForTesting - public DefaultSharedRefEnforcement() { - this.enableDraftCommentEvents = false; - } - - /** - * Returns {@link EnforcePolicy#IGNORED} for refs to be ignored {@link - * SharedRefEnforcement#isRefToBeIgnoredBySharedRefDb(String)}, {@link EnforcePolicy#REQUIRED} - * otherwise - * - * @param projectName project to be enforced - * @param refName ref name to be enforced - * @return the policy for this project/ref - */ - @Override - public EnforcePolicy getPolicy(String projectName, String refName) { - return isRefToBeIgnoredBySharedRefDb(refName) ? EnforcePolicy.IGNORED : EnforcePolicy.REQUIRED; - } - - /** - * The global refdb validation policy for 'projectName' - * - * @param projectName project to be enforced - * @return always {@link EnforcePolicy#REQUIRED} - */ - @Override - public EnforcePolicy getPolicy(String projectName) { - return EnforcePolicy.REQUIRED; - } - - @Override - public Boolean isDraftCommentEventsEnabled() { - return enableDraftCommentEvents; - } -}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java index 405d33a..7a7501c 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java
@@ -14,46 +14,121 @@ package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; +import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; +import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.RefNames; +import com.google.inject.Inject; /** Type of enforcement to implement between the local and shared RefDb. */ -public interface SharedRefEnforcement { - public enum EnforcePolicy { - IGNORED, - REQUIRED; +public class SharedRefEnforcement { + public enum Policy { + EXCLUDE, + INCLUDE_MUTABLE, + INCLUDE; + } + + private final ImmutableSet<String> storeAllRefs; + private final ImmutableSet<String> storeMutableRefs; + private final ImmutableSet<String> storeNoRefs; + private final Boolean enableDraftCommentEvents; + private final String ALL = "*"; + + @Inject + public SharedRefEnforcement( + SharedRefDbConfiguration config, + DraftCommentEventsEnabledProvider draftCommentEventsEnabledProvider) { + this.storeAllRefs = config.getSharedRefDb().getStoreAllRefs(); + this.storeMutableRefs = config.getSharedRefDb().getStoreMutableRefs(); + this.storeNoRefs = config.getSharedRefDb().getStoreNoRefs(); + this.enableDraftCommentEvents = draftCommentEventsEnabledProvider.get(); + } + + @VisibleForTesting + public SharedRefEnforcement( + ImmutableSet<String> storeAllRefs, + ImmutableSet<String> storeMutableRefs, + ImmutableSet<String> storeNoRefs, + boolean enableDraftCommentEvents) { + this.storeAllRefs = storeAllRefs; + this.storeMutableRefs = storeMutableRefs; + this.storeNoRefs = storeNoRefs; + this.enableDraftCommentEvents = enableDraftCommentEvents; + } + + @VisibleForTesting + public SharedRefEnforcement() { + this.storeAllRefs = ImmutableSet.of(); + this.storeMutableRefs = ImmutableSet.of(); + this.storeNoRefs = ImmutableSet.of(); + this.enableDraftCommentEvents = false; } /** - * Get the enforcement policy for a project/refName. + * The enforcement policy for 'refName' in 'projectName' * * @param projectName project to be enforced * @param refName ref name to be enforced - * @return the {@link EnforcePolicy} value + * @return the enforcement policy for this project/ref */ - public EnforcePolicy getPolicy(String projectName, String refName); + public Policy getPolicy(String projectName, String refName) { + Policy configuredPolicy = getPolicy(projectName); + if (configuredPolicy == Policy.INCLUDE_MUTABLE) { + return isRefToBeIgnoredBySharedRefDb(refName) ? Policy.EXCLUDE : Policy.INCLUDE; + } + return configuredPolicy; + } /** - * Get the enforcement policy for a project + * Returns the configured enforcement policy for a project. First checks the project-specific + * settings, then the global projects setting. Priority order is storeNoRefs over storeMutableRefs + * and storeAllRefs. * - * @param projectName the name of the project - * @return the {@link EnforcePolicy} value - */ - public EnforcePolicy getPolicy(String projectName); - - /** - * Get whether the streaming of draft comments events is enabled. + * <p>If no specific policy is configured, defaults to storing mutable refs. * - * @return true when enabled, false otherwise + * @param projectName the name of the project to get the policy for + * @return the enforcement policy for the project */ - public Boolean isDraftCommentEventsEnabled(); + public Policy getPolicy(String projectName) { + if (storeNoRefs.contains(projectName)) { + return Policy.EXCLUDE; + } + if (storeMutableRefs.contains(projectName)) { + return Policy.INCLUDE_MUTABLE; + } + if (storeAllRefs.contains(projectName)) { + return Policy.INCLUDE; + } + + if (storeNoRefs.contains(ALL)) { + return Policy.EXCLUDE; + } + if (storeMutableRefs.contains(ALL)) { + return Policy.INCLUDE_MUTABLE; + } + if (storeAllRefs.contains(ALL)) { + return Policy.INCLUDE; + } + return Policy.INCLUDE_MUTABLE; + } /** - * Check if a refName should be ignored by global refdb. The Default behaviour is to ignore: + * Returns whether draft comment events are enabled. + * + * @return true if draft comment events are enabled, false otherwise + */ + public Boolean isDraftCommentEventsEnabled() { + return enableDraftCommentEvents; + } + + /** + * Check if a refName should be ignored by global refdb. These rules apply when not storing all + * refs. * * <ul> - * <li>refs/draft-comments :user-specific temporary storage that does need to be seen by other - * users/sites only when their streaming is enabled via - * event.stream-events.enableDraftCommentEvents. + * <li>refs/draft-comments :user-specific temporary storage that does not need to be seen by + * other users/sites * <li>refs/changes/<non-meta>: those refs are immutable * <li>refs/cache-automerge: these refs would be never replicated anyway * </ul> @@ -61,9 +136,9 @@ * @param refName the name of the ref to check * @return true if ref should be ignored; false otherwise */ - default boolean isRefToBeIgnoredBySharedRefDb(String refName) { + boolean isRefToBeIgnoredBySharedRefDb(String refName) { return refName == null - || (refName.startsWith("refs/draft-comments") && !isDraftCommentEventsEnabled()) + || (refName.startsWith("refs/draft-comments") && !enableDraftCommentEvents) || (refName.startsWith("refs/changes") && !refName.endsWith("/meta") && !refName.endsWith(RefNames.ROBOT_COMMENTS_SUFFIX))
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java index bb50f74..86ba772 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
@@ -29,9 +29,9 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError; import com.gerritforge.gerrit.globalrefdb.validation.RefUpdateValidator.OneParameterVoidFunction; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.google.common.collect.ImmutableSet; @@ -108,7 +108,7 @@ BatchRefUpdateValidator batchRefUpdateValidator = getRefValidatorForEnforcement(tmpRefEnforcement); - doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED) + doReturn(SharedRefEnforcement.Policy.INCLUDE) .when(batchRefUpdateValidator.refEnforcement) .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1); @@ -166,9 +166,10 @@ BatchRefUpdateValidator batchRefUpdateValidator = getRefValidatorForEnforcement(tmpRefEnforcement); - doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED) + doReturn(SharedRefEnforcement.Policy.INCLUDE) .when(batchRefUpdateValidator.refEnforcement) .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1); + doReturn(false).when(sharedRefDatabase).isUpToDate(eq(A_TEST_PROJECT_NAME_KEY), any()); doReturn(true).when(sharedRefDatabase).exists(eq(A_TEST_PROJECT_NAME_KEY), any()); @@ -188,9 +189,10 @@ BatchRefUpdateValidator batchRefUpdateValidator = getRefValidatorForEnforcement(tmpRefEnforcement); - doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED) + doReturn(SharedRefEnforcement.Policy.INCLUDE) .when(batchRefUpdateValidator.refEnforcement) .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1); + doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any()); doThrow(TestError.class).when(sharedRefDatabase).compareAndPut(any(), any(), any()); @@ -212,7 +214,7 @@ BatchRefUpdateValidator batchRefUpdateValidator = getRefValidatorForEnforcement(tmpRefEnforcement); - doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED) + doReturn(SharedRefEnforcement.Policy.INCLUDE) .when(batchRefUpdateValidator.refEnforcement) .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1); @@ -257,7 +259,10 @@ } private BatchRefUpdateValidator newDefaultValidator() { - return getRefValidatorForEnforcement(new DefaultSharedRefEnforcement()); + return getRefValidatorForEnforcement( + new SharedRefEnforcement( + new SharedRefDbConfiguration(new Config(), "testplugin"), + new DraftCommentEventsEnabledProvider(new Config()))); } private BatchRefUpdateValidator getRefValidatorForEnforcement(
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java index 5b9a9dd..ea94f19 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java
@@ -21,6 +21,7 @@ import com.google.gerrit.entities.RefNames; import com.google.gerrit.json.OutputFormat; import com.google.gerrit.server.Sequence; +import com.google.gerrit.server.config.LogConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.util.SystemLog; import com.google.gson.Gson; @@ -145,7 +146,9 @@ private Log4jSharedRefLogger newLog4jSharedRefLogger() throws IOException { final Log4jSharedRefLogger log4jSharedRefLogger = - new Log4jSharedRefLogger(new SystemLog(new SitePaths(newPath()), baseConfig), repoManager); + new Log4jSharedRefLogger( + new SystemLog(new SitePaths(newPath()), baseConfig, new LogConfig(baseConfig)), + repoManager); log4jSharedRefLogger.setLogger(logWriterLogger()); return log4jSharedRefLogger; }
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java index 01018f2..496e1d8 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
@@ -25,14 +25,16 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError; import com.gerritforge.gerrit.globalrefdb.RefDbLockException; import com.gerritforge.gerrit.globalrefdb.validation.RefUpdateValidator.OneParameterFunction; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture; +import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Project; import java.io.IOException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdRef; import org.eclipse.jgit.lib.Ref; @@ -47,8 +49,10 @@ @RunWith(MockitoJUnitRunner.class) public class RefUpdateValidatorTest implements RefFixture { - private static final DefaultSharedRefEnforcement defaultRefEnforcement = - new DefaultSharedRefEnforcement(); + private static final SharedRefEnforcement defaultRefEnforcement = + new SharedRefEnforcement( + new SharedRefDbConfiguration(new Config(), "testplugin"), + new DraftCommentEventsEnabledProvider(new Config())); @Mock SharedRefDatabaseWrapper sharedRefDb;
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java index 4c6290d..92790ea 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
@@ -27,13 +27,15 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; +import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture; +import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.google.common.collect.ImmutableSet; import java.io.IOException; import java.util.Collections; import java.util.List; import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdRef; import org.eclipse.jgit.lib.ProgressMonitor; @@ -180,7 +182,9 @@ return new BatchRefUpdateValidator( sharedRefDb, validationMetrics, - new DefaultSharedRefEnforcement(), + new SharedRefEnforcement( + new SharedRefDbConfiguration(new Config(), "testplugin"), + new DraftCommentEventsEnabledProvider(new Config())), projectsFilter, projectName, refDb,
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java index 1547bd3..2330df2 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java
@@ -14,8 +14,6 @@ package com.gerritforge.gerrit.globalrefdb.validation; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.google.common.collect.ImmutableSet; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; import com.google.gerrit.acceptance.TestPlugin; @@ -104,7 +102,6 @@ bind(ValidationMetrics.class); bind(SharedRefDbGitRepositoryManager.class); - bind(SharedRefEnforcement.class).to(DefaultSharedRefEnforcement.class).in(Scopes.SINGLETON); } } }
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java deleted file mode 100644 index a21ff3e..0000000 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java +++ /dev/null
@@ -1,153 +0,0 @@ -// Copyright (C) 2020 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; - -import static com.google.common.truth.Truth.assertThat; - -import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; -import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration; -import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration.SharedRefDatabase; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; -import java.util.Arrays; -import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.lib.Ref; -import org.junit.Before; -import org.junit.Test; - -public class CustomSharedRefEnforcementByProjectTest implements RefFixture { - - SharedRefEnforcement refEnforcement; - - @Before - public void setUp() { - Config sharedRefDbConfig = new Config(); - sharedRefDbConfig.setStringList( - SharedRefDatabase.SECTION, - SharedRefDatabase.SUBSECTION_ENFORCEMENT_RULES, - EnforcePolicy.IGNORED.name(), - Arrays.asList( - "ProjectOne", - "ProjectTwo:refs/heads/master/test", - "ProjectTwo:refs/heads/master/test2")); - - refEnforcement = newCustomRefEnforcement(sharedRefDbConfig); - } - - @Test - public void projectOneShouldReturnDesiredForAllRefs() { - Ref aRef = newRef("refs/heads/master/2", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("ProjectOne", aRef.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void projectOneEnforcementShouldAlwaysPrevail() { - Ref aRef = newRef("refs/heads/master/test", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("ProjectOne", aRef.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void aNonListedProjectShouldRequireRefForMasterTest() { - Ref aRef = newRef("refs/heads/master/test", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("NonListedProject", aRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void projectTwoSpecificRefShouldReturnIgnoredPolicy() { - Ref refOne = newRef("refs/heads/master/test", AN_OBJECT_ID_1); - Ref refTwo = newRef("refs/heads/master/test2", AN_OBJECT_ID_1); - - assertThat(refEnforcement.getPolicy("ProjectTwo", refOne.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - assertThat(refEnforcement.getPolicy("ProjectTwo", refTwo.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void aNonListedProjectShouldReturnRequired() { - Ref refOne = newRef("refs/heads/master/newChange", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("NonListedProject", refOne.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void aNonListedRefInProjectShouldReturnRequired() { - Ref refOne = newRef("refs/heads/master/test3", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("ProjectTwo", refOne.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void aNonListedProjectAndRefShouldReturnRequired() { - Ref refOne = newRef("refs/heads/master/test3", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("NonListedProject", refOne.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void getProjectPolicyForProjectOneShouldReturnIgnored() { - assertThat(refEnforcement.getPolicy("ProjectOne")).isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void getProjectPolicyForProjectTwoShouldReturnRequired() { - assertThat(refEnforcement.getPolicy("ProjectTwo")).isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void getProjectPolicyForNonListedProjectShouldReturnRequired() { - assertThat(refEnforcement.getPolicy("NonListedProject")).isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void getProjectPolicyForNonListedProjectWhenSingleProject() { - SharedRefEnforcement customEnforcement = - newCustomRefEnforcementWithValue(EnforcePolicy.IGNORED, ":refs/heads/master"); - - assertThat(customEnforcement.getPolicy("NonListedProject")).isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void getANonListedProjectWhenOnlyOneProjectIsListedShouldReturnRequired() { - SharedRefEnforcement customEnforcement = - newCustomRefEnforcementWithValue(EnforcePolicy.IGNORED, "AProject:"); - assertThat(customEnforcement.getPolicy("NonListedProject", "refs/heads/master")) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - private SharedRefEnforcement newCustomRefEnforcementWithValue( - EnforcePolicy policy, String... projectAndRefs) { - Config sharedRefDbConfiguration = new Config(); - sharedRefDbConfiguration.setStringList( - SharedRefDatabase.SECTION, - SharedRefDatabase.SUBSECTION_ENFORCEMENT_RULES, - policy.name(), - Arrays.asList(projectAndRefs)); - return newCustomRefEnforcement(sharedRefDbConfiguration); - } - - private SharedRefEnforcement newCustomRefEnforcement(Config sharedRefDbConfig) { - return new CustomSharedRefEnforcementByProject( - new SharedRefDbConfiguration(sharedRefDbConfig, "testplugin"), - new DraftCommentEventsEnabledProvider(new Config())); - } - - @Override - public String testBranch() { - return "fooBranch"; - } -}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java deleted file mode 100644 index f26846c..0000000 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java +++ /dev/null
@@ -1,98 +0,0 @@ -// Copyright (C) 2020 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; - -import static com.google.common.truth.Truth.assertThat; - -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; -import com.google.gerrit.entities.RefNames; -import org.eclipse.jgit.lib.Ref; -import org.junit.Test; - -public class DefaultSharedRefEnforcementTest implements RefFixture { - - SharedRefEnforcement refEnforcement = new DefaultSharedRefEnforcement(); - - @Test - public void anImmutableChangeShouldBeIgnored() { - Ref immutableChangeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void aChangeMetaShouldNotBeIgnored() { - Ref immutableChangeRef = newRef("refs/changes/01/1/meta", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void aChangeRobotCommentsShouldNotBeIgnored() { - Ref robotCommentsMutableRef = - newRef("refs/changes/01/1" + RefNames.ROBOT_COMMENTS_SUFFIX, AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, robotCommentsMutableRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void aCacheAutomergeShouldBeIgnored() { - Ref immutableChangeRef = newRef("refs/cache-automerge/01/1/1000000", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void aDraftCommentsShouldBeIgnored() { - Ref immutableChangeRef = newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.IGNORED); - } - - @Test - public void regularRefHeadsMasterShouldNotBeIgnored() { - Ref immutableChangeRef = newRef("refs/heads/master", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void regularCommitShouldNotBeIgnored() { - Ref immutableChangeRef = newRef("refs/heads/stable-2.16", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void allUsersExternalIdsRefShouldBeRequired() { - Ref refOne = newRef("refs/meta/external-ids", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy("All-Users", refOne.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Test - public void draftCommentsShouldBeRequiredWhenDraftCommentEventsEnabled() { - SharedRefEnforcement refEnforcement = new DefaultSharedRefEnforcement(true); - - Ref draftCommentRef = newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1); - assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, draftCommentRef.getName())) - .isEqualTo(EnforcePolicy.REQUIRED); - } - - @Override - public String testBranch() { - return "fooBranch"; - } -}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcementTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcementTest.java new file mode 100644 index 0000000..3f5136b --- /dev/null +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcementTest.java
@@ -0,0 +1,236 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; + +import static com.google.common.truth.Truth.assertThat; + +import com.gerritforge.gerrit.globalrefdb.DraftCommentEventsEnabledProvider; +import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration; +import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration.SharedRefDatabase; +import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.Policy; +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.entities.RefNames; +import java.util.Arrays; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.Ref; +import org.junit.Before; +import org.junit.Test; + +public class SharedRefEnforcementTest implements RefFixture { + + SharedRefEnforcement refEnforcement; + + @Before + public void setUp() { + Config sharedRefDbConfig = new Config(); + refEnforcement = newRefEnforcement(sharedRefDbConfig); + } + + @Test(expected = RuntimeException.class) + public void expectExceptionIfProjectStoredInMultipleConfigurations() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_NO_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_ALL_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + + SharedRefEnforcement refEnforcement = newRefEnforcement(sharedRefDbConfig); + } + + @Test + public void shouldProcessStoreNoRefsBeforeStoreAllRefs() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_NO_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_ALL_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList("*")); + + SharedRefEnforcement refEnforcement = newRefEnforcement(sharedRefDbConfig); + Ref changeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.EXCLUDE); + } + + @Test + public void shouldNotIncludePatchSetRefWhenStoringNoRefs() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_NO_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + + SharedRefEnforcement refEnforcement = newRefEnforcement(sharedRefDbConfig); + Ref changeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.EXCLUDE); + } + + @Test + public void shouldIncludePatchSetRefWhenStoringAllRefs() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_ALL_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList("*")); + + SharedRefEnforcement refEnforcementIncludeAll = newRefEnforcement(sharedRefDbConfig); + + Ref immutableChangeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + Ref draftCommentRef = newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1); + Ref cacheAutomergeRef = newRef("refs/cache-automerge/01/1/1000000", AN_OBJECT_ID_1); + + assertThat( + refEnforcementIncludeAll.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.INCLUDE); + assertThat(refEnforcementIncludeAll.getPolicy(A_TEST_PROJECT_NAME, draftCommentRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.INCLUDE); + assertThat(refEnforcementIncludeAll.getPolicy(A_TEST_PROJECT_NAME, cacheAutomergeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.INCLUDE); + } + + @Test + public void shouldPrioritizeSpecificProjectConfigurationOverUsingWildcard() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_NO_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList("*")); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_ALL_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + + SharedRefEnforcement refEnforcement = newRefEnforcement(sharedRefDbConfig); + Ref changeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.INCLUDE); + } + + @Test + public void shouldExcludePatchSetRefWhenStoringMutableRefs() { + Config sharedRefDbConfig = new Config(); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_MUTABLE_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList(A_TEST_PROJECT_NAME)); + sharedRefDbConfig.setStringList( + SharedRefDatabase.SECTION, + SharedRefDatabase.STORE_ALL_REFS_KEY, + SharedRefDatabase.PROJECT, + Arrays.asList("*")); + + SharedRefEnforcement refEnforcement = newRefEnforcement(sharedRefDbConfig); + Ref changeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(SharedRefEnforcement.Policy.EXCLUDE); + } + + @Test + public void patchSetRefIsExcludedByDefault() { + Ref changeRef = newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(Policy.EXCLUDE); + } + + @Test + public void changeMetaRefIncludedByDefault() { + Ref changeRef = newRef("refs/changes/01/1/meta", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, changeRef.getName())) + .isEqualTo(Policy.INCLUDE); + } + + @Test + public void changeRobotCommentsIncludedByDefault() { + Ref robotCommentsMutableRef = + newRef("refs/changes/01/1" + RefNames.ROBOT_COMMENTS_SUFFIX, AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, robotCommentsMutableRef.getName())) + .isEqualTo(Policy.INCLUDE); + } + + @Test + public void cacheAutomergeExcludedByDefault() { + Ref immutableChangeRef = newRef("refs/cache-automerge/01/1/1000000", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) + .isEqualTo(Policy.EXCLUDE); + } + + @Test + public void draftCommentExcludedByDefault() { + Ref immutableChangeRef = newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) + .isEqualTo(Policy.EXCLUDE); + } + + @Test + public void regularRefHeadsMasterIncludedByDefault() { + Ref immutableChangeRef = newRef("refs/heads/master", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) + .isEqualTo(Policy.INCLUDE); + } + + @Test + public void regularCommitIncludedByDefault() { + Ref immutableChangeRef = newRef("refs/heads/stable-2.16", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName())) + .isEqualTo(Policy.INCLUDE); + } + + @Test + public void allUsersExternalIdsRefIncludedByDefault() { + Ref refOne = newRef("refs/meta/external-ids", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy("All-Users", refOne.getName())).isEqualTo(Policy.INCLUDE); + } + + @Test + public void draftCommentsIncludedWhenDraftCommentEventsEnabled() { + SharedRefEnforcement refEnforcement = + new SharedRefEnforcement(ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), true); + Ref draftCommentRef = newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1); + assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, draftCommentRef.getName())) + .isEqualTo(Policy.INCLUDE); + } + + private SharedRefEnforcement newRefEnforcement(Config sharedRefDbConfig) { + return new SharedRefEnforcement( + new SharedRefDbConfiguration(sharedRefDbConfig, "testplugin"), + new DraftCommentEventsEnabledProvider(sharedRefDbConfig)); + } + + @Override + public String testBranch() { + return "fooBranch"; + } +}