From 95ef42bfbe0258f56f940b2e98b0bafcd5e61171 Mon Sep 17 00:00:00 2001
From: Mahmoud Ben Hassine <mbenhassine@pivotal.io>
Date: Sun, 3 May 2020 14:28:48 +0200
Subject: [PATCH] Add Fact concept
Resolves #276
---
.../main/java/org/jeasy/rules/api/Fact.java | 81 ++++++++++++
.../main/java/org/jeasy/rules/api/Facts.java | 125 ++++++++++++------
.../jeasy/rules/core/DefaultRulesEngine.java | 5 +-
.../java/org/jeasy/rules/api/FactsTest.java | 100 +++++++-------
.../rules/core/DefaultRulesEngineTest.java | 17 +--
...> MissingFactAnnotationParameterTest.java} | 20 +--
6 files changed, 225 insertions(+), 123 deletions(-)
create mode 100644 easy-rules-core/src/main/java/org/jeasy/rules/api/Fact.java
rename easy-rules-core/src/test/java/org/jeasy/rules/core/{NullFactAnnotationParameterTest.java => MissingFactAnnotationParameterTest.java} (79%)
diff --git a/easy-rules-core/src/main/java/org/jeasy/rules/api/Fact.java b/easy-rules-core/src/main/java/org/jeasy/rules/api/Fact.java
new file mode 100644
index 0000000..6066512
--- /dev/null
+++ b/easy-rules-core/src/main/java/org/jeasy/rules/api/Fact.java
@@ -0,0 +1,81 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2020, Mahmoud Ben Hassine (mahmoud.benhassine@icloud.com)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.jeasy.rules.api;
+
+import java.util.Objects;
+
+/**
+ * A class representing a named fact. Facts have unique names within a {@link Facts}
+ * instance.
+ *
+ * @param <T> type of the fact
+ * @author Mahmoud Ben Hassine
+ */
+public class Fact<T> {
+
+ private final String name;
+
+ private final T value;
+
+ public Fact(String name, T value) {
+ Objects.requireNonNull(name, "name must not be null");
+ Objects.requireNonNull(value, "value must not be null");
+ this.name = name;
+ this.value = value;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public T getValue() {
+ return value;
+ }
+
+ @Override
+ public String toString() {
+ return "Fact{" +
+ "name='" + name + '\'' +
+ ", value=" + value +
+ '}';
+ }
+
+ /*
+ * The Facts API represents a namespace for facts where each fact has a unique name.
+ * Hence, equals/hashcode are deliberately calculated only on the fact name.
+ */
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ Fact<?> fact = (Fact<?>) o;
+ return name.equals(fact.name);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(name);
+ }
+}
diff --git a/easy-rules-core/src/main/java/org/jeasy/rules/api/Facts.java b/easy-rules-core/src/main/java/org/jeasy/rules/api/Facts.java
index 55b8811..574acec 100644
--- a/easy-rules-core/src/main/java/org/jeasy/rules/api/Facts.java
+++ b/easy-rules-core/src/main/java/org/jeasy/rules/api/Facts.java
@@ -25,58 +25,98 @@ package org.jeasy.rules.api;
import java.util.*;
-import static java.lang.String.format;
-
/**
- * Represents a set of named facts. Facts have unique name within a <code>Facts</code> object.
+ * Represents a set of named facts. Facts have unique names within a <code>Facts</code> object.
*
* @author Mahmoud Ben Hassine (mahmoud.benhassine@icloud.com)
*/
-public class Facts implements Iterable<Map.Entry<String, Object>> {
+public class Facts implements Iterable<Fact<?>> {
- private Map<String, Object> facts = new HashMap<>();
+ private final Set<Fact<?>> facts = new HashSet<>();
/**
- * Put a fact in the working memory.
- * This will replace any fact having the same name.
+ * Add a fact, replacing any fact with the same name.
*
- * @param name fact name
- * @param fact object to put in the working memory
- * @return the previous value associated with <tt>name</tt>, or
- * <tt>null</tt> if there was no mapping for <tt>name</tt>.
- * (A <tt>null</tt> return can also indicate that the map
- * previously associated <tt>null</tt> with <tt>name</tt>.)
+ * @param name of the fact to add
+ * @param value of the fact to add
*/
- public Object put(String name, Object fact) {
- Objects.requireNonNull(name);
- return facts.put(name, fact);
+ public <T> void put(String name, T value) {
+ Objects.requireNonNull(name, "fact name must not be null");
+ Objects.requireNonNull(value, "fact value must not be null");
+ Fact<?> retrievedFact = getFact(name);
+ if (retrievedFact != null) {
+ remove(retrievedFact);
+ }
+ add(new Fact<>(name, value));
+ }
+
+ /**
+ * Add a fact, replacing any fact with the same name.
+ *
+ * @param fact to add
+ */
+ public <T> void add(Fact<T> fact) {
+ Objects.requireNonNull(fact, "fact must not be null");
+ Fact<?> retrievedFact = getFact(fact.getName());
+ if (retrievedFact != null) {
+ remove(retrievedFact);
+ }
+ facts.add(fact);
}
/**
- * Remove fact.
+ * Remove a fact by name.
*
- * @param name of fact to remove
- * @return the previous value associated with <tt>name</tt>, or
- * <tt>null</tt> if there was no mapping for <tt>name</tt>.
- * (A <tt>null</tt> return can also indicate that the map
- * previously associated <tt>null</tt> with <tt>name</tt>.)
+ * @param factName name of the fact to remove
*/
- public Object remove(String name) {
- Objects.requireNonNull(name);
- return facts.remove(name);
+ public void remove(String factName) {
+ Objects.requireNonNull(factName, "fact name must not be null");
+ Fact<?> fact = getFact(factName);
+ if (fact != null) {
+ remove(fact);
+ }
}
/**
- * Get a fact by name.
+ * Remove a fact.
*
- * @param name of the fact
- * @param <T> type of the fact
- * @return the fact having the given name, or null if there is no fact with the given name
+ * @param fact to remove
+ */
+ public <T> void remove(Fact<T> fact) {
+ Objects.requireNonNull(fact, "fact must not be null");
+ facts.remove(fact);
+ }
+
+ /**
+ * Get the value of a fact by its name. This is a convenience method provided
+ * as a short version of {@code getFact(factName).getValue()}.
+ *
+ * @param factName name of the fact
+ * @param <T> type of the fact's value
+ * @return the value of the fact having the given name, or {@code null} if there is no fact with the given name
*/
@SuppressWarnings("unchecked")
- public <T> T get(String name) {
- Objects.requireNonNull(name);
- return (T) facts.get(name);
+ public <T> T get(String factName) {
+ Objects.requireNonNull(factName, "fact name must not be null");
+ Fact<?> fact = getFact(factName);
+ if (fact != null) {
+ return (T) fact.getValue();
+ }
+ return null;
+ }
+
+ /**
+ * Get a fact by name.
+ *
+ * @param factName name of the fact
+ * @return the fact having the given name, or {@code null} if there is no fact with the given name
+ */
+ public Fact<?> getFact(String factName) {
+ Objects.requireNonNull(factName, "fact name must not be null");
+ return facts.stream()
+ .filter(fact -> fact.getName().equals(factName))
+ .findFirst()
+ .orElse(null);
}
/**
@@ -86,7 +126,11 @@ public class Facts implements Iterable<Map.Entry<String, Object>> {
* @return a copy of the current facts as a {@link HashMap}
*/
public Map<String, Object> asMap() {
- return new HashMap<>(facts);
+ Map<String, Object> map = new HashMap<>();
+ for (Fact<?> fact : facts) {
+ map.put(fact.getName(), fact.getValue());
+ }
+ return map;
}
/**
@@ -96,8 +140,8 @@ public class Facts implements Iterable<Map.Entry<String, Object>> {
* @return an iterator on the set of facts
*/
@Override
- public Iterator<Map.Entry<String, Object>> iterator() {
- return facts.entrySet().iterator();
+ public Iterator<Fact<?>> iterator() {
+ return facts.iterator();
}
/**
@@ -109,16 +153,15 @@ public class Facts implements Iterable<Map.Entry<String, Object>> {
@Override
public String toString() {
+ Iterator<Fact<?>> iterator = facts.iterator();
StringBuilder stringBuilder = new StringBuilder("[");
- List<Map.Entry<String, Object>> entries = new ArrayList<>(facts.entrySet());
- for (int i = 0; i < entries.size(); i++) {
- Map.Entry<String, Object> entry = entries.get(i);
- stringBuilder.append(format(" { %s : %s } ", entry.getKey(), entry.getValue()));
- if (i < entries.size() - 1) {
+ while (iterator.hasNext()) {
+ stringBuilder.append(iterator.next().toString());
+ if (iterator.hasNext()) {
stringBuilder.append(",");
}
}
stringBuilder.append("]");
- return stringBuilder.toString();
+ return stringBuilder.toString();
}
}
diff --git a/easy-rules-core/src/main/java/org/jeasy/rules/core/DefaultRulesEngine.java b/easy-rules-core/src/main/java/org/jeasy/rules/core/DefaultRulesEngine.java
index 3ec4a0b..95d643d 100644
--- a/easy-rules-core/src/main/java/org/jeasy/rules/core/DefaultRulesEngine.java
+++ b/easy-rules-core/src/main/java/org/jeasy/rules/core/DefaultRulesEngine.java
@@ -145,9 +145,8 @@ public final class DefaultRulesEngine extends AbstractRulesEngine {
private void log(Facts facts) {
LOGGER.debug("Known facts:");
- for (Map.Entry<String, Object> fact : facts) {
- LOGGER.debug("Fact { {} : {} }",
- fact.getKey(), fact.getValue());
+ for (Fact<?> fact : facts) {
+ LOGGER.debug(fact.toString());
}
}
diff --git a/easy-rules-core/src/test/java/org/jeasy/rules/api/FactsTest.java b/easy-rules-core/src/test/java/org/jeasy/rules/api/FactsTest.java
index 8e0a982..3727452 100644
--- a/easy-rules-core/src/test/java/org/jeasy/rules/api/FactsTest.java
+++ b/easy-rules-core/src/test/java/org/jeasy/rules/api/FactsTest.java
@@ -23,87 +23,97 @@
*/
package org.jeasy.rules.api;
-import org.junit.Test;
+import java.util.Map;
-import java.util.HashMap;
+import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThat;
public class FactsTest {
- private Facts facts = new Facts();
+ private final Facts facts = new Facts();
@Test
public void factsMustHaveUniqueName() {
- facts.put("foo", 1);
- facts.put("foo", 2);
+ facts.add(new Fact<>("foo", 1));
+ facts.add(new Fact<>("foo", 2));
assertThat(facts).hasSize(1);
- Object foo = facts.get("foo");
- assertThat(foo).isEqualTo(2);
+ Fact<?> fact = facts.getFact("foo");
+ assertThat(fact.getValue()).isEqualTo(2);
}
@Test
- public void returnOfPut() {
- Object o1 = facts.put("foo", 1);
- Object o2 = facts.put("foo", 2);
-
- assertThat(o1).isEqualTo(null);
- assertThat(o2).isEqualTo(1);
+ public void testAdd() {
+ Fact<Integer> fact1 = new Fact<>("foo", 1);
+ Fact<Integer> fact2 = new Fact<>("bar", 2);
+ facts.add(fact1);
+ facts.add(fact2);
+
+ assertThat(facts).contains(fact1);
+ assertThat(facts).contains(fact2);
}
@Test
- public void remove() {
+ public void testPut() {
facts.put("foo", 1);
- facts.remove("foo");
+ facts.put("bar", 2);
- assertThat(facts).isEmpty();
+ assertThat(facts).contains(new Fact<>("foo", 1));
+ assertThat(facts).contains(new Fact<>("bar", 2));
}
@Test
- public void returnOfRemove() {
- facts.put("foo", 1);
- Object o1 = facts.remove("foo");
- Object o2 = facts.remove("bar");
+ public void testRemove() {
+ Fact<Integer> foo = new Fact<>("foo", 1);
+ facts.add(foo);
+ facts.remove(foo);
- assertThat(o1).isEqualTo(1);
- assertThat(o2).isEqualTo(null);
+ assertThat(facts).isEmpty();
}
@Test
- public void get() {
- facts.put("foo", 1);
- Object foo = facts.get("foo");
- assertThat(foo).isEqualTo(1);
+ public void testRemoveByName() {
+ Fact<Integer> foo = new Fact<>("foo", 1);
+ facts.add(foo);
+ facts.remove("foo");
+
+ assertThat(facts).isEmpty();
}
@Test
- public void asMap() {
- Object o = facts.asMap();
- assertThat(o instanceof HashMap).isTrue();
- assertThat(o).isNotEqualTo(facts);
+ public void testGet() {
+ Fact<Integer> fact = new Fact<>("foo", 1);
+ facts.add(fact);
+ Integer value = facts.get("foo");
+ assertThat(value).isEqualTo(1);
}
@Test
- public void testClear() {
- Facts facts = new Facts();
- facts.put("foo", "bar");
- facts.clear();
- assertThat(facts.asMap()).isEmpty();
+ public void testGetFact() {
+ Fact<Integer> fact = new Fact<>("foo", 1);
+ facts.add(fact);
+ Fact<?> retrievedFact = facts.getFact("foo");
+ assertThat(retrievedFact).isEqualTo(fact);
}
- @Test(expected = NullPointerException.class)
- public void whenPutNullFact_thenShouldThrowNullPointerException() {
- facts.put(null, "foo");
+ @Test
+ public void testAsMap() {
+ Fact<Integer> fact1 = new Fact<>("foo", 1);
+ Fact<Integer> fact2 = new Fact<>("bar", 2);
+ facts.add(fact1);
+ facts.add(fact2);
+ Map<String, Object> map = facts.asMap();
+ assertThat(map).containsKeys("foo", "bar");
+ assertThat(map).containsValues(1, 2);
}
- @Test(expected = NullPointerException.class)
- public void whenRemoveNullFact_thenShouldThrowNullPointerException() {
- facts.remove(null);
+ @Test
+ public void testClear() {
+ Facts facts = new Facts();
+ facts.add(new Fact<>("foo", 1));
+ facts.clear();
+ assertThat(facts).isEmpty();
}
- @Test(expected = NullPointerException.class)
- public void whenGetNullFact_thenShouldThrowNullPointerException() {
- facts.get(null);
- }
}
diff --git a/easy-rules-core/src/test/java/org/jeasy/rules/core/DefaultRulesEngineTest.java b/easy-rules-core/src/test/java/org/jeasy/rules/core/DefaultRulesEngineTest.java
index a29cbf1..72f25d5 100644
--- a/easy-rules-core/src/test/java/org/jeasy/rules/core/DefaultRulesEngineTest.java
+++ b/easy-rules-core/src/test/java/org/jeasy/rules/core/DefaultRulesEngineTest.java
@@ -36,6 +36,7 @@ import org.assertj.core.api.Assertions;
import org.jeasy.rules.annotation.Action;
import org.jeasy.rules.annotation.Condition;
import org.jeasy.rules.annotation.Priority;
+import org.jeasy.rules.api.Fact;
import org.jeasy.rules.api.RuleListener;
import org.jeasy.rules.api.RulesEngineListener;
import org.junit.After;
@@ -201,22 +202,6 @@ public class DefaultRulesEngineTest extends AbstractTest {
verify(ruleListener).beforeEvaluate(rule1, facts);
}
- @Test
- public void nullFactsShouldNotCrashTheEngine() {
- // Given
- facts.put("foo", null);
-
- // When
- try {
- rulesEngine.fire(rules, facts);
- } catch (Exception e) {
- Assertions.fail("Unable to fire rules on known facts", e);
- }
-
- // Then
- // Should not throw exception
- }
-
@Test
public void getParametersShouldReturnACopyOfTheParameters() {
// Given
diff --git a/easy-rules-core/src/test/java/org/jeasy/rules/core/NullFactAnnotationParameterTest.java b/easy-rules-core/src/test/java/org/jeasy/rules/core/MissingFactAnnotationParameterTest.java
similarity index 79%
rename from easy-rules-core/src/test/java/org/jeasy/rules/core/NullFactAnnotationParameterTest.java
rename to easy-rules-core/src/test/java/org/jeasy/rules/core/MissingFactAnnotationParameterTest.java
index d2406ac..cb9aa2f 100644
--- a/easy-rules-core/src/test/java/org/jeasy/rules/core/NullFactAnnotationParameterTest.java
+++ b/easy-rules-core/src/test/java/org/jeasy/rules/core/MissingFactAnnotationParameterTest.java
@@ -35,25 +35,9 @@ import org.junit.Test;
import java.util.Map;
/**
- * Null value in facts must be accepted, this is not same thing that fact missing
+ * Null facts are not accepted by design, a declared fact can be missing though.
*/
-public class NullFactAnnotationParameterTest extends AbstractTest {
-
- @Test
- public void testNullFact() {
- Rules rules = new Rules();
- rules.register(new AnnotatedParametersRule());
-
- Facts facts = new Facts();
- facts.put("fact1", new Object());
- facts.put("fact2", null);
-
- Map<org.jeasy.rules.api.Rule, Boolean> results = rulesEngine.check(rules, facts);
-
- for (boolean b : results.values()) {
- Assert.assertTrue(b);
- }
- }
+public class MissingFactAnnotationParameterTest extends AbstractTest {
@Test
public void testMissingFact() {