Merge pull request #55140 from nextcloud/carl/cleanup-workflowengine-app

refactor(psalm): Fix most issues with the workflowengine
pull/54834/merge
Joas Schilling 2 weeks ago committed by GitHub
commit 00f589e765
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 10
      apps/workflowengine/lib/BackgroundJobs/Rotate.php
  2. 15
      apps/workflowengine/lib/Check/AbstractStringCheck.php
  3. 48
      apps/workflowengine/lib/Check/FileSize.php
  4. 24
      apps/workflowengine/lib/Check/RequestRemoteAddress.php
  5. 4
      apps/workflowengine/lib/Check/TFileCheck.php
  6. 3
      apps/workflowengine/lib/Controller/GlobalWorkflowsController.php
  7. 25
      apps/workflowengine/lib/Entity/File.php
  8. 12
      apps/workflowengine/lib/Helper/LogContext.php
  9. 15
      apps/workflowengine/lib/Helper/ScopeContext.php
  10. 132
      apps/workflowengine/lib/Manager.php
  11. 20
      apps/workflowengine/lib/Service/Logger.php
  12. 40
      apps/workflowengine/lib/Service/RuleMatcher.php
  13. 40
      apps/workflowengine/lib/Settings/ASettings.php
  14. 12
      apps/workflowengine/lib/Settings/Section.php
  15. 63
      apps/workflowengine/tests/ManagerTest.php
  16. 89
      build/psalm-baseline.xml
  17. 2
      lib/public/Util.php
  18. 3
      lib/public/WorkflowEngine/IManager.php

@ -9,6 +9,7 @@ namespace OCA\WorkflowEngine\BackgroundJobs;
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 @@ class Rotate extends TimedJob {
$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();

@ -12,8 +12,8 @@ use OCP\WorkflowEngine\IManager;
abstract class AbstractStringCheck implements ICheck {
/** @var array[] Nested array: [Pattern => [ActualValue => Regex Result]] */
protected $matches;
/** @var array<string, array<string, false|int>> $matches Nested array: [Pattern => [ActualValue => Regex Result]] */
protected array $matches;
/**
* @param IL10N $l
@ -64,13 +64,13 @@ abstract class AbstractStringCheck implements ICheck {
* @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 @@ abstract class AbstractStringCheck implements ICheck {
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])) {

@ -14,41 +14,32 @@ use OCP\WorkflowEngine\ICheck;
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 @@ class FileSize implements ICheck {
* @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 @@ class FileSize implements ICheck {
}
}
/**
* @return string
*/
protected function getFileSizeFromHeader() {
protected function getFileSizeFromHeader(): int|float|false {
if ($this->size !== null) {
return $this->size;
}
@ -81,11 +69,11 @@ class FileSize implements ICheck {
}
}
if ($size === '') {
if ($size === '' || !is_numeric($size)) {
$size = false;
}
$this->size = $size;
$this->size = Util::numericToNumber($size);
return $this->size;
}

@ -32,13 +32,13 @@ class RequestRemoteAddress implements ICheck {
$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 @@ class RequestRemoteAddress implements ICheck {
/**
* 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 @@ class RequestRemoteAddress implements ICheck {
/**
* 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 @@ class RequestRemoteAddress implements ICheck {
/**
* 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 = '';

@ -8,7 +8,6 @@ declare(strict_types=1);
*/
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 @@ trait TFileCheck {
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();

@ -13,8 +13,7 @@ use OCP\WorkflowEngine\IManager;
class GlobalWorkflowsController extends AWorkflowController {
/** @var ScopeContext */
private $scopeContext;
private ?ScopeContext $scopeContext = null;
protected function getScopeContext(): ScopeContext {
if ($this->scopeContext === null) {

@ -21,7 +21,6 @@ use OCP\IURLGenerator;
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 @@ use OCP\WorkflowEngine\IRuleMatcher;
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 @@ class File implements IEntity, IDisplayText, IUrl, IIcon, IContextPortation {
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 @@ class File implements IEntity, IDisplayText, IUrl, IIcon, IContextPortation {
$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 @@ class File implements IEntity, IDisplayText, IUrl, IIcon, IContextPortation {
}
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 '';
}
}

@ -13,8 +13,16 @@ use OCP\WorkflowEngine\IManager;
use OCP\WorkflowEngine\IOperation;
class LogContext {
/** @var array */
protected $details;
/** @var array{
* message?: string,
* scopes?: array,
* operation?: array{class: class-string<IOperation>, name: string},
* entity?: array{class: class-string<IEntity>, name: string},
* configuration?: array,
* eventName?: string,
* }
*/
protected array $details;
public function setDescription(string $description): LogContext {
$this->details['message'] = $description;

@ -11,12 +11,9 @@ namespace OCA\WorkflowEngine\Helper;
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 @@ class ScopeContext {
return trim((string)$scopeId);
}
/**
* @return int
*/
public function getScope(): int {
return $this->scope;
}
/**
* @return string
*/
public function getScopeId(): string {
return $this->scopeId;
}

@ -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\Entity\File;
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\IEntityEvent;
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<ICheck>, operator: string, value: string, hash: string}
*/
class Manager implements IManager {
/** @var array[] */
protected $operations = [];
protected array $operations = [];
/** @var array[] */
protected $checks = [];
/** @var array<int, Check> */
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<int[]> */
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 @@ class Manager implements IManager {
$this->container,
$this->l,
$this,
$this->container->query(Logger::class)
$this->container->get(Logger::class)
);
}
@ -121,10 +123,11 @@ class Manager implements IManager {
}
/**
* @param string $operationClass
* @param class-string<IOperation> $operationClass
* @return ScopeContext[]
*/
public function getAllConfiguredScopesForOperation(string $operationClass): array {
/** @var array<class-string<IOperation>, ScopeContext[]> $scopesByOperation */
static $scopesByOperation = [];
if (isset($scopesByOperation[$operationClass])) {
return $scopesByOperation[$operationClass];
@ -132,8 +135,8 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
/**
* @param string $class
* @param string $name
* @param array[] $checks
* @param array<int, Check> $checks
* @param string $operation
* @return array The added operation
* @throws \UnexpectedValueException
@ -379,13 +382,11 @@ class Manager implements IManager {
}
/**
* @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 @@ class Manager implements IManager {
->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 @@ class Manager implements IManager {
return $result;
}
protected function validateEvents(string $entity, array $events, IOperation $operation) {
/**
* @param class-string<IEntity> $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 @@ class Manager implements IManager {
}
/**
* @param string $class
* @param string $name
* @param array[] $checks
* @param string $operation
* @param ScopeContext $scope
* @param string $entity
* @param class-string<IOperation> $class
* @param array<int, Check> $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 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
/**
* @param int[] $checkIds
* @return array[]
* @return array<int, Check>
*/
public function getChecks(array $checkIds) {
public function getChecks(array $checkIds): array {
$checkIds = array_map('intval', $checkIds);
$checks = [];
@ -541,6 +541,7 @@ class Manager implements IManager {
$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 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
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 @@ class Manager implements IManager {
return [
// None yet
];
} catch (QueryException $e) {
} catch (ContainerExceptionInterface $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return [];
}
@ -692,23 +690,23 @@ class Manager implements IManager {
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');
}
}

@ -10,6 +10,7 @@ namespace OCA\WorkflowEngine\Service;
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 @@ class Logger {
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 @@ class Logger {
$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 @@ class Logger {
$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 @@ class Logger {
$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 @@ class Logger {
$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 @@ class Logger {
$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 @@ class Logger {
$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 @@ class Logger {
$this->log($message, $context, $logContext);
}
public function logEventDone(LogContext $logContext) {
public function logEventDone(LogContext $logContext): void {
$message = 'Flow handling done for event {ev}';
$context = [

@ -11,10 +11,8 @@ namespace OCA\WorkflowEngine\Service;
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\IFileCheck;
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 @@ class RuleMatcher implements IRuleMatcher {
}
/**
* @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;
}

@ -14,7 +14,6 @@ use OCP\AppFramework\Http\TemplateResponse;
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 @@ use OCP\WorkflowEngine\ISpecificOperation;
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 @@ abstract class ASettings implements ISettings {
}
/**
* @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 @@ abstract class ASettings implements ISettings {
return 0;
}
private function entitiesToArray(array $entities) {
return array_map(function (IEntity $entity) {
$events = array_map(function (IEntityEvent $entityEvent) {
/**
* @param IEntity[] $entities
* @return array<array-key, array{id: class-string<IEntity>, icon: string, name: string, events: array<array-key, array{eventName: string, displayName: string}>}>
*/
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 @@ abstract class ASettings implements ISettings {
}, $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 @@ abstract class ASettings implements ISettings {
}, $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 [

@ -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');
}
}

@ -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\IEntityEvent;
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 @@ use Test\TestCase;
* @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 @@ class ManagerTest extends TestCase {
$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 @@ class ManagerTest extends TestCase {
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 @@ class ManagerTest extends TestCase {
]);
$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 @@ class ManagerTest extends TestCase {
]);
$this->container->expects($this->any())
->method('query')
->method('get')
->willReturnCallback(function ($className) use ($operation) {
switch ($className) {
case 'OCA\WFE\TestOp':
@ -382,7 +368,7 @@ class ManagerTest extends TestCase {
});
$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 @@ class ManagerTest extends TestCase {
$fileEntityMock = $this->createMock(File::class);
$this->container->expects($this->once())
->method('query')
->method('get')
->with(File::class)
->willReturn($fileEntityMock);
@ -510,11 +496,10 @@ class ManagerTest extends TestCase {
$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 @@ class ManagerTest extends TestCase {
->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 @@ class ManagerTest extends TestCase {
->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 @@ class ManagerTest extends TestCase {
->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 @@ class ManagerTest extends TestCase {
->method('validateCheck');
$this->container->expects($this->any())
->method('query')
->method('get')
->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) {
switch ($className) {
case IOperation::class:

@ -2618,43 +2618,6 @@
<code><![CDATA[getAppValue]]></code>
</DeprecatedMethod>
</file>
<file src="apps/workflowengine/lib/BackgroundJobs/Rotate.php">
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
</DeprecatedMethod>
</file>
<file src="apps/workflowengine/lib/Check/AbstractStringCheck.php">
<NullArgument>
<code><![CDATA[null]]></code>
</NullArgument>
</file>
<file src="apps/workflowengine/lib/Check/FileSize.php">
<FalsableReturnStatement>
<code><![CDATA[$this->size]]></code>
</FalsableReturnStatement>
<InvalidPropertyAssignmentValue>
<code><![CDATA[$size]]></code>
</InvalidPropertyAssignmentValue>
<InvalidReturnStatement>
<code><![CDATA[$this->size]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code><![CDATA[string]]></code>
</InvalidReturnType>
</file>
<file src="apps/workflowengine/lib/Check/RequestRemoteAddress.php">
<InvalidArgument>
<code><![CDATA[$decodedValue[1]]]></code>
<code><![CDATA[$decodedValue[1]]]></code>
<code><![CDATA[$decodedValue[1]]]></code>
<code><![CDATA[$decodedValue[1]]]></code>
</InvalidArgument>
</file>
<file src="apps/workflowengine/lib/Check/TFileCheck.php">
<InvalidArgument>
<code><![CDATA[['app' => Application::APP_ID, 'class' => get_class($subject)]]]></code>
</InvalidArgument>
</file>
<file src="apps/workflowengine/lib/Entity/File.php">
<DeprecatedConstant>
<code><![CDATA[MapperEvent::EVENT_ASSIGN]]></code>
@ -2665,58 +2628,6 @@
<code><![CDATA[getSubject]]></code>
<code><![CDATA[getSubject]]></code>
</DeprecatedMethod>
<InvalidReturnType>
<code><![CDATA[string]]></code>
</InvalidReturnType>
</file>
<file src="apps/workflowengine/lib/Manager.php">
<DeprecatedInterface>
<code><![CDATA[protected]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
<code><![CDATA[query]]></code>
</DeprecatedMethod>
<InvalidArgument>
<code><![CDATA[$missingCheck]]></code>
</InvalidArgument>
<InvalidOperand>
<code><![CDATA[$result]]></code>
</InvalidOperand>
<InvalidReturnStatement>
<code><![CDATA[$result]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code><![CDATA[bool]]></code>
</InvalidReturnType>
</file>
<file src="apps/workflowengine/lib/Service/Logger.php">
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
</DeprecatedMethod>
</file>
<file src="apps/workflowengine/lib/Service/RuleMatcher.php">
<DeprecatedInterface>
<code><![CDATA[protected]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[query]]></code>
</DeprecatedMethod>
</file>
<file src="apps/workflowengine/lib/Settings/ASettings.php">
<DeprecatedMethod>

@ -315,7 +315,7 @@ class Util {
}
/**
* 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

@ -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

Loading…
Cancel
Save