From 15873aac3a3358cfd1e39edc98aad50531b5131a Mon Sep 17 00:00:00 2001 From: Daniel Alksnis Date: Sat, 24 Dec 2016 12:14:25 -0500 Subject: [PATCH 1/7] allows objects to pass graphiql --- lib/graphQl/joiConverter.js | 16 ++++++++++++++++ lib/swagger/index.js | 8 ++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/graphQl/joiConverter.js b/lib/graphQl/joiConverter.js index 745f25a9..c1ca850f 100644 --- a/lib/graphQl/joiConverter.js +++ b/lib/graphQl/joiConverter.js @@ -6,6 +6,7 @@ const readTypes = require('./readTypes.js') joiConverter.simpleAttribute = joiScheme => { let type = joiScheme._type + if (type === 'any') { // { _valids: { _set: [ 'M', 'F' ] } } type = typeof (joiScheme._valids._set || [ ])[0] @@ -31,9 +32,24 @@ joiConverter.simpleAttribute = joiScheme => { } } + if (type === 'object') { + if (joiScheme._inner.children.length) { + let joinSubType = joiConverter.simpleAttribute(joiScheme._inner.children[0].schema) + + if (!joinSubType) { + throw new Error('Unable to parse Joi type, got ' + JSON.stringify(joiScheme)) + } + + return new graphQl.GraphQLList(graphQl.GraphQLString) + } else { + throw new Error('Joi objects must contain defined keys') + } + } + const uType = type[0].toUpperCase() + type.substring(1) type = graphQl[`GraphQL${uType}`] + if (!type) { throw new Error('Unable to parse Joi type, got ' + JSON.stringify(joiScheme)) } diff --git a/lib/swagger/index.js b/lib/swagger/index.js index 20a2bd30..476b2ae8 100644 --- a/lib/swagger/index.js +++ b/lib/swagger/index.js @@ -18,6 +18,7 @@ swagger._getSwaggerBase = () => { if (jsonApi._apiConfig.urlPrefixAlias) { const urlObj = url.parse(jsonApi._apiConfig.urlPrefixAlias) basePath = urlObj.pathname.replace(/\/$/, '') + if (basePath.length === 0) basePath = '/'; host = urlObj.host protocol = urlObj.protocol.replace(/:$/, '') } else { @@ -41,6 +42,13 @@ swagger._getSwaggerBase = () => { url: (swaggerConfig.license || { }).url } }, + "securityDefinitions": { + "api_key": { + "type": "apiKey", + "name": "api_key", + "in": "header" + } + }, host, basePath, schemes: [ protocol ], From 88e92c5e8e6b2ef4fc049a2142a606586030bb59 Mon Sep 17 00:00:00 2001 From: Daniel Alksnis Date: Mon, 26 Dec 2016 17:01:48 -0500 Subject: [PATCH 2/7] supress graphiql if config sets false; record originalUrl value in request object; small swagger validation fix for when using prefixalias; limit url for internal resource fetching to basepath onwards --- lib/graphQl/index.js | 10 ++++++---- lib/graphQl/joiConverter.js | 16 ---------------- lib/postProcessing/include.js | 4 ++-- lib/rerouter.js | 1 + lib/router.js | 1 + lib/swagger/index.js | 11 ++--------- 6 files changed, 12 insertions(+), 31 deletions(-) diff --git a/lib/graphQl/index.js b/lib/graphQl/index.js index efa8503f..a408d4af 100644 --- a/lib/graphQl/index.js +++ b/lib/graphQl/index.js @@ -13,10 +13,12 @@ const writeTypes = require('./writeTypes.js') jsonApiGraphQL.with = app => { const config = jsonApi._apiConfig - app.use(new RegExp(`${config.base}$`), graphqlHTTP({ - schema: jsonApiGraphQL.generate(jsonApi._resources), - graphiql: !!config.graphiql - })) + if (config.graphiql !== false) { + app.use(new RegExp(`${config.base}$`), graphqlHTTP({ + schema: jsonApiGraphQL.generate(jsonApi._resources), + graphiql: !!config.graphiql + })) + } } jsonApiGraphQL.generate = allResourceConfig => { diff --git a/lib/graphQl/joiConverter.js b/lib/graphQl/joiConverter.js index c1ca850f..745f25a9 100644 --- a/lib/graphQl/joiConverter.js +++ b/lib/graphQl/joiConverter.js @@ -6,7 +6,6 @@ const readTypes = require('./readTypes.js') joiConverter.simpleAttribute = joiScheme => { let type = joiScheme._type - if (type === 'any') { // { _valids: { _set: [ 'M', 'F' ] } } type = typeof (joiScheme._valids._set || [ ])[0] @@ -32,24 +31,9 @@ joiConverter.simpleAttribute = joiScheme => { } } - if (type === 'object') { - if (joiScheme._inner.children.length) { - let joinSubType = joiConverter.simpleAttribute(joiScheme._inner.children[0].schema) - - if (!joinSubType) { - throw new Error('Unable to parse Joi type, got ' + JSON.stringify(joiScheme)) - } - - return new graphQl.GraphQLList(graphQl.GraphQLString) - } else { - throw new Error('Joi objects must contain defined keys') - } - } - const uType = type[0].toUpperCase() + type.substring(1) type = graphQl[`GraphQL${uType}`] - if (!type) { throw new Error('Unable to parse Joi type, got ' + JSON.stringify(joiScheme)) } diff --git a/lib/postProcessing/include.js b/lib/postProcessing/include.js index 33101ddb..b72426e8 100644 --- a/lib/postProcessing/include.js +++ b/lib/postProcessing/include.js @@ -159,7 +159,7 @@ includePP._fillIncludeTree = (includeTree, request, callback) => { ids += `&${includeTree[parts[1]]._filter.join('&')}` } resourcesToFetch.push({ - url: `${jsonApi._apiConfig.pathPrefix + parts[0]}/?${ids}`, + url: `${jsonApi._apiConfig.base + parts[0]}/?${ids}`, as: relation }) }) @@ -173,7 +173,7 @@ includePP._fillIncludeTree = (includeTree, request, callback) => { ids += `&${includeTree[parts[2]]._filter.join('&')}` } resourcesToFetch.push({ - url: `${jsonApi._apiConfig.pathPrefix + parts[1]}/?${ids}`, + url: `${jsonApi._apiConfig.base + parts[1]}/?${ids}`, as: relation }) }) diff --git a/lib/rerouter.js b/lib/rerouter.js index d073dc2e..4b9c7faa 100644 --- a/lib/rerouter.js +++ b/lib/rerouter.js @@ -18,6 +18,7 @@ rerouter.route = (newRequest, callback) => { const req = { url: newRequest.uri, + originalUrl: newRequest.originalRequest.originalUrl, headers: newRequest.originalRequest.headers, cookies: newRequest.originalRequest.cookies, params: rerouter._mergeParams(url.parse(newRequest.uri.split('?')[1] || { }), newRequest.params) diff --git a/lib/router.js b/lib/router.js index bb287bf9..ad67c51c 100644 --- a/lib/router.js +++ b/lib/router.js @@ -184,6 +184,7 @@ router._getParams = req => { headers: req.headers, safeHeaders: _.omit(req.headers, headersToRemove), cookies: req.cookies, + originalUrl: req.originalUrl, route: { verb: req.method, host: req.headers.host, diff --git a/lib/swagger/index.js b/lib/swagger/index.js index 476b2ae8..cc7ca259 100644 --- a/lib/swagger/index.js +++ b/lib/swagger/index.js @@ -17,8 +17,8 @@ swagger._getSwaggerBase = () => { let basePath, host, protocol if (jsonApi._apiConfig.urlPrefixAlias) { const urlObj = url.parse(jsonApi._apiConfig.urlPrefixAlias) - basePath = urlObj.pathname.replace(/\/$/, '') - if (basePath.length === 0) basePath = '/'; + basePath = urlObj.pathname + if (basePath !== '/') basePath = basePath.replace(/\/$/, '') host = urlObj.host protocol = urlObj.protocol.replace(/:$/, '') } else { @@ -42,13 +42,6 @@ swagger._getSwaggerBase = () => { url: (swaggerConfig.license || { }).url } }, - "securityDefinitions": { - "api_key": { - "type": "apiKey", - "name": "api_key", - "in": "header" - } - }, host, basePath, schemes: [ protocol ], From 4a4a180d732dc27485bcc352e0b97d4da26f433b Mon Sep 17 00:00:00 2001 From: Daniel Alksnis Date: Tue, 27 Dec 2016 14:48:33 -0500 Subject: [PATCH 3/7] Fixed bug where comma delineated filters would upset the internal routing of includes --- example/resources/photos.js | 12 ++++++++++ lib/filter.js | 4 ++-- lib/postProcessing/include.js | 4 +++- test/_preResourceValidationCheck.js | 2 +- test/get-resource.js | 32 ++++++++++++++++++++++----- test/zzPostResourceValidationCheck.js | 2 +- 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/example/resources/photos.js b/example/resources/photos.js index f484a340..ed1e7907 100644 --- a/example/resources/photos.js +++ b/example/resources/photos.js @@ -64,6 +64,18 @@ jsonApi.define({ width: 350, tags: ['black', 'green'], photographer: { type: 'people', id: 'ad3aa89e-9c5b-4ac9-a652-6670f9f27587' } + }, + { + id: 'ed45eba1-15fe-41c7-93da-1df3dfa5289f', + type: 'photos', + title: 'Sunset Horizon', + url: 'http://www.example.com/sunset', + height: 450, + width: 1050, + raw: true, + tags: ['orange', 'sky', 'sun'], + photographer: { type: 'people', id: 'cc5cca2e-0dd8-4b95-8cfc-a11230e73116' } } + ] }) diff --git a/lib/filter.js b/lib/filter.js index 41214e7b..9bbe6a33 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -67,8 +67,6 @@ filter._parseScalarFilterElement = (attributeConfig, scalarElement) => { filter._parseFilterElementHelper = (attributeConfig, filterElement) => { if (!filterElement) return { error: 'invalid or empty filter element' } - if (typeof filterElement === 'string') filterElement = filterElement.split(FILTER_SEPERATOR) - const parsedElements = [].concat(filterElement).map(scalarElement => filter._parseScalarFilterElement(attributeConfig, scalarElement)) if (parsedElements.length === 1) return parsedElements[0] @@ -117,6 +115,8 @@ filter.parseAndValidate = request => { for (const key in request.params.filter) { filterElement = request.params.filter[key] + if (typeof filterElement === 'string') request.params.filter[key] = filterElement = filterElement.split(FILTER_SEPERATOR) + if (!Array.isArray(filterElement) && filterElement instanceof Object) continue // skip deep filters error = filter._resourceDoesNotHaveProperty(resourceConfig, key) diff --git a/lib/postProcessing/include.js b/lib/postProcessing/include.js index b72426e8..a6985dbc 100644 --- a/lib/postProcessing/include.js +++ b/lib/postProcessing/include.js @@ -12,7 +12,7 @@ const debug = require('../debugging.js') includePP.action = (request, response, callback) => { let includes = request.params.include - const filters = request.params.filter || { } + const filters = request.params.filter if (!includes) return callback() includes = (`${includes}`).split(',') @@ -47,6 +47,7 @@ includePP._arrayToTree = (request, includes, filters, callback) => { const parts = text.split('.') const first = parts.shift() const rest = parts.join('.') + if (!filter) filter = {} let resourceAttribute = node._resourceConfig.map(resourceConfig => { return resourceConfig.attributes[first] @@ -70,6 +71,7 @@ includePP._arrayToTree = (request, includes, filters, callback) => { } filter = filter[first] || { } + if (filter instanceof Array) { filter = filter.filter(i => i instanceof Object).pop() } diff --git a/test/_preResourceValidationCheck.js b/test/_preResourceValidationCheck.js index d5fedf6b..21f8b28c 100644 --- a/test/_preResourceValidationCheck.js +++ b/test/_preResourceValidationCheck.js @@ -6,7 +6,7 @@ describe('Testing jsonapi-server', () => { [ { name: 'articles', count: 4 }, { name: 'comments', count: 3 }, { name: 'people', count: 4 }, - { name: 'photos', count: 3 }, + { name: 'photos', count: 4 }, { name: 'tags', count: 5 } ].forEach(resource => { describe(`Searching for ${resource.name}`, () => { diff --git a/test/get-resource.js b/test/get-resource.js index 79cdcc2c..5bde7181 100644 --- a/test/get-resource.js +++ b/test/get-resource.js @@ -201,7 +201,7 @@ describe('Testing jsonapi-server', () => { assert.equal(res.statusCode, '200', 'Expecting 200 OK') const photoTypes = json.data.map(i => i.attributes.raw) - assert.deepEqual(photoTypes, [ true ], 'expected matching resources') + assert.deepEqual(photoTypes, [ true, true ], 'expected matching resources') done() }) @@ -530,13 +530,13 @@ describe('Testing jsonapi-server', () => { json = helpers.validateJson(json) assert.equal(res.statusCode, '200', 'Expecting 200 OK') - assert.equal(json.included.length, 7, 'Should be 7 included resources') + assert.equal(json.included.length, 8, 'Should be 8 included resources') const people = json.included.filter(resource => resource.type === 'people') assert.equal(people.length, 4, 'Should be 4 included people resources') const photos = json.included.filter(resource => resource.type === 'photos') - assert.equal(photos.length, 3, 'Should be 3 included photos resources') + assert.equal(photos.length, 4, 'Should be 4 included photos resources') done() }) @@ -552,13 +552,13 @@ describe('Testing jsonapi-server', () => { json = helpers.validateJson(json) assert.equal(res.statusCode, '200', 'Expecting 200 OK') - assert.equal(json.included.length, 7, 'Should be 7 included resources') + assert.equal(json.included.length, 8, 'Should be 8 included resources') const people = json.included.filter(resource => resource.type === 'people') assert.equal(people.length, 4, 'Should be 4 included people resources') const photos = json.included.filter(resource => resource.type === 'photos') - assert.equal(photos.length, 3, 'Should be 3 included photos resources') + assert.equal(photos.length, 4, 'Should be 4 included photos resources') done() }) @@ -585,6 +585,28 @@ describe('Testing jsonapi-server', () => { done() }) }) + + it('include author.photos with multiple filters', done => { + const url = 'http://localhost:16006/rest/articles?include=author.photos&filter[author][firstname]=Mark,Oli' + helpers.request({ + method: 'GET', + url + }, (err, res, json) => { + assert.equal(err, null) + json = helpers.validateJson(json) + + assert.equal(res.statusCode, '200', 'Expecting 200 OK') + assert.equal(json.included.length, 4, 'Should be 2 included resources') + + const people = json.included.filter(resource => resource.type === 'people') + assert.equal(people.length, 2, 'Should be 2 included people resource') + + const photos = json.included.filter(resource => resource.type === 'photos') + assert.equal(photos.length, 2, 'Should be 2 included photos resource') + + done() + }) + }) }) describe('by foreign key', () => { diff --git a/test/zzPostResourceValidationCheck.js b/test/zzPostResourceValidationCheck.js index be772276..1e9b1652 100644 --- a/test/zzPostResourceValidationCheck.js +++ b/test/zzPostResourceValidationCheck.js @@ -6,7 +6,7 @@ describe('Testing jsonapi-server', () => { [ { name: 'articles', count: 4 }, { name: 'comments', count: 2 }, { name: 'people', count: 4 }, - { name: 'photos', count: 4 }, + { name: 'photos', count: 5 }, { name: 'tags', count: 5 } ].forEach(resource => { describe(`Searching for ${resource.name}`, () => { From 62c00d98eeb7bb1fa77830aae0745402c2365cac Mon Sep 17 00:00:00 2001 From: Daniel Alksnis Date: Tue, 27 Dec 2016 14:59:59 -0500 Subject: [PATCH 4/7] Added one more test to check non-comma filter standard --- test/get-resource.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/get-resource.js b/test/get-resource.js index 5bde7181..a588d31b 100644 --- a/test/get-resource.js +++ b/test/get-resource.js @@ -587,6 +587,28 @@ describe('Testing jsonapi-server', () => { }) it('include author.photos with multiple filters', done => { + const url = 'http://localhost:16006/rest/articles?include=author.photos&filter[author]=ad3aa89e-9c5b-4ac9-a652-6670f9f27587&filter[author]=cc5cca2e-0dd8-4b95-8cfc-a11230e73116' + helpers.request({ + method: 'GET', + url + }, (err, res, json) => { + assert.equal(err, null) + json = helpers.validateJson(json) + + assert.equal(res.statusCode, '200', 'Expecting 200 OK') + assert.equal(json.included.length, 5, 'Should be 2 included resources') + + const people = json.included.filter(resource => resource.type === 'people') + assert.equal(people.length, 2, 'Should be 2 included people resource') + + const photos = json.included.filter(resource => resource.type === 'photos') + assert.equal(photos.length, 3, 'Should be 2 included photos resource') + + done() + }) + }) + + it('include author.photos with multiple filters comma delineated', done => { const url = 'http://localhost:16006/rest/articles?include=author.photos&filter[author][firstname]=Mark,Oli' helpers.request({ method: 'GET', From 8bb500ff6427a6b2e139e961e9b90752e50a7fbc Mon Sep 17 00:00:00 2001 From: Daniel A Date: Sun, 5 Feb 2017 14:34:14 -0500 Subject: [PATCH 5/7] replaced if condition with regex; added coverage folder to gitignore --- .gitignore | 2 +- lib/swagger/index.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 79f4c92f..50904f29 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ node_modules -coverage.html +coverage complexity npm-debug.log jsonapi-server.cpuprofile diff --git a/lib/swagger/index.js b/lib/swagger/index.js index cc7ca259..d69322a3 100644 --- a/lib/swagger/index.js +++ b/lib/swagger/index.js @@ -17,8 +17,7 @@ swagger._getSwaggerBase = () => { let basePath, host, protocol if (jsonApi._apiConfig.urlPrefixAlias) { const urlObj = url.parse(jsonApi._apiConfig.urlPrefixAlias) - basePath = urlObj.pathname - if (basePath !== '/') basePath = basePath.replace(/\/$/, '') + basePath = basePath.replace(/(?!^\/)\//, '') host = urlObj.host protocol = urlObj.protocol.replace(/:$/, '') } else { From c68857ab5591fc61838d9deac5959647ddb0a738 Mon Sep 17 00:00:00 2001 From: Daniel A Date: Sun, 5 Feb 2017 14:43:49 -0500 Subject: [PATCH 6/7] adjusted regex to look for forward slash from end of statement --- lib/swagger/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/swagger/index.js b/lib/swagger/index.js index d69322a3..e49790ea 100644 --- a/lib/swagger/index.js +++ b/lib/swagger/index.js @@ -17,7 +17,7 @@ swagger._getSwaggerBase = () => { let basePath, host, protocol if (jsonApi._apiConfig.urlPrefixAlias) { const urlObj = url.parse(jsonApi._apiConfig.urlPrefixAlias) - basePath = basePath.replace(/(?!^\/)\//, '') + basePath = basePath.replace(/(?!^\/)\/$/, '') host = urlObj.host protocol = urlObj.protocol.replace(/:$/, '') } else { From ca1623a820968464ffc30ffe5f426f2eb0ad5b9a Mon Sep 17 00:00:00 2001 From: Daniel A Date: Sun, 5 Feb 2017 22:20:55 -0500 Subject: [PATCH 7/7] typo in swagger basepath generate --- lib/swagger/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/swagger/index.js b/lib/swagger/index.js index e49790ea..32dfa1ee 100644 --- a/lib/swagger/index.js +++ b/lib/swagger/index.js @@ -17,7 +17,7 @@ swagger._getSwaggerBase = () => { let basePath, host, protocol if (jsonApi._apiConfig.urlPrefixAlias) { const urlObj = url.parse(jsonApi._apiConfig.urlPrefixAlias) - basePath = basePath.replace(/(?!^\/)\/$/, '') + basePath = urlObj.pathname.replace(/(?!^\/)\/$/, '') host = urlObj.host protocol = urlObj.protocol.replace(/:$/, '') } else {