Migrate submit requirements to `stable-3.6`
The following changes were applied:
* code was adjusted to the API changes
* submit requirements feature flag was removed - it is no longer
experimental
* switched to batch-ref-update event handling for keys invalidation
in PathOwnersEntriesCache
Bug: Issue 15556
Change-Id: I4643bc826500d6d2dfa83a1bba7429bd118c4d93
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
index c2ec085..69584ab 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
@@ -17,7 +17,7 @@
import com.google.common.cache.RemovalNotification;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.events.GitBatchRefUpdateListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.CacheRemovalListener;
@@ -38,7 +38,7 @@
protected void configure() {
cache(CACHE_NAME, Key.class, PathOwnersEntry.class);
bind(PathOwnersEntriesCache.class).to(PathOwnersEntriesCacheImpl.class);
- DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
+ DynamicSet.bind(binder(), GitBatchRefUpdateListener.class)
.to(OwnersRefUpdateListener.class);
DynamicSet.bind(binder(), CacheRemovalListener.class).to(OwnersCacheRemovalListener.class);
}
@@ -103,7 +103,7 @@
}
@Singleton
- class OwnersRefUpdateListener implements GitReferenceUpdatedListener {
+ class OwnersRefUpdateListener implements GitBatchRefUpdateListener {
private final PathOwnersEntriesCache cache;
private final String allUsersName;
@@ -114,16 +114,17 @@
}
@Override
- public void onGitReferenceUpdated(Event event) {
- if (supportedEvent(allUsersName, event)) {
- cache.invalidate(event.getProjectName(), event.getRefName());
+ public void onGitBatchRefUpdate(GitBatchRefUpdateListener.Event event) {
+ String projectName = event.getProjectName();
+ if (!allUsersName.equals(projectName)) {
+ event.getUpdatedRefs().stream()
+ .filter(refUpdate -> supportedEvent(refUpdate.getRefName()))
+ .forEach(refUpdate -> cache.invalidate(projectName, refUpdate.getRefName()));
}
}
- static boolean supportedEvent(String allUsersName, Event event) {
- String refName = event.getRefName();
- return !allUsersName.equals(event.getProjectName())
- && (refName.equals(RefNames.REFS_CONFIG) || !RefNames.isGerritRef(refName));
+ private boolean supportedEvent(String refName) {
+ return (refName.equals(RefNames.REFS_CONFIG) || !RefNames.isGerritRef(refName));
}
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersBatchRefUpdateListenerTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersBatchRefUpdateListenerTest.java
new file mode 100644
index 0000000..fe99ad3
--- /dev/null
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersBatchRefUpdateListenerTest.java
@@ -0,0 +1,99 @@
+// Copyright (C) 2023 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.googlesource.gerrit.owners.common;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.events.GitBatchRefUpdateListener;
+import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache.OwnersRefUpdateListener;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class OwnersBatchRefUpdateListenerTest {
+ @Parameterized.Parameters
+ public static Collection<Object[]> events() {
+ return Arrays.asList(
+ new Object[][] {
+ {mockEvent(ALL_USERS_NAME.get(), RefNames.REFS_CONFIG), 0},
+ {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_CHANGES), 0},
+ {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_SEQUENCES), 0},
+ {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_CONFIG), 1},
+ {
+ mockEvent(
+ AllProjectsNameProvider.DEFAULT, RefNames.REFS_CONFIG, RefNames.REFS_SEQUENCES),
+ 1
+ },
+ {mockEvent("foo", RefNames.fullName("bar")), 1},
+ {mockEvent("foo", RefNames.REFS_CONFIG), 1},
+ {mockEvent("foo", RefNames.REFS_CONFIG, RefNames.fullName("bar")), 2}
+ });
+ }
+
+ private static AllUsersName ALL_USERS_NAME = new AllUsersName(AllUsersNameProvider.DEFAULT);
+
+ private final GitBatchRefUpdateListener.Event input;
+ private final int expectedTimes;
+
+ public OwnersBatchRefUpdateListenerTest(
+ GitBatchRefUpdateListener.Event input, int expectedTimes) {
+ this.input = input;
+ this.expectedTimes = expectedTimes;
+ }
+
+ @Test
+ public void shouldTriggerCacheInvalidationAccordingly() {
+ // given
+ PathOwnersEntriesCache cachMock = mock(PathOwnersEntriesCache.class);
+ OwnersRefUpdateListener listener = new OwnersRefUpdateListener(cachMock, ALL_USERS_NAME);
+
+ // when
+ listener.onGitBatchRefUpdate(input);
+
+ // then
+ verify(cachMock, times(expectedTimes)).invalidate(anyString(), anyString());
+ }
+
+ private static GitBatchRefUpdateListener.Event mockEvent(String project, String... refs) {
+ GitBatchRefUpdateListener.Event eventMock = mock(GitBatchRefUpdateListener.Event.class);
+ when(eventMock.getProjectName()).thenReturn(project);
+ Set<GitBatchRefUpdateListener.UpdatedRef> updatedRefs =
+ Stream.of(refs)
+ .map(
+ ref -> {
+ GitBatchRefUpdateListener.UpdatedRef updatedRef =
+ mock(GitBatchRefUpdateListener.UpdatedRef.class);
+ when(updatedRef.getRefName()).thenReturn(ref);
+ return updatedRef;
+ })
+ .collect(Collectors.toSet());
+ when(eventMock.getUpdatedRefs()).thenReturn(updatedRefs);
+ return eventMock;
+ }
+}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersRefUpdateListenerTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersRefUpdateListenerTest.java
deleted file mode 100644
index fa20c79..0000000
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/OwnersRefUpdateListenerTest.java
+++ /dev/null
@@ -1,72 +0,0 @@
-// Copyright (C) 2023 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.googlesource.gerrit.owners.common;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
-import com.google.gerrit.server.config.AllProjectsNameProvider;
-import com.google.gerrit.server.config.AllUsersNameProvider;
-import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache.OwnersRefUpdateListener;
-import java.util.Arrays;
-import java.util.Collection;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class OwnersRefUpdateListenerTest {
- @Parameterized.Parameters
- public static Collection<Object[]> events() {
- return Arrays.asList(
- new Object[][] {
- {mockEvent(ALL_USERS_NAME, null), false},
- {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_CHANGES), false},
- {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_SEQUENCES), false},
- {mockEvent(AllProjectsNameProvider.DEFAULT, RefNames.REFS_CONFIG), true},
- {mockEvent("foo", RefNames.fullName("bar")), true},
- {mockEvent("foo", RefNames.REFS_CONFIG), true}
- });
- }
-
- private static String ALL_USERS_NAME = AllUsersNameProvider.DEFAULT;
-
- private final GitReferenceUpdatedListener.Event input;
- private final boolean expected;
-
- public OwnersRefUpdateListenerTest(GitReferenceUpdatedListener.Event input, boolean expected) {
- this.input = input;
- this.expected = expected;
- }
-
- @Test
- public void shouldParseLabelDefinition() {
- // when
- boolean result = OwnersRefUpdateListener.supportedEvent(ALL_USERS_NAME, input);
-
- // then
- assertThat(result).isEqualTo(expected);
- }
-
- private static GitReferenceUpdatedListener.Event mockEvent(String project, String ref) {
- GitReferenceUpdatedListener.Event eventMock = mock(GitReferenceUpdatedListener.Event.class);
- when(eventMock.getProjectName()).thenReturn(project);
- when(eventMock.getRefName()).thenReturn(ref);
- return eventMock;
- }
-}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index 97d8098..f26ae71 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.DiffOptions;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
@@ -348,7 +349,8 @@
// For merge commits the default base is the auto-merge commit which should be used as base IOW
// only the changes from it should be reviewed as changes against the parent 1 were already
// reviewed
- return diffOperations.listModifiedFilesAgainstParent(project, revision, 0);
+ return diffOperations.listModifiedFilesAgainstParent(
+ project, revision, 0, DiffOptions.DEFAULTS);
}
private static SubmitRecord notReady(String ownersLabel, String missingApprovals) {
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
index 667227c..9a65c45 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
@@ -30,30 +30,14 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
-import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
-import com.google.gerrit.testing.ConfigSuite;
import com.googlesource.gerrit.owners.common.LabelDefinition;
import java.util.Collection;
-import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
public class OwnersApprovalHasOperandIT extends OwnersSubmitRequirementIT {
private static final String REQUIREMENT_NAME = "Owner-Approval";
- // This configuration is needed on 3.5 only and should be removed during/after the merge to
- // stable-3.6 as it is enabled there by default.
- @ConfigSuite.Default
- public static Config defaultConfig() {
- Config cfg = new Config();
- cfg.setString(
- "experiments",
- null,
- "enabled",
- ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS);
- return cfg;
- }
-
@Before
public void setup() throws Exception {
configSubmitRequirement(
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementTest.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementTest.java
index 97113c0..bd6c232 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementTest.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementTest.java
@@ -38,7 +38,6 @@
import com.google.gerrit.entities.Project;
import com.googlesource.gerrit.owners.OwnersSubmitRequirement.LabelAndScore;
import com.googlesource.gerrit.owners.common.LabelDefinition;
-import java.sql.Timestamp;
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
@@ -456,7 +455,7 @@
private static final PatchSetApproval approvedBy(Account.Id approving, String label, int value) {
return PatchSetApproval.builder()
.key(PatchSetApproval.key(mock(PatchSet.Id.class), approving, LabelId.create(label)))
- .granted(Timestamp.from(Instant.now()))
+ .granted(Instant.now())
.realAccountId(approving)
.value(value)
.build();
