Do not try to recreate non-local groups
If a group is supplied by an external source or is a global group,
then no action is needed upon import.
Change-Id: I5336a7f5dc192e19146c982f76559a1cfd21f95c
diff --git a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroup.java b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroup.java
index 8c72e11..384c089 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroup.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroup.java
@@ -14,11 +14,14 @@
package com.googlesource.gerrit.plugins.importer;
+import static com.google.gerrit.reviewdb.client.AccountGroup.isInternalGroup;
+
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -107,7 +110,8 @@
@Override
public Response<String> apply(ConfigResource rsrc, Input input)
throws ResourceConflictException, PreconditionFailedException,
- BadRequestException, NoSuchAccountException, OrmException, IOException {
+ BadRequestException, NoSuchAccountException, OrmException, IOException,
+ MethodNotAllowedException {
GroupInfo groupInfo;
this.api = apiFactory.create(input.from, input.user, input.pass);
groupInfo = api.getGroup(group.get());
@@ -119,7 +123,12 @@
private void validate(Input input, GroupInfo groupInfo) throws ResourceConflictException,
PreconditionFailedException, BadRequestException, IOException,
- OrmException, NoSuchAccountException {
+ OrmException, NoSuchAccountException, MethodNotAllowedException {
+ if (!isInternalGroup(new AccountGroup.UUID(groupInfo.id))) {
+ throw new MethodNotAllowedException(String.format(
+ "Group with name %s is not an internal group and cannot be imported",
+ groupInfo.name));
+ }
if (getGroupByName(groupInfo.name) != null) {
throw new ResourceConflictException(String.format(
"Group with name %s already exists", groupInfo.name));
@@ -197,7 +206,7 @@
private AccountGroup createGroup(Input input, GroupInfo info) throws OrmException,
ResourceConflictException, NoSuchAccountException, BadRequestException,
- IOException, PreconditionFailedException {
+ IOException, PreconditionFailedException, MethodNotAllowedException {
AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId());
AccountGroup.UUID uuid = new AccountGroup.UUID(info.id);
AccountGroup group =
@@ -218,14 +227,16 @@
if (!info.id.equals(info.ownerId)) {
if (getGroupByUUID(info.ownerId) == null) {
- String ownerGroupName = getGroupName(info.ownerId);
- if (input.importOwnerGroup) {
- importGroupFactory.create(new AccountGroup.NameKey(ownerGroupName))
- .apply(new ConfigResource(), input);
- } else {
- throw new IllegalStateException(String.format(
- "Cannot set non-existing group %s as owner of group %s.",
- ownerGroupName, info.name));
+ if (isInternalGroup(new AccountGroup.UUID(info.ownerId))) {
+ String ownerGroupName = getGroupName(info.ownerId);
+ if (input.importOwnerGroup) {
+ importGroupFactory.create(new AccountGroup.NameKey(ownerGroupName))
+ .apply(new ConfigResource(), input);
+ } else {
+ throw new IllegalStateException(String.format(
+ "Cannot set non-existing group %s as owner of group %s.",
+ ownerGroupName, info.name));
+ }
}
}
group.setOwnerGroupUUID(new AccountGroup.UUID(info.ownerId));
@@ -261,18 +272,21 @@
String groupName, List<GroupInfo> includedGroups)
throws BadRequestException, ResourceConflictException,
PreconditionFailedException, NoSuchAccountException, OrmException,
- IOException {
+ IOException, MethodNotAllowedException {
List<AccountGroupById> includeList = new ArrayList<>();
for (GroupInfo includedGroup : includedGroups) {
- if (getGroupByUUID(includedGroup.id) == null) {
- String includedGroupName = getGroupName(includedGroup.id);
- if (input.importIncludedGroups) {
- importGroupFactory.create(new AccountGroup.NameKey(includedGroupName))
- .apply(new ConfigResource(), input);
- } else {
- throw new IllegalStateException(String.format(
- "Cannot include non-existing group %s into group %s.",
- includedGroupName, groupName));
+ if (isInternalGroup(new AccountGroup.UUID(includedGroup.id))) {
+ if (getGroupByUUID(includedGroup.id) == null) {
+ String includedGroupName = getGroupName(includedGroup.id);
+ if (input.importIncludedGroups) {
+ importGroupFactory.create(
+ new AccountGroup.NameKey(includedGroupName)).apply(
+ new ConfigResource(), input);
+ } else {
+ throw new IllegalStateException(String.format(
+ "Cannot include non-existing group %s into group %s.",
+ includedGroupName, groupName));
+ }
}
}
AccountGroup.UUID memberUUID = new AccountGroup.UUID(includedGroup.id);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroupsStep.java b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroupsStep.java
index 0be6248..bcccf39 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroupsStep.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportGroupsStep.java
@@ -14,8 +14,11 @@
package com.googlesource.gerrit.plugins.importer;
+import static com.google.gerrit.reviewdb.client.AccountGroup.isInternalGroup;
+
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -79,20 +82,22 @@
Set<AccountGroup.UUID> groupUUIDs = projectConfig.getAllGroupUUIDs();
pm.beginTask("Import Groups", groupUUIDs.size());
for (AccountGroup.UUID groupUUID : groupUUIDs) {
- if (groupCache.get(groupUUID) == null) {
- ImportGroup.Input input = new ImportGroup.Input();
- input.from = fromGerrit;
- input.user = user;
- input.pass = password;
- input.importOwnerGroup = true;
- input.importIncludedGroups = true;
- try {
- importGroupFactory.create(
- new AccountGroup.NameKey(projectConfig.getGroup(groupUUID).getName()))
- .apply(new ConfigResource(), input);
- } catch (ResourceConflictException e) {
- // should not happen
- throw new IllegalStateException(e);
+ if (isInternalGroup(groupUUID)) {
+ if (groupCache.get(groupUUID) == null) {
+ ImportGroup.Input input = new ImportGroup.Input();
+ input.from = fromGerrit;
+ input.user = user;
+ input.pass = password;
+ input.importOwnerGroup = true;
+ input.importIncludedGroups = true;
+ try {
+ importGroupFactory.create(
+ new AccountGroup.NameKey(projectConfig.getGroup(groupUUID)
+ .getName())).apply(new ConfigResource(), input);
+ } catch (ResourceConflictException | MethodNotAllowedException e) {
+ // should not happen
+ throw new IllegalStateException(e);
+ }
}
}
pm.update(1);
