opendev/gerrit
Code Issues Proposed changes

Remove cached change status from PatchSetApproval

Browse Source 
With the SQL search system removed, the caching of change
status in these objects is no longer necessary or even used.

Change-Id: I49798f477c10e49d17c1ba1a7e438fbf6c16c392
This commit is contained in:
Shawn Pearce
2013-12-04 15:41:42 -08:00
parent 3c335ee5e7
commit 169445c35a
11 changed files with 31 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
View File

@@ -129,20 +129,11 @@ public final class PatchSetApproval {
@Column(id = 3)
protected Timestamp granted;
/** <i>Cached copy of Change.open.</i> */
@Column(id = 4)
protected boolean changeOpen;
/** <i>Cached copy of Change.sortKey</i>; 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();
}

21
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
View File

@@ -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<PatchSetApproval> 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<PatchSetApproval> 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);
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
View File

@@ -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<ChangeResource, AbandonInput>,
private final ChangeHooks hooks;
private final AbandonedSender.Factory abandonedSenderFactory;
private final Provider<ReviewDb> dbProvider;
private final Provider<ApprovalsUtil> approvals;
private final ChangeJson json;
private final ChangeIndexer indexer;
@@ -59,13 +57,11 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
Abandon(ChangeHooks hooks,
AbandonedSender.Factory abandonedSenderFactory,
Provider<ReviewDb> dbProvider,
Provider<ApprovalsUtil> 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<ChangeResource, AbandonInput>,
}
message = newMessage(input, caller, change);
db.changeMessages().insert(Collections.singleton(message));
approvals.get().syncChangeStatus(change);
db.commit();
} finally {
db.rollback();

4
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -394,7 +394,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
} 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<RevisionResource, ReviewInput>
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<RevisionResource, ReviewInput>
.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<RevisionResource, ReviewInput>
PatchSetApproval c = i.next();
c.setValue((short) 0);
c.setGranted(timestamp);
c.cache(change);
i.remove();
upd.add(c);
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -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<ChangeResource, AddReviewer
PatchSet.Id patchSetId, Account.Id reviewerId) {
LabelId id =
Iterables.getLast(ctl.getLabelTypes().getLabelTypes()).getLabelId();
PatchSetApproval dummyApproval = new PatchSetApproval(
return new PatchSetApproval(
new PatchSetApproval.Key(patchSetId, reviewerId, id), (short) 0,
TimeUtil.nowTs());
dummyApproval.cache(ctl.getChange());
return dummyApproval;
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
View File

@@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
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;
@@ -52,7 +51,6 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
private final ChangeHooks hooks;
private final RestoredSender.Factory restoredSenderFactory;
private final Provider<ReviewDb> dbProvider;
private final Provider<ApprovalsUtil> approvals;
private final ChangeJson json;
private final ChangeIndexer indexer;
@@ -60,13 +58,11 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
Restore(ChangeHooks hooks,
RestoredSender.Factory restoredSenderFactory,
Provider<ReviewDb> dbProvider,
Provider<ApprovalsUtil> 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<ChangeResource, RestoreInput>,
}
message = newMessage(input, caller, change);
db.changeMessages().insert(Collections.singleton(message));
approvals.get().syncChangeStatus(change);
db.commit();
} finally {
db.rollback();

4
gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
View File

@@ -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,

5
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -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);
}

2
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -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())) {

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@@ -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<Schema_88> C = Schema_88.class;
public static final Class<Schema_89> C = Schema_89.class;
public static class Module extends AbstractModule {
@Override

25
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_89.java Normal file
View File

@@ -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<Schema_88> prior) {
super(prior);
}
}