opendev/gerrit
Code Issues Proposed changes

Rename AddToAttentionSetInput to AttentionSetInput

Browse Source 
This is because in the follow-up change, we will use AttentionSetInput
for both adding and removing users from the attention set. However,
RemoveFromAttentionSet doesn't work for this case since we need
the user to be a part of the input.

Also, in a follow-up change, we can consider removing
RemoveFromAttentionSetInput and only use AttentionSetInput.

Change-Id: I28ba68a24242010f980f8ac833898f433af810e6
This commit is contained in:
Gal Paikin
2020-06-14 13:16:10 +03:00
parent 8bf2a18575
commit d274f391ce
8 changed files with 28 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
java/com/google/gerrit/entities/AttentionSetUpdate.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.entities;
import com.google.auto.value.AutoValue; import com.google.auto.value.AutoValue;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import java.time.Instant; import java.time.Instant;
/** /**
@@ -23,7 +24,7 @@ import java.time.Instant;
* in reverse chronological order. Since each update contains all required information and * 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. * invalidates all previous state, only the most recent record is relevant for each user.
* *
* <p>See {@link com.google.gerrit.extensions.api.changes.AddToAttentionSetInput} and {@link * <p>See {@link AttentionSetInput} and {@link
* com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput} for the representation in * com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput} for the representation in
* the API. * the API.
*/ */

6
java/com/google/gerrit/extensions/api/changes/AddToAttentionSetInput.java → java/com/google/gerrit/extensions/api/changes/AttentionSetInput.java
View File

@@ -20,14 +20,14 @@ package com.google.gerrit.extensions.api.changes;
* @see RemoveFromAttentionSetInput * @see RemoveFromAttentionSetInput
* @see com.google.gerrit.extensions.common.AttentionSetEntry * @see com.google.gerrit.extensions.common.AttentionSetEntry
*/ */
public class AddToAttentionSetInput { public class AttentionSetInput {
public String user; public String user;
public String reason; public String reason;
public AddToAttentionSetInput(String user, String reason) { public AttentionSetInput(String user, String reason) {
this.user = user; this.user = user;
this.reason = reason; this.reason = reason;
} }
public AddToAttentionSetInput() {} public AttentionSetInput() {}
} }

4
java/com/google/gerrit/extensions/api/changes/ChangeApi.java
View File

@@ -302,7 +302,7 @@ public interface ChangeApi {
AttentionSetApi attention(String id) throws RestApiException; AttentionSetApi attention(String id) throws RestApiException;
/** Adds a user to the attention set. */ /** Adds a user to the attention set. */
AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException; AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException;
/** Set the assignee of a change. */ /** Set the assignee of a change. */
AccountInfo setAssignee(AssigneeInput input) throws RestApiException; AccountInfo setAssignee(AssigneeInput input) throws RestApiException;
@@ -578,7 +578,7 @@ public interface ChangeApi {
} }
@Override @Override
public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException { public AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException {
throw new NotImplementedException(); throw new NotImplementedException();
} }

2
java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java
View File

@@ -19,7 +19,7 @@ import com.google.gerrit.extensions.restapi.DefaultInput;
/** /**
* Input at API level to remove a user from the attention set. * Input at API level to remove a user from the attention set.
* *
* @see AddToAttentionSetInput * @see AttentionSetInput
* @see com.google.gerrit.extensions.common.AttentionSetEntry * @see com.google.gerrit.extensions.common.AttentionSetEntry
*/ */
public class RemoveFromAttentionSetInput { public class RemoveFromAttentionSetInput {

4
java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@@ -23,9 +23,9 @@ import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.AbandonInput; import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult; import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput; import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.AttentionSetApi; import com.google.gerrit.extensions.api.changes.AttentionSetApi;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi; import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.ChangeMessageApi; import com.google.gerrit.extensions.api.changes.ChangeMessageApi;
@@ -543,7 +543,7 @@ class ChangeApiImpl implements ChangeApi {
} }
@Override @Override
public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException { public AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException {
try { try {
return addToAttentionSet.apply(change, input).value(); return addToAttentionSet.apply(change, input).value();
} catch (Exception e) { } catch (Exception e) {

6
java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java
View File

@@ -16,7 +16,7 @@ package com.google.gerrit.server.restapi.change;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.entities.Account; import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput; import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -38,7 +38,7 @@ import com.google.inject.Singleton;
@Singleton @Singleton
public class AddToAttentionSet public class AddToAttentionSet
implements RestCollectionModifyView< implements RestCollectionModifyView<
ChangeResource, AttentionSetEntryResource, AddToAttentionSetInput> { ChangeResource, AttentionSetEntryResource, AttentionSetInput> {
private final BatchUpdate.Factory updateFactory; private final BatchUpdate.Factory updateFactory;
private final AccountResolver accountResolver; private final AccountResolver accountResolver;
private final AddToAttentionSetOp.Factory opFactory; private final AddToAttentionSetOp.Factory opFactory;
@@ -60,7 +60,7 @@ public class AddToAttentionSet
} }
@Override @Override
public Response<AccountInfo> apply(ChangeResource changeResource, AddToAttentionSetInput input) public Response<AccountInfo> apply(ChangeResource changeResource, AttentionSetInput input)
throws Exception { throws Exception {
input.user = Strings.nullToEmpty(input.user).trim(); input.user = Strings.nullToEmpty(input.user).trim();
if (input.user.isEmpty()) { if (input.user.isEmpty()) {

22
javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
View File

@@ -26,7 +26,7 @@ import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseClockStep; import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.entities.AttentionSetUpdate; import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput; import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput; import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -77,7 +77,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
public void addUser() throws Exception { public void addUser() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
int accountId = int accountId =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "first"))._accountId; change(r).addToAttentionSet(new AttentionSetInput(user.email(), "first"))._accountId;
assertThat(accountId).isEqualTo(user.id().get()); assertThat(accountId).isEqualTo(user.id().get());
AttentionSetUpdate expectedAttentionSetUpdate = AttentionSetUpdate expectedAttentionSetUpdate =
AttentionSetUpdate.createFromRead( AttentionSetUpdate.createFromRead(
@@ -86,7 +86,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
// Second add is ignored. // Second add is ignored.
accountId = accountId =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "second"))._accountId; change(r).addToAttentionSet(new AttentionSetInput(user.email(), "second"))._accountId;
assertThat(accountId).isEqualTo(user.id().get()); assertThat(accountId).isEqualTo(user.id().get());
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate); assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
} }
@@ -96,13 +96,13 @@ public class AttentionSetIT extends AbstractDaemonTest {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
Instant timestamp1 = fakeClock.now(); Instant timestamp1 = fakeClock.now();
int accountId1 = int accountId1 =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user"))._accountId; change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"))._accountId;
assertThat(accountId1).isEqualTo(user.id().get()); assertThat(accountId1).isEqualTo(user.id().get());
fakeClock.advance(Duration.ofSeconds(42)); fakeClock.advance(Duration.ofSeconds(42));
Instant timestamp2 = fakeClock.now(); Instant timestamp2 = fakeClock.now();
int accountId2 = int accountId2 =
change(r) change(r)
.addToAttentionSet(new AddToAttentionSetInput(admin.id().toString(), "admin")) .addToAttentionSet(new AttentionSetInput(admin.id().toString(), "admin"))
._accountId; ._accountId;
assertThat(accountId2).isEqualTo(admin.id().get()); assertThat(accountId2).isEqualTo(admin.id().get());
@@ -119,7 +119,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
@Test @Test
public void removeUser() throws Exception { public void removeUser() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "added")); change(r).addToAttentionSet(new AttentionSetInput(user.email(), "added"));
fakeClock.advance(Duration.ofSeconds(42)); fakeClock.advance(Duration.ofSeconds(42));
change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed")); change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
AttentionSetUpdate expectedAttentionSetUpdate = AttentionSetUpdate expectedAttentionSetUpdate =
@@ -138,7 +138,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
@Test @Test
public void changeMessageWhenAddedAndRemovedExplicitly() throws Exception { public void changeMessageWhenAddedAndRemovedExplicitly() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user")); change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
assertThat(Iterables.getLast(r.getChange().messages()).getMessage()) assertThat(Iterables.getLast(r.getChange().messages()).getMessage())
.contains("Added to attention set"); .contains("Added to attention set");
@@ -157,8 +157,8 @@ public class AttentionSetIT extends AbstractDaemonTest {
@Test @Test
public void abandonRemovesUsers() throws Exception { public void abandonRemovesUsers() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user")); change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
change(r).addToAttentionSet(new AddToAttentionSetInput(admin.email(), "admin")); change(r).addToAttentionSet(new AttentionSetInput(admin.email(), "admin"));
change(r).abandon(); change(r).abandon();
@@ -178,7 +178,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
@Test @Test
public void workInProgressRemovesUsers() throws Exception { public void workInProgressRemovesUsers() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "reason")); change(r).addToAttentionSet(new AttentionSetInput(user.email(), "reason"));
change(r).setWorkInProgress(); change(r).setWorkInProgress();
@@ -392,7 +392,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
@Test @Test
public void reviewersAreNotAddedForNoReasonBecauseOfAnUpdate() throws Exception { public void reviewersAreNotAddedForNoReasonBecauseOfAnUpdate() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user")); 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 RemoveFromAttentionSetInput("removed"));
HashtagsInput hashtagsInput = new HashtagsInput(); HashtagsInput hashtagsInput = new HashtagsInput();

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

@@ -59,8 +59,8 @@ import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames; import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput; import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes.QueryRequest; import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.DraftInput;
@@ -3016,7 +3016,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Change change1 = insert(repo, newChange(repo)); Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo)); Change change2 = insert(repo, newChange(repo));
AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "some reason"); AttentionSetInput input = new AttentionSetInput(userId.toString(), "some reason");
gApi.changes().id(change1.getChangeId()).addToAttentionSet(input); gApi.changes().id(change1.getChangeId()).addToAttentionSet(input);
assertQuery("attention:" + user.getUserName().get(), change1); assertQuery("attention:" + user.getUserName().get(), change1);
@@ -3029,11 +3029,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
TestRepository<Repo> repo = createProject("repo"); TestRepository<Repo> repo = createProject("repo");
Change change = insert(repo, newChange(repo)); Change change = insert(repo, newChange(repo));
AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "reason 1"); AttentionSetInput input = new AttentionSetInput(userId.toString(), "reason 1");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input); gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
Account.Id user2Id = Account.Id user2Id =
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
input = new AddToAttentionSetInput(user2Id.toString(), "reason 2"); input = new AttentionSetInput(user2Id.toString(), "reason 2");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input); gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
List<ChangeInfo> result = newQuery("attention:" + user2Id.toString()).get(); List<ChangeInfo> result = newQuery("attention:" + user2Id.toString()).get();