From 4600b1ed6e3e78bacbe7e6b53acb00aa6fe4d493 Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Tue, 29 Dec 2020 09:00:13 +0100 Subject: [PATCH 1/5] Update git submodules * Update plugins/replication from branch 'stable-3.1' to 0995fe0445f507279c05cb5ee60a9413671be400 - Don't check read permission when authgroup isn't set It's unnecessary to check read permission when authGroup isn't set since the then the user is a RemoteSiteUser that is-an InternalUser that has read access to everything. Change-Id: Ie6985250b0acb50c08fdcae75cc608222b1add35 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index c4df2f1eb9..0995fe0445 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit c4df2f1eb9fe470debf4958e05cf29c2ec314b43 +Subproject commit 0995fe0445f507279c05cb5ee60a9413671be400 From 358fe468439fa36c0944ee758e9aff8f042f2fab Mon Sep 17 00:00:00 2001 From: Luke Alonso Date: Tue, 29 Dec 2020 17:25:46 -0800 Subject: [PATCH 2/5] Fix 'is:submittable' query on multiple submit records When a project has multiple submit rules, which produce multiple submit records, the 'is:submittable' query stops working as the documentation indicates it should. Rather than returning changes that are ready to be submitted, it returns any change where at least one submit record is OK, despite the overall change not being submittable. For example, with the code-owners plugin, which uses a java-based submit rule, 'is:submittable' will return changes that are passing owners checks, but might have CodeReview:-2 or Verified:-1. For projects with a single submit rule, the behavior is exactly the same as before, since we simply check that *any* of the submit records is OK, exactly as before, AND that *none* of them are NOT_READY or RULE_ERROR. Bug: Issue 13884 Change-Id: I4878ce13c6673852916d6891253d5e62b46f3db5 --- .../query/change/ChangeQueryBuilder.java | 10 +- .../api/change/ChangeSubmitRequirementIT.java | 104 +++++++++++++++++- 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 886d0eea09..d7627462fe 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -621,7 +621,15 @@ public class ChangeQueryBuilder extends QueryBuilder result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + @Test + public void submittableQueryRuleNoRecord() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + // Our custom rule isn't providing any submit records. + rule.status(Optional.empty()); + change.index(); + + // is:submittable should return the change, since it was approved and the custom rule is not + // blocking it. + List result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + private List queryIsSubmittable() throws Exception { + return gApi.changes().query("is:submittable project:" + project.get()).get(); + } + + private ChangeApi newChangeApi() throws Exception { + return gApi.changes().id(createChange().getChangeId()); + } + + private void approveChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.approve()); + } + + private void rejectChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.reject()); + } + @Singleton private static class CustomSubmitRule implements SubmitRule { - private final AtomicBoolean block = new AtomicBoolean(true); + private Optional recordStatus = Optional.empty(); public void block(boolean block) { - this.block.set(block); + this.status(block ? Optional.of(SubmitRecord.Status.NOT_READY) : Optional.empty()); + } + + public void status(Optional status) { + this.recordStatus = status; } @Override public Optional evaluate(ChangeData changeData) { - if (block.get()) { + if (this.recordStatus.isPresent()) { SubmitRecord record = new SubmitRecord(); record.labels = new ArrayList<>(); - record.status = SubmitRecord.Status.NOT_READY; + record.status = this.recordStatus.get(); record.requirements = ImmutableList.of(req); return Optional.of(record); } From d032872af8d1e8d9b63e50dfe6359360a53c9f3c Mon Sep 17 00:00:00 2001 From: Luke Alonso Date: Tue, 29 Dec 2020 17:25:46 -0800 Subject: [PATCH 3/5] Fix 'is:submittable' query on multiple submit records When a project has multiple submit rules, which produce multiple submit records, the 'is:submittable' query stops working as the documentation indicates it should. Rather than returning changes that are ready to be submitted, it returns any change where at least one submit record is OK, despite the overall change not being submittable. For example, with the code-owners plugin, which uses a java-based submit rule, 'is:submittable' will return changes that are passing owners checks, but might have CodeReview:-2 or Verified:-1. For projects with a single submit rule, the behavior is exactly the same as before, since we simply check that *any* of the submit records is OK, exactly as before, AND that *none* of them are NOT_READY or RULE_ERROR. Bug: Issue 13884 Change-Id: I4878ce13c6673852916d6891253d5e62b46f3db5 --- .../query/change/ChangeQueryBuilder.java | 10 +- .../api/change/ChangeSubmitRequirementIT.java | 104 +++++++++++++++++- 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index df729cb63c..6f278ab712 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -570,7 +570,15 @@ public class ChangeQueryBuilder extends QueryBuilder result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + @Test + public void submittableQueryRuleNoRecord() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + // Our custom rule isn't providing any submit records. + rule.status(Optional.empty()); + change.index(); + + // is:submittable should return the change, since it was approved and the custom rule is not + // blocking it. + List result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + private List queryIsSubmittable() throws Exception { + return gApi.changes().query("is:submittable project:" + project.get()).get(); + } + + private ChangeApi newChangeApi() throws Exception { + return gApi.changes().id(createChange().getChangeId()); + } + + private void approveChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.approve()); + } + + private void rejectChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.reject()); + } + @Singleton private static class CustomSubmitRule implements SubmitRule { - private final AtomicBoolean block = new AtomicBoolean(true); + private Optional recordStatus = Optional.empty(); public void block(boolean block) { - this.block.set(block); + this.status(block ? Optional.of(SubmitRecord.Status.NOT_READY) : Optional.empty()); + } + + public void status(Optional status) { + this.recordStatus = status; } @Override public Optional evaluate(ChangeData changeData) { - if (block.get()) { + if (this.recordStatus.isPresent()) { SubmitRecord record = new SubmitRecord(); record.labels = new ArrayList<>(); - record.status = SubmitRecord.Status.NOT_READY; + record.status = this.recordStatus.get(); record.requirements = ImmutableList.of(req); return Optional.of(record); } From f70d917c67ef61fbe692d6dce4759523fdb07453 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 21 Dec 2020 15:35:42 +0100 Subject: [PATCH 4/5] ReceiveCommits: Avoid use of secondary index in readChangesForReplace This is a follow-up of Ifc83162780. Avoid using secondary index also in readChangesForReplace() because the project attribute is always available. Change-Id: I9d0cc632d24a631c128715b79f7a620da99f61eb --- .../gerrit/server/git/receive/ReceiveCommits.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 637a5c7940..8d92347918 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -2691,12 +2691,10 @@ class ReceiveCommits { private void readChangesForReplace() { try (TraceTimer traceTimer = newTimer("readChangesForReplace")) { - Collection allNotes = - notesFactory.createUsingIndexLookup( - replaceByChange.values().stream().map(r -> r.ontoChange).collect(toList())); - for (ChangeNotes notes : allNotes) { - replaceByChange.get(notes.getChangeId()).notes = notes; - } + replaceByChange.values().stream() + .map(r -> r.ontoChange) + .map(id -> notesFactory.create(project.getNameKey(), id)) + .forEach(notes -> replaceByChange.get(notes.getChangeId()).notes = notes); } } From 9b0b7b9b1c478b52b919c631d47e668856d1fdd8 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 5 Jan 2021 15:14:17 +0100 Subject: [PATCH 5/5] Polygerrit: Wipe out license headers in minified gr-app.js By default terser keeps all licensing comments as many licenses require you to keep a copy of the license in redistributed source: [1] --comments [filter] Preserve copyright comments in the output. By default this works like Google Closure, keeping JSDoc-style comments that contain "@license" or "@preserve". You can optionally pass one of the following arguments to this flag: - "all" to keep all comments - `false` to omit comments in the output - a valid JS RegExp like `/foo/` or `/^!/` to keep only matching comments. Note that currently not *all* comments can be kept when compression is on, because of dead code removal or cascading statements into sequences. In minified gr-app.js version we don't need to preserve license information and can remove it. This saves ca. 300KB in gr-app.js size. [1] https://github.com/terser/terser Bug: Issue 13870 Change-Id: I2736879d252994200ad2f52e414e224ebe56b65d --- polygerrit-ui/app/rollup.config.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/polygerrit-ui/app/rollup.config.js b/polygerrit-ui/app/rollup.config.js index d83f24fb25..cfc8c015fd 100644 --- a/polygerrit-ui/app/rollup.config.js +++ b/polygerrit-ui/app/rollup.config.js @@ -75,7 +75,13 @@ export default { output: { format: 'iife', compact: true, - plugins: [terser()] + plugins: [ + terser({ + output: { + comments: false + } + }) + ] }, //Context must be set to window to correctly processing global variables context: 'window',