opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'change-kind-in-revision-info'

Browse Source 
* changes:
  ChangeJson: Include change kind in revision info
  ChangeKindCache: Add method to get kind with already opened repo
  ChangeJson: Only open the repository once when creating revision info
  H2CacheImpl: Ignore errors due to renamed classes
  ChangeKind: Move to extension API
This commit is contained in:
Dave Borowitz
2016-07-01 13:57:10 +00:00
committed by Gerrit Code Review
parent d4c01e0bda 4de41115b1
commit 97efc2c000
11 changed files with 116 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
Documentation/rest-api-changes.txt
View File

@@ -353,6 +353,7 @@ default. Optional fields are:
"current_revision": "184ebe53805e102605d11f6b143486d15c23a09c",
"revisions": {
"184ebe53805e102605d11f6b143486d15c23a09c": {
"kind": "REWORK",
"_number": 1,
"ref": "refs/changes/97/97/1",
"fetch": {
@@ -936,6 +937,7 @@ is included.
"current_revision": "27cc4558b5a3d3387dd11ee2df7a117e7e581822",
"revisions": {
"27cc4558b5a3d3387dd11ee2df7a117e7e581822": {
"kind": "REWORK",
"_number": 2,
"ref": "refs/changes/99/4799/2",
"fetch": {
@@ -1314,6 +1316,7 @@ link:#current-commit[\{CURRENT_COMMIT\}] set.
"current_revision": "9adb9f4c7b40eeee0646e235de818d09164d7379",
"revisions": {
"9adb9f4c7b40eeee0646e235de818d09164d7379": {
"kind": "REWORK",
"_number": 1,
"created": "2015-05-01 15:39:57.979000000",
"uploader": {
@@ -1432,6 +1435,7 @@ link:#current-commit[\{CURRENT_COMMIT\}] set.
"current_revision": "1bd7c12a38854a2c6de426feec28800623f492c4",
"revisions": {
"1bd7c12a38854a2c6de426feec28800623f492c4": {
"kind": "REWORK",
"_number": 1,
"created": "2015-05-01 15:39:57.979000000",
"uploader": {
@@ -2659,12 +2663,14 @@ for the current patch set.
"current_revision": "674ac754f91e64a0efb8087e59a176484bd534d1",
"revisions": {
"674ac754f91e64a0efb8087e59a176484bd534d1": {
"_number": 2,
"ref": "refs/changes/65/3965/2",
"fetch": {
"http": {
"url": "http://gerrit/myProject",
"ref": "refs/changes/65/3965/2"
"kind": "REWORK",
"_number": 2,
"ref": "refs/changes/65/3965/2",
"fetch": {
"http": {
"url": "http://gerrit/myProject",
"ref": "refs/changes/65/3965/2"
}
}
}
}
@@ -2863,6 +2869,7 @@ is included.
"current_revision": "27cc4558b5a3d3387dd11ee2df7a117e7e581822",
"revisions": {
"27cc4558b5a3d3387dd11ee2df7a117e7e581822": {
"kind": "REWORK",
"_number": 2,
"ref": "refs/changes/99/4799/2",
"fetch": {
@@ -4974,6 +4981,8 @@ link:#list-changes[Query Changes].
|===========================
|Field Name ||Description
|`draft` |not set if `false`|Whether the patch set is a draft.
|`kind` ||The change kind. Valid values are `REWORK`, `TRIVIAL_REBASE`,
`MERGE_FIRST_PARENT_UPDATE`, `NO_CODE_CHANGE`, and `NO_CHANGE`.
|`_number` ||The patch set number.
|`created` ||
The link:rest-api.html#timestamp[timestamp] of when the patch set was

19
gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java vendored
View File

@@ -14,11 +14,13 @@
package com.google.gerrit.server.cache.h2;
import com.google.common.base.Throwables;
import com.google.common.cache.AbstractLoadingCache;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.CacheStats;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet;
import com.google.common.hash.BloomFilter;
import com.google.common.hash.Funnel;
import com.google.common.hash.Funnels;
@@ -81,6 +83,9 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements
PersistentCache {
private static final Logger log = LoggerFactory.getLogger(H2CacheImpl.class);
private static final ImmutableSet<String> OLD_CLASS_NAMES = ImmutableSet.of(
"com.google.gerrit.server.change.ChangeKind");
private final Executor executor;
private final SqlStore<K, V> store;
private final TypeLiteral<K> keyType;
@@ -472,7 +477,9 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements
c.get.clearParameters();
}
} catch (SQLException e) {
log.warn("Cannot read cache " + url + " for " + key, e);
if (!isOldClassNameError(e)) {
log.warn("Cannot read cache " + url + " for " + key, e);
}
c = close(c);
return null;
} finally {
@@ -480,6 +487,16 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements
}
}
private static boolean isOldClassNameError(Throwable t) {
for (Throwable c : Throwables.getCausalChain(t)) {
if (c instanceof ClassNotFoundException
&& OLD_CLASS_NAMES.contains(c.getMessage())) {
return true;
}
}
return false;
}
private boolean expired(Timestamp created) {
if (expireAfterWrite == 0) {
return false;

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKind.java → gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeKind.java
View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
package com.google.gerrit.extensions.client;
/** Operation performed by a change relative to its parent. */
public enum ChangeKind {

3
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java
View File

@@ -14,12 +14,15 @@
package com.google.gerrit.extensions.common;
import com.google.gerrit.extensions.client.ChangeKind;
import java.sql.Timestamp;
import java.util.Map;
public class RevisionInfo {
public transient boolean isCurrent;
public Boolean draft;
public ChangeKind kind;
public int _number;
public Timestamp created;
public AccountInfo uploader;

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

@@ -21,11 +21,11 @@ import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Table;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeKind;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;

39
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -163,6 +163,7 @@ public class ChangeJson {
private final GpgApiAdapter gpgApi;
private final ChangeNotes.Factory notesFactory;
private final ChangeResource.Factory changeResourceFactory;
private final ChangeKindCache changeKindCache;
private AccountLoader accountLoader;
private Map<Change.Id, List<SubmitRecord>> submitRecords;
@@ -190,6 +191,7 @@ public class ChangeJson {
GpgApiAdapter gpgApi,
ChangeNotes.Factory notesFactory,
ChangeResource.Factory changeResourceFactory,
ChangeKindCache changeKindCache,
@Assisted Set<ListChangesOption> options) {
this.db = db;
this.labelNormalizer = ln;
@@ -211,6 +213,7 @@ public class ChangeJson {
this.gpgApi = gpgApi;
this.notesFactory = notesFactory;
this.changeResourceFactory = changeResourceFactory;
this.changeKindCache = changeKindCache;
this.options = options.isEmpty()
? EnumSet.noneOf(ListChangesOption.class)
: EnumSet.copyOf(options);
@@ -472,7 +475,7 @@ public class ChangeJson {
finish(out);
if (needRevisions) {
out.revisions = revisions(ctl, src);
out.revisions = revisions(ctl, cd, src);
if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) {
@@ -887,18 +890,21 @@ public class ChangeJson {
.toSortedList(AccountInfoComparator.ORDER_NULLS_FIRST);
}
private Map<String, RevisionInfo> revisions(ChangeControl ctl,
private Map<String, RevisionInfo> revisions(ChangeControl ctl, ChangeData cd,
Map<PatchSet.Id, PatchSet> map) throws PatchListNotAvailableException,
GpgException, OrmException, IOException {
Map<String, RevisionInfo> res = new LinkedHashMap<>();
for (PatchSet in : map.values()) {
if ((has(ALL_REVISIONS)
|| in.getId().equals(ctl.getChange().currentPatchSetId()))
&& ctl.isPatchVisible(in, db.get())) {
res.put(in.getRevision().get(), toRevisionInfo(ctl, in));
try (Repository repo =
repoManager.openRepository(ctl.getProject().getNameKey())) {
for (PatchSet in : map.values()) {
if ((has(ALL_REVISIONS)
|| in.getId().equals(ctl.getChange().currentPatchSetId()))
&& ctl.isPatchVisible(in, db.get())) {
res.put(in.getRevision().get(), toRevisionInfo(ctl, cd, in, repo));
}
}
return res;
}
return res;
}
private Map<PatchSet.Id, PatchSet> loadPatchSets(ChangeData cd,
@@ -933,12 +939,17 @@ public class ChangeJson {
throws PatchListNotAvailableException, GpgException, OrmException,
IOException {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
RevisionInfo rev = toRevisionInfo(ctl, in);
accountLoader.fill();
return rev;
try (Repository repo =
repoManager.openRepository(ctl.getProject().getNameKey())) {
RevisionInfo rev = toRevisionInfo(
ctl, changeDataFactory.create(db.get(), ctl), in, repo);
accountLoader.fill();
return rev;
}
}
private RevisionInfo toRevisionInfo(ChangeControl ctl, PatchSet in)
private RevisionInfo toRevisionInfo(ChangeControl ctl, ChangeData cd,
PatchSet in, Repository repo)
throws PatchListNotAvailableException, GpgException, OrmException,
IOException {
Change c = ctl.getChange();
@@ -950,14 +961,14 @@ public class ChangeJson {
out.uploader = accountLoader.get(in.getUploader());
out.draft = in.isDraft() ? true : null;
out.fetch = makeFetchMap(ctl, in);
out.kind = changeKindCache.getChangeKind(repo, cd, in);
boolean setCommit = has(ALL_COMMITS)
|| (out.isCurrent && has(CURRENT_COMMIT));
boolean addFooters = out.isCurrent && has(COMMIT_FOOTERS);
if (setCommit || addFooters) {
Project.NameKey project = c.getProject();
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
try (RevWalk rw = new RevWalk(repo)) {
String rev = in.getRevision().get();
RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
rw.parseBody(commit);

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

@@ -14,10 +14,12 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.client.ChangeKind;
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.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -33,4 +35,6 @@ public interface ChangeKindCache {
ObjectId prior, ObjectId next);
ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch);
ChangeKind getChangeKind(Repository repo, ChangeData cd, PatchSet patch);
}

57
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
View File

@@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
import com.google.common.cache.Weigher;
import com.google.common.collect.FluentIterable;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -117,6 +118,12 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
return getChangeKindInternal(this, db, change, patch, changeDataFactory,
projectCache, repoManager);
}
@Override
public ChangeKind getChangeKind(Repository repo, ChangeData cd,
PatchSet patch) {
return getChangeKindInternal(this, repo, cd, patch, projectCache);
}
}
public static class Key implements Serializable {
@@ -332,23 +339,25 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
projectCache, repoManager);
}
@Override
public ChangeKind getChangeKind(Repository repo, ChangeData cd,
PatchSet patch) {
return getChangeKindInternal(this, repo, cd, patch, projectCache);
}
private static ChangeKind getChangeKindInternal(
ChangeKindCache cache,
ReviewDb db,
Change change,
Repository repo,
ChangeData change,
PatchSet patch,
ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) {
// TODO - dborowitz: add NEW_CHANGE type for default.
ProjectCache projectCache) {
ChangeKind kind = ChangeKind.REWORK;
// Trivial case: if we're on the first patch, we don't need to open
// Trivial case: if we're on the first patch, we don't need to use
// the repository.
if (patch.getId().get() > 1) {
try (Repository repo = repoManager.openRepository(change.getProject())) {
ProjectState projectState = projectCache.checkedGet(change.getProject());
ChangeData cd = changeDataFactory.create(db, change);
Collection<PatchSet> patchSetCollection = cd.patchSets();
try {
ProjectState projectState = projectCache.checkedGet(change.project());
Collection<PatchSet> patchSetCollection = change.patchSets();
PatchSet priorPs = patch;
for (PatchSet ps : patchSetCollection) {
if (ps.getId().get() < patch.getId().get() &&
@@ -370,6 +379,32 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
ObjectId.fromString(patch.getRevision().get()));
}
} catch (IOException | OrmException e) {
// Do nothing; assume we have a complex change
log.warn("Unable to get change kind for patchSet " + patch.getPatchSetId() +
"of change " + change.getId(), e);
}
}
return kind;
}
private static ChangeKind getChangeKindInternal(
ChangeKindCache cache,
ReviewDb db,
Change change,
PatchSet patch,
ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) {
// TODO - dborowitz: add NEW_CHANGE type for default.
ChangeKind kind = ChangeKind.REWORK;
// Trivial case: if we're on the first patch, we don't need to open
// the repository.
if (patch.getId().get() > 1) {
try (Repository repo = repoManager.openRepository(change.getProject())) {
kind = getChangeKindInternal(cache, repo,
changeDataFactory.create(db, change), patch,
projectCache);
} catch (IOException e) {
// Do nothing; assume we have a complex change
log.warn("Unable to get change kind for patchSet " + patch.getPatchSetId() +
"of change " + change.getChangeId(), e);

2
gerrit-server/src/main/java/com/google/gerrit/server/data/PatchSetAttribute.java
View File

@@ -14,7 +14,7 @@
package com.google.gerrit.server.data;
import com.google.gerrit.server.change.ChangeKind;
import com.google.gerrit.extensions.client.ChangeKind;
import java.util.List;

2
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.base.Optional;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -30,7 +31,6 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeKind;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.git.BatchUpdate;

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

@@ -22,6 +22,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -35,7 +36,6 @@ import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeKind;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;