Merge changes I4bfe80d4,Ifb317d8a

* changes:
  Add a proof-of-concept test for error messages in ReceiveCommits
  Add a Truth Subject for asserting about JGit PushResults

java/com/google/gerrit/acceptance/BUILD

@@ -13,6 +13,7 @@ java_library(
         "//java/com/google/gerrit/extensions:api",
         "//java/com/google/gerrit/extensions/common/testing:common-test-util",
         "//java/com/google/gerrit/extensions/restapi/testing:restapi-test-util",
+        "//java/com/google/gerrit/git/testing",
         "//java/com/google/gerrit/gpg/testing:gpg-test-util",
         "//java/com/google/gerrit/httpd",
         "//java/com/google/gerrit/index",

java/com/google/gerrit/git/testing/BUILD

@@ -0,0 +1,14 @@
+package(default_testonly = 1)
+
+java_library(
+    name = "testing",
+    srcs = glob(["*.java"]),
+    visibility = ["//visibility:public"],
+    deps = [
+        "//java/com/google/gerrit/common:annotations",
+        "//lib:guava",
+        "//lib:truth",
+        "//lib:truth-java8-extension",
+        "//lib/jgit/org.eclipse.jgit:jgit",
+    ],
+)

java/com/google/gerrit/git/testing/PushResultSubject.java

@@ -0,0 +1,172 @@
+// Copyright (C) 2018 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.git.testing;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.truth.Truth.assertAbout;
+import static java.util.stream.Collectors.joining;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.Subject;
+import com.google.common.truth.Truth;
+import com.google.common.truth.Truth8;
+import com.google.gerrit.common.Nullable;
+import java.util.Arrays;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+
+public class PushResultSubject extends Subject<PushResultSubject, PushResult> {
+  public static PushResultSubject assertThat(PushResult actual) {
+    return assertAbout(PushResultSubject::new).that(actual);
+  }
+
+  private PushResultSubject(FailureMetadata metadata, PushResult actual) {
+    super(metadata, actual);
+  }
+
+  public void hasNoMessages() {
+    Truth.assertWithMessage("expected no messages")
+        .that(Strings.nullToEmpty(trimMessages()))
+        .isEqualTo("");
+  }
+
+  public void hasMessages(String... expectedLines) {
+    checkArgument(expectedLines.length > 0, "use hasNoMessages()");
+    isNotNull();
+    Truth.assertThat(trimMessages()).isEqualTo(Arrays.stream(expectedLines).collect(joining("\n")));
+  }
+
+  private String trimMessages() {
+    return trimMessages(actual().getMessages());
+  }
+
+  @VisibleForTesting
+  @Nullable
+  static String trimMessages(@Nullable String msg) {
+    if (msg == null) {
+      return null;
+    }
+    int idx = msg.indexOf("Processing changes:");
+    if (idx >= 0) {
+      msg = msg.substring(0, idx);
+    }
+    return msg.trim();
+  }
+
+  public void hasProcessed(ImmutableMap<String, Integer> expected) {
+    ImmutableMap<String, Integer> actual;
+    String messages = actual().getMessages();
+    try {
+      actual = parseProcessed(messages);
+    } catch (RuntimeException e) {
+      Truth.assert_()
+          .fail(
+              "failed to parse \"Processing changes\" line from messages: %s\n%s",
+              messages, Throwables.getStackTraceAsString(e));
+      return;
+    }
+    Truth.assertThat(actual)
+        .named("processed commands")
+        .containsExactlyEntriesIn(expected)
+        .inOrder();
+  }
+
+  @VisibleForTesting
+  static ImmutableMap<String, Integer> parseProcessed(@Nullable String messages) {
+    if (messages == null) {
+      return ImmutableMap.of();
+    }
+    String toSplit = messages.trim();
+    String prefix = "Processing changes: ";
+    int idx = toSplit.lastIndexOf(prefix);
+    if (idx < 0) {
+      return ImmutableMap.of();
+    }
+    toSplit = toSplit.substring(idx + prefix.length());
+    if (toSplit.equals("done")) {
+      return ImmutableMap.of();
+    }
+    String done = ", done";
+    if (toSplit.endsWith(done)) {
+      toSplit = toSplit.substring(0, toSplit.length() - done.length());
+    }
+    return ImmutableMap.copyOf(
+        Maps.transformValues(
+            Splitter.on(',').trimResults().withKeyValueSeparator(':').split(toSplit),
+            // trimResults() doesn't trim values in the map.
+            v -> Integer.parseInt(v.trim())));
+  }
+
+  public RemoteRefUpdateSubject ref(String refName) {
+    return assertAbout(
+            (FailureMetadata m, RemoteRefUpdate a) -> new RemoteRefUpdateSubject(refName, m, a))
+        .that(actual().getRemoteUpdate(refName));
+  }
+
+  public RemoteRefUpdateSubject onlyRef(String refName) {
+    Truth8.assertThat(actual().getRemoteUpdates().stream().map(RemoteRefUpdate::getRemoteName))
+        .named("set of refs")
+        .containsExactly(refName);
+    return ref(refName);
+  }
+
+  public static class RemoteRefUpdateSubject
+      extends Subject<RemoteRefUpdateSubject, RemoteRefUpdate> {
+    private final String refName;
+
+    private RemoteRefUpdateSubject(
+        String refName, FailureMetadata metadata, RemoteRefUpdate actual) {
+      super(metadata, actual);
+      this.refName = refName;
+      named("ref update for %s", refName).isNotNull();
+    }
+
+    public void hasStatus(RemoteRefUpdate.Status status) {
+      RemoteRefUpdate u = actual();
+      Truth.assertThat(u.getStatus())
+          .named(
+              "status of ref update for %s%s",
+              refName, u.getMessage() != null ? ": " + u.getMessage() : "")
+          .isEqualTo(status);
+    }
+
+    public void hasNoMessage() {
+      Truth.assertThat(actual().getMessage())
+          .named("message of ref update for %s", refName)
+          .isNull();
+    }
+
+    public void hasMessage(String expected) {
+      Truth.assertThat(actual().getMessage())
+          .named("message of ref update for %s", refName)
+          .isEqualTo(expected);
+    }
+
+    public void isOk() {
+      hasStatus(RemoteRefUpdate.Status.OK);
+    }
+
+    public void isRejected(String expectedMessage) {
+      hasStatus(RemoteRefUpdate.Status.REJECTED_OTHER_REASON);
+      hasMessage(expectedMessage);
+    }
+  }
+}

javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java

@@ -0,0 +1,82 @@
+// Copyright (C) 2018 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.git;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.gerrit.git.testing.PushResultSubject.assertThat;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.ProjectConfig;
+import java.util.Arrays;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RefSpec;
+import org.junit.Before;
+import org.junit.Test;
+
+public class PushPermissionsIT extends AbstractDaemonTest {
+  @Before
+  public void setUp() throws Exception {
+    // Remove all push-related permissions, so they can be added back individually by test methods.
+    try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjects)) {
+      ProjectConfig cfg = ProjectConfig.read(md);
+      removeAllBranchPermissions(cfg, Permission.PUSH);
+      removeAllBranchPermissions(cfg, Permission.CREATE);
+      removeAllBranchPermissions(cfg, Permission.DELETE);
+      removeAllBranchPermissions(cfg, Permission.PUSH_MERGE);
+      saveProjectConfig(allProjects, cfg);
+    }
+  }
+
+  @Test
+  public void noDirectPushPermissions() throws Exception {
+    testRepo.branch("HEAD").commit().create();
+    PushResult r = push("HEAD:refs/heads/master");
+    assertThat(r)
+        .onlyRef("refs/heads/master")
+        .isRejected("prohibited by Gerrit: ref update access denied");
+    assertThat(r)
+        .hasMessages(
+            "Branch refs/heads/master:",
+            "You are not allowed to perform this operation.",
+            "To push into this reference you need 'Push' rights.",
+            "User: admin",
+            "Please read the documentation and contact an administrator",
+            "if you feel the configuration is incorrect");
+    assertThat(r).hasProcessed(ImmutableMap.of("refs", 1));
+  }
+
+  private static void removeAllBranchPermissions(ProjectConfig cfg, String permission) {
+    cfg.getAccessSections()
+        .stream()
+        .filter(s -> s.getName().startsWith("refs/heads/") || s.getName().equals("refs/*"))
+        .forEach(s -> s.removePermission(permission));
+  }
+
+  private PushResult push(String... refSpecs) throws Exception {
+    Iterable<PushResult> results =
+        testRepo
+            .git()
+            .push()
+            .setRemote("origin")
+            .setRefSpecs(Arrays.stream(refSpecs).map(RefSpec::new).collect(toList()))
+            .call();
+    assertWithMessage("expected 1 PushResult").that(results).hasSize(1);
+    return results.iterator().next();
+  }
+}

javatests/com/google/gerrit/git/testing/BUILD

@@ -0,0 +1,10 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+    name = "testing_tests",
+    srcs = glob(["*.java"]),
+    deps = [
+        "//java/com/google/gerrit/git/testing",
+        "//lib:truth",
+    ],
+)

javatests/com/google/gerrit/git/testing/PushResultSubjectTest.java

@@ -0,0 +1,53 @@
+// Copyright (C) 2018 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.git.testing;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.git.testing.PushResultSubject.parseProcessed;
+import static com.google.gerrit.git.testing.PushResultSubject.trimMessages;
+
+import org.junit.Test;
+
+public class PushResultSubjectTest {
+  @Test
+  public void testTrimMessages() {
+    assertThat(trimMessages(null)).isNull();
+    assertThat(trimMessages("")).isEqualTo("");
+    assertThat(trimMessages(" \n ")).isEqualTo("");
+    assertThat(trimMessages("\n Foo\nBar\n\nProcessing changes: 1, 2, 3 done   \n"))
+        .isEqualTo("Foo\nBar");
+  }
+
+  @Test
+  public void testParseProcessed() {
+    assertThat(parseProcessed(null)).isEmpty();
+    assertThat(parseProcessed("some other output")).isEmpty();
+    assertThat(parseProcessed("Processing changes: done\n")).isEmpty();
+    assertThat(parseProcessed("Processing changes: refs: 1, done \n")).containsExactly("refs", 1);
+    assertThat(parseProcessed("Processing changes: new: 1, updated: 2, refs: 3, done \n"))
+        .containsExactly("new", 1, "updated", 2, "refs", 3)
+        .inOrder();
+    assertThat(
+            parseProcessed(
+                "Some\nlonger\nmessage\nProcessing changes: new: 1\r"
+                    + "Processing changes: new: 1, updated: 1\r"
+                    + "Processing changes: new: 1, updated: 2, done"))
+        .containsExactly("new", 1, "updated", 2)
+        .inOrder();
+
+    // Atypical, but could potentially happen if there is an uncaught exception.
+    assertThat(parseProcessed("Processing changes: refs: 1")).containsExactly("refs", 1);
+  }
+}