20 Commits d480c4df3f ... 08bfd63406

Author SHA1 Message Date
  Morris Jobke 08bfd63406 Merge pull request #21704 from nextcloud/fix/18895/fix-unit-tests 3 years ago
  Morris Jobke e878c0a054 Merge pull request #21074 from jvsalo/shared-lock-multi-release 3 years ago
  Morris Jobke d72d9ff1f4 Merge pull request #21171 from nextcloud/enh/noid/tidy-up-group-ldap 3 years ago
  Morris Jobke db782fefa1 Merge pull request #21222 from nextcloud/bugfix/18965/reduce-contacts-search-load 3 years ago
  Morris Jobke fb69d6d195 Merge pull request #21238 from nextcloud/techdebt/noid/deferrable-notification-apps 3 years ago
  Morris Jobke 2b6cadbcd9 Fix unit tests from #18895 3 years ago
  Morris Jobke d78449c01c Merge pull request #18895 from nextcloud/bugfix/noid/fix_birthday_calendar_color 3 years ago
  Morris Jobke 6c825ee9a0 Merge pull request #21665 from nextcloud/debt/noid/job-list 3 years ago
  Roeland Jago Douma 2a0c7e258a Merge pull request #21692 from nextcloud/cleanup/sharebymail 3 years ago
  Roeland Jago Douma 106c733242 Merge pull request #21581 from nextcloud/3rdparty/symfony/4.4.10 3 years ago
  Morris Jobke b1cdd0dd9b Use formal type hints instead of informal PHPDoc 3 years ago
  Roeland Jago Douma f8036075c8 Bump symfony deps to 4.4.10 3 years ago
  Roeland Jago Douma 0bcc643d6e Cleanup share by mail a bit 3 years ago
  Joas Schilling 3d559159f0 Allow notification apps to defer and flush the sending 3 years ago
  Daniel Kesselberg 9b10d35477 Fix wrong phpdoc for execute method 3 years ago
  Joas Schilling 00e7b2b956 Reduce load of the contacts search when we know it can't match 3 years ago
  Arthur Schiwon 64fe042b0d tidy up Group_LDAP 3 years ago
  Jaakko Salo 6886b46ee2 In LockPlugin, only release a lock if it was acquired 3 years ago
  Jaakko Salo b7dd278e24 Fix releasing a shared lock multiple times 3 years ago
  Georg Ehrke 4d299d1c66 Changes the Birthday calendar color to slightly brighter one 4 years ago

+ 1 - 1
3rdparty

@@ -1 +1 @@
-Subproject commit b0f6df380578eb2e204b9f0c94f092bd8bafb3cd
+Subproject commit e78cd33d74d91b553d3af50b918a6857d8e54356

+ 1 - 1
apps/dav/lib/CalDAV/BirthdayService.php

@@ -165,7 +165,7 @@ class BirthdayService {
 		}
 		$this->calDavBackEnd->createCalendar($principal, self::BIRTHDAY_CALENDAR_URI, [
 			'{DAV:}displayname' => 'Contact birthdays',
-			'{http://apple.com/ns/ical/}calendar-color' => '#FFFFCA',
+			'{http://apple.com/ns/ical/}calendar-color' => '#E9D859',
 			'components'   => 'VEVENT',
 		]);
 

+ 24 - 6
apps/dav/lib/CardDAV/CardDavBackend.php

@@ -949,20 +949,38 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 * @return array an array of contacts which are arrays of key-value-pairs
 	 */
 	public function search($addressBookId, $pattern, $searchProperties, $options = []) {
-		$query2 = $this->db->getQueryBuilder();
+		$escapePattern = !\array_key_exists('escape_like_param', $options) || $options['escape_like_param'] !== false;
 
-		$query2->selectDistinct('cp.cardid')
-			->from($this->dbCardsPropertiesTable, 'cp')
-			->andWhere($query2->expr()->eq('cp.addressbookid', $query2->createNamedParameter($addressBookId)));
+		$query2 = $this->db->getQueryBuilder();
 		$or = $query2->expr()->orX();
 		foreach ($searchProperties as $property) {
+			if ($escapePattern) {
+				if ($property === 'EMAIL' && strpos($pattern, ' ') !== false) {
+					// There can be no spaces in emails
+					continue;
+				}
+
+				if ($property === 'CLOUD' && preg_match('/[^a-zA-Z0-9 _.@\-\']/', $pattern) === 1) {
+					// There can be no chars in cloud ids which are not valid for user ids
+					continue;
+				}
+			}
+
 			$or->add($query2->expr()->eq('cp.name', $query2->createNamedParameter($property)));
 		}
-		$query2->andWhere($or);
+
+		if ($or->count() === 0) {
+			return [];
+		}
+
+		$query2->selectDistinct('cp.cardid')
+			->from($this->dbCardsPropertiesTable, 'cp')
+			->andWhere($query2->expr()->eq('cp.addressbookid', $query2->createNamedParameter($addressBookId)))
+			->andWhere($or);
 
 		// No need for like when the pattern is empty
 		if ('' !== $pattern) {
-			if (\array_key_exists('escape_like_param', $options) && $options['escape_like_param'] === false) {
+			if (!$escapePattern) {
 				$query2->andWhere($query2->expr()->ilike('cp.value', $query2->createNamedParameter($pattern)));
 			} else {
 				$query2->andWhere($query2->expr()->ilike('cp.value', $query2->createNamedParameter('%' . $this->db->escapeLikeParameter($pattern) . '%')));

+ 14 - 0
apps/dav/lib/Connector/Sabre/LockPlugin.php

@@ -41,6 +41,13 @@ class LockPlugin extends ServerPlugin {
 	 */
 	private $server;
 
+	/**
+	 * State of the lock
+	 *
+	 * @var bool
+	 */
+	private $isLocked;
+
 	/**
 	 * {@inheritdoc}
 	 */
@@ -48,6 +55,7 @@ class LockPlugin extends ServerPlugin {
 		$this->server = $server;
 		$this->server->on('beforeMethod:*', [$this, 'getLock'], 50);
 		$this->server->on('afterMethod:*', [$this, 'releaseLock'], 50);
+		$this->isLocked = false;
 	}
 
 	public function getLock(RequestInterface $request) {
@@ -67,10 +75,15 @@ class LockPlugin extends ServerPlugin {
 			} catch (LockedException $e) {
 				throw new FileLocked($e->getMessage(), $e->getCode(), $e);
 			}
+			$this->isLocked = true;
 		}
 	}
 
 	public function releaseLock(RequestInterface $request) {
+		// don't try to release the lock if we never locked one
+		if ($this->isLocked === false) {
+			return;
+		}
 		if ($request->getMethod() !== 'PUT' || isset($_SERVER['HTTP_OC_CHUNKED'])) {
 			return;
 		}
@@ -81,6 +94,7 @@ class LockPlugin extends ServerPlugin {
 		}
 		if ($node instanceof Node) {
 			$node->releaseLock(ILockingProvider::LOCK_SHARED);
+			$this->isLocked = false;
 		}
 	}
 }

+ 1 - 1
apps/dav/tests/unit/CardDAV/BirthdayServiceTest.php

@@ -343,7 +343,7 @@ class BirthdayServiceTest extends TestCase {
 			->method('createCalendar')
 			->with('principal001', 'contact_birthdays', [
 				'{DAV:}displayname' => 'Contact birthdays',
-				'{http://apple.com/ns/ical/}calendar-color' => '#FFFFCA',
+				'{http://apple.com/ns/ical/}calendar-color' => '#E9D859',
 				'components'   => 'VEVENT',
 			]);
 		$this->service->ensureCalendarExists('principal001');

+ 1 - 1
apps/dav/tests/unit/CardDAV/CardDavBackendTest.php

@@ -762,7 +762,7 @@ class CardDavBackendTest extends TestCase {
 			'limit' => ['john', ['FN'], ['limit' => 1], [['uri0', 'John Doe']]],
 			'limit and offset' => ['john', ['FN'], ['limit' => 1, 'offset' => 1], [['uri1', 'John M. Doe']]],
 			'find "_" escaped' => ['_', ['CLOUD'], [], [['uri2', 'find without options']]],
-			'find not empty ClOUD' => ['%_%', ['CLOUD'], ['escape_like_param'=>false], [['uri0', 'John Doe'], ['uri2', 'find without options']]],
+			'find not empty CLOUD' => ['%_%', ['CLOUD'], ['escape_like_param'=>false], [['uri0', 'John Doe'], ['uri2', 'find without options']]],
 		];
 	}
 

File diff suppressed because it is too large
+ 0 - 0
apps/files_sharing/js/dist/files_sharing_tab.js


File diff suppressed because it is too large
+ 0 - 0
apps/files_sharing/js/dist/files_sharing_tab.js.map


+ 2 - 2
apps/files_sharing/src/services/ConfigService.js

@@ -178,7 +178,7 @@ export default class Config {
 	 * @memberof Config
 	 */
 	get isMailShareAllowed() {
-		return OC.appConfig.shareByMailEnabled !== undefined
+		return OC.getCapabilities()['files_sharing']['sharebymail'] !== undefined
 			&& OC.getCapabilities()['files_sharing']['public']['enabled'] === true
 	}
 
@@ -223,7 +223,7 @@ export default class Config {
 	 * @memberof Config
 	 */
 	get isPasswordForMailSharesRequired() {
-		return (OC.appConfig.shareByMail === undefined) ? false : OC.appConfig.shareByMail.enforcePasswordProtection === true
+		return (OC.getCapabilities()['files_sharing']['sharebymail'] === undefined) ? false : OC.getCapabilities()['files_sharing']['sharebymail']['password']['enforced']
 	}
 
 	/**

+ 0 - 27
apps/sharebymail/appinfo/app.php

@@ -1,27 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016 Bjoern Schiessle <bjoern@schiessle.org>
- *
- * @author Bjoern Schiessle <bjoern@schiessle.org>
- * @author Robin Appelman <robin@icewind.nl>
- *
- * @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/>.
- *
- */
-
-use OCA\ShareByMail\AppInfo\Application;
-
-$app = \OC::$server->query(Application::class);

+ 0 - 1
apps/sharebymail/composer/composer/autoload_classmap.php

@@ -9,7 +9,6 @@ return array(
     'OCA\\ShareByMail\\Activity' => $baseDir . '/../lib/Activity.php',
     'OCA\\ShareByMail\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php',
     'OCA\\ShareByMail\\Capabilities' => $baseDir . '/../lib/Capabilities.php',
-    'OCA\\ShareByMail\\Settings' => $baseDir . '/../lib/Settings.php',
     'OCA\\ShareByMail\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
     'OCA\\ShareByMail\\Settings\\SettingsManager' => $baseDir . '/../lib/Settings/SettingsManager.php',
     'OCA\\ShareByMail\\ShareByMailProvider' => $baseDir . '/../lib/ShareByMailProvider.php',

+ 0 - 1
apps/sharebymail/composer/composer/autoload_static.php

@@ -24,7 +24,6 @@ class ComposerStaticInitShareByMail
         'OCA\\ShareByMail\\Activity' => __DIR__ . '/..' . '/../lib/Activity.php',
         'OCA\\ShareByMail\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php',
         'OCA\\ShareByMail\\Capabilities' => __DIR__ . '/..' . '/../lib/Capabilities.php',
-        'OCA\\ShareByMail\\Settings' => __DIR__ . '/..' . '/../lib/Settings.php',
         'OCA\\ShareByMail\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
         'OCA\\ShareByMail\\Settings\\SettingsManager' => __DIR__ . '/..' . '/../lib/Settings/SettingsManager.php',
         'OCA\\ShareByMail\\ShareByMailProvider' => __DIR__ . '/..' . '/../lib/ShareByMailProvider.php',

+ 14 - 13
apps/sharebymail/lib/AppInfo/Application.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Bjoern Schiessle <bjoern@schiessle.org>
  *
@@ -27,23 +29,22 @@
 namespace OCA\ShareByMail\AppInfo;
 
 use OCA\ShareByMail\Capabilities;
-use OCA\ShareByMail\Settings;
 use OCP\AppFramework\App;
-use OCP\Util;
+use OCP\AppFramework\Bootstrap\IBootContext;
+use OCP\AppFramework\Bootstrap\IBootstrap;
+use OCP\AppFramework\Bootstrap\IRegistrationContext;
 
-class Application extends App {
-	public function __construct(array $urlParams = []) {
-		parent::__construct('sharebymail', $urlParams);
+class Application extends App implements IBootstrap {
+	public const APP_ID = 'sharebymail';
 
-		$settingsManager = \OC::$server->query(Settings\SettingsManager::class);
-		$settings = new Settings($settingsManager);
+	public function __construct() {
+		parent::__construct(self::APP_ID);
+	}
 
-		/** register capabilities */
-		$container = $this->getContainer();
-		$container->registerCapability(Capabilities::class);
+	public function register(IRegistrationContext $context): void {
+		$context->registerCapability(Capabilities::class);
+	}
 
-		/** register hooks */
-		Util::connectHook('\OCP\Config', 'js', $settings, 'announceShareProvider');
-		Util::connectHook('\OCP\Config', 'js', $settings, 'announceShareByMailSettings');
+	public function boot(IBootContext $context): void {
 	}
 }

+ 21 - 10
apps/sharebymail/lib/Capabilities.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Bjoern Schiessle <bjoern@schiessle.org>
  *
@@ -23,26 +25,35 @@
 
 namespace OCA\ShareByMail;
 
+use OCA\ShareByMail\Settings\SettingsManager;
 use OCP\Capabilities\ICapability;
 
 class Capabilities implements ICapability {
 
-	/**
-	 * Function an app uses to return the capabilities
-	 *
-	 * @return array Array containing the apps capabilities
-	 * @since 8.2.0
-	 */
-	public function getCapabilities() {
+	/** @var SettingsManager */
+	private $manager;
+
+	public function __construct(SettingsManager $manager) {
+		$this->manager = $manager;
+	}
+
+	public function getCapabilities(): array {
 		return [
 			'files_sharing' =>
 				[
 					'sharebymail' =>
 						[
 							'enabled' => true,
-							'upload_files_drop' => ['enabled' => true],
-							'password' => ['enabled' => true],
-							'expire_date' => ['enabled' => true]
+							'upload_files_drop' => [
+								'enabled' => true,
+							],
+							'password' => [
+								'enabled' => true,
+								'enforced' => $this->manager->enforcePasswordProtection(),
+							],
+							'expire_date' => [
+								'enabled' => true,
+							],
 						]
 				]
 		];

+ 0 - 53
apps/sharebymail/lib/Settings.php

@@ -1,53 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016 Bjoern Schiessle <bjoern@schiessle.org>
- *
- * @author Bjoern Schiessle <bjoern@schiessle.org>
- *
- * @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 OCA\ShareByMail;
-
-use OCA\ShareByMail\Settings\SettingsManager;
-
-class Settings {
-
-	/** @var SettingsManager */
-	private $settingsManager;
-
-	public function __construct(SettingsManager $settingsManager) {
-		$this->settingsManager = $settingsManager;
-	}
-
-	/**
-	 * announce that the share-by-mail share provider is enabled
-	 *
-	 * @param array $settings
-	 */
-	public function announceShareProvider(array $settings) {
-		$array = json_decode($settings['array']['oc_appconfig'], true);
-		$array['shareByMailEnabled'] = true;
-		$settings['array']['oc_appconfig'] = json_encode($array);
-	}
-
-	public function announceShareByMailSettings(array $settings) {
-		$array = json_decode($settings['array']['oc_appconfig'], true);
-		$array['shareByMail']['enforcePasswordProtection'] = $this->settingsManager->enforcePasswordProtection();
-		$settings['array']['oc_appconfig'] = json_encode($array);
-	}
-}

+ 4 - 2
apps/sharebymail/lib/Settings/SettingsManager.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Bjoern Schiessle <bjoern@schiessle.org>
  *
@@ -43,7 +45,7 @@ class SettingsManager {
 	 *
 	 * @return bool
 	 */
-	public function sendPasswordByMail() {
+	public function sendPasswordByMail(): bool {
 		$sendPasswordByMail = $this->config->getAppValue('sharebymail', 'sendpasswordmail', $this->sendPasswordByMailDefault);
 		return $sendPasswordByMail === 'yes';
 	}
@@ -53,7 +55,7 @@ class SettingsManager {
 	 *
 	 * @return bool
 	 */
-	public function enforcePasswordProtection() {
+	public function enforcePasswordProtection(): bool {
 		$enforcePassword = $this->config->getAppValue('sharebymail', 'enforcePasswordProtection', $this->enforcePasswordProtectionDefault);
 		return $enforcePassword === 'yes';
 	}

+ 2 - 2
apps/user_ldap/lib/Access.php

@@ -1435,7 +1435,7 @@ class Access extends LDAPUtility {
 	 * @param bool $allowAsterisk whether in * at the beginning should be preserved
 	 * @return string the escaped string
 	 */
-	public function escapeFilterPart($input, $allowAsterisk = false) {
+	public function escapeFilterPart($input, $allowAsterisk = false): string {
 		$asterisk = '';
 		if ($allowAsterisk && strlen($input) > 0 && $input[0] === '*') {
 			$asterisk = '*';
@@ -1452,7 +1452,7 @@ class Access extends LDAPUtility {
 	 * @param string[] $filters the filters to connect
 	 * @return string the combined filter
 	 */
-	public function combineFilterWithAnd($filters) {
+	public function combineFilterWithAnd($filters): string {
 		return $this->combineFilter($filters, '&');
 	}
 

+ 5 - 0
apps/user_ldap/lib/Connection.php

@@ -67,6 +67,11 @@ use OCP\ILogger;
  * @property string[] ldapBaseGroups
  * @property string ldapGroupFilter
  * @property string ldapGroupDisplayName
+ * @property string ldapLoginFilter
+ * @property string ldapDynamicGroupMemberURL
+ * @property string ldapGidNumber
+ * @property int hasMemberOfFilterSupport
+ * @property int useMemberOfToDetectMembership
  */
 class Connection extends LDAPUtility {
 	private $ldapConnectionRes = null;

+ 154 - 197
apps/user_ldap/lib/Group_LDAP.php

@@ -43,37 +43,34 @@
 
 namespace OCA\User_LDAP;
 
+use Closure;
+use Exception;
+use OC;
 use OC\Cache\CappedMemoryCache;
+use OC\ServerNotAvailableException;
 use OCP\Group\Backend\IGetDisplayNameBackend;
 use OCP\GroupInterface;
 use OCP\ILogger;
 
-class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend {
+class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend {
 	protected $enabled = false;
 
-	/**
-	 * @var string[] $cachedGroupMembers array of users with gid as key
-	 */
+	/** @var string[] $cachedGroupMembers array of users with gid as key */
 	protected $cachedGroupMembers;
-
-	/**
-	 * @var string[] $cachedGroupsByMember array of groups with uid as key
-	 */
+	/** @var string[] $cachedGroupsByMember array of groups with uid as key */
 	protected $cachedGroupsByMember;
-
-	/**
-	 * @var string[] $cachedNestedGroups array of groups with gid (DN) as key
-	 */
+	/** @var string[] $cachedNestedGroups array of groups with gid (DN) as key */
 	protected $cachedNestedGroups;
-
 	/** @var GroupPluginManager */
 	protected $groupPluginManager;
+	/** @var ILogger */
+	protected $logger;
 
 	public function __construct(Access $access, GroupPluginManager $groupPluginManager) {
 		parent::__construct($access);
 		$filter = $this->access->connection->ldapGroupFilter;
-		$gassoc = $this->access->connection->ldapGroupMemberAssocAttr;
-		if (!empty($filter) && !empty($gassoc)) {
+		$gAssoc = $this->access->connection->ldapGroupMemberAssocAttr;
+		if (!empty($filter) && !empty($gAssoc)) {
 			$this->enabled = true;
 		}
 
@@ -81,6 +78,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		$this->cachedGroupsByMember = new CappedMemoryCache();
 		$this->cachedNestedGroups = new CappedMemoryCache();
 		$this->groupPluginManager = $groupPluginManager;
+		$this->logger = OC::$server->getLogger();
 	}
 
 	/**
@@ -89,8 +87,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * @param string $uid uid of the user
 	 * @param string $gid gid of the group
 	 * @return bool
-	 *
-	 * Checks whether the user is member of a group or not.
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
 	public function inGroup($uid, $gid) {
 		if (!$this->enabled) {
@@ -105,8 +103,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		$userDN = $this->access->username2dn($uid);
 
 		if (isset($this->cachedGroupMembers[$gid])) {
-			$isInGroup = in_array($userDN, $this->cachedGroupMembers[$gid]);
-			return $isInGroup;
+			return in_array($userDN, $this->cachedGroupMembers[$gid]);
 		}
 
 		$cacheKeyMembers = 'inGroup-members:' . $gid;
@@ -175,13 +172,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * @param string $dnGroup
-	 * @return array
+	 * For a group that has user membership defined by an LDAP search url
+	 * attribute returns the users that match the search url otherwise returns
+	 * an empty array.
 	 *
-	 * For a group that has user membership defined by an LDAP search url attribute returns the users
-	 * that match the search url otherwise returns an empty array.
+	 * @throws ServerNotAvailableException
 	 */
-	public function getDynamicGroupMembers($dnGroup) {
+	public function getDynamicGroupMembers(string $dnGroup): array {
 		$dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL);
 
 		if (empty($dynamicGroupMemberURL)) {
@@ -207,26 +204,26 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 					$dynamicMembers[$value['dn'][0]] = 1;
 				}
 			} else {
-				\OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' .
-					'of group ' . $dnGroup, ILogger::DEBUG);
+				$this->logger->debug('No search filter found on member url of group {dn}',
+					[
+						'app' => 'user_ldap',
+						'dn' => $dnGroup,
+					]
+				);
 			}
 		}
 		return $dynamicMembers;
 	}
 
 	/**
-	 * @param string $dnGroup
-	 * @param array|null &$seen
-	 * @return array|mixed|null
-	 * @throws \OC\ServerNotAvailableException
+	 * @throws ServerNotAvailableException
 	 */
-	private function _groupMembers($dnGroup, &$seen = null) {
+	private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
 		if ($seen === null) {
 			$seen = [];
 		}
 		$allMembers = [];
 		if (array_key_exists($dnGroup, $seen)) {
-			// avoid loops
 			return [];
 		}
 		// used extensively in cron job, caching makes sense for nested groups
@@ -251,13 +248,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * @param string $DN
-	 * @param array|null &$seen
-	 * @return array
-	 * @throws \OC\ServerNotAvailableException
+	 * @throws ServerNotAvailableException
 	 */
-	private function _getGroupDNsFromMemberOf($DN) {
-		$groups = $this->access->readAttribute($DN, 'memberOf');
+	private function _getGroupDNsFromMemberOf(string $dn): array {
+		$groups = $this->access->readAttribute($dn, 'memberOf');
 		if (!is_array($groups)) {
 			return [];
 		}
@@ -275,17 +269,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			return $nestedGroups;
 		};
 
-		$groups = $this->walkNestedGroups($DN, $fetcher, $groups);
+		$groups = $this->walkNestedGroups($dn, $fetcher, $groups);
 		return $this->filterValidGroups($groups);
 	}
 
-	/**
-	 * @param string $dn
-	 * @param \Closure $fetcher args: string $dn, array $seen, returns: string[] of dns
-	 * @param array $list
-	 * @return array
-	 */
-	private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): array {
+	private function walkNestedGroups(string $dn, Closure $fetcher, array $list): array {
 		$nesting = (int)$this->access->connection->ldapNestedGroups;
 		// depending on the input, we either have a list of DNs or a list of LDAP records
 		// also, the output expects either DNs or records. Testing the first element should suffice.
@@ -322,11 +310,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	/**
 	 * translates a gidNumber into an ownCloud internal name
 	 *
-	 * @param string $gid as given by gidNumber on POSIX LDAP
-	 * @param string $dn a DN that belongs to the same domain as the group
 	 * @return string|bool
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
-	public function gidNumber2Name($gid, $dn) {
+	public function gidNumber2Name(string $gid, string $dn) {
 		$cacheKey = 'gidNumberToName' . $gid;
 		$groupName = $this->access->connection->getFromCache($cacheKey);
 		if (!is_null($groupName) && isset($groupName)) {
@@ -339,14 +327,22 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			'objectClass=posixGroup',
 			$this->access->connection->ldapGidNumber . '=' . $gid
 		]);
+		return $this->getNameOfGroup($filter, $cacheKey) ?? false;
+	}
+
+	/**
+	 * @throws ServerNotAvailableException
+	 * @throws Exception
+	 */
+	private function getNameOfGroup(string $filter, string $cacheKey) {
 		$result = $this->access->searchGroups($filter, ['dn'], 1);
 		if (empty($result)) {
-			return false;
+			return null;
 		}
 		$dn = $result[0]['dn'][0];
 
 		//and now the group name
-		//NOTE once we have separate ownCloud group IDs and group names we can
+		//NOTE once we have separate Nextcloud group IDs and group names we can
 		//directly read the display name attribute instead of the DN
 		$name = $this->access->dn2groupname($dn);
 
@@ -358,11 +354,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	/**
 	 * returns the entry's gidNumber
 	 *
-	 * @param string $dn
-	 * @param string $attribute
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	private function getEntryGidNumber($dn, $attribute) {
+	private function getEntryGidNumber(string $dn, string $attribute) {
 		$value = $this->access->readAttribute($dn, $attribute);
 		if (is_array($value) && !empty($value)) {
 			return $value[0];
@@ -371,22 +366,20 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * returns the group's primary ID
-	 *
-	 * @param string $dn
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	public function getGroupGidNumber($dn) {
+	public function getGroupGidNumber(string $dn) {
 		return $this->getEntryGidNumber($dn, 'gidNumber');
 	}
 
 	/**
 	 * returns the user's gidNumber
 	 *
-	 * @param string $dn
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	public function getUserGidNumber($dn) {
+	public function getUserGidNumber(string $dn) {
 		$gidNumber = false;
 		if ($this->access->connection->hasGidNumber) {
 			$gidNumber = $this->getEntryGidNumber($dn, $this->access->connection->ldapGidNumber);
@@ -398,17 +391,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * returns a filter for a "users has specific gid" search or count operation
-	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @return string
-	 * @throws \Exception
+	 * @throws ServerNotAvailableException
+	 * @throws Exception
 	 */
-	private function prepareFilterForUsersHasGidNumber($groupDN, $search = '') {
+	private function prepareFilterForUsersHasGidNumber(string $groupDN, string $search = ''): string {
 		$groupID = $this->getGroupGidNumber($groupDN);
 		if ($groupID === false) {
-			throw new \Exception('Not a valid group');
+			throw new Exception('Not a valid group');
 		}
 
 		$filterParts = [];
@@ -424,13 +413,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	/**
 	 * returns a list of users that have the given group as gid number
 	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @param int $limit
-	 * @param int $offset
-	 * @return string[]
+	 * @throws ServerNotAvailableException
 	 */
-	public function getUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) {
+	public function getUsersInGidNumber(
+		string $groupDN,
+		string $search = '',
+		?int $limit = -1,
+		?int $offset = 0
+	): array {
 		try {
 			$filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search);
 			$users = $this->access->fetchListOfUsers(
@@ -440,37 +430,18 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 				$offset
 			);
 			return $this->access->nextcloudUserNames($users);
-		} catch (\Exception $e) {
+		} catch (ServerNotAvailableException $e) {
+			throw $e;
+		} catch (Exception $e) {
 			return [];
 		}
 	}
 
 	/**
-	 * returns the number of users that have the given group as gid number
-	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @param int $limit
-	 * @param int $offset
-	 * @return int
-	 */
-	public function countUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) {
-		try {
-			$filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search);
-			$users = $this->access->countUsers($filter, ['dn'], $limit, $offset);
-			return (int)$users;
-		} catch (\Exception $e) {
-			return 0;
-		}
-	}
-
-	/**
-	 * gets the gidNumber of a user
-	 *
-	 * @param string $dn
-	 * @return string
+	 * @throws ServerNotAvailableException
+	 * @return bool
 	 */
-	public function getUserGroupByGid($dn) {
+	public function getUserGroupByGid(string $dn) {
 		$groupID = $this->getUserGidNumber($dn);
 		if ($groupID !== false) {
 			$groupName = $this->gidNumber2Name($groupID, $dn);
@@ -485,11 +456,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	/**
 	 * translates a primary group ID into an Nextcloud internal name
 	 *
-	 * @param string $gid as given by primaryGroupID on AD
-	 * @param string $dn a DN that belongs to the same domain as the group
 	 * @return string|bool
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
-	public function primaryGroupID2Name($gid, $dn) {
+	public function primaryGroupID2Name(string $gid, string $dn) {
 		$cacheKey = 'primaryGroupIDtoName';
 		$groupNames = $this->access->connection->getFromCache($cacheKey);
 		if (!is_null($groupNames) && isset($groupNames[$gid])) {
@@ -506,30 +477,16 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			$this->access->connection->ldapGroupFilter,
 			'objectsid=' . $domainObjectSid . '-' . $gid
 		]);
-		$result = $this->access->searchGroups($filter, ['dn'], 1);
-		if (empty($result)) {
-			return false;
-		}
-		$dn = $result[0]['dn'][0];
-
-		//and now the group name
-		//NOTE once we have separate Nextcloud group IDs and group names we can
-		//directly read the display name attribute instead of the DN
-		$name = $this->access->dn2groupname($dn);
-
-		$this->access->connection->writeToCache($cacheKey, $name);
-
-		return $name;
+		return $this->getNameOfGroup($filter, $cacheKey) ?? false;
 	}
 
 	/**
 	 * returns the entry's primary group ID
 	 *
-	 * @param string $dn
-	 * @param string $attribute
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	private function getEntryGroupID($dn, $attribute) {
+	private function getEntryGroupID(string $dn, string $attribute) {
 		$value = $this->access->readAttribute($dn, $attribute);
 		if (is_array($value) && !empty($value)) {
 			return $value[0];
@@ -538,22 +495,18 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * returns the group's primary ID
-	 *
-	 * @param string $dn
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	public function getGroupPrimaryGroupID($dn) {
+	public function getGroupPrimaryGroupID(string $dn) {
 		return $this->getEntryGroupID($dn, 'primaryGroupToken');
 	}
 
 	/**
-	 * returns the user's primary group ID
-	 *
-	 * @param string $dn
 	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	public function getUserPrimaryGroupIDs($dn) {
+	public function getUserPrimaryGroupIDs(string $dn) {
 		$primaryGroupID = false;
 		if ($this->access->connection->hasPrimaryGroups) {
 			$primaryGroupID = $this->getEntryGroupID($dn, 'primaryGroupID');
@@ -565,17 +518,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * returns a filter for a "users in primary group" search or count operation
-	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @return string
-	 * @throws \Exception
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
-	private function prepareFilterForUsersInPrimaryGroup($groupDN, $search = '') {
+	private function prepareFilterForUsersInPrimaryGroup(string $groupDN, string $search = ''): string {
 		$groupID = $this->getGroupPrimaryGroupID($groupDN);
 		if ($groupID === false) {
-			throw new \Exception('Not a valid group');
+			throw new Exception('Not a valid group');
 		}
 
 		$filterParts = [];
@@ -589,15 +538,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * returns a list of users that have the given group as primary group
-	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @param int $limit
-	 * @param int $offset
-	 * @return string[]
+	 * @throws ServerNotAvailableException
 	 */
-	public function getUsersInPrimaryGroup($groupDN, $search = '', $limit = -1, $offset = 0) {
+	public function getUsersInPrimaryGroup(
+		string $groupDN,
+		string $search = '',
+		?int $limit = -1,
+		?int $offset = 0
+	): array {
 		try {
 			$filter = $this->prepareFilterForUsersInPrimaryGroup($groupDN, $search);
 			$users = $this->access->fetchListOfUsers(
@@ -607,37 +555,38 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 				$offset
 			);
 			return $this->access->nextcloudUserNames($users);
-		} catch (\Exception $e) {
+		} catch (ServerNotAvailableException $e) {
+			throw $e;
+		} catch (Exception $e) {
 			return [];
 		}
 	}
 
 	/**
-	 * returns the number of users that have the given group as primary group
-	 *
-	 * @param string $groupDN
-	 * @param string $search
-	 * @param int $limit
-	 * @param int $offset
-	 * @return int
+	 * @throws ServerNotAvailableException
 	 */
-	public function countUsersInPrimaryGroup($groupDN, $search = '', $limit = -1, $offset = 0) {
+	public function countUsersInPrimaryGroup(
+		string $groupDN,
+		string $search = '',
+		int $limit = -1,
+		int $offset = 0
+	): int {
 		try {
 			$filter = $this->prepareFilterForUsersInPrimaryGroup($groupDN, $search);
 			$users = $this->access->countUsers($filter, ['dn'], $limit, $offset);
 			return (int)$users;
-		} catch (\Exception $e) {
+		} catch (ServerNotAvailableException $e) {
+			throw $e;
+		} catch (Exception $e) {
 			return 0;
 		}
 	}
 
 	/**
-	 * gets the primary group of a user
-	 *
-	 * @param string $dn
-	 * @return string
+	 * @return string|bool
+	 * @throws ServerNotAvailableException
 	 */
-	public function getUserPrimaryGroup($dn) {
+	public function getUserPrimaryGroup(string $dn) {
 		$groupID = $this->getUserPrimaryGroupIDs($dn);
 		if ($groupID !== false) {
 			$groupName = $this->primaryGroupID2Name($groupID, $dn);
@@ -650,15 +599,15 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * Get all groups a user belongs to
-	 *
-	 * @param string $uid Name of the user
-	 * @return array with group names
-	 *
 	 * This function fetches all groups a user belongs to. It does not check
 	 * if the user exists at all.
 	 *
 	 * This function includes groups based on dynamic group membership.
+	 *
+	 * @param string $uid Name of the user
+	 * @return array with group names
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
 	public function getUserGroups($uid) {
 		if (!$this->enabled) {
@@ -709,8 +658,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 						}
 					}
 				} else {
-					\OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' .
-						'of group ' . print_r($dynamicGroup, true), ILogger::DEBUG);
+					$this->logger->debug('No search filter found on member url of group {dn}',
+						[
+							'app' => 'user_ldap',
+							'dn' => $dynamicGroup,
+						]
+					);
 				}
 			}
 		}
@@ -752,8 +705,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		} elseif (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') {
 			$result = $this->access->readAttribute($userDN, 'uid');
 			if ($result === false) {
-				\OCP\Util::writeLog('user_ldap', 'No uid attribute found for DN ' . $userDN . ' on ' .
-					$this->access->connection->ldapHost, ILogger::DEBUG);
+				$this->logger->debug('No uid attribute found for DN {dn} on {host}',
+					[
+						'app' => 'user_ldap',
+						'dn' => $userDN,
+						'host' => $this->access->connection->ldapHost,
+					]
+				);
 				$uid = false;
 			} else {
 				$uid = $result[0];
@@ -788,11 +746,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * @param string $dn
-	 * @param array|null &$seen
-	 * @return array
+	 * @throws ServerNotAvailableException
 	 */
-	private function getGroupsByMember($dn, &$seen = null) {
+	private function getGroupsByMember(string $dn, array &$seen = null): array {
 		if ($seen === null) {
 			$seen = [];
 		}
@@ -826,7 +782,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * @param int $limit
 	 * @param int $offset
 	 * @return array with user ids
-	 * @throws \Exception
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
 	public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
 		if (!$this->enabled) {
@@ -934,6 +891,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * @param string $gid the internal group name
 	 * @param string $search optional, a search string
 	 * @return int|bool
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
 	public function countUsersInGroup($gid, $search = '') {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::COUNT_USERS)) {
@@ -1003,8 +962,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 					continue;
 				}
 				// dn2username will also check if the users belong to the allowed base
-				if ($ocname = $this->access->dn2username($member)) {
-					$groupUsers[] = $ocname;
+				if ($ncGroupId = $this->access->dn2username($member)) {
+					$groupUsers[] = $ncGroupId;
 				}
 			}
 		}
@@ -1027,6 +986,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * Uses a paged search if available to override a
 	 * server side search limit.
 	 * (active directory has a limit of 1000 by default)
+	 * @throws Exception
 	 */
 	public function getGroups($search = '', $limit = -1, $offset = 0) {
 		if (!$this->enabled) {
@@ -1035,7 +995,6 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		$cacheKey = 'getGroups-' . $search . '-' . $limit . '-' . $offset;
 
 		//Check cache before driving unnecessary searches
-		\OCP\Util::writeLog('user_ldap', 'getGroups ' . $cacheKey, ILogger::DEBUG);
 		$ldap_groups = $this->access->connection->getFromCache($cacheKey);
 		if (!is_null($ldap_groups)) {
 			return $ldap_groups;
@@ -1050,7 +1009,6 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			$this->access->connection->ldapGroupFilter,
 			$this->access->getFilterPartForGroupSearch($search)
 		]);
-		\OCP\Util::writeLog('user_ldap', 'getGroups Filter ' . $filter, ILogger::DEBUG);
 		$ldap_groups = $this->access->fetchListOfGroups($filter,
 			[$this->access->connection->ldapGroupDisplayName, 'dn'],
 			$limit,
@@ -1061,19 +1019,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		return $ldap_groups;
 	}
 
-	/**
-	 * @param string $group
-	 * @return bool
-	 */
-	public function groupMatchesFilter($group) {
-		return (strripos($group, $this->groupSearch) !== false);
-	}
-
 	/**
 	 * check if a group exists
 	 *
 	 * @param string $gid
 	 * @return bool
+	 * @throws ServerNotAvailableException
 	 */
 	public function groupExists($gid) {
 		$groupExists = $this->access->connection->getFromCache('groupExists' . $gid);
@@ -1094,7 +1045,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			return false;
 		}
 
-		//if group really still exists, we will be able to read its objectclass
+		//if group really still exists, we will be able to read its objectClass
 		if (!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapGroupFilter))) {
 			$this->access->connection->writeToCache('groupExists' . $gid, false);
 			return false;
@@ -1104,6 +1055,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		return true;
 	}
 
+	/**
+	 * @throws ServerNotAvailableException
+	 * @throws Exception
+	 */
 	protected function filterValidGroups(array $listOfGroups): array {
 		$validGroupDNs = [];
 		foreach ($listOfGroups as $key => $item) {
@@ -1147,7 +1102,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 *
 	 * @param string $gid
 	 * @return bool
-	 * @throws \Exception
+	 * @throws Exception
+	 * @throws ServerNotAvailableException
 	 */
 	public function createGroup($gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::CREATE_GROUP)) {
@@ -1167,7 +1123,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			}
 			return $dn != null;
 		}
-		throw new \Exception('Could not create group in LDAP backend.');
+		throw new Exception('Could not create group in LDAP backend.');
 	}
 
 	/**
@@ -1175,7 +1131,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 *
 	 * @param string $gid gid of the group to delete
 	 * @return bool
-	 * @throws \Exception
+	 * @throws Exception
 	 */
 	public function deleteGroup($gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::DELETE_GROUP)) {
@@ -1186,7 +1142,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			}
 			return $ret;
 		}
-		throw new \Exception('Could not delete group in LDAP backend.');
+		throw new Exception('Could not delete group in LDAP backend.');
 	}
 
 	/**
@@ -1195,7 +1151,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * @param string $uid Name of the user to add to group
 	 * @param string $gid Name of the group in which add the user
 	 * @return bool
-	 * @throws \Exception
+	 * @throws Exception
 	 */
 	public function addToGroup($uid, $gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::ADD_TO_GROUP)) {
@@ -1205,7 +1161,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			}
 			return $ret;
 		}
-		throw new \Exception('Could not add user to group in LDAP backend.');
+		throw new Exception('Could not add user to group in LDAP backend.');
 	}
 
 	/**
@@ -1214,7 +1170,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 * @param string $uid Name of the user to remove from group
 	 * @param string $gid Name of the group from which remove the user
 	 * @return bool
-	 * @throws \Exception
+	 * @throws Exception
 	 */
 	public function removeFromGroup($uid, $gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::REMOVE_FROM_GROUP)) {
@@ -1224,21 +1180,21 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			}
 			return $ret;
 		}
-		throw new \Exception('Could not remove user from group in LDAP backend.');
+		throw new Exception('Could not remove user from group in LDAP backend.');
 	}
 
 	/**
 	 * Gets group details
 	 *
 	 * @param string $gid Name of the group
-	 * @return array | false
-	 * @throws \Exception
+	 * @return array|false
+	 * @throws Exception
 	 */
 	public function getGroupDetails($gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::GROUP_DETAILS)) {
 			return $this->groupPluginManager->getGroupDetails($gid);
 		}
-		throw new \Exception('Could not get group details in LDAP backend.');
+		throw new Exception('Could not get group details in LDAP backend.');
 	}
 
 	/**
@@ -1248,6 +1204,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	 *
 	 * @param string $gid
 	 * @return resource of the LDAP connection
+	 * @throws ServerNotAvailableException
 	 */
 	public function getNewLDAPConnection($gid) {
 		$connection = clone $this->access->getConnection();
@@ -1255,7 +1212,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 	}
 
 	/**
-	 * @throws \OC\ServerNotAvailableException
+	 * @throws ServerNotAvailableException
 	 */
 	public function getDisplayName(string $gid): string {
 		if ($this->groupPluginManager instanceof IGetDisplayNameBackend) {

+ 37 - 36
apps/user_ldap/tests/Group_LDAPTest.php

@@ -36,8 +36,10 @@ use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\Group_LDAP as GroupLDAP;
 use OCA\User_LDAP\GroupPluginManager;
 use OCA\User_LDAP\ILDAPWrapper;
+use OCA\User_LDAP\Mapping\GroupMapping;
 use OCA\User_LDAP\User\Manager;
 use OCP\GroupInterface;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 /**
@@ -49,18 +51,18 @@ use Test\TestCase;
  */
 class Group_LDAPTest extends TestCase {
 	/**
-	 * @return \PHPUnit_Framework_MockObject_MockObject|Access
+	 * @return MockObject|Access
 	 */
 	private function getAccessMock() {
 		static $conMethods;
 		static $accMethods;
 
 		if (is_null($conMethods) || is_null($accMethods)) {
-			$conMethods = get_class_methods('\OCA\User_LDAP\Connection');
-			$accMethods = get_class_methods('\OCA\User_LDAP\Access');
+			$conMethods = get_class_methods(Connection::class);
+			$accMethods = get_class_methods(Access::class);
 		}
 		$lw = $this->createMock(ILDAPWrapper::class);
-		$connector = $this->getMockBuilder('\OCA\User_LDAP\Connection')
+		$connector = $this->getMockBuilder(Connection::class)
 			->setMethods($conMethods)
 			->setConstructorArgs([$lw, null, null])
 			->getMock();
@@ -74,14 +76,14 @@ class Group_LDAPTest extends TestCase {
 		return $access;
 	}
 
+	/**
+	 * @return MockObject|GroupPluginManager
+	 */
 	private function getPluginManagerMock() {
-		return $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')->getMock();
+		return $this->createMock(GroupPluginManager::class);
 	}
 
-	/**
-	 * @param Access|\PHPUnit_Framework_MockObject_MockObject $access
-	 */
-	private function enableGroups($access) {
+	private function enableGroups(Access $access) {
 		$access->connection = $this->createMock(Connection::class);
 
 		$access->connection->expects($this->any())
@@ -104,7 +106,6 @@ class Group_LDAPTest extends TestCase {
 		$access->expects($this->any())
 			->method('groupname2dn')
 			->willReturn($groupDN);
-
 		$access->expects($this->any())
 			->method('readAttribute')
 			->willReturnCallback(function ($dn) use ($groupDN) {
@@ -121,7 +122,6 @@ class Group_LDAPTest extends TestCase {
 		$access->expects($this->any())
 			->method('isDNPartOfBase')
 			->willReturn(true);
-
 		// for primary groups
 		$access->expects($this->once())
 			->method('countUsers')
@@ -166,6 +166,9 @@ class Group_LDAPTest extends TestCase {
 		$access->expects($this->any())
 			->method('isDNPartOfBase')
 			->willReturn(true);
+		$access->expects($this->any())
+			->method('escapeFilterPart')
+			->willReturnArgument(0);
 
 		$groupBackend = new GroupLDAP($access, $pluginManager);
 		$users = $groupBackend->countUsersInGroup('group', '3');
@@ -174,8 +177,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testCountUsersWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'countUsersInGroup'])
 			->getMock();
 
@@ -642,11 +645,9 @@ class Group_LDAPTest extends TestCase {
 		$access->expects($this->any())
 			->method('username2dn')
 			->willReturn($dn);
-
 		$access->expects($this->exactly(5))
 			->method('readAttribute')
 			->will($this->onConsecutiveCalls(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar'], [], [], [], []));
-
 		$access->expects($this->any())
 			->method('dn2groupname')
 			->willReturnArgument(0);
@@ -776,8 +777,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testCreateGroupWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'createGroup'])
 			->getMock();
 
@@ -803,8 +804,8 @@ class Group_LDAPTest extends TestCase {
 	public function testCreateGroupFailing() {
 		$this->expectException(\Exception::class);
 
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'createGroup'])
 			->getMock();
 
@@ -822,8 +823,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testDeleteGroupWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'deleteGroup'])
 			->getMock();
 
@@ -837,7 +838,7 @@ class Group_LDAPTest extends TestCase {
 			->with('gid')
 			->willReturn('result');
 
-		$mapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\GroupMapping')
+		$mapper = $this->getMockBuilder(GroupMapping::class)
 			->setMethods(['unmap'])
 			->disableOriginalConstructor()
 			->getMock();
@@ -858,8 +859,8 @@ class Group_LDAPTest extends TestCase {
 	public function testDeleteGroupFailing() {
 		$this->expectException(\Exception::class);
 
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'deleteGroup'])
 			->getMock();
 
@@ -877,8 +878,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testAddToGroupWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'addToGroup'])
 			->getMock();
 
@@ -904,8 +905,8 @@ class Group_LDAPTest extends TestCase {
 	public function testAddToGroupFailing() {
 		$this->expectException(\Exception::class);
 
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'addToGroup'])
 			->getMock();
 
@@ -923,8 +924,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testRemoveFromGroupWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'removeFromGroup'])
 			->getMock();
 
@@ -950,8 +951,8 @@ class Group_LDAPTest extends TestCase {
 	public function testRemoveFromGroupFailing() {
 		$this->expectException(\Exception::class);
 
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'removeFromGroup'])
 			->getMock();
 
@@ -969,8 +970,8 @@ class Group_LDAPTest extends TestCase {
 	}
 
 	public function testGetGroupDetailsWithPlugin() {
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'getGroupDetails'])
 			->getMock();
 
@@ -996,8 +997,8 @@ class Group_LDAPTest extends TestCase {
 	public function testGetGroupDetailsFailing() {
 		$this->expectException(\Exception::class);
 
-		/** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */
-		$pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')
+		/** @var GroupPluginManager|MockObject $pluginManager */
+		$pluginManager = $this->getMockBuilder(GroupPluginManager::class)
 			->setMethods(['implementsActions', 'getGroupDetails'])
 			->getMock();
 

+ 1 - 0
lib/composer/composer/autoload_classmap.php

@@ -408,6 +408,7 @@ return array(
     'OCP\\Notification\\AlreadyProcessedException' => $baseDir . '/lib/public/Notification/AlreadyProcessedException.php',
     'OCP\\Notification\\IAction' => $baseDir . '/lib/public/Notification/IAction.php',
     'OCP\\Notification\\IApp' => $baseDir . '/lib/public/Notification/IApp.php',
+    'OCP\\Notification\\IDeferrableApp' => $baseDir . '/lib/public/Notification/IDeferrableApp.php',
     'OCP\\Notification\\IDismissableNotifier' => $baseDir . '/lib/public/Notification/IDismissableNotifier.php',
     'OCP\\Notification\\IManager' => $baseDir . '/lib/public/Notification/IManager.php',
     'OCP\\Notification\\INotification' => $baseDir . '/lib/public/Notification/INotification.php',

+ 1 - 0
lib/composer/composer/autoload_static.php

@@ -437,6 +437,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Notification\\AlreadyProcessedException' => __DIR__ . '/../../..' . '/lib/public/Notification/AlreadyProcessedException.php',
         'OCP\\Notification\\IAction' => __DIR__ . '/../../..' . '/lib/public/Notification/IAction.php',
         'OCP\\Notification\\IApp' => __DIR__ . '/../../..' . '/lib/public/Notification/IApp.php',
+        'OCP\\Notification\\IDeferrableApp' => __DIR__ . '/../../..' . '/lib/public/Notification/IDeferrableApp.php',
         'OCP\\Notification\\IDismissableNotifier' => __DIR__ . '/../../..' . '/lib/public/Notification/IDismissableNotifier.php',
         'OCP\\Notification\\IManager' => __DIR__ . '/../../..' . '/lib/public/Notification/IManager.php',
         'OCP\\Notification\\INotification' => __DIR__ . '/../../..' . '/lib/public/Notification/INotification.php',

+ 7 - 16
lib/private/BackgroundJob/Job.php

@@ -28,29 +28,20 @@
 namespace OC\BackgroundJob;
 
 use OCP\BackgroundJob\IJob;
+use OCP\BackgroundJob\IJobList;
 use OCP\ILogger;
 
 abstract class Job implements IJob {
-	/**
-	 * @var int $id
-	 */
+	/** @var int */
 	protected $id;
 
-	/**
-	 * @var int $lastRun
-	 */
+	/** @var int */
 	protected $lastRun;
 
-	/**
-	 * @var mixed $argument
-	 */
+	/** @var mixed */
 	protected $argument;
 
-	/**
-	 * @param JobList $jobList
-	 * @param ILogger|null $logger
-	 */
-	public function execute($jobList, ILogger $logger = null) {
+	public function execute(IJobList $jobList, ILogger $logger = null) {
 		$jobList->setLastRun($this);
 		if ($logger === null) {
 			$logger = \OC::$server->getLogger();
@@ -76,11 +67,11 @@ abstract class Job implements IJob {
 
 	abstract protected function run($argument);
 
-	public function setId($id) {
+	public function setId(int $id) {
 		$this->id = $id;
 	}
 
-	public function setLastRun($lastRun) {
+	public function setLastRun(int $lastRun) {
 		$this->lastRun = $lastRun;
 	}
 

+ 2 - 1
lib/private/BackgroundJob/TimedJob.php

@@ -25,6 +25,7 @@
 
 namespace OC\BackgroundJob;
 
+use OCP\BackgroundJob\IJobList;
 use OCP\ILogger;
 
 /**
@@ -49,7 +50,7 @@ abstract class TimedJob extends Job {
 	/**
 	 * run the job if
 	 *
-	 * @param JobList $jobList
+	 * @param IJobList $jobList
 	 * @param ILogger|null $logger
 	 */
 	public function execute($jobList, ILogger $logger = null) {

+ 5 - 1
lib/private/Lock/MemcacheLockingProvider.php

@@ -95,8 +95,12 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 */
 	public function releaseLock(string $path, int $type) {
 		if ($type === self::LOCK_SHARED) {
+			$ownSharedLockCount = $this->getOwnSharedLockCount($path);
 			$newValue = 0;
-			if ($this->getOwnSharedLockCount($path) === 1) {
+			if ($ownSharedLockCount === 0) { // if we are not holding the lock, don't try to release it
+				return;
+			}
+			if ($ownSharedLockCount === 1) {
 				$removed = $this->memcache->cad($path, 1); // if we're the only one having a shared lock we can remove it in one go
 				if (!$removed) { //someone else also has a shared lock, decrease only
 					$newValue = $this->memcache->dec($path);

+ 44 - 0
lib/private/Notification/Manager.php

@@ -31,6 +31,7 @@ use OCP\AppFramework\QueryException;
 use OCP\ILogger;
 use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\IApp;
+use OCP\Notification\IDeferrableApp;
 use OCP\Notification\IDismissableNotifier;
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
@@ -55,6 +56,8 @@ class Manager implements IManager {
 
 	/** @var bool */
 	protected $preparingPushNotification;
+	/** @var bool */
+	protected $deferPushing;
 
 	public function __construct(IValidator $validator,
 								ILogger $logger) {
@@ -65,6 +68,7 @@ class Manager implements IManager {
 		$this->appClasses = [];
 		$this->notifierClasses = [];
 		$this->preparingPushNotification = false;
+		$this->deferPushing = false;
 	}
 	/**
 	 * @param string $appClass The service must implement IApp, otherwise a
@@ -199,6 +203,46 @@ class Manager implements IManager {
 		return $this->preparingPushNotification;
 	}
 
+	/**
+	 * The calling app should only "flush" when it got returned true on the defer call
+	 * @return bool
+	 * @since 20.0.0
+	 */
+	public function defer(): bool {
+		$alreadyDeferring = $this->deferPushing;
+		$this->deferPushing = true;
+
+		$apps = $this->getApps();
+
+		foreach ($apps as $app) {
+			if ($app instanceof IDeferrableApp) {
+				$app->defer();
+			}
+		}
+
+		return !$alreadyDeferring;
+	}
+
+	/**
+	 * @since 20.0.0
+	 */
+	public function flush(): void {
+		$apps = $this->getApps();
+
+		foreach ($apps as $app) {
+			if (!$app instanceof IDeferrableApp) {
+				continue;
+			}
+
+			try {
+				$app->flush();
+			} catch (\InvalidArgumentException $e) {
+			}
+		}
+
+		$this->deferPushing = false;
+	}
+
 	/**
 	 * @param INotification $notification
 	 * @throws \InvalidArgumentException When the notification is not valid

+ 4 - 6
lib/public/BackgroundJob/IJob.php

@@ -38,23 +38,21 @@ interface IJob {
 	/**
 	 * Run the background job with the registered argument
 	 *
-	 * @param \OCP\BackgroundJob\IJobList $jobList The job list that manages the state of this job
+	 * @param IJobList $jobList The job list that manages the state of this job
 	 * @param ILogger|null $logger
 	 * @since 7.0.0
 	 */
-	public function execute($jobList, ILogger $logger = null);
+	public function execute(IJobList $jobList, ILogger $logger = null);
 
 	/**
-	 * @param int $id
 	 * @since 7.0.0
 	 */
-	public function setId($id);
+	public function setId(int $id);
 
 	/**
-	 * @param int $lastRun
 	 * @since 7.0.0
 	 */
-	public function setLastRun($lastRun);
+	public function setLastRun(int $lastRun);
 
 	/**
 	 * @param mixed $argument

+ 48 - 0
lib/public/Notification/IDeferrableApp.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2020, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCP\Notification;
+
+/**
+ * Interface IDeferrableApp
+ *
+ * @package OCP\Notification
+ * @since 20.0.0
+ */
+interface IDeferrableApp extends IApp {
+	/**
+	 * Start deferring notifications until `flush()` is called
+	 *
+	 * @since 20.0.0
+	 */
+	public function defer(): void;
+
+	/**
+	 * Send all deferred notifications that have been stored since `defer()` was called
+	 *
+	 * @since 20.0.0
+	 */
+	public function flush(): void;
+}

+ 17 - 0
lib/public/Notification/IManager.php

@@ -91,4 +91,21 @@ interface IManager extends IApp, INotifier {
 	 * @since 18.0.0
 	 */
 	public function dismissNotification(INotification $notification): void;
+
+	/**
+	 * Start deferring notifications until `flush()` is called
+	 *
+	 * The calling app should only "flush" when it got returned true on the defer call,
+	 * otherwise another app is deferring the sending already.
+	 * @return bool
+	 * @since 20.0.0
+	 */
+	public function defer(): bool;
+
+	/**
+	 * Send all deferred notifications that have been stored since `defer()` was called
+	 *
+	 * @since 20.0.0
+	 */
+	public function flush(): void;
 }

Some files were not shown because too many files changed in this diff