From ddefb1a6ca3a1a3c9ecfb82f0664b2e863ec6d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 13:15:35 +0200 Subject: [PATCH 1/6] Add 'Carto-Stat-Tag', 'Carto-User-Id', and 'Carto-Client' headers --- lib/api/api-router.js | 2 ++ lib/api/map/preview-template-controller.js | 2 ++ lib/api/middlewares/increment-map-view-count.js | 5 ++++- lib/api/middlewares/user.js | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/api/api-router.js b/lib/api/api-router.js index ab2bf332..466a6b01 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -57,6 +57,7 @@ const user = require('./middlewares/user'); const sendResponse = require('./middlewares/send-response'); const syntaxError = require('./middlewares/syntax-error'); const errorMiddleware = require('./middlewares/error-middleware'); +const clientHeader = require('./middlewares/client-header'); const MapRouter = require('./map/map-router'); const TemplateRouter = require('./template/template-router'); @@ -208,6 +209,7 @@ module.exports = class ApiRouter { apiRouter.use(initializeStatusCode()); apiRouter.use(bodyParser.json()); apiRouter.use(servedByHostHeader()); + apiRouter.use(clientHeader()); apiRouter.use(stats({ enabled: this.serverOptions.useProfiler, statsClient: global.statsClient diff --git a/lib/api/map/preview-template-controller.js b/lib/api/map/preview-template-controller.js index d9a64707..10a40b93 100644 --- a/lib/api/map/preview-template-controller.js +++ b/lib/api/map/preview-template-controller.js @@ -353,6 +353,8 @@ function incrementMapViews ({ metadataBackend }) { const statTag = mapConfig.obj().stat_tag; + res.set('Carto-Stat-Tag', `${statTag}`); + metadataBackend.incMapviewCount(user, statTag, (err) => { if (err) { global.logger.log(incrementMapViewsError({ user, err })); diff --git a/lib/api/middlewares/increment-map-view-count.js b/lib/api/middlewares/increment-map-view-count.js index 00bb4673..20ddcf42 100644 --- a/lib/api/middlewares/increment-map-view-count.js +++ b/lib/api/middlewares/increment-map-view-count.js @@ -3,9 +3,12 @@ module.exports = function incrementMapViewCount (metadataBackend) { return function incrementMapViewCountMiddleware (req, res, next) { const { mapConfig, user } = res.locals; + const statTag = mapConfig.obj().stat_tag; + + res.set('Carto-Stat-Tag', `${statTag}`); // Error won't blow up, just be logged. - metadataBackend.incMapviewCount(user, mapConfig.obj().stat_tag, (err) => { + metadataBackend.incMapviewCount(user, statTag, (err) => { req.profiler.done('incMapviewCount'); if (err) { diff --git a/lib/api/middlewares/user.js b/lib/api/middlewares/user.js index 57c9b45d..ce8c73bf 100644 --- a/lib/api/middlewares/user.js +++ b/lib/api/middlewares/user.js @@ -14,6 +14,7 @@ module.exports = function user (metadataBackend) { } res.locals.userId = userId; + res.set('Carto-User-Id', `${userId}`); return next(); }); }; From aff5c9a614bf5dce1055802da7bbda47fe0dd357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 16:28:44 +0200 Subject: [PATCH 2/6] Add test to check the headers exist while instantiating a map --- lib/api/middlewares/client-header.js | 11 +++ test/acceptance/map-view-headers-test.js | 97 ++++++++++++++++++++++++ test/support/test-client.js | 13 +++- 3 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 lib/api/middlewares/client-header.js create mode 100644 test/acceptance/map-view-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..dfcf488a --- /dev/null +++ b/lib/api/middlewares/client-header.js @@ -0,0 +1,11 @@ +'use strict'; + +module.exports = function clientHeader () { + return function clientHeaderMiddleware (req, res, next) { + const { client } = req.query; + + res.set('Carto-Client', client); + + return next(); + }; +}; diff --git a/test/acceptance/map-view-headers-test.js b/test/acceptance/map-view-headers-test.js new file mode 100644 index 00000000..858f7715 --- /dev/null +++ b/test/acceptance/map-view-headers-test.js @@ -0,0 +1,97 @@ +'use strict'; + +require('../support/test-helper'); + +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); + +const defaultLayers = [{ + type: 'cartodb', + options: { + sql: TestClient.SQL.ONE_POINT + } +}]; +const defaultStatTag = 'wadus'; + +function createMapConfig (layers = defaultLayers, statTag = defaultStatTag) { + return { + version: '1.8.0', + layers: layers, + stat_tag: defaultStatTag + }; +} + +describe('map view headers', function () { + it('anonymous map instantiation should respond with map-view headers', function (done) { + const mapConfig = createMapConfig(); + const testClient = new TestClient(mapConfig); + const params = { client: 'test' }; + + testClient.getLayergroup(params, (err, body, res) => { + if (err) { + return done(err); + } + + assert.strictEqual(res.headers['carto-stat-tag'], defaultStatTag); + assert.strictEqual(res.headers['carto-client'], params.client); + assert.strictEqual(res.headers['carto-user-id'], '1'); + + testClient.drain(done); + }); + }); + + it('named map instantiation should respond with map-view headers', function (done) { + const templateid = `map-view-headers-test-${Date.now()}`; + const template = { + version: '0.0.1', + name: templateid, + layergroup: createMapConfig() + }; + + const testClient = new TestClient(template, 1234); + const params = { client: 'test' }; + + testClient.getLayergroup(params, (err, body, res) => { + if (err) { + return done(err); + } + + assert.strictEqual(res.headers['carto-stat-tag'], defaultStatTag); + assert.strictEqual(res.headers['carto-client'], params.client); + assert.strictEqual(res.headers['carto-user-id'], '1'); + + testClient.drain(done); + }); + }); + + it('preview should respond with map-view headers', function (done) { + const templateid = `map-view-headers-test-${Date.now()}`; + const template = { + version: '0.0.1', + name: templateid, + layergroup: createMapConfig([{ + type: 'cartodb', + options: { + sql: TestClient.SQL.ONE_POINT, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + }]) + }; + + const testClient = new TestClient(template, 1234); + const params = { client: 'test' }; + + testClient.getPreview(640, 480, params, (err, res) => { + if (err) { + return done(err); + } + + assert.strictEqual(res.headers['carto-stat-tag'], defaultStatTag); + assert.strictEqual(res.headers['carto-client'], params.client); + assert.strictEqual(res.headers['carto-user-id'], '1'); + + testClient.drain(done); + }); + }); +}); diff --git a/test/support/test-client.js b/test/support/test-client.js index 36183a0d..c3204249 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1033,9 +1033,13 @@ TestClient.prototype.getLayergroup = function (params, callback) { queryParams.aggregation = params.aggregation; } + if (params.client !== undefined) { + queryParams.client = params.client; + } + const path = templateId - ? urlNamed + '/' + templateId + '?' + qs.stringify(queryParams) - : url; + ? `${urlNamed}/${templateId}?${qs.stringify(queryParams)}` + : `${url}?${qs.stringify(queryParams)}`; assert.response(self.server, { @@ -1058,12 +1062,15 @@ TestClient.prototype.getLayergroup = function (params, callback) { if (res.statusCode === 200 && self.template && self.template.layergroup && self.template.layergroup.stat_tag) { self.keysToDelete[`user:localhost:mapviews:stat_tag:${self.template.layergroup.stat_tag}`] = 5; } + if (res.statusCode === 200 && self.mapConfig && self.mapConfig.stat_tag) { + self.keysToDelete[`user:localhost:mapviews:stat_tag:${self.mapConfig.stat_tag}`] = 5; + } } if (err) { return callback(err); } - return callback(null, parsedBody); + return callback(null, parsedBody, res); } ); } From f82232194c6320bdc907d8ec6e669b686930ba4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 16:31:53 +0200 Subject: [PATCH 3/6] Under if --- lib/api/middlewares/client-header.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/api/middlewares/client-header.js b/lib/api/middlewares/client-header.js index dfcf488a..de750aed 100644 --- a/lib/api/middlewares/client-header.js +++ b/lib/api/middlewares/client-header.js @@ -4,7 +4,9 @@ module.exports = function clientHeader () { return function clientHeaderMiddleware (req, res, next) { const { client } = req.query; - res.set('Carto-Client', client); + if (client) { + res.set('Carto-Client', client); + } return next(); }; From b97aeda53c2db65d4e5cc3a6a6259474b17dfec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 16:52:13 +0200 Subject: [PATCH 4/6] Adapt test-client to handle client query param --- test/support/test-client.js | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index c3204249..96a61da1 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -942,24 +942,8 @@ TestClient.prototype.getLayergroup = function (params, callback) { params = {}; } - let url = '/api/v1/map'; - const urlNamed = url + '/named'; const headers = Object.assign({ host: 'localhost', 'Content-Type': 'application/json' }, self.extraHeaders); - const queryParams = {}; - - if (self.apiKey) { - queryParams.api_key = self.apiKey; - } - - if (params.aggregation !== undefined) { - queryParams.aggregation = params.aggregation; - } - - if (Object.keys(queryParams).length) { - url += '?' + qs.stringify(queryParams); - } - var layergroupId; if (params.layergroupid) { @@ -982,7 +966,7 @@ TestClient.prototype.getLayergroup = function (params, callback) { assert.response(self.server, { - url: urlNamed + '?' + qs.stringify({ api_key: self.apiKey }), + url: `/api/v1/map/named?${qs.stringify({ api_key: self.apiKey })}`, method: 'POST', headers, data: JSON.stringify(self.template) @@ -1023,9 +1007,10 @@ TestClient.prototype.getLayergroup = function (params, callback) { }; } + const url = '/api/v1/map'; const queryParams = {}; - if (self.apiKey) { + if (self.apiKey !== undefined) { queryParams.api_key = self.apiKey; } @@ -1038,8 +1023,8 @@ TestClient.prototype.getLayergroup = function (params, callback) { } const path = templateId - ? `${urlNamed}/${templateId}?${qs.stringify(queryParams)}` - : `${url}?${qs.stringify(queryParams)}`; + ? `${url}/named/${templateId}${Object.keys(queryParams).length ? `?${qs.stringify(queryParams)}` : ''}` + : `${url}${Object.keys(queryParams).length ? `?${qs.stringify(queryParams)}` : ''}`; assert.response(self.server, { From 0090811510a1364f722fd75549df3cc70dd181ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 16:56:50 +0200 Subject: [PATCH 5/6] Typo --- test/acceptance/layergroup-metadata-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/layergroup-metadata-test.js b/test/acceptance/layergroup-metadata-test.js index 1543f230..d20fc0a5 100644 --- a/test/acceptance/layergroup-metadata-test.js +++ b/test/acceptance/layergroup-metadata-test.js @@ -17,7 +17,7 @@ describe('layergroup metadata', function () { serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; }); - [1234, 'default_public', false].forEach(apiKey => { + [1234, 'default_public', undefined].forEach(apiKey => { it(`tiles base urls ${apiKey ? `with api key: ${apiKey}` : 'without api key'}`, function (done) { const mapConfig = { version: '1.7.0', From f31e8b43b64fac372c5661df82d51c9e9945d75a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 26 May 2020 17:03:53 +0200 Subject: [PATCH 6/6] Duplicate --- test/support/test-client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 96a61da1..45fb198e 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1022,9 +1022,10 @@ TestClient.prototype.getLayergroup = function (params, callback) { queryParams.client = params.client; } + const query = Object.keys(queryParams).length ? `?${qs.stringify(queryParams)}` : ''; const path = templateId - ? `${url}/named/${templateId}${Object.keys(queryParams).length ? `?${qs.stringify(queryParams)}` : ''}` - : `${url}${Object.keys(queryParams).length ? `?${qs.stringify(queryParams)}` : ''}`; + ? `${url}/named/${templateId}${query}` + : `${url}${query}`; assert.response(self.server, {