Extract the change cache key to a separate class This is a preparation change for reusing the cache key in the next forthcoming cache last update timestamp cache. Change-Id: I3db3b6e7fb6d9e24424cd4e1d9ab228ee8b5cd25
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java new file mode 100644 index 0000000..9a6b0d6 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
@@ -0,0 +1,44 @@ +// Copyright (C) 2022 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.modules.gitrefsfilter; + +import com.google.auto.value.AutoValue; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.Project; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; + +@AutoValue +public abstract class ChangeCacheKey { + /* `repo` and `changeId` need to be part of the cache key because the + Loader requires them in order to fetch the relevant changeNote. */ + public abstract Repository repo(); + + public abstract Change.Id changeId(); + + @Nullable + public abstract ObjectId changeRevision(); + + public abstract Project.NameKey project(); + + static ChangeCacheKey create( + Repository repo, + Change.Id changeId, + @Nullable ObjectId changeRevision, + Project.NameKey project) { + return new AutoValue_ChangeCacheKey(repo, changeId, changeRevision, project); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java index 9233c54..75842e3 100644 --- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java +++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
@@ -44,7 +44,7 @@ public class ForProjectWrapper extends ForProject { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final LoadingCache<OpenChangesCache.Key, Boolean> openChangesCache; + private final LoadingCache<ChangeCacheKey, Boolean> openChangesCache; private final ForProject defaultForProject; private final Project.NameKey project; private final FilterRefsConfig config; @@ -56,7 +56,7 @@ @Inject public ForProjectWrapper( FilterRefsConfig config, - @Named(OPEN_CHANGES_CACHE) LoadingCache<OpenChangesCache.Key, Boolean> openChangesCache, + @Named(OPEN_CHANGES_CACHE) LoadingCache<ChangeCacheKey, Boolean> openChangesCache, @Assisted ForProject defaultForProject, @Assisted Project.NameKey project) { this.openChangesCache = openChangesCache; @@ -121,8 +121,7 @@ private boolean isOpen(Repository repo, Change.Id changeId, @Nullable ObjectId changeRevision) { try { - return openChangesCache.get( - OpenChangesCache.Key.create(repo, changeId, changeRevision, project)); + return openChangesCache.get(ChangeCacheKey.create(repo, changeId, changeRevision, project)); } catch (ExecutionException e) { logger.atWarning().withCause(e).log( "Error getting change '%d' from the cache. Do not hide from the advertised refs",
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java index 12a6d72..8e1c371 100644 --- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java +++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
@@ -13,12 +13,8 @@ // limitations under the License. package com.googlesource.gerrit.modules.gitrefsfilter; -import com.google.auto.value.AutoValue; import com.google.common.cache.CacheLoader; import com.google.common.flogger.FluentLogger; -import com.google.gerrit.common.Nullable; -import com.google.gerrit.entities.Change; -import com.google.gerrit.entities.Project; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.NoSuchChangeException; @@ -26,8 +22,6 @@ import com.google.inject.Module; import com.google.inject.Singleton; import com.google.inject.TypeLiteral; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Repository; public class OpenChangesCache { public static final String OPEN_CHANGES_CACHE = "open_changes"; @@ -36,36 +30,14 @@ return new CacheModule() { @Override protected void configure() { - cache(OPEN_CHANGES_CACHE, Key.class, new TypeLiteral<Boolean>() {}).loader(Loader.class); + cache(OPEN_CHANGES_CACHE, ChangeCacheKey.class, new TypeLiteral<Boolean>() {}) + .loader(Loader.class); } }; } - @AutoValue - public abstract static class Key { - /* Note: `repo` and `changeId` need to be part of the cache key because the - Loader requires them in order to compute the relevant changeNote to extract - the change openness status from. */ - public abstract Repository repo(); - - public abstract Change.Id changeId(); - - @Nullable - public abstract ObjectId changeRevision(); - - public abstract Project.NameKey project(); - - static Key create( - Repository repo, - Change.Id changeId, - @Nullable ObjectId changeRevision, - Project.NameKey project) { - return new AutoValue_OpenChangesCache_Key(repo, changeId, changeRevision, project); - } - } - @Singleton - static class Loader extends CacheLoader<Key, Boolean> { + static class Loader extends CacheLoader<ChangeCacheKey, Boolean> { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final ChangeNotes.Factory changeNotesFactory; @@ -75,7 +47,7 @@ } @Override - public Boolean load(Key key) throws Exception { + public Boolean load(ChangeCacheKey key) throws Exception { try { ChangeNotes changeNotes = changeNotesFactory.createChecked(
diff --git a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java index 857173a..a8bcf02 100644 --- a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java +++ b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
@@ -30,7 +30,7 @@ import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.name.Named; -import com.googlesource.gerrit.modules.gitrefsfilter.OpenChangesCache; +import com.googlesource.gerrit.modules.gitrefsfilter.ChangeCacheKey; import com.googlesource.gerrit.modules.gitrefsfilter.RefsFilterModule; import java.io.IOException; import java.util.ArrayList; @@ -55,7 +55,7 @@ @Inject private RequestScopeOperations requestScopeOperations; @Inject - private @Named(OPEN_CHANGES_CACHE) LoadingCache<OpenChangesCache.Key, Boolean> changeOpenCache; + private @Named(OPEN_CHANGES_CACHE) LoadingCache<ChangeCacheKey, Boolean> changeOpenCache; @Override public Module createModule() { @@ -136,7 +136,7 @@ assertThat(changeOpenCache.asMap().size()).isEqualTo(1); - Map.Entry<OpenChangesCache.Key, Boolean> cacheEntry = + Map.Entry<ChangeCacheKey, Boolean> cacheEntry = new ArrayList<>(changeOpenCache.asMap().entrySet()).get(0); assertThat(cacheEntry.getKey().project()).isEqualTo(project); @@ -156,7 +156,7 @@ assertThat(changeOpenCache.asMap().size()).isEqualTo(1); - Map.Entry<OpenChangesCache.Key, Boolean> cacheEntry = + Map.Entry<ChangeCacheKey, Boolean> cacheEntry = new ArrayList<>(changeOpenCache.asMap().entrySet()).get(0); assertThat(cacheEntry.getKey().project()).isEqualTo(project);