Fix reversing of _null, _nnull, _empty and _nempty (#18801)

* Fix reversing of _null, _nnull, _empty and _nempty

When the value given is false, at the moment both `IS NULL` and `IS NOT NULL` are added to the query which means there are never results.

* Update contributors.yml

* Add test for applyFilter with boolean operators

* Revert unnecessary line break

* Create chilled-wolves-double.md

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

.changeset/chilled-wolves-double.md

@@ -0,0 +1,5 @@
+---
+"@directus/api": patch
+---
+
+Fixed reverse usage of null, nnull, empty and nempty filter operator

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<string, any> = {
-		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<string, any> = {
+			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);
+				});
+			}
+		}
+	});
+});

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, '!=', '');
 				});

contributors.yml

@@ -38,6 +38,7 @@
 - groksrc
 - timio23
 - craigharman
+- acautin
 - ninogjoni
 - ched-dev
 - anantakrishna