diff --git a/InfoLogger/lib/controller/QueryController.js b/InfoLogger/lib/controller/QueryController.js index 89ca74f96..48a61918e 100644 --- a/InfoLogger/lib/controller/QueryController.js +++ b/InfoLogger/lib/controller/QueryController.js @@ -13,6 +13,7 @@ */ const { LogManager, updateAndSendExpressResponseFromNativeError } = require('@aliceo2/web-ui'); +const { AbortError, throwIfQueryAborted } = require('../utils/queryCancellation'); /** * Gateway for all calls that are to query InfoLogger database @@ -37,15 +38,36 @@ class QueryController { * @returns {void} */ async getLogs(req, res) { + const abortController = new AbortController(); + const { signal } = abortController; try { const { body: { criterias, options } } = req; if (!criterias || Object.keys(criterias).length === 0) { res.status(400).json({ error: 'Invalid query parameters provided' }); return; } - const logs = await this._queryService.queryFromFilters(criterias, options); + + let responseInProgress = true; + + res.on('finish', () => { + responseInProgress = false; + }); + + res.on('close', () => { + if (responseInProgress) { + abortController.abort(); + } + }); + + const logs = await this._queryService.queryFromFilters(criterias, options, signal); + throwIfQueryAborted(signal); + res.status(200).json(logs); } catch (error) { + if (signal.aborted || error instanceof AbortError) { + this._logger.infoMessage('Query was cancelled by the client'); + return; + } this._logger.errorMessage(error.toString()); updateAndSendExpressResponseFromNativeError(res, error); } diff --git a/InfoLogger/lib/services/QueryService.js b/InfoLogger/lib/services/QueryService.js index 929e6b30a..a2f7809fc 100644 --- a/InfoLogger/lib/services/QueryService.js +++ b/InfoLogger/lib/services/QueryService.js @@ -16,6 +16,7 @@ const mariadb = require('mariadb'); const { LogManager, InvalidInputError } = require('@aliceo2/web-ui'); const { fromSqlToNativeError } = require('../utils/fromSqlToNativeError'); const { processPreparedSQLStatement } = require('../utils/preparedStatementParser'); +const { throwIfQueryAborted, attachAbortDestroyHandler } = require('../utils/queryCancellation'); class QueryService { /** @@ -92,32 +93,49 @@ class QueryService { * @param {object} filters - criteria like MongoDB * @param {object} options - specific options for the query * @param {number} options.limit - how many rows to get + * @param {AbortSignal} [signal] - optional signal to cancel the query; when aborted, the DB connection is destroyed * @returns {Promise.} - {total, more, limit, rows, count, time} */ - async queryFromFilters(filters, options) { + async queryFromFilters(filters, options, signal = null) { const { limit = 100000 } = options; const { criteria, values } = this._filtersToSqlConditions(filters); const criteriaString = this._getCriteriaAsString(criteria); - const requestRows = `SELECT * FROM \`messages\` ${criteriaString} ORDER BY \`TIMESTAMP\` LIMIT ?;`; + const queryValues = [...values, limit]; const startTime = Date.now(); // ms this._logger.debugMessage(`SQL to execute: ${processPreparedSQLStatement(requestRows, values, limit)}`); let rows = []; + let connection = null; + let connectionDestroyed = false; try { if (!this._pool) { throw new Error('No database connection available'); } - rows = await this._pool.query( - { - sql: requestRows, - timeout: this._timeout, + connection = await this._pool.getConnection(); + throwIfQueryAborted(signal); + + const detachAbortHandler = attachAbortDestroyHandler( + signal, + connection, + () => { + connectionDestroyed = true; }, - [...values, limit], ); + + try { + rows = await connection.query({ sql: requestRows, timeout: this._timeout }, queryValues); + } finally { + detachAbortHandler(); + } } catch (error) { + throwIfQueryAborted(signal); fromSqlToNativeError(error); + } finally { + if (connection && !connectionDestroyed) { + connection.release(); + } } const totalTime = Date.now() - startTime; // ms diff --git a/InfoLogger/lib/utils/queryCancellation.js b/InfoLogger/lib/utils/queryCancellation.js new file mode 100644 index 000000000..c945a576b --- /dev/null +++ b/InfoLogger/lib/utils/queryCancellation.js @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2019-2020 CERN and copyright holders of ALICE O2. + * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders. + * All rights not expressly granted are reserved. + * + * This software is distributed under the terms of the GNU General Public + * License v3 (GPL Version 3), copied verbatim in the file "COPYING". + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +/** + * Custom error class for query cancellation + */ +class AbortError extends Error { + /** + * Create an AbortError + * @param {string} message - error message + */ + constructor(message = 'Query cancelled by client') { + super(message); + this.name = 'AbortError'; + this.code = 'QUERY_CANCELLED'; + } +} + +/** + * Throw an error if the given signal is already aborted. + * @param {AbortSignal|null} signal - optional abort signal + * @throws {AbortError} if signal is aborted + */ +const throwIfQueryAborted = (signal) => { + if (signal?.aborted) { + throw new AbortError(); + } +}; + +/** + * Attach a one-time abort handler to the signal that destroys the connection. + * Return a cleanup function to remove the listener + * @param {AbortSignal|null} signal - optional abort signal + * @param {object} connection - mariadb connection-like object + * @param {function(): void} onDestroyed - callback called just before destroying connection + * @returns {function(): void} cleanup callback to remove the listener + */ +const attachAbortDestroyHandler = (signal, connection, onDestroyed) => { + if (!signal) { + return () => {}; + } + + const abortHandler = () => { + onDestroyed(); + connection.destroy(); + }; + + signal.addEventListener('abort', abortHandler, { once: true }); + return () => signal.removeEventListener('abort', abortHandler); +}; + +module.exports = { + AbortError, + throwIfQueryAborted, + attachAbortDestroyHandler, +}; diff --git a/InfoLogger/public/app.css b/InfoLogger/public/app.css index 030491103..5a76d735c 100644 --- a/InfoLogger/public/app.css +++ b/InfoLogger/public/app.css @@ -29,6 +29,7 @@ --row-height: 0.91rem; /* default, overridden by JS zoom */ } .logs-content { border-top: 1px solid #aaa; } +.bold { font-weight: bold; } /* logs tables */ .table-logs-header { width: 100%; border-collapse: collapse; } diff --git a/InfoLogger/public/common/jsonFetch.js b/InfoLogger/public/common/jsonFetch.js new file mode 100644 index 000000000..e97171838 --- /dev/null +++ b/InfoLogger/public/common/jsonFetch.js @@ -0,0 +1,54 @@ +/** + * @license + * Copyright CERN and copyright holders of ALICE O2. This software is + * distributed under the terms of the GNU General Public License v3 (GPL + * Version 3), copied verbatim in the file "COPYING". + * + * See http://alice-o2.web.cern.ch/license for full licensing information. + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +import { fetchClient } from '/js/src/index.js'; + +/** + * Send a request to an endpoint, and extract the response. If errors occurred, return an error containing a message. + * + * The endpoint is expected to follow some conventions: + * - If request is valid but no data was sent as response, it must return a 204 + * - If an error occurred on the backend: + * - request can be status ok with {message: string} body describing the error + * - or request can be status error with or without body that contains a message field describing the error + * - If request is valid and data is sent as response, it must return a json with the expected data + * @param {string} endpoint - the remote endpoint to send request to + * @param {RequestInit} options - the request options, see {@link fetch } native function + * (method, headers, body, abort.signal, etc.) + * @returns {Promise.Error<{message: string}>>} resolve with the result or reject with the error message + */ +export const jsonFetch = async (endpoint, options) => { + try { + const response = await fetchClient(endpoint, options); + if (response.status === 204) { + return null; + } + + const result = await response.json(); + + if (response.ok) { + return result; + } + + const serverMessage = result && typeof result.message === 'string' + ? result.message + : null; + + throw new Error(serverMessage || `Request failed with status ${response.status}`); + } catch (error) { + if (error && typeof error.message === 'string') { + throw error; + } + throw new Error('Parsing result from server failed', { cause: error }); + } +}; diff --git a/InfoLogger/public/common/jsonPost.js b/InfoLogger/public/common/jsonPost.js new file mode 100644 index 000000000..73dcdf806 --- /dev/null +++ b/InfoLogger/public/common/jsonPost.js @@ -0,0 +1,40 @@ +/** + * @license + * Copyright CERN and copyright holders of ALICE O2. This software is + * distributed under the terms of the GNU General Public License v3 (GPL + * Version 3), copied verbatim in the file "COPYING". + * + * See http://alice-o2.web.cern.ch/license for full licensing information. + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +import { jsonFetch } from './jsonFetch.js'; + +/** + * Build and send a POST request to a remote endpoint, and extract the response. + * @param {string} endpoint - the remote endpoint to send request to + * @param {RequestInit} options - the request options, see {@link fetch } native function + * (method, headers, body, abort.signal, etc.) + * @returns {Promise.Error<{message: string}>>} resolve with the result or reject with the error + */ +export const jsonPost = async (endpoint, options = {}) => { + if (options.body && typeof options.body === 'object') { + options.body = JSON.stringify(options.body); + } + try { + const result = await jsonFetch(endpoint, { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + }, + ...options, + }); + return result; + } catch (error) { + return Promise.reject({ message: error.message || error }); + } +}; diff --git a/InfoLogger/public/log/Log.js b/InfoLogger/public/log/Log.js index fbcd8e10f..88b8ed08a 100644 --- a/InfoLogger/public/log/Log.js +++ b/InfoLogger/public/log/Log.js @@ -17,6 +17,7 @@ import LogFilter from '../logFilter/LogFilter.js'; import ContextMenu from './ContextMenu.js'; import { MODE } from '../constants/mode.const.js'; import { TIME_MS } from '../common/Timezone.js'; +import { jsonPost } from '../common/jsonPost.js'; /** * Model Log, encapsulate all log management and queries @@ -48,6 +49,7 @@ export default class Log extends Observable { this.limitReached = null; this.queryResult = RemoteData.notAsked(); + this.queryAbortController = null; this.list = []; this.item = null; @@ -327,16 +329,14 @@ export default class Log extends Observable { } /** - * Query database according to filters. - * Only is service is available and configured on server side. - * If live mode is enabled, it is turned off. - * `list` is then reset and filled with result. + * Method to execute a query with the current filters configuration + * If the user has no filters set, a prompt is shown to confirm the execution + * If the user is in live mode, first stop live mode and then execute query in order to have a consistent result + * Recalculate the stats and go to last log once query is executed + * If the query is aborted by user, restore previous query result and do nothing + * @returns {Promise} null if query is aborted, result of the query otherwise */ async query() { - if (!this.model.frameworkInfo.isSuccess() || !this.model.frameworkInfo.payload.mysql.status.ok) { - throw new Error('Query service is not available'); - } - if (!this.filter.hasActiveTextFilters()) { if (!window.confirm('No date or text filters set.' + ' This will return a large amount of data. Execute query anyway?')) { @@ -344,28 +344,35 @@ export default class Log extends Observable { } } - this.queryResult = RemoteData.loading(); - this.notify(); - if (this.isLiveModeRunning()) { this.liveStop(MODE.QUERY); } else { this.activeMode = MODE.QUERY; } - const queryArguments = { - criterias: this.filter.criterias, - options: { limit: this.limit }, - }; - const { result, ok } = await this.model.loader.post('/api/query', queryArguments, true); - if (!ok) { - this.queryResult = RemoteData.failure(result.message); - this.list = []; - } else { + const previousQueryResult = this.queryResult; + this.queryResult = RemoteData.loading(); + this.notify(); + + const abortController = new AbortController(); + this.queryAbortController = abortController; + + let result = 'Unable to execute query'; + try { + result = await jsonPost('/api/query', { + body: { + criterias: this.filter.criterias, + options: { limit: this.limit }, + }, + signal: abortController.signal, + }); + this.resetStats(); this.queryResult = RemoteData.success(result); this.list = result.rows; - this.limitReached = result.count === this.limit; + this.list.forEach((log) => this.addStats(log)); + this.goToLastItem(); + this.limitReached = result.count === this.limit; if (this.limitReached) { this.model.notification.show( `Matching results reached the buffer size of ${this.limit.toLocaleString('en-US')}.` @@ -373,13 +380,35 @@ export default class Log extends Observable { 'warning', ); } + } catch (error) { + if (abortController.signal.aborted || error.name === 'AbortError') { + this.queryResult = previousQueryResult; + } else { + result = { message: error.message || result }; + this.queryResult = RemoteData.failure(result.message); + this.list = []; + this.resetStats(); + } + } finally { + this.queryAbortController = null; } + this.notify(); + } - this.resetStats(); - this.list.forEach((log) => this.addStats(log)); + /** + * Method to allow for cancellation of ongoing HTTP request for query mode if: + * - a query is still ongoing + * - an abort controller is present. + * If the query is successfully aborted, a notification is shown to user. + * @returns {void} + */ + cancelQuery() { + if (!this.queryResult.isLoading() || !this.queryAbortController) { + return; + } - this.goToLastItem(); - this.notify(); + this.queryAbortController.abort(); + this.model.notification.show('Query cancelled', 'warning', 2000); } /** diff --git a/InfoLogger/public/log/commandLogs.js b/InfoLogger/public/log/commandLogs.js index 9fb9c8ebd..336c54ab0 100644 --- a/InfoLogger/public/log/commandLogs.js +++ b/InfoLogger/public/log/commandLogs.js @@ -29,12 +29,14 @@ let queryButtonType = BUTTON.PRIMARY; let liveButtonType = BUTTON.DEFAULT; let liveButtonIcon = iconMediaPlay(); -export default (model) => [ +/** + * Component for the command buttons (Query, Live, Clear, navigation between errors and download) + * @param {Model} model - root model of the application + * @returns {vnode} - the view of the command buttons + */ +export const commandLogs = (model) => [ userActionsDropdown(model), - h('div.btn-group.mh3', [ - queryButton(model), - liveButton(model), - ], ''), + h('div.btn-group.mh3', interactionModesGroupButton(model)), h('button.btn.mh3', { onclick: () => model.log.empty(), style: 'font-weight: bold' }, 'Clear'), h('button.btn', { disabled: !model.log.list.length, @@ -69,6 +71,84 @@ export default (model) => [ zoomButtonGroup(model.zoom), ]; +/** + * Group of buttons for switching between Query and Live modes. + * @param {Model} model - root model of the application + * @returns {vnode} - the view of the interaction mode buttons + */ +const interactionModesGroupButton = (model) => { + const { frameworkInfo } = model; + + return frameworkInfo.match({ + NotAsked: () => h('button.btn', { disabled: true }, ''), + Loading: () => h('button.btn', { disabled: true, className: 'loading' }, 'Loading'), + Failure: () => [], + Success: (frameworkInfo) => + [ + queryButton(model, frameworkInfo), + liveButton(model, frameworkInfo), + ], + }); +}; + +/** + * Query button final state depends on the following states + * - services lookup + * - services result + * - query lookup + * @param {Model} model - root model of the application + * @param {RemoteData.payload} frameworkInfo - the payload containing framework information + * @returns {vnode} - the view of the query button + */ +const queryButton = (model, frameworkInfo) => { + const { log: logModel } = model; + const { queryResult } = logModel; + const { mysql: { status: { ok: isDbReady = false } = {} } = {} } = frameworkInfo; + + if (queryResult.isLoading()) { + return h('button.btn.bold', { + id: 'cancel-query-button', + title: 'Cancel ongoing query', + className: BUTTON.DANGER, + onclick: () => logModel.cancelQuery(), + }, 'Cancel'); + } + + return h('button.btn.bold', { + id: 'query-button', + title: isDbReady ? 'Query database with filters (Enter)' : 'Query service not configured', + disabled: !isDbReady || queryResult.isLoading(), + className: queryResult.isLoading() ? 'loading' : queryButtonType, + onclick: () => toggleButtonStates(model, false), + }, 'Query'); +}; + +/** + * Live button final state depends on the following states + * - services lookup + * - services result + * - websocket status + * @param {Model} model - root model of the application + * @param {RemoteData.payload} frameworkInfo - the payload containing framework information + * @returns {vnode} - the view of the live button + */ +const liveButton = (model, frameworkInfo) => { + const { log: logModel, ws } = model; + const { queryResult } = logModel; + const { authed: isWsAuthedAndReady = false } = ws; + const { infoLoggerServer: { status: { ok: isLiveServiceReady = false } = {} } = {} } = frameworkInfo; + + const isLiveModeReady = isLiveServiceReady && isWsAuthedAndReady; + const title = isLiveModeReady ? 'Stream logs with filtering' : 'Live service not configured'; + + return h('button.btn.bold', { + title, + disabled: !isLiveModeReady || queryResult.isLoading(), + className: !isLiveModeReady ? 'loading' : liveButtonType, + onclick: () => toggleButtonStates(model, true), + }, 'Live', ' ', liveButtonIcon); +}; + /** * Button dropdown to show current user and logout link * @param {Model} model - root model of the application @@ -105,28 +185,6 @@ const saveUserProfileMenuItem = (model) => title: 'Save the columns size and visibility as your profile', }, 'Save Profile'); -/** - * Query button final state depends on the following states - * - services lookup - * - services result - * - query lookup - * @param {Model} model - root model of the application - * @returns {vnode} - the view of the query button - */ -const queryButton = (model) => h('button.btn', model.frameworkInfo.match({ - NotAsked: () => ({ disabled: true }), - Loading: () => ({ disabled: true, className: 'loading' }), - Success: (frameworkInfo) => ({ - title: frameworkInfo.mysql && frameworkInfo.mysql.status.ok - ? 'Query database with filters (Enter)' : 'Query service not configured', - disabled: !frameworkInfo.mysql || !frameworkInfo.mysql.status.ok || model.log.queryResult.isLoading(), - className: model.log.queryResult.isLoading() ? 'loading' : queryButtonType, - style: 'font-weight: bold', - onclick: () => toggleButtonStates(model, false), - }), - Failure: () => ({ disabled: true, className: 'danger' }), -}), 'Query'); - /** * Group of buttons which allow the user to engage with the download functionality * * Download queries logs - will create a file containing all logs from the table (visible/hidden) @@ -186,28 +244,6 @@ const zoomButtonGroup = (zoom) => }, h('span', { style: 'font-size:0.8em' }, iconPlus())), ]); -/** - * Live button final state depends on the following states - * - services lookup - * - services result - * - query lookup - * - websocket status - * @param {Model} model - root model of the application - * @returns {vnode} - the view of the live button - */ -const liveButton = (model) => h('button.btn', model.frameworkInfo.match({ - NotAsked: () => ({ disabled: true }), - Loading: () => ({ disabled: true, className: 'loading' }), - Success: (frameworkInfo) => ({ - title: frameworkInfo.infoLoggerServer.status.ok ? 'Stream logs with filtering' : 'Live service not configured', - disabled: !frameworkInfo.infoLoggerServer.status.ok || model.log.queryResult.isLoading(), - className: !model.ws.authed ? 'loading' : liveButtonType, - style: 'font-weight: bold', - onclick: () => toggleButtonStates(model, true), - }), - Failure: () => ({ disabled: true, className: 'danger' }), -}), 'Live', ' ', liveButtonIcon); - /** * Method to toggle states of the buttons(Query/Live) depending on the mode the tool is running on * @param {Model} model - root model of the application diff --git a/InfoLogger/public/logFilter/LogFilter.js b/InfoLogger/public/logFilter/LogFilter.js index 7e70dd646..310007dfd 100644 --- a/InfoLogger/public/logFilter/LogFilter.js +++ b/InfoLogger/public/logFilter/LogFilter.js @@ -429,8 +429,8 @@ export default class LogFilter extends Observable { $in: ['I', 'W', 'E', 'F'], }, level: { - max: 1, - $max: 1, + max: null, + $max: null, }, }; this.notify(); diff --git a/InfoLogger/public/view.js b/InfoLogger/public/view.js index 35057213c..dc45e9efa 100644 --- a/InfoLogger/public/view.js +++ b/InfoLogger/public/view.js @@ -16,7 +16,7 @@ import { h, notification } from '/js/src/index.js'; import tableFilters from './logFilter/tableFilters.js'; import commandFilters from './logFilter/commandFilters.js'; -import commandLogs from './log/commandLogs.js'; +import { commandLogs } from './log/commandLogs.js'; import statusBar from './log/statusBar.js'; import inspector from './log/inspector.js'; import tableLogsHeader from './log/tableLogsHeader.js'; diff --git a/InfoLogger/test/lib/controller/mocha-query-controller.test.js b/InfoLogger/test/lib/controller/mocha-query-controller.test.js index 877012255..4157fe38b 100644 --- a/InfoLogger/test/lib/controller/mocha-query-controller.test.js +++ b/InfoLogger/test/lib/controller/mocha-query-controller.test.js @@ -16,6 +16,7 @@ const assert = require('assert'); const { QueryController } = require('../../../lib/controller/QueryController'); const { spy, stub } = require('sinon'); const { TimeoutError } = require('@aliceo2/web-ui'); +const { AbortError } = require('../../../lib/utils/queryCancellation'); describe('QueryController test suite', () => { describe('getQueryStats() - test suite', () => { @@ -106,11 +107,20 @@ describe('QueryController test suite', () => { criterias: { key: 'value' }, options: { }, }; + const callbacks = {}; + res.on = (event, fn) => { + callbacks[event] = fn; + return res; + }; queryService.queryFromFilters.resolves(logs); await queryController.getLogs({ body }, res); - assert.ok(queryService.queryFromFilters.calledWith(body.criterias, body.options)); + assert.ok(queryService.queryFromFilters.called); + const call = queryService.queryFromFilters.getCall(0); + assert.strictEqual(call.args[0], body.criterias); + assert.strictEqual(call.args[1], body.options); + assert.ok(call.args[2], 'AbortSignal'); // third arg should be the signal assert.ok(res.status.calledWith(200)); assert.ok(res.json.calledWith(logs)); }); @@ -120,6 +130,11 @@ describe('QueryController test suite', () => { criterias: { key: 'value' }, options: {}, }; + const callbacks = {}; + res.on = (event, fn) => { + callbacks[event] = fn; + return res; + }; queryService.queryFromFilters.rejects(new TimeoutError('QUERY TIMED OUT')); await queryController.getLogs({ body }, res); @@ -127,5 +142,78 @@ describe('QueryController test suite', () => { assert.ok(res.status.calledWith(408)); assert.ok(res.json.calledWith({ title: 'Timeout', message: 'QUERY TIMED OUT', status: 408 })); }); + + it('should not send response when client closes connection before query completes', async () => { + const body = { + criterias: { key: 'value' }, + options: {}, + }; + const callbacks = {}; + res.on = (event, fn) => { + callbacks[event] = fn; + return res; + }; + + // Query that completes after close event + queryService.queryFromFilters.callsFake(() => new Promise((resolve) => { + setTimeout(() => { + resolve([{ id: 1, message: 'log1' }]); + }, 50); + })); + + const queryPromise = queryController.getLogs({ body }, res); + // Simulate client closing connection before query completes + await new Promise((r) => setTimeout(r, 10)); + callbacks.close(); + await queryPromise; + + // Response should not be sent + assert.ok(res.status.notCalled); + assert.ok(res.json.notCalled); + }); + + it('should successfully return logs even if close event fires after finish event', async () => { + const logs = [{ id: 1, message: 'log1' }]; + const body = { + criterias: { key: 'value' }, + options: {}, + }; + const callbacks = {}; + res.on = (event, fn) => { + callbacks[event] = fn; + return res; + }; + + queryService.queryFromFilters.resolves(logs); + + const queryPromise = queryController.getLogs({ body }, res); + await queryPromise; + + // Simulate normal flow: finish fires, then close + callbacks.finish(); + callbacks.close(); + + assert.ok(res.status.calledWith(200)); + assert.ok(res.json.calledWith(logs)); + }); + + it('should handle QUERY_CANCELLED error and not send response', async () => { + const body = { + criterias: { key: 'value' }, + options: {}, + }; + const callbacks = {}; + res.on = (event, fn) => { + callbacks[event] = fn; + return res; + }; + queryService.queryFromFilters.rejects(new AbortError()); + + await queryController.getLogs({ body }, res); + + // Should not send any response on cancellation + assert.ok(res.status.notCalled); + assert.ok(res.json.notCalled); + }); }); }); diff --git a/InfoLogger/test/lib/services/mocha-query-service.test.js b/InfoLogger/test/lib/services/mocha-query-service.test.js index ff3470ddd..124662017 100644 --- a/InfoLogger/test/lib/services/mocha-query-service.test.js +++ b/InfoLogger/test/lib/services/mocha-query-service.test.js @@ -16,6 +16,7 @@ const assert = require('assert'); const sinon = require('sinon'); const { QueryService } = require('../../../lib/services/QueryService.js'); const { UnauthorizedAccessError, TimeoutError, InvalidInputError } = require('@aliceo2/web-ui'); +const { AbortError } = require('../../../lib/utils/queryCancellation'); describe('\'QueryService\' test suite', () => { const filters = { @@ -233,17 +234,22 @@ describe('\'QueryService\' test suite', () => { describe('queryFromFilters() - test suite', () => { it('should throw an error when unable to query(API) due to rejected promise', async () => { const sqlDataSource = new QueryService(); - sqlDataSource._pool = { + const connection = { query: sinon.stub().rejects({ code: 'ER_ACCESS_DENIED_ERROR', errno: 1045, sqlMessage: 'Access denied', }), + release: sinon.stub(), + }; + sqlDataSource._pool = { + getConnection: sinon.stub().resolves(connection), }; await assert.rejects( sqlDataSource.queryFromFilters(realFilters, { limit: 10 }), new UnauthorizedAccessError('SQL: [ER_ACCESS_DENIED_ERROR, 1045] Access denied'), ); + assert.strictEqual(connection.release.calledOnce, true); }); it('should successfully return result when filters are provided for querying', async () => { @@ -251,11 +257,15 @@ describe('\'QueryService\' test suite', () => { + 'AND NOT(`hostname` = ? AND `hostname` IS NOT NULL) AND `severity` IN (?) ORDER BY `TIMESTAMP` LIMIT 10'; const sqlDataSource = new QueryService(); - sqlDataSource._pool = { + const connection = { query: sinon.stub().resolves([ { hostname: 'test', severity: 'W' }, { hostname: 'test', severity: 'I' }, ]), + release: sinon.stub(), + }; + sqlDataSource._pool = { + getConnection: sinon.stub().resolves(connection), }; const result = await sqlDataSource.queryFromFilters(realFilters, { limit: 10 }); delete result.time; @@ -270,6 +280,7 @@ describe('\'QueryService\' test suite', () => { queryAsString: query, }; assert.deepStrictEqual(result, expectedResult); + assert.strictEqual(connection.release.calledOnce, true); }); it('should log every executed sql query as debug', async () => { @@ -277,14 +288,76 @@ describe('\'QueryService\' test suite', () => { sqlDataSource._logger = { debugMessage: sinon.stub(), }; - sqlDataSource._pool = { + const connection = { query: sinon.stub().resolves([{ hostname: 'test', severity: 'W' }]), + release: sinon.stub(), + }; + sqlDataSource._pool = { + getConnection: sinon.stub().resolves(connection), }; await sqlDataSource.queryFromFilters(realFilters, { limit: 10 }); const completeSqlQuery = "SELECT * FROM `messages` WHERE `timestamp`>='1563794601.351' AND" + " `timestamp`<='1563794661.354' AND `hostname` = 'test' AND NOT(`hostname` = 'testEx' AND" + " `hostname` IS NOT NULL) AND `severity` IN ('D','W') ORDER BY `TIMESTAMP` LIMIT 10;"; assert.strictEqual(sqlDataSource._logger.debugMessage.calledWith(`SQL to execute: ${completeSqlQuery}`), true); + assert.strictEqual(connection.release.calledOnce, true); + }); + + it('should throw QUERY_CANCELLED when signal is already aborted before query execution', async () => { + const sqlDataSource = new QueryService(); + const connection = { + query: sinon.stub().resolves([]), + release: sinon.stub(), + }; + sqlDataSource._pool = { + getConnection: sinon.stub().resolves(connection), + }; + + const controller = new AbortController(); + controller.abort(); + + await assert.rejects( + sqlDataSource.queryFromFilters(realFilters, { limit: 10 }, controller.signal), + (error) => + error instanceof AbortError + && error.code === 'QUERY_CANCELLED' + && error.message === 'Query cancelled by client', + ); + assert.strictEqual(connection.query.called, false); + assert.strictEqual(connection.release.calledOnce, true); + }); + + it('should destroy connection and throw QUERY_CANCELLED when signal aborts during query execution', async () => { + const sqlDataSource = new QueryService(); + let rejectQuery = null; + const connection = { + query: sinon.stub().callsFake(() => new Promise((resolve, reject) => { + rejectQuery = reject; + })), + release: sinon.stub(), + destroy: sinon.stub().callsFake(() => { + rejectQuery({ + code: 'ER_ACCESS_DENIED_ERROR', + errno: 1045, + sqlMessage: 'Access denied', + }); + }), + }; + sqlDataSource._pool = { + getConnection: sinon.stub().resolves(connection), + }; + + const controller = new AbortController(); + const queryPromise = sqlDataSource.queryFromFilters(realFilters, { limit: 10 }, controller.signal); + await new Promise((resolve) => setTimeout(resolve, 0)); + controller.abort(); + + await assert.rejects( + queryPromise, + (error) => error instanceof AbortError && error.code === 'QUERY_CANCELLED' && error.message === 'Query cancelled by client', + ); + assert.strictEqual(connection.destroy.calledOnce, true); + assert.strictEqual(connection.release.called, false); }); }); diff --git a/InfoLogger/test/lib/utils/mocha-query-cancellation.js b/InfoLogger/test/lib/utils/mocha-query-cancellation.js new file mode 100644 index 000000000..809931f3e --- /dev/null +++ b/InfoLogger/test/lib/utils/mocha-query-cancellation.js @@ -0,0 +1,89 @@ +/** + * @license + * Copyright 2019-2020 CERN and copyright holders of ALICE O2. + * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders. + * All rights not expressly granted are reserved. + * + * This software is distributed under the terms of the GNU General Public + * License v3 (GPL Version 3), copied verbatim in the file "COPYING". + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +const assert = require('assert'); +const sinon = require('sinon'); +const { + AbortError, + throwIfQueryAborted, + attachAbortDestroyHandler, +} = require('../../../lib/utils/queryCancellation.js'); + +describe('\'queryCancellation\' utils test suite', () => { + describe('\'throwIfQueryAborted()\' - test suite', () => { + it('should not throw when signal is missing', () => { + assert.doesNotThrow(() => throwIfQueryAborted(null)); + }); + + it('should not throw when signal is not aborted', () => { + const controller = new AbortController(); + assert.doesNotThrow(() => throwIfQueryAborted(controller.signal)); + }); + + it('should throw a QUERY_CANCELLED error when signal is aborted', () => { + const controller = new AbortController(); + controller.abort(); + + assert.throws( + () => throwIfQueryAborted(controller.signal), + (error) => { + assert.ok(error instanceof AbortError); + assert.strictEqual(error.message, 'Query cancelled by client'); + assert.strictEqual(error.code, 'QUERY_CANCELLED'); + return true; + }, + ); + }); + }); + + describe('\'attachAbortDestroyHandler()\' - test suite ', () => { + it('should return a noop cleanup when signal is missing', () => { + const connection = { destroy: sinon.spy() }; + const onDestroyed = sinon.spy(); + + const detach = attachAbortDestroyHandler(null, connection, onDestroyed); + + assert.strictEqual(typeof detach, 'function'); + assert.doesNotThrow(() => detach()); + assert.strictEqual(connection.destroy.callCount, 0); + assert.strictEqual(onDestroyed.callCount, 0); + }); + + it('should destroy connection and call callback when signal aborts', () => { + const controller = new AbortController(); + const connection = { destroy: sinon.spy() }; + const onDestroyed = sinon.spy(); + + const detach = attachAbortDestroyHandler(controller.signal, connection, onDestroyed); + controller.abort(); + detach(); + + assert.strictEqual(onDestroyed.callCount, 1); + assert.strictEqual(connection.destroy.callCount, 1); + }); + + it('should not destroy connection after handler is detached', () => { + const controller = new AbortController(); + const connection = { destroy: sinon.spy() }; + const onDestroyed = sinon.spy(); + + const detach = attachAbortDestroyHandler(controller.signal, connection, onDestroyed); + detach(); + controller.abort(); + + assert.strictEqual(onDestroyed.callCount, 0); + assert.strictEqual(connection.destroy.callCount, 0); + }); + }); +}); diff --git a/InfoLogger/test/mocha-index.js b/InfoLogger/test/mocha-index.js index 3ec3929bb..60364ea04 100644 --- a/InfoLogger/test/mocha-index.js +++ b/InfoLogger/test/mocha-index.js @@ -15,6 +15,8 @@ /* eslint-disable no-console */ const puppeteer = require('puppeteer'); const assert = require('assert'); +const fs = require('fs/promises'); +const path = require('path'); const {spawn} = require('child_process'); const config = require('./test-config.js'); @@ -34,6 +36,8 @@ describe('InfoLogger', function() { let subprocess; // web-server runs into a subprocess let subprocessOutput = ''; let ilgServer; + const testDbSourcePath = path.join(__dirname, 'testdb.json'); + const testDbRunningPath = path.join(__dirname, 'testdb-running.json'); this.timeout(30000); this.slow(1000); @@ -41,6 +45,8 @@ describe('InfoLogger', function() { const baseUrl = `http://${config.http.hostname}:${config.http.port}/`; before(async () => { + await fs.copyFile(testDbSourcePath, testDbRunningPath); + // Add error handlers for uncaught errors process.on('unhandledRejection', (error) => { console.error('[Test Setup] Unhandled Promise Rejection at:', new Date().toISOString()); @@ -68,7 +74,7 @@ describe('InfoLogger', function() { subprocess.on('error', (error) => console.error(`Server failed due to: ${error}`)) // Start browser to test UI - browser = await puppeteer.launch({headless: 'new', args: ['--no-sandbox', '--disable-setuid-sandbox']}); + browser = await puppeteer.launch({headless: true, args: ['--no-sandbox', '--disable-setuid-sandbox']}); page = await browser.newPage(); await page.setViewport({ width: 1440, height: 900 }); // 15" screen equivalent @@ -93,12 +99,12 @@ describe('InfoLogger', function() { } }); - it('should have redirected to default page "/?q={"severity":{"in":"I W E F"},"level":{"max":1}}"', async function() { + it('should have redirected to default page "/?q={"severity":{"in":"I W E F"}}"', async function() { await page.goto(baseUrl, {waitUntil: 'networkidle0'}); const location = await page.evaluate(() => window.location); const search = decodeURIComponent(location.search); - assert.deepStrictEqual(search, '?q={"severity":{"in":"I W E F"},"level":{"max":1}}'); + assert.deepStrictEqual(search, '?q={"severity":{"in":"I W E F"}}'); }); require('./public/user-actions-mocha'); @@ -110,14 +116,18 @@ describe('InfoLogger', function() { require('./public/log-context-menu-mocha'); after(async () => { - await browser.close(); - console.log('---------------------------------------------'); - console.log('Output of server logs for the previous tests:'); - console.log('---------------------------------------------'); - console.log(subprocessOutput); - console.log('---------------------------------------------'); - subprocess.kill(); - closeServer(ilgServer); + try { + await browser.close(); + console.log('---------------------------------------------'); + console.log('Output of server logs for the previous tests:'); + console.log('---------------------------------------------'); + console.log(subprocessOutput); + console.log('---------------------------------------------'); + subprocess.kill(); + closeServer(ilgServer); + } finally { + await fs.rm(testDbRunningPath, { force: true }); + } }); }); diff --git a/InfoLogger/test/public/live-mode-mocha.js b/InfoLogger/test/public/live-mode-mocha.js index 0b7cef97e..e7e7aac63 100644 --- a/InfoLogger/test/public/live-mode-mocha.js +++ b/InfoLogger/test/public/live-mode-mocha.js @@ -27,7 +27,7 @@ describe('Live Mode test-suite', async () => { const location = await page.evaluate(() => window.location); const search = decodeURIComponent(location.search); - assert.deepStrictEqual(search, '?q={"severity":{"in":"I W E F"},"level":{"max":1}}'); + assert.deepStrictEqual(search, '?q={"severity":{"in":"I W E F"}}'); }); it('should successfully enable LIVE mode', async () => { diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index 21eb8a098..32a4afa87 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -95,7 +95,7 @@ describe('Filter actions test-suite', async () => { }); it('should redirect to default filters and show JSON parse error on malformed q in URI', async () => { - const expectedDefaultParams = '?q={"severity":{"in":"I W E F"},"level":{"max":1}}'; + const expectedDefaultParams = '?q={"severity":{"in":"I W E F"}}'; const locationAndNotification = await page.evaluate(() => { const params = { q: '{"severity":{"in":"W I E F"' }; @@ -116,8 +116,8 @@ describe('Filter actions test-suite', async () => { it('should update URI with new encoded "match" criteria', async () => { /* eslint-disable max-len */ - const decodedParams = '?q={"hostname":{"match":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"},"level":{"max":1}}'; - const expectedParams = '?q={%22hostname%22:{%22match%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22},%22level%22:{%22max%22:1}}'; + const decodedParams = '?q={"hostname":{"match":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"}}'; + const expectedParams = '?q={%22hostname%22:{%22match%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22}}'; const searchParams = await page.evaluate(() => { window.model.log.filter.setCriteria('hostname', 'match', '"%ald_qdip01%'); window.model.updateRouteOnModelChange(); @@ -130,8 +130,8 @@ describe('Filter actions test-suite', async () => { it('should update URI with new encoded "exclude" criteria', async () => { /* eslint-disable max-len */ - const decodedParams = '?q={"hostname":{"exclude":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"},"level":{"max":1}}'; - const expectedParams = '?q={%22hostname%22:{%22exclude%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22},%22level%22:{%22max%22:1}}'; + const decodedParams = '?q={"hostname":{"exclude":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"}}'; + const expectedParams = '?q={%22hostname%22:{%22exclude%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22}}'; const searchParams = await page.evaluate(() => { window.model.log.filter.resetCriteria(); window.model.log.filter.setCriteria('hostname', 'exclude', '"%ald_qdip01%'); diff --git a/InfoLogger/test/public/query-mode-mocha.js b/InfoLogger/test/public/query-mode-mocha.js index 36fd85cad..1eead702d 100644 --- a/InfoLogger/test/public/query-mode-mocha.js +++ b/InfoLogger/test/public/query-mode-mocha.js @@ -10,9 +10,7 @@ * In applying this license CERN does not waive the privileges and immunities * granted to it by virtue of its status as an Intergovernmental Organization * or submit itself to any jurisdiction. -*/ - -/* eslint-disable max-len */ + */ const assert = require('assert'); const test = require('../mocha-index'); @@ -39,6 +37,41 @@ const TEXT_FILTER_FIELD_BY_OPERATOR = { * @param {string} [options.textFilterOperator] - operator to set before querying * @returns {Promise<{confirmCalls: number, postCalls: number}>} */ +/** + * Sets up common browser-context state for cancel query tests: + * confirms all dialogs, mocks frameworkInfo as healthy, and resets log/filter state. + * @param {Page} page - puppeteer page + * @returns {Promise} + */ +const setupQueryTestState = (page) => + page.evaluate(() => { + window.confirm = () => true; + window.model.frameworkInfo = { + isSuccess: () => true, + payload: { mysql: { status: { ok: true } } }, + match: ({ Success }) => Success({ mysql: { status: { ok: true } } }), + }; + window.model.log.filter.resetCriteria(); + window.model.log.empty(); + }); + +/** + * Starts a never-resolving query in the browser context, then immediately cancels it. + * Useful as a shared setup step for tests that need to assert state after a cancellation. + * @param {Page} page - puppeteer page + * @returns {Promise} + */ +const startAndCancelQuery = (page) => + page.evaluate(async () => { + window.fetch = (_url, { signal } = {}) => new Promise((_, reject) => { + signal?.addEventListener('abort', () => reject(new DOMException('AbortError', 'AbortError'))); + }); + const queryPromise = window.model.log.query(); + await new Promise((resolve) => setTimeout(resolve, 50)); + window.model.log.cancelQuery(); + await queryPromise; + }); + const runQueryWithMocks = (page, { confirmReturn, textFilterOperator }) => // Sets up mocks for confirmation dialog and post request, needs to be run in the browser context page.evaluate(async ({ @@ -55,15 +88,16 @@ const runQueryWithMocks = (page, { confirmReturn, textFilterOperator }) => return confirmReturn; }; - window.model.loader.post = async () => { + window.fetch = async () => { postCalls += 1; - return { ok: true, result: { rows: [] } }; + return { ok: true, status: 200, json: async () => JSON.stringify({ rows: [] }) }; }; // Mock the frameworkInfo to make the query method think the query service is available in its check window.model.frameworkInfo = { isSuccess: () => true, payload: { mysql: { status: { ok: true } } }, + match: ({ Success }) => Success({ mysql: { status: { ok: true } } }), }; // Default state of filters includes no text filters @@ -94,9 +128,7 @@ describe('Query Mode test-suite', async () => { it('should fail because it is not configured', async () => { try { - await page.evaluate(async () => { - return await window.model.log.query(); - }); + await page.evaluate(async () => await window.model.log.query()); assert.fail(); } catch (e) { // code failed, so it is a successful test @@ -132,4 +164,86 @@ describe('Query Mode test-suite', async () => { } }); }); + + describe('cancel query - AbortController', () => { + beforeEach(async () => { + await setupQueryTestState(page); + }); + + it('should display a Cancel button while a query is in flight and the Query button should be absent', async () => { + const result = await page.evaluate(async () => { + // Never-resolving fetch to keep query in loading state, respects abort signal to allow clean teardown + window.fetch = (_url, { signal } = {}) => new Promise((_, reject) => { + signal?.addEventListener('abort', () => reject(new DOMException('AbortError', 'AbortError'))); + }); + + const queryPromise = window.model.log.query(); + + await new Promise((r) => setTimeout(r, 50)); + + const cancelButton = document.querySelector('button#cancel-query-button'); + const queryButton = document.querySelector('button#query-button'); + + // Clean up via cancel so the abort signal rejects the fetch and queryPromise resolves + window.model.log.cancelQuery(); + await queryPromise; + + return { + cancelButtonPresent: cancelButton !== null && cancelButton.textContent.includes('Cancel'), + queryButtonAbsent: queryButton === null, + }; + }); + + assert.ok(result.cancelButtonPresent, 'Cancel button should be visible while query is loading'); + assert.ok(result.queryButtonAbsent, 'Query button should not be visible while query is loading'); + }); + + it('should not mutate list or stats when a query is cancelled via the AbortController', async () => { + await page.evaluate(() => { + const existingLogs = [ + { severity: 'E', message: 'existing error', timestamp: Date.now() }, + { severity: 'I', message: 'existing info', timestamp: Date.now() }, + ]; + existingLogs.forEach((log) => window.model.log.addLog(log)); + }); + + await startAndCancelQuery(page); + + const result = await page.evaluate(() => ({ + listLength: window.model.log.list.length, + stats: window.model.log.stats, + abortControllerCleared: window.model.log.queryAbortController === null, + })); + + assert.strictEqual(result.listLength, 2, 'list should still contain the pre-existing logs after cancellation'); + assert.strictEqual(result.stats.error, 1, 'error stat should reflect only the pre-existing error log'); + assert.strictEqual(result.stats.info, 1, 'info stat should reflect only the pre-existing info log'); + assert.ok(result.abortControllerCleared, 'queryAbortController should be null after cancellation'); + }); + + it('should allow a new query to start successfully after a previous one was cancelled', async () => { + await startAndCancelQuery(page); + + const result = await page.evaluate(async () => { + // Second query — resolves successfully with one row + const fakeRow = { severity: 'I', message: 'ok', timestamp: Date.now() }; + window.fetch = async () => ({ + ok: true, + status: 200, + json: async () => ({ rows: [fakeRow], count: 1 }), + }); + await window.model.log.query(); + + return { + isLoading: window.model.log.queryResult.isLoading(), + isSuccess: window.model.log.queryResult.isSuccess(), + listLength: window.model.log.list.length, + }; + }); + await new Promise((r) => setTimeout(r, 200)); // wait for state to update after query + assert.ok(!result.isLoading, 'query should not be stuck in loading state after second query'); + assert.ok(result.isSuccess, 'second query should succeed'); + assert.strictEqual(result.listLength, 1, 'list should contain the row from the second query'); + }); + }); }); diff --git a/InfoLogger/test/public/user-actions-mocha.js b/InfoLogger/test/public/user-actions-mocha.js index 6a52263a1..958400604 100644 --- a/InfoLogger/test/public/user-actions-mocha.js +++ b/InfoLogger/test/public/user-actions-mocha.js @@ -64,7 +64,9 @@ describe('User Profile test-suite', async () => { it('successfully save the profile of the user when pressed the "Save Profile" menu-item', async () => { await page.evaluate(() => { + // clicking on date column header to change its size and visibility document.querySelector('body > div:nth-child(2) > div > header:nth-child(2) > table > tbody > tr > td > button').click(); + // saving to profile then saving the profile document.querySelector('body > div:nth-child(2) > div > header > div > div > div > div:nth-child(3)').click(); }); @@ -72,6 +74,22 @@ describe('User Profile test-suite', async () => { assert.ok(!actionDropdownClosed); }); + it('should successfully load profile saved for user when accessing the page', async () => { + await page.goto( + `${baseUrl}?personid=1&username=test&name=Test&access=admin&token=${testToken}`, + { waitUntil: 'networkidle0' }, + ); + + // Wait for the profile to be fully loaded + await page.waitForFunction( + () => window.model?.userProfile?.payload?.content?.colsHeader?.date !== undefined, + { timeout: 5000 }, + ); + + const userProfile = await page.evaluate(() => window.model.userProfile.payload); + assert.ok(userProfile.content.colsHeader.date.visible, 'Date column should be visible'); + }); + it('should have a button in action dropdown button to view info about the framework', async () => { const profileMenuItem = await page.evaluate(() => { const { title } = document.querySelector('body > div:nth-child(2) > div > header > div > div > div > div:nth-child(2)'); @@ -81,21 +99,5 @@ describe('User Profile test-suite', async () => { assert.strictEqual(profileMenuItem.title, 'Show/Hide details about the framework'); assert.strictEqual(profileMenuItem.innerText, 'About'); }); - - it('should successfully load profile saved for user when accessing the page', async () => { - await page.goto( - `${baseUrl}?personid=1&username=test&name=Test&access=admin&token=${testToken}`, - { waitUntil: 'networkidle0' }, - ); - const userProfile = await page.evaluate(() => { - window.model.table.colsHeader.date.size = 'cell-xl'; - document.querySelector('body > div:nth-child(2) > div > header:nth-child(2) > table > tbody > tr > td > button') - .click(); - document.querySelector('body > div:nth-child(2) > div > header > div > div > div > div:nth-child(3)') - .click(); - return window.model.userProfile.payload; - }); - assert.ok(!userProfile.content.colsHeader.date.visible); - }); }); }); diff --git a/InfoLogger/test/test-config.js b/InfoLogger/test/test-config.js index 885acf844..a697de6ea 100644 --- a/InfoLogger/test/test-config.js +++ b/InfoLogger/test/test-config.js @@ -33,5 +33,5 @@ module.exports = { expiration: '60s', maxAge: '2', }, - dbFile: './test/testdb.json', + dbFile: './test/testdb-running.json', };