From 38fbc1cb4ac014cbe1f5b291a064313bc00ec227 Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Wed, 27 Nov 2024 10:49:42 +0100 Subject: [PATCH 1/3] Fix #10695 Handle empty values for and/or/not/nor filters --- web/client/utils/ogc/Filter/FilterBuilder.js | 1 + .../ogc/Filter/__tests__/FilterBuilder-test.js | 15 ++++++++++++++- .../utils/ogc/Filter/__tests__/operators-test.js | 11 +++++++++++ web/client/utils/ogc/Filter/operators.js | 10 +++++----- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/web/client/utils/ogc/Filter/FilterBuilder.js b/web/client/utils/ogc/Filter/FilterBuilder.js index 1646c0cbb9..25d4d3ccab 100644 --- a/web/client/utils/ogc/Filter/FilterBuilder.js +++ b/web/client/utils/ogc/Filter/FilterBuilder.js @@ -135,6 +135,7 @@ module.exports = function({filterNS = "ogc", gmlVersion, wfsVersion = "1.1.0"} = and: logical.and.bind(null, filterNS), or: logical.or.bind(null, filterNS), not: logical.not.bind(null, filterNS), + nor: logical.nor.bind(null, filterNS), func: func.bind(null, filterNS), literal: getValue, // note: use valueReference method for filters and SortBy conditions. PropertyName is used in WFS 2.0 only for listing required attributes, while the rest uses ValueReference. diff --git a/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js b/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js index 81a550c742..b09955a9d0 100644 --- a/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js +++ b/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js @@ -66,7 +66,6 @@ describe('FilterBuilder', () => { expect( b.or([b.property("GEOMETRY").intersects(testGeom1), b.property("GEOMETRY").intersects(testGeom2)]) ).toBe(`${intersectsElem1}${intersectsElem2}`); - // not expect( b.not(b.property("GEOMETRY").intersects(testGeom1)) @@ -79,6 +78,20 @@ describe('FilterBuilder', () => { )).toBe(`${intersectsElem1}${intersectsElem2}` + `${intersectsElem2}${intersectsElem1}` + ``); + // empty array returns empty filter instead of undefined + // to check if is better to return an empty filter like or + expect( + b.and() + ).toBe(""); + expect( + b.or() + ).toBe(""); + expect( + b.not() + ).toBe(""); + expect( + b.nor() + ).toBe(""); }); it('valueReference 1.1.0', () => { diff --git a/web/client/utils/ogc/Filter/__tests__/operators-test.js b/web/client/utils/ogc/Filter/__tests__/operators-test.js index cac96b224b..47ed5eb508 100644 --- a/web/client/utils/ogc/Filter/__tests__/operators-test.js +++ b/web/client/utils/ogc/Filter/__tests__/operators-test.js @@ -72,6 +72,17 @@ describe('OGC Operators', () => { ogcComparisonOperators["="]("ogc", "TEST") )).toBe('TEST'); }); + it('logical functions with empty content', () => { + expect(logical.and("ogc" + + )).toBe(''); + expect(logical.or("ogc" + + )).toBe(''); + expect(logical.not("ogc" + + )).toBe(''); + }); it('spatial functions', () => { expect(spatial.intersects("ogc", propertyName("ogc", "GEOMETRY"), "TEST")).toBe("GEOMETRYTEST"); expect(spatial.bbox("ogc", propertyName("ogc", "GEOMETRY"), "TEST")).toBe("GEOMETRYTEST"); diff --git a/web/client/utils/ogc/Filter/operators.js b/web/client/utils/ogc/Filter/operators.js index 14e8341f26..cccf4f5808 100644 --- a/web/client/utils/ogc/Filter/operators.js +++ b/web/client/utils/ogc/Filter/operators.js @@ -18,10 +18,10 @@ const ogcComparisonOperators = { "isNull": (ns, content) => `<${ns}:PropertyIsNull>${content}` }; const ogcLogicalOperators = { - "AND": (ns, content) => `<${ns}:And>${content}`, - "OR": (ns, content) => `<${ns}:Or>${content}`, - "NOR": (ns, content) => `<${ns}:Not><${ns}:Or>${content}`, - "NOT": (ns, content) => `<${ns}:Not>${content}` + "AND": (ns, content) => content ? `<${ns}:And>${content}` : "", + "OR": (ns, content) => content ? `<${ns}:Or>${content}` : "", + "NOR": (ns, content) => content ? `<${ns}:Not><${ns}:Or>${content}` : "", + "NOT": (ns, content) => content ? `<${ns}:Not>${content}` : "" }; const ogcSpatialOperators = { @@ -44,7 +44,7 @@ const upper = (ns, value) => `<${ns}:UpperBoundary>${value} op(ns, Array.isArray(content) ? content.join("") : content); +const multiop = (ns, op, content) => op(ns, Array.isArray(content) ? content.join("") : content ); const logical = { and: (ns, content, ...other) => other && other.length > 0 ? multiop(ns, ogcLogicalOperators.AND, [content, ...other]) : multiop(ns, ogcLogicalOperators.AND, content), or: (ns, content, ...other) => other && other.length > 0 ? multiop(ns, ogcLogicalOperators.OR, [content, ...other]) : multiop(ns, ogcLogicalOperators.OR, content), From ced1801523806f43eeb332b65662b99d214b700d Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Wed, 27 Nov 2024 18:02:35 +0100 Subject: [PATCH 2/3] Update web/client/utils/ogc/Filter/operators.js Co-authored-by: Matteo V. --- web/client/utils/ogc/Filter/operators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/client/utils/ogc/Filter/operators.js b/web/client/utils/ogc/Filter/operators.js index cccf4f5808..bcc9da2f6f 100644 --- a/web/client/utils/ogc/Filter/operators.js +++ b/web/client/utils/ogc/Filter/operators.js @@ -44,7 +44,7 @@ const upper = (ns, value) => `<${ns}:UpperBoundary>${value} op(ns, Array.isArray(content) ? content.join("") : content ); +const multiop = (ns, op, content) => op(ns, Array.isArray(content) ? content.join("") : content); const logical = { and: (ns, content, ...other) => other && other.length > 0 ? multiop(ns, ogcLogicalOperators.AND, [content, ...other]) : multiop(ns, ogcLogicalOperators.AND, content), or: (ns, content, ...other) => other && other.length > 0 ? multiop(ns, ogcLogicalOperators.OR, [content, ...other]) : multiop(ns, ogcLogicalOperators.OR, content), From cdfd9fb6fab5d9314aaf3bf7ead3268c3933818e Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Wed, 27 Nov 2024 18:04:05 +0100 Subject: [PATCH 3/3] Apply suggestions from code review --- .../ogc/Filter/__tests__/FilterBuilder-test.js | 16 ++++------------ .../utils/ogc/Filter/__tests__/operators-test.js | 12 +++--------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js b/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js index b09955a9d0..496d3c6bed 100644 --- a/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js +++ b/web/client/utils/ogc/Filter/__tests__/FilterBuilder-test.js @@ -80,18 +80,10 @@ describe('FilterBuilder', () => { + ``); // empty array returns empty filter instead of undefined // to check if is better to return an empty filter like or - expect( - b.and() - ).toBe(""); - expect( - b.or() - ).toBe(""); - expect( - b.not() - ).toBe(""); - expect( - b.nor() - ).toBe(""); + expect(b.and()).toBe(""); + expect(b.or()).toBe(""); + expect(b.not()).toBe(""); + expect(b.nor()).toBe(""); }); it('valueReference 1.1.0', () => { diff --git a/web/client/utils/ogc/Filter/__tests__/operators-test.js b/web/client/utils/ogc/Filter/__tests__/operators-test.js index 47ed5eb508..d8fc4ddc85 100644 --- a/web/client/utils/ogc/Filter/__tests__/operators-test.js +++ b/web/client/utils/ogc/Filter/__tests__/operators-test.js @@ -73,15 +73,9 @@ describe('OGC Operators', () => { )).toBe('TEST'); }); it('logical functions with empty content', () => { - expect(logical.and("ogc" - - )).toBe(''); - expect(logical.or("ogc" - - )).toBe(''); - expect(logical.not("ogc" - - )).toBe(''); + expect(logical.and("ogc")).toBe(''); + expect(logical.or("ogc")).toBe(''); + expect(logical.not("ogc")).toBe(''); }); it('spatial functions', () => { expect(spatial.intersects("ogc", propertyName("ogc", "GEOMETRY"), "TEST")).toBe("GEOMETRYTEST");