From 86d10539ccd49a5b2074d18652d1605ff6072ce2 Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Thu, 23 Jan 2025 12:15:52 +0900 Subject: [PATCH] - drop half constraint - Disable drools-verifier test for DRL10 --- .../drl/parser/antlr4/DRLExprParserTest.java | 3 +- .../drl/parser/antlr4/DRLParserTestBase.java | 32 ++++++++++++++++++ .../drl/parser/antlr4/DescrDumperTest.java | 6 +++- .../drl/parser/antlr4/MiscDRLParserTest.java | 19 ++++------- .../antlr4/accumulate_with_nested_from.drl | 2 +- .../drl/parser/antlr4/in_operator_test.drl | 2 +- .../drl/parser/antlr4/notin_operator_test.drl | 2 +- .../drl/parser/antlr4/pluggable_operators.drl | 2 +- .../drl/parser/antlr4/DRL10Expressions.g4 | 33 +------------------ .../codegen/execmodel/BaseModelTest.java | 9 +++++ .../model/codegen/execmodel/CompilerTest.java | 3 ++ .../codegen/execmodel/HalfBinaryTest.java | 2 ++ .../codegen/execmodel/MvelOperatorsTest.java | 7 ++-- .../execmodel/NullSafeDereferencingTest.java | 2 ++ .../src/test/resources/logback-test.xml | 2 +- .../factmodel/traits/TraitMapCoreTest.java | 2 +- drools-verifier/drools-verifier-drl/pom.xml | 21 ++++++++++++ .../builder/VerifierKnowledgeBaseBuilder.java | 7 ++++ 18 files changed, 102 insertions(+), 54 deletions(-) create mode 100644 drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLParserTestBase.java diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java index 8cbebe73e6d..9a6f8747d9b 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLExprParserTest.java @@ -35,6 +35,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.condition.EnabledIf; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.ValueSource; @@ -175,7 +176,7 @@ void bindingConstraint() { @Test void bindingWithRestrictions() { - String source = "$x : property > value && < 20"; + String source = "$x : property > value && property < 20"; ConstraintConnectiveDescr result = parser.parse( source ); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLParserTestBase.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLParserTestBase.java new file mode 100644 index 00000000000..5574f6f3988 --- /dev/null +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DRLParserTestBase.java @@ -0,0 +1,32 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.drools.drl.parser.antlr4; + +import org.drools.drl.parser.DrlParser; + +public class DRLParserTestBase { + + protected static boolean isNewParser() { + return DrlParser.ANTLR4_PARSER_ENABLED; + } + + protected static boolean isOldParser() { + return !DrlParser.ANTLR4_PARSER_ENABLED; + } +} diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DescrDumperTest.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DescrDumperTest.java index 4133e9f2594..7b7a05b5fd3 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DescrDumperTest.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/DescrDumperTest.java @@ -30,11 +30,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIf; import org.kie.internal.builder.conf.LanguageLevelOption; import static org.assertj.core.api.Assertions.assertThat; -public class DescrDumperTest { +public class DescrDumperTest extends DRLParserTestBase { private DescrDumper dumper; @@ -47,6 +48,7 @@ void setUp() { dumper = new DescrDumper(); } + @EnabledIf("isOldParser") @Test void dump() { String input = "price > 10 && < 20 || == $val || == 30"; @@ -147,6 +149,7 @@ void dumpWithDateAttr() { assertThat(result).isEqualTo(expected); } + @EnabledIf("isOldParser") @Test void dumpComplex() { String input = "a ( > 60 && < 70 ) || ( > 50 && < 55 ) && a3 == \"black\" || a == 40 && a3 == \"pink\" || a == 12 && a3 == \"yellow\" || a3 == \"blue\""; @@ -220,6 +223,7 @@ void dumpBindings4() { assertThat(result).isEqualTo(expected); } + @EnabledIf("isOldParser") @Test void dumpBindingsWithRestriction() { String input = "$x : age > 10 && < 20 || > 30"; diff --git a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java index 56fbfe11152..b1e6f8a5645 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java +++ b/drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java @@ -86,7 +86,7 @@ /* * This test class is ported from org.drools.mvel.compiler.lang.RuleParserTest */ -class MiscDRLParserTest { +class MiscDRLParserTest extends DRLParserTestBase { private DrlParser parser; @@ -95,14 +95,6 @@ void setUp() { parser = ParserTestUtils.getParser(); } - private static boolean isNewParser() { - return DrlParser.ANTLR4_PARSER_ENABLED; - } - - private static boolean isOldParser() { - return !DrlParser.ANTLR4_PARSER_ENABLED; - } - private String readResource(final String filename) { Path path; try { @@ -881,6 +873,7 @@ void simpleRuleWithBindings() { assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("if ( a == b ) { " + " assert( foo3 );" + "} else {" + " retract( foo4 );" + "}" + " System.out.println( a4 );"); } + @EnabledIf("isOldParser") @Test void multipleRestrictionsConstraint() { RuleDescr rule = parseAndGetFirstRuleDescrFromFile("restrictions_test.drl"); @@ -2452,7 +2445,7 @@ void inOperator() { assertThat(pattern.getConstraint().getDescrs()).hasSize(1); ExprConstraintDescr fld = (ExprConstraintDescr) pattern.getConstraint().getDescrs().get(0); - assertThat(fld.getExpression()).isEqualTo("age > 30 && < 40"); + assertThat(fld.getExpression()).isEqualTo("age > 30 && age < 40"); // the second col, with 2 fields, the first with 2 restrictions, the // second field with one @@ -2486,7 +2479,7 @@ void notInOperator() { assertThat(pattern.getConstraint().getDescrs()).hasSize(1); ExprConstraintDescr fld = (ExprConstraintDescr) pattern.getConstraint().getDescrs().get(0); - assertThat(fld.getExpression()).isEqualTo("age > 30 && < 40"); + assertThat(fld.getExpression()).isEqualTo("age > 30 && age < 40"); // the second col, with 2 fields, the first with 2 restrictions, the // second field with one @@ -2536,6 +2529,7 @@ void constraintOrConnective() { assertThat(fcd.getExpression()).isEqualToIgnoringWhitespace("age < 42 || location==\"atlanta\""); } + @EnabledIf("isOldParser") @Test void restrictions() { final String text = "rule X when Foo( bar > 1 || == 1 ) then end\n"; @@ -2987,7 +2981,7 @@ void pluggableOperators() { assertThat(eventE.getConstraint().getDescrs()).hasSize(1); ExprConstraintDescr fcdE = (ExprConstraintDescr) eventE.getConstraint().getDescrs().get(0); - assertThat(fcdE.getExpression()).isEqualTo("this not before[1, 10] $b || after[1, 10] $c && this after[1, 5] $d"); + assertThat(fcdE.getExpression()).isEqualTo("this not before[1, 10] $b || this after[1, 10] $c && this after[1, 5] $d"); } @Test @@ -3896,6 +3890,7 @@ void constraintOperators(String constraint) { assertThat(exprConstraintDescr.getExpression()).isEqualToIgnoringWhitespace(constraint); } + @EnabledIf("isOldParser") @ParameterizedTest @ValueSource(strings = { "country matches \"[a-z]*\" || matches \"[A-Z]*\"", diff --git a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/accumulate_with_nested_from.drl b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/accumulate_with_nested_from.drl index 3696836b1f2..5911bcadfb4 100755 --- a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/accumulate_with_nested_from.drl +++ b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/accumulate_with_nested_from.drl @@ -18,7 +18,7 @@ rule "AccumulateParserTest" when // below statement makes no sense, but is useful to test parsing recursiveness - $personList : ArrayList() from accumulate( Person( $age : age > 21 || < 10 ) from collect( People() from $town.getPeople() ), + $personList : ArrayList() from accumulate( Person( $age : age > 21 || age < 10 ) from collect( People() from $town.getPeople() ), max( $age ) ); then end diff --git a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/in_operator_test.drl b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/in_operator_test.drl index 54f5225c3e8..7141a89d0d4 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/in_operator_test.drl +++ b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/in_operator_test.drl @@ -19,7 +19,7 @@ rule simple_rule when - Person(age > 30 && < 40) + Person(age > 30 && age < 40) Vehicle(type in ( "sedan", "wagon" ), age < 3) then consequence(); diff --git a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/notin_operator_test.drl b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/notin_operator_test.drl index a2eae254405..8b054e0a4fb 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/notin_operator_test.drl +++ b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/notin_operator_test.drl @@ -19,7 +19,7 @@ rule simple_rule when - Person(age > 30 && < 40) + Person(age > 30 && age < 40) Vehicle(type not in ( "sedan", "wagon" ), age < 3) then consequence(); diff --git a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/pluggable_operators.drl b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/pluggable_operators.drl index 59c31d2c8e7..a605648d783 100644 --- a/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/pluggable_operators.drl +++ b/drools-drl/drools-drl-parser-tests/src/test/resources/org/drools/drl/parser/antlr4/pluggable_operators.drl @@ -23,6 +23,6 @@ when $b : EventB( this after[1,10] $a || this not after[15,20] $a ) $c : EventC( this finishes $b ) $d : EventD( this not starts $a ) - $e : EventE( this not before[1, 10] $b || after[1, 10] $c && this after[1, 5] $d ) + $e : EventE( this not before[1, 10] $b || this after[1, 10] $c && this after[1, 5] $d ) then end \ No newline at end of file diff --git a/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL10Expressions.g4 b/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL10Expressions.g4 index 7a4c807fcb7..e5030c2362c 100644 --- a/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL10Expressions.g4 +++ b/drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL10Expressions.g4 @@ -571,7 +571,7 @@ locals [ BaseDescr lsd ] $relationalExpression::lsd = $result; } } - ( right=orRestriction + ( right=singleRestriction { if( buildDescr ) { $result = $right.result; // TODO access lsd directly instead of through dynamic context here @@ -581,36 +581,6 @@ locals [ BaseDescr lsd ] )* ; -orRestriction returns [BaseDescr result] - : left=andRestriction { if( buildDescr ) { $result = $left.result; } } - ( lop=OR args=fullAnnotation[null]? right=andRestriction - { if( buildDescr ) { - ConstraintConnectiveDescr descr = ConstraintConnectiveDescr.newOr(); - descr.addOrMerge( $result ); - descr.addOrMerge( $right.result ); - if ( $ctx.args != null ) { descr.addAnnotation( $args.result ); } - $result = descr; - } - } - )*? EOF? - ; - -andRestriction returns [BaseDescr result] - : left=singleRestriction { if( buildDescr ) { $result = $left.result; } } - ( lop=AND - { if ( isNotEOF() ) helper.emit( Location.LOCATION_LHS_INSIDE_CONDITION_OPERATOR ); } - args=fullAnnotation[null]?right=singleRestriction - { if( buildDescr ) { - ConstraintConnectiveDescr descr = ConstraintConnectiveDescr.newAnd(); - descr.addOrMerge( $result ); - descr.addOrMerge( $right.result ); - if ( $ctx.args != null ) { descr.addAnnotation( $args.result ); } - $result = descr; - } - } - )*? - ; - singleRestriction returns [BaseDescr result] : op=operator { helper.emit( Location.LOCATION_LHS_INSIDE_CONDITION_ARGUMENT ); } @@ -630,7 +600,6 @@ singleRestriction returns [BaseDescr result] } helper.emit( Location.LOCATION_LHS_INSIDE_CONDITION_END ); } - | LPAREN or=orRestriction RPAREN { $result = $or.result; } ; diff --git a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/BaseModelTest.java b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/BaseModelTest.java index 20e3bfe6141..873474c7a71 100644 --- a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/BaseModelTest.java +++ b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/BaseModelTest.java @@ -27,6 +27,7 @@ import org.drools.compiler.kie.builder.impl.InternalKieModule; import org.drools.core.reteoo.EntryPointNode; import org.drools.core.reteoo.ObjectTypeNode; +import org.drools.drl.parser.DrlParser; import org.drools.kiesession.rulebase.InternalKnowledgeBase; import org.drools.model.codegen.ExecutableModelProject; import org.kie.api.KieServices; @@ -212,4 +213,12 @@ protected ObjectTypeNode getObjectTypeNodeForClass( KieSession ksession, Class 30 && <= 40, $name : name )" + + " Person( age > 30 && age <= 40, $name : name )" + "then\n" + " list.add($name);" + "end "; @@ -152,7 +155,7 @@ public void testExcludedRange(RUN_TYPE runType) { "import " + Person.class.getCanonicalName() + "\n" + "global java.util.List list\n" + "rule R when\n" + - " Person( age <= 30 || > 40, $name : name )" + + " Person( age <= 30 || age > 40, $name : name )" + "then\n" + " list.add($name);" + "end "; diff --git a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/NullSafeDereferencingTest.java b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/NullSafeDereferencingTest.java index a442f36b28c..7fcc520c2cb 100644 --- a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/NullSafeDereferencingTest.java +++ b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/NullSafeDereferencingTest.java @@ -26,6 +26,7 @@ import org.drools.model.codegen.execmodel.domain.MysteriousMan; import org.drools.model.codegen.execmodel.domain.Person; import org.drools.model.codegen.execmodel.domain.Result; +import org.junit.jupiter.api.condition.EnabledIf; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.kie.api.runtime.KieSession; @@ -253,6 +254,7 @@ public void testNullSafeDereferncingOnSecondField(RUN_TYPE runType) { assertThat(results.get(0).getValue()).isEqualTo("John2"); } + @EnabledIf("isOldParser") @ParameterizedTest @MethodSource("parameters") public void testNullSafeDereferncingWithOrHalfBinary(RUN_TYPE runType) { diff --git a/drools-model/drools-model-codegen/src/test/resources/logback-test.xml b/drools-model/drools-model-codegen/src/test/resources/logback-test.xml index 25b6a1f4918..45cc9cf7290 100644 --- a/drools-model/drools-model-codegen/src/test/resources/logback-test.xml +++ b/drools-model/drools-model-codegen/src/test/resources/logback-test.xml @@ -36,7 +36,7 @@ - + diff --git a/drools-traits/src/test/java/org/drools/traits/compiler/factmodel/traits/TraitMapCoreTest.java b/drools-traits/src/test/java/org/drools/traits/compiler/factmodel/traits/TraitMapCoreTest.java index d4d9cae880c..e7c59734353 100644 --- a/drools-traits/src/test/java/org/drools/traits/compiler/factmodel/traits/TraitMapCoreTest.java +++ b/drools-traits/src/test/java/org/drools/traits/compiler/factmodel/traits/TraitMapCoreTest.java @@ -298,7 +298,7 @@ public void testMapCoreAliasing( ) { "\n" + "rule Log \n" + "when \n" + - " $p : PersonMap( name == \"john\", age > 10 && < 35 )\n" + + " $p : PersonMap( name == \"john\", age > 10 && age < 35 )\n" + "then \n" + " modify ( $p ) { \n" + " setHeight( 184.0 ), \n" + diff --git a/drools-verifier/drools-verifier-drl/pom.xml b/drools-verifier/drools-verifier-drl/pom.xml index 2b93a9c78b1..474178b3a6b 100644 --- a/drools-verifier/drools-verifier-drl/pom.xml +++ b/drools-verifier/drools-verifier-drl/pom.xml @@ -179,5 +179,26 @@ + + + enableNewParser + + + enableNewParser + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + true + + + + + diff --git a/drools-verifier/drools-verifier-drl/src/main/java/org/drools/verifier/builder/VerifierKnowledgeBaseBuilder.java b/drools-verifier/drools-verifier-drl/src/main/java/org/drools/verifier/builder/VerifierKnowledgeBaseBuilder.java index 10410deba94..528932959cb 100644 --- a/drools-verifier/drools-verifier-drl/src/main/java/org/drools/verifier/builder/VerifierKnowledgeBaseBuilder.java +++ b/drools-verifier/drools-verifier-drl/src/main/java/org/drools/verifier/builder/VerifierKnowledgeBaseBuilder.java @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.List; +import org.drools.compiler.builder.impl.KnowledgeBuilderImpl; import org.drools.kiesession.rulebase.InternalKnowledgeBase; import org.drools.kiesession.rulebase.KnowledgeBaseFactory; import org.drools.verifier.VerifierConfiguration; @@ -31,6 +32,7 @@ import org.kie.internal.builder.KnowledgeBuilder; import org.kie.internal.builder.KnowledgeBuilderError; import org.kie.internal.builder.KnowledgeBuilderFactory; +import org.kie.internal.builder.conf.LanguageLevelOption; public class VerifierKnowledgeBaseBuilder { @@ -41,6 +43,11 @@ public KieBase newVerifierKnowledgeBase(VerifierConfiguration configuration) { InternalKnowledgeBase verifierKnowledgeBase = KnowledgeBaseFactory.newKnowledgeBase(); KnowledgeBuilder kbuilder = KnowledgeBuilderFactory.newKnowledgeBuilder(); + LanguageLevelOption option = ((KnowledgeBuilderImpl) kbuilder).getBuilderConfiguration().getOption(LanguageLevelOption.KEY); + if (option == LanguageLevelOption.DRL10) { + // Temporarily verifier rules can be migrated to DRL10 (= fix half constraint). Need to be discussed. + throw new UnsupportedOperationException("Verifier does not support DRL10"); + } if ( configuration.getVerifyingResources() != null ) { for ( Resource resource : configuration.getVerifyingResources().keySet() ) {