Merge "Annotate REST interface calls with anonymized labels"
diff --git a/Documentation/cmd-index-project.txt b/Documentation/cmd-index-changes-in-project.txt
similarity index 60%
rename from Documentation/cmd-index-project.txt
rename to Documentation/cmd-index-changes-in-project.txt
index 2196a26..ec1626f 100644
--- a/Documentation/cmd-index-project.txt
+++ b/Documentation/cmd-index-changes-in-project.txt
@@ -1,12 +1,12 @@
-= gerrit index project
+= gerrit index changes in project
== NAME
-gerrit index project - Index all the changes in one or more projects.
+gerrit index changes in project - Index all the changes in one or more projects.
== SYNOPSIS
[verse]
--
-_ssh_ -p <port> <host> _gerrit index project_ <PROJECT> [<PROJECT> ...]
+_ssh_ -p <port> <host> _gerrit index changes-in-project_ <PROJECT> [<PROJECT> ...]
--
== DESCRIPTION
@@ -26,7 +26,7 @@
Index all changes in projects MyProject and NiceProject.
----
- $ ssh -p 29418 user@review.example.com gerrit index project MyProject NiceProject
+ $ ssh -p 29418 user@review.example.com gerrit index changes-in-project MyProject NiceProject
----
GERRIT
diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt
index f535281..2362401 100644
--- a/Documentation/cmd-index.txt
+++ b/Documentation/cmd-index.txt
@@ -136,7 +136,7 @@
link:cmd-index-changes.html[gerrit index changes]::
Index one or more changes.
-link:cmd-index-project.html[gerrit index project]::
+link:cmd-index-changes-in-project.html[gerrit index changes-in-project]::
Index all the changes in one or more projects.
link:cmd-logging-ls-level.html[gerrit logging ls-level]::
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 2737f58..71441d83 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2779,16 +2779,16 @@
it to 0 disables the dedicated thread pool and indexing will be done in the same
thread as the operation.
+
-If not set or set to a negative value, defaults to 1 plus half of the number of
-logical CPUs as returned by the JVM.
+If not set or set to a zero, defaults to the number of logical CPUs as returned
+by the JVM. If set to a negative value, defaults to a direct executor.
[[index.batchThreads]]index.batchThreads::
+
Number of threads to use for indexing in background operations, such as
online schema upgrades.
+
-If not set or set to a negative value, defaults to the number of logical
-CPUs as returned by the JVM.
+If not set or set to a zero, defaults to the number of logical CPUs as returned
+by the JVM. If set to a negative value, defaults to a direct executor.
[[index.onlineUpgrade]]index.onlineUpgrade::
+
@@ -3009,41 +3009,13 @@
+
Not set by default.
-[[elasticsearch.requestCompression]]elasticsearch.requestCompression::
+[[elasticsearch.maxRetryTimeout]]elasticsearch.maxRetryTimeout::
+
-Enable request compression.
-+
-Defaults to `false`.
-
-[[elasticsearch.connectionTimeout]]elasticsearch.connectionTimeout::
-+
-How long should Gerrit waits for connection.
+Sets the maximum timeout to honor in case of multiple retries of the same request.
+
The value is in the usual time-unit format like `1 m`, `5 m`.
+
-Defaults to `5 m`
-
-[[elasticsearch.maxConnectionIdleTime]]elasticsearch.maxConnectionIdleTime::
-+
-How long connection can stay in idle.
-+
-The value is in the usual time-unit format like `1 m`, `5 m`.
-+
-Defaults to `5 m`
-
-[[elasticsearch.maxTotalConnection]]elasticsearch.maxTotalConnection::
-+
-How many connections can be spawned simultaneously.
-+
-Defaults to `1`
-
-[[elasticsearch.maxReadTimeout]]elasticsearch.maxReadTimeout::
-+
-Timeout for read operations.
-+
-The value is in the usual time-unit format like `1 m`, `5 m`.
-+
-Defaults to `5 m`
+Defaults to `30000 ms`.
==== Elasticsearch server(s) configuration
@@ -3058,7 +3030,7 @@
[[elasticsearch.name.hostname]]elasticsearch.name.hostname::
+
Elasticsearch server hostname.
-
++
Defaults to `localhost`.
[[elasticsearch.name.port]]elasticsearch.name.port::
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index bc5a3c6..54f8022 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1354,6 +1354,31 @@
[[index]]
+=== Index project
+
+Adds or updates the current project (and children, if specified) in the secondary index.
+The indexing task is executed asynchronously in background, so this command
+returns immediately.
+
+As an input, a link:#index-project-input[IndexProjectInput] entity can be provided.
+
+.Request
+----
+ POST /projects/MyProject/index HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "index_children": "true"
+ }
+----
+
+.Response
+----
+ HTTP/1.1 202 Accepted
+ Content-Disposition: attachment
+----
+
+[[index.changes]]
=== Index all changes in a project
Adds or updates all the changes belonging to a project in the secondary index.
@@ -1362,7 +1387,7 @@
.Request
----
- POST /projects/MyProject/index HTTP/1.0
+ POST /projects/MyProject/index.changes HTTP/1.0
----
.Response
@@ -3128,6 +3153,17 @@
omitted.
|============================
+[[index-project-input]]
+=== IndexProjectInput
+The `IndexProjectInput` contains parameters for indexing a project.
+
+[options="header",cols="1,^2,4"]
+|================================
+|Field Name ||Description
+|`index_children` ||
+If children should be indexed recursively.
+|================================
+
[[inherited-boolean-info]]
=== InheritedBooleanInfo
A boolean value that can also be inherited.
diff --git a/WORKSPACE b/WORKSPACE
index 95dd520..51c3618 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -445,18 +445,18 @@
sha1 = "430b2fc839b5de1f3643b528853d5cf26096c1de",
)
-AUTO_VALUE_VERSION = "1.6"
+AUTO_VALUE_VERSION = "1.6.2"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "a3b1b1404f8acaa88594a017185e013cd342c9a8",
+ sha1 = "e7eae562942315a983eea3e191b72d755c153620",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "da725083ee79fdcd86d9f3d8a76e38174a01892a",
+ sha1 = "ed193d86e0af90cc2342aedbe73c5d86b03fa09b",
)
# Transitive dependency of commons-compress
@@ -906,8 +906,8 @@
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:5.6.9",
- sha1 = "895706412e2fba3f842fca82ec3dece1cb4ee7d1",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.0",
+ sha1 = "a95ef38262ef499aa07cdb736f4a47cb19162654",
)
JACKSON_VERSION = "2.8.9"
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
index e518d26..1b946cd 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
@@ -169,7 +169,7 @@
@Override
public void setValue(AccessSection value) {
- Collections.sort(value.getPermissions());
+ sortPermissions(value);
this.value = value;
this.readOnly = !editing || !(projectAccess.isOwnerOf(value) || projectAccess.canUpload());
@@ -204,6 +204,12 @@
}
}
+ private void sortPermissions(AccessSection accessSection) {
+ List<Permission> permissionList = new ArrayList<>(accessSection.getPermissions());
+ Collections.sort(permissionList);
+ accessSection.setPermissions(permissionList);
+ }
+
void setEditing(boolean editing) {
this.editing = editing;
}
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 6828393..abb28ce 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -397,6 +397,8 @@
baseConfig.setString("sshd", null, "listenAddress", "off");
}
+ baseConfig.setInt("index", null, "batchThreads", -1);
+
baseConfig.setInt("receive", null, "changeUpdateThreads", 4);
Module module = createModule();
if (classDesc.equals(methodDesc) && !classDesc.sandboxed() && !methodDesc.sandboxed()) {
@@ -1075,7 +1077,7 @@
ProjectConfig config = ProjectConfig.read(md);
AccessSection s = config.getAccessSection(ref, true);
Permission p = s.getPermission(permission, true);
- p.getRules().clear();
+ p.clearRules();
config.commit(md);
projectCache.evict(config.getProject());
}
diff --git a/java/com/google/gerrit/common/data/AccessSection.java b/java/com/google/gerrit/common/data/AccessSection.java
index 82dc620..f658066 100644
--- a/java/com/google/gerrit/common/data/AccessSection.java
+++ b/java/com/google/gerrit/common/data/AccessSection.java
@@ -34,11 +34,12 @@
super(refPattern);
}
+ // TODO(ekempin): Make this method return an ImmutableList once the GWT UI is gone.
public List<Permission> getPermissions() {
if (permissions == null) {
- permissions = new ArrayList<>();
+ return new ArrayList<>();
}
- return permissions;
+ return new ArrayList<>(permissions);
}
public void setPermissions(List<Permission> list) {
@@ -49,7 +50,7 @@
}
}
- permissions = list;
+ permissions = new ArrayList<>(list);
}
@Nullable
@@ -59,13 +60,19 @@
@Nullable
public Permission getPermission(String name, boolean create) {
- for (Permission p : getPermissions()) {
- if (p.getName().equalsIgnoreCase(name)) {
- return p;
+ if (permissions != null) {
+ for (Permission p : permissions) {
+ if (p.getName().equalsIgnoreCase(name)) {
+ return p;
+ }
}
}
if (create) {
+ if (permissions == null) {
+ permissions = new ArrayList<>();
+ }
+
Permission p = new Permission(name);
permissions.add(p);
return p;
@@ -75,7 +82,10 @@
}
public void addPermission(Permission permission) {
- List<Permission> permissions = getPermissions();
+ if (permissions == null) {
+ permissions = new ArrayList<>();
+ }
+
for (Permission p : permissions) {
if (p.getName().equalsIgnoreCase(permission.getName())) {
throw new IllegalArgumentException();
@@ -133,4 +143,15 @@
return new HashSet<>(getPermissions())
.equals(new HashSet<>(((AccessSection) obj).getPermissions()));
}
+
+ @Override
+ public int hashCode() {
+ int hashCode = super.hashCode();
+ if (permissions != null) {
+ for (Permission permission : permissions) {
+ hashCode += permission.hashCode();
+ }
+ }
+ return hashCode;
+ }
}
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index dff30d7..af23e39 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -155,13 +155,16 @@
exclusiveGroup = newExclusiveGroup;
}
+ // TODO(ekempin): Make this method return an ImmutableList once the GWT UI is gone.
public List<PermissionRule> getRules() {
- initRules();
- return rules;
+ if (rules == null) {
+ return new ArrayList<>();
+ }
+ return new ArrayList<>(rules);
}
public void setRules(List<PermissionRule> list) {
- rules = list;
+ rules = new ArrayList<>(list);
}
public void add(PermissionRule rule) {
@@ -181,6 +184,12 @@
}
}
+ public void clearRules() {
+ if (rules != null) {
+ rules.clear();
+ }
+ }
+
public PermissionRule getRule(GroupReference group) {
return getRule(group, false);
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
index 4184ec0..0148bf1 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
@@ -16,6 +16,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -29,21 +30,18 @@
@Singleton
class ElasticConfiguration {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private static final String DEFAULT_HOST = "localhost";
private static final String DEFAULT_PORT = "9200";
private static final String DEFAULT_PROTOCOL = "http";
private final Config cfg;
+ private final List<HttpHost> hosts;
- final List<HttpHost> urls;
final String username;
final String password;
- final boolean requestCompression;
- final long connectionTimeout;
- final long maxConnectionIdleTime;
- final TimeUnit maxConnectionIdleUnit = TimeUnit.MILLISECONDS;
- final int maxTotalConnection;
- final int readTimeout;
+ final int maxRetryTimeout;
final String prefix;
@Inject
@@ -51,39 +49,39 @@
this.cfg = cfg;
this.username = cfg.getString("elasticsearch", null, "username");
this.password = cfg.getString("elasticsearch", null, "password");
- this.requestCompression = cfg.getBoolean("elasticsearch", null, "requestCompression", false);
- this.connectionTimeout =
- cfg.getTimeUnit("elasticsearch", null, "connectionTimeout", 3000, TimeUnit.MILLISECONDS);
- this.maxConnectionIdleTime =
- cfg.getTimeUnit(
- "elasticsearch", null, "maxConnectionIdleTime", 3000, TimeUnit.MILLISECONDS);
- this.maxTotalConnection = cfg.getInt("elasticsearch", null, "maxTotalConnection", 1);
- this.readTimeout =
- (int) cfg.getTimeUnit("elasticsearch", null, "readTimeout", 3000, TimeUnit.MICROSECONDS);
+ this.maxRetryTimeout =
+ (int)
+ cfg.getTimeUnit("elasticsearch", null, "maxRetryTimeout", 30000, TimeUnit.MILLISECONDS);
this.prefix = Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix"));
Set<String> subsections = cfg.getSubsections("elasticsearch");
if (subsections.isEmpty()) {
HttpHost httpHost =
new HttpHost(DEFAULT_HOST, Integer.valueOf(DEFAULT_PORT), DEFAULT_PROTOCOL);
- this.urls = Collections.singletonList(httpHost);
+ this.hosts = Collections.singletonList(httpHost);
} else {
- this.urls = new ArrayList<>(subsections.size());
+ this.hosts = new ArrayList<>(subsections.size());
for (String subsection : subsections) {
String port = getString(cfg, subsection, "port", DEFAULT_PORT);
String host = getString(cfg, subsection, "hostname", DEFAULT_HOST);
String protocol = getString(cfg, subsection, "protocol", DEFAULT_PROTOCOL);
HttpHost httpHost = new HttpHost(host, Integer.valueOf(port), protocol);
- this.urls.add(httpHost);
+ this.hosts.add(httpHost);
}
}
+
+ logger.atInfo().log("Elasticsearch hosts: %s", hosts);
}
Config getConfig() {
return cfg;
}
+ HttpHost[] getHosts() {
+ return hosts.toArray(new HttpHost[hosts.size()]);
+ }
+
String getIndexName(String name, int schemaVersion) {
return String.format("%s%s_%04d", prefix, name, schemaVersion);
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
index c2c4548..db59257 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
@@ -22,7 +22,6 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import org.apache.http.HttpHost;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
import org.apache.http.auth.AuthScope;
@@ -38,18 +37,14 @@
class ElasticRestClientProvider implements Provider<RestClient>, LifecycleListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final HttpHost[] hosts;
- private final String username;
- private final String password;
+ private final ElasticConfiguration cfg;
private RestClient client;
private ElasticQueryAdapter adapter;
@Inject
ElasticRestClientProvider(ElasticConfiguration cfg) {
- hosts = cfg.urls.toArray(new HttpHost[cfg.urls.size()]);
- username = cfg.username;
- password = cfg.password;
+ this.cfg = cfg;
}
public static LifecycleModule module() {
@@ -131,12 +126,15 @@
}
private RestClient build() {
- RestClientBuilder builder = RestClient.builder(hosts);
+ RestClientBuilder builder = RestClient.builder(cfg.getHosts());
+ builder.setMaxRetryTimeoutMillis(cfg.maxRetryTimeout);
setConfiguredCredentialsIfAny(builder);
return builder.build();
}
private void setConfiguredCredentialsIfAny(RestClientBuilder builder) {
+ String username = cfg.username;
+ String password = cfg.password;
if (username != null && password != null) {
CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(
diff --git a/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java b/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java
new file mode 100644
index 0000000..a41f227
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java
@@ -0,0 +1,19 @@
+// 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.
+
+package com.google.gerrit.extensions.api.projects;
+
+public class IndexProjectInput {
+ public Boolean indexChildren;
+}
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index c9f47c2..63d40f0 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -191,6 +191,13 @@
void parent(String parent) throws RestApiException;
/**
+ * Reindex the project and children in case {@code indexChildren} is specified.
+ *
+ * @param indexChildren decides if children should be indexed recursively
+ */
+ void index(boolean indexChildren) throws RestApiException;
+
+ /**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
*/
@@ -344,5 +351,10 @@
public void parent(String parent) throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public void index(boolean indexChildren) throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java b/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
index 6b5da7c..e1c4b3b 100644
--- a/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
+++ b/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
@@ -17,17 +17,31 @@
/**
* Optional interface for {@link RestCollection}.
*
- * <p>Collections that implement this interface can accept a {@code DELETE} directly on the
- * collection itself.
+ * <p>This interface is used for 2 purposes:
+ *
+ * <ul>
+ * <li>to support {@code DELETE} directly on the collection itself
+ * <li>to support {@code DELETE} on a non-existing member of the collection (in order to create
+ * that member)
+ * </ul>
+ *
+ * <p>This interface is not supported for root collections.
*/
public interface AcceptsDelete<P extends RestResource> {
/**
- * Handle deletion of a child resource by DELETE on the collection.
+ * Handle
*
- * @param parent parent collection handle.
- * @param id id of the resource being created (optional).
- * @return a view to perform the deletion.
- * @throws RestApiException the view cannot be constructed.
+ * <ul>
+ * <li>{@code DELETE} directly on the collection itself (in this case id is {@code null})
+ * <li>{@code DELETE} on a non-existing member of the collection (in this case id is not {@code
+ * null})
+ * </ul>
+ *
+ * @param parent the collection
+ * @param id id of the non-existing collection member for which the {@code DELETE} request is
+ * done, {@code null} if the {@code DELETE} request is done on the collection itself
+ * @return a view to handle the {@code DELETE} request
+ * @throws RestApiException the view cannot be constructed
*/
RestModifyView<P, ?> delete(P parent, IdString id) throws RestApiException;
}
diff --git a/java/com/google/gerrit/extensions/restapi/RestApiModule.java b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
index e65e6e5..ee31113 100644
--- a/java/com/google/gerrit/extensions/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
@@ -75,7 +75,7 @@
return new ChildCollectionBinder<>(view(type, GET, name));
}
- protected <R extends RestResource> LinkedBindingBuilder<RestView<R>> view(
+ private <R extends RestResource> LinkedBindingBuilder<RestView<R>> view(
TypeLiteral<RestView<R>> viewType, String method, String name) {
return bind(viewType).annotatedWith(export(method, name));
}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index a2d901f..3f090a1 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -59,8 +59,6 @@
public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- public static final String MIME_TYPE = "application/pgp-keys";
-
private final DynamicMap<RestView<GpgKey>> views;
private final Provider<CurrentUser> self;
private final Provider<PublicKeyStore> storeProvider;
diff --git a/java/com/google/gerrit/httpd/raw/ResourceServlet.java b/java/com/google/gerrit/httpd/raw/ResourceServlet.java
index 64b5bbb..3244232 100644
--- a/java/com/google/gerrit/httpd/raw/ResourceServlet.java
+++ b/java/com/google/gerrit/httpd/raw/ResourceServlet.java
@@ -34,6 +34,7 @@
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.httpd.HtmlDomUtil;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.util.http.CacheHeaders;
import com.google.gwtjsonrpc.server.RPCServletUtils;
import java.io.IOException;
@@ -324,6 +325,7 @@
}
}
+ @UsedAt(UsedAt.Project.GOOGLE)
public static class Weigher implements com.google.common.cache.Weigher<Path, Resource> {
@Override
public int weigh(Path p, Resource r) {
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index 252eb61..c9ca972 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -214,14 +214,13 @@
return false;
}
- /** Returns the full commit message for the given project at the given patchset revision */
- public String getFullCommitMessage(Project.NameKey project, PatchSet patchSet)
- throws IOException {
+ /** Returns the commit for the given project at the given patchset revision */
+ public RevCommit getRevCommit(Project.NameKey project, PatchSet patchSet) throws IOException {
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
RevCommit src = rw.parseCommit(ObjectId.fromString(patchSet.getRevision().get()));
rw.parseBody(src);
- return src.getFullMessage();
+ return src;
}
}
}
diff --git a/java/com/google/gerrit/server/UsedAt.java b/java/com/google/gerrit/server/UsedAt.java
new file mode 100644
index 0000000..b564157
--- /dev/null
+++ b/java/com/google/gerrit/server/UsedAt.java
@@ -0,0 +1,45 @@
+// 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.
+
+package com.google.gerrit.server;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.common.annotations.GwtCompatible;
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * A marker for a method that is public solely because it is called from inside a project or an
+ * organisation using Gerrit.
+ */
+@BindingAnnotation
+@Target({METHOD, TYPE})
+@Retention(RUNTIME)
+@GwtCompatible
+public @interface UsedAt {
+ /** Enumeration of projects that call a method that would otherwise be private. */
+ enum Project {
+ GOOGLE,
+ PLUGIN_DELETE_PROJECT,
+ PLUGIN_SERVICEUSER,
+ PLUGINS_ALL, // Use this project if a method/type is generally made available to all plugins.
+ }
+
+ /** Reference to the project that uses the method annotated with this annotation. */
+ Project value();
+}
diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index 8a48167..e91ce49 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -95,6 +95,16 @@
return builder.build();
}
+ /**
+ * Returns the accounts with the given email.
+ *
+ * <p>This method behaves just like {@link #getAccountFor(String)}, except that accounts are not
+ * looked up by their preferred email. Thus, this method does not rely on the accounts index.
+ */
+ public ImmutableSet<Account.Id> getAccountForExternal(String email) throws IOException {
+ return externalIds.byEmail(email).stream().map(ExternalId::accountId).collect(toImmutableSet());
+ }
+
private <T> T executeIndexQuery(Action<T> action) throws OrmException {
try {
return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index fb42ed0..358a3a8 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -57,7 +57,6 @@
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeMessageResource;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.PureRevert;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.restapi.change.Abandon;
import com.google.gerrit.server.restapi.change.ChangeIncludedIn;
@@ -70,6 +69,7 @@
import com.google.gerrit.server.restapi.change.GetAssignee;
import com.google.gerrit.server.restapi.change.GetHashtags;
import com.google.gerrit.server.restapi.change.GetPastAssignees;
+import com.google.gerrit.server.restapi.change.GetPureRevert;
import com.google.gerrit.server.restapi.change.GetTopic;
import com.google.gerrit.server.restapi.change.Ignore;
import com.google.gerrit.server.restapi.change.Index;
@@ -153,7 +153,7 @@
private final SetWorkInProgress setWip;
private final SetReadyForReview setReady;
private final PutMessage putMessage;
- private final PureRevert pureRevert;
+ private final Provider<GetPureRevert> getPureRevertProvider;
private final StarredChangesUtil stars;
@Inject
@@ -200,7 +200,7 @@
SetWorkInProgress setWip,
SetReadyForReview setReady,
PutMessage putMessage,
- PureRevert pureRevert,
+ Provider<GetPureRevert> getPureRevertProvider,
StarredChangesUtil stars,
@Assisted ChangeResource change) {
this.changeApi = changeApi;
@@ -245,7 +245,7 @@
this.setWip = setWip;
this.setReady = setReady;
this.putMessage = putMessage;
- this.pureRevert = pureRevert;
+ this.getPureRevertProvider = getPureRevertProvider;
this.stars = stars;
this.change = change;
}
@@ -714,7 +714,9 @@
@Override
public PureRevertInfo pureRevert(@Nullable String claimedOriginal) throws RestApiException {
try {
- return pureRevert.get(change.getNotes(), claimedOriginal);
+ GetPureRevert getPureRevert = getPureRevertProvider.get();
+ getPureRevert.setClaimedOriginal(claimedOriginal);
+ return getPureRevert.apply(change);
} catch (Exception e) {
throw asRestApiException("Cannot compute pure revert", e);
}
diff --git a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 1065bff..a49f8c4 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -34,6 +34,7 @@
import com.google.gerrit.extensions.api.projects.DeleteTagsInput;
import com.google.gerrit.extensions.api.projects.DescriptionInput;
import com.google.gerrit.extensions.api.projects.HeadInput;
+import com.google.gerrit.extensions.api.projects.IndexProjectInput;
import com.google.gerrit.extensions.api.projects.ParentInput;
import com.google.gerrit.extensions.api.projects.ProjectApi;
import com.google.gerrit.extensions.api.projects.ProjectInput;
@@ -64,6 +65,7 @@
import com.google.gerrit.server.restapi.project.GetDescription;
import com.google.gerrit.server.restapi.project.GetHead;
import com.google.gerrit.server.restapi.project.GetParent;
+import com.google.gerrit.server.restapi.project.Index;
import com.google.gerrit.server.restapi.project.ListBranches;
import com.google.gerrit.server.restapi.project.ListChildProjects;
import com.google.gerrit.server.restapi.project.ListDashboards;
@@ -118,6 +120,7 @@
private final SetHead setHead;
private final GetParent getParent;
private final SetParent setParent;
+ private final Index index;
@AssistedInject
ProjectApiImpl(
@@ -150,6 +153,7 @@
SetHead setHead,
GetParent getParent,
SetParent setParent,
+ Index index,
@Assisted ProjectResource project) {
this(
permissionBackend,
@@ -182,6 +186,7 @@
setHead,
getParent,
setParent,
+ index,
null);
}
@@ -216,6 +221,7 @@
SetHead setHead,
GetParent getParent,
SetParent setParent,
+ Index index,
@Assisted String name) {
this(
permissionBackend,
@@ -248,6 +254,7 @@
setHead,
getParent,
setParent,
+ index,
name);
}
@@ -282,6 +289,7 @@
SetHead setHead,
GetParent getParent,
SetParent setParent,
+ Index index,
String name) {
this.permissionBackend = permissionBackend;
this.createProject = createProject;
@@ -314,6 +322,7 @@
this.getParent = getParent;
this.setParent = setParent;
this.name = name;
+ this.index = index;
}
@Override
@@ -595,6 +604,17 @@
}
}
+ @Override
+ public void index(boolean indexChildren) throws RestApiException {
+ try {
+ IndexProjectInput input = new IndexProjectInput();
+ input.indexChildren = indexChildren;
+ index.apply(checkExists(), input);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot index project", e);
+ }
+ }
+
private ProjectResource checkExists() throws ResourceNotFoundException {
if (project == null) {
throw new ResourceNotFoundException(name);
diff --git a/java/com/google/gerrit/server/change/ChangeEditResource.java b/java/com/google/gerrit/server/change/ChangeEditResource.java
index 08bcabe..5419ee9 100644
--- a/java/com/google/gerrit/server/change/ChangeEditResource.java
+++ b/java/com/google/gerrit/server/change/ChangeEditResource.java
@@ -20,7 +20,7 @@
import com.google.inject.TypeLiteral;
/**
- * Represents change edit resource, that is actualy two kinds of resources:
+ * Represents change edit resource, that is actually two kinds of resources:
*
* <ul>
* <li>the change edit itself
@@ -32,6 +32,10 @@
public class ChangeEditResource implements RestResource {
public static final TypeLiteral<RestView<ChangeEditResource>> CHANGE_EDIT_KIND =
new TypeLiteral<RestView<ChangeEditResource>>() {};
+ public static final TypeLiteral<RestView<ChangeEditResource.Publish>> CHANGE_EDIT_PUBLISH_KIND =
+ new TypeLiteral<RestView<ChangeEditResource.Publish>>() {};
+ public static final TypeLiteral<RestView<ChangeEditResource.Rebase>> CHANGE_EDIT_REBASE_KIND =
+ new TypeLiteral<RestView<ChangeEditResource.Rebase>>() {};
private final ChangeResource change;
private final ChangeEdit edit;
@@ -60,4 +64,16 @@
public String getPath() {
return path;
}
+
+ public static class Publish extends ChangeEditResource {
+ public Publish(ChangeResource change, ChangeEdit edit, String path) {
+ super(change, edit, path);
+ }
+ }
+
+ public static class Rebase extends ChangeEditResource {
+ public Rebase(ChangeResource change, ChangeEdit edit, String path) {
+ super(change, edit, path);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/change/PureRevert.java b/java/com/google/gerrit/server/change/PureRevert.java
index ee0e2cc..ddc9661 100644
--- a/java/com/google/gerrit/server/change/PureRevert.java
+++ b/java/com/google/gerrit/server/change/PureRevert.java
@@ -28,6 +28,7 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;
@@ -42,6 +43,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+@Singleton
public class PureRevert {
private final MergeUtil.Factory mergeUtilFactory;
private final GitRepositoryManager repoManager;
@@ -117,7 +119,7 @@
// Any differences between claimed original's parent and the rebase result indicate that the
// claimedRevert is not a pure revert but made content changes
try (DiffFormatter df = new DiffFormatter(new ByteArrayOutputStream())) {
- df.setRepository(repo);
+ df.setReader(oi.newReader(), repo.getConfig());
List<DiffEntry> entries =
df.scan(claimedOriginalCommit.getParent(0), merger.getResultTreeId());
return new PureRevertInfo(entries.isEmpty());
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index f38572d..b5f09fd 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -36,6 +36,8 @@
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.net.MalformedURLException;
+import java.net.URL;
import org.eclipse.jgit.lib.Config;
public class GitwebConfig {
@@ -178,7 +180,11 @@
private final GitwebType type;
@Inject
- GitwebConfig(GitwebCgiConfig cgiConfig, @GerritServerConfig Config cfg) {
+ GitwebConfig(
+ GitwebCgiConfig cgiConfig,
+ @GerritServerConfig Config cfg,
+ @Nullable @CanonicalWebUrl String gerritUrl)
+ throws MalformedURLException {
if (isDisabled(cfg)) {
type = null;
url = null;
@@ -191,7 +197,14 @@
// Use an externally managed gitweb instance, and not an internal one.
url = cfgUrl;
} else {
- url = firstNonNull(cfgUrl, "gitweb");
+ String baseGerritUrl;
+ if (gerritUrl != null) {
+ URL u = new URL(gerritUrl);
+ baseGerritUrl = u.getPath();
+ } else {
+ baseGerritUrl = "/";
+ }
+ url = firstNonNull(cfgUrl, baseGerritUrl + "gitweb");
}
}
}
diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java
index d957558..3c9538c 100644
--- a/java/com/google/gerrit/server/index/IndexModule.java
+++ b/java/com/google/gerrit/server/index/IndexModule.java
@@ -104,7 +104,7 @@
public IndexModule(
ListeningExecutorService interactiveExecutor, ListeningExecutorService batchExecutor) {
- this.threads = -1;
+ this.threads = 0;
this.interactiveExecutor = interactiveExecutor;
this.batchExecutor = batchExecutor;
this.closeExecutorsOnShutdown = false;
@@ -211,11 +211,12 @@
return interactiveExecutor;
}
int threads = this.threads;
- if (threads <= 0) {
- threads = config.getInt("index", null, "threads", 0);
- }
- if (threads <= 0) {
- threads = Runtime.getRuntime().availableProcessors() / 2 + 1;
+ if (threads < 0) {
+ return MoreExecutors.newDirectExecutorService();
+ } else if (threads == 0) {
+ threads =
+ config.getInt(
+ "index", null, "threads", Runtime.getRuntime().availableProcessors() / 2 + 1);
}
return MoreExecutors.listeningDecorator(
workQueue.createQueue(threads, "Index-Interactive", true));
@@ -230,7 +231,9 @@
return batchExecutor;
}
int threads = config.getInt("index", null, "batchThreads", 0);
- if (threads <= 0) {
+ if (threads < 0) {
+ return MoreExecutors.newDirectExecutorService();
+ } else if (threads == 0) {
threads = Runtime.getRuntime().availableProcessors();
}
return MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "Index-Batch", true));
diff --git a/java/com/google/gerrit/server/index/change/StalenessChecker.java b/java/com/google/gerrit/server/index/change/StalenessChecker.java
index e567d77..642385c 100644
--- a/java/com/google/gerrit/server/index/change/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/change/StalenessChecker.java
@@ -34,6 +34,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.RefState;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -103,6 +104,7 @@
parsePatterns(cd));
}
+ @UsedAt(UsedAt.Project.GOOGLE)
public static boolean isStale(
GitRepositoryManager repoManager,
Change.Id id,
diff --git a/java/com/google/gerrit/server/mail/send/EmailArguments.java b/java/com/google/gerrit/server/mail/send/EmailArguments.java
index 04f4d6c..f49951f 100644
--- a/java/com/google/gerrit/server/mail/send/EmailArguments.java
+++ b/java/com/google/gerrit/server/mail/send/EmailArguments.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.IdentifiedUser.GenericFactory;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName;
@@ -49,6 +50,7 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
+@UsedAt(UsedAt.Project.PLUGINS_ALL)
public class EmailArguments {
final GitRepositoryManager server;
final ProjectCache projectCache;
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 3653bc7..010c5c0 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -120,9 +120,7 @@
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
checkUserType(u);
if (u instanceof IdentifiedUser) {
- return noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
+ return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
} else if (u instanceof InternalUser) {
return serverIdent;
}
@@ -177,7 +175,7 @@
}
protected PersonIdent newIdent(Account.Id authorId, Date when) {
- return noteUtil.getLegacyChangeNoteWrite().newIdent(authorId, when, serverIdent);
+ return noteUtil.newIdent(authorId, when, serverIdent);
}
/** Whether no updates have been done. */
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 6b4bea7..169fd2e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -178,12 +178,7 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get());
- byte[] data =
- e.getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson());
+ byte[] data = e.getValue().build(noteUtil.getChangeNoteJson());
if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true;
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 0475fe3..483b2e9 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -14,18 +14,14 @@
package com.google.gerrit.server.notedb;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
-import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.sql.Timestamp;
-import org.eclipse.jgit.lib.Config;
@Singleton
public class ChangeNoteJson {
private final Gson gson = newGson();
- private final boolean writeJson;
static Gson newGson() {
return new GsonBuilder()
@@ -37,13 +33,4 @@
public Gson getGson() {
return gson;
}
-
- public boolean getWriteJson() {
- return writeJson;
- }
-
- @Inject
- ChangeNoteJson(@GerritServerConfig Config config) {
- this.writeJson = config.getBoolean("notedb", "writeJson", true);
- }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index 070f974..cf86c99 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -14,7 +14,15 @@
package com.google.gerrit.server.notedb;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
+import java.util.Date;
+import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.FooterKey;
public class ChangeNoteUtil {
@@ -56,17 +64,21 @@
static final String TAG = FOOTER_TAG.getName();
private final LegacyChangeNoteRead legacyChangeNoteRead;
- private final LegacyChangeNoteWrite legacyChangeNoteWrite;
private final ChangeNoteJson changeNoteJson;
+ private final AccountCache accountCache;
+ private final String serverId;
+
@Inject
public ChangeNoteUtil(
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
- LegacyChangeNoteWrite legacyChangeNoteWrite) {
+ AccountCache accountCache,
+ @GerritServerId String serverId) {
+ this.accountCache = accountCache;
+ this.serverId = serverId;
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = legacyChangeNoteRead;
- this.legacyChangeNoteWrite = legacyChangeNoteWrite;
}
public LegacyChangeNoteRead getLegacyChangeNoteRead() {
@@ -77,7 +89,18 @@
return changeNoteJson;
}
- public LegacyChangeNoteWrite getLegacyChangeNoteWrite() {
- return legacyChangeNoteWrite;
+ public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
+ Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
+ return new PersonIdent(
+ author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
+ authorId.get() + "@" + serverId,
+ when,
+ serverIdent.getTimeZone());
+ }
+
+ @VisibleForTesting
+ public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
+ return new PersonIdent(
+ author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
index 894e979..66dd5e8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
@@ -88,7 +88,7 @@
return comments;
}
- private static boolean isJson(byte[] raw, int offset) {
+ static boolean isJson(byte[] raw, int offset) {
return raw[offset] == '{' || raw[offset] == '[';
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 445f7a0..69fbfe1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -402,6 +402,7 @@
if (notes != null) {
draftUpdate = draftUpdateFactory.create(notes, accountId, realAccountId, authorIdent, when);
} else {
+ // tests will always take the notes != null path above.
draftUpdate =
draftUpdateFactory.create(getChange(), accountId, realAccountId, authorIdent, when);
}
@@ -526,14 +527,7 @@
checkComments(rnm.revisionNotes, builders);
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
- ObjectId data =
- inserter.insert(
- OBJ_BLOB,
- e.getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson()));
+ ObjectId data = inserter.insert(OBJ_BLOB, e.getValue().build(noteUtil.getChangeNoteJson()));
rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data);
}
diff --git a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
new file mode 100644
index 0000000..9a9a211
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
@@ -0,0 +1,190 @@
+// Copyright (C) 2017 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.notedb;
+
+import static com.google.gerrit.server.notedb.RevisionNote.MAX_NOTE_SZ;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.config.GerritServerIdProvider;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.Note;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.util.MutableInteger;
+
+@Singleton
+public class CommentJsonMigrator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final LegacyChangeNoteRead legacyChangeNoteRead;
+ private final ChangeNoteJson changeNoteJson;
+
+ @Inject
+ CommentJsonMigrator(
+ ChangeNoteJson changeNoteJson, GerritServerIdProvider gerritServerIdProvider) {
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = new LegacyChangeNoteRead(gerritServerIdProvider.get());
+ }
+
+ CommentJsonMigrator(ChangeNoteJson changeNoteJson, String serverId) {
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = new LegacyChangeNoteRead(serverId);
+ }
+
+ public boolean migrateChanges(
+ Project.NameKey project, Repository repo, RevWalk rw, ObjectInserter ins, BatchRefUpdate bru)
+ throws IOException {
+ boolean ok = true;
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
+ Change.Id changeId = Change.Id.fromRef(ref.getName());
+ if (changeId == null || !ref.getName().equals(RefNames.changeMetaRef(changeId))) {
+ continue;
+ }
+ ok &= migrateOne(project, rw, ins, bru, Status.PUBLISHED, changeId, ref);
+ }
+ return ok;
+ }
+
+ public boolean migrateDrafts(
+ Project.NameKey allUsers,
+ Repository allUsersRepo,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru)
+ throws IOException {
+ boolean ok = true;
+ for (Ref ref : allUsersRepo.getRefDatabase().getRefsByPrefix(RefNames.REFS_DRAFT_COMMENTS)) {
+ Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
+ if (changeId == null) {
+ continue;
+ }
+ ok &= migrateOne(allUsers, rw, ins, bru, Status.DRAFT, changeId, ref);
+ }
+ return ok;
+ }
+
+ private boolean migrateOne(
+ Project.NameKey project,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru,
+ Status status,
+ Change.Id changeId,
+ Ref ref) {
+ ObjectId oldId = ref.getObjectId();
+ try {
+ if (!hasAnyLegacyComments(rw, oldId)) {
+ return true;
+ }
+ } catch (IOException e) {
+ logger.atInfo().log(
+ String.format(
+ "Error reading change %s in %s; attempting migration anyway", changeId, project),
+ e);
+ }
+
+ try {
+ reset(rw, oldId);
+
+ ObjectReader reader = rw.getObjectReader();
+ ObjectId newId = null;
+ RevCommit c;
+ while ((c = rw.next()) != null) {
+ CommitBuilder cb = new CommitBuilder();
+ cb.setAuthor(c.getAuthorIdent());
+ cb.setCommitter(c.getCommitterIdent());
+ cb.setMessage(c.getFullMessage());
+ cb.setEncoding(c.getEncoding());
+ if (newId != null) {
+ cb.setParentId(newId);
+ }
+
+ // Read/write using the low-level RevisionNote API, which works regardless of NotesMigration
+ // state.
+ NoteMap noteMap = NoteMap.read(reader, c);
+ RevisionNoteMap<ChangeRevisionNote> revNoteMap =
+ RevisionNoteMap.parse(
+ changeNoteJson, legacyChangeNoteRead, changeId, reader, noteMap, status);
+ RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNoteMap);
+
+ for (RevId revId : revNoteMap.revisionNotes.keySet()) {
+ // Call cache.get on each known RevId to read the old note in whichever format, then write
+ // the note in JSON format.
+ byte[] data = cache.get(revId).build(changeNoteJson);
+ noteMap.set(ObjectId.fromString(revId.get()), ins.insert(OBJ_BLOB, data));
+ }
+ cb.setTreeId(noteMap.writeTree(ins));
+ newId = ins.insert(cb);
+ }
+
+ bru.addCommand(new ReceiveCommand(oldId, newId, ref.getName()));
+ return true;
+ } catch (ConfigInvalidException | IOException e) {
+ logger.atInfo().log(String.format("Error migrating change %s in %s", changeId, project), e);
+ return false;
+ }
+ }
+
+ private static boolean hasAnyLegacyComments(RevWalk rw, ObjectId id) throws IOException {
+ ObjectReader reader = rw.getObjectReader();
+ reset(rw, id);
+
+ // Check the note map at each commit, not just the tip. It's possible that the server switched
+ // from legacy to JSON partway through its history, which would have mixed legacy/JSON comments
+ // in its history. Although the tip commit would continue to parse once we remove the legacy
+ // parser, our goal is really to expunge all vestiges of the old format, which implies rewriting
+ // history (and thus returning true) in this case.
+ RevCommit c;
+ while ((c = rw.next()) != null) {
+ NoteMap noteMap = NoteMap.read(reader, c);
+ for (Note note : noteMap) {
+ // Match pre-parsing logic in RevisionNote#parse().
+ byte[] raw = reader.open(note.getData(), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
+ MutableInteger p = new MutableInteger();
+ RevisionNote.trimLeadingEmptyLines(raw, p);
+ if (!ChangeRevisionNote.isJson(raw, p.value)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private static void reset(RevWalk rw, ObjectId id) throws IOException {
+ rw.reset();
+ rw.sort(RevSort.TOPO);
+ rw.sort(RevSort.REVERSE);
+ rw.markStart(rw.parseCommit(id));
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
index 9a8c130..0cd3452 100644
--- a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
+++ b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
@@ -239,13 +239,7 @@
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders();
for (Map.Entry<RevId, RevisionNoteBuilder> entry : builders.entrySet()) {
ObjectId objectId = ObjectId.fromString(entry.getKey().get());
- byte[] data =
- entry
- .getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson());
+ byte[] data = entry.getValue().build(noteUtil.getChangeNoteJson());
if (data.length == 0) {
revNotesMap.noteMap.remove(objectId);
} else {
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
index 8bf286d..b8c7d7d 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
@@ -82,19 +82,13 @@
delete = new HashSet<>();
}
- public byte[] build(ChangeNoteUtil noteUtil, boolean writeJson) throws IOException {
- return build(noteUtil.getChangeNoteJson(), noteUtil.getLegacyChangeNoteWrite(), writeJson);
+ public byte[] build(ChangeNoteUtil noteUtil) throws IOException {
+ return build(noteUtil.getChangeNoteJson());
}
- public byte[] build(
- ChangeNoteJson changeNoteJson, LegacyChangeNoteWrite legacyChangeNoteWrite, boolean writeJson)
- throws IOException {
+ public byte[] build(ChangeNoteJson changeNoteJson) throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
- if (writeJson) {
- buildNoteJson(changeNoteJson, out);
- } else {
- buildNoteLegacy(legacyChangeNoteWrite, out);
- }
+ buildNoteJson(changeNoteJson, out);
return out.toByteArray();
}
@@ -142,22 +136,4 @@
noteUtil.getGson().toJson(data, osw);
}
}
-
- private void buildNoteLegacy(LegacyChangeNoteWrite noteUtil, OutputStream out)
- throws IOException {
- if (pushCert != null) {
- byte[] certBytes = pushCert.getBytes(UTF_8);
- out.write(certBytes, 0, trimTrailingNewlines(certBytes));
- out.write('\n');
- }
- noteUtil.buildNote(buildCommentMap(), out);
- }
-
- private static int trimTrailingNewlines(byte[] bytes) {
- int p = bytes.length;
- while (p > 1 && bytes[p - 1] == '\n') {
- p--;
- }
- return p;
- }
}
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
index 3a0d595..e7ac5b7 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
@@ -142,7 +142,7 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get());
- byte[] data = e.getValue().build(noteUtil, true);
+ byte[] data = e.getValue().build(noteUtil);
if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true;
}
diff --git a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 5d2ce97..ab2203e 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -546,7 +546,7 @@
if (id == null) {
return new PersonIdent(serverIdent, events.getWhen());
}
- return changeNoteUtil.getLegacyChangeNoteWrite().newIdent(id, events.getWhen(), serverIdent);
+ return changeNoteUtil.newIdent(id, events.getWhen(), serverIdent);
}
private List<HashtagsEvent> getHashtagsEvents(Change change, NoteDbUpdateManager manager)
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index f1b6639..15fedd7 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -21,6 +21,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.inject.Inject;
@@ -54,6 +55,7 @@
public class AutoMerger {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ @UsedAt(UsedAt.Project.GOOGLE)
public static boolean cacheAutomerge(Config cfg) {
return cfg.getBoolean("change", null, "cacheAutomerge", true);
}
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 3a17965..82001fb 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -327,8 +327,8 @@
case ABANDON:
return canAbandon();
case DELETE:
- return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
- || getProjectControl().isAdmin();
+ return (getProjectControl().isAdmin()
+ || (isOwner() && refControl.canDeleteOwnChanges(isOwner())));
case ADD_PATCH_SET:
return canAddPatchSet();
case EDIT_ASSIGNEE:
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index cd1f84a..0e3382c 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -133,6 +133,11 @@
return canPerform(Permission.EDIT_TOPIC_NAME, false, true);
}
+ /** @return true if this user can delete their own changes. */
+ boolean canDeleteOwnChanges(boolean isChangeOwner) {
+ return canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner, false);
+ }
+
/** The range of permitted values associated with a label permission. */
PermissionRange getRange(String permission) {
return getRange(permission, false);
diff --git a/java/com/google/gerrit/server/project/AccountsSection.java b/java/com/google/gerrit/server/project/AccountsSection.java
index 087a314a..30bd244 100644
--- a/java/com/google/gerrit/server/project/AccountsSection.java
+++ b/java/com/google/gerrit/server/project/AccountsSection.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.project;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.PermissionRule;
import java.util.ArrayList;
import java.util.List;
@@ -21,14 +22,14 @@
public class AccountsSection {
protected List<PermissionRule> sameGroupVisibility;
- public List<PermissionRule> getSameGroupVisibility() {
+ public ImmutableList<PermissionRule> getSameGroupVisibility() {
if (sameGroupVisibility == null) {
- sameGroupVisibility = new ArrayList<>();
+ sameGroupVisibility = ImmutableList.of();
}
- return sameGroupVisibility;
+ return ImmutableList.copyOf(sameGroupVisibility);
}
public void setSameGroupVisibility(List<PermissionRule> sameGroupVisibility) {
- this.sameGroupVisibility = sameGroupVisibility;
+ this.sameGroupVisibility = new ArrayList<>(sameGroupVisibility);
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 1796c40..d42b652 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -737,7 +737,7 @@
}
}
- private List<PermissionRule> loadPermissionRules(
+ private ImmutableList<PermissionRule> loadPermissionRules(
Config rc,
String section,
String subsection,
@@ -746,7 +746,7 @@
boolean useRange) {
Permission perm = new Permission(varName);
loadPermissionRules(rc, section, subsection, varName, groupsByName, perm, useRange);
- return perm.getRules();
+ return ImmutableList.copyOf(perm.getRules());
}
private void loadPermissionRules(
diff --git a/java/com/google/gerrit/server/restapi/account/Module.java b/java/com/google/gerrit/server/restapi/account/Module.java
index 7b09bc9..f37687f 100644
--- a/java/com/google/gerrit/server/restapi/account/Module.java
+++ b/java/com/google/gerrit/server/restapi/account/Module.java
@@ -107,6 +107,8 @@
get(ACCOUNT_KIND, "external.ids").to(GetExternalIds.class);
post(ACCOUNT_KIND, "external.ids:delete").to(DeleteExternalIds.class);
+ // The gpgkeys REST endpoints are bound via GpgApiModule.
+
factory(AccountsUpdate.Factory.class);
}
diff --git a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
index e42e5d1..4bbd8fc 100644
--- a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
+++ b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountsUpdate;
@@ -119,6 +120,7 @@
return Strings.isNullOrEmpty(newPassword) ? Response.<String>none() : Response.ok(newPassword);
}
+ @UsedAt(UsedAt.Project.PLUGIN_SERVICEUSER)
public static String generate() {
byte[] rand = new byte[LEN];
rng.nextBytes(rand);
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index ae77060..826c792 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -120,9 +120,10 @@
}
/**
- * Create handler that is activated when collection element is accessed but doesn't exist, e. g.
- * PUT request with a path was called but change edit wasn't created yet. Change edit is created
- * and PUT handler is called.
+ * This method is invoked if a DELETE request on a non-existing member is done. For change edits
+ * this is the case if a DELETE request for a file in a change edit is done and the change edit
+ * doesn't exist yet (and hence the parse method returned ResourceNotFoundException). In this case
+ * we want to create the change edit on the fly and delete the file with the given id in it.
*/
@Override
public DeleteFile delete(ChangeResource parent, IdString id) throws RestApiException {
@@ -132,6 +133,11 @@
return deleteFileFactory.create(id.get());
}
+ /**
+ * Create handler that is activated when collection element is accessed but doesn't exist, e. g.
+ * PUT request with a path was called but change edit wasn't created yet. Change edit is created
+ * and PUT handler is called.
+ */
public static class Create
implements RestCreateView<ChangeResource, ChangeEditResource, Put.Input> {
private final Put putEdit;
diff --git a/java/com/google/gerrit/server/restapi/change/GetPureRevert.java b/java/com/google/gerrit/server/restapi/change/GetPureRevert.java
index 42675f6..75019af 100644
--- a/java/com/google/gerrit/server/restapi/change/GetPureRevert.java
+++ b/java/com/google/gerrit/server/restapi/change/GetPureRevert.java
@@ -30,13 +30,15 @@
public class GetPureRevert implements RestReadView<ChangeResource> {
private final PureRevert pureRevert;
+ @Nullable private String claimedOriginal;
@Option(
name = "--claimed-original",
aliases = {"-o"},
usage = "SHA1 (40 digit hex) of the original commit")
- @Nullable
- private String claimedOriginal;
+ public void setClaimedOriginal(String claimedOriginal) {
+ this.claimedOriginal = claimedOriginal;
+ }
@Inject
GetPureRevert(PureRevert pureRevert) {
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index d27b136..ef0ba2d 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -15,6 +15,8 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_KIND;
+import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_PUBLISH_KIND;
+import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_REBASE_KIND;
import static com.google.gerrit.server.change.ChangeMessageResource.CHANGE_MESSAGE_KIND;
import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
import static com.google.gerrit.server.change.CommentResource.COMMENT_KIND;
@@ -65,6 +67,8 @@
DynamicMap.mapOf(binder(), REVIEWER_KIND);
DynamicMap.mapOf(binder(), REVISION_KIND);
DynamicMap.mapOf(binder(), CHANGE_EDIT_KIND);
+ DynamicMap.mapOf(binder(), CHANGE_EDIT_PUBLISH_KIND);
+ DynamicMap.mapOf(binder(), CHANGE_EDIT_REBASE_KIND);
DynamicMap.mapOf(binder(), VOTE_KIND);
DynamicMap.mapOf(binder(), CHANGE_MESSAGE_KIND);
diff --git a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
index b356f18..36a0561 100644
--- a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
@@ -19,8 +19,8 @@
import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
@@ -44,28 +44,32 @@
@Singleton
public class PublishChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Publish>,
+ AcceptsPost<ChangeResource> {
+ private final DynamicMap<RestView<ChangeEditResource.Publish>> views;
private final Publish publish;
@Inject
- PublishChangeEdit(Publish publish) {
+ PublishChangeEdit(DynamicMap<RestView<ChangeEditResource.Publish>> views, Publish publish) {
+ this.views = views;
this.publish = publish;
}
@Override
- public DynamicMap<RestView<ChangeEditResource>> views() {
- throw new NotImplementedException();
+ public DynamicMap<RestView<ChangeEditResource.Publish>> views() {
+ return views;
}
@Override
- public RestView<ChangeResource> list() {
- throw new NotImplementedException();
+ public RestView<ChangeResource> list() throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
- public ChangeEditResource parse(ChangeResource parent, IdString id) {
- throw new NotImplementedException();
+ public ChangeEditResource.Publish parse(ChangeResource parent, IdString id)
+ throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/PutAssignee.java b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
index b6fc010..b7be2a8 100644
--- a/java/com/google/gerrit/server/restapi/change/PutAssignee.java
+++ b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
@@ -89,7 +89,10 @@
throw new UnprocessableEntityException(input.assignee + " is not active");
}
try {
- rsrc.permissions().database(db).user(assignee).check(ChangePermission.READ);
+ rsrc.permissions()
+ .database(db)
+ .absentUser(assignee.getAccountId())
+ .check(ChangePermission.READ);
} catch (AuthException e) {
throw new AuthException("read not permitted for " + input.assignee);
}
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
index 7e1bb4d..47af894 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
@@ -20,11 +20,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeEditResource;
@@ -33,6 +32,9 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -41,28 +43,32 @@
@Singleton
public class RebaseChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Rebase>,
+ AcceptsPost<ChangeResource> {
+ private final DynamicMap<RestView<ChangeEditResource.Rebase>> views;
private final Rebase rebase;
@Inject
- RebaseChangeEdit(Rebase rebase) {
+ RebaseChangeEdit(DynamicMap<RestView<ChangeEditResource.Rebase>> views, Rebase rebase) {
+ this.views = views;
this.rebase = rebase;
}
@Override
- public DynamicMap<RestView<ChangeEditResource>> views() {
- throw new NotImplementedException();
+ public DynamicMap<RestView<ChangeEditResource.Rebase>> views() {
+ return views;
}
@Override
- public RestView<ChangeResource> list() {
- throw new NotImplementedException();
+ public RestView<ChangeResource> list() throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
- public ChangeEditResource parse(ChangeResource parent, IdString id) {
- throw new NotImplementedException();
+ public ChangeEditResource.Rebase parse(ChangeResource parent, IdString id)
+ throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
@@ -71,19 +77,23 @@
}
@Singleton
- public static class Rebase implements RestModifyView<ChangeResource, Input> {
-
+ public static class Rebase extends RetryingRestModifyView<ChangeResource, Input, Response<?>> {
private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
@Inject
- Rebase(GitRepositoryManager repositoryManager, ChangeEditModifier editModifier) {
+ Rebase(
+ RetryHelper retryHelper,
+ GitRepositoryManager repositoryManager,
+ ChangeEditModifier editModifier) {
+ super(retryHelper);
this.repositoryManager = repositoryManager;
this.editModifier = editModifier;
}
@Override
- public Response<?> apply(ChangeResource rsrc, Input in)
+ protected Response<?> applyImpl(
+ BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input in)
throws AuthException, ResourceConflictException, IOException, OrmException,
PermissionBackendException {
Project.NameKey project = rsrc.getProject();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index 1529dae..33155f1 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -157,6 +157,8 @@
bu.execute();
return Response.created(jsonFactory.noOptions().format(ins.getChange()));
}
+ } catch (InvalidNameException e) {
+ throw new BadRequestException(e.toString());
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/Index.java b/java/com/google/gerrit/server/restapi/project/Index.java
index 24f32f6..5547864 100644
--- a/java/com/google/gerrit/server/restapi/project/Index.java
+++ b/java/com/google/gerrit/server/restapi/project/Index.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2017 The Android Open Source Project
+// 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.
@@ -16,57 +16,73 @@
import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
-import com.google.common.io.ByteStreams;
+import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.extensions.api.projects.ProjectInput;
+import com.google.gerrit.extensions.api.projects.IndexProjectInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.index.project.ProjectIndexer;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.git.MultiProgressMonitor;
-import com.google.gerrit.server.git.MultiProgressMonitor.Task;
import com.google.gerrit.server.index.IndexExecutor;
-import com.google.gerrit.server.index.change.AllChangesIndexer;
-import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectResource;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.util.concurrent.Future;
-import org.eclipse.jgit.util.io.NullOutputStream;
-@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
+@RequiresCapability(GlobalCapability.MAINTAIN_SERVER)
@Singleton
-public class Index implements RestModifyView<ProjectResource, ProjectInput> {
+public class Index implements RestModifyView<ProjectResource, IndexProjectInput> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final Provider<AllChangesIndexer> allChangesIndexerProvider;
- private final ChangeIndexer indexer;
+ private final ProjectIndexer indexer;
private final ListeningExecutorService executor;
+ private final Provider<ListChildProjects> listChildProjectsProvider;
@Inject
Index(
- Provider<AllChangesIndexer> allChangesIndexerProvider,
- ChangeIndexer indexer,
- @IndexExecutor(BATCH) ListeningExecutorService executor) {
- this.allChangesIndexerProvider = allChangesIndexerProvider;
+ ProjectIndexer indexer,
+ @IndexExecutor(BATCH) ListeningExecutorService executor,
+ Provider<ListChildProjects> listChildProjectsProvider) {
this.indexer = indexer;
this.executor = executor;
+ this.listChildProjectsProvider = listChildProjectsProvider;
}
@Override
- public Response.Accepted apply(ProjectResource resource, ProjectInput input) {
- Project.NameKey project = resource.getNameKey();
- Task mpt =
- new MultiProgressMonitor(ByteStreams.nullOutputStream(), "Reindexing project")
- .beginSubTask("", MultiProgressMonitor.UNKNOWN);
- AllChangesIndexer allChangesIndexer = allChangesIndexerProvider.get();
- allChangesIndexer.setVerboseOut(NullOutputStream.INSTANCE);
- // The REST call is just a trigger for async reindexing, so it is safe to ignore the future's
- // return value.
+ public Response.Accepted apply(ProjectResource rsrc, IndexProjectInput input)
+ throws IOException, AuthException, OrmException, PermissionBackendException,
+ ResourceConflictException {
+ String response = "Project " + rsrc.getName() + " submitted for reindexing";
+
+ reindexAsync(rsrc.getNameKey());
+ if (Boolean.TRUE.equals(input.indexChildren)) {
+ ListChildProjects listChildProjects = listChildProjectsProvider.get();
+ listChildProjects.setRecursive(true);
+ listChildProjects.apply(rsrc).forEach(p -> reindexAsync(new Project.NameKey(p.name)));
+
+ response += " (indexing children recursively)";
+ }
+ return Response.accepted(response);
+ }
+
+ private void reindexAsync(Project.NameKey project) {
@SuppressWarnings("unused")
- Future<Void> ignored =
- executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt));
- return Response.accepted("Project " + project + " submitted for reindexing");
+ Future<?> possiblyIgnoredError =
+ executor.submit(
+ () -> {
+ try {
+ indexer.index(project);
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log("reindexing project %s failed", project);
+ }
+ });
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/IndexChanges.java b/java/com/google/gerrit/server/restapi/project/IndexChanges.java
new file mode 100644
index 0000000..b84f86c
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/IndexChanges.java
@@ -0,0 +1,72 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
+
+import com.google.common.io.ByteStreams;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.git.MultiProgressMonitor;
+import com.google.gerrit.server.git.MultiProgressMonitor.Task;
+import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.index.change.AllChangesIndexer;
+import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.util.concurrent.Future;
+import org.eclipse.jgit.util.io.NullOutputStream;
+
+@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
+@Singleton
+public class IndexChanges implements RestModifyView<ProjectResource, ProjectInput> {
+
+ private final Provider<AllChangesIndexer> allChangesIndexerProvider;
+ private final ChangeIndexer indexer;
+ private final ListeningExecutorService executor;
+
+ @Inject
+ IndexChanges(
+ Provider<AllChangesIndexer> allChangesIndexerProvider,
+ ChangeIndexer indexer,
+ @IndexExecutor(BATCH) ListeningExecutorService executor) {
+ this.allChangesIndexerProvider = allChangesIndexerProvider;
+ this.indexer = indexer;
+ this.executor = executor;
+ }
+
+ @Override
+ public Response.Accepted apply(ProjectResource resource, ProjectInput input) {
+ Project.NameKey project = resource.getNameKey();
+ Task mpt =
+ new MultiProgressMonitor(ByteStreams.nullOutputStream(), "Reindexing project")
+ .beginSubTask("", MultiProgressMonitor.UNKNOWN);
+ AllChangesIndexer allChangesIndexer = allChangesIndexerProvider.get();
+ allChangesIndexer.setVerboseOut(NullOutputStream.INSTANCE);
+ // The REST call is just a trigger for async reindexing, so it is safe to ignore the future's
+ // return value.
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError =
+ executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt));
+ return Response.accepted("Project " + project + " submitted for reindexing");
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/Module.java b/java/com/google/gerrit/server/restapi/project/Module.java
index d2e59cd..f2047e0 100644
--- a/java/com/google/gerrit/server/restapi/project/Module.java
+++ b/java/com/google/gerrit/server/restapi/project/Module.java
@@ -68,6 +68,7 @@
get(PROJECT_KIND, "statistics.git").to(GetStatistics.class);
post(PROJECT_KIND, "gc").to(GarbageCollect.class);
post(PROJECT_KIND, "index").to(Index.class);
+ post(PROJECT_KIND, "index.changes").to(IndexChanges.class);
child(PROJECT_KIND, "branches").to(BranchesCollection.class);
create(BRANCH_KIND).to(CreateBranch.class);
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
index 422c749..c5c8cf0 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
@@ -115,7 +115,7 @@
}
p.add(r);
}
- accessSection.getPermissions().add(p);
+ accessSection.addPermission(p);
}
sections.add(accessSection);
}
diff --git a/java/com/google/gerrit/server/rules/PrologEnvironment.java b/java/com/google/gerrit/server/rules/PrologEnvironment.java
index 083898b..412e0f9 100644
--- a/java/com/google/gerrit/server/rules/PrologEnvironment.java
+++ b/java/com/google/gerrit/server/rules/PrologEnvironment.java
@@ -18,6 +18,7 @@
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
@@ -176,6 +177,7 @@
private final int reductionLimit;
private final int compileLimit;
private final PatchSetUtil patchsetUtil;
+ private Emails emails;
@Inject
Args(
@@ -187,7 +189,8 @@
IdentifiedUser.GenericFactory userFactory,
Provider<AnonymousUser> anonymousUser,
@GerritServerConfig Config config,
- PatchSetUtil patchsetUtil) {
+ PatchSetUtil patchsetUtil,
+ Emails emails) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.repositoryManager = repositoryManager;
@@ -196,6 +199,7 @@
this.userFactory = userFactory;
this.anonymousUser = anonymousUser;
this.patchsetUtil = patchsetUtil;
+ this.emails = emails;
int limit = config.getInt("rules", null, "reductionLimit", 100000);
reductionLimit = limit <= 0 ? Integer.MAX_VALUE : limit;
@@ -247,5 +251,9 @@
public PatchSetUtil getPatchsetUtil() {
return patchsetUtil;
}
+
+ public Emails getEmails() {
+ return emails;
+ }
}
}
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index 8e39b50..e61fc90 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -297,7 +297,8 @@
try {
return LabelType.checkName(name);
} catch (IllegalArgumentException e) {
- return LabelType.checkName("Invalid-Prolog-Rules-Label-Name--" + sanitizeLabelName(name));
+ String newName = "Invalid-Prolog-Rules-Label-Name-" + sanitizeLabelName(name);
+ return LabelType.checkName(newName.replace("--", "-"));
}
}
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 6770732..8b9cfe3 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -20,7 +20,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
@@ -34,8 +33,6 @@
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -47,6 +44,7 @@
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
public final class StoredValues {
public static final StoredValue<Accounts> ACCOUNTS = create(Accounts.class);
@@ -74,32 +72,16 @@
}
}
- public static final StoredValue<PatchSetInfo> PATCH_SET_INFO =
- new StoredValue<PatchSetInfo>() {
+ public static final StoredValue<RevCommit> COMMIT =
+ new StoredValue<RevCommit>() {
@Override
- public PatchSetInfo createValue(Prolog engine) {
- Change change = getChange(engine);
- PatchSet ps = getPatchSet(engine);
- PrologEnvironment env = (PrologEnvironment) engine.control;
- PatchSetInfoFactory patchInfoFactory = env.getArgs().getPatchSetInfoFactory();
- try {
- return patchInfoFactory.get(change.getProject(), ps);
- } catch (PatchSetInfoNotAvailableException e) {
- throw new SystemException(e.getMessage());
- }
- }
- };
-
- public static final StoredValue<String> COMMIT_MESSAGE =
- new StoredValue<String>() {
- @Override
- public String createValue(Prolog engine) {
+ public RevCommit createValue(Prolog engine) {
Change change = getChange(engine);
PatchSet ps = getPatchSet(engine);
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetUtil patchSetUtil = env.getArgs().getPatchsetUtil();
try {
- return patchSetUtil.getFullCommitMessage(change.getProject(), ps);
+ return patchSetUtil.getRevCommit(change.getProject(), ps);
} catch (IOException e) {
throw new SystemException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/schema/JdbcUtil.java b/java/com/google/gerrit/server/schema/JdbcUtil.java
index 2624923..dddf23a 100644
--- a/java/com/google/gerrit/server/schema/JdbcUtil.java
+++ b/java/com/google/gerrit/server/schema/JdbcUtil.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.schema;
+import com.google.gerrit.server.UsedAt;
+
public class JdbcUtil {
public static String hostname(String hostname) {
@@ -26,6 +28,7 @@
return hostname;
}
+ @UsedAt(UsedAt.Project.PLUGINS_ALL)
public static String port(String port) {
if (port != null && !port.isEmpty()) {
return ":" + port;
diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java
index e8a59d0..61e9c92 100644
--- a/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.UsedAt;
import com.google.gwtorm.jdbc.JdbcExecutor;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.server.OrmException;
@@ -35,7 +36,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- public static final Class<Schema_168> C = Schema_168.class;
+ public static final Class<Schema_169> C = Schema_169.class;
public static int getBinaryVersion() {
return guessVersion(C);
@@ -49,6 +50,7 @@
this.versionNbr = guessVersion(getClass());
}
+ @UsedAt(UsedAt.Project.PLUGIN_DELETE_PROJECT)
public static int guessVersion(Class<?> c) {
String n = c.getName();
n = n.substring(n.lastIndexOf('_') + 1);
diff --git a/java/com/google/gerrit/server/schema/Schema_169.java b/java/com/google/gerrit/server/schema/Schema_169.java
new file mode 100644
index 0000000..11aebdd
--- /dev/null
+++ b/java/com/google/gerrit/server/schema/Schema_169.java
@@ -0,0 +1,122 @@
+// Copyright (C) 2017 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.schema;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.CommentJsonMigrator;
+import com.google.gerrit.server.notedb.MutableNotesMigration;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.SortedSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.internal.storage.file.PackInserter;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Migrate NoteDb inline comments to JSON format. */
+public class Schema_169 extends SchemaVersion {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final AllUsersName allUsers;
+ private final CommentJsonMigrator migrator;
+ private final GitRepositoryManager repoManager;
+ private final NotesMigration notesMigration;
+
+ @Inject
+ Schema_169(
+ Provider<Schema_168> prior,
+ AllUsersName allUsers,
+ CommentJsonMigrator migrator,
+ GitRepositoryManager repoManager,
+ @GerritServerConfig Config config) {
+ super(prior);
+ this.allUsers = allUsers;
+ this.migrator = migrator;
+ this.repoManager = repoManager;
+ this.notesMigration = MutableNotesMigration.fromConfig(config);
+ }
+
+ @Override
+ protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
+ migrateData(ui);
+ }
+
+ private static ObjectInserter newPackInserter(Repository repo) {
+ if (!(repo instanceof FileRepository)) {
+ return repo.newObjectInserter();
+ }
+ PackInserter ins = ((FileRepository) repo).getObjectDatabase().newPackInserter();
+ ins.checkExisting(false);
+ return ins;
+ }
+
+ @VisibleForTesting
+ protected void migrateData(UpdateUI ui) throws OrmException {
+ // If the migration hasn't started, no need to look for non-JSON
+ if (!notesMigration.commitChangeWrites()) {
+ return;
+ }
+
+ boolean ok = true;
+ ProgressMonitor pm = new TextProgressMonitor();
+ SortedSet<Project.NameKey> projects = repoManager.list();
+ pm.beginTask("Migrating projects", projects.size());
+ int skipped = 0;
+ for (Project.NameKey project : projects) {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = newPackInserter(repo)) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ ok &= migrator.migrateChanges(project, repo, rw, ins, bru);
+ if (project.equals(allUsers)) {
+ ok &= migrator.migrateDrafts(allUsers, repo, rw, ins, bru);
+ }
+
+ if (!bru.getCommands().isEmpty()) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+ } else {
+ skipped++;
+ }
+ } catch (IOException e) {
+ logger.atWarning().log("Error migrating project " + project, e);
+ ok = false;
+ }
+ pm.update(1);
+ }
+ pm.endTask();
+ ui.message(
+ "Skipped " + skipped + " project" + (skipped == 1 ? "" : "s") + " with no legacy comments");
+
+ if (!ok) {
+ throw new OrmException("Migration failed");
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 7984c76e..7be1307 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -423,7 +423,8 @@
if (newCommit != null) {
if (author == null) {
author = newCommit.getAuthorIdent();
- } else if (!author.equals(newCommit.getAuthorIdent())) {
+ } else if (!author.getName().equals(newCommit.getAuthorIdent().getName())
+ || !author.getEmailAddress().equals(newCommit.getAuthorIdent().getEmailAddress())) {
author = myIdent;
}
}
diff --git a/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java b/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
index 3c6f6fd..9bf4bb2 100644
--- a/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
+++ b/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
@@ -827,8 +827,12 @@
ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, t);
}
- private void logDebug(String msg, Object... args) {
- ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, args);
+ private void logDebug(String msg) {
+ ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg);
+ }
+
+ private void logDebug(String msg, @Nullable Object arg) {
+ ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, arg);
}
}
diff --git a/java/com/google/gerrit/sshd/commands/IndexProjectCommand.java b/java/com/google/gerrit/sshd/commands/IndexChangesInProjectCommand.java
similarity index 88%
rename from java/com/google/gerrit/sshd/commands/IndexProjectCommand.java
rename to java/com/google/gerrit/sshd/commands/IndexChangesInProjectCommand.java
index 407bbd0..56b00a5 100644
--- a/java/com/google/gerrit/sshd/commands/IndexProjectCommand.java
+++ b/java/com/google/gerrit/sshd/commands/IndexChangesInProjectCommand.java
@@ -19,7 +19,7 @@
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.restapi.project.Index;
+import com.google.gerrit.server.restapi.project.IndexChanges;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
@@ -28,10 +28,10 @@
import org.kohsuke.args4j.Argument;
@RequiresAnyCapability({MAINTAIN_SERVER})
-@CommandMetaData(name = "project", description = "Index changes of a project")
-final class IndexProjectCommand extends SshCommand {
+@CommandMetaData(name = "changes-in-project", description = "Index changes of a project")
+final class IndexChangesInProjectCommand extends SshCommand {
- @Inject private Index index;
+ @Inject private IndexChanges index;
@Argument(
index = 0,
diff --git a/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java b/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
index 599c9dc..332ed69 100644
--- a/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
+++ b/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
@@ -40,6 +40,6 @@
command(index, IndexStartCommand.class);
}
command(index, IndexChangesCommand.class);
- command(index, IndexProjectCommand.class);
+ command(index, IndexChangesInProjectCommand.class);
}
}
diff --git a/java/gerrit/AbstractCommitUserIdentityPredicate.java b/java/gerrit/AbstractCommitUserIdentityPredicate.java
index c2aaa76..1bfc95c 100644
--- a/java/gerrit/AbstractCommitUserIdentityPredicate.java
+++ b/java/gerrit/AbstractCommitUserIdentityPredicate.java
@@ -14,9 +14,12 @@
package gerrit;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.UserIdentity;
+import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.rules.PrologEnvironment;
import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.exceptions.SystemException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Predicate;
@@ -24,6 +27,8 @@
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import java.io.IOException;
+import org.eclipse.jgit.lib.PersonIdent;
abstract class AbstractCommitUserIdentityPredicate extends Predicate.P3 {
private static final SymbolTerm user = SymbolTerm.intern("user", 1);
@@ -36,7 +41,7 @@
cont = n;
}
- protected Operation exec(Prolog engine, UserIdentity userId) throws PrologException {
+ protected Operation exec(Prolog engine, PersonIdent userId) throws PrologException {
engine.setB0();
Term a1 = arg1.dereference();
Term a2 = arg2.dereference();
@@ -46,7 +51,18 @@
Term nameTerm = Prolog.Nil;
Term emailTerm = Prolog.Nil;
- Account.Id id = userId.getAccount();
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ Emails emails = env.getArgs().getEmails();
+ Account.Id id = null;
+ try {
+ ImmutableSet<Account.Id> ids = emails.getAccountForExternal(userId.getEmailAddress());
+ if (ids.size() == 1) {
+ id = ids.iterator().next();
+ }
+ } catch (IOException e) {
+ throw new SystemException(e.getMessage());
+ }
+
if (id == null) {
idTerm = anonymous;
} else {
@@ -58,7 +74,7 @@
nameTerm = SymbolTerm.create(name);
}
- String email = userId.getEmail();
+ String email = userId.getEmailAddress();
if (email != null && !email.equals("")) {
emailTerm = SymbolTerm.create(email);
}
diff --git a/java/gerrit/BUILD b/java/gerrit/BUILD
index 4644af87..8281d8e 100644
--- a/java/gerrit/BUILD
+++ b/java/gerrit/BUILD
@@ -11,5 +11,6 @@
"//lib/flogger:api",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/prolog:runtime",
+ "@guava//jar",
],
)
diff --git a/java/gerrit/PRED_commit_author_3.java b/java/gerrit/PRED_commit_author_3.java
index a876b5e..998b30e 100644
--- a/java/gerrit/PRED_commit_author_3.java
+++ b/java/gerrit/PRED_commit_author_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_author_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_author_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity author = psInfo.getAuthor();
- return exec(engine, author);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getAuthorIdent());
}
}
diff --git a/java/gerrit/PRED_commit_committer_3.java b/java/gerrit/PRED_commit_committer_3.java
index b24b004..293d8ce 100644
--- a/java/gerrit/PRED_commit_committer_3.java
+++ b/java/gerrit/PRED_commit_committer_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_committer_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_committer_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity committer = psInfo.getCommitter();
- return exec(engine, committer);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getCommitterIdent());
}
}
diff --git a/java/gerrit/PRED_commit_message_1.java b/java/gerrit/PRED_commit_message_1.java
index 05bb4bb..eb996d6 100644
--- a/java/gerrit/PRED_commit_message_1.java
+++ b/java/gerrit/PRED_commit_message_1.java
@@ -21,6 +21,7 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
/**
* Returns the commit message as a symbol
@@ -40,7 +41,8 @@
engine.setB0();
Term a1 = arg1.dereference();
- String commitMessage = StoredValues.COMMIT_MESSAGE.get(engine);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ String commitMessage = revCommit.getFullMessage();
SymbolTerm msg = SymbolTerm.create(commitMessage);
if (!a1.unify(msg, engine.trail)) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index db71ef6..d64a9d7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1022,9 +1022,19 @@
@Test
@TestProjectInput(cloneAs = "user")
- public void deleteChangeAsUserWithDeleteOwnChangesPermission() throws Exception {
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
+ deleteChangeAsUser();
+ }
+ @Test
+ @TestProjectInput(cloneAs = "user")
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
+ allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
+ deleteChangeAsUser();
+ }
+
+ private void deleteChangeAsUser() throws Exception {
try {
PushOneCommit.Result changeResult =
pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
@@ -2703,9 +2713,7 @@
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
PersonIdent expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.updated, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id), c.updated, serverIdent.get());
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitPatchSetCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
@@ -2713,10 +2721,7 @@
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
- expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ expectedAuthor = changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitChangeCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index b4a05fc..8479dd1 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -15,10 +15,13 @@
package com.google.gerrit.acceptance.api.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.AtomicLongMap;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
@@ -39,7 +42,9 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.index.IndexExecutor;
import com.google.inject.Inject;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
@@ -52,6 +57,10 @@
public class ProjectIT extends AbstractDaemonTest {
@Inject private DynamicSet<ProjectIndexedListener> projectIndexedListeners;
+ @Inject
+ @IndexExecutor(BATCH)
+ private ListeningExecutorService executor;
+
private ProjectIndexedCounter projectIndexedCounter;
private RegistrationHandle projectIndexedCounterHandle;
@@ -381,6 +390,26 @@
}
}
+ @Test
+ public void reindexProject() throws Exception {
+ createProject("child", project);
+ projectIndexedCounter.clear();
+
+ gApi.projects().name(allProjects.get()).index(false);
+ projectIndexedCounter.assertReindexOf(allProjects.get());
+ }
+
+ @Test
+ public void reindexProjectWithChildren() throws Exception {
+ Project.NameKey middle = createProject("middle", project);
+ Project.NameKey leave = createProject("leave", middle);
+ projectIndexedCounter.clear();
+
+ gApi.projects().name(project.get()).index(true);
+ projectIndexedCounter.assertReindexExactly(
+ ImmutableMap.of(project.get(), 1L, middle.get(), 1L, leave.get(), 1L));
+ }
+
private ConfigInput createTestConfigInput() {
ConfigInput input = new ConfigInput();
input.description = "some description";
@@ -427,5 +456,10 @@
void assertNoReindex() {
assertThat(countsByProject).isEmpty();
}
+
+ void assertReindexExactly(ImmutableMap<String, Long> expected) {
+ assertThat(countsByProject.asMap()).containsExactlyEntriesIn(expected);
+ clear();
+ }
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index 62138ca..ce73875 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -32,6 +32,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
@@ -436,4 +437,22 @@
RevCommit c = rw.parseCommit(commitId);
assertThat(c.getFullMessage()).isEqualTo(expectedMessage);
}
+
+ protected void expectToHaveAuthor(
+ TestRepository<?> repo, String branch, String expectedAuthorName, String expectedAuthorEmail)
+ throws Exception {
+ ObjectId commitId =
+ repo.git()
+ .fetch()
+ .setRemote("origin")
+ .call()
+ .getAdvertisedRef("refs/heads/" + branch)
+ .getObjectId();
+
+ RevWalk rw = repo.getRevWalk();
+ RevCommit c = rw.parseCommit(commitId);
+ PersonIdent authorIdent = c.getAuthorIdent();
+ assertThat(authorIdent.getName()).isEqualTo(expectedAuthorName);
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(expectedAuthorEmail);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index b362a36..80430c4 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -336,8 +336,7 @@
c ->
cfg.getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true)
.getPermission(c, true)
- .getRules()
- .clear());
+ .clearRules());
}
private PushResult push(String... refSpecs) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 1de9d29..45294fb 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -433,9 +433,7 @@
if (notesMigration.commitChangeWrites()) {
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
tr.branch(RefNames.changeMetaRef(c3.getId()))
.commit()
.author(author)
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 81ee3a0..4381ed137 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.ConfigSuite;
import org.eclipse.jgit.junit.TestRepository;
@@ -520,6 +521,87 @@
testSubmoduleSubjectCommitMessageAndExpectTruncation();
}
+ @Test
+ public void superRepoCommitHasSameAuthorAsSubmoduleCommits() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ subRepo.tick(1); // Make sure that both changes have different timestamps.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ subRepo2.tick(2); // Make sure that both changes have different timestamps.
+ String changeId2 =
+ createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic)
+ .getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the same author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the author is preserved for the superRepo commit.
+ expectToHaveAuthor(superRepo, "master", admin.fullName, admin.email);
+ }
+
+ @Test
+ public void superRepoCommitHasGerritAsAuthorIfAuthorsOfSubmoduleCommitsDiffer() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+ subRepo2 = cloneProject(new Project.NameKey(name("subscribed-to-project-2")), user);
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ // Create change as admin.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ // Create change as user.
+ PushOneCommit push =
+ pushFactory.create(db, user.getIdent(), subRepo2, "Change 2", "b.txt", "other content");
+ String changeId2 = push.to("refs/for/master/" + name(topic)).getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the different author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the Gerrit server identity is chosen as author for the superRepo commit.
+ expectToHaveAuthor(
+ superRepo, "master", serverIdent.get().getName(), serverIdent.get().getEmailAddress());
+ }
+
private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
TestRepository<?> superRepo = createProjectWithPush("super-project");
TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
diff --git a/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java b/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
index 26bc345..2bb3dca 100644
--- a/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
+++ b/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.auto.value.AutoValue;
@@ -80,8 +81,13 @@
String msg = String.format("%s %s returned %d: %s", method, uri, status, body);
if (restCall.expectedResponseCode().isPresent()) {
assertWithMessage(msg).that(status).isEqualTo(restCall.expectedResponseCode().get());
+ if (restCall.expectedMessage().isPresent()) {
+ assertWithMessage(msg).that(body).contains(restCall.expectedMessage().get());
+ }
} else {
- assertWithMessage(msg).that(status).isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND));
+ assertWithMessage(msg)
+ .that(status)
+ .isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND, SC_METHOD_NOT_ALLOWED));
assertWithMessage(msg).that(status).isLessThan(SC_INTERNAL_SERVER_ERROR);
}
}
@@ -123,6 +129,8 @@
abstract Optional<Integer> expectedResponseCode();
+ abstract Optional<String> expectedMessage();
+
String uri(String... args) {
String uriFormat = uriFormat();
int expectedArgNum = StringUtils.countMatches(uriFormat, "%s");
@@ -144,6 +152,8 @@
abstract Builder expectedResponseCode(int expectedResponseCode);
+ abstract Builder expectedMessage(String expectedMessage);
+
abstract RestCall build();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
index 8de85a9..1a401b0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
@@ -14,9 +14,20 @@
package com.google.gerrit.acceptance.rest;
+import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.PUT;
+import static com.google.gerrit.gpg.testing.TestKeys.validKeyWithoutExpiration;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
+
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.gpg.testing.TestKey;
+import com.google.gerrit.server.ServerInitiated;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
import org.junit.Test;
/**
@@ -27,6 +38,8 @@
* AbstractRestApiBindingsTest}).
*/
public class AccountsRestApiBindingsIT extends AbstractRestApiBindingsTest {
+ @Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
+
/**
* Account REST endpoints to be tested, each URL contains a placeholder for the account
* identifier.
@@ -40,7 +53,11 @@
RestCall.put("/accounts/%s/name"),
RestCall.delete("/accounts/%s/name"),
RestCall.get("/accounts/%s/username"),
- RestCall.put("/accounts/%s/username"),
+ RestCall.builder(PUT, "/accounts/%s/username")
+ // Changing the username is not allowed.
+ .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+ .expectedMessage("Username cannot be changed.")
+ .build(),
RestCall.get("/accounts/%s/active"),
RestCall.put("/accounts/%s/active"),
RestCall.delete("/accounts/%s/active"),
@@ -73,7 +90,9 @@
RestCall.post("/accounts/%s/external.ids:delete"),
RestCall.get("/accounts/%s/oauthtoken"),
RestCall.get("/accounts/%s/capabilities"),
- RestCall.get("/accounts/%s/capabilities/viewPlugins"));
+ RestCall.get("/accounts/%s/capabilities/viewPlugins"),
+ RestCall.get("/accounts/%s/gpgkeys"),
+ RestCall.post("/accounts/%s/gpgkeys"));
/**
* Email REST endpoints to be tested, each URL contains a placeholders for the account and email
@@ -89,6 +108,17 @@
RestCall.delete("/accounts/%s/emails/%s"));
/**
+ * GPG key REST endpoints to be tested, each URL contains a placeholders for the account
+ * identifier and the GPG key identifier.
+ */
+ private static final ImmutableList<RestCall> GPG_KEY_ENDPOINTS =
+ ImmutableList.of(
+ RestCall.get("/accounts/%s/gpgkeys/%s"),
+
+ // GPG key deletion must be tested last
+ RestCall.delete("/accounts/%s/gpgkeys/%s"));
+
+ /**
* SSH key REST endpoints to be tested, each URL contains a placeholders for the account and SSH
* key identifier.
*/
@@ -121,6 +151,30 @@
}
@Test
+ @GerritConfig(name = "receive.enableSignedPush", value = "true")
+ public void gpgKeyEndpoints() throws Exception {
+ TestKey key = validKeyWithoutExpiration();
+ String id = key.getKeyIdString();
+
+ String email = "test1@example.com"; // email that is hard-coded in the test GPG key
+ accountsUpdateProvider
+ .get()
+ .update(
+ "Add Email",
+ admin.getId(),
+ u ->
+ u.addExternalId(
+ ExternalId.createWithEmail(name("test"), email, admin.getId(), email)));
+
+ setApiUser(admin);
+ gApi.accounts()
+ .self()
+ .putGpgKeys(ImmutableList.of(key.getPublicKeyArmored()), ImmutableList.of());
+
+ execute(GPG_KEY_ENDPOINTS, "self", id);
+ }
+
+ @Test
@UseSsh
public void sshKeyEndpoints() throws Exception {
String sshKeySeq = Integer.toString(gApi.accounts().self().listSshKeys().size());
diff --git a/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
index 53c544d..59c0903 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
@@ -59,7 +59,6 @@
RestCall.delete("/changes/%s/topic"),
RestCall.get("/changes/%s/in"),
RestCall.get("/changes/%s/hashtags"),
- RestCall.post("/changes/%s/hashtags"),
RestCall.get("/changes/%s/comments"),
RestCall.get("/changes/%s/robotcomments"),
RestCall.get("/changes/%s/drafts"),
@@ -98,6 +97,7 @@
.expectedResponseCode(SC_NOT_FOUND)
.build(),
RestCall.get("/changes/%s/edit"),
+ RestCall.post("/changes/%s/edit"),
RestCall.post("/changes/%s/edit:rebase"),
RestCall.get("/changes/%s/edit:message"),
RestCall.put("/changes/%s/edit:message"),
@@ -115,7 +115,8 @@
* identifier.
*/
private static final ImmutableList<RestCall> CHANGE_ENDPOINTS_NOTEDB =
- ImmutableList.of(RestCall.post("/changes/%s/rebuild.notedb"));
+ ImmutableList.of(
+ RestCall.post("/changes/%s/hashtags"), RestCall.post("/changes/%s/rebuild.notedb"));
/**
* Reviewer REST endpoints to be tested, each URL contains placeholders for the change identifier
@@ -239,7 +240,38 @@
RestCall.get("/changes/%s/revisions/%s/files/%s/diff"),
RestCall.get("/changes/%s/revisions/%s/files/%s/blame"));
- // TODO(ekempin): Add tests for change message and change edit REST endpoints
+ /**
+ * Change message REST endpoints to be tested, each URL contains placeholders for the change
+ * identifier and the change message identifier.
+ */
+ private static final ImmutableList<RestCall> CHANGE_MESSAGE_ENDPOINTS =
+ ImmutableList.of(RestCall.get("/changes/%s/messages/%s"));
+
+ /**
+ * Change edit REST endpoints that create an edit to be tested, each URL contains placeholders for
+ * the change identifier and the change edit identifier.
+ */
+ private static final ImmutableList<RestCall> CHANGE_EDIT_CREATE_ENDPOINTS =
+ ImmutableList.of(
+ // Create change edit by editing an existing file.
+ RestCall.put("/changes/%s/edit/%s"),
+
+ // Create change edit by deleting an existing file.
+ RestCall.delete("/changes/%s/edit/%s"));
+
+ /**
+ * Change edit REST endpoints to be tested, each URL contains placeholders for the change
+ * identifier and the change edit identifier.
+ */
+ private static final ImmutableList<RestCall> CHANGE_EDIT_ENDPOINTS =
+ ImmutableList.of(
+ // Calls on existing change edit.
+ RestCall.get("/changes/%s/edit/%s"),
+ RestCall.put("/changes/%s/edit/%s"),
+ RestCall.get("/changes/%s/edit/%s/meta"),
+
+ // Delete content of a file in an existing change edit.
+ RestCall.delete("/changes/%s/edit/%s"));
private static final String FILENAME = "test.txt";
@@ -426,6 +458,35 @@
execute(REVISION_FILE_ENDPOINTS, changeId, "current", FILENAME);
}
+ @Test
+ public void changeMessageEndpoints() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ // A change message is created on change creation.
+ String changeMessageId = Iterables.getOnlyElement(gApi.changes().id(changeId).messages()).id;
+
+ execute(CHANGE_MESSAGE_ENDPOINTS, changeId, changeMessageId);
+ }
+
+ @Test
+ public void changeEditCreateEndpoints() throws Exception {
+ String changeId = createChange("Subject", FILENAME, "content").getChangeId();
+
+ // Each of the REST calls creates the change edit newly.
+ execute(
+ CHANGE_EDIT_CREATE_ENDPOINTS,
+ () -> adminRestSession.delete("/changes/" + changeId + "/edit"),
+ changeId,
+ FILENAME);
+ }
+
+ @Test
+ public void changeEditEndpoints() throws Exception {
+ String changeId = createChange("Subject", FILENAME, "content").getChangeId();
+ gApi.changes().id(changeId).edit().create();
+ execute(CHANGE_EDIT_ENDPOINTS, changeId, FILENAME);
+ }
+
private static Comment.Range createRange(
int startLine, int startCharacter, int endLine, int endCharacter) {
Comment.Range range = new Comment.Range();
diff --git a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
index b930b20..508d407 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
@@ -56,6 +56,7 @@
RestCall.get("/config/server/summary"),
RestCall.get("/config/server/capabilities"),
RestCall.get("/config/server/caches"),
+ RestCall.post("/config/server/caches"),
RestCall.get("/config/server/tasks"));
/**
diff --git a/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
index aba677f..07ea3d0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
@@ -42,9 +42,9 @@
RestCall.get("/plugins/%s/gerrit~status"),
// POST (and PUT) requests don't require the 'gerrit~' prefix in front of the view name.
- RestCall.post("/plugins/%s/enable"),
- RestCall.post("/plugins/%s/disable"),
- RestCall.post("/plugins/%s/reload"),
+ RestCall.post("/plugins/%s/gerrit~enable"),
+ RestCall.post("/plugins/%s/gerrit~disable"),
+ RestCall.post("/plugins/%s/gerrit~reload"),
// Plugin deletion must be tested last
RestCall.delete("/plugins/%s"));
diff --git a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
index d59f037..a86a739 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.GET;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_DASHBOARDS;
import static com.google.gerrit.server.restapi.project.DashboardsCollection.DEFAULT_DASHBOARD_NAME;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.common.collect.ImmutableList;
@@ -93,7 +94,11 @@
RestCall.get("/projects/%s/branches/%s"),
RestCall.put("/projects/%s/branches/%s"),
RestCall.get("/projects/%s/branches/%s/mergeable"),
- RestCall.get("/projects/%s/branches/%s/reflog"),
+ RestCall.builder(GET, "/projects/%s/branches/%s/reflog")
+ // The tests use DfsRepository which does not support getting the reflog.
+ .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+ .expectedMessage("reflog not supported on")
+ .build(),
RestCall.builder(GET, "/projects/%s/branches/%s/files")
// GET /projects/<project>/branches/<branch>/files is not implemented
.expectedResponseCode(SC_NOT_FOUND)
diff --git a/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
index 131ed1a..a2c4ea6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
@@ -18,6 +18,7 @@
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.GerritConfig;
import org.junit.Test;
/**
@@ -39,6 +40,7 @@
.expectedResponseCode(SC_NOT_FOUND)
.build(),
RestCall.get("/changes/"),
+ RestCall.post("/changes/"),
RestCall.get("/groups/"),
RestCall.put("/groups/new-group"),
RestCall.get("/plugins/"),
@@ -47,6 +49,7 @@
RestCall.put("/projects/new-project"));
@Test
+ @GerritConfig(name = "plugins.allowRemoteAdmin", value = "true")
public void rootEndpoints() throws Exception {
execute(ROOT_ENDPOINTS);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index e6f61fa..eb80092 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -189,7 +189,6 @@
testVoteOnBehalfOfWithComment();
}
- @GerritConfig(name = "notedb.writeJson", value = "true")
@Test
public void voteOnBehalfOfWithCommentWritingJson() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
@@ -225,7 +224,6 @@
assertThat(c.getRealAuthor().getId()).isEqualTo(admin.id);
}
- @GerritConfig(name = "notedb.writeJson", value = "true")
@Test
public void voteOnBehalfOfWithRobotComment() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index c723082..09c7e0b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -289,9 +289,7 @@
assertThat(commit.getShortMessage()).isEqualTo("Create change");
PersonIdent expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commit.getCommitterIdent())
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index f7903dd..3534959 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -649,6 +649,34 @@
assertThat(permissions2.keySet()).containsExactly(Permission.READ);
}
+ @Test
+ public void addAccessSectionForInvalidRef() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ // 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
+ String invalidRef = Constants.R_HEADS + "stable_*";
+ accessInput.add.put(invalidRef, accessSectionInfo);
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Invalid Name: " + invalidRef);
+ pApi().access(accessInput);
+ }
+
+ @Test
+ public void createAccessChangeWithAccessSectionForInvalidRef() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ // 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
+ String invalidRef = Constants.R_HEADS + "stable_*";
+ accessInput.add.put(invalidRef, accessSectionInfo);
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Invalid Name: " + invalidRef);
+ pApi().accessChange(accessInput);
+ }
+
private ProjectApi pApi() throws Exception {
return gApi.projects().name(newProjectName.get());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 5555185..211af59 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -950,8 +950,6 @@
@Test
public void jsonCommentHasLegacyFormatFalse() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isTrue();
-
PushOneCommit.Result result = createChange();
Change.Id changeId = result.getChange().getId();
addComment(result.getChangeId(), "comment");
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index ea44bd7..29c043a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -902,9 +902,7 @@
}
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
serverSideTestRepo
.branch(RefNames.changeMetaRef(id))
.commit()
diff --git a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
deleted file mode 100644
index e029e7a..0000000
--- a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright (C) 2017 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.server.change;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Comment;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.inject.Inject;
-import java.util.Collection;
-import org.eclipse.jgit.lib.Config;
-import org.junit.Before;
-import org.junit.Test;
-
-@NoHttpd
-public class LegacyCommentsIT extends AbstractDaemonTest {
- @Inject private ChangeNoteUtil noteUtil;
-
- @ConfigSuite.Default
- public static Config writeJsonFalseConfig() {
- Config c = new Config();
- c.setBoolean("noteDb", null, "writeJson", false);
- return c;
- }
-
- @Before
- public void setUp() {
- setApiUser(user);
- }
-
- @Test
- public void legacyCommentHasLegacyFormatTrue() throws Exception {
- assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isFalse();
-
- PushOneCommit.Result result = createChange();
- Change.Id changeId = result.getChange().getId();
-
- CommentInput cin = new CommentInput();
- cin.message = "comment";
- cin.path = PushOneCommit.FILE_NAME;
-
- ReviewInput rin = new ReviewInput();
- rin.comments = ImmutableMap.of(cin.path, ImmutableList.of(cin));
- gApi.changes().id(changeId.get()).current().review(rin);
-
- Collection<Comment> comments =
- notesFactory.createChecked(db, project, changeId).getComments().values();
- assertThat(comments).hasSize(1);
- Comment comment = comments.iterator().next();
- assertThat(comment.message).isEqualTo("comment");
- assertThat(comment.legacyFormat).isTrue();
- }
-}
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/ReflogIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/ReflogIT.java
index 834dbfa..bcae987 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/ReflogIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/ReflogIT.java
@@ -25,16 +25,10 @@
import java.io.File;
import org.eclipse.jgit.lib.ReflogEntry;
import org.eclipse.jgit.lib.Repository;
-import org.junit.Before;
import org.junit.Test;
@UseLocalDisk
public class ReflogIT extends AbstractDaemonTest {
- @Before
- public void setUp() throws Exception {
- assume().that(notesMigration.disableChangeReviewDb()).isTrue();
- }
-
@Test
public void guessRestApiInReflog() throws Exception {
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
index bf01a21..98dca3e 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
@@ -71,21 +71,17 @@
@Test
public void testUserPredicate() throws Exception {
- // This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
- // TODO(maximeg) get OK results
modifySubmitRules(
String.format(
"gerrit:commit_author(user(%d), '%s', '%s')",
user.getId().get(), user.fullName, user.email));
- assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
@Test
public void testCommitAuthorPredicate() throws Exception {
- // This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
- // TODO(maximeg) get OK results
modifySubmitRules("gerrit:commit_author(Id)");
- assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
private SubmitRecord.Status statusForRule() throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
index 208f380..25bb7a6 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
@@ -102,7 +102,7 @@
enableChangeIndexWrites();
changeIndexedCounter.clear();
- String cmd = Joiner.on(" ").join("gerrit", "index", "project", project.get());
+ String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
adminSshSession.exec(cmd);
adminSshSession.assertSuccess();
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
index 5694bd0..cc86c0b 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
@@ -93,7 +93,8 @@
}
}),
"index",
- ImmutableList.of("changes", "project"), // "activate" and "start" are not included
+ ImmutableList.of(
+ "changes", "changes-in-project"), // "activate" and "start" are not included
"logging",
ImmutableList.of("ls", "set"),
"plugin",
diff --git a/javatests/com/google/gerrit/common/data/AccessSectionTest.java b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
new file mode 100644
index 0000000..ecdb3c8
--- /dev/null
+++ b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
@@ -0,0 +1,253 @@
+// 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.
+
+package com.google.gerrit.common.data;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class AccessSectionTest {
+ @Rule public ExpectedException exception = ExpectedException.none();
+
+ private static final String REF_PATTERN = "refs/heads/master";
+
+ private AccessSection accessSection;
+
+ @Before
+ public void setup() {
+ this.accessSection = new AccessSection(REF_PATTERN);
+ }
+
+ @Test
+ public void getName() {
+ assertThat(accessSection.getName()).isEqualTo(REF_PATTERN);
+ }
+
+ @Test
+ public void getEmptyPermissions() {
+ assertThat(accessSection.getPermissions()).isNotNull();
+ assertThat(accessSection.getPermissions()).isEmpty();
+ }
+
+ @Test
+ public void setAndGetPermissions() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+ accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
+ assertThat(accessSection.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission)
+ .inOrder();
+
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+ accessSection.setPermissions(ImmutableList.of(submitPermission));
+ assertThat(accessSection.getPermissions()).containsExactly(submitPermission);
+ }
+
+ @Test
+ public void cannotSetDuplicatePermissions() {
+ exception.expect(IllegalArgumentException.class);
+ accessSection.setPermissions(
+ ImmutableList.of(new Permission(Permission.ABANDON), new Permission(Permission.ABANDON)));
+ }
+
+ @Test
+ public void cannotSetPermissionsWithConflictingNames() {
+ Permission abandonPermissionLowerCase =
+ new Permission(Permission.ABANDON.toLowerCase(Locale.US));
+ Permission abandonPermissionUpperCase =
+ new Permission(Permission.ABANDON.toUpperCase(Locale.US));
+
+ exception.expect(IllegalArgumentException.class);
+ accessSection.setPermissions(
+ ImmutableList.of(abandonPermissionLowerCase, abandonPermissionUpperCase));
+ }
+
+ @Test
+ public void getNonExistingPermission() {
+ assertThat(accessSection.getPermission("non-existing")).isNull();
+ assertThat(accessSection.getPermission("non-existing", false)).isNull();
+ }
+
+ @Test
+ public void getPermission() {
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+ accessSection.setPermissions(ImmutableList.of(submitPermission));
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
+ }
+
+ @Test
+ public void getPermissionWithOtherCase() {
+ Permission submitPermissionLowerCase = new Permission(Permission.SUBMIT.toLowerCase(Locale.US));
+ accessSection.setPermissions(ImmutableList.of(submitPermissionLowerCase));
+ assertThat(accessSection.getPermission(Permission.SUBMIT.toUpperCase(Locale.US)))
+ .isEqualTo(submitPermissionLowerCase);
+ }
+
+ @Test
+ public void createMissingPermissionOnGet() {
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+
+ assertThat(accessSection.getPermission(Permission.SUBMIT, true))
+ .isEqualTo(new Permission(Permission.SUBMIT));
+ }
+
+ @Test
+ public void addPermission() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+
+ accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+ accessSection.addPermission(submitPermission);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
+ assertThat(accessSection.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission, submitPermission)
+ .inOrder();
+ }
+
+ @Test
+ public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+
+ List<Permission> permissions = new ArrayList<>();
+ permissions.add(abandonPermission);
+ permissions.add(rebasePermission);
+ accessSection.setPermissions(permissions);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+ permissions.add(submitPermission);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ }
+
+ @Test
+ public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+ accessSection.getPermissions().add(submitPermission);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+
+ List<Permission> permissions = new ArrayList<>();
+ permissions.add(new Permission(Permission.ABANDON));
+ permissions.add(new Permission(Permission.REBASE));
+ accessSection.setPermissions(permissions);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ accessSection.getPermissions().add(submitPermission);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ }
+
+ @Test
+ public void removePermission() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+
+ accessSection.setPermissions(
+ ImmutableList.of(abandonPermission, rebasePermission, submitPermission));
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull();
+
+ accessSection.remove(submitPermission);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ assertThat(accessSection.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission)
+ .inOrder();
+ }
+
+ @Test
+ public void removePermissionByName() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+
+ accessSection.setPermissions(
+ ImmutableList.of(abandonPermission, rebasePermission, submitPermission));
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull();
+
+ accessSection.removePermission(Permission.SUBMIT);
+ assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ assertThat(accessSection.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission)
+ .inOrder();
+ }
+
+ @Test
+ public void removePermissionByNameOtherCase() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+
+ String submitLowerCase = Permission.SUBMIT.toLowerCase(Locale.US);
+ String submitUpperCase = Permission.SUBMIT.toUpperCase(Locale.US);
+ Permission submitPermissionLowerCase = new Permission(submitLowerCase);
+
+ accessSection.setPermissions(
+ ImmutableList.of(abandonPermission, rebasePermission, submitPermissionLowerCase));
+ assertThat(accessSection.getPermission(submitLowerCase)).isNotNull();
+ assertThat(accessSection.getPermission(submitUpperCase)).isNotNull();
+
+ accessSection.removePermission(submitUpperCase);
+ assertThat(accessSection.getPermission(submitLowerCase)).isNull();
+ assertThat(accessSection.getPermission(submitUpperCase)).isNull();
+ assertThat(accessSection.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission)
+ .inOrder();
+ }
+
+ @Test
+ public void mergeAccessSections() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission submitPermission = new Permission(Permission.SUBMIT);
+
+ AccessSection accessSection1 = new AccessSection("refs/heads/foo");
+ accessSection1.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
+
+ AccessSection accessSection2 = new AccessSection("refs/heads/bar");
+ accessSection2.setPermissions(ImmutableList.of(rebasePermission, submitPermission));
+
+ accessSection1.mergeFrom(accessSection2);
+ assertThat(accessSection1.getPermissions())
+ .containsExactly(abandonPermission, rebasePermission, submitPermission)
+ .inOrder();
+ }
+
+ @Test
+ public void testEquals() {
+ Permission abandonPermission = new Permission(Permission.ABANDON);
+ Permission rebasePermission = new Permission(Permission.REBASE);
+
+ accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
+
+ AccessSection accessSectionSamePermissionsOtherRef = new AccessSection("refs/heads/other");
+ accessSectionSamePermissionsOtherRef.setPermissions(
+ ImmutableList.of(abandonPermission, rebasePermission));
+ assertThat(accessSection.equals(accessSectionSamePermissionsOtherRef)).isFalse();
+
+ AccessSection accessSectionOther = new AccessSection(REF_PATTERN);
+ accessSectionOther.setPermissions(ImmutableList.of(abandonPermission));
+ assertThat(accessSection.equals(accessSectionOther)).isFalse();
+
+ accessSectionOther.addPermission(rebasePermission);
+ assertThat(accessSection.equals(accessSectionOther)).isTrue();
+ }
+}
diff --git a/javatests/com/google/gerrit/common/data/EncodePathSeparatorTest.java b/javatests/com/google/gerrit/common/data/EncodePathSeparatorTest.java
index 4c4c769..dcd3c05 100644
--- a/javatests/com/google/gerrit/common/data/EncodePathSeparatorTest.java
+++ b/javatests/com/google/gerrit/common/data/EncodePathSeparatorTest.java
@@ -14,20 +14,20 @@
package com.google.gerrit.common.data;
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.Truth.assertThat;
import org.junit.Test;
public class EncodePathSeparatorTest {
@Test
public void defaultBehaviour() {
- assertEquals("a/b", new GitwebType().replacePathSeparator("a/b"));
+ assertThat(new GitwebType().replacePathSeparator("a/b")).isEqualTo("a/b");
}
@Test
public void exclamationMark() {
GitwebType gitwebType = new GitwebType();
gitwebType.setPathSeparator('!');
- assertEquals("a!b", gitwebType.replacePathSeparator("a/b"));
+ assertThat(gitwebType.replacePathSeparator("a/b")).isEqualTo("a!b");
}
}
diff --git a/javatests/com/google/gerrit/common/data/ParameterizedStringTest.java b/javatests/com/google/gerrit/common/data/ParameterizedStringTest.java
index 0f067c4..b646d2b 100644
--- a/javatests/com/google/gerrit/common/data/ParameterizedStringTest.java
+++ b/javatests/com/google/gerrit/common/data/ParameterizedStringTest.java
@@ -14,9 +14,7 @@
package com.google.gerrit.common.data;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
@@ -26,155 +24,148 @@
public class ParameterizedStringTest {
@Test
public void emptyString() {
- final ParameterizedString p = new ParameterizedString("");
- assertEquals("", p.getPattern());
- assertEquals("", p.getRawPattern());
- assertTrue(p.getParameterNames().isEmpty());
+ ParameterizedString p = new ParameterizedString("");
+ assertThat(p.getPattern()).isEmpty();
+ assertThat(p.getRawPattern()).isEmpty();
+ assertThat(p.getParameterNames()).isEmpty();
- final Map<String, String> a = new HashMap<>();
- assertNotNull(p.bind(a));
- assertEquals(0, p.bind(a).length);
- assertEquals("", p.replace(a));
+ Map<String, String> a = new HashMap<>();
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).isEmpty();
+ assertThat(p.replace(a)).isEmpty();
}
@Test
public void asis1() {
- final ParameterizedString p = ParameterizedString.asis("${bar}c");
- assertEquals("${bar}c", p.getPattern());
- assertEquals("${bar}c", p.getRawPattern());
- assertTrue(p.getParameterNames().isEmpty());
+ ParameterizedString p = ParameterizedString.asis("${bar}c");
+ assertThat(p.getPattern()).isEqualTo("${bar}c");
+ assertThat(p.getRawPattern()).isEqualTo("${bar}c");
+ assertThat(p.getParameterNames()).isEmpty();
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("bar", "frobinator");
- assertNotNull(p.bind(a));
- assertEquals(0, p.bind(a).length);
- assertEquals("${bar}c", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).isEmpty();
+ assertThat(p.replace(a)).isEqualTo("${bar}c");
}
@Test
public void replace1() {
- final ParameterizedString p = new ParameterizedString("${bar}c");
- assertEquals("${bar}c", p.getPattern());
- assertEquals("{0}c", p.getRawPattern());
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("bar"));
+ ParameterizedString p = new ParameterizedString("${bar}c");
+ assertThat(p.getPattern()).isEqualTo("${bar}c");
+ assertThat(p.getRawPattern()).isEqualTo("{0}c");
+ assertThat(p.getParameterNames()).containsExactly("bar");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("bar", "frobinator");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("frobinator", p.bind(a)[0]);
- assertEquals("frobinatorc", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("frobinator");
+ assertThat(p.replace(a)).isEqualTo("frobinatorc");
}
@Test
public void replace2() {
- final ParameterizedString p = new ParameterizedString("a${bar}c");
- assertEquals("a${bar}c", p.getPattern());
- assertEquals("a{0}c", p.getRawPattern());
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("bar"));
+ ParameterizedString p = new ParameterizedString("a${bar}c");
+ assertThat(p.getPattern()).isEqualTo("a${bar}c");
+ assertThat(p.getRawPattern()).isEqualTo("a{0}c");
+ assertThat(p.getParameterNames()).containsExactly("bar");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("bar", "frobinator");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("frobinator", p.bind(a)[0]);
- assertEquals("afrobinatorc", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("frobinator");
+ assertThat(p.replace(a)).isEqualTo("afrobinatorc");
}
@Test
public void replace3() {
- final ParameterizedString p = new ParameterizedString("a${bar}");
- assertEquals("a${bar}", p.getPattern());
- assertEquals("a{0}", p.getRawPattern());
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("bar"));
+ ParameterizedString p = new ParameterizedString("a${bar}");
+ assertThat(p.getPattern()).isEqualTo("a${bar}");
+ assertThat(p.getRawPattern()).isEqualTo("a{0}");
+ assertThat(p.getParameterNames()).containsExactly("bar");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("bar", "frobinator");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("frobinator", p.bind(a)[0]);
- assertEquals("afrobinator", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("frobinator");
+ assertThat(p.replace(a)).isEqualTo("afrobinator");
}
@Test
public void replace4() {
- final ParameterizedString p = new ParameterizedString("a${bar}c");
- assertEquals("a${bar}c", p.getPattern());
- assertEquals("a{0}c", p.getRawPattern());
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("bar"));
+ ParameterizedString p = new ParameterizedString("a${bar}c");
+ assertThat(p.getPattern()).isEqualTo("a${bar}c");
+ assertThat(p.getRawPattern()).isEqualTo("a{0}c");
+ assertThat(p.getParameterNames()).containsExactly("bar");
- final Map<String, String> a = new HashMap<>();
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("", p.bind(a)[0]);
- assertEquals("ac", p.replace(a));
+ Map<String, String> a = new HashMap<>();
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEmpty();
+ assertThat(p.replace(a)).isEqualTo("ac");
}
@Test
public void replaceToLowerCase() {
- final ParameterizedString p = new ParameterizedString("${a.toLowerCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toLowerCase}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
a.put("a", "FOO");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
}
@Test
public void replaceToUpperCase() {
- final ParameterizedString p = new ParameterizedString("${a.toUpperCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toUpperCase}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
a.put("a", "FOO");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
}
@Test
public void replaceLocalName() {
- final ParameterizedString p = new ParameterizedString("${a.localPart}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.localPart}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
a.put("a", "foo");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
}
@Test
@@ -182,226 +173,216 @@
ParameterizedString p =
new ParameterizedString(
"hi, ${userName.toUpperCase},your eamil address is '${email.toLowerCase.localPart}'.right?");
- assertEquals(2, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("userName"));
- assertTrue(p.getParameterNames().contains("email"));
+ assertThat(p.getParameterNames()).containsExactly("userName", "email");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("userName", "firstName lastName");
a.put("email", "FIRSTNAME.LASTNAME@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(2, p.bind(a).length);
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(2);
- assertEquals("FIRSTNAME LASTNAME", p.bind(a)[0]);
- assertEquals("firstname.lastname", p.bind(a)[1]);
- assertEquals(
- "hi, FIRSTNAME LASTNAME,your eamil address is 'firstname.lastname'.right?", p.replace(a));
+ assertThat(p.bind(a)[0]).isEqualTo("FIRSTNAME LASTNAME");
+ assertThat(p.bind(a)[1]).isEqualTo("firstname.lastname");
+ assertThat(p.replace(a))
+ .isEqualTo("hi, FIRSTNAME LASTNAME,your eamil address is 'firstname.lastname'.right?");
}
@Test
public void replaceToUpperCaseToLowerCase() {
- final ParameterizedString p = new ParameterizedString("${a.toUpperCase.toLowerCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toUpperCase.toLowerCase}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo@example.com", p.bind(a)[0]);
- assertEquals("foo@example.com", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo@example.com");
+ assertThat(p.replace(a)).isEqualTo("foo@example.com");
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo@example.com", p.bind(a)[0]);
- assertEquals("foo@example.com", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo@example.com");
+ assertThat(p.replace(a)).isEqualTo("foo@example.com");
}
@Test
public void replaceToUpperCaseLocalName() {
- final ParameterizedString p = new ParameterizedString("${a.toUpperCase.localPart}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toUpperCase.localPart}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
}
@Test
public void replaceToUpperCaseAnUndefinedMethod() {
- final ParameterizedString p = new ParameterizedString("${a.toUpperCase.anUndefinedMethod}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toUpperCase.anUndefinedMethod}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO@EXAMPLE.COM", p.bind(a)[0]);
- assertEquals("FOO@EXAMPLE.COM", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO@EXAMPLE.COM");
+ assertThat(p.replace(a)).isEqualTo("FOO@EXAMPLE.COM");
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO@EXAMPLE.COM", p.bind(a)[0]);
- assertEquals("FOO@EXAMPLE.COM", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO@EXAMPLE.COM");
+ assertThat(p.replace(a)).isEqualTo("FOO@EXAMPLE.COM");
}
@Test
public void replaceLocalNameToUpperCase() {
- final ParameterizedString p = new ParameterizedString("${a.localPart.toUpperCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.localPart.toUpperCase}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
}
@Test
public void replaceLocalNameToLowerCase() {
- final ParameterizedString p = new ParameterizedString("${a.localPart.toLowerCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.localPart.toLowerCase}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
}
@Test
public void replaceLocalNameAnUndefinedMethod() {
- final ParameterizedString p = new ParameterizedString("${a.localPart.anUndefinedMethod}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.localPart.anUndefinedMethod}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO", p.bind(a)[0]);
- assertEquals("FOO", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO");
+ assertThat(p.replace(a)).isEqualTo("FOO");
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
}
@Test
public void replaceToLowerCaseToUpperCase() {
- final ParameterizedString p = new ParameterizedString("${a.toLowerCase.toUpperCase}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toLowerCase.toUpperCase}");
+ assertThat(p.getParameterNames()).hasSize(1);
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO@EXAMPLE.COM", p.bind(a)[0]);
- assertEquals("FOO@EXAMPLE.COM", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO@EXAMPLE.COM");
+ assertThat(p.replace(a)).isEqualTo("FOO@EXAMPLE.COM");
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("FOO@EXAMPLE.COM", p.bind(a)[0]);
- assertEquals("FOO@EXAMPLE.COM", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("FOO@EXAMPLE.COM");
+ assertThat(p.replace(a)).isEqualTo("FOO@EXAMPLE.COM");
}
@Test
public void replaceToLowerCaseLocalName() {
- final ParameterizedString p = new ParameterizedString("${a.toLowerCase.localPart}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toLowerCase.localPart}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo", p.bind(a)[0]);
- assertEquals("foo", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo");
+ assertThat(p.replace(a)).isEqualTo("foo");
}
@Test
public void replaceToLowerCaseAnUndefinedMethod() {
- final ParameterizedString p = new ParameterizedString("${a.toLowerCase.anUndefinedMethod}");
- assertEquals(1, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("a"));
+ ParameterizedString p = new ParameterizedString("${a.toLowerCase.anUndefinedMethod}");
+ assertThat(p.getParameterNames()).containsExactly("a");
- final Map<String, String> a = new HashMap<>();
+ Map<String, String> a = new HashMap<>();
a.put("a", "foo@example.com");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo@example.com", p.bind(a)[0]);
- assertEquals("foo@example.com", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo@example.com");
+ assertThat(p.replace(a)).isEqualTo("foo@example.com");
a.put("a", "FOO@EXAMPLE.COM");
- assertNotNull(p.bind(a));
- assertEquals(1, p.bind(a).length);
- assertEquals("foo@example.com", p.bind(a)[0]);
- assertEquals("foo@example.com", p.replace(a));
+ assertThat(p.bind(a)).isNotNull();
+ assertThat(p.bind(a)).hasLength(1);
+ assertThat(p.bind(a)[0]).isEqualTo("foo@example.com");
+ assertThat(p.replace(a)).isEqualTo("foo@example.com");
}
@Test
public void replaceSubmitTooltipWithVariables() {
ParameterizedString p = new ParameterizedString("Submit patch set ${patchSet} into ${branch}");
- assertEquals(2, p.getParameterNames().size());
- assertTrue(p.getParameterNames().contains("patchSet"));
+ assertThat(p.getParameterNames()).hasSize(2);
+ assertThat(p.getParameterNames()).containsExactly("patchSet", "branch");
Map<String, String> params =
ImmutableMap.of(
"patchSet", "42",
"branch", "foo");
- assertNotNull(p.bind(params));
- assertEquals(2, p.bind(params).length);
- assertEquals("42", p.bind(params)[0]);
- assertEquals("foo", p.bind(params)[1]);
- assertEquals("Submit patch set 42 into foo", p.replace(params));
+ assertThat(p.bind(params)).isNotNull();
+ assertThat(p.bind(params)).hasLength(2);
+ assertThat(p.bind(params)[0]).isEqualTo("42");
+ assertThat(p.bind(params)[1]).isEqualTo("foo");
+ assertThat(p.replace(params)).isEqualTo("Submit patch set 42 into foo");
}
@Test
@@ -411,7 +392,7 @@
ImmutableMap.of(
"patchSet", "42",
"branch", "foo");
- assertEquals(0, p.bind(params).length);
- assertEquals("Submit patch set 40 into master", p.replace(params));
+ assertThat(p.bind(params)).isEmpty();
+ assertThat(p.replace(params)).isEqualTo("Submit patch set 40 into master");
}
}
diff --git a/javatests/com/google/gerrit/common/data/PermissionTest.java b/javatests/com/google/gerrit/common/data/PermissionTest.java
new file mode 100644
index 0000000..f76323f
--- /dev/null
+++ b/javatests/com/google/gerrit/common/data/PermissionTest.java
@@ -0,0 +1,341 @@
+// 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.
+
+package com.google.gerrit.common.data;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+
+public class PermissionTest {
+ private static final String PERMISSION_NAME = "foo";
+
+ private Permission permission;
+
+ @Before
+ public void setup() {
+ this.permission = new Permission(PERMISSION_NAME);
+ }
+
+ @Test
+ public void isPermission() {
+ assertThat(Permission.isPermission(Permission.ABANDON)).isTrue();
+ assertThat(Permission.isPermission("no-permission")).isFalse();
+
+ assertThat(Permission.isPermission(Permission.LABEL + "Code-Review")).isTrue();
+ assertThat(Permission.isPermission(Permission.LABEL_AS + "Code-Review")).isTrue();
+ assertThat(Permission.isPermission("Code-Review")).isFalse();
+ }
+
+ @Test
+ public void hasRange() {
+ assertThat(Permission.hasRange(Permission.ABANDON)).isFalse();
+ assertThat(Permission.hasRange("no-permission")).isFalse();
+
+ assertThat(Permission.hasRange(Permission.LABEL + "Code-Review")).isTrue();
+ assertThat(Permission.hasRange(Permission.LABEL_AS + "Code-Review")).isTrue();
+ assertThat(Permission.hasRange("Code-Review")).isFalse();
+ }
+
+ @Test
+ public void isLabel() {
+ assertThat(Permission.isLabel(Permission.ABANDON)).isFalse();
+ assertThat(Permission.isLabel("no-permission")).isFalse();
+
+ assertThat(Permission.isLabel(Permission.LABEL + "Code-Review")).isTrue();
+ assertThat(Permission.isLabel(Permission.LABEL_AS + "Code-Review")).isFalse();
+ assertThat(Permission.isLabel("Code-Review")).isFalse();
+ }
+
+ @Test
+ public void isLabelAs() {
+ assertThat(Permission.isLabelAs(Permission.ABANDON)).isFalse();
+ assertThat(Permission.isLabelAs("no-permission")).isFalse();
+
+ assertThat(Permission.isLabelAs(Permission.LABEL + "Code-Review")).isFalse();
+ assertThat(Permission.isLabelAs(Permission.LABEL_AS + "Code-Review")).isTrue();
+ assertThat(Permission.isLabelAs("Code-Review")).isFalse();
+ }
+
+ @Test
+ public void forLabel() {
+ assertThat(Permission.forLabel("Code-Review")).isEqualTo(Permission.LABEL + "Code-Review");
+ }
+
+ @Test
+ public void forLabelAs() {
+ assertThat(Permission.forLabelAs("Code-Review")).isEqualTo(Permission.LABEL_AS + "Code-Review");
+ }
+
+ @Test
+ public void extractLabel() {
+ assertThat(Permission.extractLabel(Permission.LABEL + "Code-Review")).isEqualTo("Code-Review");
+ assertThat(Permission.extractLabel(Permission.LABEL_AS + "Code-Review"))
+ .isEqualTo("Code-Review");
+ assertThat(Permission.extractLabel("Code-Review")).isNull();
+ assertThat(Permission.extractLabel(Permission.ABANDON)).isNull();
+ }
+
+ @Test
+ public void canBeOnAllProjects() {
+ assertThat(Permission.canBeOnAllProjects(AccessSection.ALL, Permission.ABANDON)).isTrue();
+ assertThat(Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)).isFalse();
+ assertThat(Permission.canBeOnAllProjects(AccessSection.ALL, Permission.LABEL + "Code-Review"))
+ .isTrue();
+ assertThat(
+ Permission.canBeOnAllProjects(AccessSection.ALL, Permission.LABEL_AS + "Code-Review"))
+ .isTrue();
+
+ assertThat(Permission.canBeOnAllProjects("refs/heads/*", Permission.ABANDON)).isTrue();
+ assertThat(Permission.canBeOnAllProjects("refs/heads/*", Permission.OWNER)).isTrue();
+ assertThat(Permission.canBeOnAllProjects("refs/heads/*", Permission.LABEL + "Code-Review"))
+ .isTrue();
+ assertThat(Permission.canBeOnAllProjects("refs/heads/*", Permission.LABEL_AS + "Code-Review"))
+ .isTrue();
+ }
+
+ @Test
+ public void getName() {
+ assertThat(permission.getName()).isEqualTo(PERMISSION_NAME);
+ }
+
+ @Test
+ public void getLabel() {
+ assertThat(new Permission(Permission.LABEL + "Code-Review").getLabel())
+ .isEqualTo("Code-Review");
+ assertThat(new Permission(Permission.LABEL_AS + "Code-Review").getLabel())
+ .isEqualTo("Code-Review");
+ assertThat(new Permission("Code-Review").getLabel()).isNull();
+ assertThat(new Permission(Permission.ABANDON).getLabel()).isNull();
+ }
+
+ @Test
+ public void exclusiveGroup() {
+ assertThat(permission.getExclusiveGroup()).isFalse();
+
+ permission.setExclusiveGroup(true);
+ assertThat(permission.getExclusiveGroup()).isTrue();
+
+ permission.setExclusiveGroup(false);
+ assertThat(permission.getExclusiveGroup()).isFalse();
+ }
+
+ @Test
+ public void noExclusiveGroupOnOwnerPermission() {
+ Permission permission = new Permission(Permission.OWNER);
+ assertThat(permission.getExclusiveGroup()).isFalse();
+
+ permission.setExclusiveGroup(true);
+ assertThat(permission.getExclusiveGroup()).isFalse();
+ }
+
+ @Test
+ public void getEmptyRules() {
+ assertThat(permission.getRules()).isNotNull();
+ assertThat(permission.getRules()).isEmpty();
+ }
+
+ @Test
+ public void setAndGetRules() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
+
+ PermissionRule permissionRule3 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-3"), "group3"));
+ permission.setRules(ImmutableList.of(permissionRule3));
+ assertThat(permission.getRules()).containsExactly(permissionRule3);
+ }
+
+ @Test
+ public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ GroupReference groupReference3 = new GroupReference(new AccountGroup.UUID("uuid-3"), "group3");
+
+ List<PermissionRule> rules = new ArrayList<>();
+ rules.add(permissionRule1);
+ rules.add(permissionRule2);
+ permission.setRules(rules);
+ assertThat(permission.getRule(groupReference3)).isNull();
+
+ PermissionRule permissionRule3 = new PermissionRule(groupReference3);
+ rules.add(permissionRule3);
+ assertThat(permission.getRule(groupReference3)).isNull();
+ }
+
+ @Test
+ public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+ permission.getRules().add(permissionRule1);
+ assertThat(permission.getRule(groupReference1)).isNull();
+
+ List<PermissionRule> rules = new ArrayList<>();
+ rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2")));
+ rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-3"), "group3")));
+ permission.setRules(rules);
+ assertThat(permission.getRule(groupReference1)).isNull();
+ permission.getRules().add(permissionRule1);
+ assertThat(permission.getRule(groupReference1)).isNull();
+ }
+
+ @Test
+ public void getNonExistingRule() {
+ GroupReference groupReference = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
+ assertThat(permission.getRule(groupReference)).isNull();
+ assertThat(permission.getRule(groupReference, false)).isNull();
+ }
+
+ @Test
+ public void getRule() {
+ GroupReference groupReference = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
+ PermissionRule permissionRule = new PermissionRule(groupReference);
+ permission.setRules(ImmutableList.of(permissionRule));
+ assertThat(permission.getRule(groupReference)).isEqualTo(permissionRule);
+ }
+
+ @Test
+ public void createMissingRuleOnGet() {
+ GroupReference groupReference = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
+ assertThat(permission.getRule(groupReference)).isNull();
+
+ assertThat(permission.getRule(groupReference, true))
+ .isEqualTo(new PermissionRule(groupReference));
+ }
+
+ @Test
+ public void addRule() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ GroupReference groupReference3 = new GroupReference(new AccountGroup.UUID("uuid-3"), "group3");
+ assertThat(permission.getRule(groupReference3)).isNull();
+
+ PermissionRule permissionRule3 = new PermissionRule(groupReference3);
+ permission.add(permissionRule3);
+ assertThat(permission.getRule(groupReference3)).isEqualTo(permissionRule3);
+ assertThat(permission.getRules())
+ .containsExactly(permissionRule1, permissionRule2, permissionRule3)
+ .inOrder();
+ }
+
+ @Test
+ public void removeRule() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ GroupReference groupReference3 = new GroupReference(new AccountGroup.UUID("uuid-3"), "group3");
+ PermissionRule permissionRule3 = new PermissionRule(groupReference3);
+
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
+ assertThat(permission.getRule(groupReference3)).isNotNull();
+
+ permission.remove(permissionRule3);
+ assertThat(permission.getRule(groupReference3)).isNull();
+ assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
+ }
+
+ @Test
+ public void removeRuleByGroupReference() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ GroupReference groupReference3 = new GroupReference(new AccountGroup.UUID("uuid-3"), "group3");
+ PermissionRule permissionRule3 = new PermissionRule(groupReference3);
+
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
+ assertThat(permission.getRule(groupReference3)).isNotNull();
+
+ permission.removeRule(groupReference3);
+ assertThat(permission.getRule(groupReference3)).isNull();
+ assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
+ }
+
+ @Test
+ public void clearRules() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ assertThat(permission.getRules()).isNotEmpty();
+
+ permission.clearRules();
+ assertThat(permission.getRules()).isEmpty();
+ }
+
+ @Test
+ public void mergePermissions() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+ PermissionRule permissionRule3 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-3"), "group3"));
+
+ Permission permission1 = new Permission("foo");
+ permission1.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+
+ Permission permission2 = new Permission("bar");
+ permission2.setRules(ImmutableList.of(permissionRule2, permissionRule3));
+
+ permission1.mergeFrom(permission2);
+ assertThat(permission1.getRules())
+ .containsExactly(permissionRule1, permissionRule2, permissionRule3)
+ .inOrder();
+ }
+
+ @Test
+ public void testEquals() {
+ PermissionRule permissionRule1 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-1"), "group1"));
+ PermissionRule permissionRule2 =
+ new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2"));
+
+ permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+
+ Permission permissionSameRulesOtherName = new Permission("bar");
+ permissionSameRulesOtherName.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ assertThat(permission.equals(permissionSameRulesOtherName)).isFalse();
+
+ Permission permissionSameRulesSameNameOtherExclusiveGroup = new Permission("foo");
+ permissionSameRulesSameNameOtherExclusiveGroup.setRules(
+ ImmutableList.of(permissionRule1, permissionRule2));
+ permissionSameRulesSameNameOtherExclusiveGroup.setExclusiveGroup(true);
+ assertThat(permission.equals(permissionSameRulesSameNameOtherExclusiveGroup)).isFalse();
+
+ Permission permissionOther = new Permission(PERMISSION_NAME);
+ permissionOther.setRules(ImmutableList.of(permissionRule1));
+ assertThat(permission.equals(permissionOther)).isFalse();
+
+ permissionOther.add(permissionRule2);
+ assertThat(permission.equals(permissionOther)).isTrue();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 3113a8a..f405c28 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -30,6 +30,7 @@
visibility = ["//visibility:public"],
runtime_deps = [
"//lib/bouncycastle:bcprov",
+ "//prolog:gerrit-prolog-common",
],
deps = [
":custom-truth-subjects",
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index c677be5..cc7302f 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -79,24 +79,10 @@
@Ignore
@RunWith(ConfigSuite.class)
public abstract class AbstractChangeNotesTest extends GerritBaseTests {
- @ConfigSuite.Default
- public static Config changeNotesLegacy() {
- Config cfg = new Config();
- cfg.setBoolean("notedb", null, "writeJson", false);
- return cfg;
- }
-
- @ConfigSuite.Config
- public static Config changeNotesJson() {
- Config cfg = new Config();
- cfg.setBoolean("notedb", null, "writeJson", true);
- return cfg;
- }
+ private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles");
@ConfigSuite.Parameter public Config testConfig;
- private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles");
-
protected Account.Id otherUserId;
protected FakeAccountCache accountCache;
protected IdentifiedUser changeOwner;
@@ -172,7 +158,6 @@
bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
bind(MetricMaker.class).to(DisabledMetricMaker.class);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(null));
-
MutableNotesMigration migration = MutableNotesMigration.newDisabled();
migration.setFrom(NotesMigrationState.FINAL);
bind(MutableNotesMigration.class).toInstance(migration);
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index de964d8..313fa07 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -498,11 +498,7 @@
private RevCommit writeCommit(String body) throws Exception {
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
- body,
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
- false);
+ body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
}
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
@@ -513,9 +509,7 @@
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
body,
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
initWorkInProgress);
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 74ba0c2..bea61ed 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -58,17 +58,14 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
-import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -79,7 +76,6 @@
@Inject private ChangeNoteJson changeNoteJson;
@Inject private LegacyChangeNoteRead legacyChangeNoteRead;
- @Inject private ChangeNoteUtil noteUtil;
@Inject private @GerritServerId String serverId;
@@ -999,7 +995,7 @@
update.commit();
try {
- notes = newNotes(c);
+ newNotes(c);
fail("Expected IOException");
} catch (OrmException e) {
assertCause(
@@ -1149,10 +1145,8 @@
update.commit();
ChangeNotes notes = newNotes(c);
- String note = readNote(notes, commit);
- if (!testJson()) {
- assertThat(note).isEqualTo(pushCert);
- }
+ readNote(notes, commit);
+
Map<PatchSet.Id, PatchSet> patchSets = notes.getPatchSets();
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
@@ -1185,27 +1179,6 @@
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
assertThat(notes.getComments()).isNotEmpty();
-
- if (!testJson()) {
- assertThat(readNote(notes, commit))
- .isEqualTo(
- pushCert
- + "Revision: "
- + commit.name()
- + "\n"
- + "Patch-set: 2\n"
- + "File: a.txt\n"
- + "\n"
- + "1:2-3:4\n"
- + NoteDbUtil.formatTime(serverIdent, ts)
- + "\n"
- + "Author: Change Owner <1@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 7\n"
- + "Comment\n"
- + "\n");
- }
}
@Test
@@ -1591,307 +1564,6 @@
}
@Test
- public void patchLineCommentNotesFormatSide1() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String uuid3 = "uuid3";
- String message1 = "comment 1";
- String message2 = "comment 2";
- String message3 = "comment 3";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- Timestamp time3 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range2 = new CommentRange(2, 1, 3, 1);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range2,
- range2.getEndLine(),
- otherUser,
- null,
- time2,
- message2,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range3 = new CommentRange(3, 0, 4, 1);
- Comment comment3 =
- newComment(
- psId,
- "file2",
- uuid3,
- range3,
- range3.getEndLine(),
- otherUser,
- null,
- time3,
- message3,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment3);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n"
- + "File: file2\n"
- + "\n"
- + "3:0-4:1\n"
- + NoteDbUtil.formatTime(serverIdent, time3)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid3\n"
- + "Bytes: 9\n"
- + "comment 3\n"
- + "\n");
- }
- }
- }
-
- @Test
- public void patchLineCommentNotesFormatSide0() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String message1 = "comment 1";
- String message2 = "comment 2";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range2 = new CommentRange(2, 1, 3, 1);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range2,
- range2.getEndLine(),
- otherUser,
- null,
- time2,
- message2,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n");
- }
- }
- }
-
- @Test
- public void patchLineCommentNotesResolvedChangesValue() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String message1 = "comment 1";
- String message2 = "comment 2";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range1,
- range1.getEndLine(),
- otherUser,
- uuid1,
- time2,
- message2,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- true);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Parent: uuid1\n"
- + "Unresolved: true\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n");
- }
- }
- }
-
- @Test
public void patchLineCommentNotesFormatMultiplePatchSetsSameRevId() throws Exception {
Change c = newChange();
PatchSet.Id psId1 = c.currentPatchSetId();
@@ -1959,54 +1631,6 @@
update.commit();
ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
- String timeStr = NoteDbUtil.formatTime(serverIdent, time);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n"
- + "Base-for-patch-set: 2\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid3\n"
- + "Bytes: 9\n"
- + "comment 3\n"
- + "\n");
- }
- }
assertThat(notes.getComments())
.isEqualTo(
ImmutableListMultimap.of(
@@ -2048,32 +1672,6 @@
ChangeNotes notes = newNotes(c);
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Real-author: Change Owner <1@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid\n"
- + "Bytes: 7\n"
- + "comment\n"
- + "\n");
- }
- }
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment));
}
@@ -2112,32 +1710,6 @@
ChangeNotes notes = newNotes(c);
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
- String timeStr = NoteDbUtil.formatTime(serverIdent, time);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Weird\u0002User <3@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid\n"
- + "Bytes: 7\n"
- + "comment\n"
- + "\n");
- }
- }
assertThat(notes.getComments())
.isEqualTo(ImmutableListMultimap.of(new RevId(comment.revId), comment));
}
@@ -2604,7 +2176,6 @@
assertThat(notes.getDraftCommentNotes().getNoteMap().contains(objId)).isTrue();
update = newUpdate(c, otherUser);
- now = TimeUtil.nowTs();
update.setPatchSetId(psId);
update.deleteComment(comment);
update.commit();
@@ -2674,7 +2245,6 @@
assertThat(notes.getDraftComments(otherUserId)).hasSize(2);
update = newUpdate(c, otherUser);
- now = TimeUtil.nowTs();
update.setPatchSetId(ps2);
update.deleteComment(comment2);
update.commit();
@@ -3525,10 +3095,6 @@
update.commit();
}
- private boolean testJson() {
- return noteUtil.getChangeNoteJson().getWriteJson();
- }
-
private String readNote(ChangeNotes notes, ObjectId noteId) throws Exception {
ObjectId dataId = notes.revisionNoteMap.noteMap.getNote(noteId).getData();
return new String(rw.getObjectReader().open(dataId, OBJ_BLOB).getCachedBytes(), UTF_8);
diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
new file mode 100644
index 0000000..a5c4e49
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
@@ -0,0 +1,535 @@
+// Copyright (C) 2017 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.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimaps;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.testing.TestChanges;
+import com.google.inject.Inject;
+import java.io.ByteArrayOutputStream;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
+ private CommentJsonMigrator migrator;
+ @Inject private ChangeNoteUtil noteUtil;
+ @Inject private CommentsUtil commentsUtil;
+ @Inject private LegacyChangeNoteWrite legacyChangeNoteWrite;
+
+ private AtomicInteger uuidCounter;
+
+ @Before
+ public void setUpCounter() {
+ uuidCounter = new AtomicInteger();
+ migrator = new CommentJsonMigrator(new ChangeNoteJson(), "gerrit");
+ }
+
+ @Test
+ public void noOpIfAllCommentsAreJson() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ Comment ps1Comment = newComment(notes, 1, "comment on ps1");
+ update.putComment(Status.PUBLISHED, ps1Comment);
+ update.commit();
+
+ notes = newNotes(c);
+ update = newUpdate(c, changeOwner);
+ Comment ps2Comment = newComment(notes, 2, "comment on ps2");
+ update.putComment(Status.PUBLISHED, ps2Comment);
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString());
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 0);
+ assertNoDifferences(notes, oldNotes);
+ assertThat(notes.getMetaId()).isEqualTo(oldNotes.getMetaId());
+ }
+
+ @Test
+ public void migratePublishedComments() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+
+ Comment ps1Comment1 = newComment(notes, 1, "first comment on ps1");
+ Comment ps2Comment1 = newComment(notes, 2, "first comment on ps2");
+ Comment ps1Comment2 = newComment(notes, 1, "second comment on ps1");
+
+ // Construct legacy format 'by hand'.
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ps1Comment1).build(), out1);
+
+ ByteArrayOutputStream out2 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(2, ps2Comment1).build(), out2);
+
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder()
+ .put(1, ps1Comment2)
+ .put(1, ps1Comment1)
+ .build(),
+ out3);
+
+ TestRepository<Repository> testRepository = new TestRepository<>(repo, rw);
+
+ String metaRefName = RefNames.changeMetaRef(c.getId());
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ps1Comment1.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 2\n\nPatch-set: 2")
+ .add(ps2Comment1.revId, out2.toString())
+ .add(ps1Comment1.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment1.toString(),
+ getRevId(notes, 1), ps1Comment2.toString(),
+ getRevId(notes, 2), ps2Comment1.toString());
+
+ // Comments at each commit all have legacy format.
+ ImmutableList<RevCommit> oldLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertThat(oldLog).hasSize(4);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(2)))
+ .containsExactly(ps1Comment1.key, true);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(3)))
+ .containsExactly(ps1Comment1.key, true, ps1Comment2.key, true, ps2Comment1.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 1);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertNoDifferences(notes, oldNotes);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment1.toString(),
+ getRevId(notes, 1), ps1Comment2.toString(),
+ getRevId(notes, 2), ps2Comment1.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertLogEqualExceptTrees(newLog, oldLog);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2)))
+ .containsExactly(ps1Comment1.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(3)))
+ .containsExactly(ps1Comment1.key, false, ps1Comment2.key, false, ps2Comment1.key, false);
+ }
+
+ @Test
+ public void migrateDraftComments() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+ ObjectId origMetaId = notes.getMetaId();
+
+ Comment ownerCommentPs1 = newComment(notes, 1, "owner comment on ps1", changeOwner);
+ Comment ownerCommentPs2 = newComment(notes, 2, "owner comment on ps2", changeOwner);
+ Comment otherCommentPs1 = newComment(notes, 1, "other user comment on ps1", otherUser);
+
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ownerCommentPs1).build(), out1);
+
+ ByteArrayOutputStream out2 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(2, ownerCommentPs2).build(), out2);
+
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, otherCommentPs1).build(), out3);
+
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ RevWalk allUsersRw = new RevWalk(allUsersRepo)) {
+ TestRepository<Repository> testRepository = new TestRepository<>(allUsersRepo, allUsersRw);
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()))
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ownerCommentPs1.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()))
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 2")
+ .add(ownerCommentPs2.revId, out2.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()))
+ .commit()
+ .message("Review ps 2\n\nPatch-set: 2")
+ .add(otherCommentPs1.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+ }
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getDraftComments(changeOwner.getAccountId())))
+ .containsExactly(
+ getRevId(notes, 1), ownerCommentPs1.toString(),
+ getRevId(notes, 2), ownerCommentPs2.toString());
+ assertThat(getToStringRepresentations(notes.getDraftComments(otherUser.getAccountId())))
+ .containsExactly(getRevId(notes, 1), otherCommentPs1.toString());
+
+ // Comments at each commit all have legacy format.
+ ImmutableList<RevCommit> oldOwnerLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()));
+ assertThat(oldOwnerLog).hasSize(2);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOwnerLog.get(0)))
+ .containsExactly(ownerCommentPs1.key, true);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOwnerLog.get(1)))
+ .containsExactly(ownerCommentPs1.key, true, ownerCommentPs2.key, true);
+
+ ImmutableList<RevCommit> oldOtherLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()));
+ assertThat(oldOtherLog).hasSize(1);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOtherLog.get(0)))
+ .containsExactly(otherCommentPs1.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(allUsers, migrator::migrateDrafts, 2);
+ assertNoDifferences(notes, oldNotes);
+
+ // Migration doesn't touch change ref.
+ assertThat(repo.exactRef(RefNames.changeMetaRef(c.getId())).getObjectId())
+ .isEqualTo(origMetaId);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getDraftComments(changeOwner.getAccountId())))
+ .containsExactly(
+ getRevId(notes, 1), ownerCommentPs1.toString(),
+ getRevId(notes, 2), ownerCommentPs2.toString());
+ assertThat(getToStringRepresentations(notes.getDraftComments(otherUser.getAccountId())))
+ .containsExactly(getRevId(notes, 1), otherCommentPs1.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newOwnerLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()));
+ assertLogEqualExceptTrees(newOwnerLog, oldOwnerLog);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(0)))
+ .containsExactly(ownerCommentPs1.key, false);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(1)))
+ .containsExactly(ownerCommentPs1.key, false, ownerCommentPs2.key, false);
+
+ ImmutableList<RevCommit> newOtherLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()));
+ assertLogEqualExceptTrees(newOtherLog, oldOtherLog);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOtherLog.get(0)))
+ .containsExactly(otherCommentPs1.key, false);
+ }
+
+ @Test
+ public void migrateMixOfJsonAndLegacyComments() throws Exception {
+ // 3 comments: legacy, JSON, legacy. Because adding a comment necessarily rewrites the entire
+ // note, these comments need to be on separate patch sets.
+ Change c = newChange();
+ incrementPatchSet(c);
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+
+ Comment ps1Comment = newComment(notes, 1, "comment on ps1 (legacy)");
+
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ps1Comment).build(), out1);
+
+ TestRepository<Repository> testRepository = new TestRepository<>(repo, rw);
+
+ String metaRefName = RefNames.changeMetaRef(c.getId());
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ps1Comment.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ Comment ps2Comment = newComment(notes, 2, "comment on ps2 (JSON)");
+ update.putComment(Status.PUBLISHED, ps2Comment);
+ update.commit();
+
+ Comment ps3Comment = newComment(notes, 3, "comment on ps3 (legacy)");
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(3, ps3Comment).build(), out3);
+
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 3\n\nPatch-set: 3")
+ .add(ps3Comment.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString(),
+ getRevId(notes, 3), ps3Comment.toString());
+
+ // Comments at each commit match expected format.
+ ImmutableList<RevCommit> oldLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertThat(oldLog).hasSize(6);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(2))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(3)))
+ .containsExactly(ps1Comment.key, true);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(4)))
+ .containsExactly(ps1Comment.key, true, ps2Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(5)))
+ .containsExactly(ps1Comment.key, true, ps2Comment.key, false, ps3Comment.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 1);
+ assertNoDifferences(notes, oldNotes);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString(),
+ getRevId(notes, 3), ps3Comment.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertLogEqualExceptTrees(newLog, oldLog);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(3)))
+ .containsExactly(ps1Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(4)))
+ .containsExactly(ps1Comment.key, false, ps2Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(5)))
+ .containsExactly(ps1Comment.key, false, ps2Comment.key, false, ps3Comment.key, false);
+ }
+
+ @FunctionalInterface
+ interface MigrateFunction {
+ boolean call(
+ Project.NameKey project,
+ Repository repo,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru)
+ throws Exception;
+ }
+
+ private void migrate(Project.NameKey project, MigrateFunction func, int expectedCommands)
+ throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = repo.newObjectInserter()) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ assertThat(func.call(project, repo, rw, ins, bru)).isTrue();
+ assertThat(bru.getCommands()).hasSize(expectedCommands);
+ if (!bru.getCommands().isEmpty()) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+ }
+ }
+ }
+
+ private Comment newComment(ChangeNotes notes, int psNum, String message) {
+ return newComment(notes, psNum, message, changeOwner);
+ }
+
+ private Comment newComment(
+ ChangeNotes notes, int psNum, String message, IdentifiedUser commenter) {
+ return newComment(
+ new PatchSet.Id(notes.getChangeId(), psNum),
+ "filename",
+ "uuid-" + uuidCounter.getAndIncrement(),
+ null,
+ 0,
+ commenter,
+ null,
+ TimeUtil.nowTs(),
+ message,
+ (short) 1,
+ getRevId(notes, psNum).get(),
+ false);
+ }
+
+ private void incrementPatchSet(Change c) throws Exception {
+ TestChanges.incrementPatchSet(c);
+ RevCommit commit = tr.commit().message("PS" + c.currentPatchSetId().get()).create();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setCommit(rw, commit);
+ update.commit();
+ }
+
+ private static RevId getRevId(ChangeNotes notes, int psNum) {
+ PatchSet.Id psId = new PatchSet.Id(notes.getChangeId(), psNum);
+ PatchSet ps = notes.getPatchSets().get(psId);
+ checkArgument(ps != null, "no patch set %s: %s", psNum, notes.getPatchSets());
+ return ps.getRevision();
+ }
+
+ private static ListMultimap<RevId, String> getToStringRepresentations(
+ ListMultimap<RevId, Comment> comments) {
+ // Use string representation for equality comparison in this test, because Comment#equals only
+ // compares keys.
+ return Multimaps.transformValues(comments, Comment::toString);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMapForPublishedComments(
+ ChangeNotes notes, ObjectId metaId) throws Exception {
+ return getLegacyFormatMap(project, notes.getChangeId(), metaId, Status.PUBLISHED);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMapForDraftComments(
+ ChangeNotes notes, ObjectId metaId) throws Exception {
+ return getLegacyFormatMap(allUsers, notes.getChangeId(), metaId, Status.DRAFT);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMap(
+ Project.NameKey project, Change.Id changeId, ObjectId metaId, Status status)
+ throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ ObjectReader reader = repo.newObjectReader();
+ RevWalk rw = new RevWalk(reader)) {
+ NoteMap noteMap = NoteMap.read(reader, rw.parseCommit(metaId));
+ RevisionNoteMap<ChangeRevisionNote> revNoteMap =
+ RevisionNoteMap.parse(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ changeId,
+ reader,
+ noteMap,
+ status);
+ return revNoteMap
+ .revisionNotes
+ .values()
+ .stream()
+ .flatMap(crn -> crn.getComments().stream())
+ .collect(toImmutableMap(c -> c.key, c -> c.legacyFormat));
+ }
+ }
+
+ private ImmutableList<RevCommit> log(Project.NameKey project, String refName) throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo)) {
+ rw.sort(RevSort.TOPO);
+ rw.sort(RevSort.REVERSE);
+ Ref ref = repo.exactRef(refName);
+ checkArgument(ref != null, "missing ref: %s", refName);
+ rw.markStart(rw.parseCommit(ref.getObjectId()));
+ return ImmutableList.copyOf(rw);
+ }
+ }
+
+ private static void assertLogEqualExceptTrees(
+ ImmutableList<RevCommit> actualLog, ImmutableList<RevCommit> expectedLog) {
+ assertThat(actualLog).hasSize(expectedLog.size());
+ for (int i = 0; i < expectedLog.size(); i++) {
+ RevCommit actual = actualLog.get(i);
+ RevCommit expected = expectedLog.get(i);
+ assertThat(actual.getAuthorIdent())
+ .named("author of entry %s", i)
+ .isEqualTo(expected.getAuthorIdent());
+ assertThat(actual.getCommitterIdent())
+ .named("committer of entry %s", i)
+ .isEqualTo(expected.getCommitterIdent());
+ assertThat(actual.getFullMessage()).named("message of entry %s", i).isNotNull();
+ assertThat(actual.getFullMessage())
+ .named("message of entry %s", i)
+ .isEqualTo(expected.getFullMessage());
+ }
+ }
+
+ private void assertNoDifferences(ChangeNotes actual, ChangeNotes expected) throws Exception {
+ assertThat(
+ ChangeBundle.fromNotes(commentsUtil, actual)
+ .differencesFrom(ChangeBundle.fromNotes(commentsUtil, expected)))
+ .isEmpty();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
index 314941e..086dd65 100644
--- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
+++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
@@ -49,7 +49,7 @@
bind(PrologEnvironment.Args.class)
.toInstance(
new PrologEnvironment.Args(
- null, null, null, null, null, null, null, cfg, null));
+ null, null, null, null, null, null, null, cfg, null, null));
}
});
}
diff --git a/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java b/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
index f709f55..8622b32 100644
--- a/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
+++ b/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
@@ -30,18 +30,18 @@
@Test
public void labelWithSpacesIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("Label with spaces"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name--Labelwithspaces");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-Labelwithspaces");
}
@Test
public void labelStartingWithADashIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("-dashed-label"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name---dashed-label");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-dashed-label");
}
@Test
public void labelWithInvalidCharactersIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("*urgent*"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name--urgent");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-urgent");
}
}
diff --git a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
index ed94c97..c4844b1 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
+import com.google.gerrit.server.config.GerritServerIdProvider;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -94,7 +95,11 @@
Config cfg = new Config();
cfg.setString("user", null, "name", "Gerrit Code Review");
cfg.setString("user", null, "email", "gerrit@localhost");
-
+ cfg.setString(
+ GerritServerIdProvider.SECTION,
+ null,
+ GerritServerIdProvider.KEY,
+ "1234567");
bind(Config.class) //
.annotatedWith(GerritServerConfig.class) //
.toInstance(cfg);
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index f4497d3..adf2d4f8 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,8 +1,8 @@
load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar")
-_JGIT_VERS = "5.0.0.201806131550-r"
+_JGIT_VERS = "5.0.1.201806211838-r"
-_DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot
+_DOC_VERS = "5.0.0.201806131550-r" # Set to _JGIT_VERS unless using a snapshot
JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs"
@@ -26,28 +26,28 @@
name = "jgit-lib",
artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "596edbf705924bd2defd9cfc83b29b1bceb56308",
- src_sha1 = "503a4c069baa672d3ff323d36c9b9a3a5edffc94",
+ sha1 = "dbba66a425d2153ccd749d0ba9c075b0ba424655",
+ src_sha1 = "c85725a96e20d940fe20e1be4ddf50133c322f65",
unsign = True,
)
maven_jar(
name = "jgit-servlet",
artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "be2b42633f4973921e4c4b976f592f12f33bffd9",
+ sha1 = "5d9cd43e880d49f14501ac48d59b55905f4ec5bf",
unsign = True,
)
maven_jar(
name = "jgit-archive",
artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "3948643a6e07375ed0e28f35d75c0deb1cd183d8",
+ sha1 = "1d94e2bfa505dd719f62cfb036295022543af17e",
)
maven_jar(
name = "jgit-junit",
artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "d57d749ad97f42d570236e7981f36458033bfda9",
+ sha1 = "f848735061fab81f2863f68cca8d533ff403c765",
unsign = True,
)
diff --git a/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior.html b/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior.html
new file mode 100644
index 0000000..68000bc
--- /dev/null
+++ b/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior.html
@@ -0,0 +1,75 @@
+<!--
+@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.
+-->
+<script>
+(function(window) {
+ 'use strict';
+
+ window.Gerrit = window.Gerrit || {};
+
+ /** @polymerBehavior Gerrit.SafeTypes */
+ Gerrit.SafeTypes = {};
+
+ const SAFE_URL_PATTERN = /^(https?:\/\/|mailto:|\/|#)/i;
+
+ /**
+ * Wraps a string to be used as a URL. An error is thrown if the string cannot
+ * be considered safe.
+ * @constructor
+ * @param {string} url the unwrapped, potentially unsafe URL.
+ */
+ Gerrit.SafeTypes.SafeUrl = function(url) {
+ if (!SAFE_URL_PATTERN.test(url)) {
+ throw new Error(`URL not marked as safe: ${url}`);
+ }
+ this._url = url;
+ };
+
+ /**
+ * Get the string representation of the safe URL.
+ * @returns {string}
+ */
+ Gerrit.SafeTypes.SafeUrl.prototype.asString = function() {
+ return this._url;
+ };
+
+ Gerrit.SafeTypes.safeTypesBridge = function(value, type) {
+ // If the value is being bound to a URL, ensure the value is wrapped in the
+ // SafeUrl type first. If the URL is not safe, allow the SafeUrl constructor
+ // to surface the error.
+ if (type === 'URL') {
+ let safeValue = null;
+ if (value instanceof Gerrit.SafeTypes.SafeUrl) {
+ safeValue = value;
+ } else if (typeof value === 'string') {
+ safeValue = new Gerrit.SafeTypes.SafeUrl(value);
+ }
+ if (safeValue) {
+ return safeValue.asString();
+ }
+ }
+
+ // If the value is being bound to a string or a constant, then the string
+ // can be used as is.
+ if (type === 'STRING' || type === 'CONSTANT') {
+ return value;
+ }
+
+ // Otherwise fail.
+ throw new Error(`Refused to bind value as ${type}: ${value}`);
+ };
+})(window);
+</script>
diff --git a/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior_test.html b/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior_test.html
new file mode 100644
index 0000000..bc16b39
--- /dev/null
+++ b/polygerrit-ui/app/behaviors/safe-types-behavior/safe-types-behavior_test.html
@@ -0,0 +1,121 @@
+<!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.
+-->
+
+<title>safe-types-behavior</title>
+
+<script src="../../bower_components/webcomponentsjs/webcomponents.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="safe-types-behavior.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <safe-types-element></safe-types-element>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-tooltip-behavior tests', () => {
+ let element;
+ let sandbox;
+
+ suiteSetup(() => {
+ Polymer({
+ is: 'safe-types-element',
+ behaviors: [Gerrit.SafeTypes],
+ });
+ });
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('SafeUrl accepts valid urls', () => {
+ function accepts(url) {
+ const safeUrl = new element.SafeUrl(url);
+ assert.isOk(safeUrl);
+ assert.equal(url, safeUrl.asString());
+ }
+ accepts('http://www.google.com/');
+ accepts('https://www.google.com/');
+ accepts('HtTpS://www.google.com/');
+ accepts('//www.google.com/');
+ accepts('/c/1234/file/path.html@45');
+ accepts('#hash-url');
+ accepts('mailto:name@example.com');
+ });
+
+ test('SafeUrl rejects invalid urls', () => {
+ function rejects(url) {
+ assert.throws(() => { new element.SafeUrl(url); });
+ }
+ rejects('javascript://alert("evil");');
+ rejects('ftp:example.com');
+ rejects('data:text/html,scary business');
+ });
+
+ suite('safeTypesBridge', () => {
+ function acceptsString(value, type) {
+ assert.equal(Gerrit.SafeTypes.safeTypesBridge(value, type),
+ value);
+ }
+
+ function rejects(value, type) {
+ assert.throws(() => { Gerrit.SafeTypes.safeTypesBridge(value, type); });
+ }
+
+ test('accepts valid URL strings', () => {
+ acceptsString('/foo/bar', 'URL');
+ acceptsString('#baz', 'URL');
+ });
+
+ test('rejects invalid URL strings', () => {
+ rejects('javascript://void();', 'URL');
+ });
+
+ test('accepts SafeUrl values', () => {
+ const url = '/abc/123';
+ const safeUrl = new element.SafeUrl(url);
+ assert.equal(Gerrit.SafeTypes.safeTypesBridge(safeUrl, 'URL'), url);
+ });
+
+ test('rejects non-string or non-SafeUrl types', () => {
+ rejects(3.1415926, 'URL');
+ });
+
+ test('accepts any binding to STRING or CONSTANT', () => {
+ acceptsString('foo/bar/baz', 'STRING');
+ acceptsString('lorem ipsum dolor', 'CONSTANT');
+ });
+
+ test('rejects all other types', () => {
+ rejects('foo', 'JAVASCRIPT');
+ rejects('foo', 'HTML');
+ rejects('foo', 'RESOURCE_URL');
+ rejects('foo', 'STYLE');
+ });
+ });
+ });
+</script>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
index febd446..5d45811 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
@@ -111,13 +111,13 @@
</a>
<gr-select
id="force"
- class$="[[_computeForceClass(permission)]]"
+ class$="[[_computeForceClass(permission, rule.value.action)]]"
bind-value="{{rule.value.force}}"
on-change="_handleValueChange">
<select disabled$="[[!editing]]">
<template
is="dom-repeat"
- items="[[_computeForceOptions(permission)]]">
+ items="[[_computeForceOptions(permission, rule.value.action)]]">
<option value="[[item.value]]">[[item.name]]</option>
</template>
</select>
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
index 4af4952..b99125c 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
@@ -33,22 +33,24 @@
'INTERACTIVE',
];
- const DROPDOWN_OPTIONS = [
- 'ALLOW',
- 'DENY',
- 'BLOCK',
- ];
+ const Action = {
+ ALLOW: 'ALLOW',
+ DENY: 'DENY',
+ BLOCK: 'BLOCK',
+ };
- const FORCE_PUSH_OPTIONS = [
- {
- name: 'Block all pushes, block force push only',
- value: false,
- },
- {
- name: 'Allow fast-forward only push, allow all pushes',
- value: true,
- },
- ];
+ const DROPDOWN_OPTIONS = [Action.ALLOW, Action.DENY, Action.BLOCK];
+
+ const ForcePushOptions = {
+ ALLOW: [
+ {name: 'Allow pushing (but not force pushing)', value: false},
+ {name: 'Allow pushing with or without force', value: true},
+ ],
+ BLOCK: [
+ {name: 'Block pushing with or without force', value: false},
+ {name: 'Block force pushing', value: true},
+ ],
+ };
const FORCE_EDIT_OPTIONS = [
{
@@ -117,13 +119,17 @@
this._setOriginalRuleValues(rule.value);
},
- _computeForce(permission) {
- return this.permissionValues.push.id === permission ||
- this.permissionValues.editTopicName.id === permission;
+ _computeForce(permission, action) {
+ if (this.permissionValues.push.id === permission &&
+ action !== Action.DENY) {
+ return true;
+ }
+
+ return this.permissionValues.editTopicName.id === permission;
},
- _computeForceClass(permission) {
- return this._computeForce(permission) ? 'force' : '';
+ _computeForceClass(permission, action) {
+ return this._computeForce(permission, action) ? 'force' : '';
},
_computeGroupPath(group) {
@@ -156,9 +162,15 @@
return classList.join(' ');
},
- _computeForceOptions(permission) {
+ _computeForceOptions(permission, action) {
if (permission === this.permissionValues.push.id) {
- return FORCE_PUSH_OPTIONS;
+ if (action === Action.ALLOW) {
+ return ForcePushOptions.ALLOW;
+ } else if (action === Action.BLOCK) {
+ return ForcePushOptions.BLOCK;
+ } else {
+ return [];
+ }
} else if (permission === this.permissionValues.editTopicName.id) {
return FORCE_EDIT_OPTIONS;
}
@@ -166,6 +178,7 @@
},
_getDefaultRuleValues(permission, label) {
+ const ruleAction = Action.ALLOW;
const value = {};
if (permission === 'priority') {
value.action = PRIORITY_OPTIONS[0];
@@ -173,16 +186,17 @@
} else if (label) {
value.min = label.values[0].value;
value.max = label.values[label.values.length - 1].value;
- } else if (this._computeForce(permission)) {
- value.force = this._computeForceOptions(permission)[0].value;
+ } else if (this._computeForce(permission, ruleAction)) {
+ value.force =
+ this._computeForceOptions(permission, ruleAction)[0].value;
}
value.action = DROPDOWN_OPTIONS[0];
return value;
},
_setDefaultRuleValues() {
- this.set('rule.value',
- this._getDefaultRuleValues(this.permission, this.label));
+ this.set('rule.value', this._getDefaultRuleValues(this.permission,
+ this.label));
},
_computeOptions(permission) {
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
index 5b6f947..f85c2b2 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
@@ -50,16 +50,16 @@
suite('unit tests', () => {
test('_computeForce, _computeForceClass, and _computeForceOptions',
() => {
- const FORCE_PUSH_OPTIONS = [
- {
- name: 'Block all pushes, block force push only',
- value: false,
- },
- {
- name: 'Allow fast-forward only push, allow all pushes',
- value: true,
- },
- ];
+ const ForcePushOptions = {
+ ALLOW: [
+ {name: 'Allow pushing (but not force pushing)', value: false},
+ {name: 'Allow pushing with or without force', value: true},
+ ],
+ BLOCK: [
+ {name: 'Block pushing with or without force', value: false},
+ {name: 'Block force pushing', value: true},
+ ],
+ };
const FORCE_EDIT_OPTIONS = [
{
@@ -72,10 +72,26 @@
},
];
let permission = 'push';
- assert.isTrue(element._computeForce(permission));
- assert.equal(element._computeForceClass(permission), 'force');
- assert.deepEqual(element._computeForceOptions(permission),
- FORCE_PUSH_OPTIONS);
+ let action = 'ALLOW';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action),
+ 'force');
+ assert.deepEqual(element._computeForceOptions(permission, action),
+ ForcePushOptions.ALLOW);
+
+ action = 'BLOCK';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action),
+ 'force');
+ assert.deepEqual(element._computeForceOptions(permission, action),
+ ForcePushOptions.BLOCK);
+
+ action = 'DENY';
+ assert.isFalse(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action), '');
+ assert.equal(
+ element._computeForceOptions(permission, action).length, 0);
+
permission = 'editTopicName';
assert.isTrue(element._computeForce(permission));
assert.equal(element._computeForceClass(permission), 'force');
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index dc41f59..9930bf5 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -21,6 +21,8 @@
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
+ const LABEL_PREFIX_INVALID_PROLOG = 'Invalid-Prolog-Rules-Label-Name--';
+
Polymer({
is: 'gr-change-list',
@@ -180,7 +182,11 @@
},
_computeLabelShortcut(labelName) {
+ if (labelName.startsWith(LABEL_PREFIX_INVALID_PROLOG)) {
+ labelName = labelName.slice(LABEL_PREFIX_INVALID_PROLOG.length);
+ }
return labelName.split('-').reduce((a, i) => {
+ if (!i) { return a; }
return a + i[0].toUpperCase();
}, '');
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index 3dd8c9d..9ce5764 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -127,7 +127,11 @@
assert.equal(element._computeLabelShortcut('PolyGerrit-Review'), 'PR');
assert.equal(element._computeLabelShortcut('polygerrit-review'), 'PR');
assert.equal(element._computeLabelShortcut(
+ 'Invalid-Prolog-Rules-Label-Name--Verified'), 'V');
+ assert.equal(element._computeLabelShortcut(
'Some-Special-Label-7'), 'SSL7');
+ assert.equal(element._computeLabelShortcut('--Too----many----dashes---'),
+ 'TMD');
});
test('colspans', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
index c94a716..582c83b 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
@@ -37,7 +37,6 @@
threshold="[[suggestFrom]]"
query="[[query]]"
allow-non-suggested-values="[[allowAnyInput]]"
- no-debounce
on-commit="_handleInputCommit"
clear-on-commit
warn-uncommitted
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
index 5cb3b77..86e3903 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
@@ -69,6 +69,8 @@
type: String,
observer: '_inputTextChanged',
},
+
+ _loggedIn: Boolean,
},
behaviors: [
@@ -79,6 +81,9 @@
this.$.restAPI.getConfig().then(cfg => {
this._config = cfg;
});
+ this.$.restAPI.getLoggedIn().then(loggedIn => {
+ this._loggedIn = loggedIn;
+ });
},
get focusStart() {
@@ -144,7 +149,9 @@
},
_getReviewerSuggestions(input) {
- if (!this.change || !this.change._number) { return Promise.resolve([]); }
+ if (!this.change || !this.change._number || !this._loggedIn) {
+ return Promise.resolve([]);
+ }
const api = this.$.restAPI;
const xhr = this.allowAnyUser ?
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
index 7d5ddd8..03a0be8 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
@@ -65,7 +65,7 @@
let suggestion3;
let element;
- setup(() => {
+ setup(done => {
owner = makeAccount();
existingReviewer1 = makeAccount();
existingReviewer2 = makeAccount();
@@ -78,6 +78,10 @@
},
};
+ stub('gr-rest-api-interface', {
+ getLoggedIn() { return Promise.resolve(true); },
+ });
+
element = fixture('basic');
element.change = {
_number: 42,
@@ -88,6 +92,7 @@
},
};
sandbox = sinon.sandbox.create();
+ return flush(done);
});
teardown(() => {
@@ -168,6 +173,19 @@
}).then(done);
});
});
+
+ test('_getReviewerSuggestions short circuits when logged out', () => {
+ // API call is already stubbed.
+ const xhrSpy = element.$.restAPI.getChangeSuggestedReviewers;
+ element._loggedIn = false;
+ return element._getReviewerSuggestions('').then(() => {
+ assert.isFalse(xhrSpy.called);
+ element._loggedIn = true;
+ return element._getReviewerSuggestions('').then(() => {
+ assert.isTrue(xhrSpy.called);
+ });
+ });
+ });
});
test('allowAnyUser', done => {
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 4f466f4..da0d167 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
@@ -54,7 +54,7 @@
gr-button,
gr-dropdown {
/* px because don't have the same font size */
- margin-left: 12px;
+ margin-left: 8px;
}
#actionLoadingMessage {
align-items: center;
@@ -70,6 +70,14 @@
margin-right: .2rem;
width: 1.2rem;
}
+ gr-button {
+ min-height: 2.25em;
+ }
+ gr-dropdown {
+ --gr-button: {
+ min-height: 2.25em;
+ }
+ }
#moreActions iron-icon {
margin: 0;
}
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 29545c3..5a56475 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
@@ -121,7 +121,6 @@
}
.changeMetadata {
border-right: 1px solid var(--border-color);
- font-size: var(--font-size-small);
padding: 1em 0;
}
/* Prevent plugin text from overflowing. */
@@ -190,7 +189,6 @@
overflow: hidden;
}
#relatedChanges {
- font-size: var(--font-size-small);
}
#relatedChanges.collapsed {
margin-bottom: 1.1em;
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
index c0a09f3..234d903 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
@@ -78,7 +78,7 @@
assert.isOk(element._computeShowWebLink(element.change,
element.commitInfo, element.serverConfig));
assert.equal(element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig), '../../link-url');
+ element.serverConfig), 'link-url');
});
test('does not relativize web links that begin with scheme', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
index ad14a18..299508b 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
@@ -30,6 +30,12 @@
p {
margin-bottom: 1em;
}
+ @media screen and (max-width: 50em) {
+ #dialog {
+ min-width: inherit;
+ width: 100%;
+ }
+ }
</style>
<gr-confirm-dialog
id="dialog"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 41e5227..93c351c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -178,10 +178,8 @@
display: none;
}
label.show-hide {
- color: var(--link-color);
cursor: pointer;
display: block;
- font-size: var(--font-size-small);
min-width: 2em;
}
gr-diff {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 6adc286..3b4f1ec 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -309,12 +309,7 @@
if (!weblinks) { return null; }
const weblink = weblinks.find(this._isDirectCommit);
if (!weblink) { return null; }
- const url = weblink.url;
- if (url.startsWith('https:') || url.startsWith('http:')) {
- return url;
- } else {
- return `../../${url}`;
- }
+ return weblink.url;
},
_getChangeWeblinks({repo, commit, options: {weblinks}}) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
index 4599780..c3a1de4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
@@ -60,7 +60,6 @@
}
.descriptionText {
margin-left: .5rem;
- font-size: var(--font-size-small);
font-style: italic;
}
</style>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index b261a34..568c11f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -213,7 +213,6 @@
}
.resolve label {
color: var(--comment-text-color);
- font-size: var(--font-size-small);
}
gr-confirm-dialog .main {
display: flex;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index edee1ae..1b5203e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -171,7 +171,6 @@
}
.fullFileName {
display: block;
- font-size: var(--font-size-small);
font-style: italic;
min-width: 50%;
padding: 0 .1em;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 540df98..718fa17 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -40,7 +40,7 @@
}
.diffContainer {
display: flex;
- font: var(--font-size-small) var(--monospace-font-family);
+ font-family: var(--monospace-font-family);
@apply --diff-container-styles;
}
.diffContainer.hiddenscroll {
@@ -89,7 +89,7 @@
.lineNum,
.content {
/* Set font size based the user's diff preference. */
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
vertical-align: top;
white-space: pre;
}
@@ -185,7 +185,7 @@
border-bottom: 1px solid var(--border-color);
color: var(--link-color);
font-family: var(--monospace-font-family);
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
padding: 0.5em 0 0.5em 4em;
}
#sizeWarning {
@@ -209,7 +209,7 @@
td.blame {
display: none;
font-family: var(--font-family);
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
padding: 0 .5em;
white-space: pre;
}
@@ -235,7 +235,7 @@
/** Since the line limit position is determined by charachter size, blank
lines also need to have the same font size as everything else */
.full-width .blank {
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
}
/** Support the line length indicator **/
.full-width td.content,
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index efcefe2..e7bd965 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -30,10 +30,12 @@
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../bower_components/polymer-resin/standalone/polymer-resin.html">
+<link rel="import" href="../behaviors/safe-types-behavior/safe-types-behavior.html">
<script>
security.polymer_resin.install({
allowedIdentifierPrefixes: [''],
reportHandler: security.polymer_resin.CONSOLE_LOGGING_REPORT_HANDLER,
+ safeTypesBridge: Gerrit.SafeTypes.safeTypesBridge,
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
index 8fff850..6564abe 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -30,8 +30,6 @@
--background-color: var(--button-background-color, var(--default-button-background-color));
--text-color: var(--default-button-text-color);
display: inline-block;
- font-family: var(--font-family-bold);
- font-size: var(--font-size-small);
position: relative;
}
:host([hidden]) {
@@ -52,7 +50,7 @@
justify-content: center;
margin: var(--margin, 0);
min-width: var(--border, 0);
- padding: var(--padding, 5px 10px);
+ padding: var(--padding, 4px 8px);
@apply --gr-button;
}
paper-button:hover {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
index 3abe28b..f4b120a 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
@@ -78,7 +78,6 @@
}
.bottomContent {
color: var(--deemphasized-text-color);
- font-size: var(--font-size-small);
/*
* Should be 16px when the base font size is 13px (browser default of
* 16px.
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 6e61c8e..0310e58 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -204,6 +204,7 @@
'gr-patch-set-behavior/gr-patch-set-behavior_test.html',
'gr-path-list-behavior/gr-path-list-behavior_test.html',
'gr-tooltip-behavior/gr-tooltip-behavior_test.html',
+ 'safe-types-behavior/safe-types-behavior_test.html',
];
/* eslint-enable max-len */
for (let file of behaviors) {
