From 021d3cb3afc999e6f3cfa28e4bc26d6b8eaecb56 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:54:33 +0000 Subject: [PATCH 1/3] fix: come back to the listing page an import stopped in A batch that reached its download size in the middle of a folder listing left the rest of that listing out of the import: a directory was dropped from the import tree once its first page had been walked, and the import folder itself was never in the tree at all. The next batch had nothing to come back to and the import reported itself as finished, with the files of the unfinished pages missing and nothing in the log. A directory now stays in the tree, carrying the page to resume at, until its listing is exhausted. A page that cannot be listed any more, an expired token for instance, starts the directory over from its first page, and a listing that fails without one is logged. The files a page skipped because they were already there are counted even when the batch ends on that page. Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 4 + lib/Service/OnedriveStorageAPIService.php | 75 ++++- .../Service/OnedriveStorageAPIServiceTest.php | 267 ++++++++++++++++++ 3 files changed, 335 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e7115d..2e47152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - 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 +- Come back to the folder listing page an import stopped in, the files behind it were left out and the import still reported itself as finished +- Stop counting the files an import downloaded itself among the files that were already there +- Try a folder whose listing failed again in a later batch instead of leaving it out of the import +- List a folder again from its first page when the page an import stopped in cannot be listed any more - 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 dcaefd0..5a0c3eb 100644 --- a/lib/Service/OnedriveStorageAPIService.php +++ b/lib/Service/OnedriveStorageAPIService.php @@ -40,6 +40,12 @@ class OnedriveStorageAPIService { + /** A directory of the import tree that has not been listed at all yet. */ + private const DIR_NOT_STARTED = 'todo'; + + /** A directory whose first listing page a batch has begun but not finished. */ + private const DIR_STARTED = 'started'; + private const FILE_DOWNLOADED = 'downloaded'; private const FILE_ALREADY_THERE = 'already there'; private const FILE_FAILED = 'failed'; @@ -282,11 +288,19 @@ public function importFiles(string $userId, string $targetPath, ); } else { foreach ($importTree as $path => $state) { - if ($state === 'todo') { - $downloadResult = $this->downloadDir( - $userId, $topFolder, $maxDownloadSize, 0, 0, 0, (string)$path, $alreadyImportedSize, $alreadyImportedNumber, $importTree - ); + if (!isset($importTree[$path])) { + // a directory this batch already finished on its way through a parent + continue; } + // an unfinished directory is remembered as not started at all, as begun + // on its first page, or with the listing page the last batch stopped in + $startedBefore = $state !== self::DIR_NOT_STARTED; + $resumeToken = (!$startedBefore || $state === self::DIR_STARTED || !is_string($state) || $state === '') + ? null + : $state; + $downloadResult = $this->downloadDir( + $userId, $topFolder, $maxDownloadSize, 0, 0, 0, (string)$path, $alreadyImportedSize, $alreadyImportedNumber, $importTree, $resumeToken, $startedBefore + ); } } } catch (MaxDownloadSizeReachedException $e) { @@ -314,6 +328,8 @@ private function downloadDir( float $alreadyImportedSize, int $alreadyImportedNumber, array &$importTree, + ?string $resumeToken = null, + bool $resuming = false, ): array { $newDownloadedSize = (float)$downloadedSize; $newTotalSeenNumber = $totalSeenNumber; @@ -335,9 +351,40 @@ private function downloadDir( /** @var string[] $subDirs */ $subDirs = []; $params = []; + if ($resumeToken !== null) { + $params['$skiptoken'] = $resumeToken; + } + // Remember this directory as unfinished for as long as its listing is not + // exhausted. A batch that stops in the middle of it, because it reached its + // download size, has to come back to it, and to the page it stopped in. + $importTree[$path] = $resumeToken ?? self::DIR_NOT_STARTED; + $listingStartedOver = false; + // the files of a page an earlier batch had already started are not files that + // were "already there": this import downloaded them itself + $walkingThePageAgain = $resuming; do { $result = $this->onedriveApiService->request($userId, $endPoint, $params); if (isset($result['error']) || !isset($result['value']) || !is_array($result['value'])) { + if (isset($params['$skiptoken']) && !$listingStartedOver) { + // the page cannot be listed any more, its token may simply have expired: + // start the directory over once, the files it already brought are skipped + // as existing ones + $this->logger->info( + 'OneDrive could not list a page of ' . ($path === '' ? 'the import folder' : $path) . ', starting the folder over', + ['app' => Application::APP_ID] + ); + $listingStartedOver = true; + unset($params['$skiptoken']); + $importTree[$path] = self::DIR_NOT_STARTED; + $subDirs = []; + continue; + } + // the directory stays in the tree: a later batch tries it again, and an + // import that ends before that at least says in the log what it missed + $this->logger->warning( + 'OneDrive error listing ' . ($path === '' ? 'the import folder' : $path) . ': ' . ($result['error'] ?? 'no file list in the answer'), + ['app' => Application::APP_ID] + ); return [ 'downloadedSize' => $newDownloadedSize, 'totalSeenNumber' => $newTotalSeenNumber, @@ -346,6 +393,11 @@ private function downloadDir( } $pageSkipped = 0; + if (!isset($params['$skiptoken'])) { + // the first page has begun: a batch that stops inside it has to come back + // to it, and must not count its files as files that were already there + $importTree[$path] = self::DIR_STARTED; + } /** @var OneDriveItem $item */ foreach ($result['value'] as $item) { if (isset($item['file'])) { @@ -380,19 +432,16 @@ private function downloadDir( $subDirs[] = $item['name']; // mark for progress tracking $subPath = ltrim($path . '/' . $item['name']); - $importTree[$subPath] = 'todo'; + $importTree[$subPath] = self::DIR_NOT_STARTED; } } - if ($pageSkipped > 0) { + if ($pageSkipped > 0 && !$walkingThePageAgain) { // one write per listing page, skipped files are frequent on re-imports $nbSkipped = (int)$this->config->getUserValue($userId, Application::APP_ID, 'nb_skipped_files', '0'); $this->config->setUserValue($userId, Application::APP_ID, 'nb_skipped_files', (string)($nbSkipped + $pageSkipped)); } - - // if this directory was marked unfinished, remove it now - if (isset($importTree[$path])) { - unset($importTree[$path]); - } + // only the first page of a resumed directory is one an earlier batch had started + $walkingThePageAgain = false; // prepare next page if any if (isset($result['@odata.nextLink']) @@ -400,7 +449,11 @@ private function downloadDir( && preg_match('/\$skiptoken=/i', $result['@odata.nextLink']) ) { $params['$skiptoken'] = preg_replace('/.*\$skiptoken=/', '', $result['@odata.nextLink']); + // come back to this page, not to the first one, if the import stops here + $importTree[$path] = $params['$skiptoken']; } else { + // the whole directory has been listed + unset($importTree[$path]); break; } } while (true); diff --git a/tests/unit/Service/OnedriveStorageAPIServiceTest.php b/tests/unit/Service/OnedriveStorageAPIServiceTest.php index f668447..eaba16e 100644 --- a/tests/unit/Service/OnedriveStorageAPIServiceTest.php +++ b/tests/unit/Service/OnedriveStorageAPIServiceTest.php @@ -38,6 +38,12 @@ class OnedriveStorageAPIServiceTest extends TestCase { /** @var array */ private array $configStore = []; + /** @var string[] names of the files the target folder holds */ + private array $existingFiles = []; + + /** @var string[] the downloads the app asked for, in order */ + private array $downloadedFiles = []; + private const STALE_URL = 'https://stale.example.org/download?tempauth=old'; private const FRESH_URL = 'https://fresh.example.org/download?tempauth=new'; @@ -379,4 +385,265 @@ public function testFileThatCannotBeLookedUpIsLoggedAndCountedAsFailed(): void { $this->assertSame(['status' => 'failed', 'size' => 0.0], $result); } + /** + * A drive whose root listing has two pages: a folder on the first one, two files on + * the second. $fileSize decides where the batch download size runs out. + */ + private function statefulDriveWithTwoRootPages(int $fileSize, array $alreadyThere = []): void { + $this->useStatefulConfig([]); + $this->apiService->method('request')->willReturnCallback( + static function (string $userId, string $endPoint, array $params = []) { + if ($endPoint === 'me/drive') { + return ['quota' => ['used' => 1000]]; + } + $file = static fn (string $name) => [ + 'name' => $name, + 'id' => 'id-' . $name, + 'file' => [], + '@microsoft.graph.downloadUrl' => 'https://dl.example.org/' . $name, + ]; + if ($endPoint === 'me/drive/root/children') { + if (($params['$skiptoken'] ?? '') === 'page2') { + return ['value' => [$file('second-page-1.jpg'), $file('second-page-2.jpg')]]; + } + return [ + 'value' => [['name' => 'sub', 'id' => 'id-sub', 'folder' => []]], + '@odata.nextLink' => 'https://graph.example.org/me/drive/root/children?$skiptoken=page2', + ]; + } + if ($endPoint === 'me/drive/root:%2Fsub:/children') { + return ['value' => [$file('nested.jpg')]]; + } + // the folder itself, asked for to copy its modification time + return ['lastModifiedDateTime' => '2026-09-01T10:00:00Z']; + } + ); + $this->apiService->method('fileRequest')->willReturnCallback( + function (string $url) { + $this->downloadedFiles[] = basename($url); + return ['success' => true]; + } + ); + + $this->existingFiles = $alreadyThere; + $folders = []; + $folderFor = function (string $path) use (&$folders, $fileSize) { + if (!isset($folders[$path])) { + $folder = $this->createMock(Folder::class); + $folder->method('nodeExists')->willReturnCallback( + fn (string $name) => in_array($path . '/' . $name, $this->existingFiles, true) + ); + $folder->method('newFile')->willReturnCallback(function (string $name) use ($path, $fileSize) { + $this->existingFiles[] = $path . '/' . $name; + $file = $this->createMock(File::class); + $file->method('fopen')->willReturnCallback(static fn () => fopen('php://temp', 'w+')); + $file->method('stat')->willReturn(['size' => $fileSize]); + $file->method('isDeletable')->willReturn(true); + return $file; + }); + $folders[$path] = $folder; + } + return $folders[$path]; + }; + $topFolder = $this->createMock(Folder::class); + $topFolder->method('nodeExists')->willReturn(true); + $topFolder->method('get')->willReturnCallback(static fn (string $path) => $folderFor($path)); + $userFolder = $this->createUserFolderMock(); + $userFolder->method('nodeExists')->willReturn(true); + $userFolder->method('get')->willReturn($topFolder); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + } + + public function testBatchThatRunsOutInAListingComesBackForTheRestOfIt(): void { + $this->statefulDriveWithTwoRootPages(200); + $importTree = []; + + // 200 bytes per file, so the first file of the root's second page ends the batch + $first = $this->service->importFiles('user1', '/Import', 100, 0, 0, $importTree); + + $this->assertFalse($first['finished']); + $this->assertSame(['second-page-1.jpg'], $this->downloadedFiles); + $this->assertSame('todo', $importTree['/sub'] ?? null, 'the folder found on the first page is remembered'); + $this->assertSame('page2', $importTree[''] ?? null, 'the root is remembered at the page it stopped in'); + + // the job keeps handing the remembered tree to the next batch until one finishes + $batches = 1; + do { + $result = $this->service->importFiles('user1', '/Import', 100, 0, 0, $importTree); + $batches++; + } while (empty($result['finished']) && $batches < 6); + + $this->assertTrue($result['finished']); + $this->assertSame( + ['second-page-1.jpg', 'second-page-2.jpg', 'nested.jpg'], + $this->downloadedFiles, + 'every file of the drive was imported' + ); + $this->assertSame([], $importTree, 'nothing is left unfinished'); + } + + public function testFilesTheImportBroughtItselfAreNotReportedAsAlreadyThere(): void { + $this->statefulDriveWithTwoRootPages(200); + $importTree = []; + + // the first batch downloads one file of the second page, the next batch walks that + // page again and finds it in place + $this->service->importFiles('user1', '/Import', 100, 0, 0, $importTree); + $this->assertSame(['second-page-1.jpg'], $this->downloadedFiles); + $batches = 1; + do { + $result = $this->service->importFiles('user1', '/Import', 100, 0, 0, $importTree); + $batches++; + } while (empty($result['finished']) && $batches < 6); + + $this->assertSame('0', $this->configStore['nb_skipped_files'] ?? '0', 'nothing was already there'); + } + + public function testFilesThatWereAlreadyThereAreStillCounted(): void { + // the file of the sub-folder is there before the import starts, and the sub-folder + // is only reached by a later batch, which is not walking a page again + $this->statefulDriveWithTwoRootPages(200, ['/sub/nested.jpg']); + $importTree = []; + + $batches = 0; + do { + $result = $this->service->importFiles('user1', '/Import', 100, 0, 0, $importTree); + $batches++; + } while (empty($result['finished']) && $batches < 6); + + $this->assertSame('1', $this->configStore['nb_skipped_files'] ?? '0'); + } + + public function testResumingAFolderAsksForThePageItStoppedIn(): void { + $this->statefulDriveWithTwoRootPages(200); + $importTree = ['' => 'page2']; + + $this->service->importFiles('user1', '/Import', null, 0, 0, $importTree); + + // the second page holds both files, the first page only the sub-folder: asking for + // the first page again would have downloaded nothing from the root + $this->assertSame( + ['second-page-1.jpg', 'second-page-2.jpg'], + $this->downloadedFiles, + 'the remembered page was asked for, not the first one' + ); + } + + public function testFolderWhoseListingFailsIsTriedAgainByTheNextBatch(): void { + $this->useStatefulConfig([]); + $listings = []; + $failuresLeft = 1; + $this->apiService->method('request')->willReturnCallback( + static function (string $userId, string $endPoint) use (&$listings, &$failuresLeft) { + if ($endPoint === 'me/drive') { + return ['quota' => ['used' => 1000]]; + } + if ($endPoint === 'me/drive/root:%2Fsub:/children') { + $listings[] = $endPoint; + if ($failuresLeft > 0) { + $failuresLeft--; + return ['error' => 'serviceUnavailable']; + } + return ['value' => [[ + 'name' => 'nested.jpg', + 'id' => 'id-nested', + 'file' => [], + '@microsoft.graph.downloadUrl' => 'https://dl.example.org/nested.jpg', + ]]]; + } + if ($endPoint === 'me/drive/root/children') { + return ['value' => [['name' => 'sub', 'id' => 'id-sub', 'folder' => []]]]; + } + return ['lastModifiedDateTime' => '2026-09-01T10:00:00Z']; + } + ); + $this->apiService->method('fileRequest')->willReturnCallback( + function (string $url) { + $this->downloadedFiles[] = basename($url); + return ['success' => true]; + } + ); + $folder = $this->createMock(Folder::class); + $folder->method('nodeExists')->willReturn(false); + $folder->method('newFile')->willReturnCallback(function () { + $file = $this->createMock(File::class); + $file->method('fopen')->willReturnCallback(static fn () => fopen('php://temp', 'w+')); + $file->method('stat')->willReturn(['size' => 10]); + return $file; + }); + $topFolder = $this->createMock(Folder::class); + $topFolder->method('nodeExists')->willReturn(true); + $topFolder->method('get')->willReturn($folder); + $userFolder = $this->createUserFolderMock(); + $userFolder->method('nodeExists')->willReturn(true); + $userFolder->method('get')->willReturn($topFolder); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + $importTree = []; + $this->service->importFiles('user1', '/Import', null, 0, 0, $importTree); + + $this->assertSame([], $this->downloadedFiles, 'the folder could not be listed'); + $this->assertSame('todo', $importTree['/sub'] ?? null, 'it is still to do'); + + $this->service->importFiles('user1', '/Import', null, 0, 0, $importTree); + + $this->assertSame(['nested.jpg'], $this->downloadedFiles, 'the next batch listed it again'); + $this->assertSame([], $importTree); + } + + public function testFolderIsStartedOverWhenItsRememberedPageIsGone(): void { + $this->useStatefulConfig([]); + $listed = []; + $this->apiService->method('request')->willReturnCallback( + static function (string $userId, string $endPoint, array $params = []) use (&$listed) { + if ($endPoint === 'me/drive') { + return ['quota' => ['used' => 1000]]; + } + if ($endPoint === 'me/drive/root/children') { + $listed[] = $params['$skiptoken'] ?? 'first page'; + if (isset($params['$skiptoken'])) { + return ['error' => 'invalid skiptoken']; + } + return ['value' => [[ + 'name' => 'photo.jpg', + 'id' => 'id-photo', + 'file' => [], + '@microsoft.graph.downloadUrl' => 'https://dl.example.org/photo.jpg', + ]]]; + } + return ['lastModifiedDateTime' => '2026-09-01T10:00:00Z']; + } + ); + $this->apiService->method('fileRequest')->willReturnCallback( + function (string $url) { + $this->downloadedFiles[] = basename($url); + return ['success' => true]; + } + ); + $folder = $this->createMock(Folder::class); + $folder->method('nodeExists')->willReturn(false); + $folder->method('newFile')->willReturnCallback(function () { + $file = $this->createMock(File::class); + $file->method('fopen')->willReturnCallback(static fn () => fopen('php://temp', 'w+')); + $file->method('stat')->willReturn(['size' => 10]); + return $file; + }); + $topFolder = $this->createMock(Folder::class); + $topFolder->method('nodeExists')->willReturn(true); + $topFolder->method('get')->willReturn($folder); + $userFolder = $this->createUserFolderMock(); + $userFolder->method('nodeExists')->willReturn(true); + $userFolder->method('get')->willReturn($topFolder); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + // the previous batch stopped at a page whose token is not accepted any more + $importTree = ['' => 'stale-token']; + $result = $this->service->importFiles('user1', '/Import', null, 0, 0, $importTree); + + $this->assertSame(['stale-token', 'first page'], $listed, 'the folder is listed again from its first page'); + $this->assertSame(['photo.jpg'], $this->downloadedFiles); + $this->assertTrue($result['finished']); + $this->assertSame([], $importTree); + } + } From 0b8db95ada81cd85a870b6751d34423e58da2d8c Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:54:52 +0000 Subject: [PATCH 2/3] feat: let admins choose how much one import batch downloads The import job downloads 500 MB before it lets the next run continue. On a server where that is too much of a single job, or for a test that wants to see the import resume, the import_batch_size app setting now says how much a batch downloads. Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 1 + README.md | 6 ++ lib/AppInfo/Application.php | 6 ++ lib/Service/OnedriveStorageAPIService.php | 8 +- .../Service/OnedriveStorageAPIServiceTest.php | 77 +++++++++++++++++++ 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e47152..bc6470d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Added support for Nextcloud 36 +- Added the import_batch_size app setting, how much a single run of the import job downloads before it lets the next one continue ### Fixed diff --git a/README.md b/README.md index 28a6378..18535d5 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,12 @@ The account configuration and data migration happen in the "Data migration" user There also is a "Connected accounts" **admin** settings section that you must visit to configure a Microsoft Azure OAuth app to allow your Nextcloud users to authenticate to Microsoft services. +A single run of the file import job downloads 500 MB before it lets the next run continue. On a server where that is too much for one job, set another size in bytes: + +``` +occ config:app:set integration_onedrive import_batch_size --value 100000000 +``` + ## **🛠️ State of maintenance** While there are many things that could be done to further improve this app, the app is currently maintained with **limited effort**. This means: diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 1a8fc5d..1e3d6b5 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -27,6 +27,12 @@ class Application extends App implements IBootstrap { public const APP_ID = 'integration_onedrive'; public const IMPORT_JOB_TIMEOUT = 3600; + /** + * How much a single run of the import job downloads before it stops and lets the next + * one continue, in bytes. Overridable with the import_batch_size app setting. + */ + public const IMPORT_BATCH_SIZE = 500000000; + public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); } diff --git a/lib/Service/OnedriveStorageAPIService.php b/lib/Service/OnedriveStorageAPIService.php index 5a0c3eb..7aadc76 100644 --- a/lib/Service/OnedriveStorageAPIService.php +++ b/lib/Service/OnedriveStorageAPIService.php @@ -205,11 +205,15 @@ public function importOnedriveJob(string $userId): void { $importTreeStr = $this->config->getUserValue($userId, Application::APP_ID, 'import_tree', '[]'); /** @var array $importTree */ $importTree = ($importTreeStr === '[]' || $importTreeStr === '') ? [] : json_decode($importTreeStr, true); - // import by batch of 500 MB + // import by batches, 500 MB each unless the admin asked for another size + $batchSize = (int)$this->config->getAppValue(Application::APP_ID, 'import_batch_size', (string)Application::IMPORT_BATCH_SIZE); + if ($batchSize <= 0) { + $batchSize = Application::IMPORT_BATCH_SIZE; + } $alreadyImportedSize = (float)$this->config->getUserValue($userId, Application::APP_ID, 'imported_size', '0'); $alreadyImportedNumber = (int)$this->config->getUserValue($userId, Application::APP_ID, 'nb_imported_files', '0'); try { - $result = $this->importFiles($userId, $targetPath, 500000000, $alreadyImportedSize, $alreadyImportedNumber, $importTree); + $result = $this->importFiles($userId, $targetPath, $batchSize, $alreadyImportedSize, $alreadyImportedNumber, $importTree); } catch (Exception|Throwable $e) { $result = [ 'error' => 'Unknow job failure. ' . $e->getMessage(), diff --git a/tests/unit/Service/OnedriveStorageAPIServiceTest.php b/tests/unit/Service/OnedriveStorageAPIServiceTest.php index eaba16e..9669b9b 100644 --- a/tests/unit/Service/OnedriveStorageAPIServiceTest.php +++ b/tests/unit/Service/OnedriveStorageAPIServiceTest.php @@ -646,4 +646,81 @@ function (string $url) { $this->assertSame([], $importTree); } + public function testTheBatchSizeCanBeConfigured(): void { + $this->useStatefulConfig(['importing_onedrive' => '1']); + $this->config->method('getAppValue')->willReturnCallback( + static fn (string $app, string $key, string $default = '') => $key === 'import_batch_size' ? '100' : $default + ); + // two files of 200 bytes, so a 100 byte batch stops after the first one + $this->apiService->method('request')->willReturnCallback( + static function (string $userId, string $endPoint) { + if ($endPoint === 'me/drive') { + return ['quota' => ['used' => 1000]]; + } + if ($endPoint === 'me/drive/root/children') { + return ['value' => array_map(static fn (string $name) => [ + 'name' => $name, + 'id' => 'id-' . $name, + 'file' => [], + '@microsoft.graph.downloadUrl' => 'https://dl.example.org/' . $name, + ], ['a.jpg', 'b.jpg'])]; + } + return ['lastModifiedDateTime' => '2026-09-01T10:00:00Z']; + } + ); + $this->apiService->method('fileRequest')->willReturnCallback( + function (string $url) { + $this->downloadedFiles[] = basename($url); + return ['success' => true]; + } + ); + $dirFolder = $this->createMock(Folder::class); + $dirFolder->method('nodeExists')->willReturn(false); + $dirFolder->method('newFile')->willReturnCallback(function () { + $file = $this->createMock(File::class); + $file->method('fopen')->willReturnCallback(static fn () => fopen('php://temp', 'w+')); + $file->method('stat')->willReturn(['size' => 200]); + return $file; + }); + $topFolder = $this->createMock(Folder::class); + $topFolder->method('isShared')->willReturn(false); + $topFolder->method('nodeExists')->willReturn(true); + $topFolder->method('get')->willReturn($dirFolder); + $userFolder = $this->createUserFolderMock(); + $userFolder->method('nodeExists')->willReturn(true); + $userFolder->method('get')->willReturn($topFolder); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + // the job queues itself again to continue with the next batch + $this->jobList->expects($this->once())->method('add'); + + $this->service->importOnedriveJob('user1'); + + $this->assertSame(['a.jpg'], $this->downloadedFiles, 'the batch stopped after the first file'); + $this->assertSame('1', $this->configStore['importing_onedrive'], 'the import is not finished'); + $this->assertArrayHasKey('import_tree', $this->configStore); + } + + public function testTheImportIsResumedFromWhatTheBatchWroteToTheConfig(): void { + $this->statefulDriveWithTwoRootPages(200); + $this->configStore['importing_onedrive'] = '1'; + $this->config->method('getAppValue')->willReturnCallback( + static fn (string $app, string $key, string $default = '') => $key === 'import_batch_size' ? '100' : $default + ); + + // the job runs once per cron round and hands its progress over through the config + for ($round = 1; $round <= 6; $round++) { + $this->service->importOnedriveJob('user1'); + if (($this->configStore['importing_onedrive'] ?? '0') === '0') { + break; + } + } + + $this->assertSame('0', $this->configStore['importing_onedrive'], 'the import finished'); + $this->assertSame( + ['second-page-1.jpg', 'second-page-2.jpg', 'nested.jpg'], + $this->downloadedFiles, + 'every file of the drive was imported' + ); + $this->assertArrayNotHasKey('import_tree', $this->configStore); + } } From b431d4ea15d3dee1be59ddd3fc2b48fe675080fe Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 25 Sep 2026 07:54:52 +0000 Subject: [PATCH 3/3] test: import in batches that stop in the middle of a listing page The stubbed drive is imported a second time into another folder, with a batch size of 20 bytes, so that the import runs out in the middle of the second listing page and has to come back to it. The job checks that the page was listed again and that the files behind it arrived. Signed-off-by: Oleksander Piskun --- .github/workflows/integration-test.yml | 80 ++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/.github/workflows/integration-test.yml b/.github/workflows/integration-test.yml index c3bf5e9..f9e1f4f 100644 --- a/.github/workflows/integration-test.yml +++ b/.github/workflows/integration-test.yml @@ -342,6 +342,86 @@ jobs: fi exit $fail + - name: Import again in batches that end in the middle of a listing page + working-directory: server/ + run: | + # 20 bytes runs out on the first file of the second listing page, so the import + # has to come back to that page in a later batch + php occ config:app:set ${{ env.APP_NAME }} import_batch_size --value 20 + php occ user:setting admin ${{ env.APP_NAME }} onedrive_output_dir "/OneDrive resume" + # drop the notification of the first import, the one checked below has to be the + # one this import sends + curl -s -X DELETE -u admin:admin -H 'OCS-APIRequest: true' \ + 'http://localhost:8080/ocs/v2.php/apps/notifications/api/v2/notifications' >/dev/null + mkdir -p "data/admin/files/OneDrive resume" + printf 'already there' > "data/admin/files/OneDrive resume/already.txt" + php occ files:scan admin -q + echo "STUB_LOG_LINES=$(wc -l < stub.log)" >> "$GITHUB_ENV" + php occ ${{ env.APP_NAME }}:start-import admin + importing=1 + for run in {1..8}; do + php cron.php + importing=$(php occ user:setting admin ${{ env.APP_NAME }} importing_onedrive 2>/dev/null | tail -1 || echo "1") + imported=$(php occ user:setting admin ${{ env.APP_NAME }} nb_imported_files 2>/dev/null | tail -1 || echo "?") + echo "batch $run: importing=$importing imported=$imported" + if [ "$importing" = "0" ]; then break; fi + done + if [ "$importing" != "0" ]; then + echo "The import did not finish" + exit 1 + fi + + - name: Verify the import came back for the page it stopped in + working-directory: server/ + run: | + tail -n +$((STUB_LOG_LINES + 1)) stub.log > resume.log + grep -oE 'stub (GET|POST) [^ ]+ -> [0-9]+' resume.log | sort | uniq -c + fail=0 + pages=$(grep -c 'skiptoken=page2' resume.log || true) + if [ "$pages" -lt 2 ]; then + echo "the second listing page was fetched $pages time(s), the import never came back to it" + fail=1 + fi + # coming back to the page is the point: walking the whole folder again would + # fetch the first page a second time as well, and would pass the check above + first=$(grep -cE 'root/children -> 200' resume.log || true) + if [ "$first" != "1" ]; then + echo "the first listing page was fetched $first time(s), the import started the folder over" + fail=1 + fi + target="data/admin/files/OneDrive resume" + find "$target" -type f -printf '%P %s bytes\n' | sort + check() { # path, expected size + if [ ! -f "$target/$1" ]; then echo "missing: $1"; fail=1; return; fi + size=$(stat -c%s "$target/$1") + if [ "$size" != "$2" ]; then echo "$1 has $size bytes, expected $2"; fail=1; fi + } + check normal.txt 12 + check empty.txt 0 + check flaky.txt 9 + check sub/nested.txt 11 + if [ -f "$target/broken.txt" ]; then + echo "broken.txt should not have been created, its download failed twice" + fail=1 + fi + # flaky.txt was brought by the first batch and sits in the page the second batch + # walked again: it must not be reported among the files that were already there, + # and already.txt shares that page, so no file is + notification=$(curl -s -u admin:admin -H 'OCS-APIRequest: true' -H 'Accept: application/json' \ + 'http://localhost:8080/ocs/v2.php/apps/notifications/api/v2/notifications') + expected='4 files were imported from OneDrive storage. 1 file could not be downloaded, check the server logs for details.' + subjects=$(php -r 'foreach (json_decode(stream_get_contents(STDIN), true)["ocs"]["data"] as $n) { if (($n["app"] ?? "") === "integration_onedrive") { echo $n["subject"], "\n"; } }' <<< "$notification") + echo "$subjects" + if [ "$(printf '%s\n' "$subjects" | grep -c .)" != "1" ]; then + echo "expected exactly one notification of this import" + fail=1 + fi + if [ "$subjects" != "$expected" ]; then + echo "expected: $expected" + fail=1 + fi + exit $fail + - name: Show the logs on failure working-directory: server/ if: failure()