Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
IndexEventHandler: Split delete change event in its own task
Fix delete change event propagation
Change-Id: I0eae7d1e7d564be06c9b0b470d05ebf51dcb0e14
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 e6a8b0c..89ff353 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
@@ -67,18 +67,29 @@
@Override
public void onChangeIndexed(String projectName, int id) {
- executeIndexChangeTask(projectName, id, false);
+ 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);
- }
-
- @Override
- public void onGroupIndexed(String groupUUID) {
if (!Context.isForwardedEvent()) {
- IndexGroupTask task = new IndexGroupTask(groupUUID);
+ DeleteChangeTask task = new DeleteChangeTask(id, new IndexEvent());
if (queuedTasks.add(task)) {
executor.execute(task);
}
@@ -95,21 +106,12 @@
}
}
- private void executeIndexChangeTask(String projectName, int id, boolean deleted) {
+ @Override
+ public void onGroupIndexed(String groupUUID) {
if (!Context.isForwardedEvent()) {
- ChangeChecker checker = changeChecker.create(projectName + "~" + id);
- try {
- checker
- .newIndexEvent()
- .map(event -> new IndexChangeTask(projectName, id, deleted, event))
- .ifPresent(
- task -> {
- if (queuedTasks.add(task)) {
- executor.execute(task);
- }
- });
- } catch (Exception e) {
- log.warn("Unable to create task to handle change {}~{}", projectName, id, e);
+ IndexGroupTask task = new IndexGroupTask(groupUUID);
+ if (queuedTasks.add(task)) {
+ executor.execute(task);
}
}
}
@@ -135,29 +137,23 @@
}
class IndexChangeTask extends IndexTask {
- private final boolean deleted;
private final int changeId;
private final String projectName;
- IndexChangeTask(String projectName, int changeId, boolean deleted, IndexEvent indexEvent) {
+ 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, indexEvent);
- } else {
- forwarder.indexChange(projectName, changeId, indexEvent);
- }
+ forwarder.indexChange(projectName, changeId, indexEvent);
}
@Override
public int hashCode() {
- return Objects.hashCode(IndexChangeTask.class, changeId, deleted);
+ return Objects.hashCode(IndexChangeTask.class, changeId);
}
@Override
@@ -166,7 +162,7 @@
return false;
}
IndexChangeTask other = (IndexChangeTask) obj;
- return changeId == other.changeId && deleted == other.deleted;
+ return changeId == other.changeId;
}
@Override
@@ -175,6 +171,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;
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 3ec979c..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
@@ -26,6 +26,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.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;
@@ -86,6 +87,8 @@
public void shouldDeleteFromIndexInRemoteOnChangeDeletedEvent() throws Exception {
indexEventHandler.onChangeDeleted(changeId.get());
verify(forwarder).deleteChangeFromIndex(eq(CHANGE_ID), any());
+ verifyZeroInteractions(
+ changeCheckerMock); // Deleted changes should not be checked against NoteDb
}
@Test
@@ -129,7 +132,7 @@
indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get());
indexEventHandler.onChangeIndexed(PROJECT_NAME, changeId.get());
verify(poolMock, times(1))
- .execute(indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, false, null));
+ .execute(indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, null));
}
@Test
@@ -154,8 +157,7 @@
@Test
public void testIndexChangeTaskToString() throws Exception {
- IndexChangeTask task =
- indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, false, null);
+ 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));
@@ -178,33 +180,48 @@
@Test
public void testIndexChangeTaskHashCodeAndEquals() {
- IndexChangeTask task =
- indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID, false, null);
+ 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(PROJECT_NAME, CHANGE_ID, false, null);
+ 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(PROJECT_NAME, CHANGE_ID + 1, false, null)))
+ task.equals(indexEventHandler.new IndexChangeTask(PROJECT_NAME, CHANGE_ID + 1, null)))
.isFalse();
assertThat(task.hashCode()).isNotEqualTo("test".hashCode());
IndexChangeTask differentChangeIdTask =
- indexEventHandler.new IndexChangeTask(PROJECT_NAME, 123, false, null);
+ 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, null);
- 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
