From ad79717069a0f250585c137a75d241fe8ed66a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 13 Jun 2018 10:39:44 +0200 Subject: [PATCH 01/32] NEWS 2.1.0 date --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index d6347bf0..bb3bb201 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ # Changelog ## 2.1.0 -Released 2018-mm-dd +Released 2018-06-13 New features: * CI tests with Ubuntu Xenial + PostgreSQL 10.1 and Ubuntu Precise + PostgreSQL 9.5 From f5d56a2253755de1db52034b4ee5e25f04ddef7e Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 13 Jun 2018 13:17:01 +0200 Subject: [PATCH 02/32] FIX add return to callback call --- app/services/user_database_service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/user_database_service.js b/app/services/user_database_service.js index dbda65b3..23ecf89a 100644 --- a/app/services/user_database_service.js +++ b/app/services/user_database_service.js @@ -67,7 +67,7 @@ UserDatabaseService.prototype.getConnectionParams = function (username, apikeyTo commonDBConfiguration); if (isOauthAuthorization({ apikeyToken, authorizationLevel})) { - callback(null, masterDBConfiguration, masterDBConfiguration); + return callback(null, masterDBConfiguration, masterDBConfiguration); } // Default Api key fallback From 6ecd0c00ddfc10d9c9aecf504e2c0ac7774fe44f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 13 Jun 2018 13:29:26 +0200 Subject: [PATCH 03/32] stubs next version --- NEWS.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index bb3bb201..8fc50b37 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # Changelog +## 2.1.1 +Released 2018-mm-dd + + ## 2.1.0 Released 2018-06-13 diff --git a/package.json b/package.json index 1001cd15..59cb58bc 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "keywords": [ "cartodb" ], - "version": "2.1.0", + "version": "2.1.1", "repository": { "type": "git", "url": "git://github.com/CartoDB/CartoDB-SQL-API.git" From 2fa5e7b84a7e9e8c84b32ecd42236d71f1e112e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 10:44:57 +0200 Subject: [PATCH 04/32] grouping all copy related tests in a describe --- test/acceptance/copy-endpoints.js | 640 +++++++++++++++--------------- 1 file changed, 311 insertions(+), 329 deletions(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index a6bfdb2e..3e3284b2 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -19,189 +19,22 @@ const server = require('../../app/server')(statsClient); describe('copy-endpoints', function() { - before(function(done) { - const client = new Client({ - user: 'postgres', - host: 'localhost', - database: 'cartodb_test_user_1_db', - port: 5432, + describe('general', function() { + before(function(done) { + const client = new Client({ + user: 'postgres', + host: 'localhost', + database: 'cartodb_test_user_1_db', + port: 5432, + }); + client.connect(); + client.query('TRUNCATE copy_endpoints_test', (err/*, res */) => { + client.end(); + done(err); + }); }); - client.connect(); - client.query('TRUNCATE copy_endpoints_test', (err/*, res */) => { - client.end(); - done(err); - }); - }); - - it('should work with copyfrom endpoint', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" - }), - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'POST' - },{}, function(err, res) { - assert.ifError(err); - const response = JSON.parse(res.body); - assert.equal(!!response.time, true); - assert.strictEqual(response.total_rows, 6); - - assert.ok(res.headers['x-sqlapi-profiler']); - const headers = JSON.parse(res.headers['x-sqlapi-profiler']); - assert.ok(headers.copyFrom); - const metrics = headers.copyFrom; - assert.equal(metrics.size, 57); - assert.equal(metrics.format, 'CSV'); - assert.equal(metrics.time, response.time); - assert.equal(metrics.rows, response.total_rows); - assert.equal(metrics.gzip, false); - - done(); - }); - }); - - it('should fail with copyfrom endpoint and unexisting table', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: "COPY unexisting_table (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" - }), - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'POST' - },{}, function(err, res) { - assert.ifError(err); - assert.deepEqual( - JSON.parse(res.body), - { - error:['relation \"unexisting_table\" does not exist'] - } - ); - done(); - }); - }); - - it('should fail with copyfrom endpoint and without csv', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'POST' - },{}, function(err, res) { - assert.ifError(err); - assert.deepEqual( - JSON.parse(res.body), - { - error:['No rows copied'] - } - ); - done(); - }); - }); - - it('should fail with copyfrom endpoint and without q', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyfrom", - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'POST' - },{}, function(err, res) { - assert.ifError(err); - assert.deepEqual( - JSON.parse(res.body), - { - error:["SQL is missing"] - } - ); - done(); - }); - }); - - it('should work with copyto endpoint', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyto?" + querystring.stringify({ - q: 'COPY copy_endpoints_test TO STDOUT', - filename: '/tmp/output.dmp' - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{}, function(err, res) { - assert.ifError(err); - assert.strictEqual( - res.body, - '11\tPaul\t10\n12\tPeter\t10\n13\tMatthew\t10\n14\t\\N\t10\n15\tJames\t10\n16\tJohn\t10\n' - ); - - assert.equal(res.headers['content-disposition'], 'attachment; filename=%2Ftmp%2Foutput.dmp'); - assert.equal(res.headers['content-type'], 'application/octet-stream'); - - done(); - }); - }); - - it('should fail with copyto endpoint and without sql', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyto?" + querystring.stringify({ - filename: '/tmp/output.dmp' - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{}, function(err, res) { - assert.ifError(err); - assert.deepEqual( - JSON.parse(res.body), - { - error:["SQL is missing"] - } - ); - done(); - }); - }); - - it('should work with copyfrom and gzip', function(done){ - assert.response(server, { - url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: "COPY copy_endpoints_test2 (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" - }), - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv.gz'), - headers: { - host: 'vizzuality.cartodb.com', - 'content-encoding': 'gzip' - }, - method: 'POST' - },{}, function(err, res) { - assert.ifError(err); - const response = JSON.parse(res.body); - assert.equal(!!response.time, true); - assert.strictEqual(response.total_rows, 6); - - assert.ok(res.headers['x-sqlapi-profiler']); - const headers = JSON.parse(res.headers['x-sqlapi-profiler']); - assert.ok(headers.copyFrom); - const metrics = headers.copyFrom; - assert.equal(metrics.size, 57); - assert.equal(metrics.format, 'CSV'); - assert.equal(metrics.time, response.time); - assert.equal(metrics.rows, response.total_rows); - assert.equal(metrics.gzip, true); - - done(); - }); - }); - -}); - - -describe('copy-endpoints timeout', function() { - it('should fail with copyfrom and timeout', function(done){ - assert.response(server, { - url: '/api/v1/sql?q=set statement_timeout = 10', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - function(err) { - assert.ifError(err); + + it('should work with copyfrom endpoint', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" @@ -209,187 +42,336 @@ describe('copy-endpoints timeout', function() { data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), headers: {host: 'vizzuality.cartodb.com'}, method: 'POST' - }, - { - status: 429, - headers: { 'Content-Type': 'application/json; charset=utf-8' } - }, - function(err, res) { + },{}, function(err, res) { assert.ifError(err); - assert.deepEqual(JSON.parse(res.body), { - error: [ - 'You are over platform\'s limits. Please contact us to know more details' - ], - context: 'limit', - detail: 'datasource' - }); - - - assert.response(server, { - url: "/api/v1/sql?q=set statement_timeout = 2000", - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - done); + const response = JSON.parse(res.body); + console.log(response); + assert.equal(!!response.time, true); + assert.strictEqual(response.total_rows, 6); + done(); }); }); - }); - - it('should fail with copyto and timeout', function(done){ - assert.response(server, { - url: '/api/v1/sql?q=set statement_timeout = 20', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - function(err) { - assert.ifError(err); + + it('should fail with copyfrom endpoint and unexisting table', function(done){ + assert.response(server, { + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: "COPY unexisting_table (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" + }), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'POST' + },{}, function(err, res) { + assert.ifError(err); + assert.deepEqual( + JSON.parse(res.body), + { + error:['relation \"unexisting_table\" does not exist'] + } + ); + done(); + }); + }); + + it('should fail with copyfrom endpoint and without csv', function(done){ + assert.response(server, { + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'POST' + },{}, function(err, res) { + assert.ifError(err); + assert.deepEqual( + JSON.parse(res.body), + { + error:['No rows copied'] + } + ); + done(); + }); + }); + + it('should fail with copyfrom endpoint and without q', function(done){ + assert.response(server, { + url: "/api/v1/sql/copyfrom", + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'POST' + },{}, function(err, res) { + assert.ifError(err); + assert.deepEqual( + JSON.parse(res.body), + { + error:["SQL is missing"] + } + ); + done(); + }); + }); + + it('should work with copyto endpoint', function(done){ assert.response(server, { url: "/api/v1/sql/copyto?" + querystring.stringify({ - q: 'COPY populated_places_simple_reduced TO STDOUT', + q: 'COPY copy_endpoints_test TO STDOUT', filename: '/tmp/output.dmp' }), headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{}, function(err, res) { assert.ifError(err); - const error = { - error:["You are over platform's limits. Please contact us to know more details"], - context:"limit", - detail:"datasource" - }; - const expectedError = res.body.substring(res.body.length - JSON.stringify(error).length); - assert.deepEqual(JSON.parse(expectedError), error); - - assert.response(server, { - url: "/api/v1/sql?q=set statement_timeout = 2000", - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - }, - done); + assert.strictEqual( + res.body, + '11\tPaul\t10\n12\tPeter\t10\n13\tMatthew\t10\n14\t\\N\t10\n15\tJames\t10\n16\tJohn\t10\n' + ); + + assert.equal(res.headers['content-disposition'], 'attachment; filename=%2Ftmp%2Foutput.dmp'); + assert.equal(res.headers['content-type'], 'application/octet-stream'); + + done(); }); }); + + it('should fail with copyto endpoint and without sql', function(done){ + assert.response(server, { + url: "/api/v1/sql/copyto?" + querystring.stringify({ + filename: '/tmp/output.dmp' + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(err, res) { + assert.ifError(err); + assert.deepEqual( + JSON.parse(res.body), + { + error:["SQL is missing"] + } + ); + done(); + }); + }); + + it('should work with copyfrom and gzip', function(done){ + assert.response(server, { + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: "COPY copy_endpoints_test2 (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" + }), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv.gz'), + headers: { + host: 'vizzuality.cartodb.com', + 'content-encoding': 'gzip' + }, + method: 'POST' + },{}, function(err, res) { + assert.ifError(err); + const response = JSON.parse(res.body); + assert.equal(!!response.time, true); + assert.strictEqual(response.total_rows, 6); + done(); + }); + }); + }); -}); - - -describe('copy-endpoints db connections', function() { - before(function() { - this.db_pool_size = global.settings.db_pool_size; - global.settings.db_pool_size = 1; - }); - - after(function() { - global.settings.db_pool_size = this.db_pool_size; - }); - - it('copyfrom', function(done) { - const query = "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)"; - function doCopyFrom() { - return new Promise(resolve => { + + + describe('timeout', function() { + it('should fail with copyfrom and timeout', function(done){ + assert.response(server, { + url: '/api/v1/sql?q=set statement_timeout = 10', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + function(err) { + assert.ifError(err); assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: query + q: `COPY copy_endpoints_test (id, name) + FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)` }), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), headers: {host: 'vizzuality.cartodb.com'}, method: 'POST' - },{}, function(err, res) { + }, + { + status: 429, + headers: { 'Content-Type': 'application/json; charset=utf-8' } + }, + function(err, res) { assert.ifError(err); - const response = JSON.parse(res.body); - assert.ok(response.time); - resolve(); + assert.deepEqual(JSON.parse(res.body), { + error: [ + 'You are over platform\'s limits. Please contact us to know more details' + ], + context: 'limit', + detail: 'datasource' + }); + + + assert.response(server, { + url: "/api/v1/sql?q=set statement_timeout = 2000", + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + done); }); }); - } - - Promise.all([doCopyFrom(), doCopyFrom(), doCopyFrom()]).then(function() { - done(); }); - }); - - it('copyto', function(done) { - function doCopyTo() { - return new Promise(resolve => { + + it('should fail with copyto and timeout', function(done){ + assert.response(server, { + url: '/api/v1/sql?q=set statement_timeout = 20', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + function(err) { + assert.ifError(err); assert.response(server, { url: "/api/v1/sql/copyto?" + querystring.stringify({ - q: 'COPY copy_endpoints_test TO STDOUT', + q: 'COPY populated_places_simple_reduced TO STDOUT', filename: '/tmp/output.dmp' }), headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{}, function(err, res) { assert.ifError(err); - assert.ok(res.body); - resolve(); + const error = { + error:["You are over platform's limits. Please contact us to know more details"], + context:"limit", + detail:"datasource" + }; + const expectedError = res.body.substring(res.body.length - JSON.stringify(error).length); + assert.deepEqual(JSON.parse(expectedError), error); + + assert.response(server, { + url: "/api/v1/sql?q=set statement_timeout = 2000", + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + done); }); }); - } - - Promise.all([doCopyTo(), doCopyTo(), doCopyTo()]).then(function() { - done(); }); }); + + + describe('db connections', function() { + before(function() { + this.db_pool_size = global.settings.db_pool_size; + global.settings.db_pool_size = 1; + }); + + after(function() { + global.settings.db_pool_size = this.db_pool_size; + }); + + it('copyfrom', function(done) { + function doCopyFrom() { + return new Promise(resolve => { + assert.response(server, { + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: `COPY copy_endpoints_test (id, name) + FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)` + }), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'POST' + },{}, function(err, res) { + assert.ifError(err); + const response = JSON.parse(res.body); + assert.ok(response.time); + resolve(); + }); + }); + } + + Promise.all([doCopyFrom(), doCopyFrom(), doCopyFrom()]).then(function() { + done(); + }); + }); + + it('copyto', function(done) { + function doCopyTo() { + return new Promise(resolve => { + assert.response(server, { + url: "/api/v1/sql/copyto?" + querystring.stringify({ + q: 'COPY copy_endpoints_test TO STDOUT', + filename: '/tmp/output.dmp' + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(err, res) { + assert.ifError(err); + assert.ok(res.body); + resolve(); + }); + }); + } + + Promise.all([doCopyTo(), doCopyTo(), doCopyTo()]).then(function() { + done(); + }); + }); + }); + + describe('client disconnection', function() { + // Give it enough time to connect and issue the query + // but not too much so as to disconnect in the middle of the query. + const client_disconnect_timeout = 10; + + before(function() { + this.db_pool_size = global.settings.db_pool_size; + global.settings.db_pool_size = 1; + }); + + after(function() { + global.settings.db_pool_size = this.db_pool_size; + }); + + var assertCanReuseConnection = function (done) { + assert.response(server, { + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT 1', + }), + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }, {}, function(err, res) { + assert.ifError(err); + assert.ok(res.statusCode === 200); + done(); + }); + }; + + it('COPY TO returns the connection to the pool if the client disconnects', function(done) { + assert.response(server, { + url: '/api/v1/sql/copyto?' + querystring.stringify({ + q: 'COPY (SELECT * FROM generate_series(1, 100000)) TO STDOUT', + }), + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET', + timeout: client_disconnect_timeout + }, {}, function(err) { + // we're expecting a timeout error + assert.ok(err); + assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); + assertCanReuseConnection(done); + }); + }); + + it('COPY FROM returns the connection to the pool if the client disconnects', function(done) { + assert.response(server, { + url: '/api/v1/sql/copyfrom?' + querystring.stringify({ + q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)", + }), + headers: { host: 'vizzuality.cartodb.com' }, + method: 'POST', + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + timeout: client_disconnect_timeout + }, {}, function(err) { + // we're expecting a timeout error + assert.ok(err); + assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); + assertCanReuseConnection(done); + }); + }); + + }); }); - -describe('copy-endpoints client disconnection', function() { - // Give it enough time to connect and issue the query - // but not too much so as to disconnect in the middle of the query. - const client_disconnect_timeout = 10; - - before(function() { - this.db_pool_size = global.settings.db_pool_size; - global.settings.db_pool_size = 1; - }); - - after(function() { - global.settings.db_pool_size = this.db_pool_size; - }); - - var assertCanReuseConnection = function (done) { - assert.response(server, { - url: '/api/v1/sql?' + querystring.stringify({ - q: 'SELECT 1', - }), - headers: { host: 'vizzuality.cartodb.com' }, - method: 'GET' - }, {}, function(err, res) { - assert.ifError(err); - assert.ok(res.statusCode === 200); - done(); - }); - }; - - it('COPY TO returns the connection to the pool if the client disconnects', function(done) { - assert.response(server, { - url: '/api/v1/sql/copyto?' + querystring.stringify({ - q: 'COPY (SELECT * FROM generate_series(1, 100000)) TO STDOUT', - }), - headers: { host: 'vizzuality.cartodb.com' }, - method: 'GET', - timeout: client_disconnect_timeout - }, {}, function(err) { - // we're expecting a timeout error - assert.ok(err); - assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); - assertCanReuseConnection(done); - }); - }); - - it('COPY FROM returns the connection to the pool if the client disconnects', function(done) { - assert.response(server, { - url: '/api/v1/sql/copyfrom?' + querystring.stringify({ - q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)", - }), - headers: { host: 'vizzuality.cartodb.com' }, - method: 'POST', - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), - timeout: client_disconnect_timeout - }, {}, function(err) { - // we're expecting a timeout error - assert.ok(err); - assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); - assertCanReuseConnection(done); - }); - }); - -}); From 1f31b8e2ae19d4e339992bf059b92bd0294ad7a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 10:45:23 +0200 Subject: [PATCH 05/32] removing old copy logs --- app/controllers/copy_controller.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index 1bbc8bd9..cea5abbd 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -93,24 +93,16 @@ function handleCopyFrom (logger) { res.locals.user, req.get('content-encoding') === 'gzip', logger, - function(err, metrics) { // TODO: remove when data-ingestion log works: {time, rows} + function(err, metrics) { if (err) { return next(err); } - // TODO: remove when data-ingestion log works - const { time, rows, type, format, gzip, size } = metrics; - - if (!time || !rows) { + const { time, rows } = metrics; + if (!rows) { return next(new Error("No rows copied")); } - // TODO: remove when data-ingestion log works - if (req.profiler) { - req.profiler.add({copyFrom: { type, format, gzip, size, rows, time }}); - res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); - } - res.send({ time, total_rows: rows From 34c0c5c7380c143b52d595d9fbda25c0112261e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 10:48:00 +0200 Subject: [PATCH 06/32] removing console log --- test/acceptance/copy-endpoints.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 3e3284b2..61a37d79 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -45,7 +45,6 @@ describe('copy-endpoints', function() { },{}, function(err, res) { assert.ifError(err); const response = JSON.parse(res.body); - console.log(response); assert.equal(!!response.time, true); assert.strictEqual(response.total_rows, 6); done(); From b091a375fc324baaecc07f4ca0ff13822896c1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 10:49:36 +0200 Subject: [PATCH 07/32] fix ensuring right validation error --- app/controllers/copy_controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index cea5abbd..ce1836f6 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -117,12 +117,12 @@ function validateCopyQuery () { const sql = req.query.q; if (!sql) { - next(new Error("SQL is missing")); + return next(new Error("SQL is missing")); } // Only accept SQL that starts with 'COPY' if (!sql.toUpperCase().startsWith("COPY ")) { - next(new Error("SQL must start with COPY")); + return next(new Error("SQL must start with COPY")); } next(); From 4cb7de73183ca5f897d6f902dc7faf4f0054ccdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 10:51:05 +0200 Subject: [PATCH 08/32] changing let for const --- app/services/stream_copy.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/stream_copy.js b/app/services/stream_copy.js index f9bd2a70..b5c24df7 100644 --- a/app/services/stream_copy.js +++ b/app/services/stream_copy.js @@ -7,7 +7,7 @@ const { Client } = require('pg'); module.exports = { to (res, sql, userDbParams, user, logger, cb) { - let metrics = new StreamCopyMetrics(logger, 'copyto', sql, user); + const metrics = new StreamCopyMetrics(logger, 'copyto', sql, user); const pg = new PSQL(userDbParams); pg.connect(function (err, client, done) { @@ -66,7 +66,7 @@ module.exports = { }, from (req, sql, userDbParams, user, gzip, logger, cb) { - let metrics = new StreamCopyMetrics(logger, 'copyfrom', sql, user, gzip); + const metrics = new StreamCopyMetrics(logger, 'copyfrom', sql, user, gzip); const pg = new PSQL(userDbParams); pg.connect(function (err, client, done) { @@ -74,7 +74,7 @@ module.exports = { return cb(err); } - let copyFromStream = copyFrom(sql); + const copyFromStream = copyFrom(sql); const pgstream = client.query(copyFromStream); pgstream .on('error', err => { From 0757d11bd04d104190468cc44d7ad564fc5f50a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 18:10:41 +0200 Subject: [PATCH 09/32] more deterministic tests when client disconnects --- test/acceptance/copy-endpoints.js | 71 ++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 61a37d79..9e9878e2 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -5,6 +5,7 @@ const querystring = require('querystring'); const assert = require('../support/assert'); const os = require('os'); const { Client } = require('pg'); +const request = require('request'); const StatsClient = require('../../app/stats/client'); if (global.settings.statsd) { @@ -314,7 +315,7 @@ describe('copy-endpoints', function() { describe('client disconnection', function() { // Give it enough time to connect and issue the query // but not too much so as to disconnect in the middle of the query. - const client_disconnect_timeout = 10; + const CLIENT_DISCONNECT_TIMEOUT = 10; before(function() { this.db_pool_size = global.settings.db_pool_size; @@ -340,35 +341,53 @@ describe('copy-endpoints', function() { }; it('COPY TO returns the connection to the pool if the client disconnects', function(done) { - assert.response(server, { - url: '/api/v1/sql/copyto?' + querystring.stringify({ - q: 'COPY (SELECT * FROM generate_series(1, 100000)) TO STDOUT', - }), - headers: { host: 'vizzuality.cartodb.com' }, - method: 'GET', - timeout: client_disconnect_timeout - }, {}, function(err) { - // we're expecting a timeout error - assert.ok(err); - assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); - assertCanReuseConnection(done); + const listener = server.listen(0, '127.0.0.1'); + + listener.on('error', done); + listener.on('listening', function onServerListening () { + + const { address, port } = listener.address(); + const query = querystring.stringify({ + q: `COPY (SELECT * FROM generate_series(1, 1000)) TO STDOUT` + }); + + const options = { + url: `http://${address}:${port}/api/v1/sql/copyto?${query}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + const req = request(options); + + req.once('data', () => req.abort()); + req.on('end', () => assertCanReuseConnection(done)); }); }); it('COPY FROM returns the connection to the pool if the client disconnects', function(done) { - assert.response(server, { - url: '/api/v1/sql/copyfrom?' + querystring.stringify({ - q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)", - }), - headers: { host: 'vizzuality.cartodb.com' }, - method: 'POST', - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), - timeout: client_disconnect_timeout - }, {}, function(err) { - // we're expecting a timeout error - assert.ok(err); - assert.ok(err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT'); - assertCanReuseConnection(done); + const listener = server.listen(0, '127.0.0.1'); + + listener.on('error', done); + listener.on('listening', function onServerListening () { + + const { address, port } = listener.address(); + const query = querystring.stringify({ + q: `COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)` + }); + + const options = { + url: `http://${address}:${port}/api/v1/sql/copyfrom?${query}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'POST', + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv') + }; + + const req = request(options); + + setTimeout(() => { + req.abort(); + done(); + }, CLIENT_DISCONNECT_TIMEOUT); }); }); From d4e03303f916db5ef2d632ba528d42c81783b128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 18:25:38 +0200 Subject: [PATCH 10/32] done after response end in test copy to --- test/acceptance/copy-endpoints.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 9e9878e2..df399025 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -360,7 +360,11 @@ describe('copy-endpoints', function() { const req = request(options); req.once('data', () => req.abort()); - req.on('end', () => assertCanReuseConnection(done)); + req.on('response', response => { + response.on('end', () => { + assertCanReuseConnection(done) + }); + }); }); }); From 39e5395edccdf3cff62b25994cc48e0254f45b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 18:29:03 +0200 Subject: [PATCH 11/32] change gzip to isGzip --- app/services/stream_copy.js | 8 ++++---- app/services/stream_copy_metrics.js | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/services/stream_copy.js b/app/services/stream_copy.js index b5c24df7..db23df90 100644 --- a/app/services/stream_copy.js +++ b/app/services/stream_copy.js @@ -65,8 +65,8 @@ module.exports = { }); }, - from (req, sql, userDbParams, user, gzip, logger, cb) { - const metrics = new StreamCopyMetrics(logger, 'copyfrom', sql, user, gzip); + from (req, sql, userDbParams, user, isGzip, logger, cb) { + const metrics = new StreamCopyMetrics(logger, 'copyfrom', sql, user, isGzip); const pg = new PSQL(userDbParams); pg.connect(function (err, client, done) { @@ -111,7 +111,7 @@ module.exports = { } }) .on('data', data => { - if (gzip) { + if (isGzip) { metrics.addGzipSize(data.length); } else { metrics.addSize(data.length); @@ -119,7 +119,7 @@ module.exports = { }) .on('end', () => requestEnded = true); - if (gzip) { + if (isGzip) { req .pipe(zlib.createGunzip()) .on('data', data => metrics.addSize(data.length)) diff --git a/app/services/stream_copy_metrics.js b/app/services/stream_copy_metrics.js index 24143a0d..7d6946be 100644 --- a/app/services/stream_copy_metrics.js +++ b/app/services/stream_copy_metrics.js @@ -1,12 +1,12 @@ const { getFormatFromCopyQuery } = require('../utils/query_info'); module.exports = class StreamCopyMetrics { - constructor(logger, type, sql, user, gzip = null) { + constructor(logger, type, sql, user, isGzip = null) { this.logger = logger; this.type = type; this.format = getFormatFromCopyQuery(sql); - this.gzip = gzip; + this.isGzip = isGzip; this.username = user; this.size = 0; this.gzipSize = 0; @@ -49,7 +49,7 @@ module.exports = class StreamCopyMetrics { this._log( this.startTime.toISOString(), - this.gzip && this.gzipSize ? this.gzipSize : null, + this.isGzip && this.gzipSize ? this.gzipSize : null, this.error ? this.error.message : null ); } @@ -60,7 +60,7 @@ module.exports = class StreamCopyMetrics { format: this.format, size: this.size, rows: this.rows, - gzip: this.gzip, + gzip: this.isGzip, username: this.username, time: this.time, timestamp From 9097cbe16e59e1557a745a837b86ba8f4afa653f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 18:29:50 +0200 Subject: [PATCH 12/32] adding log of the error when headers are already sent --- app/controllers/copy_controller.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index ce1836f6..1e4ece63 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -132,6 +132,7 @@ function validateCopyQuery () { function errorHandler () { return function errorHandlerMiddleware (err, req, res, next) { if (res.headersSent) { + console.error("EXCEPTION REPORT: " + err.stack); const errorHandler = errorHandlerFactory(err); res.write(JSON.stringify(errorHandler.getResponse())); res.end(); From c7e01f2ed654e8e847f9e78112a29b2ad8dd55d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 14 Jun 2018 18:40:06 +0200 Subject: [PATCH 13/32] success in copy log --- app/services/stream_copy_metrics.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/services/stream_copy_metrics.js b/app/services/stream_copy_metrics.js index 7d6946be..95187008 100644 --- a/app/services/stream_copy_metrics.js +++ b/app/services/stream_copy_metrics.js @@ -16,6 +16,7 @@ module.exports = class StreamCopyMetrics { this.endTime = null; this.time = null; + this.success = true; this.error = null; this.ended = false; @@ -72,8 +73,11 @@ module.exports = class StreamCopyMetrics { if (errorMessage) { logData.error = errorMessage; + this.success = false; } + logData.success = this.success; + this.logger.info(logData); } }; From 8a450a862ca2005ae0fc80abeb5a600a17323792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 15 Jun 2018 13:25:47 +0200 Subject: [PATCH 14/32] recover the metrics header (needed for tests purposes) --- app/controllers/copy_controller.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index 1e4ece63..7ad8bba7 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -93,16 +93,23 @@ function handleCopyFrom (logger) { res.locals.user, req.get('content-encoding') === 'gzip', logger, - function(err, metrics) { + function(err, metrics) { if (err) { return next(err); - } - - const { time, rows } = metrics; + } + + // TODO: change when data-ingestion log works: const { time, rows } = metrics; + const { time, rows, type, format, isGzip, size } = metrics; if (!rows) { return next(new Error("No rows copied")); } + // TODO: remove when data-ingestion log works + if (req.profiler) { + req.profiler.add({copyFrom: { type, format, gzip: isGzip, size, rows, time }}); + res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); + } + res.send({ time, total_rows: rows From 8f727fc15a821227d9ebb604f220917b6791b3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 15 Jun 2018 13:30:42 +0200 Subject: [PATCH 15/32] cdb-user in logs --- app/services/stream_copy_metrics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/stream_copy_metrics.js b/app/services/stream_copy_metrics.js index 95187008..77f7ac06 100644 --- a/app/services/stream_copy_metrics.js +++ b/app/services/stream_copy_metrics.js @@ -62,7 +62,7 @@ module.exports = class StreamCopyMetrics { size: this.size, rows: this.rows, gzip: this.isGzip, - username: this.username, + 'cdb-user': this.username, time: this.time, timestamp }; From 7dec3f4e95f999005f080116526e681a6352dce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 15 Jun 2018 13:47:37 +0200 Subject: [PATCH 16/32] jshint happy --- test/acceptance/copy-endpoints.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index df399025..645979f6 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -362,7 +362,7 @@ describe('copy-endpoints', function() { req.once('data', () => req.abort()); req.on('response', response => { response.on('end', () => { - assertCanReuseConnection(done) + assertCanReuseConnection(done); }); }); }); From bd499b88e58cb9d85df496639478d40fe32f2860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 18 Jun 2018 10:53:44 +0200 Subject: [PATCH 17/32] Add tests to check copyto cancel works properly --- test/acceptance/copy-abort.js | 229 ++++++++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 test/acceptance/copy-abort.js diff --git a/test/acceptance/copy-abort.js b/test/acceptance/copy-abort.js new file mode 100644 index 00000000..a9f6c84c --- /dev/null +++ b/test/acceptance/copy-abort.js @@ -0,0 +1,229 @@ +const querystring = require('querystring'); +const StatsClient = require('../../app/stats/client'); +const statsClient = StatsClient.getInstance(global.settings.statsd); +const server = require('../../app/server')(statsClient); +const request = require('request'); +const assert = require('assert'); + +const copyQuery = `COPY ( + INSERT INTO copy_to_test + SELECT updated_at + FROM generate_series('1984-06-14 01:00:00'::timestamp, '2018-06-14 01:00:00'::timestamp, '1 hour'::interval) updated_at + RETURNING updated_at +) TO STDOUT`; + +const createTableQuery = `CREATE TABLE copy_to_test AS + (SELECT '2018-06-15 14:49:05.126415+00'::timestamp AS updated_at)`; + +const dropTableQuery = `DROP TABLE copy_to_test`; + +const countQuery = `SELECT count(1) as count FROM copy_to_test`; + +function countInsertedRows (host, port, callback) { + setTimeout(function () { + const count = querystring.stringify({ q: countQuery, api_key: 1234 }); + + const options = { + url: `http://${host}:${port}/api/v1/sql?${count}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + request(options, function (err, res, body) { + if (err) { + return callback(err); + } + + assert.equal(res.statusCode, 200); + + const result = JSON.parse(body) + + callback(null, result); + }); + }, 1000); +} + +describe.only('Cancel "copy to" commands', function () { + + beforeEach(function (done) { + this.listener = server.listen(0, '127.0.0.1'); + + this.listener.on('error', done); + + this.listener.on('listening', () => { + const { address, port } = this.listener.address(); + + this.host = address; + this.port = port; + + done(); + }); + }); + + beforeEach(function (done) { + const { host, port } = this; + + const createTable = querystring.stringify({ q: createTableQuery, api_key: 1234}); + + const createTableOptions = { + url: `http://${host}:${port}/api/v1/sql?${createTable}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + request(createTableOptions, function (err, res) { + if (err) { + return done(err); + } + + assert.equal(res.statusCode, 200); + + done(); + }); + }); + + afterEach(function (done) { + const { host, port } = this; + + const dropTable = querystring.stringify({ q: dropTableQuery, api_key: 1234 }); + + const createTableOptions = { + url: `http://${host}:${port}/api/v1/sql?${dropTable}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + request(createTableOptions, function (err, res) { + if (err) { + return done(err); + } + + assert.equal(res.statusCode, 200); + + done(); + }); + }); + + afterEach(function (done) { + this.listener.close(done); + }); + + it('abort on response', function (done) { + const { host, port } = this; + + const copy = querystring.stringify({ q: copyQuery, api_key: 1234 }); + + const options = { + url: `http://${host}:${port}/api/v1/sql/copyto?${copy}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + const req = request(options); + + req.on('response', function () { + req.abort(); + + countInsertedRows(host, port, function (err, result) { + if (err) { + return done(err); + } + + assert.equal(result.rows[0].count, 1); + + done(); + }); + }); + }); + + it('abort on data', function (done) { + const { host, port } = this; + + const copy = querystring.stringify({ q: copyQuery, api_key: 1234 }); + + const options = { + url: `http://${host}:${port}/api/v1/sql/copyto?${copy}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + const req = request(options); + + req.once('data', function () { + req.abort(); + + countInsertedRows(host, port, function (err, result) { + if (err) { + return done(err); + } + + assert.equal(result.rows[0].count, 1); + + done(); + }); + }); + }); + + + it('destroy on data', function (done) { + const { host, port } = this; + + const copy = querystring.stringify({ q: copyQuery, api_key: 1234 }); + + const options = { + url: `http://${host}:${port}/api/v1/sql/copyto?${copy}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + const req = request(options); + + let response; + + req.on('response', function (res) { + response = res; + }); + + req.once('data', function () { + response.destroy(); + + countInsertedRows(host, port, function (err, result) { + if (err) { + return done(err); + } + + assert.equal(result.rows[0].count, 1); + + done(); + }); + }); + }); + + it('destroy on response', function (done) { + const { host, port } = this; + + const copy = querystring.stringify({ q: copyQuery, api_key: 1234 }); + + const options = { + url: `http://${host}:${port}/api/v1/sql/copyto?${copy}`, + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }; + + const req = request(options); + + req.on('response', function (response) { + response.destroy(); + + countInsertedRows(host, port, function (err, result) { + if (err) { + return done(err); + } + + assert.equal(result.rows[0].count, 1); + + done(); + }); + }); + }); +}); From 258baa37920ac6c14b26c1ecf401ca8c327c27c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 18 Jun 2018 10:56:14 +0200 Subject: [PATCH 18/32] Remove test filter --- test/acceptance/copy-abort.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/copy-abort.js b/test/acceptance/copy-abort.js index a9f6c84c..7a7ada0c 100644 --- a/test/acceptance/copy-abort.js +++ b/test/acceptance/copy-abort.js @@ -43,7 +43,7 @@ function countInsertedRows (host, port, callback) { }, 1000); } -describe.only('Cancel "copy to" commands', function () { +describe('Cancel "copy to" commands', function () { beforeEach(function (done) { this.listener = server.listen(0, '127.0.0.1'); From 32dca8dfdb5b79bf691988e1c8f5184fbac97b5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 18 Jun 2018 18:47:12 +0200 Subject: [PATCH 19/32] reopenFileStreams to parent class --- app/services/logger.js | 5 +++++ batch/batch-logger.js | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/logger.js b/app/services/logger.js index 87b6cba1..400941b3 100644 --- a/app/services/logger.js +++ b/app/services/logger.js @@ -28,6 +28,11 @@ class Logger { warn (log, message) { this.logger.warn(log, message); } + + reopenFileStreams () { + console.log('Reloading log file', this.path); + this.logger.reopenFileStreams(); + } } module.exports = Logger; diff --git a/batch/batch-logger.js b/batch/batch-logger.js index ef132bd2..11d1ade8 100644 --- a/batch/batch-logger.js +++ b/batch/batch-logger.js @@ -11,9 +11,6 @@ class BatchLogger extends Logger { return job.log(this.logger); } - reopenFileStreams () { - this.logger.reopenFileStreams(); - } } module.exports = BatchLogger; From 7727a9d506174043ac0e739cbcd947add95ad6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 18 Jun 2018 18:48:11 +0200 Subject: [PATCH 20/32] data ingestion logger created in controller and added to app --- app/controllers/copy_controller.js | 6 ++---- app/server.js | 7 ++++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index 7ad8bba7..13ea9ef0 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -8,17 +8,15 @@ const timeoutLimitsMiddleware = require('../middlewares/timeout-limits'); const { initializeProfilerMiddleware } = require('../middlewares/profiler'); const rateLimitsMiddleware = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimitsMiddleware; -const Logger = require('../services/logger'); const errorHandlerFactory = require('../services/error_handler_factory'); const streamCopy = require('../services/stream_copy'); -function CopyController(metadataBackend, userDatabaseService, userLimitsService, statsClient) { +function CopyController(metadataBackend, userDatabaseService, userLimitsService, statsClient, logger) { this.metadataBackend = metadataBackend; this.userDatabaseService = userDatabaseService; this.userLimitsService = userLimitsService; this.statsClient = statsClient; - - this.logger = new Logger(global.settings.dataIngestionLogPath, 'data-ingestion'); + this.logger = logger; } CopyController.prototype.route = function (app) { diff --git a/app/server.js b/app/server.js index dbaedc78..dabb5773 100644 --- a/app/server.js +++ b/app/server.js @@ -28,6 +28,7 @@ var JobQueue = require('../batch/job_queue'); var JobBackend = require('../batch/job_backend'); var JobCanceller = require('../batch/job_canceller'); var JobService = require('../batch/job_service'); +const Logger = require('./services/logger'); var cors = require('./middlewares/cors'); @@ -154,6 +155,9 @@ function App(statsClient) { }; const userLimitsService = new UserLimitsService(metadataBackend, userLimitsServiceOptions); + const dataIngestionLogger = new Logger(global.settings.dataIngestionLogPath, 'data-ingestion'); + app.dataIngestionLogger = dataIngestionLogger; + var jobPublisher = new JobPublisher(redisPool); var jobQueue = new JobQueue(metadataBackend, jobPublisher); var jobBackend = new JobBackend(metadataBackend, jobQueue); @@ -175,7 +179,8 @@ function App(statsClient) { var copyController = new CopyController( metadataBackend, userDatabaseService, - userLimitsService + userLimitsService, + dataIngestionLogger ); copyController.route(app); From b4724d74087efa98f94659e4ecd907aa806c65c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 18 Jun 2018 18:48:38 +0200 Subject: [PATCH 21/32] data ingestion logger reopenFileStreams --- app.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app.js b/app.js index 245ddef6..ce79b463 100755 --- a/app.js +++ b/app.js @@ -115,6 +115,10 @@ process.on('SIGHUP', function() { if (server.batch && server.batch.logger) { server.batch.logger.reopenFileStreams(); } + + if (server.dataIngestionLogger) { + server.dataIngestionLogger.reopenFileStreams(); + } }); process.on('SIGTERM', function () { From 30402f2e890e2f38dd2900383962312d89a48075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 19 Jun 2018 10:04:17 +0200 Subject: [PATCH 22/32] remove unused statsClient --- app/controllers/copy_controller.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index 13ea9ef0..4e1d4116 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -11,11 +11,10 @@ const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimitsMiddleware; const errorHandlerFactory = require('../services/error_handler_factory'); const streamCopy = require('../services/stream_copy'); -function CopyController(metadataBackend, userDatabaseService, userLimitsService, statsClient, logger) { +function CopyController(metadataBackend, userDatabaseService, userLimitsService, logger) { this.metadataBackend = metadataBackend; this.userDatabaseService = userDatabaseService; this.userLimitsService = userLimitsService; - this.statsClient = statsClient; this.logger = logger; } From 3d30f1f762747bea98ad6ee496afe1f215c854be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 19 Jun 2018 10:34:52 +0200 Subject: [PATCH 23/32] trailing white spaces --- app/controllers/copy_controller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/copy_controller.js b/app/controllers/copy_controller.js index 4e1d4116..9b29cfec 100644 --- a/app/controllers/copy_controller.js +++ b/app/controllers/copy_controller.js @@ -102,9 +102,9 @@ function handleCopyFrom (logger) { } // TODO: remove when data-ingestion log works - if (req.profiler) { - req.profiler.add({copyFrom: { type, format, gzip: isGzip, size, rows, time }}); - res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); + if (req.profiler) { + req.profiler.add({copyFrom: { type, format, gzip: isGzip, size, rows, time }}); + res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); } res.send({ From 16c2fd728f895bb5a4cbce59092f4469ffc361c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 10:48:03 +0200 Subject: [PATCH 24/32] js hint happy --- test/acceptance/copy-abort.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/acceptance/copy-abort.js b/test/acceptance/copy-abort.js index 7a7ada0c..28ddb34a 100644 --- a/test/acceptance/copy-abort.js +++ b/test/acceptance/copy-abort.js @@ -8,7 +8,11 @@ const assert = require('assert'); const copyQuery = `COPY ( INSERT INTO copy_to_test SELECT updated_at - FROM generate_series('1984-06-14 01:00:00'::timestamp, '2018-06-14 01:00:00'::timestamp, '1 hour'::interval) updated_at + FROM generate_series( + '1984-06-14 01:00:00'::timestamp, + '2018-06-14 01:00:00'::timestamp, + '1 hour'::interval + ) updated_at RETURNING updated_at ) TO STDOUT`; @@ -35,9 +39,7 @@ function countInsertedRows (host, port, callback) { } assert.equal(res.statusCode, 200); - - const result = JSON.parse(body) - + const result = JSON.parse(body); callback(null, result); }); }, 1000); From 5021c8d466ab93674da086e7d85155c0b9c02a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 11:08:41 +0200 Subject: [PATCH 25/32] making copy test independent --- test/acceptance/copy-endpoints.js | 118 ++++++++++++++++-------------- 1 file changed, 62 insertions(+), 56 deletions(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 645979f6..4b8b9be0 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -19,22 +19,28 @@ const statsClient = StatsClient.getInstance(global.settings.statsd); const server = require('../../app/server')(statsClient); -describe('copy-endpoints', function() { - describe('general', function() { - before(function(done) { - const client = new Client({ - user: 'postgres', - host: 'localhost', - database: 'cartodb_test_user_1_db', - port: 5432, - }); - client.connect(); - client.query('TRUNCATE copy_endpoints_test', (err/*, res */) => { - client.end(); - done(err); - }); +describe.only('copy-endpoints', function() { + before(function() { + this.client = new Client({ + user: 'postgres', + host: 'localhost', + database: 'cartodb_test_user_1_db', + port: 5432, }); - + this.client.connect(); + }); + + after(function() { + this.client.end(); + }) + + afterEach(function (done) { + this.client.query('TRUNCATE copy_endpoints_test', err => { + done(err); + }); + }); + + describe('general', function() { it('should work with copyfrom endpoint', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ @@ -47,11 +53,11 @@ describe('copy-endpoints', function() { assert.ifError(err); const response = JSON.parse(res.body); assert.equal(!!response.time, true); - assert.strictEqual(response.total_rows, 6); + assert.strictEqual(response.total_rows, 6); done(); }); }); - + it('should fail with copyfrom endpoint and unexisting table', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ @@ -63,7 +69,7 @@ describe('copy-endpoints', function() { },{}, function(err, res) { assert.ifError(err); assert.deepEqual( - JSON.parse(res.body), + JSON.parse(res.body), { error:['relation \"unexisting_table\" does not exist'] } @@ -71,7 +77,7 @@ describe('copy-endpoints', function() { done(); }); }); - + it('should fail with copyfrom endpoint and without csv', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ @@ -82,7 +88,7 @@ describe('copy-endpoints', function() { },{}, function(err, res) { assert.ifError(err); assert.deepEqual( - JSON.parse(res.body), + JSON.parse(res.body), { error:['No rows copied'] } @@ -90,17 +96,17 @@ describe('copy-endpoints', function() { done(); }); }); - + it('should fail with copyfrom endpoint and without q', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom", - data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), headers: {host: 'vizzuality.cartodb.com'}, method: 'POST' },{}, function(err, res) { assert.ifError(err); assert.deepEqual( - JSON.parse(res.body), + JSON.parse(res.body), { error:["SQL is missing"] } @@ -108,7 +114,7 @@ describe('copy-endpoints', function() { done(); }); }); - + it('should work with copyto endpoint', function(done){ assert.response(server, { url: "/api/v1/sql/copyto?" + querystring.stringify({ @@ -120,17 +126,17 @@ describe('copy-endpoints', function() { },{}, function(err, res) { assert.ifError(err); assert.strictEqual( - res.body, + res.body, '11\tPaul\t10\n12\tPeter\t10\n13\tMatthew\t10\n14\t\\N\t10\n15\tJames\t10\n16\tJohn\t10\n' ); - + assert.equal(res.headers['content-disposition'], 'attachment; filename=%2Ftmp%2Foutput.dmp'); assert.equal(res.headers['content-type'], 'application/octet-stream'); - + done(); }); }); - + it('should fail with copyto endpoint and without sql', function(done){ assert.response(server, { url: "/api/v1/sql/copyto?" + querystring.stringify({ @@ -141,7 +147,7 @@ describe('copy-endpoints', function() { },{}, function(err, res) { assert.ifError(err); assert.deepEqual( - JSON.parse(res.body), + JSON.parse(res.body), { error:["SQL is missing"] } @@ -149,15 +155,15 @@ describe('copy-endpoints', function() { done(); }); }); - + it('should work with copyfrom and gzip', function(done){ assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: "COPY copy_endpoints_test2 (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" + q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" }), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv.gz'), headers: { - host: 'vizzuality.cartodb.com', + host: 'vizzuality.cartodb.com', 'content-encoding': 'gzip' }, method: 'POST' @@ -169,11 +175,11 @@ describe('copy-endpoints', function() { done(); }); }); - + }); - - - describe('timeout', function() { + + + describe('timeout', function() { it('should fail with copyfrom and timeout', function(done){ assert.response(server, { url: '/api/v1/sql?q=set statement_timeout = 10', @@ -184,7 +190,7 @@ describe('copy-endpoints', function() { assert.ifError(err); assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: `COPY copy_endpoints_test (id, name) + q: `COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)` }), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), @@ -204,8 +210,8 @@ describe('copy-endpoints', function() { context: 'limit', detail: 'datasource' }); - - + + assert.response(server, { url: "/api/v1/sql?q=set statement_timeout = 2000", headers: {host: 'vizzuality.cartodb.com'}, @@ -215,7 +221,7 @@ describe('copy-endpoints', function() { }); }); }); - + it('should fail with copyto and timeout', function(done){ assert.response(server, { url: '/api/v1/sql?q=set statement_timeout = 20', @@ -240,7 +246,7 @@ describe('copy-endpoints', function() { }; const expectedError = res.body.substring(res.body.length - JSON.stringify(error).length); assert.deepEqual(JSON.parse(expectedError), error); - + assert.response(server, { url: "/api/v1/sql?q=set statement_timeout = 2000", headers: {host: 'vizzuality.cartodb.com'}, @@ -251,24 +257,24 @@ describe('copy-endpoints', function() { }); }); }); - - + + describe('db connections', function() { before(function() { this.db_pool_size = global.settings.db_pool_size; global.settings.db_pool_size = 1; }); - + after(function() { global.settings.db_pool_size = this.db_pool_size; }); - + it('copyfrom', function(done) { function doCopyFrom() { return new Promise(resolve => { assert.response(server, { url: "/api/v1/sql/copyfrom?" + querystring.stringify({ - q: `COPY copy_endpoints_test (id, name) + q: `COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)` }), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), @@ -282,12 +288,12 @@ describe('copy-endpoints', function() { }); }); } - + Promise.all([doCopyFrom(), doCopyFrom(), doCopyFrom()]).then(function() { done(); }); }); - + it('copyto', function(done) { function doCopyTo() { return new Promise(resolve => { @@ -305,27 +311,27 @@ describe('copy-endpoints', function() { }); }); } - + Promise.all([doCopyTo(), doCopyTo(), doCopyTo()]).then(function() { done(); }); }); }); - + describe('client disconnection', function() { // Give it enough time to connect and issue the query // but not too much so as to disconnect in the middle of the query. const CLIENT_DISCONNECT_TIMEOUT = 10; - + before(function() { this.db_pool_size = global.settings.db_pool_size; global.settings.db_pool_size = 1; }); - + after(function() { global.settings.db_pool_size = this.db_pool_size; }); - + var assertCanReuseConnection = function (done) { assert.response(server, { url: '/api/v1/sql?' + querystring.stringify({ @@ -339,7 +345,7 @@ describe('copy-endpoints', function() { done(); }); }; - + it('COPY TO returns the connection to the pool if the client disconnects', function(done) { const listener = server.listen(0, '127.0.0.1'); @@ -367,7 +373,7 @@ describe('copy-endpoints', function() { }); }); }); - + it('COPY FROM returns the connection to the pool if the client disconnects', function(done) { const listener = server.listen(0, '127.0.0.1'); @@ -394,6 +400,6 @@ describe('copy-endpoints', function() { }, CLIENT_DISCONNECT_TIMEOUT); }); }); - + }); }); From c37ea82c0e93ea1b6060f537a8d5d401c7a3e8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 11:12:19 +0200 Subject: [PATCH 26/32] updating copyto test making them independent --- test/acceptance/copy-endpoints.js | 52 ++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 4b8b9be0..5db083c8 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -32,7 +32,7 @@ describe.only('copy-endpoints', function() { after(function() { this.client.end(); - }) + }); afterEach(function (done) { this.client.query('TRUNCATE copy_endpoints_test', err => { @@ -117,23 +117,34 @@ describe.only('copy-endpoints', function() { it('should work with copyto endpoint', function(done){ assert.response(server, { - url: "/api/v1/sql/copyto?" + querystring.stringify({ - q: 'COPY copy_endpoints_test TO STDOUT', - filename: '/tmp/output.dmp' + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" }), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{}, function(err, res) { + method: 'POST' + },{}, function(err) { assert.ifError(err); - assert.strictEqual( - res.body, - '11\tPaul\t10\n12\tPeter\t10\n13\tMatthew\t10\n14\t\\N\t10\n15\tJames\t10\n16\tJohn\t10\n' - ); - assert.equal(res.headers['content-disposition'], 'attachment; filename=%2Ftmp%2Foutput.dmp'); - assert.equal(res.headers['content-type'], 'application/octet-stream'); + assert.response(server, { + url: "/api/v1/sql/copyto?" + querystring.stringify({ + q: 'COPY copy_endpoints_test TO STDOUT', + filename: '/tmp/output.dmp' + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(err, res) { + assert.ifError(err); + assert.strictEqual( + res.body, + '11\tPaul\t10\n12\tPeter\t10\n13\tMatthew\t10\n14\t\\N\t10\n15\tJames\t10\n16\tJohn\t10\n' + ); - done(); + assert.equal(res.headers['content-disposition'], 'attachment; filename=%2Ftmp%2Foutput.dmp'); + assert.equal(res.headers['content-type'], 'application/octet-stream'); + + done(); + }); }); }); @@ -312,8 +323,19 @@ describe.only('copy-endpoints', function() { }); } - Promise.all([doCopyTo(), doCopyTo(), doCopyTo()]).then(function() { - done(); + assert.response(server, { + url: "/api/v1/sql/copyfrom?" + querystring.stringify({ + q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" + }), + data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'POST' + },{}, function(err) { + assert.ifError(err); + + Promise.all([doCopyTo(), doCopyTo(), doCopyTo()]).then(function() { + done(); + }); }); }); }); From 84c43344e47931d269e1959884e3f540fb5e5dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 13:18:32 +0200 Subject: [PATCH 27/32] jshgint happy --- test/acceptance/copy-endpoints.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 5db083c8..ae02cd39 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -32,7 +32,7 @@ describe.only('copy-endpoints', function() { after(function() { this.client.end(); - }); + }) afterEach(function (done) { this.client.query('TRUNCATE copy_endpoints_test', err => { From 20e43f6029efa7025a1139c7d6f6c9d1e4c39ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 14:43:50 +0200 Subject: [PATCH 28/32] ensuring query cancelation in copy from --- test/acceptance/copy-endpoints.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index ae02cd39..5b227ff5 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -19,7 +19,7 @@ const statsClient = StatsClient.getInstance(global.settings.statsd); const server = require('../../app/server')(statsClient); -describe.only('copy-endpoints', function() { +describe('copy-endpoints', function() { before(function() { this.client = new Client({ user: 'postgres', @@ -32,7 +32,7 @@ describe.only('copy-endpoints', function() { after(function() { this.client.end(); - }) + }); afterEach(function (done) { this.client.query('TRUNCATE copy_endpoints_test', err => { @@ -354,16 +354,18 @@ describe.only('copy-endpoints', function() { global.settings.db_pool_size = this.db_pool_size; }); - var assertCanReuseConnection = function (done) { + const assertCanReuseCanceledConnection = function (done) { assert.response(server, { url: '/api/v1/sql?' + querystring.stringify({ - q: 'SELECT 1', + q: 'SELECT count(*) FROM copy_endpoints_test', }), headers: { host: 'vizzuality.cartodb.com' }, method: 'GET' }, {}, function(err, res) { assert.ifError(err); assert.ok(res.statusCode === 200); + const result = JSON.parse(res.body); + assert.strictEqual(result.rows[0].count, 0); done(); }); }; @@ -390,7 +392,7 @@ describe.only('copy-endpoints', function() { req.once('data', () => req.abort()); req.on('response', response => { response.on('end', () => { - assertCanReuseConnection(done); + assertCanReuseCanceledConnection(done); }); }); }); @@ -418,7 +420,7 @@ describe.only('copy-endpoints', function() { setTimeout(() => { req.abort(); - done(); + assertCanReuseCanceledConnection(done); }, CLIENT_DISCONNECT_TIMEOUT); }); }); From 5b87defb9736a3534168928f0696b60b0b09c731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 15:09:37 +0200 Subject: [PATCH 29/32] remove unneeded test table copy_endpoints_test2 --- test/support/sql/test.sql | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/support/sql/test.sql b/test/support/sql/test.sql index 711e6183..b813f797 100644 --- a/test/support/sql/test.sql +++ b/test/support/sql/test.sql @@ -226,12 +226,3 @@ CREATE TABLE copy_endpoints_test ( ); GRANT ALL ON TABLE copy_endpoints_test TO :TESTUSER; GRANT ALL ON TABLE copy_endpoints_test TO :PUBLICUSER; - -DROP TABLE IF EXISTS copy_endpoints_test2; -CREATE TABLE copy_endpoints_test2 ( - id integer, - name text, - age integer default 10 -); -GRANT ALL ON TABLE copy_endpoints_test2 TO :TESTUSER; -GRANT ALL ON TABLE copy_endpoints_test2 TO :PUBLICUSER; From 50da363ae07be91311ebef9b5fe6e8e88d5fdd7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 18:39:17 +0200 Subject: [PATCH 30/32] improving name dropTableOptions --- test/acceptance/copy-abort.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/copy-abort.js b/test/acceptance/copy-abort.js index 28ddb34a..b7a8732c 100644 --- a/test/acceptance/copy-abort.js +++ b/test/acceptance/copy-abort.js @@ -89,13 +89,13 @@ describe('Cancel "copy to" commands', function () { const dropTable = querystring.stringify({ q: dropTableQuery, api_key: 1234 }); - const createTableOptions = { + const dropTableOptions = { url: `http://${host}:${port}/api/v1/sql?${dropTable}`, headers: { host: 'vizzuality.cartodb.com' }, method: 'GET' }; - request(createTableOptions, function (err, res) { + request(dropTableOptions, function (err, res) { if (err) { return done(err); } From 030a8f1ef48e296bc87d9c3d6347168364167204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 19:02:34 +0200 Subject: [PATCH 31/32] checking connection reuse properly --- test/acceptance/copy-endpoints.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index 5b227ff5..b7a288fe 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -354,6 +354,20 @@ describe('copy-endpoints', function() { global.settings.db_pool_size = this.db_pool_size; }); + const assertCanReuseConnection = function (done) { + assert.response(server, { + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT 1', + }), + headers: { host: 'vizzuality.cartodb.com' }, + method: 'GET' + }, {}, function(err, res) { + assert.ifError(err); + assert.ok(res.statusCode === 200); + done(); + }); + }; + const assertCanReuseCanceledConnection = function (done) { assert.response(server, { url: '/api/v1/sql?' + querystring.stringify({ @@ -392,7 +406,7 @@ describe('copy-endpoints', function() { req.once('data', () => req.abort()); req.on('response', response => { response.on('end', () => { - assertCanReuseCanceledConnection(done); + assertCanReuseConnection(done); }); }); }); From dd088215c104b66bade73bac7317637f91015e22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 20 Jun 2018 19:03:23 +0200 Subject: [PATCH 32/32] unify client disconnect timeout --- test/acceptance/copy-abort.js | 2 +- test/acceptance/copy-endpoints.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/copy-abort.js b/test/acceptance/copy-abort.js index b7a8732c..7765fc26 100644 --- a/test/acceptance/copy-abort.js +++ b/test/acceptance/copy-abort.js @@ -42,7 +42,7 @@ function countInsertedRows (host, port, callback) { const result = JSON.parse(body); callback(null, result); }); - }, 1000); + }, 100); } describe('Cancel "copy to" commands', function () { diff --git a/test/acceptance/copy-endpoints.js b/test/acceptance/copy-endpoints.js index b7a288fe..b4f5bfa1 100644 --- a/test/acceptance/copy-endpoints.js +++ b/test/acceptance/copy-endpoints.js @@ -343,7 +343,7 @@ describe('copy-endpoints', function() { describe('client disconnection', function() { // Give it enough time to connect and issue the query // but not too much so as to disconnect in the middle of the query. - const CLIENT_DISCONNECT_TIMEOUT = 10; + const CLIENT_DISCONNECT_TIMEOUT = 100; before(function() { this.db_pool_size = global.settings.db_pool_size;