From 77b8125f227bcc4c262795e94c0a80156a28f271 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 1 Feb 2021 20:18:39 +0100 Subject: [PATCH] Fix #43 via factory (#45) (#48) Co-authored-by: Robin Windey --- README.md | 13 ++++++++- lib/AppInfo/Application.php | 8 +++++- lib/OcrProcessors/OcrProcessorFactory.php | 17 ++++++++++- .../OcrProcessors/OcrProcessorFactoryTest.php | 28 +++++++++++++++++-- 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3cea73f..8b49aa5 100644 --- a/README.md +++ b/README.md @@ -186,9 +186,20 @@ To support a new mimetype for being processed with OCR you have to follow a few ```php private static $mapping = [ 'application/pdf' => PdfOcrProcessor::class, - // Add your class here + // Add your class here, for example: + 'mymimetype' => MyOcrProcessor::class ]; ``` +3. Register a factory for creating your newly added processor in `lib/OcrProcessors/OcrProcessorFactory.php` by adding an appropriate function inside of `registerOcrProcessors`. +```php +public static function registerOcrProcessors(IRegistrationContext $context) : void { + // [...] + $context->registerService(MyOcrProcessor::class, function(ContainerInterface $c) { + return new /* your factory goes here */ + }, false); + } +``` + That's all. If you now create a new workflow based on your added mimetype, your implementation should be triggered by the app. The return value of `ocrFile(string $fileContent)` will be interpreted as the file content of the scanned file. This one is used to create a new file version in Nextcloud. ## Limitations diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 8f28564..d4cadf1 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -65,12 +65,18 @@ public function register(IRegistrationContext $context): void { $context->registerServiceAlias(IOcrProcessorFactory::class, OcrProcessorFactory::class); $context->registerServiceAlias(IViewFactory::class, ViewFactory::class); $context->registerServiceAlias(IFilesystem::class, Filesystem::class); - $context->registerServiceAlias(ICommand::class, CommandWrapper::class); + + // BUG #43 + $context->registerService(ICommand::class, function () { + return new CommandWrapper(); + }, false); $context->registerService(IProcessingFileAccessor::class, function () { return ProcessingFileAccessor::getInstance(); }); + OcrProcessorFactory::registerOcrProcessors($context); + $context->registerEventListener(RegisterOperationsEvent::class, RegisterFlowOperationsListener::class); } diff --git a/lib/OcrProcessors/OcrProcessorFactory.php b/lib/OcrProcessors/OcrProcessorFactory.php index 382b0c8..0b65a84 100644 --- a/lib/OcrProcessors/OcrProcessorFactory.php +++ b/lib/OcrProcessors/OcrProcessorFactory.php @@ -24,13 +24,15 @@ namespace OCA\WorkflowOcr\OcrProcessors; use OCA\WorkflowOcr\Exception\OcrProcessorNotFoundException; +use OCA\WorkflowOcr\Wrapper\ICommand; +use OCP\AppFramework\Bootstrap\IRegistrationContext; use Psr\Container\ContainerInterface; class OcrProcessorFactory implements IOcrProcessorFactory { private static $mapping = [ 'application/pdf' => PdfOcrProcessor::class ]; - + /** @var ContainerInterface */ private $container; @@ -38,6 +40,19 @@ public function __construct(ContainerInterface $container) { $this->container = $container; } + public static function registerOcrProcessors(IRegistrationContext $context) : void { + /* + * BUG #43: registerServiceAlias uses shared = false so every call to + * get() on the container interface will return a new instance. If we + * don't register this explicitly the instance will be cached as a kind of + * "singleton per request" which leads to problems regarding the reused Command object + * under the hood. + */ + $context->registerService(PdfOcrProcessor::class, function (ContainerInterface $c) { + return new PdfOcrProcessor($c->get(ICommand::class)); + }, false); + } + public function create(string $mimeType) : IOcrProcessor { if (!array_key_exists($mimeType, self::$mapping)) { throw new OcrProcessorNotFoundException($mimeType); diff --git a/tests/Unit/OcrProcessors/OcrProcessorFactoryTest.php b/tests/Unit/OcrProcessors/OcrProcessorFactoryTest.php index 05f6eed..5649a04 100644 --- a/tests/Unit/OcrProcessors/OcrProcessorFactoryTest.php +++ b/tests/Unit/OcrProcessors/OcrProcessorFactoryTest.php @@ -27,8 +27,8 @@ use OCA\WorkflowOcr\Exception\OcrProcessorNotFoundException; use OCA\WorkflowOcr\OcrProcessors\OcrProcessorFactory; use OCA\WorkflowOcr\OcrProcessors\PdfOcrProcessor; -use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; +use Test\TestCase; class OcrProcessorFactoryTest extends TestCase { /** @var ContainerInterface */ @@ -39,7 +39,7 @@ protected function setUp() : void { $app = new Application(); $this->appContainer = $app->getContainer(); } - + public function testReturnsPdfProcessor() { $factory = new OcrProcessorFactory($this->appContainer); $processor = $factory->create('application/pdf'); @@ -51,4 +51,28 @@ public function testThrowsNotFoundExceptionOnInvalidMimeType() { $factory = new OcrProcessorFactory($this->appContainer); $factory->create('no/mimetype'); } + + // Related to BUG #43 + public function testOcrProcessorsAreNotCached() { + /** @var array mimetype -> ocr processor classname */ + $mapping = $this->invokePrivate(OcrProcessorFactory::class, 'mapping'); + $factory = new OcrProcessorFactory($this->appContainer); + + foreach ($mapping as $mimetype => $className) { + $processor1 = $factory->create($mimetype); + $processor2 = $factory->create($mimetype); + $this->assertFalse($processor1 === $processor2); + } + } + + // Related to #43 + public function testPdfCommandNotCached() { + $factory = new OcrProcessorFactory($this->appContainer); + $processor1 = $factory->create('application/pdf'); + $processor2 = $factory->create('application/pdf'); + $cmd1 = $this->invokePrivate($processor1, 'command'); + $cmd2 = $this->invokePrivate($processor2, 'command'); + + $this->assertFalse($cmd1 === $cmd2); + } }