Remove RemoveFromAttentionSetInput

RemoveFromAttentionSetInput was previously used by
RemoveFromAttentionSet. Now, postReview uses AttentionSetInput for both
adding and removing users, so it makes sense to use AttentionSetInput
for both use cases here, as well.

In this case, we will ensure that if a user is specified, it must match
the user in the URL.

The URL looks like this:
POST /changes/{change-id}/attention/{account-id}/delete

Thus, the user that should be removed is already specified there: if
the caller specifies the user again, the users specified must match
to the same user (both in the input object and in the URL).

Change-Id: I68ee52d4e71fd733e0035bee14d3d38ebcdc9752

java/com/google/gerrit/entities/AttentionSetUpdate.java

@@ -24,9 +24,7 @@ import java.time.Instant;
  * in reverse chronological order. Since each update contains all required information and
  * invalidates all previous state, only the most recent record is relevant for each user.
  *
- * <p>See {@link AttentionSetInput} and {@link
- * com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput} for the representation in
- * the API.
+ * <p>See {@link AttentionSetInput} for the representation in the API.
  */
 @AutoValue
 public abstract class AttentionSetUpdate {

java/com/google/gerrit/extensions/api/changes/AttentionSetApi.java

@@ -20,7 +20,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 /** API for managing the attention set of a change. */
 public interface AttentionSetApi {
 
-  void remove(RemoveFromAttentionSetInput input) throws RestApiException;
+  void remove(AttentionSetInput input) throws RestApiException;
 
   /**
    * A default implementation which allows source compatibility when adding new methods to the
@@ -28,7 +28,7 @@ public interface AttentionSetApi {
    */
   class NotImplemented implements AttentionSetApi {
     @Override
-    public void remove(RemoveFromAttentionSetInput input) throws RestApiException {
+    public void remove(AttentionSetInput input) throws RestApiException {
       throw new NotImplementedException();
     }
   }

java/com/google/gerrit/extensions/api/changes/AttentionSetInput.java

@@ -14,20 +14,25 @@
 
 package com.google.gerrit.extensions.api.changes;
 
+import com.google.gerrit.extensions.restapi.DefaultInput;
+
 /**
  * Input at API level to add a user to the attention set.
  *
- * @see RemoveFromAttentionSetInput
  * @see com.google.gerrit.extensions.common.AttentionSetEntry
  */
 public class AttentionSetInput {
   public String user;
-  public String reason;
+  @DefaultInput public String reason;
 
   public AttentionSetInput(String user, String reason) {
     this.user = user;
     this.reason = reason;
   }
 
+  public AttentionSetInput(String reason) {
+    this.reason = reason;
+  }
+
   public AttentionSetInput() {}
 }

java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java

@@ -1,33 +0,0 @@
-// 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.extensions.api.changes;
-
-import com.google.gerrit.extensions.restapi.DefaultInput;
-
-/**
- * Input at API level to remove a user from the attention set.
- *
- * @see AttentionSetInput
- * @see com.google.gerrit.extensions.common.AttentionSetEntry
- */
-public class RemoveFromAttentionSetInput {
-  @DefaultInput public String reason;
-
-  public RemoveFromAttentionSetInput(String reason) {
-    this.reason = reason;
-  }
-
-  public RemoveFromAttentionSetInput() {}
-}

java/com/google/gerrit/server/api/changes/AttentionSetApiImpl.java

@@ -17,7 +17,7 @@ package com.google.gerrit.server.api.changes;
 import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
 
 import com.google.gerrit.extensions.api.changes.AttentionSetApi;
-import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.change.AttentionSetEntryResource;
 import com.google.gerrit.server.restapi.change.RemoveFromAttentionSet;
@@ -41,7 +41,7 @@ public class AttentionSetApiImpl implements AttentionSetApi {
   }
 
   @Override
-  public void remove(RemoveFromAttentionSetInput input) throws RestApiException {
+  public void remove(AttentionSetInput input) throws RestApiException {
     try {
       removeFromAttentionSet.apply(attentionSetEntryResource, input);
     } catch (Exception e) {

java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java

@@ -15,11 +15,13 @@
 package com.google.gerrit.server.restapi.change;
 
 import com.google.common.base.Strings;
-import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.change.AttentionSetEntryResource;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.change.RemoveFromAttentionSetOp;
@@ -33,20 +35,24 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 
 /** Removes a single user from the attention set. */
 public class RemoveFromAttentionSet
-    implements RestModifyView<AttentionSetEntryResource, RemoveFromAttentionSetInput> {
+    implements RestModifyView<AttentionSetEntryResource, AttentionSetInput> {
   private final BatchUpdate.Factory updateFactory;
   private final RemoveFromAttentionSetOp.Factory opFactory;
+  private final AccountResolver accountResolver;
 
   @Inject
   RemoveFromAttentionSet(
-      BatchUpdate.Factory updateFactory, RemoveFromAttentionSetOp.Factory opFactory) {
+      BatchUpdate.Factory updateFactory,
+      RemoveFromAttentionSetOp.Factory opFactory,
+      AccountResolver accountResolver) {
     this.updateFactory = updateFactory;
     this.opFactory = opFactory;
+    this.accountResolver = accountResolver;
   }
 
   @Override
   public Response<Object> apply(
-      AttentionSetEntryResource attentionResource, RemoveFromAttentionSetInput input)
+      AttentionSetEntryResource attentionResource, AttentionSetInput input)
       throws RestApiException, PermissionBackendException, IOException, ConfigInvalidException,
           UpdateException {
     if (input == null) {
@@ -56,6 +62,20 @@ public class RemoveFromAttentionSet
     if (input.reason.isEmpty()) {
       throw new BadRequestException("missing field: reason");
     }
+    input.user = Strings.nullToEmpty(input.user).trim();
+    if (!input.user.isEmpty()) {
+      Account.Id attentionUserId = null;
+      try {
+        attentionUserId = accountResolver.resolve(input.user).asUnique().account().id();
+      } catch (AccountResolver.UnresolvableAccountException ex) {
+        throw new BadRequestException(
+            "The user specified in the input body couldn't be found.", ex);
+      }
+      if (attentionUserId.get() != attentionResource.getAccountId().get()) {
+        throw new BadRequestException(
+            "The field \"user\" must be empty, or must match the user specified in the URL.");
+      }
+    }
     ChangeResource changeResource = attentionResource.getChangeResource();
     try (BatchUpdate bu =
         updateFactory.create(

javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java

@@ -32,7 +32,6 @@ import com.google.gerrit.entities.AttentionSetUpdate;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AttentionSetInput;
 import com.google.gerrit.extensions.api.changes.HashtagsInput;
-import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -134,7 +133,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     PushOneCommit.Result r = createChange();
     change(r).addToAttentionSet(new AttentionSetInput(user.email(), "added"));
     fakeClock.advance(Duration.ofSeconds(42));
-    change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
+    change(r).attention(user.id().toString()).remove(new AttentionSetInput("removed"));
     AttentionSetUpdate expectedAttentionSetUpdate =
         AttentionSetUpdate.createFromRead(
             fakeClock.now(), user.id(), AttentionSetUpdate.Operation.REMOVE, "removed");
@@ -142,10 +141,33 @@ public class AttentionSetIT extends AbstractDaemonTest {
 
     // Second removal is ignored.
     fakeClock.advance(Duration.ofSeconds(42));
+    change(r).attention(user.id().toString()).remove(new AttentionSetInput("removed again"));
+    assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
+  }
+
+  @Test
+  public void removeUserWithInvalidUserInput() throws Exception {
+    PushOneCommit.Result r = createChange();
+    BadRequestException exception =
+        assertThrows(
+            BadRequestException.class,
+            () ->
                 change(r)
                     .attention(user.id().toString())
-        .remove(new RemoveFromAttentionSetInput("removed again"));
-    assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
+                    .remove(new AttentionSetInput("invalid user", "reason")));
+    assertThat(exception.getMessage())
+        .isEqualTo("The user specified in the input body couldn't be found.");
+
+    exception =
+        assertThrows(
+            BadRequestException.class,
+            () ->
+                change(r)
+                    .attention(user.id().toString())
+                    .remove(new AttentionSetInput(admin.email(), "reason")));
+    assertThat(exception.getMessage())
+        .isEqualTo(
+            "The field \"user\" must be empty, or must match the user specified in the URL.");
   }
 
   @Test
@@ -155,7 +177,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     assertThat(Iterables.getLast(r.getChange().messages()).getMessage())
         .contains("Added to attention set");
 
-    change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("foo"));
+    change(r).attention(user.id().toString()).remove(new AttentionSetInput("foo"));
     assertThat(Iterables.getLast(r.getChange().messages()).getMessage())
         .contains("Removed from attention set");
   }
@@ -163,7 +185,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
   @Test
   public void removeUnrelatedUser() throws Exception {
     PushOneCommit.Result r = createChange();
-    change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("foo"));
+    change(r).attention(user.id().toString()).remove(new AttentionSetInput("foo"));
     assertThat(r.getChange().attentionSet()).isEmpty();
   }
 
@@ -315,8 +337,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     change(r).addReviewer(user.id().toString());
     change(r)
         .attention(user.id().toString())
-        .remove(
-            new RemoveFromAttentionSetInput("removed and not re-added when re-adding as reviewer"));
+        .remove(new AttentionSetInput("removed and not re-added when re-adding as reviewer"));
 
     change(r).addReviewer(user.id().toString());
 
@@ -408,7 +429,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
   public void reviewersAreNotAddedForNoReasonBecauseOfAnUpdate() throws Exception {
     PushOneCommit.Result r = createChange();
     change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
-    change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
+    change(r).attention(user.id().toString()).remove(new AttentionSetInput("removed"));
 
     HashtagsInput hashtagsInput = new HashtagsInput();
     hashtagsInput.add = ImmutableSet.of("tag");
@@ -571,7 +592,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     ReviewInput reviewInput =
         ReviewInput.create().setWorkInProgress(true).addUserToAttentionSet(user.email(), "reason");
 
-    change(r).attention(user.email()).remove(new RemoveFromAttentionSetInput("removal"));
+    change(r).attention(user.email()).remove(new AttentionSetInput("removal"));
     change(r).current().review(reviewInput);
 
     // Attention set updates that relate to the admin (the person who replied) are filtered out.
@@ -715,7 +736,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     TestAccount user2 = accountCreator.user2();
     requestScopeOperations.setApiUser(user2.id());
 
-    change(r).attention(user.email()).remove(new RemoveFromAttentionSetInput("reason"));
+    change(r).attention(user.email()).remove(new AttentionSetInput("reason"));
     ReviewInput reviewInput = new ReviewInput();
     change(r).current().review(reviewInput);
 
@@ -743,7 +764,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     change(r).addReviewer(user.email());
     change(r)
         .attention(user.email())
-        .remove(new RemoveFromAttentionSetInput("Reviewer is not in attention-set"));
+        .remove(new AttentionSetInput("Reviewer is not in attention-set"));
 
     TestAccount cc = accountCreator.admin2();
     AddReviewerInput input = new AddReviewerInput();
@@ -809,7 +830,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
     change(r).addReviewer(input);
 
     requestScopeOperations.setApiUser(user.id());
-    change(r).attention(reviewer.email()).remove(new RemoveFromAttentionSetInput("reason"));
+    change(r).attention(reviewer.email()).remove(new AttentionSetInput("reason"));
 
     ReviewInput reviewInput = new ReviewInput();
     change(r).current().review(reviewInput);