From b6165aac046fba03403070d218156351f5f7c6f1 Mon Sep 17 00:00:00 2001 From: Robin Windey Date: Tue, 24 Dec 2024 10:24:10 +0000 Subject: [PATCH] Feature: Configurable success notification * Implements #217 --- README.md | 1 + composer.json | 3 +- composer.lock | 137 ++++++---------- lib/BackgroundJobs/ProcessFileJob.php | 37 +---- lib/Model/WorkflowSettings.php | 11 ++ lib/Notification/Notifier.php | 39 +++-- lib/OcrProcessors/OcrMyPdfBasedProcessor.php | 3 +- lib/Service/INotificationService.php | 8 + lib/Service/IOcrService.php | 12 ++ lib/Service/NotificationService.php | 16 +- lib/Service/OcrBackendInfoService.php | 11 +- lib/Service/OcrService.php | 92 ++++++++--- src/components/WorkflowOcr.vue | 7 + tests/Integration/Notification/AppFake.php | 4 + .../Notification/NotificationTest.php | 147 ++++++++++-------- .../BackgroundJobs/ProcessFileJobTest.php | 108 ++----------- .../RegisterFlowOperationsListenerTest.php | 12 +- tests/Unit/Notification/NotifierTest.php | 64 ++++++-- tests/Unit/Service/OcrServiceTest.php | 118 ++++++++++++++ tests/Unit/Settings/GlobalSettingsTest.php | 10 +- 20 files changed, 475 insertions(+), 365 deletions(-) diff --git a/README.md b/README.md index 53a42f0..43f1a4d 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ Remove tags after OCR | These tags will be removed from the file after it has be OCR mode | Controls the way files are processed, which already have OCR content. For PDF files this setting corresponds to the `--skip-text`, `--redo-ocr` and `--force-ocr` parameters of `ocrmypdf`. See [official docs](https://ocrmypdf.readthedocs.io/en/latest/advanced.html#when-ocr-is-skipped) for additional information.
**Skip text:** skip pages completely that already contain text. Such a page will not be touched and just be copied to the final output.
**Redo OCR:** perform a detailed text analysis to split up pages into areas with and without text.
**Force OCR:** all pages will be rasterized to images and OCR will be performed on every page. | Keep original file version | If the switch is set, the original file (before applying OCR) will be kept. This is done by giving the file version the label `Before OC`. This version will be excluded from the automatic expiration process (see [here](https://docs.nextcloud.com/server/latest/user_manual/en/files/version_control.html#naming-a-version) for details) | Keep original file modification date | Restore the modification date of the original file. The original modification date will be applied to the newly created file version. This is useful if you need to preserve the file modification date, for example to be able to sort files accordingly. | +Send success notification | Usually the workflow would only send a notification to the user if the OCR process failed. If this option is activated, the user will also be notified if a document has been processed successfully via OCR. | Remove background\* | If the switch is set, the OCR processor will try to remove the background of the document before processing and instead set a white background. For PDF files this setting corresponds to the [`--remove-background`](https://ocrmypdf.readthedocs.io/en/latest/cookbook.html?highlight=remove-background#image-processing) parameter of `ocrmypdf`.
:warning: Please note that this flag will currently only work with **`ocrmypdf` versions prior to 13**. It might be added in future versions again. See [here](https://github.com/ocrmypdf/OCRmyPDF/issues/884) for details. :warning:| Custom ocrMyPdf CLI arguments | If you want to pass custom arguments to the `ocrmypdf` CLI, you can do so here. Please note that the arguments will be passed as they are to the CLI, so make sure to use the correct syntax. Check the [official docs](https://ocrmypdf.readthedocs.io/en/latest/cookbook.html) for more information. | diff --git a/composer.json b/composer.json index b27c383..95f8a04 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,6 @@ { "require": { - "mikehaertl/php-shellcommand": "^1.6", - "cocur/chain": "^0.9.0" + "mikehaertl/php-shellcommand": "^1.6" }, "require-dev": { "phpunit/phpunit": "9.3.*", diff --git a/composer.lock b/composer.lock index 93e1848..2a7ced8 100644 --- a/composer.lock +++ b/composer.lock @@ -4,45 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "27ce47a5b254536ec76224f76e8380c3", + "content-hash": "03f4377de2906abf52eb3dfd1a383e7d", "packages": [ - { - "name": "cocur/chain", - "version": "v0.9.0", - "source": { - "type": "git", - "url": "https://github.com/cocur/chain.git", - "reference": "4d0a6e4fdee3b4aa53f0937f82e98d43320659a9" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/cocur/chain/zipball/4d0a6e4fdee3b4aa53f0937f82e98d43320659a9", - "reference": "4d0a6e4fdee3b4aa53f0937f82e98d43320659a9", - "shasum": "" - }, - "require": { - "php": ">=7.2" - }, - "require-dev": { - "phpunit/phpunit": "~8.0" - }, - "type": "library", - "autoload": { - "psr-4": { - "Cocur\\Chain\\": "src" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "description": "Consistent and chainable array manipulation", - "support": { - "issues": "https://github.com/cocur/chain/issues", - "source": "https://github.com/cocur/chain/tree/v0.9.0" - }, - "time": "2020-07-19T15:36:22+00:00" - }, { "name": "mikehaertl/php-shellcommand", "version": "1.7.0", @@ -278,13 +241,13 @@ }, "type": "library", "extra": { - "branch-alias": { - "dev-main": "3.x-dev" - }, "phpstan": { "includes": [ "extension.neon" ] + }, + "branch-alias": { + "dev-main": "3.x-dev" } }, "autoload": { @@ -516,29 +479,27 @@ }, { "name": "doctrine/deprecations", - "version": "1.1.3", + "version": "1.1.4", "source": { "type": "git", "url": "https://github.com/doctrine/deprecations.git", - "reference": "dfbaa3c2d2e9a9df1118213f3b8b0c597bb99fab" + "reference": "31610dbb31faa98e6b5447b62340826f54fbc4e9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/doctrine/deprecations/zipball/dfbaa3c2d2e9a9df1118213f3b8b0c597bb99fab", - "reference": "dfbaa3c2d2e9a9df1118213f3b8b0c597bb99fab", + "url": "https://api.github.com/repos/doctrine/deprecations/zipball/31610dbb31faa98e6b5447b62340826f54fbc4e9", + "reference": "31610dbb31faa98e6b5447b62340826f54fbc4e9", "shasum": "" }, "require": { "php": "^7.1 || ^8.0" }, "require-dev": { - "doctrine/coding-standard": "^9", - "phpstan/phpstan": "1.4.10 || 1.10.15", - "phpstan/phpstan-phpunit": "^1.0", + "doctrine/coding-standard": "^9 || ^12", + "phpstan/phpstan": "1.4.10 || 2.0.3", + "phpstan/phpstan-phpunit": "^1.0 || ^2", "phpunit/phpunit": "^7.5 || ^8.5 || ^9.5", - "psalm/plugin-phpunit": "0.18.4", - "psr/log": "^1 || ^2 || ^3", - "vimeo/psalm": "4.30.0 || 5.12.0" + "psr/log": "^1 || ^2 || ^3" }, "suggest": { "psr/log": "Allows logging deprecations via PSR-3 logger implementation" @@ -546,7 +507,7 @@ "type": "library", "autoload": { "psr-4": { - "Doctrine\\Deprecations\\": "lib/Doctrine/Deprecations" + "Doctrine\\Deprecations\\": "src" } }, "notification-url": "https://packagist.org/downloads/", @@ -557,9 +518,9 @@ "homepage": "https://www.doctrine-project.org/", "support": { "issues": "https://github.com/doctrine/deprecations/issues", - "source": "https://github.com/doctrine/deprecations/tree/1.1.3" + "source": "https://github.com/doctrine/deprecations/tree/1.1.4" }, - "time": "2024-01-30T19:34:25+00:00" + "time": "2024-12-07T21:18:45+00:00" }, { "name": "doctrine/instantiator", @@ -998,16 +959,16 @@ "source": { "type": "git", "url": "https://github.com/nextcloud-deps/ocp.git", - "reference": "e818fb786ae957f0ca16f3c0314a842300a30bc7" + "reference": "15a749fe867b97ae5e699a3ce9e1d50a792c1777" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/e818fb786ae957f0ca16f3c0314a842300a30bc7", - "reference": "e818fb786ae957f0ca16f3c0314a842300a30bc7", + "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/15a749fe867b97ae5e699a3ce9e1d50a792c1777", + "reference": "15a749fe867b97ae5e699a3ce9e1d50a792c1777", "shasum": "" }, "require": { - "php": "~8.0 || ~8.1 || ~8.2 || ~8.3", + "php": "~8.1 || ~8.2 || ~8.3", "psr/clock": "^1.0", "psr/container": "^2.0.2", "psr/event-dispatcher": "^1.0", @@ -1028,14 +989,18 @@ { "name": "Christoph Wurst", "email": "christoph@winzerhof-wurst.at" + }, + { + "name": "Joas Schilling", + "email": "coding@schilljs.com" } ], - "description": "Composer package containing Nextcloud's public API (classes, interfaces)", + "description": "Composer package containing Nextcloud's public OCP API and the unstable NCU API", "support": { "issues": "https://github.com/nextcloud-deps/ocp/issues", "source": "https://github.com/nextcloud-deps/ocp/tree/master" }, - "time": "2024-11-29T00:46:59+00:00" + "time": "2024-12-21T00:43:01+00:00" }, { "name": "nikic/php-parser", @@ -1318,16 +1283,16 @@ }, { "name": "phpdocumentor/reflection-docblock", - "version": "5.6.0", + "version": "5.6.1", "source": { "type": "git", "url": "https://github.com/phpDocumentor/ReflectionDocBlock.git", - "reference": "f3558a4c23426d12bffeaab463f8a8d8b681193c" + "reference": "e5e784149a09bd69d9a5e3b01c5cbd2e2bd653d8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpDocumentor/ReflectionDocBlock/zipball/f3558a4c23426d12bffeaab463f8a8d8b681193c", - "reference": "f3558a4c23426d12bffeaab463f8a8d8b681193c", + "url": "https://api.github.com/repos/phpDocumentor/ReflectionDocBlock/zipball/e5e784149a09bd69d9a5e3b01c5cbd2e2bd653d8", + "reference": "e5e784149a09bd69d9a5e3b01c5cbd2e2bd653d8", "shasum": "" }, "require": { @@ -1376,9 +1341,9 @@ "description": "With this component, a library can provide support for annotations via DocBlocks or otherwise retrieve information that is embedded in a DocBlock.", "support": { "issues": "https://github.com/phpDocumentor/ReflectionDocBlock/issues", - "source": "https://github.com/phpDocumentor/ReflectionDocBlock/tree/5.6.0" + "source": "https://github.com/phpDocumentor/ReflectionDocBlock/tree/5.6.1" }, - "time": "2024-11-12T11:25:25+00:00" + "time": "2024-12-07T09:39:29+00:00" }, { "name": "phpdocumentor/type-resolver", @@ -3204,16 +3169,16 @@ }, { "name": "spatie/array-to-xml", - "version": "3.3.0", + "version": "3.4.0", "source": { "type": "git", "url": "https://github.com/spatie/array-to-xml.git", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876" + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/f56b220fe2db1ade4c88098d83413ebdfc3bf876", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876", + "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", "shasum": "" }, "require": { @@ -3256,7 +3221,7 @@ "xml" ], "support": { - "source": "https://github.com/spatie/array-to-xml/tree/3.3.0" + "source": "https://github.com/spatie/array-to-xml/tree/3.4.0" }, "funding": [ { @@ -3268,20 +3233,20 @@ "type": "github" } ], - "time": "2024-05-01T10:20:27+00:00" + "time": "2024-12-16T12:45:15+00:00" }, { "name": "symfony/console", - "version": "v7.2.0", + "version": "v7.2.1", "source": { "type": "git", "url": "https://github.com/symfony/console.git", - "reference": "23c8aae6d764e2bae02d2a99f7532a7f6ed619cf" + "reference": "fefcc18c0f5d0efe3ab3152f15857298868dc2c3" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/console/zipball/23c8aae6d764e2bae02d2a99f7532a7f6ed619cf", - "reference": "23c8aae6d764e2bae02d2a99f7532a7f6ed619cf", + "url": "https://api.github.com/repos/symfony/console/zipball/fefcc18c0f5d0efe3ab3152f15857298868dc2c3", + "reference": "fefcc18c0f5d0efe3ab3152f15857298868dc2c3", "shasum": "" }, "require": { @@ -3345,7 +3310,7 @@ "terminal" ], "support": { - "source": "https://github.com/symfony/console/tree/v7.2.0" + "source": "https://github.com/symfony/console/tree/v7.2.1" }, "funding": [ { @@ -3361,7 +3326,7 @@ "type": "tidelift" } ], - "time": "2024-11-06T14:24:19+00:00" + "time": "2024-12-11T03:49:26+00:00" }, { "name": "symfony/deprecation-contracts", @@ -3522,8 +3487,8 @@ "type": "library", "extra": { "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" + "url": "https://github.com/symfony/polyfill", + "name": "symfony/polyfill" } }, "autoload": { @@ -3598,8 +3563,8 @@ "type": "library", "extra": { "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" + "url": "https://github.com/symfony/polyfill", + "name": "symfony/polyfill" } }, "autoload": { @@ -3676,8 +3641,8 @@ "type": "library", "extra": { "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" + "url": "https://github.com/symfony/polyfill", + "name": "symfony/polyfill" } }, "autoload": { @@ -3760,8 +3725,8 @@ "type": "library", "extra": { "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" + "url": "https://github.com/symfony/polyfill", + "name": "symfony/polyfill" } }, "autoload": { diff --git a/lib/BackgroundJobs/ProcessFileJob.php b/lib/BackgroundJobs/ProcessFileJob.php index b26568c..c506d7d 100644 --- a/lib/BackgroundJobs/ProcessFileJob.php +++ b/lib/BackgroundJobs/ProcessFileJob.php @@ -26,11 +26,8 @@ namespace OCA\WorkflowOcr\BackgroundJobs; -use OCA\WorkflowOcr\Model\WorkflowSettings; -use OCA\WorkflowOcr\Service\INotificationService; use OCA\WorkflowOcr\Service\IOcrService; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\Files\File; use Psr\Log\LoggerInterface; /** @@ -42,18 +39,14 @@ class ProcessFileJob extends \OCP\BackgroundJob\QueuedJob { protected $logger; /** @var IOcrService */ private $ocrService; - /** @var INotificationService */ - private $notificationService; public function __construct( LoggerInterface $logger, IOcrService $ocrService, - INotificationService $notificationService, ITimeFactory $timeFactory) { parent::__construct($timeFactory); $this->logger = $logger; $this->ocrService = $ocrService; - $this->notificationService = $notificationService; } /** @@ -61,35 +54,7 @@ public function __construct( */ protected function run($argument) : void { $this->logger->debug('STARTED -- Run ' . self::class . ' job. Argument: {argument}.', ['argument' => $argument]); - - try { - [$fileId, $uid, $settings] = $this->parseArguments($argument); - $this->ocrService->runOcrProcess($fileId, $uid, $settings); - } catch (\Throwable $ex) { - $this->logger->error($ex->getMessage(), ['exception' => $ex]); - $this->notificationService->createErrorNotification($uid, 'An error occured while executing the OCR process (' . $ex->getMessage() . '). Please have a look at your servers logfile for more details.'); - } - + $this->ocrService->runOcrProcessWithJobArgument($argument); $this->logger->debug('ENDED -- Run ' . self::class . ' job. Argument: {argument}.', ['argument' => $argument]); } - - /** - * @param mixed $argument - */ - private function parseArguments($argument) : array { - if (!is_array($argument)) { - throw new \InvalidArgumentException('Argument is no array in ' . self::class . ' method \'tryParseArguments\'.'); - } - - $jsonSettings = $argument['settings']; - $settings = new WorkflowSettings($jsonSettings); - $uid = $argument['uid']; - $fileId = intval($argument['fileId']); - - return [ - $fileId, - $uid, - $settings - ]; - } } diff --git a/lib/Model/WorkflowSettings.php b/lib/Model/WorkflowSettings.php index 49aee5a..0512213 100644 --- a/lib/Model/WorkflowSettings.php +++ b/lib/Model/WorkflowSettings.php @@ -55,6 +55,9 @@ class WorkflowSettings { /** @var bool */ private $keepOriginalFileDate = false; + /** @var bool */ + private $sendSuccessNotification = false; + /** @var string */ private $customCliArgs = ''; @@ -114,6 +117,13 @@ public function getKeepOriginalFileDate(): bool { return $this->keepOriginalFileDate; } + /** + * @return bool + */ + public function getSendSuccessNotification(): bool { + return $this->sendSuccessNotification; + } + /** * @return string */ @@ -154,6 +164,7 @@ private function setJson(?string $json = null) { $this->setProperty($this->tagsToAddAfterOcr, $data, 'tagsToAddAfterOcr', fn ($value) => is_array($value)); $this->setProperty($this->keepOriginalFileVersion, $data, 'keepOriginalFileVersion', fn ($value) => is_bool($value)); $this->setProperty($this->keepOriginalFileDate, $data, 'keepOriginalFileDate', fn ($value) => is_bool($value)); + $this->setProperty($this->sendSuccessNotification, $data, 'sendSuccessNotification', fn ($value) => is_bool($value)); $this->setProperty($this->customCliArgs, $data, 'customCliArgs', fn ($value) => is_string($value)); } diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index ad18d38..fdd927c 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -34,6 +34,7 @@ use OCP\Notification\AlreadyProcessedException; use OCP\Notification\INotification; use OCP\Notification\INotifier; +use OCP\Notification\UnknownNotificationException; use Psr\Log\LoggerInterface; class Notifier implements INotifier { @@ -78,20 +79,29 @@ public function getName(): string { */ public function prepare(INotification $notification, string $languageCode): INotification { if ($notification->getApp() !== Application::APP_NAME) { - throw new \InvalidArgumentException(); + throw new UnknownNotificationException(); } - // Currently we only support sending notifications for ocr_error + $l = $this->l10nFactory->get(Application::APP_NAME, $languageCode); + + // Currently we only support sending notifications for ocr_error and ocr_success $subject = $notification->getSubject(); - if ($subject !== 'ocr_error') { - $this->logger->warning('Unsupported notification subject {subject}', ['subject' => $subject]); - // Note:: AlreadyProcessedException has be be thrown before any call to $notification->set... - // otherwise notification won't be removed from the database - throw new AlreadyProcessedException(); + switch ($subject) { + case 'ocr_error': + $parsedSubject = $l->t('Workflow OCR error'); + $richSubject = $l->t('Workflow OCR error for file {file}'); + break; + case 'ocr_success': + $parsedSubject = $l->t('Workflow OCR success'); + $richSubject = $l->t('Workflow OCR success for file {file}'); + break; + default: + $this->logger->warning('Unsupported notification subject {subject}', ['subject' => $subject]); + // Note:: AlreadyProcessedException has to be thrown before any call to $notification->set... + // otherwise notification won't be removed from the database + throw new AlreadyProcessedException(); } - $l = $this->l10nFactory->get(Application::APP_NAME, $languageCode); - // Only add file info if we have some ... $richParams = false; if ($notification->getObjectType() === 'file' && @@ -99,16 +109,19 @@ public function prepare(INotification $notification, string $languageCode): INot ($uid = $notification->getUser())) { $richParams = $this->tryGetRichParamForFile($uid, intval($fileId)); if ($richParams !== false) { - $notification->setRichSubject($l->t('Workflow OCR error for file {file}'), $richParams); + $notification->setRichSubject($richSubject, $richParams); } } // Fallback to generic error message without file link if ($richParams === false) { - $notification->setParsedSubject($l->t('Workflow OCR error')); + $notification->setParsedSubject($parsedSubject); } - $message = $notification->getSubjectParameters()['message']; + // If caller sends a message, use it, otherwise use the parsed subject + $subjectParams = $notification->getSubjectParameters(); + $message = isset($subjectParams['message']) ? $subjectParams['message'] : $parsedSubject; + $notification ->setParsedMessage($message) ->setIcon($this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_NAME, 'app-dark.svg'))); @@ -136,7 +149,7 @@ private function tryGetRichParamForFile(string $uid, int $fileId) : array|bool { return [ 'file' => [ 'type' => 'file', - 'id' => $file->getId(), + 'id' => strval($file->getId()), 'name' => $file->getName(), 'path' => $relativePath, 'link' => $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $fileId]) diff --git a/lib/OcrProcessors/OcrMyPdfBasedProcessor.php b/lib/OcrProcessors/OcrMyPdfBasedProcessor.php index c4ba20f..0d1a736 100644 --- a/lib/OcrProcessors/OcrMyPdfBasedProcessor.php +++ b/lib/OcrProcessors/OcrMyPdfBasedProcessor.php @@ -23,7 +23,6 @@ namespace OCA\WorkflowOcr\OcrProcessors; -use Cocur\Chain\Chain; use OCA\WorkflowOcr\Exception\OcrNotPossibleException; use OCA\WorkflowOcr\Exception\OcrResultEmptyException; use OCA\WorkflowOcr\Helper\ISidecarFileAccessor; @@ -121,7 +120,7 @@ private function getCommandlineArgs(WorkflowSettings $settings, GlobalSettings $ // Language settings if ($settings->getLanguages()) { - $langStr = Chain::create($settings->getLanguages())->join('+'); + $langStr = implode('+', $settings->getLanguages()); $args[] = "-l $langStr"; } diff --git a/lib/Service/INotificationService.php b/lib/Service/INotificationService.php index a324df9..e3dbdcb 100644 --- a/lib/Service/INotificationService.php +++ b/lib/Service/INotificationService.php @@ -35,4 +35,12 @@ interface INotificationService { * @param int $fileId Optional file ID of the file that failed to OCR. If given, user can jump to the file via link. */ public function createErrorNotification(?string $userId, string $message, ?int $fileId = null); + + /** + * Create a new notification for the given user if the OCR process of the given file was successful. + * @param string $userId The user ID of the user that should receive the notification. + * @param string $message The success message that should be displayed in the notification. + * @param int $fileId Optional file ID of the file that was successfully OCR'd. If given, user can jump to the file via link. + */ + public function createSuccessNotification(?string $userId, ?int $fileId = null); } diff --git a/lib/Service/IOcrService.php b/lib/Service/IOcrService.php index 4e8bfe5..302b640 100644 --- a/lib/Service/IOcrService.php +++ b/lib/Service/IOcrService.php @@ -29,6 +29,18 @@ use OCA\WorkflowOcr\Model\WorkflowSettings; interface IOcrService { + /** + * Processes OCR on the given file. Creates a new file version and emits appropriate events. + * + * @param mixed $argument The argument from the \OCP\BackgroundJob\Job->run which executes this method + * + * @throws \OCA\WorkflowOcr\Exception\OcrNotPossibleException + * @throws \OCA\WorkflowOcr\Exception\OcrProcessorNotFoundException + * @throws \OCA\WorkflowOcr\Exception\OcrResultEmptyException + * @throws \InvalidArgumentException + */ + public function runOcrProcessWithJobArgument($argument) : void; + /** * Processes OCR on the given file. Creates a new file version and emits appropriate events. * diff --git a/lib/Service/NotificationService.php b/lib/Service/NotificationService.php index 46ce83c..a776e0a 100644 --- a/lib/Service/NotificationService.php +++ b/lib/Service/NotificationService.php @@ -45,16 +45,28 @@ public function __construct(IManager $notificationManager) { * @return void */ public function createErrorNotification(?string $userId, string $message, ?int $fileId = null) { + $this->createNotification($userId, 'ocr_error', $message, $fileId); + } + + /** + * @return void + */ + public function createSuccessNotification(?string $userId, ?int $fileId = null) { + $this->createNotification($userId, 'ocr_success', null, $fileId); + } + + private function createNotification(?string $userId, string $type, ?string $message = null, ?int $fileId = null) { // We don't create unbound notifications if (!$userId) { return; } - + + $parameters = $message ? ['message' => $message] : []; $notification = $this->notificationManager->createNotification(); $notification->setApp(Application::APP_NAME) ->setUser($userId) ->setDateTime(new \DateTime()) - ->setSubject('ocr_error', ['message' => $message]); + ->setSubject($type, $parameters); if ($fileId) { $notification->setObject('file', strval($fileId)); diff --git a/lib/Service/OcrBackendInfoService.php b/lib/Service/OcrBackendInfoService.php index 0ecfa82..a8ce8dc 100644 --- a/lib/Service/OcrBackendInfoService.php +++ b/lib/Service/OcrBackendInfoService.php @@ -26,7 +26,6 @@ namespace OCA\WorkflowOcr\Service; -use Cocur\Chain\Chain; use OCA\WorkflowOcr\Exception\CommandException; use OCA\WorkflowOcr\Wrapper\ICommand; use Psr\Log\LoggerInterface; @@ -70,12 +69,10 @@ public function getInstalledLanguages() : array { } $lines = explode("\n", $installedLangsStr); - $arr = Chain::create($lines) - ->slice(1) // Skip tesseract header line - ->filter(function ($line) { - return $line !== 'osd'; // Also skip "osd" (OSD is not a language) - }) - ->array; + $arr = array_filter( + array_slice($lines, 1), // Skip tesseract header line + fn ($line) => $line !== 'osd' // Also skip "osd" (OSD is not a language) + ); return array_values($arr); } } diff --git a/lib/Service/OcrService.php b/lib/Service/OcrService.php index 6fabcbf..92dcfc6 100644 --- a/lib/Service/OcrService.php +++ b/lib/Service/OcrService.php @@ -34,6 +34,7 @@ use OCA\WorkflowOcr\Helper\IProcessingFileAccessor; use OCA\WorkflowOcr\Model\WorkflowSettings; use OCA\WorkflowOcr\OcrProcessors\IOcrProcessorFactory; +use OCA\WorkflowOcr\OcrProcessors\OcrProcessorResult; use OCA\WorkflowOcr\Wrapper\IFilesystem; use OCA\WorkflowOcr\Wrapper\IViewFactory; use OCP\Files\File; @@ -85,6 +86,9 @@ class OcrService implements IOcrService { /** @var IProcessingFileAccessor */ private $processingFileAccessor; + /** @var INotificationService */ + private $notificationService; + /** @var LoggerInterface */ private $logger; @@ -100,6 +104,7 @@ public function __construct( IEventService $eventService, IViewFactory $viewFactory, IProcessingFileAccessor $processingFileAccessor, + INotificationService $notificationService, LoggerInterface $logger) { $this->ocrProcessorFactory = $ocrProcessorFactory; $this->globalSettingsService = $globalSettingsService; @@ -112,11 +117,23 @@ public function __construct( $this->eventService = $eventService; $this->viewFactory = $viewFactory; $this->processingFileAccessor = $processingFileAccessor; + $this->notificationService = $notificationService; $this->logger = $logger; } /** @inheritdoc */ - public function runOcrProcess(int $fileId, string $uid, WorkflowSettings $settings) : void { + public function runOcrProcessWithJobArgument($argument): void { + try { + [$fileId, $uid, $settings] = $this->parseArguments($argument); + $this->runOcrProcess($fileId, $uid, $settings); + } catch (\Throwable $ex) { + $this->logger->error($ex->getMessage(), ['exception' => $ex]); + $this->notificationService->createErrorNotification($uid ?? null, 'An error occured while executing the OCR process (' . $ex->getMessage() . '). Please have a look at your servers logfile for more details.'); + } + } + + /** @inheritdoc */ + public function runOcrProcess(int $fileId, string $uid, WorkflowSettings $settings) : void { // TODO :: make private try { $this->initUserEnvironment($uid); @@ -142,33 +159,32 @@ public function runOcrProcess(int $fileId, string $uid, WorkflowSettings $settin throw $ex; } - $this->processTagsAfterSuccessfulOcr($file, $settings); - - $filePath = $file->getPath(); - $fileContent = $result->getFileContent(); - $originalFileExtension = $file->getExtension(); - $newFileExtension = $result->getFileExtension(); - - // Only create a new file version if the file OCR result was not empty #130 - if ($result->getRecognizedText() !== '') { - if ($settings->getKeepOriginalFileVersion()) { - // Add label to original file to prevent its expiry - $this->setFileVersionsLabel($file, $uid, self::FILE_VERSION_LABEL_VALUE); - } - - $newFilePath = $originalFileExtension === $newFileExtension ? - $filePath : - $filePath . '.pdf'; - - $this->createNewFileVersion($newFilePath, $fileContent, $fileId, $fileMtime); - } - - $this->eventService->textRecognized($result, $file); + $this->doPostProcessing($file, $uid, $settings, $result, $fileMtime); } finally { $this->shutdownUserEnvironment(); } } + /** + * @param mixed $argument + */ + private function parseArguments($argument) : array { + if (!is_array($argument)) { + throw new \InvalidArgumentException('Argument is not an array in ' . self::class . ' method \'tryParseArguments\'.'); + } + + $jsonSettings = $argument['settings']; + $settings = new WorkflowSettings($jsonSettings); + $uid = $argument['uid']; + $fileId = intval($argument['fileId']); + + return [ + $fileId, + $uid, + $settings + ]; + } + /** * * @param string $uid The owners userId of the file to be processed */ @@ -287,4 +303,34 @@ private function setFileVersionsLabel(File $file, string $uid, string $label): v } } } + + private function doPostProcessing(Node $file, string $uid, WorkflowSettings $settings, OcrProcessorResult $result, ?int $fileMtime = null): void { + $this->processTagsAfterSuccessfulOcr($file, $settings); + + $filePath = $file->getPath(); + $fileId = $file->getId(); + $fileContent = $result->getFileContent(); + $originalFileExtension = $file->getExtension(); + $newFileExtension = $result->getFileExtension(); + + // Only create a new file version if the file OCR result was not empty #130 + if ($result->getRecognizedText() !== '') { + if ($settings->getKeepOriginalFileVersion()) { + // Add label to original file to prevent its expiry + $this->setFileVersionsLabel($file, $uid, self::FILE_VERSION_LABEL_VALUE); + } + + $newFilePath = $originalFileExtension === $newFileExtension ? + $filePath : + $filePath . '.pdf'; + + $this->createNewFileVersion($newFilePath, $fileContent, $fileId, $fileMtime); + } + + $this->eventService->textRecognized($result, $file); + + if ($settings->getSendSuccessNotification()) { + $this->notificationService->createSuccessNotification($uid, $fileId); + } + } } diff --git a/src/components/WorkflowOcr.vue b/src/components/WorkflowOcr.vue index 3a73f26..d467f13 100644 --- a/src/components/WorkflowOcr.vue +++ b/src/components/WorkflowOcr.vue @@ -99,6 +99,11 @@ type="switch"> {{ t('workflow_ocr', 'Keep original file modification date') }} + + {{ t('workflow_ocr', 'Send success notification') }} +
@@ -146,6 +151,7 @@ export default { * removeBackground: true, * keepOriginalFileVersion: true, * keepOriginalFileDate: true, + * sendSuccessNotification: true, * ocrMode: 0, * customCliArgs: '--rotate-pages-threshold 8', * } @@ -158,6 +164,7 @@ export default { removeBackground: false, keepOriginalFileVersion: false, keepOriginalFileDate: false, + sendSuccessNotification: false, ocrMode: 0, customCliArgs: '', }, diff --git a/tests/Integration/Notification/AppFake.php b/tests/Integration/Notification/AppFake.php index 0bf7338..04a2fe4 100644 --- a/tests/Integration/Notification/AppFake.php +++ b/tests/Integration/Notification/AppFake.php @@ -47,4 +47,8 @@ public function getNotifications() { public function getProcessed() { return $this->processed; } + + public function resetNotifications() { + $this->notifications = []; + } } diff --git a/tests/Integration/Notification/NotificationTest.php b/tests/Integration/Notification/NotificationTest.php index 4523c26..f4fbd81 100644 --- a/tests/Integration/Notification/NotificationTest.php +++ b/tests/Integration/Notification/NotificationTest.php @@ -21,102 +21,83 @@ namespace OCA\WorkflowOcr\Tests\Integration\Notification; -use OC\BackgroundJob\JobList; -use OCA\WorkflowOcr\BackgroundJobs\ProcessFileJob; +use OCA\Files_Versions\Versions\IVersionManager; use OCA\WorkflowOcr\Exception\OcrNotPossibleException; +use OCA\WorkflowOcr\Helper\IProcessingFileAccessor; +use OCA\WorkflowOcr\OcrProcessors\IOcrProcessorFactory; +use OCA\WorkflowOcr\Service\IEventService; +use OCA\WorkflowOcr\Service\IGlobalSettingsService; use OCA\WorkflowOcr\Service\INotificationService; -use OCA\WorkflowOcr\Service\IOcrService; use OCA\WorkflowOcr\Service\NotificationService; -use OCP\AppFramework\Utility\ITimeFactory; -use OCP\DB\QueryBuilder\IExpressionBuilder; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IConfig; +use OCA\WorkflowOcr\Service\OcrService; +use OCA\WorkflowOcr\Wrapper\IFilesystem; +use OCA\WorkflowOcr\Wrapper\IViewFactory; +use OCP\Files\IRootFolder; use OCP\IDBConnection; +use OCP\IUserManager; +use OCP\IUserSession; +use OCP\Notification\INotification; +use OCP\SystemTag\ISystemTagObjectMapper; use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +use Test\TestCase; +/** + * @group DB + */ class NotificationTest extends TestCase { /** @var LoggerInterface|MockObject */ private $logger; - /** @var IOcrService|MockObject */ - private $ocrService; - /** @var INotificationService|MockObject */ + /** @var INotificationService */ private $notificationService; - /** @var JobList */ - private $jobList; - /** @var ProcessFileJob */ - private $processFileJob; + /** @var AppFake */ + private static $appFake; - protected function setUp() : void { - parent::setUp(); + public static function setUpBeforeClass() : void { // We use a faked notification receiver app to keep track of any notifications created \OC::$server->get(\OCP\Notification\IManager::class)->registerApp(AppFake::class); + self::$appFake = \OC::$server->get(AppFake::class); + } + + protected function setUp() : void { + parent::setUp(); // Use real Notification service to be able to check if notifications get created $this->notificationService = new NotificationService(\OC::$server->get(\OCP\Notification\IManager::class)); - $this->logger = $this->createMock(LoggerInterface::class); - $this->ocrService = $this->createMock(IOcrService::class); - - $this->processFileJob = new ProcessFileJob( - $this->logger, - $this->ocrService, - $this->notificationService, - $this->createMock(ITimeFactory::class) - ); - - /** @var IConfig */ - $configMock = $this->createMock(IConfig::class); - /** @var ITimeFactory */ - $timeFactoryMock = $this->createMock(ITimeFactory::class); - /** @var MockObject|IDbConnection */ - $connectionMock = $this->createMock(IDBConnection::class); - /** @var MockObject|IQueryBuilder */ - $queryBuilderMock = $this->createMock(IQueryBuilder::class); - $expressionBuilderMock = $this->createMock(IExpressionBuilder::class); - $queryBuilderMock->method('delete') - ->withAnyParameters() - ->willReturn($queryBuilderMock); - $queryBuilderMock->method('set') - ->withAnyParameters() - ->willReturn($queryBuilderMock); - $queryBuilderMock->method('update') - ->withAnyParameters() - ->willReturn($queryBuilderMock); - $queryBuilderMock->method('expr') - ->withAnyParameters() - ->willReturn($expressionBuilderMock); - $connectionMock->method('getQueryBuilder') - ->withAnyParameters() - ->willReturn($queryBuilderMock); + $this->cleanupExistingNotifications(); + } - $this->jobList = new JobList( - $connectionMock, - $configMock, - $timeFactoryMock, + public function testOcrServiceCreatesErrorNotificationIfOcrFailed() { + // Simulate an early exception + $userManager = $this->createMock(IUserManager::class); + $userManager->method('get') + ->with('someuser') + ->willThrowException(new OcrNotPossibleException('Some error')); + $ocrService = new OcrService( + $this->createMock(IOcrProcessorFactory::class), + $this->createMock(IGlobalSettingsService::class), + $this->createMock(IVersionManager::class), + $this->createMock(ISystemTagObjectMapper::class), + $userManager, + $this->createMock(IFilesystem::class), + $this->createMock(IUserSession::class), + $this->createMock(IRootFolder::class), + $this->createMock(IEventService::class), + $this->createMock(IViewFactory::class), + $this->createMock(IProcessingFileAccessor::class), + $this->notificationService, $this->logger ); - $this->processFileJob->setId(111); - $this->processFileJob->setArgument([ + $ocrService->runOcrProcessWithJobArgument([ 'fileId' => 42, 'uid' => 'someuser', 'settings' => '{}' ]); - } - - public function testBackgroundJobCreatesErrorNotificationIfOcrFailed() { - $this->ocrService->expects($this->once()) - ->method('runOcrProcess') - ->withAnyParameters() - ->willThrowException(new OcrNotPossibleException('Some error')); - $appFake = \OC::$server->get(AppFake::class); - $this->processFileJob->start($this->jobList); - - $notifications = $appFake->getNotifications(); + $notifications = self::$appFake->getNotifications(); $this->assertCount(1, $notifications); $notification = $notifications[0]; @@ -124,4 +105,34 @@ public function testBackgroundJobCreatesErrorNotificationIfOcrFailed() { $this->assertEquals('ocr_error', $notification->getSubject()); $this->assertEquals('An error occured while executing the OCR process (Some error). Please have a look at your servers logfile for more details.', $notification->getSubjectParameters()['message']); } + + public function testCreateSuccessNotification() { + $randomFileId = rand(1, 1000); + $this->notificationService->createSuccessNotification('someuser', $randomFileId); + + $notifications = self::$appFake->getNotifications(); + $this->assertCount(1, $notifications); + + /** @var INotification */ + $notification = $notifications[0]; + $this->assertEquals('workflow_ocr', $notification->getApp()); + $this->assertEquals('ocr_success', $notification->getSubject()); + $this->assertEquals(strval($randomFileId), $notification->getObjectId()); + $this->assertEquals('someuser', $notification->getUser()); + $this->assertEquals('file', $notification->getObjectType()); + $this->assertEquals([], $notification->getSubjectParameters()); + } + + private function cleanupExistingNotifications() { + $dbConnection = \OC::$server->get(IDBConnection::class); + try { + $sql = $dbConnection->getQueryBuilder(); + $sql->delete('notifications') + ->where($sql->expr()->eq('app', $sql->createNamedParameter('workflow_ocr'))); + $sql->executeStatement(); + } finally { + $dbConnection->close(); + } + self::$appFake->resetNotifications(); + } } diff --git a/tests/Unit/BackgroundJobs/ProcessFileJobTest.php b/tests/Unit/BackgroundJobs/ProcessFileJobTest.php index 570a7b1..82361c9 100644 --- a/tests/Unit/BackgroundJobs/ProcessFileJobTest.php +++ b/tests/Unit/BackgroundJobs/ProcessFileJobTest.php @@ -23,18 +23,12 @@ namespace OCA\WorkflowOcr\Tests\Unit\BackgroundJobs; -use Exception; use OC\BackgroundJob\JobList; use OCA\WorkflowOcr\BackgroundJobs\ProcessFileJob; -use OCA\WorkflowOcr\Exception\OcrNotPossibleException; -use OCA\WorkflowOcr\Exception\OcrProcessorNotFoundException; -use OCA\WorkflowOcr\Exception\OcrResultEmptyException; -use OCA\WorkflowOcr\Service\INotificationService; use OCA\WorkflowOcr\Service\IOcrService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Files\NotFoundException; use OCP\IConfig; use OCP\IDBConnection; use PHPUnit\Framework\MockObject\MockObject; @@ -48,25 +42,27 @@ class ProcessFileJobTest extends TestCase { /** @var IOcrService|MockObject */ private $ocrService; - /** @var INotificationService|MockObject */ - private $notificationService; /** @var JobList */ private $jobList; + /** @var ProcessFileJob */ private $processFileJob; + private $argument = [ + 'fileId' => 42, + 'uid' => 'admin', + 'settings' => '{}' + ]; public function setUp() : void { parent::setUp(); $this->logger = $this->createMock(LoggerInterface::class); $this->ocrService = $this->createMock(IOcrService::class); - $this->notificationService = $this->createMock(INotificationService::class); $this->processFileJob = new ProcessFileJob( $this->logger, $this->ocrService, - $this->notificationService, $this->createMock(ITimeFactory::class) ); $this->processFileJob->setId(111); @@ -104,96 +100,14 @@ public function setUp() : void { $this->logger ); - $this->processFileJob->setArgument([ - 'fileId' => 42, - 'uid' => 'admin', - 'settings' => '{}' - ]); - } - - public function testCatchesException() { - $exception = new Exception('someEx'); - $this->ocrService->method('runOcrProcess') - ->willThrowException($exception); - - $this->logger->expects($this->once()) - ->method('error') - ->with($exception->getMessage(), ['exception' => $exception]); - - $this->processFileJob->start($this->jobList); - } - - /** - * @dataProvider dataProvider_InvalidArguments - */ - public function testLogsErrorAndDoesNothingOnInvalidArguments($argument, $errorMessagePart) { - $this->processFileJob->setArgument($argument); - $this->ocrService->expects($this->never()) - ->method('runOcrProcess') - ->withAnyParameters(); - $this->logger->expects($this->once()) - ->method('error') - ->with($this->stringContains($errorMessagePart), $this->callback(function ($loggerArgs) { - return is_array($loggerArgs) && ($loggerArgs['exception'] instanceof \Exception); - })); - - $this->processFileJob->start($this->jobList); - } - - public function testNotFoundLogsErrorAndSendsNotification() { - $this->ocrService->method('runOcrProcess') - ->willThrowException(new NotFoundException('File was not found')); - $this->logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('File was not found'), $this->callback(function ($subject) { - return is_array($subject) && ($subject['exception'] instanceof NotFoundException); - })); - $this->notificationService->expects($this->once()) - ->method('createErrorNotification') - ->with($this->stringContains('An error occured while executing the OCR process (') && $this->stringContains('File was not found')); - - $this->processFileJob->start($this->jobList); + $this->processFileJob->setArgument($this->argument); } - /** - * @dataProvider dataProvider_OcrExceptions - */ - public function testLogsError_OnOcrException(Exception $exception) { - $this->ocrService->method('runOcrProcess') - ->willThrowException($exception); - - $this->logger->expects($this->once()) - ->method('error'); - - $this->processFileJob->start($this->jobList); - } - - public function testLogsNonOcrExceptionsFromOcrService() { - $exception = new \Exception('someException'); - + public function testCallsOcrService() { $this->ocrService->expects($this->once()) - ->method('runOcrProcess') - ->willThrowException($exception); - - $this->logger->expects($this->once()) - ->method('error'); - + ->method('runOcrProcessWithJobArgument') + ->with($this->equalTo($this->argument)); + $this->processFileJob->start($this->jobList); } - - public function dataProvider_InvalidArguments() { - $arr = [ - [null, 'Argument is no array'], - [['mykey' => 'myvalue'], 'Undefined array key'] - ]; - return $arr; - } - - public function dataProvider_OcrExceptions() { - return [ - [new OcrNotPossibleException('Ocr not possible')], - [new OcrProcessorNotFoundException('audio/mpeg')], - [new OcrResultEmptyException('Ocr result was empty')] - ]; - } } diff --git a/tests/Unit/Listener/RegisterFlowOperationsListenerTest.php b/tests/Unit/Listener/RegisterFlowOperationsListenerTest.php index b833d63..e47154f 100644 --- a/tests/Unit/Listener/RegisterFlowOperationsListenerTest.php +++ b/tests/Unit/Listener/RegisterFlowOperationsListenerTest.php @@ -23,7 +23,6 @@ namespace OCA\WorkflowOcr\Tests\Unit\Listener; -use Cocur\Chain\Chain; use OCA\WorkflowOcr\AppInfo\Application; use OCA\WorkflowOcr\Listener\RegisterFlowOperationsListener; use OCA\WorkflowOcr\Operation; @@ -82,11 +81,12 @@ public function testRegistersOperationClassAndScripts_OnRegisterOperationsEvent( $scripts = Util::getScripts(); $appName = Application::APP_NAME; - $scriptCount = Chain::create($scripts) - ->filter(function ($script) use ($appName) { - return strpos($script, "$appName/l10n/") !== false || strpos($script, "$appName/js/workflow_ocr-main") !== false; - }) - ->count(); + $scriptCount = 0; + foreach ($scripts as $script) { + if (strpos($script, "$appName/l10n/") !== false || strpos($script, "$appName/js/workflow_ocr-main") !== false) { + $scriptCount++; + } + } $this->assertEquals(2, $scriptCount); } diff --git a/tests/Unit/Notification/NotifierTest.php b/tests/Unit/Notification/NotifierTest.php index 3cc3b98..3bb3100 100644 --- a/tests/Unit/Notification/NotifierTest.php +++ b/tests/Unit/Notification/NotifierTest.php @@ -58,6 +58,16 @@ class NotifierTest extends TestCase { /** @var Notifier */ private $notifier; + private static $returnValueMapError = [ + ['Workflow OCR error', [], ' Workflow OCR error'], + ['Workflow OCR error for file {file}', [], ' Workflow OCR error for file {file}'] + ]; + + private static $returnValueMapSuccess = [ + ['Workflow OCR success', [], ' Workflow OCR success'], + ['Workflow OCR success for file {file}', [], ' Workflow OCR success for file file.txt'] + ]; + public function setUp() : void { $this->l10nFactory = $this->createMock(IFactory::class); $this->rootFolder = $this->createMock(IRootFolder::class); @@ -120,10 +130,7 @@ public function testPrepareConstructsOcrErrorCorrectlyWithFileId() { $l10n = $this->createMock(IL10N::class); /** @var IRichTextFormatter|MockObject */ $rtFormatter = $this->createMock(IRichTextFormatter::class); - $l10n->expects($this->once()) - ->method('t') - ->with('Workflow OCR error for file {file}') - ->willReturn(' Workflow OCR error for file {file}'); + $l10n->method('t')->will($this->returnValueMap(self::$returnValueMapError)); $this->l10nFactory->expects($this->once()) ->method('get') ->with('workflow_ocr') @@ -186,6 +193,7 @@ public function testPrepareConstructsOcrErrorCorrectlyWithFileId() { 'path' => 'files/file.txt', 'link' => 'http://localhost/index.php/apps/files/?file=123' ]], $richSubjectParams); + $this->assertIsString($richSubjectParams['file']['id']); } public function testPrepareConstructsOcrErrorCorrectlyWithoutFile() { @@ -195,10 +203,7 @@ public function testPrepareConstructsOcrErrorCorrectlyWithoutFile() { $rtFormatter = $this->createMock(IRichTextFormatter::class); /** @var IL10N|MockObject */ $l10n = $this->createMock(IL10N::class); - $l10n->expects($this->once()) - ->method('t') - ->with('Workflow OCR error') - ->willReturn(' Workflow OCR error'); + $l10n->method('t')->will($this->returnValueMap(self::$returnValueMapError)); $this->l10nFactory->expects($this->once()) ->method('get') ->with('workflow_ocr') @@ -235,10 +240,7 @@ public function testSendsFallbackNotificationWithoutFileInfoIfFileNotFoundWasThr $rtFormatter = $this->createMock(IRichTextFormatter::class); /** @var IL10N|MockObject */ $l10n = $this->createMock(IL10N::class); - $l10n->expects($this->once()) - ->method('t') - ->with('Workflow OCR error') - ->willReturn(' Workflow OCR error'); + $l10n->method('t')->will($this->returnValueMap(self::$returnValueMapError)); $this->l10nFactory->expects($this->once()) ->method('get') ->with('workflow_ocr') @@ -288,10 +290,7 @@ public function testSendsFallbackNotificationWithoutFileInfoIfReturnedFileArrayW $rtFormatter = $this->createMock(IRichTextFormatter::class); /** @var IL10N|MockObject */ $l10n = $this->createMock(IL10N::class); - $l10n->expects($this->once()) - ->method('t') - ->with('Workflow OCR error') - ->willReturn(' Workflow OCR error'); + $l10n->method('t')->will($this->returnValueMap(self::$returnValueMapError)); $this->l10nFactory->expects($this->once()) ->method('get') ->with('workflow_ocr') @@ -332,4 +331,37 @@ public function testSendsFallbackNotificationWithoutFileInfoIfReturnedFileArrayW $this->assertEmpty($notification->getRichSubject()); $this->assertEquals(' Workflow OCR error', $notification->getParsedSubject()); } + + public function testFallbackToParsedSubjectIfMessageIsEmpty() { + /** @var IValidator|MockObject */ + $validator = $this->createMock(IValidator::class); + /** @var IRichTextFormatter|MockObject */ + $rtFormatter = $this->createMock(IRichTextFormatter::class); + /** @var IL10N|MockObject */ + $l10n = $this->createMock(IL10N::class); + $l10n->method('t')->will($this->returnValueMap(self::$returnValueMapSuccess)); + $this->l10nFactory->expects($this->once()) + ->method('get') + ->with('workflow_ocr') + ->willReturn($l10n); + + $notification = new Notification($validator, $rtFormatter); + $notification->setUser('user'); + $notification->setApp('workflow_ocr'); + $notification->setSubject('ocr_success', []); + + $this->urlGenerator->expects($this->once()) + ->method('imagePath') + ->with('workflow_ocr', 'app-dark.svg') + ->willReturn('apps/workflow_ocr/app-dark.svg'); + $this->urlGenerator->expects($this->once()) + ->method('getAbsoluteURL') + ->with('apps/workflow_ocr/app-dark.svg') + ->willReturn('http://localhost/index.php/apps/workflow_ocr/app-dark.svg'); + + $preparedNotification = $this->notifier->prepare($notification, 'en'); + + $this->assertEmpty($preparedNotification->getRichSubject()); + $this->assertEquals(' Workflow OCR success', $preparedNotification->getParsedSubject()); + } } diff --git a/tests/Unit/Service/OcrServiceTest.php b/tests/Unit/Service/OcrServiceTest.php index b0476c5..01f04f9 100644 --- a/tests/Unit/Service/OcrServiceTest.php +++ b/tests/Unit/Service/OcrServiceTest.php @@ -27,6 +27,8 @@ use InvalidArgumentException; use OC\User\NoUserException; use OCA\Files_Versions\Versions\IVersionManager; +use OCA\WorkflowOcr\Exception\OcrNotPossibleException; +use OCA\WorkflowOcr\Exception\OcrProcessorNotFoundException; use OCA\WorkflowOcr\Exception\OcrResultEmptyException; use OCA\WorkflowOcr\Helper\IProcessingFileAccessor; use OCA\WorkflowOcr\Model\GlobalSettings; @@ -36,6 +38,7 @@ use OCA\WorkflowOcr\OcrProcessors\OcrProcessorResult; use OCA\WorkflowOcr\Service\IEventService; use OCA\WorkflowOcr\Service\IGlobalSettingsService; +use OCA\WorkflowOcr\Service\INotificationService; use OCA\WorkflowOcr\Service\OcrService; use OCA\WorkflowOcr\Wrapper\IFilesystem; use OCA\WorkflowOcr\Wrapper\IView; @@ -82,6 +85,8 @@ class OcrServiceTest extends TestCase { private $user; /** @var IProcessingFileAccessor|MockObject */ private $processingFileAccessor; + /** @var INotificationService|MockObject */ + private $notificationService; /** @var File[] */ private $rootFolderGetById42ReturnValue; /** @var OcrService */ @@ -89,6 +94,12 @@ class OcrServiceTest extends TestCase { /** @var File|MockObject */ private $fileInput; + private $defaultArgument = [ + 'fileId' => 42, + 'uid' => 'admin', + 'settings' => '{}' + ]; + public function setUp() : void { parent::setUp(); @@ -103,6 +114,7 @@ public function setUp() : void { $this->filesystem = $this->createMock(IFilesystem::class); $this->userSession = $this->createMock(IUserSession::class); $this->processingFileAccessor = $this->createMock(IProcessingFileAccessor::class); + $this->notificationService = $this->createMock(INotificationService::class); /** @var MockObject|IRootFolder */ $this->rootFolder = $this->createMock(IRootFolder::class); @@ -142,6 +154,7 @@ public function setUp() : void { $this->eventService, $this->viewFactory, $this->processingFileAccessor, + $this->notificationService, $this->logger); } @@ -209,6 +222,37 @@ public function testCallsSystemTagObjectManager_WithCorrectArguments() { $this->ocrService->runOcrProcess(42, 'usr', $settings); } + public function testSendsSuccessNotificationIfConfigured() { + $mime = 'application/pdf'; + $content = 'someFileContent'; + $settings = new WorkflowSettings('{"sendSuccessNotification": true}'); + $globalSettings = new GlobalSettings(); + + $this->fileInput->method('getMimeType') + ->willReturn($mime); + $this->fileInput->method('getContent') + ->willReturn($content); + + $this->globalSettingsService->expects($this->once()) + ->method('getGlobalSettings') + ->willReturn($globalSettings); + + $this->ocrProcessorFactory->expects($this->once()) + ->method('create') + ->with($mime) + ->willReturn($this->ocrProcessor); + + $this->ocrProcessor->expects($this->once()) + ->method('ocrFile') + ->with($this->fileInput, $settings, $globalSettings); + + $this->notificationService->expects($this->once()) + ->method('createSuccessNotification') + ->with('usr', 42); + + $this->ocrService->runOcrProcess(42, 'usr', $settings); + } + public function testCatchesTagNotFoundException() { $mime = 'application/pdf'; $content = 'someFileContent'; @@ -387,6 +431,7 @@ public function testThrowsNoUserException_OnNonExistingUser() { $this->eventService, $this->viewFactory, $this->processingFileAccessor, + $this->notificationService, $this->logger); $thrown = false; @@ -547,6 +592,62 @@ public function testRestoreOriginalFileModificationDate() { $this->ocrService->runOcrProcess(42, 'usr', $settings); } + + public function testRunOcrProcessWithJobArgumentCatchedException() { + $exception = new Exception('someEx'); + $this->userManager->method('get') + ->willThrowException($exception); + + $this->logger->expects($this->once()) + ->method('error') + ->with($exception->getMessage(), ['exception' => $exception]); + + $this->ocrService->runOcrProcessWithJobArgument($this->defaultArgument); + } + + /** + * @dataProvider dataProvider_InvalidArguments + */ + public function testRunOcrProcessWithJobArgumentLogsErrorAndDoesNothingOnInvalidArguments($argument, $errorMessagePart) { + $this->userManager->expects($this->never()) + ->method('get') + ->withAnyParameters(); + $this->logger->expects($this->once()) + ->method('error') + ->with($this->stringContains($errorMessagePart), $this->callback(function ($loggerArgs) { + return is_array($loggerArgs) && ($loggerArgs['exception'] instanceof \Exception); + })); + + $this->ocrService->runOcrProcessWithJobArgument($argument); + } + + public function testRunOcrProcessWithJobArgumentLogsErrorAndSendsNotificationOnNotFound() { + $this->rootFolder->method('getById') + ->willThrowException(new NotFoundException('File was not found')); + $this->logger->expects($this->once()) + ->method('error') + ->with($this->stringContains('File was not found'), $this->callback(function ($subject) { + return is_array($subject) && ($subject['exception'] instanceof NotFoundException); + })); + $this->notificationService->expects($this->once()) + ->method('createErrorNotification') + ->with($this->stringContains('An error occured while executing the OCR process (') && $this->stringContains('File was not found')); + + $this->ocrService->runOcrProcessWithJobArgument($this->defaultArgument); + } + + /** + * @dataProvider dataProvider_ExceptionsToBeCaught + */ + public function testRunOcrProcessWithJobArgumentLogsErrorOnException(Exception $exception) { + $this->ocrProcessor->method('ocrFile') + ->willThrowException($exception); + + $this->logger->expects($this->once()) + ->method('error'); + + $this->ocrService->runOcrProcessWithJobArgument($this->defaultArgument); + } public function dataProvider_InvalidNodes() { /** @var MockObject|Node */ @@ -578,6 +679,23 @@ public function dataProvider_OriginalAndNewFilesnames() { ]; } + public function dataProvider_InvalidArguments() { + $arr = [ + [null, 'Argument is not an array'], + [['mykey' => 'myvalue'], 'Undefined array key'] + ]; + return $arr; + } + + public function dataProvider_ExceptionsToBeCaught() { + return [ + [new OcrNotPossibleException('Ocr not possible')], + [new OcrProcessorNotFoundException('audio/mpeg')], + [new OcrResultEmptyException('Ocr result was empty')], + [new Exception('Some exception')] + ]; + } + /** * @return File|MockObject */ diff --git a/tests/Unit/Settings/GlobalSettingsTest.php b/tests/Unit/Settings/GlobalSettingsTest.php index 0c09554..827ab43 100644 --- a/tests/Unit/Settings/GlobalSettingsTest.php +++ b/tests/Unit/Settings/GlobalSettingsTest.php @@ -23,7 +23,6 @@ namespace OCA\WorkflowOcr\Tests\Unit\Settings; -use Cocur\Chain\Chain; use OCA\WorkflowOcr\Settings\GlobalSettings; use Test\TestCase; @@ -46,12 +45,9 @@ public function testGetPriority() { public function testGetForm() { $templateResponse = $this->adminSettings->getForm(); - $templates = Chain::create(scandir('./templates')) - ->filter(function ($file) { - return is_file("./templates/$file"); - }); - $this->assertEquals(1, count($templates->array)); - $templateFileName = './templates/' . $templates->first(); + $templates = array_filter(scandir('./templates'), fn ($file) => is_file("./templates/$file")); + $this->assertEquals(1, count($templates)); + $templateFileName = './templates/' . $templates[array_keys($templates)[0]]; $templateNameWithoutExtension = pathinfo($templateFileName)['filename']; $this->assertEquals($templateNameWithoutExtension, $templateResponse->getTemplateName()); }