opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.14' into stable-2.15

Browse Source 
* stable-2.14:
  Add more debug logging to account creation
  Delete index after each test in elasticsearch tests

Change-Id: I65516500727e1fea53dce5ee41ca7f62de247632
This commit is contained in:
Hugo Arès
2018-04-24 14:08:55 -04:00
parent 4e34c1c036 679c04efe0
commit a7f8e5f27c
6 changed files with 83 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
View File

@@ -21,6 +21,7 @@ import com.google.inject.Guice;
import com.google.inject.Injector;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.lib.Config;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@@ -45,11 +46,22 @@ public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest {
}
}
private String testName() {
return testName.getMethodName().toLowerCase() + "_";
}
@After
public void cleanupIndex() {
if (nodeInfo != null) {
ElasticTestUtils.deleteAllIndexes(nodeInfo, testName());
}
}
@Override
protected Injector createInjector() {
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName.getMethodName().toLowerCase() + "_";
String indicesPrefix = testName();
ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix);
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));

14
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
View File

@@ -23,6 +23,7 @@ import com.google.inject.Injector;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -48,11 +49,22 @@ public class ElasticQueryChangesTest extends AbstractQueryChangesTest {
}
}
private String testName() {
return testName.getMethodName().toLowerCase() + "_";
}
@After
public void cleanupIndex() {
if (nodeInfo != null) {
ElasticTestUtils.deleteAllIndexes(nodeInfo, testName());
}
}
@Override
protected Injector createInjector() {
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName.getMethodName().toLowerCase() + "_";
String indicesPrefix = testName();
ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix);
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));

14
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java
View File

@@ -21,6 +21,7 @@ import com.google.inject.Guice;
import com.google.inject.Injector;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.lib.Config;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@@ -45,11 +46,22 @@ public class ElasticQueryGroupsTest extends AbstractQueryGroupsTest {
}
}
private String testName() {
return testName.getMethodName().toLowerCase() + "_";
}
@After
public void cleanupIndex() {
if (nodeInfo != null) {
ElasticTestUtils.deleteAllIndexes(nodeInfo, testName());
}
}
@Override
protected Injector createInjector() {
Config elasticsearchConfig = new Config(config);
InMemoryModule.setDefaults(elasticsearchConfig);
String indicesPrefix = testName.getMethodName().toLowerCase() + "_";
String indicesPrefix = testName();
ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix);
return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));

32
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java
View File

@@ -106,8 +106,36 @@ final class ElasticTestUtils {
return new ElasticNodeInfo(node, elasticDir, getHttpPort(node));
}
static void deleteAllIndexes(ElasticNodeInfo nodeInfo) {
nodeInfo.node.client().admin().indices().prepareDelete("_all").execute();
static void deleteAllIndexes(ElasticNodeInfo nodeInfo, String prefix) {
Schema<ChangeData> changeSchema = ChangeSchemaDefinitions.INSTANCE.getLatest();
nodeInfo
.node
.client()
.admin()
.indices()
.prepareDelete(String.format("%s%s_%04d", prefix, CHANGES, changeSchema.getVersion()))
.execute()
.actionGet();
Schema<AccountState> accountSchema = AccountSchemaDefinitions.INSTANCE.getLatest();
nodeInfo
.node
.client()
.admin()
.indices()
.prepareDelete(String.format("%s%s_%04d", prefix, ACCOUNTS, accountSchema.getVersion()))
.execute()
.actionGet();
Schema<InternalGroup> groupSchema = GroupSchemaDefinitions.INSTANCE.getLatest();
nodeInfo
.node
.client()
.admin()
.indices()
.prepareDelete(String.format("%s%s_%04d", prefix, GROUPS, groupSchema.getVersion()))
.execute()
.actionGet();
}
static class NodeInfo {

5
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@@ -262,9 +262,11 @@ public class AccountManager {
private AuthResult create(ReviewDb db, AuthRequest who)
throws OrmException, AccountException, IOException, ConfigInvalidException {
Account.Id newId = new Account.Id(sequences.nextAccountId());
log.debug("Assigning new Id {} to account", newId);
ExternalId extId =
ExternalId.createWithEmail(who.getExternalIdKey(), newId, who.getEmailAddress());
log.debug("Created external Id: {}", extId);
boolean isFirstAccount = awaitsFirstAccountCheck.getAndSet(false) && !accounts.hasAnyAccount();
@@ -322,11 +324,14 @@ public class AccountManager {
}
}
log.debug("Username from AuthRequest: {}", who.getUserName());
if (who.getUserName() != null) {
log.debug("Setting username for: {}", who.getUserName());
// Only set if the name hasn't been used yet, but was given to us.
//
try {
changeUserNameFactory.create(user, who.getUserName()).call();
log.debug("Identified user {} was created from {}", user, who.getUserName());
} catch (NameAlreadyUsedException e) {
String message =
"Cannot assign user name \""

11
gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
View File

@@ -34,13 +34,16 @@ import java.util.Collection;
import java.util.concurrent.Callable;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Operation to change the username of an account. */
public class ChangeUserName implements Callable<VoidResult> {
public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed.";
private static final Logger log = LoggerFactory.getLogger(ChangeUserName.class);
private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed.";
/** Generic factory to change any user's username. */
public interface Factory {
ChangeUserName create(IdentifiedUser user, String newUsername);
@@ -73,6 +76,9 @@ public class ChangeUserName implements Callable<VoidResult> {
ConfigInvalidException {
Collection<ExternalId> old = externalIds.byAccount(user.getAccountId(), SCHEME_USERNAME);
if (!old.isEmpty()) {
log.error(
"External id with scheme \"username:\" already exists for the user {}",
user.getAccountId());
throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED);
}
@@ -91,6 +97,7 @@ public class ChangeUserName implements Callable<VoidResult> {
}
}
externalIdsUpdate.insert(ExternalId.create(key, user.getAccountId(), null, password));
log.info("Created the new external Id with key: {}", key);
} catch (OrmDuplicateKeyException dupeErr) {
// If we are using this identity, don't report the exception.
//