From 436fba1e2c1ca71b36accd2a0f711d9100da7235 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 13 Apr 2018 10:53:42 -0400 Subject: [PATCH 01/48] Elasticsearch BUILD: remove unneeded dependencies Bug: Issue 6094 Change-Id: I97a848ea88e36f3e383662bbcd3ad4c5218fe4e3 --- gerrit-elasticsearch/BUILD | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/gerrit-elasticsearch/BUILD b/gerrit-elasticsearch/BUILD index 957ff1aecf..ae8a576b35 100644 --- a/gerrit-elasticsearch/BUILD +++ b/gerrit-elasticsearch/BUILD @@ -5,24 +5,19 @@ java_library( deps = [ "//gerrit-antlr:query_exception", "//gerrit-extension-api:api", - "//gerrit-reviewdb:client", "//gerrit-reviewdb:server", "//gerrit-server:server", "//lib:gson", "//lib:guava", "//lib:gwtorm", - "//lib:protobuf", "//lib/commons:codec", - "//lib/commons:lang", "//lib/elasticsearch", "//lib/guice", "//lib/guice:guice-assistedinject", "//lib/jest", "//lib/jest:jest-common", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/joda:joda-time", "//lib/log:api", - "//lib/lucene:lucene-analyzers-common", "//lib/lucene:lucene-core", ], ) @@ -35,16 +30,12 @@ java_library( srcs = glob(["src/test/java/**/ElasticTestUtils.java"]), deps = [ ":elasticsearch", - "//gerrit-extension-api:api", "//gerrit-reviewdb:client", "//gerrit-server:server", "//lib:gson", - "//lib:guava", - "//lib:junit", "//lib:truth", "//lib/elasticsearch", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/jgit/org.eclipse.jgit.junit:junit", ], ) @@ -59,7 +50,6 @@ junit_tests( ":elasticsearch", ":elasticsearch_test_utils", "//gerrit-server:query_tests_code", - "//gerrit-server:server", "//gerrit-server:testutil", "//lib/guice", "//lib/jgit/org.eclipse.jgit:jgit", From 878009189f68ab5f7640c4bb6c908e277254e3cf Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 13 Apr 2018 12:45:57 -0400 Subject: [PATCH 02/48] lib/jest for Elasticsearch: refactor dependencies Move the commons-lang3 runtime dependency from jest to jest-common, since the latter library is the one explicitly requiring it, as per [1] and [2] below. Remove the otherwise unnecessary explicit runtime dependencies, as well. For httpcore-niossl, remove it also from httpcomponents thus WORKSPACE, as the hereby removed reference to it was the very last one in Gerrit. [1] https://mvnrepository.com/artifact/io.searchbox/jest-common/2.4.0 [2] https://mvnrepository.com/artifact/io.searchbox/jest/2.4.0 Bug: Issue 6094 Change-Id: Ia0d5e64acb438a91af80b79c20d120e8500a1cb9 --- WORKSPACE | 7 ------- lib/httpcomponents/BUILD | 6 ------ lib/jest/BUILD | 6 +++--- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 7641e8019b..439b10e29f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1002,13 +1002,6 @@ maven_jar( sha1 = "a8c5e3c3bfea5ce23fb647c335897e415eb442e3", ) -maven_jar( - name = "httpcore_niossl", - artifact = "org.apache.httpcomponents:httpcore-niossl:4.0-alpha6", - attach_source = False, - sha1 = "9c662e7247ca8ceb1de5de629f685c9ef3e4ab58", -) - load("//tools/bzl:js.bzl", "npm_binary", "bower_archive") npm_binary( diff --git a/lib/httpcomponents/BUILD b/lib/httpcomponents/BUILD index 2b2cc6f804..6e8fcd81a8 100644 --- a/lib/httpcomponents/BUILD +++ b/lib/httpcomponents/BUILD @@ -45,9 +45,3 @@ java_library( data = ["//lib:LICENSE-Apache2.0"], exports = ["@httpcore_nio//jar"], ) - -java_library( - name = "httpcore-niossl", - data = ["//lib:LICENSE-Apache2.0"], - exports = ["@httpcore_niossl//jar"], -) diff --git a/lib/jest/BUILD b/lib/jest/BUILD index 3fdb5f72e6..169f27157d 100644 --- a/lib/jest/BUILD +++ b/lib/jest/BUILD @@ -5,6 +5,9 @@ java_library( data = ["//lib:LICENSE-Apache2.0"], visibility = ["//visibility:public"], exports = ["@jest_common//jar"], + runtime_deps = [ + "//lib/commons:lang3", + ], ) java_library( @@ -13,11 +16,8 @@ java_library( visibility = ["//visibility:public"], exports = ["@jest//jar"], runtime_deps = [ - ":jest-common", - "//lib/commons:lang3", "//lib/httpcomponents:httpasyncclient", "//lib/httpcomponents:httpclient", "//lib/httpcomponents:httpcore-nio", - "//lib/httpcomponents:httpcore-niossl", ], ) From 40744d99ff2d237cb43653bb4efa740009cb8b1d Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 13 Apr 2018 14:56:58 -0400 Subject: [PATCH 03/48] lib/elasticsearch: remove unnecessary dependencies Remove the hereby no longer used ones also from libs thus WORKSPACE. Bug: Issue 6094 Change-Id: Icb0ee8dd89ae30c1256ae60481eb782798f252ef --- WORKSPACE | 24 ------------------------ lib/elasticsearch/BUILD | 11 ----------- lib/jackson/BUILD | 6 ------ lib/lucene/BUILD | 13 ------------- 4 files changed, 54 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 439b10e29f..7a3284bdfb 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -458,12 +458,6 @@ maven_jar( sha1 = "f0bc3114a6b43f8e64a33c471d5b9e8ddc51564d", ) -maven_jar( - name = "lucene_codecs", - artifact = "org.apache.lucene:lucene-codecs:" + LUCENE_VERS, - sha1 = "e01fe463d9490bb1b4a6a168e771f7b7255a50b1", -) - maven_jar( name = "backward_codecs", artifact = "org.apache.lucene:lucene-backward-codecs:" + LUCENE_VERS, @@ -500,12 +494,6 @@ maven_jar( sha1 = "7409db9863d8fbc265c27793c6cc7511304182c2", ) -maven_jar( - name = "lucene_sandbox", - artifact = "org.apache.lucene:lucene-sandbox:" + LUCENE_VERS, - sha1 = "30a91f120706ba66732d5a974b56c6971b3c8a16", -) - maven_jar( name = "lucene_spatial", artifact = "org.apache.lucene:lucene-spatial:" + LUCENE_VERS, @@ -964,12 +952,6 @@ maven_jar( sha1 = "84ccf145ac2215e6bfa63baa3101c0af41017cfc", ) -maven_jar( - name = "jna", - artifact = "net.java.dev.jna:jna:4.1.0", - sha1 = "1c12d070e602efd8021891cdd7fd18bc129372d4", -) - JACKSON_VERSION = "2.6.6" maven_jar( @@ -978,12 +960,6 @@ maven_jar( sha1 = "02eb801df67aacaf5b1deb4ac626e1964508e47b", ) -maven_jar( - name = "jackson_dataformat_smile", - artifact = "com.fasterxml.jackson.dataformat:jackson-dataformat-smile:" + JACKSON_VERSION, - sha1 = "ccbfc948748ed2754a58c1af9e0a02b5cc1aed69", -) - maven_jar( name = "jackson_dataformat_cbor", artifact = "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:" + JACKSON_VERSION, diff --git a/lib/elasticsearch/BUILD b/lib/elasticsearch/BUILD index 418cb698a6..32a8cd0916 100644 --- a/lib/elasticsearch/BUILD +++ b/lib/elasticsearch/BUILD @@ -7,20 +7,15 @@ java_library( runtime_deps = [ ":compress-lzf", ":hppc", - ":jna", ":jsr166e", ":netty", ":t-digest", "//lib/jackson:jackson-core", "//lib/jackson:jackson-dataformat-cbor", - "//lib/jackson:jackson-dataformat-smile", - "//lib/joda:joda-time", - "//lib/lucene:lucene-codecs", "//lib/lucene:lucene-highlighter", "//lib/lucene:lucene-join", "//lib/lucene:lucene-memory", "//lib/lucene:lucene-queries", - "//lib/lucene:lucene-sandbox", "//lib/lucene:lucene-spatial", "//lib/lucene:lucene-suggest", ], @@ -60,9 +55,3 @@ java_library( visibility = ["//lib/elasticsearch:__pkg__"], exports = ["@t_digest//jar"], ) - -java_library( - name = "jna", - data = ["//lib:LICENSE-Apache2.0"], - exports = ["@jna//jar"], -) diff --git a/lib/jackson/BUILD b/lib/jackson/BUILD index 4847371502..5938c7b1b9 100644 --- a/lib/jackson/BUILD +++ b/lib/jackson/BUILD @@ -8,12 +8,6 @@ java_library( exports = ["@jackson_core//jar"], ) -java_library( - name = "jackson-dataformat-smile", - data = ["//lib:LICENSE-Apache2.0"], - exports = ["@jackson_dataformat_smile//jar"], -) - java_library( name = "jackson-dataformat-cbor", data = ["//lib:LICENSE-Apache2.0"], diff --git a/lib/lucene/BUILD b/lib/lucene/BUILD index bbf43a65b0..6590af4a46 100644 --- a/lib/lucene/BUILD +++ b/lib/lucene/BUILD @@ -22,13 +22,6 @@ java_library( runtime_deps = [":lucene-core-and-backward-codecs"], ) -java_library( - name = "lucene-codecs", - data = ["//lib:LICENSE-Apache2.0"], - visibility = ["//visibility:public"], - exports = ["@lucene_codecs//jar"], -) - java_library( name = "lucene-core", data = ["//lib:LICENSE-Apache2.0"], @@ -70,12 +63,6 @@ java_library( exports = ["@lucene_memory//jar"], ) -java_library( - name = "lucene-sandbox", - data = ["//lib:LICENSE-Apache2.0"], - exports = ["@lucene_sandbox//jar"], -) - java_library( name = "lucene-spatial", data = ["//lib:LICENSE-Apache2.0"], From c6d2399707d59fb373438facaaa4846603c75898 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sun, 15 Apr 2018 23:53:23 +0100 Subject: [PATCH 04/48] Log NoteDb migration state transitions ReviewDb to NoteDb migration is very delicate and needs special attention, especially in case of failures. Having a more detailed logging of the different stages in the error_log helps understand the problems and the impacts on what happens in production during those phases. Change-Id: I4a6270a52871ad453650f7c1eed483b7227f803c --- .../com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java index c11aeef5f4..3aaa0ae625 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java @@ -724,6 +724,7 @@ public class NoteDbMigrator implements AutoCloseable { // Only set in-memory state once it's been persisted to storage. globalNotesMigration.setFrom(newState); + log.info("Migration state: {} => {}", expectedOldState, newState); return newState; } From 1f3298b80d893d5c6a3b9ca8d453d657fa37be11 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Mon, 16 Apr 2018 07:44:14 +0100 Subject: [PATCH 05/48] Fix logging of a change ID that failed to rebuild. Add id to the formatting of the warning message when a change fails to rebuild. Change-Id: I9e08bd56250794894327aa6a9144ee2f172b1b46 --- .../com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java index 3aaa0ae625..df415ce107 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java @@ -615,6 +615,7 @@ public class NoteDbMigrator implements AutoCloseable { log.warn( "Change {} previously failed to rebuild;" + " skipping primary storage migration", + id, e); } else { throw e; From af97e0275b3c677ff865ae3215e8777b72390e87 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Mon, 16 Apr 2018 08:49:26 +0100 Subject: [PATCH 06/48] Add extra info when checking access to change Put additional information to the exceptions thrown when filtering the list of changes that a user has access to: - Change id that failed to be checked - Project name that was not found when checking the change Without this extra data, it is impossible for a Gerrit admin to figure out where is the problem and how to fix it. Change-Id: I79b07e0fcfa7ebcc2546006cf7667128ed9f4794 --- .../google/gerrit/server/project/DefaultPermissionBackend.java | 2 +- .../gerrit/server/query/change/ChangeIsVisibleToPredicate.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackend.java index a37d35633d..6ae3fa2988 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackend.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackend.java @@ -75,7 +75,7 @@ public class DefaultPermissionBackend extends PermissionBackend { if (state != null) { return state.controlFor(user).asForProject().database(db); } - return FailedPermissionBackend.project("not found"); + return FailedPermissionBackend.project("project '" + project.get() + "' not found"); } catch (IOException e) { return FailedPermissionBackend.project("unavailable", e); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java index 17296bc9c2..c9530dcfa0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java @@ -64,7 +64,7 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate .database(db) .test(ChangePermission.READ); } catch (PermissionBackendException e) { - throw new OrmException("unable to check permissions", e); + throw new OrmException("unable to check permissions on change " + cd.getId(), e); } if (visible) { cd.cacheVisibleTo(user); From 0fd336bbb9c31d42693a2f0a0154adfef01d6afd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 10:11:00 +0200 Subject: [PATCH 07/48] AbstractQueryChangesTest: Add test coverage for assignee related queries Bug: Issue 8605 Change-Id: Ic16110ad08cf4bbc374181b87b2c31e76cfc6ab8 --- .../change/AbstractQueryChangesTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 907a36a381..6fbe692a73 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -40,6 +40,7 @@ import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.changes.AddReviewerInput; +import com.google.gerrit.extensions.api.changes.AssigneeInput; import com.google.gerrit.extensions.api.changes.Changes.QueryRequest; import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.HashtagsInput; @@ -2004,6 +2005,24 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("bug:QUERY789"); } + @Test + public void assignee() throws Exception { + TestRepository repo = createProject("repo"); + Change change1 = insert(repo, newChange(repo)); + Change change2 = insert(repo, newChange(repo)); + + AssigneeInput input = new AssigneeInput(); + input.assignee = user.getUserName(); + gApi.changes().id(change1.getChangeId()).setAssignee(input); + + assertQuery("is:assigned", change1); + assertQuery("-is:assigned", change2); + assertQuery("is:unassigned", change2); + assertQuery("-is:unassigned", change1); + assertQuery("assignee:" + user.getUserName(), change1); + assertQuery("-assignee:" + user.getUserName(), change2); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null); } From 00e00d96e42bf85e6ea1463509c85e40df6a7eba Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 10:29:12 +0200 Subject: [PATCH 08/48] AbstractQueryChangesTest: Add test coverage for since: and until: predicates Bug: Issue 8605 Change-Id: I85570c019fd00cf1503a4f44170589ab37b4d50a --- .../change/AbstractQueryChangesTest.java | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 6fbe692a73..c06f3054af 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1131,7 +1131,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { } @Test - public void byBefore() throws Exception { + public void byBeforeUntil() throws Exception { long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS); resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS); TestRepository repo = createProject("repo"); @@ -1140,20 +1140,22 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs)); TestTimeUtil.setClockStep(0, MILLISECONDS); - assertQuery("before:2009-09-29"); - assertQuery("before:2009-09-30"); - assertQuery("before:\"2009-09-30 16:59:00 -0400\""); - assertQuery("before:\"2009-09-30 20:59:00 -0000\""); - assertQuery("before:\"2009-09-30 20:59:00\""); - assertQuery("before:\"2009-09-30 17:02:00 -0400\"", change1); - assertQuery("before:\"2009-10-01 21:02:00 -0000\"", change1); - assertQuery("before:\"2009-10-01 21:02:00\"", change1); - assertQuery("before:2009-10-01", change1); - assertQuery("before:2009-10-03", change2, change1); + for (String predicate : new String[] {"before:", "until:"}) { + assertQuery(predicate + "2009-09-29"); + assertQuery(predicate + "2009-09-30"); + assertQuery(predicate + "\"2009-09-30 16:59:00 -0400\""); + assertQuery(predicate + "\"2009-09-30 20:59:00 -0000\""); + assertQuery(predicate + "\"2009-09-30 20:59:00\""); + assertQuery(predicate + "\"2009-09-30 17:02:00 -0400\"", change1); + assertQuery(predicate + "\"2009-10-01 21:02:00 -0000\"", change1); + assertQuery(predicate + "\"2009-10-01 21:02:00\"", change1); + assertQuery(predicate + "2009-10-01", change1); + assertQuery(predicate + "2009-10-03", change2, change1); + } } @Test - public void byAfter() throws Exception { + public void byAfterSince() throws Exception { long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS); resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS); TestRepository repo = createProject("repo"); @@ -1162,11 +1164,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs)); TestTimeUtil.setClockStep(0, MILLISECONDS); - assertQuery("after:2009-10-03"); - assertQuery("after:\"2009-10-01 20:59:59 -0400\"", change2); - assertQuery("after:\"2009-10-01 20:59:59 -0000\"", change2); - assertQuery("after:2009-10-01", change2); - assertQuery("after:2009-09-30", change2, change1); + for (String predicate : new String[] {"after:", "since:"}) { + assertQuery(predicate + "2009-10-03"); + assertQuery(predicate + "\"2009-10-01 20:59:59 -0400\"", change2); + assertQuery(predicate + "\"2009-10-01 20:59:59 -0000\"", change2); + assertQuery(predicate + "2009-10-01", change2); + assertQuery(predicate + "2009-09-30", change2, change1); + } } @Test From 2b74109f6d5e50d6f827508e09d48e14909c1453 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 10:35:22 +0200 Subject: [PATCH 09/48] AbstractQueryChangesTest#byStatusOpen: Add coverage for is:pending Bug: Issue 8605 Change-Id: Ifad41ff649a4fadef8d825bb3c2ddaa28df0dbc2 --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index c06f3054af..6b4715316a 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -339,6 +339,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("status:pe", expected); assertQuery("status:pen", expected); assertQuery("is:open", expected); + assertQuery("is:pending", expected); } @Test From 521cb3d828f2c94663691e8f65fed1a95d2e66bc Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 11:16:40 +0200 Subject: [PATCH 10/48] AbstractQueryChangesTest: Add coverage for is:reviewed and status:reviewed Bug: Issue 8605 Change-Id: I8f06b5b7776f49756efef9c91c20453b0a76f148 --- .../change/AbstractQueryChangesTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 6b4715316a..beb8cdd428 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1636,6 +1636,28 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { } } + @Test + public void byReviewed() throws Exception { + TestRepository repo = createProject("repo"); + Account.Id otherUser = + accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); + Change change1 = insert(repo, newChange(repo)); + Change change2 = insert(repo, newChange(repo)); + + assertQuery("is:reviewed"); + assertQuery("status:reviewed"); + assertQuery("-is:reviewed", change2, change1); + assertQuery("-status:reviewed", change2, change1); + + requestContext.setContext(newRequestContext(otherUser)); + gApi.changes().id(change1.getChangeId()).current().review(ReviewInput.recommend()); + + assertQuery("is:reviewed", change1); + assertQuery("status:reviewed", change1); + assertQuery("-is:reviewed", change2); + assertQuery("-status:reviewed", change2); + } + @Test public void reviewerin() throws Exception { Account.Id user1 = accountManager.authenticate(AuthRequest.forUser("user1")).getAccountId(); From bf8b5dd993f26f227f9479a902167e04989ee506 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 11:34:32 +0200 Subject: [PATCH 11/48] AbstractQueryChangesTest: Add coverage for parentproject: predicate Bug: Issue 8605 Change-Id: I9300b70ce3b56e6dd02a93ff30c858d8041d2239 --- .../change/AbstractQueryChangesTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index beb8cdd428..a10d629906 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -50,6 +50,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput; import com.google.gerrit.extensions.api.changes.StarsInput; import com.google.gerrit.extensions.api.groups.GroupInput; +import com.google.gerrit.extensions.api.projects.ProjectInput; import com.google.gerrit.extensions.client.ProjectWatchInfo; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.ChangeInfo; @@ -525,6 +526,17 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("project:repo2", change2); } + @Test + public void byParentProject() throws Exception { + TestRepository repo1 = createProject("repo1"); + TestRepository repo2 = createProject("repo2", "repo1"); + Change change1 = insert(repo1, newChange(repo1)); + Change change2 = insert(repo2, newChange(repo2)); + + assertQuery("parentproject:repo1", change2, change1); + assertQuery("parentproject:repo2", change2); + } + @Test public void byProjectPrefix() throws Exception { TestRepository repo1 = createProject("repo1"); @@ -2168,6 +2180,14 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { return new TestRepository<>(repoManager.openRepository(new Project.NameKey(name))); } + protected TestRepository createProject(String name, String parent) throws Exception { + ProjectInput input = new ProjectInput(); + input.name = name; + input.parent = parent; + gApi.projects().create(input).get(); + return new TestRepository<>(repoManager.openRepository(new Project.NameKey(name))); + } + protected QueryRequest newQuery(Object query) { return gApi.changes().query(query.toString()); } From 66a701ca27111eb1b5425248042cc5716bdcc8dd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 11:51:33 +0200 Subject: [PATCH 12/48] AbstractQueryChangesTest: Add coverage for is:reviewer and reviewer:self Bug: Issue 8605 Change-Id: I592d7c24261490376ba972edc3cecb59471079da --- .../server/query/change/AbstractQueryChangesTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index a10d629906..d04d8a5bcc 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1627,6 +1627,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { TestRepository repo = createProject("repo"); Change change1 = insert(repo, newChange(repo)); Change change2 = insert(repo, newChange(repo)); + Change change3 = insert(repo, newChange(repo)); insert(repo, newChange(repo)); AddReviewerInput rin = new AddReviewerInput(); @@ -1639,6 +1640,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { rin.state = ReviewerState.CC; gApi.changes().id(change2.getId().get()).addReviewer(rin); + assertQuery("is:reviewer"); + assertQuery("reviewer:self"); + gApi.changes().id(change3.getChangeId()).revision("current").review(ReviewInput.recommend()); + assertQuery("is:reviewer", change3); + assertQuery("reviewer:self", change3); + if (notesMigration.readChanges()) { assertQuery("reviewer:" + user1, change1); assertQuery("cc:" + user1, change2); From d58e473bed6b43fddb2829064f78c2dd098623c0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 11:56:44 +0200 Subject: [PATCH 13/48] user-search: Add missing documentation of is:cc predicate Change-Id: I35ac5b436b17ba3dc338daf8c9c0d901694f39eb --- Documentation/user-search.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index 5164d5da1a..2902d58414 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -329,6 +329,11 @@ is:reviewer:: True on any change where the current user is a reviewer. Same as `reviewer:self`. +is:cc:: ++ +True on any change where the current user is in CC. +Same as `cc:self`. + is:open, is:pending:: + True if the change is open. From 89f4c7ca123663158072ed5ae6aa277edb2bb307 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 12:09:52 +0200 Subject: [PATCH 14/48] AbstractQueryChangesTest: Add coverage for the commit: predicate Bug: Issue 8605 Change-Id: Ic80cd94cbaf7b1e5db5824c35f265dc00ab4c71e --- .../server/query/change/AbstractQueryChangesTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index d04d8a5bcc..6e199b72cf 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -418,13 +418,15 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { public void byCommit() throws Exception { TestRepository repo = createProject("repo"); ChangeInserter ins = newChange(repo); - insert(repo, ins); + Change change = insert(repo, ins); String sha = ins.getCommit().name(); assertQuery("0000000000000000000000000000000000000000"); + assertQuery("commit:0000000000000000000000000000000000000000"); for (int i = 0; i <= 36; i++) { String q = sha.substring(0, 40 - i); - assertQuery(q, ins.getChange()); + assertQuery(q, change); + assertQuery("commit:" + q, change); } } From 098d3c2b965522ddad13bed003c1de86217081cd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 12:20:41 +0200 Subject: [PATCH 15/48] AbstractQueryChangesTest: Use Lists.newArrayList Instead of: new String[] { ... } Change-Id: I9ed5921f221611015d87a270f691db10a4b82b2d --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 6e199b72cf..23f0ac41b0 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1155,7 +1155,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs)); TestTimeUtil.setClockStep(0, MILLISECONDS); - for (String predicate : new String[] {"before:", "until:"}) { + for (String predicate : Lists.newArrayList("before:", "until:")) { assertQuery(predicate + "2009-09-29"); assertQuery(predicate + "2009-09-30"); assertQuery(predicate + "\"2009-09-30 16:59:00 -0400\""); @@ -1179,7 +1179,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs)); TestTimeUtil.setClockStep(0, MILLISECONDS); - for (String predicate : new String[] {"after:", "since:"}) { + for (String predicate : Lists.newArrayList("after:", "since:")) { assertQuery(predicate + "2009-10-03"); assertQuery(predicate + "\"2009-10-01 20:59:59 -0400\"", change2); assertQuery(predicate + "\"2009-10-01 20:59:59 -0000\"", change2); From addc44c5a2c0d481453cd66462be751ba040d603 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 12:24:52 +0200 Subject: [PATCH 16/48] AbstractQueryChangesTest#bySize: Make it easier to discover size: and delta: Bug: Issue 8605 Change-Id: I226805c403eba9af08ce245e5387d0227ab944cd --- .../query/change/AbstractQueryChangesTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 23f0ac41b0..4c9a321319 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1235,13 +1235,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("deleted:<=0", change1); - for (String str : Lists.newArrayList("delta", "size")) { - assertQuery(str + ":<2"); - assertQuery(str + ":3", change1); - assertQuery(str + ":>2", change1); - assertQuery(str + ":>=3", change1); - assertQuery(str + ":<3", change2); - assertQuery(str + ":<=2", change2); + for (String str : Lists.newArrayList("delta:", "size:")) { + assertQuery(str + "<2"); + assertQuery(str + "3", change1); + assertQuery(str + ">2", change1); + assertQuery(str + ">=3", change1); + assertQuery(str + "<3", change2); + assertQuery(str + "<=2", change2); } } From 120cc281f0f7a3d739b2adce324d8c0d1a994104 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 13:20:59 +0200 Subject: [PATCH 17/48] AbstractQueryChangesTest#submitRecords: Add coverage for submittable:closed Bug: Issue 8605 Change-Id: Ifcb97475008b2f91901e800f706769333b2c47c0 --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 4c9a321319..f6bacffc63 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1748,6 +1748,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { // NEED records don't have associated users. assertQuery("label:CodE-RevieW=need,user1"); assertQuery("label:CodE-RevieW=need,user"); + + gApi.changes().id(change1.getId().get()).current().submit(); + assertQuery("submittable:ok"); + assertQuery("submittable:closed", change1); } @Test From f6b0f7def37f9d18ee0ac1516f8ec67f63ddc288 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 13:46:39 +0200 Subject: [PATCH 18/48] AbstractQueryChangesTest: Add coverage for is:visible Bug: Issue 8605 Change-Id: Ib0fab616042f5fd7afbfc7a6f189fe8a6a443632 --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index f6bacffc63..3e328938bf 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1379,6 +1379,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Account.Id user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); assertQuery(q + " visibleto:" + user2.get(), change1); + + requestContext.setContext( + newRequestContext( + accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId())); + assertQuery("is:visible", change1); } @Test From d434871dfb04439332f55f38c49fbdee6848b012 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 13:52:41 +0200 Subject: [PATCH 19/48] AbstractQueryChangesTest: Add coverage of is:cc and cc:self Bug: Issue 8605 Change-Id: I01c216b516cde65ba130f544a201a7e953225e6f --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 3e328938bf..c43b396b10 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1653,12 +1653,17 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("is:reviewer", change3); assertQuery("reviewer:self", change3); + requestContext.setContext(newRequestContext(user1)); if (notesMigration.readChanges()) { assertQuery("reviewer:" + user1, change1); assertQuery("cc:" + user1, change2); + assertQuery("is:cc", change2); + assertQuery("cc:self", change2); } else { assertQuery("reviewer:" + user1, change2, change1); assertQuery("cc:" + user1); + assertQuery("is:cc"); + assertQuery("cc:self"); } } From a32cb2747888301a2591ce28d0bfbdf67aff21a0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 14:06:53 +0200 Subject: [PATCH 20/48] AbstractQueryChangesTest: Add coverage of is:mergeable Bug: Issue 8605 Change-Id: Id4674d140afa95ff8f5475c083d51c46e05abfe3 --- .../change/AbstractQueryChangesTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index c43b396b10..933e695b2e 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1587,6 +1587,26 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("conflicts:" + change4.getId().get()); } + @Test + public void mergeable() throws Exception { + TestRepository repo = createProject("repo"); + RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create()); + RevCommit commit2 = repo.parseBody(repo.commit().add("file1", "contents2").create()); + Change change1 = insert(repo, newChangeForCommit(repo, commit1)); + Change change2 = insert(repo, newChangeForCommit(repo, commit2)); + + assertQuery("conflicts:" + change1.getId().get(), change2); + assertQuery("conflicts:" + change2.getId().get(), change1); + assertQuery("is:mergeable", change2, change1); + + gApi.changes().id(change1.getChangeId()).revision("current").review(ReviewInput.approve()); + gApi.changes().id(change1.getChangeId()).revision("current").submit(); + + assertQuery("conflicts:" + change2.getId().get()); + assertQuery("status:open is:mergeable"); + assertQuery("status:open -is:mergeable", change2); + } + @Test public void reviewedBy() throws Exception { resetTimeWithClockStep(2, MINUTES); From a540ac2c87f8fa2cb1c5bcc37b82ae85656461f0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 15:42:09 +0200 Subject: [PATCH 21/48] AbstractQueryChangesTest: Add back test for visible: predicate Change Icfcb34efe removed draft workflow and at the same time removed the 'explicitVisibleTo' test, which was testing visibility of draft changes. As a result, the only test coverage of the visibleto: query was removed. Add 'explicitVisibleTo' back, but rename it to 'visible'. The implicit visibility is already covered in the 'byPrivate' test. Change-Id: I9be04156ce4401754cebb0c969d1b233a132735e --- .../query/change/AbstractQueryChangesTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 216c77f53b..3fece961eb 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1556,6 +1556,23 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery(commit.getId().getName().substring(0, 6), change); } + @Test + public void visible() throws Exception { + TestRepository repo = createProject("repo"); + Change change1 = insert(repo, newChange(repo)); + Change change2 = insert(repo, newChange(repo)); + + gApi.changes().id(change2.getChangeId()).setPrivate(true, "private"); + + String q = "project:repo"; + assertQuery(q, change2, change1); + + // Second user cannot see first user's private change. + Account.Id user2 = + accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); + assertQuery(q + " visibleto:" + user2.get(), change1); + } + @Test public void byCommentBy() throws Exception { TestRepository repo = createProject("repo"); From 31c040a2339545caeebd56ace11c7526f1f8f465 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Mon, 16 Apr 2018 09:58:43 -0400 Subject: [PATCH 22/48] lib/elasticsearch: restore jackson_dataformat_smile As removing it in 40744d99f introduced a related NoClassDefFoundError regression. Bug: Issue 6094 Change-Id: I03c4801ae8c1cfb0b74ad39ef1398cd31225ba14 --- WORKSPACE | 6 ++++++ lib/elasticsearch/BUILD | 1 + lib/jackson/BUILD | 6 ++++++ 3 files changed, 13 insertions(+) diff --git a/WORKSPACE b/WORKSPACE index 7a3284bdfb..d615125c7e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -966,6 +966,12 @@ maven_jar( sha1 = "34c7b7ff495fc6b049612bdc9db0900a68e112f8", ) +maven_jar( + name = "jackson_dataformat_smile", + artifact = "com.fasterxml.jackson.dataformat:jackson-dataformat-smile:" + JACKSON_VERSION, + sha1 = "ccbfc948748ed2754a58c1af9e0a02b5cc1aed69", +) + maven_jar( name = "httpasyncclient", artifact = "org.apache.httpcomponents:httpasyncclient:4.1.2", diff --git a/lib/elasticsearch/BUILD b/lib/elasticsearch/BUILD index 32a8cd0916..d9c6c18f94 100644 --- a/lib/elasticsearch/BUILD +++ b/lib/elasticsearch/BUILD @@ -12,6 +12,7 @@ java_library( ":t-digest", "//lib/jackson:jackson-core", "//lib/jackson:jackson-dataformat-cbor", + "//lib/jackson:jackson-dataformat-smile", "//lib/lucene:lucene-highlighter", "//lib/lucene:lucene-join", "//lib/lucene:lucene-memory", diff --git a/lib/jackson/BUILD b/lib/jackson/BUILD index 5938c7b1b9..8ade0cf8b0 100644 --- a/lib/jackson/BUILD +++ b/lib/jackson/BUILD @@ -13,3 +13,9 @@ java_library( data = ["//lib:LICENSE-Apache2.0"], exports = ["@jackson_dataformat_cbor//jar"], ) + +java_library( + name = "jackson-dataformat-smile", + data = ["//lib:LICENSE-Apache2.0"], + exports = ["@jackson_dataformat_smile//jar"], +) From ace569abb1d77f15501079e46e46ef886703fe42 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 16:35:27 +0200 Subject: [PATCH 23/48] Fix internal errors when 'destination:' refers to non-existing destination When querying with the destination: predicate using a destination name that does not exist, it is supposed to return an error: "Unknown named destination" However it was not, because of two problems: - When the revision is null, VersionedAccountDestinations's call to getPathInfos results in NPE due to dereferencing the revision. - When no destinations exist, DestinationList.getDestinations returns an empty list, but the ChangeQueryBuilder is only testing for null Fix both of the bugs and add a test that querying "destination:" with a non-existing destination results in the correct response. Adding tests for existing destinations is not within the scope of this change. Change-Id: I7787a90a508f61ef19586f327c6be427a0870e69 --- .../server/account/VersionedAccountDestinations.java | 3 +++ .../gerrit/server/query/change/ChangeQueryBuilder.java | 2 +- .../server/query/change/AbstractQueryChangesTest.java | 10 ++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java index 5116cfb359..2bb4bb75a2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java @@ -53,6 +53,9 @@ public class VersionedAccountDestinations extends VersionedMetaData { @Override protected void onLoad() throws IOException, ConfigInvalidException { + if (revision == null) { + return; + } String prefix = DestinationList.DIR_NAME + "/"; for (PathInfo p : getPathInfos(true)) { if (p.fileMode == FileMode.REGULAR_FILE) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index e042b1d39b..17f52dce30 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -1047,7 +1047,7 @@ public class ChangeQueryBuilder extends QueryBuilder { VersionedAccountDestinations d = VersionedAccountDestinations.forUser(self()); d.load(git); Set destinations = d.getDestinationList().getDestinations(name); - if (destinations != null) { + if (destinations != null && !destinations.isEmpty()) { return new DestinationPredicate(destinations, name); } } catch (RepositoryNotFoundException e) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 933e695b2e..e57fe1d717 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -2105,6 +2105,16 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("-assignee:" + user.getUserName(), change2); } + @Test + public void userDestination() throws Exception { + TestRepository repo = createProject("repo"); + insert(repo, newChange(repo)); + + assertThatQueryException("destination:foo") + .hasMessageThat() + .isEqualTo("Unknown named destination: foo"); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null); } From b24340bdf586fa9c307cf00272868ea31b0f2e95 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 16 Apr 2018 16:42:08 +0200 Subject: [PATCH 24/48] AbstractQueryChangesTest: Add stub test for "query:" predicate Add a stub test which only tests that the correct response is returned when the predicate is used with a non-existing user query. Tests for existing user queries will be added in a followup commit. Change-Id: I5fd7a703d52e238ccc91038f1575a87c2c71b5f0 --- .../server/query/change/AbstractQueryChangesTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index e57fe1d717..2f640ffa32 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -2115,6 +2115,14 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { .isEqualTo("Unknown named destination: foo"); } + @Test + public void userQuery() throws Exception { + TestRepository repo = createProject("repo"); + insert(repo, newChange(repo)); + + assertThatQueryException("query:foo").hasMessageThat().isEqualTo("Unknown named query: foo"); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null); } From d77c347bbc02683c947691736eee6e37bbb6e68b Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Mon, 16 Apr 2018 11:51:46 -0400 Subject: [PATCH 25/48] AbstractQueryChangesTest: explicitly cover is:owner Bug: Issue 8605 Change-Id: I89ff17b2fc1c59b2beeb8017c9dd44e4fe964dba --- .../gerrit/server/query/change/AbstractQueryChangesTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 933e695b2e..cc2de0b146 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -438,6 +438,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); Change change2 = insert(repo, newChange(repo), user2); + assertQuery("is:owner", change1); assertQuery("owner:" + userId.get(), change1); assertQuery("owner:" + user2, change2); From ab5ca3b1d3075108c9e03ebdf9d7f16dea21466a Mon Sep 17 00:00:00 2001 From: Borui Tao Date: Mon, 16 Apr 2018 20:07:20 -0400 Subject: [PATCH 26/48] doc: add groups index to index activate command The groups index is also supported by the activate command in the same way as the changes and accounts indices. Bug: Issue 8775 Change-Id: I1fda1794626eda266114b902e0d81b5512514824 --- Documentation/cmd-index-activate.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/cmd-index-activate.txt b/Documentation/cmd-index-activate.txt index 418e872bcc..85c89c7379 100644 --- a/Documentation/cmd-index-activate.txt +++ b/Documentation/cmd-index-activate.txt @@ -31,6 +31,7 @@ This command is intended to be used in scripts. Currently supported values: * changes * accounts + * groups == EXAMPLES Activate the latest change index: From 39ffbcbe0820de7a02eaa980b51f8ce0c3e55a91 Mon Sep 17 00:00:00 2001 From: Borui Tao Date: Mon, 16 Apr 2018 20:16:46 -0400 Subject: [PATCH 27/48] doc: fix the example of index activate command Executing the previous example command of index activate gave the error: "fatal: gerrit: activate not found". This change adds the "index" to this example command so that it can be executed correctly. Bug: Issue 8775 Change-Id: I08fe58d6920810ff0950b1ad62440b930a456126 --- Documentation/cmd-index-activate.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/cmd-index-activate.txt b/Documentation/cmd-index-activate.txt index 85c89c7379..4428d12708 100644 --- a/Documentation/cmd-index-activate.txt +++ b/Documentation/cmd-index-activate.txt @@ -37,7 +37,7 @@ This command is intended to be used in scripts. Activate the latest change index: ---- - $ ssh -p 29418 review.example.com gerrit activate changes + $ ssh -p 29418 review.example.com gerrit index activate changes ---- GERRIT From 603c45636ef464be3c215416383363bb8a67e135 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 09:10:03 +0200 Subject: [PATCH 28/48] AbstractQueryChangesTest: Add test coverage for destination: predicate Bug: Issue 8604 Change-Id: Ie8d58511a95003a90869e5922f4792889d972bfd --- .../change/AbstractQueryChangesTest.java | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index fdf78d5a47..e9dd2e3d31 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -2108,12 +2108,37 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @Test public void userDestination() throws Exception { - TestRepository repo = createProject("repo"); - insert(repo, newChange(repo)); + TestRepository repo1 = createProject("repo1"); + Change change1 = insert(repo1, newChange(repo1)); + TestRepository repo2 = createProject("repo2"); + Change change2 = insert(repo2, newChange(repo2)); assertThatQueryException("destination:foo") .hasMessageThat() .isEqualTo("Unknown named destination: foo"); + + String destination1 = "refs/heads/master\trepo1"; + String destination2 = "refs/heads/master\trepo2"; + String destination3 = "refs/heads/master\trepo1\nrefs/heads/master\trepo2"; + String destination4 = "refs/heads/master\trepo3"; + String destination5 = "refs/heads/other\trepo1"; + + TestRepository allUsers = new TestRepository<>(repoManager.openRepository(allUsersName)); + String refsUsers = RefNames.refsUsers(userId); + allUsers.branch(refsUsers).commit().add("destinations/destination1", destination1).create(); + allUsers.branch(refsUsers).commit().add("destinations/destination2", destination2).create(); + allUsers.branch(refsUsers).commit().add("destinations/destination3", destination3).create(); + allUsers.branch(refsUsers).commit().add("destinations/destination4", destination4).create(); + allUsers.branch(refsUsers).commit().add("destinations/destination5", destination5).create(); + + Ref userRef = allUsers.getRepository().exactRef(refsUsers); + assertThat(userRef).isNotNull(); + + assertQuery("destination:destination1", change1); + assertQuery("destination:destination2", change2); + assertQuery("destination:destination3", change2, change1); + assertQuery("destination:destination4"); + assertQuery("destination:destination5"); } @Test From aaa40aa1b82f37b1b488e278266f593581881ff4 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 09:38:46 +0200 Subject: [PATCH 29/48] AbstractQueryChangesTest: Add test coverage for query: predicate Bug: Issue 8604 Change-Id: I197a0e40b9622eb49cad7c8b3be1db3f3d50fa28 --- .../change/AbstractQueryChangesTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index e9dd2e3d31..0178cd3404 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -2144,9 +2144,31 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @Test public void userQuery() throws Exception { TestRepository repo = createProject("repo"); - insert(repo, newChange(repo)); + Change change1 = insert(repo, newChange(repo)); + Change change2 = insert(repo, newChangeForBranch(repo, "stable")); + + String queries = + "query1\tproject:repo\n" + + "query2\tproject:repo status:open\n" + + "query3\tproject:repo branch:stable\n" + + "query4\tproject:repo branch:other"; + + TestRepository allUsers = new TestRepository<>(repoManager.openRepository(allUsersName)); + String refsUsers = RefNames.refsUsers(userId); + allUsers.branch(refsUsers).commit().add("queries", queries).create(); + + Ref userRef = allUsers.getRepository().exactRef(refsUsers); + assertThat(userRef).isNotNull(); assertThatQueryException("query:foo").hasMessageThat().isEqualTo("Unknown named query: foo"); + + assertQuery("query:query1", change2, change1); + assertQuery("query:query2", change2, change1); + gApi.changes().id(change1.getChangeId()).revision("current").review(ReviewInput.approve()); + gApi.changes().id(change1.getChangeId()).revision("current").submit(); + assertQuery("query:query2", change2); + assertQuery("query:query3", change2); + assertQuery("query:query4"); } protected ChangeInserter newChange(TestRepository repo) throws Exception { From af487ee22a5a62245a52d426de2d467ec5ba8605 Mon Sep 17 00:00:00 2001 From: Alexandre Philbert Date: Sun, 15 Apr 2018 20:23:12 -0400 Subject: [PATCH 30/48] Use try with resource in DelegatingClassLoader Change-Id: I7cc08ad9cf0516a00d658e9f32f178401a6bb271 (cherry picked from commit a894b51ccd4926f1ed45a8185e2dc0e406e8a6eb) --- .../server/plugins/DelegatingClassLoader.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/DelegatingClassLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/DelegatingClassLoader.java index daee9c7afd..32adb9c8e1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/DelegatingClassLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/DelegatingClassLoader.java @@ -31,13 +31,17 @@ public class DelegatingClassLoader extends ClassLoader { @Override public Class findClass(String name) throws ClassNotFoundException { String path = name.replace('.', '/') + ".class"; - InputStream resource = target.getResourceAsStream(path); - if (resource != null) { - try { - byte[] bytes = ByteStreams.toByteArray(resource); - return defineClass(name, bytes, 0, bytes.length); - } catch (IOException e) { + try (InputStream resource = target.getResourceAsStream(path)) { + if (resource != null) { + try { + byte[] bytes = ByteStreams.toByteArray(resource); + return defineClass(name, bytes, 0, bytes.length); + } catch (IOException e) { + // throws ClassNotFoundException later + } } + } catch (IOException e) { + // throws ClassNotFoundException later } throw new ClassNotFoundException(name); } From c9fb3c26265f680dc1eb0d7cfc22f53c3083eda1 Mon Sep 17 00:00:00 2001 From: Alexandre Philbert Date: Sun, 15 Apr 2018 19:13:47 -0400 Subject: [PATCH 31/48] Use SecureRandom instead of Random randSuffix() mentions an issue in a comment that has to do with needing to randomize suffixes to prevent an attack. Random has a predictable pseudo-random generator, so, in order to mitigate this attack it is safer to use SecureRandom. Change-Id: I804882bf0aa47a10f325f82992991bbd8434d472 --- .../java/com/google/gerrit/server/change/FileContentUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java index ff5fb0b764..8d4cbba6fc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java @@ -34,7 +34,7 @@ import com.google.inject.Singleton; import eu.medsea.mimeutil.MimeType; import java.io.IOException; import java.io.OutputStream; -import java.util.Random; +import java.security.SecureRandom; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import org.eclipse.jgit.errors.LargeObjectException; @@ -57,7 +57,7 @@ public class FileContentUtil { private static final String X_GIT_GITLINK = "x-git/gitlink"; private static final int MAX_SIZE = 5 << 20; private static final String ZIP_TYPE = "application/zip"; - private static final Random rng = new Random(); + private static final SecureRandom rng = new SecureRandom(); private final GitRepositoryManager repoManager; private final FileTypeRegistry registry; From 2f0ec10599d4e99190ce6b4483eec8b38efd8856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Tue, 17 Apr 2018 10:58:24 +0200 Subject: [PATCH 32/48] Set version to 2.13.12-SNAPSHOT Change-Id: Ia1b4c0ddad3744a537cef1b3853d5b0dc2086a4e --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index c20a4cb909..3b4e27482b 100644 --- a/VERSION +++ b/VERSION @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = '2.13.11' +GERRIT_VERSION = '2.13.12-SNAPSHOT' From 5489ad4b31b117fe5ec557f8ffb85099e8f965d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Tue, 28 Nov 2017 12:56:11 +0100 Subject: [PATCH 33/48] Add ProjectCache.remove(Project.NameKey name) method Until now we only had ProjectCache.remove(Project p) which requires a project instance. This could be a problem when running Gerrit HA in active/active mode and a project name has to be removed from the project-list cache on the other node. The project may not exist at that point in time (because it was just deleted) and trying to load a Project instance would lead to an error which would prevent removing this project's name from the project_list cache. Change-Id: Ida124ea95bb2cb776adbf8bd4d2bb77c0bdbbeaa --- .../com/google/gerrit/server/project/ProjectCache.java | 6 ++++++ .../google/gerrit/server/project/ProjectCacheImpl.java | 9 +++++++-- .../com/google/gerrit/server/project/RefControlTest.java | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java index 67bdc88c57..fa2f639279 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java @@ -59,6 +59,12 @@ public interface ProjectCache { */ void remove(Project p); + /** + * Remove information about the given project from the cache. It will no longer be returned from + * {@link #all()}. + */ + void remove(Project.NameKey name); + /** @return sorted iteration of projects. */ Iterable all(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java index 8a08052a87..7ce19c8f42 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java @@ -183,17 +183,22 @@ public class ProjectCacheImpl implements ProjectCache { @Override public void remove(final Project p) { + remove(p.getNameKey()); + } + + @Override + public void remove(Project.NameKey name) { listLock.lock(); try { SortedSet n = Sets.newTreeSet(list.get(ListKey.ALL)); - n.remove(p.getNameKey()); + n.remove(name); list.put(ListKey.ALL, Collections.unmodifiableSortedSet(n)); } catch (ExecutionException e) { log.warn("Cannot list avaliable projects", e); } finally { listLock.unlock(); } - evict(p); + evict(name); } @Override diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index d4d77bddc8..c04c474cd3 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -269,6 +269,9 @@ public class RefControlTest { public void remove(Project p) { } + @Override + public void remove(Project.NameKey name) {} + @Override public Iterable all() { return Collections.emptySet(); From 57a8cdb22ab694beb9b11ea21a2f801c9206251e Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 11:05:16 +0200 Subject: [PATCH 34/48] FileContentUtil: Open TreeWalk in try-with-resource Also inline the instantiation of ObjectReader instead of creating a variable which results in a "potentially unclosed resource" warning. Change-Id: Ib4bec8e41958d5df2fe31fbdc964ed12c9666f5b --- .../gerrit/server/change/FileContentUtil.java | 93 +++++++++---------- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java index 8d4cbba6fc..00b7a88497 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java @@ -42,7 +42,6 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; -import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -104,35 +103,35 @@ public class FileContentUtil { throws IOException, ResourceNotFoundException { try (RevWalk rw = new RevWalk(repo)) { RevCommit commit = rw.parseCommit(revstr); - ObjectReader reader = rw.getObjectReader(); - TreeWalk tw = TreeWalk.forPath(reader, path, commit.getTree()); - if (tw == null) { - throw new ResourceNotFoundException(); - } + try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, commit.getTree())) { + if (tw == null) { + throw new ResourceNotFoundException(); + } - org.eclipse.jgit.lib.FileMode mode = tw.getFileMode(0); - ObjectId id = tw.getObjectId(0); - if (mode == org.eclipse.jgit.lib.FileMode.GITLINK) { - return BinaryResult.create(id.name()).setContentType(X_GIT_GITLINK).base64(); - } + org.eclipse.jgit.lib.FileMode mode = tw.getFileMode(0); + ObjectId id = tw.getObjectId(0); + if (mode == org.eclipse.jgit.lib.FileMode.GITLINK) { + return BinaryResult.create(id.name()).setContentType(X_GIT_GITLINK).base64(); + } - ObjectLoader obj = repo.open(id, OBJ_BLOB); - byte[] raw; - try { - raw = obj.getCachedBytes(MAX_SIZE); - } catch (LargeObjectException e) { - raw = null; - } + ObjectLoader obj = repo.open(id, OBJ_BLOB); + byte[] raw; + try { + raw = obj.getCachedBytes(MAX_SIZE); + } catch (LargeObjectException e) { + raw = null; + } - String type; - if (mode == org.eclipse.jgit.lib.FileMode.SYMLINK) { - type = X_GIT_SYMLINK; - } else { - type = registry.getMimeType(path, raw).toString(); - type = resolveContentType(project, path, FileMode.FILE, type); - } + String type; + if (mode == org.eclipse.jgit.lib.FileMode.SYMLINK) { + type = X_GIT_SYMLINK; + } else { + type = registry.getMimeType(path, raw).toString(); + type = resolveContentType(project, path, FileMode.FILE, type); + } - return asBinaryResult(raw, obj).setContentType(type).base64(); + return asBinaryResult(raw, obj).setContentType(type).base64(); + } } } @@ -166,30 +165,30 @@ public class FileContentUtil { } commit = rw.parseCommit(commit.getParent(parent - 1)); } - ObjectReader reader = rw.getObjectReader(); - TreeWalk tw = TreeWalk.forPath(reader, path, commit.getTree()); - if (tw == null) { - throw new ResourceNotFoundException(); - } + try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, commit.getTree())) { + if (tw == null) { + throw new ResourceNotFoundException(); + } - int mode = tw.getFileMode(0).getObjectType(); - if (mode != Constants.OBJ_BLOB) { - throw new ResourceNotFoundException(); - } + int mode = tw.getFileMode(0).getObjectType(); + if (mode != Constants.OBJ_BLOB) { + throw new ResourceNotFoundException(); + } - ObjectId id = tw.getObjectId(0); - ObjectLoader obj = repo.open(id, OBJ_BLOB); - byte[] raw; - try { - raw = obj.getCachedBytes(MAX_SIZE); - } catch (LargeObjectException e) { - raw = null; - } + ObjectId id = tw.getObjectId(0); + ObjectLoader obj = repo.open(id, OBJ_BLOB); + byte[] raw; + try { + raw = obj.getCachedBytes(MAX_SIZE); + } catch (LargeObjectException e) { + raw = null; + } - MimeType contentType = registry.getMimeType(path, raw); - return registry.isSafeInline(contentType) - ? wrapBlob(path, obj, raw, contentType, suffix) - : zipBlob(path, obj, commit, suffix); + MimeType contentType = registry.getMimeType(path, raw); + return registry.isSafeInline(contentType) + ? wrapBlob(path, obj, raw, contentType, suffix) + : zipBlob(path, obj, commit, suffix); + } } } From 8f1ff9ace8442e6c70546430795938eb8156a1f8 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 11:09:06 +0200 Subject: [PATCH 35/48] VersionedMetaData: Open TreeWalk in try-with-resource Change-Id: I7aa07931fd89e5e16e2f5586745921ff0fc0ec8d --- .../gerrit/server/git/VersionedMetaData.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java index 92edc471c8..d1e3381907 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java @@ -461,10 +461,11 @@ public abstract class VersionedMetaData { return new byte[] {}; } - TreeWalk tw = TreeWalk.forPath(reader, fileName, revision.getTree()); - if (tw != null) { - ObjectLoader obj = reader.open(tw.getObjectId(0), Constants.OBJ_BLOB); - return obj.getCachedBytes(Integer.MAX_VALUE); + try (TreeWalk tw = TreeWalk.forPath(reader, fileName, revision.getTree())) { + if (tw != null) { + ObjectLoader obj = reader.open(tw.getObjectId(0), Constants.OBJ_BLOB); + return obj.getCachedBytes(Integer.MAX_VALUE); + } } return new byte[] {}; } @@ -474,9 +475,10 @@ public abstract class VersionedMetaData { return null; } - TreeWalk tw = TreeWalk.forPath(reader, fileName, revision.getTree()); - if (tw != null) { - return tw.getObjectId(0); + try (TreeWalk tw = TreeWalk.forPath(reader, fileName, revision.getTree())) { + if (tw != null) { + return tw.getObjectId(0); + } } return null; From ef335f6518790268f5c10ad522b432dd3fa1854d Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 26 Mar 2018 11:14:49 +0200 Subject: [PATCH 36/48] GetAuditLog: Fix NPE if group UUID cannot be resolved A group UUID cannot be resolved if there is no groupback that handles it. This can e.g. happen if the singleusergroup was used and single user groups have been added to Gerrit groups, but then the singleusergroup plugin was uninstalled. Change-Id: I87b83c3904172e8ff32cfd07c15e9a9951921c71 Signed-off-by: Edwin Kempin (cherry picked from commit 3e20db09ec088158a255eb6cd5d3d770a2ecfaee) --- .../main/java/com/google/gerrit/server/group/GetAuditLog.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java index e29b37f40e..09aad6c2ab 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java @@ -108,7 +108,9 @@ public class GetAuditLog implements RestReadView { GroupDescription.Basic groupDescription = groupBackend.get(includedGroupUUID); member = new GroupInfo(); member.id = Url.encode(includedGroupUUID.get()); - member.name = groupDescription.getName(); + if (groupDescription != null) { + member.name = groupDescription.getName(); + } } auditEvents.add( From 5cb9186480d0e42a38616e74ae256d5e03f338b4 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 26 Mar 2018 10:54:49 +0200 Subject: [PATCH 37/48] AccountGroupAuditLogScreen: Display group UUID if group name is missing The group name is missing if there is no group backend that can resolve the UUID. This can e.g. happen if the singleusergroup was used and single user groups have been added to Gerrit groups, but then the singleusergroup plugin was uninstalled so that the single user groups can no longer be resolved. Change-Id: I1bccec4bb38b55f6ad9b2d2ac83227b379d09071 Signed-off-by: Edwin Kempin (cherry picked from commit 92f58f80b1eb8f763266d96a9be0705a2535b178) --- .../client/admin/AccountGroupAuditLogScreen.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java index b056afa9e2..5e38a1414f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java @@ -113,17 +113,19 @@ public class AccountGroupAuditLogScreen extends AccountGroupScreen { GroupInfo member = auditEvent.memberAsGroup(); if (AccountGroup.isInternalGroup(member.getGroupUUID())) { table.setWidget( - row, 3, new Hyperlink(member.name(), Dispatcher.toGroup(member.getGroupUUID()))); + row, + 3, + new Hyperlink(formatGroup(member), Dispatcher.toGroup(member.getGroupUUID()))); fmt.getElement(row, 3).setTitle(null); } else if (member.url() != null) { Anchor a = new Anchor(); - a.setText(member.name()); + a.setText(formatGroup(member)); a.setHref(member.url()); a.setTitle("UUID " + member.getGroupUUID().get()); table.setWidget(row, 3, a); fmt.getElement(row, 3).setTitle(null); } else { - table.setText(row, 3, member.name()); + table.setText(row, 3, formatGroup(member)); fmt.getElement(row, 3).setTitle("UUID " + member.getGroupUUID().get()); } break; @@ -148,4 +150,10 @@ public class AccountGroupAuditLogScreen extends AccountGroupScreen { b.append(")"); return b.toString(); } + + private static String formatGroup(GroupInfo group) { + return group.name() != null && !group.name().isEmpty() + ? group.name() + : group.getGroupUUID().get(); + } } From a3cd22efee5ff9d7bf414695df60c75b5b1b1856 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 27 Mar 2018 09:24:54 +0200 Subject: [PATCH 38/48] Docs: Clarify that for external groups the name in GroupInfo can be missing For external groups the group name is missing if there is no group backend that can resolve the group UUID. E.g. this can happen when a plugin that provided a group backend was uninstalled (e.g. the singleusergroup plugin). Change-Id: I0d7f7253698c0125d7692781fa32f4294cda3bcc Signed-off-by: Edwin Kempin (cherry picked from commit 219e797144d7e6425373bd35c7fa1f48b8ac4abb) --- Documentation/rest-api-groups.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/rest-api-groups.txt b/Documentation/rest-api-groups.txt index 378c7f5380..61b746d8bd 100644 --- a/Documentation/rest-api-groups.txt +++ b/Documentation/rest-api-groups.txt @@ -1404,8 +1404,11 @@ a Gerrit internal group, or an external group that is known to Gerrit. |Field Name ||Description |`id` ||The URL encoded UUID of the group. |`name` | -not set if returned in a map where the group name is used as map key| -The name of the group. +optional, not set if returned in a map where the group name is used as map key| +The name of the group. + +For external groups the group name is missing if there is no group +backend that can resolve the group UUID. E.g. this can happen when a +plugin that provided a group backend was uninstalled. |`url` |optional| URL to information about the group. Typically a URL to a web page that permits users to apply to join the group, or manage their membership. From ebf9f4ef24fead0ab98e387996a674ce86b729aa Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 12:07:34 +0200 Subject: [PATCH 39/48] AbstractElasticIndex: Open XContentBuilder in try-with-resource Change-Id: I6f1de6573bf0c1e8932f96c4c4c1a819a955fbb1 --- .../elasticsearch/AbstractElasticIndex.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index a60c552c25..adb1687c47 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -161,20 +161,23 @@ abstract class AbstractElasticIndex implements Index { } private String toDoc(V v) throws IOException { - XContentBuilder builder = jsonBuilder().startObject(); - for (Values values : schema.buildFields(v, fillArgs)) { - String name = values.getField().getName(); - if (values.getField().isRepeatable()) { - builder.field( - name, - Streams.stream(values.getValues()).filter(e -> shouldAddElement(e)).collect(toList())); - } else { - Object element = Iterables.getOnlyElement(values.getValues(), ""); - if (shouldAddElement(element)) { - builder.field(name, element); + try (XContentBuilder builder = jsonBuilder().startObject()) { + for (Values values : schema.buildFields(v, fillArgs)) { + String name = values.getField().getName(); + if (values.getField().isRepeatable()) { + builder.field( + name, + Streams.stream(values.getValues()) + .filter(e -> shouldAddElement(e)) + .collect(toList())); + } else { + Object element = Iterables.getOnlyElement(values.getValues(), ""); + if (shouldAddElement(element)) { + builder.field(name, element); + } } } + return builder.endObject().string(); } - return builder.endObject().string(); } } From 789bb29b895050ccb5ff243649f2518ce5ee62e0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 13:56:18 +0200 Subject: [PATCH 40/48] Clarify behavior of ownerin: and reviewerin: predicates When the ownerin: or reviewerin: predicate is used in a query without any additional explicit index predicate, it will default to only include changes in status 'OPEN'. Clarify this in the documentation, and expand the tests to show this behavior. Bug: Issue 8597 Change-Id: Iddf66a44e0bcbc00f97466a997025abc3d82b90f --- Documentation/user-search.txt | 8 ++++++-- .../server/query/change/AbstractQueryChangesTest.java | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index 2902d58414..6806cca871 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -109,7 +109,9 @@ Changes originally submitted by 'USER'. The special case of [[ownerin]] ownerin:'GROUP':: + -Changes originally submitted by a user in 'GROUP'. +Changes originally submitted by a user in 'GROUP'. When no other index +predicate is explicitly added in the query, defaults to only include +changes in status 'OPEN'. [[query]] query:'NAME':: @@ -133,7 +135,9 @@ will find changes where the caller has been CC'ed. [[reviewerin]] reviewerin:'GROUP':: + -Changes that have been, or need to be, reviewed by a user in 'GROUP'. +Changes that have been, or need to be, reviewed by a user in 'GROUP'. When +no other index predicate is explicitly added in the query, defaults to only +include changes in status 'OPEN'. [[commit]] commit:'SHA1':: diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 0178cd3404..d2004f11da 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -513,6 +513,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("ownerin:Administrators", change1); assertQuery("ownerin:\"Registered Users\"", change2, change1); + assertQuery("ownerin:\"Registered Users\" project:repo", change3, change2, change1); assertQuery("ownerin:\"Registered Users\" status:merged", change3); } @@ -1749,6 +1750,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { gApi.changes().id(change2.getId().get()).current().submit(); assertQuery("reviewerin:" + group); + assertQuery("project:repo reviewerin:" + group, change2); assertQuery("status:merged reviewerin:" + group, change2); } From 703fc33df2ed04e774ceec1003dee1b71e4d46d9 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 14:24:33 +0200 Subject: [PATCH 41/48] Set 2.13.12-SNAPSHOT in pom.xml and plugin documentation Change-Id: Idcd310e9bd29b61eb0d1ee3f5bdf87053cf7a9cc --- Documentation/dev-plugins.txt | 2 +- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-archetype/pom.xml | 2 +- gerrit-plugin-gwt-archetype/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-plugin-js-archetype/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index 9a25738ffc..2922a4e8fe 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt @@ -36,7 +36,7 @@ plugin project. ---- mvn archetype:generate -DarchetypeGroupId=com.google.gerrit \ -DarchetypeArtifactId=gerrit-plugin-archetype \ - -DarchetypeVersion=2.13.11 \ + -DarchetypeVersion=2.13.12-SNAPSHOT \ -DgroupId=com.googlesource.gerrit.plugins.testplugin \ -DartifactId=testplugin ---- diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index 79e1b23c88..ad17e69f73 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.13.11 + 2.13.12-SNAPSHOT jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 30ecca431d..ff86f9effd 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.13.11 + 2.13.12-SNAPSHOT jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index abd709f766..c77f2c1667 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.13.11 + 2.13.12-SNAPSHOT jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-archetype/pom.xml b/gerrit-plugin-archetype/pom.xml index 0ce1b4da01..1ab3fa4d99 100644 --- a/gerrit-plugin-archetype/pom.xml +++ b/gerrit-plugin-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-archetype - 2.13.11 + 2.13.12-SNAPSHOT Gerrit Code Review - Plugin Archetype Maven Archetype for Gerrit Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-plugin-gwt-archetype/pom.xml b/gerrit-plugin-gwt-archetype/pom.xml index 393128cd67..005e8a33fb 100644 --- a/gerrit-plugin-gwt-archetype/pom.xml +++ b/gerrit-plugin-gwt-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-gwt-archetype - 2.13.11 + 2.13.12-SNAPSHOT Gerrit Code Review - Web UI GWT Plugin Archetype Maven Archetype for Gerrit Web UI GWT Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index 1278c97077..39a6eb6a3e 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.13.11 + 2.13.12-SNAPSHOT jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-plugin-js-archetype/pom.xml b/gerrit-plugin-js-archetype/pom.xml index 0540c84ad8..3613e06972 100644 --- a/gerrit-plugin-js-archetype/pom.xml +++ b/gerrit-plugin-js-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-js-archetype - 2.13.11 + 2.13.12-SNAPSHOT Gerrit Code Review - Web UI JavaScript Plugin Archetype Maven Archetype for Gerrit Web UI JavaScript Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index 16c9e165cd..3b57ef70c9 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.13.11 + 2.13.12-SNAPSHOT war Gerrit Code Review - WAR Gerrit WAR From 54c5385eca5d90d150d7925e69114fbc577fcf79 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 12:13:32 +0200 Subject: [PATCH 42/48] Open instances of JsonReader in try-with-resource Change-Id: I4efc14288951978594161841a695cd01f59d3454 --- .../gerrit/acceptance/edit/ChangeEditIT.java | 14 ++++++++------ .../acceptance/rest/change/ChangeReviewersIT.java | 7 ++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 4ca4498aad..814bd06c0e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -821,16 +821,18 @@ public class ChangeEditIT extends AbstractDaemonTest { private T readContentFromJson(RestResponse r, Class clazz) throws Exception { r.assertOK(); - JsonReader jsonReader = new JsonReader(r.getReader()); - jsonReader.setLenient(true); - return newGson().fromJson(jsonReader, clazz); + try (JsonReader jsonReader = new JsonReader(r.getReader())) { + jsonReader.setLenient(true); + return newGson().fromJson(jsonReader, clazz); + } } private T readContentFromJson(RestResponse r, TypeToken typeToken) throws Exception { r.assertOK(); - JsonReader jsonReader = new JsonReader(r.getReader()); - jsonReader.setLenient(true); - return newGson().fromJson(jsonReader, typeToken.getType()); + try (JsonReader jsonReader = new JsonReader(r.getReader())) { + jsonReader.setLenient(true); + return newGson().fromJson(jsonReader, typeToken.getType()); + } } private String readContentFromJson(RestResponse r) throws Exception { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java index f291876e34..76b7646905 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java @@ -830,9 +830,10 @@ public class ChangeReviewersIT extends AbstractDaemonTest { private static T readContentFromJson(RestResponse r, int expectedStatus, Class clazz) throws Exception { r.assertStatus(expectedStatus); - JsonReader jsonReader = new JsonReader(r.getReader()); - jsonReader.setLenient(true); - return newGson().fromJson(jsonReader, clazz); + try (JsonReader jsonReader = new JsonReader(r.getReader())) { + jsonReader.setLenient(true); + return newGson().fromJson(jsonReader, clazz); + } } private static void assertReviewers( From d7d7d0ec41093a6948828cea8ca95a59fc082746 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 12:17:54 +0200 Subject: [PATCH 43/48] SmtpEmailSender: Open Writer in try-with-resource Change-Id: I7f4195c9ec6b798aa3e455a01e3443a7b9b7c000 --- .../server/mail/send/SmtpEmailSender.java | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java index a8289c4dea..b08e5942ef 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java @@ -200,30 +200,31 @@ public class SmtpEmailSender implements EmailSender { } } - Writer messageDataWriter = client.sendMessageData(); - if (messageDataWriter == null) { - /* Include rejected recipient error messages here to not lose that - * information. That piece of the puzzle is vital if zero recipients - * are accepted and the server consequently rejects the DATA command. - */ - throw new EmailException( - rejected - + "Server " - + smtpHost - + " rejected DATA command: " - + client.getReplyString()); - } + try (Writer messageDataWriter = client.sendMessageData()) { + if (messageDataWriter == null) { + /* Include rejected recipient error messages here to not lose that + * information. That piece of the puzzle is vital if zero recipients + * are accepted and the server consequently rejects the DATA command. + */ + throw new EmailException( + rejected + + "Server " + + smtpHost + + " rejected DATA command: " + + client.getReplyString()); + } - render(messageDataWriter, callerHeaders, textBody, htmlBody); + render(messageDataWriter, callerHeaders, textBody, htmlBody); - if (!client.completePendingCommand()) { - throw new EmailException( - "Server " + smtpHost + " rejected message body: " + client.getReplyString()); - } + if (!client.completePendingCommand()) { + throw new EmailException( + "Server " + smtpHost + " rejected message body: " + client.getReplyString()); + } - client.logout(); - if (rejected.length() > 0) { - throw new EmailException(rejected.toString()); + client.logout(); + if (rejected.length() > 0) { + throw new EmailException(rejected.toString()); + } } } finally { client.disconnect(); From 6615bfd8fb863e86d59215cb1f0b8635750526bf Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 17 Apr 2018 15:25:30 +0200 Subject: [PATCH 44/48] Update .mailmap Change-Id: I33311369d05d5cd9d48e0c4889efb60d09a1cc1b --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index bd4d222cbd..cdfa2fa27e 100644 --- a/.mailmap +++ b/.mailmap @@ -65,6 +65,7 @@ Ulrik Sjölin Ulrik Sjölin Ulrik Sjolin Ulrik Sjölin Ulrik Sjölin Ulrik Sjölin Ulrik Sjolin +Viktar Donich viktard Yuxuan 'fishy' Wang Yuxuan Wang Zalán Blénessy Zalan Blenessy 飞 李 lifei From fa779ef35f022336a74fd10fc7d0cbfe9c38afcb Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 27 Jan 2018 16:11:21 +0100 Subject: [PATCH 45/48] setup_gjf.sh: Add support for multiple version We are in process of upgrading to new google-java-format version. To simplify upgrade process, allow to set up multiple different versions of gjf distribution. To install currently used version 1.3 no parameter is needed $ tools/setup-gjf.sh The resulting script has now version suffix: tools/format/google-java-format-1.3 To update to a newer version, version parameter can be passed now: $ tools/setup-gjf.sh 1.5 Note, that because of SHA1 check only supported versions can be currently installed: 1.3 and 1.5. Change-Id: Id2464cd88291f7f7376df98ca2a396e2458d07a4 --- tools/setup_gjf.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tools/setup_gjf.sh b/tools/setup_gjf.sh index 0cf2f3f726..b3ac143744 100755 --- a/tools/setup_gjf.sh +++ b/tools/setup_gjf.sh @@ -17,8 +17,20 @@ set -eu # Keep this version in sync with dev-contributing.txt. -VERSION="1.3" -SHA1="a73cfe6f9af01bd6ff150c0b50c9d620400f784c" +VERSION=${1:-1.3} + +case "$VERSION" in +1.3) + SHA1="a73cfe6f9af01bd6ff150c0b50c9d620400f784c" + ;; +1.5) + SHA1="b1f79e4d39a3c501f07c0ce7e8b03ac6964ed1f1" + ;; +*) + echo "unknown google-java-format version: $VERSION" + exit 1 + ;; +esac root="$(git rev-parse --show-toplevel)" if [[ -z "$root" ]]; then @@ -33,7 +45,7 @@ name="google-java-format-$VERSION-all-deps.jar" url="https://github.com/google/google-java-format/releases/download/google-java-format-$VERSION/$name" "$root/tools/download_file.py" -o "$dir/$name" -u "$url" -v "$SHA1" -launcher="$dir/google-java-format" +launcher="$dir/google-java-format-$VERSION" cat > "$launcher" < Date: Wed, 18 Apr 2018 08:25:27 +0200 Subject: [PATCH 46/48] Upgrade auto-value to 1.6 The annotations are now provided in a separate artifact. Change-Id: I6df14c97c44b211671961810b17760d81c20c5c3 --- WORKSPACE | 12 +++++++++-- java/com/google/gerrit/acceptance/BUILD | 1 + java/com/google/gerrit/common/BUILD | 2 ++ java/com/google/gerrit/httpd/BUILD | 1 + java/com/google/gerrit/index/BUILD | 1 + java/com/google/gerrit/pgm/BUILD | 1 + java/com/google/gerrit/server/BUILD | 1 + .../google/gerrit/server/git/receive/BUILD | 1 + java/com/google/gerrit/server/restapi/BUILD | 1 + java/com/google/gerrit/server/schema/BUILD | 1 + java/com/google/gerrit/sshd/BUILD | 1 + java/com/google/gerrit/testing/BUILD | 1 + javatests/com/google/gerrit/common/BUILD | 1 + javatests/com/google/gerrit/server/BUILD | 1 + lib/auto/BUILD | 21 +++++++++++++++++-- 15 files changed, 43 insertions(+), 4 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 6967c1f299..4d1f81b920 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -415,10 +415,18 @@ maven_jar( sha1 = "430b2fc839b5de1f3643b528853d5cf26096c1de", ) +AUTO_VALUE_VERSION = "1.6" + maven_jar( name = "auto_value", - artifact = "com.google.auto.value:auto-value:1.5.4", - sha1 = "65183ddd1e9542d69d8f613fdae91540d04e1476", + artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION, + sha1 = "a3b1b1404f8acaa88594a017185e013cd342c9a8", +) + +maven_jar( + name = "auto_value_annotations", + artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION, + sha1 = "da725083ee79fdcd86d9f3d8a76e38174a01892a", ) # Transitive dependency of commons-compress diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD index d89dc92a80..8a62491214 100644 --- a/java/com/google/gerrit/acceptance/BUILD +++ b/java/com/google/gerrit/acceptance/BUILD @@ -78,6 +78,7 @@ java_library2( "//lib:truth", "//lib:truth-java8-extension", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/httpcomponents:fluent-hc", "//lib/httpcomponents:httpclient", "//lib/httpcomponents:httpcore", diff --git a/java/com/google/gerrit/common/BUILD b/java/com/google/gerrit/common/BUILD index a09ef49cc9..2565f0d1d0 100644 --- a/java/com/google/gerrit/common/BUILD +++ b/java/com/google/gerrit/common/BUILD @@ -23,6 +23,7 @@ gwt_module( "//lib:gwtorm_client", "//lib:servlet-api-3_1", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/jgit/org.eclipse.jgit:jgit", "//lib/log:api", ], @@ -48,6 +49,7 @@ java_library( "//lib:gwtorm", "//lib:servlet-api-3_1", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/jgit/org.eclipse.jgit:jgit", "//lib/log:api", ], diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD index d7cbdb8002..b62c887a67 100644 --- a/java/com/google/gerrit/httpd/BUILD +++ b/java/com/google/gerrit/httpd/BUILD @@ -30,6 +30,7 @@ java_library( "//lib:servlet-api-3_1", "//lib:soy", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/commons:codec", "//lib/guice", "//lib/guice:guice-assistedinject", diff --git a/java/com/google/gerrit/index/BUILD b/java/com/google/gerrit/index/BUILD index 015eceb13b..f293b2d5c3 100644 --- a/java/com/google/gerrit/index/BUILD +++ b/java/com/google/gerrit/index/BUILD @@ -42,6 +42,7 @@ java_library( "//lib:gwtorm", "//lib/antlr:java_runtime", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/jgit/org.eclipse.jgit:jgit", "//lib/log:api", ], diff --git a/java/com/google/gerrit/pgm/BUILD b/java/com/google/gerrit/pgm/BUILD index a255020af2..b5b513fe11 100644 --- a/java/com/google/gerrit/pgm/BUILD +++ b/java/com/google/gerrit/pgm/BUILD @@ -49,6 +49,7 @@ java_library( "//lib:protobuf", "//lib:servlet-api-3_1-without-neverlink", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/guice", "//lib/guice:guice-assistedinject", "//lib/guice:guice-servlet", diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD index 94f5062f1e..70b3d68a2e 100644 --- a/java/com/google/gerrit/server/BUILD +++ b/java/com/google/gerrit/server/BUILD @@ -62,6 +62,7 @@ java_library( "//lib:soy", "//lib:tukaani-xz", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/bouncycastle:bcpkix-neverlink", "//lib/bouncycastle:bcprov-neverlink", "//lib/commons:codec", diff --git a/java/com/google/gerrit/server/git/receive/BUILD b/java/com/google/gerrit/server/git/receive/BUILD index 52b23728c7..4d24b4b7c8 100644 --- a/java/com/google/gerrit/server/git/receive/BUILD +++ b/java/com/google/gerrit/server/git/receive/BUILD @@ -13,6 +13,7 @@ java_library( "//lib:guava", "//lib:gwtorm", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/guice", "//lib/guice:guice-assistedinject", "//lib/jgit/org.eclipse.jgit:jgit", diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD index a307d4349c..6c2a50d6e0 100644 --- a/java/com/google/gerrit/server/restapi/BUILD +++ b/java/com/google/gerrit/server/restapi/BUILD @@ -25,6 +25,7 @@ java_library( "//lib:gwtorm", "//lib:servlet-api-3_1", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/commons:codec", "//lib/commons:compress", "//lib/commons:lang", diff --git a/java/com/google/gerrit/server/schema/BUILD b/java/com/google/gerrit/server/schema/BUILD index 2292234608..32a14dbbc0 100644 --- a/java/com/google/gerrit/server/schema/BUILD +++ b/java/com/google/gerrit/server/schema/BUILD @@ -16,6 +16,7 @@ java_library( "//lib:guava", "//lib:gwtorm", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/commons:dbcp", "//lib/guice", "//lib/jgit/org.eclipse.jgit.archive:jgit-archive", diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD index 3ed1f2fb44..c36d68b766 100644 --- a/java/com/google/gerrit/sshd/BUILD +++ b/java/com/google/gerrit/sshd/BUILD @@ -25,6 +25,7 @@ java_library( "//lib:jsch", "//lib:servlet-api-3_1", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/bouncycastle:bcprov-neverlink", "//lib/commons:codec", "//lib/dropwizard:dropwizard-core", diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD index 98468258a4..dd726a8af2 100644 --- a/java/com/google/gerrit/testing/BUILD +++ b/java/com/google/gerrit/testing/BUILD @@ -32,6 +32,7 @@ java_library( "//lib:h2", "//lib:truth", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/guice", "//lib/guice:guice-servlet", "//lib/jgit/org.eclipse.jgit:jgit", diff --git a/javatests/com/google/gerrit/common/BUILD b/javatests/com/google/gerrit/common/BUILD index d906284bdb..ff196466a8 100644 --- a/javatests/com/google/gerrit/common/BUILD +++ b/javatests/com/google/gerrit/common/BUILD @@ -30,5 +30,6 @@ junit_tests( "//lib:guava", "//lib:truth", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", ], ) diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD index 24d2822e41..2ddfaa9a09 100644 --- a/javatests/com/google/gerrit/server/BUILD +++ b/javatests/com/google/gerrit/server/BUILD @@ -55,6 +55,7 @@ junit_tests( "//lib:gwtorm", "//lib:truth-java8-extension", "//lib/auto:auto-value", + "//lib/auto:auto-value-annotations", "//lib/commons:codec", "//lib/guice", "//lib/jgit/org.eclipse.jgit:jgit", diff --git a/lib/auto/BUILD b/lib/auto/BUILD index 569398ea13..89adbde1ec 100644 --- a/lib/auto/BUILD +++ b/lib/auto/BUILD @@ -1,13 +1,19 @@ java_plugin( name = "auto-annotation-plugin", processor_class = "com.google.auto.value.processor.AutoAnnotationProcessor", - deps = ["@auto_value//jar"], + deps = [ + "@auto_value//jar", + "@auto_value_annotations//jar", + ], ) java_plugin( name = "auto-value-plugin", processor_class = "com.google.auto.value.processor.AutoValueProcessor", - deps = ["@auto_value//jar"], + deps = [ + "@auto_value//jar", + "@auto_value_annotations//jar", + ], ) java_library( @@ -20,3 +26,14 @@ java_library( visibility = ["//visibility:public"], exports = ["@auto_value//jar"], ) + +java_library( + name = "auto-value-annotations", + data = ["//lib:LICENSE-Apache2.0"], + exported_plugins = [ + ":auto-annotation-plugin", + ":auto-value-plugin", + ], + visibility = ["//visibility:public"], + exports = ["@auto_value_annotations//jar"], +) From c996fe2450150c75da59355694754eb1477d812e Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 18 Apr 2018 08:50:50 +0200 Subject: [PATCH 47/48] InitPluginStepsLoader: Remove redundant warning suppression Change-Id: I9310b58fc3e7595ffa73a7eaf9478651433522c5 --- java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java b/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java index 0fd9ebaeb7..a7f9c5d543 100644 --- a/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java +++ b/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java @@ -62,7 +62,6 @@ public class InitPluginStepsLoader { return pluginsInitSteps; } - @SuppressWarnings("resource") private InitStep loadInitStep(Path jar) { try { URLClassLoader pluginLoader = From 36c1d7201e0b9c5ae1d112cdb82d9de03d65cb1a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 18 Apr 2018 08:52:53 +0200 Subject: [PATCH 48/48] Update .mailmap Change-Id: I374e9081442deeda0f83253d28a70fa5ac06235d --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index b4e81e257a..4c71059dd0 100644 --- a/.mailmap +++ b/.mailmap @@ -10,6 +10,7 @@ Brad Larson Bruce Zu Bruce Zu Carlos Eduardo Baldacin carloseduardo.baldacin +Changcheng Xiao xchangcheng Dariusz Luksza Dave Borowitz David Ostrovsky