Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/domain/dtos/GetAllEnvironmentsDto.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
const Joi = require('joi');
const PaginationDto = require('./PaginationDto');
const { FromToFilterDto } = require('./filters/FromToFilterDto');
const { validateRange, RANGE_INVALID } = require('../../utilities/rangeUtils.js');

/**
* Separate filter DTO for get all environments, because EnvironmentsFilterDto
Expand All @@ -23,7 +24,10 @@ const FilterDto = Joi.object({
ids: Joi.string().trim().optional(),
currentStatus: Joi.string().trim().optional(),
statusHistory: Joi.string().trim().optional(),
runNumbers: Joi.string().trim().optional(),
runNumbers: Joi.string().trim().custom(validateRange).messages({
[RANGE_INVALID]: '{{#message}}',
'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
}).optional(),
created: FromToFilterDto,
});

Expand Down
5 changes: 3 additions & 2 deletions lib/domain/dtos/filters/LhcFillsFilterDto.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
* or submit itself to any jurisdiction.
*/
const Joi = require('joi');
const { validateRange } = require('../../../utilities/rangeUtils');
const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils');
const { validateTimeDuration } = require('../../../utilities/validateTime');

exports.LhcFillsFilterDto = Joi.object({
hasStableBeams: Joi.boolean(),
fillNumbers: Joi.string().trim().custom(validateRange).messages({
'any.invalid': '{{#message}}',
[RANGE_INVALID]: '{{#message}}',
'string.base': 'Fill numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
}),
runDuration: validateTimeDuration,
beamDuration: validateTimeDuration,
Expand Down
5 changes: 3 additions & 2 deletions lib/domain/dtos/filters/RunFilterDto.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const { IntegerComparisonDto, FloatComparisonDto } = require('./NumericalCompari
const { RUN_CALIBRATION_STATUS } = require('../../enums/RunCalibrationStatus.js');
const { RUN_DEFINITIONS } = require('../../enums/RunDefinition.js');
const { singleRunsCollectionCustomCheck } = require('../utils.js');
const { validateRange } = require('../../../utilities/rangeUtils.js');
const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils.js');

const DetectorsFilterDto = Joi.object({
operator: Joi.string().valid('or', 'and', 'none').required(),
Expand All @@ -33,7 +33,8 @@ const EorReasonFilterDto = Joi.object({

exports.RunFilterDto = Joi.object({
runNumbers: Joi.string().trim().custom(validateRange).messages({
'any.invalid': '{{#message}}',
[RANGE_INVALID]: '{{#message}}',
'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
}),
calibrationStatuses: Joi.array().items(...RUN_CALIBRATION_STATUS),
definitions: CustomJoi.stringArray().items(Joi.string().uppercase().trim().valid(...RUN_DEFINITIONS)),
Expand Down
14 changes: 9 additions & 5 deletions lib/usecases/environment/GetAllEnvironmentsUseCase.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const { ApiConfig } = require('../../config/index.js');
const { Op } = require('sequelize');
const { dataSource } = require('../../database/DataSource.js');
const { statusAcronyms } = require('../../domain/enums/StatusAcronyms.js');
const { unpackNumberRange } = require('../../utilities/rangeUtils.js');
const { splitStringToStringsTrimmed } = require('../../utilities/stringUtils.js');

/**
* Subquery to select the latest history item for each environment.
Expand Down Expand Up @@ -182,19 +184,21 @@ class GetAllEnvironmentsUseCase {
}

if (runNumbersExpression) {
// Convert the string of run numbers to an array of numbers
const filters = runNumbersExpression.split(',').map((filter) => Number(filter.trim()));
const runNumberCriteria = splitStringToStringsTrimmed(runNumbersExpression, ',');

if (filters.length) {
const finalRunNumberList = Array.from(unpackNumberRange(runNumberCriteria));

// Check that the final run numbers list contains at least one valid run number
if (finalRunNumberList.length > 0) {
filterQueryBuilder.include({
association: 'runs',
where: {
// Filter should be like with only one filter and exact with more than one filter
runNumber: { [filters.length === 1 ? Op.substring : Op.in]: filters },
runNumber: { [finalRunNumberList.length === 1 ? Op.substring : Op.in]: finalRunNumberList },
},
});
}
}
};

const filteredEnvironmentsIds = (await EnvironmentRepository.findAll(filterQueryBuilder)).map(({ id }) => id);
// If no environments match the filter, return an empty result
Expand Down
27 changes: 11 additions & 16 deletions lib/utilities/rangeUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,25 @@
* @param {*} helpers The helpers object
* @returns {Object} The value if validation passes
*/
export const RANGE_INVALID = 'range.invalid';

export const validateRange = (value, helpers) => {
const MAX_RANGE_SIZE = 100;

const numbers = value.split(',').map((number) => number.trim());

for (const number of numbers) {
if (number.includes('-')) {
// Check if '-' occurs more than once in this part of the range
if (number.lastIndexOf('-') !== number.indexOf('-')) {
return helpers.error('any.invalid', { message: `Invalid range: ${number}` });
}
const [start, end] = number.split('-').map((n) => Number(n));
const parts = number.split('-');
const [start, end] = parts.map((n) => parseInt(n, 10));

if (Number.isNaN(start) || Number.isNaN(end) || start > end) {
return helpers.error('any.invalid', { message: `Invalid range: ${number}` });
return helpers.error(RANGE_INVALID, { message: `Invalid range: ${number}` });
}
const rangeSize = end - start + 1;

if (rangeSize > MAX_RANGE_SIZE) {
return helpers.error('any.invalid', { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` });
}
} else {
// Prevent non-numeric input.
if (isNaN(number)) {
return helpers.error('any.invalid', { message: `Invalid number: ${number}` });
return helpers.error(RANGE_INVALID, { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` });
}
}
}
Expand All @@ -58,20 +53,20 @@ export const validateRange = (value, helpers) => {
* @returns {Set<Number>} set containing the unpacked range.
*/
export function unpackNumberRange(numbersRanges, rangeSplitter = '-') {
// Set to prevent duplicate values.
const resultNumbers = new Set();

numbersRanges.forEach((number) => {
if (number.includes(rangeSplitter)) {
const [start, end] = number.split(rangeSplitter).map((n) => parseInt(n, 10));
if (!Number.isNaN(start) && !Number.isNaN(end)) {
for (let i = start; i <= end; i++) {
resultNumbers.add(Number(i));
resultNumbers.add(i);
}
}
} else {
if (!isNaN(number)) {
resultNumbers.add(Number(number));
const num = Number(number);
if (!Number.isNaN(num)) {
resultNumbers.add(num);
}
}
});
Expand Down
47 changes: 47 additions & 0 deletions test/api/environments.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,53 @@ module.exports = () => {
expect(environments[0].id).to.be.equal('TDI59So3d');
expect(environments[1].id).to.be.equal('Dxi029djX');
});

it('should successfully filter environments with query on run number range', async () => {
const response = await request(server).get('/api/environments?filter[runNumbers]=100-105');

expect(response.status).to.equal(200);
const environments = response.body.data;
expect(environments.length).to.be.equal(2);
// Should include all environments with run numbers between 100 and 105
expect(environments[0].id).to.be.equal('TDI59So3d');
expect(environments[1].id).to.be.equal('Dxi029djX');
});

it('should successfully filter environments when run number filter contains empty entries', async () => {
const response = await request(server).get('/api/environments?filter[runNumbers]=103,,');

expect(response.status).to.equal(200);
const environments = response.body.data;
expect(environments).to.have.lengthOf(1);
expect(environments[0].id).to.be.equal('TDI59So3d');
});

it('should return 400 for an invalid run number range, first number greater than second', async () => {
const response = await request(server).get('/api/environments?filter[runNumbers]=105-100');

expect(response.status).to.equal(400);
const { errors } = response.body;
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
expect(rangeError.detail).to.equal('Invalid range: 105-100');
});

it('should return 400 for an invalid run number range, negative start number', async () => {
const response = await request(server).get('/api/environments?filter[runNumbers]=-100-105');

expect(response.status).to.equal(400);
const { errors } = response.body;
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
expect(rangeError.detail).to.equal('Invalid range: -100-105');
});

it('should return 400 for an invalid run number range, no start number', async () => {
const response = await request(server).get('/api/environments?filter[runNumbers]=-105');

expect(response.status).to.equal(400);
const { errors } = response.body;
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
expect(rangeError.detail).to.equal('Invalid range: -105');
});
});
describe('POST /api/environments', () => {
it('should return 201 if valid data is provided', async () => {
Expand Down
27 changes: 27 additions & 0 deletions test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,33 @@ module.exports = () => {
expect(environments.map(({ id }) => id)).to.have.members(['TDI59So3d', 'Dxi029djX']);
});

it('should successfully filter environments on a run number range spanning multiple environments', async () => {
getAllEnvsDto.query = { filter: { runNumbers: '100-103' } };
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);

expect(environments).to.be.an('array');
expect(environments.length).to.be.equal(2);
expect(environments.map(({ id }) => id)).to.have.members(['Dxi029djX', 'TDI59So3d']);
});

it('should successfully filter environments on a single-value run number range', async () => {
getAllEnvsDto.query = { filter: { runNumbers: '105-105' } };
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);

expect(environments).to.be.an('array');
expect(environments.length).to.be.equal(1);
expect(environments[0].id).to.be.equal('TDI59So3d');
});

it('should successfully filter environments when run number filter includes empty entries', async () => {
getAllEnvsDto.query = { filter: { runNumbers: '103, ' } };
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);

expect(environments).to.be.an('array');
expect(environments.length).to.be.equal(1);
expect(environments[0].id).to.be.equal('TDI59So3d');
});

it('should successfully filter environments on created from and to', async () => {
const from = Date.now() - 24 * 60 * 60 * 1000; // environment from 24h ago which was created by CreateEnvironmentUseCase.test.js
const to = Date.now() - 10;
Expand Down
24 changes: 1 addition & 23 deletions test/lib/utilities/rangeUtils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/

const Sinon = require('sinon');
const { validateRange, unpackNumberRange } = require('../../../lib/utilities/rangeUtils.js');
const { validateRange, unpackNumberRange, RANGE_INVALID } = require('../../../lib/utilities/rangeUtils.js');
const { expect } = require('chai');

module.exports = () => {
Expand Down Expand Up @@ -61,14 +61,6 @@ module.exports = () => {
expect(result).to.equal(input);
});

it('rejects non-numeric input', () => {
const input = '5,a,7';
validateRange(input, helpers);
expect(helpers.error.calledOnce).to.be.true;
expect(helpers.error.firstCall.args[0]).to.equal('any.invalid');
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid number: a' });
});

it('rejects range with non-numeric input', () => {
const input = '3-a';
validateRange(input, helpers);
Expand All @@ -90,20 +82,6 @@ module.exports = () => {
expect(result).to.equal(input);
});

it('rejects range containing more than one `-`', () => {
const input = '1-2-3';
validateRange(input, helpers);
expect(helpers.error.calledOnce).to.be.true;
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-3' });
});

it('rejects range containing more than one `-`, at end', () => {
const input = '1-2-';
validateRange(input, helpers);
expect(helpers.error.calledOnce).to.be.true;
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-' });
});

// MAX_RANGE_SIZE = 100, should this change, also change this test...
it('rejects a range that exceeds MAX_RANGE_SIZE', () => {
const input = '1-101';
Expand Down
8 changes: 7 additions & 1 deletion test/public/envs/overview.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,15 @@ module.exports = () => {
await expectAttributeValue(page, '.runs-filter input', 'placeholder', 'e.g. 553203, 553221, ...');
await expectAttributeValue(page, '.historyItems-filter input', 'placeholder', 'e.g. D-R-X');

// range of runNumbers
await fillInput(page, '.runs-filter input', '103-104', ['change']);
await waitForTableLength(page, 1);
// substring of a runNumber
await fillInput(page, '.runs-filter input', '10', ['change']);

await fillInput(page, '.id-filter input', 'Dxi029djX, TDI59So3d', ['change']);
await page.$eval('.status-filter #checkboxes-checkbox-DESTROYED', (element) => element.click());
await fillInput(page, '.runs-filter input', '10', ['change']);

await fillInput(page, '.historyItems-filter input', 'C-R-D-X', ['change']);

const createdAtPopoverSelector = await getPopoverSelector(await page.$('.createdAt-filter .popover-trigger'));
Expand Down
Loading