Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: Update git submodules Update git submodules Update git submodules Apply diff preferences immediately after clicking save Grant the InternalUser back access to changes Rewrite upload archive tests as real integration tests Add support for Elasticsearch version 7.7.* Change-Id: I5c1a23713d3eeacc4ae36d039d6d8956a2a2ff1b
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index abfb49d..498856e 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt
@@ -342,6 +342,7 @@ * elastic * git * git-protocol-v2 +* git-upload-archive * notedb * pgm * rest
diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java index da93610..ea69971 100644 --- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java +++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -42,6 +42,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; +import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import org.eclipse.jgit.lib.Config; @@ -207,6 +208,22 @@ ImmutableList<String> cmd, File dir, ImmutableMap<String, String> env) throws IOException { ProcessBuilder pb = new ProcessBuilder(cmd); pb.directory(dir).redirectErrorStream(true); + return execute(cmd, dir, env, null); + } + + protected static String execute( + ImmutableList<String> cmd, + File dir, + ImmutableMap<String, String> env, + @Nullable Path outputPath) + throws IOException { + ProcessBuilder pb = new ProcessBuilder(cmd); + pb.directory(dir); + if (outputPath != null) { + pb.redirectOutput(outputPath.toFile()); + } else { + pb.redirectErrorStream(true); + } pb.environment().putAll(env); Process p = pb.start(); byte[] out;
diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index fb24cb0..746a386 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
@@ -27,7 +27,8 @@ V7_3("7.3.*"), V7_4("7.4.*"), V7_5("7.5.*"), - V7_6("7.6.*"); + V7_6("7.6.*"), + V7_7("7.7.*"); private final String version; private final Pattern pattern;
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java index 4ac823b..819fc2b 100644 --- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java +++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -21,6 +21,7 @@ import com.google.gerrit.index.query.IsVisibleToPredicate; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.InternalUser; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.permissions.ChangePermission; @@ -94,7 +95,7 @@ ? permissionBackend.absentUser(user.getAccountId()) : permissionBackend.user( Optional.of(user) - .filter(u -> u instanceof SingleGroupUser) + .filter(u -> u instanceof SingleGroupUser || u instanceof InternalUser) .orElseGet(anonymousUserProvider::get)); try { withUser.indexedChange(cd, notes).check(ChangePermission.READ);
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java index 18c2e72..38b7e0e 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -32,7 +32,7 @@ @ConfigSuite.Config public static Config elasticsearchV7() { - return getConfig(ElasticVersion.V7_6); + return getConfig(ElasticVersion.V7_7); } @Override
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java index f130d1f8..b539bf8 100644 --- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java +++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -33,7 +33,7 @@ @ConfigSuite.Config public static Config elasticsearchV7() { - return getConfig(ElasticVersion.V7_6); + return getConfig(ElasticVersion.V7_7); } @Override
diff --git a/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java b/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java deleted file mode 100644 index 34406e0..0000000 --- a/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java +++ /dev/null
@@ -1,165 +0,0 @@ -// Copyright (C) 2015 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.acceptance.ssh; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; - -import com.google.common.base.Splitter; -import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.GerritConfig; -import com.google.gerrit.acceptance.NoHttpd; -import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.PushOneCommit.Result; -import com.google.gerrit.acceptance.UseSsh; -import com.google.gerrit.git.ObjectIds; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.InputStream; -import java.util.Set; -import java.util.TreeSet; -import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; -import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; -import org.eclipse.jgit.transport.PacketLineIn; -import org.eclipse.jgit.transport.PacketLineOut; -import org.eclipse.jgit.util.IO; -import org.junit.Test; - -@NoHttpd -@UseSsh -public class UploadArchiveIT extends AbstractDaemonTest { - - @Test - @GerritConfig(name = "download.archive", value = "off") - public void archiveFeatureOff() throws Exception { - assertArchiveNotPermitted(); - } - - @Test - @GerritConfig( - name = "download.archive", - values = {"tar", "tbz2", "tgz", "txz"}) - public void zipFormatDisabled() throws Exception { - assertArchiveNotPermitted(); - } - - @Test - public void zipFormat() throws Exception { - PushOneCommit.Result r = createChange(); - String abbreviated = abbreviateName(r); - String c = command(r, "zip", abbreviated); - - InputStream out = - adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c)); - - // Wrap with PacketLineIn to read ACK bytes from output stream - PacketLineIn in = new PacketLineIn(out); - String tmp = in.readString(); - assertThat(tmp).isEqualTo("ACK"); - in.readString(); - - // Skip length (4 bytes) + 1 byte - // to position the output stream to the raw zip stream - byte[] buffer = new byte[5]; - IO.readFully(out, buffer, 0, 5); - Set<String> entryNames = new TreeSet<>(); - try (ZipArchiveInputStream zip = new ZipArchiveInputStream(out)) { - ZipArchiveEntry zipEntry = zip.getNextZipEntry(); - while (zipEntry != null) { - String name = zipEntry.getName(); - entryNames.add(name); - zipEntry = zip.getNextZipEntry(); - } - } - - assertThat(entryNames) - .containsExactly( - String.format("%s/", abbreviated), - String.format("%s/%s", abbreviated, PushOneCommit.FILE_NAME)) - .inOrder(); - } - - // Make sure we have coverage for the dependency on xz. - @Test - public void txzFormat() throws Exception { - PushOneCommit.Result r = createChange(); - String abbreviated = abbreviateName(r); - String c = command(r, "tar.xz", abbreviated); - - try (InputStream out = - adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c))) { - - // Wrap with PacketLineIn to read ACK bytes from output stream - PacketLineIn in = new PacketLineIn(out); - String packet = in.readString(); - assertThat(packet).isEqualTo("ACK"); - - // Discard first bit of data, which should be empty. - packet = in.readString(); - assertThat(packet).isEmpty(); - - // Make sure the next one is not on the error channel - packet = in.readString(); - - // 1 = DATA. It would be nicer to parse the OutputStream with SideBandInputStream from JGit, - // but - // that is currently not public. - char channel = packet.charAt(0); - if (channel != 1) { - assertWithMessage("got packet on channel " + (int) channel, packet).fail(); - } - } - } - - private String command(PushOneCommit.Result r, String format, String abbreviated) { - String c = - String.format( - "-f=%s --prefix=%s/ %s %s", - format, abbreviated, r.getCommit().name(), PushOneCommit.FILE_NAME); - return c; - } - - private void assertArchiveNotPermitted() throws Exception { - PushOneCommit.Result r = createChange(); - String abbreviated = abbreviateName(r); - String c = command(r, "zip", abbreviated); - - InputStream out = - adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c)); - - // Wrap with PacketLineIn to read ACK bytes from output stream - PacketLineIn in = new PacketLineIn(out); - String tmp = in.readString(); - assertThat(tmp).isEqualTo("ACK"); - in.readString(); - tmp = in.readString(); - tmp = tmp.substring(1); - assertThat(tmp).isEqualTo("fatal: upload-archive not permitted for format zip"); - } - - private String abbreviateName(Result r) throws Exception { - return ObjectIds.abbreviateName(r.getCommit(), 8, testRepo.getRevWalk().getObjectReader()); - } - - private InputStream argumentsToInputStream(String c) throws Exception { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - PacketLineOut pctOut = new PacketLineOut(out); - for (String arg : Splitter.on(' ').split(c)) { - pctOut.writeString("argument " + arg); - } - pctOut.end(); - return new ByteArrayInputStream(out.toByteArray()); - } -}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index df3ec5e..15094fd 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -58,6 +58,8 @@ return "blacktop/elasticsearch:7.5.2"; case V7_6: return "blacktop/elasticsearch:7.6.2"; + case V7_7: + return "blacktop/elasticsearch:7.7.0"; } throw new IllegalStateException("No tests for version: " + version.name()); }
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java index 7636e5d..a015103 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
@@ -36,7 +36,7 @@ public static void startIndexService() { if (container == null) { // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V7_6); + container = ElasticContainer.createAndStart(ElasticVersion.V7_7); } }
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java index cbb8300d..b1de591 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
@@ -46,7 +46,7 @@ public static void startIndexService() { if (container == null) { // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V7_6); + container = ElasticContainer.createAndStart(ElasticVersion.V7_7); client = HttpAsyncClients.createDefault(); client.start(); }
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java index 5e3c4fb..2e382d4 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
@@ -36,7 +36,7 @@ public static void startIndexService() { if (container == null) { // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V7_6); + container = ElasticContainer.createAndStart(ElasticVersion.V7_7); } }
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java index 803b5da..87a14da 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
@@ -36,7 +36,7 @@ public static void startIndexService() { if (container == null) { // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V7_6); + container = ElasticContainer.createAndStart(ElasticVersion.V7_7); } }
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index 280b884..c9a7a46 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -51,6 +51,9 @@ assertThat(ElasticVersion.forVersion("7.6.0")).isEqualTo(ElasticVersion.V7_6); assertThat(ElasticVersion.forVersion("7.6.1")).isEqualTo(ElasticVersion.V7_6); + + assertThat(ElasticVersion.forVersion("7.7.0")).isEqualTo(ElasticVersion.V7_7); + assertThat(ElasticVersion.forVersion("7.7.1")).isEqualTo(ElasticVersion.V7_7); } @Test @@ -77,6 +80,7 @@ assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); } @Test @@ -91,6 +95,7 @@ assertThat(ElasticVersion.V7_4.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_5.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_6.isV6OrLater()).isTrue(); + assertThat(ElasticVersion.V7_7.isV6OrLater()).isTrue(); } @Test @@ -105,5 +110,6 @@ assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue(); assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue(); assertThat(ElasticVersion.V7_6.isV7OrLater()).isTrue(); + assertThat(ElasticVersion.V7_7.isV7OrLater()).isTrue(); } }
diff --git a/javatests/com/google/gerrit/integration/git/BUILD b/javatests/com/google/gerrit/integration/git/BUILD index 4184f1f..28755af 100644 --- a/javatests/com/google/gerrit/integration/git/BUILD +++ b/javatests/com/google/gerrit/integration/git/BUILD
@@ -1,7 +1,13 @@ load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests") acceptance_tests( - srcs = glob(["*IT.java"]), - group = "git", + srcs = ["GitProtocolV2IT.java"], + group = "protocol-v2", labels = ["git-protocol-v2"], ) + +acceptance_tests( + srcs = ["UploadArchiveIT.java"], + group = "upload-archive", + labels = ["git-upload-archive"], +)
diff --git a/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java b/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java new file mode 100644 index 0000000..2968111 --- /dev/null +++ b/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java
@@ -0,0 +1,211 @@ +// 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.integration.git; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.PushOneCommit.FILE_CONTENT; +import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.gerrit.acceptance.GerritConfig; +import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress; +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.StandaloneSiteTest; +import com.google.gerrit.acceptance.UseSsh; +import com.google.gerrit.common.RawInputUtil; +import com.google.gerrit.entities.Project; +import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.extensions.common.CommitInfo; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.inject.Inject; +import java.io.BufferedInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetSocketAddress; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Set; +import java.util.TreeSet; +import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; +import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream; +import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; +import org.apache.commons.compress.compressors.xz.XZCompressorInputStream; +import org.junit.Test; + +@NoHttpd +@UseSsh +public class UploadArchiveIT extends StandaloneSiteTest { + private static final String[] SSH_KEYGEN_CMD = + new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"}; + private static final String GIT_SSH_COMMAND = + "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o 'IdentitiesOnly yes' -i"; + private static final String ARCHIVE = "archive"; + + @Inject private GerritApi gApi; + @Inject private @TestSshServerAddress InetSocketAddress sshAddress; + + private String sshDestination; + private String identityPath; + private Project.NameKey project; + private CommitInfo commit; + + @Test + @GerritConfig(name = "download.archive", value = "off") + public void archiveFeatureOff() throws Exception { + try (ServerContext ctx = startServer()) { + setUpTestHarness(ctx); + assertArchiveNotPermitted(); + } + } + + @Test + @GerritConfig( + name = "download.archive", + values = {"tar", "tbz2", "tgz", "txz"}) + public void zipFormatDisabled() throws Exception { + try (ServerContext ctx = startServer()) { + setUpTestHarness(ctx); + assertArchiveNotPermitted(); + } + } + + @Test + public void verifyUploadArchiveFormats() throws Exception { + try (ServerContext ctx = startServer()) { + setUpTestHarness(ctx); + setUpChange(); + for (String f : Arrays.asList("zip", "tar", "tar.gz", "tar.bz2", "tar.xz")) { + verifyUploadArchive(f); + } + } + } + + private void verifyUploadArchive(String format) throws Exception { + Path outputPath = sitePaths.data_dir.resolve(ARCHIVE); + execute( + cmd(format, commit.commit), + sitePaths.data_dir.toFile(), + ImmutableMap.of("GIT_SSH_COMMAND", GIT_SSH_COMMAND + identityPath), + outputPath); + try (InputStream fi = Files.newInputStream(outputPath); + InputStream bi = new BufferedInputStream(fi); + ArchiveInputStream archive = archiveStreamForFormat(bi, format)) { + assertEntries(archive); + } + } + + private ArchiveInputStream archiveStreamForFormat(InputStream bi, String format) + throws IOException { + switch (format) { + case "zip": + return new ZipArchiveInputStream(bi); + case "tar": + return new TarArchiveInputStream(bi); + case "tar.gz": + return new TarArchiveInputStream(new GzipCompressorInputStream(bi)); + case "tar.bz2": + return new TarArchiveInputStream(new BZip2CompressorInputStream(bi)); + case "tar.xz": + return new TarArchiveInputStream(new XZCompressorInputStream(bi)); + default: + throw new IllegalArgumentException("Unknown archive format: " + format); + } + } + + private void setUpTestHarness(ServerContext ctx) throws RestApiException, Exception { + ctx.getInjector().injectMembers(this); + project = Project.nameKey("upload-archive-project-test"); + gApi.projects().create(project.get()); + setUpAuthentication(); + sshDestination = + String.format( + "ssh://%s@%s:%s/%s", + "admin", sshAddress.getHostName(), sshAddress.getPort(), project.get()); + identityPath = sitePaths.data_dir.resolve(String.format("id_rsa_%s", "admin")).toString(); + } + + private void setUpAuthentication() throws Exception { + execute( + ImmutableList.<String>builder() + .add(SSH_KEYGEN_CMD) + .add(String.format("id_rsa_%s", "admin")) + .build()); + gApi.accounts() + .id("admin") + .addSshKey( + new String( + java.nio.file.Files.readAllBytes( + sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", "admin"))), + UTF_8)); + } + + private ImmutableList<String> cmd(String format, String commit) { + return ImmutableList.<String>builder() + .add("git") + .add("archive") + .add("-f=" + format) + .add("--prefix=" + commit + "/") + .add("--remote=" + sshDestination) + .add(commit) + .add(FILE_NAME) + .build(); + } + + private String execute(ImmutableList<String> cmd) throws Exception { + return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of()); + } + + private void assertArchiveNotPermitted() { + IOException exception = + assertThrows( + IOException.class, + () -> + execute( + cmd("zip", "master"), + sitePaths.data_dir.toFile(), + ImmutableMap.of("GIT_SSH_COMMAND", GIT_SSH_COMMAND + identityPath))); + assertThat(exception) + .hasMessageThat() + .contains("fatal: upload-archive not permitted for format zip"); + } + + private void setUpChange() throws Exception { + ChangeInput in = new ChangeInput(project.get(), "master", "Test change"); + in.newBranch = true; + String changeId = gApi.changes().create(in).info().changeId; + gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(FILE_CONTENT)); + gApi.changes().id(changeId).edit().publish(); + commit = gApi.changes().id(changeId).current().commit(false); + } + + private void assertEntries(ArchiveInputStream o) throws IOException { + Set<String> entryNames = new TreeSet<>(); + ArchiveEntry e; + while ((e = o.getNextEntry()) != null) { + entryNames.add(e.getName()); + } + assertThat(entryNames) + .containsExactly( + String.format("%s/", commit.commit), String.format("%s/%s", commit.commit, FILE_NAME)) + .inOrder(); + } +}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index a57bba7..558658b 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1810,6 +1810,11 @@ AccountGroup.uuid(gApi.groups().id(g1).get().id)); assertQuery(q + " visibleto:" + g1, change1); + // Both changes are visible to InternalUser + try (ManualRequestContext ctx = oneOffRequestContext.open()) { + assertQuery(q, change2, change1); + } + requestContext.setContext(newRequestContext(user2)); assertQuery("is:visible", change1);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.html index 21f6282..b0167b3 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.html
@@ -58,7 +58,7 @@ <div class$="diffHeader [[_computeHeaderClass(_diffPrefsChanged)]]">Diff Preferences</div> <gr-diff-preferences id="diffPreferences" - diff-prefs="{{diffPrefs}}" + diff-prefs="{{_editableDiffPrefs}}" has-unsaved-changes="{{_diffPrefsChanged}}"></gr-diff-preferences> <div class="diffActions"> <gr-button
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.js b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.js index 6ecd4d6..9b723bd 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.js
@@ -24,6 +24,17 @@ /** @type {?} */ diffPrefs: Object, + /** + * _editableDiffPrefs is a clone of diffPrefs. + * All changes in the dialog are applied to this object + * immediately, when a value in an editor is changed. + * The "Save" button replaces the "diffPrefs" object with + * the value of _editableDiffPrefs. + * + * @type {?} + */ + _editableDiffPrefs: Object, + _diffPrefsChanged: Boolean, }, @@ -52,6 +63,10 @@ }, open() { + // JSON.parse(JSON.stringify(...)) makes a deep clone of diffPrefs. + // It is known, that diffPrefs is obtained from an RestAPI call and + // it is safe to clone the object this way. + this._editableDiffPrefs = JSON.parse(JSON.stringify(this.diffPrefs)); this.$.diffPrefsOverlay.open().then(() => { const focusStops = this.getFocusStops(); this.$.diffPrefsOverlay.setFocusStops(focusStops); @@ -60,6 +75,7 @@ }, _handleSaveDiffPreferences() { + this.diffPrefs = this._editableDiffPrefs; this.$.diffPreferences.save().then(() => { this.fire('reload-diff-preference', null, {bubbles: false});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog_test.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog_test.html new file mode 100644 index 0000000..156ec69 --- /dev/null +++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog_test.html
@@ -0,0 +1,63 @@ +<!DOCTYPE html> +<!-- +@license +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. +--> + +<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> +<title>gr-diff-preferences-dialog</title> + +<script src="../../../bower_components/web-component-tester/browser.js"></script> +<link rel="import" href="../../../test/common-test-setup.html"/> + +<script>void(0);</script> + +<test-fixture id="basic"> + <template> + <gr-diff-preferences-dialog></gr-diff-preferences-dialog> + </template> +</test-fixture> + +<script> + suite('gr-diff-preferences-dialog', () => { + let element; + setup(() => { + element = basicFixture.instantiate(); + }); + test('changes applies only on save', async () => { + const originalDiffPrefs = { + line_wrapping: true, + }; + element.diffPrefs = originalDiffPrefs; + + element.open(); + await flush(); + assert.isTrue(element.$.diffPreferences.$.lineWrappingInput.checked); + + MockInteractions.tap(element.$.diffPreferences.$.lineWrappingInput); + await flush(); + assert.isFalse(element.$.diffPreferences.$.lineWrappingInput.checked); + assert.isTrue(element._diffPrefsChanged); + assert.isTrue(element.diffPrefs.line_wrapping); + assert.isTrue(originalDiffPrefs.line_wrapping); + + MockInteractions.tap(element.$.saveButton); + await flush(); + // Original prefs must remains unchanged, dialog must expose a new object + assert.isTrue(originalDiffPrefs.line_wrapping); + assert.isFalse(element.diffPrefs.line_wrapping); + }); + }); +</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js index 68ed91b..4f07664 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -644,21 +644,26 @@ }, _prefsObserver(newPrefs, oldPrefs) { - // Scan the preference objects one level deep to see if they differ. - let differ = !oldPrefs; - if (newPrefs && oldPrefs) { - for (const key in newPrefs) { - if (newPrefs[key] !== oldPrefs[key]) { - differ = true; - } - } - } - - if (differ) { + if (!this._prefsEqual(newPrefs, oldPrefs)) { this._prefsChanged(newPrefs); } }, + _prefsEqual(prefs1, prefs2) { + if (prefs1 === prefs2) { + return true; + } + if (!prefs1 || !prefs2) { + return false; + } + // Scan the preference objects one level deep to see if they differ. + const keys1 = Object.keys(prefs1); + const keys2 = Object.keys(prefs2); + return keys1.length === keys2.length && + keys1.every(key => prefs1[key] === prefs2[key]) && + keys2.every(key => prefs1[key] === prefs2[key]); + }, + _pathObserver() { // Call _prefsChanged(), because line-limit style value depends on path. this._prefsChanged(this.prefs);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html index ec2f5f1..b177bad 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -701,6 +701,22 @@ assert.isTrue(element._renderDiffTable.called); }); + test('adding/removing property in preferences re-renders diff', () => { + const stub = sandbox.stub(element, '_renderDiffTable'); + const newPrefs1 = Object.assign({}, MINIMAL_PREFS, + {line_wrapping: true}); + element.prefs = newPrefs1; + element.flushDebouncer('renderDiffTable'); + assert.isTrue(element._renderDiffTable.called); + stub.reset(); + + const newPrefs2 = Object.assign({}, newPrefs1); + delete newPrefs2.line_wrapping; + element.prefs = newPrefs2; + element.flushDebouncer('renderDiffTable'); + assert.isTrue(element._renderDiffTable.called); + }); + test('change in preferences does not re-renders diff with ' + 'noRenderOnPrefsChange', () => { sandbox.stub(element, '_renderDiffTable'); @@ -972,6 +988,24 @@ assert.equal(element._computeNewlineWarningClass(null, false), hidden); assert.equal(element._computeNewlineWarningClass('foo', false), shown); }); + + test('_prefsEqual', () => { + element = fixture('basic'); + assert.isTrue(element._prefsEqual(null, null)); + assert.isTrue(element._prefsEqual({}, {})); + assert.isTrue(element._prefsEqual({x: 1}, {x: 1})); + assert.isTrue( + element._prefsEqual({x: 1, abc: 'def'}, {x: 1, abc: 'def'})); + const somePref = {abc: 'def', p: true}; + assert.isTrue(element._prefsEqual(somePref, somePref)); + + assert.isFalse(element._prefsEqual({}, null)); + assert.isFalse(element._prefsEqual(null, {})); + assert.isFalse(element._prefsEqual({x: 1}, {x: 2})); + assert.isFalse(element._prefsEqual({x: 1, y: 'abc'}, {x: 1, y: 'abcd'})); + assert.isFalse(element._prefsEqual({x: 1, y: 'abc'}, {x: 1})); + assert.isFalse(element._prefsEqual({x: 1}, {x: 1, y: 'abc'})); + }); }); suite('key locations', () => {
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index c07e853..5270df5 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl
@@ -102,8 +102,8 @@ # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.6.2", - sha1 = "3da6691dcd1864243f11f07d51907320452c6400", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.7.0", + sha1 = "5fc25eec3940bc0e9b0ffddcf50554a609e9db8e", ) maven_jar(