OwnersSubmitRequirement: review the 'evaluate' method content
Changes:
* extract ProjectState and PathOwners objects retrieval to dedicated
methods (so that the evaluate method is focused on its purpose)
* group statements by their meaning so that it is easier to follow the
evaluation flow
Bug: Issue 15556
Change-Id: Id0966ac7e679dfe6269e28fa70fc2c3cb00b0597
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index 37f97b8..cfa4064 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -22,7 +22,6 @@
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.LabelType;
@@ -113,32 +112,10 @@
return Optional.empty();
}
- ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
- if (projectState.hasPrologRules()) {
- logger.atInfo().atMostEvery(1, TimeUnit.DAYS).log(
- "Project '%s' has prolog rules enabled. "
- + "It may interfere with the OWNERS submit requirements evaluation.",
- project);
- }
-
- String branch = change.getDest().branch();
- List<Project.NameKey> parents =
- Optional.ofNullable(projectState.getProject().getParent())
- .map(Arrays::asList)
- .orElse(Collections.emptyList());
-
- try (Repository repo = repoManager.openRepository(project)) {
- PathOwners pathOwners =
- new PathOwners(
- accounts,
- repoManager,
- repo,
- parents,
- pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
- getDiff(project, cd.currentPatchSet().commitId()),
- pluginSettings.expandGroups());
-
- Map<String, Set<Id>> fileOwners = pathOwners.getFileOwners();
+ try {
+ ProjectState projectState = getProjectState(project);
+ PathOwners pathOwners = getPathOwners(cd, projectState);
+ Map<String, Set<Account.Id>> fileOwners = pathOwners.getFileOwners();
if (fileOwners.isEmpty()) {
logger.atFinest().log(
"Project '%s': change #%d has no OWNERS submit requirements defined. "
@@ -152,11 +129,13 @@
LabelTypes labelTypes = projectState.getLabelTypes(notes);
LabelDefinition label = resolveLabel(labelTypes, pathOwners.getLabel());
Optional<LabelAndScore> ownersLabel = ownersLabel(labelTypes, label, project);
- Account.Id uploader = notes.getCurrentPatchSet().uploader();
+
Map<Account.Id, List<PatchSetApproval>> approvalsByAccount =
Streams.stream(approvalsUtil.byPatchSet(notes, cd.currentPatchSet().id()))
.collect(Collectors.groupingBy(PatchSetApproval::accountId));
+ Account.Id uploader = notes.getCurrentPatchSet().uploader();
+
Set<String> missingApprovals =
fileOwners.entrySet().stream()
.filter(
@@ -195,6 +174,42 @@
}
}
+ private ProjectState getProjectState(Project.NameKey project) {
+ ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
+ if (projectState.hasPrologRules()) {
+ logger.atInfo().atMostEvery(1, TimeUnit.DAYS).log(
+ "Project '%s' has prolog rules enabled. "
+ + "It may interfere with the OWNERS submit requirements evaluation.",
+ project);
+ }
+ return projectState;
+ }
+
+ private PathOwners getPathOwners(ChangeData cd, ProjectState projectState)
+ throws IOException, DiffNotAvailableException {
+ String branch = cd.change().getDest().branch();
+
+ List<Project.NameKey> parents =
+ Optional.<Project.NameKey>ofNullable(projectState.getProject().getParent())
+ .map(Arrays::asList)
+ .orElse(Collections.emptyList());
+
+ Project.NameKey nameKey = projectState.getNameKey();
+ try (Repository repo = repoManager.openRepository(nameKey)) {
+ PathOwners pathOwners =
+ new PathOwners(
+ accounts,
+ repoManager,
+ repo,
+ parents,
+ pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
+ getDiff(nameKey, cd.currentPatchSet().commitId()),
+ pluginSettings.expandGroups());
+
+ return pathOwners;
+ }
+ }
+
/**
* The idea is to select the label type that is configured for owner to cast the vote. If nothing
* is configured in the OWNERS file then `Code-Review` will be selected.
