TestRepository: Fix resource leak flagged by Eclipse

Since [1], included in JGit 5.3.0, TestRepository is AutoCloseable and
should be managed by try-with-resource.

[1] https://git.eclipse.org/r/#/c/134912
Change-Id: Id5fa8a47e1557ee2a5765aa13fc317f2f73ef5d1
This commit is contained in:
David Ostrovsky
2019-05-26 12:31:31 +02:00
committed by David Pursehouse
parent d74c5ac077
commit bb5205f504
20 changed files with 81 additions and 75 deletions

View File

@@ -45,8 +45,8 @@ public class GroupTestUtil {
String fileName, String fileName,
String contents) String contents)
throws Exception { throws Exception {
try (RevWalk rw = new RevWalk(allUsersRepo)) { try (RevWalk rw = new RevWalk(allUsersRepo);
TestRepository<Repository> testRepository = new TestRepository<>(allUsersRepo, rw); TestRepository<Repository> testRepository = new TestRepository<>(allUsersRepo, rw)) {
TestRepository<Repository>.CommitBuilder builder = TestRepository<Repository>.CommitBuilder builder =
testRepository testRepository
.branch(refName) .branch(refName)

View File

@@ -106,8 +106,8 @@ public class TestChanges {
// Change doesn't exist yet. NoteDb requires that there be a commit for the // Change doesn't exist yet. NoteDb requires that there be a commit for the
// first patch set, so create one. // first patch set, so create one.
GitRepositoryManager repoManager = injector.getInstance(GitRepositoryManager.class); GitRepositoryManager repoManager = injector.getInstance(GitRepositoryManager.class);
try (Repository repo = repoManager.openRepository(c.getProject())) { try (Repository repo = repoManager.openRepository(c.getProject());
TestRepository<Repository> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
PersonIdent ident = PersonIdent ident =
user.asIdentifiedUser().newCommitterIdent(update.getWhen(), TimeZone.getDefault()); user.asIdentifiedUser().newCommitterIdent(update.getWhen(), TimeZone.getDefault());
TestRepository<Repository>.CommitBuilder cb = TestRepository<Repository>.CommitBuilder cb =

View File

@@ -3952,8 +3952,8 @@ public class ChangeIT extends AbstractDaemonTest {
} }
private void modifySubmitRules(String newContent) throws Exception { private void modifySubmitRules(String newContent) throws Exception {
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> testRepo = new TestRepository<>((InMemoryRepository) repo); TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
testRepo testRepo
.branch(RefNames.REFS_CONFIG) .branch(RefNames.REFS_CONFIG)
.commit() .commit()

View File

@@ -192,9 +192,9 @@ public class DashboardIT extends AbstractDaemonTest {
throw e; throw e;
} }
} }
try (Repository r = repoManager.openRepository(project)) { try (Repository r = repoManager.openRepository(project);
TestRepository<Repository>.CommitBuilder cb = TestRepository<Repository> tr = new TestRepository<>(r)) {
new TestRepository<>(r).branch(canonicalRef).commit(); TestRepository<Repository>.CommitBuilder cb = tr.branch(canonicalRef).commit();
StringBuilder content = new StringBuilder("[dashboard]\n"); StringBuilder content = new StringBuilder("[dashboard]\n");
if (info.title != null) { if (info.title != null) {
content.append("title = ").append(info.title).append("\n"); content.append("title = ").append(info.title).append("\n");

View File

@@ -995,8 +995,8 @@ public class RevisionIT extends AbstractDaemonTest {
// Make the same change in a separate commit and update server HEAD behind Gerrit's back, which // Make the same change in a separate commit and update server HEAD behind Gerrit's back, which
// will not reindex any open changes. // will not reindex any open changes.
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
String ref = "refs/heads/master"; String ref = "refs/heads/master";
assertThat(repo.exactRef(ref).getObjectId()).isEqualTo(r1.getCommit()); assertThat(repo.exactRef(ref).getObjectId()).isEqualTo(r1.getCommit());
tr.update(ref, tr.getRevWalk().parseCommit(initial)); tr.update(ref, tr.getRevWalk().parseCommit(initial));

View File

@@ -1779,8 +1779,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
Change.Id id2 = r2.getChange().getId(); Change.Id id2 = r2.getChange().getId();
// Merge change 1 behind Gerrit's back. // Merge change 1 behind Gerrit's back.
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<?> tr = new TestRepository<>(repo)) {
tr.branch("refs/heads/master").update(r1.getCommit()); tr.branch("refs/heads/master").update(r1.getCommit());
} }
@@ -1859,12 +1859,12 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
Change c = r.getChange().change(); Change c = r.getChange().change();
RevCommit ps2Commit; RevCommit ps2Commit;
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo)) {
// Create a new patch set of the change directly in Gerrit's repository, // Create a new patch set of the change directly in Gerrit's repository,
// without pushing it. In reality it's more likely that the client would // without pushing it. In reality it's more likely that the client would
// create and push this behind Gerrit's back (e.g. an admin accidentally // create and push this behind Gerrit's back (e.g. an admin accidentally
// using direct ssh access to the repo), but that's harder to do in tests. // using direct ssh access to the repo), but that's harder to do in tests.
TestRepository<?> tr = new TestRepository<>(repo);
ps2Commit = ps2Commit =
tr.branch("refs/heads/master") tr.branch("refs/heads/master")
.commit() .commit()
@@ -1980,8 +1980,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve()); gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
gApi.changes().id(r.getChangeId()).current().submit(); gApi.changes().id(r.getChangeId()).current().submit();
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/heads/branch").commit().message("Initial commit on branch").create(); tr.branch("refs/heads/branch").commit().message("Initial commit on branch").create();
} }
@@ -2039,8 +2039,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
// expecting the change to be auto-closed, but the change metadata update // expecting the change to be auto-closed, but the change metadata update
// fails. // fails.
ObjectId c2; ObjectId c2;
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
RevCommit commit2 = RevCommit commit2 =
tr.amend(c1).message("New subject").insertChangeId(r.getChangeId().substring(1)).create(); tr.amend(c1).message("New subject").insertChangeId(r.getChangeId().substring(1)).create();
c2 = commit2.copy(); c2 = commit2.copy();

View File

@@ -71,8 +71,9 @@ public class HttpPushForReviewIT extends AbstractPushForReview {
public void uploadPackAuditEventLog() throws Exception { public void uploadPackAuditEventLog() throws Exception {
auditService.drainHttpAuditEvents(); auditService.drainHttpAuditEvents();
// testRepo is already a clone. Make a server-side change so we have something to fetch. // testRepo is already a clone. Make a server-side change so we have something to fetch.
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
new TestRepository<>(repo).branch("master").commit().create(); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("master").commit().create();
} }
testRepo.git().fetch().call(); testRepo.git().fetch().call();

View File

@@ -159,8 +159,8 @@ public class PushPermissionsIT extends AbstractDaemonTest {
@Test @Test
public void groupRefsByMessage() throws Exception { public void groupRefsByMessage() throws Exception {
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("foo").commit().create(); tr.branch("foo").commit().create();
tr.branch("bar").commit().create(); tr.branch("bar").commit().create();
} }

View File

@@ -438,8 +438,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
@Test @Test
public void receivePackOmitsMissingObject() throws Exception { public void receivePackOmitsMissingObject() throws Exception {
String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
String subject = "Subject for missing commit"; String subject = "Subject for missing commit";
Change c = new Change(cd3.change()); Change c = new Change(cd3.change());
PatchSet.Id psId = new PatchSet.Id(cd3.getId(), 2); PatchSet.Id psId = new PatchSet.Id(cd3.getId(), 2);

View File

@@ -652,8 +652,9 @@ public class SubmoduleSubscriptionsIT extends AbstractSubmoduleSubscription {
} }
private ObjectId directUpdateRef(Project.NameKey project, String ref) throws Exception { private ObjectId directUpdateRef(Project.NameKey project, String ref) throws Exception {
try (Repository serverRepo = repoManager.openRepository(project)) { try (Repository serverRepo = repoManager.openRepository(project);
return new TestRepository<>(serverRepo).branch(ref).commit().create().copy(); TestRepository<Repository> tr = new TestRepository<>(serverRepo)) {
return tr.branch(ref).commit().create().copy();
} }
} }

View File

@@ -237,9 +237,9 @@ public class ProjectsRestApiBindingsIT extends AbstractDaemonTest {
grant(project, "refs/meta/*", Permission.CREATE); grant(project, "refs/meta/*", Permission.CREATE);
gApi.projects().name(project.get()).branch(dashboardRef).create(new BranchInput()); gApi.projects().name(project.get()).branch(dashboardRef).create(new BranchInput());
try (Repository r = repoManager.openRepository(project)) { try (Repository r = repoManager.openRepository(project);
TestRepository<Repository>.CommitBuilder cb = TestRepository<Repository> tr = new TestRepository<>(r)) {
new TestRepository<>(r).branch(dashboardRef).commit(); TestRepository<Repository>.CommitBuilder cb = tr.branch(dashboardRef).commit();
StringBuilder content = new StringBuilder("[dashboard]\n"); StringBuilder content = new StringBuilder("[dashboard]\n");
content.append("title = ").append("Open Changes").append("\n"); content.append("title = ").append("Open Changes").append("\n");
content.append("[section \"").append("open").append("\"]\n"); content.append("[section \"").append("open").append("\"]\n");

View File

@@ -474,8 +474,8 @@ public class CreateProjectIT extends AbstractDaemonTest {
} }
private Optional<String> readProjectConfig(String projectName) throws Exception { private Optional<String> readProjectConfig(String projectName) throws Exception {
try (Repository repo = repoManager.openRepository(new Project.NameKey(projectName))) { try (Repository repo = repoManager.openRepository(new Project.NameKey(projectName));
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
RevWalk rw = tr.getRevWalk(); RevWalk rw = tr.getRevWalk();
Ref ref = repo.exactRef(RefNames.REFS_CONFIG); Ref ref = repo.exactRef(RefNames.REFS_CONFIG);
if (ref == null) { if (ref == null) {

View File

@@ -1508,8 +1508,9 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
if (submitType == SubmitType.FAST_FORWARD_ONLY) { if (submitType == SubmitType.FAST_FORWARD_ONLY) {
continue; continue;
} }
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
new TestRepository<>(repo).branch("master").commit().create(); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("master").commit().create();
} }
name += " after branch has advanced"; name += " after branch has advanced";
} }

View File

@@ -26,7 +26,6 @@ import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.util.Collection; import java.util.Collection;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import org.junit.Before; import org.junit.Before;
@@ -107,8 +106,8 @@ public class RulesIT extends AbstractDaemonTest {
private void modifySubmitRules(String ruleTested) throws Exception { private void modifySubmitRules(String ruleTested) throws Exception {
String newContent = String.format(RULE_TEMPLATE, ruleTested); String newContent = String.format(RULE_TEMPLATE, ruleTested);
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<?> testRepo = new TestRepository<>((InMemoryRepository) repo); TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
testRepo testRepo
.branch(RefNames.REFS_CONFIG) .branch(RefNames.REFS_CONFIG)
.commit() .commit()

View File

@@ -110,7 +110,10 @@ public class RefUpdateUtilRepoTest extends GerritBaseTests {
@Test @Test
public void deleteRef() throws Exception { public void deleteRef() throws Exception {
String ref = "refs/heads/foo"; String ref = "refs/heads/foo";
new TestRepository<>(repo).branch(ref).commit().create(); try (TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch(ref).commit().create();
}
assertThat(repo.exactRef(ref)).isNotNull(); assertThat(repo.exactRef(ref)).isNotNull();
RefUpdateUtil.deleteChecked(repo, "refs/heads/foo"); RefUpdateUtil.deleteChecked(repo, "refs/heads/foo");
assertThat(repo.exactRef(ref)).isNull(); assertThat(repo.exactRef(ref)).isNull();

View File

@@ -82,8 +82,8 @@ public class AllProjectsConfigTest {
public void noBaseConfig() throws Exception { public void noBaseConfig() throws Exception {
assertThat(getConfig().getString("foo", null, "bar")).isNull(); assertThat(getConfig().getString("foo", null, "bar")).isNull();
try (Repository repo = new FileRepository(allProjectsRepoFile)) { try (Repository repo = new FileRepository(allProjectsRepoFile);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create(); tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
} }
@@ -100,8 +100,8 @@ public class AllProjectsConfigTest {
assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base"); assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base");
try (Repository repo = new FileRepository(allProjectsRepoFile)) { try (Repository repo = new FileRepository(allProjectsRepoFile);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create(); tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
} }

View File

@@ -424,35 +424,36 @@ public class GroupNameNotesTest extends GerritBaseTests {
GroupReference g1 = newGroup("a"); GroupReference g1 = newGroup("a");
GroupReference g2 = newGroup("b"); GroupReference g2 = newGroup("b");
TestRepository<?> tr = new TestRepository<>(repo); try (TestRepository<?> tr = new TestRepository<>(repo)) {
ObjectId k1 = getNoteKey(g1); ObjectId k1 = getNoteKey(g1);
ObjectId k2 = getNoteKey(g2); ObjectId k2 = getNoteKey(g2);
ObjectId k3 = GroupNameNotes.getNoteKey(new AccountGroup.NameKey("c")); ObjectId k3 = GroupNameNotes.getNoteKey(new AccountGroup.NameKey("c"));
PersonIdent ident = newPersonIdent(); PersonIdent ident = newPersonIdent();
ObjectId origCommitId = ObjectId origCommitId =
tr.branch(REFS_GROUPNAMES) tr.branch(REFS_GROUPNAMES)
.commit() .commit()
.message("Prepopulate group name") .message("Prepopulate group name")
.author(ident) .author(ident)
.committer(ident) .committer(ident)
.add(k1.name(), "[group]\n\tuuid = a-1\n\tname = a\nanotherKey = foo\n") .add(k1.name(), "[group]\n\tuuid = a-1\n\tname = a\nanotherKey = foo\n")
.add(k2.name(), "[group]\n\tuuid = a-1\n\tname = b\n") .add(k2.name(), "[group]\n\tuuid = a-1\n\tname = b\n")
.add(k3.name(), "[group]\n\tuuid = c-3\n\tname = c\n") .add(k3.name(), "[group]\n\tuuid = c-3\n\tname = c\n")
.create() .create()
.copy(); .copy();
ident = newPersonIdent(); ident = newPersonIdent();
updateAllGroups(ident, g1, g2); updateAllGroups(ident, g1, g2);
assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2); assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
ImmutableList<CommitInfo> log = log(); ImmutableList<CommitInfo> log = log();
assertThat(log).hasSize(2); assertThat(log).hasSize(2);
assertThat(log.get(0)).commit().isEqualTo(origCommitId.name()); assertThat(log.get(0)).commit().isEqualTo(origCommitId.name());
assertThat(log.get(1)).message().isEqualTo("Store 2 group names"); assertThat(log.get(1)).message().isEqualTo("Store 2 group names");
assertThat(log.get(1)).author().matches(ident); assertThat(log.get(1)).author().matches(ident);
assertThat(log.get(1)).committer().matches(ident); assertThat(log.get(1)).committer().matches(ident);
}
// Old note content was overwritten. // Old note content was overwritten.
assertThat(readNameNote(g1)).isEqualTo("[group]\n\tuuid = a-1\n\tname = a\n"); assertThat(readNameNote(g1)).isEqualTo("[group]\n\tuuid = a-1\n\tname = a\n");

View File

@@ -175,8 +175,8 @@ public class RepoSequenceTest extends GerritBaseTests {
@Test @Test
public void failOnWrongType() throws Exception { public void failOnWrongType() throws Exception {
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project);
TestRepository<Repository> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch(RefNames.REFS_SEQUENCES + "id").commit().create(); tr.branch(RefNames.REFS_SEQUENCES + "id").commit().create();
try { try {
newSequence("id", 1, 3).next(); newSequence("id", 1, 3).next();

View File

@@ -123,9 +123,9 @@ public class NoteDbSchemaUpdaterTest extends GerritBaseTests {
@Override @Override
public void create() throws IOException { public void create() throws IOException {
try (Repository repo = repoManager.createRepository(allProjectsName)) { try (Repository repo = repoManager.createRepository(allProjectsName);
TestRepository<Repository> tr = new TestRepository<>(repo)) {
if (initialVersion.isPresent()) { if (initialVersion.isPresent()) {
TestRepository<?> tr = new TestRepository<>(repo);
tr.update(RefNames.REFS_VERSION, tr.blob(initialVersion.get().toString())); tr.update(RefNames.REFS_VERSION, tr.blob(initialVersion.get().toString()));
} }
} catch (Exception e) { } catch (Exception e) {

View File

@@ -72,8 +72,8 @@ public class ProjectConfigSchemaUpdateTest {
public void noBaseConfig() throws Exception { public void noBaseConfig() throws Exception {
assertThat(getConfig().getString("foo", null, "bar")).isNull(); assertThat(getConfig().getString("foo", null, "bar")).isNull();
try (Repository repo = new FileRepository(allProjectsRepoFile)) { try (Repository repo = new FileRepository(allProjectsRepoFile);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create(); tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
} }
@@ -90,8 +90,8 @@ public class ProjectConfigSchemaUpdateTest {
assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base"); assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base");
try (Repository repo = new FileRepository(allProjectsRepoFile)) { try (Repository repo = new FileRepository(allProjectsRepoFile);
TestRepository<?> tr = new TestRepository<>(repo); TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create(); tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
} }