Add the source content type to the comment context

This change adds a new field "source_content_type" to the CommentInfo
entity. This field contains the content type (mime type) of the source
file where the comment is written.

This is needed by the front-end to do syntax highlighting. We return the
file content type in the Get Diff endpoint (check DiffFileMetaInfo). In
this change we are reusing this small part of the diff logic to compute
the mime type of the source file while loading the comment context.

Also added some tests to make sure the content type is returned
correctly, and another test to make sure the comment context is
de/serialized correctly.

Change-Id: Ib1f7281866ca6a11e88e44024a424f253efcb758
This commit is contained in:
Youssef Elghareeb
2021-02-23 18:21:46 +01:00
parent b334bae235
commit 27d6201a4f
11 changed files with 156 additions and 22 deletions

View File

@@ -6710,6 +6710,10 @@ this comment applies.
A list of link:#context-line[ContextLine] containing the lines of the source
file where the comment was written. Available only if the "enable-context"
parameter (see link:#list-change-comments[List Change Comments]) is set.
|`source_content_type` |optional|
Mime type of the file where the comment is written. Available only if the
"enable-context" parameter (see link:#list-change-comments[List Change Comments])
is set.
|===========================

View File

@@ -34,7 +34,17 @@ public class PatchScript {
public enum FileMode {
FILE,
SYMLINK,
GITLINK
GITLINK;
public static FileMode fromJgitFileMode(org.eclipse.jgit.lib.FileMode jgitFileMode) {
PatchScript.FileMode fileMode = PatchScript.FileMode.FILE;
if (jgitFileMode == org.eclipse.jgit.lib.FileMode.SYMLINK) {
fileMode = FileMode.SYMLINK;
} else if (jgitFileMode == org.eclipse.jgit.lib.FileMode.GITLINK) {
fileMode = FileMode.GITLINK;
}
return fileMode;
}
}
public static class PatchScriptFileInfo {

View File

@@ -20,15 +20,24 @@ import com.google.common.collect.ImmutableMap;
/** An entity class representing all context lines of a comment. */
@AutoValue
public abstract class CommentContext {
private static final CommentContext EMPTY = new AutoValue_CommentContext(ImmutableMap.of());
private static final CommentContext EMPTY = new AutoValue_CommentContext(ImmutableMap.of(), "");
public static CommentContext create(ImmutableMap<Integer, String> lines) {
return new AutoValue_CommentContext(lines);
public static CommentContext create(ImmutableMap<Integer, String> lines, String contentType) {
return new AutoValue_CommentContext(lines, contentType);
}
/** Map of {line number, line text} of the context lines of a comment */
public abstract ImmutableMap<Integer, String> lines();
/**
* Content type of the source file. Useful for syntax highlighting.
*
* @return text/x-gerrit-commit-message if the file is a commit message.
* <p>text/x-gerrit-merge-list if the file is a merge list.
* <p>The content/mime type, e.g. text/x-c++src otherwise.
*/
public abstract String contentType();
public static CommentContext empty() {
return EMPTY;
}

View File

@@ -30,6 +30,9 @@ public class CommentInfo extends Comment {
*/
public List<ContextLineInfo> contextLines;
/** Mime type of the underlying source file. Only available if context lines are requested. */
public String sourceContentType;
@Override
public boolean equals(Object o) {
if (super.equals(o)) {

View File

@@ -68,7 +68,7 @@ public class CommentContextCacheImpl implements CommentContextCache {
@Override
protected void configure() {
persist(CACHE_NAME, CommentContextKey.class, CommentContext.class)
.version(2)
.version(3)
.diskLimit(1 << 30) // limit the total cache size to 1 GB
.maximumWeight(1 << 23) // Limit the size of the in-memory cache to 8 MB
.weigher(CommentContextWeigher.class)
@@ -149,6 +149,7 @@ public class CommentContextCacheImpl implements CommentContextCache {
@Override
public byte[] serialize(CommentContext commentContext) {
AllCommentContextProto.Builder allBuilder = AllCommentContextProto.newBuilder();
allBuilder.setContentType(commentContext.contentType());
commentContext
.lines()
@@ -165,9 +166,10 @@ public class CommentContextCacheImpl implements CommentContextCache {
@Override
public CommentContext deserialize(byte[] in) {
ImmutableMap.Builder<Integer, String> contextLinesMap = ImmutableMap.builder();
Protos.parseUnchecked(AllCommentContextProto.parser(), in).getContextList().stream()
AllCommentContextProto proto = Protos.parseUnchecked(AllCommentContextProto.parser(), in);
proto.getContextList().stream()
.forEach(c -> contextLinesMap.put(c.getLineNumber(), c.getContextLine()));
return CommentContext.create(contextLinesMap.build());
return CommentContext.create(contextLinesMap.build(), proto.getContentType());
}
}

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.comment;
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.util.stream.Collectors.groupingBy;
import com.google.auto.value.AutoValue;
@@ -23,16 +24,23 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentContext;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.common.ContextLineInfo;
import com.google.gerrit.server.change.FileContentUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.patch.ComparisonType;
import com.google.gerrit.server.patch.Text;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import eu.medsea.mimeutil.MimeType;
import eu.medsea.mimeutil.MimeUtil2;
import java.io.IOException;
import java.util.Collection;
import java.util.List;
@@ -53,17 +61,25 @@ import org.eclipse.jgit.treewalk.TreeWalk;
public class CommentContextLoader {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final FileTypeRegistry registry;
private final GitRepositoryManager repoManager;
private final Project.NameKey project;
private final ProjectState projectState;
public interface Factory {
CommentContextLoader create(Project.NameKey project);
}
@Inject
CommentContextLoader(GitRepositoryManager repoManager, @Assisted Project.NameKey project) {
CommentContextLoader(
FileTypeRegistry registry,
GitRepositoryManager repoManager,
ProjectCache projectCache,
@Assisted Project.NameKey project) {
this.registry = registry;
this.repoManager = repoManager;
this.project = project;
projectState = projectCache.get(project).orElseThrow(illegalState(project));
}
/**
@@ -123,7 +139,8 @@ public class CommentContextLoader {
ObjectReader reader, RevCommit commit, Range commentRange, int contextPadding)
throws IOException {
Text text = Text.forCommit(reader, commit);
return createContext(text, commentRange, contextPadding);
return createContext(
text, commentRange, contextPadding, FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE);
}
private CommentContext getContextForMergeList(
@@ -131,7 +148,8 @@ public class CommentContextLoader {
throws IOException {
ComparisonType cmp = ComparisonType.againstParent(1);
Text text = Text.forMergeList(cmp, reader, commit);
return createContext(text, commentRange, contextPadding);
return createContext(
text, commentRange, contextPadding, FileContentUtil.TEXT_X_GERRIT_MERGE_LIST);
}
private CommentContext getContextForFilePath(
@@ -152,11 +170,23 @@ public class CommentContextLoader {
}
ObjectId id = tw.getObjectId(0);
Text src = new Text(repo.open(id, Constants.OBJ_BLOB));
return createContext(src, commentRange, contextPadding);
String contentType = getContentType(tw, filePath, src);
return createContext(src, commentRange, contextPadding, contentType);
}
}
private static CommentContext createContext(Text src, Range commentRange, int contextPadding) {
private String getContentType(TreeWalk tw, String filePath, Text src) {
PatchScript.FileMode fileMode = PatchScript.FileMode.fromJgitFileMode(tw.getFileMode(0));
String mimeType = MimeUtil2.UNKNOWN_MIME_TYPE.toString();
if (src.size() > 0 && PatchScript.FileMode.SYMLINK != fileMode) {
MimeType registryMimeType = registry.getMimeType(filePath, src.getContent());
mimeType = registryMimeType.toString();
}
return FileContentUtil.resolveContentType(projectState, filePath, fileMode, mimeType);
}
private static CommentContext createContext(
Text src, Range commentRange, int contextPadding, String contentType) {
if (commentRange.start() < 1 || commentRange.end() - 1 > src.size()) {
throw new StorageException(
"Invalid comment range "
@@ -171,7 +201,7 @@ public class CommentContextLoader {
for (int i = commentRange.start(); i < commentRange.end(); i++) {
context.put(i, src.getString(i - 1));
}
return CommentContext.create(context.build());
return CommentContext.create(context.build(), contentType);
}
/**

View File

@@ -405,12 +405,7 @@ class PatchScriptBuilder {
if (mode == FileMode.MISSING) {
displayMethod = DisplayMethod.NONE;
}
PatchScript.FileMode fileMode = PatchScript.FileMode.FILE;
if (mode == FileMode.SYMLINK) {
fileMode = PatchScript.FileMode.SYMLINK;
} else if (mode == FileMode.GITLINK) {
fileMode = PatchScript.FileMode.GITLINK;
}
PatchScript.FileMode fileMode = PatchScript.FileMode.fromJgitFileMode(mode);
return new PatchSide(
treeId, path, id, mode, srcContent, src, mimeType, displayMethod, fileMode);
}

View File

@@ -171,7 +171,10 @@ public class CommentJson {
allComments.stream().map(this::createCommentContextKey).collect(toList());
ImmutableMap<CommentContextKey, CommentContext> allContext = commentContextCache.getAll(keys);
for (T c : allComments) {
c.contextLines = toContextLineInfoList(allContext.get(createCommentContextKey(c)));
CommentContextKey contextKey = createCommentContextKey(c);
CommentContext commentContext = allContext.get(contextKey);
c.contextLines = toContextLineInfoList(commentContext);
c.sourceContentType = commentContext.contentType();
}
}

View File

@@ -33,6 +33,7 @@ import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.ContextLineInfo;
import com.google.gerrit.server.change.FileContentUtil;
import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.List;
@@ -323,6 +324,82 @@ public class CommentContextIT extends AbstractDaemonTest {
IntStream.range(200, 301).boxed().collect(ImmutableList.toImmutableList()));
}
@Test
public void commentContextReturnsCorrectContentTypeForCommitMessage() throws Exception {
PushOneCommit.Result result =
createChange(testRepo, "master", SUBJECT, FILE_NAME, FILE_CONTENT, "topic");
String changeId = result.getChangeId();
String ps1 = result.getCommit().name();
CommentInput comment = CommentsUtil.newComment(COMMIT_MSG, Side.REVISION, 7, "comment", false);
CommentsUtil.addComments(gApi, changeId, ps1, comment);
List<CommentInfo> comments =
gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList();
assertThat(comments).hasSize(1);
assertThat(comments.get(0).path).isEqualTo(COMMIT_MSG);
assertThat(comments.get(0).sourceContentType)
.isEqualTo(FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE);
}
@Test
public void commentContextReturnsCorrectContentType_Java() throws Exception {
String javaContent =
"public class Main {\n"
+ " public static void main(String[]args){\n"
+ " if(args==null){\n"
+ " System.err.println(\"Something\");\n"
+ " }\n"
+ " }\n"
+ " }";
String fileName = "src.java";
String changeId = createChangeWithContent(fileName, javaContent, /* line= */ 4);
List<CommentInfo> comments =
gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList();
assertThat(comments).hasSize(1);
assertThat(comments.get(0).path).isEqualTo(fileName);
assertThat(comments.get(0).contextLines)
.isEqualTo(createContextLines("4", " System.err.println(\"Something\");"));
assertThat(comments.get(0).sourceContentType).isEqualTo("text/x-java");
}
@Test
public void commentContextReturnsCorrectContentType_Cpp() throws Exception {
String cppContent =
"#include <iostream>\n"
+ "\n"
+ "int main() {\n"
+ " std::cout << \"Hello World!\";\n"
+ " return 0;\n"
+ "}";
String fileName = "src.cpp";
String changeId = createChangeWithContent(fileName, cppContent, /* line= */ 4);
List<CommentInfo> comments =
gApi.changes().id(changeId).commentsRequest().withContext(true).getAsList();
assertThat(comments).hasSize(1);
assertThat(comments.get(0).path).isEqualTo(fileName);
assertThat(comments.get(0).contextLines)
.isEqualTo(createContextLines("4", " std::cout << \"Hello World!\";"));
assertThat(comments.get(0).sourceContentType).isEqualTo("text/x-c++src");
}
private String createChangeWithContent(String fileName, String fileContent, int line)
throws Exception {
PushOneCommit.Result result =
createChange(testRepo, "master", SUBJECT, fileName, fileContent, "topic");
String changeId = result.getChangeId();
String ps1 = result.getCommit().name();
CommentInput comment = CommentsUtil.newComment(fileName, Side.REVISION, line, "comment", false);
CommentsUtil.addComments(gApi, changeId, ps1, comment);
return changeId;
}
private String createChangeWithComment(int startLine, int endLine) throws Exception {
PushOneCommit.Result result =
createChange(testRepo, "master", SUBJECT, FILE_NAME, FILE_CONTENT, "topic");

View File

@@ -14,7 +14,7 @@ public class CommentContextSerializerTest {
@Test
public void roundTripValue() {
CommentContext commentContext =
CommentContext.create(ImmutableMap.of(1, "line_1", 2, "line_2"));
CommentContext.create(ImmutableMap.of(1, "line_1", 2, "line_2"), "text/x-java");
byte[] serialized = INSTANCE.serialize(commentContext);
CommentContext deserialized = INSTANCE.deserialize(serialized);

View File

@@ -524,13 +524,14 @@ message CommentContextKeyProto {
}
// Serialized form of a list of com.google.gerrit.extensions.common.ContextLineInfo
// Next ID: 2
// Next ID: 3
message AllCommentContextProto {
message CommentContextProto {
int32 line_number = 1;
string context_line = 2;
}
repeated CommentContextProto context = 1;
string content_type = 2;
}
// Serialized key for