From 169445c35a30995274645ce5e7ee0fdcb941567d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 4 Dec 2013 15:41:42 -0800 Subject: [PATCH] Remove cached change status from PatchSetApproval With the SQL search system removed, the caching of change status in these objects is no longer necessary or even used. Change-Id: I49798f477c10e49d17c1ba1a7e438fbf6c16c392 --- .../reviewdb/client/PatchSetApproval.java | 15 ----------- .../google/gerrit/server/ApprovalsUtil.java | 21 +++------------- .../google/gerrit/server/change/Abandon.java | 5 ---- .../gerrit/server/change/PostReview.java | 4 --- .../gerrit/server/change/PostReviewers.java | 5 +--- .../google/gerrit/server/change/Restore.java | 5 ---- .../gerrit/server/git/LabelNormalizer.java | 4 +-- .../com/google/gerrit/server/git/MergeOp.java | 5 ---- .../gerrit/server/git/ReceiveCommits.java | 2 -- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_89.java | 25 +++++++++++++++++++ 11 files changed, 31 insertions(+), 62 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_89.java diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java index 2c0f1f43ec..b97865d245 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java @@ -129,20 +129,11 @@ public final class PatchSetApproval { @Column(id = 3) protected Timestamp granted; - /** Cached copy of Change.open. */ - @Column(id = 4) - protected boolean changeOpen; - - /** Cached copy of Change.sortKey; only if {@link #changeOpen} = false */ - @Column(id = 5, length = 16, notNull = false) - protected String changeSortKey; - protected PatchSetApproval() { } public PatchSetApproval(PatchSetApproval.Key k, short v, Timestamp ts) { key = k; - changeOpen = true; setValue(v); setGranted(ts); } @@ -150,7 +141,6 @@ public final class PatchSetApproval { public PatchSetApproval(final PatchSet.Id psId, final PatchSetApproval src) { key = new PatchSetApproval.Key(psId, src.getAccountId(), src.getLabelId()); - changeOpen = true; value = src.getValue(); granted = src.granted; } @@ -187,11 +177,6 @@ public final class PatchSetApproval { granted = ts; } - public void cache(final Change c) { - changeOpen = c.open; - changeSortKey = c.sortKey; - } - public String getLabel() { return getLabelId().get(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java index 03f73f437f..029ed683ea 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java @@ -48,23 +48,10 @@ public class ApprovalsUtil { private final ReviewDb db; @Inject - public ApprovalsUtil(ReviewDb db) { + ApprovalsUtil(ReviewDb db) { this.db = db; } - /** - * Resync the changeOpen status which is cached in the approvals table for - * performance reasons - */ - public void syncChangeStatus(final Change change) throws OrmException { - final List approvals = - db.patchSetApprovals().byChange(change.getId()).toList(); - for (PatchSetApproval a : approvals) { - a.cache(change); - } - db.patchSetApprovals().update(approvals); - } - /** * Copy min/max scores from one patch set to another. * @@ -135,11 +122,9 @@ public class ApprovalsUtil { List cells = Lists.newArrayListWithCapacity(need.size()); LabelId labelId = Iterables.getLast(allTypes).getLabelId(); for (Account.Id account : need) { - PatchSetApproval psa = new PatchSetApproval( + cells.add(new PatchSetApproval( new PatchSetApproval.Key(ps.getId(), account, labelId), - (short) 0, TimeUtil.nowTs()); - psa.cache(change); - cells.add(psa); + (short) 0, TimeUtil.nowTs())); } db.patchSetApprovals().insert(cells); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 8c523e044f..6481d5ffa6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -25,7 +25,6 @@ import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.change.ChangeJson.ChangeInfo; @@ -51,7 +50,6 @@ public class Abandon implements RestModifyView, private final ChangeHooks hooks; private final AbandonedSender.Factory abandonedSenderFactory; private final Provider dbProvider; - private final Provider approvals; private final ChangeJson json; private final ChangeIndexer indexer; @@ -59,13 +57,11 @@ public class Abandon implements RestModifyView, Abandon(ChangeHooks hooks, AbandonedSender.Factory abandonedSenderFactory, Provider dbProvider, - Provider approvals, ChangeJson json, ChangeIndexer indexer) { this.hooks = hooks; this.abandonedSenderFactory = abandonedSenderFactory; this.dbProvider = dbProvider; - this.approvals = approvals; this.json = json; this.indexer = indexer; } @@ -106,7 +102,6 @@ public class Abandon implements RestModifyView, } message = newMessage(input, caller, change); db.changeMessages().insert(Collections.singleton(message)); - approvals.get().syncChangeStatus(change); db.commit(); } finally { db.rollback(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index e3578ba3ba..d54d64abf1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -394,7 +394,6 @@ public class PostReview implements RestModifyView } else if (c != null && c.getValue() != ent.getValue()) { c.setValue(ent.getValue()); c.setGranted(timestamp); - c.cache(change); upd.add(c); labelDelta.add(format(normName, c.getValue())); categories.put(normName, c.getValue()); @@ -407,7 +406,6 @@ public class PostReview implements RestModifyView lt.getLabelId()), ent.getValue(), TimeUtil.nowTs()); c.setGranted(timestamp); - c.cache(change); ins.add(c); labelDelta.add(format(normName, c.getValue())); categories.put(normName, c.getValue()); @@ -436,7 +434,6 @@ public class PostReview implements RestModifyView .getLabelId()), (short) 0, TimeUtil.nowTs()); c.setGranted(timestamp); - c.cache(change); ins.add(c); } else { // Pick a random label that is about to be deleted and keep it. @@ -444,7 +441,6 @@ public class PostReview implements RestModifyView PatchSetApproval c = i.next(); c.setValue((short) 0); c.setGranted(timestamp); - c.cache(change); i.remove(); upd.add(c); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 306e5a8dc1..5389a105b4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.change; -import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -302,10 +301,8 @@ public class PostReviewers implements RestModifyView, private final ChangeHooks hooks; private final RestoredSender.Factory restoredSenderFactory; private final Provider dbProvider; - private final Provider approvals; private final ChangeJson json; private final ChangeIndexer indexer; @@ -60,13 +58,11 @@ public class Restore implements RestModifyView, Restore(ChangeHooks hooks, RestoredSender.Factory restoredSenderFactory, Provider dbProvider, - Provider approvals, ChangeJson json, ChangeIndexer indexer) { this.hooks = hooks; this.restoredSenderFactory = restoredSenderFactory; this.dbProvider = dbProvider; - this.approvals = approvals; this.json = json; this.indexer = indexer; } @@ -107,7 +103,6 @@ public class Restore implements RestModifyView, } message = newMessage(input, caller, change); db.changeMessages().insert(Collections.singleton(message)); - approvals.get().syncChangeStatus(change); db.commit(); } finally { db.rollback(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java index 3f099168b0..f410e6e2e5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java @@ -106,9 +106,7 @@ public class LabelNormalizer { } private PatchSetApproval copy(PatchSetApproval src, ChangeControl ctl) { - PatchSetApproval dest = new PatchSetApproval(src.getPatchSetId(), src); - dest.cache(ctl.getChange()); - return dest; + return new PatchSetApproval(src.getPatchSetId(), src); } private PermissionRange getRange(ChangeControl ctl, LabelType lt, 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 97b7d8b0dc..09355b411a 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 @@ -15,12 +15,10 @@ package com.google.gerrit.server.git; import static com.google.gerrit.server.git.MergeUtil.getSubmitter; - import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; - import static org.eclipse.jgit.lib.RefDatabase.ALL; import com.google.common.base.Objects; @@ -44,7 +42,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project.SubmitType; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; @@ -899,7 +896,6 @@ public class MergeOp { submitter = a; } } - a.cache(c); } db.patchSetApprovals().update(approvals); db.patchSetApprovals().deleteKeys(toDelete); @@ -1149,7 +1145,6 @@ public class MergeOp { change.currentPatchSetId()); msg.setMessage("Project was deleted."); db.changeMessages().insert(Collections.singleton(msg)); - new ApprovalsUtil(db).syncChangeStatus(change); db.commit(); indexer.index(change); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 825749246a..e717d6125e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -2261,8 +2261,6 @@ public class ReceiveCommits { change.setStatus(Change.Status.MERGED); ChangeUtil.updated(change); - approvalsUtil.syncChangeStatus(change); - final StringBuilder msgBuf = new StringBuilder(); msgBuf.append("Change has been successfully pushed"); if (!mergedIntoRef.equals(change.getDest().get())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 145f22f079..c3edbfc872 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_88.class; + public static final Class C = Schema_89.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_89.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_89.java new file mode 100644 index 0000000000..6384375b1c --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_89.java @@ -0,0 +1,25 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.schema; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +public class Schema_89 extends SchemaVersion { + @Inject + Schema_89(Provider prior) { + super(prior); + } +}