Merge pull request #92 from owncloud/delete-group-through-manager

Go through GroupManager for group deletion

lib/CustomGroupsBackend.php

@@ -53,7 +53,7 @@ public function __construct(
 	 * @return boolean
 	 */
 	public function implementsActions($actions) {
-		return ($actions & self::GROUP_DETAILS) !== 0;
+		return ($actions & (self::GROUP_DETAILS | self::DELETE_GROUP)) !== 0;
 	}
 
 	/**
@@ -204,4 +204,19 @@ private function formatGroupId($uri) {
 	public function isVisibleForScope($scope) {
 		return ($scope === 'sharing');
 	}
+
+	/**
+	 * Delete group
+	 *
+	 * @param string $gid group id
+	 */
+	public function deleteGroup($gid) {
+		$uri = $this->extractUri($gid);
+		if ($uri !== null) {
+			$groupInfo = $this->handler->getGroupByUri($uri);
+			if ($groupInfo !== null) {
+				$this->handler->deleteGroup($groupInfo['group_id']);
+			}
+		}
+	}
 }

lib/Dav/GroupMembershipCollection.php

@@ -31,6 +31,7 @@
 use OCA\CustomGroups\Search;
 use OCA\CustomGroups\Service\MembershipHelper;
 use Symfony\Component\EventDispatcher\GenericEvent;
+use OCP\IGroupManager;
 
 /**
  * Group memberships collection for a given group
@@ -56,6 +57,13 @@ class GroupMembershipCollection implements \Sabre\DAV\ICollection, \Sabre\DAV\IP
 	 */
 	private $helper;
 
+	/**
+	 * Group manager from core
+	 *
+	 * @var IGroupManager
+	 */
+	private $groupManager;
+
 	/**
 	 * Group information
 	 *
@@ -77,10 +85,12 @@ class GroupMembershipCollection implements \Sabre\DAV\ICollection, \Sabre\DAV\IP
 	 */
 	public function __construct(
 		array $groupInfo,
+		IGroupManager $groupManager,
 		CustomGroupsDatabaseHandler $groupsHandler,
 		MembershipHelper $helper
 	) {
 		$this->groupsHandler = $groupsHandler;
+		$this->groupManager = $groupManager;
 		$this->groupInfo = $groupInfo;
 		$this->helper = $helper;
 
@@ -97,7 +107,12 @@ public function delete() {
 		if (!$this->helper->isUserAdmin($groupId)) {
 			throw new Forbidden("No permission to delete group \"$groupId\"");
 		}
-		$this->groupsHandler->deleteGroup($groupId);
+
+		$group = $this->groupManager->get('customgroup_' . $this->groupInfo['uri']);
+		if ($group === null) {
+			throw new NotFound("Group not found \"$groupId\"");
+		}
+		$group->delete();
 
 		$event = new GenericEvent(null, ['groupName' => $this->groupInfo['display_name']]);
 		$this->dispatcher->dispatch('\OCA\CustomGroups::deleteGroup', $event);

lib/Dav/GroupsCollection.php

@@ -32,6 +32,7 @@
 use OCA\CustomGroups\Service\MembershipHelper;
 use Symfony\Component\EventDispatcher\GenericEvent;
 use Sabre\DAV\Exception\Conflict;
+use OCP\IGroupManager;
 
 /**
  * Collection of custom groups
@@ -45,6 +46,12 @@ class GroupsCollection implements IExtendedCollection {
 	 */
 	private $groupsHandler;
 
+	/**
+	 * Group manager from core
+	 * @var IGroupManager
+	 */
+	private $groupManager;
+
 	/**
 	 * Membership helper
 	 *
@@ -71,10 +78,12 @@ class GroupsCollection implements IExtendedCollection {
 	 * @param MembershipHelper $helper helper
 	 */
 	public function __construct(
+		IGroupManager $groupManager,
 		CustomGroupsDatabaseHandler $groupsHandler,
 		MembershipHelper $helper,
 		$userId = null
 	) {
+		$this->groupManager = $groupManager;
 		$this->groupsHandler = $groupsHandler;
 		$this->helper = $helper;
 		$this->userId = $userId;
@@ -248,6 +257,7 @@ public function getLastModified() {
 	private function createMembershipsCollection(array $groupInfo) {
 		return new GroupMembershipCollection(
 			$groupInfo,
+			$this->groupManager,
 			$this->groupsHandler,
 			$this->helper
 		);

lib/Dav/RootCollection.php

@@ -26,6 +26,7 @@
 use Sabre\DAV\Exception\MethodNotAllowed;
 use Sabre\DAV\SimpleCollection;
 use OCA\CustomGroups\Service\MembershipHelper;
+use OCP\IGroupManager;
 
 /**
  * Root collection for the custom groups and members
@@ -34,18 +35,23 @@ class RootCollection extends SimpleCollection {
 	/**
 	 * Constructor
 	 *
+	 * @param IGroupManager $groupManager group manager
+	 * @param CustomGroupsDatabaseHandler $groupsHandler groups database handler
 	 * @param MembershipHelper $helper membership helper
 	 */
 	public function __construct(
+		IGroupManager $groupManager,
 		CustomGroupsDatabaseHandler $groupsHandler,
 		MembershipHelper $helper
 	) {
 		$children = [
 			new GroupsCollection(
+				$groupManager,
 				$groupsHandler,
 				$helper
 			),
 			new UsersCollection(
+				$groupManager,
 				$groupsHandler,
 				$helper
 			),

lib/Dav/UsersCollection.php

@@ -27,6 +27,7 @@
 use Sabre\DAV\Exception\MethodNotAllowed;
 use Sabre\DAV\Exception\Forbidden;
 use OCA\CustomGroups\Service\MembershipHelper;
+use OCP\IGroupManager;
 
 /**
  * Collection of users
@@ -47,16 +48,23 @@ class UsersCollection implements ICollection {
 	 */
 	private $helper;
 
+	/**
+	 * @var IGroupManager
+	 */
+	private $groupManager;
+
 	/**
 	 * Constructor
 	 *
 	 * @param CustomGroupsDatabaseHandler $groupsHandler custom groups handler
 	 * @param MembershipHelper $helper
 	 */
 	public function __construct(
+		IGroupManager $groupManager,
 		CustomGroupsDatabaseHandler $groupsHandler,
 		MembershipHelper $helper
 	) {
+		$this->groupManager = $groupManager;
 		$this->groupsHandler = $groupsHandler;
 		$this->helper = $helper;
 	}
@@ -94,6 +102,7 @@ public function getChild($name) {
 		// but ownCloud admin can query membership of any user
 		if ($name === $this->helper->getUserId() || $this->helper->isUserSuperAdmin()) {
 			return new GroupsCollection(
+				$this->groupManager,
 				$this->groupsHandler,
 				$this->helper,
 				$name

tests/unit/CustomGroupsBackendTest.php

@@ -48,7 +48,7 @@ public function setUp() {
 	public function testImplementsAction() {
 		$this->assertTrue($this->backend->implementsActions(GroupInterface::GROUP_DETAILS));
 		$this->assertFalse($this->backend->implementsActions(GroupInterface::CREATE_GROUP));
-		$this->assertFalse($this->backend->implementsActions(GroupInterface::DELETE_GROUP));
+		$this->assertTrue($this->backend->implementsActions(GroupInterface::DELETE_GROUP));
 		$this->assertFalse($this->backend->implementsActions(GroupInterface::ADD_TO_GROUP));
 		$this->assertFalse($this->backend->implementsActions(GroupInterface::REMOVE_FROM_GROUP));
 		$this->assertFalse($this->backend->implementsActions(GroupInterface::COUNT_USERS));

tests/unit/Dav/GroupMembershipCollectionTest.php

@@ -35,6 +35,7 @@
 use OCP\IConfig;
 use Symfony\Component\EventDispatcher\GenericEvent;
 use OCA\CustomGroups\Dav\Roles;
+use OCP\IGroup;
 
 /**
  * Class GroupMembershipCollectionTest
@@ -111,6 +112,7 @@ public function setUp() {
 
 		$this->node = new GroupMembershipCollection(
 			['group_id' => 1, 'uri' => 'group1', 'display_name' => 'Group One', 'role' => CustomGroupsDatabaseHandler::ROLE_ADMIN],
+			$this->groupManager,
 			$this->handler,
 			$this->helper
 		);
@@ -143,9 +145,14 @@ public function testBase() {
 	public function testDeleteAsAdmin() {
 		$this->setCurrentUserMemberInfo(['group_id' => 1, 'user_id' => self::CURRENT_USER, 'role' => CustomGroupsDatabaseHandler::ROLE_ADMIN]);
 
-		$this->handler->expects($this->at(1))
-			->method('deleteGroup')
-			->with(1);
+		$group = $this->createMock(IGroup::class);
+		$group->expects($this->once())
+			->method('delete');
+
+		$this->groupManager->expects($this->once())
+			->method('get')
+			->with('customgroup_group1')
+			->willReturn($group);
 
 		$called = array();
 		\OC::$server->getEventDispatcher()->addListener('\OCA\CustomGroups::deleteGroup', function ($event) use (&$called) {

tests/unit/Dav/GroupsCollectionTest.php

@@ -102,7 +102,11 @@ public function setUp() {
 			$this->createMock(IURLGenerator::class),
 			$this->config
 		);
-		$this->collection = new GroupsCollection($this->handler, $this->helper);
+		$this->collection = new GroupsCollection(
+			$this->createMock(IGroupManager::class),
+			$this->handler,
+			$this->helper
+		);
 	}
 
 	public function testBase() {
@@ -148,7 +152,12 @@ public function testListGroupsSearchPattern() {
 	}
 
 	public function testListGroupsForUser() {
-		$collection = new GroupsCollection($this->handler, $this->helper, 'user1');
+		$collection = new GroupsCollection(
+			$this->createMock(IGroupManager::class),
+			$this->handler,
+			$this->helper,
+			'user1'
+		);
 		$this->handler->expects($this->never())->method('getGroups');
 		$this->handler->expects($this->at(0))
 			->method('getUserMemberships')
@@ -170,7 +179,12 @@ public function testListGroupsForUser() {
 	public function testListGroupsForUserSearchPattern() {
 		$search = new Search('gr', 16, 256);
 
-		$collection = new GroupsCollection($this->handler, $this->helper, 'user1');
+		$collection = new GroupsCollection(
+			$this->createMock(IGroupManager::class),
+			$this->handler,
+			$this->helper,
+			'user1'
+		);
 		$this->handler->expects($this->never())->method('getGroups');
 		$this->handler->expects($this->at(0))
 			->method('getUserMemberships')
@@ -259,7 +273,11 @@ public function testCreateGroupNoPermission() {
 			->method('canCreateGroups')
 			->willReturn(false);
 
-		$this->collection = new GroupsCollection($this->handler, $helper);
+		$this->collection = new GroupsCollection(
+			$this->createMock(IGroupManager::class),
+			$this->handler,
+			$helper
+		);
 
 		$this->handler->expects($this->never())
 			->method('createGroup');

tests/unit/Dav/RootCollectionTest.php

@@ -25,6 +25,7 @@
 use OCA\CustomGroups\Dav\GroupsCollection;
 use OCA\CustomGroups\CustomGroupsDatabaseHandler;
 use OCA\CustomGroups\Service\MembershipHelper;
+use OCP\IGroupManager;
 
 /**
  * Class RootCollectionTest
@@ -39,6 +40,7 @@ public function setUp() {
 		$helper = $this->createMock(MembershipHelper::class);
 
 		$this->collection = new RootCollection(
+			$this->createMock(IGroupManager::class),
 			$handler,
 			$helper
 		);

tests/unit/Dav/UsersCollectionTest.php

@@ -93,7 +93,11 @@ public function setUp() {
 			$this->createMock(IConfig::class)
 		);
 
-		$this->collection = new UsersCollection($this->handler, $this->helper);
+		$this->collection = new UsersCollection(
+			$this->createMock(IGroupManager::class),
+			$this->handler,
+			$this->helper
+		);
 	}
 
 	public function testBase() {