Merge "Add ExternalIdUpsertPreprocessor interface"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 410bf42..0caebfc 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1342,6 +1342,7 @@
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
+ "disable_keyboard_shortcuts": true,
"diff_view": "SIDE_BY_SIDE",
"mute_common_path_prefixes": true,
"my": [
@@ -1392,6 +1393,7 @@
"size_bar_in_change_table": true,
"diff_view": "SIDE_BY_SIDE",
"publish_comments_on_push": true,
+ "disable_keyboard_shortcuts": true,
"work_in_progress_by_default": true,
"mute_common_path_prefixes": true,
"my": [
@@ -2883,6 +2885,8 @@
The base which should be pre-selected in the 'Diff Against' drop-down
list when the change screen is opened for a merge commit.
Allowed values are `AUTO_MERGE` and `FIRST_PARENT`.
+|`disable_keyboard_shortcuts` |not set if `false`|
+Whether to disable all keyboard shortcuts.
|============================================
[[query-limit-info]]
diff --git a/java/com/google/gerrit/entities/LabelTypes.java b/java/com/google/gerrit/entities/LabelTypes.java
index 1c38c59..55a9976 100644
--- a/java/com/google/gerrit/entities/LabelTypes.java
+++ b/java/com/google/gerrit/entities/LabelTypes.java
@@ -20,6 +20,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
public class LabelTypes {
protected List<LabelType> labelTypes;
@@ -36,12 +37,12 @@
return labelTypes;
}
- public LabelType byLabel(LabelId labelId) {
- return byLabel().get(labelId.get().toLowerCase());
+ public Optional<LabelType> byLabel(LabelId labelId) {
+ return Optional.ofNullable(byLabel().get(labelId.get().toLowerCase()));
}
- public LabelType byLabel(String labelName) {
- return byLabel().get(labelName.toLowerCase());
+ public Optional<LabelType> byLabel(String labelName) {
+ return Optional.ofNullable(byLabel().get(labelName.toLowerCase()));
}
private Map<String, LabelType> byLabel() {
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD
index cd3ebb9..ea7c609 100644
--- a/java/com/google/gerrit/httpd/BUILD
+++ b/java/com/google/gerrit/httpd/BUILD
@@ -20,6 +20,7 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/audit",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 7e6ab58..375aa54 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -114,6 +114,7 @@
import com.google.gerrit.server.audit.ExtendedHttpAuditEvent;
import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.cancellation.RequestCancelledException;
+import com.google.gerrit.server.cancellation.RequestStateProvider;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -712,50 +713,45 @@
messageOr(e, "Quota limit reached"),
e.caching(),
e);
- } catch (RequestCancelledException e) {
- cause = Optional.of(e);
- switch (e.getCancellationReason()) {
- case CLIENT_CLOSED_REQUEST:
- statusCode = SC_CLIENT_CLOSED_REQUEST;
- break;
- case CLIENT_PROVIDED_DEADLINE_EXCEEDED:
- case SERVER_DEADLINE_EXCEEDED:
- statusCode = SC_REQUEST_TIMEOUT;
- break;
- }
-
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append("\n\n");
- msg.append(e.getCancellationMessage().get());
- }
-
- responseBytes = replyError(req, res, statusCode, msg.toString(), e);
} catch (Exception e) {
cause = Optional.of(e);
- statusCode = SC_INTERNAL_SERVER_ERROR;
- Optional<ExceptionHook.Status> status = getStatus(e);
- statusCode = status.map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
-
- if (res.isCommitted()) {
- responseBytes = 0;
- if (statusCode == SC_INTERNAL_SERVER_ERROR) {
- logger.atSevere().withCause(e).log(
- "Error in %s %s, response already committed", req.getMethod(), uriForLogging(req));
- } else {
- logger.atWarning().log(
- "Response for %s %s already committed, wanted to set status %d",
- req.getMethod(), uriForLogging(req), statusCode);
- }
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (requestCancelledException.isPresent()) {
+ statusCode =
+ getCancellationStatusCode(requestCancelledException.get().getCancellationReason());
+ responseBytes =
+ replyError(
+ req, res, statusCode, getCancellationMessage(requestCancelledException.get()), e);
} else {
- res.reset();
- traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+ statusCode = SC_INTERNAL_SERVER_ERROR;
- if (status.isPresent()) {
- responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
+ Optional<ExceptionHook.Status> status = getStatus(e);
+ statusCode =
+ status.map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
+
+ if (res.isCommitted()) {
+ responseBytes = 0;
+ if (statusCode == SC_INTERNAL_SERVER_ERROR) {
+ logger.atSevere().withCause(e).log(
+ "Error in %s %s, response already committed",
+ req.getMethod(), uriForLogging(req));
+ } else {
+ logger.atWarning().log(
+ "Response for %s %s already committed, wanted to set status %d",
+ req.getMethod(), uriForLogging(req), statusCode);
+ }
} else {
- responseBytes = replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
+ res.reset();
+ traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+
+ if (status.isPresent()) {
+ responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
+ } else {
+ responseBytes =
+ replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
+ }
}
}
} finally {
@@ -1961,6 +1957,28 @@
return replyBinaryResult(req, res, BinaryResult.create(text).setContentType(PLAIN_TEXT));
}
+ private static int getCancellationStatusCode(RequestStateProvider.Reason cancellationReason) {
+ switch (cancellationReason) {
+ case CLIENT_CLOSED_REQUEST:
+ return SC_CLIENT_CLOSED_REQUEST;
+ case CLIENT_PROVIDED_DEADLINE_EXCEEDED:
+ case SERVER_DEADLINE_EXCEEDED:
+ return SC_REQUEST_TIMEOUT;
+ }
+ logger.atSevere().log("Unexpected cancellation reason: %s", cancellationReason);
+ return SC_INTERNAL_SERVER_ERROR;
+ }
+
+ private static String getCancellationMessage(
+ RequestCancelledException requestCancelledException) {
+ StringBuilder msg = new StringBuilder(requestCancelledException.formatCancellationReason());
+ if (requestCancelledException.getCancellationMessage().isPresent()) {
+ msg.append("\n\n");
+ msg.append(requestCancelledException.getCancellationMessage().get());
+ }
+ return msg.toString();
+ }
+
private static boolean acceptsGzip(HttpServletRequest req) {
if (req != null) {
String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING);
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index d60bc8f..326ddf4 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -151,8 +151,10 @@
ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
for (PatchSetApproval ap : approvalsUtil.byPatchSet(notes, change.currentPatchSetId())) {
- LabelType type = projectState.getLabelTypes(notes).byLabel(ap.label());
- if (type != null && ap.value() == 1 && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
+ Optional<LabelType> type = projectState.getLabelTypes(notes).byLabel(ap.label());
+ if (type.isPresent()
+ && ap.value() == 1
+ && type.get().getFunction() == LabelFunction.PATCH_SET_LOCK) {
return true;
}
}
diff --git a/java/com/google/gerrit/server/approval/ApprovalInference.java b/java/com/google/gerrit/server/approval/ApprovalInference.java
index 1efbd37..8d409e5 100644
--- a/java/com/google/gerrit/server/approval/ApprovalInference.java
+++ b/java/com/google/gerrit/server/approval/ApprovalInference.java
@@ -52,6 +52,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
+import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -397,14 +398,14 @@
if (resultByUser.contains(psa.label(), psa.accountId())) {
continue;
}
- LabelType type = labelTypes.byLabel(psa.labelId());
+ Optional<LabelType> type = labelTypes.byLabel(psa.labelId());
// Only compute modified files if there is a relevant label, since this is expensive.
if (modifiedFiles == null
- && type != null
- && type.isCopyAllScoresIfListOfFilesDidNotChange()) {
+ && type.isPresent()
+ && type.get().isCopyAllScoresIfListOfFilesDidNotChange()) {
modifiedFiles = listModifiedFiles(project, ps, priorPatchSet);
}
- if (type == null) {
+ if (!type.isPresent()) {
logger.atFine().log(
"approval %d on label %s of patch set %d of change %d cannot be copied"
+ " to patch set %d because the label no longer exists on project %s",
@@ -416,8 +417,8 @@
project.getName());
continue;
}
- if (!canCopyBasedOnBooleanLabelConfigs(project, psa, ps.id(), kind, type, modifiedFiles)
- && !canCopyBasedOnCopyCondition(notes, psa, ps.id(), type, kind)) {
+ if (!canCopyBasedOnBooleanLabelConfigs(project, psa, ps.id(), kind, type.get(), modifiedFiles)
+ && !canCopyBasedOnCopyCondition(notes, psa, ps.id(), type.get(), kind)) {
continue;
}
resultByUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
diff --git a/java/com/google/gerrit/server/approval/ApprovalsUtil.java b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
index b1e85e9..a1cdd99 100644
--- a/java/com/google/gerrit/server/approval/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
@@ -61,6 +61,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -299,8 +300,12 @@
List<PatchSetApproval> cells = new ArrayList<>(approvals.size());
Date ts = update.getWhen();
for (Map.Entry<String, Short> vote : approvals.entrySet()) {
- LabelType lt = labelTypes.byLabel(vote.getKey());
- cells.add(newApproval(ps.id(), user, lt.getLabelId(), vote.getValue(), ts).build());
+ Optional<LabelType> lt = labelTypes.byLabel(vote.getKey());
+ if (!lt.isPresent()) {
+ throw new BadRequestException(
+ String.format("label \"%s\" is not a configured label", vote.getKey()));
+ }
+ cells.add(newApproval(ps.id(), user, lt.get().getLabelId(), vote.getValue(), ts).build());
}
for (PatchSetApproval psa : cells) {
update.putApproval(psa.label(), psa.value());
@@ -310,11 +315,11 @@
public static void checkLabel(LabelTypes labelTypes, String name, Short value)
throws BadRequestException {
- LabelType label = labelTypes.byLabel(name);
- if (label == null) {
+ Optional<LabelType> label = labelTypes.byLabel(name);
+ if (!label.isPresent()) {
throw new BadRequestException(String.format("label \"%s\" is not a configured label", name));
}
- if (label.getValue(value) == null) {
+ if (label.get().getValue(value) == null) {
throw new BadRequestException(
String.format("label \"%s\": %d is not a valid value", name, value));
}
diff --git a/java/com/google/gerrit/server/cancellation/BUILD b/java/com/google/gerrit/server/cancellation/BUILD
new file mode 100644
index 0000000..05530a5
--- /dev/null
+++ b/java/com/google/gerrit/server/cancellation/BUILD
@@ -0,0 +1,14 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "cancellation",
+ srcs = glob(
+ ["*.java"],
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/common:annotations",
+ "//lib:guava",
+ "//lib/commons:lang",
+ ],
+)
diff --git a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
index 3c668fb..d89701f 100644
--- a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
+++ b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.cancellation;
+import com.google.common.base.Throwables;
import com.google.gerrit.common.Nullable;
import java.util.Optional;
import org.apache.commons.lang.WordUtils;
@@ -22,6 +23,17 @@
public class RequestCancelledException extends RuntimeException {
private static final long serialVersionUID = 1L;
+ /**
+ * Checks whether the given exception was caused by {@link RequestCancelledException}. If yes, the
+ * {@link RequestCancelledException} is returned. If not, {@link Optional#empty()} is returned.
+ */
+ public static Optional<RequestCancelledException> getFromCausalChain(Throwable e) {
+ return Throwables.getCausalChain(e).stream()
+ .filter(RequestCancelledException.class::isInstance)
+ .map(RequestCancelledException.class::cast)
+ .findFirst();
+ }
+
private final RequestStateProvider.Reason cancellationReason;
private final Optional<String> cancellationMessage;
diff --git a/java/com/google/gerrit/server/cancellation/RequestStateContext.java b/java/com/google/gerrit/server/cancellation/RequestStateContext.java
new file mode 100644
index 0000000..183d779
--- /dev/null
+++ b/java/com/google/gerrit/server/cancellation/RequestStateContext.java
@@ -0,0 +1,136 @@
+// Copyright (C) 2021 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.cancellation;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Context that allows to register {@link RequestStateProvider}s.
+ *
+ * <p>The registered {@link RequestStateProvider}s are stored in {@link ThreadLocal} so that they
+ * can be accessed during the request execution (via {@link #getRequestStateProviders()}.
+ *
+ * <p>On {@link #close()} the {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext} instance are removed from {@link ThreadLocal}.
+ *
+ * <p>Nesting {@code RequestStateContext}s is possible.
+ *
+ * <p>Currently there is no logic to automatically copy the {@link RequestStateContext} to
+ * background threads, but implementing this may be considered in the future. This means that by
+ * default we only support cancellation of the main thread, but not of background threads. That's
+ * fine as all significant work is being done in the main thread.
+ *
+ * <p>{@link com.google.gerrit.server.util.RequestContext} is also a context that is available for
+ * the time of the request, but it is not suitable to manage registrations of {@link
+ * RequestStateProvider}s. Hence {@link RequestStateProvider} registrations are managed by a
+ * separate context, which is this class, {@link RequestStateContext}:
+ *
+ * <ul>
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is an interface that has many
+ * implementations and hence cannot manage a {@link ThreadLocal} state.
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is not an {@link AutoCloseable} and
+ * hence cannot cleanup any {@link ThreadLocal} state on close (turning it into an {@link
+ * AutoCloseable} would require a large refactoring).
+ * <li>Despite the name {@link com.google.gerrit.server.util.RequestContext} is not only used for
+ * requests scopes but also for other scopes that are not a request (e.g. plugin invocations,
+ * email sending, manual scopes).
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is not copied to background and should
+ * not be, but for {@link RequestStateContext} we may consider doing this in the future.
+ * </ul>
+ */
+public class RequestStateContext implements AutoCloseable {
+ /** The {@link RequestStateProvider}s that have been registered for the thread. */
+ private static final ThreadLocal<Set<RequestStateProvider>> threadLocalRequestStateProviders =
+ new ThreadLocal<>();
+
+ /**
+ * Aborts the current request by throwing a {@link RequestCancelledException} if any of the
+ * registered {@link RequestStateProvider}s reports the request as cancelled.
+ *
+ * @throws RequestCancelledException thrown if the current request is cancelled and should be
+ * aborted
+ */
+ public static void abortIfCancelled() throws RequestCancelledException {
+ getRequestStateProviders()
+ .forEach(
+ requestStateProvider ->
+ requestStateProvider.checkIfCancelled(
+ (reason, message) -> {
+ throw new RequestCancelledException(reason, message);
+ }));
+ }
+
+ /** Returns the {@link RequestStateProvider}s that have been registered for the thread. */
+ @VisibleForTesting
+ static ImmutableSet<RequestStateProvider> getRequestStateProviders() {
+ if (threadLocalRequestStateProviders.get() == null) {
+ return ImmutableSet.of();
+ }
+ return ImmutableSet.copyOf(threadLocalRequestStateProviders.get());
+ }
+
+ /** Opens a {@code RequestStateContext}. */
+ public static RequestStateContext open() {
+ return new RequestStateContext();
+ }
+
+ /**
+ * The {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext}.
+ */
+ private Set<RequestStateProvider> requestStateProviders = new HashSet<>();
+
+ private RequestStateContext() {}
+
+ /**
+ * Registers a {@link RequestStateProvider}.
+ *
+ * @param requestStateProvider the {@link RequestStateProvider} that should be registered
+ * @return the {@code RequestStateContext} instance for chaining calls
+ */
+ public RequestStateContext addRequestStateProvider(RequestStateProvider requestStateProvider) {
+ if (threadLocalRequestStateProviders.get() == null) {
+ threadLocalRequestStateProviders.set(new HashSet<>());
+ }
+ if (threadLocalRequestStateProviders.get().add(requestStateProvider)) {
+ requestStateProviders.add(requestStateProvider);
+ }
+ return this;
+ }
+
+ /**
+ * Closes this {@code RequestStateContext}.
+ *
+ * <p>Ensures that all {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext} instance are removed from {@link #threadLocalRequestStateProviders}.
+ *
+ * <p>If no {@link RequestStateProvider}s remain in {@link #threadLocalRequestStateProviders},
+ * {@link #threadLocalRequestStateProviders} is unset.
+ */
+ @Override
+ public void close() {
+ if (threadLocalRequestStateProviders.get() != null) {
+ requestStateProviders.forEach(
+ requestStateProvider ->
+ threadLocalRequestStateProviders.get().remove(requestStateProvider));
+ if (threadLocalRequestStateProviders.get().isEmpty()) {
+ threadLocalRequestStateProviders.remove();
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/cancellation/RequestStateProvider.java b/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
index e1716eb..683ca1d 100644
--- a/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
+++ b/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
@@ -31,6 +31,7 @@
void checkIfCancelled(OnCancelled onCancelled);
/** Callback interface to be invoked if a request is cancelled. */
+ @FunctionalInterface
interface OnCancelled {
/**
* Callback that is invoked if the request is cancelled.
diff --git a/java/com/google/gerrit/server/change/LabelNormalizer.java b/java/com/google/gerrit/server/change/LabelNormalizer.java
index 30343d4..b5527d7 100644
--- a/java/com/google/gerrit/server/change/LabelNormalizer.java
+++ b/java/com/google/gerrit/server/change/LabelNormalizer.java
@@ -33,6 +33,7 @@
import com.google.inject.Singleton;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
/**
* Normalizes votes on labels according to project config.
@@ -101,12 +102,12 @@
unchanged.add(psa);
continue;
}
- LabelType label = labelTypes.byLabel(psa.labelId());
- if (label == null) {
+ Optional<LabelType> label = labelTypes.byLabel(psa.labelId());
+ if (!label.isPresent()) {
deleted.add(psa);
continue;
}
- PatchSetApproval copy = applyTypeFloor(label, psa);
+ PatchSetApproval copy = applyTypeFloor(label.get(), psa);
if (copy.value() != psa.value()) {
updated.add(copy);
} else {
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
index acff03c..5ce121b 100644
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -57,6 +57,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
@@ -103,9 +104,9 @@
for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels != null) {
for (SubmitRecord.Label r : rec.labels) {
- LabelType type = labelTypes.byLabel(r.label);
- if (type != null && (!isMerged || type.isAllowPostSubmit())) {
- toCheck.put(type.getName(), type);
+ Optional<LabelType> type = labelTypes.byLabel(r.label);
+ if (type.isPresent() && (!isMerged || type.get().isAllowPostSubmit())) {
+ toCheck.put(type.get().getName(), type.get());
}
}
}
@@ -120,18 +121,18 @@
continue;
}
for (SubmitRecord.Label r : rec.labels) {
- LabelType type = labelTypes.byLabel(r.label);
- if (type == null || (isMerged && !type.isAllowPostSubmit())) {
+ Optional<LabelType> type = labelTypes.byLabel(r.label);
+ if (!type.isPresent() || (isMerged && !type.get().isAllowPostSubmit())) {
continue;
}
- for (LabelValue v : type.getValues()) {
- boolean ok = can.contains(new LabelPermission.WithValue(type, v));
+ for (LabelValue v : type.get().getValues()) {
+ boolean ok = can.contains(new LabelPermission.WithValue(type.get(), v));
if (isMerged) {
if (labels == null) {
labels = currentLabels(filterApprovalsBy, cd);
}
- short prev = labels.getOrDefault(type.getName(), (short) 0);
+ short prev = labels.getOrDefault(type.get().getName(), (short) 0);
ok &= v.getValue() >= prev;
}
if (ok) {
@@ -176,21 +177,21 @@
setAllApprovals(accountLoader, cd, labels);
}
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
- LabelType type = labelTypes.byLabel(e.getKey());
- if (type == null) {
+ Optional<LabelType> type = labelTypes.byLabel(e.getKey());
+ if (!type.isPresent()) {
continue;
}
if (standard) {
for (PatchSetApproval psa : cd.currentApprovals()) {
- if (type.matches(psa)) {
+ if (type.get().matches(psa)) {
short val = psa.value();
Account.Id accountId = psa.accountId();
- setLabelScores(accountLoader, type, e.getValue(), val, accountId);
+ setLabelScores(accountLoader, type.get(), e.getValue(), val, accountId);
}
}
}
if (detailed) {
- setLabelValues(type, e.getValue());
+ setLabelValues(type.get(), e.getValue());
}
}
return labels;
@@ -261,9 +262,9 @@
MultimapBuilder.hashKeys().hashSetValues().build();
for (PatchSetApproval a : cd.currentApprovals()) {
allUsers.add(a.accountId());
- LabelType type = labelTypes.byLabel(a.labelId());
- if (type != null) {
- labelNames.add(type.getName());
+ Optional<LabelType> type = labelTypes.byLabel(a.labelId());
+ if (type.isPresent()) {
+ labelNames.add(type.get().getName());
// Not worth the effort to distinguish between votable/non-votable for 0
// values on closed changes, since they can't vote anyway.
current.put(a.accountId(), a);
@@ -292,8 +293,8 @@
if (detailed) {
labels.entrySet().stream()
- .filter(e -> labelTypes.byLabel(e.getKey()) != null)
- .forEach(e -> setLabelValues(labelTypes.byLabel(e.getKey()), e.getValue()));
+ .filter(e -> labelTypes.byLabel(e.getKey()).isPresent())
+ .forEach(e -> setLabelValues(labelTypes.byLabel(e.getKey()).get(), e.getValue()));
}
for (Account.Id accountId : allUsers) {
@@ -308,16 +309,16 @@
}
}
for (PatchSetApproval psa : current.get(accountId)) {
- LabelType type = labelTypes.byLabel(psa.labelId());
- if (type == null) {
+ Optional<LabelType> type = labelTypes.byLabel(psa.labelId());
+ if (!type.isPresent()) {
continue;
}
short val = psa.value();
- ApprovalInfo info = byLabel.get(type.getName());
+ ApprovalInfo info = byLabel.get(type.get().getName());
if (info != null) {
info.value = Integer.valueOf(val);
- info.permittedVotingRange = pvr.getOrDefault(type.getName(), null);
+ info.permittedVotingRange = pvr.getOrDefault(type.get().getName(), null);
info.date = psa.granted();
info.tag = psa.tag().orElse(null);
if (psa.postSubmit()) {
@@ -328,7 +329,7 @@
continue;
}
- setLabelScores(accountLoader, type, labels.get(type.getName()), val, accountId);
+ setLabelScores(accountLoader, type.get(), labels.get(type.get().getName()), val, accountId);
}
}
return labels;
@@ -428,24 +429,24 @@
PermissionBackend.ForChange perm = permissionBackend.absentUser(accountId).change(cd);
Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
- LabelType lt = labelTypes.byLabel(e.getKey());
- if (lt == null) {
+ Optional<LabelType> lt = labelTypes.byLabel(e.getKey());
+ if (!lt.isPresent()) {
// Ignore submit record for undefined label; likely the submit rule
// author didn't intend for the label to show up in the table.
continue;
}
Integer value;
- VotingRangeInfo permittedVotingRange = pvr.getOrDefault(lt.getName(), null);
+ VotingRangeInfo permittedVotingRange = pvr.getOrDefault(lt.get().getName(), null);
String tag = null;
Timestamp date = null;
- PatchSetApproval psa = current.get(accountId, lt.getName());
+ PatchSetApproval psa = current.get(accountId, lt.get().getName());
if (psa != null) {
value = Integer.valueOf(psa.value());
if (value == 0) {
// This may be a dummy approval that was inserted when the reviewer
// was added. Explicitly check whether the user can vote on this
// label.
- value = perm.test(new LabelPermission(lt)) ? 0 : null;
+ value = perm.test(new LabelPermission(lt.get())) ? 0 : null;
}
tag = psa.tag().orElse(null);
date = psa.granted();
@@ -456,7 +457,7 @@
// Either the user cannot vote on this label, or they were added as a
// reviewer but have not responded yet. Explicitly check whether the
// user can vote on this label.
- value = perm.test(new LabelPermission(lt)) ? 0 : null;
+ value = perm.test(new LabelPermission(lt.get())) ? 0 : null;
}
addApproval(
e.getValue().label(),
diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java
index d5b74a8..6189708 100644
--- a/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -38,6 +38,7 @@
import com.google.inject.Singleton;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
import java.util.TreeMap;
@Singleton
@@ -107,10 +108,8 @@
out.approvals = new TreeMap<>(labelTypes.nameComparator());
for (PatchSetApproval ca : approvals) {
- LabelType at = labelTypes.byLabel(ca.labelId());
- if (at != null) {
- out.approvals.put(at.getName(), formatValue(ca.value()));
- }
+ Optional<LabelType> at = labelTypes.byLabel(ca.labelId());
+ at.ifPresent(lt -> out.approvals.put(lt.getName(), formatValue(ca.value())));
}
// Add dummy approvals for all permitted labels for the user even if they
@@ -125,13 +124,13 @@
}
for (SubmitRecord.Label label : rec.labels) {
String name = label.label;
- LabelType type = labelTypes.byLabel(name);
- if (out.approvals.containsKey(name) || type == null) {
+ Optional<LabelType> type = labelTypes.byLabel(name);
+ if (out.approvals.containsKey(name) || !type.isPresent()) {
continue;
}
try {
- perm.check(new LabelPermission(type));
+ perm.check(new LabelPermission(type.get()));
out.approvals.put(name, formatValue((short) 0));
} catch (AuthException e) {
// Do nothing.
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 122aa26..201f122 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -167,6 +167,7 @@
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry;
import com.google.gerrit.server.notedb.NoteDbModule;
+import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
import com.google.gerrit.server.patch.DiffOperationsImpl;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.patch.PatchScriptFactory;
@@ -467,6 +468,7 @@
factory(MergedByPushOp.Factory.class);
factory(GitModules.Factory.class);
factory(VersionedAuthorizedKeys.Factory.class);
+ factory(StoreSubmitRequirementsOp.Factory.class);
bind(AccountManager.class);
bind(SubscriptionGraph.Factory.class).to(ConfiguredSubscriptionGraphFactory.class);
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 3f988a3..1bb694a 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -71,6 +71,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@@ -535,10 +536,8 @@
a.grantedOn = approval.granted().getTime() / 1000L;
a.oldValue = null;
- LabelType lt = labelTypes.byLabel(approval.labelId());
- if (lt != null) {
- a.description = lt.getName();
- }
+ Optional<LabelType> lt = labelTypes.byLabel(approval.labelId());
+ lt.ifPresent(l -> a.description = l.getName());
return a;
}
diff --git a/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
index 439f53e..edd1928 100644
--- a/java/com/google/gerrit/server/events/StreamEventsApiListener.java
+++ b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@@ -23,7 +23,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.LabelTypes;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -202,10 +201,7 @@
a.oldValue = Short.toString(oldApprovals.get(approval.getKey()));
}
}
- LabelType lt = labelTypes.byLabel(approval.getKey());
- if (lt != null) {
- a.description = lt.getName();
- }
+ labelTypes.byLabel(approval.getKey()).ifPresent(lt -> a.description = lt.getName());
if (approval.getValue() != null) {
a.value = Short.toString(approval.getValue());
}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 1da14f8..a5ea24d 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -600,11 +600,11 @@
} else if (isVerified(a.labelId())) {
tag = "Tested-by";
} else {
- final LabelType lt = project.getLabelTypes().byLabel(a.labelId());
- if (lt == null) {
+ final Optional<LabelType> lt = project.getLabelTypes().byLabel(a.labelId());
+ if (!lt.isPresent()) {
continue;
}
- tag = lt.getName();
+ tag = lt.get().getName();
}
if (!contains(footers, new FooterKey(tag), identbuf.toString())) {
diff --git a/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
index 2d854a5..1122551 100644
--- a/java/com/google/gerrit/server/git/MultiProgressMonitor.java
+++ b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
@@ -131,7 +131,7 @@
private int spinnerIndex;
private char spinnerState = NO_SPINNER;
private boolean done;
- private boolean write = true;
+ private boolean clientDisconnected;
private final long maxIntervalNanos;
@@ -343,14 +343,14 @@
}
private void send(StringBuilder s) {
- if (write) {
+ if (!clientDisconnected) {
try {
out.write(Constants.encode(s.toString()));
out.flush();
} catch (IOException e) {
logger.atWarning().withCause(e).log(
"Sending progress to client failed. Stop sending updates for task %s", taskName);
- write = false;
+ clientDisconnected = true;
}
}
}
diff --git a/java/com/google/gerrit/server/git/receive/BUILD b/java/com/google/gerrit/server/git/receive/BUILD
index b59d431..5c1cf52 100644
--- a/java/com/google/gerrit/server/git/receive/BUILD
+++ b/java/com/google/gerrit/server/git/receive/BUILD
@@ -18,6 +18,7 @@
"//java/com/google/gerrit/index",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/util/time",
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d074f1e..29c2698 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -46,6 +46,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableList;
@@ -645,10 +646,18 @@
try {
processCommandsUnsafe(commands, progress);
rejectRemaining(commands, INTERNAL_SERVER_ERROR);
- } catch (RequestCancelledException e) {
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append(String.format(" (%s)", e.getCancellationMessage().get()));
+ } catch (RuntimeException e) {
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (!requestCancelledException.isPresent()) {
+ Throwables.throwIfUnchecked(e);
+ }
+ StringBuilder msg =
+ new StringBuilder(requestCancelledException.get().formatCancellationReason());
+ if (requestCancelledException.get().getCancellationMessage().isPresent()) {
+ msg.append(
+ String.format(
+ " (%s)", requestCancelledException.get().getCancellationMessage().get()));
}
rejectRemaining(commands, msg.toString());
}
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index f00b48eb..b2a31b9 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -465,10 +465,10 @@
continue;
}
- LabelType lt = projectState.getLabelTypes().byLabel(a.labelId());
- if (lt != null) {
- current.put(lt.getName(), a);
- }
+ projectState
+ .getLabelTypes()
+ .byLabel(a.labelId())
+ .ifPresent(l -> current.put(l.getName(), a));
}
}
return current;
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 810cd4d..5caceef 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -70,6 +70,7 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.notedb.SubmitRequirementProtoConverter;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
@@ -605,7 +606,7 @@
for (PatchSetApproval a : cd.currentApprovals()) {
if (a.value() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
- LabelType labelType = cd.getLabelTypes().byLabel(a.labelId());
+ Optional<LabelType> labelType = cd.getLabelTypes().byLabel(a.labelId());
allApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, a.accountId()));
if (owners && cd.change().getOwner().equals(a.accountId())) {
allApprovals.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
@@ -622,13 +623,15 @@
}
private static List<String> getMaxMinAnyLabels(
- String label, short labelVal, LabelType labelType, @Nullable Account.Id accountId) {
+ String label, short labelVal, Optional<LabelType> labelType, @Nullable Account.Id accountId) {
List<String> labels = new ArrayList<>();
- if (labelVal == labelType.getMaxPositive()) {
- labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId));
- }
- if (labelVal == labelType.getMaxNegative()) {
- labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId));
+ if (labelType.isPresent()) {
+ if (labelVal == labelType.get().getMaxPositive()) {
+ labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId));
+ }
+ if (labelVal == labelType.get().getMaxNegative()) {
+ labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId));
+ }
}
labels.add(formatLabel(label, MagicLabelValue.ANY.name(), accountId));
return labels;
@@ -1076,6 +1079,27 @@
return result;
}
+ /** Serialized submit requirements, used for pre-populating results. */
+ public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_REQUIREMENTS =
+ storedOnly("full_submit_requirements")
+ .buildRepeatable(
+ cd ->
+ toProtos(
+ SubmitRequirementProtoConverter.INSTANCE, cd.submitRequirements().values()),
+ (cd, field) -> parseSubmitRequirements(field, cd));
+
+ private static void parseSubmitRequirements(Iterable<byte[]> values, ChangeData out) {
+ out.setSubmitRequirements(
+ StreamSupport.stream(values.spliterator(), false)
+ .map(
+ f ->
+ SubmitRequirementProtoConverter.INSTANCE.fromProto(
+ Protos.parseUnchecked(
+ SubmitRequirementProtoConverter.INSTANCE.getParser(), f)))
+ .collect(
+ ImmutableMap.toImmutableMap(sr -> sr.submitRequirement(), Function.identity())));
+ }
+
/**
* All values of all refs that were used in the course of indexing this document.
*
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 879da4f..bee0c35 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -148,10 +148,14 @@
* The computation of the {@link ChangeField#DIRECTORY} field is changed, hence reindexing is
* required.
*/
- static final Schema<ChangeData> V63 = schema(V62, false);
+ @Deprecated static final Schema<ChangeData> V63 = schema(V62, false);
/** Added support for MIN/MAX/ANY for {@link ChangeField#LABEL} */
- static final Schema<ChangeData> V64 = schema(V63, false);
+ @Deprecated static final Schema<ChangeData> V64 = schema(V63, false);
+
+ /** Added new field for submit requirements. */
+ static final Schema<ChangeData> V65 =
+ new Schema.Builder<ChangeData>().add(V64).add(ChangeField.STORED_SUBMIT_REQUIREMENTS).build();
/**
* Name of the change index to be used when contacting index backends or loading configurations.
diff --git a/java/com/google/gerrit/server/logging/BUILD b/java/com/google/gerrit/server/logging/BUILD
index c60af0d..ee0168c 100644
--- a/java/com/google/gerrit/server/logging/BUILD
+++ b/java/com/google/gerrit/server/logging/BUILD
@@ -9,6 +9,7 @@
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/util/time",
"//lib:gson",
"//lib:guava",
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 2fc19b5..681dfbc 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -24,6 +24,7 @@
import com.google.common.collect.Table;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.cancellation.RequestStateContext;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
@@ -208,6 +209,7 @@
}
private TraceTimer(Runnable startLogFn, Consumer<Long> doneLogFn) {
+ RequestStateContext.abortIfCancelled();
startLogFn.run();
this.doneLogFn = doneLogFn;
this.stopwatch = Stopwatch.createStarted();
@@ -217,6 +219,7 @@
public void close() {
stopwatch.stop();
doneLogFn.accept(stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ RequestStateContext.abortIfCancelled();
}
}
diff --git a/java/com/google/gerrit/server/mail/send/MergedSender.java b/java/com/google/gerrit/server/mail/send/MergedSender.java
index 6af2345..56528df 100644
--- a/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -79,14 +79,14 @@
Table<Account.Id, String, PatchSetApproval> pos = HashBasedTable.create();
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca : args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.id())) {
- LabelType lt = labelTypes.byLabel(ca.labelId());
- if (lt == null) {
+ Optional<LabelType> lt = labelTypes.byLabel(ca.labelId());
+ if (!lt.isPresent()) {
continue;
}
if (ca.value() > 0) {
- pos.put(ca.accountId(), lt.getName(), ca);
+ pos.put(ca.accountId(), lt.get().getName(), ca);
} else if (ca.value() < 0) {
- neg.put(ca.accountId(), lt.getName(), ca);
+ neg.put(ca.accountId(), lt.get().getName(), ca);
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index e7da025..4d6b9cf 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -61,14 +61,10 @@
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerStatusUpdateProto;
-import com.google.gerrit.server.cache.proto.Cache.SubmitRequirementResultProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
-import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementExpressionResultSerializer;
-import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementSerializer;
import com.google.gerrit.server.index.change.ChangeField.StoredSubmitRecord;
import com.google.gson.Gson;
-import com.google.protobuf.Descriptors.FieldDescriptor;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.List;
@@ -478,11 +474,6 @@
private static final Converter<String, ReviewerStateInternal> REVIEWER_STATE_CONVERTER =
Enums.stringConverter(ReviewerStateInternal.class);
- private static final FieldDescriptor SR_APPLICABILITY_EXPR_RESULT_FIELD =
- SubmitRequirementResultProto.getDescriptor().findFieldByNumber(2);
- private static final FieldDescriptor SR_OVERRIDE_EXPR_RESULT_FIELD =
- SubmitRequirementResultProto.getDescriptor().findFieldByNumber(4);
-
@Override
public byte[] serialize(ChangeNotesState object) {
checkArgument(object.metaId() != null, "meta ID is required in: %s", object);
@@ -539,7 +530,10 @@
object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c)));
object
.submitRequirementsResult()
- .forEach(sr -> b.addSubmitRequirementResult(toSubmitRequirementResultProto(sr)));
+ .forEach(
+ sr ->
+ b.addSubmitRequirementResult(
+ SubmitRequirementProtoConverter.INSTANCE.toProto(sr)));
b.setUpdateCount(object.updateCount());
if (object.mergedOn() != null) {
b.setMergedOnMillis(object.mergedOn().getTime());
@@ -634,53 +628,6 @@
return builder.build();
}
- private static SubmitRequirementResultProto toSubmitRequirementResultProto(
- SubmitRequirementResult r) {
- SubmitRequirementResultProto.Builder builder = SubmitRequirementResultProto.newBuilder();
- builder
- .setSubmitRequirement(SubmitRequirementSerializer.serialize(r.submitRequirement()))
- .setCommit(ObjectIdConverter.create().toByteString(r.patchSetCommitId()));
- if (r.applicabilityExpressionResult().isPresent()) {
- builder.setApplicabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.applicabilityExpressionResult().get()));
- }
- builder.setSubmittabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.submittabilityExpressionResult()));
- if (r.overrideExpressionResult().isPresent()) {
- builder.setOverrideExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.overrideExpressionResult().get()));
- }
- return builder.build();
- }
-
- private static SubmitRequirementResult toSubmitRequirementResult(
- SubmitRequirementResultProto proto) {
- SubmitRequirementResult.Builder builder =
- SubmitRequirementResult.builder()
- .patchSetCommitId(ObjectIdConverter.create().fromByteString(proto.getCommit()))
- .submitRequirement(
- SubmitRequirementSerializer.deserialize(proto.getSubmitRequirement()));
- if (proto.hasField(SR_APPLICABILITY_EXPR_RESULT_FIELD)) {
- builder.applicabilityExpressionResult(
- Optional.of(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getApplicabilityExpressionResult())));
- }
- builder.submittabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getSubmittabilityExpressionResult()));
- if (proto.hasField(SR_OVERRIDE_EXPR_RESULT_FIELD)) {
- builder.overrideExpressionResult(
- Optional.of(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getOverrideExpressionResult())));
- }
- return builder.build();
- }
-
@Override
public ChangeNotesState deserialize(byte[] in) {
ChangeNotesStateProto proto = Protos.parseUnchecked(ChangeNotesStateProto.parser(), in);
@@ -728,7 +675,7 @@
.collect(toImmutableListMultimap(HumanComment::getCommitId, c -> c)))
.submitRequirementsResult(
proto.getSubmitRequirementResultList().stream()
- .map(sr -> toSubmitRequirementResult(sr))
+ .map(sr -> SubmitRequirementProtoConverter.INSTANCE.fromProto(sr))
.collect(toImmutableList()))
.updateCount(proto.getUpdateCount())
.mergedOn(proto.getHasMergedOn() ? new Timestamp(proto.getMergedOnMillis()) : null);
diff --git a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
index 47948d7..4d99f93 100644
--- a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -15,16 +15,26 @@
package com.google.gerrit.server.notedb;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
+import com.google.inject.Inject;
/** A {@link BatchUpdateOp} that stores the evaluated submit requirements of a change in NoteDb. */
public class StoreSubmitRequirementsOp implements BatchUpdateOp {
private final ChangeData.Factory changeDataFactory;
+ private final SubmitRequirementsEvaluator evaluator;
- public StoreSubmitRequirementsOp(ChangeData.Factory changeDataFactory) {
+ public interface Factory {
+ StoreSubmitRequirementsOp create();
+ }
+
+ @Inject
+ public StoreSubmitRequirementsOp(
+ ChangeData.Factory changeDataFactory, SubmitRequirementsEvaluator evaluator) {
this.changeDataFactory = changeDataFactory;
+ this.evaluator = evaluator;
}
@Override
@@ -32,7 +42,7 @@
Change change = ctx.getChange();
ChangeData changeData = changeDataFactory.create(change);
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
- update.putSubmitRequirementResults(changeData.submitRequirements().values());
+ update.putSubmitRequirementResults(evaluator.getResults(changeData).values());
return !changeData.submitRequirements().isEmpty();
}
}
diff --git a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
new file mode 100644
index 0000000..416b850
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
@@ -0,0 +1,88 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import com.google.errorprone.annotations.Immutable;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.entities.converter.ProtoConverter;
+import com.google.gerrit.server.cache.proto.Cache.SubmitRequirementResultProto;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementExpressionResultSerializer;
+import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementSerializer;
+import com.google.protobuf.Descriptors.FieldDescriptor;
+import com.google.protobuf.Parser;
+import java.util.Optional;
+
+@Immutable
+public enum SubmitRequirementProtoConverter
+ implements ProtoConverter<SubmitRequirementResultProto, SubmitRequirementResult> {
+ INSTANCE;
+
+ private static final FieldDescriptor SR_APPLICABILITY_EXPR_RESULT_FIELD =
+ SubmitRequirementResultProto.getDescriptor().findFieldByNumber(2);
+ private static final FieldDescriptor SR_OVERRIDE_EXPR_RESULT_FIELD =
+ SubmitRequirementResultProto.getDescriptor().findFieldByNumber(4);
+
+ @Override
+ public SubmitRequirementResultProto toProto(SubmitRequirementResult r) {
+ SubmitRequirementResultProto.Builder builder = SubmitRequirementResultProto.newBuilder();
+ builder
+ .setSubmitRequirement(SubmitRequirementSerializer.serialize(r.submitRequirement()))
+ .setCommit(ObjectIdConverter.create().toByteString(r.patchSetCommitId()));
+ if (r.applicabilityExpressionResult().isPresent()) {
+ builder.setApplicabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(
+ r.applicabilityExpressionResult().get()));
+ }
+ builder.setSubmittabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(r.submittabilityExpressionResult()));
+ if (r.overrideExpressionResult().isPresent()) {
+ builder.setOverrideExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(
+ r.overrideExpressionResult().get()));
+ }
+ return builder.build();
+ }
+
+ @Override
+ public SubmitRequirementResult fromProto(SubmitRequirementResultProto proto) {
+ SubmitRequirementResult.Builder builder =
+ SubmitRequirementResult.builder()
+ .patchSetCommitId(ObjectIdConverter.create().fromByteString(proto.getCommit()))
+ .submitRequirement(
+ SubmitRequirementSerializer.deserialize(proto.getSubmitRequirement()));
+ if (proto.hasField(SR_APPLICABILITY_EXPR_RESULT_FIELD)) {
+ builder.applicabilityExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getApplicabilityExpressionResult())));
+ }
+ builder.submittabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getSubmittabilityExpressionResult()));
+ if (proto.hasField(SR_OVERRIDE_EXPR_RESULT_FIELD)) {
+ builder.overrideExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getOverrideExpressionResult())));
+ }
+ return builder.build();
+ }
+
+ @Override
+ public Parser<SubmitRequirementResultProto> getParser() {
+ return SubmitRequirementResultProto.parser();
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java b/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
index e33b261..62cfa47 100644
--- a/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
+++ b/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
@@ -18,6 +18,7 @@
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.PatchSet;
@@ -43,6 +44,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.stream.Collectors;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.lib.Config;
@@ -242,10 +244,9 @@
if (!patchSetApproval.label().equals(LabelId.CODE_REVIEW)) {
continue;
}
- if (!projectState
- .getLabelTypes(notes)
- .byLabel(patchSetApproval.labelId())
- .isMaxPositive(patchSetApproval)) {
+ Optional<LabelType> lt =
+ projectState.getLabelTypes(notes).byLabel(patchSetApproval.labelId());
+ if (!lt.isPresent() || !lt.get().isMaxPositive(patchSetApproval)) {
continue;
}
if (patchSetApproval.patchSetId().get() > maxPatchSetId.get()) {
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
index 65d9d9e..e66b974 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -14,6 +14,9 @@
package com.google.gerrit.server.project;
+import static com.google.gerrit.server.project.ProjectCache.illegalState;
+
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
@@ -26,16 +29,20 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.util.Map;
import java.util.Optional;
/** Evaluates submit requirements for different change data. */
@Singleton
public class SubmitRequirementsEvaluator {
private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
+ private final ProjectCache projectCache;
@Inject
- private SubmitRequirementsEvaluator(Provider<ChangeQueryBuilder> changeQueryBuilderProvider) {
+ private SubmitRequirementsEvaluator(
+ Provider<ChangeQueryBuilder> changeQueryBuilderProvider, ProjectCache projectCache) {
this.changeQueryBuilderProvider = changeQueryBuilderProvider;
+ this.projectCache = projectCache;
}
/**
@@ -50,6 +57,21 @@
changeQueryBuilderProvider.get().parse(expression.expressionString());
}
+ /**
+ * Evaluate and return all submit requirements for a change. Submit requirements are retrieved for
+ * the project containing the change and parent projects as well.
+ */
+ public Map<SubmitRequirement, SubmitRequirementResult> getResults(ChangeData cd) {
+ ProjectState state = projectCache.get(cd.project()).orElseThrow(illegalState(cd.project()));
+ Map<String, SubmitRequirement> requirements = state.getSubmitRequirements();
+ ImmutableMap.Builder<SubmitRequirement, SubmitRequirementResult> result =
+ ImmutableMap.builderWithExpectedSize(requirements.size());
+ for (SubmitRequirement requirement : requirements.values()) {
+ result.put(requirement, evaluate(requirement, cd));
+ }
+ return result.build();
+ }
+
/** Evaluate a {@link SubmitRequirement} on a given {@link ChangeData}. */
public SubmitRequirementResult evaluate(SubmitRequirement sr, ChangeData cd) {
SubmitRequirementExpressionResult blockingResult =
diff --git a/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java b/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java
index 2924e6e..326620d 100644
--- a/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java
+++ b/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java
@@ -23,6 +23,7 @@
import com.google.inject.assistedinject.Assisted;
import java.util.Collection;
import java.util.Objects;
+import java.util.Optional;
/** Predicate that matches patch set approvals we want to copy based on the value. */
public class MagicValuePredicate extends ApprovalPredicate {
@@ -47,19 +48,23 @@
@Override
public boolean match(ApprovalContext ctx) {
+ Optional<LabelType> lt =
+ getLabelType(ctx.changeNotes().getProjectName(), ctx.patchSetApproval().labelId());
short pValue;
switch (value) {
case ANY:
return true;
case MIN:
- pValue =
- getLabelType(ctx.changeNotes().getProjectName(), ctx.patchSetApproval().labelId())
- .getMaxNegative();
+ if (!lt.isPresent()) {
+ return false;
+ }
+ pValue = lt.get().getMaxNegative();
break;
case MAX:
- pValue =
- getLabelType(ctx.changeNotes().getProjectName(), ctx.patchSetApproval().labelId())
- .getMaxPositive();
+ if (!lt.isPresent()) {
+ return false;
+ }
+ pValue = lt.get().getMaxPositive();
break;
default:
throw new IllegalArgumentException("unrecognized label value: " + value);
@@ -67,7 +72,7 @@
return pValue == ctx.patchSetApproval().value();
}
- private LabelType getLabelType(Project.NameKey project, LabelId labelId) {
+ private Optional<LabelType> getLabelType(Project.NameKey project, LabelId labelId) {
return projectCache
.get(project)
.orElseThrow(() -> new IllegalStateException(project + " absent"))
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index f912250..852387f 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -106,6 +106,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@@ -932,21 +933,36 @@
return messages;
}
- /** Get all submit requirements for this change, including those from parent projects. */
+ /**
+ * Get all evaluated submit requirements for this change, including those from parent projects.
+ * For closed changes, submit requirements are read from the change notes. For active changes,
+ * submit requirements are evaluated online.
+ *
+ * <p>For changes loaded from the index, the value will be set from index field {@link
+ * com.google.gerrit.server.index.change.ChangeField#STORED_SUBMIT_REQUIREMENTS}.
+ */
public Map<SubmitRequirement, SubmitRequirementResult> submitRequirements() {
if (submitRequirements == null) {
- ProjectState state = projectCache.get(project()).orElseThrow(illegalState(project()));
- Map<String, SubmitRequirement> requirements = state.getSubmitRequirements();
- ImmutableMap.Builder<SubmitRequirement, SubmitRequirementResult> result =
- ImmutableMap.builderWithExpectedSize(requirements.size());
- for (SubmitRequirement requirement : requirements.values()) {
- result.put(requirement, submitRequirementsEvaluator.evaluate(requirement, this));
+ if (!lazyload()) {
+ return Collections.emptyMap();
}
- submitRequirements = result.build();
+ Change c = change();
+ if (c != null && c.isClosed()) {
+ submitRequirements =
+ notes().getSubmitRequirementsResult().stream()
+ .collect(Collectors.toMap(r -> r.submitRequirement(), Function.identity()));
+ } else {
+ submitRequirements = submitRequirementsEvaluator.getResults(this);
+ }
}
return submitRequirements;
}
+ public void setSubmitRequirements(
+ Map<SubmitRequirement, SubmitRequirementResult> submitRequirements) {
+ this.submitRequirements = submitRequirements;
+ }
+
public List<SubmitRecord> submitRecords(SubmitRuleOptions options) {
// If the change is not submitted yet, 'strict' and 'lenient' both have the same result. If the
// change is submitted, SubmitRecord requested with 'strict' will contain just a single entry
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 30d5e2f..ade615c 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -91,8 +91,8 @@
}
protected static LabelType type(LabelTypes types, String toFind) {
- if (types.byLabel(toFind) != null) {
- return types.byLabel(toFind);
+ if (types.byLabel(toFind).isPresent()) {
+ return types.byLabel(toFind).get();
}
for (LabelType lt : types.getLabelTypes()) {
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
index e3c58e47..2c56322 100644
--- a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
@@ -87,8 +87,8 @@
}
protected static LabelType type(LabelTypes types, String toFind) {
- if (types.byLabel(toFind) != null) {
- return types.byLabel(toFind);
+ if (types.byLabel(toFind).isPresent()) {
+ return types.byLabel(toFind).get();
}
for (LabelType lt : types.getLabelTypes()) {
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 84424a8..2c358d0 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -194,7 +194,7 @@
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
ctx.getNotes(), psId, accountId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
- if (labelTypes.byLabel(a.labelId()) == null) {
+ if (!labelTypes.byLabel(a.labelId()).isPresent()) {
continue; // Ignore undefined labels.
} else if (!a.label().equals(label)) {
// Populate map for non-matching labels, needed by VoteDeleted.
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 9263971..8c21841 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -64,6 +64,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -265,11 +266,13 @@
approvalsUtil.byPatchSet(
ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
- LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.labelId());
+ Optional<LabelType> type =
+ projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.labelId());
// Only keep veto votes, defined as votes where:
// 1- the label function allows minimum values to block submission.
// 2- the vote holds the minimum value.
- if (type == null || (type.isMaxNegative(psa) && type.getFunction().isBlock())) {
+ if (!type.isPresent()
+ || (type.get().isMaxNegative(psa) && type.get().getFunction().isBlock())) {
continue;
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 6816361..4dbb6ee 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -22,7 +22,6 @@
import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
@@ -502,8 +501,8 @@
Iterator<Map.Entry<String, Short>> itr = in.labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
- LabelType type = labelTypes.byLabel(ent.getKey());
- if (type == null) {
+ Optional<LabelType> type = labelTypes.byLabel(ent.getKey());
+ if (!type.isPresent()) {
logger.atFine().log("label %s not found", ent.getKey());
if (strictLabels) {
throw new BadRequestException(
@@ -518,15 +517,15 @@
logger.atFine().log(
"skipping on behalf of permission check for label %s"
+ " because caller is an internal user",
- type.getName());
+ type.get().getName());
} else {
try {
- perm.check(new LabelPermission.WithValue(ON_BEHALF_OF, type, ent.getValue()));
+ perm.check(new LabelPermission.WithValue(ON_BEHALF_OF, type.get(), ent.getValue()));
} catch (AuthException e) {
throw new AuthException(
String.format(
"not permitted to modify label \"%s\" on behalf of \"%s\"",
- type.getName(), in.onBehalfOf),
+ type.get().getName(), in.onBehalfOf),
e);
}
}
@@ -558,8 +557,8 @@
Iterator<Map.Entry<String, Short>> itr = labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
- LabelType lt = labelTypes.byLabel(ent.getKey());
- if (lt == null) {
+ Optional<LabelType> lt = labelTypes.byLabel(ent.getKey());
+ if (!lt.isPresent()) {
logger.atFine().log("label %s not found", ent.getKey());
if (strictLabels) {
throw new BadRequestException(
@@ -576,7 +575,7 @@
continue;
}
- if (lt.getValue(ent.getValue()) == null) {
+ if (lt.get().getValue(ent.getValue()) == null) {
logger.atFine().log("label value %s not found", ent.getValue());
if (strictLabels) {
throw new BadRequestException(
@@ -590,10 +589,10 @@
short val = ent.getValue();
try {
- perm.check(new LabelPermission.WithValue(lt, val));
+ perm.check(new LabelPermission.WithValue(lt.get(), val));
} catch (AuthException e) {
throw new AuthException(
- String.format("Applying label \"%s\": %d is restricted", lt.getName(), val), e);
+ String.format("Applying label \"%s\": %d is restricted", lt.get().getName(), val), e);
}
}
}
@@ -1356,7 +1355,10 @@
ChangeUpdate update = ctx.getUpdate(psId);
for (Map.Entry<String, Short> ent : allApprovals.entrySet()) {
String name = ent.getKey();
- LabelType lt = requireNonNull(labelTypes.byLabel(name), name);
+ LabelType lt =
+ labelTypes
+ .byLabel(name)
+ .orElseThrow(() -> new IllegalStateException("no label config for " + name));
PatchSetApproval c = current.remove(lt.getName());
String normName = lt.getName();
@@ -1448,7 +1450,10 @@
List<String> disallowed = new ArrayList<>(labelTypes.getLabelTypes().size());
for (PatchSetApproval psa : del) {
- LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
+ LabelType lt =
+ labelTypes
+ .byLabel(psa.label())
+ .orElseThrow(() -> new IllegalStateException("no label config for " + psa.label()));
String normName = lt.getName();
if (!lt.isAllowPostSubmit()) {
disallowed.add(normName);
@@ -1460,7 +1465,10 @@
}
for (PatchSetApproval psa : ups) {
- LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
+ LabelType lt =
+ labelTypes
+ .byLabel(psa.label())
+ .orElseThrow(() -> new IllegalStateException("no label config for " + psa.label()));
String normName = lt.getName();
if (!lt.isAllowPostSubmit()) {
disallowed.add(normName);
@@ -1508,9 +1516,9 @@
continue;
}
- LabelType lt = labelTypes.byLabel(a.labelId());
- if (lt != null) {
- current.put(lt.getName(), a);
+ Optional<LabelType> lt = labelTypes.byLabel(a.labelId());
+ if (lt.isPresent()) {
+ current.put(lt.get().getName(), a);
} else {
del.add(a);
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 363cdca..942f024 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -241,6 +241,7 @@
private final NotifyResolver notifyResolver;
private final RetryHelper retryHelper;
private final ChangeData.Factory changeDataFactory;
+ private final StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory;
// Changes that were updated by this MergeOp.
private final Map<Change.Id, Change> updatedChanges;
@@ -274,7 +275,8 @@
NotifyResolver notifyResolver,
TopicMetrics topicMetrics,
RetryHelper retryHelper,
- ChangeData.Factory changeDataFactory) {
+ ChangeData.Factory changeDataFactory,
+ StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory) {
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.internalUserFactory = internalUserFactory;
@@ -291,6 +293,7 @@
this.topicMetrics = topicMetrics;
this.changeDataFactory = changeDataFactory;
this.updatedChanges = new HashMap<>();
+ this.storeSubmitRequirementsOpFactory = storeSubmitRequirementsOpFactory;
}
@Override
@@ -665,7 +668,7 @@
Change.Id changeId = entry.getKey();
batchUpdatesByProject
.get(project)
- .addOp(changeId, new StoreSubmitRequirementsOp(changeDataFactory));
+ .addOp(changeId, storeSubmitRequirementsOpFactory.create());
}
try {
submissionExecutor.setAdditionalBatchUpdateListeners(
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index 0668c1e..f3bd5e1 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -17,6 +17,7 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/audit",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/sshd/SshCommand.java b/java/com/google/gerrit/sshd/SshCommand.java
index 93c6c2c..e58040f 100644
--- a/java/com/google/gerrit/sshd/SshCommand.java
+++ b/java/com/google/gerrit/sshd/SshCommand.java
@@ -14,6 +14,7 @@
package com.google.gerrit.sshd;
+import com.google.common.base.Throwables;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.DynamicOptions;
@@ -28,6 +29,7 @@
import com.google.inject.Inject;
import java.io.IOException;
import java.io.PrintWriter;
+import java.util.Optional;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.channel.ChannelSession;
import org.eclipse.jgit.lib.Config;
@@ -62,10 +64,18 @@
RequestInfo.builder(RequestInfo.RequestType.SSH, user, traceContext).build();
requestListeners.runEach(l -> l.onRequest(requestInfo));
SshCommand.this.run();
- } catch (RequestCancelledException e) {
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append(String.format(" (%s)", e.getCancellationMessage().get()));
+ } catch (RuntimeException e) {
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (!requestCancelledException.isPresent()) {
+ Throwables.throwIfUnchecked(e);
+ }
+ StringBuilder msg =
+ new StringBuilder(requestCancelledException.get().formatCancellationReason());
+ if (requestCancelledException.get().getCancellationMessage().isPresent()) {
+ msg.append(
+ String.format(
+ " (%s)", requestCancelledException.get().getCancellationMessage().get()));
}
stderr.println(msg.toString());
} finally {
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 06d9453..12da2c1 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -134,7 +134,6 @@
cfg.setString("user", null, "name", "Gerrit Code Review");
cfg.setString("user", null, "email", "gerrit@localhost");
cfg.unset("cache", null, "directory");
- cfg.setString("index", null, "type", "lucene");
cfg.setBoolean("index", "lucene", "testInmemory", true);
cfg.setInt("sendemail", null, "threadPoolSize", 0);
cfg.setBoolean("receive", null, "enableSignedPush", false);
@@ -240,7 +239,8 @@
bind(AllChangesIndexer.class).toProvider(Providers.of(null));
bind(AllGroupsIndexer.class).toProvider(Providers.of(null));
- IndexType indexType = new IndexType(cfg.getString("index", null, "type"));
+ String indexTypeCfg = cfg.getString("index", null, "type");
+ IndexType indexType = new IndexType(indexTypeCfg != null ? indexTypeCfg : "fake");
// For custom index types, callers must provide their own module.
if (indexType.isLucene()) {
install(luceneIndexModule());
diff --git a/java/com/google/gerrit/testing/IndexConfig.java b/java/com/google/gerrit/testing/IndexConfig.java
index daef2b3..d68dcad 100644
--- a/java/com/google/gerrit/testing/IndexConfig.java
+++ b/java/com/google/gerrit/testing/IndexConfig.java
@@ -38,7 +38,9 @@
}
public static Config createForLucene() {
- return create();
+ Config cfg = create();
+ cfg.setString("index", null, "type", "lucene");
+ return cfg;
}
public static Config createForElasticsearch() {
diff --git a/java/gerrit/PRED__load_commit_labels_1.java b/java/gerrit/PRED__load_commit_labels_1.java
index 5ee292ff..9a656b8 100644
--- a/java/gerrit/PRED__load_commit_labels_1.java
+++ b/java/gerrit/PRED__load_commit_labels_1.java
@@ -16,6 +16,7 @@
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import java.util.Optional;
/** Exports list of {@code commit_label( label('Code-Review', 2), user(12345789) )}. */
class PRED__load_commit_labels_1 extends Predicate.P1 {
@@ -38,13 +39,14 @@
LabelTypes types = cd.getLabelTypes();
for (PatchSetApproval a : cd.currentApprovals()) {
- LabelType t = types.byLabel(a.labelId());
- if (t == null) {
+ Optional<LabelType> t = types.byLabel(a.labelId());
+ if (!t.isPresent()) {
continue;
}
StructureTerm labelTerm =
- new StructureTerm(sym_label, SymbolTerm.intern(t.getName()), new IntegerTerm(a.value()));
+ new StructureTerm(
+ sym_label, SymbolTerm.intern(t.get().getName()), new IntegerTerm(a.value()));
StructureTerm userTerm = new StructureTerm(sym_user, new IntegerTerm(a.accountId().get()));
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index f66bc8d..aa8615b 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -75,6 +75,7 @@
i.emailStrategy = EmailStrategy.DISABLED;
i.emailFormat = EmailFormat.PLAINTEXT;
i.defaultBaseForMerges = DefaultBase.AUTO_MERGE;
+ i.disableKeyboardShortcuts = true;
i.expandInlineDiffs ^= true;
i.highlightAssigneeInChangeTable ^= true;
i.relativeDateInChangeTable ^= true;
@@ -93,6 +94,7 @@
assertThat(o.my).containsExactlyElementsIn(i.my);
assertThat(o.changeTable).containsExactlyElementsIn(i.changeTable);
assertThat(o.theme).isEqualTo(i.theme);
+ assertThat(o.disableKeyboardShortcuts).isEqualTo(i.disableKeyboardShortcuts);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c08aa7f..7785776 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4325,6 +4325,101 @@
}
@Test
+ public void submitRequirement_retrievedFromNoteDbForClosedChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+
+ voteLabel(changeId, "code-review", 2);
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+
+ gApi.changes().id(changeId).current().submit();
+
+ // Add new submit requirement
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("verified")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:verified=+1"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ // The new "verified" submit requirement is not returned, since this change is closed
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirement_backFilledFromIndexForActiveChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "code-review", 2);
+
+ // Query the change. ChangeInfo is back-filled from the change index.
+ List<ChangeInfo> changeInfos =
+ gApi.changes()
+ .query()
+ .withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
+ .withOptions(ImmutableSet.of(ListChangesOption.SUBMIT_REQUIREMENTS))
+ .get();
+ assertThat(changeInfos).hasSize(1);
+ assertSubmitRequirementStatus(
+ changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirement_backFilledFromIndexForClosedChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "code-review", 2);
+ gApi.changes().id(changeId).current().submit();
+
+ // Query the change. ChangeInfo is back-filled from the change index.
+ List<ChangeInfo> changeInfos =
+ gApi.changes()
+ .query()
+ .withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
+ .withOptions(ImmutableSet.of(ListChangesOption.SUBMIT_REQUIREMENTS))
+ .get();
+ assertThat(changeInfos).hasSize(1);
+ assertSubmitRequirementStatus(
+ changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
public void fourByteEmoji() throws Exception {
// U+1F601 GRINNING FACE WITH SMILING EYES
String smile = new String(Character.toChars(0x1f601));
diff --git a/javatests/com/google/gerrit/acceptance/rest/BUILD b/javatests/com/google/gerrit/acceptance/rest/BUILD
index 84887da..a62d551 100644
--- a/javatests/com/google/gerrit/acceptance/rest/BUILD
+++ b/javatests/com/google/gerrit/acceptance/rest/BUILD
@@ -5,6 +5,7 @@
group = "rest_bindings_collection",
labels = ["rest"],
deps = [
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
index 29d54cc..0713fe6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
@@ -121,6 +121,27 @@
}
@Test
+ public void handleWrappedRequestCancelledException() throws Exception {
+ ProjectCreationValidationListener projectCreationListener =
+ new ProjectCreationValidationListener() {
+ @Override
+ public void validateNewProject(CreateProjectArgs args) throws ValidationException {
+ // Simulate an exceeded deadline by throwing RequestCancelledException.
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ RestResponse response = adminRestSession.put("/projects/" + name("new"));
+ assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getEntityContent())
+ .isEqualTo("Server Deadline Exceeded\n\ndeadline = 10m");
+ }
+ }
+
+ @Test
public void handleClientDisconnectedForPush() throws Exception {
CommitValidationListener commitValidationListener =
new CommitValidationListener() {
@@ -185,6 +206,27 @@
}
@Test
+ public void handleWrappedRequestCancelledExceptionForPush() throws Exception {
+ CommitValidationListener commitValidationListener =
+ new CommitValidationListener() {
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ // Simulate an exceeded deadline by throwing RequestCancelledException.
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(commitValidationListener)) {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ PushOneCommit.Result r = push.to("refs/heads/master");
+ r.assertErrorStatus("Server Deadline Exceeded (deadline = 10m)");
+ }
+ }
+
+ @Test
public void handleRequestCancellationWithMessageForPush() throws Exception {
CommitValidationListener commitValidationListener =
new CommitValidationListener() {
diff --git a/javatests/com/google/gerrit/acceptance/ssh/BUILD b/javatests/com/google/gerrit/acceptance/ssh/BUILD
index 5634322..ee1b221 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/BUILD
+++ b/javatests/com/google/gerrit/acceptance/ssh/BUILD
@@ -18,6 +18,7 @@
vm_args = ["-Xmx512m"],
deps = [
":util",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
"//lib/commons:compress",
],
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
index 2cb9637..3e31c16 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
@@ -99,4 +99,22 @@
adminSshSession.assertFailure("Server Deadline Exceeded (deadline = 10m)");
}
}
+
+ @Test
+ public void handleWrappedRequestCancelledException() throws Exception {
+ ProjectCreationValidationListener projectCreationListener =
+ new ProjectCreationValidationListener() {
+ @Override
+ public void validateNewProject(CreateProjectArgs args) throws ValidationException {
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project " + name("new"));
+ adminSshSession.assertFailure("Server Deadline Exceeded (deadline = 10m)");
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 7ab7ae9..a2825322 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -55,6 +55,7 @@
"//java/com/google/gerrit/server/account/externalids/testing",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/cache/testing",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/fixes/testing",
"//java/com/google/gerrit/server/git/receive:ref_cache",
"//java/com/google/gerrit/server/ioutil",
diff --git a/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java b/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java
new file mode 100644
index 0000000..e2c43eb
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java
@@ -0,0 +1,178 @@
+// Copyright (C) 2021 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.cancellation;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+
+public class RequestStateContextTest {
+ @Test
+ public void openContext() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void openNestedContexts() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+
+ RequestStateProvider requestStateProvider3 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext2 =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider3)) {
+ RequestStateProvider requestStateProvider4 = new TestRequestStateProvider();
+ requestStateContext2.addRequestStateProvider(requestStateProvider4);
+ assertRequestStateProviders(
+ ImmutableSet.of(
+ requestStateProvider1,
+ requestStateProvider2,
+ requestStateProvider3,
+ requestStateProvider4));
+ }
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void openNestedContextsWithSameRequestStateProviders() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+
+ try (RequestStateContext requestStateContext2 =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ requestStateContext2.addRequestStateProvider(requestStateProvider2);
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void abortIfCancelled_noRequestStateProvider() {
+ assertNoRequestStateProviders();
+
+ // Calling abortIfCancelled() shouldn't throw an exception.
+ RequestStateContext.abortIfCancelled();
+ }
+
+ @Test
+ public void abortIfCancelled_requestNotCancelled() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {}
+ })) {
+ // Calling abortIfCancelled() shouldn't throw an exception.
+ RequestStateContext.abortIfCancelled();
+ }
+ }
+
+ @Test
+ public void abortIfCancelled_requestCancelled() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {
+ onCancelled.onCancel(
+ RequestStateProvider.Reason.CLIENT_CLOSED_REQUEST, /* message= */ null);
+ }
+ })) {
+ RequestCancelledException requestCancelledException =
+ assertThrows(
+ RequestCancelledException.class, () -> RequestStateContext.abortIfCancelled());
+ assertThat(requestCancelledException)
+ .hasMessageThat()
+ .isEqualTo("Request cancelled: CLIENT_CLOSED_REQUEST");
+ assertThat(requestCancelledException.getCancellationReason())
+ .isEqualTo(RequestStateProvider.Reason.CLIENT_CLOSED_REQUEST);
+ assertThat(requestCancelledException.getCancellationMessage()).isEmpty();
+ }
+ }
+
+ @Test
+ public void abortIfCancelled_requestCancelled_withMessage() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {
+ onCancelled.onCancel(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m");
+ }
+ })) {
+ RequestCancelledException requestCancelledException =
+ assertThrows(
+ RequestCancelledException.class, () -> RequestStateContext.abortIfCancelled());
+ assertThat(requestCancelledException)
+ .hasMessageThat()
+ .isEqualTo("Request cancelled: SERVER_DEADLINE_EXCEEDED (deadline = 10m)");
+ assertThat(requestCancelledException.getCancellationReason())
+ .isEqualTo(RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED);
+ assertThat(requestCancelledException.getCancellationMessage()).hasValue("deadline = 10m");
+ }
+ }
+
+ private void assertNoRequestStateProviders() {
+ assertRequestStateProviders(ImmutableSet.of());
+ }
+
+ private void assertRequestStateProviders(
+ ImmutableSet<RequestStateProvider> expectedRequestStateProviders) {
+ assertThat(RequestStateContext.getRequestStateProviders())
+ .containsExactlyElementsIn(expectedRequestStateProviders);
+ }
+
+ private static class TestRequestStateProvider implements RequestStateProvider {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {}
+ }
+}
diff --git a/javatests/com/google/gerrit/server/schema/SchemaCreatorImplTest.java b/javatests/com/google/gerrit/server/schema/SchemaCreatorImplTest.java
index fc6b412..9cba362 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaCreatorImplTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaCreatorImplTest.java
@@ -72,7 +72,7 @@
@Test
public void createSchema_Label_CodeReview() throws Exception {
- LabelType codeReview = getLabelTypes().byLabel("Code-Review");
+ LabelType codeReview = getLabelTypes().byLabel("Code-Review").get();
assertThat(codeReview).isNotNull();
assertThat(codeReview.getName()).isEqualTo("Code-Review");
assertThat(codeReview.getDefaultValue()).isEqualTo(0);
diff --git a/plugins/replication b/plugins/replication
index dc9bb2e..46cfb7d 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit dc9bb2e946e4c6c31e8a4665f30eca6d00017523
+Subproject commit 46cfb7dd5b6891f991cfe66e72c08953487c1c81
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 35e6449..a28ae59 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 35e6449a517691a880c94e7467bc07360f8e6666
+Subproject commit a28ae590486934690e4e0a95d7eb75f8b60644a6
diff --git a/plugins/webhooks b/plugins/webhooks
index 9fc9c2d..73f9dc7 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 9fc9c2d4e69f7e2701cbcd873977d3312a231a81
+Subproject commit 73f9dc72bd52f5d64853db31e711717a995f0a46
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index fcf1cf4..5f9c3c5 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -106,10 +106,6 @@
"elements/change/gr-reply-dialog/gr-reply-dialog_html.ts",
"elements/change/gr-reviewer-list/gr-reviewer-list_html.ts",
"elements/change/gr-thread-list/gr-thread-list_html.ts",
- "elements/checks/gr-hovercard-run_html.ts",
- "elements/core/gr-main-header/gr-main-header_html.ts",
- "elements/core/gr-search-bar/gr-search-bar_html.ts",
- "elements/core/gr-smart-search/gr-smart-search_html.ts",
"elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_html.ts",
"elements/diff/gr-diff-builder/gr-diff-builder-element_html.ts",
"elements/diff/gr-diff-host/gr-diff-host_html.ts",
@@ -123,17 +119,14 @@
"elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts",
"elements/shared/gr-account-list/gr-account-list_html.ts",
"elements/shared/gr-autocomplete/gr-autocomplete_html.ts",
- "elements/shared/gr-change-status/gr-change-status_html.ts",
"elements/shared/gr-comment-thread/gr-comment-thread_html.ts",
"elements/shared/gr-comment/gr-comment_html.ts",
"elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_html.ts",
- "elements/shared/gr-copy-clipboard/gr-copy-clipboard_html.ts",
"elements/shared/gr-dialog/gr-dialog_html.ts",
"elements/shared/gr-diff-preferences/gr-diff-preferences_html.ts",
"elements/shared/gr-download-commands/gr-download-commands_html.ts",
"elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts",
"elements/shared/gr-dropdown/gr-dropdown_html.ts",
- "elements/shared/gr-editable-content/gr-editable-content_html.ts",
"elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts",
"elements/shared/gr-label-info/gr-label-info_html.ts",
"elements/shared/gr-labeled-autocomplete/gr-labeled-autocomplete_html.ts",
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
index af565cb..a090aee 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
@@ -150,18 +150,25 @@
this._query = (text: string) => this._getProjectBranchesSuggestions(text);
}
+ containsDuplicateProject(changes: ChangeInfo[]) {
+ const projects: {[projectName: string]: boolean} = {};
+ for (let i = 0; i < changes.length; i++) {
+ const change = changes[i];
+ if (projects[change.project]) {
+ return true;
+ }
+ projects[change.project] = true;
+ }
+ return false;
+ }
+
updateChanges(changes: ChangeInfo[]) {
this.changes = changes;
this._statuses = {};
- const projects: {[projectName: string]: boolean} = {};
- this._duplicateProjectChanges = false;
changes.forEach(change => {
this.selectedChangeIds.add(change.id);
- if (projects[change.project]) {
- this._duplicateProjectChanges = true;
- }
- projects[change.project] = true;
});
+ this._duplicateProjectChanges = this.containsDuplicateProject(changes);
this._changesCount = changes.length;
this._showCherryPickTopic = changes.length > 1;
}
@@ -185,6 +192,10 @@
if (this.selectedChangeIds.has(changeId))
this.selectedChangeIds.delete(changeId);
else this.selectedChangeIds.add(changeId);
+ const changes = this.changes.filter(change =>
+ this.selectedChangeIds.has(change.id)
+ );
+ this._duplicateProjectChanges = this.containsDuplicateProject(changes);
}
_computeTopicErrorMessage(duplicateProjectChanges: boolean) {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
index 536a4ab..1034674 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
@@ -130,6 +130,8 @@
});
test('deselecting a change removes it from being cherry picked', () => {
+ const duplicateChangesStub = sinon.stub(element,
+ 'containsDuplicateProject');
element.branch = 'master';
const executeChangeActionStub = stubRestApi(
'executeChangeAction').returns(Promise.resolve([]));
@@ -142,6 +144,7 @@
querySelector('gr-dialog').$.confirm);
flush();
assert.equal(executeChangeActionStub.callCount, 1);
+ assert.isTrue(duplicateChangesStub.called);
});
test('deselecting all change shows error message', () => {
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
index 1ae3e2b..2316a05 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
@@ -112,6 +112,10 @@
if (!run) return true;
return !run.checkLink && !run.checkDescription;
}
+
+ _convertUndefined(value?: string) {
+ return value ?? '';
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
index 277bd16..5b2e24a 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
@@ -130,7 +130,7 @@
<div hidden$="[[!run.statusLink]]" class="row">
<div class="title">Status</div>
<div>
- <a href="[[run.statusLink]]" target="_blank"
+ <a href="[[_convertUndefined(run.statusLink)]]" target="_blank"
><iron-icon
aria-label="external link to check status"
class="small link"
@@ -202,7 +202,7 @@
<div hidden$="[[!run.checkLink]]" class="row">
<div class="title">Documentation</div>
<div>
- <a href="[[run.checkLink]]" target="_blank"
+ <a href="[[_convertUndefined(run.checkLink)]]" target="_blank"
><iron-icon
aria-label="external link to check documentation"
class="small link"
@@ -216,10 +216,7 @@
</div>
<template is="dom-repeat" items="[[computeActions(run)]]">
<div class="action">
- <gr-checks-action
- event-target="[[_target]]"
- action="[[item]]"
- ></gr-checks-action>
+ <gr-checks-action action="[[item]]"></gr-checks-action>
</div>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
index f7e61bf..fd7e25e 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
@@ -110,7 +110,7 @@
}
@property({type: String, notify: true})
- searchQuery?: string;
+ searchQuery = '';
@property({type: Boolean, reflectToAttribute: true})
loggedIn?: boolean;
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index e30e75e..841089e 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -180,7 +180,7 @@
accountSuggestions: SuggestionProvider = () => Promise.resolve([]);
@property({type: String})
- _inputVal?: string;
+ _inputVal = '';
@property({type: Number})
_threshold = 1;
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
similarity index 66%
rename from polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.js
rename to polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
index ae7e10c..71d378e 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.js
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
@@ -15,17 +15,28 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-search-bar.js';
-import '../../../scripts/util.js';
-import {TestKeyboardShortcutBinder, stubRestApi} from '../../../test/test-utils.js';
-import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {_testOnly_clearDocsBaseUrlCache} from '../../../utils/url-util.js';
+import '../../../test/common-test-setup-karma';
+import './gr-search-bar';
+import '../../../scripts/util';
+import {GrSearchBar} from './gr-search-bar';
+import {
+ TestKeyboardShortcutBinder,
+ stubRestApi,
+} from '../../../test/test-utils';
+import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
+import {_testOnly_clearDocsBaseUrlCache} from '../../../utils/url-util';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+import {
+ createChangeConfig,
+ createGerritInfo,
+ createServerInfo,
+} from '../../../test/test-data-generators';
+import {MergeabilityComputationBehavior} from '../../../constants/constants';
const basicFixture = fixtureFromElement('gr-search-bar');
suite('gr-search-bar tests', () => {
- let element;
+ let element: GrSearchBar;
suiteSetup(() => {
const kb = TestKeyboardShortcutBinder.push();
@@ -46,26 +57,34 @@
assert.equal(element._inputVal, 'foo');
});
- const getActiveElement = () => (document.activeElement.shadowRoot ?
- document.activeElement.shadowRoot.activeElement :
- document.activeElement);
+ const getActiveElement = () =>
+ document.activeElement!.shadowRoot
+ ? document.activeElement!.shadowRoot.activeElement
+ : document.activeElement;
test('enter in search input fires event', done => {
element.addEventListener('handle-search', () => {
assert.notEqual(getActiveElement(), element.$.searchInput);
- assert.notEqual(getActiveElement(), element.$.searchButton);
done();
});
element.value = 'test';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
});
test('input blurred after commit', () => {
const blurSpy = sinon.spy(element.$.searchInput.$.input, 'blur');
element.$.searchInput.text = 'fate/stay';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isTrue(blurSpy.called);
});
@@ -73,8 +92,12 @@
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
element.value = '';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isFalse(searchSpy.called);
});
@@ -82,8 +105,12 @@
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
element.value = 'added:';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isFalse(searchSpy.called);
});
@@ -91,8 +118,12 @@
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
element.value = 'age:1week';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isTrue(searchSpy.called);
});
@@ -100,8 +131,12 @@
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
element.value = 'random:1week';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isTrue(searchSpy.called);
});
@@ -109,8 +144,12 @@
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
element.value = 'random:';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
- null, 'enter');
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.searchInput.$.input,
+ 13,
+ null,
+ 'enter'
+ );
assert.isTrue(searchSpy.called);
});
@@ -129,21 +168,23 @@
});
test('Autocompletes accounts', () => {
- sinon.stub(element, 'accountSuggestions').callsFake(() =>
- Promise.resolve([{text: 'owner:fred@goog.co'}])
- );
+ sinon
+ .stub(element, 'accountSuggestions')
+ .callsFake(() => Promise.resolve([{text: 'owner:fred@goog.co'}]));
return element._getSearchSuggestions('owner:fr').then(s => {
assert.equal(s[0].value, 'owner:fred@goog.co');
});
});
test('Autocompletes groups', done => {
- sinon.stub(element, 'groupSuggestions').callsFake(() =>
- Promise.resolve([
- {text: 'ownerin:Polygerrit'},
- {text: 'ownerin:gerrit'},
- ])
- );
+ sinon
+ .stub(element, 'groupSuggestions')
+ .callsFake(() =>
+ Promise.resolve([
+ {text: 'ownerin:Polygerrit'},
+ {text: 'ownerin:gerrit'},
+ ])
+ );
element._getSearchSuggestions('ownerin:pol').then(s => {
assert.equal(s[0].value, 'ownerin:Polygerrit');
done();
@@ -151,13 +192,15 @@
});
test('Autocompletes projects', done => {
- sinon.stub(element, 'projectSuggestions').callsFake(() =>
- Promise.resolve([
- {text: 'project:Polygerrit'},
- {text: 'project:gerrit'},
- {text: 'project:gerrittest'},
- ])
- );
+ sinon
+ .stub(element, 'projectSuggestions')
+ .callsFake(() =>
+ Promise.resolve([
+ {text: 'project:Polygerrit'},
+ {text: 'project:gerrit'},
+ {text: 'project:gerrittest'},
+ ])
+ );
element._getSearchSuggestions('project:pol').then(s => {
assert.equal(s[0].value, 'project:Polygerrit');
done();
@@ -193,11 +236,15 @@
].forEach(mergeability => {
suite(`mergeability as ${mergeability}`, () => {
setup(done => {
- stubRestApi('getConfig').returns(Promise.resolve({
- change: {
- mergeability_computation_behavior: mergeability,
- },
- }));
+ stubRestApi('getConfig').returns(
+ Promise.resolve({
+ ...createServerInfo(),
+ change: {
+ ...createChangeConfig(),
+ mergeability_computation_behavior: mergeability as MergeabilityComputationBehavior,
+ },
+ })
+ );
element = basicFixture.instantiate();
flush(done);
@@ -218,11 +265,15 @@
suite('doc url', () => {
setup(done => {
- stubRestApi('getConfig').returns(Promise.resolve({
- gerrit: {
- doc_url: 'https://doc.com/',
- },
- }));
+ stubRestApi('getConfig').returns(
+ Promise.resolve({
+ ...createServerInfo(),
+ gerrit: {
+ ...createGerritInfo(),
+ doc_url: 'https://doc.com/',
+ },
+ })
+ );
_testOnly_clearDocsBaseUrlCache();
element = basicFixture.instantiate();
@@ -232,18 +283,17 @@
test('compute help doc url with correct path', () => {
assert.equal(element.docBaseUrl, 'https://doc.com/');
assert.equal(
- element._computeHelpDocLink(element.docBaseUrl),
- 'https://doc.com/user-search.html'
+ element._computeHelpDocLink(element.docBaseUrl),
+ 'https://doc.com/user-search.html'
);
});
test('compute help doc url fallback to gerrit url', () => {
assert.equal(
- element._computeHelpDocLink(),
- 'https://gerrit-review.googlesource.com/documentation/' +
+ element._computeHelpDocLink(null),
+ 'https://gerrit-review.googlesource.com/documentation/' +
'user-search.html'
);
});
});
});
-
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
index aa7e2e0..7419713 100644
--- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
+++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
@@ -32,6 +32,12 @@
const SELF_EXPRESSION = 'self';
const ME_EXPRESSION = 'me';
+declare global {
+ interface HTMLElementEventMap {
+ 'handle-search': CustomEvent<SearchBarHandleSearchDetail>;
+ }
+}
+
@customElement('gr-smart-search')
export class GrSmartSearch extends PolymerElement {
static get template() {
@@ -39,7 +45,7 @@
}
@property({type: String})
- searchQuery?: string;
+ searchQuery = '';
@property({type: Object})
_config?: ServerInfo;
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.js b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.js
deleted file mode 100644
index f3a9965..0000000
--- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.js
+++ /dev/null
@@ -1,137 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 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.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-smart-search.js';
-import {stubRestApi} from '../../../test/test-utils.js';
-
-const basicFixture = fixtureFromElement('gr-smart-search');
-
-suite('gr-smart-search tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
- });
-
- test('Autocompletes accounts', () => {
- stubRestApi('getSuggestedAccounts').callsFake(() =>
- Promise.resolve([
- {
- name: 'fred',
- email: 'fred@goog.co',
- },
- ])
- );
- return element._fetchAccounts('owner', 'fr').then(s => {
- assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
- });
- });
-
- test('Inserts self as option when valid', () => {
- stubRestApi('getSuggestedAccounts').callsFake( () =>
- Promise.resolve([
- {
- name: 'fred',
- email: 'fred@goog.co',
- },
- ])
- );
- element._fetchAccounts('owner', 's')
- .then(s => {
- assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
- assert.deepEqual(s[1], {text: 'owner:self'});
- })
- .then(() => element._fetchAccounts('owner', 'selfs'))
- .then(s => {
- assert.notEqual(s[0], {text: 'owner:self'});
- });
- });
-
- test('Inserts me as option when valid', () => {
- stubRestApi('getSuggestedAccounts').callsFake( () =>
- Promise.resolve([
- {
- name: 'fred',
- email: 'fred@goog.co',
- },
- ])
- );
- return element._fetchAccounts('owner', 'm')
- .then(s => {
- assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
- assert.deepEqual(s[1], {text: 'owner:me'});
- })
- .then(() => element._fetchAccounts('owner', 'meme'))
- .then(s => {
- assert.notEqual(s[0], {text: 'owner:me'});
- });
- });
-
- test('Autocompletes groups', () => {
- stubRestApi('getSuggestedGroups').callsFake( () =>
- Promise.resolve({
- Polygerrit: 0,
- gerrit: 0,
- gerrittest: 0,
- })
- );
- return element._fetchGroups('ownerin', 'pol').then(s => {
- assert.deepEqual(s[0], {text: 'ownerin:Polygerrit'});
- });
- });
-
- test('Autocompletes projects', () => {
- stubRestApi('getSuggestedProjects').callsFake( () =>
- Promise.resolve({Polygerrit: 0}));
- return element._fetchProjects('project', 'pol').then(s => {
- assert.deepEqual(s[0], {text: 'project:Polygerrit'});
- });
- });
-
- test('Autocomplete doesnt override exact matches to input', () => {
- stubRestApi('getSuggestedGroups').callsFake( () =>
- Promise.resolve({
- Polygerrit: 0,
- gerrit: 0,
- gerrittest: 0,
- })
- );
- return element._fetchGroups('ownerin', 'gerrit').then(s => {
- assert.deepEqual(s[0], {text: 'ownerin:Polygerrit'});
- assert.deepEqual(s[1], {text: 'ownerin:gerrit'});
- assert.deepEqual(s[2], {text: 'ownerin:gerrittest'});
- });
- });
-
- test('Autocompletes accounts with no email', () => {
- stubRestApi('getSuggestedAccounts').callsFake( () =>
- Promise.resolve([{name: 'fred'}]));
- return element._fetchAccounts('owner', 'fr').then(s => {
- assert.deepEqual(s[0], {text: 'owner:"fred"', label: 'fred'});
- });
- });
-
- test('Autocompletes accounts with email', () => {
- stubRestApi('getSuggestedAccounts').callsFake( () =>
- Promise.resolve([{email: 'fred@goog.co'}]));
- return element._fetchAccounts('owner', 'fr').then(s => {
- assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: ''});
- });
- });
-});
-
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.ts b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.ts
new file mode 100644
index 0000000..0218a8f
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search_test.ts
@@ -0,0 +1,143 @@
+/**
+ * @license
+ * Copyright (C) 2016 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.
+ */
+
+import '../../../test/common-test-setup-karma';
+import './gr-smart-search';
+import {GrSmartSearch} from './gr-smart-search';
+import {stubRestApi} from '../../../test/test-utils';
+import {EmailAddress, GroupId, UrlEncodedRepoName} from '../../../types/common';
+
+const basicFixture = fixtureFromElement('gr-smart-search');
+
+suite('gr-smart-search tests', () => {
+ let element: GrSmartSearch;
+
+ setup(() => {
+ element = basicFixture.instantiate();
+ });
+
+ test('Autocompletes accounts', () => {
+ stubRestApi('getSuggestedAccounts').callsFake(() =>
+ Promise.resolve([
+ {
+ name: 'fred',
+ email: 'fred@goog.co' as EmailAddress,
+ },
+ ])
+ );
+ return element._fetchAccounts('owner', 'fr').then(s => {
+ assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
+ });
+ });
+
+ test('Inserts self as option when valid', () => {
+ stubRestApi('getSuggestedAccounts').callsFake(() =>
+ Promise.resolve([
+ {
+ name: 'fred',
+ email: 'fred@goog.co' as EmailAddress,
+ },
+ ])
+ );
+ element
+ ._fetchAccounts('owner', 's')
+ .then(s => {
+ assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
+ assert.deepEqual(s[1], {text: 'owner:self'});
+ })
+ .then(() => element._fetchAccounts('owner', 'selfs'))
+ .then(s => {
+ assert.notEqual(s[0], {text: 'owner:self'});
+ });
+ });
+
+ test('Inserts me as option when valid', () => {
+ stubRestApi('getSuggestedAccounts').callsFake(() =>
+ Promise.resolve([
+ {
+ name: 'fred',
+ email: 'fred@goog.co' as EmailAddress,
+ },
+ ])
+ );
+ return element
+ ._fetchAccounts('owner', 'm')
+ .then(s => {
+ assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: 'fred'});
+ assert.deepEqual(s[1], {text: 'owner:me'});
+ })
+ .then(() => element._fetchAccounts('owner', 'meme'))
+ .then(s => {
+ assert.notEqual(s[0], {text: 'owner:me'});
+ });
+ });
+
+ test('Autocompletes groups', () => {
+ stubRestApi('getSuggestedGroups').callsFake(() =>
+ Promise.resolve({
+ Polygerrit: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ gerrit: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ gerrittest: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ })
+ );
+ return element._fetchGroups('ownerin', 'pol').then(s => {
+ assert.deepEqual(s[0], {text: 'ownerin:Polygerrit'});
+ });
+ });
+
+ test('Autocompletes projects', () => {
+ stubRestApi('getSuggestedProjects').callsFake(() =>
+ Promise.resolve({Polygerrit: {id: 'test' as UrlEncodedRepoName}})
+ );
+ return element._fetchProjects('project', 'pol').then(s => {
+ assert.deepEqual(s[0], {text: 'project:Polygerrit'});
+ });
+ });
+
+ test('Autocomplete doesnt override exact matches to input', () => {
+ stubRestApi('getSuggestedGroups').callsFake(() =>
+ Promise.resolve({
+ Polygerrit: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ gerrit: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ gerrittest: {id: '4c97682e6ce61b7247f3381b6f1789356666de7f' as GroupId},
+ })
+ );
+ return element._fetchGroups('ownerin', 'gerrit').then(s => {
+ assert.deepEqual(s[0], {text: 'ownerin:Polygerrit'});
+ assert.deepEqual(s[1], {text: 'ownerin:gerrit'});
+ assert.deepEqual(s[2], {text: 'ownerin:gerrittest'});
+ });
+ });
+
+ test('Autocompletes accounts with no email', () => {
+ stubRestApi('getSuggestedAccounts').callsFake(() =>
+ Promise.resolve([{name: 'fred'}])
+ );
+ return element._fetchAccounts('owner', 'fr').then(s => {
+ assert.deepEqual(s[0], {text: 'owner:"fred"', label: 'fred'});
+ });
+ });
+
+ test('Autocompletes accounts with email', () => {
+ stubRestApi('getSuggestedAccounts').callsFake(() =>
+ Promise.resolve([{email: 'fred@goog.co' as EmailAddress}])
+ );
+ return element._fetchAccounts('owner', 'fr').then(s => {
+ assert.deepEqual(s[0], {text: 'owner:fred@goog.co', label: ''});
+ });
+ });
+});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index eb50914..864fd79 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -514,9 +514,7 @@
if (beforeNumber !== 'FILE' && beforeNumber !== 'LOST') {
const responsiveMode = getResponsiveMode(this._prefs, this._renderPrefs);
const lineLimit =
- responsiveMode !== 'FULL_RESPONSIVE'
- ? this._prefs.line_length
- : Infinity;
+ responsiveMode === 'NONE' ? this._prefs.line_length : Infinity;
const contentText = this._formatText(
line.text,
this._prefs.tab_size,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
index c8b3901..f418cfa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
@@ -23,7 +23,7 @@
lineNumberToNumber,
} from '../gr-diff/gr-diff-utils';
-const tokenMatcher = new RegExp(/[a-zA-Z0-9_-]+/g);
+const tokenMatcher = new RegExp(/[\w]+/g);
/** CSS class for all tokens. */
const CSS_TOKEN = 'token';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index b576896..0f8752d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -43,6 +43,9 @@
@property({type: Boolean})
saveOnChange = false;
+ @property({type: Boolean})
+ showTooltipBelow = false;
+
private readonly restApiService = appContext.restApiService;
/** @override */
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_html.ts
index 9943b58..3ebb58f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_html.ts
@@ -34,6 +34,7 @@
id="sideBySideBtn"
link=""
has-tooltip=""
+ position-below="[[showTooltipBelow]]"
class$="[[_computeSideBySideSelected(mode)]]"
title="Side-by-side diff"
aria-pressed="[[isSideBySideSelected(mode)]]"
@@ -45,6 +46,7 @@
id="unifiedBtn"
link=""
has-tooltip=""
+ position-below="[[showTooltipBelow]]"
title="Unified diff"
class$="[[_computeUnifiedSelected(mode)]]"
aria-pressed="[[isUnifiedSelected(mode)]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index 8d69007d..743f905 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -343,6 +343,7 @@
id="modeSelect"
save-on-change="[[!_diffPrefsDisabled]]"
mode="{{changeViewState.diffMode}}"
+ show-tooltip-below=""
></gr-diff-mode-selector>
</div>
<span
@@ -355,6 +356,7 @@
link=""
class="prefsButton"
has-tooltip=""
+ position-below=""
title="Diff preferences"
on-click="_handlePrefsTap"
><iron-icon icon="gr-icons:settings"></iron-icon
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index 66214b4..1a340b9 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -198,7 +198,7 @@
.account="${account}"
.change="${change}"
?highlight-attention=${highlightAttention}
- .voteable-text=${this.voteableText}
+ .voteableText=${this.voteableText}
></gr-hovercard-account>`
: ''}
${hasAttention
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
similarity index 60%
rename from polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js
rename to polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
index df8632f..bb70855 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
@@ -15,19 +15,26 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-avatar.js';
-import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader.js';
-import {appContext} from '../../../services/app-context.js';
+import '../../../test/common-test-setup-karma';
+import './gr-avatar';
+import {GrAvatar} from './gr-avatar';
+import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader';
+import {appContext} from '../../../services/app-context';
+import {AvatarInfo} from '../../../types/common';
+import {
+ createAccountWithEmail,
+ createAccountWithId,
+} from '../../../test/test-data-generators';
const basicFixture = fixtureFromElement('gr-avatar');
suite('gr-avatar tests', () => {
- let element;
- const defaultAvatars = [
+ let element: GrAvatar;
+ const defaultAvatars: AvatarInfo[] = [
{
url: 'https://cdn.example.com/s12-p/photo.jpg',
height: 12,
+ width: 0,
},
];
@@ -36,68 +43,74 @@
});
test('account without avatar', () => {
- assert.equal(
- element._buildAvatarURL({
- _account_id: 123,
- }),
- '');
+ assert.equal(element._buildAvatarURL(createAccountWithId(123)), '');
});
test('methods', () => {
assert.equal(
- element._buildAvatarURL({
- _account_id: 123,
- avatars: defaultAvatars,
- }),
- '/accounts/123/avatar?s=16');
+ element._buildAvatarURL({
+ ...createAccountWithId(123),
+ avatars: defaultAvatars,
+ }),
+ '/accounts/123/avatar?s=16'
+ );
assert.equal(
- element._buildAvatarURL({
- email: 'test@example.com',
- avatars: defaultAvatars,
- }),
- '/accounts/test%40example.com/avatar?s=16');
+ element._buildAvatarURL({
+ ...createAccountWithEmail('test@example.com'),
+ avatars: defaultAvatars,
+ }),
+ '/accounts/test%40example.com/avatar?s=16'
+ );
assert.equal(
- element._buildAvatarURL({
- name: 'John Doe',
- avatars: defaultAvatars,
- }),
- '/accounts/John%20Doe/avatar?s=16');
+ element._buildAvatarURL({
+ name: 'John Doe',
+ avatars: defaultAvatars,
+ }),
+ '/accounts/John%20Doe/avatar?s=16'
+ );
assert.equal(
- element._buildAvatarURL({
- username: 'John_Doe',
- avatars: defaultAvatars,
- }),
- '/accounts/John_Doe/avatar?s=16');
+ element._buildAvatarURL({
+ username: 'John_Doe',
+ avatars: defaultAvatars,
+ }),
+ '/accounts/John_Doe/avatar?s=16'
+ );
assert.equal(
- element._buildAvatarURL({
- _account_id: 123,
- avatars: [
- {
- url: 'https://cdn.example.com/s12-p/photo.jpg',
- height: 12,
- },
- {
- url: 'https://cdn.example.com/s16-p/photo.jpg',
- height: 16,
- },
- {
- url: 'https://cdn.example.com/s100-p/photo.jpg',
- height: 100,
- },
- ],
- }),
- 'https://cdn.example.com/s16-p/photo.jpg');
+ element._buildAvatarURL({
+ ...createAccountWithId(123),
+ avatars: [
+ {
+ url: 'https://cdn.example.com/s12-p/photo.jpg',
+ height: 12,
+ width: 0,
+ },
+ {
+ url: 'https://cdn.example.com/s16-p/photo.jpg',
+ height: 16,
+ width: 0,
+ },
+ {
+ url: 'https://cdn.example.com/s100-p/photo.jpg',
+ height: 100,
+ width: 0,
+ },
+ ] as AvatarInfo[],
+ }),
+ 'https://cdn.example.com/s16-p/photo.jpg'
+ );
assert.equal(
- element._buildAvatarURL({
- _account_id: 123,
- avatars: [
- {
- url: 'https://cdn.example.com/s95-p/photo.jpg',
- height: 95,
- },
- ],
- }),
- '/accounts/123/avatar?s=16');
+ element._buildAvatarURL({
+ ...createAccountWithId(123),
+ avatars: [
+ {
+ url: 'https://cdn.example.com/s95-p/photo.jpg',
+ height: 95,
+ width: 0,
+ },
+ ] as AvatarInfo[],
+ }),
+ '/accounts/123/avatar?s=16'
+ );
assert.equal(element._buildAvatarURL(undefined), '');
});
@@ -114,7 +127,7 @@
element.imageSize = 64;
element.account = {
- _account_id: 123,
+ ...createAccountWithId(123),
avatars: defaultAvatars,
};
flush();
@@ -131,14 +144,14 @@
assert.isFalse(element.hasAttribute('hidden'));
assert.isTrue(
- element.style.backgroundImage.includes(
- '/accounts/123/avatar?s=64'));
+ element.style.backgroundImage.includes('/accounts/123/avatar?s=64')
+ );
});
});
});
suite('plugin has avatars', () => {
- let element;
+ let element: GrAvatar;
setup(() => {
stub('gr-avatar', '_getConfig').callsFake(() =>
@@ -166,7 +179,7 @@
});
suite('config not set', () => {
- let element;
+ let element: GrAvatar;
setup(() => {
stub('gr-avatar', '_getConfig').callsFake(() => Promise.resolve({}));
@@ -180,7 +193,7 @@
element.imageSize = 64;
element.account = {
- _account_id: 123,
+ ...createAccountWithId(123),
avatars: defaultAvatars,
};
// Emulate plugins loaded.
@@ -195,4 +208,3 @@
});
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index 65e8e9f..0bd02d5 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -77,11 +77,11 @@
@property({type: Object})
resolveWeblinks?: GeneratedWebLink[] = [];
- _computeStatusString(status: ChangeStates) {
+ _computeStatusString(status?: ChangeStates) {
if (status === ChangeStates.WIP && !this.flat) {
return 'Work in Progress';
}
- return status;
+ return status ?? '';
}
_toClassName(str?: ChangeStates) {
@@ -107,14 +107,14 @@
revertedChange?: ChangeInfo,
resolveWeblinks?: GeneratedWebLink[],
status?: ChangeStates
- ): string | undefined {
+ ): string {
if (revertedChange) {
return GerritNav.getUrlForSearchQuery(`${revertedChange._number}`);
}
if (status === ChangeStates.MERGE_CONFLICT && resolveWeblinks?.length) {
- return resolveWeblinks[0].url;
+ return resolveWeblinks[0].url ?? '';
}
- return undefined;
+ return '';
}
showResolveIcon(
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
index 4a2bcee..36cffb7 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
@@ -20,6 +20,7 @@
import {IronIconElement} from '@polymer/iron-icon';
import {assertIsDefined, queryAndAssert} from '../../../utils/common-util';
import {classMap} from 'lit-html/directives/class-map';
+import {ifDefined} from 'lit-html/directives/if-defined';
import {css, customElement, html, property} from 'lit-element';
import {GrLitElement} from '../../lit/gr-lit-element';
import {GrButton} from '../gr-button/gr-button';
@@ -126,7 +127,7 @@
link=""
?has-tooltip=${this.hasTooltip}
class="copyToClipboard"
- title="${this.buttonTitle}"
+ title="${ifDefined(this.buttonTitle)}"
@click="${this._copyToClipboard}"
aria-label="Click to copy to clipboard"
>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.ts
similarity index 73%
rename from polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js
rename to polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.ts
index 45847d7..ef62fe9 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.ts
@@ -15,14 +15,16 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-copy-clipboard.js';
-import {queryAndAssert} from '../../../test/test-utils.js';
+import '../../../test/common-test-setup-karma';
+import './gr-copy-clipboard';
+import {GrCopyClipboard} from './gr-copy-clipboard';
+import {queryAndAssert} from '../../../test/test-utils';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
const basicFixture = fixtureFromElement('gr-copy-clipboard');
suite('gr-copy-clipboard tests', () => {
- let element;
+ let element: GrCopyClipboard;
setup(async () => {
element = basicFixture.instantiate();
@@ -33,35 +35,34 @@
test('copy to clipboard', () => {
const clipboardSpy = sinon.spy(navigator.clipboard, 'writeText');
- const copyBtn = element.shadowRoot
- .querySelector('.copyToClipboard');
+ const copyBtn = queryAndAssert(element, '.copyToClipboard');
MockInteractions.click(copyBtn);
assert.isTrue(clipboardSpy.called);
});
test('focusOnCopy', () => {
element.focusOnCopy();
- const activeElement = element.shadowRoot.activeElement;
- const button = element.shadowRoot.querySelector('.copyToClipboard');
+ const activeElement = element.shadowRoot!.activeElement;
+ const button = queryAndAssert(element, '.copyToClipboard');
assert.deepEqual(activeElement, button);
});
test('_handleInputClick', () => {
// iron-input as parent should never be hidden as copy won't work
// on nested hidden elements
- const ironInputElement = element.shadowRoot.querySelector('iron-input');
+ const ironInputElement = queryAndAssert(element, 'iron-input');
assert.notEqual(getComputedStyle(ironInputElement).display, 'none');
- const inputElement = element.shadowRoot.querySelector('input');
+ const inputElement = queryAndAssert(element, 'input') as HTMLInputElement;
MockInteractions.tap(inputElement);
assert.equal(inputElement.selectionStart, 0);
- assert.equal(inputElement.selectionEnd, element.text.length - 1);
+ assert.equal(inputElement.selectionEnd, element.text!.length! - 1);
});
test('hideInput', async () => {
// iron-input as parent should never be hidden as copy won't work
// on nested hidden elements
- const ironInputElement = element.shadowRoot.querySelector('iron-input');
+ const ironInputElement = queryAndAssert(element, 'iron-input');
assert.notEqual(getComputedStyle(ironInputElement).display, 'none');
const input = queryAndAssert(element, 'input');
@@ -76,10 +77,8 @@
divParent.appendChild(element);
const clickStub = sinon.stub();
divParent.addEventListener('click', clickStub);
- element.stopPropagation = true;
- const copyBtn = element.shadowRoot.querySelector('.copyToClipboard');
+ const copyBtn = queryAndAssert(element, '.copyToClipboard');
MockInteractions.tap(copyBtn);
assert.isFalse(clickStub.called);
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 83cd380..592efba 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -106,13 +106,14 @@
_saveDisabled!: boolean;
@property({type: String, observer: '_newContentChanged'})
- _newContent?: string;
+ _newContent = '';
private readonly storage = appContext.storageService;
private readonly reporting = appContext.reportingService;
- private storeTask?: DelayedTask;
+ // Tests use this so needs to be non private
+ storeTask?: DelayedTask;
/** @override */
ready() {
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
index c6ff903..7877a1f 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
@@ -69,7 +69,7 @@
box-shadow: var(--elevation-level-1);
/* slightly up to cover rounded corner of the commit msg */
margin-top: calc(-1 * var(--spacing-xs));
- /* To make this bar pop over editor, since editor has relative position.
+ /* To make this bar pop over editor, since editor has relative position.
*/
position: relative;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.js b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
similarity index 73%
rename from polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.js
rename to polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
index 94a7b96..074678e 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
@@ -15,13 +15,17 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-editable-content.js';
+import '../../../test/common-test-setup-karma';
+import './gr-editable-content';
+import {GrEditableContent} from './gr-editable-content';
+import {queryAndAssert, stubStorage} from '../../../test/test-utils';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+import {GrButton} from '../gr-button/gr-button';
const basicFixture = fixtureFromElement('gr-editable-content');
suite('gr-editable-content tests', () => {
- let element;
+ let element: GrEditableContent;
setup(() => {
element = basicFixture.instantiate();
@@ -33,8 +37,7 @@
const handler = sinon.spy();
element.addEventListener('editable-content-save', handler);
- MockInteractions.tap(element.shadowRoot
- .querySelector('gr-button[primary]'));
+ MockInteractions.tap(queryAndAssert(element, 'gr-button[primary]'));
assert.isTrue(handler.called);
assert.equal(handler.lastCall.args[0].detail.content, 'foo');
@@ -44,8 +47,7 @@
const handler = sinon.spy();
element.addEventListener('editable-content-cancel', handler);
- MockInteractions.tap(element.shadowRoot
- .querySelector('gr-button.cancel-button'));
+ MockInteractions.tap(queryAndAssert(element, 'gr-button.cancel-button'));
assert.isTrue(handler.called);
});
@@ -79,19 +81,22 @@
});
test('save button is disabled initially', () => {
- assert.isTrue(element.shadowRoot
- .querySelector('gr-button[primary]').disabled);
+ assert.isTrue(
+ queryAndAssert<GrButton>(element, 'gr-button[primary]').disabled
+ );
});
test('save button is enabled when content changes', () => {
element._newContent = 'new content';
- assert.isFalse(element.shadowRoot
- .querySelector('gr-button[primary]').disabled);
+ assert.isFalse(
+ queryAndAssert<GrButton>(element, 'gr-button[primary]').disabled
+ );
});
});
suite('storageKey and related behavior', () => {
- let dispatchSpy;
+ let dispatchSpy: sinon.SinonSpy;
+
setup(() => {
element.content = 'current content';
element.storageKey = 'test';
@@ -99,8 +104,10 @@
});
test('editing toggled to true, has stored data', () => {
- sinon.stub(element.storage, 'getEditableContentItem')
- .returns({message: 'stored content'});
+ stubStorage('getEditableContentItem').returns({
+ message: 'stored content',
+ updated: 0,
+ });
element.editing = true;
assert.equal(element._newContent, 'stored content');
@@ -109,8 +116,7 @@
});
test('editing toggled to true, has no stored data', () => {
- sinon.stub(element.storage, 'getEditableContentItem')
- .returns({});
+ stubStorage('getEditableContentItem').returns(null);
element.editing = true;
assert.equal(element._newContent, 'current content');
@@ -118,28 +124,26 @@
});
test('edits are cached', () => {
- const storeStub =
- sinon.stub(element.storage, 'setEditableContentItem');
- const eraseStub =
- sinon.stub(element.storage, 'eraseEditableContentItem');
+ const storeStub = stubStorage('setEditableContentItem');
+ const eraseStub = stubStorage('eraseEditableContentItem');
element.editing = true;
element._newContent = 'new content';
flush();
- element.storeTask.flush();
+ element.storeTask?.flush();
assert.isTrue(storeStub.called);
assert.deepEqual(
- [element.storageKey, element._newContent],
- storeStub.lastCall.args);
+ [element.storageKey, element._newContent],
+ storeStub.lastCall.args
+ );
element._newContent = '';
flush();
- element.storeTask.flush();
+ element.storeTask?.flush();
assert.isTrue(eraseStub.called);
assert.deepEqual([element.storageKey], eraseStub.lastCall.args);
});
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 0193197..ddca5c5 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -183,7 +183,9 @@
// include pre or all regular lines but stop at next new line
while (
this._isPreFormat(lines[nextI]) ||
- (this._isRegularLine(lines[nextI]) && lines[nextI].length)
+ (this._isRegularLine(lines[nextI]) &&
+ !this._isWhitespaceLine(lines[nextI]) &&
+ lines[nextI].length)
) {
nextI++;
}
@@ -255,13 +257,17 @@
}
_isPreFormat(line: string) {
- return line && /^[ \t]/.test(line);
+ return line && /^[ \t]/.test(line) && !this._isWhitespaceLine(line);
}
_isList(line: string) {
return line && /^[-*] /.test(line);
}
+ _isWhitespaceLine(line: string) {
+ return line && /^\s+$/.test(line);
+ }
+
_makeLinkedText(content = '', isPre?: boolean) {
const text = document.createElement('gr-linked-text');
text.config = this.config;
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js
index 3e05f11..8464af7 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js
@@ -297,6 +297,14 @@
assertBlock(result, 1, 'paragraph', 'B');
});
+ test('pre format 5', () => {
+ const comment = ' Q\n <R>\n S\n \nB';
+ const result = element._computeBlocks(comment);
+ assert.lengthOf(result, 2);
+ assertBlock(result, 0, 'pre', ' Q\n <R>\n S');
+ assertBlock(result, 1, 'paragraph', ' \nB');
+ });
+
test('quote 1', () => {
const comment = '> I\'m happy\n > with quotes!\n\nSee above.';
const result = element._computeBlocks(comment);
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
index b26ec9b..bbc7b1a 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -111,7 +111,7 @@
if (!drafts[draft.path]) drafts[draft.path] = [] as DraftInfo[];
else drafts[draft.path] = [...drafts[draft.path]];
const index = drafts[draft.path].findIndex(
- d => d.__draftID === draft.__draftID || d.id === draft.id
+ d => (d.__draftID && d.__draftID === draft.__draftID) || d.id === draft.id
);
if (index !== -1) {
drafts[draft.path][index] = draft;
@@ -127,7 +127,7 @@
nextState.drafts = {...nextState.drafts};
const drafts = nextState.drafts;
const index = (drafts[draft.path] || []).findIndex(
- d => d.__draftID === draft.__draftID || d.id === draft.id
+ d => (d.__draftID && d.__draftID === draft.__draftID) || d.id === draft.id
);
if (index === -1) return;
drafts[draft.path] = [...drafts[draft.path]];
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 134003b..1996800 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -128,20 +128,20 @@
--error-foreground: var(--red-700);
--error-background: var(--red-50);
- --error-background-hover: linear-gradient(var(--red-700-04), var(--red-700-04), var(--red-50));
- --error-background-focus: linear-gradient(var(--red-700-12), var(--red-700-12), var(--red-50));
+ --error-background-hover: linear-gradient(var(--red-700-04), var(--red-700-04)), var(--red-50);
+ --error-background-focus: linear-gradient(var(--red-700-12), var(--red-700-12)), var(--red-50);
--error-ripple: var(--red-700-10);
--warning-foreground: var(--orange-700);
--warning-background: var(--orange-50);
- --warning-background-hover: linear-gradient(var(--orange-700-04), var(--orange-700-04), var(--orange-50));
- --warning-background-focus: linear-gradient(var(--orange-700-12), var(--orange-700-12), var(--orange-50));
+ --warning-background-hover: linear-gradient(var(--orange-700-04), var(--orange-700-04)), var(--orange-50);
+ --warning-background-focus: linear-gradient(var(--orange-700-12), var(--orange-700-12)), var(--orange-50);
--warning-ripple: var(--orange-700-10);
--info-foreground: var(--blue-700);
--info-background: var(--blue-50);
- --info-background-hover: linear-gradient(var(--blue-700-04), var(--blue-700-04), var(--blue-50));
- --info-background-focus: linear-gradient(var(--blue-700-12), var(--blue-700-12), var(--blue-50));
+ --info-background-hover: linear-gradient(var(--blue-700-04), var(--blue-700-04)), var(--blue-50);
+ --info-background-focus: linear-gradient(var(--blue-700-12), var(--blue-700-12)), var(--blue-50);
--info-ripple: var(--blue-700-10);
--primary-button-text-color: white;
@@ -154,14 +154,14 @@
--success-foreground: var(--green-700);
--success-background: var(--green-50);
- --success-background-hover: linear-gradient(var(--green-700-04), var(--green-700-04), var(--green-50));
- --success-background-focus: linear-gradient(var(--green-700-12), var(--green-700-12), var(--green-50));
+ --success-background-hover: linear-gradient(var(--green-700-04), var(--green-700-04)), var(--green-50);
+ --success-background-focus: linear-gradient(var(--green-700-12), var(--green-700-12)), var(--green-50);
--success-ripple: var(--green-700-10);
--gray-foreground: var(--gray-700);
--gray-background: var(--gray-100);
- --gray-background-hover: linear-gradient(var(--gray-700-04), var(--gray-700-04), var(--gray-100));
- --gray-background-focus: linear-gradient(var(--gray-700-12), var(--gray-700-12), var(--gray-100));
+ --gray-background-hover: linear-gradient(var(--gray-700-04), var(--gray-700-04)), var(--gray-100);
+ --gray-background-focus: linear-gradient(var(--gray-700-12), var(--gray-700-12)), var(--gray-100);
--gray-ripple: var(--gray-700-10);
--disabled-foreground: var(--gray-800-38);
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index 69256b2..926b02d 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -38,20 +38,20 @@
--error-foreground: var(--red-200);
--error-background: var(--red-tonal);
- --error-background-hover: linear-gradient(var(--white-04), var(--white-04), var(--red-tonal));
- --error-background-focus: linear-gradient(var(--white-12), var(--white-12), var(--red-tonal));
+ --error-background-hover: linear-gradient(var(--white-04), var(--white-04)), var(--red-tonal);
+ --error-background-focus: linear-gradient(var(--white-12), var(--white-12)), var(--red-tonal);
--error-ripple: var(--white-10);
--warning-foreground: var(--orange-200);
--warning-background: var(--orange-tonal);
- --warning-background-hover: linear-gradient(var(--white-04), var(--white-04), var(--orange-tonal));
- --warning-background-focus: linear-gradient(var(--white-12), var(--white-12), var(--orange-tonal));
+ --warning-background-hover: linear-gradient(var(--white-04), var(--white-04)), var(--orange-tonal);
+ --warning-background-focus: linear-gradient(var(--white-12), var(--white-12)), var(--orange-tonal);
--warning-ripple: var(--white-10);
--info-foreground: var(--blue-200);
--info-background: var(--blue-tonal);
- --info-background-hover: linear-gradient(var(--white-04), var(--white-04), var(--blue-tonal));
- --info-background-focus: linear-gradient(var(--white-12), var(--white-12), var(--blue-tonal));
+ --info-background-hover: linear-gradient(var(--white-04), var(--white-04)), var(--blue-tonal);
+ --info-background-focus: linear-gradient(var(--white-12), var(--white-12)), var(--blue-tonal);
--info-ripple: var(--white-10);
--primary-button-text-color: black;
@@ -64,14 +64,14 @@
--success-foreground: var(--green-200);
--success-background: var(--green-tonal);
- --success-background-hover: linear-gradient(var(--white-04), var(--white-04), var(--green-tonal));
- --success-background-focus: linear-gradient(var(--white-12), var(--white-12), var(--green-tonal));
+ --success-background-hover: linear-gradient(var(--white-04), var(--white-04)), var(--green-tonal);
+ --success-background-focus: linear-gradient(var(--white-12), var(--white-12)), var(--green-tonal);
--success-ripple: var(--white-10);
--gray-foreground: var(--gray-300);
--gray-background: var(--gray-tonal);
- --gray-background-hover: linear-gradient(var(--white-04), var(--white-04), var(--gray-tonal));
- --gray-background-focus: linear-gradient(var(--white-12), var(--white-12), var(--gray-tonal));
+ --gray-background-hover: linear-gradient(var(--white-04), var(--white-04)), var(--gray-tonal);
+ --gray-background-focus: linear-gradient(var(--white-12), var(--white-12)), var(--gray-tonal);
--gray-ripple: var(--white-10);
--disabled-foreground: var(--gray-200-38);
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 0f2608e..96c05ee 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -43,6 +43,7 @@
ActionNameToActionInfoMap,
ApprovalInfo,
AuthInfo,
+ AvatarInfo,
BasePatchSetNum,
BranchName,
BrandType,
@@ -124,6 +125,7 @@
ActionNameToActionInfoMap,
ApprovalInfo,
AuthInfo,
+ AvatarInfo,
BasePatchSetNum,
BranchName,
BrandType,
diff --git a/polygerrit-ui/app/utils/async-util_test.js b/polygerrit-ui/app/utils/async-util_test.js
deleted file mode 100644
index df29e97..0000000
--- a/polygerrit-ui/app/utils/async-util_test.js
+++ /dev/null
@@ -1,46 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../test/common-test-setup-karma.js';
-import {asyncForeach} from './async-util.js';
-
-suite('async-util tests', () => {
- test('loops over each item', () => {
- const fn = sinon.stub().returns(Promise.resolve());
- return asyncForeach([1, 2, 3], fn)
- .then(() => {
- assert.isTrue(fn.calledThrice);
- assert.equal(fn.getCall(0).args[0], 1);
- assert.equal(fn.getCall(1).args[0], 2);
- assert.equal(fn.getCall(2).args[0], 3);
- });
- });
-
- test('halts on stop condition', () => {
- const stub = sinon.stub();
- const fn = (e, stop) => {
- stub(e);
- stop();
- return Promise.resolve();
- };
- return asyncForeach([1, 2, 3], fn)
- .then(() => {
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args[0], 1);
- });
- });
-});
diff --git a/polygerrit-ui/app/utils/async-util_test.ts b/polygerrit-ui/app/utils/async-util_test.ts
new file mode 100644
index 0000000..5c8f610
--- /dev/null
+++ b/polygerrit-ui/app/utils/async-util_test.ts
@@ -0,0 +1,46 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../test/common-test-setup-karma';
+import {asyncForeach} from './async-util';
+
+suite('async-util tests', () => {
+ test('loops over each item', async () => {
+ const fn = sinon.stub().resolves();
+
+ await asyncForeach([1, 2, 3], fn);
+
+ assert.isTrue(fn.calledThrice);
+ assert.equal(fn.firstCall.firstArg, 1);
+ assert.equal(fn.secondCall.firstArg, 2);
+ assert.equal(fn.thirdCall.firstArg, 3);
+ });
+
+ test('halts on stop condition', async () => {
+ const stub = sinon.stub();
+ const fn = (item: number, stopCallback: () => void) => {
+ stub(item);
+ stopCallback();
+ return Promise.resolve();
+ };
+
+ await asyncForeach([1, 2, 3], fn);
+
+ assert.isTrue(stub.calledOnce);
+ assert.equal(stub.lastCall.firstArg, 1);
+ });
+});
diff --git a/polygerrit-ui/app/utils/date-util_test.js b/polygerrit-ui/app/utils/date-util_test.js
deleted file mode 100644
index 96d5bc1..0000000
--- a/polygerrit-ui/app/utils/date-util_test.js
+++ /dev/null
@@ -1,144 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 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.
- */
-import '../test/common-test-setup-karma.js';
-import {isValidDate, parseDate, fromNow, isWithinDay, isWithinHalfYear, formatDate, wasYesterday} from './date-util.js';
-
-suite('date-util tests', () => {
- suite('parseDate', () => {
- test('parseDate server date', () => {
- const parsed = parseDate('2015-09-15 20:34:00.000000000');
- assert.equal('2015-09-15T20:34:00.000Z', parsed.toISOString());
- });
- });
-
- suite('isValidDate', () => {
- test('date is valid', () => {
- assert.isTrue(isValidDate(new Date()));
- });
- test('broken date is invalid', () => {
- assert.isFalse(isValidDate(new Date('xxx')));
- });
- });
-
- suite('fromNow', () => {
- test('test all variants', () => {
- const fakeNow = new Date('May 08 2020 12:00:00');
- sinon.useFakeTimers(fakeNow.getTime());
- assert.equal('just now', fromNow(new Date('May 08 2020 11:59:30')));
- assert.equal('1 minute ago', fromNow(new Date('May 08 2020 11:59:00')));
- assert.equal('5 minutes ago', fromNow(new Date('May 08 2020 11:55:00')));
- assert.equal('1 hour ago', fromNow(new Date('May 08 2020 11:00:00')));
- assert.equal(
- '1 hour 5 min ago', fromNow(new Date('May 08 2020 10:55:00')));
- assert.equal('3 hours ago', fromNow(new Date('May 08 2020 9:00:00')));
- assert.equal('1 day ago', fromNow(new Date('May 07 2020 12:00:00')));
- assert.equal('1 day 2 hr ago', fromNow(new Date('May 07 2020 10:00:00')));
- assert.equal('3 days ago', fromNow(new Date('May 05 2020 12:00:00')));
- assert.equal('1 month ago', fromNow(new Date('Apr 05 2020 12:00:00')));
- assert.equal('2 months ago', fromNow(new Date('Mar 05 2020 12:00:00')));
- assert.equal('1 year ago', fromNow(new Date('May 05 2019 12:00:00')));
- assert.equal('10 years ago', fromNow(new Date('May 05 2010 12:00:00')));
- });
- test('rounding error', () => {
- const fakeNow = new Date('May 08 2020 12:00:00');
- sinon.useFakeTimers(fakeNow.getTime());
- assert.equal('2 hours ago', fromNow(new Date('May 08 2020 9:30:00')));
- });
- });
-
- suite('isWithinDay', () => {
- test('basics works', () => {
- assert.isTrue(isWithinDay(new Date('May 08 2020 12:00:00'),
- new Date('May 08 2020 02:00:00')));
- assert.isFalse(isWithinDay(new Date('May 08 2020 12:00:00'),
- new Date('May 07 2020 12:00:00')));
- });
- });
-
- suite('wasYesterday', () => {
- test('less 24 hours', () => {
- assert.isFalse(wasYesterday(new Date('May 08 2020 12:00:00'),
- new Date('May 08 2020 02:00:00')));
- assert.isTrue(wasYesterday(new Date('May 08 2020 12:00:00'),
- new Date('May 07 2020 12:00:00')));
- });
- test('more 24 hours', () => {
- assert.isTrue(wasYesterday(new Date('May 08 2020 12:00:00'),
- new Date('May 07 2020 2:00:00')));
- assert.isFalse(wasYesterday(new Date('May 08 2020 12:00:00'),
- new Date('May 06 2020 14:00:00')));
- });
- });
-
- suite('isWithinHalfYear', () => {
- test('basics works', () => {
- assert.isTrue(isWithinHalfYear(new Date('May 08 2020 12:00:00'),
- new Date('Feb 08 2020 12:00:00')));
- assert.isFalse(isWithinHalfYear(new Date('May 08 2020 12:00:00'),
- new Date('Nov 07 2019 12:00:00')));
- });
- });
-
- suite('formatDate', () => {
- test('works for standard format', () => {
- const stdFormat = 'MMM DD, YYYY';
- assert.equal('May 08, 2020',
- formatDate(new Date('May 08 2020 12:00:00'), stdFormat));
- assert.equal('Feb 28, 2020',
- formatDate(new Date('Feb 28 2020 12:00:00'), stdFormat));
-
- const time24Format = 'HH:mm:ss';
- assert.equal('Feb 28, 2020 12:01:12',
- formatDate(new Date('Feb 28 2020 12:01:12'), stdFormat + ' '
- + time24Format));
- });
- test('works for euro format', () => {
- const euroFormat = 'DD.MM.YYYY';
- assert.equal('01.12.2019',
- formatDate(new Date('Dec 01 2019 12:00:00'), euroFormat));
- assert.equal('20.01.2002',
- formatDate(new Date('Jan 20 2002 12:00:00'), euroFormat));
-
- const time24Format = 'HH:mm:ss';
- assert.equal('28.02.2020 00:01:12',
- formatDate(new Date('Feb 28 2020 00:01:12'), euroFormat + ' '
- + time24Format));
- });
- test('works for iso format', () => {
- const isoFormat = 'YYYY-MM-DD';
- assert.equal('2015-01-01',
- formatDate(new Date('Jan 01 2015 12:00:00'), isoFormat));
- assert.equal('2013-07-03',
- formatDate(new Date('Jul 03 2013 12:00:00'), isoFormat));
-
- const timeFormat = 'h:mm:ss A';
- assert.equal('2013-07-03 5:00:00 AM',
- formatDate(new Date('Jul 03 2013 05:00:00'), isoFormat + ' '
- + timeFormat));
- assert.equal('2013-07-03 5:00:00 PM',
- formatDate(new Date('Jul 03 2013 17:00:00'), isoFormat + ' '
- + timeFormat));
- });
- test('h:mm:ss A shows correctly midnight and midday', () => {
- const timeFormat = 'h:mm A';
- assert.equal('12:14 PM',
- formatDate(new Date('Jul 03 2013 12:14:00'), timeFormat));
- assert.equal('12:15 AM',
- formatDate(new Date('Jul 03 2013 00:15:00'), timeFormat));
- });
- });
-});
diff --git a/polygerrit-ui/app/utils/date-util_test.ts b/polygerrit-ui/app/utils/date-util_test.ts
new file mode 100644
index 0000000..f17ced3
--- /dev/null
+++ b/polygerrit-ui/app/utils/date-util_test.ts
@@ -0,0 +1,219 @@
+/**
+ * @license
+ * Copyright (C) 2020 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.
+ */
+import {Timestamp} from '../types/common';
+import '../test/common-test-setup-karma';
+import {
+ isValidDate,
+ parseDate,
+ fromNow,
+ isWithinDay,
+ isWithinHalfYear,
+ formatDate,
+ wasYesterday,
+} from './date-util';
+
+suite('date-util tests', () => {
+ suite('parseDate', () => {
+ test('parseDate server date', () => {
+ const parsed = parseDate('2015-09-15 20:34:00.000000000' as Timestamp);
+ assert.equal('2015-09-15T20:34:00.000Z', parsed.toISOString());
+ });
+ });
+
+ suite('isValidDate', () => {
+ test('date is valid', () => {
+ assert.isTrue(isValidDate(new Date()));
+ });
+ test('broken date is invalid', () => {
+ assert.isFalse(isValidDate(new Date('xxx')));
+ });
+ });
+
+ suite('fromNow', () => {
+ test('test all variants', () => {
+ const fakeNow = new Date('May 08 2020 12:00:00');
+ sinon.useFakeTimers(fakeNow.getTime());
+ assert.equal('just now', fromNow(new Date('May 08 2020 11:59:30')));
+ assert.equal('1 minute ago', fromNow(new Date('May 08 2020 11:59:00')));
+ assert.equal('5 minutes ago', fromNow(new Date('May 08 2020 11:55:00')));
+ assert.equal('1 hour ago', fromNow(new Date('May 08 2020 11:00:00')));
+ assert.equal(
+ '1 hour 5 min ago',
+ fromNow(new Date('May 08 2020 10:55:00'))
+ );
+ assert.equal('3 hours ago', fromNow(new Date('May 08 2020 9:00:00')));
+ assert.equal('1 day ago', fromNow(new Date('May 07 2020 12:00:00')));
+ assert.equal('1 day 2 hr ago', fromNow(new Date('May 07 2020 10:00:00')));
+ assert.equal('3 days ago', fromNow(new Date('May 05 2020 12:00:00')));
+ assert.equal('1 month ago', fromNow(new Date('Apr 05 2020 12:00:00')));
+ assert.equal('2 months ago', fromNow(new Date('Mar 05 2020 12:00:00')));
+ assert.equal('1 year ago', fromNow(new Date('May 05 2019 12:00:00')));
+ assert.equal('10 years ago', fromNow(new Date('May 05 2010 12:00:00')));
+ });
+ test('rounding error', () => {
+ const fakeNow = new Date('May 08 2020 12:00:00');
+ sinon.useFakeTimers(fakeNow.getTime());
+ assert.equal('2 hours ago', fromNow(new Date('May 08 2020 9:30:00')));
+ });
+ });
+
+ suite('isWithinDay', () => {
+ test('basics works', () => {
+ assert.isTrue(
+ isWithinDay(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 08 2020 02:00:00')
+ )
+ );
+ assert.isFalse(
+ isWithinDay(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 07 2020 12:00:00')
+ )
+ );
+ });
+ });
+
+ suite('wasYesterday', () => {
+ test('less 24 hours', () => {
+ assert.isFalse(
+ wasYesterday(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 08 2020 02:00:00')
+ )
+ );
+ assert.isTrue(
+ wasYesterday(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 07 2020 12:00:00')
+ )
+ );
+ });
+ test('more 24 hours', () => {
+ assert.isTrue(
+ wasYesterday(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 07 2020 2:00:00')
+ )
+ );
+ assert.isFalse(
+ wasYesterday(
+ new Date('May 08 2020 12:00:00'),
+ new Date('May 06 2020 14:00:00')
+ )
+ );
+ });
+ });
+
+ suite('isWithinHalfYear', () => {
+ test('basics works', () => {
+ assert.isTrue(
+ isWithinHalfYear(
+ new Date('May 08 2020 12:00:00'),
+ new Date('Feb 08 2020 12:00:00')
+ )
+ );
+ assert.isFalse(
+ isWithinHalfYear(
+ new Date('May 08 2020 12:00:00'),
+ new Date('Nov 07 2019 12:00:00')
+ )
+ );
+ });
+ });
+
+ suite('formatDate', () => {
+ test('works for standard format', () => {
+ const stdFormat = 'MMM DD, YYYY';
+ assert.equal(
+ 'May 08, 2020',
+ formatDate(new Date('May 08 2020 12:00:00'), stdFormat)
+ );
+ assert.equal(
+ 'Feb 28, 2020',
+ formatDate(new Date('Feb 28 2020 12:00:00'), stdFormat)
+ );
+
+ const time24Format = 'HH:mm:ss';
+ assert.equal(
+ 'Feb 28, 2020 12:01:12',
+ formatDate(
+ new Date('Feb 28 2020 12:01:12'),
+ stdFormat + ' ' + time24Format
+ )
+ );
+ });
+ test('works for euro format', () => {
+ const euroFormat = 'DD.MM.YYYY';
+ assert.equal(
+ '01.12.2019',
+ formatDate(new Date('Dec 01 2019 12:00:00'), euroFormat)
+ );
+ assert.equal(
+ '20.01.2002',
+ formatDate(new Date('Jan 20 2002 12:00:00'), euroFormat)
+ );
+
+ const time24Format = 'HH:mm:ss';
+ assert.equal(
+ '28.02.2020 00:01:12',
+ formatDate(
+ new Date('Feb 28 2020 00:01:12'),
+ euroFormat + ' ' + time24Format
+ )
+ );
+ });
+ test('works for iso format', () => {
+ const isoFormat = 'YYYY-MM-DD';
+ assert.equal(
+ '2015-01-01',
+ formatDate(new Date('Jan 01 2015 12:00:00'), isoFormat)
+ );
+ assert.equal(
+ '2013-07-03',
+ formatDate(new Date('Jul 03 2013 12:00:00'), isoFormat)
+ );
+
+ const timeFormat = 'h:mm:ss A';
+ assert.equal(
+ '2013-07-03 5:00:00 AM',
+ formatDate(
+ new Date('Jul 03 2013 05:00:00'),
+ isoFormat + ' ' + timeFormat
+ )
+ );
+ assert.equal(
+ '2013-07-03 5:00:00 PM',
+ formatDate(
+ new Date('Jul 03 2013 17:00:00'),
+ isoFormat + ' ' + timeFormat
+ )
+ );
+ });
+ test('h:mm:ss A shows correctly midnight and midday', () => {
+ const timeFormat = 'h:mm A';
+ assert.equal(
+ '12:14 PM',
+ formatDate(new Date('Jul 03 2013 12:14:00'), timeFormat)
+ );
+ assert.equal(
+ '12:15 AM',
+ formatDate(new Date('Jul 03 2013 00:15:00'), timeFormat)
+ );
+ });
+ });
+});
diff --git a/polygerrit-ui/app/utils/display-name-util_test.js b/polygerrit-ui/app/utils/display-name-util_test.js
deleted file mode 100644
index 9bb68dc..0000000
--- a/polygerrit-ui/app/utils/display-name-util_test.js
+++ /dev/null
@@ -1,200 +0,0 @@
-/**
- * @license
- * 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.
- */
-
-import '../test/common-test-setup-karma.js';
-import {getDisplayName, getUserName, getGroupDisplayName, getAccountDisplayName, _testOnly_accountEmail} from './display-name-util.js';
-
-suite('display-name-utils tests', () => {
- // eslint-disable-next-line no-unused-vars
- const config = {
- user: {
- anonymous_coward_name: 'Anonymous Coward',
- },
- };
-
- test('getDisplayName name only', () => {
- const account = {
- name: 'test-name',
- };
- assert.equal(getDisplayName(config, account),
- 'test-name');
- });
-
- test('getDisplayName prefer displayName', () => {
- const account = {
- name: 'test-name',
- display_name: 'better-name',
- };
- assert.equal(getDisplayName(config, account),
- 'better-name');
- });
-
- test('getDisplayName prefer username default', () => {
- const account = {
- name: 'test-name',
- username: 'user-name',
- };
- const config = {
- accounts: {
- default_display_name: 'USERNAME',
- },
- };
- assert.equal(getDisplayName(config, account),
- 'user-name');
- });
-
- test('getDisplayName firstNameOnly', () => {
- const account = {
- name: 'firstname lastname',
- };
- assert.equal(getDisplayName(config, account, true), 'firstname');
- });
-
- test('getDisplayName prefer first name default', () => {
- const account = {
- name: 'firstname lastname',
- };
- const config = {
- accounts: {
- default_display_name: 'FIRST_NAME',
- },
- };
- assert.equal(getDisplayName(config, account),
- 'firstname');
- });
-
- test('getDisplayName ignore leading whitespace for first name', () => {
- const account = {
- name: ' firstname lastname',
- };
- const config = {
- accounts: {
- default_display_name: 'FIRST_NAME',
- },
- };
- assert.equal(getDisplayName(config, account),
- 'firstname');
- });
-
- test('getDisplayName full name default', () => {
- const account = {
- name: 'firstname lastname',
- };
- const config = {
- accounts: {
- default_display_name: 'FULL_NAME',
- },
- };
- assert.equal(getDisplayName(config, account),
- 'firstname lastname');
- });
-
- test('getDisplayName name only', () => {
- const account = {
- name: 'test-name',
- };
- assert.deepEqual(getUserName(config, account),
- 'test-name');
- });
-
- test('getUserName username only', () => {
- const account = {
- username: 'test-user',
- };
- assert.deepEqual(getUserName(config, account),
- 'test-user');
- });
-
- test('getUserName email only', () => {
- const account = {
- email: 'test-user@test-url.com',
- };
- assert.deepEqual(getUserName(config, account),
- 'test-user@test-url.com');
- });
-
- test('getUserName returns not Anonymous Coward as the anon name', () => {
- assert.deepEqual(getUserName(config, null),
- 'Anonymous');
- });
-
- test('getUserName for the config returning the anon name', () => {
- const config = {
- user: {
- anonymous_coward_name: 'Test Anon',
- },
- };
- assert.deepEqual(getUserName(config, null),
- 'Test Anon');
- });
-
- test('getAccountDisplayName - account with name only', () => {
- assert.equal(
- getAccountDisplayName(config,
- {name: 'Some user name'}),
- 'Some user name');
- });
-
- test('getAccountDisplayName - account with email only', () => {
- assert.equal(
- getAccountDisplayName(config,
- {email: 'my@example.com'}),
- 'my@example.com <my@example.com>');
- });
-
- test('getAccountDisplayName - account with name and status', () => {
- assert.equal(
- getAccountDisplayName(config, {
- name: 'Some name',
- status: 'OOO',
- }),
- 'Some name (OOO)');
- });
-
- test('getAccountDisplayName - account with name and email', () => {
- assert.equal(
- getAccountDisplayName(config, {
- name: 'Some name',
- email: 'my@example.com',
- }),
- 'Some name <my@example.com>');
- });
-
- test('getAccountDisplayName - account with name, email and status', () => {
- assert.equal(
- getAccountDisplayName(config, {
- name: 'Some name',
- email: 'my@example.com',
- status: 'OOO',
- }),
- 'Some name <my@example.com> (OOO)');
- });
-
- test('getGroupDisplayName', () => {
- assert.equal(
- getGroupDisplayName({name: 'Some user name'}),
- 'Some user name (group)');
- });
-
- test('_accountEmail', () => {
- assert.equal(
- _testOnly_accountEmail('email@gerritreview.com'),
- '<email@gerritreview.com>');
- assert.equal(_testOnly_accountEmail(undefined), '');
- });
-});
-
diff --git a/polygerrit-ui/app/utils/display-name-util_test.ts b/polygerrit-ui/app/utils/display-name-util_test.ts
new file mode 100644
index 0000000..e6d4704
--- /dev/null
+++ b/polygerrit-ui/app/utils/display-name-util_test.ts
@@ -0,0 +1,225 @@
+/**
+ * @license
+ * 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.
+ */
+
+import {
+ AccountInfo,
+ DefaultDisplayNameConfig,
+ EmailAddress,
+ GroupName,
+ ServerInfo,
+} from '../api/rest-api';
+import '../test/common-test-setup-karma';
+import {
+ getDisplayName,
+ getUserName,
+ getGroupDisplayName,
+ getAccountDisplayName,
+ _testOnly_accountEmail,
+} from './display-name-util';
+import {
+ createAccountsConfig,
+ createGroupInfo,
+ createServerInfo,
+} from '../test/test-data-generators';
+
+suite('display-name-utils tests', () => {
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ user: {
+ anonymous_coward_name: 'Anonymous Coward',
+ },
+ };
+
+ test('getDisplayName name only', () => {
+ const account = {
+ name: 'test-name',
+ };
+ assert.equal(getDisplayName(config, account), 'test-name');
+ });
+
+ test('getDisplayName prefer displayName', () => {
+ const account = {
+ name: 'test-name',
+ display_name: 'better-name',
+ };
+ assert.equal(getDisplayName(config, account), 'better-name');
+ });
+
+ test('getDisplayName prefer username default', () => {
+ const account = {
+ name: 'test-name',
+ username: 'user-name',
+ };
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ accounts: {
+ ...createAccountsConfig(),
+ default_display_name: DefaultDisplayNameConfig.USERNAME,
+ },
+ };
+ assert.equal(getDisplayName(config, account), 'user-name');
+ });
+
+ test('getDisplayName firstNameOnly', () => {
+ const account = {
+ name: 'firstname lastname',
+ };
+ assert.equal(getDisplayName(config, account, true), 'firstname');
+ });
+
+ test('getDisplayName prefer first name default', () => {
+ const account = {
+ name: 'firstname lastname',
+ };
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ accounts: {
+ ...createAccountsConfig(),
+ default_display_name: DefaultDisplayNameConfig.FIRST_NAME,
+ },
+ };
+ assert.equal(getDisplayName(config, account), 'firstname');
+ });
+
+ test('getDisplayName ignore leading whitespace for first name', () => {
+ const account = {
+ name: ' firstname lastname',
+ };
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ accounts: {
+ ...createAccountsConfig(),
+ default_display_name: DefaultDisplayNameConfig.FIRST_NAME,
+ },
+ };
+ assert.equal(getDisplayName(config, account), 'firstname');
+ });
+
+ test('getDisplayName full name default', () => {
+ const account = {
+ name: 'firstname lastname',
+ };
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ accounts: {
+ ...createAccountsConfig(),
+ default_display_name: DefaultDisplayNameConfig.FULL_NAME,
+ },
+ };
+ assert.equal(getDisplayName(config, account), 'firstname lastname');
+ });
+
+ test('getDisplayName name only', () => {
+ const account = {
+ name: 'test-name',
+ };
+ assert.deepEqual(getUserName(config, account), 'test-name');
+ });
+
+ test('getUserName username only', () => {
+ const account = {
+ username: 'test-user',
+ };
+ assert.deepEqual(getUserName(config, account), 'test-user');
+ });
+
+ test('getUserName email only', () => {
+ const account: AccountInfo = {
+ email: 'test-user@test-url.com' as EmailAddress,
+ };
+ assert.deepEqual(getUserName(config, account), 'test-user@test-url.com');
+ });
+
+ test('getUserName returns not Anonymous Coward as the anon name', () => {
+ assert.deepEqual(getUserName(config, undefined), 'Anonymous');
+ });
+
+ test('getUserName for the config returning the anon name', () => {
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ user: {
+ anonymous_coward_name: 'Test Anon',
+ },
+ };
+ assert.deepEqual(getUserName(config, undefined), 'Test Anon');
+ });
+
+ test('getAccountDisplayName - account with name only', () => {
+ assert.equal(
+ getAccountDisplayName(config, {name: 'Some user name'}),
+ 'Some user name'
+ );
+ });
+
+ test('getAccountDisplayName - account with email only', () => {
+ assert.equal(
+ getAccountDisplayName(config, {
+ email: 'my@example.com' as EmailAddress,
+ }),
+ 'my@example.com <my@example.com>'
+ );
+ });
+
+ test('getAccountDisplayName - account with name and status', () => {
+ assert.equal(
+ getAccountDisplayName(config, {
+ name: 'Some name',
+ status: 'OOO',
+ }),
+ 'Some name (OOO)'
+ );
+ });
+
+ test('getAccountDisplayName - account with name and email', () => {
+ assert.equal(
+ getAccountDisplayName(config, {
+ name: 'Some name',
+ email: 'my@example.com' as EmailAddress,
+ }),
+ 'Some name <my@example.com>'
+ );
+ });
+
+ test('getAccountDisplayName - account with name, email and status', () => {
+ assert.equal(
+ getAccountDisplayName(config, {
+ name: 'Some name',
+ email: 'my@example.com' as EmailAddress,
+ status: 'OOO',
+ }),
+ 'Some name <my@example.com> (OOO)'
+ );
+ });
+
+ test('getGroupDisplayName', () => {
+ assert.equal(
+ getGroupDisplayName({
+ ...createGroupInfo(),
+ name: 'Some user name' as GroupName,
+ }),
+ 'Some user name (group)'
+ );
+ });
+
+ test('_accountEmail', () => {
+ assert.equal(
+ _testOnly_accountEmail('email@gerritreview.com'),
+ '<email@gerritreview.com>'
+ );
+ assert.equal(_testOnly_accountEmail(undefined), '');
+ });
+});
diff --git a/polygerrit-ui/app/utils/path-list-util_test.js b/polygerrit-ui/app/utils/path-list-util_test.js
deleted file mode 100644
index 4d06344..0000000
--- a/polygerrit-ui/app/utils/path-list-util_test.js
+++ /dev/null
@@ -1,161 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 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.
- */
-
-import '../test/common-test-setup-karma.js';
-import {SpecialFilePath} from '../constants/constants.js';
-import {
- addUnmodifiedFiles,
- computeDisplayPath,
- isMagicPath,
- specialFilePathCompare, truncatePath,
-} from './path-list-util.js';
-
-suite('path-list-utl tests', () => {
- test('special sort', () => {
- const testFiles = [
- '/a.h',
- '/MERGE_LIST',
- '/a.cpp',
- '/COMMIT_MSG',
- '/asdasd',
- '/mrPeanutbutter.py',
- ];
- assert.deepEqual(
- testFiles.sort(specialFilePathCompare),
- [
- '/COMMIT_MSG',
- '/MERGE_LIST',
- '/a.h',
- '/a.cpp',
- '/asdasd',
- '/mrPeanutbutter.py',
- ]);
- });
-
- test('special file path sorting', () => {
- assert.deepEqual(
- ['.b', '/COMMIT_MSG', '.a', 'file'].sort(
- specialFilePathCompare),
- ['/COMMIT_MSG', '.a', '.b', 'file']);
-
- assert.deepEqual(
- ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.h'].sort(
- specialFilePathCompare),
- ['/COMMIT_MSG', '.b', 'foo/bar/baz.h', 'foo/bar/baz.cc']);
-
- assert.deepEqual(
- ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.hpp'].sort(
- specialFilePathCompare),
- ['/COMMIT_MSG', '.b', 'foo/bar/baz.hpp', 'foo/bar/baz.cc']);
-
- assert.deepEqual(
- ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.hxx'].sort(
- specialFilePathCompare),
- ['/COMMIT_MSG', '.b', 'foo/bar/baz.hxx', 'foo/bar/baz.cc']);
-
- assert.deepEqual(
- ['foo/bar.h', 'foo/bar.hxx', 'foo/bar.hpp'].sort(
- specialFilePathCompare),
- ['foo/bar.h', 'foo/bar.hpp', 'foo/bar.hxx']);
-
- // Regression test for Issue 4448.
- assert.deepEqual(
- [
- 'minidump/minidump_memory_writer.cc',
- 'minidump/minidump_memory_writer.h',
- 'minidump/minidump_thread_writer.cc',
- 'minidump/minidump_thread_writer.h',
- ].sort(specialFilePathCompare),
- [
- 'minidump/minidump_memory_writer.h',
- 'minidump/minidump_memory_writer.cc',
- 'minidump/minidump_thread_writer.h',
- 'minidump/minidump_thread_writer.cc',
- ]);
-
- // Regression test for Issue 4545.
- assert.deepEqual(
- [
- 'task_test.go',
- 'task.go',
- ].sort(specialFilePathCompare),
- [
- 'task.go',
- 'task_test.go',
- ]);
- });
-
- test('file display name', () => {
- assert.equal(computeDisplayPath('/foo/bar/baz'), '/foo/bar/baz');
- assert.equal(computeDisplayPath('/foobarbaz'), '/foobarbaz');
- assert.equal(computeDisplayPath('/COMMIT_MSG'), 'Commit message');
- assert.equal(computeDisplayPath('/MERGE_LIST'), 'Merge list');
- });
-
- test('isMagicPath', () => {
- assert.isFalse(isMagicPath(undefined));
- assert.isFalse(isMagicPath('/foo.cc'));
- assert.isTrue(isMagicPath('/COMMIT_MSG'));
- assert.isTrue(isMagicPath('/MERGE_LIST'));
- });
-
- test('patchset level comments are hidden', () => {
- const commentedPaths = {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: true,
- 'file1.txt': true,
- };
-
- const files = {'file2.txt': {status: 'M'}};
- addUnmodifiedFiles(files, commentedPaths);
- assert.equal(files['file1.txt'].status, 'U');
- assert.equal(files['file2.txt'].status, 'M');
- assert.isFalse(files.hasOwnProperty(
- SpecialFilePath.PATCHSET_LEVEL_COMMENTS));
- });
-
- test('truncatePath with long path should add ellipsis', () => {
- let path = 'level1/level2/level3/level4/file.js';
- let shortenedPath = truncatePath(path);
- // The expected path is truncated with an ellipsis.
- const expectedPath = '\u2026/file.js';
- assert.equal(shortenedPath, expectedPath);
-
- path = 'level2/file.js';
- shortenedPath = truncatePath(path);
- assert.equal(shortenedPath, expectedPath);
- });
-
- test('truncatePath with opt_threshold', () => {
- let path = 'level1/level2/level3/level4/file.js';
- let shortenedPath = truncatePath(path, 2);
- // The expected path is truncated with an ellipsis.
- const expectedPath = '\u2026/level4/file.js';
- assert.equal(shortenedPath, expectedPath);
-
- path = 'level2/file.js';
- shortenedPath = truncatePath(path, 2);
- assert.equal(shortenedPath, path);
- });
-
- test('truncatePath with short path should not add ellipsis', () => {
- const path = 'file.js';
- const expectedPath = 'file.js';
- const shortenedPath = truncatePath(path);
- assert.equal(shortenedPath, expectedPath);
- });
-});
-
diff --git a/polygerrit-ui/app/utils/path-list-util_test.ts b/polygerrit-ui/app/utils/path-list-util_test.ts
new file mode 100644
index 0000000..79b5f09
--- /dev/null
+++ b/polygerrit-ui/app/utils/path-list-util_test.ts
@@ -0,0 +1,170 @@
+/**
+ * @license
+ * Copyright (C) 2016 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.
+ */
+
+import '../test/common-test-setup-karma';
+import {FileInfoStatus, SpecialFilePath} from '../constants/constants';
+import {
+ addUnmodifiedFiles,
+ computeDisplayPath,
+ isMagicPath,
+ specialFilePathCompare,
+ truncatePath,
+} from './path-list-util';
+import {FileInfo} from '../api/rest-api';
+import {hasOwnProperty} from './common-util';
+
+suite('path-list-utl tests', () => {
+ test('special sort', () => {
+ const testFiles = [
+ '/a.h',
+ '/MERGE_LIST',
+ '/a.cpp',
+ '/COMMIT_MSG',
+ '/asdasd',
+ '/mrPeanutbutter.py',
+ ];
+ assert.deepEqual(testFiles.sort(specialFilePathCompare), [
+ '/COMMIT_MSG',
+ '/MERGE_LIST',
+ '/a.h',
+ '/a.cpp',
+ '/asdasd',
+ '/mrPeanutbutter.py',
+ ]);
+ });
+
+ test('special file path sorting', () => {
+ assert.deepEqual(
+ ['.b', '/COMMIT_MSG', '.a', 'file'].sort(specialFilePathCompare),
+ ['/COMMIT_MSG', '.a', '.b', 'file']
+ );
+
+ assert.deepEqual(
+ ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.h'].sort(
+ specialFilePathCompare
+ ),
+ ['/COMMIT_MSG', '.b', 'foo/bar/baz.h', 'foo/bar/baz.cc']
+ );
+
+ assert.deepEqual(
+ ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.hpp'].sort(
+ specialFilePathCompare
+ ),
+ ['/COMMIT_MSG', '.b', 'foo/bar/baz.hpp', 'foo/bar/baz.cc']
+ );
+
+ assert.deepEqual(
+ ['.b', '/COMMIT_MSG', 'foo/bar/baz.cc', 'foo/bar/baz.hxx'].sort(
+ specialFilePathCompare
+ ),
+ ['/COMMIT_MSG', '.b', 'foo/bar/baz.hxx', 'foo/bar/baz.cc']
+ );
+
+ assert.deepEqual(
+ ['foo/bar.h', 'foo/bar.hxx', 'foo/bar.hpp'].sort(specialFilePathCompare),
+ ['foo/bar.h', 'foo/bar.hpp', 'foo/bar.hxx']
+ );
+
+ // Regression test for Issue 4448.
+ assert.deepEqual(
+ [
+ 'minidump/minidump_memory_writer.cc',
+ 'minidump/minidump_memory_writer.h',
+ 'minidump/minidump_thread_writer.cc',
+ 'minidump/minidump_thread_writer.h',
+ ].sort(specialFilePathCompare),
+ [
+ 'minidump/minidump_memory_writer.h',
+ 'minidump/minidump_memory_writer.cc',
+ 'minidump/minidump_thread_writer.h',
+ 'minidump/minidump_thread_writer.cc',
+ ]
+ );
+
+ // Regression test for Issue 4545.
+ assert.deepEqual(['task_test.go', 'task.go'].sort(specialFilePathCompare), [
+ 'task.go',
+ 'task_test.go',
+ ]);
+ });
+
+ test('file display name', () => {
+ assert.equal(computeDisplayPath('/foo/bar/baz'), '/foo/bar/baz');
+ assert.equal(computeDisplayPath('/foobarbaz'), '/foobarbaz');
+ assert.equal(computeDisplayPath('/COMMIT_MSG'), 'Commit message');
+ assert.equal(computeDisplayPath('/MERGE_LIST'), 'Merge list');
+ });
+
+ test('isMagicPath', () => {
+ assert.isFalse(isMagicPath(undefined));
+ assert.isFalse(isMagicPath('/foo.cc'));
+ assert.isTrue(isMagicPath('/COMMIT_MSG'));
+ assert.isTrue(isMagicPath('/MERGE_LIST'));
+ });
+
+ test('patchset level comments are hidden', () => {
+ const commentedPaths = {
+ [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: true,
+ 'file1.txt': true,
+ };
+
+ const files: {[filename: string]: FileInfo} = {
+ 'file2.txt': {
+ status: FileInfoStatus.REWRITTEN,
+ size_delta: 10,
+ size: 10,
+ },
+ };
+ addUnmodifiedFiles(files, commentedPaths);
+ assert.equal(files['file1.txt'].status, FileInfoStatus.UNMODIFIED);
+ assert.equal(files['file2.txt'].status, FileInfoStatus.REWRITTEN);
+ assert.isFalse(
+ hasOwnProperty(files, SpecialFilePath.PATCHSET_LEVEL_COMMENTS)
+ );
+ });
+
+ test('truncatePath with long path should add ellipsis', () => {
+ let path = 'level1/level2/level3/level4/file.js';
+ let shortenedPath = truncatePath(path);
+ // The expected path is truncated with an ellipsis.
+ const expectedPath = '\u2026/file.js';
+ assert.equal(shortenedPath, expectedPath);
+
+ path = 'level2/file.js';
+ shortenedPath = truncatePath(path);
+ assert.equal(shortenedPath, expectedPath);
+ });
+
+ test('truncatePath with opt_threshold', () => {
+ let path = 'level1/level2/level3/level4/file.js';
+ let shortenedPath = truncatePath(path, 2);
+ // The expected path is truncated with an ellipsis.
+ const expectedPath = '\u2026/level4/file.js';
+ assert.equal(shortenedPath, expectedPath);
+
+ path = 'level2/file.js';
+ shortenedPath = truncatePath(path, 2);
+ assert.equal(shortenedPath, path);
+ });
+
+ test('truncatePath with short path should not add ellipsis', () => {
+ const path = 'file.js';
+ const expectedPath = 'file.js';
+ const shortenedPath = truncatePath(path);
+ assert.equal(shortenedPath, expectedPath);
+ });
+});
diff --git a/polygerrit-ui/app/utils/url-util_test.js b/polygerrit-ui/app/utils/url-util_test.ts
similarity index 66%
rename from polygerrit-ui/app/utils/url-util_test.js
rename to polygerrit-ui/app/utils/url-util_test.ts
index 5cd4bb4..63dc81d 100644
--- a/polygerrit-ui/app/utils/url-util_test.js
+++ b/polygerrit-ui/app/utils/url-util_test.ts
@@ -15,7 +15,9 @@
* limitations under the License.
*/
-import '../test/common-test-setup-karma.js';
+import {ServerInfo} from '../api/rest-api';
+import '../test/common-test-setup-karma';
+import {createGerritInfo, createServerInfo} from '../test/test-data-generators';
import {
getBaseUrl,
getDocsBaseUrl,
@@ -25,11 +27,13 @@
toPath,
toPathname,
toSearchParams,
-} from './url-util.js';
+} from './url-util';
+import {appContext} from '../services/app-context';
+import {stubRestApi} from '../test/test-utils';
suite('url-util tests', () => {
suite('getBaseUrl tests', () => {
- let originalCanonicalPath;
+ let originalCanonicalPath: string | undefined;
suiteSetup(() => {
originalCanonicalPath = window.CANONICAL_PATH;
@@ -51,43 +55,50 @@
});
test('null config', async () => {
- const mockRestApi = {
- probePath: sinon.stub().returns(Promise.resolve(true)),
- };
- const docsBaseUrl = await getDocsBaseUrl(null, mockRestApi);
- assert.isTrue(
- mockRestApi.probePath.calledWith('/Documentation/index.html'));
+ const probePathMock = stubRestApi('probePath').resolves(true);
+ const docsBaseUrl = await getDocsBaseUrl(
+ undefined,
+ appContext.restApiService
+ );
+ assert.isTrue(probePathMock.calledWith('/Documentation/index.html'));
assert.equal(docsBaseUrl, '/Documentation');
});
test('no doc config', async () => {
- const mockRestApi = {
- probePath: sinon.stub().returns(Promise.resolve(true)),
+ const probePathMock = stubRestApi('probePath').resolves(true);
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ gerrit: createGerritInfo(),
};
- const config = {gerrit: {}};
- const docsBaseUrl = await getDocsBaseUrl(config, mockRestApi);
- assert.isTrue(
- mockRestApi.probePath.calledWith('/Documentation/index.html'));
+ const docsBaseUrl = await getDocsBaseUrl(
+ config,
+ appContext.restApiService
+ );
+ assert.isTrue(probePathMock.calledWith('/Documentation/index.html'));
assert.equal(docsBaseUrl, '/Documentation');
});
test('has doc config', async () => {
- const mockRestApi = {
- probePath: sinon.stub().returns(Promise.resolve(true)),
+ const probePathMock = stubRestApi('probePath').resolves(true);
+ const config: ServerInfo = {
+ ...createServerInfo(),
+ gerrit: {...createGerritInfo(), doc_url: 'foobar'},
};
- const config = {gerrit: {doc_url: 'foobar'}};
- const docsBaseUrl = await getDocsBaseUrl(config, mockRestApi);
- assert.isFalse(mockRestApi.probePath.called);
+ const docsBaseUrl = await getDocsBaseUrl(
+ config,
+ appContext.restApiService
+ );
+ assert.isFalse(probePathMock.called);
assert.equal(docsBaseUrl, 'foobar');
});
test('no probe', async () => {
- const mockRestApi = {
- probePath: sinon.stub().returns(Promise.resolve(false)),
- };
- const docsBaseUrl = await getDocsBaseUrl(null, mockRestApi);
- assert.isTrue(
- mockRestApi.probePath.calledWith('/Documentation/index.html'));
+ const probePathMock = stubRestApi('probePath').resolves(false);
+ const docsBaseUrl = await getDocsBaseUrl(
+ undefined,
+ appContext.restApiService
+ );
+ assert.isTrue(probePathMock.calledWith('/Documentation/index.html'));
assert.isNotOk(docsBaseUrl);
});
});
@@ -144,7 +155,9 @@
assert.equal(toPath('asdf', params), 'asdf');
params.set('qwer', 'zxcv');
assert.equal(toPath('asdf', params), 'asdf?qwer=zxcv');
- assert.equal(toPath(toPathname('asdf?qwer=zxcv'),
- toSearchParams('asdf?qwer=zxcv')), 'asdf?qwer=zxcv');
+ assert.equal(
+ toPath(toPathname('asdf?qwer=zxcv'), toSearchParams('asdf?qwer=zxcv')),
+ 'asdf?qwer=zxcv'
+ );
});
});
