Remove mechanism to reset named map's provider as, in the end, it's reading from storage (redis) always so cache isn't doing its job. There is already a mechanism to invalidate cache entry when a template is modified (see template-maps emits on "update" and "delete", and listeners attached at server startup)

lib/cartodb/cache/named_map_provider_cache.js

@@ -4,7 +4,6 @@ var _ = require('underscore');
 var dot = require('dot');
 var NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider');
 var templateName = require('../backends/template_maps').templateName;
-var queue = require('queue-async');
 
 var LruCache = require("lru-cache");
 
@@ -57,23 +56,7 @@ NamedMapProviderCache.prototype.get = function(user, templateId, config, authTok
 
     var namedMapProvider = namedMapProviders[providerKey];
 
-    var self = this;
+    return callback(null, namedMapProvider);
-    queue(2)
-        .defer(namedMapProvider.getTemplate.bind(namedMapProvider))
-        .defer(this.templateMaps.getTemplate.bind(this.templateMaps), user, templateId)
-        .awaitAll(function templatesQueueDone(err, results) {
-            if (err) {
-                return callback(err);
-            }
-
-            // We want to reset provider its template has changed
-            // Ideally this should be done in a passive mode where this cache gets notified of template changes
-            var uniqueFingerprints = _.uniq(results.map(self.templateMaps.fingerPrint)).length;
-            if (uniqueFingerprints > 1) {
-                namedMapProvider.reset();
-            }
-            return callback(null, namedMapProvider);
-        });
 };
 
 NamedMapProviderCache.prototype.invalidate = function(user, templateId) {

test/acceptance/named_maps_cache.js

@@ -1,13 +1,12 @@
 'use strict';
 
 require('../support/test_helper');
-var RedisPool = require('redis-mpool');
 
+const helper = require('../support/test_helper');
 var assert = require('../support/assert');
 var mapnik = require('windshaft').mapnik;
 var CartodbWindshaft = require('../../lib/cartodb/server');
 var serverOptions = require('../../lib/cartodb/server_options');
-var TemplateMaps = require('../../lib/cartodb/backends/template_maps.js');
 
 describe('named maps provider cache', function() {
     var server;
@@ -16,14 +15,8 @@ describe('named maps provider cache', function() {
         server = new CartodbWindshaft(serverOptions);
     });
 
-    // configure redis pool instance to use in tests
-    var redisPool = new RedisPool(global.environment.redis);
-
-    var templateMaps = new TemplateMaps(redisPool, {
-        max_user_templates: global.environment.maxUserTemplates
-    });
-
     var username = 'localhost';
+    const apikey = 1234;
     var templateName = 'template_with_color';
 
     var IMAGE_TOLERANCE = 20;
@@ -31,7 +24,7 @@ describe('named maps provider cache', function() {
     function createTemplate(color) {
         return {
             version: '0.0.1',
-            name: templateName,
+            name: `${templateName}_${color}`,
             auth: {
                 method: 'open'
             },
@@ -56,17 +49,13 @@ describe('named maps provider cache', function() {
         };
     }
 
-    afterEach(function (done) {
+    function getNamedTile(templateId, options, callback) {
-        templateMaps.delTemplate(username, templateName, done);
-    });
-
-    function getNamedTile(options, callback) {
         if (!callback) {
             callback = options;
             options = {};
         }
 
-        var url = '/api/v1/map/named/' + templateName + '/all/' + [0,0,0].join('/') + '.png';
+        var url = '/api/v1/map/named/' + templateId + '/all/' + [0,0,0].join('/') + '.png';
 
         var requestOptions = {
             url: url,
@@ -88,60 +77,116 @@ describe('named maps provider cache', function() {
 
         assert.response(server, requestOptions, expectedResponse, function (res, err) {
             var img;
-            if (statusCode === 200) {
+            if (res.statusCode === 200) {
-                img = mapnik.Image.fromBytes(new Buffer(res.body, 'binary'));
+                img = mapnik.Image.fromBytes(new Buffer.from(res.body, 'binary'));
             }
             return callback(err, res, img);
         });
     }
 
+    function addTemplate (template, callback) {
+        const createTemplateRequest = {
+            url: `/api/v1/map/named?api_key=${apikey}`,
+            method: 'POST',
+            headers: {
+                host: username,
+                'Content-Type': 'application/json'
+            },
+            body: JSON.stringify(template)
+        };
+
+        const expectedResponse = {
+            status: 200,
+            headers: {
+                'Content-Type': 'application/json; charset=utf-8'
+            }
+        };
+
+        assert.response(server, createTemplateRequest, expectedResponse, (res, err) => {
+            let template;
+
+            if (res.statusCode === 200) {
+                template = JSON.parse(res.body);
+            }
+
+            return callback(err, res, template);
+        });
+    }
+
+
+    function deleteTemplate (templateId, callback) {
+        const deleteTemplateRequest = {
+            url: `/api/v1/map/named/${templateId}?api_key=${apikey}`,
+            method: 'DELETE',
+            headers: {
+                host: 'localhost',
+            }
+        };
+
+        const expectedResponse = {
+            status: 204,
+        };
+
+        assert.response(server, deleteTemplateRequest, expectedResponse, (res, err) => {
+            return callback(err, res);
+        });
+    }
+
     function previewFixture(color) {
         return './test/fixtures/provider/populated_places_simple_reduced-' + color + '.png';
     }
 
-    var colors = ['red', 'red', 'green', 'blue'];
+    var colors = ['black', 'red', 'green', 'blue'];
     colors.forEach(function(color) {
         it('should return an image estimating its bounds based on dataset', function (done) {
-            templateMaps.addTemplate(username, createTemplate(color), function (err) {
+            addTemplate(createTemplate(color), function (err, res, template) {
                 if (err) {
                     return done(err);
                 }
-                getNamedTile(function(err, res, img) {
+
+                getNamedTile(template.template_id, function(err, res, img) {
                     assert.ok(!err);
-                    assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, done);
+                    assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, (err) => {
-                });
+                        assert.ifError(err);
-            });
-        });
-    });
 
-    it('should fail to use template from named map provider after template deletion', function (done) {
+                        const keysToDelete = {};
-        var color = 'black';
+                        keysToDelete['map_tpl|localhost'] = 0;
-        templateMaps.addTemplate(username, createTemplate(color), function (err) {
+                        helper.deleteRedisKeys(keysToDelete, done);
-            if (err) {
-                return done(err);
-            }
-            getNamedTile(function(err, res, img) {
-                assert.ok(!err);
-                assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, function(err) {
-                    assert.ok(!err);
-
-                    templateMaps.delTemplate(username, templateName, function (err) {
-                        assert.ok(!err);
-
-                        getNamedTile({ statusCode: 404 }, function(err, res) {
-                            assert.ok(!err);
-                            assert.deepEqual(
-                                JSON.parse(res.body).errors,
-                                ["Template 'template_with_color' of user 'localhost' not found"]
-                            );
-
-                            // add template again so it's clean in afterEach
-                            templateMaps.addTemplate(username, createTemplate(color), done);
-                        });
                     });
                 });
             });
         });
     });
 
+    it('should fail to use template from named map provider after template deletion', function (done) {
+        const color = 'black';
+        const templateId = `${templateName}_${color}`;
+
+        addTemplate(createTemplate(color), function (err) {
+            assert.ifError(err);
+
+            getNamedTile(templateId, function(err, res, img) {
+                assert.ifError(err);
+
+                assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, function (err) {
+                    assert.ifError(err);
+
+                    deleteTemplate(templateId, function (err) {
+                        assert.ifError(err);
+
+                        getNamedTile(templateId, { statusCode: 404 }, function(err, res) {
+                            assert.ifError(err);
+
+                            assert.deepEqual(
+                                JSON.parse(res.body).errors,
+                                ["Template 'template_with_color_black' of user 'localhost' not found"]
+                            );
+
+                            done();
+                        });
+                    });
+                });
+            });
+        });
+    });
 });