GUI -- Enhanced DefaultCellComparator to utilize Comparable implementation if av…

…ailable on the objects being compared, defaulting to string comparison otherwise.
- Deleted obsolete comparators.

Change-Id: Ifeb74c05f73bd974d1afb3dc27527a5c7ad1bc12

@@ -43,6 +43,9 @@ import static com.google.common.base.Preconditions.checkNotNull;
  * The table also provides a mechanism for defining how cell values for a
  * particular column should be formatted into strings, to help facilitate
  * the encoding of the table data into a JSON structure.
+ * <p>
+ * Note that it is expected that all values for a particular column will
+ * be the same class.
  */
 public class TableModel {
 

@@ -20,7 +20,11 @@ package org.onosproject.ui.table.cell;
 import org.onosproject.ui.table.CellComparator;
 
 /**
- * A default cell comparator. Implements a lexicographical compare function
+ * A default cell comparator.
+ * <p>
+ * Verifies that the objects being compared are the same class.
+ * Looks to see if the objects being compared implement comparable and, if so,
+ * delegates to that; otherwise, implements a lexicographical compare function
  * (i.e. string sorting). Uses the objects' toString() method and then
  * compares the resulting strings. Note that null values are acceptable and
  * are considered "smaller" than any non-null value.
@@ -31,7 +35,13 @@ public final class DefaultCellComparator extends AbstractCellComparator {
     private DefaultCellComparator() { }
 
     @Override
+    @SuppressWarnings("unchecked")
     protected int nonNullCompare(Object o1, Object o2) {
+        if (o1 instanceof Comparable) {
+            // if o2 is not the same class as o1, then compareTo will
+            // throw ClassCastException for us
+            return ((Comparable) o1).compareTo(o2);
+        }
         return o1.toString().compareTo(o2.toString());
     }
 

-/*
- * 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.ui.table.cell;
-
-import org.onosproject.ui.table.CellComparator;
-
-/**
- * An integer-based cell comparator.
- * Note that null values are acceptable and are considered "smaller" than
- * any non-null value.
- */
-public final class IntComparator extends AbstractCellComparator {
-
-    // non-instantiable
-    private IntComparator() { }
-
-    @Override
-    protected int nonNullCompare(Object o1, Object o2) {
-        return ((int) o1) - ((int) o2);
-    }
-
-    /**
-     * An instance of this class.
-     */
-    public static final CellComparator INSTANCE = new IntComparator();
-}

-/*
- * 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.ui.table.cell;
-
-import org.onosproject.ui.table.CellComparator;
-
-/**
- * A long-based cell comparator.
- * Note that null values are acceptable and are considered "smaller" than
- * any non-null value.
- */
-public final class LongComparator extends AbstractCellComparator {
-
-    // non-instantiable
-    private LongComparator() { }
-
-    @Override
-    protected int nonNullCompare(Object o1, Object o2) {
-        long diff = ((long) o1) - ((long) o2);
-        return diff == 0 ? 0 : (diff < 0 ? -1 : 1);
-    }
-
-    /**
-     * An instance of this class.
-     */
-    public static final CellComparator INSTANCE = new LongComparator();
-}

@@ -20,7 +20,6 @@ import org.junit.Test;
 import org.onosproject.ui.table.TableModel.SortDir;
 import org.onosproject.ui.table.cell.DefaultCellFormatter;
 import org.onosproject.ui.table.cell.HexFormatter;
-import org.onosproject.ui.table.cell.IntComparator;
 
 import static org.junit.Assert.*;
 
@@ -35,6 +34,10 @@ public class TableModelTest {
     private static final String BAR = "bar";
     private static final String ZOO = "zoo";
 
+    private enum StarWars {
+        LUKE_SKYWALKER, LEIA_ORGANA, HAN_SOLO, C3PO, R2D2, JABBA_THE_HUTT
+    }
+
     private static class ParenFormatter implements CellFormatter {
         @Override
         public String format(Object value) {
@@ -220,9 +223,6 @@ public class TableModelTest {
     public void tableNumberSort() {
         initUnsortedTable();
 
-        // first, tell the table to use an integer-based comparator
-        tm.setComparator(BAR, IntComparator.INSTANCE);
-
         // sort by number
         tm.sort(BAR, SortDir.ASC);
 
@@ -251,8 +251,7 @@ public class TableModelTest {
     public void sortAndFormat() {
         initUnsortedTable();
 
-        // set integer-based comparator and hex formatter
-        tm.setComparator(BAR, IntComparator.INSTANCE);
+        // set hex formatter
         tm.setFormatter(BAR, HexFormatter.INSTANCE);
 
         // sort by number
@@ -320,4 +319,26 @@ public class TableModelTest {
         assertEquals("null sort dir", SortDir.ASC, TableModel.sortDir(null));
     }
 
+
+    @Test
+    public void enumSort() {
+        tm = new TableModel(FOO);
+        tm.addRow().cell(FOO, StarWars.HAN_SOLO);
+        tm.addRow().cell(FOO, StarWars.C3PO);
+        tm.addRow().cell(FOO, StarWars.JABBA_THE_HUTT);
+        tm.addRow().cell(FOO, StarWars.LEIA_ORGANA);
+        tm.addRow().cell(FOO, StarWars.R2D2);
+        tm.addRow().cell(FOO, StarWars.LUKE_SKYWALKER);
+
+        tm.sort(FOO, SortDir.ASC);
+
+        // verify expected results
+        StarWars[] ordered = StarWars.values();
+        TableModel.Row[] rows = tm.getRows();
+        assertEquals("wrong length?", ordered.length, rows.length);
+        int nr = rows.length;
+        for (int i = 0; i < nr; i++) {
+            assertEquals(UNEX_SORT + i, ordered[i], rows[i].get(FOO));
+        }
+    }
 }

@@ -27,20 +27,13 @@ import static org.junit.Assert.assertTrue;
  */
 public class DefaultCellComparatorTest {
 
-    private static class TestClass {
-        @Override
-        public String toString() {
-            return SOME;
-        }
-    }
-
     private static final String SOME = "SoMeStRiNg";
     private static final String OTHER = "OtherSTRING";
-    private static final int NUMBER = 42;
-    private static final TestClass OBJECT = new TestClass();
 
     private CellComparator cmp = DefaultCellComparator.INSTANCE;
 
+    // default comparator should detect Comparable<T> impls and use that
+
     @Test
     public void sameString() {
         assertTrue("same string", cmp.compare(SOME, SOME) == 0);
@@ -57,28 +50,98 @@ public class DefaultCellComparatorTest {
     }
 
     @Test
-    public void someVsObject() {
-        assertTrue("some vs object", cmp.compare(SOME, OBJECT) == 0);
+    public void someVsNull() {
+        assertTrue("some vs null", cmp.compare(SOME, null) > 0);
     }
 
     @Test
-    public void otherVsObject() {
-        assertTrue("other vs object", cmp.compare(OTHER, OBJECT) < 0);
+    public void nullVsSome() {
+        assertTrue("null vs some", cmp.compare(null, SOME) < 0);
     }
 
+    @Test(expected = ClassCastException.class)
+    public void mismatch() {
+        cmp.compare(42, SOME);
+    }
+
+
     @Test
-    public void otherVsNumber() {
-        assertTrue("other vs 42", cmp.compare(OTHER, NUMBER) > 0);
+    public void strElevenTwo() {
+        assertTrue("str 11 vs 2", cmp.compare("11", "2") < 0);
     }
 
     @Test
-    public void someVsNull() {
-        assertTrue("some vs null", cmp.compare(SOME, null) > 0);
+    public void intElevenTwo() {
+        assertTrue("int 11 vs 2", cmp.compare(11, 2) > 0);
     }
 
+
     @Test
-    public void nullVsSome() {
-        assertTrue("null vs some", cmp.compare(null, SOME) < 0);
+    public void intSmallBig() {
+        assertTrue("int 2 vs 4", cmp.compare(2, 4) < 0);
+    }
+
+    @Test
+    public void intBigSmall() {
+        assertTrue("int 4 vs 2", cmp.compare(4, 2) > 0);
+    }
+
+    @Test
+    public void intEqual() {
+        assertTrue("int 4 vs 4", cmp.compare(4, 4) == 0);
+    }
+
+    @Test
+    public void longSmallBig() {
+        assertTrue("long 2 vs 4", cmp.compare(2L, 4L) < 0);
     }
 
+    @Test
+    public void longBigSmall() {
+        assertTrue("long 4 vs 2", cmp.compare(4L, 2L) > 0);
+    }
+
+    @Test
+    public void longEqual() {
+        assertTrue("long 4 vs 4", cmp.compare(4L, 4L) == 0);
+    }
+
+
+    private enum SmallStarWars { C3PO, R2D2, LUKE }
+
+    @Test
+    public void swEpisodeI() {
+        assertTrue("c3po r2d2",
+                   cmp.compare(SmallStarWars.C3PO, SmallStarWars.R2D2) < 0);
+    }
+
+    @Test
+    public void swEpisodeII() {
+        assertTrue("r2d2 c3po",
+                   cmp.compare(SmallStarWars.R2D2, SmallStarWars.C3PO) > 0);
+    }
+
+    @Test
+    public void swEpisodeIII() {
+        assertTrue("luke c3po",
+                   cmp.compare(SmallStarWars.LUKE, SmallStarWars.C3PO) > 0);
+    }
+
+    @Test
+    public void swEpisodeIV() {
+        assertTrue("c3po luke",
+                   cmp.compare(SmallStarWars.C3PO, SmallStarWars.LUKE) < 0);
+    }
+
+    @Test
+    public void swEpisodeV() {
+        assertTrue("luke r2d2",
+                   cmp.compare(SmallStarWars.LUKE, SmallStarWars.R2D2) > 0);
+    }
+
+    @Test
+    public void swEpisodeVI() {
+        assertTrue("r2d2 luke",
+                   cmp.compare(SmallStarWars.R2D2, SmallStarWars.LUKE) < 0);
+    }
 }

-/*
- * 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.ui.table.cell;
-
-import org.junit.Test;
-import org.onosproject.ui.table.CellComparator;
-
-import static org.junit.Assert.assertTrue;
-
-/**
- * Unit tests for {@link IntComparator}.
- */
-public class IntComparatorTest {
-
-    private CellComparator cmp = IntComparator.INSTANCE;
-
-    @Test
-    public void twoNulls() {
-        assertTrue("two nulls", cmp.compare(null, null) == 0);
-    }
-
-    @Test
-    public void nullVsNegValue() {
-        assertTrue("null vs neg value", cmp.compare(null, -5) < 0);
-    }
-
-    @Test
-    public void nullVsPosValue() {
-        assertTrue("null vs pos value", cmp.compare(null, 5) < 0);
-    }
-
-    @Test
-    public void negValueVsNull() {
-        assertTrue("neg value vs null", cmp.compare(-5, null) > 0);
-    }
-
-    @Test
-    public void posValueVsNull() {
-        assertTrue("pos value vs null", cmp.compare(5, null) > 0);
-    }
-
-
-    @Test
-    public void smallVsBig() {
-        assertTrue("small vs big", cmp.compare(25, 75) < 0);
-    }
-
-    @Test
-    public void bigVsSmall() {
-        assertTrue("big vs small", cmp.compare(75, 25) > 0);
-    }
-
-    @Test
-    public void sameValue() {
-        assertTrue("same value", cmp.compare(50, 50) == 0);
-    }
-}

-/*
- * 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.ui.table.cell;
-
-import org.junit.Test;
-import org.onosproject.ui.table.CellComparator;
-
-import static org.junit.Assert.assertTrue;
-
-/**
- * Unit tests for {@link LongComparator}.
- */
-public class LongComparatorTest {
-
-    private CellComparator cmp = LongComparator.INSTANCE;
-
-    @Test
-    public void twoNulls() {
-        assertTrue("two nulls", cmp.compare(null, null) == 0);
-    }
-
-    @Test
-    public void nullVsNegValue() {
-        assertTrue("null vs neg value", cmp.compare(null, -5L) < 0);
-    }
-
-    @Test
-    public void nullVsPosValue() {
-        assertTrue("null vs pos value", cmp.compare(null, 5L) < 0);
-    }
-
-    @Test
-    public void negValueVsNull() {
-        assertTrue("neg value vs null", cmp.compare(-5L, null) > 0);
-    }
-
-    @Test
-    public void posValueVsNull() {
-        assertTrue("pos value vs null", cmp.compare(5L, null) > 0);
-    }
-
-
-    @Test
-    public void smallVsBig() {
-        assertTrue("small vs big", cmp.compare(25L, 75L) < 0);
-    }
-
-    @Test
-    public void bigVsSmall() {
-        assertTrue("big vs small", cmp.compare(75L, 25L) > 0);
-    }
-
-    @Test
-    public void sameValue() {
-        assertTrue("same value", cmp.compare(50L, 50L) == 0);
-    }
-}

@@ -26,7 +26,6 @@ import org.onosproject.ui.RequestHandler;
 import org.onosproject.ui.UiMessageHandler;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
-import org.onosproject.ui.table.cell.IntComparator;
 import org.onosproject.ui.table.cell.TimeFormatter;
 
 import java.util.Collection;
@@ -73,7 +72,6 @@ public class ClusterViewMessageHandler extends UiMessageHandler {
         @Override
         protected TableModel createTableModel() {
             TableModel tm = super.createTableModel();
-            tm.setComparator(TCP_PORT, IntComparator.INSTANCE);
             tm.setFormatter(UPDATED, new TimeFormatter());
             return tm;
         }

@@ -31,7 +31,6 @@ import org.onosproject.ui.RequestHandler;
 import org.onosproject.ui.UiMessageHandler;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
-import org.onosproject.ui.table.cell.IntComparator;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -108,13 +107,6 @@ public class DeviceViewMessageHandler extends UiMessageHandler {
         }
 
         @Override
-        protected TableModel createTableModel() {
-            TableModel tm = super.createTableModel();
-            tm.setComparator(NUM_PORTS, IntComparator.INSTANCE);
-            return tm;
-        }
-
-        @Override
         protected void populateTable(TableModel tm, ObjectNode payload) {
             DeviceService ds = get(DeviceService.class);
             MastershipService ms = get(MastershipService.class);

@@ -30,8 +30,6 @@ import org.onosproject.ui.table.CellFormatter;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
 import org.onosproject.ui.table.cell.EnumFormatter;
-import org.onosproject.ui.table.cell.IntComparator;
-import org.onosproject.ui.table.cell.LongComparator;
 
 import java.util.Collection;
 import java.util.List;
@@ -86,13 +84,6 @@ public class FlowViewMessageHandler extends UiMessageHandler {
         @Override
         protected TableModel createTableModel() {
             TableModel tm = super.createTableModel();
-            tm.setComparator(GROUP_ID, IntComparator.INSTANCE);
-            tm.setComparator(TABLE_ID, IntComparator.INSTANCE);
-            tm.setComparator(PRIORITY, IntComparator.INSTANCE);
-            tm.setComparator(TIMEOUT, IntComparator.INSTANCE);
-            tm.setComparator(PACKETS, LongComparator.INSTANCE);
-            tm.setComparator(BYTES, LongComparator.INSTANCE);
-
             tm.setFormatter(SELECTOR, new SelectorFormatter());
             tm.setFormatter(TREATMENT, new TreatmentFormatter());
             tm.setFormatter(STATE, EnumFormatter.INSTANCE);

@@ -29,8 +29,6 @@ import org.onosproject.ui.table.CellFormatter;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
 import org.onosproject.ui.table.cell.EnumFormatter;
-import org.onosproject.ui.table.cell.IntComparator;
-import org.onosproject.ui.table.cell.LongComparator;
 
 import java.util.Collection;
 import java.util.List;
@@ -77,11 +75,6 @@ public class GroupViewMessageHandler extends UiMessageHandler {
         @Override
         protected TableModel createTableModel() {
             TableModel tm = super.createTableModel();
-
-            tm.setComparator(ID, IntComparator.INSTANCE);
-            tm.setComparator(PACKETS, LongComparator.INSTANCE);
-            tm.setComparator(BYTES, LongComparator.INSTANCE);
-
             tm.setFormatter(TYPE, EnumFormatter.INSTANCE);
             tm.setFormatter(BUCKETS, new BucketFormatter());
             return tm;

@@ -37,7 +37,6 @@ import org.onosproject.ui.table.CellFormatter;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
 import org.onosproject.ui.table.cell.AppIdFormatter;
-import org.onosproject.ui.table.cell.IntComparator;
 
 import java.util.Collection;
 import java.util.List;
@@ -87,8 +86,6 @@ public class IntentViewMessageHandler extends UiMessageHandler {
         @Override
         protected TableModel createTableModel() {
             TableModel tm = super.createTableModel();
-            tm.setComparator(PRIORITY, IntComparator.INSTANCE);
-
             tm.setFormatter(APP_ID, AppIdFormatter.INSTANCE);
             tm.setFormatter(RESOURCES, new ResourcesFormatter());
             tm.setFormatter(DETAILS, new DetailsFormatter());

@@ -26,7 +26,6 @@ import org.onosproject.ui.RequestHandler;
 import org.onosproject.ui.UiMessageHandler;
 import org.onosproject.ui.table.TableModel;
 import org.onosproject.ui.table.TableRequestHandler;
-import org.onosproject.ui.table.cell.LongComparator;
 
 import java.util.Collection;
 
@@ -72,19 +71,6 @@ public class PortViewMessageHandler extends UiMessageHandler {
         }
 
         @Override
-        protected TableModel createTableModel() {
-            TableModel tm = super.createTableModel();
-            tm.setComparator(PKT_RX, LongComparator.INSTANCE);
-            tm.setComparator(PKT_TX, LongComparator.INSTANCE);
-            tm.setComparator(BYTES_RX, LongComparator.INSTANCE);
-            tm.setComparator(BYTES_TX, LongComparator.INSTANCE);
-            tm.setComparator(PKT_RX_DRP, LongComparator.INSTANCE);
-            tm.setComparator(PKT_TX_DRP, LongComparator.INSTANCE);
-            tm.setComparator(DURATION, LongComparator.INSTANCE);
-            return tm;
-        }
-
-        @Override
         protected void populateTable(TableModel tm, ObjectNode payload) {
             String uri = string(payload, "devId");
             if (!Strings.isNullOrEmpty(uri)) {