diff --git a/client/platform/web-girder/api/dataset.service.ts b/client/platform/web-girder/api/dataset.service.ts index 36cee283d..9a9e9b423 100644 --- a/client/platform/web-girder/api/dataset.service.ts +++ b/client/platform/web-girder/api/dataset.service.ts @@ -5,7 +5,7 @@ import { registrationValuesSummary, filterRegistrationValues, mergeRegistrationValues, } from 'vue-media-annotator/alignedView/cameraRegistrationFiles'; import { - DatasetMetaMutable, FrameImage, SaveAttributeArgs, SaveAttributeTrackFilterArgs, + DatasetMetaMutable, DatasetType, FrameImage, SaveAttributeArgs, SaveAttributeTrackFilterArgs, } from 'dive-common/apispec'; import { calibrationFileMarker, jsonCalibrationFileMarker, metadataFileMarker } from 'dive-common/constants'; import { attachFrameTimestamps } from 'dive-common/frameTimestamp'; @@ -247,14 +247,33 @@ async function importCameraRegistration( return summary; } -interface ValidationResponse { - ok: boolean; - type: 'video' | 'image-sequence' | 'large-image'; - media: string[]; - annotations: string[]; +export type UploadRole = 'media' | 'annotations' | 'datasetConfig' | 'ignored'; + +/** Every validated filename under exactly one role. `ignored` is the files not accepted. */ +export type ValidatedUploadRoleMap = Record; + +/** A filename with the reason it is not uploaded. Display shape; the wire carries `reasons`. */ +export interface IgnoredUploadFile { + name: string; + reason: string; +} + +interface ValidationBase { message: string; + roles: ValidatedUploadRoleMap; + /** Why a file landed in its role, keyed by filename. Populated for the `ignored` role. */ + reasons: Record; } +/** + * Server classification of one upload selection. `roles` is authoritative for what each file + * is for, so the set to upload is the selection minus `roles.ignored`. Only an accepted + * selection has a media type, so `ok` narrows `type` to a real DatasetType. + */ +export type ValidationResponse = + | (ValidationBase & { ok: true; type: DatasetType }) + | (ValidationBase & { ok: false; type?: undefined }); + function validateUploadGroup(names: string[]) { return girderRest.post('dive_dataset/validate_files', names); } diff --git a/client/platform/web-girder/multicamFileRegistry.spec.ts b/client/platform/web-girder/multicamFileRegistry.spec.ts index a535b4a3d..a05a64a92 100644 --- a/client/platform/web-girder/multicamFileRegistry.spec.ts +++ b/client/platform/web-girder/multicamFileRegistry.spec.ts @@ -6,11 +6,25 @@ import { import { clearMulticamFileRegistry, getCalibrationFile, + getCameraPackageFiles, getLastCalibration, + mediaFileNamesForImport, saveCalibration, + stashAnnotationFile, stashCalibrationFile, } from './multicamFileRegistry'; +/** Fixed lastModified so "did the user pick this very file?" is deterministic in tests. */ +function file(name: string, content = name): File { + return new File([content], name, { type: 'application/octet-stream', lastModified: 0 }); +} + +function fileWithPath(name: string, relPath: string, content = name): File { + const f = file(name, content); + Object.defineProperty(f, 'webkitRelativePath', { value: relPath, configurable: true }); + return f; +} + describe('multicamFileRegistry calibration', () => { const storage = new Map(); @@ -30,10 +44,10 @@ describe('multicamFileRegistry calibration', () => { }); it('resolves calibration by basename when stashed with a path-like key', () => { - const file = new File(['{}'], 'stereo-cal.json', { type: 'application/json' }); - stashCalibrationFile('folder/stereo-cal.json', file); - expect(getCalibrationFile('stereo-cal.json')).toBe(file); - expect(getCalibrationFile('folder/stereo-cal.json')).toBe(file); + const cal = new File(['{}'], 'stereo-cal.json', { type: 'application/json' }); + stashCalibrationFile('folder/stereo-cal.json', cal); + expect(getCalibrationFile('stereo-cal.json')).toBe(cal); + expect(getCalibrationFile('folder/stereo-cal.json')).toBe(cal); }); it('does not restore last calibration from localStorage without a session File', async () => { @@ -43,9 +57,94 @@ describe('multicamFileRegistry calibration', () => { }); it('restores last calibration when the File is still in the registry', async () => { - const file = new File(['{}'], 'cal.json', { type: 'application/json' }); - stashCalibrationFile('cal.json', file); + const cal = new File(['{}'], 'cal.json', { type: 'application/json' }); + stashCalibrationFile('cal.json', cal); await saveCalibration('cal.json'); await expect(getLastCalibration()).resolves.toBe('cal.json'); }); }); + +describe('multicam camera package construction', () => { + beforeEach(() => { + clearMulticamFileRegistry(); + }); + + it('flattens folder files before validation so names match uploaded names', () => { + const folderFiles = [ + fileWithPath('img001.png', 'cam1/img001.png'), + fileWithPath('img002.png', 'cam1/img002.png'), + ]; + const { files: cameraFiles } = getCameraPackageFiles(folderFiles); + // The server validates and Girder uploads flat basenames, not folder paths. + expect(cameraFiles.map((f) => f.name)).toEqual(['img001.png', 'img002.png']); + expect( + cameraFiles.every((f) => !f.webkitRelativePath || f.webkitRelativePath === f.name), + ).toBe(true); + }); + + it('reports only media filenames for multicam pre-import validation', () => { + const files = [ + fileWithPath('img001.png', 'cam1/img001.png'), + fileWithPath('nav.csv', 'cam1/nav.csv'), + fileWithPath('tracks.csv', 'cam1/tracks.csv'), + ]; + expect(mediaFileNamesForImport(files, 'image-sequence')).toEqual(['img001.png']); + }); + + it('reports TIFF camera files as media so Begin Import stays enabled', () => { + // Subfolder discovery registers a camera folder of .tif images (the SealTK IR layout) and + // imports it as large-image; reporting zero files here disables Begin Import permanently. + const files = [ + fileWithPath('ir_0001.tif', 'ir/ir_0001.tif'), + fileWithPath('ir_0002.TIFF', 'ir/ir_0002.TIFF'), + fileWithPath('scan.nitf', 'ir/scan.nitf'), + fileWithPath('notes.txt', 'ir/notes.txt'), + ]; + expect(mediaFileNamesForImport(files, 'image-sequence')).toEqual([ + 'ir_0001.tif', 'ir_0002.TIFF', 'scan.nitf', + ]); + }); + + it('appends the explicit track file to the validation input', () => { + const folderFiles = [file('img001.png')]; + const track = file('tracks.csv'); + stashAnnotationFile('cam1/tracks.csv', track); + const { files: cameraFiles, replaced } = getCameraPackageFiles(folderFiles, 'cam1/tracks.csv'); + expect(cameraFiles.map((f) => f.name)).toEqual(['img001.png', 'tracks.csv']); + expect(cameraFiles).toContain(track); + expect(replaced).toEqual([]); + }); + + it('deduplicates the explicit track file by name', () => { + const track = file('tracks.csv'); + stashAnnotationFile('cam1/tracks.csv', track); + // The same File object is also part of the camera folder selection. + const folderFiles = [file('img001.png'), track]; + const { files: cameraFiles, replaced } = getCameraPackageFiles(folderFiles, 'cam1/tracks.csv'); + expect(cameraFiles.map((f) => f.name)).toEqual(['img001.png', 'tracks.csv']); + expect(cameraFiles.filter((f) => f.name === 'tracks.csv')).toHaveLength(1); + // Re-picking the folder's own file is not a file leaving the upload. + expect(replaced).toEqual([]); + }); + + it('uploads the explicitly picked track file, and reports the folder copy it displaced', () => { + const explicitTrack = file('tracks.csv', 'chosen elsewhere'); + stashAnnotationFile('cam1/tracks.csv', explicitTrack); + const folderTrack = file('tracks.csv', 'the camera folder copy'); + const folderFiles = [file('img001.png'), folderTrack]; + const { files: cameraFiles, replaced } = getCameraPackageFiles(folderFiles, 'cam1/tracks.csv'); + expect(cameraFiles.map((f) => f.name)).toEqual(['img001.png', 'tracks.csv']); + expect(cameraFiles).toContain(explicitTrack); + expect(cameraFiles).not.toContain(folderTrack); + expect(replaced).toEqual([folderTrack]); + }); + + it('keeps camera-folder annotations and config in the package, not just media', () => { + // tracks.csv and config.json are auto-detected in the camera folder, and both belong to + // the camera: the package is everything the server validates, not only the media. + const folderFiles = [file('img001.png'), file('tracks.csv'), file('config.json')]; + expect(getCameraPackageFiles(folderFiles).files.map((f) => f.name)).toEqual([ + 'img001.png', 'tracks.csv', 'config.json', + ]); + }); +}); diff --git a/client/platform/web-girder/multicamFileRegistry.ts b/client/platform/web-girder/multicamFileRegistry.ts index 4ed8d2a7e..21481b269 100644 --- a/client/platform/web-girder/multicamFileRegistry.ts +++ b/client/platform/web-girder/multicamFileRegistry.ts @@ -2,6 +2,13 @@ import { Location } from '@girder/components/src'; import { parentDatasetId } from 'dive-common/compositeDatasetId'; +import { + ImageSequenceType, + VideoType, + fileVideoTypes, + largeImageFileExtensions, +} from 'dive-common/constants'; +import { fileImageTypes } from 'dive-common/components/ImportMultiCamDialog/multicamSubfolderLayout'; import { openFromDisk, GirderUploadManager } from './utils'; const LAST_CALIBRATION_STORAGE_KEY = 'dive_web_last_calibration'; @@ -89,8 +96,26 @@ export function flattenUploadFiles(files: File[]): File[] { }); } -export function mediaFileNamesForImport(files: File[]): string[] { - return flattenUploadFiles(files).map((file) => file.name); +function fileExtension(fileName: string): string { + return fileName.split('.').pop()?.toLowerCase() ?? ''; +} + +/** + * Extensions a camera folder can contribute as importable images: the set multicam subfolder + * discovery accepts, plus the large-image formats an all-TIFF camera folder imports as. A + * camera that discovery registered must never report zero media files here, or Begin Import + * is disabled with no way to correct it. + */ +const importableImageExtensions = [...fileImageTypes, ...largeImageFileExtensions]; + +export function mediaFileNamesForImport( + files: File[], + mediaType: typeof ImageSequenceType | typeof VideoType = ImageSequenceType, +): string[] { + const allowedExtensions = mediaType === VideoType ? fileVideoTypes : importableImageExtensions; + return files + .map((file) => file.name) + .filter((name) => allowedExtensions.includes(fileExtension(name))); } export function stashAnnotationFile(key: string, file: File): void { @@ -101,6 +126,52 @@ export function getAnnotationFile(key: string): File | undefined { return annotationFilesByKey.get(key); } +export interface CameraPackage { + /** Files to validate and upload with the camera. */ + files: File[]; + /** + * Camera-folder files left out only because an explicit pick claimed their name. They are + * a different file than the one the user chose, so the caller must report them. + */ + replaced: File[]; +} + +/** Two selections of the same file on disk are distinct File objects; treat them as one. */ +function isSameSelection(a: File, b: File): boolean { + return a === b + || (a.name === b.name && a.size === b.size && a.lastModified === b.lastModified); +} + +/** + * Assemble the complete file package for one multicam camera: the camera folder's files plus + * the annotation file explicitly chosen for that camera. + * + * An explicit pick always wins over a folder file of the same name — the folder copy is + * dropped, so the user uploads the file they chose and Girder never sees a duplicate name. + * + * A dropped folder copy that is not the picked file is a real file leaving the upload, so it + * is returned in `replaced` rather than vanishing. + * + * `flattenUploadFiles` is applied here, before validation, so the names sent to + * the server for validation match the names uploaded to Girder. + */ +export function getCameraPackageFiles( + folderFiles: File[], + annotationKey?: string, +): CameraPackage { + const annotationFile = annotationKey ? getAnnotationFile(annotationKey) : undefined; + const explicitFiles = annotationFile ? [annotationFile] : []; + const explicitNames = new Set(explicitFiles.map((file) => file.name)); + const folderPackage = flattenUploadFiles( + folderFiles.filter((file) => !explicitNames.has(file.name)), + ); + return { + files: annotationFile ? [...folderPackage, annotationFile] : folderPackage, + replaced: folderFiles.filter((file) => explicitNames.has(file.name) + && !explicitFiles.some((pick) => isSameSelection(pick, file))), + }; +} + function calibrationLookupKeys(key: string): string[] { const keys = new Set(); if (key) { diff --git a/client/platform/web-girder/uploadSlots.spec.ts b/client/platform/web-girder/uploadSlots.spec.ts new file mode 100644 index 000000000..975e1dbe5 --- /dev/null +++ b/client/platform/web-girder/uploadSlots.spec.ts @@ -0,0 +1,72 @@ +// eslint-disable-next-line import/no-extraneous-dependencies -- Vitest is only used in tests +import { describe, expect, it } from 'vitest'; + +import suggestUploadSlots from './uploadSlots'; + +function file(name: string): File { + return new File([name], name, { type: 'application/octet-stream' }); +} + +function accountedNames(slots: ReturnType): string[] { + return [ + ...slots.mediaList.map((f) => f.name), + ...(slots.annotationFile ? [slots.annotationFile.name] : []), + ...(slots.configFile ? [slots.configFile.name] : []), + ...slots.unslotted.map((entry) => entry.name), + ].sort(); +} + +describe('suggestUploadSlots', () => { + it('suggests a single annotation CSV and keeps it out of the media slot', () => { + const slots = suggestUploadSlots([file('img001.png'), file('tracks.csv')]); + expect(slots.mediaList.map((f) => f.name)).toEqual(['img001.png']); + expect(slots.annotationFile?.name).toBe('tracks.csv'); + }); + + it('detects a config JSON alongside an annotation CSV', () => { + const slots = suggestUploadSlots([file('img001.png'), file('tracks.csv'), file('dataset.meta.json')]); + expect(slots.configFile?.name).toBe('dataset.meta.json'); + expect(slots.annotationFile?.name).toBe('tracks.csv'); + expect(slots.mediaList.map((f) => f.name)).toEqual(['img001.png']); + }); + + it('leaves a second CSV unslotted rather than in the media slot the server rejects', () => { + // A second CSV in the media slot makes server validation fail outright ("Can only upload a + // single CSV Annotation per import"), which blocks the whole selection. Report it instead. + const slots = suggestUploadSlots([file('img001.png'), file('tracks.csv'), file('nav_2024.csv')]); + expect(slots.mediaList.map((f) => f.name)).toEqual(['img001.png']); + expect(slots.annotationFile?.name).toBe('tracks.csv'); + expect(slots.unslotted).toEqual([ + { name: 'nav_2024.csv', reason: 'Only one annotation file can be uploaded per dataset' }, + ]); + }); + + it('reports a second configuration JSON rather than sending both to validation', () => { + const slots = suggestUploadSlots([ + file('img001.png'), file('a.meta.json'), file('b.config.json'), + ]); + expect(slots.configFile?.name).toBe('a.meta.json'); + expect(slots.annotationFile).toBeNull(); + expect(slots.mediaList.map((f) => f.name)).toEqual(['img001.png']); + expect(slots.unslotted).toEqual([ + { name: 'b.config.json', reason: 'Only one configuration file can be uploaded per dataset' }, + ]); + }); + + it('slots a lone YAML annotation instead of leaving it to look like media', () => { + const slots = suggestUploadSlots([file('video.mp4'), file('tracks.yml')]); + expect(slots.mediaList.map((f) => f.name)).toEqual(['video.mp4']); + expect(slots.annotationFile?.name).toBe('tracks.yml'); + }); + + it('never silently drops any picked file (every input is slotted or reported)', () => { + const picked = [ + file('img001.png'), file('img002.png'), + file('tracks.csv'), file('extra.csv'), + file('dataset.meta.json'), file('other.json'), + file('nav.unknown'), + ]; + const slots = suggestUploadSlots(picked); + expect(accountedNames(slots)).toEqual(picked.map((f) => f.name).sort()); + }); +}); diff --git a/client/platform/web-girder/uploadSlots.ts b/client/platform/web-girder/uploadSlots.ts new file mode 100644 index 000000000..989e33b42 --- /dev/null +++ b/client/platform/web-girder/uploadSlots.ts @@ -0,0 +1,55 @@ +import { JsonMetaRegEx } from 'dive-common/constants'; +import type { IgnoredUploadFile } from './api/dataset.service'; + +export interface SuggestedUploadSlots { + mediaList: File[]; + annotationFile: File | null; + configFile: File | null; + /** + * Picked files no slot can hold, each with the reason. A dataset takes at most one + * annotation source, so the extras are reported to the user instead of being placed in a + * slot that would make the whole selection fail validation. + */ + unslotted: IgnoredUploadFile[]; +} + +const ONE_ANNOTATION_REASON = 'Only one annotation file can be uploaded per dataset'; +const ONE_CONFIG_REASON = 'Only one configuration file can be uploaded per dataset'; + +/** Files the server classifies as annotation sources, in the order this split prefers them. */ +function annotationCandidates(files: File[], configFiles: File[]): File[] { + const matching = (test: (name: string) => boolean) => files.filter( + (file) => !configFiles.includes(file) && test(file.name), + ); + return [ + ...matching((name) => name.includes('.csv')), + ...matching((name) => name.includes('.yml') || name.includes('.yaml')), + ...matching((name) => name.includes('.json')), + ]; +} + +/** + * Auto-suggest a picked selection into the per-role upload slots. + * + * Placement is a convenience split the server re-validates at upload. Every input file lands + * in exactly one slot or in `unslotted`, so nothing the user picked is ever silently dropped. + */ +export default function suggestUploadSlots(fileList: File[]): SuggestedUploadSlots { + const configFiles = fileList.filter((f) => JsonMetaRegEx.test(f.name)); + const [configFile = null, ...extraConfigs] = configFiles; + const [annotationFile = null, ...extraAnnotations] = annotationCandidates(fileList, configFiles); + const claimed = new Set([ + ...configFiles, + ...(annotationFile ? [annotationFile] : []), + ...extraAnnotations, + ]); + return { + mediaList: fileList.filter((f) => !claimed.has(f)), + annotationFile, + configFile, + unslotted: [ + ...extraAnnotations.map((f) => ({ name: f.name, reason: ONE_ANNOTATION_REASON })), + ...extraConfigs.map((f) => ({ name: f.name, reason: ONE_CONFIG_REASON })), + ], + }; +} diff --git a/client/platform/web-girder/utils.ts b/client/platform/web-girder/utils.ts index 12a1e7ec7..6fa9ddb07 100644 --- a/client/platform/web-girder/utils.ts +++ b/client/platform/web-girder/utils.ts @@ -44,7 +44,10 @@ async function openFromDisk( ): Promise<{ canceled: boolean; filePaths: string[]; fileList?: File[]; root?: string }> { const input: HTMLInputElement = document.createElement('input'); input.type = 'file'; - const baseTypes: string[] = inputAnnotationFileTypes.map((item) => `.${item}`); + // Side files a media selection may carry: an annotation source or a metadata attachment. + // Filtering one out here would settle its fate before the server ever classified it. + const baseTypes: string[] = [...new Set([...inputAnnotationFileTypes, ...metadataFileTypes])] + .map((item) => `.${item}`); if (!['calibration', 'annotation', 'zip', 'metadata'].includes(datasetType)) { input.multiple = true; } diff --git a/client/platform/web-girder/views/Upload.spec.ts b/client/platform/web-girder/views/Upload.spec.ts new file mode 100644 index 000000000..8f08e2337 --- /dev/null +++ b/client/platform/web-girder/views/Upload.spec.ts @@ -0,0 +1,247 @@ +// @vitest-environment jsdom +/* eslint-disable import/no-extraneous-dependencies */ +import { mount } from '@vue/test-utils'; +import Vue, { CreateElement, nextTick } from 'vue'; + +import { + beforeEach, describe, expect, it, vi, +} from 'vitest'; + +import type { ValidatedUploadRoleMap, ValidationResponse } from 'platform/web-girder/api'; +import type { DatasetType } from 'dive-common/apispec'; +import { openFromDisk } from 'platform/web-girder/utils'; +import { validateUploadGroup } from 'platform/web-girder/api'; +import Upload from './Upload.vue'; + +Vue.config.ignoredElements = [/^v-/]; + +vi.mock('platform/web-girder/api', () => ({ + createGirderFolder: vi.fn(), + createMulticamDataset: vi.fn(), + deleteResources: vi.fn(), + saveMetadata: vi.fn(), + uploadCalibrationItem: vi.fn(), + uploadMetadataFileItem: vi.fn(), + validateUploadGroup: vi.fn(), + waitForFolderDatasetReady: vi.fn(), +})); + +vi.mock('platform/web-girder/utils', () => ({ + openFromDisk: vi.fn(), + GirderUploadManager: class {}, +})); + +vi.mock('vue-router/composables', () => ({ + useRouter: () => ({ push: vi.fn() }), +})); + +vi.mock('dive-common/vue-utilities/prompt-service', () => ({ + usePrompt: () => ({ prompt: vi.fn() }), +})); + +const Stub = { + render(this: Vue, h: CreateElement) { + return h('div'); + }, +}; + +/** Renders the row slot the way UploadGirder does, with a spy for the upload action. */ +function uploadGirderStub(upload: () => Promise) { + return { + name: 'UploadGirder', + render(this: Vue, h: CreateElement) { + return h('div', this.$scopedSlots.default?.({ upload })); + }, + }; +} + +function file(name: string): File { + return new File([name], name, { type: 'application/octet-stream' }); +} + +const emptyRoles: ValidatedUploadRoleMap = { + media: [], annotations: [], datasetConfig: [], ignored: [], +}; + +/** A passing validation response; unnamed roles default to empty. */ +function validation(overrides: { + type?: DatasetType; + roles?: Partial; + reasons?: Record; +} = {}): ValidationResponse { + const { roles, type = 'video', reasons = {} } = overrides; + return { + ok: true, type, message: '', reasons, roles: { ...emptyRoles, ...roles }, + }; +} + +/** A rejected selection: a blocking message, and no media type at all. */ +function rejection(message: string): ValidationResponse { + return { + ok: false, message, roles: { ...emptyRoles }, reasons: {}, + }; +} + +function mountUpload(upload: () => Promise = () => Promise.resolve()) { + return mount(Upload, { + propsData: { location: { _id: 'folder-id', _modelType: 'folder' } }, + stubs: { + ImportButton: Stub, + ImportMultiCamDialog: Stub, + ImportMultiCamBatchDialog: Stub, + UploadGirder: uploadGirderStub(upload), + }, + }); +} + +function pick(files: File[]) { + vi.mocked(openFromDisk).mockResolvedValue({ + canceled: false, + filePaths: files.map((f) => f.name), + fileList: files, + }); +} + +describe('Upload pending rows', () => { + beforeEach(() => { + vi.mocked(openFromDisk).mockReset(); + vi.mocked(validateUploadGroup).mockReset(); + }); + + it('derives fan-out from the server media role, not the client slot guess', async () => { + // camera.npz cannot be classified, so it stays in the media slot; only the server knows + // it is not media. Counting the slot instead would fan one video out into subfolders. + pick([file('dive.mp4'), file('camera.npz')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: validation({ + roles: { media: ['dive.mp4'], ignored: ['camera.npz'] }, + reasons: { 'camera.npz': 'Unsupported side file' }, + }), + } as never); + + const wrapper = mountUpload(); + await wrapper.vm.openImport('video'); + + const [row] = wrapper.vm.pendingUploads; + expect(row.createSubFolders).toBe(false); + expect(row.name).toBe('dive.mp4'); + expect(row.uploadFiles.map((f: File) => f.name)).toEqual(['dive.mp4']); + expect(row.ignored).toEqual([{ name: 'camera.npz', reason: 'Unsupported side file' }]); + }); + + it('creates a row in an error state when validation rejects the selection', async () => { + pick([file('img001.png'), file('tracks.csv')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: rejection('Can only upload a single CSV Annotation per import'), + } as never); + + const wrapper = mountUpload(); + await wrapper.vm.openImport('image-sequence'); + await nextTick(); + + expect(wrapper.vm.pendingUploads).toHaveLength(1); + const [row] = wrapper.vm.pendingUploads; + expect(row.error).toBe('Can only upload a single CSV Annotation per import'); + expect(row.uploadFiles).toEqual([]); + // The slot editor stays usable so the user can correct the selection in place. + expect(row.createSubFolders).toBe(false); + expect(row.mediaList.map((f: File) => f.name)).toEqual(['img001.png']); + expect(wrapper.text()).toContain('Can only upload a single CSV Annotation per import'); + }); + + it('leaves the media slot unfiltered while a row is in an error state', async () => { + // Until the server accepts a selection the row's type is only the menu the user opened. + // A TIFF folder picked from Import > Image Sequence validates as large-image, but a + // rejected one keeps the image-sequence guess, whose accept filter excludes TIFF -- so + // filtering by the guess would refuse the very files the user must re-pick. + pick([file('a.tif'), file('tracks.csv'), file('nav.csv')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: rejection('Can only upload a single CSV Annotation per import'), + } as never); + + const wrapper = mountUpload(); + await wrapper.vm.openImport('image-sequence'); + await nextTick(); + + const [row] = wrapper.vm.pendingUploads; + expect(wrapper.vm.mediaSlotAccept(row)).toBeUndefined(); + row.error = null; + expect(wrapper.vm.mediaSlotAccept(row)).toEqual(wrapper.vm.filterFileUpload(row.type)); + }); + + it('names a corrected row when Start upload finally validates it', async () => { + // The row exists only so the slot editor can fix the selection; until it validates the + // server has named no media, so the row has no folder name to upload into. + pick([file('a.mp4'), file('img.png')]); + vi.mocked(validateUploadGroup).mockResolvedValueOnce({ + data: rejection('Do not upload images and videos in the same batch.'), + } as never); + const upload = vi.fn().mockResolvedValue(undefined); + + const wrapper = mountUpload(upload); + await wrapper.vm.openImport('video'); + + const [row] = wrapper.vm.pendingUploads; + expect(row.name).toBe(''); + + // The user drops the stray image in the slot editor, then starts the upload. + row.mediaList = [file('a.mp4')]; + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: validation({ roles: { media: ['a.mp4'] } }), + } as never); + await wrapper.vm.prepAndUpload(upload); + + expect(row.error).toBeNull(); + expect(row.name).toBe('a.mp4'); + expect(upload).toHaveBeenCalledTimes(1); + }); + + it('lists every ignored file, including two that share a basename', async () => { + // A selection spanning subfolders repeats basenames, and both copies are dropped, so the + // notice has to name both — the list is keyed by position for that reason. + pick([file('dive.mp4'), file('tracks.csv'), file('notes.csv'), file('notes.csv')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: validation({ roles: { media: ['dive.mp4'], annotations: ['tracks.csv'] } }), + } as never); + + const wrapper = mountUpload(); + await wrapper.vm.openImport('video'); + await nextTick(); + + const [row] = wrapper.vm.pendingUploads; + expect(row.ignored.map((entry: { name: string }) => entry.name)).toEqual(['notes.csv', 'notes.csv']); + expect(wrapper.text().match(/notes\.csv/g)).toHaveLength(2); + }); + + it('retires the row the child names, not whichever row is last in the queue', async () => { + vi.mocked(validateUploadGroup).mockImplementation(async (names: string[]) => ({ + data: validation({ roles: { media: names } }), + }) as never); + + const wrapper = mountUpload(); + pick([file('a.mp4')]); + await wrapper.vm.openImport('video'); + pick([file('b.mp4')]); + await wrapper.vm.openImport('video'); + + wrapper.findComponent({ name: 'UploadGirder' }).vm.$emit('remove-upload', wrapper.vm.pendingUploads[0]); + + expect(wrapper.vm.pendingUploads.map((row: { name: string }) => row.name)).toEqual(['b.mp4']); + }); + + it('starts only one upload when Start upload is clicked twice', async () => { + pick([file('dive.mp4')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: validation({ roles: { media: ['dive.mp4'] } }), + } as never); + const upload = vi.fn().mockResolvedValue(undefined); + + const wrapper = mountUpload(upload); + await wrapper.vm.openImport('video'); + + // Both clicks land before the awaited validation round-trip resolves. + await Promise.all([wrapper.vm.prepAndUpload(upload), wrapper.vm.prepAndUpload(upload)]); + + expect(upload).toHaveBeenCalledTimes(1); + }); +}); diff --git a/client/platform/web-girder/views/Upload.vue b/client/platform/web-girder/views/Upload.vue index 209963f56..63609e896 100644 --- a/client/platform/web-girder/views/Upload.vue +++ b/client/platform/web-girder/views/Upload.vue @@ -5,11 +5,12 @@ import { import { useRouter } from 'vue-router/composables'; import { - ImageSequenceType, VideoType, DefaultVideoFPS, FPSOptions, + ImageSequenceType, VideoType, DefaultVideoFPS, FPSOptions, LargeImageType, inputAnnotationFileTypes, websafeVideoTypes, otherVideoTypes, - websafeImageTypes, otherImageTypes, JsonMetaRegEx, getLargeImageFileAccept, LargeImageType, + websafeImageTypes, otherImageTypes, getLargeImageFileAccept, metadataFileTypes, } from 'dive-common/constants'; +import suggestUploadSlots from 'platform/web-girder/uploadSlots'; import { fileSuffixRegex, @@ -32,14 +33,18 @@ import { validateUploadGroup, waitForFolderDatasetReady, } from 'platform/web-girder/api'; +import type { + IgnoredUploadFile, + ValidationResponse, +} from 'platform/web-girder/api'; import { clearMulticamFileRegistry, - getAnnotationFile, getCalibrationFile, getMetadataFile, + getCameraPackageFiles, getFilesForSourceKey, getTransformFile, - flattenUploadFiles, + mediaFileNamesForImport, removeCameraFolderFiles, renameCameraFolderFiles, stashCameraFolderFiles, @@ -75,10 +80,22 @@ export interface PendingUpload { createSubFolders: boolean; name: string; files: InteralFiles[]; - meta: null | File; - metadataFile: null | File; - annotationFile: null | File; + /** + * Per-role upload slots. The user declares each file's role by which slot it goes in; + * placement is a suggestion the server re-validates at upload. + */ mediaList: File[]; + annotationFile: File | null; + configFile: File | null; + /** Optional dataset-level file handed to pipelines that request it. */ + metadataFile: File | null; + /** Media/annotation/config package the server validated for upload (rebuilt at start). */ + uploadFiles: File[]; + /** Picked files that have no slot on this row, each with the reason it is not uploaded. */ + unslotted: IgnoredUploadFile[]; + ignored: IgnoredUploadFile[]; + /** Blocking validation message for this row; the slots stay editable until it clears. */ + error: string | null; type: DatasetType | 'zip'; fps: number; uploading: boolean; @@ -94,10 +111,9 @@ interface GirderUpload { name: string; fps: number; type: DatasetType; - mediaList: File[]; - meta?: File | null; - annotationFile?: File | null; + uploadFiles: File[]; skipTranscoding?: boolean; + parentFolderId?: string; }) => Promise<{ folder: { _id: string }; jobIds: string[] }>; } @@ -124,6 +140,54 @@ function multicamCameraSlotPercent( return MULTICAM_PROGRESS_START + (cameraIndex * span) + (subFraction * span); } +/** + * The server names every file it did not accept, so what to upload is the validated + * selection minus those names. + */ +function acceptedUploadFiles(files: File[], validation: ValidationResponse): File[] { + const ignoredNames = new Set(validation.roles.ignored); + return files.filter((file) => !ignoredNames.has(file.name)); +} + +/** The server's ignored role, each name paired with the reason it reported for it. */ +function validationIgnoredFiles(validation: ValidationResponse): IgnoredUploadFile[] { + return validation.roles.ignored.map((name) => ({ + name, + reason: validation.reasons[name] ?? 'Not accepted for upload', + })); +} + +/** Only a multi-video upload fans out into per-video subfolders. */ +function fansOutToSubFolders(validation: ValidationResponse): boolean { + return validation.type === VideoType && validation.roles.media.length > 1; +} + +/** + * The folder name a row starts with: the single media file's name, or — when the row holds + * several media files — the first one with its suffix stripped. Derived from the server's + * media role, so a row that was rejected on pick still gets a name once it validates. + */ +function defaultRowName(validation: ValidationResponse): string { + const [firstMedia = ''] = validation.roles.media; + return validation.roles.media.length > 1 + ? firstMedia.replace(fileSuffixRegex, '') + : firstMedia; +} + +/** + * Every picked file this row will not upload, with the reason: what the server ignored, and + * what had no slot to go in. + */ +function rowIgnoredFiles( + row: Pick, + validation: ValidationResponse, +): IgnoredUploadFile[] { + return [ + ...validationIgnoredFiles(validation), + ...row.unslotted, + ]; +} + export default defineComponent({ components: { ImportButton, @@ -140,6 +204,8 @@ export default defineComponent({ setup(props, { emit }) { const preUploadErrorMessage: Ref = ref(null); const pendingUploads: Ref = ref([]); + /** True from the moment Start upload is clicked until its upload settles. */ + const preparing = ref(false); const stereo = ref(false); const multiCamOpenType = ref('image-sequence'); const importMultiCamDialog = ref(false); @@ -194,150 +260,119 @@ export default defineComponent({ createSubFolders: false, name: defaultFilename, files: [], //Will be set in the GirderUpload Component - meta: null, - metadataFile: null, - annotationFile: null, mediaList: allFiles, + annotationFile: null, + configFile: null, + metadataFile: null, + uploadFiles: allFiles, + unslotted: [], + ignored: [], + error: null, type: 'zip', fps, uploading: false, }); }; + // Accept filters per slot, mirroring the file dialog's own filters. + const filterFileUpload = (type: DatasetType | 'meta' | 'annotation' | 'metadata') => { + if (type === 'meta') { + return '.json'; + } + if (type === 'annotation') { + return inputAnnotationFileTypes.map((item) => `.${item}`).join(','); + } + if (type === 'metadata') { + return metadataFileTypes.map((item) => `.${item}`).join(','); + } + if (type === 'video') { + return websafeVideoTypes.concat(otherVideoTypes); + } + if (type === 'large-image') { + return getLargeImageFileAccept(); + } + return websafeImageTypes.concat(otherImageTypes); + }; + + /** + * Until a row validates, its type is only the import menu the user opened, which the + * server may yet contradict (an image-sequence pick of TIFFs validates as large-image). + * An error row is therefore left unfiltered: filtering by the guess would hide the very + * files the user has to re-pick to clear the error. + */ + const mediaSlotAccept = (pendingUpload: PendingUpload) => ( + pendingUpload.error ? undefined : filterFileUpload(pendingUpload.type) + ); + + // Every slot file the server validates, in a single list. + const slotFileList = (pendingUpload: PendingUpload): File[] => [ + ...pendingUpload.mediaList, + ...(pendingUpload.annotationFile ? [pendingUpload.annotationFile] : []), + ...(pendingUpload.configFile ? [pendingUpload.configFile] : []), + ]; + const addPendingUpload = async ( - name: string, allFiles: File[], - meta: File | null, - annotationFile: File | null, - mediaList: File[], suggestedFps?: number, // suggested FPS for large/images expectedType?: DatasetType, ) => { - const resp = (await validateUploadGroup(allFiles.map((f) => f.name))).data; - if (!resp.ok) { - if (resp.message) { - preUploadErrorMessage.value = resp.message; - } - throw new Error(resp.message); - } - const uploadType = expectedType === LargeImageType ? LargeImageType : resp.type; - const fps = suggestedFps || clientSettings.annotationFPS || DefaultVideoFPS; - const defaultFilename = resp.media[0]; - const validFiles = resp.media.concat(resp.annotations); - // mapping needs to be done for the mixin upload functions - const internalFiles = allFiles - .filter((f) => validFiles.includes(f.name)); - let createSubFolders = false; - if (resp.type === 'video') { - if (resp.media.length > 1) { - createSubFolders = true; - } - } - pendingUploads.value.push({ - createSubFolders, - name: - internalFiles.length > 1 - ? defaultFilename.replace(fileSuffixRegex, '') - : defaultFilename, + const slots = suggestUploadSlots(allFiles); + // Validate the slotted selection so the media type is server-determined. + const row: PendingUpload = { + createSubFolders: false, + name: '', files: [], //Will be set in the GirderUpload Component - meta, + mediaList: slots.mediaList, + annotationFile: slots.annotationFile, + configFile: slots.configFile, metadataFile: null, - annotationFile, - mediaList, - type: uploadType, - fps, + uploadFiles: [], + unslotted: slots.unslotted, + ignored: [...slots.unslotted], + error: null, + type: expectedType ?? ImageSequenceType, + fps: suggestedFps || clientSettings.annotationFPS || DefaultVideoFPS, uploading: false, skipTranscoding: true, - }); - }; - /** - * Processes the imported media files to distinguish between - * Media Files - Default files that aren't the Annotation or Meta - * Annotation File - CSV or JSON file, or more in the future - * Meta File - Right now a json file which has 'meta' or 'config in the name - */ - const processImport = (files: { - canceled: boolean; filePaths: string[]; fileList?: File[]; - }) => { - //Check for auto files for meta and annotations - const output: { - annotationFile: null | File; - metaFile: null | File; - mediaList: File[]; - fullList: File[]; - } = { - annotationFile: null, - metaFile: null, - mediaList: [], - fullList: [], }; - const jsonFiles: [string, number][] = []; - const csvFiles: [string, number][] = []; - if (files.fileList) { - files.filePaths.forEach((item, index) => { - if (item.indexOf('.json') !== -1) { - jsonFiles.push([item, index]); - } else if (item.indexOf('.csv') !== -1) { - csvFiles.push([item, index]); - } - }); - output.mediaList = files.fileList.filter((item) => ( - item.name.indexOf('.json') === -1 && item.name.indexOf('.csv') === -1)); - const metaIndex = jsonFiles.findIndex((item) => (JsonMetaRegEx.test(item[0]))); - if (metaIndex !== -1) { - output.metaFile = files.fileList[jsonFiles[metaIndex][1]]; - jsonFiles.splice(metaIndex, 1); //remove chosen meta from list - } - if (jsonFiles.length === 1 && csvFiles.length === 0) { // only remaining json file - output.annotationFile = files.fileList[jsonFiles[0][1]]; - } else if (csvFiles.length) { // Prefer First CSV if both found - output.annotationFile = files.fileList[csvFiles[0][1]]; - } else if (jsonFiles.length > 1) { //multiple jsons, filter out additional meta/configs - const filtered = jsonFiles.filter((item) => (!JsonMetaRegEx.test(item[0]) && (item[0].indexOf('.json') !== -1))); - if (filtered.length) { // take first filtered JSON file - output.annotationFile = files.fileList[filtered[0][1]]; - } - } - output.fullList = [...output.mediaList]; - if (output.annotationFile) { - output.fullList.push(output.annotationFile); - } - if (output.metaFile) { - output.fullList.push(output.metaFile); - } + const slotFiles = slotFileList(row); + const validation = (await validateUploadGroup(slotFiles.map((f) => f.name))).data; + // A rejected selection still gets a row: its slot editor is the only place the user + // can correct the problem without reopening the OS file picker. + if (!validation.ok) { + row.error = validation.message || 'Upload validation failed'; + pendingUploads.value.push(row); + return; } - return output; + // Server validation is authoritative for the media/annotation/config roles. + row.createSubFolders = fansOutToSubFolders(validation); + row.name = defaultRowName(validation); + row.uploadFiles = acceptedUploadFiles(slotFiles, validation); + row.ignored = rowIgnoredFiles(row, validation); + row.type = expectedType === LargeImageType ? LargeImageType : validation.type; + pendingUploads.value.push(row); }; /** - * Initial opening of file dialog + * Initial opening of file dialog. The complete selection is sent to server + * validation, which is what classifies the files. */ const openImport = async (dstype: DatasetType | 'zip') => { const ret = await openFromDisk(dstype); - if (!ret.canceled && ret.fileList) { - const processed = processImport(ret); - if (processed?.fullList?.length === 0) return; - if (processed && processed.fullList) { - const name = processed.fullList.length === 1 ? processed.fullList[0].name : ''; - preUploadErrorMessage.value = null; - try { - if (dstype !== 'zip') { - const suggestedFps = dstype === 'image-sequence' || dstype === 'large-image' ? 1 : undefined; - await addPendingUpload( - name, - processed.fullList, - processed.metaFile, - processed.annotationFile, - processed.mediaList, - suggestedFps, - dstype, - ); - } else { - addPendingZipUpload(name, processed.fullList); - } - } catch (err) { - preUploadErrorMessage.value = err.response?.data?.message || err; - } + if (ret.canceled || !ret.fileList || ret.fileList.length === 0) { + return; + } + const allFiles = ret.fileList; + preUploadErrorMessage.value = null; + try { + if (dstype === 'zip') { + const name = allFiles.length === 1 ? allFiles[0].name : ''; + addPendingZipUpload(name, allFiles); + } else { + const suggestedFps = dstype === 'image-sequence' || dstype === 'large-image' ? 1 : undefined; + await addPendingUpload(allFiles, suggestedFps, dstype); } + } catch (err) { + preUploadErrorMessage.value = err.response?.data?.message || err; } }; const openMultiCamDialog = (args: { stereo: boolean; openType: 'image-sequence' | 'video' }) => { @@ -345,26 +380,12 @@ export default defineComponent({ multiCamOpenType.value = args.openType; importMultiCamDialog.value = true; }; - const filterFileUpload = (type: DatasetType | 'meta' | 'annotation' | 'metadata') => { - if (type === 'meta') { - return '.json'; - } if (type === 'metadata') { - return metadataFileTypes.map((item) => `.${item}`).join(','); - } if (type === 'annotation') { - return inputAnnotationFileTypes.map((item) => `.${item}`).join(','); - } if (type === 'video') { - return websafeVideoTypes.concat(otherVideoTypes); - } if (type === 'large-image') { - return getLargeImageFileAccept(); - } - return websafeImageTypes.concat(otherImageTypes); - }; - const multiCamImportCheck = (sourcePath: string): MediaImportResponse => { const files = getFilesForSourceKey(sourcePath) ?? []; + const mediaType = multiCamOpenType.value === VideoType ? VideoType : ImageSequenceType; return { jsonMeta: { - originalImageFiles: files.map((file) => file.name), + originalImageFiles: mediaFileNamesForImport(files, mediaType), }, globPattern: '', mediaConvertList: [], @@ -463,19 +484,28 @@ export default defineComponent({ .filter((name) => args.sourceList[name]) .map((name) => [name, args.sourceList[name]] as const); const totalCameras = cameraEntries.length; + // Collect files the server accepted-but-ignored per camera so they can be + // surfaced before navigation — no selected file is silently dropped. + const ignoredAcrossCameras: { camera: string; name: string; reason: string }[] = []; for (let i = 0; i < cameraEntries.length; i += 1) { const [cameraName, source] = cameraEntries[i]; - let files = flattenUploadFiles(getFilesForSourceKey(source.sourcePath) ?? []); + let folderFiles = getFilesForSourceKey(source.sourcePath) ?? []; if (source.glob) { // Cameras sharing one folder (per-modality suffixes) upload only their glob's files - files = files.filter((file) => filterByGlob(source.glob as string, [file.name]).length === 1); + folderFiles = folderFiles.filter((file) => filterByGlob(source.glob as string, [file.name]).length === 1); } - if (!files?.length) { + if (!folderFiles.length) { throw new Error(`No media files found for camera "${cameraName}"`); } + // Flatten before validation so the names the server validates match the + // names uploaded to Girder. + const { files: cameraFiles, replaced } = getCameraPackageFiles( + folderFiles, + source.trackFile, + ); // eslint-disable-next-line no-await-in-loop -- validate then upload each camera sequentially - const validation = (await validateUploadGroup(files.map((f) => f.name))).data; + const validation = (await validateUploadGroup(cameraFiles.map((f) => f.name))).data; if (!validation.ok) { throw new Error(validation.message || `Invalid files for camera "${cameraName}"`); } @@ -485,18 +515,25 @@ export default defineComponent({ if (!compatibleTypes.has(uploadType)) { throw new Error(`Camera "${cameraName}" must use ${cameraType} media`); } - const mediaList = files.filter((file) => validation.media.includes(file.name)); - const annotationFile = source.trackFile - ? getAnnotationFile(source.trackFile) - : undefined; + // Server validation is the authority: upload exactly what it accepted, + // which includes validated camera-folder sidecars and annotation/config + // files, not just validation.roles.media. + const uploadFiles = acceptedUploadFiles(cameraFiles, validation); + validationIgnoredFiles(validation).forEach((entry) => ignoredAcrossCameras.push({ + camera: cameraName, name: entry.name, reason: entry.reason, + })); + replaced.forEach((entry) => ignoredAcrossCameras.push({ + camera: cameraName, + name: entry.name, + reason: 'a different file you chose for this camera has the same name', + })); trackMulticamCameraUploadProgress(i, totalCameras, cameraName); // eslint-disable-next-line no-await-in-loop const { folder, jobIds } = await uploadComponent.uploadCameraDataset({ name: cameraName, fps, type: uploadType, - mediaList, - annotationFile: annotationFile ?? null, + uploadFiles, skipTranscoding: true, parentFolderId: datasetFolder._id, }); @@ -598,6 +635,19 @@ export default defineComponent({ }); } + if (ignoredAcrossCameras.length) { + await prompt({ + title: 'Some files were not uploaded', + text: [ + 'These selected files were not needed for the dataset and were left out:', + ...ignoredAcrossCameras.map( + (entry) => `${entry.camera}: ${entry.name} — ${entry.reason}`, + ), + ], + positiveButton: 'OK', + }); + } + if (openViewer) { setMulticamImportProgress(100, `${labelPrefix}Opening viewer…`); clearMulticamFileRegistry(); @@ -732,9 +782,51 @@ export default defineComponent({ const getFilenameInputValue = (pendingUpload: PendingUpload) => ( pendingUpload.createSubFolders && pendingUpload.type !== 'zip' ? 'default' : pendingUpload.name ); + /** + * Rebuild every non-zip row's validated media/annotation/config package from its + * (possibly edited) slots, then start the shared Girder upload. The server stays the + * authority for those roles; the metadata attachment is uploaded separately. + */ + const prepAndUpload = async (uploadFn: () => Promise) => { + // Validation is an awaited round-trip, so without this gate a second click starts the + // whole upload again before the first one has set any row's `uploading` flag. + if (preparing.value || uploading.value) { + return; + } + preparing.value = true; + preUploadErrorMessage.value = null; + try { + for (let i = 0; i < pendingUploads.value.length; i += 1) { + const pendingUpload = pendingUploads.value[i]; + if (pendingUpload.type !== 'zip') { + const slotFiles = slotFileList(pendingUpload); + // eslint-disable-next-line no-await-in-loop -- validate each row before upload + const validation = (await validateUploadGroup(slotFiles.map((f) => f.name))).data; + if (!validation.ok) { + pendingUpload.error = validation.message || 'Upload validation failed'; + return; + } + pendingUpload.error = null; + // A row rejected on pick has no name yet; name it now that the user's + // correction validates, so it never uploads into an unnamed folder. + if (!pendingUpload.name) { + pendingUpload.name = defaultRowName(validation); + } + pendingUpload.createSubFolders = fansOutToSubFolders(validation); + pendingUpload.uploadFiles = acceptedUploadFiles(slotFiles, validation); + pendingUpload.ignored = rowIgnoredFiles(pendingUpload, validation); + } + } + await uploadFn(); + } catch (err) { + preUploadErrorMessage.value = err.response?.data?.message || err.message || String(err); + } finally { + preparing.value = false; + } + }; const remove = (pendingUpload: PendingUpload) => { - const index = pendingUploads.value.indexOf(pendingUpload); - pendingUploads.value.splice(index, 1); + // Identity, not index: a row retired twice must never take a different row with it. + pendingUploads.value = pendingUploads.value.filter((row) => row !== pendingUpload); }; function close() { emit('close'); @@ -788,15 +880,14 @@ export default defineComponent({ girderUpload, multicamImporting, multicamImportProgress, + preparing, uploading, clientSettings, //methods close, closeMultiCamBatchDialog, openImport, - processImport, openMultiCamDialog, - filterFileUpload, multiCamImportCheck, multiCamImport, chooseAndScanBatch, @@ -809,7 +900,9 @@ export default defineComponent({ getFilenameInputValue, getFilenameInputStateDisabled, getFilenameInputStateHint, - addPendingUpload, + filterFileUpload, + mediaSlotAccept, + prepAndUpload, remove, abort, errorHandler, @@ -961,7 +1054,24 @@ export default defineComponent({ - + + {{ pendingUpload.error }} + + + @@ -1000,7 +1110,7 @@ export default defineComponent({ - + +
+
+ Ignored (not uploaded): +
+
+ {{ ignoredFile.name }} ({{ ignoredFile.reason }}) +
+
+ +
+ {{ computeUploadProgress(pendingUpload) }} - +
@@ -1119,12 +1258,12 @@ export default defineComponent({
mdi-upload diff --git a/client/platform/web-girder/views/UploadGirder.spec.ts b/client/platform/web-girder/views/UploadGirder.spec.ts new file mode 100644 index 000000000..e909ecfeb --- /dev/null +++ b/client/platform/web-girder/views/UploadGirder.spec.ts @@ -0,0 +1,29 @@ +// @vitest-environment jsdom +/* eslint-disable import/no-extraneous-dependencies */ +import { mount } from '@vue/test-utils'; +import Vue from 'vue'; + +import { describe, expect, it } from 'vitest'; + +import UploadGirder from './UploadGirder.vue'; + +Vue.config.ignoredElements = [/^v-/]; + +describe('UploadGirder row retirement', () => { + it('names the row to remove so the parent never retires a different one', () => { + const rows = [{ name: 'first' }, { name: 'second' }]; + const wrapper = mount(UploadGirder, { + propsData: { + location: { _id: 'folder-id', _modelType: 'folder' }, + pendingUploads: rows, + }, + provide: { girderRest: {} }, + }); + + wrapper.vm.remove(rows[0]); + + // An index payload would land on the parent's `remove(pendingUpload)` as a number, + // whose indexOf is -1, retiring the last queued row instead. + expect(wrapper.emitted('remove-upload')).toEqual([[rows[0]]]); + }); +}); diff --git a/client/platform/web-girder/views/UploadGirder.vue b/client/platform/web-girder/views/UploadGirder.vue index 418d49d02..a01f85052 100644 --- a/client/platform/web-girder/views/UploadGirder.vue +++ b/client/platform/web-girder/views/UploadGirder.vue @@ -46,8 +46,7 @@ export default Vue.extend({ this.$emit('abort'); }, remove(pendingUpload) { - const index = this.pendingUploads.indexOf(pendingUpload); - this.$emit('remove-upload', index); + this.$emit('remove-upload', pendingUpload); }, async upload() { if (this.location._modelType !== 'folder') { @@ -70,9 +69,15 @@ export default Vue.extend({ break; } } - if (!error) { - this.$emit('update:uploading', false); + if (error) { + // Reset failed/interrupted rows so the dialog recovers: the progress + // spinner stops and each row's controls (remove, FPS) re-enable. + pendingUplodsCopy.forEach((pendingUpload) => { + // eslint-disable-next-line no-param-reassign + pendingUpload.uploading = false; + }); } + this.$emit('update:uploading', false); }, convertFileToInternal(file) { if (file === null) { @@ -92,14 +97,12 @@ export default Vue.extend({ }, async uploadPending(pendingUpload, uploaded) { const { - name, createSubFolders, meta, annotationFile, mediaList, metadataFile, + name, createSubFolders, uploadFiles, metadataFile, } = pendingUpload; - //Combine the files for uploading. The optional metadata file is uploaded - //separately (as a marked item) so postprocess does not classify it. - let files = mediaList.map((item) => this.convertFileToInternal(item)); - files.push(this.convertFileToInternal(meta)); - files.push(this.convertFileToInternal(annotationFile)); - files = files.filter((item) => item !== null); + // The validated package is the only source of files to upload. + const files = uploadFiles + .map(this.convertFileToInternal) + .filter((item) => item !== null); // eslint-disable-next-line no-param-reassign pendingUpload.files = files; const fps = parseInt(pendingUpload.fps, 10); @@ -180,13 +183,12 @@ export default Vue.extend({ * Upload a single camera dataset folder (used by multicam import). */ async uploadCameraDataset({ - name, fps, type, mediaList, meta = null, annotationFile = null, skipTranscoding = true, - parentFolderId = null, + name, fps, type, uploadFiles, skipTranscoding = true, parentFolderId = null, }) { - let files = mediaList.map((item) => this.convertFileToInternal(item)); - files.push(this.convertFileToInternal(meta)); - files.push(this.convertFileToInternal(annotationFile)); - files = files.filter((item) => item !== null); + // The validated package is the only source of files to upload for the camera. + const files = uploadFiles + .map(this.convertFileToInternal) + .filter((item) => item !== null); const folder = await this.createUploadFolder(name, parseInt(fps, 10), type, parentFolderId); if (!folder) { throw new Error(`Failed to create folder for camera ${name}`); diff --git a/server/dive_server/crud_dataset.py b/server/dive_server/crud_dataset.py index feb8b6792..6d5957c04 100644 --- a/server/dive_server/crud_dataset.py +++ b/server/dive_server/crud_dataset.py @@ -1012,9 +1012,9 @@ def _source_calibration_items_in_folder_root(folder_id: str): Camera media lives in child folders; only direct items on folder_id are considered. """ for cal_item in Item().find({'folderId': _mongo_id(folder_id)}, sort=[('created', -1)]): - if _item_has_source_calibration_marker(cal_item) and constants.stereoCalibrationRegex.search( - cal_item['name'] - ): + if _item_has_source_calibration_marker( + cal_item + ) and constants.stereoCalibrationRegex.search(cal_item['name']): yield cal_item @@ -1447,57 +1447,100 @@ def create_multicam( return parent_folder_doc +UNSUPPORTED_SIDE_FILE_REASON = "Unsupported side file" + + def validate_files(files: List[str]): """ - Given a collection of filenames, guess based on regular expressions - if the collection represents a valid dataset, and if so, which files - represent which type of data + Given a collection of filenames, classify each into a semantic upload role. + + Every filename appears under exactly one key of ``roles``; files that are not needed get + the ``ignored`` role, and ``reasons`` maps a filename to why it landed there. Only the + interactive browser upload honours the per-file drop, uploading the selection minus + ``roles['ignored']`` so nothing is discarded without a reason the user can see. The + girder-worker zip path (``dive_tasks.utils.upload_zipped_flat_media_files``) uses this as a + whole-archive gate and then uploads every extracted file. + + ``type`` is present only when ``ok``: a rejected selection has no single media type. """ - ok = True - message = "" - mediatype = "" videos = [f for f in files if constants.videoRegex.search(f)] - csvs = [f for f in files if constants.csvRegex.search(f)] images = [f for f in files if constants.imageRegex.search(f)] large_images = [f for f in files if constants.largeImageRegEx.search(f)] - ymls = [f for f in files if constants.ymlRegex.search(f)] - jsons = [f for f in files if constants.jsonRegex.search(f)] + media = images + videos + large_images + + # Dataset config JSON follows the same meta/config filename contract as the client's + # JsonMetaRegEx; annotation JSON is every other .json. + dataset_config = [ + f for f in files if constants.jsonRegex.search(f) and constants.metaRegex.search(f) + ] + dataset_config_set = set(dataset_config) + + annotation_csvs = [f for f in files if constants.csvRegex.search(f)] + annotation_ymls = [f for f in files if constants.ymlRegex.search(f)] + annotation_jsons = [ + f for f in files if constants.jsonRegex.search(f) and f not in dataset_config_set + ] + annotations = annotation_csvs + annotation_ymls + annotation_jsons + + if len(videos): + mediatype = constants.VideoType + elif len(images): + mediatype = constants.ImageSequenceType + elif len(large_images): + mediatype = constants.LargeImageType + else: + mediatype = "" + + ok = True + message = "" if len(videos) and (len(images) or len(large_images)): ok = False message = "Do not upload images and videos in the same batch." elif len(large_images) and len(images): ok = False message = "Do not upload images and tile images in the same batch." - elif len(csvs) > 1: + elif len(annotation_csvs) > 1: ok = False message = "Can only upload a single CSV Annotation per import" - elif len(jsons) > 2: + elif len(dataset_config) > 1: ok = False - message = ( - "Can only upload a single JSON Annotation and single configuration JSON per import" - ) - elif len(csvs) == 1 and len(ymls): + message = "Can only upload a single configuration JSON per import" + elif len(annotation_jsons) > 1: + ok = False + message = "Can only upload a single annotation JSON per import" + elif len(annotation_csvs) and len(annotation_ymls): + ok = False + message = "Cannot mix annotation import types" + elif len(annotation_ymls) > 1: + ok = False + message = "Can only upload a single YAML Annotation per import" + elif len(annotation_csvs) + len(annotation_ymls) + len(annotation_jsons) > 1: + # Multiple annotation sources across formats (e.g. CSV + JSON) would silently + # overwrite each other at import, so only one annotation source is allowed. ok = False message = "Cannot mix annotation import types" - elif len(videos) > 1 and (len(csvs) or len(ymls) or len(jsons)): + elif len(videos) > 1 and (len(annotations) or len(dataset_config)): ok = False message = "Annotation upload is not supported when multiple videos are uploaded" - elif (not len(videos)) and (not len(images)) and (not len(large_images)): + elif not (len(videos) or len(images) or len(large_images)): ok = False message = "No supported media-type files found" - elif len(videos): - mediatype = constants.VideoType - elif len(images): - mediatype = constants.ImageSequenceType - elif len(large_images): - mediatype = constants.LargeImageType + + accepted = set(media) | set(annotations) | set(dataset_config) + ignored = [f for f in files if f not in accepted] return { "ok": ok, + # Only an accepted selection has a media type. + **({"type": mediatype} if ok else {}), "message": message, - "type": mediatype, - "media": images + videos + large_images, - "annotations": csvs + ymls + jsons, + "roles": { + "media": media, + "annotations": annotations, + "datasetConfig": dataset_config, + "ignored": ignored, + }, + "reasons": {f: UNSUPPORTED_SIDE_FILE_REASON for f in ignored}, } diff --git a/server/dive_tasks/utils.py b/server/dive_tasks/utils.py index 118b9d074..72e67a237 100644 --- a/server/dive_tasks/utils.py +++ b/server/dive_tasks/utils.py @@ -350,8 +350,8 @@ def upload_zipped_flat_media_files( root_folderId = folderId default_fps = gc.getFolder(root_folderId).get(f"meta.{constants.FPSMarker}", -1) if validation.get('ok', False): - manager.write(f"Annotations: {validation['annotations']}\n") - manager.write(f"Media: {validation['media']}\n") + manager.write(f"Annotations: {validation['roles']['annotations']}\n") + manager.write(f"Media: {validation['roles']['media']}\n") dataset_type = validation['type'] manager.write(f"Type: {dataset_type}\n") if create_subfolder != '': diff --git a/server/tests/test_validate_files.py b/server/tests/test_validate_files.py index b26f27e94..dc68fef15 100644 --- a/server/tests/test_validate_files.py +++ b/server/tests/test_validate_files.py @@ -1,29 +1,177 @@ -from dive_server import crud_dataset +from dive_server.crud_dataset import UNSUPPORTED_SIDE_FILE_REASON, validate_files from dive_utils import constants +def test_response_shape(): + # `roles` is the single owner of what each file is for, ignored included, and the client + # uploads the selection minus that role. + result = validate_files(['image_0001.jpg', 'tracks.csv']) + + assert set(result) == {"ok", "message", "type", "roles", "reasons"} + assert set(result["roles"]) == {"media", "annotations", "datasetConfig", "ignored"} + + +def test_image_sequence_with_yaml_annotation(): + result = validate_files(['image_0001.jpg', 'annotations.yml']) + + assert result['ok'] is True + assert result['type'] == constants.ImageSequenceType + assert 'annotations.yml' in result['roles']['annotations'] + + +def test_image_sequence_with_plain_txt_is_ignored(): + result = validate_files(['image_0001.jpg', 'notes.txt']) + + assert result['ok'] is True + assert result['type'] == constants.ImageSequenceType + assert 'notes.txt' in result['roles']['ignored'] + assert result['reasons']['notes.txt'] == UNSUPPORTED_SIDE_FILE_REASON + # The rest of the package is unaffected. + assert result['roles']['media'] == ['image_0001.jpg'] + + +def test_image_sequence_with_unsupported_extension_is_ignored(): + result = validate_files(['image_0001.jpg', 'weird.xyz']) + + assert result['ok'] is True + assert 'weird.xyz' in result['roles']['ignored'] + assert result['reasons']['weird.xyz'] == UNSUPPORTED_SIDE_FILE_REASON + + +def test_two_plain_annotation_csvs_are_rejected(): + result = validate_files(['image_0001.jpg', 'a.csv', 'b.csv']) + + assert result['ok'] is False + assert result['message'] == "Can only upload a single CSV Annotation per import" + + +def test_image_sequence_with_config_json_is_dataset_config(): + result = validate_files(['image_0001.jpg', 'meta.json']) + + assert result['ok'] is True + assert 'meta.json' in result['roles']['datasetConfig'] + assert 'meta.json' not in result['roles']['annotations'] + assert 'meta.json' not in result['roles']['ignored'] + + +def test_annotation_json_and_config_json_are_distinguished(): + result = validate_files(['image_0001.jpg', 'tracks.json', 'config.json']) + + assert result['ok'] is True + assert 'config.json' in result['roles']['datasetConfig'] + assert 'tracks.json' in result['roles']['annotations'] + assert 'config.json' not in result['roles']['annotations'] + + +def test_every_role_is_populated_for_a_full_selection(): + result = validate_files(['image_0001.jpg', 'tracks.csv', 'meta.json']) + + assert result['ok'] is True + assert result['roles'] == { + 'media': ['image_0001.jpg'], + 'annotations': ['tracks.csv'], + 'datasetConfig': ['meta.json'], + 'ignored': [], + } + assert result['reasons'] == {} + + +def test_images_and_videos_mixed_is_rejected(): + result = validate_files(['image_0001.jpg', 'movie.mp4']) + + assert result['ok'] is False + # A rejected selection carries no media type. + assert 'type' not in result + assert result['message'] == "Do not upload images and videos in the same batch." + + +def test_csv_and_yaml_mixed_is_rejected(): + result = validate_files(['image_0001.jpg', 'tracks.csv', 'config.yml']) + + assert result['ok'] is False + assert result['message'] == "Cannot mix annotation import types" + + +def test_csv_and_annotation_json_mixed_is_rejected(): + # Two annotation sources of different formats would silently overwrite at import. + result = validate_files(['image_0001.jpg', 'tracks.csv', 'tracks.json']) + + assert result['ok'] is False + assert result['message'] == "Cannot mix annotation import types" + + +def test_yaml_and_annotation_json_mixed_is_rejected(): + result = validate_files(['image_0001.jpg', 'tracks.yml', 'tracks.json']) + + assert result['ok'] is False + assert result['message'] == "Cannot mix annotation import types" + + +def test_multiple_videos_with_config_json_is_rejected(): + # A single dataset-config JSON cannot apply to a multi-video (subfolder) upload. + result = validate_files(['a.mp4', 'b.mp4', 'config.json']) + + assert result['ok'] is False + assert result['message'] == "Annotation upload is not supported when multiple videos are uploaded" + + +def test_multiple_videos_without_annotations_is_allowed(): + result = validate_files(['a.mp4', 'b.mp4']) + + assert result['ok'] is True + assert result['type'] == constants.VideoType + assert result['roles']['media'] == ['a.mp4', 'b.mp4'] + + +def test_no_media_is_rejected(): + result = validate_files(['tracks.csv']) + + assert result['ok'] is False + assert result['message'] == "No supported media-type files found" + + def test_validate_files_tiff_as_large_image(): files = [ 'kamera_2021_test_fl01_C_20210814_003347.198347_ir.tif', 'kamera_2021_test_fl01_C_20210814_003353.208198_ir.tif', ] - result = crud_dataset.validate_files(files) + result = validate_files(files) assert result['ok'] is True assert result['type'] == constants.LargeImageType - assert result['media'] == files + assert result['roles']['media'] == files def test_validate_files_jpg_as_image_sequence(): files = ['frame.jpg', 'frame2.jpg'] - result = crud_dataset.validate_files(files) + result = validate_files(files) assert result['ok'] is True assert result['type'] == constants.ImageSequenceType - assert result['media'] == files + assert result['roles']['media'] == files def test_validate_files_nitf_remains_large_image(): files = ['scene.nitf'] - result = crud_dataset.validate_files(files) + result = validate_files(files) assert result['ok'] is True assert result['type'] == constants.LargeImageType - assert result['media'] == files + assert result['roles']['media'] == files + + +def test_ignored_is_the_exact_complement_of_the_accepted_roles(): + # The web client uploads the selection minus `ignored`, so a file that is neither given a + # role nor listed as ignored would silently not upload. Pin the partition over a selection + # that reaches every branch: media, annotations, dataset config, and two side files. + files = [ + 'image_0001.jpg', + 'image_0002.jpg', + 'tracks.csv', + 'config.json', + 'notes.md', + 'thumbnail.png.bak', + ] + + result = validate_files(files) + + classified = [name for role in result['roles'].values() for name in role] + assert sorted(classified) == sorted(files) + assert set(result['reasons']) == set(result['roles']['ignored'])