opendev/gerrit
Code Issues Proposed changes

TestRepository: Fix resource leak flagged by Eclipse

Browse Source 
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 9cbb372690
commit 3b2df0674b
23 changed files with 96 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
View File

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

4
java/com/google/gerrit/testing/TestChanges.java
View File

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

4
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

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

6
javatests/com/google/gerrit/acceptance/api/project/DashboardIT.java
View File

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

4
javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

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

16
javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@@ -1816,8 +1816,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
Change.Id id2 = r2.getChange().getId();
// Merge change 1 behind Gerrit's back.
try (Repository repo = repoManager.openRepository(project)) {
TestRepository<?> tr = new TestRepository<>(repo);
try (Repository repo = repoManager.openRepository(project);
TestRepository<?> tr = new TestRepository<>(repo)) {
tr.branch("refs/heads/master").update(r1.getCommit());
}
@@ -1900,12 +1900,12 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
Change c = r.getChange().change();
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,
// 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
// using direct ssh access to the repo), but that's harder to do in tests.
TestRepository<?> tr = new TestRepository<>(repo);
ps2Commit =
tr.branch("refs/heads/master")
.commit()
@@ -2025,8 +2025,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
gApi.changes().id(r.getChangeId()).current().submit();
try (Repository repo = repoManager.openRepository(project)) {
TestRepository<?> tr = new TestRepository<>(repo);
try (Repository repo = repoManager.openRepository(project);
TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.branch("refs/heads/branch").commit().message("Initial commit on branch").create();
}
@@ -2088,8 +2088,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
// expecting the change to be auto-closed, but the change metadata update
// fails.
ObjectId c2;
try (Repository repo = repoManager.openRepository(project)) {
TestRepository<?> tr = new TestRepository<>(repo);
try (Repository repo = repoManager.openRepository(project);
TestRepository<Repository> tr = new TestRepository<>(repo)) {
RevCommit commit2 =
tr.amend(c1).message("New subject").insertChangeId(r.getChangeId().substring(1)).create();
c2 = commit2.copy();

5
javatests/com/google/gerrit/acceptance/git/HttpPushForReviewIT.java
View File

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

4
javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
View File

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

4
javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

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

5
javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
View File

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

6
javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
View File

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

4
javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
View File

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

5
javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
View File

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

5
javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
View File

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

5
javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
View File

@@ -583,8 +583,9 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest {
}
private void deleteRefsMetaConfig(Project.NameKey key) throws Exception {
try (Repository repo = repoManager.openRepository(key)) {
new TestRepository<>(repo).delete(REFS_CONFIG);
try (Repository repo = repoManager.openRepository(key);
TestRepository<Repository> tr = new TestRepository<>(repo)) {
tr.delete(REFS_CONFIG);
}
}
}

6
javatests/com/google/gerrit/git/ObjectIdsTest.java
View File

@@ -27,6 +27,7 @@ import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevBlob;
import org.junit.Test;
@@ -135,14 +136,15 @@ public class ObjectIdsTest {
private static ObjectReader newReaderWithAmbiguousIds() throws Exception {
// Recipe for creating ambiguous IDs courtesy of git core:
// https://github.com/git/git/blob/df799f5d99ac51d4fc791d546de3f936088582fc/t/t1512-rev-parse-disambiguation.sh
TestRepository<?> tr =
new TestRepository<>(new InMemoryRepository(new DfsRepositoryDescription("repo")));
try (TestRepository<Repository> tr =
new TestRepository<>(new InMemoryRepository(new DfsRepositoryDescription("repo")))) {
String blobData = "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n\nb1rwzyc3\n";
RevBlob blob = tr.blob(blobData);
assertThat(blob.name()).isEqualTo(AMBIGUOUS_BLOB_ID.name());
assertThat(tr.tree(tr.file("a0blgqsjc", blob)).name()).isEqualTo(AMBIGUOUS_TREE_ID.name());
return tr.getRevWalk().getObjectReader();
}
}
private static class MyObjectId extends ObjectId {
private static final long serialVersionUID = 1L;

5
javatests/com/google/gerrit/git/RefUpdateUtilRepoTest.java
View File

@@ -109,7 +109,10 @@ public class RefUpdateUtilRepoTest {
@Test
public void deleteRef() throws Exception {
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();
RefUpdateUtil.deleteChecked(repo, "refs/heads/foo");
assertThat(repo.exactRef(ref)).isNull();

8
javatests/com/google/gerrit/pgm/init/api/AllProjectsConfigTest.java
View File

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

3
javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
View File

@@ -441,7 +441,7 @@ public class GroupNameNotesTest {
GroupReference g1 = newGroup("a");
GroupReference g2 = newGroup("b");
TestRepository<?> tr = new TestRepository<>(repo);
try (TestRepository<Repository> tr = new TestRepository<>(repo)) {
ObjectId k1 = getNoteKey(g1);
ObjectId k2 = getNoteKey(g2);
ObjectId k3 = GroupNameNotes.getNoteKey(AccountGroup.nameKey("c"));
@@ -470,6 +470,7 @@ public class GroupNameNotesTest {
assertThat(log.get(1)).message().isEqualTo("Store 2 group names");
assertThat(log.get(1)).author().matches(ident);
assertThat(log.get(1)).committer().matches(ident);
}
// Old note content was overwritten.
assertThat(readNameNote(g1)).isEqualTo("[group]\n\tuuid = a-1\n\tname = a\n");

4
javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java
View File

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

5
javatests/com/google/gerrit/server/permissions/RefControlTest.java
View File

@@ -199,8 +199,9 @@ public class RefControlTest {
// Clear out All-Projects and use the lowest-level API possible for project creation, so the
// only ACL entries are exactly what is initialized by this test, and we aren't subject to
// changing defaults in SchemaCreator or ProjectCreator.
try (Repository allProjectsRepo = repoManager.createRepository(allProjectsName)) {
new TestRepository<>(allProjectsRepo).delete(REFS_CONFIG);
try (Repository allProjectsRepo = repoManager.createRepository(allProjectsName);
TestRepository<Repository> tr = new TestRepository<>(allProjectsRepo)) {
tr.delete(REFS_CONFIG);
try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjectsName)) {
ProjectConfig allProjectsConfig = projectConfigFactory.create(allProjectsName);
allProjectsConfig.load(md);

4
javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java
View File

@@ -116,9 +116,9 @@ public class NoteDbSchemaUpdaterTest {
@Override
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()) {
TestRepository<?> tr = new TestRepository<>(repo);
tr.update(RefNames.REFS_VERSION, tr.blob(initialVersion.get().toString()));
}
} catch (Exception e) {

8
javatests/com/google/gerrit/server/schema/ProjectConfigSchemaUpdateTest.java
View File

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