Merge "Ignore triple clicks for other gr-diff-highlight instances"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index fd275b8..e6492cf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -22,25 +22,20 @@
import com.google.common.base.Stopwatch;
import com.google.common.collect.ComparisonChain;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
-import com.google.common.collect.Sets;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.MultiProgressMonitor;
import com.google.gerrit.server.git.MultiProgressMonitor.Task;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.index.SiteIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.patch.AutoMerger;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.SchemaFactory;
@@ -49,33 +44,24 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
-import org.eclipse.jgit.diff.DiffEntry;
-import org.eclipse.jgit.diff.DiffFormatter;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
-import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
-import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.util.io.DisabledOutputStream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -89,8 +75,6 @@
private final ChangeIndexer.Factory indexerFactory;
private final ChangeNotes.Factory notesFactory;
private final ProjectCache projectCache;
- private final ThreeWayMergeStrategy mergeStrategy;
- private final AutoMerger autoMerger;
@Inject
AllChangesIndexer(
@@ -100,9 +84,7 @@
@IndexExecutor(BATCH) ListeningExecutorService executor,
ChangeIndexer.Factory indexerFactory,
ChangeNotes.Factory notesFactory,
- @GerritServerConfig Config config,
- ProjectCache projectCache,
- AutoMerger autoMerger) {
+ ProjectCache projectCache) {
this.schemaFactory = schemaFactory;
this.changeDataFactory = changeDataFactory;
this.repoManager = repoManager;
@@ -110,8 +92,6 @@
this.indexerFactory = indexerFactory;
this.notesFactory = notesFactory;
this.projectCache = projectCache;
- this.mergeStrategy = MergeUtil.getMergeStrategy(config);
- this.autoMerger = autoMerger;
}
private static class ProjectHolder implements Comparable<ProjectHolder> {
@@ -241,9 +221,7 @@
byId.put(r.getObjectId(), changeDataFactory.create(db, cn));
}
}
- new ProjectIndexer(
- indexer, mergeStrategy, autoMerger, byId, repo, done, failed, verboseWriter)
- .call();
+ new ProjectIndexer(indexer, byId, repo, done, failed, verboseWriter).call();
} catch (RepositoryNotFoundException rnfe) {
log.error(rnfe.getMessage());
}
@@ -259,8 +237,6 @@
private static class ProjectIndexer implements Callable<Void> {
private final ChangeIndexer indexer;
- private final ThreeWayMergeStrategy mergeStrategy;
- private final AutoMerger autoMerger;
private final ListMultimap<ObjectId, ChangeData> byId;
private final ProgressMonitor done;
private final ProgressMonitor failed;
@@ -269,16 +245,12 @@
private ProjectIndexer(
ChangeIndexer indexer,
- ThreeWayMergeStrategy mergeStrategy,
- AutoMerger autoMerger,
ListMultimap<ObjectId, ChangeData> changesByCommitId,
Repository repo,
ProgressMonitor done,
ProgressMonitor failed,
PrintWriter verboseWriter) {
this.indexer = indexer;
- this.mergeStrategy = mergeStrategy;
- this.autoMerger = autoMerger;
this.byId = changesByCommitId;
this.repo = repo;
this.done = done;
@@ -288,8 +260,7 @@
@Override
public Void call() throws Exception {
- try (ObjectInserter ins = repo.newObjectInserter();
- RevWalk walk = new RevWalk(ins.newReader())) {
+ try (RevWalk walk = new RevWalk(repo)) {
// Walk only refs first to cover as many changes as we can without having
// to mark every single change.
for (Ref ref : repo.getRefDatabase().getRefs(Constants.R_HEADS).values()) {
@@ -299,39 +270,29 @@
}
}
- // TODO(dborowitz): This is basically pointless; it computes
- // currentFilePaths faster than going through PatchListCache, but we
- // still need to go through PatchListCache for changedLines.
RevCommit bCommit;
while ((bCommit = walk.next()) != null && !byId.isEmpty()) {
if (byId.containsKey(bCommit)) {
- getPathsAndIndex(walk, ins, bCommit);
+ index(bCommit);
byId.removeAll(bCommit);
}
}
for (ObjectId id : byId.keySet()) {
- getPathsAndIndex(walk, ins, id);
+ index(id);
}
}
return null;
}
- private void getPathsAndIndex(RevWalk walk, ObjectInserter ins, ObjectId b) throws Exception {
+ private void index(ObjectId b) throws Exception {
List<ChangeData> cds = Lists.newArrayList(byId.get(b));
- try (DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
- RevCommit bCommit = walk.parseCommit(b);
- RevTree bTree = bCommit.getTree();
- RevTree aTree = aFor(bCommit, walk, ins);
- df.setRepository(repo);
+ try {
if (!cds.isEmpty()) {
- List<String> paths =
- (aTree != null) ? getPaths(df.scan(aTree, bTree)) : Collections.<String>emptyList();
Iterator<ChangeData> cdit = cds.iterator();
for (ChangeData cd; cdit.hasNext(); cdit.remove()) {
cd = cdit.next();
try {
- cd.setCurrentFilePaths(paths);
indexer.index(cd);
done.update(1);
verboseWriter.println("Reindexed change " + cd.getId());
@@ -348,43 +309,6 @@
}
}
- private List<String> getPaths(List<DiffEntry> filenames) {
- Set<String> paths = Sets.newTreeSet();
- for (DiffEntry e : filenames) {
- if (e.getOldPath() != null) {
- paths.add(e.getOldPath());
- }
- if (e.getNewPath() != null) {
- paths.add(e.getNewPath());
- }
- }
- return ImmutableList.copyOf(paths);
- }
-
- private RevTree aFor(RevCommit b, RevWalk walk, ObjectInserter ins) throws IOException {
- switch (b.getParentCount()) {
- case 0:
- return walk.parseTree(emptyTree());
- case 1:
- RevCommit a = b.getParent(0);
- walk.parseBody(a);
- return walk.parseTree(a.getTree());
- case 2:
- RevCommit am = autoMerger.merge(repo, walk, ins, b, mergeStrategy);
- return am == null ? null : am.getTree();
- default:
- return null;
- }
- }
-
- private ObjectId emptyTree() throws IOException {
- try (ObjectInserter oi = repo.newObjectInserter()) {
- ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {});
- oi.flush();
- return id;
- }
- }
-
private void fail(String error, boolean failed, Exception e) {
if (failed) {
this.failed.update(1);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummary.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummary.java
index ae4589f..877dba0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummary.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummary.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.server.ioutil.BasicSerialization.writeString;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32;
+import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
@@ -33,16 +34,26 @@
private static final long serialVersionUID = DiffSummaryKey.serialVersionUID;
private transient String[] paths;
+ private transient int insertions;
+ private transient int deletions;
- public DiffSummary(String[] paths) {
+ public DiffSummary(String[] paths, int insertions, int deletions) {
this.paths = paths;
+ this.insertions = insertions;
+ this.deletions = deletions;
}
public List<String> getPaths() {
return Collections.unmodifiableList(Arrays.asList(paths));
}
+ public ChangedLines getChangedLines() {
+ return new ChangedLines(insertions, deletions);
+ }
+
private void writeObject(ObjectOutputStream output) throws IOException {
+ writeVarInt32(output, insertions);
+ writeVarInt32(output, deletions);
writeVarInt32(output, paths.length);
try (DeflaterOutputStream out = new DeflaterOutputStream(output)) {
for (String p : paths) {
@@ -52,6 +63,8 @@
}
private void readObject(ObjectInputStream input) throws IOException {
+ insertions = readVarInt32(input);
+ deletions = readVarInt32(input);
paths = new String[readVarInt32(input)];
try (InflaterInputStream in = new InflaterInputStream(input)) {
for (int i = 0; i < paths.length; i++) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
index 8aa76cb..fa02691 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
@@ -71,6 +71,7 @@
}
}
Collections.sort(r);
- return new DiffSummary(r.toArray(new String[r.size()]));
+ return new DiffSummary(
+ r.toArray(new String[r.size()]), patchList.getInsertions(), patchList.getDeletions());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryWeigher.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryWeigher.java
index 2627792..98e17a5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryWeigher.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryWeigher.java
@@ -26,7 +26,8 @@
+ 4 * 8
+ 2 * 36 // Size of DiffSummaryKey, 64 bit JVM
+ 16
- + 8 // Size of DiffSummary
+ + 8
+ + 2 * 4 // Size of DiffSummary
+ 16
+ 8; // String[]
for (String p : value.getPaths()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
index fd8baad..c32a3f6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
@@ -32,4 +32,7 @@
DiffSummary getDiffSummary(Change change, PatchSet patchSet)
throws PatchListNotAvailableException;
+
+ DiffSummary getDiffSummary(DiffSummaryKey key, Project.NameKey project)
+ throws PatchListNotAvailableException;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index 25f0dda..edff293 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -163,7 +163,8 @@
DiffSummaryKey.fromPatchListKey(PatchListKey.againstDefaultBase(b, ws)), project);
}
- private DiffSummary getDiffSummary(DiffSummaryKey key, Project.NameKey project)
+ @Override
+ public DiffSummary getDiffSummary(DiffSummaryKey key, Project.NameKey project)
throws PatchListNotAvailableException {
try {
return diffSummaryCache.get(key, diffSummaryLoaderFactory.create(key, project));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 3fc719b..2263873 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -57,7 +57,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.DiffSummary;
-import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -331,7 +330,6 @@
private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
private List<PatchSetApproval> currentApprovals;
private Map<Integer, List<String>> files;
- private Map<Integer, Optional<PatchList>> patchLists;
private Map<Integer, Optional<DiffSummary>> diffSummaries;
private Collection<Comment> publishedComments;
private CurrentUser visibleTo;
@@ -602,26 +600,6 @@
return r;
}
- private Optional<PatchList> getPatchList(Change c, PatchSet ps) {
- Integer psId = ps.getId().get();
- if (patchLists == null) {
- patchLists = new HashMap<>();
- }
- Optional<PatchList> r = patchLists.get(psId);
- if (r == null) {
- if (!lazyLoad) {
- return Optional.empty();
- }
- try {
- r = Optional.of(patchListCache.get(c, ps));
- } catch (PatchListNotAvailableException e) {
- r = Optional.empty();
- }
- patchLists.put(psId, r);
- }
- return r;
- }
-
private Optional<DiffSummary> getDiffSummary(Change c, PatchSet ps) {
Integer psId = ps.getId().get();
if (diffSummaries == null) {
@@ -651,7 +629,11 @@
if (ps == null) {
return Optional.empty();
}
- return getPatchList(c, ps).map(p -> new ChangedLines(p.getInsertions(), p.getDeletions()));
+ Optional<DiffSummary> ds = getDiffSummary(c, ps);
+ if (ds.isPresent()) {
+ return Optional.of(ds.get().getChangedLines());
+ }
+ return Optional.empty();
}
public Optional<ChangedLines> changedLines() throws OrmException {
@@ -1269,7 +1251,7 @@
public final int insertions;
public final int deletions;
- ChangedLines(int insertions, int deletions) {
+ public ChangedLines(int insertions, int deletions) {
this.insertions = insertions;
this.deletions = deletions;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 982e400..9f31550 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -81,7 +81,6 @@
down-arrow
vertical-offset="32"
horizontal-align="right"
- on-tap-item-abandon="_handleAbandonTap"
on-tap-item-cherrypick="_handleCherrypickTap"
on-tap-item-delete="_handleDeleteTap"
hidden$="[[_shouldHideActions(_menuActions.*, _loading)]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 69f4f57..148b724 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -92,7 +92,6 @@
* set of action buttons.
*/
var MENU_ACTION_KEYS = [
- 'abandon',
'cherrypick',
'/', // '/' is the key for the delete action.
];
@@ -452,6 +451,8 @@
this._handleRevisionAction(key);
} else if (key === ChangeActions.REVERT) {
this.showRevertDialog();
+ } else if (key === ChangeActions.ABANDON) {
+ this._showActionDialog(this.$.confirmAbandonDialog);
} else if (key === QUICK_APPROVE_ACTION.key) {
var action = this._allActionValues.find(function(o) {
return o.key === key;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index e33458f..fbbaadb 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -163,13 +163,14 @@
}
var messages = this.messages || [];
var index = message._index;
- var authorId = message.author._account_id;
+ var authorId = message.author && message.author._account_id;
var mDate = util.parseDate(message.date).getTime();
// NB: Messages array has oldest messages first.
var nextMDate;
if (index > 0) {
for (var i = index - 1; i >= 0; i--) {
- if (messages[i].author._account_id === authorId) {
+ if (messages[i] && messages[i].author &&
+ messages[i].author._account_id === authorId) {
nextMDate = util.parseDate(messages[i].date).getTime();
break;
}
@@ -179,7 +180,8 @@
for (var file in comments) {
var fileComments = comments[file];
for (var i = 0; i < fileComments.length; i++) {
- if (fileComments[i].author._account_id !== authorId) {
+ if (fileComments[i].author &&
+ fileComments[i].author._account_id !== authorId) {
continue;
}
var cDate = util.parseDate(fileComments[i].updated).getTime();
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index 770da61..4c6ff36 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -231,6 +231,37 @@
comments.file2.filter(isMarvin));
assert.deepEqual(messageElements[2].comments, {});
});
+
+ test('messages without author do not throw', function() {
+ var comments = {
+ file1: [
+ {
+ message: 'message text',
+ updated: '2016-09-27 00:18:03.000000000',
+ in_reply_to: '6505d749_f0bec0aa',
+ line: 62,
+ id: '6505d749_10ed44b2',
+ patch_set: 2,
+ author: {
+ email: 'some@email.com',
+ _account_id: 123,
+ },
+ },
+ ]};
+ var messages = [{
+ _index: 5,
+ _revision_number: 4,
+ message: 'Uploaded patch set 4.',
+ date: '2016-09-28 13:36:33.000000000',
+ id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
+ }];
+ element.messages = messages;
+ element.comments = comments
+ flushAsynchronousOperations();
+ var messageEls = Polymer.dom(element.root).querySelectorAll('gr-message');
+ assert.equal(messageEls.length, 1);
+ assert.equal(messageEls[0].message.message, messages[0].message);
+ });
});
suite('gr-messages-list automate tests', function() {
@@ -311,5 +342,6 @@
//Autogenerated messages are now hidden.
assert.isFalse(!!allHiddenMessageEls.length);
});
- });
+});
+
</script>
