enh(TextToImage): Address review comments

Signed-off-by: Marcel Klehr <mklehr@gmx.net>

core/Controller/TextToImageApiController.php

@@ -58,12 +58,11 @@ class TextToImageApiController extends \OCP\AppFramework\OCSController {
 	}
 
 	/**
-	 * @PublicPage
-	 *
-	 * Check whether this feature is available
+	 * 	 * Check whether this feature is available
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array{isAvailable: bool}, array{}>
 	 */
+	#[PublicPage]
 	public function isAvailable(): DataResponse {
 		return new DataResponse([
 			'isAvailable' => $this->textToImageManager->hasProviders(),

core/Migrations/Version28000Date20230906104802.php

@@ -45,7 +45,6 @@ class Version28000Date20230906104802 extends SimpleMigrationStep {
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
 		/** @var ISchemaWrapper $schema */
 		$schema = $schemaClosure();
-		$changed = false;
 		if (!$schema->hasTable('text2image_tasks')) {
 			$table = $schema->createTable('text2image_tasks');
 
@@ -76,11 +75,8 @@ class Version28000Date20230906104802 extends SimpleMigrationStep {
 				'length' => 255,
 				'default' => '',
 			]);
-			$table->addColumn('last_updated', Types::INTEGER, [
+			$table->addColumn('last_updated', Types::DATETIME, [
 				'notnull' => false,
-				'length' => 4,
-				'default' => 0,
-				'unsigned' => true,
 			]);
 
 			$table->setPrimaryKey(['id'], 't2i_tasks_id_index');
@@ -88,10 +84,6 @@ class Version28000Date20230906104802 extends SimpleMigrationStep {
 			$table->addIndex(['status'], 't2i_tasks_status');
 			$table->addIndex(['user_id', 'app_id', 'identifier'], 't2i_tasks_uid_appid_ident');
 
-			$changed = true;
-		}
-
-		if ($changed) {
 			return $schema;
 		}
 

lib/private/TextToImage/Manager.php

@@ -29,6 +29,8 @@ use OC\AppFramework\Bootstrap\Coordinator;
 use OC\TextToImage\Db\Task as DbTask;
 use OCP\Files\AppData\IAppDataFactory;
 use OCP\Files\IAppData;
+use OCP\Files\NotFoundException;
+use OCP\Files\NotPermittedException;
 use OCP\IConfig;
 use OCP\TextToImage\Exception\TaskNotFoundException;
 use OCP\TextToImage\IManager;
@@ -100,6 +102,7 @@ class Manager implements IManager {
 	 * @inheritDoc
 	 */
 	public function runTask(Task $task): void {
+		$this->logger->debug('Running TextToImage Task');
 		if (!$this->hasProviders()) {
 			throw new PreConditionNotMetException('No text to image provider is installed that can handle this task');
 		}
@@ -107,42 +110,75 @@ class Manager implements IManager {
 
 		$json = $this->config->getAppValue('core', 'ai.text2image_provider', '');
 		if ($json !== '') {
-			$className = json_decode($json, true);
-			$provider = current(array_filter($providers, fn ($provider) => $provider::class === $className));
-			if ($provider !== false) {
-				$providers = [$provider];
+			try {
+				$className = json_decode($json, true, 512, JSON_THROW_ON_ERROR);
+				$provider = current(array_filter($providers, fn ($provider) => $provider::class === $className));
+				if ($provider !== false) {
+					$providers = [$provider];
+				}
+			} catch (\JsonException $e) {
+				$this->logger->warning('Failed to decode Text2Image setting `ai.text2image_provider`', ['exception' => $e]);
 			}
 		}
 
 		foreach ($providers as $provider) {
+			$this->logger->debug('Trying to run Text2Image provider '.$provider::class);
 			try {
 				$task->setStatus(Task::STATUS_RUNNING);
 				if ($task->getId() === null) {
+					$this->logger->debug('Inserting Text2Image task into DB');
 					$taskEntity = $this->taskMapper->insert(DbTask::fromPublicTask($task));
 					$task->setId($taskEntity->getId());
 				} else {
+					$this->logger->debug('Updating Text2Image task in DB');
 					$this->taskMapper->update(DbTask::fromPublicTask($task));
 				}
 				try {
 					$folder = $this->appData->getFolder('text2image');
-				} catch(\OCP\Files\NotFoundException $e) {
+				} catch(NotFoundException) {
+					$this->logger->debug('Creating folder in appdata for Text2Image results');
 					$folder = $this->appData->newFolder('text2image');
 				}
+				$this->logger->debug('Creating result file for Text2Image task');
 				$file = $folder->newFile((string) $task->getId());
-				$provider->generate($task->getInput(), $file->write());
+				$resource = $file->write();
+				if ($resource === false) {
+					throw new RuntimeException('Text2Image generation using provider ' . $provider->getName() . ' failed: Couldn\'t open file to write.');
+				}
+				$this->logger->debug('Calling Text2Image provider\'s generate method');
+				$provider->generate($task->getInput(), $resource);
+				if (is_resource($resource)) {
+					// If $resource hasn't been closed yet, we'll do that here
+					fclose($resource);
+				}
 				$task->setStatus(Task::STATUS_SUCCESSFUL);
+				$this->logger->debug('Updating Text2Image task in DB');
 				$this->taskMapper->update(DbTask::fromPublicTask($task));
 				return;
-			} catch (\RuntimeException $e) {
-				$this->logger->info('Text2Image generation using provider ' . $provider->getName() . ' failed', ['exception' => $e]);
-				$task->setStatus(Task::STATUS_FAILED);
-				$this->taskMapper->update(DbTask::fromPublicTask($task));
-				throw $e;
-			} catch (\Throwable $e) {
+			} catch (\RuntimeException|\Throwable $e) {
+				if (isset($resource) && is_resource($resource)) {
+					// If $resource hasn't been closed yet, we'll do that here
+					fclose($resource);
+				}
+				try {
+					if (isset($file)) {
+						$file->delete();
+					}
+				}catch(NotPermittedException $e) {
+					$this->logger->warning('Failed to clean up Text2Image result file after error', ['exception' => $e]);
+				}
 				$this->logger->info('Text2Image generation using provider ' . $provider->getName() . ' failed', ['exception' => $e]);
 				$task->setStatus(Task::STATUS_FAILED);
-				$this->taskMapper->update(DbTask::fromPublicTask($task));
-				throw new RuntimeException('Text2Image generation using provider ' . $provider->getName() . ' failed: ' . $e->getMessage(), 0, $e);
+				try {
+					$this->taskMapper->update(DbTask::fromPublicTask($task));
+				} catch (Exception $e) {
+					$this->logger->warning('Failed to update database after Text2Image error', ['exception' => $e]);
+				}
+				if ($e instanceof RuntimeException) {
+					throw $e;
+				}else {
+					throw new RuntimeException('Text2Image generation using provider ' . $provider->getName() . ' failed: ' . $e->getMessage(), 0, $e);
+				}
 			}
 		}
 

lib/public/DB/Exception.php

@@ -139,7 +139,7 @@ class Exception extends BaseException {
 
 	/**
 	 * @return int|null
-	 * @psalm-return Exception::REASON_*
+	 * @psalm-return TextToImageException::REASON_*
 	 * @since 21.0.0
 	 */
 	public function getReason(): ?int {

lib/public/TextToImage/Exception/TaskNotFoundException.php

@@ -24,5 +24,5 @@
 
 namespace OCP\TextToImage\Exception;
 
-class TaskNotFoundException extends Exception {
+class TaskNotFoundException extends TextToImageException {
 }

lib/public/TextToImage/Exception/TextToImageException.php

@@ -24,5 +24,5 @@
 
 namespace OCP\TextToImage\Exception;
 
-class Exception extends \Exception {
+class TextToImageException extends \Exception {
 }