Improvements to flows list command.

 * Added -s option which gives more succinct output.
 * Display each flow on one line for easy grepping.
 * Added ability to filter output by table ID.
 * Flows are now sorted by descending priority within a table.
 * Removed the use of toStringHelper in instructions and criterion to produce
   less verbose output.

Change-Id: I1c874c776491386488ea5a4d23627b20f1e5728b
diff --git a/cli/src/main/java/org/onosproject/cli/Comparators.java b/cli/src/main/java/org/onosproject/cli/Comparators.java
index 03d25ce..50e79a1 100644
--- a/cli/src/main/java/org/onosproject/cli/Comparators.java
+++ b/cli/src/main/java/org/onosproject/cli/Comparators.java
@@ -71,10 +71,16 @@
     public static final Comparator<FlowRule> FLOW_RULE_COMPARATOR = new Comparator<FlowRule>() {
         @Override
         public int compare(FlowRule f1, FlowRule f2) {
-            int tableCompare = Integer.valueOf(f1.tableId()).compareTo(f2.tableId());
-            return (tableCompare == 0)
+            // Compare table IDs in ascending order
+            int tableCompare = f1.tableId() - f2.tableId();
+            if (tableCompare != 0) {
+                return tableCompare;
+            }
+            // Compare priorities in descending order
+            int priorityCompare = f2.priority() - f1.priority();
+            return (priorityCompare == 0)
                     ? Long.valueOf(f1.id().value()).compareTo(f2.id().value())
-                    : tableCompare;
+                    : priorityCompare;
         }
     };
 
diff --git a/cli/src/main/java/org/onosproject/cli/net/FlowsListCommand.java b/cli/src/main/java/org/onosproject/cli/net/FlowsListCommand.java
index 331cca1..b6477c3 100644
--- a/cli/src/main/java/org/onosproject/cli/net/FlowsListCommand.java
+++ b/cli/src/main/java/org/onosproject/cli/net/FlowsListCommand.java
@@ -15,16 +15,13 @@
  */
 package org.onosproject.cli.net;
 
-import static com.google.common.collect.Lists.newArrayList;
-
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.SortedMap;
-import java.util.TreeMap;
-
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
+import org.apache.karaf.shell.commands.Option;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.cli.Comparators;
 import org.onosproject.core.ApplicationId;
@@ -35,11 +32,16 @@
 import org.onosproject.net.flow.FlowEntry;
 import org.onosproject.net.flow.FlowEntry.FlowEntryState;
 import org.onosproject.net.flow.FlowRuleService;
+import org.onosproject.net.flow.TrafficTreatment;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ArrayNode;
-import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.function.Predicate;
+
+import static com.google.common.collect.Lists.newArrayList;
 
 /**
  * Lists all currently-known flows.
@@ -48,26 +50,44 @@
          description = "Lists all currently-known flows.")
 public class FlowsListCommand extends AbstractShellCommand {
 
+    private static final Predicate<FlowEntry> TRUE_PREDICATE = f -> true;
+
     public static final String ANY = "any";
 
-    private static final String FMT =
-            "   id=%s, state=%s, bytes=%s, packets=%s, duration=%s, priority=%s, tableId=%s appId=%s, payLoad=%s";
-    private static final String TFMT = "      treatment=%s";
-    private static final String SFMT = "      selector=%s";
+    private static final String LONG_FORMAT = "    id=%s, state=%s, bytes=%s, "
+            + "packets=%s, duration=%s, priority=%s, tableId=%s, appId=%s, "
+            + "payLoad=%s, selector=%s, treatment=%s";
+
+    private static final String SHORT_FORMAT = "    %s, bytes=%s, packets=%s, "
+            + "table=%s, priority=%s, selector=%s, treatment=%s";
+
+    @Argument(index = 0, name = "state", description = "Flow Rule state",
+            required = false, multiValued = false)
+    String state = null;
 
     @Argument(index = 1, name = "uri", description = "Device ID",
               required = false, multiValued = false)
     String uri = null;
 
-    @Argument(index = 0, name = "state", description = "Flow Rule state",
-              required = false, multiValued = false)
-    String state = null;
+    @Argument(index = 2, name = "table", description = "Table ID",
+            required = false, multiValued = false)
+    String table = null;
+
+    @Option(name = "-s", aliases = "--short",
+            description = "Print more succinct output for each flow",
+            required = false, multiValued = false)
+    private boolean shortOutput = false;
+
+    private Predicate<FlowEntry> predicate = TRUE_PREDICATE;
 
     @Override
     protected void execute() {
         CoreService coreService = get(CoreService.class);
         DeviceService deviceService = get(DeviceService.class);
         FlowRuleService service = get(FlowRuleService.class);
+
+        compilePredicate();
+
         SortedMap<Device, List<FlowEntry>> flows = getSortedFlows(deviceService, service);
 
         if (outputJson()) {
@@ -94,6 +114,22 @@
         return result;
     }
 
+    /**
+     * Compiles a predicate to find matching flows based on the command
+     * arguments.
+     */
+    private void compilePredicate() {
+        if (state != null && !state.equals(ANY)) {
+            final FlowEntryState feState = FlowEntryState.valueOf(state.toUpperCase());
+            predicate = predicate.and(f -> f.state().equals(feState));
+        }
+
+        if (table != null) {
+            final int tableId = Integer.parseInt(table);
+            predicate = predicate.and(f -> f.tableId() == tableId);
+        }
+    }
+
     // Produces JSON object with the flows of the given device.
     private ObjectNode json(ObjectMapper mapper,
                             Device device, List<FlowEntry> flows) {
@@ -119,10 +155,7 @@
                                                           FlowRuleService service) {
         SortedMap<Device, List<FlowEntry>> flows = new TreeMap<>(Comparators.ELEMENT_COMPARATOR);
         List<FlowEntry> rules;
-        FlowEntryState s = null;
-        if (state != null && !state.equals("any")) {
-            s = FlowEntryState.valueOf(state.toUpperCase());
-        }
+
         Iterable<Device> devices = null;
         if (uri == null) {
             devices = deviceService.getDevices();
@@ -131,13 +164,14 @@
             devices = (dev == null) ? deviceService.getDevices()
                                     : Collections.singletonList(dev);
         }
+
         for (Device d : devices) {
-            if (s == null) {
+            if (predicate.equals(TRUE_PREDICATE)) {
                 rules = newArrayList(service.getFlowEntries(d.id()));
             } else {
                 rules = newArrayList();
                 for (FlowEntry f : service.getFlowEntries(d.id())) {
-                    if (f.state().equals(s)) {
+                    if (predicate.test(f)) {
                         rules.add(f);
                     }
                 }
@@ -159,17 +193,51 @@
                               CoreService coreService) {
         boolean empty = flows == null || flows.isEmpty();
         print("deviceId=%s, flowRuleCount=%d", d.id(), empty ? 0 : flows.size());
-        if (!empty) {
-            for (FlowEntry f : flows) {
+        if (empty) {
+            return;
+        }
+
+        for (FlowEntry f : flows) {
+            if (shortOutput) {
+                print(SHORT_FORMAT, f.state(), f.bytes(), f.packets(),
+                        f.tableId(), f.priority(), f.selector().criteria(),
+                        printTreatment(f.treatment()));
+            } else {
                 ApplicationId appId = coreService.getAppId(f.appId());
-                print(FMT, Long.toHexString(f.id().value()), f.state(),
-                      f.bytes(), f.packets(), f.life(), f.priority(), f.tableId(),
-                      appId != null ? appId.name() : "<none>",
-                      f.payLoad() == null ? null : f.payLoad().payLoad().toString());
-                print(SFMT, f.selector().criteria());
-                print(TFMT, f.treatment());
+                print(LONG_FORMAT, Long.toHexString(f.id().value()), f.state(),
+                        f.bytes(), f.packets(), f.life(), f.priority(), f.tableId(),
+                        appId != null ? appId.name() : "<none>",
+                        f.payLoad() == null ? null : f.payLoad().payLoad().toString(),
+                        f.selector().criteria(), f.treatment());
             }
         }
     }
 
+    private String printTreatment(TrafficTreatment treatment) {
+        final String delimiter = ", ";
+        StringBuilder builder = new StringBuilder("[");
+        if (!treatment.immediate().isEmpty()) {
+            builder.append("immediate=" + treatment.immediate() + delimiter);
+        }
+        if (!treatment.deferred().isEmpty()) {
+            builder.append("deferred=" + treatment.deferred() + delimiter);
+        }
+        if (treatment.clearedDeferred()) {
+            builder.append("clearDeferred" + delimiter);
+        }
+        if (treatment.tableTransition() != null) {
+            builder.append("transition=" + treatment.tableTransition() + delimiter);
+        }
+        if (treatment.metered() != null) {
+            builder.append("meter=" + treatment.metered() + delimiter);
+        }
+        if (treatment.writeMetadata() != null) {
+            builder.append("metadata=" + treatment.writeMetadata() + delimiter);
+        }
+        // Chop off last delimiter
+        builder.replace(builder.length() - delimiter.length(), builder.length(), "");
+        builder.append("]");
+        return builder.toString();
+    }
+
 }
diff --git a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index 59ebeb2..f9dde70 100644
--- a/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/cli/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -427,6 +427,8 @@
             <completers>
                 <ref component-id="flowRuleStatusCompleter"/>
                 <ref component-id="deviceIdCompleter"/>
+                <ref component-id="placeholderCompleter"/>
+                <null/>
             </completers>
         </command>