Remove ApprovalCache
In the predecessor change, we removed all uses of this cache. Now, we
can also remove all references to this cache since it's no longer used.
It's not needed since it was initially created as a performance
optimisation, but now we persist all votes in NoteDb so we can just read
the votes from there.
Change-Id: I9212c5c4b01b019690aefbdafb58a2c321a0ada3
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 3109ec7..cccbd7b 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -940,12 +940,6 @@
+
If direct updates are made to `All-Users`, this cache should be flushed.
-cache `"approvals"`::
-+
-Cache entries contain approvals for a given patch set. This includes
-approvals granted on this patch set as well as approvals copied from
-earlier patch sets.
-
cache `"adv_bases"`::
+
Used only for push over smart HTTP when branch level access controls
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index b24fdad..2809a14 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -40,7 +40,6 @@
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.ServiceUserClassifierImpl;
import com.google.gerrit.server.account.externalids.ExternalIdCacheModule;
-import com.google.gerrit.server.approval.ApprovalCacheImpl;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.cache.h2.H2CacheModule;
import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
@@ -176,7 +175,6 @@
modules.add(new GroupModule());
modules.add(new NoteDbModule());
modules.add(AccountCacheImpl.module());
- modules.add(ApprovalCacheImpl.module());
modules.add(ConflictsCacheImpl.module());
modules.add(DefaultPreferencesCacheImpl.module());
modules.add(GroupCacheImpl.module());
diff --git a/java/com/google/gerrit/server/approval/ApprovalCache.java b/java/com/google/gerrit/server/approval/ApprovalCache.java
deleted file mode 100644
index 5637249..0000000
--- a/java/com/google/gerrit/server/approval/ApprovalCache.java
+++ /dev/null
@@ -1,28 +0,0 @@
-// 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.google.gerrit.server.approval;
-
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.PatchSetApproval;
-import com.google.gerrit.server.notedb.ChangeNotes;
-
-/**
- * Cache that holds approvals per patch set and NoteDb state. This includes approvals copied forward
- * from older patch sets.
- */
-public interface ApprovalCache {
- /** Returns {@link PatchSetApproval}s for the given patch set. */
- Iterable<PatchSetApproval> get(ChangeNotes notes, PatchSet.Id psId);
-}
diff --git a/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java b/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java
deleted file mode 100644
index fd31da9..0000000
--- a/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java
+++ /dev/null
@@ -1,133 +0,0 @@
-// 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.google.gerrit.server.approval;
-
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.PatchSetApproval;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.proto.Entities;
-import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.cache.proto.Cache;
-import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
-import com.google.gerrit.server.cache.serialize.ProtobufSerializer;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.inject.Inject;
-import com.google.inject.Module;
-import com.google.inject.Singleton;
-import com.google.inject.name.Named;
-import com.google.protobuf.ByteString;
-import java.util.concurrent.ExecutionException;
-
-/** Implementation of the {@link ApprovalCache} interface */
-public class ApprovalCacheImpl implements ApprovalCache {
- private static final String CACHE_NAME = "approvals";
-
- public static Module module() {
- return new CacheModule() {
- @Override
- protected void configure() {
- bind(ApprovalCache.class).to(ApprovalCacheImpl.class);
- persist(
- CACHE_NAME,
- Cache.PatchSetApprovalsKeyProto.class,
- Cache.AllPatchSetApprovalsProto.class)
- .version(2)
- .loader(Loader.class)
- .keySerializer(new ProtobufSerializer<>(Cache.PatchSetApprovalsKeyProto.parser()))
- .valueSerializer(new ProtobufSerializer<>(Cache.AllPatchSetApprovalsProto.parser()));
- }
- };
- }
-
- private final LoadingCache<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto>
- cache;
-
- @Inject
- ApprovalCacheImpl(
- @Named(CACHE_NAME)
- LoadingCache<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto> cache) {
- this.cache = cache;
- }
-
- @Override
- public Iterable<PatchSetApproval> get(ChangeNotes notes, PatchSet.Id psId) {
- try {
- return fromProto(
- cache.get(
- Cache.PatchSetApprovalsKeyProto.newBuilder()
- .setChangeId(notes.getChangeId().get())
- .setPatchSetId(psId.get())
- .setProject(notes.getProjectName().get())
- .setId(
- ByteString.copyFrom(
- ObjectIdCacheSerializer.INSTANCE.serialize(notes.getMetaId())))
- .build()));
- } catch (ExecutionException e) {
- throw new StorageException(e);
- }
- }
-
- @Singleton
- static class Loader
- extends CacheLoader<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto> {
- private final ApprovalInference approvalInference;
- private final ChangeNotes.Factory changeNotesFactory;
-
- @Inject
- Loader(ApprovalInference approvalInference, ChangeNotes.Factory changeNotesFactory) {
- this.approvalInference = approvalInference;
- this.changeNotesFactory = changeNotesFactory;
- }
-
- @Override
- public Cache.AllPatchSetApprovalsProto load(Cache.PatchSetApprovalsKeyProto key)
- throws Exception {
- Change.Id changeId = Change.id(key.getChangeId());
- return toProto(
- approvalInference.forPatchSet(
- changeNotesFactory.createChecked(
- Project.nameKey(key.getProject()),
- changeId,
- ObjectIdCacheSerializer.INSTANCE.deserialize(key.getId().toByteArray())),
- PatchSet.id(changeId, key.getPatchSetId()),
- null
- /* revWalk= */ ,
- null
- /* repoConfig= */ ));
- }
- }
-
- private static Iterable<PatchSetApproval> fromProto(Cache.AllPatchSetApprovalsProto proto) {
- ImmutableList.Builder<PatchSetApproval> builder = ImmutableList.builder();
- for (Entities.PatchSetApproval psa : proto.getApprovalList()) {
- builder.add(PatchSetApprovalProtoConverter.INSTANCE.fromProto(psa));
- }
- return builder.build();
- }
-
- private static Cache.AllPatchSetApprovalsProto toProto(Iterable<PatchSetApproval> autoValue) {
- Cache.AllPatchSetApprovalsProto.Builder builder = Cache.AllPatchSetApprovalsProto.newBuilder();
- for (PatchSetApproval psa : autoValue) {
- builder.addApproval(PatchSetApprovalProtoConverter.INSTANCE.toProto(psa));
- }
- return builder.build();
- }
-}
diff --git a/java/com/google/gerrit/server/approval/ApprovalInference.java b/java/com/google/gerrit/server/approval/ApprovalInference.java
index 5517ab1..86944d1 100644
--- a/java/com/google/gerrit/server/approval/ApprovalInference.java
+++ b/java/com/google/gerrit/server/approval/ApprovalInference.java
@@ -51,7 +51,6 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Collection;
-import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
@@ -102,16 +101,7 @@
* on parents) approvals.
*/
Iterable<PatchSetApproval> forPatchSet(
- ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) {
- PatchSet patchset = notes.getPatchSets().get(psId);
- if (patchset == null) {
- return Collections.emptyList();
- }
- return forPatchSet(notes, patchset, rw, repoConfig);
- }
-
- Iterable<PatchSetApproval> forPatchSet(
- ChangeNotes notes, PatchSet ps, @Nullable RevWalk rw, @Nullable Config repoConfig) {
+ ChangeNotes notes, PatchSet ps, RevWalk rw, Config repoConfig) {
ProjectState project;
try (TraceTimer traceTimer =
TraceContext.newTimer(
@@ -347,11 +337,7 @@
}
private Collection<PatchSetApproval> getForPatchSetWithoutNormalization(
- ChangeNotes notes,
- ProjectState project,
- PatchSet patchSet,
- @Nullable RevWalk rw,
- @Nullable Config repoConfig) {
+ ChangeNotes notes, ProjectState project, PatchSet patchSet, RevWalk rw, Config repoConfig) {
checkState(
project.getNameKey().equals(notes.getProjectName()),
"project must match %s, %s",
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 24882cb..7878b34 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -107,7 +107,6 @@
import com.google.gerrit.server.account.externalids.ExternalIdCacheModule;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
import com.google.gerrit.server.account.externalids.ExternalIdUpsertPreprocessor;
-import com.google.gerrit.server.approval.ApprovalCacheImpl;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.auth.AuthBackend;
import com.google.gerrit.server.auth.UniversalAuthBackend;
@@ -249,7 +248,6 @@
bind(RulesCache.class);
bind(BlameCache.class).to(BlameCacheImpl.class);
install(AccountCacheImpl.module());
- install(ApprovalCacheImpl.module());
install(BatchUpdate.module());
install(ChangeKindCacheImpl.module());
install(ChangeFinder.module());
diff --git a/proto/cache.proto b/proto/cache.proto
index 16e5e95..750bea1 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -718,18 +718,3 @@
ComparisonType comparison_type = 11;
bool negative = 12;
}
-
-// Serialized form of com.google.gerrit.server.approval.ApprovalCacheImpl.Key.
-// Next ID: 5
-message PatchSetApprovalsKeyProto {
- string project = 1;
- int32 change_id = 2;
- int32 patch_set_id = 3;
- bytes id = 4;
-}
-
-// Repeated version of PatchSetApprovalProto
-// Next ID: 2
-message AllPatchSetApprovalsProto {
- repeated devtools.gerritcodereview.PatchSetApproval approval = 1;
-}
