Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[incubator-kie-drools-6007] Executable model doesn't report an error … #6013

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ public KnowledgeBuilder buildKnowledgePackages( KieBaseModelImpl kBaseModel, Bui

Set<Asset> assets = new LinkedHashSet<>();

InternalKieModule kModule = getKieModuleForKBase(kBaseModel.getName());

boolean allIncludesAreValid = true;
for (String include : getTransitiveIncludes(kBaseModel)) {
if ( StringUtils.isEmpty( include )) {
Expand All @@ -240,14 +242,18 @@ public KnowledgeBuilder buildKnowledgePackages( KieBaseModelImpl kBaseModel, Bui
}
if (compileIncludedKieBases()) {
addFiles( buildFilter, assets, getKieBaseModel( include ), includeModule, useFolders );
} else {
if (kModule != includeModule) {
// includeModule is not part of the current kModule
buildContext.addIncludeModule(getKieBaseModel(include), includeModule);
}
}
}

if (!allIncludesAreValid) {
return null;
}

InternalKieModule kModule = getKieModuleForKBase(kBaseModel.getName());
addFiles( buildFilter, assets, kBaseModel, kModule, useFolders );

KnowledgeBuilder kbuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
*/
package org.drools.compiler.kie.builder.impl;

import java.util.Collections;
import java.util.Map;

import org.kie.api.builder.model.KieBaseModel;

public class BuildContext {
private final ResultsImpl messages;

Expand All @@ -36,4 +41,12 @@ public ResultsImpl getMessages() {
public boolean registerResourceToBuild(String kBaseName, String resource) {
return true;
}

public void addIncludeModule(KieBaseModel kieBaseModel, InternalKieModule includeModule) {
// no op
}

public Map<KieBaseModel, InternalKieModule> getIncludeModules() {
return Collections.emptyMap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
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;
import org.kie.api.builder.model.KieBaseModel;

public class CanonicalModelBuildContext extends BuildContext {

Expand All @@ -36,6 +38,8 @@ public class CanonicalModelBuildContext extends BuildContext {
private final Collection<GeneratedClassWithPackage> allGeneratedPojos = new HashSet<>();
private final Map<String, Class<?>> allCompiledClasses = new HashMap<>();

private final Map<KieBaseModel, InternalKieModule> includeModules = new HashMap<>();

public CanonicalModelBuildContext() { }

public CanonicalModelBuildContext(ResultsImpl messages) {
Expand Down Expand Up @@ -84,4 +88,14 @@ private String resource2Package(String resource) {
int pathEndPos = resource.lastIndexOf('/');
return pathEndPos <= 0 ? "" :resource.substring(0, pathEndPos).replace('/', '.');
}

@Override
public void addIncludeModule(KieBaseModel kieBaseModel, InternalKieModule includeModule) {
includeModules.put(kieBaseModel, includeModule);
}

@Override
public Map<KieBaseModel, InternalKieModule> getIncludeModules() {
return includeModules;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ protected void doSecondBuildStep(Collection<CompositePackageDescr> compositePack
this.getGlobalVariableContext(),
this.sourcesGenerator,
this.packageSources,
oneClassPerRule));
oneClassPerRule,
this.getBuildContext()));

for (CompilationPhase phase : phases) {
phase.process();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

import org.drools.compiler.builder.PackageRegistryManager;
Expand All @@ -33,9 +36,9 @@
import org.drools.compiler.builder.impl.processors.FunctionCompilationPhase;
import org.drools.compiler.builder.impl.processors.GlobalCompilationPhase;
import org.drools.compiler.builder.impl.processors.IteratingPhase;
import org.drools.compiler.builder.impl.processors.RuleValidator;
import org.drools.compiler.builder.impl.processors.SinglePackagePhaseFactory;
import org.drools.compiler.builder.impl.processors.WindowDeclarationCompilationPhase;
import org.drools.compiler.kie.builder.impl.BuildContext;
import org.drools.compiler.lang.descr.CompositePackageDescr;
import org.drools.kiesession.rulebase.InternalKnowledgeBase;
import org.drools.model.codegen.execmodel.PackageModel;
Expand All @@ -61,6 +64,8 @@ public class ModelMainCompilationPhase<T> implements CompilationPhase {
private final PackageSourceManager<T> packageSourceManager;
private final boolean oneClassPerRule;

private final BuildContext buildContext;

public ModelMainCompilationPhase(
PackageModelManager packageModels,
PackageRegistryManager pkgRegistryManager,
Expand All @@ -69,7 +74,11 @@ public ModelMainCompilationPhase(
boolean hasMvel,
InternalKnowledgeBase kBase,
TypeDeclarationContext typeDeclarationContext,
GlobalVariableContext globalVariableContext, Function<PackageModel, T> sourceGenerator, PackageSourceManager<T> packageSourceManager, boolean oneClassPerRule) {
GlobalVariableContext globalVariableContext,
Function<PackageModel, T> sourceGenerator,
PackageSourceManager<T> packageSourceManager,
boolean oneClassPerRule,
BuildContext buildContext) {
this.packageModels = packageModels;
this.pkgRegistryManager = pkgRegistryManager;
this.packages = packages;
Expand All @@ -81,6 +90,7 @@ public ModelMainCompilationPhase(
this.sourceGenerator = sourceGenerator;
this.packageSourceManager = packageSourceManager;
this.oneClassPerRule = oneClassPerRule;
this.buildContext = buildContext;
}

@Override
Expand All @@ -94,7 +104,9 @@ public void process() {
phases.add(iteratingPhase((reg, acc) -> GlobalCompilationPhase.of(reg, acc, kBase, globalVariableContext, acc.getFilter())));
phases.add(new DeclaredTypeDeregistrationPhase(packages, pkgRegistryManager));

phases.add(iteratingPhase((reg, acc) -> new RuleValidator(reg, acc, configuration))); // validateUniqueRuleNames
Map<String, Set<String>> includedRuleNameMap = new HashMap<>();
phases.add(new PopulateIncludedRuleNameMapPhase(buildContext.getIncludeModules(), includedRuleNameMap));
phases.add(iteratingPhase((reg, acc) -> new ModelRuleValidator(reg, acc, configuration, includedRuleNameMap))); // validateUniqueRuleNames
phases.add(iteratingPhase((reg, acc) -> new ModelGeneratorPhase(reg, acc, packageModels.getPackageModel(acc, reg, acc.getName()), typeDeclarationContext))); // validateUniqueRuleNames
phases.add(iteratingPhase((reg, acc) -> new SourceCodeGenerationPhase<>(
packageModels.getPackageModel(acc, reg, acc.getName()), packageSourceManager, sourceGenerator, oneClassPerRule))); // validateUniqueRuleNames
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* 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.model.codegen.execmodel.processors;

import java.util.Map;
import java.util.Set;

import org.drools.compiler.builder.impl.processors.RuleValidator;
import org.drools.compiler.compiler.PackageRegistry;
import org.drools.drl.ast.descr.PackageDescr;
import org.drools.drl.ast.descr.RuleDescr;
import org.drools.drl.parser.ParserError;
import org.kie.internal.builder.KnowledgeBuilderConfiguration;

public class ModelRuleValidator extends RuleValidator {

// extra rule names which need to be checked for duplicates.
// Non-executable model doesn't need this because included kbase assets are added to the packageDescr
// Executable model requires this because included kbase assets are modeled as separated kjar, so not added to the packageDescr
private final Map<String, Set<String>> includedRuleNameMap;

public ModelRuleValidator(PackageRegistry pkgRegistry, PackageDescr packageDescr, KnowledgeBuilderConfiguration configuration, Map<String, Set<String>> includedRuleNameMap) {
super(pkgRegistry, packageDescr, configuration);
this.includedRuleNameMap = includedRuleNameMap;
}

@Override
public void process() {
super.process();

// Check with included rule names, because exec-model doesn't add assets of included kbase to the packageDescr
String packageName = packageDescr.getNamespace();
if (includedRuleNameMap.containsKey(packageName)) {
Set<String> ruleNames = includedRuleNameMap.get(packageName);
for (final RuleDescr rule : packageDescr.getRules()) {
final String name = rule.getUnitQualifiedName();
if (ruleNames.contains(name)) {
this.results.add(new ParserError(rule.getResource(),
"Duplicate rule name: " + name,
rule.getLine(),
rule.getColumn(),
packageName));
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* 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.model.codegen.execmodel.processors;

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import org.drools.compiler.builder.impl.processors.CompilationPhase;
import org.drools.compiler.kie.builder.impl.InternalKieModule;
import org.drools.compiler.kproject.models.KieBaseModelImpl;
import org.drools.model.Model;
import org.drools.modelcompiler.CanonicalKieModule;
import org.kie.api.builder.model.KieBaseModel;
import org.kie.internal.builder.KnowledgeBuilderResult;

public class PopulateIncludedRuleNameMapPhase implements CompilationPhase {

private final Map<KieBaseModel, InternalKieModule> includeModules;
private final Map<String, Set<String>> includedRuleNameMap;

public PopulateIncludedRuleNameMapPhase(Map<KieBaseModel, InternalKieModule> includeModules, Map<String, Set<String>> includedRuleNameMap) {
this.includeModules = includeModules;
this.includedRuleNameMap = includedRuleNameMap;
}

@Override
public void process() {
for (Map.Entry<KieBaseModel, InternalKieModule> entry : includeModules.entrySet()) {
KieBaseModel kieBaseModel = entry.getKey();
InternalKieModule includeModule = entry.getValue();
if ((includeModule instanceof CanonicalKieModule canonicalKieModule) && canonicalKieModule.hasModelFile()) {
Collection<Model> includeModels = canonicalKieModule.getModelForKBase((KieBaseModelImpl)kieBaseModel);
for (Model includeModel : includeModels) {
includeModel.getRules().forEach(rule -> includedRuleNameMap.computeIfAbsent(includeModel.getPackageName(), k -> new HashSet<>()).add(rule.getName()));
}
}
}
}

@Override
public Collection<? extends KnowledgeBuilderResult> getResults() {
return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ private Collection<String> getRuleClassNames() {
return ruleClassesNames;
}

private Collection<Model> getModelForKBase(KieBaseModelImpl kBaseModel) {
public Collection<Model> getModelForKBase(KieBaseModelImpl kBaseModel) {
Map<String, Model> modelsMap = getModels();
if (kBaseModel.getPackages().isEmpty()) {
return modelsMap.values();
Expand All @@ -484,6 +484,11 @@ private Collection<Model> getModelForKBase(KieBaseModelImpl kBaseModel) {
return models;
}

// This method indicates if the kjar was already compiled with the executable model
public boolean hasModelFile() {
return resourceFileExists(getModelFileWithGAV(internalKieModule.getReleaseId()));
}

private Collection<String> findRuleClassesNames() {
ReleaseId releaseId = internalKieModule.getReleaseId();
String modelFiles = readExistingResourceWithName(getModelFileWithGAV(releaseId));
Expand Down
Loading
Loading