diff --git a/here-naksha-handler-activitylog/src/jvmTest/java/com/here/naksha/handler/activitylog/ActivityLogRequestTranslationUtilTest.java b/here-naksha-handler-activitylog/src/jvmTest/java/com/here/naksha/handler/activitylog/ActivityLogRequestTranslationUtilTest.java index e9ccf776f..c9f78a1b4 100644 --- a/here-naksha-handler-activitylog/src/jvmTest/java/com/here/naksha/handler/activitylog/ActivityLogRequestTranslationUtilTest.java +++ b/here-naksha-handler-activitylog/src/jvmTest/java/com/here/naksha/handler/activitylog/ActivityLogRequestTranslationUtilTest.java @@ -133,9 +133,7 @@ void shouldTranslateActivityLogIdsToFeatureIds() { assertTrue(featureIds.containsAll(List.of(firstId, secondId))); // And: the pQuery left is effectively dead - // TODO CASL-1123: in the future we should simply delete such IPropertyQuery - POr root = (POr) readFeatures.getQuery().getProperties(); - assertTrue(root.stream().allMatch(PTrue.class::isInstance)); + assertNull(readFeatures.getQuery().getProperties()); // And: there are no guids (nothing was declared in original featureIds) assertTrue(readFeatures.getGuids().isEmpty()); diff --git a/here-naksha-lib-handlers/src/jvmMain/java/com/here/naksha/lib/handlers/util/PropertyOperationUtil.java b/here-naksha-lib-handlers/src/jvmMain/java/com/here/naksha/lib/handlers/util/PropertyOperationUtil.java index d17ab78b8..cda94d38a 100644 --- a/here-naksha-lib-handlers/src/jvmMain/java/com/here/naksha/lib/handlers/util/PropertyOperationUtil.java +++ b/here-naksha-lib-handlers/src/jvmMain/java/com/here/naksha/lib/handlers/util/PropertyOperationUtil.java @@ -21,36 +21,45 @@ import com.here.naksha.lib.core.lambdas.F1; import java.util.Collections; import java.util.HashSet; +import java.util.List; +import java.util.Optional; import java.util.Set; +import java.util.function.Supplier; import naksha.model.request.RequestQuery; import naksha.model.request.query.IPropertyQuery; import naksha.model.request.query.PAnd; -import naksha.model.request.query.PFalse; import naksha.model.request.query.PNot; import naksha.model.request.query.POr; import naksha.model.request.query.PQuery; -import naksha.model.request.query.PTrue; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -public class PropertyOperationUtil { +public final class PropertyOperationUtil { private PropertyOperationUtil() { } + /** + * Traverses the property query tree of the given {@link RequestQuery} and disables any {@link PQuery} nodes that match the provided predicate. + *

+ * The removed queries are collected and returned as a set. + * Important: this method mutates the given {@code requestQuery}. + * After execution, {@code requestQuery.getProperties()} may reference a different {@link IPropertyQuery} object than before, reflecting the removal of matching queries. + *

+ * If the request has no property query, the returned set will be empty and the request is left unchanged. + * + * @param requestQuery the request whose property query tree is to be traversed and modified + * @param shouldDisable a predicate that determines whether a {@link PQuery} should be removed + * @return a set containing all {@link PQuery} instances that were removed from the property query tree; returns an empty set if no queries were removed + */ public static Set disablePQueriesInRequest(@NotNull RequestQuery requestQuery, @NotNull F1 shouldDisable) { IPropertyQuery rootPropertyQuery = requestQuery.getProperties(); if (rootPropertyQuery != null) { - // if there is only single PQuery in the whole request, disable without tree traversal by simply removing it (set to null) - if (rootPropertyQuery instanceof PQuery rootPQuery && shouldDisable.call(rootPQuery)) { - requestQuery.setProperties(null); - return Set.of(rootPQuery); - } else { - // if there is a tree (not a PQuery) under `requestQuery.properties` - traverse the tree and logically disable matching pQuery - HashSet disabledProperties = new HashSet<>(); - disablePropertyInPropertyQueryTree(rootPropertyQuery, null, shouldDisable, disabledProperties); - return disabledProperties; - } + HashSet disabledProperties = new HashSet<>(); + IPropertyQuery newRootPropertyQuery = disablePropertyInPropertyQueryTree( + rootPropertyQuery, shouldDisable, disabledProperties + ).orElse(null); + requestQuery.setProperties(newRootPropertyQuery); + return disabledProperties; } // root property query is null -> no disabled property queries -> empty set return Collections.emptySet(); @@ -58,36 +67,82 @@ public static Set disablePQueriesInRequest(@NotNull RequestQuery request /** * @param current Currently traversed node - * @param parent Parent containing current node (can be null for first iteration, should be checked on call-site) - * @param removalCondition If evaluates to true, it effectively disables the check by replacing it with `true-ish` query + * @param removalCondition Predicate that determines whether a {@link PQuery} should be disabled * @param disabledProperties Set of so-far disabled property queries + * @return an {@link Optional} containing the updated query node, or an empty optional if the node is removed as a result of disabling */ - // TODO CASL-1123: this can be improved - we could inline "always true" statement such as AND(PTrue, PTrue) or OR(PTrue, PFalse) - // in such cases we can simply remove the node - in edge cases, we could end up without IPropertyQuery at all (if all gets resolved) - private static void disablePropertyInPropertyQueryTree( - @NotNull IPropertyQuery current, @Nullable IPropertyQuery parent, F1 removalCondition, Set disabledProperties + private static Optional disablePropertyInPropertyQueryTree( + @NotNull IPropertyQuery current, + @NotNull F1 removalCondition, + @NotNull Set disabledProperties ) { - switch (current) { - case PAnd pAnd -> pAnd.forEach(andChild -> disablePropertyInPropertyQueryTree(andChild, pAnd, removalCondition, disabledProperties)); - case POr pOr -> pOr.forEach(orChild -> disablePropertyInPropertyQueryTree(orChild, pOr, removalCondition, disabledProperties)); - case PNot pNot -> disablePropertyInPropertyQueryTree(pNot.getQuery(), pNot, removalCondition, disabledProperties); - case PQuery currentyPQuery when removalCondition.call(currentyPQuery) -> { - if (parent instanceof PAnd pAndParent) { - pAndParent.remove(current); - pAndParent.add(PTrue.INSTANCE); - disabledProperties.add(currentyPQuery); - } else if (parent instanceof POr pOrParent) { - pOrParent.remove(current); - pOrParent.add(PTrue.INSTANCE); - disabledProperties.add(currentyPQuery); - } else if (parent instanceof PNot pNotParent) { - pNotParent.setQuery(PFalse.INSTANCE); - disabledProperties.add(currentyPQuery); - } - } - default -> { - // unhandled type / not matching pQuery => stop traversal without failing + return switch (current) { + case PAnd pAnd -> handleCompoundQuery( + pAnd, + removalCondition, + disabledProperties, + PAnd::new + ); + case POr pOr -> handleCompoundQuery( + pOr, + removalCondition, + disabledProperties, + POr::new + ); + case PNot pNot -> disablePropertyInPropertyQueryTree( + pNot.getQuery(), removalCondition, disabledProperties + ).flatMap(pq -> Optional.of(new PNot(pq))); + case PQuery currentPQuery when removalCondition.call(currentPQuery) -> { + disabledProperties.add(currentPQuery); + yield disabledPropertyQuery(); } + default -> Optional.of(current); + }; + } + + private static Optional disabledPropertyQuery() { + return Optional.empty(); + } + + private static boolean allChildrenDisabled(List> children) { + return children.stream().allMatch(Optional::isEmpty); + } + + private static List removeDisabledChildren(List> children) { + return children.stream() + .filter(Optional::isPresent) + .map(Optional::get) + .toList(); + } + + private static & IPropertyQuery> Optional handleCompoundQuery( + T compoundQuery, + F1 removalCondition, + Set disabledProperties, + Supplier constructor + ) { + List> newChildren = disablePropertyInChildrenQueryTree( + compoundQuery, + removalCondition, + disabledProperties + ); + if (!compoundQuery.isEmpty() && allChildrenDisabled(newChildren)) { + return disabledPropertyQuery(); } + compoundQuery = constructor.get(); + compoundQuery.addAll(removeDisabledChildren(newChildren)); + return Optional.of(compoundQuery); + } + + private static & IPropertyQuery> List> disablePropertyInChildrenQueryTree( + T compoundQuery, + F1 removalCondition, + Set disabledProperties + ) { + return compoundQuery.stream() + .map(child -> disablePropertyInPropertyQueryTree( + child, removalCondition, disabledProperties + )) + .toList(); } } diff --git a/here-naksha-lib-handlers/src/jvmTest/java/com/here/naksha/lib/handlers/util/PropertyOperationUtilTest.java b/here-naksha-lib-handlers/src/jvmTest/java/com/here/naksha/lib/handlers/util/PropertyOperationUtilTest.java index 1e96bd48e..df7d509fd 100644 --- a/here-naksha-lib-handlers/src/jvmTest/java/com/here/naksha/lib/handlers/util/PropertyOperationUtilTest.java +++ b/here-naksha-lib-handlers/src/jvmTest/java/com/here/naksha/lib/handlers/util/PropertyOperationUtilTest.java @@ -1,127 +1,255 @@ package com.here.naksha.lib.handlers.util; -import static com.here.naksha.lib.handlers.util.PropertyOperationUtil.disablePQueriesInRequest; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; - import com.here.naksha.lib.core.lambdas.F1; -import java.util.List; -import java.util.Set; import naksha.base.StringList; import naksha.model.request.RequestQuery; -import naksha.model.request.query.DoubleOp; -import naksha.model.request.query.IPropertyQuery; -import naksha.model.request.query.PAnd; -import naksha.model.request.query.PFalse; -import naksha.model.request.query.PNot; -import naksha.model.request.query.POr; -import naksha.model.request.query.PQuery; -import naksha.model.request.query.PTrue; -import naksha.model.request.query.Property; -import naksha.model.request.query.StringOp; +import naksha.model.request.query.*; import org.junit.jupiter.api.Test; +import java.util.List; +import java.util.Set; + +import static com.here.naksha.lib.handlers.util.PropertyOperationUtil.disablePQueriesInRequest; +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.*; + class PropertyOperationUtilTest { + private final F1 dummyShouldDisable = _ -> false; + + @Test + void shouldNothingBeDisabledWhenPropertiesAbsent() { + // Given + RequestQuery query = new RequestQuery(); + assumeTrue(query.getProperties() == null); + + // When + Set disabledPQueries = disablePQueriesInRequest(query, dummyShouldDisable); + + // Then + assertEquals(0, disabledPQueries.size()); + assertNull(query.getProperties()); + } + + @Test + void shouldDisableSingularPQueryInRequest() { + // Given: request query with POp + PQuery pQuery = new PQuery( + new Property("nested", "object"), + StringOp.EQUALS, + "text_value" + ); + RequestQuery query = new RequestQuery(); + query.setProperties(pQuery); + + // When + Set disabledPQueries = disablePQueriesInRequest(query, pQueryMatchesPath("nested", "object")); + + // Then + assertEquals(1, disabledPQueries.size()); + assertEquals(pQuery, disabledPQueries.iterator().next()); + assertNull(query.getProperties()); + } + + @Test + void shouldComposedPQueryInRequest() { + // Given: request with dummy query with POp: + // - query all speed limits of 60 + // - "car allowed signs": signs with type `car_allowed` or value that is NOT `can_not_allowed` + PQuery typeIsSpeedLimit = new PQuery(new Property("sign", "type"), StringOp.EQUALS, "speed_limit"); + PQuery valueIs60 = new PQuery(new Property("sign", "value"), DoubleOp.EQ, 60.0); + PQuery typeIsCarAllowed = new PQuery(new Property("sign", "type"), StringOp.EQUALS, "car_allowed"); + PQuery valueIsCarNotAllowed = new PQuery(new Property("sign", "value"), StringOp.EQUALS, "car_not_allowed"); + IPropertyQuery originalPropertyQuery = new POr( + new PAnd(typeIsSpeedLimit, valueIs60), + new POr(typeIsCarAllowed, new PNot(valueIsCarNotAllowed)) + ); + RequestQuery query = new RequestQuery(); + query.setProperties(originalPropertyQuery); + + // When: disabling all queries related to `sign.value` + Set disabledPQueries = disablePQueriesInRequest(query, pQueryMatchesPath("sign", "value")); + + // Then: disabled subqueries are about value + assertEquals(2, disabledPQueries.size()); + assertTrue(disabledPQueries.containsAll(List.of(valueIs60, valueIsCarNotAllowed))); + + // And: request was correctly mutated + IPropertyQuery newPropertyQuery = query.getProperties(); + assertNotNull(newPropertyQuery); + // root is OR with 2 child nodes + assertInstanceOf(POr.class, newPropertyQuery); + POr root = (POr) newPropertyQuery; + assertEquals(2, root.size()); + // first child is AND with 1 child node + PAnd andUnderRoot = assertInstanceOf(PAnd.class, root.getFirst()); + assertEquals(1, andUnderRoot.size()); + assertEquals(typeIsSpeedLimit, andUnderRoot.getFirst()); + // second child is OR with 1 child node + POr orUnderRoot = assertInstanceOf(POr.class, root.get(1)); + assertEquals(1, orUnderRoot.size()); + assertEquals(typeIsCarAllowed, orUnderRoot.getFirst()); + } + + @Test + void shouldNotRemoveEmptyAnd() { + // Given + PAnd root = new PAnd(); + RequestQuery query = new RequestQuery(); + query.setProperties(root); + + // When + disablePQueriesInRequest(query, dummyShouldDisable); + + // Then + IPropertyQuery propertyQuery = query.getProperties(); + assertInstanceOf(PAnd.class, propertyQuery); + } + + @Test + void shouldNotRemoveEmptyOr() { + // Given + POr root = new POr(); + RequestQuery query = new RequestQuery(); + query.setProperties(root); + + // When + disablePQueriesInRequest(query, dummyShouldDisable); + + // Then + IPropertyQuery propertyQuery = query.getProperties(); + assertInstanceOf(POr.class, propertyQuery); + } + + @Test + void shouldRemoveOrWhenInnerQueriesDisabled() { + // Given + POr root = new POr(); + root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "a")); + root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "b")); + RequestQuery query = new RequestQuery(); + query.setProperties(root); + + // When + disablePQueriesInRequest(query, pQueryMatchesPath("foo", "bar")); + + // Then + assertNull(query.getProperties()); + } + + @Test + void shouldRemoveAndWhenInnerQueriesDisabled() { + // Given + PAnd root = new PAnd(); + root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "a")); + root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "b")); + RequestQuery query = new RequestQuery(); + query.setProperties(root); + + // When + disablePQueriesInRequest(query, pQueryMatchesPath("foo", "bar")); + + // Then + assertNull(query.getProperties()); + } + + @Test + void shouldRemoveDisabledChildFromAndAndKeepOther() { + // Given + PQuery a = new PQuery(new Property("a"), StringOp.EQUALS, "1"); + PQuery b = new PQuery(new Property("b"), StringOp.EQUALS, "2"); + + // And + RequestQuery query = new RequestQuery(); + query.setProperties(new PAnd(a, b)); + + // When + Set disabled = disablePQueriesInRequest(query, pQueryMatchesPath("a")); + + // Then + assertEquals(Set.of(a), disabled); + + // And + IPropertyQuery newRoot = query.getProperties(); + assertNotNull(newRoot); + PAnd pAnd = assertInstanceOf(PAnd.class, newRoot); + + // And + assertEquals(1, pAnd.size()); + assertEquals(b, pAnd.getFirst()); + } + + @Test + void shouldRemoveDisabledChildFromOrAndKeepOther() { + // Given + PQuery a = new PQuery(new Property("a"), StringOp.EQUALS, "1"); + PQuery b = new PQuery(new Property("b"), StringOp.EQUALS, "2"); + + // And + RequestQuery query = new RequestQuery(); + query.setProperties(new POr(a, b)); + + // When + Set disabled = disablePQueriesInRequest(query, pQueryMatchesPath("a")); + + // Then + assertEquals(Set.of(a), disabled); + + // And + IPropertyQuery newRoot = query.getProperties(); + assertNotNull(newRoot); + POr pOr = assertInstanceOf(POr.class, newRoot); + + // And + assertEquals(1, pOr.size()); + assertEquals(b, pOr.getFirst()); + } + + @Test + void shouldRemoveNotWhenInnerQueryDisabled() { + // Given + PQuery inner = new PQuery(new Property("a"), StringOp.EQUALS, "1"); + PNot root = new PNot(inner); + + // And + RequestQuery query = new RequestQuery(); + query.setProperties(root); + + // When + Set disabled = disablePQueriesInRequest(query, pQueryMatchesPath("a")); + + // Then + assertEquals(Set.of(inner), disabled); + assertNull(query.getProperties()); + } + + @Test + void shouldNotDuplicateDisabledQueries() { + // Given + PQuery a = new PQuery(new Property("a"), StringOp.EQUALS, "1"); + + RequestQuery query = new RequestQuery(); + query.setProperties(new PAnd(a, a)); + + // When + Set disabled = disablePQueriesInRequest(query, pQueryMatchesPath("a")); + + // Then + assertEquals(1, disabled.size()); + assertTrue(disabled.contains(a)); + } - @Test - void shouldDisableSingularPQueryInRequest() { - // Given: request query with POp - PQuery pQuery = new PQuery( - new Property("nested", "object"), - StringOp.EQUALS, - "text_value" - ); - RequestQuery query = new RequestQuery(); - query.setProperties(pQuery); - - // When - Set disabledPQueries = disablePQueriesInRequest(query, pQueryMatchesPath("nested", "object")); - - // Then - assertEquals(1, disabledPQueries.size()); - assertEquals(pQuery, disabledPQueries.iterator().next()); - } - - @Test - void shouldComposedPQueryInRequest() { - // Given: request with dummy query with POp: - // - query all speed limits of 60 AND "car allowed" signs - // - "car allowed signs": signs with type `car_allowed` or value that is NOT `can_not_allowed` - PQuery typeIsSpeedLimit = new PQuery(new Property("sign", "type"), StringOp.EQUALS, "speed_limit"); - PQuery valueIs60 = new PQuery(new Property("sign", "value"), DoubleOp.EQ, 60.0); - PQuery typeIsCarAllowed = new PQuery(new Property("sign", "type"), StringOp.EQUALS, "car_allowed"); - PQuery valueIsCarNotAllowed = new PQuery(new Property("sign", "value"), StringOp.EQUALS, "car_not_allowed"); - IPropertyQuery originalPropertyQuery = new POr( - new PAnd(typeIsSpeedLimit, valueIs60), - new POr(typeIsCarAllowed, new PNot(valueIsCarNotAllowed)) - ); - RequestQuery query = new RequestQuery(); - query.setProperties(originalPropertyQuery); - - // When: disabling all queries related to `sign.value` - Set disabledPQueries = disablePQueriesInRequest(query, pQueryMatchesPath("sign", "value")); - - // Then: disabled subqueries are about value - assertEquals(2, disabledPQueries.size()); - assertTrue(disabledPQueries.containsAll(List.of(valueIs60, valueIsCarNotAllowed))); - - // And: original request was correctly mutated - IPropertyQuery mutatedPropertyQuery = query.getProperties(); - assertSame(originalPropertyQuery, mutatedPropertyQuery, - "Property query should be mutated in place - mutated and original query must refer to the same instance"); - // root is OR with 2 child nodes - assertInstanceOf(POr.class, mutatedPropertyQuery); - POr root = (POr) mutatedPropertyQuery; - assertEquals(2, root.size()); - // first child is AND with 2 child nodes - including PTrue made from `valueIs60` - assertInstanceOf(PAnd.class, root.get(0)); - PAnd andUnderRoot = (PAnd) root.get(0); - assertEquals(2, andUnderRoot.size()); - assertEquals(typeIsSpeedLimit, andUnderRoot.get(0)); - assertEquals(PTrue.INSTANCE, andUnderRoot.get(1)); - // second child is OR with 2 child nodes - including mutated PNot with PFalse made from `valueIsCarNotAllowed`: - assertInstanceOf(POr.class, root.get(1)); - POr orUnderRoot = (POr) root.get(1); - assertEquals(2, orUnderRoot.size()); - assertEquals(typeIsCarAllowed, orUnderRoot.get(0)); - assertInstanceOf(PNot.class, orUnderRoot.get(1)); - PNot nestedPNot = (PNot) orUnderRoot.get(1); - assertEquals(PFalse.INSTANCE, nestedPNot.getQuery()); - } - - @Test - void shouldDisableSimpleOr(){ - // Given - POr root = new POr(); - root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "a")); - root.add(new PQuery(new Property("foo", "bar"), StringOp.EQUALS, "b")); - RequestQuery query = new RequestQuery(); - query.setProperties(root); - - // When - PropertyOperationUtil.disablePQueriesInRequest(query, pQueryMatchesPath("foo", "bar")); - - // Then - POr mutatedPropertyQuery = (POr) query.getProperties(); - assertEquals(2, mutatedPropertyQuery.size()); - assertInstanceOf(PTrue.class, mutatedPropertyQuery.get(0)); - assertInstanceOf(PTrue.class, mutatedPropertyQuery.get(1)); - } - - private F1 pQueryMatchesPath(String... expectedPath) { - return pQuery -> { - StringList queryPath = pQuery.getProperty().getPath(); - if (expectedPath.length != queryPath.size()) { - return false; - } - for (int i = 0; i < expectedPath.length; i++) { - if (!expectedPath[i].equals(queryPath.get(i))) { - return false; - } - } - return true; - }; - } + private F1 pQueryMatchesPath(String... expectedPath) { + return pQuery -> { + StringList queryPath = pQuery.getProperty().getPath(); + if (expectedPath.length != queryPath.size()) { + return false; + } + for (int i = 0; i < expectedPath.length; i++) { + if (!expectedPath[i].equals(queryPath.get(i))) { + return false; + } + } + return true; + }; + } } \ No newline at end of file