Merge "Fix updates to project config when erasing data"
diff --git a/Documentation/dev-design-doc-conclusion-template.md b/Documentation/dev-design-doc-conclusion-template.md
index 36bfa2a..0625f2b 100644
--- a/Documentation/dev-design-doc-conclusion-template.md
+++ b/Documentation/dev-design-doc-conclusion-template.md
@@ -1,3 +1,13 @@
+---
+title: "Design Doc - ${title} - Conclusion"
+sidebar: gerritdoc_sidebar
+permalink: design-doc-${folder-name}-conclusion.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+folder: design-docs/${folder-name}
+---
+
# Conclusion
Describe which decision was made and what were the reasons for it.
diff --git a/Documentation/dev-design-doc-index-template.md b/Documentation/dev-design-doc-index-template.md
index 604126d..10b4a81 100644
--- a/Documentation/dev-design-doc-index-template.md
+++ b/Documentation/dev-design-doc-index-template.md
@@ -1,3 +1,13 @@
+---
+title: "Design Doc - ${title}"
+sidebar: gerritdoc_sidebar
+permalink: design-doc-${folder-name}.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+folder: design-docs/${folder-name}
+---
+
# Design Doc - ${title}
* [Use Cases](use-cases.html)
diff --git a/Documentation/dev-design-doc-solution-template.md b/Documentation/dev-design-doc-solution-template.md
index 66b8ae7..8935902 100644
--- a/Documentation/dev-design-doc-solution-template.md
+++ b/Documentation/dev-design-doc-solution-template.md
@@ -1,3 +1,13 @@
+---
+title: "Design Doc - ${title} - Solution - ${solution-name}"
+sidebar: gerritdoc_sidebar
+permalink: design-doc-${folder-name}-solution-${solution-name}.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+folder: design-docs/${folder-name}
+---
+
# Solution - ${solution-name}
## <a id="overview"> Overview
@@ -20,8 +30,13 @@
### <a id="scalability"> Scalability
-How does the solution scale? Consider both, data size increase (if
-applicable) and traffic increase (if applicable).
+How does the solution scale?
+
+If applicable, consider:
+
+* data size increase
+* traffic increase
+* effects on replication across sites (master-slave and master-master)
## <a id="alternatives-considered"> Alternatives Considered
diff --git a/Documentation/dev-design-doc-use-cases-template.md b/Documentation/dev-design-doc-use-cases-template.md
index 704ad14c..02c2fb5 100644
--- a/Documentation/dev-design-doc-use-cases-template.md
+++ b/Documentation/dev-design-doc-use-cases-template.md
@@ -1,3 +1,13 @@
+---
+title: "Design Doc - ${title} - Use Cases"
+sidebar: gerritdoc_sidebar
+permalink: design-doc-${folder-name}-use-cases.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+folder: design-docs/${folder-name}
+---
+
# Use Cases
In a few sentences, describe the use-cases as interactions between a
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 367b59d..ae9f9ff 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -57,6 +57,11 @@
* `caches/disk_cached`: Disk entries used by persistent cache.
* `caches/disk_hit_ratio`: Disk hit ratio for persistent cache.
+=== Change
+
+* `change/submit_rule_evaluation`: Latency for evaluating submit rules on a change.
+* `change/submit_type_evaluation`: Latency for evaluating the submit type on a change.
+
=== HTTP
* `http/server/error_count`: Rate of REST API error responses.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 56376fa..ec269e4 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6831,13 +6831,6 @@
=== Requirement
The `Requirement` entity contains information about a requirement relative to a change.
-type:: Alphanumerical (plus hyphens or underscores) string to identify what the requirement is and
-why it was triggered. Can be seen as a class: requirements sharing the same type were created for a
-similar reason, and the data structure will follow one set of rules.
-
-data:: (Optional) Additional key-value data linked to this requirement. This is used in templates to
-render rich status messages.
-
[options="header",cols="1,^1,5"]
|===========================
|Field Name | |Description
diff --git a/WORKSPACE b/WORKSPACE
index 350dba1..6a86c19 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1251,8 +1251,8 @@
bower_archive(
name = "page",
package = "visionmedia/page.js",
- sha1 = "51a05428dd4f68fae1df5f12d0e2b61ba67f7757",
- version = "1.7.1",
+ sha1 = "4a31889cd75cc5e7f68a4c7f256eecaf27102eee",
+ version = "1.11.4",
)
bower_archive(
diff --git a/contrib/Refresh_plugin_in_testsite.sh b/contrib/refresh_plugin_in_testsite.sh
similarity index 100%
rename from contrib/Refresh_plugin_in_testsite.sh
rename to contrib/refresh_plugin_in_testsite.sh
diff --git a/contrib/show_new_gerrit_doc_in_chrome.sh b/contrib/show_new_gerrit_doc_in_chrome.sh
new file mode 100755
index 0000000..d57bc8a
--- /dev/null
+++ b/contrib/show_new_gerrit_doc_in_chrome.sh
@@ -0,0 +1,48 @@
+#!/bin/bash
+#
+# 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.
+
+# This script builds Gerrit's documentation and shows the current state in
+# Chrome. Specific pages (e.g. rest-api-changes.txt) including anchors can be
+# passed as parameter to jump directly to them.
+
+SCRIPT_DIR=$(dirname -- "$(readlink -f -- "$BASH_SOURCE")")
+GERRIT_CODE_DIR="$SCRIPT_DIR/.."
+cd "$GERRIT_CODE_DIR"
+
+bazel build Documentation:searchfree
+if [ $? -ne 0 ]
+then
+ echo "Building the documentation failed. Stopping."
+ exit 1
+fi
+
+TMP_DOCS_DIR=/tmp/gerrit_docs
+rm -rf "$TMP_DOCS_DIR"
+unzip bazel-bin/Documentation/searchfree.zip -d "$TMP_DOCS_DIR" </dev/null >/dev/null 2>&1 & disown
+if [ $? -ne 0 ]
+then
+ echo "Unzipping the documentation to $TMP_DOCS_DIR failed. Stopping."
+ exit 1
+fi
+
+if [ "$#" -lt 1 ]
+then
+ FILE_NAME="index.html"
+else
+ FILE_NAME="$1"
+fi
+DOC_FILE_NAME="${FILE_NAME/.txt/.html}"
+google-chrome "file:///$TMP_DOCS_DIR/Documentation/$DOC_FILE_NAME" </dev/null >/dev/null 2>&1 & disown
diff --git a/contrib/start_testsite.sh b/contrib/start_testsite.sh
new file mode 100755
index 0000000..014eba9
--- /dev/null
+++ b/contrib/start_testsite.sh
@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# 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.
+
+# This script starts the local testsite in debug mode. If the flag "-u" is
+# passed, Gerrit is built from the current state of the repository and the
+# testsite is refreshed. The path to the testsite needs to be provided by
+# the variable GERRIT_TESTSITE or as parameter (after any used flags).
+# The testsite can be stopped by interrupting this script.
+
+SCRIPT_DIR=$(dirname -- "$(readlink -f -- "$BASH_SOURCE")")
+GERRIT_CODE_DIR="$SCRIPT_DIR/.."
+cd "$GERRIT_CODE_DIR"
+
+UPDATE=false
+while getopts ':u' flag; do
+ case "${flag}" in
+ u) UPDATE=true ;;
+ esac
+done
+shift $(($OPTIND-1))
+
+if [ "$#" -lt 1 ]
+then
+ if [ -z ${GERRIT_TESTSITE+x} ]
+ then
+ echo "Path to local testsite is neither set as GERRIT_TESTSITE nor passed as first argument. Stopping."
+ exit 1
+ fi
+else
+ GERRIT_TESTSITE="$1"
+fi
+
+if [ "$UPDATE" = true ]
+then
+ echo "Refreshing testsite"
+ bazel build gerrit
+ if [ $? -ne 0 ]
+ then
+ echo "Build failed. Stopping."
+ exit 1
+ fi
+ $(bazel info output_base)/external/local_jdk/bin/java -jar bazel-bin/gerrit.war init --batch -d "$GERRIT_TESTSITE"
+ if [ $? -ne 0 ]
+ then
+ echo "Patching the testsite failed. Stopping."
+ exit 1
+ fi
+fi
+
+$(bazel info output_base)/external/local_jdk/bin/java -jar -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 bazel-bin/gerrit.war daemon -d "$GERRIT_TESTSITE" --console-log
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index e3ab70d..aa38c27 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.AuthResult;
import com.google.gerrit.server.account.AuthenticationFailedException;
+import com.google.gerrit.server.account.externalids.PasswordVerifier;
import com.google.gerrit.server.auth.NoSuchUserException;
import com.google.gerrit.server.config.AuthConfig;
import com.google.inject.Inject;
@@ -140,7 +141,7 @@
GitBasicAuthPolicy gitBasicAuthPolicy = authConfig.getGitBasicAuthPolicy();
if (gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP
|| gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP_LDAP) {
- if (who.checkPassword(password, username)) {
+ if (PasswordVerifier.checkPassword(who.getExternalIds(), username, password)) {
return succeedAuthentication(who);
}
}
@@ -157,7 +158,7 @@
setUserIdentified(whoAuthResult.getAccountId());
return true;
} catch (NoSuchUserException e) {
- if (who.checkPassword(password, username)) {
+ if (PasswordVerifier.checkPassword(who.getExternalIds(), username, password)) {
return succeedAuthentication(who);
}
logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req));
diff --git a/java/com/google/gerrit/json/EnumTypeAdapterFactory.java b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
index dc74f67..21c4891 100644
--- a/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
+++ b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
@@ -34,7 +34,7 @@
public class EnumTypeAdapterFactory implements TypeAdapterFactory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- @SuppressWarnings({"unchecked"})
+ @SuppressWarnings({"rawtypes", "unchecked"})
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
TypeAdapter<T> defaultEnumAdapter = TypeAdapters.ENUM_FACTORY.create(gson, typeToken);
@@ -43,7 +43,7 @@
return null;
}
- return (TypeAdapter<T>) new EnumTypeAdapter(defaultEnumAdapter, typeToken);
+ return new EnumTypeAdapter(defaultEnumAdapter, typeToken);
}
private static class EnumTypeAdapter<T extends Enum<T>> extends TypeAdapter<T> {
diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java
index 8555166..6eb6ca1 100644
--- a/java/com/google/gerrit/server/account/AccountState.java
+++ b/java/com/google/gerrit/server/account/AccountState.java
@@ -14,13 +14,9 @@
package com.google.gerrit.server.account;
-import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
-
import com.google.common.base.MoreObjects;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.EditPreferencesInfo;
@@ -34,7 +30,6 @@
import java.io.IOException;
import java.util.Collection;
import java.util.Optional;
-import org.apache.commons.codec.DecoderException;
import org.eclipse.jgit.lib.ObjectId;
/**
@@ -45,8 +40,6 @@
* account cache (see {@link AccountCache#get(Account.Id)}).
*/
public class AccountState {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
/**
* Creates an AccountState from the given account config.
*
@@ -175,29 +168,6 @@
return userName;
}
- public boolean checkPassword(@Nullable String password, String username) {
- if (password == null) {
- return false;
- }
- for (ExternalId id : getExternalIds()) {
- // Only process the "username:$USER" entry, which is unique.
- if (!id.isScheme(SCHEME_USERNAME) || !username.equals(id.key().id())) {
- continue;
- }
-
- String hashedStr = id.password();
- if (!Strings.isNullOrEmpty(hashedStr)) {
- try {
- return HashedPassword.decode(hashedStr).checkPassword(password);
- } catch (DecoderException e) {
- logger.atSevere().log("DecoderException for user %s: %s ", username, e.getMessage());
- return false;
- }
- }
- }
- return false;
- }
-
/** The external identities that identify the account holder. */
public ImmutableSet<ExternalId> getExternalIds() {
return externalIds;
diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index 14f279b..1d53ed2 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -14,12 +14,14 @@
package com.google.gerrit.server.account;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
-import com.google.common.collect.Streams;
+import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.SetMultimap;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -32,6 +34,8 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
/** Class to access accounts by email. */
@Singleton
@@ -65,15 +69,20 @@
* have no external ID for the preferred email. Having accounts with a preferred email that does
* not exist as external ID is an inconsistency, but existing functionality relies on still
* getting those accounts, which is why they are included. Accounts by preferred email are fetched
- * from the account index.
+ * from the account index as a fallback for email addresses that could not be resolved using
+ * {@link ExternalIds}.
*
* @see #getAccountsFor(String...)
*/
public ImmutableSet<Account.Id> getAccountFor(String email) throws IOException {
- return Streams.concat(
- externalIds.byEmail(email).stream().map(ExternalId::accountId),
- executeIndexQuery(() -> queryProvider.get().byPreferredEmail(email).stream())
- .map(a -> a.getAccount().id()))
+ ImmutableSet<Account.Id> accounts =
+ externalIds.byEmail(email).stream().map(ExternalId::accountId).collect(toImmutableSet());
+ if (!accounts.isEmpty()) {
+ return accounts;
+ }
+
+ return executeIndexQuery(() -> queryProvider.get().byPreferredEmail(email).stream())
+ .map(a -> a.getAccount().id())
.collect(toImmutableSet());
}
@@ -84,12 +93,18 @@
*/
public ImmutableSetMultimap<String, Account.Id> getAccountsFor(String... emails)
throws IOException {
- ImmutableSetMultimap.Builder<String, Account.Id> builder = ImmutableSetMultimap.builder();
+ SetMultimap<String, Account.Id> result =
+ MultimapBuilder.hashKeys(emails.length).hashSetValues(1).build();
externalIds.byEmails(emails).entries().stream()
- .forEach(e -> builder.put(e.getKey(), e.getValue().accountId()));
- executeIndexQuery(() -> queryProvider.get().byPreferredEmail(emails).entries().stream())
- .forEach(e -> builder.put(e.getKey(), e.getValue().getAccount().id()));
- return builder.build();
+ .forEach(e -> result.put(e.getKey(), e.getValue().accountId()));
+ List<String> emailsToBackfill =
+ Arrays.stream(emails).filter(e -> !result.containsKey(e)).collect(toImmutableList());
+ if (!emailsToBackfill.isEmpty()) {
+ executeIndexQuery(
+ () -> queryProvider.get().byPreferredEmail(emailsToBackfill).entries().stream())
+ .forEach(e -> result.put(e.getKey(), e.getValue().getAccount().id()));
+ }
+ return ImmutableSetMultimap.copyOf(result);
}
/**
diff --git a/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java
new file mode 100644
index 0000000..e4bf27b
--- /dev/null
+++ b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java
@@ -0,0 +1,54 @@
+// 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.account.externalids;
+
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+
+import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.account.HashedPassword;
+import java.util.Collection;
+import org.apache.commons.codec.DecoderException;
+
+/** Checks if a given username and password match a user's external IDs. */
+public class PasswordVerifier {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ /** Returns {@code true} if there is an external ID matching both the username and password. */
+ public static boolean checkPassword(
+ Collection<ExternalId> externalIds, String username, @Nullable String password) {
+ if (password == null) {
+ return false;
+ }
+ for (ExternalId id : externalIds) {
+ // Only process the "username:$USER" entry, which is unique.
+ if (!id.isScheme(SCHEME_USERNAME) || !username.equals(id.key().id())) {
+ continue;
+ }
+
+ String hashedStr = id.password();
+ if (!Strings.isNullOrEmpty(hashedStr)) {
+ try {
+ return HashedPassword.decode(hashedStr).checkPassword(password);
+ } catch (DecoderException e) {
+ logger.atSevere().log("DecoderException for user %s: %s ", username, e.getMessage());
+ return false;
+ }
+ }
+ }
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/server/auth/InternalAuthBackend.java b/java/com/google/gerrit/server/auth/InternalAuthBackend.java
index c06c66b..2821bf6 100644
--- a/java/com/google/gerrit/server/auth/InternalAuthBackend.java
+++ b/java/com/google/gerrit/server/auth/InternalAuthBackend.java
@@ -16,6 +16,7 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.externalids.PasswordVerifier;
import com.google.gerrit.server.config.AuthConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -62,7 +63,7 @@
+ ": account inactive or not provisioned in Gerrit");
}
- if (!who.checkPassword(req.getPassword().get(), username)) {
+ if (!PasswordVerifier.checkPassword(who.getExternalIds(), username, req.getPassword().get())) {
throw new InvalidCredentialsException();
}
return new AuthUser(AuthUser.UUID.create(username), username);
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 1b1869c..36c6d36 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -19,6 +19,10 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.OnlineReindexMode;
import com.google.gerrit.server.plugincontext.PluginSetContext;
@@ -44,6 +48,8 @@
private final ProjectCache projectCache;
private final PrologRule prologRule;
private final PluginSetContext<SubmitRule> submitRules;
+ private final Timer0 submitRuleEvaluationLatency;
+ private final Timer0 submitTypeEvaluationLatency;
private final SubmitRuleOptions opts;
public interface Factory {
@@ -56,10 +62,23 @@
ProjectCache projectCache,
PrologRule prologRule,
PluginSetContext<SubmitRule> submitRules,
+ MetricMaker metricMaker,
@Assisted SubmitRuleOptions options) {
this.projectCache = projectCache;
this.prologRule = prologRule;
this.submitRules = submitRules;
+ this.submitRuleEvaluationLatency =
+ metricMaker.newTimer(
+ "change/submit_rule_evaluation",
+ new Description("Latency for evaluating submit rules on a change.")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ this.submitTypeEvaluationLatency =
+ metricMaker.newTimer(
+ "change/submit_type_evaluation",
+ new Description("Latency for evaluating the submit type on a change.")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
this.opts = options;
}
@@ -87,46 +106,41 @@
* @param cd ChangeData to evaluate
*/
public List<SubmitRecord> evaluate(ChangeData cd) {
- Change change;
- ProjectState projectState;
- try {
- change = cd.change();
- if (change == null) {
- throw new StorageException("Change not found");
+ try (Timer0.Context ignored = submitRuleEvaluationLatency.start()) {
+ Change change;
+ ProjectState projectState;
+ try {
+ change = cd.change();
+ if (change == null) {
+ throw new StorageException("Change not found");
+ }
+
+ projectState = projectCache.get(cd.project());
+ if (projectState == null) {
+ throw new NoSuchProjectException(cd.project());
+ }
+ } catch (StorageException | NoSuchProjectException e) {
+ return ruleError("Error looking up change " + cd.getId(), e);
}
- projectState = projectCache.get(cd.project());
- if (projectState == null) {
- throw new NoSuchProjectException(cd.project());
+ if ((!opts.allowClosed() || OnlineReindexMode.isActive()) && change.isClosed()) {
+ SubmitRecord rec = new SubmitRecord();
+ rec.status = SubmitRecord.Status.CLOSED;
+ return Collections.singletonList(rec);
}
- } catch (StorageException | NoSuchProjectException e) {
- return ruleError("Error looking up change " + cd.getId(), e);
- }
- if ((!opts.allowClosed() || OnlineReindexMode.isActive()) && change.isClosed()) {
- SubmitRecord rec = new SubmitRecord();
- rec.status = SubmitRecord.Status.CLOSED;
- return Collections.singletonList(rec);
+ // We evaluate all the plugin-defined evaluators,
+ // and then we collect the results in one list.
+ return Streams.stream(submitRules)
+ .map(c -> c.call(s -> s.evaluate(cd)))
+ .flatMap(Collection::stream)
+ .collect(Collectors.toList());
}
-
- // We evaluate all the plugin-defined evaluators,
- // and then we collect the results in one list.
- return Streams.stream(submitRules)
- .map(c -> c.call(s -> s.evaluate(cd, opts)))
- .flatMap(Collection::stream)
- .collect(Collectors.toList());
}
private List<SubmitRecord> ruleError(String err, Exception e) {
- if (opts.logErrors()) {
- if (e == null) {
- logger.atSevere().log(err);
- } else {
- logger.atSevere().withCause(e).log(err);
- }
- return defaultRuleError();
- }
- return createRuleError(err);
+ logger.atSevere().withCause(e).log(err);
+ return defaultRuleError();
}
/**
@@ -136,24 +150,23 @@
* @param cd
*/
public SubmitTypeRecord getSubmitType(ChangeData cd) {
- ProjectState projectState;
- try {
- projectState = projectCache.get(cd.project());
- if (projectState == null) {
- throw new NoSuchProjectException(cd.project());
+ try (Timer0.Context ignored = submitTypeEvaluationLatency.start()) {
+ ProjectState projectState;
+ try {
+ projectState = projectCache.get(cd.project());
+ if (projectState == null) {
+ throw new NoSuchProjectException(cd.project());
+ }
+ } catch (NoSuchProjectException e) {
+ return typeError("Error looking up change " + cd.getId(), e);
}
- } catch (NoSuchProjectException e) {
- return typeError("Error looking up change " + cd.getId(), e);
- }
- return prologRule.getSubmitType(cd, opts);
+ return prologRule.getSubmitType(cd);
+ }
}
private SubmitTypeRecord typeError(String err, Exception e) {
- if (opts.logErrors()) {
- logger.atSevere().withCause(e).log(err);
- return defaultTypeError();
- }
- return SubmitTypeRecord.error(err);
+ logger.atSevere().withCause(e).log(err);
+ return defaultTypeError();
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRuleOptions.java b/java/com/google/gerrit/server/project/SubmitRuleOptions.java
index a4340b2..ad077c0 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleOptions.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleOptions.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.project;
import com.google.auto.value.AutoValue;
-import com.google.gerrit.common.Nullable;
/**
* Stable identifier for options passed to a particular submit rule evaluator.
@@ -26,12 +25,7 @@
@AutoValue
public abstract class SubmitRuleOptions {
private static final SubmitRuleOptions defaults =
- new AutoValue_SubmitRuleOptions.Builder()
- .allowClosed(false)
- .skipFilters(false)
- .logErrors(true)
- .rule(null)
- .build();
+ new AutoValue_SubmitRuleOptions.Builder().allowClosed(false).build();
public static SubmitRuleOptions defaults() {
return defaults;
@@ -43,25 +37,12 @@
public abstract boolean allowClosed();
- public abstract boolean skipFilters();
-
- public abstract boolean logErrors();
-
- @Nullable
- public abstract String rule();
-
public abstract Builder toBuilder();
@AutoValue.Builder
public abstract static class Builder {
public abstract SubmitRuleOptions.Builder allowClosed(boolean allowClosed);
- public abstract SubmitRuleOptions.Builder skipFilters(boolean skipFilters);
-
- public abstract SubmitRuleOptions.Builder rule(@Nullable String rule);
-
- public abstract SubmitRuleOptions.Builder logErrors(boolean logErrors);
-
public abstract SubmitRuleOptions build();
}
}
diff --git a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
index 0253ede..c38c92f 100644
--- a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
+++ b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.index.account.AccountField;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.inject.Inject;
-import java.util.Arrays;
import java.util.List;
import java.util.Set;
@@ -93,15 +92,13 @@
* @return multimap of the given emails to accounts that have a preferred email that exactly
* matches this email
*/
- public Multimap<String, AccountState> byPreferredEmail(String... emails) {
- List<String> emailList = Arrays.asList(emails);
-
+ public Multimap<String, AccountState> byPreferredEmail(List<String> emails) {
if (hasPreferredEmailExact()) {
List<List<AccountState>> r =
- query(emailList.stream().map(AccountPredicates::preferredEmailExact).collect(toList()));
+ query(emails.stream().map(AccountPredicates::preferredEmailExact).collect(toList()));
Multimap<String, AccountState> accountsByEmail = ArrayListMultimap.create();
- for (int i = 0; i < emailList.size(); i++) {
- accountsByEmail.putAll(emailList.get(i), r.get(i));
+ for (int i = 0; i < emails.size(); i++) {
+ accountsByEmail.putAll(emails.get(i), r.get(i));
}
return accountsByEmail;
}
@@ -111,10 +108,10 @@
}
List<List<AccountState>> r =
- query(emailList.stream().map(AccountPredicates::preferredEmail).collect(toList()));
+ query(emails.stream().map(AccountPredicates::preferredEmail).collect(toList()));
Multimap<String, AccountState> accountsByEmail = ArrayListMultimap.create();
- for (int i = 0; i < emailList.size(); i++) {
- String email = emailList.get(i);
+ for (int i = 0; i < emails.size(); i++) {
+ String email = emails.get(i);
Set<AccountState> matchingAccounts =
r.get(i).stream()
.filter(a -> a.getAccount().preferredEmail().equals(email))
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
index afd02a9..d5ed9a4 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
@@ -31,9 +31,8 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.rules.DefaultSubmitRule;
+import com.google.gerrit.server.rules.PrologOptions;
import com.google.gerrit.server.rules.PrologRule;
import com.google.gerrit.server.rules.RulesCache;
import com.google.inject.Inject;
@@ -46,7 +45,6 @@
private final RulesCache rules;
private final AccountLoader.Factory accountInfoFactory;
private final ProjectCache projectCache;
- private final DefaultSubmitRule defaultSubmitRule;
private final PrologRule prologRule;
@Option(name = "--filters", usage = "impact of filters in parent projects")
@@ -58,13 +56,11 @@
RulesCache rules,
AccountLoader.Factory infoFactory,
ProjectCache projectCache,
- DefaultSubmitRule defaultSubmitRule,
PrologRule prologRule) {
this.changeDataFactory = changeDataFactory;
this.rules = rules;
this.accountInfoFactory = infoFactory;
this.projectCache = projectCache;
- this.defaultSubmitRule = defaultSubmitRule;
this.prologRule = prologRule;
}
@@ -74,33 +70,23 @@
if (input == null) {
input = new TestSubmitRuleInput();
}
- if (input.rule != null && !rules.isProjectRulesEnabled()) {
+ if (input.rule == null) {
+ throw new BadRequestException("rule is required");
+ }
+ if (!rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
- SubmitRuleOptions opts =
- SubmitRuleOptions.builder()
- .skipFilters(input.filters == Filters.SKIP)
- .rule(input.rule)
- .logErrors(false)
- .build();
-
ProjectState projectState = projectCache.get(rsrc.getProject());
if (projectState == null) {
throw new BadRequestException("project not found");
}
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- List<SubmitRecord> records;
- if (projectState.hasPrologRules() || input.rule != null) {
- records = ImmutableList.copyOf(prologRule.evaluate(cd, opts));
- } else {
- // No rules were provided as input and we have no rules.pl. This means we are supposed to run
- // the default rules. Nowadays, the default rules are implemented in Java, not Prolog.
- // Therefore, we call the DefaultRuleEvaluator instead.
- records = ImmutableList.copyOf(defaultSubmitRule.evaluate(cd, opts));
- }
-
+ List<SubmitRecord> records =
+ ImmutableList.copyOf(
+ prologRule.evaluate(
+ cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP)));
List<TestSubmitRuleInfo> out = Lists.newArrayListWithCapacity(records.size());
AccountLoader accounts = accountInfoFactory.create(true);
for (SubmitRecord r : records) {
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
index 9e8ee67..cb52fcb 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
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.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -28,6 +29,8 @@
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.rules.PrologOptions;
+import com.google.gerrit.server.rules.PrologRule;
import com.google.gerrit.server.rules.RulesCache;
import com.google.inject.Inject;
import org.kohsuke.args4j.Option;
@@ -35,19 +38,16 @@
public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
- private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
+ private final PrologRule prologRule;
@Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN;
@Inject
- TestSubmitType(
- ChangeData.Factory changeDataFactory,
- RulesCache rules,
- SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
+ TestSubmitType(ChangeData.Factory changeDataFactory, RulesCache rules, PrologRule prologRule) {
this.changeDataFactory = changeDataFactory;
this.rules = rules;
- this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
+ this.prologRule = prologRule;
}
@Override
@@ -56,21 +56,18 @@
if (input == null) {
input = new TestSubmitRuleInput();
}
- if (input.rule != null && !rules.isProjectRulesEnabled()) {
+ if (input.rule == null) {
+ throw new BadRequestException("rule is required");
+ }
+ if (!rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
- SubmitRuleOptions opts =
- SubmitRuleOptions.builder()
- .logErrors(false)
- .skipFilters(input.filters == Filters.SKIP)
- .rule(input.rule)
- .build();
-
- SubmitRuleEvaluator evaluator = submitRuleEvaluatorFactory.create(opts);
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- SubmitTypeRecord rec = evaluator.getSubmitType(cd);
+ SubmitTypeRecord rec =
+ prologRule.getSubmitType(
+ cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP));
if (rec.status != SubmitTypeRecord.Status.OK) {
throw new BadRequestException(String.format("rule produced invalid result: %s", rec));
@@ -80,17 +77,30 @@
}
public static class Get implements RestReadView<RevisionResource> {
- private final TestSubmitType test;
+ private final ChangeData.Factory changeDataFactory;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject
- Get(TestSubmitType test) {
- this.test = test;
+ Get(
+ ChangeData.Factory changeDataFactory,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
+ this.changeDataFactory = changeDataFactory;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
@Override
public Response<SubmitType> apply(RevisionResource resource)
- throws AuthException, BadRequestException {
- return test.apply(resource, null);
+ throws AuthException, ResourceConflictException {
+ SubmitRuleEvaluator evaluator =
+ submitRuleEvaluatorFactory.create(SubmitRuleOptions.defaults());
+ ChangeData cd = changeDataFactory.create(resource.getNotes());
+ SubmitTypeRecord rec = evaluator.getSubmitType(cd);
+
+ if (rec.status != SubmitTypeRecord.Status.OK) {
+ throw new ResourceConflictException(String.format("rule produced invalid result: %s", rec));
+ }
+
+ return Response.ok(rec.type);
}
}
}
diff --git a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
index 8401c1d..ee997b2 100644
--- a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
+++ b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
@@ -26,7 +26,6 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -63,7 +62,7 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// In case at least one project has a rules.pl file, we let Prolog handle it.
diff --git a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
index 4695800..ff5d99e 100644
--- a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
+++ b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
@@ -28,7 +28,6 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -60,7 +59,7 @@
IgnoreSelfApprovalRule() {}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
List<LabelType> labelTypes;
List<PatchSetApproval> approvals;
try {
diff --git a/java/com/google/gerrit/server/rules/PrologOptions.java b/java/com/google/gerrit/server/rules/PrologOptions.java
new file mode 100644
index 0000000..da9b3ab
--- /dev/null
+++ b/java/com/google/gerrit/server/rules/PrologOptions.java
@@ -0,0 +1,57 @@
+// 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.rules;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import java.util.Optional;
+
+@AutoValue
+public abstract class PrologOptions {
+ public static PrologOptions defaultOptions() {
+ return new AutoValue_PrologOptions.Builder().logErrors(true).skipFilters(false).build();
+ }
+
+ public static PrologOptions dryRunOptions(String ruleToTest, boolean skipFilters) {
+ return new AutoValue_PrologOptions.Builder()
+ .logErrors(false)
+ .skipFilters(skipFilters)
+ .rule(ruleToTest)
+ .build();
+ }
+
+ /** Whether errors should be logged. */
+ abstract boolean logErrors();
+
+ /** Whether Prolog filters from parent projects should be skipped. */
+ abstract boolean skipFilters();
+
+ /**
+ * Prolog rule that should be run. If not given, the Prolog rule that is configured for the
+ * project is used (the rule from rules.pl in refs/meta/config).
+ */
+ abstract Optional<String> rule();
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract PrologOptions.Builder logErrors(boolean logErrors);
+
+ abstract PrologOptions.Builder skipFilters(boolean skipFilters);
+
+ abstract PrologOptions.Builder rule(@Nullable String rule);
+
+ abstract PrologOptions build();
+ }
+}
diff --git a/java/com/google/gerrit/server/rules/PrologRule.java b/java/com/google/gerrit/server/rules/PrologRule.java
index 0c54f40..e15b4b5 100644
--- a/java/com/google/gerrit/server/rules/PrologRule.java
+++ b/java/com/google/gerrit/server/rules/PrologRule.java
@@ -18,7 +18,6 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -37,21 +36,29 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions opts) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// We only want to run the Prolog engine if we have at least one rules.pl file to use.
- if ((projectState == null || !projectState.hasPrologRules()) && opts.rule() == null) {
+ if ((projectState == null || !projectState.hasPrologRules())) {
return Collections.emptyList();
}
+ return evaluate(cd, PrologOptions.defaultOptions());
+ }
+
+ public Collection<SubmitRecord> evaluate(ChangeData cd, PrologOptions opts) {
return getEvaluator(cd, opts).evaluate();
}
- private PrologRuleEvaluator getEvaluator(ChangeData cd, SubmitRuleOptions opts) {
- return factory.create(cd, opts);
+ public SubmitTypeRecord getSubmitType(ChangeData cd) {
+ return getSubmitType(cd, PrologOptions.defaultOptions());
}
- public SubmitTypeRecord getSubmitType(ChangeData cd, SubmitRuleOptions opts) {
+ public SubmitTypeRecord getSubmitType(ChangeData cd, PrologOptions opts) {
return getEvaluator(cd, opts).getSubmitType();
}
+
+ private PrologRuleEvaluator getEvaluator(ChangeData cd, PrologOptions opts) {
+ return factory.create(cd, opts);
+ }
}
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index c036c86..7f6450d 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RuleEvalException;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -73,7 +72,7 @@
public interface Factory {
/** Returns a new {@link PrologRuleEvaluator} with the specified options */
- PrologRuleEvaluator create(ChangeData cd, SubmitRuleOptions options);
+ PrologRuleEvaluator create(ChangeData cd, PrologOptions options);
}
/**
@@ -95,7 +94,7 @@
private final PrologEnvironment.Factory envFactory;
private final ChangeData cd;
private final ProjectState projectState;
- private final SubmitRuleOptions opts;
+ private final PrologOptions opts;
private Term submitRule;
@AssistedInject
@@ -107,7 +106,7 @@
PrologEnvironment.Factory envFactory,
ProjectCache projectCache,
@Assisted ChangeData cd,
- @Assisted SubmitRuleOptions options) {
+ @Assisted PrologOptions options) {
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
@@ -159,12 +158,6 @@
return ruleError("Error looking up change " + cd.getId(), e);
}
- if (!opts.allowClosed() && change.isClosed()) {
- SubmitRecord rec = new SubmitRecord();
- rec.status = SubmitRecord.Status.CLOSED;
- return Collections.singletonList(rec);
- }
-
List<Term> results;
try {
results =
@@ -465,22 +458,22 @@
PrologEnvironment env;
try {
PrologMachineCopy pmc;
- if (opts.rule() == null) {
+ if (opts.rule().isPresent()) {
+ pmc = rulesCache.loadMachine("stdin", new StringReader(opts.rule().get()));
+ } else {
pmc =
rulesCache.loadMachine(
projectState.getNameKey(), projectState.getConfig().getRulesId());
- } else {
- pmc = rulesCache.loadMachine("stdin", new StringReader(opts.rule()));
}
env = envFactory.create(pmc);
} catch (CompileException err) {
String msg;
- if (opts.rule() == null) {
+ if (opts.rule().isPresent()) {
+ msg = err.getMessage();
+ } else {
msg =
String.format(
"Cannot load rules.pl for %s: %s", projectState.getName(), err.getMessage());
- } else {
- msg = err.getMessage();
}
throw new RuleEvalException(msg, err);
}
diff --git a/java/com/google/gerrit/server/rules/SubmitRule.java b/java/com/google/gerrit/server/rules/SubmitRule.java
index 2a68683..20cb8fb 100644
--- a/java/com/google/gerrit/server/rules/SubmitRule.java
+++ b/java/com/google/gerrit/server/rules/SubmitRule.java
@@ -15,7 +15,6 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import java.util.Collection;
@@ -40,5 +39,5 @@
@ExtensionPoint
public interface SubmitRule {
/** Returns a {@link Collection} of {@link SubmitRecord} status for the change. */
- Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options);
+ Collection<SubmitRecord> evaluate(ChangeData changeData);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
index f087b78..1842a9ec 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
@@ -26,7 +26,6 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.SubmitRequirementInfo;
import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.Module;
@@ -67,7 +66,7 @@
private static class CustomSubmitRule implements SubmitRule {
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData changeData) {
SubmitRecord record = new SubmitRecord();
record.labels = new ArrayList<>();
record.status = SubmitRecord.Status.NOT_READY;
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index 66af8a4..0ae99b0 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -284,6 +284,7 @@
ObjectId ps1Id = forceFetch(PatchSet.id(id, 1).toRefName());
ObjectId ps2Id = testRepo.amend(ps1Id).add("file", "content").create();
PushResult r = push(ps2Id.name() + ":refs/for/master");
+ // Admin had ADD_PATCH_SET removed in setup.
assertThat(r)
.onlyRef("refs/for/master")
.isRejected("cannot add patch set to " + id.get() + ".");
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 56a9b69..15b9a93 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -46,7 +46,6 @@
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.project.CreateProjectArgs;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
@@ -680,7 +679,7 @@
boolean failOnce;
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData changeData) {
if (failOnce) {
failOnce = false;
throw new IllegalStateException("forced failure from test");
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
index 83782c9..37237c6 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
@@ -22,7 +22,6 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.rules.IgnoreSelfApprovalRule;
import com.google.inject.Inject;
import java.util.Collection;
@@ -42,8 +41,7 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).hasSize(1);
SubmitRecord result = submitRecords.iterator().next();
@@ -69,8 +67,7 @@
// Approve as admin
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).isEmpty();
}
@@ -81,8 +78,7 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).isEmpty();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
index c6f2024..efc3b5b 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
@@ -21,8 +21,8 @@
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.rules.PrologOptions;
import com.google.gerrit.server.rules.PrologRuleEvaluator;
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
@@ -156,6 +156,6 @@
}
private PrologRuleEvaluator makeEvaluator() {
- return evaluatorFactory.create(makeChangeData(), SubmitRuleOptions.defaults());
+ return evaluatorFactory.create(makeChangeData(), PrologOptions.defaultOptions());
}
}
diff --git a/javatests/com/google/gerrit/json/JsonEnumMappingTest.java b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
index 6e57b01..dd710f9 100644
--- a/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
+++ b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
@@ -71,10 +71,6 @@
private static class TestData {
TestEnum value;
-
- public TestData(TestEnum value) {
- this.value = value;
- }
}
private enum TestEnum {
diff --git a/plugins/replication b/plugins/replication
index a5a5e0c..90253f8 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit a5a5e0cd13f1ff2614d77e9bf1bacbbc1d61b696
+Subproject commit 90253f88e42653a0c3af632beae507847f71adec
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 077af1f..8500e3b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -159,6 +159,11 @@
},
_computePreferences(account, preferences) {
+ // Polymer 2: check for undefined
+ if ([account, preferences].some(arg => arg === undefined)) {
+ return;
+ }
+
this.changeTableColumns = this.columnNames;
if (account) {
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 90641df..54e2edd 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
@@ -590,6 +590,15 @@
_actionsChanged(actionsChangeRecord, revisionActionsChangeRecord,
additionalActionsChangeRecord) {
+ // Polymer 2: check for undefined
+ if ([
+ actionsChangeRecord,
+ revisionActionsChangeRecord,
+ additionalActionsChangeRecord,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
const additionalActions = (additionalActionsChangeRecord &&
additionalActionsChangeRecord.base) || [];
this.hidden = this._keyCount(actionsChangeRecord) === 0 &&
@@ -620,6 +629,15 @@
_editStatusChanged(editMode, editPatchsetLoaded,
editBasedOnCurrentPatchSet, disableEdit) {
+ // Polymer 2: check for undefined
+ if ([
+ editMode,
+ editBasedOnCurrentPatchSet,
+ disableEdit,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
if (disableEdit) {
this._deleteAndNotify('publishEdit');
this._deleteAndNotify('rebaseEdit');
@@ -1344,6 +1362,17 @@
*/
_computeAllActions(changeActionsRecord, revisionActionsRecord,
primariesRecord, additionalActionsRecord, change) {
+ // Polymer 2: check for undefined
+ if ([
+ changeActionsRecord,
+ revisionActionsRecord,
+ primariesRecord,
+ additionalActionsRecord,
+ change,
+ ].some(arg => arg === undefined)) {
+ return [];
+ }
+
const revisionActionValues = this._getActionValues(revisionActionsRecord,
primariesRecord, additionalActionsRecord, ActionType.REVISION);
const changeActionValues = this._getActionValues(changeActionsRecord,
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 154fc36..6ee5a24 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -250,6 +250,7 @@
_computeTopicReadOnly(mutable, change) {
return !mutable ||
+ !change ||
!change.actions ||
!change.actions.topic ||
!change.actions.topic.enabled;
@@ -257,6 +258,7 @@
_computeHashtagReadOnly(mutable, change) {
return !mutable ||
+ !change ||
!change.actions ||
!change.actions.hashtags ||
!change.actions.hashtags.enabled;
@@ -264,6 +266,7 @@
_computeAssigneeReadOnly(mutable, change) {
return !mutable ||
+ !change ||
!change.actions ||
!change.actions.assignee ||
!change.actions.assignee.enabled;
@@ -297,7 +300,7 @@
* the push validation.
*/
_computePushCertificateValidation(serverConfig, change) {
- if (!serverConfig || !serverConfig.receive ||
+ if (!change || !serverConfig || !serverConfig.receive ||
!serverConfig.receive.enable_signed_push) {
return null;
}
@@ -438,7 +441,7 @@
},
_computeParents(change) {
- if (!change.current_revision ||
+ if (!change || !change.current_revision ||
!change.revisions[change.current_revision] ||
!change.revisions[change.current_revision].commit) {
return undefined;
@@ -447,13 +450,13 @@
},
_computeParentsLabel(parents) {
- return parents.length > 1 ? 'Parents' : 'Parent';
+ return parents && parents.length > 1 ? 'Parents' : 'Parent';
},
_computeParentListClass(parents, parentIsCurrent) {
return [
'parentList',
- parents.length > 1 ? 'merge' : 'nonMerge',
+ parents && parents.length > 1 ? 'merge' : 'nonMerge',
parentIsCurrent ? 'current' : 'notCurrent',
].join(' ');
},
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 b37a72e..6b667c8 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
@@ -60,6 +60,7 @@
};
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
+ const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
Polymer({
@@ -303,6 +304,7 @@
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
'_patchNumChanged(_patchRange.patchNum)',
+ '_loadDynamicTabHeaderAndContent(_change, _selectedRevision)',
],
keyboardShortcuts() {
@@ -337,17 +339,6 @@
this._setDiffViewMode();
});
- Gerrit.awaitPluginsLoaded().then(() => {
- this._dynamicTabHeaderEndpoints =
- Gerrit._endpoints.getDynamicEndpoints('change-view-tab-header');
- this._dynamicTabContentEndpoints =
- Gerrit._endpoints.getDynamicEndpoints('change-view-tab-content');
- if (this._dynamicTabContentEndpoints.length
- !== this._dynamicTabHeaderEndpoints.length) {
- console.warn('Different number of tab headers and tab content.');
- }
- });
-
this.addEventListener('comment-save', this._handleCommentSave.bind(this));
this.addEventListener('comment-refresh', this._reloadDrafts.bind(this));
this.addEventListener('comment-discard',
@@ -464,8 +455,20 @@
},
_computeChangeStatusChips(change, mergeable) {
+ // Polymer 2: check for undefined
+ if ([
+ change,
+ mergeable,
+ ].some(arg => arg === undefined)) {
+ // To keep consistent with Polymer 1, we are returning undefined
+ // if not all dependencies are defined
+ return undefined;
+ }
+
// Show no chips until mergeability is loaded.
- if (mergeable === null || mergeable === undefined) { return []; }
+ if (mergeable === null) {
+ return [];
+ }
const options = {
includeDerived: true,
@@ -476,7 +479,8 @@
},
_computeHideEditCommitMessage(loggedIn, editing, change, editMode) {
- if (!loggedIn || editing || change.status === this.ChangeStatus.MERGED ||
+ if (!loggedIn || editing ||
+ (change && change.status === this.ChangeStatus.MERGED) ||
editMode) {
return true;
}
@@ -770,7 +774,47 @@
});
},
- _paramsAndChangeChanged(value) {
+ /**
+ * We use an observer to observe 'change' and 'selectedRevision'
+ * variables. This fixes an issue under Polymer 2 so that the dynamic
+ * plugins loads when these variables load.
+ */
+ _loadDynamicTabHeaderAndContent(change, selectedRevision) {
+ // These vars are unused, but because primaryTabs extension point
+ // uses it, we makes sure we doin't load the plugin until these vars
+ // exist.
+ if (!change || !selectedRevision) return;
+
+ // We cache the _dynamicTabHeaderEndpoints and _dynamicTabContentEndpoints
+ // var so that we doin't keep loading the same dynamic plugin
+ // over and over when 'change' or 'selectedRevision' change.
+ if (this._dynamicTabHeaderEndpoints || this._dynamicTabContentEndpoints) {
+ return;
+ }
+
+ Gerrit.awaitPluginsLoaded().then(() => {
+ this._dynamicTabHeaderEndpoints =
+ Gerrit._endpoints.getDynamicEndpoints('change-view-tab-header');
+ this._dynamicTabContentEndpoints =
+ Gerrit._endpoints.getDynamicEndpoints('change-view-tab-content');
+ if (this._dynamicTabContentEndpoints.length
+ !== this._dynamicTabHeaderEndpoints.length) {
+ console.warn('Different number of tab headers and tab content.');
+ }
+ }).then(() => {
+ // We need a second then(..) to ensure that the dynamic endpoints
+ // are created before we call _performPostLoadTasks(). This ensures it has
+ // enough time before the primary tab gets selected.
+ this._performPostLoadTasks();
+ });
+ },
+
+ _paramsAndChangeChanged(value, change) {
+ // Polymer 2: check for undefined
+ if ([value, change].some(arg => arg === undefined)) {
+ return;
+ }
+
// If the change number or patch range is different, then reset the
// selected file index.
const patchRangeState = this.viewState.patchRange;
@@ -910,7 +954,7 @@
},
_computeShowPrimaryTabs(dynamicTabHeaderEndpoints) {
- return dynamicTabHeaderEndpoints.length > 0;
+ return dynamicTabHeaderEndpoints && dynamicTabHeaderEndpoints.length > 0;
},
_computeChangeUrl(change) {
@@ -943,6 +987,11 @@
},
_computeChangeIdCommitMessageError(commitMessage, change) {
+ // Polymer 2: check for undefined
+ if ([commitMessage, change].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (!commitMessage) { return CHANGE_ID_ERROR.MISSING; }
// Find the last match in the commit message:
@@ -997,6 +1046,11 @@
},
_computeReplyButtonLabel(changeRecord, canStartReview) {
+ // Polymer 2: check for undefined
+ if ([changeRecord, canStartReview].some(arg => arg === undefined)) {
+ return 'Reply';
+ }
+
if (canStartReview) {
return 'Start review';
}
@@ -1358,15 +1412,17 @@
/**
* Reload the change.
- * @param {boolean=} opt_reloadRelatedChanges Reloads the related chanegs
- * when true.
+ * @param {boolean=} opt_isLocationChange Reloads the related changes
+ * when true and ends reporting events that started on location change.
* @return {Promise} A promise that resolves when the core data has loaded.
* Some non-core data loading may still be in-flight when the core data
* promise resolves.
*/
- _reload(opt_reloadRelatedChanges) {
+ _reload(opt_isLocationChange) {
this._loading = true;
this._relatedChangesCollapsed = true;
+ this.$.reporting.time(CHANGE_RELOAD_TIMING_LABEL);
+ this.$.reporting.time(CHANGE_DATA_TIMING_LABEL);
// Array to house all promises related to data requests.
const allDataPromises = [];
@@ -1380,7 +1436,12 @@
// change content may start appearing.
const loadingFlagSet = detailCompletes
.then(() => { this._loading = false; })
- .then(() => { this.$.reporting.changeDisplayed(); });
+ .then(() => {
+ this.$.reporting.timeEnd(CHANGE_RELOAD_TIMING_LABEL);
+ if (opt_isLocationChange) {
+ this.$.reporting.changeDisplayed();
+ }
+ });
// Resolves when the project config has loaded.
const projectConfigLoaded = detailCompletes
@@ -1440,16 +1501,17 @@
coreDataPromise = mergeabilityLoaded;
}
- if (opt_reloadRelatedChanges) {
+ if (opt_isLocationChange) {
const relatedChangesLoaded = coreDataPromise
.then(() => this.$.relatedChanges.reload());
allDataPromises.push(relatedChangesLoaded);
}
- this.$.reporting.time(CHANGE_DATA_TIMING_LABEL);
Promise.all(allDataPromises).then(() => {
this.$.reporting.timeEnd(CHANGE_DATA_TIMING_LABEL);
- this.$.reporting.changeFullyLoaded();
+ if (opt_isLocationChange) {
+ this.$.reporting.changeFullyLoaded();
+ }
});
return coreDataPromise;
@@ -1699,6 +1761,10 @@
},
_computeEditMode(patchRangeRecord, paramsRecord) {
+ if ([patchRangeRecord, paramsRecord].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (paramsRecord.base && paramsRecord.base.edit) { return true; }
const patchRange = patchRangeRecord.base || {};
@@ -1784,7 +1850,7 @@
},
_computeCurrentRevision(currentRevision, revisions) {
- return revisions && revisions[currentRevision];
+ return currentRevision && revisions && revisions[currentRevision];
},
_computeDiffPrefsDisabled(disableDiffPrefs, loggedIn) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index e8de93b..79d8d93 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1844,5 +1844,50 @@
MockInteractions.tap(element.$.changeStar.$$('button'));
assert.isTrue(stub.called);
});
+
+ suite('gr-reporting tests', () => {
+ setup(() => {
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 1,
+ };
+ sandbox.stub(element, '_getChangeDetail').returns(Promise.resolve());
+ sandbox.stub(element, '_getProjectConfig').returns(Promise.resolve());
+ sandbox.stub(element, '_reloadComments').returns(Promise.resolve());
+ sandbox.stub(element, '_getMergeability').returns(Promise.resolve());
+ sandbox.stub(element, '_getLatestCommitMessage')
+ .returns(Promise.resolve());
+ });
+
+ test('don\'t report changedDisplayed on reply', done => {
+ const changeDisplayStub =
+ sandbox.stub(element.$.reporting, 'changeDisplayed');
+ const changeFullyLoadedStub =
+ sandbox.stub(element.$.reporting, 'changeFullyLoaded');
+ element._handleReplySent();
+ flush(() => {
+ assert.isFalse(changeDisplayStub.called);
+ assert.isFalse(changeFullyLoadedStub.called);
+ done();
+ });
+ });
+
+ test('report changedDisplayed on _paramsChanged', done => {
+ const changeDisplayStub =
+ sandbox.stub(element.$.reporting, 'changeDisplayed');
+ const changeFullyLoadedStub =
+ sandbox.stub(element.$.reporting, 'changeFullyLoaded');
+ element._paramsChanged({
+ view: Gerrit.Nav.View.CHANGE,
+ changeNum: 101,
+ project: 'test-project',
+ });
+ flush(() => {
+ assert.isTrue(changeDisplayStub.called);
+ assert.isTrue(changeFullyLoadedStub.called);
+ done();
+ });
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
index ddab319..7e25180 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
@@ -47,11 +47,21 @@
},
_computeShowWebLink(change, commitInfo, serverConfig) {
+ // Polymer 2: check for undefined
+ if ([change, commitInfo, serverConfig].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const weblink = this._getWeblink(change, commitInfo, serverConfig);
return !!weblink && !!weblink.url;
},
_computeWebLink(change, commitInfo, serverConfig) {
+ // Polymer 2: check for undefined
+ if ([change, commitInfo, serverConfig].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const {url} = this._getWeblink(change, commitInfo, serverConfig) || {};
return url;
},
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
index d25a871..f271a70 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
@@ -121,6 +121,7 @@
},
],
};
+ element.serverConfig = {};
assert.isOk(element._computeShowWebLink(element.change,
element.commitInfo, element.serverConfig));
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
index f4523cf..2f098de 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
@@ -60,6 +60,15 @@
],
_computeMessage(changeStatus, commitNum, commitMessage) {
+ // Polymer 2: check for undefined
+ if ([
+ changeStatus,
+ commitNum,
+ commitMessage,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
let newMessage = commitMessage;
if (changeStatus === 'MERGED') {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js
index 1c3bcfb..22ce27e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js
@@ -144,6 +144,11 @@
* the corresponding value to be submitted.
*/
_updateSelectedOption(rebaseOnCurrent, hasParent) {
+ // Polymer 2: check for undefined
+ if ([rebaseOnCurrent, hasParent].some(arg => arg === undefined)) {
+ return;
+ }
+
if (this._displayParentOption(rebaseOnCurrent, hasParent)) {
this.$.rebaseOnParentInput.checked = true;
} else if (this._displayTipOption(rebaseOnCurrent, hasParent)) {
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 65dbeee..63586d0 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
@@ -157,6 +157,11 @@
},
_computeSchemes(change, patchNum) {
+ // Polymer 2: check for undefined
+ if ([change, patchNum].some(arg => arg === undefined)) {
+ return [];
+ }
+
for (const rev of Object.values(change.revisions || {})) {
if (this.patchNumEquals(rev._number, patchNum)) {
const fetch = rev.fetch;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
index a802fb9..d6bf92c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
@@ -133,10 +133,20 @@
},
_computeDescriptionReadOnly(loggedIn, change, account) {
+ // Polymer 2: check for undefined
+ if ([loggedIn, change, account].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return !(loggedIn && (account._account_id === change.owner._account_id));
},
_computePatchSetDescription(change, patchNum) {
+ // Polymer 2: check for undefined
+ if ([change, patchNum].some(arg => arg === undefined)) {
+ return;
+ }
+
const rev = this.getRevisionByPatchNum(change.revisions, patchNum);
this._patchsetDescription = (rev && rev.description) ?
rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 4f160e4..37beff4 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -817,6 +817,17 @@
},
_computeFiles(filesByPath, changeComments, patchRange, reviewed, loading) {
+ // Polymer 2: check for undefined
+ if ([
+ filesByPath,
+ changeComments,
+ patchRange,
+ reviewed,
+ loading,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
// Await all promises resolving from reload. @See Issue 9057
if (loading || !changeComments) { return; }
@@ -836,6 +847,11 @@
},
_computeFilesShown(numFilesShown, files) {
+ // Polymer 2: check for undefined
+ if ([numFilesShown, files].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const previousNumFilesShown = this._shownFiles ?
this._shownFiles.length : 0;
@@ -904,6 +920,11 @@
},
_computePatchSetDescription(revisions, patchNum) {
+ // Polymer 2: check for undefined
+ if ([revisions, patchNum].some(arg => arg === undefined)) {
+ return '';
+ }
+
const rev = this.getRevisionByPatchNum(revisions, patchNum);
return (rev && rev.description) ?
rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
index fe28a1d..9c3d69a 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
@@ -148,6 +148,11 @@
},
_computePermittedLabelValues(permittedLabels, label) {
+ // Polymer 2: check for undefined
+ if ([permittedLabels, label].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return permittedLabels[label];
},
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.js b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.js
index f18680e..548798f 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.js
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.js
@@ -82,7 +82,12 @@
return null;
},
- _computeLabels(labelRecord) {
+ _computeLabels(labelRecord, account) {
+ // Polymer 2: check for undefined
+ if ([labelRecord, account].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const labelsObj = labelRecord.base;
if (!labelsObj) { return []; }
return Object.keys(labelsObj).sort().map(key => {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index e0e3725..4072508 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -144,7 +144,7 @@
},
_computeShowReplyButton(message, loggedIn) {
- return !!message.message && loggedIn &&
+ return message && !!message.message && loggedIn &&
!this._computeIsAutomated(message);
},
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index 81c3322..13c1754 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -117,6 +117,11 @@
},
_computeItems(messages, reviewerUpdates) {
+ // Polymer 2: check for undefined
+ if ([messages, reviewerUpdates].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
messages = messages || [];
reviewerUpdates = reviewerUpdates || [];
let mi = 0;
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
index eeec437..aa56550 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
@@ -292,6 +292,17 @@
_resultsChanged(related, submittedTogether, conflicts,
cherryPicks, sameTopic) {
+ // Polymer 2: check for undefined
+ if ([
+ related,
+ submittedTogether,
+ conflicts,
+ cherryPicks,
+ sameTopic,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
const results = [
related && related.changes,
submittedTogether && submittedTogether.changes,
@@ -314,6 +325,11 @@
},
_computeConnectedRevisions(change, patchNum, relatedChanges) {
+ // Polymer 2: check for undefined
+ if ([change, patchNum, relatedChanges].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const connected = [];
let changeRevision;
if (!change) { return []; }
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index cd42606..3b97439 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -331,8 +331,8 @@
let account;
// Remove any accounts that already exist as a CC.
for (const splice of splices.indexSplices) {
- for (const addedKey of splice.addedKeys) {
- account = this.get(`_reviewers.${addedKey}`);
+ for (let i = 0; i < splice.addedCount; i++) {
+ account = splice.object[splice.index + i];
key = this._accountOrGroupKey(account);
index = this._ccs.findIndex(
account => this._accountOrGroupKey(account) === key);
@@ -623,6 +623,11 @@
},
_changeUpdated(changeRecord, owner) {
+ // Polymer 2: check for undefined
+ if ([changeRecord, owner].some(arg => arg === undefined)) {
+ return;
+ }
+
this._rebuildReviewerArrays(changeRecord.base, owner);
},
@@ -856,6 +861,19 @@
_computeSendButtonDisabled(buttonLabel, drafts, text, reviewersMutated,
labelsChanged, includeComments, disabled) {
+ // Polymer 2: check for undefined
+ if ([
+ buttonLabel,
+ drafts,
+ text,
+ reviewersMutated,
+ labelsChanged,
+ includeComments,
+ disabled,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (disabled) { return true; }
if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
const hasDrafts = includeComments && Object.keys(drafts).length;
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 e137eef..665ca63 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
@@ -1093,20 +1093,84 @@
test('_computeSendButtonDisabled', () => {
const fn = element._computeSendButtonDisabled.bind(element);
- assert.isFalse(fn('Start review'));
- assert.isTrue(fn('Send', {}, '', false, false, false));
+ assert.isFalse(fn(
+ /* buttonLabel= */ 'Start review',
+ /* drafts= */ {},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
+ assert.isTrue(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
// Mock nonempty comment draft array, with seding comments.
- assert.isFalse(fn('Send', {file: ['draft']}, '', false, false, true));
+ assert.isFalse(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {file: ['draft']},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ true,
+ /* disabled= */ false,
+ ));
// Mock nonempty comment draft array, without seding comments.
- assert.isTrue(fn('Send', {file: ['draft']}, '', false, false, false));
+ assert.isTrue(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {file: ['draft']},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
// Mock nonempty change message.
- assert.isFalse(fn('Send', {}, 'test', false, false, false));
+ assert.isFalse(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {},
+ /* text= */ 'test',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
// Mock reviewers mutated.
- assert.isFalse(fn('Send', {}, '', true, false, false));
+ assert.isFalse(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {},
+ /* text= */ '',
+ /* reviewersMutated= */ true,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
// Mock labels changed.
- assert.isFalse(fn('Send', {}, '', false, true, false));
+ assert.isFalse(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ true,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ ));
// Whole dialog is disabled.
- assert.isTrue(fn('Send', {}, '', false, true, false, true));
+ assert.isTrue(fn(
+ /* buttonLabel= */ 'Send',
+ /* drafts= */ {},
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ true,
+ /* includeComments= */ false,
+ /* disabled= */ true,
+ ));
});
test('_submit blocked when no mutations exist', () => {
@@ -1141,4 +1205,4 @@
assert.equal(element.$.pluginMessage.textContent, 'foo');
});
});
-</script>
+</script>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
index 43ea7f0..42bcd09 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
@@ -165,6 +165,11 @@
},
_reviewersChanged(changeRecord, owner) {
+ // Polymer 2: check for undefined
+ if ([changeRecord, owner].some(arg => arg === undefined)) {
+ return;
+ }
+
let result = [];
const reviewers = changeRecord.base;
for (const key in reviewers) {
@@ -194,6 +199,11 @@
},
_computeHiddenCount(reviewers, displayedReviewers) {
+ // Polymer 2: check for undefined
+ if ([reviewers, displayedReviewers].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return reviewers.length - displayedReviewers.length;
},
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
index 8f15a06..407ef0f 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
@@ -94,6 +94,15 @@
},
_computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly) {
+ // Polymer 2: check for undefined
+ if ([
+ sortedThreads,
+ unresolvedOnly,
+ draftsOnly,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return sortedThreads.filter(c => {
if (draftsOnly) {
return c.hasDraft;
diff --git a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.js b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.js
index c652658..1c9013e 100644
--- a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.js
@@ -82,6 +82,15 @@
_computeFetchCommand(revision, preferredDownloadCommand,
preferredDownloadScheme) {
+ // Polymer 2: check for undefined
+ if ([
+ revision,
+ preferredDownloadCommand,
+ preferredDownloadScheme,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (!revision) { return; }
if (!revision || !revision.fetch) { return; }
diff --git a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog_test.html b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog_test.html
index 66d0a01..577b978 100644
--- a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog_test.html
@@ -75,31 +75,40 @@
assert.isUndefined(element._computeFetchCommand({fetch: {}}));
});
+ test('not all defined', () => {
+ assert.isUndefined(
+ element._computeFetchCommand(testRev, undefined, ''));
+ assert.isUndefined(
+ element._computeFetchCommand(testRev, '', undefined));
+ assert.isUndefined(
+ element._computeFetchCommand(undefined, '', ''));
+ });
+
test('insufficiently defined scheme', () => {
assert.isUndefined(
- element._computeFetchCommand(testRev, undefined, 'badscheme'));
+ element._computeFetchCommand(testRev, '', 'badscheme'));
const rev = Object.assign({}, testRev);
rev.fetch = Object.assign({}, testRev.fetch, {nocmds: {commands: {}}});
assert.isUndefined(
- element._computeFetchCommand(rev, undefined, 'nocmds'));
+ element._computeFetchCommand(rev, '', 'nocmds'));
rev.fetch.nocmds.commands.unsupported = 'unsupported';
assert.isUndefined(
- element._computeFetchCommand(rev, undefined, 'nocmds'));
+ element._computeFetchCommand(rev, '', 'nocmds'));
});
test('default scheme and command', () => {
- const cmd = element._computeFetchCommand(testRev);
+ const cmd = element._computeFetchCommand(testRev, '', '');
assert.isTrue(cmd === 'http checkout' || cmd === 'ssh pull');
});
test('default command', () => {
assert.strictEqual(
- element._computeFetchCommand(testRev, undefined, 'http'),
+ element._computeFetchCommand(testRev, '', 'http'),
'http checkout');
assert.strictEqual(
- element._computeFetchCommand(testRev, undefined, 'ssh'),
+ element._computeFetchCommand(testRev, '', 'ssh'),
'ssh pull');
});
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
index af4510d..2ade782 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
@@ -66,6 +66,11 @@
},
_getLinks(switchAccountUrl, path) {
+ // Polymer 2: check for undefined
+ if ([switchAccountUrl, path].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const links = [{name: 'Settings', url: '/settings/'}];
if (switchAccountUrl) {
const replacements = {path};
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html
index 37d8882..e29faa8 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html
@@ -80,11 +80,15 @@
});
test('switch account', () => {
+ // Missing params.
+ assert.isUndefined(element._getLinks());
+ assert.isUndefined(element._getLinks(null));
+
// No switch account link.
- assert.equal(element._getLinks(null).length, 2);
+ assert.equal(element._getLinks(null, '').length, 2);
// Unparameterized switch account link.
- let links = element._getLinks('/switch-account');
+ let links = element._getLinks('/switch-account', '');
assert.equal(links.length, 3);
assert.deepEqual(links[1], {
name: 'Switch account',
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
index 77f9cd7..50ba9d0 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -181,6 +181,17 @@
},
_computeLinks(defaultLinks, userLinks, adminLinks, topMenus, docBaseUrl) {
+ // Polymer 2: check for undefined
+ if ([
+ defaultLinks,
+ userLinks,
+ adminLinks,
+ topMenus,
+ docBaseUrl,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const links = defaultLinks.map(menu => {
return {
title: menu.title,
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
index 7bf6728..ef14370 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
@@ -111,13 +111,24 @@
}];
// When no admin links are passed, it should use the default.
- assert.deepEqual(element._computeLinks(defaultLinks, [], adminLinks, []),
+ assert.deepEqual(element._computeLinks(
+ defaultLinks,
+ /* userLinks= */[],
+ adminLinks,
+ /* topMenus= */[],
+ /* docBaseUrl= */ '',
+ ),
defaultLinks.concat({
title: 'Browse',
links: adminLinks,
}));
- assert.deepEqual(
- element._computeLinks(defaultLinks, userLinks, adminLinks, []),
+ assert.deepEqual(element._computeLinks(
+ defaultLinks,
+ userLinks,
+ adminLinks,
+ /* topMenus= */[],
+ /* docBaseUrl= */ ''
+ ),
defaultLinks.concat([
{
title: 'Your',
@@ -126,7 +137,8 @@
{
title: 'Browse',
links: adminLinks,
- }]));
+ }])
+ );
});
test('documentation links', () => {
@@ -168,7 +180,13 @@
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
- assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ /* defaultLinks= */ [],
+ /* userLinks= */ [],
+ adminLinks,
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Browse',
links: adminLinks,
},
@@ -198,7 +216,13 @@
url: '/plugins/myplugin/index.html',
}],
}];
- assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ /* defaultLinks= */ [],
+ /* userLinks= */ [],
+ adminLinks,
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Browse',
links: adminLinks,
},
@@ -231,7 +255,13 @@
url: 'https://gerrit/plugins/plugin-manager/static/create.html',
}],
}];
- assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ /* defaultLinks= */ [],
+ /* userLinks= */ [],
+ adminLinks,
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Browse',
links: adminLinks,
}, {
@@ -262,7 +292,13 @@
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
- assert.deepEqual(element._computeLinks(defaultLinks, [], [], topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ defaultLinks,
+ /* userLinks= */ [],
+ /* adminLinks= */ [],
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Faves',
links: defaultLinks[0].links.concat([{
name: 'Manage',
@@ -287,7 +323,13 @@
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
- assert.deepEqual(element._computeLinks([], userLinks, [], topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ /* defaultLinks= */ [],
+ userLinks,
+ /* adminLinks= */ [],
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Your',
links: userLinks.concat([{
name: 'Manage',
@@ -312,7 +354,13 @@
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
- assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
+ assert.deepEqual(element._computeLinks(
+ /* defaultLinks= */ [],
+ /* userLinks= */ [],
+ adminLinks,
+ topMenus,
+ /* baseDocUrl= */ '',
+ ), [{
title: 'Browse',
links: adminLinks.concat([{
name: 'Manage',
@@ -350,4 +398,4 @@
assert.equal(element._registerText, 'Sign up');
});
});
-</script>
+ </script>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 5ddf97a..a019388 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -332,8 +332,11 @@
if (line.type !== GrDiffLine.Type.BLANK) {
td.classList.add('content');
}
- if (line.highlights.length === 0) {
- td.classList.add('no-highlights');
+
+ // If intraline info is not available, the entire line will be
+ // considered as changed and marked as dark red / green color
+ if (!line.hasIntralineInfo) {
+ td.classList.add('no-intraline-info');
}
td.classList.add(line.type);
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 2c14609..bce3d07 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
@@ -742,6 +742,15 @@
_whitespaceChanged(
preferredWhitespaceLevel, loadedWhitespaceLevel,
noRenderOnPrefsChange) {
+ // Polymer 2: check for undefined
+ if ([
+ preferredWhitespaceLevel,
+ loadedWhitespaceLevel,
+ noRenderOnPrefsChange,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
if (preferredWhitespaceLevel !== loadedWhitespaceLevel &&
!noRenderOnPrefsChange) {
this.reload();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index ccc3bb2..d4b4e2b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -437,7 +437,10 @@
if (type !== GrDiffLine.Type.ADD) line.beforeNumber = offsetLeft + i;
if (type !== GrDiffLine.Type.REMOVE) line.afterNumber = offsetRight + i;
if (opt_highlights) {
+ line.hasIntralineInfo = true;
line.highlights = opt_highlights.filter(hl => hl.contentIndex === i);
+ } else {
+ line.hasIntralineInfo = false;
}
return line;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index d604df2..eb38ba7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -260,6 +260,11 @@
},
_getFiles(changeNum, patchRangeRecord) {
+ // Polymer 2: check for undefined
+ if ([changeNum, patchRangeRecord].some(arg => arg === undefined)) {
+ return;
+ }
+
const patchRange = patchRangeRecord.base;
return this.$.restAPI.getChangeFilePathsAsSpeciallySortedArray(
changeNum, patchRange).then(files => {
@@ -680,6 +685,11 @@
},
_setReviewedObserver(_loggedIn, paramsRecord, _prefs) {
+ // Polymer 2: check for undefined
+ if ([_loggedIn, paramsRecord, _prefs].some(arg => arg === undefined)) {
+ return;
+ }
+
const params = paramsRecord.base || {};
if (!_loggedIn) { return; }
@@ -784,6 +794,15 @@
},
_formatFilesForDropdown(fileList, patchNum, changeComments) {
+ // Polymer 2: check for undefined
+ if ([
+ fileList,
+ patchNum,
+ changeComments,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+
if (!fileList) { return; }
const dropdownContent = [];
for (const path of fileList) {
@@ -921,6 +940,15 @@
},
_computeCommentSkips(commentMap, fileList, path) {
+ // Polymer 2: check for undefined
+ if ([
+ commentMap,
+ fileList,
+ path,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const skips = {previous: null, next: null};
if (!fileList.length) { return skips; }
const pathIndex = fileList.indexOf(path);
@@ -1001,6 +1029,11 @@
},
_computeFileNum(file, files) {
+ // Polymer 2: check for undefined
+ if ([file, files].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return files.findIndex(({value}) => value === file) + 1;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
index 48bb6e0..b64385d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
@@ -30,9 +30,14 @@
/** @type {number|string} */
this.beforeNumber = opt_beforeLine || 0;
+
/** @type {number|string} */
this.afterNumber = opt_afterLine || 0;
+ /** @type {boolean} */
+ this.hasIntralineInfo = false;
+
+ /** @type Array<GrDiffLine.Highlights> */
this.highlights = [];
/** @type {?Array<Object>} ?Array<!GrDiffGroup> */
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 4c96732..2d16a4b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -131,8 +131,8 @@
width: var(--content-width, 80ch);
}
.content.add .intraline,
- /* If there are no intraline changes, consider everything changed */
- .content.add.no-highlights,
+ /* If there are no intraline info, consider everything changed */
+ .content.add.no-intraline-info,
.delta.total .content.add {
background-color: var(--dark-add-highlight-color);
}
@@ -140,8 +140,8 @@
background-color: var(--light-add-highlight-color);
}
.content.remove .intraline,
- /* If there are no intraline changes, consider everything changed */
- .content.remove.no-highlights,
+ /* If there are no intraline info, consider everything changed */
+ .content.remove.no-intraline-info,
.delta.total .content.remove {
background-color: var(--dark-remove-highlight-color);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 1bafe48..c69b313 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -297,6 +297,11 @@
},
_enableSelectionObserver(loggedIn, isAttached) {
+ // Polymer 2: check for undefined
+ if ([loggedIn, isAttached].some(arg => arg === undefined)) {
+ return;
+ }
+
if (loggedIn && isAttached) {
this.listen(document, 'selectionchange', '_handleSelectionChange');
this.listen(document, 'mouseup', '_handleMouseUp');
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index ea8aa47..a9fde2b 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -64,6 +64,17 @@
_computeBaseDropdownContent(availablePatches, patchNum, _sortedRevisions,
changeComments, revisionInfo) {
+ // Polymer 2: check for undefined
+ if ([
+ availablePatches,
+ patchNum,
+ _sortedRevisions,
+ changeComments,
+ revisionInfo,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const parentCounts = revisionInfo.getParentCountMap();
const currentParentCount = parentCounts.hasOwnProperty(patchNum) ?
parentCounts[patchNum] : 1;
@@ -107,6 +118,16 @@
_computePatchDropdownContent(availablePatches, basePatchNum,
_sortedRevisions, changeComments) {
+ // Polymer 2: check for undefined
+ if ([
+ availablePatches,
+ basePatchNum,
+ _sortedRevisions,
+ changeComments,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
const dropdownContent = [];
for (const patch of availablePatches) {
const patchNum = patch.num;
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
index b32c012..08dcf51 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
@@ -210,6 +210,15 @@
},
_computeSaveDisabled(content, newContent, saving) {
+ // Polymer 2: check for undefined
+ if ([
+ content,
+ newContent,
+ saving,
+ ].some(arg => arg === undefined)) {
+ return true;
+ }
+
if (saving) {
return true;
}
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
index c8f34ad..246662d 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
@@ -149,6 +149,14 @@
},
_computeUsernameMutable(config, username) {
+ // Polymer 2: check for undefined
+ if ([
+ config,
+ username,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
// Username may not be changed once it is set.
return config.auth.editable_account_fields.includes('USER_NAME') &&
!username;
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
index a403ee6..3ce226f 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
@@ -124,6 +124,14 @@
},
_computeUsernameMutable(config, username) {
+ // Polymer 2: check for undefined
+ if ([
+ config,
+ username,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
return config.auth.editable_account_fields.includes('USER_NAME') &&
!username;
},
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
index 5603391..7f0a754 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
@@ -159,6 +159,9 @@
],
attached() {
+ // Polymer 2: anchor tag won't work on shadow DOM
+ // we need to manually calling scrollIntoView when hash changed
+ this.listen(window, 'location-change', '_handleLocationChange');
this.fire('title-change', {title: 'Settings'});
this._isDark = !!window.localStorage.getItem('dark-theme');
@@ -215,9 +218,28 @@
this._loadingPromise = Promise.all(promises).then(() => {
this._loading = false;
+
+ // Handle anchor tag for initial load
+ this._handleLocationChange();
});
},
+ detached() {
+ this.unlisten(window, 'location-change', '_handleLocationChange');
+ },
+
+ _handleLocationChange() {
+ // Handle anchor tag after dom attached
+ const urlHash = window.location.hash;
+ if (urlHash) {
+ // Use shadowRoot for Polymer 2
+ const elem = (this.shadowRoot || document).querySelector(urlHash);
+ if (elem) {
+ elem.scrollIntoView();
+ }
+ }
+ },
+
reloadAccountDetail() {
Promise.all([
this.$.accountInfo.loadData(),
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
index 7983fad..0f914fb 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
@@ -67,6 +67,14 @@
},
_computeAccountTitle(account, tooltip) {
+ // Polymer 2: check for undefined
+ if ([
+ account,
+ tooltip,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (!account) { return; }
let result = '';
if (this._computeName(account, this._serverConfig)) {
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
index d3f29dc..513b67a 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
@@ -68,31 +68,34 @@
{
name: 'Andrew Bonventre',
email: 'andybons+gerrit@gmail.com',
- }),
+ }, /* additionalText= */ ''),
'Andrew Bonventre <andybons+gerrit@gmail.com>');
assert.equal(element._computeAccountTitle(
- {name: 'Andrew Bonventre'}),
+ {name: 'Andrew Bonventre'}, /* additionalText= */ ''),
'Andrew Bonventre');
assert.equal(element._computeAccountTitle(
{
email: 'andybons+gerrit@gmail.com',
- }),
+ }, /* additionalText= */ ''),
'Anonymous <andybons+gerrit@gmail.com>');
assert.equal(element._computeShowEmailClass(
{
name: 'Andrew Bonventre',
email: 'andybons+gerrit@gmail.com',
- }), '');
+ }, /* additionalText= */ ''), '');
assert.equal(element._computeShowEmailClass(
{
email: 'andybons+gerrit@gmail.com',
- }), 'showEmail');
+ }, /* additionalText= */ ''), 'showEmail');
- assert.equal(element._computeShowEmailClass({name: 'Andrew Bonventre'}),
+ assert.equal(element._computeShowEmailClass(
+ {name: 'Andrew Bonventre'},
+ /* additionalText= */ '',
+ ),
'');
assert.equal(element._computeShowEmailClass(undefined), '');
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index c36ec21..34f1ff9 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -242,8 +242,13 @@
},
_updateSuggestions(text, threshold, noDebounce) {
+ // Polymer 2: check for undefined
+ if ([text, threshold, noDebounce].some(arg => arg === undefined)) {
+ return;
+ }
+
if (this._disableSuggestions) { return; }
- if (text === undefined || text.length < threshold) {
+ if (text.length < threshold) {
this._suggestions = [];
this.value = '';
return;
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 57835a3..cf2f57d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
@@ -211,6 +211,11 @@
},
_calculateActionstoShow(showActions, isRobotComment) {
+ // Polymer 2: check for undefined
+ if ([showActions, isRobotComment].some(arg => arg === undefined)) {
+ return;
+ }
+
this._showHumanActions = showActions && !isRobotComment;
this._showRobotActions = showActions && isRobotComment;
},
@@ -601,6 +606,11 @@
},
_loadLocalDraft(changeNum, patchNum, comment) {
+ // Polymer 2: check for undefined
+ if ([changeNum, patchNum, comment].some(arg => arg === undefined)) {
+ return;
+ }
+
// Only apply local drafts to comments that haven't been saved
// remotely, and haven't been given a default message already.
//
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
index 4d7f2bb..a39e170 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
@@ -173,6 +173,14 @@
},
_computeFullDateStr(dateStr, timeFormat) {
+ // Polymer 2: check for undefined
+ if ([
+ dateStr,
+ timeFormat,
+ ].some(arg => arg === undefined)) {
+ return undefined;
+ }
+
if (!dateStr) { return ''; }
const date = moment(util.parseDate(dateStr));
if (!date.isValid()) { return ''; }
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
index bcf3729..b6d909d 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
@@ -104,6 +104,11 @@
},
_handleValueChange(value, items) {
+ // Polymer 2: check for undefined
+ if ([value, items].some(arg => arg === undefined)) {
+ return;
+ }
+
if (!value) { return; }
const selectedObj = items.find(item => {
return item.value + '' === value + '';
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
index 18318e9..29c2f6e 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
@@ -118,6 +118,15 @@
},
_computeSaveDisabled(disabled, content, newContent) {
+ // Polymer 2: check for undefined
+ if ([
+ disabled,
+ content,
+ newContent,
+ ].some(arg => arg === undefined)) {
+ return true;
+ }
+
return disabled || (content === newContent);
},
diff --git a/polygerrit-ui/app/elements/shared/gr-limited-text/gr-limited-text.js b/polygerrit-ui/app/elements/shared/gr-limited-text/gr-limited-text.js
index 44a8791..479dd1e 100644
--- a/polygerrit-ui/app/elements/shared/gr-limited-text/gr-limited-text.js
+++ b/polygerrit-ui/app/elements/shared/gr-limited-text/gr-limited-text.js
@@ -66,6 +66,11 @@
* enabled.
*/
_updateTitle(text, limit, tooltipLimit) {
+ // Polymer 2: check for undefined
+ if ([text, limit, tooltipLimit].some(arg => arg === undefined)) {
+ return;
+ }
+
this.hasTooltip = !!limit && !!text && text.length > limit;
if (this.hasTooltip) {
this.setAttribute('title', text.substr(0, tooltipLimit));
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index faf28ad..305a6b8 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -289,7 +289,7 @@
// Any path prefixes that should resolve to index.html.
var (
- fePaths = []string{"/q/", "/c/", "/p/", "/x/", "/dashboard/", "/admin/"}
+ fePaths = []string{"/q/", "/c/", "/p/", "/x/", "/dashboard/", "/admin/", "/settings/"}
issueNumRE = regexp.MustCompile(`^\/\d+\/?$`)
)
