From 98337626b1b0e451bd593af426ed4f20d26b136f Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 20 Mar 2026 01:15:51 +0100 Subject: [PATCH] Adds upload collection flow test coverage Introduces shared upload fixtures to keep tests consistent and easier to extend. Expands coverage for item validation, direct collection submission, and two-step collection creation so library-only and mixed uploads behave reliably. Verifies retry and recovery paths after interrupted or failed collection creation, helping prevent silent post-upload errors. --- .../upload/testHelpers/uploadFixtures.ts | 102 ++++++++++++ .../upload/uploadItemTypes.test.ts | 77 +++++++++ .../upload/useUploadBatchOperations.test.ts | 107 +++++++++++++ .../upload/useUploadSubmission.test.ts | 149 ++++++++++++------ 4 files changed, 388 insertions(+), 47 deletions(-) create mode 100644 client/src/composables/upload/testHelpers/uploadFixtures.ts create mode 100644 client/src/composables/upload/uploadItemTypes.test.ts create mode 100644 client/src/composables/upload/useUploadBatchOperations.test.ts diff --git a/client/src/composables/upload/testHelpers/uploadFixtures.ts b/client/src/composables/upload/testHelpers/uploadFixtures.ts new file mode 100644 index 00000000000..7d0263aefda --- /dev/null +++ b/client/src/composables/upload/testHelpers/uploadFixtures.ts @@ -0,0 +1,102 @@ +import type { UploadCollectionConfig } from "@/composables/upload/collectionTypes"; +import type { + LibraryDatasetUploadItem, + LocalFileUploadItem, + PastedContentUploadItem, + RemoteFileUploadItem, + UrlUploadItem, +} from "@/composables/upload/uploadItemTypes"; + +export function makePastedItem(overrides: Partial = {}): PastedContentUploadItem { + return { + uploadMode: "paste-content", + name: "file.txt", + content: "hello world", + size: 11, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + ...overrides, + }; +} + +export function makeUrlItem(overrides: Partial = {}): UrlUploadItem { + return { + uploadMode: "paste-links", + name: "file.txt", + url: "http://example.com/file.txt", + size: 0, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + ...overrides, + }; +} + +export function makeRemoteFilesItem(overrides: Partial = {}): RemoteFileUploadItem { + return { + uploadMode: "remote-files", + name: "file.txt", + url: "ftp://server/file.txt", + size: 0, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + ...overrides, + }; +} + +export function makeLibraryItem(overrides: Partial = {}): LibraryDatasetUploadItem { + return { + uploadMode: "data-library", + name: "library.txt", + size: 0, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + libraryId: "lib_1", + folderId: "folder_1", + lddaId: "ldda_1", + url: "/api/libraries/lib_1/datasets/ldda_1", + ...overrides, + }; +} + +export function makeLocalFileItem(overrides: Partial = {}): LocalFileUploadItem { + const file = new File(["content"], "test.txt"); + return { + uploadMode: "local-file", + name: "test.txt", + size: file.size, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + fileData: file, + ...overrides, + }; +} + +export function makeCollectionConfig(overrides: Partial = {}): UploadCollectionConfig { + return { + name: "My Collection", + type: "list", + historyId: "hist_1", + hideSourceItems: false, + ...overrides, + }; +} diff --git a/client/src/composables/upload/uploadItemTypes.test.ts b/client/src/composables/upload/uploadItemTypes.test.ts new file mode 100644 index 00000000000..0c1493191cf --- /dev/null +++ b/client/src/composables/upload/uploadItemTypes.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; + +import { + makeLibraryItem, + makeLocalFileItem, + makePastedItem, + makeRemoteFilesItem, + makeUrlItem, +} from "@/composables/upload/testHelpers/uploadFixtures"; +import type { NewUploadItem } from "@/composables/upload/uploadItemTypes"; + +import { validateUploadItem } from "./uploadItemTypes"; + +describe("validateUploadItem", () => { + it.each([ + ["paste-content", makePastedItem()], + ["paste-links", makeUrlItem()], + ["remote-files", makeRemoteFilesItem()], + ["data-library", makeLibraryItem()], + ["local-file", makeLocalFileItem()], + ] as [string, NewUploadItem][])("accepts a valid %s item", (_mode, item) => { + expect(validateUploadItem(item)).toBeUndefined(); + }); + + it("rejects paste-content with empty content", () => { + expect(validateUploadItem(makePastedItem({ content: " " }))).toMatch(/No content provided/); + }); + + it("rejects paste-links with missing URL", () => { + expect(validateUploadItem(makeUrlItem({ url: "" }))).toMatch(/No URL provided/); + }); + + it("rejects remote-files with missing URL", () => { + expect(validateUploadItem(makeRemoteFilesItem({ url: " " }))).toMatch(/No URL provided/); + }); + + it("rejects data-library with no lddaId", () => { + expect(validateUploadItem(makeLibraryItem({ lddaId: "" }))).toMatch(/No library dataset ID/); + }); + + it("rejects local-file with no file data", () => { + const item: NewUploadItem = { + uploadMode: "local-file", + name: "missing.txt", + size: 0, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + }; + expect(validateUploadItem(item)).toMatch(/No file selected/); + }); + + it("rejects local-file with an empty file", () => { + const emptyFile = new File([], "empty.txt"); + const item: NewUploadItem = { + uploadMode: "local-file", + name: "empty.txt", + size: 0, + targetHistoryId: "hist_1", + dbkey: "?", + extension: "auto", + spaceToTab: false, + toPosixLines: false, + deferred: false, + fileData: emptyFile, + }; + expect(validateUploadItem(item)).toMatch(/is empty/); + }); + + it("rejects an unknown upload mode", () => { + const item = { ...makePastedItem(), uploadMode: "unknown-mode" } as unknown as NewUploadItem; + expect(validateUploadItem(item)).toMatch(/Unknown upload mode/); + }); +}); diff --git a/client/src/composables/upload/useUploadBatchOperations.test.ts b/client/src/composables/upload/useUploadBatchOperations.test.ts new file mode 100644 index 00000000000..60404ee7c36 --- /dev/null +++ b/client/src/composables/upload/useUploadBatchOperations.test.ts @@ -0,0 +1,107 @@ +import { suppressExpectedErrorMessages } from "@tests/vitest/helpers"; +import flushPromises from "flush-promises"; +import { http, HttpResponse } from "msw"; +import { createPinia, setActivePinia } from "pinia"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { useServerMock } from "@/api/client/__mocks__"; +import { useUploadState } from "@/components/Panels/Upload/uploadState"; +import { makeCollectionConfig, makePastedItem, makeUrlItem } from "@/composables/upload/testHelpers/uploadFixtures"; + +import { useUploadBatchOperations } from "./useUploadBatchOperations"; + +const { server } = useServerMock(); + +describe("useUploadBatchOperations", () => { + beforeEach(() => { + setActivePinia(createPinia()); + useUploadState().clearAll(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + useUploadState().clearAll(); + }); + + it("processes the direct collection path atomically", async () => { + server.use( + http.get("/api/configuration", () => HttpResponse.json({ chunk_upload_size: 42 })), + http.post("/api/tools/fetch", async ({ request }) => { + const body = await request.json(); + expect(body).toMatchObject({ + history_id: "hist_1", + targets: [ + { + destination: { type: "hdca" }, + collection_type: "list", + name: "My Collection", + }, + ], + }); + return HttpResponse.json({ outputs: [{ id: "hdca_1", src: "hdca" }] }); + }), + ); + + const state = useUploadState(); + const operations = useUploadBatchOperations({ autoRecover: false }); + const first = makeUrlItem({ name: "a.txt" }); + const second = makeUrlItem({ name: "b.txt" }); + const batchId = state.addBatch(makeCollectionConfig(), [], true); + const id1 = state.addUploadItem(first, batchId); + const id2 = state.addUploadItem(second, batchId); + state.getBatch(batchId)!.uploadIds = [id1, id2]; + + await operations.processDirectBatch(batchId, [id1, id2], [first, second]); + + expect(state.getBatch(batchId)?.status).toBe("completed"); + expect(state.activeItems.value.every((item) => item.status === "completed")).toBe(true); + }); + + it("retries collection creation after an earlier two-step failure", async () => { + suppressExpectedErrorMessages(["Temporary error"]); + server.use(http.post("/api/dataset_collections", () => HttpResponse.json({ id: "col_retried" }))); + + const state = useUploadState(); + const operations = useUploadBatchOperations({ autoRecover: false }); + const uploadId = state.addUploadItem(makePastedItem()); + state.updateProgress(uploadId, 100); + + const batchId = state.addBatch(makeCollectionConfig(), [uploadId], false); + state.addBatchDatasetId(batchId, "ds_1"); + state.setBatchError(batchId, "Temporary error"); + + const item = state.activeItems.value.find((entry) => entry.id === uploadId); + if (item) { + item.error = "Uploaded successfully, but collection creation failed"; + } + + await operations.retryCollectionCreation(batchId); + + expect(state.getBatch(batchId)?.status).toBe("completed"); + expect(state.getBatch(batchId)?.collectionId).toBe("col_retried"); + expect(state.activeItems.value.find((entry) => entry.id === uploadId)?.error).toBeUndefined(); + }); + + it("recovers interrupted two-step collection creation from persisted state", async () => { + server.use(http.post("/api/dataset_collections", () => HttpResponse.json({ id: "col_recovered" }))); + + const state = useUploadState(); + const itemId = state.addUploadItem(makePastedItem({ name: "recovered.txt" })); + state.setStatus(itemId, "uploading"); + state.updateProgress(itemId, 100); + + const batchId = state.addBatch( + { name: "Recovery Collection", type: "list", hideSourceItems: false, historyId: "hist_1" }, + [itemId], + false, + ); + state.addBatchDatasetId(batchId, "ds_recovered"); + + const operations = useUploadBatchOperations({ autoRecover: false }); + operations.recoverIncompleteBatches(); + await flushPromises(); + + expect(state.getBatch(batchId)?.collectionId).toBe("col_recovered"); + expect(state.getBatch(batchId)?.status).toBe("completed"); + }); +}); diff --git a/client/src/composables/upload/useUploadSubmission.test.ts b/client/src/composables/upload/useUploadSubmission.test.ts index 21a1b672606..faeddd40504 100644 --- a/client/src/composables/upload/useUploadSubmission.test.ts +++ b/client/src/composables/upload/useUploadSubmission.test.ts @@ -1,4 +1,4 @@ -import { getLocalVue } from "@tests/vitest/helpers"; +import { getLocalVue, suppressExpectedErrorMessages } from "@tests/vitest/helpers"; import { mount } from "@vue/test-utils"; import flushPromises from "flush-promises"; import { http, HttpResponse } from "msw"; @@ -9,8 +9,7 @@ import { defineComponent, ref } from "vue"; import { useServerMock } from "@/api/client/__mocks__"; import type { PreparedUpload } from "@/components/Panels/Upload/types"; import { useUploadState } from "@/components/Panels/Upload/uploadState"; -import type { UploadCollectionConfig } from "@/composables/upload/collectionTypes"; -import type { LibraryDatasetUploadItem, UrlUploadItem } from "@/composables/upload/uploadItemTypes"; +import { makeCollectionConfig, makeLibraryItem, makeUrlItem } from "@/composables/upload/testHelpers/uploadFixtures"; import { buildPreparedUpload } from "@/utils/upload"; import { useUploadSubmission } from "./useUploadSubmission"; @@ -24,41 +23,6 @@ const SELECTORS = { const localVue = getLocalVue(); const { server } = useServerMock(); -function makeUrlItem(overrides: Partial = {}): UrlUploadItem { - return { - uploadMode: "paste-links", - name: "remote.txt", - url: "https://example.org/remote.txt", - size: 0, - targetHistoryId: "hist_1", - dbkey: "?", - extension: "auto", - spaceToTab: false, - toPosixLines: false, - deferred: false, - ...overrides, - }; -} - -function makeLibraryItem(overrides: Partial = {}): LibraryDatasetUploadItem { - return { - uploadMode: "data-library", - name: "library.txt", - size: 0, - targetHistoryId: "hist_1", - dbkey: "?", - extension: "auto", - spaceToTab: false, - toPosixLines: false, - deferred: false, - libraryId: "lib_1", - folderId: "folder_1", - lddaId: "ldda_1", - url: "/api/libraries/datasets/ldda_1", - ...overrides, - }; -} - function mountHarness(prepared: PreparedUpload) { const Harness = defineComponent({ setup() { @@ -89,14 +53,11 @@ function mountHarness(prepared: PreparedUpload) { return mount(Harness, { localVue, pinia: createPinia() }); } -function makeCollectionConfig(overrides: Partial = {}): UploadCollectionConfig { - return { +function makeSubmissionCollectionConfig() { + return makeCollectionConfig({ name: "Uploaded Collection", - type: "list", hideSourceItems: true, - historyId: "hist_1", - ...overrides, - }; + }); } describe("useUploadSubmission", () => { @@ -136,7 +97,7 @@ describe("useUploadSubmission", () => { }), ); - const apiItem = makeUrlItem(); + const apiItem = makeUrlItem({ name: "remote.txt", url: "https://example.org/remote.txt" }); const apiPrepared = buildPreparedUpload([apiItem]); const wrapper = mountHarness({ apiItems: apiPrepared.apiItems, @@ -166,7 +127,7 @@ describe("useUploadSubmission", () => { http.post("/api/tools/fetch", () => HttpResponse.json({ err_msg: "upload failed" }, { status: 500 })), ); - const apiItem = makeUrlItem({ url: "https://example.org/broken.txt" }); + const apiItem = makeUrlItem({ name: "remote.txt", url: "https://example.org/broken.txt" }); const apiPrepared = buildPreparedUpload([apiItem]); const wrapper = mountHarness({ apiItems: apiPrepared.apiItems, @@ -238,7 +199,7 @@ describe("useUploadSubmission", () => { const firstItem = makeUrlItem({ name: "1.bed", url: "https://example.org/1.bed" }); const secondItem = makeUrlItem({ name: "2.bed", url: "https://example.org/2.bed" }); - const prepared = buildPreparedUpload([firstItem, secondItem], makeCollectionConfig()); + const prepared = buildPreparedUpload([firstItem, secondItem], makeSubmissionCollectionConfig()); const wrapper = mountHarness(prepared); await flushPromises(); @@ -258,4 +219,98 @@ describe("useUploadSubmission", () => { expect(state.orderedUploadItems.value[0]?.type).toBe("batch"); expect(state.activeItems.value.every((item) => item.batchId === batch?.id)).toBe(true); }); + + it("creates a two-step collection for library-only uploads", async () => { + server.use( + http.post("/api/histories/hist_1/contents/datasets", () => HttpResponse.json({ id: "hda_lib_1", hid: 3 })), + http.post("/api/dataset_collections", async ({ request }) => { + const body = await request.json(); + expect(body).toMatchObject({ + history_id: "hist_1", + collection_type: "list", + name: "Uploaded Collection", + element_identifiers: [{ id: "hda_lib_1", src: "hda" }], + }); + return HttpResponse.json({ id: "hdca_lib_1" }); + }), + ); + + const prepared = buildPreparedUpload([makeLibraryItem()], makeSubmissionCollectionConfig()); + const wrapper = mountHarness(prepared); + await flushPromises(); + + await wrapper.find(SELECTORS.RUN).trigger("click"); + await flushPromises(); + + expect(wrapper.find(SELECTORS.RESULT).text()).toContain('"id":"hda_lib_1"'); + + const batch = useUploadState().activeBatches.value[0]; + expect(batch?.status).toBe("completed"); + expect(batch?.collectionId).toBe("hdca_lib_1"); + expect(batch?.datasetIds).toEqual(["hda_lib_1"]); + }); + + it("creates a two-step collection for mixed api and library uploads", async () => { + server.use( + http.post("/api/tools/fetch", () => + HttpResponse.json({ + outputs: [{ id: "hda_api_1", name: "api dataset", hid: 1, src: "hda" }], + }), + ), + http.post("/api/histories/hist_1/contents/datasets", () => HttpResponse.json({ id: "hda_lib_2", hid: 2 })), + http.post("/api/dataset_collections", async ({ request }) => { + const body = await request.json(); + expect(body).toMatchObject({ + history_id: "hist_1", + collection_type: "list", + name: "Uploaded Collection", + element_identifiers: [ + { id: "hda_api_1", src: "hda" }, + { id: "hda_lib_2", src: "hda" }, + ], + }); + return HttpResponse.json({ id: "hdca_mixed_1" }); + }), + ); + + const apiItem = makeUrlItem({ name: "api-first.txt" }); + const libraryItem = makeLibraryItem({ name: "library-second.txt", lddaId: "ldda_2" }); + const wrapper = mountHarness(buildPreparedUpload([apiItem, libraryItem], makeSubmissionCollectionConfig())); + await flushPromises(); + + await wrapper.find(SELECTORS.RUN).trigger("click"); + await flushPromises(); + + expect(wrapper.find(SELECTORS.RESULT).text()).toContain('"id":"hda_api_1"'); + expect(wrapper.find(SELECTORS.RESULT).text()).toContain('"id":"hda_lib_2"'); + + const batch = useUploadState().activeBatches.value[0]; + expect(batch?.status).toBe("completed"); + expect(batch?.collectionId).toBe("hdca_mixed_1"); + expect(batch?.datasetIds).toEqual(["hda_api_1", "hda_lib_2"]); + }); + + it("surfaces two-step collection creation failures after uploads succeed", async () => { + suppressExpectedErrorMessages(["Collection error"]); + server.use( + http.post("/api/histories/hist_1/contents/datasets", () => HttpResponse.json({ id: "hda_lib_3", hid: 4 })), + http.post("/api/dataset_collections", () => + HttpResponse.json({ err_msg: "Collection error" }, { status: 500 }), + ), + ); + + const wrapper = mountHarness( + buildPreparedUpload([makeLibraryItem({ lddaId: "ldda_3" })], makeSubmissionCollectionConfig()), + ); + await flushPromises(); + + await wrapper.find(SELECTORS.RUN).trigger("click"); + await flushPromises(); + + expect(wrapper.find(SELECTORS.ERROR).text()).toContain("Collection error"); + + const batch = useUploadState().activeBatches.value[0]; + expect(batch?.status).toBe("error"); + expect(batch?.collectionId).toBeUndefined(); + }); });