Merge branch 'master' into batch-move-user-metadata-service

This commit is contained in:
Daniel García Aubert 2016-05-27 11:47:28 +02:00
commit d3aa4a4140
9 changed files with 208 additions and 66 deletions

28
NEWS.md
View File

@ -1,12 +1,36 @@
1.28.2 - 2016-mm-dd
1.29.3 - 2016-mm-dd
-------------------
1.29.2 - 2016-05-25
-------------------
Bug fixes:
* Fixed issue with status transition in fallback jobs #308
1.29.1 - 2016-05-24
-------------------
Announcements:
* Change Batch API size limit: 8kb per job.
1.29.0 - 2016-05-24
-------------------
New features:
* Add support for fallback-jobs in Batch API #296
Bug fixes:
* Fix issue in Batch API when a 'no longer running' job reports as 'running' before and after a job cancel #293
1.28.1 - 2016-05-12
-------------------
Bug fixes:
* OGR with _needSRS=true fails for empty tables #299
* OGR with _needSRS=true_ fails for empty tables #299
1.28.0 - 2016-05-11

View File

@ -11,23 +11,12 @@ var handleException = require('../utils/error_handler');
var cdbReq = new CdbRequest();
var ONE_KILOBYTE_IN_BYTES = 1024;
var MAX_LIMIT_QUERY_SIZE_IN_BYTES = 4 * ONE_KILOBYTE_IN_BYTES; // 4kb
function reachMaxQuerySizeLimit(query) {
var querySize;
try {
querySize = (typeof query === 'string') ? query.length : JSON.stringify(query).length;
} catch (e) {
return false;
}
return querySize > MAX_LIMIT_QUERY_SIZE_IN_BYTES;
}
var MAX_LIMIT_QUERY_SIZE_IN_KB = 8;
var MAX_LIMIT_QUERY_SIZE_IN_BYTES = MAX_LIMIT_QUERY_SIZE_IN_KB * ONE_KILOBYTE_IN_BYTES;
function getMaxSizeErrorMessage(sql) {
return util.format([
'Your payload is too large (%s). Max size allowed is %s (%skb).',
'Your payload is too large: %s bytes. Max size allowed is %s bytes (%skb).',
'Are you trying to import data?.',
'Please, check out import api http://docs.cartodb.com/cartodb-platform/import-api/'
].join(' '),
@ -42,15 +31,26 @@ function JobController(userDatabaseService, jobService) {
this.jobService = jobService;
}
function bodyPayloadSizeMiddleware(req, res, next) {
var payload = JSON.stringify(req.body);
if (payload.length > MAX_LIMIT_QUERY_SIZE_IN_BYTES) {
return handleException(new Error(getMaxSizeErrorMessage(payload)), res);
} else {
return next(null);
}
}
module.exports = JobController;
module.exports.MAX_LIMIT_QUERY_SIZE_IN_BYTES = MAX_LIMIT_QUERY_SIZE_IN_BYTES;
module.exports.getMaxSizeErrorMessage = getMaxSizeErrorMessage;
JobController.prototype.route = function (app) {
app.post(global.settings.base_url + '/sql/job', this.createJob.bind(this));
app.post(global.settings.base_url + '/sql/job', bodyPayloadSizeMiddleware, this.createJob.bind(this));
app.get(global.settings.base_url + '/sql/job', this.listJob.bind(this));
app.get(global.settings.base_url + '/sql/job/:job_id', this.getJob.bind(this));
app.delete(global.settings.base_url + '/sql/job/:job_id', this.cancelJob.bind(this));
app.put(global.settings.base_url + '/sql/job/:job_id', this.updateJob.bind(this));
app.patch(global.settings.base_url + '/sql/job/:job_id', this.updateJob.bind(this));
app.put(global.settings.base_url + '/sql/job/:job_id', bodyPayloadSizeMiddleware, this.updateJob.bind(this));
app.patch(global.settings.base_url + '/sql/job/:job_id', bodyPayloadSizeMiddleware, this.updateJob.bind(this));
};
JobController.prototype.cancelJob = function (req, res) {
@ -253,18 +253,12 @@ JobController.prototype.getJob = function (req, res) {
};
JobController.prototype.createJob = function (req, res) {
// jshint maxcomplexity: 7
var self = this;
var body = (req.body) ? req.body : {};
var params = _.extend({}, req.query, body); // clone so don't modify req.params or req.body so oauth is not broken
var sql = (params.query === "" || _.isUndefined(params.query)) ? null : params.query;
var cdbUsername = cdbReq.userByReq(req);
// TODO: in job.validate()
if (reachMaxQuerySizeLimit(sql)) {
return handleException(new Error(getMaxSizeErrorMessage(sql)), res);
}
if ( req.profiler ) {
req.profiler.start('sqlapi.job');
req.profiler.done('init');
@ -331,7 +325,6 @@ JobController.prototype.createJob = function (req, res) {
};
JobController.prototype.updateJob = function (req, res) {
// jshint maxcomplexity: 7
var self = this;
var job_id = req.params.job_id;
var body = (req.body) ? req.body : {};
@ -339,11 +332,6 @@ JobController.prototype.updateJob = function (req, res) {
var sql = (params.query === "" || _.isUndefined(params.query)) ? null : params.query;
var cdbUsername = cdbReq.userByReq(req);
// TODO: in jobValidate
if (reachMaxQuerySizeLimit(sql)) {
return handleException(new Error(getMaxSizeErrorMessage(sql)), res);
}
if ( req.profiler ) {
req.profiler.start('sqlapi.job');
req.profiler.done('init');

View File

@ -26,6 +26,10 @@ function doCancel(job_id, userDatabaseMetadata, callback) {
return callback(err);
}
if (!pid) {
return callback();
}
doCancelQuery(pg, pid, function (err, isCancelled) {
if (err) {
return callback(err);
@ -35,7 +39,7 @@ function doCancel(job_id, userDatabaseMetadata, callback) {
return callback(new Error('Query has not been cancelled'));
}
callback(null);
callback();
});
});
}
@ -49,7 +53,8 @@ function getQueryPID(pg, job_id, callback) {
}
if (!result.rows[0] || !result.rows[0].pid) {
return callback(new Error('Query is not running currently'));
// query is not running actually, but we have to callback w/o error to cancel the job anyway.
return callback();
}
callback(null, result.rows[0].pid);

View File

@ -36,19 +36,19 @@ JobService.prototype.list = function (user, callback) {
}
var jobList = dataList.map(function (data) {
var job;
var job;
try {
job = JobFactory.create(data);
} catch (err) {
return debug(err);
}
try {
job = JobFactory.create(data);
} catch (err) {
return debug(err);
}
return job;
})
.filter(function (job) {
return job !== undefined;
});
return job;
})
.filter(function (job) {
return job !== undefined;
});
callback(null, jobList);
});

View File

@ -76,7 +76,7 @@ JobFallback.is = function (query) {
};
JobFallback.prototype.init = function () {
// jshint maxcomplexity: 10
// jshint maxcomplexity: 8
for (var i = 0; i < this.data.query.query.length; i++) {
if ((this.data.query.query[i].onsuccess || this.data.query.query[i].onerror) &&
!this.data.query.query[i].status) {
@ -111,7 +111,7 @@ JobFallback.prototype._hasNextQueryFromQuery = function () {
};
JobFallback.prototype._getNextQueryFromQuery = function () {
// jshint maxcomplexity: 10
// jshint maxcomplexity: 8
for (var i = 0; i < this.data.query.query.length; i++) {
if (this.data.query.query[i].fallback_status) {
@ -238,7 +238,7 @@ JobFallback.prototype._setJobStatus = function (status, isChangeAppliedToQueryFa
};
JobFallback.prototype._shiftJobStatus = function (status, isChangeAppliedToQueryFallback) {
// jshint maxcomplexity: 10
// jshint maxcomplexity: 7
// In some scenarios we have to change the normal flow in order to keep consistency
// between query's status and job's status.
@ -257,6 +257,11 @@ JobFallback.prototype._shiftJobStatus = function (status, isChangeAppliedToQuery
};
JobFallback.prototype._shouldTryToApplyStatusTransitionToQueryFallback = function (index) {
return (this.data.query.query[index].status === jobStatus.DONE && this.data.query.query[index].onsuccess) ||
(this.data.query.query[index].status === jobStatus.FAILED && this.data.query.query[index].onerror);
};
JobFallback.prototype._setQueryStatus = function (status, errorMesssage) {
// jshint maxcomplexity: 7
var isValid = false;
@ -275,7 +280,7 @@ JobFallback.prototype._setQueryStatus = function (status, errorMesssage) {
break;
}
if (this.data.query.query[i].fallback_status) {
if (this._shouldTryToApplyStatusTransitionToQueryFallback(i)) {
isValid = this.isValidStatusTransition(this.data.query.query[i].fallback_status, status);
if (isValid) {

2
npm-shrinkwrap.json generated
View File

@ -1,6 +1,6 @@
{
"name": "cartodb_sql_api",
"version": "1.28.2",
"version": "1.29.2",
"dependencies": {
"cartodb-psql": {
"version": "0.6.1",

View File

@ -5,7 +5,7 @@
"keywords": [
"cartodb"
],
"version": "1.28.2",
"version": "1.29.3",
"repository": {
"type": "git",
"url": "git://github.com/CartoDB/CartoDB-SQL-API.git"

View File

@ -875,6 +875,81 @@ describe('Batch API fallback job', function () {
});
});
describe('"onerror" should not be triggered for any query', function () {
var fallbackJob = {};
it('should create a job', function (done) {
assert.response(app, {
url: '/api/v2/sql/job?api_key=1234',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
'host': 'vizzuality.cartodb.com'
},
method: 'POST',
data: querystring.stringify({
query: {
query: [{
query: "SELECT * FROM untitle_table_4 limit 1",
onerror: "SELECT * FROM untitle_table_4 limit 2"
}, {
query: "SELECT * FROM untitle_table_4 limit 3",
onerror: "SELECT * FROM untitle_table_4 limit 4"
}]
}
})
}, {
status: 201
}, function (res, err) {
if (err) {
return done(err);
}
fallbackJob = JSON.parse(res.body);
done();
});
});
it('job should be failed', function (done) {
var expectedQuery = {
query: [{
query: 'SELECT * FROM untitle_table_4 limit 1',
onerror: 'SELECT * FROM untitle_table_4 limit 2',
status: 'done',
fallback_status: 'pending'
}, {
query: 'SELECT * FROM untitle_table_4 limit 3',
onerror: 'SELECT * FROM untitle_table_4 limit 4',
status: 'done',
fallback_status: 'pending'
}]
};
var interval = setInterval(function () {
assert.response(app, {
url: '/api/v2/sql/job/' + fallbackJob.job_id + '?api_key=1234&',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
'host': 'vizzuality.cartodb.com'
},
method: 'GET'
}, {
status: 200
}, function (res, err) {
if (err) {
return done(err);
}
var job = JSON.parse(res.body);
if (job.status === jobStatus.DONE) {
clearInterval(interval);
assert.deepEqual(job.query, expectedQuery);
done();
} else if (job.status === jobStatus.FAILED || job.status === jobStatus.CANCELLED) {
clearInterval(interval);
done(new Error('Job ' + job.job_id + ' is ' + job.status + ', expected to be failed'));
}
});
}, 50);
});
});
describe('"onsuccess" for first query should fail', function () {
var fallbackJob = {};

View File

@ -13,10 +13,11 @@
*
*/
require('../helper');
var JobController = require('../../app/controllers/job_controller');
var app = require(global.settings.app_root + '/app/app')();
var assert = require('../support/assert');
var querystring = require('querystring');
var querystring = require('qs');
var metadataBackend = require('cartodb-redis')({
host: global.settings.redis_host,
port: global.settings.redis_port,
@ -25,11 +26,23 @@ var metadataBackend = require('cartodb-redis')({
reapIntervalMillis: global.settings.redisReapIntervalMillis
});
var queryMaxSize = new Array(4097).join('a');
function payload(query) {
return JSON.stringify({query: query});
}
function payloadSize(query) {
return payload(query).length;
}
var minPayloadSize = payloadSize('');
var queryMaxSize = new Array(JobController.MAX_LIMIT_QUERY_SIZE_IN_BYTES - minPayloadSize + 1).join('a');
var queryTooLong = queryMaxSize.concat('a');
describe('job query limit', function() {
function expectedErrorMessage(query) {
return JobController.getMaxSizeErrorMessage(payload(query));
}
after(function (done) {
// batch services is not activate, so we need empty the queue to avoid unexpected
// behaviour in further tests
@ -49,13 +62,7 @@ describe('job query limit', function() {
status: 400
}, function (res) {
var error = JSON.parse(res.body);
assert.deepEqual(error, { error: [
[
'Your payload is too large (4097). Max size allowed is 4096 (4kb).',
'Are you trying to import data?.',
'Please, check out import api http://docs.cartodb.com/cartodb-platform/import-api/'
].join(' ')
]});
assert.deepEqual(error, { error: [expectedErrorMessage(queryTooLong)] });
done();
});
});
@ -73,13 +80,7 @@ describe('job query limit', function() {
status: 400
}, function (res) {
var error = JSON.parse(res.body);
assert.deepEqual(error, { error: [
[
'Your payload is too large (4097). Max size allowed is 4096 (4kb).',
'Are you trying to import data?.',
'Please, check out import api http://docs.cartodb.com/cartodb-platform/import-api/'
].join(' ')
]});
assert.deepEqual(error, { error: [expectedErrorMessage(queryTooLong)] });
done();
});
});
@ -102,4 +103,48 @@ describe('job query limit', function() {
});
});
it('POST /api/v2/sql/job with a invalid query size should consider multiple queries', function (done){
var queries = [queryTooLong, 'select 1'];
assert.response(app, {
url: '/api/v2/sql/job?api_key=1234',
headers: { 'host': 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' },
method: 'POST',
data: querystring.stringify({
query: queries
})
}, {
status: 400
}, function (res) {
var error = JSON.parse(res.body);
assert.deepEqual(error, { error: [expectedErrorMessage(queries)] });
done();
});
});
it('POST /api/v2/sql/job with a invalid query size should consider fallback queries/callbacks', function (done){
var fallbackQueries = {
query: [{
query: queryTooLong,
onsuccess: "SELECT * FROM untitle_table_4 limit 1"
}, {
query: "SELECT * FROM untitle_table_4 limit 2",
onsuccess: "SELECT * FROM untitle_table_4 limit 3"
}]
};
assert.response(app, {
url: '/api/v2/sql/job?api_key=1234',
headers: { 'host': 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' },
method: 'POST',
data: querystring.stringify({
query: fallbackQueries
})
}, {
status: 400
}, function (res) {
var error = JSON.parse(res.body);
assert.deepEqual(error, { error: [expectedErrorMessage(fallbackQueries)] });
done();
});
});
});