[TimeAPI] check for change before triggering

Update the TimeSettingsURLHandler to check for changes to the search
parameters and only trigger a bounds change when there is a change.

Added integration tests between Time API and settings url handler.

Prevents extraneous bounds events.

Fixes https://github.com/nasa/openmct/issues/1636

src/adapter/runs/TimeSettingsURLHandler.js

@@ -20,7 +20,11 @@
  * at runtime from the About dialog for additional information.
  *****************************************************************************/
 
-define([], function () {
+define([
+    'lodash'
+], function (
+    _
+) {
     // Parameter names in query string
     var SEARCH = {
         MODE: 'tc.mode',
@@ -73,43 +77,70 @@ define([], function () {
         this.$location.search(SEARCH.END_DELTA, deltas.end);
     };
 
-    TimeSettingsURLHandler.prototype.updateTime = function () {
+    TimeSettingsURLHandler.prototype.parseQueryParams = function () {
-        var searchParams = this.$location.search();
+        var searchParams = _.pick(this.$location.search(), _.values(SEARCH));
-        var mode = searchParams[SEARCH.MODE];
+        var parsedParams = {
-        var timeSystem = searchParams[SEARCH.TIME_SYSTEM];
+            clock: searchParams[SEARCH.MODE],
-        var clockOffsets = {
+            timeSystem: searchParams[SEARCH.TIME_SYSTEM]
+        };
+        if (!isNaN(parseInt(searchParams[SEARCH.START_DELTA], 0xA)) &&
+            !isNaN(parseInt(searchParams[SEARCH.END_DELTA], 0xA))) {
+            parsedParams.clockOffsets = {
                 start: -searchParams[SEARCH.START_DELTA],
                 end: +searchParams[SEARCH.END_DELTA]
             };
-        var bounds = {
+        }
+        if (!isNaN(parseInt(searchParams[SEARCH.START_BOUND], 0xA)) &&
+            !isNaN(parseInt(searchParams[SEARCH.END_BOUND], 0xA))) {
+            parsedParams.bounds = {
                 start: +searchParams[SEARCH.START_BOUND],
                 end: +searchParams[SEARCH.END_BOUND]
             };
-        var fixed = (mode === 'fixed');
+        }
-        var clock = fixed ? undefined : mode;
+        return parsedParams;
-        var hasDeltas =
+    };
-            !isNaN(parseInt(searchParams[SEARCH.START_DELTA], 0xA)) &&
-            !isNaN(parseInt(searchParams[SEARCH.END_DELTA], 0xA));
-        var hasBounds =
-            !isNaN(parseInt(searchParams[SEARCH.START_BOUND], 0xA)) &&
-            !isNaN(parseInt(searchParams[SEARCH.END_BOUND], 0xA));
 
-        if (fixed && timeSystem && hasBounds) {
+    TimeSettingsURLHandler.prototype.updateTime = function () {
-            this.time.timeSystem(timeSystem, bounds);
+        var params = this.parseQueryParams();
+        if (_.isEqual(params, this.last)) {
+            return; // Do nothing;
+        }
+        this.last = params;
+
+        if (params.bounds) {
+            if (!this.time.timeSystem() ||
+                this.time.timeSystem().key !== params.timeSystem) {
+
+                this.time.timeSystem(
+                    params.timeSystem,
+                    params.bounds
+                );
+            } else if (!_.isEqual(this.time.bounds(), params.bounds)) {
+                this.time.bounds(params.bounds);
+            }
+            if (this.time.clock()) {
                 this.time.stopClock();
             }
-
+        } else if (params.clockOffsets) {
-        if (!fixed && clock && hasDeltas) {
+            if (params.clock === 'fixed') {
-            this.time.clock(clock, clockOffsets);
+                this.time.stopClock();
-            this.time.timeSystem(timeSystem);
+                return;
             }
+            if (!this.time.clock() ||
+                this.time.clock().key !== params.clock) {
 
-        if (hasDeltas && !fixed) {
+                this.time.clock(params.clock, params.clockOffsets);
-            this.time.clockOffsets(clockOffsets);
+            } else if (!_.isEqual(this.time.clockOffsets(), params.clockOffsets)) {
+                this.time.clockOffsets(params.clockOffsets);
             }
+            if (!this.time.timeSystem() ||
+                this.time.timeSystem().key !== params.timeSystem) {
 
-        if (hasBounds && fixed) {
+                this.time.timeSystem(params.timeSystem);
-            this.time.bounds(bounds);
+            }
+        } else {
+            // Neither found, update from timeSystem.
+            this.updateQueryParams();
         }
     };
 

src/adapter/runs/TimeSettingsURLHandlerSpec.js

@@ -20,23 +20,74 @@
  * at runtime from the About dialog for additional information.
  *****************************************************************************/
 
-define(['./TimeSettingsURLHandler'], function (TimeSettingsURLHandler) {
+define([
+    './TimeSettingsURLHandler',
+    '../../api/time/TimeAPI'
+], function (
+    TimeSettingsURLHandler,
+    TimeAPI
+) {
     describe("TimeSettingsURLHandler", function () {
         var time;
         var $location;
         var $rootScope;
         var search;
         var handler;
+        var clockA;
+        var clockB;
+        var timeSystemA;
+        var timeSystemB;
+        var boundsA;
+        var boundsB;
+        var offsetsA;
+        var offsetsB;
+        var initialize;
+        var triggerLocationChange;
+
 
         beforeEach(function () {
-            time = jasmine.createSpyObj('time', [
+            clockA = jasmine.createSpyObj('clockA', ['on', 'off']);
+            clockA.key = 'clockA';
+            clockA.currentValue = function () { return 1000; }
+            clockB = jasmine.createSpyObj('clockB', ['on', 'off']);
+            clockB.key = 'clockB';
+            clockB.currentValue = function () { return 2000; }
+            timeSystemA = {key: 'timeSystemA'};
+            timeSystemB = {key: 'timeSystemB'};
+            boundsA = {
+                start: 10,
+                end: 20
+            };
+            boundsB = {
+                start: 120,
+                end: 360
+            };
+            offsetsA = {
+                start: -100,
+                end: 0
+            };
+            offsetsB = {
+                start: -50,
+                end: 50
+            };
+
+            time = new TimeAPI();
+
+            [
                 'on',
                 'bounds',
                 'clockOffsets',
                 'timeSystem',
                 'clock',
                 'stopClock'
-            ]);
+            ].forEach(function (method) {
+                spyOn(time, method).andCallThrough();
+            });
+            time.addTimeSystem(timeSystemA);
+            time.addTimeSystem(timeSystemB);
+            time.addClock(clockA);
+            time.addClock(clockB);
+
             $location = jasmine.createSpyObj('$location', [
                 'search'
             ]);
@@ -44,8 +95,6 @@ define(['./TimeSettingsURLHandler'], function (TimeSettingsURLHandler) {
                 '$on'
             ]);
 
-            time.timeSystem.andReturn({ key: 'test-time-system' });
-
             search = {};
             $location.search.andCallFake(function (key, value) {
                 if (arguments.length === 0) {
@@ -59,142 +108,451 @@ define(['./TimeSettingsURLHandler'], function (TimeSettingsURLHandler) {
                 return this;
             });
 
+            expect(time.timeSystem()).toBeUndefined();
+            expect(time.bounds()).toEqual({});
+            expect(time.clockOffsets()).toBeUndefined();
+            expect(time.clock()).toBeUndefined();
+
+            initialize = function () {
                 handler = new TimeSettingsURLHandler(
                     time,
                     $location,
                     $rootScope
                 );
-        });
+                expect($rootScope.$on).toHaveBeenCalledWith(
+                    '$locationChangeSuccess',
+                    jasmine.any(Function)
+                );
+                triggerLocationChange = $rootScope.$on.mostRecentCall.args[1];
 
-        ['bounds', 'timeSystem', 'clock', 'clockOffsets'].forEach(function (event) {
-            it("listens for " + event + " time events", function () {
-                expect(time.on)
-                    .toHaveBeenCalledWith(event, jasmine.any(Function));
-            });
-
-            describe("when " + event + " time event occurs with no clock", function () {
-                var expected;
-
-                beforeEach(function () {
-                    expected = {
-                        'tc.mode': 'fixed',
-                        'tc.timeSystem': 'test-time-system',
-                        'tc.startBound': '123',
-                        'tc.endBound': '456'
             };
-                    time.clock.andReturn(undefined);
+        });
-                    time.bounds.andReturn({ start: 123, end: 456 });
 
-                    time.on.calls.forEach(function (call) {
+        it("can initalize fixed mode from location", function () {
-                        if (call.args[0] === event) {
+            search['tc.mode'] = 'fixed';
-                            call.args[1]();
+            search['tc.timeSystem'] = 'timeSystemA';
+            search['tc.startBound'] = '123';
+            search['tc.endBound'] = '456';
+
+            initialize();
+
+            expect(time.timeSystem).toHaveBeenCalledWith(
+                'timeSystemA',
+                {
+                    start: 123,
+                    end: 456
                 }
-                    });
+            );
         });
 
-                it("updates query parameters for fixed mode", function () {
+        it("can initialize clock mode from location", function () {
-                    expect(search).toEqual(expected);
+            search['tc.mode'] = 'clockA';
-                });
+            search['tc.timeSystem'] = 'timeSystemA';
-            });
+            search['tc.startDelta'] = '123';
+            search['tc.endDelta'] = '456';
 
-            describe("when " + event + " time event occurs with no time system", function () {
+            initialize();
-                beforeEach(function () {
+
-                    time.timeSystem.andReturn(undefined);
+            expect(time.clock).toHaveBeenCalledWith(
-                    time.on.calls.forEach(function (call) {
+                'clockA',
-                        if (call.args[0] === event) {
+                {
-                            call.args[1]();
+                    start: -123,
+                    end: 456
                 }
-                    });
+            );
+            expect(time.timeSystem).toHaveBeenCalledWith(
+                'timeSystemA'
+            );
         });
 
-                it("clears the time system from the URL", function () {
+        it("can initialize fixed mode from time API", function () {
-                    expect(search['tc.timeSystem']).toBeUndefined();
+            time.timeSystem(timeSystemA.key, boundsA);
-                });
+            initialize();
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.mode', 'fixed');
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.timeSystem', 'timeSystemA');
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.startBound', 10);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.endBound', 20);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.startDelta', null);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.endDelta', null);
         });
 
-            describe("when " + event + " time event occurs with a clock", function () {
+        it("can initialize clock mode from time API", function () {
-                var expected;
+            time.clock(clockA.key, offsetsA);
+            time.timeSystem(timeSystemA.key);
+            initialize();
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.mode', 'clockA');
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.timeSystem', 'timeSystemA');
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.startBound', null);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.endBound', null);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.startDelta', 100);
+            expect($location.search)
+                .toHaveBeenCalledWith('tc.endDelta', 0);
+        });
+
+        describe('location changes in fixed mode', function () {
 
             beforeEach(function () {
-                    expected = {
+                time.timeSystem(timeSystemA.key, boundsA);
-                        'tc.mode': 'clocky',
+                initialize();
-                        'tc.timeSystem': 'test-time-system',
+                time.timeSystem.reset();
-                        'tc.startDelta': '123',
+                time.bounds.reset();
-                        'tc.endDelta': '456'
+                time.clock.reset();
-                    };
+                time.stopClock.reset();
-                    time.clock.andReturn({ key: 'clocky' });
+            });
-                    time.clockOffsets.andReturn({ start: -123, end: 456 });
 
-                    time.on.calls.forEach(function (call) {
+            it("does not change on spurious location change", function () {
-                        if (call.args[0] === event) {
+                triggerLocationChange();
-                            call.args[1]();
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    'timeSystemA',
+                    jasmine.any(Object)
+                );
+                expect(time.bounds).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+                expect(time.stopClock).not.toHaveBeenCalled();
+            });
+
+            it("updates timeSystem changes", function () {
+                search['tc.timeSystem'] = 'timeSystemB';
+                triggerLocationChange();
+                expect(time.timeSystem).toHaveBeenCalledWith(
+                    'timeSystemB',
+                    {
+                        start: 10,
+                        end: 20
                     }
+                );
+            });
+
+            it("updates bounds changes", function () {
+                search['tc.startBound'] = '100';
+                search['tc.endBound'] = '200';
+                triggerLocationChange();
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    jasmine.any(), jasmine.any()
+                );
+                expect(time.bounds).toHaveBeenCalledWith({
+                    start: 100,
+                    end: 200
+                });
+                search['tc.endBound'] = '300';
+                triggerLocationChange();
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    jasmine.any(), jasmine.any()
+                );
+                expect(time.bounds).toHaveBeenCalledWith({
+                    start: 100,
+                    end: 300
                 });
             });
 
-                it("updates query parameters for realtime mode", function () {
+            it("updates clock mode w/o timeSystem change", function () {
-                    expect(search).toEqual(expected);
+                search['tc.mode'] = 'clockA';
+                search['tc.startDelta'] = '50';
+                search['tc.endDelta'] = '50';
+                delete search['tc.endBound'];
+                delete search['tc.startBound'];
+                triggerLocationChange();
+                expect(time.clock).toHaveBeenCalledWith(
+                    'clockA',
+                    {
+                        start: -50,
+                        end: 50
+                    }
+                );
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    jasmine.any(), jasmine.any()
+                );
             });
+
+            it("updates clock mode and timeSystem", function () {
+                search['tc.mode'] = 'clockA';
+                search['tc.startDelta'] = '50';
+                search['tc.endDelta'] = '50';
+                search['tc.timeSystem'] = 'timeSystemB';
+                delete search['tc.endBound'];
+                delete search['tc.startBound'];
+                triggerLocationChange();
+                expect(time.clock).toHaveBeenCalledWith(
+                    'clockA',
+                    {
+                        start: -50,
+                        end: 50
+                    }
+                );
+                expect(time.timeSystem).toHaveBeenCalledWith('timeSystemB');
             });
         });
 
-        it("listens for location changes", function () {
+        describe('location changes in clock mode', function () {
-            expect($rootScope.$on)
-                .toHaveBeenCalledWith('$locationChangeSuccess', jasmine.any(Function));
-        });
 
-        [false, true].forEach(function (fixed) {
-            var name = fixed ? "fixed-time" : "real-time";
-            var suffix = fixed ? 'Bound' : 'Delta';
-            describe("when " + name + " location changes occur", function () {
             beforeEach(function () {
-                    search['tc.mode'] = fixed ? 'fixed' : 'clocky';
+                time.clock(clockA.key, offsetsA);
-                    search['tc.timeSystem'] = 'some-time-system';
+                time.timeSystem(timeSystemA.key);
-                    search['tc.start' + suffix] = '12321';
+                initialize();
-                    search['tc.end' + suffix] = '32123';
+                time.timeSystem.reset();
-                    $rootScope.$on.mostRecentCall.args[1]();
+                time.bounds.reset();
+                time.clock.reset();
+                time.clockOffsets.reset();
+                time.stopClock.reset();
+            });
+
+            it("does not change on spurious location change", function () {
+                triggerLocationChange();
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    'timeSystemA',
+                    jasmine.any(Object)
+                );
+                expect(time.clockOffsets).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+                expect(time.clock).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+                expect(time.bounds).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+            });
+
+            it("changes time system", function () {
+                search['tc.timeSystem'] = 'timeSystemB';
+                triggerLocationChange();
+                expect(time.timeSystem).toHaveBeenCalledWith(
+                    'timeSystemB'
+                );
+                expect(time.clockOffsets).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+                expect(time.clock).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+                expect(time.stopClock).not.toHaveBeenCalled();
+                expect(time.bounds).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+            });
+
+            it("changes offsets", function () {
+                search['tc.startDelta'] = '50';
+                search['tc.endDelta'] = '50';
+                triggerLocationChange();
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    'timeSystemA',
+                    jasmine.any(Object)
+                );
+                expect(time.clockOffsets).toHaveBeenCalledWith(
+                    {
+                        start: -50,
+                        end: 50
+                    }
+                );
+                expect(time.clock).not.toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+            });
+
+            it("updates to fixed w/o timeSystem change", function () {
+                search['tc.mode'] = 'fixed';
+                search['tc.startBound'] = '234';
+                search['tc.endBound'] = '567';
+                delete search['tc.endDelta'];
+                delete search['tc.startDelta'];
+
+                triggerLocationChange();
+                expect(time.stopClock).toHaveBeenCalled();
+                expect(time.bounds).toHaveBeenCalledWith({
+                    start: 234,
+                    end: 567
+                });
+                expect(time.timeSystem).not.toHaveBeenCalledWith(
+                    jasmine.any(), jasmine.any()
+                );
+            });
+
+            it("updates fixed and timeSystem", function () {
+                search['tc.mode'] = 'fixed';
+                search['tc.startBound'] = '234';
+                search['tc.endBound'] = '567';
+                search['tc.timeSystem'] = 'timeSystemB';
+                delete search['tc.endDelta'];
+                delete search['tc.startDelta'];
+
+                triggerLocationChange();
+                expect(time.stopClock).toHaveBeenCalled();
+                expect(time.timeSystem).toHaveBeenCalledWith(
+                    'timeSystemB',
+                    {
+                        start: 234,
+                        end: 567
+                    }
+                );
+            });
+
+            it("updates clock", function () {
+                search['tc.mode'] = 'clockB';
+                triggerLocationChange();
+                expect(time.clock).toHaveBeenCalledWith(
+                    'clockB',
+                    {
+                        start: -100,
+                        end: 0
+                    }
+                );
+                expect(time.timeSystem).not.toHaveBeenCalledWith(jasmine.any());
+            });
+
+            it("updates clock and timeSystem", function () {
+                search['tc.mode'] = 'clockB';
+                search['tc.timeSystem'] = 'timeSystemB';
+                triggerLocationChange();
+                expect(time.clock).toHaveBeenCalledWith(
+                    'clockB',
+                    {
+                        start: -100,
+                        end: 0
+                    }
+                );
+                expect(time.timeSystem).toHaveBeenCalledWith(
+                    'timeSystemB'
+                );
+            });
+
+            it("updates clock and timeSystem and offsets", function () {
+                search['tc.mode'] = 'clockB';
+                search['tc.timeSystem'] = 'timeSystemB';
+                search['tc.startDelta'] = '50';
+                search['tc.endDelta'] = '50';
+                triggerLocationChange();
+                expect(time.clock).toHaveBeenCalledWith(
+                    'clockB',
+                    {
+                        start: -50,
+                        end: 50
+                    }
+                );
+                expect(time.timeSystem).toHaveBeenCalledWith(
+                    'timeSystemB'
+                );
             });
 
-                if (fixed) {
-                    var bounds = { start: 12321, end: 32123 };
             it("stops the clock", function () {
+                // this is a robustness test, unsure if desired, requires
+                // user to be manually editing location strings.
+                search['tc.mode'] = 'fixed';
+                triggerLocationChange();
                 expect(time.stopClock).toHaveBeenCalled();
             });
-
-                    it("sets the bounds", function () {
-                        expect(time.bounds).toHaveBeenCalledWith(bounds);
         });
 
-                    it("sets the time system with bounds", function () {
-                        expect(time.timeSystem).toHaveBeenCalledWith(
-                            search['tc.timeSystem'],
-                            bounds
-                        );
-                    });
-                } else {
-                    var clockOffsets = { start: -12321, end: 32123 };
 
-                    it("sets the clock", function () {
+        describe("location updates from time API in fixed", function () {
-                        expect(time.stopClock).not.toHaveBeenCalled();
+            beforeEach(function () {
-                        expect(time.clock).toHaveBeenCalledWith(
+                time.timeSystem(timeSystemA.key, boundsA);
-                            search['tc.mode'],
+                initialize();
-                            clockOffsets
-                        );
             });
 
-                    it("sets clock offsets", function () {
+            it("updates on bounds change", function () {
-                        expect(time.clockOffsets)
+                time.bounds(boundsB);
-                            .toHaveBeenCalledWith(clockOffsets);
+                expect(search).toEqual({
-                    });
+                    'tc.mode': 'fixed',
-
+                    'tc.startBound': '120',
-                    it("sets the time system without bounds", function () {
+                    'tc.endBound': '360',
-                        expect(time.timeSystem).toHaveBeenCalledWith(
+                    'tc.timeSystem': 'timeSystemA'
-                            search['tc.timeSystem']
-                        );
-                    });
-                }
                 });
             });
 
+            it("updates on timeSystem change", function () {
+                time.timeSystem(timeSystemB, boundsA);
+                expect(search).toEqual({
+                    'tc.mode': 'fixed',
+                    'tc.startBound': '10',
+                    'tc.endBound': '20',
+                    'tc.timeSystem': 'timeSystemB'
+                });
+                time.timeSystem(timeSystemA, boundsB);
+                expect(search).toEqual({
+                    'tc.mode': 'fixed',
+                    'tc.startBound': '120',
+                    'tc.endBound': '360',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+            });
+
+            it("Updates to clock", function () {
+                time.clock(clockA, offsetsA);
+                expect(search).toEqual({
+                    'tc.mode': 'clockA',
+                    'tc.startDelta': '100',
+                    'tc.endDelta': '0',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+            });
+        });
+
+        describe("location updates from time API in fixed", function () {
+            beforeEach(function () {
+                time.clock(clockA.key, offsetsA);
+                time.timeSystem(timeSystemA.key);
+                initialize();
+            });
+
+            it("updates offsets", function () {
+                time.clockOffsets(offsetsB);
+                expect(search).toEqual({
+                    'tc.mode': 'clockA',
+                    'tc.startDelta': '50',
+                    'tc.endDelta': '50',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+            });
+
+            it("updates clocks", function () {
+                time.clock(clockB, offsetsA);
+                expect(search).toEqual({
+                    'tc.mode': 'clockB',
+                    'tc.startDelta': '100',
+                    'tc.endDelta': '0',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+                time.clock(clockA, offsetsB);
+                expect(search).toEqual({
+                    'tc.mode': 'clockA',
+                    'tc.startDelta': '50',
+                    'tc.endDelta': '50',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+            });
+
+            it("updates timesystems", function () {
+                time.timeSystem(timeSystemB);
+                expect(search).toEqual({
+                    'tc.mode': 'clockA',
+                    'tc.startDelta': '100',
+                    'tc.endDelta': '0',
+                    'tc.timeSystem': 'timeSystemB'
+                });
+            });
+
+            it("stops the clock", function () {
+                time.stopClock();
+                expect(search).toEqual({
+                    'tc.mode': 'fixed',
+                    'tc.startBound': '900',
+                    'tc.endBound': '1000',
+                    'tc.timeSystem': 'timeSystemA'
+                });
+            });
+        });
     });
 });