opendev/gerrit
Code Issues Proposed changes

Replace use of AccountGroup by an interface for various classes

Browse Source 
Previously, GroupDescription.Internal served only as a marker for
internal groups and simply delegated to AccountGroup. By converting
GroupDescription.Internal to a real interface, we gain a lot of
flexibility. For instance, we are free to use another class to
represent internal groups (which will be necessary for the migration
of groups to NoteDb).

Change-Id: If7397898d8508184a2ccdc7c371ed60ada7d6f3e
This commit is contained in:
Alice Kober-Sotzek
2017-08-16 17:41:38 +02:00
parent 6da54a4009
commit 3a68432798
28 changed files with 154 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
View File

@@ -25,19 +25,15 @@ import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.group.CreateGroup;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.inject.Inject;
import java.util.Arrays;
import java.util.List;
@@ -48,8 +44,6 @@ import org.junit.Test;
public class SuggestReviewersIT extends AbstractDaemonTest {
@Inject private CreateGroup.Factory createGroupFactory;
@Inject private GroupsCollection groups;
private AccountGroup group1;
private AccountGroup group2;
private AccountGroup group3;
@@ -432,8 +426,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
private AccountGroup group(String name) throws Exception {
GroupInfo group = createGroupFactory.create(name(name)).apply(TopLevelResource.INSTANCE, null);
GroupDescription.Basic d = groups.parseInternal(Url.decode(group.id));
return GroupDescriptions.toAccountGroup(d);
return groupCache.get(new AccountGroup.UUID(group.id));
}
private TestAccount user(String name, String fullName, String emailName, AccountGroup... groups)

15
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDescription.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.common.data;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.AccountGroup;
import java.sql.Timestamp;
/** Group methods exposed by the GroupBackend. */
public class GroupDescription {
@@ -42,10 +43,18 @@ public class GroupDescription {
String getUrl();
}
/** The extended information exposed by internal groups backed by an AccountGroup. */
/** The extended information exposed by internal groups. */
public interface Internal extends Basic {
/** @return the backing AccountGroup. */
AccountGroup getAccountGroup();
AccountGroup.Id getId();
String getDescription();
AccountGroup.UUID getOwnerGroupUUID();
boolean isVisibleToAll();
Timestamp getCreatedOn();
}
private GroupDescription() {}

40
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDescriptions.java
View File

@@ -17,18 +17,11 @@ package com.google.gerrit.common.data;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.AccountGroup;
import java.sql.Timestamp;
/** Utility class for building GroupDescription objects. */
public class GroupDescriptions {
@Nullable
public static AccountGroup toAccountGroup(GroupDescription.Basic group) {
if (group instanceof GroupDescription.Internal) {
return ((GroupDescription.Internal) group).getAccountGroup();
}
return null;
}
public static GroupDescription.Internal forAccountGroup(AccountGroup group) {
return new GroupDescription.Internal() {
@Override
@@ -41,11 +34,6 @@ public class GroupDescriptions {
return group.getName();
}
@Override
public AccountGroup getAccountGroup() {
return group;
}
@Override
@Nullable
public String getEmailAddress() {
@@ -53,10 +41,34 @@ public class GroupDescriptions {
}
@Override
@Nullable
public String getUrl() {
return "#" + PageLinks.toGroup(getGroupUUID());
}
@Override
public AccountGroup.Id getId() {
return group.getId();
}
@Override
public String getDescription() {
return group.getDescription();
}
@Override
public AccountGroup.UUID getOwnerGroupUUID() {
return group.getOwnerGroupUUID();
}
@Override
public boolean isVisibleToAll() {
return group.isVisibleToAll();
}
@Override
public Timestamp getCreatedOn() {
return group.getCreatedOn();
}
};
}

3
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java
View File

@@ -46,8 +46,7 @@ public class GroupInfo {
url = a.getUrl();
if (a instanceof GroupDescription.Internal) {
AccountGroup group = ((GroupDescription.Internal) a).getAccountGroup();
description = group.getDescription();
description = ((GroupDescription.Internal) a).getDescription();
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
View File

@@ -18,7 +18,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAI
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.InvalidSshKeyException;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.annotations.RequiresCapability;
@@ -211,8 +211,8 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
Set<AccountGroup.UUID> groupUuids = new HashSet<>();
if (groups != null) {
for (String g : groups) {
AccountGroup group = GroupDescriptions.toAccountGroup(groupsCollection.parseInternal(g));
groupUuids.add(group.getGroupUUID());
GroupDescription.Internal internalGroup = groupsCollection.parseInternal(g);
groupUuids.add(internalGroup.getGroupUUID());
}
}
return groupUuids;

19
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
View File

@@ -142,12 +142,15 @@ public class GroupControl {
}
public boolean isOwner() {
AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
if (accountGroup == null) {
isOwner = false;
} else if (isOwner == null) {
AccountGroup.UUID ownerUUID = accountGroup.getOwnerGroupUUID();
if (isOwner != null) {
return isOwner;
}
if (group instanceof GroupDescription.Internal) {
AccountGroup.UUID ownerUUID = ((GroupDescription.Internal) group).getOwnerGroupUUID();
isOwner = getUser().getEffectiveGroups().contains(ownerUUID) || canAdministrateServer();
} else {
isOwner = false;
}
return isOwner;
}
@@ -189,7 +192,9 @@ public class GroupControl {
}
private boolean canSeeMembers() {
AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
return (accountGroup != null && accountGroup.isVisibleToAll()) || isOwner();
if (group instanceof GroupDescription.Internal) {
return ((GroupDescription.Internal) group).isVisibleToAll() || isOwner();
}
return false;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
View File

@@ -85,6 +85,6 @@ public class InternalGroupBackend implements GroupBackend {
@Override
public boolean isVisibleToAll(AccountGroup.UUID uuid) {
GroupDescription.Internal g = get(uuid);
return g != null && g.getAccountGroup().isVisibleToAll();
return g != null && g.isVisibleToAll();
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
View File

@@ -87,10 +87,8 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
public List<GroupInfo> apply(GroupResource resource, Input input)
throws MethodNotAllowedException, AuthException, UnprocessableEntityException, OrmException,
ResourceNotFoundException {
AccountGroup group = resource.toAccountGroup();
if (group == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal group =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
input = Input.init(input);
GroupControl control = resource.getControl();

7
gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.group;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.client.AuthType;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -111,10 +112,8 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
public List<AccountInfo> apply(GroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
IOException, ConfigInvalidException, ResourceNotFoundException {
AccountGroup internalGroup = resource.toAccountGroup();
if (internalGroup == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
input = Input.init(input);
GroupControl control = resource.getControl();

4
gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java
View File

@@ -166,8 +166,8 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
private AccountGroup.Id owner(GroupInput input) throws UnprocessableEntityException {
if (input.ownerId != null) {
GroupDescription.Basic d = groups.parseInternal(Url.decode(input.ownerId));
return GroupDescriptions.toAccountGroup(d).getId();
GroupDescription.Internal d = groups.parseInternal(Url.decode(input.ownerId));
return d.getId();
}
return null;
}

6
gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java
View File

@@ -54,10 +54,8 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
public Response<?> apply(GroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
ResourceNotFoundException {
AccountGroup internalGroup = resource.toAccountGroup();
if (internalGroup == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
input = Input.init(input);
final GroupControl control = resource.getControl();

7
gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -56,10 +57,8 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
public Response<?> apply(GroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
IOException, ConfigInvalidException, ResourceNotFoundException {
AccountGroup internalGroup = resource.toAccountGroup();
if (internalGroup == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
input = Input.init(input);
final GroupControl control = resource.getControl();

7
gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java
View File

@@ -64,10 +64,9 @@ public class GetAuditLog implements RestReadView<GroupResource> {
@Override
public List<? extends GroupAuditEventInfo> apply(GroupResource rsrc)
throws AuthException, MethodNotAllowedException, OrmException {
AccountGroup group = rsrc.toAccountGroup();
if (group == null) {
throw new MethodNotAllowedException();
} else if (!rsrc.getControl().isOwner()) {
GroupDescription.Internal group =
rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
if (!rsrc.getControl().isOwner()) {
throw new AuthException("Not group owner");
}

8
gerrit-server/src/main/java/com/google/gerrit/server/group/GetDescription.java
View File

@@ -15,19 +15,17 @@
package com.google.gerrit.server.group;
import com.google.common.base.Strings;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.inject.Singleton;
@Singleton
public class GetDescription implements RestReadView<GroupResource> {
@Override
public String apply(GroupResource resource) throws MethodNotAllowedException {
AccountGroup group = resource.toAccountGroup();
if (group == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal group =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
return Strings.nullToEmpty(group.getDescription());
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/group/GetOwner.java
View File

@@ -14,12 +14,12 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.GroupControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -40,10 +40,8 @@ public class GetOwner implements RestReadView<GroupResource> {
@Override
public GroupInfo apply(GroupResource resource)
throws MethodNotAllowedException, ResourceNotFoundException, OrmException {
AccountGroup group = resource.toAccountGroup();
if (group == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal group =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
try {
GroupControl c = controlFactory.validateFor(group.getOwnerGroupUUID());
return json.format(c.getGroup());

27
gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java
View File

@@ -19,7 +19,6 @@ import static com.google.gerrit.extensions.client.ListGroupsOption.MEMBERS;
import com.google.common.base.Strings;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.client.ListGroupsOption;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.GroupOptionsInfo;
@@ -37,8 +36,7 @@ import java.util.EnumSet;
public class GroupJson {
public static GroupOptionsInfo createOptions(GroupDescription.Basic group) {
GroupOptionsInfo options = new GroupOptionsInfo();
AccountGroup ag = GroupDescriptions.toAccountGroup(group);
if (ag != null && ag.isVisibleToAll()) {
if (isInternalGroup(group) && ((GroupDescription.Internal) group).isVisibleToAll()) {
options.visibleToAll = true;
}
return options;
@@ -96,25 +94,30 @@ public class GroupJson {
info.url = Strings.emptyToNull(group.getUrl());
info.options = createOptions(group);
AccountGroup g = GroupDescriptions.toAccountGroup(group);
if (g != null) {
info.description = Strings.emptyToNull(g.getDescription());
info.groupId = g.getId().get();
if (g.getOwnerGroupUUID() != null) {
info.ownerId = Url.encode(g.getOwnerGroupUUID().get());
GroupDescription.Basic o = groupBackend.get(g.getOwnerGroupUUID());
if (isInternalGroup(group)) {
GroupDescription.Internal internalGroup = (GroupDescription.Internal) group;
info.description = Strings.emptyToNull(internalGroup.getDescription());
info.groupId = internalGroup.getId().get();
AccountGroup.UUID ownerGroupUUID = internalGroup.getOwnerGroupUUID();
if (ownerGroupUUID != null) {
info.ownerId = Url.encode(ownerGroupUUID.get());
GroupDescription.Basic o = groupBackend.get(ownerGroupUUID);
if (o != null) {
info.owner = o.getName();
}
}
info.createdOn = g.getCreatedOn();
info.createdOn = internalGroup.getCreatedOn();
}
return info;
}
private static boolean isInternalGroup(GroupDescription.Basic group) {
return group instanceof GroupDescription.Internal;
}
private GroupInfo initMembersAndIncludes(GroupResource rsrc, GroupInfo info) throws OrmException {
if (rsrc.toAccountGroup() == null) {
if (!rsrc.isInternalGroup()) {
return info;
}
try {

16
gerrit-server/src/main/java/com/google/gerrit/server/group/GroupResource.java
View File

@@ -15,12 +15,11 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.GroupControl;
import com.google.inject.TypeLiteral;
import java.util.Optional;
public class GroupResource implements RestResource {
public static final TypeLiteral<RestView<GroupResource>> GROUP_KIND =
@@ -44,12 +43,17 @@ public class GroupResource implements RestResource {
return getGroup().getName();
}
public AccountGroup.UUID getGroupUUID() {
return getGroup().getGroupUUID();
public boolean isInternalGroup() {
GroupDescription.Basic group = getGroup();
return group instanceof GroupDescription.Internal;
}
public AccountGroup toAccountGroup() {
return GroupDescriptions.toAccountGroup(getGroup());
public Optional<GroupDescription.Internal> asInternalGroup() {
GroupDescription.Basic group = getGroup();
if (group instanceof GroupDescription.Internal) {
return Optional.of((GroupDescription.Internal) group);
}
return Optional.empty();
}
public GroupControl getControl() {

10
gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.group;
import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -142,12 +141,13 @@ public class GroupsCollection
* @throws UnprocessableEntityException thrown if the group ID cannot be resolved, if the group is
* not visible to the calling user or if it's an external group
*/
public GroupDescription.Basic parseInternal(String id) throws UnprocessableEntityException {
public GroupDescription.Internal parseInternal(String id) throws UnprocessableEntityException {
GroupDescription.Basic group = parse(id);
if (GroupDescriptions.toAccountGroup(group) == null) {
throw new UnprocessableEntityException(String.format("External Group Not Allowed: %s", id));
if (group instanceof GroupDescription.Internal) {
return (GroupDescription.Internal) group;
}
return group;
throw new UnprocessableEntityException(String.format("External Group Not Allowed: %s", id));
}
/**

9
gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java
View File

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.group.AddIncludedGroups.PutIncludedGroup;
import com.google.gwtorm.server.OrmException;
@@ -67,10 +66,8 @@ public class IncludedGroupsCollection
@Override
public IncludedGroupResource parse(GroupResource resource, IdString id)
throws MethodNotAllowedException, AuthException, ResourceNotFoundException, OrmException {
AccountGroup parent = resource.toAccountGroup();
if (parent == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal parent =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
GroupDescription.Basic member =
groupsCollection.parse(TopLevelResource.INSTANCE, id).getGroup();
@@ -80,7 +77,7 @@ public class IncludedGroupsCollection
throw new ResourceNotFoundException(id);
}
private boolean isMember(AccountGroup parent, GroupDescription.Basic member)
private boolean isMember(GroupDescription.Internal parent, GroupDescription.Basic member)
throws OrmException, ResourceNotFoundException {
try {
return groups.isIncluded(dbProvider.get(), parent.getGroupUUID(), member.getGroupUUID());

10
gerrit-server/src/main/java/com/google/gerrit/server/group/Index.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -44,14 +43,15 @@ public class Index implements RestModifyView<GroupResource, Input> {
throw new AuthException("not allowed to index group");
}
AccountGroup group = GroupDescriptions.toAccountGroup(rsrc.getGroup());
if (group == null) {
AccountGroup.UUID groupUuid = rsrc.getGroup().getGroupUUID();
if (!rsrc.isInternalGroup()) {
throw new UnprocessableEntityException(
String.format("External Group Not Allowed: %s", rsrc.getGroupUUID().get()));
String.format("External Group Not Allowed: %s", groupUuid.get()));
}
AccountGroup accountGroup = groupCache.get(groupUuid);
// evicting the group from the cache, reindexes the group
groupCache.evict(group);
groupCache.evict(accountGroup);
return Response.none();
}
}

10
gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.group;
import static com.google.common.base.Strings.nullToEmpty;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -51,14 +52,13 @@ public class ListIncludedGroups implements RestReadView<GroupResource> {
@Override
public List<GroupInfo> apply(GroupResource rsrc) throws MethodNotAllowedException, OrmException {
if (rsrc.toAccountGroup() == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal group =
rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
boolean ownerOfParent = rsrc.getControl().isOwner();
List<GroupInfo> included = new ArrayList<>();
Collection<AccountGroup.UUID> includedGroupUuids =
groupIncludeCache.subgroupsOf(rsrc.toAccountGroup().getGroupUUID());
groupIncludeCache.subgroupsOf(group.getGroupUUID());
for (AccountGroup.UUID includedGroupUuid : includedGroupUuids) {
try {
GroupControl i = controlFactory.controlFor(includedGroupUuid);
@@ -69,7 +69,7 @@ public class ListIncludedGroups implements RestReadView<GroupResource> {
log.warn(
String.format(
"Group %s no longer available, included into %s",
includedGroupUuid, rsrc.getGroup().getName()));
includedGroupUuid, group.getName()));
continue;
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.group;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -61,11 +62,9 @@ public class ListMembers implements RestReadView<GroupResource> {
@Override
public List<AccountInfo> apply(GroupResource resource)
throws MethodNotAllowedException, OrmException {
if (resource.toAccountGroup() == null) {
throw new MethodNotAllowedException();
}
return apply(resource.getGroupUUID());
GroupDescription.Internal group =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
return apply(group.getGroupUUID());
}
public List<AccountInfo> apply(AccountGroup group) throws OrmException {

9
gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsCreate;
@@ -71,10 +72,8 @@ public class MembersCollection
public MemberResource parse(GroupResource parent, IdString id)
throws MethodNotAllowedException, AuthException, ResourceNotFoundException, OrmException,
IOException, ConfigInvalidException {
AccountGroup group = parent.toAccountGroup();
if (group == null) {
throw new MethodNotAllowedException();
}
GroupDescription.Internal group =
parent.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
IdentifiedUser user = accounts.parse(TopLevelResource.INSTANCE, id).getUser();
if (parent.getControl().canSeeMember(user.getAccountId()) && isMember(group, user)) {
@@ -83,7 +82,7 @@ public class MembersCollection
throw new ResourceNotFoundException(id);
}
private boolean isMember(AccountGroup group, IdentifiedUser user)
private boolean isMember(GroupDescription.Internal group, IdentifiedUser user)
throws OrmException, ResourceNotFoundException {
AccountGroup.UUID groupUuid = group.getGroupUUID();
try {

12
gerrit-server/src/main/java/com/google/gerrit/server/group/PutDescription.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.group;
import com.google.common.base.Strings;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.DefaultInput;
@@ -56,16 +57,15 @@ public class PutDescription implements RestModifyView<GroupResource, Input> {
input = new Input(); // Delete would set description to null.
}
AccountGroup accountGroup = resource.toAccountGroup();
if (accountGroup == null) {
throw new MethodNotAllowedException();
} else if (!resource.getControl().isOwner()) {
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
if (!resource.getControl().isOwner()) {
throw new AuthException("Not group owner");
}
String newDescription = Strings.emptyToNull(input.description);
if (!Objects.equals(accountGroup.getDescription(), newDescription)) {
AccountGroup.UUID groupUuid = resource.getGroupUUID();
if (!Objects.equals(internalGroup.getDescription(), newDescription)) {
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
try {
groupsUpdateProvider
.get()

13
gerrit-server/src/main/java/com/google/gerrit/server/group/PutName.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.group;
import com.google.common.base.Strings;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -52,9 +53,9 @@ public class PutName implements RestModifyView<GroupResource, Input> {
public String apply(GroupResource rsrc, Input input)
throws MethodNotAllowedException, AuthException, BadRequestException,
ResourceConflictException, ResourceNotFoundException, OrmException, IOException {
if (rsrc.toAccountGroup() == null) {
throw new MethodNotAllowedException();
} else if (!rsrc.getControl().isOwner()) {
GroupDescription.Internal internalGroup =
rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
if (!rsrc.getControl().isOwner()) {
throw new AuthException("Not group owner");
} else if (input == null || Strings.isNullOrEmpty(input.name)) {
throw new BadRequestException("name is required");
@@ -64,15 +65,15 @@ public class PutName implements RestModifyView<GroupResource, Input> {
throw new BadRequestException("name is required");
}
if (rsrc.toAccountGroup().getName().equals(newName)) {
if (internalGroup.getName().equals(newName)) {
return newName;
}
renameGroup(rsrc.toAccountGroup(), newName);
renameGroup(internalGroup, newName);
return newName;
}
private void renameGroup(AccountGroup group, String newName)
private void renameGroup(GroupDescription.Internal group, String newName)
throws ResourceConflictException, ResourceNotFoundException, OrmException, IOException {
AccountGroup.UUID groupUuid = group.getGroupUUID();
try {

12
gerrit-server/src/main/java/com/google/gerrit/server/group/PutOptions.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.GroupOptionsInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -44,10 +45,9 @@ public class PutOptions implements RestModifyView<GroupResource, GroupOptionsInf
public GroupOptionsInfo apply(GroupResource resource, GroupOptionsInfo input)
throws MethodNotAllowedException, AuthException, BadRequestException,
ResourceNotFoundException, OrmException, IOException {
AccountGroup accountGroup = resource.toAccountGroup();
if (accountGroup == null) {
throw new MethodNotAllowedException();
} else if (!resource.getControl().isOwner()) {
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
if (!resource.getControl().isOwner()) {
throw new AuthException("Not group owner");
}
@@ -58,8 +58,8 @@ public class PutOptions implements RestModifyView<GroupResource, GroupOptionsInf
input.visibleToAll = false;
}
if (accountGroup.isVisibleToAll() != input.visibleToAll) {
AccountGroup.UUID groupUuid = accountGroup.getGroupUUID();
if (internalGroup.isVisibleToAll() != input.visibleToAll) {
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
try {
groupsUpdateProvider
.get()

11
gerrit-server/src/main/java/com/google/gerrit/server/group/PutOwner.java
View File

@@ -61,10 +61,9 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
public GroupInfo apply(GroupResource resource, Input input)
throws ResourceNotFoundException, MethodNotAllowedException, AuthException,
BadRequestException, UnprocessableEntityException, OrmException, IOException {
AccountGroup accountGroup = resource.toAccountGroup();
if (accountGroup == null) {
throw new MethodNotAllowedException();
} else if (!resource.getControl().isOwner()) {
GroupDescription.Internal internalGroup =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
if (!resource.getControl().isOwner()) {
throw new AuthException("Not group owner");
}
@@ -73,8 +72,8 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
}
GroupDescription.Basic owner = groupsCollection.parse(input.owner);
if (!accountGroup.getOwnerGroupUUID().equals(owner.getGroupUUID())) {
AccountGroup.UUID groupUuid = resource.getGroupUUID();
if (!internalGroup.getOwnerGroupUUID().equals(owner.getGroupUUID())) {
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
try {
groupsUpdateProvider
.get()

5
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ProjectWatch.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.mail.send;
import com.google.common.base.Strings;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.index.query.Predicate;
@@ -175,12 +174,12 @@ public class ProjectWatch {
continue;
}
AccountGroup ig = GroupDescriptions.toAccountGroup(group);
if (ig == null) {
if (!(group instanceof GroupDescription.Internal)) {
// Non-internal groups cannot be expanded by the server.
continue;
}
GroupDescription.Internal ig = (GroupDescription.Internal) group;
try {
args.groups.getMembers(db, ig.getGroupUUID()).forEach(matching.accounts::add);
} catch (NoSuchGroupException e) {