diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab28c6..0e7115d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ 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 +- 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 6a8b21f..dcaefd0 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'); @@ -470,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/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. */ diff --git a/tests/unit/Service/OnedriveStorageAPIServiceTest.php b/tests/unit/Service/OnedriveStorageAPIServiceTest.php index 0cb5c85..f668447 100644 --- a/tests/unit/Service/OnedriveStorageAPIServiceTest.php +++ b/tests/unit/Service/OnedriveStorageAPIServiceTest.php @@ -12,7 +12,9 @@ use OCA\Onedrive\Service\UserScopeService; use OCP\BackgroundJob\IJobList; 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; @@ -24,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; @@ -42,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, @@ -333,4 +337,46 @@ 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); + } + + 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); + } }