From 076b5c8309bae6e9455c70b4060f59a602c94803 Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Tue, 13 Sep 2016 17:16:50 +0200 Subject: [PATCH] GET PUT DELETE Rest endpoints for change Assignee field changes//assignee GET returns an AccountInfo: { "_account_id": 1000000, "email": "user@gerrit.com", "username": "user" } PUT takes an account identifier: name-email, email, full name, "self" and returns an AccountInfo for the user that is assigned after the call. Change-Id: Icce1be90c6c9558950502fe24435317e8372bb64 --- .../extensions/api/changes/AssigneeInput.java | 22 +++ .../gerrit/extensions/common/ChangeInfo.java | 1 + .../gerrit/server/change/ChangeJson.java | 10 ++ .../gerrit/server/change/DeleteAssignee.java | 130 +++++++++++++++++ .../gerrit/server/change/GetAssignee.java | 48 +++++++ .../google/gerrit/server/change/Module.java | 4 + .../gerrit/server/change/PutAssignee.java | 60 ++++++++ .../gerrit/server/change/SetAssigneeOp.java | 136 ++++++++++++++++++ .../server/notedb/ChangeNotesParser.java | 13 +- .../gerrit/server/notedb/ChangeUpdate.java | 12 +- .../query/change/AssigneePredicate.java | 6 +- .../gerrit/server/notedb/ChangeNotesTest.java | 7 +- 12 files changed, 436 insertions(+), 13 deletions(-) create mode 100644 gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AssigneeInput.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteAssignee.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AssigneeInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AssigneeInput.java new file mode 100644 index 0000000000..61b5b85c26 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AssigneeInput.java @@ -0,0 +1,22 @@ +// Copyright (C) 2016 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; + +public class AssigneeInput { + @DefaultInput + public String assignee; +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java index 003ab245b1..ba22094fd1 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java @@ -28,6 +28,7 @@ public class ChangeInfo { public String project; public String branch; public String topic; + public AccountInfo assignee; public Collection hashtags; public String changeId; public String subject; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index d8078ca519..01eec38f79 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -101,6 +101,8 @@ import com.google.gerrit.server.api.accounts.GpgApiAdapter; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LabelNormalizer; import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.index.change.ChangeField; +import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.patch.PatchListNotAvailableException; @@ -171,6 +173,7 @@ public class ChangeJson { private final ChangeNotes.Factory notesFactory; private final ChangeResource.Factory changeResourceFactory; private final ChangeKindCache changeKindCache; + private final ChangeIndexCollection indexes; private boolean lazyLoad = true; private AccountLoader accountLoader; @@ -201,6 +204,7 @@ public class ChangeJson { ChangeNotes.Factory notesFactory, ChangeResource.Factory changeResourceFactory, ChangeKindCache changeKindCache, + ChangeIndexCollection indexes, @Assisted Set options) { this.db = db; this.labelNormalizer = ln; @@ -223,6 +227,7 @@ public class ChangeJson { this.notesFactory = notesFactory; this.changeResourceFactory = changeResourceFactory; this.changeKindCache = changeKindCache; + this.indexes = indexes; this.options = options.isEmpty() ? EnumSet.noneOf(ListChangesOption.class) : EnumSet.copyOf(options); @@ -436,6 +441,11 @@ public class ChangeJson { out.project = in.getProject().get(); out.branch = in.getDest().getShortName(); out.topic = in.getTopic(); + if (indexes.getSearchIndex().getSchema().hasField(ChangeField.ASSIGNEE)) { + if (cd.assignee().isPresent()) { + out.assignee = accountLoader.get(cd.assignee().get()); + } + } out.hashtags = cd.hashtags(); out.changeId = in.getKey().get(); if (in.getStatus().isOpen()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteAssignee.java new file mode 100644 index 0000000000..00e2ad39f6 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteAssignee.java @@ -0,0 +1,130 @@ +// Copyright (C) 2016 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.common.base.Optional; +import com.google.gerrit.common.TimeUtil; +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.RestApiException; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.ChangeMessage; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeMessagesUtil; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.change.DeleteAssignee.Input; +import com.google.gerrit.server.account.AccountInfoCacheFactory; +import com.google.gerrit.server.account.AccountJson; +import com.google.gerrit.server.config.AnonymousCowardName; +import com.google.gerrit.server.git.BatchUpdate; +import com.google.gerrit.server.git.BatchUpdate.ChangeContext; +import com.google.gerrit.server.git.UpdateException; +import com.google.gerrit.server.notedb.ChangeUpdate; +import com.google.gerrit.server.notedb.NotesMigration; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +@Singleton +public class DeleteAssignee implements + RestModifyView { + public static class Input { + + } + private BatchUpdate.Factory batchUpdateFactory; + private final NotesMigration notesMigration; + private final ChangeMessagesUtil cmUtil; + private final Provider db; + private final AccountInfoCacheFactory.Factory accountInfos; + private final String anonymousCowardName; + + @Inject + DeleteAssignee(NotesMigration notesMigration, + BatchUpdate.Factory batchUpdateFactory, + ChangeMessagesUtil cmUtil, + Provider db, + AccountInfoCacheFactory.Factory accountInfosFactory, + @AnonymousCowardName String anonymousCowardName) { + this.batchUpdateFactory = batchUpdateFactory; + this.notesMigration = notesMigration; + this.cmUtil = cmUtil; + this.db = db; + this.accountInfos = accountInfosFactory; + this.anonymousCowardName = anonymousCowardName; + } + + @Override + public Object apply(ChangeResource rsrc, Input input) + throws RestApiException, UpdateException + { + try (BatchUpdate bu = batchUpdateFactory.create(db.get(), + rsrc.getProject(), + rsrc.getUser(), TimeUtil.nowTs())) { + Op op = new Op(); + bu.addOp(rsrc.getChange().getId(), op); + bu.execute(); + if (op.getDeletedAssignee() == null) { + return Response.none(); + } + return Response.ok(AccountJson.toAccountInfo(op.getDeletedAssignee())); + } + } + + private class Op extends BatchUpdate.Op { + private Account deletedAssignee; + + @Override + public boolean updateChange(ChangeContext ctx) + throws RestApiException, OrmException{ + if (!notesMigration.readChanges()) { + throw new BadRequestException( + "Cannot add Assignee; NoteDb is disabled"); + } + if (!ctx.getControl().canEditAssignee()) { + throw new AuthException("Delete Assignee not permitted"); + } + ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId()); + Optional currentAssigneeId = update.getNotes().getAssignee(); + if (!currentAssigneeId.isPresent()) { + return false; + } + Account account = accountInfos.create().get(currentAssigneeId.get()); + update.setAssignee(Optional.absent()); + addMessage(ctx, update, account); + deletedAssignee = account; + return true; + } + + public Account getDeletedAssignee() { + return deletedAssignee; + } + + private void addMessage(BatchUpdate.ChangeContext ctx, + ChangeUpdate update, Account deleted) throws OrmException { + ChangeMessage cmsg = new ChangeMessage( + new ChangeMessage.Key( + ctx.getChange().getId(), + ChangeUtil.messageUUID(ctx.getDb())), + ctx.getAccountId(), ctx.getWhen(), + ctx.getChange().currentPatchSetId()); + cmsg.setMessage( + "Assignee deleted: " + deleted.getName(anonymousCowardName)); + cmUtil.addChangeMessage(ctx.getDb(), update, cmsg); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java new file mode 100644 index 0000000000..6c7ac980db --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java @@ -0,0 +1,48 @@ +// Copyright (C) 2016 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.common.base.Optional; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.server.account.AccountInfoCacheFactory; +import com.google.gerrit.server.account.AccountJson; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Singleton; + +@Singleton +public class GetAssignee implements RestReadView { + private final AccountInfoCacheFactory.Factory accountInfo; + + @Inject + GetAssignee(AccountInfoCacheFactory.Factory accountInfo) { + this.accountInfo = accountInfo; + } + + @Override + public Response apply(ChangeResource rsrc) throws OrmException { + + Optional assignee = + rsrc.getControl().getNotes().load().getAssignee(); + if (assignee.isPresent()) { + Account account = accountInfo.create().get(assignee.get()); + return Response.ok(AccountJson.toAccountInfo(account)); + } + return Response.none(); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index c2793dbd72..1dad241bc4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -53,6 +53,9 @@ public class Module extends RestApiModule { get(CHANGE_KIND, "detail").to(GetDetail.class); get(CHANGE_KIND, "topic").to(GetTopic.class); get(CHANGE_KIND, "in").to(IncludedIn.class); + get(CHANGE_KIND, "assignee").to(GetAssignee.class); + put(CHANGE_KIND, "assignee").to(PutAssignee.class); + delete(CHANGE_KIND, "assignee").to(DeleteAssignee.class); get(CHANGE_KIND, "hashtags").to(GetHashtags.class); get(CHANGE_KIND, "comments").to(ListChangeComments.class); get(CHANGE_KIND, "drafts").to(ListChangeDrafts.class); @@ -139,6 +142,7 @@ public class Module extends RestApiModule { factory(PatchSetInserter.Factory.class); factory(RebaseChangeOp.Factory.class); factory(ReviewerResource.Factory.class); + factory(SetAssigneeOp.Factory.class); factory(SetHashtagsOp.Factory.class); factory(ChangeResource.Factory.class); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java new file mode 100644 index 0000000000..2df0d16aeb --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java @@ -0,0 +1,60 @@ +// Copyright (C) 2016 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.common.TimeUtil; +import com.google.gerrit.extensions.api.changes.AssigneeInput; +import com.google.gerrit.extensions.common.AccountInfo; +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.reviewdb.server.ReviewDb; +import com.google.gerrit.server.account.AccountJson; +import com.google.gerrit.server.git.BatchUpdate; +import com.google.gerrit.server.git.UpdateException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +@Singleton +public class PutAssignee + implements RestModifyView { + + private SetAssigneeOp.Factory assigneeFactory; + private BatchUpdate.Factory batchUpdateFactory; + private Provider db; + + @Inject + PutAssignee(SetAssigneeOp.Factory assigneeFactory, + BatchUpdate.Factory batchUpdateFactory, + Provider db) { + this.assigneeFactory = assigneeFactory; + this.batchUpdateFactory = batchUpdateFactory; + this.db = db; + } + + @Override + public Response apply(ChangeResource rsrc, AssigneeInput input) + throws RestApiException, UpdateException { + try (BatchUpdate bu = batchUpdateFactory.create(db.get(), + rsrc.getChange().getProject(), rsrc.getControl().getUser(), + TimeUtil.nowTs())) { + SetAssigneeOp op = assigneeFactory.create(input); + bu.addOp(rsrc.getId(), op); + bu.execute(); + return Response.ok(AccountJson.toAccountInfo(op.getNewAssignee())); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java new file mode 100644 index 0000000000..f6f9ee0b84 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java @@ -0,0 +1,136 @@ +// Copyright (C) 2016 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.common.base.Optional; +import com.google.gerrit.extensions.api.changes.AssigneeInput; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.ChangeMessage; +import com.google.gerrit.server.ChangeMessagesUtil; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountInfoCacheFactory; +import com.google.gerrit.server.account.AccountsCollection; +import com.google.gerrit.server.config.AnonymousCowardName; +import com.google.gerrit.server.git.BatchUpdate; +import com.google.gerrit.server.notedb.ChangeUpdate; +import com.google.gerrit.server.notedb.NotesMigration; +import com.google.gwtorm.server.OrmException; +import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; + +public class SetAssigneeOp extends BatchUpdate.Op { + public interface Factory { + SetAssigneeOp create(AssigneeInput input); + } + + private final AssigneeInput input; + private final AccountsCollection accounts; + private final ChangeMessagesUtil cmUtil; + private final AccountInfoCacheFactory.Factory accountInfosFactory; + private final NotesMigration notesMigration; + private final String anonymousCowardName; + private Account newAssignee; + + @AssistedInject + SetAssigneeOp(AccountsCollection accounts, + NotesMigration notesMigration, + ChangeMessagesUtil cmUtil, + AccountInfoCacheFactory.Factory accountInfosFactory, + @AnonymousCowardName String anonymousCowardName, + @Assisted AssigneeInput input) { + this.accounts = accounts; + this.notesMigration = notesMigration; + this.cmUtil = cmUtil; + this.accountInfosFactory = accountInfosFactory; + this.anonymousCowardName = anonymousCowardName; + this.input = input; + } + + @Override + public boolean updateChange(BatchUpdate.ChangeContext ctx) + throws OrmException, RestApiException { + if (!notesMigration.readChanges()) { + throw new BadRequestException( + "Cannot add Assignee; NoteDb is disabled"); + } + if (!ctx.getControl().canEditAssignee()) { + throw new AuthException("Changing Assignee not permitted"); + } + ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId()); + Optional oldAssigneeId = update.getNotes().getAssignee(); + if (input.assignee == null) { + if (oldAssigneeId != null && oldAssigneeId.isPresent()) { + throw new AuthException("Cannot set Assignee to empty"); + } + return false; + } + Account oldAssignee = null; + if (oldAssigneeId != null && oldAssigneeId.isPresent()) { + oldAssignee = accountInfosFactory.create().get(oldAssigneeId.get()); + } + IdentifiedUser newAssigneeUser = accounts.parse(input.assignee); + if (oldAssigneeId != null && + oldAssigneeId.equals(newAssigneeUser.getAccountId())) { + newAssignee = oldAssignee; + return false; + } + if (!newAssigneeUser.getAccount().isActive()) { + throw new UnprocessableEntityException(String.format( + "Account of %s is not active", newAssigneeUser.getUserName())); + } + if (!ctx.getControl().forUser(newAssigneeUser).isRefVisible()) { + throw new AuthException(String.format( + "Change %s is not visible to %s.", + ctx.getChange().getChangeId(), + newAssigneeUser.getUserName())); + } + update.setAssignee(Optional.fromNullable(newAssigneeUser.getAccountId())); + this.newAssignee = newAssigneeUser.getAccount(); + addMessage(ctx, update, oldAssignee); + return true; + } + + private void addMessage(BatchUpdate.ChangeContext ctx, ChangeUpdate update, + Account previousAssignee) throws OrmException { + StringBuilder msg = new StringBuilder(); + msg.append("Assignee "); + if (previousAssignee == null) { + msg.append("added: "); + msg.append(newAssignee.getName(anonymousCowardName)); + } else { + msg.append("changed from: "); + msg.append(previousAssignee.getName(anonymousCowardName)); + msg.append(" to: "); + msg.append(newAssignee.getName(anonymousCowardName)); + } + ChangeMessage cmsg = new ChangeMessage( + new ChangeMessage.Key( + ctx.getChange().getId(), + ChangeUtil.messageUUID(ctx.getDb())), + ctx.getAccountId(), ctx.getWhen(), + ctx.getChange().currentPatchSetId()); + cmsg.setMessage(msg.toString()); + cmUtil.addChangeMessage(ctx.getDb(), update, cmsg); + } + + public Account getNewAssignee() { + return newAssignee; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index e3afe633bc..89d7e8a0b4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -137,7 +137,7 @@ class ChangeNotesParser { private String branch; private Change.Status status; private String topic; - private Account.Id assignee; + private Optional assignee; private Set hashtags; private Timestamp createdOn; private Timestamp lastUpdatedOn; @@ -212,7 +212,7 @@ class ChangeNotesParser { submissionId, status, - assignee, + assignee != null ? assignee.orNull() : null, hashtags, patchSets, buildApprovals(), @@ -484,9 +484,14 @@ class ChangeNotesParser { return; } String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE); - if (assigneeValue != null) { + if (assigneeValue == null){ + //footer not found + } else if (assigneeValue.equals("")) { + // empty footer found, assignee deleted + assignee = Optional.absent(); + } else { PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue); - assignee = noteUtil.parseIdent(ident, id); + assignee = Optional.fromNullable(noteUtil.parseIdent(ident, id)); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index 08b617e6ee..88b167affa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -125,7 +125,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { private String submissionId; private String topic; private String commit; - private Account.Id assignee; + private Optional assignee; private Set hashtags; private String changeMessage; private String tag; @@ -378,7 +378,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { this.hashtags = hashtags; } - public void setAssignee(Account.Id assignee) { + public void setAssignee(Optional assignee) { this.assignee = assignee; } @@ -551,8 +551,12 @@ public class ChangeUpdate extends AbstractChangeUpdate { } if (assignee != null) { - addFooter(msg, FOOTER_ASSIGNEE); - addIdent(msg, assignee).append('\n'); + if (assignee.isPresent()) { + addFooter(msg, FOOTER_ASSIGNEE); + addIdent(msg, assignee.get()).append('\n'); + } else { + addFooter(msg, FOOTER_ASSIGNEE).append('\n'); + } } Joiner comma = Joiner.on(','); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AssigneePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AssigneePredicate.java index f8fa8c5e85..0b92cd8cd5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AssigneePredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AssigneePredicate.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.query.change; +import com.google.common.base.Optional; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.index.change.ChangeField; import com.google.gwtorm.server.OrmException; @@ -29,9 +30,10 @@ class AssigneePredicate extends ChangeIndexPredicate { @Override public boolean match(final ChangeData object) throws OrmException { if (id.get() == ChangeField.NO_ASSIGNEE) { - return object.notes().load().getAssignee() == null; + Optional assignee = object.notes().load().getAssignee(); + return assignee == null || !assignee.isPresent(); } - return id.equals(object.notes().load().getAssignee()); + return id.equals(object.notes().load().getAssignee().get()); } @Override diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 0b04ee9a19..62a682e06e 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -24,6 +24,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import static org.junit.Assert.fail; +import com.google.common.base.Optional; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; @@ -565,7 +566,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { public void assigneeCommit() throws Exception { Change c = newChange(); ChangeUpdate update = newUpdate(c, changeOwner); - update.setAssignee(otherUserId); + update.setAssignee(Optional.fromNullable(otherUserId)); ObjectId result = update.commit(); assertThat(result).isNotNull(); try (RevWalk rw = new RevWalk(repo)) { @@ -587,14 +588,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { public void assigneeChangeNotes() throws Exception { Change c = newChange(); ChangeUpdate update = newUpdate(c, changeOwner); - update.setAssignee(otherUserId); + update.setAssignee(Optional.fromNullable(otherUserId)); update.commit(); ChangeNotes notes = newNotes(c); assertThat(notes.getAssignee().get()).isEqualTo(otherUserId); update = newUpdate(c, changeOwner); - update.setAssignee(changeOwner.getAccountId()); + update.setAssignee(Optional.fromNullable(changeOwner.getAccountId())); update.commit(); notes = newNotes(c);