diff --git a/packages/modules/apps/backend/src/__tests__/app-artifact.service.test.ts b/packages/modules/apps/backend/src/__tests__/app-artifact.service.test.ts index b0df8ca8cd7..332e37d9ac9 100644 --- a/packages/modules/apps/backend/src/__tests__/app-artifact.service.test.ts +++ b/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({ ...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); + }); }); diff --git a/packages/modules/apps/backend/src/__tests__/app-blob-store.test.ts b/packages/modules/apps/backend/src/__tests__/app-blob-store.test.ts index 870ec305623..b9f34b5d770 100644 --- a/packages/modules/apps/backend/src/__tests__/app-blob-store.test.ts +++ b/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(); + const binaryData = mock(); + const artifacts = mock(); + const locks = mock(); + const ctx = mock(); + locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx)); const store = new AppBlobStore( fs, mock({ modeTag: mode }), - mock(), - mock(), - mock(), + 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(); + client.checkConnection.mockRejectedValue(error); + Container.set(ObjectStoreConfig, mock({ bucket: { name: 'apps' } })); + Container.set(ObjectStoreService, client); + const { store } = createStore('s3'); + + await expect(store.init()).rejects.toBe(error); +}); diff --git a/packages/modules/apps/backend/src/__tests__/app-version.service.test.ts b/packages/modules/apps/backend/src/__tests__/app-version.service.test.ts new file mode 100644 index 00000000000..5fe105a2dab --- /dev/null +++ b/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('')]]); + +function createService() { + const apps = mock(); + const versions = mock(); + const sourceVersions = mock(); + const buildResults = mock(); + const retention = mock(); + const locks = mock(); + const ctx = mock(); + locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx)); + const version = mock({ + id: versionId, + appId, + label: 'Original', + buildStatus: 'queued', + }); + apps.findInProject.mockResolvedValue(mock({ id: appId, draftVersionId: versionId })); + versions.findVersionById.mockResolvedValue(version); + sourceVersions.createVersion.mockImplementation(async (data) => { + versions.findVersionById.mockResolvedValue( + mock({ ...version, id: data.id, label: data.label }), + ); + }); + const service = new AppVersionService( + apps, + versions, + sourceVersions, + buildResults, + retention, + locks, + mock(), + ); + 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({ 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({ 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({ + ...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(); + }); +}); diff --git a/packages/modules/apps/backend/src/__tests__/snapshot-app-version-stores.test.ts b/packages/modules/apps/backend/src/__tests__/snapshot-app-version-stores.test.ts new file mode 100644 index 00000000000..1de24daef36 --- /dev/null +++ b/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('')]]); + +function createStores() { + const archives = mock(); + const artifacts = mock(); + const versions = mock(); + const apps = mock(); + const locks = mock(); + const ctx = mock(); + locks.withLockContext.mockImplementation(async (_key, fn) => await fn(ctx)); + const artifact = mock({ id: 1 }); + archives.pack.mockResolvedValue(Buffer.from('archive')); + artifacts.upload.mockResolvedValue(artifact); + apps.findById.mockResolvedValue(mock()); + apps.advanceDraft.mockResolvedValue(true); + versions.findVersionById.mockResolvedValue(mock({ 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(); + }); +});