Merge "Reload actions when new change is loaded, not on new changeNum"
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java b/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
index 00dc05a5..db37147 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
@@ -840,7 +840,7 @@
case FORCED:
break;
case LOCK_FAILURE:
- throw new LockFailureException("Updating external IDs failed with " + res);
+ throw new LockFailureException("Updating external IDs failed with " + res, u);
case IO_FAILURE:
case NOT_ATTEMPTED:
case REJECTED:
diff --git a/java/com/google/gerrit/server/change/SetReadyForReview.java b/java/com/google/gerrit/server/change/SetReadyForReview.java
index eb61d3c..3d258c3 100644
--- a/java/com/google/gerrit/server/change/SetReadyForReview.java
+++ b/java/com/google/gerrit/server/change/SetReadyForReview.java
@@ -75,7 +75,7 @@
@Override
public Description getDescription(ChangeResource rsrc) {
return new Description()
- .setLabel("Ready")
+ .setLabel("Start Review")
.setTitle("Set Ready For Review")
.setVisible(
rsrc.isUserOwner()
diff --git a/java/com/google/gerrit/server/git/InMemoryInserter.java b/java/com/google/gerrit/server/git/InMemoryInserter.java
index 9c43bdb..b80f846 100644
--- a/java/com/google/gerrit/server/git/InMemoryInserter.java
+++ b/java/com/google/gerrit/server/git/InMemoryInserter.java
@@ -95,6 +95,10 @@
return ImmutableList.copyOf(inserted.values());
}
+ public int getInsertedObjectCount() {
+ return inserted.values().size();
+ }
+
public void clear() {
inserted.clear();
}
diff --git a/java/com/google/gerrit/server/git/LockFailureException.java b/java/com/google/gerrit/server/git/LockFailureException.java
index 7380b0a..02a30e0 100644
--- a/java/com/google/gerrit/server/git/LockFailureException.java
+++ b/java/com/google/gerrit/server/git/LockFailureException.java
@@ -14,13 +14,38 @@
package com.google.gerrit.server.git;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.ReceiveCommand;
/** Thrown when updating a ref in Git fails with LOCK_FAILURE. */
public class LockFailureException extends IOException {
private static final long serialVersionUID = 1L;
- public LockFailureException(String message) {
+ private final ImmutableList<String> refs;
+
+ public LockFailureException(String message, RefUpdate refUpdate) {
super(message);
+ refs = ImmutableList.of(refUpdate.getName());
+ }
+
+ public LockFailureException(String message, BatchRefUpdate batchRefUpdate) {
+ super(message);
+ refs =
+ batchRefUpdate
+ .getCommands()
+ .stream()
+ .filter(c -> c.getResult() == ReceiveCommand.Result.LOCK_FAILURE)
+ .map(ReceiveCommand::getRefName)
+ .collect(toImmutableList());
+ }
+
+ /** Subset of ref names that caused the lock failure. */
+ public ImmutableList<String> getFailedRefs() {
+ return refs;
}
}
diff --git a/java/com/google/gerrit/server/git/VersionedMetaData.java b/java/com/google/gerrit/server/git/VersionedMetaData.java
index 1bf3586..92edc47 100644
--- a/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -395,7 +395,8 @@
+ " in "
+ db.getDirectory()
+ ": "
- + ru.getResult());
+ + ru.getResult(),
+ ru);
case FORCED:
case IO_FAILURE:
case NOT_ATTEMPTED:
diff --git a/java/com/google/gerrit/server/notedb/ChangeBundle.java b/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 200e0d6..a9663c7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -68,6 +68,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
@@ -387,6 +388,8 @@
boolean excludeCreatedOn = false;
boolean excludeCurrentPatchSetId = false;
boolean excludeTopic = false;
+ Timestamp aCreated = a.getCreatedOn();
+ Timestamp bCreated = b.getCreatedOn();
Timestamp aUpdated = a.getLastUpdatedOn();
Timestamp bUpdated = b.getLastUpdatedOn();
@@ -397,8 +400,10 @@
String aSubj = Strings.nullToEmpty(a.getSubject());
String bSubj = Strings.nullToEmpty(b.getSubject());
- // Allow created timestamp in NoteDb to be either the created timestamp of
- // the change, or the timestamp of the first remaining patch set.
+ // Allow created timestamp in NoteDb to be any of:
+ // - The created timestamp of the change.
+ // - The timestamp of the first remaining patch set.
+ // - The last updated timestamp, if it is less than the created timestamp.
//
// Ignore subject if the NoteDb subject starts with the ReviewDb subject.
// The NoteDb subject is read directly from the commit, whereas the ReviewDb
@@ -434,8 +439,14 @@
//
// Use max timestamp of all ReviewDb entities when comparing with NoteDb.
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
+ boolean createdOnMatchesFirstPs =
+ !timestampsDiffer(bundleA, bundleA.getFirstPatchSetTime(), bundleB, bCreated);
+ boolean createdOnMatchesLastUpdatedOn =
+ !timestampsDiffer(bundleA, aUpdated, bundleB, bCreated);
+ boolean createdAfterUpdated = aCreated.compareTo(aUpdated) > 0;
excludeCreatedOn =
- !timestampsDiffer(bundleA, bundleA.getFirstPatchSetTime(), bundleB, b.getCreatedOn());
+ createdOnMatchesFirstPs || (createdAfterUpdated && createdOnMatchesLastUpdatedOn);
+
aSubj = cleanReviewDbSubject(aSubj);
bSubj = cleanNoteDbSubject(bSubj);
excludeCurrentPatchSetId = !bundleA.validPatchSetPredicate().apply(a.currentPatchSetId());
@@ -446,8 +457,14 @@
Objects.equals(aTopic, b.getTopic()) || "".equals(aTopic) && b.getTopic() == null;
aUpdated = bundleA.getLatestTimestamp();
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
+ boolean createdOnMatchesFirstPs =
+ !timestampsDiffer(bundleA, aCreated, bundleB, bundleB.getFirstPatchSetTime());
+ boolean createdOnMatchesLastUpdatedOn =
+ !timestampsDiffer(bundleA, aCreated, bundleB, bUpdated);
+ boolean createdAfterUpdated = bCreated.compareTo(bUpdated) > 0;
excludeCreatedOn =
- !timestampsDiffer(bundleA, a.getCreatedOn(), bundleB, bundleB.getFirstPatchSetTime());
+ createdOnMatchesFirstPs || (createdAfterUpdated && createdOnMatchesLastUpdatedOn);
+
aSubj = cleanNoteDbSubject(aSubj);
bSubj = cleanReviewDbSubject(bSubj);
excludeCurrentPatchSetId = !bundleB.validPatchSetPredicate().apply(b.currentPatchSetId());
@@ -651,6 +668,8 @@
List<String> diffs, ChangeBundle bundleA, ChangeBundle bundleB) {
Map<PatchSet.Id, PatchSet> as = bundleA.patchSets;
Map<PatchSet.Id, PatchSet> bs = bundleB.patchSets;
+ Optional<PatchSet.Id> minA = as.keySet().stream().min(intKeyOrdering());
+ Optional<PatchSet.Id> minB = bs.keySet().stream().min(intKeyOrdering());
Set<PatchSet.Id> ids = diffKeySets(diffs, as, bs);
// Old versions of Gerrit had a bug that created patch sets during
@@ -663,11 +682,14 @@
// ignore the createdOn timestamps if both:
// * ReviewDb timestamps are non-monotonic.
// * NoteDb timestamps are monotonic.
- boolean excludeCreatedOn = false;
+ //
+ // Allow the timestamp of the first patch set to match the creation time of
+ // the change.
+ boolean excludeAllCreatedOn = false;
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
- excludeCreatedOn = !createdOnIsMonotonic(as, ids) && createdOnIsMonotonic(bs, ids);
+ excludeAllCreatedOn = !createdOnIsMonotonic(as, ids) && createdOnIsMonotonic(bs, ids);
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
- excludeCreatedOn = createdOnIsMonotonic(as, ids) && !createdOnIsMonotonic(bs, ids);
+ excludeAllCreatedOn = createdOnIsMonotonic(as, ids) && !createdOnIsMonotonic(bs, ids);
}
for (PatchSet.Id id : ids) {
@@ -676,11 +698,16 @@
String desc = describe(id);
String pushCertField = "pushCertificate";
+ boolean excludeCreatedOn = excludeAllCreatedOn;
boolean excludeDesc = false;
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
excludeDesc = Objects.equals(trimOrNull(a.getDescription()), b.getDescription());
+ excludeCreatedOn |=
+ Optional.of(id).equals(minB) && b.getCreatedOn().equals(bundleB.change.getCreatedOn());
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
excludeDesc = Objects.equals(a.getDescription(), trimOrNull(b.getDescription()));
+ excludeCreatedOn |=
+ Optional.of(id).equals(minA) && a.getCreatedOn().equals(bundleA.change.getCreatedOn());
}
List<String> exclude = Lists.newArrayList(pushCertField);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index c9a28f4..7799d2d 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -779,6 +779,7 @@
rebuildResult = checkNotNull(r);
checkNotNull(r.newState());
checkNotNull(r.staged());
+ checkNotNull(r.staged().changeObjects());
return LoadHandle.create(
ChangeNotesCommit.newStagedRevWalk(repo, r.staged().changeObjects()),
r.newState().getChangeMetaId());
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index ea1f891..3e36de9 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -98,13 +98,13 @@
ImmutableList<InsertedObject> changeObjects = ImmutableList.of();
if (changeRepo != null) {
changeCommands = changeRepo.getCommandsSnapshot();
- changeObjects = changeRepo.tempIns.getInsertedObjects();
+ changeObjects = changeRepo.getInsertedObjects();
}
ImmutableList<ReceiveCommand> allUsersCommands = ImmutableList.of();
ImmutableList<InsertedObject> allUsersObjects = ImmutableList.of();
if (allUsersRepo != null) {
allUsersCommands = allUsersRepo.getCommandsSnapshot();
- allUsersObjects = allUsersRepo.tempIns.getInsertedObjects();
+ allUsersObjects = allUsersRepo.getInsertedObjects();
}
return new AutoValue_NoteDbUpdateManager_StagedResult(
id, delta,
@@ -119,10 +119,32 @@
public abstract ImmutableList<ReceiveCommand> changeCommands();
+ /**
+ * Objects inserted into the change repo for this change.
+ *
+ * <p>Includes all objects inserted for any change in this repo that may have been processed by
+ * the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+ * inserted to handle this specific change's updates.
+ *
+ * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+ * configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+ */
+ @Nullable
public abstract ImmutableList<InsertedObject> changeObjects();
public abstract ImmutableList<ReceiveCommand> allUsersCommands();
+ /**
+ * Objects inserted into the All-Users repo for this change.
+ *
+ * <p>Includes all objects inserted into All-Users for any change that may have been processed
+ * by the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+ * inserted to handle this specific change's updates.
+ *
+ * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+ * configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+ */
+ @Nullable
public abstract ImmutableList<InsertedObject> allUsersObjects();
}
@@ -144,17 +166,20 @@
public final RevWalk rw;
public final ChainedReceiveCommands cmds;
- private final InMemoryInserter tempIns;
+ private final InMemoryInserter inMemIns;
+ private final ObjectInserter tempIns;
@Nullable private final ObjectInserter finalIns;
private final boolean close;
+ private final boolean saveObjects;
private OpenRepo(
Repository repo,
RevWalk rw,
@Nullable ObjectInserter ins,
ChainedReceiveCommands cmds,
- boolean close) {
+ boolean close,
+ boolean saveObjects) {
ObjectReader reader = rw.getObjectReader();
checkArgument(
ins == null || reader.getCreatedFromInserter() == ins,
@@ -162,11 +187,21 @@
ins,
reader.getCreatedFromInserter());
this.repo = checkNotNull(repo);
- this.tempIns = new InMemoryInserter(rw.getObjectReader());
+
+ if (saveObjects) {
+ this.inMemIns = new InMemoryInserter(rw.getObjectReader());
+ this.tempIns = inMemIns;
+ } else {
+ checkArgument(ins != null);
+ this.inMemIns = null;
+ this.tempIns = ins;
+ }
+
this.rw = new RevWalk(tempIns.newReader());
this.finalIns = ins;
this.cmds = checkNotNull(cmds);
this.close = close;
+ this.saveObjects = saveObjects;
}
public Optional<ObjectId> getObjectId(String refName) throws IOException {
@@ -177,17 +212,25 @@
return ImmutableList.copyOf(cmds.getCommands().values());
}
+ @Nullable
+ ImmutableList<InsertedObject> getInsertedObjects() {
+ return saveObjects ? inMemIns.getInsertedObjects() : null;
+ }
+
void flush() throws IOException {
flushToFinalInserter();
finalIns.flush();
}
void flushToFinalInserter() throws IOException {
+ if (!saveObjects) {
+ return;
+ }
checkState(finalIns != null);
- for (InsertedObject obj : tempIns.getInsertedObjects()) {
+ for (InsertedObject obj : inMemIns.getInsertedObjects()) {
finalIns.insert(obj.type(), obj.data().toByteArray());
}
- tempIns.clear();
+ inMemIns.clear();
}
@Override
@@ -219,6 +262,8 @@
private OpenRepo allUsersRepo;
private Map<Change.Id, StagedResult> staged;
private boolean checkExpectedState = true;
+ private boolean saveObjects = true;
+ private boolean atomicRefUpdates = true;
private String refLogMessage;
private PersonIdent refLogIdent;
private PushCertificate pushCert;
@@ -264,14 +309,14 @@
public NoteDbUpdateManager setChangeRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(changeRepo == null, "change repo already initialized");
- changeRepo = new OpenRepo(repo, rw, ins, cmds, false);
+ changeRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
public NoteDbUpdateManager setAllUsersRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(allUsersRepo == null, "All-Users repo already initialized");
- allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false);
+ allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
@@ -280,6 +325,37 @@
return this;
}
+ /**
+ * Set whether to save objects and make them available in {@link StagedResult}s.
+ *
+ * <p>If set, all objects inserted into all repos managed by this instance will be buffered in
+ * memory, and the {@link StagedResult}s will return non-null lists from {@link
+ * StagedResult#changeObjects()} and {@link StagedResult#allUsersObjects()}.
+ *
+ * <p>Not recommended if modifying a large number of changes with a single manager.
+ *
+ * @param saveObjects whether to save objects; defaults to true.
+ * @return this
+ */
+ public NoteDbUpdateManager setSaveObjects(boolean saveObjects) {
+ this.saveObjects = saveObjects;
+ return this;
+ }
+
+ /**
+ * Set whether to use atomic ref updates.
+ *
+ * <p>Can be set to false when the change updates represented by this manager aren't logically
+ * related, e.g. when the updater is only used to group objects together with a single inserter.
+ *
+ * @param atomicRefUpdates whether to use atomic ref updates; defaults to true.
+ * @return this
+ */
+ public NoteDbUpdateManager setAtomicRefUpdates(boolean atomicRefUpdates) {
+ this.atomicRefUpdates = atomicRefUpdates;
+ return this;
+ }
+
public NoteDbUpdateManager setRefLogMessage(String message) {
this.refLogMessage = message;
return this;
@@ -336,7 +412,7 @@
ObjectInserter ins = repo.newObjectInserter(); // Closed by OpenRepo#close.
ObjectReader reader = ins.newReader(); // Not closed by OpenRepo#close.
try (RevWalk rw = new RevWalk(reader)) { // Doesn't escape OpenRepo constructor.
- return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true) {
+ return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true, saveObjects) {
@Override
public void close() {
reader.close();
@@ -543,6 +619,7 @@
} else {
// OpenRepo buffers objects separately; caller may assume that objects are available in the
// inserter it previously passed via setChangeRepo.
+ checkState(saveObjects, "cannot use dryrun with saveObjects = false");
or.flushToFinalInserter();
}
@@ -554,6 +631,7 @@
bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
}
bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
+ bru.setAtomic(atomicRefUpdates);
or.cmds.addTo(bru);
bru.setAllowNonFastForwards(true);
diff --git a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 166d8a9..f96e96c 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -187,7 +187,7 @@
}
try (NoteDbUpdateManager manager = updateManagerFactory.create(change.getProject())) {
buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
- return execute(db, changeId, manager, checkReadOnly);
+ return execute(db, changeId, manager, checkReadOnly, true);
}
}
@@ -216,11 +216,15 @@
@Override
public Result execute(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
throws OrmException, IOException {
- return execute(db, changeId, manager, true);
+ return execute(db, changeId, manager, true, true);
}
public Result execute(
- ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager, boolean checkReadOnly)
+ ReviewDb db,
+ Change.Id changeId,
+ NoteDbUpdateManager manager,
+ boolean checkReadOnly,
+ boolean executeManager)
throws OrmException, IOException {
db = ReviewDbUtil.unwrapDb(db);
Change change = checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
@@ -277,11 +281,13 @@
// to the caller so they know to use the staged results instead of reading from the repo.
throw new OrmException(NoteDbUpdateManager.CHANGES_READ_ONLY);
}
- manager.execute();
+ if (executeManager) {
+ manager.execute();
+ }
return r;
}
- private static Change checkNoteDbState(Change c) throws OrmException {
+ static Change checkNoteDbState(Change c) throws OrmException {
// Can only rebuild a change if its primary storage is ReviewDb.
NoteDbChangeState s = NoteDbChangeState.parse(c);
if (s != null && s.getPrimaryStorage() != PrimaryStorage.REVIEW_DB) {
@@ -299,6 +305,13 @@
if (bundle.getPatchSets().isEmpty()) {
throw new NoPatchSetsException(change.getId());
}
+ if (change.getLastUpdatedOn().compareTo(change.getCreatedOn()) < 0) {
+ // A bug in data migration might set created_on to the time of the migration. The
+ // correct timestamps were lost, but we can at least set it so created_on is not after
+ // last_updated_on.
+ // See https://bugs.chromium.org/p/gerrit/issues/detail?id=7397
+ change.setCreatedOn(change.getLastUpdatedOn());
+ }
// We will rebuild all events, except for draft comments, in buckets based on author and
// timestamp.
@@ -428,9 +441,11 @@
new EventSorter(events).sort();
// Ensure the first event in the list creates the change, setting the author and any required
- // footers.
+ // footers. Also force the creation time of the first patch set to match the creation time of
+ // the change.
Event first = events.get(0);
if (first instanceof PatchSetEvent && change.getOwner().equals(first.user)) {
+ first.when = change.getCreatedOn();
((PatchSetEvent) first).createChange = true;
} else {
events.add(0, new CreateChangeEvent(change, minPsNum));
diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index add6d57..2981174 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -26,6 +26,7 @@
import static com.google.gerrit.server.notedb.NotesMigrationState.WRITE;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import com.google.common.annotations.VisibleForTesting;
@@ -56,12 +57,17 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.notedb.ChangeBundleReader;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.MutableNotesMigration;
import com.google.gerrit.server.notedb.NoteDbTable;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.NotesMigrationState;
import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
@@ -74,17 +80,24 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.function.Predicate;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.io.NullOutputStream;
@@ -119,10 +132,12 @@
private final SitePaths sitePaths;
private final SchemaFactory<ReviewDb> schemaFactory;
private final GitRepositoryManager repoManager;
+ private final NoteDbUpdateManager.Factory updateManagerFactory;
+ private final ChangeBundleReader bundleReader;
private final AllProjectsName allProjects;
private final InternalUser.Factory userFactory;
private final ThreadLocalRequestContext requestContext;
- private final ChangeRebuilder rebuilder;
+ private final ChangeRebuilderImpl rebuilder;
private final WorkQueue workQueue;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
@@ -144,10 +159,12 @@
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
GitRepositoryManager repoManager,
+ NoteDbUpdateManager.Factory updateManagerFactory,
+ ChangeBundleReader bundleReader,
AllProjectsName allProjects,
ThreadLocalRequestContext requestContext,
InternalUser.Factory userFactory,
- ChangeRebuilder rebuilder,
+ ChangeRebuilderImpl rebuilder,
WorkQueue workQueue,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
@@ -159,6 +176,8 @@
this.sitePaths = sitePaths;
this.schemaFactory = schemaFactory;
this.repoManager = repoManager;
+ this.updateManagerFactory = updateManagerFactory;
+ this.bundleReader = bundleReader;
this.allProjects = allProjects;
this.requestContext = requestContext;
this.userFactory = userFactory;
@@ -318,6 +337,8 @@
sitePaths,
schemaFactory,
repoManager,
+ updateManagerFactory,
+ bundleReader,
allProjects,
requestContext,
userFactory,
@@ -343,10 +364,12 @@
private final FileBasedConfig noteDbConfig;
private final SchemaFactory<ReviewDb> schemaFactory;
private final GitRepositoryManager repoManager;
+ private final NoteDbUpdateManager.Factory updateManagerFactory;
+ private final ChangeBundleReader bundleReader;
private final AllProjectsName allProjects;
private final ThreadLocalRequestContext requestContext;
private final InternalUser.Factory userFactory;
- private final ChangeRebuilder rebuilder;
+ private final ChangeRebuilderImpl rebuilder;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
private final DynamicSet<NotesMigrationStateListener> listeners;
@@ -365,10 +388,12 @@
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
GitRepositoryManager repoManager,
+ NoteDbUpdateManager.Factory updateManagerFactory,
+ ChangeBundleReader bundleReader,
AllProjectsName allProjects,
ThreadLocalRequestContext requestContext,
InternalUser.Factory userFactory,
- ChangeRebuilder rebuilder,
+ ChangeRebuilderImpl rebuilder,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
DynamicSet<NotesMigrationStateListener> listeners,
@@ -392,6 +417,8 @@
this.schemaFactory = schemaFactory;
this.rebuilder = rebuilder;
this.repoManager = repoManager;
+ this.updateManagerFactory = updateManagerFactory;
+ this.bundleReader = bundleReader;
this.allProjects = allProjects;
this.requestContext = requestContext;
this.userFactory = userFactory;
@@ -720,6 +747,12 @@
return ImmutableListMultimap.copyOf(out);
}
+ private static ObjectInserter newPackInserter(Repository repo) {
+ return repo instanceof FileRepository
+ ? ((FileRepository) repo).getObjectDatabase().newPackInserter()
+ : repo.newObjectInserter();
+ }
+
private boolean rebuildProject(
ReviewDb db,
ImmutableListMultimap<Project.NameKey, Change.Id> allChanges,
@@ -729,43 +762,105 @@
ProgressMonitor pm =
new TextProgressMonitor(
new PrintWriter(new BufferedWriter(new OutputStreamWriter(progressOut, UTF_8))));
- pm.beginTask(FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
- try {
+ try (Repository changeRepo = repoManager.openRepository(project);
+ ObjectInserter changeIns = newPackInserter(changeRepo);
+ ObjectReader changeReader = changeIns.newReader();
+ RevWalk changeRw = new RevWalk(changeReader);
+ NoteDbUpdateManager manager =
+ updateManagerFactory
+ .create(project)
+ .setSaveObjects(false)
+ .setAtomicRefUpdates(false)
+ // Only use a PackInserter for the change repo, not All-Users.
+ //
+ // It's not possible to share a single inserter for All-Users across all project
+ // tasks, and we don't want to add one pack per project to All-Users. Adding many
+ // loose objects is preferable to many packs.
+ //
+ // Anyway, the number of objects inserted into All-Users is proportional to the
+ // number of pending draft comments, which should not be high (relative to the total
+ // number of changes), so the number of loose objects shouldn't be too unreasonable.
+ .setChangeRepo(
+ changeRepo, changeRw, changeIns, new ChainedReceiveCommands(changeRepo))) {
+ Set<Change.Id> skipExecute = new HashSet<>();
Collection<Change.Id> changes = allChanges.get(project);
- for (Change.Id changeId : changes) {
- // Update one change at a time, which ends up creating one NoteDbUpdateManager per change as
- // well. This turns out to be no more expensive than batching, since each NoteDb operation
- // is only writing single loose ref updates and loose objects. Plus we have to do one
- // ReviewDb transaction per change due to the AtomicUpdate, so if we somehow batched NoteDb
- // operations, ReviewDb would become the bottleneck.
- try {
- rebuilder.rebuild(db, changeId);
- } catch (NoPatchSetsException e) {
- log.warn(e.getMessage());
- } catch (RepositoryNotFoundException e) {
- log.warn("Repository {} not found while rebuilding change {}", project, changeId);
- } catch (ConflictingUpdateException e) {
- log.warn(
- "Rebuilding detected a conflicting ReviewDb update for change {};"
- + " will be auto-rebuilt at runtime",
- changeId);
- } catch (LockFailureException e) {
- log.warn(
- "Rebuilding detected a conflicting NoteDb update for change {};"
- + " will be auto-rebuilt at runtime",
- changeId);
- } catch (Throwable t) {
- log.error("Failed to rebuild change " + changeId, t);
- ok = false;
+ pm.beginTask(FormatUtil.elide("Rebuilding " + project.get(), 50), changes.size());
+ try {
+ for (Change.Id changeId : changes) {
+ boolean staged = false;
+ try {
+ stage(db, changeId, manager);
+ staged = true;
+ } catch (NoPatchSetsException e) {
+ log.warn(e.getMessage());
+ } catch (Throwable t) {
+ log.error("Failed to rebuild change " + changeId, t);
+ ok = false;
+ }
+ pm.update(1);
+ if (!staged) {
+ skipExecute.add(changeId);
+ }
}
- pm.update(1);
+ } finally {
+ pm.endTask();
}
- } finally {
- pm.endTask();
+
+ pm.beginTask(
+ FormatUtil.elide("Saving " + project.get(), 50), changes.size() - skipExecute.size());
+ try {
+ for (Change.Id changeId : changes) {
+ if (skipExecute.contains(changeId)) {
+ continue;
+ }
+ try {
+ rebuilder.execute(db, changeId, manager, true, false);
+ } catch (ConflictingUpdateException e) {
+ log.warn(
+ "Rebuilding detected a conflicting ReviewDb update for change {};"
+ + " will be auto-rebuilt at runtime",
+ changeId);
+ } catch (Throwable t) {
+ log.error("Failed to rebuild change " + changeId, t);
+ ok = false;
+ }
+ pm.update(1);
+ }
+ } finally {
+ pm.endTask();
+ }
+
+ try {
+ manager.execute();
+ } catch (LockFailureException e) {
+ log.warn(
+ "Rebuilding detected a conflicting NoteDb update for the following refs, which will"
+ + " be auto-rebuilt at runtime: {}",
+ e.getFailedRefs().stream().distinct().sorted().collect(joining(", ")));
+ } catch (OrmException | IOException e) {
+ log.error("Failed to save NoteDb state for " + project, e);
+ }
+ } catch (RepositoryNotFoundException e) {
+ log.warn("Repository {} not found", project);
+ } catch (IOException e) {
+ log.error("Failed to rebuild project " + project, e);
}
return ok;
}
+ private void stage(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
+ throws OrmException, IOException {
+ // Match ChangeRebuilderImpl#stage, but without calling manager.stage(), since that can only be
+ // called after building updates for all changes.
+ Change change =
+ ChangeRebuilderImpl.checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
+ if (change == null) {
+ // Could log here instead, but this matches the behavior of ChangeRebuilderImpl#stage.
+ throw new NoSuchChangeException(changeId);
+ }
+ rebuilder.buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
+ }
+
private static boolean futuresToBoolean(List<ListenableFuture<Boolean>> futures, String errMsg) {
try {
return Futures.allAsList(futures).get().stream().allMatch(b -> b);
diff --git a/java/com/google/gerrit/server/update/RefUpdateUtil.java b/java/com/google/gerrit/server/update/RefUpdateUtil.java
index ab0b78e..86b4eef 100644
--- a/java/com/google/gerrit/server/update/RefUpdateUtil.java
+++ b/java/com/google/gerrit/server/update/RefUpdateUtil.java
@@ -74,7 +74,7 @@
}
if (lockFailure + aborted == bru.getCommands().size()) {
- throw new LockFailureException("Update aborted with one or more lock failures: " + bru);
+ throw new LockFailureException("Update aborted with one or more lock failures: " + bru, bru);
} else if (failure > 0) {
throw new IOException("Update failed: " + bru);
}
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 6e5efcd..81ee3a0 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -244,7 +244,8 @@
"Update git submodules\n\n"
+ "* Update "
+ name("subscribed-to-project")
- + " from branch 'master'\n to " + subHEAD.getName());
+ + " from branch 'master'\n to "
+ + subHEAD.getName());
// The next commit should generate only its commit message,
// omitting previous commit logs
@@ -256,7 +257,8 @@
"Update git submodules\n\n"
+ "* Update "
+ name("subscribed-to-project")
- + " from branch 'master'\n to " + subHEAD.getName()
+ + " from branch 'master'\n to "
+ + subHEAD.getName()
+ "\n - "
+ subCommitMsg.getShortMessage());
}
@@ -280,7 +282,8 @@
"Update git submodules\n\n"
+ "* Update "
+ name("subscribed-to-project")
- + " from branch 'master'\n to " + subHEAD.getName());
+ + " from branch 'master'\n to "
+ + subHEAD.getName());
// The next commit should generate only its commit message,
// omitting previous commit logs
@@ -292,7 +295,8 @@
"Update git submodules\n\n"
+ "* Update "
+ name("subscribed-to-project")
- + " from branch 'master'\n to " + subHEAD.getName()
+ + " from branch 'master'\n to "
+ + subHEAD.getName()
+ "\n - "
+ subCommitMsg.getFullMessage().replace("\n", "\n "));
}
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index 4169696..e021203 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -867,6 +867,45 @@
}
@Test
+ public void allTimestampsExceptUpdatedAreEqualDueToBadMigration() throws Exception {
+ // https://bugs.chromium.org/p/gerrit/issues/detail?id=7397
+ PushOneCommit.Result r = createChange();
+ Change c = r.getChange().change();
+ Change.Id id = c.getId();
+ Timestamp ts = TimeUtil.nowTs();
+ Timestamp origUpdated = c.getLastUpdatedOn();
+
+ c.setCreatedOn(ts);
+ assertThat(c.getCreatedOn()).isGreaterThan(c.getLastUpdatedOn());
+ db.changes().update(Collections.singleton(c));
+
+ List<ChangeMessage> cm = db.changeMessages().byChange(id).toList();
+ cm.forEach(m -> m.setWrittenOn(ts));
+ db.changeMessages().update(cm);
+
+ List<PatchSet> ps = db.patchSets().byChange(id).toList();
+ ps.forEach(p -> p.setCreatedOn(ts));
+ db.patchSets().update(ps);
+
+ List<PatchSetApproval> psa = db.patchSetApprovals().byChange(id).toList();
+ psa.forEach(p -> p.setGranted(ts));
+ db.patchSetApprovals().update(psa);
+
+ List<PatchLineComment> plc = db.patchComments().byChange(id).toList();
+ plc.forEach(p -> p.setWrittenOn(ts));
+ db.patchComments().update(plc);
+
+ checker.rebuildAndCheckChanges(id);
+
+ setNotesMigration(true, true);
+ ChangeNotes notes = notesFactory.create(db, project, id);
+ assertThat(notes.getChange().getCreatedOn()).isEqualTo(origUpdated);
+ assertThat(notes.getChange().getLastUpdatedOn()).isAtLeast(origUpdated);
+ assertThat(notes.getPatchSets().get(new PatchSet.Id(id, 1)).getCreatedOn())
+ .isEqualTo(origUpdated);
+ }
+
+ @Test
public void createWithAutoRebuildingDisabled() throws Exception {
ReviewDb oldDb = db;
setNotesMigration(true, true);
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index ca5bd5e..7d33845 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -59,8 +59,17 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -373,10 +382,14 @@
Change.Id id1 = r1.getChange().getId();
Change.Id id2 = r2.getChange().getId();
- migrate(b -> b.setThreads(threads));
- assertNotesMigrationState(NOTE_DB, false, false);
+ Set<String> objectFiles = getObjectFiles(project);
+ assertThat(objectFiles).isNotEmpty();
+ migrate(b -> b.setThreads(threads));
+
+ assertNotesMigrationState(NOTE_DB, false, false);
assertThat(sequences.nextChangeId()).isEqualTo(503);
+ assertThat(getObjectFiles(project)).containsExactlyElementsIn(objectFiles);
ObjectId oldMetaId = null;
int rowVersion = 0;
@@ -531,4 +544,23 @@
private void addListener(NotesMigrationStateListener listener) {
addedListeners.add(listeners.add(listener));
}
+
+ private SortedSet<String> getObjectFiles(Project.NameKey project) throws Exception {
+ SortedSet<String> files = new TreeSet<>();
+ try (Repository repo = repoManager.openRepository(project)) {
+ Files.walkFileTree(
+ ((FileRepository) repo).getObjectDatabase().getDirectory().toPath(),
+ new SimpleFileVisitor<Path>() {
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
+ String name = file.getFileName().toString();
+ if (!attrs.isDirectory() && !name.endsWith(".pack") && !name.endsWith(".idx")) {
+ files.add(name);
+ }
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ }
+ return files;
+ }
}
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 539e9fb..3df89d9 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -20,10 +20,17 @@
],
)
-junit_tests(
- name = "elasticsearch_tests",
+ELASTICSEARCH_TESTS = {i: "ElasticQuery" + i.capitalize() + "sTest.java" for i in [
+ "account",
+ "change",
+ "group",
+ "project",
+]}
+
+[junit_tests(
+ name = "elasticsearch_%ss_test" % name,
size = "large",
- srcs = glob(["**/*Test.java"]),
+ srcs = [src],
tags = [
"elastic",
],
@@ -33,9 +40,9 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//javatests/com/google/gerrit/server:query_tests_code",
+ "//javatests/com/google/gerrit/server:abstract_query_tests",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
],
-)
+) for name, src in ELASTICSEARCH_TESTS.items()]
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index e51a330..a7f2e09 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -81,14 +81,21 @@
],
)
-QUERY_TESTS = glob(
- ["query/**/*.java"],
+ABSTRACT_QUERY_TESTS = glob(
+ ["query/**/AbstractQuery*Test.java"],
)
+LUCENE_QUERY_TESTS = {i: "query/" + i + "/LuceneQuery" + i.capitalize() + "sTest.java" for i in [
+ "account",
+ "change",
+ "group",
+ "project",
+]}
+
java_library(
- name = "query_tests_code",
+ name = "abstract_query_tests",
testonly = 1,
- srcs = QUERY_TESTS,
+ srcs = ABSTRACT_QUERY_TESTS,
visibility = ["//visibility:public"],
deps = TESTUTIL_DEPS + [
"//java/com/google/gerrit/testing:gerrit-test-util",
@@ -96,23 +103,23 @@
],
)
-junit_tests(
- name = "query_tests",
+[junit_tests(
+ name = "lucene_query_%ss_test" % name,
size = "large",
- srcs = QUERY_TESTS,
+ srcs = [src],
visibility = ["//visibility:public"],
deps = TESTUTIL_DEPS + [
+ ":abstract_query_tests",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//prolog:gerrit-prolog-common",
],
-)
+) for name, src in LUCENE_QUERY_TESTS.items()]
junit_tests(
name = "server_tests",
size = "large",
srcs = glob(
["**/*.java"],
- exclude = CUSTOM_TRUTH_SUBJECTS + PROLOG_TESTS + PROLOG_TEST_CASE + QUERY_TESTS,
+ exclude = CUSTOM_TRUTH_SUBJECTS + PROLOG_TESTS + PROLOG_TEST_CASE + ABSTRACT_QUERY_TESTS + LUCENE_QUERY_TESTS.values(),
),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/server"],
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeBundleTest.java b/javatests/com/google/gerrit/server/notedb/ChangeBundleTest.java
index 0949a44..650262a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeBundleTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeBundleTest.java
@@ -668,6 +668,39 @@
}
@Test
+ public void diffChangesAllowsCreatedToMatchLastUpdated() throws Exception {
+ Change c1 = TestChanges.newChange(new Project.NameKey("project"), new Account.Id(100));
+ c1.setCreatedOn(TimeUtil.nowTs());
+ assertThat(c1.getCreatedOn()).isGreaterThan(c1.getLastUpdatedOn());
+ Change c2 = clone(c1);
+ c2.setCreatedOn(c2.getLastUpdatedOn());
+
+ // Both ReviewDb.
+ ChangeBundle b1 =
+ new ChangeBundle(
+ c1, messages(), patchSets(), approvals(), comments(), reviewers(), REVIEW_DB);
+ ChangeBundle b2 =
+ new ChangeBundle(
+ c2, messages(), patchSets(), approvals(), comments(), reviewers(), REVIEW_DB);
+ assertDiffs(
+ b1,
+ b2,
+ "createdOn differs for Change.Id "
+ + c1.getId()
+ + ": {2009-09-30 17:00:06.0} != {2009-09-30 17:00:00.0}");
+
+ // One NoteDb.
+ b1 =
+ new ChangeBundle(
+ c1, messages(), patchSets(), approvals(), comments(), reviewers(), REVIEW_DB);
+ b2 =
+ new ChangeBundle(
+ c2, messages(), patchSets(), approvals(), comments(), reviewers(), NOTE_DB);
+ assertNoDiffs(b1, b2);
+ assertNoDiffs(b2, b1);
+ }
+
+ @Test
public void diffChangeMessageKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
@@ -1394,6 +1427,90 @@
}
@Test
+ public void diffPatchSetsAllowsFirstPatchSetCreatedOnToMatchChangeCreatedOn() {
+ Change c = TestChanges.newChange(project, accountId);
+ c.setLastUpdatedOn(TimeUtil.nowTs());
+
+ PatchSet goodPs1 = new PatchSet(new PatchSet.Id(c.getId(), 1));
+ goodPs1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ goodPs1.setUploader(accountId);
+ goodPs1.setCreatedOn(TimeUtil.nowTs());
+ assertThat(goodPs1.getCreatedOn()).isGreaterThan(c.getCreatedOn());
+
+ PatchSet ps1AtCreatedOn = clone(goodPs1);
+ ps1AtCreatedOn.setCreatedOn(c.getCreatedOn());
+
+ PatchSet goodPs2 = new PatchSet(new PatchSet.Id(c.getId(), 2));
+ goodPs2.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ goodPs2.setUploader(accountId);
+ goodPs2.setCreatedOn(TimeUtil.nowTs());
+
+ PatchSet ps2AtCreatedOn = clone(goodPs2);
+ ps2AtCreatedOn.setCreatedOn(c.getCreatedOn());
+
+ // Both ReviewDb, exact match required.
+ ChangeBundle b1 =
+ new ChangeBundle(
+ c,
+ messages(),
+ patchSets(goodPs1, goodPs2),
+ approvals(),
+ comments(),
+ reviewers(),
+ REVIEW_DB);
+ ChangeBundle b2 =
+ new ChangeBundle(
+ c,
+ messages(),
+ patchSets(ps1AtCreatedOn, ps2AtCreatedOn),
+ approvals(),
+ comments(),
+ reviewers(),
+ REVIEW_DB);
+ assertDiffs(
+ b1,
+ b2,
+ "createdOn differs for PatchSet.Id "
+ + c.getId()
+ + ",1: {2009-09-30 17:00:12.0} != {2009-09-30 17:00:00.0}",
+ "createdOn differs for PatchSet.Id "
+ + c.getId()
+ + ",2: {2009-09-30 17:00:18.0} != {2009-09-30 17:00:00.0}");
+
+ // One ReviewDb, PS1 is allowed to match change createdOn, but PS2 isn't.
+ b1 =
+ new ChangeBundle(
+ c,
+ messages(),
+ patchSets(goodPs1, goodPs2),
+ approvals(),
+ comments(),
+ reviewers(),
+ REVIEW_DB);
+ b2 =
+ new ChangeBundle(
+ c,
+ messages(),
+ patchSets(ps1AtCreatedOn, ps2AtCreatedOn),
+ approvals(),
+ comments(),
+ reviewers(),
+ NOTE_DB);
+ assertDiffs(
+ b1,
+ b2,
+ "createdOn differs for PatchSet.Id "
+ + c.getId()
+ + ",2 in NoteDb vs. ReviewDb: {2009-09-30 17:00:00.0} != {2009-09-30 17:00:18.0}");
+ assertDiffs(
+ b2,
+ b1,
+ "createdOn differs for PatchSet.Id "
+ + c.getId()
+ + ",2 in NoteDb vs. ReviewDb: {2009-09-30 17:00:00.0} != {2009-09-30 17:00:18.0}");
+ }
+
+ @Test
public void diffPatchSetApprovalKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index e3dcbc5..6b4c5ea 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -129,6 +129,12 @@
setUpDatabase();
}
+ @After
+ public void cleanUp() {
+ lifecycle.stop();
+ db.close();
+ }
+
protected void setUpDatabase() throws Exception {
db = schemaFactory.open();
schemaCreator.create(db);
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index b47c3b1..ee3097e 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -217,6 +217,12 @@
setUpDatabase();
}
+ @After
+ public void cleanUp() {
+ lifecycle.stop();
+ db.close();
+ }
+
protected void setUpDatabase() throws Exception {
try (ReviewDb underlyingDb = inMemoryDatabase.getDatabase().open()) {
schemaCreator.create(underlyingDb);
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index bce151b..b55eee6 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -126,6 +126,12 @@
setUpDatabase();
}
+ @After
+ public void cleanUp() {
+ lifecycle.stop();
+ db.close();
+ }
+
protected void setUpDatabase() throws Exception {
db = schemaFactory.open();
schemaCreator.create(db);
diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
index 5c8114d..dfabd55 100644
--- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
@@ -113,6 +113,12 @@
setUpDatabase();
}
+ @After
+ public void cleanUp() {
+ lifecycle.stop();
+ db.close();
+ }
+
protected void setUpDatabase() throws Exception {
db = schemaFactory.open();
schemaCreator.create(db);
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index 1d964b8..6e655c3 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,12 +1,12 @@
load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar")
-_JGIT_VERS = "4.9.0.201710071750-r"
+_JGIT_VERS = "4.9.0.201710071750-r.30-g651e17bac"
-_DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot
+_DOC_VERS = "4.9.0.201710071750-r" # Set to _JGIT_VERS unless using a snapshot
JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs"
-_JGIT_REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL.
+_JGIT_REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
# set this to use a local version.
# "/home/<user>/projects/jgit"
@@ -26,28 +26,28 @@
name = "jgit_lib",
artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "69d8510b335d4d33d551a133505a4141311f970a",
- src_sha1 = "6fd1eb331447b6163898b4d10aa769e2ac193740",
+ sha1 = "5f0ab69e6dd369c9efcf2fc23f7ae923113d6d3d",
+ src_sha1 = "8dcfff9612815fc1b2bb0e8d032c9b99a6ed89e4",
unsign = True,
)
maven_jar(
name = "jgit_servlet",
artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "93fb0075988b9c6bb97c725c03706f2341965b6b",
+ sha1 = "58629a74278c502f7fb6926b9a4b483da46072b5",
unsign = True,
)
maven_jar(
name = "jgit_archive",
artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "a15aee805c758516ad7e9fa3f16e27bb9f4a1c2e",
+ sha1 = "a31a98bac30977bbad867aa89a0338d3260eb46a",
)
maven_jar(
name = "jgit_junit",
artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "b6e712e743ea5798134f54547ae80456fad07f76",
+ sha1 = "358989980e6faf77c3af955f5eea268020a6407d",
unsign = True,
)
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 8b15880..ba6dbd3 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -445,7 +445,6 @@
patch-num="{{_patchRange.patchNum}}"
base-patch-num="{{_patchRange.basePatchNum}}"
files-expanded="[[_filesExpanded]]"
- revisions="[[_sortedRevisions]]"
on-open-diff-prefs="_handleOpenDiffPrefs"
on-open-download-dialog="_handleOpenDownloadDialog"
on-expand-diffs="_expandAllDiffs"
@@ -458,7 +457,7 @@
patch-range="{{_patchRange}}"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
- revisions="[[_sortedRevisions]]"
+ revisions="[[_change.revisions]]"
project-config="[[_projectConfig]]"
selected-index="{{viewState.selectedFileIndex}}"
diff-view-mode="[[viewState.diffMode]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index c658e8d..21cd100 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -201,7 +201,6 @@
},
/** @type {?number} */
_updateCheckTimerHandle: Number,
- _sortedRevisions: Array,
_editLoaded: {
type: Boolean,
computed: '_computeEditLoaded(_patchRange.*)',
@@ -227,7 +226,6 @@
observers: [
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
- '_updateSortedRevisions(_change.revisions.*)',
],
keyBindings: {
@@ -294,11 +292,6 @@
});
},
- _updateSortedRevisions(revisionsRecord) {
- const revisions = revisionsRecord.base;
- this._sortedRevisions = this.sortRevisions(Object.values(revisions));
- },
-
_handleEditCommitMessage(e) {
this._editingCommitMessage = true;
this.$.commitMessageEditor.focusTextarea();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index b23a568..6c95046 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -56,6 +56,7 @@
_fetchSharedCacheURL() { return Promise.resolve({}); },
});
element = fixture('basic');
+ sandbox.stub(element.$.actions, 'reload').returns(Promise.resolve());
});
teardown(done => {
@@ -66,10 +67,6 @@
});
suite('keyboard shortcuts', () => {
- setup(() => {
- sandbox.stub(element, '_updateSortedRevisions');
- });
-
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
@@ -219,8 +216,6 @@
actions: {},
};
- sandbox.stub(element.$.actions, 'reload');
-
navigateToChangeStub.restore();
navigateToChangeStub = sandbox.stub(Gerrit.Nav, 'navigateToChange',
(change, patchNum, basePatchNum) => {
@@ -448,7 +443,6 @@
});
test('change num change', () => {
- sandbox.stub(element, '_updateSortedRevisions');
element._changeNum = null;
element._patchRange = {
basePatchNum: 'PARENT',
@@ -886,7 +880,6 @@
suite('reply dialog tests', () => {
setup(() => {
sandbox.stub(element.$.replyDialog, '_draftChanged');
- sandbox.stub(element, '_updateSortedRevisions');
sandbox.stub(element.$.replyDialog, 'fetchIsLatestKnown',
() => { return Promise.resolve(true); });
element._change = {labels: {}};
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
index 36cfe06..07db340 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
@@ -41,7 +41,6 @@
patchNum: String,
basePatchNum: String,
filesExpanded: String,
- revisions: Array,
// Caps the number of files that can be shown and have the 'show diffs' /
// 'hide diffs' buttons still be functional.
_maxFilesForBulkActions: {
@@ -144,11 +143,6 @@
});
},
- _computeBasePatchDisabled(patchNum, currentPatchNum) {
- return this.findSortedIndex(patchNum, this.revisions) >=
- this.findSortedIndex(currentPatchNum, this.revisions);
- },
-
_computePrefsButtonHidden(prefs, loggedIn) {
return !loggedIn || !prefs;
},
@@ -158,20 +152,6 @@
return shownFileCount <= maxFilesForBulkActions;
},
- /**
- * Determines if a patch number should be disabled based on value of the
- * basePatchNum from gr-file-list.
- * @param {number} patchNum Patch number available in dropdown
- * @param {number|string} basePatchNum Base patch number from file list
- * @return {boolean}
- */
- _computePatchSetDisabled(patchNum, basePatchNum) {
- if (basePatchNum === 'PARENT') { return false; }
-
- return this.findSortedIndex(patchNum, this.revisions) <=
- this.findSortedIndex(basePatchNum, this.revisions);
- },
-
_handlePatchChange(e) {
const {basePatchNum, patchNum} = e.detail;
if (this.patchNumEquals(basePatchNum, this.basePatchNum) &&
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
index 3a2fd63..1e82984 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
@@ -96,31 +96,6 @@
'Add patchset description');
});
- test('_computePatchSetDisabled', () => {
- element.revisions = [
- {_number: 1},
- {_number: 2},
- {_number: element.EDIT_NAME, basePatchNum: 2},
- {_number: 3},
- ];
- let basePatchNum = 'PARENT';
- let patchNum = 1;
- assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
- false);
- basePatchNum = 1;
- assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
- true);
- patchNum = 2;
- assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
- false);
- basePatchNum = element.EDIT_NAME;
- assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
- true);
- patchNum = '3';
- assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
- false);
- });
-
test('_handleDescriptionChanged', () => {
const putDescStub = sandbox.stub(element.$.restAPI, 'setDescription')
.returns(Promise.resolve({ok: true}));
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index f7b1af9..054f0f1 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -39,7 +39,6 @@
changeNum: String,
comments: Object,
drafts: Object,
- // Already sorted by the change-view.
revisions: Array,
projectConfig: Object,
selectedIndex: {
@@ -119,7 +118,6 @@
type: Boolean,
observer: '_loadingChanged',
},
- _sortedRevisions: Array,
},
behaviors: [
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index 3d287a6..127f6d8 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -153,7 +153,7 @@
}
}
</style>
- <div class="container">
+ <div class="container" tabindex="-1">
<section class="peopleContainer">
<div class="peopleList">
<div class="peopleListLabel">Owner</div>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index ccf0055..e76fecb 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -16,7 +16,10 @@
const RoutePattern = {
ROOT: '/',
- DASHBOARD: '/dashboard/(.*)',
+
+ DASHBOARD: /^\/dashboard\/(.+)$/,
+ CUSTOM_DASHBOARD: /^\/dashboard\/?$/,
+
ADMIN_PLACEHOLDER: '/admin/(.*)',
AGREEMENTS: /^\/settings\/(agreements|new-agreement)/,
REGISTER: /^\/register(\/.*)?$/,
@@ -521,6 +524,9 @@
this._mapRoute(RoutePattern.DASHBOARD, '_handleDashboardRoute');
+ this._mapRoute(RoutePattern.CUSTOM_DASHBOARD,
+ '_handleCustomDashboardRoute');
+
this._mapRoute(RoutePattern.GROUP_INFO, '_handleGroupInfoRoute', true);
this._mapRoute(RoutePattern.GROUP_AUDIT_LOG, '_handleGroupAuditLogRoute',
@@ -715,14 +721,38 @@
},
/**
- * Handle dashboard routes. These may be user, custom, or project
- * dashboards.
+ * Handle dashboard routes. These may be user, or project dashboards.
+ *
+ * @param {!Object} data The parsed route data.
+ */
+ _handleDashboardRoute(data) {
+ // User dashboard. We require viewing user to be logged in, else we
+ // redirect to login for self dashboard or simple owner search for
+ // other user dashboard.
+ return this.$.restAPI.getLoggedIn().then(loggedIn => {
+ if (!loggedIn) {
+ if (data.params[0].toLowerCase() === 'self') {
+ this._redirectToLogin(data.canonicalPath);
+ } else {
+ this._redirect('/q/owner:' + encodeURIComponent(data.params[0]));
+ }
+ } else {
+ this._setParams({
+ view: Gerrit.Nav.View.DASHBOARD,
+ user: data.params[0],
+ });
+ }
+ });
+ },
+
+ /**
+ * Handle custom dashboard routes.
*
* @param {!Object} data The parsed route data.
* @param {string=} opt_qs Optional query string associated with the route.
* If not given, window.location.search is used. (Used by tests).
*/
- _handleDashboardRoute(data, opt_qs) {
+ _handleCustomDashboardRoute(data, opt_qs) {
// opt_qs may be provided by a test, and it may have a falsy value
const qs = opt_qs !== undefined ? opt_qs : window.location.search;
const queryParams = this._parseQueryString(qs);
@@ -745,36 +775,16 @@
// Custom dashboard view.
this._setParams({
view: Gerrit.Nav.View.DASHBOARD,
- user: data.params[0] || 'self',
+ user: 'self',
sections,
title,
});
return Promise.resolve();
}
- if (!data.params[0] && sections.length === 0) {
- // Redirect /dashboard/ -> /dashboard/self.
- this._redirect('/dashboard/self');
- return Promise.resolve();
- }
-
- // User dashboard. We require viewing user to be logged in, else we
- // redirect to login for self dashboard or simple owner search for
- // other user dashboard.
- return this.$.restAPI.getLoggedIn().then(loggedIn => {
- if (!loggedIn) {
- if (data.params[0].toLowerCase() === 'self') {
- this._redirectToLogin(data.canonicalPath);
- } else {
- this._redirect('/q/owner:' + encodeURIComponent(data.params[0]));
- }
- } else {
- this._setParams({
- view: Gerrit.Nav.View.DASHBOARD,
- user: data.params[0],
- });
- }
- });
+ // Redirect /dashboard/ -> /dashboard/self.
+ this._redirect('/dashboard/self');
+ return Promise.resolve();
},
_handleGroupInfoRoute(data) {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index 03b6bf2..b6ee5b5 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -172,6 +172,7 @@
// it performed for them.
const selfAuthenticatingHandlers = [
'_handleDashboardRoute',
+ '_handleCustomDashboardRoute',
'_handleRootRoute',
];
@@ -738,16 +739,6 @@
redirectToLoginStub = sandbox.stub(element, '_redirectToLogin');
});
- test('no user specified', () => {
- const data = {canonicalPath: '/dashboard/', params: {0: ''}};
- return element._handleDashboardRoute(data, '').then(() => {
- assert.isFalse(setParamsStub.called);
- assert.isFalse(redirectToLoginStub.called);
- assert.isTrue(redirectStub.called);
- assert.equal(redirectStub.lastCall.args[0], '/dashboard/self');
- });
- });
-
test('own dashboard but signed out redirects to login', () => {
sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(false));
@@ -785,28 +776,46 @@
});
});
});
+ });
+
+ suite('_handleCustomDashboardRoute', () => {
+ let redirectToLoginStub;
+
+ setup(() => {
+ redirectToLoginStub = sandbox.stub(element, '_redirectToLogin');
+ });
+
+ test('no user specified', () => {
+ const data = {canonicalPath: '/dashboard/', params: {0: ''}};
+ return element._handleCustomDashboardRoute(data, '').then(() => {
+ assert.isFalse(setParamsStub.called);
+ assert.isTrue(redirectStub.called);
+ assert.equal(redirectStub.lastCall.args[0], '/dashboard/self');
+ });
+ });
test('custom dashboard without title', () => {
const data = {canonicalPath: '/dashboard/', params: {0: ''}};
- return element._handleDashboardRoute(data, '?a=b&c&d=e').then(() => {
- assert.isFalse(redirectToLoginStub.called);
- assert.isFalse(redirectStub.called);
- assert.isTrue(setParamsStub.calledOnce);
- assert.deepEqual(setParamsStub.lastCall.args[0], {
- view: Gerrit.Nav.View.DASHBOARD,
- user: 'self',
- sections: [
- {name: 'a', query: 'b'},
- {name: 'd', query: 'e'},
- ],
- title: 'Custom Dashboard',
- });
- });
+ return element._handleCustomDashboardRoute(data, '?a=b&c&d=e')
+ .then(() => {
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(setParamsStub.calledOnce);
+ assert.deepEqual(setParamsStub.lastCall.args[0], {
+ view: Gerrit.Nav.View.DASHBOARD,
+ user: 'self',
+ sections: [
+ {name: 'a', query: 'b'},
+ {name: 'd', query: 'e'},
+ ],
+ title: 'Custom Dashboard',
+ });
+ });
});
test('custom dashboard with title', () => {
const data = {canonicalPath: '/dashboard/', params: {0: ''}};
- return element._handleDashboardRoute(data, '?a=b&c&d=&=e&title=t')
+ return element._handleCustomDashboardRoute(data,
+ '?a=b&c&d=&=e&title=t')
.then(() => {
assert.isFalse(redirectToLoginStub.called);
assert.isFalse(redirectStub.called);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index d58b6be..335ed36 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -133,6 +133,7 @@
.editing .ack,
.editing .done,
.editing .edit,
+ .editing .discard,
.editing .unresolved {
display: none;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index 8b6ec5e..1dc6c45 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -351,7 +351,7 @@
element.editing = true;
assert.isFalse(isVisible(element.$$('.edit')), 'edit is not visible');
- assert.isTrue(isVisible(element.$$('.discard')), 'discard is visible');
+ assert.isFalse(isVisible(element.$$('.discard')), 'discard not visible');
assert.isTrue(isVisible(element.$$('.save')), 'save is visible');
assert.isFalse(isVisible(element.$$('.cancel')), 'cancel is visible');
assert.isTrue(isVisible(element.$$('.resolve')), 'resolve is visible');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index e896d10..a080396 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -148,10 +148,12 @@
overflow: auto;
}
#trigger {
- -moz-user-select: text;
- -ms-user-select: text;
- -webkit-user-select: text;
- user-select: text;
+ --gr-button: {
+ -moz-user-select: text;
+ -ms-user-select: text;
+ -webkit-user-select: text;
+ user-select: text;
+ }
}
.editLoaded .hideOnEdit {
display: none;
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
index ad25c0d..62a4785 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
@@ -37,11 +37,15 @@
},
},
- _handleEditTap() {
+ _handleEditTap(e) {
+ e.preventDefault();
+ e.stopPropagation();
this._dispatchFileAction(GrEditConstants.Actions.EDIT.id, this.filePath);
},
_handleActionTap(e) {
+ e.preventDefault();
+ e.stopPropagation();
this._dispatchFileAction(e.detail.id, this.filePath);
},
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
index 49e5a5e..90de1ea 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -162,6 +162,8 @@
* @param {!Event} e
*/
_showDropdownTapHandler(e) {
+ e.preventDefault();
+ e.stopPropagation();
this._open();
},
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index 12197b7..e63e739 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -154,7 +154,7 @@
src.add(m.group(1))
# Exceptions: both source and lib
if p.endswith('libquery_parser.jar') or \
- p.endswith('prolog/libcommon.jar'):
+ p.endswith('libgerrit-prolog-common.jar'):
lib.add(p)
# JGit dependency from external repository
if 'gerrit-' not in p and 'jgit' in p:
