Include name of owner group into GroupInfo

The GroupInfo only contained the UUID of the owner group. Now in
addition the name of the owner group is included. This saves one
request when displaying the group info in the UI since there is no
need to fetch the name of the owner group with an own request. This
also fixes a wrong message in the UI. When the owner group was not
visible the name of the owner group could not be looked up and the UI
told it's a deleted reference.

Change-Id: I36082fb693a5b83bae26232a5de606d27956e220
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-02-11 12:39:13 +01:00
parent f807bc3836
commit c6993fb005
16 changed files with 157 additions and 100 deletions

View File

@@ -41,6 +41,7 @@ by group name.
},
"description": "Gerrit Site Administrators",
"group_id": 1,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
},
"Anonymous Users": {
@@ -51,6 +52,7 @@ by group name.
},
"description": "Any user, signed-in or not",
"group_id": 2,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
},
"MyProject_Committers": {
@@ -61,6 +63,7 @@ by group name.
"visible_to_all": true,
},
"group_id": 6,
"owner": "MyProject_Committers",
"owner_id": "834ec36dd5e0ed21a2ff5d7e2255da082d63bbd7"
},
"Non-Interactive Users": {
@@ -71,6 +74,7 @@ by group name.
},
"description": "Users who perform batch actions on Gerrit",
"group_id": 4,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
},
"Project Owners": {
@@ -81,6 +85,7 @@ by group name.
},
"description": "Any owner of the project",
"group_id": 5,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
},
"Registered Users": {
@@ -91,6 +96,7 @@ by group name.
},
"description": "Any signed-in user",
"group_id": 3,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
}
@@ -134,6 +140,7 @@ returned.
},
"description":"contains all committers for MyProject",
"group_id": 551,
"owner": "MyProject-Owners",
"owner_id": "7ca042f4d5847936fcb90ca91057673157fd06fc"
}
}
@@ -168,6 +175,7 @@ describes the group.
},
"description": "Gerrit Site Administrators",
"group_id": 1,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
----
@@ -188,6 +196,7 @@ link:#group-input[GroupInput].
{
"description": "contains all committers for MyProject",
"visible_to_all": true,
"owner": "MyProject-Owners",
"owner_id": "7ca042f4d5847936fcb90ca91057673157fd06fc"
}
----
@@ -212,6 +221,7 @@ describes the created group.
},
"description":"contains all committers for MyProject",
"group_id": 551,
"owner": "MyProject-Owners",
"owner_id": "7ca042f4d5847936fcb90ca91057673157fd06fc"
}
----
@@ -416,6 +426,7 @@ describes the owner group.
},
"description": "Gerrit Site Administrators",
"group_id": 1,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
----
@@ -459,6 +470,7 @@ describes the new owner group.
},
"description": "Gerrit Site Administrators",
"group_id": 1,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
----
@@ -752,6 +764,7 @@ The entries in the list are sorted by group name and UUID.
"options": {
},
"group_id": 38,
"owner": "MyProject-Verifiers",
"owner_id": "7ca042f4d5847936fcb90ca91057673157fd06fc"
}
]
@@ -785,6 +798,7 @@ describes the included group.
"options": {
},
"group_id": 38,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
----
@@ -817,6 +831,7 @@ describes the included group.
"options": {
},
"group_id": 8,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
}
----
@@ -872,6 +887,7 @@ group was already included in the group.
"options": {
},
"group_id": 8,
"owner": "Administrators",
"owner_id": "6a1e70e1a88782771a91808c8af9bbb7a9871389"
},
{
@@ -882,6 +898,7 @@ group was already included in the group.
"options": {
},
"group_id": 10,
"owner": "MyOtherGroup",
"owner_id": "5057f3cbd3519d6ab69364429a89ffdffba50f73"
}
]
@@ -984,6 +1001,7 @@ permits users to apply to join the group, or manage their membership.
|`options` ||link:#group-options-info[Options of the group]
|`description` |only for internal groups|The description of the group.
|`group_id` |only for internal groups|The numeric ID of the group.
|`owner` |only for internal groups|The name of the owner group.
|`owner_id` |only for internal groups|The URL encoded UUID of the owner group.
|===========================

View File

@@ -19,8 +19,6 @@ import com.google.gerrit.client.VoidResult;
import com.google.gerrit.client.groups.GroupApi;
import com.google.gerrit.client.groups.GroupInfo;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.client.ui.AccountGroupSuggestOracle;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.client.ui.RPCSuggestOracle;
@@ -216,22 +214,7 @@ public class AccountGroupInfoScreen extends AccountGroupScreen {
protected void display(final GroupInfo group, final boolean canModify) {
groupUUIDLabel.setText(group.getGroupUUID().get());
groupNameTxt.setText(group.name());
GroupApi.getGroupName(group.getOwnerUUID(), new GerritCallback<NativeString>() {
@Override
public void onSuccess(NativeString result) {
ownerTxt.setText(result.asString());
}
@Override
public void onFailure(Throwable caught) {
if (RestApi.isNotFound(caught)) {
ownerTxt.setText(Util.M.deletedReference(group.getOwnerUUID().get()));
} else {
super.onFailure(caught);
}
}
});
ownerTxt.setText(group.owner() != null?group.owner():Util.M.deletedReference(group.getOwnerUUID().get()));
descTxt.setText(group.description());
visibleToAllCheckBox.setValue(group.options().isVisibleToAll());
setMembersTabVisible(AccountGroup.isInternalGroup(group.getGroupUUID())

View File

@@ -70,6 +70,7 @@ public abstract class AccountGroupScreen extends MenuScreen {
protected void updateOwnerGroup(GroupInfo ownerGroup) {
group.setOwnerUUID(ownerGroup.getGroupUUID());
group.owner(ownerGroup.name());
}
protected AccountGroup.UUID getOwnerGroupUUID() {

View File

@@ -32,6 +32,8 @@ public class GroupInfo extends JavaScriptObject {
public final native GroupOptionsInfo options() /*-{ return this.options; }-*/;
public final native String description() /*-{ return this.description; }-*/;
public final native String url() /*-{ return this.url; }-*/;
public final native String owner() /*-{ return this.owner; }-*/;
public final native void owner(String o) /*-{ if(o)this.owner=o; }-*/;
private final native int group_id() /*-{ return this.group_id; }-*/;
private final native String owner_id() /*-{ return this.owner_id; }-*/;

View File

@@ -20,17 +20,20 @@ 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.gerrit.server.group.GroupJson;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.inject.Inject;
import java.util.List;
public class GetGroups implements RestReadView<AccountResource> {
private final GroupControl.Factory groupControlFactory;
private final GroupJson json;
@Inject
GetGroups(GroupControl.Factory groupControlFactory) {
GetGroups(GroupControl.Factory groupControlFactory, GroupJson json) {
this.groupControlFactory = groupControlFactory;
this.json = json;
}
@Override
@@ -46,7 +49,7 @@ public class GetGroups implements RestReadView<AccountResource> {
continue;
}
if (ctl.isVisible() && ctl.canSeeMember(userId)) {
groups.add(new GroupInfo(ctl.getGroup()));
groups.add(json.format(ctl.getGroup()));
}
}
return groups;

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupIncludeCache;
import com.google.gerrit.server.group.AddIncludedGroups.Input;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -67,14 +68,16 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
private final Provider<GroupsCollection> groupsCollection;
private final GroupIncludeCache groupIncludeCache;
private final ReviewDb db;
private final GroupJson json;
@Inject
public AddIncludedGroups(Provider<GroupsCollection> groupsCollection,
GroupIncludeCache groupIncludeCache,
ReviewDb db) {
ReviewDb db, GroupJson json) {
this.groupsCollection = groupsCollection;
this.groupIncludeCache = groupIncludeCache;
this.db = db;
this.json = json;
}
@Override
@@ -119,7 +122,7 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
newIncludedGroupsAudits.add(new AccountGroupIncludeByUuidAudit(agi, me));
}
}
result.add(new GroupInfo(d));
result.add(json.format(d));
}
badRequest.failOnError();

View File

@@ -34,6 +34,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PerformCreateGroup;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.CreateGroup.Input;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -59,19 +60,18 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
private final Provider<IdentifiedUser> self;
private final GroupsCollection groups;
private final PerformCreateGroup.Factory op;
private final GroupJson json;
private final boolean defaultVisibleToAll;
private final String name;
@Inject
CreateGroup(
Provider<IdentifiedUser> self,
GroupsCollection groups,
PerformCreateGroup.Factory performCreateGroupFactory,
@GerritServerConfig Config cfg,
@Assisted String name) {
CreateGroup(Provider<IdentifiedUser> self, GroupsCollection groups,
PerformCreateGroup.Factory performCreateGroupFactory, GroupJson json,
@GerritServerConfig Config cfg, @Assisted String name) {
this.self = self;
this.groups = groups;
this.op = performCreateGroupFactory;
this.json = json;
this.defaultVisibleToAll = cfg.getBoolean("groups", "newGroupsVisibleToAll", false);
this.name = name;
}
@@ -102,7 +102,7 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
} catch (PermissionDeniedException e) {
throw new AuthException(e.getMessage());
}
return new GroupInfo(GroupDescriptions.forAccountGroup(group));
return json.format(GroupDescriptions.forAccountGroup(group));
}
private AccountGroup.Id owner(Input input) throws BadRequestException {

View File

@@ -15,10 +15,19 @@
package com.google.gerrit.server.group;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.inject.Inject;
class GetGroup implements RestReadView<GroupResource> {
private final GroupJson json;
@Inject
GetGroup(GroupJson json) {
this.json = json;
}
@Override
public Object apply(GroupResource resource) {
return new GroupInfo(resource.getGroup());
public GroupInfo apply(GroupResource resource) {
return json.format(resource.getGroup());
}
}

View File

@@ -15,10 +15,19 @@
package com.google.gerrit.server.group;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.inject.Inject;
public class GetIncludedGroup implements RestReadView<IncludedGroupResource> {
private final GroupJson json;
@Inject
GetIncludedGroup(GroupJson json) {
this.json = json;
}
@Override
public GroupInfo apply(IncludedGroupResource rsrc) {
return new GroupInfo(rsrc.getMemberDescription());
return json.format(rsrc.getMemberDescription());
}
}

View File

@@ -19,26 +19,29 @@ 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.gerrit.server.group.GroupJson.GroupInfo;
import com.google.inject.Inject;
public class GetOwner implements RestReadView<GroupResource> {
private final GroupControl.Factory controlFactory;
private final GroupJson json;
@Inject
GetOwner(GroupControl.Factory controlFactory) {
GetOwner(GroupControl.Factory controlFactory, GroupJson json) {
this.controlFactory = controlFactory;
this.json = json;
}
@Override
public Object apply(GroupResource resource) throws ResourceNotFoundException {
public GroupInfo apply(GroupResource resource) throws ResourceNotFoundException {
AccountGroup group = resource.toAccountGroup();
if (group == null) {
throw new ResourceNotFoundException();
}
try {
GroupControl c = controlFactory.validateFor(group.getOwnerGroupUUID());
return new GroupInfo(c.getGroup());
return json.format(c.getGroup());
} catch (NoSuchGroupException e) {
throw new ResourceNotFoundException();
}

View File

@@ -1,54 +0,0 @@
// 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.common.data.GroupDescriptions;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
public class GroupInfo {
final String kind = "gerritcodereview#group";
public String id;
public String name;
public String url;
public GroupOptionsInfo options;
// These fields are only supplied for internal groups.
public String description;
public Integer groupId;
public String ownerId;
public GroupInfo(GroupDescription.Basic group) {
id = Url.encode(group.getGroupUUID().get());
name = Strings.emptyToNull(group.getName());
url = Strings.emptyToNull(group.getUrl());
options = new GroupOptionsInfo(group);
AccountGroup internalGroup = GroupDescriptions.toAccountGroup(group);
if (internalGroup != null) {
set(internalGroup);
}
}
private void set(AccountGroup d) {
description = Strings.emptyToNull(d.getDescription());
groupId = d.getId().get();
ownerId = d.getOwnerGroupUUID() != null
? Url.encode(d.getOwnerGroupUUID().get())
: null;
}
}

View File

@@ -0,0 +1,69 @@
// 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.common.data.GroupDescriptions;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.GroupCache;
import com.google.inject.Inject;
public class GroupJson {
private final GroupCache groupCache;
@Inject
GroupJson(GroupCache groupCache) {
this.groupCache = groupCache;
}
public GroupInfo format(GroupDescription.Basic group) {
GroupInfo info = new GroupInfo();
info.id = Url.encode(group.getGroupUUID().get());
info.name = Strings.emptyToNull(group.getName());
info.url = Strings.emptyToNull(group.getUrl());
info.options = new GroupOptionsInfo(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());
AccountGroup o = groupCache.get(g.getOwnerGroupUUID());
if (o != null) {
info.owner = o.getName();
}
}
}
return info;
}
public static class GroupInfo {
final String kind = "gerritcodereview#group";
public String id;
public String name;
public String url;
public GroupOptionsInfo options;
// These fields are only supplied for internal groups.
public String description;
public Integer groupId;
public String owner;
public String ownerId;
}
}

View File

@@ -37,6 +37,7 @@ import com.google.gerrit.server.account.GetGroups;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupComparator;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
@@ -61,6 +62,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
private final Provider<IdentifiedUser> identifiedUser;
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<GetGroups> accountGetGroups;
private final GroupJson json;
@Option(name = "--project", aliases = {"-p"},
usage = "projects for which the groups should be listed")
@@ -96,13 +98,14 @@ public class ListGroups implements RestReadView<TopLevelResource> {
final GroupControl.GenericFactory genericGroupControlFactory,
final Provider<IdentifiedUser> identifiedUser,
final IdentifiedUser.GenericFactory userFactory,
final Provider<GetGroups> accountGetGroups) {
final Provider<GetGroups> accountGetGroups, GroupJson json) {
this.groupCache = groupCache;
this.groupControlFactory = groupControlFactory;
this.genericGroupControlFactory = genericGroupControlFactory;
this.identifiedUser = identifiedUser;
this.userFactory = userFactory;
this.accountGetGroups = accountGetGroups;
this.json = json;
}
public Account.Id getUser() {
@@ -159,7 +162,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
}
groupInfos = Lists.newArrayListWithCapacity(groupList.size());
for (AccountGroup group : groupList) {
groupInfos.add(new GroupInfo(GroupDescriptions.forAccountGroup(group)));
groupInfos.add(json.format(GroupDescriptions.forAccountGroup(group)));
}
}
}
@@ -173,7 +176,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
try {
if (genericGroupControlFactory.controlFor(user, g.getGroupUUID())
.isOwner()) {
groups.add(new GroupInfo(ctl.getGroup()));
groups.add(json.format(ctl.getGroup()));
}
} catch (NoSuchGroupException e) {
continue;

View File

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
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.GroupJson.GroupInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -38,12 +39,14 @@ public class ListIncludedGroups implements RestReadView<GroupResource> {
private final GroupControl.Factory controlFactory;
private final Provider<ReviewDb> dbProvider;
private final GroupJson json;
@Inject
ListIncludedGroups(GroupControl.Factory controlFactory,
Provider<ReviewDb> dbProvider) {
Provider<ReviewDb> dbProvider, GroupJson json) {
this.controlFactory = controlFactory;
this.dbProvider = dbProvider;
this.json = json;
}
@Override
@@ -61,7 +64,7 @@ public class ListIncludedGroups implements RestReadView<GroupResource> {
try {
GroupControl i = controlFactory.controlFor(u.getIncludeUUID());
if (ownerOfParent || i.isVisible()) {
included.add(new GroupInfo(i.getGroup()));
included.add(json.format(i.getGroup()));
}
} catch (NoSuchGroupException notFound) {
log.warn(String.format("Group %s no longer available, included into ",

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.gerrit.server.group.PutOwner.Input;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -44,14 +45,16 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
private final GroupCache groupCache;
private final GroupControl.Factory controlFactory;
private final ReviewDb db;
private final GroupJson json;
@Inject
PutOwner(Provider<GroupsCollection> groupsCollection, GroupCache groupCache,
GroupControl.Factory controlFactory, ReviewDb db) {
GroupControl.Factory controlFactory, ReviewDb db, GroupJson json) {
this.groupsCollection = groupsCollection;
this.groupCache = groupCache;
this.controlFactory = controlFactory;
this.db = db;
this.json = json;
}
@Override
@@ -88,7 +91,7 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
db.accountGroups().update(Collections.singleton(group));
groupCache.evict(group);
}
return new GroupInfo(c.getGroup());
return json.format(c.getGroup());
} catch (NoSuchGroupException e) {
throw new BadRequestException(String.format("No such group: %s", input.owner));
}

View File

@@ -23,7 +23,8 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GetGroups;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.group.GroupInfo;
import com.google.gerrit.server.group.GroupJson;
import com.google.gerrit.server.group.GroupJson.GroupInfo;
import com.google.gerrit.server.group.ListGroups;
import com.google.gerrit.server.ioutil.ColumnFormatter;
import com.google.gerrit.sshd.BaseCommand;
@@ -73,9 +74,10 @@ public class ListGroupsCommand extends BaseCommand {
final GroupControl.GenericFactory genericGroupControlFactory,
final Provider<IdentifiedUser> identifiedUser,
final IdentifiedUser.GenericFactory userFactory,
final Provider<GetGroups> accountGetGroups) {
final Provider<GetGroups> accountGetGroups,
final GroupJson json) {
super(groupCache, groupControlFactory, genericGroupControlFactory,
identifiedUser, userFactory, accountGetGroups);
identifiedUser, userFactory, accountGetGroups, json);
}
void display(final PrintWriter out) throws NoSuchGroupException {