commit 4774e1175047634939816dbbe1d1d4900136691b
author Youssef Elghareeb <ghareeb@google.com> Mon Sep 27 10:38:39 2021 +0200
committer Youssef Elghareeb <ghareeb@google.com> Mon Sep 27 10:44:57 2021 +0200
tree 7e47e1e14ad0d54e6609539b1b912ce36a7cab07
parent 27e7a31713f0facebca341c1ad70c3be3065f831

ChangeField: Refactor the #getLabels method

Moving around some pieces of code to make it more readable and prepare
for adding the "count=$count" argument to the label predicate.

Change-Id: If128ceb84bda71c2c02842d9152c697b69882d3b

java/com/google/gerrit/server/index/change/ChangeField.java

diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index bfe1ee1..f8c38c8 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -610,31 +610,47 @@
     Set<String> distinctApprovals = new HashSet<>();
     for (PatchSetApproval a : cd.currentApprovals()) {
       if (a.value() != 0 && !a.isLegacySubmit()) {
-        allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
         Optional<LabelType> labelType = cd.getLabelTypes().byLabel(a.labelId());
-        allApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, a.accountId()));
-        if (cd.change().getOwner().equals(a.accountId())) {
-          allApprovals.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
-          allApprovals.addAll(
-              getMaxMinAnyLabels(
-                  a.label(), a.value(), labelType, ChangeQueryBuilder.OWNER_ACCOUNT_ID));
-        }
-        if (!cd.currentPatchSet().uploader().equals(a.accountId())) {
-          allApprovals.add(
-              formatLabel(a.label(), a.value(), ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID));
-          allApprovals.addAll(
-              getMaxMinAnyLabels(
-                  a.label(), a.value(), labelType, ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID));
-        }
+
+        allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
+        allApprovals.addAll(getMagicLabelFormats(a.label(), a.value(), labelType, a.accountId()));
+        allApprovals.addAll(getLabelOwnerFormats(a, cd, labelType));
+        allApprovals.addAll(getLabelNonUploaderFormats(a, cd, labelType));
         distinctApprovals.add(formatLabel(a.label(), a.value()));
-        distinctApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, null));
+        distinctApprovals.addAll(
+            getMagicLabelFormats(a.label(), a.value(), labelType, /* accountId= */ null));
       }
     }
     allApprovals.addAll(distinctApprovals);
     return allApprovals;
   }
 
-  private static List<String> getMaxMinAnyLabels(
+  private static List<String> getLabelOwnerFormats(
+      PatchSetApproval a, ChangeData cd, Optional<LabelType> labelType) {
+    List<String> allFormats = new ArrayList<>();
+    if (cd.change().getOwner().equals(a.accountId())) {
+      allFormats.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
+      allFormats.addAll(
+          getMagicLabelFormats(
+              a.label(), a.value(), labelType, ChangeQueryBuilder.OWNER_ACCOUNT_ID));
+    }
+    return allFormats;
+  }
+
+  private static List<String> getLabelNonUploaderFormats(
+      PatchSetApproval a, ChangeData cd, Optional<LabelType> labelType) {
+    List<String> allFormats = new ArrayList<>();
+    if (!cd.currentPatchSet().uploader().equals(a.accountId())) {
+      allFormats.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID));
+      allFormats.addAll(
+          getMagicLabelFormats(
+              a.label(), a.value(), labelType, ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID));
+    }
+    return allFormats;
+  }
+
+  /** Get magic label formats corresponding to the {MIN, MAX, ANY} label votes. */
+  private static List<String> getMagicLabelFormats(
       String label, short labelVal, Optional<LabelType> labelType, @Nullable Account.Id accountId) {
     List<String> labels = new ArrayList<>();
     if (labelType.isPresent()) {
@@ -733,10 +749,6 @@
         + (accountId != null ? "," + formatAccount(accountId) : "");
   }
 
-  public static String formatLabel(String label, String value) {
-    return formatLabel(label, value, null);
-  }
-
   public static String formatLabel(String label, String value, @Nullable Account.Id accountId) {
     return label.toLowerCase()
         + "="