diff options
author | Marija Savtchouk <mariasavtchouk@google.com> | 2022-05-11 10:18:25 +0100 |
---|---|---|
committer | Marija Savtchouk <mariasavtchouk@google.com> | 2022-05-11 11:18:25 +0000 |
commit | f0bf177636b1a2aae675c3d81796ff1c9a38ee11 (patch) | |
tree | 4662cb776f44167f20bb2493a4c5095fc5fa916c | |
parent | 3a7525cc3c8138be9225cf2149e7c2cac40eb121 (diff) |
Make PatchSetApproval UUID parsing forwards compatible
The change 324937 is backward incompatible: the data,
written by release stable-3.6+ can not be parsed by prior versions.
This fix skips UUID from the footer, allowing old releases to parse new
data format.
Issue: 15909
Change-Id: I9aeabd2fc34d1d2c616d4a06de368cff72754036
Release-Notes: this change has to be reverted, when merging to stable-3.6 branch.
-rw-r--r-- | java/com/google/gerrit/server/notedb/ChangeNotesParser.java | 35 | ||||
-rw-r--r-- | javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java | 34 |
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" |