diff --git a/src/services/drive/drive-item.service.ts b/src/services/drive/drive-item.service.ts index 030d4af0..87b9c9e2 100644 --- a/src/services/drive/drive-item.service.ts +++ b/src/services/drive/drive-item.service.ts @@ -1,14 +1,86 @@ import { DriveItemRepository } from '../database/drive-item/drive-item.repository'; import { DriveFileItem, DriveFolderItem } from '../../types/drive.types'; -import { ErrorUtils } from '../../utils/errors.utils'; +import { ErrorUtils, NotFoundError } from '../../utils/errors.utils'; import { webdavLogger } from '../../utils/logger.utils'; import { DriveFileService } from './drive-file.service'; import { DriveFolderService } from './drive-folder.service'; import { DriveItemBD } from '../database/drive-item/drive-item.domain'; +const RECENT_WRITE_TRUST_MS = 10_000; +const RECENT_CHANGE_TRUST_MS = 60_000; + +const normalizePath = (path: string): string => (path.length > 1 && path.endsWith('/') ? path.slice(0, -1) : path); + +/** A not found that comes from our own recent change, not from an API failure. */ +class StaleItemError extends NotFoundError { + constructor(message: string) { + super(message); + this.name = 'StaleItemError'; + Object.setPrototypeOf(this, StaleItemError.prototype); + } +} + export class DriveItemService { static readonly instance = new DriveItemService(); + private readonly recentFileWrites = new Map(); + /** Where this server last put an item, keyed by uuid; a null path means it was deleted. */ + private readonly recentChanges = new Map(); + + private readonly rememberChange = (uuid: string, path: string | null): void => { + const now = Date.now(); + for (const [changedUuid, change] of this.recentChanges) { + if (change.expiresAt <= now) this.recentChanges.delete(changedUuid); + } + this.recentChanges.set(uuid, { path, expiresAt: now + RECENT_CHANGE_TRUST_MS }); + }; + + public rememberDeletion = (uuid: string): void => this.rememberChange(uuid, null); + + public rememberMove = (uuid: string, path: string): void => this.rememberChange(uuid, normalizePath(path)); + + /** The replicas can keep returning an item from where it was before this server moved or + * deleted it, and acting on that is destructive, eg replacing a file that is now in the trash. */ + public isStale = (uuid: string, path: string): boolean => { + const change = this.recentChanges.get(uuid); + if (!change || change.expiresAt <= Date.now()) return false; + return change.path !== normalizePath(path); + }; + + private readonly throwIfStale = (uuid: string, path: string, itemType: 'file' | 'folder'): void => { + if (!this.isStale(uuid, path)) return; + webdavLogger.info(`[LOOKUP] Ignoring '${path}', it was moved or deleted moments ago`, { uuid }); + throw new StaleItemError(`${itemType === 'file' ? 'File' : 'Folder'} not found at path: ${path}`); + }; + + /** The API reads from replicas that can lag a moment behind a write, so a file this server + * just created can briefly look missing; remember it so that 404 is not trusted. */ + public rememberFileWrite = (path: string, item: DriveFileItem): void => { + const now = Date.now(); + for (const [writtenPath, write] of this.recentFileWrites) { + if (write.expiresAt <= now) this.recentFileWrites.delete(writtenPath); + } + this.recentFileWrites.set(path, { item, expiresAt: now + RECENT_WRITE_TRUST_MS }); + }; + + /** Only while the path cache still maps to the written uuid: our own DELETE and MOVE clear it. */ + private readonly getRecentFileWrite = ( + cached: DriveItemBD | undefined, + path: string, + error: unknown, + ): DriveFileItem | undefined => { + if (!cached || !ErrorUtils.isNotFoundError(error)) return; + + const write = this.recentFileWrites.get(path); + if (!write || write.expiresAt <= Date.now() || write.item.uuid !== cached.uuid) return; + if (this.isStale(write.item.uuid, path)) return; + + webdavLogger.info(`[LOOKUP] '${path}' was written moments ago, ignoring the API's not found`, { + uuid: cached.uuid, + }); + return write.item; + }; + /** Only drop the cached uuid when the API confirms the item is gone; evicting on a timeout * pushes every later request onto the slow path lookup. */ private readonly dropCacheIfGone = async (uuid: string, error: unknown): Promise => { @@ -19,6 +91,7 @@ export class DriveItemService { private readonly tryGetFileByUuid = async (cached: DriveItemBD, path: string): Promise => { try { const item = await DriveFileService.instance.getFileMetadata(cached.uuid); + this.throwIfStale(item.uuid, path, 'file'); await DriveItemRepository.instance.createOrUpdate([ { uuid: cached.uuid, @@ -30,10 +103,15 @@ export class DriveItemService { ]); return item; } catch (error) { - webdavLogger.warn(ErrorUtils.withRequestId('File metadata by uuid failed, falling back to path lookup', error), { - path, - uuid: cached.uuid, - }); + const recentWrite = this.getRecentFileWrite(cached, path, error); + if (recentWrite) return recentWrite; + + if (!(error instanceof StaleItemError)) { + webdavLogger.warn( + ErrorUtils.withRequestId('File metadata by uuid failed, falling back to path lookup', error), + { path, uuid: cached.uuid }, + ); + } await this.dropCacheIfGone(cached.uuid, error); } }; @@ -44,6 +122,7 @@ export class DriveItemService { ): Promise => { try { const item = await DriveFolderService.instance.getFolderMetaByUuid(cached.uuid); + this.throwIfStale(item.uuid, path, 'folder'); await DriveItemRepository.instance.createOrUpdate([ { uuid: cached.uuid, @@ -55,10 +134,12 @@ export class DriveItemService { ]); return item; } catch (error) { - webdavLogger.warn( - ErrorUtils.withRequestId('Folder metadata by uuid failed, falling back to path lookup', error), - { path, uuid: cached.uuid }, - ); + if (!(error instanceof StaleItemError)) { + webdavLogger.warn( + ErrorUtils.withRequestId('Folder metadata by uuid failed, falling back to path lookup', error), + { path, uuid: cached.uuid }, + ); + } await this.dropCacheIfGone(cached.uuid, error); } }; @@ -73,6 +154,7 @@ export class DriveItemService { try { const item = await DriveFileService.instance.getFileMetadataByPath(path); + this.throwIfStale(item.uuid, path, 'file'); await DriveItemRepository.instance.createOrUpdate([ { uuid: item.uuid, @@ -84,6 +166,9 @@ export class DriveItemService { ]); return item; } catch (error) { + const recentWrite = this.getRecentFileWrite(cached, path, error); + if (recentWrite) return recentWrite; + ErrorUtils.logIfUnexpected(webdavLogger, 'File lookup by path failed', error, { path }); throw ErrorUtils.toLookupError('file', path, error); } @@ -99,6 +184,7 @@ export class DriveItemService { try { const item = await DriveFolderService.instance.getFolderMetaByPath(path); + this.throwIfStale(item.uuid, path, 'folder'); await DriveItemRepository.instance.createOrUpdate([ { uuid: item.uuid, diff --git a/src/utils/webdav.utils.ts b/src/utils/webdav.utils.ts index 94890362..991fbcc7 100644 --- a/src/utils/webdav.utils.ts +++ b/src/utils/webdav.utils.ts @@ -118,6 +118,7 @@ export class WebDavUtils { }); webdavLogger.info(`[DELETE] [${driveItem.uuid}] ${type} trashed successfully`); } + DriveItemService.instance.rememberDeletion(driveItem.uuid); await DriveItemRepository.instance.delete([driveItem.uuid]); } diff --git a/src/webdav/handlers/MOVE.handler.ts b/src/webdav/handlers/MOVE.handler.ts index bb8671eb..e47d4c40 100644 --- a/src/webdav/handlers/MOVE.handler.ts +++ b/src/webdav/handlers/MOVE.handler.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import { DriveFileService } from '../../services/drive/drive-file.service'; import { DriveFolderService } from '../../services/drive/drive-folder.service'; import { DriveItemRepository } from '../../services/database/drive-item/drive-item.repository'; +import { DriveItemService } from '../../services/drive/drive-item.service'; import { WebDavMethodHandler } from '../../types/webdav.types'; import { NotFoundError, PreconditionFailedError } from '../../utils/errors.utils'; import { webdavLogger } from '../../utils/logger.utils'; @@ -96,6 +97,7 @@ export class MOVERequestHandler implements WebDavMethodHandler { } } + DriveItemService.instance.rememberMove(originalDriveItem.uuid, destinationResource.url); await DriveItemRepository.instance.delete([originalDriveItem.uuid]); await DriveItemRepository.instance.createOrUpdate([ { diff --git a/src/webdav/handlers/PROPFIND.handler.ts b/src/webdav/handlers/PROPFIND.handler.ts index 4cbec033..99df0416 100644 --- a/src/webdav/handlers/PROPFIND.handler.ts +++ b/src/webdav/handlers/PROPFIND.handler.ts @@ -4,6 +4,7 @@ import { DriveFileItem, DriveFolderItem } from '../../types/drive.types'; import { DriveItemBD } from '../../services/database/drive-item/drive-item.domain'; import { DriveItemRepository } from '../../services/database/drive-item/drive-item.repository'; import { DriveFolderService } from '../../services/drive/drive-folder.service'; +import { DriveItemService } from '../../services/drive/drive-item.service'; import { DriveUtils } from '../../utils/drive.utils'; import { FormatUtils } from '../../utils/format.utils'; import { Request, Response } from 'express'; @@ -99,6 +100,7 @@ export class PROPFINDRequestHandler implements WebDavMethodHandler { for (const folder of folderContent.folders) { const folderRelativePath = WebDavUtils.joinURL(relativePath, folder.plainName, '/'); + if (DriveItemService.instance.isStale(folder.uuid, folderRelativePath)) continue; xmlNodes.push( this.driveFolderItemToXMLNode( @@ -134,6 +136,7 @@ export class PROPFINDRequestHandler implements WebDavMethodHandler { relativePath, file.type ? `${file.plainName}.${file.type}` : file.plainName, ); + if (DriveItemService.instance.isStale(file.uuid, fileRelativePath)) continue; xmlNodes.push( this.driveFileItemToXMLNode( diff --git a/src/webdav/handlers/PUT.handler.ts b/src/webdav/handlers/PUT.handler.ts index 3d02e65b..433ad9ca 100644 --- a/src/webdav/handlers/PUT.handler.ts +++ b/src/webdav/handlers/PUT.handler.ts @@ -1,6 +1,7 @@ import { Request, Response } from 'express'; import { DriveFileService } from '../../services/drive/drive-file.service'; import { DriveItemRepository } from '../../services/database/drive-item/drive-item.repository'; +import { DriveItemService } from '../../services/drive/drive-item.service'; import { AuthService } from '../../services/auth.service'; import { WebDavMethodHandler } from '../../types/webdav.types'; import { ConflictError, ErrorUtils } from '../../utils/errors.utils'; @@ -12,6 +13,11 @@ import { WebDavFolderService } from '../../services/webdav/webdav-folder.service import { ThumbnailService } from '../../services/thumbnail.service'; import { FormatUtils } from '../../utils/format.utils'; import { UploadUtils } from '../../utils/upload.utils'; +import { AsyncUtils } from '../../utils/async.utils'; +import { DriveFileItem } from '../../types/drive.types'; +import { StorageTypes } from '@internxt/sdk/dist/drive'; + +const STALE_READ_RETRY_DELAYS_MS = [0, 500, 1000]; export class PUTRequestHandler implements WebDavMethodHandler { handle = async (req: Request, res: Response) => { @@ -110,22 +116,26 @@ export class PUTRequestHandler implements WebDavMethodHandler { let file; if (driveFileItem?.itemType === 'file') { + file = await this.replaceExistingFile(driveFileItem, filePayload, resource.url); + } else { try { - file = await DriveFileService.instance.replaceFile(driveFileItem.uuid, filePayload); + file = await DriveFileService.instance.createFile(filePayload); } catch (error) { - webdavLogger.warn( - ErrorUtils.withRequestId( - `[PUT] File replace failed for '${resource.url}', falling back to delete and create: ${ - error instanceof Error ? error.message : String(error) - }`, - error, - ), + if (!ErrorUtils.isAlreadyExistsError(error)) throw error; + + const existingFile = await this.findUnseenExistingFile( + filePayload.folderUuid, + { plainName: filePayload.plainName, type: fileType }, + resource.url, ); - await WebDavUtils.deleteOrTrashItem(driveFileItem); - file = await DriveFileService.instance.createFile(filePayload); + if (!existingFile) throw error; + + webdavLogger.info( + `[PUT] File '${resource.url}' already exists on Drive but the lookup missed it, it will be replaced...`, + ); + isReplacement = true; + file = await this.replaceExistingFile(existingFile, filePayload, resource.url); } - } else { - file = await DriveFileService.instance.createFile(filePayload); } timings.driveUpload = driveTimer.stop(); @@ -138,6 +148,7 @@ export class PUTRequestHandler implements WebDavMethodHandler { updatedAt: new Date(), }, ]); + DriveItemService.instance.rememberFileWrite(resource.url, file); const thumbnailTimer = CLIUtils.timer(); await ThumbnailService.instance.tryUploadThumbnail({ @@ -167,4 +178,56 @@ export class PUTRequestHandler implements WebDavMethodHandler { res.header('ETag', WebDavUtils.getItemETag(file)); res.status(statusCode).send(); }; + + private readonly replaceExistingFile = async ( + existingFile: DriveFileItem, + filePayload: StorageTypes.FileEntryByUuid, + url: string, + ): Promise => { + try { + return await DriveFileService.instance.replaceFile(existingFile.uuid, filePayload); + } catch (error) { + webdavLogger.warn( + ErrorUtils.withRequestId( + `[PUT] File replace failed for '${url}', falling back to delete and create: ${ + error instanceof Error ? error.message : String(error) + }`, + error, + ), + ); + await WebDavUtils.deleteOrTrashItem(existingFile); + return await this.createFileAfterTrashing(filePayload); + } + }; + + /** The create's 409 check reads a replica that can still see the file we just trashed. */ + private readonly createFileAfterTrashing = async ( + filePayload: StorageTypes.FileEntryByUuid, + ): Promise => { + let lastError: unknown; + for (const delayMs of STALE_READ_RETRY_DELAYS_MS) { + await AsyncUtils.sleep(delayMs); + try { + return await DriveFileService.instance.createFile(filePayload); + } catch (error) { + if (!ErrorUtils.isAlreadyExistsError(error)) throw error; + lastError = error; + } + } + throw lastError; + }; + + /** A 409 on create means the file exists even though the path lookup missed it, usually + * because the read replica has not caught up with a file created moments ago. */ + private readonly findUnseenExistingFile = async ( + folderUuid: string, + file: StorageTypes.FileStructure, + url: string, + ): Promise => { + for (const delayMs of STALE_READ_RETRY_DELAYS_MS) { + await AsyncUtils.sleep(delayMs); + const existingFile = await DriveFileService.instance.findExistentFile(folderUuid, file); + if (existingFile && !DriveItemService.instance.isStale(existingFile.uuid, url)) return existingFile; + } + }; } diff --git a/test/services/drive/drive-item.service.test.ts b/test/services/drive/drive-item.service.test.ts index a91ee778..65a6ed1b 100644 --- a/test/services/drive/drive-item.service.test.ts +++ b/test/services/drive/drive-item.service.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, vi } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; import { DriveItemService } from '../../../src/services/drive/drive-item.service'; import { DriveItemRepository } from '../../../src/services/database/drive-item/drive-item.repository'; import { DriveItemBD } from '../../../src/services/database/drive-item/drive-item.domain'; @@ -215,6 +215,92 @@ describe('Drive Item Service', () => { }); }); + describe('trusting a file this server just wrote', () => { + const cacheEntry = (path: string, uuid: string) => + new DriveItemBD({ uuid, path, type: 'file', createdAt: new Date(), updatedAt: new Date() }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test('when the API says a just-written file is missing, then the written file is returned and kept in cache', async () => { + const path = '/recent/just-written.txt'; + const written = newFileItem({ uuid: 'written-uuid' }); + sut.rememberFileWrite(path, written); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(cacheEntry(path, written.uuid)); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(404)); + const pathLookupSpy = vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath'); + const deleteSpy = vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + const result = await sut.getFileByPath(path); + + expect(result).toBe(written); + expect(pathLookupSpy).not.toHaveBeenCalled(); + expect(deleteSpy).not.toHaveBeenCalled(); + }); + + test('when the API finds a just-written file, then the API answer is returned', async () => { + const path = '/recent/found.txt'; + const written = newFileItem({ uuid: 'found-uuid' }); + const fromApi = newFileItem({ uuid: 'found-uuid' }); + sut.rememberFileWrite(path, written); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(cacheEntry(path, written.uuid)); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockResolvedValue(fromApi); + vi.spyOn(DriveItemRepository.instance, 'createOrUpdate').mockResolvedValue(undefined); + + const result = await sut.getFileByPath(path); + + expect(result).toBe(fromApi); + }); + + test('when the uuid lookup fails transiently and the path lookup says missing, then the written file is returned', async () => { + const path = '/recent/transient.txt'; + const written = newFileItem({ uuid: 'transient-uuid' }); + sut.rememberFileWrite(path, written); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(cacheEntry(path, written.uuid)); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(504)); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + + const result = await sut.getFileByPath(path); + + expect(result).toBe(written); + }); + + test('when the write is older than the trust window, then the not found is reported', async () => { + vi.useFakeTimers(); + const path = '/recent/expired.txt'; + const written = newFileItem({ uuid: 'expired-uuid' }); + sut.rememberFileWrite(path, written); + vi.advanceTimersByTime(10_001); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(cacheEntry(path, written.uuid)); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + }); + + test('when the path no longer maps to the written file, then the not found is reported', async () => { + const path = '/recent/replaced-elsewhere.txt'; + sut.rememberFileWrite(path, newFileItem({ uuid: 'old-uuid' })); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(cacheEntry(path, 'other-uuid')); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + }); + + test('when the written file was deleted through this server, then the not found is reported', async () => { + const path = '/recent/deleted.txt'; + sut.rememberFileWrite(path, newFileItem({ uuid: 'deleted-uuid' })); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(undefined); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + }); + }); + describe('getting a folder by path', () => { test('when the folder is in cache and the API responds, then the cached folder is returned', async () => { const path = '/test/folder/'; @@ -369,4 +455,119 @@ describe('Drive Item Service', () => { expect(deleteSpy).not.toHaveBeenCalled(); }); }); + describe('ignoring items this server just moved or deleted', () => { + test('when a path lookup returns a file deleted moments ago, then a not found error is thrown', async () => { + const path = '/stale/deleted.txt'; + const deleted = newFileItem({ uuid: 'stale-deleted-uuid' }); + sut.rememberDeletion(deleted.uuid); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(undefined); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockResolvedValue(deleted); + const createOrUpdateSpy = vi.spyOn(DriveItemRepository.instance, 'createOrUpdate'); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + expect(createOrUpdateSpy).not.toHaveBeenCalled(); + }); + + test('when a path lookup returns a file at the path it was moved away from, then a not found error is thrown', async () => { + const moved = newFileItem({ uuid: 'stale-moved-uuid' }); + sut.rememberMove(moved.uuid, '/stale/new-place.txt'); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(undefined); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockResolvedValue(moved); + + await expect(sut.getFileByPath('/stale/old-place.txt')).rejects.toBeInstanceOf(NotFoundError); + }); + + test('when a lookup returns a moved file at its new path, then the file is returned', async () => { + const moved = newFileItem({ uuid: 'fresh-moved-uuid' }); + sut.rememberMove(moved.uuid, '/stale/destination.txt'); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue(undefined); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockResolvedValue(moved); + vi.spyOn(DriveItemRepository.instance, 'createOrUpdate').mockResolvedValue(undefined); + + await expect(sut.getFileByPath('/stale/destination.txt')).resolves.toBe(moved); + }); + + test('when a cached uuid lookup returns a folder deleted moments ago, then a not found error is thrown', async () => { + const path = '/stale/deleted-folder'; + const deleted = newFolderItem({ uuid: 'stale-folder-uuid' }); + sut.rememberDeletion(deleted.uuid); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue( + new DriveItemBD({ uuid: deleted.uuid, path, type: 'folder', createdAt: new Date(), updatedAt: new Date() }), + ); + vi.spyOn(DriveFolderService.instance, 'getFolderMetaByUuid').mockResolvedValue(deleted); + vi.spyOn(DriveFolderService.instance, 'getFolderMetaByPath').mockResolvedValue(deleted); + const deleteSpy = vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFolderByPath(path)).rejects.toBeInstanceOf(NotFoundError); + expect(deleteSpy).toHaveBeenCalledWith([deleted.uuid]); + }); + + test('when a file written moments ago was then deleted, then the remembered write is not returned', async () => { + const path = '/stale/written-then-deleted.txt'; + const written = newFileItem({ uuid: 'written-then-deleted-uuid' }); + sut.rememberFileWrite(path, written); + sut.rememberDeletion(written.uuid); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue( + new DriveItemBD({ uuid: written.uuid, path, type: 'file', createdAt: new Date(), updatedAt: new Date() }), + ); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + }); + + test('when a file written moments ago was then moved, then the remembered write is not returned at the old path', async () => { + const path = '/stale/written-then-moved.txt'; + const written = newFileItem({ uuid: 'written-then-moved-uuid' }); + sut.rememberFileWrite(path, written); + sut.rememberMove(written.uuid, '/stale/moved-destination.txt'); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue( + new DriveItemBD({ uuid: written.uuid, path, type: 'file', createdAt: new Date(), updatedAt: new Date() }), + ); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(newApiError(404)); + vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + }); + + test('when a cached uuid lookup returns a file deleted moments ago, then it is not logged as an API failure', async () => { + const path = '/stale/deleted-cached.txt'; + const deleted = newFileItem({ uuid: 'stale-cached-file-uuid' }); + sut.rememberDeletion(deleted.uuid); + vi.spyOn(DriveItemRepository.instance, 'getByPath').mockResolvedValue( + new DriveItemBD({ uuid: deleted.uuid, path, type: 'file', createdAt: new Date(), updatedAt: new Date() }), + ); + vi.spyOn(DriveFileService.instance, 'getFileMetadata').mockResolvedValue(deleted); + vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockResolvedValue(deleted); + const deleteSpy = vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + + await expect(sut.getFileByPath(path)).rejects.toBeInstanceOf(NotFoundError); + + expect(deleteSpy).toHaveBeenCalledWith([deleted.uuid]); + expect(webdavLogger.warn).not.toHaveBeenCalled(); + expect(webdavLogger.info).toHaveBeenCalledWith( + expect.stringContaining('moved or deleted moments ago'), + expect.objectContaining({ uuid: deleted.uuid }), + ); + }); + + test('when the deletion is older than the trust window, then the item is no longer ignored', () => { + vi.useFakeTimers(); + sut.rememberDeletion('expired-deletion-uuid'); + vi.advanceTimersByTime(60_001); + + expect(sut.isStale('expired-deletion-uuid', '/stale/anything.txt')).toBe(false); + vi.useRealTimers(); + }); + + test('when a folder was moved, then a trailing slash in the looked up path does not matter', () => { + sut.rememberMove('slash-folder-uuid', '/stale/moved-folder/'); + + expect(sut.isStale('slash-folder-uuid', '/stale/moved-folder')).toBe(false); + expect(sut.isStale('slash-folder-uuid', '/stale/moved-folder/')).toBe(false); + expect(sut.isStale('slash-folder-uuid', '/stale/old-folder/')).toBe(true); + }); + }); }); diff --git a/test/utils/webdav.utils.test.ts b/test/utils/webdav.utils.test.ts index 17837d31..5866e154 100644 --- a/test/utils/webdav.utils.test.ts +++ b/test/utils/webdav.utils.test.ts @@ -404,6 +404,8 @@ describe('Webdav utils', () => { const trashItemsStub = vi.spyOn(TrashService.instance, 'trashItems').mockResolvedValue(); const deleteCacheSpy = vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); + const rememberDeletionSpy = vi.spyOn(DriveItemService.instance, 'rememberDeletion'); + await WebDavUtils.deleteOrTrashItem(fileItem); expect(trashItemsStub).toHaveBeenCalledWith({ @@ -411,6 +413,7 @@ describe('Webdav utils', () => { }); expect(deleteItemPermanentlyStub).not.toHaveBeenCalled(); expect(deleteCacheSpy).toHaveBeenCalledWith([fileItem.uuid]); + expect(rememberDeletionSpy).toHaveBeenCalledWith(fileItem.uuid); }); test('when permanent deletion is disabled for folders, then folders are trashed and cache is cleared', async () => { diff --git a/test/webdav/handlers/MOVE.handler.test.ts b/test/webdav/handlers/MOVE.handler.test.ts index 2f3badb7..4ea6ecba 100644 --- a/test/webdav/handlers/MOVE.handler.test.ts +++ b/test/webdav/handlers/MOVE.handler.test.ts @@ -12,6 +12,7 @@ import { DriveFolderService } from '../../../src/services/drive/drive-folder.ser import { DriveFileService } from '../../../src/services/drive/drive-file.service'; import { WebDavFolderService } from '../../../src/services/webdav/webdav-folder.service'; import { DriveItemRepository } from '../../../src/services/database/drive-item/drive-item.repository'; +import { DriveItemService } from '../../../src/services/drive/drive-item.service'; import { newFolderItem, newFileItem } from '../../fixtures/drive.fixture'; describe('MOVE request handler', () => { @@ -230,6 +231,7 @@ describe('MOVE request handler', () => { const moveFileStub = vi.spyOn(DriveFileService.instance, 'moveFile').mockResolvedValue(undefined); const deleteSpy = vi.spyOn(DriveItemRepository.instance, 'delete').mockResolvedValue(undefined); const createOrUpdateSpy = vi.spyOn(DriveItemRepository.instance, 'createOrUpdate').mockResolvedValue(undefined); + const rememberMoveSpy = vi.spyOn(DriveItemService.instance, 'rememberMove'); await sut.handle(request, response); @@ -240,6 +242,7 @@ describe('MOVE request handler', () => { }); expect(deleteSpy).toHaveBeenCalledWith(['file-uuid']); expect(createOrUpdateSpy).toHaveBeenCalled(); + expect(rememberMoveSpy).toHaveBeenCalledWith('file-uuid', '/dest/file.txt'); expect(response.status).toHaveBeenCalledWith(201); }); }); diff --git a/test/webdav/handlers/PROPFIND.handler.test.ts b/test/webdav/handlers/PROPFIND.handler.test.ts index 006e141f..04edacbb 100644 --- a/test/webdav/handlers/PROPFIND.handler.test.ts +++ b/test/webdav/handlers/PROPFIND.handler.test.ts @@ -6,7 +6,7 @@ import { DriveFolderService } from '../../../src/services/drive/drive-folder.ser import { DriveItemService } from '../../../src/services/drive/drive-item.service'; import { DriveItemRepository } from '../../../src/services/database/drive-item/drive-item.repository'; import { UserSettingsFixture } from '../../fixtures/auth.fixture'; -import { newFileItem, newFolderItem, newPaginatedFolder } from '../../fixtures/drive.fixture'; +import { newFileItem, newFolderItem, newPaginatedFile, newPaginatedFolder } from '../../fixtures/drive.fixture'; import { createWebDavRequestFixture, createWebDavResponseFixture, @@ -346,4 +346,33 @@ describe('PROPFIND request handler', () => { expect(getRequestedResourceStub).toHaveBeenCalledOnce(); expect(getAndSearchItemFromResourceStub).toHaveBeenCalledOnce(); }); + test('when a listing returns items moved or deleted moments ago, then the server leaves them out', async () => { + const requestedFolderResource = getRequestedFolderResource({ parentFolder: '/', folderName: 'folder_b' }); + const request = createWebDavRequestFixture({ method: 'PROPFIND', url: '/folder_b/', user: UserSettingsFixture }); + const response = createWebDavResponseFixture({}); + + const deletedFolder = newPaginatedFolder({ uuid: 'DELETED_FOLDER_UUID', plainName: 'deleted_child' }); + const keptFolder = newPaginatedFolder({ uuid: 'KEPT_FOLDER_UUID', plainName: 'kept_child' }); + const movedFile = newPaginatedFile({ uuid: 'MOVED_FILE_UUID', plainName: 'moved', type: 'txt' }); + DriveItemService.instance.rememberDeletion(deletedFolder.uuid); + DriveItemService.instance.rememberMove(movedFile.uuid, '/elsewhere/moved.txt'); + + vi.spyOn(WebDavUtils, 'getRequestedResource').mockResolvedValue(requestedFolderResource); + vi.spyOn(WebDavUtils, 'getDriveItemFromResource').mockResolvedValue( + newFolderItem({ name: requestedFolderResource.name }), + ); + vi.spyOn(DriveFolderService.instance, 'getFolderContent').mockResolvedValue({ + files: [movedFile], + folders: [deletedFolder, keptFolder], + }); + const createOrUpdateSpy = vi.spyOn(DriveItemRepository.instance, 'createOrUpdate').mockResolvedValue(undefined); + + await sut.handle(request, response); + + const xml = vi.mocked(response.send).mock.calls[0][0] as string; + expect(xml).toContain('kept_child'); + expect(xml).not.toContain('deleted_child'); + expect(xml).not.toContain('moved.txt'); + expect(createOrUpdateSpy).toHaveBeenCalledWith([expect.objectContaining({ uuid: 'KEPT_FOLDER_UUID' })]); + }); }); diff --git a/test/webdav/handlers/PUT.handler.test.ts b/test/webdav/handlers/PUT.handler.test.ts index cad2c12c..357b73c6 100644 --- a/test/webdav/handlers/PUT.handler.test.ts +++ b/test/webdav/handlers/PUT.handler.test.ts @@ -8,6 +8,7 @@ import { getRequestedFolderResource, } from '../../fixtures/webdav.fixture'; import { DriveFileService } from '../../../src/services/drive/drive-file.service'; +import { DriveItemService } from '../../../src/services/drive/drive-item.service'; import { AuthService } from '../../../src/services/auth.service'; import { NetworkFacade } from '../../../src/services/network/network-facade.service'; import { PUTRequestHandler } from '../../../src/webdav/handlers/PUT.handler'; @@ -19,6 +20,7 @@ import { CLIUtils } from '../../../src/utils/cli.utils'; import { UsageService } from '../../../src/services/usage.service'; import { webdavLogger } from '../../../src/utils/logger.utils'; import { AxiosResponseError } from '@internxt/sdk/dist/shared/types/errors'; +import { AsyncUtils } from '../../../src/utils/async.utils'; describe('PUT request handler', () => { let networkFacade: NetworkFacade; @@ -182,6 +184,7 @@ describe('PUT request handler', () => { const uploadStub = vi.spyOn(networkFacade, 'uploadFile').mockResolvedValue('uploaded-file-id'); const createDriveFileStub = vi.spyOn(DriveFileService.instance, 'createFile').mockResolvedValue(fileFixture); const replaceDriveFileStub = vi.spyOn(DriveFileService.instance, 'replaceFile').mockResolvedValue(fileFixture); + const rememberFileWriteStub = vi.spyOn(DriveItemService.instance, 'rememberFileWrite'); await sut.handle(request, response); expect(response.status).toHaveBeenCalledWith(204); @@ -202,6 +205,7 @@ describe('PUT request handler', () => { ); expect(createDriveFileStub).not.toHaveBeenCalled(); expect(deleteDriveFileStub).not.toHaveBeenCalled(); + expect(rememberFileWriteStub).toHaveBeenCalledWith(requestedFileResource.url, fileFixture); }); test('when replacing an existing file fails, then the request id is logged and the file is deleted and created', async () => { @@ -254,4 +258,171 @@ describe('PUT request handler', () => { expect(deleteDriveFileStub).toHaveBeenCalledWith(fileFixture); expect(createDriveFileStub).toHaveBeenCalledOnce(); }); + describe('when replacing fails and the trashed file still conflicts with the new one', () => { + const conflictError = new AxiosResponseError('Request failed with status code 409', 'POST /files', { + status: 409, + data: { message: 'File already exists' }, + headers: {}, + statusText: 'Conflict', + // @ts-expect-error partial AxiosResponse fixture, only the fields read by AxiosResponseError are needed + config: {}, + }); + + const setupReplaceFallback = () => { + const requestedFileResource: WebDavRequestedResource = getRequestedFileResource(); + const requestedParentFolderResource: WebDavRequestedResource = getRequestedFolderResource({ + parentFolder: '/', + folderName: '', + }); + const folderFixture = newFolderItem({ name: requestedParentFolderResource.name }); + const fileFixture = newFileItem({ folderUuid: folderFixture.uuid, size: 0, fileId: undefined }); + + const request = createWebDavRequestFixture({ + method: 'PUT', + url: requestedFileResource.url, + headers: { 'content-length': '0' }, + }); + const response = createWebDavResponseFixture({ + status: vi.fn().mockReturnValue({ send: vi.fn() }), + }); + + vi.spyOn(WebDavUtils, 'getRequestedResource') + .mockResolvedValueOnce(requestedFileResource) + .mockResolvedValueOnce(requestedParentFolderResource); + vi.spyOn(WebDavUtils, 'getDriveItemFromResource').mockResolvedValueOnce(fileFixture); + vi.spyOn(WebDavUtils, 'getDriveFolderFromResource').mockResolvedValue(folderFixture); + vi.spyOn(AuthService.instance, 'getAuthDetails').mockResolvedValue(UserCredentialsFixture); + vi.spyOn(WebDavUtils, 'deleteOrTrashItem').mockResolvedValue(); + vi.spyOn(DriveFileService.instance, 'replaceFile').mockRejectedValue( + new Error('Request failed with status code 500'), + ); + vi.spyOn(AsyncUtils, 'sleep').mockResolvedValue(); + + return { request, response, fileFixture }; + }; + + test('when the conflict clears on a retry, then the file is created', async () => { + const { request, response, fileFixture } = setupReplaceFallback(); + const createDriveFileStub = vi + .spyOn(DriveFileService.instance, 'createFile') + .mockRejectedValueOnce(conflictError) + .mockResolvedValueOnce(fileFixture); + + await sut.handle(request, response); + + expect(response.status).toHaveBeenCalledWith(204); + expect(createDriveFileStub).toHaveBeenCalledTimes(2); + }); + + test('when the conflict never clears, then the conflict is returned', async () => { + const { request, response } = setupReplaceFallback(); + const createDriveFileStub = vi.spyOn(DriveFileService.instance, 'createFile').mockRejectedValue(conflictError); + + await expect(sut.handle(request, response)).rejects.toBe(conflictError); + + expect(createDriveFileStub).toHaveBeenCalledTimes(3); + }); + }); + + describe('when the file was not found at the path but creating it conflicts', () => { + const conflictError = new AxiosResponseError('Request failed with status code 409', 'POST /files', { + status: 409, + data: { message: 'File already exists' }, + headers: {}, + statusText: 'Conflict', + // @ts-expect-error partial AxiosResponse fixture, only the fields read by AxiosResponseError are needed + config: {}, + }); + + const setupConflictingUpload = () => { + const requestedFileResource: WebDavRequestedResource = getRequestedFileResource(); + const requestedParentFolderResource: WebDavRequestedResource = getRequestedFolderResource({ + parentFolder: '/', + folderName: '', + }); + const folderFixture = newFolderItem({ name: requestedParentFolderResource.name }); + const existingFileFixture = newFileItem({ folderUuid: folderFixture.uuid }); + + const request = createWebDavRequestFixture({ + method: 'PUT', + url: requestedFileResource.url, + headers: { 'content-length': '100' }, + }); + const response = createWebDavResponseFixture({ + status: vi.fn().mockReturnValue({ send: vi.fn() }), + }); + + vi.spyOn(WebDavUtils, 'getRequestedResource') + .mockResolvedValueOnce(requestedFileResource) + .mockResolvedValueOnce(requestedParentFolderResource); + vi.spyOn(WebDavUtils, 'getDriveItemFromResource').mockResolvedValue(undefined); + vi.spyOn(WebDavUtils, 'getDriveFolderFromResource').mockResolvedValue(folderFixture); + vi.spyOn(AuthService.instance, 'getAuthDetails').mockResolvedValue(UserCredentialsFixture); + vi.spyOn(networkFacade, 'uploadFile').mockResolvedValue('uploaded-file-id'); + vi.spyOn(AsyncUtils, 'sleep').mockResolvedValue(); + + return { request, response, requestedFileResource, folderFixture, existingFileFixture }; + }; + + test('when the existing file is found, then the server replaces it', async () => { + const { request, response, requestedFileResource, folderFixture, existingFileFixture } = setupConflictingUpload(); + const createDriveFileStub = vi.spyOn(DriveFileService.instance, 'createFile').mockRejectedValue(conflictError); + const findExistentFileStub = vi + .spyOn(DriveFileService.instance, 'findExistentFile') + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(existingFileFixture); + const replaceDriveFileStub = vi + .spyOn(DriveFileService.instance, 'replaceFile') + .mockResolvedValue(existingFileFixture); + + await sut.handle(request, response); + + expect(response.status).toHaveBeenCalledWith(204); + expect(createDriveFileStub).toHaveBeenCalledOnce(); + expect(findExistentFileStub).toHaveBeenCalledTimes(2); + expect(findExistentFileStub).toHaveBeenCalledWith(folderFixture.uuid, { + plainName: requestedFileResource.path.name, + type: requestedFileResource.path.ext.replace('.', ''), + }); + expect(replaceDriveFileStub).toHaveBeenCalledWith( + existingFileFixture.uuid, + expect.objectContaining({ fileId: 'uploaded-file-id', size: 100 }), + ); + }); + + test('when the existing file never shows up, then the conflict is returned', async () => { + const { request, response } = setupConflictingUpload(); + vi.spyOn(DriveFileService.instance, 'createFile').mockRejectedValue(conflictError); + const findExistentFileStub = vi.spyOn(DriveFileService.instance, 'findExistentFile').mockResolvedValue(undefined); + const replaceDriveFileStub = vi.spyOn(DriveFileService.instance, 'replaceFile'); + + await expect(sut.handle(request, response)).rejects.toBe(conflictError); + + expect(findExistentFileStub).toHaveBeenCalledTimes(3); + expect(replaceDriveFileStub).not.toHaveBeenCalled(); + }); + + test('when the only file found was deleted moments ago, then the conflict is returned', async () => { + const { request, response, existingFileFixture } = setupConflictingUpload(); + DriveItemService.instance.rememberDeletion(existingFileFixture.uuid); + vi.spyOn(DriveFileService.instance, 'createFile').mockRejectedValue(conflictError); + vi.spyOn(DriveFileService.instance, 'findExistentFile').mockResolvedValue(existingFileFixture); + const replaceDriveFileStub = vi.spyOn(DriveFileService.instance, 'replaceFile'); + + await expect(sut.handle(request, response)).rejects.toBe(conflictError); + + expect(replaceDriveFileStub).not.toHaveBeenCalled(); + }); + + test('when creating the file fails for another reason, then no existing file is looked up', async () => { + const { request, response } = setupConflictingUpload(); + const serverError = new Error('Request failed with status code 500'); + vi.spyOn(DriveFileService.instance, 'createFile').mockRejectedValue(serverError); + const findExistentFileStub = vi.spyOn(DriveFileService.instance, 'findExistentFile'); + + await expect(sut.handle(request, response)).rejects.toBe(serverError); + + expect(findExistentFileStub).not.toHaveBeenCalled(); + }); + }); });