diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d53249..3db4a78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## v1.3.4 + +**Bugfix:** + +- Fix [Boolean value based plugins do not work with validation (in case of typed selector values) and will fail with unexpected error](https://github.com/massfords/ts-rsql-query/issues/10). +- Plugin configuration section of [README.md](./README.md#plugin-configuration) updated. + +> IMPORTANT NOTE: the fix affects boolean-value-based plugins (like the provided `IsNullPlugin`) which might be created by +> developers so far in a "non-backwards-compatible" way. +> +> **Migration path**: add `allowedValues: : [ 'value1', value2, ...]` to your `RsqlOperatorPlugin` +> configuration to enable a proper functionality of your custom plugin. + +**Internals:** + +- Tests added for fix. +- Added new `Makefile` target: `tl` to execute live-DB tests. +- Fix `lastModified` selector-column mapping in `TestQueryConfig` (column was configured case-sensitive in DB but not in selector). + ## v1.3.3 **Bugfixes:** diff --git a/Makefile b/Makefile index 52991b4..4e665ea 100644 --- a/Makefile +++ b/Makefile @@ -29,3 +29,10 @@ tc: @printf "* Running tests with coverage.\n" @printf "*******************************************************************************\n" npm run test:coverage + +# Execute tests with live-DB. +tl: + @printf "*******************************************************************************\n" + @printf "* Running tests with live-DB.\n" + @printf "*******************************************************************************\n" + npm run it diff --git a/README.md b/README.md index 5c9bc99..76350d7 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,13 @@ export type RsqlOperatorPlugin = { * @returns The SQL code. */ toSql(options: RsqlOperatorPluginToSqlOptions): string; + /** + * Pass your plugin's allowed values for validation. + * Use this if you plugin accepts other values than configured in the selector's `type`. + * + * @default No action. + */ + readonly allowedValues?: string[]; }; ``` @@ -245,7 +252,7 @@ const context: SqlContext = { The following codes shows an example of how to implement a plugin by the predefined plugin `IsNullPlugin`: ```typescript -import { CustomOperator, formatKeyword, isBooleanValueInvariant, RsqlOperatorPlugin, RsqlOperatorPluginToSqlOptions } from "ts-rsql-query"; +import { BOOLEAN_PLUGIN_VALUES, CustomOperator, formatKeyword, isBooleanValueInvariant, RsqlOperatorPlugin, RsqlOperatorPluginToSqlOptions } from "ts-rsql-query"; /** * Plugin for an is-null operation. @@ -265,6 +272,7 @@ export const IsNullPlugin: RsqlOperatorPlugin = { } = options; return `${selector} ${formatKeyword("IS", options)}${operands?.[0] === "false" ? ` ${formatKeyword("NOT", keywordsLowerCase)}` : ""} null`; }, + allowedValues: BOOLEAN_PLUGIN_VALUES, }; ``` @@ -303,8 +311,14 @@ export const MapInToEqualsAnyPlugin: RsqlOperatorPlugin = { ### Out-of-the-box plugins > **IMPORTANT NOTE:** The plugins [`IsEmptyPlugin`](#isemptyplugin) and [`IsNullOrEmptyPlugin`](#isnulloremptyplugin) -> are intended to be used on fields which are `TEXT`-like, if you use them on other types (e.g. `TIMESTAMP`) -> you might experience errors on SQL or RSQL validation level. So, be careful when using it. +> are intended to be used on fields which are `TEXT`-like, if you use them on other types (e.g. `TIMESTAMP` or `INTEGER`) +> you might experience errors on SQL validation level, e.g.: +> +> ```text +> error: invalid input syntax for type integer: "" +> ``` +> +> These messages could be different depending on the underlying SQL driver/framework implementation So, be careful when using it. #### `MapInToEqualsAnyPlugin` diff --git a/package-lock.json b/package-lock.json index 8303b37..3b45789 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ts-rsql-query", - "version": "1.3.3", + "version": "1.3.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ts-rsql-query", - "version": "1.3.3", + "version": "1.3.4", "license": "MIT", "dependencies": { "date-fns": "^4.1.0", diff --git a/package.json b/package.json index 151db62..523022b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ts-rsql-query", - "version": "1.3.3", + "version": "1.3.4", "description": "Transforms the AST from ts-rsql into a SQL query", "main": "./dist/index.js", "scripts": { diff --git a/src/context.ts b/src/context.ts index 65267aa..7e600d4 100644 --- a/src/context.ts +++ b/src/context.ts @@ -111,6 +111,13 @@ export type RsqlOperatorPlugin = { * @returns The SQL code. */ toSql(options: RsqlOperatorPluginToSqlOptions): string; + /** + * Pass your plugin's allowed values for validation. + * Use this if you plugin accepts other values than configured in the selector's `type`. + * + * @default No validation action. + */ + readonly allowedValues?: string[]; }; /** diff --git a/src/index.ts b/src/index.ts index 09b5c93..c50de5a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,12 +1,13 @@ -export { assembleFullQuery } from "./query"; export * from "./context"; export { lastRowToKeySet, toKeySet } from "./keyset"; export { formatKeyword, formatValue } from "./llb/to-sql"; export { + BOOLEAN_PLUGIN_VALUES, isBooleanValueInvariant, - MapInToEqualsAnyPlugin, - MapOutToNotEqualsAllPlugin, - IsNullPlugin, IsEmptyPlugin, IsNullOrEmptyPlugin, + IsNullPlugin, + MapInToEqualsAnyPlugin, + MapOutToNotEqualsAllPlugin, } from "./plugin"; +export { assembleFullQuery } from "./query"; diff --git a/src/llb/operators.ts b/src/llb/operators.ts index 5e09de6..6abf1bf 100644 --- a/src/llb/operators.ts +++ b/src/llb/operators.ts @@ -1,4 +1,3 @@ -import type { RsqlOperatorPlugin } from "../context"; import { formatKeyword } from "./to-sql"; /** @@ -88,19 +87,3 @@ export const isKnownOperator = (maybe: string): maybe is KnownOperator => { return false; } }; - -/** - * Checks if passed operator is a configured RSQL plugin operator. - * - * @param maybe - The RSQL plugin operator. - * @param plugins - The RSQL operator plugins. - * @returns A `true` if is a configured RSQL plugin operator, else `false`. - */ -export const isPluginOperator = ( - maybe: string, - plugins?: RsqlOperatorPlugin[], -): boolean => { - return plugins?.length - ? plugins.some((plugin) => plugin.operator.toLowerCase() === maybe) - : false; -}; diff --git a/src/llb/validate.ts b/src/llb/validate.ts index f4ce948..243952e 100644 --- a/src/llb/validate.ts +++ b/src/llb/validate.ts @@ -2,8 +2,9 @@ import { parseISO } from "date-fns"; import invariant from "tiny-invariant"; import type { ASTNode } from "ts-rsql"; import type { SelectorConfig, SqlContext } from "../context"; +import { findPluginByOperator } from "../plugin"; import { isAstNode, isComparisonNode } from "./ast"; -import { isKnownOperator, isPluginOperator } from "./operators"; +import { isKnownOperator } from "./operators"; export const validate = ( ast: ASTNode, @@ -47,12 +48,8 @@ export const validate = ( } return { isValid: true }; } else if (isComparisonNode(ast)) { - if ( - !( - isKnownOperator(ast.operator) || - isPluginOperator(ast.operator, context.plugins) - ) - ) { + const plugin = findPluginByOperator(ast.operator, context.plugins); + if (!(isKnownOperator(ast.operator) || plugin)) { return { isValid: false, err: `unknown operator: ${JSON.stringify(ast.operator)}`, @@ -75,7 +72,17 @@ export const validate = ( }; } if (typeof selector === "object") { - if (!isValueValid(selector, value)) { + if (!isValueValid(selector, value, plugin?.allowedValues)) { + if (plugin?.allowedValues?.length) { + return { + isValid: false, + err: `bad value for "${ast.selector}": "${value}" (with "${ + ast.operator + }" operator), must be one of ${JSON.stringify( + plugin.allowedValues, + )}`, + }; + } if (selector.enum) { return { isValid: false, @@ -98,7 +105,14 @@ export const validate = ( return { isValid: false, err: `unknown node type: ${JSON.stringify(ast)}` }; }; -const isValueValid = (selector: SelectorConfig, val: string): boolean => { +const isValueValid = ( + selector: SelectorConfig, + val: string, + allowedValues?: string[], +): boolean => { + if (allowedValues?.length) { + return allowedValues.some((allowedValue) => val == allowedValue); + } if (selector.enum) { return selector.enum.some((allowedValue) => val == allowedValue); } diff --git a/src/plugin.ts b/src/plugin.ts index cb77817..2d237c3 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -8,6 +8,11 @@ import type { import { isKnownOperator } from "./llb/operators"; import { formatKeyword, formatValue } from "./llb/to-sql"; +/** + * The allowed values for boolean-based plugins. + */ +export const BOOLEAN_PLUGIN_VALUES = ["true", "false"]; + /** * Custom RSQL operators supported out-of-the-box by this library. */ @@ -25,6 +30,22 @@ export const OverwrittenOperator = { OUT: "=out=", } as const; +/** + * Find a plugin for given operator. + * + * @param operator - The currently evaluated RSQL operator. + * @param plugins - The RSQL operator plugins. + * @returns A `RsqlOperatorPlugin` if `operator` is a configured RSQL plugin operator, else `undefined`. + */ +export const findPluginByOperator = ( + operator: string, + plugins?: RsqlOperatorPlugin[], +): RsqlOperatorPlugin | undefined => { + return plugins?.length + ? plugins.find((plugin) => plugin.operator.toLowerCase() === operator) + : undefined; +}; + /** * Executes any plugin found for RSQL `currentOperator`. If none found, it returns `undefined`. * @@ -40,9 +61,7 @@ export const maybeExecuteRsqlOperatorPlugin = ( ): string | undefined => { const { keywordsLowerCase, plugins, values } = context; /* Check for plugin (custom operator or overwrite of known operator). */ - const plugin = plugins?.length - ? plugins.find((plugin) => plugin.operator.toLowerCase() === ast.operator) - : undefined; + const plugin = findPluginByOperator(ast.operator, plugins); if (plugin) { invariant( /* Case: overwrite any known operator. */ @@ -77,10 +96,6 @@ export const isBooleanValueInvariant = (ast: ComparisonNode): void => { const message = "operator value must be 'true' or 'false'"; invariant(ast.operands, `operator must have one value, ${message}`); invariant(ast.operands[0], `operator must have one value, ${message}`); - invariant( - ast.operands[0] === "true" || ast.operands[0] === "false", - `${message}, but was: '${ast.operands[0]}'`, - ); }; /** @@ -155,6 +170,7 @@ export const IsNullPlugin: RsqlOperatorPlugin = { : "" } null`; }, + allowedValues: BOOLEAN_PLUGIN_VALUES, }; /** @@ -180,6 +196,7 @@ export const IsEmptyPlugin: RsqlOperatorPlugin = { (operands as string[])[0] === "true" ? "=" : "<>" } ''`; }, + allowedValues: BOOLEAN_PLUGIN_VALUES, }; /** @@ -214,4 +231,5 @@ export const IsNullOrEmptyPlugin: RsqlOperatorPlugin = { keywordsLowerCase, )} ${IsEmptyPlugin.toSql(maybeReversedOptions)})`; }, + allowedValues: BOOLEAN_PLUGIN_VALUES, }; diff --git a/src/tests/fixture.ts b/src/tests/fixture.ts index 36e1f2e..9a1a2c9 100644 --- a/src/tests/fixture.ts +++ b/src/tests/fixture.ts @@ -37,7 +37,7 @@ export const TestQueryConfig: StaticQueryConfig = { type: "boolean", }, lastModified: { - sql: "u.lastModified", + sql: `u."lastModified"`, type: "date-time", }, birthday: { diff --git a/src/tests/live-db.it.ts b/src/tests/live-db.it.ts index 1b07e88..20bb718 100644 --- a/src/tests/live-db.it.ts +++ b/src/tests/live-db.it.ts @@ -191,6 +191,54 @@ describe("runs the sql with a real db connection", () => { filter: "address=nullorempty=false;interest=nullorempty=true", rows: 0, }, + { + filter: "birthday=null=true", + rows: 0, + }, + { + filter: "birthday=null=false", + rows: 3, + }, + { + filter: "active=null=true", + rows: 0, + }, + { + filter: "active=null=false", + rows: 3, + }, + { + filter: "tier=null=true", + rows: 0, + }, + { + filter: "tier=null=false", + rows: 3, + }, + { + filter: "tier=nullorempty=true", + rows: 0, + }, + { + filter: "tier=nullorempty=false", + rows: 3, + }, + { + filter: "points=null=true", + rows: 0, + }, + { + filter: "points=null=false", + rows: 3, + }, + { + filter: "lastModified=null=true", + rows: 0, + }, + { + filter: "lastModified=null=false", + rows: 3, + }, ]; it.each(inputs)("$filter", async ({ filter, rows }) => { expect.hasAssertions(); @@ -207,7 +255,7 @@ describe("runs the sql with a real db connection", () => { }, context, ); - invariant(sql.isValid); + invariant(sql.isValid, !sql.isValid ? sql.err : undefined); expect(await db.manyOrNone(sql.sql, context.values)).toHaveLength(rows); }); }); diff --git a/src/tests/operators.test.ts b/src/tests/operators.test.ts index e799109..d6c8dc7 100644 --- a/src/tests/operators.test.ts +++ b/src/tests/operators.test.ts @@ -1,8 +1,4 @@ -import { - isPluginOperator, - KnownOperator, - toSqlOperator, -} from "../llb/operators"; +import { KnownOperator, toSqlOperator } from "../llb/operators"; describe("operators tests", () => { describe("toSqlOperator", () => { @@ -127,27 +123,4 @@ describe("operators tests", () => { }, ); }); - - describe("isPluginOperator", () => { - const operator = "=custom="; - - it("should return false if plugins configuration is empty array", () => { - expect(isPluginOperator(operator, [])).toBe(false); - }); - - it("should return false if plugins configuration is undefined", () => { - expect(isPluginOperator(operator)).toBe(false); - }); - - it("should return false if plugins configuration is empty array", () => { - expect( - isPluginOperator(operator, [ - { - operator, - toSql: jest.fn(), - }, - ]), - ).toBe(true); - }); - }); }); diff --git a/src/tests/plugin.test.ts b/src/tests/plugin.test.ts index 273cde7..b075d93 100644 --- a/src/tests/plugin.test.ts +++ b/src/tests/plugin.test.ts @@ -6,6 +6,7 @@ import type { } from "../context"; import { CustomOperator, + findPluginByOperator, isBooleanValueInvariant, IsEmptyPlugin, IsNullOrEmptyPlugin, @@ -39,6 +40,28 @@ describe("tests for sql generation by plugins", () => { }; }; + describe("findPluginByOperator", () => { + const operator = "=custom="; + + it("should return undefined if plugins configuration is empty array", () => { + expect(findPluginByOperator(operator, [])).toBeUndefined(); + }); + + it("should return undefined if plugins configuration is undefined", () => { + expect(findPluginByOperator(operator)).toBeUndefined(); + }); + + it("should return plugin if plugins configuration contains plugin", () => { + const plugin = { + operator, + toSql: jest.fn(), + }; + const result = findPluginByOperator(operator, [plugin]); + expect(result).toBeDefined(); + expect(result).toBe(plugin); + }); + }); + describe("maybeExecuteRsqlOperatorPlugin", () => { const mockInvariant = jest.fn(); const sql = "SQL"; @@ -166,16 +189,6 @@ describe("tests for sql generation by plugins", () => { "operator must have one value, operator value must be 'true' or 'false'", ); }); - - it("should not pass if operands is not 'true' or 'false'", () => { - expect(() => - isBooleanValueInvariant({ - operands: ["invalid"], - selector, - operator: "", - }), - ).toThrow("operator value must be 'true' or 'false', but was: 'invalid'"); - }); }); describe("MapInToEqualsAnyPlugin", () => { diff --git a/src/tests/to-sql.test.ts b/src/tests/to-sql.test.ts index 09602d0..9c71c34 100644 --- a/src/tests/to-sql.test.ts +++ b/src/tests/to-sql.test.ts @@ -1,5 +1,7 @@ +const mockFindPluginByOperator = jest.fn(); const mockMaybeExecuteRsqlOperatorPlugin = jest.fn(); jest.mock("../plugin", () => ({ + findPluginByOperator: mockFindPluginByOperator, maybeExecuteRsqlOperatorPlugin: mockMaybeExecuteRsqlOperatorPlugin, })); @@ -231,13 +233,13 @@ describe("tests for sql generation", () => { describe("plugin", () => { beforeEach(() => { - mockMaybeExecuteRsqlOperatorPlugin.mockClear(); + jest.clearAllMocks(); }); it("should be executed if configured", () => { expect.hasAssertions(); - const operator = "=isNull="; + const operator = "=null="; const ast = parseRsql(`name${operator}true`); /* Just simulate that the function is called and returns a value. */ @@ -256,11 +258,18 @@ describe("tests for sql generation", () => { selectors: {}, lax: true, }; + mockFindPluginByOperator.mockReturnValueOnce(context.plugins?.[0]); const actual = toSql(ast, context); expect(actual).toStrictEqual({ isValid: true, sql: expectedSql }); + expect(mockFindPluginByOperator).toHaveBeenCalledTimes(1); + expect(mockFindPluginByOperator).toHaveBeenCalledWith( + operator, + context.plugins, + ); + expect(mockMaybeExecuteRsqlOperatorPlugin).toHaveBeenCalledTimes(1); expect(mockMaybeExecuteRsqlOperatorPlugin).toHaveBeenCalledWith( context, @@ -290,6 +299,12 @@ describe("tests for sql generation", () => { expect(actual).toStrictEqual({ isValid: true, sql: "name=$1" }); + expect(mockFindPluginByOperator).toHaveBeenCalledTimes(1); + expect(mockFindPluginByOperator).toHaveBeenCalledWith( + operator, + context.plugins, + ); + expect(mockMaybeExecuteRsqlOperatorPlugin).toHaveBeenCalledTimes(1); expect(mockMaybeExecuteRsqlOperatorPlugin).toHaveBeenCalledWith( context, diff --git a/src/tests/validate.test.ts b/src/tests/validate.test.ts index eff6382..4879fae 100644 --- a/src/tests/validate.test.ts +++ b/src/tests/validate.test.ts @@ -1,6 +1,6 @@ import { parseRsql } from "ts-rsql"; import { toSql } from "../llb/to-sql"; -import { TestQueryConfig } from "./fixture"; +import { TestQueryConfig, TestQueryConfigWithPlugins } from "./fixture"; describe("validate AST tests", () => { const inputs: Array<{ rsql: string; err: string }> = [ @@ -69,4 +69,30 @@ describe("validate AST tests", () => { }); expect(actual).toStrictEqual({ isValid: false, err }); }); + + describe("(custom) plugin query tests", () => { + const inputs: Array<{ rsql: string; err: string }> = [ + { + rsql: "points=null=invalid", + err: `bad value for "points": "invalid" (with "=null=" operator), must be one of ["true","false"]`, + }, + { + rsql: "tier=nullorempty=invalid", + err: `bad value for "tier": "invalid" (with "=nullorempty=" operator), must be one of ["true","false"]`, + }, + { + rsql: "tier=empty=invalid", + err: `bad value for "tier": "invalid" (with "=empty=" operator), must be one of ["true","false"]`, + }, + ]; + it.each(inputs)("$rsql", ({ rsql, err }) => { + expect.hasAssertions(); + const ast = parseRsql(rsql); + const actual = toSql(ast, { + ...TestQueryConfigWithPlugins, + values: [], + }); + expect(actual).toStrictEqual({ isValid: false, err }); + }); + }); });