Merge changes Ib708eabf,I0bbb1018,Ideaf4da3,I8294c7b2,I27f5db09, ...

* changes: Test push with unknown push option CorsIT: Don't use the deprecated Executor#cookieStore(CookieStore) method FileDiffCacheImpl: Do not use deprecated Streams#stream(Collection<T>) method Add serialVersionUID to classes that miss it AbstractDaemonTest: Remove unnecessary cast from Integer to int Remove unused fields and variables LabelsJson: Remove unused private method DynamicOptionsIT: Remove unneeded throws declaration CherryPickChange: Remove unnecessary nesting with else clause ChangeETagComputation: Fix broken link in javadoc Fix Javadoc Invalid member type qualification warnings FileDiffCacheImpl: Fix raw type warning StarredChanges: Use common Input class instead of custom EmptyInput class
2021-02-07 12:37:10 +00:00
parent 3e656952f1 4cd6ad5714
commit 41623da45e
20 changed files with 40 additions and 61 deletions
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -812,8 +812,7 @@ public abstract class AbstractDaemonTest {
                .boxed()
                .collect(
                    Collectors.toMap(
-                        i -> fileNames.get((int) i - 1),
-                        i -> fileNames.get((int) i - 1) + "-" + i)));
+                        i -> fileNames.get(i - 1), i -> fileNames.get(i - 1) + "-" + i)));

    m.setParents(pushResults.stream().map(PushOneCommit.Result::getCommit).collect(toList()));
    PushOneCommit.Result result = m.to(ref);
--- a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
@@ -288,7 +288,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {

    try (AutoCloseable ignored =
        installPlugin("my-plugin", PluginDefinedBulkExceptionModule.class)) {
-      PluginDefinedInfo errorInfo = new PluginDefinedInfo();
      List<PluginDefinedInfo> outputInfos = getter.call(id).get(id);
      assertThat(outputInfos).hasSize(1);
      assertThat(outputInfos.get(0).name).isEqualTo("my-plugin");
--- a/java/com/google/gerrit/server/CurrentUser.java
+++ b/java/com/google/gerrit/server/CurrentUser.java
@@ -138,8 +138,9 @@ public abstract class CurrentUser {
  }

  /**
-   * Returns all {@link ExternalId.Key}s associated with this user. For {@link AnonymousUser} and
-   * other users that don't represent a person user or service account, this set will be empty.
+   * Returns all {@link com.google.gerrit.server.account.externalids.ExternalId.Key}s associated
+   * with this user. For {@link AnonymousUser} and other users that don't represent a person user or
+   * service account, this set will be empty.
   */
  public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
    return ImmutableSet.of();
--- a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -365,8 +365,7 @@ public class AccountApiImpl implements AccountApi {
  @Override
  public void starChange(String changeId) throws RestApiException {
    try {
-      starredChangesCreate.apply(
-          account, IdString.fromUrl(changeId), new StarredChanges.EmptyInput());
+      starredChangesCreate.apply(account, IdString.fromUrl(changeId), new Input());
    } catch (Exception e) {
      throw asRestApiException("Cannot star change", e);
    }
@@ -378,7 +377,7 @@ public class AccountApiImpl implements AccountApi {
      ChangeResource rsrc = changes.parse(TopLevelResource.INSTANCE, IdString.fromUrl(changeId));
      AccountResource.StarredChange starredChange =
          new AccountResource.StarredChange(account.getUser(), rsrc);
-      starredChangesDelete.apply(starredChange, new StarredChanges.EmptyInput());
+      starredChangesDelete.apply(starredChange, new Input());
    } catch (Exception e) {
      throw asRestApiException("Cannot unstar change", e);
    }
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -767,10 +767,7 @@ class ChangeApiImpl implements ChangeApi {
    private final CmdLineParser.Factory cmdLineParserFactory;

    @Inject
-    DynamicOptionParser(
-        CmdLineParser.Factory cmdLineParserFactory,
-        Injector injector,
-        DynamicMap<DynamicOptions.DynamicBean> dynamicBeans) {
+    DynamicOptionParser(CmdLineParser.Factory cmdLineParserFactory) {
      this.cmdLineParserFactory = cmdLineParserFactory;
    }

--- a/java/com/google/gerrit/server/change/ChangeETagComputation.java
+++ b/java/com/google/gerrit/server/change/ChangeETagComputation.java
@@ -26,7 +26,7 @@ import com.google.gerrit.extensions.annotations.ExtensionPoint;
 * <ul>
 *   <li>providing plugin defined attributes to {@link
 *       com.google.gerrit.extensions.common.ChangeInfo#plugins} (see {@link
- *       ChangeAttributeFactory})
+ *       ChangePluginDefinedInfoFactory})
 *   <li>implementing a {@link com.google.gerrit.server.rules.SubmitRule} which affects the
 *       computation of {@link com.google.gerrit.extensions.common.ChangeInfo#submittable}
 * </ul>
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -20,14 +20,12 @@ import static java.util.stream.Collectors.toList;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.MultimapBuilder;
 import com.google.common.collect.SetMultimap;
-import com.google.common.collect.Sets;
 import com.google.common.collect.Table;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.primitives.Ints;
@@ -38,7 +36,6 @@ import com.google.gerrit.entities.LabelTypes;
 import com.google.gerrit.entities.LabelValue;
 import com.google.gerrit.entities.PatchSetApproval;
 import com.google.gerrit.entities.SubmitRecord;
-import com.google.gerrit.extensions.client.ListChangesOption;
 import com.google.gerrit.extensions.common.ApprovalInfo;
 import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.common.VotingRangeInfo;
@@ -156,11 +153,6 @@ public class LabelsJson {
    return permitted.asMap();
  }

-  private static boolean containsAnyOf(
-      ImmutableSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
-    return !Sets.intersection(toFind, set).isEmpty();
-  }
-
  private static boolean isOnlyZero(Collection<String> values) {
    return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
  }
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -138,8 +138,9 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
    }

    /**
-     * Create change notes based on a {@link Change.Id}. This requires using the Change index and
-     * should only be used when {@link Project.NameKey} and the numeric change ID are not available.
+     * Create change notes based on a {@link com.google.gerrit.entities.Change.Id}. This requires
+     * using the Change index and should only be used when {@link
+     * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
     */
    public ChangeNotes createCheckedUsingIndexLookup(Change.Id changeId) {
      InternalChangeQuery query = queryProvider.get().noFields();
@@ -155,9 +156,9 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
    }

    /**
-     * Create change notes based on a list of {@link Change.Id}s. This requires using the Change
-     * index and should only be used when {@link Project.NameKey} and the numeric change ID are not
-     * available.
+     * Create change notes based on a list of {@link com.google.gerrit.entities.Change.Id}s. This
+     * requires using the Change index and should only be used when {@link
+     * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
     */
    public List<ChangeNotes> createUsingIndexLookup(Collection<Change.Id> changeIds) {
      List<ChangeNotes> notes = new ArrayList<>();
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -1027,7 +1027,7 @@ class ChangeNotesParser {
   * @return {@link Optional} value of the parsed footer or {@code null} if the footer is missing in
   *     this commit.
   * @throws ConfigInvalidException if the footer value could not be parsed as a valid {@link
-   *     PatchSet.Id}.
+   *     com.google.gerrit.entities.PatchSet.Id}.
   */
  @Nullable
  private Optional<PatchSet.Id> parseCherryPickOf(ChangeNotesCommit commit)
--- a/java/com/google/gerrit/server/patch/DiffNotAvailableException.java
+++ b/java/com/google/gerrit/server/patch/DiffNotAvailableException.java
@@ -22,6 +22,8 @@ import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCache;
 * if the implementations failed to retrieve the modified files between the 2 commits.
 */
 public class DiffNotAvailableException extends Exception {
+  private static final long serialVersionUID = 1L;
+
  public DiffNotAvailableException(Throwable cause) {
    super(cause);
  }
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -418,7 +418,7 @@ public class FileDiffCacheImpl implements FileDiffCache {

    private static ImmutableList<TaggedEdit> asTaggedEdits(
        List<Edit> normalEdits, List<Edit> rebaseEdits) {
-      Set<Edit> rebaseEditsSet = new HashSet(rebaseEdits);
+      Set<Edit> rebaseEditsSet = new HashSet<>(rebaseEdits);
      ImmutableList.Builder<TaggedEdit> result =
          ImmutableList.builderWithExpectedSize(normalEdits.size());
      for (Edit e : normalEdits) {
@@ -497,7 +497,7 @@ public class FileDiffCacheImpl implements FileDiffCache {
      String filePath = editsPerFilePath.keys().iterator().next();
      Collection<ContextAwareEdit> edits = editsPerFilePath.get(filePath);
      return FileEdits.create(
-          Streams.stream(edits)
+          edits.stream()
              .map(ContextAwareEdit::toEdit)
              .filter(Optional::isPresent)
              .map(Optional::get)
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
@@ -32,6 +32,7 @@ import java.util.stream.Collectors;
 /** File diff for a single file path. Produced as output of the {@link FileDiffCache}. */
@AutoValue
 public abstract class FileDiffOutput implements Serializable {
+  private static final long serialVersionUID = 1L;

  /**
   * The file path at the old commit. Returns an empty Optional if {@link #changeType()} is equal to
--- a/java/com/google/gerrit/server/restapi/account/StarredChanges.java
+++ b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
@@ -17,6 +17,7 @@ package com.google.gerrit.server.restapi.account;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.exceptions.DuplicateKeyException;
 import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.common.Input;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -96,8 +97,7 @@ public class StarredChanges

  @Singleton
  public static class Create
-      implements RestCollectionCreateView<
-          AccountResource, AccountResource.StarredChange, EmptyInput> {
+      implements RestCollectionCreateView<AccountResource, AccountResource.StarredChange, Input> {
    private final Provider<CurrentUser> self;
    private final ChangesCollection changes;
    private final StarredChangesUtil starredChangesUtil;
@@ -113,7 +113,7 @@ public class StarredChanges
    }

    @Override
-    public Response<?> apply(AccountResource rsrc, IdString id, EmptyInput in)
+    public Response<?> apply(AccountResource rsrc, IdString id, Input in)
        throws RestApiException, IOException {
      if (!self.get().hasSameAccountId(rsrc.getUser())) {
        throw new AuthException("not allowed to add starred change");
@@ -148,7 +148,7 @@ public class StarredChanges
  }

  @Singleton
-  public static class Put implements RestModifyView<AccountResource.StarredChange, EmptyInput> {
+  public static class Put implements RestModifyView<AccountResource.StarredChange, Input> {
    private final Provider<CurrentUser> self;

    @Inject
@@ -157,8 +157,7 @@ public class StarredChanges
    }

    @Override
-    public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
-        throws AuthException {
+    public Response<?> apply(AccountResource.StarredChange rsrc, Input in) throws AuthException {
      if (!self.get().hasSameAccountId(rsrc.getUser())) {
        throw new AuthException("not allowed update starred changes");
      }
@@ -167,7 +166,7 @@ public class StarredChanges
  }

  @Singleton
-  public static class Delete implements RestModifyView<AccountResource.StarredChange, EmptyInput> {
+  public static class Delete implements RestModifyView<AccountResource.StarredChange, Input> {
    private final Provider<CurrentUser> self;
    private final StarredChangesUtil starredChangesUtil;

@@ -178,7 +177,7 @@ public class StarredChanges
    }

    @Override
-    public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
+    public Response<?> apply(AccountResource.StarredChange rsrc, Input in)
        throws AuthException, IOException, IllegalLabelException {
      if (!self.get().hasSameAccountId(rsrc.getUser())) {
        throw new AuthException("not allowed remove starred change");
@@ -192,6 +191,4 @@ public class StarredChanges
      return Response.none();
    }
  }
-
-  public static class EmptyInput {}
 }
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -28,7 +28,6 @@ import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.MergeConflictException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -44,7 +43,6 @@ import com.google.gerrit.server.change.NotifyResolver;
 import com.google.gerrit.server.change.PatchSetInserter;
 import com.google.gerrit.server.change.ResetCherryPickOp;
 import com.google.gerrit.server.change.SetCherryPickOp;
-import com.google.gerrit.server.config.UrlFormatter;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -114,7 +112,6 @@ public class CherryPickChange {
  private final ApprovalsUtil approvalsUtil;
  private final NotifyResolver notifyResolver;
  private final BatchUpdate.Factory batchUpdateFactory;
-  private final DynamicItem<UrlFormatter> urlFormatter;

  @Inject
  CherryPickChange(
@@ -131,8 +128,7 @@ public class CherryPickChange {
      ProjectCache projectCache,
      ApprovalsUtil approvalsUtil,
      NotifyResolver notifyResolver,
-      BatchUpdate.Factory batchUpdateFactory,
-      DynamicItem<UrlFormatter> urlFormatter) {
+      BatchUpdate.Factory batchUpdateFactory) {
    this.seq = seq;
    this.queryProvider = queryProvider;
    this.gitManager = gitManager;
@@ -147,7 +143,6 @@ public class CherryPickChange {
    this.approvalsUtil = approvalsUtil;
    this.notifyResolver = notifyResolver;
    this.batchUpdateFactory = batchUpdateFactory;
-    this.urlFormatter = urlFormatter;
  }

  /**
@@ -585,9 +580,8 @@ public class CherryPickChange {
      String commitMessage, @Nullable ObjectId changeIdForNewChange) {
    if (changeIdForNewChange != null) {
      return CommitMessageUtil.getChangeIdFromObjectId(changeIdForNewChange);
-    } else {
-      return CommitMessageUtil.getChangeIdFromCommitMessageFooter(commitMessage).orElse(null);
    }
+    return CommitMessageUtil.getChangeIdFromCommitMessageFooter(commitMessage).orElse(null);
  }

  /**
--- a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
@@ -23,7 +23,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ChildProjects;
 import com.google.gerrit.server.project.ProjectResource;
@@ -40,16 +39,11 @@ public class ListChildProjects implements RestReadView<ProjectResource> {
  @Option(name = "--limit", usage = "maximum number of parents projects to list")
  private int limit;

-  private final PermissionBackend permissionBackend;
  private final ChildProjects childProjects;
  private final Provider<QueryProjects> queryProvider;

  @Inject
-  ListChildProjects(
-      PermissionBackend permissionBackend,
-      ChildProjects childProjects,
-      Provider<QueryProjects> queryProvider) {
-    this.permissionBackend = permissionBackend;
+  ListChildProjects(ChildProjects childProjects, Provider<QueryProjects> queryProvider) {
    this.childProjects = childProjects;
    this.queryProvider = queryProvider;
  }
@@ -82,7 +76,6 @@ public class ListChildProjects implements RestReadView<ProjectResource> {
  }

  private List<ProjectInfo> directChildProjects(Project.NameKey parent) throws RestApiException {
-    PermissionBackend.WithUser currentUser = permissionBackend.currentUser();
    return queryProvider.get().withQuery("parent:" + parent.get()).withLimit(limit).apply().stream()
        .collect(toList());
  }
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -767,7 +767,6 @@ public class RevisionIT extends AbstractDaemonTest {
        pushFactory
            .create(admin.newIdent(), testRepo, SUBJECT, FILE_NAME, "a")
            .to("refs/for/master");
-    String t1 = project.get() + "~master~" + r1.getChangeId();

    BranchInput bin = new BranchInput();
    bin.revision = r1.getCommit().getParent(0).name();
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -2753,6 +2753,14 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
    assertThat(r.getChange().attentionSet()).isEmpty();
  }

+  @Test
+  public void pushForMasterWithUnknownOption() throws Exception {
+    PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+    push.setPushOptions(ImmutableList.of("unknown=foo"));
+    PushOneCommit.Result r = push.to("refs/for/master");
+    r.assertErrorStatus("\"--unknown\" is not a valid option");
+  }
+
  private DraftInput newDraft(String path, int line, String message) {
    DraftInput d = new DraftInput();
    d.path = path;
--- a/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
@@ -204,7 +204,7 @@ public class CorsIT extends AbstractDaemonTest {
  public void crossDomainPutTopic() throws Exception {
    Result change = createChange();
    BasicCookieStore cookies = new BasicCookieStore();
-    Executor http = Executor.newInstance().cookieStore(cookies);
+    Executor http = Executor.newInstance().use(cookies);

    Request req = Request.Get(canonicalWebUrl.get() + "/login/?account_id=" + admin.id().get());
    http.execute(req);
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -26,7 +26,6 @@ import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -388,7 +387,6 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
    // Create a change
    PushOneCommit change = pushFactory.create(user.newIdent(), testRepo, "fix", "a.txt", "foo");
    PushOneCommit.Result changeResult = change.to("refs/for/master");
-    PatchSet.Id patchSetId = changeResult.getPatchSetId();

    // Create a successor change.
    PushOneCommit change2 =
--- a/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/DynamicOptionsIT.java
@@ -23,7 +23,6 @@ import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gson.reflect.TypeToken;
 import com.google.inject.Module;
-import java.io.IOException;
 import java.util.List;
 import org.junit.Test;

@@ -46,7 +45,7 @@ public class DynamicOptionsIT extends AbstractDynamicOptionsTest {
    }
  }

-  protected List<String> getSamplesList(String sshOutput) throws IOException {
+  protected List<String> getSamplesList(String sshOutput) {
    return GSON.fromJson(sshOutput, new TypeToken<List<String>>() {}.getType());
  }
 }