This is an automated email from the ASF dual-hosted git repository. rantunes pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/incubator-kie-tools-temporary-rnd-do-not-use.git
commit c40917a0632ceba4da41a2ba45a1c6779cf62b59 Author: Toni Rikkola <[email protected]> AuthorDate: Tue Dec 5 15:16:04 2023 +0200 KOGITO-9964: Yard Validation: Masked rows check the mask wrong way around (#2068) --- .../src/main/java/org/yard/validator/Parser.java | 14 ++++--- .../org/yard/validator/checks/CheckProducer.java | 28 +++----------- .../yard/validator/checks/SubsumptionCheck.java | 28 +++++++------- .../main/java/org/yard/validator/checks/Util.java | 45 ++++++++++++++++++++++ .../java/org/yard/validator/YardValidatorTest.java | 45 +++++++++++++++++++++- .../org/yard/validator/package-prices.yml | 30 +++++++++++++++ .../org/yard/validator/when-then-redundancy.yml | 25 ++++++++++++ 7 files changed, 171 insertions(+), 44 deletions(-) diff --git a/packages/yard-validator-worker/src/main/java/org/yard/validator/Parser.java b/packages/yard-validator-worker/src/main/java/org/yard/validator/Parser.java index 6926262542..36093dcbd5 100644 --- a/packages/yard-validator-worker/src/main/java/org/yard/validator/Parser.java +++ b/packages/yard-validator-worker/src/main/java/org/yard/validator/Parser.java @@ -38,8 +38,7 @@ public class Parser { Logger.log("YaRD model has been read."); Logger.log("YaRD model name is " + model.getName()); - final TreeMap<RowLocation, CustomTreeSet> result - = visit(findTableStartRow(yaml), model); + final TreeMap<RowLocation, CustomTreeSet> result = visit(findTableStartRow(yaml), model); return new ParserResult(result, hitPolicy.toUpperCase()); } catch (Exception e) { @@ -72,16 +71,19 @@ public class Parser { hitPolicy = dt.getHitPolicy(); final List<Rule> rules = dt.getRules(); - for (Rule rule : rules) { - final RowLocation location = new RowLocation( - rule.getRowNumber(), - rule.getRowNumber() + rulesRow); + for (final Rule rule : rules) { if (rule instanceof WhenThenRule) { + final RowLocation location = new RowLocation( + rule.getRowNumber(), + rule.getRowNumber() * 2 - 1 + rulesRow); final CustomTreeSet keys = getWhenThenKeys(dt, (WhenThenRule) rule, location); if (!keys.isEmpty()) { result.put(location, keys); } } else if (rule instanceof InlineRule) { + final RowLocation location = new RowLocation( + rule.getRowNumber(), + rule.getRowNumber() + rulesRow); final CustomTreeSet keys = getInlineRuleKeys(dt, (InlineRule) rule, location); if (!keys.isEmpty()) { result.put(location, keys); diff --git a/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/CheckProducer.java b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/CheckProducer.java index 876d9900b8..1a42ffa88d 100644 --- a/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/CheckProducer.java +++ b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/CheckProducer.java @@ -28,6 +28,9 @@ import org.yard.validator.util.Logger; import java.util.*; import java.util.stream.Collectors; +import static org.yard.validator.checks.Util.getHigherInTable; +import static org.yard.validator.checks.Util.getLowerInTable; + public class CheckProducer { public static List<Check> getChecks(final ParserResult parse) { @@ -91,8 +94,8 @@ public class CheckProducer { if (i == j) { continue; } - final RowLocation locationA = getHigher(locations, i, j); - final RowLocation locationB = getLower(locations, i, j); + final RowLocation locationA = getHigherInTable(locations, i, j); + final RowLocation locationB = getLowerInTable(locations, i, j); final CheckItem checkItemA = new CheckItem( locationA.getTableRowNumber(), @@ -112,25 +115,4 @@ public class CheckProducer { return result; } - private static RowLocation getHigher( - final RowLocation[] locations, - final int i, - final int j) { - if (locations[i].getTableRowNumber() > locations[j].getTableRowNumber()) { - return locations[i]; - } else { - return locations[j]; - } - } - - private static RowLocation getLower( - final RowLocation[] locations, - final int i, - final int j) { - if (locations[i].getTableRowNumber() < locations[j].getTableRowNumber()) { - return locations[i]; - } else { - return locations[j]; - } - } } diff --git a/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/SubsumptionCheck.java b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/SubsumptionCheck.java index 01d2b43e2d..6eb39a263a 100644 --- a/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/SubsumptionCheck.java +++ b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/SubsumptionCheck.java @@ -32,43 +32,43 @@ public class SubsumptionCheck implements Check { private final String hitPolicy; - private final CheckItem checkItemA; - private final CheckItem checkItemB; + private final CheckItem higherCheckItem; + private final CheckItem lowerCheckItem; public SubsumptionCheck( final String hitPolicy, - final CheckItem checkItemA, - final CheckItem checkItemB) { + final CheckItem higherCheckItem, + final CheckItem lowerCheckItem) { this.hitPolicy = hitPolicy; - this.checkItemA = checkItemA; - this.checkItemB = checkItemB; + this.higherCheckItem = higherCheckItem; + this.lowerCheckItem = lowerCheckItem; } @Override public Optional<Issue> check() { - final Optional<Issue> aToB = getIssue(checkItemA, checkItemB); + final Optional<Issue> aToB = subsumes(higherCheckItem, lowerCheckItem); if (isFirstHitPolicy()) { if (aToB.isPresent()) { - // No need to check for redundancy. The first row masks the rest. + // No need to check for redundancy. The first row masks the other. return Optional.of(new Issue( "Masking row. The higher row prevents the activation of the other row.", - checkItemA.getLocation(), - checkItemB.getLocation())); + higherCheckItem.getLocation(), + lowerCheckItem.getLocation())); } else { // No need to check the other row for subsumption, // since subsumption is likely there by rule design return Optional.empty(); } } else { - final Optional<Issue> bToA = getIssue(checkItemB, checkItemA); + final Optional<Issue> bToA = subsumes(lowerCheckItem, higherCheckItem); // If the counterpart in the table is subsumptant, meaning the other item subsumes this, we have redundancy. if (aToB.isPresent() && bToA.isPresent()) { return Optional.of(new Issue( getRedundancyMessage(), - checkItemA.getLocation(), - checkItemB.getLocation())); + higherCheckItem.getLocation(), + lowerCheckItem.getLocation())); } else if (aToB.isPresent()) { return aToB; } else { @@ -100,7 +100,7 @@ public class SubsumptionCheck || Objects.equals("PRIORITY", hitPolicy); } - private Optional<Issue> getIssue( + private Optional<Issue> subsumes( final CheckItem checkItemA, final CheckItem checkItemB) { // All values and ranges in A are covered by B diff --git a/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/Util.java b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/Util.java new file mode 100644 index 0000000000..ed77acd7f0 --- /dev/null +++ b/packages/yard-validator-worker/src/main/java/org/yard/validator/checks/Util.java @@ -0,0 +1,45 @@ +/* + * 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.yard.validator.checks; + +import org.yard.validator.key.RowLocation; + +public class Util { + public static RowLocation getHigherInTable( + final RowLocation[] locations, + final int i, + final int j) { + if (locations[i].getTableRowNumber() < locations[j].getTableRowNumber()) { + return locations[i]; + } else { + return locations[j]; + } + } + + public static RowLocation getLowerInTable( + final RowLocation[] locations, + final int i, + final int j) { + if (locations[i].getTableRowNumber() > locations[j].getTableRowNumber()) { + return locations[i]; + } else { + return locations[j]; + } + } +} diff --git a/packages/yard-validator-worker/src/test/java/org/yard/validator/YardValidatorTest.java b/packages/yard-validator-worker/src/test/java/org/yard/validator/YardValidatorTest.java index 6a8b259349..7a68e1b1d8 100644 --- a/packages/yard-validator-worker/src/test/java/org/yard/validator/YardValidatorTest.java +++ b/packages/yard-validator-worker/src/test/java/org/yard/validator/YardValidatorTest.java @@ -70,7 +70,7 @@ public class YardValidatorTest { @Test public void maskingRule() throws FileNotFoundException { - final String read = read("subsumption-the-other-way.yml", "First"); + final String read = read("subsumption.yml", "First"); final YardValidator validator = new YardValidator(); final List<Issue> issues = new ArrayList<>(); @@ -81,6 +81,30 @@ public class YardValidatorTest { assertIssue(issues.get(0), "Masking row. The higher row prevents the activation of the other row.", 2, 3); } + @Test + public void subsumptionButFirstHPMakesItOK() throws FileNotFoundException { + final String read = read("subsumption-the-other-way.yml", "First"); + final YardValidator validator = new YardValidator(); + + final List<Issue> issues = new ArrayList<>(); + + validator.validate(read, issues::add); + + assertEquals(0, issues.size()); + } + + @Test + public void noIssues() throws FileNotFoundException { + final String read = read("package-prices.yml"); + final YardValidator validator = new YardValidator(); + + final List<Issue> issues = new ArrayList<>(); + + validator.validate(read, issues::add); + + assertEquals(0, issues.size()); + } + @Test public void redundancy() throws FileNotFoundException { final String read = read("redundancy.yml"); @@ -98,6 +122,24 @@ public class YardValidatorTest { assertIssue(issues.get(0), "Redundancy found. If both rows return the same result, the other can be removed. If they return different results, the table fails to return a value.", 1, 2); } + @Test + public void checkCorrectIssueRowsWhenThen() throws FileNotFoundException { + final String read = read("when-then-redundancy.yml"); + final YardValidator validator = new YardValidator(); + + final List<Issue> issues = new ArrayList<>(); + + validator.validate(read, issues::add); + + assertEquals(1, issues.size()); + final Issue issue = issues.get(0); + assertIssue(issue, "Redundancy found. If both rows return the same result, the other can be removed. If they return different results, the table fails to return a value.", 1, 2); + final RowLocation first = (RowLocation) issue.getLocations()[0]; + final RowLocation second = (RowLocation) issue.getLocations()[1]; + assertEquals(22, first.getActualRowNumberInFile()); + assertEquals(24, second.getActualRowNumberInFile()); + } + @Test public void redundancyWithUniqueHP() throws FileNotFoundException { final String read = read("redundancy.yml", "Unique"); @@ -114,6 +156,7 @@ public class YardValidatorTest { assertEquals(1, issues.size()); assertIssue(issues.get(0), "Redundancy found. Unique hit policy fails when more than one row returns results.", 1, 2); } + private void assertIssue( final Issue issue, final String message, diff --git a/packages/yard-validator-worker/src/test/resources/org/yard/validator/package-prices.yml b/packages/yard-validator-worker/src/test/resources/org/yard/validator/package-prices.yml new file mode 100644 index 0000000000..d29bafd2f0 --- /dev/null +++ b/packages/yard-validator-worker/src/test/resources/org/yard/validator/package-prices.yml @@ -0,0 +1,30 @@ +specVersion: alpha +kind: YaRD +name: "Traffic Violation" +# expressionLang: FEEL +inputs: + - name: "Length" + type: integer + - name: "Width" + type: number + - name: "Height" + type: number + - name: "Weight" + type: number +elements: + - name: Package + type: Decision + logic: + type: DecisionTable + # First matching result will be picked + hitPolicy: FIRST + inputs: ["Height", "Width", "Length", "Weight"] + rules: + - when: ["<= 3", "<= 25", "<= 35", "<= 2"] + then: '{ "Size": "S", "Cost": 10.90 }' + - when: ["<= 11", "<= 32", "<= 42", "<=25"] + then: '{ "Size": "M", "Cost": 16.90 }' + - when: ["<= 19", "<= 36", "<= 60", "<= 25"] + then: '{ "Size": "L", "Cost": 18.90 }' + - when: ["<= 37", "<= 36", "<= 60", "<= 25"] + then: '{ "Size": "XL", "Cost": 25.90}' diff --git a/packages/yard-validator-worker/src/test/resources/org/yard/validator/when-then-redundancy.yml b/packages/yard-validator-worker/src/test/resources/org/yard/validator/when-then-redundancy.yml new file mode 100644 index 0000000000..85116ec7eb --- /dev/null +++ b/packages/yard-validator-worker/src/test/resources/org/yard/validator/when-then-redundancy.yml @@ -0,0 +1,25 @@ +specVersion: alpha +kind: YaRD +name: "Traffic Violation" +# expressionLang: FEEL +inputs: + - name: "Length" + type: integer + - name: "Width" + type: number + - name: "Height" + type: number + - name: "Weight" + type: number +elements: + - name: Package + type: Decision + logic: + type: DecisionTable + # First matching result will be picked + inputs: ["Height", "Width", "Length", "Weight"] + rules: + - when: ["<= 3", "<= 25", "<= 35", "<= 2"] + then: '{ "Size": "S", "Cost": 10.90 }' + - when: ["<= 3", "<= 25", "<= 35", "<= 2"] + then: '{ "Size": "S", "Cost": 10.90 }' --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
