Merge "Add explicit return values for eslint 5.* compliance"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 397b99a..56200c4 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -841,6 +841,17 @@
This category permits users to delete their own changes if they are not merged
yet. This means only own changes that are open or abandoned can be deleted.
+[[category_delete_changes]]
+=== Delete Changes
+
+This category permits users to delete other users' changes if they are not merged
+yet. This means only changes that are open or abandoned can be deleted.
+
+Having this permission implies having the link:#category_delete_own_changes[
+Delete Own Changes] permission.
+
+Administrators may always delete changes without having this permission.
+
[[category_edit_topic_name]]
=== Edit Topic Name
diff --git a/Documentation/cmd-set-account.txt b/Documentation/cmd-set-account.txt
index 884c8cc..276306e 100644
--- a/Documentation/cmd-set-account.txt
+++ b/Documentation/cmd-set-account.txt
@@ -12,6 +12,7 @@
[--preferred-email <EMAIL>]
[--add-ssh-key - | <KEY>]
[--delete-ssh-key - | <KEY> | ALL]
+ [--generate-http-password]
[--http-password <PASSWORD>]
[--clear-http-password] <USER>
--
@@ -25,8 +26,9 @@
verification step we force within the UI.
== ACCESS
-Caller must be a member of the privileged 'Administrators' group,
-or have been granted
+Users can call this to update their own accounts. To update a different
+account, a caller must be a member of the privileged 'Administrators'
+group, or have been granted
link:access-control.html#capability_modifyAccount[the 'Modify Account' global capability].
For security reasons only the members of the privileged 'Administrators'
group can add or delete SSH keys for a user.
@@ -93,6 +95,11 @@
May be supplied more than once to delete multiple SSH
keys in a single command execution.
+--generate-http-password::
+ Generate a new random HTTP password for the user account
+ similar to the web ui. The password will be output to the
+ user on success with a line: `New password: <PASSWORD>`.
+
--http-password::
Set the HTTP password for the user account.
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 71441d8..da43d9f 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2989,6 +2989,9 @@
merged into the default `_doc` type. The latter is also used for the accounts and
groups indices starting with Elasticsearch 6.2.
+Note that when Gerrit is configured to use Elasticsearch, the Elasticsearch
+server(s) must be reachable during the site initialization.
+
[[elasticsearch.prefix]]elasticsearch.prefix::
+
This setting can be used to prefix index names to allow multiple Gerrit
@@ -2997,17 +3000,17 @@
+
Not set by default.
-[[elasticsearch.username]]elasticsearch.username::
+[[elasticsearch.server]]elasticsearch.server::
+
-Username used to connect to Elasticsearch.
+Elasticsearch server URI in the form `http[s]://hostname:port`. The `port` is
+optional and defaults to `9200` if not specified.
+
-Not set by default.
-
-[[elasticsearch.password]]elasticsearch.password::
+At least one server must be specified. May be specified multiple times to
+configure multiple Elasticsearch servers.
+
-Password used to connect to Elasticsearch.
-+
-Not set by default.
+Note that the site initialization program only allows to configure a single
+server. To configure multiple servers the `gerrit.config` file must be edited
+manually.
[[elasticsearch.maxRetryTimeout]]elasticsearch.maxRetryTimeout::
+
@@ -3017,28 +3020,29 @@
+
Defaults to `30000 ms`.
-==== Elasticsearch server(s) configuration
+==== Elasticsearch Security
-Each section corresponds to one Elasticsearch server.
+When security is enabled in Elasticsearch, the username and password must be provided.
+Note that the same username and password are used for all servers.
-[[elasticsearch.name.protocol]]elasticsearch.name.protocol::
-+
-Elasticsearch server protocol. Can be `http` or `https`.
-+
-Defaults to `http`.
+For further information about Elasticsearch security, please refer to the documentation:
-[[elasticsearch.name.hostname]]elasticsearch.name.hostname::
-+
-Elasticsearch server hostname.
-+
-Defaults to `localhost`.
+* link:https://www.elastic.co/guide/en/elasticsearch/plugins/2.4/security.html[Elasticsearch 2.4]
+* link:https://www.elastic.co/guide/en/x-pack/5.6/security-getting-started.html[Elasticsearch 5.6]
+* link:https://www.elastic.co/guide/en/x-pack/6.2/security-getting-started.html[Elasticsearch 6.2]
+* link:https://www.elastic.co/guide/en/elastic-stack-overview/6.3/security-getting-started.html[Elasticsearch 6.3]
-[[elasticsearch.name.port]]elasticsearch.name.port::
+[[elasticsearch.username]]elasticsearch.username::
+
-Elasticsearch server port.
+Username used to connect to Elasticsearch.
+
-Defaults to `9200`.
+If a password is set, defaults to `elastic`, otherwise not set by default.
+[[elasticsearch.password]]elasticsearch.password::
++
+Password used to connect to Elasticsearch.
++
+Not set by default.
[[ldap]]
=== Section ldap
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index cf78c6d..3db621d 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -368,6 +368,9 @@
ignored if the label doesn't apply for that branch.
Additionally, the `branch` modifier has no effect when the submit rule
is customized in the rules.pl of the project or inherited from parent projects.
+Branch can be a ref pattern similar to what is documented
+link:access-control.html#reference[here], but must not contain `${username}` or
+`${shardeduserid}`.
[[label_example]]
=== Example
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 1b28424..35ff40b 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2313,6 +2313,16 @@
e.g. a plugin can provide a list of servers on which the change was
deployed.
+[[change-report-formatting]]
+== Change Report Formatting
+
+When a change is pushed for review from the command line, Gerrit reports
+the change(s) received with their URL and subject.
+
+By implementing the
+`com.google.gerrit.server.git.ChangeReportFormatter` interface, a plugin
+may change the formatting of the report.
+
[[links-to-external-tools]]
== Links To External Tools
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 3804734d..fb94b67 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -258,7 +258,9 @@
For open or abandoned changes, the `Delete Change` button will be available
and if the user is the change owner and is granted the
link:access-control.html#category_delete_own_changes[Delete Own Changes]
-permission or if they are an administrator.
+permission, if they are granted the
+link:access-control.html#category_delete_changes[Delete Changes] permission,
+or if they are an administrator.
** [[plugin-actions]]Further actions may be available if plugins are installed.
diff --git a/WORKSPACE b/WORKSPACE
index 51c3618..70200d1 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -906,8 +906,8 @@
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.0",
- sha1 = "a95ef38262ef499aa07cdb736f4a47cb19162654",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.1",
+ sha1 = "99de036a2cd99dbecec1cc84f5d0e19032e74fa7",
)
JACKSON_VERSION = "2.8.9"
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index af23e39..de6108e 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -27,6 +27,7 @@
public static final String CREATE_SIGNED_TAG = "createSignedTag";
public static final String CREATE_TAG = "createTag";
public static final String DELETE = "delete";
+ public static final String DELETE_CHANGES = "deleteChanges";
public static final String DELETE_OWN_CHANGES = "deleteOwnChanges";
public static final String EDIT_ASSIGNEE = "editAssignee";
public static final String EDIT_HASHTAGS = "editHashtags";
@@ -58,6 +59,7 @@
NAMES_LC.add(CREATE_SIGNED_TAG.toLowerCase());
NAMES_LC.add(CREATE_TAG.toLowerCase());
NAMES_LC.add(DELETE.toLowerCase());
+ NAMES_LC.add(DELETE_CHANGES.toLowerCase());
NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase());
NAMES_LC.add(EDIT_ASSIGNEE.toLowerCase());
NAMES_LC.add(EDIT_HASHTAGS.toLowerCase());
diff --git a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
index 0148bf1..8d29d21 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
@@ -14,16 +14,18 @@
package com.google.gerrit.elasticsearch;
-import com.google.common.base.MoreObjects;
+import static com.google.common.base.MoreObjects.firstNonNull;
+
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.ProvisionException;
import com.google.inject.Singleton;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
-import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.apache.http.HttpHost;
import org.eclipse.jgit.lib.Config;
@@ -32,9 +34,16 @@
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";
+ static final String SECTION_ELASTICSEARCH = "elasticsearch";
+ static final String KEY_PASSWORD = "password";
+ static final String KEY_USERNAME = "username";
+ static final String KEY_MAX_RETRY_TIMEOUT = "maxRetryTimeout";
+ static final String KEY_PREFIX = "prefix";
+ static final String KEY_SERVER = "server";
+ static final String DEFAULT_PORT = "9200";
+ static final String DEFAULT_USERNAME = "elastic";
+ static final int DEFAULT_MAX_RETRY_TIMEOUT_MS = 30000;
+ static final TimeUnit MAX_RETRY_TIMEOUT_UNIT = TimeUnit.MILLISECONDS;
private final Config cfg;
private final List<HttpHost> hosts;
@@ -47,31 +56,40 @@
@Inject
ElasticConfiguration(@GerritServerConfig Config cfg) {
this.cfg = cfg;
- this.username = cfg.getString("elasticsearch", null, "username");
- this.password = cfg.getString("elasticsearch", null, "password");
+ this.password = cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD);
+ this.username =
+ password == null
+ ? null
+ : firstNonNull(
+ cfg.getString(SECTION_ELASTICSEARCH, null, KEY_USERNAME), DEFAULT_USERNAME);
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.hosts = Collections.singletonList(httpHost);
- } else {
- 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);
+ cfg.getTimeUnit(
+ SECTION_ELASTICSEARCH,
+ null,
+ KEY_MAX_RETRY_TIMEOUT,
+ DEFAULT_MAX_RETRY_TIMEOUT_MS,
+ MAX_RETRY_TIMEOUT_UNIT);
+ this.prefix = Strings.nullToEmpty(cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PREFIX));
+ this.hosts = new ArrayList<>();
+ for (String server : cfg.getStringList(SECTION_ELASTICSEARCH, null, KEY_SERVER)) {
+ try {
+ URI uri = new URI(server);
+ int port = uri.getPort();
+ HttpHost httpHost =
+ new HttpHost(
+ uri.getHost(), port == -1 ? Integer.valueOf(DEFAULT_PORT) : port, uri.getScheme());
this.hosts.add(httpHost);
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ logger.atSevere().log("Invalid server URI %s: %s", server, e.getMessage());
}
}
- logger.atInfo().log("Elasticsearch hosts: %s", hosts);
+ if (hosts.isEmpty()) {
+ throw new ProvisionException("No valid Elasticsearch servers configured");
+ }
+
+ logger.atInfo().log("Elasticsearch servers: %s", hosts);
}
Config getConfig() {
@@ -85,8 +103,4 @@
String getIndexName(String name, int schemaVersion) {
return String.format("%s%s_%04d", prefix, name, schemaVersion);
}
-
- private String getString(Config cfg, String subsection, String name, String defaultValue) {
- return MoreObjects.firstNonNull(cfg.getString("elasticsearch", subsection, name), defaultValue);
- }
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 1196e47..a454c00 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -337,7 +337,7 @@
}
}
if (viewData.view == null) {
- viewData = view(rsrc, rc, req.getMethod(), path);
+ viewData = view(rc, req.getMethod(), path);
}
}
checkRequiresCapability(viewData);
@@ -392,7 +392,7 @@
}
}
if (viewData.view == null) {
- viewData = view(rsrc, c, req.getMethod(), path);
+ viewData = view(c, req.getMethod(), path);
}
checkRequiresCapability(viewData);
}
@@ -1109,10 +1109,7 @@
}
private ViewData view(
- RestResource rsrc,
- RestCollection<RestResource, RestResource> rc,
- String method,
- List<IdString> path)
+ RestCollection<RestResource, RestResource> rc, String method, List<IdString> path)
throws AmbiguousViewException, RestApiException {
DynamicMap<RestView<RestResource>> views = rc.views();
final IdString projection = path.isEmpty() ? IdString.fromUrl("/") : path.remove(0);
@@ -1136,10 +1133,8 @@
return new ViewData(p.get(0), view);
}
view = views.get(p.get(0), "GET." + viewname);
- if (view != null && view instanceof AcceptsPost && "POST".equals(method)) {
- @SuppressWarnings("unchecked")
- AcceptsPost<RestResource> ap = (AcceptsPost<RestResource>) view;
- return new ViewData(p.get(0), ap.post(rsrc));
+ if (view != null) {
+ return new ViewData(p.get(0), view);
}
throw new ResourceNotFoundException(projection);
}
@@ -1150,10 +1145,8 @@
return new ViewData(null, core);
}
core = views.get("gerrit", "GET." + p.get(0));
- if (core instanceof AcceptsPost && "POST".equals(method)) {
- @SuppressWarnings("unchecked")
- AcceptsPost<RestResource> ap = (AcceptsPost<RestResource>) core;
- return new ViewData(null, ap.post(rsrc));
+ if (core != null) {
+ return new ViewData(null, core);
}
Map<String, RestView<RestResource>> r = new TreeMap<>();
diff --git a/java/com/google/gerrit/pgm/init/InitIndex.java b/java/com/google/gerrit/pgm/init/InitIndex.java
index ee6c440..0de08f2 100644
--- a/java/com/google/gerrit/pgm/init/InitIndex.java
+++ b/java/com/google/gerrit/pgm/init/InitIndex.java
@@ -15,7 +15,6 @@
package com.google.gerrit.pgm.init;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
import com.google.gerrit.index.SchemaDefinitions;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitFlags;
@@ -63,13 +62,7 @@
if (type == IndexType.ELASTICSEARCH) {
Section elasticsearch = sections.get("elasticsearch", null);
elasticsearch.string("Index Prefix", "prefix", "gerrit_");
- String name = ui.readString("default", "Server Name");
-
- Section defaultServer = sections.get("elasticsearch", name);
- defaultServer.select(
- "Transport protocol", "protocol", "http", Sets.newHashSet("http", "https"));
- defaultServer.string("Hostname", "hostname", "localhost");
- defaultServer.string("Port", "port", "9200");
+ elasticsearch.string("Server", "server", "http://localhost:9200");
index.string("Result window size", "maxLimit", "10000");
}
diff --git a/java/com/google/gerrit/server/ApprovalCopier.java b/java/com/google/gerrit/server/ApprovalCopier.java
index 922922c..095263e 100644
--- a/java/com/google/gerrit/server/ApprovalCopier.java
+++ b/java/com/google/gerrit/server/ApprovalCopier.java
@@ -79,7 +79,6 @@
*
* @param db review database.
* @param notes change notes for user uploading PatchSet
- * @param user user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -88,12 +87,11 @@
public void copyInReviewDb(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
- copyInReviewDb(db, notes, user, ps, rw, repoConfig, Collections.emptyList());
+ copyInReviewDb(db, notes, ps, rw, repoConfig, Collections.emptyList());
}
/**
@@ -101,7 +99,6 @@
*
* @param db review database.
* @param notes change notes for user uploading PatchSet
- * @param user user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -111,33 +108,30 @@
public void copyInReviewDb(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy)
throws OrmException {
if (PrimaryStorage.of(notes.getChange()) == PrimaryStorage.REVIEW_DB) {
- db.patchSetApprovals().insert(getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy));
+ db.patchSetApprovals().insert(getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy));
}
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
return getForPatchSet(
- db, notes, user, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
+ db, notes, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
@@ -147,13 +141,12 @@
if (ps == null) {
return Collections.emptyList();
}
- return getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy);
+ return getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy);
}
private Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
@@ -211,7 +204,7 @@
byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId()));
}
}
- return labelNormalizer.normalize(notes, user, byUser.values()).getNormalized();
+ return labelNormalizer.normalize(notes, byUser.values()).getNormalized();
} catch (IOException e) {
throw new OrmException(e);
}
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java
index 8365ddb..0d12eca 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -388,7 +388,6 @@
public Iterable<PatchSetApproval> byPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
@@ -396,13 +395,12 @@
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
}
- return copier.getForPatchSet(db, notes, user, psId, rw, repoConfig);
+ return copier.getForPatchSet(db, notes, psId, rw, repoConfig);
}
public Iterable<PatchSetApproval> byPatchSetUser(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
Account.Id accountId,
@Nullable RevWalk rw,
@@ -411,7 +409,7 @@
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSetUser(psId, accountId));
}
- return filterApprovals(byPatchSet(db, notes, user, psId, rw, repoConfig), accountId);
+ return filterApprovals(byPatchSet(db, notes, psId, rw, repoConfig), accountId);
}
public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes, PatchSet.Id c) {
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index c9ca972..236eb8c 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -202,9 +202,8 @@
ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
for (PatchSetApproval ap :
- approvalsUtil.byPatchSet(
- dbProvider.get(), notes, user, change.currentPatchSetId(), null, null)) {
- LabelType type = projectState.getLabelTypes(notes, user).byLabel(ap.getLabel());
+ approvalsUtil.byPatchSet(dbProvider.get(), notes, change.currentPatchSetId(), null, null)) {
+ LabelType type = projectState.getLabelTypes(notes).byLabel(ap.getLabel());
if (type != null
&& ap.getValue() == 1
&& type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
index d7baee2..1365cb6 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
@@ -48,7 +48,6 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
-import org.h2.jdbc.JdbcSQLException;
/**
* Hybrid in-memory and database backed cache built on H2.
@@ -341,8 +340,10 @@
b.put(keyType.get(r, 1));
}
}
- } catch (JdbcSQLException e) {
- if (e.getCause() instanceof InvalidClassException) {
+ } catch (Exception e) {
+ if (Throwables.getCausalChain(e)
+ .stream()
+ .anyMatch(InvalidClassException.class::isInstance)) {
// If deserialization failed using default Java serialization, this means we are using
// the old serialVersionUID-based invalidation strategy. In that case, authors are
// most likely bumping serialVersionUID rather than using the new versioning in the
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index c6fe93b..8ca364b 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -521,8 +521,7 @@
if (fireRevisionCreated) {
revisionCreated.fire(change, patchSet, ctx.getAccount(), ctx.getWhen(), notify);
if (approvals != null && !approvals.isEmpty()) {
- List<LabelType> labels =
- projectState.getLabelTypes(change.getDest(), ctx.getUser()).getLabelTypes();
+ List<LabelType> labels = projectState.getLabelTypes(change.getDest()).getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 2feea47..47ace73 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1171,7 +1171,6 @@
approvalsUtil.byPatchSetUser(
db.get(),
lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()),
- user,
cd.change().currentPatchSetId(),
user.getAccountId(),
null,
diff --git a/java/com/google/gerrit/server/change/LabelNormalizer.java b/java/com/google/gerrit/server/change/LabelNormalizer.java
index f4dbf21..1ec1717 100644
--- a/java/com/google/gerrit/server/change/LabelNormalizer.java
+++ b/java/com/google/gerrit/server/change/LabelNormalizer.java
@@ -26,11 +26,8 @@
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -71,43 +68,25 @@
}
}
- private final IdentifiedUser.GenericFactory userFactory;
private final ProjectCache projectCache;
@Inject
- LabelNormalizer(IdentifiedUser.GenericFactory userFactory, ProjectCache projectCache) {
- this.userFactory = userFactory;
+ LabelNormalizer(ProjectCache projectCache) {
this.projectCache = projectCache;
}
/**
- * @param notes change containing the given approvals.
+ * @param notes change notes containing the given approvals.
* @param approvals list of approvals.
* @return copies of approvals normalized to the defined ranges for the label type. Approvals for
* unknown labels are not included in the output.
- * @throws OrmException
*/
public Result normalize(ChangeNotes notes, Collection<PatchSetApproval> approvals)
- throws OrmException, IOException {
- IdentifiedUser user = userFactory.create(notes.getChange().getOwner());
- return normalize(notes, user, approvals);
- }
-
- /**
- * @param notes change notes containing the given approvals.
- * @param user current user.
- * @param approvals list of approvals.
- * @return copies of approvals normalized to the defined ranges for the label type. Approvals for
- * unknown labels are not included in the output.
- */
- public Result normalize(
- ChangeNotes notes, CurrentUser user, Collection<PatchSetApproval> approvals)
throws IOException {
List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
- LabelTypes labelTypes =
- projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes, user);
+ LabelTypes labelTypes = projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes);
for (PatchSetApproval psa : approvals) {
Change.Id changeId = psa.getKey().getParentKey().getParentKey();
checkArgument(
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index b979240..0398309 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -273,12 +273,7 @@
change.setCurrentPatchSet(patchSetInfo);
if (copyApprovals) {
approvalCopier.copyInReviewDb(
- db,
- ctx.getNotes(),
- ctx.getUser(),
- patchSet,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig());
+ db, ctx.getNotes(), patchSet, ctx.getRevWalk(), ctx.getRepoView().getConfig());
}
if (changeMessage != null) {
cmUtil.addChangeMessage(db, update, changeMessage);
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 637be24..0231378 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -361,7 +361,7 @@
PatchSetApproval submitAudit = null;
- for (PatchSetApproval a : safeGetApprovals(notes, user, psId)) {
+ for (PatchSetApproval a : safeGetApprovals(notes, psId)) {
if (a.getValue() <= 0) {
// Negative votes aren't counted.
continue;
@@ -460,10 +460,9 @@
return "Verified".equalsIgnoreCase(id.get());
}
- private Iterable<PatchSetApproval> safeGetApprovals(
- ChangeNotes notes, CurrentUser user, PatchSet.Id psId) {
+ private Iterable<PatchSetApproval> safeGetApprovals(ChangeNotes notes, PatchSet.Id psId) {
try {
- return approvalsUtil.byPatchSet(db.get(), notes, user, psId, null, null);
+ return approvalsUtil.byPatchSet(db.get(), notes, psId, null, null);
} catch (OrmException e) {
logger.atSevere().withCause(e).log("Can't read approval records for %s", psId);
return Collections.emptyList();
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 36c5005..3e7942f 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -309,7 +309,6 @@
approvalCopier.copyInReviewDb(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
newPatchSet,
ctx.getRevWalk(),
ctx.getRepoView().getConfig(),
@@ -408,7 +407,6 @@
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
priorPatchSetId,
ctx.getAccountId(),
ctx.getRevWalk(),
@@ -541,10 +539,7 @@
* show a transition from an oldValue of 0 to the new value.
*/
List<LabelType> labels =
- projectCache
- .checkedGet(ctx.getProject())
- .getLabelTypes(notes, ctx.getUser())
- .getLabelTypes();
+ projectCache.checkedGet(ctx.getProject()).getLabelTypes(notes).getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 932d1f8..57786a6 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -342,7 +342,7 @@
sb.append('\n');
sb.append("Hint: A potential ");
sb.append(FooterConstants.CHANGE_ID.getName());
- sb.append("Change-Id was found, but it was not in the ");
+ sb.append(" was found, but it was not in the ");
sb.append("footer (last paragraph) of the commit message.");
}
}
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 0e0bca6..bf76bde 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -323,7 +323,6 @@
.byPatchSetUser(
ctx.getDb(),
notes,
- ctx.getUser(),
psId,
ctx.getAccountId(),
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/mail/send/MergedSender.java b/java/com/google/gerrit/server/mail/send/MergedSender.java
index cf9257c..34f959c 100644
--- a/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -70,12 +70,7 @@
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca :
args.approvalsUtil.byPatchSet(
- args.db.get(),
- changeData.notes(),
- args.identifiedUserFactory.create(changeData.change().getOwner()),
- patchSet.getId(),
- null,
- null)) {
+ args.db.get(), changeData.notes(), patchSet.getId(), null, null)) {
LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) {
continue;
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 82001fb..bb752a0 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -327,8 +327,7 @@
case ABANDON:
return canAbandon();
case DELETE:
- return (getProjectControl().isAdmin()
- || (isOwner() && refControl.canDeleteOwnChanges(isOwner())));
+ return (getProjectControl().isAdmin() || (refControl.canDeleteChanges(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 0e3382c..f68a7a2 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -133,9 +133,10 @@
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);
+ /** @return true if this user can delete changes. */
+ boolean canDeleteChanges(boolean isChangeOwner) {
+ return canPerform(Permission.DELETE_CHANGES)
+ || (isChangeOwner && canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner, false));
}
/** The range of permitted values associated with a label permission. */
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index a490f10..87dbcfc 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -40,7 +40,6 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
@@ -394,13 +393,13 @@
return labelTypes;
}
- /** All available label types for this change and user. */
- public LabelTypes getLabelTypes(ChangeNotes notes, CurrentUser user) {
- return getLabelTypes(notes.getChange().getDest(), user);
+ /** All available label types for this change. */
+ public LabelTypes getLabelTypes(ChangeNotes notes) {
+ return getLabelTypes(notes.getChange().getDest());
}
- /** All available label types for this branch and user. */
- public LabelTypes getLabelTypes(Branch.NameKey destination, CurrentUser user) {
+ /** All available label types for this branch. */
+ public LabelTypes getLabelTypes(Branch.NameKey destination) {
List<LabelType> all = getLabelTypes().getLabelTypes();
List<LabelType> r = Lists.newArrayListWithCapacity(all.size());
@@ -410,7 +409,15 @@
r.add(l);
} else {
for (String refPattern : refs) {
- if (RefConfigSection.isValid(refPattern) && match(destination, refPattern, user)) {
+ if (refPattern.contains("${")) {
+ logger.atWarning().log(
+ "Ref pattern for label %s in project %s contains illegal expanded parameters: %s."
+ + " Ref pattern will be ignored.",
+ l, getName(), refPattern);
+ continue;
+ }
+
+ if (RefConfigSection.isValid(refPattern) && match(destination, refPattern)) {
r.add(l);
break;
}
@@ -558,7 +565,7 @@
return new LabelTypes(Collections.unmodifiableList(all));
}
- private boolean match(Branch.NameKey destination, String refPattern, CurrentUser user) {
- return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), user);
+ private boolean match(Branch.NameKey destination, String refPattern) {
+ return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), null);
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 1a336fc..515f98a 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -590,7 +590,7 @@
} catch (IOException e) {
throw new OrmException("project state not available", e);
}
- labelTypes = state.getLabelTypes(change().getDest(), userFactory.create(change().getOwner()));
+ labelTypes = state.getLabelTypes(change().getDest());
}
return labelTypes;
}
@@ -633,13 +633,7 @@
try {
currentApprovals =
ImmutableList.copyOf(
- approvalsUtil.byPatchSet(
- db,
- notes(),
- userFactory.create(c.getOwner()),
- c.currentPatchSetId(),
- null,
- null));
+ approvalsUtil.byPatchSet(db, notes(), c.currentPatchSetId(), null, null));
} catch (OrmException e) {
if (e.getCause() instanceof NoSuchChangeException) {
currentApprovals = Collections.emptyList();
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
index 91f5d15..2cc4ce4 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
@@ -131,8 +131,7 @@
currChange = ctx.getChange();
currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
- LabelTypes labelTypes =
- projectCache.checkedGet(ctx.getProject()).getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectCache.checkedGet(ctx.getProject()).getLabelTypes(ctx.getNotes());
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index f268a30..dc44e65 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -172,7 +172,7 @@
ps = psUtil.current(db.get(), ctx.getNotes());
boolean found = false;
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Account.Id accountId = accountState.getAccount().getId();
@@ -180,7 +180,6 @@
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
psId,
accountId,
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 42531c5..8cbca0f 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -44,7 +44,6 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeJson;
@@ -88,7 +87,6 @@
private final PatchSetUtil psUtil;
private final ApprovalsUtil approvalsUtil;
private final ProjectCache projectCache;
- private final Provider<CurrentUser> userProvider;
@Inject
Move(
@@ -101,8 +99,7 @@
RetryHelper retryHelper,
PatchSetUtil psUtil,
ApprovalsUtil approvalsUtil,
- ProjectCache projectCache,
- Provider<CurrentUser> userProvider) {
+ ProjectCache projectCache) {
super(retryHelper);
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
@@ -113,7 +110,6 @@
this.psUtil = psUtil;
this.approvalsUtil = approvalsUtil;
this.projectCache = projectCache;
- this.userProvider = userProvider;
}
@Override
@@ -261,15 +257,9 @@
List<PatchSetApproval> approvals = new ArrayList<>();
for (PatchSetApproval psa :
approvalsUtil.byPatchSet(
- ctx.getDb(),
- ctx.getNotes(),
- userProvider.get(),
- psId,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig())) {
+ ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
ProjectState projectState = projectCache.checkedGet(project);
- LabelType type =
- projectState.getLabelTypes(ctx.getNotes(), ctx.getUser()).byLabel(psa.getLabelId());
+ LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.getLabelId());
// Only keep veto votes, defined as votes where:
// 1- the label function allows minimum values to block submission.
// 2- the vote holds the minimum value.
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index e6f4f69..c6dbda3 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -237,7 +237,7 @@
throw new ResourceConflictException("cannot post review on edit");
}
ProjectState projectState = projectCache.checkedGet(revision.getProject());
- LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes(), revision.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes());
input.drafts = firstNonNull(input.drafts, DraftHandling.KEEP);
if (input.onBehalfOf != null) {
revision = onBehalfOf(revision, labelTypes, input);
@@ -1148,7 +1148,7 @@
List<PatchSetApproval> del = new ArrayList<>();
List<PatchSetApproval> ups = new ArrayList<>();
Map<String, PatchSetApproval> current = scanLabels(projectState, ctx, del);
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Map<String, Short> allApprovals =
getAllApprovals(labelTypes, approvalsByKey(current.values()), inLabels);
Map<String, Short> previous =
@@ -1297,11 +1297,7 @@
// If no existing label is being set to 0, hack in the caller
// as a reviewer by picking the first server-wide LabelType.
LabelId labelId =
- projectState
- .getLabelTypes(ctx.getNotes(), ctx.getUser())
- .getLabelTypes()
- .get(0)
- .getLabelId();
+ projectState.getLabelTypes(ctx.getNotes()).getLabelTypes().get(0).getLabelId();
PatchSetApproval c = ApprovalsUtil.newApproval(psId, user, labelId, 0, ctx.getWhen());
c.setTag(in.tag);
c.setGranted(ctx.getWhen());
@@ -1322,14 +1318,13 @@
private Map<String, PatchSetApproval> scanLabels(
ProjectState projectState, ChangeContext ctx, List<PatchSetApproval> del)
throws OrmException, IOException {
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Map<String, PatchSetApproval> current = new HashMap<>();
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
psId,
user.getAccountId(),
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java b/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
index 0502e91..3cb6136 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
@@ -169,7 +169,7 @@
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
projectCache
.checkedGet(rsrc.getProject())
- .getLabelTypes(rsrc.getChange().getDest(), ctx.getUser()),
+ .getLabelTypes(rsrc.getChange().getDest()),
rsrc.getChange(),
reviewers);
if (addedReviewers.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
index cfd20c2..49e5a2e 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
@@ -105,7 +105,7 @@
perm,
cd,
approvalsUtil.byPatchSetUser(
- db.get(), cd.notes(), perm.user(), psId, new Account.Id(out._accountId), null, null));
+ db.get(), cd.notes(), psId, new Account.Id(out._accountId), null, null));
}
public ReviewerInfo format(
diff --git a/java/com/google/gerrit/server/restapi/change/Votes.java b/java/com/google/gerrit/server/restapi/change/Votes.java
index b931c7e..3b2548c 100644
--- a/java/com/google/gerrit/server/restapi/change/Votes.java
+++ b/java/com/google/gerrit/server/restapi/change/Votes.java
@@ -87,7 +87,6 @@
approvalsUtil.byPatchSetUser(
db.get(),
rsrc.getChangeResource().getNotes(),
- rsrc.getChangeResource().getUser(),
rsrc.getChange().currentPatchSetId(),
rsrc.getReviewerUser().getAccountId(),
null,
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index 62dabae..abe3632 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -357,12 +357,7 @@
Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
for (PatchSetApproval psa :
args.approvalsUtil.byPatchSet(
- ctx.getDb(),
- ctx.getNotes(),
- ctx.getUser(),
- psId,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig())) {
+ ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
byKey.put(psa.getKey(), psa);
}
@@ -376,7 +371,7 @@
// was added. So we need to make sure votes are accurate now. This way if
// permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized =
- args.labelNormalizer.normalize(ctx.getNotes(), ctx.getUser(), byKey.values());
+ args.labelNormalizer.normalize(ctx.getNotes(), byKey.values());
update.putApproval(submitter.getLabel(), submitter.getValue());
saveApprovals(normalized, ctx, update, false);
return normalized;
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 7be1307..0ed189e 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -421,10 +421,11 @@
RevCommit newCommit = updateSubmodule(dc, ed, msgbuf, s);
count++;
if (newCommit != null) {
+ PersonIdent newCommitAuthor = newCommit.getAuthorIdent();
if (author == null) {
- author = newCommit.getAuthorIdent();
- } else if (!author.getName().equals(newCommit.getAuthorIdent().getName())
- || !author.getEmailAddress().equals(newCommit.getAuthorIdent().getEmailAddress())) {
+ author = new PersonIdent(newCommitAuthor, myIdent.getWhen());
+ } else if (!author.getName().equals(newCommitAuthor.getName())
+ || !author.getEmailAddress().equals(newCommitAuthor.getEmailAddress())) {
author = myIdent;
}
}
diff --git a/java/com/google/gerrit/server/util/PluginLogFile.java b/java/com/google/gerrit/server/util/PluginLogFile.java
index 351fbd4..de8b3aa 100644
--- a/java/com/google/gerrit/server/util/PluginLogFile.java
+++ b/java/com/google/gerrit/server/util/PluginLogFile.java
@@ -38,7 +38,7 @@
@Override
public void start() {
- AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout);
+ AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true);
Logger logger = LogManager.getLogger(logName);
logger.removeAppender(logName);
logger.addAppender(asyncAppender);
diff --git a/java/com/google/gerrit/server/util/SystemLog.java b/java/com/google/gerrit/server/util/SystemLog.java
index 224a6d9..ec2f386 100644
--- a/java/com/google/gerrit/server/util/SystemLog.java
+++ b/java/com/google/gerrit/server/util/SystemLog.java
@@ -77,13 +77,18 @@
}
private AsyncAppender createAsyncAppender(String name, Layout layout, boolean rotate) {
+ return createAsyncAppender(name, layout, rotate, false);
+ }
+
+ public AsyncAppender createAsyncAppender(
+ String name, Layout layout, boolean rotate, boolean forPlugin) {
AsyncAppender async = new AsyncAppender();
async.setName(name);
async.setBlocking(true);
async.setBufferSize(asyncLoggingBufferSize);
async.setLocationInfo(false);
- if (shouldConfigure()) {
+ if (forPlugin || shouldConfigure()) {
async.addAppender(createAppender(site.logs_dir, name, layout, rotate));
} else {
Appender appender = LogManager.getLogger(name).getAppender(name);
diff --git a/java/com/google/gerrit/sshd/commands/ApproveOption.java b/java/com/google/gerrit/sshd/commands/ApproveOption.java
index 633eaa0..38e0d07 100644
--- a/java/com/google/gerrit/sshd/commands/ApproveOption.java
+++ b/java/com/google/gerrit/sshd/commands/ApproveOption.java
@@ -114,6 +114,16 @@
return false;
}
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public String[] forbids() {
+ return null;
+ }
+
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public boolean help() {
+ return false;
+ }
+
String getLabelName() {
return type.getName();
}
diff --git a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index 852969f..9bcb103 100644
--- a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -18,9 +18,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.common.RawInputUtil;
-import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.errors.EmailException;
-import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.accounts.SshKeyInput;
import com.google.gerrit.extensions.common.EmailInfo;
@@ -31,11 +29,15 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
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.reviewdb.client.Account;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountSshKey;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.restapi.account.AddSshKey;
import com.google.gerrit.server.restapi.account.CreateEmail;
@@ -52,6 +54,7 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
@@ -66,7 +69,6 @@
/** Set a user's account settings. * */
@CommandMetaData(name = "set-account", description = "Change an account's settings")
-@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
final class SetAccountCommand extends SshCommand {
@Argument(
@@ -118,6 +120,9 @@
@Option(name = "--clear-http-password", usage = "clear HTTP password for the account")
private boolean clearHttpPassword;
+ @Option(name = "--generate-http-password", usage = "generate a new HTTP password for the account")
+ private boolean generateHttpPassword;
+
@Inject private IdentifiedUser.GenericFactory genericUserFactory;
@Inject private CreateEmail createEmail;
@@ -142,20 +147,54 @@
@Inject private DeleteSshKey deleteSshKey;
+ @Inject private PermissionBackend permissionBackend;
+
+ @Inject private Provider<CurrentUser> userProvider;
+
private AccountResource rsrc;
@Override
public void run() throws Exception {
+ user = genericUserFactory.create(id);
+
validate();
setAccount();
}
private void validate() throws UnloggedFailure {
- if (active && inactive) {
- throw die("--active and --inactive options are mutually exclusive.");
+ PermissionBackend.WithUser userPermission = permissionBackend.user(userProvider.get());
+
+ boolean isAdmin = userPermission.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
+ boolean canModifyAccount =
+ isAdmin || userPermission.testOrFalse(GlobalPermission.MODIFY_ACCOUNT);
+
+ if (!user.hasSameAccountId(userProvider.get()) && !canModifyAccount) {
+ throw die(
+ "Setting another user's account information requries 'modify account' or 'administrate server' capabilities.");
}
- if (clearHttpPassword && !Strings.isNullOrEmpty(httpPassword)) {
- throw die("--http-password and --clear-http-password options are mutually exclusive.");
+ if (active || inactive) {
+ if (!canModifyAccount) {
+ throw die(
+ "--active and --inactive require 'modify account' or 'administrate server' capabilities.");
+ }
+ if (active && inactive) {
+ throw die("--active and --inactive options are mutually exclusive.");
+ }
+ }
+
+ if (generateHttpPassword && clearHttpPassword) {
+ throw die("--generate-http-password and --clear-http-password are mutually exclusive.");
+ }
+ if (!Strings.isNullOrEmpty(httpPassword)) { // gave --http-password
+ if (!isAdmin) {
+ throw die("--http-password requires 'administrate server' capabilities.");
+ }
+ if (generateHttpPassword) {
+ throw die("--http-password and --generate-http-password options are mutually exclusive.");
+ }
+ if (clearHttpPassword) {
+ throw die("--http-password and --clear-http-password options are mutually exclusive.");
+ }
}
if (addSshKeys.contains("-") && deleteSshKeys.contains("-")) {
throw die("Only one option may use the stdin");
@@ -197,10 +236,16 @@
putName.apply(rsrc, in);
}
- if (httpPassword != null || clearHttpPassword) {
+ if (httpPassword != null || clearHttpPassword || generateHttpPassword) {
HttpPasswordInput in = new HttpPasswordInput();
in.httpPassword = httpPassword;
- putHttpPassword.apply(rsrc, in);
+ if (generateHttpPassword) {
+ in.generate = true;
+ }
+ Response<String> resp = putHttpPassword.apply(rsrc, in);
+ if (generateHttpPassword) {
+ stdout.print("New password: " + resp.value() + "\n");
+ }
}
if (active) {
diff --git a/java/com/google/gerrit/util/cli/CmdLineParser.java b/java/com/google/gerrit/util/cli/CmdLineParser.java
index f2d07ed..fb6ff1e 100644
--- a/java/com/google/gerrit/util/cli/CmdLineParser.java
+++ b/java/com/google/gerrit/util/cli/CmdLineParser.java
@@ -377,6 +377,16 @@
return o.depends();
}
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public String[] forbids() {
+ return null;
+ }
+
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public boolean help() {
+ return false;
+ }
+
@Override
public Class<? extends Annotation> annotationType() {
return o.annotationType();
@@ -563,6 +573,16 @@
public boolean isMultiValued() {
return false;
}
+
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public String[] forbids() {
+ return null;
+ }
+
+ // TODO(hanwen): add @Override after args4j upgrade.
+ public boolean help() {
+ return false;
+ }
}
public CmdLineException reject(String message) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d64a9d7..f9284bd 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -44,6 +44,7 @@
import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
+import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.Util.category;
import static com.google.gerrit.server.project.testing.Util.value;
@@ -91,6 +92,8 @@
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.api.projects.ProjectApi;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.Comment.Range;
@@ -999,12 +1002,7 @@
@Test
public void deleteNewChangeAsAdmin() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
-
- gApi.changes().id(changeId).delete();
-
- assertThat(query(changeId)).isEmpty();
+ deleteChangeAsUser(admin, admin);
}
@Test
@@ -1021,51 +1019,86 @@
}
@Test
- @TestProjectInput(cloneAs = "user")
- public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
- allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
- deleteChangeAsUser();
+ public void deleteNewChangeAsUserWithDeleteChangesPermissionForGroup() throws Exception {
+ allow("refs/*", Permission.DELETE_CHANGES, REGISTERED_USERS);
+ deleteChangeAsUser(admin, user);
}
@Test
- @TestProjectInput(cloneAs = "user")
- public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
- allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
- deleteChangeAsUser();
+ public void deleteNewChangeAsUserWithDeleteChangesPermissionForProjectOwners() throws Exception {
+ GroupApi groupApi = gApi.groups().create(name("delete-change"));
+ groupApi.addMembers("user");
+
+ ProjectInput in = new ProjectInput();
+ in.name = name("delete-change");
+ in.owners = Lists.newArrayListWithCapacity(1);
+ in.owners.add(groupApi.name());
+ in.createEmptyCommit = true;
+ ProjectApi api = gApi.projects().create(in);
+
+ Project.NameKey nameKey = new Project.NameKey(api.get().name);
+
+ try (ProjectConfigUpdate u = updateProject(nameKey)) {
+ Util.allow(u.getConfig(), Permission.DELETE_CHANGES, PROJECT_OWNERS, "refs/*");
+ u.save();
+ }
+
+ deleteChangeAsUser(nameKey, admin, user);
}
- private void deleteChangeAsUser() throws Exception {
- try {
- PushOneCommit.Result changeResult =
- pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
- String changeId = changeResult.getChangeId();
- int id = changeResult.getChange().getId().id;
- RevCommit commit = changeResult.getCommit();
+ @Test
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
+ allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
+ deleteChangeAsUser(user, user);
+ }
- setApiUser(user);
+ @Test
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
+ allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
+ deleteChangeAsUser(user, user);
+ }
+
+ private void deleteChangeAsUser(
+ com.google.gerrit.acceptance.TestAccount owner,
+ com.google.gerrit.acceptance.TestAccount deleteAs)
+ throws Exception {
+ deleteChangeAsUser(project, owner, deleteAs);
+ }
+
+ private void deleteChangeAsUser(
+ Project.NameKey projectName,
+ com.google.gerrit.acceptance.TestAccount owner,
+ com.google.gerrit.acceptance.TestAccount deleteAs)
+ throws Exception {
+ try {
+ setApiUser(owner);
+ ChangeInput in = new ChangeInput();
+ in.project = projectName.get();
+ in.branch = "refs/heads/master";
+ in.subject = "test";
+ ChangeInfo changeInfo = gApi.changes().create(in).get();
+ String changeId = changeInfo.changeId;
+ int id = changeInfo._number;
+ String commit = changeInfo.currentRevision;
+
+ assertThat(gApi.changes().id(changeId).info().owner._accountId).isEqualTo(owner.id.get());
+
+ setApiUser(deleteAs);
gApi.changes().id(changeId).delete();
assertThat(query(changeId)).isEmpty();
String ref = new Change.Id(id).toRefPrefix() + "1";
- eventRecorder.assertRefUpdatedEvents(project.get(), ref, null, commit, commit, null);
+ eventRecorder.assertRefUpdatedEvents(projectName.get(), ref, null, commit, commit, null);
} finally {
removePermission(project, "refs/*", Permission.DELETE_OWN_CHANGES);
+ removePermission(project, "refs/*", Permission.DELETE_CHANGES);
}
}
@Test
- @TestProjectInput(cloneAs = "user")
public void deleteNewChangeOfAnotherUserAsAdmin() throws Exception {
- PushOneCommit.Result changeResult =
- pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
- changeResult.assertOkStatus();
- String changeId = changeResult.getChangeId();
-
- setApiUser(admin);
- gApi.changes().id(changeId).delete();
-
- assertThat(query(changeId)).isEmpty();
+ deleteChangeAsUser(user, admin);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 4c8f53f..51946e9 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -366,6 +366,102 @@
}
@Test
+ public void singleHunkAtBeginningIsFollowedByCorrectCommonLines() throws Exception {
+ String filePath = "a_new_file.txt";
+ String fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n";
+ gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
+ gApi.changes().id(changeId).edit().publish();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, filePath, content -> content.replace("Line 1\n", "Line one\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, filePath).withBase(previousPatchSetId).get();
+ assertThat(diffInfo).content().element(0).linesOfA().isNotEmpty();
+ assertThat(diffInfo).content().element(0).linesOfB().isNotEmpty();
+ assertThat(diffInfo)
+ .content()
+ .element(1)
+ .commonLines()
+ .containsExactly("Line 2", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void singleHunkAtEndIsPrecededByCorrectCommonLines() throws Exception {
+ String filePath = "a_new_file.txt";
+ String fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n";
+ gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
+ gApi.changes().id(changeId).edit().publish();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, filePath, content -> content.replace("Line 5\n", "Line five\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, filePath).withBase(previousPatchSetId).get();
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsExactly("Line 1", "Line 2", "Line 3", "Line 4")
+ .inOrder();
+ assertThat(diffInfo).content().element(1).linesOfA().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfB().isNotEmpty();
+ }
+
+ @Test
+ public void singleHunkInTheMiddleIsSurroundedByCorrectCommonLines() throws Exception {
+ String filePath = "a_new_file.txt";
+ String fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6\n";
+ gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
+ gApi.changes().id(changeId).edit().publish();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, filePath, content -> content.replace("Line 3\n", "Line three\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, filePath).withBase(previousPatchSetId).get();
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsExactly("Line 1", "Line 2")
+ .inOrder();
+ assertThat(diffInfo).content().element(1).linesOfA().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfB().isNotEmpty();
+ assertThat(diffInfo)
+ .content()
+ .element(2)
+ .commonLines()
+ .containsExactly("Line 4", "Line 5", "Line 6")
+ .inOrder();
+ }
+
+ @Test
+ public void twoHunksAreSeparatedByCorrectCommonLines() throws Exception {
+ String filePath = "a_new_file.txt";
+ String fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n";
+ gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
+ gApi.changes().id(changeId).edit().publish();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(
+ changeId,
+ filePath,
+ content -> content.replace("Line 2\n", "Line two\n").replace("Line 5\n", "Line five\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, filePath).withBase(previousPatchSetId).get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfA().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfB().isNotEmpty();
+ assertThat(diffInfo)
+ .content()
+ .element(2)
+ .commonLines()
+ .containsExactly("Line 3", "Line 4")
+ .inOrder();
+ assertThat(diffInfo).content().element(3).linesOfA().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfB().isNotEmpty();
+ }
+
+ @Test
public void rebaseHunksAtStartOfFileAreIdentified() throws Exception {
String newFileContent =
FILE_CONTENT.replace("Line 1\n", "Line one\n").replace("Line 5\n", "Line five\n");
@@ -381,15 +477,15 @@
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo).content().element(0).isDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(3);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 5");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line five");
assertThat(diffInfo).content().element(2).isDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(44);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(4).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(4).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(4).isNotDueToRebase();
- assertThat(diffInfo).content().element(5).commonLines().hasSize(50);
+ assertThat(diffInfo).content().element(5).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -412,15 +508,15 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(49);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(9);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 60");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line sixty");
assertThat(diffInfo).content().element(3).isDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(39);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 100");
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line one hundred");
assertThat(diffInfo).content().element(5).isDueToRebase();
@@ -450,15 +546,15 @@
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo).content().element(0).isNotDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(38);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 40");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line forty");
assertThat(diffInfo).content().element(2).isDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(4).linesOfA().containsExactly("Line 45");
assertThat(diffInfo).content().element(4).linesOfB().containsExactly("Line forty five");
assertThat(diffInfo).content().element(4).isDueToRebase();
- assertThat(diffInfo).content().element(5).commonLines().hasSize(54);
+ assertThat(diffInfo).content().element(5).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(6).linesOfA().containsExactly("Line 100");
assertThat(diffInfo).content().element(6).linesOfB().containsExactly("Line one hundred");
assertThat(diffInfo).content().element(6).isNotDueToRebase();
@@ -489,11 +585,11 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(41);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 40");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line forty");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(59);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 100");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line one hundred");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
@@ -522,7 +618,7 @@
assertThat(diffInfo).content().element(0).linesOfA().isNull();
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line zero");
assertThat(diffInfo).content().element(0).isNotDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(9);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 10");
assertThat(diffInfo)
.content()
@@ -530,11 +626,11 @@
.linesOfB()
.containsExactly("Line ten", "Line ten and a half");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(29);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(4).linesOfA().containsExactly("Line 40");
assertThat(diffInfo).content().element(4).linesOfB().containsExactly("Line forty");
assertThat(diffInfo).content().element(4).isDueToRebase();
- assertThat(diffInfo).content().element(5).commonLines().hasSize(60);
+ assertThat(diffInfo).content().element(5).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -562,11 +658,11 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(37);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 40");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line forty");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(59);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 100");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line one hundred");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
@@ -595,15 +691,15 @@
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().isNull();
assertThat(diffInfo).content().element(0).isNotDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(8);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 10", "Line 11");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line ten");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(28);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(4).linesOfA().containsExactly("Line 40");
assertThat(diffInfo).content().element(4).linesOfB().containsExactly("Line forty");
assertThat(diffInfo).content().element(4).isDueToRebase();
- assertThat(diffInfo).content().element(5).commonLines().hasSize(60);
+ assertThat(diffInfo).content().element(5).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -623,7 +719,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(39);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 40");
assertThat(diffInfo)
.content()
@@ -631,7 +727,7 @@
.linesOfB()
.containsExactly("Line modified after rebase");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(60);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -655,7 +751,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(38);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 39", "Line 40");
assertThat(diffInfo)
.content()
@@ -663,7 +759,7 @@
.linesOfB()
.containsExactly("Line thirty nine", "Line forty");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(60);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -687,7 +783,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(40);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 41", "Line 42");
assertThat(diffInfo)
.content()
@@ -695,7 +791,7 @@
.linesOfB()
.containsExactly("Line forty one", "Line forty two");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(58);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -717,7 +813,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(38);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo)
.content()
.element(1)
@@ -729,7 +825,7 @@
.linesOfB()
.containsExactly("Line thirty nine", "A different line forty", "Line forty one");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(59);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -755,7 +851,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(40);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 41", "Line 42");
assertThat(diffInfo)
.content()
@@ -763,11 +859,11 @@
.linesOfB()
.containsExactly("Line forty one", "Line forty two", "Line forty two and a half");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(17);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 60");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line sixty");
assertThat(diffInfo).content().element(3).isDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(40);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -791,15 +887,15 @@
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo).content().element(0).isDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 3");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line three");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(4).linesOfA().containsExactly("Line 5");
assertThat(diffInfo).content().element(4).linesOfB().containsExactly("Line five");
assertThat(diffInfo).content().element(4).isDueToRebase();
- assertThat(diffInfo).content().element(5).commonLines().hasSize(95);
+ assertThat(diffInfo).content().element(5).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -834,15 +930,15 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 2");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line two");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(7);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 10");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line ten");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(90);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -879,19 +975,19 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 2");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line two");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line seven");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 7");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 9");
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line nine");
assertThat(diffInfo).content().element(5).isNotDueToRebase();
- assertThat(diffInfo).content().element(6).commonLines().hasSize(8);
+ assertThat(diffInfo).content().element(6).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(7).linesOfA().containsExactly("Line 18", "Line 19");
assertThat(diffInfo)
.content()
@@ -899,15 +995,15 @@
.linesOfB()
.containsExactly("Line eighteen", "Line nineteen");
assertThat(diffInfo).content().element(7).isDueToRebase();
- assertThat(diffInfo).content().element(8).commonLines().hasSize(29);
+ assertThat(diffInfo).content().element(8).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(9).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(9).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(9).isDueToRebase();
- assertThat(diffInfo).content().element(10).commonLines().hasSize(9);
+ assertThat(diffInfo).content().element(10).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(11).linesOfA().containsExactly("Line 60");
assertThat(diffInfo).content().element(11).linesOfB().containsExactly("Line sixty");
assertThat(diffInfo).content().element(11).isNotDueToRebase();
- assertThat(diffInfo).content().element(12).commonLines().hasSize(40);
+ assertThat(diffInfo).content().element(12).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -980,11 +1076,11 @@
assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1");
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one");
assertThat(diffInfo).content().element(0).isDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(48);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(50);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(initialPatchSetId);
@@ -1015,15 +1111,15 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, renamedFilePath).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(44);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(50);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -1059,11 +1155,11 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, newFilePath2).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(95);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
}
@Test
@@ -1096,11 +1192,11 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, newFilePath3).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(95);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
}
@Test
@@ -1125,19 +1221,19 @@
DiffInfo renamedFileDiffInfo =
getDiffRequest(changeId, CURRENT, renamedFileName).withBase(initialPatchSetId).get();
- assertThat(renamedFileDiffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(renamedFileDiffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(renamedFileDiffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(renamedFileDiffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(renamedFileDiffInfo).content().element(1).isDueToRebase();
- assertThat(renamedFileDiffInfo).content().element(2).commonLines().hasSize(95);
+ assertThat(renamedFileDiffInfo).content().element(2).commonLines().isNotEmpty();
DiffInfo copiedFileDiffInfo =
getDiffRequest(changeId, CURRENT, copyFileName).withBase(initialPatchSetId).get();
- assertThat(copiedFileDiffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(copiedFileDiffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(copiedFileDiffInfo).content().element(1).linesOfA().containsExactly("Line 5");
assertThat(copiedFileDiffInfo).content().element(1).linesOfB().containsExactly("Line five");
assertThat(copiedFileDiffInfo).content().element(1).isDueToRebase();
- assertThat(copiedFileDiffInfo).content().element(2).commonLines().hasSize(95);
+ assertThat(copiedFileDiffInfo).content().element(2).commonLines().isNotEmpty();
}
/*
@@ -1168,23 +1264,23 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line five");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line 5");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(14);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 20");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line twenty");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(14);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 35");
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line thirty five");
assertThat(diffInfo).content().element(5).isDueToRebase();
- assertThat(diffInfo).content().element(6).commonLines().hasSize(24);
+ assertThat(diffInfo).content().element(6).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(7).linesOfA().containsExactly("Line 60");
assertThat(diffInfo).content().element(7).linesOfB().containsExactly("Line sixty");
assertThat(diffInfo).content().element(7).isDueToRebase();
- assertThat(diffInfo).content().element(8).commonLines().hasSize(40);
+ assertThat(diffInfo).content().element(8).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -1239,19 +1335,19 @@
String currentPatchSetId = gApi.changes().id(changeId).get().currentRevision;
DiffInfo diffInfo =
getDiffRequest(changeId, initialPatchSetId, FILE_NAME).withBase(currentPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(4);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line five");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line 5");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(14);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line twenty");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 20");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(14);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().isNull();
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line 35");
assertThat(diffInfo).content().element(5).isDueToRebase();
- assertThat(diffInfo).content().element(6).commonLines().hasSize(65);
+ assertThat(diffInfo).content().element(6).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).revision(initialPatchSetId).files(currentPatchSetId);
@@ -1282,7 +1378,7 @@
.intralineEditsOfB()
.containsExactly(ImmutableList.of(5, 3));
assertThat(diffInfo).content().element(0).isNotDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(99);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
}
@Test
@@ -1312,11 +1408,11 @@
.intralineEditsOfB()
.containsExactly(ImmutableList.of(5, 3));
assertThat(diffInfo).content().element(0).isDueToRebase();
- assertThat(diffInfo).content().element(1).commonLines().hasSize(48);
+ assertThat(diffInfo).content().element(1).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 50");
assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line fifty");
assertThat(diffInfo).content().element(2).isNotDueToRebase();
- assertThat(diffInfo).content().element(3).commonLines().hasSize(50);
+ assertThat(diffInfo).content().element(3).commonLines().isNotEmpty();
}
@Test
@@ -1335,7 +1431,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(3);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4", "{", "Line 6");
assertThat(diffInfo)
.content()
@@ -1343,7 +1439,7 @@
.linesOfB()
.containsExactly("Line four", "{", "Line six");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(94);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -1372,19 +1468,19 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(3);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line four");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 6");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line six");
assertThat(diffInfo).content().element(3).isDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(13);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 20");
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line twenty");
assertThat(diffInfo).content().element(5).isNotDueToRebase();
- assertThat(diffInfo).content().element(6).commonLines().hasSize(80);
+ assertThat(diffInfo).content().element(6).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -1411,19 +1507,19 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(3);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line four");
assertThat(diffInfo).content().element(1).isDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 6");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line six");
assertThat(diffInfo).content().element(3).isNotDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 8");
assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line eight");
assertThat(diffInfo).content().element(5).isDueToRebase();
- assertThat(diffInfo).content().element(6).commonLines().hasSize(92);
+ assertThat(diffInfo).content().element(6).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
@@ -1452,7 +1548,7 @@
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get();
- assertThat(diffInfo).content().element(0).commonLines().hasSize(3);
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4", "{", "Line 6");
assertThat(diffInfo)
.content()
@@ -1460,11 +1556,11 @@
.linesOfB()
.containsExactly("Line four", "{", "Line six");
assertThat(diffInfo).content().element(1).isNotDueToRebase();
- assertThat(diffInfo).content().element(2).commonLines().hasSize(1);
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 8");
assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line eight!");
assertThat(diffInfo).content().element(3).isDueToRebase();
- assertThat(diffInfo).content().element(4).commonLines().hasSize(92);
+ assertThat(diffInfo).content().element(4).commonLines().isNotEmpty();
Map<String, FileInfo> changedFiles =
gApi.changes().id(changeId).current().files(previousPatchSetId);
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index ce73875..252ec88 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -438,9 +438,7 @@
assertThat(c.getFullMessage()).isEqualTo(expectedMessage);
}
- protected void expectToHaveAuthor(
- TestRepository<?> repo, String branch, String expectedAuthorName, String expectedAuthorEmail)
- throws Exception {
+ protected PersonIdent getAuthor(TestRepository<?> repo, String branch) throws Exception {
ObjectId commitId =
repo.git()
.fetch()
@@ -451,8 +449,6 @@
RevWalk rw = repo.getRevWalk();
RevCommit c = rw.parseCommit(commitId);
- PersonIdent authorIdent = c.getAuthorIdent();
- assertThat(authorIdent.getName()).isEqualTo(expectedAuthorName);
- assertThat(authorIdent.getEmailAddress()).isEqualTo(expectedAuthorEmail);
+ return c.getAuthorIdent();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 4381ed137..847004f 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
+import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.GerritConfig;
@@ -23,9 +24,11 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.ConfigSuite;
+import com.google.gerrit.testing.TestTimeUtil;
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.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushResult;
@@ -522,84 +525,134 @@
}
@Test
+ public void superRepoCommitHasSameAuthorAsSubmoduleCommit() throws Exception {
+ // Make sure that the commit is created at an earlier timestamp than the submit timestamp.
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ try {
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+
+ PushOneCommit.Result pushResult =
+ createChange(subRepo, "refs/heads/master", "Change", "a.txt", "some content", null);
+ approve(pushResult.getChangeId());
+ gApi.changes().id(pushResult.getChangeId()).current().submit();
+
+ // Expect that the author name/email is preserved for the superRepo commit, but a new author
+ // timestamp is used.
+ PersonIdent authorIdent = getAuthor(superRepo, "master");
+ assertThat(authorIdent.getName()).isEqualTo(admin.fullName);
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email);
+ assertThat(authorIdent.getWhen())
+ .isGreaterThan(pushResult.getCommit().getAuthorIdent().getWhen());
+ } finally {
+ TestTimeUtil.useSystemTime();
+ }
+ }
+
+ @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");
+ // Make sure that the commits are created at different timestamps and that the submit timestamp
+ // is afterwards.
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ try {
+ 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");
+ 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);
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
- String topic = "foo";
+ 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);
+ PushOneCommit.Result pushResult1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
+ approve(pushResult1.getChangeId());
- 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);
+ PushOneCommit.Result pushResult2 =
+ createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic);
+ approve(pushResult2.getChangeId());
- // Submit the topic, 2 changes with the same author.
- gApi.changes().id(changeId1).current().submit();
+ // Submit the topic, 2 changes with the same author.
+ gApi.changes().id(pushResult1.getChangeId()).current().submit();
- // Expect that the author is preserved for the superRepo commit.
- expectToHaveAuthor(superRepo, "master", admin.fullName, admin.email);
+ // Expect that the author name/email is preserved for the superRepo commit, but a new author
+ // timestamp is used.
+ PersonIdent authorIdent = getAuthor(superRepo, "master");
+ assertThat(authorIdent.getName()).isEqualTo(admin.fullName);
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email);
+ assertThat(authorIdent.getWhen())
+ .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
+ assertThat(authorIdent.getWhen())
+ .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
+ } finally {
+ TestTimeUtil.useSystemTime();
+ }
}
@Test
public void superRepoCommitHasGerritAsAuthorIfAuthorsOfSubmoduleCommitsDiffer() throws Exception {
assume().that(isSubmitWholeTopicEnabled()).isTrue();
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ // Make sure that the commits are created at different timestamps and that the submit timestamp
+ // is afterwards.
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ try {
+ 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);
+ 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");
+ 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);
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
- String topic = "foo";
+ 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 admin.
+ PushOneCommit.Result pushResult1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
+ approve(pushResult1.getChangeId());
- // 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);
+ // Create change as user.
+ PushOneCommit push =
+ pushFactory.create(db, user.getIdent(), subRepo2, "Change 2", "b.txt", "other content");
+ PushOneCommit.Result pushResult2 = push.to("refs/for/master/" + name(topic));
+ approve(pushResult2.getChangeId());
- // Submit the topic, 2 changes with the different author.
- gApi.changes().id(changeId1).current().submit();
+ // Submit the topic, 2 changes with the different author.
+ gApi.changes().id(pushResult1.getChangeId()).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());
+ // Expect that the Gerrit server identity is chosen as author for the superRepo commit and a
+ // new author timestamp is used.
+ PersonIdent authorIdent = getAuthor(superRepo, "master");
+ assertThat(authorIdent.getName()).isEqualTo(serverIdent.get().getName());
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(serverIdent.get().getEmailAddress());
+ assertThat(authorIdent.getWhen())
+ .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
+ assertThat(authorIdent.getWhen())
+ .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
+ } finally {
+ TestTimeUtil.useSystemTime();
+ }
}
private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/pgm/BUILD b/javatests/com/google/gerrit/acceptance/pgm/BUILD
index a8d5644..583ecc8 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/BUILD
+++ b/javatests/com/google/gerrit/acceptance/pgm/BUILD
@@ -22,6 +22,7 @@
group = "elastic",
labels = [
"elastic",
+ "flaky",
"pgm",
"no_windows",
],
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
index d388aeb..13bb2f2 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -32,7 +32,8 @@
elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
String indicesPrefix = UUID.randomUUID().toString();
Config cfg = new Config();
- ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix);
+ String password = version == ElasticVersion.V5_6 ? "changeme" : null;
+ ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix, password);
return cfg;
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
new file mode 100644
index 0000000..559b8c7
--- /dev/null
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
@@ -0,0 +1,153 @@
+// 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.elasticsearch;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.DEFAULT_MAX_RETRY_TIMEOUT_MS;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.DEFAULT_USERNAME;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_MAX_RETRY_TIMEOUT;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_PASSWORD;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_PREFIX;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_SERVER;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_USERNAME;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.MAX_RETRY_TIMEOUT_UNIT;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.SECTION_ELASTICSEARCH;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.ProvisionException;
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class ElasticConfigurationTest {
+ @Rule public ExpectedException exception = ExpectedException.none();
+
+ @Test
+ public void singleServerNoOtherConfig() throws Exception {
+ Config cfg = newConfig();
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertHosts(esCfg, "http://elastic:1234");
+ assertThat(esCfg.username).isNull();
+ assertThat(esCfg.password).isNull();
+ assertThat(esCfg.prefix).isEmpty();
+ assertThat(esCfg.maxRetryTimeout).isEqualTo(DEFAULT_MAX_RETRY_TIMEOUT_MS);
+ }
+
+ @Test
+ public void serverWithoutPortSpecified() throws Exception {
+ Config cfg = new Config();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertHosts(esCfg, "http://elastic:9200");
+ }
+
+ @Test
+ public void prefix() throws Exception {
+ Config cfg = newConfig();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PREFIX, "myprefix");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertThat(esCfg.prefix).isEqualTo("myprefix");
+ }
+
+ @Test
+ public void maxRetryTimeoutInDefaultUnit() {
+ Config cfg = newConfig();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_MAX_RETRY_TIMEOUT, "45000");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertThat(esCfg.maxRetryTimeout).isEqualTo(45000);
+ }
+
+ @Test
+ public void maxRetryTimeoutInOtherUnit() {
+ Config cfg = newConfig();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_MAX_RETRY_TIMEOUT, "45 s");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertThat(esCfg.maxRetryTimeout)
+ .isEqualTo(MAX_RETRY_TIMEOUT_UNIT.convert(45, TimeUnit.SECONDS));
+ }
+
+ @Test
+ public void withAuthentication() throws Exception {
+ Config cfg = newConfig();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_USERNAME, "myself");
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertThat(esCfg.username).isEqualTo("myself");
+ assertThat(esCfg.password).isEqualTo("s3kr3t");
+ }
+
+ @Test
+ public void withAuthenticationPasswordOnlyUsesDefaultUsername() throws Exception {
+ Config cfg = newConfig();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertThat(esCfg.username).isEqualTo(DEFAULT_USERNAME);
+ assertThat(esCfg.password).isEqualTo("s3kr3t");
+ }
+
+ @Test
+ public void multipleServers() throws Exception {
+ Config cfg = new Config();
+ cfg.setStringList(
+ SECTION_ELASTICSEARCH,
+ null,
+ KEY_SERVER,
+ ImmutableList.of("http://elastic1:1234", "http://elastic2:1234"));
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234");
+ }
+
+ @Test
+ public void noServers() throws Exception {
+ assertProvisionException(new Config());
+ }
+
+ @Test
+ public void singleServerInvalid() throws Exception {
+ Config cfg = new Config();
+ cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "foo");
+ assertProvisionException(cfg);
+ }
+
+ @Test
+ public void multipleServersIncludingInvalid() throws Exception {
+ Config cfg = new Config();
+ cfg.setStringList(
+ SECTION_ELASTICSEARCH, null, KEY_SERVER, ImmutableList.of("http://elastic1:1234", "foo"));
+ ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ assertHosts(esCfg, "http://elastic1:1234");
+ }
+
+ private static Config newConfig() {
+ Config config = new Config();
+ config.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic:1234");
+ return config;
+ }
+
+ private void assertHosts(ElasticConfiguration cfg, Object... hostURIs) throws Exception {
+ assertThat(Arrays.asList(cfg.getHosts()).stream().map(h -> h.toURI()).collect(toList()))
+ .containsExactly(hostURIs);
+ }
+
+ private void assertProvisionException(Config cfg) throws Exception {
+ exception.expect(ProvisionException.class);
+ exception.expectMessage("No valid Elasticsearch servers configured");
+ new ElasticConfiguration(cfg);
+ }
+}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 277fadc..bdad355 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -45,11 +45,11 @@
case V2_4:
return "elasticsearch:2.4.6-alpine";
case V5_6:
- return "elasticsearch:5.6.10-alpine";
+ return "docker.elastic.co/elasticsearch/elasticsearch:5.6.10";
case V6_2:
return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.2.4";
case V6_3:
- return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.0";
+ return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.1";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
index ca52e2a..02e0ba2 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -32,13 +32,18 @@
}
}
- public static void configure(Config config, int port, String prefix) {
+ public static void configure(Config config, int port, String prefix, String password) {
config.setEnum("index", null, "type", IndexType.ELASTICSEARCH);
- config.setString("elasticsearch", "test", "protocol", "http");
- config.setString("elasticsearch", "test", "hostname", "localhost");
- config.setInt("elasticsearch", "test", "port", port);
+ config.setString("elasticsearch", null, "server", "http://localhost:" + port);
config.setString("elasticsearch", null, "prefix", prefix);
- config.setString("index", null, "maxLimit", "10000");
+ config.setInt("index", null, "maxLimit", 10000);
+ if (password != null) {
+ config.setString("elasticsearch", null, "password", password);
+ }
+ }
+
+ public static void configure(Config config, int port, String prefix) {
+ configure(config, port, prefix, null);
}
public static void createAllIndexes(Injector injector) throws IOException {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
index 649fc6f..0fcdb20 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
@@ -67,7 +67,7 @@
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName();
- ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+ ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
}
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
index 4aa08fa..4520020 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
@@ -67,7 +67,7 @@
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName();
- ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+ ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
}
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
index 72f8b49..d953139 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
@@ -67,7 +67,7 @@
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName();
- ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+ ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
}
}
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index c73171e..26f2a51 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit c73171ea9abbeb765d585a92753ce01151355a5c
+Subproject commit 26f2a517fa1f8aec90de85d7cce403fe6a8faaf3
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index e601b93..3e886d5 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -20,7 +20,8 @@
// Latency reporting constants.
const TIMING = {
TYPE: 'timing-report',
- CATEGORY: 'UI Latency',
+ CATEGORY_UI_LATENCY: 'UI Latency',
+ CATEGORY_RPC: 'RPC Timing',
// Reported events - alphabetize below.
APP_STARTED: 'App Started',
PAGE_LOADED: 'Page Loaded',
@@ -170,7 +171,16 @@
report.apply(this, args);
},
- defaultReporter(type, category, eventName, eventValue) {
+ /**
+ * The default reporter reports events immediately.
+ * @param {string} type
+ * @param {string} category
+ * @param {string} eventName
+ * @param {string|number} eventValue
+ * @param {boolean|undefined} opt_noLog If true, the event will not be
+ * logged to the JS console.
+ */
+ defaultReporter(type, category, eventName, eventValue, opt_noLog) {
const detail = {
type,
category,
@@ -178,6 +188,7 @@
value: eventValue,
};
document.dispatchEvent(new CustomEvent(type, {detail}));
+ if (opt_noLog) { return; }
if (type === ERROR.TYPE) {
console.error(eventValue.error || eventName);
} else {
@@ -186,7 +197,17 @@
}
},
- cachingReporter(type, category, eventName, eventValue) {
+ /**
+ * The caching reporter will queue reports until plugins have loaded, and
+ * log events immediately if they're reported after plugins have loaded.
+ * @param {string} type
+ * @param {string} category
+ * @param {string} eventName
+ * @param {string|number} eventValue
+ * @param {boolean|undefined} opt_noLog If true, the event will not be
+ * logged to the JS console.
+ */
+ cachingReporter(type, category, eventName, eventValue, opt_noLog) {
if (type === ERROR.TYPE) {
console.error(eventValue.error || eventName);
}
@@ -196,9 +217,9 @@
this.reporter(...args);
}
}
- this.reporter(type, category, eventName, eventValue);
+ this.reporter(type, category, eventName, eventValue, opt_noLog);
} else {
- pending.push([type, category, eventName, eventValue]);
+ pending.push([type, category, eventName, eventValue, opt_noLog]);
}
},
@@ -208,8 +229,8 @@
appStarted(hidden) {
const startTime =
new Date().getTime() - this.performanceTiming.navigationStart;
- this.reporter(
- TIMING.TYPE, TIMING.CATEGORY, TIMING.APP_STARTED, startTime);
+ this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
+ TIMING.APP_STARTED, startTime);
if (hidden) {
this.reporter(PAGE_VISIBILITY.TYPE, PAGE_VISIBILITY.CATEGORY,
PAGE_VISIBILITY.STARTED_HIDDEN);
@@ -226,8 +247,8 @@
} else {
const loadTime = this.performanceTiming.loadEventEnd -
this.performanceTiming.navigationStart;
- this.reporter(
- TIMING.TYPE, TIMING.CATEGORY, TIMING.PAGE_LOADED, loadTime);
+ this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
+ TIMING.PAGE_LOADED, loadTime);
}
},
@@ -344,7 +365,8 @@
* @param {number} time The time to report as an integer of milliseconds.
*/
_reportTiming(name, time) {
- this.reporter(TIMING.TYPE, TIMING.CATEGORY, name, Math.round(time));
+ this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY, name,
+ Math.round(time));
},
/**
@@ -395,6 +417,16 @@
return timer.reset();
},
+ /**
+ * Log timing information for an RPC.
+ * @param {string} anonymizedUrl The URL of the RPC with tokens obfuscated.
+ * @param {number} elapsed The time elapsed of the RPC.
+ */
+ reportRpcTiming(anonymizedUrl, elapsed) {
+ this.reporter(TIMING.TYPE, TIMING.CATEGORY_RPC, 'RPC-' + anonymizedUrl,
+ elapsed, true);
+ },
+
reportInteraction(eventName, opt_msg) {
this.reporter(INTERACTION_TYPE, this.category, eventName, opt_msg);
},
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index e78f49fb..8b85074 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -264,7 +264,7 @@
sandbox.stub(element, 'now').returns(42);
element.pluginsLoaded();
assert.isTrue(element.defaultReporter.calledWithExactly(
- 'timing-report', 'UI Latency', 'PluginsLoaded', 42
+ 'timing-report', 'UI Latency', 'PluginsLoaded', 42, undefined
));
});
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 7acb680..b4aec99 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -99,6 +99,7 @@
'page-error': '_handlePageError',
'title-change': '_handleTitleChange',
'location-change': '_handleLocationChange',
+ 'rpc-log': '_handleRpcLog',
},
observers: [
@@ -332,5 +333,15 @@
console.log(`Please file bugs and feedback at: ${this._feedbackUrl}`);
console.groupEnd();
},
+
+ /**
+ * Intercept RPC log events emitted by REST API interfaces.
+ * Note: the REST API interface cannot use gr-reporting directly because
+ * that would create a cyclic dependency.
+ */
+ _handleRpcLog(e) {
+ this.$.reporting.reportRpcTiming(e.detail.anonymizedUrl,
+ e.detail.elapsed);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 5d68e01..fa1ad11 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -28,6 +28,15 @@
Defs.patchRange;
/**
+ * @typedef {{
+ * url: string,
+ * fetchOptions: (Object|null|undefined),
+ * anonymizedUrl: (string|undefined),
+ * }}
+ */
+ Defs.FetchRequest;
+
+ /**
* Object to describe a request for passing into _fetchJSON or _fetchRawJSON.
* - url is the URL for the request (excluding get params)
* - errFn is a function to invoke when the request fails.
@@ -40,6 +49,8 @@
* cancelCondition: (function()|null|undefined),
* params: (Object|null|undefined),
* fetchOptions: (Object|null|undefined),
+ * anonymizedUrl: (string|undefined),
+ * reportUrlAsIs: (boolean|undefined),
* }}
*/
Defs.FetchJSONRequest;
@@ -53,6 +64,8 @@
* cancelCondition: (function()|null|undefined),
* params: (Object|null|undefined),
* fetchOptions: (Object|null|undefined),
+ * anonymizedEndpoint: (string|undefined),
+ * reportEndpointAsIs: (boolean|undefined),
* }}
*/
Defs.ChangeFetchRequest;
@@ -78,6 +91,8 @@
* contentType: (string|null|undefined),
* headers: (Object|undefined),
* parseResponse: (boolean|undefined),
+ * anonymizedUrl: (string|undefined),
+ * reportUrlAsIs: (boolean|undefined),
* }}
*/
Defs.SendRequest;
@@ -93,6 +108,8 @@
* contentType: (string|null|undefined),
* headers: (Object|undefined),
* parseResponse: (boolean|undefined),
+ * anonymizedEndpoint: (string|undefined),
+ * reportEndpointAsIs: (boolean|undefined),
* }}
*/
Defs.ChangeSendRequest;
@@ -115,6 +132,9 @@
'Saving draft resulted in HTTP 200 (OK) but expected HTTP 201 (Created)';
const HEADER_REPORTING_BLACKLIST = /^set-cookie$/i;
+ const ANONYMIZED_CHANGE_BASE_URL = '/changes/*~*';
+ const ANONYMIZED_REVISION_BASE_URL = ANONYMIZED_CHANGE_BASE_URL +
+ '/revisions/*';
Polymer({
is: 'gr-rest-api-interface',
@@ -143,6 +163,12 @@
* @event auth-error
*/
+ /**
+ * Fired after an RPC completes.
+ *
+ * @event rpc-log
+ */
+
properties: {
_cache: {
type: Object,
@@ -182,15 +208,14 @@
/**
* Wraps calls to the underlying authenticated fetch function (_auth.fetch)
* with timing and logging.
- * @param {string} url
- * @param {Object=} opt_fetchOptions
+ * @param {Defs.FetchRequest} req
*/
- _fetch(url, opt_fetchOptions) {
+ _fetch(req) {
const start = Date.now();
- const xhr = this._auth.fetch(url, opt_fetchOptions);
+ const xhr = this._auth.fetch(req.url, req.fetchOptions);
// Log the call after it completes.
- xhr.then(res => this._logCall(url, opt_fetchOptions, start, res.status));
+ xhr.then(res => this._logCall(req, start, res.status));
// Return the XHR directly (without the log).
return xhr;
@@ -200,18 +225,27 @@
* Log information about a REST call. Because the elapsed time is determined
* by this method, it should be called immediately after the request
* finishes.
- * @param {string} url
- * @param {Object|undefined} fetchOptions
+ * @param {Defs.FetchRequest} req
* @param {number} startTime the time that the request was started.
* @param {number} status the HTTP status of the response. The status value
* is used here rather than the response object so there is no way this
* method can read the body stream.
*/
- _logCall(url, fetchOptions, startTime, status) {
- const method = (fetchOptions && fetchOptions.method) ?
- fetchOptions.method : 'GET';
- const elapsed = (Date.now() - startTime) + 'ms';
- console.log(['HTTP', status, method, elapsed, url].join(' '));
+ _logCall(req, startTime, status) {
+ const method = (req.fetchOptions && req.fetchOptions.method) ?
+ req.fetchOptions.method : 'GET';
+ const elapsed = (Date.now() - startTime);
+ console.log([
+ 'HTTP',
+ status,
+ method,
+ elapsed + 'ms',
+ req.anonymizedUrl || req.url,
+ ].join(' '));
+ if (req.anonymizedUrl) {
+ this.fire('rpc-log',
+ {status, method, elapsed, anonymizedUrl: req.anonymizedUrl});
+ }
},
/**
@@ -223,7 +257,12 @@
*/
_fetchRawJSON(req) {
const urlWithParams = this._urlWithParams(req.url, req.params);
- return this._fetch(urlWithParams, req.fetchOptions).then(res => {
+ const fetchReq = {
+ url: urlWithParams,
+ fetchOptions: req.fetchOptions,
+ anonymizedUrl: req.reportUrlAsIs ? urlWithParams : req.anonymizedUrl,
+ };
+ return this._fetch(fetchReq).then(res => {
if (req.cancelCondition && req.cancelCondition()) {
res.body.cancel();
return;
@@ -324,10 +363,16 @@
getConfig(noCache) {
if (!noCache) {
- return this._fetchSharedCacheURL({url: '/config/server/info'});
+ return this._fetchSharedCacheURL({
+ url: '/config/server/info',
+ reportUrlAsIs: true,
+ });
}
- return this._fetchJSON({url: '/config/server/info'});
+ return this._fetchJSON({
+ url: '/config/server/info',
+ reportUrlAsIs: true,
+ });
},
getRepo(repo, opt_errFn) {
@@ -336,6 +381,7 @@
return this._fetchSharedCacheURL({
url: '/projects/' + encodeURIComponent(repo),
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*',
});
},
@@ -345,6 +391,7 @@
return this._fetchSharedCacheURL({
url: '/projects/' + encodeURIComponent(repo) + '/config',
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/config',
});
},
@@ -353,6 +400,7 @@
// supports it.
return this._fetchSharedCacheURL({
url: '/access/?project=' + encodeURIComponent(repo),
+ anonymizedUrl: '/access/?project=*',
});
},
@@ -362,6 +410,7 @@
return this._fetchSharedCacheURL({
url: `/projects/${encodeURIComponent(repo)}/dashboards?inherited`,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/dashboards?inherited',
});
},
@@ -374,6 +423,7 @@
url: `/projects/${encodeName}/config`,
body: config,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/config',
});
},
@@ -387,6 +437,7 @@
url: `/projects/${encodeName}/gc`,
body: '',
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/gc',
});
},
@@ -404,6 +455,7 @@
url: `/projects/${encodeName}`,
body: config,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*',
});
},
@@ -419,6 +471,7 @@
url: `/groups/${encodeName}`,
body: config,
errFn: opt_errFn,
+ anonymizedUrl: '/groups/*',
});
},
@@ -426,6 +479,7 @@
return this._fetchJSON({
url: `/groups/${encodeURIComponent(group)}/detail`,
errFn: opt_errFn,
+ anonymizedUrl: '/groups/*/detail',
});
},
@@ -445,6 +499,7 @@
url: `/projects/${encodeName}/branches/${encodeRef}`,
body: '',
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/branches/*',
});
},
@@ -464,6 +519,7 @@
url: `/projects/${encodeName}/tags/${encodeRef}`,
body: '',
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/tags/*',
});
},
@@ -484,6 +540,7 @@
url: `/projects/${encodeName}/branches/${encodeBranch}`,
body: revision,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/branches/*',
});
},
@@ -504,6 +561,7 @@
url: `/projects/${encodeName}/tags/${encodeTag}`,
body: revision,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/tags/*',
});
},
@@ -513,7 +571,11 @@
*/
getIsGroupOwner(groupName) {
const encodeName = encodeURIComponent(groupName);
- return this._fetchSharedCacheURL({url: `/groups/?owned&q=${encodeName}`})
+ const req = {
+ url: `/groups/?owned&q=${encodeName}`,
+ anonymizedUrl: '/groups/owned&q=*',
+ };
+ return this._fetchSharedCacheURL(req)
.then(configs => configs.hasOwnProperty(groupName));
},
@@ -522,12 +584,15 @@
return this._fetchJSON({
url: `/groups/${encodeName}/members/`,
errFn: opt_errFn,
+ anonymizedUrl: '/groups/*/members',
});
},
getIncludedGroup(groupName) {
- const encodeName = encodeURIComponent(groupName);
- return this._fetchJSON({url: `/groups/${encodeName}/groups/`});
+ return this._fetchJSON({
+ url: `/groups/${encodeURIComponent(groupName)}/groups/`,
+ anonymizedUrl: '/groups/*/groups',
+ });
},
saveGroupName(groupId, name) {
@@ -536,6 +601,7 @@
method: 'PUT',
url: `/groups/${encodeId}/name`,
body: {name},
+ anonymizedUrl: '/groups/*/name',
});
},
@@ -545,6 +611,7 @@
method: 'PUT',
url: `/groups/${encodeId}/owner`,
body: {owner: ownerId},
+ anonymizedUrl: '/groups/*/owner',
});
},
@@ -554,6 +621,7 @@
method: 'PUT',
url: `/groups/${encodeId}/description`,
body: {description},
+ anonymizedUrl: '/groups/*/description',
});
},
@@ -563,6 +631,7 @@
method: 'PUT',
url: `/groups/${encodeId}/options`,
body: options,
+ anonymizedUrl: '/groups/*/options',
});
},
@@ -570,6 +639,7 @@
return this._fetchSharedCacheURL({
url: '/groups/' + group + '/log.audit',
errFn: opt_errFn,
+ anonymizedUrl: '/groups/*/log.audit',
});
},
@@ -580,6 +650,7 @@
method: 'PUT',
url: `/groups/${encodeName}/members/${encodeMember}`,
parseResponse: true,
+ anonymizedUrl: '/groups/*/members/*',
});
},
@@ -590,6 +661,7 @@
method: 'PUT',
url: `/groups/${encodeName}/groups/${encodeIncludedGroup}`,
errFn: opt_errFn,
+ anonymizedUrl: '/groups/*/groups/*',
};
return this._send(req).then(response => {
if (response.ok) {
@@ -604,6 +676,7 @@
return this._send({
method: 'DELETE',
url: `/groups/${encodeName}/members/${encodeMember}`,
+ anonymizedUrl: '/groups/*/members/*',
});
},
@@ -613,11 +686,15 @@
return this._send({
method: 'DELETE',
url: `/groups/${encodeName}/groups/${encodeIncludedGroup}`,
+ anonymizedUrl: '/groups/*/groups/*',
});
},
getVersion() {
- return this._fetchSharedCacheURL({url: '/config/server/version'});
+ return this._fetchSharedCacheURL({
+ url: '/config/server/version',
+ reportUrlAsIs: true,
+ });
},
getDiffPreferences() {
@@ -625,6 +702,7 @@
if (loggedIn) {
return this._fetchSharedCacheURL({
url: '/accounts/self/preferences.diff',
+ reportUrlAsIs: true,
});
}
// These defaults should match the defaults in
@@ -655,6 +733,7 @@
if (loggedIn) {
return this._fetchSharedCacheURL({
url: '/accounts/self/preferences.edit',
+ reportUrlAsIs: true,
});
}
// These defaults should match the defaults in
@@ -696,6 +775,7 @@
url: '/accounts/self/preferences',
body: prefs,
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
@@ -711,6 +791,7 @@
url: '/accounts/self/preferences.diff',
body: prefs,
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
@@ -726,12 +807,14 @@
url: '/accounts/self/preferences.edit',
body: prefs,
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
getAccount() {
return this._fetchSharedCacheURL({
url: '/accounts/self/detail',
+ reportUrlAsIs: true,
errFn: resp => {
if (!resp || resp.status === 403) {
this._cache['/accounts/self/detail'] = null;
@@ -741,7 +824,10 @@
},
getExternalIds() {
- return this._fetchJSON({url: '/accounts/self/external.ids'});
+ return this._fetchJSON({
+ url: '/accounts/self/external.ids',
+ reportUrlAsIs: true,
+ });
},
deleteAccountIdentity(id) {
@@ -750,6 +836,7 @@
url: '/accounts/self/external.ids:delete',
body: id,
parseResponse: true,
+ reportUrlAsIs: true,
});
},
@@ -760,11 +847,15 @@
getAccountDetails(userId) {
return this._fetchJSON({
url: `/accounts/${encodeURIComponent(userId)}/detail`,
+ anonymizedUrl: '/accounts/*/detail',
});
},
getAccountEmails() {
- return this._fetchSharedCacheURL({url: '/accounts/self/emails'});
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/emails',
+ reportUrlAsIs: true,
+ });
},
/**
@@ -776,6 +867,7 @@
method: 'PUT',
url: '/accounts/self/emails/' + encodeURIComponent(email),
errFn: opt_errFn,
+ anonymizedUrl: '/account/self/emails/*',
});
},
@@ -788,6 +880,7 @@
method: 'DELETE',
url: '/accounts/self/emails/' + encodeURIComponent(email),
errFn: opt_errFn,
+ anonymizedUrl: '/accounts/self/email/*',
});
},
@@ -797,8 +890,13 @@
*/
setPreferredAccountEmail(email, opt_errFn) {
const encodedEmail = encodeURIComponent(email);
- const url = `/accounts/self/emails/${encodedEmail}/preferred`;
- return this._send({method: 'PUT', url, errFn: opt_errFn}).then(() => {
+ const req = {
+ method: 'PUT',
+ url: `/accounts/self/emails/${encodedEmail}/preferred`,
+ errFn: opt_errFn,
+ anonymizedUrl: '/accounts/self/emails/*/preferred',
+ };
+ return this._send(req).then(() => {
// If result of getAccountEmails is in cache, update it in the cache
// so we don't have to invalidate it.
const cachedEmails = this._cache['/accounts/self/emails'];
@@ -840,6 +938,7 @@
body: {name},
errFn: opt_errFn,
parseResponse: true,
+ reportUrlAsIs: true,
};
return this._send(req)
.then(newName => this._updateCachedAccount({name: newName}));
@@ -856,6 +955,7 @@
body: {username},
errFn: opt_errFn,
parseResponse: true,
+ reportUrlAsIs: true,
};
return this._send(req)
.then(newName => this._updateCachedAccount({username: newName}));
@@ -872,6 +972,7 @@
body: {status},
errFn: opt_errFn,
parseResponse: true,
+ reportUrlAsIs: true,
};
return this._send(req)
.then(newStatus => this._updateCachedAccount({status: newStatus}));
@@ -880,15 +981,22 @@
getAccountStatus(userId) {
return this._fetchJSON({
url: `/accounts/${encodeURIComponent(userId)}/status`,
+ anonymizedUrl: '/accounts/*/status',
});
},
getAccountGroups() {
- return this._fetchJSON({url: '/accounts/self/groups'});
+ return this._fetchJSON({
+ url: '/accounts/self/groups',
+ reportUrlAsIs: true,
+ });
},
getAccountAgreements() {
- return this._fetchJSON({url: '/accounts/self/agreements'});
+ return this._fetchJSON({
+ url: '/accounts/self/agreements',
+ reportUrlAsIs: true,
+ });
},
saveAccountAgreement(name) {
@@ -896,6 +1004,7 @@
method: 'PUT',
url: '/accounts/self/agreements',
body: name,
+ reportUrlAsIs: true,
});
},
@@ -911,6 +1020,7 @@
}
return this._fetchSharedCacheURL({
url: '/accounts/self/capabilities' + queryString,
+ anonymizedUrl: '/accounts/self/capabilities?q=*',
});
},
@@ -937,8 +1047,9 @@
return;
}
this._credentialCheck.checking = true;
+ const req = {url: '/accounts/self/detail', reportUrlAsIs: true};
// Skip the REST response cache.
- return this._fetchRawJSON({url: '/accounts/self/detail'}).then(res => {
+ return this._fetchRawJSON(req).then(res => {
if (!res) { return; }
if (res.status === 403) {
this.fire('auth-error');
@@ -958,21 +1069,24 @@
},
getDefaultPreferences() {
- return this._fetchSharedCacheURL({url: '/config/server/preferences'});
+ return this._fetchSharedCacheURL({
+ url: '/config/server/preferences',
+ reportUrlAsIs: true,
+ });
},
getPreferences() {
return this.getLoggedIn().then(loggedIn => {
if (loggedIn) {
- return this._fetchSharedCacheURL({url: '/accounts/self/preferences'})
- .then(res => {
- if (this._isNarrowScreen()) {
- res.default_diff_view = DiffViewMode.UNIFIED;
- } else {
- res.default_diff_view = res.diff_view;
- }
- return Promise.resolve(res);
- });
+ const req = {url: '/accounts/self/preferences', reportUrlAsIs: true};
+ return this._fetchSharedCacheURL(req).then(res => {
+ if (this._isNarrowScreen()) {
+ res.default_diff_view = DiffViewMode.UNIFIED;
+ } else {
+ res.default_diff_view = res.diff_view;
+ }
+ return Promise.resolve(res);
+ });
}
return Promise.resolve({
@@ -988,6 +1102,7 @@
getWatchedProjects() {
return this._fetchSharedCacheURL({
url: '/accounts/self/watched.projects',
+ reportUrlAsIs: true,
});
},
@@ -1002,6 +1117,7 @@
body: projects,
errFn: opt_errFn,
parseResponse: true,
+ reportUrlAsIs: true,
});
},
@@ -1015,6 +1131,7 @@
url: '/accounts/self/watched.projects:delete',
body: projects,
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
@@ -1079,7 +1196,12 @@
this._maybeInsertInLookup(change);
}
};
- return this._fetchJSON({url: '/changes/', params}).then(response => {
+ const req = {
+ url: '/changes/',
+ params,
+ reportUrlAsIs: true,
+ };
+ return this._fetchJSON(req).then(response => {
// Response may be an array of changes OR an array of arrays of
// changes.
if (opt_query instanceof Array) {
@@ -1173,6 +1295,7 @@
cancelCondition: opt_cancelCondition,
params: {O: params},
fetchOptions: this._etags.getOptions(urlWithParams),
+ anonymizedUrl: '/changes/*~*/detail?O=' + params,
};
return this._fetchRawJSON(req).then(response => {
if (response && response.status === 304) {
@@ -1213,6 +1336,7 @@
changeNum,
endpoint: '/commit?links',
patchNum,
+ reportEndpointAsIs: true,
});
},
@@ -1233,6 +1357,7 @@
endpoint: '/files',
patchNum: patchRange.patchNum,
params,
+ reportEndpointAsIs: true,
});
},
@@ -1242,10 +1367,16 @@
*/
getChangeEditFiles(changeNum, patchRange) {
let endpoint = '/edit?list';
+ let anonymizedEndpoint = endpoint;
if (patchRange.basePatchNum !== 'PARENT') {
endpoint += '&base=' + encodeURIComponent(patchRange.basePatchNum + '');
+ anonymizedEndpoint += '&base=*';
}
- return this._getChangeURLAndFetch({changeNum, endpoint});
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint,
+ anonymizedEndpoint,
+ });
},
/**
@@ -1259,6 +1390,7 @@
changeNum,
endpoint: `/files?q=${encodeURIComponent(query)}`,
patchNum,
+ anonymizedEndpoint: '/files?q=*',
});
},
@@ -1287,7 +1419,12 @@
},
getChangeRevisionActions(changeNum, patchNum) {
- const req = {changeNum, endpoint: '/actions', patchNum};
+ const req = {
+ changeNum,
+ endpoint: '/actions',
+ patchNum,
+ reportEndpointAsIs: true,
+ };
return this._getChangeURLAndFetch(req).then(revisionActions => {
// The rebase button on change screen is always enabled.
if (revisionActions.rebase) {
@@ -1312,6 +1449,7 @@
endpoint: '/suggest_reviewers',
errFn: opt_errFn,
params,
+ reportEndpointAsIs: true,
});
},
@@ -1319,7 +1457,11 @@
* @param {number|string} changeNum
*/
getChangeIncludedIn(changeNum) {
- return this._getChangeURLAndFetch({changeNum, endpoint: '/in'});
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/in',
+ reportEndpointAsIs: true,
+ });
},
_computeFilter(filter) {
@@ -1345,6 +1487,7 @@
return this._fetchSharedCacheURL({
url: `/groups/?n=${groupsPerPage + 1}&S=${offset}` +
this._computeFilter(filter),
+ anonymizedUrl: '/groups/?*',
});
},
@@ -1362,6 +1505,7 @@
return this._fetchSharedCacheURL({
url: `/projects/?d&n=${reposPerPage + 1}&S=${offset}` +
this._computeFilter(filter),
+ anonymizedUrl: '/projects/?*',
});
},
@@ -1372,6 +1516,7 @@
method: 'PUT',
url: `/projects/${encodeURIComponent(repo)}/HEAD`,
body: {ref},
+ anonymizedUrl: '/projects/*/HEAD',
});
},
@@ -1391,7 +1536,11 @@
const url = `/projects/${repo}/branches?n=${count}&S=${offset}${filter}`;
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchJSON({url, errFn: opt_errFn});
+ return this._fetchJSON({
+ url,
+ errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/branches?*',
+ });
},
/**
@@ -1411,7 +1560,11 @@
encodedFilter;
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchJSON({url, errFn: opt_errFn});
+ return this._fetchJSON({
+ url,
+ errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/tags',
+ });
},
/**
@@ -1426,7 +1579,11 @@
const encodedFilter = this._computeFilter(filter);
const n = pluginsPerPage + 1;
const url = `/plugins/?all&n=${n}&S=${offset}${encodedFilter}`;
- return this._fetchJSON({url, errFn: opt_errFn});
+ return this._fetchJSON({
+ url,
+ errFn: opt_errFn,
+ anonymizedUrl: '/plugins/?all',
+ });
},
getRepoAccessRights(repoName, opt_errFn) {
@@ -1435,6 +1592,7 @@
return this._fetchJSON({
url: `/projects/${encodeURIComponent(repoName)}/access`,
errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/access',
});
},
@@ -1445,6 +1603,7 @@
method: 'POST',
url: `/projects/${encodeURIComponent(repoName)}/access`,
body: repoInfo,
+ anonymizedUrl: '/projects/*/access',
});
},
@@ -1454,6 +1613,7 @@
url: `/projects/${encodeURIComponent(projectName)}/access:review`,
body: projectInfo,
parseResponse: true,
+ anonymizedUrl: '/projects/*/access:review',
});
},
@@ -1469,6 +1629,7 @@
url: '/groups/',
errFn: opt_errFn,
params,
+ reportUrlAsIs: true,
});
},
@@ -1488,6 +1649,7 @@
url: '/projects/',
errFn: opt_errFn,
params,
+ reportUrlAsIs: true,
});
},
@@ -1506,6 +1668,7 @@
url: '/accounts/',
errFn: opt_errFn,
params,
+ anonymizedUrl: '/accounts/?n=*',
});
},
@@ -1541,6 +1704,7 @@
changeNum,
endpoint: '/related',
patchNum,
+ reportEndpointAsIs: true,
});
},
@@ -1548,6 +1712,7 @@
return this._getChangeURLAndFetch({
changeNum,
endpoint: '/submitted_together',
+ reportEndpointAsIs: true,
});
},
@@ -1560,7 +1725,11 @@
O: options,
q: 'status:open is:mergeable conflicts:' + changeNum,
};
- return this._fetchJSON({url: '/changes/', params});
+ return this._fetchJSON({
+ url: '/changes/',
+ params,
+ anonymizedUrl: '/changes/conflicts:*',
+ });
},
getChangeCherryPicks(project, changeID, changeNum) {
@@ -1578,7 +1747,11 @@
O: options,
q: query,
};
- return this._fetchJSON({url: '/changes/', params});
+ return this._fetchJSON({
+ url: '/changes/',
+ params,
+ anonymizedUrl: '/changes/change:*',
+ });
},
getChangesWithSameTopic(topic) {
@@ -1592,7 +1765,11 @@
O: options,
q: 'status:open topic:' + topic,
};
- return this._fetchJSON({url: '/changes/', params});
+ return this._fetchJSON({
+ url: '/changes/',
+ params,
+ anonymizedUrl: '/changes/topic:*',
+ });
},
getReviewedFiles(changeNum, patchNum) {
@@ -1600,6 +1777,7 @@
changeNum,
endpoint: '/files?reviewed',
patchNum,
+ reportEndpointAsIs: true,
});
},
@@ -1617,6 +1795,7 @@
patchNum,
endpoint: `/files/${encodeURIComponent(path)}/reviewed`,
errFn: opt_errFn,
+ anonymizedEndpoint: '/files/*/reviewed',
});
},
@@ -1649,6 +1828,7 @@
changeNum,
endpoint: '/edit/',
params,
+ reportEndpointAsIs: true,
});
});
},
@@ -1679,6 +1859,7 @@
base_commit: opt_baseCommit,
},
parseResponse: true,
+ reportUrlAsIs: true,
});
},
@@ -1725,6 +1906,7 @@
endpoint: `/files/${encodeURIComponent(path)}/content`,
errFn: opt_errFn,
headers: {Accept: 'application/json'},
+ anonymizedEndpoint: '/files/*/content',
});
},
@@ -1739,6 +1921,7 @@
method: 'GET',
endpoint: '/edit/' + encodeURIComponent(path),
headers: {Accept: 'application/json'},
+ anonymizedEndpoint: '/edit/*',
});
},
@@ -1747,6 +1930,7 @@
changeNum,
method: 'POST',
endpoint: '/edit:rebase',
+ reportEndpointAsIs: true,
});
},
@@ -1755,6 +1939,7 @@
changeNum,
method: 'DELETE',
endpoint: '/edit',
+ reportEndpointAsIs: true,
});
},
@@ -1764,6 +1949,7 @@
method: 'POST',
endpoint: '/edit',
body: {restore_path},
+ reportEndpointAsIs: true,
});
},
@@ -1773,6 +1959,7 @@
method: 'POST',
endpoint: '/edit',
body: {old_path, new_path},
+ reportEndpointAsIs: true,
});
},
@@ -1781,6 +1968,7 @@
changeNum,
method: 'DELETE',
endpoint: '/edit/' + encodeURIComponent(path),
+ anonymizedEndpoint: '/edit/*',
});
},
@@ -1791,6 +1979,7 @@
endpoint: '/edit/' + encodeURIComponent(path),
body: contents,
contentType: 'text/plain',
+ anonymizedEndpoint: '/edit/*',
});
},
@@ -1801,6 +1990,7 @@
method: 'PUT',
endpoint: '/edit:message',
body: {message},
+ reportEndpointAsIs: true,
});
},
@@ -1809,6 +1999,7 @@
changeNum,
method: 'POST',
endpoint: '/edit:publish',
+ reportEndpointAsIs: true,
});
},
@@ -1818,13 +2009,16 @@
method: 'PUT',
endpoint: '/message',
body: {message},
+ reportEndpointAsIs: true,
});
},
saveChangeStarred(changeNum, starred) {
- const url = '/accounts/self/starred.changes/' + changeNum;
- const method = starred ? 'PUT' : 'DELETE';
- return this._send({method, url});
+ return this._send({
+ method: starred ? 'PUT' : 'DELETE',
+ url: '/accounts/self/starred.changes/' + changeNum,
+ anonymizedUrl: '/accounts/self/starred.changes/*',
+ });
},
/**
@@ -1850,7 +2044,12 @@
}
const url = req.url.startsWith('http') ?
req.url : this.getBaseUrl() + req.url;
- const xhr = this._fetch(url, options).then(response => {
+ const fetchReq = {
+ url,
+ fetchOptions: options,
+ anonymizedUrl: req.reportUrlAsIs ? url : req.anonymizedUrl,
+ };
+ const xhr = this._fetch(fetchReq).then(response => {
if (!response.ok) {
if (req.errFn) {
return req.errFn.call(undefined, response);
@@ -1928,6 +2127,7 @@
errFn: opt_errFn,
cancelCondition: opt_cancelCondition,
params,
+ anonymizedEndpoint: '/files/*/diff',
});
},
@@ -2019,6 +2219,7 @@
changeNum,
endpoint,
patchNum: opt_patchNum,
+ reportEndpointAsIs: true,
});
};
@@ -2109,8 +2310,10 @@
_sendDiffDraftRequest(method, changeNum, patchNum, draft) {
const isCreate = !draft.id && method === 'PUT';
let endpoint = '/drafts';
+ let anonymizedEndpoint = endpoint;
if (draft.id) {
endpoint += '/' + draft.id;
+ anonymizedEndpoint += '/*';
}
let body;
if (method === 'PUT') {
@@ -2121,8 +2324,16 @@
this._pendingRequests[Requests.SEND_DIFF_DRAFT] = [];
}
- const promise = this._getChangeURLAndSend(
- {changeNum, method, patchNum, endpoint, body});
+ const req = {
+ changeNum,
+ method,
+ patchNum,
+ endpoint,
+ body,
+ anonymizedEndpoint,
+ };
+
+ const promise = this._getChangeURLAndSend(req);
this._pendingRequests[Requests.SEND_DIFF_DRAFT].push(promise);
if (isCreate) {
@@ -2136,11 +2347,12 @@
return this._fetchJSON({
url: '/projects/' + encodeURIComponent(project) +
'/commits/' + encodeURIComponent(commit),
+ anonymizedUrl: '/projects/*/comments/*',
});
},
_fetchB64File(url) {
- return this._fetch(this.getBaseUrl() + url)
+ return this._fetch({url: this.getBaseUrl() + url})
.then(response => {
if (!response.ok) { return Promise.reject(response.statusText); }
const type = response.headers.get('X-FYI-Content-Type');
@@ -2241,6 +2453,7 @@
endpoint: '/topic',
body: {topic},
parseResponse: true,
+ reportUrlAsIs: true,
});
},
@@ -2256,6 +2469,7 @@
endpoint: '/hashtags',
body: hashtag,
parseResponse: true,
+ reportUrlAsIs: true,
});
},
@@ -2263,6 +2477,7 @@
return this._send({
method: 'DELETE',
url: '/accounts/self/password.http',
+ reportUrlAsIs: true,
});
},
@@ -2277,11 +2492,15 @@
url: '/accounts/self/password.http',
body: {generate: true},
parseResponse: true,
+ reportUrlAsIs: true,
});
},
getAccountSSHKeys() {
- return this._fetchSharedCacheURL({url: '/accounts/self/sshkeys'});
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/sshkeys',
+ reportUrlAsIs: true,
+ });
},
addAccountSSHKey(key) {
@@ -2290,6 +2509,7 @@
url: '/accounts/self/sshkeys',
body: key,
contentType: 'plain/text',
+ reportUrlAsIs: true,
};
return this._send(req)
.then(response => {
@@ -2308,15 +2528,24 @@
return this._send({
method: 'DELETE',
url: '/accounts/self/sshkeys/' + id,
+ anonymizedUrl: '/accounts/self/sshkeys/*',
});
},
getAccountGPGKeys() {
- return this._fetchJSON({url: '/accounts/self/gpgkeys'});
+ return this._fetchJSON({
+ url: '/accounts/self/gpgkeys',
+ reportUrlAsIs: true,
+ });
},
addAccountGPGKey(key) {
- const req = {method: 'POST', url: '/accounts/self/gpgkeys', body: key};
+ const req = {
+ method: 'POST',
+ url: '/accounts/self/gpgkeys',
+ body: key,
+ reportUrlAsIs: true,
+ };
return this._send(req)
.then(response => {
if (response.status < 200 && response.status >= 300) {
@@ -2334,6 +2563,7 @@
return this._send({
method: 'DELETE',
url: '/accounts/self/gpgkeys/' + id,
+ anonymizedUrl: '/accounts/self/gpgkeys/*',
});
},
@@ -2342,6 +2572,7 @@
changeNum,
method: 'DELETE',
endpoint: `/reviewers/${account}/votes/${encodeURIComponent(label)}`,
+ anonymizedEndpoint: '/reviewers/*/votes/*',
});
},
@@ -2351,6 +2582,7 @@
method: 'PUT', patchNum,
endpoint: '/description',
body: {description: desc},
+ reportUrlAsIs: true,
});
},
@@ -2359,6 +2591,7 @@
method: 'PUT',
url: '/config/server/email.confirm',
body: {token},
+ reportUrlAsIs: true,
};
return this._send(req).then(response => {
if (response.status === 204) {
@@ -2372,6 +2605,7 @@
return this._fetchJSON({
url: '/config/server/capabilities',
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
@@ -2381,6 +2615,7 @@
method: 'PUT',
endpoint: '/assignee',
body: {assignee},
+ reportUrlAsIs: true,
});
},
@@ -2389,6 +2624,7 @@
changeNum,
method: 'DELETE',
endpoint: '/assignee',
+ reportUrlAsIs: true,
});
},
@@ -2408,7 +2644,13 @@
if (opt_message) {
body.message = opt_message;
}
- const req = {changeNum, method: 'POST', endpoint: '/wip', body};
+ const req = {
+ changeNum,
+ method: 'POST',
+ endpoint: '/wip',
+ body,
+ reportUrlAsIs: true,
+ };
return this._getChangeURLAndSend(req).then(response => {
if (response.status === 204) {
return 'Change marked as Work In Progress.';
@@ -2428,6 +2670,7 @@
endpoint: '/ready',
body: opt_body,
errFn: opt_errFn,
+ reportUrlAsIs: true,
});
},
@@ -2444,6 +2687,7 @@
endpoint: `/comments/${commentID}/delete`,
body: {reason},
parseResponse: true,
+ anonymizedEndpoint: '/comments/*/delete',
});
},
@@ -2459,6 +2703,7 @@
return this._fetchJSON({
url: `/changes/?q=change:${changeNum}`,
errFn: opt_errFn,
+ anonymizedUrl: '/changes/?q=change:*',
}).then(res => {
if (!res || !res.length) { return null; }
return res[0];
@@ -2509,6 +2754,11 @@
* @return {!Promise<!Object>}
*/
_getChangeURLAndSend(req) {
+ const anonymizedBaseUrl = req.patchNum ?
+ ANONYMIZED_REVISION_BASE_URL : ANONYMIZED_CHANGE_BASE_URL;
+ const anonymizedEndpoint = req.reportEndpointAsIs ?
+ req.endpoint : req.anonymizedEndpoint;
+
return this._changeBaseURL(req.changeNum, req.patchNum).then(url => {
return this._send({
method: req.method,
@@ -2518,6 +2768,8 @@
contentType: req.contentType,
headers: req.headers,
parseResponse: req.parseResponse,
+ anonymizedUrl: anonymizedEndpoint ?
+ (anonymizedBaseUrl + anonymizedEndpoint) : undefined,
});
});
},
@@ -2528,6 +2780,10 @@
* @return {!Promise<!Object>}
*/
_getChangeURLAndFetch(req) {
+ const anonymizedEndpoint = req.reportEndpointAsIs ?
+ req.endpoint : req.anonymizedEndpoint;
+ const anonymizedBaseUrl = req.patchNum ?
+ ANONYMIZED_REVISION_BASE_URL : ANONYMIZED_CHANGE_BASE_URL;
return this._changeBaseURL(req.changeNum, req.patchNum).then(url => {
return this._fetchJSON({
url: url + req.endpoint,
@@ -2535,6 +2791,8 @@
cancelCondition: req.cancelCondition,
params: req.params,
fetchOptions: req.fetchOptions,
+ anonymizedUrl: anonymizedEndpoint ?
+ (anonymizedBaseUrl + anonymizedEndpoint) : undefined,
});
});
},
@@ -2577,6 +2835,7 @@
endpoint: `/files/${encodedPath}/blame`,
patchNum,
params: opt_base ? {base: 't'} : undefined,
+ anonymizedEndpoint: '/files/*/blame',
});
},
@@ -2621,7 +2880,11 @@
getDashboard(project, dashboard, opt_errFn) {
const url = '/projects/' + encodeURIComponent(project) + '/dashboards/' +
encodeURIComponent(dashboard);
- return this._fetchSharedCacheURL({url, errFn: opt_errFn});
+ return this._fetchSharedCacheURL({
+ url,
+ errFn: opt_errFn,
+ anonymizedUrl: '/projects/*/dashboards/*',
+ });
},
getMergeable(changeNum) {
@@ -2629,6 +2892,7 @@
changeNum,
endpoint: '/revisions/current/mergeable',
parseResponse: true,
+ reportEndpointAsIs: true,
});
},
});
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 70d1465..193d306 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -711,11 +711,10 @@
sandbox.spy(element, '_send');
element.confirmEmail('foo');
assert.isTrue(element._send.calledOnce);
- assert.deepEqual(element._send.lastCall.args[0], {
- method: 'PUT',
- url: '/config/server/email.confirm',
- body: {token: 'foo'},
- });
+ assert.equal(element._send.lastCall.args[0].method, 'PUT');
+ assert.equal(element._send.lastCall.args[0].url,
+ '/config/server/email.confirm');
+ assert.deepEqual(element._send.lastCall.args[0].body, {token: 'foo'});
});
test('GrReviewerUpdatesParser.parse is used', () => {
@@ -924,11 +923,10 @@
const fetchStub = sandbox.stub(element, '_getChangeURLAndFetch')
.returns(Promise.resolve());
return element.queryChangeFiles('42', 'edit', 'test/path.js').then(() => {
- assert.deepEqual(fetchStub.lastCall.args[0], {
- changeNum: '42',
- endpoint: '/files?q=test%2Fpath.js',
- patchNum: 'edit',
- });
+ assert.equal(fetchStub.lastCall.args[0].changeNum, '42');
+ assert.equal(fetchStub.lastCall.args[0].endpoint,
+ '/files?q=test%2Fpath.js');
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 'edit');
});
});
@@ -1387,12 +1385,25 @@
sandbox.stub(element._auth, 'fetch').returns(Promise.resolve(response));
const startTime = 123;
sandbox.stub(Date, 'now').returns(startTime);
- return element._fetch(url, fetchOptions).then(() => {
+ const req = {url, fetchOptions};
+ return element._fetch(req).then(() => {
assert.isTrue(logStub.calledOnce);
- assert.isTrue(logStub.calledWith(
- url, fetchOptions, startTime, response.status));
+ assert.isTrue(logStub.calledWith(req, startTime, response.status));
assert.isFalse(response.text.called);
});
});
+
+ test('_logCall only reports requests with anonymized URLss', () => {
+ sandbox.stub(Date, 'now').returns(200);
+ const handler = sinon.stub();
+ element.addEventListener('rpc-log', handler);
+
+ element._logCall({url: 'url'}, 100, 200);
+ assert.isFalse(handler.called);
+
+ element._logCall({url: 'url', anonymizedUrl: 'not url'}, 100, 200);
+ flushAsynchronousOperations();
+ assert.isTrue(handler.calledOnce);
+ });
});
</script>
