Moved duplicated isUpdateAcceptable method to IntentData
and wrote a unit test for it.
Change-Id: I8b38476c41fba70abbba7ed0b37364696f17966d
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentData.java b/core/api/src/main/java/org/onosproject/net/intent/IntentData.java
index a71b20e..63ce928 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentData.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentData.java
@@ -19,11 +19,19 @@
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 @@
*/
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 @@
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);
diff --git a/core/api/src/test/java/org/onosproject/net/intent/IntentDataTest.java b/core/api/src/test/java/org/onosproject/net/intent/IntentDataTest.java
index a953149..9c4cf7e 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/IntentDataTest.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/IntentDataTest.java
@@ -23,8 +23,10 @@
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 @@
.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));
+ }
}
diff --git a/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java b/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
index 14abd83..8bef293 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
@@ -454,9 +454,8 @@
return -1;
}
MockTimestamp that = (MockTimestamp) o;
- return (this.value > that.value ? -1 : (this.value == that.value ? 0 : 1));
+ return this.value - that.value;
}
}
-
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
index 9a8f9e7..370b0b5 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
@@ -150,86 +150,14 @@
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) {
diff --git a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
index 7ef6247..e94a7f7 100644
--- a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
+++ b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
@@ -89,78 +89,6 @@
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 @@
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 {