Merge IntentInstaller's role into IntentCompiler
It resolves naming mismatch in naming of IntentProcessPhases and states
handled in the phases. It is described in ONOS-1064.
- Define FlowRuleIntent that enables flow rule level operation
as an intent.
- Remove IntentInstaller interface
- Existing installable intents such as PathIntent, LinkCollectionIntent,
OpticalPathIntent and MplsPathIntent now become non installable intents.
Only FlowRuleIntent is categorized as installable intent now.
- Implement intent compilers for PathIntent, LinkCollectionIntent,
OpticalPathIntent and MplsPathIntent. They generates FlowRuleIntents.
- Write unit tests for the newly created intent compilers according to
the intent installers' unit tests
- Remove all intent installers and their unit tests
Change-Id: I22d6c7acb65a4c066145de0018bd0727f44bd54a
diff --git a/core/api/src/main/java/org/onosproject/net/intent/FlowRuleIntent.java b/core/api/src/main/java/org/onosproject/net/intent/FlowRuleIntent.java
new file mode 100644
index 0000000..3f66f3b
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/intent/FlowRuleIntent.java
@@ -0,0 +1,87 @@
+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed 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.onosproject.net.intent;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.net.NetworkResource;
+import org.onosproject.net.flow.FlowRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * An intent that enables to tell flow level operation.
+ * This instance holds a collection of flow rules that may be executed in parallel.
+ */
+public class FlowRuleIntent extends Intent {
+
+ private final Collection<FlowRule> flowRules;
+
+ /**
+ * Creates an flow rule intent with the specified flow rules to be set.
+ *
+ * @param appId application id
+ * @param flowRules flow rules to be set.
+ */
+ public FlowRuleIntent(ApplicationId appId, List<FlowRule> flowRules) {
+ this(appId, null, flowRules, Collections.emptyList());
+ }
+
+ /**
+ * Creates an flow rule intent with the specified key, flow rules to be set, and
+ * required network resources.
+ *
+ * @param appId application id
+ * @param key key
+ * @param flowRules flow rules
+ * @param resources network resources
+ */
+ public FlowRuleIntent(ApplicationId appId, Key key, Collection<FlowRule> flowRules,
+ Collection<NetworkResource> resources) {
+ super(appId, key, resources, DEFAULT_INTENT_PRIORITY);
+ this.flowRules = ImmutableList.copyOf(checkNotNull(flowRules));
+ }
+
+ /**
+ * Returns a collection of flow rules to be set.
+ *
+ * @return a collection of flow rules
+ */
+ public Collection<FlowRule> flowRules() {
+ return flowRules;
+ }
+
+ @Override
+ public boolean isInstallable() {
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("id", id())
+ .add("key", key())
+ .add("appId", appId())
+ .add("resources", resources())
+ .add("flowRule", flowRules)
+ .toString();
+ }
+}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentExtensionService.java b/core/api/src/main/java/org/onosproject/net/intent/IntentExtensionService.java
index cefcb90..1d7c5ae 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentExtensionService.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentExtensionService.java
@@ -45,28 +45,4 @@
* @return the set of compiler bindings
*/
Map<Class<? extends Intent>, IntentCompiler<? extends Intent>> getCompilers();
-
- /**
- * Registers the specified installer for the given installable intent class.
- *
- * @param cls installable intent class
- * @param installer intent installer
- * @param <T> the type of installable intent
- */
- <T extends Intent> void registerInstaller(Class<T> cls, IntentInstaller<T> installer);
-
- /**
- * Unregisters the installer for the given installable intent class.
- *
- * @param cls installable intent class
- * @param <T> the type of installable intent
- */
- <T extends Intent> void unregisterInstaller(Class<T> cls);
-
- /**
- * Returns immutable set of bindings of currently registered intent installers.
- *
- * @return the set of installer bindings
- */
- Map<Class<? extends Intent>, IntentInstaller<? extends Intent>> getInstallers();
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java b/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
index 8cea253..bc12ab7 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
@@ -222,11 +222,6 @@
}
@Override
- public boolean isInstallable() {
- return true;
- }
-
- @Override
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("id", id())
diff --git a/core/api/src/main/java/org/onosproject/net/intent/OpticalPathIntent.java b/core/api/src/main/java/org/onosproject/net/intent/OpticalPathIntent.java
index 754d433..438f208 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/OpticalPathIntent.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/OpticalPathIntent.java
@@ -154,11 +154,6 @@
}
@Override
- public boolean isInstallable() {
- return true;
- }
-
- @Override
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("id", id())
diff --git a/core/api/src/main/java/org/onosproject/net/intent/PathIntent.java b/core/api/src/main/java/org/onosproject/net/intent/PathIntent.java
index 1cb960f..fd9789f 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/PathIntent.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/PathIntent.java
@@ -184,12 +184,6 @@
}
@Override
- public boolean isInstallable() {
- return true;
- }
-
-
- @Override
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("id", id())
diff --git a/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java b/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
index dca55d1..56ffb5d 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
@@ -37,8 +37,6 @@
private final Set<IntentListener> listeners = new HashSet<>();
private final Map<Class<? extends Intent>, IntentCompiler<? extends Intent>> compilers = new HashMap<>();
- private final Map<Class<? extends Intent>, IntentInstaller<? extends Intent>> installers
- = new HashMap<>();
private final ExecutorService executor = Executors.newSingleThreadExecutor();
private final List<IntentException> exceptions = new ArrayList<>();
@@ -88,16 +86,6 @@
return compiler;
}
- private <T extends Intent> IntentInstaller<T> getInstaller(T intent) {
- @SuppressWarnings("unchecked")
- IntentInstaller<T> installer = (IntentInstaller<T>) installers.get(intent
- .getClass());
- if (installer == null) {
- throw new IntentException("no installer for class " + intent.getClass());
- }
- return installer;
- }
-
private <T extends Intent> void executeCompilingPhase(T intent) {
setState(intent, IntentState.COMPILING);
try {
@@ -118,10 +106,6 @@
List<Intent> installable) {
setState(intent, IntentState.INSTALLING);
try {
- for (Intent ii : installable) {
- registerSubclassInstallerIfNeeded(ii);
- getInstaller(ii).install(ii);
- }
setState(intent, IntentState.INSTALLED);
putInstallable(intent.key(), installable);
dispatch(new IntentEvent(IntentEvent.Type.INSTALLED, intent));
@@ -136,9 +120,6 @@
List<Intent> installable) {
setState(intent, IntentState.WITHDRAWING);
try {
- for (Intent ii : installable) {
- getInstaller(ii).uninstall(ii);
- }
removeInstallable(intent.key());
setState(intent, IntentState.WITHDRAWN);
dispatch(new IntentEvent(IntentEvent.Type.WITHDRAWN, intent));
@@ -263,23 +244,6 @@
return Collections.unmodifiableMap(compilers);
}
- @Override
- public <T extends Intent> void registerInstaller(Class<T> cls,
- IntentInstaller<T> installer) {
- installers.put(cls, installer);
- }
-
- @Override
- public <T extends Intent> void unregisterInstaller(Class<T> cls) {
- installers.remove(cls);
- }
-
- @Override
- public Map<Class<? extends Intent>,
- IntentInstaller<? extends Intent>> getInstallers() {
- return Collections.unmodifiableMap(installers);
- }
-
private void registerSubclassCompilerIfNeeded(Intent intent) {
if (!compilers.containsKey(intent.getClass())) {
Class<?> cls = intent.getClass();
@@ -296,23 +260,4 @@
}
}
}
-
- private void registerSubclassInstallerIfNeeded(Intent intent) {
- if (!installers.containsKey(intent.getClass())) {
- Class<?> cls = intent.getClass();
- while (cls != Object.class) {
- // As long as we're within the Intent class
- // descendants
- if (Intent.class.isAssignableFrom(cls)) {
- IntentInstaller<?> installer = installers.get(cls);
- if (installer != null) {
- installers.put(intent.getClass(), installer);
- return;
- }
- }
- cls = cls.getSuperclass();
- }
- }
- }
-
}
diff --git a/core/api/src/test/java/org/onosproject/net/intent/IntentServiceTest.java b/core/api/src/test/java/org/onosproject/net/intent/IntentServiceTest.java
index c733933..58b21ef 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/IntentServiceTest.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/IntentServiceTest.java
@@ -19,12 +19,10 @@
import org.junit.Before;
import org.junit.Test;
import org.onosproject.core.IdGenerator;
-import org.onosproject.net.flow.FlowRuleOperation;
import org.onosproject.net.resource.LinkResourceAllocations;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@@ -76,7 +74,6 @@
// Register a compiler and an installer both setup for success.
service.registerCompiler(TestIntent.class, new TestCompiler(new TestInstallableIntent(INSTALLABLE_IID)));
- service.registerInstaller(TestInstallableIntent.class, new TestInstaller(false));
final Intent intent = new TestIntent(IID);
service.submit(intent);
@@ -143,29 +140,6 @@
validateEvents(intent, INSTALL_REQ, FAILED);
}
- @Test
- public void failedInstallation() {
- // Register a compiler programmed for success and installer for failure
- service.registerCompiler(TestIntent.class, new TestCompiler(new TestInstallableIntent(INSTALLABLE_IID)));
- service.registerInstaller(TestInstallableIntent.class, new TestInstaller(true));
-
- // Submit an intent
- final Intent intent = new TestIntent(IID);
- service.submit(intent);
-
- // Allow a small window of time until the intent is in the expected state
- TestTools.assertAfter(GRACE_MS, new Runnable() {
- @Override
- public void run() {
- assertEquals("incorrect intent state", IntentState.FAILED,
- service.getIntentState(intent.key()));
- }
- });
-
- // Make sure that all expected events have been emitted
- validateEvents(intent, INSTALL_REQ, FAILED);
- }
-
/**
* Validates that the test event listener has received the following events
* for the specified intent. Events received for other intents will not be
@@ -210,23 +184,6 @@
}
@Test
- public void installerBasics() {
- // Make sure there are no installers
- assertEquals("incorrect installer count", 0, service.getInstallers().size());
-
- // Add an installer and make sure that it appears in the map
- IntentInstaller<TestInstallableIntent> installer = new TestInstaller(false);
- service.registerInstaller(TestInstallableIntent.class, installer);
- assertEquals("incorrect installer", installer,
- service.getInstallers().get(TestInstallableIntent.class));
-
- // Remove the same and make sure that it no longer appears in the map
- service.unregisterInstaller(TestInstallableIntent.class);
- assertNull("installer should not be registered",
- service.getInstallers().get(TestInstallableIntent.class));
- }
-
- @Test
public void implicitRegistration() {
// Add a compiler and make sure that it appears in the map
IntentCompiler<TestIntent> compiler = new TestCompiler(new TestSubclassInstallableIntent(INSTALLABLE_IID));
@@ -234,13 +191,6 @@
assertEquals("incorrect compiler", compiler,
service.getCompilers().get(TestIntent.class));
- // Add a installer and make sure that it appears in the map
- IntentInstaller<TestInstallableIntent> installer = new TestInstaller(false);
- service.registerInstaller(TestInstallableIntent.class, installer);
- assertEquals("incorrect installer", installer,
- service.getInstallers().get(TestInstallableIntent.class));
-
-
// Submit an intent which is a subclass of the one we registered
final Intent intent = new TestSubclassIntent(IID);
service.submit(intent);
@@ -259,11 +209,6 @@
assertEquals("incorrect compiler", compiler,
service.getCompilers().get(TestSubclassIntent.class));
- // Make sure that now we have an implicit registration of the installer
- // under the intent subclass
- assertEquals("incorrect installer", installer,
- service.getInstallers().get(TestSubclassInstallableIntent.class));
-
// TODO: discuss whether or if implicit registration should require implicit unregistration
// perhaps unregister by compiler or installer itself, rather than by class would be better
}
@@ -304,36 +249,4 @@
return compiled;
}
}
-
- // Controllable installer
- private class TestInstaller implements IntentInstaller<TestInstallableIntent> {
- private final boolean fail;
-
- TestInstaller(boolean fail) {
- this.fail = fail;
- }
-
- @Override
- public List<Collection<FlowRuleOperation>> install(TestInstallableIntent intent) {
- if (fail) {
- throw new IntentException("install failed by design");
- }
- return null;
- }
-
- @Override
- public List<Collection<FlowRuleOperation>> uninstall(TestInstallableIntent intent) {
- if (fail) {
- throw new IntentException("remove failed by design");
- }
- return null;
- }
-
- @Override
- public List<Collection<FlowRuleOperation>> replace(TestInstallableIntent intent,
- TestInstallableIntent newIntent) {
- return null;
- }
- }
-
}
diff --git a/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java b/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
index 4263531..9d1028c 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
@@ -113,7 +113,7 @@
final Set<Link> createdLinks = collectionIntent.links();
assertThat(createdLinks, hasSize(1));
- assertThat(collectionIntent.isInstallable(), is(true));
+ assertThat(collectionIntent.isInstallable(), is(false));
assertThat(collectionIntent.treatment(), is(treatment));
assertThat(collectionIntent.selector(), is(selector));
assertThat(collectionIntent.ingressPoints(), is(ImmutableSet.of(ingress)));
@@ -147,7 +147,7 @@
final Set<Link> createdLinks = collectionIntent.links();
assertThat(createdLinks, hasSize(1));
- assertThat(collectionIntent.isInstallable(), is(true));
+ assertThat(collectionIntent.isInstallable(), is(false));
assertThat(collectionIntent.treatment(), is(treatment));
assertThat(collectionIntent.selector(), is(selector));
assertThat(collectionIntent.ingressPoints(), is(ImmutableSet.of(ingress)));
@@ -169,7 +169,7 @@
final Set<Link> createdLinks = collectionIntent.links();
assertThat(createdLinks, nullValue());
- assertThat(collectionIntent.isInstallable(), is(true));
+ assertThat(collectionIntent.isInstallable(), is(false));
assertThat(collectionIntent.treatment(), nullValue());
assertThat(collectionIntent.selector(), nullValue());
assertThat(collectionIntent.ingressPoints(), nullValue());