From 84dc544631455ce6ed34302c6e5d3452a0418ed1 Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Sun, 11 Oct 2026 15:09:32 +0200 Subject: [PATCH] feat(import): Provide extended import report Signed-off-by: Kostiantyn Miakshyn --- cypress.config.mjs | 15 + cypress/e2e/tables-import.cy.js | 41 +- cypress/fixtures/test-import-large.csv | 22 +- lib/Activity/ActivityManager.php | 45 +- lib/Activity/TablesProvider.php | 31 +- lib/BackgroundJob/ImportTableJob.php | 32 +- lib/Controller/Errors.php | 5 + lib/Controller/ImportController.php | 122 ++++- lib/Model/ImportProblem.php | 112 +++++ lib/Model/ImportProblemsFile.php | 37 ++ lib/Model/ImportStats.php | 40 +- lib/Notification/NotificationHelper.php | 18 +- lib/Notification/Notifier.php | 50 ++ lib/ResponseDefinitions.php | 25 + lib/Service/ImportService.php | 314 +++++++++++- openapi.json | 471 ++++++++++++++++++ src/modules/modals/FileActionImport.vue | 70 ++- src/modules/modals/Import.vue | 62 ++- .../modals/ImportProblemsFileButton.vue | 51 ++ src/modules/modals/ImportResults.vue | 216 +++++--- src/shared/components/ActivityEntry.vue | 16 +- src/types/openapi/openapi.ts | 273 ++++++++++ .../Service/ImportServiceAuditLogTest.php | 100 +++- tests/unit/Service/ImportServiceTest.php | 290 +++++++++++ 24 files changed, 2261 insertions(+), 197 deletions(-) create mode 100644 lib/Model/ImportProblem.php create mode 100644 lib/Model/ImportProblemsFile.php create mode 100644 src/modules/modals/ImportProblemsFileButton.vue create mode 100644 tests/unit/Service/ImportServiceTest.php diff --git a/cypress.config.mjs b/cypress.config.mjs index c195c60136..f2e262ef87 100644 --- a/cypress.config.mjs +++ b/cypress.config.mjs @@ -2,6 +2,7 @@ * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ +import { exec } from 'node:child_process' import { defineConfig } from 'cypress' import vitePreprocessor from 'cypress-vite' import { nodePolyfills } from 'vite-plugin-node-polyfills' @@ -24,6 +25,20 @@ export default defineConfig({ configFile: false, })) + on('task', { + log(message) { + console.log(message) + return null + }, + exec({ command, timeout = 60000 }) { + return new Promise((resolve) => { + exec(command, { timeout, cwd: config.projectRoot }, (error, stdout, stderr) => { + resolve({ code: error ? (error.code ?? 1) : 0, stdout, stderr }) + }) + }) + }, + }) + // Disable spell checking to prevent rendering differences on('before:browser:launch', (browser, launchOptions) => { if (browser.family === 'chromium' && browser.name !== 'electron') { diff --git a/cypress/e2e/tables-import.cy.js b/cypress/e2e/tables-import.cy.js index fe62fb64bc..95d709196e 100644 --- a/cypress/e2e/tables-import.cy.js +++ b/cypress/e2e/tables-import.cy.js @@ -60,6 +60,38 @@ describe('Import csv', () => { cy.intercept({ method: 'POST', url: '**/apps/tables/v2/import/table/*' }).as('importUploadReq') cy.get('.modal__content button').contains('Import').click() cy.wait('@importUploadReq').its('response.statusCode').should('equal', 200) + + // The import runs as a queued background job — execute it via cron.php CLI + let cronResult + cy.task('exec', { command: 'php ../../cron.php -v "OCA\\Tables\\BackgroundJob\\ImportTableJob"', timeout: 120000 }, { timeout: 130000 }) + .then(result => { + cronResult = result + cy.task('log', `[cron.php] code=${result.code}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`) + }) + + const waitForProblemsFile = (retries = 15) => { + cy.task('exec', { command: 'find ../../data -name "*import-problems*" -print -quit' }) + .then(result => { + if (result.stdout.trim() !== '') { + cy.visit('apps/files/files?dir=/Tables/Imports') + cy.get('[data-cy-files-list-row-name*="import-problems"]', { timeout: 20000 }) + .should('be.visible') + return + } + if (retries <= 0) { + cy.task('exec', { command: 'tail -500 ../../data/nextcloud.log | grep -i "import\\|report" | tail -40' }) + .then(r => { + throw new Error('Import problems file was not created. ' + + `cron.php: code=${cronResult?.code} stderr=${JSON.stringify(cronResult?.stderr)}\n` + + `log tail: ${r.stdout}`) + }) + return + } + cy.wait(2000) + waitForProblemsFile(retries - 1) + }) + } + waitForProblemsFile() }) it('Import small csv from device', () => { @@ -100,7 +132,7 @@ describe('Import csv', () => { cy.wait('@importUploadReq').its('response.statusCode').should('equal', 200) }) - it('Import csv from device with updating of existent files', () => { + it('Import csv from device with updating of existent files and a broken row', () => { cy.intercept({ method: 'GET', url: '**/apps/tables/row/table/*' }).as('rowsReq') cy.loadTable('Welcome to Nextcloud Tables!') @@ -110,6 +142,7 @@ describe('Import csv', () => { const csv = [ ['id', 'What', 'How to do'], [firstRow.id, 'What (Updated)', 'How to do (Updated)'], + ['not-a-number', 'Skipped row', 'Skipped row'], ] cy.writeFile('cypress/fixtures/test-import-update.csv', csv.map(row => row.join(',')).join('\n')) @@ -133,7 +166,11 @@ describe('Import csv', () => { cy.get('[data-cy="importResultRowsInserted"]').should('contain.text', '0') cy.get('[data-cy="importResultRowsUpdated"]').should('contain.text', '1') cy.get('[data-cy="importResultParsingErrors"]').should('contain.text', '0') - cy.get('[data-cy="importResultRowErrors"]').should('contain.text', '0') + cy.get('[data-cy="importResultRowErrors"]').should('contain.text', '1') + cy.get('[data-cy="importResultProblems"] tbody tr').should('have.length', 1) + cy.get('[data-cy="importResultProblems"] tbody tr').first() + .should('contain.text', '3') + .and('contain.text', 'not numeric') }) it('Import small csv from device with an extra ignored column', () => { diff --git a/cypress/fixtures/test-import-large.csv b/cypress/fixtures/test-import-large.csv index a11f445cc8..c9f6990279 100644 --- a/cypress/fixtures/test-import-large.csv +++ b/cypress/fixtures/test-import-large.csv @@ -97,7 +97,7 @@ Q1,Q2,4,True R1,R2,5,False S1,S2,0,True T1,T2,1,False -U1,U2,2,True +U1,U2,99,True V1,V2,3,False W1,W2,4,True X1,X2,5,False @@ -197,7 +197,7 @@ M1,M2,2,True N1,N2,3,False O1,O2,4,True P1,P2,5,False -Q1,Q2,0,True +Q1,Q2,99,True R1,R2,1,False S1,S2,2,True T1,T2,3,False @@ -297,7 +297,7 @@ I1,I2,0,True J1,J2,1,False K1,K2,2,True L1,L2,3,False -M1,M2,4,True +M1,M2,99,True N1,N2,5,False O1,O2,0,True P1,P2,1,False @@ -397,7 +397,7 @@ E1,E2,4,True F1,F2,5,False G1,G2,0,True H1,H2,1,False -I1,I2,2,True +I1,I2,99,True J1,J2,3,False K1,K2,4,True L1,L2,5,False @@ -497,7 +497,7 @@ A1,A2,2,True B1,B2,3,False C1,C2,4,True D1,D2,5,False -E1,E2,0,True +E1,E2,99,True F1,F2,1,False G1,G2,2,True H1,H2,3,False @@ -597,7 +597,7 @@ W1,W2,0,True X1,X2,1,False Y1,Y2,2,True Z1,Z2,3,False -A1,A2,4,True +A1,A2,99,True B1,B2,5,False C1,C2,0,True D1,D2,1,False @@ -697,7 +697,7 @@ S1,S2,4,True T1,T2,5,False U1,U2,0,True V1,V2,1,False -W1,W2,2,True +W1,W2,99,True X1,X2,3,False Y1,Y2,4,True Z1,Z2,5,False @@ -797,7 +797,7 @@ O1,O2,2,True P1,P2,3,False Q1,Q2,4,True R1,R2,5,False -S1,S2,0,True +S1,S2,99,True T1,T2,1,False U1,U2,2,True V1,V2,3,False @@ -897,7 +897,7 @@ K1,K2,0,True L1,L2,1,False M1,M2,2,True N1,N2,3,False -O1,O2,4,True +O1,O2,99,True P1,P2,5,False Q1,Q2,0,True R1,R2,1,False @@ -997,7 +997,7 @@ G1,G2,4,True H1,H2,5,False I1,I2,0,True J1,J2,1,False -K1,K2,2,True +K1,K2,99,True L1,L2,3,False M1,M2,4,True N1,N2,5,False @@ -1097,5 +1097,5 @@ C1,C2,2,True D1,D2,3,False E1,E2,4,True F1,F2,5,False -G1,G2,0,True +G1,G2,99,True H1,H2,1,False diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index 64c30201f9..dd58d478c2 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -221,6 +221,10 @@ private function createEvent(string $objectType, Row2|Table|View|Column $object, break; case self::SUBJECT_IMPORT_FINISHED: $subjectParams['importStats'] = $additionalParams['importStats'] ?? null; + $subjectParams['importProblemsFile'] = $additionalParams['importProblemsFile'] ?? null; + break; + case self::SUBJECT_IMPORT_FAILED: + $subjectParams['errorMessage'] = $additionalParams['errorMessage'] ?? null; break; default: throw new \Exception(sprintf('Unknown subject "%s" for activity.', $subject)); @@ -322,6 +326,14 @@ private function sendToUsers(IEvent $event, Row2|Table|View|Column $object) { return; } + if ($subject === self::SUBJECT_IMPORT_FAILED) { + if ($event->getAuthor() !== '') { + $event->setAffectedUser($event->getAuthor()); + $this->manager->publish($event); + } + return; + } + if ($object instanceof Table || $object instanceof View) { $uniqueAffectedUser = $this->findRecipientsByElement($object); } elseif ($object instanceof Row2 || $object instanceof Column) { @@ -333,7 +345,15 @@ private function sendToUsers(IEvent $event, Row2|Table|View|Column $object) { return; } + $subjectParams = $event->getSubjectParameters(); foreach ($uniqueAffectedUser as $userId) { + if ($subject === self::SUBJECT_IMPORT_FINISHED) { + $params = $subjectParams; + if ($userId !== $event->getAuthor()) { + unset($params['importStats'], $params['importProblemsFile']); + } + $event->setSubject($subject, $params); + } $event->setAffectedUser($userId); /** @noinspection DisconnectedForeachInstructionInspection */ @@ -592,7 +612,17 @@ public function getActivitySubject(string $language, $subjectIdentifier, array $ } if ($subjectIdentifier === self::SUBJECT_IMPORT_FINISHED) { - return $ownActivity ? $l->t('You have imported file to table {table}') : $l->t('{user} has imported file to table {table}'); + $hasProblemsFile = isset($subjectParams['importProblemsFile']['id'], $subjectParams['importProblemsFile']['name'], $subjectParams['importProblemsFile']['path']); + if ($ownActivity) { + return $hasProblemsFile + ? $l->t('You have imported file to table {table} — problems report: {problemsFile}') + : $l->t('You have imported file to table {table}'); + } + return $l->t('{user} has imported file to table {table}'); + } + + if ($subjectIdentifier === self::SUBJECT_IMPORT_FAILED) { + return $ownActivity ? $l->t('Your import to table {table} has failed') : $l->t('{user}\'s import to table {table} has failed'); } return ''; @@ -787,11 +817,17 @@ private function isLinkShare(mixed $sharedWith): bool { return is_array($sharedWith) && ($sharedWith['type'] ?? null) === ShareReceiverType::LINK; } - public function getActivityMessage(string $language, $subjectIdentifier) { + /** + * @param array $subjectParams + */ + public function getActivityMessage(string $language, $subjectIdentifier, array $subjectParams = []) { $l = $this->l10nFactory->get(Application::APP_ID, $language); switch ($subjectIdentifier) { case self::SUBJECT_IMPORT_FINISHED: + if (!is_array($subjectParams['importStats'] ?? null)) { + return null; + } $lines = [ $l->t('Found columns: {foundColumnsCount}'), $l->t('Matching columns: {matchingColumnsCount}'), @@ -802,6 +838,11 @@ public function getActivityMessage(string $language, $subjectIdentifier) { $l->t('Row creation errors: {errorsCount}'), ]; return implode("\n", $lines); + case self::SUBJECT_IMPORT_FAILED: + $errorMessage = $subjectParams['errorMessage'] ?? null; + return is_string($errorMessage) && $errorMessage !== '' + ? $l->t('Error: %s', [$errorMessage]) + : null; default: return null; } diff --git a/lib/Activity/TablesProvider.php b/lib/Activity/TablesProvider.php index 324fc9d2c4..ad51749c18 100644 --- a/lib/Activity/TablesProvider.php +++ b/lib/Activity/TablesProvider.php @@ -41,6 +41,9 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null): $subjectIdentifier = $event->getSubject(); $subjectParams = $event->getSubjectParameters(); $ownActivity = ($event->getAuthor() === $this->userId); + if (!$ownActivity && in_array($subjectIdentifier, [ActivityManager::SUBJECT_IMPORT_FINISHED, ActivityManager::SUBJECT_IMPORT_FAILED], true)) { + unset($subjectParams['importStats'], $subjectParams['importProblemsFile'], $subjectParams['errorMessage']); + } /** * Map stored parameter objects to rich string types @@ -206,19 +209,29 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null): } $messageParameters = []; - if ($event->getSubject() === ActivityManager::SUBJECT_IMPORT_FINISHED) { - $messageParameters['{foundColumnsCount}'] = $subjectParams['importStats']['foundColumnsCount']; - $messageParameters['{matchingColumnsCount}'] = $subjectParams['importStats']['matchingColumnsCount']; - $messageParameters['{createdColumnsCount}'] = $subjectParams['importStats']['createdColumnsCount']; - $messageParameters['{insertedRowsCount}'] = $subjectParams['importStats']['insertedRowsCount']; - $messageParameters['{updatedRowsCount}'] = $subjectParams['importStats']['updatedRowsCount']; - $messageParameters['{errorsParsingCount}'] = $subjectParams['importStats']['errorsParsingCount']; - $messageParameters['{errorsCount}'] = $subjectParams['importStats']['errorsCount']; + if ($event->getSubject() === ActivityManager::SUBJECT_IMPORT_FINISHED && is_array($subjectParams['importStats'] ?? null)) { + $messageParameters['{foundColumnsCount}'] = (string)$subjectParams['importStats']['foundColumnsCount']; + $messageParameters['{matchingColumnsCount}'] = (string)$subjectParams['importStats']['matchingColumnsCount']; + $messageParameters['{createdColumnsCount}'] = (string)$subjectParams['importStats']['createdColumnsCount']; + $messageParameters['{insertedRowsCount}'] = (string)$subjectParams['importStats']['insertedRowsCount']; + $messageParameters['{updatedRowsCount}'] = (string)$subjectParams['importStats']['updatedRowsCount']; + $messageParameters['{errorsParsingCount}'] = (string)$subjectParams['importStats']['errorsParsingCount']; + $messageParameters['{errorsCount}'] = (string)$subjectParams['importStats']['errorsCount']; + + if (isset($subjectParams['importProblemsFile']['id'], $subjectParams['importProblemsFile']['name'], $subjectParams['importProblemsFile']['path'])) { + $subjectParameters['problemsFile'] = [ + 'type' => 'file', + 'id' => (string)$subjectParams['importProblemsFile']['id'], + 'name' => (string)$subjectParams['importProblemsFile']['name'], + 'path' => (string)$subjectParams['importProblemsFile']['path'], + 'link' => $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $subjectParams['importProblemsFile']['id']]), + ]; + } } try { $subject = $this->activityManager->getActivitySubject($language, $subjectIdentifier, $subjectParams, $ownActivity); - $message = $this->activityManager->getActivityMessage($language, $subjectIdentifier); + $message = $this->activityManager->getActivityMessage($language, $subjectIdentifier, $subjectParams); $this->parseEvent($event, $subject, $subjectParameters, $message, $messageParameters); } catch (\Exception $e) { $this->logger->warning('Could not parse activity: ' . $e->getMessage(), ['exception' => $e]); diff --git a/lib/BackgroundJob/ImportTableJob.php b/lib/BackgroundJob/ImportTableJob.php index 63ac73b063..d287ea0ae5 100644 --- a/lib/BackgroundJob/ImportTableJob.php +++ b/lib/BackgroundJob/ImportTableJob.php @@ -8,8 +8,10 @@ namespace OCA\Tables\BackgroundJob; use OCA\Tables\Activity\ActivityManager; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; +use OCA\Tables\Model\ImportProblem; use OCA\Tables\Notification\NotificationHelper; use OCA\Tables\Service\ImportService; use OCP\AppFramework\Db\DoesNotExistException; @@ -17,6 +19,7 @@ use OCP\BackgroundJob\QueuedJob; use OCP\IUserManager; use OCP\IUserSession; +use OCP\L10N\IFactory; use Psr\Log\LoggerInterface; class ImportTableJob extends QueuedJob { @@ -25,11 +28,11 @@ public function __construct( private readonly IUserManager $userManager, private readonly IUserSession $userSession, private readonly ImportService $importService, - private readonly ActivityManager $activityManager, private readonly TableMapper $tableMapper, private readonly ViewMapper $viewMapper, private readonly NotificationHelper $notificationHelper, private readonly LoggerInterface $logger, + private readonly IFactory $l10nFactory, ) { parent::__construct($time); } @@ -40,6 +43,7 @@ public function __construct( * view_id: integer|null, * user_file_path: string|null, * import_file_name: string|null, + * original_file_name?: string|null, * create_missing_columns: bool, * columns_config: array * } $argument @@ -64,6 +68,8 @@ public function run($argument): void { $oldUser = $this->userSession->getUser(); $importSuccess = false; + $importStats = null; + $importErrorMessage = null; try { $user = $this->userManager->get($userId); @@ -80,28 +86,28 @@ public function run($argument): void { $tableId, $viewId, $argument['create_missing_columns'], - $argument['columns_config'] + $argument['columns_config'], + $argument['original_file_name'] ?? $argument['import_file_name'] ?? null, + collectProblemsPreview: false, ); $importSuccess = true; - } catch (\Exception $e) { + } catch (\Throwable $e) { + $l10n = $this->l10nFactory->get(Application::APP_ID, $this->l10nFactory->getUserLanguage($this->userManager->get($userId))); + $importErrorMessage = ImportProblem::describeCode($l10n, ImportProblem::codeForException($e)); $this->logger->error('Import failed: ' . $e->getMessage(), ['exception' => $e]); } finally { $this->userSession->setUser($oldUser); } if ($importSuccess) { - $this->activityManager->triggerEvent( - objectType: ActivityManager::TABLES_OBJECT_TABLE, - object: $table, - subject: ActivityManager::SUBJECT_IMPORT_FINISHED, - additionalParams: [ - 'importStats' => $importStats, - ], - author: $userId - ); $notifySubject = ActivityManager::SUBJECT_IMPORT_FINISHED; + $notifyParams = [ + 'importStats' => $importStats->withoutProblems(), + 'importProblemsFile' => $this->importService->getStoredProblemsFile(), + ]; } else { $notifySubject = ActivityManager::SUBJECT_IMPORT_FAILED; + $notifyParams = ['errorMessage' => $importErrorMessage]; } if ($view !== null) { @@ -109,6 +115,7 @@ public function run($argument): void { objectType: ActivityManager::TABLES_OBJECT_VIEW, object: $view, subject: $notifySubject, + additionalParams: $notifyParams, author: $userId ); } else { @@ -116,6 +123,7 @@ public function run($argument): void { objectType: ActivityManager::TABLES_OBJECT_TABLE, object: $table, subject: $notifySubject, + additionalParams: $notifyParams, author: $userId ); } diff --git a/lib/Controller/Errors.php b/lib/Controller/Errors.php index a7fbb871f7..bafb9402e0 100644 --- a/lib/Controller/Errors.php +++ b/lib/Controller/Errors.php @@ -18,6 +18,11 @@ trait Errors { + /** + * @template T of array|null|object|scalar + * @param Closure(): T $callback + * @return DataResponse|DataResponse + */ protected function handleError(Closure $callback): DataResponse { try { return new DataResponse($callback()); diff --git a/lib/Controller/ImportController.php b/lib/Controller/ImportController.php index f5e655f349..ad11777070 100644 --- a/lib/Controller/ImportController.php +++ b/lib/Controller/ImportController.php @@ -7,13 +7,17 @@ namespace OCA\Tables\Controller; +use Closure; use OCA\Tables\AppInfo\Application; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\ImportProblem; +use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ImportService; use OCA\Tables\UploadException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\AppFramework\Http\DataResponse; use OCP\Files\NotPermittedException; @@ -22,6 +26,9 @@ use OCP\Util; use Psr\Log\LoggerInterface; +/** + * @psalm-import-type TablesImportV2Response from ResponseDefinitions + */ class ImportController extends Controller { public const MIME_TYPES = [ 'text/csv', @@ -33,18 +40,38 @@ class ImportController extends Controller { 'application/vnd.oasis.opendocument.spreadsheet', ]; - use Errors; - public function __construct( IRequest $request, protected LoggerInterface $logger, - private ImportService $service, - private string $userId, - private IL10N $l10n, + private readonly ImportService $service, + private readonly string $userId, + private readonly IL10N $l10n, ) { parent::__construct(Application::APP_ID, $request); } + /** + * @template T of array|null|object|scalar + * @param Closure(): T $callback + * @return DataResponse|DataResponse + */ + protected function handleError(Closure $callback): DataResponse { + try { + return new DataResponse($callback()); + } catch (\Throwable $e) { + $this->logger->error('Import request failed', ['exception' => $e]); + $code = ImportProblem::codeForException($e); + $status = match ($code) { + ImportProblem::CODE_RESOURCE_NOT_FOUND => Http::STATUS_NOT_FOUND, + ImportProblem::CODE_PERMISSION_DENIED => Http::STATUS_FORBIDDEN, + ImportProblem::CODE_FILE_READ_FAILED => Http::STATUS_BAD_REQUEST, + default => Http::STATUS_INTERNAL_SERVER_ERROR, + }; + $message = ImportProblem::describeCode($this->l10n, $code); + return new DataResponse(['message' => $message], $status); + } + } + #[NoAdminRequired] #[UserRateLimit(limit: 20, period: 60)] #[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] @@ -63,9 +90,29 @@ public function importInTable(int $tableId, String $path, bool $createMissingCol fn () => $this->service->import($tableId, null, $path, $createMissingColumns, $columnsConfig)); } + /** + * Import data into a table from a file in the user's storage + * + * Small files are imported synchronously and the result is returned + * in `result`. Larger files are queued as a background job and + * `async` is set to true; the import stats are then published via + * an activity entry once the job finished. + * + * @param int $tableId Table ID + * @param string $path Path to a file in the user's storage + * @param bool $createMissingColumns Create missing columns + * @param list> $columnsConfig Column configuration for the import + * @return DataResponse|DataResponse + * + * 200: Import finished or queued + * 400: Invalid request + * 403: No permissions + * 404: Not found + */ #[NoAdminRequired] #[UserRateLimit(limit: 20, period: 60)] #[RequirePermission(permission: Application::PERMISSION_CREATE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] + #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function importV2InTable(int $tableId, String $path, bool $createMissingColumns = true, array $columnsConfig = []): DataResponse { return $this->handleError(function () use ($tableId, $path, $createMissingColumns, $columnsConfig) { // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer @@ -74,7 +121,7 @@ public function importV2InTable(int $tableId, String $path, bool $createMissingC return ['async' => true]; } $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_USER_FILE, $path, $tableId, null, $createMissingColumns, $columnsConfig); - return ['async' => false, 'result' => $result]; + return ['async' => false, 'result' => $result->jsonSerialize()]; }); } @@ -96,9 +143,29 @@ public function importInView(int $viewId, String $path, bool $createMissingColum fn () => $this->service->import(null, $viewId, $path, $createMissingColumns, $columnsConfig)); } + /** + * Import data into a view from a file in the user's storage + * + * Small files are imported synchronously and the result is returned + * in `result`. Larger files are queued as a background job and + * `async` is set to true; the import stats are then published via + * an activity entry once the job finished. + * + * @param int $viewId View ID + * @param string $path Path to a file in the user's storage + * @param bool $createMissingColumns Create missing columns + * @param list> $columnsConfig Column configuration for the import + * @return DataResponse|DataResponse + * + * 200: Import finished or queued + * 400: Invalid request + * 403: No permissions + * 404: Not found + */ #[NoAdminRequired] #[UserRateLimit(limit: 20, period: 60)] #[RequirePermission(permission: Application::PERMISSION_CREATE, type: Application::NODE_TYPE_VIEW, idParam: 'viewId')] + #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function importV2InView(int $viewId, String $path, bool $createMissingColumns = true, array $columnsConfig = []): DataResponse { return $this->handleError(function () use ($viewId, $path, $createMissingColumns, $columnsConfig) { // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer @@ -107,7 +174,7 @@ public function importV2InView(int $viewId, String $path, bool $createMissingCol return ['async' => true]; } $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_USER_FILE, $path, null, $viewId, $createMissingColumns, $columnsConfig); - return ['async' => false, 'result' => $result]; + return ['async' => false, 'result' => $result->jsonSerialize()]; }); } @@ -119,8 +186,7 @@ public function previewUploadImportTable(int $tableId): DataResponse { $file = $this->getUploadedFile('uploadfile'); return $this->handleError(fn () => $this->service->previewImport($tableId, null, $file['tmp_name'])); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } @@ -137,8 +203,7 @@ public function importUploadInTable(int $tableId, bool $createMissingColumns = t // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer fn () => $this->service->import($tableId, null, $file['tmp_name'], $createMissingColumns, $columnsConfigArray)); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } @@ -150,8 +215,7 @@ public function previewUploadImportView(int $viewId): DataResponse { $file = $this->getUploadedFile('uploadfile'); return $this->handleError(fn () => $this->service->previewImport(null, $viewId, $file['tmp_name'])); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } @@ -165,15 +229,14 @@ public function importV2UploadInTable(int $tableId, bool $createMissingColumns = return $this->handleError(function () use ($tableId, $file, $createMissingColumns, $columnsConfigArray) { // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer if ($this->service->shouldImportAsync($file['tmp_name'])) { - $this->service->scheduleImport($tableId, null, $file['tmp_name'], $createMissingColumns, $columnsConfigArray); + $this->service->scheduleImport($tableId, null, $file['tmp_name'], $createMissingColumns, $columnsConfigArray, $file['name'] ?? null); return ['async' => true]; } - $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_UPLOADED_FILE, $file['tmp_name'], $tableId, null, $createMissingColumns, $columnsConfigArray); - return ['async' => false, 'result' => $result]; + $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_UPLOADED_FILE, $file['tmp_name'], $tableId, null, $createMissingColumns, $columnsConfigArray, $file['name'] ?? null); + return ['async' => false, 'result' => $result->jsonSerialize()]; }); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } @@ -187,8 +250,7 @@ public function importUploadInView(int $viewId, bool $createMissingColumns = tru // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer fn () => $this->service->import(null, $viewId, $file['tmp_name'], $createMissingColumns, $columnsConfigArray)); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } @@ -202,18 +264,28 @@ public function importV2UploadInView(int $viewId, bool $createMissingColumns = t return $this->handleError(function () use ($viewId, $file, $createMissingColumns, $columnsConfigArray) { // minimal permission is checked, creating columns requires MANAGE permissions - currently tested on service layer if ($this->service->shouldImportAsync($file['tmp_name'])) { - $this->service->scheduleImport(null, $viewId, $file['tmp_name'], $createMissingColumns, $columnsConfigArray); + $this->service->scheduleImport(null, $viewId, $file['tmp_name'], $createMissingColumns, $columnsConfigArray, $file['name'] ?? null); return ['async' => true]; } - $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_UPLOADED_FILE, $file['tmp_name'], null, $viewId, $createMissingColumns, $columnsConfigArray); - return ['async' => false, 'result' => $result]; + $result = $this->service->importV2($this->userId, ImportService::IMPORT_TYPE_UPLOADED_FILE, $file['tmp_name'], null, $viewId, $createMissingColumns, $columnsConfigArray, $file['name'] ?? null); + return ['async' => false, 'result' => $result->jsonSerialize()]; }); } catch (UploadException|NotPermittedException $e) { - $this->logger->error('Upload error', ['exception' => $e]); - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); + return $this->handleUploadError($e); } } + /** + * @return DataResponse + */ + private function handleUploadError(UploadException|NotPermittedException $exception): DataResponse { + $this->logger->error('Upload error', ['exception' => $exception]); + $message = $exception instanceof UploadException + ? $exception->getMessage() + : ImportProblem::describeCode($this->l10n, ImportProblem::CODE_PERMISSION_DENIED); + return new DataResponse(['message' => $message], Http::STATUS_BAD_REQUEST); + } + /** * @param string $key * @return array diff --git a/lib/Model/ImportProblem.php b/lib/Model/ImportProblem.php new file mode 100644 index 0000000000..de2dcbc570 --- /dev/null +++ b/lib/Model/ImportProblem.php @@ -0,0 +1,112 @@ + $this->code, + 'row' => $this->row, + 'column' => $this->column, + 'value' => $this->value, + 'description' => $this->description, + ]; + } + + public static function codeForException(Throwable $exception): string { + return match (true) { + $exception instanceof NotFoundError, $exception instanceof NotFoundException, $exception instanceof NoUserException => self::CODE_RESOURCE_NOT_FOUND, + $exception instanceof PermissionError, $exception instanceof NotPermittedException => self::CODE_PERMISSION_DENIED, + $exception instanceof BadRequestError, $exception instanceof InvalidArgumentException, $exception instanceof ReaderException => self::CODE_FILE_READ_FAILED, + default => self::CODE_IMPORT_FAILED, + }; + } + + /** + * Render a single-line description of a problem, including the offending + * value where it helps identify the problem. + */ + public static function describeProblem(IL10N $l, string $code, ?string $value): string { + if ($value === null || $value === '') { + return self::describeCode($l, $code); + } + return match ($code) { + self::CODE_ID_NOT_NUMERIC => $l->t('The ID value "%s" is not numeric', [$value]), + self::CODE_PARSE_FAILED => $l->t('The value "%s" could not be parsed', [$value]), + default => self::describeCode($l, $code), + }; + } + + /** + * Render the reason for a problem code, without the row/column location. + */ + public static function describeCode(IL10N $l, string $code): string { + return match ($code) { + self::CODE_EMPTY_COLUMN_TITLE => $l->t('Column title is empty'), + self::CODE_COLUMN_TITLE_GAP => $l->t('There is a gap in the column titles'), + self::CODE_NO_COLUMNS_FOUND => $l->t('No matching columns were found'), + self::CODE_ID_NOT_NUMERIC => $l->t('The ID value is not numeric'), + self::CODE_MISSING_MANDATORY => $l->t('A mandatory value is missing'), + self::CODE_PARSE_FAILED => $l->t('The value could not be parsed'), + self::CODE_ROW_INSERT_FAILED => $l->t('The row could not be created'), + self::CODE_ROW_UPDATE_FAILED => $l->t('The row could not be updated'), + self::CODE_PERMISSION_DENIED => $l->t('Permission denied'), + self::CODE_RESOURCE_NOT_FOUND => $l->t('A file, table, view, or row needed for the import could not be found.'), + self::CODE_FILE_READ_FAILED => $l->t('The import file could not be read. Check its format and contents.'), + self::CODE_IMPORT_FAILED => $l->t('An unexpected error occurred during the import. Please contact your administrator.'), + default => $l->t('An unknown problem occurred'), + }; + } +} diff --git a/lib/Model/ImportProblemsFile.php b/lib/Model/ImportProblemsFile.php new file mode 100644 index 0000000000..8bf0db2ae7 --- /dev/null +++ b/lib/Model/ImportProblemsFile.php @@ -0,0 +1,37 @@ + $this->id, + 'name' => $this->name, + 'path' => $this->path, + ]; + } +} diff --git a/lib/Model/ImportStats.php b/lib/Model/ImportStats.php index 5c7fa296d3..835f1d3923 100644 --- a/lib/Model/ImportStats.php +++ b/lib/Model/ImportStats.php @@ -7,7 +7,16 @@ namespace OCA\Tables\Model; -final class ImportStats { +use JsonSerializable; +use OCA\Tables\ResponseDefinitions; + +/** + * @psalm-import-type TablesImportStats from ResponseDefinitions + */ +final class ImportStats implements JsonSerializable { + /** + * @param list $problems + */ public function __construct( public int $foundColumnsCount, public int $matchingColumnsCount, @@ -16,6 +25,35 @@ public function __construct( public int $updatedRowsCount, public int $errorsParsingCount, public int $errorsCount, + public array $problems = [], + public ?ImportProblemsFile $problemsFile = null, ) { } + + /** + * Copy without the problems list, for activity/notification parameters + * where only the counters are rendered. + */ + public function withoutProblems(): self { + $clone = clone $this; + $clone->problems = []; + return $clone; + } + + /** + * @return TablesImportStats + */ + public function jsonSerialize(): array { + return [ + 'foundColumnsCount' => $this->foundColumnsCount, + 'matchingColumnsCount' => $this->matchingColumnsCount, + 'createdColumnsCount' => $this->createdColumnsCount, + 'insertedRowsCount' => $this->insertedRowsCount, + 'updatedRowsCount' => $this->updatedRowsCount, + 'errorsParsingCount' => $this->errorsParsingCount, + 'errorsCount' => $this->errorsCount, + 'problems' => array_map(static fn (ImportProblem $problem): array => $problem->jsonSerialize(), $this->problems), + 'problemsFile' => $this->problemsFile?->jsonSerialize(), + ]; + } } diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index 63214c74d1..145f9b04c3 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -53,12 +53,12 @@ public function sendNotification(string $objectType, Table|View|Row2|Column $obj switch ($objectType) { case ActivityManager::TABLES_OBJECT_TABLE: if ($object instanceof Table) { - $this->sendTableNotification($object, $subject, $author); + $this->sendTableNotification($object, $subject, $additionalParams, $author); } break; case ActivityManager::TABLES_OBJECT_VIEW: if ($object instanceof View) { - $this->sendViewNotification($object, $subject, $author); + $this->sendViewNotification($object, $subject, $additionalParams, $author); } break; case ActivityManager::TABLES_OBJECT_ROW: @@ -82,7 +82,10 @@ public function sendNotification(string $objectType, Table|View|Row2|Column $obj } } - private function sendTableNotification(Table $object, string $subject, ?string $author): void { + /** + * @param array $additionalParams + */ + private function sendTableNotification(Table $object, string $subject, array $additionalParams, ?string $author): void { $subjectParams = [ 'author' => $author, 'objectType' => ActivityManager::TABLES_OBJECT_TABLE, @@ -95,7 +98,7 @@ private function sendTableNotification(Table $object, string $subject, ?string $ if ($subject === ActivityManager::SUBJECT_IMPORT_FINISHED || $subject === ActivityManager::SUBJECT_IMPORT_FAILED) { $notification = $this->generateNotification( subject: $subject, - subjectParams: $subjectParams, + subjectParams: array_merge($subjectParams, $additionalParams), objectType: ActivityManager::TABLES_OBJECT_TABLE, objectId: (string)$object->getId(), receiver: $author, @@ -115,7 +118,10 @@ private function sendTableNotification(Table $object, string $subject, ?string $ ); } - private function sendViewNotification(View $object, string $subject, ?string $author): void { + /** + * @param array $additionalParams + */ + private function sendViewNotification(View $object, string $subject, array $additionalParams, ?string $author): void { $subjectParams = [ 'author' => $author, 'objectType' => ActivityManager::TABLES_OBJECT_VIEW, @@ -134,7 +140,7 @@ private function sendViewNotification(View $object, string $subject, ?string $au $notification = $this->generateNotification( subject: $subject, - subjectParams: $subjectParams, + subjectParams: array_merge($subjectParams, $additionalParams), objectType: ActivityManager::TABLES_OBJECT_VIEW, objectId: (string)$object->getId(), receiver: $author, diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 74758258cb..84d7dc6d7a 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -11,6 +11,7 @@ use OCA\Tables\Activity\ActivityManager; use OCA\Tables\AppInfo\Application; use OCA\Tables\Service\PermissionsService; +use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\L10N\IFactory; @@ -363,9 +364,58 @@ public function prepare(INotification $notification, string $languageCode): INot ->setRichSubject($subject, $richParams) ->setLink($link); + $this->addImportDetailsMessage($notification, $l, $params); + return $notification; } + /** + * @param array $params + */ + private function addImportDetailsMessage(INotification $notification, IL10N $l, array $params): void { + if ($notification->getSubject() === ActivityManager::SUBJECT_IMPORT_FINISHED) { + $stats = $params['importStats'] ?? null; + if (!is_array($stats)) { + return; + } + $problemCount = (int)($stats['errorsCount'] ?? 0) + (int)($stats['errorsParsingCount'] ?? 0); + if ($problemCount <= 0) { + return; + } + $lines = [$l->n( + '%n problem occurred during the import', + '%n problems occurred during the import', + $problemCount, + )]; + $report = $params['importProblemsFile'] ?? null; + if (is_array($report) && isset($report['id'], $report['name'], $report['path'])) { + $lines[] = $l->t('Problems report: {file}'); + $message = implode("\n", $lines); + $notification->setParsedMessage(str_replace('{file}', (string)$report['name'], $message)); + $notification->setRichMessage($message, [ + 'file' => [ + 'type' => 'file', + 'id' => (string)$report['id'], + 'name' => (string)$report['name'], + 'path' => (string)$report['path'], + 'link' => $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $report['id']]), + ], + ]); + return; + } + + $notification->setParsedMessage(implode("\n", $lines)); + return; + } + + if ($notification->getSubject() === ActivityManager::SUBJECT_IMPORT_FAILED) { + $errorMessage = $params['errorMessage'] ?? null; + if (is_string($errorMessage) && $errorMessage !== '') { + $notification->setParsedMessage($l->t('Error: %s', [$errorMessage])); + } + } + } + private function tablesUrl(string $endpoint): string { return $this->url->linkToRouteAbsolute('tables.page.index') . '#/' . trim($endpoint, '/'); } diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 194aa0a24a..25db089336 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -228,6 +228,31 @@ * errors_count: int, * } * + * @psalm-type TablesImportProblem = array{ + * code: string, + * row: ?int, + * column: ?string, + * value: ?string, + * description: ?string, + * } + * + * @psalm-type TablesImportStats = array{ + * foundColumnsCount: int, + * matchingColumnsCount: int, + * createdColumnsCount: int, + * insertedRowsCount: int, + * updatedRowsCount: int, + * errorsParsingCount: int, + * errorsCount: int, + * problems: list, + * problemsFile: ?array{id: int, name: string, path: string}, + * } + * + * @psalm-type TablesImportV2Response = array{ + * async: bool, + * result?: TablesImportStats, + * } + * * @psalm-type TablesContext = array{ * id: int, * name: string, diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index fbc0054832..799f3f4fbc 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -11,6 +11,7 @@ use InvalidArgumentException; use LogicException; use OC\User\NoUserException; +use OCA\Tables\Activity\ActivityManager; use OCA\Tables\AppInfo\Application; use OCA\Tables\AuditLog\Operation; use OCA\Tables\BackgroundJob\ImportTableJob; @@ -21,6 +22,8 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; use OCA\Tables\Helper\ColumnsHelper; +use OCA\Tables\Model\ImportProblem; +use OCA\Tables\Model\ImportProblemsFile; use OCA\Tables\Model\ImportStats; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; use OCA\Tables\Service\Support\AuditLogServiceInterface; @@ -37,22 +40,27 @@ use OCP\DB\Exception; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\IAppData; use OCP\Files\IFilenameValidator; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\IL10N; use OCP\ITempManager; use OCP\IUserManager; +use OCP\L10N\IFactory; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; +use SplFileObject; use TypeError; use function file_exists; use function is_string; use function is_uploaded_file; use function mb_strlen; +use function mb_substr; use function preg_match; class ImportService extends SuperService { @@ -73,17 +81,30 @@ class ImportService extends SuperService { private int $countUpdatedRows = 0; private int $countErrors = 0; private int $countParsingErrors = 0; + private int $problemCount = 0; + /** + * @var list + */ + private array $problems = []; + private bool $collectProblemsPreview = false; private array $rawColumnTitles = []; private array $rawColumnDataTypes = []; private array $columnsConfig = []; + private ?SplFileObject $problemsFileStream = null; + private ?ImportProblemsFile $storedProblemsFile = null; + private ?IL10N $l10n = null; + public const IMPORT_TYPE_USER_FILE = 'user_file'; public const IMPORT_TYPE_APP_FILE = 'app_file'; public const IMPORT_TYPE_UPLOADED_FILE = 'uploaded_file'; private const MAX_ROWS_FOR_IMMEDIATE_IMPORT = 200; private const MAX_COLUMNS_FOR_IMMEDIATE_IMPORT = 20; + private const MAX_PROBLEM_VALUE_LENGTH = 50; + private const MAX_PROBLEMS_PREVIEW = 50; + private const IMPORTS_FOLDER = 'Tables/Imports'; public function __construct( PermissionsService $permissionsService, @@ -101,12 +122,15 @@ public function __construct( private readonly IFilenameValidator $filenameValidator, IAppDataFactory $appDataFactory, private readonly AuditLogServiceInterface $auditLogService, + private readonly IFactory $l10nFactory, + private readonly ActivityManager $activityManager, ) { parent::__construct($logger, $userId, $permissionsService); $this->appData = $appDataFactory->get(Application::APP_ID); } public function previewImport(?int $tableId, ?int $viewId, string $path): array { + $this->resetImportState(); if ($viewId !== null) { $this->viewId = $viewId; } elseif ($tableId) { @@ -315,6 +339,7 @@ private function getPreviewData(Worksheet $worksheet): array { * @throws PermissionError */ public function import(?int $tableId, ?int $viewId, string $path, bool $createMissingColumns = true, array $columnsConfig = []): array { + $this->resetImportState(); if ($viewId !== null) { $view = $this->viewService->find($viewId); if (!$this->permissionsService->canCreateRows($view)) { @@ -399,13 +424,15 @@ public function import(?int $tableId, ?int $viewId, string $path, bool $createMi * @param ?int $viewId * @param string $path * @param bool $createMissingColumns + * @param array $columnsConfig + * @param ?string $originalFileName Original client side name of an uploaded file * @throws DoesNotExistException * @throws InternalError * @throws MultipleObjectsReturnedException * @throws NotFoundError * @throws PermissionError */ - public function scheduleImport(?int $tableId, ?int $viewId, string $path, bool $createMissingColumns = true, array $columnsConfig = []): void { + public function scheduleImport(?int $tableId, ?int $viewId, string $path, bool $createMissingColumns = true, array $columnsConfig = [], ?string $originalFileName = null): void { if ($viewId !== null) { $view = $this->viewService->find($viewId); if (!$this->permissionsService->canCreateRows($view)) { @@ -444,19 +471,18 @@ public function scheduleImport(?int $tableId, ?int $viewId, string $path, bool $ } $importFileName = null; + // uploaded file: copy it into app data so the background job + // can read it later without adding it to the user's files if (is_uploaded_file($path) && file_exists($path)) { $content = \file_get_contents($path); if ($content === false) { throw new InvalidArgumentException('Cannot read import payload'); } - $importFileName = 'import_u-' . $this->userId; - if ($this->tableId) { - $importFileName .= '_t-' . $this->tableId; + if ($originalFileName === null || !$this->filenameValidator->isFilenameValid($originalFileName)) { + $originalFileName = 'import-' . bin2hex(random_bytes(8)); } - if ($this->viewId) { - $importFileName .= '_v-' . $this->viewId; - } - $importFileName .= '_' . \date('Y-m-d_H-i-s') . '.' . pathinfo($path, PATHINFO_EXTENSION); + $extension = pathinfo($originalFileName, PATHINFO_EXTENSION); + $importFileName = 'import-' . bin2hex(random_bytes(16)) . ($extension !== '' ? '.' . $extension : ''); $importFileName = $this->filenameValidator->sanitizeFilename($importFileName); $this->getImportAppDataDir() @@ -480,8 +506,9 @@ public function scheduleImport(?int $tableId, ?int $viewId, string $path, bool $ 'user_id' => $this->userId, 'table_id' => $this->tableId, 'view_id' => $this->viewId, - 'user_file_path' => $importFileName ? null : $path, + 'user_file_path' => $importFileName === null ? $path : null, 'import_file_name' => $importFileName, + 'original_file_name' => $originalFileName, 'create_missing_columns' => $createMissingColumns, 'columns_config' => $columnsConfig, ] @@ -498,17 +525,28 @@ public function scheduleImport(?int $tableId, ?int $viewId, string $path, bool $ * or Path to uploaded file * @param bool $createMissingColumns * @param array $columnsConfig + * @param ?string $originalFileName Original client side name of an uploaded file, used for the issue report name + * @param bool $collectProblemsPreview Collect a bounded preview for synchronous imports * @return ImportStats * @throws DoesNotExistException * @throws MultipleObjectsReturnedException * @throws NotFoundError */ - public function importV2(string $userId, string $importType, string $path, ?int $tableId, ?int $viewId, bool $createMissingColumns = true, array $columnsConfig = []): ImportStats { + public function importV2(string $userId, string $importType, string $path, ?int $tableId, ?int $viewId, bool $createMissingColumns = true, array $columnsConfig = [], ?string $originalFileName = null, bool $collectProblemsPreview = true): ImportStats { + $this->resetImportState(); $this->userId = $userId; + $l10n = $this->l10nFactory->get( + Application::APP_ID, + $this->l10nFactory->getUserLanguage($this->userManager->get($userId)), + ); + $this->l10n = $l10n; + $this->openProblemsFileStream(); $this->tableId = $tableId; $this->viewId = $viewId; $this->createUnknownColumns = $createMissingColumns; $this->columnsConfig = $columnsConfig; + $this->collectProblemsPreview = $collectProblemsPreview; + $sourcePath = $importType === self::IMPORT_TYPE_USER_FILE ? $path : $originalFileName; try { if ($importType === self::IMPORT_TYPE_USER_FILE) { @@ -531,12 +569,21 @@ public function importV2(string $userId, string $importType, string $path, ?int } else { throw new NotFoundError('No file for import given.'); } - } catch (NotFoundException|NotPermittedException|NoUserException|InternalError|PermissionError $e) { - $this->logger->warning('Storage for user could not be found', ['exception' => $e]); - throw new NotFoundError('Storage for user could not be found', 0, $e); + $this->saveProblemsFile($userId, $sourcePath); + } catch (\Throwable $e) { + $this->finishImport($userId, errorMessage: ImportProblem::describeCode($l10n, ImportProblem::codeForException($e))); + throw $e; + } finally { + $this->problemsFileStream = null; } $this->logImportCompleted(); + $stats = $this->getImportStats(); + $this->finishImport($userId, $stats); + return $stats; + } + + private function getImportStats(): ImportStats { return new ImportStats( count(array_filter($this->columns, fn ($column): bool => $column instanceof Column)), $this->countMatchingColumns, @@ -544,10 +591,233 @@ public function importV2(string $userId, string $importType, string $path, ?int $this->countInsertedRows, $this->countUpdatedRows, $this->countParsingErrors, - $this->countErrors + $this->countErrors, + $this->problems, + $this->storedProblemsFile, + ); + } + + private function saveProblemsFile(string $userId, ?string $sourcePath): void { + try { + $this->storedProblemsFile = $this->storeProblemsFile($userId, $sourcePath); + } catch (\Throwable $e) { + $this->logger->warning('Could not store the import problems file', ['exception' => $e]); + } + } + + private function finishImport(string $userId, ?ImportStats $stats = null, ?string $errorMessage = null): void { + try { + if ($this->viewId !== null) { + $element = $this->tableService->find($this->viewService->find($this->viewId, true, $userId)->getTableId(), true, $userId); + } elseif ($this->tableId !== null) { + $element = $this->tableService->find($this->tableId, true, $userId); + } else { + return; + } + $this->activityManager->triggerEvent( + objectType: ActivityManager::TABLES_OBJECT_TABLE, + object: $element, + subject: $errorMessage === null ? ActivityManager::SUBJECT_IMPORT_FINISHED : ActivityManager::SUBJECT_IMPORT_FAILED, + additionalParams: $stats === null + ? ['errorMessage' => $errorMessage] + : [ + 'importStats' => $stats->withoutProblems(), + 'importProblemsFile' => $this->storedProblemsFile, + ], + author: $userId, + ); + } catch (\Throwable $e) { + $this->logger->warning('Could not create the import activity entry: ' . $e->getMessage(), ['exception' => $e]); + } + } + + /** + * File information of the problems file stored by the last import, if any. + * Used by the background job to link the file in notifications. + */ + public function getStoredProblemsFile(): ?ImportProblemsFile { + return $this->storedProblemsFile; + } + + private function resetImportState(): void { + $this->tableId = null; + $this->viewId = null; + $this->columns = []; + $this->createUnknownColumns = true; + $this->idColumnIndex = null; + $this->countMatchingColumns = 0; + $this->countCreatedColumns = 0; + $this->countInsertedRows = 0; + $this->countUpdatedRows = 0; + $this->countErrors = 0; + $this->countParsingErrors = 0; + $this->problemCount = 0; + $this->problems = []; + $this->collectProblemsPreview = false; + $this->rawColumnTitles = []; + $this->rawColumnDataTypes = []; + $this->columnsConfig = []; + $this->problemsFileStream = null; + $this->storedProblemsFile = null; + $this->l10n = null; + } + + private function openProblemsFileStream(): void { + if ($this->l10n === null) { + return; + } + try { + $tempPath = $this->tempManager->getTemporaryFile('.csv'); + if ($tempPath === false) { + $this->logger->warning('Could not create a temporary file for the import problems file'); + return; + } + $this->problemsFileStream = new SplFileObject($tempPath, 'w+'); + // UTF-8 BOM so spreadsheet apps detect the encoding + $this->problemsFileStream->fwrite("\xEF\xBB\xBF"); + $this->writeProblemRecord([ + $this->l10n->t('Row'), + $this->l10n->t('Column'), + $this->l10n->t('Code'), + $this->l10n->t('Value'), + $this->l10n->t('Problem'), + ]); + } catch (\Throwable $e) { + $this->problemsFileStream = null; + $this->logger->warning('Could not create the import problems file', ['exception' => $e]); + } + } + + /** + * Persist the collected problems file to the user's storage. + * Returns file information for activity/notification parameters or null + * when no problems were recorded. + */ + public function storeProblemsFile(string $userId, ?string $sourcePath): ?ImportProblemsFile { + if ($this->problemsFileStream === null || $this->problemCount === 0) { + return null; + } + + $importsFolder = $this->getImportsFolder($userId); + if ($importsFolder === null) { + return null; + } + + $baseName = $sourcePath !== null ? (pathinfo($sourcePath, PATHINFO_FILENAME) ?: 'import') : 'import'; + + try { + $problemsFileName = $baseName . '.import-problems-' . date('Ymd-His') . '.csv'; + if (!$this->filenameValidator->isFilenameValid($problemsFileName)) { + $problemsFileName = 'import-problems-' . bin2hex(random_bytes(8)) . '.csv'; + } + $problemsFileName = $importsFolder->getNonExistingName($problemsFileName); + $this->problemsFileStream->fflush(); + $reportStream = fopen($this->problemsFileStream->getPathname(), 'rb'); + if ($reportStream === false) { + $this->logger->warning('Could not open the import problems file for storage'); + return null; + } + try { + $problemsFile = $importsFolder->newFile($problemsFileName); + $problemsFile->putContent($reportStream); + } finally { + if (is_resource($reportStream)) { + fclose($reportStream); + } + } + } catch (NotPermittedException $e) { + $this->logger->warning('Could not store the import problems file', ['exception' => $e]); + return null; + } + + try { + $userFolder = $this->rootFolder->getUserFolder($userId); + $relativePath = $userFolder->getRelativePath($problemsFile->getPath()); + } catch (NotPermittedException|NoUserException) { + $relativePath = null; + } + + return new ImportProblemsFile( + $problemsFile->getId(), + $problemsFile->getName(), + ltrim($relativePath ?? '', '/'), ); } + /** + * Get (and lazily create) the folder used for generated import + * issue reports in the user's storage. + */ + private function getImportsFolder(string $userId): ?Folder { + try { + $dir = $this->rootFolder->getUserFolder($userId); + } catch (NotPermittedException|NoUserException $e) { + $this->logger->warning('Could not open user folder for the import', ['exception' => $e]); + return null; + } + + foreach (explode('/', self::IMPORTS_FOLDER) as $part) { + try { + $child = $dir->get($part); + } catch (NotFoundException) { + try { + $child = $dir->newFolder($part); + } catch (NotPermittedException $e) { + $this->logger->warning('Could not create the imports folder', ['exception' => $e]); + return null; + } + } + if (!$child instanceof Folder) { + $this->logger->warning('Imports folder path is occupied by a file: ' . $child->getPath()); + return null; + } + $dir = $child; + } + + return $dir; + } + + private function addProblem(string $code, ?int $row = null, ?string $column = null, ?string $value = null): void { + $this->problemCount++; + if ($this->l10n === null) { + return; + } + if ($this->collectProblemsPreview && count($this->problems) < self::MAX_PROBLEMS_PREVIEW) { + $previewValue = $value; + if ($previewValue !== null && mb_strlen($previewValue) > self::MAX_PROBLEM_VALUE_LENGTH) { + $previewValue = mb_substr($previewValue, 0, self::MAX_PROBLEM_VALUE_LENGTH) . '…'; + } + $this->problems[] = new ImportProblem($code, $row, $column, $previewValue, ImportProblem::describeProblem($this->l10n, $code, $previewValue)); + } + $this->writeProblemRecord([ + $row === null ? '' : (string)$row, + $column ?? '', + $code, + $value ?? '', + ImportProblem::describeCode($this->l10n, $code), + ]); + } + + /** + * @param list $fields + */ + private function writeProblemRecord(array $fields): void { + try { + $result = $this->problemsFileStream?->fputcsv(array_map(self::escapeCsvCell(...), $fields), ',', '"', ''); + if ($result === false) { + throw new \RuntimeException('Could not write an import problem record'); + } + } catch (\Throwable $e) { + $this->logger->warning('Could not write to the import problems file', ['exception' => $e]); + $this->problemsFileStream = null; + } + } + + private static function escapeCsvCell(string $value): string { + // CSV quoting alone does not prevent spreadsheet formula evaluation. + return preg_match('/^(?:[\x00-\x20]*[=+\-@]|\t|\r|\n)/', $value) === 1 ? "'" . $value : $value; + } + /** * @param Worksheet $worksheet * @throws DoesNotExistException @@ -568,6 +838,8 @@ private function loop(Worksheet $worksheet): void { $this->getColumns($firstRow, $secondRow); if (empty(array_filter($this->columns))) { + $this->countErrors++; + $this->addProblem(ImportProblem::CODE_NO_COLUMNS_FOUND); return; } @@ -588,11 +860,12 @@ private function loop(Worksheet $worksheet): void { /** * @return false|string */ - private function parseValueByColumnType(string $value, Column $column, IColumnTypeBusiness $columnBusiness): string|false { + private function parseValueByColumnType(string $value, Column $column, IColumnTypeBusiness $columnBusiness, int $rowIndex): string|false { try { if (!$columnBusiness->canBeParsedDisplayValue($value, $column)) { $this->logger->warning('Value ' . $value . ' could not be parsed for column ' . $column->getTitle()); $this->countParsingErrors++; + $this->addProblem(ImportProblem::CODE_PARSE_FAILED, $rowIndex, $column->getTitle(), $value); return ''; } return $columnBusiness->parseDisplayValue($value, $column); @@ -615,11 +888,11 @@ private function upsertRow(Row $row, array $columnBusinesses): void { $cellIterator = $row->getCellIterator(); $cellIterator->setIterateOnlyExistingCells(false); + $rowId = null; try { $i = -1; $data = []; $hasData = false; - $rowId = null; foreach ($cellIterator as $cell) { $i++; @@ -631,6 +904,7 @@ private function upsertRow(Row $row, array $columnBusinesses): void { if ($rowId !== null && !is_numeric($rowId)) { $this->logger->warning('ID column value is not numeric: ' . $rowId); $this->countErrors++; + $this->addProblem(ImportProblem::CODE_ID_NOT_NUMERIC, $row->getRowIndex(), null, (string)$rowId); return; } $rowId = (int)$rowId; @@ -652,6 +926,7 @@ private function upsertRow(Row $row, array $columnBusinesses): void { if ($column->getMandatory()) { $this->logger->warning('Mandatory column "' . $column->getTitle() . '" was not set'); $this->countErrors++; + $this->addProblem(ImportProblem::CODE_MISSING_MANDATORY, $row->getRowIndex(), $column->getTitle()); return; } continue; @@ -681,7 +956,7 @@ private function upsertRow(Row $row, array $columnBusinesses): void { $data[] = [ 'columnId' => $column->getId(), - 'value' => json_decode($this->parseValueByColumnType($value, $column, $columnBusinesses[$column->getId()])), + 'value' => json_decode($this->parseValueByColumnType($value, $column, $columnBusinesses[$column->getId()], $row->getRowIndex())), ]; } @@ -700,14 +975,17 @@ private function upsertRow(Row $row, array $columnBusinesses): void { } catch (PermissionError $e) { $this->logger->error('Could not create/update row while importing, no permission.', ['exception' => $e]); $this->countErrors++; + $this->addProblem(ImportProblem::CODE_PERMISSION_DENIED, $row->getRowIndex()); } catch (BadRequestError|InternalError $e) { $this->logger->error('Error while creating/updating new row for import.', ['exception' => $e]); $this->countErrors++; + $this->addProblem($rowId ? ImportProblem::CODE_ROW_UPDATE_FAILED : ImportProblem::CODE_ROW_INSERT_FAILED, $row->getRowIndex()); } catch (NotFoundError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new NotFoundError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), 0, $e); } catch (\Throwable $e) { $this->countErrors++; + $this->addProblem($rowId ? ImportProblem::CODE_ROW_UPDATE_FAILED : ImportProblem::CODE_ROW_INSERT_FAILED, $row->getRowIndex()); $this->logger->error('Error while creating/updating new row for import.', ['exception' => $e]); } } @@ -835,6 +1113,7 @@ private function getColumns(Row $firstRow, Row $secondRow): void { continue; } $this->countErrors++; + $this->addProblem(ImportProblem::CODE_EMPTY_COLUMN_TITLE, 1, $cell ? $cell->getColumn() : Coordinate::stringFromColumnIndex($index + 1)); } $secondRowCellIterator->next(); $index++; @@ -843,6 +1122,7 @@ private function getColumns(Row $firstRow, Row $secondRow): void { if ($hasGapInTitles) { $this->logger->info('Imported table is having a gap in column titles'); $this->countErrors++; + $this->addProblem(ImportProblem::CODE_COLUMN_TITLE_GAP, 1); } $this->rawColumnTitles = $rawColumnTitles; diff --git a/openapi.json b/openapi.json index c140ed6423..e7c46e970f 100644 --- a/openapi.json +++ b/openapi.json @@ -330,6 +330,38 @@ } } }, + "ImportProblem": { + "type": "object", + "required": [ + "code", + "row", + "column", + "value", + "description" + ], + "properties": { + "code": { + "type": "string" + }, + "row": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "column": { + "type": "string", + "nullable": true + }, + "value": { + "type": "string", + "nullable": true + }, + "description": { + "type": "string", + "nullable": true + } + } + }, "ImportState": { "type": "object", "required": [ @@ -372,6 +404,91 @@ } } }, + "ImportStats": { + "type": "object", + "required": [ + "foundColumnsCount", + "matchingColumnsCount", + "createdColumnsCount", + "insertedRowsCount", + "updatedRowsCount", + "errorsParsingCount", + "errorsCount", + "problems", + "problemsFile" + ], + "properties": { + "foundColumnsCount": { + "type": "integer", + "format": "int64" + }, + "matchingColumnsCount": { + "type": "integer", + "format": "int64" + }, + "createdColumnsCount": { + "type": "integer", + "format": "int64" + }, + "insertedRowsCount": { + "type": "integer", + "format": "int64" + }, + "updatedRowsCount": { + "type": "integer", + "format": "int64" + }, + "errorsParsingCount": { + "type": "integer", + "format": "int64" + }, + "errorsCount": { + "type": "integer", + "format": "int64" + }, + "problems": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ImportProblem" + } + }, + "problemsFile": { + "type": "object", + "nullable": true, + "required": [ + "id", + "name", + "path" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "name": { + "type": "string" + }, + "path": { + "type": "string" + } + } + } + } + }, + "ImportV2Response": { + "type": "object", + "required": [ + "async" + ], + "properties": { + "async": { + "type": "boolean" + }, + "result": { + "$ref": "#/components/schemas/ImportStats" + } + } + }, "Index": { "type": "object", "required": [ @@ -6610,6 +6727,360 @@ } } }, + "/index.php/apps/tables/v2/import/table/{tableId}": { + "post": { + "operationId": "import-import-v2-in-table", + "summary": "Import data into a table from a file in the user's storage", + "description": "Small files are imported synchronously and the result is returned in `result`. Larger files are queued as a background job and `async` is set to true; the import stats are then published via an activity entry once the job finished.", + "tags": [ + "import" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "path" + ], + "properties": { + "path": { + "type": "string", + "description": "Path to a file in the user's storage" + }, + "createMissingColumns": { + "type": "boolean", + "default": true, + "description": "Create missing columns" + }, + "columnsConfig": { + "type": "array", + "default": [], + "description": "Column configuration for the import", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } + } + } + } + } + } + }, + "parameters": [ + { + "name": "tableId", + "in": "path", + "description": "Table ID", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Import finished or queued", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ImportV2Response" + } + } + } + }, + "400": { + "description": "Invalid request", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "403": { + "description": "No permissions", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "500": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "404": { + "description": "Not found", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + }, + "/index.php/apps/tables/v2/import/view/{viewId}": { + "post": { + "operationId": "import-import-v2-in-view", + "summary": "Import data into a view from a file in the user's storage", + "description": "Small files are imported synchronously and the result is returned in `result`. Larger files are queued as a background job and `async` is set to true; the import stats are then published via an activity entry once the job finished.", + "tags": [ + "import" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "path" + ], + "properties": { + "path": { + "type": "string", + "description": "Path to a file in the user's storage" + }, + "createMissingColumns": { + "type": "boolean", + "default": true, + "description": "Create missing columns" + }, + "columnsConfig": { + "type": "array", + "default": [], + "description": "Column configuration for the import", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } + } + } + } + } + } + }, + "parameters": [ + { + "name": "viewId", + "in": "path", + "description": "View ID", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Import finished or queued", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ImportV2Response" + } + } + } + }, + "400": { + "description": "Invalid request", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "403": { + "description": "No permissions", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "500": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "404": { + "description": "Not found", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/tables/api/2/init": { "get": { "operationId": "api_general-index", diff --git a/src/modules/modals/FileActionImport.vue b/src/modules/modals/FileActionImport.vue index 46c970b230..9584479932 100644 --- a/src/modules/modals/FileActionImport.vue +++ b/src/modules/modals/FileActionImport.vue @@ -8,6 +8,9 @@ size="normal" @closing="closeImportDialog">