ONOS-1721 - GUI -- Websocket Service unit tests written, topo nav glyph added, o…

…ther minor improvements.

Change-Id: I8199024e884d8538cd7c7d891d4fb4c81541150d

@@ -80,7 +80,7 @@
             ev = JSON.parse(msgEvent.data);
         } catch (e) {
             $log.error('Message.data is not valid JSON', msgEvent.data, e);
-            return;
+            return null;
         }
         $log.debug(' << *Rx* ', ev.event, ev.payload);
 
@@ -89,6 +89,7 @@
                 h(ev.payload);
             } catch (e) {
                 $log.error('Problem handling event:', ev, e);
+                return null;
             }
         } else {
             $log.warn('Unhandled event:', ev);
@@ -120,8 +121,7 @@
 
     function findGuiSuccessor() {
         var ncn = clusterNodes.length,
-            ip = undefined,
-            node;
+            ip, node;
 
         while (connectRetries < ncn && !ip) {
             connectRetries++;
@@ -134,7 +134,7 @@
     }
 
     function informListeners(host, url) {
-        angular.forEach(openListeners, function(lsnr) {
+        angular.forEach(openListeners, function (lsnr) {
             lsnr.cb(host, url);
         });
     }
@@ -144,6 +144,14 @@
         ws.send(JSON.stringify(ev));
     }
 
+    function noHandlersWarn(handlers, caller) {
+        if (!handlers || fs.isEmptyObject(handlers)) {
+            $log.warn('WSS.' + caller + '(): no event handlers');
+            return true;
+        }
+        return false;
+    }
+
     // ===================
     // === API Functions
 
@@ -151,6 +159,21 @@
     function resetSid() {
         sid = 0;
     }
+    function resetState() {
+        webSockOpts = undefined;
+        ws = null;
+        wsUp = false;
+        host = undefined;
+        url = undefined;
+        pendingEvents = [];
+        handlers = {};
+        sid = 0;
+        clusterNodes = [];
+        clusterIndex = -1;
+        connectRetries = 0;
+        openListeners = {};
+        nextListenerId = 1;
+    }
 
     // Currently supported opts:
     //   wsport: web socket port (other than default 8181)
@@ -181,9 +204,14 @@
     //     * an API object which has an event handler for the key
     //
     function bindHandlers(handlerMap) {
-        var m = d3.map(handlerMap),
+        var m,
             dups = [];
 
+        if (noHandlersWarn(handlerMap, 'bindHandlers')) {
+            return null;
+        }
+        m = d3.map(handlerMap);
+
         m.forEach(function (eventId, api) {
             var fn = fs.isF(api) || fs.isF(api[eventId]);
             if (!fn) {
@@ -205,7 +233,12 @@
     // Unbinds the specified message handlers.
     //   Expected that the same map will be used, but we only care about keys
     function unbindHandlers(handlerMap) {
-        var m = d3.map(handlerMap);
+        var m;
+
+        if (noHandlersWarn(handlerMap, 'unbindHandlers')) {
+            return null;
+        }
+        m = d3.map(handlerMap);
 
         m.forEach(function (eventId) {
             delete handlers[eventId];
@@ -227,8 +260,14 @@
     }
 
     function removeOpenListener(lsnr) {
-        var id = lsnr && lsnr.id,
+        var id = fs.isO(lsnr) && lsnr.id,
+            o;
+        if (!id) {
+            $log.warn('WSS.removeOpenListener(): invalid listener', lsnr);
+            return null;
+        }
         o = openListeners[id];
+
         if (o) {
             delete openListeners[id];
         }
@@ -270,6 +309,7 @@
 
             return {
                 resetSid: resetSid,
+                resetState: resetState,
                 createWebSocket: createWebSocket,
                 bindHandlers: bindHandlers,
                 unbindHandlers: unbindHandlers,

@@ -114,6 +114,18 @@
             "-2,4.1-2.9,7-2.9c2.9,0,5.1,0.9,6.9,2.9c5,5.4,5.6,17.1,5.4,22.6" +
             "h-25C42.3,43.1,43.1,31.5,48.1,26.1z",
 
+            topo: 'M97.2,76.3H86.6l-7.7-21.9H82c1,0,1.9-0.8,1.9-1.9V35.7c' +
+            '0-1-0.8-1.9-1.9-1.9H65.2c-1,0-1.9,0.8-1.9,1.9v2.6L33.4,26.1v-11' +
+            'c0-1-0.8-1.9-1.9-1.9H14.7c-1,0-1.9,0.8-1.9,1.9v16.8c0,1,0.8,' +
+            '1.9,1.9,1.9h16.8c1,0,1.9-0.8,1.9-1.9v-2.6l29.9,12.2v9L30.5,76.9' +
+            'c-0.3-0.3-0.8-0.5-1.3-0.5H12.4c-1,0-1.9,0.8-1.9,1.9V95c0,1,0.8,' +
+            '1.9,1.9,1.9h16.8c1,0,1.9-0.8,1.9-1.9v-6.9h47.4V95c0,1,0.8,1.9,' +
+            '1.9,1.9h16.8c1,0,1.9-0.8,1.9-1.9V78.2C99.1,77.2,98.2,76.3,97.2,' +
+            '76.3z M31.1,85.1v-4.9l32.8-26.4c0.3,0.3,0.8,0.5,1.3,0.5h10.5l' +
+            '7.7,21.9h-3c-1,0-1.9,0.8-1.9,1.9v6.9H31.1z',
+
+            // --- Navigation glyphs ------------------------------------
+
             flowTable: 'M15.9,19.1h-8v-13h8V19.1z M90.5,6.1H75.6v13h14.9V6.1z' +
             ' M71.9,6.1H56.9v13h14.9V6.1z M53.2,6.1H38.3v13h14.9V6.1z M34.5,' +
             '6.1H19.6v13h14.9V6.1z M102.2,6.1h-8v13h8V6.1z M102.2,23.6H7.9v' +

@@ -51,7 +51,7 @@
 
         nav_apps: 'bird',
         nav_cluster: 'node',
-        nav_topo: 'unknown', // TODO: need a topology glyph
+        nav_topo: 'topo',
         nav_devs: 'switch',
         nav_links: 'ports',
         nav_hosts: 'endstation',

@@ -252,7 +252,7 @@
                 cb: function () {
                     ns.navTo(flowPath, { devId: data.props['URI'] });
                 },
-                tt: 'Show flows for this device'
+                tt: 'Show flows table for this device'
             });
         }
 

@@ -78,7 +78,7 @@
                 self.$route = $route;
                 self.$routeParams = $routeParams;
                 self.$location = $location;
-                self.version = '1.3.0';
+                self.version = '1.2.0';
 
                 // initialize services...
                 ts.init();

@@ -20,8 +20,34 @@
 describe('factory: fw/remote/websocket.js', function () {
     var $log, fs, wss;
 
+    var noop = function () {},
+        send = jasmine.createSpy('send').and.callFake(function (ev) {
+            return ev;
+        }),
+        mockWebSocket = {
+            send: send
+        };
+
     beforeEach(module('onosRemote', 'onosLayer', 'ngRoute', 'onosNav', 'onosSvg'));
 
+    beforeEach(function () {
+        mockWebSocket = {
+            send: send
+        };
+    });
+
+    beforeEach(function () {
+        module(function ($provide) {
+            $provide.factory('WSock', function () {
+                return {
+                    newWebSocket: function () {
+                        return mockWebSocket;
+                    }
+                };
+            });
+        });
+    });
+
     beforeEach(module(function($provide) {
         $provide.factory('$location', function () {
             return {
@@ -36,6 +62,7 @@ describe('factory: fw/remote/websocket.js', function () {
         $log = _$log_;
         fs = FnService;
         wss = WebSocketService;
+        wss.resetState();
     }));
 
 
@@ -45,21 +72,197 @@ describe('factory: fw/remote/websocket.js', function () {
 
     it('should define api functions', function () {
         expect(fs.areFunctions(wss, [
-            'resetSid', 'createWebSocket', 'bindHandlers', 'unbindHandlers',
+            'resetSid', 'resetState',
+            'createWebSocket', 'bindHandlers', 'unbindHandlers',
             'addOpenListener', 'removeOpenListener', 'sendEvent'
         ])).toBeTruthy();
     });
 
-    it('should use the appropriate URL', function () {
+    it('should use the appropriate URL, createWebsocket', function () {
         var url = wss.createWebSocket();
         expect(url).toEqual('ws://foo:80/onos/ui/websock/core');
     });
 
-    it('should use the appropriate URL with modified port', function () {
+    it('should use the appropriate URL with modified port, createWebsocket',
+        function () {
             var url = wss.createWebSocket({ wsport: 1243 });
             expect(url).toEqual('ws://foo:1243/onos/ui/websock/core');
     });
 
-    // TODO: inject mock WSock service and write more tests ...
+    it('should verify websocket event handlers, createWebsocket', function () {
+        wss.createWebSocket({ wsport: 1234 });
+        expect(fs.isF(mockWebSocket.onopen)).toBeTruthy();
+        expect(fs.isF(mockWebSocket.onmessage)).toBeTruthy();
+        expect(fs.isF(mockWebSocket.onclose)).toBeTruthy();
+    });
+
+    it('should invoke listener callbacks when websocket is up, handleOpen',
+        function () {
+            var num = 0;
+            function incrementNum() { num++; }
+            wss.addOpenListener(incrementNum);
+            wss.createWebSocket({ wsport: 1234 });
+            mockWebSocket.onopen();
+            expect(num).toBe(1);
+    });
+
+    it('should send pending events, handleOpen', function () {
+        var fakeEvent = {
+            event: 'mockEv',
+            sid: 1,
+            payload: { mock: 'thing' }
+        };
+        wss.sendEvent(fakeEvent.event, fakeEvent.payload);
+        expect(mockWebSocket.send).not.toHaveBeenCalled();
+        wss.createWebSocket({ wsport: 1234 });
+        mockWebSocket.onopen();
+        expect(mockWebSocket.send).toHaveBeenCalledWith(JSON.stringify(fakeEvent));
+    });
+
+    it('should handle an incoming bad JSON message, handleMessage', function () {
+        spyOn($log, 'error');
+        var badMsg = {
+            data: 'bad message'
+        };
+        wss.createWebSocket({ wsport: 1234 });
+        expect(mockWebSocket.onmessage(badMsg)).toBeNull();
+        expect($log.error).toHaveBeenCalled();
+    });
+
+    it('should verify message was handled, handleMessage', function () {
+        var num = 0,
+            fakeHandler = {
+                mockEvResp: function () { num++; }
+            },
+            data = JSON.stringify({
+                event: 'mockEvResp',
+                payload: {}
+            }),
+            event = {
+                data: data
+            };
+        wss.createWebSocket({ wsport: 1234 });
+        wss.bindHandlers(fakeHandler);
+        expect(mockWebSocket.onmessage(event)).toBe(undefined);
+        expect(num).toBe(1);
+    });
+
+    it('should warn if there is an unhandled event, handleMessage', function () {
+        spyOn($log, 'warn');
+        var data = { foo: 'bar', bar: 'baz'},
+            dataString = JSON.stringify(data),
+            badEv = {
+                data: dataString
+            };
+        wss.createWebSocket({ wsport: 1234 });
+        mockWebSocket.onmessage(badEv);
+        expect($log.warn).toHaveBeenCalledWith('Unhandled event:', data);
+    });
+
+    it('should not warn if valid input, bindHandlers', function () {
+        spyOn($log, 'warn');
+        expect(wss.bindHandlers({
+            foo: noop,
+            bar: noop
+        })).toBe(undefined);
+        expect($log.warn).not.toHaveBeenCalled();
+    });
+
+    it('should warn if no arguments, bindHandlers', function () {
+        spyOn($log, 'warn');
+        expect(wss.bindHandlers()).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.bindHandlers(): no event handlers'
+        );
+        expect(wss.bindHandlers({})).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.bindHandlers(): no event handlers'
+        );
+    });
+
+    it('should warn if handler is not a function, bindHandlers', function () {
+        spyOn($log, 'warn');
+        expect(wss.bindHandlers({
+            foo: 'handler1',
+            bar: 3,
+            baz: noop
+        })).toBe(undefined);
+        expect($log.warn).toHaveBeenCalledWith('foo handler not a function');
+        expect($log.warn).toHaveBeenCalledWith('bar handler not a function');
+    });
+
+    it('should warn if duplicate handlers were given, bindHandlers',
+        function () {
+            spyOn($log, 'warn');
+            wss.bindHandlers({
+                foo: noop
+            });
+            expect(wss.bindHandlers({
+                foo: noop
+            })).toBe(undefined);
+            expect($log.warn).toHaveBeenCalledWith('duplicate bindings ignored:',
+                                                    ['foo']);
+    });
+
+    it('should warn if no arguments, unbindHandlers', function () {
+        spyOn($log, 'warn');
+        expect(wss.unbindHandlers()).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.unbindHandlers(): no event handlers'
+        );
+        expect(wss.unbindHandlers({})).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.unbindHandlers(): no event handlers'
+        );
+    });
+    // Note: cannot test unbindHandlers' forEach due to it using closure variable
+
+    it('should not warn if valid argument, addOpenListener', function () {
+        spyOn($log, 'warn');
+        var o = wss.addOpenListener(noop);
+        expect(o.id === 1);
+        expect(o.cb === noop);
+        expect($log.warn).not.toHaveBeenCalled();
+        o = wss.addOpenListener(noop);
+        expect(o.id === 2);
+        expect(o.cb === noop);
+        expect($log.warn).not.toHaveBeenCalled();
+    });
+
+    it('should log error if callback not a function, addOpenListener',
+        function () {
+            spyOn($log, 'error');
+            var o = wss.addOpenListener('foo');
+            expect(o.id === 1);
+            expect(o.cb === 'foo');
+            expect(o.error === 'No callback defined');
+            expect($log.error).toHaveBeenCalledWith(
+                'WSS.addOpenListener(): callback not a function'
+            );
+    });
+
+    it('should not warn if valid listener object, removeOpenListener', function () {
+        spyOn($log, 'warn');
+        expect(wss.removeOpenListener({
+            id: 1,
+            cb: noop
+        })).toBe(undefined);
+        expect($log.warn).not.toHaveBeenCalled();
+    });
+
+    it('should warn if listener is invalid, removeOpenListener', function () {
+        spyOn($log, 'warn');
+        expect(wss.removeOpenListener({})).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.removeOpenListener(): invalid listener', {}
+        );
+        expect(wss.removeOpenListener('listener')).toBeNull();
+        expect($log.warn).toHaveBeenCalledWith(
+            'WSS.removeOpenListener(): invalid listener', 'listener'
+        );
+    });
+
+    // Note: handleClose is not currently tested due to all work it does relies
+    //       on closure variables that cannot be mocked
 
 });

@@ -20,7 +20,7 @@
 describe('factory: fw/svg/glyph.js', function() {
     var $log, fs, gs, d3Elem, svg;
 
-    var numBaseGlyphs = 36,
+    var numBaseGlyphs = 38,
         vbBird = '352 224 113 112',
         vbGlyph = '0 0 110 110',
         vbBadge = '0 0 10 10',
@@ -38,6 +38,10 @@ describe('factory: fw/svg/glyph.js', function() {
             chain: 'M60.4,77.6c-',
             crown: 'M99.5,21.6c0,',
             lock: 'M79.4,48.6h',
+            topo: 'M97.2,76.3H86.6',
+
+            // navigation specific glyphs
+            flowTable: 'M15.9,19.1h-8v-13h',
 
             // toolbar specific glyphs
             summary: longPrefix + 'M16.7',
@@ -75,7 +79,7 @@ describe('factory: fw/svg/glyph.js', function() {
         },
         glyphIds = [
             'unknown', 'node', 'switch', 'roadm', 'endstation', 'router',
-            'bgpSpeaker', 'chain', 'crown', 'lock',
+            'bgpSpeaker', 'chain', 'crown', 'lock', 'topo', 'flowTable',
             'summary', 'details', 'ports', 'map', 'cycleLabels', 'oblique',
             'filters', 'resetZoom', 'relatedIntents', 'nextIntent',
             'prevIntent', 'intentTraffic', 'allTraffic', 'flows', 'eqMaster'

@@ -30,7 +30,6 @@ describe('factory: fw/widget/tableBuilder.js', function () {
 
     beforeEach(module('onosWidget', 'onosUtil', 'onosRemote'));
 
-    // TODO: actual websocket calls should be tested in the websocket service
     beforeEach(function () {
         module(function ($provide) {
             $provide.value('WebSocketService', mockWss);
@@ -93,8 +92,4 @@ describe('factory: fw/widget/tableBuilder.js', function () {
         expect(mockWss.unbindHandlers).toHaveBeenCalled();
     });
 
-    // TODO: figure out how to test respCb.
-    // should it just be verified by the fact that the callback function
-    // is called by the wss?
-
 });

@@ -29,7 +29,7 @@ describe('Controller: OnosCtrl', function () {
         ctrl = $controller('OnosCtrl');
     }));
 
-    it('should report version 1.3.0', function () {
-        expect(ctrl.version).toEqual('1.3.0');
+    it('should report version 1.2.0', function () {
+        expect(ctrl.version).toEqual('1.2.0');
     });
 });
\ No newline at end of file