diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 2862287ce0..20fe52dcab 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -3,8 +3,8 @@ Gerrit Code Review - Access Controls Access controls in Gerrit are group based. Every user account is a member of one or more groups, and access and privileges are granted -to those groups. Groups cannot be nested, and access rights cannot -be granted to individual users. +to those groups. Access rights cannot be granted to individual +users. System Groups @@ -88,9 +88,6 @@ Account groups contain a list of zero or more user account members, added individually by a group owner. Any user account listed as a group member is given any access rights granted to the group. -To keep the schema simple to manage, groups cannot be nested. -Only individual user accounts can be added as a member. - Every group has one other group designated as its owner. Users who are members of the owner group can: diff --git a/Documentation/cmd-create-group.txt b/Documentation/cmd-create-group.txt index d5bc7578ee..1e46e14e19 100644 --- a/Documentation/cmd-create-group.txt +++ b/Documentation/cmd-create-group.txt @@ -12,6 +12,7 @@ SYNOPSIS [\--owner ] \ [\--description ] \ [\--member ] \ +[\--group ] \ DESCRIPTION @@ -50,6 +51,10 @@ Description values containing spaces should be quoted in single quotes User name to become initial member of the group. Multiple \--member options may be specified to add more initial members. +\--group:: + Group name to include in the group. Multiple \--group options may + be specified to include more initial groups. + EXAMPLES -------- Create a new account group called `gerritdev` with two initial members diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 7a116c4b02..bebce29de2 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -366,6 +366,11 @@ Gerrit group membership obtained from the `account_group_members` table is cached under the `"accounts"` cache, above. External group membership obtained from LDAP is cached under `"ldap_groups"`. +cache `"groups_byinclude"`:: ++ +Caches group inclusions in other groups. If direct updates are made +to the `account_group_includes` table, this cache should be flushed. + cache `"ldap_groups"`:: + Caches the LDAP groups that a user belongs to, if LDAP has been diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java index 445bdb4418..ce508cca86 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java @@ -16,6 +16,7 @@ package com.google.gerrit.common.data; import com.google.gerrit.common.auth.SignInRequired; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwtjsonrpc.client.RemoteJsonService; @@ -69,7 +70,15 @@ public interface GroupAdminService extends RemoteJsonService { void addGroupMember(AccountGroup.Id groupId, String nameOrEmail, AsyncCallback callback); + @SignInRequired + void addGroupInclude(AccountGroup.Id groupId, String groupName, + AsyncCallback callback); + @SignInRequired void deleteGroupMembers(AccountGroup.Id groupId, Set keys, AsyncCallback callback); + + @SignInRequired + void deleteGroupIncludes(AccountGroup.Id groupId, + Set keys, AsyncCallback callback); } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java index 5ddca26a40..69e535c34b 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java @@ -15,14 +15,17 @@ package com.google.gerrit.common.data; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import java.util.List; public class GroupDetail { public AccountInfoCache accounts; + public GroupInfoCache groups; public AccountGroup group; public List members; + public List includes; public AccountGroup ownerGroup; public boolean canModify; @@ -33,6 +36,10 @@ public class GroupDetail { accounts = c; } + public void setGroups(GroupInfoCache c) { + groups = c; + } + public void setGroup(AccountGroup g) { group = g; } @@ -41,6 +48,10 @@ public class GroupDetail { members = m; } + public void setIncludes(List i) { + includes = i; + } + public void setOwnerGroup(AccountGroup g) { ownerGroup = g; } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java new file mode 100644 index 0000000000..55577f27bd --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java @@ -0,0 +1,64 @@ +// Copyright (C) 2011 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.common.data; + +import com.google.gerrit.reviewdb.AccountGroup; + +/** Summary information about an {@link AccountGroup}, for simple tabular displays. */ +public class GroupInfo { + protected AccountGroup.Id id; + protected String name; + protected String description; + + protected GroupInfo() { + } + + /** + * Create an anonymous group info, when only the id is known. + *

+ * This constructor should only be a last-ditch effort, when the usual group + * lookup has failed and a stale group id has been discovered in the data + * store. + */ + public GroupInfo(final AccountGroup.Id id) { + this.id = id; + } + + /** + * Create a group description from a real data store record. + * + * @param a the data store record holding the specific group details. + */ + public GroupInfo(final AccountGroup a) { + id = a.getId(); + name = a.getName(); + description = a.getDescription(); + } + + /** @return the unique local id of the group */ + public AccountGroup.Id getId() { + return id; + } + + /** @return the name of the group; null if not supplied */ + public String getName() { + return name; + } + + /** @return the description of the group; null if not supplied */ + public String getDescription() { + return description; + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java new file mode 100644 index 0000000000..80d875648d --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java @@ -0,0 +1,79 @@ +// Copyright (C) 2011 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.common.data; + +import com.google.gerrit.reviewdb.AccountGroup; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** In-memory table of {@link GroupInfo}, indexed by {@link AccountGroup.Id}. */ +public class GroupInfoCache { + private static final GroupInfoCache EMPTY; + static { + EMPTY = new GroupInfoCache(); + EMPTY.groups = Collections.emptyMap(); + } + + /** Obtain an empty cache singleton. */ + public static GroupInfoCache empty() { + return EMPTY; + } + + protected Map groups; + + protected GroupInfoCache() { + } + + public GroupInfoCache(final Iterable list) { + groups = new HashMap(); + for (final GroupInfo gi : list) { + groups.put(gi.getId(), gi); + } + } + + /** + * Lookup the group summary + *

+ * The return value can take on one of three forms: + *

    + *
  • null, if id == null.
  • + *
  • a valid info block, if id was loaded.
  • + *
  • an anonymous info block, if id was not loaded.
  • + *
+ * + * @param id the id desired. + * @return info block for the group. + */ + public GroupInfo get(final AccountGroup.Id id) { + if (id == null) { + return null; + } + + GroupInfo r = groups.get(id); + if (r == null) { + r = new GroupInfo(id); + groups.put(id, r); + } + return r; + } + + /** Merge the information from another cache into this one. */ + public void merge(final GroupInfoCache other) { + assert this != EMPTY; + groups.putAll(other.groups); + } +} \ No newline at end of file diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java b/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java index f7115b9343..4e117b1bc8 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java @@ -37,4 +37,12 @@ public class NoSuchGroupException extends Exception { public NoSuchGroupException(final AccountGroup.NameKey k, final Throwable why) { super(MESSAGE + k.toString(), why); } + + public NoSuchGroupException(String who) { + this(who, null); + } + + public NoSuchGroupException(String who, final Throwable why) { + super(MESSAGE + who, why); + } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java index 981fee402e..56abefa4d6 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java @@ -14,6 +14,7 @@ package com.google.gerrit.client.admin; +import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.ScreenLoadCallback; @@ -21,15 +22,20 @@ import com.google.gerrit.client.ui.AccountDashboardLink; import com.google.gerrit.client.ui.AccountGroupSuggestOracle; import com.google.gerrit.client.ui.AccountScreen; import com.google.gerrit.client.ui.AddMemberBox; +import com.google.gerrit.client.ui.AddIncludedGroupBox; import com.google.gerrit.client.ui.FancyFlexTable; +import com.google.gerrit.client.ui.Hyperlink; import com.google.gerrit.client.ui.OnEditEnabler; import com.google.gerrit.client.ui.RPCSuggestOracle; import com.google.gerrit.client.ui.SmallHeading; import com.google.gerrit.common.data.AccountInfoCache; import com.google.gerrit.common.data.GroupDetail; import com.google.gerrit.common.data.GroupOptions; +import com.google.gerrit.common.data.GroupInfo; +import com.google.gerrit.common.data.GroupInfoCache; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gwt.event.dom.client.ChangeEvent; import com.google.gwt.event.dom.client.ChangeHandler; @@ -59,7 +65,9 @@ import java.util.List; public class AccountGroupScreen extends AccountScreen { private final AccountGroup.Id groupId; private AccountInfoCache accounts = AccountInfoCache.empty(); + private GroupInfoCache groups = GroupInfoCache.empty(); private MemberTable members; + private IncludeTable includes; private NpTextBox groupNameTxt; private Button saveName; @@ -79,6 +87,10 @@ public class AccountGroupScreen extends AccountScreen { private AddMemberBox addMemberBox; private Button delMember; + private Panel includePanel; + private AddIncludedGroupBox addIncludeBox; + private Button delInclude; + private Panel externalPanel; private Label externalName; private NpTextBox externalNameFilter; @@ -122,6 +134,7 @@ public class AccountGroupScreen extends AccountScreen { initGroupOptions(); initGroupType(); initMemberList(); + initIncludeList(); initExternal(); } @@ -299,7 +312,7 @@ public class AccountGroupScreen extends AccountScreen { addMemberBox.addClickHandler(new ClickHandler() { @Override public void onClick(final ClickEvent event) { - doAddNew(); + doAddNewMember(); } }); @@ -321,6 +334,34 @@ public class AccountGroupScreen extends AccountScreen { add(memberPanel); } + private void initIncludeList() { + addIncludeBox = new AddIncludedGroupBox(); + + addIncludeBox.addClickHandler(new ClickHandler() { + @Override + public void onClick(final ClickEvent event) { + doAddNewInclude(); + } + }); + + includes = new IncludeTable(); + + delInclude = new Button(Util.C.buttonDeleteIncludedGroup()); + delInclude.addClickHandler(new ClickHandler() { + @Override + public void onClick(final ClickEvent event) { + includes.deleteChecked(); + } + }); + + includePanel = new FlowPanel(); + includePanel.add(new SmallHeading(Util.C.headingIncludedGroups())); + includePanel.add(addIncludeBox); + includePanel.add(includes); + includePanel.add(delInclude); + add(includePanel); + } + private void initExternal() { externalName = new Label(); @@ -366,6 +407,7 @@ public class AccountGroupScreen extends AccountScreen { typeSelect.setVisible(!system); saveType.setVisible(!system); memberPanel.setVisible(newType == AccountGroup.Type.INTERNAL); + includePanel.setVisible(newType == AccountGroup.Type.INTERNAL); externalPanel.setVisible(newType == AccountGroup.Type.LDAP); externalNameFilter.setText(groupNameTxt.getText()); @@ -489,7 +531,9 @@ public class AccountGroupScreen extends AccountScreen { switch (group.getType()) { case INTERNAL: accounts = result.accounts; + groups = result.groups; members.display(result.members); + includes.display(result.includes); break; case LDAP: @@ -503,7 +547,7 @@ public class AccountGroupScreen extends AccountScreen { visibleToAllCheckBox.setValue(group.isVisibleToAll()); } - void doAddNew() { + void doAddNewMember() { final String nameEmail = addMemberBox.getText(); if (nameEmail.length() == 0) { return; @@ -529,6 +573,32 @@ public class AccountGroupScreen extends AccountScreen { }); } + void doAddNewInclude() { + final String groupName = addIncludeBox.getText(); + if (groupName.length() == 0) { + return; + } + + addIncludeBox.setEnabled(false); + Util.GROUP_SVC.addGroupInclude(groupId, groupName, + new GerritCallback() { + public void onSuccess(final GroupDetail result) { + addIncludeBox.setEnabled(true); + addIncludeBox.setText(""); + if (result.groups != null && result.includes != null) { + groups.merge(result.groups); + includes.display(result.includes); + } + } + + @Override + public void onFailure(final Throwable caught) { + addIncludeBox.setEnabled(true); + super.onFailure(caught); + } + }); + } + private class MemberTable extends FancyFlexTable { private boolean enabled = true; @@ -613,4 +683,77 @@ public class AccountGroupScreen extends AccountScreen { setRowItem(row, k); } } + + private class IncludeTable extends FancyFlexTable { + IncludeTable() { + table.setText(0, 2, Util.C.columnGroupName()); + table.setText(0, 3, Util.C.columnGroupDescription()); + + final FlexCellFormatter fmt = table.getFlexCellFormatter(); + fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().iconHeader()); + fmt.addStyleName(0, 2, Gerrit.RESOURCES.css().dataHeader()); + fmt.addStyleName(0, 3, Gerrit.RESOURCES.css().dataHeader()); + } + + void deleteChecked() { + final HashSet keys = + new HashSet(); + for (int row = 1; row < table.getRowCount(); row++) { + final AccountGroupInclude k = getRowItem(row); + if (k != null && ((CheckBox) table.getWidget(row, 1)).getValue()) { + keys.add(k.getKey()); + } + } + if (!keys.isEmpty()) { + Util.GROUP_SVC.deleteGroupIncludes(groupId, keys, + new GerritCallback() { + public void onSuccess(final VoidResult result) { + for (int row = 1; row < table.getRowCount();) { + final AccountGroupInclude k = getRowItem(row); + if (k != null && keys.contains(k.getKey())) { + table.removeRow(row); + } else { + row++; + } + } + } + }); + } + } + + void insertMember(final AccountGroupInclude k) { + final int row = table.getRowCount(); + table.insertRow(row); + applyDataRowStyle(row); + populate(row, k); + } + + void display(final List result) { + while (1 < table.getRowCount()) + table.removeRow(table.getRowCount() - 1); + + for (final AccountGroupInclude k : result) { + final int row = table.getRowCount(); + table.insertRow(row); + applyDataRowStyle(row); + populate(row, k); + } + } + + void populate(final int row, final AccountGroupInclude k) { + AccountGroup.Id id = k.getIncludeId(); + GroupInfo group = groups.get(id); + table.setWidget(row, 1, new CheckBox()); + table.setWidget(row, 2, new Hyperlink(group.getName(), Dispatcher + .toAccountGroup(id))); + table.setText(row, 3, groups.get(id).getDescription()); + + final FlexCellFormatter fmt = table.getFlexCellFormatter(); + fmt.addStyleName(row, 1, Gerrit.RESOURCES.css().iconCell()); + fmt.addStyleName(row, 2, Gerrit.RESOURCES.css().dataCell()); + fmt.addStyleName(row, 3, Gerrit.RESOURCES.css().dataCell()); + + setRowItem(row, k); + } + } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java index 6738f5bb87..ca191c6470 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java @@ -22,6 +22,8 @@ public interface AdminConstants extends Constants { String defaultBranchName(); String defaultRevisionSpec(); + String buttonDeleteIncludedGroup(); + String buttonAddIncludedGroup(); String buttonDeleteGroupMembers(); String buttonAddGroupMember(); String buttonSaveDescription(); @@ -47,6 +49,7 @@ public interface AdminConstants extends Constants { String headingProjectOptions(); String headingGroupType(); String headingMembers(); + String headingIncludedGroups(); String headingExternalGroup(); String headingCreateGroup(); String headingAccessRights(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties index f7c27b6528..94bb87f6be 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties @@ -3,6 +3,8 @@ defaultAccountGroupName = Group Name defaultBranchName = Branch Name defaultRevisionSpec = Revision (Branch or SHA-1) +buttonDeleteIncludedGroup = Delete +buttonAddIncludedGroup = Add buttonDeleteGroupMembers = Delete buttonAddGroupMember = Add buttonRenameGroup = Rename Group @@ -28,6 +30,7 @@ headingDescription = Description headingProjectOptions = Project Options headingGroupType = Group Type headingMembers = Members +headingIncludedGroups = Included Groups headingExternalGroup = Selected External Group headingCreateGroup = Create New Group headingAccessRights = Access Rights diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java new file mode 100644 index 0000000000..02e2745f5d --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java @@ -0,0 +1,104 @@ +// Copyright (C) 2011 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.ui; + +import com.google.gerrit.client.admin.Util; +import com.google.gerrit.client.ui.HintTextBox; +import com.google.gerrit.client.ui.RPCSuggestOracle; +import com.google.gwt.event.dom.client.ClickEvent; +import com.google.gwt.event.dom.client.ClickHandler; +import com.google.gwt.event.dom.client.KeyCodes; +import com.google.gwt.event.dom.client.KeyPressEvent; +import com.google.gwt.event.dom.client.KeyPressHandler; +import com.google.gwt.event.logical.shared.SelectionEvent; +import com.google.gwt.event.logical.shared.SelectionHandler; +import com.google.gwt.user.client.ui.Button; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.FlowPanel; +import com.google.gwt.user.client.ui.SuggestBox; +import com.google.gwt.user.client.ui.SuggestOracle.Suggestion; + +public class AddIncludedGroupBox extends Composite { + private final FlowPanel addPanel; + private final Button addMember; + private final HintTextBox nameTxtBox; + private final SuggestBox nameTxt; + private boolean submitOnSelection; + + public AddIncludedGroupBox() { + addPanel = new FlowPanel(); + addMember = new Button(Util.C.buttonAddIncludedGroup()); + nameTxtBox = new HintTextBox(); + nameTxt = new SuggestBox(new RPCSuggestOracle( + new AccountGroupSuggestOracle()), nameTxtBox); + + nameTxtBox.setVisibleLength(50); + nameTxtBox.setHintText(Util.C.defaultAccountGroupName()); + nameTxtBox.addKeyPressHandler(new KeyPressHandler() { + @Override + public void onKeyPress(KeyPressEvent event) { + submitOnSelection = false; + + if (event.getCharCode() == KeyCodes.KEY_ENTER) { + if (nameTxt.isSuggestionListShowing()) { + submitOnSelection = true; + } else { + doAdd(); + } + } + } + }); + nameTxt.addSelectionHandler(new SelectionHandler() { + @Override + public void onSelection(SelectionEvent event) { + if (submitOnSelection) { + submitOnSelection = false; + doAdd(); + } + } + }); + + addPanel.add(nameTxt); + addPanel.add(addMember); + + initWidget(addPanel); + } + + public void setAddButtonText(final String text) { + addMember.setText(text); + } + + public void addClickHandler(final ClickHandler handler) { + addMember.addClickHandler(handler); + } + + public String getText() { + String s = nameTxtBox.getText(); + return s == null ? "" : s; + } + + public void setEnabled(boolean enabled) { + addMember.setEnabled(enabled); + nameTxtBox.setEnabled(enabled); + } + + public void setText(String text) { + nameTxtBox.setText(text); + } + + private void doAdd() { + addMember.fireEvent(new ClickEvent() {}); + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java index 6bb905f182..8f9d96edfe 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java @@ -87,8 +87,7 @@ public class BaseServiceImplementation { } } catch (Failure e) { if (e.getCause() instanceof NoSuchProjectException - || e.getCause() instanceof NoSuchChangeException - || e.getCause() instanceof NoSuchGroupException) { + || e.getCause() instanceof NoSuchChangeException) { callback.onFailure(new NoSuchEntityException()); } else { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java index e4db047566..d5cd0ff769 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java @@ -25,6 +25,8 @@ import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; +import java.util.Collections; + class CreateGroup extends Handler { interface Factory { CreateGroup create(String groupName); @@ -46,6 +48,6 @@ class CreateGroup extends Handler { public AccountGroup.Id call() throws OrmException, NameAlreadyUsedException { final PerformCreateGroup performCreateGroup = performCreateGroupFactory.create(); final Account.Id me = user.getAccountId(); - return performCreateGroup.createGroup(groupName, null, false, null, me); + return performCreateGroup.createGroup(groupName, null, false, null, Collections.singleton(me), null); } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java index 0b59ea2d05..b3918d3dad 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java @@ -25,6 +25,8 @@ import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.httpd.rpc.BaseServiceImplementation; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.AccountGroupIncludeAudit; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.ReviewDb; @@ -33,6 +35,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupControl; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.account.Realm; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwtjsonrpc.client.VoidResult; @@ -54,6 +57,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements private final AccountResolver accountResolver; private final Realm accountRealm; private final GroupCache groupCache; + private final GroupIncludeCache groupIncludeCache; private final GroupControl.Factory groupControlFactory; private final CreateGroup.Factory createGroupFactory; @@ -63,8 +67,10 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements @Inject GroupAdminServiceImpl(final Provider schema, final Provider currentUser, - final AccountCache accountCache, final AccountResolver accountResolver, - final Realm accountRealm, final GroupCache groupCache, + final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, + final AccountResolver accountResolver, final Realm accountRealm, + final GroupCache groupCache, final GroupControl.Factory groupControlFactory, final CreateGroup.Factory createGroupFactory, final RenameGroup.Factory renameGroupFactory, @@ -72,6 +78,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements super(schema, currentUser); this.identifiedUser = currentUser; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.accountResolver = accountResolver; this.accountRealm = accountRealm; this.groupCache = groupCache; @@ -203,8 +210,8 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements public void searchExternalGroups(final String searchFilter, final AsyncCallback> callback) { final ArrayList matches = - new ArrayList(accountRealm - .lookupGroups(searchFilter)); + new ArrayList( + accountRealm.lookupGroups(searchFilter)); Collections.sort(matches, new Comparator() { @Override public int compare(AccountGroup.ExternalNameKey a, @@ -229,7 +236,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements if (!a.isActive()) { throw new Failure(new InactiveAccountException(a.getFullName())); } - if (!control.canAdd(a.getId())) { + if (!control.canAddMember(a.getId())) { throw new Failure(new NoSuchEntityException()); } @@ -250,6 +257,38 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements }); } + public void addGroupInclude(final AccountGroup.Id groupId, + final String groupName, final AsyncCallback callback) { + run(callback, new Action() { + public GroupDetail run(ReviewDb db) throws OrmException, Failure, + NoSuchGroupException { + final GroupControl control = groupControlFactory.validateFor(groupId); + if (control.getAccountGroup().getType() != AccountGroup.Type.INTERNAL) { + throw new Failure(new NameAlreadyUsedException()); + } + + final AccountGroup a = findGroup(groupName); + if (!control.canAddGroup(a.getId())) { + throw new Failure(new NoSuchEntityException()); + } + + final AccountGroupInclude.Key key = + new AccountGroupInclude.Key(groupId, a.getId()); + AccountGroupInclude m = db.accountGroupIncludes().get(key); + if (m == null) { + m = new AccountGroupInclude(key); + db.accountGroupIncludesAudit().insert( + Collections.singleton(new AccountGroupIncludeAudit(m, + getAccountId()))); + db.accountGroupIncludes().insert(Collections.singleton(m)); + groupIncludeCache.evictInclude(a.getId()); + } + + return groupDetailFactory.create(groupId).call(); + } + }); + } + public void deleteGroupMembers(final AccountGroup.Id groupId, final Set keys, final AsyncCallback callback) { @@ -271,7 +310,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements for (final AccountGroupMember.Key k : keys) { final AccountGroupMember m = db.accountGroupMembers().get(k); if (m != null) { - if (!control.canRemove(m.getAccountId())) { + if (!control.canRemoveMember(m.getAccountId())) { throw new Failure(new NoSuchEntityException()); } @@ -304,6 +343,56 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements }); } + public void deleteGroupIncludes(final AccountGroup.Id groupId, + final Set keys, + final AsyncCallback callback) { + run(callback, new Action() { + public VoidResult run(final ReviewDb db) throws OrmException, + NoSuchGroupException, Failure { + final GroupControl control = groupControlFactory.validateFor(groupId); + if (control.getAccountGroup().getType() != AccountGroup.Type.INTERNAL) { + throw new Failure(new NameAlreadyUsedException()); + } + + for (final AccountGroupInclude.Key k : keys) { + if (!groupId.equals(k.getGroupId())) { + throw new Failure(new NoSuchEntityException()); + } + } + + final Account.Id me = getAccountId(); + for (final AccountGroupInclude.Key k : keys) { + final AccountGroupInclude m = + db.accountGroupIncludes().get(k); + if (m != null) { + if (!control.canRemoveGroup(m.getIncludeId())) { + throw new Failure(new NoSuchEntityException()); + } + + AccountGroupIncludeAudit audit = null; + for (AccountGroupIncludeAudit a : db + .accountGroupIncludesAudit().byGroupInclude( + m.getGroupId(), m.getIncludeId())) { + if (a.isActive()) { + audit = a; + break; + } + } + + if (audit != null) { + audit.removed(me); + db.accountGroupIncludesAudit().update( + Collections.singleton(audit)); + } + db.accountGroupIncludes().delete(Collections.singleton(m)); + groupIncludeCache.evictInclude(m.getIncludeId()); + } + } + return VoidResult.INSTANCE; + } + }); + } + private void assertAmGroupOwner(final ReviewDb db, final AccountGroup group) throws Failure { try { @@ -323,4 +412,14 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements } return r; } + + private AccountGroup findGroup(final String name) throws OrmException, + Failure { + final AccountGroup g = groupCache.get(new AccountGroup.NameKey(name)); + if (g == null) { + throw new Failure(new NoSuchGroupException(name)); + } + return g; + } + } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java index 28352a85df..da6ab65ef7 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java @@ -19,11 +19,13 @@ import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.account.AccountInfoCacheFactory; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupControl; +import com.google.gerrit.server.account.GroupInfoCacheFactory; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -42,6 +44,7 @@ class GroupDetailFactory extends Handler { private final GroupControl.Factory groupControl; private final GroupCache groupCache; private final AccountInfoCacheFactory aic; + private final GroupInfoCacheFactory gic; private final AccountGroup.Id groupId; private GroupControl control; @@ -50,11 +53,13 @@ class GroupDetailFactory extends Handler { GroupDetailFactory(final ReviewDb db, final GroupControl.Factory groupControl, final GroupCache groupCache, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, + final GroupInfoCacheFactory.Factory groupInfoCacheFactory, @Assisted final AccountGroup.Id groupId) { this.db = db; this.groupControl = groupControl; this.groupCache = groupCache; this.aic = accountInfoCacheFactory.create(); + this.gic = groupInfoCacheFactory.create(); this.groupId = groupId; } @@ -69,17 +74,19 @@ class GroupDetailFactory extends Handler { switch (group.getType()) { case INTERNAL: detail.setMembers(loadMembers()); + detail.setIncludes(loadIncludes()); break; } detail.setAccounts(aic.create()); detail.setCanModify(control.isOwner()); + detail.setGroups(gic.create()); return detail; } private List loadMembers() throws OrmException { List members = new ArrayList(); for (final AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) { - if (control.canSee(m.getAccountId())) { + if (control.canSeeMember(m.getAccountId())) { aic.want(m.getAccountId()); members.add(m); } @@ -109,4 +116,40 @@ class GroupDetailFactory extends Handler { }); return members; } + + private List loadIncludes() throws OrmException { + List groups = new ArrayList(); + + for (final AccountGroupInclude m : db.accountGroupIncludes().byGroup(groupId)) { + if (control.canSeeGroup(m.getIncludeId())) { + gic.want(m.getIncludeId()); + groups.add(m); + } + } + + Collections.sort(groups, new Comparator() { + public int compare(final AccountGroupInclude o1, + final AccountGroupInclude o2) { + final AccountGroup a = gic.get(o1.getIncludeId()); + final AccountGroup b = gic.get(o2.getIncludeId()); + return n(a).compareTo(n(b)); + } + + private String n(final AccountGroup a) { + String n = a.getName(); + if (n != null && n.length() > 0) { + return n; + } + + n = a.getDescription(); + if (n != null && n.length() > 0) { + return n; + } + + return a.getId().toString(); + } + }); + + return groups; + } } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java new file mode 100644 index 0000000000..86b0966f94 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java @@ -0,0 +1,81 @@ +// Copyright (C) 2011 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.reviewdb; + +import com.google.gwtorm.client.Column; +import com.google.gwtorm.client.CompoundKey; + +/** Membership of an {@link AccountGroup} in an {@link AccountGroup}. */ +public final class AccountGroupInclude { + public static class Key extends CompoundKey { + private static final long serialVersionUID = 1L; + + @Column(id = 1) + protected AccountGroup.Id groupId; + + @Column(id = 2) + protected AccountGroup.Id includeId; + + protected Key() { + groupId = new AccountGroup.Id(); + includeId = new AccountGroup.Id(); + } + + public Key(final AccountGroup.Id g, final AccountGroup.Id i) { + groupId = g; + includeId = i; + } + + @Override + public AccountGroup.Id getParentKey() { + return groupId; + } + + public AccountGroup.Id getGroupId() { + return groupId; + } + + public AccountGroup.Id getIncludeId() { + return includeId; + } + + @Override + public com.google.gwtorm.client.Key[] members() { + return new com.google.gwtorm.client.Key[] {includeId}; + } + } + + @Column(id = 1, name = Column.NONE) + protected Key key; + + protected AccountGroupInclude() { + } + + public AccountGroupInclude(final AccountGroupInclude.Key k) { + key = k; + } + + public AccountGroupInclude.Key getKey() { + return key; + } + + public AccountGroup.Id getGroupId() { + return key.groupId; + } + + public AccountGroup.Id getIncludeId() { + return key.includeId; + } +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java new file mode 100644 index 0000000000..4881635e9d --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java @@ -0,0 +1,33 @@ +// Copyright (C) 2011 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.reviewdb; + +import com.google.gwtorm.client.Access; +import com.google.gwtorm.client.OrmException; +import com.google.gwtorm.client.PrimaryKey; +import com.google.gwtorm.client.Query; +import com.google.gwtorm.client.ResultSet; + +public interface AccountGroupIncludeAccess extends + Access { + @PrimaryKey("key") + AccountGroupInclude get(AccountGroupInclude.Key key) throws OrmException; + + @Query("WHERE key.includeId = ?") + ResultSet byInclude(AccountGroup.Id id) throws OrmException; + + @Query("WHERE key.groupId = ?") + ResultSet byGroup(AccountGroup.Id id) throws OrmException; +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java new file mode 100644 index 0000000000..e99e48094d --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java @@ -0,0 +1,97 @@ +// Copyright (C) 2011 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.reviewdb; + +import com.google.gwtorm.client.Column; +import com.google.gwtorm.client.CompoundKey; + +import java.sql.Timestamp; + +/** Inclusion of an {@link AccountGroup} in another {@link AccountGroup}. */ +public final class AccountGroupIncludeAudit { + public static class Key extends CompoundKey { + private static final long serialVersionUID = 1L; + + @Column(id = 1) + protected AccountGroup.Id groupId; + + @Column(id = 2) + protected AccountGroup.Id includeId; + + @Column(id = 3) + protected Timestamp addedOn; + + protected Key() { + groupId = new AccountGroup.Id(); + includeId = new AccountGroup.Id(); + } + + public Key(final AccountGroup.Id g, final AccountGroup.Id i, final Timestamp t) { + groupId = g; + includeId = i; + addedOn = t; + } + + @Override + public AccountGroup.Id getParentKey() { + return groupId; + } + + @Override + public com.google.gwtorm.client.Key[] members() { + return new com.google.gwtorm.client.Key[] {includeId}; + } + } + + @Column(id = 1, name = Column.NONE) + protected Key key; + + @Column(id = 2) + protected Account.Id addedBy; + + @Column(id = 3, notNull = false) + protected Account.Id removedBy; + + @Column(id = 4, notNull = false) + protected Timestamp removedOn; + + protected AccountGroupIncludeAudit() { + } + + public AccountGroupIncludeAudit(final AccountGroupInclude m, + final Account.Id adder) { + final AccountGroup.Id group = m.getGroupId(); + final AccountGroup.Id include = m.getIncludeId(); + key = new AccountGroupIncludeAudit.Key(group, include, now()); + addedBy = adder; + } + + public AccountGroupIncludeAudit.Key getKey() { + return key; + } + + public boolean isActive() { + return removedOn == null; + } + + public void removed(final Account.Id deleter) { + removedBy = deleter; + removedOn = now(); + } + + private static Timestamp now() { + return new Timestamp(System.currentTimeMillis()); + } +} \ No newline at end of file diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java new file mode 100644 index 0000000000..55b50e8393 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java @@ -0,0 +1,32 @@ +// Copyright (C) 2011 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.reviewdb; + +import com.google.gwtorm.client.Access; +import com.google.gwtorm.client.OrmException; +import com.google.gwtorm.client.PrimaryKey; +import com.google.gwtorm.client.Query; +import com.google.gwtorm.client.ResultSet; + +public interface AccountGroupIncludeAuditAccess extends + Access { + @PrimaryKey("key") + AccountGroupIncludeAudit get(AccountGroupIncludeAudit.Key key) + throws OrmException; + + @Query("WHERE key.groupId = ? AND key.includeId = ?") + ResultSet byGroupInclude(AccountGroup.Id groupId, + AccountGroup.Id incGroupId) throws OrmException; +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java index f9b3cfae1e..49e07ff756 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java @@ -72,6 +72,12 @@ public interface ReviewDb extends Schema { @Relation AccountGroupMemberAuditAccess accountGroupMembersAudit(); + @Relation + AccountGroupIncludeAccess accountGroupIncludes(); + + @Relation + AccountGroupIncludeAuditAccess accountGroupIncludesAudit(); + @Relation AccountGroupAgreementAccess accountGroupAgreements(); diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql index 0d41729425..d33d24d210 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql @@ -44,6 +44,13 @@ CREATE INDEX account_group_members_byGroup ON account_group_members (group_id); +-- ********************************************************************* +-- AccountGroupIncludeAccess +-- @PrimaryKey covers: byGroup +CREATE INDEX account_group_includes_byInclude +ON account_group_includes (include_id); + + -- ********************************************************************* -- AccountProjectWatchAccess -- @PrimaryKey covers: byAccount diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql index b44351cd77..cb8196e806 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql @@ -83,6 +83,13 @@ CREATE INDEX account_group_members_byGroup ON account_group_members (group_id); +-- ********************************************************************* +-- AccountGroupIncludeAccess +-- @PrimaryKey covers: byGroup +CREATE INDEX account_group_includes_byInclude +ON account_group_includes (include_id); + + -- ********************************************************************* -- AccountProjectWatchAccess -- @PrimaryKey covers: byAccount diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java index 78cbed3bb8..e46957d1cb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java @@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.StarredChange; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.CanonicalWebUrl; @@ -46,7 +47,9 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; import java.util.Set; import java.util.TimeZone; @@ -61,15 +64,18 @@ public class IdentifiedUser extends CurrentUser { private final Provider canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; @Inject GenericFactory(final AuthConfig authConfig, final @CanonicalWebUrl Provider canonicalUrl, - final Realm realm, final AccountCache accountCache) { + final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache) { this.authConfig = authConfig; this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; } public IdentifiedUser create(final Account.Id id) { @@ -78,13 +84,13 @@ public class IdentifiedUser extends CurrentUser { public IdentifiedUser create(Provider db, Account.Id id) { return new IdentifiedUser(AccessPath.UNKNOWN, authConfig, canonicalUrl, - realm, accountCache, null, db, id); + realm, accountCache, groupIncludeCache, null, db, id); } public IdentifiedUser create(AccessPath accessPath, Provider remotePeerProvider, Account.Id id) { return new IdentifiedUser(accessPath, authConfig, canonicalUrl, realm, - accountCache, remotePeerProvider, null, id); + accountCache, groupIncludeCache, remotePeerProvider, null, id); } } @@ -100,6 +106,7 @@ public class IdentifiedUser extends CurrentUser { private final Provider canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; private final Provider remotePeerProvider; private final Provider dbProvider; @@ -108,6 +115,7 @@ public class IdentifiedUser extends CurrentUser { RequestFactory(final AuthConfig authConfig, final @CanonicalWebUrl Provider canonicalUrl, final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, final @RemotePeer Provider remotePeerProvider, final Provider dbProvider) { @@ -115,6 +123,7 @@ public class IdentifiedUser extends CurrentUser { this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.remotePeerProvider = remotePeerProvider; this.dbProvider = dbProvider; @@ -123,7 +132,7 @@ public class IdentifiedUser extends CurrentUser { public IdentifiedUser create(final AccessPath accessPath, final Account.Id id) { return new IdentifiedUser(accessPath, authConfig, canonicalUrl, realm, - accountCache, remotePeerProvider, dbProvider, id); + accountCache, groupIncludeCache, remotePeerProvider, dbProvider, id); } } @@ -133,6 +142,7 @@ public class IdentifiedUser extends CurrentUser { private final Provider canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; @Nullable private final Provider remotePeerProvider; @@ -151,12 +161,14 @@ public class IdentifiedUser extends CurrentUser { private IdentifiedUser(final AccessPath accessPath, final AuthConfig authConfig, final Provider canonicalUrl, final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, @Nullable final Provider remotePeerProvider, @Nullable final Provider dbProvider, final Account.Id id) { super(accessPath, authConfig); this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.remotePeerProvider = remotePeerProvider; this.dbProvider = dbProvider; this.accountId = id; @@ -207,14 +219,35 @@ public class IdentifiedUser extends CurrentUser { @Override public Set getEffectiveGroups() { if (effectiveGroups == null) { - if (authConfig.isIdentityTrustable(state().getExternalIds())) { - effectiveGroups = realm.groups(state()); + Set seedGroups; + if (authConfig.isIdentityTrustable(state().getExternalIds())) { + seedGroups = realm.groups(state()); } else { - effectiveGroups = authConfig.getRegisteredGroups(); + seedGroups = authConfig.getRegisteredGroups(); + } + + effectiveGroups = getIncludedGroups(seedGroups); + } + + return effectiveGroups; + } + + private Set getIncludedGroups(Set seedGroups) { + Set includes = new HashSet (seedGroups); + Queue groupQueue = new LinkedList (seedGroups); + + while (groupQueue.size() > 0) { + AccountGroup.Id id = groupQueue.remove(); + + for (final AccountGroup.Id groupId : groupIncludeCache.getByInclude(id)) { + if (includes.add(groupId)) { + groupQueue.add(groupId); + } } } - return effectiveGroups; + + return Collections.unmodifiableSet(includes); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java index 20f9ec22df..602b59338a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java @@ -83,15 +83,27 @@ public class GroupControl { || getCurrentUser().isAdministrator(); } - public boolean canAdd(final Account.Id id) { + public boolean canAddMember(final Account.Id id) { return isOwner(); } - public boolean canRemove(final Account.Id id) { + public boolean canRemoveMember(final Account.Id id) { return isOwner(); } - public boolean canSee(Account.Id id) { + public boolean canSeeMember(Account.Id id) { + return isVisible(); + } + + public boolean canAddGroup(final AccountGroup.Id id) { + return isOwner(); + } + + public boolean canRemoveGroup(final AccountGroup.Id id) { + return isOwner(); + } + + public boolean canSeeGroup(AccountGroup.Id id) { return isVisible(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java new file mode 100644 index 0000000000..308880638b --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java @@ -0,0 +1,27 @@ +// Copyright (C) 2011 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.gerrit.reviewdb.AccountGroup; + +import java.util.Collection; + +/** Tracks group inclusions in memory for efficient access. */ +public interface GroupIncludeCache { + public Collection getByInclude(AccountGroup.Id groupId); + + public void evictInclude(AccountGroup.Id groupId); +} + diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java new file mode 100644 index 0000000000..76e92319f0 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java @@ -0,0 +1,98 @@ +// Copyright (C) 2011 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.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.cache.Cache; +import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.cache.EntryCreator; +import com.google.gwtorm.client.SchemaFactory; + +import com.google.inject.Inject; +import com.google.inject.Module; +import com.google.inject.Singleton; +import com.google.inject.TypeLiteral; +import com.google.inject.name.Named; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; + +/** Tracks group inclusions in memory for efficient access. */ +@Singleton +public class GroupIncludeCacheImpl implements GroupIncludeCache { + private static final String BYINCLUDE_NAME = "groups_byinclude"; + + public static Module module() { + return new CacheModule() { + @Override + protected void configure() { + final TypeLiteral>> byInclude = + new TypeLiteral>>() {}; + core(byInclude, BYINCLUDE_NAME).populateWith(ByIncludeLoader.class); + + bind(GroupIncludeCacheImpl.class); + bind(GroupIncludeCache.class).to(GroupIncludeCacheImpl.class); + } + }; + } + + private final Cache> byInclude; + + @Inject + GroupIncludeCacheImpl( + @Named(BYINCLUDE_NAME) Cache> byInclude) { + this.byInclude = byInclude; + } + + public Collection getByInclude(final AccountGroup.Id groupId) { + return byInclude.get(groupId); + } + + public void evictInclude(AccountGroup.Id groupId) { + byInclude.remove(groupId); + } + + static class ByIncludeLoader extends EntryCreator> { + private final SchemaFactory schema; + + @Inject + ByIncludeLoader(final SchemaFactory sf) { + schema = sf; + } + + @Override + public Collection createEntry(final AccountGroup.Id key) throws Exception { + final ReviewDb db = schema.open(); + try { + ArrayList groupArray = new ArrayList (); + for (AccountGroupInclude agi : db.accountGroupIncludes().byInclude(key)) { + groupArray.add(agi.getGroupId()); + } + + return Collections.unmodifiableCollection(groupArray); + } finally { + db.close(); + } + } + + @Override + public Collection missing(final AccountGroup.Id key) { + return Collections.emptyList(); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java new file mode 100644 index 0000000000..a3e592f036 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java @@ -0,0 +1,75 @@ +// Copyright (C) 2011 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.gerrit.common.data.GroupInfo; +import com.google.gerrit.common.data.GroupInfoCache; +import com.google.gerrit.reviewdb.AccountGroup; +import com.google.inject.Inject; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** Efficiently builds a {@link GroupInfoCache}. */ +public class GroupInfoCacheFactory { + public interface Factory { + GroupInfoCacheFactory create(); + } + + private final GroupCache groupCache; + private final Map out; + + @Inject + GroupInfoCacheFactory(final GroupCache groupCache) { + this.groupCache = groupCache; + this.out = new HashMap(); + } + + /** + * Indicate a group will be needed later on. + * + * @param id identity that will be needed in the future; may be null. + */ + public void want(final AccountGroup.Id id) { + if (id != null && !out.containsKey(id)) { + out.put(id, groupCache.get(id)); + } + } + + /** Indicate one or more groups will be needed later on. */ + public void want(final Iterable ids) { + for (final AccountGroup.Id id : ids) { + want(id); + } + } + + public AccountGroup get(final AccountGroup.Id id) { + want(id); + return out.get(id); + } + + /** + * Create an GroupInfoCache with the currently loaded AccountGroup entities. + * */ + public GroupInfoCache create() { + final List r = new ArrayList(out.size()); + for (final AccountGroup a : out.values()) { + r.add(new GroupInfo(a)); + } + return new GroupInfoCache(r); + } +} \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java index 8e5d274d0f..aab1cda276 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java @@ -17,6 +17,8 @@ package com.google.gerrit.server.account; import com.google.gerrit.common.errors.NameAlreadyUsedException; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.AccountGroupIncludeAudit; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.AccountGroupName; @@ -27,6 +29,7 @@ import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -38,13 +41,15 @@ public class PerformCreateGroup { private final ReviewDb db; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; private final IdentifiedUser currentUser; @Inject PerformCreateGroup(final ReviewDb db, final AccountCache accountCache, - final IdentifiedUser currentUser) { + final GroupIncludeCache groupIncludeCache, final IdentifiedUser currentUser) { this.db = db; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.currentUser = currentUser; } @@ -60,6 +65,7 @@ public class PerformCreateGroup { * @param ownerGroupId the group that should own the new group, if * null the new group will own itself * @param initialMembers initial members to be added to the new group + * @param initialGroups initial groups to include in the new group * @return the id of the new group * @throws OrmException is thrown in case of any data store read or write * error @@ -68,7 +74,9 @@ public class PerformCreateGroup { */ public AccountGroup.Id createGroup(final String groupName, final String groupDescription, final boolean visibleToAll, - final AccountGroup.Id ownerGroupId, final Account.Id... initialMembers) + final AccountGroup.Id ownerGroupId, + final Collection initialMembers, + final Collection initialGroups) throws OrmException, NameAlreadyUsedException { final AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId()); @@ -93,11 +101,15 @@ public class PerformCreateGroup { addMembers(groupId, initialMembers); + if (initialGroups != null) { + addGroups(groupId, initialGroups); + } + return groupId; } private void addMembers(final AccountGroup.Id groupId, - final Account.Id... members) throws OrmException { + final Collection members) throws OrmException { final List memberships = new ArrayList(); final List membershipsAudit = @@ -119,4 +131,26 @@ public class PerformCreateGroup { } } + private void addGroups(final AccountGroup.Id groupId, + final Collection groups) throws OrmException { + final List includeList = + new ArrayList(); + final List includesAudit = + new ArrayList(); + for (AccountGroup.Id includeId : groups) { + final AccountGroupInclude groupInclude = + new AccountGroupInclude(new AccountGroupInclude.Key(groupId, includeId)); + includeList.add(groupInclude); + + final AccountGroupIncludeAudit audit = + new AccountGroupIncludeAudit(groupInclude, currentUser.getAccountId()); + includesAudit.add(audit); + } + db.accountGroupIncludes().insert(includeList); + db.accountGroupIncludesAudit().insert(includesAudit); + + for (AccountGroup.Id includeId : groups) { + groupIncludeCache.evictInclude(includeId); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index f77550e446..df10c27052 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -35,6 +35,8 @@ import com.google.gerrit.server.account.AccountInfoCacheFactory; import com.google.gerrit.server.account.DefaultRealm; import com.google.gerrit.server.account.EmailExpander; import com.google.gerrit.server.account.GroupCacheImpl; +import com.google.gerrit.server.account.GroupInfoCacheFactory; +import com.google.gerrit.server.account.GroupIncludeCacheImpl; import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.auth.ldap.LdapModule; import com.google.gerrit.server.cache.CachePool; @@ -153,11 +155,13 @@ public class GerritGlobalModule extends FactoryModule { install(AccountByEmailCacheImpl.module()); install(AccountCacheImpl.module()); install(GroupCacheImpl.module()); + install(GroupIncludeCacheImpl.module()); install(PatchListCacheImpl.module()); install(ProjectCacheImpl.module()); install(new AccessControlModule()); factory(AccountInfoCacheFactory.Factory.class); + factory(GroupInfoCacheFactory.Factory.class); factory(ProjectState.Factory.class); factory(RefControl.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index fb5f1f8286..ecd446d80d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - private static final Class C = Schema_50.class; + private static final Class C = Schema_51.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java new file mode 100644 index 0000000000..d1111601c9 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java @@ -0,0 +1,41 @@ +// Copyright (C) 2011 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.schema; + +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gwtorm.jdbc.JdbcSchema; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.sql.SQLException; +import java.sql.Statement; + +public class Schema_51 extends SchemaVersion { + @Inject + Schema_51(Provider prior) { + super(prior); + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) throws SQLException { + Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + try { + stmt.execute("CREATE INDEX account_group_includes_byInclude" + + " ON account_group_includes (include_id)"); + } finally { + stmt.close(); + } + } +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java index b239fab9ea..36e064b44c 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java @@ -58,6 +58,13 @@ public class AdminCreateGroup extends BaseCommand { @Option(name = "--visible-to-all", usage = "to make the group visible to all registered users") private boolean visibleToAll; + private final Set initialGroups = new HashSet(); + + @Option(name = "--group", aliases = "-g", metaVar = "GROUP", usage = "initial set of groups to be included in the group") + void addGroup(final AccountGroup.Id id) { + initialGroups.add(id); + } + @Inject private PerformCreateGroup.Factory performCreateGroupFactory; @@ -77,8 +84,7 @@ public class AdminCreateGroup extends BaseCommand { performCreateGroupFactory.create(); try { performCreateGroup.createGroup(groupName, groupDescription, visibleToAll, - ownerGroupId, - initialMembers.toArray(new Account.Id[initialMembers.size()])); + ownerGroupId, initialMembers, initialGroups); } catch (NameAlreadyUsedException e) { throw die(e); }