Skip to content

Commit

Permalink
Remove kuzzle-common-objects from dependencies
Browse files Browse the repository at this point in the history 
# Description

* Update all dependencies, except for `js-combinatorics` which becomes TS-only after v1.0. This one will be updated when TS interfaces are added to Koncorde
* Remove kuzzle-common-objects from dependencies
  * This means that Koncorde now throws regular Error objects instead of BadRequestError

# Other changes

* Enhanced a few error messages that were a bit confusing.
* Added a new check to the `exists` keyword (object-form) to make Koncorde throw a proper "missing field attribute" error.
* Added a proper error explanation when attempting to parse regular expressions
* Standardized `npm unit-test` to `npm test:unit` and `npm test:unit:coverage`: the former is a lot faster, useful when working on the code, while the latter includes coverage information, useful for the CI
  • Loading branch information
@scottinet
scottinet authored May 27, 2021
1 parent 267a120 commit 77e3925
Show file tree
Hide file tree
Showing 20 changed files with 1,606 additions and 1,041 deletions.
3 changes: 1 addition & 2 deletions lib/transform/canonical.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

const Combinatorics = require('js-combinatorics');

const { BadRequestError } = require('kuzzle-common-objects');
const { Espresso } = require('espresso-logic-minimizer');

const strcmp = require('../util/stringCompare');
Expand Down Expand Up @@ -66,7 +65,7 @@ class Canonical {

const conditions = this._extractConditions(filters);
if (this.config.maxMinTerms && conditions.length > this.config.maxMinTerms) {
throw new BadRequestError('Maximum number of sub conditions reached.');
throw new Error('Maximum number of sub conditions reached.');
}

const normalized = this._normalize(filters, conditions);
Expand Down
16 changes: 1 addition & 15 deletions lib/transform/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
* limitations under the License.
*/

const {
KuzzleError,
InternalError: KuzzleInternalError
} = require('kuzzle-common-objects');

const Standardizer = require('./standardize');
const Canonical = require('./canonical');

Expand All @@ -49,16 +44,7 @@ class Transformer {
normalize(filters) {
const standardized = this.standardizer.standardize(filters);

try {
return this.canonical.convert(standardized);
}
catch (e) {
if (e instanceof KuzzleError) {
throw e;
}

throw new KuzzleInternalError(e);
}
return this.canonical.convert(standardized);
}

/**
Expand Down
89 changes: 47 additions & 42 deletions lib/transform/standardize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
* limitations under the License.
*/

const { BadRequestError } = require('kuzzle-common-objects');
const RE2 = require('re2');

const convertGeopoint = require('../util/convertGeopoint');
Expand Down Expand Up @@ -75,11 +74,11 @@ class Standardizer {
}

if (keywords.length > 1) {
throw new BadRequestError('Invalid filter syntax. Filters must have one keyword only');
throw new Error('Invalid filter syntax. Filters must have one keyword only');
}

if (!this.allowedKeywords.has(keywords[0])) {
throw new BadRequestError(`Unknown DSL keyword: ${keywords[0]}`);
throw new Error(`Unknown DSL keyword: ${keywords[0]}`);
}

return this[keywords[0]](filters);
Expand Down Expand Up @@ -107,19 +106,20 @@ class Standardizer {
// Syntax: {exists: '<field name>'}
if (typeof filter[name] === 'string') {
if (filter[name].length === 0) {
throw new BadRequestError(`${name}: cannot test empty field name`);
throw new Error(`${name}: cannot test empty field name`);
}

return parseFieldSyntax(filter[name], name);
}

// Syntax: {exists: {field: '<field name>'}}
const field = mustBeNonEmptyObject(filter[name], name);
requireAttribute(filter[name], 'exists', 'field');
onlyOneFieldAttribute(field, name);
mustBeString(filter[name], name, 'field');

if (filter[name].field.length === 0) {
throw new BadRequestError(`${name}: cannot test empty field name`);
throw new Error(`${name}: cannot test empty field name`);
}

return parseFieldSyntax(filter[name].field, name);
Expand All @@ -139,7 +139,7 @@ class Standardizer {
mustBeNonEmptyArray(filter.ids, 'ids', 'values');

if (filter.ids.values.findIndex(v => typeof v !== 'string') > -1) {
throw new BadRequestError('Array "values" in keyword "ids" can only contain strings');
throw new Error('Array "values" in keyword "ids" can only contain strings');
}

const result = {
Expand Down Expand Up @@ -191,19 +191,19 @@ class Standardizer {
let error = null;

if (index > -1) {
throw new BadRequestError(`"range.${rangeField}" accepts only the following attributes : gt, gte, lt, lte`);
throw new Error(`"range.${rangeField}" accepts only the following attributes : gt, gte, lt, lte`);
}

index = rangeValues.findIndex(v => typeof filter.range[rangeField][v] !== 'number');

if (index > -1) {
throw new BadRequestError(`"range.${rangeField}.${rangeValues[index]}" must be a number`);
throw new Error(`"range.${rangeField}.${rangeValues[index]}" must be a number`);
}

rangeValues.forEach(rangeType => {
if (rangeType.startsWith('lt')) {
if (high !== Infinity) {
error = new BadRequestError(`"range.${rangeField}:" only 1 upper boundary allowed`);
error = new Error(`"range.${rangeField}:" only 1 upper boundary allowed`);
}
else {
high = filter.range[rangeField][rangeType];
Expand All @@ -212,7 +212,7 @@ class Standardizer {

if (rangeType.startsWith('gt')) {
if (low !== -Infinity) {
error = new BadRequestError(`"range.${rangeField}:" only 1 lower boundary allowed`);
error = new Error(`"range.${rangeField}:" only 1 lower boundary allowed`);
}
else {
low = filter.range[rangeField][rangeType];
Expand All @@ -225,7 +225,7 @@ class Standardizer {
}

if (high <= low) {
throw new BadRequestError(`"range.${rangeField}:" lower boundary must be strictly inferior to the upper one`);
throw new Error(`"range.${rangeField}:" lower boundary must be strictly inferior to the upper one`);
}

return filter;
Expand All @@ -251,7 +251,7 @@ class Standardizer {
&& Object.keys(filter.regexp[regexpField]).length > 0);

if (!isObject && !isString) {
throw new BadRequestError(`regexp.${regexpField} must be either a string or a non-empty object`);
throw new Error(`regexp.${regexpField} must be either a string or a non-empty object`);
}

let regexValue;
Expand All @@ -262,7 +262,7 @@ class Standardizer {
}
else {
if (Object.keys(filter.regexp[regexpField]).findIndex(v => ['flags', 'value'].indexOf(v) === -1) > -1) {
throw new BadRequestError('Keyword "regexp" can only contain the following attributes: flags, value');
throw new Error('Keyword "regexp" can only contain the following attributes: flags, value');
}

requireAttribute(filter.regexp[regexpField], 'regexp', 'value');
Expand All @@ -279,7 +279,7 @@ class Standardizer {
new this.RegExpConstructor(regexValue, flags); //NOSONAR
}
catch (err) {
throw new BadRequestError(err.message);
throw new Error(`Cannot parse regexp expression "/${regexValue}/${flags}": ${err.message}`);
}

// standardize regexp to the unique format {<field>: {value, flags}}
Expand Down Expand Up @@ -320,7 +320,7 @@ class Standardizer {
mustBeNonEmptyArray(filter.in, 'in', inValue);

if (filter.in[inValue].findIndex(v => typeof v !== 'string') > -1) {
throw new BadRequestError(`Array "${inValue}" in keyword "in" can only contain strings`);
throw new Error(`Array "${inValue}" in keyword "in" can only contain strings`);
}

const result = {
Expand Down Expand Up @@ -364,7 +364,7 @@ class Standardizer {
const n = Number.parseFloat(bBox[v]);

if (isNaN(n)) {
throw new BadRequestError(`Invalid geoBoundingBox format: ${JSON.stringify(bBox)}`);
throw new Error(`Invalid geoBoundingBox format: ${JSON.stringify(bBox)}`);
}

standardized[v] = n;
Expand All @@ -386,7 +386,7 @@ class Standardizer {
}

if (BBOX_PROPERTIES.some(v => standardized[v] === null || standardized[v] === undefined)) {
throw new BadRequestError(`Unrecognized geo-point format in "geoBoundingBox.${field[0]}`);
throw new Error(`Unrecognized geo-point format in "geoBoundingBox.${field[0]}`);
}

return {
Expand All @@ -409,7 +409,7 @@ class Standardizer {
const fields = mustBeNonEmptyObject(filter.geoDistance, 'geoDistance');

if (fields.length !== 2 || !fields.includes('distance')) {
throw new BadRequestError('"geoDistance" keyword requires a document field and a "distance" attribute');
throw new Error('"geoDistance" keyword must (only) contain a document field and a "distance" attribute');
}

mustBeString(filter.geoDistance, 'geoDistance', 'distance');
Expand All @@ -418,7 +418,7 @@ class Standardizer {
const point = convertGeopoint(filter.geoDistance[docField]);

if (point === null) {
throw new BadRequestError(`geoDistance.${docField}: unrecognized point format`);
throw new Error(`geoDistance.${docField}: unrecognized point format`);
}

standardized.geospatial.geoDistance[docField] = {lat: point.lat, lon: point.lon};
Expand All @@ -436,7 +436,7 @@ class Standardizer {
const fields = mustBeNonEmptyObject(filter.geoDistanceRange, 'geoDistanceRange');

if (fields.length !== 3 || !fields.includes('from') || !fields.includes('to')) {
throw new BadRequestError('"geoDistanceRange" keyword requires a document field and the following attributes: "from", "to"');
throw new Error('"geoDistanceRange" keyword must (only) contain a document field and the following attributes: "from", "to"');
}

const docField = Object.keys(filter.geoDistanceRange).find(f => f !== 'from' && f !== 'to');
Expand All @@ -447,14 +447,14 @@ class Standardizer {
const point = convertGeopoint(filter.geoDistanceRange[docField]);

if (point === null) {
throw new BadRequestError(`geoDistanceRange.${docField}: unrecognized point format`);
throw new Error(`geoDistanceRange.${docField}: unrecognized point format`);
}

const from = convertDistance(filter.geoDistanceRange.from);
const to = convertDistance(filter.geoDistanceRange.to);

if (from >= to) {
throw new BadRequestError(`geoDistanceRange.${docField}: inner radius must be smaller than outer radius`);
throw new Error(`geoDistanceRange.${docField}: inner radius must be smaller than outer radius`);
}

return {
Expand Down Expand Up @@ -487,14 +487,14 @@ class Standardizer {
const points = [];

if (filter.geoPolygon[docField].points.length < 3) {
throw new BadRequestError(`"geoPolygon.${docField}": at least 3 points are required to build a polygon`);
throw new Error(`"geoPolygon.${docField}": at least 3 points are required to build a polygon`);
}

for (let i = 0; i < filter.geoPolygon[docField].points.length; ++i) {
const point = convertGeopoint(filter.geoPolygon[docField].points[i]);

if (point === null) {
throw new BadRequestError(`geoPolygon.${docField}: unrecognized point format (${JSON.stringify(filter.geoPolygon[docField].points[i])})`);
throw new Error(`geoPolygon.${docField}: unrecognized point format (${JSON.stringify(filter.geoPolygon[docField].points[i])})`);
}

points.push([point.lat, point.lon]);
Expand All @@ -515,8 +515,8 @@ class Standardizer {
* @param {string} [keyword] name - user keyword entry (and, must, should_not).. used to display errors if any
* @returns {Object} standardized filter
*/
and (filter, keyword = 'and') {
mustBeNonEmptyArray(filter, 'and', keyword);
and (filter) {
mustBeNonEmptyArray(filter, 'and');
return this._standardizeFilterArray(filter, 'and');
}

Expand All @@ -526,8 +526,8 @@ class Standardizer {
* @param {string} [keyword] name - user keyword entry (or, should, must_not).. used to display errors if any
* @returns {Object} standardized filter
*/
or (filter, keyword = 'or') {
mustBeNonEmptyArray(filter, 'or', keyword);
or (filter) {
mustBeNonEmptyArray(filter, 'or');
return this._standardizeFilterArray(filter, 'or');
}

Expand Down Expand Up @@ -586,7 +586,7 @@ class Standardizer {
const fields = mustBeNonEmptyObject(filter.bool, 'bool');

if (fields.findIndex(field => BOOL_ATTRIBUTES.indexOf(field) === -1) > -1) {
throw new BadRequestError(`"bool" operand accepts only the following attributes: ${BOOL_ATTRIBUTES.join(', ')}`);
throw new Error(`"bool" operand accepts only the following attributes: ${BOOL_ATTRIBUTES.join(', ')}`);
}

const f = {and: []};
Expand Down Expand Up @@ -627,7 +627,7 @@ class Standardizer {
});

if (idx > -1) {
throw new BadRequestError(`"${operand}" operand can only contain non-empty objects`);
throw new Error(`"${operand}" operand can only contain non-empty objects`);
}

const result = {
Expand Down Expand Up @@ -694,7 +694,7 @@ module.exports = Standardizer;
*/
function onlyOneFieldAttribute(fieldsList, keyword) {
if (fieldsList.length > 1) {
throw new BadRequestError(`"${keyword}" can contain only one attribute`);
throw new Error(`"${keyword}" can contain only one attribute`);
}
}

Expand All @@ -705,8 +705,8 @@ function onlyOneFieldAttribute(fieldsList, keyword) {
* @param attribute
*/
function requireAttribute(filter, keyword, attribute) {
if (!filter[attribute]) {
throw new BadRequestError(`"${keyword}" requires the following attribute: ${attribute}`);
if (filter[attribute] === undefined) {
throw new Error(`"${keyword}" requires the following attribute: ${attribute}`);
}
}

Expand All @@ -718,13 +718,13 @@ function requireAttribute(filter, keyword, attribute) {
*/
function mustBeNonEmptyObject(filter, keyword) {
if (!filter || typeof filter !== 'object' || Array.isArray(filter)) {
throw new BadRequestError(`"${keyword}" must be a non-empty object`);
throw new Error(`"${keyword}" must be a non-empty object`);
}

const fields = Object.keys(filter);

if (fields.length === 0) {
throw new BadRequestError(`"${keyword}" must be a non-empty object`);
throw new Error(`"${keyword}" must be a non-empty object`);
}

return fields;
Expand All @@ -739,7 +739,7 @@ function mustBeNonEmptyObject(filter, keyword) {
*/
function mustBeScalar (filter, keyword, field) {
if (filter[field] instanceof Object || filter[field] === undefined) {
throw new BadRequestError(`"${field}" in "${keyword}" must be either a string, a number, a boolean or null`);
throw new Error(`"${field}" in "${keyword}" must be either a string, a number, a boolean or null`);
}
}

Expand All @@ -751,7 +751,7 @@ function mustBeScalar (filter, keyword, field) {
*/
function mustBeString (filter, keyword, field) {
if (typeof filter[field] !== 'string') {
throw new BadRequestError(`Attribute "${field}" in "${keyword}" must be a string`);
throw new Error(`Attribute "${field}" in "${keyword}" must be a string`);
}
}

Expand All @@ -762,12 +762,17 @@ function mustBeString (filter, keyword, field) {
* @param field
*/
function mustBeNonEmptyArray (filter, keyword, field) {
if (!Array.isArray(filter[field])) {
throw new BadRequestError(`Attribute "${field}" in "${keyword}" must be an array`);
const prefix = field
? `Attribute "${field}" in "${keyword}"`
: `Attribute "${keyword}"`;
const value = field ? filter[field] : filter[keyword];

if (!Array.isArray(value)) {
throw new Error(`${prefix} must be an array`);
}

if (filter[field].length === 0) {
throw new BadRequestError(`Attribute "${field}" in "${keyword}" cannot be empty`);
if (value.length === 0) {
throw new Error(`${prefix} cannot be empty`);
}
}

Expand Down Expand Up @@ -807,7 +812,7 @@ function parseFieldSyntax(field, keyword) {
value = JSON.parse(rawValue);
}
catch (error) {
throw new BadRequestError(`[${keyword}] Invalid array value "${rawValue}"`);
throw new Error(`[${keyword}] Invalid array value "${rawValue}"`);
}
}

Expand Down
3 changes: 1 addition & 2 deletions lib/util/convertDistance.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
*/

const units = require('node-units');
const { BadRequestError } = require('kuzzle-common-objects');

/**
* Converts a distance string value to a number of meters
Expand All @@ -41,7 +40,7 @@ function convertDistance (distance) {
converted = units.convert(cleaned + ' to m');
}
catch (e) {
throw new BadRequestError(`unable to parse distance value "${distance}"`);
throw new Error(`unable to parse distance value "${distance}"`);
}

return converted;
Expand Down
Loading

0 comments on commit 77e3925

Please sign in to comment.