Enhance number field handling in utility functions and add tests
All checks were successful
farmcontrol/farmcontrol-api/pipeline/head This commit looks good

This commit introduces new utility functions to support number field validation and regex condition building in the `utils.js` file. It updates the `buildEquality`, `buildComparison`, and `parseLeafCondition` functions to accommodate number fields, ensuring proper handling of numeric values in filters. Additionally, a new test suite is added to validate the behavior of the `parseFilter` function with number fields, covering various scenarios including exact matches, wildcard handling, and non-numeric input. This enhancement improves the robustness of the filtering mechanism in the application.
This commit is contained in:
Tom Butcher 2026-08-30 12:48:04 +01:00
parent 5f59c23fbe
commit 5e4830b036
4 changed files with 167 additions and 16 deletions

View File

@ -0,0 +1,41 @@
import { describe, expect, it } from '@jest/globals';
import { parseFilter } from '../utils.js';
import { jobModel } from '../database/schemas/production/job.schema.js';
describe('parseFilter number fields', () => {
it('matches exact numeric quantity without regex', async () => {
await expect(parseFilter('quantity', '12', jobModel)).resolves.toEqual({ quantity: 12 });
});
it('matches multiple quantities with $in', async () => {
await expect(parseFilter('quantity', '1|2|3', jobModel)).resolves.toEqual({
quantity: { $in: [1, 2, 3] },
});
});
it('does not put $regex or $options on quantity for wildcards', async () => {
const result = await parseFilter('quantity', '1*', jobModel);
expect(result.quantity).toBeUndefined();
expect(JSON.stringify(result)).not.toMatch(/\$options/);
expect(result).toEqual({
$expr: {
$regexMatch: {
input: { $toString: '$quantity' },
regex: '^1.*$',
options: 'i',
},
},
});
});
it('treats non-numeric quantity text as no match', async () => {
await expect(parseFilter('quantity', 'abc', jobModel)).resolves.toEqual({
quantity: { $in: [] },
});
});
it('still uses $regex on string fields', async () => {
const result = await parseFilter('_reference', 'JOB*', jobModel);
expect(result._reference).toEqual({ $regex: '^JOB.*$', $options: 'i' });
});
});

View File

@ -776,6 +776,15 @@ export const getPropertyValues = async ({ model, property, filter = {} }) => {
return []; return [];
} }
try {
return await getPropertyValuesUnsafe({ model, property, filter });
} catch (error) {
logger.error('getPropertyValues error:', error);
return [];
}
};
const getPropertyValuesUnsafe = async ({ model, property, filter = {} }) => {
const convertedFilter = const convertedFilter =
filter && Object.keys(filter).length > 0 ? convertObjectIdStringsInFilter(filter) : {}; filter && Object.keys(filter).length > 0 ? convertObjectIdStringsInFilter(filter) : {};

View File

@ -40,11 +40,12 @@ import {
getJobNeighborsRouteHandler, getJobNeighborsRouteHandler,
} from '../../services/production/jobs.js'; } from '../../services/production/jobs.js';
import { convertPropertiesString, getFilter, getSort } from '../../utils.js'; import { convertPropertiesString, getFilter, getSort } from '../../utils.js';
import { jobModel } from '../../database/schemas/production/job.schema.js';
// list of jobs // list of jobs
router.get('/', isAuthenticated, checkPermissions('job', 'list'), async (req, res) => { router.get('/', isAuthenticated, checkPermissions('job', 'list'), async (req, res) => {
const { page, limit, property, search, sortProperty, sortOrder } = req.query; const { page, limit, property, search, sortProperty, sortOrder } = req.query;
const filter = await getFilter(req.query, listAllowedFilters); const filter = await getFilter(req.query, listAllowedFilters, true, jobModel);
listJobsRouteHandler( listJobsRouteHandler(
req, req,
res, res,
@ -70,10 +71,15 @@ router.get('/properties', checkPermissions('job', 'list'), isAuthenticated, asyn
router.get('/values', checkPermissions('job', 'list'), isAuthenticated, async (req, res) => { router.get('/values', checkPermissions('job', 'list'), isAuthenticated, async (req, res) => {
const { property } = req.query; const { property } = req.query;
const filter = await getFilter(req.query, listAllowedFilters, true); const filter = await getFilter(req.query, listAllowedFilters, true, jobModel);
var masterFilter = {}; var masterFilter = {};
if (req.query.masterFilter) { if (req.query.masterFilter) {
masterFilter = await getFilter(JSON.parse(req.query.masterFilter), listAllowedFilters, true); masterFilter = await getFilter(
JSON.parse(req.query.masterFilter),
listAllowedFilters,
true,
jobModel
);
} }
getJobPropertyValuesRouteHandler(req, res, property, filter, masterFilter); getJobPropertyValuesRouteHandler(req, res, property, filter, masterFilter);
}); });

View File

@ -84,6 +84,37 @@ function isBooleanField(property, model = null) {
return found; return found;
} }
function isNumberSchemaPath(path) {
if (!path) return false;
if (path.instance === 'Number') return true;
if (path.instance === 'Array') {
return getEmbeddedSchemaType(path)?.instance === 'Number';
}
return false;
}
function isNumberField(property, model = null) {
if (model?.schema) {
return isNumberSchemaPath(model.schema.path(property));
}
for (const entry of Object.values(models)) {
if (isNumberSchemaPath(entry.model?.schema?.path(property))) return true;
}
return false;
}
function buildNumberRegexCondition(property, pattern, negated = false) {
const expr = {
$regexMatch: {
input: { $toString: `$${property}` },
regex: pattern,
options: 'i',
},
};
return { query: { $expr: negated ? { $not: expr } : expr } };
}
function isObjectIdPath(property) { function isObjectIdPath(property) {
return property === '_id' || property.endsWith('._id'); return property === '_id' || property.endsWith('._id');
} }
@ -392,7 +423,14 @@ function stripIgnoreCase(str) {
} }
// Builds an equality condition (value, date range, or wildcard regex). // Builds an equality condition (value, date range, or wildcard regex).
function buildEquality(value, isDateField, isBooleanField = false, isObjectIdField = false) { function buildEquality(
value,
isDateField,
isBooleanField = false,
isObjectIdField = false,
isNumberField = false,
property = ''
) {
if (isBooleanField) return buildBooleanEquality(value); if (isBooleanField) return buildBooleanEquality(value);
if (isObjectIdField) { if (isObjectIdField) {
const { suffix } = parsePrefixedValue(value); const { suffix } = parsePrefixedValue(value);
@ -410,6 +448,12 @@ function buildEquality(value, isDateField, isBooleanField = false, isObjectIdFie
if (lower === 'false') return { value: false }; if (lower === 'false') return { value: false };
if (isObjectIdString(value)) return { value: new mongoose.Types.ObjectId(value) }; if (isObjectIdString(value)) return { value: new mongoose.Types.ObjectId(value) };
if (isNumeric(value)) return { value: Number(value) }; if (isNumeric(value)) return { value: Number(value) };
if (isNumberField) {
if (/[*?]/.test(value)) {
return buildNumberRegexCondition(property, buildWildcardRegexPattern(value));
}
return NO_MATCH_CONDITION;
}
return { op: buildRegexOp(buildWildcardRegexPattern(value)) }; return { op: buildRegexOp(buildWildcardRegexPattern(value)) };
} }
@ -419,7 +463,9 @@ function buildComparison(
value, value,
isDateField, isDateField,
isBooleanField = false, isBooleanField = false,
isObjectIdField = false isObjectIdField = false,
isNumberField = false,
property = ''
) { ) {
if (isBooleanField) return buildBooleanComparison(name, value); if (isBooleanField) return buildBooleanComparison(name, value);
if (isObjectIdField) { if (isObjectIdField) {
@ -431,7 +477,9 @@ function buildComparison(
} }
return NO_MATCH_CONDITION; return NO_MATCH_CONDITION;
} }
if (name === 'eq') return buildEquality(value, isDateField); if (name === 'eq') {
return buildEquality(value, isDateField, false, false, isNumberField, property);
}
if (name === 'ne') { if (name === 'ne') {
if (isDateField) { if (isDateField) {
@ -440,17 +488,30 @@ function buildComparison(
if (start && end) return { op: { $not: { $gte: start, $lte: end } } }; if (start && end) return { op: { $not: { $gte: start, $lte: end } } };
} }
if (/[*?]/.test(value)) { if (/[*?]/.test(value)) {
if (isNumberField) {
return buildNumberRegexCondition(property, buildWildcardRegexPattern(value), true);
}
return { op: { $not: buildRegexOp(buildWildcardRegexPattern(value)) } }; return { op: { $not: buildRegexOp(buildWildcardRegexPattern(value)) } };
} }
if (isNumberField && !isNumeric(value)) return NO_MATCH_CONDITION;
return { op: { $ne: coerceScalar(value) } }; return { op: { $ne: coerceScalar(value) } };
} }
if (isNumberField && !isNumeric(value)) return NO_MATCH_CONDITION;
// For dates, < and >= align to the start of the day, > and <= to the end. // For dates, < and >= align to the start of the day, > and <= to the end.
const boundary = name === 'gt' || name === 'lte' ? 'end' : 'start'; const boundary = name === 'gt' || name === 'lte' ? 'end' : 'start';
return { op: { [`$${name}`]: coerceBoundary(value, isDateField, boundary) } }; return { op: { [`$${name}`]: coerceBoundary(value, isDateField, boundary) } };
} }
function parseLeafComparisonOperators(token, isDateField, isBooleanField, isObjectIdField) { function parseLeafComparisonOperators(
token,
isDateField,
isBooleanField,
isObjectIdField,
isNumberField,
property
) {
const operators = [ const operators = [
['<>', 'ne'], ['<>', 'ne'],
['>=', 'gte'], ['>=', 'gte'],
@ -466,7 +527,9 @@ function parseLeafComparisonOperators(token, isDateField, isBooleanField, isObje
stripIgnoreCase(token.slice(symbol.length).trim()), stripIgnoreCase(token.slice(symbol.length).trim()),
isDateField, isDateField,
isBooleanField, isBooleanField,
isObjectIdField isObjectIdField,
isNumberField,
property
); );
} }
} }
@ -478,11 +541,13 @@ function parseLeafCondition(
rawToken, rawToken,
isDateField, isDateField,
isBooleanField = false, isBooleanField = false,
isObjectIdField = false isObjectIdField = false,
isNumberField = false,
property = ''
) { ) {
const token = rawToken.trim(); const token = rawToken.trim();
if (token === '') { if (token === '') {
if (isBooleanField || isObjectIdField) return NO_MATCH_CONDITION; if (isBooleanField || isObjectIdField || isNumberField) return NO_MATCH_CONDITION;
return { op: buildRegexOp('^$') }; return { op: buildRegexOp('^$') };
} }
@ -492,6 +557,11 @@ function parseLeafCondition(
if (isBooleanField || isObjectIdField) return NO_MATCH_CONDITION; if (isBooleanField || isObjectIdField) return NO_MATCH_CONDITION;
const lo = stripIgnoreCase(token.slice(0, rangeIdx).trim()); const lo = stripIgnoreCase(token.slice(0, rangeIdx).trim());
const hi = stripIgnoreCase(token.slice(rangeIdx + 2).trim()); const hi = stripIgnoreCase(token.slice(rangeIdx + 2).trim());
if (isNumberField) {
if ((lo !== '' && !isNumeric(lo)) || (hi !== '' && !isNumeric(hi))) {
return NO_MATCH_CONDITION;
}
}
const op = {}; const op = {};
if (lo !== '') op.$gte = coerceBoundary(lo, isDateField, 'start'); if (lo !== '') op.$gte = coerceBoundary(lo, isDateField, 'start');
if (hi !== '') op.$lte = coerceBoundary(hi, isDateField, 'end'); if (hi !== '') op.$lte = coerceBoundary(hi, isDateField, 'end');
@ -504,11 +574,20 @@ function parseLeafCondition(
token, token,
isDateField, isDateField,
isBooleanField, isBooleanField,
isObjectIdField isObjectIdField,
isNumberField,
property
); );
if (comparison) return comparison; if (comparison) return comparison;
return buildEquality(stripIgnoreCase(token), isDateField, isBooleanField, isObjectIdField); return buildEquality(
stripIgnoreCase(token),
isDateField,
isBooleanField,
isObjectIdField,
isNumberField,
property
);
} }
// Converts a condition descriptor into a MongoDB query object for `property`. // Converts a condition descriptor into a MongoDB query object for `property`.
@ -557,13 +636,21 @@ function buildCondition(
property, property,
isDateField, isDateField,
isBooleanField = false, isBooleanField = false,
isObjectIdField = false isObjectIdField = false,
isNumberField = false
) { ) {
if (node.type === 'leaf') { if (node.type === 'leaf') {
return parseLeafCondition(node.token, isDateField, isBooleanField, isObjectIdField); return parseLeafCondition(
node.token,
isDateField,
isBooleanField,
isObjectIdField,
isNumberField,
property
);
} }
const children = node.items.map((item) => const children = node.items.map((item) =>
buildCondition(item, property, isDateField, isBooleanField, isObjectIdField) buildCondition(item, property, isDateField, isBooleanField, isObjectIdField, isNumberField)
); );
return node.type === 'and' return node.type === 'and'
? combineAnd(children, property) ? combineAnd(children, property)
@ -770,8 +857,16 @@ async function parseFilter(property, value, model = null) {
const isDateField = looksLikeDateField(property); const isDateField = looksLikeDateField(property);
const isObjectIdField = fieldKind.kind === 'objectId'; const isObjectIdField = fieldKind.kind === 'objectId';
const isNumField = isNumberField(filterProperty, model);
const tree = parseExpression(expression); const tree = parseExpression(expression);
const condition = buildCondition(tree, filterProperty, isDateField, isBoolField, isObjectIdField); const condition = buildCondition(
tree,
filterProperty,
isDateField,
isBoolField,
isObjectIdField,
isNumField
);
return conditionToQuery(condition, filterProperty); return conditionToQuery(condition, filterProperty);
} }