From 4532ae3739237a3b5a84f00eed09219b4bae13ef Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:13:12 +0100 Subject: [PATCH 1/8] extracted apiProxy request logic to a reusable method --- server/middleware/apiProxy.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/server/middleware/apiProxy.js b/server/middleware/apiProxy.js index 924578ab..e783a50e 100644 --- a/server/middleware/apiProxy.js +++ b/server/middleware/apiProxy.js @@ -13,20 +13,15 @@ var separator = '/-/'; module.exports = apiProxy; function apiProxy(dataAdapter) { - return function(req, res, next) { + var middleware = function(req, res, next) { var api; api = _.pick(req, 'query', 'method', 'body'); api.path = apiProxy.getApiPath(req.path); api.api = apiProxy.getApiName(req.path); - api.headers = { - 'x-forwarded-for': apiProxy.getXForwardedForHeader(req.headers, req.ip) - }; - dataAdapter.request(req, api, { - convertErrorCode: false - }, function(err, response, body) { + middleware.proxyRequest(req, res, api, function(err, response, body) { if (err) return next(err); // Pass through statusCode. @@ -34,6 +29,16 @@ function apiProxy(dataAdapter) { res.json(body); }); }; + + middleware.proxyRequest = function proxyRequest(req, res, api, callback) { + api.headers = { + 'x-forwarded-for': apiProxy.getXForwardedForHeader(req.headers, req.ip) + }; + + dataAdapter.request(req, api, { convertErrorCode: false }, callback); + }; + + return middleware; }; apiProxy.getApiPath = function getApiPath(path) { From 23b83ff4b43cb0fcd41e1290a618e83624610f2f Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:15:41 +0100 Subject: [PATCH 2/8] headers are no longer overwritten every time --- server/middleware/apiProxy.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/middleware/apiProxy.js b/server/middleware/apiProxy.js index e783a50e..8b356fca 100644 --- a/server/middleware/apiProxy.js +++ b/server/middleware/apiProxy.js @@ -31,9 +31,8 @@ function apiProxy(dataAdapter) { }; middleware.proxyRequest = function proxyRequest(req, res, api, callback) { - api.headers = { - 'x-forwarded-for': apiProxy.getXForwardedForHeader(req.headers, req.ip) - }; + api.headers = api.headers || {}; + api.headers['x-forwarded-for'] = apiProxy.getXForwardedForHeader(req.headers, req.ip); dataAdapter.request(req, api, { convertErrorCode: false }, callback); }; From 851513e4f7128900933ca40d29bac37c393c3612 Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:20:04 +0100 Subject: [PATCH 3/8] extracted apiProxy middleware variable --- server/server.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/server.js b/server/server.js index 2a42bb65..b437ec6c 100644 --- a/server/server.js +++ b/server/server.js @@ -80,7 +80,11 @@ function Server(options) { Server.prototype.configure = function(fn) { var dataAdapter = this.dataAdapter, apiPath = this.options.apiPath, - notApiRegExp = new RegExp('^(?!' + apiPath.replace('/', '\\/') + '\\/)'); + notApiRegExp = new RegExp('^(?!' + apiPath.replace('/', '\\/') + '\\/)'), + apiProxyMiddleware; + + this.options.apiProxy = this.options.apiProxy || middleware.apiProxy; + apiProxyMiddleware = this.options.apiProxy(dataAdapter); this._configured = true; @@ -120,8 +124,7 @@ Server.prototype.configure = function(fn) { /** * Add the API handler. */ - this.options.apiProxy = this.options.apiProxy || middleware.apiProxy; - this.expressApp.use(this.options.apiPath, this.options.apiProxy(dataAdapter)); + this.expressApp.use(this.options.apiPath, apiProxyMiddleware); /** * Add the routes for everything defined in our routes file. From 8a699c97c22b2840d67e98cf29300d1d5fe5855c Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:23:43 +0100 Subject: [PATCH 4/8] added the proxyRequest method to the global app object --- server/middleware/initApp.js | 4 ++++ server/server.js | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/server/middleware/initApp.js b/server/middleware/initApp.js index 89bf3354..7cb963b3 100644 --- a/server/middleware/initApp.js +++ b/server/middleware/initApp.js @@ -50,6 +50,10 @@ module.exports = function(appAttributes, options) { var app = new App(attributes, appOptions); + if (options.proxyRequest) { + app.proxyRequest = options.proxyRequest; + } + /** * Stash the app instance on the request so can be accessed in other middleware. */ diff --git a/server/server.js b/server/server.js index b437ec6c..709164a8 100644 --- a/server/server.js +++ b/server/server.js @@ -112,7 +112,8 @@ Server.prototype.configure = function(fn) { this.expressApp.use(middleware.initApp(this.options.appData, { apiPath: this.options.apiPath, entryPath: this.options.entryPath, - modelUtils: this.options.modelUtils + modelUtils: this.options.modelUtils, + proxyRequest: apiProxyMiddleware.proxyRequest })); /** From 2095092d84729b14533f55f8aa24d0212bd01cf8 Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:26:00 +0100 Subject: [PATCH 5/8] added the res object to the global app object --- server/middleware/initApp.js | 1 + shared/app.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/server/middleware/initApp.js b/server/middleware/initApp.js index 7cb963b3..c094265f 100644 --- a/server/middleware/initApp.js +++ b/server/middleware/initApp.js @@ -24,6 +24,7 @@ module.exports = function(appAttributes, options) { * This will only be accessible on the server. */ req: req, + res: res, entryPath: options.entryPath, modelUtils: options.modelUtils }; diff --git a/shared/app.js b/shared/app.js index 99cca5fa..d02b7cfb 100644 --- a/shared/app.js +++ b/shared/app.js @@ -44,6 +44,10 @@ module.exports = Backbone.Model.extend({ this.req = this.options.req; } + if (this.options.res) { + this.res = this.options.res; + } + /** * Initialize the `templateAdapter`, allowing application developers to use whichever * templating system they want. From 934ad3a8028c735ff9d69f26b8fbc5e71c07043a Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:29:23 +0100 Subject: [PATCH 6/8] use the apiProxy proxyRequest instead of the data adapter --- shared/syncer.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shared/syncer.js b/shared/syncer.js index 0a3e2838..d51196ee 100644 --- a/shared/syncer.js +++ b/shared/syncer.js @@ -58,7 +58,7 @@ function clientSync(method, model, options) { } function serverSync(method, model, options) { - var api, data, urlParts, verb, req; + var api, data, urlParts, verb, req, res; data = _.clone(options.data); data = addApiParams(method, model, data); @@ -66,6 +66,7 @@ function serverSync(method, model, options) { verb = methodMap[method]; urlParts = options.url.split('?'); req = this.app.req; + res = this.app.res; api = { method: verb, @@ -85,7 +86,7 @@ function serverSync(method, model, options) { _.extend(api.query, data); } - req.dataAdapter.request(req, api, function(err, response, body) { + this.app.proxyRequest(req, res, api, function(err, response, body) { var resp; if (err) { resp = { From b9e63e44a6d3468d1822e94ebe79cbd9082563d4 Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 13:39:13 +0100 Subject: [PATCH 7/8] removed data adapter from the req object the syncer now uses the apiProx instead of the data adapter --- server/server.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/server/server.js b/server/server.js index 709164a8..a47fdb49 100644 --- a/server/server.js +++ b/server/server.js @@ -88,24 +88,6 @@ Server.prototype.configure = function(fn) { this._configured = true; - /** - * Attach the `dataAdapter` to the `req` so that the `syncer` can access it. - */ - this.expressApp.use(function(req, res, next) { - req.dataAdapter = dataAdapter; - - /** - * Proxy `res.end` so we can remove the reference to `dataAdapter` to prevent leaks. - */ - var end = res.end; - res.end = function(data, encoding) { - res.end = end; - req.dataAdapter = null; - res.end(data, encoding); - }; - next(); - }); - /** * Initialize the Rendr app, accessible at `req.rendrApp`. */ From f0827cbe508a3f6d9a6954be76d9a6c030033abf Mon Sep 17 00:00:00 2001 From: Alexander Schmidt Date: Fri, 10 Jan 2014 16:05:39 +0100 Subject: [PATCH 8/8] convertError option is now only set to false in the apiProxyMiddleware --- server/middleware/apiProxy.js | 6 +++--- shared/syncer.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/middleware/apiProxy.js b/server/middleware/apiProxy.js index 8b356fca..12a2a249 100644 --- a/server/middleware/apiProxy.js +++ b/server/middleware/apiProxy.js @@ -21,7 +21,7 @@ function apiProxy(dataAdapter) { api.path = apiProxy.getApiPath(req.path); api.api = apiProxy.getApiName(req.path); - middleware.proxyRequest(req, res, api, function(err, response, body) { + middleware.proxyRequest(req, res, api, { convertErrorCode: false }, function(err, response, body) { if (err) return next(err); // Pass through statusCode. @@ -30,11 +30,11 @@ function apiProxy(dataAdapter) { }); }; - middleware.proxyRequest = function proxyRequest(req, res, api, callback) { + middleware.proxyRequest = function proxyRequest(req, res, api, options, callback) { api.headers = api.headers || {}; api.headers['x-forwarded-for'] = apiProxy.getXForwardedForHeader(req.headers, req.ip); - dataAdapter.request(req, api, { convertErrorCode: false }, callback); + dataAdapter.request(req, api, options, callback); }; return middleware; diff --git a/shared/syncer.js b/shared/syncer.js index d51196ee..83a8aa12 100644 --- a/shared/syncer.js +++ b/shared/syncer.js @@ -86,7 +86,7 @@ function serverSync(method, model, options) { _.extend(api.query, data); } - this.app.proxyRequest(req, res, api, function(err, response, body) { + this.app.proxyRequest(req, res, api, {}, function(err, response, body) { var resp; if (err) { resp = {