Merge "Increase contrast of +x/-y delta count text"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 47afdba..8509b1f 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -1425,8 +1425,7 @@
any of their groups is used.
This limit applies not only to the link:cmd-query.html[`gerrit query`]
-command, but also to the web UI results pagination size in the new
-PolyGerrit UI and, limited to the full project list, in the old GWT UI.
+command, but also to the web UI results pagination size.
[[capability_readAs]]
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 8ad8669..1bf51a1 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -828,6 +828,49 @@
+
If 0 or negative, disk storage for the cache is disabled.
+[[cache.name.expireAfterWrite]]cache.<name>.expireAfterWrite::
++
+Duration after which a cached value will be evicted and not
+read anymore.
++
+Values should use common unit suffixes to express their setting:
++
+* ms, milliseconds
+* s, sec, second, seconds
+* m, min, minute, minutes
+* h, hr, hour, hours
++
+Disabled by default.
+
+[[cache.name.refreshAfterWrite]]cache.<name>.refreshAfterWrite::
++
+Duration after which we asynchronously refresh the cached value.
++
+Values should use common unit suffixes to express their setting:
++
+* ms, milliseconds
+* s, sec, second, seconds
+* m, min, minute, minutes
+* h, hr, hour, hours
++
+This applies only to these caches that support refreshing:
++
+* `"projects"`: Caching project information in-memory. Defaults to 15 minutes.
+
+[[cache.refreshThreadPoolSize]]cache.refreshThreadPoolSize::
++
+Number of threads that are available to refresh cached values that became
+out of date. This applies only to these caches that support refreshing:
++
+* `"projects"`: Caching project information in-memory
++
+Refreshes will only be scheduled on this executor if the values are
+out of sync.
+The check if they are is cheap and always happens on the thread that
+inquires for a cached value.
++
+Defaults to 2.
+
==== [[cache_names]]Standard Caches
cache `"accounts"`::
@@ -1125,22 +1168,6 @@
+
Default is true, enabled.
-[[cache.projects.checkFrequency]]cache.projects.checkFrequency::
-+
-How often project configuration should be checked for update from Git.
-Gerrit Code Review caches project access rules and configuration in
-memory, checking the refs/meta/config branch every checkFrequency
-minutes to see if a new revision should be loaded and used for future
-access. Values can be specified using standard time unit abbreviations
-('ms', 'sec', 'min', etc.).
-+
-If set to 0, checks occur every time, which may slow down operations.
-If set to 'disabled' or 'off', no check will ever be done.
-Administrators may force the cache to flush with
-link:cmd-flush-caches.html[gerrit flush-caches].
-+
-Default is 5 minutes.
-
[[cache.projects.loadOnStartup]]cache.projects.loadOnStartup::
+
If the project cache should be loaded during server startup.
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 7e6799be..3040348 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -64,6 +64,7 @@
* `caches/memory_eviction_count`: Memory eviction count.
* `caches/disk_cached`: Disk entries used by persistent cache.
* `caches/disk_hit_ratio`: Disk hit ratio for persistent cache.
+* `caches/refresh_count`: The number of refreshes per cache with an indicator if a reload was necessary.
=== Change
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 0ef6ad5..2d62608 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -483,7 +483,6 @@
cfg.setString("gerrit", null, "basePath", "git");
cfg.setBoolean("sendemail", null, "enable", true);
cfg.setInt("sendemail", null, "threadPoolSize", 0);
- cfg.setInt("cache", "projects", "checkFrequency", 0);
cfg.setInt("plugins", null, "checkFrequency", 0);
cfg.setInt("sshd", null, "threads", 1);
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index 05992d4..0c3b7b0 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -246,7 +246,7 @@
}
private boolean sshdOff() {
- return new SshAddressesModule().getListenAddresses(config).isEmpty();
+ return new SshAddressesModule().provideListenAddresses(config).isEmpty();
}
private Injector createCfgInjector() {
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 034e042e..57bec71 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -380,7 +380,7 @@
}
private boolean sshdOff() {
- return new SshAddressesModule().getListenAddresses(config).isEmpty();
+ return new SshAddressesModule().provideListenAddresses(config).isEmpty();
}
private String myVersion() {
diff --git a/java/com/google/gerrit/server/CacheRefreshExecutor.java b/java/com/google/gerrit/server/CacheRefreshExecutor.java
new file mode 100644
index 0000000..1a377c3
--- /dev/null
+++ b/java/com/google/gerrit/server/CacheRefreshExecutor.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+
+/**
+ * Marker on the global {@link java.util.concurrent.ThreadPoolExecutor} used to refresh outdated
+ * values in caches.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface CacheRefreshExecutor {}
diff --git a/java/com/google/gerrit/server/PublishCommentsOp.java b/java/com/google/gerrit/server/PublishCommentsOp.java
index df57629..745755b 100644
--- a/java/com/google/gerrit/server/PublishCommentsOp.java
+++ b/java/com/google/gerrit/server/PublishCommentsOp.java
@@ -19,6 +19,7 @@
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.change.EmailReviewComments;
@@ -31,6 +32,7 @@
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.CommentsRejectedException;
import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.update.RepoView;
import com.google.gerrit.server.util.LabelVote;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -114,7 +116,16 @@
PatchSet ps = psUtil.get(changeNotes, psId);
NotifyResolver.Result notify = ctx.getNotify(changeNotes.getChangeId());
if (notify.shouldNotify()) {
- email.create(notify, changeNotes, ps, user, message, comments, null, labelDelta).sendAsync();
+ RepoView repoView;
+ try {
+ repoView = ctx.getRepoView();
+ } catch (IOException ex) {
+ throw new StorageException(
+ String.format("Repository %s not found", ctx.getProject().get()), ex);
+ }
+ email
+ .create(notify, changeNotes, ps, user, message, comments, null, labelDelta, repoView)
+ .sendAsync();
}
commentAdded.fire(
changeNotes.getChange(),
diff --git a/java/com/google/gerrit/server/cache/CacheBinding.java b/java/com/google/gerrit/server/cache/CacheBinding.java
index 9d90d073..99db64e 100644
--- a/java/com/google/gerrit/server/cache/CacheBinding.java
+++ b/java/com/google/gerrit/server/cache/CacheBinding.java
@@ -29,6 +29,13 @@
/** Set the time an element lives after last access before being expired. */
CacheBinding<K, V> expireFromMemoryAfterAccess(Duration duration);
+ /**
+ * Set the time that an element will be refreshed after. Elements older than this but younger than
+ * {@link #expireAfterWrite(Duration)} will still be returned, but on access a task is queued to
+ * refresh their value asynchronously.
+ */
+ CacheBinding<K, V> refreshAfterWrite(Duration duration);
+
/** Populate the cache with items from the CacheLoader. */
CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz);
diff --git a/java/com/google/gerrit/server/cache/CacheDef.java b/java/com/google/gerrit/server/cache/CacheDef.java
index d0c633e..31a453e 100644
--- a/java/com/google/gerrit/server/cache/CacheDef.java
+++ b/java/com/google/gerrit/server/cache/CacheDef.java
@@ -51,6 +51,9 @@
Duration expireFromMemoryAfterAccess();
@Nullable
+ Duration refreshAfterWrite();
+
+ @Nullable
Weigher<K, V> weigher();
@Nullable
diff --git a/java/com/google/gerrit/server/cache/CacheProvider.java b/java/com/google/gerrit/server/cache/CacheProvider.java
index fe4244c..2dd9e1f 100644
--- a/java/com/google/gerrit/server/cache/CacheProvider.java
+++ b/java/com/google/gerrit/server/cache/CacheProvider.java
@@ -38,6 +38,7 @@
private long maximumWeight;
private Duration expireAfterWrite;
private Duration expireFromMemoryAfterAccess;
+ private Duration refreshAfterWrite;
private Provider<CacheLoader<K, V>> loader;
private Provider<Weigher<K, V>> weigher;
@@ -90,6 +91,13 @@
}
@Override
+ public CacheBinding<K, V> refreshAfterWrite(Duration duration) {
+ checkNotFrozen();
+ refreshAfterWrite = duration;
+ return this;
+ }
+
+ @Override
public CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> impl) {
checkNotFrozen();
loader = module.bindCacheLoader(this, impl);
@@ -151,6 +159,11 @@
}
@Override
+ public Duration refreshAfterWrite() {
+ return refreshAfterWrite;
+ }
+
+ @Override
@Nullable
public Weigher<K, V> weigher() {
return weigher != null ? weigher.get() : null;
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
index 5d9ce60..aa62745 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
@@ -43,6 +43,11 @@
}
@Override
+ public Duration refreshAfterWrite() {
+ return source.refreshAfterWrite();
+ }
+
+ @Override
public Weigher<K, V> weigher() {
Weigher<K, V> weigher = source.weigher();
if (weigher == null) {
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index 8f7e360..82615a4 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -237,6 +237,7 @@
def.valueSerializer(),
def.version(),
maxSize,
- def.expireAfterWrite());
+ def.expireAfterWrite(),
+ def.expireFromMemoryAfterAccess());
}
}
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
index ef4e44c..7a53600 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
@@ -23,6 +23,9 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.common.hash.BloomFilter;
+import com.google.common.util.concurrent.FutureCallback;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.cache.PersistentCache;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
@@ -40,6 +43,7 @@
import java.sql.Statement;
import java.sql.Timestamp;
import java.time.Duration;
+import java.time.Instant;
import java.util.Calendar;
import java.util.Map;
import java.util.concurrent.ArrayBlockingQueue;
@@ -122,7 +126,12 @@
@Override
public V get(K key) throws ExecutionException {
if (mem instanceof LoadingCache) {
- return ((LoadingCache<K, ValueHolder<V>>) mem).get(key).value;
+ LoadingCache<K, ValueHolder<V>> asLoadingCache = (LoadingCache<K, ValueHolder<V>>) mem;
+ ValueHolder<V> valueHolder = asLoadingCache.get(key);
+ if (store.needsRefresh(valueHolder.created)) {
+ asLoadingCache.refresh(key);
+ }
+ return valueHolder.value;
}
throw new UnsupportedOperationException();
}
@@ -139,8 +148,8 @@
}
}
- ValueHolder<V> h = new ValueHolder<>(valueLoader.call());
- h.created = TimeUtil.nowMs();
+ ValueHolder<V> h =
+ new ValueHolder<>(valueLoader.call(), Instant.ofEpochMilli(TimeUtil.nowMs()));
executor.execute(() -> store.put(key, h));
return h;
})
@@ -149,8 +158,7 @@
@Override
public void put(K key, V val) {
- final ValueHolder<V> h = new ValueHolder<>(val);
- h.created = TimeUtil.nowMs();
+ final ValueHolder<V> h = new ValueHolder<>(val, Instant.ofEpochMilli(TimeUtil.nowMs()));
mem.put(key, h);
executor.execute(() -> store.put(key, h));
}
@@ -217,11 +225,12 @@
static class ValueHolder<V> {
final V value;
- long created;
+ final Instant created;
volatile boolean clean;
- ValueHolder(V value) {
+ ValueHolder(V value, Instant created) {
this.value = value;
+ this.created = created;
}
}
@@ -248,12 +257,34 @@
}
}
- final ValueHolder<V> h = new ValueHolder<>(loader.load(key));
- h.created = TimeUtil.nowMs();
+ final ValueHolder<V> h =
+ new ValueHolder<>(loader.load(key), Instant.ofEpochMilli(TimeUtil.nowMs()));
executor.execute(() -> store.put(key, h));
return h;
}
}
+
+ @Override
+ public ListenableFuture<ValueHolder<V>> reload(K key, ValueHolder<V> oldValue)
+ throws Exception {
+ ListenableFuture<V> reloadedValue = loader.reload(key, oldValue.value);
+ Futures.addCallback(
+ reloadedValue,
+ new FutureCallback<V>() {
+ @Override
+ public void onSuccess(V result) {
+ store.put(key, new ValueHolder<>(result, TimeUtil.now()));
+ }
+
+ @Override
+ public void onFailure(Throwable t) {
+ logger.atWarning().withCause(t).log("Unable to reload cache value");
+ }
+ },
+ executor);
+
+ return Futures.transform(reloadedValue, v -> new ValueHolder<>(v, TimeUtil.now()), executor);
+ }
}
static class SqlStore<K, V> {
@@ -263,6 +294,7 @@
private final int version;
private final long maxSize;
@Nullable private final Duration expireAfterWrite;
+ @Nullable private final Duration refreshAfterWrite;
private final BlockingQueue<SqlHandle> handles;
private final AtomicLong hitCount = new AtomicLong();
private final AtomicLong missCount = new AtomicLong();
@@ -276,13 +308,15 @@
CacheSerializer<V> valueSerializer,
int version,
long maxSize,
- @Nullable Duration expireAfterWrite) {
+ @Nullable Duration expireAfterWrite,
+ @Nullable Duration refreshAfterWrite) {
this.url = jdbcUrl;
this.keyType = createKeyType(keyType, keySerializer);
this.valueSerializer = valueSerializer;
this.version = version;
this.maxSize = maxSize;
this.expireAfterWrite = expireAfterWrite;
+ this.refreshAfterWrite = refreshAfterWrite;
int cores = Runtime.getRuntime().availableProcessors();
int keep = Math.min(cores, 16);
@@ -394,14 +428,14 @@
}
Timestamp created = r.getTimestamp(2);
- if (expired(created)) {
+ if (expired(created.toInstant())) {
invalidate(key);
missCount.incrementAndGet();
return null;
}
V val = valueSerializer.deserialize(r.getBytes(1));
- ValueHolder<V> h = new ValueHolder<>(val);
+ ValueHolder<V> h = new ValueHolder<>(val, created.toInstant());
h.clean = true;
hitCount.incrementAndGet();
touch(c, key);
@@ -429,14 +463,22 @@
return false;
}
- private boolean expired(Timestamp created) {
+ private boolean expired(Instant created) {
if (expireAfterWrite == null) {
return false;
}
- Duration age = Duration.between(created.toInstant(), TimeUtil.now());
+ Duration age = Duration.between(created, TimeUtil.now());
return age.compareTo(expireAfterWrite) > 0;
}
+ private boolean needsRefresh(Instant created) {
+ if (refreshAfterWrite == null) {
+ return false;
+ }
+ Duration age = Duration.between(created, TimeUtil.now());
+ return age.compareTo(refreshAfterWrite) > 0;
+ }
+
private void touch(SqlHandle c, K key) throws IOException, SQLException {
if (c.touch == null) {
c.touch = c.conn.prepareStatement("UPDATE data SET accessed=? WHERE k=? AND version=?");
@@ -474,7 +516,7 @@
keyType.set(c.put, 1, key);
c.put.setBytes(2, valueSerializer.serialize(holder.value));
c.put.setInt(3, version);
- c.put.setTimestamp(4, new Timestamp(holder.created));
+ c.put.setTimestamp(4, Timestamp.from(holder.created));
c.put.setTimestamp(5, TimeUtil.nowTs());
c.put.executeUpdate();
holder.clean = true;
@@ -560,7 +602,7 @@
while (maxSize < used && r.next()) {
K key = keyType.get(r, 1);
Timestamp created = r.getTimestamp(3);
- if (mem.getIfPresent(key) != null && !expired(created)) {
+ if (mem.getIfPresent(key) != null && !expired(created.toInstant())) {
touch(c, key);
} else {
invalidate(c, key);
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index 9906b3d..23caca7 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -105,6 +105,21 @@
builder.expireAfterAccess(expireAfterAccess.toNanos(), NANOSECONDS);
}
+ Duration refreshAfterWrite = def.refreshAfterWrite();
+ if (has(def.configKey(), "refreshAfterWrite")) {
+ builder.refreshAfterWrite(
+ ConfigUtil.getTimeUnit(
+ cfg,
+ "cache",
+ def.configKey(),
+ "refreshAfterWrite",
+ toSeconds(refreshAfterWrite),
+ SECONDS),
+ SECONDS);
+ } else if (refreshAfterWrite != null) {
+ builder.refreshAfterWrite(refreshAfterWrite.toNanos(), NANOSECONDS);
+ }
+
return builder;
}
@@ -141,6 +156,21 @@
builder.expireAfterAccess(expireAfterAccess.toNanos(), NANOSECONDS);
}
+ Duration refreshAfterWrite = def.refreshAfterWrite();
+ if (has(def.configKey(), "refreshAfterWrite")) {
+ builder.expireAfterAccess(
+ ConfigUtil.getTimeUnit(
+ cfg,
+ "cache",
+ def.configKey(),
+ "refreshAfterWrite",
+ toSeconds(refreshAfterWrite),
+ SECONDS),
+ SECONDS);
+ } else if (refreshAfterWrite != null) {
+ builder.refreshAfterWrite(refreshAfterWrite.toNanos(), NANOSECONDS);
+ }
+
return builder;
}
diff --git a/java/com/google/gerrit/server/change/AbandonOp.java b/java/com/google/gerrit/server/change/AbandonOp.java
index eb6e8d7..9e228d9 100644
--- a/java/com/google/gerrit/server/change/AbandonOp.java
+++ b/java/com/google/gerrit/server/change/AbandonOp.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.extensions.events.ChangeAbandoned;
import com.google.gerrit.server.mail.send.AbandonedSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -42,6 +43,7 @@
private final ChangeMessagesUtil cmUtil;
private final PatchSetUtil psUtil;
private final ChangeAbandoned changeAbandoned;
+ private final MessageIdGenerator messageIdGenerator;
private final String msgTxt;
private final AccountState accountState;
@@ -61,12 +63,14 @@
ChangeMessagesUtil cmUtil,
PatchSetUtil psUtil,
ChangeAbandoned changeAbandoned,
+ MessageIdGenerator messageIdGenerator,
@Assisted @Nullable AccountState accountState,
@Assisted @Nullable String msgTxt) {
this.abandonedSenderFactory = abandonedSenderFactory;
this.cmUtil = cmUtil;
this.psUtil = psUtil;
this.changeAbandoned = changeAbandoned;
+ this.messageIdGenerator = messageIdGenerator;
this.accountState = accountState;
this.msgTxt = Strings.nullToEmpty(msgTxt);
@@ -116,6 +120,7 @@
}
cm.setChangeMessage(message.getMessage(), ctx.getWhen());
cm.setNotify(notify);
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
cm.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
diff --git a/java/com/google/gerrit/server/change/AddReviewersEmail.java b/java/com/google/gerrit/server/change/AddReviewersEmail.java
index 2778bdd..ae3851e 100644
--- a/java/com/google/gerrit/server/change/AddReviewersEmail.java
+++ b/java/com/google/gerrit/server/change/AddReviewersEmail.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.AddReviewerSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Collection;
@@ -37,13 +38,16 @@
private final AddReviewerSender.Factory addReviewerSenderFactory;
private final ExecutorService sendEmailsExecutor;
+ private final MessageIdGenerator messageIdGenerator;
@Inject
AddReviewersEmail(
AddReviewerSender.Factory addReviewerSenderFactory,
- @SendEmailExecutor ExecutorService sendEmailsExecutor) {
+ @SendEmailExecutor ExecutorService sendEmailsExecutor,
+ MessageIdGenerator messageIdGenerator) {
this.addReviewerSenderFactory = addReviewerSenderFactory;
this.sendEmailsExecutor = sendEmailsExecutor;
+ this.messageIdGenerator = messageIdGenerator;
}
public void emailReviewersAsync(
@@ -86,6 +90,9 @@
cm.addReviewersByEmail(immutableAddedByEmail);
cm.addExtraCC(immutableToCopy);
cm.addExtraCCByEmail(immutableCopiedByEmail);
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(
+ change.getProject(), change.currentPatchSetId()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index bbb94ea..467c4a2 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -59,6 +59,7 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.send.CreateChangeSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -108,6 +109,7 @@
private final RevisionCreated revisionCreated;
private final CommentAdded commentAdded;
private final ReviewerAdder reviewerAdder;
+ private final MessageIdGenerator messageIdGenerator;
private final Change.Id changeId;
private final PatchSet.Id psId;
@@ -156,6 +158,7 @@
CommentAdded commentAdded,
RevisionCreated revisionCreated,
ReviewerAdder reviewerAdder,
+ MessageIdGenerator messageIdGenerator,
@Assisted Change.Id changeId,
@Assisted ObjectId commitId,
@Assisted String refName) {
@@ -171,6 +174,7 @@
this.revisionCreated = revisionCreated;
this.commentAdded = commentAdded;
this.reviewerAdder = reviewerAdder;
+ this.messageIdGenerator = messageIdGenerator;
this.changeId = changeId;
this.psId = PatchSet.id(changeId, INITIAL_PATCH_SET_ID);
@@ -475,6 +479,8 @@
cm.addExtraCC(reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCs));
cm.addExtraCCByEmail(
reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCsByEmail));
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
cm.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log(
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
index 3bc9324..e52375f 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
@@ -21,6 +21,7 @@
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -36,6 +37,8 @@
}
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
+ private final MessageIdGenerator messageIdGenerator;
+
private final Address reviewer;
private ChangeMessage changeMessage;
@@ -43,8 +46,11 @@
@Inject
DeleteReviewerByEmailOp(
- DeleteReviewerSender.Factory deleteReviewerSenderFactory, @Assisted Address reviewer) {
+ DeleteReviewerSender.Factory deleteReviewerSenderFactory,
+ MessageIdGenerator messageIdGenerator,
+ @Assisted Address reviewer) {
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
+ this.messageIdGenerator = messageIdGenerator;
this.reviewer = reviewer;
}
@@ -79,6 +85,8 @@
cm.addReviewersByEmail(Collections.singleton(reviewer));
cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
cm.setNotify(notify);
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log("Cannot email update for change %s", change.getId());
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 099334d..3d43f59 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.extensions.events.ReviewerDeleted;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
@@ -45,6 +46,7 @@
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.update.RepoView;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
@@ -69,6 +71,7 @@
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
private final RemoveReviewerControl removeReviewerControl;
private final ProjectCache projectCache;
+ private final MessageIdGenerator messageIdGenerator;
private final AccountState reviewer;
private final DeleteReviewerInput input;
@@ -90,6 +93,7 @@
DeleteReviewerSender.Factory deleteReviewerSenderFactory,
RemoveReviewerControl removeReviewerControl,
ProjectCache projectCache,
+ MessageIdGenerator messageIdGenerator,
@Assisted AccountState reviewerAccount,
@Assisted DeleteReviewerInput input) {
this.approvalsUtil = approvalsUtil;
@@ -101,6 +105,7 @@
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
this.removeReviewerControl = removeReviewerControl;
this.projectCache = projectCache;
+ this.messageIdGenerator = messageIdGenerator;
this.reviewer = reviewerAccount;
this.input = input;
}
@@ -177,7 +182,7 @@
}
try {
if (notify.shouldNotify()) {
- emailReviewers(ctx.getProject(), currChange, changeMessage, notify);
+ emailReviewers(ctx.getProject(), currChange, changeMessage, notify, ctx.getRepoView());
}
} catch (Exception err) {
logger.atSevere().withCause(err).log("Cannot email update for change %s", currChange.getId());
@@ -211,8 +216,9 @@
Project.NameKey projectName,
Change change,
ChangeMessage changeMessage,
- NotifyResolver.Result notify)
- throws EmailException {
+ NotifyResolver.Result notify,
+ RepoView repoView)
+ throws EmailException, IOException {
Account.Id userId = user.get().getAccountId();
if (userId.equals(reviewer.account().id())) {
// The user knows they removed themselves, don't bother emailing them.
@@ -223,6 +229,7 @@
cm.addReviewers(Collections.singleton(reviewer.account().id()));
cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
cm.setNotify(notify);
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(repoView, change.currentPatchSetId()));
cm.send();
}
}
diff --git a/java/com/google/gerrit/server/change/EmailReviewComments.java b/java/com/google/gerrit/server/change/EmailReviewComments.java
index f7e45e7..f1eff6f 100644
--- a/java/com/google/gerrit/server/change/EmailReviewComments.java
+++ b/java/com/google/gerrit/server/change/EmailReviewComments.java
@@ -25,8 +25,10 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.CommentSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.update.RepoView;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -65,13 +67,15 @@
ChangeMessage message,
List<Comment> comments,
String patchSetComment,
- List<LabelVote> labels);
+ List<LabelVote> labels,
+ RepoView repoView);
}
private final ExecutorService sendEmailsExecutor;
private final PatchSetInfoFactory patchSetInfoFactory;
private final CommentSender.Factory commentSenderFactory;
private final ThreadLocalRequestContext requestContext;
+ private final MessageIdGenerator messageIdGenerator;
private final NotifyResolver.Result notify;
private final ChangeNotes notes;
@@ -81,6 +85,7 @@
private final List<Comment> comments;
private final String patchSetComment;
private final List<LabelVote> labels;
+ private final RepoView repoView;
@Inject
EmailReviewComments(
@@ -88,6 +93,7 @@
PatchSetInfoFactory patchSetInfoFactory,
CommentSender.Factory commentSenderFactory,
ThreadLocalRequestContext requestContext,
+ MessageIdGenerator messageIdGenerator,
@Assisted NotifyResolver.Result notify,
@Assisted ChangeNotes notes,
@Assisted PatchSet patchSet,
@@ -95,11 +101,13 @@
@Assisted ChangeMessage message,
@Assisted List<Comment> comments,
@Nullable @Assisted String patchSetComment,
- @Assisted List<LabelVote> labels) {
+ @Assisted List<LabelVote> labels,
+ @Assisted RepoView repoView) {
this.sendEmailsExecutor = executor;
this.patchSetInfoFactory = patchSetInfoFactory;
this.commentSenderFactory = commentSenderFactory;
this.requestContext = requestContext;
+ this.messageIdGenerator = messageIdGenerator;
this.notify = notify;
this.notes = notes;
this.patchSet = patchSet;
@@ -108,6 +116,7 @@
this.comments = COMMENT_ORDER.sortedCopy(comments);
this.patchSetComment = patchSetComment;
this.labels = labels;
+ this.repoView = repoView;
}
public void sendAsync() {
@@ -127,6 +136,7 @@
cm.setPatchSetComment(patchSetComment);
cm.setLabels(labels);
cm.setNotify(notify);
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(repoView, patchSet.id()));
cm.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email comments for %s", patchSet.id());
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 988d178..d4d74a3 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -79,6 +80,7 @@
private final ChangeMessagesUtil cmUtil;
private final PatchSetUtil psUtil;
private final WorkInProgressStateChanged wipStateChanged;
+ private final MessageIdGenerator messageIdGenerator;
// Assisted-injected fields.
private final PatchSet.Id psId;
@@ -120,6 +122,7 @@
RevisionCreated revisionCreated,
ProjectCache projectCache,
WorkInProgressStateChanged wipStateChanged,
+ MessageIdGenerator messageIdGenerator,
@Assisted ChangeNotes notes,
@Assisted PatchSet.Id psId,
@Assisted ObjectId commitId) {
@@ -133,6 +136,7 @@
this.revisionCreated = revisionCreated;
this.projectCache = projectCache;
this.wipStateChanged = wipStateChanged;
+ this.messageIdGenerator = messageIdGenerator;
this.origNotes = notes;
this.psId = psId;
@@ -291,6 +295,7 @@
cm.addReviewers(oldReviewers.byState(REVIEWER));
cm.addExtraCC(oldReviewers.byState(CC));
cm.setNotify(notify);
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(
diff --git a/java/com/google/gerrit/server/change/SetAssigneeOp.java b/java/com/google/gerrit/server/change/SetAssigneeOp.java
index 9848150..74536aa 100644
--- a/java/com/google/gerrit/server/change/SetAssigneeOp.java
+++ b/java/com/google/gerrit/server/change/SetAssigneeOp.java
@@ -24,6 +24,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.AssigneeChanged;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.SetAssigneeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.plugincontext.PluginSetContext;
@@ -50,6 +51,7 @@
private final SetAssigneeSender.Factory setAssigneeSenderFactory;
private final Provider<IdentifiedUser> user;
private final IdentifiedUser.GenericFactory userFactory;
+ private final MessageIdGenerator messageIdGenerator;
private Change change;
private IdentifiedUser oldAssignee;
@@ -62,6 +64,7 @@
SetAssigneeSender.Factory setAssigneeSenderFactory,
Provider<IdentifiedUser> user,
IdentifiedUser.GenericFactory userFactory,
+ MessageIdGenerator messageIdGenerator,
@Assisted IdentifiedUser newAssignee) {
this.cmUtil = cmUtil;
this.validationListeners = validationListeners;
@@ -69,6 +72,7 @@
this.setAssigneeSenderFactory = setAssigneeSenderFactory;
this.user = user;
this.userFactory = userFactory;
+ this.messageIdGenerator = messageIdGenerator;
this.newAssignee = requireNonNull(newAssignee, "assignee");
}
@@ -122,6 +126,8 @@
setAssigneeSenderFactory.create(
change.getProject(), change.getId(), newAssignee.getAccountId());
cm.setFrom(user.get().getAccountId());
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(
diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java
index 283cff8..f0ebb80 100644
--- a/java/com/google/gerrit/server/change/WorkInProgressOp.java
+++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java
@@ -20,6 +20,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.InputWithMessage;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -30,8 +31,10 @@
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.update.RepoView;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.io.IOException;
/* Set work in progress or ready for review state on a change */
public class WorkInProgressOp implements BatchUpdateOp {
@@ -131,6 +134,13 @@
|| !sendEmail) {
return;
}
+ RepoView repoView;
+ try {
+ repoView = ctx.getRepoView();
+ } catch (IOException ex) {
+ throw new StorageException(
+ String.format("Repository %s not found", ctx.getProject().get()), ex);
+ }
email
.create(
notify,
@@ -140,7 +150,8 @@
cmsg,
ImmutableList.of(),
cmsg.getMessage(),
- ImmutableList.of())
+ ImmutableList.of(),
+ repoView)
.sendAsync();
}
}
diff --git a/java/com/google/gerrit/server/config/SysExecutorModule.java b/java/com/google/gerrit/server/config/SysExecutorModule.java
index e7f4540..ea45b12 100644
--- a/java/com/google/gerrit/server/config/SysExecutorModule.java
+++ b/java/com/google/gerrit/server/config/SysExecutorModule.java
@@ -14,7 +14,11 @@
package com.google.gerrit.server.config;
+import static com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService;
+
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.gerrit.server.CacheRefreshExecutor;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.git.WorkQueue;
import com.google.inject.AbstractModule;
@@ -24,7 +28,7 @@
import org.eclipse.jgit.lib.Config;
/**
- * Module providing the {@link ReceiveCommitsExecutor}.
+ * Module providing different executors.
*
* <p>This module is intended to be installed at the top level when creating a {@code sysInjector}
* in {@code Daemon} or similar, not nested in another module. This ensures the module can be
@@ -37,7 +41,7 @@
@Provides
@Singleton
@ReceiveCommitsExecutor
- public ExecutorService createReceiveCommitsExecutor(
+ public ExecutorService provideReceiveCommitsExecutor(
@GerritServerConfig Config config, WorkQueue queues) {
int poolSize =
config.getInt(
@@ -48,11 +52,11 @@
@Provides
@Singleton
@SendEmailExecutor
- public ExecutorService createSendEmailExecutor(
+ public ExecutorService provideSendEmailExecutor(
@GerritServerConfig Config config, WorkQueue queues) {
int poolSize = config.getInt("sendemail", null, "threadPoolSize", 1);
if (poolSize == 0) {
- return MoreExecutors.newDirectExecutorService();
+ return newDirectExecutorService();
}
return queues.createQueue(poolSize, "SendEmail", true);
}
@@ -60,11 +64,24 @@
@Provides
@Singleton
@FanOutExecutor
- public ExecutorService createFanOutExecutor(@GerritServerConfig Config config, WorkQueue queues) {
+ public ExecutorService provideFanOutExecutor(
+ @GerritServerConfig Config config, WorkQueue queues) {
int poolSize = config.getInt("execution", null, "fanOutThreadPoolSize", 25);
if (poolSize == 0) {
- return MoreExecutors.newDirectExecutorService();
+ return newDirectExecutorService();
}
return queues.createQueue(poolSize, "FanOut");
}
+
+ @Provides
+ @Singleton
+ @CacheRefreshExecutor
+ public ListeningExecutorService provideCacheRefreshExecutor(
+ @GerritServerConfig Config config, WorkQueue queues) {
+ int poolSize = config.getInt("cache", null, "refreshThreadPoolSize", 2);
+ if (poolSize == 0) {
+ return newDirectExecutorService();
+ }
+ return MoreExecutors.listeningDecorator(queues.createQueue(poolSize, "CacheRefresh"));
+ }
}
diff --git a/java/com/google/gerrit/server/config/UrlFormatter.java b/java/com/google/gerrit/server/config/UrlFormatter.java
index 6b2510e..d3f90e5 100644
--- a/java/com/google/gerrit/server/config/UrlFormatter.java
+++ b/java/com/google/gerrit/server/config/UrlFormatter.java
@@ -47,6 +47,16 @@
return getWebUrl().map(url -> url + "c/" + project.get() + "/+/" + id.get());
}
+ /** Returns the URL for viewing the comment tab view of a change. */
+ default Optional<String> getCommentsTabView(Change change) {
+ return getChangeViewUrl(change.getProject(), change.getId()).map(url -> url + "?tab=comments");
+ }
+
+ /** Returns the URL for viewing the findings tab view of a change. */
+ default Optional<String> getFindingsTabView(Change change) {
+ return getChangeViewUrl(change.getProject(), change.getId()).map(url -> url + "?tab=findings");
+ }
+
/** Returns the URL for viewing a file in a given patch set of a change. */
default Optional<String> getPatchFileView(Change change, int patchsetId, String filename) {
return getChangeViewUrl(change.getProject(), change.getId())
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index df53133..329530c 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.extensions.events.ChangeReverted;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.RevertedSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -86,6 +87,7 @@
private final ChangeMessagesUtil cmUtil;
private final ChangeReverted changeReverted;
private final BatchUpdate.Factory updateFactory;
+ private final MessageIdGenerator messageIdGenerator;
@Inject
CommitUtil(
@@ -98,7 +100,8 @@
RevertedSender.Factory revertedSenderFactory,
ChangeMessagesUtil cmUtil,
ChangeReverted changeReverted,
- BatchUpdate.Factory updateFactory) {
+ BatchUpdate.Factory updateFactory,
+ MessageIdGenerator messageIdGenerator) {
this.repoManager = repoManager;
this.serverIdent = serverIdent;
this.seq = seq;
@@ -109,6 +112,7 @@
this.cmUtil = cmUtil;
this.changeReverted = changeReverted;
this.updateFactory = updateFactory;
+ this.messageIdGenerator = messageIdGenerator;
}
public static CommitInfo toCommitInfo(RevCommit commit) throws IOException {
@@ -308,6 +312,8 @@
RevertedSender cm = revertedSenderFactory.create(ctx.getProject(), change.getId());
cm.setFrom(ctx.getAccountId());
cm.setNotify(ctx.getNotify(change.getId()));
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(
diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java
index b272cba..89d6bd3 100644
--- a/java/com/google/gerrit/server/git/MergedByPushOp.java
+++ b/java/com/google/gerrit/server/git/MergedByPushOp.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.extensions.events.ChangeMerged;
import com.google.gerrit.server.mail.send.MergedSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -70,6 +71,7 @@
private final PatchSetUtil psUtil;
private final ExecutorService sendEmailExecutor;
private final ChangeMerged changeMerged;
+ private final MessageIdGenerator messageIdGenerator;
private final PatchSet.Id psId;
private final SubmissionId submissionId;
@@ -90,6 +92,7 @@
PatchSetUtil psUtil,
@SendEmailExecutor ExecutorService sendEmailExecutor,
ChangeMerged changeMerged,
+ MessageIdGenerator messageIdGenerator,
@Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
@Assisted SubmissionId submissionId,
@@ -101,6 +104,7 @@
this.psUtil = psUtil;
this.sendEmailExecutor = sendEmailExecutor;
this.changeMerged = changeMerged;
+ this.messageIdGenerator = messageIdGenerator;
this.requestScopePropagator = requestScopePropagator;
this.submissionId = submissionId;
this.psId = psId;
@@ -189,6 +193,8 @@
mergedSenderFactory.create(ctx.getProject(), psId.changeId());
cm.setFrom(ctx.getAccountId());
cm.setPatchSet(patchSet, info);
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
cm.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log(
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 0baecf5..f5c69e0 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -61,6 +61,7 @@
import com.google.gerrit.server.git.MergedByPushOp;
import com.google.gerrit.server.git.receive.ReceiveCommits.MagicBranchInput;
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -128,6 +129,8 @@
private final ReplacePatchSetSender.Factory replacePatchSetFactory;
private final ProjectCache projectCache;
private final ReviewerAdder reviewerAdder;
+ private final Change change;
+ private final MessageIdGenerator messageIdGenerator;
private final ProjectState projectState;
private final BranchNameKey dest;
@@ -140,7 +143,6 @@
private final PatchSetInfo info;
private final MagicBranchInput magicBranch;
private final PushCertificate pushCertificate;
- private final Change change;
private List<String> groups;
private final Map<String, Short> approvals = new HashMap<>();
@@ -172,6 +174,7 @@
@SendEmailExecutor ExecutorService sendEmailExecutor,
ReviewerAdder reviewerAdder,
Change change,
+ MessageIdGenerator messageIdGenerator,
@Assisted ProjectState projectState,
@Assisted BranchNameKey dest,
@Assisted boolean checkMergedInto,
@@ -197,6 +200,8 @@
this.projectCache = projectCache;
this.sendEmailExecutor = sendEmailExecutor;
this.reviewerAdder = reviewerAdder;
+ this.change = change;
+ this.messageIdGenerator = messageIdGenerator;
this.projectState = projectState;
this.dest = dest;
@@ -210,7 +215,6 @@
this.groups = groups;
this.magicBranch = magicBranch;
this.pushCertificate = pushCertificate;
- this.change = change;
}
@Override
@@ -533,6 +537,7 @@
oldRecipients.getCcOnly().stream(),
reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCs).stream())
.collect(toImmutableSet()));
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSetId));
// TODO(dborowitz): Support byEmail
cm.send();
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 3bb4770..0a99d83 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -31,116 +31,123 @@
/** Metadata that is provided to {@link PerformanceLogger}s as context for performance records. */
@AutoValue
public abstract class Metadata {
- // The numeric ID of an account.
+ /** The numeric ID of an account. */
public abstract Optional<Integer> accountId();
- // The type of an action (ACCOUNT_UPDATE, CHANGE_UPDATE, GROUP_UPDATE, INDEX_QUERY,
- // PLUGIN_UPDATE).
+ /**
+ * The type of an action (ACCOUNT_UPDATE, CHANGE_UPDATE, GROUP_UPDATE, INDEX_QUERY,
+ * PLUGIN_UPDATE).
+ */
public abstract Optional<String> actionType();
- // An authentication domain name.
+ /** An authentication domain name. */
public abstract Optional<String> authDomainName();
- // The name of a branch.
+ /** The name of a branch. */
public abstract Optional<String> branchName();
- // Key of an entity in a cache.
+ /** Key of an entity in a cache. */
public abstract Optional<String> cacheKey();
- // The name of a cache.
+ /** The name of a cache. */
public abstract Optional<String> cacheName();
- // The name of the implementation class.
+ /** The name of the implementation class. */
public abstract Optional<String> className();
- // The numeric ID of a change.
+ /** The numeric ID of a change. */
public abstract Optional<Integer> changeId();
- // The type of change ID which the user used to identify a change (e.g. numeric ID, triplet etc.).
+ /**
+ * The type of change ID which the user used to identify a change (e.g. numeric ID, triplet etc.).
+ */
public abstract Optional<String> changeIdType();
- // The cause of an error.
+ /** The cause of an error. */
public abstract Optional<String> cause();
- // The type of an event.
+ /** The type of an event. */
public abstract Optional<String> eventType();
- // The value of the @Export annotation which was used to register a plugin extension.
+ /** The value of the @Export annotation which was used to register a plugin extension. */
public abstract Optional<String> exportValue();
- // Path of a file in a repository.
+ /** Path of a file in a repository. */
public abstract Optional<String> filePath();
- // Garbage collector name.
+ /** Garbage collector name. */
public abstract Optional<String> garbageCollectorName();
- // Git operation (CLONE, FETCH).
+ /** Git operation (CLONE, FETCH). */
public abstract Optional<String> gitOperation();
- // The numeric ID of an internal group.
+ /** The numeric ID of an internal group. */
public abstract Optional<Integer> groupId();
- // The name of a group.
+ /** The name of a group. */
public abstract Optional<String> groupName();
- // The UUID of a group.
+ /** The UUID of a group. */
public abstract Optional<String> groupUuid();
- // HTTP status response code.
+ /** HTTP status response code. */
public abstract Optional<Integer> httpStatus();
- // The name of a secondary index.
+ /** The name of a secondary index. */
public abstract Optional<String> indexName();
- // The version of a secondary index.
+ /** The version of a secondary index. */
public abstract Optional<Integer> indexVersion();
- // The name of the implementation method.
+ /** The name of the implementation method. */
public abstract Optional<String> methodName();
- // One or more resources
+ /** One or more resources */
public abstract Optional<Boolean> multiple();
- // The name of an operation that is performed.
+ /** The name of an operation that is performed. */
public abstract Optional<String> operationName();
- // Partial or full computation
+ /** Partial or full computation */
public abstract Optional<Boolean> partial();
- // Path of a metadata file in NoteDb.
+ /** If a value is still current or not */
+ public abstract Optional<Boolean> outdated();
+
+ /** Path of a metadata file in NoteDb. */
public abstract Optional<String> noteDbFilePath();
- // Name of a metadata ref in NoteDb.
+ /** Name of a metadata ref in NoteDb. */
public abstract Optional<String> noteDbRefName();
- // Type of a sequence in NoteDb (ACCOUNTS, CHANGES, GROUPS).
+ /** Type of a sequence in NoteDb (ACCOUNTS, CHANGES, GROUPS). */
public abstract Optional<String> noteDbSequenceType();
- // The ID of a patch set.
+ /** The ID of a patch set. */
public abstract Optional<Integer> patchSetId();
- // Plugin metadata that doesn't fit into any other category.
+ /** Plugin metadata that doesn't fit into any other category. */
public abstract ImmutableList<PluginMetadata> pluginMetadata();
- // The name of a plugin.
+ /** The name of a plugin. */
public abstract Optional<String> pluginName();
- // The name of a Gerrit project (aka Git repository).
+ /** The name of a Gerrit project (aka Git repository). */
public abstract Optional<String> projectName();
- // The type of a Git push to Gerrit (CREATE_REPLACE, NORMAL, AUTOCLOSE).
+ /** The type of a Git push to Gerrit (CREATE_REPLACE, NORMAL, AUTOCLOSE). */
public abstract Optional<String> pushType();
- // The number of resources that is processed.
+ /** The number of resources that is processed. */
public abstract Optional<Integer> resourceCount();
- // The name of a REST view.
+ /** The name of a REST view. */
public abstract Optional<String> restViewName();
- // The SHA1 of Git commit.
+ /** The SHA1 of Git commit. */
public abstract Optional<String> revision();
- // The username of an account.
+ /** The username of an account. */
public abstract Optional<String> username();
/**
@@ -305,6 +312,8 @@
public abstract Builder partial(boolean partial);
+ public abstract Builder outdated(boolean outdated);
+
public abstract Builder noteDbFilePath(@Nullable String noteDbFilePath);
public abstract Builder noteDbRefName(@Nullable String noteDbRefName);
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 9c3dd02..f004e4b 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -57,6 +57,7 @@
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.mail.MailFilter;
import com.google.gerrit.server.mail.send.InboundEmailRejectionSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -115,6 +116,7 @@
private final AccountCache accountCache;
private final DynamicItem<UrlFormatter> urlFormatter;
private final PluginSetContext<CommentValidator> commentValidators;
+ private final MessageIdGenerator messageIdGenerator;
@Inject
public MailProcessor(
@@ -133,7 +135,8 @@
CommentAdded commentAdded,
AccountCache accountCache,
DynamicItem<UrlFormatter> urlFormatter,
- PluginSetContext<CommentValidator> commentValidators) {
+ PluginSetContext<CommentValidator> commentValidators,
+ MessageIdGenerator messageIdGenerator) {
this.emails = emails;
this.emailRejectionSender = emailRejectionSender;
this.retryHelper = retryHelper;
@@ -150,6 +153,7 @@
this.accountCache = accountCache;
this.urlFormatter = urlFormatter;
this.commentValidators = commentValidators;
+ this.messageIdGenerator = messageIdGenerator;
}
/**
@@ -222,6 +226,7 @@
try {
InboundEmailRejectionSender em =
emailRejectionSender.create(message.from(), message.id(), reason);
+ em.setMessageId(messageIdGenerator.fromMailMessage(message));
em.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot send email to warn for an error");
@@ -366,7 +371,8 @@
changeMessage,
comments,
patchSetComment,
- ImmutableList.of())
+ ImmutableList.of(),
+ ctx.getRepoView())
.sendAsync();
// Get previous approvals from this user
Map<String, Short> approvals = new HashMap<>();
diff --git a/java/com/google/gerrit/server/mail/send/AddKeySender.java b/java/com/google/gerrit/server/mail/send/AddKeySender.java
index 3b7b2aa..e02f02a 100644
--- a/java/com/google/gerrit/server/mail/send/AddKeySender.java
+++ b/java/com/google/gerrit/server/mail/send/AddKeySender.java
@@ -35,11 +35,16 @@
private final IdentifiedUser user;
private final AccountSshKey sshKey;
private final List<String> gpgKeys;
+ private final MessageIdGenerator messageIdGenerator;
@AssistedInject
public AddKeySender(
- EmailArguments args, @Assisted IdentifiedUser user, @Assisted AccountSshKey sshKey) {
+ EmailArguments args,
+ MessageIdGenerator messageIdGenerator,
+ @Assisted IdentifiedUser user,
+ @Assisted AccountSshKey sshKey) {
super(args, "addkey");
+ this.messageIdGenerator = messageIdGenerator;
this.user = user;
this.sshKey = sshKey;
this.gpgKeys = null;
@@ -47,8 +52,12 @@
@AssistedInject
public AddKeySender(
- EmailArguments args, @Assisted IdentifiedUser user, @Assisted List<String> gpgKeys) {
+ EmailArguments args,
+ MessageIdGenerator messageIdGenerator,
+ @Assisted IdentifiedUser user,
+ @Assisted List<String> gpgKeys) {
super(args, "addkey");
+ this.messageIdGenerator = messageIdGenerator;
this.user = user;
this.sshKey = null;
this.gpgKeys = gpgKeys;
@@ -58,6 +67,7 @@
protected void init() throws EmailException {
super.init();
setHeader("Subject", String.format("[Gerrit Code Review] New %s Keys Added", getKeyType()));
+ setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
add(RecipientType.TO, Address.create(getEmail()));
}
diff --git a/java/com/google/gerrit/server/mail/send/CommentSender.java b/java/com/google/gerrit/server/mail/send/CommentSender.java
index 48d342e..f3cccf2 100644
--- a/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -88,6 +88,16 @@
.orElse(null);
}
+ /** @return a web link to the comment tab view of a change. */
+ public String getCommentsTabLink() {
+ return args.urlFormatter.get().getCommentsTabView(change).orElse(null);
+ }
+
+ /** @return a web link to the findings tab view of a change. */
+ public String getFindingsTabLink() {
+ return args.urlFormatter.get().getFindingsTabView(change).orElse(null);
+ }
+
/**
* @return A title for the group, i.e. "Commit Message", "Merge List", or "File [[filename]]".
*/
@@ -96,6 +106,8 @@
return "Commit Message";
} else if (Patch.MERGE_LIST.equals(filename)) {
return "Merge List";
+ } else if (Patch.PATCHSET_LEVEL.equals(filename)) {
+ return "Patchset";
} else {
return "File " + filename;
}
@@ -379,7 +391,9 @@
for (CommentSender.FileCommentGroup group : getGroupedInlineComments(repo)) {
Map<String, Object> groupData = new HashMap<>();
- groupData.put("link", group.getFileLink());
+ if (!group.filename.equals(Patch.PATCHSET_LEVEL)) {
+ groupData.put("link", group.getFileLink());
+ }
groupData.put("title", group.getTitle());
groupData.put("patchSetId", group.patchSetId);
@@ -407,7 +421,14 @@
commentData.put("startLine", startLine);
// Set the comment link.
- if (comment.lineNbr == 0) {
+
+ if (comment.key.filename.equals(Patch.PATCHSET_LEVEL)) {
+ if (comment instanceof RobotComment) {
+ commentData.put("link", group.getFindingsTabLink());
+ } else {
+ commentData.put("link", group.getCommentsTabLink());
+ }
+ } else if (comment.lineNbr == 0) {
commentData.put("link", group.getFileLink());
} else {
commentData.put("link", group.getCommentLink(comment.side, startLine));
diff --git a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
index 3df7f05..ce336ff 100644
--- a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
+++ b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
@@ -38,11 +38,16 @@
private final IdentifiedUser user;
private final AccountSshKey sshKey;
private final List<String> gpgKeyFingerprints;
+ private final MessageIdGenerator messageIdGenerator;
@AssistedInject
public DeleteKeySender(
- EmailArguments args, @Assisted IdentifiedUser user, @Assisted AccountSshKey sshKey) {
+ EmailArguments args,
+ MessageIdGenerator messageIdGenerator,
+ @Assisted IdentifiedUser user,
+ @Assisted AccountSshKey sshKey) {
super(args, "deletekey");
+ this.messageIdGenerator = messageIdGenerator;
this.user = user;
this.gpgKeyFingerprints = Collections.emptyList();
this.sshKey = sshKey;
@@ -51,9 +56,11 @@
@AssistedInject
public DeleteKeySender(
EmailArguments args,
+ MessageIdGenerator messageIdGenerator,
@Assisted IdentifiedUser user,
@Assisted List<String> gpgKeyFingerprints) {
super(args, "deletekey");
+ this.messageIdGenerator = messageIdGenerator;
this.user = user;
this.gpgKeyFingerprints = gpgKeyFingerprints;
this.sshKey = null;
@@ -63,6 +70,7 @@
protected void init() throws EmailException {
super.init();
setHeader("Subject", String.format("[Gerrit Code Review] %s Keys Deleted", getKeyType()));
+ setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
add(RecipientType.TO, Address.create(getEmail()));
}
diff --git a/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java b/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
index cec2bb5..5b3b34c 100644
--- a/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
+++ b/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -29,11 +30,16 @@
private final IdentifiedUser user;
private final String operation;
+ private final MessageIdGenerator messageIdGenerator;
@AssistedInject
public HttpPasswordUpdateSender(
- EmailArguments args, @Assisted IdentifiedUser user, @Assisted String operation) {
+ EmailArguments args,
+ MessageIdGenerator messageIdGenerator,
+ @Assisted IdentifiedUser user,
+ @Assisted String operation) {
super(args, "HttpPasswordUpdate");
+ this.messageIdGenerator = messageIdGenerator;
this.user = user;
this.operation = operation;
}
@@ -42,6 +48,9 @@
protected void init() throws EmailException {
super.init();
setHeader("Subject", "[Gerrit Code Review] HTTP password was " + operation);
+ setMessageId(
+ messageIdGenerator.fromReasonAccountIdAndTimestamp(
+ "HTTP password change", user.getAccountId(), TimeUtil.nowTs()));
add(RecipientType.TO, Address.create(getEmail()));
}
diff --git a/java/com/google/gerrit/server/mail/send/MessageIdGenerator.java b/java/com/google/gerrit/server/mail/send/MessageIdGenerator.java
new file mode 100644
index 0000000..01e165d
--- /dev/null
+++ b/java/com/google/gerrit/server/mail/send/MessageIdGenerator.java
@@ -0,0 +1,126 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.send;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.mail.MailMessage;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.update.RepoView;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+
+/** A generator class that creates a {@link MessageId} */
+public class MessageIdGenerator {
+ private final GitRepositoryManager repositoryManager;
+ private final AllUsersName allUsersName;
+
+ @Inject
+ public MessageIdGenerator(GitRepositoryManager repositoryManager, AllUsersName allUsersName) {
+ this.repositoryManager = repositoryManager;
+ this.allUsersName = allUsersName;
+ }
+
+ /**
+ * A unique id used which is a part of the header of all emails sent through by Gerrit. All of the
+ * emails are sent via {@link OutgoingEmail#send()}.
+ */
+ @AutoValue
+ public abstract static class MessageId {
+ public abstract String id();
+ }
+
+ /**
+ * Create a {@link MessageId} as a result of a change update.
+ *
+ * @param repoView
+ * @param patchsetId
+ * @return MessageId that depends on the patchset.
+ */
+ public MessageId fromChangeUpdate(RepoView repoView, PatchSet.Id patchsetId) {
+ String metaRef = patchsetId.changeId().toRefPrefix() + "meta";
+ Optional<ObjectId> metaSha1;
+ try {
+ metaSha1 = repoView.getRef(metaRef);
+ } catch (IOException ex) {
+ throw new StorageException("unable to extract info for Message-Id", ex);
+ }
+ return metaSha1
+ .map(optional -> new AutoValue_MessageIdGenerator_MessageId(optional.getName()))
+ .orElseThrow(() -> new IllegalStateException(metaRef + " doesn't exist"));
+ }
+
+ public MessageId fromChangeUpdate(Project.NameKey project, PatchSet.Id patchsetId) {
+ String metaRef = patchsetId.changeId().toRefPrefix() + "meta";
+ Ref ref = getRef(metaRef, project);
+ checkState(ref != null, metaRef + " must exist");
+ return new AutoValue_MessageIdGenerator_MessageId(ref.getObjectId().getName());
+ }
+
+ /**
+ * @param accountId Create a {@link MessageId} as a result of an account update.
+ * @return MessageId that depends on the account id.
+ */
+ public MessageId fromAccountUpdate(Account.Id accountId) {
+ String userRef = RefNames.refsUsers(accountId);
+ Ref ref = getRef(userRef, allUsersName);
+ checkState(ref != null, userRef + " must exist");
+ return new AutoValue_MessageIdGenerator_MessageId(ref.getObjectId().getName());
+ }
+
+ /**
+ * Create a {@link MessageId} from a mail message.
+ *
+ * @param mailMessage The message that was sent but was rejected.
+ * @return MessageId that depends on the MailMessage that was rejected.
+ */
+ public MessageId fromMailMessage(MailMessage mailMessage) {
+ return new AutoValue_MessageIdGenerator_MessageId(mailMessage.id() + "-REJECTION");
+ }
+
+ /**
+ * Create a {@link MessageId} from a reason, Account.Id, and timestamp.
+ *
+ * @param reason for performing this account update
+ * @param accountId
+ * @param timestamp
+ * @return MessageId that depends on the reason, accountId, and timestamp.
+ */
+ public MessageId fromReasonAccountIdAndTimestamp(
+ String reason, Account.Id accountId, Timestamp timestamp) {
+ return new AutoValue_MessageIdGenerator_MessageId(
+ reason + "-" + accountId.toString() + "-" + timestamp.toString());
+ }
+
+ private Ref getRef(String userRef, Project.NameKey project) {
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ return repository.getRefDatabase().findRef(userRef);
+ } catch (IOException ex) {
+ throw new StorageException("unable to extract info for Message-Id", ex);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/mail/send/NewChangeSender.java b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
index 83c3a94..d81dca4 100644
--- a/java/com/google/gerrit/server/mail/send/NewChangeSender.java
+++ b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
@@ -57,7 +57,6 @@
super.init();
String threadId = getChangeMessageThreadId();
- setHeader("Message-ID", threadId);
setHeader("References", threadId);
switch (notify.handling()) {
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index b35bbec..fdd6b81 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -67,6 +67,7 @@
private Address smtpFromAddress;
private StringBuilder textBody;
private StringBuilder htmlBody;
+ private MessageIdGenerator.MessageId messageId;
protected Map<String, Object> soyContext;
protected Map<String, Object> soyContextEmailData;
protected List<String> footers;
@@ -88,6 +89,10 @@
this.notify = requireNonNull(notify);
}
+ public void setMessageId(MessageIdGenerator.MessageId messageId) {
+ this.messageId = messageId;
+ }
+
/**
* Format and enqueue the message for delivery.
*
@@ -108,6 +113,9 @@
}
init();
+ if (messageId == null) {
+ throw new IllegalStateException("All emails must have a messageId");
+ }
if (useHtml()) {
appendHtml(soyHtmlTemplate("HeaderHtml"));
}
@@ -201,31 +209,21 @@
va.htmlBody = null;
}
- for (OutgoingEmailValidationListener validator : args.outgoingEmailValidationListeners) {
- try {
- validator.validateOutgoingEmail(va);
- } catch (ValidationException e) {
- logger.atFine().log(
- "Not sending '%s': Rejected by outgoing email validator: %s",
- messageClass, e.getMessage());
- return;
- }
- }
-
Set<Address> intersection = Sets.intersection(va.smtpRcptTo, smtpRcptToPlaintextOnly);
if (!intersection.isEmpty()) {
logger.atSevere().log("Email '%s' will be sent twice to %s", messageClass, intersection);
}
-
if (!va.smtpRcptTo.isEmpty()) {
// Send multipart message
+ addMessageId(va, "-HTML");
+ if (!validateEmail(va)) return;
logger.atFine().log(
"Sending multipart '%s' from %s to %s",
messageClass, va.smtpFromAddress, va.smtpRcptTo);
args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
}
-
if (!smtpRcptToPlaintextOnly.isEmpty()) {
+ addMessageId(va, "-PLAIN");
// Send plaintext message
Map<String, EmailHeader> shallowCopy = new HashMap<>();
shallowCopy.putAll(headers);
@@ -238,6 +236,7 @@
to.add(a);
shallowCopy.put(FieldName.TO, to);
}
+ if (!validateEmail(va)) return;
logger.atFine().log(
"Sending plaintext '%s' from %s to %s",
messageClass, va.smtpFromAddress, smtpRcptToPlaintextOnly);
@@ -246,6 +245,26 @@
}
}
+ private boolean validateEmail(OutgoingEmailValidationListener.Args va) {
+ for (OutgoingEmailValidationListener validator : args.outgoingEmailValidationListeners) {
+ try {
+ validator.validateOutgoingEmail(va);
+ } catch (ValidationException e) {
+ logger.atFine().log(
+ "Not sending '%s': Rejected by outgoing email validator: %s",
+ messageClass, e.getMessage());
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private void addMessageId(OutgoingEmailValidationListener.Args va, String suffix) {
+ if (messageId != null) {
+ va.headers.put(FieldName.MESSAGE_ID, new EmailHeader.String(messageId.id() + suffix));
+ }
+ }
+
/** Format the message body by calling {@link #appendText(String)}. */
protected abstract void format() throws EmailException;
@@ -262,7 +281,6 @@
headers.put(FieldName.FROM, new EmailHeader.AddressList(smtpFromAddress));
headers.put(FieldName.TO, new EmailHeader.AddressList());
headers.put(FieldName.CC, new EmailHeader.AddressList());
- setHeader(FieldName.MESSAGE_ID, "");
setHeader(MailHeader.AUTO_SUBMITTED.fieldName(), "auto-generated");
for (RecipientType recipientType : notify.accounts().keySet()) {
diff --git a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
index 128e185..1ead03c 100644
--- a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
+++ b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
@@ -27,6 +27,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.function.Consumer;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -47,12 +48,16 @@
private static final String EMPTY_TREE_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
private static final String DRAFT_REFS_PREFIX = "refs/draft-comments";
- private static final int CHUNK_SIZE = 100; // log progress after deleting every CHUNK_SIZE refs
+
+ // Number of refs deleted at once in a batch ref-update.
+ // Log progress after deleting every CHUNK_SIZE refs
+ private static final int CHUNK_SIZE = 3000;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final int cleanupPercentage;
private Repository allUsersRepo;
+ private final Consumer<String> uiConsumer;
public interface Factory {
DeleteZombieCommentsRefs create(int cleanupPercentage);
@@ -63,9 +68,18 @@
AllUsersName allUsers,
GitRepositoryManager repoManager,
@Assisted Integer cleanupPercentage) {
+ this(allUsers, repoManager, cleanupPercentage, (msg) -> {});
+ }
+
+ public DeleteZombieCommentsRefs(
+ AllUsersName allUsers,
+ GitRepositoryManager repoManager,
+ Integer cleanupPercentage,
+ Consumer<String> uiConsumer) {
this.allUsers = allUsers;
this.repoManager = repoManager;
this.cleanupPercentage = (cleanupPercentage == null) ? 100 : cleanupPercentage;
+ this.uiConsumer = uiConsumer;
}
public void execute() throws IOException {
@@ -74,15 +88,17 @@
List<Ref> draftRefs = allUsersRepo.getRefDatabase().getRefsByPrefix(DRAFT_REFS_PREFIX);
List<Ref> zombieRefs = filterZombieRefs(draftRefs);
- logger.atInfo().log(
- "Found a total of %d zombie draft refs in %s repo.", zombieRefs.size(), allUsers.get());
+ logInfo(
+ String.format(
+ "Found a total of %d zombie draft refs in %s repo.",
+ zombieRefs.size(), allUsers.get()));
- logger.atInfo().log("Cleanup percentage = %d", cleanupPercentage);
+ logInfo(String.format("Cleanup percentage = %d", cleanupPercentage));
zombieRefs =
zombieRefs.stream()
.filter(ref -> Change.Id.fromAllUsersRef(ref.getName()).get() % 100 < cleanupPercentage)
.collect(toImmutableList());
- logger.atInfo().log("Number of zombie refs to be cleaned = %d", zombieRefs.size());
+ logInfo(String.format("Number of zombie refs to be cleaned = %d", zombieRefs.size()));
long zombieRefsCnt = zombieRefs.size();
long deletedRefsCnt = 0;
@@ -124,8 +140,15 @@
return allUsersRepo.parseCommit(ref.getObjectId()).getTree().getName().equals(EMPTY_TREE_ID);
}
+ private void logInfo(String message) {
+ logger.atInfo().log(message);
+ uiConsumer.accept(message);
+ }
+
private void logProgress(long deletedRefsCount, long allRefsCount, long elapsed) {
- logger.atInfo().log(
- "Deleted %d/%d zombie draft refs (%d seconds)\n", deletedRefsCount, allRefsCount, elapsed);
+ logInfo(
+ String.format(
+ "Deleted %d/%d zombie draft refs (%d seconds)",
+ deletedRefsCount, allRefsCount, elapsed));
}
}
diff --git a/java/com/google/gerrit/server/patch/DiffExecutorModule.java b/java/com/google/gerrit/server/patch/DiffExecutorModule.java
index eb6a280..b9e644f 100644
--- a/java/com/google/gerrit/server/patch/DiffExecutorModule.java
+++ b/java/com/google/gerrit/server/patch/DiffExecutorModule.java
@@ -31,7 +31,7 @@
@Provides
@Singleton
@DiffExecutor
- public ExecutorService createDiffExecutor() {
+ public ExecutorService provideDiffExecutor() {
return new LoggingContextAwareExecutorService(
Executors.newCachedThreadPool(
new ThreadFactoryBuilder().setNameFormat("Diff-%d").setDaemon(true).build()));
diff --git a/java/com/google/gerrit/server/project/ProjectCacheClock.java b/java/com/google/gerrit/server/project/ProjectCacheClock.java
deleted file mode 100644
index eb451fd..0000000
--- a/java/com/google/gerrit/server/project/ProjectCacheClock.java
+++ /dev/null
@@ -1,100 +0,0 @@
-// Copyright (C) 2012 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.project;
-
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
-import com.google.gerrit.extensions.events.LifecycleListener;
-import com.google.gerrit.server.config.ConfigUtil;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorService;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
-import org.eclipse.jgit.lib.Config;
-
-/** Ticks periodically to force refresh events for {@link ProjectCacheImpl}. */
-@Singleton
-public class ProjectCacheClock implements LifecycleListener {
- private final Config serverConfig;
-
- private final AtomicLong generation = new AtomicLong();
-
- private ScheduledExecutorService executor;
-
- @Inject
- public ProjectCacheClock(@GerritServerConfig Config serverConfig) {
- this.serverConfig = serverConfig;
- }
-
- @Override
- public void start() {
- long checkFrequencyMillis = checkFrequency(serverConfig);
-
- if (checkFrequencyMillis == Long.MAX_VALUE) {
- // Start with generation 1 (to avoid magic 0 below).
- // Do not begin background thread, disabling the clock.
- generation.set(1);
- } else if (10 < checkFrequencyMillis) {
- // Start with generation 1 (to avoid magic 0 below).
- generation.set(1);
- executor =
- new LoggingContextAwareScheduledExecutorService(
- Executors.newScheduledThreadPool(
- 1,
- new ThreadFactoryBuilder()
- .setNameFormat("ProjectCacheClock-%d")
- .setDaemon(true)
- .setPriority(Thread.MIN_PRIORITY)
- .build()));
- @SuppressWarnings("unused") // Runnable already handles errors
- Future<?> possiblyIgnoredError =
- executor.scheduleAtFixedRate(
- generation::incrementAndGet,
- checkFrequencyMillis,
- checkFrequencyMillis,
- TimeUnit.MILLISECONDS);
- } else {
- // Magic generation 0 triggers ProjectState to always
- // check on each needsRefresh() request we make to it.
- generation.set(0);
- }
- }
-
- @Override
- public void stop() {
- if (executor != null) {
- executor.shutdown();
- }
- }
-
- long read() {
- return generation.get();
- }
-
- private static long checkFrequency(Config serverConfig) {
- String freq = serverConfig.getString("cache", "projects", "checkFrequency");
- if (freq != null && ("disabled".equalsIgnoreCase(freq) || "off".equalsIgnoreCase(freq))) {
- return Long.MAX_VALUE;
- }
- return TimeUnit.MILLISECONDS.convert(
- ConfigUtil.getTimeUnit(
- serverConfig, "cache", "projects", "checkFrequency", 5, TimeUnit.MINUTES),
- TimeUnit.MINUTES);
- }
-}
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 9d09eeb..a5c07e5 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -24,16 +24,23 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.project.ProjectIndexer;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.metrics.Counter2;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
+import com.google.gerrit.server.CacheRefreshExecutor;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
@@ -48,6 +55,7 @@
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
+import java.time.Duration;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
@@ -55,6 +63,7 @@
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
/** Cache of project information, including access rights. */
@@ -70,7 +79,10 @@
return new CacheModule() {
@Override
protected void configure() {
- cache(CACHE_NAME, String.class, ProjectState.class).loader(Loader.class);
+ cache(CACHE_NAME, Project.NameKey.class, ProjectState.class)
+ .loader(Loader.class)
+ .refreshAfterWrite(Duration.ofMinutes(15))
+ .expireAfterWrite(Duration.ofHours(1));
cache(CACHE_LIST, ListKey.class, new TypeLiteral<ImmutableSortedSet<Project.NameKey>>() {})
.maximumWeight(1)
@@ -84,7 +96,6 @@
@Override
protected void configure() {
listener().to(ProjectCacheWarmer.class);
- listener().to(ProjectCacheClock.class);
}
});
}
@@ -93,10 +104,9 @@
private final AllProjectsName allProjectsName;
private final AllUsersName allUsersName;
- private final LoadingCache<String, ProjectState> byName;
+ private final LoadingCache<Project.NameKey, ProjectState> byName;
private final LoadingCache<ListKey, ImmutableSortedSet<Project.NameKey>> list;
private final Lock listLock;
- private final ProjectCacheClock clock;
private final Provider<ProjectIndexer> indexer;
private final Timer0 guessRelevantGroupsLatency;
@@ -104,9 +114,8 @@
ProjectCacheImpl(
final AllProjectsName allProjectsName,
final AllUsersName allUsersName,
- @Named(CACHE_NAME) LoadingCache<String, ProjectState> byName,
+ @Named(CACHE_NAME) LoadingCache<Project.NameKey, ProjectState> byName,
@Named(CACHE_LIST) LoadingCache<ListKey, ImmutableSortedSet<Project.NameKey>> list,
- ProjectCacheClock clock,
Provider<ProjectIndexer> indexer,
MetricMaker metricMaker) {
this.allProjectsName = allProjectsName;
@@ -114,7 +123,6 @@
this.byName = byName;
this.list = list;
this.listLock = new ReentrantLock(true /* fair */);
- this.clock = clock;
this.indexer = indexer;
this.guessRelevantGroupsLatency =
@@ -142,13 +150,8 @@
}
try {
- ProjectState state = byName.get(projectName.get());
- if (state != null && state.needsRefresh(clock.read())) {
- byName.invalidate(projectName.get());
- state = byName.get(projectName.get());
- }
- return Optional.of(state);
- } catch (Exception e) {
+ return Optional.of(byName.get(projectName));
+ } catch (ExecutionException e) {
if ((e.getCause() instanceof RepositoryNotFoundException)) {
logger.atFine().log("Cannot find project %s", projectName.get());
return Optional.empty();
@@ -167,7 +170,7 @@
public void evict(Project.NameKey p) {
if (p != null) {
logger.atFine().log("Evict project '%s'", p.get());
- byName.invalidate(p.get());
+ byName.invalidate(p);
}
indexer.get().index(p);
}
@@ -222,7 +225,7 @@
public Set<AccountGroup.UUID> guessRelevantGroupUUIDs() {
try (Timer0.Context ignored = guessRelevantGroupsLatency.start()) {
return all().stream()
- .map(n -> byName.getIfPresent(n.get()))
+ .map(n -> byName.getIfPresent(n))
.filter(Objects::nonNull)
.flatMap(p -> p.getConfig().getAllGroupUUIDs().stream())
// getAllGroupUUIDs shouldn't really return null UUIDs, but harden
@@ -245,41 +248,67 @@
}
}
- static class Loader extends CacheLoader<String, ProjectState> {
+ @Singleton
+ static class Loader extends CacheLoader<Project.NameKey, ProjectState> {
private final ProjectState.Factory projectStateFactory;
private final GitRepositoryManager mgr;
- private final ProjectCacheClock clock;
private final ProjectConfig.Factory projectConfigFactory;
+ private final ListeningExecutorService cacheRefreshExecutor;
+ private final Counter2<String, Boolean> refreshCounter;
@Inject
Loader(
ProjectState.Factory psf,
GitRepositoryManager g,
- ProjectCacheClock clock,
- ProjectConfig.Factory projectConfigFactory) {
+ ProjectConfig.Factory projectConfigFactory,
+ @CacheRefreshExecutor ListeningExecutorService cacheRefreshExecutor,
+ MetricMaker metricMaker) {
projectStateFactory = psf;
mgr = g;
- this.clock = clock;
this.projectConfigFactory = projectConfigFactory;
+ this.cacheRefreshExecutor = cacheRefreshExecutor;
+ refreshCounter =
+ metricMaker.newCounter(
+ "caches/refresh_count",
+ new Description("count").setRate(),
+ Field.ofString("cache", Metadata.Builder::className).build(),
+ Field.ofBoolean("outdated", Metadata.Builder::outdated).build());
}
@Override
- public ProjectState load(String projectName) throws Exception {
+ public ProjectState load(Project.NameKey key) throws Exception {
try (TraceTimer timer =
TraceContext.newTimer(
- "Loading project", Metadata.builder().projectName(projectName).build())) {
- long now = clock.read();
- Project.NameKey key = Project.nameKey(projectName);
+ "Loading project", Metadata.builder().projectName(key.get()).build())) {
try (Repository git = mgr.openRepository(key)) {
ProjectConfig cfg = projectConfigFactory.create(key);
cfg.load(key, git);
-
- ProjectState state = projectStateFactory.create(cfg);
- state.initLastCheck(now);
- return state;
+ return projectStateFactory.create(cfg);
}
}
}
+
+ @Override
+ public ListenableFuture<ProjectState> reload(Project.NameKey key, ProjectState oldState)
+ throws Exception {
+ try (TraceTimer timer =
+ TraceContext.newTimer(
+ "Reload project", Metadata.builder().projectName(key.get()).build())) {
+ try (Repository git = mgr.openRepository(key)) {
+ Ref configRef = git.exactRef(RefNames.REFS_CONFIG);
+ if (configRef != null
+ && configRef.getObjectId().equals(oldState.getConfig().getRevision())) {
+ refreshCounter.increment(CACHE_NAME, false);
+ return Futures.immediateFuture(oldState);
+ }
+ }
+
+ // Repository is not thread safe, so we have to open it on the thread that does the loading.
+ // Just invoke the loader on the other thread.
+ refreshCounter.increment(CACHE_NAME, true);
+ return cacheRefreshExecutor.submit(() -> load(key));
+ }
+ }
}
static class ListKey {
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index e52f344..efadcc8 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -32,7 +32,6 @@
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -59,7 +58,6 @@
import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
/**
@@ -86,9 +84,6 @@
private final long globalMaxObjectSizeLimit;
private final boolean inheritProjectMaxObjectSizeLimit;
- /** Last system time the configuration's revision was examined. */
- private volatile long lastCheckGeneration;
-
/** Local access sections, wrapped in SectionMatchers for faster evaluation. */
private volatile List<SectionMatcher> localAccessSections;
@@ -140,33 +135,6 @@
}
}
- void initLastCheck(long generation) {
- lastCheckGeneration = generation;
- }
-
- boolean needsRefresh(long generation) {
- if (generation <= 0) {
- return isRevisionOutOfDate();
- }
- if (lastCheckGeneration != generation) {
- lastCheckGeneration = generation;
- return isRevisionOutOfDate();
- }
- return false;
- }
-
- private boolean isRevisionOutOfDate() {
- try (Repository git = gitMgr.openRepository(getNameKey())) {
- Ref ref = git.getRefDatabase().exactRef(RefNames.REFS_CONFIG);
- if (ref == null || ref.getObjectId() == null) {
- return true;
- }
- return !ref.getObjectId().equals(config.getRevision());
- } catch (IOException gone) {
- return true;
- }
- }
-
/**
* @return cached computation of all global capabilities. This should only be invoked on the state
* from {@link ProjectCache#getAllProjects()}. Null on any other project.
diff --git a/java/com/google/gerrit/server/restapi/account/CreateEmail.java b/java/com/google/gerrit/server/restapi/account/CreateEmail.java
index fee5eab..89b931f 100644
--- a/java/com/google/gerrit/server/restapi/account/CreateEmail.java
+++ b/java/com/google/gerrit/server/restapi/account/CreateEmail.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.mail.send.RegisterNewEmailSender;
import com.google.gerrit.server.permissions.GlobalPermission;
@@ -80,6 +81,7 @@
private final RegisterNewEmailSender.Factory registerNewEmailFactory;
private final PutPreferred putPreferred;
private final OutgoingEmailValidator validator;
+ private final MessageIdGenerator messageIdGenerator;
private final boolean isDevMode;
@Inject
@@ -91,7 +93,8 @@
AccountManager accountManager,
RegisterNewEmailSender.Factory registerNewEmailFactory,
PutPreferred putPreferred,
- OutgoingEmailValidator validator) {
+ OutgoingEmailValidator validator,
+ MessageIdGenerator messageIdGenerator) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
@@ -100,6 +103,7 @@
this.putPreferred = putPreferred;
this.validator = validator;
this.isDevMode = authConfig.getAuthType() == DEVELOPMENT_BECOME_ANY_ACCOUNT;
+ this.messageIdGenerator = messageIdGenerator;
}
@Override
@@ -161,6 +165,7 @@
if (!sender.isAllowed()) {
throw new MethodNotAllowedException("Not allowed to add email address " + email);
}
+ sender.setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
sender.send();
info.pendingConfirmation = true;
} catch (EmailException | RuntimeException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 4c39763..bfe7177 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.change.VoteResource;
import com.google.gerrit.server.extensions.events.VoteDeleted;
import com.google.gerrit.server.mail.send.DeleteVoteSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
@@ -77,6 +78,7 @@
private final NotifyResolver notifyResolver;
private final RemoveReviewerControl removeReviewerControl;
private final ProjectCache projectCache;
+ private final MessageIdGenerator messageIdGenerator;
@Inject
DeleteVote(
@@ -89,7 +91,8 @@
DeleteVoteSender.Factory deleteVoteSenderFactory,
NotifyResolver notifyResolver,
RemoveReviewerControl removeReviewerControl,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ MessageIdGenerator messageIdGenerator) {
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil;
@@ -100,6 +103,7 @@
this.notifyResolver = notifyResolver;
this.removeReviewerControl = removeReviewerControl;
this.projectCache = projectCache;
+ this.messageIdGenerator = messageIdGenerator;
}
@Override
@@ -229,6 +233,8 @@
cm.setFrom(user.getAccountId());
cm.setChangeMessage(changeMessage.getMessage(), ctx.getWhen());
cm.setNotify(notify);
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
}
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 1e2e644..4ba1ab7 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -54,6 +54,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RobotComment;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -904,9 +905,23 @@
}
NotifyResolver.Result notify = ctx.getNotify(notes.getChangeId());
if (notify.shouldNotify()) {
- email
- .create(notify, notes, ps, user, message, comments, in.message, labelDelta)
- .sendAsync();
+ try {
+ email
+ .create(
+ notify,
+ notes,
+ ps,
+ user,
+ message,
+ comments,
+ in.message,
+ labelDelta,
+ ctx.getRepoView())
+ .sendAsync();
+ } catch (IOException ex) {
+ throw new StorageException(
+ String.format("Repository %s not found", ctx.getProject().get()), ex);
+ }
}
commentAdded.fire(
notes.getChange(),
diff --git a/java/com/google/gerrit/server/restapi/change/Restore.java b/java/com/google/gerrit/server/restapi/change/Restore.java
index a72192e..d1506b7 100644
--- a/java/com/google/gerrit/server/restapi/change/Restore.java
+++ b/java/com/google/gerrit/server/restapi/change/Restore.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.extensions.events.ChangeRestored;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.mail.send.RestoredSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -65,6 +66,7 @@
private final PatchSetUtil psUtil;
private final ChangeRestored changeRestored;
private final ProjectCache projectCache;
+ private final MessageIdGenerator messageIdGenerator;
@Inject
Restore(
@@ -74,7 +76,8 @@
ChangeMessagesUtil cmUtil,
PatchSetUtil psUtil,
ChangeRestored changeRestored,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ MessageIdGenerator messageIdGenerator) {
this.updateFactory = updateFactory;
this.restoredSenderFactory = restoredSenderFactory;
this.json = json;
@@ -82,6 +85,7 @@
this.psUtil = psUtil;
this.changeRestored = changeRestored;
this.projectCache = projectCache;
+ this.messageIdGenerator = messageIdGenerator;
}
@Override
@@ -149,6 +153,8 @@
ReplyToChangeSender cm = restoredSenderFactory.create(ctx.getProject(), change.getId());
cm.setFrom(ctx.getAccountId());
cm.setChangeMessage(message.getMessage(), ctx.getWhen());
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 88db66e..e2b898f 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -58,6 +58,7 @@
import com.google.gerrit.server.extensions.events.ChangeReverted;
import com.google.gerrit.server.git.CommitUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.RevertedSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.Sequences;
@@ -126,6 +127,7 @@
private final BatchUpdate.Factory updateFactory;
private final ChangeResource.Factory changeResourceFactory;
private final GetRelated getRelated;
+ private final MessageIdGenerator messageIdGenerator;
private CherryPickInput cherryPickInput;
private List<ChangeInfo> results;
@@ -154,7 +156,8 @@
NotifyResolver notifyResolver,
BatchUpdate.Factory updateFactory,
ChangeResource.Factory changeResourceFactory,
- GetRelated getRelated) {
+ GetRelated getRelated,
+ MessageIdGenerator messageIdGenerator) {
this.queryProvider = queryProvider;
this.user = user;
this.permissionBackend = permissionBackend;
@@ -175,6 +178,7 @@
this.updateFactory = updateFactory;
this.changeResourceFactory = changeResourceFactory;
this.getRelated = getRelated;
+ this.messageIdGenerator = messageIdGenerator;
results = new ArrayList<>();
cherryPickInput = null;
}
@@ -601,6 +605,8 @@
RevertedSender cm = revertedSenderFactory.create(ctx.getProject(), change.getId());
cm.setFrom(ctx.getAccountId());
cm.setNotify(ctx.getNotify(change.getId()));
+ cm.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()));
cm.send();
} catch (Exception err) {
logger.atSevere().withCause(err).log(
diff --git a/java/com/google/gerrit/server/schema/Schema_182.java b/java/com/google/gerrit/server/schema/Schema_182.java
index 3928b2b..a61a175 100644
--- a/java/com/google/gerrit/server/schema/Schema_182.java
+++ b/java/com/google/gerrit/server/schema/Schema_182.java
@@ -28,7 +28,8 @@
public void upgrade(Arguments args, UpdateUI ui) throws Exception {
AllUsersName allUsers = args.allUsers;
GitRepositoryManager gitRepoManager = args.repoManager;
- DeleteZombieCommentsRefs cleanup = new DeleteZombieCommentsRefs(allUsers, gitRepoManager, 100);
+ DeleteZombieCommentsRefs cleanup =
+ new DeleteZombieCommentsRefs(allUsers, gitRepoManager, 100, ui::message);
cleanup.execute();
}
}
diff --git a/java/com/google/gerrit/server/ssh/SshAddressesModule.java b/java/com/google/gerrit/server/ssh/SshAddressesModule.java
index 0a6bcac..1159e06 100644
--- a/java/com/google/gerrit/server/ssh/SshAddressesModule.java
+++ b/java/com/google/gerrit/server/ssh/SshAddressesModule.java
@@ -40,7 +40,7 @@
@Provides
@Singleton
@SshListenAddresses
- public List<SocketAddress> getListenAddresses(@GerritServerConfig Config cfg) {
+ public List<SocketAddress> provideListenAddresses(@GerritServerConfig Config cfg) {
List<SocketAddress> listen = Lists.newArrayListWithExpectedSize(2);
String[] want = cfg.getStringList("sshd", null, "listenaddress");
if (want == null || want.length == 0) {
@@ -71,7 +71,7 @@
@Provides
@Singleton
@SshAdvertisedAddresses
- List<String> getAdvertisedAddresses(
+ List<String> provideAdvertisedAddresses(
@GerritServerConfig Config cfg, @SshListenAddresses List<SocketAddress> listen) {
String[] want = cfg.getStringList("sshd", null, "advertisedaddress");
if (want.length > 0) {
diff --git a/java/com/google/gerrit/server/submit/EmailMerge.java b/java/com/google/gerrit/server/submit/EmailMerge.java
index c94d49e..c433ee6 100644
--- a/java/com/google/gerrit/server/submit/EmailMerge.java
+++ b/java/com/google/gerrit/server/submit/EmailMerge.java
@@ -24,6 +24,8 @@
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.MergedSender;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
+import com.google.gerrit.server.update.RepoView;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.Inject;
@@ -38,20 +40,23 @@
interface Factory {
EmailMerge create(
Project.NameKey project,
- Change.Id changeId,
+ Change change,
Account.Id submitter,
- NotifyResolver.Result notify);
+ NotifyResolver.Result notify,
+ RepoView repoView);
}
private final ExecutorService sendEmailsExecutor;
private final MergedSender.Factory mergedSenderFactory;
private final ThreadLocalRequestContext requestContext;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
+ private final MessageIdGenerator messageIdGenerator;
private final Project.NameKey project;
- private final Change.Id changeId;
+ private final Change change;
private final Account.Id submitter;
private final NotifyResolver.Result notify;
+ private final RepoView repoView;
@Inject
EmailMerge(
@@ -59,18 +64,22 @@
MergedSender.Factory mergedSenderFactory,
ThreadLocalRequestContext requestContext,
IdentifiedUser.GenericFactory identifiedUserFactory,
+ MessageIdGenerator messageIdGenerator,
@Assisted Project.NameKey project,
- @Assisted Change.Id changeId,
+ @Assisted Change change,
@Assisted @Nullable Account.Id submitter,
- @Assisted NotifyResolver.Result notify) {
+ @Assisted NotifyResolver.Result notify,
+ @Assisted RepoView repoView) {
this.sendEmailsExecutor = executor;
this.mergedSenderFactory = mergedSenderFactory;
this.requestContext = requestContext;
this.identifiedUserFactory = identifiedUserFactory;
+ this.messageIdGenerator = messageIdGenerator;
this.project = project;
- this.changeId = changeId;
+ this.change = change;
this.submitter = submitter;
this.notify = notify;
+ this.repoView = repoView;
}
void sendAsync() {
@@ -82,14 +91,15 @@
public void run() {
RequestContext old = requestContext.setContext(this);
try {
- MergedSender cm = mergedSenderFactory.create(project, changeId);
+ MergedSender cm = mergedSenderFactory.create(project, change.getId());
if (submitter != null) {
cm.setFrom(submitter);
}
cm.setNotify(notify);
+ cm.setMessageId(messageIdGenerator.fromChangeUpdate(repoView, change.currentPatchSetId()));
cm.send();
} catch (Exception e) {
- logger.atSevere().withCause(e).log("Cannot email merged notification for %s", changeId);
+ logger.atSevere().withCause(e).log("Cannot email merged notification for %s", change.getId());
} finally {
requestContext.setContext(old);
}
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index a4141be..3bd052f 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -500,7 +500,12 @@
// have failed fast in one of the other steps.
try {
args.mergedSenderFactory
- .create(ctx.getProject(), getId(), submitter.accountId(), ctx.getNotify(getId()))
+ .create(
+ ctx.getProject(),
+ toMerge.change(),
+ submitter.accountId(),
+ ctx.getNotify(getId()),
+ ctx.getRepoView())
.sendAsync();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId());
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 8800463..a682d33 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -15,10 +15,11 @@
package com.google.gerrit.testing;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService;
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
-import com.google.common.util.concurrent.MoreExecutors;
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
import com.google.gerrit.extensions.client.AuthType;
@@ -30,6 +31,7 @@
import com.google.gerrit.index.project.ProjectSchemaDefinitions;
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.server.CacheRefreshExecutor;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -209,7 +211,7 @@
@Singleton
@DiffExecutor
public ExecutorService createDiffExecutor() {
- return MoreExecutors.newDirectExecutorService();
+ return newDirectExecutorService();
}
});
install(new DefaultMemoryCacheModule());
@@ -277,7 +279,7 @@
@Singleton
@SendEmailExecutor
public ExecutorService createSendEmailExecutor() {
- return MoreExecutors.newDirectExecutorService();
+ return newDirectExecutorService();
}
@Provides
@@ -287,6 +289,13 @@
return queues.createQueue(2, "FanOut");
}
+ @Provides
+ @Singleton
+ @CacheRefreshExecutor
+ public ListeningExecutorService createCacheRefreshExecutor() {
+ return newDirectExecutorService();
+ }
+
private Module luceneIndexModule() {
return indexModule("com.google.gerrit.lucene.LuceneIndexModule");
}
diff --git a/javatests/com/google/gerrit/acceptance/OutgoingEmailIT.java b/javatests/com/google/gerrit/acceptance/OutgoingEmailIT.java
new file mode 100644
index 0000000..7cb5377
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/OutgoingEmailIT.java
@@ -0,0 +1,126 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.accounts.EmailInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.mail.Address;
+import com.google.gerrit.mail.EmailHeader;
+import com.google.gerrit.testing.FakeEmailSender;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class OutgoingEmailIT extends AbstractDaemonTest {
+
+ @Test
+ public void messageIdHeaderFromChangeUpdate() throws Exception {
+ Repository repository = repoManager.openRepository(project);
+ PushOneCommit.Result result = createChange();
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.reviewer = user.email();
+ gApi.changes().id(result.getChangeId()).addReviewer(addReviewerInput);
+ sender.clear();
+
+ gApi.changes().id(result.getChangeId()).abandon();
+ assertThat(getMessageId(sender))
+ .isEqualTo(
+ repository
+ .getRefDatabase()
+ .exactRef(result.getChange().getId().toRefPrefix() + "meta")
+ .getObjectId()
+ .getName()
+ + "-HTML");
+ sender.clear();
+
+ gApi.changes().id(result.getChangeId()).restore();
+ assertThat(getMessageId(sender))
+ .isEqualTo(
+ repository
+ .getRefDatabase()
+ .exactRef(result.getChange().getId().toRefPrefix() + "meta")
+ .getObjectId()
+ .getName()
+ + "-HTML");
+ }
+
+ @Test
+ @GerritConfig(
+ name = "auth.registerEmailPrivateKey",
+ value = "HsOc6l_2lhS9G7sE_RsnS7Z6GJjdRDX14co=")
+ public void messageIdHeaderFromAccountUpdate() throws Exception {
+ Repository allUsersRepo = repoManager.openRepository(allUsers);
+ String email = "new.email@example.com";
+ EmailInput input = new EmailInput();
+ input.email = email;
+ sender.clear();
+ gApi.accounts().self().addEmail(input);
+
+ assertThat(sender.getMessages()).hasSize(1);
+ FakeEmailSender.Message m = sender.getMessages().get(0);
+ assertThat(m.rcpt()).containsExactly(Address.create(email));
+
+ assertThat(getMessageId(sender))
+ .isEqualTo(
+ allUsersRepo
+ .getRefDatabase()
+ .exactRef(RefNames.refsUsers(admin.id()))
+ .getObjectId()
+ .getName()
+ + "-HTML");
+ }
+
+ @Test
+ public void messageIdHeaderFromPasswordUpdate() throws Exception {
+ sender.clear();
+ String newPassword = gApi.accounts().self().generateHttpPassword();
+ assertThat(getMessageId(sender)).contains("HTTP password change-" + admin.id().toString());
+ }
+
+ @Test
+ public void htmlAndPlainTextSuffixAddedToMessageId() throws Exception {
+ PushOneCommit.Result result = createChange();
+ GeneralPreferencesInfo generalPreferencesInfo = new GeneralPreferencesInfo();
+ generalPreferencesInfo.emailFormat = GeneralPreferencesInfo.EmailFormat.PLAINTEXT;
+ gApi.accounts().id(user.id().get()).setPreferences(generalPreferencesInfo);
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.reviewer = user.email();
+ gApi.changes().id(result.getChangeId()).addReviewer(addReviewerInput);
+ sender.clear();
+
+ gApi.changes().id(result.getChangeId()).current().review(ReviewInput.approve());
+ assertThat(getMessageId(sender)).contains("-PLAIN");
+ sender.clear();
+
+ generalPreferencesInfo.emailFormat = GeneralPreferencesInfo.EmailFormat.HTML_PLAINTEXT;
+ gApi.accounts().id(user.id().get()).setPreferences(generalPreferencesInfo);
+ sender.clear();
+
+ gApi.changes().id(result.getChangeId()).current().review(ReviewInput.reject());
+ assertThat(getMessageId(sender)).contains("-HTML");
+ }
+
+ private static String getMessageId(FakeEmailSender sender) {
+ return ((EmailHeader.String)
+ (Iterables.getOnlyElement(sender.getMessages()).headers().get("Message-ID")))
+ .getString();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/MessageIdGeneratorIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/MessageIdGeneratorIT.java
new file mode 100644
index 0000000..3228900
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/MessageIdGeneratorIT.java
@@ -0,0 +1,82 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.accounts;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.mail.Address;
+import com.google.gerrit.mail.MailMessage;
+import com.google.gerrit.server.mail.send.MessageIdGenerator;
+import com.google.gerrit.server.util.time.TimeUtil;
+import java.sql.Timestamp;
+import java.time.Instant;
+import javax.inject.Inject;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class MessageIdGeneratorIT extends AbstractDaemonTest {
+ @Inject private MessageIdGenerator messageIdGenerator;
+
+ @Test
+ public void fromAccountUpdate() throws Exception {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ String messageId = messageIdGenerator.fromAccountUpdate(admin.id()).id();
+ String sha1 =
+ repo.getRefDatabase().getRef(RefNames.refsUsers(admin.id())).getObjectId().getName();
+ assertThat(sha1).isEqualTo(messageId);
+ }
+ }
+
+ @Test
+ public void fromChangeUpdate() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ PushOneCommit.Result result = createChange();
+ PatchSet.Id patchsetId = result.getChange().currentPatchSet().id();
+ String messageId = messageIdGenerator.fromChangeUpdate(project, patchsetId).id();
+ String sha1 =
+ repo.getRefDatabase()
+ .getRef(String.format("%smeta", patchsetId.changeId().toRefPrefix()))
+ .getObjectId()
+ .getName();
+ assertThat(sha1).isEqualTo(messageId);
+ }
+ }
+
+ @Test
+ public void fromMailMessage() throws Exception {
+ String id = "unique-id";
+ MailMessage mailMessage =
+ MailMessage.builder()
+ .id(id)
+ .from(Address.create("email@email.com"))
+ .dateReceived(Instant.EPOCH)
+ .subject("subject")
+ .build();
+ assertThat(messageIdGenerator.fromMailMessage(mailMessage).id()).isEqualTo(id + "-REJECTION");
+ }
+
+ @Test
+ public void fromReasonAccountIdAndTimestamp() throws Exception {
+ String reason = "reason";
+ Timestamp timestamp = TimeUtil.nowTs();
+ assertThat(
+ messageIdGenerator.fromReasonAccountIdAndTimestamp(reason, admin.id(), timestamp).id())
+ .isEqualTo(reason + "-" + admin.id().toString() + "-" + timestamp.toString());
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 4fac821..e48c9d5 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -212,6 +212,20 @@
}
@Test
+ public void patchsetLevelCommentEmailNotification() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String ps1 = result.getCommit().name();
+
+ CommentInput comment = newCommentWithOnlyMandatoryFields(PATCHSET_LEVEL, "comment");
+ addComments(changeId, ps1, comment);
+
+ String emailBody = Iterables.getOnlyElement(email.getMessages()).body();
+ assertThat(emailBody).contains("Patchset");
+ assertThat(emailBody).doesNotContain("/PATCHSET_LEVEL");
+ }
+
+ @Test
public void patchsetLevelCommentCantHaveLine() throws Exception {
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index fc44822..4ae77da 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -40,6 +40,7 @@
import com.google.gerrit.extensions.validators.CommentForValidation;
import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.mail.EmailHeader;
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailProcessingUtil;
import com.google.gerrit.server.mail.receive.MailProcessor;
@@ -296,6 +297,10 @@
assertNotifyTo(user);
Message message = sender.nextMessage();
assertThat(message.body()).contains("rejected one or more comments");
+
+ // ensure the message header contains a valid message id.
+ assertThat(((EmailHeader.String) (message.headers().get("Message-ID"))).getString())
+ .isEqualTo("some id-REJECTION-HTML");
}
@Test
diff --git a/javatests/com/google/gerrit/server/cache/h2/BUILD b/javatests/com/google/gerrit/server/cache/h2/BUILD
index 98f1b0e..438990c 100644
--- a/javatests/com/google/gerrit/server/cache/h2/BUILD
+++ b/javatests/com/google/gerrit/server/cache/h2/BUILD
@@ -6,10 +6,12 @@
deps = [
"//java/com/google/gerrit/server/cache/h2",
"//java/com/google/gerrit/server/cache/serialize",
+ "//java/com/google/gerrit/server/util/time",
"//lib:guava",
"//lib:h2",
"//lib:junit",
"//lib/guice",
+ "//lib/mockito",
"//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
index 69c2799..fb3a0db 100644
--- a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
@@ -16,16 +16,27 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
import com.google.gerrit.server.cache.serialize.StringCacheSerializer;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.TypeLiteral;
+import java.time.Duration;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
+import javax.annotation.Nullable;
import org.junit.Test;
public class H2CacheTest {
@@ -38,23 +49,31 @@
}
private static H2CacheImpl<String, String> newH2CacheImpl(
- int id, Cache<String, ValueHolder<String>> mem, int version) {
- SqlStore<String, String> store =
- new SqlStore<>(
- "jdbc:h2:mem:Test_" + id,
- KEY_TYPE,
- StringCacheSerializer.INSTANCE,
- StringCacheSerializer.INSTANCE,
- version,
- 1 << 20,
- null);
+ SqlStore<String, String> store, Cache<String, ValueHolder<String>> mem) {
return new H2CacheImpl<>(MoreExecutors.directExecutor(), store, KEY_TYPE, mem);
}
+ private static SqlStore<String, String> newStore(
+ int id,
+ int version,
+ @Nullable Duration expireAfterWrite,
+ @Nullable Duration refreshAfterWrite) {
+ return new SqlStore<>(
+ "jdbc:h2:mem:Test_" + id,
+ KEY_TYPE,
+ StringCacheSerializer.INSTANCE,
+ StringCacheSerializer.INSTANCE,
+ version,
+ 1 << 20,
+ expireAfterWrite,
+ refreshAfterWrite);
+ }
+
@Test
public void get() throws ExecutionException {
Cache<String, ValueHolder<String>> mem = CacheBuilder.newBuilder().build();
- H2CacheImpl<String, String> impl = newH2CacheImpl(nextDbId(), mem, DEFAULT_VERSION);
+ H2CacheImpl<String, String> impl =
+ newH2CacheImpl(newStore(nextDbId(), DEFAULT_VERSION, null, null), mem);
assertThat(impl.getIfPresent("foo")).isNull();
@@ -94,11 +113,12 @@
}
@Test
- public void version() throws Exception {
+ public void version() {
int id = nextDbId();
- H2CacheImpl<String, String> oldImpl = newH2CacheImpl(id, disableMemCache(), DEFAULT_VERSION);
+ H2CacheImpl<String, String> oldImpl =
+ newH2CacheImpl(newStore(id, DEFAULT_VERSION, null, null), disableMemCache());
H2CacheImpl<String, String> newImpl =
- newH2CacheImpl(id, disableMemCache(), DEFAULT_VERSION + 1);
+ newH2CacheImpl(newStore(id, DEFAULT_VERSION + 1, null, null), disableMemCache());
assertThat(oldImpl.diskStats().space()).isEqualTo(0);
assertThat(newImpl.diskStats().space()).isEqualTo(0);
@@ -124,6 +144,57 @@
assertThat(oldImpl.getIfPresent("key")).isNull();
}
+ @Test
+ public void refreshAfterWrite_triggeredWhenConfigured() throws Exception {
+ SqlStore<String, String> store =
+ newStore(nextDbId(), DEFAULT_VERSION, null, Duration.ofMillis(10));
+
+ // This is the loader that we configure for the cache when calling .loader(...)
+ @SuppressWarnings("unchecked")
+ CacheLoader<String, String> baseLoader = (CacheLoader<String, String>) mock(CacheLoader.class);
+ resetLoaderAndAnswerLoadAndRefreshCalls(baseLoader);
+
+ // We wrap baseLoader just like H2CacheFactory is wrapping it. The wrapped version will call out
+ // to the store for refreshing values.
+ H2CacheImpl.Loader<String, String> wrappedLoader =
+ new H2CacheImpl.Loader<>(MoreExecutors.directExecutor(), store, baseLoader);
+ // memCache is the in-memory variant of the cache. Its loader is wrappedLoader which will call
+ // out to the store to save or delete cached values.
+ LoadingCache<String, ValueHolder<String>> memCache =
+ CacheBuilder.newBuilder().maximumSize(10).build(wrappedLoader);
+
+ // h2Cache puts it all together
+ H2CacheImpl<String, String> h2Cache = newH2CacheImpl(store, memCache);
+
+ // Initial load and cache retrieval do not trigger refresh
+ // This works because we use a directExecutor() for refreshes
+ TimeUtil.setCurrentMillisSupplier(() -> 0);
+ assertThat(h2Cache.get("foo")).isEqualTo("load:foo");
+ verify(baseLoader).load("foo");
+ assertThat(h2Cache.get("foo")).isEqualTo("load:foo");
+ verifyNoMoreInteractions(baseLoader);
+ resetLoaderAndAnswerLoadAndRefreshCalls(baseLoader);
+
+ // Load after refresh duration returns old value, triggers refresh and returns new value
+ TimeUtil.setCurrentMillisSupplier(() -> 11);
+ assertThat(h2Cache.get("foo")).isEqualTo("load:foo");
+ assertThat(h2Cache.get("foo")).isEqualTo("reload:foo");
+ verify(baseLoader).reload("foo", "load:foo");
+ verifyNoMoreInteractions(baseLoader);
+ resetLoaderAndAnswerLoadAndRefreshCalls(baseLoader);
+
+ // Refreshed value was persisted
+ memCache.invalidateAll(); // Invalidates only the memcache, not the store.
+ assertThat(h2Cache.getIfPresent("foo")).isEqualTo("reload:foo");
+ }
+
+ private static void resetLoaderAndAnswerLoadAndRefreshCalls(CacheLoader<String, String> loader)
+ throws Exception {
+ reset(loader);
+ when(loader.load("foo")).thenReturn("load:foo");
+ when(loader.reload("foo", "load:foo")).thenReturn(Futures.immediateFuture("reload:foo"));
+ }
+
private static <K, V> Cache<K, ValueHolder<V>> disableMemCache() {
return CacheBuilder.newBuilder().maximumSize(0).build();
}
diff --git a/plugins/replication b/plugins/replication
index 9ae2087..3854fe6 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 9ae20871646a0757979e3bef03aaf4e74af8ff73
+Subproject commit 3854fe670a93235840be4886415203f6ee22c5ad
diff --git a/polygerrit-ui/app/constants/constants.js b/polygerrit-ui/app/constants/constants.js
index 74ebebd..ff34442 100644
--- a/polygerrit-ui/app/constants/constants.js
+++ b/polygerrit-ui/app/constants/constants.js
@@ -21,6 +21,9 @@
*/
export const PrimaryTab = {
FILES: 'files',
+ /**
+ * When renaming this, the links in UrlFormatter must be updated.
+ */
COMMENT_THREADS: 'comments',
FINDINGS: 'findings',
};
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_html.js b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_html.js
index 4add1da..b322f38 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_html.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_html.js
@@ -86,14 +86,15 @@
href$="[[_computeNavLink(_query, _offset, -1, _changesPerPage)]]"
class$="[[_computePrevArrowClass(_offset)]]"
>
- <iron-icon icon="gr-icons:chevron-left"></iron-icon>
+ <iron-icon icon="gr-icons:chevron-left" aria-label="Older"> </iron-icon>
</a>
<a
id="nextArrow"
href$="[[_computeNavLink(_query, _offset, 1, _changesPerPage)]]"
class$="[[_computeNextArrowClass(_changes)]]"
>
- <iron-icon icon="gr-icons:chevron-right"></iron-icon>
+ <iron-icon icon="gr-icons:chevron-right" aria-label="Newer">
+ </iron-icon>
</a>
</nav>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.js
index 916a56f..200ffc9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.js
@@ -137,7 +137,6 @@
<gr-dropdown
id="moreActions"
link=""
- tabindex="0"
vertical-offset="32"
horizontal-align="right"
on-tap-item="_handleOverflowItemTap"
@@ -145,7 +144,8 @@
disabled-ids="[[_disabledMenuActions]]"
items="[[_menuActions]]"
>
- <iron-icon icon="gr-icons:more-vert"></iron-icon>
+ <iron-icon icon="gr-icons:more-vert" aria-labelledby="moreMessage">
+ </iron-icon>
<span id="moreMessage">More</span>
</gr-dropdown>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 12b5aad..d09c942 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -225,6 +225,11 @@
computed: '_computeShowDynamicColumns(_dynamicHeaderEndpoints, ' +
'_dynamicContentEndpoints, _dynamicSummaryEndpoints)',
},
+ _showPrependedDynamicColumns: {
+ type: Boolean,
+ computed: '_computeShowPrependedDynamicColumns(' +
+ '_dynamicPrependedHeaderEndpoints, _dynamicPrependedContentEndpoints)',
+ },
/** @type {Array<string>} */
_dynamicHeaderEndpoints: {
type: Array,
@@ -237,6 +242,14 @@
_dynamicSummaryEndpoints: {
type: Array,
},
+ /** @type {Array<string>} */
+ _dynamicPrependedHeaderEndpoints: {
+ type: Array,
+ },
+ /** @type {Array<string>} */
+ _dynamicPrependedContentEndpoints: {
+ type: Array,
+ },
};
}
@@ -295,18 +308,27 @@
attached() {
super.attached();
pluginLoader.awaitPluginsLoaded().then(() => {
- this._dynamicHeaderEndpoints = pluginEndpoints.getDynamicEndpoints(
- 'change-view-file-list-header');
- this._dynamicContentEndpoints = pluginEndpoints.getDynamicEndpoints(
- 'change-view-file-list-content');
- this._dynamicSummaryEndpoints = pluginEndpoints.getDynamicEndpoints(
- 'change-view-file-list-summary');
+ this._dynamicHeaderEndpoints = pluginEndpoints
+ .getDynamicEndpoints('change-view-file-list-header');
+ this._dynamicContentEndpoints = pluginEndpoints
+ .getDynamicEndpoints('change-view-file-list-content');
+ this._dynamicPrependedHeaderEndpoints = pluginEndpoints
+ .getDynamicEndpoints('change-view-file-list-header-prepend');
+ this._dynamicPrependedContentEndpoints = pluginEndpoints
+ .getDynamicEndpoints('change-view-file-list-content-prepend');
+ this._dynamicSummaryEndpoints = pluginEndpoints
+ .getDynamicEndpoints('change-view-file-list-summary');
if (this._dynamicHeaderEndpoints.length !==
this._dynamicContentEndpoints.length) {
console.warn(
'Different number of dynamic file-list header and content.');
}
+ if (this._dynamicPrependedHeaderEndpoints.length !==
+ this._dynamicPrependedContentEndpoints.length) {
+ console.warn(
+ 'Different number of dynamic file-list header and content.');
+ }
if (this._dynamicHeaderEndpoints.length !==
this._dynamicSummaryEndpoints.length) {
console.warn(
@@ -1432,11 +1454,23 @@
_computeShowDynamicColumns(
headerEndpoints, contentEndpoints, summaryEndpoints) {
return headerEndpoints && contentEndpoints && summaryEndpoints &&
+ headerEndpoints.length &&
headerEndpoints.length === contentEndpoints.length &&
headerEndpoints.length === summaryEndpoints.length;
}
/**
+ * Shows registered dynamic prepended columns iff the 'header', 'content'
+ * endpoints are registered the exact same number of times.
+ */
+ _computeShowPrependedDynamicColumns(
+ headerEndpoints, contentEndpoints) {
+ return headerEndpoints && contentEndpoints &&
+ headerEndpoints.length &&
+ headerEndpoints.length === contentEndpoints.length;
+ }
+
+ /**
* Returns true if none of the inline diffs have been expanded.
*
* @return {boolean}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js
index dab5f3a..ccf7f1a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js
@@ -297,10 +297,22 @@
</style>
<div id="container" on-click="_handleFileListClick">
<div class="header-row row">
+ <!-- endpoint: change-view-file-list-header-prepend -->
+ <template is="dom-if" if="[[_showPrependedDynamicColumns]]">
+ <template
+ is="dom-repeat"
+ items="[[_dynamicPrependedHeaderEndpoints]]"
+ as="headerEndpoint"
+ >
+ <gr-endpoint-decorator name$="[[headerEndpoint]]">
+ </gr-endpoint-decorator>
+ </template>
+ </template>
<div class="path">File</div>
<div class="comments">Comments</div>
<div class="sizeBars">Size</div>
<div class="header-stats">Delta</div>
+ <!-- endpoint: change-view-file-list-header -->
<template is="dom-if" if="[[_showDynamicColumns]]">
<template
is="dom-repeat"
@@ -332,6 +344,23 @@
data-file$="[[_computeFileRange(file)]]"
tabindex="-1"
>
+ <!-- endpoint: change-view-file-list-content-prepend -->
+ <template is="dom-if" if="[[_showPrependedDynamicColumns]]">
+ <template
+ is="dom-repeat"
+ items="[[_dynamicPrependedContentEndpoints]]"
+ as="contentEndpoint"
+ >
+ <gr-endpoint-decorator name="[[contentEndpoint]]">
+ <gr-endpoint-param name="changeNum" value="[[changeNum]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" value="[[patchRange]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="path" value="[[file.__path]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </template>
+ </template>
<!-- TODO: Remove data-url as it appears its not used -->
<span
data-url="[[_computeDiffURL(change, patchRange, file.__path, editMode)]]"
@@ -433,6 +462,7 @@
file.size_delta)]]
</span>
</div>
+ <!-- endpoint: change-view-file-list-content -->
<template is="dom-if" if="[[_showDynamicColumns]]">
<template
is="dom-repeat"
@@ -537,6 +567,7 @@
-[[_patchChange.deleted]]
</span>
</div>
+ <!-- endpoint: change-view-file-list-summary -->
<template is="dom-if" if="[[_showDynamicColumns]]">
<template
is="dom-repeat"
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.js b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.js
index 132cd16..c42c734 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.js
@@ -74,7 +74,9 @@
}
_computeGroups(includedIn, filterText) {
- if (!includedIn) { return []; }
+ if (!includedIn || filterText === undefined) {
+ return [];
+ }
const filter = item => !filterText.length ||
item.toLowerCase().indexOf(filterText.toLowerCase()) !== -1;
@@ -105,12 +107,6 @@
_computeLoadingClass(loaded) {
return loaded ? 'loading loaded' : 'loading';
}
-
- _onFilterChanged() {
- this.debounce('filter-change', () => {
- this._filterText = this.$.filterInput.bindValue;
- }, 100);
- }
}
customElements.define(GrIncludedInDialog.is, GrIncludedInDialog);
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_html.js b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_html.js
index f5948e8..2faac52 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_html.js
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_html.js
@@ -73,12 +73,15 @@
>Close</gr-button
>
</span>
- <iron-input placeholder="Filter" on-bind-value-changed="_onFilterChanged">
+ <iron-input
+ id="filterInput"
+ placeholder="Filter"
+ bind-value="{{_filterText}}"
+ >
<input
- id="filterInput"
is="iron-input"
placeholder="Filter"
- on-bind-value-changed="_onFilterChanged"
+ bind-value="{{_filterText}}"
/>
</iron-input>
</header>
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_test.html b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_test.html
index fe6119e..5d5b1fc 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog_test.html
@@ -81,5 +81,20 @@
{title: 'Tags', items: ['v2.0', 'v2.1']},
]);
});
+
+ test('_computeGroups with .bindValue', done => {
+ element.$.filterInput.bindValue = 'stable-3.2';
+ const includedIn = {branches: [], tags: []};
+ includedIn.branches.push('master', 'stable-3.2');
+
+ setTimeout(() => {
+ const filterText = element._filterText;
+ assert.deepEqual(element._computeGroups(includedIn, filterText), [
+ {title: 'Branches', items: ['stable-3.2']},
+ ]);
+
+ done();
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 3dda25b..8c9d2de 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -74,14 +74,6 @@
computed: '_computeAuthor(message)',
},
/**
- * Used in cleaner_changelog experiment
- *
- * @type {Array} - array of threads attached to the message
- */
- commentThreads: {
- type: Array,
- },
- /**
* TODO(taoalpha): remove once the change log experiment is launched
*
* @type {Object} - a map on file and comments on it
@@ -140,11 +132,11 @@
type: String,
computed:
'_computeMessageContentCollapsed(message.message, message.tag,' +
- ' commentThreads)',
+ ' message.commentThreads)',
},
_commentCountText: {
type: Number,
- computed: '_computeCommentCountText(comments, commentThreads,'
+ computed: '_computeCommentCountText(comments, message.commentThreads,'
+ ' _isCleanerLogExperimentEnabled)',
},
_loggedIn: {
@@ -273,7 +265,7 @@
_computeMessageContentCollapsed(content, tag, commentThreads) {
const summary =
this._computeMessageContent(content, tag, false);
- if (summary || !commentThreads) return;
+ if (summary || !commentThreads) return summary;
return this._patchsetCommentSummary(commentThreads);
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
index 6425232..25d0f65 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
@@ -258,8 +258,8 @@
<template is="dom-if" if="[[_isCleanerLogExperimentEnabled]]">
<gr-thread-list
change="[[change]]"
- hidden$="[[!commentThreads.length]]"
- threads="[[commentThreads]]"
+ hidden$="[[!message.commentThreads.length]]"
+ threads="[[message.commentThreads]]"
change-num="[[changeNum]]"
logged-in="[[_loggedIn]]"
hide-toggle-buttons
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index edcb7f6..332ca52 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -252,6 +252,8 @@
const original = 'Uploaded patch set 1.';
const tag = 'autogenerated:gerrit:newPatchSet';
let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, element._computeMessageContentCollapsed(
+ original, tag, []));
assert.equal(actual, original);
actual = element._computeMessageContent(original, tag, false);
assert.equal(actual, original);
@@ -263,6 +265,8 @@
const expected = 'Patch Set 26 was rebased';
let actual = element._computeMessageContent(original, tag, true);
assert.equal(actual, expected);
+ assert.equal(actual, element._computeMessageContentCollapsed(
+ original, tag, []));
actual = element._computeMessageContent(original, tag, false);
assert.equal(actual, expected);
});
@@ -273,6 +277,8 @@
const expected = 'This change is ready for review.';
let actual = element._computeMessageContent(original, tag, true);
assert.equal(actual, expected);
+ assert.equal(actual, element._computeMessageContentCollapsed(
+ original, tag, []));
actual = element._computeMessageContent(original, tag, false);
assert.equal(actual, expected);
});
@@ -414,8 +420,9 @@
path: '/PATCHSET_LEVEL',
rootId: 'e365b138_bed65caa',
}];
- assert.equal(element._patchsetCommentSummary(threads),
- 'testing the load');
+ assert.equal(element._computeMessageContentCollapsed(
+ '', undefined, threads), 'testing the load');
+ assert.equal(element._computeMessageContent('', undefined, false), '');
});
test('single patchset comment with reply', () => {
@@ -446,7 +453,9 @@
path: '/PATCHSET_LEVEL',
rootId: 'e365b138_bed65caa',
}];
- assert.equal(element._patchsetCommentSummary(threads), 'n');
+ assert.equal(element._computeMessageContentCollapsed(
+ '', undefined, threads), 'n');
+ assert.equal(element._computeMessageContent('', undefined, false), '');
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
index ccc4a76..9037152 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
@@ -275,8 +275,9 @@
* all messages and updates, aligns or massages some of the properties.
*/
_computeCombinedMessages(messages, reviewerUpdates, changeComments) {
- messages = messages || [];
- reviewerUpdates = reviewerUpdates || [];
+ const params = [messages, reviewerUpdates, changeComments];
+ if (params.some(o => o === undefined)) return [];
+
let mi = 0;
let ri = 0;
let combinedMessages = [];
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
index 83edab3..5fd64d2 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
@@ -72,8 +72,10 @@
<paper-toggle-button
class="showAllActivityToggle"
checked="{{_showAllActivity}}"
+ aria-labelledby="showAllEntriesLabel"
+ role="switch"
></paper-toggle-button>
- <div>
+ <div id="showAllEntriesLabel">
<span>Show all entries</span>
<span class="hiddenEntries" hidden$="[[_showAllActivity]]">
([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
@@ -111,7 +113,6 @@
<gr-message
change-num="[[changeNum]]"
message="[[message]]"
- comment-threads="[[message.commentThreads]]"
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-message-anchor-tap="_handleAnchorClick"
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.html
index 71f25b7..65229e1 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.html
@@ -302,11 +302,11 @@
const messageElements = getMessages();
// threads
assert.equal(
- messageElements[0].commentThreads.length,
+ messageElements[0].message.commentThreads.length,
3);
// first thread contains 1 comment
assert.equal(
- messageElements[0].commentThreads[0].comments.length,
+ messageElements[0].message.commentThreads[0].comments.length,
1);
});
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
index ddae150..5adfc53 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
@@ -79,8 +79,10 @@
<paper-toggle-button
id="automatedMessageToggle"
checked="{{_hideAutomated}}"
+ aria-labelledby="onlyCommentsLabel"
+ role="switch"
></paper-toggle-button
- >Only comments
+ ><span id="onlyCommentsLabel">Only comments</span>
<span class="transparent separator"></span>
</span>
<gr-button
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.js
index 721e7510..162a7c0 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.js
@@ -57,20 +57,16 @@
<template is="dom-if" if="[[!hideToggleButtons]]">
<div class="header">
<div class="toggleItem">
- <paper-toggle-button
- id="unresolvedToggle"
- checked="{{_unresolvedOnly}}"
- ></paper-toggle-button>
- Only unresolved threads
+ <paper-toggle-button id="unresolvedToggle" checked="{{_unresolvedOnly}}"
+ >Only unresolved threads</paper-toggle-button
+ >
</div>
<div
class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]"
>
- <paper-toggle-button
- id="draftToggle"
- checked="{{_draftsOnly}}"
- ></paper-toggle-button>
- Only threads with drafts
+ <paper-toggle-button id="draftToggle" checked="{{_draftsOnly}}"
+ >Only threads with drafts</paper-toggle-button
+ >
</div>
</div>
</template>
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.js
index 1d4b440..cc11cfa 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.js
@@ -51,7 +51,7 @@
}
}
</style>
- <span class="patchRange">
+ <span class="patchRange" aria-label="patch range starts with">
<gr-dropdown-list
id="basePatchDropdown"
value="[[basePatchNum]]"
@@ -67,8 +67,8 @@
>
</template>
</span>
- <span class="arrow">→</span>
- <span class="patchRange">
+ <span aria-hidden="true" class="arrow">→</span>
+ <span class="patchRange" aria-label="patch range ends with">
<gr-dropdown-list
id="patchNumDropdown"
value="[[patchNum]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.js b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.js
index 96e0160..ee19374 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.js
@@ -92,7 +92,6 @@
link=""
hidden$="[[!removable]]"
hidden=""
- tabindex="-1"
aria-label="Remove"
class$="remove [[_getBackgroundClass(transparentBackground)]]"
on-click="_handleRemoveTap"
diff --git a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.js b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.js
index 17e7f49..ce2dc9e 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.js
@@ -40,7 +40,7 @@
}
</style>
<span>
- <a href$="[[_computeOwnerLink(account)]]" tabindex="-1">
+ <a href$="[[_computeOwnerLink(account)]]">
<gr-account-label
show-attention="[[showAttention]]"
hide-avatar="[[hideAvatar]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index 6ef4807..bdac50f 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -65,6 +65,11 @@
value: false,
reflectToAttribute: true,
},
+ ariaDisabled: {
+ type: Boolean,
+ computed: '_computeDisabled(disabled, loading)',
+ reflectToAttribute: true,
+ },
_disabled: {
type: Boolean,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
index 153b98d..6697880 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
@@ -347,6 +347,10 @@
return collapsed ? 'gr-icons:expand-more' : 'gr-icons:expand-less';
}
+ _computeShowHideAriaLabel(collapsed) {
+ return collapsed ? 'Expand' : 'Collapse';
+ }
+
_calculateActionstoShow(showActions, isRobotComment) {
// Polymer 2: check for undefined
if ([showActions, isRobotComment].some(arg => arg === undefined)) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.js
index 3d27042..b83cdf2 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.js
@@ -277,7 +277,10 @@
></gr-date-formatter>
</span>
<div class="show-hide">
- <label class="show-hide">
+ <label
+ class="show-hide"
+ aria-label="[[_computeShowHideAriaLabel(collapsed)]]"
+ >
<input
type="checkbox"
class="show-hide"
diff --git a/polygerrit-ui/app/samples/extra-column-on-file-list.js b/polygerrit-ui/app/samples/extra-column-on-file-list.js
new file mode 100644
index 0000000..2e37c01
--- /dev/null
+++ b/polygerrit-ui/app/samples/extra-column-on-file-list.js
@@ -0,0 +1,77 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/**
+ * This plugin will an extra column to file list on change page to show
+ * the first character of the path.
+ */
+
+// Header of this extra column
+class ColumnHeader extends Polymer.Element {
+ static get is() { return 'column-header'; }
+
+ static get template() {
+ return Polymer.html`
+ <style>
+ :host {
+ display: block;
+ padding-right: var(--spacing-m);
+ min-width: 5em;
+ }
+ </style>
+ <div>First Char</div>
+ `;
+ }
+}
+
+customElements.define(ColumnHeader.is, ColumnHeader);
+
+// Content of this extra column
+class ColumnContent extends Polymer.Element {
+ static get is() { return 'column-content'; }
+
+ static get properties() {
+ return {
+ path: String,
+ };
+ }
+
+ static get template() {
+ return Polymer.html`
+ <style>
+ :host {
+ display:block;
+ padding-right: var(--spacing-m);
+ min-width: 5em;
+ }
+ </style>
+ <div>[[getStatus(path)]]</div>
+ `;
+ }
+
+ getStatus(path) {
+ return path.charAt(0);
+ }
+}
+
+customElements.define(ColumnContent.is, ColumnContent);
+
+Gerrit.install(plugin => {
+ plugin.registerDynamicCustomComponent(
+ 'change-view-file-list-header-prepend', ColumnHeader.is);
+ plugin.registerDynamicCustomComponent(
+ 'change-view-file-list-content-prepend', ColumnContent.is);
+});
\ No newline at end of file
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index 120aff5..c44493d 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -61,6 +61,7 @@
dirListingMux := http.NewServeMux()
dirListingMux.Handle("/styles/", http.StripPrefix("/styles/", http.FileServer(http.Dir("app/styles"))))
+ dirListingMux.Handle("/samples/", http.StripPrefix("/samples/", http.FileServer(http.Dir("app/samples"))))
dirListingMux.Handle("/elements/", http.StripPrefix("/elements/", http.FileServer(http.Dir("app/elements"))))
dirListingMux.Handle("/behaviors/", http.StripPrefix("/behaviors/", http.FileServer(http.Dir("app/behaviors"))))
diff --git a/tools/bzl/BUILD b/tools/bzl/BUILD
index a0f5bd1..7febbac 100644
--- a/tools/bzl/BUILD
+++ b/tools/bzl/BUILD
@@ -3,3 +3,8 @@
"test_empty.sh",
"test_license.sh",
])
+
+sh_test(
+ name = "always_pass_test",
+ srcs = ["always_pass_test.sh"],
+)
diff --git a/tools/bzl/always_pass_test.sh b/tools/bzl/always_pass_test.sh
new file mode 100755
index 0000000..15c58ca
--- /dev/null
+++ b/tools/bzl/always_pass_test.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+#
+# Copyright (C) 2020 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This is a dummy test to put on the command line to avoid no tests
+# found outcome in `bazel test` command. See this upstream issue:
+# https://github.com/bazelbuild/bazel/issues/11465
+
+exit 0
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index eced448..c37ce12 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -23,8 +23,8 @@
maven_jar(
name = "dropwizard-core",
- artifact = "io.dropwizard.metrics:metrics-core:4.1.8",
- sha1 = "f4765fc53af5d5712261a7e29afe97beb6b30118",
+ artifact = "io.dropwizard.metrics:metrics-core:4.1.9",
+ sha1 = "dd76a62b007ffea9e6aba10f64c04173ef65f895",
)
SSHD_VERS = "2.4.0"
