Merge "Disable typescript-eslint/no-explicit-any warnings"
diff --git a/java/com/google/gerrit/entities/Patch.java b/java/com/google/gerrit/entities/Patch.java
index e6b2167..856765b 100644
--- a/java/com/google/gerrit/entities/Patch.java
+++ b/java/com/google/gerrit/entities/Patch.java
@@ -160,5 +160,40 @@
}
}
+ /**
+ * Constants describing various file modes recognized by GIT. This is the Gerrit entity for {@link
+ * org.eclipse.jgit.lib.FileMode}.
+ */
+ public enum FileMode implements CodedEnum {
+ /** Mode indicating an entry is a tree (aka directory). */
+ TREE('T'),
+
+ /** Mode indicating an entry is a symbolic link. */
+ SYMLINK('S'),
+
+ /** Mode indicating an entry is a non-executable file. */
+ REGULAR_FILE('R'),
+
+ /** Mode indicating an entry is an executable file. */
+ EXECUTABLE_FILE('E'),
+
+ /** Mode indicating an entry is a submodule commit in another repository. */
+ GITLINK('G'),
+
+ /** Mode indicating an entry is missing during parallel walks. */
+ MISSING('M');
+
+ private final char code;
+
+ FileMode(char c) {
+ code = c;
+ }
+
+ @Override
+ public char getCode() {
+ return code;
+ }
+ }
+
private Patch() {}
}
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index c17cd97..f1da6b7 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -123,9 +123,7 @@
options.add(ListChangesOption.LABELS);
options.add(ListChangesOption.DETAILED_ACCOUNTS);
- if (isEnabledAttentionSet(serverApi)) {
- options.add(ListChangesOption.DETAILED_LABELS);
- } else {
+ if (!isEnabledAttentionSet(serverApi)) {
options.add(ListChangesOption.REVIEWED);
}
return ListOption.toHex(options);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index d48cbc4..b816264 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -64,6 +64,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -460,6 +461,11 @@
return state.updateCount();
}
+ /** @return {@link Optional} value of time when the change was merged. */
+ public Optional<Timestamp> getMergedOn() {
+ return Optional.ofNullable(state.mergedOn());
+ }
+
public ImmutableListMultimap<ObjectId, HumanComment> getDraftComments(Account.Id author) {
return getDraftComments(author, null);
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 220e683..c554ca5 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -191,7 +191,8 @@
+ list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
- + I; // updateCount
+ + I // updateCount
+ + T; // mergedOn
}
private static int str(String s) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index fae29f8..6403b95 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -156,6 +156,7 @@
private Change.Id revertOf;
private int updateCount;
private PatchSet.Id cherryPickOf;
+ private Timestamp mergedOn;
ChangeNotesParser(
Change.Id changeId,
@@ -259,7 +260,8 @@
firstNonNull(hasReviewStarted, true),
revertOf,
cherryPickOf,
- updateCount);
+ updateCount,
+ mergedOn);
}
private Map<PatchSet.Id, PatchSet> buildPatchSets() throws ConfigInvalidException {
@@ -322,9 +324,9 @@
private void parse(ChangeNotesCommit commit) throws ConfigInvalidException {
updateCount++;
- Timestamp ts = new Timestamp(commit.getCommitterIdent().getWhen().getTime());
+ Timestamp commitTimestamp = getCommitTimestamp(commit);
- createdOn = ts;
+ createdOn = commitTimestamp;
parseTag(commit);
if (branch == null) {
@@ -364,21 +366,19 @@
originalSubject = currSubject;
}
- parseChangeMessage(psId, accountId, realAccountId, commit, ts);
+ parseChangeMessage(psId, accountId, realAccountId, commit, commitTimestamp);
if (topic == null) {
topic = parseTopic(commit);
}
parseHashtags(commit);
parseAttentionSetUpdates(commit);
- parseAssigneeUpdates(ts, commit);
+ parseAssigneeUpdates(commitTimestamp, commit);
- if (submissionId == null) {
- submissionId = parseSubmissionId(commit);
- }
+ parseSubmission(commit, commitTimestamp);
- if (lastUpdatedOn == null || ts.after(lastUpdatedOn)) {
- lastUpdatedOn = ts;
+ if (lastUpdatedOn == null || commitTimestamp.after(lastUpdatedOn)) {
+ lastUpdatedOn = commitTimestamp;
}
if (deletedPatchSets.contains(psId)) {
@@ -392,16 +392,10 @@
ObjectId currRev = parseRevision(commit);
if (currRev != null) {
- parsePatchSet(psId, currRev, accountId, ts);
+ parsePatchSet(psId, currRev, accountId, commitTimestamp);
}
parseCurrentPatchSet(psId, commit);
- if (submitRecords.isEmpty()) {
- // Only parse the most recent set of submit records; any older ones are
- // still there, but not currently used.
- parseSubmitRecords(commit.getFooterLineValues(FOOTER_SUBMITTED_WITH));
- }
-
if (status == null) {
status = parseStatus(commit);
}
@@ -409,15 +403,15 @@
// Parse approvals after status to treat approvals in the same commit as
// "Status: merged" as non-post-submit.
for (String line : commit.getFooterLineValues(FOOTER_LABEL)) {
- parseApproval(psId, accountId, realAccountId, ts, line);
+ parseApproval(psId, accountId, realAccountId, commitTimestamp, line);
}
for (ReviewerStateInternal state : ReviewerStateInternal.values()) {
for (String line : commit.getFooterLineValues(state.getFooterKey())) {
- parseReviewer(ts, state, line);
+ parseReviewer(commitTimestamp, state, line);
}
for (String line : commit.getFooterLineValues(state.getByEmailFooterKey())) {
- parseReviewerByEmail(ts, state, line);
+ parseReviewerByEmail(commitTimestamp, state, line);
}
// Don't update timestamp when a reviewer was added, matching RevewDb
// behavior.
@@ -439,6 +433,24 @@
parseWorkInProgress(commit);
}
+ private void parseSubmission(ChangeNotesCommit commit, Timestamp commitTimestamp)
+ throws ConfigInvalidException {
+ // Only parse the most recent sumbit commit (there should be exactly one).
+ if (submissionId == null) {
+ submissionId = parseSubmissionId(commit);
+ }
+
+ if (submissionId != null && mergedOn == null) {
+ mergedOn = commitTimestamp;
+ }
+
+ if (submitRecords.isEmpty()) {
+ // Only parse the most recent set of submit records; any older ones are
+ // still there, but not currently used.
+ parseSubmitRecords(commit.getFooterLineValues(FOOTER_SUBMITTED_WITH));
+ }
+ }
+
private String parseSubmissionId(ChangeNotesCommit commit) throws ConfigInvalidException {
return parseOneFooter(commit, FOOTER_SUBMISSION_ID);
}
@@ -1016,6 +1028,23 @@
}
}
+ /**
+ * Returns the {@link Timestamp} when the commit was applied.
+ *
+ * <p>The author's date only notes when the commit was originally made. Thus, use the commiter's
+ * date as it accounts for the rebase, cherry-pick, commit --amend and other commands that rewrite
+ * the history of the branch.
+ *
+ * <p>Don't use {@link org.eclipse.jgit.revwalk.RevCommit#getCommitTime} directly because it
+ * returns int and would overflow.
+ *
+ * @param commit the commit to return commit time.
+ * @return the timestamp when the commit was applied.
+ */
+ private Timestamp getCommitTimestamp(ChangeNotesCommit commit) {
+ return new Timestamp(commit.getCommitterIdent().getWhen().getTime());
+ }
+
private void pruneReviewers() {
Iterator<Table.Cell<Account.Id, ReviewerStateInternal, Timestamp>> rit =
reviewers.cellSet().iterator();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 27cfb70..af8c8c8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -136,7 +136,8 @@
boolean reviewStarted,
@Nullable Change.Id revertOf,
@Nullable PatchSet.Id cherryPickOf,
- int updateCount) {
+ int updateCount,
+ @Nullable Timestamp mergedOn) {
requireNonNull(
metaId,
() ->
@@ -184,6 +185,7 @@
.changeMessages(changeMessages)
.publishedComments(publishedComments)
.updateCount(updateCount)
+ .mergedOn(mergedOn)
.build();
}
@@ -329,6 +331,9 @@
abstract int updateCount();
+ @Nullable
+ abstract Timestamp mergedOn();
+
Change newChange(Project.NameKey project) {
ChangeColumns c = requireNonNull(columns(), "columns are required");
Change change =
@@ -445,6 +450,8 @@
abstract Builder updateCount(int updateCount);
+ abstract Builder mergedOn(Timestamp mergedOn);
+
abstract ChangeNotesState build();
}
@@ -515,6 +522,10 @@
.forEach(m -> b.addChangeMessage(toByteString(m, ChangeMessageProtoConverter.INSTANCE)));
object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c)));
b.setUpdateCount(object.updateCount());
+ if (object.mergedOn() != null) {
+ b.setMergedOnMillis(object.mergedOn().getTime());
+ b.setHasMergedOn(true);
+ }
return Protos.toByteArray(b.build());
}
@@ -655,7 +666,8 @@
proto.getPublishedCommentList().stream()
.map(r -> GSON.fromJson(r, HumanComment.class))
.collect(toImmutableListMultimap(HumanComment::getCommitId, c -> c)))
- .updateCount(proto.getUpdateCount());
+ .updateCount(proto.getUpdateCount())
+ .mergedOn(proto.getHasMergedOn() ? new Timestamp(proto.getMergedOnMillis()) : null);
return b.build();
}
diff --git a/java/com/google/gerrit/server/patch/DiffExecutor.java b/java/com/google/gerrit/server/patch/DiffExecutor.java
index 072c2da..63d5c50 100644
--- a/java/com/google/gerrit/server/patch/DiffExecutor.java
+++ b/java/com/google/gerrit/server/patch/DiffExecutor.java
@@ -16,6 +16,7 @@
import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import com.google.gerrit.server.patch.filediff.PatchListLoader;
import com.google.inject.BindingAnnotation;
import java.lang.annotation.Retention;
import java.util.concurrent.ExecutorService;
diff --git a/java/com/google/gerrit/server/patch/DiffMappings.java b/java/com/google/gerrit/server/patch/DiffMappings.java
index a6f026e..57132f8 100644
--- a/java/com/google/gerrit/server/patch/DiffMappings.java
+++ b/java/com/google/gerrit/server/patch/DiffMappings.java
@@ -23,8 +23,8 @@
import com.google.gerrit.server.patch.GitPositionTransformer.Mapping;
import com.google.gerrit.server.patch.GitPositionTransformer.Range;
import com.google.gerrit.server.patch.GitPositionTransformer.RangeMapping;
-import com.google.gerrit.server.patch.entities.Edit;
-import com.google.gerrit.server.patch.entities.FileEdits;
+import com.google.gerrit.server.patch.filediff.Edit;
+import com.google.gerrit.server.patch.filediff.FileEdits;
import java.util.List;
/** Mappings derived from diffs. */
diff --git a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index b663b9d..a3e9a54 100644
--- a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.patch.filediff.PatchListLoader;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
@@ -37,7 +38,7 @@
/** Provides a cached list of {@link PatchListEntry}. */
@Singleton
public class PatchListCacheImpl implements PatchListCache {
- static final String FILE_NAME = "diff";
+ public static final String FILE_NAME = "diff";
static final String INTRA_NAME = "diff_intraline";
static final String DIFF_SUMMARY = "diff_summary";
diff --git a/java/com/google/gerrit/server/patch/PatchListEntry.java b/java/com/google/gerrit/server/patch/PatchListEntry.java
index c91355a..1d1d0ea 100644
--- a/java/com/google/gerrit/server/patch/PatchListEntry.java
+++ b/java/com/google/gerrit/server/patch/PatchListEntry.java
@@ -77,7 +77,7 @@
// Note: When adding new fields, the serialVersionUID in PatchListKey must be
// incremented so that entries from the cache are automatically invalidated.
- PatchListEntry(
+ public PatchListEntry(
FileHeader hdr, List<Edit> editList, Set<Edit> editsDueToRebase, long size, long sizeDelta) {
changeType = toChangeType(hdr);
patchType = toPatchType(hdr);
diff --git a/java/com/google/gerrit/server/patch/entities/Edit.java b/java/com/google/gerrit/server/patch/filediff/Edit.java
similarity index 92%
rename from java/com/google/gerrit/server/patch/entities/Edit.java
rename to java/com/google/gerrit/server/patch/filediff/Edit.java
index 683bbec..b1b336e 100644
--- a/java/com/google/gerrit/server/patch/entities/Edit.java
+++ b/java/com/google/gerrit/server/patch/filediff/Edit.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.patch.entities;
+package com.google.gerrit.server.patch.filediff;
import com.google.auto.value.AutoValue;
@@ -23,7 +23,7 @@
*/
@AutoValue
public abstract class Edit {
- static Edit create(int beginA, int endA, int beginB, int endB) {
+ public static Edit create(int beginA, int endA, int beginB, int endB) {
return new AutoValue_Edit(beginA, endA, beginB, endB);
}
diff --git a/java/com/google/gerrit/server/patch/EditTransformer.java b/java/com/google/gerrit/server/patch/filediff/EditTransformer.java
similarity index 98%
rename from java/com/google/gerrit/server/patch/EditTransformer.java
rename to java/com/google/gerrit/server/patch/filediff/EditTransformer.java
index 06e6f72..55568e4 100644
--- a/java/com/google/gerrit/server/patch/EditTransformer.java
+++ b/java/com/google/gerrit/server/patch/filediff/EditTransformer.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.patch;
+package com.google.gerrit.server.patch.filediff;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
@@ -25,13 +25,13 @@
import com.google.common.collect.Multimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Patch;
+import com.google.gerrit.server.patch.DiffMappings;
+import com.google.gerrit.server.patch.GitPositionTransformer;
import com.google.gerrit.server.patch.GitPositionTransformer.Mapping;
import com.google.gerrit.server.patch.GitPositionTransformer.OmitPositionOnConflict;
import com.google.gerrit.server.patch.GitPositionTransformer.Position;
import com.google.gerrit.server.patch.GitPositionTransformer.PositionedEntity;
import com.google.gerrit.server.patch.GitPositionTransformer.Range;
-import com.google.gerrit.server.patch.entities.Edit;
-import com.google.gerrit.server.patch.entities.FileEdits;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
diff --git a/java/com/google/gerrit/server/patch/entities/FileEdits.java b/java/com/google/gerrit/server/patch/filediff/FileEdits.java
similarity index 96%
rename from java/com/google/gerrit/server/patch/entities/FileEdits.java
rename to java/com/google/gerrit/server/patch/filediff/FileEdits.java
index c914919..c51cc45 100644
--- a/java/com/google/gerrit/server/patch/entities/FileEdits.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileEdits.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.patch.entities;
+package com.google.gerrit.server.patch.filediff;
import static com.google.common.collect.ImmutableList.toImmutableList;
diff --git a/java/com/google/gerrit/server/patch/PatchListLoader.java b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
similarity index 94%
rename from java/com/google/gerrit/server/patch/PatchListLoader.java
rename to java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
index df34aa6..5b7eddb 100644
--- a/java/com/google/gerrit/server/patch/PatchListLoader.java
+++ b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
@@ -1,19 +1,21 @@
-// Copyright (C) 2009 The Android Open Source Project
+// Copyright (C) 2020 The Android Open Source Project
//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+//
//
-package com.google.gerrit.server.patch;
+package com.google.gerrit.server.patch.filediff;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
@@ -38,8 +40,17 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.patch.EditTransformer.ContextAwareEdit;
-import com.google.gerrit.server.patch.entities.FileEdits;
+import com.google.gerrit.server.patch.AutoMerger;
+import com.google.gerrit.server.patch.ComparisonType;
+import com.google.gerrit.server.patch.DiffExecutor;
+import com.google.gerrit.server.patch.PatchList;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListCacheImpl;
+import com.google.gerrit.server.patch.PatchListEntry;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.patch.Text;
+import com.google.gerrit.server.patch.filediff.EditTransformer.ContextAwareEdit;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
@@ -82,7 +93,7 @@
import org.eclipse.jgit.util.io.DisabledOutputStream;
public class PatchListLoader implements Callable<PatchList> {
- static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ public static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
PatchListLoader create(PatchListKey key, Project.NameKey project);
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
new file mode 100644
index 0000000..9827a69
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
@@ -0,0 +1,169 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.Patch.PatchType;
+import java.util.Optional;
+import org.eclipse.jgit.patch.CombinedFileHeader;
+import org.eclipse.jgit.patch.FileHeader;
+
+/** A utility class for the {@link FileHeader} JGit object */
+public class FileHeaderUtil {
+ private static final Byte NUL = '\0';
+
+ /**
+ * The maximum number of characters to lookup in the binary file {@link FileHeader}. This is used
+ * to scan the file header for the occurrence of the {@link #NUL} character.
+ *
+ * <p>This limit assumes a uniform distribution of all characters, hence the probability of the
+ * occurrence of each character = (1 / 256). We want to find the limit that makes the prob. of
+ * finding {@link #NUL} > 0.999. 1 - (255 / 256) ^ N > 0.999 yields N = 1766. We set the limit to
+ * this value multiplied by 10 for more confidence.
+ */
+ private static final int BIN_FILE_MAX_SCAN_LIMIT = 20000;
+
+ /** Converts the {@link FileHeader} parameter ot a String representation. */
+ static String toString(FileHeader header) {
+ return new String(FileHeaderUtil.toByteArray(header), UTF_8);
+ }
+
+ /** Converts the {@link FileHeader} parameter to a byte array. */
+ static byte[] toByteArray(FileHeader header) {
+ int end = getEndOffset(header);
+ if (header.getStartOffset() == 0 && end == header.getBuffer().length) {
+ return header.getBuffer();
+ }
+
+ final byte[] buf = new byte[end - header.getStartOffset()];
+ System.arraycopy(header.getBuffer(), header.getStartOffset(), buf, 0, buf.length);
+ return buf;
+ }
+
+ /**
+ * Returns the old file path associated with the {@link FileHeader}, or empty if the file is
+ * {@link Patch.ChangeType#ADDED} or {@link Patch.ChangeType#REWRITE}.
+ */
+ static Optional<String> getOldPath(FileHeader header) {
+ Patch.ChangeType changeType = getChangeType(header);
+ switch (changeType) {
+ case DELETED:
+ case COPIED:
+ case RENAMED:
+ case MODIFIED:
+ return Optional.of(header.getOldPath());
+
+ case ADDED:
+ case REWRITE:
+ return Optional.empty();
+ }
+ return Optional.empty();
+ }
+
+ /**
+ * Returns the new file path associated with the {@link FileHeader}, or empty if the file is
+ * {@link Patch.ChangeType#DELETED}.
+ */
+ static Optional<String> getNewPath(FileHeader header) {
+ Patch.ChangeType changeType = getChangeType(header);
+ switch (changeType) {
+ case DELETED:
+ return Optional.empty();
+
+ case ADDED:
+ case MODIFIED:
+ case REWRITE:
+ case COPIED:
+ case RENAMED:
+ return Optional.of(header.getNewPath());
+ }
+ return Optional.empty();
+ }
+
+ /** Returns the change type associated with the file header. */
+ static Patch.ChangeType getChangeType(FileHeader header) {
+ // In Gerrit, we define our own entities of the JGit entities, so that we have full control
+ // over their behaviors (e.g. making sure that these entities are immutable so that we can add
+ // them as fields of keys / values of persisted caches).
+
+ // TODO(ghareeb): remove the dead code of the value REWRITE and all its handling
+ switch (header.getChangeType()) {
+ case ADD:
+ return Patch.ChangeType.ADDED;
+ case MODIFY:
+ return Patch.ChangeType.MODIFIED;
+ case DELETE:
+ return Patch.ChangeType.DELETED;
+ case RENAME:
+ return Patch.ChangeType.RENAMED;
+ case COPY:
+ return Patch.ChangeType.COPIED;
+ default:
+ throw new IllegalArgumentException("Unsupported type " + header.getChangeType());
+ }
+ }
+
+ static PatchType getPatchType(FileHeader header) {
+ PatchType patchType;
+
+ switch (header.getPatchType()) {
+ case UNIFIED:
+ patchType = Patch.PatchType.UNIFIED;
+ break;
+ case GIT_BINARY:
+ case BINARY:
+ patchType = Patch.PatchType.BINARY;
+ break;
+ default:
+ throw new IllegalArgumentException("Unsupported type " + header.getPatchType());
+ }
+
+ if (patchType != PatchType.BINARY) {
+ byte[] buf = header.getBuffer();
+ // TODO(ghareeb): should we adjust the max limit threshold?
+ // JGit sometimes misses the detection of binary files. In this case we look into the file
+ // header for the occurrence of NUL characters, which is a definite signal that the file is
+ // binary. We limit the number of characters to lookup to avoid performance bottlenecks.
+ for (int ptr = header.getStartOffset();
+ ptr < Math.min(header.getEndOffset(), BIN_FILE_MAX_SCAN_LIMIT);
+ ptr++) {
+ if (buf[ptr] == NUL) {
+ // It's really binary, but Git couldn't see the nul early enough to realize its binary,
+ // and instead produced the diff.
+ //
+ // Force it to be a binary; it really should have been that.
+ return PatchType.BINARY;
+ }
+ }
+ }
+ return patchType;
+ }
+
+ /**
+ * Returns the end offset of the diff header line of the {@code FileHeader parameter} before the
+ * appearance of any file edits (diff hunks).
+ */
+ private static int getEndOffset(FileHeader fileHeader) {
+ if (fileHeader instanceof CombinedFileHeader) {
+ return fileHeader.getEndOffset();
+ }
+ if (!fileHeader.getHunks().isEmpty()) {
+ return fileHeader.getHunks().get(0).getStartOffset();
+ }
+ return fileHeader.getEndOffset();
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
new file mode 100644
index 0000000..bce3d44
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
@@ -0,0 +1,305 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.Patch.ChangeType;
+import com.google.gerrit.entities.Patch.PatchType;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.server.cache.proto.Cache.GitFileDiffProto;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.patch.filediff.Edit;
+import com.google.protobuf.Descriptors.FieldDescriptor;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.diff.DiffEntry;
+import org.eclipse.jgit.diff.DiffFormatter;
+import org.eclipse.jgit.lib.AbbreviatedObjectId;
+import org.eclipse.jgit.lib.FileMode;
+import org.eclipse.jgit.patch.FileHeader;
+
+/**
+ * Entity representing a modified file (added, deleted, modified, renamed, etc...) between two
+ * different git commits.
+ */
+@AutoValue
+public abstract class GitFileDiff {
+ private static final Map<FileMode, Patch.FileMode> fileModeMap =
+ ImmutableMap.of(
+ FileMode.TREE,
+ Patch.FileMode.TREE,
+ FileMode.SYMLINK,
+ Patch.FileMode.SYMLINK,
+ FileMode.REGULAR_FILE,
+ Patch.FileMode.REGULAR_FILE,
+ FileMode.EXECUTABLE_FILE,
+ Patch.FileMode.EXECUTABLE_FILE,
+ FileMode.MISSING,
+ Patch.FileMode.MISSING);
+
+ private static Patch.FileMode mapFileMode(FileMode jgitFileMode) {
+ if (!fileModeMap.containsKey(jgitFileMode)) {
+ throw new IllegalArgumentException("Unsupported type " + jgitFileMode);
+ }
+ return fileModeMap.get(jgitFileMode);
+ }
+
+ /**
+ * Creates a {@link GitFileDiff} using the {@code diffEntry} and the {@code diffFormatter}
+ * parameters.
+ */
+ static GitFileDiff create(DiffEntry diffEntry, DiffFormatter diffFormatter) throws IOException {
+ FileHeader fileHeader = diffFormatter.toFileHeader(diffEntry);
+ ImmutableList<Edit> edits =
+ fileHeader.toEditList().stream().map(Edit::fromJGitEdit).collect(toImmutableList());
+
+ return builder()
+ .edits(edits)
+ .oldId(diffEntry.getOldId())
+ .newId(diffEntry.getNewId())
+ .fileHeader(FileHeaderUtil.toString(fileHeader))
+ .oldPath(FileHeaderUtil.getOldPath(fileHeader))
+ .newPath(FileHeaderUtil.getNewPath(fileHeader))
+ .changeType(Optional.of(FileHeaderUtil.getChangeType(fileHeader)))
+ .patchType(Optional.of(FileHeaderUtil.getPatchType(fileHeader)))
+ .oldMode(Optional.of(mapFileMode(diffEntry.getOldMode())))
+ .newMode(Optional.of(mapFileMode(diffEntry.getNewMode())))
+ .build();
+ }
+
+ /**
+ * Represents an empty file diff, which means that the file was not modified between the two git
+ * trees identified by {@link #oldId()} and {@link #newId()}.
+ *
+ * @param newFilePath the file name at the {@link #newId()} git tree.
+ */
+ static GitFileDiff empty(
+ AbbreviatedObjectId oldId, AbbreviatedObjectId newId, String newFilePath) {
+ return builder()
+ .oldId(oldId)
+ .newId(newId)
+ .newPath(Optional.of(newFilePath))
+ .edits(ImmutableList.of())
+ .fileHeader("")
+ .build();
+ }
+
+ /** An {@link ImmutableList} of the modified regions in the file. */
+ public abstract ImmutableList<Edit> edits();
+
+ /** A string representation of the {@link org.eclipse.jgit.patch.FileHeader}. */
+ public abstract String fileHeader();
+
+ /** The file name at the old git tree identified by {@link #oldId()} */
+ public abstract Optional<String> oldPath();
+
+ /** The file name at the new git tree identified by {@link #newId()} */
+ public abstract Optional<String> newPath();
+
+ /** The 20 bytes SHA-1 object ID of the old git tree of the diff. */
+ public abstract AbbreviatedObjectId oldId();
+
+ /** The 20 bytes SHA-1 object ID of the new git tree of the diff. */
+ public abstract AbbreviatedObjectId newId();
+
+ /** The file mode of the old file at the old git tree diff identified by {@link #oldId()}. */
+ public abstract Optional<Patch.FileMode> oldMode();
+
+ /** The file mode of the new file at the new git tree diff identified by {@link #newId()}. */
+ public abstract Optional<Patch.FileMode> newMode();
+
+ /** The change type associated with the file. */
+ public abstract Optional<ChangeType> changeType();
+
+ /** The patch type associated with the file. */
+ public abstract Optional<PatchType> patchType();
+
+ /**
+ * Returns true if the object was created using the {@link #empty(AbbreviatedObjectId,
+ * AbbreviatedObjectId, String)} method.
+ */
+ public boolean isEmpty() {
+ return edits().isEmpty();
+ }
+
+ /** Returns the size of the object in bytes. */
+ public int weight() {
+ int result = 20 * 2; // oldId and newId
+ result += 16 * edits().size(); // each edit contains 4 integers (hence 16 bytes)
+ result += stringSize(fileHeader());
+ if (oldPath().isPresent()) {
+ result += stringSize(oldPath().get());
+ }
+ if (newPath().isPresent()) {
+ result += stringSize(newPath().get());
+ }
+ if (changeType().isPresent()) {
+ result += 4;
+ }
+ if (patchType().isPresent()) {
+ result += 4;
+ }
+ if (oldMode().isPresent()) {
+ result += 4;
+ }
+ if (newMode().isPresent()) {
+ result += 4;
+ }
+ return result;
+ }
+
+ private static int stringSize(String str) {
+ if (str != null) {
+ // each character in the string occupies two bytes. Ignoring the fixed overhead for the string
+ // (length, offset and hash code) since they are negligible and do not affect the comparison
+ // of two strings
+ return str.length() * 2;
+ }
+ return 0;
+ }
+
+ public static Builder builder() {
+ return new AutoValue_GitFileDiff.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder edits(ImmutableList<Edit> value);
+
+ public abstract Builder fileHeader(String value);
+
+ public abstract Builder oldPath(Optional<String> value);
+
+ public abstract Builder newPath(Optional<String> value);
+
+ public abstract Builder oldId(AbbreviatedObjectId value);
+
+ public abstract Builder newId(AbbreviatedObjectId value);
+
+ public abstract Builder oldMode(Optional<Patch.FileMode> value);
+
+ public abstract Builder newMode(Optional<Patch.FileMode> value);
+
+ public abstract Builder changeType(Optional<ChangeType> value);
+
+ public abstract Builder patchType(Optional<PatchType> value);
+
+ public abstract GitFileDiff build();
+ }
+
+ public enum Serializer implements CacheSerializer<GitFileDiff> {
+ INSTANCE;
+
+ private static final FieldDescriptor OLD_PATH_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("old_path");
+
+ private static final FieldDescriptor NEW_PATH_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("new_path");
+
+ private static final FieldDescriptor OLD_MODE_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("old_mode");
+
+ private static final FieldDescriptor NEW_MODE_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("new_mode");
+
+ private static final FieldDescriptor CHANGE_TYPE_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("change_type");
+
+ private static final FieldDescriptor PATCH_TYPE_DESCRIPTOR =
+ GitFileDiffProto.getDescriptor().findFieldByName("patch_type");
+
+ @Override
+ public byte[] serialize(GitFileDiff gitFileDiff) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ GitFileDiffProto.Builder builder =
+ GitFileDiffProto.newBuilder()
+ .setFileHeader(gitFileDiff.fileHeader())
+ .setOldId(idConverter.toByteString(gitFileDiff.oldId().toObjectId()))
+ .setNewId(idConverter.toByteString(gitFileDiff.newId().toObjectId()));
+ gitFileDiff
+ .edits()
+ .forEach(
+ e ->
+ builder.addEdits(
+ GitFileDiffProto.Edit.newBuilder()
+ .setBeginA(e.beginA())
+ .setEndA(e.endA())
+ .setBeginB(e.beginB())
+ .setEndB(e.endB())));
+ if (gitFileDiff.oldPath().isPresent()) {
+ builder.setOldPath(gitFileDiff.oldPath().get());
+ }
+ if (gitFileDiff.newPath().isPresent()) {
+ builder.setNewPath(gitFileDiff.newPath().get());
+ }
+ if (gitFileDiff.oldMode().isPresent()) {
+ builder.setOldMode(gitFileDiff.oldMode().get().name());
+ }
+ if (gitFileDiff.newMode().isPresent()) {
+ builder.setNewMode(gitFileDiff.newMode().get().name());
+ }
+ if (gitFileDiff.changeType().isPresent()) {
+ builder.setChangeType(gitFileDiff.changeType().get().name());
+ }
+ if (gitFileDiff.patchType().isPresent()) {
+ builder.setPatchType(gitFileDiff.patchType().get().name());
+ }
+ return Protos.toByteArray(builder.build());
+ }
+
+ @Override
+ public GitFileDiff deserialize(byte[] in) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ GitFileDiffProto proto = Protos.parseUnchecked(GitFileDiffProto.parser(), in);
+ GitFileDiff.Builder builder = GitFileDiff.builder();
+ builder
+ .edits(
+ proto.getEditsList().stream()
+ .map(e -> Edit.create(e.getBeginA(), e.getEndA(), e.getBeginB(), e.getEndB()))
+ .collect(toImmutableList()))
+ .fileHeader(proto.getFileHeader())
+ .oldId(AbbreviatedObjectId.fromObjectId(idConverter.fromByteString(proto.getOldId())))
+ .newId(AbbreviatedObjectId.fromObjectId(idConverter.fromByteString(proto.getNewId())));
+
+ if (proto.hasField(OLD_PATH_DESCRIPTOR)) {
+ builder.oldPath(Optional.of(proto.getOldPath()));
+ }
+ if (proto.hasField(NEW_PATH_DESCRIPTOR)) {
+ builder.newPath(Optional.of(proto.getNewPath()));
+ }
+ if (proto.hasField(OLD_MODE_DESCRIPTOR)) {
+ builder.oldMode(Optional.of(Patch.FileMode.valueOf(proto.getOldMode())));
+ }
+ if (proto.hasField(NEW_MODE_DESCRIPTOR)) {
+ builder.newMode(Optional.of(Patch.FileMode.valueOf(proto.getNewMode())));
+ }
+ if (proto.hasField(CHANGE_TYPE_DESCRIPTOR)) {
+ builder.changeType(Optional.of(Patch.ChangeType.valueOf(proto.getChangeType())));
+ }
+ if (proto.hasField(PATCH_TYPE_DESCRIPTOR)) {
+ builder.patchType(Optional.of(Patch.PatchType.valueOf(proto.getPatchType())));
+ }
+ return builder.build();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCache.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCache.java
new file mode 100644
index 0000000..2516761
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCache.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+
+/** This cache computes pure git diff for a single file path according to a git tree diff. */
+public interface GitFileDiffCache {
+
+ /**
+ * Returns the git file diff for a single file path identified by its key.
+ *
+ * @param key identifies two git trees, a specific file path and other diff parameters.
+ * @return the file diff for a single file path identified by its key.
+ * @throws DiffNotAvailableException if the tree IDs of the key are invalid for this project or if
+ * file contents could not be read.
+ */
+ GitFileDiff get(GitFileDiffCacheKey key) throws DiffNotAvailableException;
+
+ /**
+ * Returns the file diff for a collection of file paths identified by their keys.
+ *
+ * @param keys identifying different file paths of different projects.
+ * @return a map of the input keys to their corresponding git file diffs.
+ * @throws DiffNotAvailableException if the diff failed to be evaluated for one or more of the
+ * input keys due to invalid tree IDs or if file contents could not be read.
+ */
+ ImmutableMap<GitFileDiffCacheKey, GitFileDiff> getAll(Iterable<GitFileDiffCacheKey> keys)
+ throws DiffNotAvailableException;
+}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java
new file mode 100644
index 0000000..97cf37d32
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java
@@ -0,0 +1,273 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import static java.util.function.Function.identity;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Streams;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.name.Named;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.diff.DiffEntry;
+import org.eclipse.jgit.diff.DiffEntry.ChangeType;
+import org.eclipse.jgit.diff.DiffFormatter;
+import org.eclipse.jgit.diff.HistogramDiff;
+import org.eclipse.jgit.diff.RawTextComparator;
+import org.eclipse.jgit.lib.AbbreviatedObjectId;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.util.io.DisabledOutputStream;
+
+/** Implementation of the {@link GitFileDiffCache} */
+public class GitFileDiffCacheImpl implements GitFileDiffCache {
+ private static final String GIT_DIFF = "git_file_diff";
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ bind(GitFileDiffCache.class).to(GitFileDiffCacheImpl.class);
+ persist(GIT_DIFF, GitFileDiffCacheKey.class, GitFileDiff.class)
+ .maximumWeight(10 << 20)
+ .weigher(GitFileDiffWeigher.class)
+ .keySerializer(GitFileDiffCacheKey.Serializer.INSTANCE)
+ .valueSerializer(GitFileDiff.Serializer.INSTANCE)
+ .loader(GitFileDiffCacheImpl.Loader.class);
+ }
+ };
+ }
+
+ /** Enum for the supported diff algorithms for the file diff computation. */
+ public enum DiffAlgorithm {
+ HISTOGRAM,
+ HISTOGRAM_WITHOUT_MYERS_FALLBACK
+ }
+
+ /** Creates a new JGit diff algorithm instance using the Gerrit's {@link DiffAlgorithm} enum. */
+ public static class DiffAlgorithmFactory {
+ public static org.eclipse.jgit.diff.DiffAlgorithm create(DiffAlgorithm diffAlgorithm) {
+ HistogramDiff result = new HistogramDiff();
+ if (diffAlgorithm.equals(DiffAlgorithm.HISTOGRAM_WITHOUT_MYERS_FALLBACK)) {
+ result.setFallbackAlgorithm(null);
+ }
+ return result;
+ }
+ }
+
+ private final LoadingCache<GitFileDiffCacheKey, GitFileDiff> cache;
+
+ @Inject
+ public GitFileDiffCacheImpl(
+ @Named(GIT_DIFF) LoadingCache<GitFileDiffCacheKey, GitFileDiff> cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public GitFileDiff get(GitFileDiffCacheKey key) throws DiffNotAvailableException {
+ try {
+ return cache.get(key);
+ } catch (ExecutionException e) {
+ throw new DiffNotAvailableException(e);
+ }
+ }
+
+ @Override
+ public ImmutableMap<GitFileDiffCacheKey, GitFileDiff> getAll(Iterable<GitFileDiffCacheKey> keys)
+ throws DiffNotAvailableException {
+ try {
+ return cache.getAll(keys);
+ } catch (ExecutionException e) {
+ throw new DiffNotAvailableException(e);
+ }
+ }
+
+ static class Loader extends CacheLoader<GitFileDiffCacheKey, GitFileDiff> {
+ /**
+ * Extractor for the file path from a {@link DiffEntry}. Returns the old file path if the entry
+ * corresponds to a deleted file, otherwise it returns the new file path.
+ */
+ private static final Function<DiffEntry, String> pathExtractor =
+ (DiffEntry entry) ->
+ entry.getChangeType().equals(ChangeType.DELETE)
+ ? entry.getOldPath()
+ : entry.getNewPath();
+
+ private final GitRepositoryManager repoManager;
+
+ @Inject
+ public Loader(GitRepositoryManager repoManager) {
+ this.repoManager = repoManager;
+ }
+
+ @Override
+ public GitFileDiff load(GitFileDiffCacheKey key) throws IOException {
+ return loadAll(ImmutableList.of(key)).get(key);
+ }
+
+ @Override
+ public Map<GitFileDiffCacheKey, GitFileDiff> loadAll(
+ Iterable<? extends GitFileDiffCacheKey> keys) throws IOException {
+ ImmutableMap.Builder<GitFileDiffCacheKey, GitFileDiff> result =
+ ImmutableMap.builderWithExpectedSize(Iterables.size(keys));
+
+ Map<Project.NameKey, List<GitFileDiffCacheKey>> byProject =
+ Streams.stream(keys)
+ .distinct()
+ .collect(Collectors.groupingBy(GitFileDiffCacheKey::project));
+
+ for (Map.Entry<Project.NameKey, List<GitFileDiffCacheKey>> entry : byProject.entrySet()) {
+ try (Repository repo = repoManager.openRepository(entry.getKey());
+ ObjectReader reader = repo.newObjectReader()) {
+
+ // Grouping keys by diff options because each group of keys will be processed with a
+ // separate call to JGit using the DiffFormatter object.
+ Map<DiffOptions, List<GitFileDiffCacheKey>> optionsGroups =
+ entry.getValue().stream().collect(Collectors.groupingBy(DiffOptions::fromKey));
+
+ for (Map.Entry<DiffOptions, List<GitFileDiffCacheKey>> group : optionsGroups.entrySet()) {
+ result.putAll(loadAllImpl(repo, reader, group.getKey(), group.getValue()));
+ }
+ }
+ }
+ return result.build();
+ }
+
+ /**
+ * Loads the git file diffs for all keys of the same repository, and having the same diff {@code
+ * options}.
+ *
+ * @return The git file diffs for all input keys.
+ */
+ private Map<GitFileDiffCacheKey, GitFileDiff> loadAllImpl(
+ Repository repo, ObjectReader reader, DiffOptions options, List<GitFileDiffCacheKey> keys)
+ throws IOException {
+ ImmutableMap.Builder<GitFileDiffCacheKey, GitFileDiff> result =
+ ImmutableMap.builderWithExpectedSize(keys.size());
+ Map<GitFileDiffCacheKey, String> filePaths =
+ keys.stream().collect(Collectors.toMap(identity(), GitFileDiffCacheKey::newFilePath));
+ DiffFormatter formatter = createDiffFormatter(options, repo, reader);
+ Map<String, DiffEntry> diffEntries = loadDiffEntries(formatter, options, filePaths.values());
+ for (GitFileDiffCacheKey key : filePaths.keySet()) {
+ String newFilePath = filePaths.get(key);
+ if (diffEntries.containsKey(newFilePath)) {
+ result.put(key, GitFileDiff.create(diffEntries.get(newFilePath), formatter));
+ continue;
+ }
+ result.put(
+ key,
+ GitFileDiff.empty(
+ AbbreviatedObjectId.fromObjectId(key.oldTree()),
+ AbbreviatedObjectId.fromObjectId(key.newTree()),
+ newFilePath));
+ }
+ return result.build();
+ }
+
+ private static Map<String, DiffEntry> loadDiffEntries(
+ DiffFormatter diffFormatter, DiffOptions diffOptions, Collection<String> filePaths)
+ throws IOException {
+ Set<String> filePathsSet = ImmutableSet.copyOf(filePaths);
+ List<DiffEntry> diffEntries =
+ diffFormatter.scan(diffOptions.oldTree(), diffOptions.newTree());
+
+ return diffEntries.stream()
+ .filter(d -> filePathsSet.contains(pathExtractor.apply(d)))
+ .collect(Collectors.toMap(d -> pathExtractor.apply(d), identity()));
+ }
+
+ private static DiffFormatter createDiffFormatter(
+ DiffOptions diffOptions, Repository repo, ObjectReader reader) {
+ try (DiffFormatter diffFormatter = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
+ diffFormatter.setReader(reader, repo.getConfig());
+ RawTextComparator cmp = comparatorFor(diffOptions.whitespace());
+ diffFormatter.setDiffComparator(cmp);
+ if (diffOptions.renameScore() != -1) {
+ diffFormatter.setDetectRenames(true);
+ diffFormatter.getRenameDetector().setRenameScore(diffOptions.renameScore());
+ }
+ diffFormatter.setDiffAlgorithm(DiffAlgorithmFactory.create(diffOptions.diffAlgorithm()));
+ return diffFormatter;
+ }
+ }
+
+ private static RawTextComparator comparatorFor(Whitespace ws) {
+ switch (ws) {
+ case IGNORE_ALL:
+ return RawTextComparator.WS_IGNORE_ALL;
+
+ case IGNORE_TRAILING:
+ return RawTextComparator.WS_IGNORE_TRAILING;
+
+ case IGNORE_LEADING_AND_TRAILING:
+ return RawTextComparator.WS_IGNORE_CHANGE;
+
+ case IGNORE_NONE:
+ default:
+ return RawTextComparator.DEFAULT;
+ }
+ }
+ }
+
+ /** An entity representing the options affecting the diff computation. */
+ @AutoValue
+ abstract static class DiffOptions {
+ /** Convert a {@link GitFileDiffCacheKey} input to a {@link DiffOptions}. */
+ static DiffOptions fromKey(GitFileDiffCacheKey key) {
+ return create(
+ key.oldTree(), key.newTree(), key.renameScore(), key.whitespace(), key.diffAlgorithm());
+ }
+
+ private static DiffOptions create(
+ ObjectId oldTree,
+ ObjectId newTree,
+ Integer renameScore,
+ Whitespace whitespace,
+ DiffAlgorithm diffAlgorithm) {
+ return new AutoValue_GitFileDiffCacheImpl_DiffOptions(
+ oldTree, newTree, renameScore, whitespace, diffAlgorithm);
+ }
+
+ abstract ObjectId oldTree();
+
+ abstract ObjectId newTree();
+
+ abstract Integer renameScore();
+
+ abstract Whitespace whitespace();
+
+ abstract DiffAlgorithm diffAlgorithm();
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheKey.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheKey.java
new file mode 100644
index 0000000..d570ada
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheKey.java
@@ -0,0 +1,135 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.server.cache.proto.Cache.GitFileDiffKeyProto;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm;
+import org.eclipse.jgit.lib.ObjectId;
+
+@AutoValue
+public abstract class GitFileDiffCacheKey {
+
+ /** A specific git project / repository. */
+ public abstract Project.NameKey project();
+
+ /** The old 20 bytes SHA-1 git tree ID used in the git tree diff */
+ public abstract ObjectId oldTree();
+
+ /** The new 20 bytes SHA-1 git tree ID used in the git tree diff */
+ public abstract ObjectId newTree();
+
+ /** File name in the tree identified by {@link #newTree()} */
+ public abstract String newFilePath();
+
+ /**
+ * Percentage score used to identify a file as a "rename". A special value of -1 means that the
+ * computation will ignore renames and rename detection will be disabled.
+ */
+ public abstract int renameScore();
+
+ public abstract DiffAlgorithm diffAlgorithm();
+
+ public abstract DiffPreferencesInfo.Whitespace whitespace();
+
+ public int weight() {
+ return stringSize(project().get())
+ + 20 * 2 // oldTree and newTree
+ + stringSize(newFilePath())
+ + 4 // renameScore
+ + 4 // diffAlgorithm
+ + 4; // whitespace
+ }
+
+ private static int stringSize(String str) {
+ if (str != null) {
+ // each character in the string occupies 2 bytes. Ignoring the fixed overhead for the string
+ // (length, offset and hash code) since they are negligible and do not
+ // affect the comparison of 2 strings
+ return str.length() * 2;
+ }
+ return 0;
+ }
+
+ public static Builder builder() {
+ return new AutoValue_GitFileDiffCacheKey.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder project(NameKey value);
+
+ public abstract Builder oldTree(ObjectId value);
+
+ public abstract Builder newTree(ObjectId value);
+
+ public abstract Builder newFilePath(String value);
+
+ public abstract Builder renameScore(Integer value);
+
+ public Builder disableRenameDetection() {
+ renameScore(-1);
+ return this;
+ }
+
+ public abstract Builder diffAlgorithm(DiffAlgorithm value);
+
+ public abstract Builder whitespace(Whitespace value);
+
+ public abstract GitFileDiffCacheKey build();
+ }
+
+ public enum Serializer implements CacheSerializer<GitFileDiffCacheKey> {
+ INSTANCE;
+
+ @Override
+ public byte[] serialize(GitFileDiffCacheKey key) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return Protos.toByteArray(
+ GitFileDiffKeyProto.newBuilder()
+ .setProject(key.project().get())
+ .setATree(idConverter.toByteString(key.oldTree()))
+ .setBTree(idConverter.toByteString(key.newTree()))
+ .setFilePath(key.newFilePath())
+ .setRenameScore(key.renameScore())
+ .setDiffAlgorithm(key.diffAlgorithm().name())
+ .setWhitepsace(key.whitespace().name())
+ .build());
+ }
+
+ @Override
+ public GitFileDiffCacheKey deserialize(byte[] in) {
+ GitFileDiffKeyProto proto = Protos.parseUnchecked(GitFileDiffKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return GitFileDiffCacheKey.builder()
+ .project(Project.nameKey(proto.getProject()))
+ .oldTree(idConverter.fromByteString(proto.getATree()))
+ .newTree(idConverter.fromByteString(proto.getBTree()))
+ .newFilePath(proto.getFilePath())
+ .renameScore(proto.getRenameScore())
+ .diffAlgorithm(DiffAlgorithm.valueOf(proto.getDiffAlgorithm()))
+ .whitespace(Whitespace.valueOf(proto.getWhitepsace()))
+ .build();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffWeigher.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffWeigher.java
new file mode 100644
index 0000000..47f7791
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffWeigher.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch.gitfilediff;
+
+import com.google.common.cache.Weigher;
+
+public class GitFileDiffWeigher implements Weigher<GitFileDiffCacheKey, GitFileDiff> {
+
+ @Override
+ public int weigh(GitFileDiffCacheKey key, GitFileDiff gitFileDiff) {
+ return key.weight() + gitFileDiff.weight();
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 432b48a..6de263e 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -307,7 +307,7 @@
private Integer unresolvedCommentCount;
private Integer totalCommentCount;
private LabelTypes labelTypes;
-
+ private Optional<Timestamp> mergedOn;
private ImmutableList<byte[]> refStates;
private ImmutableList<byte[]> refStatePatterns;
@@ -616,6 +616,29 @@
}
/**
+ * Returns the {@link Optional} value of time when the change was merged.
+ *
+ * <p>The value can be set from index field, see {@link ChangeData#setMergedOn} or loaded from the
+ * database (available in {@link ChangeNotes})
+ *
+ * @return {@link Optional} value of time when the change was merged.
+ * @throws StorageException if {@code lazyLoad} is off, {@link ChangeNotes} can not be loaded
+ * because we do not expect to call the database.
+ */
+ public Optional<Timestamp> getMergedOn() throws StorageException {
+ if (mergedOn == null) {
+ // The value was not loaded yet, try to get from the database.
+ mergedOn = notes().getMergedOn();
+ }
+ return mergedOn;
+ }
+
+ /** Sets the value e.g. when loading from index. */
+ public void setMergedOn(@Nullable Timestamp mergedOn) {
+ this.mergedOn = Optional.ofNullable(mergedOn);
+ }
+
+ /**
* Sets the specified attention set. If two or more entries refer to the same user, throws an
* {@link IllegalStateException}.
*/
@@ -670,7 +693,9 @@
return allApprovals;
}
- /** @return The submit ('SUBM') approval label */
+ /* @return legacy submit ('SUBM') approval label */
+ // TODO(mariasavtchouk): Deprecate legacy submit label,
+ // see com.google.gerrit.entities.LabelId.LEGACY_SUBMIT_NAME
public Optional<PatchSetApproval> getSubmitApproval() {
return currentApprovals().stream().filter(PatchSetApproval::isLegacySubmit).findFirst();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 085d23d..2891d4b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -1386,6 +1386,17 @@
}
}
+ @Test
+ public void submitSetsMergedOn() throws Throwable {
+ PushOneCommit.Result r = createChange();
+ assertThat(r.getChange().getMergedOn()).isEmpty();
+ submit(r.getChangeId());
+ assertThat(r.getChange().getMergedOn()).isPresent();
+ ChangeInfo change = gApi.changes().id(r.getChangeId()).get();
+ assertThat(r.getChange().getMergedOn().get()).isEqualTo(change.updated);
+ assertThat(r.getChange().getMergedOn().get()).isEqualTo(change.submitted);
+ }
+
@Override
protected void updateProjectInput(ProjectInput in) {
in.submitType = getSubmitType();
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffKeySerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffKeySerializerTest.java
new file mode 100644
index 0000000..12d8d00
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffKeySerializerTest.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm;
+import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheKey;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class GitFileDiffKeySerializerTest {
+ private static final ObjectId TREE_ID_1 =
+ ObjectId.fromString("123e9fa8a286255ac7d5ba11b598892735758391");
+ private static final ObjectId TREE_ID_2 =
+ ObjectId.fromString("d07a03a9818c120301cb5b4a969b035479400b5f");
+
+ @Test
+ public void roundTrip() {
+ GitFileDiffCacheKey key =
+ GitFileDiffCacheKey.builder()
+ .project(Project.nameKey("project/x"))
+ .oldTree(TREE_ID_1)
+ .newTree(TREE_ID_2)
+ .newFilePath("some_file.txt")
+ .renameScore(65)
+ .diffAlgorithm(DiffAlgorithm.HISTOGRAM)
+ .whitespace(Whitespace.IGNORE_ALL)
+ .build();
+
+ byte[] serialized = GitFileDiffCacheKey.Serializer.INSTANCE.serialize(key);
+
+ assertThat(GitFileDiffCacheKey.Serializer.INSTANCE.deserialize(serialized)).isEqualTo(key);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java
new file mode 100644
index 0000000..8030818
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java
@@ -0,0 +1,59 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Patch.ChangeType;
+import com.google.gerrit.entities.Patch.FileMode;
+import com.google.gerrit.entities.Patch.PatchType;
+import com.google.gerrit.server.patch.filediff.Edit;
+import com.google.gerrit.server.patch.gitfilediff.GitFileDiff;
+import com.google.gerrit.server.patch.gitfilediff.GitFileDiff.Serializer;
+import java.util.Optional;
+import org.eclipse.jgit.lib.AbbreviatedObjectId;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class GitFileDiffSerializerTest {
+ private static final ObjectId OLD_ID =
+ ObjectId.fromString("123e9fa8a286255ac7d5ba11b598892735758391");
+ private static final ObjectId NEW_ID =
+ ObjectId.fromString("d07a03a9818c120301cb5b4a969b035479400b5f");
+
+ @Test
+ public void roundTrip() {
+ ImmutableList<Edit> edits =
+ ImmutableList.of(Edit.create(1, 5, 3, 4), Edit.create(21, 30, 150, 158));
+
+ GitFileDiff gitFileDiff =
+ GitFileDiff.builder()
+ .edits(edits)
+ .fileHeader("file_header")
+ .oldPath(Optional.of("old_file_path.txt"))
+ .newPath(Optional.empty())
+ .oldId(AbbreviatedObjectId.fromObjectId(OLD_ID))
+ .newId(AbbreviatedObjectId.fromObjectId(NEW_ID))
+ .changeType(Optional.of(ChangeType.DELETED))
+ .patchType(Optional.of(PatchType.UNIFIED))
+ .oldMode(Optional.of(FileMode.REGULAR_FILE))
+ .newMode(Optional.of(FileMode.REGULAR_FILE))
+ .build();
+
+ byte[] serialized = Serializer.INSTANCE.serialize(gitFileDiff);
+ assertThat(Serializer.INSTANCE.deserialize(serialized)).isEqualTo(gitFileDiff);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 321e4da..6c5bc2e 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -837,6 +837,7 @@
"publishedComments",
new TypeLiteral<ImmutableListMultimap<ObjectId, HumanComment>>() {}.getType())
.put("updateCount", int.class)
+ .put("mergedOn", Timestamp.class)
.build());
}
@@ -977,6 +978,19 @@
}
@Test
+ public void serializeMergedOn() throws Exception {
+ assertRoundTrip(
+ newBuilder().mergedOn(new Timestamp(234567L)).build(),
+ ChangeNotesStateProto.newBuilder()
+ .setMetaId(SHA_BYTES)
+ .setChangeId(ID.get())
+ .setMergedOnMillis(234567L)
+ .setHasMergedOn(true)
+ .setColumns(colsProto.toBuilder())
+ .build());
+ }
+
+ @Test
public void changeMessageFields() throws Exception {
assertThatSerializedClass(ChangeMessage.Key.class)
.hasAutoValueMethods(ImmutableMap.of("changeId", Change.Id.class, "uuid", String.class));
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index cc0b109..bae2f46 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -677,6 +677,74 @@
}
@Test
+ public void mergedOnEmptyIfNotSubmitted() throws Exception {
+ Change c = newChange();
+
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ // Make sure unrelevent update does not set mergedOn.
+ update.setTopic("topic");
+ update.commit();
+ assertThat(newNotes(c).getMergedOn()).isEmpty();
+ }
+
+ @Test
+ public void mergedOnSetWhenSubmitted() throws Exception {
+ Change c = newChange();
+
+ SubmissionId submissionId = new SubmissionId(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setSubjectForCommit("Update patch set 1");
+ update.merge(
+ submissionId,
+ ImmutableList.of(
+ submitRecord("OK", null, submitLabel("Code-Review", "OK", otherUser.getAccountId()))));
+ update.commit();
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getMergedOn()).isPresent();
+ Timestamp mergedOn = notes.getMergedOn().get();
+ assertThat(mergedOn).isEqualTo(notes.getChange().getLastUpdatedOn());
+
+ // Next update does not change mergedOn date.
+ update = newUpdate(c, changeOwner);
+ update.putApproval("Code-Review", (short) 1);
+ update.commit();
+ notes = newNotes(c);
+ assertThat(notes.getMergedOn().get()).isEqualTo(mergedOn);
+ assertThat(notes.getMergedOn().get()).isLessThan(notes.getChange().getLastUpdatedOn());
+ }
+
+ @Test
+ public void latestMergedOn() throws Exception {
+ Change c = newChange();
+ SubmissionId submissionId = new SubmissionId(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setSubjectForCommit("Update patch set 1");
+ update.merge(
+ submissionId,
+ ImmutableList.of(
+ submitRecord("OK", null, submitLabel("Code-Review", "OK", otherUser.getAccountId()))));
+ update.commit();
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getMergedOn()).isPresent();
+ Timestamp mergedOn = notes.getMergedOn().get();
+ assertThat(mergedOn).isEqualTo(notes.getChange().getLastUpdatedOn());
+
+ incrementPatchSet(c);
+ update = newUpdate(c, changeOwner);
+ update.setSubjectForCommit("Update patch set 2");
+ update.merge(
+ submissionId,
+ ImmutableList.of(
+ submitRecord(
+ "OK", null, submitLabel("Code-Review", "OK", changeOwner.getAccountId()))));
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(notes.getMergedOn().get()).isGreaterThan(mergedOn);
+ assertThat(notes.getMergedOn().get()).isEqualTo(notes.getChange().getLastUpdatedOn());
+ }
+
+ @Test
public void emptyChangeUpdate() throws Exception {
Change c = newChange();
Ref initial = repo.exactRef(changeMetaRef(c.getId()));
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index 2266ba0..052a750 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -187,6 +187,9 @@
npm run test:debug async-foreach-behavior_test.js
```
+When converting a test file to typescript, the command for running tests is
+still using the .js suffix and not the new .ts suffix.
+
Commands `test:debug` and `test:single` assumes that compiled code is located
in the `./ts-out/polygerrit-ui/app` directory. It's up to you how to achieve it.
For example, the following options are possible:
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
index 2f85a0b..912d59e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -306,7 +306,7 @@
return this.getContentTdByLine(line, side, row);
}
- getLineElByNumber(lineNumber: string | number, side?: Side) {
+ getLineElByNumber(lineNumber: LineNumber, side?: Side) {
const sideSelector = side ? '.' + side : '';
return this.diffElement.querySelector(
`.lineNum[data-value="${lineNumber}"]${sideSelector}`
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index 08a567b..c788d76 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -30,6 +30,7 @@
import {GrSelectionActionBox} from '../gr-selection-action-box/gr-selection-action-box';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
import {FILE} from '../gr-diff/gr-diff-line';
+import {getRange, getSide} from '../gr-diff/gr-diff-utils';
interface SidedRange {
side: Side;
@@ -199,13 +200,9 @@
}
_indexForThreadEl(threadEl: HTMLElement) {
- const side = threadEl.getAttribute('comment-side') as Side;
- const rangeString = threadEl.getAttribute('range');
- if (!rangeString) return undefined;
- const range = JSON.parse(rangeString) as CommentRange;
-
- if (!range) return undefined;
-
+ const side = getSide(threadEl);
+ const range = getRange(threadEl);
+ if (!side || !range) return undefined;
return this._indexOfCommentRange(side, range);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 4b4f429..2223734 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -24,7 +24,12 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-diff-host_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {rangesEqual} from '../gr-diff/gr-diff-utils';
+import {
+ getLine,
+ getRange,
+ getSide,
+ rangesEqual,
+} from '../gr-diff/gr-diff-utils';
import {appContext} from '../../../services/app-context';
import {
getParentIndex,
@@ -808,11 +813,7 @@
throw new Error(`Unknown side: ${commentSide}`);
}
function matchesRange(threadEl: GrCommentThread) {
- const rangeAtt = threadEl.getAttribute('range');
- const threadRange = rangeAtt
- ? (JSON.parse(rangeAtt) as CommentRange)
- : undefined;
- return rangesEqual(threadRange, range);
+ return rangesEqual(getRange(threadEl), range);
}
const filteredThreadEls = this._filterThreadElsForLocation(
@@ -830,21 +831,18 @@
) {
function matchesLeftLine(threadEl: GrCommentThread) {
return (
- threadEl.getAttribute('comment-side') === Side.LEFT &&
- threadEl.getAttribute('line-num') === String(lineInfo.beforeNumber)
+ getSide(threadEl) === Side.LEFT &&
+ getLine(threadEl) === lineInfo.beforeNumber
);
}
function matchesRightLine(threadEl: GrCommentThread) {
return (
- threadEl.getAttribute('comment-side') === Side.RIGHT &&
- threadEl.getAttribute('line-num') === String(lineInfo.afterNumber)
+ getSide(threadEl) === Side.RIGHT &&
+ getLine(threadEl) === lineInfo.afterNumber
);
}
function matchesFileComment(threadEl: GrCommentThread) {
- return (
- threadEl.getAttribute('comment-side') === side &&
- threadEl.getAttribute('line-num') === FILE
- );
+ return getSide(threadEl) === side && getLine(threadEl) === FILE;
}
// Select the appropriate matchers for the desired side and line
@@ -855,7 +853,7 @@
if (side === Side.RIGHT) {
matchers.push(matchesRightLine);
}
- if (lineInfo.afterNumber === 'FILE' || lineInfo.beforeNumber === 'FILE') {
+ if (lineInfo.afterNumber === FILE || lineInfo.beforeNumber === FILE) {
matchers.push(matchesFileComment);
}
return threadEls.filter(threadEl =>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
index 03740ba..08c8226 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
@@ -30,6 +30,7 @@
import {DiffInfo} from '../../../types/diff';
import {Side} from '../../../constants/constants';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
+import {getSide, isThreadEl} from '../gr-diff/gr-diff-utils';
/**
* Possible CSS classes indicating the state of selection. Dynamically added/
@@ -96,10 +97,10 @@
}
_handleDownOnRangeComment(node: Element) {
- if (node?.nodeName?.toLowerCase() === 'gr-comment-thread') {
+ if (isThreadEl(node)) {
this._setClasses([
SelectionClass.COMMENT,
- node.getAttribute('comment-side') === Side.LEFT
+ getSide(node) === Side.LEFT
? SelectionClass.LEFT
: SelectionClass.RIGHT,
]);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index 8984dc8..b13b3c5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -17,6 +17,7 @@
import {CommentRange} from '../../../types/common';
import {FILE, LineNumber} from './gr-diff-line';
+import {Side} from '../../../constants/constants';
/**
* Compare two ranges. Either argument may be falsy, but will only return
@@ -46,3 +47,47 @@
const lineNumber = Number(lineNumberStr);
return Number.isInteger(lineNumber) ? lineNumber : null;
}
+
+export function getLine(threadEl: HTMLElement): LineNumber {
+ const lineAtt = threadEl.getAttribute('line-num');
+ if (!lineAtt || lineAtt === 'FILE') return FILE;
+ const line = Number(lineAtt);
+ if (isNaN(line)) throw new Error(`cannot parse line number: ${lineAtt}`);
+ if (line < 1) throw new Error(`line number smaller than 1: ${line}`);
+ return line;
+}
+
+export function getSide(threadEl: HTMLElement): Side | undefined {
+ const sideAtt = threadEl.getAttribute('comment-side');
+ if (!sideAtt) {
+ console.warn('comment thread without side');
+ return undefined;
+ }
+ if (sideAtt !== Side.LEFT && sideAtt !== Side.RIGHT)
+ throw Error(`unexpected value for side: ${sideAtt}`);
+ return sideAtt as Side;
+}
+
+export function getRange(threadEl: HTMLElement): CommentRange | undefined {
+ const rangeAtt = threadEl.getAttribute('range');
+ if (!rangeAtt) return undefined;
+ const range = JSON.parse(rangeAtt) as CommentRange;
+ if (!range.start_line) throw new Error(`invalid range: ${rangeAtt}`);
+ return range;
+}
+
+// TODO: This type should be exposed to gr-diff clients in a separate type file.
+// For Gerrit these are instances of GrCommentThread, but other gr-diff users
+// have different HTML elements in use for comment threads.
+// TODO: Also document the required HTML attritbutes that thread elements must
+// have, e.g. 'comment-side', 'range', 'line-num', 'data-value'.
+export interface GrDiffThreadElement extends HTMLElement {
+ rootId: string;
+}
+
+export function isThreadEl(node: Node): node is GrDiffThreadElement {
+ return (
+ node.nodeType === Node.ELEMENT_NODE &&
+ (node as Element).classList.contains('comment-thread')
+ );
+}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index ae3e016..472a6bd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -27,8 +27,16 @@
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {htmlTemplate} from './gr-diff_html';
-import {FILE, LineNumber} from './gr-diff-line';
-import {getLineNumber, rangesEqual} from './gr-diff-utils';
+import {LineNumber} from './gr-diff-line';
+import {
+ getLine,
+ getLineNumber,
+ getRange,
+ getSide,
+ GrDiffThreadElement,
+ isThreadEl,
+ rangesEqual,
+} from './gr-diff-utils';
import {getHiddenScroll} from '../../../scripts/hiddenscroll';
import {isMergeParent, patchNumEquals} from '../../../utils/patch-set-util';
import {customElement, observe, property} from '@polymer/decorators';
@@ -53,7 +61,7 @@
PolymerDomWrapper,
} from '../../../types/types';
import {CommentRangeLayer} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
-import {DiffViewMode, Side, CommentSide} from '../../../constants/constants';
+import {CommentSide, DiffViewMode, Side} from '../../../constants/constants';
import {KeyLocations} from '../gr-diff-processor/gr-diff-processor';
import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
@@ -67,27 +75,6 @@
const FULL_CONTEXT = -1;
const LIMITED_CONTEXT = 10;
-function getSide(threadEl: GrCommentThread): Side {
- const sideAtt = threadEl.getAttribute('comment-side');
- if (!sideAtt) throw Error('comment thread without side');
- if (sideAtt !== 'left' && sideAtt !== 'right')
- throw Error(`unexpected value for side: ${sideAtt}`);
- return sideAtt as Side;
-}
-
-function isThreadEl(node: Node): node is GrCommentThread {
- return (
- node.nodeType === Node.ELEMENT_NODE &&
- (node as Element).classList.contains('comment-thread')
- );
-}
-
-// TODO(TS): Replace by proper GrCommentThread once converted.
-type GrCommentThread = PolymerElement & {
- rootId: string;
- range: CommentRange;
-};
-
const COMMIT_MSG_PATH = '/COMMIT_MSG';
/**
* 72 is the unofficial length standard for git commit messages.
@@ -380,17 +367,16 @@
// TODO(brohlfs): Rewrite gr-diff to be agnostic of GrCommentThread, because
// other users of gr-diff may use different comment widgets.
_updateRanges(
- addedThreadEls: GrCommentThread[],
- removedThreadEls: GrCommentThread[]
+ addedThreadEls: GrDiffThreadElement[],
+ removedThreadEls: GrDiffThreadElement[]
) {
function commentRangeFromThreadEl(
- threadEl: GrCommentThread
+ threadEl: GrDiffThreadElement
): CommentRangeLayer | undefined {
const side = getSide(threadEl);
-
- const rangeAtt = threadEl.getAttribute('range');
- if (!rangeAtt) return undefined;
- const range = JSON.parse(rangeAtt) as CommentRange;
+ if (!side) return undefined;
+ const range = getRange(threadEl);
+ if (!range) return undefined;
return {side, range, hovering: false, rootId: threadEl.rootId};
}
@@ -433,12 +419,13 @@
for (const threadEl of threadEls) {
const side = getSide(threadEl);
- const lineNum = threadEl.getAttribute('line-num') || FILE;
- const commentRange = threadEl.range || {};
+ if (!side) continue;
+ const lineNum = getLine(threadEl);
+ const commentRange = getRange(threadEl);
keyLocations[side][lineNum] = true;
// Add start_line as well if exists,
// the being and end of the range should not be collapsed.
- if (commentRange.start_line) {
+ if (commentRange?.start_line) {
keyLocations[side][commentRange.start_line] = true;
}
}
@@ -446,7 +433,7 @@
}
// Dispatch events that are handled by the gr-diff-highlight.
- _redispatchHoverEvents(addedThreadEls: GrCommentThread[]) {
+ _redispatchHoverEvents(addedThreadEls: HTMLElement[]) {
for (const threadEl of addedThreadEls) {
threadEl.addEventListener('mouseenter', () => {
threadEl.dispatchEvent(
@@ -891,10 +878,11 @@
// for each line from the start.
let lastEl;
for (const threadEl of addedThreadEls) {
- const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
+ const lineNum = getLine(threadEl);
const commentSide = getSide(threadEl);
+ if (!commentSide) continue;
const lineEl = this.$.diffBuilder.getLineElByNumber(
- lineNumString,
+ lineNum,
commentSide
);
// When the line the comment refers to does not exist, log an error
@@ -904,7 +892,7 @@
console.error(
'thread attached to line ',
commentSide,
- lineNumString,
+ lineNum,
' which does not exist.'
);
continue;
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 0ec9aaf..5ffdfe2 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -1424,9 +1424,7 @@
ListChangesOption.LABELS,
ListChangesOption.DETAILED_ACCOUNTS,
];
- if (config?.change && config.change.enable_attention_set) {
- options.push(ListChangesOption.DETAILED_LABELS);
- } else {
+ if (!config?.change?.enable_attention_set) {
options.push(ListChangesOption.REVIEWED);
}
diff --git a/proto/cache.proto b/proto/cache.proto
index aa71b87..0676f1b 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -76,7 +76,7 @@
// Instead, we just take the tedious yet simple approach of having a "has_foo"
// field for each nullable field "foo", indicating whether or not foo is null.
//
-// Next ID: 25
+// Next ID: 27
message ChangeNotesStateProto {
// Effectively required, even though the corresponding ChangeNotesState field
// is optional, since the field is only absent when NoteDb is disabled, in
@@ -223,6 +223,10 @@
// Includes all attention set updates.
repeated AttentionSetUpdateProto all_attention_set_update = 24;
+
+ // Epoch millis.
+ int64 merged_on_millis = 25;
+ bool has_merged_on = 26;
}
// Serialized form of com.google.gerrit.server.query.change.ConflictKey
@@ -561,3 +565,37 @@
message ModifiedFilesProto {
repeated ModifiedFileProto modifiedFile = 1;
}
+
+// Serialized form of a collection of
+// com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.Key
+// Next ID: 8
+message GitFileDiffKeyProto {
+ string project = 1;
+ bytes a_tree = 2;
+ bytes b_tree = 3;
+ string file_path = 4;
+ int32 rename_score = 5;
+ string diff_algorithm = 6; // ENUM as string
+ string whitepsace = 7; // ENUM as string
+}
+
+// Serialized form of com.google.gerrit.server.patch.gitfilediff.GitFileDiff
+// Next ID: 11
+message GitFileDiffProto {
+ message Edit {
+ int32 begin_a = 1;
+ int32 end_a = 2;
+ int32 begin_b = 3;
+ int32 end_b = 4;
+ }
+ repeated Edit edits = 1;
+ string file_header = 2;
+ string old_path = 3;
+ string new_path = 4;
+ bytes old_id = 5;
+ bytes new_id = 6;
+ string old_mode = 7; // ENUM as string
+ string new_mode = 8; // ENUM as string
+ string change_type = 9; // ENUM as string
+ string patch_type = 10; // ENUM as string
+}
