From 9a9528227bdc1ea08d4d747ae766a9f351e0da4a Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:44:24 +0000 Subject: [PATCH 1/3] fix: count only the files the current import brought An import that was interrupted hard leaves nb_imported_files behind, and the number of imported files reported when the next one finishes was added on top of it. The size counter next to it was already reset at the start. Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 1 + lib/Service/OnedriveStorageAPIService.php | 1 + .../Service/OnedriveStorageAPIServiceTest.php | 29 +++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab28c6..7dc015b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Report files that could not be downloaded in the import finished notification, with their names, which also no longer counts them as imported - Mention files that were already there in the import finished notification, so re-running an import does not look like a failure - Count imported empty files as imported +- Report the number of files the current import brought, a counter left over from an interrupted import is no longer added to it - Test the import against a stubbed Graph API, including failed downloads, empty files and paging ## [3.5.2] - 2026-07-28 diff --git a/lib/Service/OnedriveStorageAPIService.php b/lib/Service/OnedriveStorageAPIService.php index 6a8b21f..956f7d7 100644 --- a/lib/Service/OnedriveStorageAPIService.php +++ b/lib/Service/OnedriveStorageAPIService.php @@ -140,6 +140,7 @@ public function startImportOnedrive(string $userId): array { } $this->config->setUserValue($userId, Application::APP_ID, 'importing_onedrive', '1'); $this->config->setUserValue($userId, Application::APP_ID, 'imported_size', '0'); + $this->config->setUserValue($userId, Application::APP_ID, 'nb_imported_files', '0'); $this->config->setUserValue($userId, Application::APP_ID, 'nb_failed_files', '0'); $this->config->setUserValue($userId, Application::APP_ID, 'nb_skipped_files', '0'); $this->config->setUserValue($userId, Application::APP_ID, 'last_onedrive_import_timestamp', '0'); diff --git a/tests/unit/Service/OnedriveStorageAPIServiceTest.php b/tests/unit/Service/OnedriveStorageAPIServiceTest.php index 0cb5c85..39fe122 100644 --- a/tests/unit/Service/OnedriveStorageAPIServiceTest.php +++ b/tests/unit/Service/OnedriveStorageAPIServiceTest.php @@ -12,6 +12,7 @@ use OCA\Onedrive\Service\UserScopeService; use OCP\BackgroundJob\IJobList; use OCP\Files\File; +use OCP\Files\FileInfo; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\IUserFolder; @@ -333,4 +334,32 @@ static function (string $userId, string $endPoint) { $this->assertArrayNotHasKey('failed_files', $this->configStore); $this->assertSame('0', $this->configStore['importing_onedrive']); } + + public function testStartingAnImportForgetsTheCountersOfThePreviousOne(): void { + $this->useStatefulConfig([ + 'nb_imported_files' => '41', + 'nb_failed_files' => '2', + 'nb_skipped_files' => '3', + 'failed_files' => '["x.jpg"]', + 'imported_size' => '123456', + 'import_tree' => '{"/sub":"todo"}', + ]); + $folder = $this->createMock(Folder::class); + $folder->method('getType')->willReturn(FileInfo::TYPE_FOLDER); + $userFolder = $this->createUserFolderMock(); + $userFolder->method('nodeExists')->willReturn(true); + $userFolder->method('get')->willReturn($folder); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + $this->jobList->expects($this->once())->method('add'); + + $this->service->startImportOnedrive('user1'); + + $this->assertSame('1', $this->configStore['importing_onedrive']); + $this->assertSame('0', $this->configStore['nb_imported_files']); + $this->assertSame('0', $this->configStore['nb_failed_files']); + $this->assertSame('0', $this->configStore['nb_skipped_files']); + $this->assertSame('0', $this->configStore['imported_size']); + $this->assertArrayNotHasKey('failed_files', $this->configStore); + $this->assertArrayNotHasKey('import_tree', $this->configStore); + } } From 63669ea762793e9514d53bb6d7c7e3629572d85d Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:44:54 +0000 Subject: [PATCH 2/3] fix: log the file an import cannot look up in the target folder A file the target folder refuses to answer for is counted as a file that could not be downloaded, and the notification reporting it says to check the server logs, where nothing was written about it. Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 1 + lib/Service/OnedriveStorageAPIService.php | 3 +++ .../Service/OnedriveStorageAPIServiceTest.php | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc015b..0e7115d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Mention files that were already there in the import finished notification, so re-running an import does not look like a failure - Count imported empty files as imported - Report the number of files the current import brought, a counter left over from an interrupted import is no longer added to it +- Say in the log which file of an import could not be looked up in the target folder - Test the import against a stubbed Graph API, including failed downloads, empty files and paging ## [3.5.2] - 2026-07-28 diff --git a/lib/Service/OnedriveStorageAPIService.php b/lib/Service/OnedriveStorageAPIService.php index 956f7d7..dcaefd0 100644 --- a/lib/Service/OnedriveStorageAPIService.php +++ b/lib/Service/OnedriveStorageAPIService.php @@ -471,6 +471,9 @@ private function getFile(string $userId, Folder $folder, array $fileItem): array try { $fileExists = $folder->nodeExists($fileName); } catch (ForbiddenException $e) { + // it is counted as a failed file below, so say why: the notification that + // reports it points at the log + $this->logger->warning('OneDrive cannot check whether file ' . $fileName . ' is already there: ' . $e->getMessage(), ['app' => Application::APP_ID]); return ['status' => self::FILE_FAILED, 'size' => 0.0]; } if ($fileExists) { diff --git a/tests/unit/Service/OnedriveStorageAPIServiceTest.php b/tests/unit/Service/OnedriveStorageAPIServiceTest.php index 39fe122..f668447 100644 --- a/tests/unit/Service/OnedriveStorageAPIServiceTest.php +++ b/tests/unit/Service/OnedriveStorageAPIServiceTest.php @@ -14,6 +14,7 @@ use OCP\Files\File; use OCP\Files\FileInfo; use OCP\Files\Folder; +use OCP\Files\ForbiddenException; use OCP\Files\IRootFolder; use OCP\Files\IUserFolder; use OCP\IConfig; @@ -25,6 +26,7 @@ class OnedriveStorageAPIServiceTest extends TestCase { private OnedriveAPIService|MockObject $apiService; + private LoggerInterface|MockObject $logger; private IRootFolder|MockObject $rootFolder; private IConfig|MockObject $config; private IJobList|MockObject $jobList; @@ -43,12 +45,13 @@ public function setUp(): void { parent::setUp(); $this->apiService = $this->createMock(OnedriveAPIService::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->rootFolder = $this->createMock(IRootFolder::class); $this->config = $this->createMock(IConfig::class); $this->jobList = $this->createMock(IJobList::class); $this->service = new OnedriveStorageAPIService( 'integration_onedrive', - $this->createMock(LoggerInterface::class), + $this->logger, $this->rootFolder, $this->config, $this->jobList, @@ -362,4 +365,18 @@ public function testStartingAnImportForgetsTheCountersOfThePreviousOne(): void { $this->assertArrayNotHasKey('failed_files', $this->configStore); $this->assertArrayNotHasKey('import_tree', $this->configStore); } + + public function testFileThatCannotBeLookedUpIsLoggedAndCountedAsFailed(): void { + $folder = $this->createMock(Folder::class); + $folder->method('nodeExists')->willThrowException(new ForbiddenException('no reading here', false)); + $this->apiService->expects($this->never())->method('fileRequest'); + $this->logger->expects($this->once()) + ->method('warning') + ->with($this->stringContains('photo.jpg'), ['app' => 'integration_onedrive']); + + $method = new ReflectionMethod(OnedriveStorageAPIService::class, 'getFile'); + $result = $method->invoke($this->service, 'user1', $folder, ['name' => 'photo.jpg']); + + $this->assertSame(['status' => 'failed', 'size' => 0.0], $result); + } } From 8b5f45a483c2990b8f1804335adf6ccefa666412 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:44:54 +0000 Subject: [PATCH 3/3] test: answer nothing but from a development server in the Graph stub The stub sits in the app directory, so a web server could be asked to run it. Signed-off-by: Oleksander Piskun --- tests/integration/graph-stub.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/graph-stub.php b/tests/integration/graph-stub.php index 8aa6453..f60989d 100644 --- a/tests/integration/graph-stub.php +++ b/tests/integration/graph-stub.php @@ -26,6 +26,12 @@ * The first listing page carries an @odata.nextLink, so paging is covered as well. */ +// the router of a development server is the only thing this is ever meant to be +if (PHP_SAPI !== 'cli-server') { + http_response_code(404); + return; +} + const MODIFIED = '2026-09-01T10:00:00Z'; /** Every file of the fake drive, with what its download URLs should answer. */