#4705 Fix conditional format fall through

gchq · Jan 22, 2025 · 3fe9657 · 3fe9657
1 parent b65d0eb
commit 3fe9657
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 30 deletions.
diff --git a/...om-query-common/src/main/java/stroom/query/common/v2/ConditionalFormattingRowCreator.java b/...om-query-common/src/main/java/stroom/query/common/v2/ConditionalFormattingRowCreator.java
@@ -69,8 +69,8 @@ public static Optional<ItemMapper<Row>> create(final List<Column> originalColumn
                                         rule.getExpression(),
                                         queryFieldIndex,
                                         dateTimeSettings);
-                        optionalValuesPredicate.ifPresent(columnExpressionMatcher ->
-                                ruleAndMatchers.add(new RuleAndMatcher(rule, columnExpressionMatcher)));
+                        final Predicate<Val[]> predicate = optionalValuesPredicate.orElse(t -> true);
+                        ruleAndMatchers.add(new RuleAndMatcher(rule, predicate));
                     } catch (final RuntimeException e) {
                         throw new RuntimeException("Error evaluating conditional formatting rule: " +
                                                    rule.getExpression() +

diff --git a/.../stroom-query-common/src/main/java/stroom/query/common/v2/ExpressionPredicateFactory.java b/.../stroom-query-common/src/main/java/stroom/query/common/v2/ExpressionPredicateFactory.java
@@ -36,6 +36,7 @@
 import java.math.BigDecimal;
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
@@ -96,21 +97,29 @@ private <T> Optional<Predicate<T>> createOperatorPredicate(final ExpressionOpera
                                                                final ValueFunctionFactories<T> queryFieldIndex,
                                                                final DateTimeSettings dateTimeSettings,
                                                                final WordListProvider wordListProvider) {
-        if (!operator.enabled() || operator.getChildren() == null || operator.getChildren().isEmpty()) {
+        // If the operator is not enabled then ignore this branch.
+        if (!operator.enabled()) {
             return Optional.empty();
         }
 
+        // Create child predicates.
+        final List<Predicate<T>> predicates;
+        if (operator.getChildren() != null && !operator.getChildren().isEmpty()) {
+            predicates = new ArrayList<>(operator.getChildren().size());
+            for (final ExpressionItem child : operator.getChildren()) {
+                Optional<Predicate<T>> optional = createPredicate(
+                        child,
+                        queryFieldIndex,
+                        dateTimeSettings,
+                        wordListProvider);
+                optional.ifPresent(predicates::add);
+            }
+        } else {
+            predicates = Collections.emptyList();
+        }
+
         return switch (operator.op()) {
             case AND -> {
-                final List<Predicate<T>> predicates = new ArrayList<>(operator.getChildren().size());
-                for (final ExpressionItem child : operator.getChildren()) {
-                    Optional<Predicate<T>> optional = createPredicate(
-                            child,
-                            queryFieldIndex,
-                            dateTimeSettings,
-                            wordListProvider);
-                    optional.ifPresent(predicates::add);
-                }
                 if (predicates.isEmpty()) {
                     yield Optional.empty();
                 }
@@ -120,15 +129,6 @@ private <T> Optional<Predicate<T>> createOperatorPredicate(final ExpressionOpera
                 yield AndPredicate.create(predicates);
             }
             case OR -> {
-                final List<Predicate<T>> predicates = new ArrayList<>(operator.getChildren().size());
-                for (final ExpressionItem child : operator.getChildren()) {
-                    Optional<Predicate<T>> optional = createPredicate(
-                            child,
-                            queryFieldIndex,
-                            dateTimeSettings,
-                            wordListProvider);
-                    optional.ifPresent(predicates::add);
-                }
                 if (predicates.isEmpty()) {
                     yield Optional.empty();
                 }
@@ -138,18 +138,13 @@ private <T> Optional<Predicate<T>> createOperatorPredicate(final ExpressionOpera
                 yield OrPredicate.create(predicates);
             }
             case NOT -> {
-                if (operator.getChildren().size() > 1) {
+                if (predicates.size() > 1) {
                     throw new MatchException("Unexpected number of child terms in NOT");
                 }
-                final Optional<Predicate<T>> optionalChildPredicate = createPredicate(
-                        operator.getChildren().getFirst(),
-                        queryFieldIndex,
-                        dateTimeSettings,
-                        wordListProvider);
-                if (optionalChildPredicate.isEmpty()) {
-                    yield Optional.empty();
+                if (predicates.isEmpty()) {
+                    yield Optional.of(t -> false);
                 }
-                yield NotPredicate.create(optionalChildPredicate.get());
+                yield NotPredicate.create(predicates.getFirst());
             }
         };
     }

diff --git a/unreleased_changes/20250122_195124_112__4705.md b/unreleased_changes/20250122_195124_112__4705.md
@@ -0,0 +1,24 @@
+* Issue **#4705** : Fix conditional format fall through.
+
+
+```sh
+# ********************************************************************************
+# Issue title: Conditional Formatting catch-all broken post 7.5
+# Issue link:  https://github.com/gchq/stroom/issues/4705
+# ********************************************************************************
+
+# ONLY the top line will be included as a change entry in the CHANGELOG.
+# The entry should be in GitHub flavour markdown and should be written on a SINGLE
+# line with no hard breaks. You can have multiple change files for a single GitHub issue.
+# The  entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
+# 'Fixed nasty bug'.
+#
+# Examples of acceptable entries are:
+#
+#
+# * Issue **123** : Fix bug with an associated GitHub issue in this repository
+#
+# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
+#
+# * Fix bug with no associated GitHub issue.
+```