From e9223063849851c1f95f87c498e89feedf7ec65c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Thu, 5 Sep 2019 10:31:41 +0200 Subject: [PATCH 1/7] Allow admins to index a change even if the branch is not Readable for them The ChangesCollection rejects the request if the current user doesn't have the Read permission on the loaded change. Therefore, an attempt to reindex a change: PUT /changes/myProject~12345/index gets rejected even before it reaches the indexing code. On the other side, the admin can reindex all changes of that project and thus also index the change 12345: POST /projects/myProject/index.changes Indexing all changes of a project in order to just index one change is too much. Add a new REST endpoint where a set of change-ids to index is given in the request body. POST /config/server/index.changes Content-Type: application/json; charset=UTF-8 {changes: ["foo~101", "bar~202"]} Bug: issue 11205 Change-Id: I05fcc5a024c7634a969a7baac9fd61f3d0f3d12a --- Documentation/rest-api-config.txt | 33 +++++++ .../acceptance/ChangeIndexedCounter.java | 2 +- .../server/restapi/config/IndexChanges.java | 83 ++++++++++++++++ .../gerrit/server/restapi/config/Module.java | 1 + .../rest/ConfigRestApiBindingsIT.java | 3 +- .../rest/config/IndexChangesIT.java | 98 +++++++++++++++++++ 6 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 java/com/google/gerrit/server/restapi/config/IndexChanges.java create mode 100644 javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt index 3ec989ee1e..7be8c324b4 100644 --- a/Documentation/rest-api-config.txt +++ b/Documentation/rest-api-config.txt @@ -1376,6 +1376,28 @@ EditPreferencesInfo] is returned. } ---- +[[index.changes]] +=== Index a set of changes + +This endpoint allows Gerrit admins to index a set of changes with one request +by providing a link:#index-changes-input[IndexChangesInput] entity. + +Using this endpoint Gerrit admins can also index change(s) which are not visible to them. + +.Request +---- + POST /config/server/index.changes HTTP/1.0 + Content-Type: application/json; charset=UTF-8 + + {changes: ["foo~101", "bar~202"]} +---- + +.Response +---- + HTTP/1.1 200 OK + Content-Disposition: attachment +---- + [[ids]] == IDs @@ -1825,6 +1847,17 @@ Hit ratio for cache entries that are held on disk (0 \<= value \<= 100). Only set for disk caches. |================================== +[[index-changes-input]] +=== IndexChangesInput +The `IndexChangesInput` contains a list of numerical changes IDs to index. + +[options="header",cols="1,^2,4"] +|================================ +|Field Name ||Description +|`changes` || +List of link:rest-api-changes.html#change-id[change-ids] +|================================ + [[jvm-summary-info]] === JvmSummaryInfo The `JvmSummaryInfo` entity contains information about the JVM. diff --git a/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java b/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java index 286b045047..27ed603cae 100644 --- a/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java +++ b/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java @@ -37,7 +37,7 @@ public class ChangeIndexedCounter implements ChangeIndexedListener { countsByChange.clear(); } - long getCount(ChangeInfo info) { + public long getCount(ChangeInfo info) { return countsByChange.get(info._number); } diff --git a/java/com/google/gerrit/server/restapi/config/IndexChanges.java b/java/com/google/gerrit/server/restapi/config/IndexChanges.java new file mode 100644 index 0000000000..b178118368 --- /dev/null +++ b/java/com/google/gerrit/server/restapi/config/IndexChanges.java @@ -0,0 +1,83 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.restapi.config; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.data.GlobalCapability; +import com.google.gerrit.extensions.annotations.RequiresCapability; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.change.ChangeFinder; +import com.google.gerrit.server.config.ConfigResource; +import com.google.gerrit.server.index.change.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.restapi.config.IndexChanges.Input; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.SchemaFactory; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.io.IOException; +import java.util.Set; + +@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER) +@Singleton +public class IndexChanges implements RestModifyView { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + public static class Input { + public Set changes; + } + + private final ChangeFinder changeFinder; + private final SchemaFactory schemaFactory; + private final ChangeData.Factory changeDataFactory; + private final ChangeIndexer indexer; + + @Inject + IndexChanges( + ChangeFinder changeFinder, + SchemaFactory schemaFactory, + ChangeData.Factory changeDataFactory, + ChangeIndexer indexer) { + this.changeFinder = changeFinder; + this.schemaFactory = schemaFactory; + this.changeDataFactory = changeDataFactory; + this.indexer = indexer; + } + + @Override + public Object apply(ConfigResource resource, Input input) throws OrmException { + if (input == null || input.changes == null) { + return Response.ok("Nothing to index"); + } + + try (ReviewDb db = schemaFactory.open()) { + for (String id : input.changes) { + for (ChangeNotes n : changeFinder.find(id)) { + try { + indexer.index(changeDataFactory.create(db, n)); + logger.atFine().log("Indexed change %s", id); + } catch (IOException e) { + logger.atSevere().withCause(e).log("Failed to index change %s", id); + } + } + } + } + + return Response.ok("Indexed changes " + input.changes); + } +} diff --git a/java/com/google/gerrit/server/restapi/config/Module.java b/java/com/google/gerrit/server/restapi/config/Module.java index c4a6f56e44..78a7f481c7 100644 --- a/java/com/google/gerrit/server/restapi/config/Module.java +++ b/java/com/google/gerrit/server/restapi/config/Module.java @@ -37,6 +37,7 @@ public class Module extends RestApiModule { get(CONFIG_KIND, "version").to(GetVersion.class); get(CONFIG_KIND, "info").to(GetServerInfo.class); post(CONFIG_KIND, "check.consistency").to(CheckConsistency.class); + post(CONFIG_KIND, "index.changes").to(IndexChanges.class); post(CONFIG_KIND, "reload").to(ReloadConfig.class); get(CONFIG_KIND, "preferences").to(GetPreferences.class); put(CONFIG_KIND, "preferences").to(SetPreferences.class); diff --git a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java index 4a2c81b6aa..2e5ea00553 100644 --- a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java @@ -57,7 +57,8 @@ public class ConfigRestApiBindingsIT extends AbstractRestApiBindingsTest { RestCall.get("/config/server/capabilities"), RestCall.get("/config/server/caches"), RestCall.post("/config/server/caches"), - RestCall.get("/config/server/tasks")); + RestCall.get("/config/server/tasks"), + RestCall.post("/config/server/index.changes")); /** * Cache REST endpoints to be tested, the URLs contain a placeholder for the cache identifier. diff --git a/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java new file mode 100644 index 0000000000..fa35d19813 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java @@ -0,0 +1,98 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.rest.config; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.ChangeIndexedCounter; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.events.ChangeIndexedListener; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.extensions.registration.RegistrationHandle; +import com.google.gerrit.server.restapi.config.IndexChanges; +import com.google.inject.Inject; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class IndexChangesIT extends AbstractDaemonTest { + + @Inject private DynamicSet changeIndexedListeners; + + private ChangeIndexedCounter changeIndexedCounter; + private RegistrationHandle changeIndexedCounterHandle; + + @Before + public void addChangeIndexedCounter() { + changeIndexedCounter = new ChangeIndexedCounter(); + changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter); + } + + @After + public void removeChangeIndexedCounter() { + if (changeIndexedCounterHandle != null) { + changeIndexedCounterHandle.remove(); + } + } + + @Test + public void indexRequestFromNonAdminRejected() throws Exception { + String changeId = createChange().getChangeId(); + IndexChanges.Input in = new IndexChanges.Input(); + in.changes = ImmutableSet.of(changeId); + changeIndexedCounter.clear(); + userRestSession.post("/config/server/index.changes", in).assertForbidden(); + assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(0); + } + + @Test + public void indexVisibleChange() throws Exception { + String changeId = createChange().getChangeId(); + IndexChanges.Input in = new IndexChanges.Input(); + in.changes = ImmutableSet.of(changeId); + changeIndexedCounter.clear(); + adminRestSession.post("/config/server/index.changes", in).assertOK(); + assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1); + } + + @Test + public void indexNonVisibleChange() throws Exception { + String changeId = createChange().getChangeId(); + ChangeInfo changeInfo = info(changeId); + blockRead("refs/heads/master"); + IndexChanges.Input in = new IndexChanges.Input(); + changeIndexedCounter.clear(); + in.changes = ImmutableSet.of(changeId); + adminRestSession.post("/config/server/index.changes", in).assertOK(); + assertThat(changeIndexedCounter.getCount(changeInfo)).isEqualTo(1); + } + + @Test + public void indexMultipleChanges() throws Exception { + ImmutableSet.Builder changeIds = ImmutableSet.builder(); + for (int i = 0; i < 10; i++) { + changeIds.add(createChange().getChangeId()); + } + IndexChanges.Input in = new IndexChanges.Input(); + in.changes = changeIds.build(); + changeIndexedCounter.clear(); + adminRestSession.post("/config/server/index.changes", in).assertOK(); + for (String changeId : in.changes) { + assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1); + } + } +} From 212b8bfb5dc099f514b41d1e2608e7738b1a23f1 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sat, 14 Sep 2019 09:47:18 +0100 Subject: [PATCH 2/7] Update git submodules * Update plugins/singleusergroup from branch 'stable-2.15' to 7b1ed0b747ce4ffac97f7786173c210d2a429aea - Test all-numeric username Verify that a username with all numbers works as expected and generates an equivalent group associated with it. Change-Id: Ie98181fdecbf196c320b8b9d0ca0c7bd88b0d238 --- plugins/singleusergroup | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/singleusergroup b/plugins/singleusergroup index 0ffbdf1e5c..7b1ed0b747 160000 --- a/plugins/singleusergroup +++ b/plugins/singleusergroup @@ -1 +1 @@ -Subproject commit 0ffbdf1e5c3daa9d118ee9f53d5c00497cff6454 +Subproject commit 7b1ed0b747ce4ffac97f7786173c210d2a429aea From d22f04aabefece2c5e8026c8777f5e6302ff6a47 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sun, 15 Sep 2019 11:10:04 +0100 Subject: [PATCH 3/7] Update git submodules * Update plugins/singleusergroup from branch 'stable-2.16' to 8a81e853a07b1bbd28766793e31d44787dfde1f6 - Fix the testAllNumericUserGroup all-numeric username Bug: Issue 11498 Change-Id: Iee6dc050f8d1e97920c3e105af7bd2a69479dcea - Merge branch 'stable-2.15' into stable-2.16 * stable-2.15: Test all-numeric username Change-Id: Ie659c4e19b9258dcb5b4ada96305d47b591cd5b7 - Test all-numeric username Verify that a username with all numbers works as expected and generates an equivalent group associated with it. Change-Id: Ie98181fdecbf196c320b8b9d0ca0c7bd88b0d238 --- plugins/singleusergroup | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/singleusergroup b/plugins/singleusergroup index ae9a9ab370..8a81e853a0 160000 --- a/plugins/singleusergroup +++ b/plugins/singleusergroup @@ -1 +1 @@ -Subproject commit ae9a9ab370d8026ad809a73b4dae3fb7d79ad8a4 +Subproject commit 8a81e853a07b1bbd28766793e31d44787dfde1f6 From d36fa16e29aab12321747fbab00ed4fb9c25a8e4 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sun, 15 Sep 2019 10:57:09 +0900 Subject: [PATCH 4/7] Remove obsolete permissions from gr-access-behavior The 'deleteDrafts' and 'viewDrafts' permissions do not exist any more; they were removed when the draft status was replaced with Work-in-Progress and private change states. Change-Id: I7f5f94229ba691151d39c6381daf5119f55a2098 --- .../behaviors/gr-access-behavior/gr-access-behavior.html | 8 -------- 1 file changed, 8 deletions(-) diff --git a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html index 4e67fda341..8c37b17ac6 100644 --- a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html +++ b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html @@ -51,10 +51,6 @@ limitations under the License. id: 'delete', name: 'Delete Reference', }, - deleteDrafts: { - id: 'deleteDrafts', - name: 'Delete Drafts', - }, deleteOwnChanges: { id: 'deleteOwnChanges', name: 'Delete Own Changes', @@ -119,10 +115,6 @@ limitations under the License. id: 'submitAs', name: 'Submit (On Behalf Of)', }, - viewDrafts: { - id: 'viewDrafts', - name: 'View Drafts', - }, viewPrivateChanges: { id: 'viewPrivateChanges', name: 'View Private Changes', From 0c4e3dc1ca00dbeea5910b409c640ac3e248e0e5 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Fri, 6 Sep 2019 12:28:19 +0200 Subject: [PATCH 5/7] Add deleteChanges to list of supported permissions Change-Id: Ia121dd63b1087b195eab2b123aada837c5300e58 --- .../app/behaviors/gr-access-behavior/gr-access-behavior.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html index 8c37b17ac6..ff5b96e04a 100644 --- a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html +++ b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html @@ -51,6 +51,10 @@ limitations under the License. id: 'delete', name: 'Delete Reference', }, + deleteChanges: { + id: 'deleteChanges', + name: 'Delete Changes', + }, deleteOwnChanges: { id: 'deleteOwnChanges', name: 'Delete Own Changes', From 40214e02bf132a71e4efbbe7002a75616f56aa68 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sun, 15 Sep 2019 15:24:18 +0900 Subject: [PATCH 6/7] Upgrade elasticsearch-rest-client to 7.3.2 Also update the test container to use 7.3.2 for V7_3 tests. Change-Id: I2a32d2c5f0bd3b84c1b0e3da81407e40833d3e09 --- WORKSPACE | 4 ++-- .../com/google/gerrit/elasticsearch/ElasticContainer.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 2d0d1acaa7..fff3bfb66d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -942,8 +942,8 @@ maven_jar( # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.3.1", - sha1 = "f5793c89b50a159cbb3e15e17bb981ff854cbe51", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.3.2", + sha1 = "38721e908cad8a30fa3f8e659c0571150a60cab3", ) maven_jar( diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticContainer.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticContainer.java index 7739125d54..265c61ccf3 100644 --- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -59,7 +59,7 @@ public class ElasticContainer extends ElasticsearchContainer { case V7_2: return "blacktop/elasticsearch:7.2.1"; case V7_3: - return "blacktop/elasticsearch:7.3.1"; + return "blacktop/elasticsearch:7.3.2"; } throw new IllegalStateException("No tests for version: " + version.name()); } From d9ade193ce4fb76c9942933eacf8fa6f67f15069 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 16 Sep 2019 16:52:39 +0200 Subject: [PATCH 7/7] Switch bazel version to 0.29.1 We previously updated to 1.0.0rc2 which isn't released yet to avoid a security issue. The security issue has been cherry-picked back to 0.29 and released in 0.29.1: https://github.com/bazelbuild/bazel/commit/1b771ec1fc8f859c64c3c14a57c53bf0aa2ecb51 Since 0.29.1 is released, update our requirement to that, so that people can continue to use released things. Change-Id: Ib48fdbb61bc94a5e78e81e485a5e1d550445a5a5 --- .bazelversion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bazelversion b/.bazelversion index 8862dbae5d..25939d35c7 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -1.0.0rc2 +0.29.1