From 61dfccbb1a7750672a4028c4496288ccb8becaba Mon Sep 17 00:00:00 2001
From: Saurav Das <saurav.das@bigswitch.com>
Date: Fri, 24 May 2013 09:47:50 -0700
Subject: [PATCH] Registering counters returns an IDebugCounter instead of a
 counterID. Fixing the REST API to account for hierarchical counters More unit
 tests.

---
 .../debugcounter/DebugCounter.java            | 159 ++++++----
 .../debugcounter/IDebugCounter.java           |  38 +++
 .../debugcounter/IDebugCounterService.java    |  53 ++--
 .../debugcounter/NullDebugCounter.java        |  33 +-
 .../web/DebugCounterResource.java             |  41 +--
 .../debugcounter/CounterHierarchyPutTest.java |   8 +-
 .../debugcounter/DebugCounterTest.java        | 283 ++++++++++++++++++
 7 files changed, 483 insertions(+), 132 deletions(-)
 create mode 100644 src/main/java/net/floodlightcontroller/debugcounter/IDebugCounter.java
 create mode 100644 src/test/java/net/floodlightcontroller/debugcounter/DebugCounterTest.java

diff --git a/src/main/java/net/floodlightcontroller/debugcounter/DebugCounter.java b/src/main/java/net/floodlightcontroller/debugcounter/DebugCounter.java
index f094b3203..3b016914b 100644
--- a/src/main/java/net/floodlightcontroller/debugcounter/DebugCounter.java
+++ b/src/main/java/net/floodlightcontroller/debugcounter/DebugCounter.java
@@ -54,32 +54,33 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
      * protected class to store counter information
      */
     public static class CounterInfo {
-        String moduleCounterName;
+        String moduleCounterHierarchy;
         String counterDesc;
         CounterType ctype;
         String moduleName;
-        String counterName;
+        String counterHierarchy;
         int counterId;
         boolean enabled;
         Object[] metaData;
 
-        public CounterInfo(int counterId, boolean enabled, Object[] metaData,
-                           String moduleName, String counterName,
-                           String desc, CounterType ctype) {
-            this.moduleCounterName = moduleName + "/" + counterName;
+        public CounterInfo(int counterId, boolean enabled,
+                           String moduleName, String counterHierarchy,
+                           String desc, CounterType ctype, Object... metaData) {
+            this.moduleCounterHierarchy = moduleName + "/" + counterHierarchy;
             this.moduleName = moduleName;
-            this.counterName = counterName;
+            this.counterHierarchy = counterHierarchy;
             this.counterDesc = desc;
             this.ctype = ctype;
             this.counterId = counterId;
             this.enabled = enabled;
+            this.metaData = metaData;
         }
 
-        public String getModuleCounterName() { return moduleCounterName; }
+        public String getModuleCounterHierarchy() { return moduleCounterHierarchy; }
         public String getCounterDesc() { return counterDesc; }
         public CounterType getCtype() { return ctype; }
         public String getModuleName() { return moduleName; }
-        public String getCounterName() { return counterName; }
+        public String getCounterHierarchy() { return counterHierarchy; }
         public int getCounterId() { return counterId; }
         public boolean isEnabled() { return enabled; }
         public Object[] getMetaData() { return metaData; }
@@ -180,42 +181,93 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
         }
     };
 
+    //*******************************
+    //   IDebugCounter
+    //*******************************
+
+    private class CounterImpl implements IDebugCounter {
+        private final int counterId;
+
+        public CounterImpl(int counterId) {
+            this.counterId = counterId;
+        }
+
+        @Override
+        public void updateCounterWithFlush() {
+            if (!validCounterId()) return;
+            updateCounter(counterId, 1, true);
+        }
+
+        @Override
+        public void updateCounterNoFlush() {
+            if (!validCounterId()) return;
+            updateCounter(counterId, 1, false);
+        }
+
+        @Override
+        public void updateCounterWithFlush(int incr) {
+            if (!validCounterId()) return;
+            updateCounter(counterId, incr, true);
+        }
+
+        @Override
+        public void updateCounterNoFlush(int incr) {
+            if (!validCounterId()) return;
+            updateCounter(counterId, incr, false);
+        }
+
+        @Override
+        public long getCounterValue() {
+            if (!validCounterId()) return -1;
+            return allCounters[counterId].cvalue.get();
+        }
+
+        private boolean validCounterId() {
+            if (counterId < 0 || counterId >= MAX_COUNTERS) {
+                log.error("Invalid counterId invoked");
+                return false;
+            }
+            return true;
+        }
+
+    }
+
    //*******************************
    //   IDebugCounterService
    //*******************************
 
    @Override
-   public int registerCounter(String moduleName, String counterName,
-                              String counterDescription, CounterType counterType,
-                              Object[] metaData)  throws MaxCountersRegistered{
+   public IDebugCounter registerCounter(String moduleName, String counterHierarchy,
+                           String counterDescription, CounterType counterType,
+                           Object... metaData)  throws MaxCountersRegistered{
        // check if counter already exists
        if (!moduleCounters.containsKey(moduleName)) {
            moduleCounters.putIfAbsent(moduleName,
                 new ConcurrentHashMap<String, CounterIndexStore>());
        }
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        if (rci.allLevelsFound) {
            // counter exists
            log.info("Counter exists for {}/{} -- resetting counters", moduleName,
-                    counterName);
-           resetCounterHierarchy(moduleName, counterName);
-           return rci.ctrIds[rci.foundUptoLevel-1];
+                    counterHierarchy);
+           resetCounterHierarchy(moduleName, counterHierarchy);
+           return new CounterImpl(rci.ctrIds[rci.foundUptoLevel-1]);
        }
        // check for validity of counter
        if (rci.levels.length > MAX_HIERARCHY) {
-           log.error("Registry of counterName {} exceeds max hierachy {}.. aborting",
-                     counterName, MAX_HIERARCHY);
-           return -1;
+           log.error("Registry of counterHierarchy {} exceeds max hierachy {}.. aborting",
+                     counterHierarchy, MAX_HIERARCHY);
+           return new CounterImpl(-1);
        }
        if (rci.foundUptoLevel < rci.levels.length-1) {
            String needToRegister = "";
            for (int i=0; i<=rci.foundUptoLevel; i++) {
                needToRegister += rci.levels[i];
            }
-           log.error("Attempting to register hierarchical counterName {}, "+
+           log.error("Attempting to register hierarchical counterHierarchy {}, "+
                      "but parts of hierarchy missing. Please register {} first ",
-                     counterName, needToRegister);
-           return -1;
+                     counterHierarchy, needToRegister);
+           return new CounterImpl(-1);
        }
 
        // get a new counter id
@@ -225,8 +277,9 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
        }
        // create storage for counter
        boolean enabled = (counterType == CounterType.ALWAYS_COUNT) ? true : false;
-       CounterInfo ci = new CounterInfo(counterId, enabled, metaData, moduleName,
-                                        counterName, counterDescription, counterType);
+       CounterInfo ci = new CounterInfo(counterId, enabled, moduleName,
+                                        counterHierarchy, counterDescription,
+                                        counterType, metaData);
        allCounters[counterId] = new DebugCounterInfo(ci);
 
        // account for the new counter in the module counter hierarchy
@@ -236,17 +289,10 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
        if (enabled) {
            currentCounters.add(counterId);
        }
-       return counterId;
+       return new CounterImpl(counterId);
    }
 
-
-   @Override
-   public void updateCounter(int counterId, boolean flushNow) {
-       updateCounter(counterId, 1, flushNow);
-   }
-
-   @Override
-   public void updateCounter(int counterId, int incr, boolean flushNow) {
+   private void updateCounter(int counterId, int incr, boolean flushNow) {
        if (counterId < 0 || counterId >= MAX_COUNTERS) return;
 
        LocalCounterInfo[] thiscounters =  this.threadlocalCounters.get();
@@ -285,7 +331,6 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
                }
            }
        }
-
    }
 
    @Override
@@ -319,14 +364,16 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
        // the global set.
        Sets.SetView<Integer> sv = Sets.difference(currentCounters, thisset);
        for (int counterId : sv) {
-           if (thiscounters[counterId] != null) thiscounters[counterId].enabled = true;
-           thisset.add(counterId);
+           if (thiscounters[counterId] != null) {
+               thiscounters[counterId].enabled = true;
+               thisset.add(counterId);
+           }
        }
    }
 
    @Override
-   public void resetCounterHierarchy(String moduleName, String counterName) {
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+   public void resetCounterHierarchy(String moduleName, String counterHierarchy) {
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        if (!rci.allLevelsFound) {
            String missing = rci.levels[rci.foundUptoLevel];
            log.error("Cannot reset counter hierarchy - missing counter {}", missing);
@@ -371,8 +418,8 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
    }
 
    @Override
-   public void enableCtrOnDemand(String moduleName, String counterName) {
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+   public void enableCtrOnDemand(String moduleName, String counterHierarchy) {
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        if (!rci.allLevelsFound) {
            String missing = rci.levels[rci.foundUptoLevel];
            log.error("Cannot enable counter - counter not found {}", missing);
@@ -385,8 +432,8 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
    }
 
    @Override
-   public void disableCtrOnDemand(String moduleName, String counterName) {
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+   public void disableCtrOnDemand(String moduleName, String counterHierarchy) {
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        if (!rci.allLevelsFound) {
            String missing = rci.levels[rci.foundUptoLevel];
            log.error("Cannot disable counter - counter not found {}", missing);
@@ -403,8 +450,8 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
 
    @Override
    public List<DebugCounterInfo> getCounterHierarchy(String moduleName,
-                                                     String counterName) {
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+                                                     String counterHierarchy) {
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        if (!rci.allLevelsFound) {
            String missing = rci.levels[rci.foundUptoLevel];
            log.error("Cannot fetch counter - counter not found {}", missing);
@@ -452,9 +499,10 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
    }
 
    @Override
-   public boolean containsModuleCounterName(String moduleName, String counterName) {
+   public boolean containsModuleCounterHierarchy(String moduleName,
+                                                 String counterHierarchy) {
        if (!moduleCounters.containsKey(moduleName)) return false;
-       RetCtrInfo rci = getCounterId(moduleName, counterName);
+       RetCtrInfo rci = getCounterId(moduleName, counterHierarchy);
        return rci.allLevelsFound;
    }
 
@@ -469,7 +517,7 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
 
    protected class RetCtrInfo {
        boolean allLevelsFound; // counter indices found all the way down the hierarchy
-       boolean hierarchical; // true if counterName is hierarchical
+       boolean hierarchical; // true if counterHierarchy is hierarchical
        int foundUptoLevel;
        int[]  ctrIds;
        String[] levels;
@@ -495,12 +543,15 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
 
    }
 
-   protected RetCtrInfo getCounterId(String moduleName, String counterName) {
+   protected RetCtrInfo getCounterId(String moduleName, String counterHierarchy) {
        RetCtrInfo rci = new RetCtrInfo();
        Map<String, CounterIndexStore> templevel = moduleCounters.get(moduleName);
-       rci.levels = counterName.split("/");
+       rci.levels = counterHierarchy.split("/");
        if (rci.levels.length > 1) rci.hierarchical = true;
-       if (templevel == null) return rci;
+       if (templevel == null) {
+           log.error("moduleName {} does not exist in debugCounters", moduleName);
+           return rci;
+       }
 
        /*
        if (rci.levels.length > MAX_HIERARCHY) {
@@ -514,7 +565,7 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
            if (templevel != null) {
                CounterIndexStore cis = templevel.get(rci.levels[i]) ;
                if (cis == null) {
-                   // could not find counterName part at this level
+                   // could not find counterHierarchy part at this level
                    break;
                } else {
                    rci.ctrIds[i] = cis.index;
@@ -526,7 +577,7 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
                }
            } else {
                // there are no more levels, which means that some part of the
-               // counterName has no corresponding map
+               // counterHierarchy has no corresponding map
                break;
            }
        }
@@ -592,7 +643,8 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
        }
    }
 
-   protected void printAllCounters() {
+   protected void printAllCounterIds() {
+       log.info("<moduleCounterHierarchy>");
        Set<String> keys = moduleCounters.keySet();
        for (String key : keys) {
            log.info("ModuleName: {}", key);
@@ -618,6 +670,7 @@ public class DebugCounter implements IFloodlightModule, IDebugCounterService {
                }
            }
        }
+       log.info("<\\moduleCounterHierarchy>");
    }
 
    //*******************************
diff --git a/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounter.java b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounter.java
new file mode 100644
index 000000000..dbde1854e
--- /dev/null
+++ b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounter.java
@@ -0,0 +1,38 @@
+package net.floodlightcontroller.debugcounter;
+
+public interface IDebugCounter {
+    /**
+     * Increments the counter by 1 thread-locally, and immediately flushes to
+     * the global counter storage. This method should be used for counters that
+     * are updated outside the OF message processing pipeline.
+     */
+    void updateCounterWithFlush();
+
+    /**
+     * Increments the counter by 1 thread-locally. Flushing to the global
+     * counter storage is delayed (happens with flushCounters() in IDebugCounterService),
+     * resulting in higher performance. This method should be used for counters
+     * updated in the OF message processing pipeline.
+     */
+    void updateCounterNoFlush();
+
+    /**
+     * Increments the counter thread-locally by the 'incr' specified, and immediately
+     * flushes to the global counter storage. This method should be used for counters
+     * that are updated outside the OF message processing pipeline.
+     */
+    void updateCounterWithFlush(int incr);
+
+    /**
+     * Increments the counter thread-locally by the 'incr' specified. Flushing to the global
+     * counter storage is delayed (happens with flushCounters() in IDebugCounterService),
+     * resulting in higher performance. This method should be used for counters
+     * updated in the OF message processing pipeline.
+     */
+    void updateCounterNoFlush(int incr);
+
+    /**
+     * Retrieve the value of the counter from the global counter store
+     */
+    long getCounterValue();
+}
diff --git a/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java
index 2460086ed..d7aa3ad01 100644
--- a/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java
+++ b/src/main/java/net/floodlightcontroller/debugcounter/IDebugCounterService.java
@@ -19,7 +19,7 @@ public interface IDebugCounterService extends IFloodlightService {
     /**
      *  A limit on the maximum number of counters that can be created
      */
-    public static final int MAX_COUNTERS = 10000;
+    public static final int MAX_COUNTERS = 5000;
 
     /**
      * exception thrown when MAX_COUNTERS have been registered
@@ -37,9 +37,9 @@ public interface IDebugCounterService extends IFloodlightService {
      * All modules that wish to have the DebugCounterService count for them, must
      * register their counters by making this call (typically from that module's
      * 'startUp' method). The counter can then be updated, displayed, reset etc.
-     * using the registered moduleCounterName.
+     * using the registered moduleCounterHierarchy.
      *
-     * @param moduleCounterName    the counter name which MUST be have the following
+     * @param moduleCounterHierarchy    the counter name which MUST be have the following
      *                             syntax:  <module name>-<counter name>
      *                             eg.: linkdiscovery-incoming
      *                             There should be only a single '-' in the name
@@ -52,24 +52,11 @@ public interface IDebugCounterService extends IFloodlightService {
      *                             methods in this API -- i.e. registering them is
      *                             not enough to start counting.
      * @return                     false if the counter has already been registered
-     *                             or if the moduleCounterName is not as expected.
+     *                             or if the moduleCounterHierarchy is not as expected.
      */
-    public int registerCounter(String moduleName, String counterName,
-                               String counterDescription, CounterType counterType,
-                               Object[] metaData) throws MaxCountersRegistered;
-
-    /**
-     * Increments the counter by 1, if the counter is meant to be always counted,
-     * or if the counter has been enabled for counting.
-     * @param moduleCounterName   the registered counter name.
-     */
-    void updateCounter(int counterId, boolean flushNow);
-
-    /**
-     * Increments the counter by the number specified
-     * @param moduleCounterName   the registered counter name.
-     */
-    void updateCounter(int counterId, int incr, boolean flushNow);
+    public IDebugCounter registerCounter(String moduleName, String counterHierarchy,
+                             String counterDescription, CounterType counterType,
+                             Object... metaData) throws MaxCountersRegistered;
 
     /**
      * Update the global counter map with values from the thread local maps. This
@@ -83,9 +70,9 @@ public interface IDebugCounterService extends IFloodlightService {
      * Resets the value of the counter to zero if it is currently enabled. Note
      * that with live traffic, it is not necessary that the counter will display
      * zero with a get call as it may get updated between the reset and get calls.
-     * @param moduleCounterName the registered counter name.
+     * @param moduleCounterHierarchy the registered counter name.
      */
-    void resetCounterHierarchy(String moduleName, String counterName);
+    void resetCounterHierarchy(String moduleName, String counterHierarchy);
 
     /**
      * Resets the values of all counters that are currently enabled to zero.
@@ -95,7 +82,7 @@ public interface IDebugCounterService extends IFloodlightService {
     /**
      * Resets the values of all counters that are currently active and belong
      * to a module with the given 'moduleName'. The moduleName MUST be the
-     * part of the moduleCounterName with which the counters were registered.
+     * part of the moduleCounterHierarchy with which the counters were registered.
      * eg. if 'linkdiscovery-incoming' and 'linkdiscovery-lldpeol' are two counters
      * the module name is 'linkdiscovery'
      * @param moduleName
@@ -109,28 +96,28 @@ public interface IDebugCounterService extends IFloodlightService {
      * enough (as is the case for CounterType.ALWAYS_COUNT). Note that newly
      * enabled counter starts from an initial value of zero.
      *
-     * @param moduleCounterName  the registered counter name.
+     * @param moduleCounterHierarchy  the registered counter name.
      */
-    public void enableCtrOnDemand(String moduleName, String counterName);
+    public void enableCtrOnDemand(String moduleName, String counterHierarchy);
 
     /**
      * This method applies only to CounterType.ALWAYS_COUNT. It is used to disable
      * counting on this counter. Note that disabling a counter results in a loss
      * of the counter value. When re-enabled the counter will restart from zero.
      *
-     * @param moduleCounterName the registered counter name.
+     * @param moduleCounterHierarchy the registered counter name.
      */
-    public void disableCtrOnDemand(String moduleName, String counterName);
+    public void disableCtrOnDemand(String moduleName, String counterHierarchy);
 
     /**
      * Get counter value and associated information for a specific counter if it
      * is active.
      *
-     * @param moduleCounterName
+     * @param moduleCounterHierarchy
      * @return DebugCounterInfo or null if the counter could not be found
      */
     public List<DebugCounterInfo> getCounterHierarchy(String moduleName,
-                                                      String counterName);
+                                                      String counterHierarchy);
 
     /**
      * Get counter values and associated information for all active counters
@@ -149,15 +136,15 @@ public interface IDebugCounterService extends IFloodlightService {
     public  List<DebugCounterInfo> getModuleCounterValues(String moduleName);
 
     /**
-     * Convenience method to figure out if the the given 'moduleCounterName' corresponds
-     * to a registered moduleCounterName or not. Note that the counter may or
+     * Convenience method to figure out if the the given 'moduleCounterHierarchy' corresponds
+     * to a registered moduleCounterHierarchy or not. Note that the counter may or
      * may not be enabled for counting, but if it is registered the method will
      * return true.
      *
      * @param param
-     * @return false if moduleCounterName is not a registered counter
+     * @return false if moduleCounterHierarchy is not a registered counter
      */
-    public boolean containsModuleCounterName(String moduleName, String counterName);
+    public boolean containsModuleCounterHierarchy(String moduleName, String counterHierarchy);
 
     /**
      * Convenience method to figure out if the the given 'moduleName' corresponds
diff --git a/src/main/java/net/floodlightcontroller/debugcounter/NullDebugCounter.java b/src/main/java/net/floodlightcontroller/debugcounter/NullDebugCounter.java
index d72a028af..b0c82c369 100644
--- a/src/main/java/net/floodlightcontroller/debugcounter/NullDebugCounter.java
+++ b/src/main/java/net/floodlightcontroller/debugcounter/NullDebugCounter.java
@@ -70,23 +70,23 @@ public class NullDebugCounter implements IFloodlightModule, IDebugCounterService
 
 
     @Override
-    public void resetCounterHierarchy(String moduleName, String counterName) {
+    public void resetCounterHierarchy(String moduleName, String counterHierarchy) {
 
     }
 
     @Override
-    public void enableCtrOnDemand(String moduleName, String counterName) {
+    public void enableCtrOnDemand(String moduleName, String counterHierarchy) {
 
     }
 
     @Override
-    public void disableCtrOnDemand(String moduleName, String counterName) {
+    public void disableCtrOnDemand(String moduleName, String counterHierarchy) {
 
     }
 
     @Override
     public List<DebugCounterInfo> getCounterHierarchy(String moduleName,
-                                                      String counterName) {
+                                                      String counterHierarchy) {
         return null;
     }
 
@@ -101,8 +101,8 @@ public class NullDebugCounter implements IFloodlightModule, IDebugCounterService
     }
 
     @Override
-    public boolean containsModuleCounterName(String moduleName,
-                                             String counterName) {
+    public boolean containsModuleCounterHierarchy(String moduleName,
+                                             String counterHierarchy) {
         return false;
     }
 
@@ -113,26 +113,15 @@ public class NullDebugCounter implements IFloodlightModule, IDebugCounterService
 
     @Override
     public
-            int
-            registerCounter(String moduleName, String counterName,
+            IDebugCounter
+            registerCounter(String moduleName, String counterHierarchy,
                             String counterDescription,
-                            CounterType counterType, Object[] metaData)
-                                                                       throws MaxCountersRegistered {
-        // TODO Auto-generated method stub
-        return 0;
-    }
-
-    @Override
-    public void updateCounter(int counterId, boolean flushNow) {
-        // TODO Auto-generated method stub
-
+                            CounterType counterType, Object... metaData)
+                                 throws MaxCountersRegistered {
+        return null;
     }
 
-    @Override
-    public void updateCounter(int counterId, int incr, boolean flushNow) {
-        // TODO Auto-generated method stub
 
-    }
 
 
 }
diff --git a/src/main/java/net/floodlightcontroller/debugcounter/web/DebugCounterResource.java b/src/main/java/net/floodlightcontroller/debugcounter/web/DebugCounterResource.java
index 6773b3d79..7c68ff422 100644
--- a/src/main/java/net/floodlightcontroller/debugcounter/web/DebugCounterResource.java
+++ b/src/main/java/net/floodlightcontroller/debugcounter/web/DebugCounterResource.java
@@ -95,9 +95,9 @@ public class DebugCounterResource extends DebugCounterResourceBase {
      * The Reset command will reset the counter specified as well as all counters
      * in the hierarchical levels below. For example, if a counter hierarchy exists
      * as switch/00:00:00:00:01:02:03:04/pktin/drops, then a reset command with just
-     * the moduleName (param1=switch) and counterName (param2=00:00:00:00:01:02:03:04)
+     * the moduleName (param1=switch) and counterHierarchy (param2=00:00:00:00:01:02:03:04)
      * will reset all counters for that switch. Continuing the example -
-     * for a counterName (param2=00:00:00:00:01:02:03:04 and param3=pktin), the reset
+     * for a counterHierarchy (param2=00:00:00:00:01:02:03:04 and param3=pktin), the reset
      * command will remove all pktin counters for that switch.
      *
      * The enable/disable command will ONLY disable a specific counter (and only if
@@ -123,18 +123,18 @@ public class DebugCounterResource extends DebugCounterResourceBase {
             choice = Option.ALL;
         }
 
-        String counterName = "";
+        String counterHierarchy = "";
         if (param2 != null) {
-            counterName += "/" + param2;
+            counterHierarchy += "/" + param2;
             if (param3 != null) {
-                counterName += "/" + param3;
+                counterHierarchy += "/" + param3;
                 if (param4 != null) {
-                    counterName += "/" + param4;
+                    counterHierarchy += "/" + param4;
                 }
             }
         }
 
-        if (!moduleName.equals("all") && counterName.equals("")) {
+        if (!moduleName.equals("all") && counterHierarchy.equals("")) {
             // only module name specified
             boolean isRegistered = debugCounter.containsModuleName(param1);
             if (isRegistered) {
@@ -142,10 +142,10 @@ public class DebugCounterResource extends DebugCounterResourceBase {
             } else {
                 choice = Option.ERROR_BAD_MODULE_NAME;
             }
-        } else if (!moduleName.equals("all") && !counterName.equals("")) {
+        } else if (!moduleName.equals("all") && !counterHierarchy.equals("")) {
             // both module and counter names specified
-            boolean isRegistered = debugCounter.containsModuleCounterName(moduleName,
-                                                                          counterName);
+            boolean isRegistered = debugCounter.
+                    containsModuleCounterHierarchy(moduleName, counterHierarchy);
             if (isRegistered) {
                 choice = Option.MODULE_COUNTER_HIERARCHY;
             } else {
@@ -171,11 +171,11 @@ public class DebugCounterResource extends DebugCounterResourceBase {
                 break;
             case MODULE_COUNTER_HIERARCHY:
                 if (reset)
-                    debugCounter.resetCounterHierarchy(moduleName, counterName);
+                    debugCounter.resetCounterHierarchy(moduleName, counterHierarchy);
                 else if (turnOnOff && postData.getEnable())
-                    debugCounter.enableCtrOnDemand(moduleName, counterName);
+                    debugCounter.enableCtrOnDemand(moduleName, counterHierarchy);
                 else if (turnOnOff && !postData.getEnable())
-                    debugCounter.disableCtrOnDemand(moduleName, counterName);
+                    debugCounter.disableCtrOnDemand(moduleName, counterHierarchy);
                 break;
             case ERROR_BAD_MODULE_NAME:
                 output.error = "Module name has no corresponding registered counters";
@@ -226,16 +226,17 @@ public class DebugCounterResource extends DebugCounterResourceBase {
             choice = Option.ALL;
         }
 
-        String counterName = "";
+        String counterHierarchy = "";
         if (param2 != null) {
-            counterName += "/" + param2;
+            counterHierarchy += "/" + param2;
             if (param3 != null) {
-                counterName += "/" + param3;
+                counterHierarchy += "/" + param3;
                 if (param4 != null) {
-                    counterName += "/" + param4;
+                    counterHierarchy += "/" + param4;
                 }
             }
-            boolean isRegistered = debugCounter.containsModuleCounterName(param1, counterName);
+            boolean isRegistered = debugCounter.
+                    containsModuleCounterHierarchy(param1, counterHierarchy);
             if (isRegistered) {
                 choice = Option.MODULE_COUNTER_HIERARCHY;
             } else {
@@ -261,7 +262,7 @@ public class DebugCounterResource extends DebugCounterResourceBase {
                 populateCounters(debugCounter.getModuleCounterValues(param1), output);
                 break;
             case MODULE_COUNTER_HIERARCHY:
-                populateCounters(debugCounter.getCounterHierarchy(param1, counterName),
+                populateCounters(debugCounter.getCounterHierarchy(param1, counterHierarchy),
                                       output);
                 break;
             case ERROR_BAD_MODULE_NAME:
@@ -281,7 +282,7 @@ public class DebugCounterResource extends DebugCounterResourceBase {
                                        DebugCounterInfoOutput output) {
         if (debugCounterInfo != null)
             output.counterMap.put(debugCounterInfo.getCounterInfo().
-                                  getModuleCounterName(),
+                                  getModuleCounterHierarchy(),
                                   debugCounterInfo);
     }
 
diff --git a/src/test/java/net/floodlightcontroller/debugcounter/CounterHierarchyPutTest.java b/src/test/java/net/floodlightcontroller/debugcounter/CounterHierarchyPutTest.java
index d039f8f8c..a41a23953 100644
--- a/src/test/java/net/floodlightcontroller/debugcounter/CounterHierarchyPutTest.java
+++ b/src/test/java/net/floodlightcontroller/debugcounter/CounterHierarchyPutTest.java
@@ -60,7 +60,7 @@ public class CounterHierarchyPutTest extends FloodlightTestCase {
         printRCI("got ==>", rci);
         printRCI("exp ==>", exp);
         assertEquals(rci, exp);
-        dc.printAllCounters();
+        dc.printAllCounterIds();
         // add and then check for 2nd level of hierarchy
         dc.addToModuleCounterHierarchy("switch", 77, rci);
         rci = dc.getCounterId("switch", counterName);
@@ -72,7 +72,7 @@ public class CounterHierarchyPutTest extends FloodlightTestCase {
         exp.levels = counterName.split("/");
         printRCI("got ==>", rci);
         printRCI("exp ==>", exp);
-        dc.printAllCounters();
+        dc.printAllCounterIds();
         assertEquals(rci, exp);
 
         counterName = "100hp/pktin/drops";
@@ -86,7 +86,7 @@ public class CounterHierarchyPutTest extends FloodlightTestCase {
         printRCI("got ==>", rci);
         printRCI("exp ==>", exp);
         assertEquals(rci, exp);
-        dc.printAllCounters();
+        dc.printAllCounterIds();
         // add and then check for 3rd level of hierarchy
         dc.addToModuleCounterHierarchy("switch", 132, rci);
         rci = dc.getCounterId("switch", counterName);
@@ -99,7 +99,7 @@ public class CounterHierarchyPutTest extends FloodlightTestCase {
         exp.levels = counterName.split("/");
         printRCI("got ==>", rci);
         printRCI("exp ==>", exp);
-        dc.printAllCounters();
+        dc.printAllCounterIds();
         assertEquals(rci, exp);
 
     }
diff --git a/src/test/java/net/floodlightcontroller/debugcounter/DebugCounterTest.java b/src/test/java/net/floodlightcontroller/debugcounter/DebugCounterTest.java
new file mode 100644
index 000000000..97f9257f2
--- /dev/null
+++ b/src/test/java/net/floodlightcontroller/debugcounter/DebugCounterTest.java
@@ -0,0 +1,283 @@
+package net.floodlightcontroller.debugcounter;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import net.floodlightcontroller.debugcounter.DebugCounter.DebugCounterInfo;
+import net.floodlightcontroller.debugcounter.IDebugCounterService.CounterType;
+import net.floodlightcontroller.debugcounter.IDebugCounterService.MaxCountersRegistered;
+import net.floodlightcontroller.test.FloodlightTestCase;
+
+public class DebugCounterTest extends FloodlightTestCase {
+    DebugCounter dc;
+    protected static Logger log = LoggerFactory.getLogger(DebugCounterTest.class);
+    IDebugCounter S1, S2, S1_pi, S1_pi_d, S1_pi_e, S1_po, L_t;
+    List<DebugCounterInfo> dclist;
+
+    @Override
+    @Before
+    public void setUp() throws Exception {
+        dc = new DebugCounter();
+        S1 =       dc.registerCounter("switch", "01", "switch01",
+                                      CounterType.ALWAYS_COUNT);
+        S2 =       dc.registerCounter("switch", "02", "switch02",
+                                      CounterType.ALWAYS_COUNT);
+        S1_pi =    dc.registerCounter("switch", "01/pktin",
+                                      "switch01-pktin",
+                                      CounterType.ALWAYS_COUNT);
+        S1_pi_d =  dc.registerCounter("switch", "01/pktin/drops",
+                                      "switch01-pktin drops for all reasons",
+                                      CounterType.ALWAYS_COUNT, "warn");
+        S1_pi_e =  dc.registerCounter("switch", "01/pktin/err",
+                                      "switch01-pktin errors",
+                                      CounterType.ALWAYS_COUNT, "error", "snmp");
+        S1_po =    dc.registerCounter("switch", "01/pktout",
+                                      "switch01-pktout",
+                                      CounterType.ALWAYS_COUNT);
+        L_t =      dc.registerCounter("linkd", "tunnel",
+                                      "tunnel links",
+                                      CounterType.ALWAYS_COUNT);
+    }
+
+
+    @Test
+    public void testBasicCounterWorking() {
+        dc.printAllCounterIds();
+        S1.updateCounterNoFlush();
+        assertEquals(S1.getCounterValue(), 0);
+        S1.updateCounterWithFlush();
+        assertEquals(S1.getCounterValue(), 2);
+        S1.updateCounterNoFlush(30);
+        assertEquals(S1.getCounterValue(), 2);
+        S1.updateCounterWithFlush(10);
+        assertEquals(S1.getCounterValue(), 42);
+        S1.updateCounterNoFlush();
+        assertEquals(S1.getCounterValue(), 42);
+        dc.flushCounters();
+        assertEquals(S1.getCounterValue(), 43);
+    }
+
+    @Test
+    public void testCounterHierarchy() {
+        S1.updateCounterNoFlush();
+        S2.updateCounterNoFlush(2);
+        L_t.updateCounterNoFlush(3);
+        S1_pi.updateCounterNoFlush(10);
+        S1_po.updateCounterNoFlush(20);
+        S1_pi_d.updateCounterNoFlush(100);
+        S1_pi_e.updateCounterNoFlush(105);
+        dc.flushCounters();
+        checkCounters(1, 2, 3, 10, 20, 100, 105);
+    }
+
+    private void checkCounters(long S1_val, long S2_val, long L_t_val, // 1st level
+                               long S1_pi_val, long S1_po_val,         // 2nd level
+                               long S1_pi_d_val, long S1_pi_e_val) {   // 3rd level
+        assertEquals(S1.getCounterValue(), S1_val);
+        assertEquals(S2.getCounterValue(), S2_val);
+        assertEquals(L_t.getCounterValue(), L_t_val);
+        assertEquals(S1_pi.getCounterValue(), S1_pi_val);
+        assertEquals(S1_po.getCounterValue(), S1_po_val);
+        assertEquals(S1_pi_d.getCounterValue(), S1_pi_d_val);
+        assertEquals(S1_pi_e.getCounterValue(), S1_pi_e_val);
+    }
+
+    @Test
+    public void testBasicCounterReset() {
+        testCounterHierarchy();
+        dc.resetCounterHierarchy("linkd", "tunnel");
+        checkCounters(1, 2, 0, 10, 20, 100, 105);
+        // missing counter
+        dc.resetCounterHierarchy("switch", "S2");
+        checkCounters(1, 2, 0, 10, 20, 100, 105);
+        // missing module
+        dc.resetCounterHierarchy("swicth", "02");
+        checkCounters(1, 2, 0, 10, 20, 100, 105);
+        // the actual counter
+        dc.resetCounterHierarchy("switch", "02");
+        checkCounters(1, 0, 0, 10, 20, 100, 105);
+        // leafs
+        dc.resetCounterHierarchy("switch", "01/pktin/err");
+        checkCounters(1, 0, 0, 10, 20, 100, 0);
+        dc.resetCounterHierarchy("switch", "01/pktin/drops");
+        checkCounters(1, 0, 0, 10, 20, 0, 0);
+    }
+
+    @Test
+    public void testHierarchicalCounterReset1() {
+        testCounterHierarchy();
+        dc.resetCounterHierarchy("switch", "01/pktin");
+        checkCounters(1, 2, 3, 0, 20, 0, 0);
+    }
+    @Test
+    public void testHierarchicalCounterReset2() {
+        testCounterHierarchy();
+        dc.resetCounterHierarchy("switch", "01");
+        checkCounters(0, 2, 3, 0, 0, 0, 0);
+    }
+    @Test
+    public void testHierarchicalCounterReset3() {
+        testCounterHierarchy();
+        dc.resetAllModuleCounters("switch");
+        checkCounters(0, 0, 3, 0, 0, 0, 0);
+        dc.resetAllModuleCounters("linkd");
+        checkCounters(0, 0, 0, 0, 0, 0, 0);
+        testCounterHierarchy();
+    }
+    @Test
+    public void testHierarchicalCounterReset4() {
+        testCounterHierarchy();
+        dc.resetAllCounters();
+        checkCounters(0, 0, 0, 0, 0, 0, 0);
+        testCounterHierarchy();
+    }
+
+    private void verifyCounters(List<DebugCounterInfo> dclist, Long...longs ) {
+        List<Long> a = Arrays.asList(longs.clone());
+        for (DebugCounterInfo dci : dclist) {
+            assertEquals(true, a.contains(dci.cvalue.get()));
+        }
+        assertEquals(dclist.size(), longs.length);
+    }
+
+    @Test
+    public void testBasicCounterGet() {
+        testCounterHierarchy();
+        dclist = dc.getCounterHierarchy("switch", "02");
+        verifyCounters(dclist, 2L);
+        dclist = dc.getCounterHierarchy("linkd", "tunnel");
+        verifyCounters(dclist, 3L);
+        dclist = dc.getCounterHierarchy("switch", "01/pktin/err");
+        verifyCounters(dclist, 105L);
+        dclist = dc.getCounterHierarchy("switch", "01/pktin/drops");
+        verifyCounters(dclist, 100L);
+    }
+
+    @Test
+    public void testHierarchicalCounterGet() {
+        testCounterHierarchy();
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 10L, 100L, 105L);
+        dclist = dc.getCounterHierarchy("switch", "01");
+        verifyCounters(dclist, 1L, 20L, 10L, 100L, 105L);
+        dclist = dc.getModuleCounterValues("switch");
+        verifyCounters(dclist, 2L, 1L, 20L, 10L, 100L, 105L);
+        dclist = dc.getModuleCounterValues("linkd");
+        verifyCounters(dclist, 3L);
+        dclist = dc.getAllCounterValues();
+        verifyCounters(dclist, 3L, 2L, 1L, 20L, 10L, 100L, 105L);
+    }
+
+    @Test
+    public void testEnableDisableCounter() throws MaxCountersRegistered {
+        testCounterHierarchy();
+        IDebugCounter S1_pi_u, S1_fm, S1_fm_d;
+
+        S1_pi_u =  dc.registerCounter("switch", "01/pktin/unknowns",
+                                      "switch01-pktin unknowns",
+                                      CounterType.COUNT_ON_DEMAND, "warn");
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 0L, 10L, 100L, 105L);
+        S1_pi_u.updateCounterWithFlush(112);
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 0L, 10L, 100L, 105L);
+
+        dc.enableCtrOnDemand("switch", "01/pktin/unknowns");
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 0L, 10L, 100L, 105L);
+        dc.flushCounters(); // required for sync of thread-local store to global store
+        S1_pi_u.updateCounterWithFlush(112);
+        assertEquals(112L, S1_pi_u.getCounterValue());
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 10L, 100L, 105L, 112L);
+        S1_pi_u.updateCounterWithFlush();
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 10L, 100L, 105L, 113L);
+
+        dc.disableCtrOnDemand("switch", "01/pktin/unknowns");
+        S1_pi_u.updateCounterWithFlush();
+        S1_pi_u.updateCounterWithFlush();
+        S1_pi_u.updateCounterWithFlush();
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 10L, 100L, 105L, 0L);
+
+        //cannot disable ALWAYS_COUNT counter
+        dc.disableCtrOnDemand("switch", "01/pktin/err");
+        S1_pi_e.updateCounterWithFlush();
+        dclist = dc.getCounterHierarchy("switch", "01/pktin");
+        verifyCounters(dclist, 10L, 100L, 106L, 0L);
+
+        //enable/disable counter inside hierarchy
+        S1_fm =  dc.registerCounter("switch", "01/fm",
+                                      "switch01-flow-mods",
+                                      CounterType.COUNT_ON_DEMAND, "warn");
+        S1_fm_d =  dc.registerCounter("switch", "01/fm/dup",
+                                      "switch01- duplicate flow mods",
+                                      CounterType.ALWAYS_COUNT, "warn");
+        S1_fm.updateCounterWithFlush(8000);
+        S1_fm_d.updateCounterWithFlush(5000);
+        dclist = dc.getCounterHierarchy("switch", "01/fm");
+        verifyCounters(dclist, 5000L, 0L);
+        dc.enableCtrOnDemand("switch", "01/fm");
+        dc.flushCounters();
+        S1_fm.updateCounterWithFlush(8000);
+        S1_fm_d.updateCounterWithFlush(5000);
+        dclist = dc.getCounterHierarchy("switch", "01/fm");
+        verifyCounters(dclist, 10000L, 8000L);
+        dc.disableCtrOnDemand("switch", "01/fm");
+        S1_fm.updateCounterWithFlush(8000);
+        S1_fm_d.updateCounterWithFlush(5000);
+        dclist = dc.getCounterHierarchy("switch", "01/fm");
+        verifyCounters(dclist, 15000L, 0L);
+    }
+
+    @Test
+    public void testCounterReregistry() throws MaxCountersRegistered {
+        testCounterHierarchy();
+        checkCounters(1, 2, 3, 10, 20, 100, 105);
+        S1 =  dc.registerCounter("switch", "01", "switch01",
+                                 CounterType.ALWAYS_COUNT);
+        checkCounters(0, 2, 3, 0, 0, 0, 0); // switch 1 counter re-setted
+        assertEquals(0, S1.getCounterValue());
+    }
+
+    @Test
+    public void testContains() {
+        testCounterHierarchy();
+        assertTrue(dc.containsModuleName("switch"));
+        assertTrue(dc.containsModuleName("linkd"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "01"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "02"));
+        assertFalse(dc.containsModuleCounterHierarchy("switch", "03"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "01/pktin"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "01/pktout"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "01/pktin/err"));
+        assertTrue(dc.containsModuleCounterHierarchy("switch", "01/pktin/drops"));
+        assertFalse(dc.containsModuleCounterHierarchy("switch", "01/pktin/unknowns"));
+        assertFalse(dc.containsModuleCounterHierarchy("switch", "01/pktin/errr"));
+    }
+
+    @Test
+    public void testMetadata() {
+        testCounterHierarchy();
+        dclist = dc.getCounterHierarchy("switch", "01/pktin/err");
+        for (DebugCounterInfo dc : dclist) {
+            assertTrue(dc.cinfo.metaData[0].equals("error"));
+            assertTrue(dc.cinfo.metaData[1].equals("snmp"));
+            log.info("metadata: {}", Arrays.toString(dc.cinfo.getMetaData()));
+        }
+        verifyCounters(dclist, 105L);
+
+        dclist = dc.getCounterHierarchy("switch", "02");
+        for (DebugCounterInfo dc : dclist) {
+            assertTrue(dc.cinfo.getMetaData().length == 0);
+            log.info("metadata: {}", Arrays.toString(dc.cinfo.getMetaData()));
+        }
+    }
+
+}
-- 
GitLab