From 6c59cb84c9195712e439f389ff17156476c9d33a Mon Sep 17 00:00:00 2001 From: Jared Davis Date: Wed, 10 Apr 2024 10:18:21 -0400 Subject: [PATCH 1/3] Add optional Regular Expression cache to MatchesOperator --- .../model/operators/MatchesOperator.java | 39 ++++++++++++++++++- .../model/operators/MatchesOperatorTest.java | 23 +++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java diff --git a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java index 880748db1ec..b34e698484b 100644 --- a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java +++ b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java @@ -20,13 +20,50 @@ import org.drools.model.functions.Operator; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + + public enum MatchesOperator implements Operator.SingleValue { INSTANCE; + private static final String CACHE_MATCHES_COMPILED_MAX_PROPERTY = "drools.matches.compiled.cache.count"; + + // default to 0 for no cache + private final static int MAX_SIZE_CACHE = Integer.parseInt(System.getProperty(CACHE_MATCHES_COMPILED_MAX_PROPERTY, "0")); + + // store Pattern for regular expressions using the regular expression as the key up to MAX_SIZE_CACHE entries. + public final Map patternMap = Collections.synchronizedMap(new LinkedHashMap<>() { + @Override + protected boolean removeEldestEntry( Map.Entry eldest ) { + return size() > (MAX_SIZE_CACHE); + } + }); + + + + // S1 is the candidate string + // S2 is the regular expression @Override public boolean eval( String s1, String s2 ) { - return s1 != null && s1.matches( s2 ); + if (s1 == null) { + return false; + } else if (MAX_SIZE_CACHE ==0 ) { + return s1.matches( s2 ); + } else { + Pattern pattern = patternMap.get( s2 ); + if (pattern == null) { + // cache miss on s2, compile it, then store it + pattern = Pattern.compile(s2); + patternMap.put(s2, pattern); + } + Matcher matcher = pattern.matcher(s1); + return matcher.matches(); + } } @Override diff --git a/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java b/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java new file mode 100644 index 00000000000..947dcc24382 --- /dev/null +++ b/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java @@ -0,0 +1,23 @@ +package org.drools.model.operators; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + + +public class MatchesOperatorTest { + @Test + public void testMatchesOperator() { + MatchesOperator instance = MatchesOperator.INSTANCE; + // Regular expression is second parameter + assertThat(instance.eval("a","a")).isTrue(); + assertThat(instance.eval("a","b")).isFalse(); + assertThat(instance.eval("a","a")).isTrue(); + assertThat(instance.eval("b","b")).isTrue(); + + // s1 maybe null + assertThat(instance.eval(null,"anything")).isFalse(); + + } + +} From 6039841c3f1396179e9e2ec8f939ccce1c898ccb Mon Sep 17 00:00:00 2001 From: Jared Davis Date: Mon, 15 Apr 2024 08:06:26 -0400 Subject: [PATCH 2/3] Improve test cases, variable names and comments. --- .../model/operators/MatchesOperator.java | 53 ++++++++++++------ .../model/operators/MatchesOperatorTest.java | 56 +++++++++++++++++-- 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java index b34e698484b..5ce9cc17e55 100644 --- a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java +++ b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java @@ -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 @@ -31,37 +31,54 @@ public enum MatchesOperator implements Operator.SingleValue { INSTANCE; - private static final String CACHE_MATCHES_COMPILED_MAX_PROPERTY = "drools.matches.compiled.cache.count"; - - // default to 0 for no cache - private final static int MAX_SIZE_CACHE = Integer.parseInt(System.getProperty(CACHE_MATCHES_COMPILED_MAX_PROPERTY, "0")); + // not final due to unit tests + private int MAX_SIZE_CACHE = getMaxSizeCache(); // store Pattern for regular expressions using the regular expression as the key up to MAX_SIZE_CACHE entries. - public final Map patternMap = Collections.synchronizedMap(new LinkedHashMap<>() { + private final Map patternMap = Collections.synchronizedMap(new LinkedHashMap<>() { @Override - protected boolean removeEldestEntry( Map.Entry eldest ) { + protected boolean removeEldestEntry(Map.Entry eldest) { return size() > (MAX_SIZE_CACHE); } }); + // 0 default disables the Pattern map + private static int getMaxSizeCache() { + final String CACHE_MATCHES_COMPILED_MAX_PROPERTY = "drools.matches.compiled.cache.count"; + return Integer.parseInt(System.getProperty(CACHE_MATCHES_COMPILED_MAX_PROPERTY, "0")); + } + + // package-private for unit testing + void forceCacheSize(int size) { + MAX_SIZE_CACHE = size; + patternMap.clear(); + } + // package-private for unit testing + void reInitialize() { + forceCacheSize(getMaxSizeCache()); + } + + // package-private for unit testing + int mapSize() { + return patternMap.size(); + } - // S1 is the candidate string - // S2 is the regular expression @Override - public boolean eval( String s1, String s2 ) { - if (s1 == null) { + public boolean eval(String input, String regex) { + if (input == null) { return false; - } else if (MAX_SIZE_CACHE ==0 ) { - return s1.matches( s2 ); + } else if (MAX_SIZE_CACHE == 0) { + return input.matches(regex); } else { - Pattern pattern = patternMap.get( s2 ); + Pattern pattern = patternMap.get(regex); if (pattern == null) { - // cache miss on s2, compile it, then store it - pattern = Pattern.compile(s2); - patternMap.put(s2, pattern); + // Cache miss on regex, compile it, store it. + // Storing in patternMap may remove the oldest entry per MAX_SIZE_CACHE. + pattern = Pattern.compile(regex); + patternMap.put(regex, pattern); } - Matcher matcher = pattern.matcher(s1); + Matcher matcher = pattern.matcher(input); return matcher.matches(); } } diff --git a/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java b/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java index 947dcc24382..4520828c4ea 100644 --- a/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java +++ b/drools-model/drools-canonical-model/src/test/java/org/drools/model/operators/MatchesOperatorTest.java @@ -1,23 +1,69 @@ +/** + * 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.operators; +import org.junit.After; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; public class MatchesOperatorTest { + + @Test - public void testMatchesOperator() { + public void testMatchesOperatorCache() { MatchesOperator instance = MatchesOperator.INSTANCE; - // Regular expression is second parameter + instance.forceCacheSize(100); + + // input maybe null + assertThat(instance.eval(null,"anything")).isFalse(); + assertThat(instance.mapSize()).isEqualTo(0); // not added to cache with null input + // cache enabled assertThat(instance.eval("a","a")).isTrue(); + assertThat(instance.mapSize()).isEqualTo(1); assertThat(instance.eval("a","b")).isFalse(); - assertThat(instance.eval("a","a")).isTrue(); - assertThat(instance.eval("b","b")).isTrue(); + assertThat(instance.mapSize()).isEqualTo(2); + assertThat(instance.eval("a","a")).isTrue(); // regular expression "a" in map. + assertThat(instance.eval("b","b")).isTrue(); // regular expression "b" in map. + assertThat(instance.eval("c","a")).isFalse(); // regular expression "a" in map. + assertThat(instance.eval("c","b")).isFalse(); // regular expression "b" in map. + assertThat(instance.mapSize()).isEqualTo(2); + } - // s1 maybe null + @Test + public void testMatchesOperatorNoCache() { + MatchesOperator instance = MatchesOperator.INSTANCE; + instance.forceCacheSize(0); + // input maybe null assertThat(instance.eval(null,"anything")).isFalse(); + assertThat(instance.eval("a","a")).isTrue(); + assertThat(instance.eval("a","b")).isFalse(); + assertThat(instance.eval("b","a")).isFalse(); + assertThat(instance.eval("b","b")).isTrue(); + assertThat(instance.mapSize()).isEqualTo(0); + } + @After + public void resetCache() { + MatchesOperator instance = MatchesOperator.INSTANCE; + instance.reInitialize(); } } From b21c1540ab5a544adfb6984033e476a4bf978680 Mon Sep 17 00:00:00 2001 From: Jared Davis Date: Mon, 15 Apr 2024 09:09:53 -0400 Subject: [PATCH 3/3] Added static to two fields --- .../main/java/org/drools/model/operators/MatchesOperator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java index 5ce9cc17e55..6635f790ca3 100644 --- a/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java +++ b/drools-model/drools-canonical-model/src/main/java/org/drools/model/operators/MatchesOperator.java @@ -32,10 +32,10 @@ public enum MatchesOperator implements Operator.SingleValue { INSTANCE; // not final due to unit tests - private int MAX_SIZE_CACHE = getMaxSizeCache(); + private static int MAX_SIZE_CACHE = getMaxSizeCache(); // store Pattern for regular expressions using the regular expression as the key up to MAX_SIZE_CACHE entries. - private final Map patternMap = Collections.synchronizedMap(new LinkedHashMap<>() { + private static final Map patternMap = Collections.synchronizedMap(new LinkedHashMap<>() { @Override protected boolean removeEldestEntry(Map.Entry eldest) { return size() > (MAX_SIZE_CACHE);