Merge "Add theme var --iron-overlay-backdrop-background-color"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0ad6905..d126c96 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1391,9 +1391,15 @@
[[changeCleanup.abandonIfMergeable]]changeCleanup.abandonIfMergeable::
+
-Whether changes which are mergeable should be auto-abandoned.
+Whether changes which are mergeable should be auto-abandoned. When set
+to `false`, `-is:mergeable` is appended to the query used to find
+the changes to auto-abandon.
+
-By default `true`.
+By default `true`, meaning mergeable changes are auto-abandoned.
++
+If link:#index.change.indexMergeable[`index.change.indexMergeable`]
+is disabled, setting this option to `false` has no effect and it
+behaves as though it were set to `true`.
[[changeCleanup.cleanupAccountPatchReview]]changeCleanup.cleanupAccountPatchReview::
+
diff --git a/Documentation/dev-crafting-changes.txt b/Documentation/dev-crafting-changes.txt
index bf4453c..eb9dee4 100644
--- a/Documentation/dev-crafting-changes.txt
+++ b/Documentation/dev-crafting-changes.txt
@@ -230,6 +230,13 @@
* Tests for new code will greatly help your change get approved.
+[[javadoc]]
+== Javadoc
+
+ * Javadocs for new code (especially public classes and
+ public/protected methods) will greatly help your change get
+ approved.
+
[[change-size]]
== Change Size/Number of Files Touched
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index b13ae83..16929ae 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -538,7 +538,9 @@
----
$ git push origin HEAD:refs/for/master%wip
----
-Alternatively, click *WIP* from the Change screen.
+Alternatively, click *WIP* from the *More* menu on the Change screen.
+The Change screen updates with a yellow header, indicating that
+the change is in a Work-in-Progress state.
To mark the change as ready for review, append `%ready` to your push
request.
@@ -546,18 +548,12 @@
----
$ git push origin HEAD:refs/for/master%ready
----
-Alternatively, click *Ready* from the Change screen.
+Alternatively, click *Start Review* from the Change screen.
Change owners, project owners, site administrators and members of a group that
was granted "Toggle Work In Progress state" permission can mark changes as
`work-in-progress` and `ready`.
-[[wip-polygerrit]]
-In the new PolyGerrit UI, you can mark a change as WIP, by selecting *WIP* from
-the *More* menu. The Change screen updates with a yellow header, indicating that
-the change is in a Work-in-Progress state. To mark a change as ready for review,
-click *Start Review*.
-
[[private-changes]]
== Private Changes
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index 308e045..8725cee 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -192,3 +192,10 @@
of all changes in NoteDb is accurate, and so is only safe once all changes are
NoteDb primary. Otherwise, reading changes only from NoteDb might result in
inaccurate results, and writing to NoteDb would compound the problem. +
+
+== NoteDB to ReviewDB rollback
+
+In case of rollback from NoteDB to ReviewDB, all the meta refs and the
+sequence ref need to be removed.
+The [remove-notedb-refs.sh](https://gerrit.googlesource.com/gerrit/+/refs/heads/master/contrib/remove-notedb-refs.sh)
+script has been written to automate this process.
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 6ef4e20..85cdace 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -733,8 +733,8 @@
[
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -767,8 +767,8 @@
)]}'
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -791,9 +791,9 @@
.Request
----
POST /accounts/self/sshkeys HTTP/1.0
- Content-Type: plain/text
+ Content-Type: text/plain
- AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d
+ ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com
----
As response an link:#ssh-key-info[SshKeyInfo] entity is returned that
@@ -808,8 +808,8 @@
)]}'
{
"seq": 2,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 021a1bb..affe1ea 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1572,6 +1572,21 @@
link:rest-api-changes.html#change-info[ChangeInfo] will never be set.
|=============================
+[[change-index-config-info]]
+=== ChangeIndexConfigInfo
+The `ChangeIndexConfigInfo` entity contains information about Gerrit
+configuration from the link:config-gerrit.html#index.change[index.change]
+section.
+
+[options="header",cols="1,^1,5"]
+|=============================
+|Field Name ||Description
+|`index_mergeable` |not set if `false`|
+Value of the link:config-gerrit.html#index.change.indexMergeable[
+configuration parameter] that controls whether the mergeability bit is
+indexed (hence queryable using `is:mergeable`).
+|=============================
+
[[check-account-external-ids-input]]
=== CheckAccountExternalIdsInput
The `CheckAccountExternalIdsInput` entity contains input for the
@@ -1821,6 +1836,21 @@
link:config-gerrit.html#gerrit.reportBugUrl[URL to report bugs].
|=================================
+[[index-config-info]]
+=== IndexConfigInfo
+The `IndexConfigInfo` entity contains information about Gerrit
+configuration from the link:config-gerrit.html#index[index]
+section.
+
+[options="header",cols="1,^1,5"]
+|=============================
+|Field Name ||Description
+|`change` ||
+Information about the configuration from the
+link:config-gerrit.html#index.change[index.change] section as
+link:#index.change[ChangeIndexConfigInfo] entity.
+|=============================
+
[[hit-ration-info]]
=== HitRatioInfo
The `HitRatioInfo` entity contains information about the hit ratio of a
@@ -1947,6 +1977,10 @@
Information about the configuration from the
link:config-gerrit.html#gerrit[gerrit] section as link:#gerrit-info[
GerritInfo] entity.
+|`index` ||
+Information about the configuration from the
+link:config-gerrit.html#index[index] section as link:#index[
+IndexConfigInfo] entity.
|`note_db_enabled` |not set if `false`|
Whether the NoteDb storage backend is fully enabled.
|`plugin` ||
diff --git a/WORKSPACE b/WORKSPACE
index 7529d8a..307e28c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -235,6 +235,31 @@
sha1 = GUAVA_BIN_SHA1,
)
+CAFFEINE_VERS = "2.8.0"
+
+maven_jar(
+ name = "caffeine",
+ artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
+ sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
+)
+
+# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
+# naming collision between caffeine guava adapater and guava library itself.
+# Remove this renaming procedure, once this upstream issue is fixed:
+# https://github.com/ben-manes/caffeine/issues/364.
+http_file(
+ name = "caffeine-guava-renamed",
+ downloaded_file_path = "caffeine-guava-" + CAFFEINE_VERS + ".jar",
+ sha256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1",
+ urls = [
+ "https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/guava/" +
+ CAFFEINE_VERS +
+ "/guava-" +
+ CAFFEINE_VERS +
+ ".jar",
+ ],
+)
+
maven_jar(
name = "guava-failureaccess",
artifact = "com.google.guava:failureaccess:1.0.1",
@@ -748,7 +773,7 @@
sha1 = "f7be08ec23c21485b9b5a1cf1654c2ec8c58168d",
)
-GITILES_VERS = "0.3-6"
+GITILES_VERS = "0.3-7"
GITILES_REPO = GERRIT
@@ -757,14 +782,14 @@
artifact = "com.google.gitiles:blame-cache:" + GITILES_VERS,
attach_source = False,
repository = GITILES_REPO,
- sha1 = "bd1ec86570b8a6e4b68c5af6311c8cd10aa3f295",
+ sha1 = "af6212a62363906c63d367f8276ae1645f83bf93",
)
maven_jar(
name = "gitiles-servlet",
artifact = "com.google.gitiles:gitiles-servlet:" + GITILES_VERS,
repository = GITILES_REPO,
- sha1 = "98bf06ca9abc871beb3d6c01e6f053243d4e911a",
+ sha1 = "6a53f722f8572a2f1bcb7d86e5692168844bab76",
)
# prettify must match the version used in Gitiles
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index af8af90..f90df67 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -269,7 +269,7 @@
private static final ImmutableMap<String, Level> LOG_LEVELS =
ImmutableMap.<String, Level>builder()
- .put("com.google.gerrit", Level.INFO)
+ .put("com.google.gerrit", getGerritLogLevel())
// Silence non-critical messages from MINA SSHD.
.put("org.apache.mina", Level.WARN)
@@ -307,6 +307,14 @@
.put("org.eclipse.jgit.util.FS", Level.WARN)
.build();
+ private static Level getGerritLogLevel() {
+ String value = Strings.nullToEmpty(System.getenv("GERRIT_LOG_LEVEL"));
+ if (value.isEmpty()) {
+ value = Strings.nullToEmpty(System.getProperty("gerrit.logLevel"));
+ }
+ return Level.toLevel(value, Level.INFO);
+ }
+
private static boolean forceLocalDisk() {
String value = Strings.nullToEmpty(System.getenv("GERRIT_FORCE_LOCAL_DISK"));
if (value.isEmpty()) {
diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
index 309ee3e5..574a226 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
@@ -30,7 +30,8 @@
V7_1("7.1.*"),
V7_2("7.2.*"),
V7_3("7.3.*"),
- V7_4("7.4.*");
+ V7_4("7.4.*"),
+ V7_5("7.5.*");
private final String version;
private final Pattern pattern;
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index 739bd38..04e97dc 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -26,6 +26,7 @@
import java.security.SecureRandom;
import java.sql.Timestamp;
import java.util.Arrays;
+import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -116,13 +117,30 @@
@AutoValue
public abstract static class Id {
- /** Parse a Change.Id out of a string representation. */
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @deprecated use {@link #tryParse(String)} instead.
+ */
+ @Deprecated
public static Id parse(String str) {
Integer id = Ints.tryParse(str);
checkArgument(id != null, "invalid change ID: %s", str);
return Change.id(id);
}
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @param str the string to parse
+ * @return Optional containing the Change.Id, or {@code Optional.empty()} if str does not
+ * represent a valid Change.Id.
+ */
+ public static Optional<Id> tryParse(String str) {
+ Integer id = Ints.tryParse(str);
+ return id != null ? Optional.of(Change.id(id)) : Optional.empty();
+ }
+
public static Id fromRef(String ref) {
if (RefNames.isRefsEdit(ref)) {
return fromEditRefPart(ref);
diff --git a/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java b/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java
new file mode 100644
index 0000000..7bca79e
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class ChangeIndexConfigInfo {
+ public Boolean indexMergeable;
+}
diff --git a/java/com/google/gerrit/extensions/common/IndexConfigInfo.java b/java/com/google/gerrit/extensions/common/IndexConfigInfo.java
new file mode 100644
index 0000000..084c53a
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/IndexConfigInfo.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class IndexConfigInfo {
+ public ChangeIndexConfigInfo change;
+}
diff --git a/java/com/google/gerrit/extensions/common/ServerInfo.java b/java/com/google/gerrit/extensions/common/ServerInfo.java
index 82d5bc8..9cf1ec1 100644
--- a/java/com/google/gerrit/extensions/common/ServerInfo.java
+++ b/java/com/google/gerrit/extensions/common/ServerInfo.java
@@ -20,6 +20,7 @@
public ChangeConfigInfo change;
public DownloadInfo download;
public GerritInfo gerrit;
+ public IndexConfigInfo index;
public Boolean noteDbEnabled;
public PluginConfigInfo plugin;
public SshdInfo sshd;
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
index 89ad878..77c5381 100644
--- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -24,6 +24,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -46,17 +47,15 @@
if (idString.endsWith("/")) {
idString = idString.substring(0, idString.length() - 1);
}
- Change.Id id;
- try {
- id = Change.Id.parse(idString);
- } catch (IllegalArgumentException e) {
+ Optional<Change.Id> id = Change.Id.tryParse(idString);
+ if (!id.isPresent()) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
ChangeResource changeResource;
try {
- changeResource = changesCollection.parse(id);
+ changeResource = changesCollection.parse(id.get());
} catch (ResourceConflictException | ResourceNotFoundException e) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
diff --git a/java/com/google/gerrit/pgm/init/AccountsOnInit.java b/java/com/google/gerrit/pgm/init/AccountsOnInit.java
index 4ffe942..536ddcd 100644
--- a/java/com/google/gerrit/pgm/init/AccountsOnInit.java
+++ b/java/com/google/gerrit/pgm/init/AccountsOnInit.java
@@ -14,9 +14,8 @@
package com.google.gerrit.pgm.init;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Objects.requireNonNull;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
@@ -129,9 +128,10 @@
private File getPath() {
Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
- checkArgument(basePath != null, "gerrit.basePath must be configured");
- File file = FileKey.resolve(basePath.resolve(allUsers).toFile(), FS.DETECTED);
- checkState(file != null, "%s does not exist", file.getAbsolutePath());
- return file;
+ requireNonNull(basePath, "gerrit.basePath must be configured");
+ File file = basePath.resolve(allUsers).toFile();
+ File resolvedFile = FileKey.resolve(file, FS.DETECTED);
+ requireNonNull(resolvedFile, () -> String.format("%s does not exist", file.getAbsolutePath()));
+ return resolvedFile;
}
}
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 979ab6f..45d037a 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -112,6 +112,7 @@
"//lib/commons:lang",
"//lib/commons:net",
"//lib/commons:validator",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 988d871..a41a36c 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -516,12 +516,22 @@
* @throws IOException if an error occurs.
*/
public Result resolve(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate());
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
public Result resolve(String input, Predicate<AccountState> accountActivityPredicate)
throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate);
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate);
+ }
+
+ public Result resolveIgnoreVisibility(String input) throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate());
+ }
+
+ public Result resolveIgnoreVisibility(
+ String input, Predicate<AccountState> accountActivityPredicate)
+ throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate);
}
/**
@@ -550,13 +560,23 @@
@Deprecated
public Result resolveByNameOrEmail(String input) throws ConfigInvalidException, IOException {
return searchImpl(
- input, nameOrEmailSearchers, visibilitySupplier(), accountActivityPredicate());
+ input, nameOrEmailSearchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
- private Supplier<Predicate<AccountState>> visibilitySupplier() {
+ private Supplier<Predicate<AccountState>> visibilitySupplierCanSee() {
return () -> accountControlFactory.get()::canSee;
}
+ private Supplier<Predicate<AccountState>> visibilitySupplierAll() {
+ return () -> all();
+ }
+
+ private Predicate<AccountState> all() {
+ return accountState -> {
+ return true;
+ };
+ }
+
private Predicate<AccountState> accountActivityPredicate() {
return (AccountState accountState) -> accountState.account().isActive();
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
index 25420ee..8887e06 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -71,6 +71,7 @@
private final Counter1<Boolean> reloadCounter;
private final Timer0 reloadDifferential;
private final boolean enablePartialReloads;
+ private final boolean isPersistentCache;
@Inject
ExternalIdCacheLoader(
@@ -101,6 +102,8 @@
.setUnit(Units.MILLISECONDS));
this.enablePartialReloads =
config.getBoolean("cache", ExternalIdCacheImpl.CACHE_NAME, "enablePartialReloads", true);
+ this.isPersistentCache =
+ config.getInt("cache", ExternalIdCacheImpl.CACHE_NAME, "diskLimit", 0) > 0;
}
@Override
@@ -156,8 +159,11 @@
}
}
if (oldExternalIds == null) {
- logger.atWarning().log(
- "Unable to find an old ExternalId cache state, falling back to full reload");
+ if (isPersistentCache) {
+ // If there is no persistence, this is normal. Don't upset admins reading the logs.
+ logger.atWarning().log(
+ "Unable to find an old ExternalId cache state, falling back to full reload");
+ }
return reloadAllExternalIds(notesRev);
}
diff --git a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
index 4d41ed7..afd159c 100644
--- a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
+++ b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.config;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.config.ScheduleConfig.Schedule;
import com.google.inject.Inject;
@@ -25,6 +26,8 @@
@Singleton
public class ChangeCleanupConfig {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private static String SECTION = "changeCleanup";
private static String KEY_ABANDON_AFTER = "abandonAfter";
private static String KEY_ABANDON_IF_MERGEABLE = "abandonIfMergeable";
@@ -48,12 +51,26 @@
this.urlFormatter = urlFormatter;
schedule = ScheduleConfig.createSchedule(cfg, SECTION);
abandonAfter = readAbandonAfter(cfg);
- abandonIfMergeable = cfg.getBoolean(SECTION, null, KEY_ABANDON_IF_MERGEABLE, true);
+ boolean indexMergeable = cfg.getBoolean("index", "change", "indexMergeable", true);
+ if (!indexMergeable) {
+ if (!readAbandonIfMergeable(cfg)) {
+ logger.atWarning().log(
+ "index.change.indexMergeable is disabled; %s.%s=false will be ineffective",
+ SECTION, KEY_ABANDON_IF_MERGEABLE);
+ }
+ abandonIfMergeable = true;
+ } else {
+ abandonIfMergeable = readAbandonIfMergeable(cfg);
+ }
cleanupAccountPatchReview =
cfg.getBoolean(SECTION, null, KEY_CLEANUP_ACCOUNT_PATCH_REVIEW, false);
abandonMessage = readAbandonMessage(cfg);
}
+ private boolean readAbandonIfMergeable(Config cfg) {
+ return cfg.getBoolean(SECTION, null, KEY_ABANDON_IF_MERGEABLE, true);
+ }
+
private long readAbandonAfter(Config cfg) {
long abandonAfter =
ConfigUtil.getTimeUnit(cfg, SECTION, null, KEY_ABANDON_AFTER, 0, TimeUnit.MILLISECONDS);
diff --git a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
index 8421e54..f7472b9 100644
--- a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
+++ b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
@@ -15,15 +15,17 @@
package com.google.gerrit.server.git;
import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import java.io.IOException;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -75,10 +77,9 @@
return null;
}
- Map<String, Ref> result;
+ Collection<Ref> result;
try {
- result =
- forProject.filter(ImmutableMap.of(name, ref), getDelegate(), RefFilterOptions.defaults());
+ result = forProject.filter(ImmutableList.of(ref), getDelegate(), RefFilterOptions.defaults());
} catch (PermissionBackendException e) {
if (e.getCause() instanceof IOException) {
throw (IOException) e.getCause();
@@ -91,7 +92,7 @@
Preconditions.checkState(
result.size() == 1, "Only one element expected, but was: " + result.size());
- return Iterables.getOnlyElement(result.values());
+ return Iterables.getOnlyElement(result);
}
@SuppressWarnings("deprecation")
@@ -102,13 +103,13 @@
return refs;
}
- Map<String, Ref> result;
+ Collection<Ref> result;
try {
- result = forProject.filter(refs, getDelegate(), RefFilterOptions.defaults());
+ result = forProject.filter(refs.values(), getDelegate(), RefFilterOptions.defaults());
} catch (PermissionBackendException e) {
throw new IOException("");
}
- return result;
+ return result.stream().collect(toMap(Ref::getName, r -> r));
}
@Override
diff --git a/java/com/google/gerrit/server/index/StalenessCheckResult.java b/java/com/google/gerrit/server/index/StalenessCheckResult.java
index cd3f592..fe35e6e 100644
--- a/java/com/google/gerrit/server/index/StalenessCheckResult.java
+++ b/java/com/google/gerrit/server/index/StalenessCheckResult.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.index;
import com.google.auto.value.AutoValue;
+import com.google.errorprone.annotations.FormatMethod;
import java.util.Optional;
/** Structured result of a staleness check. */
@@ -29,6 +30,7 @@
return new AutoValue_StalenessCheckResult(true, Optional.of(reason));
}
+ @FormatMethod
public static StalenessCheckResult stale(String reason, Object... args) {
return stale(String.format(reason, args));
}
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 64d156f..d312530 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -95,9 +95,6 @@
// The version of a secondary index.
public abstract Optional<Integer> indexVersion();
- // The number of inputs to an operation, eg. Reachable.fromRefs.
- public abstract Optional<Integer> inputSize();
-
// The name of the implementation method.
public abstract Optional<String> methodName();
@@ -303,8 +300,6 @@
public abstract Builder indexVersion(int indexVersion);
- public abstract Builder inputSize(int size);
-
public abstract Builder methodName(@Nullable String methodName);
public abstract Builder multiple(boolean multiple);
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 36b3c20..d7f09d2 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -180,6 +180,7 @@
setChangeSubjectHeader();
setHeader(MailHeader.CHANGE_ID.fieldName(), "" + change.getKey().get());
setHeader(MailHeader.CHANGE_NUMBER.fieldName(), "" + change.getChangeId());
+ setHeader(MailHeader.PROJECT.fieldName(), "" + change.getProject());
setChangeUrlHeader();
setCommitIdHeader();
diff --git a/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index f928bf0..0fb5c6f 100644
--- a/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -122,7 +122,7 @@
soyContext.put("branch", branchData);
footers.add(MailHeader.PROJECT.withDelimiter() + branch.project().get());
- footers.add("Gerrit-Branch: " + branch.shortName());
+ footers.add(MailHeader.BRANCH.withDelimiter() + branch.shortName());
}
@VisibleForTesting
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 7d2fa0a..e81f7f4 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -211,12 +211,12 @@
}
}
- Set<Address> intersection = Sets.intersection(smtpRcptTo, smtpRcptToPlaintextOnly);
+ Set<Address> intersection = Sets.intersection(va.smtpRcptTo, smtpRcptToPlaintextOnly);
if (!intersection.isEmpty()) {
logger.atSevere().log("Email '%s' will be sent twice to %s", messageClass, intersection);
}
- if (!smtpRcptTo.isEmpty()) {
+ if (!va.smtpRcptTo.isEmpty()) {
// Send multipart message
logger.atFine().log(
"Sending multipart '%s' from %s to %s",
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index ce88f07..d301d34 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -51,11 +51,11 @@
private final Change change;
protected final PersonIdent serverIdent;
- protected PatchSet.Id psId;
+ @Nullable protected PatchSet.Id psId;
private ObjectId result;
- protected boolean rootOnly;
+ boolean rootOnly;
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNotes notes,
CurrentUser user,
PersonIdent serverIdent,
@@ -72,7 +72,7 @@
this.when = when;
}
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNoteUtil noteUtil,
PersonIdent serverIdent,
@Nullable ChangeNotes notes,
@@ -172,7 +172,7 @@
public abstract boolean isEmpty();
/** Wether this update can only be a root commit. */
- public boolean isRootOnly() {
+ boolean isRootOnly() {
return rootOnly;
}
@@ -256,7 +256,7 @@
protected abstract CommitBuilder applyImpl(RevWalk rw, ObjectInserter ins, ObjectId curr)
throws IOException;
- protected static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
+ static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
ObjectId getResult() {
return result;
@@ -270,7 +270,7 @@
return ins.insert(Constants.OBJ_TREE, new byte[] {});
}
- protected void verifyComment(Comment c) {
+ void verifyComment(Comment c) {
checkArgument(c.getCommitId() != null, "commit ID required for comment: %s", c);
checkArgument(
c.author.getId().equals(getAccountId()),
diff --git a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
index 5d909d0..030cfb2 100644
--- a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
@@ -91,7 +91,7 @@
executor.submit(
() -> {
try (OpenRepo allUsersRepo = OpenRepo.open(repoManager, allUsersName)) {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
allUsersRepo.flush();
BatchRefUpdate bru = allUsersRepo.repo.getRefDatabase().newBatchUpdate();
bru.setPushCertificate(pushCert);
diff --git a/java/com/google/gerrit/server/notedb/LimitExceededException.java b/java/com/google/gerrit/server/notedb/LimitExceededException.java
new file mode 100644
index 0000000..69f9241
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/LimitExceededException.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import com.google.gerrit.exceptions.StorageException;
+
+/**
+ * A write operation was rejected because a limit would be exceeded. Limits are currently imposed
+ * on:
+ *
+ * <ul>
+ * <li>The number of NoteDb updates per change.
+ * <li>The number of patch sets per change.
+ * </ul>
+ */
+public class LimitExceededException extends StorageException {
+ private static final long serialVersionUID = 1L;
+
+ LimitExceededException(String message) {
+ super(message);
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 886e02b..1b92c0e 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -64,6 +64,10 @@
* {@link #stage()}.
*/
public class NoteDbUpdateManager implements AutoCloseable {
+ private static final int MAX_UPDATES_DEFAULT = 1000;
+ /** Limits the number of patch sets that can be created. Can be overridden in the config. */
+ private static final int MAX_PATCH_SETS_DEFAULT = 1500;
+
public interface Factory {
NoteDbUpdateManager create(Project.NameKey projectName);
}
@@ -74,6 +78,7 @@
private final NoteDbMetrics metrics;
private final Project.NameKey projectName;
private final int maxUpdates;
+ private final int maxPatchSets;
private final ListMultimap<String, ChangeUpdate> changeUpdates;
private final ListMultimap<String, ChangeDraftUpdate> draftUpdates;
private final ListMultimap<String, RobotCommentUpdate> robotCommentUpdates;
@@ -103,7 +108,8 @@
this.metrics = metrics;
this.updateAllUsersAsync = updateAllUsersAsync;
this.projectName = projectName;
- maxUpdates = cfg.getInt("change", null, "maxUpdates", 1000);
+ maxUpdates = cfg.getInt("change", null, "maxUpdates", MAX_UPDATES_DEFAULT);
+ maxPatchSets = cfg.getInt("change", null, "maxPatchSets", MAX_PATCH_SETS_DEFAULT);
changeUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
draftUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
robotCommentUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
@@ -351,17 +357,17 @@
}
private void addCommands() throws IOException {
- changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates));
+ changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates), Optional.of(maxPatchSets));
if (!draftUpdates.isEmpty()) {
boolean publishOnly = draftUpdates.values().stream().allMatch(ChangeDraftUpdate::canRunAsync);
if (publishOnly) {
updateAllUsersAsync.setDraftUpdates(draftUpdates);
} else {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
}
}
if (!robotCommentUpdates.isEmpty()) {
- changeRepo.addUpdates(robotCommentUpdates);
+ changeRepo.addUpdatesNoLimits(robotCommentUpdates);
}
if (!rewriters.isEmpty()) {
addRewrites(rewriters, changeRepo);
@@ -375,17 +381,16 @@
private void doDelete(Change.Id id) throws IOException {
String metaRef = RefNames.changeMetaRef(id);
Optional<ObjectId> old = changeRepo.cmds.get(metaRef);
- if (old.isPresent()) {
- changeRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), metaRef));
- }
+ old.ifPresent(
+ objectId -> changeRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), metaRef)));
// Just scan repo for ref names, but get "old" values from cmds.
for (Ref r :
allUsersRepo.repo.getRefDatabase().getRefsByPrefix(RefNames.refsDraftCommentsPrefix(id))) {
old = allUsersRepo.cmds.get(r.getName());
- if (old.isPresent()) {
- allUsersRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), r.getName()));
- }
+ old.ifPresent(
+ objectId ->
+ allUsersRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), r.getName())));
}
}
diff --git a/java/com/google/gerrit/server/notedb/OpenRepo.java b/java/com/google/gerrit/server/notedb/OpenRepo.java
index de88684..351f31d 100644
--- a/java/com/google/gerrit/server/notedb/OpenRepo.java
+++ b/java/com/google/gerrit/server/notedb/OpenRepo.java
@@ -43,6 +43,15 @@
* objects that are jointly closed when invoking {@link #close}.
*/
class OpenRepo implements AutoCloseable {
+ final Repository repo;
+ final RevWalk rw;
+ final ChainedReceiveCommands cmds;
+ final ObjectInserter tempIns;
+
+ private final InMemoryInserter inMemIns;
+ @Nullable private final ObjectInserter finalIns;
+ private final boolean close;
+
/** Returns a {@link OpenRepo} wrapping around an open {@link Repository}. */
static OpenRepo open(GitRepositoryManager repoManager, Project.NameKey project)
throws IOException {
@@ -60,15 +69,6 @@
}
}
- final Repository repo;
- final RevWalk rw;
- final ChainedReceiveCommands cmds;
- final ObjectInserter tempIns;
-
- private final InMemoryInserter inMemIns;
- @Nullable private final ObjectInserter finalIns;
- private final boolean close;
-
OpenRepo(
Repository repo,
RevWalk rw,
@@ -125,12 +125,15 @@
return updates.iterator().next().allowWriteToNewRef();
}
- <U extends AbstractChangeUpdate> void addUpdates(ListMultimap<String, U> all) throws IOException {
- addUpdates(all, Optional.empty());
+ <U extends AbstractChangeUpdate> void addUpdatesNoLimits(ListMultimap<String, U> all)
+ throws IOException {
+ addUpdates(
+ all, Optional.empty() /* unlimited updates */, Optional.empty() /* unlimited patch sets */);
}
<U extends AbstractChangeUpdate> void addUpdates(
- ListMultimap<String, U> all, Optional<Integer> maxUpdates) throws IOException {
+ ListMultimap<String, U> all, Optional<Integer> maxUpdates, Optional<Integer> maxPatchSets)
+ throws IOException {
for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
String refName = e.getKey();
Collection<U> updates = e.getValue();
@@ -142,29 +145,43 @@
continue;
}
- int updateCount;
+ int updateCount = 0;
U first = updates.iterator().next();
if (maxUpdates.isPresent()) {
checkState(first.getNotes() != null, "expected ChangeNotes on %s", first);
updateCount = first.getNotes().getUpdateCount();
- } else {
- updateCount = 0;
}
ObjectId curr = old;
- for (U u : updates) {
- if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) {
+ for (U update : updates) {
+ if (maxPatchSets.isPresent() && update.psId != null) {
+ // Patch set IDs are assigned consecutively. Patch sets may have been deleted, but the ID
+ // is still a good estimate and an upper bound.
+ if (update.psId.get() > maxPatchSets.get()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %d may not exceed %d patch sets. To continue working on this change, "
+ + "recreate it with a new Change-Id, then abandon this one.",
+ update.getId().get(), maxPatchSets.get()));
+ }
+ }
+ if (update.isRootOnly() && !old.equals(ObjectId.zeroId())) {
throw new StorageException("Given ChangeUpdate is only allowed on initial commit");
}
- ObjectId next = u.apply(rw, tempIns, curr);
+ ObjectId next = update.apply(rw, tempIns, curr);
if (next == null) {
continue;
}
if (maxUpdates.isPresent()
&& !Objects.equals(next, curr)
&& ++updateCount > maxUpdates.get()
- && !u.bypassMaxUpdates()) {
- throw new TooManyUpdatesException(u.getId(), maxUpdates.get());
+ && !update.bypassMaxUpdates()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %s may not exceed %d updates. It may still be abandoned or submitted. To"
+ + " continue working on this change, recreate it with a new Change-Id, then"
+ + " abandon this one.",
+ update.getId(), maxUpdates.get()));
}
curr = next;
}
diff --git a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java b/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
deleted file mode 100644
index 9c6faaf..0000000
--- a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright (C) 2019 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.notedb;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.exceptions.StorageException;
-
-/**
- * Exception indicating that the change has received too many updates. Further actions apart from
- * {@code abandon} or {@code submit} are blocked.
- */
-public class TooManyUpdatesException extends StorageException {
- @VisibleForTesting
- public static String message(Change.Id id, int maxUpdates) {
- return "Change "
- + id
- + " may not exceed "
- + maxUpdates
- + " updates. It may still be abandoned or submitted. To continue working on this "
- + "change, recreate it with a new Change-Id, then abandon this one.";
- }
-
- private static final long serialVersionUID = 1L;
-
- TooManyUpdatesException(Change.Id id, int maxUpdates) {
- super(message(id, maxUpdates));
- }
-}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 0f228fe..1435c5e 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -126,7 +126,7 @@
edits = new ArrayList<>(content.getEdits());
ImmutableSet<Edit> editsDueToRebase = content.getEditsDueToRebase();
- if (isModify(content) && diffPrefs.intralineDifference) {
+ if (isModify(content) && diffPrefs.intralineDifference && isIntralineModeAllowed(b)) {
IntraLineDiff d =
patchListCache.getIntraLineDiff(
IntraLineDiffKey.create(a.id, b.id, diffPrefs.ignoreWhitespace),
@@ -260,6 +260,16 @@
}
}
+ private static boolean isIntralineModeAllowed(Side side) {
+ // The intraline diff cache keys are the same for these cases. It's better to not show
+ // intraline results than showing completely wrong diffs or to run into a server error.
+ return !Patch.isMagic(side.path) && !isSubmoduleCommit(side.mode);
+ }
+
+ private static boolean isSubmoduleCommit(FileMode mode) {
+ return mode.getObjectType() == Constants.OBJ_COMMIT;
+ }
+
private void correctForDifferencesInNewlineAtEnd(Side a, Side b) {
// a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
int aSize = a.src.size();
@@ -272,6 +282,12 @@
return;
}
+ if (edits.isEmpty() && (aSize != bSize)) {
+ // Only edits due to rebase were present. If we now added the edits for the newlines, the
+ // code which later assembles the file contents would fail.
+ return;
+ }
+
Optional<Edit> lastEdit = getLast(edits);
if (isNewlineAtEndDeleted(a, b)) {
Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 16bbdaf..47a48b7 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -20,11 +20,10 @@
import static com.google.gerrit.entities.RefNames.REFS_CONFIG;
import static com.google.gerrit.entities.RefNames.REFS_USERS_SELF;
import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toCollection;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
@@ -60,6 +59,7 @@
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -129,7 +129,7 @@
}
/** Filters given refs and tags by visibility. */
- Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
+ Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
logger.atFinest().log(
"Filter refs for repository %s by visibility (options = %s, refs = %s)",
@@ -145,10 +145,10 @@
// See if we can get away with a single, cheap ref evaluation.
if (refs.size() == 1) {
- String refName = Iterables.getOnlyElement(refs.values()).getName();
+ String refName = Iterables.getOnlyElement(refs).getName();
if (opts.filterMeta() && isMetadata(refName)) {
logger.atFinest().log("Filter out metadata ref %s", refName);
- return ImmutableMap.of();
+ return ImmutableList.of();
}
if (RefNames.isRefsChanges(refName)) {
boolean isChangeRefVisisble = canSeeSingleChangeRef(refName);
@@ -157,18 +157,18 @@
return refs;
}
logger.atFinest().log("Filter out non-visible change ref %s", refName);
- return ImmutableMap.of();
+ return ImmutableList.of();
}
}
// Perform an initial ref filtering with all the refs the caller asked for. If we find tags that
// we have to investigate separately (deferred tags) then perform a reachability check starting
// from all visible branches (refs/heads/*).
- Result initialRefFilter = filterRefs(refs, repo, opts);
- Map<String, Ref> visibleRefs = initialRefFilter.visibleRefs();
+ Result initialRefFilter = filterRefs(new ArrayList<>(refs), repo, opts);
+ List<Ref> visibleRefs = initialRefFilter.visibleRefs();
if (!initialRefFilter.deferredTags().isEmpty()) {
try (TraceTimer traceTimer = TraceContext.newTimer("Check visibility of deferred tags")) {
- Result allVisibleBranches = filterRefs(getTaggableRefsMap(repo), repo, opts);
+ Result allVisibleBranches = filterRefs(getTaggableRefs(repo), repo, opts);
checkState(
allVisibleBranches.deferredTags().isEmpty(),
"unexpected tags found when filtering refs/heads/* "
@@ -177,12 +177,12 @@
TagMatcher tags =
tagCache
.get(projectState.getNameKey())
- .matcher(tagCache, repo, allVisibleBranches.visibleRefs().values());
+ .matcher(tagCache, repo, allVisibleBranches.visibleRefs());
for (Ref tag : initialRefFilter.deferredTags()) {
try {
if (tags.isReachable(tag)) {
logger.atFinest().log("Include reachable tag %s", tag.getName());
- visibleRefs.put(tag.getName(), tag);
+ visibleRefs.add(tag);
} else {
logger.atFinest().log("Filter out non-reachable tag %s", tag.getName());
}
@@ -202,7 +202,7 @@
* separately for later rev-walk-based visibility computation. Tags where visibility is trivial to
* compute will be returned as part of {@link Result#visibleRefs()}.
*/
- Result filterRefs(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
+ Result filterRefs(List<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
logger.atFinest().log("Filter refs (refs = %s)", refs);
@@ -252,9 +252,9 @@
identifiedUser = null;
}
- Map<String, Ref> resultRefs = new HashMap<>();
+ List<Ref> resultRefs = new ArrayList<>(refs.size());
List<Ref> deferredTags = new ArrayList<>();
- for (Ref ref : refs.values()) {
+ for (Ref ref : refs) {
String name = ref.getName();
Change.Id changeId;
Account.Id accountId;
@@ -268,7 +268,7 @@
// Edits are visible only to the owning user, if change is visible.
if (viewMetadata || visibleEdit(repo, name)) {
logger.atFinest().log("Include edit ref %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out edit ref %s", name);
}
@@ -276,7 +276,7 @@
// Change ref is visible only if the change is visible.
if (viewMetadata || visible(repo, changeId)) {
logger.atFinest().log("Include change ref %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out change ref %s", name);
}
@@ -284,7 +284,7 @@
// Account ref is visible only to the corresponding account.
if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
logger.atFinest().log("Include user ref %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out user ref %s", name);
}
@@ -296,7 +296,7 @@
&& isGroupOwner(group, identifiedUser, isAdmin)
&& canReadRef(name))) {
logger.atFinest().log("Include group ref %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out group ref %s", name);
}
@@ -312,7 +312,7 @@
// the regular Git tree that users interact with, not on any of the Gerrit trees, so this
// is a negligible risk.
logger.atFinest().log("Include tag ref %s because user has read on refs/*", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
// If its a tag, consider it later.
if (ref.getObjectId() != null) {
@@ -326,7 +326,7 @@
// Sequences are internal database implementation details.
if (viewMetadata) {
logger.atFinest().log("Include sequence ref %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out sequence ref %s", name);
}
@@ -336,7 +336,7 @@
// users.
if (viewMetadata) {
logger.atFinest().log("Include external IDs branch %s", name);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else {
logger.atFinest().log("Filter out external IDs branch %s", name);
}
@@ -346,13 +346,13 @@
// not symbolic then getLeaf() is a no-op returning ref itself.
logger.atFinest().log(
"Include ref %s because its leaf %s is readable", name, ref.getLeaf().getName());
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
} else if (isRefsUsersSelf(ref)) {
// viewMetadata allows to see all account refs, hence refs/users/self should be included as
// well
if (viewMetadata) {
logger.atFinest().log("Include ref %s", REFS_USERS_SELF);
- resultRefs.put(name, ref);
+ resultRefs.add(ref);
}
} else {
logger.atFinest().log("Filter out ref %s", name);
@@ -370,38 +370,39 @@
* <p>We exclude symbolic refs because their target will be included and this will suffice for
* computing reachability.
*/
- private static Map<String, Ref> getTaggableRefsMap(Repository repo)
- throws PermissionBackendException {
+ private static List<Ref> getTaggableRefs(Repository repo) throws PermissionBackendException {
try {
- return repo.getRefDatabase().getRefs().stream()
+ List<Ref> allRefs = repo.getRefDatabase().getRefs();
+ return allRefs.stream()
.filter(
r ->
!RefNames.isGerritRef(r.getName())
&& !r.getName().startsWith(RefNames.REFS_TAGS)
&& !r.isSymbolic())
- .collect(toMap(Ref::getName, r -> r));
+ // Don't use the default Java Collections.toList() as that is not size-aware and would
+ // expand an array list as new elements are added. Instead, provide a list that has the
+ // right size. This spares incremental list expansion which is quadratic in complexity.
+ .collect(toCollection(() -> new ArrayList<>(allRefs.size())));
} catch (IOException e) {
throw new PermissionBackendException(e);
}
}
- private Map<String, Ref> fastHideRefsMetaConfig(Map<String, Ref> refs)
- throws PermissionBackendException {
- if (refs.containsKey(REFS_CONFIG) && !canReadRef(REFS_CONFIG)) {
- Map<String, Ref> r = new HashMap<>(refs);
- r.remove(REFS_CONFIG);
- return r;
+ private List<Ref> fastHideRefsMetaConfig(List<Ref> refs) throws PermissionBackendException {
+ if (!canReadRef(REFS_CONFIG)) {
+ return refs.stream()
+ .filter(r -> !r.getName().equals(REFS_CONFIG))
+ // Don't use the default Java Collections.toList() as that is not size-aware and would
+ // expand an array list as new elements are added. Instead, provide a list that has the
+ // right size. This spares incremental list expansion which is quadratic in complexity.
+ .collect(toCollection(() -> new ArrayList<>(refs.size())));
}
return refs;
}
- private Map<String, Ref> addUsersSelfSymref(Repository repo, Map<String, Ref> refs)
+ private List<Ref> addUsersSelfSymref(Repository repo, List<Ref> refs)
throws PermissionBackendException {
if (user.isIdentifiedUser()) {
- // User self symref is already there
- if (refs.containsKey(REFS_USERS_SELF)) {
- return refs;
- }
String refName = RefNames.refsUsers(user.getAccountId());
try {
Ref r = repo.exactRef(refName);
@@ -411,8 +412,8 @@
}
SymbolicRef s = new SymbolicRef(REFS_USERS_SELF, r);
- refs = new HashMap<>(refs);
- refs.put(s.getName(), s);
+ refs = new ArrayList<>(refs);
+ refs.add(s);
logger.atFinest().log("Added %s as alias for user ref %s", REFS_USERS_SELF, refName);
} catch (IOException e) {
throw new PermissionBackendException(e);
@@ -614,7 +615,7 @@
@AutoValue
abstract static class Result {
/** Subset of the refs passed into the computation that is visible to the user. */
- abstract Map<String, Ref> visibleRefs();
+ abstract List<Ref> visibleRefs();
/**
* List of tags where we couldn't figure out visibility in the first pass and need to do an
diff --git a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
index 0800d6b..2344781 100644
--- a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.permissions.PermissionBackend.WithUser;
import com.google.gerrit.server.query.change.ChangeData;
import java.util.Collection;
-import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -142,7 +141,7 @@
}
@Override
- public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
+ public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 9149a1d..d831ab6 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.permissions;
import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
import com.google.auto.value.AutoValue;
@@ -41,7 +40,6 @@
import java.util.EnumSet;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Ref;
@@ -329,33 +327,18 @@
public abstract BooleanCondition testCond(CoreOrPluginProjectPermission perm);
/**
- * Filter a map of references by visibility.
- *
- * @param refs a map of references to filter.
- * @param repo an open {@link Repository} handle for this instance's project
- * @param opts further options for filtering.
- * @return a partition of the provided refs that are visible to the user that this instance is
- * scoped to.
- * @throws PermissionBackendException if failure consulting backend configuration.
- */
- public abstract Map<String, Ref> filter(
- Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
- throws PermissionBackendException;
-
- /**
* Filter a list of references by visibility.
*
- * @param refs a list of references to filter.
+ * @param refs a collection of references to filter.
* @param repo an open {@link Repository} handle for this instance's project
* @param opts further options for filtering.
* @return a partition of the provided refs that are visible to the user that this instance is
* scoped to.
* @throws PermissionBackendException if failure consulting backend configuration.
*/
- public Map<String, Ref> filter(List<Ref> refs, Repository repo, RefFilterOptions opts)
- throws PermissionBackendException {
- return filter(refs.stream().collect(toMap(Ref::getName, r -> r, (a, b) -> b)), repo, opts);
- }
+ public abstract Collection<Ref> filter(
+ Collection<Ref> refs, Repository repo, RefFilterOptions opts)
+ throws PermissionBackendException;
}
/** Options for filtering refs using {@link ForProject}. */
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index cc3b666..145e0b6 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -404,7 +404,7 @@
}
@Override
- public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
+ public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
if (refFilter == null) {
refFilter = refFilterFactory.create(ProjectControl.this);
diff --git a/java/com/google/gerrit/server/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java
index c30378b..57e9a7e 100644
--- a/java/com/google/gerrit/server/project/Reachable.java
+++ b/java/com/google/gerrit/server/project/Reachable.java
@@ -26,8 +26,8 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Collection;
import java.util.List;
-import java.util.Map;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -56,7 +56,7 @@
public boolean fromRefs(
Project.NameKey project, Repository repo, RevCommit commit, List<Ref> refs) {
try (RevWalk rw = new RevWalk(repo)) {
- Map<String, Ref> filtered =
+ Collection<Ref> filtered =
permissionBackend
.currentUser()
.project(project)
@@ -67,8 +67,8 @@
try (TraceTimer timer =
TraceContext.newTimer(
"IncludedInResolver.includedInAny",
- Metadata.builder().projectName(project.get()).inputSize(refs.size()).build())) {
- return IncludedInResolver.includedInAny(repo, rw, commit, filtered.values());
+ Metadata.builder().projectName(project.get()).resourceCount(refs.size()).build())) {
+ return IncludedInResolver.includedInAny(repo, rw, commit, filtered);
}
} catch (IOException | PermissionBackendException e) {
logger.atSevere().withCause(e).log(
diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java
index 1dac751..5d6379a 100644
--- a/java/com/google/gerrit/server/project/RefUtil.java
+++ b/java/com/google/gerrit/server/project/RefUtil.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -46,16 +47,15 @@
try {
ObjectId revid = repo.resolve(baseRevision);
if (revid == null) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
return revid;
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Cannot resolve \"%s\" in project \"%s\"", baseRevision, projectName.get());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
} catch (RevisionSyntaxException err) {
- logger.atSevere().withCause(err).log("Invalid revision syntax \"%s\"", baseRevision);
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
}
@@ -66,7 +66,7 @@
try {
rw.markStart(rw.parseCommit(revid));
} catch (IncorrectObjectTypeException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
RefDatabase refDb = repo.getRefDatabase();
Iterable<Ref> refs =
@@ -86,11 +86,11 @@
rw.checkConnectivity();
return rw;
} catch (IncorrectObjectTypeException | MissingObjectException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Repository \"%s\" may be corrupt; suggest running git fsck", repo.getDirectory());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
}
@@ -125,8 +125,8 @@
public static final String MESSAGE = "Invalid Revision";
- InvalidRevisionException() {
- super(MESSAGE);
+ InvalidRevisionException(@Nullable String invalidRevision) {
+ super(MESSAGE + ": " + invalidRevision);
}
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 61b90f1..df5d291 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -579,7 +579,7 @@
if ("mergeable".equalsIgnoreCase(value)) {
if (!args.indexMergeable) {
- throw new QueryParseException("server does not support 'mergeable'. check configs");
+ throw new QueryParseException("'is:mergeable' operator is not supported by server");
}
return new BooleanPredicate(ChangeField.MERGEABLE);
}
@@ -1394,7 +1394,11 @@
private List<Change> parseChange(String value) throws QueryParseException {
if (PAT_LEGACY_ID.matcher(value).matches()) {
- return asChanges(args.queryProvider.get().byLegacyChangeId(Change.Id.parse(value)));
+ Optional<Change.Id> id = Change.Id.tryParse(value);
+ if (!id.isPresent()) {
+ throw error("Invalid change id " + value);
+ }
+ return asChanges(args.queryProvider.get().byLegacyChangeId(id.get()));
} else if (PAT_CHANGE_ID.matcher(value).matches()) {
List<Change> changes = asChanges(args.queryProvider.get().byKeyPrefix(parseChangeId(value)));
if (changes.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 94c379b..b44bb29 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -95,11 +95,12 @@
String.format("change is %s.", ChangeUtil.status(changeResource.getChange())));
}
- String submissionId =
- requireNonNull(
- changeResource.getChange().getSubmissionId(),
- String.format("merged change %s has no submission ID", changeResource.getId()));
-
+ String submissionId = changeResource.getChange().getSubmissionId();
+ if (submissionId == null) {
+ throw new ResourceConflictException(
+ "This change is merged but doesn't have a submission id,"
+ + " meaning it was not submitted through Gerrit.");
+ }
List<ChangeData> changeDatas = queryProvider.get().bySubmissionId(submissionId);
for (ChangeData changeData : changeDatas) {
diff --git a/java/com/google/gerrit/server/restapi/change/Reviewers.java b/java/com/google/gerrit/server/restapi/change/Reviewers.java
index b2714da..d702142 100644
--- a/java/com/google/gerrit/server/restapi/change/Reviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/Reviewers.java
@@ -21,12 +21,11 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.restapi.account.AccountsCollection;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -37,22 +36,22 @@
public class Reviewers implements ChildCollection<ChangeResource, ReviewerResource> {
private final DynamicMap<RestView<ReviewerResource>> views;
private final ApprovalsUtil approvalsUtil;
- private final AccountsCollection accounts;
private final ReviewerResource.Factory resourceFactory;
private final ListReviewers list;
+ private final AccountResolver accountResolver;
@Inject
Reviewers(
ApprovalsUtil approvalsUtil,
- AccountsCollection accounts,
ReviewerResource.Factory resourceFactory,
DynamicMap<RestView<ReviewerResource>> views,
- ListReviewers list) {
+ ListReviewers list,
+ AccountResolver accountResolver) {
this.approvalsUtil = approvalsUtil;
- this.accounts = accounts;
this.resourceFactory = resourceFactory;
this.views = views;
this.list = list;
+ this.accountResolver = accountResolver;
}
@Override
@@ -68,22 +67,18 @@
@Override
public ReviewerResource parse(ChangeResource rsrc, IdString id)
throws ResourceNotFoundException, AuthException, IOException, ConfigInvalidException {
- Address address = Address.tryParse(id.get());
-
- Account.Id accountId = null;
try {
- accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
- } catch (ResourceNotFoundException e) {
- if (address == null) {
- throw e;
+
+ AccountResolver.Result result = accountResolver.resolveIgnoreVisibility(id.get());
+ if (fetchAccountIds(rsrc).contains(result.asUniqueUser().getAccountId())) {
+ return resourceFactory.create(rsrc, result.asUniqueUser().getAccountId());
+ }
+ } catch (AccountResolver.UnresolvableAccountException e) {
+ if (e.isSelf()) {
+ throw new AuthException(e.getMessage(), e);
}
}
- // See if the id exists as a reviewer for this change
- if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
- return resourceFactory.create(rsrc, accountId);
- }
-
- // See if the address exists as a reviewer on the change
+ Address address = Address.tryParse(id.get());
if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
return new ReviewerResource(rsrc, address);
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
index 2d504c7..2d4bfe6 100644
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -23,9 +23,11 @@
import com.google.gerrit.extensions.common.AccountsInfo;
import com.google.gerrit.extensions.common.AuthInfo;
import com.google.gerrit.extensions.common.ChangeConfigInfo;
+import com.google.gerrit.extensions.common.ChangeIndexConfigInfo;
import com.google.gerrit.extensions.common.DownloadInfo;
import com.google.gerrit.extensions.common.DownloadSchemeInfo;
import com.google.gerrit.extensions.common.GerritInfo;
+import com.google.gerrit.extensions.common.IndexConfigInfo;
import com.google.gerrit.extensions.common.PluginConfigInfo;
import com.google.gerrit.extensions.common.ReceiveInfo;
import com.google.gerrit.extensions.common.ServerInfo;
@@ -141,6 +143,7 @@
info.change = getChangeInfo();
info.download = getDownloadInfo();
info.gerrit = getGerritInfo();
+ info.index = getIndexInfo();
info.noteDbEnabled = true;
info.plugin = getPluginInfo();
info.defaultTheme = getDefaultTheme();
@@ -296,6 +299,14 @@
return info;
}
+ private IndexConfigInfo getIndexInfo() {
+ ChangeIndexConfigInfo change = new ChangeIndexConfigInfo();
+ change.indexMergeable = toBoolean(config.getBoolean("index", "change", "indexMergeable", true));
+ IndexConfigInfo index = new IndexConfigInfo();
+ index.change = change;
+ return index;
+ }
+
private String getDocUrl() {
String docUrl = config.getString("gerrit", null, "docUrl");
if (Strings.isNullOrEmpty(docUrl)) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index 56948c1..67213c5 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.entities.RefNames.isConfigRef;
+import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
@@ -93,7 +94,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
while (ref.startsWith("/")) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateTag.java b/java/com/google/gerrit/server/restapi/project/CreateTag.java
index dca6e9a..8fdf5e4 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateTag.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java
@@ -88,7 +88,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListTags.java b/java/com/google/gerrit/server/restapi/project/ListTags.java
index 36cc1ac..8cea7f5 100644
--- a/java/com/google/gerrit/server/restapi/project/ListTags.java
+++ b/java/com/google/gerrit/server/restapi/project/ListTags.java
@@ -41,8 +41,8 @@
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.List;
-import java.util.Map;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
@@ -127,10 +127,10 @@
permissionBackend.currentUser().project(resource.getNameKey());
try (Repository repo = getRepository(resource.getNameKey());
RevWalk rw = new RevWalk(repo)) {
- Map<String, Ref> all =
+ Collection<Ref> all =
visibleTags(
resource.getNameKey(), repo, repo.getRefDatabase().getRefsByPrefix(Constants.R_TAGS));
- for (Ref ref : all.values()) {
+ for (Ref ref : all) {
tags.add(
createTagInfo(perm.ref(ref.getName()), ref, rw, resource.getProjectState(), links));
}
@@ -223,7 +223,7 @@
}
}
- private Map<String, Ref> visibleTags(Project.NameKey project, Repository repo, List<Ref> tags)
+ private Collection<Ref> visibleTags(Project.NameKey project, Repository repo, List<Ref> tags)
throws PermissionBackendException {
return permissionBackend
.currentUser()
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index e240f6a..1e86d52 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -52,8 +52,8 @@
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.LimitExceededException;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -181,7 +181,7 @@
private static void wrapAndThrowException(Exception e) throws UpdateException, RestApiException {
// Convert common non-REST exception types with user-visible messages to corresponding REST
// exception types.
- if (e instanceof InvalidChangeOperationException || e instanceof TooManyUpdatesException) {
+ if (e instanceof InvalidChangeOperationException || e instanceof LimitExceededException) {
throw new ResourceConflictException(e.getMessage(), e);
} else if (e instanceof NoSuchChangeException
|| e instanceof NoSuchRefException
diff --git a/java/com/google/gerrit/sshd/commands/LsUserRefs.java b/java/com/google/gerrit/sshd/commands/LsUserRefs.java
index 8946688..b4a2b42 100644
--- a/java/com/google/gerrit/sshd/commands/LsUserRefs.java
+++ b/java/com/google/gerrit/sshd/commands/LsUserRefs.java
@@ -35,7 +35,7 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
import java.io.IOException;
-import java.util.Map;
+import java.util.Collection;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Ref;
@@ -89,14 +89,14 @@
try (Repository repo = repoManager.openRepository(projectName);
ManualRequestContext ctx = requestContext.openAs(userAccountId)) {
try {
- Map<String, Ref> refsMap =
+ Collection<Ref> refsMap =
permissionBackend
.user(ctx.getUser())
.project(projectName)
.filter(repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults());
- for (String ref : refsMap.keySet()) {
- if (!onlyRefsHeads || ref.startsWith(RefNames.REFS_HEADS)) {
+ for (Ref ref : refsMap) {
+ if (!onlyRefsHeads || ref.getName().startsWith(RefNames.REFS_HEADS)) {
stdout.println(ref);
}
}
diff --git a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index e0c4624..df1e3ed 100644
--- a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -275,7 +275,7 @@
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
for (String sshKey : sshKeys) {
SshKeyInput in = new SshKeyInput();
- in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "plain/text");
+ in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "text/plain");
addSshKey.apply(rsrc, in);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index b375f22..bc441ba 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -1478,11 +1478,9 @@
public void refsUsersSelfIsAdvertised() throws Exception {
try (Repository allUsersRepo = repoManager.openRepository(allUsers)) {
assertThat(
- permissionBackend
- .currentUser()
- .project(allUsers)
- .filter(ImmutableList.of(), allUsersRepo, RefFilterOptions.defaults())
- .keySet())
+ permissionBackend.currentUser().project(allUsers)
+ .filter(ImmutableList.of(), allUsersRepo, RefFilterOptions.defaults()).stream()
+ .map(Ref::getName))
.containsExactly(RefNames.REFS_USERS_SELF);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index 0090ed1..7ecbe69 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.stream.Collectors.toList;
+import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -32,9 +33,11 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.AbandonUtil;
@@ -45,6 +48,7 @@
import java.util.List;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class AbandonIT extends AbstractDaemonTest {
@@ -136,6 +140,93 @@
}
@Test
+ @UseClockStep
+ @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
+ @GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void notAbandonedIfMergeableWhenMergeableOperatorIsEnabled() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+
+ // create 2 changes
+ int id1 = createChange().getChange().getId().get();
+ int id2 = createChange().getChange().getId().get();
+
+ // create 2 changes that conflict with each other
+ testRepo.reset(initial);
+ int id3 = createChange("change 3", "file.txt", "content").getChange().getId().get();
+ testRepo.reset(initial);
+ int id4 = createChange("change 4", "file.txt", "other content").getChange().getId().get();
+
+ // make all 4 previously created changes older than 1 week
+ TestTimeUtil.incrementClock(7 * 24, HOURS);
+
+ // create 1 new change that will not be abandoned because it is not older than 1 week
+ testRepo.reset(initial);
+ ChangeData cd = createChange().getChange();
+ int id5 = cd.getId().get();
+
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id1, id2, id3, id4, id5);
+ assertThat(query("is:abandoned")).isEmpty();
+
+ // submit one of the conflicting changes
+ gApi.changes().id(id3).current().review(ReviewInput.approve());
+ gApi.changes().id(id3).current().submit();
+ assertThat(toChangeNumbers(query("is:merged"))).containsExactly(id3);
+ assertThat(toChangeNumbers(query("-is:mergeable"))).containsExactly(id4);
+
+ abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5, id2, id1);
+ assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4);
+ }
+
+ /**
+ * When indexMergeable is disabled then the abandonIfMergeable option is ineffective and the auto
+ * abandon behaves as though it were set to its default value (true).
+ */
+ @Test
+ @UseClockStep
+ @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
+ @GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ public void abandonedIfMergeableWhenMergeableOperatorIsDisabled() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+
+ // create 2 changes
+ int id1 = createChange().getChange().getId().get();
+ int id2 = createChange().getChange().getId().get();
+
+ // create 2 changes that conflict with each other
+ testRepo.reset(initial);
+ int id3 = createChange("change 3", "file.txt", "content").getChange().getId().get();
+ testRepo.reset(initial);
+ int id4 = createChange("change 4", "file.txt", "other content").getChange().getId().get();
+
+ // make all 4 previously created changes older than 1 week
+ TestTimeUtil.incrementClock(7 * 24, HOURS);
+
+ // create 1 new change that will not be abandoned because it is not older than 1 week
+ testRepo.reset(initial);
+ ChangeData cd = createChange().getChange();
+ int id5 = cd.getId().get();
+
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id1, id2, id3, id4, id5);
+ assertThat(query("is:abandoned")).isEmpty();
+
+ // submit one of the conflicting changes
+ gApi.changes().id(id3).current().review(ReviewInput.approve());
+ gApi.changes().id(id3).current().submit();
+ assertThat(toChangeNumbers(query("is:merged"))).containsExactly(id3);
+
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> query("-is:mergeable"));
+ assertThat(thrown).hasMessageThat().contains("operator is not supported");
+
+ abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5);
+ assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4, id2, id1);
+ }
+
+ @Test
public void changeCleanupConfigDefaultAbandonMessage() throws Exception {
assertThat(cleanupConfig.getAbandonMessage())
.startsWith(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 477ec38..74f753d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2204,7 +2204,7 @@
gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) 2));
requestScopeOperations.setApiUser(user.id());
gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.dislike());
@@ -2212,7 +2212,26 @@
m = gApi.changes().id(r.getChangeId()).reviewer(user.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) -1));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) -1));
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "NONE")
+ public void listVotesEvenWhenAccountsAreNotVisible() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+
+ requestScopeOperations.setApiUser(user.id());
+
+ // check finding by address works
+ Map<String, Short> m = gApi.changes().id(r.getChangeId()).reviewer(admin.email()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+
+ // check finding by id works
+ m = gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 24ac209..d4a4c45 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
+import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.FileInfoSubject.assertThat;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
@@ -41,6 +42,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
@@ -401,6 +403,24 @@
}
@Test
+ public void diffBetweenPatchSetsOfMergeCommitCanBeRetrievedForCommitMessageAndMergeList()
+ throws Exception {
+ PushOneCommit.Result result = createMergeCommitChange("refs/for/master", "my_file.txt");
+ String changeId = result.getChangeId();
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ addModifiedPatchSet(changeId, "my_file.txt", content -> content.concat("Line I\nLine II\n"));
+
+ // Call both of them in succession to ensure that they don't share the same cache keys.
+ DiffInfo commitMessageDiffInfo =
+ getDiffRequest(changeId, CURRENT, COMMIT_MSG).withBase(previousPatchSetId).get();
+ DiffInfo mergeListDiffInfo =
+ getDiffRequest(changeId, CURRENT, MERGE_LIST).withBase(previousPatchSetId).get();
+
+ assertThat(commitMessageDiffInfo).content().hasSize(3);
+ assertThat(mergeListDiffInfo).content().hasSize(1);
+ }
+
+ @Test
public void diffOfUnmodifiedFileMarksAllLinesAsCommon() throws Exception {
String filePath = "a_new_file.txt";
String fileContent = "Line 1\nLine 2\nLine 3\n";
@@ -578,6 +598,109 @@
}
@Test
+ public void addedNewlineAtEndOfFileIsMarkedInDiffWhenOtherwiseOnlyEditsDueToRebaseExist()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70 modification
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(102);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void addedNewlineAtEndOfFileIsNotIdentifiedAsDueToRebaseEvenThoughItShould()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ // Add a comment so that file contents are not 'skipped'. To be able to add a comment, touch
+ // (= modify) the file in the change.
+ addModifiedPatchSet(
+ changeId, FILE_NAME, fileContent -> fileContent.replace("Line 2\n", "Line two\n"));
+ CommentInput comment = createCommentInput(3, 0, 4, 0, "Comment to not skip file content.");
+ addCommentTo(changeId, CURRENT, FILE_NAME, comment);
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfA().isNull();
+ assertThat(diffInfo).content().element(1).linesOfB().containsExactly("");
+ // This should actually be isDueToRebase().
+ assertThat(diffInfo).content().element(1).isNotDueToRebase();
+ }
+
+ @Test
+ public void
+ addedNewlineAtEndOfFileIsMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceConsidered()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70.5 insertion
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(103);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void
+ addedNewlineAtEndOfFileIsNotMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceIgnoredEvenThoughItShould()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
+ }
+
+ @Test
public void addedLastLineWithoutNewlineBeforeAndAfterwardsIsMarkedInDiff() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
@@ -2374,9 +2497,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2395,6 +2516,112 @@
}
@Test
+ public void
+ diffOfFileWithOnlyRebaseHunksAndWithCommentAndConsideringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void diffOfFileWithOnlyRebaseHunksAndWithCommentAndIgnoringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\nLine 102\nLine 103\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.concat("Line 101\nLine 103\nLine 104");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
public void diffOfNonExistentFileIsAnEmptyDiffResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
@@ -2432,9 +2659,7 @@
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(20, 0, 21, 0, "Should be 'Line 20'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2476,9 +2701,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
String newFilePath = "a_new_file.txt";
gApi.changes().id(changeId).edit().renameFile(FILE_NAME, newFilePath);
gApi.changes().id(changeId).edit().publish();
@@ -2507,6 +2730,14 @@
return comment;
}
+ private void addCommentTo(
+ String changeId, String previousPatchSetId, String fileName, CommentInput comment)
+ throws RestApiException {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.comments = ImmutableMap.of(fileName, ImmutableList.of(comment));
+ gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ }
+
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index d1d197b..9e44753 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance.git;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.TruthJUnit.assume;
@@ -24,10 +25,8 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.stream.Collectors.toList;
-import static java.util.stream.Collectors.toMap;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -59,12 +58,10 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.List;
-import java.util.Map;
-import java.util.function.Function;
import java.util.function.Predicate;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.junit.TestRepository;
@@ -1365,15 +1362,18 @@
expectedAllRefs.addAll(expectedMetaRefs);
try (Repository repo = repoManager.openRepository(allUsers)) {
- Map<String, Ref> all = getAllRefs(repo);
-
PermissionBackend.ForProject forProject = newFilter(allUsers, admin);
- assertThat(forProject.filter(all, repo, RefFilterOptions.defaults()).keySet())
+ assertThat(
+ names(
+ forProject.filter(
+ repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults())))
.containsExactlyElementsIn(expectedAllRefs);
assertThat(
- forProject
- .filter(all, repo, RefFilterOptions.builder().setFilterMeta(true).build())
- .keySet())
+ names(
+ forProject.filter(
+ repo.getRefDatabase().getRefs(),
+ repo,
+ RefFilterOptions.builder().setFilterMeta(true).build())))
.containsExactlyElementsIn(expectedNonMetaRefs);
}
}
@@ -1384,8 +1384,8 @@
String patchSetRef = change.getPatchSetId().toRefName();
try (AutoCloseable ignored = disableChangeIndex();
Repository repo = repoManager.openRepository(project)) {
- Map<String, Ref> singleRef = ImmutableMap.of(patchSetRef, repo.exactRef(patchSetRef));
- Map<String, Ref> filteredRefs =
+ Collection<Ref> singleRef = ImmutableList.of(repo.exactRef(patchSetRef));
+ Collection<Ref> filteredRefs =
permissionBackend
.user(user(admin))
.project(project)
@@ -1482,8 +1482,7 @@
return AccountGroup.uuid(gApi.groups().create(groupInput).get().id);
}
- private static Map<String, Ref> getAllRefs(Repository repo) throws IOException {
- return repo.getRefDatabase().getRefs().stream()
- .collect(toMap(Ref::getName, Function.identity()));
+ private static Collection<String> names(Collection<Ref> refs) {
+ return refs.stream().map(Ref::getName).collect(toImmutableList());
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
index dcdca86..eeeacab 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -37,7 +37,7 @@
@ConfigSuite.Config
public static Config elasticsearchV7() {
- return getConfig(ElasticVersion.V7_4);
+ return getConfig(ElasticVersion.V7_5);
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
index 996119d..9573eb0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
@@ -180,6 +180,9 @@
assertThat(i.gerrit.allUsers).isEqualTo(AllUsersNameProvider.DEFAULT);
assertThat(i.gerrit.reportBugUrl).isNull();
+ // index
+ assertThat(i.index.change.indexMergeable).isNull(); // false in all tests
+
// plugin
assertThat(i.plugin.jsResourcePaths).isEmpty();
@@ -206,4 +209,11 @@
ServerInfo i = gApi.config().server().getInfo();
assertThat(i.change.excludeMergeableInChangeInfo).isTrue();
}
+
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void indexMergeableIsTrueWhenTrueInConfig() throws Exception {
+ ServerInfo i = gApi.config().server().getInfo();
+ assertThat(i.index.change.indexMergeable).isTrue();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index e5ef5ba..85d383e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -36,9 +36,11 @@
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -146,6 +148,116 @@
"Not allowed to create group branch.");
}
+ @Test
+ public void createWithRevision() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ // update master so that points to a different revision than the revision on which we create the
+ // new branch
+ pushTo("refs/heads/master");
+ assertThat(projectOperations.project(project).getHead("master")).isNotEqualTo(revision);
+
+ BranchInput input = new BranchInput();
+ input.revision = revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithoutSpecifyingRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = null;
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithEmptyRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createRevisionIsTrimmed() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "\t" + revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithFullBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "refs/heads/master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingBranchNameAsRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "refs/heads/non-existing",
+ BadRequestException.class,
+ "invalid revision \"refs/heads/non-existing\"");
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
+ BadRequestException.class,
+ "invalid revision \"deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\"");
+ }
+
+ @Test
+ public void cannotCreateWithInvalidRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "invalid\trevision",
+ BadRequestException.class,
+ "invalid revision \"invalid\trevision\"");
+ }
+
private void blockCreateReference() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
index 3d1a148..3becb81 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -41,6 +41,7 @@
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.util.List;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
@NoHttpd
@@ -357,6 +358,53 @@
assertThat(thrown).hasMessageThat().contains("Invalid base revision");
}
+ @Test
+ public void noBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = null;
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void emptyBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "";
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void baseRevisionIsTrimmed() throws Exception {
+ grantTagPermissions();
+
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "\t" + revision.name();
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(revision.name());
+ }
+
private void assertTagList(FluentIterable<String> expected, List<TagInfo> actual)
throws Exception {
assertThat(actual).hasSize(expected.size());
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 65f8aa0..ba41d7e 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -319,8 +319,6 @@
@Test
public void postCommentsUnreachableData() throws Exception {
- requestScopeOperations.setApiUser(admin.id());
-
String file = "file";
PushOneCommit push =
pushFactory.create(admin.newIdent(), testRepo, "first subject", file, "l1\nl2\n");
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
index 1c41b55..1940274 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -38,7 +38,7 @@
@ConfigSuite.Config
public static Config elasticsearchV7() {
- return getConfig(ElasticVersion.V7_4);
+ return getConfig(ElasticVersion.V7_5);
}
@Override
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 955930b..6521166 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -30,6 +30,11 @@
"//lib:jgit",
]
+HTTP_TEST_DEPS = [
+ "//lib/httpcomponents:httpasyncclient",
+ "//lib/httpcomponents:httpclient",
+]
+
QUERY_TESTS_DEP = "//javatests/com/google/gerrit/server/query/%s:abstract_query_tests"
TYPES = [
@@ -66,7 +71,7 @@
size = "large",
srcs = [src],
tags = ELASTICSEARCH_TAGS,
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + HTTP_TEST_DEPS,
) for name, src in ELASTICSEARCH_TESTS_V6.items()]
[junit_tests(
@@ -74,10 +79,7 @@
size = "large",
srcs = [src],
tags = ELASTICSEARCH_TAGS,
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + [
- "//lib/httpcomponents:httpasyncclient",
- "//lib/httpcomponents:httpclient",
- ],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + HTTP_TEST_DEPS,
) for name, src in ELASTICSEARCH_TESTS_V7.items()]
junit_tests(
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index c15f1a7..c692a3b 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -62,6 +62,8 @@
return "blacktop/elasticsearch:7.3.2";
case V7_4:
return "blacktop/elasticsearch:7.4.2";
+ case V7_5:
+ return "blacktop/elasticsearch:7.5.0";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
index da64d53..1601756 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
@@ -14,6 +14,8 @@
package com.google.gerrit.elasticsearch;
+import static java.util.concurrent.TimeUnit.MINUTES;
+
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
import com.google.gerrit.server.query.change.AbstractQueryChangesTest;
import com.google.gerrit.testing.ConfigSuite;
@@ -22,7 +24,12 @@
import com.google.gerrit.testing.IndexConfig;
import com.google.inject.Guice;
import com.google.inject.Injector;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
+import org.apache.http.impl.nio.client.HttpAsyncClients;
import org.eclipse.jgit.lib.Config;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -35,6 +42,7 @@
private static ElasticNodeInfo nodeInfo;
private static ElasticContainer container;
+ private static CloseableHttpAsyncClient client;
@BeforeClass
public static void startIndexService() {
@@ -45,6 +53,8 @@
container = ElasticContainer.createAndStart(ElasticVersion.V6_8);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
+ client = HttpAsyncClients.createDefault();
+ client.start();
}
@AfterClass
@@ -56,6 +66,19 @@
@Rule public final GerritTestName testName = new GerritTestName();
+ @After
+ public void closeIndex() throws Exception {
+ client
+ .execute(
+ new HttpPost(
+ String.format(
+ "http://localhost:%d/%s*/_close",
+ nodeInfo.port, testName.getSanitizedMethodName())),
+ HttpClientContext.create(),
+ null)
+ .get(5, MINUTES);
+ }
+
@Override
protected void initAfterLifecycleStart() throws Exception {
super.initAfterLifecycleStart();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
index 021a80d..c55417e 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
index 6bb2f8fb..b4feb32 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
@@ -51,7 +51,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
client = HttpAsyncClients.createDefault();
client.start();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
index 9312f01..c2d6246 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
index 4b9ede8..2fc4366 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
index f9cfe35..5d2f99c 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -60,6 +60,9 @@
assertThat(ElasticVersion.forVersion("7.4.0")).isEqualTo(ElasticVersion.V7_4);
assertThat(ElasticVersion.forVersion("7.4.1")).isEqualTo(ElasticVersion.V7_4);
+
+ assertThat(ElasticVersion.forVersion("7.5.0")).isEqualTo(ElasticVersion.V7_5);
+ assertThat(ElasticVersion.forVersion("7.5.1")).isEqualTo(ElasticVersion.V7_5);
}
@Test
@@ -89,6 +92,7 @@
assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
+ assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
}
@Test
@@ -106,6 +110,7 @@
assertThat(ElasticVersion.V7_2.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V7_3.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V7_4.isV6OrLater()).isTrue();
+ assertThat(ElasticVersion.V7_5.isV6OrLater()).isTrue();
}
@Test
@@ -123,5 +128,6 @@
assertThat(ElasticVersion.V7_2.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_3.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue();
+ assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue();
}
}
diff --git a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
index 2485613..de23ef4 100644
--- a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
+++ b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
@@ -36,7 +36,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import java.util.Collection;
-import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -85,7 +84,7 @@
}
@Override
- public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
+ public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
throw new UnsupportedOperationException("not implemented");
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 962b691..9c12052 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -3093,7 +3093,7 @@
assertThat(thrown.getCause()).isInstanceOf(QueryParseException.class);
assertThat(thrown)
.hasMessageThat()
- .contains("server does not support 'mergeable'. check configs");
+ .contains("'is:mergeable' operator is not supported by server");
}
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index a2f800a..5860c48 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.InMemoryTestEnvironment;
import com.google.inject.Inject;
@@ -51,6 +50,7 @@
public class BatchUpdateTest {
private static final int MAX_UPDATES = 4;
+ private static final int MAX_PATCH_SETS = 3;
@Rule
public InMemoryTestEnvironment testEnvironment =
@@ -58,6 +58,7 @@
() -> {
Config cfg = new Config();
cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
+ cfg.setInt("change", null, "maxPatchSets", MAX_PATCH_SETS);
return cfg;
});
@@ -106,11 +107,10 @@
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Excessive update"));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -118,17 +118,16 @@
@Test
public void cannotExceedMaxUpdatesCountingMultipleChangeUpdatesInSingleBatch() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Update on PS1", PatchSet.id(id, 1)));
bu.addOp(id, new AddMessageOp("Update on PS2", PatchSet.id(id, 2)));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES - 1);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -187,7 +186,7 @@
@Test
public void exceedingMaxUpdatesAllowedWithSubmitAfterOtherOp() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Message on PS1", PatchSet.id(id, 1)));
@@ -222,6 +221,28 @@
assertThat(getMetaId(id)).isNotEqualTo(oldMetaId);
}
+ @Test
+ public void limitPatchSetCount_exceed() throws Exception {
+ Change.Id changeId = createChangeWithPatchSets(MAX_PATCH_SETS);
+ ObjectId oldMetaId = getMetaId(changeId);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("kaboom").create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, MAX_PATCH_SETS + 1), commitId)
+ .setMessage("kaboom"));
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Change " + changeId + " may not exceed " + MAX_PATCH_SETS + " patch sets");
+ }
+ assertThat(changeNotesFactory.create(project, changeId).getPatchSets()).hasSize(MAX_PATCH_SETS);
+ assertThat(getMetaId(changeId)).isEqualTo(oldMetaId);
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);
@@ -243,21 +264,22 @@
return id;
}
- private Change.Id createChangeWithTwoPatchSets(int totalUpdates) throws Exception {
- Change.Id id = createChangeWithUpdates(totalUpdates - 1);
+ private Change.Id createChangeWithPatchSets(int patchSets) throws Exception {
+ checkArgument(patchSets >= 2);
+ Change.Id id = createChangeWithUpdates(MAX_UPDATES - 2);
ChangeNotes notes = changeNotesFactory.create(project, id);
-
- try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
- ObjectId commitId = repo.amend(notes.getCurrentPatchSet().commitId()).message("PS2").create();
- bu.addOp(
- id,
- patchSetInserterFactory
- .create(notes, PatchSet.id(id, 2), commitId)
- .setMessage("Add PS2"));
- bu.execute();
+ for (int i = 2; i <= patchSets; ++i) {
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("PS" + i).create();
+ bu.addOp(
+ id,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(id, i), commitId)
+ .setMessage("Add PS" + i));
+ bu.execute();
+ }
}
-
- assertThat(getUpdateCount(id)).isEqualTo(totalUpdates);
return id;
}
@@ -291,7 +313,7 @@
}
}
- private int getUpdateCount(Change.Id changeId) throws Exception {
+ private int getUpdateCount(Change.Id changeId) {
return changeNotesFactory.create(project, changeId).getUpdateCount();
}
diff --git a/lib/BUILD b/lib/BUILD
index 39c622a..199f4fb 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -528,7 +528,7 @@
"//java/com/google/gerrit/acceptance:__pkg__",
"//java/com/google/gerrit/extensions:__pkg__",
"//java/com/google/gerrit/server:__pkg__",
- "//plugins:__pkg__",
+ "//plugins:__subpackages__",
],
exports = ["@javax-annotation//jar"],
)
diff --git a/modules/jgit b/modules/jgit
index 0356613..a7e454b 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 0356613f48ebee2e3d2d65780e71d9e0b43a752e
+Subproject commit a7e454bc51d359c2d46b19fd559f770cad8fd7d4
diff --git a/package.json b/package.json
index 5ed4671..95edf0b 100644
--- a/package.json
+++ b/package.json
@@ -15,8 +15,8 @@
"scripts": {
"start": "polygerrit-ui/run-server.sh",
"test": "WCT_HEADLESS_MODE=1 WCT_ARGS='--verbose -l chrome' ./polygerrit-ui/app/run_test.sh",
- "eslint": "./node_modules/eslint/bin/eslint.js --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app || exit 0",
- "eslintfix": "./node_modules/eslint/bin/eslint.js --fix --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app || exit 0",
+ "eslint": "./node_modules/eslint/bin/eslint.js --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app",
+ "eslintfix": "npm run eslint -- --fix",
"test-template": "./polygerrit-ui/app/run_template_test.sh",
"polylint": "bazel test polygerrit-ui/app:polylint_test"
},
diff --git a/plugins/delete-project b/plugins/delete-project
index 38f4fde..39dd25c 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 38f4fde24ce24cf1bf8d1e4d074f1d784ed983b8
+Subproject commit 39dd25c822be14a69282d38e5205f0ca4f2902c1
diff --git a/plugins/gitiles b/plugins/gitiles
index 0f8de56..3531010 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 0f8de56cc0f68047e38ac62e11271dbc6d76aa29
+Subproject commit 3531010e04d9d548fe1fd93662ca85ae25d4a9a6
diff --git a/plugins/hooks b/plugins/hooks
index f4bf0ff..6316be2 160000
--- a/plugins/hooks
+++ b/plugins/hooks
@@ -1 +1 @@
-Subproject commit f4bf0ffbd13a748cc46a3368a8fadcc2cbab6e21
+Subproject commit 6316be2828808dafc546ecd11c055396d0b4951b
diff --git a/plugins/replication b/plugins/replication
index 4689b41..c72a720 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4689b419eab61ea204daf2dfca47296667ac317c
+Subproject commit c72a72058ff03d9d83b09882a9044a75467c4b1f
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 5ff820b..3a70ac53 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -55,7 +55,6 @@
exclude = [
"bower_components/**",
"**/*_test.html",
- "embed/test.html",
"test/**",
"samples/**",
],
@@ -166,33 +165,6 @@
],
) for directory in DIRECTORIES]
-# Embed bundle
-polygerrit_bundle(
- name = "polygerrit_embed_ui",
- srcs = glob(
- [
- "**/*.html",
- "**/*.js",
- ],
- exclude = [
- "bower_components/**",
- "test/**",
- "**/*_test.html",
- ],
- ),
- outs = ["polygerrit_embed_ui.zip"],
- app = "embed/embed.html",
-)
-
-filegroup(
- name = "embed_test_files",
- srcs = glob(
- [
- "embed/**/*_test.html",
- ],
- ),
-)
-
filegroup(
name = "template_test_srcs",
srcs = [
@@ -200,21 +172,3 @@
"template_test_srcs/template_test.js",
],
)
-
-sh_test(
- name = "embed_test",
- size = "small",
- srcs = ["embed_test.sh"],
- data = [
- "embed/test.html",
- "test/common-test-setup.html",
- ":embed_test_files",
- ":pg_code.zip",
- ":test_components.zip",
- ],
- # Should not run sandboxed.
- tags = [
- "local",
- "manual",
- ],
-)
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
index e6eef75..04bdab7 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
@@ -197,7 +197,10 @@
assert.equal(Polymer.dom(element.root)
.querySelectorAll('.sectionTitle').length, 3);
assert.equal(element.$$('.breadcrumbText').innerText, 'Test Repo');
- assert.equal(element.$$('#pageSelect').items.length, 6);
+ assert.equal(
+ element.shadowRoot.querySelector('#pageSelect').items.length,
+ 6
+ );
done();
});
});
@@ -439,13 +442,16 @@
element.reload().then(() => {
assert.deepEqual(element._filteredLinks, expectedFilteredLinks);
assert.deepEqual(element._subsectionLinks, expectedSubsectionLinks);
- assert.equal(element.$$('#pageSelect').value, 'repoaccess');
+ assert.equal(
+ element.shadowRoot.querySelector('#pageSelect').value,
+ 'repoaccess'
+ );
assert.isTrue(element._selectedIsCurrentPage.calledOnce);
// Doesn't trigger navigation from the page select menu.
assert.isFalse(Gerrit.Nav.navigateToRelativeUrl.called);
// When explicitly changed, navigation is called
- element.$$('#pageSelect').value = 'repo';
+ element.shadowRoot.querySelector('#pageSelect').value = 'repo';
assert.isTrue(element._selectedIsCurrentPage.calledTwice);
assert.isTrue(Gerrit.Nav.navigateToRelativeUrl.calledOnce);
done();
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index 6cb7e4a..7919b28 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -385,7 +385,8 @@
assert.isFalse(element._originalExclusiveValue);
assert.isNotOk(element.permission.value.modified);
- MockInteractions.tap(element.$$('#exclusiveToggle'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#exclusiveToggle'));
flushAsynchronousOperations();
assert.isTrue(element.permission.value.exclusive);
assert.isTrue(element.permission.value.modified);
@@ -405,21 +406,25 @@
});
test('Exclusive hidden for owner permission', () => {
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'flex');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'flex');
element.set(['permission', 'id'], 'owner');
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'none');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'none');
});
test('Exclusive hidden for any global permissions', () => {
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'flex');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'flex');
element.section = 'GLOBAL_CAPABILITIES';
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'none');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'none');
});
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
index 90eaba5..fc7dea8 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
@@ -282,7 +282,8 @@
id: 'test-project',
};
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#editInheritFromInput'))
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#editInheritFromInput'))
.display, 'none');
MockInteractions.tap(element.$.editBtn);
@@ -300,7 +301,8 @@
assert.notEqual(getComputedStyle(element.$.saveBtn).display, 'none');
assert.isTrue(element.$.saveBtn.disabled);
}
- assert.notEqual(getComputedStyle(element.$$('#editInheritFromInput'))
+ assert.notEqual(getComputedStyle(element.shadowRoot
+ .querySelector('#editInheritFromInput'))
.display, 'none');
// Save button should be enabled after access is modified
@@ -365,7 +367,7 @@
id: 'test-project',
};
flushAsynchronousOperations();
- element.$$('#editInheritFromInput').fire('commit');
+ element.shadowRoot.querySelector('#editInheritFromInput').fire('commit');
sandbox.spy(element, '_handleAccessModified');
element.fire('access-modified');
assert.isTrue(element._handleAccessModified.called);
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
index 3652e70..cbc7bcd 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
+++ b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
@@ -50,9 +50,13 @@
this.$.createOverlay.close();
}
- _pickerConfirm() {
+ _pickerConfirm(e) {
this.$.createOverlay.close();
const detail = {repo: this._repo, branch: this._branch};
+ // e is a 'confirm' event from gr-dialog. We want to fire a more detailed
+ // 'confirm' event here, so let's stop propagation of the bare event.
+ e.preventDefault();
+ e.stopPropagation();
this.dispatchEvent(new CustomEvent('confirm', {detail, bubbles: false}));
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 0817762..6703651 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -269,6 +269,25 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
+ <gr-dialog
+ id="showRevertSubmissionChangesDialog"
+ class="confirmDialog"
+ confirm-label="Close"
+ cancel-label=''
+ on-confirm="_handleShowRevertSubmissionChangesConfirm">
+ <div class="header" slot="header">
+ Reverted Changes
+ </div>
+ <div class="main" slot="main">
+ <template is="dom-repeat" items="[[_revertChanges]]">
+ <div>
+ <a href$="[[item.link]]" target="_blank">
+ Change [[item._number]]
+ </a>
+ </div>
+ </template>
+ </div>
+ </gr-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 2ce0d7d..8ad2a06 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -420,6 +420,10 @@
type: Boolean,
value: true,
},
+ _revertChanges: {
+ type: Array,
+ value: [],
+ },
};
}
@@ -1258,6 +1262,7 @@
_handleResponse(action, response) {
if (!response) { return; }
return this.$.restAPI.getResponseObject(response).then(obj => {
+ let revertChanges = [];
switch (action.__key) {
case ChangeActions.REVERT:
this._waitForChangeReachable(obj._number)
@@ -1282,6 +1287,29 @@
case ChangeActions.REBASE_EDIT:
Gerrit.Nav.navigateToChange(this.change);
break;
+ case ChangeActions.REVERT_SUBMISSION:
+ revertChanges = obj.revert_changes || [];
+ revertChanges = revertChanges.map(change => {
+ change.link = '/q/' + encodeURIComponent(change.change_id);
+ return change;
+ });
+ // list of reverted changes can never be 0
+ if (revertChanges.length === 1) {
+ // redirect to the change if only 1 change is reverted
+ const change = revertChanges[0];
+ this._waitForChangeReachable(change._number).then(success => {
+ if (success) {
+ Gerrit.Nav.navigateToChange(change);
+ } else {
+ console.error('Change ' + change._number + ' not reachable');
+ }
+ });
+ } else {
+ // show multiple reverted changes in a dialog
+ this._revertChanges = revertChanges;
+ this._showActionDialog(this.$.showRevertSubmissionChangesDialog);
+ }
+ break;
default:
this.dispatchEvent(new CustomEvent('reload-change',
{detail: {action: action.__key}, bubbles: false}));
@@ -1290,6 +1318,10 @@
});
}
+ _handleShowRevertSubmissionChangesConfirm() {
+ this._hideAllDialogs();
+ }
+
_handleResponseError(action, response, body) {
if (action && action.__key === RevisionActions.CHERRYPICK) {
if (response && response.status === 409 &&
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index a10f028..d9b7986 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -466,7 +466,9 @@
element._handleDeleteEditTap();
assert.isFalse(element.$.confirmDeleteEditDialog.hidden);
MockInteractions.tap(
- element.$$('#confirmDeleteEditDialog').$$('gr-button[primary]'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteEditDialog')
+ .$$('gr-button[primary]'));
flushAsynchronousOperations();
assert.equal(fireActionStub.lastCall.args[0], '/edit');
@@ -980,9 +982,12 @@
test('shows confirm dialog', () => {
element._handleDeleteTap();
- assert.isFalse(element.$$('#confirmDeleteDialog').hidden);
+ assert.isFalse(element.shadowRoot
+ .querySelector('#confirmDeleteDialog').hidden);
MockInteractions.tap(
- element.$$('#confirmDeleteDialog').$$('gr-button[primary]'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteDialog')
+ .$$('gr-button[primary]'));
flushAsynchronousOperations();
assert.isTrue(fireActionStub.calledWith('/', deleteAction, false));
});
@@ -990,9 +995,12 @@
test('hides delete confirm on cancel', () => {
element._handleDeleteTap();
MockInteractions.tap(
- element.$$('#confirmDeleteDialog').$$('gr-button:not([primary])'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteDialog')
+ .$$('gr-button:not([primary])'));
flushAsynchronousOperations();
- assert.isTrue(element.$$('#confirmDeleteDialog').hidden);
+ assert.isTrue(element.shadowRoot
+ .querySelector('#confirmDeleteDialog').hidden);
assert.isFalse(fireActionStub.called);
});
});
@@ -1422,6 +1430,7 @@
let payload;
let onShowError;
let onShowAlert;
+ let getResponseObjectStub;
setup(() => {
cleanup = sinon.stub();
@@ -1437,12 +1446,18 @@
suite('happy path', () => {
let sendStub;
-
+ let waitForChangeReachableStub;
setup(() => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true}));
sendStub = sandbox.stub(element.$.restAPI, 'executeChangeAction')
.returns(Promise.resolve({}));
+ getResponseObjectStub = sandbox.stub(element.$.restAPI,
+ 'getResponseObject');
+ waitForChangeReachableStub = sandbox.stub(element,
+ '_waitForChangeReachable').returns(Promise.resolve(true));
+ sandbox.stub(Gerrit.Nav,
+ 'navigateToChange').returns(Promise.resolve(true));
});
test('change action', () => {
@@ -1455,6 +1470,49 @@
});
});
+ suite('single changes revert', () => {
+ setup(() => {
+ getResponseObjectStub
+ .returns(Promise.resolve({revert_changes: [
+ {change_id: 12345},
+ ]}));
+ showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ });
+
+ test('revert submission single change', done => {
+ element._send('POST', {message: 'Revert submission'},
+ '/revert_submission', false, cleanup).then(res => {
+ element._handleResponse({__key: 'revert_submission'}, {}).
+ then(() => {
+ assert.isTrue(waitForChangeReachableStub.called);
+ done();
+ });
+ });
+ });
+ });
+
+ suite('multiple changes revert', () => {
+ let showActionDialogStub;
+ setup(() => {
+ getResponseObjectStub
+ .returns(Promise.resolve({revert_changes: [
+ {change_id: 12345}, {change_id: 23456},
+ ]}));
+ showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ });
+
+ test('revert submission multiple change', done => {
+ element._send('POST', {message: 'Revert submission'},
+ '/revert_submission', false, cleanup).then(res => {
+ element._handleResponse({__key: 'revert_submission'}, {}).then(
+ () => {
+ assert.isTrue(showActionDialogStub.called);
+ done();
+ });
+ });
+ });
+ });
+
test('revision action', () => {
return element._send('DELETE', payload, '/endpoint', true, cleanup)
.then(() => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 5de378b..b9e9e6c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -437,7 +437,8 @@
}
_handleFileTabChange(e) {
- const selectedIndex = this.$$('#primaryTabs').selected;
+ const selectedIndex = this.shadowRoot
+ .querySelector('#primaryTabs').selected;
this._showFileTabContent = selectedIndex === 0;
// Initial tab is the static files list.
const newSelectedTab =
@@ -458,8 +459,8 @@
console.warn(e.detail.tab + ' tab not found');
return;
}
- this.$$('#primaryTabs').selected = idx + 1;
- this.$$('#primaryTabs').scrollIntoView();
+ this.shadowRoot.querySelector('#primaryTabs').selected = idx + 1;
+ this.shadowRoot.querySelector('#primaryTabs').scrollIntoView();
this.$.reporting.reportInteraction('show-tab', e.detail.tab);
}
@@ -799,7 +800,7 @@
// Selected has to be set after the paper-tabs are visible because
// the selected underline depends on calculations made by the browser.
this.$.commentTabs.selected = 0;
- const primaryTabs = this.$$('#primaryTabs');
+ const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
if (primaryTabs) primaryTabs.selected = 0;
}
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
index ab849de..9329ba5 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
@@ -68,7 +68,8 @@
}
getFocusStops() {
- const links = this.$$('#archives').querySelectorAll('a');
+ const links = this.shadowRoot
+ .querySelector('#archives').querySelectorAll('a');
return {
start: this.$.closeButton,
end: links[links.length - 1],
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
index ea7ea8f..10efaff 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
@@ -216,8 +216,8 @@
test('expand/collapse buttons are toggled correctly', () => {
element.shownFileCount = 10;
flushAsynchronousOperations();
- const expandBtn = element.$$('#expandBtn');
- const collapseBtn = element.$$('#collapseBtn');
+ const expandBtn = element.shadowRoot.querySelector('#expandBtn');
+ const collapseBtn = element.shadowRoot.querySelector('#collapseBtn');
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index 9caf13d..b0747f4 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -231,7 +231,8 @@
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Expand all.
flushAsynchronousOperations();
let messages = getMessages();
@@ -255,8 +256,10 @@
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
- MockInteractions.tap(element.$$('#collapse-messages')); // Collapse all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Expand all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Collapse all.
flushAsynchronousOperations();
let messages = getMessages();
@@ -283,13 +286,15 @@
MockInteractions.tap(allMessageEls[1]);
assert.isTrue(allMessageEls[1]._expanded);
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isTrue(message._expanded);
}
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isFalse(message._expanded);
@@ -297,28 +302,33 @@
});
test('expand/collapse from external keypress', () => {
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
let allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isTrue(message._expanded);
}
// Expand/collapse all text also changes.
- assert.equal(element.$$('#collapse-messages').textContent.trim(),
- 'Collapse all');
+ assert.equal(element.shadowRoot
+ .querySelector('#collapse-messages').textContent.trim(),
+ 'Collapse all');
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isFalse(message._expanded);
}
// Expand/collapse all text also changes.
- assert.equal(element.$$('#collapse-messages').textContent.trim(),
- 'Expand all');
+ assert.equal(element.shadowRoot
+ .querySelector('#collapse-messages').textContent.trim(),
+ 'Expand all');
});
test('hide messages does not appear when no automated messages', () => {
- assert.isOk(element.$$('#automatedMessageToggleContainer[hidden]'));
+ assert.isOk(element.shadowRoot
+ .querySelector('#automatedMessageToggleContainer[hidden]'));
});
test('scroll to message', () => {
@@ -476,7 +486,8 @@
});
test('hide autogenerated button is not hidden', () => {
- assert.isNotOk(element.$$('#automatedMessageToggle[hidden]'));
+ assert.isNotOk(element.shadowRoot
+ .querySelector('#automatedMessageToggle[hidden]'));
});
test('autogenerated messages are not hidden initially', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index badd62a..e44f669 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -187,7 +187,7 @@
});
test('keep drafts with reply', done => {
- MockInteractions.tap(element.$$('#includeComments'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#includeComments'));
assert.equal(element._includeComments, false);
// Async tick is needed because iron-selector content is distributed and
@@ -460,7 +460,8 @@
flushAsynchronousOperations();
assert.isFalse(element._reviewersMutated);
assert.isTrue(element.$.ccs.allowAnyInput);
- assert.isFalse(element.$$('#reviewers').allowAnyInput);
+ assert.isFalse(element.shadowRoot
+ .querySelector('#reviewers').allowAnyInput);
element.$.ccs.dispatchEvent(new CustomEvent('account-text-changed',
{bubbles: true, composed: true}));
assert.isTrue(element._reviewersMutated);
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
index fe8384e..d29858e 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
@@ -90,7 +90,7 @@
}
gr-smart-search {
flex-grow: 1;
- margin-left: var(--spacing-m);
+ margin: 0 var(--spacing-m);
max-width: 500px;
}
gr-dropdown,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 2a57162..01e1f1f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -77,7 +77,6 @@
properties: {
diff: Object,
- diffPath: String,
changeNum: String,
patchNum: String,
viewMode: String,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index f5dc438..bdb914f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -301,7 +301,7 @@
const layers = [this.$.syntaxLayer];
// Get layers from plugins (if any).
for (const pluginLayer of this.$.jsAPI.getDiffLayers(
- this.diffPath, this.changeNum, this.patchNum)) {
+ this.path, this.changeNum, this.patchNum)) {
layers.push(pluginLayer);
}
this._layers = layers;
@@ -313,7 +313,9 @@
this._coverageRanges = [];
const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this;
- this.$.jsAPI.getCoverageRanges(changeNum, path, basePatchNum, patchNum).
+ this.$.jsAPI.getCoverageRanges(
+ changeNum, path, basePatchNum, patchNum
+ ).
then(coverageRanges => {
if (changeNum !== this.changeNum ||
path !== this.path ||
@@ -344,6 +346,8 @@
return this._loadDiffAssets(diff);
});
+ // Not waiting for getCoverageRanges intentionally as
+ // plugin loading should not block the content rendering
return Promise.all([diffRequest, assetRequest])
.then(results => {
const diff = results[0];
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 1446fd4..3b64f55 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -373,7 +373,7 @@
coverage-ranges="[[coverageRanges]]"
project-name="[[projectName]]"
diff="[[diff]]"
- diff-path="[[path]]"
+ path="[[path]]"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
view-mode="[[viewMode]]"
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
index 2d7dc9c..b7e52a1 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
@@ -82,7 +82,7 @@
});
test('open', () => {
- MockInteractions.tap(element.$$('#open'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#open'));
element.patchNum = 1;
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element._hideAllDialogs.called);
@@ -101,7 +101,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#open'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#open'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.openDialog.disabled);
openAutoCcmplete.noDebounce = true;
@@ -129,7 +129,7 @@
test('delete', () => {
deleteStub.returns(Promise.resolve({ok: true}));
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
@@ -152,7 +152,7 @@
test('delete fails', () => {
deleteStub.returns(Promise.resolve({ok: false}));
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
@@ -173,7 +173,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
element.$.deleteDialog.querySelector('gr-autocomplete').text =
@@ -204,7 +204,7 @@
test('rename', () => {
renameStub.returns(Promise.resolve({ok: true}));
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
@@ -232,7 +232,7 @@
test('rename fails', () => {
renameStub.returns(Promise.resolve({ok: false}));
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
@@ -258,7 +258,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
element.$.renameDialog.querySelector('gr-autocomplete').text =
@@ -285,13 +285,14 @@
});
test('restore hidden by default', () => {
- assert.isTrue(element.$$('#restore').classList.contains('invisible'));
+ assert.isTrue(element.shadowRoot
+ .querySelector('#restore').classList.contains('invisible'));
});
test('restore', () => {
restoreStub.returns(Promise.resolve({ok: true}));
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button[primary]'));
flushAsynchronousOperations();
@@ -309,7 +310,7 @@
test('restore fails', () => {
restoreStub.returns(Promise.resolve({ok: false}));
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button[primary]'));
flushAsynchronousOperations();
@@ -325,7 +326,7 @@
test('cancel', () => {
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button'));
assert.isFalse(navStub.called);
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
index fb5ab15..fb5d64f 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
@@ -80,10 +80,10 @@
data-name$="[[item.name]]"
data-url$="[[item.url]]"
on-click="_handleShowAgreement"
- disabled$="[[_disableAggreements(item, _groups, _signedAgreements)]]">
+ disabled$="[[_disableAgreements(item, _groups, _signedAgreements)]]">
<label id="claNewAgreementsLabel">[[item.name]]</label>
</span>
- <div class$="alreadySubmittedText [[_hideAggreements(item, _groups, _signedAgreements)]]">
+ <div class$="alreadySubmittedText [[_hideAgreements(item, _groups, _signedAgreements)]]">
Agreement already submitted.
</div>
<div class="agreementsUrl">
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
index 98f1413..863577c 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
@@ -117,7 +117,8 @@
return agreements ? 'show' : '';
}
- _disableAggreements(item, groups, signedAgreements) {
+ _disableAgreements(item, groups, signedAgreements) {
+ if (!groups) return false;
for (const group of groups) {
if ((item && item.auto_verify_group &&
item.auto_verify_group.id === group.id) ||
@@ -128,8 +129,8 @@
return false;
}
- _hideAggreements(item, groups, signedAgreements) {
- return this._disableAggreements(item, groups, signedAgreements) ?
+ _hideAgreements(item, groups, signedAgreements) {
+ return this._disableAgreements(item, groups, signedAgreements) ?
'' : 'hide';
}
@@ -141,6 +142,7 @@
// if specified it returns 'hideAgreementsTextBox' which
// then hides the text box and submit button.
_computeHideAgreementClass(name, config) {
+ if (!config) return '';
for (const key in config) {
if (!config.hasOwnProperty(key)) {
continue;
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
index 833fa39..13c4de4 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
@@ -142,28 +142,31 @@
'none');
});
- test('_disableAggreements', () => {
+ test('_disableAgreements', () => {
// In the auto verify group and have not yet signed agreement
assert.isTrue(
- element._disableAggreements(auth, groups, signedAgreements));
+ element._disableAgreements(auth, groups, signedAgreements));
// Not in the auto verify group and have not yet signed agreement
assert.isFalse(
- element._disableAggreements(auth2, groups, signedAgreements));
+ element._disableAgreements(auth2, groups, signedAgreements));
// Not in the auto verify group, have signed agreement
assert.isTrue(
- element._disableAggreements(auth3, groups, signedAgreements));
+ element._disableAgreements(auth3, groups, signedAgreements));
+ // Make sure the undefined check works
+ assert.isFalse(
+ element._disableAgreements(auth, undefined, signedAgreements));
});
- test('_hideAggreements', () => {
+ test('_hideAgreements', () => {
// Not in the auto verify group and have not yet signed agreement
assert.equal(
- element._hideAggreements(auth, groups, signedAgreements), '');
+ element._hideAgreements(auth, groups, signedAgreements), '');
// In the auto verify group
assert.equal(
- element._hideAggreements(auth2, groups, signedAgreements), 'hide');
+ element._hideAgreements(auth2, groups, signedAgreements), 'hide');
// Not in the auto verify group, have signed agreement
assert.equal(
- element._hideAggreements(auth3, groups, signedAgreements), '');
+ element._hideAgreements(auth3, groups, signedAgreements), '');
});
test('_disableAgreementsText', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
index d7510ec..89b28d5 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -58,6 +58,9 @@
#container.unresolved {
background-color: var(--unresolved-comment-background-color);
}
+ #container.robotComment {
+ background-color: var(--robot-comment-background-color);
+ }
#commentInfoContainer {
border-top: 1px dotted var(--border-color);
display: flex;
@@ -82,7 +85,7 @@
<span class="descriptionText">Patchset [[patchNum]]</span>
</div>
</template>
- <div id="container" class$="[[_computeHostClass(unresolved)]]">
+ <div id="container" class$="[[_computeHostClass(unresolved, isRobotComment)]]">
<template id="commentList" is="dom-repeat" items="[[_orderedComments]]"
as="comment">
<gr-comment
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index 72fc45d..8687e77 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -137,6 +137,11 @@
_lastComment: Object,
_orderedComments: Array,
_projectConfig: Object,
+ isRobotComment: {
+ type: Boolean,
+ value: false,
+ reflectToAttribute: true,
+ },
};
}
@@ -221,6 +226,7 @@
this._lastComment = this._getLastComment();
this.unresolved = this._lastComment.unresolved;
this.hasDraft = this._lastComment.__draft;
+ this.isRobotComment = !!(this._lastComment.robot_id);
}
}
@@ -498,6 +504,9 @@
}
_computeHostClass(unresolved) {
+ if (this.isRobotComment) {
+ return 'robotComment';
+ }
return unresolved ? 'unresolved' : '';
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index 46fa3cf..cb87bd9 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -247,7 +247,7 @@
<div id="container" class="container">
<div class="header" id="header" on-click="_handleToggleCollapsed">
<div class="headerLeft">
- <span class="authorName">[[comment.author.name]]</span>
+ <span class="authorName">[[_computeAuthorName(comment)]]</span>
<span class="draftLabel">DRAFT</span>
<gr-tooltip-content class="draftTooltip"
has-tooltip
@@ -258,6 +258,13 @@
<div class="headerMiddle">
<span class="collapsedContent">[[comment.message]]</span>
</div>
+ <div hidden$="[[_computeHideRunDetails(comment, collapsed)]]" class="runIdMessage message">
+ <div class="runIdInformation">
+ <a class="robotRunLink" href$="[[comment.url]]">
+ <span class="robotRun link">Run Details</span>
+ </a>
+ </div>
+ </div>
<gr-button
id="deleteBtn"
link
@@ -284,10 +291,9 @@
</div>
</div>
<div class="body">
- <template is="dom-if" if="[[comment.robot_id]]">
+ <template is="dom-if" if="[[isRobotComment]]">
<div class="robotId" hidden$="[[collapsed]]">
- <iron-icon class="robotIcon" icon="gr-icons:robot"></iron-icon>
- [[comment.robot_id]]
+ [[comment.author.name]]
</div>
</template>
<template is="dom-if" if="[[editing]]">
@@ -306,19 +312,6 @@
content="[[comment.message]]"
no-trailing-margin="[[!comment.__draft]]"
config="[[projectConfig.commentlinks]]"></gr-formatted-text>
- <div hidden$="[[!comment.robot_run_id]]" class="message">
- <div class="runIdInformation" hidden$="[[collapsed]]">
- Run ID:
- <template is="dom-if" if="[[comment.url]]">
- <a class="robotRunLink" href$="[[comment.url]]">
- <span class="robotRun link">[[comment.robot_run_id]]</span>
- </a>
- </template>
- <template is="dom-if" if="[[!comment.url]]">
- <span class="robotRun text">[[comment.robot_run_id]]</span>
- </template>
- </div>
- </div>
<div class="actions humanActions" hidden$="[[!_showHumanActions]]">
<div class="action resolve hideOnPublished">
<label>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
index dd2855c..7951d20 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
@@ -75,7 +75,7 @@
static get properties() {
return {
changeNum: String,
- /** @type {?} */
+ /** @type {!Gerrit.Comment} */
comment: {
type: Object,
notify: true,
@@ -196,14 +196,15 @@
}
get textarea() {
- return this.$$('#editTextarea');
+ return this.shadowRoot.querySelector('#editTextarea');
}
get confirmDeleteOverlay() {
if (!this._overlays.confirmDelete) {
this._enableOverlay = true;
Polymer.dom.flush();
- this._overlays.confirmDelete = this.$$('#confirmDeleteOverlay');
+ this._overlays.confirmDelete = this.shadowRoot
+ .querySelector('#confirmDeleteOverlay');
}
return this._overlays.confirmDelete;
}
@@ -212,7 +213,8 @@
if (!this._overlays.confirmDiscard) {
this._enableOverlay = true;
Polymer.dom.flush();
- this._overlays.confirmDiscard = this.$$('#confirmDiscardOverlay');
+ this._overlays.confirmDiscard = this.shadowRoot
+ .querySelector('#confirmDiscardOverlay');
}
return this._overlays.confirmDiscard;
}
@@ -672,6 +674,19 @@
return overlay.open();
}
+ _computeAuthorName(comment) {
+ if (!comment) return '';
+ if (comment.robot_id) {
+ return comment.robot_id;
+ }
+ return comment.author && comment.author.name;
+ }
+
+ _computeHideRunDetails(comment, collapsed) {
+ if (!comment) return true;
+ return !(comment.robot_id && comment.url && !collapsed);
+ }
+
_closeOverlay(overlay) {
Polymer.dom(Gerrit.getRootElement()).removeChild(overlay);
overlay.close();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
index de75b40..ad8b5e4 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
@@ -456,17 +456,13 @@
element.editing = false;
element.collapsed = false;
flushAsynchronousOperations();
- assert.isNotOk(element.$$('.robotRun.link'));
- assert.notEqual(getComputedStyle(element.$$('.robotRun.text')).display,
- 'none');
+ assert.isTrue(element.$$('.robotRun.link').textContent === 'Run Details');
// A robot comment with run ID and url should display a link.
element.set(['comment', 'url'], '/path/to/run');
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(element.$$('.robotRun.link')).display,
'none');
- assert.equal(getComputedStyle(element.$$('.robotRun.text')).display,
- 'none');
});
test('collapsible drafts', () => {
@@ -527,6 +523,37 @@
'header middle content is not visible');
});
+ test('robot comment layout', () => {
+ const comment = Object.assign({
+ robot_id: 'happy_robot_id',
+ url: '/robot/comment',
+ author: {
+ name: 'Happy Robot',
+ },
+ }, element.comment);
+ element.comment = comment;
+ element.collapsed = false;
+ flushAsynchronousOperations();
+
+ let runIdMessage;
+ runIdMessage = element.$$('.runIdMessage');
+ assert.isFalse(runIdMessage.hidden);
+
+ const runDetailsLink = element.$$('.robotRunLink');
+ assert.isTrue(runDetailsLink.href.indexOf(element.comment.url) !== -1);
+
+ const robotServiceName = element.$$('.authorName');
+ assert.isTrue(robotServiceName.textContent === 'happy_robot_id');
+
+ const authorName = element.$$('.robotId');
+ assert.isTrue(authorName.innerText === 'Happy Robot');
+
+ element.collapsed = true;
+ flushAsynchronousOperations();
+ runIdMessage = element.$$('.runIdMessage');
+ assert.isTrue(runIdMessage.hidden);
+ });
+
test('draft creation/cancellation', done => {
assert.isFalse(element.editing);
MockInteractions.tap(element.$$('.edit'));
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
index 1802a4a..ae08007 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
@@ -270,10 +270,26 @@
// Only one coverage provider makes sense. If there are more, then we
// simply ignore them.
if (provider) {
- return provider(changeNum, path, basePatchNum, patchNum);
+ return annotationApi;
}
}
- return [];
+ return null;
+ }).then(annotationApi => {
+ if (!annotationApi) return [];
+ const provider = annotationApi.getCoverageProvider();
+ return provider(changeNum, path, basePatchNum, patchNum)
+ .then(ranges => {
+ ranges = ranges || [];
+ // Notify with the coverage data.
+ ranges.forEach(range => {
+ annotationApi.notify(
+ path,
+ range.code_range.start_line,
+ range.code_range.end_line,
+ range.side);
+ });
+ return ranges;
+ });
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
index 8f87b38..f99d5d9 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
@@ -126,11 +126,13 @@
});
test('createNew link appears correctly', () => {
- assert.isFalse(element.$$('#createNewContainer').classList
+ assert.isFalse(element.shadowRoot
+ .querySelector('#createNewContainer').classList
.contains('show'));
element.createNew = true;
flushAsynchronousOperations();
- assert.isTrue(element.$$('#createNewContainer').classList
+ assert.isTrue(element.shadowRoot
+ .querySelector('#createNewContainer').classList
.contains('show'));
});
@@ -139,7 +141,7 @@
element.addEventListener('create-clicked', clickHandler);
element.createNew = true;
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#createNew'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#createNew'));
assert.isTrue(clickHandler.called);
});
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 eae25ae..75593e8 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
@@ -2354,7 +2354,7 @@
method: 'POST',
url: '/accounts/self/sshkeys',
body: key,
- contentType: 'plain/text',
+ contentType: 'text/plain',
reportUrlAsIs: true,
};
return this._restApiHelper.send(req)
diff --git a/polygerrit-ui/app/embed/embed.html b/polygerrit-ui/app/embed/embed.html
deleted file mode 100644
index 64e0137..0000000
--- a/polygerrit-ui/app/embed/embed.html
+++ /dev/null
@@ -1,27 +0,0 @@
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-<script>
- window.Gerrit = window.Gerrit || {};
-</script>
-<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../elements/change/gr-change-view/gr-change-view.html">
-<link rel="import" href="../elements/core/gr-search-bar/gr-search-bar.html">
-<link rel="import" href="../elements/diff/gr-diff-view/gr-diff-view.html">
-<link rel="import" href="../elements/change-list/gr-change-list-view/gr-change-list-view.html">
-<link rel="import" href="../elements/change-list/gr-dashboard-view/gr-dashboard-view.html">
-<link rel="import" href="../elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html">
-<link rel="import" href="../styles/themes/app-theme.html">
diff --git a/polygerrit-ui/app/embed/embed_test.html b/polygerrit-ui/app/embed/embed_test.html
deleted file mode 100644
index 5d81b7e..0000000
--- a/polygerrit-ui/app/embed/embed_test.html
+++ /dev/null
@@ -1,98 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>embed_test</title>
-
-<script src="/bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
-
-<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/bower_components/web-component-tester/browser.js"></script>
-<link rel="import" href="embed.html"/>
-
-<script>void(0);</script>
-
-<test-fixture id="change-view">
- <template>
- <gr-change-view></gr-change-view>
- </template>
-</test-fixture>
-
-<test-fixture id="diff-view">
- <template>
- <gr-diff-view></gr-diff-view>
- </template>
-</test-fixture>
-
-<test-fixture id="dashboard-view">
- <template>
- <gr-dashboard-view></gr-dashboard-view>
- </template>
-</test-fixture>
-
-<test-fixture id="change-list-view">
- <template>
- <gr-change-list-view></gr-change-list-view>
- </template>
-</test-fixture>
-
-<test-fixture id="change-list">
- <template>
- <gr-change-list></gr-change-list>
- </template>
-</test-fixture>
-
-<test-fixture id="search-bar">
- <template>
- <gr-search-bar></gr-search-bar>
- </template>
-</test-fixture>
-
-<script>
- suite('embed test', () => {
- test('gr-change-view is embedded', () => {
- const element = fixture('change-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-view');
- });
-
- test('diff-view is embedded', () => {
- const element = fixture('diff-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-diff-view');
- });
-
- test('dashboard-view is embedded', () => {
- const element = fixture('dashboard-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-dashboard-view');
- });
-
- test('change-list-view is embedded', () => {
- const element = fixture('change-list-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-list-view');
- });
-
- test('change-list is embedded', () => {
- const element = fixture('change-list');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-list');
- });
-
- test('search-bar is embedded', () => {
- const element = fixture('search-bar');
- assert.equal(element.tagName.toLowerCase(), 'gr-search-bar');
- });
- });
-</script>
diff --git a/polygerrit-ui/app/embed/test.html b/polygerrit-ui/app/embed/test.html
deleted file mode 100644
index 955eaee..0000000
--- a/polygerrit-ui/app/embed/test.html
+++ /dev/null
@@ -1,26 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>Embed Test Runner</title>
-<meta charset="utf-8">
-<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/bower_components/web-component-tester/browser.js"></script>
-<script>
- WCT.loadSuites(['../embed/embed_test.html']);
-</script>
diff --git a/polygerrit-ui/app/embed_test.sh b/polygerrit-ui/app/embed_test.sh
deleted file mode 100755
index 0d8f58f..0000000
--- a/polygerrit-ui/app/embed_test.sh
+++ /dev/null
@@ -1,69 +0,0 @@
-#!/bin/sh
-
-set -ex
-
-t=$(mktemp -d || mktemp -d -t wct-XXXXXXXXXX)
-components=$TEST_SRCDIR/gerrit/polygerrit-ui/app/test_components.zip
-code=$TEST_SRCDIR/gerrit/polygerrit-ui/app/pg_code.zip
-
-echo $t
-unzip -qd $t $components
-unzip -qd $t $code
-# Purge test/ directory contents coming from pg_code.zip.
-rm -rf $t/test
-mkdir -p $t/test
-cp $TEST_SRCDIR/gerrit/polygerrit-ui/app/embed/test.html $t/test/
-
-if [ "${WCT_HEADLESS_MODE:-0}" != "0" ]; then
- CHROME_OPTIONS=[\'start-maximized\',\'headless\',\'disable-gpu\',\'no-sandbox\']
- FIREFOX_OPTIONS=[\'-headless\']
-else
- CHROME_OPTIONS=[\'start-maximized\']
- FIREFOX_OPTIONS=[\'\']
-fi
-
-# For some reason wct tries to install selenium into its node_modules
-# directory on first run. If you've installed into /usr/local and
-# aren't running wct as root, you're screwed. Turning this option off
-# through skipSeleniumInstall seems to still work, so there's that.
-
-# Sauce tests are disabled by default in order to run local tests
-# only. Run it with (saucelabs.com account required; free for open
-# source): WCT_ARGS='--plugin sauce' ./polygerrit-ui/app/embed_test.sh
-
-cat <<EOF > $t/wct.conf.js
-module.exports = {
- 'suites': ['test'],
- 'webserver': {
- 'pathMappings': [
- {'/components/bower_components': 'bower_components'}
- ]
- },
- 'plugins': {
- 'local': {
- 'skipSeleniumInstall': true,
- 'browserOptions': {
- 'chrome': ${CHROME_OPTIONS},
- 'firefox': ${FIREFOX_OPTIONS}
- }
- },
- 'sauce': {
- 'disabled': true,
- 'browsers': [
- 'OS X 10.12/chrome',
- 'Windows 10/chrome',
- 'Linux/firefox',
- 'OS X 10.12/safari',
- 'Windows 10/microsoftedge'
- ]
- }
- }
- };
-EOF
-
-export PATH="$(dirname $NPM):$PATH"
-
-cd $t
-test -n "${WCT}"
-
-${WCT} ${WCT_ARGS}
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 16c0f29..e1304e6 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -89,6 +89,8 @@
# we extract from the zip, but depend on the component for license checking.
"@webcomponentsjs//:zipfile",
"//lib/js:webcomponentsjs",
+ "@font-roboto-local//:zipfile",
+ "//lib/js:font-roboto-local",
],
outs = outs,
cmd = " && ".join([
@@ -100,6 +102,7 @@
"for f in $(locations " + name + "_theme_sources); do cp $$f $$TMP/polygerrit_ui/styles/themes; done",
"for f in $(locations //lib/js:highlightjs_files); do cp $$f $$TMP/polygerrit_ui/bower_components/highlightjs/ ; done",
"unzip -qd $$TMP/polygerrit_ui/bower_components $(location @webcomponentsjs//:zipfile) webcomponentsjs/webcomponents-lite.js",
+ "unzip -qd $$TMP/polygerrit_ui/bower_components $(location @font-roboto-local//:zipfile) font-roboto-local/fonts/\*/\*.ttf",
"cd $$TMP",
"find . -exec touch -t 198001010000 '{}' ';'",
"zip -qr $$ROOT/$@ *",
diff --git a/polygerrit-ui/app/run_test.sh b/polygerrit-ui/app/run_test.sh
index e9be18d..76a4fa1 100755
--- a/polygerrit-ui/app/run_test.sh
+++ b/polygerrit-ui/app/run_test.sh
@@ -48,5 +48,4 @@
--test_env="DISPLAY=${DISPLAY}" \
--test_env="WCT_HEADLESS_MODE=${WCT_HEADLESS_MODE}" \
"$@" \
- //polygerrit-ui/app:embed_test \
//polygerrit-ui/app:wct_test
diff --git a/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.html b/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.html
index 5c5194c..76e978c 100644
--- a/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.html
+++ b/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.html
@@ -29,6 +29,10 @@
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
</style>
<style>
+ :host {
+ border-top: 1px solid var(--border-color);
+ display: block;
+ }
.header {
color: var(--primary-text-color);
background-color: var(--table-header-background-color);
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index 76ddac4..7bb8fff 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -45,6 +45,7 @@
--assignee-highlight-color: #fcfad6;
--chip-background-color: #eee;
--comment-background-color: #fcfad6;
+ --robot-comment-background-color: #e8f0fe;
--default-button-background-color: white;
--dialog-background-color: white;
--dropdown-background-color: white;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index 015d372..732ad2e 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -45,6 +45,7 @@
--assignee-highlight-color: #3a361c;
--chip-background-color: #131416;
--comment-background-color: #0b162b;
+ --robot-comment-background-color: #e8f0fe;
--default-button-background-color: #3c4043;
--dialog-background-color: #131416;
--dropdown-background-color: #131416;
diff --git a/polygerrit-ui/app/types/types.js b/polygerrit-ui/app/types/types.js
index 8dd65cb..3b91407 100644
--- a/polygerrit-ui/app/types/types.js
+++ b/polygerrit-ui/app/types/types.js
@@ -273,4 +273,27 @@
* makeSuggestionItem: function(Object): Gerrit.GrSuggestionItem,
* }}
*/
-Gerrit.GrSuggestionsProvider;
\ No newline at end of file
+Gerrit.GrSuggestionsProvider;
+
+/**
+ * @typedef {{
+ * patch_set: ?number,
+ * id: ?string,
+ * path: ?Object,
+ * side: ?string,
+ * parent: ?number,
+ * line: ?Object,
+ * in_reply_to: ?string,
+ * message: ?Object,
+ * updated: ?string,
+ * author: ?Object,
+ * tag: ?Object,
+ * unresolved: ?boolean,
+ * robot_id: ?string,
+ * robot_run_id: ?string,
+ * url: ?string,
+ * properties: ?Object,
+ * fix_suggestions: ?Object,
+ * }}
+ */
+Gerrit.Comment;
diff --git a/tools/BUILD b/tools/BUILD
index 5531c3e..f0a4ffa 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -53,6 +53,7 @@
"-Xep:ExpectedExceptionChecker:ERROR",
"-Xep:Finally:ERROR",
"-Xep:FloatingPointLiteralPrecision:ERROR",
+ "-Xep:FormatStringAnnotation:ERROR",
"-Xep:FragmentInjection:ERROR",
"-Xep:FragmentNotInstantiable:ERROR",
"-Xep:FunctionalInterfaceClash:ERROR",
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 357faa4..6a29da6 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -69,8 +69,8 @@
# elasticsearch-rest-client explicitly depends on this version
maven_jar(
name = "httpcore-nio",
- artifact = "org.apache.httpcomponents:httpcore-nio:4.4.11",
- sha1 = "7d0a97d01d39cff9aa3e6db81f21fddb2435f4e6",
+ artifact = "org.apache.httpcomponents:httpcore-nio:4.4.12",
+ sha1 = "84cd29eca842f31db02987cfedea245af020198b",
)
maven_jar(
@@ -102,8 +102,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.4.2",
- sha1 = "f48725523c0b3402f869214433602f8d3f4c737c",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.0",
+ sha1 = "62535b6fc3a4e943e88e7640eac22e29f03a696d",
)
maven_jar(
@@ -170,18 +170,18 @@
sha1 = "3e83394258ae2089be7219b971ec21a8288528ad",
)
- TESTCONTAINERS_VERSION = "1.12.3"
+ TESTCONTAINERS_VERSION = "1.12.4"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "e424a4549640e120acceac641ac909fcda58bf62",
+ sha1 = "456b6facac12c4b67130d9056a43c011679e9f0c",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "c0796de5032070b8768ce78c78949b48f13c30db",
+ sha1 = "9e210c277a35a95a76d03a79e2812575bd07391c",
)
maven_jar(
