diff --git a/.changeset/chilled-wolves-double.md b/.changeset/chilled-wolves-double.md new file mode 100644 index 0000000000..ca74f70139 --- /dev/null +++ b/.changeset/chilled-wolves-double.md @@ -0,0 +1,5 @@ +--- +"@directus/api": patch +--- + +Fixed reverse usage of null, nnull, empty and nempty filter operator diff --git a/api/src/utils/apply-query.test.ts b/api/src/utils/apply-query.test.ts index ec0ad42fc5..51301320d7 100644 --- a/api/src/utils/apply-query.test.ts +++ b/api/src/utils/apply-query.test.ts @@ -1,89 +1,91 @@ -import { expect, vi, test, describe } from 'vitest'; -import { applySearch } from './apply-query.js'; import type { SchemaOverview } from '@directus/types'; +import knex from 'knex'; +import { MockClient, createTracker } from 'knex-mock-client'; +import { describe, expect, test, vi } from 'vitest'; +import { applyFilter, applySearch } from './apply-query.js'; -function mockDatabase() { - const self: Record = { - andWhere: vi.fn(() => self), - orWhere: vi.fn(() => self), - orWhereRaw: vi.fn(() => self), - }; - - return self; -} - -describe('applySearch', () => { - const FAKE_SCHEMA: SchemaOverview = { - collections: { - test: { - collection: 'test', - primary: 'id', - singleton: false, - sortField: null, - note: null, - accountability: null, - fields: { - text: { - field: 'text', - defaultValue: null, - nullable: false, - generated: false, - type: 'text', - dbType: null, - precision: null, - scale: null, - special: [], - note: null, - validation: null, - alias: false, - }, - float: { - field: 'float', - defaultValue: null, - nullable: false, - generated: false, - type: 'float', - dbType: null, - precision: null, - scale: null, - special: [], - note: null, - validation: null, - alias: false, - }, - integer: { - field: 'integer', - defaultValue: null, - nullable: false, - generated: false, - type: 'integer', - dbType: null, - precision: null, - scale: null, - special: [], - note: null, - validation: null, - alias: false, - }, - id: { - field: 'id', - defaultValue: null, - nullable: false, - generated: false, - type: 'uuid', - dbType: null, - precision: null, - scale: null, - special: [], - note: null, - validation: null, - alias: false, - }, +const FAKE_SCHEMA: SchemaOverview = { + collections: { + test: { + collection: 'test', + primary: 'id', + singleton: false, + sortField: null, + note: null, + accountability: null, + fields: { + text: { + field: 'text', + defaultValue: null, + nullable: false, + generated: false, + type: 'text', + dbType: null, + precision: null, + scale: null, + special: [], + note: null, + validation: null, + alias: false, + }, + float: { + field: 'float', + defaultValue: null, + nullable: false, + generated: false, + type: 'float', + dbType: null, + precision: null, + scale: null, + special: [], + note: null, + validation: null, + alias: false, + }, + integer: { + field: 'integer', + defaultValue: null, + nullable: false, + generated: false, + type: 'integer', + dbType: null, + precision: null, + scale: null, + special: [], + note: null, + validation: null, + alias: false, + }, + id: { + field: 'id', + defaultValue: null, + nullable: false, + generated: false, + type: 'uuid', + dbType: null, + precision: null, + scale: null, + special: [], + note: null, + validation: null, + alias: false, }, }, }, - relations: [], - }; + }, + relations: [], +}; + +describe('applySearch', () => { + function mockDatabase() { + const self: Record = { + andWhere: vi.fn(() => self), + orWhere: vi.fn(() => self), + orWhereRaw: vi.fn(() => self), + }; + + return self; + } test.each(['0x56071c902718e681e274DB0AaC9B4Ed2d027924d', '0b11111', '0.42e3', 'Infinity', '42.000'])( 'Prevent %s from being cast to number', @@ -120,3 +122,72 @@ describe('applySearch', () => { expect(db['orWhereRaw']).toBeCalledTimes(1); }); }); + +describe('applyFilter', () => { + describe('boolean filter operators', () => { + const operators = [ + { + filterOperator: 'null', + sqlWhereClause: { + true: '$column is null', + false: '$column is not null', + }, + }, + { + filterOperator: 'empty', + sqlWhereClause: { + true: '($column is null or $column = ?)', + false: '($column is not null and $column != ?)', + }, + }, + ]; + + const withReverseOperators = operators.reduce((acc, cur) => { + const reverse = { + filterOperator: `n${cur.filterOperator}`, + sqlWhereClause: { + true: cur.sqlWhereClause.false, + false: cur.sqlWhereClause.true, + }, + }; + + acc.push(reverse); + return acc; + }, operators); + + for (const { filterOperator, sqlWhereClause } of withReverseOperators) { + for (const filterValue of [true, false]) { + test(`${filterOperator} with value ${filterValue}`, async () => { + class Client_SQLite3 extends MockClient {} + + const db = vi.mocked(knex.default({ client: Client_SQLite3 })); + + const queryBuilder = db.queryBuilder(); + + const collection = 'test'; + const field = 'text'; + + const rootFilter = { + _and: [{ [field]: { [`_${filterOperator}`]: filterValue } }], + }; + + const { query } = applyFilter(db, FAKE_SCHEMA, queryBuilder, rootFilter, collection, {}); + + const tracker = createTracker(db); + tracker.on.select('*').response([]); + + await query; + + const sql = tracker.history.select[0]?.sql.match(/select \* where \((.*)\)/)?.[1]; + + const expectedSql = sqlWhereClause[filterValue ? 'true' : 'false'].replaceAll( + '$column', + `"${collection}"."${field}"` + ); + + expect(sql).toEqual(expectedSql); + }); + } + } + }); +}); diff --git a/api/src/utils/apply-query.ts b/api/src/utils/apply-query.ts index cbf171b992..16540ca9d9 100644 --- a/api/src/utils/apply-query.ts +++ b/api/src/utils/apply-query.ts @@ -12,6 +12,7 @@ import type { import { getFilterOperatorsForType, getOutputTypeForFunction } from '@directus/utils'; import type { Knex } from 'knex'; import { clone, isPlainObject } from 'lodash-es'; +import { customAlphabet } from 'nanoid/non-secure'; import validate from 'uuid-validate'; import { getHelpers } from '../database/helpers/index.js'; import { InvalidQueryException } from '../exceptions/invalid-query.js'; @@ -21,9 +22,6 @@ import { getColumn } from './get-column.js'; import { getRelationInfo } from './get-relation-info.js'; import { stripFunction } from './strip-function.js'; -// @ts-ignore -import { customAlphabet } from 'nanoid/non-secure'; - export const generateAlias = customAlphabet('abcdefghijklmnopqrstuvwxyz', 5); /** @@ -576,21 +574,21 @@ export function applyFilter( // See https://github.com/knex/knex/issues/4518 @TODO remove as any once knex is updated // These operators don't rely on a value, and can thus be used without one (eg `?filter[field][_null]`) - if (operator === '_null' || (operator === '_nnull' && compareValue === false)) { + if ((operator === '_null' && compareValue !== false) || (operator === '_nnull' && compareValue === false)) { dbQuery[logical].whereNull(selectionRaw); } - if (operator === '_nnull' || (operator === '_null' && compareValue === false)) { + if ((operator === '_nnull' && compareValue !== false) || (operator === '_null' && compareValue === false)) { dbQuery[logical].whereNotNull(selectionRaw); } - if (operator === '_empty' || (operator === '_nempty' && compareValue === false)) { + if ((operator === '_empty' && compareValue !== false) || (operator === '_nempty' && compareValue === false)) { dbQuery[logical].andWhere((query) => { query.whereNull(key).orWhere(key, '=', ''); }); } - if (operator === '_nempty' || (operator === '_empty' && compareValue === false)) { + if ((operator === '_nempty' && compareValue !== false) || (operator === '_empty' && compareValue === false)) { dbQuery[logical].andWhere((query) => { query.whereNotNull(key).andWhere(key, '!=', ''); }); diff --git a/contributors.yml b/contributors.yml index 399a1e4379..ed001bac35 100644 --- a/contributors.yml +++ b/contributors.yml @@ -38,6 +38,7 @@ - groksrc - timio23 - craigharman +- acautin - ninogjoni - ched-dev - anantakrishna