commit 6eaa89988af4b3b34c9c3391f382ca78c5536236
author Youssef Elghareeb <ghareeb@google.com> Wed Jun 23 14:20:50 2021 +0200
committer Youssef Elghareeb <ghareeb@google.com> Wed Jun 23 14:27:47 2021 +0200
tree baa709616a3e1fe7b59865d58d704bb98161797f
parent 3546b572e569cc0175debdbb2ba577d959e33c2c

Refactor keys transformation in comment context

This change is a no/op. We perform two transformations on comment
context keys before requesting context from the cache: 1) We adjust max
padding: If padding exceeds a threshold we set padding to the max.  2)
We hash the file path (cache keys should not contain plain file paths).

Chaining both transormations in a single statement and keeping a map
from the input keys to the cache key for better readability.

This change is already covered by tests in CommentContextIT.

Motivation: we saw few exceeptions in logs that suggested putting a null
value in the result map, i.e. allContext.get(key) returned null. This
could have been due to a hidden bug while doing the two transformations.
This should never happen because the cache should return a result for
every input key by contract, or throw an exception otherwise.

Change-Id: I6803b31b20392f5eff4c54556fde77dfeb094384

java/com/google/gerrit/server/comment/CommentContextCacheImpl.java

diff --git a/java/com/google/gerrit/server/comment/CommentContextCacheImpl.java b/java/com/google/gerrit/server/comment/CommentContextCacheImpl.java
index e12b538..80b4fee 100644
--- a/java/com/google/gerrit/server/comment/CommentContextCacheImpl.java
+++ b/java/com/google/gerrit/server/comment/CommentContextCacheImpl.java
@@ -99,25 +99,26 @@
       Iterable<CommentContextKey> inputKeys) {
     ImmutableMap.Builder<CommentContextKey, CommentContext> result = ImmutableMap.builder();
 
-    List<CommentContextKey> adjustedKeys =
+    // We do two transformations to the input keys: first we adjust the max context padding, and
+    // second we hash the file path. The transformed keys are used to request context from the
+    // cache. Keeping a map of the original inputKeys to the transformed keys
+    Map<CommentContextKey, CommentContextKey> inputKeysToCacheKeys =
         Streams.stream(inputKeys)
-            .map(CommentContextCacheImpl::adjustMaxContextPadding)
-            .collect(ImmutableList.toImmutableList());
-
-    // Convert the input keys to the same keys but with their file paths hashed
-    Map<CommentContextKey, CommentContextKey> keysToCacheKeys =
-        adjustedKeys.stream()
             .collect(
                 Collectors.toMap(
                     Function.identity(),
-                    k -> k.toBuilder().path(Loader.hashPath(k.path())).build()));
+                    k ->
+                        adjustMaxContextPadding(k)
+                            .toBuilder()
+                            .path(Loader.hashPath(k.path()))
+                            .build()));
 
     try {
       ImmutableMap<CommentContextKey, CommentContext> allContext =
-          contextCache.getAll(keysToCacheKeys.values());
+          contextCache.getAll(inputKeysToCacheKeys.values());
 
       for (CommentContextKey inputKey : inputKeys) {
-        CommentContextKey cacheKey = keysToCacheKeys.get(adjustMaxContextPadding(inputKey));
+        CommentContextKey cacheKey = inputKeysToCacheKeys.get(inputKey);
         result.put(inputKey, allContext.get(cacheKey));
       }
       return result.build();