Merge pull request #247 from CartoDB/last-modified-header
Set Last-Modified header based on affected tables
This commit is contained in:
commit
df589b5e85
6
NEWS.md
6
NEWS.md
@ -1,6 +1,10 @@
|
||||
1.24.1 - 2015-mm-dd
|
||||
1.25.0 - 2015-mm-dd
|
||||
-------------------
|
||||
|
||||
New features:
|
||||
|
||||
* Set `Last-Modified` header based on affected tables (#101)
|
||||
|
||||
|
||||
1.24.0 - 2015-08-04
|
||||
-------------------
|
||||
|
@ -365,26 +365,39 @@ function handleQuery(req, res) {
|
||||
tableCacheItem.hits++;
|
||||
return false;
|
||||
} else {
|
||||
//TODO: sanitize cdbuser
|
||||
pg.query("SELECT CDB_QueryTables($quotesql$" + sql + "$quotesql$)", function (err, result) {
|
||||
if (err) {
|
||||
self(err);
|
||||
return;
|
||||
}
|
||||
if ( result.rowCount === 1 ) {
|
||||
var raw_tables = result.rows[0].cdb_querytables;
|
||||
var tables = raw_tables.split(/^\{(.*)\}$/)[1].split(',');
|
||||
self(null, tables);
|
||||
} else {
|
||||
console.error(
|
||||
"Unexpected result from CDB_QueryTables($quotesql$" + sql + "$quotesql$): " + result
|
||||
);
|
||||
self(null, []);
|
||||
}
|
||||
});
|
||||
var affectedTablesAndLastUpdatedTimeQuery = [
|
||||
'WITH querytables AS (',
|
||||
'SELECT * FROM CDB_QueryTablesText($quotesql$' + sql + '$quotesql$) as tablenames',
|
||||
')',
|
||||
'SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max',
|
||||
'FROM CDB_TableMetadata m',
|
||||
'WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])'
|
||||
].join(' ');
|
||||
|
||||
pg.query(affectedTablesAndLastUpdatedTimeQuery, function (err, resultSet) {
|
||||
var tableNames = [];
|
||||
var lastUpdatedTime = Date.now();
|
||||
|
||||
if (!err && resultSet.rowCount === 1) {
|
||||
var result = resultSet.rows[0];
|
||||
// This is an Array, so no need to split into parts
|
||||
tableNames = result.tablenames;
|
||||
if (Number.isFinite(result.max)) {
|
||||
lastUpdatedTime = result.max * 1000;
|
||||
}
|
||||
} else {
|
||||
var errorMessage = (err && err.message) || 'unknown error';
|
||||
console.error("Error on query explain '%s': %s", sql, errorMessage);
|
||||
}
|
||||
|
||||
return self(null, {
|
||||
affectedTables: tableNames,
|
||||
lastUpdatedTime: lastUpdatedTime
|
||||
});
|
||||
});
|
||||
}
|
||||
},
|
||||
function setHeaders(err, tables){
|
||||
function setHeaders(err, result) {
|
||||
assert.ifError(err);
|
||||
|
||||
if ( req.profiler ) {
|
||||
@ -394,13 +407,14 @@ function handleQuery(req, res) {
|
||||
checkAborted('setHeaders');
|
||||
|
||||
// store explain result in local Cache
|
||||
if ( ! tableCacheItem && tables.length ) {
|
||||
if ( ! tableCacheItem && result && result.affectedTables ) {
|
||||
tableCacheItem = {
|
||||
affected_tables: tables,
|
||||
// check if query may possibly write
|
||||
may_write: queryMayWrite(sql),
|
||||
// initialise hit counter
|
||||
hits: 1
|
||||
affected_tables: result.affectedTables,
|
||||
last_modified: result.lastUpdatedTime,
|
||||
// check if query may possibly write
|
||||
may_write: queryMayWrite(sql),
|
||||
// initialise hit counter
|
||||
hits: 1
|
||||
};
|
||||
tableCache.set(sql_md5, tableCacheItem);
|
||||
}
|
||||
@ -449,14 +463,10 @@ function handleQuery(req, res) {
|
||||
res.header('X-Cache-Channel', generateCacheKey(dbopts.dbname, tableCacheItem, authenticated));
|
||||
}
|
||||
|
||||
// Set Last-Modified header
|
||||
//
|
||||
// Currently sets it to NOW
|
||||
//
|
||||
// TODO: use a real value, querying for most recent change in
|
||||
// any of the source tables
|
||||
//
|
||||
res.header('Last-Modified', new Date().toUTCString());
|
||||
var lastModified = (tableCacheItem && tableCacheItem.last_modified) ?
|
||||
tableCacheItem.last_modified :
|
||||
Date.now();
|
||||
res.header('Last-Modified', new Date(lastModified).toUTCString());
|
||||
|
||||
return null;
|
||||
},
|
||||
|
5
npm-shrinkwrap.json
generated
5
npm-shrinkwrap.json
generated
@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "cartodb_sql_api",
|
||||
"version": "1.24.1",
|
||||
"version": "1.25.0",
|
||||
"dependencies": {
|
||||
"cartodb-psql": {
|
||||
"version": "0.6.0",
|
||||
@ -142,7 +142,8 @@
|
||||
},
|
||||
"inherits": {
|
||||
"version": "2.0.1",
|
||||
"from": "inherits@~2.0.1"
|
||||
"from": "inherits@2",
|
||||
"resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.1.tgz"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
@ -5,7 +5,7 @@
|
||||
"keywords": [
|
||||
"cartodb"
|
||||
],
|
||||
"version": "1.24.1",
|
||||
"version": "1.25.0",
|
||||
"repository": {
|
||||
"type": "git",
|
||||
"url": "git://github.com/CartoDB/CartoDB-SQL-API.git"
|
||||
|
117
test/acceptance/last-modified-header.js
Normal file
117
test/acceptance/last-modified-header.js
Normal file
@ -0,0 +1,117 @@
|
||||
require('../helper');
|
||||
|
||||
var app = require(global.settings.app_root + '/app/controllers/app')();
|
||||
var assert = require('../support/assert');
|
||||
var qs = require('querystring');
|
||||
|
||||
describe('last modified header', function() {
|
||||
|
||||
var scenarios = [
|
||||
{
|
||||
tables: ['untitle_table_4'],
|
||||
desc: 'should use last updated time from public table',
|
||||
expectedLastModified: 'Wed, 01 Jan 2014 23:31:30 GMT'
|
||||
},
|
||||
{
|
||||
tables: ['private_table'],
|
||||
desc: 'should use last updated time from private table',
|
||||
expectedLastModified: 'Thu, 01 Jan 2015 23:31:30 GMT'
|
||||
},
|
||||
{
|
||||
tables: ['untitle_table_4', 'private_table'],
|
||||
desc: 'should use most recent last updated time from private and public table',
|
||||
expectedLastModified: 'Thu, 01 Jan 2015 23:31:30 GMT'
|
||||
},
|
||||
{
|
||||
tables: ['populated_places_simple_reduced', 'private_table'],
|
||||
desc: 'should use last updated time from table in cdb_tablemetadata instead of now() from unknown table',
|
||||
expectedLastModified: 'Thu, 01 Jan 2015 23:31:30 GMT'
|
||||
}
|
||||
];
|
||||
|
||||
scenarios.forEach(function(scenario) {
|
||||
it(scenario.desc, function(done) {
|
||||
var query = qs.stringify({
|
||||
q: scenario.tables.map(function(table) {
|
||||
return 'select cartodb_id from ' + table;
|
||||
}).join(' UNION ALL '),
|
||||
api_key: 1234
|
||||
});
|
||||
assert.response(app,
|
||||
{
|
||||
url: '/api/v1/sql?' + query,
|
||||
headers: {
|
||||
host: 'vizzuality.cartodb.com'
|
||||
},
|
||||
method: 'GET'
|
||||
},
|
||||
{
|
||||
statusCode: 200
|
||||
},
|
||||
function(res) {
|
||||
assert.equal(res.headers['last-modified'], scenario.expectedLastModified);
|
||||
done();
|
||||
}
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('should use Date.now() for tables not present in cdb_tablemetadata', function(done) {
|
||||
var query = qs.stringify({
|
||||
q: 'select cartodb_id from populated_places_simple_reduced limit 1',
|
||||
api_key: 1234
|
||||
});
|
||||
var fixedDateNow = Date.now();
|
||||
var dateNowFn = Date.now;
|
||||
Date.now = function() {
|
||||
return fixedDateNow;
|
||||
};
|
||||
assert.response(app,
|
||||
{
|
||||
url: '/api/v1/sql?' + query,
|
||||
headers: {
|
||||
host: 'vizzuality.cartodb.com'
|
||||
},
|
||||
method: 'GET'
|
||||
},
|
||||
{
|
||||
statusCode: 200
|
||||
},
|
||||
function(res) {
|
||||
Date.now = dateNowFn;
|
||||
assert.equal(res.headers['last-modified'], new Date(fixedDateNow).toUTCString());
|
||||
done();
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
it('should use Date.now() for functions or results with no table associated', function(done) {
|
||||
var query = qs.stringify({
|
||||
q: 'select 1',
|
||||
api_key: 1234
|
||||
});
|
||||
var fixedDateNow = Date.now();
|
||||
var dateNowFn = Date.now;
|
||||
Date.now = function() {
|
||||
return fixedDateNow;
|
||||
};
|
||||
assert.response(app,
|
||||
{
|
||||
url: '/api/v1/sql?' + query,
|
||||
headers: {
|
||||
host: 'vizzuality.cartodb.com'
|
||||
},
|
||||
method: 'GET'
|
||||
},
|
||||
{
|
||||
statusCode: 200
|
||||
},
|
||||
function(res) {
|
||||
Date.now = dateNowFn;
|
||||
assert.equal(res.headers['last-modified'], new Date(fixedDateNow).toUTCString());
|
||||
done();
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
});
|
@ -137,3 +137,16 @@ DROP TABLE IF EXISTS cpg_test;
|
||||
CREATE TABLE cpg_test (a int);
|
||||
GRANT ALL ON TABLE cpg_test TO :TESTUSER;
|
||||
GRANT SELECT ON TABLE cpg_test TO :PUBLICUSER;
|
||||
|
||||
|
||||
CREATE TABLE IF NOT EXISTS
|
||||
CDB_TableMetadata (
|
||||
tabname regclass not null primary key,
|
||||
updated_at timestamp with time zone not null default now()
|
||||
);
|
||||
|
||||
INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('untitle_table_4'::regclass, '2014-01-01T23:31:30.123Z');
|
||||
INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('private_table'::regclass, '2015-01-01T23:31:30.123Z');
|
||||
|
||||
GRANT SELECT ON CDB_TableMetadata TO :PUBLICUSER;
|
||||
GRANT SELECT ON CDB_TableMetadata TO :TESTUSER;
|
||||
|
Loading…
Reference in New Issue
Block a user