From 7b1eedb11e5f55d563831d8e58c44e615267669c Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 8 Oct 2019 18:05:13 +0200
Subject: [PATCH] take group creation result into consideration

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 lib/private/Group/Manager.php   | 19 ++++++++++---------
 tests/lib/Group/ManagerTest.php | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index 7567f719b0..b372eb1786 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -180,7 +180,7 @@ class Manager extends PublicEmitter implements IGroupManager {
 	protected function getGroupObject($gid, $displayName = null) {
 		$backends = [];
 		foreach ($this->backends as $backend) {
-			if ($backend->implementsActions(\OC\Group\Backend::GROUP_DETAILS)) {
+			if ($backend->implementsActions(Backend::GROUP_DETAILS)) {
 				$groupData = $backend->getGroupDetails($gid);
 				if (is_array($groupData) && !empty($groupData)) {
 					// take the display name from the first backend that has a non-null one
@@ -210,7 +210,7 @@ class Manager extends PublicEmitter implements IGroupManager {
 
 	/**
 	 * @param string $gid
-	 * @return \OC\Group\Group
+	 * @return IGroup|bool|null
 	 */
 	public function createGroup($gid) {
 		if ($gid === '' || $gid === null) {
@@ -218,13 +218,14 @@ class Manager extends PublicEmitter implements IGroupManager {
 		} else if ($group = $this->get($gid)) {
 			return $group;
 		} else {
-			$this->emit('\OC\Group', 'preCreate', array($gid));
+			$this->emit('\OC\Group', 'preCreate', [$gid]);
 			foreach ($this->backends as $backend) {
-				if ($backend->implementsActions(\OC\Group\Backend::CREATE_GROUP)) {
-					$backend->createGroup($gid);
-					$group = $this->getGroupObject($gid);
-					$this->emit('\OC\Group', 'postCreate', array($group));
-					return $group;
+				if ($backend->implementsActions(Backend::CREATE_GROUP)) {
+					if($backend->createGroup($gid)) {
+						$group = $this->getGroupObject($gid);
+						$this->emit('\OC\Group', 'postCreate', [$group]);
+						return $group;
+					}
 				}
 			}
 			return null;
@@ -300,7 +301,7 @@ class Manager extends PublicEmitter implements IGroupManager {
 	 */
 	public function isAdmin($userId) {
 		foreach ($this->backends as $backend) {
-			if ($backend->implementsActions(\OC\Group\Backend::IS_ADMIN) && $backend->isAdmin($userId)) {
+			if ($backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) {
 				return true;
 			}
 		}
diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 32e3021775..b8f798832d 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -190,6 +190,7 @@ class ManagerTest extends TestCase {
 			->method('createGroup')
 			->will($this->returnCallback(function () use (&$backendGroupCreated) {
 				$backendGroupCreated = true;
+				return true;
 			}));
 
 		$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger);
@@ -199,6 +200,35 @@ class ManagerTest extends TestCase {
 		$this->assertEquals('group1', $group->getGID());
 	}
 
+	public function testCreateFailure() {
+		/**@var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
+		$backendGroupCreated = false;
+		$backend = $this->getTestBackend(
+			GroupInterface::ADD_TO_GROUP |
+			GroupInterface::REMOVE_FROM_GOUP |
+			GroupInterface::COUNT_USERS |
+			GroupInterface::CREATE_GROUP |
+			GroupInterface::DELETE_GROUP |
+			GroupInterface::GROUP_DETAILS
+		);
+		$backend->expects($this->any())
+			->method('groupExists')
+			->with('group1')
+			->willReturn(false);
+		$backend->expects($this->once())
+			->method('createGroup')
+			->willReturn(false);
+		$backend->expects($this->once())
+			->method('getGroupDetails')
+			->willReturn([]);
+
+		$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger);
+		$manager->addBackend($backend);
+
+		$group = $manager->createGroup('group1');
+		$this->assertEquals(null, $group);
+	}
+
 	public function testCreateExists() {
 		/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
 		$backend = $this->getTestBackend();