From a4a100e65a23207309e8c67471735ab46604491e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=CC=81lvaro=20Manera?= Date: Mon, 22 Jun 2020 09:47:22 +0200 Subject: [PATCH 1/5] respond with user-id and client in the headers added headers to the test-client callback to be able to check them --- lib/api/middlewares/client-header.js | 13 ++++++++ lib/api/middlewares/user.js | 1 + lib/api/sql/query-controller.js | 2 ++ test/acceptance/client-headers-test.js | 43 ++++++++++++++++++++++++++ test/support/test-client.js | 10 ++++-- 5 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 lib/api/middlewares/client-header.js create mode 100644 test/acceptance/client-headers-test.js diff --git a/lib/api/middlewares/client-header.js b/lib/api/middlewares/client-header.js new file mode 100644 index 00000000..de750aed --- /dev/null +++ b/lib/api/middlewares/client-header.js @@ -0,0 +1,13 @@ +'use strict'; + +module.exports = function clientHeader () { + return function clientHeaderMiddleware (req, res, next) { + const { client } = req.query; + + if (client) { + res.set('Carto-Client', client); + } + + return next(); + }; +}; diff --git a/lib/api/middlewares/user.js b/lib/api/middlewares/user.js index ccd1f40c..d39e2d5b 100644 --- a/lib/api/middlewares/user.js +++ b/lib/api/middlewares/user.js @@ -19,6 +19,7 @@ module.exports = function user (metadataBackend) { } res.locals.userId = userId; + res.set('Carto-User-Id', `${userId}`); return next(); }); }; diff --git a/lib/api/sql/query-controller.js b/lib/api/sql/query-controller.js index f9a8247a..1eb21a61 100644 --- a/lib/api/sql/query-controller.js +++ b/lib/api/sql/query-controller.js @@ -19,6 +19,7 @@ const surrogateKey = require('../middlewares/surrogate-key'); const lastModified = require('../middlewares/last-modified'); const formatter = require('../middlewares/formatter'); const content = require('../middlewares/content'); +const clientHeader = require('../middlewares/client-header'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimits; const PSQL = require('cartodb-psql'); @@ -55,6 +56,7 @@ module.exports = class QueryController { lastModified(), formatter(), content(), + clientHeader(), handleQuery({ stats: this.stats }) ]; }; diff --git a/test/acceptance/client-headers-test.js b/test/acceptance/client-headers-test.js new file mode 100644 index 00000000..1684c3ec --- /dev/null +++ b/test/acceptance/client-headers-test.js @@ -0,0 +1,43 @@ +'use strict'; + +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); + +describe('SQL api metric headers', function () { + const publicSQL = 'select * from untitle_table_4'; + + it('should get client header if client param is present', function (done) { + this.testClient = new TestClient(); + const params = { client: 'test' }; + + this.testClient.getResult(publicSQL, params, (err, result, headers) => { + assert.ifError(err); + assert.strictEqual(result.length, 6); + assert.strictEqual(headers['carto-client'], 'test'); + done(); + }); + }); + + it('should not get the client header if no client is provided', function (done) { + this.testClient = new TestClient(); + + this.testClient.getResult(publicSQL, (err, result, headers) => { + assert.ifError(err); + assert.strictEqual(result.length, 6); + assert.strictEqual(headers['carto-client'], undefined); + done(); + }); + }); + + it('should not get the user id in the response header', function (done) { + this.testClient = new TestClient(); + + this.testClient.getResult(publicSQL, (err, result, headers) => { + assert.ifError(err); + assert.strictEqual(result.length, 6); + assert.strictEqual(headers['carto-user-id'], '1'); + done(); + }); + }); + +}); diff --git a/test/support/test-client.js b/test/support/test-client.js index db9ecd7e..52623a5e 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -56,10 +56,10 @@ TestClient.prototype.getResult = function (query, override, callback) { var result = JSON.parse(res.body); if (res.statusCode > 299) { - return callback(null, result); + return callback(null, result, res.headers); } - return callback(null, result.rows || [], result); + return callback(null, result.rows, res.headers || [], result, res.headers); } ); }; @@ -89,7 +89,11 @@ TestClient.prototype.getUrl = function (override) { return '/api/v1/sql?'; } - return '/api/v2/sql?api_key=' + (override.apiKey || this.config.apiKey || '1234'); + let url = '/api/v2/sql?api_key=' + (override.apiKey || this.config.apiKey || '1234'); + if (override.client) { + url = url + '&client=' + override.client; + } + return url; }; TestClient.prototype.getExpectedResponse = function (override) { From 2e914d5454a3e1ab4b6074e1aa1c3c355bd29d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=CC=81lvaro=20Manera?= Date: Mon, 22 Jun 2020 09:56:48 +0200 Subject: [PATCH 2/5] fix lint --- test/acceptance/client-headers-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/acceptance/client-headers-test.js b/test/acceptance/client-headers-test.js index 1684c3ec..1d7c42cc 100644 --- a/test/acceptance/client-headers-test.js +++ b/test/acceptance/client-headers-test.js @@ -21,7 +21,7 @@ describe('SQL api metric headers', function () { it('should not get the client header if no client is provided', function (done) { this.testClient = new TestClient(); - this.testClient.getResult(publicSQL, (err, result, headers) => { + this.testClient.getResult(publicSQL, (err, result, headers) => { assert.ifError(err); assert.strictEqual(result.length, 6); assert.strictEqual(headers['carto-client'], undefined); @@ -39,5 +39,4 @@ describe('SQL api metric headers', function () { done(); }); }); - }); From 598e7615a5d3ce84fe508464af40b68a69cf7b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=CC=81lvaro=20Manera?= Date: Mon, 22 Jun 2020 12:50:48 +0200 Subject: [PATCH 3/5] missing return on error case --- lib/api/middlewares/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/middlewares/user.js b/lib/api/middlewares/user.js index d39e2d5b..f25df538 100644 --- a/lib/api/middlewares/user.js +++ b/lib/api/middlewares/user.js @@ -15,7 +15,7 @@ module.exports = function user (metadataBackend) { error.subtype = 'user-not-found'; error.http_status = 404; error.message = errorUserNotFoundMessageTemplate(res.locals.user); - next(error); + return next(error); } res.locals.userId = userId; From 0046480231527620b5eae123f2eea4e4b8ab7685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=CC=81lvaro=20Manera?= Date: Mon, 22 Jun 2020 12:51:35 +0200 Subject: [PATCH 4/5] move middleware to sql-router --- lib/api/sql/query-controller.js | 2 -- lib/api/sql/sql-router.js | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/sql/query-controller.js b/lib/api/sql/query-controller.js index 1eb21a61..f9a8247a 100644 --- a/lib/api/sql/query-controller.js +++ b/lib/api/sql/query-controller.js @@ -19,7 +19,6 @@ const surrogateKey = require('../middlewares/surrogate-key'); const lastModified = require('../middlewares/last-modified'); const formatter = require('../middlewares/formatter'); const content = require('../middlewares/content'); -const clientHeader = require('../middlewares/client-header'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimits; const PSQL = require('cartodb-psql'); @@ -56,7 +55,6 @@ module.exports = class QueryController { lastModified(), formatter(), content(), - clientHeader(), handleQuery({ stats: this.stats }) ]; }; diff --git a/lib/api/sql/sql-router.js b/lib/api/sql/sql-router.js index b0b4397e..2053d3de 100644 --- a/lib/api/sql/sql-router.js +++ b/lib/api/sql/sql-router.js @@ -10,6 +10,7 @@ const logger = require('../middlewares/logger'); const profiler = require('../middlewares/profiler'); const cors = require('../middlewares/cors'); const servedByHostHeader = require('../middlewares/served-by-host-header'); +const clientHeader = require('../middlewares/client-header'); const QueryController = require('./query-controller'); const CopyController = require('./copy-controller'); @@ -61,6 +62,7 @@ module.exports = class SqlRouter { sqlRouter.use(logger()); sqlRouter.use(profiler({ statsClient: this.statsClient })); sqlRouter.use(cors()); + sqlRouter.use(clientHeader()); sqlRouter.use(servedByHostHeader()); this.queryController.route(sqlRouter); From 3994deda1f72482a0ba89a0520c00f058d25f724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=CC=81lvaro=20Manera?= Date: Mon, 22 Jun 2020 12:51:45 +0200 Subject: [PATCH 5/5] change test description --- test/acceptance/client-headers-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/client-headers-test.js b/test/acceptance/client-headers-test.js index 1d7c42cc..c175b715 100644 --- a/test/acceptance/client-headers-test.js +++ b/test/acceptance/client-headers-test.js @@ -29,7 +29,7 @@ describe('SQL api metric headers', function () { }); }); - it('should not get the user id in the response header', function (done) { + it('should get the user id in the response header', function (done) { this.testClient = new TestClient(); this.testClient.getResult(publicSQL, (err, result, headers) => {