Skip to content

Commit

Permalink
Move new methods to a new interface in OCP
Browse files Browse the repository at this point in the history
This avoids breaking compatibility for group backends not based on
 ABackend abstract class.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
  • Loading branch information
come-nc committed Sep 5, 2023
1 parent be1e67c commit ebfebc8
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 37 deletions.
3 changes: 2 additions & 1 deletion apps/user_ldap/lib/Group_Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
namespace OCA\User_LDAP;

use OC\ServerNotAvailableException;
use OCP\Group\Backend\IBatchMethodsBackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\Group\Backend\IGetDisplayNameBackend;
use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Backend\INamedBackend;
use OCP\GroupInterface;

class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend {
class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend, IBatchMethodsBackend {
private $backends = [];
private ?Group_LDAP $refBackend = null;
private Helper $helper;
Expand Down
2 changes: 2 additions & 0 deletions lib/private/Group/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IAddToGroupBackend;
use OCP\Group\Backend\IBatchMethodsBackend;
use OCP\Group\Backend\ICountDisabledInGroup;
use OCP\Group\Backend\ICountUsersBackend;
use OCP\Group\Backend\ICreateGroupBackend;
Expand Down Expand Up @@ -61,6 +62,7 @@ class Database extends ABackend implements
IRemoveFromGroupBackend,
ISetDisplayNameBackend,
ISearchableGroupBackend,
IBatchMethodsBackend,
INamedBackend {
/** @var array<string, array{gid: string, displayname: string}> */
private $groupCache = [];
Expand Down
22 changes: 17 additions & 5 deletions lib/private/Group/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

use OC\Hooks\PublicEmitter;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Group\Backend\IBatchMethodsBackend;
use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Events\BeforeGroupCreatedEvent;
use OCP\Group\Events\GroupCreatedEvent;
Expand Down Expand Up @@ -212,7 +213,7 @@ protected function getGroupObject($gid, $displayName = null) {
* @param array<string, string> $displayNames Array containing already know display name for a groupId
* @return array<string, IGroup>
*/
protected function getGroupsObject(array $gids, array $displayNames = []): array {
protected function getGroupsObjects(array $gids, array $displayNames = []): array {
$backends = [];
$groups = [];
foreach ($gids as $gid) {
Expand All @@ -224,7 +225,14 @@ protected function getGroupsObject(array $gids, array $displayNames = []): array
foreach ($this->backends as $backend) {
if ($backend instanceof IGroupDetailsBackend || $backend->implementsActions(GroupInterface::GROUP_DETAILS)) {
/** @var IGroupDetailsBackend $backend */
$groupDatas = $backend->getGroupsDetails($gids);
if ($backend instanceof IBatchMethodsBackend) {
$groupDatas = $backend->getGroupsDetails($gids);
} else {
$groupDatas = [];
foreach ($gids as $gid) {
$groupDatas[$gid] = $backend->getGroupDetails($gid);
}
}
foreach ($groupDatas as $gid => $groupData) {
if (!empty($groupData)) {
// take the display name from the last backend that has a non-null one
Expand All @@ -235,15 +243,19 @@ protected function getGroupsObject(array $gids, array $displayNames = []): array
}
}
} else {
$existingGroups = $backend->groupsExists($gids);
if ($backend instanceof IBatchMethodsBackend) {
$existingGroups = $backend->groupsExists($gids);
} else {
$existingGroups = array_filter($gids, fn (string $gid): bool => $backend->groupExists($gid));
}
foreach ($existingGroups as $group) {
$backends[$group][] = $backend;
}
}
}
foreach ($gids as $gid) {
if (count($backends[$gid]) === 0) {
continue;
continue;
}
$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]);
$groups[$gid] = $this->cachedGroups[$gid];
Expand Down Expand Up @@ -295,7 +307,7 @@ public function search(string $search, ?int $limit = null, ?int $offset = 0) {
$groups = [];
foreach ($this->backends as $backend) {
$groupIds = $backend->getGroups($search, $limit ?? -1, $offset ?? 0);
$newGroups = $this->getGroupsObject($groupIds);
$newGroups = $this->getGroupsObjects($groupIds);
foreach ($newGroups as $groupId => $group) {
$groups[$groupId] = $group;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/public/Group/Backend/ABackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
/**
* @since 14.0.0
*/
abstract class ABackend implements GroupInterface {
abstract class ABackend implements GroupInterface, IBatchMethodsBackend {
/**
* @deprecated 14.0.0
* @since 14.0.0
Expand Down Expand Up @@ -68,7 +68,7 @@ public function implementsActions($actions): bool {
}

/**
* @since 26.0.0
* @since 28.0.0
*/
public function groupsExists(array $gids): array {
return array_values(array_filter(
Expand All @@ -78,7 +78,7 @@ public function groupsExists(array $gids): array {
}

/**
* @since 26.0.0
* @since 28.0.0
*/
public function getGroupsDetails(array $gids): array {

Check failure on line 83 in lib/public/Group/Backend/ABackend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidReturnType

lib/public/Group/Backend/ABackend.php:83:49: InvalidReturnType: The declared return type 'array<string, array{displayName: string}>' for OCP\Group\Backend\ABackend::getGroupsDetails is incorrect, got 'array<string, array{displayName?: string}>' (see https://psalm.dev/011)

Check failure on line 83 in lib/public/Group/Backend/ABackend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ocp

InvalidReturnType

lib/public/Group/Backend/ABackend.php:83:49: InvalidReturnType: The declared return type 'array<string, array{displayName: string}>' for OCP\Group\Backend\ABackend::getGroupsDetails is incorrect, got 'array<string, array{displayName?: string}>' (see https://psalm.dev/011)

Check failure

Code scanning / Psalm

InvalidReturnType Error

The declared return type 'array<string, array{displayName: string}>' for OCP\Group\Backend\ABackend::getGroupsDetails is incorrect, got 'array<string, array{displayName?: string}>'
if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) {
Expand Down
61 changes: 61 additions & 0 deletions lib/public/Group/Backend/IBatchMethodsBackend.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Carl Schwan <carl@carlschwan.eu>
* @author Côme Chilliet <come.chilliet@nextcloud.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCP\Group\Backend;

/**
* @brief Optional interface for group backends
* @since 28.0.0
*/
interface IBatchMethodsBackend {
/**
* @brief Batch method to check if a list of groups exists
*
* The default implementation in ABackend will just call groupExists in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @param list<string> $gids
* @return list<string> the list of group that exists
* @since 28.0.0
*/
public function groupsExists(array $gids): array;

/**
* @brief Batch method to get the group details of a list of groups
*
* The default implementation in ABackend will just call getGroupDetail in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @throw \RuntimeException if called on a backend that doesn't implements IGroupDetailsBackend
*
* @return array<string, array{displayName: string}>
* @since 28.0.0
*/
public function getGroupsDetails(array $gids): array;
}
15 changes: 0 additions & 15 deletions lib/public/Group/Backend/IGroupDetailsBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,4 @@ interface IGroupDetailsBackend {
* @since 14.0.0
*/
public function getGroupDetails(string $gid): array;


/**
* @brief Batch method to get the group details of a list of groups
*
* The default implementation in ABackend will just call getGroupDetail in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @throw \RuntimeException if called on a backend that doesn't implements IGroupDetailsBackend
*
* @return array<string, array{displayName: string}>
* @since 26.0.0
*/
public function getGroupsDetails(array $gids): array;
}
13 changes: 0 additions & 13 deletions lib/public/GroupInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,19 +107,6 @@ public function getGroups(string $search = '', int $limit = -1, int $offset = 0)
*/
public function groupExists($gid);

/**
* @brief Batch method to check if a list of groups exists
*
* The default implementation in ABackend will just call groupExists in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @param list<string> $gids
* @return list<string> the list of group that exists
* @since 25.0.0
*/
public function groupsExists(array $gids): array;

/**
* @brief Get a list of user ids in a group matching the given search parameters.
*
Expand Down

0 comments on commit ebfebc8

Please sign in to comment.