diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index 2976e96dac..1efe839a8e 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt @@ -912,12 +912,15 @@ project. Plugins can refer to groups so that when they are renamed, the project config will also be updated in this section. The proper format to use is -the string representation of a GroupReference, as shown below. +the same as for any other group reference in the `project.config`, as shown below. ---- -Group[group_name / group_uuid] +group group_name ---- +The file `groups` must also contains the mapping of the group name and its UUID, +refer to link:config-project-config.html#file-groups[file groups] + [[project-specific-configuration]] == Project Specific Configuration in own config file diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java index 5f835c6192..dc22d62bc4 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java @@ -43,12 +43,6 @@ public class GroupReference implements Comparable { return configValue.substring(PREFIX.length()).trim(); } - public static GroupReference fromString(String ref) { - String name = ref.substring(ref.indexOf("[") + 1, ref.lastIndexOf("/")).trim(); - String uuid = ref.substring(ref.lastIndexOf("/") + 1, ref.lastIndexOf("]")).trim(); - return new GroupReference(new AccountGroup.UUID(uuid), name); - } - protected String uuid; protected String name; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java index 1b124959b3..2ac6695d68 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.config; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.Iterables; +import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.project.ProjectState; import java.util.Arrays; @@ -152,4 +153,12 @@ public class PluginConfig { public Set getNames() { return cfg.getNames(PLUGIN, pluginName, true); } + + public GroupReference getGroupReference(String name) { + return projectConfig.getGroup(GroupReference.extractGroupName(getString(name))); + } + + public void setGroupReference(String name, GroupReference value) { + setString(name, value.toConfigValue()); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index c19a234e7a..fae36517f1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -182,6 +182,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private boolean checkReceivedObjects; private Set sectionsWithUnknownPermissions; private boolean hasLegacyPermissions; + private Map groupsByName; public static ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException { @@ -410,6 +411,14 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. return groupList.byUUID(uuid); } + /** + * @return the group reference corresponding to the specified group name if the group is used by + * at least one rule or plugin value. + */ + public GroupReference getGroup(String groupName) { + return groupsByName.get(groupName); + } + /** @return set of all groups used by this configuration. */ public Set getAllGroupUUIDs() { return groupList.uuids(); @@ -473,7 +482,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. @Override protected void onLoad() throws IOException, ConfigInvalidException { readGroupList(); - Map groupsByName = mapGroupReferences(); + groupsByName = mapGroupReferences(); rulesId = getObjectId("rules.pl"); Config rc = readConfig(PROJECT_CONFIG); @@ -515,11 +524,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. p.setDefaultDashboard(rc.getString(DASHBOARD, null, KEY_DEFAULT)); p.setLocalDefaultDashboard(rc.getString(DASHBOARD, null, KEY_LOCAL_DEFAULT)); - loadAccountsSection(rc, groupsByName); - loadContributorAgreements(rc, groupsByName); - loadAccessSections(rc, groupsByName); + loadAccountsSection(rc); + loadContributorAgreements(rc); + loadAccessSections(rc); loadBranchOrderSection(rc); - loadNotifySections(rc, groupsByName); + loadNotifySections(rc); loadLabelSections(rc); loadCommentLinkSections(rc); loadSubscribeSections(rc); @@ -528,13 +537,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. loadReceiveSection(rc); } - private void loadAccountsSection(Config rc, Map groupsByName) { + private void loadAccountsSection(Config rc) { accountsSection = new AccountsSection(); accountsSection.setSameGroupVisibility( loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, groupsByName, false)); } - private void loadContributorAgreements(Config rc, Map groupsByName) { + private void loadContributorAgreements(Config rc) { contributorAgreements = new HashMap<>(); for (String name : rc.getSubsections(CONTRIBUTOR_AGREEMENT)) { ContributorAgreement ca = getContributorAgreement(name, true); @@ -594,7 +603,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. * type = submitted_changes * */ - private void loadNotifySections(Config rc, Map groupsByName) { + private void loadNotifySections(Config rc) { notifySections = new HashMap<>(); for (String sectionName : rc.getSubsections(NOTIFY)) { NotifyConfig n = new NotifyConfig(); @@ -639,7 +648,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } } - private void loadAccessSections(Config rc, Map groupsByName) { + private void loadAccessSections(Config rc) { accessSections = new HashMap<>(); sectionsWithUnknownPermissions = new HashSet<>(); for (String refName : rc.getSubsections(ACCESS)) { @@ -940,17 +949,15 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. pluginConfigs.put(plugin, pluginConfig); for (String name : rc.getNames(PLUGIN, plugin)) { String value = rc.getString(PLUGIN, plugin, name); - if (value.startsWith("Group[")) { - GroupReference refFromString = GroupReference.fromString(value); - GroupReference ref = groupList.byUUID(refFromString.getUUID()); + String groupName = GroupReference.extractGroupName(value); + if (groupName != null) { + GroupReference ref = groupsByName.get(groupName); if (ref == null) { - ref = refFromString; error( new ValidationError( - PROJECT_CONFIG, - "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME)); + PROJECT_CONFIG, "group \"" + groupName + "\" not in " + GroupList.FILE_NAME)); } - rc.setString(PLUGIN, plugin, name, ref.toString()); + rc.setString(PLUGIN, plugin, name, value); } pluginConfig.setStringList( PLUGIN, plugin, name, Arrays.asList(rc.getStringList(PLUGIN, plugin, name))); @@ -1354,11 +1361,12 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. Config pluginConfig = e.getValue(); for (String name : pluginConfig.getNames(PLUGIN, plugin)) { String value = pluginConfig.getString(PLUGIN, plugin, name); - if (value.startsWith("Group[")) { - GroupReference ref = resolve(GroupReference.fromString(value)); - if (ref.getUUID() != null) { + String groupName = GroupReference.extractGroupName(value); + if (groupName != null) { + GroupReference ref = groupsByName.get(groupName); + if (ref != null && ref.getUUID() != null) { keepGroups.add(ref.getUUID()); - pluginConfig.setString(PLUGIN, plugin, name, ref.toString()); + pluginConfig.setString(PLUGIN, plugin, name, "group " + ref.getName()); } } rc.setStringList( diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java index ab68c10a92..904f8702ac 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java @@ -27,8 +27,10 @@ import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; +import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -325,6 +327,147 @@ public class ProjectConfigTest extends LocalDiskRepositoryTestCase { + " read = group Developers\n"); } + @Test + public void readExistingPluginConfig() throws Exception { + RevCommit rev = + util.commit( + util.tree( // + util.file( + "project.config", + util.blob( + "" // + + "[plugin \"somePlugin\"]\n" // + + " key1 = value1\n" // + + " key2 = value2a\n" + + " key2 = value2b\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin"); + assertThat(pluginCfg.getNames().size()).isEqualTo(2); + assertThat(pluginCfg.getString("key1")).isEqualTo("value1"); + assertThat(pluginCfg.getStringList(("key2"))).isEqualTo(new String[] {"value2a", "value2b"}); + } + + @Test + public void readUnexistingPluginConfig() throws Exception { + ProjectConfig cfg = new ProjectConfig(new Project.NameKey("test")); + cfg.load(db); + PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin"); + assertThat(pluginCfg.getNames()).isEmpty(); + } + + @Test + public void editPluginConfig() throws Exception { + RevCommit rev = + util.commit( + util.tree( // + util.file( + "project.config", + util.blob( + "" // + + "[plugin \"somePlugin\"]\n" // + + " key1 = value1\n" // + + " key2 = value2a\n" // + + " key2 = value2b\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin"); + pluginCfg.setString("key1", "updatedValue1"); + pluginCfg.setStringList("key2", Arrays.asList("updatedValue2a", "updatedValue2b")); + rev = commit(cfg); + assertThat(text(rev, "project.config")) + .isEqualTo( + "" // + + "[plugin \"somePlugin\"]\n" // + + "\tkey1 = updatedValue1\n" // + + "\tkey2 = updatedValue2a\n" // + + "\tkey2 = updatedValue2b\n"); + } + + @Test + public void readPluginConfigGroupReference() throws Exception { + RevCommit rev = + util.commit( + util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file( + "project.config", + util.blob( + "" // + + "[plugin \"somePlugin\"]\n" // + + "key1 = " + + developers.toConfigValue() + + "\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin"); + assertThat(pluginCfg.getNames().size()).isEqualTo(1); + assertThat(pluginCfg.getGroupReference("key1")).isEqualTo(developers); + } + + @Test + public void readPluginConfigGroupReferenceNotInGroupsFile() throws Exception { + RevCommit rev = + util.commit( + util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file( + "project.config", + util.blob( + "" // + + "[plugin \"somePlugin\"]\n" // + + "key1 = " + + staff.toConfigValue() + + "\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + assertThat(cfg.getValidationErrors()).hasSize(1); + assertThat(Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage()) + .isEqualTo( + "project.config: group \"" + staff.getName() + "\" not in " + GroupList.FILE_NAME); + } + + @Test + public void editPluginConfigGroupReference() throws Exception { + RevCommit rev = + util.commit( + util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file( + "project.config", + util.blob( + "" // + + "[plugin \"somePlugin\"]\n" // + + "key1 = " + + developers.toConfigValue() + + "\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin"); + assertThat(pluginCfg.getNames().size()).isEqualTo(1); + assertThat(pluginCfg.getGroupReference("key1")).isEqualTo(developers); + + pluginCfg.setGroupReference("key1", staff); + rev = commit(cfg); + assertThat(text(rev, "project.config")) + .isEqualTo( + "" // + + "[plugin \"somePlugin\"]\n" // + + "\tkey1 = " + + staff.toConfigValue() + + "\n"); + } + private ProjectConfig read(RevCommit rev) throws IOException, ConfigInvalidException { ProjectConfig cfg = new ProjectConfig(new Project.NameKey("test")); cfg.load(db, rev);