Enforce draftComments when their streaming is enabled Change I02e17592b introduced the possibility to make draft comment events publishing configurable. As a consequence of this, replication mechanisms such as the replication and pull-replication plugins might replicate draft-comments refs on different Gerrit nodes. To avoid split-brain, they have to be treated as any other ref and thus their global status needs to be enforced at a global refdb level. Account for the `event.stream-events.enableDraftCommentEvents` option when evaluating the default policy enforcement for the `draft-comments` ref-space and ignore them only when their streaming is disabled. Bug: Issue 367623925 Change-Id: I269ae1bc928e8e813740ebbed925b94472dc0063
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/DraftCommentEventsEnabledProvider.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/DraftCommentEventsEnabledProvider.java new file mode 100644 index 0000000..527d105 --- /dev/null +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/DraftCommentEventsEnabledProvider.java
@@ -0,0 +1,35 @@ +// 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; + +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import org.eclipse.jgit.lib.Config; + +/** Provides {@link Boolean} indicating whether the streaming of draft comment events is enabled. */ +@Singleton +public class DraftCommentEventsEnabledProvider { + private final Config config; + + @Inject + public DraftCommentEventsEnabledProvider(@GerritServerConfig Config config) { + this.config = config; + } + + public Boolean get() { + return config.getBoolean("event", "stream-events", "enableDraftCommentEvents", false); + } +}
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 index 84f6a64..394aa58 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProject.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProject.java
@@ -16,6 +16,7 @@ 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; @@ -34,6 +35,7 @@ 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 @@ -42,8 +44,11 @@ * @param config the libModule configuration */ @Inject - public CustomSharedRefEnforcementByProject(SharedRefDbConfiguration config) { + public CustomSharedRefEnforcementByProject( + SharedRefDbConfiguration config, + DraftCommentEventsEnabledProvider draftCommentEventsEnabledProvider) { this.predefEnforcements = memoize(() -> parseDryRunEnforcementsToMap(config)); + this.draftCommentEventsEnabled = draftCommentEventsEnabledProvider.get(); } private static Map<String, Map<String, EnforcePolicy>> parseDryRunEnforcementsToMap( @@ -128,4 +133,9 @@ 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 index ecff62f..eabd83b 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcement.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcement.java
@@ -14,6 +14,10 @@ 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 @@ -21,6 +25,24 @@ */ 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} @@ -45,4 +67,9 @@ 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 4e8b2f7..405d33a 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
@@ -41,11 +41,19 @@ public EnforcePolicy getPolicy(String projectName); /** + * Get whether the streaming of draft comments events is enabled. + * + * @return true when enabled, false otherwise + */ + public Boolean isDraftCommentEventsEnabled(); + + /** * Check if a refName should be ignored by global refdb. The Default behaviour is to ignore: * * <ul> - * <li>refs/draft-comments :user-specific temporary storage that does not need to be seen by - * other users/sites + * <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/changes/<non-meta>: those refs are immutable * <li>refs/cache-automerge: these refs would be never replicated anyway * </ul> @@ -55,7 +63,7 @@ */ default boolean isRefToBeIgnoredBySharedRefDb(String refName) { return refName == null - || refName.startsWith("refs/draft-comments") + || (refName.startsWith("refs/draft-comments") && !isDraftCommentEventsEnabled()) || (refName.startsWith("refs/changes") && !refName.endsWith("/meta") && !refName.endsWith(RefNames.ROBOT_COMMENTS_SUFFIX))
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 index 118d8e8..a21ff3e 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java
@@ -16,6 +16,7 @@ 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; @@ -141,7 +142,8 @@ private SharedRefEnforcement newCustomRefEnforcement(Config sharedRefDbConfig) { return new CustomSharedRefEnforcementByProject( - new SharedRefDbConfiguration(sharedRefDbConfig, "testplugin")); + new SharedRefDbConfiguration(sharedRefDbConfig, "testplugin"), + new DraftCommentEventsEnabledProvider(new Config())); } @Override
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 index d8f5ff7..f26846c 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
@@ -82,6 +82,15 @@ .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";