opendev/gerrit
Code Issues Proposed changes

CommentDetail/PatchScriptFactory: Remove unused AccountInfoCache

Browse Source 
AccountInfoCache was populated but nobody used it.

This was the last usage of AccountInfoCache, hence it can be removed
now.

Change-Id: I429e3e10bcd3b0148a272ec45d62c3babc80be1f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-02-06 14:38:05 +01:00
parent 39648ab2c4
commit dde9fe9f64
5 changed files with 16 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
gerrit-common/src/main/java/com/google/gerrit/common/data/AccountInfoCache.java
View File

@@ -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);
}
}

19
gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java
View File

@@ -27,7 +27,6 @@ import java.util.Map;
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,23 @@ public class CommentDetail {
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)) {
} else if (idB.equals(psId)) {
b.add(p);
return true;
}
break;
}
return false;
}
public void setAccountInfoCache(final AccountInfoCache a) {
accounts = a;
}
public AccountInfoCache getAccounts() {
return accounts;
}
public List<Comment> getCommentsA() {

75
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfoCacheFactory.java
View File

@@ -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);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -77,7 +77,6 @@ import com.google.gerrit.server.Sequences;
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 @@ public class GerritGlobalModule extends FactoryModule {
bind(AccountResolver.class);
factory(AccountInfoCacheFactory.Factory.class);
factory(AddReviewerSender.Factory.class);
factory(DeleteReviewerSender.Factory.class);
factory(AddKeySender.Factory.class);

44
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
View File

@@ -33,7 +33,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
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 @@ public class PatchScriptFactory implements Callable<PatchScript> {
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 @@ public class PatchScriptFactory implements Callable<PatchScript> {
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ReviewDb db,
AccountInfoCacheFactory.Factory aicFactory,
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
@Assisted ChangeControl control,
@@ -131,7 +128,6 @@ public class PatchScriptFactory implements Callable<PatchScript> {
this.patchListCache = patchListCache;
this.db = db;
this.control = control;
this.aicFactory = aicFactory;
this.commentsUtil = commentsUtil;
this.editReader = editReader;
@@ -150,7 +146,6 @@ public class PatchScriptFactory implements Callable<PatchScript> {
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ReviewDb db,
AccountInfoCacheFactory.Factory aicFactory,
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
@Assisted ChangeControl control,
@@ -164,7 +159,6 @@ public class PatchScriptFactory implements Callable<PatchScript> {
this.patchListCache = patchListCache;
this.db = db;
this.control = control;
this.aicFactory = aicFactory;
this.commentsUtil = commentsUtil;
this.editReader = editReader;
@@ -344,24 +338,23 @@ public class PatchScriptFactory implements Callable<PatchScript> {
}
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 @@ public class PatchScriptFactory implements Callable<PatchScript> {
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 @@ public class PatchScriptFactory implements Callable<PatchScript> {
}
}
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);