Committed by
Brian O'Connor
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
Showing
7 changed files
with
107 additions
and
39 deletions
... | @@ -46,7 +46,7 @@ class Compiling implements IntentUpdate { | ... | @@ -46,7 +46,7 @@ class Compiling implements IntentUpdate { |
46 | try { | 46 | try { |
47 | List<Intent> installables = (current != null) ? current.installables() : null; | 47 | List<Intent> installables = (current != null) ? current.installables() : null; |
48 | pending.setInstallables(intentManager.compileIntent(pending.intent(), installables)); | 48 | pending.setInstallables(intentManager.compileIntent(pending.intent(), installables)); |
49 | - return Optional.of(new Coordinating(intentManager, pending, current)); | 49 | + return Optional.of(new InstallCoordinating(intentManager, pending, current)); |
50 | } catch (PathNotFoundException e) { | 50 | } catch (PathNotFoundException e) { |
51 | log.debug("Path not found for intent {}", pending.intent()); | 51 | log.debug("Path not found for intent {}", pending.intent()); |
52 | // TODO: revisit to implement failure handling | 52 | // TODO: revisit to implement failure handling | ... | ... |
... | @@ -24,17 +24,20 @@ import java.util.Optional; | ... | @@ -24,17 +24,20 @@ import java.util.Optional; |
24 | 24 | ||
25 | import static com.google.common.base.Preconditions.checkNotNull; | 25 | import static com.google.common.base.Preconditions.checkNotNull; |
26 | 26 | ||
27 | -// TODO: better naming because install() method actually generate FlowRuleBatchOperations | 27 | +/** |
28 | -class Coordinating implements IntentUpdate { | 28 | + * Represents a phase to create a {@link FlowRuleOperations} instance |
29 | + * with using registered intent installers. | ||
30 | + */ | ||
31 | +class InstallCoordinating implements IntentUpdate { | ||
29 | 32 | ||
30 | - private static final Logger log = LoggerFactory.getLogger(Coordinating.class); | 33 | + private static final Logger log = LoggerFactory.getLogger(InstallCoordinating.class); |
31 | 34 | ||
32 | private final IntentManager intentManager; | 35 | private final IntentManager intentManager; |
33 | private final IntentData pending; | 36 | private final IntentData pending; |
34 | private final IntentData current; | 37 | private final IntentData current; |
35 | 38 | ||
36 | // TODO: define an interface and use it, instead of IntentManager | 39 | // TODO: define an interface and use it, instead of IntentManager |
37 | - Coordinating(IntentManager intentManager, IntentData pending, IntentData current) { | 40 | + InstallCoordinating(IntentManager intentManager, IntentData pending, IntentData current) { |
38 | this.intentManager = checkNotNull(intentManager); | 41 | this.intentManager = checkNotNull(intentManager); |
39 | this.pending = checkNotNull(pending); | 42 | this.pending = checkNotNull(pending); |
40 | this.current = current; | 43 | this.current = current; | ... | ... |
... | @@ -24,7 +24,10 @@ import java.util.Optional; | ... | @@ -24,7 +24,10 @@ import java.util.Optional; |
24 | 24 | ||
25 | import static com.google.common.base.Preconditions.checkNotNull; | 25 | import static com.google.common.base.Preconditions.checkNotNull; |
26 | 26 | ||
27 | -// TODO: better naming because install() method actually generate FlowRuleBatchOperations | 27 | +/** |
28 | + * Represents a phase of installing an intent with calling | ||
29 | + * {@link org.onosproject.net.flow.FlowRuleService}. | ||
30 | + */ | ||
28 | class Installing implements IntentUpdate { | 31 | class Installing implements IntentUpdate { |
29 | 32 | ||
30 | private static final Logger log = LoggerFactory.getLogger(Installing.class); | 33 | private static final Logger log = LoggerFactory.getLogger(Installing.class); | ... | ... |
... | @@ -17,7 +17,6 @@ package org.onosproject.net.intent.impl; | ... | @@ -17,7 +17,6 @@ package org.onosproject.net.intent.impl; |
17 | 17 | ||
18 | import com.google.common.collect.ImmutableList; | 18 | import com.google.common.collect.ImmutableList; |
19 | import com.google.common.collect.ImmutableMap; | 19 | import com.google.common.collect.ImmutableMap; |
20 | -import com.google.common.collect.Lists; | ||
21 | import org.apache.felix.scr.annotations.Activate; | 20 | import org.apache.felix.scr.annotations.Activate; |
22 | import org.apache.felix.scr.annotations.Component; | 21 | import org.apache.felix.scr.annotations.Component; |
23 | import org.apache.felix.scr.annotations.Deactivate; | 22 | import org.apache.felix.scr.annotations.Deactivate; |
... | @@ -316,6 +315,42 @@ public class IntentManager | ... | @@ -316,6 +315,42 @@ public class IntentManager |
316 | }); | 315 | }); |
317 | } | 316 | } |
318 | 317 | ||
318 | + /** | ||
319 | + * Generate a {@link FlowRuleOperations} instance from the specified intent data. | ||
320 | + * | ||
321 | + * @param current intent data stored in the store | ||
322 | + * @return flow rule operations | ||
323 | + */ | ||
324 | + FlowRuleOperations uninstallCoordinate(IntentData current) { | ||
325 | + List<Intent> installables = current.installables(); | ||
326 | + List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(); | ||
327 | + for (Intent installable : installables) { | ||
328 | + try { | ||
329 | + plans.add(getInstaller(installable).uninstall(installable)); | ||
330 | + } catch (IntentException e) { | ||
331 | + log.warn("Unable to uninstall intent {} due to:", current.intent().id(), e); | ||
332 | + throw new FlowRuleBatchOperationConversionException(null/*FIXME*/, e); | ||
333 | + } | ||
334 | + } | ||
335 | + | ||
336 | + return merge(plans).build(new FlowRuleOperationsContext() { | ||
337 | + @Override | ||
338 | + public void onSuccess(FlowRuleOperations ops) { | ||
339 | + log.info("Completed withdrawing: {}", current.key()); | ||
340 | + current.setState(WITHDRAWN); | ||
341 | + store.write(current); | ||
342 | + } | ||
343 | + | ||
344 | + @Override | ||
345 | + public void onError(FlowRuleOperations ops) { | ||
346 | + log.warn("Failed withdraw: {}", current.key()); | ||
347 | + current.setState(FAILED); | ||
348 | + store.write(current); | ||
349 | + } | ||
350 | + }); | ||
351 | + } | ||
352 | + | ||
353 | + | ||
319 | // FIXME... needs tests... or maybe it's just perfect | 354 | // FIXME... needs tests... or maybe it's just perfect |
320 | private FlowRuleOperations.Builder merge(List<List<FlowRuleBatchOperation>> plans) { | 355 | private FlowRuleOperations.Builder merge(List<List<FlowRuleBatchOperation>> plans) { |
321 | FlowRuleOperations.Builder builder = FlowRuleOperations.builder(); | 356 | FlowRuleOperations.Builder builder = FlowRuleOperations.builder(); |
... | @@ -358,30 +393,6 @@ public class IntentManager | ... | @@ -358,30 +393,6 @@ public class IntentManager |
358 | } | 393 | } |
359 | 394 | ||
360 | /** | 395 | /** |
361 | - * Uninstalls all installable intents associated with the given intent. | ||
362 | - * | ||
363 | - * @param intent intent | ||
364 | - * @param installables installable intents | ||
365 | - * @return list of batches to uninstall intent | ||
366 | - */ | ||
367 | - //FIXME | ||
368 | - FlowRuleOperations uninstallIntent(Intent intent, List<Intent> installables) { | ||
369 | - List<FlowRuleBatchOperation> batches = Lists.newArrayList(); | ||
370 | - for (Intent installable : installables) { | ||
371 | - trackerService.removeTrackedResources(intent.id(), | ||
372 | - installable.resources()); | ||
373 | - try { | ||
374 | - // FIXME need to aggregate the FlowRuleOperations across installables | ||
375 | - getInstaller(installable).uninstall(installable); //.build(null/*FIXME*/); | ||
376 | - } catch (IntentException e) { | ||
377 | - log.warn("Unable to uninstall intent {} due to:", intent.id(), e); | ||
378 | - // TODO: this should never happen. but what if it does? | ||
379 | - } | ||
380 | - } | ||
381 | - return null; //FIXME | ||
382 | - } | ||
383 | - | ||
384 | - /** | ||
385 | * Registers an intent compiler of the specified intent if an intent compiler | 396 | * Registers an intent compiler of the specified intent if an intent compiler |
386 | * for the intent is not registered. This method traverses the class hierarchy of | 397 | * for the intent is not registered. This method traverses the class hierarchy of |
387 | * the intent. Once an intent compiler for a parent type is found, this method | 398 | * the intent. Once an intent compiler for a parent type is found, this method | ... | ... |
1 | +/* | ||
2 | + * Copyright 2015 Open Networking Laboratory | ||
3 | + * | ||
4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | + * you may not use this file except in compliance with the License. | ||
6 | + * You may obtain a copy of the License at | ||
7 | + * | ||
8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | + * | ||
10 | + * Unless required by applicable law or agreed to in writing, software | ||
11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | + * See the License for the specific language governing permissions and | ||
14 | + * limitations under the License. | ||
15 | + */ | ||
16 | +package org.onosproject.net.intent.impl; | ||
17 | + | ||
18 | +import org.onosproject.net.flow.FlowRuleOperations; | ||
19 | +import org.onosproject.net.intent.IntentData; | ||
20 | + | ||
21 | +import java.util.Optional; | ||
22 | + | ||
23 | +import static com.google.common.base.Preconditions.checkNotNull; | ||
24 | + | ||
25 | +/** | ||
26 | + * Represents a phase to create a {@link FlowRuleOperations} instance | ||
27 | + * with using registered intent installers. | ||
28 | + */ | ||
29 | +class WithdrawCoordinating implements IntentUpdate { | ||
30 | + | ||
31 | + // TODO: define an interface and use it, instead of IntentManager | ||
32 | + private final IntentManager intentManager; | ||
33 | + private final IntentData pending; | ||
34 | + private final IntentData current; | ||
35 | + | ||
36 | + WithdrawCoordinating(IntentManager intentManager, IntentData pending, IntentData current) { | ||
37 | + this.intentManager = checkNotNull(intentManager); | ||
38 | + this.pending = checkNotNull(pending); | ||
39 | + this.current = checkNotNull(current); | ||
40 | + } | ||
41 | + | ||
42 | + @Override | ||
43 | + public Optional<IntentUpdate> execute() { | ||
44 | + FlowRuleOperations flowRules = intentManager.uninstallCoordinate(current); | ||
45 | + return Optional.of(new Withdrawing(intentManager, pending, flowRules)); | ||
46 | + } | ||
47 | +} |
... | @@ -21,6 +21,9 @@ import java.util.Optional; | ... | @@ -21,6 +21,9 @@ import java.util.Optional; |
21 | 21 | ||
22 | import static com.google.common.base.Preconditions.checkNotNull; | 22 | import static com.google.common.base.Preconditions.checkNotNull; |
23 | 23 | ||
24 | +/** | ||
25 | + * Represents a phase of requesting a withdraw of an intent. | ||
26 | + */ | ||
24 | class WithdrawRequest implements IntentUpdate { | 27 | class WithdrawRequest implements IntentUpdate { |
25 | 28 | ||
26 | // TODO: define an interface and use it, instead of IntentManager | 29 | // TODO: define an interface and use it, instead of IntentManager |
... | @@ -39,6 +42,6 @@ class WithdrawRequest implements IntentUpdate { | ... | @@ -39,6 +42,6 @@ class WithdrawRequest implements IntentUpdate { |
39 | //TODO perhaps we want to validate that the pending and current are the | 42 | //TODO perhaps we want to validate that the pending and current are the |
40 | // same version i.e. they are the same | 43 | // same version i.e. they are the same |
41 | // Note: this call is not just the symmetric version of submit | 44 | // Note: this call is not just the symmetric version of submit |
42 | - return Optional.of(new Withdrawing(intentManager, pending, current)); | 45 | + return Optional.of(new WithdrawCoordinating(intentManager, pending, current)); |
43 | } | 46 | } |
44 | } | 47 | } | ... | ... |
... | @@ -22,25 +22,26 @@ import java.util.Optional; | ... | @@ -22,25 +22,26 @@ import java.util.Optional; |
22 | 22 | ||
23 | import static com.google.common.base.Preconditions.checkNotNull; | 23 | import static com.google.common.base.Preconditions.checkNotNull; |
24 | 24 | ||
25 | +/** | ||
26 | + * Represents a phase of withdrawing an intent with calling | ||
27 | + * {@link import org.onosproject.net.flow.FlowRuleService}. | ||
28 | + */ | ||
25 | class Withdrawing implements IntentUpdate { | 29 | class Withdrawing implements IntentUpdate { |
26 | 30 | ||
27 | // TODO: define an interface and use it, instead of IntentManager | 31 | // TODO: define an interface and use it, instead of IntentManager |
28 | private final IntentManager intentManager; | 32 | private final IntentManager intentManager; |
29 | private final IntentData pending; | 33 | private final IntentData pending; |
30 | - private final IntentData current; | 34 | + private final FlowRuleOperations flowRules; |
31 | 35 | ||
32 | - Withdrawing(IntentManager intentManager, IntentData pending, IntentData current) { | 36 | + Withdrawing(IntentManager intentManager, IntentData pending, FlowRuleOperations flowRules) { |
33 | this.intentManager = checkNotNull(intentManager); | 37 | this.intentManager = checkNotNull(intentManager); |
34 | this.pending = checkNotNull(pending); | 38 | this.pending = checkNotNull(pending); |
35 | - this.current = checkNotNull(current); | 39 | + this.flowRules = checkNotNull(flowRules); |
36 | } | 40 | } |
37 | 41 | ||
38 | @Override | 42 | @Override |
39 | public Optional<IntentUpdate> execute() { | 43 | public Optional<IntentUpdate> execute() { |
40 | - FlowRuleOperations flowRules | 44 | + intentManager.flowRuleService.apply(flowRules); |
41 | - = intentManager.uninstallIntent(current.intent(), current.installables()); | ||
42 | - intentManager.flowRuleService.apply(flowRules); //FIXME | ||
43 | - | ||
44 | return Optional.of(new Withdrawn(pending)); | 45 | return Optional.of(new Withdrawn(pending)); |
45 | } | 46 | } |
46 | } | 47 | } | ... | ... |
-
Please register or login to post a comment