diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts index c79caeaf8d..6b18efdc33 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts @@ -1,10 +1,12 @@ +import path from 'path-browserify' import type { OutputStream, - OutputFileData, ProjectFileData, PyodideWorkerRequest, PyodideWorkerResponse, } from './pyodide-worker-messages' +import type { BatchUploadItem } from '@/infrastructure/batch-file-uploader' +import type { FileUploader } from './python-runner' export type OutputCallback = ( stream: OutputStream, @@ -23,7 +25,7 @@ export type LifecycleCallback = ( executionId: string success: boolean outputs: string[] - outputFiles: OutputFileData[] + failedUploads: string[] } ) => void @@ -37,17 +39,20 @@ export class PyodideWorkerClient { private pendingMessages: PyodideWorkerRequest[] = [] private outputCallback: OutputCallback | null private lifecycleCallback: LifecycleCallback | null + private fileUploader: FileUploader constructor(options: { baseAssetPath: string createWorker: () => Worker onOutput?: OutputCallback onLifecycle?: LifecycleCallback + fileUploader: FileUploader }) { this.baseAssetPath = options.baseAssetPath this.createWorker = options.createWorker this.outputCallback = options.onOutput ?? null this.lifecycleCallback = options.onLifecycle ?? null + this.fileUploader = options.fileUploader this.worker = this.createWorker() this.worker.addEventListener('message', this.receive) @@ -119,7 +124,7 @@ export class PyodideWorkerClient { } } - private receive = (event: MessageEvent) => { + private receive = async (event: MessageEvent) => { // Discard messages from a previously terminated worker if (event.target !== this.worker) { return @@ -158,15 +163,55 @@ export class PyodideWorkerClient { ) return - case 'run-code-result': + case 'run-code-result': { + let success = response.success + const failedUploads: string[] = [] + + if (success && response.outputFiles.length > 0) { + const items: BatchUploadItem[] = response.outputFiles.map(file => ({ + file: new Blob([file.content as Uint8Array]), + name: path.basename(file.relativePath), + relativePath: file.relativePath, + })) + + try { + const results = await this.fileUploader(items) + for (const result of results) { + if (result.status === 'error') { + failedUploads.push(result.relativePath!) + this.outputCallback?.( + 'stderr', + `Failed to upload output file ${result.relativePath!}: ${result.error}`, + response.fileId, + response.executionId + ) + } + } + if (failedUploads.length > 0) { + success = false + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + this.outputCallback?.( + 'stderr', + `Failed to upload output files: ${message}`, + response.fileId, + response.executionId + ) + failedUploads.push(...items.map(item => item.relativePath!)) + success = false + } + } + this.lifecycleCallback?.({ type: 'run-finished', fileId: response.fileId, executionId: response.executionId, - success: response.success, + success, outputs: response.outputs, - outputFiles: response.outputFiles, + failedUploads, }) + } } } } diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts b/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts index 4aa9a12025..bf042339df 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts @@ -5,6 +5,12 @@ import { v4 as uuid } from 'uuid' import { debugConsole } from '@/utils/debugging' import { PyodideWorkerClient } from './pyodide-worker-client' import type { OutputStream } from './pyodide-worker-messages' +import type { + BatchUploadItem, + UploadResult, +} from '@/infrastructure/batch-file-uploader' + +export type FileUploader = (items: BatchUploadItem[]) => Promise const MAX_OUTPUT_LINES = 100 @@ -45,6 +51,8 @@ export class PythonRunner { private readonly baseAssetPath: string private readonly createWorker: () => Worker private readonly getExecutionContext: () => Promise + private readonly fileUploader: FileUploader + private listeners = new Set() private activeExecutionId: string | null = null @@ -54,12 +62,14 @@ export class PythonRunner { fileId: string, baseAssetPath: string, getExecutionContext: () => Promise, - createWorker: () => Worker + createWorker: () => Worker, + fileUploader: FileUploader ) { this.fileId = fileId this.baseAssetPath = baseAssetPath this.createWorker = createWorker this.getExecutionContext = getExecutionContext + this.fileUploader = fileUploader } subscribe = (listener: Listener): (() => void) => { @@ -100,6 +110,7 @@ export class PythonRunner { this.client = new PyodideWorkerClient({ baseAssetPath: this.baseAssetPath, createWorker: this.createWorker, + fileUploader: this.fileUploader, onLifecycle: event => { switch (event.type) { case 'loaded': @@ -111,15 +122,17 @@ export class PythonRunner { this.updateState({ status: 'errored', error: event.error }) return - case 'run-finished': + case 'run-finished': { if ( event.fileId !== this.fileId || this.activeExecutionId !== event.executionId ) { return } + this.activeExecutionId = null this.updateState({ status: 'finished' }) + } } }, onOutput: (stream, line, fileId, executionId) => { diff --git a/services/web/frontend/js/features/ide-react/context/python-execution-context.tsx b/services/web/frontend/js/features/ide-react/context/python-execution-context.tsx index 417bbd79cd..e7767e7b3a 100644 --- a/services/web/frontend/js/features/ide-react/context/python-execution-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/python-execution-context.tsx @@ -12,6 +12,11 @@ import getMeta from '@/utils/meta' import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree-path' import { useEditorManagerContext } from '@/features/ide-react/context/editor-manager-context' import { useProjectContext } from '@/shared/context/project-context' +import { useFileTreeData } from '@/shared/context/file-tree-data-context' +import { + uploadBatch, + BatchUploadItem, +} from '@/infrastructure/batch-file-uploader' import { PythonRunner, ExecutionContext, @@ -41,14 +46,22 @@ export const PythonExecutionProvider: FC = ({ children, }) => { const { openDocs } = useEditorManagerContext() - const { projectSnapshot } = useProjectContext() + const { projectId, projectSnapshot } = useProjectContext() const { pathInFolder } = useFileTreePathContext() + const { fileTreeData } = useFileTreeData() const runnersRef = useRef(new Map()) const baseAssetPathRef = useRef(null) const pathInFolderRef = useRef(pathInFolder) pathInFolderRef.current = pathInFolder + // Ref so the upload closure built into each PythonRunner reads the + // current value at call time rather than capturing a potentially-stale + // value from when the runner was constructed (fileTreeData may load + // after the runner is created). + const fileTreeDataRef = useRef(fileTreeData) + fileTreeDataRef.current = fileTreeData + // Refreshes the project snapshot and resolves the source code and all project // files for the given fileId, to be passed to the executor for running. const getExecutionContext = useCallback( @@ -95,17 +108,31 @@ export const PythonExecutionProvider: FC = ({ ).toString() } + const uploadOutputFiles = (items: BatchUploadItem[]) => { + const folderId = fileTreeDataRef.current?._id + if (!folderId) { + return Promise.reject( + new Error('File tree not loaded; cannot upload output files') + ) + } + return uploadBatch(items, { + projectId, + folderId, + }) + } + const runner = new PythonRunner( fileId, baseAssetPathRef.current, () => getExecutionContext(fileId), - createPyodideWorker + createPyodideWorker, + uploadOutputFiles ) runner.init() runnersRef.current.set(fileId, runner) return runner }, - [getExecutionContext] + [getExecutionContext, projectId] ) useEffect(() => { diff --git a/services/web/frontend/js/infrastructure/batch-file-uploader.ts b/services/web/frontend/js/infrastructure/batch-file-uploader.ts new file mode 100644 index 0000000000..9f18c9e18e --- /dev/null +++ b/services/web/frontend/js/infrastructure/batch-file-uploader.ts @@ -0,0 +1,112 @@ +import pLimit from 'p-limit' +import getMeta from '@/utils/meta' +import { getErrorMessageForStatusCode } from './http-status-messages' + +export type BatchUploadItem = { + file: Blob + name: string + relativePath?: string +} + +export type BatchUploadOptions = { + projectId: string + folderId: string + /** + * Maximum number of uploads to run in parallel. + * Must be greater than 0; non-positive values fall back to the default of 3. + */ + concurrency?: number +} + +export type UploadResult = + | { + status: 'success' + name: string + relativePath?: string + data: unknown + } + | { + status: 'error' + name: string + relativePath?: string + error: string + } + +const DEFAULT_CONCURRENCY = 3 + +export async function uploadBatch( + items: BatchUploadItem[], + options: BatchUploadOptions +): Promise { + if (items.length === 0) { + return [] + } + + const concurrency = + options.concurrency && options.concurrency > 0 + ? options.concurrency + : DEFAULT_CONCURRENCY + const limit = pLimit(concurrency) + return Promise.all(items.map(item => limit(() => uploadOne(item, options)))) +} + +async function uploadOne( + item: BatchUploadItem, + options: BatchUploadOptions +): Promise { + const formData = new FormData() + formData.append('qqfile', item.file, item.name) + formData.append('name', item.name) + if (item.relativePath) { + formData.append('relativePath', item.relativePath) + } + + const url = `/project/${options.projectId}/upload?folder_id=${options.folderId}` + + try { + const response = await fetch(url, { + method: 'POST', + body: formData, + headers: { + 'X-CSRF-TOKEN': getMeta('ol-csrfToken'), + }, + }) + + if (!response.ok) { + const error = await extractErrorMessage(response) + return { + status: 'error', + name: item.name, + relativePath: item.relativePath, + error, + } + } + + const data = await response.json() + return { + status: 'success', + name: item.name, + relativePath: item.relativePath, + data, + } + } catch (err) { + return { + status: 'error', + name: item.name, + relativePath: item.relativePath, + error: err instanceof Error ? err.message : String(err), + } + } +} + +async function extractErrorMessage(response: Response): Promise { + try { + const body = await response.json() + if (typeof body?.error === 'string') { + return body.error + } + } catch { + // JSON body not available — fall back to status-code message + } + return getErrorMessageForStatusCode(response.status) +} diff --git a/services/web/frontend/js/infrastructure/fetch-json.ts b/services/web/frontend/js/infrastructure/fetch-json.ts index 12600763cd..5510bfa9fb 100644 --- a/services/web/frontend/js/infrastructure/fetch-json.ts +++ b/services/web/frontend/js/infrastructure/fetch-json.ts @@ -5,6 +5,7 @@ // - parse JSON response body, unless response is empty import OError from '@overleaf/o-error' import getMeta from '@/utils/meta' +import { getErrorMessageForStatusCode } from './http-status-messages' type FetchPath = string // Custom config types are merged with `fetch`s RequestInit type @@ -29,25 +30,6 @@ export function deleteJSON(path: FetchPath, options?: FetchConfig) { return fetchJSON(path, { ...options, method: 'DELETE' }) } -function getErrorMessageForStatusCode(statusCode?: number) { - if (!statusCode) { - return 'Unknown Error' - } - - const statusCodes: { readonly [K: number]: string } = { - 400: 'Bad Request', - 401: 'Unauthorized', - 403: 'Forbidden', - 404: 'Not Found', - 429: 'Too Many Requests', - 500: 'Internal Server Error', - 502: 'Bad Gateway', - 503: 'Service Unavailable', - } - - return statusCodes[statusCode] ?? `Unexpected Error: ${statusCode}` -} - export class FetchError extends OError { public url: string public options?: RequestInit diff --git a/services/web/frontend/js/infrastructure/http-status-messages.ts b/services/web/frontend/js/infrastructure/http-status-messages.ts new file mode 100644 index 0000000000..5ddd7b76cb --- /dev/null +++ b/services/web/frontend/js/infrastructure/http-status-messages.ts @@ -0,0 +1,19 @@ +export function getErrorMessageForStatusCode(statusCode?: number) { + if (!statusCode) { + return 'Unknown Error' + } + + const statusCodes: { readonly [K: number]: string } = { + 400: 'Bad Request', + 401: 'Unauthorized', + 403: 'Forbidden', + 404: 'Not Found', + 422: 'Unprocessable Entity', + 429: 'Too Many Requests', + 500: 'Internal Server Error', + 502: 'Bad Gateway', + 503: 'Service Unavailable', + } + + return statusCodes[statusCode] ?? `Unexpected Error: ${statusCode}` +} diff --git a/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts b/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts index 814251e0dc..26b2576567 100644 --- a/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts +++ b/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts @@ -1,12 +1,17 @@ import { expect } from 'chai' +import sinon from 'sinon' import { PyodideWorkerClient, type LifecycleCallback, + type OutputCallback, } from '@/features/ide-react/components/editor/python/pyodide-worker-client' +import type { FileUploader } from '@/features/ide-react/components/editor/python/python-runner' import { WorkerMock, createWorker } from './worker-mock' const BASE_ASSET_PATH = 'https://assets.example.test/' +const fileUploaderStub: FileUploader = () => Promise.resolve([]) + describe('PyodideWorkerClient', function () { beforeEach(function () { WorkerMock.instances.length = 0 @@ -16,6 +21,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const worker = WorkerMock.instances[0] @@ -50,6 +56,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const worker = WorkerMock.instances[0] worker.emitMessage({ type: 'listening' }) @@ -78,12 +85,45 @@ describe('PyodideWorkerClient', function () { onLifecycle: event => { lifecycleEvents.push(event) }, + fileUploader: fileUploaderStub, }) const worker = WorkerMock.instances[0] worker.emitMessage({ type: 'listening' }) return { client, worker, lifecycleEvents } } + function setupClientWithUploadTracking(options: { + fileUploader: FileUploader + }) { + const lifecycleEvents: Parameters[0][] = [] + const outputCalls: Parameters[] = [] + + const client = new PyodideWorkerClient({ + baseAssetPath: BASE_ASSET_PATH, + createWorker, + onLifecycle: event => { + lifecycleEvents.push(event) + }, + onOutput: (...args) => { + outputCalls.push(args) + }, + fileUploader: options.fileUploader, + }) + const worker = WorkerMock.instances[0] + worker.emitMessage({ type: 'listening' }) + return { client, worker, lifecycleEvents, outputCalls } + } + + async function waitFor(predicate: () => boolean, timeoutMs = 200) { + const deadline = Date.now() + timeoutMs + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error('waitFor timed out') + } + await new Promise(resolve => setTimeout(resolve, 0)) + } + } + it('emits run-finished lifecycle event from run-code-result', function () { const { client, worker, lifecycleEvents } = setupClientWithLifecycleTracking() @@ -109,7 +149,7 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-3', success: true, outputs: ['/project/output.txt'], - outputFiles: [], + failedUploads: [], }, ]) }) @@ -139,7 +179,7 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-4', success: true, outputs: ['/project/fig1.png', '/project/results/data.csv'], - outputFiles: [], + failedUploads: [], }, ]) }) @@ -169,12 +209,12 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-5', success: true, outputs: [], - outputFiles: [], + failedUploads: [], }, ]) }) - it('surfaces success and outputFiles from run-code-result', function () { + it('surfaces success and outputFiles from run-code-result', async function () { const { client, worker, lifecycleEvents } = setupClientWithLifecycleTracking() @@ -197,6 +237,10 @@ describe('PyodideWorkerClient', function () { ], }) + await waitFor(() => + Boolean(lifecycleEvents.find(e => e.type === 'run-finished')) + ) + const finished = lifecycleEvents.find(e => e.type === 'run-finished') expect(finished).to.deep.equal({ type: 'run-finished', @@ -204,10 +248,7 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-success', success: true, outputs: ['/project/data.csv', '/project/plot.png'], - outputFiles: [ - { relativePath: 'data.csv', content: csvContent }, - { relativePath: 'plot.png', content: pngContent }, - ], + failedUploads: [], }) }) @@ -236,7 +277,7 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-error', success: false, outputs: [], - outputFiles: [], + failedUploads: [], }) }) @@ -265,7 +306,7 @@ describe('PyodideWorkerClient', function () { executionId: 'exec-nowrites', success: true, outputs: [], - outputFiles: [], + failedUploads: [], }) }) @@ -278,6 +319,7 @@ describe('PyodideWorkerClient', function () { onLifecycle: event => { lifecycleEvents.push(event) }, + fileUploader: fileUploaderStub, }) const worker = WorkerMock.instances[0] @@ -302,6 +344,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const worker = WorkerMock.instances[0] @@ -314,11 +357,185 @@ describe('PyodideWorkerClient', function () { expect(worker.terminated).to.equal(true) }) + context('upload behavior', function () { + const successResult = (name: string, relativePath: string) => ({ + status: 'success' as const, + name, + relativePath, + data: { success: true }, + }) + const errorResult = ( + name: string, + relativePath: string, + error: string + ) => ({ + status: 'error' as const, + name, + relativePath, + error, + }) + + function emitRunResult( + worker: WorkerMock, + executionId: string, + outputFiles: Array<{ relativePath: string; content: Uint8Array }>, + success = true + ) { + worker.emitMessage({ + type: 'run-code-result', + fileId: 'main.py', + executionId, + success, + outputs: [], + outputFiles, + }) + } + + const findFinished = ( + lifecycleEvents: Parameters[0][] + ) => lifecycleEvents.find(e => e.type === 'run-finished') + + it('invokes fileUploader with mapped items when run-code-result has output files', async function () { + const uploader = sinon + .stub() + .resolves([successResult('data.csv', 'output/data.csv')]) + const { worker, lifecycleEvents } = setupClientWithUploadTracking({ + fileUploader: uploader, + }) + + emitRunResult(worker, 'exec-up-1', [ + { + relativePath: 'output/data.csv', + content: new TextEncoder().encode('a,b\n1,2'), + }, + ]) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + expect(uploader.calledOnce).to.be.true + const [items] = uploader.firstCall.args + expect(items).to.have.lengthOf(1) + expect(items[0].name).to.equal('data.csv') + expect(items[0].relativePath).to.equal('output/data.csv') + expect(items[0].file).to.be.instanceOf(Blob) + }) + + it('emits success: true and empty failedUploads when all uploads succeed', async function () { + const uploader = sinon + .stub() + .resolves([ + successResult('a.csv', 'a.csv'), + successResult('b.csv', 'b.csv'), + ]) + const { worker, lifecycleEvents } = setupClientWithUploadTracking({ + fileUploader: uploader, + }) + + emitRunResult(worker, 'exec-up-2', [ + { relativePath: 'a.csv', content: new TextEncoder().encode('1') }, + { relativePath: 'b.csv', content: new TextEncoder().encode('2') }, + ]) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + const finished = findFinished(lifecycleEvents) + expect(finished).to.deep.include({ success: true, failedUploads: [] }) + }) + + it('flips success to false and lists failed paths when an upload fails', async function () { + const uploader = sinon + .stub() + .resolves([ + successResult('good.csv', 'good.csv'), + errorResult('bad.csv', 'output/bad.csv', 'duplicate_file_name'), + ]) + const { worker, lifecycleEvents, outputCalls } = + setupClientWithUploadTracking({ fileUploader: uploader }) + + emitRunResult(worker, 'exec-up-3', [ + { relativePath: 'good.csv', content: new TextEncoder().encode('1') }, + { + relativePath: 'output/bad.csv', + content: new TextEncoder().encode('2'), + }, + ]) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + const finished = findFinished(lifecycleEvents) + expect(finished).to.deep.include({ + success: false, + failedUploads: ['output/bad.csv'], + }) + + // user-facing stderr line surfaced via outputCallback + expect(outputCalls).to.have.lengthOf(1) + const [stream, line] = outputCalls[0] + expect(stream).to.equal('stderr') + expect(line).to.include('output/bad.csv') + expect(line).to.include('duplicate_file_name') + }) + + it('lists every file in failedUploads and surfaces a single stderr line when fileUploader rejects', async function () { + const uploader = sinon.stub().rejects(new Error('network down')) + const { worker, lifecycleEvents, outputCalls } = + setupClientWithUploadTracking({ fileUploader: uploader }) + + emitRunResult(worker, 'exec-up-4', [ + { relativePath: 'a.csv', content: new TextEncoder().encode('1') }, + { relativePath: 'b.csv', content: new TextEncoder().encode('2') }, + ]) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + const finished = findFinished(lifecycleEvents) + expect(finished).to.deep.include({ success: false }) + expect(finished) + .to.have.property('failedUploads') + .that.has.members(['a.csv', 'b.csv']) + + expect(outputCalls).to.have.lengthOf(1) + const [stream, line] = outputCalls[0] + expect(stream).to.equal('stderr') + expect(line).to.include('network down') + }) + + it('does not invoke fileUploader when run-code-result has success: false', async function () { + const uploader = sinon.stub().resolves([]) + const { worker, lifecycleEvents } = setupClientWithUploadTracking({ + fileUploader: uploader, + }) + + emitRunResult( + worker, + 'exec-up-5', + [{ relativePath: 'a.csv', content: new TextEncoder().encode('1') }], + false + ) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + expect(uploader.called).to.be.false + expect(findFinished(lifecycleEvents)).to.deep.include({ + success: false, + failedUploads: [], + }) + }) + + it('does not invoke fileUploader when outputFiles is empty', async function () { + const uploader = sinon.stub().resolves([]) + const { worker, lifecycleEvents } = setupClientWithUploadTracking({ + fileUploader: uploader, + }) + + emitRunResult(worker, 'exec-up-6', []) + await waitFor(() => Boolean(findFinished(lifecycleEvents))) + + expect(uploader.called).to.be.false + }) + }) + describe('reset', function () { it('terminates the current worker and creates a new one', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const originalWorker = WorkerMock.instances[0] originalWorker.emitMessage({ type: 'listening' }) @@ -334,6 +551,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const originalWorker = WorkerMock.instances[0] originalWorker.emitMessage({ type: 'listening' }) @@ -357,6 +575,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const originalWorker = WorkerMock.instances[0] originalWorker.emitMessage({ type: 'listening' }) @@ -389,6 +608,7 @@ describe('PyodideWorkerClient', function () { const client = new PyodideWorkerClient({ baseAssetPath: BASE_ASSET_PATH, createWorker, + fileUploader: fileUploaderStub, }) const originalWorker = WorkerMock.instances[0] originalWorker.emitMessage({ type: 'listening' }) diff --git a/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts b/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts index a8f3aef3a3..69fc11d931 100644 --- a/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts +++ b/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts @@ -2,8 +2,10 @@ import { expect } from 'chai' import sinon from 'sinon' import { PythonRunner, + PythonRunnerState, DEFAULT_STATE, ExecutionContext, + type FileUploader, } from '@/features/ide-react/components/editor/python/python-runner' import { WorkerMock, createWorker } from './worker-mock' @@ -14,6 +16,7 @@ function createRunner( overrides: { fileId?: string getExecutionContext?: () => Promise + fileUploader?: FileUploader } = {} ) { const fileId = overrides.fileId ?? FILE_ID @@ -25,11 +28,14 @@ function createRunner( files: [{ relativePath: 'main.py', content: 'print("hello")' }], })) + const fileUploader = overrides.fileUploader ?? sinon.stub().resolves([]) + const runner = new PythonRunner( fileId, BASE_ASSET_PATH, getExecutionContext, - createWorker + createWorker, + fileUploader ) return runner } @@ -42,6 +48,24 @@ function initAndLoad(runner: PythonRunner) { return worker } +function waitForState( + runner: PythonRunner, + predicate: (state: PythonRunnerState) => boolean +): Promise { + return new Promise(resolve => { + if (predicate(runner.getState())) { + resolve(runner.getState()) + return + } + const unsubscribe = runner.subscribe(() => { + if (predicate(runner.getState())) { + unsubscribe() + resolve(runner.getState()) + } + }) + }) +} + describe('PythonRunner', function () { beforeEach(function () { WorkerMock.instances.length = 0 @@ -110,9 +134,12 @@ describe('PythonRunner', function () { type: 'run-code-result', fileId: FILE_ID, executionId: runMsg.executionId, + success: true, outputs: [], + outputFiles: [], }) + await waitForState(runner, s => s.status === 'finished') expect(runner.getState().status).to.equal('finished') }) @@ -133,7 +160,9 @@ describe('PythonRunner', function () { type: 'run-code-result', fileId: FILE_ID, executionId: runMsg.executionId, + success: true, outputs: [], + outputFiles: [], }) expect(runner.getState().output).to.deep.equal([ { stream: 'stdout', line: 'first run output' }, diff --git a/services/web/test/frontend/infrastructure/batch-file-uploader.test.ts b/services/web/test/frontend/infrastructure/batch-file-uploader.test.ts new file mode 100644 index 0000000000..20dfa3b6af --- /dev/null +++ b/services/web/test/frontend/infrastructure/batch-file-uploader.test.ts @@ -0,0 +1,257 @@ +import { expect } from 'chai' +import fetchMock from 'fetch-mock' +import { + uploadBatch, + BatchUploadOptions, +} from '@/infrastructure/batch-file-uploader' + +describe('uploadBatch', function () { + const batchUploadOptions = { + projectId: 'test-project', + folderId: 'test-folder', + } + + const batchUploadItems = [ + { + file: new Blob(['col1,col2\n1,2\n']), + name: 'data.csv', + relativePath: 'output/data.csv', + }, + { + file: new Blob(['hello world']), + name: 'notes.txt', + relativePath: 'output/notes.txt', + }, + { + file: new Blob([new Uint8Array([137, 80, 78, 71])]), + name: 'figure.png', + relativePath: 'output/figure.png', + }, + { + file: new Blob(['{"result":42}']), + name: 'data.json', + }, + ] + + afterEach(function () { + fetchMock.removeRoutes().clearHistory() + }) + + it('returns an empty array and makes no requests when items is empty', async function () { + const results = await uploadBatch([], batchUploadOptions) + + expect(results).to.deep.equal([]) + expect(fetchMock.callHistory.called()).to.be.false + }) + + context('when all uploads succeed', function () { + const expectedUrl = `/project/${batchUploadOptions.projectId}/upload?folder_id=${batchUploadOptions.folderId}` + let results: Awaited> + let calls: ReturnType + + beforeEach(async function () { + fetchMock.post(expectedUrl, { + status: 200, + body: { success: true }, + }) + results = await uploadBatch(batchUploadItems, batchUploadOptions) + calls = fetchMock.callHistory.calls() + }) + + const findRequestFor = (name: string) => + calls.find(c => (c.options.body as FormData).get('name') === name)! + + it('makes one request per item', function () { + expect(calls).to.have.lengthOf(4) + }) + + it('posts each request to the upload URL', function () { + for (const call of calls) { + expect(call.url).to.include(expectedUrl) + } + }) + + it('sets the name form field from the item name', function () { + for (const item of batchUploadItems) { + const body = findRequestFor(item.name).options.body as FormData + expect(body.get('name')).to.equal(item.name) + } + }) + + it('sets the relativePath form field from the item path', function () { + for (const item of batchUploadItems.filter(i => i.relativePath)) { + const body = findRequestFor(item.name).options.body as FormData + expect(body.get('relativePath')).to.equal(item.relativePath) + } + }) + + it('omits the relativePath form field when the item has no relativePath', function () { + for (const item of batchUploadItems.filter(i => !i.relativePath)) { + const body = findRequestFor(item.name).options.body as FormData + expect(body.has('relativePath')).to.be.false + } + }) + + it('attaches the item file as qqfile', function () { + for (const item of batchUploadItems) { + const body = findRequestFor(item.name).options.body as FormData + expect(body.get('qqfile')).to.be.instanceOf(Blob) + } + }) + + it('returns a success result per item with name, relativePath, and server data', function () { + expect(results).to.deep.equal([ + { + status: 'success', + name: 'data.csv', + relativePath: 'output/data.csv', + data: { success: true }, + }, + { + status: 'success', + name: 'notes.txt', + relativePath: 'output/notes.txt', + data: { success: true }, + }, + { + status: 'success', + name: 'figure.png', + relativePath: 'output/figure.png', + data: { success: true }, + }, + { + status: 'success', + name: 'data.json', + relativePath: undefined, + data: { success: true }, + }, + ]) + }) + }) + + context('with mixed upload outcomes', function () { + const expectedUrl = `/project/${batchUploadOptions.projectId}/upload?folder_id=${batchUploadOptions.folderId}` + let results: Awaited> + + beforeEach(async function () { + fetchMock.post(expectedUrl, callLog => { + const name = (callLog.options.body as FormData).get('name') + switch (name) { + case 'data.csv': + return { status: 200, body: { success: true } } + case 'notes.txt': + return { + status: 422, + body: { success: false, error: 'duplicate_file_name' }, + } + case 'figure.png': + return { status: 500, body: {} } + case 'data.json': + return Promise.reject(new Error('network down')) + default: + throw new Error(`unexpected item name: ${name}`) + } + }) + results = await uploadBatch(batchUploadItems, batchUploadOptions) + }) + + it('returns a success result when the upload succeeds', function () { + expect(results[0]).to.deep.equal({ + status: 'success', + name: 'data.csv', + relativePath: 'output/data.csv', + data: { success: true }, + }) + }) + + it('returns the server-provided error string when the response body has one', function () { + expect(results[1]).to.deep.equal({ + status: 'error', + name: 'notes.txt', + relativePath: 'output/notes.txt', + error: 'duplicate_file_name', + }) + }) + + it('falls back to a status-code message when the error body has no error field', function () { + expect(results[2]).to.deep.equal({ + status: 'error', + name: 'figure.png', + relativePath: 'output/figure.png', + error: 'Internal Server Error', + }) + }) + + it('returns the rejection message when fetch rejects', function () { + expect(results[3]).to.deep.equal({ + status: 'error', + name: 'data.json', + relativePath: undefined, + error: 'network down', + }) + }) + }) + + describe('default concurrency', function () { + const expectedUrl = `/project/${batchUploadOptions.projectId}/upload?folder_id=${batchUploadOptions.folderId}` + + const manyItems = Array.from({ length: 6 }, (_, i) => ({ + file: new Blob([`content ${i}`]), + name: `file-${i}.txt`, + })) + + const waitFor = async (predicate: () => boolean, timeoutMs = 200) => { + const deadline = Date.now() + timeoutMs + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error('waitFor timed out') + } + await new Promise(resolve => setTimeout(resolve, 0)) + } + } + + const observeMaxInFlight = async (options: BatchUploadOptions) => { + let inFlight = 0 + let maxInFlight = 0 + let releaseAll!: () => void + const release = new Promise(resolve => { + releaseAll = resolve + }) + + fetchMock.post(expectedUrl, async () => { + inFlight++ + maxInFlight = Math.max(maxInFlight, inFlight) + await release + inFlight-- + return { status: 200, body: { success: true } } + }) + + const batchPromise = uploadBatch(manyItems, options) + await waitFor(() => inFlight === 3) + releaseAll() + await batchPromise + return maxInFlight + } + + it('uses a default cap of 3 when no concurrency is set', async function () { + const max = await observeMaxInFlight(batchUploadOptions) + expect(max).to.equal(3) + }) + + it('falls back to the default when concurrency is 0', async function () { + const max = await observeMaxInFlight({ + ...batchUploadOptions, + concurrency: 0, + }) + expect(max).to.equal(3) + }) + + it('falls back to the default when concurrency is negative', async function () { + const max = await observeMaxInFlight({ + ...batchUploadOptions, + concurrency: -1, + }) + expect(max).to.equal(3) + }) + }) +})