Skip to content

Commit

Permalink
kie-issues#1448: matches() function wrongly behaves (#6085)
Browse files Browse the repository at this point in the history
* matches and replace using saxon

* saxon usage simplified and modified tests

* clean up code as per standards

* changes as per suggestions

* changes as per suggestions and restored the earlier tests

* changes as per suggestions and restored the earlier tests

* changes as per suggestions

* exceptions handled and changes to util class

* wip

* wip

* wip

* wip

* wip

* CR

* CR

* CR

* CR

* Notice updated

* CR

---------

Co-authored-by: bncriju <[email protected]>
  • Loading branch information
yesamer and bncriju committed Sep 18, 2024
1 parent 1f2e67c commit 8dac313
Show file tree
Hide file tree
Showing 9 changed files with 409 additions and 186 deletions.
4 changes: 4 additions & 0 deletions NOTICE
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ This product also includes the following third-party components:
Downloaded from: https://lunrjs.com/
License: MIT

* Saxon-HE
Downloaded from: https://www.saxonica.com/
License: Mozilla Public License 2.0

* search-ui
Downloaded from: https://gitlab.com/antora/antora-lunr-extension
License: Mozilla Public License 2.0
15 changes: 15 additions & 0 deletions kie-dmn/kie-dmn-feel/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,19 @@
<java.module.name>org.kie.dmn.feel</java.module.name>
<surefire.forkCount>2</surefire.forkCount>
<enforcer.skip>true</enforcer.skip>
<version.net.sf.saxon.Saxon-HE>12.5</version.net.sf.saxon.Saxon-HE>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>net.sf.saxon</groupId>
<artifactId>Saxon-HE</artifactId>
<version>${version.net.sf.saxon.Saxon-HE}</version>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<!-- Internal dependencies -->
<dependency>
Expand All @@ -52,6 +63,10 @@
</dependency>

<!-- External dependencies -->
<dependency>
<groupId>net.sf.saxon</groupId>
<artifactId>Saxon-HE</artifactId>
</dependency>
<dependency>
<groupId>org.antlr</groupId>
<artifactId>antlr4-runtime</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,9 @@
*/
package org.kie.dmn.feel.runtime.functions;

import java.security.InvalidParameterException;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;

import org.kie.dmn.api.feel.runtime.events.FEELEvent.Severity;
import org.kie.dmn.feel.runtime.events.InvalidParametersEvent;
import org.kie.dmn.feel.util.XQueryImplUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -44,58 +38,23 @@ public FEELFnResult<Boolean> invoke(@ParameterName("input") String input, @Param
}

public FEELFnResult<Boolean> invoke(@ParameterName("input") String input, @ParameterName("pattern") String pattern, @ParameterName("flags") String flags) {
try {
return matchFunctionWithFlags(input,pattern,flags);
} catch ( PatternSyntaxException t ) {
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "pattern", "is invalid and can not be compiled", t ) );
} catch (InvalidParameterException t ) {
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, t.getMessage(), "cannot be null", t ) );
} catch (IllegalArgumentException t ) {
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "flags", "contains unknown flags", t ) );
} catch (Throwable t) {
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "pattern", "is invalid and can not be compiled", t ) );
}
}

static FEELFnResult<Boolean> matchFunctionWithFlags(String input, String pattern, String flags) {
log.debug("Input: {} , Pattern: {}, Flags: {}", input, pattern, flags);

if ( input == null ) {
throw new InvalidParameterException("input");
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "input", "cannot be null" ) );
}
if ( pattern == null ) {
throw new InvalidParameterException("pattern");
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "pattern", "cannot be null" ) );
}
final String flagsString;
if (flags != null && !flags.isEmpty()) {
checkFlags(flags);
if(!flags.contains("U")){
flags += "U";
}
flagsString = String.format("(?%s)", flags);
} else {
flagsString = "";
}
log.debug("flagsString: {}", flagsString);
String stringToBeMatched = flagsString + pattern;
log.debug("stringToBeMatched: {}", stringToBeMatched);
Pattern p=Pattern.compile(stringToBeMatched);
Matcher m = p.matcher( input );
boolean matchFound=m.find();
log.debug("matchFound: {}", matchFound);
return FEELFnResult.ofResult(matchFound);
}

static void checkFlags(String flags) {
Set<Character> allowedChars = Set.of('s','i','x','m');
boolean isValidFlag= flags.chars()
.mapToObj(c -> (char) c)
.allMatch(allowedChars::contains)
&& flags.chars()
.mapToObj(c -> (char) c)
.collect(Collectors.toSet())
.size() == flags.length();
if(!isValidFlag){
throw new IllegalArgumentException("Not a valid flag parameter " +flags);
}
try {
return FEELFnResult.ofResult(XQueryImplUtil.executeMatchesFunction(input, pattern, flags));
} catch (Exception e) {
String errorMessage = String.format("Provided parameters lead to an error. Input: '%s', Pattern: '%s', Flags: '%s'. ", input, pattern, flags);
if (e.getMessage() != null && !e.getMessage().isEmpty()) {
errorMessage += e.getMessage();
}
return FEELFnResult.ofError(new InvalidParametersEvent(Severity.ERROR, errorMessage, e));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.kie.dmn.api.feel.runtime.events.FEELEvent.Severity;
import org.kie.dmn.feel.runtime.events.InvalidParametersEvent;
import org.kie.dmn.feel.util.XQueryImplUtil;

public class ReplaceFunction
extends BaseFEELFunction {
Expand All @@ -30,12 +31,12 @@ private ReplaceFunction() {
super( "replace" );
}

public FEELFnResult<Object> invoke(@ParameterName("input") String input, @ParameterName("pattern") String pattern,
public FEELFnResult<String> invoke(@ParameterName("input") String input, @ParameterName("pattern") String pattern,
@ParameterName( "replacement" ) String replacement ) {
return invoke(input, pattern, replacement, null);
}

public FEELFnResult<Object> invoke(@ParameterName("input") String input, @ParameterName("pattern") String pattern,
public FEELFnResult<String> invoke(@ParameterName("input") String input, @ParameterName("pattern") String pattern,
@ParameterName( "replacement" ) String replacement, @ParameterName("flags") String flags) {
if ( input == null ) {
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "input", "cannot be null" ) );
Expand All @@ -47,14 +48,16 @@ public FEELFnResult<Object> invoke(@ParameterName("input") String input, @Parame
return FEELFnResult.ofError( new InvalidParametersEvent( Severity.ERROR, "replacement", "cannot be null" ) );
}

final String flagsString;
if (flags != null && !flags.isEmpty()) {
flagsString = "(?" + flags + ")";
} else {
flagsString = "";
try {
return FEELFnResult.ofResult(XQueryImplUtil.executeReplaceFunction(input, pattern, replacement, flags));
} catch (Exception e) {
String errorMessage = String.format("Provided parameters lead to an error. Input: '%s', Pattern: '%s', Replacement: '%s', Flags: '%s'. ",
input, pattern, replacement, flags);
if (e.getMessage() != null && !e.getMessage().isEmpty()) {
errorMessage += e.getMessage();
}
return FEELFnResult.ofError(new InvalidParametersEvent(Severity.ERROR, errorMessage, e));
}

return FEELFnResult.ofResult( input.replaceAll( flagsString + pattern, replacement ) );
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* 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.kie.dmn.feel.util;

import net.sf.saxon.s9api.Processor;
import net.sf.saxon.s9api.XdmAtomicValue;
import net.sf.saxon.s9api.XdmItem;
import net.sf.saxon.s9api.XQueryCompiler;
import net.sf.saxon.s9api.XQueryEvaluator;
import net.sf.saxon.s9api.XQueryExecutable;
import net.sf.saxon.s9api.SaxonApiException;

public class XQueryImplUtil {

public static Boolean executeMatchesFunction(String input, String pattern, String flags) {
flags = flags == null ? "" : flags;
String xQueryExpression = String.format("matches('%s', '%s', '%s')", input, pattern, flags);
return evaluateXQueryExpression(xQueryExpression, Boolean.class);
}

public static String executeReplaceFunction(String input, String pattern, String replacement, String flags) {
flags = flags == null ? "" : flags;
String xQueryExpression = String.format("replace('%s', '%s', '%s', '%s')", input, pattern, replacement, flags);
return evaluateXQueryExpression(xQueryExpression, String.class);
}

static <T> T evaluateXQueryExpression(String expression, Class<T> expectedTypeResult) {
try {
Processor processor = new Processor(false);
XQueryCompiler compiler = processor.newXQueryCompiler();
XQueryExecutable executable = compiler.compile(expression);
XQueryEvaluator queryEvaluator = executable.load();
XdmItem resultItem = queryEvaluator.evaluateSingle();

Object value = switch (expectedTypeResult.getSimpleName()) {
case "Boolean" -> ((XdmAtomicValue) resultItem).getBooleanValue();
case "String" -> resultItem.getStringValue();
default -> throw new UnsupportedOperationException("Type " + expectedTypeResult.getSimpleName() + " is not managed.");
};

return expectedTypeResult.cast(value);
} catch (SaxonApiException e) {
throw new IllegalArgumentException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ private static Collection<Object[]> data() {
final Object[][] cases = new Object[][] {
// constants
{ "string(1.1)", "1.1" , null},
{ "replace( \" foo bar zed \", \"^(\\s)+|(\\s)+$|\\s+(?=\\s)\", \"\" )", "foo bar zed", null },
{ "string(null)", null, null},
{ "string(date(\"2016-08-14\"))", "2016-08-14" , null},
{ "string(\"Happy %.0fth birthday, Mr %s!\", 38, \"Doe\")", "Happy 38th birthday, Mr Doe!", null},
Expand Down Expand Up @@ -85,6 +84,14 @@ private static Collection<Object[]> data() {
{ "matches(\"one\\ntwo\\nthree\", \"^two$\", \"m\")", Boolean.TRUE , null}, // MULTILINE flag set by "m"
{ "matches(\"FoO\", \"foo\")", Boolean.FALSE , null},
{ "matches(\"FoO\", \"foo\", \"i\")", Boolean.TRUE , null}, // CASE_INSENSITIVE flag set by "i"
{ "matches(\"\\u212A\", \"k\", \"i\")", Boolean.TRUE , null},
{ "matches(\"O\", \"[A-Z-[OI]]\", \"i\")", Boolean.FALSE , null},
{ "matches(\"i\", \"[A-Z-[OI]]\", \"i\")", Boolean.FALSE , null},
{ "matches(\"hello world\", \"hello\\ sworld\", \"x\")", Boolean.TRUE , null},
{ "matches(\"abracadabra\", \"bra\", \"p\")", null , FEELEvent.Severity.ERROR},
{ "matches(\"input\", \"pattern\", \" \")", null , FEELEvent.Severity.ERROR},
{ "matches(\"input\", \"pattern\", \"X\")", null , FEELEvent.Severity.ERROR},
{ "matches(\"h\", \"(.)\\2\")", null , FEELEvent.Severity.ERROR},
{ "replace(\"banana\",\"a\",\"o\")", "bonono" , null},
{ "replace(\"banana\",\"(an)+\", \"**\")", "b**a" , null},
{ "replace(\"banana\",\"[aeiouy]\",\"[$0]\")", "b[a]n[a]n[a]" , null},
Expand Down
Loading

0 comments on commit 8dac313

Please sign in to comment.