From 26ec584f5f5dadc36f01e79c4cbeb1344ae3a25a Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sat, 26 May 2018 20:48:55 +0100 Subject: [PATCH 1/5] PlotlyAPI: use lowercase filenames --- backend/persistent/QueryScheduler.js | 2 +- backend/persistent/{PlotlyAPI.js => plotly-api.js} | 0 backend/routes.js | 2 +- package.json | 2 +- test/backend/QueryScheduler.spec.js | 2 +- test/backend/{PlotlyAPI.spec.js => plotly-api.spec.js} | 2 +- test/backend/utils.js | 2 +- 7 files changed, 6 insertions(+), 6 deletions(-) rename backend/persistent/{PlotlyAPI.js => plotly-api.js} (100%) rename test/backend/{PlotlyAPI.spec.js => plotly-api.spec.js} (98%) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 94e3eda60..e73aef205 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -3,7 +3,7 @@ import {getConnectionById} from './Connections'; import {getQuery, getQueries, saveQuery, deleteQuery} from './Queries'; import {getSetting} from '../settings'; import Logger from '../logger'; -import {PlotlyAPIRequest, updateGrid} from './PlotlyAPI'; +import {PlotlyAPIRequest, updateGrid} from './plotly-api.js'; import {has} from 'ramda'; class QueryScheduler { diff --git a/backend/persistent/PlotlyAPI.js b/backend/persistent/plotly-api.js similarity index 100% rename from backend/persistent/PlotlyAPI.js rename to backend/persistent/plotly-api.js diff --git a/backend/routes.js b/backend/routes.js index dddaa603a..a71e68899 100644 --- a/backend/routes.js +++ b/backend/routes.js @@ -23,7 +23,7 @@ import QueryScheduler from './persistent/QueryScheduler.js'; import {getSetting, saveSetting} from './settings.js'; import {generateAndSaveAccessToken} from './utils/authUtils'; import {getAccessTokenCookieOptions, getCookieOptions} from './constants'; -import {checkWritePermissions, newDatacache} from './persistent/PlotlyAPI.js'; +import {checkWritePermissions, newDatacache} from './persistent/plotly-api.js'; import {contains, keys, isEmpty, merge, pluck} from 'ramda'; import {getCerts, timeoutFetchAndSaveCerts, setRenewalJob} from './certificates'; import Logger from './logger'; diff --git a/package.json b/package.json index 510c95dd0..4008eabbd 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test-unit-oauth2": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/routes.oauth2.spec.js", "test-unit-oracle": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.oracle.spec.js", "test-unit-oracle:node": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.oracle.spec.js", - "test-unit-plotly": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/PlotlyAPI.spec.js", + "test-unit-plotly": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/plotly-api.spec.js", "test-unit-scheduler": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/QueryScheduler.spec.js", "test-unit-routes": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/routes.spec.js", "test-unit-mock": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.mock.spec.js", diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index a3d12682e..9e3a408ff 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -6,7 +6,7 @@ import {merge} from 'ramda'; import QueryScheduler from '../../backend/persistent/QueryScheduler.js'; import {saveConnection} from '../../backend/persistent/Connections.js'; import {getQueries} from '../../backend/persistent/Queries.js'; -import {PlotlyAPIRequest, updateGrid} from '../../backend/persistent/PlotlyAPI.js'; +import {PlotlyAPIRequest, updateGrid} from '../../backend/persistent/plotly-api.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; import { apiKey, diff --git a/test/backend/PlotlyAPI.spec.js b/test/backend/plotly-api.spec.js similarity index 98% rename from test/backend/PlotlyAPI.spec.js rename to test/backend/plotly-api.spec.js index 5911657e8..d5b5f4a2e 100644 --- a/test/backend/PlotlyAPI.spec.js +++ b/test/backend/plotly-api.spec.js @@ -3,7 +3,7 @@ import {assert} from 'chai'; import { PlotlyAPIRequest, updateGrid -} from '../../backend/persistent/PlotlyAPI.js'; +} from '../../backend/persistent/plotly-api.js'; import {saveSetting} from '../../backend/settings.js'; import { apiKey, diff --git a/test/backend/utils.js b/test/backend/utils.js index 6fb07ed17..700954ec3 100644 --- a/test/backend/utils.js +++ b/test/backend/utils.js @@ -9,7 +9,7 @@ const restify = require('restify'); import {dissoc, merge} from 'ramda'; -import {PlotlyAPIRequest} from '../../backend/persistent/PlotlyAPI.js'; +import {PlotlyAPIRequest} from '../../backend/persistent/plotly-api.js'; import Servers from '../../backend/routes.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; From c26fa9d47e5532183d69b2118c4dd509b91eab7c Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sat, 26 May 2018 22:11:47 +0100 Subject: [PATCH 2/5] settings: add getCredentials(username) * Refactor code to retrieve a user's credentials into a function. --- backend/persistent/QueryScheduler.js | 36 ++++++++----- backend/persistent/plotly-api.js | 81 ++++++++++++---------------- backend/settings.js | 11 +++- 3 files changed, 66 insertions(+), 62 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index e73aef205..fa3ad473b 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -1,11 +1,23 @@ -import * as Connections from './datastores/Datastores'; -import {getConnectionById} from './Connections'; -import {getQuery, getQueries, saveQuery, deleteQuery} from './Queries'; -import {getSetting} from '../settings'; -import Logger from '../logger'; -import {PlotlyAPIRequest, updateGrid} from './plotly-api.js'; import {has} from 'ramda'; +import {getConnectionById} from './Connections.js'; +import * as Connections from './datastores/Datastores.js'; +import Logger from '../logger'; +import { + getQuery, + getQueries, + saveQuery, + deleteQuery +} from './Queries.js'; +import { + getCredentials, + getSetting +} from '../settings.js'; +import { + PlotlyAPIRequest, + updateGrid +} from './plotly-api.js'; + class QueryScheduler { constructor() { this.scheduleQuery = this.scheduleQuery.bind(this); @@ -118,16 +130,12 @@ class QueryScheduler { * If the user is the owner, then requestor === fid.split(':')[0] * If the user is a collaborator, then requestor is different */ - const username = requestor; - const user = getSetting('USERS').find( - u => u.username === username - ); + const {username, apiKey, accessToken} = getCredentials(requestor); // Check if the user even exists - if (!user || !(user.apiKey || user.accessToken)) { + if (!username || !(apiKey || accessToken)) { /* - * Heads up - the front end looks for "Unauthenticated" in this - * error message. So don't change it! + * Warning: The front end looks for "Unauthenticated" in this error message. Don't change it! */ const errorMessage = ( `Unauthenticated: Attempting to update grid ${fid} but the ` + @@ -136,7 +144,7 @@ class QueryScheduler { Logger.log(errorMessage, 0); throw new Error(errorMessage); } - const {apiKey, accessToken} = user; + // Check if the credentials are valid return PlotlyAPIRequest('users/current', { method: 'GET', username, apiKey, accessToken diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index c1e33cecd..3d57115d4 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -1,8 +1,13 @@ import fetch from 'node-fetch'; -import {getSetting} from '../settings.js'; -import Logger from '../logger'; import FormData from 'form-data'; +import Logger from '../logger.js'; +import { + getCredentials, + getSetting +} from '../settings.js'; + + // Module to access Plot.ly REST API // // See API documentation at https://api.plot.ly/v2/ @@ -31,19 +36,15 @@ export function PlotlyAPIRequest(relativeUrl, {body, username, apiKey, accessTok } export function newDatacache(payloadJSON, type, requestor) { - const form = new FormData(); - form.append('type', type); - form.append('origin', 'Falcon'); - form.append('payload', payloadJSON); - const body = form; - - const users = getSetting('USERS'); - const user = users.find( - u => u.username === requestor - ); - - if (user) { - form.append('username', user.username); + const {username, apiKey, accessToken} = getCredentials(requestor); + + const body = new FormData(); + body.append('type', type); + body.append('origin', 'Falcon'); + body.append('payload', payloadJSON); + + if (username) { + body.append('username', username); } /* @@ -52,17 +53,12 @@ export function newDatacache(payloadJSON, type, requestor) { * to proceed with blank `Authorization` header. */ let authorization = ''; - if (user) { - const apiKey = user.apiKey; - const accessToken = user.accessToken; - - if (apiKey) { - authorization = 'Basic ' + new Buffer( - requestor + ':' + apiKey - ).toString('base64'); - } else if (accessToken) { - authorization = `Bearer ${accessToken}`; - } + if (apiKey) { + authorization = 'Basic ' + new Buffer( + requestor + ':' + apiKey + ).toString('base64'); + } else if (accessToken) { + authorization = `Bearer ${accessToken}`; } return fetch(`${getSetting('PLOTLY_URL')}/datacache`, { @@ -85,13 +81,7 @@ export function newDatacache(payloadJSON, type, requestor) { } export function updateGrid(rows, fid, uids, requestor) { - const username = requestor; - const users = getSetting('USERS'); - const user = users.find( - u => u.username === username - ); - const apiKey = user.apiKey; - const accessToken = user.accessToken; + const {username, apiKey, accessToken} = getCredentials(requestor); // TODO - Test case where no rows are returned. if (uids.length !== rows[0].length) { @@ -129,16 +119,12 @@ export function updateGrid(rows, fid, uids, requestor) { // Resolve if the requestor has permission to update fid, reject otherwise export function checkWritePermissions(fid, requestor) { - const owner = fid.split(':')[0]; - const user = getSetting('USERS').find( - u => u.username === requestor - ); + const {username, apiKey, accessToken} = getCredentials(requestor); // Check if the user even exists - if (!user || !(user.apiKey || user.accessToken)) { + if (!username || !(apiKey || accessToken)) { /* - * Heads up - the front end looks for "Unauthenticated" in this - * error message. So don't change it! + * Warning: The front end looks for "Unauthenticated" in this error message. Don't change it! */ const errorMessage = ( `Unauthenticated: Attempting to update grid ${fid} but the ` + @@ -147,7 +133,7 @@ export function checkWritePermissions(fid, requestor) { Logger.log(errorMessage, 0); throw new Error(errorMessage); } - const {apiKey, accessToken} = user; + return PlotlyAPIRequest(`grids/${fid}`, { username: requestor, apiKey, @@ -166,14 +152,17 @@ export function checkWritePermissions(fid, requestor) { return res.json(); } }).then(function(filemeta) { - if (filemeta.collaborators && + const owner = fid.split(':')[0]; + + if (owner === requestor) { + return Promise.resolve(); + } else if (filemeta.collaborators && filemeta.collaborators.results && - Boolean(filemeta.collaborators.results.find(collab => requestor === collab.username)) + filemeta.collaborators.results.find(collab => requestor === collab.username) ) { - return new Promise(function(resolve) {resolve();}); - } else if (owner === requestor) { - return new Promise(function(resolve) {resolve();}); + return Promise.resolve(); } + throw new Error('Permission denied'); }); } diff --git a/backend/settings.js b/backend/settings.js index 55702a442..13c8be1fd 100644 --- a/backend/settings.js +++ b/backend/settings.js @@ -1,9 +1,11 @@ import fs from 'fs'; +import os from 'os'; +import path from 'path'; + import {concat, contains, has} from 'ramda'; import YAML from 'yamljs'; + import {createStoragePath} from './utils/homeFiles'; -import path from 'path'; -import os from 'os'; const DEFAULT_SETTINGS = { HEADLESS: false, @@ -226,3 +228,8 @@ export function saveSetting(settingName, settingValue) { */ fs.writeFileSync(getSetting('SETTINGS_PATH'), YAML.stringify(settingsOnFile)); } + +// Get user credentials +export function getCredentials(username) { + return getSetting('USERS').find(u => u.username === username) || {}; +} From b3504bf231b43a919bfbbb2eb6e0b837cb299b49 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sun, 27 May 2018 02:49:25 +0100 Subject: [PATCH 3/5] PlotlyAPI: refactor * Move all the code that invokes `plotlyAPIRequest` into `backend/persistent/plotly-api.js`. * Ensure grids created for testing are deleted when the test completes. --- backend/persistent/QueryScheduler.js | 19 ++--- backend/persistent/plotly-api.js | 118 +++++++++++++++++++++++---- test/backend/QueryScheduler.spec.js | 23 ++++-- test/backend/plotly-api.spec.js | 16 ++-- test/backend/routes.spec.js | 14 ++-- test/backend/utils.js | 62 +++++++------- 6 files changed, 166 insertions(+), 86 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index fa3ad473b..fd9010720 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -14,7 +14,8 @@ import { getSetting } from '../settings.js'; import { - PlotlyAPIRequest, + getCurrentUser, + getGridMetadata, updateGrid } from './plotly-api.js'; @@ -146,9 +147,7 @@ class QueryScheduler { } // Check if the credentials are valid - return PlotlyAPIRequest('users/current', { - method: 'GET', username, apiKey, accessToken - }).then(res => { + return getCurrentUser(username).then(res => { if (res.status !== 200) { const errorMessage = ( `Unauthenticated: ${getSetting('PLOTLY_API_URL')} failed to identify ${username}.` @@ -204,16 +203,9 @@ class QueryScheduler { * make an additional API call to GET it */ - return PlotlyAPIRequest( - `grids/${fid}`, - {username, apiKey, accessToken, method: 'GET'} - ).then(resFromGET => { + return getGridMetadata(fid, username).then(resFromGET => { if (resFromGET.status === 404) { - Logger.log(('' + - `Grid ID ${fid} doesn't exist on Plotly anymore, ` + - 'removing persistent query.'), - 2 - ); + Logger.log(`Grid ID ${fid} doesn't exist on Plotly anymore, removing persistent query.`, 2); this.clearQuery(fid); return deleteQuery(fid); } @@ -224,7 +216,6 @@ class QueryScheduler { try { filemeta = JSON.parse(text); } catch (e) { - // Well, that's annoying. Logger.log(`Failed to parse the JSON of request ${fid}`, 0); Logger.log(e); Logger.log('Text response: ' + text, 0); diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index 3d57115d4..5e8ff4d35 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -11,8 +11,11 @@ import { // Module to access Plot.ly REST API // // See API documentation at https://api.plot.ly/v2/ +// +// TODO: Refactor as a class with a constructor that takes username as input, +// so that we don't call getCredentials(username) for every request. -export function PlotlyAPIRequest(relativeUrl, {body, username, apiKey, accessToken, method}) { +export function plotlyAPIRequest(relativeUrl, {body, username, apiKey, accessToken, method}) { let authorization; if (apiKey) { authorization = 'Basic ' + new Buffer( @@ -80,11 +83,80 @@ export function newDatacache(payloadJSON, type, requestor) { }); } +export function getCurrentUser(requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + return plotlyAPIRequest('users/current', { + method: 'GET', + username, + apiKey, + accessToken + }); +} + +export function newGrid(filename, columnnames, rows, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + const columns = getColumns(rows, columnnames.length); + + const cols = {}; + columnnames.forEach((name, i) => { + cols[name] = {'data': columns[i], order: i}; + }); + const grid = {cols}; + + return plotlyAPIRequest('grids', { + method: 'POST', + username, + apiKey, + accessToken, + body: { + data: grid, + world_readable: true, + parent: -1, + filename + } + }); +} + +export function getGridMetadata(fid, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + return plotlyAPIRequest(`grids/${fid}`, { + method: 'GET', + username, + apiKey, + accessToken + }); +} + +export function getGrid(fid, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + return plotlyAPIRequest(`grids/${fid}/content`, { + method: 'GET', + username, + apiKey, + accessToken + }); +} + +export function deleteGrid(fid, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + return plotlyAPIRequest(`grids/${fid}`, { + method: 'DELETE', + username, + apiKey, + accessToken + }); +} + export function updateGrid(rows, fid, uids, requestor) { const {username, apiKey, accessToken} = getCredentials(requestor); // TODO - Test case where no rows are returned. - if (uids.length !== rows[0].length) { + if (uids.length !== (rows[0] || []).length) { Logger.log(` A different number of columns was returned in the query than what was initially saved in the grid. @@ -96,25 +168,39 @@ export function updateGrid(rows, fid, uids, requestor) { `); } - const columns = uids.map(() => []); - for (let i = 0; i < rows.length; i++) { - const row = rows[i]; - /* - * For now, only update up to the number of columns that are - * already saved. In the future, we should just create more - * columns. See error message above. - */ - for (let j = 0; j < Math.min(uids.length, row.length); j++) { - columns[j][i] = row[j]; - } - } + /* + * For now, only update up to the number of columns that are + * already saved. In the future, we should just create more + * columns. See error message above. + */ + const columns = getColumns(rows, uids.length); + const url = `grids/${fid}/col?uid=${uids.join(',')}`; const body = { cols: JSON.stringify(columns.map(column => ({ data: column }))) }; - return PlotlyAPIRequest(url, {body, username, apiKey, accessToken, method: 'PUT'}); + return plotlyAPIRequest(url, {body, username, apiKey, accessToken, method: 'PUT'}); +} + +function getColumns(rows, maxColumns) { + const columns = []; + + // don't return more than maxColumns + const columnsLength = Math.min(maxColumns, (rows[0] || []).length); + + for (let i = 0; i < columnsLength; i++) { + columns.push([]); + } + + for (let rowIndex = 0; rowIndex < rows.length; rowIndex++) { + for (let columnIndex = 0; columnIndex < columnsLength; columnIndex++) { + columns[columnIndex][rowIndex] = rows[rowIndex][columnIndex]; + } + } + + return columns; } // Resolve if the requestor has permission to update fid, reject otherwise @@ -134,7 +220,7 @@ export function checkWritePermissions(fid, requestor) { throw new Error(errorMessage); } - return PlotlyAPIRequest(`grids/${fid}`, { + return plotlyAPIRequest(`grids/${fid}`, { username: requestor, apiKey, accessToken, diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index 9e3a408ff..59d09fdb0 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -3,17 +3,21 @@ import sinon from 'sinon'; import {merge} from 'ramda'; -import QueryScheduler from '../../backend/persistent/QueryScheduler.js'; import {saveConnection} from '../../backend/persistent/Connections.js'; +import { + deleteGrid, + getGrid, + updateGrid +} from '../../backend/persistent/plotly-api.js'; import {getQueries} from '../../backend/persistent/Queries.js'; -import {PlotlyAPIRequest, updateGrid} from '../../backend/persistent/plotly-api.js'; +import QueryScheduler from '../../backend/persistent/QueryScheduler.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; import { apiKey, assertResponseStatus, clearSettings, - createGrid, getResponseJson, + initGrid, names, sqlConnections, username, @@ -173,7 +177,7 @@ describe('QueryScheduler', function() { * it only updates them */ let fid, uids, queryObject; - return createGrid('test delete') + return initGrid('test delete') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { fid = json.file.fid; @@ -195,7 +199,7 @@ describe('QueryScheduler', function() { assert.deepEqual(getQueries(), [queryObject], 'Query has not been saved'); assert(Boolean(queryScheduler.queryJobs[fid]), 'Query has not been scheduled'); - return PlotlyAPIRequest(`grids/${fid}`, {username, apiKey, method: 'DELETE'}); + return deleteGrid(fid, username); }) .then(assertResponseStatus(204)) .then(() => { @@ -215,7 +219,7 @@ describe('QueryScheduler', function() { it('queries a database and updates a plotly grid on an interval', function() { function checkGridAgainstQuery(fid, name) { - return PlotlyAPIRequest(`grids/${fid}/content`, {username, apiKey, method: 'GET'}) + return getGrid(fid, username) .then(assertResponseStatus(200)) .then(getResponseJson).then(json => { assert.deepEqual( @@ -254,7 +258,7 @@ describe('QueryScheduler', function() { function resetAndVerifyGridContents(fid, uids) { return updateGrid([[1, 2, 3, 4, 5, 6]], fid, uids, username, apiKey) .then(assertResponseStatus(200)).then(() => { - return PlotlyAPIRequest(`grids/${fid}/content`, {username, apiKey, method: 'GET'}); + return getGrid(fid, username); }) .then(assertResponseStatus(200)) .then(getResponseJson).then(json => { @@ -282,7 +286,7 @@ describe('QueryScheduler', function() { * it only updates them */ let fid, uids, queryObject; - return createGrid('test interval') + return initGrid('test interval') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { fid = json.file.fid; @@ -302,7 +306,8 @@ describe('QueryScheduler', function() { .then(() => checkGridAgainstQuery(fid, 'First check')) .then(() => resetAndVerifyGridContents(fid, uids)) .then(() => wait(1.5 * refreshInterval * 1000)) - .then(() => checkGridAgainstQuery(fid, 'Second check')); + .then(() => checkGridAgainstQuery(fid, 'Second check')) + .then(() => deleteGrid(fid, username)); }); }); diff --git a/test/backend/plotly-api.spec.js b/test/backend/plotly-api.spec.js index d5b5f4a2e..eb14db8b9 100644 --- a/test/backend/plotly-api.spec.js +++ b/test/backend/plotly-api.spec.js @@ -1,21 +1,20 @@ import {assert} from 'chai'; import { - PlotlyAPIRequest, + deleteGrid, + getGrid, updateGrid } from '../../backend/persistent/plotly-api.js'; import {saveSetting} from '../../backend/settings.js'; import { apiKey, assertResponseStatus, - createGrid, getResponseJson, + initGrid, names, username } from './utils.js'; -// Suppressing ESLint cause Mocha ensures `this` is bound in test functions -/* eslint-disable no-invalid-this */ describe('Grid API Functions', function () { before(() => { saveSetting('USERS', [{username, apiKey}]); @@ -29,7 +28,7 @@ describe('Grid API Functions', function () { // it works off of the assumption that a grid exists let fid; - return createGrid('Test updateGrid') + return initGrid('Test updateGrid') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { fid = json.file.fid; @@ -47,8 +46,7 @@ describe('Grid API Functions', function () { }) .then(assertResponseStatus(200)) .then(() => { - const url = `grids/${fid}/content`; - return PlotlyAPIRequest(url, {username, apiKey, method: 'GET'}); + return getGrid(fid, username); }) .then(assertResponseStatus(200)) .then(getResponseJson).then(json => { @@ -76,7 +74,7 @@ describe('Grid API Functions', function () { json.cols[names[5]].data, [130, 140, 150] ); - }); + }) + .then(() => deleteGrid(fid, username)); }); }); -/* eslint-enable no-invalid-this */ diff --git a/test/backend/routes.spec.js b/test/backend/routes.spec.js index d20badf6c..56ca3e442 100644 --- a/test/backend/routes.spec.js +++ b/test/backend/routes.spec.js @@ -3,10 +3,11 @@ const fs = require('fs'); import {assert} from 'chai'; import {assoc, contains, dissoc, isEmpty, keys, merge} from 'ramda'; -import Servers from '../../backend/routes.js'; +import {setCertificatesSettings} from '../../backend/certificates'; import {getConnections, saveConnection} from '../../backend/persistent/Connections.js'; +import {deleteGrid} from '../../backend/persistent/plotly-api.js'; +import Servers from '../../backend/routes.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; -import {setCertificatesSettings} from '../../backend/certificates'; import { accessToken, apacheDrillConnections, @@ -14,12 +15,12 @@ import { assertResponseStatus, clearSettings, closeTestServers, - createGrid, createTestServers, DELETE, fakeCerts, GET, getResponseJson, + initGrid, mysqlConnection, PATCH, POST, @@ -1643,10 +1644,11 @@ describe('Routes:', () => { it('queries registers a query and returns saved queries', function() { // Save a grid that we can update - return createGrid('test interval') + let fid; + return initGrid('test interval') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { - const fid = json.file.fid; + fid = json.file.fid; const uids = json.file.cols.map(col => col.uid); queryObject = { @@ -1668,6 +1670,8 @@ describe('Routes:', () => { }) .then(getResponseJson).then(json => { assert.deepEqual(json, [queryObject]); + + return deleteGrid(fid, username); }); }); diff --git a/test/backend/utils.js b/test/backend/utils.js index 700954ec3..ebb980d4b 100644 --- a/test/backend/utils.js +++ b/test/backend/utils.js @@ -9,7 +9,7 @@ const restify = require('restify'); import {dissoc, merge} from 'ramda'; -import {PlotlyAPIRequest} from '../../backend/persistent/plotly-api.js'; +import {newGrid} from '../../backend/persistent/plotly-api.js'; import Servers from '../../backend/routes.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; @@ -122,18 +122,6 @@ export function clearSettings(...settingKeys) { }); } -export const names = [ - 'country', 'month', 'year', 'lat', 'lon', 'value' -]; -export const columns = [ - ['a', 'b', 'c'], // 'country' - [1, 2, 3], // 'month' - [4, 5, 6], // 'year' - [7, 8, 9], // 'lat' - [10, 11, 12], // 'lon' - [13, 14, 15] // 'value' -]; - // test account on prod export const username = 'plotly-database-connector'; export const apiKey = 'ccJMY9txxWhRRMWNLgOT'; @@ -158,6 +146,34 @@ export const validFid = 'plotly-database-connector:197'; export const validUids = ['d5d91e', '89d77e', '45b645', 'a7011b', '7cf34b', '881702', '442fd5', 'f5993c', '6d6a67', 'c3246c', 'eac785', '3c3ca8', '7ce7d7', 'f8cd7a', 'e52820', '0a91cc', 'c7dd62', 'b84d17', 'a6c128', 'ae9094']; + +// Helper function to initialise a grid for testing +export const names = [ + 'country', 'month', 'year', 'lat', 'lon', 'value' +]; + +export const columns = [ + ['a', 'b', 'c'], // 'country' + [1, 2, 3], // 'month' + [4, 5, 6], // 'year' + [7, 8, 9], // 'lat' + [10, 11, 12], // 'lon' + [13, 14, 15] // 'value' +]; + +export const rows = [ + ['a', 1, 4, 7, 10, 13], + ['b', 2, 5, 8, 11, 14], + ['c', 3, 6, 9, 12, 15] +]; + +export function initGrid(filename) { + const uniqueFilename = `${filename} - ${Math.random().toString(36).substr(2, 5)}`; + + return newGrid(uniqueFilename, names, rows, username); +} + + // Helper functions for Servers export function createTestServers() { const servers = new Servers({createCerts: false, startHttps: false, isElectron: false}); @@ -199,26 +215,6 @@ export function closeTestServers(servers) { }); } -// Helper functions for PlotlyAPIRequest -export function createGrid(filename) { - const cols = {}; - names.forEach((name, i) => { - cols[name] = {'data': columns[i], order: i}; - }); - const grid = {cols}; - return PlotlyAPIRequest('grids', { - method: 'POST', - username, - apiKey, - body: { - data: grid, - world_readable: true, - parent: -1, - filename: `${filename} - ${Math.random().toString(36).substr(2, 5)}` - } - }); -} - export const sqlConnections = { username: 'masteruser', password: 'connecttoplotly', From 3aa1c34b478939885417d4fc09d722e736d3bfe0 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sun, 27 May 2018 14:34:11 +0100 Subject: [PATCH 4/5] routes: extend entrypoint POST queries * POST queries accepts field `filename` to create a grid for storing the results of the scheduled query. Closes #440 --- backend/persistent/QueryScheduler.js | 79 +++++- backend/routes.js | 75 ++++-- package.json | 7 +- test/backend/routes.queries.spec.js | 349 +++++++++++++++++++++++++++ test/backend/routes.spec.js | 281 --------------------- 5 files changed, 474 insertions(+), 317 deletions(-) create mode 100644 test/backend/routes.queries.spec.js diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index fd9010720..5e18a1e22 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -16,6 +16,7 @@ import { import { getCurrentUser, getGridMetadata, + newGrid, updateGrid } from './plotly-api.js'; @@ -25,6 +26,7 @@ class QueryScheduler { this.loadQueries = this.loadQueries.bind(this); this.clearQuery = this.clearQuery.bind(this); this.clearQueries = this.clearQueries.bind(this); + this.queryAndCreateGrid = this.queryAndCreateGrid.bind(this); this.queryAndUpdateGrid = this.queryAndUpdateGrid.bind(this); // this.job wraps this.queryAndUpdateGrid to avoid concurrent runs of the same job @@ -119,7 +121,69 @@ class QueryScheduler { Object.keys(this.queryJobs).forEach(this.clearQuery); } - queryAndUpdateGrid (fid, uids, queryString, connectionId, requestor) { + queryAndCreateGrid(filename, query, connectionId, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + let startTime; + + // Check if the user even exists + if (!username || !(apiKey || accessToken)) { + /* + * Warning: The front end looks for "Unauthenticated" in this error message. Don't change it! + */ + const errorMessage = ( + 'Unauthenticated: Attempting to create a grid but the ' + + `authentication credentials for the user "${username}" do not exist.` + ); + Logger.log(errorMessage, 0); + throw new Error(errorMessage); + } + + // Check if the credentials are valid + return getCurrentUser(username).then(res => { + if (res.status !== 200) { + const errorMessage = ( + `Unauthenticated: ${getSetting('PLOTLY_API_URL')} failed to identify ${username}.` + ); + Logger.log(errorMessage, 0); + throw new Error(errorMessage); + } + + + startTime = process.hrtime(); + + Logger.log(`Querying "${query}" with connection ${connectionId} to create a new grid`, 2); + return Connections.query(query, getConnectionById(connectionId)); + + }).then(({rows, columnnames}) => { + Logger.log(`Query "${query}" took ${process.hrtime(startTime)[0]} seconds`, 2); + Logger.log('Create a new grid with new data', 2); + Logger.log(`First row: ${JSON.stringify(rows.slice(0, 1))}`, 2); + + startTime = process.hrtime(); + + return newGrid( + filename, + columnnames, + rows, + requestor + ); + + }).then(res => { + Logger.log(`Request to Plotly for creating a grid took ${process.hrtime(startTime)[0]} seconds`, 2); + + if (res.status !== 201) { + Logger.log(`Error ${res.status} while creating a grid`, 2); + } + + return res.json().then((json) => { + Logger.log(`Grid ${json.file.fid} has been updated.`, 2); + return json; + }); + }); + + } + + queryAndUpdateGrid(fid, uids, query, connectionId, requestor) { const requestedDBConnections = getConnectionById(connectionId); let startTime = process.hrtime(); @@ -156,21 +220,22 @@ class QueryScheduler { throw new Error(errorMessage); } - Logger.log(`Querying "${queryString}" with connection ${connectionId} to update grid ${fid}`, 2); - return Connections.query(queryString, requestedDBConnections); + Logger.log(`Querying "${query}" with connection ${connectionId} to update grid ${fid}`, 2); + return Connections.query(query, requestedDBConnections); - }).then(rowsAndColumns => { + }).then(({rows}) => { - Logger.log(`Query "${queryString}" took ${process.hrtime(startTime)[0]} seconds`, 2); + Logger.log(`Query "${query}" took ${process.hrtime(startTime)[0]} seconds`, 2); Logger.log(`Updating grid ${fid} with new data`, 2); Logger.log( 'First row: ' + - JSON.stringify(rowsAndColumns.rows.slice(0, 1)), + JSON.stringify(rows.slice(0, 1)), 2); startTime = process.hrtime(); + return updateGrid( - rowsAndColumns.rows, + rows, fid, uids, requestor diff --git a/backend/routes.js b/backend/routes.js index a71e68899..296f3cff6 100644 --- a/backend/routes.js +++ b/backend/routes.js @@ -643,42 +643,65 @@ export default class Servers { return next(); }); - // register or overwrite a query + // register/update a query (and create/update a grid) /* * TODO - Updating a query should be a PATCH or PUT under * the endpoint `/queries/:fid` */ server.post('/queries', function postQueriesHandler(req, res, next) { - // Make the query and update the user's grid - const {fid, uids, query, connectionId, requestor} = req.params; + const {filename, fid, uids, query, connectionId, requestor} = req.params; + + // If a filename has been provided, + // make the query and create a new grid + if (filename) { + return that.queryScheduler.queryAndCreateGrid( + filename, query, connectionId, requestor + ) + .then((newGridResponse) => { + const queryObject = { + ...req.params, + fid: newGridResponse.file.fid, + uids: newGridResponse.file.cols.map(col => col.uid) + }; + that.queryScheduler.scheduleQuery(queryObject); + res.json(201, queryObject); + return next(); + }) + .catch(onError); + } + + // If a grid fid has been provided, + // check the user has permission to edit, + // make the query and update the grid + if (fid) { + return checkWritePermissions(fid, requestor).then(function () { + return that.queryScheduler.queryAndUpdateGrid( + fid, uids, query, connectionId, requestor + ); + }) + .then(() => { + let status; + if (getQuery(req.params.fid)) { + // TODO - Technically, this should be + // under the endpoint `/queries/:fid` + status = 200; + } else { + status = 201; + } + that.queryScheduler.scheduleQuery(req.params); + res.json(status, {}); + return next(); + }) + .catch(onError); + } - // Check that the user has permission to edit the grid + return onError(new Error('Bad request')); - checkWritePermissions(fid, requestor) - .then(function nowQueryAndUpdateGrid() { - return that.queryScheduler.queryAndUpdateGrid( - fid, uids, query, connectionId, requestor - ); - }) - .then(function returnSuccess() { - let status; - if (getQuery(req.params.fid)) { - // TODO - Technically, this should be - // under the endpoint `/queries/:fid` - status = 200; - } else { - status = 201; - } - that.queryScheduler.scheduleQuery(req.params); - res.json(status, {}); - return next(); - }) - .catch(function returnError(error) { + function onError(error) { Logger.log(error, 0); res.json(400, {error: {message: error.message}}); return next(); - }); - + } }); // delete a query diff --git a/package.json b/package.json index 4008eabbd..3741cef7b 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "test-unit-all": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --bail --full-trace --timeout 90000 --compilers js:babel-register --recursive test/**/*.spec.js", "test-unit-all-watch": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --bail --full-trace --timeout 90000 --compilers js:babel-register --recursive test/**/*.spec.js --watch", "test-unit-watch": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --bail --full-trace --timeout 90000 --watch --compilers js:babel-register ", + "test-unit-athena": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.athena.spec.js", "test-unit-certificates": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/certificates.spec.js", "test-unit-csv": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.csv.spec.js", "test-unit-datastores": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/Datastores.spec.js", @@ -39,14 +40,14 @@ "test-unit-ibmdb": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.ibmdb.spec.js", "test-unit-impala": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.impala.spec.js", "test-unit-livy": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.livy.spec.js", - "test-unit-athena": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.athena.spec.js", + "test-unit-mock": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.mock.spec.js", "test-unit-oauth2": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/routes.oauth2.spec.js", "test-unit-oracle": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.oracle.spec.js", "test-unit-oracle:node": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.oracle.spec.js", "test-unit-plotly": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/plotly-api.spec.js", - "test-unit-scheduler": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/QueryScheduler.spec.js", + "test-unit-queries": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/routes.queries.spec.js", "test-unit-routes": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/routes.spec.js", - "test-unit-mock": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/datastores.mock.spec.js", + "test-unit-scheduler": "cross-env NODE_ENV=test BABEL_DISABLE_CACHE=1 electron-mocha --full-trace --timeout 90000 --compilers js:babel-register test/backend/QueryScheduler.spec.js", "pack": "cross-env NODE_ENV=production electron-builder --publish=never", "package": "cross-env NODE_ENV=production node -r babel-register package.js", "package-all": "yarn run package -- --all", diff --git a/test/backend/routes.queries.spec.js b/test/backend/routes.queries.spec.js new file mode 100644 index 000000000..81c1c2c72 --- /dev/null +++ b/test/backend/routes.queries.spec.js @@ -0,0 +1,349 @@ +import {assert} from 'chai'; +import {merge} from 'ramda'; +import uuid from 'uuid'; + +import {saveConnection} from '../../backend/persistent/Connections.js'; +import {deleteGrid} from '../../backend/persistent/plotly-api.js'; +import {getSetting, saveSetting} from '../../backend/settings.js'; +import { + accessToken, + assertResponseStatus, + closeTestServers, + createTestServers, + DELETE, + GET, + getResponseJson, + initGrid, + POST, + sqlConnections, + username, + validFid, + validUids +} from './utils.js'; + + +describe('Routes:', () => { + let queryObject; + let servers; + let connectionId; + + beforeEach(() => { + servers = createTestServers(); + + // Initialize query object + connectionId = saveConnection(sqlConnections); + queryObject = { + fid: validFid, + uids: validUids.slice(0, 2), // since this particular query only has 2 columns + refreshInterval: 60, // every minute + query: 'SELECT * FROM ebola_2014 LIMIT 1', + connectionId: connectionId, + requestor: validFid.split(':')[0] + }; + + // Sets cookies using `oauth` route, so that following requests will be authenticated + return POST('oauth2', {access_token: accessToken}) + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {}); + }); + }); + + afterEach(() => { + return closeTestServers(servers); + }); + + describe('queries:', function() { + beforeEach(function() { + // Verify that there are no queries saved + return GET('queries') + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, []); + }); + }); + + it('can create a grid when it registers a query', function() { + let fid; + + queryObject = { + requestor: username, + refreshInterval: 60, + connectionId, + query: 'SELECT * from ebola_2014 LIMIT 2' + }; + + return POST('queries', {...queryObject, filename: `test queries ${uuid.v4()}`}) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + fid = json.fid; + + assert(json.fid, 'undefined fid'); + assert(json.uids, 'undefined uids'); + + queryObject.fid = json.fid; + queryObject.uids = json.uids; + + return GET('queries'); + }) + .then(getResponseJson).then(json => { + assert.deepEqual(json, [queryObject]); + + return deleteGrid(fid, username); + }); + }); + + it('registers a query and returns saved queries', function() { + // Save a grid that we can update + let fid; + return initGrid('test interval') + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + fid = json.file.fid; + const uids = json.file.cols.map(col => col.uid); + + queryObject = { + fid, + requestor: fid.split(':')[0], + uids, + refreshInterval: 60, + connectionId, + query: 'SELECT * from ebola_2014 LIMIT 2' + }; + + return POST('queries', queryObject); + }) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {}); + + return GET('queries'); + }) + .then(getResponseJson).then(json => { + assert.deepEqual(json, [queryObject]); + + return deleteGrid(fid, username); + }); + }); + + it('can register queries if the user is a collaborator', function() { + /* + * Plotly doesn't have a v2 endpoint for creating + * collaborators, so we'll just use these hardcoded + * fids and uids that already have a collaborator + * assigned to them. + * This test won't work against any plotly server + * except https://plot.ly + */ + const collaborator = 'plotly-connector-collaborator'; + saveSetting('USERS', [{ + username: collaborator, + apiKey: 'I6j80cqCVaBAnvH9ESD2' + }]); + const fid = 'plotly-database-connector:718'; + const uids = ['d8ba6c', 'dfa411']; + + queryObject = { + fid, + requestor: collaborator, + uids, + refreshInterval: 60, + connectionId, + query: 'SELECT * from ebola_2014 LIMIT 2' + }; + + return POST('queries', queryObject) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {}); + return GET('queries'); + }) + .then(getResponseJson).then(json => { + assert.deepEqual(json, [queryObject]); + }); + }); + + it("can't register queries if the user can't view it", function() { + // The grid is not shared with this user + const viewer = 'plotly-connector-viewer'; + saveSetting('USERS', [{ + username: viewer, + apiKey: 'mUSjMmwa55d6hjvwvgI4' + }]); + const fid = 'plotly-database-connector:718'; + const uids = ['d8ba6c', 'dfa411']; + + queryObject = { + fid, + requestor: viewer, + uids, + refreshInterval: 60, + connectionId, + query: 'SELECT * from ebola_2014 LIMIT 2' + }; + + return POST('queries', queryObject) + .then(assertResponseStatus(400)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {error: {message: 'Not found'}}); + return GET('queries'); + }) + .then(getResponseJson).then(json => { + assert.deepEqual(json, [], 'No queries were saved'); + }); + }); + + it("can't register queries if the user isn't a collaborator", function() { + // The grid is not shared with this user + const viewer = 'plotly-connector-viewer'; + saveSetting('USERS', [{ + username: viewer, + apiKey: 'mUSjMmwa55d6hjvwvgI4' + }]); + /* + * Unlike plotly-database-connector:718, this grid is public + * so requests won't fail because of a 404. They should, however, + * fail because there are no collaborators associated with this + * plot + */ + const fid = 'plotly-database-connector:719'; + const uids = ['3a6df9', 'b95e9d']; + + queryObject = { + fid, + requestor: viewer, + uids, + refreshInterval: 60, + connectionId, + query: 'SELECT * from ebola_2014 LIMIT 2' + }; + + return POST('queries', queryObject) + .then(assertResponseStatus(400)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {error: {message: 'Permission denied'}}); + return GET('queries'); + }) + .then(getResponseJson).then(json => { + assert.deepEqual(json, [], 'still no queries saved'); + }); + }); + + it('gets individual queries', function() { + return GET(`queries/${queryObject.fid}`) + .then(assertResponseStatus(404)).then(() => { + return POST('queries', queryObject); + }) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {}); + return GET(`queries/${queryObject.fid}`); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, queryObject); + }); + }); + + it('deletes queries', function() { + return POST('queries', queryObject) + .then(assertResponseStatus(201)) + .then(() => DELETE(`queries/${queryObject.fid}`)) + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, {}); + return GET(`queries/${queryObject.fid}`); + }) + .then(assertResponseStatus(404)); + }); + + it('returns 404s when getting queries that don\'t exist', function() { + return GET('queries/asdfasdf').then(assertResponseStatus(404)); + }); + + it('returns 404s when deleting queries that don\'t exist', function() { + return DELETE('queries/asdfasdf').then(assertResponseStatus(404)); + }); + + it("fails when the user's API keys or oauth creds aren't saved", function() { + saveSetting('USERS', []); + assert.deepEqual(getSetting('USERS'), []); + + return POST('queries', queryObject) + .then(assertResponseStatus(500)) + .then(getResponseJson).then(json => { + assert.deepEqual( + json, + {error: { + message: ( + 'Unauthenticated: Attempting to update grid ' + + 'plotly-database-connector:197 ' + + 'but the authentication credentials ' + + 'for the user "plotly-database-connector" ' + + 'do not exist.' + ) + }} + ); + }); + }); + + it("fails when the user's API keys aren't correct", function() { + const creds = [{username: 'plotly-database-connector', apiKey: 'lah lah lemons'}]; + saveSetting('USERS', creds); + assert.deepEqual(getSetting('USERS'), creds); + + return POST('queries', queryObject) + .then(assertResponseStatus(400)) + .then(getResponseJson).then(json => { + assert.deepEqual( + json, + {error: { + message: ( + 'Unauthenticated' + ) + }} + ); + }); + }); + + it("fails when it can't connect to the plotly server", function() { + const nonExistantServer = 'plotly.lah-lah-lemons.com'; + saveSetting('PLOTLY_API_DOMAIN', nonExistantServer); + assert.deepEqual(getSetting('PLOTLY_API_URL'), `https://${nonExistantServer}`); + + return POST('queries', queryObject) + .then(assertResponseStatus(400)) + .then(getResponseJson).then(json => { + assert.deepEqual( + json, + { + error: { + message: ( + 'request to ' + + 'https://plotly.lah-lah-lemons.com/v2/grids/plotly-database-connector:197 ' + + 'failed, reason: getaddrinfo ENOTFOUND plotly.lah-lah-lemons.com ' + + 'plotly.lah-lah-lemons.com:443' + ) + } + } + ); + }); + }); + + it('fails when there is a syntax error in the query', function() { + const invalidQueryObject = merge( + queryObject, + {query: 'SELECZ'} + ); + + return POST('queries', invalidQueryObject) + .then(assertResponseStatus(400)) + .then(getResponseJson).then(json => { + assert.deepEqual( + json, + {error: {message: 'syntax error at or near "SELECZ"'}} + ); + }); + }); + }); +}); diff --git a/test/backend/routes.spec.js b/test/backend/routes.spec.js index 56ca3e442..93404fc54 100644 --- a/test/backend/routes.spec.js +++ b/test/backend/routes.spec.js @@ -5,7 +5,6 @@ import {assoc, contains, dissoc, isEmpty, keys, merge} from 'ramda'; import {setCertificatesSettings} from '../../backend/certificates'; import {getConnections, saveConnection} from '../../backend/persistent/Connections.js'; -import {deleteGrid} from '../../backend/persistent/plotly-api.js'; import Servers from '../../backend/routes.js'; import {getSetting, saveSetting} from '../../backend/settings.js'; import { @@ -20,7 +19,6 @@ import { fakeCerts, GET, getResponseJson, - initGrid, mysqlConnection, PATCH, POST, @@ -31,14 +29,10 @@ import { testConnections, testSqlConnections, username, - validFid, - validUids, wait } from './utils.js'; - -let queryObject; let servers; let connectionId; @@ -66,8 +60,6 @@ function closeServers() { }); } -// Suppressing ESLint cause Mocha ensures `this` is bound in test functions -/* eslint-disable no-invalid-this */ describe('Servers:', () => { beforeEach(() => { clearSettings('KEY_FILE', 'CERT_FILE', 'SETTINGS_PATH'); @@ -134,14 +126,6 @@ describe('Routes:', () => { // Initialize query object connectionId = saveConnection(sqlConnections); - queryObject = { - fid: validFid, - uids: validUids.slice(0, 2), // since this particular query only has 2 columns - refreshInterval: 60, // every minute - query: 'SELECT * FROM ebola_2014 LIMIT 1', - connectionId: connectionId, - requestor: validFid.split(':')[0] - }; // Sets cookies using `oauth` route, so that following requests will be authenticated return POST('oauth2', {access_token: accessToken}) @@ -1631,269 +1615,4 @@ describe('Routes:', () => { }); }); }); - - describe('persistent queries:', function() { - beforeEach(function() { - // Verify that there are no queries saved - return GET('queries') - .then(assertResponseStatus(200)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, []); - }); - }); - - it('queries registers a query and returns saved queries', function() { - // Save a grid that we can update - let fid; - return initGrid('test interval') - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - fid = json.file.fid; - const uids = json.file.cols.map(col => col.uid); - - queryObject = { - fid, - requestor: fid.split(':')[0], - uids, - refreshInterval: 60, - connectionId, - query: 'SELECT * from ebola_2014 LIMIT 2' - }; - - return POST('queries', queryObject); - }) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [queryObject]); - - return deleteGrid(fid, username); - }); - }); - - it('queries can register queries if the user is a collaborator', function() { - /* - * Plotly doesn't have a v2 endpoint for creating - * collaborators, so we'll just use these hardcoded - * fids and uids that already have a collaborator - * assigned to them. - * This test won't work against any plotly server - * except https://plot.ly - */ - const collaborator = 'plotly-connector-collaborator'; - saveSetting('USERS', [{ - username: collaborator, - apiKey: 'I6j80cqCVaBAnvH9ESD2' - }]); - const fid = 'plotly-database-connector:718'; - const uids = ['d8ba6c', 'dfa411']; - - queryObject = { - fid, - requestor: collaborator, - uids, - refreshInterval: 60, - connectionId, - query: 'SELECT * from ebola_2014 LIMIT 2' - }; - - return POST('queries', queryObject) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [queryObject]); - }); - }); - - it("queries can't register queries if the user can't view it", function() { - // The grid is not shared with this user - const viewer = 'plotly-connector-viewer'; - saveSetting('USERS', [{ - username: viewer, - apiKey: 'mUSjMmwa55d6hjvwvgI4' - }]); - const fid = 'plotly-database-connector:718'; - const uids = ['d8ba6c', 'dfa411']; - - queryObject = { - fid, - requestor: viewer, - uids, - refreshInterval: 60, - connectionId, - query: 'SELECT * from ebola_2014 LIMIT 2' - }; - - return POST('queries', queryObject) - .then(assertResponseStatus(400)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {error: {message: 'Not found'}}); - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [], 'No queries were saved'); - }); - }); - - it("queries can't register queries if the user isn't a collaborator", function() { - // The grid is not shared with this user - const viewer = 'plotly-connector-viewer'; - saveSetting('USERS', [{ - username: viewer, - apiKey: 'mUSjMmwa55d6hjvwvgI4' - }]); - /* - * Unlike plotly-database-connector:718, this grid is public - * so requests won't fail because of a 404. They should, however, - * fail because there are no collaborators associated with this - * plot - */ - const fid = 'plotly-database-connector:719'; - const uids = ['3a6df9', 'b95e9d']; - - queryObject = { - fid, - requestor: viewer, - uids, - refreshInterval: 60, - connectionId, - query: 'SELECT * from ebola_2014 LIMIT 2' - }; - - return POST('queries', queryObject) - .then(assertResponseStatus(400)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {error: {message: 'Permission denied'}}); - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [], 'still no queries saved'); - }); - }); - - it('queries gets individual queries', function() { - return GET(`queries/${queryObject.fid}`) - .then(assertResponseStatus(404)).then(() => { - return POST('queries', queryObject); - }) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - return GET(`queries/${queryObject.fid}`); - }) - .then(assertResponseStatus(200)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, queryObject); - }); - }); - - it('queries deletes queries', function() { - return POST('queries', queryObject) - .then(assertResponseStatus(201)) - .then(() => DELETE(`queries/${queryObject.fid}`)) - .then(assertResponseStatus(200)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - return GET(`queries/${queryObject.fid}`); - }) - .then(assertResponseStatus(404)); - }); - - it('queries returns 404s when getting queries that don\'t exist', function() { - return GET('queries/asdfasdf').then(assertResponseStatus(404)); - }); - - it('queries returns 404s when deleting queries that don\'t exist', function() { - return DELETE('queries/asdfasdf').then(assertResponseStatus(404)); - }); - - it("queries POST /queries fails when the user's API keys or oauth creds aren't saved", function() { - saveSetting('USERS', []); - assert.deepEqual(getSetting('USERS'), []); - - return POST('queries', queryObject) - .then(assertResponseStatus(500)) - .then(getResponseJson).then(json => { - assert.deepEqual( - json, - {error: { - message: ( - 'Unauthenticated: Attempting to update grid ' + - 'plotly-database-connector:197 ' + - 'but the authentication credentials ' + - 'for the user "plotly-database-connector" ' + - 'do not exist.' - ) - }} - ); - }); - }); - - it("queries POST /queries fails when the user's API keys aren't correct", function() { - const creds = [{username: 'plotly-database-connector', apiKey: 'lah lah lemons'}]; - saveSetting('USERS', creds); - assert.deepEqual(getSetting('USERS'), creds); - - return POST('queries', queryObject) - .then(assertResponseStatus(400)) - .then(getResponseJson).then(json => { - assert.deepEqual( - json, - {error: { - message: ( - 'Unauthenticated' - ) - }} - ); - }); - }); - - it("queries POST /queries fails when it can't connect to the plotly server", function() { - const nonExistantServer = 'plotly.lah-lah-lemons.com'; - saveSetting('PLOTLY_API_DOMAIN', nonExistantServer); - assert.deepEqual(getSetting('PLOTLY_API_URL'), `https://${nonExistantServer}`); - - return POST('queries', queryObject) - .then(assertResponseStatus(400)) - .then(getResponseJson).then(json => { - assert.deepEqual( - json, - { - error: { - message: ( - 'request to ' + - 'https://plotly.lah-lah-lemons.com/v2/grids/plotly-database-connector:197 ' + - 'failed, reason: getaddrinfo ENOTFOUND plotly.lah-lah-lemons.com ' + - 'plotly.lah-lah-lemons.com:443' - ) - } - } - ); - }); - }); - - it('queries POST /queries fails when there is a syntax error in the query', function() { - const invalidQueryObject = merge( - queryObject, - {query: 'SELECZ'} - ); - - return POST('queries', invalidQueryObject) - .then(assertResponseStatus(400)) - .then(getResponseJson).then(json => { - assert.deepEqual( - json, - {error: {message: 'syntax error at or near "SELECZ"'}} - ); - }); - }); - }); }); -/* eslint-enable no-invalid-this */ From 728888b68b21b1fea7df11296d884745e60ff2ce Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sun, 27 May 2018 21:08:09 +0100 Subject: [PATCH 5/5] PlotlyAPI: rename getGridMetadat to getGridMeta * To be consistent with Plotly nomenclature: https://github.com/plotly/falcon-sql-client/pull/444#discussion_r191239813 --- backend/persistent/QueryScheduler.js | 4 ++-- backend/persistent/plotly-api.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 5e18a1e22..19f5b517d 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -15,7 +15,7 @@ import { } from '../settings.js'; import { getCurrentUser, - getGridMetadata, + getGridMeta, newGrid, updateGrid } from './plotly-api.js'; @@ -268,7 +268,7 @@ class QueryScheduler { * make an additional API call to GET it */ - return getGridMetadata(fid, username).then(resFromGET => { + return getGridMeta(fid, username).then(resFromGET => { if (resFromGET.status === 404) { Logger.log(`Grid ID ${fid} doesn't exist on Plotly anymore, removing persistent query.`, 2); this.clearQuery(fid); diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index 5e8ff4d35..02268c25c 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -119,7 +119,7 @@ export function newGrid(filename, columnnames, rows, requestor) { }); } -export function getGridMetadata(fid, requestor) { +export function getGridMeta(fid, requestor) { const {username, apiKey, accessToken} = getCredentials(requestor); return plotlyAPIRequest(`grids/${fid}`, {