Merge branch 'stable-3.0' into stable-3.1
* stable-3.0:
Set PerThreadCache as readonly after creating a new patch-set
Set PerThreadCache as readonly when formatting change e-mails
Set PerThreadCache as readonly when formatting change JSON
Set PerThreadCache as readonly after deleting a change
Set PerThreadCache as readonly after abandoning a change
Set PerThreadCache as readonly after merging a change
Set PerThreadCache as readonly after posting review comments
Introduce unloaders on PerThreadCache entries
RepoRefCache: Hold a reference to the refDatabase with ref counting
Remove use of RefCache in ChangeNotes
Cache change /meta ref SHA1 for each change indexing task
Also adapt TestRepositoryWithRefCounting by overriding the
getIdentifier() method to adapt to the new Repository interface
definition in JGit.
Release-Notes: skip
Change-Id: Ic7a9772f1624513607ed9a255591466763851490
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 8b7f9a0..ede488c 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -109,6 +109,7 @@
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.RepoRefCache;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -188,6 +189,7 @@
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
+import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.rules.TemporaryFolder;
@@ -200,6 +202,7 @@
public abstract class AbstractDaemonTest {
private static GerritServer commonServer;
private static Description firstTest;
+ private static String perThreadCacheCheckStaleEntry;
@ClassRule public static TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -302,6 +305,12 @@
private String systemTimeZone;
private SystemReader oldSystemReader;
+ @BeforeClass
+ public static void enablePerThreadCacheStalenessCheck() {
+ perThreadCacheCheckStaleEntry =
+ System.setProperty(RepoRefCache.REPO_REF_CACHE_CHECK_STALE_ENTRIES_PROPERTY, "true");
+ }
+
@Before
public void clearSender() {
if (sender != null) {
@@ -349,6 +358,16 @@
}
}
+ @AfterClass
+ public static void disablePerThreadCacheStalenessCheck() {
+ if (perThreadCacheCheckStaleEntry == null) {
+ System.clearProperty(RepoRefCache.REPO_REF_CACHE_CHECK_STALE_ENTRIES_PROPERTY);
+ } else {
+ System.setProperty(
+ RepoRefCache.REPO_REF_CACHE_CHECK_STALE_ENTRIES_PROPERTY, perThreadCacheCheckStaleEntry);
+ }
+ }
+
/** Controls which project and branches should be reset after each test case. */
protected ProjectResetter.Config resetProjects() {
return new ProjectResetter.Config()
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 03769a2..805afc2 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -40,8 +40,8 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
-import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
import com.google.gerrit.server.index.AutoFlush;
+import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
import com.google.gerrit.server.ssh.NoSshModule;
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.gerrit.server.util.SocketUtil;
@@ -415,7 +415,8 @@
"accountPatchReviewDb", null, "url", JdbcAccountPatchReviewStore.TEST_IN_MEMORY_URL);
daemon.setEnableHttpd(desc.httpd());
daemon.setLuceneModule(
- LuceneIndexModule.singleVersionAllLatest(0, ReplicaUtil.isReplica(baseConfig), AutoFlush.ENABLED));
+ LuceneIndexModule.singleVersionAllLatest(
+ 0, ReplicaUtil.isReplica(baseConfig), AutoFlush.ENABLED));
daemon.setDatabaseForTesting(
ImmutableList.of(
new InMemoryTestingDatabaseModule(cfg, site, inMemoryRepoManager),
diff --git a/java/com/google/gerrit/server/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
index 609fce7..9b04561 100644
--- a/java/com/google/gerrit/server/cache/PerThreadCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -17,12 +17,19 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.base.Objects;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.git.RefCache;
+import java.util.Collection;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Optional;
+import java.util.function.Consumer;
import java.util.function.Supplier;
+import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
/**
@@ -60,10 +67,51 @@
private static final int PER_THREAD_CACHE_SIZE = 25;
/**
- * Optional HTTP request associated with the per-thread cache, should the thread be associated
- * with the incoming HTTP thread pool.
+ * System property for disabling caching specific key types. TODO: DO NOT MERGE into stable-3.2
+ * onwards.
*/
- private final Optional<HttpServletRequest> httpRequest;
+ public static final String PER_THREAD_CACHE_DISABLED_TYPES_PROPERTY =
+ "PerThreadCache_disabledTypes";
+
+ /**
+ * True when the current thread is associated with an incoming API request that is not changing
+ * any repository /meta refs and therefore caching repo refs is safe. TODO: DO NOT MERGE into
+ * stable-3.2 onwards.
+ */
+ private boolean allowRefCache;
+
+ /**
+ * Sets the request status flag to read-only temporarily. TODO: DO NOT MERGE into stable-3.2
+ * onwards.
+ */
+ public interface ReadonlyRequestWindow extends AutoCloseable {
+
+ /**
+ * Close the request read-only status, restoring the previous value.
+ *
+ * <p>NOTE: If the previous status was not read-only, the cache is getting cleared for making
+ * sure that all potential stale entries coming from a read-only windows are cleared.
+ */
+ @Override
+ default void close() {}
+ }
+
+ private class ReadonlyRequestWindowImpl implements ReadonlyRequestWindow {
+ private final boolean oldAllowRepoRefsCache;
+
+ private ReadonlyRequestWindowImpl() {
+ oldAllowRepoRefsCache = allowRefCache();
+ allowRefCache(true);
+ }
+
+ @Override
+ public void close() {
+ allowRefCache(oldAllowRepoRefsCache);
+ }
+ }
+
+ private final Map<Key<?>, Consumer<Object>> unloaders =
+ Maps.newHashMapWithExpectedSize(PER_THREAD_CACHE_SIZE);
/**
* Unique key for key-value mappings stored in PerThreadCache. The key is based on the value's
@@ -110,9 +158,29 @@
}
}
+ /**
+ * Creates a thread-local cache associated to an incoming HTTP request.
+ *
+ * <p>The request is considered as read-only if the associated method is GET or HEAD.
+ *
+ * @param httpRequest HTTP request associated with the thread-local cache
+ * @return thread-local cache
+ */
public static PerThreadCache create(@Nullable HttpServletRequest httpRequest) {
checkState(CACHE.get() == null, "called create() twice on the same request");
- PerThreadCache cache = new PerThreadCache(httpRequest);
+ PerThreadCache cache = new PerThreadCache(httpRequest, false);
+ CACHE.set(cache);
+ return cache;
+ }
+
+ /**
+ * Creates a thread-local cache associated to an incoming read-only request.
+ *
+ * @return thread-local cache
+ */
+ public static PerThreadCache createReadOnly() {
+ checkState(CACHE.get() == null, "called create() twice on the same request");
+ PerThreadCache cache = new PerThreadCache(null, true);
CACHE.set(cache);
return cache;
}
@@ -122,48 +190,159 @@
return CACHE.get();
}
+ /**
+ * Return a cached value associated with a key fetched with a loader and released with an unloader
+ * function.
+ *
+ * @param <T> The data type of the cached value
+ * @param key the key associated with the value
+ * @param loader the loader function for fetching the value from the key
+ * @param unloader the unloader function for releasing the value when unloaded from the cache
+ * @return Optional of the cached value or empty if the value could not be cached for any reason
+ * (e.g. cache full)
+ */
+ public static <T> Optional<T> get(Key<T> key, Supplier<T> loader, Consumer<T> unloader) {
+ return Optional.ofNullable(get()).flatMap(c -> c.getWithLoader(key, loader, unloader));
+ }
+
+ /**
+ * Legacy way for retrieving a cached element through a loader.
+ *
+ * <p>This method is deprecated because it was error-prone due to the unclear ownership of the
+ * objects created through the loader. When the cache has space available, the entries are loaded
+ * and cached, hence owned and reused by the cache.
+ *
+ * <p>When the cache is full, this method just short-circuit to the invocation of the loader and
+ * the objects created aren't owned or stored by the cache, leaving the space for potential memory
+ * and resources leaks.
+ *
+ * <p>Because of the unclear semantics of the method (who owns the instances? are they reused?)
+ * this is now deprecated the the caller should use instead the {@link PerThreadCache#get(Key,
+ * Supplier, Consumer)} which has a clear ownership policy.
+ *
+ * @deprecated use {@link PerThreadCache#get(Key, Supplier, Consumer)}
+ */
public static <T> T getOrCompute(Key<T> key, Supplier<T> loader) {
PerThreadCache cache = get();
return cache != null ? cache.get(key, loader) : loader.get();
}
private final Map<Key<?>, Object> cache = Maps.newHashMapWithExpectedSize(PER_THREAD_CACHE_SIZE);
+ private final ImmutableSet<String> disabledTypes;
- private PerThreadCache(@Nullable HttpServletRequest req) {
- httpRequest = Optional.ofNullable(req);
+ private PerThreadCache(@Nullable HttpServletRequest req, boolean alwaysCacheRepoRefs) {
+ disabledTypes =
+ ImmutableSet.copyOf(
+ Splitter.on(',')
+ .split(System.getProperty(PER_THREAD_CACHE_DISABLED_TYPES_PROPERTY, "")));
+
+ allowRefCache =
+ alwaysCacheRepoRefs
+ || (req != null
+ && (req.getMethod().equalsIgnoreCase("GET")
+ || req.getMethod().equalsIgnoreCase("HEAD")));
}
/**
- * Returns an instance of {@code T} that was either loaded from the cache or obtained from the
- * provided {@link Supplier}.
+ * Legacy way of retrieving an instance of {@code T} that was either loaded from the cache or
+ * obtained from the provided {@link Supplier}.
+ *
+ * <p>This method is deprecated because it was error-prone due to the unclear ownership of the
+ * objects created through the loader. When the cache has space available, the entries are loaded
+ * and cached, hence owned and reused by the cache.
+ *
+ * <p>When the cache is full, this method just short-circuit to the invocation of the loader and
+ * the objects created aren't owned or stored by the cache, leaving the space for potential memory
+ * and resources leaks.
+ *
+ * <p>Because of the unclear semantics of the method (who owns the instances? are they reused?)
+ * this is now deprecated the the caller should use instead the {@link PerThreadCache#get(Key,
+ * Supplier, Consumer)} which has a clear ownership policy.
+ *
+ * @deprecated use {@link PerThreadCache#getWithLoader(Key, Supplier, Consumer)}
*/
public <T> T get(Key<T> key, Supplier<T> loader) {
- @SuppressWarnings("unchecked")
+ return getWithLoader(key, loader, null).orElse(loader.get());
+ }
+
+ @SuppressWarnings("unchecked")
+ public <T> Optional<T> getWithLoader(
+ Key<T> key, Supplier<T> loader, @Nullable Consumer<T> unloader) {
+ if (disabledTypes.contains(key.clazz.getCanonicalName())) {
+ return Optional.empty();
+ }
+
T value = (T) cache.get(key);
- if (value == null) {
+ if (value == null && cache.size() < PER_THREAD_CACHE_SIZE) {
value = loader.get();
- if (cache.size() < PER_THREAD_CACHE_SIZE) {
- cache.put(key, value);
+ cache.put(key, value);
+ if (unloader != null) {
+ unloaders.put(key, (Consumer<Object>) unloader);
}
}
- return value;
+ return Optional.ofNullable(value);
}
- /** Returns the optional HTTP request associated with the local thread cache. */
- public Optional<HttpServletRequest> getHttpRequest() {
- return httpRequest;
+ /** Returns an instance of {@code T} that is already loaded from the cache or null otherwise. */
+ @SuppressWarnings("unchecked")
+ public <T> T get(Key<T> key) {
+ return (T) cache.get(key);
}
- /** Returns true if there is an HTTP request associated and is a GET or HEAD */
- public boolean hasReadonlyRequest() {
- return httpRequest
- .map(HttpServletRequest::getMethod)
- .filter(m -> m.equalsIgnoreCase("GET") || m.equalsIgnoreCase("HEAD"))
- .isPresent();
+ /**
+ * Returns true if the associated request is read-only and therefore the repo refs are safe to be
+ * cached
+ */
+ public boolean allowRefCache() {
+ return allowRefCache;
+ }
+
+ /**
+ * Set the cache read-only request status temporarily, for enabling caching of all entries.
+ *
+ * @return {@link ReadonlyRequestWindow} associated with the incoming request
+ */
+ public static ReadonlyRequestWindow openReadonlyRequestWindow() {
+ PerThreadCache perThreadCache = CACHE.get();
+ return perThreadCache == null
+ ? new ReadonlyRequestWindow() {}
+ : perThreadCache.new ReadonlyRequestWindowImpl();
}
@Override
public void close() {
+ unload(unloaders.entrySet());
CACHE.remove();
}
+
+ private void unload(Collection<Entry<Key<?>, Consumer<Object>>> entriesToUnload) {
+ ImmutableSet<Entry<Key<?>, Consumer<Object>>> toUnload = ImmutableSet.copyOf(entriesToUnload);
+ try {
+ toUnload.stream().forEach(this::unload);
+ } finally {
+ toUnload.stream()
+ .forEach(
+ e -> {
+ cache.remove(e.getKey());
+ unloaders.remove(e.getKey());
+ });
+ }
+ }
+
+ private <T> void unload(Entry<Key<?>, Consumer<Object>> unloaderEntry) {
+ Object valueToUnload = cache.get(unloaderEntry.getKey());
+ unloaderEntry.getValue().accept(valueToUnload);
+ cache.remove(unloaderEntry.getKey());
+ }
+
+ private void allowRefCache(boolean allowed) {
+ allowRefCache = allowed;
+
+ if (!allowRefCache) {
+ unload(
+ unloaders.entrySet().stream()
+ .filter(e -> RefCache.class.isAssignableFrom(e.getKey().clazz))
+ .collect(Collectors.toSet()));
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 3b7a2c4..27e5347 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -85,6 +85,8 @@
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountInfoComparator;
import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.index.change.ChangeField;
@@ -490,7 +492,8 @@
private <I extends ChangeInfo> I toChangeInfo(
ChangeData cd, Optional<PatchSet.Id> limitToPsId, Supplier<I> changeInfoSupplier)
throws PatchListNotAvailableException, GpgException, PermissionBackendException, IOException {
- try (Timer0.Context ignored = metrics.toChangeInfoLatency.start()) {
+ try (Timer0.Context ignored = metrics.toChangeInfoLatency.start();
+ ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
return toChangeInfoImpl(cd, limitToPsId, changeInfoSupplier);
}
}
diff --git a/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
index 18b6a5e..1769f7d 100644
--- a/java/com/google/gerrit/server/events/StreamEventsApiListener.java
+++ b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@@ -47,6 +47,8 @@
import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ApprovalAttribute;
import com.google.gerrit.server.data.ChangeAttribute;
@@ -273,14 +275,18 @@
@Override
public void onRevisionCreated(RevisionCreatedListener.Event ev) {
try {
- ChangeNotes notes = getNotes(ev.getChange());
- Change change = notes.getChange();
- PatchSet patchSet = getPatchSet(notes, ev.getRevision());
- PatchSetCreatedEvent event = new PatchSetCreatedEvent(change);
+ Change change;
+ PatchSetCreatedEvent event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ ChangeNotes notes = getNotes(ev.getChange());
+ change = notes.getChange();
+ PatchSet patchSet = getPatchSet(notes, ev.getRevision());
+ event = new PatchSetCreatedEvent(change);
- event.change = changeAttributeSupplier(change, notes);
- event.patchSet = patchSetAttributeSupplier(change, patchSet);
- event.uploader = accountAttributeSupplier(ev.getWho());
+ event.change = changeAttributeSupplier(change, notes);
+ event.patchSet = patchSetAttributeSupplier(change, patchSet);
+ event.uploader = accountAttributeSupplier(ev.getWho());
+ }
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -374,17 +380,24 @@
@Override
public void onCommentAdded(CommentAddedListener.Event ev) {
- try {
- ChangeNotes notes = getNotes(ev.getChange());
- Change change = notes.getChange();
- PatchSet ps = getPatchSet(notes, ev.getRevision());
- CommentAddedEvent event = new CommentAddedEvent(change);
+ Change change;
- event.change = changeAttributeSupplier(change, notes);
- event.author = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, ps);
- event.comment = ev.getComment();
- event.approvals = approvalsAttributeSupplier(change, ev.getApprovals(), ev.getOldApprovals());
+ try {
+ CommentAddedEvent event;
+
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ ChangeNotes notes = getNotes(ev.getChange());
+ change = notes.getChange();
+ PatchSet ps = getPatchSet(notes, ev.getRevision());
+ event = new CommentAddedEvent(change);
+
+ event.change = changeAttributeSupplier(change, notes);
+ event.author = accountAttributeSupplier(ev.getWho());
+ event.patchSet = patchSetAttributeSupplier(change, ps);
+ event.comment = ev.getComment();
+ event.approvals =
+ approvalsAttributeSupplier(change, ev.getApprovals(), ev.getOldApprovals());
+ }
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -413,14 +426,18 @@
@Override
public void onChangeMerged(ChangeMergedListener.Event ev) {
try {
- ChangeNotes notes = getNotes(ev.getChange());
- Change change = notes.getChange();
- ChangeMergedEvent event = new ChangeMergedEvent(change);
+ ChangeMergedEvent event;
+ Change change;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ ChangeNotes notes = getNotes(ev.getChange());
+ change = notes.getChange();
+ event = new ChangeMergedEvent(change);
- event.change = changeAttributeSupplier(change, notes);
- event.submitter = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
- event.newRev = ev.getNewRevisionId();
+ event.change = changeAttributeSupplier(change, notes);
+ event.submitter = accountAttributeSupplier(ev.getWho());
+ event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.newRev = ev.getNewRevisionId();
+ }
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -431,14 +448,18 @@
@Override
public void onChangeAbandoned(ChangeAbandonedListener.Event ev) {
try {
- ChangeNotes notes = getNotes(ev.getChange());
- Change change = notes.getChange();
- ChangeAbandonedEvent event = new ChangeAbandonedEvent(change);
+ ChangeAbandonedEvent event;
+ Change change;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ ChangeNotes notes = getNotes(ev.getChange());
+ change = notes.getChange();
+ event = new ChangeAbandonedEvent(change);
- event.change = changeAttributeSupplier(change, notes);
- event.abandoner = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
- event.reason = ev.getReason();
+ event.change = changeAttributeSupplier(change, notes);
+ event.abandoner = accountAttributeSupplier(ev.getWho());
+ event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.reason = ev.getReason();
+ }
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -505,12 +526,17 @@
@Override
public void onChangeDeleted(ChangeDeletedListener.Event ev) {
try {
- ChangeNotes notes = getNotes(ev.getChange());
- Change change = notes.getChange();
- ChangeDeletedEvent event = new ChangeDeletedEvent(change);
+ Change change;
+ ChangeDeletedEvent event;
- event.change = changeAttributeSupplier(change, notes);
- event.deleter = accountAttributeSupplier(ev.getWho());
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ ChangeNotes notes = getNotes(ev.getChange());
+ change = notes.getChange();
+ event = new ChangeDeletedEvent(change);
+
+ event.change = changeAttributeSupplier(change, notes);
+ event.deleter = accountAttributeSupplier(ev.getWho());
+ }
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java b/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
index b27ffb9..ccf58cf 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
@@ -25,6 +25,8 @@
import com.google.gerrit.extensions.events.ChangeAbandonedListener;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -58,14 +60,17 @@
return;
}
try {
- Event event =
- new Event(
- util.changeInfo(change),
- util.revisionInfo(change.getProject(), ps),
- util.accountInfo(abandoner),
- reason,
- when,
- notifyHandling);
+ Event event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ event =
+ new Event(
+ util.changeInfo(change),
+ util.revisionInfo(change.getProject(), ps),
+ util.accountInfo(abandoner),
+ reason,
+ when,
+ notifyHandling);
+ }
listeners.runEach(l -> l.onChangeAbandoned(event));
} catch (PatchListObjectTooLargeException e) {
logger.atWarning().log("Couldn't fire event: %s", e.getMessage());
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java b/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
index df71f27..a24873c 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
@@ -22,6 +22,8 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.events.ChangeDeletedListener;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -45,7 +47,10 @@
return;
}
try {
- Event event = new Event(util.changeInfo(change), util.accountInfo(deleter), when);
+ Event event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ event = new Event(util.changeInfo(change), util.accountInfo(deleter), when);
+ }
listeners.runEach(l -> l.onChangeDeleted(event));
} catch (StorageException e) {
logger.atSevere().withCause(e).log("Couldn't fire event");
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeMerged.java b/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
index add1c51..5ce38ff 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
@@ -25,6 +25,8 @@
import com.google.gerrit.extensions.events.ChangeMergedListener;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -52,14 +54,18 @@
if (listeners.isEmpty()) {
return;
}
+
try {
- Event event =
- new Event(
- util.changeInfo(change),
- util.revisionInfo(change.getProject(), ps),
- util.accountInfo(merger),
- newRevisionId,
- when);
+ Event event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ event =
+ new Event(
+ util.changeInfo(change),
+ util.revisionInfo(change.getProject(), ps),
+ util.accountInfo(merger),
+ newRevisionId,
+ when);
+ }
listeners.runEach(l -> l.onChangeMerged(event));
} catch (PatchListObjectTooLargeException e) {
logger.atWarning().log("Couldn't fire event: %s", e.getMessage());
diff --git a/java/com/google/gerrit/server/extensions/events/CommentAdded.java b/java/com/google/gerrit/server/extensions/events/CommentAdded.java
index e45b206..e72b4d4 100644
--- a/java/com/google/gerrit/server/extensions/events/CommentAdded.java
+++ b/java/com/google/gerrit/server/extensions/events/CommentAdded.java
@@ -26,6 +26,8 @@
import com.google.gerrit.extensions.events.CommentAddedListener;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -61,15 +63,18 @@
return;
}
try {
- Event event =
- new Event(
- util.changeInfo(change),
- util.revisionInfo(change.getProject(), ps),
- util.accountInfo(author),
- comment,
- util.approvals(author, approvals, when),
- util.approvals(author, oldApprovals, when),
- when);
+ Event event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ event =
+ new Event(
+ util.changeInfo(change),
+ util.revisionInfo(change.getProject(), ps),
+ util.accountInfo(author),
+ comment,
+ util.approvals(author, approvals, when),
+ util.approvals(author, oldApprovals, when),
+ when);
+ }
listeners.runEach(l -> l.onCommentAdded(event));
} catch (PatchListObjectTooLargeException e) {
logger.atWarning().log("Couldn't fire event: %s", e.getMessage());
diff --git a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
index bdfa8c1..50fbaa1 100644
--- a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
+++ b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
@@ -25,6 +25,8 @@
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
@@ -74,13 +76,16 @@
return;
}
try {
- Event event =
- new Event(
- util.changeInfo(change),
- util.revisionInfo(change.getProject(), patchSet),
- util.accountInfo(uploader),
- when,
- notify.handling());
+ Event event;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ event =
+ new Event(
+ util.changeInfo(change),
+ util.revisionInfo(change.getProject(), patchSet),
+ util.accountInfo(uploader),
+ when,
+ notify.handling());
+ }
listeners.runEach(l -> l.onRevisionCreated(event));
} catch (PatchListObjectTooLargeException e) {
logger.atWarning().log("Couldn't fire event: %s", e.getMessage());
diff --git a/java/com/google/gerrit/server/git/RefCache.java b/java/com/google/gerrit/server/git/RefCache.java
index 5a5cae9..2dee427 100644
--- a/java/com/google/gerrit/server/git/RefCache.java
+++ b/java/com/google/gerrit/server/git/RefCache.java
@@ -37,4 +37,7 @@
* present with a value of {@link ObjectId#zeroId()}.
*/
Optional<ObjectId> get(String refName) throws IOException;
+
+ /** Closes this cache, releasing the references to any underlying resources. */
+ void close();
}
diff --git a/java/com/google/gerrit/server/git/RepoRefCache.java b/java/com/google/gerrit/server/git/RepoRefCache.java
index 9086da7..c813b83 100644
--- a/java/com/google/gerrit/server/git/RepoRefCache.java
+++ b/java/com/google/gerrit/server/git/RepoRefCache.java
@@ -14,36 +14,66 @@
package com.google.gerrit.server.git;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.cache.PerThreadCache;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
-/** {@link RefCache} backed directly by a repository. */
+/**
+ * {@link RefCache} backed directly by a repository. TODO: DO NOT MERGE
+ * PerThreadCache.CacheStalenessCheck into stable-3.2 onwards.
+ */
public class RepoRefCache implements RefCache {
+
+ /**
+ * System property for enabling the check for stale cache entries. TODO: DO NOT MERGE into
+ * stable-3.2 onwards.
+ */
+ public static final String REPO_REF_CACHE_CHECK_STALE_ENTRIES_PROPERTY =
+ "RepoRefCache_checkStaleEntries";
+
+ private static FluentLogger log = FluentLogger.forEnclosingClass();
+
private final RefDatabase refdb;
private final Map<String, Optional<ObjectId>> ids;
+ private final Repository repo;
+ private final boolean checkStaleEntries;
+ private final AtomicBoolean closed;
public static Optional<RefCache> getOptional(Repository repo) {
PerThreadCache cache = PerThreadCache.get();
- if (cache != null && cache.hasReadonlyRequest()) {
- return Optional.of(
- cache.get(
- PerThreadCache.Key.create(RepoRefCache.class, repo), () -> new RepoRefCache(repo)));
+ if (cache != null && cache.allowRefCache()) {
+ return cache
+ .getWithLoader(
+ PerThreadCache.Key.create(RepoRefCache.class, repo),
+ () -> new RepoRefCache(repo),
+ RepoRefCache::close)
+ .map(c -> (RefCache) c);
}
return Optional.empty();
}
public RepoRefCache(Repository repo) {
+ checkStaleEntries =
+ Boolean.valueOf(System.getProperty(REPO_REF_CACHE_CHECK_STALE_ENTRIES_PROPERTY, "false"));
+
+ repo.incrementOpen();
+ this.repo = repo;
this.refdb = repo.getRefDatabase();
this.ids = new HashMap<>();
+ this.closed = new AtomicBoolean();
}
@Override
@@ -62,4 +92,69 @@
public Map<String, Optional<ObjectId>> getCachedRefs() {
return Collections.unmodifiableMap(ids);
}
+
+ @Override
+ public void close() {
+ if (closed.getAndSet(true)) {
+ log.atWarning().log("RepoRefCache of {} closed more than once", repo.getDirectory());
+ return;
+ }
+
+ if (checkStaleEntries) {
+ checkStaleness();
+ }
+
+ repo.close();
+ }
+
+ /** TODO: DO NOT MERGE into stable-3.2 onwards. */
+ @VisibleForTesting
+ void checkStaleness() {
+ List<String> staleRefs = staleRefs();
+ if (staleRefs.size() > 0) {
+ throw new IllegalStateException(
+ "Repository "
+ + repo
+ + " had modifications on refs "
+ + staleRefs
+ + " during a readonly window");
+ }
+ }
+
+ private List<String> staleRefs() {
+ return ids.entrySet().stream()
+ .filter(this::isStale)
+ .map(Map.Entry::getKey)
+ .collect(Collectors.toList());
+ }
+
+ private boolean isStale(Map.Entry<String, Optional<ObjectId>> refEntry) {
+ String refName = refEntry.getKey();
+ Optional<ObjectId> id = ids.get(refName);
+ if (id == null) {
+ return false;
+ }
+
+ try {
+ ObjectId diskId = refdb.exactRef(refName).getObjectId();
+ boolean isStale = !Optional.ofNullable(diskId).equals(id);
+ if (isStale) {
+ log.atSevere().log(
+ "Repository "
+ + repo
+ + " has a stale ref "
+ + refName
+ + " (cache="
+ + id
+ + ", disk="
+ + diskId
+ + ")");
+ }
+ return isStale;
+ } catch (IOException e) {
+ log.atSevere().withCause(e).log(
+ "Unable to check if ref={} from repository={} is stale", refName, repo);
+ return true;
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 19c1fa2..0ff6072 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -85,7 +85,7 @@
protected boolean emailOnlyAuthors;
protected ChangeEmail(EmailArguments args, String messageClass, ChangeData changeData) {
- super(args, messageClass, changeData.change().getDest());
+ super(args, messageClass, changeData);
this.changeData = changeData;
this.change = changeData.change();
this.emailOnlyAuthors = false;
diff --git a/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index 0fb5c6f..1883875 100644
--- a/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -25,7 +25,10 @@
import com.google.gerrit.mail.Address;
import com.google.gerrit.mail.MailHeader;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
+import com.google.gerrit.server.query.change.ChangeData;
import java.util.HashMap;
import java.util.Map;
@@ -35,9 +38,11 @@
protected BranchNameKey branch;
- protected NotificationEmail(EmailArguments args, String messageClass, BranchNameKey branch) {
+ protected NotificationEmail(EmailArguments args, String messageClass, ChangeData cd) {
super(args, messageClass);
- this.branch = branch;
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ this.branch = cd.change().getDest();
+ }
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index fd4233e..0448688 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -24,7 +24,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.UserIdentity;
import com.google.gerrit.exceptions.EmailException;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
@@ -33,6 +32,8 @@
import com.google.gerrit.mail.EmailHeader.AddressList;
import com.google.gerrit.mail.MailHeader;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
@@ -94,138 +95,141 @@
* @throws EmailException
*/
public void send() throws EmailException {
- if (!args.emailSender.isEnabled()) {
- // Server has explicitly disabled email sending.
- //
- logger.atFine().log(
- "Not sending '%s': Email sending is disabled by server config", messageClass);
- return;
- }
-
- if (!notify.shouldNotify()) {
- logger.atFine().log("Not sending '%s': Notify handling is NONE", messageClass);
- return;
- }
-
- init();
- if (useHtml()) {
- appendHtml(soyHtmlTemplate("HeaderHtml"));
- }
- format();
- appendText(textTemplate("Footer"));
- if (useHtml()) {
- appendHtml(soyHtmlTemplate("FooterHtml"));
- }
-
- Set<Address> smtpRcptToPlaintextOnly = new HashSet<>();
- if (shouldSendMessage()) {
- if (fromId != null) {
- Optional<AccountState> fromUser = args.accountCache.get(fromId);
- if (fromUser.isPresent()) {
- GeneralPreferencesInfo senderPrefs = fromUser.get().generalPreferences();
- if (senderPrefs != null && senderPrefs.getEmailStrategy() == CC_ON_OWN_COMMENTS) {
- // If we are impersonating a user, make sure they receive a CC of
- // this message so they can always review and audit what we sent
- // on their behalf to others.
- //
- add(RecipientType.CC, fromId);
- } else if (!notify.accounts().containsValue(fromId) && rcptTo.remove(fromId)) {
- // If they don't want a copy, but we queued one up anyway,
- // drop them from the recipient lists.
- //
- removeUser(fromUser.get().account());
- }
- }
- }
- // Check the preferences of all recipients. If any user has disabled
- // his email notifications then drop him from recipients' list.
- // In addition, check if users only want to receive plaintext email.
- for (Account.Id id : rcptTo) {
- Optional<AccountState> thisUser = args.accountCache.get(id);
- if (thisUser.isPresent()) {
- Account thisUserAccount = thisUser.get().account();
- GeneralPreferencesInfo prefs = thisUser.get().generalPreferences();
- if (prefs == null || prefs.getEmailStrategy() == DISABLED) {
- removeUser(thisUserAccount);
- } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) {
- removeUser(thisUserAccount);
- smtpRcptToPlaintextOnly.add(
- new Address(thisUserAccount.fullName(), thisUserAccount.preferredEmail()));
- }
- }
- if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
- logger.atFine().log("Not sending '%s': No SMTP recipients", messageClass);
- return;
- }
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+ if (!args.emailSender.isEnabled()) {
+ // Server has explicitly disabled email sending.
+ //
+ logger.atFine().log(
+ "Not sending '%s': Email sending is disabled by server config", messageClass);
+ return;
}
- // Set Reply-To only if it hasn't been set by a child class
- // Reply-To will already be populated for the message types where Gerrit supports
- // inbound email replies.
- if (!headers.containsKey(FieldName.REPLY_TO)) {
- StringJoiner j = new StringJoiner(", ");
- if (fromId != null) {
- Address address = toAddress(fromId);
- if (address != null) {
- j.add(address.getEmail());
- }
- }
- smtpRcptTo.stream().forEach(a -> j.add(a.getEmail()));
- smtpRcptToPlaintextOnly.stream().forEach(a -> j.add(a.getEmail()));
- setHeader(FieldName.REPLY_TO, j.toString());
+ if (!notify.shouldNotify()) {
+ logger.atFine().log("Not sending '%s': Notify handling is NONE", messageClass);
+ return;
}
- String textPart = textBody.toString();
- OutgoingEmailValidationListener.Args va = new OutgoingEmailValidationListener.Args();
- va.messageClass = messageClass;
- va.smtpFromAddress = smtpFromAddress;
- va.smtpRcptTo = smtpRcptTo;
- va.headers = headers;
- va.body = textPart;
-
+ init();
if (useHtml()) {
- va.htmlBody = htmlBody.toString();
- } else {
- va.htmlBody = null;
+ appendHtml(soyHtmlTemplate("HeaderHtml"));
+ }
+ format();
+ appendText(textTemplate("Footer"));
+ if (useHtml()) {
+ appendHtml(soyHtmlTemplate("FooterHtml"));
}
- 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> smtpRcptToPlaintextOnly = new HashSet<>();
+ if (shouldSendMessage()) {
+ if (fromId != null) {
+ Optional<AccountState> fromUser = args.accountCache.get(fromId);
+ if (fromUser.isPresent()) {
+ GeneralPreferencesInfo senderPrefs = fromUser.get().generalPreferences();
+ if (senderPrefs != null && senderPrefs.getEmailStrategy() == CC_ON_OWN_COMMENTS) {
+ // If we are impersonating a user, make sure they receive a CC of
+ // this message so they can always review and audit what we sent
+ // on their behalf to others.
+ //
+ add(RecipientType.CC, fromId);
+ } else if (!notify.accounts().containsValue(fromId) && rcptTo.remove(fromId)) {
+ // If they don't want a copy, but we queued one up anyway,
+ // drop them from the recipient lists.
+ //
+ removeUser(fromUser.get().account());
+ }
+ }
}
- }
-
- 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
- logger.atFine().log("Sending multipart '%s'", messageClass);
- args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
- }
-
- if (!smtpRcptToPlaintextOnly.isEmpty()) {
- logger.atFine().log("Sending plaintext '%s'", messageClass);
- // Send plaintext message
- Map<String, EmailHeader> shallowCopy = new HashMap<>();
- shallowCopy.putAll(headers);
- // Remove To and Cc
- shallowCopy.remove(FieldName.TO);
- shallowCopy.remove(FieldName.CC);
- for (Address a : smtpRcptToPlaintextOnly) {
- // Add new To
- EmailHeader.AddressList to = new EmailHeader.AddressList();
- to.add(a);
- shallowCopy.put(FieldName.TO, to);
+ // Check the preferences of all recipients. If any user has disabled
+ // his email notifications then drop him from recipients' list.
+ // In addition, check if users only want to receive plaintext email.
+ for (Account.Id id : rcptTo) {
+ Optional<AccountState> thisUser = args.accountCache.get(id);
+ if (thisUser.isPresent()) {
+ Account thisUserAccount = thisUser.get().account();
+ GeneralPreferencesInfo prefs = thisUser.get().generalPreferences();
+ if (prefs == null || prefs.getEmailStrategy() == DISABLED) {
+ removeUser(thisUserAccount);
+ } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) {
+ removeUser(thisUserAccount);
+ smtpRcptToPlaintextOnly.add(
+ new Address(thisUserAccount.fullName(), thisUserAccount.preferredEmail()));
+ }
+ }
+ if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
+ logger.atFine().log("Not sending '%s': No SMTP recipients", messageClass);
+ return;
+ }
}
- args.emailSender.send(va.smtpFromAddress, smtpRcptToPlaintextOnly, shallowCopy, va.body);
+
+ // Set Reply-To only if it hasn't been set by a child class
+ // Reply-To will already be populated for the message types where Gerrit supports
+ // inbound email replies.
+ if (!headers.containsKey(FieldName.REPLY_TO)) {
+ StringJoiner j = new StringJoiner(", ");
+ if (fromId != null) {
+ Address address = toAddress(fromId);
+ if (address != null) {
+ j.add(address.getEmail());
+ }
+ }
+ smtpRcptTo.stream().forEach(a -> j.add(a.getEmail()));
+ smtpRcptToPlaintextOnly.stream().forEach(a -> j.add(a.getEmail()));
+ setHeader(FieldName.REPLY_TO, j.toString());
+ }
+
+ String textPart = textBody.toString();
+ OutgoingEmailValidationListener.Args va = new OutgoingEmailValidationListener.Args();
+ va.messageClass = messageClass;
+ va.smtpFromAddress = smtpFromAddress;
+ va.smtpRcptTo = smtpRcptTo;
+ va.headers = headers;
+ va.body = textPart;
+
+ if (useHtml()) {
+ va.htmlBody = htmlBody.toString();
+ } else {
+ 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
+ logger.atFine().log("Sending multipart '%s'", messageClass);
+ args.emailSender.send(
+ va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
+ }
+
+ if (!smtpRcptToPlaintextOnly.isEmpty()) {
+ logger.atFine().log("Sending plaintext '%s'", messageClass);
+ // Send plaintext message
+ Map<String, EmailHeader> shallowCopy = new HashMap<>();
+ shallowCopy.putAll(headers);
+ // Remove To and Cc
+ shallowCopy.remove(FieldName.TO);
+ shallowCopy.remove(FieldName.CC);
+ for (Address a : smtpRcptToPlaintextOnly) {
+ // Add new To
+ EmailHeader.AddressList to = new EmailHeader.AddressList();
+ to.add(a);
+ shallowCopy.put(FieldName.TO, to);
+ }
+ args.emailSender.send(va.smtpFromAddress, smtpRcptToPlaintextOnly, shallowCopy, va.body);
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index df4bc24..d6a32d3 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -311,7 +311,6 @@
}
private final boolean shouldExist;
- private final RefCache refs;
private Change change;
private ChangeNotesState state;
@@ -334,7 +333,6 @@
super(args, change.getId());
this.change = new Change(change);
this.shouldExist = shouldExist;
- this.refs = refs;
}
public Change getChange() {
@@ -567,8 +565,7 @@
@Override
protected ObjectId readRef(Repository repo) throws IOException {
- Optional<RefCache> refsCache =
- Optional.ofNullable(refs).map(Optional::of).orElse(RepoRefCache.getOptional(repo));
+ Optional<RefCache> refsCache = RepoRefCache.getOptional(repo);
return refsCache.isPresent()
? refsCache.get().get(getRefName()).orElse(null)
: super.readRef(repo);
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 2c7a83a..137eb32 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -46,6 +46,8 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
@@ -207,7 +209,8 @@
try (MergeOp op = mergeOpProvider.get()) {
Change updatedChange;
- try {
+ try (ReadonlyRequestWindow readonlyRequestWindow =
+ PerThreadCache.openReadonlyRequestWindow()) {
updatedChange = op.merge(change, submitter, true, input, false);
} catch (Exception e) {
Throwables.throwIfInstanceOf(e, RestApiException.class);
diff --git a/java/com/google/gerrit/server/update/ChainedReceiveCommands.java b/java/com/google/gerrit/server/update/ChainedReceiveCommands.java
index c223aec..9a39575 100644
--- a/java/com/google/gerrit/server/update/ChainedReceiveCommands.java
+++ b/java/com/google/gerrit/server/update/ChainedReceiveCommands.java
@@ -39,13 +39,19 @@
public class ChainedReceiveCommands implements RefCache {
private final Map<String, ReceiveCommand> commands = new LinkedHashMap<>();
private final RepoRefCache refCache;
+ private final boolean closeRefCache;
public ChainedReceiveCommands(Repository repo) {
- this(new RepoRefCache(repo));
+ this(new RepoRefCache(repo), true);
}
public ChainedReceiveCommands(RepoRefCache refCache) {
+ this(refCache, false);
+ }
+
+ private ChainedReceiveCommands(RepoRefCache refCache, boolean closeRefCache) {
this.refCache = requireNonNull(refCache);
+ this.closeRefCache = closeRefCache;
}
public RepoRefCache getRepoRefCache() {
@@ -122,4 +128,11 @@
public Map<String, ReceiveCommand> getCommands() {
return Collections.unmodifiableMap(commands);
}
+
+ @Override
+ public void close() {
+ if (closeRefCache) {
+ refCache.close();
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/cache/BUILD b/javatests/com/google/gerrit/server/cache/BUILD
index bb6ea30..346c578 100644
--- a/javatests/com/google/gerrit/server/cache/BUILD
+++ b/javatests/com/google/gerrit/server/cache/BUILD
@@ -7,6 +7,7 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//javatests/com/google/gerrit/util/http/testutil",
+ "//lib:jgit",
"//lib:junit",
"//lib/truth",
"//lib/truth:truth-java8-extension",
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
index 9d0e0e0..35a28c0 100644
--- a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
@@ -18,10 +18,17 @@
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.server.cache.PerThreadCache.Key;
+import com.google.gerrit.server.cache.PerThreadCache.ReadonlyRequestWindow;
+import com.google.gerrit.server.git.RefCache;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
+import java.io.IOException;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
+import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class PerThreadCacheTest {
@@ -47,14 +54,13 @@
PerThreadCache cache = PerThreadCache.get();
PerThreadCache.Key<String> key1 = PerThreadCache.Key.create(String.class);
- String value1 = cache.get(key1, () -> "value1");
- assertThat(value1).isEqualTo("value1");
+ assertThat(cache.getWithLoader(key1, () -> "value1", null)).hasValue("value1");
Supplier<String> neverCalled =
() -> {
throw new IllegalStateException("this method must not be called");
};
- assertThat(cache.get(key1, neverCalled)).isEqualTo("value1");
+ assertThat(cache.getWithLoader(key1, neverCalled, null)).hasValue("value1");
}
}
@@ -63,20 +69,30 @@
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
try (PerThreadCache ignored = PerThreadCache.create(null)) {
PerThreadCache cache = PerThreadCache.get();
- String value1 = cache.get(key, () -> "value1");
- assertThat(value1).isEqualTo("value1");
+ assertThat(cache.getWithLoader(key, () -> "value1", null)).hasValue("value1");
}
// Create a second cache and assert that it is not connected to the first one.
// This ensures that the cleanup is actually working.
try (PerThreadCache ignored = PerThreadCache.create(null)) {
PerThreadCache cache = PerThreadCache.get();
- String value1 = cache.get(key, () -> "value2");
- assertThat(value1).isEqualTo("value2");
+ assertThat(cache.getWithLoader(key, () -> "value2", null)).hasValue("value2");
}
}
@Test
+ public void unloaderCalledUponCleanup() {
+ AtomicBoolean unloaderCalled = new AtomicBoolean();
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
+ PerThreadCache cache = PerThreadCache.get();
+ cache.getWithLoader(key, () -> "value1", v -> unloaderCalled.set(true));
+ assertThat(unloaderCalled.get()).isFalse();
+ }
+ assertThat(unloaderCalled.get()).isTrue();
+ }
+
+ @Test
public void doubleInstantiationFails() {
try (PerThreadCache ignored = PerThreadCache.create(null)) {
IllegalStateException thrown =
@@ -89,8 +105,7 @@
public void isAssociatedWithHttpReadonlyRequest() {
HttpServletRequest getRequest = new FakeHttpServletRequest();
try (PerThreadCache cache = PerThreadCache.create(getRequest)) {
- assertThat(cache.getHttpRequest()).hasValue(getRequest);
- assertThat(cache.hasReadonlyRequest()).isTrue();
+ assertThat(cache.allowRefCache()).isTrue();
}
}
@@ -104,32 +119,211 @@
}
};
try (PerThreadCache cache = PerThreadCache.create(putRequest)) {
- assertThat(cache.getHttpRequest()).hasValue(putRequest);
- assertThat(cache.hasReadonlyRequest()).isFalse();
+ assertThat(cache.allowRefCache()).isFalse();
}
}
@Test
public void isNotAssociatedWithHttpRequest() {
try (PerThreadCache cache = PerThreadCache.create(null)) {
- assertThat(cache.getHttpRequest()).isEmpty();
- assertThat(cache.hasReadonlyRequest()).isFalse();
+ assertThat(cache.allowRefCache()).isFalse();
}
}
@Test
+ public void isAssociatedWithReadonlyRequest() {
+ try (PerThreadCache cache = PerThreadCache.createReadOnly()) {
+ assertThat(cache.allowRefCache()).isTrue();
+ }
+ }
+
+ @Test
+ public void openNestedTemporaryReadonlyWindows() throws Exception {
+
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ assertThat(cache.allowRefCache()).isFalse();
+
+ try (ReadonlyRequestWindow outerWindow = PerThreadCache.openReadonlyRequestWindow()) {
+ assertThat(cache.allowRefCache()).isTrue();
+
+ try (ReadonlyRequestWindow innerWindow = PerThreadCache.openReadonlyRequestWindow()) {
+ assertThat(cache.allowRefCache()).isTrue();
+ }
+
+ assertThat(cache.allowRefCache()).isTrue();
+ }
+
+ assertThat(cache.allowRefCache()).isFalse();
+ }
+ }
+
+ static class TestCacheValue implements RefCache {
+ public static final String STALENESS_FAILED_MESSAGE = "Staleness check failed";
+
+ private final String value;
+
+ public TestCacheValue(String value) {
+ this.value = value;
+ }
+
+ @Override
+ public String toString() {
+ return value;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return obj != null
+ && (obj instanceof TestCacheValue)
+ && ((TestCacheValue) obj).value.equals(value);
+ }
+
+ @Override
+ public int hashCode() {
+ return value.hashCode();
+ }
+
+ public void checkStaleness() throws IllegalStateException {
+ throw new IllegalStateException(STALENESS_FAILED_MESSAGE);
+ }
+
+ @Override
+ public Optional<ObjectId> get(String refName) throws IOException {
+ return Optional.empty();
+ }
+
+ @Override
+ public void close() {}
+ }
+
+ @Test
+ public void clearOutStaleEntriesAfterReadonlyWindow() throws Exception {
+ Key<TestCacheValue> key = PerThreadCache.Key.create(TestCacheValue.class, "key");
+ TestCacheValue cachedValue = new TestCacheValue("cached value");
+ TestCacheValue updatedValue = new TestCacheValue("updated value");
+
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ try (ReadonlyRequestWindow outerWindow = PerThreadCache.openReadonlyRequestWindow()) {
+ assertThat(PerThreadCache.get(key, () -> cachedValue, v -> {})).hasValue(cachedValue);
+ assertThat(PerThreadCache.get(key, () -> updatedValue, v -> {})).hasValue(cachedValue);
+ }
+
+ assertThat(PerThreadCache.get(key, () -> updatedValue, v -> {})).hasValue(updatedValue);
+ }
+ }
+
+ @Test
+ public void checkForStaleEntriesAfterReadonlyWindow() {
+ Key<TestCacheValue> key = PerThreadCache.Key.create(TestCacheValue.class, "key");
+
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () -> {
+ try (ReadonlyRequestWindow outerWindow =
+ PerThreadCache.openReadonlyRequestWindow()) {
+ PerThreadCache.get(
+ key, () -> new TestCacheValue(""), TestCacheValue::checkStaleness);
+ }
+ });
+ assertThat(thrown).hasMessageThat().isEqualTo(TestCacheValue.STALENESS_FAILED_MESSAGE);
+ }
+ }
+
+ @Test
+ public void allowStaleEntriesAfterReadonlyWindow() {
+ Key<TestCacheValue> key = PerThreadCache.Key.create(TestCacheValue.class, "key");
+ TestCacheValue value = new TestCacheValue("");
+ Optional<TestCacheValue> cachedValue;
+
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ try (ReadonlyRequestWindow window = PerThreadCache.openReadonlyRequestWindow()) {
+
+ cachedValue = PerThreadCache.get(key, () -> value, null);
+ }
+ }
+
+ assertThat(cachedValue).hasValue(value);
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void disableCachingForSpecificTypes() {
+ System.setProperty(
+ PerThreadCache.PER_THREAD_CACHE_DISABLED_TYPES_PROPERTY,
+ String.class.getCanonicalName() + "," + Integer.class.getCanonicalName());
+ try {
+ try (PerThreadCache cache = PerThreadCache.createReadOnly()) {
+ assertThat(
+ PerThreadCache.get(
+ PerThreadCache.Key.create(String.class, "key"), () -> "oldValue", null))
+ .isEmpty();
+ assertThat(
+ PerThreadCache.getOrCompute(
+ PerThreadCache.Key.create(String.class, "key"), () -> "newValue"))
+ .isEqualTo("newValue");
+
+ assertThat(
+ PerThreadCache.get(PerThreadCache.Key.create(Integer.class, "key"), () -> 1, null))
+ .isEmpty();
+ assertThat(
+ PerThreadCache.getOrCompute(
+ PerThreadCache.Key.create(Integer.class, "key"), () -> 2))
+ .isEqualTo(2);
+
+ assertThat(PerThreadCache.get(PerThreadCache.Key.create(Long.class, "key"), () -> 1L, null))
+ .hasValue(1L);
+ assertThat(
+ PerThreadCache.getOrCompute(PerThreadCache.Key.create(Long.class, "key"), () -> 2L))
+ .isEqualTo(1L);
+ }
+ } finally {
+ System.setProperty(PerThreadCache.PER_THREAD_CACHE_DISABLED_TYPES_PROPERTY, "");
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
public void enforceMaxSize() {
try (PerThreadCache cache = PerThreadCache.create(null)) {
- // Fill the cache
- for (int i = 0; i < 50; i++) {
- PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);
- cache.get(key, () -> "cached value");
- }
+ fillTheCache(cache);
+
// Assert that the value was not persisted
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, 1000);
- cache.get(key, () -> "new value");
+ assertThat(cache.getWithLoader(key, () -> "new value", null)).isEmpty();
String value = cache.get(key, () -> "directly served");
assertThat(value).isEqualTo("directly served");
}
}
+
+ @Test
+ public void returnEmptyWhenCacheIsFull() {
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ fillTheCache(cache);
+
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, 1000);
+ assertThat(cache.getWithLoader(key, () -> "new value", null)).isEmpty();
+ }
+ }
+
+ @Test
+ public void unloaderNotCalledUponCleanupIfCacheWasFull() {
+ AtomicBoolean unloaderCalled = new AtomicBoolean();
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
+ PerThreadCache cache = PerThreadCache.get();
+ fillTheCache(cache);
+
+ cache.getWithLoader(key, () -> "value1", v -> unloaderCalled.set(true));
+ }
+ assertThat(unloaderCalled.get()).isFalse();
+ }
+
+ private void fillTheCache(PerThreadCache cache) {
+ for (int i = 0; i < 50; i++) {
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);
+ cache.getWithLoader(key, () -> "cached value", null);
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/git/RepoRefCacheTest.java b/javatests/com/google/gerrit/server/git/RepoRefCacheTest.java
new file mode 100644
index 0000000..99618db
--- /dev/null
+++ b/javatests/com/google/gerrit/server/git/RepoRefCacheTest.java
@@ -0,0 +1,334 @@
+// Copyright (C) 2022 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.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.server.cache.PerThreadCache;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.attributes.AttributesNodeProvider;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository.Builder;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectDatabase;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.RefRename;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
+import org.eclipse.jgit.lib.ReflogReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.Test;
+
+public class RepoRefCacheTest {
+ private static final String TEST_BRANCH = "main";
+
+ @Test
+ @SuppressWarnings("resource")
+ public void repositoryUseShouldBeTrackedByRepoRefCache() throws Exception {
+ RefCache cache;
+ TestRepositoryWithRefCounting repoWithRefCounting;
+
+ try (TestRepositoryWithRefCounting repo =
+ TestRepositoryWithRefCounting.createWithBranch(TEST_BRANCH)) {
+ assertThat(repo.refCounter()).isEqualTo(1);
+ repoWithRefCounting = repo;
+ cache = new RepoRefCache(repo);
+ }
+
+ assertThat(repoWithRefCounting.refCounter()).isEqualTo(1);
+ assertThat(cache.get(Constants.R_HEADS + TEST_BRANCH)).isNotNull();
+ }
+
+ @Test
+ @SuppressWarnings("resource")
+ public void shouldNotKeepReferenceToReposWhenCacheIsFull() throws Exception {
+ TestRepositoryWithRefCounting repoPointedFromCache;
+
+ try (PerThreadCache threadCache = PerThreadCache.createReadOnly()) {
+ fillUpAllThreadCache(threadCache);
+
+ try (TestRepositoryWithRefCounting repo =
+ TestRepositoryWithRefCounting.createWithBranch(TEST_BRANCH)) {
+ repoPointedFromCache = repo;
+ assertThat(repo.refCounter()).isEqualTo(1);
+ Optional<RefCache> refCache = RepoRefCache.getOptional(repo);
+ assertThat(refCache).isEqualTo(Optional.empty());
+ assertThat(repo.refCounter()).isEqualTo(1);
+ }
+
+ assertThat(repoPointedFromCache.refCounter()).isEqualTo(0);
+ }
+ }
+
+ @Test
+ public void shouldCheckForStaleness() throws Exception {
+ String refName = "refs/heads/foo";
+
+ try (TestRepositoryWithRefCounting repo =
+ TestRepositoryWithRefCounting.createWithBranch(TEST_BRANCH)) {
+ RepoRefCache refCache = new RepoRefCache(repo);
+ TestRepository<Repository> testRepo = new TestRepository<>(repo);
+
+ Optional<ObjectId> cachedObjId = refCache.get(refName);
+
+ assertThat(cachedObjId).isEqualTo(Optional.empty());
+
+ RefUpdate refUpdate = repo.getRefDatabase().newUpdate(refName, true);
+ refUpdate.setNewObjectId(testRepo.commit().create().getId());
+
+ assertThat(refUpdate.forceUpdate()).isEqualTo(Result.NEW);
+
+ IllegalStateException thrown =
+ assertThrows(IllegalStateException.class, () -> refCache.checkStaleness());
+ assertThat(thrown).hasMessageThat().contains(refName);
+ }
+ }
+
+ private void fillUpAllThreadCache(PerThreadCache cache) {
+
+ // Fill the cache
+ for (int i = 0; i < 50; i++) {
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);
+ cache.getWithLoader(key, () -> "cached value", null);
+ }
+ }
+
+ private static class TestRepositoryWithRefCounting extends Repository {
+ private int refCounter;
+
+ static TestRepositoryWithRefCounting createWithBranch(String branchName) throws Exception {
+ Builder builder =
+ new InMemoryRepository.Builder()
+ .setRepositoryDescription(new DfsRepositoryDescription(""))
+ .setFS(FS.detect().setUserHome(null));
+ TestRepositoryWithRefCounting testRepo = new TestRepositoryWithRefCounting(builder);
+ new TestRepository<>(testRepo).branch(branchName).commit().message("").create();
+ return testRepo;
+ }
+
+ private final Repository repo;
+
+ private TestRepositoryWithRefCounting(InMemoryRepository.Builder builder) throws IOException {
+ super(builder);
+
+ repo = builder.build();
+ refCounter = 1;
+ }
+
+ public int refCounter() {
+ return refCounter;
+ }
+
+ @Override
+ public void incrementOpen() {
+ repo.incrementOpen();
+ refCounter++;
+ }
+
+ @Override
+ public void close() {
+ repo.close();
+ refCounter--;
+ }
+
+ @Override
+ public void create(boolean bare) throws IOException {}
+
+ @Override
+ public ObjectDatabase getObjectDatabase() {
+ checkIsOpen();
+ return repo.getObjectDatabase();
+ }
+
+ @Override
+ public RefDatabase getRefDatabase() {
+ RefDatabase refDatabase = repo.getRefDatabase();
+ return new RefDatabase() {
+
+ @Override
+ public int hashCode() {
+ return refDatabase.hashCode();
+ }
+
+ @Override
+ public void create() throws IOException {
+ refDatabase.create();
+ }
+
+ @Override
+ public void close() {
+ checkIsOpen();
+ refDatabase.close();
+ }
+
+ @Override
+ public boolean isNameConflicting(String name) throws IOException {
+ checkIsOpen();
+ return refDatabase.isNameConflicting(name);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return refDatabase.equals(obj);
+ }
+
+ @Override
+ public Collection<String> getConflictingNames(String name) throws IOException {
+ checkIsOpen();
+ return refDatabase.getConflictingNames(name);
+ }
+
+ @Override
+ public RefUpdate newUpdate(String name, boolean detach) throws IOException {
+ checkIsOpen();
+ return refDatabase.newUpdate(name, detach);
+ }
+
+ @Override
+ public RefRename newRename(String fromName, String toName) throws IOException {
+ checkIsOpen();
+ return refDatabase.newRename(fromName, toName);
+ }
+
+ @Override
+ public BatchRefUpdate newBatchUpdate() {
+ checkIsOpen();
+ return refDatabase.newBatchUpdate();
+ }
+
+ @Override
+ public boolean performsAtomicTransactions() {
+ checkIsOpen();
+ return refDatabase.performsAtomicTransactions();
+ }
+
+ @Override
+ public Ref exactRef(String name) throws IOException {
+ checkIsOpen();
+ return refDatabase.exactRef(name);
+ }
+
+ @Override
+ public String toString() {
+ return refDatabase.toString();
+ }
+
+ @Override
+ public Map<String, Ref> exactRef(String... refs) throws IOException {
+ checkIsOpen();
+ return refDatabase.exactRef(refs);
+ }
+
+ @Override
+ public Ref firstExactRef(String... refs) throws IOException {
+ checkIsOpen();
+ return refDatabase.firstExactRef(refs);
+ }
+
+ @Override
+ public List<Ref> getRefs() throws IOException {
+ checkIsOpen();
+ return refDatabase.getRefs();
+ }
+
+ @Override
+ public Map<String, Ref> getRefs(String prefix) throws IOException {
+ checkIsOpen();
+ return refDatabase.getRefs(prefix);
+ }
+
+ @Override
+ public List<Ref> getRefsByPrefix(String prefix) throws IOException {
+ checkIsOpen();
+ return refDatabase.getRefsByPrefix(prefix);
+ }
+
+ @Override
+ public boolean hasRefs() throws IOException {
+ checkIsOpen();
+ return refDatabase.hasRefs();
+ }
+
+ @Override
+ public List<Ref> getAdditionalRefs() throws IOException {
+ checkIsOpen();
+ return refDatabase.getAdditionalRefs();
+ }
+
+ @Override
+ public Ref peel(Ref ref) throws IOException {
+ checkIsOpen();
+ return refDatabase.peel(ref);
+ }
+
+ @Override
+ public void refresh() {
+ checkIsOpen();
+ refDatabase.refresh();
+ }
+ };
+ }
+
+ @Override
+ public StoredConfig getConfig() {
+ return repo.getConfig();
+ }
+
+ @Override
+ public AttributesNodeProvider createAttributesNodeProvider() {
+ checkIsOpen();
+ return repo.createAttributesNodeProvider();
+ }
+
+ @Override
+ public void scanForRepoChanges() throws IOException {
+ checkIsOpen();
+ }
+
+ @Override
+ public void notifyIndexChanged(boolean internal) {
+ checkIsOpen();
+ }
+
+ @Override
+ public ReflogReader getReflogReader(String refName) throws IOException {
+ checkIsOpen();
+ return repo.getReflogReader(refName);
+ }
+
+ private void checkIsOpen() {
+ if (refCounter <= 0) {
+ throw new IllegalStateException("Repository is not open (refCounter=" + refCounter + ")");
+ }
+ }
+
+ @Override
+ public String getIdentifier() {
+ return "foo";
+ }
+ }
+}
