opendev/gerrit
Code Issues Proposed changes

REST endpoints for managing the attention set

Browse Source 
Attention set is returned by default in change details.

Background:
https://www.gerritcodereview.com/design-docs/attention-set.html

Upcoming further changes:
- Add to index
- Extend ReviewInput API to add reviewers
- Add invariants (e.g. user must be reviewer)
- Send notifications in *Attention*Op#postUpdate()
- Consider adding NotificationHandling in AttentionInput
- Consider adding validation listeners

Change-Id: I52ae870a94852ac98f731fef63f65cd2a7064742
This commit is contained in:
Joerg Zieren 2020-03-06 13:44:25 +01:00
parent d4550a1e0b
commit fdbea383d6
38 changed files with 1111 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -40,6 +40,7 @@ import com.google.common.collect.Lists;
import com.google.common.jimfs.Jimfs;
import com.google.common.primitives.Chars;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.Nullable;
@ -61,6 +62,7 @@ import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
@ -836,6 +838,10 @@ public abstract class AbstractDaemonTest {
return gApi.changes().id(id).info();
}
protected ChangeApi change(Result r) throws RestApiException {
return gApi.changes().id(r.getChange().getId().get());
}
protected Optional<EditInfo> getEdit(String id) throws RestApiException {
return gApi.changes().id(id).edit().get();
}

17
java/com/google/gerrit/entities/AttentionStatus.java → java/com/google/gerrit/entities/AttentionSetUpdate.java
View File

@ -21,14 +21,13 @@ import java.time.Instant;
/**
* A single update to the attention set. To reconstruct the attention set these instances are parsed
* in reverse chronological order. Since each update contains all required information and
* invalidates all previous state (hence the name -Status rather than -Update), 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 <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">here</a> for
* details.
* <p>See {@link com.google.gerrit.extensions.api.changes.AttentionSetInput} for the representation
* in the API.
*/
@AutoValue
public abstract class AttentionStatus {
public abstract class AttentionSetUpdate {
/** Users can be added to or removed from the attention set. */
public enum Operation {
@ -56,17 +55,17 @@ public abstract class AttentionStatus {
* Create an instance from data read from NoteDB. This includes the timestamp taken from the
* commit.
*/
public static AttentionStatus createFromRead(
public static AttentionSetUpdate createFromRead(
Instant timestamp, Account.Id account, Operation operation, String reason) {
return new AutoValue_AttentionStatus(timestamp, account, operation, reason);
return new AutoValue_AttentionSetUpdate(timestamp, account, operation, reason);
}
/**
* Create an instance to be written to NoteDB. This has no timestamp because the timestamp of the
* commit will be used.
*/
public static AttentionStatus createForWrite(
public static AttentionSetUpdate createForWrite(
Account.Id account, Operation operation, String reason) {
return new AutoValue_AttentionStatus(null, account, operation, reason);
return new AutoValue_AttentionSetUpdate(null, account, operation, reason);
}
}

33
java/com/google/gerrit/extensions/api/changes/AddToAttentionSetInput.java Normal file
View File

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

35
java/com/google/gerrit/extensions/api/changes/AttentionSetApi.java Normal file
View File

@ -0,0 +1,35 @@
// 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.NotImplementedException;
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;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
*/
class NotImplemented implements AttentionSetApi {
@Override
public void remove(RemoveFromAttentionSetInput input) throws RestApiException {
throw new NotImplementedException();
}
}
}

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

@ -312,6 +312,16 @@ public interface ChangeApi {
*/
Set<String> getHashtags() throws RestApiException;
/**
* Manage the attention set.
*
* @param id The account identifier.
*/
AttentionSetApi attention(String id) throws RestApiException;
/** Adds a user to the attention set. */
AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException;
/** Set the assignee of a change. */
AccountInfo setAssignee(AssigneeInput input) throws RestApiException;
@ -580,6 +590,16 @@ public interface ChangeApi {
throw new NotImplementedException();
}
@Override
public AttentionSetApi attention(String id) throws RestApiException {
throw new NotImplementedException();
}
@Override
public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException {
throw new NotImplementedException();
}
@Override
public AccountInfo setAssignee(AssigneeInput input) throws RestApiException {
throw new NotImplementedException();

33
java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java Normal file
View File

@ -0,0 +1,33 @@
// 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 AddToAttentionSetInput
* @see com.google.gerrit.extensions.common.AttentionSetEntry
*/
public class RemoveFromAttentionSetInput {
@DefaultInput public String reason;
public RemoveFromAttentionSetInput(String reason) {
this.reason = reason;
}
public RemoveFromAttentionSetInput() {}
}

39
java/com/google/gerrit/extensions/common/AttentionSetEntry.java Normal file
View File

@ -0,0 +1,39 @@
// 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.common;
import java.sql.Timestamp;
/**
* Represents a single user included in the attention set. Used in the API. See {@link
* com.google.gerrit.entities.AttentionSetUpdate} for the internal representation.
*
* <p>See <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">here</a> for
* background.
*/
public class AttentionSetEntry {
/** The user included in the attention set. */
public AccountInfo accountInfo;
/** The timestamp of the last update. */
public Timestamp lastUpdate;
/** The human readable reason why the user was added. */
public String reason;
public AttentionSetEntry(AccountInfo accountInfo, Timestamp lastUpdate, String reason) {
this.accountInfo = accountInfo;
this.lastUpdate = lastUpdate;
this.reason = reason;
}
}

15
java/com/google/gerrit/extensions/common/ChangeInfo.java
View File

@ -22,13 +22,28 @@ import java.util.Collection;
import java.util.List;
import java.util.Map;
/**
* Representation of a change used in the API. Internally {@link
* com.google.gerrit.server.query.change.ChangeData} and {@link com.google.gerrit.entities.Change}
* are used.
*
* <p>Many fields are actually nullable.
*/
public class ChangeInfo {
// ActionJson#copy(List, ChangeInfo) must be adapted if new fields are added that are not
// protected by any ListChangesOption.
public String id;
public String project;
public String branch;
public String topic;
/**
* The <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">attention set</a>
* for this change. Keyed by account ID. We don't use {@link
* com.google.gerrit.entities.Account.Id} to avoid a circular dependency.
*/
public Map<Integer, AttentionSetEntry> attentionSet;
public AccountInfo assignee;
public Collection<String> hashtags;
public String changeId;

2
java/com/google/gerrit/server/ChangeMessagesUtil.java
View File

@ -49,6 +49,8 @@ public class ChangeMessagesUtil {
public static final String TAG_RESTORE = AUTOGENERATED_TAG_PREFIX + "gerrit:restore";
public static final String TAG_REVERT = AUTOGENERATED_TAG_PREFIX + "gerrit:revert";
public static final String TAG_SET_ASSIGNEE = AUTOGENERATED_TAG_PREFIX + "gerrit:setAssignee";
public static final String TAG_UPDATE_ATTENTION_SET =
AUTOGENERATED_TAG_PREFIX + "gerrit:updateAttentionSet";
public static final String TAG_SET_DESCRIPTION =
AUTOGENERATED_TAG_PREFIX + "gerrit:setPsDescription";
public static final String TAG_SET_HASHTAGS = AUTOGENERATED_TAG_PREFIX + "gerrit:setHashtag";

51
java/com/google/gerrit/server/api/changes/AttentionSetApiImpl.java Normal file
View File

@ -0,0 +1,51 @@
// 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.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.restapi.RestApiException;
import com.google.gerrit.server.change.AttentionSetEntryResource;
import com.google.gerrit.server.restapi.change.RemoveFromAttentionSet;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
public class AttentionSetApiImpl implements AttentionSetApi {
interface Factory {
AttentionSetApiImpl create(AttentionSetEntryResource attentionSetEntryResource);
}
private final RemoveFromAttentionSet removeFromAttentionSet;
private final AttentionSetEntryResource attentionSetEntryResource;
@Inject
AttentionSetApiImpl(
RemoveFromAttentionSet removeFromAttentionSet,
@Assisted AttentionSetEntryResource attentionSetEntryResource) {
this.removeFromAttentionSet = removeFromAttentionSet;
this.attentionSetEntryResource = attentionSetEntryResource;
}
@Override
public void remove(RemoveFromAttentionSetInput input) throws RestApiException {
try {
removeFromAttentionSet.apply(attentionSetEntryResource, input);
} catch (Exception e) {
throw asRestApiException("Cannot remove from attention set", e);
}
}
}

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

@ -23,7 +23,9 @@ import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
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.AttentionSetApi;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.ChangeMessageApi;
@ -66,6 +68,8 @@ import com.google.gerrit.server.change.ChangeMessageResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.restapi.change.Abandon;
import com.google.gerrit.server.restapi.change.AddToAttentionSet;
import com.google.gerrit.server.restapi.change.AttentionSet;
import com.google.gerrit.server.restapi.change.ChangeIncludedIn;
import com.google.gerrit.server.restapi.change.ChangeMessages;
import com.google.gerrit.server.restapi.change.Check;
@ -147,6 +151,9 @@ class ChangeApiImpl implements ChangeApi {
private final Provider<GetChange> getChangeProvider;
private final PostHashtags postHashtags;
private final GetHashtags getHashtags;
private final AttentionSet attentionSet;
private final AttentionSetApiImpl.Factory attentionSetApi;
private final AddToAttentionSet addToAttentionSet;
private final PutAssignee putAssignee;
private final GetAssignee getAssignee;
private final GetPastAssignees getPastAssignees;
@ -197,6 +204,9 @@ class ChangeApiImpl implements ChangeApi {
Provider<GetChange> getChangeProvider,
PostHashtags postHashtags,
GetHashtags getHashtags,
AttentionSet attentionSet,
AttentionSetApiImpl.Factory attentionSetApi,
AddToAttentionSet addToAttentionSet,
PutAssignee putAssignee,
GetAssignee getAssignee,
GetPastAssignees getPastAssignees,
@ -245,6 +255,9 @@ class ChangeApiImpl implements ChangeApi {
this.getChangeProvider = getChangeProvider;
this.postHashtags = postHashtags;
this.getHashtags = getHashtags;
this.attentionSet = attentionSet;
this.attentionSetApi = attentionSetApi;
this.addToAttentionSet = addToAttentionSet;
this.putAssignee = putAssignee;
this.getAssignee = getAssignee;
this.getPastAssignees = getPastAssignees;
@ -529,6 +542,24 @@ class ChangeApiImpl implements ChangeApi {
}
}
@Override
public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException {
try {
return addToAttentionSet.apply(change, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot add to attention set", e);
}
}
@Override
public AttentionSetApi attention(String id) throws RestApiException {
try {
return attentionSetApi.create(attentionSet.parse(change, IdString.fromDecoded(id)));
} catch (Exception e) {
throw asRestApiException("Cannot parse account", e);
}
}
@Override
public AccountInfo setAssignee(AssigneeInput input) throws RestApiException {
try {

1
java/com/google/gerrit/server/api/changes/Module.java
View File

@ -32,5 +32,6 @@ public class Module extends FactoryModule {
factory(RevisionReviewerApiImpl.Factory.class);
factory(ChangeEditApiImpl.Factory.class);
factory(ChangeMessageApiImpl.Factory.class);
factory(AttentionSetApiImpl.Factory.class);
}
}

9
java/com/google/gerrit/server/change/ActionJson.java
View File

@ -89,14 +89,12 @@ public class ActionJson {
return Lists.newArrayList(visitorSet);
}
public ChangeInfo addChangeActions(ChangeInfo to, ChangeNotes notes) {
void addChangeActions(ChangeInfo to, ChangeNotes notes) {
List<ActionVisitor> visitors = visitors();
to.actions = toActionMap(notes, visitors, copy(visitors, to));
return to;
}
public RevisionInfo addRevisionActions(
@Nullable ChangeInfo changeInfo, RevisionInfo to, RevisionResource rsrc) {
void addRevisionActions(@Nullable ChangeInfo changeInfo, RevisionInfo to, RevisionResource rsrc) {
List<ActionVisitor> visitors = visitors();
if (!visitors.isEmpty()) {
if (changeInfo != null) {
@ -106,7 +104,6 @@ public class ActionJson {
}
}
to.actions = toActionMap(rsrc, visitors, changeInfo, copy(visitors, to));
return to;
}
private ChangeInfo copy(List<ActionVisitor> visitors, ChangeInfo changeInfo) {
@ -119,6 +116,8 @@ public class ActionJson {
copy.project = changeInfo.project;
copy.branch = changeInfo.branch;
copy.topic = changeInfo.topic;
copy.attentionSet =
changeInfo.attentionSet == null ? null : ImmutableMap.copyOf(changeInfo.attentionSet);
copy.assignee = changeInfo.assignee;
copy.hashtags = changeInfo.hashtags;
copy.changeId = changeInfo.changeId;

85
java/com/google/gerrit/server/change/AddToAttentionSetOp.java Normal file
View File

@ -0,0 +1,85 @@
// 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.server.change;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Map;
import java.util.function.Function;
/** Add a specified user to the attention set. */
public class AddToAttentionSetOp implements BatchUpdateOp {
public interface Factory {
AddToAttentionSetOp create(Account.Id attentionUserId, String reason);
}
private final ChangeData.Factory changeDataFactory;
private final ChangeMessagesUtil cmUtil;
private final Account.Id attentionUserId;
private final String reason;
@Inject
AddToAttentionSetOp(
ChangeData.Factory changeDataFactory,
ChangeMessagesUtil cmUtil,
@Assisted Account.Id attentionUserId,
@Assisted String reason) {
this.changeDataFactory = changeDataFactory;
this.cmUtil = cmUtil;
this.attentionUserId = requireNonNull(attentionUserId, "user");
this.reason = requireNonNull(reason, "reason");
}
@Override
public boolean updateChange(ChangeContext ctx) throws RestApiException {
ChangeData changeData = changeDataFactory.create(ctx.getNotes());
Map<Account.Id, AttentionSetUpdate> attentionMap =
changeData.attentionSet().stream()
.collect(toImmutableMap(AttentionSetUpdate::account, Function.identity()));
AttentionSetUpdate existingEntry = attentionMap.get(attentionUserId);
if (existingEntry != null && existingEntry.operation() == Operation.ADD) {
return false;
}
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
update.setAttentionSetUpdates(
ImmutableList.of(
AttentionSetUpdate.createForWrite(
attentionUserId, AttentionSetUpdate.Operation.ADD, reason)));
addMessage(ctx, update);
return true;
}
private void addMessage(ChangeContext ctx, ChangeUpdate update) {
String message = "Added to attention set: " + attentionUserId;
cmUtil.addChangeMessage(
update,
ChangeMessagesUtil.newMessage(ctx, message, ChangeMessagesUtil.TAG_UPDATE_ATTENTION_SET));
}
}

46
java/com/google/gerrit/server/change/AttentionSetEntryResource.java Normal file
View File

@ -0,0 +1,46 @@
// 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.server.change;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.inject.TypeLiteral;
/** REST resource that represents an entry in the attention set of a change. */
public class AttentionSetEntryResource implements RestResource {
public static final TypeLiteral<RestView<AttentionSetEntryResource>> ATTENTION_SET_ENTRY_KIND =
new TypeLiteral<RestView<AttentionSetEntryResource>>() {};
public interface Factory {
AttentionSetEntryResource create(ChangeResource change, Account.Id id);
}
private final ChangeResource changeResource;
private final Account.Id accountId;
public AttentionSetEntryResource(ChangeResource changeResource, Account.Id accountId) {
this.changeResource = changeResource;
this.accountId = accountId;
}
public ChangeResource getChangeResource() {
return changeResource;
}
public Account.Id getAccountId() {
return accountId;
}
}

18
java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -14,6 +14,7 @@
package com.google.gerrit.server.change;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_COMMITS;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CHANGE_ACTIONS;
@ -49,6 +50,7 @@ import com.google.gerrit.common.data.SubmitRecord.Status;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
@ -60,6 +62,7 @@ import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.AttentionSetEntry;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.LabelInfo;
@ -102,6 +105,7 @@ import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@ -504,6 +508,20 @@ public class ChangeJson {
out.project = in.getProject().get();
out.branch = in.getDest().shortName();
out.topic = in.getTopic();
if (!cd.attentionSet().isEmpty()) {
out.attentionSet =
cd.attentionSet().stream()
// This filtering should match GetAttentionSet.
.filter(a -> a.operation() == AttentionSetUpdate.Operation.ADD)
.collect(
toImmutableMap(
a -> a.account().get(),
a ->
new AttentionSetEntry(
accountLoader.get(a.account()),
Timestamp.from(a.timestamp()),
a.reason())));
}
out.assignee = in.getAssignee() != null ? accountLoader.get(in.getAssignee()) : null;
out.hashtags = cd.hashtags();
out.changeId = in.getKey().get();

84
java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java Normal file
View File

@ -0,0 +1,84 @@
// 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.server.change;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Map;
import java.util.function.Function;
/** Remove a specified user from the attention set. */
public class RemoveFromAttentionSetOp implements BatchUpdateOp {
public interface Factory {
RemoveFromAttentionSetOp create(Account.Id attentionUserId, String reason);
}
private final ChangeData.Factory changeDataFactory;
private final ChangeMessagesUtil cmUtil;
private final Account.Id attentionUserId;
private final String reason;
@Inject
RemoveFromAttentionSetOp(
ChangeData.Factory changeDataFactory,
ChangeMessagesUtil cmUtil,
@Assisted Account.Id attentionUserId,
@Assisted String reason) {
this.changeDataFactory = changeDataFactory;
this.cmUtil = cmUtil;
this.attentionUserId = requireNonNull(attentionUserId, "user");
this.reason = requireNonNull(reason, "reason");
}
@Override
public boolean updateChange(ChangeContext ctx) throws RestApiException {
ChangeData changeData = changeDataFactory.create(ctx.getNotes());
Map<Account.Id, AttentionSetUpdate> attentionMap =
changeData.attentionSet().stream()
.collect(toImmutableMap(AttentionSetUpdate::account, Function.identity()));
AttentionSetUpdate existingEntry = attentionMap.get(attentionUserId);
if (existingEntry == null || existingEntry.operation() == Operation.REMOVE) {
return false;
}
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
update.setAttentionSetUpdates(
ImmutableList.of(
AttentionSetUpdate.createForWrite(attentionUserId, Operation.REMOVE, reason)));
addMessage(ctx, update);
return true;
}
private void addMessage(ChangeContext ctx, ChangeUpdate update) {
String message = "Removed from attention set: " + attentionUserId;
cmUtil.addChangeMessage(
update,
ChangeMessagesUtil.newMessage(ctx, message, ChangeMessagesUtil.TAG_UPDATE_ATTENTION_SET));
}
}

18
java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
View File

@ -16,7 +16,7 @@ package com.google.gerrit.server.notedb;
import com.google.auto.value.AutoValue;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gson.Gson;
@ -171,11 +171,11 @@ public class ChangeNoteUtil {
private static class AttentionStatusInNoteDb {
final String personIdent;
final AttentionStatus.Operation operation;
final AttentionSetUpdate.Operation operation;
final String reason;
AttentionStatusInNoteDb(
String personIndent, AttentionStatus.Operation operation, String reason) {
String personIndent, AttentionSetUpdate.Operation operation, String reason) {
this.personIdent = personIndent;
this.operation = operation;
this.reason = reason;
@ -183,7 +183,7 @@ public class ChangeNoteUtil {
}
/** The returned {@link Optional} holds the parsed entity or is empty if parsing failed. */
static Optional<AttentionStatus> attentionStatusFromJson(
static Optional<AttentionSetUpdate> attentionStatusFromJson(
Instant timestamp, String attentionString) {
AttentionStatusInNoteDb inNoteDb =
gson.fromJson(attentionString, AttentionStatusInNoteDb.class);
@ -193,18 +193,20 @@ public class ChangeNoteUtil {
}
Optional<Account.Id> account = NoteDbUtil.parseIdent(personIdent);
return account.map(
id -> AttentionStatus.createFromRead(timestamp, id, inNoteDb.operation, inNoteDb.reason));
id ->
AttentionSetUpdate.createFromRead(timestamp, id, inNoteDb.operation, inNoteDb.reason));
}
String attentionStatusToJson(AttentionStatus attentionStatus) {
String attentionSetUpdateToJson(AttentionSetUpdate attentionSetUpdate) {
PersonIdent personIdent =
new PersonIdent(
getUsername(attentionStatus.account()), getEmailAddress(attentionStatus.account()));
getUsername(attentionSetUpdate.account()),
getEmailAddress(attentionSetUpdate.account()));
StringBuilder stringBuilder = new StringBuilder();
appendIdentString(stringBuilder, personIdent.getName(), personIdent.getEmailAddress());
return gson.toJson(
new AttentionStatusInNoteDb(
stringBuilder.toString(), attentionStatus.operation(), attentionStatus.reason()));
stringBuilder.toString(), attentionSetUpdate.operation(), attentionSetUpdate.reason()));
}
static void appendIdentString(StringBuilder stringBuilder, String name, String emailAddress) {

7
java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@ -40,7 +40,7 @@ import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@ -376,8 +376,9 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return state.reviewerUpdates();
}
public ImmutableList<AttentionStatus> getAttentionUpdates() {
return state.attentionUpdates();
/** Returns the most recent update (i.e. status) per user. */
public ImmutableList<AttentionSetUpdate> getAttentionSet() {
return state.attentionSet();
}
/**

10
java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@ -59,7 +59,7 @@ import com.google.common.primitives.Ints;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@ -118,7 +118,7 @@ class ChangeNotesParser {
private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates;
/** Holds only the most recent update per user. Older updates are discarded. */
private final Map<Account.Id, AttentionStatus> latestAttentionStatus;
private final Map<Account.Id, AttentionSetUpdate> latestAttentionStatus;
private final List<AssigneeStatusUpdate> assigneeUpdates;
private final List<SubmitRecord> submitRecords;
@ -367,7 +367,7 @@ class ChangeNotesParser {
}
parseHashtags(commit);
parseAttentionUpdates(commit);
parseAttentionSetUpdates(commit);
parseAssigneeUpdates(ts, commit);
if (submissionId == null) {
@ -578,11 +578,11 @@ class ChangeNotesParser {
}
}
private void parseAttentionUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
private void parseAttentionSetUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
List<String> attentionStrings = commit.getFooterLineValues(FOOTER_ATTENTION);
for (String attentionString : attentionStrings) {
Optional<AttentionStatus> attentionStatus =
Optional<AttentionSetUpdate> attentionStatus =
ChangeNoteUtil.attentionStatusFromJson(
Instant.ofEpochSecond(commit.getCommitTime()), attentionString);
if (!attentionStatus.isPresent()) {

44
java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@ -35,7 +35,7 @@ import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@ -56,7 +56,7 @@ import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionSetUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@ -119,7 +119,7 @@ public abstract class ChangeNotesState {
ReviewerByEmailSet pendingReviewersByEmail,
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
List<AttentionStatus> attentionStatusUpdates,
List<AttentionSetUpdate> attentionSetUpdates,
List<AssigneeStatusUpdate> assigneeUpdates,
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
@ -170,7 +170,7 @@ public abstract class ChangeNotesState {
.pendingReviewersByEmail(pendingReviewersByEmail)
.allPastReviewers(allPastReviewers)
.reviewerUpdates(reviewerUpdates)
.attentionUpdates(attentionStatusUpdates)
.attentionSet(attentionSetUpdates)
.assigneeUpdates(assigneeUpdates)
.submitRecords(submitRecords)
.changeMessages(changeMessages)
@ -305,7 +305,8 @@ public abstract class ChangeNotesState {
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();
abstract ImmutableList<AttentionStatus> attentionUpdates();
/** Returns the most recent update (i.e. current status status) per user. */
abstract ImmutableList<AttentionSetUpdate> attentionSet();
abstract ImmutableList<AssigneeStatusUpdate> assigneeUpdates();
@ -384,7 +385,7 @@ public abstract class ChangeNotesState {
.pendingReviewersByEmail(ReviewerByEmailSet.empty())
.allPastReviewers(ImmutableList.of())
.reviewerUpdates(ImmutableList.of())
.attentionUpdates(ImmutableList.of())
.attentionSet(ImmutableList.of())
.assigneeUpdates(ImmutableList.of())
.submitRecords(ImmutableList.of())
.changeMessages(ImmutableList.of())
@ -418,7 +419,7 @@ public abstract class ChangeNotesState {
abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
abstract Builder attentionUpdates(List<AttentionStatus> attentionUpdates);
abstract Builder attentionSet(List<AttentionSetUpdate> attentionSetUpdates);
abstract Builder assigneeUpdates(List<AssigneeStatusUpdate> assigneeUpdates);
@ -487,7 +488,7 @@ public abstract class ChangeNotesState {
object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get()));
object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u)));
object.attentionUpdates().forEach(u -> b.addAttentionStatus(toAttentionStatusProto(u)));
object.attentionSet().forEach(u -> b.addAttentionSetUpdate(toAttentionSetUpdateProto(u)));
object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u)));
object
.submitRecords()
@ -571,12 +572,13 @@ public abstract class ChangeNotesState {
.build();
}
private static AttentionStatusProto toAttentionStatusProto(AttentionStatus attentionStatus) {
return AttentionStatusProto.newBuilder()
.setTimestampMillis(attentionStatus.timestamp().toEpochMilli())
.setAccount(attentionStatus.account().get())
.setOperation(attentionStatus.operation().name())
.setReason(attentionStatus.reason())
private static AttentionSetUpdateProto toAttentionSetUpdateProto(
AttentionSetUpdate attentionSetUpdate) {
return AttentionSetUpdateProto.newBuilder()
.setTimestampMillis(attentionSetUpdate.timestamp().toEpochMilli())
.setAccount(attentionSetUpdate.account().get())
.setOperation(attentionSetUpdate.operation().name())
.setReason(attentionSetUpdate.reason())
.build();
}
@ -620,7 +622,7 @@ public abstract class ChangeNotesState {
.allPastReviewers(
proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList()))
.reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList()))
.attentionUpdates(toAttentionUpdateList(proto.getAttentionStatusList()))
.attentionSet(toAttentionSetUpdateList(proto.getAttentionSetUpdateList()))
.assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList()))
.submitRecords(
proto.getSubmitRecordList().stream()
@ -719,15 +721,15 @@ public abstract class ChangeNotesState {
return b.build();
}
private static ImmutableList<AttentionStatus> toAttentionUpdateList(
List<AttentionStatusProto> protos) {
ImmutableList.Builder<AttentionStatus> b = ImmutableList.builder();
for (AttentionStatusProto proto : protos) {
private static ImmutableList<AttentionSetUpdate> toAttentionSetUpdateList(
List<AttentionSetUpdateProto> protos) {
ImmutableList.Builder<AttentionSetUpdate> b = ImmutableList.builder();
for (AttentionSetUpdateProto proto : protos) {
b.add(
AttentionStatus.createFromRead(
AttentionSetUpdate.createFromRead(
Instant.ofEpochMilli(proto.getTimestampMillis()),
Account.id(proto.getAccount()),
AttentionStatus.Operation.valueOf(proto.getOperation()),
AttentionSetUpdate.Operation.valueOf(proto.getOperation()),
proto.getReason()));
}
return b.build();

22
java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@ -54,7 +54,7 @@ import com.google.common.collect.Table;
import com.google.common.collect.TreeBasedTable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Project;
@ -128,7 +128,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private String submissionId;
private String topic;
private String commit;
private List<AttentionStatus> attentionUpdates;
private List<AttentionSetUpdate> attentionSetUpdates;
private Optional<Account.Id> assignee;
private Set<String> hashtags;
private String changeMessage;
@ -369,15 +369,15 @@ public class ChangeUpdate extends AbstractChangeUpdate {
* All updates must have a timestamp of null since we use the commit's timestamp. There also must
* not be multiple updates for a single user.
*/
void setAttentionUpdates(List<AttentionStatus> attentionUpdates) {
public void setAttentionSetUpdates(List<AttentionSetUpdate> attentionSetUpdates) {
checkArgument(
attentionUpdates.stream().noneMatch(x -> x.timestamp() != null),
attentionSetUpdates.stream().noneMatch(a -> a.timestamp() != null),
"must not specify timestamp for write");
checkArgument(
attentionUpdates.stream().map(AttentionStatus::account).distinct().count()
== attentionUpdates.size(),
attentionSetUpdates.stream().map(AttentionSetUpdate::account).distinct().count()
== attentionSetUpdates.size(),
"must not specify multiple updates for single user");
this.attentionUpdates = attentionUpdates;
this.attentionSetUpdates = attentionSetUpdates;
}
public void setAssignee(Account.Id assignee) {
@ -588,9 +588,9 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addFooter(msg, FOOTER_COMMIT, commit);
}
if (attentionUpdates != null) {
for (AttentionStatus attentionUpdate : attentionUpdates) {
addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionStatusToJson(attentionUpdate));
if (attentionSetUpdates != null) {
for (AttentionSetUpdate attentionSetUpdate : attentionSetUpdates) {
addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionSetUpdateToJson(attentionSetUpdate));
}
}
@ -730,7 +730,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
&& status == null
&& submissionId == null
&& submitRecords == null
&& attentionUpdates == null
&& attentionSetUpdates == null
&& assignee == null
&& hashtags == null
&& topic == null

13
java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -37,6 +37,7 @@ import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@ -297,6 +298,7 @@ public class ChangeData {
private List<ReviewerStatusUpdate> reviewerUpdates;
private PersonIdent author;
private PersonIdent committer;
private ImmutableList<AttentionSetUpdate> attentionSet;
private int parentCount;
private Integer unresolvedCommentCount;
private Integer totalCommentCount;
@ -598,6 +600,17 @@ public class ChangeData {
return true;
}
/** Returns the most recent update (i.e. status) per user. */
public ImmutableList<AttentionSetUpdate> attentionSet() {
if (attentionSet == null) {
if (!lazyLoad) {
return ImmutableList.of();
}
attentionSet = notes().getAttentionSet();
}
return attentionSet;
}
/** @return patches for the change, in patch set ID order. */
public Collection<PatchSet> patchSets() {
if (patchSets == null) {

93
java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java Normal file
View File

@ -0,0 +1,93 @@
// 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.server.restapi.change;
import com.google.common.base.Strings;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.AttentionSetEntryResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Singleton;
/** Adds a single user to the attention set. */
@Singleton
public class AddToAttentionSet
implements RestCollectionModifyView<
ChangeResource, AttentionSetEntryResource, AddToAttentionSetInput> {
private final BatchUpdate.Factory updateFactory;
private final AccountResolver accountResolver;
private final AddToAttentionSetOp.Factory opFactory;
private final AccountLoader.Factory accountLoaderFactory;
private final PermissionBackend permissionBackend;
@Inject
AddToAttentionSet(
BatchUpdate.Factory updateFactory,
AccountResolver accountResolver,
AddToAttentionSetOp.Factory opFactory,
AccountLoader.Factory accountLoaderFactory,
PermissionBackend permissionBackend) {
this.updateFactory = updateFactory;
this.accountResolver = accountResolver;
this.opFactory = opFactory;
this.accountLoaderFactory = accountLoaderFactory;
this.permissionBackend = permissionBackend;
}
@Override
public Response<AccountInfo> apply(ChangeResource changeResource, AddToAttentionSetInput input)
throws Exception {
input.user = Strings.nullToEmpty(input.user).trim();
if (input.user.isEmpty()) {
throw new BadRequestException("missing field: user");
}
input.reason = Strings.nullToEmpty(input.reason).trim();
if (input.reason.isEmpty()) {
throw new BadRequestException("missing field: reason");
}
Account.Id attentionUserId = accountResolver.resolve(input.user).asUnique().account().id();
try {
permissionBackend
.absentUser(attentionUserId)
.change(changeResource.getNotes())
.check(ChangePermission.READ);
} catch (AuthException e) {
throw new AuthException("read not permitted for " + attentionUserId, e);
}
try (BatchUpdate bu =
updateFactory.create(
changeResource.getChange().getProject(), changeResource.getUser(), TimeUtil.nowTs())) {
AddToAttentionSetOp op = opFactory.create(attentionUserId, input.reason);
bu.addOp(changeResource.getId(), op);
bu.execute();
return Response.ok(accountLoaderFactory.create(true).fillOne(attentionUserId));
}
}
}

69
java/com/google/gerrit/server/restapi/change/AttentionSet.java Normal file
View File

@ -0,0 +1,69 @@
// 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.server.restapi.change;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountResolver.UnresolvableAccountException;
import com.google.gerrit.server.change.AttentionSetEntryResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class AttentionSet implements ChildCollection<ChangeResource, AttentionSetEntryResource> {
private final DynamicMap<RestView<AttentionSetEntryResource>> views;
private final AccountResolver accountResolver;
private final GetAttentionSet getAttentionSet;
@Inject
AttentionSet(
DynamicMap<RestView<AttentionSetEntryResource>> views,
GetAttentionSet getAttentionSet,
AccountResolver accountResolver) {
this.views = views;
this.accountResolver = accountResolver;
this.getAttentionSet = getAttentionSet;
}
@Override
public DynamicMap<RestView<AttentionSetEntryResource>> views() {
return views;
}
@Override
public RestView<ChangeResource> list() throws ResourceNotFoundException {
return getAttentionSet;
}
@Override
public AttentionSetEntryResource parse(ChangeResource changeResource, IdString idString)
throws ResourceNotFoundException, AuthException, IOException, ConfigInvalidException {
try {
Account.Id accountId = accountResolver.resolve(idString.get()).asUnique().account().id();
return new AttentionSetEntryResource(changeResource, accountId);
} catch (UnresolvableAccountException e) {
throw new ResourceNotFoundException(idString, e);
}
}
}

59
java/com/google/gerrit/server/restapi/change/GetAttentionSet.java Normal file
View File

@ -0,0 +1,59 @@
// 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.server.restapi.change;
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.extensions.common.AttentionSetEntry;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.sql.Timestamp;
import java.util.List;
/** Reads the list of users currently in the attention set. */
@Singleton
public class GetAttentionSet implements RestReadView<ChangeResource> {
private final AccountLoader.Factory accountLoaderFactory;
@Inject
GetAttentionSet(AccountLoader.Factory accountLoaderFactory) {
this.accountLoaderFactory = accountLoaderFactory;
}
@Override
public Response<List<AttentionSetEntry>> apply(ChangeResource changeResource)
throws PermissionBackendException {
AccountLoader accountLoader = accountLoaderFactory.create(true);
ImmutableList<AttentionSetEntry> response =
changeResource.getNotes().getAttentionSet().stream()
// This filtering should match ChangeJson.
.filter(a -> a.operation() == Operation.ADD)
.map(
a ->
new AttentionSetEntry(
accountLoader.get(a.account()), Timestamp.from(a.timestamp()), a.reason()))
.collect(toImmutableList());
accountLoader.fill();
return Response.ok(response);
}
}

10
java/com/google/gerrit/server/restapi/change/Module.java
View File

@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.change.AttentionSetEntryResource.ATTENTION_SET_ENTRY_KIND;
import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_KIND;
import static com.google.gerrit.server.change.ChangeMessageResource.CHANGE_MESSAGE_KIND;
import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
@ -30,6 +31,7 @@ import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.AddReviewersOp;
import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.DeleteChangeOp;
@ -38,6 +40,7 @@ import com.google.gerrit.server.change.DeleteReviewerOp;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.RebaseChangeOp;
import com.google.gerrit.server.change.RemoveFromAttentionSetOp;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.SetAssigneeOp;
import com.google.gerrit.server.change.SetCherryPickOp;
@ -72,6 +75,7 @@ public class Module extends RestApiModule {
DynamicMap.mapOf(binder(), CHANGE_EDIT_KIND);
DynamicMap.mapOf(binder(), VOTE_KIND);
DynamicMap.mapOf(binder(), CHANGE_MESSAGE_KIND);
DynamicMap.mapOf(binder(), ATTENTION_SET_ENTRY_KIND);
postOnCollection(CHANGE_KIND).to(CreateChange.class);
get(CHANGE_KIND).to(GetChange.class);
@ -79,6 +83,10 @@ public class Module extends RestApiModule {
get(CHANGE_KIND, "detail").to(GetDetail.class);
get(CHANGE_KIND, "topic").to(GetTopic.class);
get(CHANGE_KIND, "in").to(ChangeIncludedIn.class);
child(CHANGE_KIND, "attention").to(AttentionSet.class);
delete(ATTENTION_SET_ENTRY_KIND).to(RemoveFromAttentionSet.class);
post(ATTENTION_SET_ENTRY_KIND, "delete").to(RemoveFromAttentionSet.class);
postOnCollection(ATTENTION_SET_ENTRY_KIND).to(AddToAttentionSet.class);
get(CHANGE_KIND, "assignee").to(GetAssignee.class);
get(CHANGE_KIND, "past_assignees").to(GetPastAssignees.class);
put(CHANGE_KIND, "assignee").to(PutAssignee.class);
@ -207,5 +215,7 @@ public class Module extends RestApiModule {
factory(SetPrivateOp.Factory.class);
factory(WorkInProgressOp.Factory.class);
factory(SetTopicOp.Factory.class);
factory(AddToAttentionSetOp.Factory.class);
factory(RemoveFromAttentionSetOp.Factory.class);
}
}

70
java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java Normal file
View File

@ -0,0 +1,70 @@
// 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.server.restapi.change;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
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.change.AttentionSetEntryResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RemoveFromAttentionSetOp;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
/** Removes a single user from the attention set. */
public class RemoveFromAttentionSet
implements RestModifyView<AttentionSetEntryResource, RemoveFromAttentionSetInput> {
private final BatchUpdate.Factory updateFactory;
private final RemoveFromAttentionSetOp.Factory opFactory;
@Inject
RemoveFromAttentionSet(
BatchUpdate.Factory updateFactory, RemoveFromAttentionSetOp.Factory opFactory) {
this.updateFactory = updateFactory;
this.opFactory = opFactory;
}
@Override
public Response<Object> apply(
AttentionSetEntryResource attentionResource, RemoveFromAttentionSetInput input)
throws RestApiException, PermissionBackendException, IOException, ConfigInvalidException,
UpdateException {
if (input == null) {
throw new BadRequestException("input may not be null");
}
input.reason = Strings.nullToEmpty(input.reason).trim();
if (input.reason.isEmpty()) {
throw new BadRequestException("missing field: reason");
}
ChangeResource changeResource = attentionResource.getChangeResource();
try (BatchUpdate bu =
updateFactory.create(
changeResource.getProject(), changeResource.getUser(), TimeUtil.nowTs())) {
RemoveFromAttentionSetOp op =
opFactory.create(attentionResource.getAccountId(), input.reason);
bu.addOp(changeResource.getId(), op);
bu.execute();
}
return Response.none();
}
}

24
javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java
View File

@ -53,14 +53,14 @@ public class CheckProjectIT extends AbstractDaemonTest {
PushOneCommit.Result r = createChange("refs/for/master");
String branch = r.getChange().change().getDest().branch();
ChangeInfo info = gApi.changes().id(r.getChange().getId().get()).info();
ChangeInfo info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
CheckProjectResultInfo checkResult =
gApi.projects().name(project.get()).check(checkProjectInputForAutoCloseableCheck(branch));
assertThat(checkResult.autoCloseableChangesCheckResult.autoCloseableChanges).isEmpty();
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
}
@ -121,7 +121,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
RevCommit amendedCommit = serverSideTestRepo.amend(r.getCommit()).create();
serverSideTestRepo.branch(branch).update(amendedCommit);
ChangeInfo info = gApi.changes().id(r.getChange().getId().get()).info();
ChangeInfo info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
CheckProjectResultInfo checkResult =
@ -132,7 +132,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
.collect(toSet()))
.containsExactly(r.getChange().getId().get());
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
}
@ -144,7 +144,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
RevCommit amendedCommit = serverSideTestRepo.amend(r.getCommit()).create();
serverSideTestRepo.branch(branch).update(amendedCommit);
ChangeInfo info = gApi.changes().id(r.getChange().getId().get()).info();
ChangeInfo info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
CheckProjectInput input = checkProjectInputForAutoCloseableCheck(branch);
@ -156,7 +156,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
.collect(toSet()))
.containsExactly(r.getChange().getId().get());
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
}
@ -170,7 +170,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
serverSideTestRepo.commit(amendedCommit);
ChangeInfo info = gApi.changes().id(r.getChange().getId().get()).info();
ChangeInfo info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
CheckProjectInput input = checkProjectInputForAutoCloseableCheck(branch);
@ -179,7 +179,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
CheckProjectResultInfo checkResult = gApi.projects().name(project.get()).check(input);
assertThat(checkResult.autoCloseableChangesCheckResult.autoCloseableChanges).isEmpty();
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
input.autoCloseableChangesCheck.maxCommits = 2;
@ -190,7 +190,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
.collect(toSet()))
.containsExactly(r.getChange().getId().get());
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
}
@ -204,7 +204,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
serverSideTestRepo.commit(amendedCommit);
ChangeInfo info = gApi.changes().id(r.getChange().getId().get()).info();
ChangeInfo info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
CheckProjectInput input = checkProjectInputForAutoCloseableCheck(branch);
@ -213,7 +213,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
CheckProjectResultInfo checkResult = gApi.projects().name(project.get()).check(input);
assertThat(checkResult.autoCloseableChangesCheckResult.autoCloseableChanges).isEmpty();
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
input.autoCloseableChangesCheck.skipCommits = 1;
@ -224,7 +224,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
.collect(toSet()))
.containsExactly(r.getChange().getId().get());
info = gApi.changes().id(r.getChange().getId().get()).info();
info = change(r).info();
assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
}

10
javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@ -303,13 +303,7 @@ public class RevisionIT extends AbstractDaemonTest {
PushOneCommit.Result r = createChange();
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
assertThrows(
AuthException.class,
() ->
gApi.changes()
.id(r.getChange().getId().get())
.current()
.review(ReviewInput.approve()));
assertThrows(AuthException.class, () -> change(r).current().review(ReviewInput.approve()));
assertThat(thrown).hasMessageThat().contains("is restricted");
}
@ -560,7 +554,7 @@ public class RevisionIT extends AbstractDaemonTest {
PushOneCommit.Result r = push.to("refs/for/master%topic=someTopic");
// Verify before the cherry-pick that the change has exactly 1 message.
ChangeApi changeApi = gApi.changes().id(r.getChange().getId().get());
ChangeApi changeApi = change(r);
assertThat(changeApi.get().messages).hasSize(1);
// Cherry-pick the change to the other branch, that should fail with a conflict.

4
javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -437,7 +437,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.assertErrorStatus("change " + url + " closed");
// Check change message that was added on auto-close
ChangeInfo change = gApi.changes().id(r.getChange().getId().get()).get();
ChangeInfo change = change(r).get();
assertThat(Iterables.getLast(change.messages).message)
.isEqualTo("Change has been successfully pushed.");
}
@ -477,7 +477,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.assertErrorStatus("change " + url + " closed");
// Check that new commit was added as patch set
ChangeInfo change = gApi.changes().id(r.getChange().getId().get()).get();
ChangeInfo change = change(r).get();
assertThat(change.revisions).hasSize(2);
assertThat(change.currentRevision).isEqualTo(c.name());
}

15
javatests/com/google/gerrit/acceptance/rest/binding/ChangesRestApiBindingsIT.java
View File

@ -63,6 +63,8 @@ public class ChangesRestApiBindingsIT extends AbstractDaemonTest {
RestCall.get("/changes/%s/comments"),
RestCall.get("/changes/%s/robotcomments"),
RestCall.get("/changes/%s/drafts"),
RestCall.get("/changes/%s/attention"),
RestCall.post("/changes/%s/attention"),
RestCall.get("/changes/%s/assignee"),
RestCall.get("/changes/%s/past_assignees"),
RestCall.put("/changes/%s/assignee"),
@ -267,6 +269,11 @@ public class ChangesRestApiBindingsIT extends AbstractDaemonTest {
// Delete content of a file in an existing change edit.
RestCall.delete("/changes/%s/edit/%s"));
private static final ImmutableList<RestCall> ATTENTION_SET_ENDPOINTS =
ImmutableList.of(
RestCall.post("/changes/%s/attention/%s/delete"),
RestCall.delete("/changes/%s/attention/%s"));
private static final String FILENAME = "test.txt";
@Test
@ -477,6 +484,14 @@ public class ChangesRestApiBindingsIT extends AbstractDaemonTest {
RestApiCallHelper.execute(adminRestSession, CHANGE_EDIT_ENDPOINTS, changeId, FILENAME);
}
@Test
public void attentionSetEndpoints() throws Exception {
String changeId = createChange().getChangeId();
gApi.changes().id(changeId).edit().create();
RestApiCallHelper.execute(
adminRestSession, ATTENTION_SET_ENDPOINTS, changeId, user.id().toString());
}
private static Comment.Range createRange(
int startLine, int startCharacter, int endLine, int endCharacter) {
Comment.Range range = new Comment.Range();

8
javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
View File

@ -188,11 +188,11 @@ public class AssigneeIT extends AbstractDaemonTest {
}
private AccountInfo getAssignee(PushOneCommit.Result r) throws Exception {
return gApi.changes().id(r.getChange().getId().get()).getAssignee();
return change(r).getAssignee();
}
private List<AccountInfo> getPastAssignees(PushOneCommit.Result r) throws Exception {
return gApi.changes().id(r.getChange().getId().get()).getPastAssignees();
return change(r).getPastAssignees();
}
private Iterable<AccountInfo> getReviewers(PushOneCommit.Result r, ReviewerState state)
@ -203,10 +203,10 @@ public class AssigneeIT extends AbstractDaemonTest {
private AccountInfo setAssignee(PushOneCommit.Result r, String identifieer) throws Exception {
AssigneeInput input = new AssigneeInput();
input.assignee = identifieer;
return gApi.changes().id(r.getChange().getId().get()).setAssignee(input);
return change(r).setAssignee(input);
}
private AccountInfo deleteAssignee(PushOneCommit.Result r) throws Exception {
return gApi.changes().id(r.getChange().getId().get()).deleteAssignee();
return change(r).deleteAssignee();
}
}

136
javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java Normal file
View File

@ -0,0 +1,136 @@
// 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.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
import com.google.gerrit.server.util.time.TimeUtil;
import java.time.Duration;
import java.time.Instant;
import java.util.concurrent.TimeUnit;
import java.util.function.LongSupplier;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
@UseClockStep(clockStepUnit = TimeUnit.MINUTES)
public class AttentionSetIT extends AbstractDaemonTest {
/** Simulates a fake clock. Uses second granularity. */
private static class FakeClock implements LongSupplier {
Instant now = Instant.now();
@Override
public long getAsLong() {
return TimeUnit.SECONDS.toMillis(now.getEpochSecond());
}
Instant now() {
return Instant.ofEpochSecond(now.getEpochSecond());
}
void advance(Duration duration) {
now = now.plus(duration);
}
}
private FakeClock fakeClock = new FakeClock();
@Before
public void setUp() {
TimeUtil.setCurrentMillisSupplier(fakeClock);
}
@Test
public void emptyAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
assertThat(r.getChange().attentionSet()).isEmpty();
}
@Test
public void addUser() throws Exception {
PushOneCommit.Result r = createChange();
int accountId =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "first"))._accountId;
assertThat(accountId).isEqualTo(user.id().get());
AttentionSetUpdate expectedAttentionSetUpdate =
AttentionSetUpdate.createFromRead(
fakeClock.now(), user.id(), AttentionSetUpdate.Operation.ADD, "first");
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
// Second add is ignored.
accountId =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "second"))._accountId;
assertThat(accountId).isEqualTo(user.id().get());
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
}
@Test
public void addMultipleUsers() throws Exception {
PushOneCommit.Result r = createChange();
Instant timestamp1 = fakeClock.now();
int accountId1 =
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user"))._accountId;
assertThat(accountId1).isEqualTo(user.id().get());
fakeClock.advance(Duration.ofSeconds(42));
Instant timestamp2 = fakeClock.now();
int accountId2 =
change(r)
.addToAttentionSet(new AddToAttentionSetInput(admin.id().toString(), "admin"))
._accountId;
assertThat(accountId2).isEqualTo(admin.id().get());
AttentionSetUpdate expectedAttentionSetUpdate1 =
AttentionSetUpdate.createFromRead(
timestamp1, user.id(), AttentionSetUpdate.Operation.ADD, "user");
AttentionSetUpdate expectedAttentionSetUpdate2 =
AttentionSetUpdate.createFromRead(
timestamp2, admin.id(), AttentionSetUpdate.Operation.ADD, "admin");
assertThat(r.getChange().attentionSet())
.containsExactly(expectedAttentionSetUpdate1, expectedAttentionSetUpdate2);
}
@Test
public void removeUser() throws Exception {
PushOneCommit.Result r = createChange();
change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "added"));
fakeClock.advance(Duration.ofSeconds(42));
change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
AttentionSetUpdate expectedAttentionSetUpdate =
AttentionSetUpdate.createFromRead(
fakeClock.now(), user.id(), AttentionSetUpdate.Operation.REMOVE, "removed");
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
// Second removal is ignored.
fakeClock.advance(Duration.ofSeconds(42));
change(r)
.attention(user.id().toString())
.remove(new RemoveFromAttentionSetInput("removed again"));
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
}
@Test
public void removeUnrelatedUser() throws Exception {
PushOneCommit.Result r = createChange();
change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("foo"));
assertThat(r.getChange().attentionSet()).isEmpty();
}
}

13
javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java
View File

@ -226,7 +226,7 @@ public class HashtagsIT extends AbstractDaemonTest {
HashtagsInput input = new HashtagsInput();
input.add = Sets.newHashSet("tag3", "tag4");
input.remove = Sets.newHashSet("tag1");
gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
change(r).setHashtags(input);
assertThatGet(r).containsExactly("tag2", "tag3", "tag4");
assertMessage(r, "Hashtags added: tag3, tag4\nHashtag removed: tag1");
@ -235,7 +235,7 @@ public class HashtagsIT extends AbstractDaemonTest {
input = new HashtagsInput();
input.add = Sets.newHashSet("tag3", "tag4");
input.remove = Sets.newHashSet("tag3");
gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
change(r).setHashtags(input);
assertThatGet(r).containsExactly("tag1", "tag2", "tag4");
assertMessage(r, "Hashtag removed: tag3");
}
@ -271,19 +271,19 @@ public class HashtagsIT extends AbstractDaemonTest {
}
private IterableSubject assertThatGet(PushOneCommit.Result r) throws Exception {
return assertThat(gApi.changes().id(r.getChange().getId().get()).getHashtags());
return assertThat(change(r).getHashtags());
}
private void addHashtags(PushOneCommit.Result r, String... toAdd) throws Exception {
HashtagsInput input = new HashtagsInput();
input.add = Sets.newHashSet(toAdd);
gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
change(r).setHashtags(input);
}
private void removeHashtags(PushOneCommit.Result r, String... toRemove) throws Exception {
HashtagsInput input = new HashtagsInput();
input.remove = Sets.newHashSet(toRemove);
gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
change(r).setHashtags(input);
}
private void assertMessage(PushOneCommit.Result r, String expectedMessage) throws Exception {
@ -299,8 +299,7 @@ public class HashtagsIT extends AbstractDaemonTest {
}
private ChangeMessageInfo getLastMessage(PushOneCommit.Result r) throws Exception {
ChangeMessageInfo lastMessage =
Iterables.getLast(gApi.changes().id(r.getChange().getId().get()).get().messages, null);
ChangeMessageInfo lastMessage = Iterables.getLast(change(r).get().messages, null);
assertWithMessage(lastMessage.message).that(lastMessage).isNotNull();
return lastMessage;
}

28
javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
View File

@ -28,7 +28,7 @@ import com.google.common.collect.Iterables;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@ -45,7 +45,7 @@ import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionSetUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@ -600,31 +600,31 @@ public class ChangeNotesStateTest {
}
@Test
public void serializeAttentionUpdates() throws Exception {
public void serializeAttentionSetUpdates() throws Exception {
assertRoundTrip(
newBuilder()
.attentionUpdates(
.attentionSet(
ImmutableList.of(
AttentionStatus.createFromRead(
AttentionSetUpdate.createFromRead(
Instant.EPOCH.plusSeconds(23),
Account.id(1000),
AttentionStatus.Operation.ADD,
AttentionSetUpdate.Operation.ADD,
"reason 1"),
AttentionStatus.createFromRead(
AttentionSetUpdate.createFromRead(
Instant.EPOCH.plusSeconds(42),
Account.id(2000),
AttentionStatus.Operation.REMOVE,
AttentionSetUpdate.Operation.REMOVE,
"reason 2")))
.build(),
newProtoBuilder()
.addAttentionStatus(
AttentionStatusProto.newBuilder()
.addAttentionSetUpdate(
AttentionSetUpdateProto.newBuilder()
.setTimestampMillis(23_000) // epoch millis
.setAccount(1000)
.setOperation("ADD")
.setReason("reason 1"))
.addAttentionStatus(
AttentionStatusProto.newBuilder()
.addAttentionSetUpdate(
AttentionSetUpdateProto.newBuilder()
.setTimestampMillis(42_000) // epoch millis
.setAccount(2000)
.setOperation("REMOVE")
@ -789,8 +789,8 @@ public class ChangeNotesStateTest {
"reviewerUpdates",
new TypeLiteral<ImmutableList<ReviewerStatusUpdate>>() {}.getType())
.put(
"attentionUpdates",
new TypeLiteral<ImmutableList<AttentionStatus>>() {}.getType())
"attentionSet",
new TypeLiteral<ImmutableList<AttentionSetUpdate>>() {}.getType())
.put(
"assigneeUpdates",
new TypeLiteral<ImmutableList<AssigneeStatusUpdate>>() {}.getType())

71
javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@ -38,8 +38,8 @@ import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.AttentionStatus.Operation;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@ -694,51 +694,51 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void defaultAttentionSetIsEmpty() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.getAttentionUpdates()).isEmpty();
assertThat(notes.getAttentionSet()).isEmpty();
}
@Test
public void addAttentionStatus() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionStatus attentionStatus =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
AttentionSetUpdate attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
update.setAttentionSetUpdates(ImmutableList.of(attentionSetUpdate));
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
assertThat(notes.getAttentionSet()).containsExactly(addTimestamp(attentionSetUpdate, c));
}
@Test
public void filterLatestAttentionStatus() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionStatus attentionStatus =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
AttentionSetUpdate attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
update.setAttentionSetUpdates(ImmutableList.of(attentionSetUpdate));
update.commit();
update = newUpdate(c, changeOwner);
attentionStatus =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
update.setAttentionSetUpdates(ImmutableList.of(attentionSetUpdate));
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
assertThat(notes.getAttentionSet()).containsExactly(addTimestamp(attentionSetUpdate, c));
}
@Test
public void addAttentionStatus_rejectTimestamp() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionStatus attentionStatus =
AttentionStatus.createFromRead(
AttentionSetUpdate attentionSetUpdate =
AttentionSetUpdate.createFromRead(
Instant.now(), changeOwner.getAccountId(), Operation.ADD, "test");
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> update.setAttentionUpdates(ImmutableList.of(attentionStatus)));
() -> update.setAttentionSetUpdates(ImmutableList.of(attentionSetUpdate)));
assertThat(thrown).hasMessageThat().contains("must not specify timestamp for write");
}
@ -746,14 +746,16 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void addAttentionStatus_rejectMultiplePerUser() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionStatus attentionStatus0 =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
AttentionStatus attentionStatus1 =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
AttentionSetUpdate attentionSetUpdate0 =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
AttentionSetUpdate attentionSetUpdate1 =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1)));
() ->
update.setAttentionSetUpdates(
ImmutableList.of(attentionSetUpdate0, attentionSetUpdate1)));
assertThat(thrown)
.hasMessageThat()
.contains("must not specify multiple updates for single user");
@ -763,17 +765,18 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void addAttentionStatusForMultipleUsers() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionStatus attentionStatus0 =
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
AttentionStatus attentionStatus1 =
AttentionStatus.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
AttentionSetUpdate attentionSetUpdate0 =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
AttentionSetUpdate attentionSetUpdate1 =
AttentionSetUpdate.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1));
update.setAttentionSetUpdates(ImmutableList.of(attentionSetUpdate0, attentionSetUpdate1));
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getAttentionUpdates())
.containsExactly(addTimestamp(attentionStatus0, c), addTimestamp(attentionStatus1, c));
assertThat(notes.getAttentionSet())
.containsExactly(
addTimestamp(attentionSetUpdate0, c), addTimestamp(attentionSetUpdate1, c));
}
@Test
@ -3230,12 +3233,12 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
return tr.parseBody(commit);
}
private AttentionStatus addTimestamp(AttentionStatus attentionStatus, Change c) {
private AttentionSetUpdate addTimestamp(AttentionSetUpdate attentionSetUpdate, Change c) {
Timestamp timestamp = newNotes(c).getChange().getLastUpdatedOn();
return AttentionStatus.createFromRead(
return AttentionSetUpdate.createFromRead(
timestamp.toInstant(),
attentionStatus.account(),
attentionStatus.operation(),
attentionStatus.reason());
attentionSetUpdate.account(),
attentionSetUpdate.operation(),
attentionSetUpdate.reason());
}
}

10
proto/cache.proto
View File

@ -208,17 +208,17 @@ message ChangeNotesStateProto {
}
repeated AssigneeStatusUpdateProto assignee_update = 22;
// An update to the attention set of the change. See class AttentionStatus for
// context.
message AttentionStatusProto {
// An update to the attention set of the change. See class AttentionSetUpdate
// for context.
message AttentionSetUpdateProto {
// Epoch millis.
int64 timestamp_millis = 1;
int32 account = 2;
// Maps to enum AttentionStatus.Operation
// Maps to enum AttentionSetUpdate.Operation
string operation = 3;
string reason = 4;
}
repeated AttentionStatusProto attention_status = 23;
repeated AttentionSetUpdateProto attention_set_update = 23;
}
// Serialized form of com.google.gerrit.server.query.change.ConflictKey