Add Fact concept

Resolves #276

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);
+	}
+}

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();
     }
 }

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());
         }
     }
 

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);
-    }
 }

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

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() {