Skip to content

Commit

Permalink
Boolean value based plugins do not work with validation fix by allowe…
Browse files Browse the repository at this point in the history
…d values (#12)

Fix bug by allowValues

---------

Co-authored-by: Jens Krefeldt <jens.krefeldt@smavoo.com>
  • Loading branch information
deadratfink and Jens Krefeldt authored Dec 8, 2024
1 parent 8ef00a8 commit 50815c5
Show file tree
Hide file tree
Showing 16 changed files with 224 additions and 86 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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:**
Expand Down
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 17 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
};
```

Expand All @@ -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.
Expand All @@ -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,
};
```

Expand Down Expand Up @@ -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`
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
7 changes: 7 additions & 0 deletions src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
};

/**
Expand Down
9 changes: 5 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -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";
17 changes: 0 additions & 17 deletions src/llb/operators.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { RsqlOperatorPlugin } from "../context";
import { formatKeyword } from "./to-sql";

/**
Expand Down Expand Up @@ -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;
};
32 changes: 23 additions & 9 deletions src/llb/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)}`,
Expand All @@ -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,
Expand All @@ -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);
}
Expand Down
32 changes: 25 additions & 7 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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`.
*
Expand All @@ -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. */
Expand Down Expand Up @@ -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]}'`,
);
};

/**
Expand Down Expand Up @@ -155,6 +170,7 @@ export const IsNullPlugin: RsqlOperatorPlugin = {
: ""
} null`;
},
allowedValues: BOOLEAN_PLUGIN_VALUES,
};

/**
Expand All @@ -180,6 +196,7 @@ export const IsEmptyPlugin: RsqlOperatorPlugin = {
(operands as string[])[0] === "true" ? "=" : "<>"
} ''`;
},
allowedValues: BOOLEAN_PLUGIN_VALUES,
};

/**
Expand Down Expand Up @@ -214,4 +231,5 @@ export const IsNullOrEmptyPlugin: RsqlOperatorPlugin = {
keywordsLowerCase,
)} ${IsEmptyPlugin.toSql(maybeReversedOptions)})`;
},
allowedValues: BOOLEAN_PLUGIN_VALUES,
};
2 changes: 1 addition & 1 deletion src/tests/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const TestQueryConfig: StaticQueryConfig = {
type: "boolean",
},
lastModified: {
sql: "u.lastModified",
sql: `u."lastModified"`,
type: "date-time",
},
birthday: {
Expand Down
50 changes: 49 additions & 1 deletion src/tests/live-db.it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
});
});
Expand Down
Loading

0 comments on commit 50815c5

Please sign in to comment.