Split Withdrawing class into two classes WithdrawCoordinating and Withdrawing
Resolve ONOS-992
- Rename Coordinating to InstallCoordinating for naming consistency
- Add Javadoc to newly added/modified classes
- Rename uninstallIntent() to uninstallCoordinate() in IntentManager
- Rename coordinate() to installCoordinate() in IntentManager
Change-Id: I226592d96cfd8caf248addab2db0d28803c7ca12
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/Compiling.java b/core/net/src/main/java/org/onosproject/net/intent/impl/Compiling.java
index eef8d0b..11f9a28 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/Compiling.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/Compiling.java
@@ -46,7 +46,7 @@
try {
List<Intent> installables = (current != null) ? current.installables() : null;
pending.setInstallables(intentManager.compileIntent(pending.intent(), installables));
- return Optional.of(new Coordinating(intentManager, pending, current));
+ return Optional.of(new InstallCoordinating(intentManager, pending, current));
} catch (PathNotFoundException e) {
log.debug("Path not found for intent {}", pending.intent());
// TODO: revisit to implement failure handling
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java b/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinating.java
similarity index 82%
rename from core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java
rename to core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinating.java
index 3682dbf..e21b2fa 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinating.java
@@ -24,17 +24,20 @@
import static com.google.common.base.Preconditions.checkNotNull;
-// TODO: better naming because install() method actually generate FlowRuleBatchOperations
-class Coordinating implements IntentUpdate {
+/**
+ * Represents a phase to create a {@link FlowRuleOperations} instance
+ * with using registered intent installers.
+ */
+class InstallCoordinating implements IntentUpdate {
- private static final Logger log = LoggerFactory.getLogger(Coordinating.class);
+ private static final Logger log = LoggerFactory.getLogger(InstallCoordinating.class);
private final IntentManager intentManager;
private final IntentData pending;
private final IntentData current;
// TODO: define an interface and use it, instead of IntentManager
- Coordinating(IntentManager intentManager, IntentData pending, IntentData current) {
+ InstallCoordinating(IntentManager intentManager, IntentData pending, IntentData current) {
this.intentManager = checkNotNull(intentManager);
this.pending = checkNotNull(pending);
this.current = current;
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/Installing.java b/core/net/src/main/java/org/onosproject/net/intent/impl/Installing.java
index c8d0294..205345c 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/Installing.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/Installing.java
@@ -24,7 +24,10 @@
import static com.google.common.base.Preconditions.checkNotNull;
-// TODO: better naming because install() method actually generate FlowRuleBatchOperations
+/**
+ * Represents a phase of installing an intent with calling
+ * {@link org.onosproject.net.flow.FlowRuleService}.
+ */
class Installing implements IntentUpdate {
private static final Logger log = LoggerFactory.getLogger(Installing.class);
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
index 751f7ad..8fbf252 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
@@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -316,6 +315,42 @@
});
}
+ /**
+ * Generate a {@link FlowRuleOperations} instance from the specified intent data.
+ *
+ * @param current intent data stored in the store
+ * @return flow rule operations
+ */
+ FlowRuleOperations uninstallCoordinate(IntentData current) {
+ List<Intent> installables = current.installables();
+ List<List<FlowRuleBatchOperation>> plans = new ArrayList<>();
+ for (Intent installable : installables) {
+ try {
+ plans.add(getInstaller(installable).uninstall(installable));
+ } catch (IntentException e) {
+ log.warn("Unable to uninstall intent {} due to:", current.intent().id(), e);
+ throw new FlowRuleBatchOperationConversionException(null/*FIXME*/, e);
+ }
+ }
+
+ return merge(plans).build(new FlowRuleOperationsContext() {
+ @Override
+ public void onSuccess(FlowRuleOperations ops) {
+ log.info("Completed withdrawing: {}", current.key());
+ current.setState(WITHDRAWN);
+ store.write(current);
+ }
+
+ @Override
+ public void onError(FlowRuleOperations ops) {
+ log.warn("Failed withdraw: {}", current.key());
+ current.setState(FAILED);
+ store.write(current);
+ }
+ });
+ }
+
+
// FIXME... needs tests... or maybe it's just perfect
private FlowRuleOperations.Builder merge(List<List<FlowRuleBatchOperation>> plans) {
FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
@@ -358,30 +393,6 @@
}
/**
- * Uninstalls all installable intents associated with the given intent.
- *
- * @param intent intent
- * @param installables installable intents
- * @return list of batches to uninstall intent
- */
- //FIXME
- FlowRuleOperations uninstallIntent(Intent intent, List<Intent> installables) {
- List<FlowRuleBatchOperation> batches = Lists.newArrayList();
- for (Intent installable : installables) {
- trackerService.removeTrackedResources(intent.id(),
- installable.resources());
- try {
- // FIXME need to aggregate the FlowRuleOperations across installables
- getInstaller(installable).uninstall(installable); //.build(null/*FIXME*/);
- } catch (IntentException e) {
- log.warn("Unable to uninstall intent {} due to:", intent.id(), e);
- // TODO: this should never happen. but what if it does?
- }
- }
- return null; //FIXME
- }
-
- /**
* Registers an intent compiler of the specified intent if an intent compiler
* for the intent is not registered. This method traverses the class hierarchy of
* the intent. Once an intent compiler for a parent type is found, this method
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java b/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawCoordinating.java
similarity index 60%
copy from core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java
copy to core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawCoordinating.java
index 3682dbf..9dd1019 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/Coordinating.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawCoordinating.java
@@ -17,37 +17,31 @@
import org.onosproject.net.flow.FlowRuleOperations;
import org.onosproject.net.intent.IntentData;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import java.util.Optional;
import static com.google.common.base.Preconditions.checkNotNull;
-// TODO: better naming because install() method actually generate FlowRuleBatchOperations
-class Coordinating implements IntentUpdate {
+/**
+ * Represents a phase to create a {@link FlowRuleOperations} instance
+ * with using registered intent installers.
+ */
+class WithdrawCoordinating implements IntentUpdate {
- private static final Logger log = LoggerFactory.getLogger(Coordinating.class);
-
+ // TODO: define an interface and use it, instead of IntentManager
private final IntentManager intentManager;
private final IntentData pending;
private final IntentData current;
- // TODO: define an interface and use it, instead of IntentManager
- Coordinating(IntentManager intentManager, IntentData pending, IntentData current) {
+ WithdrawCoordinating(IntentManager intentManager, IntentData pending, IntentData current) {
this.intentManager = checkNotNull(intentManager);
this.pending = checkNotNull(pending);
- this.current = current;
+ this.current = checkNotNull(current);
}
@Override
public Optional<IntentUpdate> execute() {
- try {
- FlowRuleOperations flowRules = intentManager.coordinate(pending);
- return Optional.of(new Installing(intentManager, pending, flowRules));
- } catch (FlowRuleBatchOperationConversionException e) {
- log.warn("Unable to install intent {} due to:", pending.intent().id(), e.getCause());
- return Optional.of(new InstallingFailed(pending)); //FIXME
- }
+ FlowRuleOperations flowRules = intentManager.uninstallCoordinate(current);
+ return Optional.of(new Withdrawing(intentManager, pending, flowRules));
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawRequest.java b/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawRequest.java
index 1a2da58..f58a7a8 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawRequest.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/WithdrawRequest.java
@@ -21,6 +21,9 @@
import static com.google.common.base.Preconditions.checkNotNull;
+/**
+ * Represents a phase of requesting a withdraw of an intent.
+ */
class WithdrawRequest implements IntentUpdate {
// TODO: define an interface and use it, instead of IntentManager
@@ -39,6 +42,6 @@
//TODO perhaps we want to validate that the pending and current are the
// same version i.e. they are the same
// Note: this call is not just the symmetric version of submit
- return Optional.of(new Withdrawing(intentManager, pending, current));
+ return Optional.of(new WithdrawCoordinating(intentManager, pending, current));
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/Withdrawing.java b/core/net/src/main/java/org/onosproject/net/intent/impl/Withdrawing.java
index de91bc2..858af7c 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/Withdrawing.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/Withdrawing.java
@@ -22,25 +22,26 @@
import static com.google.common.base.Preconditions.checkNotNull;
+/**
+ * Represents a phase of withdrawing an intent with calling
+ * {@link import org.onosproject.net.flow.FlowRuleService}.
+ */
class Withdrawing implements IntentUpdate {
// TODO: define an interface and use it, instead of IntentManager
private final IntentManager intentManager;
private final IntentData pending;
- private final IntentData current;
+ private final FlowRuleOperations flowRules;
- Withdrawing(IntentManager intentManager, IntentData pending, IntentData current) {
+ Withdrawing(IntentManager intentManager, IntentData pending, FlowRuleOperations flowRules) {
this.intentManager = checkNotNull(intentManager);
this.pending = checkNotNull(pending);
- this.current = checkNotNull(current);
+ this.flowRules = checkNotNull(flowRules);
}
@Override
public Optional<IntentUpdate> execute() {
- FlowRuleOperations flowRules
- = intentManager.uninstallIntent(current.intent(), current.installables());
- intentManager.flowRuleService.apply(flowRules); //FIXME
-
+ intentManager.flowRuleService.apply(flowRules);
return Optional.of(new Withdrawn(pending));
}
}