Support disabling NoteDb reads for tests
In I0a9eb0ad we fixed a NoteDb-specific issue where loading the default search results page caused ChangeNotes to get loaded, despite not causing changes to load from ReviewDb. Support this configuration in tests by adding a method to NotesMigration that is called in AbstractChangeNotes#load(). In production this method always returns false so the cost is negligible. Change-Id: I22622cb3c834eeb1391d5b80247beafe36fffedf
This commit is contained in:
		@@ -529,6 +529,16 @@ public abstract class AbstractDaemonTest {
 | 
				
			|||||||
        atrScope.newContext(reviewDbProvider, null, anonymousUser.get()));
 | 
					        atrScope.newContext(reviewDbProvider, null, anonymousUser.get()));
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  protected Context disableDb() {
 | 
				
			||||||
 | 
					    notesMigration.setFailOnLoad(true);
 | 
				
			||||||
 | 
					    return atrScope.disableDb();
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  protected void enableDb(Context preDisableContext) {
 | 
				
			||||||
 | 
					    notesMigration.setFailOnLoad(false);
 | 
				
			||||||
 | 
					    atrScope.set(preDisableContext);
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  protected static Gson newGson() {
 | 
					  protected static Gson newGson() {
 | 
				
			||||||
    return OutputFormat.JSON_COMPACT.newGson();
 | 
					    return OutputFormat.JSON_COMPACT.newGson();
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -31,6 +31,7 @@ import com.google.common.base.Function;
 | 
				
			|||||||
import com.google.common.collect.ImmutableSet;
 | 
					import com.google.common.collect.ImmutableSet;
 | 
				
			||||||
import com.google.common.collect.Iterables;
 | 
					import com.google.common.collect.Iterables;
 | 
				
			||||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
 | 
					import com.google.gerrit.acceptance.AbstractDaemonTest;
 | 
				
			||||||
 | 
					import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
 | 
				
			||||||
import com.google.gerrit.acceptance.NoHttpd;
 | 
					import com.google.gerrit.acceptance.NoHttpd;
 | 
				
			||||||
import com.google.gerrit.acceptance.PushOneCommit;
 | 
					import com.google.gerrit.acceptance.PushOneCommit;
 | 
				
			||||||
import com.google.gerrit.acceptance.TestProjectInput;
 | 
					import com.google.gerrit.acceptance.TestProjectInput;
 | 
				
			||||||
@@ -983,16 +984,20 @@ public class ChangeIT extends AbstractDaemonTest {
 | 
				
			|||||||
    createChange();
 | 
					    createChange();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    setApiUserAnonymous(); // Identified user may async get stars from DB.
 | 
					    setApiUserAnonymous(); // Identified user may async get stars from DB.
 | 
				
			||||||
    atrScope.disableDb();
 | 
					    AcceptanceTestRequestScope.Context ctx = disableDb();
 | 
				
			||||||
    assertThat(gApi.changes().query()
 | 
					    try {
 | 
				
			||||||
          .withQuery(
 | 
					      assertThat(gApi.changes().query()
 | 
				
			||||||
            "project:{" + project.get() + "} (status:open OR status:closed)")
 | 
					            .withQuery(
 | 
				
			||||||
          // Options should match defaults in AccountDashboardScreen.
 | 
					              "project:{" + project.get() + "} (status:open OR status:closed)")
 | 
				
			||||||
          .withOption(ListChangesOption.LABELS)
 | 
					            // Options should match defaults in AccountDashboardScreen.
 | 
				
			||||||
          .withOption(ListChangesOption.DETAILED_ACCOUNTS)
 | 
					            .withOption(ListChangesOption.LABELS)
 | 
				
			||||||
          .withOption(ListChangesOption.REVIEWED)
 | 
					            .withOption(ListChangesOption.DETAILED_ACCOUNTS)
 | 
				
			||||||
          .get())
 | 
					            .withOption(ListChangesOption.REVIEWED)
 | 
				
			||||||
        .hasSize(2);
 | 
					            .get())
 | 
				
			||||||
 | 
					          .hasSize(2);
 | 
				
			||||||
 | 
					    } finally {
 | 
				
			||||||
 | 
					      enableDb(ctx);
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  @Test
 | 
					  @Test
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 | 
				
			|||||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 | 
					import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
 | 
					import com.google.gerrit.acceptance.AbstractDaemonTest;
 | 
				
			||||||
 | 
					import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
 | 
				
			||||||
import com.google.gerrit.acceptance.NoHttpd;
 | 
					import com.google.gerrit.acceptance.NoHttpd;
 | 
				
			||||||
import com.google.gerrit.acceptance.PushOneCommit;
 | 
					import com.google.gerrit.acceptance.PushOneCommit;
 | 
				
			||||||
import com.google.gerrit.common.data.AccessSection;
 | 
					import com.google.gerrit.common.data.AccessSection;
 | 
				
			||||||
@@ -77,7 +78,6 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
 | 
				
			|||||||
        .getGroupUUID();
 | 
					        .getGroupUUID();
 | 
				
			||||||
    setUpPermissions();
 | 
					    setUpPermissions();
 | 
				
			||||||
    setUpChanges();
 | 
					    setUpChanges();
 | 
				
			||||||
    atrScope.disableDb();
 | 
					 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  private void setUpPermissions() throws Exception {
 | 
					  private void setUpPermissions() throws Exception {
 | 
				
			||||||
@@ -269,6 +269,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
 | 
				
			|||||||
      }
 | 
					      }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    AcceptanceTestRequestScope.Context ctx = disableDb();
 | 
				
			||||||
    try (Repository repo = repoManager.openRepository(project)) {
 | 
					    try (Repository repo = repoManager.openRepository(project)) {
 | 
				
			||||||
      ProjectControl ctl = projectControlFactory.controlFor(project,
 | 
					      ProjectControl ctl = projectControlFactory.controlFor(project,
 | 
				
			||||||
          identifiedUserFactory.create(Providers.of(db), user.getId()));
 | 
					          identifiedUserFactory.create(Providers.of(db), user.getId()));
 | 
				
			||||||
@@ -277,6 +278,8 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
 | 
				
			|||||||
      Map<String, Ref> all = repo.getAllRefs();
 | 
					      Map<String, Ref> all = repo.getAllRefs();
 | 
				
			||||||
      assertThat(filter.filter(all, false).keySet())
 | 
					      assertThat(filter.filter(all, false).keySet())
 | 
				
			||||||
          .containsExactlyElementsIn(expected);
 | 
					          .containsExactlyElementsIn(expected);
 | 
				
			||||||
 | 
					    } finally {
 | 
				
			||||||
 | 
					      enableDb(ctx);
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -124,6 +124,9 @@ public abstract class AbstractChangeNotes<T> {
 | 
				
			|||||||
      loadDefaults();
 | 
					      loadDefaults();
 | 
				
			||||||
      return self();
 | 
					      return self();
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					    if (args.migration.failOnLoad()) {
 | 
				
			||||||
 | 
					      throw new OrmException("Reading from NoteDb is disabled");
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
    try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
 | 
					    try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
 | 
				
			||||||
        Repository repo = args.repoManager.openRepository(getProjectName());
 | 
					        Repository repo = args.repoManager.openRepository(getProjectName());
 | 
				
			||||||
        LoadHandle handle = openHandle(repo)) {
 | 
					        LoadHandle handle = openHandle(repo)) {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -42,6 +42,15 @@ public abstract class NotesMigration {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
  public abstract boolean writeAccounts();
 | 
					  public abstract boolean writeAccounts();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  /**
 | 
				
			||||||
 | 
					   * Whether to fail when reading any data from NoteDb.
 | 
				
			||||||
 | 
					   * <p>
 | 
				
			||||||
 | 
					   * Used in conjunction with {@link #readChanges()} for tests.
 | 
				
			||||||
 | 
					   */
 | 
				
			||||||
 | 
					  public boolean failOnLoad() {
 | 
				
			||||||
 | 
					    return false;
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public boolean enabled() {
 | 
					  public boolean enabled() {
 | 
				
			||||||
    return writeChanges() || readChanges()
 | 
					    return writeChanges() || readChanges()
 | 
				
			||||||
        || writeAccounts() || readAccounts();
 | 
					        || writeAccounts() || readAccounts();
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -22,6 +22,7 @@ import com.google.inject.Singleton;
 | 
				
			|||||||
public class TestNotesMigration extends NotesMigration {
 | 
					public class TestNotesMigration extends NotesMigration {
 | 
				
			||||||
  private volatile boolean readChanges;
 | 
					  private volatile boolean readChanges;
 | 
				
			||||||
  private volatile boolean writeChanges;
 | 
					  private volatile boolean writeChanges;
 | 
				
			||||||
 | 
					  private volatile boolean failOnLoad;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  @Override
 | 
					  @Override
 | 
				
			||||||
  public boolean readChanges() {
 | 
					  public boolean readChanges() {
 | 
				
			||||||
@@ -43,6 +44,11 @@ public class TestNotesMigration extends NotesMigration {
 | 
				
			|||||||
    return false;
 | 
					    return false;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  @Override
 | 
				
			||||||
 | 
					  public boolean failOnLoad() {
 | 
				
			||||||
 | 
					    return failOnLoad;
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public TestNotesMigration setReadChanges(boolean readChanges) {
 | 
					  public TestNotesMigration setReadChanges(boolean readChanges) {
 | 
				
			||||||
    this.readChanges = readChanges;
 | 
					    this.readChanges = readChanges;
 | 
				
			||||||
    return this;
 | 
					    return this;
 | 
				
			||||||
@@ -53,6 +59,11 @@ public class TestNotesMigration extends NotesMigration {
 | 
				
			|||||||
    return this;
 | 
					    return this;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  public TestNotesMigration setFailOnLoad(boolean failOnLoad) {
 | 
				
			||||||
 | 
					    this.failOnLoad = failOnLoad;
 | 
				
			||||||
 | 
					    return this;
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public TestNotesMigration setAllEnabled(boolean enabled) {
 | 
					  public TestNotesMigration setAllEnabled(boolean enabled) {
 | 
				
			||||||
    return setReadChanges(enabled).setWriteChanges(enabled);
 | 
					    return setReadChanges(enabled).setWriteChanges(enabled);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user