Merge pull request #3884 from nextcloud/downstream-26956

Skip null groups in group manager 

3 files changed, 232 insertions, 88 deletions

diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 1a7ced5f1ba..23a35024e0a 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -22,18 +22,24 @@
  */
 namespace Test\Group;
 
+use OC\Group\Database;
 use OC\User\Manager;
+use OCP\ILogger;
 use OCP\IUser;
 use OCP\GroupInterface;
+use Test\TestCase;
 
-class ManagerTest extends \Test\TestCase {
+class ManagerTest extends TestCase {
 	/** @var Manager|\PHPUnit_Framework_MockObject_MockObject $userManager */
 	protected $userManager;
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject $userManager */
+	protected $logger;
 
 	protected function setUp() {
 		parent::setUp();
 
 		$this->userManager = $this->createMock(Manager::class);
+		$this->logger = $this->createMock(ILogger::class);
 	}
 
 	private function getTestUser($userId) {
@@ -47,8 +53,12 @@ class ManagerTest extends \Test\TestCase {
 		return $mockUser;
 	}
 
+	/**
+	 * @param null|int $implementedActions
+	 * @return \PHPUnit_Framework_MockObject_MockObject
+	 */
 	private function getTestBackend($implementedActions = null) {
-		if (is_null($implementedActions)) {
+		if ($implementedActions === null) {
 			$implementedActions =
 				GroupInterface::ADD_TO_GROUP |
 				GroupInterface::REMOVE_FROM_GOUP |
@@ -58,7 +68,7 @@ class ManagerTest extends \Test\TestCase {
 		}
 		// need to declare it this way due to optional methods
 		// thanks to the implementsActions logic
-		$backend = $this->getMockBuilder(\OCP\GroupInterface::class)
+		$backend = $this->getMockBuilder(GroupInterface::class)
 			->disableOriginalConstructor()
 			->setMethods([
 				'getGroupDetails',
@@ -91,7 +101,7 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$group = $manager->get('group1');
@@ -100,7 +110,7 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testGetNoBackend() {
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 
 		$this->assertNull($manager->get('group1'));
 	}
@@ -115,7 +125,7 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(false));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$this->assertNull($manager->get('group1'));
@@ -125,7 +135,7 @@ class ManagerTest extends \Test\TestCase {
 		$backend = new \Test\Util\Group\Dummy();
 		$backend->createGroup('group1');
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$group = $manager->get('group1');
@@ -152,7 +162,7 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend1);
 		$manager->addBackend($backend2);
 
@@ -162,9 +172,7 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testCreate() {
-		/**
-		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
-		 */
+		/**@var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
 		$backendGroupCreated = false;
 		$backend = $this->getTestBackend();
 		$backend->expects($this->any())
@@ -177,9 +185,9 @@ class ManagerTest extends \Test\TestCase {
 			->method('createGroup')
 			->will($this->returnCallback(function () use (&$backendGroupCreated) {
 				$backendGroupCreated = true;
-			}));;
+			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$group = $manager->createGroup('group1');
@@ -187,9 +195,7 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testCreateExists() {
-		/**
-		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
-		 */
+		/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
 		$backend = $this->getTestBackend();
 		$backend->expects($this->any())
 			->method('groupExists')
@@ -198,7 +204,7 @@ class ManagerTest extends \Test\TestCase {
 		$backend->expects($this->never())
 			->method('createGroup');
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$group = $manager->createGroup('group1');
@@ -219,11 +225,11 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$groups = $manager->search('1');
-		$this->assertEquals(1, count($groups));
+		$this->assertCount(1, $groups);
 		$group1 = reset($groups);
 		$this->assertEquals('group1', $group1->getGID());
 	}
@@ -253,12 +259,12 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend1);
 		$manager->addBackend($backend2);
 
 		$groups = $manager->search('1');
-		$this->assertEquals(2, count($groups));
+		$this->assertCount(2, $groups);
 		$group1 = reset($groups);
 		$group12 = next($groups);
 		$this->assertEquals('group1', $group1->getGID());
@@ -290,18 +296,40 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend1);
 		$manager->addBackend($backend2);
 
 		$groups = $manager->search('1', 2, 1);
-		$this->assertEquals(2, count($groups));
+		$this->assertCount(2, $groups);
 		$group1 = reset($groups);
 		$group12 = next($groups);
 		$this->assertEquals('group1', $group1->getGID());
 		$this->assertEquals('group12', $group12->getGID());
 	}
 
+	public function testSearchResultExistsButGroupDoesNot() {
+		/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
+		$backend = $this->createMock(Database::class);
+		$backend->expects($this->once())
+			->method('getGroups')
+			->with('1')
+			->will($this->returnValue(['group1']));
+		$backend->expects($this->once())
+			->method('groupExists')
+			->with('group1')
+			->will($this->returnValue(false));
+
+		/** @var \OC\User\Manager $userManager */
+		$userManager = $this->createMock(Manager::class);
+
+		$manager = new \OC\Group\Manager($userManager, $this->logger);
+		$manager->addBackend($backend);
+
+		$groups = $manager->search('1');
+		$this->assertEmpty($groups);
+	}
+
 	public function testGetUserGroups() {
 		/**
 		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
@@ -316,18 +344,18 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$groups = $manager->getUserGroups($this->getTestUser('user1'));
-		$this->assertEquals(1, count($groups));
+		$this->assertCount(1, $groups);
 		$group1 = reset($groups);
 		$this->assertEquals('group1', $group1->getGID());
 	}
 
 	public function testGetUserGroupIds() {
 		/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Manager $manager */
-		$manager = $this->getMockBuilder('OC\Group\Manager')
+		$manager = $this->getMockBuilder(\OC\Group\Manager::class)
 			->disableOriginalConstructor()
 			->setMethods(['getUserGroups'])
 			->getMock();
@@ -338,19 +366,44 @@ class ManagerTest extends \Test\TestCase {
 				'abc' => 'abc',
 			]);
 
-		/** @var \OC\User\User $user */
-		$user = $this->getMockBuilder('OC\User\User')
-			->disableOriginalConstructor()
-			->getMock();
+		/** @var \OC\User\User|\PHPUnit_Framework_MockObject_MockObject $user */
+		$user = $this->createMock(IUser::class);
 
 		$groups = $manager->getUserGroupIds($user);
-		$this->assertEquals(2, count($groups));
+		$this->assertCount(2, $groups);
 
 		foreach ($groups as $group) {
 			$this->assertInternalType('string', $group);
 		}
 	}
 
+	public function testGetUserGroupsWithDeletedGroup() {
+		/**
+		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
+		 */
+		$backend = $this->createMock(Database::class);
+		$backend->expects($this->once())
+			->method('getUserGroups')
+			->with('user1')
+			->will($this->returnValue(array('group1')));
+		$backend->expects($this->any())
+			->method('groupExists')
+			->with('group1')
+			->will($this->returnValue(false));
+
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
+		$manager->addBackend($backend);
+
+		/** @var \OC\User\User|\PHPUnit_Framework_MockObject_MockObject $user */
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->atLeastOnce())
+			->method('getUID')
+			->willReturn('user1');
+
+		$groups = $manager->getUserGroups($user);
+		$this->assertEmpty($groups);
+	}
+
 	public function testInGroup() {
 		/**
 		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
@@ -364,7 +417,7 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$this->assertTrue($manager->isInGroup('user1', 'group1'));
@@ -383,7 +436,7 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$this->assertTrue($manager->isAdmin('user1'));
@@ -402,7 +455,7 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$this->assertFalse($manager->isAdmin('user1'));
@@ -433,12 +486,12 @@ class ManagerTest extends \Test\TestCase {
 			->method('groupExists')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend1);
 		$manager->addBackend($backend2);
 
 		$groups = $manager->getUserGroups($this->getTestUser('user1'));
-		$this->assertEquals(2, count($groups));
+		$this->assertCount(2, $groups);
 		$group1 = reset($groups);
 		$group2 = next($groups);
 		$this->assertEquals('group1', $group1->getGID());
@@ -468,14 +521,10 @@ class ManagerTest extends \Test\TestCase {
 					}
 			}));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('searchDisplayName')
 			->with('user3')
-			->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) {
+			->will($this->returnCallback(function($search, $limit, $offset) {
 				switch($offset) {
 					case 0 : return ['user3' => $this->getTestUser('user3'),
 									'user33' => $this->getTestUser('user33')];
@@ -485,7 +534,7 @@ class ManagerTest extends \Test\TestCase {
 			}));
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -496,11 +545,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', 'user3');
-		$this->assertEquals(1, count($users));
+		$this->assertCount(1, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertFalse(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -531,14 +580,10 @@ class ManagerTest extends \Test\TestCase {
 					}
 			}));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('searchDisplayName')
 			->with('user3')
-			->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) {
+			->will($this->returnCallback(function($search, $limit, $offset) {
 				switch($offset) {
 					case 0 : return ['user3' => $this->getTestUser('user3'),
 									'user33' => $this->getTestUser('user33')];
@@ -548,7 +593,7 @@ class ManagerTest extends \Test\TestCase {
 			}));
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -560,11 +605,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', 'user3', 1);
-		$this->assertEquals(1, count($users));
+		$this->assertCount(1, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertFalse(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -596,14 +641,10 @@ class ManagerTest extends \Test\TestCase {
 					}
 			}));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('searchDisplayName')
 			->with('user3')
-			->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) {
+			->will($this->returnCallback(function($search, $limit, $offset) {
 				switch($offset) {
 					case 0 :
 						return [
@@ -616,7 +657,7 @@ class ManagerTest extends \Test\TestCase {
 			}));
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -628,11 +669,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1);
-		$this->assertEquals(1, count($users));
+		$this->assertCount(1, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertFalse(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -655,13 +696,9 @@ class ManagerTest extends \Test\TestCase {
 			->with('testgroup', '', -1, 0)
 			->will($this->returnValue(array('user2', 'user33')));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -672,11 +709,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', '');
-		$this->assertEquals(2, count($users));
+		$this->assertCount(2, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertTrue(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -698,13 +735,9 @@ class ManagerTest extends \Test\TestCase {
 			->with('testgroup', '', 1, 0)
 			->will($this->returnValue(array('user2')));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -715,11 +748,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', '', 1);
-		$this->assertEquals(1, count($users));
+		$this->assertCount(1, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertTrue(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -741,13 +774,9 @@ class ManagerTest extends \Test\TestCase {
 			->with('testgroup', '', 1, 1)
 			->will($this->returnValue(array('user33')));
 
-		$userBackend = $this->getMockBuilder('\OC\User\Backend')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->userManager->expects($this->any())
 			->method('get')
-			->will($this->returnCallback(function($uid) use ($userBackend) {
+			->will($this->returnCallback(function($uid) {
 				switch($uid) {
 					case 'user1' : return $this->getTestUser('user1');
 					case 'user2' : return $this->getTestUser('user2');
@@ -758,11 +787,11 @@ class ManagerTest extends \Test\TestCase {
 				}
 			}));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$users = $manager->displayNamesInGroup('testgroup', '', 1, 1);
-		$this->assertEquals(1, count($users));
+		$this->assertCount(1, $users);
 		$this->assertFalse(isset($users['user1']));
 		$this->assertFalse(isset($users['user2']));
 		$this->assertFalse(isset($users['user3']));
@@ -786,7 +815,7 @@ class ManagerTest extends \Test\TestCase {
 			->with('group1')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		// prime cache
@@ -797,11 +826,11 @@ class ManagerTest extends \Test\TestCase {
 		// add user
 		$group = $manager->get('group1');
 		$group->addUser($user1);
-		$expectedGroups = ['group1'];
+		$expectedGroups[] = 'group1';
 
 		// check result
 		$groups = $manager->getUserGroups($user1);
-		$this->assertEquals(1, count($groups));
+		$this->assertCount(1, $groups);
 		$group1 = reset($groups);
 		$this->assertEquals('group1', $group1->getGID());
 	}
@@ -829,13 +858,13 @@ class ManagerTest extends \Test\TestCase {
 			->method('removeFromGroup')
 			->will($this->returnValue(true));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		// prime cache
 		$user1 = $this->getTestUser('user1');
 		$groups = $manager->getUserGroups($user1);
-		$this->assertEquals(1, count($groups));
+		$this->assertCount(1, $groups);
 		$group1 = reset($groups);
 		$this->assertEquals('group1', $group1->getGID());
 
@@ -859,7 +888,7 @@ class ManagerTest extends \Test\TestCase {
 			->with('user1')
 			->will($this->returnValue(null));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		$groups = $manager->getUserIdGroups('user1');
@@ -885,7 +914,7 @@ class ManagerTest extends \Test\TestCase {
 				['group2', ['gid' => 'group2']],
 			]));
 
-		$manager = new \OC\Group\Manager($this->userManager);
+		$manager = new \OC\Group\Manager($this->userManager, $this->logger);
 		$manager->addBackend($backend);
 
 		// group with display name
diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php
index 5870da8d218..a1e78313558 100644
--- a/tests/lib/Share20/DefaultShareProviderTest.php
+++ b/tests/lib/Share20/DefaultShareProviderTest.php
@@ -1524,6 +1524,48 @@ class DefaultShareProviderTest extends \Test\TestCase {
 		$this->provider->deleteFromSelf($share, 'user2');
 	}
 
+	/**
+	 * @expectedException \OC\Share20\Exception\ProviderException
+	 * @expectedExceptionMessage Group "group" does not exist
+	 */
+	public function testDeleteFromSelfGroupDoesNotExist() {
+		$qb = $this->dbConn->getQueryBuilder();
+		$stmt = $qb->insert('share')
+			->values([
+				'share_type'    => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_GROUP),
+				'share_with'    => $qb->expr()->literal('group'),
+				'uid_owner'     => $qb->expr()->literal('user1'),
+				'uid_initiator' => $qb->expr()->literal('user1'),
+				'item_type'     => $qb->expr()->literal('file'),
+				'file_source'   => $qb->expr()->literal(1),
+				'file_target'   => $qb->expr()->literal('myTarget1'),
+				'permissions'   => $qb->expr()->literal(2)
+			])->execute();
+		$this->assertEquals(1, $stmt);
+		$id = $qb->getLastInsertId();
+
+		$user1 = $this->getMock('\OCP\IUser');
+		$user1->method('getUID')->willReturn('user1');
+		$user2 = $this->getMock('\OCP\IUser');
+		$user2->method('getUID')->willReturn('user2');
+		$this->userManager->method('get')->will($this->returnValueMap([
+			['user1', $user1],
+			['user2', $user2],
+		]));
+
+		$this->groupManager->method('get')->with('group')->willReturn(null);
+
+		$file = $this->getMock('\OCP\Files\File');
+		$file->method('getId')->willReturn(1);
+
+		$this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf());
+		$this->rootFolder->method('getById')->with(1)->willReturn([$file]);
+
+		$share = $this->provider->getShareById($id);
+
+		$this->provider->deleteFromSelf($share, 'user2');
+	}
+
 	public function testDeleteFromSelfUser() {
 		$qb = $this->dbConn->getQueryBuilder();
 		$stmt = $qb->insert('share')
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 448373a8368..2a7df0df50f 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -1139,6 +1139,39 @@ class ManagerTest extends \Test\TestCase {
 		self::invokePrivate($this->manager, 'userCreateChecks', [$share]);
 	}
 
+ 	public function testUserCreateChecksIdenticalPathSharedViaDeletedGroup() {
+		$share  = $this->manager->newShare();
+
+		$sharedWith = $this->getMock('\OCP\IUser');
+		$sharedWith->method('getUID')->willReturn('sharedWith');
+
+		$this->userManager->method('get')->with('sharedWith')->willReturn($sharedWith);
+
+		$path = $this->getMock('\OCP\Files\Node');
+
+		$share->setSharedWith('sharedWith')
+			->setNode($path)
+			->setShareOwner('shareOwner')
+			->setProviderId('foo')
+			->setId('bar');
+
+		$share2 = $this->manager->newShare();
+		$share2->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
+			->setShareOwner('shareOwner2')
+			->setProviderId('foo')
+			->setId('baz')
+			->setSharedWith('group');
+
+		$this->groupManager->method('get')->with('group')->willReturn(null);
+
+		$this->defaultProvider
+			->method('getSharesByPath')
+			->with($path)
+			->willReturn([$share2]);
+
+		$this->assertNull($this->invokePrivate($this->manager, 'userCreateChecks', [$share]));
+	}
+
 	public function testUserCreateChecksIdenticalPathNotSharedWithUser() {
 		$share = $this->manager->newShare();
 		$sharedWith = $this->createMock(IUser::class);
@@ -1216,6 +1249,29 @@ class ManagerTest extends \Test\TestCase {
 		self::invokePrivate($this->manager, 'groupCreateChecks', [$share]);
 	}
 
+	/**
+	 * @expectedException Exception
+	 * @expectedExceptionMessage Only sharing within your own groups is allowed
+	 */
+	public function testGroupCreateChecksShareWithGroupMembersOnlyNullGroup() {
+		$share = $this->manager->newShare();
+
+		$user = $this->getMock('\OCP\IUser');
+		$share->setSharedBy('user')->setSharedWith('group');
+
+		$this->groupManager->method('get')->with('group')->willReturn(null);
+		$this->userManager->method('get')->with('user')->willReturn($user);
+
+		$this->config
+			->method('getAppValue')
+			->will($this->returnValueMap([
+				['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
+				['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
+			]));
+
+		$this->assertNull($this->invokePrivate($this->manager, 'groupCreateChecks', [$share]));
+	}
+
 	public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() {
 		$share = $this->manager->newShare();
 
@@ -2666,6 +2722,23 @@ class ManagerTest extends \Test\TestCase {
 		$this->manager->moveShare($share, 'recipient');
 	}
 
+	/**
+	 * @expectedException \InvalidArgumentException
+	 * @expectedExceptionMessage Group "shareWith" does not exist
+	 */
+	public function testMoveShareGroupNull() {
+		$share = $this->manager->newShare();
+		$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP);
+		$share->setSharedWith('shareWith');
+
+		$recipient = $this->getMock('\OCP\IUser');
+
+		$this->groupManager->method('get')->with('shareWith')->willReturn(null);
+		$this->userManager->method('get')->with('recipient')->willReturn($recipient);
+
+		$this->manager->moveShare($share, 'recipient');
+	}
+
 	public function testMoveShareGroup() {
 		$share = $this->manager->newShare();
 		$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)