Merge "Fix an internal server error when diffing patch sets (non-merge commit)"
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/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..e370f69 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -748,7 +748,7 @@
sha1 = "f7be08ec23c21485b9b5a1cf1654c2ec8c58168d",
)
-GITILES_VERS = "0.3-6"
+GITILES_VERS = "0.3-7"
GITILES_REPO = GERRIT
@@ -757,14 +757,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/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/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/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java
index c30378b..6d28646a 100644
--- a/java/com/google/gerrit/server/project/Reachable.java
+++ b/java/com/google/gerrit/server/project/Reachable.java
@@ -67,7 +67,7 @@
try (TraceTimer timer =
TraceContext.newTimer(
"IncludedInResolver.includedInAny",
- Metadata.builder().projectName(project.get()).inputSize(refs.size()).build())) {
+ Metadata.builder().projectName(project.get()).resourceCount(refs.size()).build())) {
return IncludedInResolver.includedInAny(repo, rw, commit, filtered.values());
}
} catch (IOException | PermissionBackendException e) {
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/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/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/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/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index 0090ed1..abfc23d 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);
+ }
+
+ @Test
+ @UseClockStep
+ @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
+ @GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ /**
+ * 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).
+ */
+ 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/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/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/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/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/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 d7c09fb..4689b41 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit d7c09fbb4c18b1743d6060d361171c2a5237f22b
+Subproject commit 4689b419eab61ea204daf2dfca47296667ac317c
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/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..532c573 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
@@ -1422,6 +1422,7 @@
let payload;
let onShowError;
let onShowAlert;
+ let getResponseObjectStub;
setup(() => {
cleanup = sinon.stub();
@@ -1437,12 +1438,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 +1462,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/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-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/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..1dfc2f4 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -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(
