[VOL-2525] Fixing the OLT app meter removal and optimizing the bandwith profile to meter map with a Multimap

Change-Id: I62b1fe58770df8f1a9e73a4b5bc1dec751d47ad9
diff --git a/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java b/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
index 630ae4a..bf00b92 100644
--- a/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
@@ -22,7 +22,9 @@
 import org.opencord.olt.internalapi.AccessDeviceMeterService;
 
 import java.util.Map;
-import java.util.Set;
+import java.util.Collection;
+
+
 
 @Command(scope = "onos", name = "volt-bpmeter-mappings",
         description = "Shows information about bandwidthProfile-meterKey (device / meter) mappings")
@@ -31,11 +33,11 @@
     @Override
     protected void execute() {
         AccessDeviceMeterService service = AbstractShellCommand.get(AccessDeviceMeterService.class);
-        Map<String, Set<MeterKey>> bpMeterMappings = service.getBpMeterMappings();
+        Map<String, Collection<MeterKey>> bpMeterMappings = service.getBpMeterMappings();
         bpMeterMappings.forEach(this::display);
     }
 
-    private void display(String bpInfo, Set<MeterKey> meterKeyList) {
+    private void display(String bpInfo, Collection<MeterKey> meterKeyList) {
         meterKeyList.forEach(meterKey ->
                 print("bpInfo=%s deviceId=%s meterId=%s",
                         bpInfo, meterKey.deviceId(), meterKey.meterId()));
diff --git a/app/src/main/java/org/opencord/olt/impl/OltMeterService.java b/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
index df434eb..3fff30c 100644
--- a/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
+++ b/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
@@ -15,9 +15,12 @@
  */
 package org.opencord.olt.impl;
 
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
+import com.google.common.collect.SetMultimap;
 import com.google.common.collect.Sets;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -51,7 +54,7 @@
 import org.slf4j.Logger;
 
 import java.util.ArrayList;
-
+import java.util.Collection;
 import java.util.Dictionary;
 import java.util.Iterator;
 import java.util.List;
@@ -68,6 +71,9 @@
 import static org.onlab.util.Tools.groupedThreads;
 import static org.slf4j.LoggerFactory.getLogger;
 
+/**
+ * Provisions Meters on access devices.
+ */
 @Service
 @Component(immediate = true)
 public class OltMeterService implements AccessDeviceMeterService {
@@ -85,7 +91,8 @@
             label = "Deleting Meters based on flow count statistics")
     protected boolean deleteMeters = true;
 
-    protected Map<String, Set<MeterKey>> bpInfoToMeter;
+    protected SetMultimap<String, MeterKey> bpInfoToMeter =
+            Multimaps.synchronizedSetMultimap(HashMultimap.create());
     protected Set<MeterKey> programmedMeters;
     private ApplicationId appId;
     private static final String APP_NAME = "org.opencord.olt";
@@ -101,7 +108,6 @@
         eventExecutor = Executors.newFixedThreadPool(5, groupedThreads("onos/olt",
                 "events-%d", log));
         appId = coreService.registerApplication(APP_NAME);
-        bpInfoToMeter = Maps.newConcurrentMap();
         programmedMeters = Sets.newConcurrentHashSet();
         meterService.addListener(meterListener);
         componentConfigService.registerProperties(getClass());
@@ -125,20 +131,13 @@
     }
 
     @Override
-    public ImmutableMap<String, Set<MeterKey>> getBpMeterMappings() {
-        return ImmutableMap.copyOf(bpInfoToMeter);
+    public ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings() {
+        return ImmutableMap.copyOf(bpInfoToMeter.asMap());
     }
 
     @Override
     public void addMeterIdToBpMapping(DeviceId deviceId, MeterId meterId, String bandwidthProfile) {
-        bpInfoToMeter.compute(bandwidthProfile, (k, v) -> {
-            if (v == null) {
-                return Sets.newHashSet(MeterKey.key(deviceId, meterId));
-            } else {
-                v.add(MeterKey.key(deviceId, meterId));
-                return v;
-            }
-        });
+        bpInfoToMeter.put(bandwidthProfile, MeterKey.key(deviceId, meterId));
     }
 
     @Override
@@ -199,7 +198,8 @@
 
                     @Override
                     public void onError(MeterRequest op, MeterFailReason reason) {
-                        bpInfoToMeter.remove(MeterKey.key(deviceId, meterIdRef.get()));
+                        bpInfoToMeter.remove(bpInfo.id(),
+                                             MeterKey.key(deviceId, meterIdRef.get()));
                         meterFuture.complete(reason);
                     }
                 })
@@ -242,12 +242,16 @@
         public void event(MeterEvent meterEvent) {
             eventExecutor.execute(() -> {
                 Meter meter = meterEvent.subject();
+                if (meter == null) {
+                    log.error("Meter in event {} is null", meterEvent);
+                    return;
+                }
                 MeterKey key = MeterKey.key(meter.deviceId(), meter.id());
                 if (deleteMeters && MeterEvent.Type.METER_REFERENCE_COUNT_ZERO.equals(meterEvent.type())) {
                     log.info("Zero Count Meter Event is received. Meter is {}", meter.id());
                     incrementMeterCount(key);
 
-                    if (meter != null && appId.equals(meter.appId()) && pendingRemoveMeters.get(key).get() == 3) {
+                    if (appId.equals(meter.appId()) && pendingRemoveMeters.get(key).get() == 3) {
                         log.info("Deleting unreferenced, no longer programmed Meter {}", meter.id());
                         deleteMeter(meter.deviceId(), meter.id());
                     }
@@ -256,7 +260,7 @@
                     log.info("Meter Removed Event is received for {}", meter.id());
                     programmedMeters.remove(key);
                     pendingRemoveMeters.remove(key);
-                    removeMeterFromBpMapping(meter);
+                    removeMeterFromBpMapping(key);
                 }
             });
         }
@@ -290,12 +294,11 @@
             }
         }
 
-        private void removeMeterFromBpMapping(Meter meter) {
-            MeterKey meterKey = MeterKey.key(meter.deviceId(), meter.id());
-            Iterator<Map.Entry<String, Set<MeterKey>>> iterator = bpInfoToMeter.entrySet().iterator();
+        private void removeMeterFromBpMapping(MeterKey meterKey) {
+            Iterator<Map.Entry<String, MeterKey>> iterator = bpInfoToMeter.entries().iterator();
             while (iterator.hasNext()) {
-                Map.Entry<String, Set<MeterKey>> entry = iterator.next();
-                if (entry.getValue().contains(meterKey)) {
+                Map.Entry<String, MeterKey> entry = iterator.next();
+                if (entry.getValue().equals(meterKey)) {
                     iterator.remove();
                     log.info("Deleted meter for MeterKey {} - Last prog meters {}", meterKey, programmedMeters);
                     break;
diff --git a/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java b/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
index d2d51b2..4f07131 100644
--- a/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
+++ b/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
@@ -22,7 +22,7 @@
 import org.onosproject.net.meter.MeterKey;
 import org.opencord.sadis.BandwidthProfileInformation;
 
-import java.util.Set;
+import java.util.Collection;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -36,7 +36,7 @@
      *
      * @return an immutable map of bandwidthProfile-meterKey (device / meter) mappings
      */
-    ImmutableMap<String, Set<MeterKey>> getBpMeterMappings();
+    ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings();
 
     /**
      * Adds a bandwidthProfile-meterKey (device / meter) mapping that have been programmed
diff --git a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
index 8a80ee0..16b407b 100644
--- a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
+++ b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
@@ -183,7 +183,7 @@
 
     private class MockOltMeterService implements org.opencord.olt.internalapi.AccessDeviceMeterService {
         @Override
-        public ImmutableMap<String, Set<MeterKey>> getBpMeterMappings() {
+        public ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings() {
             return null;
         }
 
diff --git a/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java b/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
index 7100b0f..da7e3e1 100644
--- a/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
+++ b/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
@@ -15,8 +15,9 @@
  */
 package org.opencord.olt.impl;
 
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
 import com.google.common.collect.Sets;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,7 +31,6 @@
 import org.opencord.sadis.BandwidthProfileInformation;
 
 import java.util.Collection;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
 public class OltMeterTest extends TestBase {
@@ -41,7 +41,7 @@
     @Before
     public void setUp() {
         oltMeterService = new OltMeterService();
-        oltMeterService.bpInfoToMeter = Maps.newConcurrentMap();
+        oltMeterService.bpInfoToMeter = Multimaps.synchronizedSetMultimap(HashMultimap.create());
         oltMeterService.programmedMeters = Sets.newConcurrentHashSet();
         oltMeterService.meterService = new MockMeterService();
     }
@@ -56,7 +56,7 @@
         MeterId dsMeterId = oltMeterService.getMeterIdFromBpMapping(DEVICE_ID_1, dsBpId);
         assert  dsMeterId.equals(this.dsMeterId);
 
-        ImmutableMap<String, Set<MeterKey>> meterMappings = oltMeterService.getBpMeterMappings();
+        ImmutableMap<String, Collection<MeterKey>> meterMappings = oltMeterService.getBpMeterMappings();
         assert  meterMappings.size() == 2;
     }