Merge "Make PatchSetApproval UUID parsing forwards compatible" into stable-3.5

2 files changed, 60 insertions, 9 deletions

diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 5cf3a64f14..8aa29cc64d 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -814,17 +814,25 @@ class ChangeNotesParser {
     Account.Id accountId, realAccountId = null;
     String labelVoteStr;
     String tag = null;
-    int s = line.indexOf(' ');
     int tagStart = line.indexOf(":\"");
-
+    // UUID introduced in https://gerrit-review.googlesource.com/c/gerrit/+/324937
+    // Only parsed for backward compatibility
+    // Footer has the following format in this case:
+    // Copied-Label: <LABEL>=VOTE <Gerrit Account>,<Gerrit Real Account> :"<TAG>"
+    int uuidStart = line.indexOf(", ");
+    // Wired tag that contains uuid delimiter. The uuid is actually not present.
+    if (tagStart != -1 && uuidStart > tagStart) {
+      uuidStart = -1;
+    }
+    int identitiesStart = line.indexOf(' ', uuidStart != -1 ? uuidStart + 2 : 0);
     // The first account is the accountId, and second (if applicable) is the realAccountId.
     try {
-      labelVoteStr = line.substring(0, s);
+      labelVoteStr = line.substring(0, uuidStart != -1 ? uuidStart : identitiesStart);
     } catch (StringIndexOutOfBoundsException ex) {
       throw new ConfigInvalidException(ex.getMessage(), ex);
     }
     String[] identities =
-        line.substring(s + 1, tagStart == -1 ? line.length() : tagStart).split(",");
+        line.substring(identitiesStart + 1, tagStart == -1 ? line.length() : tagStart).split(",");
     PersonIdent ident = RawParseUtils.parsePersonIdent(identities[0]);
     checkFooter(ident != null, FOOTER_COPIED_LABEL, line);
     accountId = parseIdent(ident);
@@ -892,18 +900,27 @@ class ChangeNotesParser {
     //     user.
     Account.Id effectiveAccountId;
     String labelVoteStr;
-    int s = line.indexOf(' ');
-    if (s > 0) {
+    // UUID introduced in https://gerrit-review.googlesource.com/c/gerrit/+/324937
+    // Only parsed for backward compatibility
+    // Footer has the following format in this case: Label: <LABEL>=VOTE, <UUID> <Gerrit Account>
+    int uuidStart = line.indexOf(", ");
+    int reviewerStart = line.indexOf(' ', uuidStart != -1 ? uuidStart + 2 : 0);
+    if (uuidStart != -1) {
+      labelVoteStr = line.substring(0, uuidStart);
+    } else if (reviewerStart != -1) {
+      labelVoteStr = line.substring(0, reviewerStart);
+    } else {
+      labelVoteStr = line;
+    }
+    if (reviewerStart > 0) {
       // Account in the label line (2) becomes the effective ID of the
       // approval. If there is a real user (3) different from the commit user
       // (2), we actually don't store that anywhere in this case; it's more
       // important to record that the real user (3) actually initiated submit.
-      labelVoteStr = line.substring(0, s);
-      PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
+      PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(reviewerStart + 1));
       checkFooter(ident != null, FOOTER_LABEL, line);
       effectiveAccountId = parseIdent(ident);
     } else {
-      labelVoteStr = line;
       effectiveAccountId = committerId;
     }
 
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 256544c1f7..361932e7f7 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -153,6 +153,21 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   }
 
   @Test
+  public void parseApprovalWithUUID() throws Exception {
+    // Introduced by https://gerrit-review.googlesource.com/c/gerrit/+/324937
+    assertParseSucceeds(
+        "Update change\n"
+            + "\n"
+            + "Branch: refs/heads/master\n"
+            + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+            + "Patch-set: 1\n"
+            + "Label: Label1=+1, 577fb248e474018276351785930358ec0450e9f7\n"
+            + "Label: Label1=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 2 <2@gerrit>\n"
+            + "Label: Label1=0, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 2 <2@gerrit>\n"
+            + "Subject: This is a test change\n");
+  }
+
+  @Test
   public void parseCopiedApproval() throws Exception {
     assertParseSucceeds(
         "Update change\n"
@@ -190,6 +205,25 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   }
 
   @Test
+  public void parseCopiedApprovalWithUUID() throws Exception {
+    // Introduced by https://gerrit-review.googlesource.com/c/gerrit/+/324937
+    assertParseSucceeds(
+        "Update change\n"
+            + "\n"
+            + "Branch: refs/heads/master\n"
+            + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+            + "Patch-set: 1\n"
+            + "Copied-Label: Label2=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit>\n"
+            + "Copied-Label: Label1=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit>,Gerrit User 2 <2@gerrit>\n"
+            + "Copied-Label: Label3=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit>,Gerrit User 2 <2@gerrit> :\"tag\"\n"
+            + "Copied-Label: Label4=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit> :\"tag with characters %^#@^( *::!\"\n"
+            + "Copied-Label: Label4=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit> :\"tag with uuid delimiter , \"\n"
+            + "Copied-Label: Label4=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit>,Gerrit User 2 <2@gerrit> :\"tag with characters %^#@^( *::!\"\n"
+            + "Copied-Label: Label4=+1, 577fb248e474018276351785930358ec0450e9f7 Gerrit User 1 <1@gerrit>,Gerrit User 2 <2@gerrit> :\"tag with uuid delimiter , \"\n"
+            + "Subject: This is a test change\n");
+  }
+
+  @Test
   public void parseSubmitRecords() throws Exception {
     assertParseSucceeds(
         "Update change\n"