Merge "Test for null values in DefaultRefFilter"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 8d05ea1..0e62938 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -799,14 +799,13 @@
return accountState.get();
}
- protected Context disableDb() {
+ protected AutoCloseable disableNoteDb() {
changeNotesArgs.failOnLoadForTest.set(true);
- return atrScope.disableDb();
- }
-
- protected void enableDb(Context preDisableContext) {
- changeNotesArgs.failOnLoadForTest.set(false);
- atrScope.set(preDisableContext);
+ Context oldContext = atrScope.disableNoteDb();
+ return () -> {
+ changeNotesArgs.failOnLoadForTest.set(false);
+ atrScope.set(oldContext);
+ };
}
protected void disableChangeIndexWrites() {
@@ -827,55 +826,49 @@
protected AutoCloseable disableChangeIndex() {
disableChangeIndexWrites();
- ChangeIndex searchIndex = changeIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledChangeIndex)) {
- changeIndexes.setSearchIndex(new DisabledChangeIndex(searchIndex), false);
+ ChangeIndex maybeDisabledSearchIndex = changeIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledChangeIndex)) {
+ changeIndexes.setSearchIndex(new DisabledChangeIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() throws Exception {
- enableChangeIndexWrites();
- ChangeIndex searchIndex = changeIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledChangeIndex) {
- changeIndexes.setSearchIndex(((DisabledChangeIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ enableChangeIndexWrites();
+ ChangeIndex maybeEnabledSearchIndex = changeIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledChangeIndex) {
+ changeIndexes.setSearchIndex(
+ ((DisabledChangeIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
protected AutoCloseable disableAccountIndex() {
- AccountIndex searchIndex = accountIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledAccountIndex)) {
- accountIndexes.setSearchIndex(new DisabledAccountIndex(searchIndex), false);
+ AccountIndex maybeDisabledSearchIndex = accountIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledAccountIndex)) {
+ accountIndexes.setSearchIndex(new DisabledAccountIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() {
- AccountIndex searchIndex = accountIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledAccountIndex) {
- accountIndexes.setSearchIndex(((DisabledAccountIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ AccountIndex maybeEnabledSearchIndex = accountIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledAccountIndex) {
+ accountIndexes.setSearchIndex(
+ ((DisabledAccountIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
protected AutoCloseable disableProjectIndex() {
disableProjectIndexWrites();
- ProjectIndex searchIndex = projectIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledProjectIndex)) {
- projectIndexes.setSearchIndex(new DisabledProjectIndex(searchIndex), false);
+ ProjectIndex maybeDisabledSearchIndex = projectIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledProjectIndex)) {
+ projectIndexes.setSearchIndex(new DisabledProjectIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() {
- enableProjectIndexWrites();
- ProjectIndex searchIndex = projectIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledProjectIndex) {
- projectIndexes.setSearchIndex(((DisabledProjectIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ enableProjectIndexWrites();
+ ProjectIndex maybeEnabledSearchIndex = projectIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledProjectIndex) {
+ projectIndexes.setSearchIndex(
+ ((DisabledProjectIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
diff --git a/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java b/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
index 8420ff2..50536d8 100644
--- a/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
+++ b/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
@@ -145,7 +145,10 @@
return current.get();
}
- public Context disableDb() {
+ /**
+ * Disables read and write access to NoteDb and returns the context prior to that modification.
+ */
+ public Context disableNoteDb() {
Context old = current.get();
Context ctx = new Context(old.session, old.user, old.created);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 81acb3f..31de4cf 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -58,7 +58,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
@@ -2735,8 +2734,7 @@
createChange();
requestScopeOperations.setApiUser(user.getId());
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertThat(
gApi.changes()
.query()
@@ -2747,8 +2745,6 @@
.withOption(REVIEWED)
.get())
.hasSize(2);
- } finally {
- enableDb(ctx);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 90f0a2a..b00837d 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -24,7 +24,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -923,16 +922,13 @@
addRobotComment(r2.getChangeId(), createRobotCommentInputWithMandatoryFields());
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo result = Iterables.getOnlyElement(query(r2.getChangeId()));
// currently, we create all robot comments as 'resolved' by default.
// if we allow users to resolve a robot comment, then this test should
// be modified.
assertThat(result.unresolvedCommentCount).isEqualTo(0);
assertThat(result.totalCommentCount).isEqualTo(1);
- } finally {
- enableDb(ctx);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java b/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java
index 3939c67..10e701e 100644
--- a/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -67,15 +66,12 @@
ChangeData change = createChange().getChange();
// TODO(hiesel) Rework as AutoClosable. Here and below.
changeRefCache.resetBootstrappedProjects();
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertUploadPackRefs(
"HEAD",
"refs/heads/master",
RefNames.changeMetaRef(change.getId()),
change.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
}
}
@@ -87,29 +83,22 @@
public void serveResultsFromCacheAfterInitialBootstrap() throws Exception {
ChangeData change = createChange().getChange();
changeRefCache.resetBootstrappedProjects();
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertUploadPackRefs(
"HEAD",
"refs/heads/master",
RefNames.changeMetaRef(change.getId()),
change.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
}
- // No change since our first call, so this time we don't bootstrap or touch NoteDb
- AcceptanceTestRequestScope.Context ctx2 = disableDb();
- try {
- try (AutoCloseable ignored = disableChangeIndex()) {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change.getId()),
- change.currentPatchSet().getId().toRefName());
- }
- } finally {
- enableDb(ctx2);
+ // No change since our first call, so this time we don't bootstrap or touch the NoteDb
+ try (AutoCloseable ignored = disableChangeIndex();
+ AutoCloseable ignored2 = disableNoteDb()) {
+ assertUploadPackRefs(
+ "HEAD",
+ "refs/heads/master",
+ RefNames.changeMetaRef(change.getId()),
+ change.currentPatchSet().getId().toRefName());
}
}
@@ -120,17 +109,14 @@
@Test
public void useIndexForBootstrappingAndDbForDeltaReload() throws Exception {
ChangeData change1 = createChange().getChange();
- AcceptanceTestRequestScope.Context ctx = disableDb();
// Bootstrap: No NoteDb access as we expect it to use the index.
changeRefCache.resetBootstrappedProjects();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertUploadPackRefs(
"HEAD",
"refs/heads/master",
RefNames.changeMetaRef(change1.getId()),
change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
}
// Delta reload: No index access as we expect it to use NoteDb.
ChangeData change2 = createChange().getChange();
@@ -157,17 +143,14 @@
.to("refs/for/master")
.getChange();
- AcceptanceTestRequestScope.Context ctx = disableDb();
// Bootstrap: No NoteDb access as we expect it to use the index.
changeRefCache.resetBootstrappedProjects();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertUploadPackRefs(
"HEAD",
"refs/heads/master",
RefNames.changeMetaRef(change1.getId()),
change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
}
// Delta reload: No index access as we expect it to use NoteDb.
@@ -201,16 +184,13 @@
.to("refs/for/master")
.getChange();
// Bootstrap: No NoteDb access as we expect it to use the index.
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
changeRefCache.resetBootstrappedProjects();
assertUploadPackRefs(
"HEAD",
"refs/heads/master",
RefNames.changeMetaRef(change1.getId()),
change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
}
try (AutoCloseable ignored = disableChangeIndex()) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index 13c76e3..b5d3838 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -323,14 +322,11 @@
input.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(input);
- Context oldCtx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo info =
Iterables.getOnlyElement(
gApi.changes().query(r.getChangeId()).withOption(DETAILED_LABELS).get());
assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc)));
- } finally {
- enableDb(oldCtx);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 0a4d972..d5ceb9a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -27,7 +27,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -734,8 +733,7 @@
assertThat(comments.get(FILE_NAME)).hasSize(1);
addComment(result, "comment 2", false, true, comments.get(FILE_NAME).get(0).id);
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo changeInfo1 = Iterables.getOnlyElement(query(changeId1));
ChangeInfo changeInfo2 = Iterables.getOnlyElement(query(changeId2));
ChangeInfo changeInfo3 = Iterables.getOnlyElement(query(changeId3));
@@ -745,8 +743,6 @@
assertThat(changeInfo2.totalCommentCount).isEqualTo(2);
assertThat(changeInfo3.unresolvedCommentCount).isEqualTo(1);
assertThat(changeInfo3.totalCommentCount).isEqualTo(2);
- } finally {
- enableDb(ctx);
}
}
