Merge "Switch off intraline diffs for some cases (invalid cache key sharing)"
diff --git a/WORKSPACE b/WORKSPACE
index e370f69..307e28c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -235,6 +235,31 @@
sha1 = GUAVA_BIN_SHA1,
)
+CAFFEINE_VERS = "2.8.0"
+
+maven_jar(
+ name = "caffeine",
+ artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
+ sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
+)
+
+# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
+# naming collision between caffeine guava adapater and guava library itself.
+# Remove this renaming procedure, once this upstream issue is fixed:
+# https://github.com/ben-manes/caffeine/issues/364.
+http_file(
+ name = "caffeine-guava-renamed",
+ downloaded_file_path = "caffeine-guava-" + CAFFEINE_VERS + ".jar",
+ sha256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1",
+ urls = [
+ "https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/guava/" +
+ CAFFEINE_VERS +
+ "/guava-" +
+ CAFFEINE_VERS +
+ ".jar",
+ ],
+)
+
maven_jar(
name = "guava-failureaccess",
artifact = "com.google.guava:failureaccess:1.0.1",
diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
index 309ee3e5..574a226 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
@@ -30,7 +30,8 @@
V7_1("7.1.*"),
V7_2("7.2.*"),
V7_3("7.3.*"),
- V7_4("7.4.*");
+ V7_4("7.4.*"),
+ V7_5("7.5.*");
private final String version;
private final Pattern pattern;
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 086a6a4..1435c5e 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -282,6 +282,12 @@
return;
}
+ if (edits.isEmpty() && (aSize != bSize)) {
+ // Only edits due to rebase were present. If we now added the edits for the newlines, the
+ // code which later assembles the file contents would fail.
+ return;
+ }
+
Optional<Edit> lastEdit = getLast(edits);
if (isNewlineAtEndDeleted(a, b)) {
Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 8a9cdf5..d4a4c45 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -42,6 +42,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
@@ -597,6 +598,109 @@
}
@Test
+ public void addedNewlineAtEndOfFileIsMarkedInDiffWhenOtherwiseOnlyEditsDueToRebaseExist()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70 modification
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(102);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void addedNewlineAtEndOfFileIsNotIdentifiedAsDueToRebaseEvenThoughItShould()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ // Add a comment so that file contents are not 'skipped'. To be able to add a comment, touch
+ // (= modify) the file in the change.
+ addModifiedPatchSet(
+ changeId, FILE_NAME, fileContent -> fileContent.replace("Line 2\n", "Line two\n"));
+ CommentInput comment = createCommentInput(3, 0, 4, 0, "Comment to not skip file content.");
+ addCommentTo(changeId, CURRENT, FILE_NAME, comment);
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfA().isNull();
+ assertThat(diffInfo).content().element(1).linesOfB().containsExactly("");
+ // This should actually be isDueToRebase().
+ assertThat(diffInfo).content().element(1).isNotDueToRebase();
+ }
+
+ @Test
+ public void
+ addedNewlineAtEndOfFileIsMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceConsidered()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70.5 insertion
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(103);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void
+ addedNewlineAtEndOfFileIsNotMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceIgnoredEvenThoughItShould()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
+ }
+
+ @Test
public void addedLastLineWithoutNewlineBeforeAndAfterwardsIsMarkedInDiff() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
@@ -2393,9 +2497,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2414,6 +2516,112 @@
}
@Test
+ public void
+ diffOfFileWithOnlyRebaseHunksAndWithCommentAndConsideringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void diffOfFileWithOnlyRebaseHunksAndWithCommentAndIgnoringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\nLine 102\nLine 103\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.concat("Line 101\nLine 103\nLine 104");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
public void diffOfNonExistentFileIsAnEmptyDiffResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
@@ -2451,9 +2659,7 @@
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(20, 0, 21, 0, "Should be 'Line 20'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2495,9 +2701,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
String newFilePath = "a_new_file.txt";
gApi.changes().id(changeId).edit().renameFile(FILE_NAME, newFilePath);
gApi.changes().id(changeId).edit().publish();
@@ -2526,6 +2730,14 @@
return comment;
}
+ private void addCommentTo(
+ String changeId, String previousPatchSetId, String fileName, CommentInput comment)
+ throws RestApiException {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.comments = ImmutableMap.of(fileName, ImmutableList.of(comment));
+ gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ }
+
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
index dcdca86..eeeacab 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -37,7 +37,7 @@
@ConfigSuite.Config
public static Config elasticsearchV7() {
- return getConfig(ElasticVersion.V7_4);
+ return getConfig(ElasticVersion.V7_5);
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
index 1c41b55..1940274 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -38,7 +38,7 @@
@ConfigSuite.Config
public static Config elasticsearchV7() {
- return getConfig(ElasticVersion.V7_4);
+ return getConfig(ElasticVersion.V7_5);
}
@Override
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index c15f1a7..c692a3b 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -62,6 +62,8 @@
return "blacktop/elasticsearch:7.3.2";
case V7_4:
return "blacktop/elasticsearch:7.4.2";
+ case V7_5:
+ return "blacktop/elasticsearch:7.5.0";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
index da64d53..1601756 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
@@ -14,6 +14,8 @@
package com.google.gerrit.elasticsearch;
+import static java.util.concurrent.TimeUnit.MINUTES;
+
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
import com.google.gerrit.server.query.change.AbstractQueryChangesTest;
import com.google.gerrit.testing.ConfigSuite;
@@ -22,7 +24,12 @@
import com.google.gerrit.testing.IndexConfig;
import com.google.inject.Guice;
import com.google.inject.Injector;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
+import org.apache.http.impl.nio.client.HttpAsyncClients;
import org.eclipse.jgit.lib.Config;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -35,6 +42,7 @@
private static ElasticNodeInfo nodeInfo;
private static ElasticContainer container;
+ private static CloseableHttpAsyncClient client;
@BeforeClass
public static void startIndexService() {
@@ -45,6 +53,8 @@
container = ElasticContainer.createAndStart(ElasticVersion.V6_8);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
+ client = HttpAsyncClients.createDefault();
+ client.start();
}
@AfterClass
@@ -56,6 +66,19 @@
@Rule public final GerritTestName testName = new GerritTestName();
+ @After
+ public void closeIndex() throws Exception {
+ client
+ .execute(
+ new HttpPost(
+ String.format(
+ "http://localhost:%d/%s*/_close",
+ nodeInfo.port, testName.getSanitizedMethodName())),
+ HttpClientContext.create(),
+ null)
+ .get(5, MINUTES);
+ }
+
@Override
protected void initAfterLifecycleStart() throws Exception {
super.initAfterLifecycleStart();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
index 021a80d..c55417e 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
index 6bb2f8fb..b4feb32 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
@@ -51,7 +51,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
client = HttpAsyncClients.createDefault();
client.start();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
index 9312f01..c2d6246 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
index 4b9ede8..2fc4366 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V7_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V7_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
index f9cfe35..5d2f99c 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -60,6 +60,9 @@
assertThat(ElasticVersion.forVersion("7.4.0")).isEqualTo(ElasticVersion.V7_4);
assertThat(ElasticVersion.forVersion("7.4.1")).isEqualTo(ElasticVersion.V7_4);
+
+ assertThat(ElasticVersion.forVersion("7.5.0")).isEqualTo(ElasticVersion.V7_5);
+ assertThat(ElasticVersion.forVersion("7.5.1")).isEqualTo(ElasticVersion.V7_5);
}
@Test
@@ -89,6 +92,7 @@
assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
+ assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
}
@Test
@@ -106,6 +110,7 @@
assertThat(ElasticVersion.V7_2.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V7_3.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V7_4.isV6OrLater()).isTrue();
+ assertThat(ElasticVersion.V7_5.isV6OrLater()).isTrue();
}
@Test
@@ -123,5 +128,6 @@
assertThat(ElasticVersion.V7_2.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_3.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue();
+ assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue();
}
}
diff --git a/lib/BUILD b/lib/BUILD
index 39c622a..199f4fb 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -528,7 +528,7 @@
"//java/com/google/gerrit/acceptance:__pkg__",
"//java/com/google/gerrit/extensions:__pkg__",
"//java/com/google/gerrit/server:__pkg__",
- "//plugins:__pkg__",
+ "//plugins:__subpackages__",
],
exports = ["@javax-annotation//jar"],
)
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 2a57162..01e1f1f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -77,7 +77,6 @@
properties: {
diff: Object,
- diffPath: String,
changeNum: String,
patchNum: String,
viewMode: String,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index f5dc438..bdb914f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -301,7 +301,7 @@
const layers = [this.$.syntaxLayer];
// Get layers from plugins (if any).
for (const pluginLayer of this.$.jsAPI.getDiffLayers(
- this.diffPath, this.changeNum, this.patchNum)) {
+ this.path, this.changeNum, this.patchNum)) {
layers.push(pluginLayer);
}
this._layers = layers;
@@ -313,7 +313,9 @@
this._coverageRanges = [];
const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this;
- this.$.jsAPI.getCoverageRanges(changeNum, path, basePatchNum, patchNum).
+ this.$.jsAPI.getCoverageRanges(
+ changeNum, path, basePatchNum, patchNum
+ ).
then(coverageRanges => {
if (changeNum !== this.changeNum ||
path !== this.path ||
@@ -344,6 +346,8 @@
return this._loadDiffAssets(diff);
});
+ // Not waiting for getCoverageRanges intentionally as
+ // plugin loading should not block the content rendering
return Promise.all([diffRequest, assetRequest])
.then(results => {
const diff = results[0];
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 1446fd4..3b64f55 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -373,7 +373,7 @@
coverage-ranges="[[coverageRanges]]"
project-name="[[projectName]]"
diff="[[diff]]"
- diff-path="[[path]]"
+ path="[[path]]"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
view-mode="[[viewMode]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
index 1802a4a..ae08007 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
@@ -270,10 +270,26 @@
// Only one coverage provider makes sense. If there are more, then we
// simply ignore them.
if (provider) {
- return provider(changeNum, path, basePatchNum, patchNum);
+ return annotationApi;
}
}
- return [];
+ return null;
+ }).then(annotationApi => {
+ if (!annotationApi) return [];
+ const provider = annotationApi.getCoverageProvider();
+ return provider(changeNum, path, basePatchNum, patchNum)
+ .then(ranges => {
+ ranges = ranges || [];
+ // Notify with the coverage data.
+ ranges.forEach(range => {
+ annotationApi.notify(
+ path,
+ range.code_range.start_line,
+ range.code_range.end_line,
+ range.side);
+ });
+ return ranges;
+ });
});
}
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 1dfc2f4..6a29da6 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -69,8 +69,8 @@
# elasticsearch-rest-client explicitly depends on this version
maven_jar(
name = "httpcore-nio",
- artifact = "org.apache.httpcomponents:httpcore-nio:4.4.11",
- sha1 = "7d0a97d01d39cff9aa3e6db81f21fddb2435f4e6",
+ artifact = "org.apache.httpcomponents:httpcore-nio:4.4.12",
+ sha1 = "84cd29eca842f31db02987cfedea245af020198b",
)
maven_jar(
@@ -102,8 +102,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.4.2",
- sha1 = "f48725523c0b3402f869214433602f8d3f4c737c",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.0",
+ sha1 = "62535b6fc3a4e943e88e7640eac22e29f03a696d",
)
maven_jar(
