From aa0054e4884a607fac78b954bef99f9e6671d307 Mon Sep 17 00:00:00 2001 From: Luciano Vernaschi Date: Wed, 29 Jan 2025 23:13:12 +0100 Subject: [PATCH 1/5] Extract files from regular request --- packages/ts/frontend/src/Connect.ts | 48 +++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/ts/frontend/src/Connect.ts b/packages/ts/frontend/src/Connect.ts index 5609e63bef..88d115749b 100644 --- a/packages/ts/frontend/src/Connect.ts +++ b/packages/ts/frontend/src/Connect.ts @@ -187,6 +187,30 @@ function isFlowLoaded(): boolean { return $wnd.Vaadin?.Flow?.clients?.TypeScript !== undefined; } +function extractFiles(obj: Record): [Record, Map] { + const fileMap = new Map(); + + function recursiveExtract(prop: unknown, path: string): unknown { + if (prop !== null && typeof prop === 'object' && !(prop instanceof File)) { + if (Array.isArray(prop)) { + return prop.map((item, index) => recursiveExtract(item, `${path}/${index}`)); + } + return Object.entries(prop).reduce>((acc, [key, value]) => { + const newPath = `${path}/${key}`; + if (value instanceof File) { + fileMap.set(newPath, value); + } else { + acc[key] = recursiveExtract(value, newPath); + } + return acc; + }, {}); + } + return prop; + } + + return [recursiveExtract(obj, '') as Record, fileMap]; +} + /** * A list of parameters supported by {@link ConnectClient.call | the call() method in ConnectClient}. */ @@ -308,9 +332,29 @@ export class ConnectClient { ...csrfHeaders, }; + const [paramsWithoutFiles, files] = extractFiles(params ?? {}); + let body; + + if (files.size > 0) { + // in this case params is not undefined, otherwise there would be no files + body = new FormData(); + body.append( + 'data', + JSON.stringify(paramsWithoutFiles, (_, value) => (value === undefined ? null : value)), + ); + + for (const [path, file] of files) { + body.append(path, file); + } + + headers['Content-Type'] = 'multipart/form-data'; + } else { + body = + params !== undefined ? JSON.stringify(params, (_, value) => (value === undefined ? null : value)) : undefined; + } + const request = new Request(`${this.prefix}/${endpoint}/${method}`, { - body: - params !== undefined ? JSON.stringify(params, (_, value) => (value === undefined ? null : value)) : undefined, + body, headers, method: 'POST', }); From ae6ae1fc9198f7bc6832764f0877e419b8cc440b Mon Sep 17 00:00:00 2001 From: Luciano Vernaschi Date: Thu, 30 Jan 2025 07:56:41 +0100 Subject: [PATCH 2/5] Fix header --- packages/ts/frontend/src/Connect.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ts/frontend/src/Connect.ts b/packages/ts/frontend/src/Connect.ts index 88d115749b..0981c2b504 100644 --- a/packages/ts/frontend/src/Connect.ts +++ b/packages/ts/frontend/src/Connect.ts @@ -24,6 +24,8 @@ $wnd.Vaadin.registrations.push({ is: 'endpoint', }); +export const bodyPartName = 'hilla_body_part'; + export type MaybePromise = Promise | T; /** @@ -328,7 +330,6 @@ export class ConnectClient { const csrfHeaders = globalThis.document ? getCsrfTokenHeadersForEndpointRequest(globalThis.document) : {}; const headers: Record = { Accept: 'application/json', - 'Content-Type': 'application/json', ...csrfHeaders, }; @@ -339,22 +340,21 @@ export class ConnectClient { // in this case params is not undefined, otherwise there would be no files body = new FormData(); body.append( - 'data', + bodyPartName, JSON.stringify(paramsWithoutFiles, (_, value) => (value === undefined ? null : value)), ); for (const [path, file] of files) { body.append(path, file); } - - headers['Content-Type'] = 'multipart/form-data'; } else { + headers['Content-Type'] = 'application/json'; body = params !== undefined ? JSON.stringify(params, (_, value) => (value === undefined ? null : value)) : undefined; } const request = new Request(`${this.prefix}/${endpoint}/${method}`, { - body, + body, // automatically sets Content-Type header headers, method: 'POST', }); From 0febc1ad1a47e135c323565ea7e709bd528302eb Mon Sep 17 00:00:00 2001 From: Luciano Vernaschi Date: Thu, 30 Jan 2025 07:56:49 +0100 Subject: [PATCH 3/5] Add tests --- packages/ts/frontend/test/Connect.test.ts | 37 +++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/ts/frontend/test/Connect.test.ts b/packages/ts/frontend/test/Connect.test.ts index 1bb8144238..14030431a2 100644 --- a/packages/ts/frontend/test/Connect.test.ts +++ b/packages/ts/frontend/test/Connect.test.ts @@ -17,6 +17,7 @@ import { type MiddlewareFunction, UnauthorizedResponseError, type FluxConnection, + bodyPartName, } from '../src/index.js'; import type { Vaadin, VaadinGlobal } from '../src/types.js'; import { subscribeStub } from './mocks/atmosphere.js'; @@ -477,6 +478,42 @@ describe('@vaadin/hilla-frontend', () => { expect(await request?.json()).to.deep.equal({ fooParam: 'foo' }); }); + it('should use multipart if a param is of File type', async () => { + const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); + await client.call('FooEndpoint', 'fooMethod', { fooParam: file }); + + const request = fetchMock.lastCall()?.request; + expect(request).to.exist; + expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); + const formData = await request!.formData(); + + const uploadedFile = formData.get('/fooParam') as File | null; + expect(uploadedFile).to.be.instanceOf(File); + expect(uploadedFile!.name).to.equal('foo.txt'); + expect(await uploadedFile!.text()).to.equal('foo'); + + const body = formData.get(bodyPartName); + expect(body).to.equal('{}'); + }); + + it('should use multipart if a param has a property if File type', async () => { + const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); + await client.call('FooEndpoint', 'fooMethod', { fooParam: { a: 'abc', b: file } }); + + const request = fetchMock.lastCall()?.request; + expect(request).to.exist; + expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); + const formData = await request!.formData(); + + const uploadedFile = formData.get('/fooParam/b') as File | null; + expect(uploadedFile).to.be.instanceOf(File); + expect(uploadedFile!.name).to.equal('foo.txt'); + expect(await uploadedFile!.text()).to.equal('foo'); + + const body = formData.get(bodyPartName); + expect(body).to.equal('{"fooParam":{"a":"abc"}}'); + }); + describe('middleware invocation', () => { it('should not invoke middleware before call', () => { const spyMiddleware = sinon.spy(async (context: MiddlewareContext, next: MiddlewareNext) => next(context)); From 8ae5f0f62d06c4c5cfd98fae00a6034a4eb70584 Mon Sep 17 00:00:00 2001 From: Luciano Vernaschi Date: Thu, 30 Jan 2025 08:08:13 +0100 Subject: [PATCH 4/5] Fix arrays --- packages/ts/frontend/src/Connect.ts | 12 +++++++++++- packages/ts/frontend/test/Connect.test.ts | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/ts/frontend/src/Connect.ts b/packages/ts/frontend/src/Connect.ts index 0981c2b504..03a63fadfe 100644 --- a/packages/ts/frontend/src/Connect.ts +++ b/packages/ts/frontend/src/Connect.ts @@ -189,11 +189,21 @@ function isFlowLoaded(): boolean { return $wnd.Vaadin?.Flow?.clients?.TypeScript !== undefined; } +/** + * Extracts file objects from the object that is used to build the request body. + * + * @param obj - The object to extract files from. + * @returns A tuple with the object without files and a map of files. + */ function extractFiles(obj: Record): [Record, Map] { const fileMap = new Map(); function recursiveExtract(prop: unknown, path: string): unknown { - if (prop !== null && typeof prop === 'object' && !(prop instanceof File)) { + if (prop !== null && typeof prop === 'object') { + if (prop instanceof File) { + fileMap.set(path, prop); + return null; + } if (Array.isArray(prop)) { return prop.map((item, index) => recursiveExtract(item, `${path}/${index}`)); } diff --git a/packages/ts/frontend/test/Connect.test.ts b/packages/ts/frontend/test/Connect.test.ts index 14030431a2..7d735ebae6 100644 --- a/packages/ts/frontend/test/Connect.test.ts +++ b/packages/ts/frontend/test/Connect.test.ts @@ -514,6 +514,24 @@ describe('@vaadin/hilla-frontend', () => { expect(body).to.equal('{"fooParam":{"a":"abc"}}'); }); + it('should use multipart if a File is found in array', async () => { + const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); + await client.call('FooEndpoint', 'fooMethod', { fooParam: ['a', file, 'c'], other: 'abc' }); + + const request = fetchMock.lastCall()?.request; + expect(request).to.exist; + expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); + const formData = await request!.formData(); + + const uploadedFile = formData.get('/fooParam/1') as File | null; + expect(uploadedFile).to.be.instanceOf(File); + expect(uploadedFile!.name).to.equal('foo.txt'); + expect(await uploadedFile!.text()).to.equal('foo'); + + const body = formData.get(bodyPartName); + expect(body).to.equal('{"fooParam":["a",null,"c"],"other":"abc"}'); + }); + describe('middleware invocation', () => { it('should not invoke middleware before call', () => { const spyMiddleware = sinon.spy(async (context: MiddlewareContext, next: MiddlewareNext) => next(context)); From deb377766df8246bb740d982d10f3f52b5aec18f Mon Sep 17 00:00:00 2001 From: Luciano Vernaschi Date: Fri, 31 Jan 2025 16:00:10 +0100 Subject: [PATCH 5/5] Add missing callHistory --- packages/ts/frontend/test/Connect.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ts/frontend/test/Connect.test.ts b/packages/ts/frontend/test/Connect.test.ts index 4d66f14c53..4eb946d2e4 100644 --- a/packages/ts/frontend/test/Connect.test.ts +++ b/packages/ts/frontend/test/Connect.test.ts @@ -497,7 +497,7 @@ describe('@vaadin/hilla-frontend', () => { const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); await client.call('FooEndpoint', 'fooMethod', { fooParam: file }); - const request = fetchMock.lastCall()?.request; + const request = fetchMock.callHistory.lastCall()?.request; expect(request).to.exist; expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); const formData = await request!.formData(); @@ -515,7 +515,7 @@ describe('@vaadin/hilla-frontend', () => { const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); await client.call('FooEndpoint', 'fooMethod', { fooParam: { a: 'abc', b: file } }); - const request = fetchMock.lastCall()?.request; + const request = fetchMock.callHistory.lastCall()?.request; expect(request).to.exist; expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); const formData = await request!.formData(); @@ -533,7 +533,7 @@ describe('@vaadin/hilla-frontend', () => { const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }); await client.call('FooEndpoint', 'fooMethod', { fooParam: ['a', file, 'c'], other: 'abc' }); - const request = fetchMock.lastCall()?.request; + const request = fetchMock.callHistory.lastCall()?.request; expect(request).to.exist; expect(request?.headers.get('content-type')).to.match(/^multipart\/form-data;/u); const formData = await request!.formData();