opendev/gerrit
Code Issues Proposed changes

Remove public bumpRowVersionNoLastUpdatedOn from ChangeUtil

Browse Source 
This public method was used in exactly one remaining place, to
invalidate a change's ETag when the mergeability cache is refreshed
after a branch update. All other usages of this functionality are via
BatchUpdate with ctx.bumpLastUpdatedOn(false), so we no longer need
a public, centrally located method to do this.

Get rid of the public method and move it to a private method in
Mergeable. We still haven't solved the ETag problem noted in that
code, so we need to continue using the existing implementation. While
we're there, make sure this works when NoteDb is enabled.

Change-Id: I0e1b92e743f4d3132ec7c48dd784ae31aa1ae78a
This commit is contained in:
Dave Borowitz
2016-04-29 11:20:38 -04:00
parent 1e20979bdb
commit a1ed3a1084
2 changed files with 17 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server;
import com.google.common.base.Function;
import com.google.common.collect.Ordering;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.util.IdGenerator;
@@ -28,7 +27,6 @@ import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.Collections;
import java.util.Map;
@Singleton
@@ -77,15 +75,6 @@ public class ChangeUtil {
return u + '_' + l;
}
public static void bumpRowVersionNotLastUpdatedOn(Change.Id id, ReviewDb db)
throws OrmException {
// Empty update of Change to bump rowVersion, changing its ETag.
Change c = db.changes().get(id);
if (c != null) {
db.changes().update(Collections.singleton(c));
}
}
public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs,
PatchSet.Id id) {
PatchSet.Id next = nextPatchSetId(id);

20
gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
View File

@@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.git.BranchOrderSection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -33,6 +32,7 @@ import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -47,6 +47,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
@@ -175,10 +176,23 @@ public class Mergeable implements RestReadView<RevisionResource> {
final boolean mergeable =
cache.get(commit, ref, type, strategy, change.getDest(), git);
if (!Objects.equals(mergeable, old)) {
// TODO(dborowitz): Include cache info in ETag somehow instead.
ChangeUtil.bumpRowVersionNotLastUpdatedOn(change.getId(), db.get());
invalidateETag(change.getId(), db.get());
indexer.index(db.get(), change);
}
return mergeable;
}
private static void invalidateETag(Change.Id id, ReviewDb db)
throws OrmException {
// Empty update of Change to bump rowVersion, changing its ETag.
// TODO(dborowitz): Include cache info in ETag somehow instead.
if (db instanceof DisabledChangesReviewDbWrapper) {
db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
}
Change c = db.changes().get(id);
if (c != null) {
db.changes().update(Collections.singleton(c));
}
}
}