Merge "Remove remnants of project-specific themes support"
diff --git a/.mailmap b/.mailmap
index c863847..b5c119c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -6,6 +6,7 @@
Alex Ryazantsev <alex.ryazantsev@gmail.com> alex.ryazantsev <alex.ryazantsev@gmail.com>
Andrew Bonventre <andybons@chromium.org> <andybons@google.com>
Becky Siegel <beckysiegel@google.com> beckysiegel <beckysiegel@google.com>
+Ben Rohlfs <brohlfs@google.com> brohlfs <brohlfs@google.com>
Brad Larson <bklarson@gmail.com> <brad.larson@garmin.com>
Bruce Zu <bruce.zu.run10@gmail.com> <bruce.zu@sonyericsson.com>
Bruce Zu <bruce.zu.run10@gmail.com> <bruce.zu@sonymobile.com>
diff --git a/Documentation/config-plugins.txt b/Documentation/config-plugins.txt
index cda02fb..3a80910 100644
--- a/Documentation/config-plugins.txt
+++ b/Documentation/config-plugins.txt
@@ -9,6 +9,10 @@
link:config-gerrit.html#plugins.checkFrequency[a few minutes] until
the server picks up new and updated plugins.
+Due to caching, you might need to flush your browser cache after
+installing a plugin. Users will usually see the result within
+several minutes.
+
Plugins can also be installed via
link:rest-api-plugins.html#install-plugin[REST] and
link:cmd-plugin-install.html[SSH].
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/intro-user.txt b/Documentation/intro-user.txt
index 544039b..17c9a61 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -548,7 +548,8 @@
----
Alternatively, click *Ready* from the Change screen.
-Only change owners, project owners and site administrators can mark changes as
+Change owners, project owners, site administrators and members of a group that
+was granted "Toggle Work In Progress state" permission can mark changes as
`work-in-progress` and `ready`.
[[wip-polygerrit]]
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/Documentation/user-search.txt b/Documentation/user-search.txt
index 5d7a78b..cafd5ca 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -216,7 +216,8 @@
[[hashtag]]
hashtag:'HASHTAG'::
+
-Changes whose link:intro-user.html#hashtags[hashtag] matches 'HASHTAG' exactly.
+Changes whose link:intro-user.html#hashtags[hashtag] matches 'HASHTAG'.
+The match is case-insensitive.
[[ref]]
ref:'REF'::
diff --git a/WORKSPACE b/WORKSPACE
index ca29e49..c2629ba 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -14,9 +14,9 @@
http_archive(
name = "io_bazel_rules_closure",
- sha256 = "4f2c173ebf95e94d98a0d5cb799e734536eaf3eca280eb15e124f5e5ef8b6e39",
- strip_prefix = "rules_closure-6fd76e645b5c622221c9920f41a4d0bc578a3046",
- urls = ["https://github.com/bazelbuild/rules_closure/archive/6fd76e645b5c622221c9920f41a4d0bc578a3046.tar.gz"],
+ sha256 = "0e6de40666f2ebb2b30dc0339745a274d9999334a249b05a3b1f46462e489adf",
+ strip_prefix = "rules_closure-87d24b1df8b62405de8dd059cb604fd9d4b1e395",
+ urls = ["https://github.com/bazelbuild/rules_closure/archive/87d24b1df8b62405de8dd059cb604fd9d4b1e395.tar.gz"],
)
# File is specific to Polymer and copied from the Closure Github -- should be
@@ -47,11 +47,11 @@
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "ee5fe78fe417c685ecb77a0a725dc9f6040ae5beb44a0ba4ddb55453aad23a8a",
- url = "https://github.com/bazelbuild/rules_go/releases/download/0.16.0/rules_go-0.16.0.tar.gz",
+ sha256 = "6776d68ebb897625dead17ae510eac3d5f6342367327875210df44dbe2aeeb19",
+ url = "https://github.com/bazelbuild/rules_go/releases/download/0.17.1/rules_go-0.17.1.tar.gz",
)
-load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies")
+load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
go_rules_dependencies()
@@ -59,8 +59,8 @@
http_archive(
name = "bazel_gazelle",
- sha256 = "c0a5739d12c6d05b6c1ad56f2200cb0b57c5a70e03ebd2f7b87ce88cabf09c7b",
- urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.14.0/bazel-gazelle-0.14.0.tar.gz"],
+ sha256 = "3c681998538231a2d24d0c07ed5a7658cb72bfb5fd4bf9911157c0e9ac6a2687",
+ urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.17.0/bazel-gazelle-0.17.0.tar.gz"],
)
load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository")
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index 2e9c2d6..3ba0ba7 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -46,6 +46,7 @@
public static final String REMOVE_REVIEWER = "removeReviewer";
public static final String SUBMIT = "submit";
public static final String SUBMIT_AS = "submitAs";
+ public static final String TOGGLE_WORK_IN_PROGRESS_STATE = "toggleWipState";
public static final String VIEW_PRIVATE_CHANGES = "viewPrivateChanges";
private static final List<String> NAMES_LC;
@@ -78,6 +79,7 @@
NAMES_LC.add(REMOVE_REVIEWER.toLowerCase());
NAMES_LC.add(SUBMIT.toLowerCase());
NAMES_LC.add(SUBMIT_AS.toLowerCase());
+ NAMES_LC.add(TOGGLE_WORK_IN_PROGRESS_STATE.toLowerCase());
NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase());
LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL);
diff --git a/java/com/google/gerrit/extensions/config/CapabilityDefinition.java b/java/com/google/gerrit/extensions/config/CapabilityDefinition.java
index aafb583..c76ec6c 100644
--- a/java/com/google/gerrit/extensions/config/CapabilityDefinition.java
+++ b/java/com/google/gerrit/extensions/config/CapabilityDefinition.java
@@ -18,7 +18,4 @@
/** Specifies a capability declared by a plugin. */
@ExtensionPoint
-public abstract class CapabilityDefinition {
- /** @return description of the capability. */
- public abstract String getDescription();
-}
+public abstract class CapabilityDefinition implements PluginPermissionDefinition {}
diff --git a/java/com/google/gerrit/extensions/config/PluginPermissionDefinition.java b/java/com/google/gerrit/extensions/config/PluginPermissionDefinition.java
new file mode 100644
index 0000000..11b1981
--- /dev/null
+++ b/java/com/google/gerrit/extensions/config/PluginPermissionDefinition.java
@@ -0,0 +1,25 @@
+// 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.extensions.config;
+
+/** Specifies a permission declared by a plugin. */
+public interface PluginPermissionDefinition {
+ /**
+ * Gets the description of a permission declared by a plugin.
+ *
+ * @return description of the permission.
+ */
+ String getDescription();
+}
diff --git a/java/com/google/gerrit/extensions/config/PluginProjectPermissionDefinition.java b/java/com/google/gerrit/extensions/config/PluginProjectPermissionDefinition.java
new file mode 100644
index 0000000..d1d9f9e
--- /dev/null
+++ b/java/com/google/gerrit/extensions/config/PluginProjectPermissionDefinition.java
@@ -0,0 +1,21 @@
+// 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.extensions.config;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+/** Specifies a repository permission declared by a plugin. */
+@ExtensionPoint
+public abstract class PluginProjectPermissionDefinition implements PluginPermissionDefinition {}
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/change/SetPrivateOp.java b/java/com/google/gerrit/server/change/SetPrivateOp.java
index 2b00a40..017dbec 100644
--- a/java/com/google/gerrit/server/change/SetPrivateOp.java
+++ b/java/com/google/gerrit/server/change/SetPrivateOp.java
@@ -22,6 +22,7 @@
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.PrivateStateChanged;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -83,7 +84,8 @@
}
if (isPrivate && !change.isNew()) {
- throw new BadRequestException("cannot set a non-open change to private");
+ throw new BadRequestException(
+ String.format("cannot set %s change to private", ChangeUtil.status(change)));
}
ChangeNotes notes = ctx.getNotes();
ps = psUtil.get(notes, change.currentPatchSetId());
diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java
index 02870fb..e5fdd8f 100644
--- a/java/com/google/gerrit/server/change/WorkInProgressOp.java
+++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java
@@ -18,20 +18,14 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeMessagesUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -57,34 +51,6 @@
WorkInProgressOp create(boolean workInProgress, Input in);
}
- public static void checkPermissions(
- PermissionBackend permissionBackend, CurrentUser user, Change change)
- throws PermissionBackendException, AuthException {
- if (!user.isIdentifiedUser()) {
- throw new AuthException("Authentication required");
- }
-
- if (change.getOwner().equals(user.asIdentifiedUser().getAccountId())) {
- return;
- }
-
- try {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
- return;
- } catch (AuthException e) {
- // Skip.
- }
-
- try {
- permissionBackend
- .user(user)
- .project(change.getProject())
- .check(ProjectPermission.WRITE_CONFIG);
- } catch (AuthException exp) {
- throw new AuthException("not allowed to toggle work in progress");
- }
- }
-
private final ChangeMessagesUtil cmUtil;
private final EmailReviewComments.Factory email;
private final PatchSetUtil psUtil;
diff --git a/java/com/google/gerrit/server/config/ConfigResource.java b/java/com/google/gerrit/server/config/ConfigResource.java
index ec0e0c2..4275dc4 100644
--- a/java/com/google/gerrit/server/config/ConfigResource.java
+++ b/java/com/google/gerrit/server/config/ConfigResource.java
@@ -14,11 +14,22 @@
package com.google.gerrit.server.config;
+import com.google.gerrit.extensions.restapi.CacheControl;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.inject.TypeLiteral;
+import java.util.concurrent.TimeUnit;
public class ConfigResource implements RestResource {
public static final TypeLiteral<RestView<ConfigResource>> CONFIG_KIND =
new TypeLiteral<RestView<ConfigResource>>() {};
+
+ /**
+ * Default cache control that gets set on the 'Cache-Control' header for responses on this
+ * resource that are cachable.
+ *
+ * <p>Not all resources are cacheable and in fact the vast majority might not be. Caching is a
+ * trade-off between the freshness of data and the number of QPS that the web UI sends.
+ */
+ public static CacheControl DEFAULT_CACHE_CONTROL = CacheControl.PRIVATE(300, TimeUnit.SECONDS);
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 4158346..711efaf 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.config.DownloadScheme;
import com.google.gerrit.extensions.config.ExternalIncludedIn;
import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
import com.google.gerrit.extensions.events.AccountIndexedListener;
import com.google.gerrit.extensions.events.AgreementSignupListener;
import com.google.gerrit.extensions.events.AssigneeChangedListener;
@@ -309,6 +310,7 @@
DynamicMap.mapOf(binder(), new TypeLiteral<Cache<?, ?>>() {});
DynamicSet.setOf(binder(), CacheRemovalListener.class);
DynamicMap.mapOf(binder(), CapabilityDefinition.class);
+ DynamicMap.mapOf(binder(), PluginProjectPermissionDefinition.class);
DynamicSet.setOf(binder(), GitReferenceUpdatedListener.class);
DynamicSet.setOf(binder(), AssigneeChangedListener.class);
DynamicSet.setOf(binder(), ChangeAbandonedListener.class);
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/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 250ca03..309bf51 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -176,6 +176,14 @@
return refControl.canForceEditTopicName();
}
+ /** Can this user toggle WorkInProgress state? */
+ private boolean canToggleWorkInProgressState() {
+ return isOwner()
+ || getProjectControl().isOwner()
+ || refControl.canPerform(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
+ || getProjectControl().isAdmin();
+ }
+
/** Can this user edit the description? */
private boolean canEditDescription() {
if (getChange().isNew()) {
@@ -299,6 +307,8 @@
return canRestore();
case SUBMIT:
return refControl.canSubmit(isOwner());
+ case TOGGLE_WORK_IN_PROGRESS_STATE:
+ return canToggleWorkInProgressState();
case REMOVE_REVIEWER:
case SUBMIT_AS:
diff --git a/java/com/google/gerrit/server/permissions/ChangePermission.java b/java/com/google/gerrit/server/permissions/ChangePermission.java
index ca1c460..2fba4ef 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermission.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermission.java
@@ -55,7 +55,8 @@
*/
REBASE,
SUBMIT,
- SUBMIT_AS("submit on behalf of other users");
+ SUBMIT_AS("submit on behalf of other users"),
+ TOGGLE_WORK_IN_PROGRESS_STATE;
private final String description;
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java b/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
index ece29df..d56bc78 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
@@ -97,6 +97,9 @@
.put(ChangePermission.REBASE, Permission.REBASE)
.put(ChangePermission.SUBMIT, Permission.SUBMIT)
.put(ChangePermission.SUBMIT_AS, Permission.SUBMIT_AS)
+ .put(
+ ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
.build();
private static <T extends Enum<T>> void checkMapContainsAllEnumValues(
diff --git a/java/com/google/gerrit/server/permissions/PluginPermissionsUtil.java b/java/com/google/gerrit/server/permissions/PluginPermissionsUtil.java
new file mode 100644
index 0000000..a837ed7
--- /dev/null
+++ b/java/com/google/gerrit/server/permissions/PluginPermissionsUtil.java
@@ -0,0 +1,110 @@
+// 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.permissions;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.gerrit.extensions.config.PluginPermissionDefinition;
+import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.Extension;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.regex.Pattern;
+
+/** Utilities for plugin permissions. */
+@Singleton
+public final class PluginPermissionsUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private static final String PLUGIN_NAME_PATTERN_STRING = "[a-zA-Z0-9-]+";
+
+ /**
+ * Name pattern for a plugin non-capability permission stored in the config file.
+ *
+ * <p>This pattern requires a plugin declared permission to have a name in the access section of
+ * {@code ProjectConfig} with a format like "plugin-{pluginName}-{permissionName}", which makes it
+ * easier to tell if a config name represents a plugin permission or not. Note "-" isn't clear
+ * enough for this purpose since some core permissions, e.g. "label-", also contain "-".
+ */
+ private static final Pattern PLUGIN_PERMISSION_NAME_IN_CONFIG_PATTERN =
+ Pattern.compile("^plugin-" + PLUGIN_NAME_PATTERN_STRING + "-[a-zA-Z]+$");
+
+ /** Name pattern for a Gerrit plugin. */
+ private static final Pattern PLUGIN_NAME_PATTERN =
+ Pattern.compile("^" + PLUGIN_NAME_PATTERN_STRING + "$");
+
+ private final DynamicMap<CapabilityDefinition> capabilityDefinitions;
+ private final DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions;
+
+ @Inject
+ PluginPermissionsUtil(
+ DynamicMap<CapabilityDefinition> capabilityDefinitions,
+ DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions) {
+ this.capabilityDefinitions = capabilityDefinitions;
+ this.pluginProjectPermissionDefinitions = pluginProjectPermissionDefinitions;
+ }
+
+ /**
+ * Collects all the plugin declared capabilities.
+ *
+ * @return a map of plugin declared capabilities with "pluginName" as its keys and
+ * "pluginName-{permissionName}" as its values.
+ */
+ public ImmutableMap<String, String> collectPluginCapabilities() {
+ return collectPermissions(capabilityDefinitions, "");
+ }
+
+ /**
+ * Collects all the plugin declared project permissions.
+ *
+ * @return a map of plugin declared project permissions with "{pluginName}" as its keys and
+ * "plugin-{pluginName}-{permissionName}" as its values.
+ */
+ public ImmutableMap<String, String> collectPluginProjectPermissions() {
+ return collectPermissions(pluginProjectPermissionDefinitions, "plugin-");
+ }
+
+ private static <T extends PluginPermissionDefinition>
+ ImmutableMap<String, String> collectPermissions(DynamicMap<T> definitions, String prefix) {
+ ImmutableMap.Builder<String, String> permissionIdNames = ImmutableMap.builder();
+
+ for (Extension<T> extension : definitions) {
+ String pluginName = extension.getPluginName();
+ if (!PLUGIN_NAME_PATTERN.matcher(pluginName).matches()) {
+ logger.atWarning().log(
+ "Plugin name '%s' must match '%s' to use permissions; rename the plugin",
+ pluginName, PLUGIN_NAME_PATTERN.pattern());
+ continue;
+ }
+
+ String id = prefix + pluginName + "-" + extension.getExportName();
+ permissionIdNames.put(id, extension.get().getDescription());
+ }
+
+ return permissionIdNames.build();
+ }
+
+ /**
+ * Checks if a given name matches the plugin declared permission name pattern for configs.
+ *
+ * @param name a config name which may stand for a plugin permission.
+ * @return whether the name matches the plugin permission name pattern for configs.
+ */
+ public static boolean isPluginPermission(String name) {
+ return PLUGIN_PERMISSION_NAME_IN_CONFIG_PATTERN.matcher(name).matches();
+ }
+}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index c4ef32e..439e7c0 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -18,6 +18,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.common.data.Permission.isPermission;
import static com.google.gerrit.reviewdb.client.Project.DEFAULT_SUBMIT_TYPE;
+import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isPluginPermission;
import static java.util.stream.Collectors.toList;
import com.google.common.base.CharMatcher;
@@ -747,7 +748,7 @@
for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
for (String n : Splitter.on(EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN).split(varName)) {
n = convertLegacyPermission(n);
- if (isPermission(n)) {
+ if (isCoreOrPluginPermission(n)) {
as.getPermission(n, true).setExclusiveGroup(true);
}
}
@@ -755,7 +756,7 @@
for (String varName : rc.getNames(ACCESS, refName)) {
String convertedName = convertLegacyPermission(varName);
- if (isPermission(convertedName)) {
+ if (isCoreOrPluginPermission(convertedName)) {
Permission perm = as.getPermission(convertedName, true);
loadPermissionRules(
rc,
@@ -784,6 +785,12 @@
}
}
+ private boolean isCoreOrPluginPermission(String permission) {
+ // Since plugins are loaded dynamically, here we can't load all plugin permissions and verify
+ // their existence.
+ return isPermission(permission) || isPluginPermission(permission);
+ }
+
private boolean isValidRegex(String refPattern) {
try {
RefPattern.validateRegExp(refPattern);
@@ -1360,7 +1367,7 @@
}
for (String varName : rc.getNames(ACCESS, refName)) {
- if (isPermission(convertLegacyPermission(varName))
+ if (isCoreOrPluginPermission(convertLegacyPermission(varName))
&& !have.contains(varName.toLowerCase())) {
rc.unset(ACCESS, refName, varName);
}
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 dfbc221..ada9f27 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -14,6 +14,11 @@
package com.google.gerrit.server.query.change;
+import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.flogger.LazyArgs.lazy;
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.index.query.Predicate;
@@ -21,6 +26,7 @@
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -41,20 +47,27 @@
import org.eclipse.jgit.revwalk.RevWalk;
public class ConflictsPredicate {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
// UI code may depend on this string, so use caution when changing.
protected static final String TOO_MANY_FILES = "too many files to find conflicts";
private ConflictsPredicate() {}
public static Predicate<ChangeData> create(Arguments args, String value, Change c)
- throws QueryParseException, OrmException {
+ throws QueryParseException {
ChangeData cd;
List<String> files;
try {
cd = args.changeDataFactory.create(c);
files = cd.currentFilePaths();
- } catch (IOException e) {
- throw new OrmException(e);
+ } catch (IOException | OrmException e) {
+ warnWithOccasionalStackTrace(
+ e,
+ "Error constructing conflicts predicates for change %s in %s",
+ c.getId(),
+ c.getProject());
+ return ChangeIndexPredicate.none();
}
if (3 + files.size() > args.indexConfig.maxTerms()) {
@@ -95,52 +108,66 @@
}
@Override
- public boolean match(ChangeData object) throws OrmException {
- Change otherChange = object.change();
- if (otherChange == null || !otherChange.getDest().equals(dest)) {
- return false;
- }
-
- SubmitTypeRecord str = object.submitTypeRecord();
- if (!str.isOk()) {
- return false;
- }
-
- ProjectState projectState;
+ public boolean match(ChangeData object) {
+ Change.Id id = object.getId();
+ Project.NameKey otherProject = null;
+ ObjectId other = null;
try {
- projectState = changeDataCache.getProjectState();
- } catch (NoSuchProjectException e) {
- return false;
- }
+ Change otherChange = object.change();
+ if (otherChange == null || !otherChange.getDest().equals(dest)) {
+ return false;
+ }
+ otherProject = otherChange.getProject();
- ObjectId other = ObjectId.fromString(object.currentPatchSet().getRevision().get());
- ConflictKey conflictsKey =
- ConflictKey.create(
- changeDataCache.getTestAgainst(),
- other,
- str.type,
- projectState.is(BooleanProjectConfig.USE_CONTENT_MERGE));
- Boolean maybeConflicts = args.conflictsCache.getIfPresent(conflictsKey);
- if (maybeConflicts != null) {
- return maybeConflicts;
- }
+ SubmitTypeRecord str = object.submitTypeRecord();
+ if (!str.isOk()) {
+ return false;
+ }
- try (Repository repo = args.repoManager.openRepository(otherChange.getProject());
- CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
- boolean conflicts =
- !args.submitDryRun.run(
- null,
- str.type,
- repo,
- rw,
- otherChange.getDest(),
+ ProjectState projectState;
+ try {
+ projectState = changeDataCache.getProjectState();
+ } catch (NoSuchProjectException e) {
+ return false;
+ }
+
+ other = ObjectId.fromString(object.currentPatchSet().getRevision().get());
+ ConflictKey conflictsKey =
+ ConflictKey.create(
changeDataCache.getTestAgainst(),
other,
- getAlreadyAccepted(repo, rw));
- args.conflictsCache.put(conflictsKey, conflicts);
- return conflicts;
- } catch (IntegrationException | NoSuchProjectException | IOException e) {
- throw new OrmException(e);
+ str.type,
+ projectState.is(BooleanProjectConfig.USE_CONTENT_MERGE));
+ Boolean maybeConflicts = args.conflictsCache.getIfPresent(conflictsKey);
+ if (maybeConflicts != null) {
+ return maybeConflicts;
+ }
+
+ try (Repository repo = args.repoManager.openRepository(otherChange.getProject());
+ CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
+ boolean conflicts =
+ !args.submitDryRun.run(
+ null,
+ str.type,
+ repo,
+ rw,
+ otherChange.getDest(),
+ changeDataCache.getTestAgainst(),
+ other,
+ getAlreadyAccepted(repo, rw));
+ args.conflictsCache.put(conflictsKey, conflicts);
+ return conflicts;
+ }
+ } catch (IntegrationException | NoSuchProjectException | OrmException | IOException e) {
+ ObjectId finalOther = other;
+ warnWithOccasionalStackTrace(
+ e,
+ "Merge failure checking conflicts of change %s in %s (%s): %s",
+ id,
+ firstNonNull(otherProject, "unknown project"),
+ lazy(() -> finalOther != null ? finalOther.name() : "unknown commit"),
+ e.getMessage());
+ return false;
}
}
@@ -202,4 +229,13 @@
return alreadyAccepted;
}
}
+
+ 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/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 8afaa45..ff27bfa 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -340,8 +340,10 @@
return Response.withStatusCode(SC_BAD_REQUEST, output);
}
- WorkInProgressOp.checkPermissions(
- permissionBackend, revision.getUser(), revision.getChange());
+ revision
+ .getChangeResource()
+ .permissions()
+ .check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
if (input.ready) {
output.ready = true;
diff --git a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
index 94914b0..aacf58b 100644
--- a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
+++ b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -25,48 +24,36 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.change.WorkInProgressOp.Input;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, Input, Response<?>>
implements UiAction<ChangeResource> {
private final WorkInProgressOp.Factory opFactory;
- private final PermissionBackend permissionBackend;
- private final Provider<CurrentUser> user;
@Inject
- SetReadyForReview(
- RetryHelper retryHelper,
- WorkInProgressOp.Factory opFactory,
- PermissionBackend permissionBackend,
- Provider<CurrentUser> user) {
+ SetReadyForReview(RetryHelper retryHelper, WorkInProgressOp.Factory opFactory) {
super(retryHelper);
this.opFactory = opFactory;
- this.permissionBackend = permissionBackend;
- this.user = user;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
+ rsrc.permissions().check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
Change change = rsrc.getChange();
if (!change.isNew()) {
@@ -94,15 +81,6 @@
.setVisible(
and(
rsrc.getChange().isNew() && rsrc.getChange().isWorkInProgress(),
- or(
- rsrc.isUserOwner(),
- or(
- permissionBackend
- .currentUser()
- .testCond(GlobalPermission.ADMINISTRATE_SERVER),
- permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .testCond(ProjectPermission.WRITE_CONFIG)))));
+ rsrc.permissions().testCond(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE)));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
index d087e47..852813e 100644
--- a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
+++ b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -25,48 +24,36 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.change.WorkInProgressOp.Input;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, Input, Response<?>>
implements UiAction<ChangeResource> {
private final WorkInProgressOp.Factory opFactory;
- private final PermissionBackend permissionBackend;
- private final Provider<CurrentUser> user;
@Inject
- SetWorkInProgress(
- WorkInProgressOp.Factory opFactory,
- RetryHelper retryHelper,
- PermissionBackend permissionBackend,
- Provider<CurrentUser> user) {
+ SetWorkInProgress(WorkInProgressOp.Factory opFactory, RetryHelper retryHelper) {
super(retryHelper);
this.opFactory = opFactory;
- this.permissionBackend = permissionBackend;
- this.user = user;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
+ rsrc.permissions().check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
Change change = rsrc.getChange();
if (!change.isNew()) {
@@ -94,15 +81,6 @@
.setVisible(
and(
rsrc.getChange().isNew() && !rsrc.getChange().isWorkInProgress(),
- or(
- rsrc.isUserOwner(),
- or(
- permissionBackend
- .currentUser()
- .testCond(GlobalPermission.ADMINISTRATE_SERVER),
- permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .testCond(ProjectPermission.WRITE_CONFIG)))));
+ rsrc.permissions().testCond(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE)));
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetVersion.java b/java/com/google/gerrit/server/restapi/config/GetVersion.java
index 8135719..ee206d6 100644
--- a/java/com/google/gerrit/server/restapi/config/GetVersion.java
+++ b/java/com/google/gerrit/server/restapi/config/GetVersion.java
@@ -15,19 +15,22 @@
package com.google.gerrit.server.restapi.config;
import com.google.gerrit.common.Version;
+import com.google.gerrit.extensions.restapi.CacheControl;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.config.ConfigResource;
import com.google.inject.Singleton;
+import java.util.concurrent.TimeUnit;
@Singleton
public class GetVersion implements RestReadView<ConfigResource> {
@Override
- public String apply(ConfigResource resource) throws ResourceNotFoundException {
+ public Response<String> apply(ConfigResource resource) throws ResourceNotFoundException {
String version = Version.getVersion();
if (version == null) {
throw new ResourceNotFoundException();
}
- return version;
+ return Response.ok(version).caching(CacheControl.PRIVATE(30, TimeUnit.SECONDS));
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/ListCapabilities.java b/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
index fa9bfde..9bb2e6d 100644
--- a/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
+++ b/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
@@ -14,38 +14,32 @@
package com.google.gerrit.server.restapi.config;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+
import com.google.common.collect.ImmutableMap;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.GlobalCapability;
-import com.google.gerrit.extensions.config.CapabilityDefinition;
-import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.config.CapabilityConstants;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PluginPermissionsUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.HashMap;
import java.util.Map;
-import java.util.regex.Pattern;
/** List capabilities visible to the calling user. */
@Singleton
public class ListCapabilities implements RestReadView<ConfigResource> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private static final Pattern PLUGIN_NAME_PATTERN = Pattern.compile("^[a-zA-Z0-9-]+$");
-
private final PermissionBackend permissionBackend;
- private final DynamicMap<CapabilityDefinition> pluginCapabilities;
+ private final PluginPermissionsUtil pluginPermissionsUtil;
@Inject
public ListCapabilities(
- PermissionBackend permissionBackend, DynamicMap<CapabilityDefinition> pluginCapabilities) {
+ PermissionBackend permissionBackend, PluginPermissionsUtil pluginPermissionsUtil) {
this.permissionBackend = permissionBackend;
- this.pluginCapabilities = pluginCapabilities;
+ this.pluginPermissionsUtil = pluginPermissionsUtil;
}
@Override
@@ -59,21 +53,16 @@
}
public Map<String, CapabilityInfo> collectPluginCapabilities() {
- Map<String, CapabilityInfo> output = new HashMap<>();
- for (String pluginName : pluginCapabilities.plugins()) {
- if (!PLUGIN_NAME_PATTERN.matcher(pluginName).matches()) {
- logger.atWarning().log(
- "Plugin name '%s' must match '%s' to use capabilities; rename the plugin",
- pluginName, PLUGIN_NAME_PATTERN.pattern());
- continue;
- }
- for (Map.Entry<String, Provider<CapabilityDefinition>> entry :
- pluginCapabilities.byPlugin(pluginName).entrySet()) {
- String id = String.format("%s-%s", pluginName, entry.getKey());
- output.put(id, new CapabilityInfo(id, entry.getValue().get().getDescription()));
- }
- }
- return output;
+ return convertToPermissionInfos(pluginPermissionsUtil.collectPluginCapabilities());
+ }
+
+ private static ImmutableMap<String, CapabilityInfo> convertToPermissionInfos(
+ ImmutableMap<String, String> permissionIdNames) {
+ return permissionIdNames
+ .entrySet()
+ .stream()
+ .collect(
+ toImmutableMap(Map.Entry::getKey, e -> new CapabilityInfo(e.getKey(), e.getValue())));
}
private Map<String, CapabilityInfo> collectCoreCapabilities()
diff --git a/java/com/google/gerrit/server/restapi/config/ListTopMenus.java b/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
index c296a7d..2feb580 100644
--- a/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
+++ b/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
@@ -14,8 +14,10 @@
package com.google.gerrit.server.restapi.config;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.webui.TopMenu;
+import com.google.gerrit.extensions.webui.TopMenu.MenuEntry;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
@@ -33,9 +35,9 @@
}
@Override
- public List<TopMenu.MenuEntry> apply(ConfigResource resource) {
+ public Response<List<MenuEntry>> apply(ConfigResource resource) {
List<TopMenu.MenuEntry> entries = new ArrayList<>();
extensions.runEach(extension -> entries.addAll(extension.getEntries()));
- return entries;
+ return Response.ok(entries).caching(ConfigResource.DEFAULT_CACHE_CONTROL);
}
}
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..c38f686 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -277,7 +277,7 @@
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(changeId).setWorkInProgress();
}
@@ -323,7 +323,7 @@
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(changeId).setReadyForReview();
}
@@ -500,6 +500,37 @@
}
@Test
+ public void toggleWorkInProgressStateByNonOwnerWithPermission() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String refactor = "Needs some refactoring";
+ String ptal = "PTAL";
+
+ grant(
+ project,
+ "refs/heads/master",
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ false,
+ REGISTERED_USERS);
+
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(changeId).setWorkInProgress(refactor);
+
+ ChangeInfo info = gApi.changes().id(changeId).get();
+
+ assertThat(info.workInProgress).isTrue();
+ assertThat(Iterables.getLast(info.messages).message).contains(refactor);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_WIP);
+
+ gApi.changes().id(changeId).setReadyForReview(ptal);
+
+ info = gApi.changes().id(changeId).get();
+ assertThat(info.workInProgress).isNull();
+ assertThat(Iterables.getLast(info.messages).message).contains(ptal);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_READY);
+ }
+
+ @Test
public void reviewAndStartReview() throws Exception {
PushOneCommit.Result r = createWorkInProgressChange();
r.assertOkStatus();
@@ -588,17 +619,33 @@
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(r.getChangeId()).current().review(in);
}
@Test
+ public void reviewWithWorkInProgressByNonOwnerWithPermission() throws Exception {
+ PushOneCommit.Result r = createChange();
+ ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
+ grant(
+ project,
+ "refs/heads/master",
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ false,
+ REGISTERED_USERS);
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(r.getChangeId()).current().review(in);
+ ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ assertThat(info.workInProgress).isTrue();
+ }
+
+ @Test
public void reviewWithReadyByNonOwnerReturnsError() throws Exception {
PushOneCommit.Result r = createChange();
ReviewInput in = ReviewInput.noScore().setReady(true);
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(r.getChangeId()).current().review(in);
}
@@ -3533,6 +3580,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/api/change/PrivateChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
index 7ccf9a9c..7973241 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
@@ -81,7 +81,7 @@
}
@Test
- public void setMergedChangePrivate() throws Exception {
+ public void cannotSetMergedChangePrivate() throws Exception {
PushOneCommit.Result result = createChange();
approve(result.getChangeId());
merge(result);
@@ -90,7 +90,20 @@
assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
exception.expect(BadRequestException.class);
- exception.expectMessage("cannot set a non-open change to private");
+ exception.expectMessage("cannot set merged change to private");
+ gApi.changes().id(changeId).setPrivate(true);
+ }
+
+ @Test
+ public void cannotSetAbandonedChangePrivate() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+
+ gApi.changes().id(changeId).abandon();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("cannot set abandoned change to private");
gApi.changes().id(changeId).setPrivate(true);
}
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/acceptance/rest/project/PluginAccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/PluginAccessIT.java
index 1e6afa8..e7663f7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/PluginAccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/PluginAccessIT.java
@@ -26,15 +26,21 @@
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.permissions.PluginPermissionsUtil;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import com.google.inject.Module;
+import java.util.Set;
import org.junit.Test;
-public class PluginAccessIT extends AbstractDaemonTest {
+public final class PluginAccessIT extends AbstractDaemonTest {
+ private static final String TEST_PLUGIN_NAME = "gerrit";
+ private static final String TEST_PLUGIN_CAPABILITY = "aPluginCapability";
+ private static final String TEST_PLUGIN_PROJECT_PERMISSION = "aPluginProjectPermission";
- private static final String CORE_PLUGIN_PREFIX = "gerrit-";
- private static final String PLUGIN_CAPABILITY = "printHello";
+ @Inject PluginPermissionsUtil pluginPermissionsUtil;
@Override
public Module createModule() {
@@ -42,12 +48,21 @@
@Override
protected void configure() {
bind(CapabilityDefinition.class)
- .annotatedWith(Exports.named(PLUGIN_CAPABILITY))
+ .annotatedWith(Exports.named(TEST_PLUGIN_CAPABILITY))
.toInstance(
new CapabilityDefinition() {
@Override
public String getDescription() {
- return "Print Hello";
+ return "A Plugin Capability";
+ }
+ });
+ bind(PluginProjectPermissionDefinition.class)
+ .annotatedWith(Exports.named(TEST_PLUGIN_PROJECT_PERMISSION))
+ .toInstance(
+ new PluginProjectPermissionDefinition() {
+ @Override
+ public String getDescription() {
+ return "A Plugin Project Permission";
}
});
}
@@ -55,24 +70,47 @@
}
@Test
- public void addPluginCapability() throws Exception {
- ProjectAccessInput accessInput = new ProjectAccessInput();
- AccessSectionInfo accessSectionInfo = new AccessSectionInfo();
- PermissionInfo email = new PermissionInfo(null, null);
- PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ public void setAccessAddPluginCapabilitySucceed() throws Exception {
+ String pluginCapability = TEST_PLUGIN_NAME + "-" + TEST_PLUGIN_CAPABILITY;
+ ProjectAccessInput accessInput =
+ createAccessInput(AccessSection.GLOBAL_CAPABILITIES, pluginCapability);
- email.rules = ImmutableMap.of(SystemGroupBackend.REGISTERED_USERS.get(), pri);
- accessSectionInfo.permissions = ImmutableMap.of(CORE_PLUGIN_PREFIX + PLUGIN_CAPABILITY, email);
- accessInput.add = ImmutableMap.of(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
-
- ProjectAccessInfo updatedAccessSectionInfo =
+ ProjectAccessInfo projectAccessInfo =
gApi.projects().name(allProjects.get()).access(accessInput);
- assertThat(
- updatedAccessSectionInfo
- .local
- .get(AccessSection.GLOBAL_CAPABILITIES)
- .permissions
- .keySet())
- .containsAllIn(accessSectionInfo.permissions.keySet());
+
+ Set<String> capabilities =
+ projectAccessInfo.local.get(AccessSection.GLOBAL_CAPABILITIES).permissions.keySet();
+ assertThat(capabilities).contains(pluginCapability);
+ // Verifies the plugin defined capability could be listed.
+ assertThat(pluginPermissionsUtil.collectPluginCapabilities()).containsKey(pluginCapability);
+ }
+
+ @Test
+ public void setAccessAddPluginProjectPermissionSucceed() throws Exception {
+ String pluginProjectPermission =
+ "plugin-" + TEST_PLUGIN_NAME + "-" + TEST_PLUGIN_PROJECT_PERMISSION;
+ String accessSection = "refs/heads/plugin-permission";
+ ProjectAccessInput accessInput = createAccessInput(accessSection, pluginProjectPermission);
+
+ ProjectAccessInfo projectAccessInfo =
+ gApi.projects().name(allProjects.get()).access(accessInput);
+
+ Set<String> permissions = projectAccessInfo.local.get(accessSection).permissions.keySet();
+ assertThat(permissions).contains(pluginProjectPermission);
+ // Verifies the plugin defined capability could be listed.
+ assertThat(pluginPermissionsUtil.collectPluginProjectPermissions())
+ .containsKey(pluginProjectPermission);
+ }
+
+ private static ProjectAccessInput createAccessInput(String accessSection, String permissionName) {
+ ProjectAccessInput accessInput = new ProjectAccessInput();
+ PermissionRuleInfo ruleInfo = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ PermissionInfo email = new PermissionInfo(null, null);
+ email.rules = ImmutableMap.of(SystemGroupBackend.REGISTERED_USERS.get(), ruleInfo);
+ AccessSectionInfo accessSectionInfo = new AccessSectionInfo();
+ accessSectionInfo.permissions = ImmutableMap.of(permissionName, email);
+ accessInput.add = ImmutableMap.of(accessSection, accessSectionInfo);
+
+ return accessInput;
}
}
diff --git a/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java b/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
index 081a2f7..30fabdc 100644
--- a/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
+++ b/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
@@ -16,9 +16,11 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
@@ -44,8 +46,18 @@
@Override
protected void configure() {
DynamicMap.mapOf(binder(), CapabilityDefinition.class);
+ DynamicMap.mapOf(binder(), PluginProjectPermissionDefinition.class);
bind(CapabilityDefinition.class)
- .annotatedWith(Exports.named("printHello"))
+ .annotatedWith(Exports.named("foo"))
+ .toInstance(
+ new CapabilityDefinition() {
+ @Override
+ public String getDescription() {
+ return "Print Hello";
+ }
+ });
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named("bar"))
.toInstance(
new CapabilityDefinition() {
@Override
@@ -69,10 +81,11 @@
assertThat(m.get(id).name).isNotNull();
}
- String pluginCapability = "gerrit-printHello";
- assertThat(m).containsKey(pluginCapability);
- assertThat(m.get(pluginCapability).id).isEqualTo(pluginCapability);
- assertThat(m.get(pluginCapability).name).isEqualTo("Print Hello");
+ for (String pluginCapability : ImmutableSet.of("gerrit-foo", "gerrit-bar")) {
+ assertThat(m).containsKey(pluginCapability);
+ assertThat(m.get(pluginCapability).id).isEqualTo(pluginCapability);
+ assertThat(m.get(pluginCapability).name).isEqualTo("Print Hello");
+ }
}
@Singleton
diff --git a/javatests/com/google/gerrit/server/permissions/PluginPermissionsUtilTest.java b/javatests/com/google/gerrit/server/permissions/PluginPermissionsUtilTest.java
new file mode 100644
index 0000000..85d85fb
--- /dev/null
+++ b/javatests/com/google/gerrit/server/permissions/PluginPermissionsUtilTest.java
@@ -0,0 +1,56 @@
+// 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.permissions;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isPluginPermission;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+/** Small tests for {@link PluginPermissionsUtil}. */
+public final class PluginPermissionsUtilTest {
+
+ @Test
+ public void isPluginPermissionReturnsTrueForValidName() {
+ // "-" is allowed for a plugin name. Here "foo-a" should be the name of the plugin.
+ ImmutableList<String> validPluginPermissions =
+ ImmutableList.of("plugin-foo-a", "plugin-foo-a-b");
+
+ for (String permission : validPluginPermissions) {
+ assertThat(isPluginPermission(permission))
+ .named("valid plugin permission: %s", permission)
+ .isTrue();
+ }
+ }
+
+ @Test
+ public void isPluginPermissionReturnsFalseForInvalidName() {
+ ImmutableList<String> inValidPluginPermissions =
+ ImmutableList.of(
+ "create",
+ "label-Code-Review",
+ "plugin-foo",
+ "plugin-foo",
+ "plugin-foo-a-",
+ "plugin-foo-a1");
+
+ for (String permission : inValidPluginPermissions) {
+ assertThat(isPluginPermission(permission))
+ .named("invalid plugin permission: %s", permission)
+ .isFalse();
+ }
+ }
+}
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/behaviors/gr-access-behavior/gr-access-behavior.html b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html
index fb0c685..e949a87 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
@@ -120,6 +120,10 @@
id: 'submitAs',
name: 'Submit (On Behalf Of)',
},
+ toggleWipState: {
+ id: 'toggleWipState',
+ name: 'Toggle Work In Progress State',
+ },
viewDrafts: {
id: 'viewDrafts',
name: 'View Drafts',
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 6b6e90b..278875e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -32,6 +32,7 @@
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-confirm-abandon-dialog/gr-confirm-abandon-dialog.html">
<link rel="import" href="../gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html">
+<link rel="import" href="../gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.html">
<link rel="import" href="../gr-confirm-move-dialog/gr-confirm-move-dialog.html">
<link rel="import" href="../gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html">
<link rel="import" href="../gr-confirm-revert-dialog/gr-confirm-revert-dialog.html">
@@ -187,6 +188,15 @@
on-cancel="_handleConfirmDialogCancel"
project="[[change.project]]"
hidden></gr-confirm-cherrypick-dialog>
+ <gr-confirm-cherrypick-conflict-dialog id="confirmCherrypickConflict"
+ class="confirmDialog"
+ change-status="[[changeStatus]]"
+ commit-message="[[commitMessage]]"
+ commit-num="[[commitNum]]"
+ on-confirm="_handleCherrypickConflictConfirm"
+ on-cancel="_handleConfirmDialogCancel"
+ project="[[change.project]]"
+ hidden></gr-confirm-cherrypick-conflict-dialog>
<gr-confirm-move-dialog id="confirmMove"
class="confirmDialog"
on-confirm="_handleMoveConfirm"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 368b11b..3e5c4f7 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -991,6 +991,14 @@
},
_handleCherrypickConfirm() {
+ this._handleCherryPickRestApi(false);
+ },
+
+ _handleCherrypickConflictConfirm() {
+ this._handleCherryPickRestApi(true);
+ },
+
+ _handleCherryPickRestApi(conflicts) {
const el = this.$.confirmCherrypick;
if (!el.branch) {
// TODO(davido): Fix error handling
@@ -1011,6 +1019,7 @@
destination: el.branch,
base: el.baseCommit ? el.baseCommit : null,
message: el.message,
+ allow_conflicts: conflicts,
}
);
},
@@ -1113,8 +1122,9 @@
_fireAction(endpoint, action, revAction, opt_payload) {
const cleanupFn =
this._setLoadingOnButtonWithKey(action.__type, action.__key);
- this._send(action.method, opt_payload, endpoint, revAction, cleanupFn)
- .then(this._handleResponse.bind(this, action));
+
+ this._send(action.method, opt_payload, endpoint, revAction, cleanupFn,
+ action).then(this._handleResponse.bind(this, action));
},
_showActionDialog(dialog) {
@@ -1171,7 +1181,14 @@
});
},
- _handleResponseError(response) {
+ _handleResponseError(action, response, body) {
+ if (action && action.__key === RevisionActions.CHERRYPICK) {
+ if (response && response.status === 409 &&
+ body && !body.allow_conflicts) {
+ return this._showActionDialog(
+ this.$.confirmCherrypickConflict);
+ }
+ }
return response.text().then(errText => {
this.fire('show-error',
{message: `Could not perform action: ${errText}`});
@@ -1187,13 +1204,12 @@
* @param {string} actionEndpoint
* @param {boolean} revisionAction
* @param {?Function} cleanupFn
- * @param {?Function=} opt_errorFn
+ * @param {!Object|undefined} action
*/
- _send(method, payload, actionEndpoint, revisionAction, cleanupFn,
- opt_errorFn) {
+ _send(method, payload, actionEndpoint, revisionAction, cleanupFn, action) {
const handleError = response => {
cleanupFn.call(this);
- this._handleResponseError(response);
+ this._handleResponseError(action, response, payload);
};
return this.fetchChangeUpdates(this.change, this.$.restAPI)
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 1e63d53..1d83819 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -599,6 +599,38 @@
destination: 'master',
base: null,
message: 'foo message',
+ allow_conflicts: false,
+ },
+ ]);
+ });
+
+ test('cherry pick even with conflicts', () => {
+ element._handleCherrypickTap();
+ const action = {
+ __key: 'cherrypick',
+ __type: 'revision',
+ __primary: false,
+ enabled: true,
+ label: 'Cherry pick',
+ method: 'POST',
+ title: 'Cherry pick change to a different branch',
+ };
+
+ element.$.confirmCherrypick.branch = 'master';
+
+ // Add attributes that are used to determine the message.
+ element.$.confirmCherrypick.commitMessage = 'foo message';
+ element.$.confirmCherrypick.changeStatus = 'OPEN';
+ element.$.confirmCherrypick.commitNum = '123';
+
+ element._handleCherrypickConflictConfirm();
+
+ assert.deepEqual(fireActionStub.lastCall.args, [
+ '/cherrypick', action, true, {
+ destination: 'master',
+ base: null,
+ message: 'foo message',
+ allow_conflicts: true,
},
]);
});
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-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.html
new file mode 100644
index 0000000..cd196ec
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.html
@@ -0,0 +1,51 @@
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
+
+<dom-module id="gr-confirm-cherrypick-conflict-dialog">
+ <template>
+ <style include="shared-styles">
+ :host {
+ display: block;
+ }
+ :host([disabled]) {
+ opacity: .5;
+ pointer-events: none;
+ }
+ .main {
+ display: flex;
+ flex-direction: column;
+ width: 100%;
+ }
+ </style>
+ <gr-dialog
+ confirm-label="Continue"
+ on-confirm="_handleConfirmTap"
+ on-cancel="_handleCancelTap">
+ <div class="header" slot="header">Cherry Pick Conflict!</div>
+ <div class="main" slot="main">
+ <span>Cherry Pick failed! (merge conflicts)</span>
+
+ <span>Please select "Continue" to continue with conflicts or select "cancel" to close the dialog.</span>
+ </div>
+ </gr-dialog>
+ </template>
+ <script src="gr-confirm-cherrypick-conflict-dialog.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.js
new file mode 100644
index 0000000..355aa78
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog.js
@@ -0,0 +1,45 @@
+/**
+ * @license
+ * Copyright (C) 2018 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.
+ */
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-confirm-cherrypick-conflict-dialog',
+
+ /**
+ * Fired when the confirm button is pressed.
+ *
+ * @event confirm
+ */
+
+ /**
+ * Fired when the cancel button is pressed.
+ *
+ * @event cancel
+ */
+
+ _handleConfirmTap(e) {
+ e.preventDefault();
+ this.fire('confirm', null, {bubbles: false});
+ },
+
+ _handleCancelTap(e) {
+ e.preventDefault();
+ this.fire('cancel', null, {bubbles: false});
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog_test.html
new file mode 100644
index 0000000..77b102c
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog_test.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-confirm-cherrypick-conflict-dialog</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-confirm-cherrypick-conflict-dialog.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-confirm-cherrypick-conflict-dialog></gr-confirm-cherrypick-conflict-dialog>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-confirm-cherrypick-conflict-dialog tests', () => {
+ let element;
+ let sandbox;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ });
+
+ teardown(() => { sandbox.restore(); });
+
+ test('_handleConfirmTap', () => {
+ const confirmHandler = sandbox.stub();
+ element.addEventListener('confirm', confirmHandler);
+ sandbox.stub(element, '_handleConfirmTap');
+ element.$$('gr-dialog').fire('confirm');
+ assert.isTrue(confirmHandler.called);
+ assert.isTrue(element._handleConfirmTap.called);
+ });
+
+ test('_handleCancelTap', () => {
+ const cancelHandler = sandbox.stub();
+ element.addEventListener('cancel', cancelHandler);
+ sandbox.stub(element, '_handleCancelTap');
+ element.$$('gr-dialog').fire('cancel');
+ assert.isTrue(cancelHandler.called);
+ assert.isTrue(element._handleCancelTap.called);
+ });
+ });
+</script>
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/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
index 6699bd1..17f883f 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
@@ -65,6 +65,7 @@
'is:reviewed',
'is:reviewer',
'is:starred',
+ 'is:submittable',
'is:watched',
'is:wip',
'label:',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 2227b45..29cc3b9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -80,12 +80,6 @@
*/
/**
- * Fired when the diff is rendered.
- *
- * @event render
- */
-
- /**
* Fired when the diff finishes rendering text content, but not
* necessarily syntax highlights.
*
@@ -169,8 +163,6 @@
return this.$.syntaxLayer.process().then(() => {
reporting.timeEnd(TimingLabel.SYNTAX);
reporting.timeEnd(TimingLabel.TOTAL);
- this.dispatchEvent(
- new CustomEvent('render', {bubbles: true}));
});
});
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 0fc4748..67d8c19 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -850,14 +850,13 @@
assert.strictEqual(sections[1], section[1]);
});
- test('render-start and render are fired', done => {
+ test('render-start and render-content are fired', done => {
const dispatchEventStub = sandbox.stub(element, 'dispatchEvent');
element.render(keyLocations, {}).then(() => {
const firedEventTypes = dispatchEventStub.getCalls()
.map(c => { return c.args[0].type; });
assert.include(firedEventTypes, 'render-start');
assert.include(firedEventTypes, 'render-content');
- assert.include(firedEventTypes, 'render');
done();
});
});
@@ -869,10 +868,6 @@
test('rendering large diff disables syntax', done => {
// Before it renders, set the first diff line to 500 '*' characters.
element.diff.content[0].a = [new Array(501).join('*')];
- element.addEventListener('render', () => {
- assert.isFalse(element.$.syntaxLayer.enabled);
- done();
- });
const prefs = {
line_length: 10,
show_tabs: true,
@@ -880,7 +875,10 @@
context: -1,
syntax_highlighting: true,
};
- element.render(keyLocations, prefs);
+ element.render(keyLocations, prefs).then(() => {
+ assert.isFalse(element.$.syntaxLayer.enabled);
+ done();
+ });
});
test('cancel', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index d5fb878..5ecc2fc3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -116,11 +116,18 @@
* @event show-auth-required
*/
- /**
- * Fired when a comment is created
- *
- * @event create-comment
- */
+ /**
+ * Fired when a comment is created
+ *
+ * @event create-comment
+ */
+
+ /**
+ * Fired when rendering, including syntax highlighting, is done. Also fired
+ * when no rendering can be done because required preferences are not set.
+ *
+ * @event render
+ */
properties: {
changeNum: String,
@@ -700,7 +707,11 @@
this._showWarning = false;
const keyLocations = this._computeKeyLocations();
- this.$.diffBuilder.render(keyLocations, this._getBypassPrefs());
+ this.$.diffBuilder.render(keyLocations, this._getBypassPrefs())
+ .then(() => {
+ this.dispatchEvent(
+ new CustomEvent('render', {bubbles: true}));
+ });
},
_handleRenderContent() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 300807c..849ad0d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -943,7 +943,8 @@
setup(() => {
element = fixture('basic');
element.prefs = {};
- renderStub = sandbox.stub(element.$.diffBuilder, 'render');
+ renderStub = sandbox.stub(element.$.diffBuilder, 'render')
+ .returns(new Promise(() => {}));
});
test('lineOfInterest is a key location', () => {
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index 533136c..0301a13 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -42,6 +42,7 @@
safeTypesBridge: Gerrit.SafeTypes.safeTypesBridge,
});
</script>
+<script src="../bower_components/moment/moment.js"></script>
<link rel="import" href="../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
index 1090fea..481dd2f 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
@@ -20,7 +20,6 @@
<link rel="import" href="../gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
-<script src="../../../bower_components/moment/moment.js"></script>
<script src="../../../scripts/util.js"></script>
<dom-module id="gr-date-formatter">
diff --git a/polygerrit-ui/app/test/common-test-setup.html b/polygerrit-ui/app/test/common-test-setup.html
index 92b99e3..c5979fa 100644
--- a/polygerrit-ui/app/test/common-test-setup.html
+++ b/polygerrit-ui/app/test/common-test-setup.html
@@ -60,3 +60,4 @@
<link rel="import"
href="../bower_components/iron-test-helpers/iron-test-helpers.html" />
<link rel="import" href="test-router.html" />
+<script src="../bower_components/moment/moment.js"></script>
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 3b91650..32bd396 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -71,6 +71,7 @@
'change/gr-commit-info/gr-commit-info_test.html',
'change/gr-confirm-abandon-dialog/gr-confirm-abandon-dialog_test.html',
'change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.html',
+ 'change/gr-confirm-cherrypick-conflict-dialog/gr-confirm-cherrypick-conflict-dialog_test.html',
'change/gr-confirm-move-dialog/gr-confirm-move-dialog_test.html',
'change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html',
'change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html',
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}
diff --git a/resources/com/google/gerrit/server/mail/Abandoned.soy b/resources/com/google/gerrit/server/mail/Abandoned.soy
index 623cfe26..2785ffc 100644
--- a/resources/com/google/gerrit/server/mail/Abandoned.soy
+++ b/resources/com/google/gerrit/server/mail/Abandoned.soy
@@ -19,12 +19,12 @@
/**
* .Abandoned template will determine the contents of the email related to a
* change being abandoned.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Abandoned kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has abandoned this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/AbandonedHtml.soy b/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
index 75d940f..9ad996e 100644
--- a/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param coverLetter
- * @param email
- * @param fromName
- */
{template .AbandonedHtml}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>abandoned</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/AddKey.soy b/resources/com/google/gerrit/server/mail/AddKey.soy
index be76aee..8b609cf 100644
--- a/resources/com/google/gerrit/server/mail/AddKey.soy
+++ b/resources/com/google/gerrit/server/mail/AddKey.soy
@@ -19,9 +19,9 @@
/**
* The .AddKey template will determine the contents of the email related to
* adding a new SSH or GPG key to an account.
- * @param email
*/
{template .AddKey kind="text"}
+ {@param email: ?}
One or more new {$email.keyType} keys have been added to Gerrit Code Review at
{sp}{$email.gerritHost}:
diff --git a/resources/com/google/gerrit/server/mail/AddKeyHtml.soy b/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
index 04a0635..ed4f435 100644
--- a/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
+++ b/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
@@ -16,10 +16,8 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- */
{template .AddKeyHtml}
+ {@param email: ?}
<p>
One or more new {$email.keyType} keys have been added to Gerrit Code Review
at {$email.gerritHost}:
diff --git a/resources/com/google/gerrit/server/mail/ChangeFooter.soy b/resources/com/google/gerrit/server/mail/ChangeFooter.soy
index f1d201b..a8170ca 100644
--- a/resources/com/google/gerrit/server/mail/ChangeFooter.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeFooter.soy
@@ -19,9 +19,9 @@
/**
* The .ChangeFooter template will determine the contents of the footer text
* that will be appended to ALL emails related to changes.
- * @param email
*/
{template .ChangeFooter kind="text"}
+ {@param email: ?}
--{sp}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy b/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
index f802366..55e6ef5 100644
--- a/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param change
- * @param email
- */
{template .ChangeFooterHtml}
+ {@param change: ?}
+ {@param email: ?}
{if $email.changeUrl or $email.settingsUrl}
<p>
{if $email.changeUrl}
diff --git a/resources/com/google/gerrit/server/mail/ChangeSubject.soy b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
index 48ec9a2..7fcd213 100644
--- a/resources/com/google/gerrit/server/mail/ChangeSubject.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
@@ -19,13 +19,13 @@
/**
* The .ChangeSubject template will determine the contents of the email subject
* line for ALL emails related to changes.
- * @param branch
- * @param change
- * @param shortProjectName
- * @param instanceAndProjectName
- * @param addInstanceNameInSubject boolean
*/
{template .ChangeSubject kind="text"}
+ {@param branch: ?}
+ {@param change: ?}
+ {@param shortProjectName: ?}
+ {@param instanceAndProjectName: ?}
+ {@param addInstanceNameInSubject: ?} /** boolean */
{if not $addInstanceNameInSubject}
Change in {$shortProjectName}[{$branch.shortName}]: {$change.shortSubject}
{else}
diff --git a/resources/com/google/gerrit/server/mail/Comment.soy b/resources/com/google/gerrit/server/mail/Comment.soy
index f9a11cd..1eb016b 100644
--- a/resources/com/google/gerrit/server/mail/Comment.soy
+++ b/resources/com/google/gerrit/server/mail/Comment.soy
@@ -19,13 +19,13 @@
/**
* The .Comment template will determine the contents of the email related to a
* user submitting comments on changes.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
- * @param commentFiles
*/
{template .Comment kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param commentFiles: ?}
{$fromName} has posted comments on this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/CommentHtml.soy b/resources/com/google/gerrit/server/mail/CommentHtml.soy
index d554258..534cbdb 100644
--- a/resources/com/google/gerrit/server/mail/CommentHtml.soy
+++ b/resources/com/google/gerrit/server/mail/CommentHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param commentFiles
- * @param commentCount
- * @param email
- * @param labels
- * @param patchSet
- * @param patchSetCommentBlocks
- */
{template .CommentHtml}
+ {@param commentFiles: ?}
+ {@param commentCount: ?}
+ {@param email: ?}
+ {@param labels: ?}
+ {@param patchSet: ?}
+ {@param patchSetCommentBlocks: ?}
{let $commentHeaderStyle kind="css"}
margin-bottom: 4px;
{/let}
diff --git a/resources/com/google/gerrit/server/mail/DeleteReviewer.soy b/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
index 065348a..3310249 100644
--- a/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
@@ -19,12 +19,12 @@
/**
* The .DeleteReviewer template will determine the contents of the email related
* to removal of a reviewer (and the reviewer's votes) from reviews.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .DeleteReviewer kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has removed{sp}
{for $reviewerName in $email.reviewerNames}
{if not isFirst($reviewerName)},{sp}{/if}
diff --git a/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy b/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
index 0599b52..54720fe 100644
--- a/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .DeleteReviewerHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName}{sp}
<strong>
diff --git a/resources/com/google/gerrit/server/mail/DeleteVote.soy b/resources/com/google/gerrit/server/mail/DeleteVote.soy
index 724e90d..d869205 100644
--- a/resources/com/google/gerrit/server/mail/DeleteVote.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteVote.soy
@@ -19,11 +19,11 @@
/**
* The .DeleteVote template will determine the contents of the email related
* to removing votes on changes.
- * @param change
- * @param coverLetter
- * @param fromName
*/
{template .DeleteVote kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param fromName: ?}
{$fromName} has removed a vote on this change.{\n}
{\n}
Change subject: {$change.subject}{\n}
diff --git a/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy b/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
index cb8162d..3a82927 100644
--- a/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param coverLetter
- * @param email
- * @param fromName
- */
{template .DeleteVoteHtml}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>removed a vote</strong> from this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/Footer.soy b/resources/com/google/gerrit/server/mail/Footer.soy
index e1890a8..7483cd9 100644
--- a/resources/com/google/gerrit/server/mail/Footer.soy
+++ b/resources/com/google/gerrit/server/mail/Footer.soy
@@ -20,9 +20,9 @@
* The .Footer template will determine the contents of the footer text
* appended to the end of all outgoing emails after the ChangeFooter and
* CommentFooter.
- * @param footers
*/
{template .Footer kind="text"}
+ {@param footers: ?}
{for $footer in $footers}
{$footer}{\n}
{/for}
diff --git a/resources/com/google/gerrit/server/mail/FooterHtml.soy b/resources/com/google/gerrit/server/mail/FooterHtml.soy
index 938655c..ce934d3 100644
--- a/resources/com/google/gerrit/server/mail/FooterHtml.soy
+++ b/resources/com/google/gerrit/server/mail/FooterHtml.soy
@@ -16,10 +16,8 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param footers
- */
{template .FooterHtml}
+ {@param footers: ?}
{\n}
{\n}
{for $footer in $footers}
diff --git a/resources/com/google/gerrit/server/mail/Merged.soy b/resources/com/google/gerrit/server/mail/Merged.soy
index 40924e6..04d54c4 100644
--- a/resources/com/google/gerrit/server/mail/Merged.soy
+++ b/resources/com/google/gerrit/server/mail/Merged.soy
@@ -20,11 +20,11 @@
/**
* The .Merged template will determine the contents of the email related to
* a change successfully merged to the head.
- * @param change
- * @param email
- * @param fromName
*/
{template .Merged kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has submitted this change and it was merged.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/MergedHtml.soy b/resources/com/google/gerrit/server/mail/MergedHtml.soy
index b11c5e5..e8c04a5 100644
--- a/resources/com/google/gerrit/server/mail/MergedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/MergedHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- */
{template .MergedHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>merged</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/NewChange.soy b/resources/com/google/gerrit/server/mail/NewChange.soy
index f11edfe..84a3075 100644
--- a/resources/com/google/gerrit/server/mail/NewChange.soy
+++ b/resources/com/google/gerrit/server/mail/NewChange.soy
@@ -19,13 +19,13 @@
/**
* The .NewChange template will determine the contents of the email related to a
* user submitting a new change for review.
- * @param change
- * @param email
- * @param ownerName
- * @param patchSet
- * @param projectName
*/
{template .NewChange kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param ownerName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
{if $email.reviewerNames}
Hello{sp}
{for $reviewerName in $email.reviewerNames}
diff --git a/resources/com/google/gerrit/server/mail/NewChangeHtml.soy b/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
index 5bce806..9de8707 100644
--- a/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
+++ b/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- * @param ownerName
- * @param patchSet
- * @param projectName
- */
{template .NewChangeHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param ownerName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{if $email.reviewerNames}
{$fromName} would like{sp}
diff --git a/resources/com/google/gerrit/server/mail/Private.soy b/resources/com/google/gerrit/server/mail/Private.soy
index bb32a7e9..510f15e 100644
--- a/resources/com/google/gerrit/server/mail/Private.soy
+++ b/resources/com/google/gerrit/server/mail/Private.soy
@@ -22,17 +22,17 @@
/**
* Private template to generate "View Change" buttons.
- * @param email
*/
{template .ViewChangeButton}
+ {@param email: ?}
<a href="{$email.changeUrl}">View Change</a>
{/template}
/**
* Private template to render PRE block with consistent font-sizing.
- * @param content
*/
{template .Pre}
+ {@param content: ?}
{let $preStyle kind="css"}
font-family: monospace,monospace; // Use this to avoid browsers scaling down
// monospace text.
@@ -53,10 +53,9 @@
*
* This mechanism encodes as little structure as possible in order to depend on
* the Soy autoescape mechanism for all of the content.
- *
- * @param content
*/
{template .WikiFormat}
+ {@param content: ?}
{let $blockquoteStyle kind="css"}
border-left: 1px solid #aaa;
margin: 10px 0;
@@ -87,10 +86,8 @@
{/for}
{/template}
-/**
- * @param diffLines
- */
{template .UnifiedDiff}
+ {@param diffLines: ?}
{let $addStyle kind="css"}
color: hsl(120, 100%, 40%);
{/let}
diff --git a/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy b/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
index 2886cc0..ee03de0 100644
--- a/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
+++ b/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
@@ -19,9 +19,9 @@
/**
* The .RegisterNewEmail template will determine the contents of the email
* related to registering new email accounts.
- * @param email
*/
{template .RegisterNewEmail kind="text"}
+ {@param email: ?}
Welcome to Gerrit Code Review at {$email.gerritHost}.{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy b/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
index 1cb0110..bb84cf1 100644
--- a/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
+++ b/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
@@ -19,14 +19,14 @@
/**
* The .ReplacePatchSet template will determine the contents of the email
* related to a user submitting a new patchset for a change.
- * @param change
- * @param email
- * @param fromEmail
- * @param fromName
- * @param patchSet
- * @param projectName
*/
{template .ReplacePatchSet kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromEmail: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
{if $email.reviewerNames and $fromEmail == $change.ownerEmail}
Hello{sp}
{for $reviewerName in $email.reviewerNames}
diff --git a/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy b/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
index e618bef..96cba5f 100644
--- a/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
+++ b/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param change
- * @param email
- * @param fromName
- * @param fromEmail
- * @param patchSet
- * @param projectName
- */
{template .ReplacePatchSetHtml}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param fromEmail: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{$fromName} <strong>uploaded patch set #{$patchSet.patchSetId}</strong>{sp}
to{sp}
diff --git a/resources/com/google/gerrit/server/mail/Restored.soy b/resources/com/google/gerrit/server/mail/Restored.soy
index 4fc6d8c..0ec65b30 100644
--- a/resources/com/google/gerrit/server/mail/Restored.soy
+++ b/resources/com/google/gerrit/server/mail/Restored.soy
@@ -19,12 +19,12 @@
/**
* The .Restored template will determine the contents of the email related to a
* change being restored.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Restored kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has restored this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/RestoredHtml.soy b/resources/com/google/gerrit/server/mail/RestoredHtml.soy
index bb856ac..bcd358f 100644
--- a/resources/com/google/gerrit/server/mail/RestoredHtml.soy
+++ b/resources/com/google/gerrit/server/mail/RestoredHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .RestoredHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>restored</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/Reverted.soy b/resources/com/google/gerrit/server/mail/Reverted.soy
index fba8744..32a65c6 100644
--- a/resources/com/google/gerrit/server/mail/Reverted.soy
+++ b/resources/com/google/gerrit/server/mail/Reverted.soy
@@ -19,12 +19,12 @@
/**
* The .Reverted template will determine the contents of the email related
* to a change being reverted.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Reverted kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has created a revert of this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/RevertedHtml.soy b/resources/com/google/gerrit/server/mail/RevertedHtml.soy
index b7b254e..69260ad 100644
--- a/resources/com/google/gerrit/server/mail/RevertedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/RevertedHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .RevertedHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} has <strong>created a revert</strong> of this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/SetAssignee.soy b/resources/com/google/gerrit/server/mail/SetAssignee.soy
index 98290e9..1fdf690 100644
--- a/resources/com/google/gerrit/server/mail/SetAssignee.soy
+++ b/resources/com/google/gerrit/server/mail/SetAssignee.soy
@@ -19,13 +19,13 @@
/**
* The .SetAssignee template will determine the contents of the email related
* to a user being assigned to a change.
- * @param change
- * @param email
- * @param fromName
- * @param patchSet
- * @param projectName
*/
{template .SetAssignee kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
Hello{sp}
{$email.assigneeName},
diff --git a/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy b/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
index dbd3fae..1826314 100644
--- a/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
+++ b/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
@@ -16,14 +16,12 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- * @param patchSet
- * @param projectName
- */
{template .SetAssigneeHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{$fromName} has <strong>assigned</strong> a change to{sp}
{$email.assigneeName}.{sp}
diff --git a/tools/bzl/classpath.bzl b/tools/bzl/classpath.bzl
index f997fcf..0d43be7 100644
--- a/tools/bzl/classpath.bzl
+++ b/tools/bzl/classpath.bzl
@@ -1,14 +1,14 @@
def _classpath_collector(ctx):
- all = depset()
+ all = []
for d in ctx.attr.deps:
if hasattr(d, "java"):
- all += d.java.transitive_runtime_deps
+ all.append(d.java.transitive_runtime_deps)
if hasattr(d.java.compilation_info, "runtime_classpath"):
- all += d.java.compilation_info.runtime_classpath
+ all.append(d.java.compilation_info.runtime_classpath)
elif hasattr(d, "files"):
- all += d.files
+ all.append(d.files)
- as_strs = [c.path for c in all.to_list()]
+ as_strs = [c.path for c in depset(transitive = all).to_list()]
ctx.actions.write(
output = ctx.outputs.runtime,
content = "\n".join(sorted(as_strs)),
diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl
index d315f8f..fcf9f33 100644
--- a/tools/bzl/javadoc.bzl
+++ b/tools/bzl/javadoc.bzl
@@ -17,13 +17,10 @@
def _impl(ctx):
zip_output = ctx.outputs.zip
- transitive_jar_set = depset()
- source_jars = depset()
- for l in ctx.attr.libs:
- source_jars += l.java.source_jars
- transitive_jar_set += l.java.transitive_deps
+ transitive_jars = depset(transitive = [l.java.transitive_deps for l in ctx.attr.libs])
+ source_jars = depset(transitive = [l.java.source_jars for l in ctx.attr.libs])
- transitive_jar_paths = [j.path for j in transitive_jar_set.to_list()]
+ transitive_jar_paths = [j.path for j in transitive_jars.to_list()]
dir = ctx.outputs.zip.path + ".dir"
source = ctx.outputs.zip.path + ".source"
external_docs = ["http://docs.oracle.com/javase/8/docs/api"] + ctx.attr.external_docs
@@ -56,7 +53,7 @@
"(cd %s && zip -Xqr ../%s *)" % (dir, ctx.outputs.zip.basename),
]
ctx.actions.run_shell(
- inputs = transitive_jar_set.to_list() + source_jars.to_list() + ctx.files._jdk,
+ inputs = transitive_jars.to_list() + source_jars.to_list() + ctx.files._jdk,
outputs = [zip_output],
command = " && ".join(cmd),
)
diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl
index 19d4436..a7714a1 100644
--- a/tools/bzl/js.bzl
+++ b/tools/bzl/js.bzl
@@ -131,20 +131,20 @@
)
def _bower_component_impl(ctx):
- transitive_zipfiles = depset([ctx.file.zipfile])
- for d in ctx.attr.deps:
- transitive_zipfiles += d.transitive_zipfiles
+ transitive_zipfiles = depset(
+ direct = [ctx.file.zipfile],
+ transitive = [d.transitive_zipfiles for d in ctx.attr.deps],
+ )
- transitive_licenses = depset()
- if ctx.file.license:
- transitive_licenses += depset([ctx.file.license])
+ transitive_licenses = depset(
+ direct = [ctx.file.license],
+ transitive = [d.transitive_licenses for d in ctx.attr.deps],
+ )
- for d in ctx.attr.deps:
- transitive_licenses += d.transitive_licenses
-
- transitive_versions = depset(ctx.files.version_json)
- for d in ctx.attr.deps:
- transitive_versions += d.transitive_versions
+ transitive_versions = depset(
+ direct = ctx.files.version_json,
+ transitive = [d.transitive_versions for d in ctx.attr.deps],
+ )
return struct(
transitive_licenses = transitive_licenses,
@@ -183,12 +183,12 @@
mnemonic = "GenBowerZip",
)
- licenses = depset()
+ licenses = []
if ctx.file.license:
- licenses += depset([ctx.file.license])
+ licenses.append(ctx.file.license)
return struct(
- transitive_licenses = licenses,
+ transitive_licenses = depset(licenses),
transitive_versions = depset(),
transitive_zipfiles = list([ctx.outputs.zip]),
)
@@ -233,15 +233,16 @@
"""A bunch of bower components zipped up."""
zips = depset()
for d in ctx.attr.deps:
- zips += d.transitive_zipfiles
+ files = d.transitive_zipfiles
- versions = depset()
- for d in ctx.attr.deps:
- versions += d.transitive_versions
+ # TODO(davido): Make sure the field always contains a depset
+ if type(files) == "list":
+ files = depset(files)
+ zips = depset(transitive = [zips, files])
- licenses = depset()
- for d in ctx.attr.deps:
- licenses += d.transitive_versions
+ versions = depset(transitive = [d.transitive_versions for d in ctx.attr.deps])
+
+ licenses = depset(transitive = [d.transitive_versions for d in ctx.attr.deps])
out_zip = ctx.outputs.zip
out_versions = ctx.outputs.version_json
@@ -299,11 +300,7 @@
# intermediate artifact if split is wanted.
if ctx.attr.split:
- bundled = ctx.new_file(
- ctx.configuration.genfiles_dir,
- ctx.outputs.html,
- ".bundled.html",
- )
+ bundled = ctx.actions.declare_file(ctx.outputs.html.path + ".bundled.html")
else:
bundled = ctx.outputs.html
destdir = ctx.outputs.html.path + ".dir"
diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl
index 1dd6d7e..a480908 100644
--- a/tools/bzl/pkg_war.bzl
+++ b/tools/bzl/pkg_war.bzl
@@ -75,35 +75,39 @@
]
# Add lib
- transitive_lib_deps = depset()
+ transitive_libs = []
for l in ctx.attr.libs:
if hasattr(l, "java"):
- transitive_lib_deps += l.java.transitive_runtime_deps
+ transitive_libs.append(l.java.transitive_runtime_deps)
elif hasattr(l, "files"):
- transitive_lib_deps += l.files
+ transitive_libs.append(l.files)
+ transitive_lib_deps = depset(transitive = transitive_libs)
for dep in transitive_lib_deps.to_list():
cmd += _add_file(dep, build_output + "/WEB-INF/lib/")
inputs.append(dep)
# Add pgm lib
- transitive_pgmlib_deps = depset()
+ transitive_pgmlibs = []
for l in ctx.attr.pgmlibs:
- transitive_pgmlib_deps += l.java.transitive_runtime_deps
+ transitive_pgmlibs.append(l.java.transitive_runtime_deps)
+ transitive_pgmlib_deps = depset(transitive = transitive_pgmlibs)
for dep in transitive_pgmlib_deps.to_list():
if dep not in inputs:
cmd += _add_file(dep, build_output + "/WEB-INF/pgm-lib/")
inputs.append(dep)
# Add context
- transitive_context_deps = depset()
+ transitive_context_libs = []
if ctx.attr.context:
for jar in ctx.attr.context:
if hasattr(jar, "java"):
- transitive_context_deps += jar.java.transitive_runtime_deps
+ transitive_context_libs.append(jar.java.transitive_runtime_deps)
elif hasattr(jar, "files"):
- transitive_context_deps += jar.files
+ transitive_context_libs.append(jar.files)
+
+ transitive_context_deps = depset(transitive = transitive_context_libs)
for dep in transitive_context_deps.to_list():
cmd += _add_context(dep, build_output)
inputs.append(dep)
