Auto-reindex: Restore indexIfNeeded for Groups
Introduce a new indexIfNeeded implementation for GroupReindexRunnable.
Change is based on stable-2.15 branch with additional checks for:
[1]Create new group while passive node is down.
[2]Adding to subgroup new user while passive node is down.
After introducing Gerrit 2.16 with NoteDB, the old implementation didn't
support changes in ReviewDb interface and was removed. New ReviewDb
interface didn't contain methods required to get necessary data anymore.
Implementation based on NoteDB compatible interfaces was not presented.
In result auto-reindex feature didn't reindex groups after startup.
Bug: Issue 10289
Change-Id: I4686b810fda014e3277cf253ae6a63720a59ff8a
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java
index 71a0280..53bf774 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java
@@ -14,23 +14,54 @@
package com.ericsson.gerrit.plugins.highavailability.autoreindex;
+import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexGroupHandler;
+import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation;
import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.AbstractIndexRestApiServlet;
+import com.google.common.collect.Streams;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
+import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import java.io.IOException;
import java.sql.Timestamp;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Objects;
import java.util.Optional;
+import java.util.stream.Stream;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Repository;
public class GroupReindexRunnable extends ReindexRunnable<GroupReference> {
+ private static final FluentLogger log = FluentLogger.forEnclosingClass();
+
private final Groups groups;
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsers;
+ private final ForwardedIndexGroupHandler indexer;
@Inject
- public GroupReindexRunnable(IndexTs indexTs, OneOffRequestContext ctx, Groups groups) {
+ public GroupReindexRunnable(
+ ForwardedIndexGroupHandler indexer,
+ IndexTs indexTs,
+ OneOffRequestContext ctx,
+ Groups groups,
+ GitRepositoryManager repoManager,
+ AllUsersName allUsers) {
super(AbstractIndexRestApiServlet.IndexName.GROUP, indexTs, ctx);
this.groups = groups;
+ this.repoManager = repoManager;
+ this.allUsers = allUsers;
+ this.indexer = indexer;
}
@Override
@@ -40,6 +71,54 @@
@Override
protected Optional<Timestamp> indexIfNeeded(ReviewDb db, GroupReference g, Timestamp sinceTs) {
+ try {
+ Optional<InternalGroup> internalGroup = groups.getGroup(g.getUUID());
+ if (internalGroup.isPresent()) {
+ InternalGroup group = internalGroup.get();
+ Timestamp groupCreationTs = group.getCreatedOn();
+
+ Repository allUsersRepo = repoManager.openRepository(allUsers);
+
+ List<AccountGroupByIdAud> subGroupMembersAud =
+ groups.getSubgroupsAudit(allUsersRepo, g.getUUID());
+ Stream<Timestamp> groupIdAudAddedTs =
+ subGroupMembersAud.stream()
+ .map(AccountGroupByIdAud::getAddedOn)
+ .filter(Objects::nonNull);
+ Stream<Timestamp> groupIdAudRemovedTs =
+ subGroupMembersAud.stream()
+ .map(AccountGroupByIdAud::getRemovedOn)
+ .filter(Objects::nonNull);
+
+ List<AccountGroupMemberAudit> groupMembersAud =
+ groups.getMembersAudit(allUsersRepo, g.getUUID());
+ Stream<Timestamp> groupMemberAudAddedTs =
+ groupMembersAud.stream()
+ .map(AccountGroupMemberAudit::getAddedOn)
+ .filter(Objects::nonNull);
+ Stream<Timestamp> groupMemberAudRemovedTs =
+ groupMembersAud.stream()
+ .map(AccountGroupMemberAudit::getRemovedOn)
+ .filter(Objects::nonNull);
+
+ Optional<Timestamp> groupLastTs =
+ Streams.concat(
+ groupIdAudAddedTs,
+ groupIdAudRemovedTs,
+ groupMemberAudAddedTs,
+ groupMemberAudRemovedTs,
+ Stream.of(groupCreationTs))
+ .max(Comparator.naturalOrder());
+
+ if (groupLastTs.isPresent() && groupLastTs.get().after(sinceTs)) {
+ log.atInfo().log("Index {}/{}/{}", g.getUUID(), g.getName(), groupLastTs.get());
+ indexer.index(g.getUUID(), Operation.INDEX, Optional.empty());
+ return groupLastTs;
+ }
+ }
+ } catch (OrmException | IOException | ConfigInvalidException e) {
+ log.atSevere().withCause(e).log("Reindex failed");
+ }
return Optional.empty();
}
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnableTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnableTest.java
new file mode 100644
index 0000000..32cc23a
--- /dev/null
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnableTest.java
@@ -0,0 +1,200 @@
+// Copyright (C) 2021 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.ericsson.gerrit.plugins.highavailability.autoreindex;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexGroupHandler;
+import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.reviewdb.client.Account.Id;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
+import com.google.gerrit.reviewdb.client.AccountGroupById;
+import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
+import com.google.gerrit.reviewdb.client.AccountGroupMember;
+import com.google.gerrit.reviewdb.client.AccountGroupMember.Key;
+import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.group.InternalGroup;
+import com.google.gerrit.server.group.db.Groups;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import java.sql.Timestamp;
+import java.time.LocalDateTime;
+import java.util.Collections;
+import java.util.Optional;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class GroupReindexRunnableTest {
+
+ @Mock private ForwardedIndexGroupHandler indexer;
+ @Mock private IndexTs indexTs;
+ @Mock private OneOffRequestContext ctx;
+ @Mock private Groups groups;
+ @Mock private GitRepositoryManager repoManager;
+ @Mock private AllUsersName allUsers;
+ @Mock private GroupReference groupReference;
+
+ private GroupReindexRunnable groupReindexRunnable;
+ private static UUID uuid;
+
+ @Before
+ public void setUp() throws Exception {
+ groupReindexRunnable =
+ new GroupReindexRunnable(indexer, indexTs, ctx, groups, repoManager, allUsers);
+ uuid = new UUID("123");
+ when(groupReference.getUUID()).thenReturn(uuid);
+ }
+
+ @Test
+ public void groupIsIndexedWhenItIsCreatedAfterLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp afterCurrentTime = new Timestamp(currentTime.getTime() + 1000L);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(afterCurrentTime));
+
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isTrue();
+ assertThat(groupLastTs.get()).isEqualTo(afterCurrentTime);
+ verify(indexer).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ @Test
+ public void groupIsNotIndexedWhenItIsCreatedBeforeLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp beforeCurrentTime = new Timestamp(currentTime.getTime() - 1000L);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(beforeCurrentTime));
+
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isFalse();
+ verify(indexer, never()).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ @Test
+ public void groupIsNotIndexedGroupReferenceNotPresent() {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isFalse();
+ }
+
+ @Test
+ public void groupIsIndexedWhenNewUserAddedAfterLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp afterCurrentTime = new Timestamp(currentTime.getTime() + 1000L);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(currentTime));
+ when(groups.getMembersAudit(any(), any()))
+ .thenReturn(
+ Collections.singletonList(
+ new AccountGroupMemberAudit(
+ new AccountGroupMember(new Key(new Id(1), new AccountGroup.Id(2))),
+ null,
+ afterCurrentTime)));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isTrue();
+ assertThat(groupLastTs.get()).isEqualTo(afterCurrentTime);
+ verify(indexer).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ @Test
+ public void groupIsIndexedWhenUserRemovedAfterLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp afterCurrentTime = new Timestamp(currentTime.getTime() + 1000L);
+ Timestamp beforeCurrentTime = new Timestamp(currentTime.getTime() - 1000L);
+
+ AccountGroupMemberAudit accountGroupMemberAudit =
+ new AccountGroupMemberAudit(
+ new AccountGroupMember(new Key(new Id(1), new AccountGroup.Id(2))),
+ null,
+ beforeCurrentTime);
+ accountGroupMemberAudit.removed(new Id(1), afterCurrentTime);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(currentTime));
+ when(groups.getMembersAudit(any(), any()))
+ .thenReturn(Collections.singletonList(accountGroupMemberAudit));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isTrue();
+ assertThat(groupLastTs.get()).isEqualTo(afterCurrentTime);
+ verify(indexer).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ @Test
+ public void groupIsIndexedWhenItIsSubGroupAddedAfterLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp afterCurrentTime = new Timestamp(currentTime.getTime() + 1000L);
+ Timestamp beforeCurrentTime = new Timestamp(currentTime.getTime() - 1000L);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(beforeCurrentTime));
+ when(groups.getSubgroupsAudit(any(), any()))
+ .thenReturn(
+ Collections.singletonList(
+ new AccountGroupByIdAud(
+ new AccountGroupById(
+ new AccountGroupById.Key(new AccountGroup.Id(1), new UUID("123"))),
+ null,
+ afterCurrentTime)));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isTrue();
+ assertThat(groupLastTs.get()).isEqualTo(afterCurrentTime);
+ verify(indexer).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ @Test
+ public void groupIsIndexedWhenItIsSubGroupRemovedAfterLastGroupReindex() throws Exception {
+ Timestamp currentTime = Timestamp.valueOf(LocalDateTime.now());
+ Timestamp afterCurrentTime = new Timestamp(currentTime.getTime() + 1000L);
+ Timestamp beforeCurrentTime = new Timestamp(currentTime.getTime() - 1000L);
+
+ AccountGroupByIdAud accountGroupByIdAud =
+ new AccountGroupByIdAud(
+ new AccountGroupById(new AccountGroupById.Key(new AccountGroup.Id(1), new UUID("123"))),
+ null,
+ beforeCurrentTime);
+ accountGroupByIdAud.removed(new Id(1), afterCurrentTime);
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(beforeCurrentTime));
+ when(groups.getSubgroupsAudit(any(), any()))
+ .thenReturn(Collections.singletonList(accountGroupByIdAud));
+
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(null, groupReference, currentTime);
+ assertThat(groupLastTs.isPresent()).isTrue();
+ assertThat(groupLastTs.get()).isEqualTo(afterCurrentTime);
+ verify(indexer).index(uuid, Operation.INDEX, Optional.empty());
+ }
+
+ private Optional<InternalGroup> getInternalGroup(Timestamp timestamp) {
+ AccountGroup accountGroup =
+ new AccountGroup(new AccountGroup.NameKey("Test"), new AccountGroup.Id(1), uuid, timestamp);
+ return Optional.ofNullable(
+ InternalGroup.create(
+ accountGroup,
+ ImmutableSet.<Id>builder().build(),
+ ImmutableSet.<UUID>builder().build(),
+ null));
+ }
+}
