diff --git a/apps/workflowengine/lib/BackgroundJobs/Rotate.php b/apps/workflowengine/lib/BackgroundJobs/Rotate.php index d7984b1226a8e..0479211d92599 100644 --- a/apps/workflowengine/lib/BackgroundJobs/Rotate.php +++ b/apps/workflowengine/lib/BackgroundJobs/Rotate.php @@ -9,6 +9,7 @@ use OCA\WorkflowEngine\AppInfo\Application; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +use OCP\IAppConfig; use OCP\IConfig; use OCP\Log\RotationTrait; use OCP\Server; @@ -21,17 +22,18 @@ public function __construct(ITimeFactory $time) { $this->setInterval(60 * 60 * 3); } - protected function run($argument) { + protected function run($argument): void { $config = Server::get(IConfig::class); - $default = $config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/flow.log'; - $this->filePath = trim((string)$config->getAppValue(Application::APP_ID, 'logfile', $default)); + $appConfig = Server::get(IAppConfig::class); + $default = $config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/flow.log'; + $this->filePath = trim($appConfig->getValueString(Application::APP_ID, 'logfile', $default)); if ($this->filePath === '') { // disabled, nothing to do return; } - $this->maxSize = $config->getSystemValue('log_rotate_size', 100 * 1024 * 1024); + $this->maxSize = $config->getSystemValueInt('log_rotate_size', 100 * 1024 * 1024); if ($this->shouldRotateBySize()) { $this->rotate(); diff --git a/apps/workflowengine/lib/Check/AbstractStringCheck.php b/apps/workflowengine/lib/Check/AbstractStringCheck.php index d92e990136509..2d564705c1917 100644 --- a/apps/workflowengine/lib/Check/AbstractStringCheck.php +++ b/apps/workflowengine/lib/Check/AbstractStringCheck.php @@ -12,8 +12,8 @@ abstract class AbstractStringCheck implements ICheck { - /** @var array[] Nested array: [Pattern => [ActualValue => Regex Result]] */ - protected $matches; + /** @var array> $matches Nested array: [Pattern => [ActualValue => Regex Result]] */ + protected array $matches; /** * @param IL10N $l @@ -64,13 +64,13 @@ protected function executeStringCheck($operator, $checkValue, $actualValue) { * @param string $value * @throws \UnexpectedValueException */ - public function validateCheck($operator, $value) { + public function validateCheck($operator, $value): void { if (!in_array($operator, ['is', '!is', 'matches', '!matches'])) { throw new \UnexpectedValueException($this->l->t('The given operator is invalid'), 1); } if (in_array($operator, ['matches', '!matches']) - && @preg_match($value, null) === false) { + && @preg_match($value, '') === false) { throw new \UnexpectedValueException($this->l->t('The given regular expression is invalid'), 2); } } @@ -85,12 +85,7 @@ public function isAvailableForScope(int $scope): bool { return $scope === IManager::SCOPE_ADMIN; } - /** - * @param string $pattern - * @param string $subject - * @return int|bool - */ - protected function match($pattern, $subject) { + protected function match(string $pattern, string $subject): int|false { $patternHash = md5($pattern); $subjectHash = md5($subject); if (isset($this->matches[$patternHash][$subjectHash])) { diff --git a/apps/workflowengine/lib/Check/FileSize.php b/apps/workflowengine/lib/Check/FileSize.php index 5ee03ccc9cf7c..7d3d4e4782769 100644 --- a/apps/workflowengine/lib/Check/FileSize.php +++ b/apps/workflowengine/lib/Check/FileSize.php @@ -14,41 +14,32 @@ class FileSize implements ICheck { - /** @var int */ - protected $size; + protected int|float|null $size = null; - /** - * @param IL10N $l - * @param IRequest $request - */ public function __construct( - protected IL10N $l, - protected IRequest $request, + protected readonly IL10N $l, + protected readonly IRequest $request, ) { } /** * @param string $operator * @param string $value - * @return bool */ - public function executeCheck($operator, $value) { + public function executeCheck($operator, $value): bool { $size = $this->getFileSizeFromHeader(); + if ($size === false) { + return false; + } $value = Util::computerFileSize($value); - if ($size !== false) { - switch ($operator) { - case 'less': - return $size < $value; - case '!less': - return $size >= $value; - case 'greater': - return $size > $value; - case '!greater': - return $size <= $value; - } - } - return false; + return match ($operator) { + 'less' => $size < $value, + '!less' => $size >= $value, + 'greater' => $size > $value, + '!greater' => $size <= $value, + default => false, + }; } /** @@ -56,7 +47,7 @@ public function executeCheck($operator, $value) { * @param string $value * @throws \UnexpectedValueException */ - public function validateCheck($operator, $value) { + public function validateCheck($operator, $value): void { if (!in_array($operator, ['less', '!less', 'greater', '!greater'])) { throw new \UnexpectedValueException($this->l->t('The given operator is invalid'), 1); } @@ -66,10 +57,7 @@ public function validateCheck($operator, $value) { } } - /** - * @return string - */ - protected function getFileSizeFromHeader() { + protected function getFileSizeFromHeader(): int|float|false { if ($this->size !== null) { return $this->size; } @@ -81,11 +69,11 @@ protected function getFileSizeFromHeader() { } } - if ($size === '') { + if ($size === '' || !is_numeric($size)) { $size = false; } - $this->size = $size; + $this->size = Util::numericToNumber($size); return $this->size; } diff --git a/apps/workflowengine/lib/Check/RequestRemoteAddress.php b/apps/workflowengine/lib/Check/RequestRemoteAddress.php index b6f8fef5aedb1..284cf87b50ffe 100644 --- a/apps/workflowengine/lib/Check/RequestRemoteAddress.php +++ b/apps/workflowengine/lib/Check/RequestRemoteAddress.php @@ -32,13 +32,13 @@ public function executeCheck($operator, $value) { $decodedValue = explode('/', $value); if ($operator === 'matchesIPv4') { - return $this->matchIPv4($actualValue, $decodedValue[0], $decodedValue[1]); + return $this->matchIPv4($actualValue, $decodedValue[0], (int)$decodedValue[1]); } elseif ($operator === '!matchesIPv4') { - return !$this->matchIPv4($actualValue, $decodedValue[0], $decodedValue[1]); + return !$this->matchIPv4($actualValue, $decodedValue[0], (int)$decodedValue[1]); } elseif ($operator === 'matchesIPv6') { - return $this->matchIPv6($actualValue, $decodedValue[0], $decodedValue[1]); + return $this->matchIPv6($actualValue, $decodedValue[0], (int)$decodedValue[1]); } else { - return !$this->matchIPv6($actualValue, $decodedValue[0], $decodedValue[1]); + return !$this->matchIPv6($actualValue, $decodedValue[0], (int)$decodedValue[1]); } } @@ -76,12 +76,8 @@ public function validateCheck($operator, $value) { /** * Based on https://stackoverflow.com/a/594134 - * @param string $ip - * @param string $rangeIp - * @param int $bits - * @return bool */ - protected function matchIPv4($ip, $rangeIp, $bits) { + protected function matchIPv4(string $ip, string $rangeIp, int $bits): bool { $rangeDecimal = ip2long($rangeIp); $ipDecimal = ip2long($ip); $mask = -1 << (32 - $bits); @@ -90,12 +86,8 @@ protected function matchIPv4($ip, $rangeIp, $bits) { /** * Based on https://stackoverflow.com/a/7951507 - * @param string $ip - * @param string $rangeIp - * @param int $bits - * @return bool */ - protected function matchIPv6($ip, $rangeIp, $bits) { + protected function matchIPv6(string $ip, string $rangeIp, int $bits): bool { $ipNet = inet_pton($ip); $binaryIp = $this->ipv6ToBits($ipNet); $ipNetBits = substr($binaryIp, 0, $bits); @@ -109,10 +101,8 @@ protected function matchIPv6($ip, $rangeIp, $bits) { /** * Based on https://stackoverflow.com/a/7951507 - * @param string $packedIp - * @return string */ - protected function ipv6ToBits($packedIp) { + protected function ipv6ToBits(string $packedIp): string { $unpackedIp = unpack('A16', $packedIp); $unpackedIp = str_split($unpackedIp[1]); $binaryIp = ''; diff --git a/apps/workflowengine/lib/Check/TFileCheck.php b/apps/workflowengine/lib/Check/TFileCheck.php index a514352e0473a..336542026dafa 100644 --- a/apps/workflowengine/lib/Check/TFileCheck.php +++ b/apps/workflowengine/lib/Check/TFileCheck.php @@ -8,7 +8,6 @@ */ namespace OCA\WorkflowEngine\Check; -use OCA\WorkflowEngine\AppInfo\Application; use OCA\WorkflowEngine\Entity\File; use OCP\Files\Node; use OCP\Files\NotFoundException; @@ -44,8 +43,7 @@ public function setEntitySubject(IEntity $entity, $subject): void { if ($entity instanceof File) { if (!$subject instanceof Node) { throw new \UnexpectedValueException( - 'Expected Node subject for File entity, got {class}', - ['app' => Application::APP_ID, 'class' => get_class($subject)] + 'Expected Node subject for File entity, got ' . get_class($subject), ); } $this->storage = $subject->getStorage(); diff --git a/apps/workflowengine/lib/Controller/GlobalWorkflowsController.php b/apps/workflowengine/lib/Controller/GlobalWorkflowsController.php index 001c673df3536..f7cfb1527bb44 100644 --- a/apps/workflowengine/lib/Controller/GlobalWorkflowsController.php +++ b/apps/workflowengine/lib/Controller/GlobalWorkflowsController.php @@ -13,8 +13,7 @@ class GlobalWorkflowsController extends AWorkflowController { - /** @var ScopeContext */ - private $scopeContext; + private ?ScopeContext $scopeContext = null; protected function getScopeContext(): ScopeContext { if ($this->scopeContext === null) { diff --git a/apps/workflowengine/lib/Entity/File.php b/apps/workflowengine/lib/Entity/File.php index 64d552e173717..6e85af2143277 100644 --- a/apps/workflowengine/lib/Entity/File.php +++ b/apps/workflowengine/lib/Entity/File.php @@ -21,7 +21,6 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; -use OCP\SystemTag\ISystemTag; use OCP\SystemTag\ISystemTagManager; use OCP\SystemTag\MapperEvent; use OCP\WorkflowEngine\EntityContext\IContextPortation; @@ -34,16 +33,10 @@ class File implements IEntity, IDisplayText, IUrl, IIcon, IContextPortation { private const EVENT_NAMESPACE = '\OCP\Files::'; - /** @var string */ - protected $eventName; - /** @var Event */ - protected $event; - /** @var ?Node */ - private $node; - /** @var ?IUser */ - private $actingUser = null; - /** @var UserMountCache */ - private $userMountCache; + protected ?string $eventName = null; + protected ?Event $event = null; + private ?Node $node = null; + private ?IUser $actingUser = null; public function __construct( protected IL10N $l10n, @@ -52,10 +45,9 @@ public function __construct( private IUserSession $userSession, private ISystemTagManager $tagManager, private IUserManager $userManager, - UserMountCache $userMountCache, + private UserMountCache $userMountCache, private IMountManager $mountManager, ) { - $this->userMountCache = $userMountCache; } public function getName(): string { @@ -185,7 +177,6 @@ public function getDisplayText(int $verbosity = 0): string { $tagIDs = $this->event->getTags(); $tagObjects = $this->tagManager->getTagsByIds($tagIDs); foreach ($tagObjects as $systemTag) { - /** @var ISystemTag $systemTag */ if ($systemTag->isUserVisible()) { $tagNames[] = $systemTag->getName(); } @@ -198,15 +189,15 @@ public function getDisplayText(int $verbosity = 0): string { } array_push($options, $tagString, $filename); return $this->l10n->t('%s assigned %s to %s', $options); + default: + return ''; } } public function getUrl(): string { try { return $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $this->getNode()->getId()]); - } catch (InvalidPathException $e) { - return ''; - } catch (NotFoundException $e) { + } catch (InvalidPathException|NotFoundException) { return ''; } } diff --git a/apps/workflowengine/lib/Helper/LogContext.php b/apps/workflowengine/lib/Helper/LogContext.php index 9d740680bb644..929e9efd664f0 100644 --- a/apps/workflowengine/lib/Helper/LogContext.php +++ b/apps/workflowengine/lib/Helper/LogContext.php @@ -13,8 +13,16 @@ use OCP\WorkflowEngine\IOperation; class LogContext { - /** @var array */ - protected $details; + /** @var array{ + * message?: string, + * scopes?: array, + * operation?: array{class: class-string, name: string}, + * entity?: array{class: class-string, name: string}, + * configuration?: array, + * eventName?: string, + * } + */ + protected array $details; public function setDescription(string $description): LogContext { $this->details['message'] = $description; diff --git a/apps/workflowengine/lib/Helper/ScopeContext.php b/apps/workflowengine/lib/Helper/ScopeContext.php index 05379f5ff4310..3a82001233d25 100644 --- a/apps/workflowengine/lib/Helper/ScopeContext.php +++ b/apps/workflowengine/lib/Helper/ScopeContext.php @@ -11,12 +11,9 @@ use OCP\WorkflowEngine\IManager; class ScopeContext { - /** @var int */ - private $scope; - /** @var string */ - private $scopeId; - /** @var string */ - private $hash; + private int $scope; + private string $scopeId; + private ?string $hash = null; public function __construct(int $scope, ?string $scopeId = null) { $this->scope = $this->evaluateScope($scope); @@ -38,16 +35,10 @@ private function evaluateScopeId(?string $scopeId = null): string { return trim((string)$scopeId); } - /** - * @return int - */ public function getScope(): int { return $this->scope; } - /** - * @return string - */ public function getScopeId(): string { return $this->scopeId; } diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php index 306f9cd410667..c402f84c83002 100644 --- a/apps/workflowengine/lib/Manager.php +++ b/apps/workflowengine/lib/Manager.php @@ -6,8 +6,6 @@ */ namespace OCA\WorkflowEngine; -use Doctrine\DBAL\Exception; -use OCA\WorkflowEngine\AppInfo\Application; use OCA\WorkflowEngine\Check\FileMimeType; use OCA\WorkflowEngine\Check\FileName; use OCA\WorkflowEngine\Check\FileSize; @@ -21,15 +19,14 @@ use OCA\WorkflowEngine\Helper\ScopeContext; use OCA\WorkflowEngine\Service\Logger; use OCA\WorkflowEngine\Service\RuleMatcher; -use OCP\AppFramework\QueryException; +use OCP\AppFramework\Services\IAppConfig; use OCP\Cache\CappedMemoryCache; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\ICacheFactory; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; -use OCP\IServerContainer; use OCP\IUserSession; use OCP\WorkflowEngine\Events\RegisterChecksEvent; use OCP\WorkflowEngine\Events\RegisterEntitiesEvent; @@ -41,36 +38,41 @@ use OCP\WorkflowEngine\IManager; use OCP\WorkflowEngine\IOperation; use OCP\WorkflowEngine\IRuleMatcher; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; +/** + * @psalm-type Check = array{id: int, class: class-string, operator: string, value: string, hash: string} + */ class Manager implements IManager { /** @var array[] */ - protected $operations = []; + protected array $operations = []; - /** @var array[] */ - protected $checks = []; + /** @var array */ + protected array $checks = []; /** @var IEntity[] */ - protected $registeredEntities = []; + protected array $registeredEntities = []; /** @var IOperation[] */ - protected $registeredOperators = []; + protected array $registeredOperators = []; /** @var ICheck[] */ - protected $registeredChecks = []; + protected array $registeredChecks = []; /** @var CappedMemoryCache */ protected CappedMemoryCache $operationsByScope; public function __construct( - protected IDBConnection $connection, - protected IServerContainer $container, - protected IL10N $l, - protected LoggerInterface $logger, - protected IUserSession $session, - private IEventDispatcher $dispatcher, - private IConfig $config, - private ICacheFactory $cacheFactory, + protected readonly IDBConnection $connection, + protected readonly ContainerInterface $container, + protected readonly IL10N $l, + protected readonly LoggerInterface $logger, + protected readonly IUserSession $session, + private readonly IEventDispatcher $dispatcher, + private readonly IAppConfig $appConfig, + private readonly ICacheFactory $cacheFactory, ) { $this->operationsByScope = new CappedMemoryCache(64); } @@ -81,7 +83,7 @@ public function getRuleMatcher(): IRuleMatcher { $this->container, $this->l, $this, - $this->container->query(Logger::class) + $this->container->get(Logger::class) ); } @@ -121,10 +123,11 @@ public function getAllConfiguredEvents() { } /** - * @param string $operationClass + * @param class-string $operationClass * @return ScopeContext[] */ public function getAllConfiguredScopesForOperation(string $operationClass): array { + /** @var array, ScopeContext[]> $scopesByOperation */ static $scopesByOperation = []; if (isset($scopesByOperation[$operationClass])) { return $scopesByOperation[$operationClass]; @@ -132,8 +135,8 @@ public function getAllConfiguredScopesForOperation(string $operationClass): arra try { /** @var IOperation $operation */ - $operation = $this->container->query($operationClass); - } catch (QueryException $e) { + $operation = $this->container->get($operationClass); + } catch (ContainerExceptionInterface $e) { return []; } @@ -187,8 +190,8 @@ public function getAllOperations(ScopeContext $scopeContext): array { while ($row = $result->fetch()) { try { /** @var IOperation $operation */ - $operation = $this->container->query($row['class']); - } catch (QueryException $e) { + $operation = $this->container->get($row['class']); + } catch (ContainerExceptionInterface $e) { continue; } @@ -261,7 +264,7 @@ protected function insertOperation( /** * @param string $class * @param string $name - * @param array[] $checks + * @param array $checks * @param string $operation * @return array The added operation * @throws \UnexpectedValueException @@ -379,13 +382,11 @@ public function updateOperation( } /** - * @param int $id - * @return bool * @throws \UnexpectedValueException * @throws Exception * @throws \DomainException */ - public function deleteOperation($id, ScopeContext $scopeContext) { + public function deleteOperation(int $id, ScopeContext $scopeContext): bool { if (!$this->canModify($id, $scopeContext)) { throw new \DomainException('Target operation not within scope'); }; @@ -397,7 +398,7 @@ public function deleteOperation($id, ScopeContext $scopeContext) { ->executeStatement(); if ($result) { $qb = $this->connection->getQueryBuilder(); - $result &= (bool)$qb->delete('flow_operations_scope') + $result = (bool)$qb->delete('flow_operations_scope') ->where($qb->expr()->eq('operation_id', $qb->createNamedParameter($id))) ->executeStatement(); } @@ -416,11 +417,14 @@ public function deleteOperation($id, ScopeContext $scopeContext) { return $result; } - protected function validateEvents(string $entity, array $events, IOperation $operation) { + /** + * @param class-string $entity + * @param array $events + */ + protected function validateEvents(string $entity, array $events, IOperation $operation): void { try { - /** @var IEntity $instance */ - $instance = $this->container->query($entity); - } catch (QueryException $e) { + $instance = $this->container->get($entity); + } catch (ContainerExceptionInterface $e) { throw new \UnexpectedValueException($this->l->t('Entity %s does not exist', [$entity])); } @@ -448,20 +452,16 @@ protected function validateEvents(string $entity, array $events, IOperation $ope } /** - * @param string $class - * @param string $name - * @param array[] $checks - * @param string $operation - * @param ScopeContext $scope - * @param string $entity + * @param class-string $class + * @param array $checks * @param array $events * @throws \UnexpectedValueException */ - public function validateOperation($class, $name, array $checks, $operation, ScopeContext $scope, string $entity, array $events) { + public function validateOperation(string $class, string $name, array $checks, string $operation, ScopeContext $scope, string $entity, array $events): void { try { /** @var IOperation $instance */ - $instance = $this->container->query($class); - } catch (QueryException $e) { + $instance = $this->container->get($class); + } catch (ContainerExceptionInterface $e) { throw new \UnexpectedValueException($this->l->t('Operation %s does not exist', [$class])); } @@ -479,7 +479,7 @@ public function validateOperation($class, $name, array $checks, $operation, Scop throw new \UnexpectedValueException($this->l->t('At least one check needs to be provided')); } - if (strlen((string)$operation) > IManager::MAX_OPERATION_VALUE_BYTES) { + if (strlen($operation) > IManager::MAX_OPERATION_VALUE_BYTES) { throw new \UnexpectedValueException($this->l->t('The provided operation data is too long')); } @@ -492,8 +492,8 @@ public function validateOperation($class, $name, array $checks, $operation, Scop try { /** @var ICheck $instance */ - $instance = $this->container->query($check['class']); - } catch (QueryException $e) { + $instance = $this->container->get($check['class']); + } catch (ContainerExceptionInterface) { throw new \UnexpectedValueException($this->l->t('Check %s does not exist', [$class])); } @@ -517,9 +517,9 @@ public function validateOperation($class, $name, array $checks, $operation, Scop /** * @param int[] $checkIds - * @return array[] + * @return array */ - public function getChecks(array $checkIds) { + public function getChecks(array $checkIds): array { $checkIds = array_map('intval', $checkIds); $checks = []; @@ -541,6 +541,7 @@ public function getChecks(array $checkIds) { $result = $query->executeQuery(); while ($row = $result->fetch()) { + /** @var Check $row */ $this->checks[(int)$row['id']] = $row; $checks[(int)$row['id']] = $row; } @@ -550,19 +551,16 @@ public function getChecks(array $checkIds) { if (!empty($checkIds)) { $missingCheck = array_pop($checkIds); - throw new \UnexpectedValueException($this->l->t('Check #%s does not exist', $missingCheck)); + throw new \UnexpectedValueException($this->l->t('Check #%s does not exist', (string)$missingCheck)); } return $checks; } /** - * @param string $class - * @param string $operator - * @param string $value * @return int Check unique ID */ - protected function addCheck($class, $operator, $value) { + protected function addCheck(string $class, string $operator, string $value): int { $hash = md5($class . '::' . $operator . '::' . $value); $query = $this->connection->getQueryBuilder(); @@ -664,9 +662,9 @@ public function registerCheck(ICheck $check): void { protected function getBuildInEntities(): array { try { return [ - File::class => $this->container->query(File::class), + File::class => $this->container->get(File::class), ]; - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return []; } @@ -680,7 +678,7 @@ protected function getBuildInOperators(): array { return [ // None yet ]; - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return []; } @@ -692,23 +690,23 @@ protected function getBuildInOperators(): array { protected function getBuildInChecks(): array { try { return [ - $this->container->query(FileMimeType::class), - $this->container->query(FileName::class), - $this->container->query(FileSize::class), - $this->container->query(FileSystemTags::class), - $this->container->query(RequestRemoteAddress::class), - $this->container->query(RequestTime::class), - $this->container->query(RequestURL::class), - $this->container->query(RequestUserAgent::class), - $this->container->query(UserGroupMembership::class), + $this->container->get(FileMimeType::class), + $this->container->get(FileName::class), + $this->container->get(FileSize::class), + $this->container->get(FileSystemTags::class), + $this->container->get(RequestRemoteAddress::class), + $this->container->get(RequestTime::class), + $this->container->get(RequestURL::class), + $this->container->get(RequestUserAgent::class), + $this->container->get(UserGroupMembership::class), ]; - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return []; } } public function isUserScopeEnabled(): bool { - return $this->config->getAppValue(Application::APP_ID, 'user_scope_disabled', 'no') === 'no'; + return !$this->appConfig->getAppValueBool('user_scope_disabled'); } } diff --git a/apps/workflowengine/lib/Service/Logger.php b/apps/workflowengine/lib/Service/Logger.php index 494240bc40385..5e85b7ba272f0 100644 --- a/apps/workflowengine/lib/Service/Logger.php +++ b/apps/workflowengine/lib/Service/Logger.php @@ -10,6 +10,7 @@ use OCA\WorkflowEngine\AppInfo\Application; use OCA\WorkflowEngine\Helper\LogContext; +use OCP\AppFramework\Services\IAppConfig; use OCP\IConfig; use OCP\ILogger; use OCP\Log\IDataLogger; @@ -23,19 +24,20 @@ public function __construct( protected LoggerInterface $generalLogger, private IConfig $config, private ILogFactory $logFactory, + private IAppConfig $appConfig, ) { $this->initLogger(); } protected function initLogger(): void { $default = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/flow.log'; - $logFile = trim((string)$this->config->getAppValue(Application::APP_ID, 'logfile', $default)); + $logFile = trim($this->appConfig->getAppValueString('logfile', $default)); if ($logFile !== '') { $this->flowLogger = $this->logFactory->getCustomPsrLogger($logFile); } } - public function logFlowRequests(LogContext $logContext) { + public function logFlowRequests(LogContext $logContext): void { $message = 'Flow activation: rules were requested for operation {op}'; $context = ['op' => $logContext->getDetails()['operation']['name'], 'level' => ILogger::DEBUG]; @@ -44,7 +46,7 @@ public function logFlowRequests(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logScopeExpansion(LogContext $logContext) { + public function logScopeExpansion(LogContext $logContext): void { $message = 'Flow rule of a different user is legit for operation {op}'; $context = ['op' => $logContext->getDetails()['operation']['name']]; @@ -53,7 +55,7 @@ public function logScopeExpansion(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logPassedCheck(LogContext $logContext) { + public function logPassedCheck(LogContext $logContext): void { $message = 'Flow rule qualified to run {op}, config: {config}'; $context = [ 'op' => $logContext->getDetails()['operation']['name'], @@ -66,7 +68,7 @@ public function logPassedCheck(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logRunSingle(LogContext $logContext) { + public function logRunSingle(LogContext $logContext): void { $message = 'Last qualified flow configuration is going to run {op}'; $context = [ 'op' => $logContext->getDetails()['operation']['name'], @@ -77,7 +79,7 @@ public function logRunSingle(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logRunAll(LogContext $logContext) { + public function logRunAll(LogContext $logContext): void { $message = 'All qualified flow configurations are going to run {op}'; $context = [ 'op' => $logContext->getDetails()['operation']['name'], @@ -88,7 +90,7 @@ public function logRunAll(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logRunNone(LogContext $logContext) { + public function logRunNone(LogContext $logContext): void { $message = 'No flow configurations is going to run {op}'; $context = [ 'op' => $logContext->getDetails()['operation']['name'], @@ -100,7 +102,7 @@ public function logRunNone(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logEventInit(LogContext $logContext) { + public function logEventInit(LogContext $logContext): void { $message = 'Flow activated by event {ev}'; $context = [ @@ -113,7 +115,7 @@ public function logEventInit(LogContext $logContext) { $this->log($message, $context, $logContext); } - public function logEventDone(LogContext $logContext) { + public function logEventDone(LogContext $logContext): void { $message = 'Flow handling done for event {ev}'; $context = [ diff --git a/apps/workflowengine/lib/Service/RuleMatcher.php b/apps/workflowengine/lib/Service/RuleMatcher.php index c95387e14eeed..8c7080d55d8bb 100644 --- a/apps/workflowengine/lib/Service/RuleMatcher.php +++ b/apps/workflowengine/lib/Service/RuleMatcher.php @@ -11,10 +11,8 @@ use OCA\WorkflowEngine\Helper\LogContext; use OCA\WorkflowEngine\Helper\ScopeContext; use OCA\WorkflowEngine\Manager; -use OCP\AppFramework\QueryException; use OCP\Files\Storage\IStorage; use OCP\IL10N; -use OCP\IServerContainer; use OCP\IUserSession; use OCP\WorkflowEngine\ICheck; use OCP\WorkflowEngine\IEntity; @@ -23,27 +21,27 @@ use OCP\WorkflowEngine\IManager; use OCP\WorkflowEngine\IOperation; use OCP\WorkflowEngine\IRuleMatcher; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\ContainerInterface; use RuntimeException; +/** + * @psalm-import-type Check from Manager + */ class RuleMatcher implements IRuleMatcher { - /** @var array */ - protected $contexts; - /** @var array */ - protected $fileInfo = []; - /** @var IOperation */ - protected $operation; - /** @var IEntity */ - protected $entity; - /** @var string */ - protected $eventName; + protected array $contexts; + protected array $fileInfo = []; + protected ?IOperation $operation = null; + protected ?IEntity $entity = null; + protected ?string $eventName = null; public function __construct( - protected IUserSession $session, - protected IServerContainer $container, - protected IL10N $l, - protected Manager $manager, - protected Logger $logger, + protected readonly IUserSession $session, + protected readonly ContainerInterface $container, + protected readonly IL10N $l, + protected readonly Manager $manager, + protected readonly Logger $logger, ) { } @@ -181,13 +179,13 @@ public function getMatchingOperations(string $class, bool $returnFirstMatchingOp } /** - * @param array $check + * @param Check $check * @return bool */ - public function check(array $check) { + public function check(array $check): bool { try { - $checkInstance = $this->container->query($check['class']); - } catch (QueryException $e) { + $checkInstance = $this->container->get($check['class']); + } catch (ContainerExceptionInterface $e) { // Check does not exist, assume it matches. return true; } diff --git a/apps/workflowengine/lib/Settings/ASettings.php b/apps/workflowengine/lib/Settings/ASettings.php index 23e958755de10..ffa9491136420 100644 --- a/apps/workflowengine/lib/Settings/ASettings.php +++ b/apps/workflowengine/lib/Settings/ASettings.php @@ -14,7 +14,6 @@ use OCP\AppFramework\Services\IInitialState; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; -use OCP\IL10N; use OCP\IURLGenerator; use OCP\Settings\ISettings; use OCP\WorkflowEngine\Events\LoadSettingsScriptsEvent; @@ -27,21 +26,16 @@ abstract class ASettings implements ISettings { public function __construct( - private string $appName, - private IL10N $l10n, - private IEventDispatcher $eventDispatcher, - protected Manager $manager, - private IInitialState $initialStateService, - private IConfig $config, - private IURLGenerator $urlGenerator, + private readonly IEventDispatcher $eventDispatcher, + protected readonly Manager $manager, + private readonly IInitialState $initialStateService, + private readonly IConfig $config, + private readonly IURLGenerator $urlGenerator, ) { } abstract public function getScope(): int; - /** - * @return TemplateResponse - */ public function getForm(): TemplateResponse { // @deprecated in 20.0.0: retire this one in favor of the typed event $this->eventDispatcher->dispatch( @@ -87,7 +81,7 @@ public function getForm(): TemplateResponse { } /** - * @return string the section ID, e.g. 'sharing' + * @return string|null the section ID, e.g. 'sharing' */ public function getSection(): ?string { return 'workflow'; @@ -104,9 +98,13 @@ public function getPriority(): int { return 0; } - private function entitiesToArray(array $entities) { - return array_map(function (IEntity $entity) { - $events = array_map(function (IEntityEvent $entityEvent) { + /** + * @param IEntity[] $entities + * @return array, icon: string, name: string, events: array}> + */ + private function entitiesToArray(array $entities): array { + return array_map(function (IEntity $entity): array { + $events = array_map(function (IEntityEvent $entityEvent): array { return [ 'eventName' => $entityEvent->getEventName(), 'displayName' => $entityEvent->getDisplayName() @@ -122,10 +120,8 @@ private function entitiesToArray(array $entities) { }, $entities); } - private function operatorsToArray(array $operators) { - $operators = array_filter($operators, function (IOperation $operator) { - return $operator->isAvailableForScope($this->getScope()); - }); + private function operatorsToArray(array $operators): array { + $operators = array_filter($operators, fn (IOperation $operator): bool => $operator->isAvailableForScope($this->getScope())); return array_map(function (IOperation $operator) { return [ @@ -140,10 +136,8 @@ private function operatorsToArray(array $operators) { }, $operators); } - private function checksToArray(array $checks) { - $checks = array_filter($checks, function (ICheck $check) { - return $check->isAvailableForScope($this->getScope()); - }); + private function checksToArray(array $checks): array { + $checks = array_filter($checks, fn (ICheck $check): bool => $check->isAvailableForScope($this->getScope())); return array_map(function (ICheck $check) { return [ diff --git a/apps/workflowengine/lib/Settings/Section.php b/apps/workflowengine/lib/Settings/Section.php index aa790c9ddcc74..99de4479dace5 100644 --- a/apps/workflowengine/lib/Settings/Section.php +++ b/apps/workflowengine/lib/Settings/Section.php @@ -17,36 +17,36 @@ class Section implements IIconSection { * @param IL10N $l */ public function __construct( - private IURLGenerator $url, - private IL10N $l, + private readonly IURLGenerator $url, + private readonly IL10N $l, ) { } /** * {@inheritdoc} */ - public function getID() { + public function getID(): string { return 'workflow'; } /** * {@inheritdoc} */ - public function getName() { + public function getName(): string { return $this->l->t('Flow'); } /** * {@inheritdoc} */ - public function getPriority() { + public function getPriority(): int { return 55; } /** * {@inheritdoc} */ - public function getIcon() { + public function getIcon(): string { return $this->url->imagePath(Application::APP_ID, 'app-dark.svg'); } } diff --git a/apps/workflowengine/tests/ManagerTest.php b/apps/workflowengine/tests/ManagerTest.php index 56e45936b8288..1b28c13898d16 100644 --- a/apps/workflowengine/tests/ManagerTest.php +++ b/apps/workflowengine/tests/ManagerTest.php @@ -7,21 +7,19 @@ namespace OCA\WorkflowEngine\Tests; use OC\Files\Config\UserMountCache; -use OC\L10N\L10N; use OCA\WorkflowEngine\Entity\File; use OCA\WorkflowEngine\Helper\ScopeContext; use OCA\WorkflowEngine\Manager; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Services\IAppConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\NodeCreatedEvent; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; use OCP\ICache; use OCP\ICacheFactory; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; -use OCP\IServerContainer; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\IUserSession; @@ -34,6 +32,7 @@ use OCP\WorkflowEngine\IManager; use OCP\WorkflowEngine\IOperation; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -44,31 +43,21 @@ * @group DB */ class ManagerTest extends TestCase { - /** @var Manager */ - protected $manager; - /** @var MockObject|IDBConnection */ - protected $db; - /** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */ - protected $logger; - /** @var MockObject|IServerContainer */ - protected $container; - /** @var MockObject|IUserSession */ - protected $session; - /** @var MockObject|L10N */ - protected $l; - /** @var MockObject|IEventDispatcher */ - protected $dispatcher; - /** @var MockObject|IConfig */ - protected $config; - /** @var MockObject|ICacheFactory */ - protected $cacheFactory; + protected Manager $manager; + protected IDBConnection $db; + protected LoggerInterface&MockObject $logger; + protected ContainerInterface&MockObject $container; + protected IUserSession&MockObject $session; + protected IL10N&MockObject $l; + protected IEventDispatcher&MockObject $dispatcher; + protected IAppConfig&MockObject $config; + protected ICacheFactory&MockObject $cacheFactory; protected function setUp(): void { parent::setUp(); $this->db = Server::get(IDBConnection::class); - $this->container = $this->createMock(IServerContainer::class); - /** @var IL10N|MockObject $l */ + $this->container = $this->createMock(ContainerInterface::class); $this->l = $this->createMock(IL10N::class); $this->l->method('t') ->willReturnCallback(function ($text, $parameters = []) { @@ -78,11 +67,11 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->session = $this->createMock(IUserSession::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); - $this->config = $this->createMock(IConfig::class); + $this->config = $this->createMock(IAppConfig::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->manager = new Manager( - Server::get(IDBConnection::class), + $this->db, $this->container, $this->l, $this->logger, @@ -99,10 +88,7 @@ protected function tearDown(): void { parent::tearDown(); } - /** - * @return MockObject|ScopeContext - */ - protected function buildScope(?string $scopeId = null): MockObject { + protected function buildScope(?string $scopeId = null): MockObject&ScopeContext { $scopeContext = $this->createMock(ScopeContext::class); $scopeContext->expects($this->any()) ->method('getScope') @@ -201,7 +187,7 @@ public function testGetAllOperations(): void { ]); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($adminOperation, $userOperation) { switch ($className) { case 'OCA\WFE\TestAdminOp': @@ -297,7 +283,7 @@ public function testGetOperations(): void { ]); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($operation) { switch ($className) { case 'OCA\WFE\TestOp': @@ -382,7 +368,7 @@ public function testUpdateOperation(): void { }); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($class) use ($operationMock) { if (substr($class, -2) === 'Op') { return $operationMock; @@ -496,7 +482,7 @@ public function testGetEntitiesListBuildInOnly(): void { $fileEntityMock = $this->createMock(File::class); $this->container->expects($this->once()) - ->method('query') + ->method('get') ->with(File::class) ->willReturn($fileEntityMock); @@ -510,11 +496,10 @@ public function testGetEntitiesList(): void { $fileEntityMock = $this->createMock(File::class); $this->container->expects($this->once()) - ->method('query') + ->method('get') ->with(File::class) ->willReturn($fileEntityMock); - /** @var MockObject|IEntity $extraEntity */ $extraEntity = $this->createMock(IEntity::class); $this->dispatcher->expects($this->once()) @@ -581,7 +566,7 @@ public function testValidateOperationOK(): void { ->method('validateCheck'); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) { switch ($className) { case IOperation::class: @@ -641,7 +626,7 @@ public function testValidateOperationCheckInputLengthError(): void { ->method('validateCheck'); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) { switch ($className) { case IOperation::class: @@ -705,7 +690,7 @@ public function testValidateOperationDataLengthError(): void { ->method('validateCheck'); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) { switch ($className) { case IOperation::class: @@ -769,7 +754,7 @@ public function testValidateOperationScopeNotAvailable(): void { ->method('validateCheck'); $this->container->expects($this->any()) - ->method('query') + ->method('get') ->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) { switch ($className) { case IOperation::class: diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index ef1831115672b..db48d49989015 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2618,43 +2618,6 @@ - - - - - - - - - - - - - size]]> - - - - - - size]]> - - - - - - - - - - - - - - - - Application::APP_ID, 'class' => get_class($subject)]]]> - - @@ -2665,58 +2628,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/lib/public/Util.php b/lib/public/Util.php index b3111c54fc70a..8e57e32fdec43 100644 --- a/lib/public/Util.php +++ b/lib/public/Util.php @@ -315,7 +315,7 @@ public static function getDefaultEmailAddress(string $user_part): string { } /** - * Converts string to int of float depending if it fits an int + * Converts string to int of float depending on if it fits an int * @param numeric-string|float|int $number numeric string * @return int|float int if it fits, float if it is too big * @since 26.0.0 diff --git a/lib/public/WorkflowEngine/IManager.php b/lib/public/WorkflowEngine/IManager.php index f66a9460f064b..f9655dfa35b49 100644 --- a/lib/public/WorkflowEngine/IManager.php +++ b/lib/public/WorkflowEngine/IManager.php @@ -6,11 +6,14 @@ */ namespace OCP\WorkflowEngine; +use OCP\AppFramework\Attribute\Consumable; + /** * Interface IManager * * @since 9.1 */ +#[Consumable(since: '9.1')] interface IManager { /** * @since 18.0.0