test: Add more tests

packages/modules/apps/backend/src/__tests__/app-artifact.service.test.ts

@@ -1,5 +1,6 @@
 import { Logger } from '@n8n/backend-common';
 import { DbLockService, type OperationContext } from '@n8n/db';
+import { BadRequestError, ConflictError, NotFoundError, UnexpectedError } from '@n8n/errors';
 import { mock } from 'vitest-mock-extended';
 
 import type { AppArtifact } from '../app-artifact.entity';
@@ -39,6 +40,93 @@ function createService() {
 }
 
 describe('AppArtifactService', () => {
+	afterEach(() => vi.useRealTimers());
+
+	it('rejects an oversized artifact before reserving storage', async () => {
+		const { service, repository, blobs } = createService();
+
+		await expect(service.upload(Buffer.alloc(101))).rejects.toThrow(BadRequestError);
+
+		expect(repository.reserve).not.toHaveBeenCalled();
+		expect(blobs.write).not.toHaveBeenCalled();
+	});
+
+	it('rejects an upload that would exceed the total quota', async () => {
+		const { service, repository, blobs } = createService();
+		repository.totalSize.mockResolvedValue(901);
+
+		await expect(service.upload(Buffer.alloc(100))).rejects.toThrow(BadRequestError);
+
+		expect(repository.reserve).not.toHaveBeenCalled();
+		expect(blobs.write).not.toHaveBeenCalled();
+	});
+
+	it('accepts an upload at the artifact-size and total-quota limits', async () => {
+		const { service, repository, artifact } = createService();
+		repository.totalSize.mockResolvedValue(900);
+
+		expect(await service.upload(Buffer.alloc(100))).toEqual(artifact);
+	});
+
+	it('discards an upload that expires while bytes are written', async () => {
+		vi.useFakeTimers({ toFake: ['Date'] });
+		const now = new Date('2026-01-01T00:00:00Z');
+		vi.setSystemTime(now);
+		const { service, blobs, artifact } = createService();
+		blobs.write.mockImplementation(async () => {
+			vi.setSystemTime(now.getTime() + 31 * 60 * 1000);
+		});
+
+		await expect(service.upload(Buffer.from('test'))).rejects.toThrow(ConflictError);
+
+		expect(blobs.delete).toHaveBeenCalledWith(artifact);
+	});
+
+	it('rejects an upload that can no longer be completed', async () => {
+		const { service, repository, artifact, ctx } = createService();
+		repository.completeUpload.mockResolvedValue(false);
+
+		await expect(service.completeUpload(artifact, ctx)).rejects.toThrow(ConflictError);
+	});
+
+	it('does not expose bytes from a pending upload', async () => {
+		const { service, blobs, artifact } = createService();
+
+		await expect(service.read(artifact.id)).rejects.toThrow(NotFoundError);
+
+		expect(blobs.read).not.toHaveBeenCalled();
+	});
+
+	it('returns completed artifact bytes', async () => {
+		const { service, blobs, artifact } = createService();
+		artifact.uploadStatus = 'completed';
+		const body = Buffer.from('test');
+		blobs.read.mockResolvedValue(body);
+
+		expect(await service.read(artifact.id)).toEqual(body);
+	});
+
+	it.each([
+		{ body: null, error: NotFoundError, reason: 'missing' },
+		{ body: Buffer.from('bad'), error: UnexpectedError, reason: 'the wrong size' },
+	])('rejects completed content that is $reason', async ({ body, error }) => {
+		const { service, blobs, artifact } = createService();
+		artifact.uploadStatus = 'completed';
+		blobs.read.mockResolvedValue(body);
+
+		await expect(service.read(artifact.id)).rejects.toThrow(error);
+	});
+
+	it('rejects an oversized stored artifact before loading its bytes', async () => {
+		const { service, blobs, artifact } = createService();
+		artifact.uploadStatus = 'completed';
+		artifact.sizeBytes = 101;
+
+		await expect(service.read(artifact.id)).rejects.toThrow(BadRequestError);
+
+		expect(blobs.read).not.toHaveBeenCalled();
+	});
+
 	it.each([
 		{ operation: 'upload', reservation: 'missing' },
 		{ operation: 'upload', reservation: 'reused' },
@@ -97,4 +185,40 @@ describe('AppArtifactService', () => {
 		expect(blobs.delete).not.toHaveBeenCalled();
 		expect(repository.removeUnused).not.toHaveBeenCalled();
 	});
+
+	it('keeps a pending upload that is still within its upload lifetime', async () => {
+		const { service, repository, blobs, artifact } = createService();
+		repository.findUnused.mockResolvedValueOnce([artifact]);
+
+		await service.cleanup();
+
+		expect(blobs.delete).not.toHaveBeenCalled();
+		expect(repository.removeUnused).not.toHaveBeenCalled();
+	});
+
+	it('removes an expired pending upload', async () => {
+		const { service, repository, blobs, artifact, ctx } = createService();
+		artifact.createdAt = new Date(Date.now() - 31 * 60 * 1000);
+		repository.findUnused.mockResolvedValueOnce([artifact]);
+
+		await service.cleanup();
+
+		expect(blobs.delete).toHaveBeenCalledWith(artifact);
+		expect(repository.removeUnused).toHaveBeenCalledWith(artifact, ctx);
+	});
+
+	it('retains a failed deletion for retry and cleans the next artifact', async () => {
+		const { service, repository, blobs, artifact, ctx } = createService();
+		artifact.uploadStatus = 'completed';
+		const next = mock<AppArtifact>({ ...artifact, id: 2, storageKey: 'apps/artifacts/next.tgz' });
+		repository.findUnused.mockResolvedValueOnce([artifact, next]);
+		repository.findArtifact.mockImplementation(async (id) =>
+			id === artifact.id ? artifact : next,
+		);
+		blobs.delete.mockRejectedValueOnce(new Error('Storage is unavailable.'));
+
+		await service.cleanup();
+
+		expect(repository.removeUnused).toHaveBeenCalledExactlyOnceWith(next, ctx);
+	});
 });

packages/modules/apps/backend/src/__tests__/app-blob-store.test.ts

@@ -1,7 +1,13 @@
 import { AzureBlobConfig, ObjectStoreConfig } from '@n8n/blob-storage';
 import { ObjectStoreService } from '@n8n/blob-storage/object-store';
 import { AzureBlobService } from '@n8n/blob-storage/azure-blob';
-import { BinaryDataRepository, DbLockService, type ExecutionDataStorageLocation } from '@n8n/db';
+import {
+	BinaryDataRepository,
+	DbLock,
+	DbLockService,
+	type ExecutionDataStorageLocation,
+	type OperationContext,
+} from '@n8n/db';
 import { Container } from '@n8n/di';
 import { FsByteStoreService, StorageConfig } from 'n8n-core';
 import { mock } from 'vitest-mock-extended';
@@ -11,14 +17,19 @@ import { AppBlobStore } from '../storage/app-blob-store';
 
 function createStore(mode: ExecutionDataStorageLocation = 'fs') {
 	const fs = mock<FsByteStoreService>();
+	const binaryData = mock<BinaryDataRepository>();
+	const artifacts = mock<AppArtifactRepository>();
+	const locks = mock<DbLockService>();
+	const ctx = mock<OperationContext>();
+	locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx));
 	const store = new AppBlobStore(
 		fs,
 		mock<StorageConfig>({ modeTag: mode }),
-		mock<BinaryDataRepository>(),
-		mock<AppArtifactRepository>(),
-		mock<DbLockService>(),
+		binaryData,
+		artifacts,
+		locks,
 	);
-	return { store, fs };
+	return { store, fs, binaryData, artifacts, locks, ctx };
 }
 
 beforeEach(() => {
@@ -95,3 +106,49 @@ it('writes, reads, and deletes Azure bytes through the shared store', async () =
 	expect(client.delete).toHaveBeenCalledWith(blob.storageKey);
 	expect(await store.read(blob)).toBeNull();
 });
+
+it('writes database content under the storage lock', async () => {
+	const { store, artifacts, locks, ctx } = createStore('db');
+	const blob = { ...store.newBlob(), id: 1 };
+	const body = Buffer.from('content');
+
+	await store.write(blob, body);
+
+	expect(locks.withLockContext).toHaveBeenCalledWith(DbLock.APPS_STORAGE, expect.any(Function));
+	expect(artifacts.storeDatabaseContent).toHaveBeenCalledWith(blob, body, ctx);
+});
+
+it('reads and deletes filesystem content after new writes switch to the database', async () => {
+	const { store, fs } = createStore('db');
+	const blob = { storedAt: 'fs', storageKey: 'apps/artifacts/existing.tgz' } as const;
+	const body = Buffer.from('existing content');
+	fs.read.mockResolvedValue(body);
+
+	expect(await store.read(blob)).toEqual(body);
+	await store.delete(blob);
+
+	expect(fs.delete).toHaveBeenCalledWith([blob.storageKey]);
+});
+
+it('reads and deletes database content after new writes switch to the filesystem', async () => {
+	const { store, binaryData } = createStore('fs');
+	const blob = { storedAt: 'db', storageKey: '11111111-1111-4111-8111-111111111111' } as const;
+	const body = Buffer.from('existing content');
+	binaryData.findContentByFileId.mockResolvedValue(body);
+
+	expect(await store.read(blob)).toEqual(body);
+	await store.delete(blob);
+
+	expect(binaryData.deleteByFileIds).toHaveBeenCalledWith([blob.storageKey]);
+});
+
+it('fails startup when the selected backend cannot connect', async () => {
+	const error = new Error('Storage is unavailable.');
+	const client = mock<ObjectStoreService>();
+	client.checkConnection.mockRejectedValue(error);
+	Container.set(ObjectStoreConfig, mock<ObjectStoreConfig>({ bucket: { name: 'apps' } }));
+	Container.set(ObjectStoreService, client);
+	const { store } = createStore('s3');
+
+	await expect(store.init()).rejects.toBe(error);
+});

packages/modules/apps/backend/src/__tests__/app-version.service.test.ts

@@ -0,0 +1,192 @@
+import { Logger } from '@n8n/backend-common';
+import { DbLockService, type OperationContext } from '@n8n/db';
+import { BadRequestError, ConflictError, NotFoundError } from '@n8n/errors';
+import { mock } from 'vitest-mock-extended';
+
+import { AppRetentionService } from '../app-retention.service';
+import type { AppVersion } from '../app-version.entity';
+import { AppVersionRepository } from '../app-version.repository';
+import { AppVersionService } from '../app-version.service';
+import type { App } from '../app.entity';
+import { AppRepository } from '../app.repository';
+import { AppSourceVersionStore, AppBuildResultStore } from '../storage/app-version-stores';
+
+const appId = '11111111-1111-4111-8111-111111111111';
+const versionId = '22222222-2222-4222-8222-222222222222';
+const files = new Map([['index.html', Buffer.from('<html/>')]]);
+
+function createService() {
+	const apps = mock<AppRepository>();
+	const versions = mock<AppVersionRepository>();
+	const sourceVersions = mock<AppSourceVersionStore>();
+	const buildResults = mock<AppBuildResultStore>();
+	const retention = mock<AppRetentionService>();
+	const locks = mock<DbLockService>();
+	const ctx = mock<OperationContext>();
+	locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx));
+	const version = mock<AppVersion>({
+		id: versionId,
+		appId,
+		label: 'Original',
+		buildStatus: 'queued',
+	});
+	apps.findInProject.mockResolvedValue(mock<App>({ id: appId, draftVersionId: versionId }));
+	versions.findVersionById.mockResolvedValue(version);
+	sourceVersions.createVersion.mockImplementation(async (data) => {
+		versions.findVersionById.mockResolvedValue(
+			mock<AppVersion>({ ...version, id: data.id, label: data.label }),
+		);
+	});
+	const service = new AppVersionService(
+		apps,
+		versions,
+		sourceVersions,
+		buildResults,
+		retention,
+		locks,
+		mock<Logger>(),
+	);
+	return { service, apps, versions, sourceVersions, buildResults, retention, version };
+}
+
+describe('AppVersionService', () => {
+	it('does not read source files from another app', async () => {
+		const { service, versions, sourceVersions } = createService();
+		versions.findVersionById.mockResolvedValue(
+			mock<AppVersion>({ id: versionId, appId: 'other-app' }),
+		);
+
+		await expect(service.readSource('project', appId, versionId)).rejects.toThrow(NotFoundError);
+
+		expect(sourceVersions.readSource).not.toHaveBeenCalled();
+	});
+
+	it('does not change a label on a version from another app', async () => {
+		const { service, versions } = createService();
+		versions.findVersionById.mockResolvedValue(
+			mock<AppVersion>({ id: versionId, appId: 'other-app' }),
+		);
+
+		await expect(service.setLabel('project', appId, versionId, 'Updated')).rejects.toThrow(
+			NotFoundError,
+		);
+
+		expect(versions.setLabel).not.toHaveBeenCalled();
+	});
+
+	it('does not read a bundle when the app is outside the requested project', async () => {
+		const { service, apps, buildResults } = createService();
+		apps.findInProject.mockResolvedValue(null);
+
+		await expect(service.readBundle('other-project', appId, versionId)).rejects.toThrow(
+			NotFoundError,
+		);
+
+		expect(buildResults.readBundle).not.toHaveBeenCalled();
+	});
+
+	it.each([
+		{ appId: 'invalid', versionId },
+		{ appId, versionId: 'invalid' },
+	])('returns not found for malformed IDs: $appId / $versionId', async (ids) => {
+		const { service } = createService();
+
+		await expect(service.getVersion('project', ids.appId, ids.versionId)).rejects.toThrow(
+			NotFoundError,
+		);
+	});
+
+	it('returns not found for a missing version', async () => {
+		const { service, versions } = createService();
+		versions.findVersionById.mockResolvedValue(null);
+
+		await expect(service.getVersion('project', appId, versionId)).rejects.toThrow(NotFoundError);
+	});
+
+	it('rejects a stale draft before creating a source version', async () => {
+		const { service, sourceVersions } = createService();
+
+		await expect(service.saveSource('project', appId, null, files)).rejects.toThrow(ConflictError);
+
+		expect(sourceVersions.createVersion).not.toHaveBeenCalled();
+	});
+
+	it.each(['succeeded', 'failed'] as const)(
+		'rejects a bundle for a %s version',
+		async (buildStatus) => {
+			const { service, versions, buildResults, version } = createService();
+			versions.findVersionById.mockResolvedValue({ ...version, buildStatus });
+
+			await expect(service.attachBundle('project', appId, versionId, files)).rejects.toThrow(
+				ConflictError,
+			);
+
+			expect(buildResults.recordSuccess).not.toHaveBeenCalled();
+		},
+	);
+
+	it.each(['queued', 'running'] as const)(
+		'returns the completed build for a %s version',
+		async (buildStatus) => {
+			const { service, versions, buildResults, version } = createService();
+			const completed = mock<AppVersion>({
+				...version,
+				buildStatus: 'succeeded',
+				bundleArtifactId: 2,
+			});
+			versions.findVersionById.mockResolvedValue({ ...version, buildStatus });
+			buildResults.recordSuccess.mockImplementation(async () => {
+				versions.findVersionById.mockResolvedValue(completed);
+			});
+
+			expect(await service.attachBundle('project', appId, versionId, files)).toEqual(completed);
+		},
+	);
+
+	it('restores files and the label into a new version', async () => {
+		const { service, sourceVersions } = createService();
+		sourceVersions.readSource.mockResolvedValue(files);
+
+		const restored = await service.restoreSource('project', appId, versionId, versionId);
+
+		expect(restored.id).not.toBe(versionId);
+		expect(sourceVersions.createVersion).toHaveBeenCalledWith(
+			{ id: restored.id, appId, expectedDraftVersionId: versionId, label: 'Original' },
+			files,
+		);
+	});
+
+	it('returns an accepted save when retention fails', async () => {
+		const { service, retention } = createService();
+		retention.prune.mockRejectedValue(new Error('Cleanup failed.'));
+
+		const saved = await service.saveSource('project', appId, versionId, files);
+
+		expect(saved.id).not.toBe(versionId);
+		expect(retention.prune).toHaveBeenCalledWith(appId);
+	});
+
+	it.each(['Updated', null])('returns the version after setting its label to %s', async (label) => {
+		const { service, versions, version } = createService();
+		versions.setLabel.mockImplementation(async (id, nextLabel) => ({
+			...version,
+			id,
+			label: nextLabel,
+		}));
+
+		expect(await service.setLabel('project', appId, versionId, label)).toEqual({
+			...version,
+			label,
+		});
+	});
+
+	it('rejects an oversized label before changing the version', async () => {
+		const { service, versions } = createService();
+
+		await expect(service.setLabel('project', appId, versionId, 'a'.repeat(256))).rejects.toThrow(
+			BadRequestError,
+		);
+
+		expect(versions.setLabel).not.toHaveBeenCalled();
+	});
+});

packages/modules/apps/backend/src/__tests__/snapshot-app-version-stores.test.ts

@@ -0,0 +1,159 @@
+import { DbLockService, type OperationContext } from '@n8n/db';
+import { ConflictError, NotFoundError } from '@n8n/errors';
+import { mock } from 'vitest-mock-extended';
+
+import type { AppArtifact } from '../app-artifact.entity';
+import { AppArtifactService } from '../app-artifact.service';
+import type { AppVersion } from '../app-version.entity';
+import { AppVersionRepository } from '../app-version.repository';
+import type { App } from '../app.entity';
+import { AppRepository } from '../app.repository';
+import { AppArchive } from '../storage/app-archive';
+import type { NewSourceVersion } from '../storage/app-version-stores';
+import {
+	SnapshotAppSourceVersionStore,
+	SnapshotAppBuildResultStore,
+} from '../storage/snapshot-app-version-stores';
+
+const version: NewSourceVersion = {
+	id: 'version',
+	appId: 'app',
+	expectedDraftVersionId: 'draft',
+	label: 'Snapshot',
+};
+const files = new Map([['index.html', Buffer.from('<html/>')]]);
+
+function createStores() {
+	const archives = mock<AppArchive>();
+	const artifacts = mock<AppArtifactService>();
+	const versions = mock<AppVersionRepository>();
+	const apps = mock<AppRepository>();
+	const locks = mock<DbLockService>();
+	const ctx = mock<OperationContext>();
+	locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx));
+	const artifact = mock<AppArtifact>({ id: 1 });
+	archives.pack.mockResolvedValue(Buffer.from('archive'));
+	artifacts.upload.mockResolvedValue(artifact);
+	apps.findById.mockResolvedValue(mock<App>());
+	apps.advanceDraft.mockResolvedValue(true);
+	versions.findVersionById.mockResolvedValue(mock<AppVersion>({ bundleArtifactId: null }));
+	versions.attachBundle.mockResolvedValue(true);
+	return {
+		source: new SnapshotAppSourceVersionStore(archives, artifacts, versions, apps, locks),
+		builds: new SnapshotAppBuildResultStore(archives, artifacts, versions, locks),
+		artifacts,
+		versions,
+		apps,
+		artifact,
+		ctx,
+	};
+}
+
+describe('SnapshotAppSourceVersionStore', () => {
+	it('creates a source-backed version and advances the expected draft', async () => {
+		const { source, artifacts, versions, apps, artifact, ctx } = createStores();
+
+		await source.createVersion(version, files);
+
+		expect(artifacts.completeUpload).toHaveBeenCalledWith(artifact, ctx);
+		expect(versions.insertVersion).toHaveBeenCalledWith(
+			{ id: version.id, appId: version.appId, label: version.label, sourceArtifactId: artifact.id },
+			ctx,
+		);
+		expect(apps.advanceDraft).toHaveBeenCalledWith(
+			version.appId,
+			version.expectedDraftVersionId,
+			version.id,
+			ctx,
+		);
+		expect(artifacts.discard).not.toHaveBeenCalled();
+	});
+
+	it('discards the upload when the app disappears', async () => {
+		const { source, artifacts, versions, apps, artifact } = createStores();
+		apps.findById.mockResolvedValue(null);
+
+		await expect(source.createVersion(version, files)).rejects.toThrow(NotFoundError);
+
+		expect(versions.insertVersion).not.toHaveBeenCalled();
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('discards a stale save when the draft changes during the upload', async () => {
+		const { source, artifacts, apps, artifact } = createStores();
+		apps.advanceDraft.mockResolvedValue(false);
+
+		await expect(source.createVersion(version, files)).rejects.toThrow(ConflictError);
+
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('discards the upload and preserves a metadata write error', async () => {
+		const { source, artifacts, versions, artifact } = createStores();
+		const error = new Error('Could not save the version.');
+		versions.insertVersion.mockRejectedValue(error);
+
+		await expect(source.createVersion(version, files)).rejects.toBe(error);
+
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('returns not found when reading a missing version', async () => {
+		const { source, artifacts, versions } = createStores();
+		versions.findVersionById.mockResolvedValue(null);
+
+		await expect(source.readSource(version.id)).rejects.toThrow(NotFoundError);
+
+		expect(artifacts.read).not.toHaveBeenCalled();
+	});
+});
+
+describe('SnapshotAppBuildResultStore', () => {
+	it('attaches the uploaded bundle in the upload-completion transaction', async () => {
+		const { builds, artifacts, versions, artifact, ctx } = createStores();
+
+		await builds.recordSuccess(version.id, files);
+
+		expect(artifacts.completeUpload).toHaveBeenCalledWith(artifact, ctx);
+		expect(versions.attachBundle).toHaveBeenCalledWith(version.id, artifact.id, ctx);
+		expect(artifacts.discard).not.toHaveBeenCalled();
+	});
+
+	it('discards the bundle when the version disappears', async () => {
+		const { builds, artifacts, versions, artifact } = createStores();
+		versions.findVersionById.mockResolvedValue(null);
+
+		await expect(builds.recordSuccess(version.id, files)).rejects.toThrow(NotFoundError);
+
+		expect(versions.attachBundle).not.toHaveBeenCalled();
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('discards the bundle when the version no longer accepts a build result', async () => {
+		const { builds, artifacts, versions, artifact } = createStores();
+		versions.attachBundle.mockResolvedValue(false);
+
+		await expect(builds.recordSuccess(version.id, files)).rejects.toThrow(ConflictError);
+
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('does not attach an expired upload', async () => {
+		const { builds, artifacts, versions, artifact } = createStores();
+		const error = new ConflictError('Upload expired.');
+		artifacts.completeUpload.mockRejectedValue(error);
+
+		await expect(builds.recordSuccess(version.id, files)).rejects.toBe(error);
+
+		expect(versions.attachBundle).not.toHaveBeenCalled();
+		expect(artifacts.discard).toHaveBeenCalledWith(artifact);
+	});
+
+	it('returns not found when the version has no bundle', async () => {
+		const { builds, artifacts } = createStores();
+
+		await expect(builds.readBundle(version.id)).rejects.toThrow(NotFoundError);
+
+		expect(artifacts.read).not.toHaveBeenCalled();
+	});
+});