From 52b62acbeb424494900f0e0f49922bd72647950c Mon Sep 17 00:00:00 2001 From: Petr Stefan Date: Sat, 2 Dec 2017 19:58:49 +0100 Subject: [PATCH 1/3] Use JSON instead of urlencoded form data --- package.json | 1 - src/redux/helpers/api/tools.js | 30 +++------------------- test/redux/helpers/api/tools-test.js | 37 +++------------------------- yarn.lock | 8 +----- 4 files changed, 8 insertions(+), 68 deletions(-) diff --git a/package.json b/package.json index d118e061c..ded917d93 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,6 @@ "exenv": "^1.2.1", "express": "^4.13.4", "file-saver": "^1.3.3", - "flat": "^2.0.1", "flow-bin": "^0.46.0", "global": "^4.3.1", "immutable": "^3.8.1", diff --git a/src/redux/helpers/api/tools.js b/src/redux/helpers/api/tools.js index ab71dc6e8..190c78af8 100644 --- a/src/redux/helpers/api/tools.js +++ b/src/redux/helpers/api/tools.js @@ -1,6 +1,5 @@ import statusCode from 'statuscode'; import { addNotification } from '../../modules/notifications'; -import flatten from 'flat'; import { logout } from '../../modules/auth'; import { isTokenValid, decode } from '../../helpers/token'; @@ -14,28 +13,6 @@ export const API_BASE = process.env.API_BASE || 'http://localhost:4000/v1'; const maybeShash = endpoint => (endpoint.indexOf('/') === 0 ? '' : '/'); const getUrl = endpoint => API_BASE + maybeShash(endpoint) + endpoint; -export const flattenBody = body => { - const flattened = flatten(body, { delimiter: ':' }); - body = {}; - - Object.keys(flattened).map(key => { - // 'a:b:c:d' => 'a[b][c][d]' - const bracketedKey = key.replace(/:([^:$]+)/g, '[$1]'); - body[bracketedKey] = flattened[key]; - }); - - return body; -}; - -const createFormData = body => { - const data = new FormData(); - const flattened = flattenBody(body); - for (let key in flattened) { - data.append(key, flattened[key]); - } - return data; -}; - const maybeQuestionMark = (endpoint, query) => Object.keys(query).length === 0 ? '' @@ -51,18 +28,19 @@ export const createRequest = (endpoint, query = {}, method, headers, body) => fetch(getUrl(assembleEndpoint(endpoint, query)), { method, headers, - body: body ? createFormData(body) : undefined + body: body ? JSON.stringify(body) : undefined }); export const getHeaders = (headers, accessToken) => { + const jsonHeaders = { 'Content-Type': 'application/json', ...headers }; if (accessToken) { return { Authorization: `Bearer ${accessToken}`, - ...headers + ...jsonHeaders }; } - return headers; + return jsonHeaders; }; /** diff --git a/test/redux/helpers/api/tools-test.js b/test/redux/helpers/api/tools-test.js index 5a5a19b7d..3701429ce 100644 --- a/test/redux/helpers/api/tools-test.js +++ b/test/redux/helpers/api/tools-test.js @@ -2,8 +2,7 @@ import { expect } from 'chai'; import { getHeaders, - assembleEndpoint, - flattenBody + assembleEndpoint } from '../../../../src/redux/helpers/api/tools'; describe('API middleware and helper functions', () => { @@ -11,7 +10,8 @@ describe('API middleware and helper functions', () => { it('must add access token to the headers', () => { expect(getHeaders({ a: 'b' }, 'abcd')).to.eql({ a: 'b', - Authorization: 'Bearer abcd' + Authorization: 'Bearer abcd', + 'Content-Type': 'application/json' }); }); @@ -33,36 +33,5 @@ describe('API middleware and helper functions', () => { 'http://www.blabla.com/abcd' ); }); - - it('must flatten the body object to urlencoded format', () => { - expect(flattenBody({})).to.eql({}); - expect(flattenBody([])).to.eql({}); - expect(flattenBody({ a: 'b' })).to.eql({ a: 'b' }); - expect(flattenBody({ a: { b: 'c' } })).to.eql({ 'a[b]': 'c' }); - expect(flattenBody({ a: { b: { c: 'd' } } })).to.eql({ 'a[b][c]': 'd' }); - expect(flattenBody({ a: { b: ['c', 'd'] } })).to.eql({ - 'a[b][0]': 'c', - 'a[b][1]': 'd' - }); - expect( - flattenBody({ - a: [ - { - b: [{ c: 'A' }, { d: 'B' }] - }, - [{ f: 'C', g: 'D' }, { h: 'E' }] - ] - }) - ).to.eql({ - 'a[0][b][0][c]': 'A', - 'a[0][b][1][d]': 'B', - 'a[1][0][f]': 'C', - 'a[1][0][g]': 'D', - 'a[1][1][h]': 'E' - }); - - // not a POJO - expect(() => flattenBody({ a: new MouseEvent('click') })).to.throw(); - }); }); }); diff --git a/yarn.lock b/yarn.lock index d1d3fec97..6dbff1cf1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2991,12 +2991,6 @@ flat-cache@^1.2.1: graceful-fs "^4.1.2" write "^0.2.1" -flat@^2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/flat/-/flat-2.0.1.tgz#70e29188a74be0c3c89409eed1fa9577907ae32f" - dependencies: - is-buffer "~1.1.2" - flatten@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/flatten/-/flatten-1.0.2.tgz#dae46a9d78fbe25292258cc1e780a41d95c03782" @@ -3861,7 +3855,7 @@ is-binary-path@^1.0.0: dependencies: binary-extensions "^1.0.0" -is-buffer@^1.1.5, is-buffer@~1.1.2: +is-buffer@^1.1.5: version "1.1.5" resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.5.tgz#1f3b26ef613b214b88cbca23cc6c01d87961eecc" From fcd1055a6d5f22230b894b357d71e5e3efe9c7f6 Mon Sep 17 00:00:00 2001 From: Petr Stefan Date: Sat, 2 Dec 2017 23:15:06 +0100 Subject: [PATCH 2/3] Special handling for uploading files --- package.json | 1 + src/redux/helpers/api/tools.js | 51 +++++++++++++++++--- src/redux/middleware/apiMiddleware.js | 8 +-- src/redux/modules/additionalExerciseFiles.js | 3 +- src/redux/modules/pipelineFiles.js | 3 +- src/redux/modules/supplementaryFiles.js | 3 +- src/redux/modules/upload.js | 3 +- yarn.lock | 10 ++++ 8 files changed, 67 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index ded917d93..aa3129e5f 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "exenv": "^1.2.1", "express": "^4.13.4", "file-saver": "^1.3.3", + "flat": "^4.0.0", "flow-bin": "^0.46.0", "global": "^4.3.1", "immutable": "^3.8.1", diff --git a/src/redux/helpers/api/tools.js b/src/redux/helpers/api/tools.js index 190c78af8..8c4b63d65 100644 --- a/src/redux/helpers/api/tools.js +++ b/src/redux/helpers/api/tools.js @@ -1,5 +1,6 @@ import statusCode from 'statuscode'; import { addNotification } from '../../modules/notifications'; +import { flatten } from 'flat'; import { logout } from '../../modules/auth'; import { isTokenValid, decode } from '../../helpers/token'; @@ -24,23 +25,56 @@ const generateQuery = query => export const assembleEndpoint = (endpoint, query = {}) => endpoint + maybeQuestionMark(endpoint, query) + generateQuery(query); -export const createRequest = (endpoint, query = {}, method, headers, body) => +export const flattenBody = body => { + const flattened = flatten(body, { delimiter: ':' }); + body = {}; + + Object.keys(flattened).map(key => { + // 'a:b:c:d' => 'a[b][c][d]' + const bracketedKey = key.replace(/:([^:$]+)/g, '[$1]'); + body[bracketedKey] = flattened[key]; + }); + + return body; +}; + +const createFormData = body => { + const data = new FormData(); + const flattened = flattenBody(body); + for (let key in flattened) { + data.append(key, flattened[key]); + } + return data; +}; + +export const createRequest = ( + endpoint, + query = {}, + method, + headers, + body, + uploadFiles +) => fetch(getUrl(assembleEndpoint(endpoint, query)), { method, headers, - body: body ? JSON.stringify(body) : undefined + body: body + ? uploadFiles ? createFormData(body) : JSON.stringify(body) + : undefined }); -export const getHeaders = (headers, accessToken) => { - const jsonHeaders = { 'Content-Type': 'application/json', ...headers }; +export const getHeaders = (headers, accessToken, skipContentType) => { + const usedHeaders = skipContentType + ? headers + : { 'Content-Type': 'application/json', ...headers }; if (accessToken) { return { Authorization: `Bearer ${accessToken}`, - ...jsonHeaders + ...usedHeaders }; } - return jsonHeaders; + return usedHeaders; }; /** @@ -57,11 +91,12 @@ export const createApiCallPromise = ( accessToken = '', body = undefined, wasSuccessful = () => true, - doNotProcess = false + doNotProcess = false, + uploadFiles = false }, dispatch = undefined ) => { - let call = createRequest(endpoint, query, method, headers, body) + let call = createRequest(endpoint, query, method, headers, body, uploadFiles) .catch(err => detectUnreachableServer(err, dispatch)) .then(res => { if ( diff --git a/src/redux/middleware/apiMiddleware.js b/src/redux/middleware/apiMiddleware.js index 7ba01ac64..2bb8c3a9d 100644 --- a/src/redux/middleware/apiMiddleware.js +++ b/src/redux/middleware/apiMiddleware.js @@ -18,7 +18,8 @@ export const apiCall = ( body = undefined, meta = undefined, wasSuccessful = isTwoHundredCode, - doNotProcess = false + doNotProcess = false, + uploadFiles = false }, dispatch = undefined ) => ({ @@ -29,10 +30,11 @@ export const apiCall = ( endpoint, query, method, - headers: getHeaders(headers, accessToken), + headers: getHeaders(headers, accessToken, uploadFiles), body, wasSuccessful, - doNotProcess + doNotProcess, + uploadFiles }, dispatch ), diff --git a/src/redux/modules/additionalExerciseFiles.js b/src/redux/modules/additionalExerciseFiles.js index 05353554c..ddbfcfe52 100644 --- a/src/redux/modules/additionalExerciseFiles.js +++ b/src/redux/modules/additionalExerciseFiles.js @@ -41,7 +41,8 @@ export const addAdditionalExerciseFiles = (exerciseId, files) => tmpId: Math.random().toString(), file: uploaded.file })) - } + }, + uploadFiles: true }); export const removeAdditionalExerciseFile = (exerciseId, fileId) => diff --git a/src/redux/modules/pipelineFiles.js b/src/redux/modules/pipelineFiles.js index 6da682957..a4775437d 100644 --- a/src/redux/modules/pipelineFiles.js +++ b/src/redux/modules/pipelineFiles.js @@ -39,7 +39,8 @@ export const addPipelineFiles = (pipelineId, files) => tmpId: Math.random().toString(), file: uploaded.file })) - } + }, + uploadFiles: true }); /** diff --git a/src/redux/modules/supplementaryFiles.js b/src/redux/modules/supplementaryFiles.js index c6b20f6af..361a4566d 100644 --- a/src/redux/modules/supplementaryFiles.js +++ b/src/redux/modules/supplementaryFiles.js @@ -41,7 +41,8 @@ export const addSupplementaryFiles = (exerciseId, files) => tmpId: Math.random().toString(), file: uploaded.file })) - } + }, + uploadFiles: true }); export const removeSupplementaryFile = (exerciseId, fileId) => diff --git a/src/redux/modules/upload.js b/src/redux/modules/upload.js index f84603e7e..df6858788 100644 --- a/src/redux/modules/upload.js +++ b/src/redux/modules/upload.js @@ -21,7 +21,8 @@ export const uploadFile = (id, file, endpoint = '/uploaded-files') => method: 'POST', endpoint, body: { [file.name]: file }, - meta: { id, fileName: file.name } + meta: { id, fileName: file.name }, + uploadFiles: true }); const wrapWithName = (id, file) => ({ [file.name]: file }); diff --git a/yarn.lock b/yarn.lock index 6dbff1cf1..8a39bc5fc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2991,6 +2991,12 @@ flat-cache@^1.2.1: graceful-fs "^4.1.2" write "^0.2.1" +flat@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/flat/-/flat-4.0.0.tgz#3abc7f3b588e64ce77dc42fd59aa35806622fea8" + dependencies: + is-buffer "~1.1.5" + flatten@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/flatten/-/flatten-1.0.2.tgz#dae46a9d78fbe25292258cc1e780a41d95c03782" @@ -3859,6 +3865,10 @@ is-buffer@^1.1.5: version "1.1.5" resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.5.tgz#1f3b26ef613b214b88cbca23cc6c01d87961eecc" +is-buffer@~1.1.5: + version "1.1.6" + resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.6.tgz#efaa2ea9daa0d7ab2ea13a97b2b8ad51fefbe8be" + is-builtin-module@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/is-builtin-module/-/is-builtin-module-1.0.0.tgz#540572d34f7ac3119f8f76c30cbc1b1e037affbe" From 387179a5c134ce2e716958aa569cf0fce63a8d95 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 4 Dec 2017 18:19:05 +0100 Subject: [PATCH 3/3] Fixing an issue of empty JSON bodies. --- src/redux/helpers/api/tools.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/redux/helpers/api/tools.js b/src/redux/helpers/api/tools.js index 8c4b63d65..6d444333d 100644 --- a/src/redux/helpers/api/tools.js +++ b/src/redux/helpers/api/tools.js @@ -47,6 +47,19 @@ const createFormData = body => { return data; }; +const encodeBody = (body, method, encodeAsMultipart) => { + if (method.toUpperCase() !== 'POST') { + return undefined; + } + + if (encodeAsMultipart) { + return body ? createFormData(body) : undefined; + } else { + // otherwise we encode in JSON + return JSON.stringify(body || []); + } +}; + export const createRequest = ( endpoint, query = {}, @@ -58,9 +71,7 @@ export const createRequest = ( fetch(getUrl(assembleEndpoint(endpoint, query)), { method, headers, - body: body - ? uploadFiles ? createFormData(body) : JSON.stringify(body) - : undefined + body: encodeBody(body, method, uploadFiles) }); export const getHeaders = (headers, accessToken, skipContentType) => {