Merge changes Ice480cab,I429e3e10,I4379e493,I592312ed,I69e6d993, ...
* changes:
CommentDetail: Rewrite switch statement in include method with if + else
CommentDetail/PatchScriptFactory: Remove unused AccountInfoCache
EventUtil: Inline AccountJson
Assignee endpoints: Use AccountLoader to populate AccountInfo
Anotate GetPastAssignees as singleton
Assignee endpoints: Use IdentifiedUser#getNameEmail() to format user
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountInfoCache.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountInfoCache.java
deleted file mode 100644
index d7803c1..0000000
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountInfoCache.java
+++ /dev/null
@@ -1,79 +0,0 @@
-// Copyright (C) 2008 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.common.data;
-
-import com.google.gerrit.reviewdb.client.Account;
-
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-/** In-memory table of {@link AccountInfo}, indexed by {@code Account.Id}. */
-public class AccountInfoCache {
- private static final AccountInfoCache EMPTY;
- static {
- EMPTY = new AccountInfoCache();
- EMPTY.accounts = Collections.emptyMap();
- }
-
- /** Obtain an empty cache singleton. */
- public static AccountInfoCache empty() {
- return EMPTY;
- }
-
- protected Map<Account.Id, AccountInfo> accounts;
-
- protected AccountInfoCache() {
- }
-
- public AccountInfoCache(final Iterable<AccountInfo> list) {
- accounts = new HashMap<>();
- for (final AccountInfo ai : list) {
- accounts.put(ai.getId(), ai);
- }
- }
-
- /**
- * Lookup the account summary
- * <p>
- * The return value can take on one of three forms:
- * <ul>
- * <li>{@code null}, if {@code id == null}.</li>
- * <li>a valid info block, if {@code id} was loaded.</li>
- * <li>an anonymous info block, if {@code id} was not loaded.</li>
- * </ul>
- *
- * @param id the id desired.
- * @return info block for the account.
- */
- public AccountInfo get(final Account.Id id) {
- if (id == null) {
- return null;
- }
-
- AccountInfo r = accounts.get(id);
- if (r == null) {
- r = new AccountInfo(id);
- accounts.put(id, r);
- }
- return r;
- }
-
- /** Merge the information from another cache into this one. */
- public void merge(final AccountInfoCache other) {
- assert this != EMPTY;
- accounts.putAll(other.accounts);
- }
-}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java
index ff5402f..fa282ca 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java
@@ -27,7 +27,6 @@
public class CommentDetail {
protected List<Comment> a;
protected List<Comment> b;
- protected AccountInfoCache accounts;
private transient PatchSet.Id idA;
private transient PatchSet.Id idB;
@@ -44,37 +43,19 @@
protected CommentDetail() {
}
- public boolean include(Change.Id changeId, Comment p) {
+ public void include(Change.Id changeId, Comment p) {
PatchSet.Id psId = new PatchSet.Id(changeId, p.key.patchSetId);
- switch (p.side) {
- case 0:
- if (idA == null && idB.equals(psId)) {
- a.add(p);
- return true;
- }
- break;
-
- case 1:
- if (idA != null && idA.equals(psId)) {
- a.add(p);
- return true;
- }
-
- if (idB.equals(psId)) {
- b.add(p);
- return true;
- }
- break;
+ if (p.side == 0) {
+ if (idA == null && idB.equals(psId)) {
+ a.add(p);
+ }
+ } else if (p.side == 1) {
+ if (idA != null && idA.equals(psId)) {
+ a.add(p);
+ } else if (idB.equals(psId)) {
+ b.add(p);
+ }
}
- return false;
- }
-
- public void setAccountInfoCache(final AccountInfoCache a) {
- accounts = a;
- }
-
- public AccountInfoCache getAccounts() {
- return accounts;
}
public List<Comment> getCommentsA() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfoCacheFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfoCacheFactory.java
deleted file mode 100644
index 32781f0..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfoCacheFactory.java
+++ /dev/null
@@ -1,75 +0,0 @@
-// Copyright (C) 2009 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.account;
-
-import com.google.gerrit.common.data.AccountInfo;
-import com.google.gerrit.common.data.AccountInfoCache;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.inject.Inject;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-/** Efficiently builds an {@link AccountInfoCache}. */
-public class AccountInfoCacheFactory {
- public interface Factory {
- AccountInfoCacheFactory create();
- }
-
- private final AccountCache accountCache;
- private final Map<Account.Id, Account> out;
-
- @Inject
- AccountInfoCacheFactory(final AccountCache accountCache) {
- this.accountCache = accountCache;
- this.out = new HashMap<>();
- }
-
- /**
- * Indicate an account will be needed later on.
- *
- * @param id identity that will be needed in the future; may be null.
- */
- public void want(final Account.Id id) {
- if (id != null && !out.containsKey(id)) {
- out.put(id, accountCache.get(id).getAccount());
- }
- }
-
- /** Indicate one or more accounts will be needed later on. */
- public void want(final Iterable<Account.Id> ids) {
- for (final Account.Id id : ids) {
- want(id);
- }
- }
-
- public Account get(final Account.Id id) {
- want(id);
- return out.get(id);
- }
-
- /**
- * Create an AccountInfoCache with the currently loaded Account entities.
- * */
- public AccountInfoCache create() {
- final List<AccountInfo> r = new ArrayList<>(out.size());
- for (final Account a : out.values()) {
- r.add(new AccountInfo(a));
- }
- return new AccountInfoCache(r);
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountJson.java
deleted file mode 100644
index 7193564..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountJson.java
+++ /dev/null
@@ -1,33 +0,0 @@
-// 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.account;
-
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.reviewdb.client.Account;
-
-public class AccountJson {
-
- public static AccountInfo toAccountInfo(Account account) {
- if (account == null || account.getId() == null) {
- return null;
- }
- AccountInfo accountInfo = new AccountInfo(account.getId().get());
- accountInfo.email = account.getPreferredEmail();
- accountInfo.name = account.getFullName();
- accountInfo.username = account.getUserName();
- return accountInfo;
- }
-
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
index 420e631..1ddc762 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
@@ -107,4 +107,10 @@
}
fill();
}
+
+ public AccountInfo fillOne(Account.Id id) throws OrmException {
+ AccountInfo info = get(id);
+ fill();
+ return info;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 8275e8d..2a4b5ca 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -488,7 +488,7 @@
try {
Response<AccountInfo> r = deleteAssignee.apply(change, null);
return r.isNone() ? null : r.value();
- } catch (UpdateException e) {
+ } catch (UpdateException | OrmException e) {
throw new RestApiException("Cannot delete assignee", e);
}
}
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
index c5343e6..4d5b739 100644
--- 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
@@ -25,10 +25,9 @@
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.account.AccountInfoCacheFactory;
-import com.google.gerrit.server.account.AccountJson;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.DeleteAssignee.Input;
-import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.extensions.events.AssigneeChanged;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
@@ -48,38 +47,39 @@
private final BatchUpdate.Factory batchUpdateFactory;
private final ChangeMessagesUtil cmUtil;
private final Provider<ReviewDb> db;
- private final AccountInfoCacheFactory.Factory accountInfos;
private final AssigneeChanged assigneeChanged;
- private final String anonymousCowardName;
+ private final IdentifiedUser.GenericFactory userFactory;
+ private final AccountLoader.Factory accountLoaderFactory;
@Inject
DeleteAssignee(BatchUpdate.Factory batchUpdateFactory,
ChangeMessagesUtil cmUtil,
Provider<ReviewDb> db,
- AccountInfoCacheFactory.Factory accountInfosFactory,
AssigneeChanged assigneeChanged,
- @AnonymousCowardName String anonymousCowardName) {
+ IdentifiedUser.GenericFactory userFactory,
+ AccountLoader.Factory accountLoaderFactory) {
this.batchUpdateFactory = batchUpdateFactory;
this.cmUtil = cmUtil;
this.db = db;
- this.accountInfos = accountInfosFactory;
this.assigneeChanged = assigneeChanged;
- this.anonymousCowardName = anonymousCowardName;
+ this.userFactory = userFactory;
+ this.accountLoaderFactory = accountLoaderFactory;
}
@Override
public Response<AccountInfo> apply(ChangeResource rsrc, Input input)
- throws RestApiException, UpdateException {
+ throws RestApiException, UpdateException, OrmException {
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();
- Account deletedAssignee = op.getDeletedAssignee();
+ Account.Id deletedAssignee = op.getDeletedAssignee();
return deletedAssignee == null
? Response.none()
- : Response.ok(AccountJson.toAccountInfo(deletedAssignee));
+ : Response.ok(accountLoaderFactory.create(true)
+ .fillOne(deletedAssignee));
}
}
@@ -99,30 +99,33 @@
if (currentAssigneeId == null) {
return false;
}
- deletedAssignee = accountInfos.create().get(currentAssigneeId);
+ IdentifiedUser deletedAssigneeUser =
+ userFactory.create(currentAssigneeId);
+ deletedAssignee = deletedAssigneeUser.getAccount();
// noteDb
update.removeAssignee();
// reviewDb
change.setAssignee(null);
- addMessage(ctx, update, deletedAssignee);
+ addMessage(ctx, update, deletedAssigneeUser);
return true;
}
- public Account getDeletedAssignee() {
- return deletedAssignee;
+ public Account.Id getDeletedAssignee() {
+ return deletedAssignee != null ? deletedAssignee.getId() : null;
}
- private void addMessage(BatchUpdate.ChangeContext ctx,
- ChangeUpdate update, Account deleted) throws OrmException {
+ private void addMessage(BatchUpdate.ChangeContext ctx, ChangeUpdate update,
+ IdentifiedUser deletedAssignee) throws OrmException {
ChangeMessage cmsg = ChangeMessagesUtil.newMessage(
- ctx, "Assignee deleted: " + deleted.getName(anonymousCowardName),
+ ctx, "Assignee deleted: " + deletedAssignee.getNameEmail(),
ChangeMessagesUtil.TAG_DELETE_ASSIGNEE);
cmUtil.addChangeMessage(ctx.getDb(), update, cmsg);
}
@Override
public void postUpdate(Context ctx) throws OrmException {
- assigneeChanged.fire(change, ctx.getAccount(), deletedAssignee, ctx.getWhen());
+ assigneeChanged.fire(change, ctx.getAccount(), deletedAssignee,
+ ctx.getWhen());
}
}
}
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
index 5ad259b..ea81ad3 100644
--- 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
@@ -18,8 +18,7 @@
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.gerrit.server.account.AccountLoader;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -28,11 +27,11 @@
@Singleton
public class GetAssignee implements RestReadView<ChangeResource> {
- private final AccountInfoCacheFactory.Factory accountInfo;
+ private final AccountLoader.Factory accountLoaderFactory;
@Inject
- GetAssignee(AccountInfoCacheFactory.Factory accountInfo) {
- this.accountInfo = accountInfo;
+ GetAssignee(AccountLoader.Factory accountLoaderFactory) {
+ this.accountLoaderFactory = accountLoaderFactory;
}
@Override
@@ -40,8 +39,8 @@
Optional<Account.Id> assignee =
Optional.ofNullable(rsrc.getChange().getAssignee());
if (assignee.isPresent()) {
- Account account = accountInfo.create().get(assignee.get());
- return Response.ok(AccountJson.toAccountInfo(account));
+ return Response.ok(
+ accountLoaderFactory.create(true).fillOne(assignee.get()));
}
return Response.none();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPastAssignees.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPastAssignees.java
index fa9c0e8..c37efed 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPastAssignees.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPastAssignees.java
@@ -20,21 +20,22 @@
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.gerrit.server.account.AccountLoader;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import java.util.Collections;
import java.util.List;
import java.util.Set;
+@Singleton
public class GetPastAssignees implements RestReadView<ChangeResource> {
- private final AccountInfoCacheFactory.Factory accountInfos;
+ private final AccountLoader.Factory accountLoaderFactory;
@Inject
- GetPastAssignees(AccountInfoCacheFactory.Factory accountInfosFactory) {
- this.accountInfos = accountInfosFactory;
+ GetPastAssignees(AccountLoader.Factory accountLoaderFactory) {
+ this.accountLoaderFactory = accountLoaderFactory;
}
@Override
@@ -46,11 +47,11 @@
if (pastAssignees == null) {
return Response.ok(Collections.emptyList());
}
- AccountInfoCacheFactory accountInfoFactory = accountInfos.create();
- return Response.ok(pastAssignees.stream()
- .map(accountInfoFactory::get)
- .map(AccountJson::toAccountInfo)
- .collect(toList()));
+ AccountLoader accountLoader = accountLoaderFactory.create(true);
+ List<AccountInfo> infos =
+ pastAssignees.stream().map(accountLoader::get).collect(toList());
+ accountLoader.fill();
+ return Response.ok(infos);
}
}
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
index 271fd33..8afb0e6 100644
--- 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
@@ -27,7 +27,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.account.AccountJson;
+import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.PostReviewers.Addition;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.UpdateException;
@@ -46,16 +46,19 @@
private final BatchUpdate.Factory batchUpdateFactory;
private final Provider<ReviewDb> db;
private final PostReviewers postReviewers;
+ private final AccountLoader.Factory accountLoaderFactory;
@Inject
PutAssignee(SetAssigneeOp.Factory assigneeFactory,
BatchUpdate.Factory batchUpdateFactory,
Provider<ReviewDb> db,
- PostReviewers postReviewers) {
+ PostReviewers postReviewers,
+ AccountLoader.Factory accountLoaderFactory) {
this.assigneeFactory = assigneeFactory;
this.batchUpdateFactory = batchUpdateFactory;
this.db = db;
this.postReviewers = postReviewers;
+ this.accountLoaderFactory = accountLoaderFactory;
}
@Override
@@ -79,7 +82,8 @@
bu.addOp(rsrc.getId(), reviewersAddition.op);
bu.execute();
- return Response.ok(AccountJson.toAccountInfo(op.getNewAssignee()));
+ return Response.ok(
+ accountLoaderFactory.create(true).fillOne(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
index 47323c2..f687400 100644
--- 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
@@ -26,9 +26,7 @@
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.server.ChangeMessagesUtil;
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.extensions.events.AssigneeChanged;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.Context;
@@ -44,8 +42,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.Optional;
-
public class SetAssigneeOp extends BatchUpdate.Op {
private static final Logger log =
LoggerFactory.getLogger(SetAssigneeOp.class);
@@ -56,13 +52,12 @@
private final AccountsCollection accounts;
private final ChangeMessagesUtil cmUtil;
- private final AccountInfoCacheFactory.Factory accountInfosFactory;
private final DynamicSet<AssigneeValidationListener> validationListeners;
private final String assignee;
- private final String anonymousCowardName;
private final AssigneeChanged assigneeChanged;
private final SetAssigneeSender.Factory setAssigneeSenderFactory;
private final Provider<IdentifiedUser> user;
+ private final IdentifiedUser.GenericFactory userFactory;
private Change change;
private Account newAssignee;
@@ -71,22 +66,20 @@
@AssistedInject
SetAssigneeOp(AccountsCollection accounts,
ChangeMessagesUtil cmUtil,
- AccountInfoCacheFactory.Factory accountInfosFactory,
DynamicSet<AssigneeValidationListener> validationListeners,
- @AnonymousCowardName String anonymousCowardName,
AssigneeChanged assigneeChanged,
SetAssigneeSender.Factory setAssigneeSenderFactory,
Provider<IdentifiedUser> user,
+ IdentifiedUser.GenericFactory userFactory,
@Assisted String assignee) {
this.accounts = accounts;
this.cmUtil = cmUtil;
- this.accountInfosFactory = accountInfosFactory;
this.validationListeners = validationListeners;
this.assigneeChanged = assigneeChanged;
- this.anonymousCowardName = anonymousCowardName;
- this.assignee = checkNotNull(assignee);
this.setAssigneeSenderFactory = setAssigneeSenderFactory;
this.user = user;
+ this.userFactory = userFactory;
+ this.assignee = checkNotNull(assignee);
}
@Override
@@ -94,19 +87,17 @@
throws OrmException, RestApiException {
change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
- Optional<Account.Id> oldAssigneeId =
- Optional.ofNullable(change.getAssignee());
- oldAssignee = null;
- if (oldAssigneeId.isPresent()) {
- oldAssignee = accountInfosFactory.create().get(oldAssigneeId.get());
- }
IdentifiedUser newAssigneeUser = accounts.parse(assignee);
- if (oldAssigneeId.isPresent() &&
- oldAssigneeId.get().equals(newAssigneeUser.getAccountId())) {
- newAssignee = oldAssignee;
- return false;
+ newAssignee = newAssigneeUser.getAccount();
+ IdentifiedUser oldAssigneeUser = null;
+ if (change.getAssignee() != null) {
+ oldAssigneeUser = userFactory.create(change.getAssignee());
+ oldAssignee = oldAssigneeUser.getAccount();
+ if (newAssignee.equals(oldAssignee)) {
+ return false;
+ }
}
- if (!newAssigneeUser.getAccount().isActive()) {
+ if (!newAssignee.isActive()) {
throw new UnprocessableEntityException(String.format(
"Account of %s is not active", assignee));
}
@@ -118,32 +109,32 @@
}
try {
for (AssigneeValidationListener validator : validationListeners) {
- validator.validateAssignee(change, newAssigneeUser.getAccount());
+ validator.validateAssignee(change, newAssignee);
}
} catch (ValidationException e) {
throw new ResourceConflictException(e.getMessage());
}
// notedb
- update.setAssignee(newAssigneeUser.getAccountId());
+ update.setAssignee(newAssignee.getId());
// reviewdb
- change.setAssignee(newAssigneeUser.getAccountId());
- this.newAssignee = newAssigneeUser.getAccount();
- addMessage(ctx, update, oldAssignee);
+ change.setAssignee(newAssignee.getId());
+ addMessage(ctx, update, oldAssigneeUser, newAssigneeUser);
return true;
}
private void addMessage(BatchUpdate.ChangeContext ctx, ChangeUpdate update,
- Account previousAssignee) throws OrmException {
+ IdentifiedUser previousAssignee, IdentifiedUser newAssignee)
+ throws OrmException {
StringBuilder msg = new StringBuilder();
msg.append("Assignee ");
if (previousAssignee == null) {
msg.append("added: ");
- msg.append(newAssignee.getName(anonymousCowardName));
+ msg.append(newAssignee.getNameEmail());
} else {
msg.append("changed from: ");
- msg.append(previousAssignee.getName(anonymousCowardName));
+ msg.append(previousAssignee.getNameEmail());
msg.append(" to: ");
- msg.append(newAssignee.getName(anonymousCowardName));
+ msg.append(newAssignee.getNameEmail());
}
ChangeMessage cmsg = ChangeMessagesUtil.newMessage(ctx, msg.toString(),
ChangeMessagesUtil.TAG_SET_ASSIGNEE);
@@ -164,7 +155,7 @@
assigneeChanged.fire(change, ctx.getAccount(), oldAssignee, ctx.getWhen());
}
- public Account getNewAssignee() {
- return newAssignee;
+ public Account.Id getNewAssignee() {
+ return newAssignee != null ? newAssignee.getId() : null;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 5c8a251..e941de0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -77,7 +77,6 @@
import com.google.gerrit.server.account.AccountByEmailCacheImpl;
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.AccountControl;
-import com.google.gerrit.server.account.AccountInfoCacheFactory;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountVisibility;
@@ -241,7 +240,6 @@
bind(AccountResolver.class);
- factory(AccountInfoCacheFactory.Factory.class);
factory(AddReviewerSender.Factory.class);
factory(DeleteReviewerSender.Factory.class);
factory(AddKeySender.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 162a6b1..a267802 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -25,7 +25,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GpgException;
-import com.google.gerrit.server.account.AccountJson;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -84,7 +83,11 @@
if (a == null || a.getId() == null) {
return null;
}
- return AccountJson.toAccountInfo(a);
+ AccountInfo accountInfo = new AccountInfo(a.getId().get());
+ accountInfo.email = a.getPreferredEmail();
+ accountInfo.name = a.getFullName();
+ accountInfo.username = a.getUserName();
+ return accountInfo;
}
public Map<String, ApprovalInfo> approvals(Account a,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index cf48516..44b3966 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -33,7 +33,6 @@
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.account.AccountInfoCacheFactory;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -87,7 +86,6 @@
private final Provider<PatchScriptBuilder> builderFactory;
private final PatchListCache patchListCache;
private final ReviewDb db;
- private final AccountInfoCacheFactory.Factory aicFactory;
private final CommentsUtil commentsUtil;
private final String fileName;
@@ -117,7 +115,6 @@
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ReviewDb db,
- AccountInfoCacheFactory.Factory aicFactory,
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
@Assisted ChangeControl control,
@@ -131,7 +128,6 @@
this.patchListCache = patchListCache;
this.db = db;
this.control = control;
- this.aicFactory = aicFactory;
this.commentsUtil = commentsUtil;
this.editReader = editReader;
@@ -150,7 +146,6 @@
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ReviewDb db,
- AccountInfoCacheFactory.Factory aicFactory,
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
@Assisted ChangeControl control,
@@ -164,7 +159,6 @@
this.patchListCache = patchListCache;
this.db = db;
this.control = control;
- this.aicFactory = aicFactory;
this.commentsUtil = commentsUtil;
this.editReader = editReader;
@@ -344,24 +338,23 @@
}
if (loadComments && edit == null) {
- AccountInfoCacheFactory aic = aicFactory.create();
comments = new CommentDetail(psa, psb);
switch (changeType) {
case ADDED:
case MODIFIED:
- loadPublished(byKey, aic, newName);
+ loadPublished(byKey, newName);
break;
case DELETED:
- loadPublished(byKey, aic, newName);
+ loadPublished(byKey, newName);
break;
case COPIED:
case RENAMED:
if (psa != null) {
- loadPublished(byKey, aic, oldName);
+ loadPublished(byKey, oldName);
}
- loadPublished(byKey, aic, newName);
+ loadPublished(byKey, newName);
break;
case REWRITE:
@@ -374,38 +367,33 @@
switch (changeType) {
case ADDED:
case MODIFIED:
- loadDrafts(byKey, aic, me, newName);
+ loadDrafts(byKey, me, newName);
break;
case DELETED:
- loadDrafts(byKey, aic, me, newName);
+ loadDrafts(byKey, me, newName);
break;
case COPIED:
case RENAMED:
if (psa != null) {
- loadDrafts(byKey, aic, me, oldName);
+ loadDrafts(byKey, me, oldName);
}
- loadDrafts(byKey, aic, me, newName);
+ loadDrafts(byKey, me, newName);
break;
case REWRITE:
break;
}
}
-
- comments.setAccountInfoCache(aic.create());
}
}
- private void loadPublished(final Map<Patch.Key, Patch> byKey,
- final AccountInfoCacheFactory aic, final String file) throws OrmException {
+ private void loadPublished(Map<Patch.Key, Patch> byKey, String file)
+ throws OrmException {
ChangeNotes notes = control.getNotes();
for (Comment c : commentsUtil.publishedByChangeFile(db, notes, changeId, file)) {
- if (comments.include(change.getId(), c)) {
- aic.want(c.author.getId());
- }
-
+ comments.include(change.getId(), c);
PatchSet.Id psId = new PatchSet.Id(change.getId(), c.key.patchSetId);
Patch.Key pKey = new Patch.Key(psId, c.key.filename);
Patch p = byKey.get(pKey);
@@ -415,15 +403,11 @@
}
}
- private void loadDrafts(final Map<Patch.Key, Patch> byKey,
- final AccountInfoCacheFactory aic, final Account.Id me, final String file)
- throws OrmException {
+ private void loadDrafts(Map<Patch.Key, Patch> byKey, Account.Id me,
+ String file) throws OrmException {
for (Comment c :
commentsUtil.draftByChangeFileAuthor(db, control.getNotes(), file, me)) {
- if (comments.include(change.getId(), c)) {
- aic.want(me);
- }
-
+ comments.include(change.getId(), c);
PatchSet.Id psId = new PatchSet.Id(change.getId(), c.key.patchSetId);
Patch.Key pKey = new Patch.Key(psId, c.key.filename);
Patch p = byKey.get(pKey);
