From 94e21adb4ae3efe577274b827c0191c2b98993b9 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 27 Oct 2020 17:25:41 +0100 Subject: [PATCH] feat(rest-api-link): add support for text/plain and multipart/form-data (#651) * feat(rest-api-link): set correct content-type for endpoints * feat(rest-api-link): add support for text/plain and multipart/form-data * feat(rest-api-link): adjust request body according to Content-Type * fix(rest-api-link): throw error if data can't be converted to form-data * fix(rest-api-link): prevent boundary error for multipart/form-data * fix(rest-api-link): add additional multipart/form-data endpoints * fix(rest-api-link): adjust types for convertToFormData (object>FromData) --- .../RestAPILink/queryToRequestOptions.ts | 27 ++- .../multipartFormDataMatchers.test.ts | 81 +++++++ .../multipartFormDataMatchers.ts | 34 +++ .../requestContentType.test.ts | 101 +++++++++ .../requestContentType.ts | 97 +++++++++ .../textPlainMatchers.test.ts | 203 ++++++++++++++++++ .../textPlainMatchers.ts | 112 ++++++++++ 7 files changed, 644 insertions(+), 11 deletions(-) create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.test.ts create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.ts create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.test.ts create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.ts create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.test.ts create mode 100644 services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.ts diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions.ts b/services/data/src/links/RestAPILink/queryToRequestOptions.ts index be8b6982..58ddfb90 100644 --- a/services/data/src/links/RestAPILink/queryToRequestOptions.ts +++ b/services/data/src/links/RestAPILink/queryToRequestOptions.ts @@ -1,4 +1,9 @@ import { ResolvedResourceQuery, FetchType } from '../../engine' +import { + requestContentType, + requestBodyForContentType, + requestHeadersForContentType, +} from './queryToRequestOptions/requestContentType' const getMethod = (type: FetchType): string => { switch (type) { @@ -17,15 +22,15 @@ const getMethod = (type: FetchType): string => { export const queryToRequestOptions = ( type: FetchType, - { data }: ResolvedResourceQuery, + query: ResolvedResourceQuery, signal?: AbortSignal -): RequestInit => ({ - method: getMethod(type), - body: data ? JSON.stringify(data) : undefined, - headers: data - ? { - 'Content-Type': 'application/json', - } - : undefined, - signal, -}) +): RequestInit => { + const contentType = requestContentType(type, query) + + return { + method: getMethod(type), + body: requestBodyForContentType(contentType, query), + headers: requestHeadersForContentType(contentType), + signal, + } +} diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.test.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.test.ts new file mode 100644 index 00000000..61ec2d24 --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.test.ts @@ -0,0 +1,81 @@ +import { + isFileResourceUpload, + isMessageConversationAttachment, + isStaticContentUpload, + isAppInstall, +} from './multipartFormDataMatchers' + +describe('isFileResourceUpload', () => { + it('returns true for a POST to "fileResources"', () => { + expect( + isFileResourceUpload('create', { + resource: 'fileResources', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isFileResourceUpload('create', { + resource: 'notFileResources', + }) + ).toEqual(false) + }) +}) + +describe('isMessageConversationAttachment', () => { + it('returns true for a POST to "messageConversations/attachments"', () => { + expect( + isMessageConversationAttachment('create', { + resource: 'messageConversations/attachments', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isMessageConversationAttachment('create', { + resource: 'messageConversations/notAttachments', + }) + ).toEqual(false) + }) +}) + +describe('isStaticContentUpload', () => { + it('returns true for a POST to "staticContent/logo_banner"', () => { + expect( + isStaticContentUpload('create', { + resource: 'staticContent/logo_banner', + }) + ).toEqual(true) + }) + it('returns true for a POST to "staticContent/logo_front"', () => { + expect( + isStaticContentUpload('create', { + resource: 'staticContent/logo_front', + }) + ).toEqual(true) + }) + it('returns false for a request to a different resource', () => { + expect( + isStaticContentUpload('create', { + resource: 'staticContent/no_logo', + }) + ).toEqual(false) + }) +}) + +describe('isAppInstall', () => { + it('returns true for a POST to "fileResources"', () => { + expect( + isAppInstall('create', { + resource: 'apps', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isAppInstall('create', { + resource: 'notApps', + }) + ).toEqual(false) + }) +}) diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.ts new file mode 100644 index 00000000..af767fbf --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/multipartFormDataMatchers.ts @@ -0,0 +1,34 @@ +import { ResolvedResourceQuery, FetchType } from '../../../engine' + +/* + * Requests that expect a "multipart/form-data" Content-Type have been collected by scanning + * the developer documentation: + * https://docs.dhis2.org/master/en/developer/html/dhis2_developer_manual_full.html + */ + +// POST to 'fileResources' (upload a file resource) +export const isFileResourceUpload = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => type === 'create' && resource === 'fileResources' + +// POST to 'messageConversations/attachments' (upload a message conversation attachment) +export const isMessageConversationAttachment = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => type === 'create' && resource === 'messageConversations/attachments' + +// POST to `staticContent/${key}` (upload staticContent: logo_banner | logo_front) +export const isStaticContentUpload = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => { + const pattern = /^staticContent\/(?:logo_banner|logo_front)$/ + return type === 'create' && pattern.test(resource) +} + +// POST to 'apps' (install an app) +export const isAppInstall = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => type === 'create' && resource === 'apps' diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.test.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.test.ts new file mode 100644 index 00000000..46e11303 --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.test.ts @@ -0,0 +1,101 @@ +import { + requestContentType, + requestHeadersForContentType, + requestBodyForContentType, + FORM_DATA_ERROR_MSG, +} from './requestContentType' + +describe('requestContentType', () => { + it('returns "application/json" for a normal resource', () => { + expect( + requestContentType('create', { resource: 'test', data: 'test' }) + ).toEqual('application/json') + }) + it('returns "multipart/form-data" for a specific resource that expects it', () => { + expect( + requestContentType('create', { + resource: 'fileResources', + data: 'test', + }) + ).toEqual('multipart/form-data') + }) + it('returns "text/plain" for a specific resource that expects it', () => { + expect( + requestContentType('create', { + resource: 'messageConversations/feedback', + data: 'test', + }) + ).toEqual('text/plain') + }) +}) + +describe('requestHeadersForContentType', () => { + it('returns undefined if contentType is null', () => { + expect(requestHeadersForContentType(null)).toEqual(undefined) + }) + it('returns undefined if contentType is "multipart/form-data"', () => { + expect(requestHeadersForContentType('multipart/form-data')).toEqual( + undefined + ) + }) + it('returns a headers object with the contentType for "application/json"', () => { + expect(requestHeadersForContentType('application/json')).toEqual({ + 'Content-Type': 'application/json', + }) + }) + it('returns a headers object with the contentType for "text/plain"', () => { + expect(requestHeadersForContentType('text/plain')).toEqual({ + 'Content-Type': 'text/plain', + }) + }) +}) + +describe('requestBodyForContentType', () => { + it('returns undefined if data is undefined', () => { + expect( + requestBodyForContentType('application/json', { resource: 'test' }) + ).toEqual(undefined) + }) + it('JSON stringifies the data if contentType is "application/json"', () => { + const dataIn = { a: 'AAAA', b: 1, c: true } + const dataOut = JSON.stringify(dataIn) + + expect( + requestBodyForContentType('application/json', { + resource: 'test', + data: dataIn, + }) + ).toEqual(dataOut) + }) + it('converts to FormData if contentType is "multipart/form-data"', () => { + const file = new File(['foo'], 'foo.txt', { type: 'text/plain' }) + const data = { a: 'AAA', file } + + const result = requestBodyForContentType('multipart/form-data', { + resource: 'test', + data, + }) + + expect(result instanceof FormData).toEqual(true) + expect(result.get('a')).toEqual('AAA') + expect(result.get('file')).toEqual(file) + }) + it('throws an error if contentType is "multipart/form-data" and data does have own string-keyd properties', () => { + expect(() => { + requestBodyForContentType('multipart/form-data', { + resource: 'test', + data: new File(['foo'], 'foo.txt', { type: 'text/plain' }), + }) + }).toThrow(new Error(FORM_DATA_ERROR_MSG)) + }) + it('returns the data as received if contentType is "text/plain"', () => { + const data = 'Something' + + expect( + requestBodyForContentType('text/plain', { + resource: 'messageConversations/feedback', + data, + }) + ).toEqual(data) + }) +}) diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.ts new file mode 100644 index 00000000..ff9c23c7 --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/requestContentType.ts @@ -0,0 +1,97 @@ +import { ResolvedResourceQuery, FetchType } from '../../../engine' +import * as textPlainMatchers from './textPlainMatchers' +import * as multipartFormDataMatchers from './multipartFormDataMatchers' + +type RequestContentType = + | 'application/json' + | 'text/plain' + | 'multipart/form-data' + | null + +const resourceExpectsTextPlain = ( + type: FetchType, + query: ResolvedResourceQuery +) => + Object.values(textPlainMatchers).some(textPlainMatcher => + textPlainMatcher(type, query) + ) + +const resourceExpectsMultipartFormData = ( + type: FetchType, + query: ResolvedResourceQuery +) => + Object.values(multipartFormDataMatchers).some(multipartFormDataMatcher => + multipartFormDataMatcher(type, query) + ) + +export const FORM_DATA_ERROR_MSG = + 'Could not convert data to FormData: object does not have own enumerable string-keyed properties' + +const convertToFormData = (data: Record): FormData => { + const dataEntries = Object.entries(data) + + if (dataEntries.length === 0) { + throw new Error(FORM_DATA_ERROR_MSG) + } + + return dataEntries.reduce((formData, [key, value]) => { + formData.append(key, value) + return formData + }, new FormData()) +} + +export const requestContentType = ( + type: FetchType, + query: ResolvedResourceQuery +) => { + if (!query.data) { + return null + } + + if (resourceExpectsTextPlain(type, query)) { + return 'text/plain' + } + + if (resourceExpectsMultipartFormData(type, query)) { + return 'multipart/form-data' + } + + return 'application/json' +} + +export const requestHeadersForContentType = ( + contentType: RequestContentType +) => { + /* + * Explicitely setting Content-Type to 'multipart/form-data' produces + * a "multipart boundary not found" error. By not setting a Content-Type + * the browser will correctly set it for us and also apply multipart + * boundaries if the request body is an instance of FormData + * See https://stackoverflow.com/a/39281156/1143502 + */ + if (!contentType || contentType === 'multipart/form-data') { + return undefined + } + + return { 'Content-Type': contentType } +} + +export const requestBodyForContentType = ( + contentType: RequestContentType, + { data }: ResolvedResourceQuery +) => { + if (typeof data === 'undefined') { + return undefined + } + + if (contentType === 'application/json') { + return JSON.stringify(data) + } + + if (contentType === 'multipart/form-data') { + return convertToFormData(data) + } + + // 'text/plain' + return data +} diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.test.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.test.ts new file mode 100644 index 00000000..256a4833 --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.test.ts @@ -0,0 +1,203 @@ +import { + isReplyToMessageConversation, + isCreateFeedbackMessage, + isCreateOrUpdateInterpretation, + isCommentOnInterpretation, + isInterpretationCommentUpdate, + isAddOrUpdateSystemOrUserSetting, + addOrUpdateConfigurationProperty, + isMetadataPackageInstallation, +} from './textPlainMatchers' + +describe('isReplyToMessageConversation', () => { + it('retuns true for POST to `messageConversations/${id}`', () => { + expect( + isReplyToMessageConversation('create', { + resource: 'messageConversations/oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isReplyToMessageConversation('create', { + resource: 'test/oXD88WWSQpR', + }) + ).toEqual(false) + }) +}) + +describe('isCreateFeedbackMessage', () => { + it('returns true for a POST to "messageConversations/feedback"', () => { + expect( + isCreateFeedbackMessage('create', { + resource: 'messageConversations/feedback', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isCreateFeedbackMessage('create', { + resource: 'messageConversations/somethingelse', + }) + ).toEqual(false) + }) +}) + +describe('isCreateOrUpdateInterpretation', () => { + it('returns true for a POST to "interpretations/chart/${id}"', () => { + expect( + isCreateOrUpdateInterpretation('create', { + resource: 'interpretations/chart/oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('returns true for a PUT to "interpretations/chart/${id}"', () => { + expect( + isCreateOrUpdateInterpretation('replace', { + resource: 'interpretations/chart/oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('returns true for PUT with populated query.id', () => { + expect( + isCreateOrUpdateInterpretation('replace', { + resource: 'interpretations/chart', + id: 'oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('retuns false for PATCH requests with a valid query', () => { + expect( + isCreateOrUpdateInterpretation('update', { + resource: 'interpretations/chart/oXD88WWSQpR', + }) + ).toEqual(false) + }) + it('returns false for a request to a different resource', () => { + expect( + isCreateOrUpdateInterpretation('create', { + resource: 'interpretations/dummy/oXD88WWSQpR', + }) + ).toEqual(false) + }) +}) + +describe('isCommentOnInterpretation', () => { + it('retuns true for POST to `interpretations/${id}/comments`', () => { + expect( + isCommentOnInterpretation('create', { + resource: 'interpretations/oXD88WWSQpR/comments', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isCommentOnInterpretation('create', { + resource: 'test/oXD88WWSQpR/comments', + }) + ).toEqual(false) + }) +}) + +describe('isInterpretationCommentUpdate', () => { + it('returns true for a PUT to `interpretations/${interpretationId}/comments/${commentId}`', () => { + expect( + isInterpretationCommentUpdate('replace', { + resource: 'interpretations/oXD88WWSQpR/comments/oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('returns true for PUT with populated query.id', () => { + expect( + isInterpretationCommentUpdate('replace', { + resource: 'interpretations', + id: 'oXD88WWSQpR/comments/oXD88WWSQpR', + }) + ).toEqual(true) + expect( + isInterpretationCommentUpdate('replace', { + resource: 'interpretations/oXD88WWSQpR/comments', + id: 'oXD88WWSQpR', + }) + ).toEqual(true) + }) + it('retuns false for PATCH requests with a valid query', () => { + expect( + isInterpretationCommentUpdate('update', { + resource: 'interpretations/oXD88WWSQpR/comments/oXD88WWSQpR', + }) + ).toEqual(false) + }) + it('returns false for a request to a different resource', () => { + expect( + isInterpretationCommentUpdate('create', { + resource: 'interpretations/oXD88WWSQpR/dummy/oXD88WWSQpR', + }) + ).toEqual(false) + }) +}) + +describe('isAddOrUpdateSystemOrUserSetting', () => { + it('retuns true for POST to `systemSettings/${settingKey}`', () => { + expect( + isAddOrUpdateSystemOrUserSetting('create', { + resource: 'systemSettings/keyWhatever', + }) + ).toEqual(true) + }) + it('retuns true for POST to `userSettings/${settingKey}`', () => { + expect( + isAddOrUpdateSystemOrUserSetting('create', { + resource: 'userSettings/keyWhatever', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isAddOrUpdateSystemOrUserSetting('create', { + resource: 'test/keyWhatever', + }) + ).toEqual(false) + }) +}) + +describe('addOrUpdateConfigurationProperty', () => { + it('retuns true for POST to `configuration/${property}`', () => { + expect( + addOrUpdateConfigurationProperty('create', { + resource: 'configuration/whatever', + }) + ).toEqual(true) + }) + it('retuns false for POST to `configuration/corsWhitelist`, which needs "application/json"', () => { + expect( + addOrUpdateConfigurationProperty('create', { + resource: 'configuration/corsWhitelist', + }) + ).toEqual(false) + }) + it('retuns false for a POST to a different resource', () => { + expect( + addOrUpdateConfigurationProperty('create', { + resource: 'test/whatever', + }) + ).toEqual(false) + }) +}) + +describe('isMetadataPackageInstallation', () => { + it('returns true for a POST to "synchronization/metadataPull"', () => { + expect( + isMetadataPackageInstallation('create', { + resource: 'synchronization/metadataPull', + }) + ).toEqual(true) + }) + it('retuns false for a POST to a different resource', () => { + expect( + isMetadataPackageInstallation('create', { + resource: 'synchronization/somethingelse', + }) + ).toEqual(false) + }) +}) diff --git a/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.ts b/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.ts new file mode 100644 index 00000000..56e92018 --- /dev/null +++ b/services/data/src/links/RestAPILink/queryToRequestOptions/textPlainMatchers.ts @@ -0,0 +1,112 @@ +import { ResolvedResourceQuery, FetchType } from '../../../engine' + +/* + * Requests that expect a "text/plain" Content-Type have been collected by scanning + * the developer documentation: + * https://docs.dhis2.org/master/en/developer/html/dhis2_developer_manual_full.html + * + * Note that currently it is not allowed to include an id property on a "create" + * mutation object. This means that currently the `id` will always be included in + * the resource property (string). If we decide to allow the `id` property for + * "create" mutation-objects, we will have to include additional checks below. + */ + +// POST to `messageConversations/${id}` (reply to a messagConversation) +export const isReplyToMessageConversation = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => { + const pattern = /^messageConversations\/[a-zA-Z0-9]{11}$/ + return type === 'create' && pattern.test(resource) +} + +// POST to 'messageConversations/feedback' (create a feedback message) +export const isCreateFeedbackMessage = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => type === 'create' && resource === 'messageConversations/feedback' + +// POST or PUT to `interpretations/${objectType}/${id}` (add or update an interpretation) +export const isCreateOrUpdateInterpretation = ( + type: FetchType, + { resource, id }: ResolvedResourceQuery +) => { + if (type !== 'create' && type !== 'replace') { + return false + } + + let resourcePattern + if (type === 'replace' && id) { + resourcePattern = /^interpretations\/(?:reportTable|chart|visualization|map|eventReport|eventChart|dataSetReport)$/ + const idPattern = /^[a-zA-Z0-9]{11}$/ + + return resourcePattern.test(resource) && idPattern.test(id) + } + + resourcePattern = /^interpretations\/(?:reportTable|chart|visualization|map|eventReport|eventChart|dataSetReport)\/[a-zA-Z0-9]{11}$/ + + return resourcePattern.test(resource) +} + +// POST to `interpretations/${id}/comments` (comment on an interpretation) +export const isCommentOnInterpretation = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => { + const pattern = /^interpretations\/[a-zA-Z0-9]{11}\/comments$/ + return type === 'create' && pattern.test(resource) +} + +// PUT to `interpretations/${interpretationId}/comments/${commentId}` +// (update an interpretation comment) +export const isInterpretationCommentUpdate = ( + type: FetchType, + { resource, id }: ResolvedResourceQuery +) => { + if (type !== 'replace') { + return false + } + + if (id) { + const idPatternLong = /^[a-zA-Z0-9]{11}\/comments\/[a-zA-Z0-9]{11}$/ + const idPatternShort = /^[a-zA-Z0-9]{11}$/ + const resourcePattern = /^interpretations\/[a-zA-Z0-9]{11}\/comments$/ + + return ( + (resource === 'interpretations' && idPatternLong.test(id)) || + (resourcePattern.test(resource) && idPatternShort.test(id)) + ) + } + + const pattern = /^interpretations\/[a-zA-Z0-9]{11}\/comments\/[a-zA-Z0-9]{11}$/ + return pattern.test(resource) +} + +// POST to `systemSettings/${settingKey}` or `userSettings/${settingKey}` +// (add or update a single system or user setting) +export const isAddOrUpdateSystemOrUserSetting = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => { + // At least 4 chars because the all start with 'key' (i.e. keyStyle) + const pattern = /^(?:systemSettings|userSettings)\/[a-zA-Z]{4,}$/ + return type === 'create' && pattern.test(resource) +} + +// POST to `configuration/${configurationProperty}` +// (add or update a single configuration property) +export const addOrUpdateConfigurationProperty = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => { + // NOTE: The corsWhitelist property does expect "application/json" + const pattern = /^(configuration)\/([a-zA-Z]{1,50})$/ + const match = resource.match(pattern) + return type === 'create' && !!match && match[2] !== 'corsWhitelist' +} + +// POST to 'synchronization/metadataPull' (install a metadata package) +export const isMetadataPackageInstallation = ( + type: FetchType, + { resource }: ResolvedResourceQuery +) => type === 'create' && resource === 'synchronization/metadataPull'