Merge branch 'stable-2.16'
* stable-2.16:
Disable the shared ref-db by configuration
Change-Id: I80e50a489aaa8e9b7b01974dad45d80d4d1f3dc9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
deleted file mode 100644
index ff932da..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// 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.googlesource.gerrit.plugins.multisite;
-
-import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-@Singleton
-public class GerritNoteDbStatus implements NoteDbStatus {
- private final NotesMigration notesMigration;
-
- @Inject
- public GerritNoteDbStatus(NotesMigration notesMigration) {
- this.notesMigration = notesMigration;
- }
-
- @Override
- public boolean enabled() {
- return notesMigration.commitChangeWrites();
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
index 5e91c80..adb74c8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
@@ -16,12 +16,13 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.ModuleImpl;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gson.Gson;
import com.google.inject.CreationException;
import com.google.inject.Inject;
import com.google.inject.Provides;
-import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import com.google.inject.spi.Message;
import com.googlesource.gerrit.plugins.multisite.broker.BrokerGson;
@@ -46,15 +47,15 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+@ModuleImpl(name = GitRepositoryManager.LOCAL_REPOSITORY_MODULE)
public class Module extends LifecycleModule {
private static final Logger log = LoggerFactory.getLogger(Module.class);
- private Configuration config;
- private NoteDbStatus noteDb;
+ private final Configuration config;
private final boolean disableGitRepositoryValidation;
@Inject
- public Module(Configuration config, NoteDbStatus noteDb) {
- this(config, noteDb, false);
+ public Module(Configuration config) {
+ this(config, false);
}
// TODO: It is not possible to properly test the libModules in Gerrit.
@@ -62,23 +63,13 @@
// support
// in Gerrit for it.
@VisibleForTesting
- public Module(Configuration config, NoteDbStatus noteDb, boolean disableGitRepositoryValidation) {
- init(config, noteDb);
- this.disableGitRepositoryValidation = disableGitRepositoryValidation;
- }
-
- private void init(Configuration config, NoteDbStatus noteDb) {
+ public Module(Configuration config, boolean disableGitRepositoryValidation) {
this.config = config;
- this.noteDb = noteDb;
+ this.disableGitRepositoryValidation = disableGitRepositoryValidation;
}
@Override
protected void configure() {
- if (!noteDb.enabled()) {
- throw new ProvisionException(
- "Gerrit is still running on ReviewDb: please migrate to NoteDb "
- + "and then reload the multi-site plugin.");
- }
Collection<Message> validationErrors = config.validate();
if (!validationErrors.isEmpty()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java
deleted file mode 100644
index f47e503..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// 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.googlesource.gerrit.plugins.multisite;
-
-import com.google.inject.ImplementedBy;
-
-/** Returns the status of changes migration. */
-@ImplementedBy(GerritNoteDbStatus.class)
-public interface NoteDbStatus {
-
- /**
- * Status of NoteDb migration.
- *
- * @return true if Gerrit has been migrated to NoteDb
- */
- boolean enabled();
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
index 4b0a142..2d25c51 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
@@ -135,7 +135,7 @@
private void reindex(ChangeNotes notes) throws IOException, OrmException {
try (ManualRequestContext ctx = oneOffCtx.open()) {
notes.reload();
- indexer.index(ctx.getReviewDbProvider().get(), notes.getChange());
+ indexer.index(notes.getChange());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
index 29435bc..d96a8b2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
@@ -16,7 +16,6 @@
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.CommentsUtil;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -148,16 +147,14 @@
}
private Optional<Long> computeLastChangeTs() throws OrmException {
- try (ReviewDb db = oneOffReqCtx.open().getReviewDbProvider().get()) {
- return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(db, notes));
- }
+ return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(notes));
}
- private long getTsFromChangeAndDraftComments(ReviewDb db, ChangeNotes notes) {
+ private long getTsFromChangeAndDraftComments(ChangeNotes notes) {
Change change = notes.getChange();
Timestamp changeTs = change.getLastUpdatedOn();
try {
- for (Comment comment : commentsUtil.draftByChange(db, changeNotes.get())) {
+ for (Comment comment : commentsUtil.draftByChange(changeNotes.get())) {
Timestamp commentTs = comment.writtenOn;
changeTs = commentTs.after(changeTs) ? commentTs : changeTs;
}
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index 9e29e71..980aa06 100644
--- a/src/main/resources/Documentation/about.md
+++ b/src/main/resources/Documentation/about.md
@@ -8,7 +8,6 @@
The masters must be:
-* migrated to NoteDb
* connected to the same message broker
* behind a load balancer (e.g., HAProxy)
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
index 759fe7c..5281371 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
@@ -36,15 +36,13 @@
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Configuration configMock;
- @Mock private NoteDbStatus noteDb;
-
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
private Module module;
@Before
public void setUp() {
- module = new Module(configMock, noteDb);
+ module = new Module(configMock);
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
index 2793253..d6c3576 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -26,7 +26,6 @@
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.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -34,7 +33,6 @@
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gwtorm.server.OrmException;
-import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
@@ -61,9 +59,7 @@
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;
@@ -71,7 +67,6 @@
@Mock private ChangeIndexer indexerMock;
@Mock private OneOffRequestContext ctxMock;
@Mock private ManualRequestContext manualRequestContextMock;
- @Mock private ReviewDb dbMock;
@Mock private ChangeNotes changeNotes;
@Mock private Configuration configurationMock;
@Mock private Configuration.Index index;
@@ -87,7 +82,6 @@
@Before
public void setUp() throws Exception {
when(ctxMock.open()).thenReturn(manualRequestContextMock);
- when(manualRequestContextMock.getReviewDbProvider()).thenReturn(Providers.of(dbMock));
id = new Change.Id(TEST_CHANGE_NUMBER);
change = new Change(null, id, null, null, TimeUtil.nowTs());
when(changeNotes.getChange()).thenReturn(change);
@@ -103,7 +97,7 @@
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));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -111,7 +105,7 @@
setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_OUTDATED);
handler.index(
TEST_CHANGE_ID, Operation.INDEX, Optional.of(new ChangeIndexEvent("foo", 1, false)));
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -125,21 +119,12 @@
setupChangeAccessRelatedMocks(CHANGE_DOES_NOT_EXIST, CHANGE_OUTDATED);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
verify(indexerMock, never()).delete(id);
- verify(indexerMock, never())
- .index(any(ReviewDb.class), any(Project.NameKey.class), any(Change.Id.class));
- }
-
- @Test
- public void schemaThrowsExceptionWhenLookingUpForChange() throws Exception {
- setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_ORM_EXCEPTION, CHANGE_UP_TO_DATE);
- exception.expect(OrmException.class);
- handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
+ verify(indexerMock, never()).index(any(Project.NameKey.class), any(Change.Id.class));
}
@Test
public void indexerThrowsIOExceptionTryingToIndexChange() throws Exception {
- setupChangeAccessRelatedMocks(
- CHANGE_EXISTS, DO_NOT_THROW_ORM_EXCEPTION, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE);
+ setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE);
exception.expect(IOException.class);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
}
@@ -156,13 +141,13 @@
return null;
})
.when(indexerMock)
- .index(any(ReviewDb.class), any(Change.class));
+ .index(any(Change.class));
assertThat(Context.isForwardedEvent()).isFalse();
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -175,7 +160,7 @@
throw new IOException("someMessage");
})
.when(indexerMock)
- .index(any(ReviewDb.class), any(Change.class));
+ .index(any(Change.class));
assertThat(Context.isForwardedEvent()).isFalse();
try {
@@ -186,36 +171,22 @@
}
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
private void setupChangeAccessRelatedMocks(boolean changeExist, boolean changeUpToDate)
throws Exception {
- setupChangeAccessRelatedMocks(
- changeExist, DO_NOT_THROW_ORM_EXCEPTION, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate);
+ setupChangeAccessRelatedMocks(changeExist, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate);
}
private void setupChangeAccessRelatedMocks(
- 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(ctxMock).open();
- }
-
+ boolean changeExists, boolean ioException, boolean changeIsUpToDate)
+ throws IOException, OrmException {
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));
+ doThrow(new IOException("io-error")).when(indexerMock).index(any(Change.class));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
index 3e8b3ca..5d64d28 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
@@ -39,7 +39,6 @@
import com.google.inject.TypeLiteral;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.Module;
-import com.googlesource.gerrit.plugins.multisite.NoteDbStatus;
import com.googlesource.gerrit.plugins.multisite.broker.BrokerGson;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
import java.io.IOException;
@@ -69,10 +68,6 @@
"com.googlesource.gerrit.plugins.multisite.kafka.consumer.EventConsumerIT$KafkaTestContainerModule";
private static final int QUEUE_POLL_TIMEOUT_MSECS = 10000;
- static {
- System.setProperty("gerrit.notedb", "ON");
- }
-
public static class KafkaTestContainerModule extends LifecycleModule {
public class KafkaStopAtShutdown implements LifecycleListener {
@@ -97,11 +92,11 @@
private final Module multiSiteModule;
@Inject
- public KafkaTestContainerModule(SitePaths sitePaths, NoteDbStatus noteDb) {
+ public KafkaTestContainerModule(SitePaths sitePaths) {
this.config =
new FileBasedConfig(
sitePaths.etc_dir.resolve(Configuration.MULTI_SITE_CONFIG).toFile(), FS.DETECTED);
- this.multiSiteModule = new Module(new Configuration(config, new Config()), noteDb, true);
+ this.multiSiteModule = new Module(new Configuration(config, new Config()), true);
}
@Override
@@ -127,6 +122,10 @@
config.setBoolean("kafka", "subscriber", "enabled", true);
config.setBoolean("ref-database", null, "enabled", false);
config.save();
+ Configuration multiSiteConfig = new Configuration(config, new Config());
+ bind(Configuration.class).toInstance(multiSiteConfig);
+
+ listener().toInstance(new KafkaStopAtShutdown(kafkaContainer));
return kafkaContainer;
}
