diff --git a/lib/api/map/anonymous-map-controller.js b/lib/api/map/anonymous-map-controller.js index 135da65f..46ed876a 100644 --- a/lib/api/map/anonymous-map-controller.js +++ b/lib/api/map/anonymous-map-controller.js @@ -87,9 +87,6 @@ module.exports = class AnonymousMapController { from: { req: { query: { client: 'client' } - }, - res: { - body: { layergroupid: 'map_id' } } } }; @@ -230,6 +227,7 @@ function createLayergroup (mapBackend, userLimitsBackend, pgConnection, affected ); res.locals.mapConfig = mapConfig; + res.locals.mapConfigProvider = mapConfigProvider; res.locals.analysesResults = context.analysesResults; const mapParams = { dbuser, dbname, dbpassword, dbhost, dbport }; @@ -243,7 +241,6 @@ function createLayergroup (mapBackend, userLimitsBackend, pgConnection, affected res.statusCode = 200; res.body = layergroup; - res.locals.mapConfigProvider = mapConfigProvider; next(); }); diff --git a/lib/api/middlewares/last-modified-header.js b/lib/api/middlewares/last-modified-header.js index 4f00c0d2..4afd283f 100644 --- a/lib/api/middlewares/last-modified-header.js +++ b/lib/api/middlewares/last-modified-header.js @@ -36,6 +36,8 @@ module.exports = function setLastModifiedHeader () { res.set('Last-Modified', lastModifiedDate.toUTCString()); + res.locals.cache_buster = lastUpdatedAt; + next(); }); }; diff --git a/lib/api/middlewares/last-updated-time-layergroup.js b/lib/api/middlewares/last-updated-time-layergroup.js index a1c4593a..1c4a3cab 100644 --- a/lib/api/middlewares/last-updated-time-layergroup.js +++ b/lib/api/middlewares/last-updated-time-layergroup.js @@ -22,6 +22,8 @@ module.exports = function setLastUpdatedTimeToLayergroup () { layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + res.locals.cache_buster = lastUpdateTime; + next(); }); }; diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index 82924f47..898c2069 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -3,7 +3,7 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -module.exports = function metrics ({ enabled, metricsBackend, logger, tags }) { +module.exports = function metrics ({ enabled, tags, metricsBackend, logger }) { if (!enabled) { return function metricsDisabledMiddleware (req, res, next) { next(); @@ -47,6 +47,8 @@ function getEventData (req, res, tags) { event_time: new Date().toISOString(), user_id: res.locals.userId, user_agent: req.get('User-Agent'), + map_id: getLayergroupid({ res }), + cache_buster: getCacheBuster({ res }), response_code: res.statusCode.toString(), response_time: getResponseTime(res), source_domain: req.hostname, @@ -67,6 +69,30 @@ function normalizedField (field) { return field.toString().trim().substr(0, MAX_LENGTH); } +function getLayergroupid ({ res }) { + if (res.locals.token) { + return res.locals.token; + } + + if (res.locals.mapConfig) { + return res.locals.mapConfig.id(); + } + + if (res.locals.mapConfigProvider && res.locals.mapConfigProvider.mapConfig) { + return res.locals.mapConfigProvider.mapConfig.id(); + } +} + +function getCacheBuster ({ res }) { + if (res.locals.cache_buster !== undefined) { + return `${res.locals.cache_buster}`; + } + + if (res.locals.mapConfigProvider) { + return `${res.locals.mapConfigProvider.getCacheBuster()}`; + } +} + // FIXME: 'X-Tiler-Profiler' might not be accurate enough function getResponseTime (res) { const profiler = res.get('X-Tiler-Profiler'); diff --git a/lib/api/template/named-template-controller.js b/lib/api/template/named-template-controller.js index b1fd7348..88270b89 100644 --- a/lib/api/template/named-template-controller.js +++ b/lib/api/template/named-template-controller.js @@ -85,9 +85,6 @@ module.exports = class NamedMapController { from: { req: { query: { client: 'client' } - }, - res: { - body: { layergroupid: 'map_id' } } } }; diff --git a/lib/models/mapconfig/provider/named-map-provider.js b/lib/models/mapconfig/provider/named-map-provider.js index d0823f89..0818eef2 100644 --- a/lib/models/mapconfig/provider/named-map-provider.js +++ b/lib/models/mapconfig/provider/named-map-provider.js @@ -40,6 +40,10 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { this.authToken = authToken; this.params = params; + // FIXME: why is this different thah that: + // this.cacheBuster = params.cache_buster || 0; + // test: "should fail to use template from named map provider after template deletion" + // check named-map-provider-cache invalidation this.cacheBuster = Date.now(); // use template after call to mapConfig diff --git a/test/index.js b/test/index.js index 84f43989..d1e27973 100644 --- a/test/index.js +++ b/test/index.js @@ -77,6 +77,8 @@ async function populateDatabase () { async function populateRedis () { const commands = ` + FLUSHALL + HMSET rails:users:localhost \ id ${TEST_USER_ID} \ database_name "${TEST_DB}" \ diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 147224aa..56ff4f7f 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -3,6 +3,7 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); const MetricsBackend = require('../../lib/backends/metrics'); +const LayergroupToken = require('../../lib/models/layergroup-token'); const apikey = 1234; const mapConfig = { version: '1.8.0', @@ -48,8 +49,9 @@ describe('metrics middleware', function () { }; }); - afterEach(function () { + afterEach(function (done) { MetricsBackend.prototype.send = this.originalMetricsBackendSendMethod; + return this.testClient.drain(done); }); it('should not send event if not enabled', function (done) { @@ -59,9 +61,10 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': '1' }; const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } @@ -69,16 +72,17 @@ describe('metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - return testClient.drain(done); + return done(); }); }); it('should not send event if headers not present', function (done) { const extraHeaders = {}; const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } @@ -86,7 +90,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - return testClient.drain(done); + return done(); }); }); @@ -103,14 +107,18 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -118,8 +126,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -137,14 +147,18 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': 1 }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -152,8 +166,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -181,10 +197,12 @@ describe('metrics middleware', function () { } ] }; - const testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + const params = { response: { status: 400 } }; - testClient.getLayergroup(params, (err, body) => { + this.testClient.getLayergroup(params, (err, body) => { if (err) { return done(err); } @@ -196,8 +214,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -212,9 +232,10 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getTile(0, 0, 0, (err, res, tile) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getTile(0, 0, 0, (err, res, tile) => { if (err) { return done(err); } @@ -224,8 +245,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -240,7 +263,8 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); const params = { response: { @@ -251,7 +275,7 @@ describe('metrics middleware', function () { } }; - testClient.getTile(0, 0, 2, params, (err, res, tile) => { + this.testClient.getTile(0, 0, 2, params, (err, res, tile) => { if (err) { return done(err); } @@ -261,8 +285,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -280,14 +306,18 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'map' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -295,8 +325,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -330,7 +362,7 @@ describe('metrics middleware', function () { } }; - const testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); const params = { response: { @@ -340,7 +372,8 @@ describe('metrics middleware', function () { } } }; - testClient.getLayergroup(params, (err, body) => { + + this.testClient.getLayergroup(params, (err, body) => { if (err) { return done(err); } @@ -352,8 +385,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -369,9 +404,10 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'tile' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getTile(0, 0, 0, (err, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getTile(0, 0, 0, (err, body) => { if (err) { return done(err); } @@ -381,8 +417,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -400,9 +438,10 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'preview' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getPreview(640, 480, {}, (err, res, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getPreview(640, 480, {}, (err, res, body) => { if (err) { return done(err); } @@ -414,8 +453,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -433,7 +474,9 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'preview-errored' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + const widthTooLarge = 8193; const params = { response: { @@ -444,7 +487,7 @@ describe('metrics middleware', function () { } }; - testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { + this.testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { if (err) { return done(err); } @@ -455,8 +498,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); });