[Blackbird] Fixing Intent purge case (ONOS-1207)

Master needs to remove from the current map.

Change-Id: I30eccbe188997949ef2d63d6dbd37b0d8d4b3f5e

@@ -85,35 +85,61 @@ public class IntentRemoveCommand extends AbstractShellCommand {
         Intent intent = intentService.getIntent(key);
 
         if (intent != null) {
-            // set up latch and listener to track uninstall progress
-            CountDownLatch latch = new CountDownLatch(1);
-            IntentListener listener = (IntentEvent event) -> {
-                if (Objects.equals(event.subject().key(), key) &&
-                        (event.type() == IntentEvent.Type.WITHDRAWN ||
-                         event.type() == IntentEvent.Type.FAILED)) {
-                    latch.countDown();
-                }
-            };
-            intentService.addListener(listener);
+            IntentListener listener = null;
+            final CountDownLatch withdrawLatch, purgeLatch;
+            if (purgeAfterRemove || sync) {
+                // set up latch and listener to track uninstall progress
+                withdrawLatch = new CountDownLatch(1);
+                purgeLatch = purgeAfterRemove ? new CountDownLatch(1) : null;
+                listener = (IntentEvent event) -> {
+                    if (Objects.equals(event.subject().key(), key)) {
+                        if (event.type() == IntentEvent.Type.WITHDRAWN ||
+                                event.type() == IntentEvent.Type.FAILED) {
+                            withdrawLatch.countDown();
+                        } else if (purgeAfterRemove &&
+                                event.type() == IntentEvent.Type.PURGED) {
+                            purgeLatch.countDown();
+                        }
+                    }
+                };
+                intentService.addListener(listener);
+            } else {
+                purgeLatch = null;
+                withdrawLatch = null;
+            }
 
             // request the withdraw
             intentService.withdraw(intent);
 
             if (purgeAfterRemove || sync) {
-                try {
-                    latch.await(5, TimeUnit.SECONDS);
+                try { // wait for withdraw event
+                    withdrawLatch.await(5, TimeUnit.SECONDS);
                 } catch (InterruptedException e) {
-                    print("Timed out waiting for intent {}", key);
+                    print("Timed out waiting for intent {} withdraw", key);
                 }
                 // double check the state
                 IntentState state = intentService.getIntentState(key);
                 if (purgeAfterRemove && (state == WITHDRAWN || state == FAILED)) {
-                    intentService.purge(key);
+                    intentService.purge(intent);
+                }
+                if (sync) { // wait for purge event
+                    /* TODO
+                       Technically, the event comes before map.remove() is called.
+                       If we depend on sync and purge working together, we will
+                       need to address this.
+                    */
+                    try {
+                        purgeLatch.await(5, TimeUnit.SECONDS);
+                    } catch (InterruptedException e) {
+                        print("Timed out waiting for intent {} purge", key);
+                    }
                 }
             }
-            // clean up the listener
-            intentService.removeListener(listener);
-        }
 
+            if (listener != null) {
+                // clean up the listener
+                intentService.removeListener(listener);
+            }
+        }
     }
 }

@@ -46,7 +46,12 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> {
         /**
          * Signifies that an intent has been withdrawn from the system.
          */
-        WITHDRAWN
+        WITHDRAWN,
+
+        /**
+         * Signifies that an intent has been purged from the system.
+         */
+        PURGED
     }
 
     /**
@@ -110,6 +115,9 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> {
             case FAILED:
                 type = Type.FAILED;
                 break;
+            case PURGE_REQ:
+                type = Type.PURGED;
+                break;
 
             // fallthrough to default from here
             case COMPILING:

@@ -43,6 +43,14 @@ public interface IntentService {
     void withdraw(Intent intent);
 
     /**
+     * Purges a specific intent from the system if it is <b>FAILED</b> or
+     * <b>WITHDRAWN</b>. Otherwise, the intent remains in its current state.
+     *
+     * @param intent intent to purge
+     */
+    void purge(Intent intent);
+
+    /**
      * Fetches an intent based on its key.
      *
      * @param key key of the intent
@@ -112,11 +120,4 @@ public interface IntentService {
      * @param listener listener to be removed
      */
     void removeListener(IntentListener listener);
-
-    /**
-     * Purges a specific intent from the system if it is FAILED or WITHDRAWN.
-     *
-     * @param key key of the intent to purge
-     */
-    void purge(Key key);
 }

@@ -92,5 +92,14 @@ public enum IntentState {
      * Signifies that the intent has failed compiling, installing or
      * recompiling states.
      */
-    FAILED //TODO consider renaming to UNSAT.
+    FAILED, //TODO consider renaming to UNSAT.
+
+    /**
+     * Indicates that the intent should be purged from the database.
+     * <p>
+     * Note: This operation will only be performed if the intent is already
+     * in WITHDRAWN or FAILED.
+     * </p>
+     */
+    PURGE_REQ
 }

@@ -109,10 +109,4 @@ public interface IntentStore extends Store<IntentEvent, IntentStoreDelegate> {
      * @return pending intents
      */
     Iterable<Intent> getPending();
-
-    /** Purges a specific intent from the system if it is FAILED or WITHDRAWN.
-     *
-     * @param key key of the intent to purge
-     */
-    void purge(Key key);
 }

@@ -186,6 +186,17 @@ public class FakeIntentManager implements TestableIntentService {
     }
 
     @Override
+    public void purge(Intent intent) {
+        IntentState currentState = intentStates.get(intent.key());
+        if (currentState == IntentState.WITHDRAWN
+                || currentState == IntentState.FAILED) {
+            intents.remove(intent.key());
+            installables.remove(intent.key());
+            intentStates.remove(intent.key());
+        }
+    }
+
+    @Override
     public Set<Intent> getIntents() {
         return Collections.unmodifiableSet(new HashSet<>(intents.values()));
     }
@@ -230,11 +241,6 @@ public class FakeIntentManager implements TestableIntentService {
         listeners.remove(listener);
     }
 
-    @Override
-    public void purge(Key key) {
-        // FIXME implement this
-    }
-
     private void dispatch(IntentEvent event) {
         for (IntentListener listener : listeners) {
             listener.event(event);

@@ -33,6 +33,11 @@ public class IntentServiceAdapter implements IntentService {
     }
 
     @Override
+    public void purge(Intent intent) {
+
+    }
+
+    @Override
     public Iterable<Intent> getIntents() {
         return null;
     }
@@ -76,9 +81,4 @@ public class IntentServiceAdapter implements IntentService {
     public void removeListener(IntentListener listener) {
 
     }
-
-    @Override
-    public void purge(Key key) {
-
-    }
 }

@@ -151,6 +151,13 @@ public class IntentManager
     }
 
     @Override
+    public void purge(Intent intent) {
+        checkNotNull(intent, INTENT_NULL);
+        IntentData data = new IntentData(intent, IntentState.PURGE_REQ, null);
+        store.addPending(data);
+    }
+
+    @Override
     public Intent getIntent(Key key) {
         return store.getIntent(key);
     }
@@ -193,11 +200,6 @@ public class IntentManager
     }
 
     @Override
-    public void purge(Key key) {
-        store.purge(key);
-    }
-
-    @Override
     public <T extends Intent> void registerCompiler(Class<T> cls, IntentCompiler<T> compiler) {
         compilerRegistry.registerCompiler(cls, compiler);
     }

@@ -56,6 +56,8 @@ public interface IntentProcessPhase {
                 } else {
                     return new WithdrawRequest(processor, data, current);
                 }
+            case PURGE_REQ:
+                return new PurgeRequest(data, current);
             default:
                 // illegal state
                 return new CompilingFailed(data);

+/*
+ * 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.impl.phase;
+
+import org.onosproject.net.intent.IntentData;
+import org.onosproject.net.intent.IntentState;
+import org.slf4j.Logger;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.slf4j.LoggerFactory.getLogger;
+
+/**
+ * Represents a phase of requesting a purge of an intent.
+ * <p>
+ * Note: The purge will only succeed if the intent is FAILED or WITHDRAWN.
+ * </p>
+ */
+final class PurgeRequest extends FinalIntentProcessPhase {
+
+    private static final Logger log = getLogger(PurgeRequest.class);
+
+    private final IntentData pending;
+    private final IntentData current;
+
+    PurgeRequest(IntentData intentData, IntentData current) {
+        this.pending = checkNotNull(intentData);
+        this.current = checkNotNull(current);
+    }
+
+    private boolean shouldAcceptPurge() {
+        if (current.state() == IntentState.WITHDRAWN
+                || current.state() == IntentState.FAILED) {
+            return true;
+        }
+        log.info("Purge for intent {} is rejected", pending.key());
+        return false;
+    }
+
+    @Override
+    public IntentData data() {
+        if (shouldAcceptPurge()) {
+            return pending;
+        } else {
+            return current;
+        }
+    }
+}

@@ -412,6 +412,11 @@ public class EventuallyConsistentMapImpl<K, V>
                         peerUpdateFunction.apply(key, value));
             notifyListeners(new EventuallyConsistentMapEvent<>(
                     EventuallyConsistentMapEvent.Type.REMOVE, key, value));
+        } else {
+            // TODO remove this extra call when ONOS-1207 is resolved
+            Timestamped<V> latest = (Timestamped) items.get(key);
+            log.info("Remove of intent {} failed; request time {} vs. latest time {}",
+                    key, timestamp, latest.timestamp());
         }
     }
 

@@ -224,6 +224,8 @@ public class GossipIntentStore
             }
             return true;
 
+        case PURGE_REQ:
+            return true;
 
         case COMPILING:
         case RECOMPILING:
@@ -241,7 +243,11 @@ public class GossipIntentStore
         if (isUpdateAcceptable(currentData, newData)) {
             // Only the master is modifying the current state. Therefore assume
             // this always succeeds
-            currentMap.put(newData.key(), copyData(newData));
+            if (newData.state() == PURGE_REQ) {
+                currentMap.remove(newData.key(), newData);
+            } else {
+                currentMap.put(newData.key(), copyData(newData));
+            }
 
             // if current.put succeeded
             pendingMap.remove(newData.key(), newData);
@@ -325,14 +331,6 @@ public class GossipIntentStore
                 .collect(Collectors.toList());
     }
 
-    @Override
-    public void purge(Key key) {
-        IntentData data = currentMap.get(key);
-        if (data.state() == WITHDRAWN || data.state() == FAILED) {
-            currentMap.remove(key, data);
-        }
-    }
-
     private void notifyDelegateIfNotNull(IntentEvent event) {
         if (event != null) {
             notifyDelegate(event);

@@ -180,7 +180,11 @@ public class SimpleIntentStore
             IntentData pendingData = pending.get(newData.key());
 
             if (isUpdateAcceptable(currentData, newData)) {
-                current.put(newData.key(), copyData(newData));
+                if (pendingData.state() == PURGE_REQ) {
+                    current.remove(newData.key(), newData);
+                } else {
+                    current.put(newData.key(), copyData(newData));
+                }
 
                 if (pendingData != null
                         // pendingData version is less than or equal to newData's
@@ -253,12 +257,4 @@ public class SimpleIntentStore
                 .map(IntentData::intent)
                 .collect(Collectors.toList());
     }
-
-    @Override
-    public void purge(Key key) {
-        IntentData data = current.get(key);
-        if (data.state() == IntentState.WITHDRAWN || data.state() == IntentState.FAILED) {
-            current.remove(key, data);
-        }
-    }
 }