opendev/gerrit
Code Issues Proposed changes

Remove unused dbProvider field from IdentifiedUser

Browse Source 
This field is also no longer used in some factory methods, which means
many callers need to get updated as well. One side effect is to
consolidate many of the GenericFactory#create methods. This in turn
means that the remote peer provider is never null; previously the code
was treating a null Provider and a Provider that returns null
identically, so this should have no behavior change.

Change-Id: I107fcfbd1a1d8daa15ee3afc02c8e7e709ffb3ea
This commit is contained in:
Andrew Bonventre
2016-05-16 12:07:42 -07:00
committed by Dave Borowitz
parent 1766703b05
commit 0f8b4118b7
17 changed files with 47 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -81,7 +81,6 @@ import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.util.Providers;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -532,7 +531,7 @@ public abstract class AbstractDaemonTest {
private Context newRequestContext(TestAccount account) {
return atrScope.newContext(reviewDbProvider, new SshSession(server, account),
identifiedUserFactory.create(Providers.of(db), account.getId()));
identifiedUserFactory.create(account.getId()));
}
protected Context setApiUser(TestAccount account) {
@@ -717,7 +716,7 @@ public abstract class AbstractDaemonTest {
}
protected IdentifiedUser user(TestAccount testAccount) {
return identifiedUserFactory.create(Providers.of(db), testAccount.getId());
return identifiedUserFactory.create(testAccount.getId());
}
protected RevisionResource parseCurrentRevisionResource(String changeId)

3
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
View File

@@ -38,7 +38,6 @@ import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.testutil.DisabledReviewDb;
import com.google.inject.Inject;
import com.google.inject.util.Providers;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -272,7 +271,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
AcceptanceTestRequestScope.Context ctx = disableDb();
try (Repository repo = repoManager.openRepository(project)) {
ProjectControl ctl = projectControlFactory.controlFor(project,
identifiedUserFactory.create(Providers.of(db), user.getId()));
identifiedUserFactory.create(user.getId()));
VisibleRefFilter filter = new VisibleRefFilter(
tagCache, changeCache, repo, ctl, new DisabledReviewDb(), true);
Map<String, Ref> all = repo.getAllRefs();

2
gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
View File

@@ -169,7 +169,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
if (extId == null) {
return CheckResult.bad("Key is not associated with any users");
}
IdentifiedUser user = userFactory.create(db, extId.getAccountId());
IdentifiedUser user = userFactory.create(extId.getAccountId());
Set<String> allowedUserIds = getAllowedUserIds(user);
if (allowedUserIds.isEmpty()) {
return CheckResult.bad("No identities found for user");

4
gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
View File

@@ -151,12 +151,12 @@ public class GerritPublicKeyCheckerTest {
private IdentifiedUser addUser(String name) throws Exception {
AuthRequest req = AuthRequest.forUser(name);
Account.Id id = accountManager.authenticate(req).getAccountId();
return userFactory.create(Providers.of(db), id);
return userFactory.create(id);
}
private IdentifiedUser reloadUser() {
accountCache.evict(userId);
user = userFactory.create(Providers.of(db), userId);
user = userFactory.create(userId);
return user;
}

61
gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
View File

@@ -20,7 +20,6 @@ import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.CapabilityControl;
@@ -90,29 +89,20 @@ public class IdentifiedUser extends CurrentUser {
this.disableReverseDnsLookup = disableReverseDnsLookup;
}
public IdentifiedUser create(final Account.Id id) {
public IdentifiedUser create(Account.Id id) {
return create((SocketAddress) null, id);
}
public IdentifiedUser create(Provider<ReviewDb> db, Account.Id id) {
return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
groupBackend, disableReverseDnsLookup, null, db, id, null);
}
public IdentifiedUser create(SocketAddress remotePeer, Account.Id id) {
return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), null,
id, null);
return runAs(remotePeer, id, null);
}
public CurrentUser runAs(SocketAddress remotePeer, Account.Id id,
public IdentifiedUser runAs(SocketAddress remotePeer, Account.Id id,
@Nullable CurrentUser caller) {
return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), null,
id, caller);
groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), id,
caller);
}
}
@@ -133,23 +123,20 @@ public class IdentifiedUser extends CurrentUser {
private final AccountCache accountCache;
private final GroupBackend groupBackend;
private final Boolean disableReverseDnsLookup;
private final Provider<SocketAddress> remotePeerProvider;
private final Provider<ReviewDb> dbProvider;
@Inject
RequestFactory(
CapabilityControl.Factory capabilityControlFactory,
@Nullable StarredChangesUtil starredChangesUtil,
final AuthConfig authConfig,
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName final String anonymousCowardName,
@CanonicalWebUrl final Provider<String> canonicalUrl,
final AccountCache accountCache,
final GroupBackend groupBackend,
@DisableReverseDnsLookup final Boolean disableReverseDnsLookup,
@RemotePeer final Provider<SocketAddress> remotePeerProvider,
final Provider<ReviewDb> dbProvider) {
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@DisableReverseDnsLookup Boolean disableReverseDnsLookup,
@RemotePeer Provider<SocketAddress> remotePeerProvider) {
this.capabilityControlFactory = capabilityControlFactory;
this.starredChangesUtil = starredChangesUtil;
this.authConfig = authConfig;
@@ -160,21 +147,19 @@ public class IdentifiedUser extends CurrentUser {
this.groupBackend = groupBackend;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
this.dbProvider = dbProvider;
}
public IdentifiedUser create(Account.Id id) {
return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
groupBackend, disableReverseDnsLookup, remotePeerProvider, dbProvider,
id, null);
groupBackend, disableReverseDnsLookup, remotePeerProvider, id, null);
}
public IdentifiedUser runAs(Account.Id id, CurrentUser caller) {
return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
groupBackend, disableReverseDnsLookup, remotePeerProvider, dbProvider,
id, caller);
groupBackend, disableReverseDnsLookup, remotePeerProvider, id,
caller);
}
}
@@ -196,12 +181,7 @@ public class IdentifiedUser extends CurrentUser {
private final Set<String> validEmails =
Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
@Nullable
private final Provider<SocketAddress> remotePeerProvider;
@Nullable
private final Provider<ReviewDb> dbProvider;
private final Account.Id accountId;
private AccountState state;
@@ -224,7 +204,6 @@ public class IdentifiedUser extends CurrentUser {
final GroupBackend groupBackend,
final Boolean disableReverseDnsLookup,
@Nullable final Provider<SocketAddress> remotePeerProvider,
@Nullable final Provider<ReviewDb> dbProvider,
final Account.Id id,
@Nullable CurrentUser realUser) {
super(capabilityControlFactory);
@@ -237,7 +216,6 @@ public class IdentifiedUser extends CurrentUser {
this.anonymousCowardName = anonymousCowardName;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
this.dbProvider = dbProvider;
this.accountId = id;
this.realUser = realUser != null ? realUser : this;
}
@@ -386,15 +364,12 @@ public class IdentifiedUser extends CurrentUser {
user = user + "|" + "account-" + ua.getId().toString();
String host = null;
if (remotePeerProvider != null) {
final SocketAddress remotePeer = remotePeerProvider.get();
SocketAddress remotePeer = remotePeerProvider.get();
if (remotePeer instanceof InetSocketAddress) {
final InetSocketAddress sa = (InetSocketAddress) remotePeer;
final InetAddress in = sa.getAddress();
InetSocketAddress sa = (InetSocketAddress) remotePeer;
InetAddress in = sa.getAddress();
host = in != null ? getHost(in) : sa.getHostName();
}
}
if (host == null || host.isEmpty()) {
host = "unknown";
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -967,7 +967,7 @@ public class ChangeJson {
if (in.getPushCertificate() != null) {
out.pushCertificate = gpgApi.checkPushCertificate(
in.getPushCertificate(),
userFactory.create(db, in.getUploader()));
userFactory.create(in.getUploader()));
} else {
out.pushCertificate = new PushCertificateInfo();
}

3
gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
View File

@@ -64,8 +64,7 @@ public class SuggestChangeReviewers extends SuggestReviewers
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
IdentifiedUser who =
identifiedUserFactory.create(dbProvider, account);
IdentifiedUser who = identifiedUserFactory.create(account);
// we can't use changeControl directly as it won't suggest reviewers
// to drafts
return rsrc.getControl().forUser(who).isRefVisible();

3
gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java
View File

@@ -112,8 +112,7 @@ public class EmailMerge implements Runnable, RequestContext {
@Override
public CurrentUser getUser() {
if (submitter != null) {
return identifiedUserFactory.create(
getReviewDbProvider(), submitter).getRealUser();
return identifiedUserFactory.create(submitter).getRealUser();
}
throw new OutOfScopeException("No user on email thread");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java
View File

@@ -174,8 +174,7 @@ public class ProjectWatch {
private boolean add(Watchers matching, AccountProjectWatch w, NotifyType type)
throws OrmException {
IdentifiedUser user =
args.identifiedUserFactory.create(args.db, w.getAccountId());
IdentifiedUser user = args.identifiedUserFactory.create(w.getAccountId());
try {
if (filterMatch(user, w.getFilter())) {

8
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -301,7 +301,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
} catch (ProvisionException e) {
// Doesn't match current user, continue.
}
return asUser(userFactory.create(db, otherId));
return asUser(userFactory.create(otherId));
}
IdentifiedUser getIdentifiedUser() throws QueryParseException {
@@ -736,7 +736,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
if (!m.isEmpty()) {
List<Predicate<ChangeData>> p = Lists.newArrayListWithCapacity(m.size());
for (Account.Id id : m) {
return visibleto(args.userFactory.create(args.db, id));
return visibleto(args.userFactory.create(id));
}
return Predicate.or(p);
}
@@ -791,7 +791,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
if (g == null) {
throw error("Group " + group + " not found");
}
return new OwnerinPredicate(args.db, args.userFactory, g.getUUID());
return new OwnerinPredicate(args.userFactory, g.getUUID());
}
@Operator
@@ -818,7 +818,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
if (g == null) {
throw error("Group " + group + " not found");
}
return new ReviewerinPredicate(args.db, args.userFactory, g.getUUID());
return new ReviewerinPredicate(args.userFactory, g.getUUID());
}
@Operator

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
View File

@@ -112,7 +112,7 @@ class EqualsLabelPredicate extends IndexPredicate<ChangeData> {
if (psVal == expVal) {
// Double check the value is still permitted for the user.
//
IdentifiedUser reviewer = userFactory.create(dbProvider, approver);
IdentifiedUser reviewer = userFactory.create(approver);
try {
ChangeControl cc =
ccFactory.controlFor(dbProvider.get(), change, reviewer);

11
gerrit-server/src/main/java/com/google/gerrit/server/query/change/OwnerinPredicate.java
View File

@@ -16,21 +16,17 @@ package com.google.gerrit.server.query.change;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.query.OperatorPredicate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
class OwnerinPredicate extends OperatorPredicate<ChangeData> {
private final Provider<ReviewDb> dbProvider;
private final IdentifiedUser.GenericFactory userFactory;
private final AccountGroup.UUID uuid;
OwnerinPredicate(Provider<ReviewDb> dbProvider,
IdentifiedUser.GenericFactory userFactory, AccountGroup.UUID uuid) {
OwnerinPredicate(IdentifiedUser.GenericFactory userFactory,
AccountGroup.UUID uuid) {
super(ChangeQueryBuilder.FIELD_OWNERIN, uuid.toString());
this.dbProvider = dbProvider;
this.userFactory = userFactory;
this.uuid = uuid;
}
@@ -45,8 +41,7 @@ class OwnerinPredicate extends OperatorPredicate<ChangeData> {
if (change == null) {
return false;
}
final IdentifiedUser owner = userFactory.create(dbProvider,
change.getOwner());
final IdentifiedUser owner = userFactory.create(change.getOwner());
return owner.getEffectiveGroups().contains(uuid);
}

10
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerinPredicate.java
View File

@@ -16,21 +16,17 @@ package com.google.gerrit.server.query.change;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.query.OperatorPredicate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
class ReviewerinPredicate extends OperatorPredicate<ChangeData> {
private final Provider<ReviewDb> dbProvider;
private final IdentifiedUser.GenericFactory userFactory;
private final AccountGroup.UUID uuid;
ReviewerinPredicate(Provider<ReviewDb> dbProvider,
IdentifiedUser.GenericFactory userFactory, AccountGroup.UUID uuid) {
ReviewerinPredicate(IdentifiedUser.GenericFactory userFactory,
AccountGroup.UUID uuid) {
super(ChangeQueryBuilder.FIELD_REVIEWERIN, uuid.toString());
this.dbProvider = dbProvider;
this.userFactory = userFactory;
this.uuid = uuid;
}
@@ -42,7 +38,7 @@ class ReviewerinPredicate extends OperatorPredicate<ChangeData> {
@Override
public boolean match(final ChangeData object) throws OrmException {
for (Account.Id accountId : object.reviewers().values()) {
IdentifiedUser reviewer = userFactory.create(dbProvider, accountId);
IdentifiedUser reviewer = userFactory.create(accountId);
if (reviewer.getEffectiveGroups().contains(uuid)) {
return true;
}

10
gerrit-server/src/main/java/gerrit/PRED_current_user_2.java
View File

@@ -17,12 +17,10 @@ package gerrit;
import static com.googlecode.prolog_cafe.lang.SymbolTerm.intern;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.PrologEnvironment;
import com.google.gerrit.rules.StoredValues;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.inject.util.Providers;
import com.googlecode.prolog_cafe.exceptions.IllegalTypeException;
import com.googlecode.prolog_cafe.exceptions.PInstantiationException;
@@ -90,14 +88,8 @@ class PRED_current_user_2 extends Predicate.P2 {
Account.Id accountId = new Account.Id(((IntegerTerm) idTerm).intValue());
user = cache.get(accountId);
if (user == null) {
ReviewDb db = StoredValues.REVIEW_DB.getOrNull(engine);
IdentifiedUser.GenericFactory userFactory = userFactory(engine);
IdentifiedUser who;
if (db != null) {
who = userFactory.create(Providers.of(db), accountId);
} else {
who = userFactory.create(accountId);
}
IdentifiedUser who = userFactory.create(accountId);
cache.put(accountId, who);
user = who;
}

2
gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java
View File

@@ -90,7 +90,7 @@ public class LabelNormalizerTest {
schemaCreator.create(db);
userId = accountManager.authenticate(AuthRequest.forUser("user"))
.getAccountId();
user = userFactory.create(Providers.of(db), userId);
user = userFactory.create(userId);
requestContext.setContext(new RequestContext() {
@Override

2
gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
View File

@@ -79,7 +79,7 @@ public class ProjectControlTest {
schemaCreator.create(db);
Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user"))
.getAccountId();
user = userFactory.create(Providers.of(db), userId);
user = userFactory.create(userId);
Project.NameKey name = new Project.NameKey("project");
InMemoryRepository inMemoryRepo = repoManager.createRepository(name);

6
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -154,13 +154,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Account userAccount = db.accounts().get(userId);
userAccount.setPreferredEmail("user@example.com");
db.accounts().update(ImmutableList.of(userAccount));
user = userFactory.create(Providers.of(db), userId);
user = userFactory.create(userId);
requestContext.setContext(newRequestContext(userAccount.getId()));
}
protected RequestContext newRequestContext(Account.Id requestUserId) {
final CurrentUser requestUser =
userFactory.create(Providers.of(db), requestUserId);
userFactory.create(requestUserId);
return new RequestContext() {
@Override
public CurrentUser getUser() {
@@ -1522,7 +1522,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Project.NameKey project = new Project.NameKey(
repo.getRepository().getDescription().getRepositoryName());
Account.Id ownerId = owner != null ? owner : userId;
IdentifiedUser user = userFactory.create(Providers.of(db), ownerId);
IdentifiedUser user = userFactory.create(ownerId);
try (BatchUpdate bu =
updateFactory.create(db, project, user, TimeUtil.nowTs())) {
bu.insertChange(ins);