From fe144bdc70a7fac7b2e6487de9372f7e22a8cf18 Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Wed, 19 Jun 2024 15:38:50 +0900 Subject: [PATCH 1/2] [RHDM-2029] Executable model doesn't report an error when duplicated rule name with "include" kbase --- .../kie/builder/impl/AbstractKieProject.java | 2 + .../kie/builder/impl/BuildContext.java | 11 ++ .../builder/CanonicalModelBuildContext.java | 13 +++ .../builder/ModelBuilderImpl.java | 53 +++++++++ .../test-compiler-integration/pom.xml | 7 +- .../integrationtests/KieBaseIncludesTest.java | 104 ++++++++++++++++++ 6 files changed, 189 insertions(+), 1 deletion(-) diff --git a/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/AbstractKieProject.java b/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/AbstractKieProject.java index e6f660ea1f5..70a2e145326 100644 --- a/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/AbstractKieProject.java +++ b/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/AbstractKieProject.java @@ -236,6 +236,8 @@ public KnowledgeBuilder buildKnowledgePackages( KieBaseModelImpl kBaseModel, Bui } if (compileIncludedKieBases()) { addFiles( buildFilter, assets, getKieBaseModel( include ), includeModule, useFolders ); + } else { + buildContext.addIncludeModule(include, includeModule); } } diff --git a/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/BuildContext.java b/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/BuildContext.java index dc8b1027605..b28c4584592 100644 --- a/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/BuildContext.java +++ b/drools-compiler/src/main/java/org/drools/compiler/kie/builder/impl/BuildContext.java @@ -15,6 +15,9 @@ package org.drools.compiler.kie.builder.impl; +import java.util.HashMap; +import java.util.Map; + public class BuildContext { private final ResultsImpl messages; @@ -33,4 +36,12 @@ public ResultsImpl getMessages() { public boolean registerResourceToBuild(String kBaseName, String resource) { return true; } + + public void addIncludeModule(String kBaseName, InternalKieModule includeModule) { + // no op + } + + public Map getIncludeModules() { + return new HashMap<>(); + } } diff --git a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/CanonicalModelBuildContext.java b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/CanonicalModelBuildContext.java index 1f95a928f2c..ead14730cfb 100644 --- a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/CanonicalModelBuildContext.java +++ b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/CanonicalModelBuildContext.java @@ -23,6 +23,7 @@ import java.util.Set; import org.drools.compiler.kie.builder.impl.BuildContext; +import org.drools.compiler.kie.builder.impl.InternalKieModule; import org.drools.compiler.kie.builder.impl.ResultsImpl; public class CanonicalModelBuildContext extends BuildContext { @@ -33,6 +34,8 @@ public class CanonicalModelBuildContext extends BuildContext { private final Collection allGeneratedPojos = new HashSet<>(); private final Map> allCompiledClasses = new HashMap<>(); + private final Map includeModules = new HashMap<>(); + public CanonicalModelBuildContext() { } public CanonicalModelBuildContext(ResultsImpl messages) { @@ -81,4 +84,14 @@ private String resource2Package(String resource) { int pathEndPos = resource.lastIndexOf('/'); return pathEndPos <= 0 ? "" :resource.substring(0, pathEndPos).replace('/', '.'); } + + @Override + public void addIncludeModule(String kBaseName, InternalKieModule includeModule) { + includeModules.put(kBaseName, includeModule); + } + + @Override + public Map getIncludeModules() { + return includeModules; + } } diff --git a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/ModelBuilderImpl.java b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/ModelBuilderImpl.java index b0fe1b541f5..64dbd33dd83 100644 --- a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/ModelBuilderImpl.java +++ b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/ModelBuilderImpl.java @@ -16,6 +16,7 @@ package org.drools.modelcompiler.builder; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -29,24 +30,35 @@ import org.drools.compiler.builder.impl.KnowledgeBuilderImpl; import org.drools.compiler.builder.impl.TypeDeclarationFactory; import org.drools.compiler.compiler.PackageRegistry; +import org.drools.compiler.compiler.ParserError; +import org.drools.compiler.kie.builder.impl.AbstractKieModule; import org.drools.compiler.kie.builder.impl.BuildContext; +import org.drools.compiler.kie.builder.impl.InternalKieModule; import org.drools.compiler.lang.descr.AbstractClassTypeDeclarationDescr; import org.drools.compiler.lang.descr.CompositePackageDescr; import org.drools.compiler.lang.descr.EnumDeclarationDescr; import org.drools.compiler.lang.descr.GlobalDescr; import org.drools.compiler.lang.descr.ImportDescr; import org.drools.compiler.lang.descr.PackageDescr; +import org.drools.compiler.lang.descr.RuleDescr; import org.drools.compiler.lang.descr.TypeDeclarationDescr; import org.drools.core.addon.TypeResolver; import org.drools.core.definitions.InternalKnowledgePackage; import org.drools.core.rule.ImportDeclaration; import org.drools.core.rule.TypeDeclaration; import org.drools.core.util.StringUtils; +import org.drools.modelcompiler.CanonicalKieModule; import org.drools.modelcompiler.builder.errors.UnsupportedFeatureError; import org.drools.modelcompiler.builder.generator.DRLIdGenerator; import org.drools.modelcompiler.builder.generator.DrlxParseUtil; import org.drools.modelcompiler.builder.generator.declaredtype.POJOGenerator; +import org.kie.api.KieBase; +import org.kie.api.KieServices; +import org.kie.api.builder.KieModule; +import org.kie.api.builder.KieRepository; import org.kie.api.builder.ReleaseId; +import org.kie.api.runtime.KieContainer; +import org.kie.internal.builder.KnowledgeBuilder; import org.kie.internal.builder.ResultSeverity; import static com.github.javaparser.StaticJavaParser.parseImport; @@ -303,4 +315,45 @@ public T getPackageSource(String packageName) { protected BuildContext createBuildContext() { return new CanonicalModelBuildContext(); } + + // Overridden, because exec-model doesn't add assets of included kbase to the packageDescr + @Override + protected void validateUniqueRuleNames(final PackageDescr packageDescr) { + super.validateUniqueRuleNames(packageDescr); + + // check for duplicated rule names in included kbase + Map includeModules = getBuildContext().getIncludeModules(); + List ruleNamesInIncludeKBases = new ArrayList<>(); + for (Map.Entry entry : includeModules.entrySet()) { + String kBaseName = entry.getKey(); + InternalKieModule includeModule = entry.getValue(); + if (!(includeModule instanceof CanonicalKieModule)) { + continue; + } + CanonicalKieModule canonicalKieModule = (CanonicalKieModule) includeModule; + InternalKieModule internalKieModule = canonicalKieModule.getInternalKieModule(); + if (!(internalKieModule instanceof AbstractKieModule)) { + continue; + } + AbstractKieModule abstractKieModule = (AbstractKieModule) internalKieModule; + ReleaseId internalReleaseId = abstractKieModule.getReleaseId(); + KieServices ks = KieServices.Factory.get(); + KieContainer kieContainer = ks.newKieContainer(internalReleaseId); + KieBase kieBase = kieContainer.getKieBase(kBaseName); + // add rule names to ruleNamesInIncludeKBases + kieBase.getKiePackages().stream() + .filter(kPkg -> kPkg.getName().equals(packageDescr.getNamespace())) + .flatMap(kPkg -> kPkg.getRules().stream()) + .forEach(rule -> ruleNamesInIncludeKBases.add(rule.getName())); + } + for (final RuleDescr ruleDescr : packageDescr.getRules()) { + if (ruleNamesInIncludeKBases.contains(ruleDescr.getName())) { + addBuilderResult(new ParserError(ruleDescr.getResource(), + "Duplicate rule name: " + ruleDescr.getName(), + ruleDescr.getLine(), + ruleDescr.getColumn(), + packageDescr.getNamespace())); + } + } + } } diff --git a/drools-test-coverage/test-compiler-integration/pom.xml b/drools-test-coverage/test-compiler-integration/pom.xml index a6d5c7d90e4..806b27e721c 100644 --- a/drools-test-coverage/test-compiler-integration/pom.xml +++ b/drools-test-coverage/test-compiler-integration/pom.xml @@ -157,7 +157,12 @@ hamcrest-core - + + + org.kie + kie-ci + test + diff --git a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java index 44f859eaa54..d31c3906ecf 100644 --- a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java +++ b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java @@ -16,21 +16,31 @@ package org.drools.mvel.integrationtests; +import java.io.IOException; import java.util.Collection; +import java.util.List; +import org.drools.compiler.kie.builder.impl.InternalKieModule; +import org.drools.core.util.FileManager; import org.drools.testcoverage.common.util.KieBaseTestConfiguration; import org.drools.testcoverage.common.util.KieUtil; +import org.drools.testcoverage.common.util.MavenUtil; import org.drools.testcoverage.common.util.TestParametersUtil; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.kie.api.KieBase; import org.kie.api.KieServices; +import org.kie.api.builder.KieBuilder; import org.kie.api.builder.KieFileSystem; +import org.kie.api.builder.Message; import org.kie.api.builder.ReleaseId; import org.kie.api.definition.KiePackage; import org.kie.api.definition.rule.Rule; import org.kie.api.runtime.KieContainer; +import org.kie.scanner.KieMavenRepository; import static org.assertj.core.api.Assertions.assertThat; @@ -40,6 +50,19 @@ public class KieBaseIncludesTest { private final KieBaseTestConfiguration kieBaseTestConfiguration; + private FileManager fileManager; + + @Before + public void setUp() throws Exception { + this.fileManager = new FileManager(); + this.fileManager.setUp(); + } + + @After + public void tearDown() throws Exception { + this.fileManager.tearDown(); + } + public KieBaseIncludesTest(final KieBaseTestConfiguration kieBaseTestConfiguration) { this.kieBaseTestConfiguration = kieBaseTestConfiguration; } @@ -241,4 +264,85 @@ private static long getNumberOfRules(KieBase kieBase) { } return nrOfRules; } + + /** + * Test the inclusion of a KieBase defined in one KJAR into the KieBase of another KJAR. + *

+ * The 2 KieBases use the duplicate rule names, so an error should be reported + */ + @Test + public void kieBaseIncludesCrossKJarDuplicateRuleNames_shouldReportError() throws IOException { + + String pomContentMain = "\n" + + "4.0.0\n" + + "org.kie\n" + + "rules-main\n" + + "1.0.0\n" + + "jar\n" + + "\n" + + "\n" + + "org.kie\n" + + "rules-sub\n" + + "1.0.0\n" + + "\n" + + "\n" + + "\n"; + + String kmoduleContentMain = "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + ""; + + String drlMain = "package rules\n" + + "\n" + + "rule \"RuleA\"\n" + + "when\n" + + "then\n" + + "System.out.println(\"Rule in KieBaseMain\");\n" + + "end"; + + String kmoduleContentSub = "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + ""; + + String drlSub = "package rules\n" + + "\n" + + "rule \"RuleA\"\n" + + "when\n" + + "then\n" + + "System.out.println(\"Rule in KieBaseSub\");\n" + + "end"; + + KieServices ks = KieServices.Factory.get(); + ReleaseId releaseIdSub = ks.newReleaseId("org.kie", "rules-sub", "1.0.0"); + + //First deploy the second KJAR on which the first one depends. + KieFileSystem kfsSub = ks.newKieFileSystem() + .generateAndWritePomXML(releaseIdSub) + .write("src/main/resources/rules/rules.drl", drlSub) + .writeKModuleXML(kmoduleContentSub); + + KieBuilder kieBuilderSub = KieUtil.getKieBuilderFromKieFileSystem(kieBaseTestConfiguration, kfsSub, true); + + // install "rules-sub" into maven local repository, remove it from in-memory KieRepository. Simulating kie-maven-plugin use case + final KieMavenRepository repository = KieMavenRepository.getKieMavenRepository(); + repository.installArtifact(releaseIdSub, (InternalKieModule) kieBuilderSub.getKieModule(), MavenUtil.createPomXml(fileManager, releaseIdSub)); + ks.getRepository().removeKieModule(releaseIdSub); + + KieFileSystem kfsMain = ks.newKieFileSystem() + .writePomXML(pomContentMain) + .write("src/main/resources/rules/rules.drl", drlMain) + .writeKModuleXML(kmoduleContentMain); + + KieBuilder kieBuilderMain = KieUtil.getKieBuilderFromKieFileSystem(kieBaseTestConfiguration, kfsMain, false); + List messages = kieBuilderMain.getResults().getMessages(Message.Level.ERROR); + + assertThat(messages).as("Duplication error should be reported") + .extracting(Message::getText).anyMatch(text -> text.contains("Duplicate rule name")); + } } From b6bffa6f95189ae849ff16c9e11f058c692a461c Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Wed, 19 Jun 2024 16:55:17 +0900 Subject: [PATCH 2/2] removing kie-ci from dependency, because it causes a test failure in KieBaseIncludeTest --- drools-test-coverage/test-compiler-integration/pom.xml | 5 ----- .../mvel/integrationtests/KieBaseIncludesTest.java | 10 +--------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/drools-test-coverage/test-compiler-integration/pom.xml b/drools-test-coverage/test-compiler-integration/pom.xml index 806b27e721c..d166cf7e8a1 100644 --- a/drools-test-coverage/test-compiler-integration/pom.xml +++ b/drools-test-coverage/test-compiler-integration/pom.xml @@ -158,11 +158,6 @@ - - org.kie - kie-ci - test - diff --git a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java index d31c3906ecf..f7059304717 100644 --- a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java +++ b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/integrationtests/KieBaseIncludesTest.java @@ -20,11 +20,9 @@ import java.util.Collection; import java.util.List; -import org.drools.compiler.kie.builder.impl.InternalKieModule; import org.drools.core.util.FileManager; import org.drools.testcoverage.common.util.KieBaseTestConfiguration; import org.drools.testcoverage.common.util.KieUtil; -import org.drools.testcoverage.common.util.MavenUtil; import org.drools.testcoverage.common.util.TestParametersUtil; import org.junit.After; import org.junit.Before; @@ -40,7 +38,6 @@ import org.kie.api.definition.KiePackage; import org.kie.api.definition.rule.Rule; import org.kie.api.runtime.KieContainer; -import org.kie.scanner.KieMavenRepository; import static org.assertj.core.api.Assertions.assertThat; @@ -327,12 +324,7 @@ public void kieBaseIncludesCrossKJarDuplicateRuleNames_shouldReportError() throw .write("src/main/resources/rules/rules.drl", drlSub) .writeKModuleXML(kmoduleContentSub); - KieBuilder kieBuilderSub = KieUtil.getKieBuilderFromKieFileSystem(kieBaseTestConfiguration, kfsSub, true); - - // install "rules-sub" into maven local repository, remove it from in-memory KieRepository. Simulating kie-maven-plugin use case - final KieMavenRepository repository = KieMavenRepository.getKieMavenRepository(); - repository.installArtifact(releaseIdSub, (InternalKieModule) kieBuilderSub.getKieModule(), MavenUtil.createPomXml(fileManager, releaseIdSub)); - ks.getRepository().removeKieModule(releaseIdSub); + KieUtil.getKieBuilderFromKieFileSystem(kieBaseTestConfiguration, kfsSub, true); KieFileSystem kfsMain = ks.newKieFileSystem() .writePomXML(pomContentMain)