Merge "Remove unused dbProvider field from IdentifiedUser"

This commit is contained in:
David Pursehouse
2016-05-24 04:52:58 +00:00
committed by Gerrit Code Review
17 changed files with 47 additions and 94 deletions

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)

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();

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");

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

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,14 +364,11 @@ public class IdentifiedUser extends CurrentUser {
user = user + "|" + "account-" + ua.getId().toString();
String host = null;
if (remotePeerProvider != null) {
final SocketAddress remotePeer = remotePeerProvider.get();
if (remotePeer instanceof InetSocketAddress) {
final InetSocketAddress sa = (InetSocketAddress) remotePeer;
final InetAddress in = sa.getAddress();
host = in != null ? getHost(in) : sa.getHostName();
}
SocketAddress remotePeer = remotePeerProvider.get();
if (remotePeer instanceof InetSocketAddress) {
InetSocketAddress sa = (InetSocketAddress) remotePeer;
InetAddress in = sa.getAddress();
host = in != null ? getHost(in) : sa.getHostName();
}
if (host == null || host.isEmpty()) {
host = "unknown";

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

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();

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

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())) {

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

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

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

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

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

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

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

View File

@@ -156,13 +156,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() {
@@ -1540,7 +1540,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);