Merge "Add allowEmpty to CherryPickInput"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 23ab39b..88edde2 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -1311,7 +1311,9 @@
Allow group creation. Groups are used to grant users access to different
actions in projects. This capability allows the granted group members to
-either link:cmd-create-group.html[create new groups via ssh] or via the web UI.
+either link:cmd-create-group.html[create new groups via ssh] or via the web UI
+by navigating at the top of the page to BROWSE -> Groups, and then pushing the
+"CREATE NEW" button.
[[capability_createProject]]
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 11980a0..29d3a02 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3123,11 +3123,9 @@
for production use. For compatibility information, please refer to the
link:https://www.gerritcodereview.com/elasticsearch.html[project homepage,role=external,window=_blank].
-When using Elasticsearch version 5.6, the open and closed changes are
-indexed in a single index, separated into types `open_changes` and `closed_changes`
-respectively. When using version 6.2 or later, the open and closed changes are
-merged into the default `_doc` type. The latter is also used for the accounts and
-groups indices starting with Elasticsearch 6.2.
+In Elasticsearch version 6.2 or later, the open and closed changes are 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.
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
index 70fbd60..1a9f74c 100644
--- a/Documentation/dev-contributing.txt
+++ b/Documentation/dev-contributing.txt
@@ -245,6 +245,79 @@
get reverted so that there is no unused or half-finished code in the
code base.
+[[esc-dd-evaluation]]
+== How the ESC evaluates design documents
+This section describes how the ESC evaluates design documents. It’s
+meant as a guideline rather than being prescriptive for both ESC
+members and contributors.
+
+=== General Process
+As part of the design process, the ESC makes a final decision if a
+design gets to be implemented. If there are multiple alternative
+solutions, the ESC will decide which solution can be implemented.
+
+The ESC should wait until all contributors had the chance to
+voice their opinion in review comments or by proposing alternative
+solutions. Due to the infrequent ESC meetings (every 2-4 weeks)
+the ESC might discuss documents in cases where the discussion is
+already advanced far enough, but not make a decision yet. In this
+case, contributors can still voice concerns or discuss alternatives.
+The decision can be at the next meeting or via email in between
+meetings.
+
+=== Evaluation
+Product/Vision fit
+Q: `Do we believe this feature belongs to Gerrit Code Review use-cases?`
+* Yes: Customizable dashboards
+* No: UI for managing an LDAP server
+
+Q: `Is the proposed solution aligned with Gerrit’s vision?`
+* Yes: Showing comments of older patch sets in newer patch sets to
+ keep track (core code review)
+* No: Implement a bug tracker in Gerrit (not core code review).
+
+=== Impact
+Q: `Will the new feature have a measurable, positive impact?`
+* Yes: Increased productivity, faster/smoother workflow, etc.
+* Yes: Better latency, more reliable system.
+* No: Unclear impact or lacking metrics to measure.
+
+=== Complexity
+Q: `Can we support/maintain this feature once it is in Gerrit?`
+* Yes: Code will fit into codebase, be well tested, easy to
+ understand.
+* No: Will add code or a workflow that is hard to understand
+ and easy to misinterpret.
+Q: `Is the proposed feature or rework adding unnecessary complexity?`
+* Yes: Adding a dependency on a well-supported library.
+* No: Adding a dependency on a library that is not widely used
+ or not actively maintained.
+
+=== Core vs. Plugin decision
+Q: `Would this fit better in a plugin?`
+* Yes:The proposed feature or rework is an implementation (e.g. Lucene
+ is an index implementation) of a generic concept that others
+ might want to implement differently.
+* Yes: The proposed feature or rework is very specific to a custom setup.
+* No: The proposed feature or rework is applicable to a wider user
+ base.
+* No: The proposed feature or rework is a `core code review feature`.
+
+=== Commitment
+Q: `Is someone willing to implement it?` (this question is
+especially important when reviewers propose alternative designs
+to the author’s own solution).
+* Yes: The author or someone else commits to implementing the
+ proposed solution.
+* Yes: If a mentorship is required, a mentor is willing to help.
+* No: Unclear ownership, mentorship or implementation plan.
+
+=== Community
+Q: `If in doubt, is there a substantial benefit to a long-standing
+community member with many users?`
+* The community shapes the future of Gerrit as a product. In
+ cases of doubt, the ESC can be more generous with long-standing community members compared to `drive-by` contributions.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-core-plugins.txt b/Documentation/dev-core-plugins.txt
index 12726b7..04e2420 100644
--- a/Documentation/dev-core-plugins.txt
+++ b/Documentation/dev-core-plugins.txt
@@ -27,6 +27,11 @@
Non-Gerrit maintainers cannot have link:access-control.html#category_owner[
Owner] permissions for core plugins.
+[[list]]
+== Which core plugins exist?
+
+See link:config-plugins.html#core-plugins[here].
+
[[criteria]]
=== Criteria for Core Plugins
@@ -82,10 +87,73 @@
their ownership. If the current plugin owners disagree, forking the plugin is
possible, but this should happen only in exceptional cases.
-[[list]]
-== Which core plugins exist?
+[[process-to-make-a-plugin-a-core-plugin]]
+== Process to make a plugin a core plugin
-See link:config-plugins.html#core-plugins[here].
+Anyone can propose to make a plugin a core plugin, but to be accepted as core
+plugin the plugin must fulfill certain link:#criteria[criteria].
+
+1. Propose a plugin as core plugin:
++
+File a link:https://bugs.chromium.org/p/gerrit/issues/entry?template=Core+Plugin+Request[
+Core Plugin Request] in the issue tracker and provide the information that is
+being asked for in the request template.
+
+2. Community Feedback:
++
+Anyone can comment on the issue to raise concerns or to support the request.
+The issue should stay open for at least 10 calendar days so that everyone in
+the community has a chance to comment.
+
+3. ESC Decision:
++
+The ESC should discuss the request and reject/approve it or ask for further
+information that the reporter or commenters should provide.
++
+Any decision must be based on the link:#criteria[criteria] that core plugins
+are expected to fulfill and should take the feedback from the community into
+account.
++
+Accepting the request is only possible if the issue was open for at least 10
+calendar days (see 2., this gives the community time to comment).
++
+When the ESC approves/rejects the request a summary of the reasons for the
+decision should be added to the issue.
++
+If a request is rejected, it's possible to ask for a revalidation if the
+concerns that led to the rejection have been addressed (e.g. the plugin was
+rejected due to missing tests, but tests have been added afterwards).
+
+4. Add plugin as core plugin:
++
+If the request was accepted, a Gerrit maintainer should add the plugin as
+core plugin:
++
+--
+** Host the plugin repo on link:https://gerrit-review.googlesource.com/[
+ gerrit-review].
+** Ensure that the plugin repo inherits from the
+ link:https://gerrit-review.googlesource.com/admin/repos/Public-Plugins[
+ Public-Plugins] repo.
+** Remove all permissions on the plugin repo (the inherited permissions from
+ `Public-Plugins` should be enough). Especially make sure that there are no
+ link:access-control.html#category_owner[Owner],
+ link:access-control.html#category_push_direct[Direct Push],
+ link:access-control.html#category_submit[Submit] or
+ link:access-control.html#category_review_labels[Code-Review+2]
+ permissions for non-Gerrit maintainers.
+** Create a component for the plugin in
+ link:https://bugs.chromium.org/p/gerrit/adminComponents[Monorail] and assign
+ all issues that already exist for the plugin to this component.
+** Add the plugin as
+ link:https://gerrit.googlesource.com/gerrit/+/refs/heads/master/.gitmodules[Git
+ submodule].
+** Register the plugin as core plugin in
+ link:https://gerrit.googlesource.com/gerrit/+/refs/heads/master/tools/bzl/plugins.bzl[
+ plugins.bzl].
+** Announce the new core plugin in the
+ link:https://www.gerritcodereview.com/news.html[project news].
+--
GERRIT
------
diff --git a/Documentation/dev-roles.txt b/Documentation/dev-roles.txt
index c16037e..2ca7f22 100644
--- a/Documentation/dev-roles.txt
+++ b/Documentation/dev-roles.txt
@@ -336,7 +336,10 @@
steering committee members] at the same time so that they can represent
the community without conflict of interest.
-Nomination process, election process and election period for the
+Anybody from the Gerrit community can candidate as community manager.
+This means, in contrast to candidating for the ESC, candidating as
+community manager is not limited to Gerrit maintainers. Otherwise the
+nomination process, election process and election period for the
non-Google community manager are the same as for
link:dev-processes.html#steering-committee-election[steering committee
members].
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index dd55517..efc9de8 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2728,6 +2728,20 @@
PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0
----
+To upload a file as binary data in the request body:
+
+.Request
+----
+ PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "binary_content": "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ=="
+ }
+----
+
+Note that it must be base-64 encoded data uri.
+
When change edit doesn't exist for this change yet it is created. When file
content isn't provided, it is wiped out for that file. As response
"`204 No Content`" is returned.
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index 730af90..a21f9f5 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -138,8 +138,7 @@
SitePaths sitePaths,
Schema<V> schema,
ElasticRestClientProvider client,
- String indexName,
- String indexType) {
+ String indexName) {
this.config = config;
this.sitePaths = sitePaths;
this.schema = schema;
@@ -148,16 +147,7 @@
this.indexName = config.getIndexName(indexName, schema.getVersion());
this.indexNameRaw = indexName;
this.client = client;
- this.type = client.adapter().getType(indexType);
- }
-
- AbstractElasticIndex(
- ElasticConfiguration cfg,
- SitePaths sitePaths,
- Schema<V> schema,
- ElasticRestClientProvider client,
- String indexName) {
- this(cfg, sitePaths, schema, client, indexName, indexName);
+ this.type = client.adapter().getType();
}
@Override
@@ -223,8 +213,8 @@
protected abstract String getId(V v);
- protected String getMappingsForSingleType(String candidateType, MappingProperties properties) {
- return getMappingsFor(client.adapter().getType(candidateType), properties);
+ protected String getMappingsForSingleType(MappingProperties properties) {
+ return getMappingsFor(client.adapter().getType(), properties);
}
protected String getMappingsFor(String type, MappingProperties properties) {
@@ -240,8 +230,8 @@
return gson.toJson(mappings);
}
- protected String delete(String type, K id) {
- return new DeleteRequest(id.toString(), indexName, type, client.adapter()).toString();
+ protected String getDeleteRequest(K id) {
+ return new DeleteRequest(id.toString(), indexName).toString();
}
protected abstract V fromDocument(JsonObject doc, Set<String> fields);
diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
index c3e3264..33217bd 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
@@ -74,7 +74,7 @@
@Override
public void replace(AccountState as) {
BulkRequest bulk =
- new IndexRequest(getId(as), indexName, type, client.adapter())
+ new IndexRequest(getId(as), indexName)
.add(new UpdateRequest<>(schema, as, ImmutableSet.of()));
String uri = getURI(type, BULK);
@@ -105,12 +105,12 @@
@Override
protected String getDeleteActions(Account.Id a) {
- return delete(type, a);
+ return getDeleteRequest(a);
}
@Override
protected String getMappings() {
- return getMappingsForSingleType(ACCOUNTS, mapping.accounts);
+ return getMappingsForSingleType(mapping.accounts);
}
@Override
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 7fc4d0e..fe55eab 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -21,7 +21,6 @@
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
@@ -30,7 +29,6 @@
import com.google.common.collect.Sets;
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
import com.google.gerrit.elasticsearch.bulk.BulkRequest;
-import com.google.gerrit.elasticsearch.bulk.DeleteRequest;
import com.google.gerrit.elasticsearch.bulk.IndexRequest;
import com.google.gerrit.elasticsearch.bulk.UpdateRequest;
import com.google.gerrit.entities.Account;
@@ -119,24 +117,8 @@
@Override
public void replace(ChangeData cd) {
- String deleteIndex;
- String insertIndex;
-
- if (cd.change().isNew()) {
- insertIndex = OPEN_CHANGES;
- deleteIndex = CLOSED_CHANGES;
- } else {
- insertIndex = CLOSED_CHANGES;
- deleteIndex = OPEN_CHANGES;
- }
-
- ElasticQueryAdapter adapter = client.adapter();
BulkRequest bulk =
- new IndexRequest(getId(cd), indexName, adapter.getType(insertIndex), adapter)
- .add(new UpdateRequest<>(schema, cd, skipFields));
- if (adapter.deleteToReplace()) {
- bulk.add(new DeleteRequest(cd.getId().toString(), indexName, deleteIndex, adapter));
- }
+ new IndexRequest(getId(cd), indexName).add(new UpdateRequest<>(schema, cd, skipFields));
String uri = getURI(type, BULK);
Response response = postRequest(uri, bulk, getRefreshParam());
@@ -190,18 +172,12 @@
@Override
protected String getDeleteActions(Change.Id c) {
- if (!client.adapter().useV5Type()) {
- return delete(client.adapter().getType(), c);
- }
- return delete(OPEN_CHANGES, c) + delete(CLOSED_CHANGES, c);
+ return getDeleteRequest(c);
}
@Override
protected String getMappings() {
- if (!client.adapter().useV5Type()) {
- return getMappingsFor(client.adapter().getType(), mapping.changes);
- }
- return gson.toJson(ImmutableMap.of(MAPPINGS, mapping));
+ return getMappingsFor(client.adapter().getType(), mapping.changes);
}
@Override
diff --git a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
index ce2025f..a16ec7f 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
@@ -74,7 +74,7 @@
@Override
public void replace(InternalGroup group) {
BulkRequest bulk =
- new IndexRequest(getId(group), indexName, type, client.adapter())
+ new IndexRequest(getId(group), indexName)
.add(new UpdateRequest<>(schema, group, ImmutableSet.of()));
String uri = getURI(type, BULK);
@@ -97,12 +97,12 @@
@Override
protected String getDeleteActions(AccountGroup.UUID g) {
- return delete(type, g);
+ return getDeleteRequest(g);
}
@Override
protected String getMappings() {
- return getMappingsForSingleType(GROUPS, mapping.groups);
+ return getMappingsForSingleType(mapping.groups);
}
@Override
diff --git a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
index 392c776..ed15e34 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
@@ -76,7 +76,7 @@
@Override
public void replace(ProjectData projectState) {
BulkRequest bulk =
- new IndexRequest(projectState.getProject().getName(), indexName, type, client.adapter())
+ new IndexRequest(projectState.getProject().getName(), indexName)
.add(new UpdateRequest<>(schema, projectState, ImmutableSet.of()));
String uri = getURI(type, BULK);
@@ -99,12 +99,12 @@
@Override
protected String getDeleteActions(Project.NameKey nameKey) {
- return delete(type, nameKey);
+ return getDeleteRequest(nameKey);
}
@Override
protected String getMappings() {
- return getMappingsForSingleType(PROJECTS, mapping.projects);
+ return getMappingsForSingleType(mapping.projects);
}
@Override
diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
index 72c52b0..7233682 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
@@ -16,15 +16,12 @@
import static com.google.gerrit.elasticsearch.ElasticVersion.V6_7;
-import com.google.gson.JsonObject;
-
public class ElasticQueryAdapter {
static final String V6_TYPE = "_doc";
private static final String INCLUDE_TYPE = "include_type_name=true";
private static final String INDICES = "?allow_no_indices=false";
- private final boolean useV5Type;
private final boolean useV6Type;
private final boolean omitType;
private final int defaultNumberOfShards;
@@ -39,7 +36,6 @@
private final String includeTypeNameParam;
ElasticQueryAdapter(ElasticVersion version) {
- this.useV5Type = !version.isV6OrLater();
this.useV6Type = version.isV6();
this.omitType = version.isV7OrLater();
this.defaultNumberOfShards = version.isV7OrLater() ? 1 : 5;
@@ -54,12 +50,6 @@
this.includeTypeNameParam = version.isAtLeastMinorVersion(V6_7) ? "?" + INCLUDE_TYPE : "";
}
- public void setType(JsonObject properties, String type) {
- if (useV5Type) {
- properties.addProperty("_type", type);
- }
- }
-
public String searchFilteringName() {
return searchFilteringName;
}
@@ -84,14 +74,6 @@
return rawFieldsKey;
}
- boolean deleteToReplace() {
- return useV5Type;
- }
-
- boolean useV5Type() {
- return useV5Type;
- }
-
boolean useV6Type() {
return useV6Type;
}
@@ -105,14 +87,7 @@
}
String getType() {
- return getType("");
- }
-
- String getType(String type) {
- if (useV6Type()) {
- return V6_TYPE;
- }
- return useV5Type() ? type : "";
+ return useV6Type() ? V6_TYPE : "";
}
String getVersionDiscoveryUrl(String name) {
diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
index 6f4ade2..2d023cf 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
@@ -18,7 +18,6 @@
import java.util.regex.Pattern;
public enum ElasticVersion {
- V5_6("5.6.*"),
V6_2("6.2.*"),
V6_3("6.3.*"),
V6_4("6.4.*"),
diff --git a/java/com/google/gerrit/elasticsearch/bulk/ActionRequest.java b/java/com/google/gerrit/elasticsearch/bulk/ActionRequest.java
index 7392d09..16b821c 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/ActionRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/ActionRequest.java
@@ -14,7 +14,6 @@
package com.google.gerrit.elasticsearch.bulk;
-import com.google.gerrit.elasticsearch.ElasticQueryAdapter;
import com.google.gson.JsonObject;
abstract class ActionRequest extends BulkRequest {
@@ -22,16 +21,11 @@
private final String action;
private final String id;
private final String index;
- private final String type;
- private final ElasticQueryAdapter adapter;
- protected ActionRequest(
- String action, String id, String index, String type, ElasticQueryAdapter adapter) {
+ protected ActionRequest(String action, String id, String index) {
this.action = action;
this.id = id;
this.index = index;
- this.type = type;
- this.adapter = adapter;
}
@Override
@@ -39,7 +33,6 @@
JsonObject properties = new JsonObject();
properties.addProperty("_id", id);
properties.addProperty("_index", index);
- adapter.setType(properties, type);
JsonObject jsonAction = new JsonObject();
jsonAction.add(action, properties);
diff --git a/java/com/google/gerrit/elasticsearch/bulk/DeleteRequest.java b/java/com/google/gerrit/elasticsearch/bulk/DeleteRequest.java
index 570d5a0..6451b0f 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/DeleteRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/DeleteRequest.java
@@ -14,11 +14,9 @@
package com.google.gerrit.elasticsearch.bulk;
-import com.google.gerrit.elasticsearch.ElasticQueryAdapter;
-
public class DeleteRequest extends ActionRequest {
- public DeleteRequest(String id, String index, String type, ElasticQueryAdapter adapter) {
- super("delete", id, index, type, adapter);
+ public DeleteRequest(String id, String index) {
+ super("delete", id, index);
}
}
diff --git a/java/com/google/gerrit/elasticsearch/bulk/IndexRequest.java b/java/com/google/gerrit/elasticsearch/bulk/IndexRequest.java
index c571a0e..d90b80f 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/IndexRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/IndexRequest.java
@@ -14,11 +14,9 @@
package com.google.gerrit.elasticsearch.bulk;
-import com.google.gerrit.elasticsearch.ElasticQueryAdapter;
-
public class IndexRequest extends ActionRequest {
- public IndexRequest(String id, String index, String type, ElasticQueryAdapter adapter) {
- super("index", id, index, type, adapter);
+ public IndexRequest(String id, String index) {
+ super("index", id, index);
}
}
diff --git a/java/com/google/gerrit/extensions/api/changes/FileContentInput.java b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
index 93c253d..0cfe908 100644
--- a/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
@@ -20,4 +20,5 @@
/** Content to be added to a file (new or existing) via change edit. */
public class FileContentInput {
@DefaultInput public RawInput content;
+ public String binary_content;
}
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index 68e39e7..9a25f52 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
@@ -35,6 +36,7 @@
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
@@ -66,9 +68,12 @@
import java.io.IOException;
import java.util.List;
import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.Base64;
import org.kohsuke.args4j.Option;
@Singleton
@@ -277,6 +282,10 @@
/** Put handler that is activated when PUT request is called on collection element. */
@Singleton
public static class Put implements RestModifyView<ChangeEditResource, FileContentInput> {
+ private static final Pattern BINARY_DATA_PATTERN =
+ Pattern.compile("data:([\\w/.-]*);([\\w]+),(.*)");
+ private static final String BASE64 = "base64";
+
private final ChangeEditModifier editModifier;
private final GitRepositoryManager repositoryManager;
private final EditMessage editMessage;
@@ -301,22 +310,36 @@
public Response<Object> apply(ChangeResource rsrc, String path, FileContentInput input)
throws AuthException, BadRequestException, ResourceConflictException, IOException,
PermissionBackendException {
- if (input.content == null) {
- throw new BadRequestException("new content required");
+
+ if (input.content == null && input.binary_content == null) {
+ throw new BadRequestException("either content or binary_content is required");
}
- if (Patch.COMMIT_MSG.equals(path)) {
+ RawInput newContent;
+ if (input.binary_content != null) {
+ Matcher m = BINARY_DATA_PATTERN.matcher(input.binary_content);
+ if (m.matches() && BASE64.equals(m.group(2))) {
+ newContent = RawInputUtil.create(Base64.decode(m.group(3)));
+ } else {
+ throw new BadRequestException("binary_content must be encoded as base64 data uri");
+ }
+ } else {
+ newContent = input.content;
+ }
+
+ if (Patch.COMMIT_MSG.equals(path) && input.binary_content == null) {
EditMessage.Input editCommitMessageInput = new EditMessage.Input();
editCommitMessageInput.message =
- new String(ByteStreams.toByteArray(input.content.getInputStream()), UTF_8);
+ new String(ByteStreams.toByteArray(newContent.getInputStream()), UTF_8);
return editMessage.apply(rsrc, editCommitMessageInput);
}
+
if (Strings.isNullOrEmpty(path) || path.charAt(0) == '/') {
throw new ResourceConflictException("Invalid path: " + path);
}
try (Repository repository = repositoryManager.openRepository(rsrc.getProject())) {
- editModifier.modifyFile(repository, rsrc.getNotes(), path, input.content);
+ editModifier.modifyFile(repository, rsrc.getNotes(), path, newContent);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index b10dd41..266931a 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -61,12 +61,10 @@
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.ReviewResult;
-import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
-import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.FixReplacementInfo;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -330,23 +328,9 @@
for (ReviewerAddition reviewerResult : reviewerResults) {
reviewerResult.op.suppressEmail(); // Send a single batch email below.
bu.addOp(revision.getChange().getId(), reviewerResult.op);
- if (!ccOrReviewer && reviewerResult.result.reviewers != null) {
- for (ReviewerInfo reviewerInfo : reviewerResult.result.reviewers) {
- if (Objects.equals(id.get(), reviewerInfo._accountId)) {
- logger.atFine().log("calling user is explicitly added as reviewer");
- ccOrReviewer = true;
- break;
- }
- }
- }
- if (!ccOrReviewer && reviewerResult.result.ccs != null) {
- for (AccountInfo accountInfo : reviewerResult.result.ccs) {
- if (Objects.equals(id.get(), accountInfo._accountId)) {
- logger.atFine().log("calling user is explicitly added as cc");
- ccOrReviewer = true;
- break;
- }
- }
+ if (!ccOrReviewer && reviewerResult.reviewers.contains(id)) {
+ logger.atFine().log("calling user is explicitly added as reviewer or CC");
+ ccOrReviewer = true;
}
}
@@ -389,8 +373,7 @@
revision.getChange().getId(), new Op(projectState, revision.getPatchSet().id(), input));
// Notify based on ReviewInput, ignoring the notify settings from any AddReviewerInputs.
- NotifyResolver.Result notify =
- notifyResolver.resolve(getNotifyHandling(input, output, revision), input.notifyDetails);
+ NotifyResolver.Result notify = notifyResolver.resolve(input.notify, input.notifyDetails);
bu.setNotify(notify);
bu.execute();
@@ -408,17 +391,6 @@
return Response.ok(output);
}
- private NotifyHandling getNotifyHandling(
- ReviewInput input, ReviewResult output, RevisionResource revision) {
- if (input.notify != null) {
- return input.notify;
- }
- if ((output.ready != null && output.ready) || !revision.getChange().isWorkInProgress()) {
- return NotifyHandling.ALL;
- }
- return NotifyHandling.NONE;
- }
-
private NotifyHandling defaultNotify(Change c, ReviewInput in) {
boolean workInProgress = c.isWorkInProgress();
if (in.workInProgress) {
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 362fb1b..0b8f441 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -149,7 +149,7 @@
@Test
public void addedRobotCommentsAreLinkedToChangeMessages() throws Exception {
TestTimeUtil.resetWithClockStep(0, TimeUnit.SECONDS);
- PushOneCommit.Result r = createChange();
+ createChange();
/* Advancing the time after creating the change so that the first robot comment is not in the same timestamp as with the change creation */
TestTimeUtil.incrementClock(5, TimeUnit.SECONDS);
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 64349a4..b717eb7 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -96,6 +96,15 @@
private static final byte[] CONTENT_NEW = "baz".getBytes(UTF_8);
private static final String CONTENT_NEW2_STR = "quxÄÜÖßµ";
private static final byte[] CONTENT_NEW2 = CONTENT_NEW2_STR.getBytes(UTF_8);
+ private static final String CONTENT_BINARY_ENCODED_NEW =
+ "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==";
+ private static final byte[] CONTENT_BINARY_DECODED_NEW = "Hello, World!".getBytes(UTF_8);
+ private static final String CONTENT_BINARY_ENCODED_NEW2 =
+ "data:text/plain;base64,VXBsb2FkaW5nIHRvIGFuIGVkaXQgd29ya2VkIQ==";
+ private static final byte[] CONTENT_BINARY_DECODED_NEW2 =
+ "Uploading to an edit worked!".getBytes(UTF_8);
+ private static final String CONTENT_BINARY_ENCODED_NEW3 =
+ "data:text/plain,VXBsb2FkaW5nIHRvIGFuIGVkaXQgd29ya2VkIQ==";
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -316,7 +325,7 @@
assertThrows(
BadRequestException.class,
() -> gApi.changes().id(changeId).edit().modifyFile(Patch.COMMIT_MSG, (RawInput) null));
- assertThat(ex).hasMessageThat().isEqualTo("new content required");
+ assertThat(ex).hasMessageThat().isEqualTo("either content or binary_content is required");
}
@Test
@@ -560,11 +569,30 @@
}
@Test
+ public void createAndUploadBinaryInChangeEditOneRequestRest() throws Exception {
+ FileContentInput in = new FileContentInput();
+ in.binary_content = CONTENT_BINARY_ENCODED_NEW;
+ adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
+ ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW);
+ in.binary_content = CONTENT_BINARY_ENCODED_NEW2;
+ adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
+ ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW2);
+ }
+
+ @Test
+ public void invalidBase64UploadBinaryInChangeEditOneRequestRest() throws Exception {
+ FileContentInput in = new FileContentInput();
+ in.binary_content = CONTENT_BINARY_ENCODED_NEW3;
+ adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertBadRequest();
+ }
+
+ @Test
public void changeEditNoContentProvidedRest() throws Exception {
createEmptyEditFor(changeId);
- adminRestSession
- .put(urlEditFile(changeId, FILE_NAME), new FileContentInput())
- .assertBadRequest();
+
+ FileContentInput in = new FileContentInput();
+ in.binary_content = null;
+ adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertBadRequest();
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
index 5bbaf40..18c2e72 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -26,11 +26,6 @@
public class ElasticReindexIT extends AbstractReindexTests {
@ConfigSuite.Default
- public static Config elasticsearchV5() {
- return getConfig(ElasticVersion.V5_6);
- }
-
- @ConfigSuite.Config
public static Config elasticsearchV6() {
return getConfig(ElasticVersion.V6_8);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
index 8cfcbab..264ced6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
@@ -45,7 +45,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
-import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
import java.nio.charset.Charset;
import java.util.ArrayList;
@@ -55,11 +54,9 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.util.RawParseUtils;
import org.junit.Test;
-import org.junit.runner.RunWith;
@UseClockStep
@UseTimezone(timezone = "US/Eastern")
-@RunWith(ConfigSuite.class)
public class ChangeMessagesIT extends AbstractDaemonTest {
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailIT.java
index 0d31a96..c296a5c 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailIT.java
@@ -30,10 +30,8 @@
import org.eclipse.jgit.lib.Config;
import org.junit.Rule;
import org.junit.Test;
-import org.junit.runner.RunWith;
@NoHttpd
-@RunWith(ConfigSuite.class)
public class MailIT extends AbstractDaemonTest {
private static final String RECEIVEEMAIL = "receiveemail";
private static final String HOST = "localhost";
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
index 049fb8b..f130d1f8 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -27,11 +27,6 @@
public class ElasticIndexIT extends AbstractIndexTests {
@ConfigSuite.Default
- public static Config elasticsearchV5() {
- return getConfig(ElasticVersion.V5_6);
- }
-
- @ConfigSuite.Config
public static Config elasticsearchV6() {
return getConfig(ElasticVersion.V6_8);
}
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 9783f52..ab2bb12 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -47,8 +47,6 @@
SUFFIX = "sTest.java"
-ELASTICSEARCH_TESTS_V5 = {i: "ElasticV5Query" + i.capitalize() + SUFFIX for i in TYPES}
-
ELASTICSEARCH_TESTS_V6 = {i: "ElasticV6Query" + i.capitalize() + SUFFIX for i in TYPES}
ELASTICSEARCH_TESTS_V7 = {i: "ElasticV7Query" + i.capitalize() + SUFFIX for i in TYPES}
@@ -60,14 +58,6 @@
]
[junit_tests(
- name = "elasticsearch_query_%ss_test_V5" % name,
- size = "large",
- srcs = [src],
- tags = ELASTICSEARCH_TAGS,
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
-) for name, src in ELASTICSEARCH_TESTS_V5.items()]
-
-[junit_tests(
name = "elasticsearch_query_%ss_test_V6" % name,
size = "large",
srcs = [src],
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 9d0319c..879c7c5 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -38,8 +38,6 @@
private static String getImageName(ElasticVersion version) {
switch (version) {
- case V5_6:
- return "blacktop/elasticsearch:5.6.16";
case V6_2:
return "blacktop/elasticsearch:6.2.4";
case V6_3:
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
index 2ae59b0..dcc6880 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -23,22 +23,13 @@
import org.eclipse.jgit.lib.Config;
public final class ElasticTestUtils {
- public static void configure(
- Config config, ElasticContainer container, String prefix, ElasticVersion version) {
+ public static void configure(Config config, ElasticContainer container, String prefix) {
String hostname = container.getHttpHost().getHostName();
int port = container.getHttpHost().getPort();
config.setString("index", null, "type", "elasticsearch");
config.setString("elasticsearch", null, "server", "http://" + hostname + ":" + port);
config.setString("elasticsearch", null, "prefix", prefix);
config.setInt("index", null, "maxLimit", 10000);
- String password = version == ElasticVersion.V5_6 ? "changeme" : null;
- if (password != null) {
- config.setString("elasticsearch", null, "password", password);
- }
- }
-
- public static void configure(Config config, ElasticContainer container, String prefix) {
- configure(config, container, prefix, null);
}
public static void createAllIndexes(Injector injector) {
@@ -53,7 +44,7 @@
ElasticContainer container = ElasticContainer.createAndStart(version);
String indicesPrefix = UUID.randomUUID().toString();
Config cfg = new Config();
- configure(cfg, container, indicesPrefix, version);
+ configure(cfg, container, indicesPrefix);
return cfg;
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
deleted file mode 100644
index ab2b98d..0000000
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
+++ /dev/null
@@ -1,64 +0,0 @@
-// 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 com.google.gerrit.server.query.account.AbstractQueryAccountsTest;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.gerrit.testing.IndexConfig;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import org.eclipse.jgit.lib.Config;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-
-public class ElasticV5QueryAccountsTest extends AbstractQueryAccountsTest {
- @ConfigSuite.Default
- public static Config defaultConfig() {
- return IndexConfig.createForElasticsearch();
- }
-
- private static ElasticContainer container;
-
- @BeforeClass
- public static void startIndexService() {
- if (container == null) {
- // Only start Elasticsearch once
- container = ElasticContainer.createAndStart(ElasticVersion.V5_6);
- }
- }
-
- @AfterClass
- public static void stopElasticsearchServer() {
- if (container != null) {
- container.stop();
- }
- }
-
- @Override
- protected void initAfterLifecycleStart() throws Exception {
- super.initAfterLifecycleStart();
- ElasticTestUtils.createAllIndexes(injector);
- }
-
- @Override
- protected Injector createInjector() {
- Config elasticsearchConfig = new Config(config);
- InMemoryModule.setDefaults(elasticsearchConfig);
- String indicesPrefix = testName.getSanitizedMethodName();
- ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix, ElasticVersion.V5_6);
- return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
- }
-}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
deleted file mode 100644
index 97f235c..0000000
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
+++ /dev/null
@@ -1,68 +0,0 @@
-// 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 com.google.gerrit.server.query.change.AbstractQueryChangesTest;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.GerritTestName;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.gerrit.testing.IndexConfig;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import org.eclipse.jgit.lib.Config;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-
-public class ElasticV5QueryChangesTest extends AbstractQueryChangesTest {
- @ConfigSuite.Default
- public static Config defaultConfig() {
- return IndexConfig.createForElasticsearch();
- }
-
- private static ElasticContainer container;
-
- @BeforeClass
- public static void startIndexService() {
- if (container == null) {
- // Only start Elasticsearch once
- container = ElasticContainer.createAndStart(ElasticVersion.V5_6);
- }
- }
-
- @AfterClass
- public static void stopElasticsearchServer() {
- if (container != null) {
- container.stop();
- }
- }
-
- @Rule public final GerritTestName testName = new GerritTestName();
-
- @Override
- protected void initAfterLifecycleStart() throws Exception {
- super.initAfterLifecycleStart();
- ElasticTestUtils.createAllIndexes(injector);
- }
-
- @Override
- protected Injector createInjector() {
- Config elasticsearchConfig = new Config(config);
- InMemoryModule.setDefaults(elasticsearchConfig);
- String indicesPrefix = testName.getSanitizedMethodName();
- ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix, ElasticVersion.V5_6);
- return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
- }
-}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
deleted file mode 100644
index c490e78..0000000
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
+++ /dev/null
@@ -1,64 +0,0 @@
-// 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 com.google.gerrit.server.query.group.AbstractQueryGroupsTest;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.gerrit.testing.IndexConfig;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import org.eclipse.jgit.lib.Config;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-
-public class ElasticV5QueryGroupsTest extends AbstractQueryGroupsTest {
- @ConfigSuite.Default
- public static Config defaultConfig() {
- return IndexConfig.createForElasticsearch();
- }
-
- private static ElasticContainer container;
-
- @BeforeClass
- public static void startIndexService() {
- if (container == null) {
- // Only start Elasticsearch once
- container = ElasticContainer.createAndStart(ElasticVersion.V5_6);
- }
- }
-
- @AfterClass
- public static void stopElasticsearchServer() {
- if (container != null) {
- container.stop();
- }
- }
-
- @Override
- protected void initAfterLifecycleStart() throws Exception {
- super.initAfterLifecycleStart();
- ElasticTestUtils.createAllIndexes(injector);
- }
-
- @Override
- protected Injector createInjector() {
- Config elasticsearchConfig = new Config(config);
- InMemoryModule.setDefaults(elasticsearchConfig);
- String indicesPrefix = testName.getSanitizedMethodName();
- ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix, ElasticVersion.V5_6);
- return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
- }
-}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java
deleted file mode 100644
index a6a8605..0000000
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java
+++ /dev/null
@@ -1,64 +0,0 @@
-// 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 com.google.gerrit.server.query.project.AbstractQueryProjectsTest;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.gerrit.testing.IndexConfig;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import org.eclipse.jgit.lib.Config;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-
-public class ElasticV5QueryProjectsTest extends AbstractQueryProjectsTest {
- @ConfigSuite.Default
- public static Config defaultConfig() {
- return IndexConfig.createForElasticsearch();
- }
-
- private static ElasticContainer container;
-
- @BeforeClass
- public static void startIndexService() {
- if (container == null) {
- // Only start Elasticsearch once
- container = ElasticContainer.createAndStart(ElasticVersion.V5_6);
- }
- }
-
- @AfterClass
- public static void stopElasticsearchServer() {
- if (container != null) {
- container.stop();
- }
- }
-
- @Override
- protected void initAfterLifecycleStart() throws Exception {
- super.initAfterLifecycleStart();
- ElasticTestUtils.createAllIndexes(injector);
- }
-
- @Override
- protected Injector createInjector() {
- Config elasticsearchConfig = new Config(config);
- InMemoryModule.setDefaults(elasticsearchConfig);
- String indicesPrefix = testName.getSanitizedMethodName();
- ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix, ElasticVersion.V5_6);
- return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
- }
-}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
index 0482211..20c8a17 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -22,9 +22,6 @@
public class ElasticVersionTest {
@Test
public void supportedVersion() throws Exception {
- assertThat(ElasticVersion.forVersion("5.6.0")).isEqualTo(ElasticVersion.V5_6);
- assertThat(ElasticVersion.forVersion("5.6.11")).isEqualTo(ElasticVersion.V5_6);
-
assertThat(ElasticVersion.forVersion("6.2.0")).isEqualTo(ElasticVersion.V6_2);
assertThat(ElasticVersion.forVersion("6.2.4")).isEqualTo(ElasticVersion.V6_2);
@@ -82,7 +79,6 @@
@Test
public void atLeastMinorVersion() throws Exception {
- assertThat(ElasticVersion.V5_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
@@ -101,7 +97,6 @@
@Test
public void version6OrLater() throws Exception {
- assertThat(ElasticVersion.V5_6.isV6OrLater()).isFalse();
assertThat(ElasticVersion.V6_2.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V6_3.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V6_4.isV6OrLater()).isTrue();
@@ -120,7 +115,6 @@
@Test
public void version7OrLater() throws Exception {
- assertThat(ElasticVersion.V5_6.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_2.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_3.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_4.isV7OrLater()).isFalse();
diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index 5e25c13..b32ac19 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -25,7 +25,6 @@
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.util.time.TimeUtil;
-import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.TestChanges;
import java.util.Date;
import java.util.TimeZone;
@@ -34,9 +33,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.Test;
-import org.junit.runner.RunWith;
-@RunWith(ConfigSuite.class)
public class CommitMessageOutputTest extends AbstractChangeNotesTest {
@Test
public void approvalsCommitFormatSimple() throws Exception {
diff --git a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
index f7f29cd..9f34377 100644
--- a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
+++ b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
@@ -25,7 +25,10 @@
import com.google.gerrit.server.CommentsUtil;
import java.sql.Timestamp;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+@RunWith(JUnit4.class)
public class ListChangeCommentsTest {
@Test
@@ -40,8 +43,6 @@
getNewChangeMessage("cm2key", "cm2", Timestamp.valueOf("2018-01-01 09:01:15"));
ChangeMessage cm3 =
getNewChangeMessage("cm3key", "cm3", Timestamp.valueOf("2018-01-01 09:01:27"));
- ChangeMessage cm4 =
- getNewChangeMessage("cm4key", "cm4", Timestamp.valueOf("2018-01-01 09:01:32"));
assertThat(c1.changeMessageId).isNull();
assertThat(c2.changeMessageId).isNull();
diff --git a/polygerrit-ui/app/.eslintrc.js b/polygerrit-ui/app/.eslintrc.js
index 92ecf2a..5e4d9b8 100644
--- a/polygerrit-ui/app/.eslintrc.js
+++ b/polygerrit-ui/app/.eslintrc.js
@@ -28,19 +28,6 @@
"browser": true,
"es6": true
},
- "globals": {
- "__dirname": false,
- "app": false,
- "page": false,
- "Polymer": false,
- "process": false,
- "require": false,
- "Gerrit": false,
- "Promise": false,
- "assert": false,
- "test": false,
- "flushAsynchronousOperations": false
- },
"rules": {
"no-confusing-arrow": "error",
"newline-per-chained-call": ["error", {"ignoreChainWithDepth": 2}],
@@ -97,7 +84,9 @@
"message": "Remove suite.only."
}
],
- "no-undef": "off",
+ // no-undef disables global variable.
+ // "globals" declares allowed global variables.
+ "no-undef": ["error"],
"no-useless-escape": "off",
"no-var": "error",
"operator-linebreak": "off",
@@ -169,11 +158,131 @@
"import/no-unused-modules": 2,
"import/no-default-export": 2
},
+
+ // List of allowed globals in all files
+ "globals": {
+ // Polygerrit global variables.
+ // You must not add anything new in this list!
+ // Instead export variables from modules
+ // TODO(dmfilippov): Remove global variables from polygerrit
+ "Auth": "readonly",
+ "EventEmitter": "readonly",
+ "FetchPromisesCache": "readonly",
+ "Gerrit": "readonly",
+ "GrAdminApi": "readonly",
+ "GrAnnotation": "readonly",
+ "GrAnnotationActionsContext": "readonly",
+ "GrAnnotationActionsInterface": "readonly",
+ "GrAttributeHelper": "readonly",
+ "GrChangeActionsInterface": "readonly",
+ "GrChangeMetadataApi": "readonly",
+ "GrChangeReplyInterface": "readonly",
+ "GrChangeViewApi": "readonly",
+ "GrCountStringFormatter": "readonly",
+ "GrDiffBuilder": "readonly",
+ "GrDiffBuilderBinary": "readonly",
+ "GrDiffBuilderImage": "readonly",
+ "GrDiffBuilderSideBySide": "readonly",
+ "GrDiffBuilderUnified": "readonly",
+ "GrDiffGroup": "readonly",
+ "GrDiffLine": "readonly",
+ "GrDisplayNameUtils": "readonly",
+ "GrDomHook": "readonly",
+ "GrDomHooksManager": "readonly",
+ "GrEditConstants": "readonly",
+ "GrEmailSuggestionsProvider": "readonly",
+ "GrEtagDecorator": "readonly",
+ "GrEventHelper": "readonly",
+ "GrFileListConstants": "readonly",
+ "GrGroupSuggestionsProvider": "readonly",
+ "GrLinkTextParser": "readonly",
+ "GrPluginActionContext": "readonly",
+ "GrPluginEndpoints": "readonly",
+ "GrPluginRestApi": "readonly",
+ "GrPopupInterface": "readonly",
+ "GrRangeNormalizer": "readonly",
+ "GrRepoApi": "readonly",
+ "GrReporting": "readonly",
+ "GrRestApiHelper": "readonly",
+ "GrReviewerSuggestionsProvider": "readonly",
+ "GrReviewerUpdatesParser": "readonly",
+ "GrSettingsApi": "readonly",
+ "GrStylesApi": "readonly",
+ "GrThemeApi": "readonly",
+ "PluginLoader": "readonly",
+ "SiteBasedCache": "readonly",
+ "util": "readonly",
+ // Global variables from 3rd party libraries.
+ // You should not add anything in this list, always try to import
+ // If import is not possible - you can extend this list
+ "Polymer": "readonly",
+ "ShadyCSS": "readonly",
+ "linkify": "readonly",
+ "moment": "readonly",
+ "page": "readonly",
+ "security": "readonly",
+ },
"overrides": [
{
"files": ["*.html", "test.js", "test-infra.js", "template_test.js"],
"rules": {
"jsdoc/require-file-overview": "off"
+ },
+ },
+ {
+ "files": ["*.html", "common-test-setup.js"],
+ // Additional global variables allowed in tests
+ "globals": {
+ // Global variables from 3rd party test libraries/frameworks.
+ // You can extend this list if you want to use other global
+ // variables from these libraries and import is not possible
+ "MockInteractions": "readonly",
+ "_": "readonly",
+ "a11ySuite": "readonly",
+ "assert": "readonly",
+ "expect": "readonly",
+ "fixture": "readonly",
+ "flush": "readonly",
+ "flushAsynchronousOperations": "readonly",
+ "setup": "readonly",
+ "sinon": "readonly",
+ "stub": "readonly",
+ "suite": "readonly",
+ "suiteSetup": "readonly",
+ "teardown": "readonly",
+ "test": "readonly",
+ // Polygerrit global variables.
+ // You must not add anything new in this list!
+ // Instead export variables from modules
+ // TODO(dmfilippov): Remove global variables from polygerrit
+ "isHidden": "readonly",
+ "mockPromise": "readonly",
+
+ }
+ },
+ {
+ "files": "import-href.js",
+ "globals": {
+ "HTMLImports": "readonly",
+ }
+ },
+ {
+ "files": ["test/functional/**/*.js", "wct.conf.js", "template_test.js"],
+ // Settings for functional tests. These scripts are node scripts.
+ // Turn off "no-undef" to allow any global variable
+ "env": {
+ "browser": false,
+ "node": true,
+ "es6": false
+ },
+ "rules": {
+ "no-undef": "off",
+ }
+ },
+ {
+ "files": "test/index.html",
+ "globals": {
+ "WCT": "readonly",
}
},
{
@@ -190,5 +299,5 @@
],
"settings": {
"html/report-bad-indent": "error"
- }
+ },
};
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js
index 7985e46..e4b50a0 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.js
@@ -46,9 +46,9 @@
this._getParentIndex =
Gerrit.PatchSetBehavior.getParentIndex;
- this._comments = comments;
- this._robotComments = robotComments;
- this._drafts = drafts;
+ this._comments = comments || {};
+ this._robotComments = robotComments || {};
+ this._drafts = drafts || {};
this._changeNum = changeNum;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index 37c3d52..222109e 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
@@ -337,8 +337,8 @@
}
do {
newIndex = newIndex + delta;
- } while (newIndex > 0 &&
- newIndex < this.stops.length - 1 &&
+ } while ((delta > 0 || newIndex > 0) &&
+ (delta < 0 || newIndex < this.stops.length - 1) &&
opt_condition && !opt_condition(this.stops[newIndex]));
newIndex = Math.max(0, Math.min(this.stops.length - 1, newIndex));
