Merge "Allow plugins to declare project permissions"
diff --git a/Documentation/dev-intellij.txt b/Documentation/dev-intellij.txt
index ca47690..b87cbf4 100644
--- a/Documentation/dev-intellij.txt
+++ b/Documentation/dev-intellij.txt
@@ -107,7 +107,9 @@
=== Copyright
Copy the folder `$(gerrit_source_code)/tools/intellij/copyright` (not just the
contents) to `$(project_data_directory)/.idea`. If it already exists, replace
-it.
+it. Then go to *File -> Settings -> Editor -> Copyright -> Copyright Profiles*,
+and import `Gerrit_Copyright.xml` to IntelliJ in case it doesn't pick the
+copyright up automatically.
=== File header
By default, IntelliJ adds a file header containing the name of the author and
diff --git a/Documentation/pg-plugin-endpoints.txt b/Documentation/pg-plugin-endpoints.txt
index cb1041f8..cb80c74 100644
--- a/Documentation/pg-plugin-endpoints.txt
+++ b/Documentation/pg-plugin-endpoints.txt
@@ -149,11 +149,44 @@
=== change-list-header
The `change-list-header` extension point adds a header to the change list view.
-
=== change-list-item-cell
The `change-list-item-cell` extension point adds a cell to the change list item.
+In addition to default parameters, the following are available:
+
* `change`
+
current change of the row, an instance of
link:rest-api-changes.html#change-info[ChangeInfo]
+
+=== change-view-tab-header
+The `change-view-tab-header` extension point adds a primary tab to the change
+view. This must be used in conjunction with `change-view-tab-content`.
+
+In addition to default parameters, the following are available:
+
+* `change`
++
+current change displayed, an instance of
+link:rest-api-changes.html#change-info[ChangeInfo]
+
+* `revision`
++
+current revision displayed, an instance of
+link:rest-api-changes.html#revision-info[RevisionInfo]
+
+=== change-view-tab-content
+The `change-view-tab-content` extension point adds primary tab content to
+the change view. This must be used in conjunction with `change-view-tab-header`.
+
+In addition to default parameters, the following are available:
+
+* `change`
++
+current change displayed, an instance of
+link:rest-api-changes.html#change-info[ChangeInfo]
+
+* `revision`
++
+current revision displayed, an instance of
+link:rest-api-changes.html#revision-info[RevisionInfo]
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 2b00d7c..d6dca48 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -392,8 +392,12 @@
if (isRead(req)) {
viewData = new ViewData(null, c.list());
} else if (isPost(req)) {
+ // TODO: Here and on other collection methods: There is a bug that binds child views
+ // with pluginName="gerrit" instead of the real plugin name. This has never worked
+ // correctly and should be fixed where the binding gets created (DynamicMapProvider)
+ // and here.
RestView<RestResource> restCollectionView =
- c.views().get(viewData.pluginName, "POST_ON_COLLECTION./");
+ c.views().get(PluginName.GERRIT, "POST_ON_COLLECTION./");
if (restCollectionView != null) {
viewData = new ViewData(null, restCollectionView);
} else {
@@ -401,7 +405,7 @@
}
} else if (isDelete(req)) {
RestView<RestResource> restCollectionView =
- c.views().get(viewData.pluginName, "DELETE_ON_COLLECTION./");
+ c.views().get(PluginName.GERRIT, "DELETE_ON_COLLECTION./");
if (restCollectionView != null) {
viewData = new ViewData(null, restCollectionView);
} else {
@@ -1264,6 +1268,8 @@
return new ViewData(PluginName.GERRIT, core);
}
+ // Check if we want to delegate to a child collection. Child collections are bound with
+ // GET.name so we have to check for this since we haven't found any other views.
core = views.get(PluginName.GERRIT, "GET." + p.get(0));
if (core != null) {
return new ViewData(PluginName.GERRIT, core);
@@ -1277,6 +1283,17 @@
}
}
+ if (r.isEmpty()) {
+ // Check if we want to delegate to a child collection. Child collections are bound with
+ // GET.name so we have to check for this since we haven't found any other views.
+ for (String plugin : views.plugins()) {
+ RestView<RestResource> action = views.get(plugin, "GET." + p.get(0));
+ if (action != null) {
+ r.put(plugin, action);
+ }
+ }
+ }
+
if (r.size() == 1) {
Map.Entry<String, RestView<RestResource>> entry = Iterables.getOnlyElement(r.entrySet());
return new ViewData(entry.getKey(), entry.getValue());
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index f9fb251..f0077cd 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -351,6 +351,7 @@
} else {
List<Change.Id> autoclosed = resultChangeIds.get(ResultChangeIds.Key.AUTOCLOSED);
metrics.changes.record(ResultChangeIds.Key.AUTOCLOSED, autoclosed.size());
+ totalChanges += autoclosed.size();
}
if (totalChanges > 0) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
index 1eb2770..7428e3a 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
@@ -17,9 +17,22 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.IndexPredicate;
import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.Predicate;
public abstract class ChangeIndexPredicate extends IndexPredicate<ChangeData>
implements Matchable<ChangeData> {
+ /**
+ * Returns an index predicate that matches no changes in the index.
+ *
+ * <p>This predicate should be used in preference to a non-index predicate (such as {@code
+ * Predicate.not(Predicate.any())}), since it can be matched efficiently against the index.
+ *
+ * @return an index predicate matching no changes.
+ */
+ public static Predicate<ChangeData> none() {
+ return ChangeStatusPredicate.NONE;
+ }
+
protected ChangeIndexPredicate(FieldDef<ChangeData, ?> def, String value) {
super(def, value);
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
index 8dc17d3..5a7efb8 100644
--- a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
@@ -41,7 +41,7 @@
*/
public final class ChangeStatusPredicate extends ChangeIndexPredicate {
private static final String INVALID_STATUS = "__invalid__";
- private static final Predicate<ChangeData> NONE = new ChangeStatusPredicate(null);
+ static final Predicate<ChangeData> NONE = new ChangeStatusPredicate(null);
private static final TreeMap<String, Predicate<ChangeData>> PREDICATES;
private static final Predicate<ChangeData> CLOSED;
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 3fc512a..ada9f27 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -19,7 +19,6 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import com.google.common.flogger.FluentLogger;
-import com.google.common.flogger.LoggingApi;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.index.query.Predicate;
@@ -42,8 +41,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
-import java.util.function.BiConsumer;
-import java.util.function.Supplier;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -65,13 +62,12 @@
cd = args.changeDataFactory.create(c);
files = cd.currentFilePaths();
} catch (IOException | OrmException e) {
- logWithOccasionalStackTrace(
- logger::atWarning,
+ warnWithOccasionalStackTrace(
e,
"Error constructing conflicts predicates for change %s in %s",
c.getId(),
c.getProject());
- return Predicate.not(Predicate.any());
+ return ChangeIndexPredicate.none();
}
if (3 + files.size() > args.indexConfig.maxTerms()) {
@@ -164,8 +160,7 @@
}
} catch (IntegrationException | NoSuchProjectException | OrmException | IOException e) {
ObjectId finalOther = other;
- logWithOccasionalStackTrace(
- logger::atWarning,
+ warnWithOccasionalStackTrace(
e,
"Merge failure checking conflicts of change %s in %s (%s): %s",
id,
@@ -235,12 +230,12 @@
}
}
- private static <API extends LoggingApi<API>> void logWithOccasionalStackTrace(
- Supplier<LoggingApi<API>> logSupplier, Throwable cause, String format, Object... args) {
- BiConsumer<LoggingApi<API>, String> logConsumer = (api, f) -> api.logVarargs(f, args);
- logConsumer.accept(logSupplier.get(), format);
- logConsumer.accept(
- logSupplier.get().withCause(cause).atMostEvery(1, MINUTES),
- "(Re-logging with stack trace) " + format);
+ private static void warnWithOccasionalStackTrace(Throwable cause, String format, Object... args) {
+ logger.atWarning().logVarargs(format, args);
+ logger
+ .atWarning()
+ .withCause(cause)
+ .atMostEvery(1, MINUTES)
+ .logVarargs("(Re-logging with stack trace) " + format, args);
}
}
diff --git a/java/com/google/gerrit/server/util/CommitMessageUtil.java b/java/com/google/gerrit/server/util/CommitMessageUtil.java
index fa55597..e984f46 100644
--- a/java/com/google/gerrit/server/util/CommitMessageUtil.java
+++ b/java/com/google/gerrit/server/util/CommitMessageUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.util;
import com.google.common.base.Strings;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.BadRequestException;
/** Utility functions to manipulate commit messages. */
@@ -23,18 +24,22 @@
private CommitMessageUtil() {}
/**
- * Checks for null or empty commit messages and appends a newline character to the commit message.
+ * Checks for invalid (empty or containing \0) commit messages and appends a newline character to
+ * the commit message.
*
* @throws BadRequestException if the commit message is null or empty
* @returns the trimmed message with a trailing newline character
*/
- public static String checkAndSanitizeCommitMessage(String commitMessage)
+ public static String checkAndSanitizeCommitMessage(@Nullable String commitMessage)
throws BadRequestException {
- String wellFormedMessage = Strings.nullToEmpty(commitMessage).trim();
- if (wellFormedMessage.isEmpty()) {
+ String trimmed = Strings.nullToEmpty(commitMessage).trim();
+ if (trimmed.isEmpty()) {
throw new BadRequestException("Commit message cannot be null or empty");
}
- wellFormedMessage = wellFormedMessage + "\n";
- return wellFormedMessage;
+ if (trimmed.indexOf(0) >= 0) {
+ throw new BadRequestException("Commit message cannot have NUL character");
+ }
+ trimmed = trimmed + "\n";
+ return trimmed;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d90c4bd..d3e41f7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3533,6 +3533,18 @@
}
@Test
+ public void changeCommitMessageNullNotAllowed() throws Exception {
+ PushOneCommit.Result r = createChange();
+ assertThat(getCommitMessage(r.getChangeId()))
+ .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("NUL character");
+ gApi.changes()
+ .id(r.getChangeId())
+ .setMessage("test\0commit\n\nChange-Id: " + r.getChangeId() + "\n");
+ }
+
+ @Test
public void changeCommitMessageWithWrongChangeIdFails() throws Exception {
PushOneCommit.Result otherChange = createChange();
PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
new file mode 100644
index 0000000..27df565
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
@@ -0,0 +1,160 @@
+// 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.binding;
+
+import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.rest.util.RestApiCallHelper;
+import com.google.gerrit.acceptance.rest.util.RestCall;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.ChildCollection;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestApiModule;
+import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import org.junit.Test;
+
+/**
+ * Tests for checking plugin-provided REST API bindings nested under a core collection.
+ *
+ * <p>These tests only verify that the plugin-provided REST endpoints are correctly bound, they do
+ * not test the functionality of the plugin REST endpoints.
+ */
+public class PluginProvidedChildRestApiBindingsIT extends AbstractDaemonTest {
+
+ /** Resource to bind a child collection. */
+ public static final TypeLiteral<RestView<TestPluginResource>> TEST_KIND =
+ new TypeLiteral<RestView<TestPluginResource>>() {};
+
+ private static final String PLUGIN_NAME = "my-plugin";
+
+ private static final ImmutableSet<RestCall> TEST_CALLS =
+ ImmutableSet.of(
+ // Calls that have the plugin name as part of the collection name
+ RestCall.get("/changes/%s/revisions/%s/" + PLUGIN_NAME + "~test-collection/"),
+ RestCall.get("/changes/%s/revisions/%s/" + PLUGIN_NAME + "~test-collection/1/detail"),
+ RestCall.post("/changes/%s/revisions/%s/" + PLUGIN_NAME + "~test-collection/"),
+ RestCall.post("/changes/%s/revisions/%s/" + PLUGIN_NAME + "~test-collection/1/update"),
+ // Same tests but without the plugin name as part of the collection name. This works as
+ // long as there is no core collection with the same name (which takes precedence) and no
+ // other plugin binds a collection with the same name. We highly encourage plugin authors
+ // to use the fully qualified collection name instead.
+ RestCall.get("/changes/%s/revisions/%s/test-collection/"),
+ RestCall.get("/changes/%s/revisions/%s/test-collection/1/detail"),
+ RestCall.post("/changes/%s/revisions/%s/test-collection/"),
+ RestCall.post("/changes/%s/revisions/%s/test-collection/1/update"));
+
+ /**
+ * Module for all sys bindings.
+ *
+ * <p>TODO: This should actually just move into MyPluginHttpModule. However, that doesn't work
+ * currently. This TODO is for fixing this bug.
+ */
+ static class MyPluginSysModule extends AbstractModule {
+ @Override
+ public void configure() {
+ install(
+ new RestApiModule() {
+ @Override
+ public void configure() {
+ DynamicMap.mapOf(binder(), TEST_KIND);
+ child(REVISION_KIND, "test-collection").to(TestChildCollection.class);
+
+ postOnCollection(TEST_KIND).to(TestPostOnCollection.class);
+ post(TEST_KIND, "update").to(TestPost.class);
+ get(TEST_KIND, "detail").to(TestGet.class);
+ }
+ });
+ }
+ }
+
+ static class TestPluginResource implements RestResource {}
+
+ @Singleton
+ static class TestChildCollection
+ implements ChildCollection<RevisionResource, TestPluginResource> {
+ private final DynamicMap<RestView<TestPluginResource>> views;
+
+ @Inject
+ TestChildCollection(DynamicMap<RestView<TestPluginResource>> views) {
+ this.views = views;
+ }
+
+ @Override
+ public RestView<RevisionResource> list() throws RestApiException {
+ return (RestReadView<RevisionResource>) resource -> ImmutableList.of("one", "two");
+ }
+
+ @Override
+ public TestPluginResource parse(RevisionResource parent, IdString id) throws Exception {
+ return new TestPluginResource();
+ }
+
+ @Override
+ public DynamicMap<RestView<TestPluginResource>> views() {
+ return views;
+ }
+ }
+
+ @Singleton
+ static class TestPostOnCollection
+ implements RestCollectionModifyView<RevisionResource, TestPluginResource, String> {
+ @Override
+ public Object apply(RevisionResource parentResource, String input) throws Exception {
+ return "test";
+ }
+ }
+
+ @Singleton
+ static class TestPost implements RestModifyView<TestPluginResource, String> {
+ @Override
+ public String apply(TestPluginResource resource, String input) throws Exception {
+ return "test";
+ }
+ }
+
+ @Singleton
+ static class TestGet implements RestReadView<TestPluginResource> {
+ @Override
+ public String apply(TestPluginResource resource) throws Exception {
+ return "test";
+ }
+ }
+
+ @Test
+ public void testEndpoints() throws Exception {
+ PatchSet.Id patchSetId = createChange().getPatchSetId();
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class, null, null)) {
+ RestApiCallHelper.execute(
+ adminRestSession,
+ TEST_CALLS.asList(),
+ String.valueOf(patchSetId.changeId.id),
+ String.valueOf(patchSetId.patchSetId));
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRestApiBindingsIT.java
deleted file mode 100644
index 82c065a..0000000
--- a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRestApiBindingsIT.java
+++ /dev/null
@@ -1,75 +0,0 @@
-// 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.binding;
-
-import static javax.servlet.http.HttpServletResponse.SC_OK;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.rest.util.RestApiCallHelper;
-import com.google.gerrit.acceptance.rest.util.RestCall;
-import com.google.inject.Singleton;
-import com.google.inject.servlet.ServletModule;
-import java.io.IOException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.junit.Test;
-
-/**
- * Tests for checking plugin-provided REST API bindings.
- *
- * <p>These tests only verify that the plugin-provided REST endpoints are correctly bound, they do
- * not test the functionality of the plugin REST endpoints.
- */
-public class PluginProvidedRestApiBindingsIT extends AbstractDaemonTest {
-
- /**
- * Plugin REST endpoints bound by {@link MyPluginModule} with Guice serlvet definitions.
- *
- * <p>Each URL contains a placeholder for the plugin identifier.
- *
- * <p>Currently does not include any resource or documentation URLs, since those would require
- * installing a plugin from a jar, which is trickier than just defining a module in this file.
- */
- private static final ImmutableList<RestCall> SERVER_TOP_LEVEL_PLUGIN_ENDPOINTS =
- ImmutableList.of(RestCall.get("/plugins/%s/hello"));
-
- static class MyPluginModule extends ServletModule {
- @Override
- public void configureServlets() {
- serve("/hello").with(HelloServlet.class);
- }
- }
-
- @Singleton
- static class HelloServlet extends HttpServlet {
- private static final long serialVersionUID = 1L;
-
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException {
- res.setStatus(SC_OK);
- res.getWriter().println("Hello world");
- }
- }
-
- @Test
- public void serverPluginTopLevelEndpoints() throws Exception {
- String pluginName = "my-plugin";
- try (AutoCloseable ignored = installPlugin(pluginName, null, MyPluginModule.class, null)) {
- RestApiCallHelper.execute(adminRestSession, SERVER_TOP_LEVEL_PLUGIN_ENDPOINTS, pluginName);
- }
- }
-}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java
new file mode 100644
index 0000000..8a8ea9b
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java
@@ -0,0 +1,199 @@
+// 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.binding;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.rest.util.RestApiCallHelper;
+import com.google.gerrit.acceptance.rest.util.RestCall;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.ChildCollection;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestApiModule;
+import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.httpd.restapi.RestApiServlet;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import com.google.inject.servlet.ServletModule;
+import java.io.IOException;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.Test;
+
+/**
+ * Tests for checking plugin-provided REST API bindings directly under {@code /}.
+ *
+ * <p>These tests only verify that the plugin-provided REST endpoints are correctly bound, they do
+ * not test the functionality of the plugin REST endpoints.
+ */
+public class PluginProvidedRootRestApiBindingsIT extends AbstractDaemonTest {
+
+ /** Resource to bind a child collection. */
+ public static final TypeLiteral<RestView<TestPluginResource>> TEST_KIND =
+ new TypeLiteral<RestView<TestPluginResource>>() {};
+
+ private static final String PLUGIN_NAME = "my-plugin";
+
+ private static final ImmutableSet<RestCall> TEST_CALLS =
+ ImmutableSet.of(
+ RestCall.get("/plugins/" + PLUGIN_NAME + "/test-collection/"),
+ RestCall.get("/plugins/" + PLUGIN_NAME + "/test-collection/1/detail"),
+ RestCall.post("/plugins/" + PLUGIN_NAME + "/test-collection/"),
+ RestCall.post("/plugins/" + PLUGIN_NAME + "/test-collection/1/update"));
+
+ /** Module for all HTTP bindings. */
+ static class MyPluginHttpModule extends ServletModule {
+ @Override
+ public void configureServlets() {
+ bind(TestRootCollection.class);
+
+ install(
+ new RestApiModule() {
+ @Override
+ public void configure() {
+ DynamicMap.mapOf(binder(), TEST_KIND);
+
+ postOnCollection(TEST_KIND).to(TestPostOnCollection.class);
+ post(TEST_KIND, "update").to(TestPost.class);
+ get(TEST_KIND, "detail").to(TestGet.class);
+ }
+ });
+
+ serveRegex("/(?:a/)?test-collection/(.*)$").with(TestRestApiServlet.class);
+ }
+ }
+
+ @Singleton
+ static class TestRestApiServlet extends RestApiServlet {
+ private static final long serialVersionUID = 1L;
+
+ @Inject
+ TestRestApiServlet(RestApiServlet.Globals globals, Provider<TestRootCollection> collection) {
+ super(globals, collection);
+ }
+
+ @Override
+ public void service(ServletRequest servletRequest, ServletResponse servletResponse)
+ throws ServletException, IOException {
+ // This is...unfortunate. HttpPluginServlet (and/or ContextMapper) doesn't properly set the
+ // servlet path on the wrapped request. Based on what RestApiServlet produces for non-plugin
+ // requests, it should be:
+ // contextPath = "/plugins/checks"
+ // servletPath = "/checkers/"
+ // pathInfo = checkerUuid
+ // Instead it does:
+ // contextPath = "/plugins/checks"
+ // servletPath = ""
+ // pathInfo = "/checkers/" + checkerUuid
+ // This results in RestApiServlet splitting the pathInfo into ["", "checkers", checkerUuid],
+ // and it passes the "" to CheckersCollection#parse, which understandably, but unfortunately,
+ // fails.
+ //
+ // This frankly seems like a bug that should be fixed, but it would quite likely break
+ // existing plugins in confusing ways. So, we work around it by introducing our own request
+ // wrapper with the correct paths.
+ HttpServletRequest req = (HttpServletRequest) servletRequest;
+ String pathInfo = req.getPathInfo();
+ String correctServletPath = "/test-collection/";
+ String fixedPathInfo = pathInfo.substring(correctServletPath.length());
+ HttpServletRequestWrapper wrapped =
+ new HttpServletRequestWrapper(req) {
+ @Override
+ public String getServletPath() {
+ return correctServletPath;
+ }
+
+ @Override
+ public String getPathInfo() {
+ return fixedPathInfo;
+ }
+ };
+
+ super.service(wrapped, (HttpServletResponse) servletResponse);
+ }
+ }
+
+ static class TestPluginResource implements RestResource {}
+
+ @Singleton
+ static class TestRootCollection implements ChildCollection<TopLevelResource, TestPluginResource> {
+ private final DynamicMap<RestView<TestPluginResource>> views;
+
+ @Inject
+ TestRootCollection(DynamicMap<RestView<TestPluginResource>> views) {
+ this.views = views;
+ }
+
+ @Override
+ public RestView<TopLevelResource> list() throws RestApiException {
+ return (RestReadView<TopLevelResource>) resource -> ImmutableList.of("one", "two");
+ }
+
+ @Override
+ public TestPluginResource parse(TopLevelResource parent, IdString id) throws Exception {
+ return new TestPluginResource();
+ }
+
+ @Override
+ public DynamicMap<RestView<TestPluginResource>> views() {
+ return views;
+ }
+ }
+
+ @Singleton
+ static class TestPostOnCollection
+ implements RestCollectionModifyView<TopLevelResource, TestPluginResource, String> {
+ @Override
+ public Object apply(TopLevelResource parentResource, String input) throws Exception {
+ return "test";
+ }
+ }
+
+ @Singleton
+ static class TestPost implements RestModifyView<TestPluginResource, String> {
+ @Override
+ public String apply(TestPluginResource resource, String input) throws Exception {
+ return "test";
+ }
+ }
+
+ @Singleton
+ static class TestGet implements RestReadView<TestPluginResource> {
+ @Override
+ public String apply(TestPluginResource resource) throws Exception {
+ return "test";
+ }
+ }
+
+ @Test
+ public void testEndpoints() throws Exception {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, null, MyPluginHttpModule.class, null)) {
+ RestApiCallHelper.execute(adminRestSession, TEST_CALLS.asList());
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 5eecd0f..100c25e 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
@@ -69,6 +70,7 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
+import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
@@ -3134,6 +3136,26 @@
assertQuery("assignee:self", change);
}
+ @Test
+ public void none() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change change = insert(repo, newChange(repo));
+
+ assertQuery(ChangeIndexPredicate.none());
+
+ for (Predicate<ChangeData> matchingOneChange :
+ ImmutableList.of(
+ // One index query, one post-filtering query.
+ queryBuilder.parse(change.getId().toString()),
+ queryBuilder.parse("ownerin:Administrators"))) {
+ assertQuery(matchingOneChange, change);
+ assertQuery(Predicate.or(ChangeIndexPredicate.none(), matchingOneChange), change);
+ assertQuery(Predicate.and(ChangeIndexPredicate.none(), matchingOneChange));
+ assertQuery(
+ Predicate.and(Predicate.not(ChangeIndexPredicate.none()), matchingOneChange), change);
+ }
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
return newChange(repo, null, null, null, null, false);
}
@@ -3307,21 +3329,35 @@
List<ChangeInfo> result = query.get();
Iterable<Change.Id> ids = ids(result);
assertThat(ids)
- .named(format(query, ids, changes))
+ .named(format(query.getQuery(), ids, changes))
.containsExactlyElementsIn(Arrays.asList(changes))
.inOrder();
return result;
}
- private String format(
- QueryRequest query, Iterable<Change.Id> actualIds, Change.Id... expectedChanges)
+ protected void assertQuery(Predicate<ChangeData> predicate, Change... changes) throws Exception {
+ ImmutableList<Change.Id> actualIds =
+ queryProvider
+ .get()
+ .query(predicate)
+ .stream()
+ .map(ChangeData::getId)
+ .collect(toImmutableList());
+ Change.Id[] expectedIds = Arrays.stream(changes).map(Change::getId).toArray(Change.Id[]::new);
+ assertThat(actualIds)
+ .named(format(predicate.toString(), actualIds, expectedIds))
+ .containsExactlyElementsIn(expectedIds)
+ .inOrder();
+ }
+
+ private String format(String query, Iterable<Change.Id> actualIds, Change.Id... expectedChanges)
throws RestApiException {
- StringBuilder b = new StringBuilder();
- b.append("query '").append(query.getQuery()).append("' with expected changes ");
- b.append(format(Arrays.asList(expectedChanges)));
- b.append(" and result ");
- b.append(format(actualIds));
- return b.toString();
+ return "query '"
+ + query
+ + "' with expected changes "
+ + format(Arrays.asList(expectedChanges))
+ + " and result "
+ + format(actualIds);
}
private String format(Iterable<Change.Id> changeIds) throws RestApiException {
diff --git a/plugins/codemirror-editor b/plugins/codemirror-editor
index c4cf42b..0ffcc1c 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit c4cf42b96a049a0fb854bcbcb85b56a82d91a009
+Subproject commit 0ffcc1c5865bbfecb811d8631e010251fae04317
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index fc0cab0..730f527 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -519,8 +519,27 @@
</div>
</div>
</section>
+
<section class="patchInfo">
- <gr-file-list-header
+ <template is="dom-if" if="[[_showPrimaryTabs]]">
+ <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
+ <paper-tab>Files</paper-tab>
+ <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
+ as="tabHeader">
+ <paper-tab>
+ <gr-endpoint-decorator name$="[[tabHeader]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </paper-tab>
+ </template>
+ </paper-tabs>
+ </template>
+
+ <div hidden$="[[!_showFileTabContent]]">
+ <gr-file-list-header
id="fileListHeader"
account="[[_account]]"
all-patch-sets="[[_allPatchSets]]"
@@ -539,6 +558,7 @@
base-patch-num="{{_patchRange.basePatchNum}}"
files-expanded="[[_filesExpanded]]"
diff-prefs-disabled="[[_diffPrefsDisabled]]"
+ show-title="[[!_showPrimaryTabs]]"
on-open-diff-prefs="_handleOpenDiffPrefs"
on-open-download-dialog="_handleOpenDownloadDialog"
on-open-upload-help-dialog="_handleOpenUploadHelpDialog"
@@ -566,7 +586,18 @@
on-files-shown-changed="_setShownFiles"
on-file-action-tap="_handleFileActionTap"
on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list>
+ </div>
+
+ <template is="dom-if" if="[[!_showFileTabContent]]">
+ <gr-endpoint-decorator name$="[[_selectedFilesTabPluginEndpoint]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </template>
</section>
+
<gr-endpoint-decorator name="change-view-integration">
<gr-endpoint-param name="change" value="[[_change]]">
</gr-endpoint-param>
@@ -575,7 +606,7 @@
</gr-endpoint-decorator>
<paper-tabs
id="commentTabs"
- on-selected-changed="_handleTabChange">
+ on-selected-changed="_handleCommentTabChange">
<paper-tab class="changeLog">Change Log</paper-tab>
<paper-tab
class="commentThreads">
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index bb51fcc..defbcdc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -253,6 +253,25 @@
type: Boolean,
value: true,
},
+ _showFileTabContent: {
+ type: Boolean,
+ value: true,
+ },
+ /** @type {Array<string>} */
+ _dynamicTabHeaderEndpoints: {
+ type: Array,
+ },
+ _showPrimaryTabs: {
+ type: Boolean,
+ computed: '_computeShowPrimaryTabs(_dynamicTabHeaderEndpoints)',
+ },
+ /** @type {Array<string>} */
+ _dynamicTabContentEndpoints: {
+ type: Array,
+ },
+ _selectedFilesTabPluginEndpoint: {
+ type: String,
+ },
},
behaviors: [
@@ -310,6 +329,17 @@
this._setDiffViewMode();
});
+ Gerrit.awaitPluginsLoaded().then(() => {
+ this._dynamicTabHeaderEndpoints =
+ Gerrit._endpoints.getDynamicEndpoints('change-view-tab-header');
+ this._dynamicTabContentEndpoints =
+ Gerrit._endpoints.getDynamicEndpoints('change-view-tab-content');
+ if (this._dynamicTabContentEndpoints.length
+ !== this._dynamicTabHeaderEndpoints.length) {
+ console.warn('Different number of tab headers and tab content.');
+ }
+ });
+
this.addEventListener('comment-save', this._handleCommentSave.bind(this));
this.addEventListener('comment-refresh', this._reloadDrafts.bind(this));
this.addEventListener('comment-discard',
@@ -368,10 +398,18 @@
}
},
- _handleTabChange() {
+ _handleCommentTabChange() {
this._showMessagesView = this.$.commentTabs.selected === 0;
},
+ _handleFileTabChange() {
+ const selectedIndex = this.$$('#primaryTabs').selected;
+ this._showFileTabContent = selectedIndex === 0;
+ // Initial tab is the static files list.
+ this._selectedFilesTabPluginEndpoint =
+ this._dynamicTabContentEndpoints[selectedIndex - 1];
+ },
+
_handleEditCommitMessage(e) {
this._editingCommitMessage = true;
this.$.commitMessageEditor.focusTextarea();
@@ -699,6 +737,8 @@
// Selected has to be set after the paper-tabs are visible because
// the selected underline depends on calculations made by the browser.
this.$.commentTabs.selected = 0;
+ const primaryTabs = this.$$('#primaryTabs');
+ if (primaryTabs) primaryTabs.selected = 0;
this.async(() => {
if (this.viewState.scrollTop) {
@@ -821,6 +861,10 @@
this.fire('title-change', {title});
},
+ _computeShowPrimaryTabs(dynamicTabContentEndpoints) {
+ return dynamicTabContentEndpoints.length > 0;
+ },
+
_computeChangeUrl(change) {
return Gerrit.Nav.getUrlForChange(change);
},
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
index 924ddab..1a307f7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
@@ -154,7 +154,10 @@
</style>
<div class$="patchInfo-header [[_computeEditModeClass(editMode)]] [[_computePatchInfoClass(patchNum, allPatchSets)]]">
<div class="patchInfo-left">
- <h3 class="label">Files</h3>
+ <template is="dom-if"
+ if="[[showTitle]]">
+ <h3 class="label">Files</h3>
+ </template>
<div class="patchInfoContent">
<gr-patch-range-select
id="rangeSelect"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
index b9e6288..031f2d5 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
@@ -81,6 +81,10 @@
type: String,
value: '',
},
+ showTitle: {
+ type: Boolean,
+ value: true,
+ },
_descriptionReadOnly: {
type: Boolean,
computed: '_computeDescriptionReadOnly(loggedIn, change, account)',
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index 9801b44..e4f699c 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -16,16 +16,14 @@
{namespace com.google.gerrit.httpd.raw}
-/**
- * @param canonicalPath
- * @param staticResourcePath
- * @param? assetsPath {string} URL to static assets root, if served from CDN.
- * @param? assetsBundle {string} Assets bundle .html file, served from $assetsPath.
- * @param? faviconPath
- * @param? versionInfo
- * @param? deprecateGwtUi
- */
{template .Index}
+ {@param canonicalPath: ?}
+ {@param staticResourcePath: ?}
+ {@param? assetsPath: ?} /** {string} URL to static assets root, if served from CDN. */
+ {@param? assetsBundle: ?} /** {string} Assets bundle .html file, served from $assetsPath. */
+ {@param? faviconPath: ?}
+ {@param? versionInfo: ?}
+ {@param? deprecateGwtUi: ?}
<!DOCTYPE html>{\n}
<html lang="en">{\n}
<meta charset="utf-8">{\n}
