Skip to content

Commit 35d5603

Browse files
authored
Update error message from AmbiguousNotRule (#2959)
Update the error message returned by the AmbiguousNotRule to avoid users assuming that the suggested syntax is overall correct and appropriate for copy/pasting. Closes #2939
1 parent 4ba4a16 commit 35d5603

File tree

2 files changed

+13
-23
lines changed

2 files changed

+13
-23
lines changed

warehouse/query-core/src/main/java/datawave/query/rules/AmbiguousNotRule.java

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package datawave.query.rules;
22

33
import java.util.List;
4-
import java.util.stream.Collectors;
54

65
import org.apache.log4j.Logger;
7-
import org.apache.lucene.queryparser.flexible.core.nodes.AndQueryNode;
86
import org.apache.lucene.queryparser.flexible.core.nodes.NotBooleanQueryNode;
97
import org.apache.lucene.queryparser.flexible.core.nodes.QueryNode;
108

@@ -61,26 +59,21 @@ private String formatMessage(NotBooleanQueryNode node) {
6159
StringBuilder sb = new StringBuilder();
6260
sb.append("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: ");
6361

64-
String precedingTerms = getPrecedingTerms(node);
62+
String lastPrecedingTerm = getLastPrecedingTerm(node);
63+
6564
// @formatter:off
66-
return sb.append("\"")
67-
.append(precedingTerms)
68-
.append(" NOT\" should be \"(")
69-
.append(precedingTerms)
70-
.append(") NOT\".")
65+
return sb.append("the NOT clause will only be applied to \"")
66+
.append(lastPrecedingTerm)
67+
.append("\".")
7168
.toString();
7269
// @formatter:on
7370
}
7471

75-
// Return the terms preceding the NOT in the given node as a nicely formatted query string.
76-
private String getPrecedingTerms(NotBooleanQueryNode node) {
72+
// Return the last term preceding the NOT in the given, in query format.
73+
private String getLastPrecedingTerm(NotBooleanQueryNode node) {
7774
QueryNode junctionNode = node.getChildren().get(0);
78-
String junction = junctionNode instanceof AndQueryNode ? " AND " : " OR ";
79-
// @formatter:off
80-
return junctionNode.getChildren().stream()
81-
.map(LuceneQueryStringBuildingVisitor::build)
82-
.collect(Collectors.joining(junction));
83-
// @formatter:on
75+
List<QueryNode> children = junctionNode.getChildren();
76+
QueryNode lastChild = children.get(children.size() - 1);
77+
return LuceneQueryStringBuildingVisitor.build(lastChild);
8478
}
85-
8679
}

warehouse/query-core/src/test/java/datawave/query/rules/AmbiguousNotRuleTest.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ void testNOTWithWrappedMultiplePrecedingTerms(String junction) throws Exception
6666
void testNOTWithUnwrappedMultiplePrecedingTerms(String junction) throws Exception {
6767
givenQuery("FIELD1:abc " + junction + " FIELD2:def NOT FIELD:ghi");
6868

69-
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: \"FIELD1:abc " + junction
70-
+ " FIELD2:def NOT\" should be \"(FIELD1:abc " + junction + " FIELD2:def) NOT\".");
69+
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: the NOT clause will only be applied to \"FIELD2:def\".");
7170

7271
assertResult();
7372
}
@@ -79,8 +78,7 @@ void testNOTWithUnwrappedMultiplePrecedingTerms(String junction) throws Exceptio
7978
void testNOTWithUnwrappedAutomaticallyAndedPreceedingTerms() throws Exception {
8079
givenQuery("FIELD1:abc FIELD2:def NOT FIELD:ghi");
8180

82-
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: \"FIELD1:abc AND FIELD2:def NOT\" should be "
83-
+ "\"(FIELD1:abc AND FIELD2:def) NOT\".");
81+
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: the NOT clause will only be applied to \"FIELD2:def\".");
8482

8583
assertResult();
8684
}
@@ -103,8 +101,7 @@ void testNOTWithWrappedAutomaticallyAndedPreceedingTerms() throws Exception {
103101
void testQueryWithTermThatIsNotPartOfNOT() throws Exception {
104102
givenQuery("FIELD1:abc OR (FIELD2:abc FIELD3:def NOT FIELD4:ghi)");
105103

106-
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: \"FIELD2:abc AND FIELD3:def NOT\" should be "
107-
+ "\"(FIELD2:abc AND FIELD3:def) NOT\".");
104+
expectMessage("Ambiguous usage of NOT detected with multiple unwrapped preceding terms: the NOT clause will only be applied to \"FIELD3:def\".");
108105

109106
assertResult();
110107
}

0 commit comments

Comments
 (0)