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. */