Merge "Fix FilesInCommitCollection when parent = 0"
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
index 477641b..01857da 100644
--- a/Documentation/dev-contributing.txt
+++ b/Documentation/dev-contributing.txt
@@ -113,6 +113,11 @@
nags and pester you if you haven't replied or made a fix, so it helps
them know if you missed it or decided against it.
+Features or API extensions, even if they are small, will incur
+long-time maintenance and support burden, so they should be left
+pending for at least 24 hours to give maintainers in all timezones a
+chance to evaluate.
+
[[design-driven-contribution-process]]
=== Design-driven Contribution Process
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index f59daf5..5b27088 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -812,8 +812,7 @@
.boxed()
.collect(
Collectors.toMap(
- i -> fileNames.get((int) i - 1),
- i -> fileNames.get((int) i - 1) + "-" + i)));
+ i -> fileNames.get(i - 1), i -> fileNames.get(i - 1) + "-" + i)));
m.setParents(pushResults.stream().map(PushOneCommit.Result::getCommit).collect(toList()));
PushOneCommit.Result result = m.to(ref);
diff --git a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
index 29dc6a3..91fbf9e 100644
--- a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
@@ -288,7 +288,6 @@
try (AutoCloseable ignored =
installPlugin("my-plugin", PluginDefinedBulkExceptionModule.class)) {
- PluginDefinedInfo errorInfo = new PluginDefinedInfo();
List<PluginDefinedInfo> outputInfos = getter.call(id).get(id);
assertThat(outputInfos).hasSize(1);
assertThat(outputInfos.get(0).name).isEqualTo("my-plugin");
diff --git a/java/com/google/gerrit/server/CurrentUser.java b/java/com/google/gerrit/server/CurrentUser.java
index afbc74e..7012944 100644
--- a/java/com/google/gerrit/server/CurrentUser.java
+++ b/java/com/google/gerrit/server/CurrentUser.java
@@ -138,8 +138,9 @@
}
/**
- * Returns all {@link ExternalId.Key}s associated with this user. For {@link AnonymousUser} and
- * other users that don't represent a person user or service account, this set will be empty.
+ * Returns all {@link com.google.gerrit.server.account.externalids.ExternalId.Key}s associated
+ * with this user. For {@link AnonymousUser} and other users that don't represent a person user or
+ * service account, this set will be empty.
*/
public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
return ImmutableSet.of();
diff --git a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 4f85412..1eee10f 100644
--- a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -365,8 +365,7 @@
@Override
public void starChange(String changeId) throws RestApiException {
try {
- starredChangesCreate.apply(
- account, IdString.fromUrl(changeId), new StarredChanges.EmptyInput());
+ starredChangesCreate.apply(account, IdString.fromUrl(changeId), new Input());
} catch (Exception e) {
throw asRestApiException("Cannot star change", e);
}
@@ -378,7 +377,7 @@
ChangeResource rsrc = changes.parse(TopLevelResource.INSTANCE, IdString.fromUrl(changeId));
AccountResource.StarredChange starredChange =
new AccountResource.StarredChange(account.getUser(), rsrc);
- starredChangesDelete.apply(starredChange, new StarredChanges.EmptyInput());
+ starredChangesDelete.apply(starredChange, new Input());
} catch (Exception e) {
throw asRestApiException("Cannot unstar change", e);
}
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index d349dda..8893039 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -767,10 +767,7 @@
private final CmdLineParser.Factory cmdLineParserFactory;
@Inject
- DynamicOptionParser(
- CmdLineParser.Factory cmdLineParserFactory,
- Injector injector,
- DynamicMap<DynamicOptions.DynamicBean> dynamicBeans) {
+ DynamicOptionParser(CmdLineParser.Factory cmdLineParserFactory) {
this.cmdLineParserFactory = cmdLineParserFactory;
}
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheBaseFactory.java b/java/com/google/gerrit/server/cache/PersistentCacheBaseFactory.java
new file mode 100644
index 0000000..9553acc
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/PersistentCacheBaseFactory.java
@@ -0,0 +1,105 @@
+// 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.cache;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.lib.Config;
+
+/**
+ * Base class for persistent cache factory. If the cache.directory property is unset, or disk limit
+ * is zero or negative, it will fall back to in-memory only caches.
+ */
+public abstract class PersistentCacheBaseFactory implements PersistentCacheFactory {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ protected final MemoryCacheFactory memCacheFactory;
+ protected final Path cacheDir;
+ protected boolean diskEnabled;
+ protected final Config config;
+
+ public PersistentCacheBaseFactory(
+ MemoryCacheFactory memCacheFactory, @GerritServerConfig Config config, SitePaths site) {
+ this.cacheDir = getCacheDir(site, config.getString("cache", null, "directory"));
+ this.diskEnabled = cacheDir != null;
+ this.memCacheFactory = memCacheFactory;
+ this.config = config;
+ }
+
+ protected abstract <K, V> Cache<K, V> buildImpl(
+ PersistentCacheDef<K, V> in, long diskLimit, CacheBackend backend);
+
+ protected abstract <K, V> LoadingCache<K, V> buildImpl(
+ PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, long diskLimit, CacheBackend backend);
+
+ @Override
+ public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in, CacheBackend backend) {
+ long limit = getDiskLimit(in);
+
+ if (isInMemoryCache(limit)) {
+ return memCacheFactory.build(in, backend);
+ }
+
+ return buildImpl(in, limit, backend);
+ }
+
+ @Override
+ public <K, V> LoadingCache<K, V> build(
+ PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, CacheBackend backend) {
+ long limit = getDiskLimit(in);
+
+ if (isInMemoryCache(limit)) {
+ return memCacheFactory.build(in, loader, backend);
+ }
+
+ return buildImpl(in, loader, limit, backend);
+ }
+
+ private <K, V> long getDiskLimit(PersistentCacheDef<K, V> in) {
+ return config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
+ }
+
+ private <K, V> boolean isInMemoryCache(long diskLimit) {
+ return !diskEnabled || diskLimit <= 0;
+ }
+
+ private static Path getCacheDir(SitePaths site, String name) {
+ if (name == null) {
+ return null;
+ }
+ Path loc = site.resolve(name);
+ if (!Files.exists(loc)) {
+ try {
+ Files.createDirectories(loc);
+ } catch (IOException e) {
+ logger.atWarning().log("Can't create disk cache: %s", loc.toAbsolutePath());
+ return null;
+ }
+ }
+ if (!Files.isWritable(loc)) {
+ logger.atWarning().log("Can't write to disk cache: %s", loc.toAbsolutePath());
+ return null;
+ }
+ logger.atInfo().log("Enabling disk cache %s", loc.toAbsolutePath());
+ return loc;
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index 82615a4..16d62b3 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -23,8 +23,8 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.MemoryCacheFactory;
+import com.google.gerrit.server.cache.PersistentCacheBaseFactory;
import com.google.gerrit.server.cache.PersistentCacheDef;
-import com.google.gerrit.server.cache.PersistentCacheFactory;
import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -34,9 +34,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -52,12 +49,9 @@
* is unset, it will fall back to in-memory caches.
*/
@Singleton
-class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
+class H2CacheFactory extends PersistentCacheBaseFactory implements LifecycleListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final MemoryCacheFactory memCacheFactory;
- private final Config config;
- private final Path cacheDir;
private final List<H2CacheImpl<?, ?>> caches;
private final DynamicMap<Cache<?, ?>> cacheMap;
private final ExecutorService executor;
@@ -71,15 +65,13 @@
@GerritServerConfig Config cfg,
SitePaths site,
DynamicMap<Cache<?, ?>> cacheMap) {
- this.memCacheFactory = memCacheFactory;
- config = cfg;
- cacheDir = getCacheDir(site, cfg.getString("cache", null, "directory"));
+ super(memCacheFactory, cfg, site);
h2CacheSize = cfg.getLong("cache", null, "h2CacheSize", -1);
h2AutoServer = cfg.getBoolean("cache", null, "h2AutoServer", false);
caches = new LinkedList<>();
this.cacheMap = cacheMap;
- if (cacheDir != null) {
+ if (diskEnabled) {
executor =
new LoggingContextAwareExecutorService(
Executors.newFixedThreadPool(
@@ -98,27 +90,6 @@
}
}
- private static Path getCacheDir(SitePaths site, String name) {
- if (name == null) {
- return null;
- }
- Path loc = site.resolve(name);
- if (!Files.exists(loc)) {
- try {
- Files.createDirectories(loc);
- } catch (IOException e) {
- logger.atWarning().log("Can't create disk cache: %s", loc.toAbsolutePath());
- return null;
- }
- }
- if (!Files.isWritable(loc)) {
- logger.atWarning().log("Can't write to disk cache: %s", loc.toAbsolutePath());
- return null;
- }
- logger.atInfo().log("Enabling disk cache %s", loc.toAbsolutePath());
- return loc;
- }
-
@Override
public void start() {
if (executor != null) {
@@ -161,13 +132,8 @@
@SuppressWarnings({"unchecked"})
@Override
- public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in, CacheBackend backend) {
- long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
-
- if (cacheDir == null || limit <= 0) {
- return memCacheFactory.build(in, backend);
- }
-
+ public <K, V> Cache<K, V> buildImpl(
+ PersistentCacheDef<K, V> in, long limit, CacheBackend backend) {
H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
SqlStore<K, V> store = newSqlStore(def, limit);
H2CacheImpl<K, V> cache =
@@ -184,14 +150,8 @@
@SuppressWarnings("unchecked")
@Override
- public <K, V> LoadingCache<K, V> build(
- PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, CacheBackend backend) {
- long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
-
- if (cacheDir == null || limit <= 0) {
- return memCacheFactory.build(in, loader, backend);
- }
-
+ public <K, V> LoadingCache<K, V> buildImpl(
+ PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, long limit, CacheBackend backend) {
H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
SqlStore<K, V> store = newSqlStore(def, limit);
Cache<K, ValueHolder<V>> mem =
diff --git a/java/com/google/gerrit/server/change/ChangeETagComputation.java b/java/com/google/gerrit/server/change/ChangeETagComputation.java
index a5b7d49..2fd5755 100644
--- a/java/com/google/gerrit/server/change/ChangeETagComputation.java
+++ b/java/com/google/gerrit/server/change/ChangeETagComputation.java
@@ -26,7 +26,7 @@
* <ul>
* <li>providing plugin defined attributes to {@link
* com.google.gerrit.extensions.common.ChangeInfo#plugins} (see {@link
- * ChangeAttributeFactory})
+ * ChangePluginDefinedInfoFactory})
* <li>implementing a {@link com.google.gerrit.server.rules.SubmitRule} which affects the
* computation of {@link com.google.gerrit.extensions.common.ChangeInfo#submittable}
* </ul>
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
index 16e9023..0dd71507 100644
--- a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
+++ b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
@@ -47,10 +47,9 @@
try {
if (base == null) {
return asFileInfo(
- diffs.getModifiedFilesAgainstParentOrAutoMerge(change.getProject(), objectId, null));
+ diffs.listModifiedFilesAgainstParent(change.getProject(), objectId, null));
} else {
- return asFileInfo(
- diffs.getModifiedFilesBetweenPatchsets(change.getProject(), base.commitId(), objectId));
+ return asFileInfo(diffs.listModifiedFiles(change.getProject(), base.commitId(), objectId));
}
} catch (DiffNotAvailableException e) {
convertException(e);
@@ -64,7 +63,7 @@
throws ResourceConflictException, PatchListNotAvailableException {
try {
Map<String, FileDiffOutput> modifiedFiles =
- diffs.getModifiedFilesAgainstParentOrAutoMerge(project, objectId, parent + 1);
+ diffs.listModifiedFilesAgainstParent(project, objectId, parent + 1);
return asFileInfo(modifiedFiles);
} catch (DiffNotAvailableException e) {
convertException(e);
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
index 76992e8..acff03c 100644
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -20,14 +20,12 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
-import com.google.common.collect.Sets;
import com.google.common.collect.Table;
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
@@ -38,7 +36,6 @@
import com.google.gerrit.entities.LabelValue;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.SubmitRecord;
-import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.VotingRangeInfo;
@@ -156,11 +153,6 @@
return permitted.asMap();
}
- private static boolean containsAnyOf(
- ImmutableSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
- return !Sets.intersection(toFind, set).isEmpty();
- }
-
private static boolean isOnlyZero(Collection<String> values) {
return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index b816264..e8f1fe1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -138,8 +138,9 @@
}
/**
- * Create change notes based on a {@link Change.Id}. This requires using the Change index and
- * should only be used when {@link Project.NameKey} and the numeric change ID are not available.
+ * Create change notes based on a {@link com.google.gerrit.entities.Change.Id}. This requires
+ * using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public ChangeNotes createCheckedUsingIndexLookup(Change.Id changeId) {
InternalChangeQuery query = queryProvider.get().noFields();
@@ -155,9 +156,9 @@
}
/**
- * Create change notes based on a list of {@link Change.Id}s. This requires using the Change
- * index and should only be used when {@link Project.NameKey} and the numeric change ID are not
- * available.
+ * Create change notes based on a list of {@link com.google.gerrit.entities.Change.Id}s. This
+ * requires using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public List<ChangeNotes> createUsingIndexLookup(Collection<Change.Id> changeIds) {
List<ChangeNotes> notes = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 846d4b8..97fec4c 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -1027,7 +1027,7 @@
* @return {@link Optional} value of the parsed footer or {@code null} if the footer is missing in
* this commit.
* @throws ConfigInvalidException if the footer value could not be parsed as a valid {@link
- * PatchSet.Id}.
+ * com.google.gerrit.entities.PatchSet.Id}.
*/
@Nullable
private Optional<PatchSet.Id> parseCherryPickOf(ChangeNotesCommit commit)
diff --git a/java/com/google/gerrit/server/patch/DiffNotAvailableException.java b/java/com/google/gerrit/server/patch/DiffNotAvailableException.java
index e75adec..ea92a99 100644
--- a/java/com/google/gerrit/server/patch/DiffNotAvailableException.java
+++ b/java/com/google/gerrit/server/patch/DiffNotAvailableException.java
@@ -22,6 +22,8 @@
* if the implementations failed to retrieve the modified files between the 2 commits.
*/
public class DiffNotAvailableException extends Exception {
+ private static final long serialVersionUID = 1L;
+
public DiffNotAvailableException(Throwable cause) {
super(cause);
}
@@ -29,4 +31,8 @@
public DiffNotAvailableException(String message) {
super(message);
}
+
+ public DiffNotAvailableException(String message, Throwable cause) {
+ super(message, cause);
+ }
}
diff --git a/java/com/google/gerrit/server/patch/DiffOperations.java b/java/com/google/gerrit/server/patch/DiffOperations.java
index a6c7c81..1d8438e 100644
--- a/java/com/google/gerrit/server/patch/DiffOperations.java
+++ b/java/com/google/gerrit/server/patch/DiffOperations.java
@@ -27,7 +27,7 @@
* <ul>
* <li>The list of modified files between two commits.
* <li>The list of modified files between a commit and its parent or the auto-merge.
- * <li>The detailed file diff for a single file path (TODO:ghareeb).
+ * <li>The detailed file diff for a single file path.
* <li>The Intra-line diffs for a single file path (TODO:ghareeb).
* </ul>
*/
@@ -51,7 +51,7 @@
* parents, if the {@code newCommit} could not be parsed for extracting the base commit, or if
* an internal error occurred in Git while evaluating the diff.
*/
- Map<String, FileDiffOutput> getModifiedFilesAgainstParentOrAutoMerge(
+ Map<String, FileDiffOutput> listModifiedFilesAgainstParent(
Project.NameKey project, ObjectId newCommit, @Nullable Integer parentNum)
throws DiffNotAvailableException;
@@ -66,7 +66,41 @@
* @throws DiffNotAvailableException if an internal error occurred in Git while evaluating the
* diff.
*/
- Map<String, FileDiffOutput> getModifiedFilesBetweenPatchsets(
+ Map<String, FileDiffOutput> listModifiedFiles(
Project.NameKey project, ObjectId oldCommit, ObjectId newCommit)
throws DiffNotAvailableException;
+
+ /**
+ * Returns the diff for a single file between a patchset commit against its parent or the
+ * auto-merge commit. For deleted files, the {@code fileName} parameter should contain the old
+ * name of the file.
+ *
+ * @param project a project name representing a git repository.
+ * @param newCommit 20 bytes SHA-1 of the new commit used in the diff.
+ * @param parentNum integer specifying which parent to use as base. If null, the only parent will
+ * be used or the auto-merge if {@code newCommit} is a merge commit.
+ * @param fileName the file name for which the diff should be evaluated.
+ * @return the diff for the single file between the two commits.
+ * @throws DiffNotAvailableException if an internal error occurred in Git while evaluating the
+ * diff, or if an exception happened while parsing the base commit.
+ */
+ FileDiffOutput getModifiedFileAgainstParent(
+ Project.NameKey project, ObjectId newCommit, @Nullable Integer parentNum, String fileName)
+ throws DiffNotAvailableException;
+
+ /**
+ * Returns the diff for a single file between two patchset commits. For deleted files, the {@code
+ * fileName} parameter should contain the old name of the file.
+ *
+ * @param project a project name representing a git repository.
+ * @param oldCommit 20 bytes SHA-1 of the old commit used in the diff.
+ * @param newCommit 20 bytes SHA-1 of the new commit used in the diff.
+ * @param fileName the file name for which the diff should be evaluated.
+ * @return the diff for the single file between the two commits.
+ * @throws DiffNotAvailableException if an internal error occurred in Git while evaluating the
+ * diff.
+ */
+ FileDiffOutput getModifiedFile(
+ Project.NameKey project, ObjectId oldCommit, ObjectId newCommit, String fileName)
+ throws DiffNotAvailableException;
}
diff --git a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
index 719536a..2d98d42 100644
--- a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
+++ b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
@@ -14,6 +14,10 @@
package com.google.gerrit.server.patch;
+import static com.google.gerrit.entities.Patch.COMMIT_MSG;
+import static com.google.gerrit.entities.Patch.MERGE_LIST;
+
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
@@ -37,11 +41,10 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.stream.Collectors;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.revwalk.RevObject;
/**
* Provides different file diff operations. Uses the underlying Git/Gerrit caches to speed up the
@@ -81,73 +84,84 @@
}
@Override
- public Map<String, FileDiffOutput> getModifiedFilesAgainstParentOrAutoMerge(
+ public Map<String, FileDiffOutput> listModifiedFilesAgainstParent(
Project.NameKey project, ObjectId newCommit, @Nullable Integer parent)
throws DiffNotAvailableException {
try {
- if (parent != null) {
- RevObject base = baseCommitUtil.getBaseCommit(project, newCommit, parent);
- return getModifiedFiles(project, base, newCommit, ComparisonType.againstParent(parent));
- }
- int numParents = baseCommitUtil.getNumParents(project, newCommit);
- if (numParents == 1) {
- RevObject base = baseCommitUtil.getBaseCommit(project, newCommit, parent);
- ComparisonType cmp = ComparisonType.againstParent(1);
- return getModifiedFiles(project, base, newCommit, cmp);
- }
- if (numParents > 2) {
- logger.atFine().log(
- "Diff against auto-merge for merge commits "
- + "with more than two parents is not supported. Commit "
- + newCommit
- + " has "
- + numParents
- + " parents. Falling back to the diff against the first parent.");
- ObjectId firstParentId = baseCommitUtil.getBaseCommit(project, newCommit, 1).getId();
- ImmutableList.Builder<FileDiffCacheKey> keys = ImmutableList.builder();
- keys.add(createFileDiffCacheKey(project, firstParentId, newCommit, Patch.COMMIT_MSG));
- keys.add(createFileDiffCacheKey(project, firstParentId, newCommit, Patch.MERGE_LIST));
- return getModifiedFilesForKeys(keys.build());
- }
- RevObject autoMerge = baseCommitUtil.getBaseCommit(project, newCommit, null);
- return getModifiedFiles(project, autoMerge, newCommit, ComparisonType.againstAutoMerge());
+ DiffParameters diffParams = computeDiffParameters(project, newCommit, parent);
+ return getModifiedFiles(project, newCommit, diffParams);
} catch (IOException e) {
- throw new DiffNotAvailableException(e);
+ throw new DiffNotAvailableException(
+ "Failed to evaluate the parent/base commit for commit " + newCommit, e);
}
}
@Override
- public Map<String, FileDiffOutput> getModifiedFilesBetweenPatchsets(
+ public Map<String, FileDiffOutput> listModifiedFiles(
Project.NameKey project, ObjectId oldCommit, ObjectId newCommit)
throws DiffNotAvailableException {
- ComparisonType cmp = ComparisonType.againstOtherPatchSet();
- return getModifiedFiles(project, oldCommit, newCommit, cmp);
+ DiffParameters params =
+ DiffParameters.builder()
+ .project(project)
+ .newCommit(newCommit)
+ .baseCommit(oldCommit)
+ .comparisonType(ComparisonType.againstOtherPatchSet())
+ .build();
+ return getModifiedFiles(project, newCommit, params);
+ }
+
+ @Override
+ public FileDiffOutput getModifiedFileAgainstParent(
+ Project.NameKey project, ObjectId newCommit, @Nullable Integer parent, String fileName)
+ throws DiffNotAvailableException {
+ try {
+ DiffParameters diffParams = computeDiffParameters(project, newCommit, parent);
+ FileDiffCacheKey key =
+ createFileDiffCacheKey(project, diffParams.baseCommit(), newCommit, fileName);
+ return getModifiedFilesForKeys(ImmutableList.of(key)).get(fileName);
+ } catch (IOException e) {
+ throw new DiffNotAvailableException(
+ "Failed to evaluate the parent/base commit for commit " + newCommit, e);
+ }
+ }
+
+ @Override
+ public FileDiffOutput getModifiedFile(
+ Project.NameKey project, ObjectId oldCommit, ObjectId newCommit, String fileName)
+ throws DiffNotAvailableException {
+ FileDiffCacheKey key = createFileDiffCacheKey(project, oldCommit, newCommit, fileName);
+ return getModifiedFilesForKeys(ImmutableList.of(key)).get(fileName);
}
private Map<String, FileDiffOutput> getModifiedFiles(
- Project.NameKey project, ObjectId oldCommit, ObjectId newCommit, ComparisonType cmp)
+ Project.NameKey project, ObjectId newCommit, DiffParameters diffParams)
throws DiffNotAvailableException {
try {
+ ObjectId oldCommit = diffParams.baseCommit();
+ ComparisonType cmp = diffParams.comparisonType();
+
ImmutableList<ModifiedFile> modifiedFiles =
modifiedFilesCache.get(createModifiedFilesKey(project, oldCommit, newCommit));
- List<FileDiffCacheKey> fileCacheKeys =
- modifiedFiles.stream()
- .map(
- entity ->
- createFileDiffCacheKey(
- project,
- oldCommit,
- newCommit,
- entity.newPath().isPresent()
- ? entity.newPath().get()
- : entity.oldPath().get()))
- .collect(Collectors.toList());
-
- fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, Patch.COMMIT_MSG));
+ List<FileDiffCacheKey> fileCacheKeys = new ArrayList<>();
+ fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, COMMIT_MSG));
if (cmp.isAgainstAutoMerge() || isMergeAgainstParent(cmp, project, newCommit)) {
- fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, Patch.MERGE_LIST));
+ fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, MERGE_LIST));
+ }
+
+ if (diffParams.skipFiles() == null) {
+ modifiedFiles.stream()
+ .map(
+ entity ->
+ createFileDiffCacheKey(
+ project,
+ oldCommit,
+ newCommit,
+ entity.newPath().isPresent()
+ ? entity.newPath().get()
+ : entity.oldPath().get()))
+ .forEach(fileCacheKeys::add);
}
return getModifiedFilesForKeys(fileCacheKeys);
} catch (IOException e) {
@@ -206,4 +220,78 @@
.whitespace(Whitespace.IGNORE_NONE)
.build();
}
+
+ @AutoValue
+ abstract static class DiffParameters {
+ abstract Project.NameKey project();
+
+ abstract ObjectId newCommit();
+
+ abstract ObjectId baseCommit();
+
+ abstract ComparisonType comparisonType();
+
+ @Nullable
+ abstract Integer parent();
+
+ /** Compute the diff for {@value Patch#COMMIT_MSG} and {@link Patch#MERGE_LIST} only. */
+ @Nullable
+ abstract Boolean skipFiles();
+
+ static Builder builder() {
+ return new AutoValue_DiffOperationsImpl_DiffParameters.Builder();
+ }
+
+ @AutoValue.Builder
+ abstract static class Builder {
+
+ abstract Builder project(Project.NameKey project);
+
+ abstract Builder newCommit(ObjectId newCommit);
+
+ abstract Builder baseCommit(ObjectId baseCommit);
+
+ abstract Builder parent(@Nullable Integer parent);
+
+ abstract Builder skipFiles(@Nullable Boolean skipFiles);
+
+ abstract Builder comparisonType(ComparisonType comparisonType);
+
+ public abstract DiffParameters build();
+ }
+ }
+
+ /** Compute Diff parameters - the base commit and the comparison type - using the input args. */
+ private DiffParameters computeDiffParameters(
+ Project.NameKey project, ObjectId newCommit, Integer parent) throws IOException {
+ DiffParameters.Builder result =
+ DiffParameters.builder().project(project).newCommit(newCommit).parent(parent);
+ if (parent != null) {
+ result.baseCommit(baseCommitUtil.getBaseCommit(project, newCommit, parent));
+ result.comparisonType(ComparisonType.againstParent(parent));
+ return result.build();
+ }
+ int numParents = baseCommitUtil.getNumParents(project, newCommit);
+ if (numParents == 1) {
+ result.baseCommit(baseCommitUtil.getBaseCommit(project, newCommit, parent));
+ result.comparisonType(ComparisonType.againstParent(1));
+ return result.build();
+ }
+ if (numParents > 2) {
+ logger.atFine().log(
+ "Diff against auto-merge for merge commits "
+ + "with more than two parents is not supported. Commit "
+ + newCommit
+ + " has "
+ + numParents
+ + " parents. Falling back to the diff against the first parent.");
+ result.baseCommit(baseCommitUtil.getBaseCommit(project, newCommit, 1).getId());
+ result.comparisonType(ComparisonType.againstParent(1));
+ result.skipFiles(true);
+ } else {
+ result.baseCommit(baseCommitUtil.getBaseCommit(project, newCommit, null));
+ result.comparisonType(ComparisonType.againstAutoMerge());
+ }
+ return result.build();
+ }
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index 091b02c..b2a628b 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.patch.filediff;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
@@ -418,7 +419,7 @@
private static ImmutableList<TaggedEdit> asTaggedEdits(
List<Edit> normalEdits, List<Edit> rebaseEdits) {
- Set<Edit> rebaseEditsSet = new HashSet(rebaseEdits);
+ Set<Edit> rebaseEditsSet = new HashSet<>(rebaseEdits);
ImmutableList.Builder<TaggedEdit> result =
ImmutableList.builderWithExpectedSize(normalEdits.size());
for (Edit e : normalEdits) {
@@ -457,9 +458,7 @@
new EditTransformer(
ImmutableList.of(
FileEdits.create(
- parentVsParentDiff.edits().stream()
- .map(Edit::toJGitEdit)
- .collect(Collectors.toList()),
+ parentVsParentDiff.edits().stream().collect(toImmutableList()),
parentVsParentDiff.oldPath(),
parentVsParentDiff.newPath())));
@@ -468,9 +467,7 @@
editTransformer.transformReferencesOfSideA(
ImmutableList.of(
FileEdits.create(
- oldVsParDiff.edits().stream()
- .map(Edit::toJGitEdit)
- .collect(Collectors.toList()),
+ oldVsParDiff.edits().stream().collect(toImmutableList()),
oldVsParDiff.oldPath(),
oldVsParDiff.newPath())));
}
@@ -480,9 +477,7 @@
editTransformer.transformReferencesOfSideB(
ImmutableList.of(
FileEdits.create(
- newVsParDiff.edits().stream()
- .map(Edit::toJGitEdit)
- .collect(Collectors.toList()),
+ newVsParDiff.edits().stream().collect(toImmutableList()),
newVsParDiff.oldPath(),
newVsParDiff.newPath())));
}
@@ -497,11 +492,12 @@
String filePath = editsPerFilePath.keys().iterator().next();
Collection<ContextAwareEdit> edits = editsPerFilePath.get(filePath);
return FileEdits.create(
- Streams.stream(edits)
+ edits.stream()
.map(ContextAwareEdit::toEdit)
.filter(Optional::isPresent)
.map(Optional::get)
- .collect(Collectors.toList()),
+ .map(Edit::fromJGitEdit)
+ .collect(toImmutableList()),
edits.iterator().next().getOldFilePath(),
edits.iterator().next().getNewFilePath());
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
index e89f148..46ce28b 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
@@ -32,6 +32,7 @@
/** File diff for a single file path. Produced as output of the {@link FileDiffCache}. */
@AutoValue
public abstract class FileDiffOutput implements Serializable {
+ private static final long serialVersionUID = 1L;
/**
* The file path at the old commit. Returns an empty Optional if {@link #changeType()} is equal to
diff --git a/java/com/google/gerrit/server/patch/filediff/FileEdits.java b/java/com/google/gerrit/server/patch/filediff/FileEdits.java
index 376bbc2..a009a02 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileEdits.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileEdits.java
@@ -18,7 +18,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
-import java.util.List;
import java.util.Optional;
/**
@@ -28,12 +27,16 @@
@AutoValue
public abstract class FileEdits {
public static FileEdits create(
- List<org.eclipse.jgit.diff.Edit> jgitEdits,
+ ImmutableList<Edit> edits, Optional<String> oldPath, Optional<String> newPath) {
+ return new AutoValue_FileEdits(edits, oldPath, newPath);
+ }
+
+ public static FileEdits createFromJgitEdits(
+ ImmutableList<org.eclipse.jgit.diff.Edit> jgitEdits,
Optional<String> oldPath,
Optional<String> newPath) {
- ImmutableList<Edit> edits =
- jgitEdits.stream().map(Edit::fromJGitEdit).collect(toImmutableList());
- return new AutoValue_FileEdits(edits, oldPath, newPath);
+ return new AutoValue_FileEdits(
+ jgitEdits.stream().map(Edit::fromJGitEdit).collect(toImmutableList()), oldPath, newPath);
}
public abstract ImmutableList<Edit> edits();
diff --git a/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
index 5b7eddb..d1c0b45 100644
--- a/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
+++ b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
@@ -348,7 +348,7 @@
newName = Optional.of(patchListEntry.getNewName());
break;
}
- return FileEdits.create(patchListEntry.getEdits(), oldName, newName);
+ return FileEdits.createFromJgitEdits(patchListEntry.getEdits(), oldName, newName);
}
private static boolean isRootOrMergeCommit(RevCommit commit) {
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index bc859f3..6272cda 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -318,7 +318,7 @@
&& !r.getName().startsWith(RefNames.REFS_TAGS)
&& !r.isSymbolic()
&& !r.getName().equals(RefNames.REFS_CONFIG))
- .collect(toCollection(() -> new ArrayList<>(allRefs.size())));
+ .collect(Collectors.toList());
} catch (IOException e) {
throw new PermissionBackendException(e);
}
diff --git a/java/com/google/gerrit/server/restapi/account/StarredChanges.java b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
index c108dcb..e67fe9e 100644
--- a/java/com/google/gerrit/server/restapi/account/StarredChanges.java
+++ b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
@@ -17,6 +17,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.DuplicateKeyException;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -96,8 +97,7 @@
@Singleton
public static class Create
- implements RestCollectionCreateView<
- AccountResource, AccountResource.StarredChange, EmptyInput> {
+ implements RestCollectionCreateView<AccountResource, AccountResource.StarredChange, Input> {
private final Provider<CurrentUser> self;
private final ChangesCollection changes;
private final StarredChangesUtil starredChangesUtil;
@@ -113,7 +113,7 @@
}
@Override
- public Response<?> apply(AccountResource rsrc, IdString id, EmptyInput in)
+ public Response<?> apply(AccountResource rsrc, IdString id, Input in)
throws RestApiException, IOException {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to add starred change");
@@ -148,7 +148,7 @@
}
@Singleton
- public static class Put implements RestModifyView<AccountResource.StarredChange, EmptyInput> {
+ public static class Put implements RestModifyView<AccountResource.StarredChange, Input> {
private final Provider<CurrentUser> self;
@Inject
@@ -157,8 +157,7 @@
}
@Override
- public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
- throws AuthException {
+ public Response<?> apply(AccountResource.StarredChange rsrc, Input in) throws AuthException {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed update starred changes");
}
@@ -167,7 +166,7 @@
}
@Singleton
- public static class Delete implements RestModifyView<AccountResource.StarredChange, EmptyInput> {
+ public static class Delete implements RestModifyView<AccountResource.StarredChange, Input> {
private final Provider<CurrentUser> self;
private final StarredChangesUtil starredChangesUtil;
@@ -178,7 +177,7 @@
}
@Override
- public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
+ public Response<?> apply(AccountResource.StarredChange rsrc, Input in)
throws AuthException, IOException, IllegalLabelException {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed remove starred change");
@@ -192,6 +191,4 @@
return Response.none();
}
}
-
- public static class EmptyInput {}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index a3e0cf0..ee6484c 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -28,7 +28,6 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -44,7 +43,6 @@
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.ResetCherryPickOp;
import com.google.gerrit.server.change.SetCherryPickOp;
-import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -114,7 +112,6 @@
private final ApprovalsUtil approvalsUtil;
private final NotifyResolver notifyResolver;
private final BatchUpdate.Factory batchUpdateFactory;
- private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
CherryPickChange(
@@ -131,8 +128,7 @@
ProjectCache projectCache,
ApprovalsUtil approvalsUtil,
NotifyResolver notifyResolver,
- BatchUpdate.Factory batchUpdateFactory,
- DynamicItem<UrlFormatter> urlFormatter) {
+ BatchUpdate.Factory batchUpdateFactory) {
this.seq = seq;
this.queryProvider = queryProvider;
this.gitManager = gitManager;
@@ -147,7 +143,6 @@
this.approvalsUtil = approvalsUtil;
this.notifyResolver = notifyResolver;
this.batchUpdateFactory = batchUpdateFactory;
- this.urlFormatter = urlFormatter;
}
/**
@@ -585,9 +580,8 @@
String commitMessage, @Nullable ObjectId changeIdForNewChange) {
if (changeIdForNewChange != null) {
return CommitMessageUtil.getChangeIdFromObjectId(changeIdForNewChange);
- } else {
- return CommitMessageUtil.getChangeIdFromCommitMessageFooter(commitMessage).orElse(null);
}
+ return CommitMessageUtil.getChangeIdFromCommitMessageFooter(commitMessage).orElse(null);
}
/**
diff --git a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
index 6a0fc97..fd18c8d 100644
--- a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
@@ -23,7 +23,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChildProjects;
import com.google.gerrit.server.project.ProjectResource;
@@ -40,16 +39,11 @@
@Option(name = "--limit", usage = "maximum number of parents projects to list")
private int limit;
- private final PermissionBackend permissionBackend;
private final ChildProjects childProjects;
private final Provider<QueryProjects> queryProvider;
@Inject
- ListChildProjects(
- PermissionBackend permissionBackend,
- ChildProjects childProjects,
- Provider<QueryProjects> queryProvider) {
- this.permissionBackend = permissionBackend;
+ ListChildProjects(ChildProjects childProjects, Provider<QueryProjects> queryProvider) {
this.childProjects = childProjects;
this.queryProvider = queryProvider;
}
@@ -82,7 +76,6 @@
}
private List<ProjectInfo> directChildProjects(Project.NameKey parent) throws RestApiException {
- PermissionBackend.WithUser currentUser = permissionBackend.currentUser();
return queryProvider.get().withQuery("parent:" + parent.get()).withLimit(limit).apply().stream()
.collect(toList());
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index b27cb9b..7f434ca 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -533,6 +533,9 @@
// Multiply the timeout by the number of projects we're actually attempting to
// submit.
.defaultTimeoutMultiplier(cs.projects().size())
+ // By default, we only retry lock failures. Here it's better to also retry unexpected
+ // runtime exceptions.
+ .retryOn(t -> t instanceof RuntimeException)
.call();
submissionExecutor.afterExecutions(orm);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 04e8706..0d92c60 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -767,7 +767,6 @@
pushFactory
.create(admin.newIdent(), testRepo, SUBJECT, FILE_NAME, "a")
.to("refs/for/master");
- String t1 = project.get() + "~master~" + r1.getChangeId();
BranchInput bin = new BranchInput();
bin.revision = r1.getCommit().getParent(0).name();
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 4139eeb..845c461 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -2753,6 +2753,14 @@
assertThat(r.getChange().attentionSet()).isEmpty();
}
+ @Test
+ public void pushForMasterWithUnknownOption() throws Exception {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ push.setPushOptions(ImmutableList.of("unknown=foo"));
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertErrorStatus("\"--unknown\" is not a valid option");
+ }
+
private DraftInput newDraft(String path, int line, String message) {
DraftInput d = new DraftInput();
d.path = path;
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 7f8add8..02916c7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -43,7 +43,6 @@
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.httpd.restapi.ParameterParser;
import com.google.gerrit.httpd.restapi.RestApiServlet;
-import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.validators.CommitValidationException;
@@ -713,7 +712,7 @@
@Test
@GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
- public void autoRetryWithTrace() throws Exception {
+ public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
String changeId = createChange().getChangeId();
approve(changeId);
@@ -723,49 +722,6 @@
RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).startsWith("retry-on-failure-");
- assertThat(traceSubmitRule.traceId).startsWith("retry-on-failure-");
- assertThat(traceSubmitRule.isLoggingForced).isTrue();
- }
- }
-
- @Test
- @GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
- public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
- String changeId = createChange().getChangeId();
- approve(changeId);
-
- TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
- traceSubmitRule.failAlways = true;
- try (Registration registration =
- extensionRegistry
- .newRegistration()
- .add(traceSubmitRule)
- .add(
- new ExceptionHook() {
- @Override
- public boolean shouldRetry(String actionType, String actionName, Throwable t) {
- return true;
- }
- })) {
- RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
- assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
- assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
- assertThat(traceSubmitRule.traceId).isNull();
- assertThat(traceSubmitRule.isLoggingForced).isFalse();
- }
- }
-
- @Test
- public void noAutoRetryWithTraceIfDisabled() throws Exception {
- String changeId = createChange().getChangeId();
- approve(changeId);
-
- TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
- traceSubmitRule.failOnce = true;
- try (Registration registration = extensionRegistry.newRegistration().add(traceSubmitRule)) {
- RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
- assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
- assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
assertThat(traceSubmitRule.traceId).isNull();
assertThat(traceSubmitRule.isLoggingForced).isFalse();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
index 3b26459..d3dd801 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
@@ -204,7 +204,7 @@
public void crossDomainPutTopic() throws Exception {
Result change = createChange();
BasicCookieStore cookies = new BasicCookieStore();
- Executor http = Executor.newInstance().cookieStore(cookies);
+ Executor http = Executor.newInstance().use(cookies);
Request req = Request.Get(canonicalWebUrl.get() + "/login/?account_id=" + admin.id().get());
http.execute(req);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index a322faf..fff3cb6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -26,7 +26,6 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -388,7 +387,6 @@
// Create a change
PushOneCommit change = pushFactory.create(user.newIdent(), testRepo, "fix", "a.txt", "foo");
PushOneCommit.Result changeResult = change.to("refs/for/master");
- PatchSet.Id patchSetId = changeResult.getPatchSetId();
// Create a successor change.
PushOneCommit change2 =
diff --git a/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java b/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java
index c0f2b36..4efa247 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java
@@ -23,7 +23,6 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Module;
-import java.io.IOException;
import java.util.List;
import org.junit.Test;
@@ -46,7 +45,7 @@
}
}
- protected List<String> getSamplesList(String sshOutput) throws IOException {
+ protected List<String> getSamplesList(String sshOutput) {
return GSON.fromJson(sshOutput, new TypeToken<List<String>>() {}.getType());
}
}
diff --git a/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
index 933ffb4..b185558 100644
--- a/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
@@ -25,7 +25,6 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import org.junit.Test;
@@ -174,23 +173,6 @@
assertThat(convertedChangeMessage).isEqualTo(changeMessage);
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.ChangeMessage proto =
- Entities.ChangeMessage.newBuilder()
- .setKey(
- Entities.ChangeMessage_Key.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(543))
- .setUuid("change-message-21"))
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.ChangeMessage> parser = changeMessageProtoConverter.getParser();
- Entities.ChangeMessage parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
index bca5eea..bf39ff8 100644
--- a/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
@@ -27,7 +27,6 @@
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.inject.TypeLiteral;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.util.Date;
@@ -165,29 +164,6 @@
assertThat(patchSetApproval.postSubmit()).isEqualTo(false);
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.PatchSetApproval proto =
- Entities.PatchSetApproval.newBuilder()
- .setKey(
- Entities.PatchSetApproval_Key.newBuilder()
- .setPatchSetId(
- Entities.PatchSet_Id.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(42))
- .setId(14))
- .setAccountId(Entities.Account_Id.newBuilder().setId(100013))
- .setLabelId(Entities.LabelId.newBuilder().setId("label-8")))
- .setValue(456)
- .setGranted(987654L)
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.PatchSetApproval> parser = protoConverter.getParser();
- Entities.PatchSetApproval parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
index 2519e75..efeb24f 100644
--- a/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
@@ -26,7 +26,6 @@
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.inject.TypeLiteral;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.util.Optional;
@@ -148,23 +147,6 @@
.build());
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.PatchSet proto =
- Entities.PatchSet.newBuilder()
- .setId(
- Entities.PatchSet_Id.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(103))
- .setId(73))
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.PatchSet> parser = patchSetProtoConverter.getParser();
- Entities.PatchSet parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/plugins/replication b/plugins/replication
index b730e71..ab80790 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit b730e71f3287e49bba0d2bff220ea9597873bd13
+Subproject commit ab8079055a92fa4068a2982306c11425f347e12f
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
index a0c53c6..3548ec8 160000
--- a/plugins/singleusergroup
+++ b/plugins/singleusergroup
@@ -1 +1 @@
-Subproject commit a0c53c6c5ad1ba8f8967ed6d2bcb18995f734cad
+Subproject commit 3548ec83c0c271a8768a6b03b0c28711521ed6cf
diff --git a/polygerrit-ui/app/api/core.ts b/polygerrit-ui/app/api/core.ts
index 91c2b7b..5820139 100644
--- a/polygerrit-ui/app/api/core.ts
+++ b/polygerrit-ui/app/api/core.ts
@@ -32,7 +32,7 @@
* however a range with end_line set to 5 and end_character equal to 0 will not
* include any characters on line 5.
*/
-export interface CommentRange {
+export declare interface CommentRange {
/** The start line number of the range. (1-based) */
start_line: number;
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 0a6366d..b5624c6 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -219,7 +219,7 @@
end_line: number;
}
-export interface CoverageRange {
+export declare interface CoverageRange {
type: CoverageType;
side: Side;
code_range: LineRange;
@@ -245,3 +245,41 @@
side: Side;
lineNum: LineNumber;
}
+
+export enum GrDiffLineType {
+ ADD = 'add',
+ BOTH = 'both',
+ BLANK = 'blank',
+ REMOVE = 'remove',
+}
+
+/** Describes a line to be rendered in a diff. */
+export declare interface GrDiffLine {
+ readonly type: GrDiffLineType;
+ /** The line number on the left side of the diff - 0 means none. */
+ beforeNumber: LineNumber;
+ /** The line number on the right side of the diff - 0 means none. */
+ afterNumber: LineNumber;
+}
+
+/**
+ * Interface to implemented to define a new layer in the diff.
+ *
+ * Layers can affect how the text of the diff or its line numbers
+ * are rendered.
+ */
+export declare interface DiffLayer {
+ /**
+ * Called during rendering and allows annotating the diff text or line number
+ * by mutating those elements.
+ *
+ * @param textElement The rendered text of one side of the diff.
+ * @param lineNumberElement The rendered line number of one side of the diff.
+ * @param line Describes the line that should be annotated.
+ */
+ annotate(
+ textElement: HTMLElement,
+ lineNumberElement: HTMLElement,
+ line: GrDiffLine
+ ): void;
+}
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index b0f87d7..03d5000 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -53,6 +53,7 @@
TAG_SET_WIP = 'autogenerated:gerrit:setWorkInProgress',
TAG_SET_ASSIGNEE = 'autogenerated:gerrit:setAssignee',
TAG_UNSET_ASSIGNEE = 'autogenerated:gerrit:deleteAssignee',
+ TAG_MERGED = 'autogenerated:gerrit:merged',
}
/**
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
index 2c66a3e..e77a5e8 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
@@ -178,7 +178,9 @@
}
_handleCreateClicked() {
- this.$.createOverlay.open();
+ this.$.createOverlay.open().then(() => {
+ this.$.createNewModal.focus();
+ });
}
_visibleToAll(item: GroupInfo) {
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.js b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.js
index ed196e4..2df1ac6 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.js
@@ -155,7 +155,8 @@
});
test('_handleCreateClicked opens modal', () => {
- const openStub = sinon.stub(element.$.createOverlay, 'open');
+ const openStub = sinon.stub(element.$.createOverlay, 'open').returns(
+ Promise.resolve());
element._handleCreateClicked();
assert.isTrue(openStub.called);
});
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
index 5902717..b68f720 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
@@ -55,6 +55,10 @@
this.hasNewGroupName = !!name;
}
+ focus() {
+ this.shadowRoot?.querySelector('input')?.focus();
+ }
+
handleCreateGroup() {
const name = this._name as GroupName;
return this.restApiService.createGroup({name}).then(groupRegistered => {
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index d3ce98a..1b16052 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -83,6 +83,10 @@
return getBaseUrl() + '/admin/repos/' + encodeURL(repoName, true);
}
+ focus() {
+ this.shadowRoot?.querySelector('input')?.focus();
+ }
+
@observe('_repoConfig.name')
_updateRepoName(name: string) {
this.hasNewRepoName = !!name;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
index 6f6f926..c1c8475 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -169,7 +169,9 @@
}
_handleCreateClicked() {
- this.$.createOverlay.open();
+ this.$.createOverlay.open().then(() => {
+ this.$.createNewModal.focus();
+ });
}
_readOnly(repo: ProjectInfoWithName) {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
index 6bf73d1..4904bf4 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
@@ -151,7 +151,8 @@
});
test('_handleCreateClicked opens modal', () => {
- const openStub = sinon.stub(element.$.createOverlay, 'open');
+ const openStub = sinon.stub(element.$.createOverlay, 'open').returns(
+ Promise.resolve());
element._handleCreateClicked();
assert.isTrue(openStub.called);
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 78c48f8..5089b95 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -114,6 +114,7 @@
getApprovalInfo,
getVotingRange,
} from '../../../utils/label-util';
+import {CommentThread} from '../../../utils/comment-util';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -442,6 +443,9 @@
@property({type: String})
_actionLoadingMessage = '';
+ @property({type: Array})
+ commentThreads: CommentThread[] = [];
+
@property({
type: Array,
computed:
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
index 2bbfbbf..71500ac 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
@@ -216,6 +216,7 @@
action="[[_revisionSubmitAction]]"
on-cancel="_handleConfirmDialogCancel"
on-confirm="_handleSubmitConfirm"
+ comment-threads="[[commentThreads]]"
hidden=""
></gr-confirm-submit-dialog>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 3813a03..eded5a1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -21,8 +21,13 @@
import {appContext} from '../../../services/app-context';
import {KnownExperimentId} from '../../../services/flags/flags';
import {Category, CheckRun, Link} from '../../../api/checks';
-import {allRuns$, RunResult} from '../../../services/checks/checks-model';
+import {
+ allRuns$,
+ aPluginHasRegistered,
+ RunResult,
+} from '../../../services/checks/checks-model';
import {fireShowPrimaryTab} from '../../../utils/event-util';
+import '../../shared/gr-avatar/gr-avatar';
import {
hasCompleted,
isRunning,
@@ -33,8 +38,12 @@
CommentThread,
isResolved,
isUnresolved,
+ getFirstComment,
} from '../../../utils/comment-util';
import {pluralize} from '../../../utils/string-util';
+import {AccountInfo} from '../../../types/common';
+import {notUndefined} from '../../../types/types';
+import {uniqueDefinedAvatar} from '../../../utils/account-util';
function filterResults(runs: CheckRun[], category: Category): RunResult[] {
return runs.filter(isRunningOrHasCompleted).reduce((results, run) => {
@@ -71,8 +80,7 @@
color: var(--chip-color);
cursor: pointer;
display: inline-block;
- margin-right: var(--spacing-s);
- padding: var(--spacing-xxs) var(--spacing-m) var(--spacing-xxs)
+ padding: var(--spacing-xxs) var(--spacing-s) var(--spacing-xxs)
var(--spacing-s);
border-radius: 12px;
border: 1px solid gray;
@@ -263,26 +271,29 @@
@customElement('gr-change-summary')
export class GrChangeSummary extends GrLitElement {
- private readonly ciRebootChecksEnabled = appContext.flagsService.isEnabled(
- KnownExperimentId.CI_REBOOT_CHECKS
- );
-
private readonly newChangeSummaryUiEnabled = appContext.flagsService.isEnabled(
KnownExperimentId.NEW_CHANGE_SUMMARY_UI
);
- @property({type: Array})
+ @property({type: Object})
changeComments?: ChangeComments;
- @property({type: Object})
+ @property({type: Array})
commentThreads?: CommentThread[];
+ @property({type: Object})
+ selfAccount?: AccountInfo;
+
@property()
runs: CheckRun[] = [];
+ @property()
+ showChecksSummary = false;
+
constructor() {
super();
this.subscribe('runs', allRuns$);
+ this.subscribe('showChecksSummary', aPluginHasRegistered);
}
static get styles() {
@@ -307,6 +318,12 @@
margin-right: var(--spacing-s);
margin-left: var(--spacing-m);
}
+ gr-avatar {
+ height: var(--line-height-small, 16px);
+ width: var(--line-height-small, 16px);
+ vertical-align: top;
+ margin-right: var(--spacing-xs);
+ }
`,
];
}
@@ -316,15 +333,16 @@
const errors = filterResults(runs, Category.ERROR);
const warnings = filterResults(runs, Category.WARNING);
const infos = filterResults(runs, Category.INFO);
- const numResolvedComments =
+ const countResolvedComments =
this.commentThreads?.filter(isResolved).length ?? 0;
- const numUnResolvedComments =
- this.commentThreads?.filter(isUnresolved).length ?? 0;
+ const unresolvedThreads = this.commentThreads?.filter(isUnresolved) ?? [];
+ const countUnresolvedComments = unresolvedThreads.length;
+ const unresolvedAuthors = this.getAccounts(unresolvedThreads);
const draftCount = this.changeComments?.computeDraftCount() ?? 0;
return html`
<div>
<table>
- <tr ?hidden=${!this.ciRebootChecksEnabled}>
+ <tr ?hidden=${!this.showChecksSummary}>
<td class="key">Checks</td>
<td class="value">
<gr-checks-chip
@@ -359,9 +377,9 @@
<td class="value">
<gr-summary-chip
styleType=${SummaryChipStyles.INFO}
- ?hidden=${!!numResolvedComments ||
+ ?hidden=${!!countResolvedComments ||
!!draftCount ||
- !!numUnResolvedComments}
+ !!countUnresolvedComments}
>
No Comments</gr-summary-chip
>
@@ -375,14 +393,23 @@
<gr-summary-chip
styleType=${SummaryChipStyles.WARNING}
icon="message"
- ?hidden=${!numUnResolvedComments}
- >${numUnResolvedComments} unresolved</gr-summary-chip
+ ?hidden=${!countUnresolvedComments}
+ >
+ ${unresolvedAuthors.map(
+ account =>
+ html`<gr-avatar
+ .account="${account}"
+ image-size="32"
+ aria-label="Account avatar"
+ ></gr-avatar>`
+ )}
+ ${countUnresolvedComments} unresolved</gr-summary-chip
>
<gr-summary-chip
styleType=${SummaryChipStyles.CHECK}
icon="markChatRead"
- ?hidden=${!numResolvedComments}
- >${numResolvedComments} resolved</gr-summary-chip
+ ?hidden=${!countResolvedComments}
+ >${countResolvedComments} resolved</gr-summary-chip
>
</td>
</tr>
@@ -394,6 +421,16 @@
</div>
`;
}
+
+ getAccounts(commentThreads: CommentThread[]): AccountInfo[] {
+ const uniqueAuthors = commentThreads
+ .map(getFirstComment)
+ .map(comment => comment?.author ?? this.selfAccount)
+ .filter(notUndefined)
+ .filter(account => !!account?.avatars?.[0]?.url)
+ .filter(uniqueDefinedAvatar);
+ return uniqueAuthors.slice(0, 3);
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 2d140a9f..e361f9f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -161,6 +161,10 @@
import {KnownExperimentId} from '../../../services/flags/flags';
import {fireTitleChange} from '../../../utils/event-util';
import {GerritView} from '../../../services/router/router-model';
+import {takeUntil} from 'rxjs/operators';
+import {aPluginHasRegistered} from '../../../services/checks/checks-model';
+import {Subject} from 'rxjs';
+import {GrRelatedChangesListExperimental} from '../gr-related-changes-list-experimental/gr-related-changes-list-experimental';
const CHANGE_ID_ERROR = {
MISMATCH: 'mismatch',
@@ -544,13 +548,16 @@
_throttledToggleChangeStar?: EventListener;
- _isChecksEnabled = false;
+ @property({type: Boolean})
+ _showChecksTab = false;
@property({type: Boolean})
_isNewChangeSummaryUiEnabled = false;
restApiService = appContext.restApiService;
+ checksService = appContext.checksService;
+
keyboardShortcuts() {
return {
[Shortcut.SEND_REPLY]: null, // DOC_ONLY binding
@@ -574,12 +581,14 @@
};
}
+ disconnected$ = new Subject();
+
/** @override */
ready() {
super.ready();
- this._isChecksEnabled = this.flagsService.isEnabled(
- KnownExperimentId.CI_REBOOT_CHECKS
- );
+ aPluginHasRegistered.pipe(takeUntil(this.disconnected$)).subscribe(b => {
+ this._showChecksTab = b;
+ });
this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
KnownExperimentId.NEW_CHANGE_SUMMARY_UI
);
@@ -594,6 +603,12 @@
}
/** @override */
+ disconnectedCallback() {
+ this.disconnected$.next();
+ super.disconnectedCallback();
+ }
+
+ /** @override */
created() {
super.created();
@@ -2195,6 +2210,7 @@
// are loaded.
const detailCompletes = this._getChangeDetail();
allDataPromises.push(detailCompletes);
+ this.checksService.reloadAll();
// Resolves when the loading flag is set to false, meaning that some
// change content may start appearing.
@@ -2281,6 +2297,9 @@
this._editingCommitMessage = false;
const relatedChangesLoaded = coreDataPromise.then(() => {
this.getRelatedChangesList()?.reload();
+ if (this._isNewChangeSummaryUiEnabled) {
+ this.getRelatedChangesListExperimental()?.reload();
+ }
});
allDataPromises.push(relatedChangesLoaded);
}
@@ -2804,6 +2823,12 @@
'#relatedChanges'
);
}
+
+ getRelatedChangesListExperimental() {
+ return this.shadowRoot!.querySelector<GrRelatedChangesListExperimental>(
+ '#relatedChangesExperimental'
+ );
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 36e73b2..ed33916 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -449,6 +449,7 @@
on-edit-tap="_handleEditTap"
on-stop-edit-tap="_handleStopEditTap"
on-download-tap="_handleOpenDownloadDialog"
+ comment-threads="[[_commentThreads]]"
></gr-change-actions>
</div>
<!-- end commit actions -->
@@ -580,6 +581,7 @@
class$="new-change-summary-[[_isNewChangeSummaryUiEnabled]]"
change-comments="[[_changeComments]]"
comment-threads="[[_commentThreads]]"
+ self-account="[[_account]]"
>
</gr-change-summary>
<gr-endpoint-decorator name="commit-container">
@@ -592,11 +594,14 @@
</gr-endpoint-param>
</gr-endpoint-decorator>
</div>
- <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
- <gr-related-changes-list-experimental></gr-related-changes-list-experimental>
- </template>
- <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
- <div class="relatedChanges">
+ <div class="relatedChanges">
+ <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+ <gr-related-changes-list-experimental
+ change="[[_change]]"
+ id="relatedChangesExperimental"
+ ></gr-related-changes-list-experimental>
+ </template>
+ <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
<gr-related-changes-list
id="relatedChanges"
class$="[[_computeRelatedChangesClass(_relatedChangesCollapsed)]]"
@@ -618,8 +623,8 @@
[[_computeCollapseText(_relatedChangesCollapsed)]]
</gr-button>
</div>
- </div>
- </template>
+ </template>
+ </div>
</div>
</div>
</div>
@@ -639,7 +644,7 @@
<span>Comments</span></gr-tooltip-content
>
</paper-tab>
- <template is="dom-if" if="[[_isChecksEnabled]]">
+ <template is="dom-if" if="[[_showChecksTab]]">
<paper-tab data-name$="[[_constants.PrimaryTab.CHECKS]]"
>Checks</paper-tab
>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
index 6e2e595..45b61ae 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
@@ -20,6 +20,7 @@
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../../../styles/shared-styles';
+import '../gr-thread-list/gr-thread-list';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
@@ -28,6 +29,7 @@
import {ChangeInfo, ActionInfo} from '../../../types/common';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {pluralize} from '../../../utils/string-util';
+import {CommentThread, isUnresolved} from '../../../utils/comment-util';
export interface GrConfirmSubmitDialog {
$: {
@@ -60,6 +62,9 @@
@property({type: Object})
action?: ActionInfo;
+ @property({type: Array})
+ commentThreads: CommentThread[] = [];
+
resetFocus() {
this.$.dialog.resetFocus();
}
@@ -72,6 +77,10 @@
);
}
+ _computeUnresolvedThreads(commentThreads: CommentThread[]) {
+ return commentThreads.filter(thread => isUnresolved(thread));
+ }
+
_computeUnresolvedCommentsWarning(change: ChangeInfo) {
const unresolvedCount = change.unresolved_comment_count;
if (!unresolvedCount) throw new Error('unresolved comments undefined or 0');
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_html.ts
index 6c7b1c2..f62c5b82 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_html.ts
@@ -67,6 +67,15 @@
></iron-icon>
[[_computeUnresolvedCommentsWarning(change)]]
</p>
+ <gr-thread-list
+ id="commentList"
+ threads="[[_computeUnresolvedThreads(commentThreads)]]"
+ change="[[change]]"
+ change-num="[[change._number]]"
+ logged-in="true"
+ hide-toggle-buttons
+ >
+ </gr-thread-list>
</template>
<template is="dom-if" if="[[_computeHasChangeEdit(change)]]">
<iron-icon
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index bc61dad..39cdf22 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -27,7 +27,7 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-message_html';
-import {SpecialFilePath} from '../../../constants/constants';
+import {MessageTag, SpecialFilePath} from '../../../constants/constants';
import {customElement, property, computed, observe} from '@polymer/decorators';
import {
ChangeInfo,
@@ -54,6 +54,8 @@
const PATCH_SET_PREFIX_PATTERN = /^(?:Uploaded\s*)?(?:P|p)atch (?:S|s)et \d+:\s*(.*)/;
const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?[.]?$/;
+const UPLOADED_NEW_PATCHSET_PATTERN = /Uploaded patch set (\d+)./;
+const MERGED_PATCHSET_PATTERN = /(\d+) is the latest approved patch-set/;
declare global {
interface HTMLElementTagNameMap {
@@ -268,28 +270,49 @@
return this._patchsetCommentSummary(commentThreads);
}
+ _showViewDiffButton(message?: ChangeMessage) {
+ return (
+ this._isNewPatchsetTag(message?.tag) || this._isMergePatchset(message)
+ );
+ }
+
+ _isMergePatchset(message?: ChangeMessage) {
+ return (
+ message?.tag === MessageTag.TAG_MERGED &&
+ message?.message.match(MERGED_PATCHSET_PATTERN)
+ );
+ }
+
_isNewPatchsetTag(tag?: ReviewInputTag) {
- return tag?.endsWith(':newPatchSet') || tag?.endsWith(':newWipPatchSet');
+ return (
+ tag === MessageTag.TAG_NEW_PATCHSET ||
+ tag === MessageTag.TAG_NEW_WIP_PATCHSET
+ );
}
_handleViewPatchsetDiff(e: Event) {
if (!this.message || !this.change) return;
- const match = this.message.message.match(/Uploaded patch set (\d+)./);
let patchNum: PatchSetNum;
- // Message is of the form "Commit Message was updated" or "Patchset X
- // was rebased"
- if (!match || match.length < 1) {
- patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
- } else {
+ let basePatchNum: PatchSetNum;
+ if (this.message.message.match(UPLOADED_NEW_PATCHSET_PATTERN)) {
+ const match = this.message.message.match(UPLOADED_NEW_PATCHSET_PATTERN)!;
if (isNaN(Number(match[1])))
throw new Error('invalid patchnum in message');
patchNum = Number(match[1]) as PatchSetNum;
+ basePatchNum = computePredecessor(patchNum)!;
+ } else if (this.message.message.match(MERGED_PATCHSET_PATTERN)) {
+ const match = this.message.message.match(MERGED_PATCHSET_PATTERN)!;
+ if (isNaN(Number(match[1])))
+ throw new Error('invalid patchnum in message');
+ basePatchNum = Number(match[1]) as PatchSetNum;
+ patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
+ } else {
+ // Message is of the form "Commit Message was updated" or "Patchset X
+ // was rebased"
+ patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
+ basePatchNum = computePredecessor(patchNum)!;
}
- GerritNav.navigateToChange(
- this.change,
- patchNum,
- computePredecessor(patchNum)
- );
+ GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
// stop propagation to stop message expansion
e.stopPropagation();
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index 48e5e24..070aa2d 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -279,7 +279,7 @@
</div>
</template>
<span class="dateContainer">
- <template is="dom-if" if="[[_isNewPatchsetTag(message.tag)]]">
+ <template is="dom-if" if="[[_showViewDiffButton(message)]]">
<gr-button
class="patchsetDiffButton"
on-click="_handleViewPatchsetDiff"
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
index e2c7e86..3f4e13a 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
@@ -266,6 +266,14 @@
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(navStub.calledWithExactly(element.change, 4, 3));
});
+
+ test('Merged patchset change message', () => {
+ element.message = {
+ message: 'abcd↵3 is the latest approved patch-set.↵abc',
+ };
+ element._handleViewPatchsetDiff(new MouseEvent('click'));
+ assert.isTrue(navStub.calledWithExactly(element.change, 4, 3));
+ });
});
suite('compute messages', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
index ab7566f..a66bb04 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
@@ -16,22 +16,217 @@
*/
import {html} from 'lit-html';
import {GrLitElement} from '../../lit/gr-lit-element';
-import {customElement} from 'lit-element';
+import {customElement, property, css} from 'lit-element';
import {sharedStyles} from '../../../styles/shared-styles';
+import {
+ SubmittedTogetherInfo,
+ ChangeInfo,
+ RelatedChangeAndCommitInfo,
+} from '../../../types/common';
+import {appContext} from '../../../services/app-context';
+import {ParsedChangeInfo} from '../../../types/types';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {pluralize} from '../../../utils/string-util';
+import {ChangeStatus} from '../../../constants/constants';
+function isChangeInfo(
+ x: ChangeInfo | RelatedChangeAndCommitInfo | ParsedChangeInfo
+): x is ChangeInfo | ParsedChangeInfo {
+ return (x as ChangeInfo)._number !== undefined;
+}
@customElement('gr-related-changes-list-experimental')
export class GrRelatedChangesListExperimental extends GrLitElement {
+ @property()
+ change?: ParsedChangeInfo;
+
+ @property()
+ _submittedTogether?: SubmittedTogetherInfo = {
+ changes: [],
+ non_visible_changes: 0,
+ };
+
+ private readonly restApiService = appContext.restApiService;
+
static get styles() {
- return [sharedStyles];
+ return [
+ sharedStyles,
+ css`
+ .title {
+ font-weight: var(--font-weight-bold);
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
+ h4,
+ section gr-related-change {
+ display: flex;
+ }
+ h4:before,
+ section gr-related-change:before {
+ content: ' ';
+ flex-shrink: 0;
+ width: 1.2em;
+ }
+ .note {
+ color: var(--error-text-color);
+ }
+ `,
+ ];
}
render() {
- return html``;
+ const submittedTogetherChanges = this._submittedTogether?.changes ?? [];
+ const countNonVisibleChanges =
+ this._submittedTogether?.non_visible_changes ?? 0;
+ return html` <section
+ id="submittedTogether"
+ ?hidden=${!submittedTogetherChanges?.length &&
+ !this._submittedTogether?.non_visible_changes}
+ >
+ <h4 class="title">Submitted together</h4>
+ ${submittedTogetherChanges.map(
+ relatedChange =>
+ html`<gr-related-change
+ .currentChange="${this._changesEqual(relatedChange, this.change)}"
+ .change="${relatedChange}"
+ ></gr-related-change>`
+ )}
+ <div class="note" ?hidden=${!countNonVisibleChanges}>
+ (+ ${pluralize(countNonVisibleChanges, 'non-visible change')})
+ </div>
+ </section>`;
+ }
+
+ reload() {
+ if (!this.change) return Promise.reject(new Error('change missing'));
+ return this.restApiService
+ .getChangesSubmittedTogether(this.change._number)
+ .then(response => {
+ this._submittedTogether = response;
+ });
+ }
+
+ /**
+ * Do the given objects describe the same change? Compares the changes by
+ * their numbers.
+ */
+ _changesEqual(
+ a?: ChangeInfo | RelatedChangeAndCommitInfo,
+ b?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
+ ) {
+ const aNum = this._getChangeNumber(a);
+ const bNum = this._getChangeNumber(b);
+ return aNum === bNum;
+ }
+
+ /**
+ * Get the change number from either a ChangeInfo (such as those included in
+ * SubmittedTogetherInfo responses) or get the change number from a
+ * RelatedChangeAndCommitInfo (such as those included in a
+ * RelatedChangesInfo response).
+ */
+ _getChangeNumber(
+ change?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
+ ) {
+ // Default to 0 if change property is not defined.
+ if (!change) return 0;
+
+ if (isChangeInfo(change)) {
+ return change._number;
+ }
+ return change._change_number;
+ }
+}
+
+@customElement('gr-related-change')
+export class GrRelatedChange extends GrLitElement {
+ @property()
+ change?: ChangeInfo;
+
+ @property()
+ currentChange = false;
+
+ static get styles() {
+ return [
+ sharedStyles,
+ css`
+ a {
+ display: block;
+ }
+ .changeContainer,
+ a {
+ max-width: 100%;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ white-space: nowrap;
+ }
+ .changeContainer {
+ display: flex;
+ }
+ .strikethrough {
+ color: var(--deemphasized-text-color);
+ text-decoration: line-through;
+ }
+ .submittableCheck {
+ padding-left: var(--spacing-s);
+ color: var(--positive-green-text-color);
+ display: none;
+ }
+ .submittableCheck.submittable {
+ display: inline;
+ }
+ .arrowToCurrentChange {
+ position: absolute;
+ }
+ `,
+ ];
+ }
+
+ render() {
+ const change = this.change;
+ if (!change) throw new Error('Missing change');
+ const linkClass = this._computeLinkClass(change);
+ return html`<span
+ role="img"
+ class="arrowToCurrentChange"
+ aria-label="Arrow marking current change"
+ ?hidden=${!this.currentChange}
+ >➔</span
+ >
+ <div class="changeContainer">
+ <a
+ href="${GerritNav.getUrlForChangeById(
+ change._number,
+ change.project
+ )}"
+ class="${linkClass}"
+ >${change.project}: ${change.branch}: ${change.subject}</a
+ >
+ <span
+ tabindex="-1"
+ title="Submittable"
+ class="submittableCheck ${linkClass}"
+ role="img"
+ aria-label="Submittable"
+ >✓</span
+ >
+ </div> `;
+ }
+
+ _computeLinkClass(change: ChangeInfo) {
+ const statuses = [];
+ if (change.status === ChangeStatus.ABANDONED) {
+ statuses.push('strikethrough');
+ }
+ if (change.submittable) {
+ statuses.push('submittable');
+ }
+ return statuses.join(' ');
}
}
declare global {
interface HTMLElementTagNameMap {
'gr-related-changes-list-experimental': GrRelatedChangesListExperimental;
+ 'gr-related-change': GrRelatedChange;
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.ts
index 26d413f..2927101 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.ts
@@ -15,20 +15,17 @@
* limitations under the License.
*/
-import {LineNumber} from '../../../api/diff';
+import {
+ GrDiffLine as GrDiffLineApi,
+ GrDiffLineType,
+ LineNumber,
+} from '../../../api/diff';
-export {LineNumber};
+export {GrDiffLineType, LineNumber};
export const FILE = 'FILE';
-export enum GrDiffLineType {
- ADD = 'add',
- BOTH = 'both',
- BLANK = 'blank',
- REMOVE = 'remove',
-}
-
-export class GrDiffLine {
+export class GrDiffLine implements GrDiffLineApi {
constructor(
readonly type: GrDiffLineType,
public beforeNumber: LineNumber = 0,
diff --git a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
index b7a1216..82c1087 100644
--- a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
@@ -46,7 +46,7 @@
constructor(readonly plugin: PluginApi) {}
announceUpdate() {
- this.checksService.announceUpdate(this.plugin.getPluginName());
+ this.checksService.reload(this.plugin.getPluginName());
}
register(provider: ChecksProvider, config?: ChecksApiConfig): void {
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index bce46c3..1116db0 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -24,7 +24,7 @@
LinkIcon,
RunStatus,
} from '../../api/checks';
-import {map} from 'rxjs/operators';
+import {distinctUntilChanged, map} from 'rxjs/operators';
// This is a convenience type for working with results, because when working
// with a bunch of results you will typically also want to know about the run
@@ -48,6 +48,11 @@
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const checksState$: Observable<ChecksState> = privateState$;
+export const aPluginHasRegistered = checksState$.pipe(
+ map(state => Object.keys(state).length > 0),
+ distinctUntilChanged()
+);
+
export const allRuns$ = checksState$.pipe(
map(state => {
return Object.values(state).reduce(
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts
index 439a44f..a8dd8b8 100644
--- a/polygerrit-ui/app/services/checks/checks-service.ts
+++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -15,7 +15,13 @@
* limitations under the License.
*/
-import {switchMap, takeWhile, tap, withLatestFrom} from 'rxjs/operators';
+import {
+ switchMap,
+ takeWhile,
+ tap,
+ throttleTime,
+ withLatestFrom,
+} from 'rxjs/operators';
import {
ChecksApiConfig,
ChecksProvider,
@@ -36,12 +42,16 @@
export class ChecksService {
private readonly providers: {[name: string]: ChecksProvider} = {};
- private readonly anouncementSubjects: {[name: string]: Subject<void>} = {};
+ private readonly reloadSubjects: {[name: string]: Subject<void>} = {};
private changeAndPatchNum$ = change$.pipe(withLatestFrom(currentPatchNum$));
- announceUpdate(pluginName: string) {
- this.anouncementSubjects[pluginName].next();
+ reload(pluginName: string) {
+ this.reloadSubjects[pluginName].next();
+ }
+
+ reloadAll() {
+ Object.keys(this.providers).forEach(key => this.reload(key));
}
register(
@@ -50,12 +60,12 @@
config: ChecksApiConfig
) {
this.providers[pluginName] = provider;
- this.anouncementSubjects[pluginName] = new BehaviorSubject<void>(undefined);
+ this.reloadSubjects[pluginName] = new BehaviorSubject<void>(undefined);
updateStateSetProvider(pluginName, config);
// Both, changed numbers and and announceUpdate request should trigger.
combineLatest([
this.changeAndPatchNum$,
- this.anouncementSubjects[pluginName],
+ this.reloadSubjects[pluginName].pipe(throttleTime(1000)),
])
.pipe(
takeWhile(_ => !!this.providers[pluginName]),
@@ -77,5 +87,6 @@
})
)
.subscribe(() => {});
+ this.reload(pluginName);
}
}
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index c43a8fd..5edbf87 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -25,6 +25,9 @@
*/
export enum KnownExperimentId {
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
+ // Note that this flag is not supposed to be used by Gerrit itself, but can
+ // be used by plugins. The new Checks UI will show up, if a plugin registers
+ // with the new Checks plugin API.
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
PORTING_COMMENTS = 'UiFeature__porting_comments',
diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts
index a7e654c..be80411 100644
--- a/polygerrit-ui/app/types/types.ts
+++ b/polygerrit-ui/app/types/types.ts
@@ -14,9 +14,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+import {DiffLayer as DiffLayerApi} from '../api/diff';
import {DiffViewMode, MessageTag, Side} from '../constants/constants';
import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer';
-import {GrDiffLine} from '../elements/diff/gr-diff/gr-diff-line';
import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer';
import {PaperInputElement} from '@polymer/paper-input/paper-input';
import {
@@ -150,8 +150,7 @@
side: Side
) => void;
-export interface DiffLayer {
- annotate(el: HTMLElement, lineEl: HTMLElement, line: GrDiffLine): void;
+export interface DiffLayer extends DiffLayerApi {
addListener?(listener: DiffLayerListener): void;
removeListener?(listener: DiffLayerListener): void;
}
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 99c864e..bb9f328 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -35,3 +35,17 @@
export function removeServiceUsers(accounts?: AccountInfo[]): AccountInfo[] {
return accounts?.filter(a => !isServiceUser(a)) || [];
}
+
+export function hasSameAvatar(account?: AccountInfo, other?: AccountInfo) {
+ return account?.avatars?.[0]?.url === other?.avatars?.[0]?.url;
+}
+
+export function uniqueDefinedAvatar(
+ account: AccountInfo,
+ index: number,
+ accountArray: AccountInfo[]
+) {
+ return (
+ index === accountArray.findIndex(other => hasSameAvatar(account, other))
+ );
+}
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 9c34b60..d7e7595 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -173,6 +173,10 @@
return thread && len ? thread.comments[len - 1] : undefined;
}
+export function getFirstComment(thread?: CommentThread): UIComment | undefined {
+ return thread?.comments?.[0];
+}
+
export function isUnresolved(thread?: CommentThread): boolean {
return !isResolved(thread);
}
