From 6cdb872bb581772d27f86e9acfc7e6bc7c3d2ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 8 Jul 2019 17:55:04 +0200 Subject: [PATCH 1/7] Going red: add test to check the bad behavour of max-age directive when an analysis table is in affected tables of the map --- package-lock.json | 51 ++++++++--------- package.json | 2 +- test/acceptance/cache/cache-control-header.js | 55 ++++++++++++++++++- 3 files changed, 79 insertions(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1339f77c..7babced5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32,7 +32,7 @@ "resolved": "https://registry.npmjs.org/@carto/mapnik/-/mapnik-3.6.2-carto.15.tgz", "integrity": "sha512-O6YpFQ8NRJJZjiQJxoS92Hpjci1tvd4SWKxaicG3SuRBqVGY3BKdhChL+6Ck6tRZvW3O577987vVwEnYKALTCA==", "requires": { - "mapnik-vector-tile": "github:cartodb/mapnik-vector-tile#e7ca5471f9e5de81243e6035e70444321fc0a82f", + "mapnik-vector-tile": "github:cartodb/mapnik-vector-tile#v1.6.1-carto.2", "nan": "2.14.0", "node-pre-gyp": "0.13.0" }, @@ -389,7 +389,7 @@ }, "carto": { "version": "github:cartodb/carto#85881d99dd7fcf2c4e16478b04db67108d27a50c", - "from": "github:cartodb/carto#0.15.1-cdb5", + "from": "github:cartodb/carto#master", "requires": { "mapnik-reference": "~6.0.2", "optimist": "~0.6.0", @@ -417,14 +417,13 @@ "integrity": "sha512-1z3Dk9G8KQlNGurbcmGBvNj8DVCh1Keue9uzyyvB6hKOYzBHMxixAMG0D+8nSsA7oQmWUsx/xkZZ5ZxT9toEHA==", "requires": { "debug": "^3.1.0", - "pg": "github:cartodb/node-postgres#5417d7b29b7272ca2e71bb396899ab3f177a9ae6", + "pg": "github:cartodb/node-postgres#6.4.2-cdb2", "underscore": "~1.6.0" } }, "cartodb-query-tables": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/cartodb-query-tables/-/cartodb-query-tables-0.4.0.tgz", - "integrity": "sha512-abhhz2juO1IO9hZvv10WAtgt77UTk2Fc/6BK7pml+0Cnd0HMfDC+XKzfY+r/wEjyEa20IXTc//cPsP0BgevyMw==" + "version": "github:cartodb/node-cartodb-query-tables#7a4e48ce7e1cc7dd8e17b4faec095efa660e6bf1", + "from": "github:cartodb/node-cartodb-query-tables#skip-analysis-tables" }, "cartodb-redis": { "version": "2.1.0", @@ -2044,7 +2043,7 @@ "carto": "0.16.3", "debug": "~3.1.0", "generic-pool": "~2.2.0", - "millstone": "github:cartodb/millstone#f201885250d2d7ea6c870a1e1060ca6bcfdef186", + "millstone": "github:cartodb/millstone#v0.6.17-carto.2", "postcss": "~5.2.8", "postcss-scss": "0.4.0", "postcss-strip-inline-comments": "0.1.5", @@ -3925,26 +3924,10 @@ "resolved": "https://registry.npmjs.org/torque.js/-/torque.js-3.1.1.tgz", "integrity": "sha512-kfIrmI7TGqJT/J9DH8Mgvd9VEwcvAtnvyYyqymSN6WZ5L4BaVQEQ+zu5FgLChNAqCaRkqGc7bKp0Hj9A0rempA==", "requires": { - "carto": "github:cartodb/carto#85881d99dd7fcf2c4e16478b04db67108d27a50c", + "carto": "github:cartodb/carto#master", "d3": "3.5.17", "turbo-carto": "^0.21.1", "turf-jenks": "~1.0.1" - }, - "dependencies": { - "carto": { - "version": "github:cartodb/carto#85881d99dd7fcf2c4e16478b04db67108d27a50c", - "from": "github:cartodb/carto#master", - "requires": { - "mapnik-reference": "~6.0.2", - "optimist": "~0.6.0", - "underscore": "1.8.3" - } - }, - "underscore": { - "version": "1.8.3", - "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.8.3.tgz", - "integrity": "sha1-Tz+1OxBuYJf8+ctBCfKl6b36UCI=" - } } }, "tough-cookie": { @@ -4116,7 +4099,7 @@ "@carto/cartonik": "^0.6.0", "@carto/mapnik": "3.6.2-carto.15", "canvas": "^2.4.1", - "carto": "github:cartodb/carto#85881d99dd7fcf2c4e16478b04db67108d27a50c", + "carto": "github:cartodb/carto#0.15.1-cdb5", "cartodb-psql": "0.13.1", "debug": "3.1.0", "grainstore": "^2.0.0", @@ -4126,6 +4109,24 @@ "semver": "5.5.0", "torque.js": "3.1.1", "underscore": "1.6.0" + }, + "dependencies": { + "carto": { + "version": "github:cartodb/carto#85881d99dd7fcf2c4e16478b04db67108d27a50c", + "from": "github:cartodb/carto#0.15.1-cdb5", + "requires": { + "mapnik-reference": "~6.0.2", + "optimist": "~0.6.0", + "underscore": "1.8.3" + }, + "dependencies": { + "underscore": { + "version": "1.8.3", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.8.3.tgz", + "integrity": "sha1-Tz+1OxBuYJf8+ctBCfKl6b36UCI=" + } + } + } } }, "wordwrap": { diff --git a/package.json b/package.json index 374b1923..084f8f0b 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "body-parser": "1.18.3", "camshaft": "^0.64.0", "cartodb-psql": "0.13.1", - "cartodb-query-tables": "0.4.0", + "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#skip-analysis-tables", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", diff --git a/test/acceptance/cache/cache-control-header.js b/test/acceptance/cache/cache-control-header.js index 6bd4e9e3..b962feb2 100644 --- a/test/acceptance/cache/cache-control-header.js +++ b/test/acceptance/cache/cache-control-header.js @@ -16,15 +16,23 @@ const defaultLayers = [{ cartocss_version: '2.3.0' } }]; +const defaultDatavies = {}; +const defaultAnalyses = []; -function createMapConfig (layers = defaultLayers) { +function createMapConfig ({ + layers = defaultLayers, + dataviews = defaultDatavies, + analyses = defaultAnalyses +} = {}) { return { version: '1.8.0', - layers: layers + layers: layers, + dataviews: dataviews || {}, + analyses: analyses || [] }; } -describe('cache-control header', function () { +describe.only('cache-control header', function () { describe('max-age directive', function () { it('tile from a table which is included in cdb_tablemetada', function (done) { const ttl = ONE_YEAR_IN_SECONDS; @@ -120,5 +128,46 @@ describe('cache-control header', function () { testClient.drain(done); }); }); + + it('tile from a cached analysis table which is not included in cdb_tablemetada', function (done) { + const ttl = ONE_YEAR_IN_SECONDS; + const mapConfig = createMapConfig({ + layers: [{ + type: 'cartodb', + options: { + source: { + id: 'HEAD' + }, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + }], + analyses: [{ + id: 'HEAD', + type: 'buffer', + params: { + source: { + id: 'source_1', + type: 'source', + params: { + query: 'select * from populated_places_simple_reduced' + } + }, + radius: 60000 + } + }] + }); + + const testClient = new TestClient(mapConfig, 1234); + + testClient.getTile(0, 0, 0, {}, function (err, res, layergroup) { + if (err) { + return done(err); + } + + assert.equal(res.headers['cache-control'], `public,max-age=${ttl}`); + testClient.drain(done); + }); + }); }); }); From a84184852e61a19f765d7c120f5a5505c4860883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 8 Jul 2019 18:05:30 +0200 Subject: [PATCH 2/7] Going green: skip analyses table while computing max-age directive --- lib/cartodb/api/middlewares/cache-control-header.js | 7 ++++++- test/acceptance/cache/cache-control-header.js | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/api/middlewares/cache-control-header.js b/lib/cartodb/api/middlewares/cache-control-header.js index c3c57ea0..03a23592 100644 --- a/lib/cartodb/api/middlewares/cache-control-header.js +++ b/lib/cartodb/api/middlewares/cache-control-header.js @@ -42,5 +42,10 @@ module.exports = function setCacheControlHeader ({ }; function everyAffectedTablesCanBeInvalidated (affectedTables) { - return affectedTables && affectedTables.getTables().every(table => table.updated_at !== null); + const skipNotUpdatedAtTables = false; + const skipAnalysisCachedTables = true; + + return affectedTables && + affectedTables.getTables(skipNotUpdatedAtTables, skipAnalysisCachedTables) + .every(table => table.updated_at !== null); } diff --git a/test/acceptance/cache/cache-control-header.js b/test/acceptance/cache/cache-control-header.js index b962feb2..102741b9 100644 --- a/test/acceptance/cache/cache-control-header.js +++ b/test/acceptance/cache/cache-control-header.js @@ -150,7 +150,7 @@ describe.only('cache-control header', function () { id: 'source_1', type: 'source', params: { - query: 'select * from populated_places_simple_reduced' + query: 'select * from test_table' } }, radius: 60000 From 45af291e6a0674f9a3d4eab8b6fbd7ac0995c0a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 8 Jul 2019 18:08:08 +0200 Subject: [PATCH 3/7] Remove test filter --- test/acceptance/cache/cache-control-header.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/cache/cache-control-header.js b/test/acceptance/cache/cache-control-header.js index 102741b9..0d6427b2 100644 --- a/test/acceptance/cache/cache-control-header.js +++ b/test/acceptance/cache/cache-control-header.js @@ -32,7 +32,7 @@ function createMapConfig ({ }; } -describe.only('cache-control header', function () { +describe('cache-control header', function () { describe('max-age directive', function () { it('tile from a table which is included in cdb_tablemetada', function (done) { const ttl = ONE_YEAR_IN_SECONDS; From c066e2c3cf620589b8dc720012f8fe167826196a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jul 2019 12:45:33 +0200 Subject: [PATCH 4/7] Update cartodb-query-tables to version 0.5.0 --- package-lock.json | 5 +++-- package.json | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7babced5..2b38e3d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -422,8 +422,9 @@ } }, "cartodb-query-tables": { - "version": "github:cartodb/node-cartodb-query-tables#7a4e48ce7e1cc7dd8e17b4faec095efa660e6bf1", - "from": "github:cartodb/node-cartodb-query-tables#skip-analysis-tables" + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/cartodb-query-tables/-/cartodb-query-tables-0.5.0.tgz", + "integrity": "sha512-xVHWKF6DMSn/Ksqhm6uNNnr8ckWDWWzsTz9XsoELAW5ZpCJDaxn3YGxgRtD1bwHdQd1tjDVeud/pRk/mCFWpmQ==" }, "cartodb-redis": { "version": "2.1.0", diff --git a/package.json b/package.json index 084f8f0b..f6582f01 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "body-parser": "1.18.3", "camshaft": "^0.64.0", "cartodb-psql": "0.13.1", - "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#skip-analysis-tables", + "cartodb-query-tables": "^0.5.0", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", From ef39f23d1f4f4d2148d02854e4f5c1d2c7e624b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jul 2019 14:47:40 +0200 Subject: [PATCH 5/7] Fix tests to use map-config builder properly --- test/acceptance/cache/cache-control-header.js | 74 ++++++++++--------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/test/acceptance/cache/cache-control-header.js b/test/acceptance/cache/cache-control-header.js index 0d6427b2..b2d6abae 100644 --- a/test/acceptance/cache/cache-control-header.js +++ b/test/acceptance/cache/cache-control-header.js @@ -36,14 +36,16 @@ describe('cache-control header', function () { describe('max-age directive', function () { it('tile from a table which is included in cdb_tablemetada', function (done) { const ttl = ONE_YEAR_IN_SECONDS; - const mapConfig = createMapConfig([{ - type: 'cartodb', - options: { - sql: 'select * from test_table', - cartocss: TestClient.CARTOCSS.POINTS, - cartocss_version: '2.3.0' - } - }]); + const mapConfig = createMapConfig({ + layers: [{ + type: 'cartodb', + options: { + sql: 'select * from test_table', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + }] + }); const testClient = new TestClient(mapConfig); @@ -59,14 +61,16 @@ describe('cache-control header', function () { it('tile from a table which is NOT included in cdb_tablemetada', function (done) { const ttl = global.environment.varnish.fallbackTtl || FIVE_MINUTES_IN_SECONDS; - const mapConfig = createMapConfig([{ - type: 'cartodb', - options: { - sql: 'select * from test_table_2', - cartocss: TestClient.CARTOCSS.POINTS, - cartocss_version: '2.3.0' - } - }]); + const mapConfig = createMapConfig({ + layers: [{ + type: 'cartodb', + options: { + sql: 'select * from test_table_2', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + }] + }); const testClient = new TestClient(mapConfig); @@ -82,24 +86,26 @@ describe('cache-control header', function () { it('tile from joined tables which one of them is NOT included in cdb_tablemetada', function (done) { const ttl = global.environment.varnish.fallbackTtl || FIVE_MINUTES_IN_SECONDS; - const mapConfig = createMapConfig([{ - type: 'cartodb', - options: { - sql: ` - select - t.cartodb_id, - t.the_geom, - t.the_geom_webmercator - from - test_table t, - test_table_2 t2 - where - t.cartodb_id = t2.cartodb_id - `, - cartocss: TestClient.CARTOCSS.POINTS, - cartocss_version: '2.3.0' - } - }]); + const mapConfig = createMapConfig({ + layers: [{ + type: 'cartodb', + options: { + sql: ` + select + t.cartodb_id, + t.the_geom, + t.the_geom_webmercator + from + test_table t, + test_table_2 t2 + where + t.cartodb_id = t2.cartodb_id + `, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + }] + }); const testClient = new TestClient(mapConfig); From bca723bcf3c1fbb101fa309c735843e651d1c781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jul 2019 14:59:16 +0200 Subject: [PATCH 6/7] PLease jshint --- test/acceptance/cache/cache-control-header.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/cache/cache-control-header.js b/test/acceptance/cache/cache-control-header.js index b2d6abae..5a738700 100644 --- a/test/acceptance/cache/cache-control-header.js +++ b/test/acceptance/cache/cache-control-header.js @@ -166,7 +166,7 @@ describe('cache-control header', function () { const testClient = new TestClient(mapConfig, 1234); - testClient.getTile(0, 0, 0, {}, function (err, res, layergroup) { + testClient.getTile(0, 0, 0, {}, function (err, res) { if (err) { return done(err); } From 619cad9c3506f905823e5744fded1d77e2cac1bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jul 2019 15:44:34 +0200 Subject: [PATCH 7/7] Typo --- lib/cartodb/api/middlewares/cache-control-header.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/api/middlewares/cache-control-header.js b/lib/cartodb/api/middlewares/cache-control-header.js index 03a23592..d15b259c 100644 --- a/lib/cartodb/api/middlewares/cache-control-header.js +++ b/lib/cartodb/api/middlewares/cache-control-header.js @@ -24,7 +24,7 @@ module.exports = function setCacheControlHeader ({ const directives = [ 'public' ]; - if (everyAffectedTablesCanBeInvalidated(affectedTables)) { + if (everyAffectedTableCanBeInvalidated(affectedTables)) { directives.push(`max-age=${ttl}`); } else { directives.push(`max-age=${fallbackTtl}`); @@ -41,7 +41,7 @@ module.exports = function setCacheControlHeader ({ }; }; -function everyAffectedTablesCanBeInvalidated (affectedTables) { +function everyAffectedTableCanBeInvalidated (affectedTables) { const skipNotUpdatedAtTables = false; const skipAnalysisCachedTables = true;