From d3743450a2981a3cada725e1af54f229bdc1738d Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 28 Oct 2011 15:57:04 -0700 Subject: [PATCH] Avoid opening extra ReviewDb connection in PatchSetInfoFactory Every caller already had a ReviewDb "handy". Rather than making a new connection, use the existing one from the caller to read records about the patch set from the database. Change-Id: I68de1aa293f81b4318d681851706556bb73a5fd7 --- .../rpc/changedetail/PatchSetDetailFactory.java | 2 +- .../changedetail/PatchSetPublishDetailFactory.java | 2 +- .../java/com/google/gerrit/rules/StoredValues.java | 2 +- .../java/com/google/gerrit/server/git/MergeOp.java | 2 +- .../com/google/gerrit/server/mail/ChangeEmail.java | 2 +- .../gerrit/server/patch/PatchSetInfoFactory.java | 14 ++------------ .../gerrit/server/patch/PublishComments.java | 2 +- 7 files changed, 8 insertions(+), 18 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java index 78f4ded474..9ef715ff66 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java @@ -138,7 +138,7 @@ class PatchSetDetailFactory extends Handler { detail = new PatchSetDetail(); detail.setPatchSet(patchSet); - detail.setInfo(infoFactory.get(psIdNew)); + detail.setInfo(infoFactory.get(db, psIdNew)); detail.setPatches(patches); final CurrentUser user = control.getCurrentUser(); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java index 8ec1549b8c..32a58d0148 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java @@ -83,7 +83,7 @@ final class PatchSetPublishDetailFactory extends Handler final Change.Id changeId = patchSetId.getParentKey(); final ChangeControl control = changeControlFactory.validateFor(changeId); change = control.getChange(); - patchSetInfo = infoFactory.get(patchSetId); + patchSetInfo = infoFactory.get(db, patchSetId); drafts = db.patchComments().draftByPatchSetAuthor(patchSetId, user.getAccountId()).toList(); aic.want(change.getOwner()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java index cdcbb41fd6..de85742ebc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java +++ b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java @@ -51,7 +51,7 @@ public final class StoredValues { PatchSetInfoFactory patchInfoFactory = env.getInjector().getInstance(PatchSetInfoFactory.class); try { - return patchInfoFactory.get(patchSetId); + return patchInfoFactory.get(REVIEW_DB.get(engine), patchSetId); } catch (PatchSetInfoNotAvailableException e) { throw new SystemException(e.getMessage()); } 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 fd6f94b6c4..9bfef55dd6 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 @@ -1254,7 +1254,7 @@ public class MergeOp { // Go back to the patch set that was actually merged. // try { - c.setCurrentPatchSet(patchSetInfoFactory.get(merged)); + c.setCurrentPatchSet(patchSetInfoFactory.get(schema, merged)); } catch (PatchSetInfoNotAvailableException e1) { log.error("Cannot read merged patch set " + merged, e1); } 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 508408843a..3ef747c97e 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 @@ -136,7 +136,7 @@ public abstract class ChangeEmail extends OutgoingEmail { if (patchSet != null && patchSetInfo == null) { try { - patchSetInfo = args.patchSetInfoFactory.get(patchSet.getId()); + patchSetInfo = args.patchSetInfoFactory.get(args.db.get(), patchSet.getId()); } catch (PatchSetInfoNotAvailableException err) { patchSetInfo = null; } 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 16a958224a..0c70eacb8d 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 @@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.UserIdentity; import com.google.gerrit.server.account.AccountByEmailCache; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gwtorm.client.OrmException; -import com.google.gwtorm.client.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -49,15 +48,12 @@ import java.util.Set; @Singleton public class PatchSetInfoFactory { private final GitRepositoryManager repoManager; - private final SchemaFactory schemaFactory; private final AccountByEmailCache byEmailCache; @Inject public PatchSetInfoFactory(final GitRepositoryManager grm, - final SchemaFactory schemaFactory, final AccountByEmailCache byEmailCache) { this.repoManager = grm; - this.schemaFactory = schemaFactory; this.byEmailCache = byEmailCache; } @@ -68,16 +64,13 @@ public class PatchSetInfoFactory { info.setAuthor(toUserIdentity(src.getAuthorIdent())); info.setCommitter(toUserIdentity(src.getCommitterIdent())); info.setRevId(src.getName()); - return info; } - public PatchSetInfo get(PatchSet.Id patchSetId) - throws PatchSetInfoNotAvailableException { - ReviewDb db = null; + public PatchSetInfo get(ReviewDb db, PatchSet.Id patchSetId) + throws PatchSetInfoNotAvailableException { Repository repo = null; try { - db = schemaFactory.open(); final PatchSet patchSet = db.patchSets().get(patchSetId); final Change change = db.changes().get(patchSet.getId().getParentKey()); final Project.NameKey projectKey = change.getProject(); @@ -97,9 +90,6 @@ public class PatchSetInfoFactory { } catch (IOException e) { throw new PatchSetInfoNotAvailableException(e); } finally { - if (db != null) { - db.close(); - } if (repo != null) { repo.close(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java index 34194af058..838c31fdfe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java @@ -288,7 +288,7 @@ public class PublishComments implements Callable { if (message != null) { final CommentSender cm = commentSenderFactory.create(change); cm.setFrom(user.getAccountId()); - cm.setPatchSet(patchSet, patchSetInfoFactory.get(patchSetId)); + cm.setPatchSet(patchSet, patchSetInfoFactory.get(db, patchSetId)); cm.setChangeMessage(message); cm.setPatchLineComments(drafts); cm.send();