From 44a9efa667df2f3b4ed8ed97acb9b40f0d54dc2e Mon Sep 17 00:00:00 2001 From: Yaroslav Serhieiev Date: Thu, 23 May 2024 12:00:57 +0300 Subject: [PATCH] fix: attachment fs operations (#43) --- .../attachment-handlers/moveHandler.ts | 7 +-- .../attachment-handlers/placeAttachment.ts | 3 +- src/utils/fastMove.test.ts | 55 +++++++++++++++++++ src/utils/fastMove.ts | 13 +++++ src/utils/getFullExtension.test.ts | 33 +++++++++++ src/utils/getFullExtension.ts | 10 ++++ src/utils/index.ts | 2 + 7 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 src/utils/fastMove.test.ts create mode 100644 src/utils/fastMove.ts create mode 100644 src/utils/getFullExtension.test.ts create mode 100644 src/utils/getFullExtension.ts diff --git a/src/runtime/attachment-handlers/moveHandler.ts b/src/runtime/attachment-handlers/moveHandler.ts index ed2cb434..adb3f8c4 100644 --- a/src/runtime/attachment-handlers/moveHandler.ts +++ b/src/runtime/attachment-handlers/moveHandler.ts @@ -1,13 +1,10 @@ -import path from 'node:path'; -import fs from 'node:fs/promises'; - import type { FileAttachmentHandler } from '../types'; +import { fastMove } from '../../utils'; import { placeAttachment } from './placeAttachment'; export const moveHandler: FileAttachmentHandler = async (context) => { const destination = placeAttachment(context); - await fs.mkdir(path.dirname(destination), { recursive: true }); - await fs.rename(context.sourcePath, destination); + await fastMove(context.sourcePath, destination); return destination; }; diff --git a/src/runtime/attachment-handlers/placeAttachment.ts b/src/runtime/attachment-handlers/placeAttachment.ts index 6f1f8654..5a71d589 100644 --- a/src/runtime/attachment-handlers/placeAttachment.ts +++ b/src/runtime/attachment-handlers/placeAttachment.ts @@ -2,9 +2,10 @@ import path from 'node:path'; import { randomUUID } from 'node:crypto'; import type { AttachmentContext, ContentAttachmentContext, FileAttachmentContext } from '../types'; +import { getFullExtension } from '../../utils'; export function placeAttachment(context: AttachmentContext): string { const { outDir, name, sourcePath } = context as FileAttachmentContext & ContentAttachmentContext; const fileName = name || sourcePath || ''; - return path.join(outDir, randomUUID() + path.extname(fileName)); + return path.join(outDir, randomUUID() + getFullExtension(fileName)); } diff --git a/src/utils/fastMove.test.ts b/src/utils/fastMove.test.ts new file mode 100644 index 00000000..99afad60 --- /dev/null +++ b/src/utils/fastMove.test.ts @@ -0,0 +1,55 @@ +import os from 'node:os'; +import path from 'node:path'; +import fs from 'node:fs/promises'; + +import { fastMove } from './fastMove'; + +describe('fastMove', () => { + let rootDirectory: string; + let source: string; + let destination: string; + + beforeEach(() => { + jest.spyOn(fs, 'copyFile'); + jest.spyOn(fs, 'rm'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + beforeEach(async () => { + rootDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'fastmove-')); + source = path.join(rootDirectory, 'source.txt'); + destination = path.join(rootDirectory, 'destination.txt'); + await fs.writeFile(source, 'Test content'); + }); + + afterEach(async () => { + await fs.rm(rootDirectory, { recursive: true }); + }); + + it('should move the file to the destination', async () => { + await fastMove(source, destination); + + const movedFile = await fs.readFile(destination, 'utf8'); + expect(movedFile).toBe('Test content'); + await expect(fs.access(source)).rejects.toThrow(); + expect(fs.copyFile).not.toHaveBeenCalled(); + expect(fs.rm).not.toHaveBeenCalled(); + }); + + it('should handle cross-device file movement', async () => { + jest + .spyOn(fs, 'rename') + .mockRejectedValueOnce({ code: 'EXDEV', message: 'cross-device link not permitted' }); + + await fastMove(source, destination); + + const movedFile = await fs.readFile(destination, 'utf8'); + expect(movedFile).toBe('Test content'); + await expect(fs.access(source)).rejects.toThrow(); + expect(fs.copyFile).toHaveBeenCalled(); + expect(fs.rm).toHaveBeenCalled(); + }); +}); diff --git a/src/utils/fastMove.ts b/src/utils/fastMove.ts new file mode 100644 index 00000000..82dfa0ee --- /dev/null +++ b/src/utils/fastMove.ts @@ -0,0 +1,13 @@ +import path from 'node:path'; +import fs from 'node:fs/promises'; + +export async function fastMove(source: string, destination: string) { + await fs.mkdir(path.dirname(destination), { recursive: true }); + + try { + await fs.rename(source, destination); + } catch { + await fs.copyFile(source, destination); + await fs.rm(source, { force: true }); + } +} diff --git a/src/utils/getFullExtension.test.ts b/src/utils/getFullExtension.test.ts new file mode 100644 index 00000000..9ea29b3b --- /dev/null +++ b/src/utils/getFullExtension.test.ts @@ -0,0 +1,33 @@ +import { getFullExtension } from './getFullExtension'; + +describe('getFullExtension', () => { + it('should return the full extension for a file with multiple dots', () => { + const filePath = 'example.viewhierarchy.zip'; + const extension = getFullExtension(filePath); + expect(extension).toBe('.viewhierarchy.zip'); + }); + + it('should return the extension for a file with a single dot', () => { + const filePath = 'example.txt'; + const extension = getFullExtension(filePath); + expect(extension).toBe('.txt'); + }); + + it('should return the extension for a file starting with a dot', () => { + const filePath = '.gitignore'; + const extension = getFullExtension(filePath); + expect(extension).toBe('.gitignore'); + }); + + it('should return an empty string for a file without an extension', () => { + const filePath = 'example'; + const extension = getFullExtension(filePath); + expect(extension).toBe(''); + }); + + it('should return an empty string for empty or dot paths', () => { + expect(getFullExtension('')).toBe(''); + expect(getFullExtension('.')).toBe(''); + expect(getFullExtension('..')).toBe(''); + }); +}); diff --git a/src/utils/getFullExtension.ts b/src/utils/getFullExtension.ts new file mode 100644 index 00000000..89899519 --- /dev/null +++ b/src/utils/getFullExtension.ts @@ -0,0 +1,10 @@ +import path from 'node:path'; + +export function getFullExtension(filePath: string) { + if (!filePath || filePath === '.' || filePath === '..') return ''; + + const fileName = path.basename(filePath); + const lastDotIndex = fileName.indexOf('.'); + + return lastDotIndex === -1 ? '' : fileName.slice(lastDotIndex); +} diff --git a/src/utils/index.ts b/src/utils/index.ts index 05fdab30..19d9eeb5 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -3,7 +3,9 @@ export * from './asArray'; export * from './asMaybeArray'; export * from './autoIndent'; export * from './compactObject'; +export * from './fastMove'; export * from './FileNavigator'; +export * from './getFullExtension'; export * from './getStatusDetails'; export * from './hijackFunction'; export * from './importFrom';