From 177f39cb5c30ed02b738d61bb42551415dd25ae6 Mon Sep 17 00:00:00 2001 From: Oliver Rumbelow Date: Sat, 14 May 2016 20:14:45 +0100 Subject: [PATCH 1/4] Reroute instead of re-requestincluded resources --- lib/postProcess.js | 26 ++++++------------- lib/postProcessing/include.js | 31 ++++++---------------- lib/rerouter.js | 48 +++++++++++++++++++++++++++++++++++ lib/router.js | 11 ++++++-- package.json | 13 +++++----- 5 files changed, 80 insertions(+), 49 deletions(-) create mode 100644 lib/rerouter.js diff --git a/lib/postProcess.js b/lib/postProcess.js index 5501ded1..e57572b2 100644 --- a/lib/postProcess.js +++ b/lib/postProcess.js @@ -4,9 +4,7 @@ var postProcess = module.exports = { }; var jsonApi = require(".."); var debug = require("./debugging.js"); -var externalRequest = require("request").defaults({ - pool: { maxSockets: Infinity } -}); +var rerouter = require("./rerouter.js"); var async = require("async"); postProcess._applySort = require("./postProcessing/sort.js").action; postProcess._applyFilter = require("./postProcessing/filter.js").action; @@ -57,23 +55,15 @@ postProcess._fetchRelatedResources = function(request, mainResource, callback) { async.map(resourcesToFetch, function(related, done) { debug.include(related); - externalRequest({ + rerouter.route({ method: "GET", uri: related, - headers: request.safeHeaders - }, function(err, externalRes, json) { - if (err || !json) return done(null, [ ]); - - try { - json = JSON.parse(json); - } catch(e) { - json = null; - } - - if (!json) return done(null, [ ]); - - if (externalRes.statusCode >= 400) { - return done(json.errors); + headers: request.safeHeaders, + cookies: request.cookies + }, function(err, json) { + if (err) { + debug.include("!!", JSON.stringify(err)); + return done(err.errors); } var data = json.data; diff --git a/lib/postProcessing/include.js b/lib/postProcessing/include.js index 29f320a0..30ae8d80 100644 --- a/lib/postProcessing/include.js +++ b/lib/postProcessing/include.js @@ -6,9 +6,7 @@ var jsonApi = require("../jsonApi.js"); var _ = { unique: require("lodash.uniq") }; -var externalRequest = require("request").defaults({ - pool: { maxSockets: Infinity } -}); +var rerouter = require("../rerouter.js"); var async = require("async"); var debug = require("../debugging.js"); @@ -186,28 +184,15 @@ includePP._fillIncludeTree = function(includeTree, request, callback) { var parts = related.as.split("~~"); debug.include(related); - externalRequest({ + rerouter.route({ method: "GET", uri: related.url, - headers: request.safeHeaders - }, function(err, res, json) { - // console.log(err, json) - if (err || !json) { - return done(null); - } - - try { - json = JSON.parse(json); - } catch(e) { - debug.include("!!", JSON.stringify(json)); - json = null; - } - - if (!json) return done(null, [ ]); - - if (res.statusCode >= 400) { - debug.include("!!", JSON.stringify(json)); - return done(json.errors); + headers: request.safeHeaders, + cookies: request.cookies + }, function(err, json) { + if (err) { + debug.include("!!", JSON.stringify(err)); + return done(err.errors); } var data = json.data; diff --git a/lib/rerouter.js b/lib/rerouter.js new file mode 100644 index 00000000..4cc50b6b --- /dev/null +++ b/lib/rerouter.js @@ -0,0 +1,48 @@ +/* @flow weak */ +"use strict"; +var rerouter = module.exports = { }; + +var router = require("./router.js"); +var jsonApi = require("./jsonApi.js"); +var url = require("qs"); + + +rerouter.route = function(newRequest, callback) { + var validRoutes = router._routes[newRequest.method.toLowerCase()]; + var path = newRequest.uri.split(jsonApi._apiConfig.base); + path.shift(); + path = path.join(jsonApi._apiConfig.base).split("?")[0].replace(/\/$/, ""); + + var route = Object.keys(validRoutes).filter(function(someRoute) { + someRoute = someRoute.replace(/(\:[a-z]+)/g, "[^/]*?"); + someRoute = new RegExp("^" + someRoute); + return someRoute.test(path); + }).pop(); + var req = { + url: newRequest.uri, + headers: newRequest.headers, + cookies: newRequest.cookies, + params: url.parse(newRequest.uri.split("?").pop()) + }; + + route.split("/").forEach(function(urlPart, i) { + if (urlPart[0] !== ":") return; + req.params[urlPart.substring(1)] = path.split("/")[i]; + }); + + var res = { + status: function(httpCode) { + res.httpCode = httpCode; + return res; + }, + json: function(payload) { + var err = null; + if (res.httpCode >= 400) { + err = payload; + payload = undefined; + } + return callback(err, payload); + } + }; + validRoutes[route](req, res); +}; diff --git a/lib/router.js b/lib/router.js index 7dbe7b68..a513c7db 100644 --- a/lib/router.js +++ b/lib/router.js @@ -88,15 +88,22 @@ router.close = function() { server = null; }; +router._routes = { }; router.bindRoute = function(config, callback) { - app[config.verb](jsonApi._apiConfig.base + config.path, function(req, res) { + var path = jsonApi._apiConfig.base + config.path; + var verb = config.verb.toLowerCase(); + + var routeHandler = function(req, res) { var request = router._getParams(req); var resourceConfig = jsonApi._resources[request.params.type]; request.resourceConfig = resourceConfig; router.authenticate(request, res, function() { return callback(request, resourceConfig, res); }); - }); + }; + router._routes[verb] = router._routes[verb] || { }; + router._routes[verb][config.path] = routeHandler; + app[verb](path, routeHandler); }; router.authenticate = function(request, res, callback) { diff --git a/package.json b/package.json index e6d6067a..cf6d204b 100644 --- a/package.json +++ b/package.json @@ -24,26 +24,27 @@ "cookie-parser": "1.4.0", "debug": "2.2.0", "express": "4.13.3", - "flow-bin": "^0.23.1", "joi": "6.10.1", - "jscpd": "^0.6.1", "lodash.assign": "3.2.0", "lodash.isequal": "3.0.4", "lodash.omit": "3.1.0", "lodash.pick": "3.1.0", "lodash.uniq": "3.2.2", "node-uuid": "1.4.7", + "qs": "^6.2.0", "request": "2.67.0" }, "devDependencies": { - "mocha": "2.2.5", - "eslint": "0.24.1", "blanket": "1.1.7", - "mocha-lcov-reporter": "0.0.2", "coveralls": "2.11.2", - "plato": "1.5.0", + "eslint": "0.24.1", + "flow-bin": "^0.23.1", + "jscpd": "^0.6.1", + "mocha": "2.2.5", + "mocha-lcov-reporter": "0.0.2", "mocha-performance": "0.1.0", "node-inspector": "0.12.5", + "plato": "1.5.0", "v8-profiler": "5.5.0" }, "scripts": { From dc70d74d81c72eb9e6552e046072e36eba301044 Mon Sep 17 00:00:00 2001 From: Oliver Rumbelow Date: Sat, 14 May 2016 20:31:01 +0100 Subject: [PATCH 2/4] Reroute instead of re-requestincluded resources --- lib/rerouter.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/rerouter.js b/lib/rerouter.js index 4cc50b6b..6ce5d1b2 100644 --- a/lib/rerouter.js +++ b/lib/rerouter.js @@ -9,9 +9,18 @@ var url = require("qs"); rerouter.route = function(newRequest, callback) { var validRoutes = router._routes[newRequest.method.toLowerCase()]; - var path = newRequest.uri.split(jsonApi._apiConfig.base); - path.shift(); - path = path.join(jsonApi._apiConfig.base).split("?")[0].replace(/\/$/, ""); + + var path = newRequest.uri; + if (path.match(/^https?\:\/\//)) { + path = path.split("/").slice(3).join("/"); + } + if (jsonApi._apiConfig.base !== "/") { + if (path[0] !== "/") path = "/" + path; + path = path.split(jsonApi._apiConfig.base); + path.shift(); + path = path.join(jsonApi._apiConfig.base); + } + path = path.replace(/^\//, "").split("?")[0].replace(/\/$/, ""); var route = Object.keys(validRoutes).filter(function(someRoute) { someRoute = someRoute.replace(/(\:[a-z]+)/g, "[^/]*?"); From e3289bfe6d1dfa77610a7731d456fb607d2012a1 Mon Sep 17 00:00:00 2001 From: theninj4 Date: Mon, 16 May 2016 10:03:39 +0100 Subject: [PATCH 3/4] Pass through additional user-data on inclusions --- lib/postProcess.js | 3 +-- lib/postProcessing/include.js | 3 +-- lib/rerouter.js | 9 ++++++--- lib/router.js | 3 ++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/postProcess.js b/lib/postProcess.js index e57572b2..1c9a675c 100644 --- a/lib/postProcess.js +++ b/lib/postProcess.js @@ -58,8 +58,7 @@ postProcess._fetchRelatedResources = function(request, mainResource, callback) { rerouter.route({ method: "GET", uri: related, - headers: request.safeHeaders, - cookies: request.cookies + originalRequest: request }, function(err, json) { if (err) { debug.include("!!", JSON.stringify(err)); diff --git a/lib/postProcessing/include.js b/lib/postProcessing/include.js index 30ae8d80..d66204e1 100644 --- a/lib/postProcessing/include.js +++ b/lib/postProcessing/include.js @@ -187,8 +187,7 @@ includePP._fillIncludeTree = function(includeTree, request, callback) { rerouter.route({ method: "GET", uri: related.url, - headers: request.safeHeaders, - cookies: request.cookies + originalRequest: request }, function(err, json) { if (err) { debug.include("!!", JSON.stringify(err)); diff --git a/lib/rerouter.js b/lib/rerouter.js index 6ce5d1b2..d5b30970 100644 --- a/lib/rerouter.js +++ b/lib/rerouter.js @@ -5,6 +5,9 @@ var rerouter = module.exports = { }; var router = require("./router.js"); var jsonApi = require("./jsonApi.js"); var url = require("qs"); +var _ = { + omit: require("lodash.omit") +}; rerouter.route = function(newRequest, callback) { @@ -29,8 +32,8 @@ rerouter.route = function(newRequest, callback) { }).pop(); var req = { url: newRequest.uri, - headers: newRequest.headers, - cookies: newRequest.cookies, + headers: newRequest.originalRequest.headers, + cookies: newRequest.originalRequest.cookies, params: url.parse(newRequest.uri.split("?").pop()) }; @@ -53,5 +56,5 @@ rerouter.route = function(newRequest, callback) { return callback(err, payload); } }; - validRoutes[route](req, res); + validRoutes[route](req, res, _.omit(newRequest.originalRequest, [ "params", "route" ])); }; diff --git a/lib/router.js b/lib/router.js index a513c7db..48719b4b 100644 --- a/lib/router.js +++ b/lib/router.js @@ -93,8 +93,9 @@ router.bindRoute = function(config, callback) { var path = jsonApi._apiConfig.base + config.path; var verb = config.verb.toLowerCase(); - var routeHandler = function(req, res) { + var routeHandler = function(req, res, extras) { var request = router._getParams(req); + request = _.assign(request, extras); var resourceConfig = jsonApi._resources[request.params.type]; request.resourceConfig = resourceConfig; router.authenticate(request, res, function() { From 0d2fe0bc23c9e7f6ae4bcab886e39417be2a9461 Mon Sep 17 00:00:00 2001 From: theninj4 Date: Mon, 16 May 2016 10:34:52 +0100 Subject: [PATCH 4/4] Extract reroute complexity into private functions --- lib/rerouter.js | 54 ++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/lib/rerouter.js b/lib/rerouter.js index d5b30970..171ef63a 100644 --- a/lib/rerouter.js +++ b/lib/rerouter.js @@ -13,34 +13,16 @@ var _ = { rerouter.route = function(newRequest, callback) { var validRoutes = router._routes[newRequest.method.toLowerCase()]; - var path = newRequest.uri; - if (path.match(/^https?\:\/\//)) { - path = path.split("/").slice(3).join("/"); - } - if (jsonApi._apiConfig.base !== "/") { - if (path[0] !== "/") path = "/" + path; - path = path.split(jsonApi._apiConfig.base); - path.shift(); - path = path.join(jsonApi._apiConfig.base); - } - path = path.replace(/^\//, "").split("?")[0].replace(/\/$/, ""); + var path = rerouter._generateSanePath(newRequest); + var route = rerouter._pickFirstMatchingRoute(validRoutes, path); - var route = Object.keys(validRoutes).filter(function(someRoute) { - someRoute = someRoute.replace(/(\:[a-z]+)/g, "[^/]*?"); - someRoute = new RegExp("^" + someRoute); - return someRoute.test(path); - }).pop(); var req = { url: newRequest.uri, headers: newRequest.originalRequest.headers, cookies: newRequest.originalRequest.cookies, params: url.parse(newRequest.uri.split("?").pop()) }; - - route.split("/").forEach(function(urlPart, i) { - if (urlPart[0] !== ":") return; - req.params[urlPart.substring(1)] = path.split("/")[i]; - }); + rerouter._extendUrlParamsOntoReq(route, path, req); var res = { status: function(httpCode) { @@ -58,3 +40,33 @@ rerouter.route = function(newRequest, callback) { }; validRoutes[route](req, res, _.omit(newRequest.originalRequest, [ "params", "route" ])); }; + +rerouter._generateSanePath = function(newRequest) { + var path = newRequest.uri; + if (path.match(/^https?\:\/\//)) { + path = path.split("/").slice(3).join("/"); + } + if (jsonApi._apiConfig.base !== "/") { + if (path[0] !== "/") path = "/" + path; + path = path.split(jsonApi._apiConfig.base); + path.shift(); + path = path.join(jsonApi._apiConfig.base); + } + path = path.replace(/^\//, "").split("?")[0].replace(/\/$/, ""); + return path; +}; + +rerouter._pickFirstMatchingRoute = function(validRoutes, path) { + return Object.keys(validRoutes).filter(function(someRoute) { + someRoute = someRoute.replace(/(\:[a-z]+)/g, "[^/]*?"); + someRoute = new RegExp("^" + someRoute); + return someRoute.test(path); + }).pop(); +}; + +rerouter._extendUrlParamsOntoReq = function(route, path, req) { + route.split("/").forEach(function(urlPart, i) { + if (urlPart[0] !== ":") return; + req.params[urlPart.substring(1)] = path.split("/")[i]; + }); +};