[Table] #972 Further refactoring, tests, style fixes

platform/features/table/src/controllers/HistoricalTableController.js

@@ -40,11 +40,11 @@ define(
             var self = this;
 
             this.$timeout = $timeout;
-            this.timeouts = [];
+            this.timeoutHandle = undefined;
             this.batchSize = BATCH_SIZE;
 
             $scope.$on("$destroy", function () {
-                self.cancelAllTimeouts();
+                clearTimeout(self.timeoutHandle);
             });
 
             TableController.call(this, $scope, telemetryHandler, telemetryFormatter);
@@ -52,28 +52,6 @@ define(
 
         HistoricalTableController.prototype = Object.create(TableController.prototype);
 
-        /**
-         * Cancels outstanding processing
-         * @private
-         */
-        HistoricalTableController.prototype.cancelAllTimeouts = function() {
-            this.timeouts.forEach(function (timeout) {
-              clearTimeout(timeout);
-            });
-            this.timeouts = [];
-        };
-
-        /**
-         * Will yield execution of a long running process, allowing
-         * execution of UI and other activities
-         * @private
-         * @param callback the function to execute after yielding
-         * @returns {number}
-         */
-        HistoricalTableController.prototype.yield = function(callback) {
-            return setTimeout(callback, 0);
-        };
-
         /**
         * Populates historical data on scope when it becomes available from
         * the telemetry API
@@ -83,32 +61,39 @@ define(
                 self = this,
                 telemetryObjects = this.handle.getTelemetryObjects();
 
-            this.cancelAllTimeouts();
-
             function processTelemetryObject(offset) {
                 var telemetryObject = telemetryObjects[offset],
                     series = self.handle.getSeries(telemetryObject) || {},
                     pointCount = series.getPointCount ? series.getPointCount() : 0;
 
-                function processBatch(start, end, done) {
+                function processBatch(start, end) {
                     var i;
+                    end = Math.min(pointCount, end);
 
-                    if (start < pointCount) {
-                        for (i = start; i < end; i++) {
-                            rowData.push(self.table.getRowValues(telemetryObject,
-                                self.handle.makeDatum(telemetryObject, series, i)));
-                        }
-                        self.timeouts.push(self.yield(function () {
-                            processBatch(end, end + self.batchSize, done);
-                        }));
+                    clearTimeout(self.timeoutHandle);
+                    delete self.timeoutHandle;
+
+                    //The row offset (ie. batch start point) does not exceed the rows available
+                    for (i = start; i < end; i++) {
+                        rowData.push(self.table.getRowValues(telemetryObject,
+                            self.handle.makeDatum(telemetryObject, series, i)));
+                    }
+                    if (end < pointCount) {
+                        //Yield if necessary
+                        self.timeoutHandle = setTimeout(function () {
+                            processBatch(end, end + self.batchSize);
+                        }, 0);
                     } else {
+                        //All rows for this object have been processed, so check if there are more objects to process
                         offset++;
                         if (offset < telemetryObjects.length) {
+                            //More telemetry object to process
                             processTelemetryObject(offset);
                         } else {
-                            // Apply digest. Digest may not be necessary here, so
-                            // using $timeout instead of $scope.$apply to avoid
-                            // in progress error
+                            // No more objects to process. Apply rows to scope
+                            // Apply digest. Digest may be in progress (if batch small
+                            // enough to not require yield), so using $timeout instead
+                            // of $scope.$apply to avoid in progress error
                             self.$timeout(function () {
                                 self.$scope.loading = false;
                                 self.$scope.rows = rowData;

platform/features/table/src/controllers/TelemetryTableController.js

@@ -83,7 +83,7 @@ define(
          * @private
          */
         TelemetryTableController.prototype.registerChangeListeners = function () {
-            var self=this;
+            var self = this;
             this.unregisterChangeListeners();
 
             // When composition changes, re-subscribe to the various

platform/features/table/test/controllers/HistoricalTableControllerSpec.js

@@ -34,6 +34,7 @@ define(
                 mockDomainObject,
                 mockTable,
                 mockConfiguration,
+                mockAngularTimeout,
                 watches,
                 controller;
 
@@ -63,6 +64,8 @@ define(
                     watches[expression] = callback;
                 });
 
+                mockAngularTimeout = jasmine.createSpy('$timeout');
+
                 mockConfiguration = {
                     'range1': true,
                     'range2': true,
@@ -107,7 +110,7 @@ define(
                 ]);
                 mockTelemetryHandler.handle.andReturn(mockTelemetryHandle);
 
-                controller = new TableController(mockScope, mockTelemetryHandler, mockTelemetryFormatter);
+                controller = new TableController(mockScope, mockTelemetryHandler, mockTelemetryFormatter, mockAngularTimeout);
                 controller.table = mockTable;
                 controller.handle = mockTelemetryHandle;
             });
@@ -163,6 +166,9 @@ define(
 
                 controller.addHistoricalData(mockDomainObject, mockSeries);
 
+                expect(mockAngularTimeout).toHaveBeenCalled();
+                mockAngularTimeout.mostRecentCall.args[0]();
+
                 expect(controller.$scope.rows.length).toBe(5);
                 expect(controller.$scope.rows[0]).toBe(mockRow);
             });
@@ -198,7 +204,7 @@ define(
                     ' object composition changes', function () {
                     controller.registerChangeListeners();
                     expect(watches['domainObject.getModel().composition']).toBeDefined();
-                    watches['domainObject.getModel().composition']();
+                    watches['domainObject.getModel().composition']([], []);
                     expect(controller.subscribe).toHaveBeenCalled();
                 });
 
@@ -219,6 +225,89 @@ define(
                 });
 
             });
+            describe('Yields thread', function () {
+                var mockSeries,
+                    mockRow,
+                    mockWindowTimeout = {};
+
+                beforeEach(function () {
+                    mockSeries = {
+                        getPointCount: function () {
+                            return 5;
+                        },
+                        getDomainValue: function () {
+                            return 'Domain Value';
+                        },
+                        getRangeValue: function () {
+                            return 'Range Value';
+                        }
+                    };
+                    mockRow = {'domain': 'Domain Value', 'range': 'Range Value'};
+
+                    mockTelemetryHandle.makeDatum.andCallFake(function () {
+                        return mockRow;
+                    });
+                    mockTable.getRowValues.andReturn(mockRow);
+                    mockTelemetryHandle.getTelemetryObjects.andReturn([mockDomainObject]);
+                    mockTelemetryHandle.getSeries.andReturn(mockSeries);
+
+                    jasmine.getGlobal().setTimeout = jasmine.createSpy("setTimeout");
+                    jasmine.getGlobal().setTimeout.andReturn(mockWindowTimeout);
+                    jasmine.getGlobal().clearTimeout = jasmine.createSpy("clearTimeout");
+
+                });
+                it('only when necessary', function () {
+
+                    controller.batchSize = 1000;
+                    controller.addHistoricalData(mockDomainObject, mockSeries);
+
+                    expect(mockAngularTimeout).toHaveBeenCalled();
+                    mockAngularTimeout.mostRecentCall.args[0]();
+
+                    expect(controller.$scope.rows.length).toBe(5);
+                    expect(controller.$scope.rows[0]).toBe(mockRow);
+
+                    expect(jasmine.getGlobal().setTimeout).not.toHaveBeenCalled();
+
+                });
+                it('when row count exceeds batch size', function () {
+                    controller.batchSize = 3;
+                    controller.addHistoricalData(mockDomainObject, mockSeries);
+
+                    expect(jasmine.getGlobal().setTimeout).toHaveBeenCalled();
+                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
+
+                    expect(mockAngularTimeout).toHaveBeenCalled();
+                    mockAngularTimeout.mostRecentCall.args[0]();
+
+                    expect(controller.$scope.rows.length).toBe(5);
+                    expect(controller.$scope.rows[0]).toBe(mockRow);
+                });
+                it('cancelling any outstanding timeouts', function () {
+                    controller.batchSize = 3;
+                    controller.addHistoricalData(mockDomainObject, mockSeries);
+
+                    expect(jasmine.getGlobal().setTimeout).toHaveBeenCalled();
+                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
+
+                    controller.addHistoricalData(mockDomainObject, mockSeries);
+
+                    expect(jasmine.getGlobal().clearTimeout).toHaveBeenCalledWith(mockWindowTimeout);
+                });
+                it('cancels timeout on scope destruction', function () {
+                    controller.batchSize = 3;
+                    controller.addHistoricalData(mockDomainObject, mockSeries);
+
+                    expect(jasmine.getGlobal().setTimeout).toHaveBeenCalled();
+                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
+
+                    //Call destroy function
+                    expect(mockScope.$on).toHaveBeenCalledWith("$destroy", jasmine.any(Function));
+                    mockScope.$on.mostRecentCall.args[1]();
+                    expect(jasmine.getGlobal().clearTimeout).toHaveBeenCalledWith(mockWindowTimeout);
+
+                });
+            });
         });
     }
 );