diff --git a/l10n/en.js b/l10n/en.js index 579aecf1b..1f8c6f9a5 100644 --- a/l10n/en.js +++ b/l10n/en.js @@ -1073,7 +1073,11 @@ OC.L10N.register( "Application published": "Application published", "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.", "Application registered by": "Application registered by", - "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions." + "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.", + "The file could not be sent. Check your connection and try again.": "The file could not be sent. Check your connection and try again.", + "The organisation register is not configured, so there are no organisations to choose from.": "The organisation register is not configured, so there are no organisations to choose from.", + "The organisations could not be loaded. Reload the page to try again.": "The organisations could not be loaded. Reload the page to try again.", + "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Whether a service desk is connected could not be loaded. Reload the page to try again." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/en.json b/l10n/en.json index 494792fa9..800f5f8b7 100644 --- a/l10n/en.json +++ b/l10n/en.json @@ -1072,6 +1072,10 @@ "Application published": "Application published", "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.", "Application registered by": "Application registered by", - "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions." + "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.", + "The file could not be sent. Check your connection and try again.": "The file could not be sent. Check your connection and try again.", + "The organisation register is not configured, so there are no organisations to choose from.": "The organisation register is not configured, so there are no organisations to choose from.", + "The organisations could not be loaded. Reload the page to try again.": "The organisations could not be loaded. Reload the page to try again.", + "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Whether a service desk is connected could not be loaded. Reload the page to try again." } } diff --git a/l10n/nl.js b/l10n/nl.js index 77d23a5e6..5b3007bec 100644 --- a/l10n/nl.js +++ b/l10n/nl.js @@ -1143,7 +1143,11 @@ OC.L10N.register( "Application published": "Applicatie gepubliceerd", "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "De publicatiedatum van de applicatie waar deze versie bij hoort, daarvan overgenomen. Een versie is alleen openbaar zolang haar applicatie dat is.", "Application registered by": "Applicatie geregistreerd door", - "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook." + "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook.", + "The file could not be sent. Check your connection and try again.": "Het bestand kon niet worden verstuurd. Controleer uw verbinding en probeer het opnieuw.", + "The organisation register is not configured, so there are no organisations to choose from.": "Het organisatieregister is niet ingesteld, dus er zijn geen organisaties om uit te kiezen.", + "The organisations could not be loaded. Reload the page to try again.": "De organisaties konden niet worden geladen. Herlaad de pagina om het opnieuw te proberen.", + "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Kon niet worden geladen of er een servicedesk is gekoppeld. Herlaad de pagina om het opnieuw te proberen." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/nl.json b/l10n/nl.json index 7de3e9617..23f291cf0 100644 --- a/l10n/nl.json +++ b/l10n/nl.json @@ -1142,6 +1142,10 @@ "Application published": "Applicatie gepubliceerd", "The publication date of the application this version belongs to, copied from it. A version is public only while its application is.": "De publicatiedatum van de applicatie waar deze versie bij hoort, daarvan overgenomen. Een versie is alleen openbaar zolang haar applicatie dat is.", "Application registered by": "Applicatie geregistreerd door", - "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook." + "Who registered the application this version belongs to, copied from it. A supplier's application is public, and so are its versions.": "Wie de applicatie registreerde waar deze versie bij hoort, daarvan overgenomen. De applicatie van een leverancier is openbaar, en haar versies ook.", + "The file could not be sent. Check your connection and try again.": "Het bestand kon niet worden verstuurd. Controleer uw verbinding en probeer het opnieuw.", + "The organisation register is not configured, so there are no organisations to choose from.": "Het organisatieregister is niet ingesteld, dus er zijn geen organisaties om uit te kiezen.", + "The organisations could not be loaded. Reload the page to try again.": "De organisaties konden niet worden geladen. Herlaad de pagina om het opnieuw te proberen.", + "Whether a service desk is connected could not be loaded. Reload the page to try again.": "Kon niet worden geladen of er een servicedesk is gekoppeld. Herlaad de pagina om het opnieuw te proberen." } } diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 7e31c1f78..a185d5a50 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -22,6 +22,7 @@ use OCA\OpenRegister\Contract\ObjectServiceInterface; use OCA\OpenRegister\Event\ObjectCreatedEvent; +use OCA\OpenRegister\Event\ObjectDeletedEvent; use OCA\OpenRegister\Event\ObjectUpdatedEvent; use OCA\OpenRegister\Event\UserProfileUpdatedEvent; use OCA\OpenRegister\Service\OrganisationService as OpenRegisterOrganisationService; @@ -815,6 +816,7 @@ private function registerEventListeners(IRegistrationContext $context): void { // A module version is public only while its application is (publication-field-rules). $context->registerEventListener(ObjectCreatedEvent::class, ModuleVersionPublicationListener::class); $context->registerEventListener(ObjectUpdatedEvent::class, ModuleVersionPublicationListener::class); + $context->registerEventListener(ObjectDeletedEvent::class, ModuleVersionPublicationListener::class); // Sync user profile updates into the contactpersoon mirror. $context->registerEventListener(UserProfileUpdatedEvent::class, UserProfileUpdatedEventListener::class); diff --git a/lib/Controller/CmdbImportController.php b/lib/Controller/CmdbImportController.php index 821deaece..a8425feb0 100644 --- a/lib/Controller/CmdbImportController.php +++ b/lib/Controller/CmdbImportController.php @@ -101,7 +101,7 @@ public function import(): JSONResponse { ['error' => $e->getErrorCode(), 'details' => $e->getDetails(), 'reason' => $e->getMessage()] ); return $this->fromException(e: $e); - } catch (\Exception $e) { + } catch (\Throwable $e) { $this->logger->error('CmdbImportController: import failed', ['exception' => $e]); return $this->error(code: 'IMPORT_FAILED', status: Http::STATUS_INTERNAL_SERVER_ERROR); } diff --git a/lib/EventListener/ModuleVersionPublicationListener.php b/lib/EventListener/ModuleVersionPublicationListener.php index 581ba7335..1e143b513 100644 --- a/lib/EventListener/ModuleVersionPublicationListener.php +++ b/lib/EventListener/ModuleVersionPublicationListener.php @@ -21,6 +21,7 @@ namespace OCA\Stackiq\EventListener; use OCA\OpenRegister\Event\ObjectCreatedEvent; +use OCA\OpenRegister\Event\ObjectDeletedEvent; use OCA\OpenRegister\Event\ObjectUpdatedEvent; use OCA\Stackiq\Service\ModuleVersionPublicationService; use OCP\EventDispatcher\Event; @@ -29,7 +30,7 @@ use Throwable; /** - * Hands every created or updated object to the publication mirror. + * Hands every created, updated or deleted object to the publication mirror. * * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is * @@ -59,21 +60,18 @@ public function __construct( * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is */ public function handle(Event $event): void { - $object = null; - if ($event instanceof ObjectUpdatedEvent) { - $object = $event->getNewObject(); - } - - if ($event instanceof ObjectCreatedEvent) { - $object = $event->getObject(); - } + try { + if ($event instanceof ObjectUpdatedEvent) { + $this->publication->objectSaved(object: $event->getNewObject(), previous: $event->getOldObject()); + } - if ($object === null) { - return; - } + if ($event instanceof ObjectCreatedEvent) { + $this->publication->objectSaved(object: $event->getObject()); + } - try { - $this->publication->objectSaved(object: $object); + if ($event instanceof ObjectDeletedEvent) { + $this->publication->objectDeleted(object: $event->getObject()); + } } catch (Throwable $e) { $this->logger->error('ModuleVersionPublicationListener: could not mirror the publication', ['error' => $e->getMessage()]); } diff --git a/lib/Repair/BackfillModuleVersionPublication.php b/lib/Repair/BackfillModuleVersionPublication.php index 9a0b9b28f..861084554 100644 --- a/lib/Repair/BackfillModuleVersionPublication.php +++ b/lib/Repair/BackfillModuleVersionPublication.php @@ -7,7 +7,9 @@ * so for anonymous readers they read as unpublished until their module is * saved again. That is the safe direction; this step makes the published ones * public again without waiting for an edit. It is idempotent: a version that - * already holds its module's values is not written. + * already holds its module's values is not written. It reads the modules page + * by page, and after a pass in which nothing failed it records that in the app + * config, so later upgrades skip it. * * @category Repair * @package OCA\Stackiq\Repair @@ -27,9 +29,12 @@ namespace OCA\Stackiq\Repair; use OCA\OpenRegister\Contract\ObjectEntityInterface; +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\Stackiq\AppInfo\Application; use OCA\Stackiq\Service\ModuleVersionPublicationService; use OCA\Stackiq\Service\SettingsService; use OCP\App\IAppManager; +use OCP\IAppConfig; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use Throwable; @@ -41,17 +46,33 @@ */ class BackfillModuleVersionPublication implements IRepairStep { + /** + * App-config key set after a pass in which every module and version was handled. + * + * @var string + */ + public const DONE_CONFIG_KEY = 'module_version_publication_backfilled'; + + /** + * How many modules one read returns. + * + * @var integer + */ + public const PAGE_SIZE = 200; + /** * Constructor. * * @param IAppManager $appManager Tells whether OpenRegister is installed. * @param SettingsService $settingsService Resolves the module schema and the object service. * @param ModuleVersionPublicationService $publication The mirror. + * @param IAppConfig $appConfig Holds the done marker. */ public function __construct( private readonly IAppManager $appManager, private readonly SettingsService $settingsService, private readonly ModuleVersionPublicationService $publication, + private readonly IAppConfig $appConfig, ) { }//end __construct() @@ -74,6 +95,10 @@ public function getName(): string { * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is */ public function run(IOutput $output): void { + if ($this->appConfig->getValueBool(Application::APP_ID, self::DONE_CONFIG_KEY, false) === true) { + return; + } + if (in_array('openregister', $this->appManager->getInstalledApps(), true) === false) { $output->info('OpenRegister not installed, so there are no versions to update.'); return; @@ -88,19 +113,54 @@ public function run(IOutput $output): void { } try { - $modules = $objects->setRegister($register)->setSchema($schema)->findAll([], false, false); + $result = $this->backfillAll(objects: $objects, register: $register, schema: $schema); } catch (Throwable $e) { $output->warning('Could not read the applications: ' . $e->getMessage()); return; } - $written = 0; - foreach ((array) $modules as $module) { - if (($module instanceof ObjectEntityInterface) === true) { - $written += $this->publication->moduleSaved(module: $module); - } + $output->info($result['written'] . ' versions now follow their application\'s publication.'); + if ($result['failed'] > 0) { + $output->warning($result['failed'] . ' versions or searches failed; the next upgrade tries again.'); + return; } - $output->info($written . ' versions now follow their application\'s publication.'); + $this->appConfig->setValueBool(Application::APP_ID, self::DONE_CONFIG_KEY, true); }//end run() + + /** + * Backfill every module, page by page. + * + * @param ObjectServiceInterface $objects The object service. + * @param int|string $register The module register. + * @param int|string $schema The module schema. + * + * @return array{written: int, failed: int} The versions written, and the versions or searches that failed. + * + * @throws Throwable When a page of modules cannot be read. + */ + private function backfillAll(ObjectServiceInterface $objects, int|string $register, int|string $schema): array { + $total = ['written' => 0, 'failed' => 0]; + $offset = 0; + do { + $modules = (array) $objects->setRegister($register)->setSchema($schema)->findAll( + ['limit' => self::PAGE_SIZE, 'offset' => $offset], + false, + false + ); + + foreach ($modules as $module) { + if (($module instanceof ObjectEntityInterface) === true) { + $result = $this->publication->backfillModule(module: $module); + $total['written'] += $result['written']; + $total['failed'] += $result['failed']; + } + } + + $offset += self::PAGE_SIZE; + $pageSize = count($modules); + } while ($pageSize === self::PAGE_SIZE); + + return $total; + }//end backfillAll() }//end class diff --git a/lib/Service/ArchiMateImportService.php b/lib/Service/ArchiMateImportService.php index f3cc5e218..7f06a382c 100644 --- a/lib/Service/ArchiMateImportService.php +++ b/lib/Service/ArchiMateImportService.php @@ -498,8 +498,7 @@ public function importArchiMateFileFromPathOptimized(array $options = []): array ); if ($this->operationId !== null) { - $this->progressTracker->addError(message: $e->getMessage()); - $this->progressTracker->completeOperation(); + $this->progressTracker->failOperation(message: $e->getMessage()); } return [ diff --git a/lib/Service/CmdbExportImportService.php b/lib/Service/CmdbExportImportService.php index 793e9b836..93e988a62 100644 --- a/lib/Service/CmdbExportImportService.php +++ b/lib/Service/CmdbExportImportService.php @@ -217,7 +217,7 @@ public function supportsMissingRecords(string $mode): bool { * * @param string $operationId The operation id. * - * @return bool False when no `cmdb_import` operation has this id. + * @return bool False when no running `cmdb_import` operation has this id. * * @spec openspec/changes/cmdb-export-import/tasks.md#task-7 */ @@ -227,7 +227,10 @@ public function requestCancel(string $operationId): bool { } $progress = $this->progressTracker->getProgress(operationId: $operationId); - if (is_array($progress) === false || ($progress['operation_type'] ?? null) !== self::OPERATION_TYPE) { + if (is_array($progress) === false + || ($progress['operation_type'] ?? null) !== self::OPERATION_TYPE + || ($progress['status'] ?? null) !== 'running' + ) { return false; } @@ -282,25 +285,32 @@ public function import(string $path, array $options): array { $this->progressTracker->setPhase(phase: 'processing_elements', data: ['total_items' => count($rows)]); $updateExisting = (($options['updateExisting'] ?? true) !== false); - foreach ($rows as $index => $row) { - if ($this->progressTracker->isCancelRequested(operationId: $operationId) === true) { - $report->markCancelled(); - break; - } + try { + foreach ($rows as $index => $row) { + if ($this->progressTracker->isCancelRequested(operationId: $operationId) === true) { + $report->markCancelled(); + break; + } - $this->processRow( - row: $row, - municipalityUuid: $municipality['uuid'], - updateExisting: $updateExisting, - startedAt: $startedAt, - date1904: $workbook['date1904'], - report: $report - ); - $this->progressTracker->updateProgress(processedItems: ($index + 1)); - } + $this->processRow( + row: $row, + municipalityUuid: $municipality['uuid'], + updateExisting: $updateExisting, + startedAt: $startedAt, + date1904: $workbook['date1904'], + report: $report + ); + $this->progressTracker->updateProgress(processedItems: ($index + 1)); + } - $result = $report->toArray(); - $this->finishOperation(report: $result); + $result = $report->toArray(); + $this->finishOperation(report: $result); + } catch (Throwable $e) { + // Rows catch their own errors; this is the run itself failing, so the + // operation stops as failed instead of staying running until it expires. + $this->progressTracker->failOperation(message: $e->getMessage()); + throw $e; + }//end try $this->logger->info( 'CmdbExportImportService: import finished', diff --git a/lib/Service/ItsmExchangeService.php b/lib/Service/ItsmExchangeService.php index bd9032319..e14991f39 100644 --- a/lib/Service/ItsmExchangeService.php +++ b/lib/Service/ItsmExchangeService.php @@ -265,20 +265,29 @@ public function buildFlows(string $desk, string $organisation, string $runAs, st * @spec openspec/changes/sharing-itsm-exchange/specs/itsm-exchange/spec.md#requirement-req-itx-001-an-administrator-sets-up-the-exchange-without-stackiq-holding-a-credential */ public function setUp(string $desk, string $organisation, string $runAs, string $templateId = ''): array { - $source = $this->preconditions(desk: $desk, organisation: $organisation); - if (is_string($source) === true) { - return $this->refuse(message: $source); + $found = $this->preconditions(desk: $desk, organisation: $organisation); + if (is_string($found) === true) { + return $this->refuse(message: $found); } - $flows = $this->buildFlows( - desk: $desk, - organisation: $organisation, - runAs: $runAs, - location: (string) ($source['location'] ?? ''), - templateId: $templateId - ); + // The flows compare this with a usage's consumer uuid, so store the uuid + // even when the admin gave an id or a slug. + $organisation = $found['organisation']; + try { + $flows = $this->buildFlows( + desk: $desk, + organisation: $organisation, + runAs: $runAs, + location: (string) ($found['source']['location'] ?? ''), + templateId: $templateId + ); + + $blocking = $this->blockingFindings(flows: $flows); + } catch (Throwable $e) { + $this->logger->error('[ItsmExchangeService] Checking the exchange flows failed', ['exception' => $e]); + return $this->refuse(message: 'Nothing was created. The flows could not be checked: ' . $e->getMessage()); + } - $blocking = $this->blockingFindings(flows: $flows); if ($blocking !== []) { $first = (array) reset($blocking); $entry = (array) ($first[0] ?? []); @@ -317,7 +326,9 @@ public function setUp(string $desk, string $organisation, string $runAs, string * @param string $desk The desk key. * @param string $organisation The organisation uuid. * - * @return array|string The integriq source, or why the set-up cannot start. + * @return array{source: array, organisation: string}|string The integriq source and the + * organisation's uuid, or why the + * set-up cannot start. */ private function preconditions(string $desk, string $organisation): array|string { if ($this->gateway->available() === false) { @@ -328,7 +339,8 @@ private function preconditions(string $desk, string $organisation): array|string return 'Unknown service desk "' . $desk . '".'; } - if ($this->gateway->findObject(register: self::REGISTER, schema: 'organization', id: $organisation) === null) { + $found = $this->gateway->findObject(register: self::REGISTER, schema: 'organization', id: $organisation); + if ($found === null || (string) ($found['uuid'] ?? '') === '') { return 'The organisation ' . $organisation . ' does not exist in stackiq.'; } @@ -338,7 +350,7 @@ private function preconditions(string $desk, string $organisation): array|string return 'Integriq has no source "' . $profile['source'] . '". Add the ' . $profile['label'] . ' source in integriq first.'; } - return $source; + return ['source' => $source, 'organisation' => (string) $found['uuid']]; }//end preconditions() /** diff --git a/lib/Service/ItsmFileImportService.php b/lib/Service/ItsmFileImportService.php index 6efad1cee..7e53c9dba 100644 --- a/lib/Service/ItsmFileImportService.php +++ b/lib/Service/ItsmFileImportService.php @@ -29,6 +29,7 @@ use OCA\Stackiq\AppInfo\Application; use OCA\Stackiq\Service\Itsm\ItsmFlowGateway; use OCP\IAppConfig; +use Psr\Log\LoggerInterface; use RuntimeException; use Throwable; @@ -46,6 +47,20 @@ class ItsmFileImportService { */ public const MAX_ROWS = 5000; + /** + * The largest file one import reads, in bytes (10 MiB, as the CMDB import). + * + * @var integer + */ + public const MAX_FILE_BYTES = 10485760; + + /** + * PhpSpreadsheet's XLSX reader, as OpenRegister ships it. + * + * @var string + */ + public const XLSX_READER = '\PhpOffice\PhpSpreadsheet\Reader\Xlsx'; + /** * The column every row must fill. * @@ -58,10 +73,12 @@ class ItsmFileImportService { * * @param ItsmFlowGateway $gateway OpenRegister's flow store. * @param IAppConfig $appConfig The app settings. + * @param LoggerInterface $logger Logs a flow that could not be started. */ public function __construct( private readonly ItsmFlowGateway $gateway, private readonly IAppConfig $appConfig, + private readonly LoggerInterface $logger, ) { }//end __construct() @@ -85,6 +102,10 @@ public function import(string $path, string $name): array { return ['started' => false, 'message' => 'Set up the exchange first. The file import uses the flow the set-up creates.']; } + if (is_file($path) === true && filesize($path) > self::MAX_FILE_BYTES) { + return ['started' => false, 'message' => 'The file is larger than ' . (self::MAX_FILE_BYTES / 1048576) . ' MB; one import takes at most that.']; + } + try { $rows = $this->readRows(path: $path, name: $name); } catch (Throwable $e) { @@ -96,7 +117,12 @@ public function import(string $path, string $name): array { return ['started' => false, 'message' => $problem]; } - $run = $this->gateway->run(uuid: $flow, payload: ['rows' => $rows]); + try { + $run = $this->gateway->run(uuid: $flow, payload: ['rows' => $rows]); + } catch (Throwable $e) { + $this->logger->error('[ItsmFileImportService] Starting the file import flow failed', ['exception' => $e]); + return ['started' => false, 'message' => 'The file import flow could not be started: ' . $e->getMessage()]; + } return ['started' => true, 'run' => $run, 'rows' => count($rows)]; }//end import() @@ -107,6 +133,9 @@ public function import(string $path, string $name): array { * @param string $path The file. * @param string $name Its name. * + * Reading stops one row past MAX_ROWS, so an oversized file is refused + * without reading the rest of it. + * * @return list> The rows, empty cells left out. * * @throws RuntimeException When the type is not CSV or XLSX, or XLSX cannot be read here. @@ -114,11 +143,18 @@ public function import(string $path, string $name): array { * @spec openspec/changes/sharing-itsm-exchange/specs/itsm-exchange/spec.md#requirement-req-itx-006-a-file-feeds-the-same-import */ public function readRows(string $path, string $name): array { - $table = $this->readTable(path: $path, name: $name); - - $header = array_map(static fn ($cell): string => trim((string) $cell), (array) array_shift($table)); + $header = null; $rows = []; - foreach ($table as $cells) { + foreach ($this->readTable(path: $path, name: $name) as $cells) { + if ($header === null) { + $header = array_map(static fn ($cell): string => trim((string) $cell), $cells); + continue; + } + + if (count($rows) > self::MAX_ROWS) { + break; + } + $row = []; foreach ($header as $index => $column) { $cell = trim((string) ($cells[$index] ?? '')); @@ -141,11 +177,11 @@ public function readRows(string $path, string $name): array { * @param string $path The file. * @param string $name Its name. * - * @return list> The cells. + * @return iterable> The cells, row by row. * * @throws RuntimeException When the type is not CSV or XLSX. */ - private function readTable(string $path, string $name): array { + private function readTable(string $path, string $name): iterable { $extension = strtolower(pathinfo($name, PATHINFO_EXTENSION)); if ($extension === 'csv') { return $this->readCsv(path: $path); @@ -173,7 +209,7 @@ public function checkRows(array $rows): ?string { } if (count($rows) > self::MAX_ROWS) { - return 'The file holds ' . count($rows) . ' rows; one import takes at most ' . self::MAX_ROWS . '.'; + return 'The file holds more than ' . self::MAX_ROWS . ' rows; one import takes at most ' . self::MAX_ROWS . '.'; } foreach ($rows as $index => $row) { @@ -190,49 +226,93 @@ public function checkRows(array $rows): ?string { * * @param string $path The file. * - * @return list> The cells. + * @return \Generator> The cells, row by row; the file closes when reading stops. */ - private function readCsv(string $path): array { + private function readCsv(string $path): \Generator { $handle = fopen($path, 'r'); if ($handle === false) { throw new RuntimeException('cannot open the uploaded file'); } - $first = (string) fgets($handle); - $delimiter = ','; - if (substr_count($first, ';') > substr_count($first, ',')) { - $delimiter = ';'; - } + try { + $first = (string) fgets($handle); + $delimiter = ','; + if (substr_count($first, ';') > substr_count($first, ',')) { + $delimiter = ';'; + } - rewind($handle); - $table = []; - while (($cells = fgetcsv($handle, null, $delimiter, '"', '\\')) !== false) { - $table[] = array_map(static fn ($cell): string => (string) $cell, $cells); - } + rewind($handle); + $isFirst = true; + while (($cells = fgetcsv($handle, null, $delimiter, '"', '\\')) !== false) { + $cells = array_map(static fn ($cell): string => (string) $cell, $cells); + if ($isFirst === true && isset($cells[0]) === true) { + $cells[0] = (string) preg_replace('/^\xEF\xBB\xBF/', '', $cells[0]); + } - fclose($handle); - if (isset($table[0][0]) === true) { - $table[0][0] = preg_replace('/^\xEF\xBB\xBF/', '', $table[0][0]); + $isFirst = false; + yield $cells; + } + } finally { + fclose($handle); } - - return $table; }//end readCsv() /** * Read the first sheet of an XLSX file into rows of cells, with PhpSpreadsheet as OpenRegister ships it. * + * Always the XLSX reader, whatever the content looks like, with data only. + * A formula gives the value Excel cached; it is never calculated here. + * * @param string $path The file. * - * @return list> The cells. + * @return \Generator> The cells, row by row; the workbook is released when reading stops. */ - private function readXlsx(string $path): array { - $factory = '\PhpOffice\PhpSpreadsheet\IOFactory'; - if (class_exists($factory) === false) { + private function readXlsx(string $path): \Generator { + $readerClass = self::XLSX_READER; + if (class_exists($readerClass) === false) { throw new RuntimeException('reading .xlsx needs PhpSpreadsheet, which OpenRegister provides; save the sheet as .csv instead'); } - $sheet = $factory::load($path)->getActiveSheet()->toArray(null, true, false, false); + $reader = new $readerClass(); + $reader->setReadDataOnly(true); + $spreadsheet = $reader->load($path); + try { + foreach ($spreadsheet->getActiveSheet()->getRowIterator() as $row) { + $iterator = $row->getCellIterator(); + $iterator->setIterateOnlyExistingCells(false); + $cells = []; + foreach ($iterator as $cell) { + $cells[] = self::cellText(cell: $cell); + } - return array_map(static fn ($cells): array => array_map(static fn ($cell): string => (string) $cell, (array) $cells), (array) $sheet); + yield $cells; + } + } finally { + $spreadsheet->disconnectWorksheets(); + } }//end readXlsx() + + /** + * The text of one XLSX cell; for a formula, the value Excel cached. + * + * @param object $cell The PhpSpreadsheet cell. + * + * @return string The text, or an empty string for a value that is not text or a number. + */ + private static function cellText(object $cell): string { + $value = $cell->getValue(); + if ($cell->getDataType() === 'f') { + $value = $cell->getOldCalculatedValue(); + } + + if (is_object($value) === true && method_exists($value, 'getPlainText') === true) { + return (string) $value->getPlainText(); + } + + if (is_scalar($value) === false) { + return ''; + } + + return (string) $value; + }//end cellText() }//end class diff --git a/lib/Service/ModuleVersionPublicationService.php b/lib/Service/ModuleVersionPublicationService.php index 0126dfb5d..829a58001 100644 --- a/lib/Service/ModuleVersionPublicationService.php +++ b/lib/Service/ModuleVersionPublicationService.php @@ -42,7 +42,7 @@ class ModuleVersionPublicationService { /** - * The most versions one module save updates. + * How many versions one search reads; a module with more is read page by page. * * @var integer */ @@ -96,15 +96,25 @@ private static function text(mixed $value): ?string { /** * React to a saved object: a module updates its versions, a version reads its module. * - * @param ObjectEntityInterface $object The saved object. + * A module update that leaves its publication date and registrant as they + * were has nothing to copy, so its versions are not searched. + * + * @param ObjectEntityInterface $object The saved object. + * @param ObjectEntityInterface|null $previous The object before an update, or null for a new one. * * @return integer The number of versions written. * * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is */ - public function objectSaved(ObjectEntityInterface $object): int { + public function objectSaved(ObjectEntityInterface $object, ?ObjectEntityInterface $previous = null): int { $schema = (string) $object->getSchema(); if ($schema === (string) $this->settingsService->getSchemaIdForObjectType('module')) { + if ($previous !== null + && self::mirrorOf(module: (array) $previous->getObject()) === self::mirrorOf(module: (array) $object->getObject()) + ) { + return 0; + } + return $this->moduleSaved(module: $object); } @@ -115,6 +125,23 @@ public function objectSaved(ObjectEntityInterface $object): int { return 0; }//end objectSaved() + /** + * React to a deleted object: the versions of a deleted module stop following a publication. + * + * @param ObjectEntityInterface $object The deleted object. + * + * @return integer The number of versions written. + * + * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is + */ + public function objectDeleted(ObjectEntityInterface $object): int { + if ((string) $object->getSchema() !== (string) $this->settingsService->getSchemaIdForObjectType('module')) { + return 0; + } + + return $this->copyOntoVersions(moduleUuid: (string) $object->getUuid(), mirror: self::mirrorOf(module: []))['written']; + }//end objectDeleted() + /** * Copy a module's publication onto every version of it that differs. * @@ -125,34 +152,142 @@ public function objectSaved(ObjectEntityInterface $object): int { * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is */ public function moduleSaved(ObjectEntityInterface $module): int { + return $this->backfillModule(module: $module)['written']; + }//end moduleSaved() + + /** + * Copy a module's publication onto its versions and say what could not be copied. + * + * @param ObjectEntityInterface $module The module. + * + * @return array{written: int, failed: int} The versions written, and the versions or searches that failed. + * + * @spec openspec/changes/publication-field-rules/specs/publication-field-rules/spec.md#requirement-req-pfr-002-a-module-version-is-public-only-while-its-application-is + */ + public function backfillModule(ObjectEntityInterface $module): array { + return $this->copyOntoVersions(moduleUuid: (string) $module->getUuid(), mirror: self::mirrorOf(module: (array) $module->getObject())); + }//end backfillModule() + + /** + * Copy a mirror onto every version of a module that differs, page by page. + * + * @param string $moduleUuid The module. + * @param array{modulePublicationDate: string|null, moduleRegisteredBy: string|null} $mirror The values to hold. + * + * @return array{written: int, failed: int} The versions written, and the versions or searches that failed. + */ + private function copyOntoVersions(string $moduleUuid, array $mirror): array { $objects = $this->objectService(); $register = $this->settingsService->getRegisterIdForObjectType('moduleVersion'); $schema = $this->settingsService->getSchemaIdForObjectType('moduleVersion'); + $result = ['written' => 0, 'failed' => 0]; if ($objects === null || $register === null || $schema === null) { - return 0; + return $result; } - try { - $versions = $objects->searchObjects( - query: ['register' => $register, 'schema' => $schema, 'module' => $module->getUuid(), '_limit' => self::VERSION_LIMIT], - _rbac: false, - _multitenancy: false - ); - } catch (Throwable $e) { - $this->logger->error('ModuleVersionPublicationService: could not read the versions', ['error' => $e->getMessage()]); - return 0; - } + $offset = 0; + do { + try { + $versions = (array) $objects->searchObjects( + query: [ + 'register' => $register, + 'schema' => $schema, + 'module' => $moduleUuid, + '_limit' => self::VERSION_LIMIT, + '_offset' => $offset, + ], + _rbac: false, + _multitenancy: false + ); + } catch (Throwable $e) { + $this->logFailure( + message: 'ModuleVersionPublicationService: could not read the versions', + context: ['module' => $moduleUuid, 'error' => $e->getMessage()], + depublishes: (self::isPublicNow(mirror: $mirror) === false) + ); + $result['failed']++; + return $result; + } + + $page = $this->writeVersions(objects: $objects, versions: $versions, mirror: $mirror); + $result['written'] += $page['written']; + $result['failed'] += $page['failed']; + + $offset += self::VERSION_LIMIT; + $pageSize = count($versions); + } while ($pageSize === self::VERSION_LIMIT); + + return $result; + }//end copyOntoVersions() + + /** + * Write a mirror onto each version of one page. + * + * @param ObjectServiceInterface $objects The object service. + * @param array $versions The page. + * @param array{modulePublicationDate: string|null, moduleRegisteredBy: string|null} $mirror The values to hold. + * + * @return array{written: int, failed: int} The versions written and the writes that failed. + */ + private function writeVersions(ObjectServiceInterface $objects, array $versions, array $mirror): array { + $result = ['written' => 0, 'failed' => 0]; + foreach ($versions as $version) { + if (($version instanceof ObjectEntityInterface) === false) { + continue; + } + + $outcome = $this->write(objects: $objects, version: $version, mirror: $mirror); + if ($outcome === true) { + $result['written']++; + } - $mirror = self::mirrorOf(module: (array) $module->getObject()); - $written = 0; - foreach ((array) $versions as $version) { - if (($version instanceof ObjectEntityInterface) === true && $this->write(objects: $objects, version: $version, mirror: $mirror) === true) { - $written++; + if ($outcome === null) { + $result['failed']++; } } - return $written; - }//end moduleSaved() + return $result; + }//end writeVersions() + + /** + * Whether a version holding this mirror is public now, by the moduleVersion read rule. + * + * @param array $mirror The mirrored fields. + * + * @return boolean True when an anonymous reader may read it. + */ + private static function isPublicNow(array $mirror): bool { + if (($mirror['moduleRegisteredBy'] ?? null) === 'Supplier') { + return true; + } + + $date = ($mirror['modulePublicationDate'] ?? null); + if (is_string($date) === false || $date === '') { + return false; + } + + $time = strtotime($date); + + return $time !== false && $time <= time(); + }//end isPublicNow() + + /** + * Log a mirror that could not be written: critical when it leaves a version public that should not be. + * + * @param string $message The message. + * @param array $context The context. + * @param boolean $depublishes Whether the write would have taken a version out of public view. + * + * @return void + */ + private function logFailure(string $message, array $context, bool $depublishes): void { + if ($depublishes === true) { + $this->logger->critical($message . '; the version stays public until it is saved again or the backfill runs', $context); + return; + } + + $this->logger->error($message, $context); + }//end logFailure() /** * Copy the module's publication onto a saved version, when it differs. @@ -216,9 +351,13 @@ private static function referenceOf(mixed $value): ?string { * @param ObjectEntityInterface $version The version. * @param array{modulePublicationDate: string|null, moduleRegisteredBy: string|null} $mirror The values to hold. * - * @return boolean True when it was written. + * The version is saved without validation: only the two mirrored fields + * change, and a version holding older data the schema no longer accepts + * must still follow its module. + * + * @return boolean|null True when it was written, false when it was in step, null when the write failed. */ - private function write(ObjectServiceInterface $objects, ObjectEntityInterface $version, array $mirror): bool { + private function write(ObjectServiceInterface $objects, ObjectEntityInterface $version, array $mirror): ?bool { $data = (array) $version->getObject(); if (($data['modulePublicationDate'] ?? null) === $mirror['modulePublicationDate'] && ($data['moduleRegisteredBy'] ?? null) === $mirror['moduleRegisteredBy'] @@ -234,14 +373,16 @@ private function write(ObjectServiceInterface $objects, ObjectEntityInterface $v schema: $version->getSchema(), uuid: $version->getUuid(), _rbac: false, - _multitenancy: false + _multitenancy: false, + _validation: false ); } catch (Throwable $e) { - $this->logger->error( - 'ModuleVersionPublicationService: could not copy the publication onto a version', - ['uuid' => $version->getUuid(), 'error' => $e->getMessage()] + $this->logFailure( + message: 'ModuleVersionPublicationService: could not copy the publication onto a version', + context: ['uuid' => $version->getUuid(), 'error' => $e->getMessage()], + depublishes: (self::isPublicNow(mirror: $data) === true && self::isPublicNow(mirror: $mirror) === false) ); - return false; + return null; } return true; diff --git a/lib/Service/ProgressTracker.php b/lib/Service/ProgressTracker.php index 8bc62e4e6..a2b90aa75 100644 --- a/lib/Service/ProgressTracker.php +++ b/lib/Service/ProgressTracker.php @@ -32,6 +32,11 @@ * admin, the request after a cron run) can read it. Who may read an operation * is decided by SettingsController::getProgress(), not by where it is stored. * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) Each public method is one step of an + * operation's life (start, phase, progress, warning, error, statistics, complete, fail, + * cancel) that an import calls on the same snapshot; splitting them would hand that + * snapshot from class to class. + * * @category Service * @package OCA\Stackiq\Service * @author Conduction b.v. @@ -361,6 +366,42 @@ public function completeOperation(array $finalStatistics = []): void { ); }//end completeOperation() + /** + * Mark the current operation as failed, keeping the percentage it reached. + * + * A page following the operation then sees it stop as failed rather than + * as running until the snapshot expires, or as completed at 100%. + * + * @param string $message Why the operation failed + * + * @return void + * + * @spec openspec/specs/progress-tracking/spec.md + */ + public function failOperation(string $message): void { + $this->progress['errors'][] = [ + 'message' => $message, + 'context' => [], + 'timestamp' => time(), + ]; + $this->progress['phase_description'] = 'Failed'; + $this->progress['status'] = 'failed'; + $this->progress['estimated_completion'] = time(); + $this->saveProgress(); + + if ($this->progress['operation_id'] !== null) { + $this->store->remove(key: 'cancel_' . $this->progress['operation_id']); + } + + $this->logger->error( + 'Operation failed', + [ + 'operation_id' => $this->progress['operation_id'], + 'message' => $message, + ] + ); + }//end failOperation() + /** * Ask a running operation to stop. * diff --git a/lib/Settings/register.d/publication-field-rules.json b/lib/Settings/register.d/publication-field-rules.json index ffe8456c4..a939858af 100644 --- a/lib/Settings/register.d/publication-field-rules.json +++ b/lib/Settings/register.d/publication-field-rules.json @@ -3,7 +3,7 @@ "components": { "schemas": { "module": { - "version": "0.3.6", + "version": "0.3.7", "properties": { "contactPerson": { "authorization": { @@ -32,6 +32,41 @@ "authenticated" ] } + }, + "externalId": { + "authorization": { + "read": [ + "authenticated" + ] + } + }, + "externalNumber": { + "authorization": { + "read": [ + "authenticated" + ] + } + }, + "externalKey": { + "authorization": { + "read": [ + "authenticated" + ] + } + }, + "externalCreatedAt": { + "authorization": { + "read": [ + "authenticated" + ] + } + }, + "externalModifiedAt": { + "authorization": { + "read": [ + "authenticated" + ] + } } } }, diff --git a/openapi.json b/openapi.json index f0d63eddc..ed08dd11a 100644 --- a/openapi.json +++ b/openapi.json @@ -200,6 +200,217 @@ } } } + }, + "/index.php/apps/stackiq/api/itsm/status": { + "get": { + "operationId": "itsmExchange-status", + "summary": "Whether the service desk exchange runs, and with which desk", + "description": "Any signed-in user, CSRF-protected (requesttoken header or OCS-APIRequest: true). The CMDB page reads it. See openspec/changes/sharing-itsm-exchange.", + "tags": [ + "itsm-exchange" + ], + "responses": { + "200": { + "description": "The exchange summary", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmStatus" + } + } + } + }, + "401": { + "description": "Not signed in" + }, + "412": { + "description": "Missing or invalid CSRF token" + } + } + } + }, + "/index.php/apps/stackiq/api/itsm/config": { + "get": { + "operationId": "itsmExchange-config", + "summary": "The full service desk exchange set-up, for the admin section", + "description": "Nextcloud admins and users delegated the stackiq admin settings, CSRF-protected (requesttoken header or OCS-APIRequest: true).", + "tags": [ + "itsm-exchange" + ], + "responses": { + "200": { + "description": "The set-up", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmConfig" + } + } + } + }, + "401": { + "description": "Not signed in" + }, + "403": { + "description": "Not an admin of the stackiq settings" + }, + "412": { + "description": "Missing or invalid CSRF token" + } + } + } + }, + "/index.php/apps/stackiq/api/itsm/setup": { + "post": { + "operationId": "itsmExchange-setUp", + "summary": "Set up, or set up again, the service desk exchange flows", + "description": "Nextcloud admins and users delegated the stackiq admin settings, CSRF-protected (requesttoken header or OCS-APIRequest: true). Every flow is checked by OpenRegister's preflight first; nothing is saved unless all pass. The scheduled imports run as the signed-in user. The source and its credential live in integriq.", + "tags": [ + "itsm-exchange" + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "desk", + "organisation" + ], + "properties": { + "desk": { + "type": "string", + "enum": [ + "topdesk", + "servicenow" + ], + "description": "The service desk" + }, + "organisation": { + "type": "string", + "description": "Id, uuid or slug of the stackiq organization whose applications are exchanged; stored as its uuid" + }, + "templateId": { + "type": "string", + "default": "", + "description": "The TOPdesk asset template a new record is created from; empty for ServiceNow" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Every flow was created or updated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmSetUpResult" + } + } + } + }, + "401": { + "description": "Not signed in", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmSetUpRefusal" + } + } + } + }, + "403": { + "description": "Not an admin of the stackiq settings" + }, + "412": { + "description": "Missing or invalid CSRF token" + }, + "422": { + "description": "Nothing was set up: the flow engine, the desk, the organisation or integriq's source is missing, a flow failed preflight, a template could not be read, or saving the flows failed", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmSetUpRefusal" + } + } + } + } + } + } + }, + "/index.php/apps/stackiq/api/itsm/import": { + "post": { + "operationId": "itsmExchange-import", + "summary": "Import a CSV or XLSX file through the file import flow", + "description": "Nextcloud admins and users delegated the stackiq admin settings, CSRF-protected (requesttoken header or OCS-APIRequest: true). Needs a set-up exchange: the rows are handed, as one run, to the file flow the set-up created.", + "tags": [ + "itsm-exchange" + ], + "requestBody": { + "required": true, + "content": { + "multipart/form-data": { + "schema": { + "type": "object", + "required": [ + "file" + ], + "properties": { + "file": { + "type": "string", + "format": "binary", + "description": "A .csv (comma or semicolon) or .xlsx file, at most 10 MB, with a header row and at most 5000 rows; every row needs a recordId. Columns: recordId, name, supplierName, installedVersion, status" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "The run started", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmImportStarted" + } + } + } + }, + "400": { + "description": "No file was uploaded", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmImportRefusal" + } + } + } + }, + "401": { + "description": "Not signed in" + }, + "403": { + "description": "Not an admin of the stackiq settings" + }, + "412": { + "description": "Missing or invalid CSRF token" + }, + "422": { + "description": "Nothing started: no exchange is set up, the file is too large, not CSV or XLSX, unreadable, empty, over 5000 rows or has a row without recordId, or the flow could not be started", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItsmImportRefusal" + } + } + } + } + } + } } }, "components": { @@ -390,6 +601,229 @@ "additionalProperties": true } } + }, + "ItsmStatus": { + "type": "object", + "required": [ + "available", + "enabled", + "desk", + "setUpAt", + "desks" + ], + "properties": { + "available": { + "type": "boolean", + "description": "Whether OpenRegister's flow engine is there" + }, + "enabled": { + "type": "boolean", + "description": "Whether the exchange is switched on" + }, + "desk": { + "type": "string", + "nullable": true, + "description": "The desk key, or null before a set-up" + }, + "setUpAt": { + "type": "string", + "format": "date-time", + "nullable": true + }, + "desks": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "label" + ], + "properties": { + "id": { + "type": "string" + }, + "label": { + "type": "string" + } + } + } + } + } + }, + "ItsmConfig": { + "type": "object", + "required": [ + "available", + "enabled", + "desk", + "organisation", + "flows", + "setUpAt", + "desks" + ], + "properties": { + "available": { + "type": "boolean" + }, + "enabled": { + "type": "boolean" + }, + "desk": { + "type": "string", + "nullable": true + }, + "organisation": { + "type": "string", + "nullable": true, + "description": "The organisation uuid" + }, + "flows": { + "type": "object", + "description": "Flow uuids by flow key (applications, relations, licences, contracts, outbound, file)", + "additionalProperties": { + "type": "string" + } + }, + "setUpAt": { + "type": "string", + "format": "date-time", + "nullable": true + }, + "desks": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "label" + ], + "properties": { + "id": { + "type": "string" + }, + "label": { + "type": "string" + } + } + } + } + } + }, + "ItsmSetUpResult": { + "type": "object", + "required": [ + "created", + "desk", + "flows" + ], + "properties": { + "created": { + "type": "boolean", + "enum": [ + true + ] + }, + "desk": { + "type": "string" + }, + "flows": { + "type": "object", + "description": "Flow uuids by flow key", + "additionalProperties": { + "type": "string" + } + } + } + }, + "ItsmSetUpRefusal": { + "type": "object", + "required": [ + "created", + "message" + ], + "properties": { + "created": { + "type": "boolean", + "enum": [ + false + ] + }, + "message": { + "type": "string", + "description": "What stopped the set-up, to show as it is" + }, + "blocking": { + "description": "OpenRegister's blocking preflight findings by flow key; an empty list when the refusal is not a preflight finding", + "oneOf": [ + { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "object", + "properties": { + "step": { + "type": "string" + }, + "reason": { + "type": "string" + } + }, + "additionalProperties": true + } + } + }, + { + "type": "array", + "maxItems": 0, + "items": {} + } + ] + } + } + }, + "ItsmImportStarted": { + "type": "object", + "required": [ + "started", + "run", + "rows" + ], + "properties": { + "started": { + "type": "boolean", + "enum": [ + true + ] + }, + "run": { + "type": "string", + "description": "The uuid of the flow run" + }, + "rows": { + "type": "integer", + "description": "The rows handed to the run" + } + } + }, + "ItsmImportRefusal": { + "type": "object", + "required": [ + "started", + "message" + ], + "properties": { + "started": { + "type": "boolean", + "enum": [ + false + ] + }, + "message": { + "type": "string", + "description": "Why nothing started, to show as it is" + } + } } } } diff --git a/src/utils/archiMateImportProgress.js b/src/utils/archiMateImportProgress.js index 74884e1cb..6374d9c6e 100644 --- a/src/utils/archiMateImportProgress.js +++ b/src/utils/archiMateImportProgress.js @@ -85,7 +85,9 @@ export function progressView(progress) { * Read the progress of an operation every two seconds until stopped. * * An operation that is not readable yet (the import has not started it) is - * skipped quietly; the next tick tries again. + * skipped quietly; the next tick tries again. A tick that comes while the + * previous request is still waiting sends nothing, so a slow server never + * collects concurrent progress requests. * * @param {object} options The options * @param {string} options.operationId The operation to follow @@ -106,7 +108,12 @@ export function startProgressPolling({ const url = generateUrl('/apps/stackiq/api/progress/{operationId}', { operationId, }) + let inFlight = false const handle = setIntervalFn(async () => { + if (inFlight) { + return + } + inFlight = true try { const response = await http.get(url) if (response?.data?.progress) { @@ -114,6 +121,8 @@ export function startProgressPolling({ } } catch { // Not readable yet or briefly unavailable: try again on the next tick. + } finally { + inFlight = false } }, 2000) return () => clearIntervalFn(handle) diff --git a/src/utils/archiMateImportProgress.spec.js b/src/utils/archiMateImportProgress.spec.js index 1d307fc7d..4413f6c95 100644 --- a/src/utils/archiMateImportProgress.spec.js +++ b/src/utils/archiMateImportProgress.spec.js @@ -101,6 +101,36 @@ describe('startProgressPolling', () => { await tick() expect(seen).toEqual([]) }) + + it('sends no new request while the previous one is still waiting', async () => { + let tick = null + let answer = null + const get = jest.fn( + () => + new Promise((resolve) => { + answer = resolve + }), + ) + startProgressPolling({ + operationId: 'archimate_import_abc12345', + http: { get }, + onProgress: () => {}, + setIntervalFn: (fn) => { + tick = fn + return 1 + }, + clearIntervalFn: () => {}, + }) + + const first = tick() + await tick() + expect(get).toHaveBeenCalledTimes(1) + + answer({ data: { progress: { percentage: 10 } } }) + await first + tick() + expect(get).toHaveBeenCalledTimes(2) + }) }) describe('cancelImport', () => { diff --git a/src/views/cmdb/CmdbOverview.vue b/src/views/cmdb/CmdbOverview.vue index 466be640b..d705f76ce 100644 --- a/src/views/cmdb/CmdbOverview.vue +++ b/src/views/cmdb/CmdbOverview.vue @@ -51,7 +51,15 @@

{{ t('stackiq', 'Service desk exchange') }}