Merge branch 'stable-3.3' into master
* stable-3.3:
Rename GsonParser to CacheKeyJsonParser
Fix deserialization of the projects cache keys and eviction
Auto-reindex: Restore indexIfNeeded for Groups
Adapt GroupReindexRunnable{Test} to master's InternalGroup package move.
Change-Id: I2301c28dfa1583e0944f904b795edddc6be3bf1b
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 1d31ebc..42698c9 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,22 +14,52 @@
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.entities.AccountGroupByIdAudit;
+import com.google.gerrit.entities.AccountGroupMemberAudit;
import com.google.gerrit.entities.GroupReference;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.util.OneOffRequestContext;
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
@@ -39,6 +69,54 @@
@Override
protected Optional<Timestamp> indexIfNeeded(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<AccountGroupByIdAudit> subGroupMembersAud =
+ groups.getSubgroupsAudit(allUsersRepo, g.getUUID());
+ Stream<Timestamp> groupIdAudAddedTs =
+ subGroupMembersAud.stream()
+ .map(AccountGroupByIdAudit::addedOn)
+ .filter(Objects::nonNull);
+ Stream<Timestamp> groupIdAudRemovedTs =
+ subGroupMembersAud.stream()
+ .map(AccountGroupByIdAudit::removedOn)
+ .filter(Optional<Timestamp>::isPresent)
+ .map(Optional<Timestamp>::get);
+
+ List<AccountGroupMemberAudit> groupMembersAud =
+ groups.getMembersAudit(allUsersRepo, g.getUUID());
+ Stream<Timestamp> groupMemberAudAddedTs =
+ groupMembersAud.stream().map(AccountGroupMemberAudit::addedOn).filter(Objects::nonNull);
+ Stream<Timestamp> groupMemberAudRemovedTs =
+ groupMembersAud.stream()
+ .map(AccountGroupMemberAudit::removedOn)
+ .filter(Optional<Timestamp>::isPresent)
+ .map(Optional<Timestamp>::get);
+
+ 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 (IOException | ConfigInvalidException e) {
+ log.atSevere().withCause(e).log("Reindex failed");
+ }
return Optional.empty();
}
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParser.java
similarity index 90%
rename from src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
rename to src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParser.java
index de02912..01c7dbc 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParser.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.events.EventGson;
import com.google.gson.Gson;
import com.google.gson.JsonElement;
@@ -26,11 +27,11 @@
import com.google.inject.Singleton;
@Singleton
-class GsonParser {
+class CacheKeyJsonParser {
private final Gson gson;
@Inject
- public GsonParser(@EventGson Gson gson) {
+ public CacheKeyJsonParser(@EventGson Gson gson) {
this.gson = gson;
}
@@ -39,6 +40,9 @@
Object key;
// Need to add a case for 'adv_bases'
if (!json.isJsonObject()) {
+ if (Constants.PROJECTS.equals(cacheName)) {
+ return Project.nameKey(json.getAsString());
+ }
return json.getAsString();
}
JsonObject asJsonObject = json.getAsJsonObject();
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
index 60e782f..86d64d2 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
@@ -34,13 +34,14 @@
private static final long serialVersionUID = -1L;
private final ForwardedCacheEvictionHandler forwardedCacheEvictionHandler;
- private final GsonParser gson;
+ private final CacheKeyJsonParser cacheKeyParser;
@Inject
CacheRestApiServlet(
- ForwardedCacheEvictionHandler forwardedCacheEvictionHandler, GsonParser gson) {
+ ForwardedCacheEvictionHandler forwardedCacheEvictionHandler,
+ CacheKeyJsonParser cacheKeyParser) {
this.forwardedCacheEvictionHandler = forwardedCacheEvictionHandler;
- this.gson = gson;
+ this.cacheKeyParser = cacheKeyParser;
}
@Override
@@ -51,7 +52,7 @@
String cacheName = params.get(CACHENAME_INDEX);
String json = req.getReader().readLine();
forwardedCacheEvictionHandler.evict(
- CacheEntry.from(cacheName, gson.fromJson(cacheName, json)));
+ CacheEntry.from(cacheName, cacheKeyParser.fromJson(cacheName, json)));
rsp.setStatus(SC_NO_CONTENT);
} catch (CacheNotFoundException e) {
log.atSevere().log("Failed to process eviction request: %s", e.getMessage());
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..afd6df4
--- /dev/null
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnableTest.java
@@ -0,0 +1,209 @@
+// 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.entities.Account;
+import com.google.gerrit.entities.Account.Id;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.AccountGroup.UUID;
+import com.google.gerrit.entities.AccountGroupByIdAudit;
+import com.google.gerrit.entities.AccountGroupMemberAudit;
+import com.google.gerrit.entities.GroupReference;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+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;
+ 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 = UUID.parse("123");
+ groupReference = GroupReference.create(uuid, "123");
+ }
+
+ @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(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(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(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(
+ AccountGroupMemberAudit.builder()
+ .addedBy(Account.Id.tryParse("1").get())
+ .addedOn(afterCurrentTime)
+ .memberId(Account.Id.tryParse("1").get())
+ .groupId(AccountGroup.Id.parse("1"))
+ .build()));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(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 =
+ AccountGroupMemberAudit.builder()
+ .addedBy(Account.Id.tryParse("1").get())
+ .addedOn(beforeCurrentTime)
+ .memberId(Account.Id.tryParse("1").get())
+ .groupId(AccountGroup.Id.parse("2"))
+ .removed(Account.Id.tryParse("2").get(), afterCurrentTime)
+ .build();
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(currentTime));
+ when(groups.getMembersAudit(any(), any()))
+ .thenReturn(Collections.singletonList(accountGroupMemberAudit));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(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(
+ AccountGroupByIdAudit.builder()
+ .groupId(AccountGroup.Id.parse("1"))
+ .includeUuid(UUID.parse("123"))
+ .addedBy(Account.Id.tryParse("1").get())
+ .addedOn(afterCurrentTime)
+ .build()));
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(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);
+
+ AccountGroupByIdAudit accountGroupByIdAud =
+ AccountGroupByIdAudit.builder()
+ .groupId(AccountGroup.Id.parse("1"))
+ .includeUuid(UUID.parse("123"))
+ .addedBy(Account.Id.tryParse("1").get())
+ .addedOn(beforeCurrentTime)
+ .removed(Account.Id.tryParse("2").get(), afterCurrentTime)
+ .build();
+
+ when(groups.getGroup(uuid)).thenReturn(getInternalGroup(beforeCurrentTime));
+ when(groups.getSubgroupsAudit(any(), any()))
+ .thenReturn(Collections.singletonList(accountGroupByIdAud));
+
+ Optional<Timestamp> groupLastTs =
+ groupReindexRunnable.indexIfNeeded(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) {
+ return Optional.ofNullable(
+ InternalGroup.builder()
+ .setId(AccountGroup.Id.parse("1"))
+ .setNameKey(AccountGroup.nameKey("Test"))
+ .setOwnerGroupUUID(uuid)
+ .setVisibleToAll(true)
+ .setGroupUUID(UUID.parse("12"))
+ .setCreatedOn(timestamp)
+ .setMembers(ImmutableSet.<Id>builder().build())
+ .setSubgroups(ImmutableSet.<UUID>builder().build())
+ .build());
+ }
+}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParserTest.java
similarity index 81%
rename from src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
rename to src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParserTest.java
index 5b76598..3df4e67 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheKeyJsonParserTest.java
@@ -19,14 +19,15 @@
import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.events.EventGsonProvider;
import com.google.gson.Gson;
import org.junit.Test;
-public class GsonParserTest {
+public class CacheKeyJsonParserTest {
private static final Object EMPTY_JSON = "{}";
private final Gson gson = new EventGsonProvider().get();
- private final GsonParser objectUnderTest = new GsonParser(gson);
+ private final CacheKeyJsonParser objectUnderTest = new CacheKeyJsonParser(gson);
@Test
public void accountIDParse() {
@@ -51,10 +52,17 @@
}
@Test
+ public void projectNameKeyParse() {
+ Project.NameKey name = Project.nameKey("foo");
+ String json = gson.toJson(name);
+ assertThat(name).isEqualTo(objectUnderTest.fromJson(Constants.PROJECTS, json));
+ }
+
+ @Test
public void stringParse() {
String key = "key";
String json = gson.toJson(key);
- assertThat(key).isEqualTo(objectUnderTest.fromJson(Constants.PROJECTS, json));
+ assertThat(key).isEqualTo(objectUnderTest.fromJson("string-keyed-cache", json));
}
@Test
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
index 44dd7f0..e4ffd59 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
@@ -46,7 +46,8 @@
@Before
public void setUp() {
servlet =
- new CacheRestApiServlet(forwardedCacheEvictionHandlerMock, new GsonParser(new Gson()));
+ new CacheRestApiServlet(
+ forwardedCacheEvictionHandlerMock, new CacheKeyJsonParser(new Gson()));
}
@Test
