Merge changes Ic063facd,Ie4feae69,I090bb11e,Iedb4bf4d,Ic0f2adfe, ...

* changes:
  Move general preferences from Account to AccountState
  Accounts: Load AccountState instead of Account
  ExternalIds: Allow to specify revision for getting ext IDs by account
  AccountConfig: Read revision of the external IDs branch
  Allow to update accounts and general preferences atomically
  ExternalIdNotes: Make constructor and load() method private
  Allow to update accounts and project watches atomically
This commit is contained in:
Edwin Kempin
2018-01-11 10:17:05 +00:00
committed by Gerrit Code Review
43 changed files with 948 additions and 734 deletions

View File

@@ -1334,11 +1334,10 @@ class ReceiveCommits {
this.publish = cmd.getRefName().startsWith(MagicBranch.NEW_PUBLISH_CHANGE);
this.labelTypes = labelTypes;
this.notesMigration = notesMigration;
GeneralPreferencesInfo prefs = user.getAccount().getGeneralPreferencesInfo();
GeneralPreferencesInfo prefs = user.state().getGeneralPreferences();
this.defaultPublishComments =
prefs != null
? firstNonNull(
user.getAccount().getGeneralPreferencesInfo().publishCommentsOnPush, false)
? firstNonNull(user.state().getGeneralPreferences().publishCommentsOnPush, false)
: false;
}

View File

@@ -14,11 +14,14 @@
package com.google.gerrit.server.git.validators;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountConfig;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -28,6 +31,7 @@ import java.util.List;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
public class AccountValidator {
@@ -42,20 +46,21 @@ public class AccountValidator {
}
public List<String> validate(
Account.Id accountId, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
Account.Id accountId, Repository repo, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
throws IOException {
Optional<Account> oldAccount = Optional.empty();
if (oldId != null && !ObjectId.zeroId().equals(oldId)) {
try {
oldAccount = loadAccount(accountId, rw, oldId);
oldAccount = loadAccount(accountId, repo, rw, oldId, null);
} catch (ConfigInvalidException e) {
// ignore, maybe the new commit is repairing it now
}
}
List<String> messages = new ArrayList<>();
Optional<Account> newAccount;
try {
newAccount = loadAccount(accountId, rw, newId);
newAccount = loadAccount(accountId, repo, rw, newId, messages);
} catch (ConfigInvalidException e) {
return ImmutableList.of(
String.format(
@@ -67,7 +72,6 @@ public class AccountValidator {
return ImmutableList.of(String.format("account '%s' does not exist", accountId.get()));
}
List<String> messages = new ArrayList<>();
if (accountId.equals(self.get().getAccountId()) && !newAccount.get().isActive()) {
messages.add("cannot deactivate own account");
}
@@ -87,11 +91,24 @@ public class AccountValidator {
return ImmutableList.copyOf(messages);
}
private Optional<Account> loadAccount(Account.Id accountId, RevWalk rw, ObjectId commit)
private Optional<Account> loadAccount(
Account.Id accountId,
Repository repo,
RevWalk rw,
ObjectId commit,
@Nullable List<String> messages)
throws IOException, ConfigInvalidException {
rw.reset();
AccountConfig accountConfig = new AccountConfig(accountId);
accountConfig.load(rw, commit);
AccountConfig accountConfig = new AccountConfig(accountId, repo);
accountConfig.setEagerParsing(true).load(rw, commit);
if (messages != null) {
messages.addAll(
accountConfig
.getValidationErrors()
.stream()
.map(ValidationError::getMessage)
.collect(toSet()));
}
return accountConfig.getLoadedAccount();
}
}

View File

@@ -34,7 +34,6 @@ import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.WatchConfig;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
@@ -42,6 +41,7 @@ import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -85,6 +85,7 @@ public class CommitValidators {
private final PersonIdent gerritIdent;
private final String canonicalWebUrl;
private final DynamicSet<CommitValidationListener> pluginValidators;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final AllProjectsName allProjects;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
@@ -98,6 +99,7 @@ public class CommitValidators {
@CanonicalWebUrl @Nullable String canonicalWebUrl,
@GerritServerConfig Config cfg,
DynamicSet<CommitValidationListener> pluginValidators,
GitRepositoryManager repoManager,
AllUsersName allUsers,
AllProjectsName allProjects,
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
@@ -106,6 +108,7 @@ public class CommitValidators {
this.gerritIdent = gerritIdent;
this.canonicalWebUrl = canonicalWebUrl;
this.pluginValidators = pluginValidators;
this.repoManager = repoManager;
this.allUsers = allUsers;
this.allProjects = allProjects;
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
@@ -138,7 +141,7 @@ public class CommitValidators {
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
new AccountCommitValidator(allUsers, accountValidator),
new AccountCommitValidator(repoManager, allUsers, accountValidator),
new GroupCommitValidator(allUsers)));
}
@@ -164,7 +167,7 @@ public class CommitValidators {
new ConfigValidator(branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
new AccountCommitValidator(allUsers, accountValidator),
new AccountCommitValidator(repoManager, allUsers, accountValidator),
new GroupCommitValidator(allUsers)));
}
@@ -418,34 +421,6 @@ public class CommitValidators {
}
}
if (allUsers.equals(branch.getParentKey()) && RefNames.isRefsUsers(branch.get())) {
List<CommitValidationMessage> messages = new ArrayList<>();
Account.Id accountId = Account.Id.fromRef(branch.get());
if (accountId != null) {
try {
WatchConfig wc = new WatchConfig(accountId);
wc.load(rw, receiveEvent.command.getNewId());
if (!wc.getValidationErrors().isEmpty()) {
addError("Invalid project configuration:", messages);
for (ValidationError err : wc.getValidationErrors()) {
addError(" " + err.getMessage(), messages);
}
throw new ConfigInvalidException("invalid watch configuration");
}
} catch (IOException | ConfigInvalidException e) {
log.error(
"User "
+ user.getUserName()
+ " tried to push an invalid watch configuration "
+ receiveEvent.command.getNewId().name()
+ " for account "
+ accountId.get(),
e);
throw new CommitValidationException("invalid watch configuration", messages);
}
}
}
return Collections.emptyList();
}
}
@@ -729,10 +704,15 @@ public class CommitValidators {
}
public static class AccountCommitValidator implements CommitValidationListener {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final AccountValidator accountValidator;
public AccountCommitValidator(AllUsersName allUsers, AccountValidator accountValidator) {
public AccountCommitValidator(
GitRepositoryManager repoManager,
AllUsersName allUsers,
AccountValidator accountValidator) {
this.repoManager = repoManager;
this.allUsers = allUsers;
this.accountValidator = accountValidator;
}
@@ -755,10 +735,11 @@ public class CommitValidators {
return Collections.emptyList();
}
try {
try (Repository repo = repoManager.openRepository(allUsers)) {
List<String> errorMessages =
accountValidator.validate(
accountId,
repo,
receiveEvent.revWalk,
receiveEvent.command.getOldId(),
receiveEvent.commit);

View File

@@ -285,7 +285,7 @@ public class MergeValidators {
}
try (RevWalk rw = new RevWalk(repo)) {
List<String> errorMessages = accountValidator.validate(accountId, rw, null, commit);
List<String> errorMessages = accountValidator.validate(accountId, repo, rw, null, commit);
if (!errorMessages.isEmpty()) {
throw new MergeValidationException(
"invalid account configuration: " + Joiner.on("; ").join(errorMessages));