From 7f96cc0da377c7661e312e212c5c859eb1b86906 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 16 Oct 2023 16:19:19 +0200 Subject: [PATCH] enh(TextToImage): Address review comments Signed-off-by: Marcel Klehr --- core/Controller/TextToImageApiController.php | 5 +- .../Version28000Date20230906104802.php | 10 +-- lib/private/TextToImage/Manager.php | 64 +++++++++++++++---- lib/public/DB/Exception.php | 2 +- .../Exception/TaskNotFoundException.php | 2 +- ...Exception.php => TextToImageException.php} | 2 +- 6 files changed, 56 insertions(+), 29 deletions(-) rename lib/public/TextToImage/Exception/{Exception.php => TextToImageException.php} (94%) diff --git a/core/Controller/TextToImageApiController.php b/core/Controller/TextToImageApiController.php index 6ec715547fea4..02692f09cdf35 100644 --- a/core/Controller/TextToImageApiController.php +++ b/core/Controller/TextToImageApiController.php @@ -58,12 +58,11 @@ public function __construct( } /** - * @PublicPage - * - * Check whether this feature is available + * * Check whether this feature is available * * @return DataResponse */ + #[PublicPage] public function isAvailable(): DataResponse { return new DataResponse([ 'isAvailable' => $this->textToImageManager->hasProviders(), diff --git a/core/Migrations/Version28000Date20230906104802.php b/core/Migrations/Version28000Date20230906104802.php index 662bdd648b7d3..61f0d01dff0e4 100644 --- a/core/Migrations/Version28000Date20230906104802.php +++ b/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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt '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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addIndex(['status'], 't2i_tasks_status'); $table->addIndex(['user_id', 'app_id', 'identifier'], 't2i_tasks_uid_appid_ident'); - $changed = true; - } - - if ($changed) { return $schema; } diff --git a/lib/private/TextToImage/Manager.php b/lib/private/TextToImage/Manager.php index 6333292a5034d..fd9e2212abda5 100644 --- a/lib/private/TextToImage/Manager.php +++ b/lib/private/TextToImage/Manager.php @@ -29,6 +29,8 @@ 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 @@ public function hasProviders(): bool { * @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 @@ public function runTask(Task $task): void { $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); + } } } diff --git a/lib/public/DB/Exception.php b/lib/public/DB/Exception.php index f977ffa739630..987666b05a96e 100644 --- a/lib/public/DB/Exception.php +++ b/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 { diff --git a/lib/public/TextToImage/Exception/TaskNotFoundException.php b/lib/public/TextToImage/Exception/TaskNotFoundException.php index abdd8e1526158..7bb81d0114611 100644 --- a/lib/public/TextToImage/Exception/TaskNotFoundException.php +++ b/lib/public/TextToImage/Exception/TaskNotFoundException.php @@ -24,5 +24,5 @@ namespace OCP\TextToImage\Exception; -class TaskNotFoundException extends Exception { +class TaskNotFoundException extends TextToImageException { } diff --git a/lib/public/TextToImage/Exception/Exception.php b/lib/public/TextToImage/Exception/TextToImageException.php similarity index 94% rename from lib/public/TextToImage/Exception/Exception.php rename to lib/public/TextToImage/Exception/TextToImageException.php index 5e0e7ab1fdac8..59322bd44a4a7 100644 --- a/lib/public/TextToImage/Exception/Exception.php +++ b/lib/public/TextToImage/Exception/TextToImageException.php @@ -24,5 +24,5 @@ namespace OCP\TextToImage\Exception; -class Exception extends \Exception { +class TextToImageException extends \Exception { }