Fix types in the Group Manager

Psalm found an issue. However the issue found was because of lying
docblocks. Fixed those and did some typing to make it all better.

For #25839

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This commit is contained in:
Roeland Jago Douma 2021-03-02 19:34:20 +01:00
parent 252d2d3958
commit 68ec18323d
6 changed files with 16 additions and 11 deletions

View File

@ -76,7 +76,7 @@ class UserGlobalStoragesService extends GlobalStoragesService {
$userMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_USER, $this->getUser()->getUID()); $userMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_USER, $this->getUser()->getUID());
$globalMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_GLOBAL, null); $globalMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_GLOBAL, null);
$groups = $this->groupManager->getUserGroupIds($this->getUser()); $groups = $this->groupManager->getUserGroupIds($this->getUser());
if (is_array($groups) && count($groups) !== 0) { if (count($groups) !== 0) {
$groupMounts = $this->dbConfig->getAdminMountsForMultiple(DBConfigService::APPLICABLE_TYPE_GROUP, $groups); $groupMounts = $this->dbConfig->getAdminMountsForMultiple(DBConfigService::APPLICABLE_TYPE_GROUP, $groups);
} else { } else {
$groupMounts = []; $groupMounts = [];

View File

@ -234,6 +234,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
) )
); );
$securityMiddleware = new SecurityMiddleware( $securityMiddleware = new SecurityMiddleware(
$c->get(IRequest::class), $c->get(IRequest::class),
$c->get(IControllerMethodReflector::class), $c->get(IControllerMethodReflector::class),
@ -242,7 +244,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$server->query(ILogger::class), $server->query(ILogger::class),
$c->get('AppName'), $c->get('AppName'),
$server->getUserSession()->isLoggedIn(), $server->getUserSession()->isLoggedIn(),
$server->getGroupManager()->isAdmin($this->getUserId()), $this->getUserId() !== null && $server->getGroupManager()->isAdmin($this->getUserId()),
$server->getUserSession()->getUser() !== null && $server->query(ISubAdmin::class)->isSubAdmin($server->getUserSession()->getUser()), $server->getUserSession()->getUser() !== null && $server->query(ISubAdmin::class)->isSubAdmin($server->getUserSession()->getUser()),
$server->getAppManager(), $server->getAppManager(),
$server->getL10N('lib') $server->getL10N('lib')

View File

@ -276,7 +276,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @param string $uid the user id * @param string $uid the user id
* @return \OC\Group\Group[] * @return \OC\Group\Group[]
*/ */
public function getUserIdGroups($uid) { public function getUserIdGroups(string $uid): array {
$groups = []; $groups = [];
foreach ($this->getUserIdGroupIds($uid) as $groupId) { foreach ($this->getUserIdGroupIds($uid) as $groupId) {
@ -321,17 +321,17 @@ class Manager extends PublicEmitter implements IGroupManager {
* get a list of group ids for a user * get a list of group ids for a user
* *
* @param IUser $user * @param IUser $user
* @return array with group ids * @return string[] with group ids
*/ */
public function getUserGroupIds(IUser $user) { public function getUserGroupIds(IUser $user): array {
return $this->getUserIdGroupIds($user->getUID()); return $this->getUserIdGroupIds($user->getUID());
} }
/** /**
* @param string $uid the user id * @param string $uid the user id
* @return GroupInterface[] * @return string[]
*/ */
private function getUserIdGroupIds($uid) { private function getUserIdGroupIds(string $uid): array {
if (!isset($this->cachedUserGroups[$uid])) { if (!isset($this->cachedUserGroups[$uid])) {
$groups = []; $groups = [];
foreach ($this->backends as $backend) { foreach ($this->backends as $backend) {

View File

@ -167,7 +167,7 @@ class JSConfigHelper {
$countOfDataLocation = 0; $countOfDataLocation = 0;
$dataLocation = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''), $countOfDataLocation); $dataLocation = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''), $countOfDataLocation);
if ($countOfDataLocation !== 1 || !$this->groupManager->isAdmin($uid)) { if ($countOfDataLocation !== 1 || $uid === null || !$this->groupManager->isAdmin($uid)) {
$dataLocation = false; $dataLocation = false;
} }
@ -198,7 +198,7 @@ class JSConfigHelper {
$array = [ $array = [
"_oc_debug" => $this->config->getSystemValue('debug', false) ? 'true' : 'false', "_oc_debug" => $this->config->getSystemValue('debug', false) ? 'true' : 'false',
"_oc_isadmin" => $this->groupManager->isAdmin($uid) ? 'true' : 'false', "_oc_isadmin" => $uid !== null && $this->groupManager->isAdmin($uid) ? 'true' : 'false',
"backendAllowsPasswordConfirmation" => $userBackendAllowsPasswordConfirmation ? 'true' : 'false', "backendAllowsPasswordConfirmation" => $userBackendAllowsPasswordConfirmation ? 'true' : 'false',
"oc_dataURL" => is_string($dataLocation) ? "\"" . $dataLocation . "\"" : 'false', "oc_dataURL" => is_string($dataLocation) ? "\"" . $dataLocation . "\"" : 'false',
"_oc_webroot" => "\"" . \OC::$WEBROOT . "\"", "_oc_webroot" => "\"" . \OC::$WEBROOT . "\"",

View File

@ -112,10 +112,10 @@ interface IGroupManager {
/** /**
* @param \OCP\IUser $user * @param \OCP\IUser $user
* @return array with group names * @return string[] with group names
* @since 8.0.0 * @since 8.0.0
*/ */
public function getUserGroupIds(IUser $user); public function getUserGroupIds(IUser $user): array;
/** /**
* get a list of all display names in a group * get a list of all display names in a group

View File

@ -395,6 +395,7 @@ class ManagerTest extends TestCase {
*/ */
$backend = $this->getTestBackend(); $backend = $this->getTestBackend();
$backend->method('getUserGroups') $backend->method('getUserGroups')
->with('myUID')
->willReturn(['123', 'abc']); ->willReturn(['123', 'abc']);
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger); $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger);
@ -402,6 +403,8 @@ class ManagerTest extends TestCase {
/** @var \OC\User\User|\PHPUnit\Framework\MockObject\MockObject $user */ /** @var \OC\User\User|\PHPUnit\Framework\MockObject\MockObject $user */
$user = $this->createMock(IUser::class); $user = $this->createMock(IUser::class);
$user->method('getUID')
->willReturn('myUID');
$groups = $manager->getUserGroupIds($user); $groups = $manager->getUserGroupIds($user);
$this->assertCount(2, $groups); $this->assertCount(2, $groups);