Merge changes Iddd04aa3,Ica535e34,I19d7c6f0,I79433c5e,I1566242d

* changes:
  RefPatternMatcher#getUsernames: Remove premature optimization
  RefPatternMatcher#getUsernames: Return ImmutableSet
  IdentifiedUser#getEmailAddresses: Return ImmutableSet
  Add IdentifiedUser#getLoggableName()
  AccountManager#update: Improve logging when username is not changed/set
This commit is contained in:
Dave Borowitz
2018-01-23 18:07:05 +00:00
committed by Gerrit Code Review
5 changed files with 25 additions and 20 deletions

View File

@@ -301,6 +301,11 @@ public class IdentifiedUser extends CurrentUser {
return state().getUserName().orElse(null); return state().getUserName().orElse(null);
} }
/** @return unique name of the user for logging, never {@code null} */
public String getLoggableName() {
return getUserName() != null ? getUserName() : "a/" + getAccountId().get();
}
public Account getAccount() { public Account getAccount() {
return state().getAccount(); return state().getAccount();
} }
@@ -320,12 +325,12 @@ public class IdentifiedUser extends CurrentUser {
return false; return false;
} }
public Set<String> getEmailAddresses() { public ImmutableSet<String> getEmailAddresses() {
if (!loadedAllEmails) { if (!loadedAllEmails) {
validEmails.addAll(realm.getEmailAddresses(this)); validEmails.addAll(realm.getEmailAddresses(this));
loadedAllEmails = true; loadedAllEmails = true;
} }
return validEmails; return ImmutableSet.copyOf(validEmails);
} }
public String getName() { public String getName() {

View File

@@ -239,11 +239,14 @@ public class AccountManager {
} }
if (!realm.allowsEdit(AccountFieldName.USER_NAME) if (!realm.allowsEdit(AccountFieldName.USER_NAME)
&& who.getUserName() != null && !Strings.isNullOrEmpty(who.getUserName())
&& !Objects.equals(user.getUserName(), Strings.emptyToNull(who.getUserName()))) { && !who.getUserName().equals(user.getUserName())) {
log.warn( if (user.getUserName() != null) {
String.format( log.warn(
"Not changing already set username %s to %s", user.getUserName(), who.getUserName())); "Not changing already set username {} to {}", user.getUserName(), who.getUserName());
} else {
log.warn("Not setting username to {}", who.getUserName());
}
} }
if (!accountUpdates.isEmpty()) { if (!accountUpdates.isEmpty()) {

View File

@@ -918,7 +918,7 @@ class ReceiveCommits {
reject(cmd, "invalid project configuration"); reject(cmd, "invalid project configuration");
logError( logError(
"User " "User "
+ user.getUserName() + user.getLoggableName()
+ " tried to push invalid project configuration " + " tried to push invalid project configuration "
+ cmd.getNewId().name() + cmd.getNewId().name()
+ " for " + " for "
@@ -994,7 +994,7 @@ class ReceiveCommits {
reject(cmd, "invalid project configuration"); reject(cmd, "invalid project configuration");
logError( logError(
"User " "User "
+ user.getUserName() + user.getLoggableName()
+ " tried to push invalid project configuration " + " tried to push invalid project configuration "
+ cmd.getNewId().name() + cmd.getNewId().name()
+ " for " + " for "

View File

@@ -447,7 +447,7 @@ public class CommitValidators {
} catch (ConfigInvalidException | IOException e) { } catch (ConfigInvalidException | IOException e) {
log.error( log.error(
"User " "User "
+ user.getUserName() + user.getLoggableName()
+ " tried to push an invalid project configuration " + " tried to push an invalid project configuration "
+ receiveEvent.command.getNewId().name() + receiveEvent.command.getNewId().name()
+ " for project " + " for project "

View File

@@ -14,11 +14,12 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.project.RefPattern.isRE; import static com.google.gerrit.server.project.RefPattern.isRE;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Streams;
import com.google.gerrit.common.data.ParameterizedString; import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
@@ -26,7 +27,6 @@ import com.google.gerrit.server.CurrentUser;
import dk.brics.automaton.Automaton; import dk.brics.automaton.Automaton;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern; import java.util.regex.Pattern;
public abstract class RefPatternMatcher { public abstract class RefPatternMatcher {
@@ -132,15 +132,12 @@ public abstract class RefPatternMatcher {
return false; return false;
} }
private Iterable<String> getUsernames(CurrentUser user) { private ImmutableSet<String> getUsernames(CurrentUser user) {
if (user.isIdentifiedUser()) { if (user.isIdentifiedUser()) {
Set<String> emails = user.asIdentifiedUser().getEmailAddresses(); return Streams.concat(
if (user.getUserName() == null) { user.asIdentifiedUser().getEmailAddresses().stream(),
return emails; ImmutableSet.of(user.getUserName()).stream())
} else if (emails.isEmpty()) { .collect(toImmutableSet());
return ImmutableSet.of(user.getUserName());
}
return Iterables.concat(emails, ImmutableSet.of(user.getUserName()));
} }
if (user.getUserName() != null) { if (user.getUserName() != null) {
return ImmutableSet.of(user.getUserName()); return ImmutableSet.of(user.getUserName());