From 1637610f6697a4fb0144b2bfa2b4278cade92c39 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Fri, 18 Oct 2013 13:29:06 +0200
Subject: [PATCH] Set a meaningful X-Cache-Channel with cache_policy=persist

Closes #105
---
 NEWS.md                     |  1 +
 app/controllers/app.js      | 12 ++++++++----
 test/acceptance/app.test.js |  7 ++-----
 3 files changed, 11 insertions(+), 9 deletions(-)

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'], '*');