Merge "Avoid @Sandboxed annotations by resetting project states after each test"

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -195,7 +195,7 @@ public abstract class AbstractDaemonTest {
                 firstTest = description;
               }
               beforeTest(description);
-              try {
+              try (ProjectResetter resetter = resetProjects(projectResetter.builder())) {
                 base.evaluate();
               } finally {
                 afterTest();
@@ -229,6 +229,7 @@ public abstract class AbstractDaemonTest {
   @Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
   @Inject protected PatchSetUtil psUtil;
   @Inject protected ProjectCache projectCache;
+  @Inject protected ProjectResetter.Builder.Factory projectResetter;
   @Inject protected Provider<InternalChangeQuery> queryProvider;
   @Inject protected PushOneCommit.Factory pushFactory;
   @Inject protected PluginConfigFactory pluginConfig;
@@ -304,6 +305,25 @@ public abstract class AbstractDaemonTest {
     TempFileUtil.cleanup();
   }
 
+  /** Controls which project and branches should be reset after each test case. */
+  protected ProjectResetter resetProjects(ProjectResetter.Builder resetter) throws IOException {
+    return resetter
+        // Don't reset all refs so that refs/sequences/changes is not touched and change IDs are
+        // not reused.
+        .reset(allProjects, RefNames.REFS_CONFIG)
+        // Don't reset group branches since this would make the groups inconsistent between
+        // ReviewDb and NoteDb.
+        // Don't reset refs/sequences/accounts so that account IDs are not reused.
+        .reset(
+            allUsers,
+            RefNames.REFS_CONFIG,
+            RefNames.REFS_USERS + "*",
+            RefNames.REFS_EXTERNAL_IDS,
+            RefNames.REFS_STARRED_CHANGES + "*",
+            RefNames.REFS_DRAFT_COMMENTS + "*")
+        .build();
+  }
+
   protected static Config submitWholeTopicEnabledConfig() {
     Config cfg = new Config();
     cfg.setBoolean("change", null, "submitWholeTopic", true);

java/com/google/gerrit/acceptance/AbstractNotificationTest.java

@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.truth.FailureMetadata;
 import com.google.common.truth.Subject;
 import com.google.common.truth.Truth;
+import com.google.gerrit.acceptance.ProjectResetter.Builder;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -61,6 +62,13 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest {
     gApi.projects().name(project.get()).config(conf);
   }
 
+  @Override
+  protected ProjectResetter resetProjects(Builder resetter) throws IOException {
+    // Don't reset anything so that stagedUsers can be cached across all tests.
+    // Without this caching these tests become much too slow.
+    return resetter.build();
+  }
+
   protected static FakeEmailSenderSubject assertThat(FakeEmailSender sender) {
     return assertAbout(FakeEmailSenderSubject::new).that(sender);
   }

java/com/google/gerrit/acceptance/AccountCreator.java

@@ -47,6 +47,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -183,6 +184,10 @@ public class AccountCreator {
     return checkNotNull(accounts.get(username), "No TestAccount created for %s", username);
   }
 
+  public void evict(Collection<Account.Id> ids) {
+    accounts.values().removeIf(a -> ids.contains(a.id));
+  }
+
   public static KeyPair genSshKey() throws JSchException {
     JSch jsch = new JSch();
     return KeyPair.genKeyPair(jsch, KeyPair.RSA);

java/com/google/gerrit/acceptance/GerritServer.java

@@ -407,6 +407,7 @@ public class GerritServer implements AutoCloseable {
             install(InProcessProtocol.module());
             install(new NoSshModule());
             install(new AsyncReceiveCommits.Module());
+            factory(ProjectResetter.Builder.Factory.class);
           }
         };
     return sysInjector.createChildInjector(module);

java/com/google/gerrit/acceptance/ProjectResetter.java

@@ -0,0 +1,320 @@
+// Copyright (C) 2017 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.google.gerrit.acceptance;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_USERS;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.index.RefState;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.RefPatternMatcher;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Saves the states of given projects and resets the project states on close.
+ *
+ * <p>Saving the project states is done by saving the states of all refs in the project. On close
+ * those refs are reset to the saved states. Refs that were newly created are deleted.
+ *
+ * <p>By providing ref patterns per project it can be controlled which refs should be reset on
+ * close.
+ *
+ * <p>If resetting touches {@code refs/meta/config} branches the corresponding projects are evicted
+ * from the project cache.
+ *
+ * <p>If resetting touches user branches or the {@code refs/meta/external-ids} branch the
+ * corresponding accounts are evicted from the account cache and also if needed from the cache in
+ * {@link AccountCreator}.
+ *
+ * <p>At the moment this class has the following limitations:
+ *
+ * <ul>
+ *   <li>Resetting group branches doesn't evict the corresponding groups from the group cache.
+ *   <li>Changes are not reindexed if change meta refs are reset.
+ *   <li>Changes are not reindexed if starred-changes refs in All-Users are reset.
+ *   <li>If accounts are deleted changes may still refer to these accounts (e.g. as reviewers).
+ * </ul>
+ *
+ * Primarily this class is intended to reset the states of the All-Projects and All-Users projects
+ * after each test. These projects rarely contain changes and it's currently not a problem if these
+ * changes get stale. For creating changes each test gets a brand new project. Since this project is
+ * not used outside of the test method that creates it, it doesn't need to be reset.
+ */
+public class ProjectResetter implements AutoCloseable {
+  public static class Builder {
+    public interface Factory {
+      Builder builder();
+    }
+
+    private final GitRepositoryManager repoManager;
+    private final AllUsersName allUsersName;
+    @Nullable private final AccountCreator accountCreator;
+    @Nullable private final AccountCache accountCache;
+    @Nullable private final ProjectCache projectCache;
+
+    private final Multimap<Project.NameKey, String> refsByProject;
+
+    @Inject
+    public Builder(
+        GitRepositoryManager repoManager,
+        AllUsersName allUsersName,
+        @Nullable AccountCreator accountCreator,
+        @Nullable AccountCache accountCache,
+        @Nullable ProjectCache projectCache) {
+      this.repoManager = repoManager;
+      this.allUsersName = allUsersName;
+      this.accountCreator = accountCreator;
+      this.accountCache = accountCache;
+      this.projectCache = projectCache;
+      this.refsByProject = MultimapBuilder.hashKeys().arrayListValues().build();
+    }
+
+    public Builder reset(Project.NameKey project, String... refPatterns) {
+      List<String> refPatternList = Arrays.asList(refPatterns);
+      if (refPatternList.isEmpty()) {
+        refPatternList = ImmutableList.of(RefNames.REFS + "*");
+      }
+      refsByProject.putAll(project, refPatternList);
+      return this;
+    }
+
+    public ProjectResetter build() throws IOException {
+      return new ProjectResetter(
+          repoManager, allUsersName, accountCreator, accountCache, projectCache, refsByProject);
+    }
+  }
+
+  private final GitRepositoryManager repoManager;
+  private final AllUsersName allUsersName;
+  @Nullable private final AccountCreator accountCreator;
+  @Nullable private final AccountCache accountCache;
+  @Nullable private final ProjectCache projectCache;
+  private final Multimap<Project.NameKey, String> refsPatternByProject;
+  private final Multimap<Project.NameKey, RefState> savedRefStatesByProject;
+
+  private Multimap<Project.NameKey, String> keptRefsByProject;
+  private Multimap<Project.NameKey, String> restoredRefsByProject;
+  private Multimap<Project.NameKey, String> deletedRefsByProject;
+
+  private ProjectResetter(
+      GitRepositoryManager repoManager,
+      AllUsersName allUsersName,
+      @Nullable AccountCreator accountCreator,
+      @Nullable AccountCache accountCache,
+      @Nullable ProjectCache projectCache,
+      Multimap<Project.NameKey, String> refPatternByProject)
+      throws IOException {
+    this.repoManager = repoManager;
+    this.allUsersName = allUsersName;
+    this.accountCreator = accountCreator;
+    this.accountCache = accountCache;
+    this.projectCache = projectCache;
+    this.refsPatternByProject = refPatternByProject;
+    this.savedRefStatesByProject = readRefStates();
+  }
+
+  @Override
+  public void close() throws Exception {
+    keptRefsByProject = MultimapBuilder.hashKeys().arrayListValues().build();
+    restoredRefsByProject = MultimapBuilder.hashKeys().arrayListValues().build();
+    deletedRefsByProject = MultimapBuilder.hashKeys().arrayListValues().build();
+
+    restoreRefs();
+    deleteNewlyCreatedRefs();
+    evictCachesAndReindex();
+  }
+
+  /** Read the states of all matching refs. */
+  private Multimap<Project.NameKey, RefState> readRefStates() throws IOException {
+    Multimap<Project.NameKey, RefState> refStatesByProject =
+        MultimapBuilder.hashKeys().arrayListValues().build();
+    for (Map.Entry<Project.NameKey, Collection<String>> e :
+        refsPatternByProject.asMap().entrySet()) {
+      try (Repository repo = repoManager.openRepository(e.getKey())) {
+        Collection<Ref> refs = repo.getAllRefs().values();
+        for (String refPattern : e.getValue()) {
+          RefPatternMatcher matcher = RefPatternMatcher.getMatcher(refPattern);
+          for (Ref ref : refs) {
+            if (matcher.match(ref.getName(), null)) {
+              refStatesByProject.put(e.getKey(), RefState.create(ref.getName(), ref.getObjectId()));
+            }
+          }
+        }
+      }
+    }
+    return refStatesByProject;
+  }
+
+  private void restoreRefs() throws IOException {
+    for (Map.Entry<Project.NameKey, Collection<RefState>> e :
+        savedRefStatesByProject.asMap().entrySet()) {
+      try (Repository repo = repoManager.openRepository(e.getKey())) {
+        for (RefState refState : e.getValue()) {
+          if (refState.match(repo)) {
+            keptRefsByProject.put(e.getKey(), refState.ref());
+            continue;
+          }
+          Ref ref = repo.exactRef(refState.ref());
+          RefUpdate updateRef = repo.updateRef(refState.ref());
+          updateRef.setExpectedOldObjectId(ref != null ? ref.getObjectId() : ObjectId.zeroId());
+          updateRef.setNewObjectId(refState.id());
+          updateRef.setForceUpdate(true);
+          RefUpdate.Result result = updateRef.update();
+          checkState(
+              result == RefUpdate.Result.FORCED || result == RefUpdate.Result.NEW,
+              "resetting branch %s in %s failed",
+              refState.ref(),
+              e.getKey());
+          restoredRefsByProject.put(e.getKey(), refState.ref());
+        }
+      }
+    }
+  }
+
+  private void deleteNewlyCreatedRefs() throws IOException {
+    for (Map.Entry<Project.NameKey, Collection<String>> e :
+        refsPatternByProject.asMap().entrySet()) {
+      try (Repository repo = repoManager.openRepository(e.getKey())) {
+        Collection<Ref> nonRestoredRefs =
+            repo.getAllRefs()
+                .values()
+                .stream()
+                .filter(
+                    r ->
+                        !keptRefsByProject.containsEntry(e.getKey(), r.getName())
+                            && !restoredRefsByProject.containsEntry(e.getKey(), r.getName()))
+                .collect(toSet());
+        for (String refPattern : e.getValue()) {
+          RefPatternMatcher matcher = RefPatternMatcher.getMatcher(refPattern);
+          for (Ref ref : nonRestoredRefs) {
+            if (matcher.match(ref.getName(), null)
+                && !deletedRefsByProject.containsEntry(e.getKey(), ref.getName())) {
+              RefUpdate updateRef = repo.updateRef(ref.getName());
+              updateRef.setExpectedOldObjectId(ref.getObjectId());
+              updateRef.setNewObjectId(ObjectId.zeroId());
+              updateRef.setForceUpdate(true);
+              RefUpdate.Result result = updateRef.delete();
+              checkState(
+                  result == RefUpdate.Result.FORCED,
+                  "deleting branch %s in %s failed",
+                  ref.getName(),
+                  e.getKey());
+              deletedRefsByProject.put(e.getKey(), ref.getName());
+            }
+          }
+        }
+      }
+    }
+  }
+
+  private void evictCachesAndReindex() throws IOException {
+    evictAndReindexProjects();
+    evictAndReindexAccounts();
+
+    // TODO(ekempin): Evict groups from cache if group refs were modified.
+    // TODO(ekempin): Reindex changes if starred-changes refs in All-Users were modified.
+  }
+
+  /** Evict projects for which the config was changed. */
+  private void evictAndReindexProjects() throws IOException {
+    if (projectCache == null) {
+      return;
+    }
+
+    for (Project.NameKey project :
+        Sets.union(
+            projectsWithConfigChanges(restoredRefsByProject),
+            projectsWithConfigChanges(deletedRefsByProject))) {
+      projectCache.evict(project);
+    }
+  }
+
+  private Set<Project.NameKey> projectsWithConfigChanges(
+      Multimap<Project.NameKey, String> projects) {
+    return projects
+        .entries()
+        .stream()
+        .filter(e -> e.getValue().equals(RefNames.REFS_CONFIG))
+        .map(Map.Entry::getKey)
+        .collect(toSet());
+  }
+
+  /** Evict accounts that were modified. */
+  private void evictAndReindexAccounts() throws IOException {
+    Set<Account.Id> deletedAccounts = accountIds(deletedRefsByProject.get(allUsersName));
+    if (accountCreator != null) {
+      accountCreator.evict(deletedAccounts);
+    }
+    if (accountCache != null) {
+      Set<Account.Id> modifiedAccounts =
+          new HashSet<>(accountIds(restoredRefsByProject.get(allUsersName)));
+
+      if (restoredRefsByProject.get(allUsersName).contains(RefNames.REFS_EXTERNAL_IDS)
+          || deletedRefsByProject.get(allUsersName).contains(RefNames.REFS_EXTERNAL_IDS)) {
+        // The external IDs have been modified but we don't know which accounts were affected.
+        // Make sure all accounts are evicted and reindexed.
+        try (Repository repo = repoManager.openRepository(allUsersName)) {
+          for (Account.Id id :
+              accountIds(
+                  repo.getAllRefs().values().stream().map(r -> r.getName()).collect(toSet()))) {
+            accountCache.evict(id);
+          }
+        }
+
+        // Remove deleted accounts from the cache and index.
+        for (Account.Id id : deletedAccounts) {
+          accountCache.evict(id);
+        }
+      } else {
+        // Evict and reindex all modified and deleted accounts.
+        for (Account.Id id : Sets.union(modifiedAccounts, deletedAccounts)) {
+          accountCache.evict(id);
+        }
+      }
+    }
+  }
+
+  private Set<Account.Id> accountIds(Collection<String> refs) {
+    return refs.stream()
+        .filter(r -> r.startsWith(REFS_USERS))
+        .map(r -> Account.Id.fromRef(r))
+        .filter(Objects::nonNull)
+        .collect(toSet());
+  }
+}

javatests/com/google/gerrit/acceptance/ProjectResetterTest.java

@@ -0,0 +1,442 @@
+// Copyright (C) 2017 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.google.gerrit.acceptance;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.testing.GerritBaseTests;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import com.google.gerrit.testing.TestTimeUtil;
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import org.easymock.EasyMock;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ProjectResetterTest extends GerritBaseTests {
+  private InMemoryRepositoryManager repoManager;
+  private Project.NameKey project;
+  private Repository repo;
+
+  @Before
+  public void setUp() throws Exception {
+    repoManager = new InMemoryRepositoryManager();
+    project = new Project.NameKey("foo");
+    repo = repoManager.createRepository(project);
+  }
+
+  @Before
+  public void setTimeForTesting() {
+    TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+  }
+
+  @After
+  public void resetTime() {
+    TestTimeUtil.useSystemTime();
+  }
+
+  @Test
+  public void resetAllRefs() throws Exception {
+    Ref matchingRef = createRef("refs/any/test");
+
+    try (ProjectResetter resetProject = builder().reset(project).build()) {
+      updateRef(matchingRef);
+    }
+
+    // The matching refs are reset to the old state.
+    assertRef(matchingRef);
+  }
+
+  @Test
+  public void onlyResetMatchingRefs() throws Exception {
+    Ref matchingRef = createRef("refs/match/test");
+    Ref anotherMatchingRef = createRef("refs/another-match/test");
+    Ref nonMatchingRef = createRef("refs/no-match/test");
+
+    Ref updatedNonMatchingRef;
+    try (ProjectResetter resetProject =
+        builder().reset(project, "refs/match/*", "refs/another-match/*").build()) {
+      updateRef(matchingRef);
+      updateRef(anotherMatchingRef);
+      updatedNonMatchingRef = updateRef(nonMatchingRef);
+    }
+
+    // The matching refs are reset to the old state.
+    assertRef(matchingRef);
+    assertRef(anotherMatchingRef);
+
+    // The non-matching ref is not reset, hence it still has the updated state.
+    assertRef(updatedNonMatchingRef);
+  }
+
+  @Test
+  public void onlyDeleteNewlyCreatedMatchingRefs() throws Exception {
+    Ref matchingRef;
+    Ref anotherMatchingRef;
+    Ref nonMatchingRef;
+    try (ProjectResetter resetProject =
+        builder().reset(project, "refs/match/*", "refs/another-match/*").build()) {
+      matchingRef = createRef("refs/match/test");
+      anotherMatchingRef = createRef("refs/another-match/test");
+      nonMatchingRef = createRef("refs/no-match/test");
+    }
+
+    // The matching refs are deleted since they didn't exist before.
+    assertDeletedRef(matchingRef);
+    assertDeletedRef(anotherMatchingRef);
+
+    // The non-matching ref is not deleted.
+    assertRef(nonMatchingRef);
+  }
+
+  @Test
+  public void onlyResetMatchingRefsMultipleProjects() throws Exception {
+    Project.NameKey project2 = new Project.NameKey("bar");
+    Repository repo2 = repoManager.createRepository(project2);
+
+    Ref matchingRefProject1 = createRef("refs/foo/test");
+    Ref nonMatchingRefProject1 = createRef("refs/bar/test");
+
+    Ref matchingRefProject2 = createRef(repo2, "refs/bar/test");
+    Ref nonMatchingRefProject2 = createRef(repo2, "refs/foo/test");
+
+    Ref updatedNonMatchingRefProject1;
+    Ref updatedNonMatchingRefProject2;
+    try (ProjectResetter resetProject =
+        builder().reset(project, "refs/foo/*").reset(project2, "refs/bar/*").build()) {
+      updateRef(matchingRefProject1);
+      updatedNonMatchingRefProject1 = updateRef(nonMatchingRefProject1);
+
+      updateRef(repo2, matchingRefProject2);
+      updatedNonMatchingRefProject2 = updateRef(repo2, nonMatchingRefProject2);
+    }
+
+    // The matching refs are reset to the old state.
+    assertRef(matchingRefProject1);
+    assertRef(repo2, matchingRefProject2);
+
+    // The non-matching refs are not reset, hence they still has the updated states.
+    assertRef(updatedNonMatchingRefProject1);
+    assertRef(repo2, updatedNonMatchingRefProject2);
+  }
+
+  @Test
+  public void onlyDeleteNewlyCreatedMatchingRefsMultipleProjects() throws Exception {
+    Project.NameKey project2 = new Project.NameKey("bar");
+    Repository repo2 = repoManager.createRepository(project2);
+
+    Ref matchingRefProject1;
+    Ref nonMatchingRefProject1;
+    Ref matchingRefProject2;
+    Ref nonMatchingRefProject2;
+    try (ProjectResetter resetProject =
+        builder().reset(project, "refs/foo/*").reset(project2, "refs/bar/*").build()) {
+      matchingRefProject1 = createRef("refs/foo/test");
+      nonMatchingRefProject1 = createRef("refs/bar/test");
+
+      matchingRefProject2 = createRef(repo2, "refs/bar/test");
+      nonMatchingRefProject2 = createRef(repo2, "refs/foo/test");
+    }
+
+    // The matching refs are deleted since they didn't exist before.
+    assertDeletedRef(matchingRefProject1);
+    assertDeletedRef(repo2, matchingRefProject2);
+
+    // The non-matching ref is not deleted.
+    assertRef(nonMatchingRefProject1);
+    assertRef(repo2, nonMatchingRefProject2);
+  }
+
+  @Test
+  public void onlyDeleteNewlyCreatedWithOverlappingRefPatterns() throws Exception {
+    Ref matchingRef;
+    try (ProjectResetter resetProject =
+        builder().reset(project, "refs/match/*", "refs/match/test").build()) {
+      // This ref matches 2 ref pattern, ProjectResetter should try to delete it only once.
+      matchingRef = createRef("refs/match/test");
+    }
+
+    // The matching ref is deleted since it didn't exist before.
+    assertDeletedRef(matchingRef);
+  }
+
+  @Test
+  public void projectEvictionIfRefsMetaConfigIsReset() throws Exception {
+    Project.NameKey project2 = new Project.NameKey("bar");
+    Repository repo2 = repoManager.createRepository(project2);
+    Ref metaConfig = createRef(repo2, RefNames.REFS_CONFIG);
+
+    ProjectCache projectCache = EasyMock.createNiceMock(ProjectCache.class);
+    projectCache.evict(project2);
+    EasyMock.expectLastCall();
+    EasyMock.replay(projectCache);
+
+    Ref nonMetaConfig = createRef("refs/heads/master");
+
+    try (ProjectResetter resetProject =
+        builder(null, null, projectCache).reset(project).reset(project2).build()) {
+      updateRef(nonMetaConfig);
+      updateRef(repo2, metaConfig);
+    }
+
+    EasyMock.verify(projectCache);
+  }
+
+  @Test
+  public void projectEvictionIfRefsMetaConfigIsDeleted() throws Exception {
+    Project.NameKey project2 = new Project.NameKey("bar");
+    Repository repo2 = repoManager.createRepository(project2);
+
+    ProjectCache projectCache = EasyMock.createNiceMock(ProjectCache.class);
+    projectCache.evict(project2);
+    EasyMock.expectLastCall();
+    EasyMock.replay(projectCache);
+
+    try (ProjectResetter resetProject =
+        builder(null, null, projectCache).reset(project).reset(project2).build()) {
+      createRef("refs/heads/master");
+      createRef(repo2, RefNames.REFS_CONFIG);
+    }
+
+    EasyMock.verify(projectCache);
+  }
+
+  @Test
+  public void accountEvictionIfUserBranchIsReset() throws Exception {
+    Account.Id accountId = new Account.Id(1);
+    Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+    Repository allUsersRepo = repoManager.createRepository(allUsers);
+    Ref userBranch = createRef(allUsersRepo, RefNames.refsUsers(accountId));
+
+    AccountCache accountCache = EasyMock.createNiceMock(AccountCache.class);
+    accountCache.evict(accountId);
+    EasyMock.expectLastCall();
+    EasyMock.replay(accountCache);
+
+    // Non-user branch because it's not in All-Users.
+    Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(2)));
+
+    try (ProjectResetter resetProject =
+        builder(null, accountCache, null).reset(project).reset(allUsers).build()) {
+      updateRef(nonUserBranch);
+      updateRef(allUsersRepo, userBranch);
+    }
+
+    EasyMock.verify(accountCache);
+  }
+
+  @Test
+  public void accountEvictionIfUserBranchIsDeleted() throws Exception {
+    Account.Id accountId = new Account.Id(1);
+    Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+    Repository allUsersRepo = repoManager.createRepository(allUsers);
+
+    AccountCache accountCache = EasyMock.createNiceMock(AccountCache.class);
+    accountCache.evict(accountId);
+    EasyMock.expectLastCall();
+    EasyMock.replay(accountCache);
+
+    try (ProjectResetter resetProject =
+        builder(null, accountCache, null).reset(project).reset(allUsers).build()) {
+      // Non-user branch because it's not in All-Users.
+      createRef(RefNames.refsUsers(new Account.Id(2)));
+
+      createRef(allUsersRepo, RefNames.refsUsers(accountId));
+    }
+
+    EasyMock.verify(accountCache);
+  }
+
+  @Test
+  public void accountEvictionIfExternalIdsBranchIsReset() throws Exception {
+    Account.Id accountId = new Account.Id(1);
+    Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+    Repository allUsersRepo = repoManager.createRepository(allUsers);
+    Ref externalIds = createRef(allUsersRepo, RefNames.REFS_EXTERNAL_IDS);
+    createRef(allUsersRepo, RefNames.refsUsers(accountId));
+
+    Account.Id accountId2 = new Account.Id(2);
+
+    AccountCache accountCache = EasyMock.createNiceMock(AccountCache.class);
+    accountCache.evict(accountId);
+    EasyMock.expectLastCall();
+    accountCache.evict(accountId2);
+    EasyMock.expectLastCall();
+    EasyMock.replay(accountCache);
+
+    // Non-user branch because it's not in All-Users.
+    Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3)));
+
+    try (ProjectResetter resetProject =
+        builder(null, accountCache, null).reset(project).reset(allUsers).build()) {
+      updateRef(nonUserBranch);
+      updateRef(allUsersRepo, externalIds);
+      createRef(allUsersRepo, RefNames.refsUsers(accountId2));
+    }
+
+    EasyMock.verify(accountCache);
+  }
+
+  @Test
+  public void accountEvictionIfExternalIdsBranchIsDeleted() throws Exception {
+    Account.Id accountId = new Account.Id(1);
+    Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+    Repository allUsersRepo = repoManager.createRepository(allUsers);
+    createRef(allUsersRepo, RefNames.refsUsers(accountId));
+
+    Account.Id accountId2 = new Account.Id(2);
+
+    AccountCache accountCache = EasyMock.createNiceMock(AccountCache.class);
+    accountCache.evict(accountId);
+    EasyMock.expectLastCall();
+    accountCache.evict(accountId2);
+    EasyMock.expectLastCall();
+    EasyMock.replay(accountCache);
+
+    // Non-user branch because it's not in All-Users.
+    Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3)));
+
+    try (ProjectResetter resetProject =
+        builder(null, accountCache, null).reset(project).reset(allUsers).build()) {
+      updateRef(nonUserBranch);
+      createRef(allUsersRepo, RefNames.REFS_EXTERNAL_IDS);
+      createRef(allUsersRepo, RefNames.refsUsers(accountId2));
+    }
+
+    EasyMock.verify(accountCache);
+  }
+
+  @Test
+  public void accountEvictionFromAccountCreatorIfUserBranchIsDeleted() throws Exception {
+    Account.Id accountId = new Account.Id(1);
+    Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+    Repository allUsersRepo = repoManager.createRepository(allUsers);
+
+    AccountCreator accountCreator = EasyMock.createNiceMock(AccountCreator.class);
+    accountCreator.evict(ImmutableSet.of(accountId));
+    EasyMock.expectLastCall();
+    EasyMock.replay(accountCreator);
+
+    try (ProjectResetter resetProject =
+        builder(accountCreator, null, null).reset(project).reset(allUsers).build()) {
+      createRef(allUsersRepo, RefNames.refsUsers(accountId));
+    }
+
+    EasyMock.verify(accountCreator);
+  }
+
+  private Ref createRef(String ref) throws IOException {
+    return createRef(repo, ref);
+  }
+
+  private Ref createRef(Repository repo, String ref) throws IOException {
+    try (ObjectInserter oi = repo.newObjectInserter();
+        RevWalk rw = new RevWalk(repo)) {
+      ObjectId emptyCommit = createCommit(repo);
+      RefUpdate updateRef = repo.updateRef(ref);
+      updateRef.setExpectedOldObjectId(ObjectId.zeroId());
+      updateRef.setNewObjectId(emptyCommit);
+      assertThat(updateRef.update(rw)).isEqualTo(RefUpdate.Result.NEW);
+      return repo.exactRef(ref);
+    }
+  }
+
+  private Ref updateRef(Ref ref) throws IOException {
+    return updateRef(repo, ref);
+  }
+
+  private Ref updateRef(Repository repo, Ref ref) throws IOException {
+    try (ObjectInserter oi = repo.newObjectInserter();
+        RevWalk rw = new RevWalk(repo)) {
+      ObjectId emptyCommit = createCommit(repo);
+      RefUpdate updateRef = repo.updateRef(ref.getName());
+      updateRef.setExpectedOldObjectId(ref.getObjectId());
+      updateRef.setNewObjectId(emptyCommit);
+      updateRef.setForceUpdate(true);
+      assertThat(updateRef.update(rw)).isEqualTo(RefUpdate.Result.FORCED);
+      Ref updatedRef = repo.exactRef(ref.getName());
+      assertThat(updatedRef.getObjectId()).isNotEqualTo(ref.getObjectId());
+      return updatedRef;
+    }
+  }
+
+  private void assertRef(Ref ref) throws IOException {
+    assertRef(repo, ref);
+  }
+
+  private void assertRef(Repository repo, Ref ref) throws IOException {
+    assertThat(repo.exactRef(ref.getName()).getObjectId()).isEqualTo(ref.getObjectId());
+  }
+
+  private void assertDeletedRef(Ref ref) throws IOException {
+    assertDeletedRef(repo, ref);
+  }
+
+  private void assertDeletedRef(Repository repo, Ref ref) throws IOException {
+    assertThat(repo.exactRef(ref.getName())).isNull();
+  }
+
+  private ObjectId createCommit(Repository repo) throws IOException {
+    try (ObjectInserter oi = repo.newObjectInserter()) {
+      PersonIdent ident =
+          new PersonIdent(new PersonIdent("Foo Bar", "foo.bar@baz.com"), TimeUtil.nowTs());
+      CommitBuilder cb = new CommitBuilder();
+      cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
+      cb.setCommitter(ident);
+      cb.setAuthor(ident);
+      cb.setMessage("Test commit");
+
+      ObjectId commit = oi.insert(cb);
+      oi.flush();
+      return commit;
+    }
+  }
+
+  private ProjectResetter.Builder builder() {
+    return builder(null, null, null);
+  }
+
+  private ProjectResetter.Builder builder(
+      @Nullable AccountCreator accountCreator,
+      @Nullable AccountCache accountCache,
+      @Nullable ProjectCache projectCache) {
+    return new ProjectResetter.Builder(
+        repoManager,
+        new AllUsersName(AllUsersNameProvider.DEFAULT),
+        accountCreator,
+        accountCache,
+        projectCache);
+  }
+}

javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -48,7 +48,6 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.AccountCreator;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.common.Nullable;
@@ -839,23 +838,20 @@ public class AccountIT extends AbstractDaemonTest {
   public void putStatus() throws Exception {
     List<String> statuses = ImmutableList.of("OOO", "Busy");
     AccountInfo info;
-    try {
+    for (String status : statuses) {
-      for (String status : statuses) {
+      gApi.accounts().self().setStatus(status);
-        gApi.accounts().self().setStatus(status);
-        info = gApi.accounts().self().get();
-        assertUser(info, admin, status);
-        accountIndexedCounter.assertReindexOf(admin);
-      }
-    } finally {
-      gApi.accounts().self().setStatus(null);
       info = gApi.accounts().self().get();
-      assertUser(info, admin);
+      assertUser(info, admin, status);
       accountIndexedCounter.assertReindexOf(admin);
     }
+
+    gApi.accounts().self().setStatus(null);
+    info = gApi.accounts().self().get();
+    assertUser(info, admin);
+    accountIndexedCounter.assertReindexOf(admin);
   }
 
   @Test
-  @Sandboxed
   public void fetchUserBranch() throws Exception {
     setApiUser(user);
 
@@ -1123,7 +1119,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void pushAccountConfigToUserBranchForReviewDeactivateOtherAccount() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
 
@@ -1387,7 +1382,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void pushAccountConfigToUserBranchDeactivateOtherAccount() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
 
@@ -1421,7 +1415,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotCreateUserBranch() throws Exception {
     grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
     grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
@@ -1438,7 +1431,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void createUserBranchWithAccessDatabaseCapability() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
@@ -1454,7 +1446,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotCreateNonUserBranchUnderRefsUsersWithAccessDatabaseCapability()
       throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
@@ -1473,7 +1464,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void createDefaultUserBranch() throws Exception {
     try (Repository repo = repoManager.openRepository(allUsers)) {
       assertThat(repo.exactRef(RefNames.REFS_USERS_DEFAULT)).isNull();
@@ -1494,7 +1484,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotDeleteUserBranch() throws Exception {
     grant(
         allUsers,
@@ -1516,7 +1505,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void deleteUserBranchWithAccessDatabaseCapability() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     grant(
@@ -1732,7 +1720,6 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void checkConsistency() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     resetCurrentApiUser();

javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java

@@ -21,7 +21,6 @@ import static com.google.gerrit.acceptance.GitUtil.fetch;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
 import com.google.gerrit.extensions.client.Theme;
@@ -34,7 +33,6 @@ import org.junit.After;
 import org.junit.Test;
 
 @NoHttpd
-@Sandboxed
 public class DiffPreferencesIT extends AbstractDaemonTest {
   @After
   public void cleanUp() throws Exception {

javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java

@@ -19,7 +19,6 @@ import static com.google.gerrit.acceptance.AssertUtil.assertPrefs;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
@@ -41,7 +40,6 @@ import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
-@Sandboxed
 public class GeneralPreferencesIT extends AbstractDaemonTest {
   private TestAccount user42;
 

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -38,6 +38,8 @@ import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.ProjectResetter;
+import com.google.gerrit.acceptance.ProjectResetter.Builder;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
@@ -156,6 +158,14 @@ public class GroupsIT extends AbstractDaemonTest {
     }
   }
 
+  @Override
+  protected ProjectResetter resetProjects(Builder resetter) throws IOException {
+    // Don't reset All-Users since deleting users makes groups inconsistent (e.g. groups would
+    // contain members that no longer exist) and as result of this the group consistency checker
+    // that is executed after each test would fail.
+    return resetter.reset(allProjects, RefNames.REFS_CONFIG).build();
+  }
+
   @Test
   public void systemGroupCanBeRetrievedFromIndex() throws Exception {
     List<GroupInfo> groupInfos = gApi.groups().query("name:Administrators").get();
@@ -1052,14 +1062,12 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotCreateGroupBranch() throws Exception {
     testCannotCreateGroupBranch(
         RefNames.REFS_GROUPS + "*", RefNames.refsGroups(new AccountGroup.UUID(name("foo"))));
   }
 
   @Test
-  @Sandboxed
   public void cannotCreateDeletedGroupBranch() throws Exception {
     testCannotCreateGroupBranch(
         RefNames.REFS_DELETED_GROUPS + "*",
@@ -1067,26 +1075,29 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   @IgnoreGroupInconsistencies
   public void cannotCreateGroupNamesBranch() throws Exception {
     assume().that(groupsInNoteDb()).isTrue();
 
-    // Manually delete group names ref
+    // Use ProjectResetter to restore the group names ref
-    try (Repository repo = repoManager.openRepository(allUsers);
+    try (ProjectResetter resetter =
-        RevWalk rw = new RevWalk(repo)) {
+        projectResetter.builder().reset(allUsers, RefNames.REFS_GROUPNAMES).build()) {
-      RevCommit commit = rw.parseCommit(repo.exactRef(RefNames.REFS_GROUPNAMES).getObjectId());
+      // Manually delete group names ref
-      RefUpdate updateRef = repo.updateRef(RefNames.REFS_GROUPNAMES);
+      try (Repository repo = repoManager.openRepository(allUsers);
-      updateRef.setExpectedOldObjectId(commit.toObjectId());
+          RevWalk rw = new RevWalk(repo)) {
-      updateRef.setNewObjectId(ObjectId.zeroId());
+        RevCommit commit = rw.parseCommit(repo.exactRef(RefNames.REFS_GROUPNAMES).getObjectId());
-      updateRef.setForceUpdate(true);
+        RefUpdate updateRef = repo.updateRef(RefNames.REFS_GROUPNAMES);
-      assertThat(updateRef.delete()).isEqualTo(RefUpdate.Result.FORCED);
+        updateRef.setExpectedOldObjectId(commit.toObjectId());
+        updateRef.setNewObjectId(ObjectId.zeroId());
+        updateRef.setForceUpdate(true);
+        assertThat(updateRef.delete()).isEqualTo(RefUpdate.Result.FORCED);
+      }
+
+      // refs/meta/group-names is only visible with ACCESS_DATABASE
+      allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+
+      testCannotCreateGroupBranch(RefNames.REFS_GROUPNAMES, RefNames.REFS_GROUPNAMES);
     }
-
-    // refs/meta/group-names is only visible with ACCESS_DATABASE
-    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-
-    testCannotCreateGroupBranch(RefNames.REFS_GROUPNAMES, RefNames.REFS_GROUPNAMES);
   }
 
   private void testCannotCreateGroupBranch(String refPattern, String groupRef) throws Exception {
@@ -1104,14 +1115,12 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotDeleteGroupBranch() throws Exception {
     assume().that(groupsInNoteDb()).isTrue();
     testCannotDeleteGroupBranch(RefNames.REFS_GROUPS + "*", RefNames.refsGroups(adminGroupUuid()));
   }
 
   @Test
-  @Sandboxed
   public void cannotDeleteDeletedGroupBranch() throws Exception {
     String groupRef = RefNames.refsDeletedGroups(new AccountGroup.UUID(name("foo")));
     createBranch(allUsers, groupRef);
@@ -1119,7 +1128,6 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void cannotDeleteGroupNamesBranch() throws Exception {
     assume().that(groupsInNoteDb()).isTrue();
 

javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java

@@ -27,8 +27,8 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.ProjectResetter;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.AccessSection;
@@ -57,6 +57,7 @@ import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.testing.NoteDbMode;
 import com.google.gerrit.testing.TestChanges;
 import com.google.inject.Inject;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -309,35 +310,31 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   @Test
   public void uploadPackSubsetOfRefsVisibleWithAccessDatabase() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-    try {
+    deny("refs/heads/master", Permission.READ, REGISTERED_USERS);
-      deny("refs/heads/master", Permission.READ, REGISTERED_USERS);
+    allow("refs/heads/branch", Permission.READ, REGISTERED_USERS);
-      allow("refs/heads/branch", Permission.READ, REGISTERED_USERS);
 
-      String changeId = c1.change().getKey().get();
+    String changeId = c1.change().getKey().get();
-      setApiUser(admin);
+    setApiUser(admin);
-      gApi.changes().id(changeId).edit().create();
+    gApi.changes().id(changeId).edit().create();
-      setApiUser(user);
+    setApiUser(user);
 
-      assertUploadPackRefs(
+    assertUploadPackRefs(
-          // Change 1 is visible due to accessDatabase capability, even though
+        // Change 1 is visible due to accessDatabase capability, even though
-          // refs/heads/master is not.
+        // refs/heads/master is not.
-          r1 + "1",
+        r1 + "1",
-          r1 + "meta",
+        r1 + "meta",
-          r2 + "1",
+        r2 + "1",
-          r2 + "meta",
+        r2 + "meta",
-          r3 + "1",
+        r3 + "1",
-          r3 + "meta",
+        r3 + "meta",
-          r4 + "1",
+        r4 + "1",
-          r4 + "meta",
+        r4 + "meta",
-          "refs/heads/branch",
+        "refs/heads/branch",
-          "refs/tags/branch-tag",
+        "refs/tags/branch-tag",
-          // See comment in subsetOfBranchesVisibleNotIncludingHead.
+        // See comment in subsetOfBranchesVisibleNotIncludingHead.
-          "refs/tags/master-tag",
+        "refs/tags/master-tag",
-          // All edits are visible due to accessDatabase capability.
+        // All edits are visible due to accessDatabase capability.
-          "refs/users/00/1000000/edit-" + c1.getId() + "/1");
+        "refs/users/00/1000000/edit-" + c1.getId() + "/1");
-    } finally {
-      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-    }
   }
 
   @Test
@@ -375,12 +372,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
       assertRefs(repo, newFilter(repo, allProjects), true);
 
       allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-      try {
+      setApiUser(user);
-        setApiUser(user);
+      assertRefs(repo, newFilter(repo, allProjects), true, "refs/sequences/changes");
-        assertRefs(repo, newFilter(repo, allProjects), true, "refs/sequences/changes");
-      } finally {
-        removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-      }
     }
   }
 
@@ -485,68 +478,47 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   @Test
   public void advertisedReferencesIncludeAllUserBranchesWithAccessDatabase() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-    try {
+    TestRepository<?> userTestRepository = cloneProject(allUsers, user);
-      TestRepository<?> userTestRepository = cloneProject(allUsers, user);
+    try (Git git = userTestRepository.git()) {
-      try (Git git = userTestRepository.git()) {
+      assertThat(getUserRefs(git))
-        assertThat(getUserRefs(git))
+          .containsExactly(
-            .containsExactly(
+              RefNames.REFS_USERS_SELF, RefNames.refsUsers(user.id), RefNames.refsUsers(admin.id));
-                RefNames.REFS_USERS_SELF,
-                RefNames.refsUsers(user.id),
-                RefNames.refsUsers(admin.id));
-      }
-    } finally {
-      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     }
   }
 
   @Test
-  @Sandboxed
   @GerritConfig(name = "noteDb.groups.write", value = "true")
   public void advertisedReferencesDontShowGroupBranchToOwnerWithoutRead() throws Exception {
-    createSelfOwnedGroup("Foos", user);
+    try (ProjectResetter resetter = resetGroups()) {
-    TestRepository<?> userTestRepository = cloneProject(allUsers, user);
+      createSelfOwnedGroup("Foos", user);
-    try (Git git = userTestRepository.git()) {
+      TestRepository<?> userTestRepository = cloneProject(allUsers, user);
-      assertThat(getGroupRefs(git)).isEmpty();
+      try (Git git = userTestRepository.git()) {
+        assertThat(getGroupRefs(git)).isEmpty();
+      }
     }
   }
 
   @Test
-  @Sandboxed
   @GerritConfig(name = "noteDb.groups.write", value = "true")
   public void advertisedReferencesOmitGroupBranchesOfNonOwnedGroups() throws Exception {
-    allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
+    try (ProjectResetter resetter = resetGroups()) {
-    AccountGroup.UUID users = createGroup("Users", admins, user);
+      allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
-    AccountGroup.UUID foos = createGroup("Foos", users);
+      AccountGroup.UUID users = createGroup("Users", admins, user);
-    AccountGroup.UUID bars = createSelfOwnedGroup("Bars", user);
+      AccountGroup.UUID foos = createGroup("Foos", users);
-    TestRepository<?> userTestRepository = cloneProject(allUsers, user);
+      AccountGroup.UUID bars = createSelfOwnedGroup("Bars", user);
-    try (Git git = userTestRepository.git()) {
+      TestRepository<?> userTestRepository = cloneProject(allUsers, user);
-      assertThat(getGroupRefs(git))
+      try (Git git = userTestRepository.git()) {
-          .containsExactly(RefNames.refsGroups(foos), RefNames.refsGroups(bars));
+        assertThat(getGroupRefs(git))
+            .containsExactly(RefNames.refsGroups(foos), RefNames.refsGroups(bars));
+      }
     }
   }
 
   @Test
-  @Sandboxed
   @GerritConfig(name = "noteDb.groups.write", value = "true")
   public void advertisedReferencesIncludeAllGroupBranchesWithAccessDatabase() throws Exception {
-    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    try (ProjectResetter resetter = resetGroups()) {
-    AccountGroup.UUID users = createGroup("Users", admins);
+      allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-    TestRepository<?> userTestRepository = cloneProject(allUsers, user);
-    try (Git git = userTestRepository.git()) {
-      assertThat(getGroupRefs(git))
-          .containsExactly(
-              RefNames.refsGroups(admins),
-              RefNames.refsGroups(nonInteractiveUsers),
-              RefNames.refsGroups(users));
-    }
-  }
-
-  @Test
-  @GerritConfig(name = "noteDb.groups.write", value = "true")
-  public void advertisedReferencesIncludeAllGroupBranchesForAdmins() throws Exception {
-    allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
-    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
-    try {
       AccountGroup.UUID users = createGroup("Users", admins);
       TestRepository<?> userTestRepository = cloneProject(allUsers, user);
       try (Git git = userTestRepository.git()) {
@@ -556,8 +528,22 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
                 RefNames.refsGroups(nonInteractiveUsers),
                 RefNames.refsGroups(users));
       }
-    } finally {
+    }
-      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
+  }
+
+  @Test
+  @GerritConfig(name = "noteDb.groups.write", value = "true")
+  public void advertisedReferencesIncludeAllGroupBranchesForAdmins() throws Exception {
+    allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
+    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
+    AccountGroup.UUID users = createGroup("Users", admins);
+    TestRepository<?> userTestRepository = cloneProject(allUsers, user);
+    try (Git git = userTestRepository.git()) {
+      assertThat(getGroupRefs(git))
+          .containsExactly(
+              RefNames.refsGroups(admins),
+              RefNames.refsGroups(nonInteractiveUsers),
+              RefNames.refsGroups(users));
     }
   }
 
@@ -600,7 +586,6 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void advertisedReferencesOmitDraftCommentRefsOfOtherUsers() throws Exception {
     assume().that(notesMigration.commitChangeWrites()).isTrue();
 
@@ -623,7 +608,6 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void advertisedReferencesOmitStarredChangesRefsOfOtherUsers() throws Exception {
     assume().that(notesMigration.commitChangeWrites()).isTrue();
 
@@ -645,50 +629,46 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   @GerritConfig(name = "noteDb.groups.write", value = "true")
   public void hideMetadata() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
-    try {
+    // create change
-      // create change
+    TestRepository<?> allUsersRepo = cloneProject(allUsers);
-      TestRepository<?> allUsersRepo = cloneProject(allUsers);
+    fetch(allUsersRepo, RefNames.REFS_USERS_SELF + ":userRef");
-      fetch(allUsersRepo, RefNames.REFS_USERS_SELF + ":userRef");
+    allUsersRepo.reset("userRef");
-      allUsersRepo.reset("userRef");
+    PushOneCommit.Result mr =
-      PushOneCommit.Result mr =
+        pushFactory
-          pushFactory
+            .create(db, admin.getIdent(), allUsersRepo)
-              .create(db, admin.getIdent(), allUsersRepo)
+            .to("refs/for/" + RefNames.REFS_USERS_SELF);
-              .to("refs/for/" + RefNames.REFS_USERS_SELF);
+    mr.assertOkStatus();
-      mr.assertOkStatus();
 
-      List<String> expectedNonMetaRefs =
+    List<String> expectedNonMetaRefs =
-          ImmutableList.of(
+        ImmutableList.of(
-              RefNames.REFS_USERS_SELF,
+            RefNames.REFS_USERS_SELF,
-              RefNames.refsUsers(admin.id),
+            RefNames.refsUsers(admin.id),
-              RefNames.refsUsers(user.id),
+            RefNames.refsUsers(user.id),
-              RefNames.REFS_EXTERNAL_IDS,
+            RefNames.REFS_EXTERNAL_IDS,
-              RefNames.REFS_GROUPNAMES,
+            RefNames.REFS_GROUPNAMES,
-              RefNames.refsGroups(admins),
+            RefNames.refsGroups(admins),
-              RefNames.refsGroups(nonInteractiveUsers),
+            RefNames.refsGroups(nonInteractiveUsers),
-              RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS,
+            RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS,
-              RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS,
+            RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS,
-              RefNames.REFS_CONFIG);
+            RefNames.REFS_CONFIG);
 
-      List<String> expectedMetaRefs =
+    List<String> expectedMetaRefs =
-          new ArrayList<>(ImmutableList.of(mr.getPatchSetId().toRefName()));
+        new ArrayList<>(ImmutableList.of(mr.getPatchSetId().toRefName()));
-      if (NoteDbMode.get() != NoteDbMode.OFF) {
+    if (NoteDbMode.get() != NoteDbMode.OFF) {
-        expectedMetaRefs.add(changeRefPrefix(mr.getChange().getId()) + "meta");
+      expectedMetaRefs.add(changeRefPrefix(mr.getChange().getId()) + "meta");
-      }
+    }
 
-      List<String> expectedAllRefs = new ArrayList<>(expectedNonMetaRefs);
+    List<String> expectedAllRefs = new ArrayList<>(expectedNonMetaRefs);
-      expectedAllRefs.addAll(expectedMetaRefs);
+    expectedAllRefs.addAll(expectedMetaRefs);
 
-      try (Repository repo = repoManager.openRepository(allUsers)) {
+    try (Repository repo = repoManager.openRepository(allUsers)) {
-        Map<String, Ref> all = repo.getAllRefs();
+      Map<String, Ref> all = repo.getAllRefs();
 
-        VisibleRefFilter filter = refFilterFactory.create(projectCache.get(allUsers), repo);
+      VisibleRefFilter filter = refFilterFactory.create(projectCache.get(allUsers), repo);
-        assertThat(filter.filter(all, false).keySet()).containsExactlyElementsIn(expectedAllRefs);
+      assertThat(filter.filter(all, false).keySet()).containsExactlyElementsIn(expectedAllRefs);
 
-        assertThat(filter.setShowMetadata(false).filter(all, false).keySet())
+      assertThat(filter.setShowMetadata(false).filter(all, false).keySet())
-            .containsExactlyElementsIn(expectedNonMetaRefs);
+          .containsExactlyElementsIn(expectedNonMetaRefs);
-      }
-    } finally {
-      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     }
   }
 
@@ -788,4 +768,18 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
         Arrays.stream(members).map(m -> String.valueOf(m.id.get())).collect(toList());
     return new AccountGroup.UUID(gApi.groups().create(groupInput).get().id);
   }
+
+  /**
+   * Create a resetter to reset the group branches in All-Users. This makes the group data between
+   * ReviewDb and NoteDb inconsistent, but in the context of this test class we only care about refs
+   * and hence this is not an issue. Once groups are no longer in ReviewDb and {@link
+   * AbstractDaemonTest#resetProjects} takes care to reset group branches we no longer need this
+   * method.
+   */
+  private ProjectResetter resetGroups() throws IOException {
+    return projectResetter
+        .builder()
+        .reset(allUsers, RefNames.REFS_GROUPS + "*", RefNames.REFS_GROUPNAMES)
+        .build();
+  }
 }

javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java

@@ -85,7 +85,6 @@ import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
 import org.eclipse.jgit.util.MutableInteger;
 import org.junit.Test;
 
-@Sandboxed
 public class ExternalIdIT extends AbstractDaemonTest {
   @Inject private ExternalIdsUpdate.Server extIdsUpdate;
   @Inject private ExternalIds externalIds;
@@ -816,6 +815,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
   }
 
   @Test
+  @Sandboxed
   public void checkNoReloadAfterUpdate() throws Exception {
     Set<ExternalId> expectedExtIds = new HashSet<>(externalIds.byAccount(admin.id));
     externalIdReader.setFailOnLoad(true);
@@ -840,6 +840,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
   }
 
   @Test
+  @Sandboxed
   public void byAccountFailIfReadingExternalIdsFails() throws Exception {
     externalIdReader.setFailOnLoad(true);
 
@@ -851,6 +852,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
   }
 
   @Test
+  @Sandboxed
   public void byEmailFailIfReadingExternalIdsFails() throws Exception {
     externalIdReader.setFailOnLoad(true);
 

javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java

@@ -24,7 +24,6 @@ import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AssigneeInput;
 import com.google.gerrit.extensions.client.ReviewerState;
@@ -133,7 +132,6 @@ public class AssigneeIT extends AbstractDaemonTest {
   }
 
   @Test
-  @Sandboxed
   public void setAssigneeToInactiveUser() throws Exception {
     PushOneCommit.Result r = createChange();
     gApi.accounts().id(user.getId().get()).setActive(false);

javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java

@@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -41,7 +40,6 @@ import java.util.List;
 import org.junit.Before;
 import org.junit.Test;
 
-@Sandboxed
 public class SuggestReviewersIT extends AbstractDaemonTest {
   @Inject private CreateGroup.Factory createGroupFactory;
 

javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java

@@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -48,7 +47,6 @@ import org.eclipse.jgit.junit.TestRepository;
 import org.junit.Test;
 
 @NoHttpd
-@Sandboxed
 public class ProjectWatchIT extends AbstractDaemonTest {
   @Inject private WatchConfig.Accessor watchConfig;