Fix #43 via factory (#45) (#48)

Co-authored-by: Robin Windey <[email protected]>

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

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

lib/OcrProcessors/OcrProcessorFactory.php

@@ -24,20 +24,35 @@
 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;
 
 	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);

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);
+	}
 }