From 0d5b39770937bfc360f49a10cbe24ab841271126 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 16 Jan 2013 23:45:49 -0800 Subject: [PATCH] GET /accounts/{id}/groups Obtain the list of groups a target user is a member of from within the user resource, rather than /groups/?user={id} query. This is a different projection of essentially the same data, but I think it makes sense to allow a user to ask for their groups within their account. A nice benefit is we can now ask /accounts/self/groups to see our own groups at any time, whereas /groups/?user={id} does not accept self as an argument. Unlike /groups/?user={id} query any user can see a subset of another user's groups if two things are true: - The caller can see the other user. Often true when both users have a group in common. - The caller can see the group. True when the caller is an owner of that group, or the group is visible to everyone. - The caller can see the members of the group. True if the caller is an owner, or the group is visible to everyone. Change-Id: I43d43f2f774dafb1f77fd047b50b6fa050f82d07 --- .../gerrit/client/account/MyGroupsScreen.java | 6 +-- .../gerrit/client/admin/GroupTable.java | 12 +++-- .../gerrit/client/groups/GroupList.java | 29 ++++++++++ .../google/gerrit/client/groups/GroupMap.java | 7 --- .../gerrit/server/account/GetGroups.java | 54 +++++++++++++++++++ .../google/gerrit/server/account/Module.java | 1 + .../google/gerrit/server/group/GetGroup.java | 35 ------------ .../gerrit/server/group/GetIncludedGroup.java | 3 +- .../google/gerrit/server/group/GroupInfo.java | 50 +++++++++++++++++ .../gerrit/server/group/ListGroups.java | 1 - .../server/group/ListIncludedGroups.java | 3 +- 11 files changed, 147 insertions(+), 54 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupList.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyGroupsScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyGroupsScreen.java index 8b3bf560b0..e3bda5d557 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyGroupsScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyGroupsScreen.java @@ -15,7 +15,7 @@ package com.google.gerrit.client.account; import com.google.gerrit.client.admin.GroupTable; -import com.google.gerrit.client.groups.GroupMap; +import com.google.gerrit.client.groups.GroupList; import com.google.gerrit.client.rpc.ScreenLoadCallback; public class MyGroupsScreen extends SettingsScreen { @@ -31,9 +31,9 @@ public class MyGroupsScreen extends SettingsScreen { @Override protected void onLoad() { super.onLoad(); - GroupMap.my(new ScreenLoadCallback(this) { + GroupList.my(new ScreenLoadCallback(this) { @Override - protected void preDisplay(final GroupMap result) { + protected void preDisplay(GroupList result) { groups.display(result); groups.finishDisplay(); }}); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/GroupTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/GroupTable.java index 6a16f1a3f2..2ec0edc8c4 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/GroupTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/GroupTable.java @@ -17,6 +17,7 @@ package com.google.gerrit.client.admin; import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.groups.GroupInfo; +import com.google.gerrit.client.groups.GroupList; import com.google.gerrit.client.groups.GroupMap; import com.google.gerrit.client.ui.HighlightingInlineHyperlink; import com.google.gerrit.client.ui.NavigationTable; @@ -76,15 +77,18 @@ public class GroupTable extends NavigationTable { History.newItem(Dispatcher.toGroup(getRowItem(row).getGroupId())); } - public void display(final GroupMap groups) { - display(groups, null); + public void display(GroupMap groups, String toHighlight) { + display(groups.values().asList(), toHighlight); } - public void display(final GroupMap groups, final String toHighlight) { + public void display(GroupList groups) { + display(groups.asList(), null); + } + + public void display(List list, String toHighlight) { while (1 < table.getRowCount()) table.removeRow(table.getRowCount() - 1); - List list = groups.values().asList(); Collections.sort(list, new Comparator() { @Override public int compare(GroupInfo a, GroupInfo b) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupList.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupList.java new file mode 100644 index 0000000000..f838c37596 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupList.java @@ -0,0 +1,29 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.client.groups; + +import com.google.gerrit.client.rpc.NativeList; +import com.google.gerrit.client.rpc.RestApi; +import com.google.gwt.user.client.rpc.AsyncCallback; + +/** Groups available from {@code /groups/} or {@code /accounts/{id}/groups}. */ +public class GroupList extends NativeList { + public static void my(AsyncCallback callback) { + new RestApi("/accounts/self/groups").get(callback); + } + + protected GroupList() { + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupMap.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupMap.java index e3a984f06c..1d00b128b0 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupMap.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupMap.java @@ -14,7 +14,6 @@ package com.google.gerrit.client.groups; -import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.rpc.NativeMap; import com.google.gerrit.client.rpc.RestApi; import com.google.gwt.user.client.rpc.AsyncCallback; @@ -26,12 +25,6 @@ public class GroupMap extends NativeMap { .get(NativeMap.copyKeysIntoChildren(callback)); } - public static void my(AsyncCallback callback) { - new RestApi("/groups/") - .addParameter("user", Gerrit.getUserAccount().getId().get()) - .get(NativeMap.copyKeysIntoChildren(callback)); - } - public static void match(String match, AsyncCallback cb) { if (match == null || "".equals(match)) { all(cb); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java new file mode 100644 index 0000000000..1d71377320 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java @@ -0,0 +1,54 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.account; + +import com.google.common.collect.Lists; +import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.group.GroupInfo; +import com.google.inject.Inject; + +import java.util.List; + +class GetGroups implements RestReadView { + private final GroupControl.Factory groupControlFactory; + + @Inject + GetGroups(GroupControl.Factory groupControlFactory) { + this.groupControlFactory = groupControlFactory; + } + + @Override + public List apply(AccountResource resource) { + IdentifiedUser user = resource.getUser(); + Account.Id userId = user.getAccountId(); + List groups = Lists.newArrayList(); + for (AccountGroup.UUID uuid : user.getEffectiveGroups().getKnownGroups()) { + GroupControl ctl; + try { + ctl = groupControlFactory.controlFor(uuid); + } catch (NoSuchGroupException e) { + continue; + } + if (ctl.isVisible() && ctl.canSeeMember(userId)) { + groups.add(new GroupInfo(ctl.getGroup())); + } + } + return groups; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java index 928f5972cf..24140e5376 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java @@ -30,6 +30,7 @@ public class Module extends RestApiModule { DynamicMap.mapOf(binder(), CAPABILITY_KIND); child(ACCOUNT_KIND, "capabilities").to(Capabilities.class); + get(ACCOUNT_KIND, "groups").to(GetGroups.class); get(CAPABILITY_KIND).to(GetCapabilities.CheckOne.class); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java index aba56d3776..c6c6c76b4b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java @@ -14,50 +14,15 @@ 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.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.server.util.Url; class GetGroup implements RestReadView { - @Override public Object apply(GroupResource resource) throws AuthException, BadRequestException, ResourceConflictException, Exception { return new GroupInfo(resource.getControl().getGroup()); } - - public static class GroupInfo { - final String kind = "gerritcodereview#group"; - String id; - String name; - Boolean visibleToAll; - - // These fields are only supplied for internal groups. - String description; - Integer groupId; - String ownerId; - - GroupInfo(GroupDescription.Basic group) { - id = Url.encode(group.getGroupUUID().get()); - name = Strings.emptyToNull(group.getName()); - visibleToAll = group.isVisibleToAll() ? true : null; - - if (group instanceof GroupDescription.Internal) { - set(((GroupDescription.Internal) group).getAccountGroup()); - } - } - - private void set(AccountGroup d) { - description = Strings.emptyToNull(d.getDescription()); - groupId = d.getId().get(); - ownerId = d.getOwnerGroupUUID() != null - ? Url.encode(d.getOwnerGroupUUID().get()) - : null; - } - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java index 86879bdf7a..175ee77f9b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java @@ -15,11 +15,10 @@ package com.google.gerrit.server.group; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.group.GetGroup.GroupInfo; public class GetIncludedGroup implements RestReadView { @Override public GroupInfo apply(IncludedGroupResource rsrc) { - return new GetGroup.GroupInfo(rsrc.getGroup()); + return new GroupInfo(rsrc.getGroup()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java new file mode 100644 index 0000000000..bcefe81bb0 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java @@ -0,0 +1,50 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.group; + +import com.google.common.base.Strings; +import com.google.gerrit.common.data.GroupDescription; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.util.Url; + +public class GroupInfo { + final String kind = "gerritcodereview#group"; + public String id; + public String name; + Boolean visibleToAll; + + // These fields are only supplied for internal groups. + String description; + Integer groupId; + String ownerId; + + public GroupInfo(GroupDescription.Basic group) { + id = Url.encode(group.getGroupUUID().get()); + name = Strings.emptyToNull(group.getName()); + visibleToAll = group.isVisibleToAll() ? true : null; + + if (group instanceof GroupDescription.Internal) { + set(((GroupDescription.Internal) group).getAccountGroup()); + } + } + + private void set(AccountGroup d) { + description = Strings.emptyToNull(d.getDescription()); + groupId = d.getId().get(); + ownerId = d.getOwnerGroupUUID() != null + ? Url.encode(d.getOwnerGroupUUID().get()) + : null; + } +} \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java index a5b43bdfe5..7905f3ee7f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java @@ -28,7 +28,6 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.OutputFormat; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.VisibleGroups; -import com.google.gerrit.server.group.GetGroup.GroupInfo; import com.google.gerrit.server.ioutil.ColumnFormatter; import com.google.gerrit.server.project.ProjectControl; import com.google.gson.JsonElement; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java index 3924857401..afe5fd8afc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java @@ -24,7 +24,6 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.account.GroupControl; -import com.google.gerrit.server.group.GetGroup.GroupInfo; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -63,7 +62,7 @@ public class ListIncludedGroups implements RestReadView { try { GroupControl i = controlFactory.controlFor(u.getIncludeUUID()); if (ownerOfParent || i.isVisible()) { - included.add(new GetGroup.GroupInfo(i.getGroup())); + included.add(new GroupInfo(i.getGroup())); } } catch (NoSuchGroupException notFound) { log.warn(String.format("Group %s no longer available, included into ",