opendev
/
gerrit
Code Issues Proposed changes

Merge branch 'stable-2.14' into stable-2.15 

* stable-2.14:
  ProjectLevelConfig: Import Collectors.toList static
  Merge parent config values with child values
  Set version to 2.14.8-SNAPSHOT
  Fix the missing DB entry in Gerrit DB
  Fix unit test "addGroupAsReviewersWhenANotPerfectMatchedUserExist"

Change-Id: Iae2f6b4dfb8bebb2a817485330b92a2b7b8b8548
This commit is contained in:
David Pursehouse 2018-03-14 22:01:19 +09:00
parent 1e0559eabc 67d78639fc
commit 4470d21fbe
5 changed files with 185 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@ -1560,16 +1560,17 @@ public class ChangeIT extends AbstractDaemonTest {
String oldETag = rsrc.getETag();
Timestamp oldTs = rsrc.getChange().getLastUpdatedOn();
//create a group named "us" with one user: testUser
TestAccount testUser = accountCreator.create("testUser", "testUser@test.com", "testUser");
String testGroup =
createGroupWithRealName(user.fullName.substring(0, user.fullName.length() / 2));
//create a group named "kobe" with one user: lee
TestAccount testUser = accountCreator.create("kobebryant", "kobebryant@test.com", "kobebryant");
TestAccount myGroupUser = accountCreator.create("lee", "lee@test.com", "lee");
String testGroup = createGroupWithRealName("kobe");
GroupApi groupApi = gApi.groups().id(testGroup);
groupApi.description("test group");
groupApi.addMembers(testUser.fullName);
groupApi.addMembers(myGroupUser.fullName);
//ensure that user "user" is not in the group
groupApi.removeMembers(user.fullName);
groupApi.removeMembers(testUser.fullName);
AddReviewerInput in = new AddReviewerInput();
in.reviewer = testGroup;
@ -1578,11 +1579,11 @@ public class ChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(testUser.emailAddress);
assertThat(m.body()).contains("Hello " + testUser.fullName + ",\n");
assertThat(m.rcpt()).containsExactly(myGroupUser.emailAddress);
assertThat(m.body()).contains("Hello " + myGroupUser.fullName + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, testUser.email);
assertMailReplyTo(m, myGroupUser.email);
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
// When NoteDb is enabled adding a reviewer records that user as reviewer
@ -1592,7 +1593,7 @@ public class ChangeIT extends AbstractDaemonTest {
Collection<AccountInfo> reviewers = c.reviewers.get(REVIEWER);
assertThat(reviewers).isNotNull();
assertThat(reviewers).hasSize(1);
assertThat(reviewers.iterator().next()._accountId).isEqualTo(testUser.getId().get());
assertThat(reviewers.iterator().next()._accountId).isEqualTo(myGroupUser.getId().get());
// Ensure ETag and lastUpdatedOn are updated.
rsrc = parseResource(r);

64
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.project.ProjectState;
import java.util.Arrays;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
@ -114,4 +115,67 @@ public class ProjectLevelConfigIT extends AbstractDaemonTest {
assertThat(state.getConfig(configName).get().toText()).isEqualTo(cfg.toText());
}
@Test
public void withMergedInheritance() throws Exception {
String configName = "test.config";
Config parentCfg = new Config();
parentCfg.setString("s1", null, "k1", "parentValue1");
parentCfg.setString("s1", null, "k2", "parentValue2");
parentCfg.setString("s2", "ss", "k3", "parentValue3");
parentCfg.setString("s2", "ss", "k4", "parentValue4");
pushFactory
.create(
db,
admin.getIdent(),
testRepo,
"Create Project Level Config",
configName,
parentCfg.toText())
.to(RefNames.REFS_CONFIG)
.assertOkStatus();
Project.NameKey childProject = createProject("child", project);
TestRepository<?> childTestRepo = cloneProject(childProject);
fetch(childTestRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
childTestRepo.reset("refs/heads/config");
Config cfg = new Config();
cfg.setString("s1", null, "k1", "parentValue1");
cfg.setString("s1", null, "k2", "parentValue2");
cfg.setString("s2", "ss", "k3", "parentValue3");
cfg.setString("s2", "ss", "k4", "parentValue4");
cfg.setString("s1", null, "k1", "childValue1");
cfg.setString("s2", "ss", "k3", "childValue2");
cfg.setString("s3", null, "k5", "childValue3");
cfg.setString("s3", "ss", "k6", "childValue4");
pushFactory
.create(
db,
admin.getIdent(),
childTestRepo,
"Create Project Level Config",
configName,
cfg.toText())
.to(RefNames.REFS_CONFIG)
.assertOkStatus();
ProjectState state = projectCache.get(childProject);
Config expectedCfg = new Config();
expectedCfg.setStringList("s1", null, "k1", Arrays.asList("childValue1", "parentValue1"));
expectedCfg.setString("s1", null, "k2", "parentValue2");
expectedCfg.setStringList("s2", "ss", "k3", Arrays.asList("childValue2", "parentValue3"));
expectedCfg.setString("s2", "ss", "k4", "parentValue4");
expectedCfg.setString("s3", null, "k5", "childValue3");
expectedCfg.setString("s3", "ss", "k6", "childValue4");
assertThat(state.getConfig(configName).getWithInheritance(true).toText())
.isEqualTo(expectedCfg.toText());
assertThat(state.getConfig(configName).get().toText()).isEqualTo(cfg.toText());
}
}

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

@ -14,6 +14,8 @@
package com.google.gerrit.server.account;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.AccessSection;
@ -136,8 +138,22 @@ public class AccountManager {
try (ReviewDb db = schema.open()) {
ExternalId id = externalIds.get(who.getExternalIdKey());
if (id == null) {
if (who.getUserName() != null) {
ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, who.getUserName());
ExternalId existingId = externalIds.get(key);
if (existingId != null) {
// An inconsistency is detected in the database, having a record for scheme "username:"
// but no record for scheme "gerrit:". Try to recover by linking
// "gerrit:" identity to the existing account.
log.warn(
"User {} already has an account; link new identity to the existing account.",
who.getUserName());
return link(existingId.accountId(), who);
}
}
// New account, automatically create and return.
//
log.info("External ID not found. Attempting to create new account.");
return create(db, who);
}
@ -387,6 +403,7 @@ public class AccountManager {
public AuthResult link(Account.Id to, AuthRequest who)
throws AccountException, OrmException, IOException, ConfigInvalidException {
ExternalId extId = externalIds.get(who.getExternalIdKey());
log.info("Link another authentication identity to an existing account");
if (extId != null) {
if (!extId.accountId().equals(to)) {
throw new AccountException(
@ -394,6 +411,7 @@ public class AccountManager {
}
update(who, extId);
} else {
log.info("Linking new external ID to the existing account");
externalIdsUpdateFactory
.create()
.insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));

52
gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java
View File

@ -288,7 +288,32 @@ public class PluginConfigFactory implements ReloadPluginListener {
*/
public Config getProjectPluginConfigWithInheritance(
Project.NameKey projectName, String pluginName) throws NoSuchProjectException {
return getPluginConfig(projectName, pluginName).getWithInheritance();
return getPluginConfig(projectName, pluginName).getWithInheritance(false);
}
/**
* Returns the configuration for the specified plugin that is stored in the '{@code
* <plugin-name>.config}' file in the 'refs/meta/config' branch of the specified project.
* Parameters from the '{@code <plugin-name>.config}' of the parent project are appended to this
* project's '{@code <plugin-name>.config}' files.
*
* <p>E.g.: child project: [mySection "mySubsection"] myKey = childValue
*
* <p>parent project: [mySection "mySubsection"] myKey = parentValue anotherKey = someValue
*
* <p>return: [mySection "mySubsection"] myKey = childValue myKey = parentValue anotherKey =
* someValue
*
* @param projectName the name of the project for which the plugin configuration should be
* returned
* @param pluginName the name of the plugin for which the configuration should be returned
* @return the plugin configuration from the '{@code <plugin-name>.config}' file of the specified
* project with parameters from the parent projects appended to the project values
* @throws NoSuchProjectException thrown if the specified project does not exist
*/
public Config getProjectPluginConfigWithMergedInheritance(
Project.NameKey projectName, String pluginName) throws NoSuchProjectException {
return getPluginConfig(projectName, pluginName).getWithInheritance(true);
}
/**
@ -310,7 +335,30 @@ public class PluginConfigFactory implements ReloadPluginListener {
*/
public Config getProjectPluginConfigWithInheritance(
ProjectState projectState, String pluginName) {
return projectState.getConfig(pluginName + EXTENSION).getWithInheritance();
return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(false);
}
/**
* Returns the configuration for the specified plugin that is stored in the '{@code
* <plugin-name>.config}' file in the 'refs/meta/config' branch of the specified project.
* Parameters from the '{@code <plugin-name>.config}' of the parent project are appended to this
* project's '{@code <plugin-name>.config}' files.
*
* <p>E.g.: child project: [mySection "mySubsection"] myKey = childValue
*
* <p>parent project: [mySection "mySubsection"] myKey = parentValue anotherKey = someValue
*
* <p>return: [mySection "mySubsection"] myKey = childValue myKey = parentValue anotherKey =
* someValue
*
* @param projectState the project for which the plugin configuration should be returned
* @param pluginName the name of the plugin for which the configuration should be returned
* @return the plugin configuration from the '{@code <plugin-name>.config}' file of the specified
* project with inheriting non-set parameters from the parent projects
*/
public Config getProjectPluginConfigWithMergedInheritance(
ProjectState projectState, String pluginName) {
return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(true);
}
private ProjectLevelConfig getPluginConfig(Project.NameKey projectName, String pluginName)

44
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectLevelConfig.java
View File

@ -14,12 +14,16 @@
package com.google.gerrit.server.git;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.project.ProjectState;
import java.io.IOException;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
@ -53,6 +57,22 @@ public class ProjectLevelConfig extends VersionedMetaData {
}
public Config getWithInheritance() {
return getWithInheritance(false);
}
/**
* Get a Config that includes the values from all parent projects.
*
* <p>Merging means that matching sections/subsection will be merged to include the values from
* both parent and child config.
*
* <p>No merging means that matching sections/subsections in the child project will replace the
* corresponding value from the parent.
*
* @param merge whether to merge parent values with child values or not.
* @return a combined config.
*/
public Config getWithInheritance(boolean merge) {
Config cfgWithInheritance = new Config();
try {
cfgWithInheritance.fromText(get().toText());
@ -65,21 +85,41 @@ public class ProjectLevelConfig extends VersionedMetaData {
for (String section : parentCfg.getSections()) {
Set<String> allNames = get().getNames(section);
for (String name : parentCfg.getNames(section)) {
String[] parentValues = parentCfg.getStringList(section, null, name);
if (!allNames.contains(name)) {
cfgWithInheritance.setStringList(section, null, name, Arrays.asList(parentValues));
} else if (merge) {
cfgWithInheritance.setStringList(
section, null, name, Arrays.asList(parentCfg.getStringList(section, null, name)));
section,
null,
name,
Stream.concat(
Arrays.stream(cfg.getStringList(section, null, name)),
Arrays.stream(parentValues))
.sorted()
.distinct()
.collect(toList()));
}
}
for (String subsection : parentCfg.getSubsections(section)) {
allNames = get().getNames(section, subsection);
for (String name : parentCfg.getNames(section, subsection)) {
String[] parentValues = parentCfg.getStringList(section, subsection, name);
if (!allNames.contains(name)) {
cfgWithInheritance.setStringList(
section, subsection, name, Arrays.asList(parentValues));
} else if (merge) {
cfgWithInheritance.setStringList(
section,
subsection,
name,
Arrays.asList(parentCfg.getStringList(section, subsection, name)));
Streams.concat(
Arrays.stream(cfg.getStringList(section, subsection, name)),
Arrays.stream(parentValues))
.sorted()
.distinct()
.collect(toList()));
}
}
}