opendev/gerrit
Code Issues Proposed changes

Align group reference from plugin with core group reference

Browse Source 
I1160f1f55 added support to allow plugins to reference groups in
project.config but the way of referencing differs from what is done
in the core.

Even though the reference contains both group_name and group_uuid, it
was still required to have the mapping in groups file.

projet.config
  [plugin "somePlugin"]
    key = Group[someGroup/d6da7dc4e99e6f6e5b5196e21b6f504fc530bba]

groups
  3d6da7dc4e99e6f6e5b5196e21b6f504fc530bba    someGroups

Re-align group referencing with what is done all the other sections of
project.config to prevent having to put the group_uuid in both
project.config and groups file. Also this will make project.config more
consistent.

project.config
  [plugin "somePlugin"]
    key = group someGroup

groups
  3d6da7dc4e99e6f6e5b5196e21b6f504fc530bba    someGroup

This change is not backward compatible but searching[1] the open source
plugins, there is not a single use of GroupReference.fromString method
which should be used to load a group reference from project.config.

[1]http://cs.bazel.build/search?q=r%3Aplugins+GroupReference+fromString&num=50

Change-Id: I0f21de8fabe33dc60905333202e9dad8006bd05a
This commit is contained in:
Hugo Arès
2017-06-16 09:31:08 -04:00
parent 279ab7cb13
commit 532e0a3b57
5 changed files with 49 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@@ -189,6 +189,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private Set<String> sectionsWithUnknownPermissions;
private boolean hasLegacyPermissions;
private Map<String, List<String>> extensionPanelSections;
private Map<String, GroupReference> groupsByName;
public static ProjectConfig read(MetaDataUpdate update)
throws IOException, ConfigInvalidException {
@@ -421,6 +422,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<AccountGroup.UUID> getAllGroupUUIDs() {
return groupList.uuids();
@@ -484,7 +493,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
readGroupList();
Map<String, GroupReference> groupsByName = mapGroupReferences();
groupsByName = mapGroupReferences();
rulesId = getObjectId("rules.pl");
Config rc = readConfig(PROJECT_CONFIG);
@@ -528,11 +537,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);
@@ -542,7 +551,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
loadExtensionPanelSections(rc);
}
private void loadAccountsSection(Config rc, Map<String, GroupReference> groupsByName) {
private void loadAccountsSection(Config rc) {
accountsSection = new AccountsSection();
accountsSection.setSameGroupVisibility(
loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, groupsByName, false));
@@ -567,7 +576,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
}
private void loadContributorAgreements(Config rc, Map<String, GroupReference> groupsByName) {
private void loadContributorAgreements(Config rc) {
contributorAgreements = new HashMap<>();
for (String name : rc.getSubsections(CONTRIBUTOR_AGREEMENT)) {
ContributorAgreement ca = getContributorAgreement(name, true);
@@ -627,7 +636,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
* type = submitted_changes
* </pre>
*/
private void loadNotifySections(Config rc, Map<String, GroupReference> groupsByName) {
private void loadNotifySections(Config rc) {
notifySections = new HashMap<>();
for (String sectionName : rc.getSubsections(NOTIFY)) {
NotifyConfig n = new NotifyConfig();
@@ -672,7 +681,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
}
private void loadAccessSections(Config rc, Map<String, GroupReference> groupsByName) {
private void loadAccessSections(Config rc) {
accessSections = new HashMap<>();
sectionsWithUnknownPermissions = new HashSet<>();
for (String refName : rc.getSubsections(ACCESS)) {
@@ -973,17 +982,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)));
@@ -1409,11 +1416,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(