opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.16' into stable-3.0

Browse Source 
* stable-2.16:
  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: I531f7336bdbe191b5d88bbc1282a59861250c897
This commit is contained in:
David Pursehouse
2020-05-15 10:16:56 +09:00
parent 518d3ddffc 4415cc33c7
commit aa7a8a0946
22 changed files with 429 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Documentation/dev-bazel.txt
View File

@@ -343,6 +343,7 @@ The following values are currently supported for the group name:
* edit
* elastic
* git
* git-upload-archive
* notedb
* pgm
* rest

55
java/com/google/gerrit/acceptance/StandaloneSiteTest.java
View File

@@ -15,10 +15,15 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static org.junit.Assert.fail;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Streams;
import com.google.common.io.ByteStreams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.groups.GroupInput;
@@ -34,6 +39,10 @@ import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Injector;
import com.google.inject.Module;
import java.io.File;
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;
@@ -214,4 +223,50 @@ public abstract class StandaloneSiteTest {
protected static void runGerrit(Iterable<String>... multiArgs) throws Exception {
runGerrit(Arrays.stream(multiArgs).flatMap(Streams::stream).toArray(String[]::new));
}
protected static String execute(
ImmutableList<String> cmd, File dir, ImmutableMap<String, String> env) throws IOException {
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;
try (InputStream in = p.getInputStream()) {
out = ByteStreams.toByteArray(in);
} finally {
p.getOutputStream().close();
}
int status;
try {
status = p.waitFor();
} catch (InterruptedException e) {
InterruptedIOException iioe =
new InterruptedIOException(
"interrupted waiting for: " + Joiner.on(' ').join(pb.command()));
iioe.initCause(e);
throw iioe;
}
String result = new String(out, UTF_8);
if (status != 0) {
throw new IOException(result);
}
return result.trim();
}
}

3
java/com/google/gerrit/elasticsearch/ElasticVersion.java
View File

@@ -27,7 +27,8 @@ public enum ElasticVersion {
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;

3
java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
View File

@@ -21,6 +21,7 @@ import com.google.gerrit.index.query.IsVisibleToPredicate;
import com.google.gerrit.reviewdb.client.Change;
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 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
? 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);

2
javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
View File

@@ -32,7 +32,7 @@ public class ElasticReindexIT extends AbstractReindexTests {
@ConfigSuite.Config
public static Config elasticsearchV7() {
return getConfig(ElasticVersion.V7_6);
return getConfig(ElasticVersion.V7_7);
}
@Override

2
javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
View File

@@ -31,7 +31,7 @@ public class ElasticIndexIT extends AbstractIndexTests {
@ConfigSuite.Config
public static Config elasticsearchV7() {
return getConfig(ElasticVersion.V7_6);
return getConfig(ElasticVersion.V7_7);
}
@Override

158
javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java
View File

@@ -1,158 +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 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.UseSsh;
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 = r.getCommit().abbreviate(8).name();
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 = r.getCommit().abbreviate(8).name();
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) {
fail("got packet on channel " + (int) channel, packet);
}
}
}
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 = r.getCommit().abbreviate(8).name();
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 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());
}
}

2
javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
View File

@@ -58,6 +58,8 @@ public class ElasticContainer extends ElasticsearchContainer {
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());
}

2
javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
View File

@@ -36,7 +36,7 @@ public class ElasticV7QueryAccountsTest extends AbstractQueryAccountsTest {
public static void startIndexService() {
if (container == null) {
// Only start Elasticsearch once
container = ElasticContainer.createAndStart(ElasticVersion.V7_6);
container = ElasticContainer.createAndStart(ElasticVersion.V7_7);
}
}

2
javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
View File

@@ -42,7 +42,7 @@ public class ElasticV7QueryChangesTest extends AbstractQueryChangesTest {
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();
}

2
javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
View File

@@ -36,7 +36,7 @@ public class ElasticV7QueryGroupsTest extends AbstractQueryGroupsTest {
public static void startIndexService() {
if (container == null) {
// Only start Elasticsearch once
container = ElasticContainer.createAndStart(ElasticVersion.V7_6);
container = ElasticContainer.createAndStart(ElasticVersion.V7_7);
}
}

2
javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
View File

@@ -36,7 +36,7 @@ public class ElasticV7QueryProjectsTest extends AbstractQueryProjectsTest {
public static void startIndexService() {
if (container == null) {
// Only start Elasticsearch once
container = ElasticContainer.createAndStart(ElasticVersion.V7_6);
container = ElasticContainer.createAndStart(ElasticVersion.V7_7);
}
}

6
javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
View File

@@ -51,6 +51,9 @@ public class ElasticVersionTest extends GerritBaseTests {
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
@@ -73,6 +76,7 @@ public class ElasticVersionTest extends GerritBaseTests {
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
@@ -87,6 +91,7 @@ public class ElasticVersionTest extends GerritBaseTests {
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
@@ -101,5 +106,6 @@ public class ElasticVersionTest extends GerritBaseTests {
assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_6.isV7OrLater()).isTrue();
assertThat(ElasticVersion.V7_7.isV7OrLater()).isTrue();
}
}

7
javatests/com/google/gerrit/integration/git/BUILD Normal file
View File

@@ -0,0 +1,7 @@
load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
acceptance_tests(
srcs = ["UploadArchiveIT.java"],
group = "upload-archive",
labels = ["git-upload-archive"],
)

211
javatests/com/google/gerrit/integration/git/UploadArchiveIT.java Normal file
View File

@@ -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.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.gerrit.reviewdb.client.Project;
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 = new 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();
}
}

5
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -1881,6 +1881,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
new 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);

2
polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.html
View File

@@ -57,7 +57,7 @@ limitations under the License.
<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

16
polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.js
View File

@@ -25,6 +25,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,
},
@@ -49,6 +60,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);
@@ -57,6 +72,7 @@
},
_handleSaveDiffPreferences() {
this.diffPrefs = this._editableDiffPrefs;
this.$.diffPreferences.save().then(() => {
this.fire('reload-diff-preference', null, {bubbles: false});

63
polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog_test.html Normal file
View File

@@ -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>

27
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
View File

@@ -636,21 +636,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);

34
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
View File

@@ -698,6 +698,22 @@ limitations under the License.
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');
@@ -952,6 +968,24 @@ limitations under the License.
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', () => {

4
tools/nongoogle.bzl
View File

@@ -94,8 +94,8 @@ def declare_nongoogle_deps():
# 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(