Refactor evaluateAllRequirements to make ImmutableMap.copyOf unneeded

It's better to avoid the map copying and rather build an ImmutableMap
with ImmutableMap.Builder right away.

Doing this simplifies the code a bit and make it more readable.

Bug: Issue 15410
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ia0842f3e814bc60df069c43c55e2005229fc7d3f
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 16645a5..00f6876 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -35,7 +35,6 @@
 import com.google.inject.Module;
 import com.google.inject.Provider;
 import com.google.inject.Scopes;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
@@ -84,16 +83,15 @@
   @Override
   public ImmutableMap<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
       ChangeData cd, boolean includeLegacy) {
-    Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements = getRequirements(cd);
-    Map<SubmitRequirement, SubmitRequirementResult> result = projectConfigRequirements;
-    if (includeLegacy) {
-      Map<SubmitRequirement, SubmitRequirementResult> legacyReqs =
-          SubmitRequirementsAdapter.getLegacyRequirements(cd);
-      result =
-          submitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
-              projectConfigRequirements, legacyReqs, cd.project());
+    ImmutableMap<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements =
+        getRequirements(cd);
+    if (!includeLegacy) {
+      return projectConfigRequirements;
     }
-    return ImmutableMap.copyOf(result);
+    Map<SubmitRequirement, SubmitRequirementResult> legacyReqs =
+        SubmitRequirementsAdapter.getLegacyRequirements(cd);
+    return submitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
+        projectConfigRequirements, legacyReqs, cd.project());
   }
 
   @Override
@@ -147,7 +145,7 @@
    * <p>The behaviour in case of the name match is controlled by {@link
    * SubmitRequirement#allowOverrideInChildProjects} of global {@link SubmitRequirement}.
    */
-  private Map<SubmitRequirement, SubmitRequirementResult> getRequirements(ChangeData cd) {
+  private ImmutableMap<SubmitRequirement, SubmitRequirementResult> getRequirements(ChangeData cd) {
     Map<String, SubmitRequirement> globalRequirements = getGlobalRequirements();
 
     ProjectState state = projectCache.get(cd.project()).orElseThrow(illegalState(cd.project()));
@@ -167,11 +165,12 @@
                         globalSubmitRequirement.allowOverrideInChildProjects()
                             ? projectConfigRequirement
                             : globalSubmitRequirement));
-    Map<SubmitRequirement, SubmitRequirementResult> results = new HashMap<>();
+    ImmutableMap.Builder<SubmitRequirement, SubmitRequirementResult> results =
+        ImmutableMap.builder();
     for (SubmitRequirement requirement : requirements.values()) {
       results.put(requirement, evaluateRequirement(requirement, cd));
     }
-    return results;
+    return results.build();
   }
 
   /**
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
index e34ab1d..65508ae 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.project;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.SubmitRequirement;
 import com.google.gerrit.entities.SubmitRequirementResult;
@@ -95,10 +96,12 @@
    * @return a map that is the result of merging both input maps, while eliminating requirements
    *     with the same name and status.
    */
-  public Map<SubmitRequirement, SubmitRequirementResult> mergeLegacyAndNonLegacyRequirements(
-      Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements,
-      Map<SubmitRequirement, SubmitRequirementResult> legacyRequirements,
-      Project.NameKey project) {
+  public ImmutableMap<SubmitRequirement, SubmitRequirementResult>
+      mergeLegacyAndNonLegacyRequirements(
+          Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements,
+          Map<SubmitRequirement, SubmitRequirementResult> legacyRequirements,
+          Project.NameKey project) {
+    // Cannot use ImmutableMap.Builder here since entries in the map may be overridden.
     Map<SubmitRequirement, SubmitRequirementResult> result = new HashMap<>();
     result.putAll(projectConfigRequirements);
     Map<String, SubmitRequirementResult> requirementsByName =
@@ -116,7 +119,7 @@
       metrics.submitRequirementsMismatchingWithLegacy.increment(project.get(), srName);
       result.put(legacy.getKey(), legacy.getValue());
     }
-    return result;
+    return ImmutableMap.copyOf(result);
   }
 
   /** Returns true if both input results are equal in allowing/disallowing change submission. */