Moved duplicated isUpdateAcceptable method to IntentData

and wrote a unit test for it.

Change-Id: I8b38476c41fba70abbba7ed0b37364696f17966d

@@ -19,11 +19,19 @@ import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import org.onosproject.cluster.NodeId;
 import org.onosproject.store.Timestamp;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.Objects;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.onosproject.net.intent.IntentState.FAILED;
+import static org.onosproject.net.intent.IntentState.INSTALLED;
+import static org.onosproject.net.intent.IntentState.INSTALLING;
+import static org.onosproject.net.intent.IntentState.PURGE_REQ;
+import static org.onosproject.net.intent.IntentState.WITHDRAWING;
+import static org.onosproject.net.intent.IntentState.WITHDRAWN;
 
 /**
  * A wrapper class that contains an intents, its state, and other metadata for
@@ -31,6 +39,9 @@ import static com.google.common.base.Preconditions.checkNotNull;
  */
 public class IntentData { //FIXME need to make this "immutable"
                           // manager should be able to mutate a local copy while processing
+
+    private static final Logger log = LoggerFactory.getLogger(IntentData.class);
+
     private final Intent intent;
 
     private IntentState state;
@@ -167,6 +178,81 @@ public class IntentData { //FIXME need to make this "immutable"
         return installables;
     }
 
+    /**
+     * Determines whether an intent data update is allowed. The update must
+     * either have a higher version than the current data, or the state
+     * transition between two updates of the same version must be sane.
+     *
+     * @param currentData existing intent data in the store
+     * @param newData new intent data update proposal
+     * @return true if we can apply the update, otherwise false
+     */
+    public static boolean isUpdateAcceptable(IntentData currentData, IntentData newData) {
+
+        if (currentData == null) {
+            return true;
+        } else if (currentData.version().compareTo(newData.version()) < 0) {
+            return true;
+        } else if (currentData.version().compareTo(newData.version()) > 0) {
+            return false;
+        }
+
+        // current and new data versions are the same
+        IntentState currentState = currentData.state();
+        IntentState newState = newData.state();
+
+        switch (newState) {
+        case INSTALLING:
+            if (currentState == INSTALLING) {
+                return false;
+            }
+            // FALLTHROUGH
+        case INSTALLED:
+            if (currentState == INSTALLED) {
+                return false;
+            } else if (currentState == WITHDRAWING || currentState == WITHDRAWN
+                    || currentState == PURGE_REQ) {
+                log.warn("Invalid state transition from {} to {} for intent {}",
+                         currentState, newState, newData.key());
+                return false;
+            }
+            return true;
+
+        case WITHDRAWING:
+            if (currentState == WITHDRAWING) {
+                return false;
+            }
+            // FALLTHROUGH
+        case WITHDRAWN:
+            if (currentState == WITHDRAWN) {
+                return false;
+            } else if (currentState == INSTALLING || currentState == INSTALLED
+                    || currentState == PURGE_REQ) {
+                log.warn("Invalid state transition from {} to {} for intent {}",
+                         currentState, newState, newData.key());
+                return false;
+            }
+            return true;
+
+        case FAILED:
+            if (currentState == FAILED) {
+                return false;
+            }
+            return true;
+
+        case PURGE_REQ:
+            return true;
+
+        case COMPILING:
+        case RECOMPILING:
+        case INSTALL_REQ:
+        case WITHDRAW_REQ:
+        default:
+            log.warn("Invalid state {} for intent {}", newState, newData.key());
+            return false;
+        }
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(intent, version);

@@ -23,8 +23,10 @@ import org.onosproject.store.Timestamp;
 
 import com.google.common.testing.EqualsTester;
 
+import static junit.framework.TestCase.assertFalse;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertTrue;
 import static org.onosproject.net.intent.IntentTestsMocks.MockIntent;
 import static org.onosproject.net.intent.IntentTestsMocks.MockTimestamp;
 
@@ -97,4 +99,81 @@ public class IntentDataTest {
                 .addEqualityGroup(data3, data3Copy)
                 .testEquals();
     }
+
+    @Test
+    public void testIsUpdateAcceptable() {
+        // Going from null to something is always allowed
+        assertTrue(IntentData.isUpdateAcceptable(null, data1));
+
+        // we can go from older version to newer but not they other way
+        assertTrue(IntentData.isUpdateAcceptable(data1, data2));
+        assertFalse(IntentData.isUpdateAcceptable(data2, data1));
+
+        IntentData installing = new IntentData(intent1, IntentState.INSTALLING, timestamp1);
+        IntentData installed = new IntentData(intent1, IntentState.INSTALLED, timestamp1);
+        IntentData withdrawing = new IntentData(intent1, IntentState.WITHDRAWING, timestamp1);
+        IntentData withdrawn = new IntentData(intent1, IntentState.WITHDRAWN, timestamp1);
+
+        IntentData failed = new IntentData(intent1, IntentState.FAILED, timestamp1);
+        IntentData purgeReq = new IntentData(intent1, IntentState.PURGE_REQ, timestamp1);
+
+        IntentData compiling = new IntentData(intent1, IntentState.COMPILING, timestamp1);
+        IntentData recompiling = new IntentData(intent1, IntentState.RECOMPILING, timestamp1);
+        IntentData installReq = new IntentData(intent1, IntentState.INSTALL_REQ, timestamp1);
+        IntentData withdrawReq = new IntentData(intent1, IntentState.WITHDRAW_REQ, timestamp1);
+
+        // We can't change to the same state
+        assertFalse(IntentData.isUpdateAcceptable(installing, installing));
+        assertFalse(IntentData.isUpdateAcceptable(installed, installed));
+
+        // From installing we can change to installed
+        assertTrue(IntentData.isUpdateAcceptable(installing, installed));
+
+        // Sanity checks in case the manager submits bogus state transitions
+        assertFalse(IntentData.isUpdateAcceptable(installing, withdrawing));
+        assertFalse(IntentData.isUpdateAcceptable(installing, withdrawn));
+        assertFalse(IntentData.isUpdateAcceptable(installed, withdrawing));
+        assertFalse(IntentData.isUpdateAcceptable(installed, withdrawn));
+
+        // We can't change to the same state
+        assertFalse(IntentData.isUpdateAcceptable(withdrawing, withdrawing));
+        assertFalse(IntentData.isUpdateAcceptable(withdrawn, withdrawn));
+
+        // From withdrawing we can change to withdrawn
+        assertTrue(IntentData.isUpdateAcceptable(withdrawing, withdrawn));
+
+        // Sanity checks in case the manager submits bogus state transitions
+        assertFalse(IntentData.isUpdateAcceptable(withdrawing, installing));
+        assertFalse(IntentData.isUpdateAcceptable(withdrawing, installed));
+        assertFalse(IntentData.isUpdateAcceptable(withdrawn, installing));
+        assertFalse(IntentData.isUpdateAcceptable(withdrawn, installed));
+
+        // We can't go from failed to failed
+        assertFalse(IntentData.isUpdateAcceptable(failed, failed));
+
+        // But we can go from any install* or withdraw* state to failed
+        assertTrue(IntentData.isUpdateAcceptable(installing, failed));
+        assertTrue(IntentData.isUpdateAcceptable(installed, failed));
+        assertTrue(IntentData.isUpdateAcceptable(withdrawing, failed));
+        assertTrue(IntentData.isUpdateAcceptable(withdrawn, failed));
+
+        // We can go from anything to purgeReq
+        assertTrue(IntentData.isUpdateAcceptable(installing, purgeReq));
+        assertTrue(IntentData.isUpdateAcceptable(installed, purgeReq));
+        assertTrue(IntentData.isUpdateAcceptable(withdrawing, purgeReq));
+        assertTrue(IntentData.isUpdateAcceptable(withdrawn, purgeReq));
+        assertTrue(IntentData.isUpdateAcceptable(failed, purgeReq));
+
+        // We can't go from purgeReq back to anything else
+        assertFalse(IntentData.isUpdateAcceptable(purgeReq, withdrawn));
+        assertFalse(IntentData.isUpdateAcceptable(purgeReq, withdrawing));
+        assertFalse(IntentData.isUpdateAcceptable(purgeReq, installed));
+        assertFalse(IntentData.isUpdateAcceptable(purgeReq, installing));
+
+        // We're never allowed to store transient states
+        assertFalse(IntentData.isUpdateAcceptable(installing, compiling));
+        assertFalse(IntentData.isUpdateAcceptable(installing, recompiling));
+        assertFalse(IntentData.isUpdateAcceptable(installing, installReq));
+        assertFalse(IntentData.isUpdateAcceptable(installing, withdrawReq));
+    }
 }

@@ -454,9 +454,8 @@ public class IntentTestsMocks {
                 return -1;
             }
             MockTimestamp that = (MockTimestamp) o;
-            return (this.value > that.value ? -1 : (this.value == that.value ? 0 : 1));
+            return this.value - that.value;
         }
     }
 
-
 }

@@ -150,86 +150,14 @@ public class GossipIntentStore
         return null;
     }
 
-    /**
-     * Determines whether an intent data update is allowed. The update must
-     * either have a higher version than the current data, or the state
-     * transition between two updates of the same version must be sane.
-     *
-     * @param currentData existing intent data in the store
-     * @param newData new intent data update proposal
-     * @return true if we can apply the update, otherwise false
-     */
-    private boolean isUpdateAcceptable(IntentData currentData, IntentData newData) {
-
-        if (currentData == null) {
-            return true;
-        } else if (currentData.version().compareTo(newData.version()) < 0) {
-            return true;
-        } else if (currentData.version().compareTo(newData.version()) > 0) {
-            return false;
-        }
-
-        // current and new data versions are the same
-        IntentState currentState = currentData.state();
-        IntentState newState = newData.state();
-
-        switch (newState) {
-        case INSTALLING:
-            if (currentState == INSTALLING) {
-                return false;
-            }
-            // FALLTHROUGH
-        case INSTALLED:
-            if (currentState == INSTALLED) {
-                return false;
-            } else if (currentState == WITHDRAWING || currentState == WITHDRAWN) {
-                log.warn("Invalid state transition from {} to {} for intent {}",
-                         currentState, newState, newData.key());
-                return false;
-            }
-            return true;
-
-        case WITHDRAWING:
-            if (currentState == WITHDRAWING) {
-                return false;
-            }
-            // FALLTHROUGH
-        case WITHDRAWN:
-            if (currentState == WITHDRAWN) {
-                return false;
-            } else if (currentState == INSTALLING || currentState == INSTALLED) {
-                log.warn("Invalid state transition from {} to {} for intent {}",
-                         currentState, newState, newData.key());
-                return false;
-            }
-            return true;
-
 
-        case FAILED:
-            if (currentState == FAILED) {
-                return false;
-            }
-            return true;
-
-        case PURGE_REQ:
-            return true;
-
-        case COMPILING:
-        case RECOMPILING:
-        case INSTALL_REQ:
-        case WITHDRAW_REQ:
-        default:
-            log.warn("Invalid state {} for intent {}", newState, newData.key());
-            return false;
-        }
-    }
 
     @Override
     public void write(IntentData newData) {
         checkNotNull(newData);
 
         IntentData currentData = currentMap.get(newData.key());
-        if (isUpdateAcceptable(currentData, newData)) {
+        if (IntentData.isUpdateAcceptable(currentData, newData)) {
             // Only the master is modifying the current state. Therefore assume
             // this always succeeds
             if (newData.state() == PURGE_REQ) {

@@ -89,78 +89,6 @@ public class SimpleIntentStore
         return null;
     }
 
-    /**
-     * Determines whether an intent data update is allowed. The update must
-     * either have a higher version than the current data, or the state
-     * transition between two updates of the same version must be sane.
-     *
-     * @param currentData existing intent data in the store
-     * @param newData new intent data update proposal
-     * @return true if we can apply the update, otherwise false
-     */
-    private boolean isUpdateAcceptable(IntentData currentData, IntentData newData) {
-
-        if (currentData == null) {
-            return true;
-        } else if (currentData.version().compareTo(newData.version()) < 0) {
-            return true;
-        } else if (currentData.version().compareTo(newData.version()) > 0) {
-            return false;
-        }
-
-        // current and new data versions are the same
-        IntentState currentState = currentData.state();
-        IntentState newState = newData.state();
-
-        switch (newState) {
-            case INSTALLING:
-                if (currentState == INSTALLING) {
-                    return false;
-                }
-            // FALLTHROUGH
-            case INSTALLED:
-                if (currentState == INSTALLED) {
-                    return false;
-                } else if (currentState == WITHDRAWING || currentState == WITHDRAWN) {
-                    log.warn("Invalid state transition from {} to {} for intent {}",
-                             currentState, newState, newData.key());
-                    return false;
-                }
-                return true;
-
-            case WITHDRAWING:
-                if (currentState == WITHDRAWING) {
-                    return false;
-                }
-            // FALLTHOUGH
-            case WITHDRAWN:
-                if (currentState == WITHDRAWN) {
-                    return false;
-                } else if (currentState == INSTALLING || currentState == INSTALLED) {
-                    log.warn("Invalid state transition from {} to {} for intent {}",
-                             currentState, newState, newData.key());
-                    return false;
-                }
-                return true;
-
-
-            case FAILED:
-                if (currentState == FAILED) {
-                    return false;
-                }
-                return true;
-
-
-            case COMPILING:
-            case RECOMPILING:
-            case INSTALL_REQ:
-            case WITHDRAW_REQ:
-            default:
-                log.warn("Invalid state {} for intent {}", newState, newData.key());
-                return false;
-        }
-    }
-
     @Override
     public void write(IntentData newData) {
         checkNotNull(newData);
@@ -170,7 +98,7 @@ public class SimpleIntentStore
             IntentData currentData = current.get(newData.key());
             IntentData pendingData = pending.get(newData.key());
 
-            if (isUpdateAcceptable(currentData, newData)) {
+            if (IntentData.isUpdateAcceptable(currentData, newData)) {
                 if (pendingData.state() == PURGE_REQ) {
                     current.remove(newData.key(), newData);
                 } else {