diff --git a/NEWS.md b/NEWS.md index 4ba4f369..d9323845 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ 1.6.1 - 2013-MM-DD ------------------ +* Still set a meaningful X-Cache-Channel with cache_policy=persist (#105) 1.6.0 - 2013-10-02 ------------------ diff --git a/app/controllers/app.js b/app/controllers/app.js index 07611a68..96e0d61f 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -261,16 +261,20 @@ function handleQuery(req, res) { var cache_policy = req.query.cache_policy; if ( cache_policy === 'persist' ) { res.header('Cache-Control', 'public,max-age=' + ttl); - res.header('X-Cache-Channel', ''); // forever } else { if ( ! tableCacheItem || tableCacheItem.may_write ) { + // Tell clients this response is already expired + // TODO: prevent cache_policy from overriding this ? ttl = 0; - } else { - res.header('X-Cache-Channel', generateCacheKey(database, tableCacheItem, authenticated)); - } + } res.header('Cache-Control', 'no-cache,max-age='+ttl+',must-revalidate,public'); } + // Only set an X-Cache-Channel for responses we want Varnish to cache. + if ( tableCacheItem && ! tableCacheItem.may_write ) { + res.header('X-Cache-Channel', generateCacheKey(database, tableCacheItem, authenticated)); + } + // Set Last-Modified header // // Currently sets it to NOW diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index d2a80171..a22ebdb2 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -101,7 +101,6 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. No oAuth included ', fu },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); // Check cache headers - // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); assert.equal(res.headers['cache-control'], expected_cache_control); done(); @@ -115,9 +114,9 @@ test('cache_policy=persist', function(done){ },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); // Check cache headers - // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 assert.ok(res.headers.hasOwnProperty('x-cache-channel')); - assert.equal(res.headers['x-cache-channel'], ''); + // See https://github.com/CartoDB/CartoDB-SQL-API/issues/105 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); assert.equal(res.headers['cache-control'], expected_cache_control_persist); done(); }); @@ -143,7 +142,6 @@ function(done){ },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); // Check cache headers - // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); assert.equal(res.headers['cache-control'], expected_cache_control); done(); @@ -1071,7 +1069,6 @@ test('GET /api/v1/sql with SQL parameter on SELECT only should return CORS heade },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); // Check cache headers - // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); assert.equal(res.headers['cache-control'], expected_cache_control); assert.equal(res.headers['access-control-allow-origin'], '*');