From edfdae1dc64496b507bfa8fc3c0935319fee926c Mon Sep 17 00:00:00 2001 From: Jens Krefeldt Date: Thu, 28 Nov 2024 15:02:38 +0100 Subject: [PATCH] Bugfixes and README corrections (#7) Bugfixes and README corrections chore: bump cross-spawn version --------- Co-authored-by: Jens Krefeldt Co-authored-by: markford --- .gitignore | 1 + .npmignore | 1 + CHANGELOG.md | 15 ++++++++ README.md | 11 +++--- package-lock.json | 16 ++++----- src/context.ts | 2 +- src/llb/operators.ts | 4 +-- src/llb/to-sql.ts | 70 ++++++++++++++++++++++++------------- src/plugin.ts | 3 +- src/tests/live-db.it.ts | 48 +++++++++++++++++++------ src/tests/operators.test.ts | 4 +-- src/tests/plugin.test.ts | 22 ++++++++++++ src/tests/to-sql.test.ts | 32 ++++++++++++++++- 13 files changed, 175 insertions(+), 54 deletions(-) diff --git a/.gitignore b/.gitignore index 78d451a..ebee508 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ tmp dist *.tgz coverage/ +.vscode/ diff --git a/.npmignore b/.npmignore index ca24741..36ffbfc 100644 --- a/.npmignore +++ b/.npmignore @@ -9,3 +9,4 @@ __tests__ *.test.js coverage Makefile +.vscode diff --git a/CHANGELOG.md b/CHANGELOG.md index beda7b0..9fb8920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## v1.3.1 + +**Bugfixes:** + +- Fix [No wildcard(s) support for `!=` operator](https://github.com/massfords/ts-rsql-query/issues/6). +- Fixed a bug where `RsqlOperatorPluginToSqlOptions.keywordsLowerCase` was not passed to the options when it would be configured to `true`. +- Corrected implementation examples for plugin section in (./README.md#plugins). + +**Internals:** + +- Added some bugfix related (non-)equality tests to live-DB. +- Git- and npm-ignored `.vscode/` folder. +- Fixed internal function `toSqlOperator` to return SQL `=` for RSQL `==` operator (actually, case was never used before, but now it is used and fixed therefore). +- Added some bugfix related (non-)equality tests to live-DB. + ## v1.3.0 **New feature implementations:** diff --git a/README.md b/README.md index 2dd4ce9..5c9bc99 100644 --- a/README.md +++ b/README.md @@ -168,7 +168,7 @@ const sql = assembleFullQuery( sort: parsedSorts, keyset, }, - context + context, ); if (sql.isValid) { const rows = await db.manyOrNone(sql.sql, context.values); @@ -242,7 +242,7 @@ const context: SqlContext = { ### Plugin implementation -The following codes shows an example of how to implement a plugin by the predefined plugin `MapInToEqualsAnyPlugin`: +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"; @@ -259,10 +259,11 @@ export const IsNullPlugin: RsqlOperatorPlugin = { invariant: isBooleanValueInvariant, toSql: (options: RsqlOperatorPluginToSqlOptions): string => { const { + keywordsLowerCase, selector, ast: { operands }, } = options; - return `${selector} ${formatKeyword("IS", options)}${operands?.[0] === "false" ? ` ${formatKeyword("NOT", options)}` : ""} null`; + return `${selector} ${formatKeyword("IS", options)}${operands?.[0] === "false" ? ` ${formatKeyword("NOT", keywordsLowerCase)}` : ""} null`; }, }; ``` @@ -292,9 +293,9 @@ export const MapInToEqualsAnyPlugin: RsqlOperatorPlugin = { invariant(ast.operands); }, toSql: (options: RsqlOperatorPluginToSqlOptions): string => { - const { ast, selector, values, config } = options; + const { ast, keywordsLowerCase, selector, values, config } = options; values.push(formatValue({ ast, allowArray: true }, config)); - return `${selector} = ${formatKeyword("ANY", options)}($${values.length})`; + return `${selector} = ${formatKeyword("ANY", keywordsLowerCase)}($${values.length})`; }, }; ``` diff --git a/package-lock.json b/package-lock.json index e262bd4..5356730 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ts-rsql-query", - "version": "1.3.0", + "version": "1.3.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ts-rsql-query", - "version": "1.3.0", + "version": "1.3.1", "license": "MIT", "dependencies": { "date-fns": "^4.1.0", @@ -2493,9 +2493,9 @@ "license": "MIT" }, "node_modules/cross-spawn": { - "version": "7.0.3", - "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", - "integrity": "sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==", + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", "dev": true, "dependencies": { "path-key": "^3.1.0", @@ -8174,9 +8174,9 @@ "dev": true }, "cross-spawn": { - "version": "7.0.3", - "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", - "integrity": "sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==", + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", "dev": true, "requires": { "path-key": "^3.1.0", diff --git a/src/context.ts b/src/context.ts index 31c3896..65267aa 100644 --- a/src/context.ts +++ b/src/context.ts @@ -78,7 +78,7 @@ export type RsqlOperatorPluginToSqlOptions = { * * @default false */ - readonly keywordsLowerCase?: boolean; + readonly keywordsLowerCase?: true | undefined; /** * The static query configuration. */ diff --git a/src/llb/operators.ts b/src/llb/operators.ts index 8b2d3d8..5e09de6 100644 --- a/src/llb/operators.ts +++ b/src/llb/operators.ts @@ -21,6 +21,7 @@ export type KnownOperator = SymbolicOperator | NamedOperator; * * > NOTE: * > - Option `keywordsLowerCase` has no effect for: + * > - `==` * > - `!=` * > - `<` * > - `=lt=` @@ -31,7 +32,6 @@ export type KnownOperator = SymbolicOperator | NamedOperator; * > - `>=` * > - `=ge=` * > - Option `detachedOperators` has no effect for: - * > - `==` * > - `=in=` * > - `=out=` * @@ -48,7 +48,7 @@ export const toSqlOperator = ( const space = detachedOperators ? " " : ""; switch (operator) { case "==": - return formatKeyword("LIKE", keywordsLowerCase); + return `${space}=${space}`; case "!=": return `${space}<>${space}`; case "<": diff --git a/src/llb/to-sql.ts b/src/llb/to-sql.ts index e211c12..f79c10b 100644 --- a/src/llb/to-sql.ts +++ b/src/llb/to-sql.ts @@ -1,15 +1,15 @@ -import { ASTNode, ComparisonNode, Operand, parseRsql } from "ts-rsql"; import invariant from "tiny-invariant"; -import { isAstNode, isComparisonNode } from "./ast"; +import { ASTNode, ComparisonNode, Operand, parseRsql } from "ts-rsql"; import type { SelectorConfig, SqlContext, StaticQueryConfig, Value, } from "../context"; +import { maybeExecuteRsqlOperatorPlugin } from "../plugin"; +import { isAstNode, isComparisonNode } from "./ast"; import { KnownOperator, toSqlOperator } from "./operators"; import { validate } from "./validate"; -import { maybeExecuteRsqlOperatorPlugin } from "../plugin"; /** * Formats a keyword according to configuration (either fully upper- or lower-case). @@ -126,6 +126,46 @@ export const toSql = ( return { isValid: true, sql: _toSql(ast, context) }; }; +/** + * Creates the SQL string from the `==` or `!=` RSQL operator including any wildcard(s) occurrence in its operand. + * + * @param ast - The comparison node. + * @param context - The SQL context. + * @param operator - The `==` or `!=` RSQL operator. + * @param selector - The current selector. + * @returns The proper SQL string for `==` or `!=` RSQL operator. + */ +const _handleEqualOrNotEqualOperator = ( + ast: ComparisonNode, + context: SqlContext, + operator: "==" | "!=", + selector: string, +): string => { + const { detachedOperators, keywordsLowerCase, values } = context; + invariant(ast.operands); + invariant(ast.operands[0]); + const operand = ast.operands[0]; + const leadingWildcard = operand.startsWith("*"); + const trailingWildcard = operand.endsWith("*"); + if (leadingWildcard && trailingWildcard) { + values.push(`%${operand.substring(1, operand.length - 1)}%`); + } else if (leadingWildcard) { + values.push(`%${operand.substring(1)}`); + } else if (trailingWildcard) { + values.push(`${operand.substring(0, operand.length - 1)}%`); + } else { + values.push(formatValue({ ast }, context)); + } + return `${selector}${ + leadingWildcard || trailingWildcard + ? ` ${formatKeyword( + `${operator === "!=" ? "NOT " : ""}ILIKE`, + keywordsLowerCase, + )} ` + : `${toSqlOperator(operator, keywordsLowerCase, detachedOperators)}` + }$${values.length}`; +}; + const _toSql = (ast: ASTNode | null | Operand, context: SqlContext): string => { if (!isAstNode(ast)) { // We would have reported this as an error in validate. @@ -166,28 +206,10 @@ const _toSql = (ast: ASTNode | null | Operand, context: SqlContext): string => { } switch (op) { - case "==": { - invariant(ast.operands); - invariant(ast.operands[0]); - const operand = ast.operands[0]; - const leadingWildcard = operand.startsWith("*"); - const trailingWildcard = operand.endsWith("*"); - if (leadingWildcard && trailingWildcard) { - values.push(`%${operand.substring(1, operand.length - 1)}%`); - } else if (leadingWildcard) { - values.push(`%${operand.substring(1)}`); - } else if (trailingWildcard) { - values.push(`${operand.substring(0, operand.length - 1)}%`); - } else { - values.push(formatValue({ ast }, config)); - } - return `${selector}${ - leadingWildcard || trailingWildcard - ? ` ${formatKeyword("ILIKE", keywordsLowerCase)} ` - : `${detachedOperators ? " " : ""}=${detachedOperators ? " " : ""}` - }$${values.length}`; + case "==": + case "!=": { + return _handleEqualOrNotEqualOperator(ast, context, op, selector); } - case "!=": case "<": case "<=": case ">": diff --git a/src/plugin.ts b/src/plugin.ts index 423b1d3..cb77817 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -38,7 +38,7 @@ export const maybeExecuteRsqlOperatorPlugin = ( ast: ComparisonNode, formattedSelector: string, ): string | undefined => { - const { plugins, values } = context; + 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) @@ -60,6 +60,7 @@ export const maybeExecuteRsqlOperatorPlugin = ( selector: formattedSelector, ast, values, + keywordsLowerCase, config: context, }); } diff --git a/src/tests/live-db.it.ts b/src/tests/live-db.it.ts index 85f2a6b..1b07e88 100644 --- a/src/tests/live-db.it.ts +++ b/src/tests/live-db.it.ts @@ -1,3 +1,13 @@ +import { + PostgreSqlContainer, + StartedPostgreSqlContainer, +} from "testcontainers"; +import invariant from "tiny-invariant"; +import { parseSort, SortNode } from "ts-rsql"; +import type { SqlContext } from "../context"; +import { lastRowToKeySet, toKeySet } from "../keyset"; +import { assembleFullQuery } from "../query"; +import { TestQueryConfig, TestQueryConfigWithPlugins } from "./fixture"; import { db, destroyDb, @@ -5,16 +15,6 @@ import { initDb, UserRecord, } from "./fixture-db"; -import invariant from "tiny-invariant"; -import { TestQueryConfig, TestQueryConfigWithPlugins } from "./fixture"; -import type { SqlContext } from "../context"; -import { assembleFullQuery } from "../query"; -import { lastRowToKeySet, toKeySet } from "../keyset"; -import { parseSort, SortNode } from "ts-rsql"; -import { - PostgreSqlContainer, - StartedPostgreSqlContainer, -} from "testcontainers"; describe("runs the sql with a real db connection", () => { let startedContainer: StartedPostgreSqlContainer | null = null; @@ -40,6 +40,34 @@ describe("runs the sql with a real db connection", () => { filter: "firstName==Alice", rows: 1, }, + { + filter: "firstName==*Alice", + rows: 1, + }, + { + filter: "firstName==Alice*", + rows: 1, + }, + { + filter: "firstName==*Alice*", + rows: 1, + }, + { + filter: "firstName!=Alice", + rows: 2, + }, + { + filter: "firstName!=*Alice", + rows: 2, + }, + { + filter: "firstName!=Alice*", + rows: 2, + }, + { + filter: "firstName!=*Alice*", + rows: 2, + }, { filter: "firstName==Alice,firstName==Bo*", rows: 2, diff --git a/src/tests/operators.test.ts b/src/tests/operators.test.ts index 99f0cd0..e799109 100644 --- a/src/tests/operators.test.ts +++ b/src/tests/operators.test.ts @@ -12,7 +12,7 @@ describe("operators tests", () => { }> = [ { rsql: "==", - sql: "LIKE", + sql: "=", }, { rsql: "!=", @@ -71,7 +71,7 @@ describe("operators tests", () => { }> = [ { rsql: "==", - sql: "like", + sql: " = ", }, { rsql: "!=", diff --git a/src/tests/plugin.test.ts b/src/tests/plugin.test.ts index 7ea6d26..273cde7 100644 --- a/src/tests/plugin.test.ts +++ b/src/tests/plugin.test.ts @@ -83,6 +83,28 @@ describe("tests for sql generation by plugins", () => { }); }); + it("should pass the keywordsLowerCase configuration from context", () => { + const newContext = { + ...context, + keywordsLowerCase: true, + } as unknown as SqlContext; + expect( + maybeExecuteRsqlOperatorPlugin(newContext, ast, formattedSelector), + ).toBe(sql); + + expect(mockInvariant).toHaveBeenCalledTimes(1); + expect(mockInvariant).toHaveBeenCalledWith(ast); + + expect(mockToSql).toHaveBeenCalledTimes(1); + expect(mockToSql).toHaveBeenCalledWith({ + selector: formattedSelector, + ast, + values, + keywordsLowerCase: true, + config: newContext, + }); + }); + it("should return undefined if plugins configuration is empty array", () => { expect( maybeExecuteRsqlOperatorPlugin( diff --git a/src/tests/to-sql.test.ts b/src/tests/to-sql.test.ts index 7f64a1c..09602d0 100644 --- a/src/tests/to-sql.test.ts +++ b/src/tests/to-sql.test.ts @@ -3,9 +3,9 @@ jest.mock("../plugin", () => ({ maybeExecuteRsqlOperatorPlugin: mockMaybeExecuteRsqlOperatorPlugin, })); -import { formatKeyword, toSql } from "../llb/to-sql"; import { parseRsql } from "ts-rsql"; import type { SqlContext, Value } from "../context"; +import { formatKeyword, toSql } from "../llb/to-sql"; import { TestQueryConfig } from "./fixture"; describe("tests for sql generation", () => { @@ -68,6 +68,21 @@ describe("tests for sql generation", () => { sql: `name ILIKE $1`, values: ["%Bill%"], }, + { + filter: `name!=Bill*`, + sql: `name NOT ILIKE $1`, + values: ["Bill%"], + }, + { + filter: `name!=*Bill`, + sql: `name NOT ILIKE $1`, + values: ["%Bill"], + }, + { + filter: `name!=*Bill*`, + sql: `name NOT ILIKE $1`, + values: ["%Bill%"], + }, ]; it.each(inputs)("filter", ({ filter, sql, values }) => { expect.hasAssertions(); @@ -132,6 +147,21 @@ describe("tests for sql generation", () => { sql: `name ILIKE $1`, values: ["%Bill%"], }, + { + filter: `name!=Bill*`, + sql: `name NOT ILIKE $1`, + values: ["Bill%"], + }, + { + filter: `name!=*Bill`, + sql: `name NOT ILIKE $1`, + values: ["%Bill"], + }, + { + filter: `name!=*Bill*`, + sql: `name NOT ILIKE $1`, + values: ["%Bill%"], + }, ]; it.each(inputsWithDetachedOperators)( "filter with detached operators",