diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 790d3c58d4..c7958ccd9e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -222,53 +222,37 @@ public class SubmitOnPushIT extends AbstractDaemonTest { } private void assertCommit(Project.NameKey project, String branch) throws IOException { - Repository r = repoManager.openRepository(project); - try { - RevWalk rw = new RevWalk(r); - try { - RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId()); - assertThat(c.getShortMessage()).isEqualTo(PushOneCommit.SUBJECT); - assertThat(c.getAuthorIdent().getEmailAddress()).isEqualTo(admin.email); - assertThat(c.getCommitterIdent().getEmailAddress()).isEqualTo( - admin.email); - } finally { - rw.release(); - } - } finally { - r.close(); + try (Repository r = repoManager.openRepository(project); + RevWalk rw = new RevWalk(r)) { + RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId()); + assertThat(c.getShortMessage()).isEqualTo(PushOneCommit.SUBJECT); + assertThat(c.getAuthorIdent().getEmailAddress()).isEqualTo(admin.email); + assertThat(c.getCommitterIdent().getEmailAddress()).isEqualTo( + admin.email); } } private void assertMergeCommit(String branch, String subject) throws IOException { - Repository r = repoManager.openRepository(project); - try { - RevWalk rw = new RevWalk(r); - try { - RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId()); - assertThat(c.getParentCount()).is(2); - assertThat(c.getShortMessage()).isEqualTo("Merge \"" + subject + "\""); - assertThat(c.getAuthorIdent().getEmailAddress()).isEqualTo(admin.email); - assertThat(c.getCommitterIdent().getEmailAddress()).isEqualTo( - serverIdent.getEmailAddress()); - } finally { - rw.release(); - } - } finally { - r.close(); + try (Repository r = repoManager.openRepository(project); + RevWalk rw = new RevWalk(r)) { + RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId()); + assertThat(c.getParentCount()).is(2); + assertThat(c.getShortMessage()).isEqualTo("Merge \"" + subject + "\""); + assertThat(c.getAuthorIdent().getEmailAddress()).isEqualTo(admin.email); + assertThat(c.getCommitterIdent().getEmailAddress()).isEqualTo( + serverIdent.getEmailAddress()); } } private void assertTag(Project.NameKey project, String branch, PushOneCommit.Tag tag) throws IOException { - Repository repo = repoManager.openRepository(project); - try { + try (Repository repo = repoManager.openRepository(project)) { Ref tagRef = repo.getRef(tag.name); assertThat(tagRef).isNotNull(); ObjectId taggedCommit = null; if (tag instanceof PushOneCommit.AnnotatedTag) { PushOneCommit.AnnotatedTag annotatedTag = (PushOneCommit.AnnotatedTag)tag; - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { RevObject object = rw.parseAny(tagRef.getObjectId()); assertThat(object).isInstanceOf(RevTag.class); RevTag tagObject = (RevTag) object; @@ -276,8 +260,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest { .isEqualTo(annotatedTag.message); assertThat(tagObject.getTaggerIdent()).isEqualTo(annotatedTag.tagger); taggedCommit = tagObject.getObject(); - } finally { - rw.dispose(); } } else { taggedCommit = tagRef.getObjectId(); @@ -285,8 +267,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest { ObjectId headCommit = repo.getRef(branch).getObjectId(); assertThat(taggedCommit).isNotNull(); assertThat(taggedCommit).isEqualTo(headCommit); - } finally { - repo.close(); } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index ec78be5503..216823adb7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -359,40 +359,23 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { } protected RevCommit getRemoteHead() throws IOException { - Repository repo = repoManager.openRepository(project); - try { + try (Repository repo = repoManager.openRepository(project)) { return getHead(repo, "refs/heads/master"); - } finally { - repo.close(); } } protected List getRemoteLog() throws IOException { - Repository repo = repoManager.openRepository(project); - try { - RevWalk rw = new RevWalk(repo); - try { - rw.markStart(rw.parseCommit( - repo.getRef("refs/heads/master").getObjectId())); - return Lists.newArrayList(rw); - } finally { - rw.release(); - } - } finally { - repo.close(); + try (Repository repo = repoManager.openRepository(project); + RevWalk rw = new RevWalk(repo)) { + rw.markStart(rw.parseCommit( + repo.getRef("refs/heads/master").getObjectId())); + return Lists.newArrayList(rw); } } private RevCommit getHead(Repository repo, String name) throws IOException { - try { - RevWalk rw = new RevWalk(repo); - try { - return rw.parseCommit(repo.getRef(name).getObjectId()); - } finally { - rw.release(); - } - } finally { - repo.close(); + try (RevWalk rw = new RevWalk(repo)) { + return rw.parseCommit(repo.getRef(name).getObjectId()); } } @@ -403,28 +386,22 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { } private String getLatestRemoteDiff() throws IOException { - Repository repo = repoManager.openRepository(project); - try { - RevWalk rw = new RevWalk(repo); - try { - ObjectId oldTreeId = repo.resolve("refs/heads/master~1^{tree}"); - ObjectId newTreeId = repo.resolve("refs/heads/master^{tree}"); - return getLatestDiff(repo, oldTreeId, newTreeId); - } finally { - rw.release(); - } - } finally { - repo.close(); + try (Repository repo = repoManager.openRepository(project); + RevWalk rw = new RevWalk(repo)) { + ObjectId oldTreeId = repo.resolve("refs/heads/master~1^{tree}"); + ObjectId newTreeId = repo.resolve("refs/heads/master^{tree}"); + return getLatestDiff(repo, oldTreeId, newTreeId); } } private String getLatestDiff(Repository repo, ObjectId oldTreeId, ObjectId newTreeId) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); - DiffFormatter fmt = new DiffFormatter(out); - fmt.setRepository(repo); - fmt.format(oldTreeId, newTreeId); - fmt.flush(); - return out.toString(); + try (DiffFormatter fmt = new DiffFormatter(out)) { + fmt.setRepository(repo); + fmt.format(oldTreeId, newTreeId); + fmt.flush(); + return out.toString(); + } } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java index 998c5ea450..68c9a5e894 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java @@ -238,11 +238,10 @@ public class CreateProjectIT extends AbstractDaemonTest { private void assertEmptyCommit(String projectName, String... refs) throws RepositoryNotFoundException, IOException { - Repository repo = - repoManager.openRepository(new Project.NameKey(projectName)); - RevWalk rw = new RevWalk(repo); - TreeWalk tw = new TreeWalk(repo); - try { + Project.NameKey projectKey = new Project.NameKey(projectName); + try (Repository repo = repoManager.openRepository(projectKey); + RevWalk rw = new RevWalk(repo); + TreeWalk tw = new TreeWalk(rw.getObjectReader())) { for (String ref : refs) { RevCommit commit = rw.lookupCommit(repo.getRef(ref).getObjectId()); rw.parseBody(commit); @@ -250,9 +249,6 @@ public class CreateProjectIT extends AbstractDaemonTest { assertThat(tw.next()).isFalse(); tw.reset(); } - } finally { - rw.release(); - repo.close(); } } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java index dddcd67353..72fc008d75 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java @@ -202,32 +202,29 @@ public class CatServlet extends HttpServlet { final RevCommit fromCommit; final String suffix; final String path = patchKey.getFileName(); - try { - final ObjectReader reader = repo.newObjectReader(); - try { - final RevWalk rw = new RevWalk(reader); - final RevCommit c; - final TreeWalk tw; + try (ObjectReader reader = repo.newObjectReader(); + RevWalk rw = new RevWalk(reader)) { + final RevCommit c; - c = rw.parseCommit(ObjectId.fromString(revision)); - if (side == 0) { - fromCommit = c; - suffix = "new"; - - } else if (1 <= side && side - 1 < c.getParentCount()) { - fromCommit = rw.parseCommit(c.getParent(side - 1)); - if (c.getParentCount() == 1) { - suffix = "old"; - } else { - suffix = "old" + side; - } + c = rw.parseCommit(ObjectId.fromString(revision)); + if (side == 0) { + fromCommit = c; + suffix = "new"; + } else if (1 <= side && side - 1 < c.getParentCount()) { + fromCommit = rw.parseCommit(c.getParent(side - 1)); + if (c.getParentCount() == 1) { + suffix = "old"; } else { - rsp.sendError(HttpServletResponse.SC_NOT_FOUND); - return; + suffix = "old" + side; } - tw = TreeWalk.forPath(reader, path, fromCommit.getTree()); + } else { + rsp.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + + try (TreeWalk tw = TreeWalk.forPath(reader, path, fromCommit.getTree())) { if (tw == null) { rsp.sendError(HttpServletResponse.SC_NOT_FOUND); return; @@ -240,8 +237,6 @@ public class CatServlet extends HttpServlet { rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return; } - } finally { - reader.release(); } } catch (IOException e) { getServletContext().log("Cannot read repository", e); diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java index 88115b6172..ab20f531f5 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java @@ -185,11 +185,8 @@ public class RebuildNotedb extends SiteProgram { private static void execute(BatchRefUpdate bru, Repository repo) throws IOException { - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { bru.execute(rw, NullProgressMonitor.INSTANCE); - } finally { - rw.release(); } } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java index 054a97b917..10d93eeaa0 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java @@ -137,49 +137,41 @@ public class AllProjectsConfig extends VersionedMetaData { throw new IOException("All-Projects does not exist."); } - Repository repo = new FileRepository(path); - try { + try (Repository repo = new FileRepository(path)) { inserter = repo.newObjectInserter(); reader = repo.newObjectReader(); - try { - RevWalk rw = new RevWalk(reader); - try { - RevTree srcTree = revision != null ? rw.parseTree(revision) : null; - newTree = readTree(srcTree); - saveConfig(ProjectConfig.PROJECT_CONFIG, cfg); - saveGroupList(); - ObjectId res = newTree.writeTree(inserter); - if (res.equals(srcTree)) { - // If there are no changes to the content, don't create the commit. - return; - } - - CommitBuilder commit = new CommitBuilder(); - commit.setAuthor(ident); - commit.setCommitter(ident); - commit.setMessage(msg); - commit.setTreeId(res); - if (revision != null) { - commit.addParentId(revision); - } - ObjectId newRevision = inserter.insert(commit); - updateRef(repo, ident, newRevision, "commit: " + msg); - revision = newRevision; - } finally { - rw.release(); + try (RevWalk rw = new RevWalk(reader)) { + RevTree srcTree = revision != null ? rw.parseTree(revision) : null; + newTree = readTree(srcTree); + saveConfig(ProjectConfig.PROJECT_CONFIG, cfg); + saveGroupList(); + ObjectId res = newTree.writeTree(inserter); + if (res.equals(srcTree)) { + // If there are no changes to the content, don't create the commit. + return; } + + CommitBuilder commit = new CommitBuilder(); + commit.setAuthor(ident); + commit.setCommitter(ident); + commit.setMessage(msg); + commit.setTreeId(res); + if (revision != null) { + commit.addParentId(revision); + } + ObjectId newRevision = inserter.insert(commit); + updateRef(repo, ident, newRevision, "commit: " + msg); + revision = newRevision; } finally { if (inserter != null) { - inserter.release(); + inserter.close(); inserter = null; } if (reader != null) { - reader.release(); + reader.close(); reader = null; } } - } finally { - repo.close(); } // we need to invalidate the JGit cache if the group list is invalidated in diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 93bbd7b4e7..9faa516995 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetAncestor; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.ChangeInserter; @@ -227,119 +228,107 @@ public class ChangeUtil { } Change changeToRevert = db.get().changes().get(changeId); - Repository git; - try { - git = gitManager.openRepository(ctl.getChange().getProject()); + Project.NameKey project = ctl.getChange().getProject(); + try (Repository git = gitManager.openRepository(project); + RevWalk revWalk = new RevWalk(git)) { + RevCommit commitToRevert = + revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); + + PersonIdent authorIdent = + user().newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone()); + + RevCommit parentToCommitToRevert = commitToRevert.getParent(0); + revWalk.parseHeaders(parentToCommitToRevert); + + CommitBuilder revertCommitBuilder = new CommitBuilder(); + revertCommitBuilder.addParentId(commitToRevert); + revertCommitBuilder.setTreeId(parentToCommitToRevert.getTree()); + revertCommitBuilder.setAuthor(authorIdent); + revertCommitBuilder.setCommitter(authorIdent); + + if (message == null) { + message = MessageFormat.format( + ChangeMessages.get().revertChangeDefaultMessage, + changeToRevert.getSubject(), patch.getRevision().get()); + } + + ObjectId computedChangeId = + ChangeIdUtil.computeChangeId(parentToCommitToRevert.getTree(), + commitToRevert, authorIdent, myIdent, message); + revertCommitBuilder.setMessage( + ChangeIdUtil.insertId(message, computedChangeId, true)); + + RevCommit revertCommit; + try (ObjectInserter oi = git.newObjectInserter()) { + ObjectId id = oi.insert(revertCommitBuilder); + oi.flush(); + revertCommit = revWalk.parseCommit(id); + } + + RefControl refControl = ctl.getRefControl(); + Change change = new Change( + new Change.Key("I" + computedChangeId.name()), + new Change.Id(db.get().nextChangeId()), + user().getAccountId(), + changeToRevert.getDest(), + TimeUtil.nowTs()); + change.setTopic(changeToRevert.getTopic()); + ChangeInserter ins = + changeInserterFactory.create(refControl.getProjectControl(), + change, revertCommit); + PatchSet ps = ins.getPatchSet(); + + String ref = refControl.getRefName(); + String cmdRef = MagicBranch.NEW_PUBLISH_CHANGE + + ref.substring(ref.lastIndexOf('/') + 1); + CommitReceivedEvent commitReceivedEvent = new CommitReceivedEvent( + new ReceiveCommand(ObjectId.zeroId(), revertCommit.getId(), cmdRef), + refControl.getProjectControl().getProject(), + refControl.getRefName(), revertCommit, user()); + + try { + commitValidatorsFactory.create(refControl, sshInfo, git) + .validateForGerritCommits(commitReceivedEvent); + } catch (CommitValidationException e) { + throw new InvalidChangeOperationException(e.getMessage()); + } + + RefUpdate ru = git.updateRef(ps.getRefName()); + ru.setExpectedOldObjectId(ObjectId.zeroId()); + ru.setNewObjectId(revertCommit); + ru.disableRefLog(); + if (ru.update(revWalk) != RefUpdate.Result.NEW) { + throw new IOException(String.format( + "Failed to create ref %s in %s: %s", ps.getRefName(), + change.getDest().getParentKey().get(), ru.getResult())); + } + + ChangeMessage cmsg = new ChangeMessage( + new ChangeMessage.Key(changeId, messageUUID(db.get())), + user().getAccountId(), TimeUtil.nowTs(), patchSetId); + StringBuilder msgBuf = new StringBuilder(); + msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted"); + msgBuf.append("\n\n"); + msgBuf.append("This patchset was reverted in change: ") + .append(change.getKey().get()); + cmsg.setMessage(msgBuf.toString()); + + ins.setMessage(cmsg).insert(); + + try { + RevertedSender cm = revertedSenderFactory.create(change); + cm.setFrom(user().getAccountId()); + cm.setChangeMessage(cmsg); + cm.send(); + } catch (Exception err) { + log.error("Cannot send email for revert change " + change.getId(), + err); + } + + return change.getId(); } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(changeId, e); } - try { - RevWalk revWalk = new RevWalk(git); - try { - RevCommit commitToRevert = - revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); - - PersonIdent authorIdent = - user().newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone()); - - RevCommit parentToCommitToRevert = commitToRevert.getParent(0); - revWalk.parseHeaders(parentToCommitToRevert); - - CommitBuilder revertCommitBuilder = new CommitBuilder(); - revertCommitBuilder.addParentId(commitToRevert); - revertCommitBuilder.setTreeId(parentToCommitToRevert.getTree()); - revertCommitBuilder.setAuthor(authorIdent); - revertCommitBuilder.setCommitter(authorIdent); - - if (message == null) { - message = MessageFormat.format( - ChangeMessages.get().revertChangeDefaultMessage, - changeToRevert.getSubject(), patch.getRevision().get()); - } - - ObjectId computedChangeId = - ChangeIdUtil.computeChangeId(parentToCommitToRevert.getTree(), - commitToRevert, authorIdent, myIdent, message); - revertCommitBuilder.setMessage( - ChangeIdUtil.insertId(message, computedChangeId, true)); - - RevCommit revertCommit; - ObjectInserter oi = git.newObjectInserter(); - try { - ObjectId id = oi.insert(revertCommitBuilder); - oi.flush(); - revertCommit = revWalk.parseCommit(id); - } finally { - oi.release(); - } - - RefControl refControl = ctl.getRefControl(); - Change change = new Change( - new Change.Key("I" + computedChangeId.name()), - new Change.Id(db.get().nextChangeId()), - user().getAccountId(), - changeToRevert.getDest(), - TimeUtil.nowTs()); - change.setTopic(changeToRevert.getTopic()); - ChangeInserter ins = - changeInserterFactory.create(refControl.getProjectControl(), - change, revertCommit); - PatchSet ps = ins.getPatchSet(); - - String ref = refControl.getRefName(); - String cmdRef = MagicBranch.NEW_PUBLISH_CHANGE - + ref.substring(ref.lastIndexOf('/') + 1); - CommitReceivedEvent commitReceivedEvent = new CommitReceivedEvent( - new ReceiveCommand(ObjectId.zeroId(), revertCommit.getId(), cmdRef), - refControl.getProjectControl().getProject(), - refControl.getRefName(), revertCommit, user()); - - try { - commitValidatorsFactory.create(refControl, sshInfo, git) - .validateForGerritCommits(commitReceivedEvent); - } catch (CommitValidationException e) { - throw new InvalidChangeOperationException(e.getMessage()); - } - - RefUpdate ru = git.updateRef(ps.getRefName()); - ru.setExpectedOldObjectId(ObjectId.zeroId()); - ru.setNewObjectId(revertCommit); - ru.disableRefLog(); - if (ru.update(revWalk) != RefUpdate.Result.NEW) { - throw new IOException(String.format( - "Failed to create ref %s in %s: %s", ps.getRefName(), - change.getDest().getParentKey().get(), ru.getResult())); - } - - ChangeMessage cmsg = new ChangeMessage( - new ChangeMessage.Key(changeId, messageUUID(db.get())), - user().getAccountId(), TimeUtil.nowTs(), patchSetId); - StringBuilder msgBuf = new StringBuilder(); - msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted"); - msgBuf.append("\n\n"); - msgBuf.append("This patchset was reverted in change: ") - .append(change.getKey().get()); - cmsg.setMessage(msgBuf.toString()); - - ins.setMessage(cmsg).insert(); - - try { - RevertedSender cm = revertedSenderFactory.create(change); - cm.setFrom(user().getAccountId()); - cm.setChangeMessage(cmsg); - cm.send(); - } catch (Exception err) { - log.error("Cannot send email for revert change " + change.getId(), - err); - } - - return change.getId(); - } finally { - revWalk.release(); - } - } finally { - git.close(); - } } public Change.Id editCommitMessage(ChangeControl ctl, PatchSet ps, @@ -354,68 +343,56 @@ public class ChangeUtil { "The commit message cannot be empty"); } - Repository git; - try { - git = gitManager.openRepository(ctl.getChange().getProject()); + Project.NameKey project = ctl.getChange().getProject(); + try (Repository git = gitManager.openRepository(project); + RevWalk revWalk = new RevWalk(git)) { + RevCommit commit = + revWalk.parseCommit(ObjectId.fromString(ps.getRevision() + .get())); + if (commit.getFullMessage().equals(message)) { + throw new InvalidChangeOperationException( + "New commit message cannot be same as existing commit message"); + } + + Date now = myIdent.getWhen(); + PersonIdent authorIdent = + user().newCommitterIdent(now, myIdent.getTimeZone()); + + CommitBuilder commitBuilder = new CommitBuilder(); + commitBuilder.setTreeId(commit.getTree()); + commitBuilder.setParentIds(commit.getParents()); + commitBuilder.setAuthor(commit.getAuthorIdent()); + commitBuilder.setCommitter(authorIdent); + commitBuilder.setMessage(message); + + RevCommit newCommit; + try (ObjectInserter oi = git.newObjectInserter()) { + ObjectId id = oi.insert(commitBuilder); + oi.flush(); + newCommit = revWalk.parseCommit(id); + } + + PatchSet.Id id = nextPatchSetId(git, change.currentPatchSetId()); + PatchSet newPatchSet = new PatchSet(id); + newPatchSet.setCreatedOn(new Timestamp(now.getTime())); + newPatchSet.setUploader(user().getAccountId()); + newPatchSet.setRevision(new RevId(newCommit.name())); + + String msg = "Patch Set " + newPatchSet.getPatchSetId() + + ": Commit message was updated"; + + change = patchSetInserterFactory + .create(git, revWalk, ctl, newCommit) + .setPatchSet(newPatchSet) + .setMessage(msg) + .setValidatePolicy(RECEIVE_COMMITS) + .setDraft(ps.isDraft()) + .insert(); + + return change.getId(); } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(changeId, e); } - try { - RevWalk revWalk = new RevWalk(git); - try { - RevCommit commit = - revWalk.parseCommit(ObjectId.fromString(ps.getRevision() - .get())); - if (commit.getFullMessage().equals(message)) { - throw new InvalidChangeOperationException( - "New commit message cannot be same as existing commit message"); - } - - Date now = myIdent.getWhen(); - PersonIdent authorIdent = - user().newCommitterIdent(now, myIdent.getTimeZone()); - - CommitBuilder commitBuilder = new CommitBuilder(); - commitBuilder.setTreeId(commit.getTree()); - commitBuilder.setParentIds(commit.getParents()); - commitBuilder.setAuthor(commit.getAuthorIdent()); - commitBuilder.setCommitter(authorIdent); - commitBuilder.setMessage(message); - - RevCommit newCommit; - ObjectInserter oi = git.newObjectInserter(); - try { - ObjectId id = oi.insert(commitBuilder); - oi.flush(); - newCommit = revWalk.parseCommit(id); - } finally { - oi.release(); - } - - PatchSet.Id id = nextPatchSetId(git, change.currentPatchSetId()); - PatchSet newPatchSet = new PatchSet(id); - newPatchSet.setCreatedOn(new Timestamp(now.getTime())); - newPatchSet.setUploader(user().getAccountId()); - newPatchSet.setRevision(new RevId(newCommit.name())); - - String msg = "Patch Set " + newPatchSet.getPatchSetId() - + ": Commit message was updated"; - - change = patchSetInserterFactory - .create(git, revWalk, ctl, newCommit) - .setPatchSet(newPatchSet) - .setMessage(msg) - .setValidatePolicy(RECEIVE_COMMITS) - .setDraft(ps.isDraft()) - .insert(); - - return change.getId(); - } finally { - revWalk.release(); - } - } finally { - git.close(); - } } public String getMessage(Change change) @@ -427,25 +404,14 @@ public class ChangeUtil { throw new NoSuchChangeException(changeId); } - Repository git; - try { - git = gitManager.openRepository(change.getProject()); + try (Repository git = gitManager.openRepository(change.getProject()); + RevWalk revWalk = new RevWalk(git)) { + RevCommit commit = revWalk.parseCommit( + ObjectId.fromString(ps.getRevision().get())); + return commit.getFullMessage(); } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(changeId, e); } - try { - RevWalk revWalk = new RevWalk(git); - try { - RevCommit commit = - revWalk.parseCommit(ObjectId.fromString(ps.getRevision() - .get())); - return commit.getFullMessage(); - } finally { - revWalk.release(); - } - } finally { - git.close(); - } } public void deleteDraftChange(Change change) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java index 76cf49cb72..23039aa486 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java @@ -189,8 +189,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { return ChangeKind.NO_CODE_CHANGE; } - RevWalk walk = new RevWalk(key.repo); - try { + try (RevWalk walk = new RevWalk(key.repo)) { RevCommit prior = walk.parseCommit(key.prior); walk.parseBody(prior); RevCommit next = walk.parseCommit(key.next); @@ -227,7 +226,6 @@ public class ChangeKindCacheImpl implements ChangeKindCache { } } finally { key.repo = null; - walk.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index 48944fa13d..b386894ece 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -121,96 +121,82 @@ public class CherryPickChange { Project.NameKey project = change.getProject(); IdentifiedUser identifiedUser = (IdentifiedUser) currentUser.get(); - final Repository git; - try { - git = gitManager.openRepository(project); + try (Repository git = gitManager.openRepository(project); + RevWalk revWalk = new RevWalk(git)) { + Ref destRef = git.getRef(destinationBranch); + if (destRef == null) { + throw new InvalidChangeOperationException("Branch " + + destinationBranch + " does not exist."); + } + + final RevCommit mergeTip = revWalk.parseCommit(destRef.getObjectId()); + + RevCommit commitToCherryPick = + revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); + + PersonIdent committerIdent = + identifiedUser.newCommitterIdent(TimeUtil.nowTs(), + serverTimeZone); + + final ObjectId computedChangeId = + ChangeIdUtil + .computeChangeId(commitToCherryPick.getTree(), mergeTip, + commitToCherryPick.getAuthorIdent(), committerIdent, message); + String commitMessage = + ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n'; + + RevCommit cherryPickCommit; + try (ObjectInserter oi = git.newObjectInserter()) { + ProjectState projectState = refControl.getProjectControl().getProjectState(); + cherryPickCommit = + mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, + commitToCherryPick, committerIdent, commitMessage, revWalk); + } catch (MergeIdenticalTreeException | MergeConflictException e) { + throw new MergeException("Cherry pick failed: " + e.getMessage()); + } + + Change.Key changeKey; + final List idList = cherryPickCommit.getFooterLines( + FooterConstants.CHANGE_ID); + if (!idList.isEmpty()) { + final String idStr = idList.get(idList.size() - 1).trim(); + changeKey = new Change.Key(idStr); + } else { + changeKey = new Change.Key("I" + computedChangeId.name()); + } + + Branch.NameKey newDest = + new Branch.NameKey(change.getProject(), destRef.getName()); + List destChanges = queryProvider.get() + .setLimit(2) + .byBranchKey(newDest, changeKey); + if (destChanges.size() > 1) { + throw new InvalidChangeOperationException("Several changes with key " + + changeKey + " reside on the same branch. " + + "Cannot create a new patch set."); + } else if (destChanges.size() == 1) { + // The change key exists on the destination branch. The cherry pick + // will be added as a new patch set. + return insertPatchSet(git, revWalk, destChanges.get(0).change(), + cherryPickCommit, refControl, identifiedUser); + } else { + // Change key not found on destination branch. We can create a new + // change. + Change newChange = createNewChange(git, revWalk, changeKey, project, + destRef, cherryPickCommit, refControl, + identifiedUser, change.getTopic()); + + addMessageToSourceChange(change, patch.getId(), destinationBranch, + cherryPickCommit, identifiedUser, refControl); + + addMessageToDestinationChange(newChange, change.getDest().getShortName(), + identifiedUser, refControl); + + return newChange.getId(); + } } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(change.getId(), e); } - - try { - RevWalk revWalk = new RevWalk(git); - try { - Ref destRef = git.getRef(destinationBranch); - if (destRef == null) { - throw new InvalidChangeOperationException("Branch " - + destinationBranch + " does not exist."); - } - - final RevCommit mergeTip = revWalk.parseCommit(destRef.getObjectId()); - - RevCommit commitToCherryPick = - revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get())); - - PersonIdent committerIdent = - identifiedUser.newCommitterIdent(TimeUtil.nowTs(), - serverTimeZone); - - final ObjectId computedChangeId = - ChangeIdUtil - .computeChangeId(commitToCherryPick.getTree(), mergeTip, - commitToCherryPick.getAuthorIdent(), committerIdent, message); - String commitMessage = - ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n'; - - RevCommit cherryPickCommit; - ObjectInserter oi = git.newObjectInserter(); - try { - ProjectState projectState = refControl.getProjectControl().getProjectState(); - cherryPickCommit = - mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, - commitToCherryPick, committerIdent, commitMessage, revWalk); - } catch (MergeIdenticalTreeException | MergeConflictException e) { - throw new MergeException("Cherry pick failed: " + e.getMessage()); - } finally { - oi.release(); - } - - Change.Key changeKey; - final List idList = cherryPickCommit.getFooterLines( - FooterConstants.CHANGE_ID); - if (!idList.isEmpty()) { - final String idStr = idList.get(idList.size() - 1).trim(); - changeKey = new Change.Key(idStr); - } else { - changeKey = new Change.Key("I" + computedChangeId.name()); - } - - Branch.NameKey newDest = - new Branch.NameKey(change.getProject(), destRef.getName()); - List destChanges = queryProvider.get() - .setLimit(2) - .byBranchKey(newDest, changeKey); - if (destChanges.size() > 1) { - throw new InvalidChangeOperationException("Several changes with key " - + changeKey + " reside on the same branch. " - + "Cannot create a new patch set."); - } else if (destChanges.size() == 1) { - // The change key exists on the destination branch. The cherry pick - // will be added as a new patch set. - return insertPatchSet(git, revWalk, destChanges.get(0).change(), - cherryPickCommit, refControl, identifiedUser); - } else { - // Change key not found on destination branch. We can create a new - // change. - Change newChange = createNewChange(git, revWalk, changeKey, project, - destRef, cherryPickCommit, refControl, - identifiedUser, change.getTopic()); - - addMessageToSourceChange(change, patch.getId(), destinationBranch, - cherryPickCommit, identifiedUser, refControl); - - addMessageToDestinationChange(newChange, change.getDest().getShortName(), - identifiedUser, refControl); - - return newChange.getId(); - } - } finally { - revWalk.release(); - } - } finally { - git.close(); - } } private Change.Id insertPatchSet(Repository git, RevWalk revWalk, Change change, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java index ad1e16046b..ee28ed204c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -153,7 +153,7 @@ public class ConsistencyChecker { return Result.create(c, problems); } finally { if (rw != null) { - rw.release(); + rw.close(); } if (repo != null) { repo.close(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java index 18552028fd..4dffd67619 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java @@ -158,71 +158,63 @@ public class CreateChange implements } Project.NameKey project = rsrc.getNameKey(); - Repository git = gitManager.openRepository(project); - - try { - RevWalk rw = new RevWalk(git); - try { - ObjectId parentCommit; - if (input.baseChange != null) { - List changes = changeUtil.findChanges(input.baseChange); - if (changes.size() != 1) { - throw new InvalidChangeOperationException( - "Base change not found: " + input.baseChange); - } - Change change = Iterables.getOnlyElement(changes); - if (!rsrc.getControl().controlFor(change).isVisible(db.get())) { - throw new InvalidChangeOperationException( - "Base change not found: " + input.baseChange); - } - PatchSet ps = db.get().patchSets().get( - new PatchSet.Id(change.getId(), - change.currentPatchSetId().get())); - parentCommit = ObjectId.fromString(ps.getRevision().get()); - } else { - Ref destRef = git.getRef(refName); - if (destRef == null) { - throw new UnprocessableEntityException(String.format( - "Branch %s does not exist.", refName)); - } - parentCommit = destRef.getObjectId(); + try (Repository git = gitManager.openRepository(project); + RevWalk rw = new RevWalk(git)) { + ObjectId parentCommit; + if (input.baseChange != null) { + List changes = changeUtil.findChanges(input.baseChange); + if (changes.size() != 1) { + throw new InvalidChangeOperationException( + "Base change not found: " + input.baseChange); } - RevCommit mergeTip = rw.parseCommit(parentCommit); - - Timestamp now = TimeUtil.nowTs(); - IdentifiedUser me = (IdentifiedUser) userProvider.get(); - PersonIdent author = me.newCommitterIdent(now, serverTimeZone); - - ObjectId id = ChangeIdUtil.computeChangeId(mergeTip.getTree(), - mergeTip, author, author, input.subject); - String commitMessage = ChangeIdUtil.insertId(input.subject, id); - - RevCommit c = newCommit(git, rw, author, mergeTip, commitMessage); - - Change change = new Change( - getChangeId(id, c), - new Change.Id(db.get().nextChangeId()), - me.getAccountId(), - new Branch.NameKey(project, refName), - now); - - ChangeInserter ins = - changeInserterFactory.create(refControl.getProjectControl(), - change, c); - - validateCommit(git, refControl, c, me, ins); - updateRef(git, rw, c, change, ins.getPatchSet()); - - change.setTopic(input.topic); - ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT); - ins.insert(); - - return Response.created(json.format(change.getId())); - } finally { - rw.release(); + Change change = Iterables.getOnlyElement(changes); + if (!rsrc.getControl().controlFor(change).isVisible(db.get())) { + throw new InvalidChangeOperationException( + "Base change not found: " + input.baseChange); + } + PatchSet ps = db.get().patchSets().get( + new PatchSet.Id(change.getId(), + change.currentPatchSetId().get())); + parentCommit = ObjectId.fromString(ps.getRevision().get()); + } else { + Ref destRef = git.getRef(refName); + if (destRef == null) { + throw new UnprocessableEntityException(String.format( + "Branch %s does not exist.", refName)); + } + parentCommit = destRef.getObjectId(); } - } finally { - git.close(); + RevCommit mergeTip = rw.parseCommit(parentCommit); + + Timestamp now = TimeUtil.nowTs(); + IdentifiedUser me = (IdentifiedUser) userProvider.get(); + PersonIdent author = me.newCommitterIdent(now, serverTimeZone); + + ObjectId id = ChangeIdUtil.computeChangeId(mergeTip.getTree(), + mergeTip, author, author, input.subject); + String commitMessage = ChangeIdUtil.insertId(input.subject, id); + + RevCommit c = newCommit(git, rw, author, mergeTip, commitMessage); + + Change change = new Change( + getChangeId(id, c), + new Change.Id(db.get().nextChangeId()), + me.getAccountId(), + new Branch.NameKey(project, refName), + now); + + ChangeInserter ins = + changeInserterFactory.create(refControl.getProjectControl(), + change, c); + + validateCommit(git, refControl, c, me, ins); + updateRef(git, rw, c, change, ins.getPatchSet()); + + change.setTopic(input.topic); + ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT); + ins.insert(); + + return Response.created(json.format(change.getId())); } } @@ -275,8 +267,7 @@ public class CreateChange implements PersonIdent authorIdent, RevCommit mergeTip, String commitMessage) throws IOException { RevCommit emptyCommit; - ObjectInserter oi = git.newObjectInserter(); - try { + try (ObjectInserter oi = git.newObjectInserter()) { CommitBuilder commit = new CommitBuilder(); commit.setTreeId(mergeTip.getTree().getId()); commit.setParentId(mergeTip); @@ -284,8 +275,6 @@ public class CreateChange implements commit.setCommitter(authorIdent); commit.setMessage(commitMessage); emptyCommit = rw.parseCommit(insert(oi, commit)); - } finally { - oi.release(); } return emptyCommit; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java index 5335cb97cb..a8e1793fee 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java @@ -58,64 +58,57 @@ public class FileContentUtil { public BinaryResult getContent(ProjectState project, ObjectId revstr, String path) throws ResourceNotFoundException, IOException { - Repository repo = openRepository(project); - try { - RevWalk rw = new RevWalk(repo); - try { - RevCommit commit = rw.parseCommit(revstr); - ObjectReader reader = rw.getObjectReader(); - TreeWalk tw = TreeWalk.forPath(reader, path, commit.getTree()); - if (tw == null) { - throw new ResourceNotFoundException(); - } - - org.eclipse.jgit.lib.FileMode mode = tw.getFileMode(0); - ObjectId id = tw.getObjectId(0); - if (mode == org.eclipse.jgit.lib.FileMode.GITLINK) { - return BinaryResult.create(id.name()) - .setContentType(X_GIT_GITLINK) - .base64(); - } - - final ObjectLoader obj = repo.open(id, OBJ_BLOB); - byte[] raw; - try { - raw = obj.getCachedBytes(MAX_SIZE); - } catch (LargeObjectException e) { - raw = null; - } - - BinaryResult result; - if (raw != null) { - result = BinaryResult.create(raw); - } else { - result = asBinaryResult(obj); - } - - String type; - if (mode == org.eclipse.jgit.lib.FileMode.SYMLINK) { - type = X_GIT_SYMLINK; - } else { - type = registry.getMimeType(path, raw).toString(); - type = resolveContentType(project, path, FileMode.FILE, type); - } - return result.setContentType(type).base64(); - } finally { - rw.release(); + try (Repository repo = openRepository(project); + RevWalk rw = new RevWalk(repo)) { + RevCommit commit = rw.parseCommit(revstr); + ObjectReader reader = rw.getObjectReader(); + TreeWalk tw = TreeWalk.forPath(reader, path, commit.getTree()); + if (tw == null) { + throw new ResourceNotFoundException(); } - } finally { - repo.close(); + + org.eclipse.jgit.lib.FileMode mode = tw.getFileMode(0); + ObjectId id = tw.getObjectId(0); + if (mode == org.eclipse.jgit.lib.FileMode.GITLINK) { + return BinaryResult.create(id.name()) + .setContentType(X_GIT_GITLINK) + .base64(); + } + + final ObjectLoader obj = repo.open(id, OBJ_BLOB); + byte[] raw; + try { + raw = obj.getCachedBytes(MAX_SIZE); + } catch (LargeObjectException e) { + raw = null; + } + + BinaryResult result; + if (raw != null) { + result = BinaryResult.create(raw); + } else { + result = asBinaryResult(obj); + } + + String type; + if (mode == org.eclipse.jgit.lib.FileMode.SYMLINK) { + type = X_GIT_SYMLINK; + } else { + type = registry.getMimeType(path, raw).toString(); + type = resolveContentType(project, path, FileMode.FILE, type); + } + return result.setContentType(type).base64(); } } private static BinaryResult asBinaryResult(final ObjectLoader obj) { - @SuppressWarnings("resource") BinaryResult result = new BinaryResult() { @Override public void writeTo(OutputStream os) throws IOException { obj.copyTo(os); } - }.setContentLength(obj.getSize()); + }; + result.setContentLength(obj.getSize()); return result; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java index 1c0b0a4eac..e19b6a91b8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java @@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountPatchReview; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; @@ -178,30 +179,24 @@ public class Files implements ChildCollection { private List query(RevisionResource resource) throws RepositoryNotFoundException, IOException { - Repository git = - gitManager.openRepository(resource.getChange().getProject()); - try { - TreeWalk tw = new TreeWalk(git); - try { - RevCommit c = new RevWalk(tw.getObjectReader()) - .parseCommit(ObjectId.fromString( - resource.getPatchSet().getRevision().get())); + Project.NameKey project = resource.getChange().getProject(); + try (Repository git = gitManager.openRepository(project); + ObjectReader or = git.newObjectReader(); + RevWalk rw = new RevWalk(or); + TreeWalk tw = new TreeWalk(or)) { + RevCommit c = rw.parseCommit( + ObjectId.fromString(resource.getPatchSet().getRevision().get())); - tw.addTree(c.getTree()); - tw.setRecursive(true); - List paths = new ArrayList<>(); - while (tw.next() && paths.size() < 20) { - String s = tw.getPathString(); - if (s.contains(query)) { - paths.add(s); - } + tw.addTree(c.getTree()); + tw.setRecursive(true); + List paths = new ArrayList<>(); + while (tw.next() && paths.size() < 20) { + String s = tw.getPathString(); + if (s.contains(query)) { + paths.add(s); } - return paths; - } finally { - tw.release(); } - } finally { - git.close(); + return paths; } } @@ -261,76 +256,69 @@ public class Files implements ChildCollection { private List copy(Set paths, PatchSet.Id old, RevisionResource resource, Account.Id userId) throws IOException, PatchListNotAvailableException, OrmException { - Repository git = - gitManager.openRepository(resource.getChange().getProject()); - try { - ObjectReader reader = git.newObjectReader(); - try { - PatchList oldList = patchListCache.get( - resource.getChange(), - db.get().patchSets().get(old)); - - PatchList curList = patchListCache.get( - resource.getChange(), - resource.getPatchSet()); - - int sz = paths.size(); - List inserts = Lists.newArrayListWithCapacity(sz); - List pathList = Lists.newArrayListWithCapacity(sz); - + Project.NameKey project = resource.getChange().getProject(); + try (Repository git = gitManager.openRepository(project); + ObjectReader reader = git.newObjectReader(); RevWalk rw = new RevWalk(reader); - TreeWalk tw = new TreeWalk(reader); - tw.setFilter(PathFilterGroup.createFromStrings(paths)); - tw.setRecursive(true); - int o = tw.addTree(rw.parseCommit(oldList.getNewId()).getTree()); - int c = tw.addTree(rw.parseCommit(curList.getNewId()).getTree()); + TreeWalk tw = new TreeWalk(reader)) { + PatchList oldList = patchListCache.get( + resource.getChange(), + db.get().patchSets().get(old)); - int op = -1; - if (oldList.getOldId() != null) { - op = tw.addTree(rw.parseTree(oldList.getOldId())); - } + PatchList curList = patchListCache.get( + resource.getChange(), + resource.getPatchSet()); - int cp = -1; - if (curList.getOldId() != null) { - cp = tw.addTree(rw.parseTree(curList.getOldId())); - } + int sz = paths.size(); + List inserts = Lists.newArrayListWithCapacity(sz); + List pathList = Lists.newArrayListWithCapacity(sz); - while (tw.next()) { - String path = tw.getPathString(); - if (tw.getRawMode(o) != 0 && tw.getRawMode(c) != 0 - && tw.idEqual(o, c) - && paths.contains(path)) { - // File exists in previously reviewed oldList and in curList. - // File content is identical. - inserts.add(new AccountPatchReview( - new Patch.Key( - resource.getPatchSet().getId(), - path), - userId)); - pathList.add(path); - } else if (op >= 0 && cp >= 0 - && tw.getRawMode(o) == 0 && tw.getRawMode(c) == 0 - && tw.getRawMode(op) != 0 && tw.getRawMode(cp) != 0 - && tw.idEqual(op, cp) - && paths.contains(path)) { - // File was deleted in previously reviewed oldList and curList. - // File exists in ancestor of oldList and curList. - // File content is identical in ancestors. - inserts.add(new AccountPatchReview( - new Patch.Key( - resource.getPatchSet().getId(), - path), - userId)); - pathList.add(path); - } - } - db.get().accountPatchReviews().insert(inserts); - return pathList; - } finally { - reader.release(); + tw.setFilter(PathFilterGroup.createFromStrings(paths)); + tw.setRecursive(true); + int o = tw.addTree(rw.parseCommit(oldList.getNewId()).getTree()); + int c = tw.addTree(rw.parseCommit(curList.getNewId()).getTree()); + + int op = -1; + if (oldList.getOldId() != null) { + op = tw.addTree(rw.parseTree(oldList.getOldId())); } - } finally { - git.close(); + + int cp = -1; + if (curList.getOldId() != null) { + cp = tw.addTree(rw.parseTree(curList.getOldId())); + } + + while (tw.next()) { + String path = tw.getPathString(); + if (tw.getRawMode(o) != 0 && tw.getRawMode(c) != 0 + && tw.idEqual(o, c) + && paths.contains(path)) { + // File exists in previously reviewed oldList and in curList. + // File content is identical. + inserts.add(new AccountPatchReview( + new Patch.Key( + resource.getPatchSet().getId(), + path), + userId)); + pathList.add(path); + } else if (op >= 0 && cp >= 0 + && tw.getRawMode(o) == 0 && tw.getRawMode(c) == 0 + && tw.getRawMode(op) != 0 && tw.getRawMode(cp) != 0 + && tw.idEqual(op, cp) + && paths.contains(path)) { + // File was deleted in previously reviewed oldList and curList. + // File exists in ancestor of oldList and curList. + // File content is identical in ancestors. + inserts.add(new AccountPatchReview( + new Patch.Key( + resource.getPatchSet().getId(), + path), + userId)); + pathList.add(path); + } + } + db.get().accountPatchReviews().insert(inserts); + return pathList; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetArchive.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetArchive.java index d602c47d6d..913f69e26e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetArchive.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetArchive.java @@ -126,7 +126,7 @@ public class GetArchive implements RestReadView { @Override public void close() throws IOException { - rw.release(); + rw.close(); repo.close(); } }; @@ -139,7 +139,7 @@ public class GetArchive implements RestReadView { return bin; } finally { if (close) { - rw.release(); + rw.close(); } } } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPatch.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPatch.java index 4a8be84379..afd11569ff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPatch.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPatch.java @@ -94,15 +94,16 @@ public class GetPatch implements RestReadView { private void format(OutputStream out) throws IOException { out.write(formatEmailHeader(commit).getBytes(UTF_8)); - DiffFormatter fmt = new DiffFormatter(out); - fmt.setRepository(repo); - fmt.format(base.getTree(), commit.getTree()); - fmt.flush(); + try (DiffFormatter fmt = new DiffFormatter(out)) { + fmt.setRepository(repo); + fmt.format(base.getTree(), commit.getTree()); + fmt.flush(); + } } @Override public void close() throws IOException { - rw.release(); + rw.close(); repo.close(); } }; @@ -123,7 +124,7 @@ public class GetPatch implements RestReadView { return bin; } finally { if (close) { - rw.release(); + rw.close(); } } } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java index f58a2a0805..8c01403924 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java @@ -80,19 +80,12 @@ public class GetRelated implements RestReadView { @Override public RelatedInfo apply(RevisionResource rsrc) throws RepositoryNotFoundException, IOException, OrmException { - Repository git = gitMgr.openRepository(rsrc.getChange().getProject()); - try { + try (Repository git = gitMgr.openRepository(rsrc.getChange().getProject()); + RevWalk rw = new RevWalk(git)) { Ref ref = git.getRef(rsrc.getChange().getDest().get()); - RevWalk rw = new RevWalk(git); - try { - RelatedInfo info = new RelatedInfo(); - info.changes = walk(rsrc, rw, ref); - return info; - } finally { - rw.release(); - } - } finally { - git.close(); + RelatedInfo info = new RelatedInfo(); + info.changes = walk(rsrc, rw, ref); + return info; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedIn.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedIn.java index e0cdebb89a..7e9bb14747 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedIn.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedIn.java @@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.project.ChangeControl; @@ -55,26 +56,19 @@ class IncludedIn implements RestReadView { ChangeControl ctl = rsrc.getControl(); PatchSet ps = db.get().patchSets().get(ctl.getChange().currentPatchSetId()); - Repository r = - repoManager.openRepository(ctl.getProject().getNameKey()); - try { - RevWalk rw = new RevWalk(r); + Project.NameKey project = ctl.getProject().getNameKey(); + try (Repository r = repoManager.openRepository(project); + RevWalk rw = new RevWalk(r)) { + rw.setRetainBody(false); + RevCommit rev; try { - rw.setRetainBody(false); - RevCommit rev; - try { - rev = rw.parseCommit(ObjectId.fromString(ps.getRevision().get())); - } catch (IncorrectObjectTypeException err) { - throw new BadRequestException(err.getMessage()); - } catch (MissingObjectException err) { - throw new ResourceConflictException(err.getMessage()); - } - return new IncludedInInfo(IncludedInResolver.resolve(r, rw, rev)); - } finally { - rw.release(); + rev = rw.parseCommit(ObjectId.fromString(ps.getRevision().get())); + } catch (IncorrectObjectTypeException err) { + throw new BadRequestException(err.getMessage()); + } catch (MissingObjectException err) { + throw new ResourceConflictException(err.getMessage()); } - } finally { - r.close(); + return new IncludedInInfo(IncludedInResolver.resolve(r, rw, rev)); } } @@ -87,4 +81,4 @@ class IncludedIn implements RestReadView { tags = in.getTags(); } } -} \ No newline at end of file +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java index 5f1ed5bde5..d6b6ab8376 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java @@ -225,8 +225,7 @@ public class MergeabilityCacheImpl implements MergeabilityCache { } try { Map refs = key.load.repo.getAllRefs(); - RevWalk rw = CodeReviewCommit.newRevWalk(key.load.repo); - try { + try (RevWalk rw = CodeReviewCommit.newRevWalk(key.load.repo)) { RevFlag canMerge = rw.newFlag("CAN_MERGE"); CodeReviewCommit rev = parse(rw, key.commit); rev.add(canMerge); @@ -243,8 +242,6 @@ public class MergeabilityCacheImpl implements MergeabilityCache { canMerge, accepted, key.load.dest).dryRun(tip, rev); - } finally { - rw.release(); } } finally { key.load = null; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java index f80a4740c4..6b49206f31 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java @@ -119,14 +119,9 @@ public class RebaseChange { "Cannot rebase: New patch sets are not allowed to be added to change: " + changeId.toString()); } - Repository git = null; - RevWalk rw = null; - ObjectInserter inserter = null; - try { - git = gitManager.openRepository(change.getProject()); - rw = new RevWalk(git); - inserter = git.newObjectInserter(); - + try (Repository git = gitManager.openRepository(change.getProject()); + RevWalk rw = new RevWalk(git); + ObjectInserter inserter = git.newObjectInserter()) { String baseRev = newBaseRev; if (baseRev == null) { baseRev = findBaseRevision(patchSetId, db.get(), @@ -149,16 +144,6 @@ public class RebaseChange { committerIdent, true, ValidatePolicy.GERRIT); } catch (MergeConflictException e) { throw new IOException(e.getMessage()); - } finally { - if (inserter != null) { - inserter.release(); - } - if (rw != null) { - rw.release(); - } - if (git != null) { - git.close(); - } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java index 87895e9f01..d48d0e4b07 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java @@ -29,6 +29,7 @@ import com.google.gerrit.extensions.restapi.RawInput; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; @@ -115,26 +116,20 @@ public class ChangeEditModifier { } IdentifiedUser me = (IdentifiedUser) currentUser.get(); - Repository repo = gitManager.openRepository(change.getProject()); String refPrefix = editRefPrefix(me.getAccountId(), change.getId()); - try { + try (Repository repo = gitManager.openRepository(change.getProject())) { Map refs = repo.getRefDatabase().getRefs(refPrefix); if (!refs.isEmpty()) { throw new ResourceConflictException("edit already exists"); } - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { ObjectId revision = ObjectId.fromString(ps.getRevision().get()); String editRefName = editRefName(me.getAccountId(), change.getId(), ps.getId()); return update(repo, me, editRefName, rw, ObjectId.zeroId(), revision); - } finally { - rw.release(); } - } finally { - repo.close(); } } @@ -159,59 +154,51 @@ public class ChangeEditModifier { IdentifiedUser me = (IdentifiedUser) currentUser.get(); String refName = editRefName(me.getAccountId(), change.getId(), current.getId()); - Repository repo = gitManager.openRepository(change.getProject()); - try { - RevWalk rw = new RevWalk(repo); + try (Repository repo = gitManager.openRepository(change.getProject()); + RevWalk rw = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter()) { BatchRefUpdate ru = repo.getRefDatabase().newBatchUpdate(); - ObjectInserter inserter = repo.newObjectInserter(); - try { - RevCommit editCommit = edit.getEditCommit(); - if (editCommit.getParentCount() == 0) { - throw new InvalidChangeOperationException( - "Rebase edit against root commit not supported"); - } - RevCommit tip = rw.parseCommit(ObjectId.fromString( - current.getRevision().get())); - ThreeWayMerger m = MergeStrategy.RESOLVE.newMerger(repo, true); - m.setObjectInserter(inserter); - m.setBase(ObjectId.fromString( - edit.getBasePatchSet().getRevision().get())); - - if (m.merge(tip, editCommit)) { - ObjectId tree = m.getResultTreeId(); - - CommitBuilder commit = new CommitBuilder(); - commit.setTreeId(tree); - for (int i = 0; i < tip.getParentCount(); i++) { - commit.addParentId(tip.getParent(i)); - } - commit.setAuthor(editCommit.getAuthorIdent()); - commit.setCommitter(new PersonIdent( - editCommit.getCommitterIdent(), TimeUtil.nowTs())); - commit.setMessage(editCommit.getFullMessage()); - ObjectId newEdit = inserter.insert(commit); - inserter.flush(); - - ru.addCommand(new ReceiveCommand(ObjectId.zeroId(), newEdit, - refName)); - ru.addCommand(new ReceiveCommand(edit.getRef().getObjectId(), - ObjectId.zeroId(), edit.getRefName())); - ru.execute(rw, NullProgressMonitor.INSTANCE); - for (ReceiveCommand cmd : ru.getCommands()) { - if (cmd.getResult() != ReceiveCommand.Result.OK) { - throw new IOException("failed: " + cmd); - } - } - } else { - // TODO(davido): Allow to resolve conflicts inline - throw new ResourceConflictException("merge conflict"); - } - } finally { - rw.release(); - inserter.release(); + RevCommit editCommit = edit.getEditCommit(); + if (editCommit.getParentCount() == 0) { + throw new InvalidChangeOperationException( + "Rebase edit against root commit not supported"); + } + RevCommit tip = rw.parseCommit(ObjectId.fromString( + current.getRevision().get())); + ThreeWayMerger m = MergeStrategy.RESOLVE.newMerger(repo, true); + m.setObjectInserter(inserter); + m.setBase(ObjectId.fromString( + edit.getBasePatchSet().getRevision().get())); + + if (m.merge(tip, editCommit)) { + ObjectId tree = m.getResultTreeId(); + + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(tree); + for (int i = 0; i < tip.getParentCount(); i++) { + commit.addParentId(tip.getParent(i)); + } + commit.setAuthor(editCommit.getAuthorIdent()); + commit.setCommitter(new PersonIdent( + editCommit.getCommitterIdent(), TimeUtil.nowTs())); + commit.setMessage(editCommit.getFullMessage()); + ObjectId newEdit = inserter.insert(commit); + inserter.flush(); + + ru.addCommand(new ReceiveCommand(ObjectId.zeroId(), newEdit, + refName)); + ru.addCommand(new ReceiveCommand(edit.getRef().getObjectId(), + ObjectId.zeroId(), edit.getRefName())); + ru.execute(rw, NullProgressMonitor.INSTANCE); + for (ReceiveCommand cmd : ru.getCommands()) { + if (cmd.getResult() != ReceiveCommand.Result.OK) { + throw new IOException("failed: " + cmd); + } + } + } else { + // TODO(davido): Allow to resolve conflicts inline + throw new ResourceConflictException("merge conflict"); } - } finally { - repo.close(); } } @@ -240,23 +227,16 @@ public class ChangeEditModifier { } IdentifiedUser me = (IdentifiedUser) currentUser.get(); - Repository repo = gitManager.openRepository(edit.getChange().getProject()); - try { - RevWalk rw = new RevWalk(repo); - ObjectInserter inserter = repo.newObjectInserter(); - try { - String refName = edit.getRefName(); - ObjectId commit = createCommit(me, inserter, prevEdit, - prevEdit.getTree(), - msg); - inserter.flush(); - return update(repo, me, refName, rw, prevEdit, commit); - } finally { - rw.release(); - inserter.release(); - } - } finally { - repo.close(); + Project.NameKey project = edit.getChange().getProject(); + try (Repository repo = gitManager.openRepository(project); + RevWalk rw = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter()) { + String refName = edit.getRefName(); + ObjectId commit = createCommit(me, inserter, prevEdit, + prevEdit.getTree(), + msg); + inserter.flush(); + return update(repo, me, refName, rw, prevEdit, commit); } } @@ -333,36 +313,28 @@ public class ChangeEditModifier { throw new AuthException("Authentication required"); } IdentifiedUser me = (IdentifiedUser) currentUser.get(); - Repository repo = gitManager.openRepository(edit.getChange().getProject()); - try { - RevWalk rw = new RevWalk(repo); - ObjectInserter inserter = repo.newObjectInserter(); - ObjectReader reader = repo.newObjectReader(); - try { - String refName = edit.getRefName(); - RevCommit prevEdit = edit.getEditCommit(); - ObjectId newTree = writeNewTree(op, - rw, - inserter, - prevEdit, - reader, - file, - newFile, - toBlob(inserter, content)); - if (ObjectId.equals(newTree, prevEdit.getTree())) { - throw new InvalidChangeOperationException("no changes were made"); - } - - ObjectId commit = createCommit(me, inserter, prevEdit, newTree); - inserter.flush(); - return update(repo, me, refName, rw, prevEdit, commit); - } finally { - rw.release(); - inserter.release(); - reader.release(); + Project.NameKey project = edit.getChange().getProject(); + try (Repository repo = gitManager.openRepository(project); + RevWalk rw = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter(); + ObjectReader reader = repo.newObjectReader()) { + String refName = edit.getRefName(); + RevCommit prevEdit = edit.getEditCommit(); + ObjectId newTree = writeNewTree(op, + rw, + inserter, + prevEdit, + reader, + file, + newFile, + toBlob(inserter, content)); + if (ObjectId.equals(newTree, prevEdit.getTree())) { + throw new InvalidChangeOperationException("no changes were made"); } - } finally { - repo.close(); + + ObjectId commit = createCommit(me, inserter, prevEdit, newTree); + inserter.flush(); + return update(repo, me, refName, rw, prevEdit, commit); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 3635b5656e..3d2aa6f4e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -109,8 +109,7 @@ public class ChangeEditUtil { */ public Optional byChange(Change change, IdentifiedUser user) throws IOException { - Repository repo = gitManager.openRepository(change.getProject()); - try { + try (Repository repo = gitManager.openRepository(change.getProject())) { String editRefPrefix = editRefPrefix(user.getAccountId(), change.getId()); Map refs = repo.getRefDatabase().getRefs(editRefPrefix); if (refs.isEmpty()) { @@ -121,16 +120,11 @@ public class ChangeEditUtil { // where there is more than one ref, we could silently delete all but the // current one. Ref ref = Iterables.getOnlyElement(refs.values()); - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { RevCommit commit = rw.parseCommit(ref.getObjectId()); PatchSet basePs = getBasePatchSet(change, ref); return Optional.of(new ChangeEdit(user, change, ref, commit, basePs)); - } finally { - rw.release(); } - } finally { - repo.close(); } } @@ -150,29 +144,19 @@ public class ChangeEditUtil { NoSuchChangeException, IOException, InvalidChangeOperationException, OrmException, ResourceConflictException { Change change = edit.getChange(); - Repository repo = gitManager.openRepository(change.getProject()); - try { - RevWalk rw = new RevWalk(repo); - ObjectInserter inserter = repo.newObjectInserter(); - try { - - PatchSet basePatchSet = edit.getBasePatchSet(); - if (!basePatchSet.getId().equals(change.currentPatchSetId())) { - throw new ResourceConflictException( - "only edit for current patch set can be published"); - } - - insertPatchSet(edit, change, repo, rw, basePatchSet, - squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet)); - } finally { - inserter.release(); - rw.release(); + try (Repository repo = gitManager.openRepository(change.getProject()); + RevWalk rw = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter()) { + PatchSet basePatchSet = edit.getBasePatchSet(); + if (!basePatchSet.getId().equals(change.currentPatchSetId())) { + throw new ResourceConflictException( + "only edit for current patch set can be published"); } + insertPatchSet(edit, change, repo, rw, basePatchSet, + squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet)); // TODO(davido): This should happen in the same BatchRefUpdate. deleteRef(repo, edit); - } finally { - repo.close(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java index 21465ef2c6..d8e56b3310 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java @@ -101,46 +101,38 @@ public class BanCommit { NoteMap banCommitNotes = NoteMap.newEmptyMap(); // Add a note for each banned commit to notes. final Project.NameKey project = projectControl.getProject().getNameKey(); - final Repository repo = repoManager.openRepository(project); - try { - final RevWalk revWalk = new RevWalk(repo); - final ObjectInserter inserter = repo.newObjectInserter(); - try { - ObjectId noteId = null; - for (final ObjectId commitToBan : commitsToBan) { - try { - revWalk.parseCommit(commitToBan); - } catch (MissingObjectException e) { - // Ignore exception, non-existing commits can be banned. - } catch (IncorrectObjectTypeException e) { - result.notACommit(commitToBan); - continue; - } - if (noteId == null) { - noteId = createNoteContent(reason, inserter); - } - banCommitNotes.set(commitToBan, noteId); + try (Repository repo = repoManager.openRepository(project); + RevWalk revWalk = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter()) { + ObjectId noteId = null; + for (final ObjectId commitToBan : commitsToBan) { + try { + revWalk.parseCommit(commitToBan); + } catch (MissingObjectException e) { + // Ignore exception, non-existing commits can be banned. + } catch (IncorrectObjectTypeException e) { + result.notACommit(commitToBan); + continue; } - NotesBranchUtil notesBranchUtil = - notesBranchUtilFactory.create(project, repo, inserter); - NoteMap newlyCreated = - notesBranchUtil.commitNewNotes(banCommitNotes, REFS_REJECT_COMMITS, - createPersonIdent(), buildCommitMessage(commitsToBan, reason)); - - for (Note n : banCommitNotes) { - if (newlyCreated.contains(n)) { - result.commitBanned(n); - } else { - result.commitAlreadyBanned(n); - } + if (noteId == null) { + noteId = createNoteContent(reason, inserter); } - return result; - } finally { - revWalk.release(); - inserter.release(); + banCommitNotes.set(commitToBan, noteId); } - } finally { - repo.close(); + NotesBranchUtil notesBranchUtil = + notesBranchUtilFactory.create(project, repo, inserter); + NoteMap newlyCreated = + notesBranchUtil.commitNewNotes(banCommitNotes, REFS_REJECT_COMMITS, + createPersonIdent(), buildCommitMessage(commitsToBan, reason)); + + for (Note n : banCommitNotes) { + if (newlyCreated.contains(n)) { + result.commitBanned(n); + } else { + result.commitAlreadyBanned(n); + } + } + return result; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 7d3e1b3644..05e864b2ac 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -332,10 +332,10 @@ public class MergeOp { throw new MergeException("Cannot query the database", e); } finally { if (inserter != null) { - inserter.release(); + inserter.close(); } if (rw != null) { - rw.release(); + rw.close(); } if (repo != null) { repo.close(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/NotesBranchUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/NotesBranchUtil.java index f1b0a78de7..fb96a6b5de 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/NotesBranchUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/NotesBranchUtil.java @@ -153,8 +153,8 @@ public class NotesBranchUtil { } updateRef(notesBranch); } finally { - revWalk.release(); - reader.release(); + revWalk.close(); + reader.close(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java index 0e5d9fb4dd..3cf9fed69d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java @@ -382,7 +382,7 @@ public class SubmoduleOp { + subscriber.get(), e); } finally { if (recRw != null) { - recRw.release(); + recRw.close(); } if (pdb != null) { pdb.close(); @@ -392,8 +392,7 @@ public class SubmoduleOp { private static DirCache readTree(final Repository pdb, final Ref branch) throws MissingObjectException, IncorrectObjectTypeException, IOException { - final RevWalk rw = new RevWalk(pdb); - try { + try (RevWalk rw = new RevWalk(pdb)) { final DirCache dc = DirCache.newInCore(); final DirCacheBuilder b = dc.builder(); b.addTree(new byte[0], // no prefix path @@ -401,8 +400,6 @@ public class SubmoduleOp { pdb.newObjectReader(), rw.parseTree(branch.getObjectId())); b.finish(); return dc; - } finally { - rw.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/TagSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/TagSet.java index 799e22053c..a003235483 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/TagSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/TagSet.java @@ -147,7 +147,7 @@ class TagSet { } } finally { if (rw != null) { - rw.release(); + rw.close(); } } } @@ -157,9 +157,8 @@ class TagSet { return; } - TagWalk rw = new TagWalk(git); - rw.setRetainBody(false); - try { + try (TagWalk rw = new TagWalk(git)) { + rw.setRetainBody(false); for (Ref ref : git.getRefDatabase().getRefs(RefDatabase.ALL).values()) { if (skip(ref)) { continue; @@ -188,8 +187,6 @@ class TagSet { } } catch (IOException e) { log.warn("Error building tags for repository " + projectName, e); - } finally { - rw.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java index 8dab448762..b905f67c87 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java @@ -122,7 +122,7 @@ public abstract class VersionedMetaData { revision = id != null ? new RevWalk(reader).parseCommit(id) : null; onLoad(); } finally { - reader.release(); + reader.close(); reader = null; } } @@ -319,13 +319,14 @@ public abstract class VersionedMetaData { public void close() { newTree = null; + rw.close(); if (inserter != null) { - inserter.release(); + inserter.close(); inserter = null; } if (reader != null) { - reader.release(); + reader.close(); reader = null; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java index a0c27c4625..c0522d5ea2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java @@ -323,41 +323,36 @@ public class SiteIndexer { getPathsAndIndex(id); } } finally { - walk.release(); + walk.close(); } return null; } private void getPathsAndIndex(ObjectId b) throws Exception { List cds = Lists.newArrayList(byId.get(b)); - try { + try (DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) { RevCommit bCommit = walk.parseCommit(b); RevTree bTree = bCommit.getTree(); RevTree aTree = aFor(bCommit, walk); - DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE); - try { - df.setRepository(repo); - if (!cds.isEmpty()) { - List paths = (aTree != null) - ? getPaths(df.scan(aTree, bTree)) - : Collections.emptyList(); - Iterator cdit = cds.iterator(); - for (ChangeData cd ; cdit.hasNext(); cdit.remove()) { - cd = cdit.next(); - try { - cd.setCurrentFilePaths(paths); - indexer.index(cd); - done.update(1); - if (verboseWriter != null) { - verboseWriter.println("Reindexed change " + cd.getId()); - } - } catch (Exception e) { - fail("Failed to index change " + cd.getId(), true, e); + df.setRepository(repo); + if (!cds.isEmpty()) { + List paths = (aTree != null) + ? getPaths(df.scan(aTree, bTree)) + : Collections.emptyList(); + Iterator cdit = cds.iterator(); + for (ChangeData cd ; cdit.hasNext(); cdit.remove()) { + cd = cdit.next(); + try { + cd.setCurrentFilePaths(paths); + indexer.index(cd); + done.update(1); + if (verboseWriter != null) { + verboseWriter.println("Reindexed change " + cd.getId()); } + } catch (Exception e) { + fail("Failed to index change " + cd.getId(), true, e); } } - } finally { - df.release(); } } catch (Exception e) { fail("Failed to index commit " + b.name(), false, e); @@ -396,13 +391,10 @@ public class SiteIndexer { } private ObjectId emptyTree() throws IOException { - ObjectInserter oi = repo.newObjectInserter(); - try { + try (ObjectInserter oi = repo.newObjectInserter()) { ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {}); oi.flush(); return id; - } finally { - oi.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java index 405b3a8c55..ac2345565a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java @@ -396,28 +396,28 @@ public abstract class ChangeEmail extends NotificationEmail { TemporaryBuffer.Heap buf = new TemporaryBuffer.Heap(args.settings.maximumDiffSize); - DiffFormatter fmt = new DiffFormatter(buf); - Repository git; - try { - git = args.server.openRepository(change.getProject()); - } catch (IOException e) { - log.error("Cannot open repository to format patch", e); - return ""; - } - try { - fmt.setRepository(git); - fmt.setDetectRenames(true); - fmt.format(patchList.getOldId(), patchList.getNewId()); - return RawParseUtils.decode(buf.toByteArray()); - } catch (IOException e) { - if (JGitText.get().inMemoryBufferLimitExceeded.equals(e.getMessage())) { + try (DiffFormatter fmt = new DiffFormatter(buf)) { + Repository git; + try { + git = args.server.openRepository(change.getProject()); + } catch (IOException e) { + log.error("Cannot open repository to format patch", e); return ""; } - log.error("Cannot format patch", e); - return ""; - } finally { - fmt.release(); - git.close(); + try { + fmt.setRepository(git); + fmt.setDetectRenames(true); + fmt.format(patchList.getOldId(), patchList.getNewId()); + return RawParseUtils.decode(buf.toByteArray()); + } catch (IOException e) { + if (JGitText.get().inMemoryBufferLimitExceeded.equals(e.getMessage())) { + return ""; + } + log.error("Cannot format patch", e); + return ""; + } finally { + git.close(); + } } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java index 667d834422..53efa1eaff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java @@ -82,15 +82,11 @@ public class PatchSetNotificationSender { final Change updatedChange, final PatchSet updatedPatchSet, final LabelTypes labelTypes) throws OrmException, IOException { - final Repository git = repoManager.openRepository(updatedChange.getProject()); - try { - final RevWalk revWalk = new RevWalk(git); + try (Repository git = repoManager.openRepository(updatedChange.getProject())) { final RevCommit commit; - try { + try (RevWalk revWalk = new RevWalk(git)) { commit = revWalk.parseCommit(ObjectId.fromString( updatedPatchSet.getRevision().get())); - } finally { - revWalk.release(); } final PatchSetInfo info = patchSetInfoFactory.get(commit, updatedPatchSet.getId()); final List footerLines = commit.getFooterLines(); @@ -134,8 +130,6 @@ public class PatchSetNotificationSender { log.error("Cannot send email for new patch set " + updatedPatchSet.getId(), e); } } - } finally { - git.close(); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index e97111d02f..6d4f3a4225 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -272,9 +272,9 @@ public class ChangeNotes extends AbstractChangeNotes { loadDefaults(); return; } - RevWalk walk = new RevWalk(reader); - try (ChangeNotesParser parser = - new ChangeNotesParser(change, rev, walk, repoManager)) { + try (RevWalk walk = new RevWalk(reader); + ChangeNotesParser parser = + new ChangeNotesParser(change, rev, walk, repoManager)) { parser.parseAll(); if (parser.status != null) { @@ -301,8 +301,6 @@ public class ChangeNotes extends AbstractChangeNotes { this.allPastReviewers = ImmutableList.copyOf(parser.allPastReviewers); submitRecords = ImmutableList.copyOf(parser.submitRecords); - } finally { - walk.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java index 1053f09bb1..76dfdc8a13 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java @@ -200,12 +200,9 @@ public class ChangeRebuilder { private void writeToBatch(BatchMetaDataUpdate batch, AbstractChangeUpdate update, Repository repo) throws IOException, OrmException { - ObjectInserter inserter = repo.newObjectInserter(); - try { + try (ObjectInserter inserter = repo.newObjectInserter()) { update.setInserter(inserter); update.writeCommit(batch); - } finally { - inserter.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java index d40358bbac..20d6c4aa3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java @@ -132,16 +132,14 @@ public class DraftCommentNotes extends AbstractChangeNotes { return; } - RevWalk walk = new RevWalk(reader); - try (DraftCommentNotesParser parser = new DraftCommentNotesParser( - getChangeId(), walk, rev, repoManager, draftsProject, author)) { + try (RevWalk walk = new RevWalk(reader); + DraftCommentNotesParser parser = new DraftCommentNotesParser( + getChangeId(), walk, rev, repoManager, draftsProject, author)) { parser.parseDraftComments(); buildCommentTable(draftBaseComments, parser.draftBaseComments); buildCommentTable(draftPsComments, parser.draftPsComments); noteMap = parser.noteMap; - } finally { - walk.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java index 2e63a76a03..8740a6b75f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java @@ -48,9 +48,8 @@ public class PatchFile { this.repo = repo; this.entry = patchList.get(fileName); - final ObjectReader reader = repo.newObjectReader(); - try { - final RevWalk rw = new RevWalk(reader); + try (ObjectReader reader = repo.newObjectReader(); + RevWalk rw = new RevWalk(reader)) { final RevCommit bCommit = rw.parseCommit(patchList.getNewId()); if (Patch.COMMIT_MSG.equals(fileName)) { @@ -74,8 +73,6 @@ public class PatchFile { } bTree = bCommit.getTree(); } - } finally { - reader.release(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index 0c8c5ecac1..bfb8c005e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -116,9 +116,9 @@ public class PatchListLoader extends CacheLoader { private PatchList readPatchList(final PatchListKey key, final Repository repo) throws IOException, PatchListNotAvailableException { final RawTextComparator cmp = comparatorFor(key.getWhitespace()); - final ObjectReader reader = repo.newObjectReader(); - try { - final RevWalk rw = new RevWalk(reader); + try (ObjectReader reader = repo.newObjectReader(); + RevWalk rw = new RevWalk(reader); + DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) { final RevCommit b = rw.parseCommit(key.getNewId()); final RevObject a = aFor(key, repo, rw, b); @@ -138,7 +138,6 @@ public class PatchListLoader extends CacheLoader { RevTree aTree = rw.parseTree(a); RevTree bTree = b.getTree(); - DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE); df.setRepository(repo); df.setDiffComparator(cmp); df.setDetectRenames(true); @@ -170,8 +169,6 @@ public class PatchListLoader extends CacheLoader { } return new PatchList(a, b, againstParent, entries.toArray(new PatchListEntry[entries.size()])); - } finally { - reader.release(); } } @@ -271,8 +268,7 @@ public class PatchListLoader extends CacheLoader { } ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(repo, true); - final ObjectInserter ins = repo.newObjectInserter(); - try { + try (ObjectInserter ins = repo.newObjectInserter()) { DirCache dc = DirCache.newInCore(); m.setDirCache(dc); m.setObjectInserter(new ObjectInserter.Filter() { @@ -397,19 +393,14 @@ public class PatchListLoader extends CacheLoader { } return rw.lookupTree(treeId); - } finally { - ins.release(); } } private static ObjectId emptyTree(final Repository repo) throws IOException { - ObjectInserter oi = repo.newObjectInserter(); - try { + try (ObjectInserter oi = repo.newObjectInserter()) { ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {}); oi.flush(); return id; - } finally { - oi.release(); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java index d2316647b1..ea427eb4f3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java @@ -119,7 +119,7 @@ class PatchScriptBuilder { try { return build(content, comments, history); } finally { - reader.release(); + reader.close(); } } @@ -514,9 +514,10 @@ class PatchScriptBuilder { if (path == null || within == null) { return null; } - final RevWalk rw = new RevWalk(reader); - final RevTree tree = rw.parseTree(within); - return TreeWalk.forPath(reader, path, tree); + try (RevWalk rw = new RevWalk(reader)) { + final RevTree tree = rw.parseTree(within); + return TreeWalk.forPath(reader, path, tree); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java index 340300af19..1e4ffce1a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java @@ -85,17 +85,12 @@ public class PatchSetInfoFactory { } catch (IOException e) { throw new PatchSetInfoNotAvailableException(e); } - try { - final RevWalk rw = new RevWalk(repo); - try { - final RevCommit src = - rw.parseCommit(ObjectId.fromString(patchSet.getRevision().get())); - PatchSetInfo info = get(src, patchSet.getId()); - info.setParents(toParentInfos(src.getParents(), rw)); - return info; - } finally { - rw.release(); - } + try (RevWalk rw = new RevWalk(repo)) { + final RevCommit src = + rw.parseCommit(ObjectId.fromString(patchSet.getRevision().get())); + PatchSetInfo info = get(src, patchSet.getId()); + info.setParents(toParentInfos(src.getParents(), rw)); + return info; } catch (IOException e) { throw new PatchSetInfoNotAvailableException(e); } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java index 1939c84faa..882e25f6f1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java @@ -44,44 +44,45 @@ public class Text extends RawText { public static final Text EMPTY = new Text(NO_BYTES); public static Text forCommit(ObjectReader reader, AnyObjectId commitId) throws IOException { - RevWalk rw = new RevWalk(reader); - RevCommit c; - if (commitId instanceof RevCommit) { - c = (RevCommit) commitId; - } else { - c = rw.parseCommit(commitId); - } - - StringBuilder b = new StringBuilder(); - switch (c.getParentCount()) { - case 0: - break; - case 1: { - RevCommit p = c.getParent(0); - rw.parseBody(p); - b.append("Parent: "); - b.append(reader.abbreviate(p, 8).name()); - b.append(" ("); - b.append(p.getShortMessage()); - b.append(")\n"); - break; + try (RevWalk rw = new RevWalk(reader)) { + RevCommit c; + if (commitId instanceof RevCommit) { + c = (RevCommit) commitId; + } else { + c = rw.parseCommit(commitId); } - default: - for (int i = 0; i < c.getParentCount(); i++) { - RevCommit p = c.getParent(i); + + StringBuilder b = new StringBuilder(); + switch (c.getParentCount()) { + case 0: + break; + case 1: { + RevCommit p = c.getParent(0); rw.parseBody(p); - b.append(i == 0 ? "Merge Of: " : " "); + b.append("Parent: "); b.append(reader.abbreviate(p, 8).name()); b.append(" ("); b.append(p.getShortMessage()); b.append(")\n"); + break; } + default: + for (int i = 0; i < c.getParentCount(); i++) { + RevCommit p = c.getParent(i); + rw.parseBody(p); + b.append(i == 0 ? "Merge Of: " : " "); + b.append(reader.abbreviate(p, 8).name()); + b.append(" ("); + b.append(p.getShortMessage()); + b.append(")\n"); + } + } + appendPersonIdent(b, "Author", c.getAuthorIdent()); + appendPersonIdent(b, "Commit", c.getCommitterIdent()); + b.append("\n"); + b.append(c.getFullMessage()); + return new Text(b.toString().getBytes("UTF-8")); } - appendPersonIdent(b, "Author", c.getAuthorIdent()); - appendPersonIdent(b, "Commit", c.getCommitterIdent()); - b.append("\n"); - b.append(c.getFullMessage()); - return new Text(b.toString().getBytes("UTF-8")); } private static void appendPersonIdent(StringBuilder b, String field, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java index e5e7bed21c..4879bb7b88 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java @@ -65,26 +65,19 @@ public class CommitsCollection implements throw new ResourceNotFoundException(id); } - Repository repo = repoManager.openRepository(parent.getNameKey()); - try { - RevWalk rw = new RevWalk(repo); - try { - RevCommit commit = rw.parseCommit(objectId); - rw.parseBody(commit); - if (!parent.getControl().canReadCommit(db.get(), rw, commit)) { - throw new ResourceNotFoundException(id); - } - for (int i = 0; i < commit.getParentCount(); i++) { - rw.parseBody(rw.parseCommit(commit.getParent(i))); - } - return new CommitResource(parent, commit); - } catch (MissingObjectException | IncorrectObjectTypeException e) { + try (Repository repo = repoManager.openRepository(parent.getNameKey()); + RevWalk rw = new RevWalk(repo)) { + RevCommit commit = rw.parseCommit(objectId); + rw.parseBody(commit); + if (!parent.getControl().canReadCommit(db.get(), rw, commit)) { throw new ResourceNotFoundException(id); - } finally { - rw.release(); } - } finally { - repo.close(); + for (int i = 0; i < commit.getParentCount(); i++) { + rw.parseBody(rw.parseCommit(commit.getParent(i))); + } + return new CommitResource(parent, commit); + } catch (MissingObjectException | IncorrectObjectTypeException e) { + throw new ResourceNotFoundException(id); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java index bdc67ac774..d6e93f08c5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java @@ -99,11 +99,8 @@ class DeleteBranches implements RestModifyView { for (String branch : input.branches) { batchUpdate.addCommand(createDeleteCommand(project, r, branch)); } - RevWalk rw = new RevWalk(r); - try { + try (RevWalk rw = new RevWalk(r)) { batchUpdate.execute(rw, NullProgressMonitor.INSTANCE); - } finally { - rw.release(); } StringBuilder errorMessages = new StringBuilder(); for (ReceiveCommand command : batchUpdate.getCommands()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java index 0530a4cdf8..2efd257bcf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java @@ -49,9 +49,7 @@ public class GetHead implements RestReadView { @Override public String apply(ProjectResource rsrc) throws AuthException, ResourceNotFoundException, IOException { - Repository repo = null; - try { - repo = repoManager.openRepository(rsrc.getNameKey()); + try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) { Ref head = repo.getRef(Constants.HEAD); if (head == null) { throw new ResourceNotFoundException(Constants.HEAD); @@ -62,8 +60,7 @@ public class GetHead implements RestReadView { } throw new AuthException("not allowed to see HEAD"); } else if (head.getObjectId() != null) { - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { RevCommit commit = rw.parseCommit(head.getObjectId()); if (rsrc.getControl().canReadCommit(db.get(), rw, commit)) { return head.getObjectId().name(); @@ -74,17 +71,11 @@ public class GetHead implements RestReadView { return head.getObjectId().name(); } throw new AuthException("not allowed to see HEAD"); - } finally { - rw.release(); } } throw new ResourceNotFoundException(Constants.HEAD); } catch (RepositoryNotFoundException e) { throw new ResourceNotFoundException(rsrc.getName()); - } finally { - if (repo != null) { - repo.close(); - } } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListDashboards.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListDashboards.java index 07fd095f74..456abfcdf5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListDashboards.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListDashboards.java @@ -82,53 +82,45 @@ class ListDashboards implements RestReadView { private List scan(ProjectControl ctl, String project, boolean setDefault) throws ResourceNotFoundException, IOException { - Repository git; - try { - git = gitManager.openRepository(ctl.getProject().getNameKey()); + Project.NameKey projectName = ctl.getProject().getNameKey(); + try (Repository git = gitManager.openRepository(projectName); + RevWalk rw = new RevWalk(git)) { + List all = Lists.newArrayList(); + for (Ref ref : git.getRefDatabase().getRefs(REFS_DASHBOARDS).values()) { + if (ctl.controlForRef(ref.getName()).canRead()) { + all.addAll(scanDashboards(ctl.getProject(), git, rw, ref, + project, setDefault)); + } + } + return all; } catch (RepositoryNotFoundException e) { throw new ResourceNotFoundException(); } - try { - RevWalk rw = new RevWalk(git); - try { - List all = Lists.newArrayList(); - for (Ref ref : git.getRefDatabase().getRefs(REFS_DASHBOARDS).values()) { - if (ctl.controlForRef(ref.getName()).canRead()) { - all.addAll(scanDashboards(ctl.getProject(), git, rw, ref, - project, setDefault)); - } - } - return all; - } finally { - rw.release(); - } - } finally { - git.close(); - } } private List scanDashboards(Project definingProject, Repository git, RevWalk rw, Ref ref, String project, boolean setDefault) throws IOException { List list = Lists.newArrayList(); - TreeWalk tw = new TreeWalk(rw.getObjectReader()); - tw.addTree(rw.parseTree(ref.getObjectId())); - tw.setRecursive(true); - while (tw.next()) { - if (tw.getFileMode(0) == FileMode.REGULAR_FILE) { - try { - list.add(DashboardsCollection.parse( - definingProject, - ref.getName().substring(REFS_DASHBOARDS.length()), - tw.getPathString(), - new BlobBasedConfig(null, git, tw.getObjectId(0)), - project, - setDefault)); - } catch (ConfigInvalidException e) { - log.warn(String.format( - "Cannot parse dashboard %s:%s:%s: %s", - definingProject.getName(), ref.getName(), tw.getPathString(), - e.getMessage())); + try (TreeWalk tw = new TreeWalk(rw.getObjectReader())) { + tw.addTree(rw.parseTree(ref.getObjectId())); + tw.setRecursive(true); + while (tw.next()) { + if (tw.getFileMode(0) == FileMode.REGULAR_FILE) { + try { + list.add(DashboardsCollection.parse( + definingProject, + ref.getName().substring(REFS_DASHBOARDS.length()), + tw.getPathString(), + new BlobBasedConfig(null, git, tw.getObjectId(0)), + project, + setDefault)); + } catch (ConfigInvalidException e) { + log.warn(String.format( + "Cannot parse dashboard %s:%s:%s: %s", + definingProject.getName(), ref.getName(), tw.getPathString(), + e.getMessage())); + } } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java index e12b38a2f0..b01e563e1e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java @@ -98,26 +98,17 @@ public class ListTags implements RestReadView { public TagInfo get(ProjectResource resource, IdString id) throws ResourceNotFoundException, IOException { - Repository repo = getRepository(resource.getNameKey()); - - String tagName = id.get(); - if (!tagName.startsWith(Constants.R_TAGS)) { - tagName = Constants.R_TAGS + tagName; - } - - try { - RevWalk rw = new RevWalk(repo); - try { - Ref ref = repo.getRefDatabase().getRef(tagName); - if (ref != null && !visibleTags(resource.getControl(), repo, - ImmutableMap.of(ref.getName(), ref)).isEmpty()) { - return createTagInfo(ref, rw); - } - } finally { - rw.dispose(); + try (Repository repo = getRepository(resource.getNameKey()); + RevWalk rw = new RevWalk(repo)) { + String tagName = id.get(); + if (!tagName.startsWith(Constants.R_TAGS)) { + tagName = Constants.R_TAGS + tagName; + } + Ref ref = repo.getRefDatabase().getRef(tagName); + if (ref != null && !visibleTags(resource.getControl(), repo, + ImmutableMap.of(ref.getName(), ref)).isEmpty()) { + return createTagInfo(ref, rw); } - } finally { - repo.close(); } throw new ResourceNotFoundException(id); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PerformCreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PerformCreateProject.java index 03c954b54a..689920b6d1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PerformCreateProject.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PerformCreateProject.java @@ -270,8 +270,7 @@ public class PerformCreateProject { private void createEmptyCommits(final Repository repo, final Project.NameKey project, final List refs) throws IOException { - ObjectInserter oi = repo.newObjectInserter(); - try { + try (ObjectInserter oi = repo.newObjectInserter()) { CommitBuilder cb = new CommitBuilder(); cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {})); cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent()); @@ -300,8 +299,6 @@ public class PerformCreateProject { "Cannot create empty commit for " + createProjectArgs.getProjectName(), e); throw e; - } finally { - oi.release(); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index f8279a0a07..a43247ec6e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -523,18 +523,11 @@ public class ChangeData { return false; } String sha1 = ps.getRevision().get(); - Repository repo = repoManager.openRepository(change().getProject()); - try { - RevWalk walk = new RevWalk(repo); - try { - RevCommit c = walk.parseCommit(ObjectId.fromString(sha1)); - commitMessage = c.getFullMessage(); - commitFooters = c.getFooterLines(); - } finally { - walk.release(); - } - } finally { - repo.close(); + try (Repository repo = repoManager.openRepository(change().getProject()); + RevWalk walk = new RevWalk(repo)) { + RevCommit c = walk.parseCommit(ObjectId.fromString(sha1)); + commitMessage = c.getFullMessage(); + commitFooters = c.getFooterLines(); } return true; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java index 87c33a2129..207074620f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java @@ -107,37 +107,24 @@ class ConflictsPredicate extends OrPredicate { if (conflicts != null) { return conflicts; } - try { - Repository repo = + try (Repository repo = args.repoManager.openRepository(otherChange.getProject()); - try { - RevWalk rw = CodeReviewCommit.newRevWalk(repo); - try { - RevFlag canMergeFlag = rw.newFlag("CAN_MERGE"); - CodeReviewCommit commit = - (CodeReviewCommit) rw.parseCommit(changeDataCache.getTestAgainst()); - SubmitStrategy strategy = - args.submitStrategyFactory.create(submitType, - db.get(), repo, rw, null, canMergeFlag, - getAlreadyAccepted(repo, rw, commit), - otherChange.getDest()); - CodeReviewCommit otherCommit = - (CodeReviewCommit) rw.parseCommit(other); - otherCommit.add(canMergeFlag); - conflicts = !strategy.dryRun(commit, otherCommit); - args.conflictsCache.put(conflictsKey, conflicts); - return conflicts; - } catch (MergeException e) { - throw new IllegalStateException(e); - } catch (NoSuchProjectException e) { - throw new IllegalStateException(e); - } finally { - rw.release(); - } - } finally { - repo.close(); - } - } catch (IOException e) { + RevWalk rw = CodeReviewCommit.newRevWalk(repo)) { + RevFlag canMergeFlag = rw.newFlag("CAN_MERGE"); + CodeReviewCommit commit = + (CodeReviewCommit) rw.parseCommit(changeDataCache.getTestAgainst()); + SubmitStrategy strategy = + args.submitStrategyFactory.create(submitType, + db.get(), repo, rw, null, canMergeFlag, + getAlreadyAccepted(repo, rw, commit), + otherChange.getDest()); + CodeReviewCommit otherCommit = + (CodeReviewCommit) rw.parseCommit(other); + otherCommit.add(canMergeFlag); + conflicts = !strategy.dryRun(commit, otherCommit); + args.conflictsCache.put(conflictsKey, conflicts); + return conflicts; + } catch (MergeException | NoSuchProjectException | IOException e) { throw new IllegalStateException(e); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevWalkPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevWalkPredicate.java index fd57a44f40..0947fae24e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevWalkPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevWalkPredicate.java @@ -102,18 +102,9 @@ public abstract class RevWalkPredicate extends OperatorPredicate { Arguments args = new Arguments(patchSet, revision, objectId, change, projectName); - try { - final Repository repo = repoManager.openRepository(projectName); - try { - final RevWalk rw = new RevWalk(repo); - try { - return match(repo, rw, args); - } finally { - rw.release(); - } - } finally { - repo.close(); - } + try (Repository repo = repoManager.openRepository(projectName); + RevWalk rw = new RevWalk(repo)) { + return match(repo, rw, args); } catch (RepositoryNotFoundException e) { log.error("Repository \"" + projectName.get() + "\" unknown.", e); } catch (IOException e) { diff --git a/gerrit-server/src/main/java/gerrit/PRED_commit_edits_2.java b/gerrit-server/src/main/java/gerrit/PRED_commit_edits_2.java index 2c7949c191..509faf0313 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_commit_edits_2.java +++ b/gerrit-server/src/main/java/gerrit/PRED_commit_edits_2.java @@ -73,11 +73,10 @@ public class PRED_commit_edits_2 extends Predicate.P2 { PatchList pl = StoredValues.PATCH_LIST.get(engine); Repository repo = StoredValues.REPOSITORY.get(engine); - final ObjectReader reader = repo.newObjectReader(); - final RevTree aTree; - final RevTree bTree; - try { - final RevWalk rw = new RevWalk(reader); + try (ObjectReader reader = repo.newObjectReader(); + RevWalk rw = new RevWalk(reader)) { + final RevTree aTree; + final RevTree bTree; final RevCommit bCommit = rw.parseCommit(pl.getNewId()); if (pl.getOldId() != null) { @@ -129,8 +128,6 @@ public class PRED_commit_edits_2 extends Predicate.P2 { } } catch (IOException err) { throw new JavaException(this, 1, err); - } finally { - reader.release(); } return engine.fail(); @@ -161,4 +158,4 @@ public class PRED_commit_edits_2 extends Predicate.P2 { } return new Text(reader.open(tw.getObjectId(0), Constants.OBJ_BLOB)); } -} \ No newline at end of file +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/IncludedInResolverTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/IncludedInResolverTest.java index dc9724c782..4cd31abdea 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/change/IncludedInResolverTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/IncludedInResolverTest.java @@ -86,6 +86,8 @@ public class IncludedInResolverTest extends RepositoryTestCase { */ + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") Git git = new Git(db); revWalk = new RevWalk(db); // Version 1.0 @@ -129,7 +131,7 @@ public class IncludedInResolverTest extends RepositoryTestCase { @Override @After public void tearDown() throws Exception { - revWalk.release(); + revWalk.close(); super.tearDown(); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java index f4f989fc96..c2f3f6f4cc 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java @@ -118,34 +118,37 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { public void testEmptyCommit() throws Exception { expect(schemaFactory.open()).andReturn(schema); - final Repository realDb = createWorkRepository(); - final Git git = new Git(realDb); + try (Repository realDb = createWorkRepository()) { + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git git = new Git(realDb); - final RevCommit mergeTip = git.commit().setMessage("test").call(); + final RevCommit mergeTip = git.commit().setMessage("test").call(); - final Branch.NameKey branchNameKey = - new Branch.NameKey(new Project.NameKey("test-project"), "test-branch"); + final Branch.NameKey branchNameKey = + new Branch.NameKey(new Project.NameKey("test-project"), "test-branch"); - expect(urlProvider.get()).andReturn("http://localhost:8080"); + expect(urlProvider.get()).andReturn("http://localhost:8080"); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - final ResultSet emptySubscriptions = - new ListResultSet<>(new ArrayList()); - expect(subscriptions.bySubmodule(branchNameKey)).andReturn( - emptySubscriptions); + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + final ResultSet emptySubscriptions = + new ListResultSet<>(new ArrayList()); + expect(subscriptions.bySubmodule(branchNameKey)).andReturn( + emptySubscriptions); - schema.close(); + schema.close(); - doReplay(); + doReplay(); - final SubmoduleOp submoduleOp = - new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider, - schemaFactory, realDb, null, new ArrayList(), null, null, - null, null, null, null); + final SubmoduleOp submoduleOp = + new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider, + schemaFactory, realDb, null, new ArrayList(), null, null, + null, null, null, null); - submoduleOp.update(); + submoduleOp.update(); - doVerify(); + doVerify(); + } } /** @@ -588,85 +591,89 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { public void testOneSubscriberToUpdate() throws Exception { expect(schemaFactory.open()).andReturn(schema); - final Repository sourceRepository = createWorkRepository(); - final Git sourceGit = new Git(sourceRepository); + try (Repository sourceRepository = createWorkRepository(); + Repository targetRepository = createWorkRepository()) { + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git sourceGit = new Git(sourceRepository); + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git targetGit = new Git(targetRepository); - addRegularFileToIndex("file.txt", "test content", sourceRepository); + addRegularFileToIndex("file.txt", "test content", sourceRepository); - final RevCommit sourceMergeTip = - sourceGit.commit().setMessage("test").call(); + final RevCommit sourceMergeTip = + sourceGit.commit().setMessage("test").call(); - final Branch.NameKey sourceBranchNameKey = - new Branch.NameKey(new Project.NameKey("source-project"), - "refs/heads/master"); + final Branch.NameKey sourceBranchNameKey = + new Branch.NameKey(new Project.NameKey("source-project"), + "refs/heads/master"); - final CodeReviewCommit codeReviewCommit = - new CodeReviewCommit(sourceMergeTip.toObjectId()); - final Change submittedChange = new Change( - new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1), - new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs()); + final CodeReviewCommit codeReviewCommit = + new CodeReviewCommit(sourceMergeTip.toObjectId()); + final Change submittedChange = new Change( + new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1), + new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs()); - final Map mergedCommits = new HashMap<>(); - mergedCommits.put(submittedChange.getId(), codeReviewCommit); + final Map mergedCommits = new HashMap<>(); + mergedCommits.put(submittedChange.getId(), codeReviewCommit); - final List submitted = new ArrayList<>(); - submitted.add(submittedChange); + final List submitted = new ArrayList<>(); + submitted.add(submittedChange); - final Repository targetRepository = createWorkRepository(); - final Git targetGit = new Git(targetRepository); + addGitLinkToIndex("a", sourceMergeTip.copy(), targetRepository); - addGitLinkToIndex("a", sourceMergeTip.copy(), targetRepository); + targetGit.commit().setMessage("test").call(); - targetGit.commit().setMessage("test").call(); + final Branch.NameKey targetBranchNameKey = + new Branch.NameKey(new Project.NameKey("target-project"), + sourceBranchNameKey.get()); - final Branch.NameKey targetBranchNameKey = - new Branch.NameKey(new Project.NameKey("target-project"), - sourceBranchNameKey.get()); + expect(urlProvider.get()).andReturn("http://localhost:8080"); - expect(urlProvider.get()).andReturn("http://localhost:8080"); + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + final ResultSet subscribers = + new ListResultSet<>(Collections + .singletonList(new SubmoduleSubscription(targetBranchNameKey, + sourceBranchNameKey, "source-project"))); + expect(subscriptions.bySubmodule(sourceBranchNameKey)).andReturn( + subscribers); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - final ResultSet subscribers = - new ListResultSet<>(Collections - .singletonList(new SubmoduleSubscription(targetBranchNameKey, - sourceBranchNameKey, "source-project"))); - expect(subscriptions.bySubmodule(sourceBranchNameKey)).andReturn( - subscribers); + expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) + .andReturn(targetRepository).anyTimes(); - expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) - .andReturn(targetRepository).anyTimes(); + Capture ruCapture = new Capture<>(); + gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), + capture(ruCapture)); + changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), + anyObject(RefUpdate.class), EasyMock.isNull()); - Capture ruCapture = new Capture<>(); - gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), - capture(ruCapture)); - changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), - anyObject(RefUpdate.class), EasyMock.isNull()); + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + final ResultSet emptySubscriptions = + new ListResultSet<>(new ArrayList()); + expect(subscriptions.bySubmodule(targetBranchNameKey)).andReturn( + emptySubscriptions); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - final ResultSet emptySubscriptions = - new ListResultSet<>(new ArrayList()); - expect(subscriptions.bySubmodule(targetBranchNameKey)).andReturn( - emptySubscriptions); + schema.close(); - schema.close(); + final PersonIdent myIdent = + new PersonIdent("test-user", "test-user@email.com"); - final PersonIdent myIdent = - new PersonIdent("test-user", "test-user@email.com"); + doReplay(); - doReplay(); + final SubmoduleOp submoduleOp = + new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( + sourceRepository), urlProvider, schemaFactory, sourceRepository, + new Project(sourceBranchNameKey.getParentKey()), submitted, + mergedCommits, myIdent, repoManager, gitRefUpdated, null, + changeHooks); - final SubmoduleOp submoduleOp = - new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( - sourceRepository), urlProvider, schemaFactory, sourceRepository, - new Project(sourceBranchNameKey.getParentKey()), submitted, - mergedCommits, myIdent, repoManager, gitRefUpdated, null, - changeHooks); + submoduleOp.update(); - submoduleOp.update(); - - doVerify(); - RefUpdate ru = ruCapture.getValue(); - assertEquals(ru.getName(), targetBranchNameKey.get()); + doVerify(); + RefUpdate ru = ruCapture.getValue(); + assertEquals(ru.getName(), targetBranchNameKey.get()); + } } /** @@ -693,86 +700,90 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { public void testAvoidingCircularReference() throws Exception { expect(schemaFactory.open()).andReturn(schema); - final Repository sourceRepository = createWorkRepository(); - final Git sourceGit = new Git(sourceRepository); + try (Repository sourceRepository = createWorkRepository(); + Repository targetRepository = createWorkRepository()) { + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git sourceGit = new Git(sourceRepository); + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git targetGit = new Git(targetRepository); - addRegularFileToIndex("file.txt", "test content", sourceRepository); + addRegularFileToIndex("file.txt", "test content", sourceRepository); - final RevCommit sourceMergeTip = - sourceGit.commit().setMessage("test").call(); + final RevCommit sourceMergeTip = + sourceGit.commit().setMessage("test").call(); - final Branch.NameKey sourceBranchNameKey = - new Branch.NameKey(new Project.NameKey("source-project"), - "refs/heads/master"); + final Branch.NameKey sourceBranchNameKey = + new Branch.NameKey(new Project.NameKey("source-project"), + "refs/heads/master"); - final CodeReviewCommit codeReviewCommit = - new CodeReviewCommit(sourceMergeTip.toObjectId()); - final Change submittedChange = new Change( - new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1), - new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs()); + final CodeReviewCommit codeReviewCommit = + new CodeReviewCommit(sourceMergeTip.toObjectId()); + final Change submittedChange = new Change( + new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1), + new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs()); - final Map mergedCommits = new HashMap<>(); - mergedCommits.put(submittedChange.getId(), codeReviewCommit); + final Map mergedCommits = new HashMap<>(); + mergedCommits.put(submittedChange.getId(), codeReviewCommit); - final List submitted = new ArrayList<>(); - submitted.add(submittedChange); + final List submitted = new ArrayList<>(); + submitted.add(submittedChange); - final Repository targetRepository = createWorkRepository(); - final Git targetGit = new Git(targetRepository); + addGitLinkToIndex("a", sourceMergeTip.copy(), targetRepository); - addGitLinkToIndex("a", sourceMergeTip.copy(), targetRepository); + targetGit.commit().setMessage("test").call(); - targetGit.commit().setMessage("test").call(); + final Branch.NameKey targetBranchNameKey = + new Branch.NameKey(new Project.NameKey("target-project"), + sourceBranchNameKey.get()); - final Branch.NameKey targetBranchNameKey = - new Branch.NameKey(new Project.NameKey("target-project"), - sourceBranchNameKey.get()); + expect(urlProvider.get()).andReturn("http://localhost:8080"); - expect(urlProvider.get()).andReturn("http://localhost:8080"); + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + final ResultSet subscribers = + new ListResultSet<>(Collections + .singletonList(new SubmoduleSubscription(targetBranchNameKey, + sourceBranchNameKey, "source-project"))); + expect(subscriptions.bySubmodule(sourceBranchNameKey)).andReturn( + subscribers); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - final ResultSet subscribers = - new ListResultSet<>(Collections - .singletonList(new SubmoduleSubscription(targetBranchNameKey, - sourceBranchNameKey, "source-project"))); - expect(subscriptions.bySubmodule(sourceBranchNameKey)).andReturn( - subscribers); + expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) + .andReturn(targetRepository).anyTimes(); - expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) - .andReturn(targetRepository).anyTimes(); + Capture ruCapture = new Capture<>(); + gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), + capture(ruCapture)); + changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), + anyObject(RefUpdate.class), EasyMock.isNull()); - Capture ruCapture = new Capture<>(); - gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()), - capture(ruCapture)); - changeHooks.doRefUpdatedHook(eq(targetBranchNameKey), - anyObject(RefUpdate.class), EasyMock.isNull()); + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + final ResultSet incorrectSubscriptions = + new ListResultSet<>(Collections + .singletonList(new SubmoduleSubscription(sourceBranchNameKey, + targetBranchNameKey, "target-project"))); + expect(subscriptions.bySubmodule(targetBranchNameKey)).andReturn( + incorrectSubscriptions); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - final ResultSet incorrectSubscriptions = - new ListResultSet<>(Collections - .singletonList(new SubmoduleSubscription(sourceBranchNameKey, - targetBranchNameKey, "target-project"))); - expect(subscriptions.bySubmodule(targetBranchNameKey)).andReturn( - incorrectSubscriptions); + schema.close(); - schema.close(); + final PersonIdent myIdent = + new PersonIdent("test-user", "test-user@email.com"); - final PersonIdent myIdent = - new PersonIdent("test-user", "test-user@email.com"); + doReplay(); - doReplay(); + final SubmoduleOp submoduleOp = + new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( + sourceRepository), urlProvider, schemaFactory, sourceRepository, + new Project(sourceBranchNameKey.getParentKey()), submitted, + mergedCommits, myIdent, repoManager, gitRefUpdated, null, changeHooks); - final SubmoduleOp submoduleOp = - new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( - sourceRepository), urlProvider, schemaFactory, sourceRepository, - new Project(sourceBranchNameKey.getParentKey()), submitted, - mergedCommits, myIdent, repoManager, gitRefUpdated, null, changeHooks); + submoduleOp.update(); - submoduleOp.update(); - - doVerify(); - RefUpdate ru = ruCapture.getValue(); - assertEquals(ru.getName(), targetBranchNameKey.get()); + doVerify(); + RefUpdate ru = ruCapture.getValue(); + assertEquals(ru.getName(), targetBranchNameKey.get()); + } } /** @@ -862,67 +873,70 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase { final List previousSubscriptions) throws Exception { expect(schemaFactory.open()).andReturn(schema); - final Repository realDb = createWorkRepository(); - final Git git = new Git(realDb); + try (Repository realDb = createWorkRepository()) { + // TODO(dborowitz): Use try/finally when this doesn't double-close the repo. + @SuppressWarnings("resource") + final Git git = new Git(realDb); - addRegularFileToIndex(".gitmodules", gitModulesFileContent, realDb); + addRegularFileToIndex(".gitmodules", gitModulesFileContent, realDb); - final RevCommit mergeTip = git.commit().setMessage("test").call(); + final RevCommit mergeTip = git.commit().setMessage("test").call(); - expect(urlProvider.get()).andReturn("http://localhost:8080").times(2); + expect(urlProvider.get()).andReturn("http://localhost:8080").times(2); - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - expect(subscriptions.bySuperProject(mergedBranch)).andReturn( - new ListResultSet<>(previousSubscriptions)); - - SortedSet existingProjects = new TreeSet<>(); - - for (SubmoduleSubscription extracted : extractedSubscriptions) { - existingProjects.add(extracted.getSubmodule().getParentKey()); - } - - for (int index = 0; index < extractedSubscriptions.size(); index++) { - expect(repoManager.list()).andReturn(existingProjects); - } - - final Set alreadySubscribeds = new HashSet<>(); - for (SubmoduleSubscription s : extractedSubscriptions) { - if (previousSubscriptions.contains(s)) { - alreadySubscribeds.add(s); - } - } - - final Set subscriptionsToRemove = - new HashSet<>(previousSubscriptions); - final List subscriptionsToInsert = - new ArrayList<>(extractedSubscriptions); - - subscriptionsToRemove.removeAll(subscriptionsToInsert); - subscriptionsToInsert.removeAll(alreadySubscribeds); - - if (!subscriptionsToRemove.isEmpty()) { expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - subscriptions.delete(subscriptionsToRemove); + expect(subscriptions.bySuperProject(mergedBranch)).andReturn( + new ListResultSet<>(previousSubscriptions)); + + SortedSet existingProjects = new TreeSet<>(); + + for (SubmoduleSubscription extracted : extractedSubscriptions) { + existingProjects.add(extracted.getSubmodule().getParentKey()); + } + + for (int index = 0; index < extractedSubscriptions.size(); index++) { + expect(repoManager.list()).andReturn(existingProjects); + } + + final Set alreadySubscribeds = new HashSet<>(); + for (SubmoduleSubscription s : extractedSubscriptions) { + if (previousSubscriptions.contains(s)) { + alreadySubscribeds.add(s); + } + } + + final Set subscriptionsToRemove = + new HashSet<>(previousSubscriptions); + final List subscriptionsToInsert = + new ArrayList<>(extractedSubscriptions); + + subscriptionsToRemove.removeAll(subscriptionsToInsert); + subscriptionsToInsert.removeAll(alreadySubscribeds); + + if (!subscriptionsToRemove.isEmpty()) { + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + subscriptions.delete(subscriptionsToRemove); + } + + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + subscriptions.insert(subscriptionsToInsert); + + expect(schema.submoduleSubscriptions()).andReturn(subscriptions); + expect(subscriptions.bySubmodule(mergedBranch)).andReturn( + new ListResultSet<>(new ArrayList())); + + schema.close(); + + doReplay(); + + final SubmoduleOp submoduleOp = + new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb), + urlProvider, schemaFactory, realDb, new Project(mergedBranch + .getParentKey()), new ArrayList(), null, null, + repoManager, null, null, null); + + submoduleOp.update(); } - - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - subscriptions.insert(subscriptionsToInsert); - - expect(schema.submoduleSubscriptions()).andReturn(subscriptions); - expect(subscriptions.bySubmodule(mergedBranch)).andReturn( - new ListResultSet<>(new ArrayList())); - - schema.close(); - - doReplay(); - - final SubmoduleOp submoduleOp = - new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb), - urlProvider, schemaFactory, realDb, new Project(mergedBranch - .getParentKey()), new ArrayList(), null, null, - repoManager, null, null, null); - - submoduleOp.update(); } /** diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index 53d9fb1c99..c41c4ecf0c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java @@ -43,7 +43,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { @After public void tearDownTestRepo() throws Exception { - walk.release(); + walk.close(); } @Test @@ -176,8 +176,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { private RevCommit writeCommit(String body, PersonIdent author) throws Exception { - ObjectInserter ins = testRepo.getRepository().newObjectInserter(); - try { + try (ObjectInserter ins = testRepo.getRepository().newObjectInserter()) { CommitBuilder cb = new CommitBuilder(); cb.setAuthor(author); cb.setCommitter(new PersonIdent(serverIdent, author.getWhen())); @@ -188,8 +187,6 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { RevCommit commit = walk.parseCommit(id); walk.parseBody(commit); return commit; - } finally { - ins.release(); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index db17f8059b..aea966a281 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -348,13 +348,10 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { hashtags.add("tag2"); update.setHashtags(hashtags); update.commit(); - RevWalk walk = new RevWalk(repo); - try { + try (RevWalk walk = new RevWalk(repo)) { RevCommit commit = walk.parseCommit(update.getRevision()); walk.parseBody(commit); assertTrue(commit.getFullMessage().endsWith("Hashtags: tag1,tag2\n")); - } finally { - walk.release(); } } @@ -433,8 +430,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update2.putApproval("Code-Review", (short) 2); update2.writeCommit(batch); - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { batch.commit(); bru.execute(rw, NullProgressMonitor.INSTANCE); @@ -464,7 +460,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { notesWithApprovals.close(); } finally { batch.close(); - rw.release(); } } @@ -506,11 +501,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertEquals(update1.getRefName(), cmds.get(0).getRefName()); assertEquals(update2.getRefName(), cmds.get(1).getRefName()); - RevWalk rw = new RevWalk(repo); - try { + try (RevWalk rw = new RevWalk(repo)) { bru.execute(rw, NullProgressMonitor.INSTANCE); - } finally { - rw.release(); } assertEquals(ReceiveCommand.Result.OK, cmds.get(0).getResult()); @@ -712,43 +704,44 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { ChangeNotes notes = newNotes(c); - RevWalk walk = new RevWalk(repo); - ArrayList notesInTree = - Lists.newArrayList(notes.getNoteMap().iterator()); - Note note = Iterables.getOnlyElement(notesInTree); + try (RevWalk walk = new RevWalk(repo)) { + ArrayList notesInTree = + Lists.newArrayList(notes.getNoteMap().iterator()); + Note note = Iterables.getOnlyElement(notesInTree); - byte[] bytes = - walk.getObjectReader().open( - note.getData(), Constants.OBJ_BLOB).getBytes(); - String noteString = new String(bytes, UTF_8); - assertEquals("Patch-set: 1\n" - + "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n" - + "File: file1\n" - + "\n" - + "1:1-2:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" - + "Author: Other Account <2@gerrit>\n" - + "UUID: uuid1\n" - + "Bytes: 9\n" - + "comment 1\n" - + "\n" - + "2:1-3:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" - + "Author: Other Account <2@gerrit>\n" - + "UUID: uuid2\n" - + "Bytes: 9\n" - + "comment 2\n" - + "\n" - + "File: file2\n" - + "\n" - + "3:1-4:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n" - + "Author: Other Account <2@gerrit>\n" - + "UUID: uuid3\n" - + "Bytes: 9\n" - + "comment 3\n" - + "\n", - noteString); + byte[] bytes = + walk.getObjectReader().open( + note.getData(), Constants.OBJ_BLOB).getBytes(); + String noteString = new String(bytes, UTF_8); + assertEquals("Patch-set: 1\n" + + "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n" + + "File: file1\n" + + "\n" + + "1:1-2:1\n" + + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + + "Author: Other Account <2@gerrit>\n" + + "UUID: uuid1\n" + + "Bytes: 9\n" + + "comment 1\n" + + "\n" + + "2:1-3:1\n" + + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + + "Author: Other Account <2@gerrit>\n" + + "UUID: uuid2\n" + + "Bytes: 9\n" + + "comment 2\n" + + "\n" + + "File: file2\n" + + "\n" + + "3:1-4:1\n" + + CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n" + + "Author: Other Account <2@gerrit>\n" + + "UUID: uuid3\n" + + "Bytes: 9\n" + + "comment 3\n" + + "\n", + noteString); + } } @Test @@ -782,34 +775,35 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { ChangeNotes notes = newNotes(c); - RevWalk walk = new RevWalk(repo); - ArrayList notesInTree = - Lists.newArrayList(notes.getNoteMap().iterator()); - Note note = Iterables.getOnlyElement(notesInTree); + try (RevWalk walk = new RevWalk(repo)) { + ArrayList notesInTree = + Lists.newArrayList(notes.getNoteMap().iterator()); + Note note = Iterables.getOnlyElement(notesInTree); - byte[] bytes = - walk.getObjectReader().open( - note.getData(), Constants.OBJ_BLOB).getBytes(); - String noteString = new String(bytes, UTF_8); - assertEquals("Base-for-patch-set: 1\n" - + "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n" - + "File: file1\n" - + "\n" - + "1:1-2:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" - + "Author: Other Account <2@gerrit>\n" - + "UUID: uuid1\n" - + "Bytes: 9\n" - + "comment 1\n" - + "\n" - + "2:1-3:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" - + "Author: Other Account <2@gerrit>\n" - + "UUID: uuid2\n" - + "Bytes: 9\n" - + "comment 2\n" - + "\n", - noteString); + byte[] bytes = + walk.getObjectReader().open( + note.getData(), Constants.OBJ_BLOB).getBytes(); + String noteString = new String(bytes, UTF_8); + assertEquals("Base-for-patch-set: 1\n" + + "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n" + + "File: file1\n" + + "\n" + + "1:1-2:1\n" + + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + + "Author: Other Account <2@gerrit>\n" + + "UUID: uuid1\n" + + "Bytes: 9\n" + + "comment 1\n" + + "\n" + + "2:1-3:1\n" + + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + + "Author: Other Account <2@gerrit>\n" + + "UUID: uuid2\n" + + "Bytes: 9\n" + + "comment 2\n" + + "\n", + noteString); + } } @Test diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java index 86f9702f83..328509ae7c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java @@ -242,13 +242,10 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { if (id instanceof RevCommit) { return (RevCommit) id; } - RevWalk walk = new RevWalk(repo); - try { + try (RevWalk walk = new RevWalk(repo)) { RevCommit commit = walk.parseCommit(id); walk.parseBody(commit); return commit; - } finally { - walk.release(); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/CommitMsgHookTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/CommitMsgHookTest.java index 17fe068581..9c8e86aa9d 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/CommitMsgHookTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/CommitMsgHookTest.java @@ -423,21 +423,17 @@ public class CommitMsgHookTest extends HookTestCase { } private DirCacheEntry file(final String name) throws IOException { - final ObjectInserter oi = repository.newObjectInserter(); - try { + try (ObjectInserter oi = repository.newObjectInserter()) { final DirCacheEntry e = new DirCacheEntry(name); e.setFileMode(FileMode.REGULAR_FILE); e.setObjectId(oi.insert(Constants.OBJ_BLOB, Constants.encode(name))); oi.flush(); return e; - } finally { - oi.release(); } } private void setHEAD() throws Exception { - final ObjectInserter oi = repository.newObjectInserter(); - try { + try (ObjectInserter oi = repository.newObjectInserter()) { final CommitBuilder commit = new CommitBuilder(); commit.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {})); commit.setAuthor(author); @@ -456,8 +452,6 @@ public class CommitMsgHookTest extends HookTestCase { default: fail(Constants.HEAD + " did not change: " + ref.getResult()); } - } finally { - oi.release(); } } } diff --git a/plugins/reviewnotes b/plugins/reviewnotes index ba824869c6..603b7b3885 160000 --- a/plugins/reviewnotes +++ b/plugins/reviewnotes @@ -1 +1 @@ -Subproject commit ba824869c6b24348647f26e04cf80e1ae82266ec +Subproject commit 603b7b3885a1d2953ca891f58d2a6095e72e5313