Merge branch 'stable-2.14' into stable-2.15 * stable-2.14: Docker: Allow init to write HA plugin config file Docker: Support optional non-default gerrit userid Docker: Use CI archive as 2.14 is no longer built Keep using the CI archive for this branch, as 2.15 is no longer built. Configure the plugin-manager plugin's jenkinsUrl accordingly. Such configuration is not supported in the stable-2.14 branch; gerrit-ci (non-archive) is hardcoded there. However, tolerate the exception thrown at runtime as plugin-manager tries to use a currently non-existing Jenkins view [1]. The latter should be configured soon though, similarly with [2], which 2.14 was also EOL'ed earlier. [1] http://archive-ci.gerritforge.com/view/Plugins-stable-2.15/ [2] http://archive-ci.gerritforge.com/view/Plugins-stable-2.14/ Change-Id: I8bf808df47e0db7fb5889cca747dff1e0cdd19e5
diff --git a/WORKSPACE b/WORKSPACE index 8428490..04862c2 100644 --- a/WORKSPACE +++ b/WORKSPACE
@@ -3,7 +3,7 @@ load("//:bazlets.bzl", "load_bazlets") load_bazlets( - commit = "e48c6a61d5a3f9ab4589f324fc85b8e3777d8854", + commit = "ec949feac1b9dae6cc2c8c25d254f34924c54296", #local_path = "/home/<user>/projects/bazlets", )
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl index fc6118e..2aecc68 100644 --- a/external_plugin_deps.bzl +++ b/external_plugin_deps.bzl
@@ -9,8 +9,8 @@ maven_jar( name = "mockito", - artifact = "org.mockito:mockito-core:2.27.0", - sha1 = "835fc3283b481f4758b8ef464cd560c649c08b00", + artifact = "org.mockito:mockito-core:2.28.2", + sha1 = "91110215a8cb9b77a46e045ee758f77d79167cc0", deps = [ "@byte-buddy//jar", "@byte-buddy-agent//jar",
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java index d4c1272..edd96d7 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
@@ -51,6 +51,8 @@ // common parameters to cache and index sections static final String THREAD_POOL_SIZE_KEY = "threadPoolSize"; + static final int DEFAULT_INDEX_MAX_TRIES = 2; + static final int DEFAULT_INDEX_RETRY_INTERVAL = 30000; static final int DEFAULT_THREAD_POOL_SIZE = 4; static final String NUM_STRIPED_LOCKS = "numStripedLocks"; static final int DEFAULT_NUM_STRIPED_LOCKS = 10; @@ -446,8 +448,12 @@ public static class Index extends Forwarding { static final String INDEX_SECTION = "index"; + static final String MAX_TRIES_KEY = "maxTries"; + static final String RETRY_INTERVAL_KEY = "retryInterval"; private final int threadPoolSize; + private final int retryInterval; + private final int maxTries; private final int numStripedLocks; @@ -455,6 +461,8 @@ super(cfg, INDEX_SECTION); threadPoolSize = getInt(cfg, INDEX_SECTION, THREAD_POOL_SIZE_KEY, DEFAULT_THREAD_POOL_SIZE); numStripedLocks = getInt(cfg, INDEX_SECTION, NUM_STRIPED_LOCKS, DEFAULT_NUM_STRIPED_LOCKS); + retryInterval = getInt(cfg, INDEX_SECTION, RETRY_INTERVAL_KEY, DEFAULT_INDEX_RETRY_INTERVAL); + maxTries = getInt(cfg, INDEX_SECTION, MAX_TRIES_KEY, DEFAULT_INDEX_MAX_TRIES); } public int threadPoolSize() { @@ -464,6 +472,14 @@ public int numStripedLocks() { return numStripedLocks; } + + public int retryInterval() { + return retryInterval; + } + + public int maxTries() { + return maxTries; + } } public static class Websession extends Forwarding {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/ExecutorProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/ExecutorProvider.java index ea283d5..3d3212b 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/ExecutorProvider.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/ExecutorProvider.java
@@ -17,10 +17,11 @@ import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.server.git.WorkQueue; import com.google.inject.Provider; -import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; -public abstract class ExecutorProvider implements Provider<Executor>, LifecycleListener { - private WorkQueue.Executor executor; +public abstract class ExecutorProvider + implements Provider<ScheduledExecutorService>, LifecycleListener { + private ScheduledExecutorService executor; protected ExecutorProvider(WorkQueue workQueue, int threadPoolSize, String threadNamePrefix) { executor = workQueue.createQueue(threadPoolSize, threadNamePrefix); @@ -34,12 +35,11 @@ @Override public void stop() { executor.shutdown(); - executor.unregisterWorkQueue(); executor = null; } @Override - public Executor get() { + public ScheduledExecutorService get() { return executor; } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AccountReindexRunnable.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AccountReindexRunnable.java index 3b315a7..487f3da 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AccountReindexRunnable.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AccountReindexRunnable.java
@@ -19,9 +19,9 @@ import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.AbstractIndexRestApiServlet; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.account.Accounts; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import java.io.IOException; import java.sql.Timestamp; @@ -34,16 +34,22 @@ private final ForwardedIndexAccountHandler accountIdx; + private final Accounts accounts; + @Inject public AccountReindexRunnable( - ForwardedIndexAccountHandler accountIdx, IndexTs indexTs, OneOffRequestContext ctx) { + ForwardedIndexAccountHandler accountIdx, + IndexTs indexTs, + OneOffRequestContext ctx, + Accounts accounts) { super(AbstractIndexRestApiServlet.IndexName.ACCOUNT, indexTs, ctx); this.accountIdx = accountIdx; + this.accounts = accounts; } @Override - protected ResultSet<Account> fetchItems(ReviewDb db) throws OrmException { - return db.accounts().all(); + protected Iterable<Account> fetchItems(ReviewDb db) throws Exception { + return accounts.all(); } @Override @@ -52,7 +58,7 @@ Timestamp accountTs = a.getRegisteredOn(); if (accountTs.after(sinceTs)) { log.info("Index {}/{}/{}/{}", a.getId(), a.getFullName(), a.getPreferredEmail(), accountTs); - accountIdx.index(a.getId(), Operation.INDEX); + accountIdx.index(a.getId(), Operation.INDEX, Optional.empty()); return Optional.of(accountTs); } } catch (IOException | OrmException e) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AutoReindexScheduler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AutoReindexScheduler.java index 075d718..c0d90d1 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AutoReindexScheduler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/AutoReindexScheduler.java
@@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,7 +34,7 @@ private final ChangeReindexRunnable changeReindex; private final AccountReindexRunnable accountReindex; private final GroupReindexRunnable groupReindex; - private final WorkQueue.Executor executor; + private final ScheduledExecutorService executor; private final List<Future<?>> futureTasks = new ArrayList<>(); @Inject @@ -75,6 +76,5 @@ public void stop() { futureTasks.forEach(t -> t.cancel(true)); executor.shutdown(); - executor.unregisterWorkQueue(); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ChangeReindexRunnable.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ChangeReindexRunnable.java index 3c4f6ec..1f5b56e 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ChangeReindexRunnable.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ChangeReindexRunnable.java
@@ -17,15 +17,23 @@ import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexChangeHandler; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation; import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.AbstractIndexRestApiServlet; +import com.google.common.collect.Streams; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult; +import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import java.io.IOException; import java.sql.Timestamp; +import java.util.Iterator; import java.util.Optional; +import java.util.stream.Stream; +import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,16 +42,55 @@ private final ForwardedIndexChangeHandler changeIdx; + private final ProjectCache projectCache; + + private final GitRepositoryManager repoManager; + + private final ChangeNotes.Factory notesFactory; + + private static class StreamIterable implements Iterable<Change> { + + private final Stream<Change> stream; + + public StreamIterable(Stream<Change> stream) { + this.stream = stream; + } + + @Override + public Iterator<Change> iterator() { + return stream.iterator(); + } + } + @Inject public ChangeReindexRunnable( - ForwardedIndexChangeHandler changeIdx, IndexTs indexTs, OneOffRequestContext ctx) { + ForwardedIndexChangeHandler changeIdx, + IndexTs indexTs, + OneOffRequestContext ctx, + ProjectCache projectCache, + GitRepositoryManager repoManager, + ChangeNotes.Factory notesFactory) { super(AbstractIndexRestApiServlet.IndexName.CHANGE, indexTs, ctx); this.changeIdx = changeIdx; + this.projectCache = projectCache; + this.repoManager = repoManager; + this.notesFactory = notesFactory; } @Override - protected ResultSet<Change> fetchItems(ReviewDb db) throws OrmException { - return db.changes().all(); + protected Iterable<Change> fetchItems(ReviewDb db) throws Exception { + Stream<Change> allChangesStream = Stream.empty(); + Iterable<Project.NameKey> projects = projectCache.all(); + for (Project.NameKey projectName : projects) { + try (Repository repo = repoManager.openRepository(projectName)) { + Stream<Change> projectChangesStream = + notesFactory + .scan(repo, db, projectName) + .map((ChangeNotesResult changeNotes) -> changeNotes.notes().getChange()); + allChangesStream = Streams.concat(allChangesStream, projectChangesStream); + } + } + return new StreamIterable(allChangesStream); } @Override @@ -53,7 +100,7 @@ if (changeTs.after(sinceTs)) { log.info( "Index {}/{}/{} was updated after {}", c.getProject(), c.getId(), changeTs, sinceTs); - changeIdx.index(c.getId(), Operation.INDEX); + changeIdx.index(c.getProject() + "~" + c.getId(), Operation.INDEX, Optional.empty()); return Optional.of(changeTs); } } catch (OrmException | IOException e) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java index a9ebdb8..640457d 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/GroupReindexRunnable.java
@@ -76,7 +76,7 @@ if (groupLastTs.isPresent() && groupLastTs.get().after(sinceTs)) { log.info("Index {}/{}/{}", g.getGroupUUID(), g.getName(), groupLastTs.get()); - indexer.index(g.getGroupUUID(), Operation.INDEX); + indexer.index(g.getGroupUUID(), Operation.INDEX, Optional.empty()); return groupLastTs; } } catch (OrmException | IOException e) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java index 4c3eafa..952fcd1 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java
@@ -20,10 +20,10 @@ import com.google.gerrit.extensions.events.AccountIndexedListener; import com.google.gerrit.extensions.events.ChangeIndexedListener; import com.google.gerrit.extensions.events.GroupIndexedListener; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.git.WorkQueue; -import com.google.gwtorm.server.OrmException; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -34,6 +34,7 @@ import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; import java.util.Optional; +import java.util.concurrent.ScheduledExecutorService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,9 +45,10 @@ private static final DateTimeFormatter formatter = DateTimeFormatter.ISO_DATE_TIME; private final Path dataDir; - private final WorkQueue.Executor exec; + private final ScheduledExecutorService exec; private final FlusherRunner flusher; private final SchemaFactory<ReviewDb> schemaFactory; + private final ChangeFinder changeFinder; private volatile LocalDateTime changeTs; private volatile LocalDateTime accountTs; @@ -75,11 +77,16 @@ } @Inject - public IndexTs(@PluginData Path dataDir, WorkQueue queue, SchemaFactory<ReviewDb> schemaFactory) { + public IndexTs( + @PluginData Path dataDir, + WorkQueue queue, + SchemaFactory<ReviewDb> schemaFactory, + ChangeFinder changeFinder) { this.dataDir = dataDir; this.exec = queue.getDefaultQueue(); this.flusher = new FlusherRunner(); this.schemaFactory = schemaFactory; + this.changeFinder = changeFinder; } @Override @@ -93,13 +100,15 @@ } @Override - public void onChangeIndexed(int id) { + public void onChangeIndexed(String projectName, int id) { try (ReviewDb db = schemaFactory.open()) { - Change change = db.changes().get(new Change.Id(id)); + ChangeNotes changeNotes = changeFinder.findOne(projectName + "~" + id); update( IndexName.CHANGE, - change == null ? LocalDateTime.now() : change.getLastUpdatedOn().toLocalDateTime()); - } catch (OrmException e) { + changeNotes == null + ? LocalDateTime.now() + : changeNotes.getChange().getLastUpdatedOn().toLocalDateTime()); + } catch (Exception e) { log.warn("Unable to update the latest TS for change {}", id, e); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ReindexRunnable.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ReindexRunnable.java index fe76d71..df3a0fd 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ReindexRunnable.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/ReindexRunnable.java
@@ -19,8 +19,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; -import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import java.sql.Timestamp; import java.time.LocalDateTime; @@ -105,7 +103,7 @@ return ts2; } - protected abstract ResultSet<T> fetchItems(ReviewDb db) throws OrmException; + protected abstract Iterable<T> fetchItems(ReviewDb db) throws Exception; protected abstract Optional<Timestamp> indexIfNeeded(ReviewDb db, T item, Timestamp sinceTs); }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java index 57b23dd..b3696fb 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java
@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.notedb.ChangeNotes.Factory; +import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; import com.google.inject.Provider; @@ -31,10 +32,17 @@ ForwardedAwareEventBroker( DynamicSet<UserScopedEventListener> listeners, DynamicSet<EventListener> unrestrictedListeners, + PermissionBackend permissionBackend, ProjectCache projectCache, Factory notesFactory, Provider<ReviewDb> dbProvider) { - super(listeners, unrestrictedListeners, projectCache, notesFactory, dbProvider); + super( + listeners, + unrestrictedListeners, + permissionBackend, + projectCache, + notesFactory, + dbProvider); } @Override
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedEventHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedEventHandler.java index 6a0a672..4e01abf 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedEventHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedEventHandler.java
@@ -16,6 +16,7 @@ import com.google.gerrit.common.EventDispatcher; import com.google.gerrit.server.events.Event; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -44,7 +45,7 @@ * @param event The event to dispatch * @throws OrmException If an error occur while retrieving the change the event belongs to. */ - public void dispatch(Event event) throws OrmException { + public void dispatch(Event event) throws OrmException, PermissionBackendException { try { Context.setForwardedEvent(true); log.debug("dispatching event {}", event.getType());
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandler.java index c891eba..649ffc8 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandler.java
@@ -20,6 +20,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; +import java.util.Optional; /** * Index an account using {@link AccountIndexer}. This class is meant to be used on the receiving @@ -38,13 +39,13 @@ } @Override - protected void doIndex(Account.Id id) throws IOException { + protected void doIndex(Account.Id id, Optional<IndexEvent> indexEvent) throws IOException { indexer.index(id); log.debug("Account {} successfully indexed", id); } @Override - protected void doDelete(Account.Id id) { + protected void doDelete(Account.Id id, Optional<IndexEvent> indexEvent) { throw new UnsupportedOperationException("Delete from account index not supported"); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java index 221ab2f..d34959c 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
@@ -15,16 +15,26 @@ package com.ericsson.gerrit.plugins.highavailability.forwarder; import com.ericsson.gerrit.plugins.highavailability.Configuration; +import com.ericsson.gerrit.plugins.highavailability.Configuration.Index; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeChecker; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeCheckerImpl; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeDb; +import com.ericsson.gerrit.plugins.highavailability.index.ForwardedIndexExecutor; +import com.google.common.base.Splitter; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.Change.Id; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.index.change.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.util.ManualRequestContext; +import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; +import java.util.Optional; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; /** * Index a change using {@link ChangeIndexer}. This class is meant to be used on the receiving side @@ -33,45 +43,126 @@ * done for the same change id */ @Singleton -public class ForwardedIndexChangeHandler extends ForwardedIndexingHandler<Change.Id> { +public class ForwardedIndexChangeHandler extends ForwardedIndexingHandler<String> { private final ChangeIndexer indexer; - private final SchemaFactory<ReviewDb> schemaFactory; + private final ChangeDb changeDb; + private final ScheduledExecutorService indexExecutor; + private final OneOffRequestContext oneOffCtx; + private final int retryInterval; + private final int maxTries; + private final ChangeCheckerImpl.Factory changeCheckerFactory; @Inject ForwardedIndexChangeHandler( - ChangeIndexer indexer, SchemaFactory<ReviewDb> schemaFactory, Configuration config) { + ChangeIndexer indexer, + ChangeDb changeDb, + Configuration config, + @ForwardedIndexExecutor ScheduledExecutorService indexExecutor, + OneOffRequestContext oneOffCtx, + ChangeCheckerImpl.Factory changeCheckerFactory) { super(config.index()); this.indexer = indexer; - this.schemaFactory = schemaFactory; + this.changeDb = changeDb; + this.indexExecutor = indexExecutor; + this.oneOffCtx = oneOffCtx; + this.changeCheckerFactory = changeCheckerFactory; + + Index indexConfig = config.index(); + this.retryInterval = indexConfig != null ? indexConfig.retryInterval() : 0; + this.maxTries = indexConfig != null ? indexConfig.maxTries() : 0; } @Override - protected void doIndex(Change.Id id) throws IOException, OrmException { - Change change = null; - try (ReviewDb db = schemaFactory.open()) { - change = db.changes().get(id); - if (change != null) { - indexer.index(db, change); - log.debug("Change {} successfully indexed", id); + protected void doIndex(String id, Optional<IndexEvent> indexEvent) + throws IOException, OrmException { + doIndex(id, indexEvent, 0); + } + + private void doIndex(String id, Optional<IndexEvent> indexEvent, int retryCount) + throws IOException, OrmException { + try { + ChangeChecker checker = changeCheckerFactory.create(id); + Optional<ChangeNotes> changeNotes = checker.getChangeNotes(); + if (changeNotes.isPresent()) { + ChangeNotes notes = changeNotes.get(); + reindex(notes); + + if (checker.isChangeUpToDate(indexEvent)) { + if (retryCount > 0) { + log.warn("Change {} has been eventually indexed after {} attempt(s)", id, retryCount); + } else { + log.debug("Change {} successfully indexed", id); + } + } else { + log.warn( + "Change {} seems too old compared to the event timestamp (event-Ts={} >> change-Ts={})", + id, + indexEvent, + checker); + rescheduleIndex(id, indexEvent, retryCount + 1); + } + } else { + indexer.delete(parseChangeId(id)); + log.warn( + "Change {} could not be found in the local Git repository (eventTs={}), deleted from index", + id, + indexEvent); } } catch (Exception e) { - if (!isCausedByNoSuchChangeException(e)) { - throw e; + if (isCausedByNoSuchChangeException(e)) { + indexer.delete(parseChangeId(id)); + log.warn("Error trying to index Change {}. Deleted from index", id, e); + return; } - log.debug("Change {} was deleted, aborting forwarded indexing the change.", id.get()); - } - if (change == null) { - indexer.delete(id); - log.debug("Change {} not found, deleted from index", id); + + throw e; } } + private void reindex(ChangeNotes notes) throws IOException, OrmException { + try (ReviewDb db = changeDb.open()) { + notes.reload(); + indexer.index(db, notes.getChange()); + } + } + + private void rescheduleIndex(String id, Optional<IndexEvent> indexEvent, int retryCount) { + if (retryCount > maxTries) { + log.error( + "Change {} could not be indexed after {} retries. Change index could be stale.", + id, + retryCount); + return; + } + + log.warn( + "Retrying for the #{} time to index Change {} after {} msecs", + retryCount, + id, + retryInterval); + indexExecutor.schedule( + () -> { + try (ManualRequestContext ctx = oneOffCtx.open()) { + Context.setForwardedEvent(true); + doIndex(id, indexEvent, retryCount); + } catch (Exception e) { + log.warn("Change {} could not be indexed", id, e); + } + }, + retryInterval, + TimeUnit.MILLISECONDS); + } + @Override - protected void doDelete(Id id) throws IOException { - indexer.delete(id); + protected void doDelete(String id, Optional<IndexEvent> indexEvent) throws IOException { + indexer.delete(parseChangeId(id)); log.debug("Change {} successfully deleted from index", id); } + private static Change.Id parseChangeId(String id) { + return new Change.Id(Integer.parseInt(Splitter.on("~").splitToList(id).get(1))); + } + private static boolean isCausedByNoSuchChangeException(Throwable throwable) { Throwable cause = throwable; while (cause != null) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandler.java index 9ff7f9e..7c6d723 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandler.java
@@ -20,6 +20,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; +import java.util.Optional; /** * Index a group using {@link GroupIndexer}. This class is meant to be used on the receiving side of @@ -38,13 +39,14 @@ } @Override - protected void doIndex(AccountGroup.UUID uuid) throws IOException { + protected void doIndex(AccountGroup.UUID uuid, Optional<IndexEvent> indexEvent) + throws IOException { indexer.index(uuid); log.debug("Group {} successfully indexed", uuid); } @Override - protected void doDelete(AccountGroup.UUID uuid) { + protected void doDelete(AccountGroup.UUID uuid, Optional<IndexEvent> indexEvent) { throw new UnsupportedOperationException("Delete from group index not supported"); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexingHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexingHandler.java index 3fd0e24..bfedf3f 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexingHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexingHandler.java
@@ -18,6 +18,7 @@ import com.google.common.util.concurrent.Striped; import com.google.gwtorm.server.OrmException; import java.io.IOException; +import java.util.Optional; import java.util.concurrent.locks.Lock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,9 +44,10 @@ private final Striped<Lock> idLocks; - protected abstract void doIndex(T id) throws IOException, OrmException; + protected abstract void doIndex(T id, Optional<IndexEvent> indexEvent) + throws IOException, OrmException; - protected abstract void doDelete(T id) throws IOException; + protected abstract void doDelete(T id, Optional<IndexEvent> indexEvent) throws IOException; protected ForwardedIndexingHandler(Configuration.Index indexConfig) { idLocks = Striped.lock(indexConfig.numStripedLocks()); @@ -56,11 +58,13 @@ * * @param id The id to index. * @param operation The operation to do; index or delete + * @param indexEvent The index event details. * @throws IOException If an error occur while indexing. * @throws OrmException If an error occur while retrieving a change related to the item to index */ - public void index(T id, Operation operation) throws IOException, OrmException { - log.debug("{} {}", operation, id); + public void index(T id, Operation operation, Optional<IndexEvent> indexEvent) + throws IOException, OrmException { + log.debug("{} {} {}", operation, id, indexEvent); try { Context.setForwardedEvent(true); Lock idLock = idLocks.get(id); @@ -68,10 +72,10 @@ try { switch (operation) { case INDEX: - doIndex(id); + doIndex(id, indexEvent); break; case DELETE: - doDelete(id); + doDelete(id, indexEvent); break; default: log.error("unexpected operation: {}", operation);
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/Forwarder.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/Forwarder.java index 12449de..b156563 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/Forwarder.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/Forwarder.java
@@ -23,33 +23,38 @@ * Forward a account indexing event to the other master. * * @param accountId the account to index. + * @param indexEvent the details of the index event. * @return true if successful, otherwise false. */ - boolean indexAccount(int accountId); + boolean indexAccount(int accountId, IndexEvent indexEvent); /** * Forward a change indexing event to the other master. * + * @param projectName the project of the change to index. * @param changeId the change to index. + * @param indexEvent the details of the index event. * @return true if successful, otherwise false. */ - boolean indexChange(int changeId); + boolean indexChange(String projectName, int changeId, IndexEvent indexEvent); /** * Forward a delete change from index event to the other master. * * @param changeId the change to remove from the index. + * @param indexEvent the details of the index event. * @return rue if successful, otherwise false. */ - boolean deleteChangeFromIndex(int changeId); + boolean deleteChangeFromIndex(int changeId, IndexEvent indexEvent); /** * Forward a group indexing event to the other master. * * @param uuid the group to index. + * @param indexEvent the details of the index event. * @return true if successful, otherwise false. */ - boolean indexGroup(String uuid); + boolean indexGroup(String uuid, IndexEvent indexEvent); /** * Forward a stream event to the other master.
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/IndexEvent.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/IndexEvent.java new file mode 100644 index 0000000..037c1c6 --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/IndexEvent.java
@@ -0,0 +1,34 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.forwarder; + +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; + +public class IndexEvent { + public long eventCreatedOn = System.currentTimeMillis() / 1000; + public String targetSha; + + @Override + public String toString() { + return "IndexEvent@" + format(eventCreatedOn) + ((targetSha != null) ? "/" + targetSha : ""); + } + + public static String format(long eventTs) { + return LocalDateTime.ofEpochSecond(eventTs, 0, ZoneOffset.UTC) + .format(DateTimeFormatter.ISO_DATE_TIME); + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java index a6e9395..8c429de 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java
@@ -21,13 +21,20 @@ import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation; +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; +import com.google.common.base.Charsets; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import com.google.gwtorm.server.OrmException; import java.io.IOException; +import java.io.InputStreamReader; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; public abstract class AbstractIndexRestApiServlet<T> extends AbstractRestApiServlet { private static final long serialVersionUID = -1L; + private static final Gson gson = new GsonBuilder().create(); private final ForwardedIndexingHandler<T> forwardedIndexingHandler; private final IndexName indexName; @@ -77,10 +84,10 @@ private void process(HttpServletRequest req, HttpServletResponse rsp, Operation operation) { setHeaders(rsp); - String path = req.getPathInfo(); + String path = req.getRequestURI(); T id = parse(path.substring(path.lastIndexOf('/') + 1)); try { - forwardedIndexingHandler.index(id, operation); + forwardedIndexingHandler.index(id, operation, parseBody(req)); rsp.setStatus(SC_NO_CONTENT); } catch (IOException e) { sendError(rsp, SC_CONFLICT, e.getMessage()); @@ -91,4 +98,14 @@ log.debug(msg, e); } } + + protected Optional<IndexEvent> parseBody(HttpServletRequest req) throws IOException { + String contentType = req.getContentType(); + if (contentType != null && contentType.contains("application/json")) { + return Optional.ofNullable( + gson.fromJson( + new InputStreamReader(req.getInputStream(), Charsets.UTF_8), IndexEvent.class)); + } + return Optional.empty(); + } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java index 033aaa7..fb49cf2 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
@@ -34,10 +34,13 @@ private static final long serialVersionUID = -1L; private final ForwardedCacheEvictionHandler forwardedCacheEvictionHandler; + private final GsonParser gson; @Inject - CacheRestApiServlet(ForwardedCacheEvictionHandler forwardedCacheEvictionHandler) { + CacheRestApiServlet( + ForwardedCacheEvictionHandler forwardedCacheEvictionHandler, GsonParser gson) { this.forwardedCacheEvictionHandler = forwardedCacheEvictionHandler; + this.gson = gson; } @Override @@ -48,7 +51,7 @@ String cacheName = params.get(CACHENAME_INDEX); String json = req.getReader().readLine(); forwardedCacheEvictionHandler.evict( - CacheEntry.from(cacheName, GsonParser.fromJson(cacheName, json))); + CacheEntry.from(cacheName, gson.fromJson(cacheName, json))); rsp.setStatus(SC_NO_CONTENT); } catch (CacheNotFoundException e) { log.error("Failed to process eviction request: {}", e.getMessage());
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java index fad5cb3..3da5081 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java
@@ -21,14 +21,10 @@ import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedEventHandler; -import com.google.common.base.Supplier; import com.google.common.io.CharStreams; import com.google.common.net.MediaType; import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.EventDeserializer; -import com.google.gerrit.server.events.SupplierDeserializer; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -41,10 +37,12 @@ private static final long serialVersionUID = -1L; private final ForwardedEventHandler forwardedEventHandler; + private final GsonProvider gson; @Inject - EventRestApiServlet(ForwardedEventHandler forwardedEventHandler) { + EventRestApiServlet(ForwardedEventHandler forwardedEventHandler, GsonProvider gson) { this.forwardedEventHandler = forwardedEventHandler; + this.gson = gson; } @Override @@ -60,19 +58,14 @@ } catch (OrmException e) { log.debug("Error trying to find a change ", e); sendError(rsp, SC_NOT_FOUND, "Change not found\n"); - } catch (IOException e) { + } catch (IOException | PermissionBackendException e) { log.error("Unable to re-trigger event", e); sendError(rsp, SC_BAD_REQUEST, e.getMessage()); } } - private static Event getEventFromRequest(HttpServletRequest req) throws IOException { + private Event getEventFromRequest(HttpServletRequest req) throws IOException { String jsonEvent = CharStreams.toString(req.getReader()); - Gson gson = - new GsonBuilder() - .registerTypeAdapter(Event.class, new EventDeserializer()) - .registerTypeAdapter(Supplier.class, new SupplierDeserializer()) - .create(); - return gson.fromJson(jsonEvent, Event.class); + return gson.get().fromJson(jsonEvent, Event.class); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java index 974f20b..8db2488 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
@@ -19,14 +19,19 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gson.Gson; -import com.google.gson.GsonBuilder; +import com.google.inject.Inject; +import com.google.inject.Singleton; -final class GsonParser { +@Singleton +class GsonParser { + private final Gson gson; - private GsonParser() {} + @Inject + GsonParser(GsonProvider gson) { + this.gson = gson.get(); + } - static Object fromJson(String cacheName, String json) { - Gson gson = new GsonBuilder().create(); + public Object fromJson(String cacheName, String json) { Object key; // Need to add a case for 'adv_bases' switch (cacheName) { @@ -52,26 +57,4 @@ } return key; } - - static String toJson(String cacheName, Object key) { - Gson gson = new GsonBuilder().create(); - String json; - // Need to add a case for 'adv_bases' - switch (cacheName) { - case Constants.ACCOUNTS: - json = gson.toJson(key, Account.Id.class); - break; - case Constants.GROUPS: - json = gson.toJson(key, AccountGroup.Id.class); - break; - case Constants.GROUPS_BYINCLUDE: - case Constants.GROUPS_MEMBERS: - json = gson.toJson(key, AccountGroup.UUID.class); - break; - case Constants.PROJECT_LIST: - default: - json = gson.toJson(key); - } - return json; - } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java new file mode 100644 index 0000000..e62cec2 --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java
@@ -0,0 +1,34 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.ericsson.gerrit.plugins.highavailability.forwarder.rest; + +import com.google.common.base.Supplier; +import com.google.gerrit.server.events.Event; +import com.google.gerrit.server.events.EventDeserializer; +import com.google.gerrit.server.events.SupplierDeserializer; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.inject.Provider; + +public class GsonProvider implements Provider<Gson> { + + @Override + public Gson get() { + return new GsonBuilder() + .registerTypeAdapter(Event.class, new EventDeserializer()) + .registerTypeAdapter(Supplier.class, new SupplierDeserializer()) + .create(); + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSession.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSession.java index d9bb352..f2ac080 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSession.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSession.java
@@ -15,38 +15,73 @@ package com.ericsson.gerrit.plugins.highavailability.forwarder.rest; import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.HttpResponseHandler.HttpResult; -import com.google.common.base.Strings; +import com.google.common.base.Supplier; import com.google.common.net.MediaType; +import com.google.gerrit.server.events.SupplierSerializer; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import com.google.inject.Inject; import java.io.IOException; +import java.net.URI; import java.nio.charset.StandardCharsets; import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; import org.apache.http.client.methods.HttpPost; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; class HttpSession { private final CloseableHttpClient httpClient; + private final Gson gson = + new GsonBuilder().registerTypeAdapter(Supplier.class, new SupplierSerializer()).create(); @Inject HttpSession(CloseableHttpClient httpClient) { this.httpClient = httpClient; } - HttpResult post(String endpoint) throws IOException { - return post(endpoint, null); + HttpResult post(String uri) throws IOException { + return post(uri, null); } - HttpResult post(String uri, String content) throws IOException { + HttpResult post(String uri, Object content) throws IOException { HttpPost post = new HttpPost(uri); - if (!Strings.isNullOrEmpty(content)) { - post.addHeader("Content-Type", MediaType.JSON_UTF_8.toString()); - post.setEntity(new StringEntity(content, StandardCharsets.UTF_8)); - } + setContent(post, content); return httpClient.execute(post, new HttpResponseHandler()); } HttpResult delete(String uri) throws IOException { - return httpClient.execute(new HttpDelete(uri), new HttpResponseHandler()); + return delete(uri, null); + } + + HttpResult delete(String uri, Object content) throws IOException { + HttpDeleteWithBody delete = new HttpDeleteWithBody(uri); + setContent(delete, content); + return httpClient.execute(delete, new HttpResponseHandler()); + } + + private void setContent(HttpEntityEnclosingRequestBase request, Object content) { + if (content != null) { + request.addHeader("Content-Type", MediaType.JSON_UTF_8.toString()); + request.setEntity(new StringEntity(jsonEncode(content), StandardCharsets.UTF_8)); + } + } + + private String jsonEncode(Object content) { + if (content instanceof String) { + return (String) content; + } + return gson.toJson(content); + } + + private class HttpDeleteWithBody extends HttpEntityEnclosingRequestBase { + @Override + public String getMethod() { + return HttpDelete.METHOD_NAME; + } + + private HttpDeleteWithBody(String uri) { + setURI(URI.create(uri)); + } } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java index a702478..046611e 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java
@@ -15,12 +15,12 @@ package com.ericsson.gerrit.plugins.highavailability.forwarder.rest; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexChangeHandler; -import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.extensions.restapi.Url; import com.google.inject.Inject; import com.google.inject.Singleton; @Singleton -class IndexChangeRestApiServlet extends AbstractIndexRestApiServlet<Change.Id> { +class IndexChangeRestApiServlet extends AbstractIndexRestApiServlet<String> { private static final long serialVersionUID = -1L; @Inject @@ -29,7 +29,7 @@ } @Override - Change.Id parse(String id) { - return new Change.Id(Integer.parseInt(id)); + String parse(String id) { + return Url.decode(id); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java index 8f933e6..ef1b26d 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java
@@ -17,15 +17,13 @@ import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.ericsson.gerrit.plugins.highavailability.cache.Constants; import com.ericsson.gerrit.plugins.highavailability.forwarder.Forwarder; +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.HttpResponseHandler.HttpResult; import com.ericsson.gerrit.plugins.highavailability.peers.PeerInfo; import com.google.common.base.Joiner; -import com.google.common.base.Supplier; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.SupplierSerializer; -import com.google.gson.GsonBuilder; import com.google.inject.Inject; import com.google.inject.Provider; import java.io.IOException; @@ -51,52 +49,65 @@ private final String pluginRelativePath; private final Configuration cfg; private final Provider<Set<PeerInfo>> peerInfoProvider; + private final GsonProvider gson; @Inject RestForwarder( HttpSession httpClient, @PluginName String pluginName, Configuration cfg, - Provider<Set<PeerInfo>> peerInfoProvider) { + Provider<Set<PeerInfo>> peerInfoProvider, + GsonProvider gson) { this.httpSession = httpClient; this.pluginRelativePath = Joiner.on("/").join("plugins", pluginName); this.cfg = cfg; this.peerInfoProvider = peerInfoProvider; + this.gson = gson; } @Override - public boolean indexAccount(final int accountId) { - return execute(RequestMethod.POST, "index account", "index/account", accountId); + public boolean indexAccount(final int accountId, IndexEvent event) { + return execute(RequestMethod.POST, "index account", "index/account", accountId, event); } @Override - public boolean indexChange(final int changeId) { - return execute(RequestMethod.POST, "index change", "index/change", changeId); + public boolean indexChange(String projectName, int changeId, IndexEvent event) { + return execute( + RequestMethod.POST, + "index change", + "index/change", + buildIndexEndpoint(projectName, changeId), + event); } @Override - public boolean deleteChangeFromIndex(final int changeId) { - return execute(RequestMethod.DELETE, "delete change", "index/change", changeId); + public boolean deleteChangeFromIndex(final int changeId, IndexEvent event) { + return execute( + RequestMethod.DELETE, "delete change", "index/change", buildIndexEndpoint(changeId), event); } @Override - public boolean indexGroup(String uuid) { - return execute(RequestMethod.POST, "index group", "index/group", uuid); + public boolean indexGroup(final String uuid, IndexEvent event) { + return execute(RequestMethod.POST, "index group", "index/group", uuid, event); + } + + private String buildIndexEndpoint(int changeId) { + return buildIndexEndpoint("", changeId); + } + + private String buildIndexEndpoint(String projectName, int changeId) { + String escapedProjectName = Url.encode(projectName); + return escapedProjectName + '~' + changeId; } @Override public boolean send(final Event event) { - String serializedEvent = - new GsonBuilder() - .registerTypeAdapter(Supplier.class, new SupplierSerializer()) - .create() - .toJson(event); - return execute(RequestMethod.POST, "send event", "event", event.type, serializedEvent); + return execute(RequestMethod.POST, "send event", "event", event.type, event); } @Override public boolean evict(final String cacheName, final Object key) { - String json = GsonParser.toJson(cacheName, key); + String json = gson.get().toJson(key); return execute(RequestMethod.POST, "invalidate cache " + cacheName, "cache", cacheName, json); } @@ -127,7 +138,7 @@ } private boolean execute( - RequestMethod method, String action, String endpoint, Object id, String payload) { + RequestMethod method, String action, String endpoint, Object id, Object payload) { List<CompletableFuture<Boolean>> futures = peerInfoProvider.get().stream() .map(peer -> createRequest(method, peer, action, endpoint, id, payload)) @@ -143,7 +154,7 @@ String action, String endpoint, Object id, - String payload) { + Object payload) { String destination = peer.getDirectUrl(); return new Request(action, id, destination) { @Override
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderServletModule.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderServletModule.java index 5e98a5c..184157d 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderServletModule.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderServletModule.java
@@ -20,7 +20,7 @@ @Override protected void configureServlets() { serveRegex("/index/account/\\d+$").with(IndexAccountRestApiServlet.class); - serveRegex("/index/change/\\d+$").with(IndexChangeRestApiServlet.class); + serveRegex("/index/change/.*$").with(IndexChangeRestApiServlet.class); serveRegex("/index/group/\\w+$").with(IndexGroupRestApiServlet.class); serve("/event/*").with(EventRestApiServlet.class); serve("/cache/project_list/*").with(ProjectListApiServlet.class);
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServlet.java index b12fdfb..6d98d83 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServlet.java
@@ -14,6 +14,7 @@ package com.ericsson.gerrit.plugins.highavailability.health; +import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; @@ -21,6 +22,7 @@ import com.google.gerrit.extensions.annotations.PluginData; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.permissions.PermissionBackend; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -41,17 +43,22 @@ private static final long serialVersionUID = -1L; private final Provider<CurrentUser> currentUserProvider; + private final PermissionBackend permissionBackend; private final File unhealthyFile; @Inject - HealthServlet(Provider<CurrentUser> currentUserProvider, @PluginData Path pluginDataDir) { + HealthServlet( + Provider<CurrentUser> currentUserProvider, + PermissionBackend permissionBackend, + @PluginData Path pluginDataDir) { this.currentUserProvider = currentUserProvider; + this.permissionBackend = permissionBackend; this.unhealthyFile = pluginDataDir.resolve("unhealthy.txt").toFile(); } @Override protected void doPost(HttpServletRequest req, HttpServletResponse rsp) { - if (!currentUserProvider.get().getCapabilities().canAdministrateServer()) { + if (!permissionBackend.user(currentUserProvider.get()).testOrFalse(ADMINISTRATE_SERVER)) { sendError(rsp, SC_FORBIDDEN); return; } @@ -66,7 +73,7 @@ @Override protected void doDelete(HttpServletRequest req, HttpServletResponse rsp) { - if (!currentUserProvider.get().getCapabilities().canAdministrateServer()) { + if (!permissionBackend.user(currentUserProvider.get()).testOrFalse(ADMINISTRATE_SERVER)) { sendError(rsp, SC_FORBIDDEN); return; }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeChecker.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeChecker.java new file mode 100644 index 0000000..ce04589 --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeChecker.java
@@ -0,0 +1,63 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gwtorm.server.OrmException; +import java.io.IOException; +import java.util.Optional; + +/** Encapsulates the logic of verifying the up-to-date status of a change. */ +public interface ChangeChecker { + + /** + * Return the Change notes read from ReviewDb or NoteDb. + * + * @return notes of the Change + * @throws OrmException if ReviewDb or NoteDb cannot be opened + */ + Optional<ChangeNotes> getChangeNotes() throws OrmException; + + /** + * Create a new index event POJO associated with the current Change. + * + * @return new IndexEvent + * @throws IOException if the current Change cannot read + * @throws OrmException if ReviewDb cannot be opened + */ + Optional<IndexEvent> newIndexEvent() throws IOException, OrmException; + + /** + * Check if the local Change is aligned with the indexEvent received. + * + * @param indexEvent indexing event + * @return true if the local Change is up-to-date, false otherwise. + * @throws IOException if an I/O error occurred while reading the local Change + * @throws OrmException if the local ReviewDb cannot be opened + */ + boolean isChangeUpToDate(Optional<IndexEvent> indexEvent) throws IOException, OrmException; + + /** + * Return the last computed up-to-date Change time-stamp. + * + * <p>Compute the up-to-date Change time-stamp when it is invoked for the very first time. + * + * @return the Change timestamp epoch in seconds + * @throws IOException if an I/O error occurred while reading the local Change + * @throws OrmException if the local ReviewDb cannot be opened + */ + Optional<Long> getComputedChangeTs() throws IOException, OrmException; +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java new file mode 100644 index 0000000..9dca6dd --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java
@@ -0,0 +1,170 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Comment; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeFinder; +import com.google.gerrit.server.CommentsUtil; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.util.ManualRequestContext; +import com.google.gerrit.server.util.OneOffRequestContext; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import java.io.IOException; +import java.sql.Timestamp; +import java.util.Objects; +import java.util.Optional; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ChangeCheckerImpl implements ChangeChecker { + private static final Logger log = LoggerFactory.getLogger(ChangeCheckerImpl.class); + private final GitRepositoryManager gitRepoMgr; + private final CommentsUtil commentsUtil; + private final ChangeDb changeDb; + private final OneOffRequestContext oneOffReqCtx; + private final String changeId; + private final ChangeFinder changeFinder; + private Optional<Long> computedChangeTs = Optional.empty(); + private Optional<ChangeNotes> changeNotes = Optional.empty(); + + public interface Factory { + ChangeChecker create(String changeId); + } + + @Inject + public ChangeCheckerImpl( + GitRepositoryManager gitRepoMgr, + CommentsUtil commentsUtil, + ChangeDb changeDb, + ChangeFinder changeFinder, + OneOffRequestContext oneOffReqCtx, + @Assisted String changeId) { + this.changeFinder = changeFinder; + this.gitRepoMgr = gitRepoMgr; + this.commentsUtil = commentsUtil; + this.changeDb = changeDb; + this.oneOffReqCtx = oneOffReqCtx; + this.changeId = changeId; + } + + @Override + public Optional<IndexEvent> newIndexEvent() throws IOException, OrmException { + return getComputedChangeTs() + .map( + ts -> { + IndexEvent event = new IndexEvent(); + event.eventCreatedOn = ts; + event.targetSha = getBranchTargetSha(); + return event; + }); + } + + @Override + public Optional<ChangeNotes> getChangeNotes() throws OrmException { + try (ManualRequestContext ctx = oneOffReqCtx.open()) { + changeNotes = Optional.ofNullable(changeFinder.findOne(changeId)); + return changeNotes; + } + } + + @Override + public boolean isChangeUpToDate(Optional<IndexEvent> indexEvent) + throws IOException, OrmException { + getComputedChangeTs(); + log.debug("Checking change {} against index event {}", this, indexEvent); + if (!computedChangeTs.isPresent()) { + log.warn("Unable to compute last updated ts for change {}", changeId); + return false; + } + + if (indexEvent.isPresent() && indexEvent.get().targetSha == null) { + return indexEvent.map(e -> (computedChangeTs.get() >= e.eventCreatedOn)).orElse(true); + } + + return indexEvent + .map( + e -> + (computedChangeTs.get() > e.eventCreatedOn) + || (computedChangeTs.get() == e.eventCreatedOn) + && (Objects.equals(getBranchTargetSha(), e.targetSha))) + .orElse(true); + } + + @Override + public Optional<Long> getComputedChangeTs() throws IOException, OrmException { + if (!computedChangeTs.isPresent()) { + computedChangeTs = computeLastChangeTs(); + } + return computedChangeTs; + } + + @Override + public String toString() { + try { + return "change-id=" + + changeId + + "@" + + getComputedChangeTs().map(IndexEvent::format) + + "/" + + getBranchTargetSha(); + } catch (IOException | OrmException e) { + log.error("Unable to render change {}", changeId, e); + return "change-id=" + changeId; + } + } + + private String getBranchTargetSha() { + try (Repository repo = gitRepoMgr.openRepository(changeNotes.get().getProjectName())) { + String refName = changeNotes.get().getChange().getDest().get(); + Ref ref = repo.exactRef(refName); + if (ref == null) { + log.warn("Unable to find target ref {} for change {}", refName, changeId); + return null; + } + return ref.getTarget().getObjectId().getName(); + } catch (IOException e) { + log.warn("Unable to resolve target branch SHA for change {}", changeId, e); + return null; + } + } + + private Optional<Long> computeLastChangeTs() throws OrmException { + try (ReviewDb db = changeDb.open()) { + return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(db, notes)); + } + } + + private long getTsFromChangeAndDraftComments(ReviewDb db, ChangeNotes notes) { + Change change = notes.getChange(); + Timestamp changeTs = change.getLastUpdatedOn(); + try { + for (Comment comment : commentsUtil.draftByChange(db, changeNotes.get())) { + Timestamp commentTs = comment.writtenOn; + changeTs = commentTs.after(changeTs) ? commentTs : changeTs; + } + } catch (OrmException e) { + log.warn("Unable to access draft comments for change {}", change, e); + } + return changeTs.getTime() / 1000; + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeDb.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeDb.java new file mode 100644 index 0000000..bef5363 --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeDb.java
@@ -0,0 +1,38 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.notedb.NotesMigration; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.SchemaFactory; +import com.google.inject.Inject; + +public class ChangeDb { + private static final DisabledReviewDb disabledReviewDb = new DisabledReviewDb(); + + private final NotesMigration migration; + private final SchemaFactory<ReviewDb> schemaFactory; + + @Inject + public ChangeDb(NotesMigration migration, SchemaFactory<ReviewDb> schemaFactory) { + this.migration = migration; + this.schemaFactory = schemaFactory; + } + + public ReviewDb open() throws OrmException { + return migration.readChanges() ? disabledReviewDb : schemaFactory.open(); + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/DisabledReviewDb.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/DisabledReviewDb.java new file mode 100644 index 0000000..192ee8c --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/DisabledReviewDb.java
@@ -0,0 +1,244 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import com.google.common.util.concurrent.CheckedFuture; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Change.Id; +import com.google.gerrit.reviewdb.server.AccountGroupAccess; +import com.google.gerrit.reviewdb.server.AccountGroupByIdAccess; +import com.google.gerrit.reviewdb.server.AccountGroupByIdAudAccess; +import com.google.gerrit.reviewdb.server.AccountGroupMemberAccess; +import com.google.gerrit.reviewdb.server.AccountGroupMemberAuditAccess; +import com.google.gerrit.reviewdb.server.AccountGroupNameAccess; +import com.google.gerrit.reviewdb.server.ChangeAccess; +import com.google.gerrit.reviewdb.server.ChangeMessageAccess; +import com.google.gerrit.reviewdb.server.PatchLineCommentAccess; +import com.google.gerrit.reviewdb.server.PatchSetAccess; +import com.google.gerrit.reviewdb.server.PatchSetApprovalAccess; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.reviewdb.server.SchemaVersionAccess; +import com.google.gerrit.reviewdb.server.SystemConfigAccess; +import com.google.gwtorm.server.Access; +import com.google.gwtorm.server.AtomicUpdate; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; +import com.google.gwtorm.server.StatementExecutor; +import java.util.Map; + +/** ReviewDb that is disabled. */ +@SuppressWarnings("deprecation") +public class DisabledReviewDb implements ReviewDb { + public static class Disabled extends RuntimeException { + private static final long serialVersionUID = 1L; + + private Disabled() { + super("ReviewDb is disabled for changes"); + } + } + + public static class DisabledChangeAccess implements ChangeAccess { + + @Override + public String getRelationName() { + throw new Disabled(); + } + + @Override + public int getRelationID() { + throw new Disabled(); + } + + @Override + public ResultSet<Change> iterateAllEntities() throws OrmException { + throw new Disabled(); + } + + @Override + public Id primaryKey(Change entity) { + throw new Disabled(); + } + + @Override + public Map<Id, Change> toMap(Iterable<Change> c) { + throw new Disabled(); + } + + @Override + public CheckedFuture<Change, OrmException> getAsync(Id key) { + throw new Disabled(); + } + + @Override + public ResultSet<Change> get(Iterable<Id> keys) throws OrmException { + throw new Disabled(); + } + + @Override + public void insert(Iterable<Change> instances) throws OrmException { + throw new Disabled(); + } + + @Override + public void update(Iterable<Change> instances) throws OrmException { + throw new Disabled(); + } + + @Override + public void upsert(Iterable<Change> instances) throws OrmException { + throw new Disabled(); + } + + @Override + public void deleteKeys(Iterable<Id> keys) throws OrmException { + throw new Disabled(); + } + + @Override + public void delete(Iterable<Change> instances) throws OrmException { + throw new Disabled(); + } + + @Override + public void beginTransaction(Id key) throws OrmException { + throw new Disabled(); + } + + @Override + public Change atomicUpdate(Id key, AtomicUpdate<Change> update) throws OrmException { + throw new Disabled(); + } + + @Override + public Change get(Id id) throws OrmException { + return null; + } + + @Override + public ResultSet<Change> all() throws OrmException { + return null; + } + } + + @Override + public void close() { + // Do nothing. + } + + @Override + public void commit() { + throw new Disabled(); + } + + @Override + public void rollback() { + throw new Disabled(); + } + + @Override + public void updateSchema(StatementExecutor e) { + throw new Disabled(); + } + + @Override + public void pruneSchema(StatementExecutor e) { + throw new Disabled(); + } + + @Override + public Access<?, ?>[] allRelations() { + throw new Disabled(); + } + + @Override + public SchemaVersionAccess schemaVersion() { + throw new Disabled(); + } + + @Override + public SystemConfigAccess systemConfig() { + throw new Disabled(); + } + + @Override + public AccountGroupAccess accountGroups() { + throw new Disabled(); + } + + @Override + public AccountGroupNameAccess accountGroupNames() { + throw new Disabled(); + } + + @Override + public AccountGroupMemberAccess accountGroupMembers() { + throw new Disabled(); + } + + @Override + public AccountGroupMemberAuditAccess accountGroupMembersAudit() { + throw new Disabled(); + } + + @Override + public ChangeAccess changes() { + return new DisabledChangeAccess(); + } + + @Override + public PatchSetApprovalAccess patchSetApprovals() { + throw new Disabled(); + } + + @Override + public ChangeMessageAccess changeMessages() { + throw new Disabled(); + } + + @Override + public PatchSetAccess patchSets() { + throw new Disabled(); + } + + @Override + public PatchLineCommentAccess patchComments() { + throw new Disabled(); + } + + @Override + public AccountGroupByIdAccess accountGroupById() { + throw new Disabled(); + } + + @Override + public AccountGroupByIdAudAccess accountGroupByIdAud() { + throw new Disabled(); + } + + @Override + public int nextAccountId() { + throw new Disabled(); + } + + @Override + public int nextAccountGroupId() { + throw new Disabled(); + } + + @Override + public int nextChangeId() { + throw new Disabled(); + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutor.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutor.java new file mode 100644 index 0000000..44c84dc --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutor.java
@@ -0,0 +1,24 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.inject.BindingAnnotation; +import java.lang.annotation.Retention; + +@Retention(RUNTIME) +@BindingAnnotation +public @interface ForwardedIndexExecutor {}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutorProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutorProvider.java new file mode 100644 index 0000000..2112dbe --- /dev/null +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ForwardedIndexExecutorProvider.java
@@ -0,0 +1,30 @@ +// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.index; + +import com.ericsson.gerrit.plugins.highavailability.Configuration; +import com.ericsson.gerrit.plugins.highavailability.ExecutorProvider; +import com.google.gerrit.server.git.WorkQueue; +import com.google.inject.Inject; +import com.google.inject.Singleton; + +@Singleton +class ForwardedIndexExecutorProvider extends ExecutorProvider { + + @Inject + ForwardedIndexExecutorProvider(WorkQueue workQueue, Configuration config) { + super(workQueue, config.index().threadPoolSize(), "Forwarded-Index-Event"); + } +}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandler.java index dc98c97..3922f54 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandler.java
@@ -16,6 +16,7 @@ import com.ericsson.gerrit.plugins.highavailability.forwarder.Context; import com.ericsson.gerrit.plugins.highavailability.forwarder.Forwarder; +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; import com.google.common.base.Objects; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.events.AccountIndexedListener; @@ -26,20 +27,28 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; class IndexEventHandler implements ChangeIndexedListener, AccountIndexedListener, GroupIndexedListener { + private static final Logger log = LoggerFactory.getLogger(IndexEventHandler.class); private final Executor executor; private final Forwarder forwarder; private final String pluginName; private final Set<IndexTask> queuedTasks = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final ChangeCheckerImpl.Factory changeChecker; @Inject IndexEventHandler( - @IndexExecutor Executor executor, @PluginName String pluginName, Forwarder forwarder) { + @IndexExecutor Executor executor, + @PluginName String pluginName, + Forwarder forwarder, + ChangeCheckerImpl.Factory changeChecker) { this.forwarder = forwarder; this.executor = executor; this.pluginName = pluginName; + this.changeChecker = changeChecker; } @Override @@ -53,13 +62,34 @@ } @Override - public void onChangeIndexed(int id) { - executeIndexChangeTask(id, false); + public void onChangeIndexed(String projectName, int id) { + if (!Context.isForwardedEvent()) { + String changeId = projectName + "~" + id; + try { + changeChecker + .create(changeId) + .newIndexEvent() + .map(event -> new IndexChangeTask(projectName, id, event)) + .ifPresent( + task -> { + if (queuedTasks.add(task)) { + executor.execute(task); + } + }); + } catch (Exception e) { + log.warn("Unable to create task to reindex change {}", changeId, e); + } + } } @Override public void onChangeDeleted(int id) { - executeIndexChangeTask(id, true); + if (!Context.isForwardedEvent()) { + DeleteChangeTask task = new DeleteChangeTask(id, new IndexEvent()); + if (queuedTasks.add(task)) { + executor.execute(task); + } + } } @Override @@ -72,16 +102,17 @@ } } - private void executeIndexChangeTask(int id, boolean deleted) { - if (!Context.isForwardedEvent()) { - IndexChangeTask task = new IndexChangeTask(id, deleted); - if (queuedTasks.add(task)) { - executor.execute(task); - } - } - } - abstract class IndexTask implements Runnable { + protected final IndexEvent indexEvent; + + IndexTask() { + indexEvent = new IndexEvent(); + } + + IndexTask(IndexEvent indexEvent) { + this.indexEvent = indexEvent; + } + @Override public void run() { queuedTasks.remove(this); @@ -92,26 +123,23 @@ } class IndexChangeTask extends IndexTask { - private final boolean deleted; private final int changeId; + private final String projectName; - IndexChangeTask(int changeId, boolean deleted) { + IndexChangeTask(String projectName, int changeId, IndexEvent indexEvent) { + super(indexEvent); + this.projectName = projectName; this.changeId = changeId; - this.deleted = deleted; } @Override public void execute() { - if (deleted) { - forwarder.deleteChangeFromIndex(changeId); - } else { - forwarder.indexChange(changeId); - } + forwarder.indexChange(projectName, changeId, indexEvent); } @Override public int hashCode() { - return Objects.hashCode(IndexChangeTask.class, changeId, deleted); + return Objects.hashCode(IndexChangeTask.class, changeId); } @Override @@ -120,7 +148,7 @@ return false; } IndexChangeTask other = (IndexChangeTask) obj; - return changeId == other.changeId && deleted == other.deleted; + return changeId == other.changeId; } @Override @@ -129,6 +157,39 @@ } } + class DeleteChangeTask extends IndexTask { + private final int changeId; + + DeleteChangeTask(int changeId, IndexEvent indexEvent) { + super(indexEvent); + this.changeId = changeId; + } + + @Override + public void execute() { + forwarder.deleteChangeFromIndex(changeId, indexEvent); + } + + @Override + public int hashCode() { + return Objects.hashCode(DeleteChangeTask.class, changeId); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof DeleteChangeTask)) { + return false; + } + DeleteChangeTask other = (DeleteChangeTask) obj; + return changeId == other.changeId; + } + + @Override + public String toString() { + return String.format("[%s] Delete change %s in target instance", pluginName, changeId); + } + } + class IndexAccountTask extends IndexTask { private final int accountId; @@ -138,7 +199,7 @@ @Override public void execute() { - forwarder.indexAccount(accountId); + forwarder.indexAccount(accountId, indexEvent); } @Override @@ -170,7 +231,7 @@ @Override public void execute() { - forwarder.indexGroup(groupUUID); + forwarder.indexGroup(groupUUID, indexEvent); } @Override
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexModule.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexModule.java index f88a806..ebf8fdf 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexModule.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexModule.java
@@ -19,16 +19,26 @@ import com.google.gerrit.extensions.events.GroupIndexedListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.inject.assistedinject.FactoryModuleBuilder; import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; public class IndexModule extends LifecycleModule { @Override protected void configure() { bind(Executor.class).annotatedWith(IndexExecutor.class).toProvider(IndexExecutorProvider.class); + bind(ScheduledExecutorService.class) + .annotatedWith(ForwardedIndexExecutor.class) + .toProvider(ForwardedIndexExecutorProvider.class); listener().to(IndexExecutorProvider.class); DynamicSet.bind(binder(), ChangeIndexedListener.class).to(IndexEventHandler.class); DynamicSet.bind(binder(), AccountIndexedListener.class).to(IndexEventHandler.class); DynamicSet.bind(binder(), GroupIndexedListener.class).to(IndexEventHandler.class); + + install( + new FactoryModuleBuilder() + .implement(ChangeChecker.class, ChangeCheckerImpl.class) + .build(ChangeCheckerImpl.Factory.class)); } }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index a8d4906..bb72501 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -195,6 +195,17 @@ : Maximum number of threads used to send index events to the target instance. Defaults to 4. +```index.maxTries``` +: Maximum number of times the plugin should attempt to reindex changes. + Setting this value to 0 will disable retries. After this number of failed tries, + an error is logged and the local index should be considered stale and needs + to be investigated and manually reindexed. + Defaults to 2. + +```index.retryInterval``` +: The interval of time in milliseconds between the subsequent auto-retries. + Defaults to 30000 (30 seconds). + ```websession.synchronize``` : Whether to synchronize web sessions. Defaults to true.
diff --git a/src/test/docker/etc/gerrit.config b/src/test/docker/etc/gerrit.config index c110835..f7ee7ec 100644 --- a/src/test/docker/etc/gerrit.config +++ b/src/test/docker/etc/gerrit.config
@@ -27,3 +27,5 @@ scheme = http scheme = ssh scheme = anon_http +[plugin "plugin-manager"] + jenkinsUrl = https://archive-ci.gerritforge.com
diff --git a/src/test/docker/gerrit/Dockerfile b/src/test/docker/gerrit/Dockerfile index fce65fb..2580e96 100644 --- a/src/test/docker/gerrit/Dockerfile +++ b/src/test/docker/gerrit/Dockerfile
@@ -1,6 +1,6 @@ -FROM gerritcodereview/gerrit:2.14.20 +FROM gerritcodereview/gerrit:2.15.17 -ENV GERRIT_BRANCH=stable-2.14 +ENV GERRIT_BRANCH=stable-2.15 ENV GERRIT_CI_URL=https://archive-ci.gerritforge.com/job
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheExecutorProviderTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheExecutorProviderTest.java index 3edcc7d..7f7071b 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheExecutorProviderTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheExecutorProviderTest.java
@@ -21,6 +21,7 @@ import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.google.gerrit.server.git.WorkQueue; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,7 +32,7 @@ @RunWith(MockitoJUnitRunner.class) public class CacheExecutorProviderTest { - @Mock private WorkQueue.Executor executorMock; + @Mock private ScheduledThreadPoolExecutor executorMock; private CacheExecutorProvider cacheExecutorProvider; @@ -56,7 +57,6 @@ assertThat(cacheExecutorProvider.get()).isEqualTo(executorMock); cacheExecutorProvider.stop(); verify(executorMock).shutdown(); - verify(executorMock).unregisterWorkQueue(); assertThat(cacheExecutorProvider.get()).isNull(); } }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/event/EventExecutorProviderTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/event/EventExecutorProviderTest.java index b1addcc..6e666ed 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/event/EventExecutorProviderTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/event/EventExecutorProviderTest.java
@@ -20,6 +20,7 @@ import static org.mockito.Mockito.when; import com.google.gerrit.server.git.WorkQueue; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,7 +29,7 @@ @RunWith(MockitoJUnitRunner.class) public class EventExecutorProviderTest { - @Mock private WorkQueue.Executor executorMock; + @Mock private ScheduledThreadPoolExecutor executorMock; private EventExecutorProvider eventsExecutorProvider; @Before @@ -49,7 +50,6 @@ assertThat(eventsExecutorProvider.get()).isEqualTo(executorMock); eventsExecutorProvider.stop(); verify(executorMock).shutdown(); - verify(executorMock).unregisterWorkQueue(); assertThat(eventsExecutorProvider.get()).isNull(); } }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java index 9933838..02b4a47 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java
@@ -35,7 +35,7 @@ listenerMock = mock(EventListener.class); DynamicSet<EventListener> listeners = DynamicSet.emptySet(); listeners.add(listenerMock); - broker = new ForwardedAwareEventBroker(null, listeners, null, null, null); + broker = new ForwardedAwareEventBroker(null, listeners, null, null, null, null); } @Test
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandlerTest.java index 78e8903..c2d3659 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandlerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexAccountHandlerTest.java
@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.index.account.AccountIndexer; import java.io.IOException; +import java.util.Optional; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -54,7 +55,7 @@ @Test public void testSuccessfulIndexing() throws Exception { - handler.index(id, Operation.INDEX); + handler.index(id, Operation.INDEX, Optional.empty()); verify(indexerMock).index(id); } @@ -62,7 +63,7 @@ public void deleteIsNotSupported() throws Exception { exception.expect(UnsupportedOperationException.class); exception.expectMessage("Delete from account index not supported"); - handler.index(id, Operation.DELETE); + handler.index(id, Operation.DELETE, Optional.empty()); } @Test @@ -79,7 +80,7 @@ .index(id); assertThat(Context.isForwardedEvent()).isFalse(); - handler.index(id, Operation.INDEX); + handler.index(id, Operation.INDEX, Optional.empty()); assertThat(Context.isForwardedEvent()).isFalse(); verify(indexerMock).index(id); @@ -98,7 +99,7 @@ assertThat(Context.isForwardedEvent()).isFalse(); try { - handler.index(id, Operation.INDEX); + handler.index(id, Operation.INDEX, Optional.empty()); fail("should have thrown an IOException"); } catch (IOException e) { assertThat(e.getMessage()).isEqualTo("someMessage");
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java index 86b605f..8bfe47d 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -16,24 +16,28 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeChecker; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeCheckerImpl; +import com.ericsson.gerrit.plugins.highavailability.index.ChangeDb; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.server.ChangeAccess; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.index.change.ChangeIndexer; -import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.SchemaFactory; import java.io.IOException; +import java.util.Optional; +import java.util.concurrent.ScheduledExecutorService; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -46,87 +50,97 @@ @RunWith(MockitoJUnitRunner.class) public class ForwardedIndexChangeHandlerTest { + private static final int TEST_CHANGE_NUMBER = 123; + private static String TEST_PROJECT = "test/project"; + private static String TEST_CHANGE_ID = TEST_PROJECT + "~" + TEST_CHANGE_NUMBER; private static final boolean CHANGE_EXISTS = true; private static final boolean CHANGE_DOES_NOT_EXIST = false; private static final boolean DO_NOT_THROW_IO_EXCEPTION = false; private static final boolean DO_NOT_THROW_ORM_EXCEPTION = false; private static final boolean THROW_IO_EXCEPTION = true; private static final boolean THROW_ORM_EXCEPTION = true; + private static final boolean CHANGE_UP_TO_DATE = true; + private static final boolean CHANGE_OUTDATED = false; @Rule public ExpectedException exception = ExpectedException.none(); @Mock private ChangeIndexer indexerMock; - @Mock private SchemaFactory<ReviewDb> schemaFactoryMock; + @Mock private ChangeDb changeDbMock; @Mock private ReviewDb dbMock; - @Mock private ChangeAccess changeAccessMock; + @Mock private ChangeNotes changeNotes; @Mock private Configuration configMock; @Mock private Configuration.Index indexMock; + @Mock private ScheduledExecutorService indexExecutorMock; + @Mock private OneOffRequestContext ctxMock; + @Mock private ChangeCheckerImpl.Factory changeCheckerFactoryMock; + @Mock private ChangeChecker changeCheckerAbsentMock; + @Mock private ChangeChecker changeCheckerPresentMock; private ForwardedIndexChangeHandler handler; private Change.Id id; - private Change change; @Before public void setUp() throws Exception { - when(schemaFactoryMock.open()).thenReturn(dbMock); - when(dbMock.changes()).thenReturn(changeAccessMock); + when(changeDbMock.open()).thenReturn(dbMock); + id = new Change.Id(TEST_CHANGE_NUMBER); + Change change = new Change(null, id, null, null, TimeUtil.nowTs()); + when(changeNotes.getChange()).thenReturn(change); when(configMock.index()).thenReturn(indexMock); when(indexMock.numStripedLocks()).thenReturn(10); - id = new Change.Id(123); - change = new Change(null, id, null, null, TimeUtil.nowTs()); - handler = new ForwardedIndexChangeHandler(indexerMock, schemaFactoryMock, configMock); + when(changeCheckerFactoryMock.create(any())).thenReturn(changeCheckerAbsentMock); + handler = + new ForwardedIndexChangeHandler( + indexerMock, + changeDbMock, + configMock, + indexExecutorMock, + ctxMock, + changeCheckerFactoryMock); } @Test - public void changeIsIndexed() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_EXISTS); - handler.index(id, Operation.INDEX); - verify(indexerMock, times(1)).index(dbMock, change); + public void changeIsIndexedWhenUpToDate() throws Exception { + setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_UP_TO_DATE); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); + verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class)); + } + + @Test + public void changeIsStillIndexedEvenWhenOutdated() throws Exception { + setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_OUTDATED); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.of(new IndexEvent())); + verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class)); } @Test public void changeIsDeletedFromIndex() throws Exception { - handler.index(id, Operation.DELETE); + handler.index(TEST_CHANGE_ID, Operation.DELETE, Optional.empty()); verify(indexerMock, times(1)).delete(id); } @Test public void changeToIndexDoesNotExist() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_DOES_NOT_EXIST); - handler.index(id, Operation.INDEX); + setupChangeAccessRelatedMocks(CHANGE_DOES_NOT_EXIST, CHANGE_OUTDATED); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); verify(indexerMock, times(1)).delete(id); } @Test public void schemaThrowsExceptionWhenLookingUpForChange() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_ORM_EXCEPTION); + setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_ORM_EXCEPTION, CHANGE_UP_TO_DATE); exception.expect(OrmException.class); - handler.index(id, Operation.INDEX); - } - - @Test - public void indexerThrowsNoSuchChangeExceptionTryingToPostChange() throws Exception { - doThrow(new NoSuchChangeException(id)).when(schemaFactoryMock).open(); - handler.index(id, Operation.INDEX); - verify(indexerMock, times(1)).delete(id); - } - - @Test - public void indexerThrowsNestedNoSuchChangeExceptionTryingToPostChange() throws Exception { - OrmException e = new OrmException("test", new NoSuchChangeException(id)); - doThrow(e).when(schemaFactoryMock).open(); - handler.index(id, Operation.INDEX); - verify(indexerMock, times(1)).delete(id); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); } @Test public void indexerThrowsIOExceptionTryingToIndexChange() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_EXISTS, DO_NOT_THROW_ORM_EXCEPTION, THROW_IO_EXCEPTION); + setupChangeAccessRelatedMocks( + CHANGE_EXISTS, DO_NOT_THROW_ORM_EXCEPTION, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE); exception.expect(IOException.class); - handler.index(id, Operation.INDEX); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); } @Test public void shouldSetAndUnsetForwardedContext() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_EXISTS); + setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_UP_TO_DATE); // this doAnswer is to allow to assert that context is set to forwarded // while cache eviction is called. doAnswer( @@ -136,18 +150,18 @@ return null; }) .when(indexerMock) - .index(dbMock, change); + .index(any(ReviewDb.class), any(Change.class)); assertThat(Context.isForwardedEvent()).isFalse(); - handler.index(id, Operation.INDEX); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); assertThat(Context.isForwardedEvent()).isFalse(); - verify(indexerMock, times(1)).index(dbMock, change); + verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class)); } @Test public void shouldSetAndUnsetForwardedContextEvenIfExceptionIsThrown() throws Exception { - setupChangeAccessRelatedMocks(CHANGE_EXISTS); + setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_UP_TO_DATE); doAnswer( (Answer<Void>) invocation -> { @@ -155,47 +169,52 @@ throw new IOException("someMessage"); }) .when(indexerMock) - .index(dbMock, change); + .index(any(ReviewDb.class), any(Change.class)); assertThat(Context.isForwardedEvent()).isFalse(); try { - handler.index(id, Operation.INDEX); + handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()); fail("should have thrown an IOException"); } catch (IOException e) { assertThat(e.getMessage()).isEqualTo("someMessage"); } assertThat(Context.isForwardedEvent()).isFalse(); - verify(indexerMock, times(1)).index(dbMock, change); + verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class)); } - private void setupChangeAccessRelatedMocks(boolean changeExist) throws Exception { + private void setupChangeAccessRelatedMocks(boolean changeExist, boolean changeUpToDate) + throws Exception { setupChangeAccessRelatedMocks( - changeExist, DO_NOT_THROW_ORM_EXCEPTION, DO_NOT_THROW_IO_EXCEPTION); - } - - private void setupChangeAccessRelatedMocks(boolean changeExist, boolean ormException) - throws OrmException, IOException { - setupChangeAccessRelatedMocks(changeExist, ormException, DO_NOT_THROW_IO_EXCEPTION); + changeExist, DO_NOT_THROW_ORM_EXCEPTION, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate); } private void setupChangeAccessRelatedMocks( - boolean changeExists, boolean ormException, boolean ioException) + boolean changeExist, boolean ormException, boolean changeUpToDate) + throws OrmException, IOException { + setupChangeAccessRelatedMocks( + changeExist, ormException, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate); + } + + private void setupChangeAccessRelatedMocks( + boolean changeExists, boolean ormException, boolean ioException, boolean changeIsUpToDate) throws OrmException, IOException { if (ormException) { - doThrow(new OrmException("")).when(schemaFactoryMock).open(); + doThrow(new OrmException("")).when(changeDbMock).open(); } else { - when(schemaFactoryMock.open()).thenReturn(dbMock); - ChangeAccess ca = mock(ChangeAccess.class); - when(dbMock.changes()).thenReturn(ca); - if (changeExists) { - when(ca.get(id)).thenReturn(change); - if (ioException) { - doThrow(new IOException("io-error")).when(indexerMock).index(dbMock, change); - } - } else { - when(ca.get(id)).thenReturn(null); + when(changeDbMock.open()).thenReturn(dbMock); + } + + if (changeExists) { + when(changeCheckerFactoryMock.create(TEST_CHANGE_ID)).thenReturn(changeCheckerPresentMock); + when(changeCheckerPresentMock.getChangeNotes()).thenReturn(Optional.of(changeNotes)); + if (ioException) { + doThrow(new IOException("io-error")) + .when(indexerMock) + .index(any(ReviewDb.class), any(Change.class)); } } + + when(changeCheckerPresentMock.isChangeUpToDate(any())).thenReturn(changeIsUpToDate); } }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandlerTest.java index 43c8824..ab55b73 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandlerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexGroupHandlerTest.java
@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.index.group.GroupIndexer; import java.io.IOException; +import java.util.Optional; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -54,7 +55,7 @@ @Test public void testSuccessfulIndexing() throws Exception { - handler.index(uuid, Operation.INDEX); + handler.index(uuid, Operation.INDEX, Optional.empty()); verify(indexerMock).index(uuid); } @@ -62,7 +63,7 @@ public void deleteIsNotSupported() throws Exception { exception.expect(UnsupportedOperationException.class); exception.expectMessage("Delete from group index not supported"); - handler.index(uuid, Operation.DELETE); + handler.index(uuid, Operation.DELETE, Optional.empty()); } @Test @@ -79,7 +80,7 @@ .index(uuid); assertThat(Context.isForwardedEvent()).isFalse(); - handler.index(uuid, Operation.INDEX); + handler.index(uuid, Operation.INDEX, Optional.empty()); assertThat(Context.isForwardedEvent()).isFalse(); verify(indexerMock).index(uuid); @@ -98,7 +99,7 @@ assertThat(Context.isForwardedEvent()).isFalse(); try { - handler.index(uuid, Operation.INDEX); + handler.index(uuid, Operation.INDEX, Optional.empty()); fail("should have thrown an IOException"); } catch (IOException e) { assertThat(e.getMessage()).isEqualTo("someMessage");
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java index 29f63fd..8b08d24 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
@@ -44,7 +44,9 @@ @Before public void setUp() { - servlet = new CacheRestApiServlet(forwardedCacheEvictionHandlerMock); + servlet = + new CacheRestApiServlet( + forwardedCacheEvictionHandlerMock, new GsonParser(new GsonProvider())); } @Test
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java index e8747b7..835977f 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java
@@ -58,7 +58,7 @@ @Before public void createEventsRestApiServlet() throws Exception { - eventRestApiServlet = new EventRestApiServlet(forwardedEventHandlerMock); + eventRestApiServlet = new EventRestApiServlet(forwardedEventHandlerMock, new GsonProvider()); when(requestMock.getContentType()).thenReturn(MediaType.JSON_UTF_8.toString()); }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java index b4f4dac..2f6ce7e 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
@@ -19,43 +19,47 @@ import com.ericsson.gerrit.plugins.highavailability.cache.Constants; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gson.Gson; import org.junit.Test; public class GsonParserTest { private static final Object EMPTY_JSON = "{}"; + private final Gson gson = new Gson(); + private final GsonParser objectUnderTest = new GsonParser(new GsonProvider()); @Test public void accountIDParse() { Account.Id accountId = new Account.Id(1); - String json = GsonParser.toJson(Constants.ACCOUNTS, accountId); - assertThat(accountId).isEqualTo(GsonParser.fromJson(Constants.ACCOUNTS, json)); + String json = gson.toJson(accountId); + assertThat(accountId).isEqualTo(objectUnderTest.fromJson(Constants.ACCOUNTS, json)); } @Test public void accountGroupIDParse() { AccountGroup.Id accountGroupId = new AccountGroup.Id(1); - String json = GsonParser.toJson(Constants.GROUPS, accountGroupId); - assertThat(accountGroupId).isEqualTo(GsonParser.fromJson(Constants.GROUPS, json)); + String json = gson.toJson(accountGroupId); + assertThat(accountGroupId).isEqualTo(objectUnderTest.fromJson(Constants.GROUPS, json)); } @Test public void accountGroupUUIDParse() { AccountGroup.UUID accountGroupUuid = new AccountGroup.UUID("abc123"); - String json = GsonParser.toJson(Constants.GROUPS_BYINCLUDE, accountGroupUuid); - assertThat(accountGroupUuid).isEqualTo(GsonParser.fromJson(Constants.GROUPS_BYINCLUDE, json)); + String json = gson.toJson(accountGroupUuid); + assertThat(accountGroupUuid) + .isEqualTo(objectUnderTest.fromJson(Constants.GROUPS_BYINCLUDE, json)); } @Test public void stringParse() { String key = "key"; - String json = GsonParser.toJson(Constants.PROJECTS, key); - assertThat(key).isEqualTo(GsonParser.fromJson(Constants.PROJECTS, json)); + String json = gson.toJson(key); + assertThat(key).isEqualTo(objectUnderTest.fromJson(Constants.PROJECTS, json)); } @Test public void noKeyParse() { Object object = new Object(); - String json = GsonParser.toJson(Constants.PROJECT_LIST, object); + String json = gson.toJson(object); assertThat(json).isEqualTo(EMPTY_JSON); } }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSessionTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSessionTest.java index 9b3c85c..5e0d4c9 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSessionTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/HttpSessionTest.java
@@ -56,14 +56,13 @@ @Rule public WireMockRule wireMockRule = new WireMockRule(0); - private Configuration configMock; private String uri; @Before public void setUp() throws Exception { String url = "http://localhost:" + wireMockRule.port(); uri = url + ENDPOINT; - configMock = mock(Configuration.class, Answers.RETURNS_DEEP_STUBS); + Configuration configMock = mock(Configuration.class, Answers.RETURNS_DEEP_STUBS); when(configMock.http().user()).thenReturn("user"); when(configMock.http().password()).thenReturn("pass"); when(configMock.http().maxTries()).thenReturn(MAX_TRIES);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java index b740293..4622a17 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java
@@ -17,6 +17,8 @@ import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -50,13 +52,14 @@ public void setUpMocks() { servlet = new IndexAccountRestApiServlet(handlerMock); id = new Account.Id(ACCOUNT_NUMBER); - when(requestMock.getPathInfo()).thenReturn("/index/account/" + ACCOUNT_NUMBER); + when(requestMock.getRequestURI()) + .thenReturn("http://gerrit.com/index/account/" + ACCOUNT_NUMBER); } @Test public void accountIsIndexed() throws Exception { servlet.doPost(requestMock, responseMock); - verify(handlerMock, times(1)).index(id, Operation.INDEX); + verify(handlerMock, times(1)).index(eq(id), eq(Operation.INDEX), any()); verify(responseMock).setStatus(SC_NO_CONTENT); } @@ -68,14 +71,14 @@ @Test public void indexerThrowsIOExceptionTryingToIndexAccount() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(id, Operation.INDEX); + doThrow(new IOException(IO_ERROR)).when(handlerMock).index(eq(id), eq(Operation.INDEX), any()); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR); } @Test public void sendErrorThrowsIOException() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(id, Operation.INDEX); + doThrow(new IOException(IO_ERROR)).when(handlerMock).index(eq(id), eq(Operation.INDEX), any()); doThrow(new IOException("someError")).when(responseMock).sendError(SC_CONFLICT, IO_ERROR); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServletTest.java index 811df97..c1fa765 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServletTest.java
@@ -17,6 +17,8 @@ import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -24,7 +26,6 @@ import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexChangeHandler; import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedIndexingHandler.Operation; -import com.google.gerrit.reviewdb.client.Change; import com.google.gwtorm.server.OrmException; import java.io.IOException; import javax.servlet.http.HttpServletRequest; @@ -38,53 +39,61 @@ @RunWith(MockitoJUnitRunner.class) public class IndexChangeRestApiServletTest { private static final int CHANGE_NUMBER = 1; + private static final String PROJECT_NAME = "test/project"; + private static final String PROJECT_NAME_URL_ENC = "test%2Fproject"; + private static final String CHANGE_ID = PROJECT_NAME + "~" + CHANGE_NUMBER; private static final String IO_ERROR = "io-error"; @Mock private ForwardedIndexChangeHandler handlerMock; @Mock private HttpServletRequest requestMock; @Mock private HttpServletResponse responseMock; - private Change.Id id; private IndexChangeRestApiServlet servlet; @Before public void setUpMocks() { servlet = new IndexChangeRestApiServlet(handlerMock); - id = new Change.Id(CHANGE_NUMBER); - when(requestMock.getPathInfo()).thenReturn("/index/change/" + CHANGE_NUMBER); + when(requestMock.getRequestURI()) + .thenReturn("http://gerrit.com/index/change/" + PROJECT_NAME_URL_ENC + "~" + CHANGE_NUMBER); } @Test public void changeIsIndexed() throws Exception { servlet.doPost(requestMock, responseMock); - verify(handlerMock, times(1)).index(id, Operation.INDEX); + verify(handlerMock, times(1)).index(eq(CHANGE_ID), eq(Operation.INDEX), any()); verify(responseMock).setStatus(SC_NO_CONTENT); } @Test public void changeIsDeletedFromIndex() throws Exception { servlet.doDelete(requestMock, responseMock); - verify(handlerMock, times(1)).index(id, Operation.DELETE); + verify(handlerMock, times(1)).index(eq(CHANGE_ID), eq(Operation.DELETE), any()); verify(responseMock).setStatus(SC_NO_CONTENT); } @Test public void indexerThrowsIOExceptionTryingToIndexChange() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(id, Operation.INDEX); + doThrow(new IOException(IO_ERROR)) + .when(handlerMock) + .index(eq(CHANGE_ID), eq(Operation.INDEX), any()); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR); } @Test public void indexerThrowsOrmExceptionTryingToIndexChange() throws Exception { - doThrow(new OrmException("some message")).when(handlerMock).index(id, Operation.INDEX); + doThrow(new OrmException("some message")) + .when(handlerMock) + .index(eq(CHANGE_ID), eq(Operation.INDEX), any()); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_NOT_FOUND, "Error trying to find change"); } @Test public void sendErrorThrowsIOException() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(id, Operation.INDEX); + doThrow(new IOException(IO_ERROR)) + .when(handlerMock) + .index(eq(CHANGE_ID), eq(Operation.INDEX), any()); doThrow(new IOException("someError")).when(responseMock).sendError(SC_CONFLICT, IO_ERROR); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java index 1864e53..6291142 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java
@@ -17,6 +17,8 @@ import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -50,13 +52,13 @@ public void setUpMocks() { servlet = new IndexGroupRestApiServlet(handlerMock); uuid = new AccountGroup.UUID(UUID); - when(requestMock.getPathInfo()).thenReturn("/index/group/" + UUID); + when(requestMock.getRequestURI()).thenReturn("http://gerrit.com/index/group/" + UUID); } @Test public void groupIsIndexed() throws Exception { servlet.doPost(requestMock, responseMock); - verify(handlerMock, times(1)).index(uuid, Operation.INDEX); + verify(handlerMock, times(1)).index(eq(uuid), eq(Operation.INDEX), any()); verify(responseMock).setStatus(SC_NO_CONTENT); } @@ -68,14 +70,18 @@ @Test public void indexerThrowsIOExceptionTryingToIndexGroup() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(uuid, Operation.INDEX); + doThrow(new IOException(IO_ERROR)) + .when(handlerMock) + .index(eq(uuid), eq(Operation.INDEX), any()); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR); } @Test public void sendErrorThrowsIOException() throws Exception { - doThrow(new IOException(IO_ERROR)).when(handlerMock).index(uuid, Operation.INDEX); + doThrow(new IOException(IO_ERROR)) + .when(handlerMock) + .index(eq(uuid), eq(Operation.INDEX), any()); doThrow(new IOException("someError")).when(responseMock).sendError(SC_CONFLICT, IO_ERROR); servlet.doPost(requestMock, responseMock); verify(responseMock).sendError(SC_CONFLICT, IO_ERROR);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java index fb12578..55a3c2a 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java
@@ -15,13 +15,16 @@ package com.ericsson.gerrit.plugins.highavailability.forwarder.rest; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.ericsson.gerrit.plugins.highavailability.cache.Constants; +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; import com.ericsson.gerrit.plugins.highavailability.forwarder.TestEvent; import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.HttpResponseHandler.HttpResult; import com.ericsson.gerrit.plugins.highavailability.peers.PeerInfo; @@ -30,7 +33,7 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.events.Event; -import com.google.gson.GsonBuilder; +import com.google.gson.Gson; import com.google.inject.Provider; import java.io.IOException; import java.util.Set; @@ -45,7 +48,6 @@ private static final String EMPTY_MSG = ""; private static final String ERROR = "Error"; private static final String PLUGINS = "plugins"; - private static final String PROJECT_NAME = "projectName"; private static final String PROJECT_TO_ADD = "projectToAdd"; private static final String PROJECT_TO_DELETE = "projectToDelete"; private static final String SUCCESS = "Success"; @@ -54,8 +56,18 @@ // Index private static final int CHANGE_NUMBER = 1; + private static final String PROJECT_NAME = "test/project"; + private static final String PROJECT_NAME_URL_END = "test%2Fproject"; private static final String INDEX_CHANGE_ENDPOINT = - Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "index/change", CHANGE_NUMBER); + Joiner.on("/") + .join( + URL, + PLUGINS, + PLUGIN_NAME, + "index/change", + PROJECT_NAME_URL_END + "~" + CHANGE_NUMBER); + private static final String DELETE_CHANGE_ENDPOINT = + Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "index/change", "~" + CHANGE_NUMBER); private static final int ACCOUNT_NUMBER = 2; private static final String INDEX_ACCOUNT_ENDPOINT = Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "index/account", ACCOUNT_NUMBER); @@ -63,11 +75,13 @@ private static final String INDEX_GROUP_ENDPOINT = Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "index/group", UUID); + private GsonProvider gsonProvider = new GsonProvider(); + private Gson gson = gsonProvider.get(); + // Event private static Event event = new TestEvent(); private static final String EVENT_ENDPOINT = Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "event", event.type); - private static String eventJson = new GsonBuilder().create().toJson(event); private RestForwarder forwarder; private HttpSession httpSessionMock; @@ -83,113 +97,116 @@ when(peersMock.get()).thenReturn(ImmutableSet.of(new PeerInfo(URL))); forwarder = new RestForwarder( - httpSessionMock, PLUGIN_NAME, configMock, peersMock); // TODO: Create provider + httpSessionMock, + PLUGIN_NAME, + configMock, + peersMock, + gsonProvider); // TODO: Create provider } @Test public void testIndexAccountOK() throws Exception { - when(httpSessionMock.post(INDEX_ACCOUNT_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_ACCOUNT_ENDPOINT), any())) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); - assertThat(forwarder.indexAccount(ACCOUNT_NUMBER)).isTrue(); + assertThat(forwarder.indexAccount(ACCOUNT_NUMBER, new IndexEvent())).isTrue(); } @Test public void testIndexAccountFailed() throws Exception { - when(httpSessionMock.post(INDEX_ACCOUNT_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_ACCOUNT_ENDPOINT), any())) .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); - assertThat(forwarder.indexAccount(ACCOUNT_NUMBER)).isFalse(); + assertThat(forwarder.indexAccount(ACCOUNT_NUMBER, new IndexEvent())).isFalse(); } @Test public void testIndexAccountThrowsException() throws Exception { - doThrow(new IOException()).when(httpSessionMock).post(INDEX_ACCOUNT_ENDPOINT, null); - assertThat(forwarder.indexAccount(ACCOUNT_NUMBER)).isFalse(); + doThrow(new IOException()).when(httpSessionMock).post(eq(INDEX_ACCOUNT_ENDPOINT), any()); + assertThat(forwarder.indexAccount(ACCOUNT_NUMBER, new IndexEvent())).isFalse(); } @Test public void testIndexGroupOK() throws Exception { - when(httpSessionMock.post(INDEX_GROUP_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_GROUP_ENDPOINT), any())) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); - assertThat(forwarder.indexGroup(UUID)).isTrue(); + assertThat(forwarder.indexGroup(UUID, new IndexEvent())).isTrue(); } @Test public void testIndexGroupFailed() throws Exception { - when(httpSessionMock.post(INDEX_GROUP_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_GROUP_ENDPOINT), any())) .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); - assertThat(forwarder.indexGroup(UUID)).isFalse(); + assertThat(forwarder.indexGroup(UUID, new IndexEvent())).isFalse(); } @Test public void testIndexGroupThrowsException() throws Exception { - doThrow(new IOException()).when(httpSessionMock).post(INDEX_GROUP_ENDPOINT, null); - assertThat(forwarder.indexGroup(UUID)).isFalse(); + doThrow(new IOException()).when(httpSessionMock).post(eq(INDEX_GROUP_ENDPOINT), any()); + assertThat(forwarder.indexGroup(UUID, new IndexEvent())).isFalse(); } @Test public void testIndexChangeOK() throws Exception { - when(httpSessionMock.post(INDEX_CHANGE_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_CHANGE_ENDPOINT), any())) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); - assertThat(forwarder.indexChange(CHANGE_NUMBER)).isTrue(); + assertThat(forwarder.indexChange(PROJECT_NAME, CHANGE_NUMBER, new IndexEvent())).isTrue(); } @Test public void testIndexChangeFailed() throws Exception { - when(httpSessionMock.post(INDEX_CHANGE_ENDPOINT, null)) + when(httpSessionMock.post(eq(INDEX_CHANGE_ENDPOINT), any())) .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); - assertThat(forwarder.indexChange(CHANGE_NUMBER)).isFalse(); + assertThat(forwarder.indexChange(PROJECT_NAME, CHANGE_NUMBER, new IndexEvent())).isFalse(); } @Test public void testIndexChangeThrowsException() throws Exception { - doThrow(new IOException()).when(httpSessionMock).post(INDEX_CHANGE_ENDPOINT, null); - assertThat(forwarder.indexChange(CHANGE_NUMBER)).isFalse(); + doThrow(new IOException()).when(httpSessionMock).post(eq(INDEX_CHANGE_ENDPOINT), any()); + assertThat(forwarder.indexChange(PROJECT_NAME, CHANGE_NUMBER, new IndexEvent())).isFalse(); } @Test public void testChangeDeletedFromIndexOK() throws Exception { - when(httpSessionMock.delete(INDEX_CHANGE_ENDPOINT)) + when(httpSessionMock.delete(eq(DELETE_CHANGE_ENDPOINT))) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); - assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER)).isTrue(); + assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER, new IndexEvent())).isTrue(); } @Test public void testChangeDeletedFromIndexFailed() throws Exception { - when(httpSessionMock.delete(INDEX_CHANGE_ENDPOINT)) + when(httpSessionMock.delete(eq(DELETE_CHANGE_ENDPOINT))) .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); - assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER)).isFalse(); + assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER, new IndexEvent())).isFalse(); } @Test public void testChangeDeletedFromThrowsException() throws Exception { - doThrow(new IOException()).when(httpSessionMock).delete(INDEX_CHANGE_ENDPOINT); - assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER)).isFalse(); + doThrow(new IOException()).when(httpSessionMock).delete(eq(DELETE_CHANGE_ENDPOINT)); + assertThat(forwarder.deleteChangeFromIndex(CHANGE_NUMBER, new IndexEvent())).isFalse(); } @Test public void testEventSentOK() throws Exception { - when(httpSessionMock.post(EVENT_ENDPOINT, eventJson)) + when(httpSessionMock.post(EVENT_ENDPOINT, event)) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.send(event)).isTrue(); } @Test public void testEventSentFailed() throws Exception { - when(httpSessionMock.post(EVENT_ENDPOINT, eventJson)) - .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); + when(httpSessionMock.post(EVENT_ENDPOINT, event)).thenReturn(new HttpResult(FAILED, EMPTY_MSG)); assertThat(forwarder.send(event)).isFalse(); } @Test public void testEventSentThrowsException() throws Exception { - doThrow(new IOException()).when(httpSessionMock).post(EVENT_ENDPOINT, eventJson); + doThrow(new IOException()).when(httpSessionMock).post(EVENT_ENDPOINT, event); assertThat(forwarder.send(event)).isFalse(); } @Test public void testEvictProjectOK() throws Exception { String key = PROJECT_NAME; - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); when(httpSessionMock.post(buildCacheEndpoint(Constants.PROJECTS), keyJson)) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.evict(Constants.PROJECTS, key)).isTrue(); @@ -198,7 +215,7 @@ @Test public void testEvictAccountsOK() throws Exception { Account.Id key = new Account.Id(123); - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); when(httpSessionMock.post(buildCacheEndpoint(Constants.ACCOUNTS), keyJson)) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.evict(Constants.ACCOUNTS, key)).isTrue(); @@ -207,7 +224,7 @@ @Test public void testEvictGroupsOK() throws Exception { AccountGroup.Id key = new AccountGroup.Id(123); - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); String endpoint = buildCacheEndpoint(Constants.GROUPS); when(httpSessionMock.post(endpoint, keyJson)).thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.evict(Constants.GROUPS, key)).isTrue(); @@ -216,7 +233,7 @@ @Test public void testEvictGroupsByIncludeOK() throws Exception { AccountGroup.UUID key = new AccountGroup.UUID("90b3042d9094a37985f3f9281391dbbe9a5addad"); - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); when(httpSessionMock.post(buildCacheEndpoint(Constants.GROUPS_BYINCLUDE), keyJson)) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.evict(Constants.GROUPS_BYINCLUDE, key)).isTrue(); @@ -225,7 +242,7 @@ @Test public void testEvictGroupsMembersOK() throws Exception { AccountGroup.UUID key = new AccountGroup.UUID("90b3042d9094a37985f3f9281391dbbe9a5addad"); - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); when(httpSessionMock.post(buildCacheEndpoint(Constants.GROUPS_MEMBERS), keyJson)) .thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG)); assertThat(forwarder.evict(Constants.GROUPS_MEMBERS, key)).isTrue(); @@ -234,7 +251,7 @@ @Test public void testEvictCacheFailed() throws Exception { String key = PROJECT_NAME; - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); when(httpSessionMock.post(buildCacheEndpoint(Constants.PROJECTS), keyJson)) .thenReturn(new HttpResult(FAILED, EMPTY_MSG)); assertThat(forwarder.evict(Constants.PROJECTS, key)).isFalse(); @@ -243,7 +260,7 @@ @Test public void testEvictCacheThrowsException() throws Exception { String key = PROJECT_NAME; - String keyJson = new GsonBuilder().create().toJson(key); + String keyJson = gson.toJson(key); doThrow(new IOException()) .when(httpSessionMock) .post(buildCacheEndpoint(Constants.PROJECTS), keyJson);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServletTest.java index 1ad17cb..c8fe4e8 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServletTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/health/HealthServletTest.java
@@ -14,6 +14,7 @@ package com.ericsson.gerrit.plugins.highavailability.health; +import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; @@ -24,7 +25,8 @@ import static org.mockito.Mockito.when; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.account.CapabilityControl; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackend.WithUser; import com.google.inject.Provider; import java.io.IOException; import java.nio.file.Files; @@ -44,16 +46,19 @@ @Mock private Provider<CurrentUser> currentUserProviderMock; @Mock private CurrentUser currentUserMock; - @Mock private CapabilityControl capabilityControlMock; + @Mock private PermissionBackend permissionBackendMock; + @Mock private WithUser withUserMock; private HealthServlet servlet; @Before public void setUp() throws Exception { when(currentUserProviderMock.get()).thenReturn(currentUserMock); - when(currentUserMock.getCapabilities()).thenReturn(capabilityControlMock); - when(capabilityControlMock.canAdministrateServer()).thenReturn(true); - servlet = new HealthServlet(currentUserProviderMock, tempFolder.getRoot().toPath()); + when(permissionBackendMock.user(currentUserMock)).thenReturn(withUserMock); + when(withUserMock.testOrFalse(ADMINISTRATE_SERVER)).thenReturn(true); + servlet = + new HealthServlet( + currentUserProviderMock, permissionBackendMock, tempFolder.getRoot().toPath()); } @Test @@ -82,7 +87,7 @@ public void testTransitionToUnhealthyByNonAdmins() throws IOException { assertIsHealthy(); - when(capabilityControlMock.canAdministrateServer()).thenReturn(false); + when(withUserMock.testOrFalse(ADMINISTRATE_SERVER)).thenReturn(false); HttpServletResponse responseMock = mock(HttpServletResponse.class); servlet.doDelete(null, responseMock); verify(responseMock).sendError(SC_FORBIDDEN); @@ -124,7 +129,7 @@ servlet.doDelete(null, mock(HttpServletResponse.class)); assertIsUnhealthy(); - when(capabilityControlMock.canAdministrateServer()).thenReturn(false); + when(withUserMock.testOrFalse(ADMINISTRATE_SERVER)).thenReturn(false); HttpServletResponse responseMock = mock(HttpServletResponse.class); servlet.doPost(null, responseMock); verify(responseMock).sendError(SC_FORBIDDEN);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AccountIndexForwardingIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AccountIndexForwardingIT.java index df3af5c..4b2462e 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AccountIndexForwardingIT.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AccountIndexForwardingIT.java
@@ -21,7 +21,7 @@ @Override public void beforeAction() throws Exception { - testAccount = accounts.create("someUser"); + testAccount = accountCreator.create("someUser"); } @Override
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeIndexForwardingIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeIndexForwardingIT.java index 938d37f..f80f56f 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeIndexForwardingIT.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeIndexForwardingIT.java
@@ -24,7 +24,7 @@ @Override public String getExpectedRequest() { - return "/plugins/high-availability/index/change/" + changeId; + return "/plugins/high-availability/index/change/" + project.get() + "~" + changeId; } @Override
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java index 7b3d14b..d9e1b22 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java
@@ -15,13 +15,18 @@ package com.ericsson.gerrit.plugins.highavailability.index; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; import com.ericsson.gerrit.plugins.highavailability.forwarder.Context; import com.ericsson.gerrit.plugins.highavailability.forwarder.Forwarder; +import com.ericsson.gerrit.plugins.highavailability.forwarder.IndexEvent; +import com.ericsson.gerrit.plugins.highavailability.index.IndexEventHandler.DeleteChangeTask; import com.ericsson.gerrit.plugins.highavailability.index.IndexEventHandler.IndexAccountTask; import com.ericsson.gerrit.plugins.highavailability.index.IndexEventHandler.IndexChangeTask; import com.ericsson.gerrit.plugins.highavailability.index.IndexEventHandler.IndexGroupTask; @@ -29,7 +34,8 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.git.WorkQueue.Executor; +import java.util.Optional; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,6 +45,7 @@ @RunWith(MockitoJUnitRunner.class) public class IndexEventHandlerTest { private static final String PLUGIN_NAME = "high-availability"; + private static final String PROJECT_NAME = "test/project"; private static final int CHANGE_ID = 1; private static final int ACCOUNT_ID = 2; private static final String UUID = "3"; @@ -46,47 +53,54 @@ private IndexEventHandler indexEventHandler; @Mock private Forwarder forwarder; + @Mock private ChangeCheckerImpl.Factory changeCheckerFactoryMock; + @Mock private ChangeChecker changeCheckerMock; private Change.Id changeId; private Account.Id accountId; private AccountGroup.UUID accountGroupUUID; @Before - public void setUpMocks() { + public void setUpMocks() throws Exception { changeId = new Change.Id(CHANGE_ID); accountId = new Account.Id(ACCOUNT_ID); accountGroupUUID = new AccountGroup.UUID(UUID); + when(changeCheckerFactoryMock.create(any())).thenReturn(changeCheckerMock); + when(changeCheckerMock.newIndexEvent()).thenReturn(Optional.of(new IndexEvent())); indexEventHandler = - new IndexEventHandler(MoreExecutors.directExecutor(), PLUGIN_NAME, forwarder); + new IndexEventHandler( + MoreExecutors.directExecutor(), PLUGIN_NAME, forwarder, changeCheckerFactoryMock); } @Test public void shouldIndexInRemoteOnChangeIndexedEvent() throws Exception { - indexEventHandler.onChangeIndexed(changeId.get()); - verify(forwarder).indexChange(CHANGE_ID); + indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get()); + verify(forwarder).indexChange(eq(PROJECT_NAME), eq(CHANGE_ID), any()); } @Test public void shouldIndexInRemoteOnAccountIndexedEvent() throws Exception { indexEventHandler.onAccountIndexed(accountId.get()); - verify(forwarder).indexAccount(ACCOUNT_ID); + verify(forwarder).indexAccount(eq(ACCOUNT_ID), any()); } @Test public void shouldDeleteFromIndexInRemoteOnChangeDeletedEvent() throws Exception { indexEventHandler.onChangeDeleted(changeId.get()); - verify(forwarder).deleteChangeFromIndex(CHANGE_ID); + verify(forwarder).deleteChangeFromIndex(eq(CHANGE_ID), any()); + verifyZeroInteractions( + changeCheckerMock); // Deleted changes should not be checked against NoteDb } @Test public void shouldIndexInRemoteOnGroupIndexedEvent() throws Exception { indexEventHandler.onGroupIndexed(accountGroupUUID.get()); - verify(forwarder).indexGroup(UUID); + verify(forwarder).indexGroup(eq(UUID), any()); } @Test public void shouldNotCallRemoteWhenChangeEventIsForwarded() throws Exception { Context.setForwardedEvent(true); - indexEventHandler.onChangeIndexed(changeId.get()); + indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get()); indexEventHandler.onChangeDeleted(changeId.get()); Context.unsetForwardedEvent(); verifyZeroInteractions(forwarder); @@ -112,17 +126,20 @@ @Test public void duplicateChangeEventOfAQueuedEventShouldGetDiscarded() { - Executor poolMock = mock(Executor.class); - indexEventHandler = new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder); - indexEventHandler.onChangeIndexed(changeId.get()); - indexEventHandler.onChangeIndexed(changeId.get()); - verify(poolMock, times(1)).execute(indexEventHandler.new IndexChangeTask(CHANGE_ID, false)); + ScheduledThreadPoolExecutor poolMock = mock(ScheduledThreadPoolExecutor.class); + indexEventHandler = + new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder, changeCheckerFactoryMock); + indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get()); + indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get()); + verify(poolMock, times(1)) + .execute(indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, null)); } @Test public void duplicateAccountEventOfAQueuedEventShouldGetDiscarded() { - Executor poolMock = mock(Executor.class); - indexEventHandler = new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder); + ScheduledThreadPoolExecutor poolMock = mock(ScheduledThreadPoolExecutor.class); + indexEventHandler = + new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder, changeCheckerFactoryMock); indexEventHandler.onAccountIndexed(accountId.get()); indexEventHandler.onAccountIndexed(accountId.get()); verify(poolMock, times(1)).execute(indexEventHandler.new IndexAccountTask(ACCOUNT_ID)); @@ -130,8 +147,9 @@ @Test public void duplicateGroupEventOfAQueuedEventShouldGetDiscarded() { - Executor poolMock = mock(Executor.class); - indexEventHandler = new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder); + ScheduledThreadPoolExecutor poolMock = mock(ScheduledThreadPoolExecutor.class); + indexEventHandler = + new IndexEventHandler(poolMock, PLUGIN_NAME, forwarder, changeCheckerFactoryMock); indexEventHandler.onGroupIndexed(accountGroupUUID.get()); indexEventHandler.onGroupIndexed(accountGroupUUID.get()); verify(poolMock, times(1)).execute(indexEventHandler.new IndexGroupTask(UUID)); @@ -139,7 +157,7 @@ @Test public void testIndexChangeTaskToString() throws Exception { - IndexChangeTask task = indexEventHandler.new IndexChangeTask(CHANGE_ID, false); + IndexChangeTask task = indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, null); assertThat(task.toString()) .isEqualTo( String.format("[%s] Index change %s in target instance", PLUGIN_NAME, CHANGE_ID)); @@ -162,27 +180,48 @@ @Test public void testIndexChangeTaskHashCodeAndEquals() { - IndexChangeTask task = indexEventHandler.new IndexChangeTask(CHANGE_ID, false); + IndexChangeTask task = indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, null); IndexChangeTask sameTask = task; assertThat(task.equals(sameTask)).isTrue(); assertThat(task.hashCode()).isEqualTo(sameTask.hashCode()); - IndexChangeTask identicalTask = indexEventHandler.new IndexChangeTask(CHANGE_ID, false); + IndexChangeTask identicalTask = + indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, null); assertThat(task.equals(identicalTask)).isTrue(); assertThat(task.hashCode()).isEqualTo(identicalTask.hashCode()); assertThat(task.equals(null)).isFalse(); - assertThat(task.equals(indexEventHandler.new IndexChangeTask(CHANGE_ID + 1, false))).isFalse(); + assertThat( + task.equals(indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID + 1, null))) + .isFalse(); assertThat(task.hashCode()).isNotEqualTo("test".hashCode()); - IndexChangeTask differentChangeIdTask = indexEventHandler.new IndexChangeTask(123, false); + IndexChangeTask differentChangeIdTask = + indexEventHandler.new IndexChangeTask(PROJECT_NAME, 123, null); assertThat(task.equals(differentChangeIdTask)).isFalse(); assertThat(task.hashCode()).isNotEqualTo(differentChangeIdTask.hashCode()); + } - IndexChangeTask removeTask = indexEventHandler.new IndexChangeTask(CHANGE_ID, true); - assertThat(task.equals(removeTask)).isFalse(); - assertThat(task.hashCode()).isNotEqualTo(removeTask.hashCode()); + @Test + public void testDeleteChangeTaskHashCodeAndEquals() { + DeleteChangeTask task = indexEventHandler.new DeleteChangeTask(CHANGE_ID, null); + + DeleteChangeTask sameTask = task; + assertThat(task.equals(sameTask)).isTrue(); + assertThat(task.hashCode()).isEqualTo(sameTask.hashCode()); + + DeleteChangeTask identicalTask = indexEventHandler.new DeleteChangeTask(CHANGE_ID, null); + assertThat(task.equals(identicalTask)).isTrue(); + assertThat(task.hashCode()).isEqualTo(identicalTask.hashCode()); + + assertThat(task.equals(null)).isFalse(); + assertThat(task.equals(indexEventHandler.new DeleteChangeTask(CHANGE_ID + 1, null))).isFalse(); + assertThat(task.hashCode()).isNotEqualTo("test".hashCode()); + + DeleteChangeTask differentChangeIdTask = indexEventHandler.new DeleteChangeTask(123, null); + assertThat(task.equals(differentChangeIdTask)).isFalse(); + assertThat(task.hashCode()).isNotEqualTo(differentChangeIdTask.hashCode()); } @Test
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexExecutorProviderTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexExecutorProviderTest.java index 59e37c9..abab0b9 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexExecutorProviderTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexExecutorProviderTest.java
@@ -21,6 +21,7 @@ import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.google.gerrit.server.git.WorkQueue; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -30,12 +31,12 @@ @RunWith(MockitoJUnitRunner.class) public class IndexExecutorProviderTest { - @Mock private WorkQueue.Executor executorMock; + @Mock private ScheduledThreadPoolExecutor executorMock; private IndexExecutorProvider indexExecutorProvider; @Before public void setUp() throws Exception { - executorMock = mock(WorkQueue.Executor.class); + executorMock = mock(ScheduledThreadPoolExecutor.class); WorkQueue workQueueMock = mock(WorkQueue.class); when(workQueueMock.createQueue(4, "Forward-Index-Event")).thenReturn(executorMock); Configuration configMock = mock(Configuration.class, Answers.RETURNS_DEEP_STUBS); @@ -54,7 +55,6 @@ assertThat(indexExecutorProvider.get()).isEqualTo(executorMock); indexExecutorProvider.stop(); verify(executorMock).shutdown(); - verify(executorMock).unregisterWorkQueue(); assertThat(indexExecutorProvider.get()).isNull(); } }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/websession/file/FileBasedWebSessionCacheCleanerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/websession/file/FileBasedWebSessionCacheCleanerTest.java index 45ac4b7..0a8fc92 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/websession/file/FileBasedWebSessionCacheCleanerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/websession/file/FileBasedWebSessionCacheCleanerTest.java
@@ -27,9 +27,9 @@ import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.ericsson.gerrit.plugins.highavailability.websession.file.FileBasedWebSessionCacheCleaner.CleanupTask; import com.google.gerrit.server.git.WorkQueue; -import com.google.gerrit.server.git.WorkQueue.Executor; import com.google.inject.Provider; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; @@ -44,7 +44,7 @@ private static long CLEANUP_INTERVAL = 5000; private static String SOME_PLUGIN_NAME = "somePluginName"; - @Mock private Executor executorMock; + @Mock private ScheduledThreadPoolExecutor executorMock; @Mock private ScheduledFuture<?> scheduledFutureMock; @Mock private WorkQueue workQueueMock; @Mock private Provider<CleanupTask> cleanupTaskProviderMock;