Skip to content

Commit

Permalink
[Incubator-kie-issues#1356] DMN Elements with null IDs are not correc…
Browse files Browse the repository at this point in the history
…tly managed (#6033)

* [incubator-kie-issues#1356] Implemented TupleIdentifier as key for nods maps inside DMNModelImpl

* [incubator-kie-issues#1356] Working with tests

* [incubator-kie-issues#1356] Fix null management inside TupleIdentifier. Add  tests.

* [incubator-kie-issues#1356] Add symmetrical equality tests for TupleIdentifier

---------

Co-authored-by: Gabriele-Cardosi <[email protected]>
  • Loading branch information
gitgabrio and Gabriele-Cardosi authored Aug 1, 2024
1 parent e5c9de4 commit fed30f8
Show file tree
Hide file tree
Showing 22 changed files with 667 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* "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
* 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
Expand All @@ -24,6 +24,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;

import javax.xml.namespace.QName;

Expand All @@ -45,17 +46,21 @@ public abstract class DMNBaseNode implements DMNNode {

private Map<String, QName> importAliases = new HashMap<>();

private final String id;

public DMNBaseNode() {
id = UUID.randomUUID().toString();
}

public DMNBaseNode(NamedElement source) {
this.source = source;
id = source != null && source.getId() != null ? source.getId() : UUID.randomUUID().toString();
}

public abstract DMNType getType();

public String getId() {
return source != null ? source.getId() : null;
return id;
}

public String getName() {
Expand Down Expand Up @@ -88,7 +93,7 @@ public String getModelName() {

public String getIdentifierString() {
String identifier = "[unnamed]";
if( source != null ) {
if (source != null) {
identifier = source.getName() != null ? source.getName() : source.getId();
}
return identifier;
Expand All @@ -107,21 +112,21 @@ public void setDependencies(Map<String, DMNNode> dependencies) {
}

public void addDependency(String name, DMNNode dependency) {
this.dependencies.put( name, dependency );
this.dependencies.put(name, dependency);
}

public List<InformationRequirement> getInformationRequirement() {
if ( source instanceof Decision ) {
if (source instanceof Decision) {
return ((Decision) source).getInformationRequirement();
} else {
return Collections.emptyList();
}
}

public List<KnowledgeRequirement> getKnowledgeRequirement() {
if ( source instanceof Decision ) {
if (source instanceof Decision) {
return ((Decision) source).getKnowledgeRequirement();
} else if( source instanceof BusinessKnowledgeModel ) {
} else if (source instanceof BusinessKnowledgeModel) {
return ((BusinessKnowledgeModel) source).getKnowledgeRequirement();
} else {
return Collections.emptyList();
Expand All @@ -148,5 +153,4 @@ public String toString() {
builder.append("]");
return builder.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ public void setItemDef(ItemDefinition itemDef) {
this.itemDef = itemDef;
}

public String getId() {
return itemDef.getId();
}

public String getName() {
return itemDef.getName();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ private void processItemDefinitions(DMNCompilerContext ctx, DMNModelImpl model,
Msg.DUPLICATED_ITEM_DEFINITION,
id.getName());
}
if (id.getItemComponent() != null && id.getItemComponent().size() > 0) {
if (id.getItemComponent() != null && !id.getItemComponent().isEmpty()) {
DMNCompilerHelper.checkVariableName(model, id, id.getName());
CompositeTypeImpl compType = new CompositeTypeImpl(model.getNamespace(), id.getName(), id.getId(), id.isIsCollection());
DMNType preregistered = model.getTypeRegistry().registerType(compType);
Expand All @@ -402,6 +402,9 @@ private void processItemDefinitions(DMNCompilerContext ctx, DMNModelImpl model,
private void processDrgElements(DMNCompilerContext ctx, DMNModelImpl model, Definitions dmndefs) {
for ( DRGElement e : dmndefs.getDrgElement() ) {
boolean foundIt = false;
if (e.getId() == null) {
e.setId(UUID.randomUUID().toString());
}
for( DRGElementCompiler dc : drgCompilers ) {
if ( dc.accept( e ) ) {
foundIt = true;
Expand Down Expand Up @@ -436,10 +439,8 @@ private void processDrgElements(DMNCompilerContext ctx, DMNModelImpl model, Defi
ds.setVariable(variable);
}
// continuing with normal compilation of Decision Service:
boolean foundIt = false;
for (DRGElementCompiler dc : drgCompilers) {
if (dc.accept(ds)) {
foundIt = true;
dc.compileNode(ds, this, model);
}
}
Expand Down Expand Up @@ -487,7 +488,7 @@ private void processDrgElements(DMNCompilerContext ctx, DMNModelImpl model, Defi
}

@FunctionalInterface
public static interface AfterProcessDrgElements {
public interface AfterProcessDrgElements {
void callback(DMNCompilerImpl compiler, DMNCompilerContext ctx, DMNModelImpl model);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.kie.dmn.core.impl.BaseDMNTypeImpl;
import org.kie.dmn.core.impl.CompositeTypeImpl;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.feel.lang.Scope;
import org.kie.dmn.feel.lang.Type;
import org.kie.dmn.feel.lang.types.BuiltInType;
Expand All @@ -41,12 +42,12 @@
public abstract class DMNTypeRegistryAbstract implements DMNTypeRegistry, FEELTypeRegistry {

protected Map<String, Map<String, DMNType>> types = new HashMap<>();
protected Map<String, QName> aliases;
protected Map<TupleIdentifier, QName> aliases;
protected ScopeImpl feelTypesScope = new ScopeImpl(); // no parent scope, intentional.
protected Map<String, ScopeImpl> feelTypesScopeChildLU = new HashMap<>();
protected Map<TupleIdentifier, ScopeImpl> feelTypesScopeChildLU = new HashMap<>();


public DMNTypeRegistryAbstract(Map<String, QName> aliases) {
public DMNTypeRegistryAbstract(Map<TupleIdentifier, QName> aliases) {
this.aliases = aliases;
String feelNamespace = feelNS();
Map<String, DMNType> feelTypes = new HashMap<>( );
Expand Down Expand Up @@ -85,10 +86,10 @@ public Scope getItemDefScope(Scope parent) {
public Type resolveFEELType(List<String> qns) {
if (qns.size() == 1) {
return feelTypesScope.resolve(qns.get(0)).getType();
} else if (qns.size() == 2 && feelTypesScopeChildLU.containsKey(qns.get(0))) {
return feelTypesScopeChildLU.get(qns.get(0)).resolve(qns.get(1)).getType();
} else if (qns.size() == 2 && feelTypesScopeChildLU.containsKey(new TupleIdentifier(null, qns.get(0)))) {
return feelTypesScopeChildLU.get(new TupleIdentifier(null, qns.get(0))).resolve(qns.get(1)).getType();
} else {
throw new IllegalStateException("Inconsistent state when resolving for qns: " + qns.toString());
throw new IllegalStateException("Inconsistent state when resolving for qns: " + qns);
}
}

Expand All @@ -98,21 +99,21 @@ public Map<String, Map<String, DMNType>> getTypes() {
}

protected void registerAsFEELType(DMNType dmnType) {
Optional<String> optAliasKey = keyfromNS(dmnType.getNamespace());
Optional<TupleIdentifier> optAliasKey = keyfromNS(dmnType.getNamespace());
Type feelType = ((BaseDMNTypeImpl) dmnType).getFeelType();
if (optAliasKey.isEmpty()) {
feelTypesScope.define(new TypeSymbol(dmnType.getName(), feelType));
} else {
String aliasKey = optAliasKey.get();
TupleIdentifier aliasKey = optAliasKey.get();
feelTypesScopeChildLU.computeIfAbsent(aliasKey, k -> {
ScopeImpl importScope = new ScopeImpl(k, feelTypesScope);
feelTypesScope.define(new TypeSymbol(k, null));
ScopeImpl importScope = new ScopeImpl(k.getName(), feelTypesScope);
feelTypesScope.define(new TypeSymbol(k.getName(), null));
return importScope;
}).define(new TypeSymbol(dmnType.getName(), feelType));
}
}

private Optional<String> keyfromNS(String ns) {
private Optional<TupleIdentifier> keyfromNS(String ns) {
return aliases == null ? Optional.empty() : aliases.entrySet().stream().filter(kv -> kv.getValue().getNamespaceURI().equals(ns)).map(kv -> kv.getKey()).findFirst();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@

import org.kie.dmn.api.core.DMNType;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.feel.lang.types.BuiltInType;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.model.v1_1.KieDMNModelInstrumentedBase;

public class DMNTypeRegistryV11 extends DMNTypeRegistryAbstract {

public DMNTypeRegistryV11(Map<String, QName> aliases) {
public DMNTypeRegistryV11(Map<TupleIdentifier, QName> aliases) {
super(aliases);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import org.kie.dmn.api.core.DMNType;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.feel.lang.types.BuiltInType;
import org.kie.dmn.model.v1_2.KieDMNModelInstrumentedBase;

Expand All @@ -38,7 +39,7 @@ public DMNTypeRegistryV12() {
super(Collections.emptyMap());
}

public DMNTypeRegistryV12(Map<String, QName> aliases) {
public DMNTypeRegistryV12(Map<TupleIdentifier, QName> aliases) {
super(aliases);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@

import org.kie.dmn.api.core.DMNType;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.model.v1_3.KieDMNModelInstrumentedBase;

public class DMNTypeRegistryV13 extends DMNTypeRegistryAbstract {

private static final DMNType UNKNOWN = SimpleTypeImpl.UNKNOWN_DMNTYPE(KieDMNModelInstrumentedBase.URI_FEEL);

public DMNTypeRegistryV13(Map<String, QName> aliases) {
public DMNTypeRegistryV13(Map<TupleIdentifier, QName> aliases) {
super(aliases);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@

import org.kie.dmn.api.core.DMNType;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.model.v1_4.KieDMNModelInstrumentedBase;

public class DMNTypeRegistryV14 extends DMNTypeRegistryAbstract {

private static final DMNType UNKNOWN = SimpleTypeImpl.UNKNOWN_DMNTYPE(KieDMNModelInstrumentedBase.URI_FEEL);

public DMNTypeRegistryV14(Map<String, QName> aliases) {
public DMNTypeRegistryV14(Map<TupleIdentifier, QName> aliases) {
super(aliases);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.kie.dmn.api.core.DMNType;
import org.kie.dmn.core.impl.SimpleTypeImpl;
import org.kie.dmn.core.impl.TupleIdentifier;
import org.kie.dmn.model.v1_5.KieDMNModelInstrumentedBase;

import javax.xml.namespace.QName;
Expand All @@ -30,7 +31,7 @@ public class DMNTypeRegistryV15 extends DMNTypeRegistryAbstract {
private static final DMNType UNKNOWN = SimpleTypeImpl.UNKNOWN_DMNTYPE(KieDMNModelInstrumentedBase.URI_FEEL);


public DMNTypeRegistryV15(Map<String, QName> aliases) {
public DMNTypeRegistryV15(Map<TupleIdentifier, QName> aliases) {
super(aliases);
}

Expand Down
Loading

0 comments on commit fed30f8

Please sign in to comment.