[Tables] #972 Refactored to simplify code. Fixes #972

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

@@ -44,7 +44,9 @@ define(
             this.batchSize = BATCH_SIZE;
 
             $scope.$on("$destroy", function () {
-                clearTimeout(self.timeoutHandle);
+                if (self.timeoutHandle) {
+                    self.$timeout.cancel(self.timeoutHandle);
+                }
             });
 
             TableController.call(this, $scope, telemetryHandler, telemetryFormatter);
@@ -52,63 +54,64 @@ define(
 
         HistoricalTableController.prototype = Object.create(TableController.prototype);
 
+        /**
+         * Set provided row data on scope, and cancel loading spinner
+         * @private
+         */
+        HistoricalTableController.prototype.doneProcessing = function (rowData) {
+            this.$scope.rows = rowData;
+            this.$scope.loading = false;
+        };
+
+        /**
+         * Processes an array of objects, formatting the telemetry available
+         * for them and setting it on scope when done
+         * @private
+         */
+        HistoricalTableController.prototype.processTelemetryObjects = function (objects, offset, start, rowData) {
+            var telemetryObject = objects[offset],
+                series,
+                i = start,
+                pointCount,
+                end;
+
+            //No more objects to process
+            if (!telemetryObject) {
+                return this.doneProcessing(rowData);
+            }
+
+            series = this.handle.getSeries(telemetryObject);
+
+            pointCount = series.getPointCount();
+            end = Math.min(start + this.batchSize, pointCount);
+
+            //Process rows in a batch with size not exceeding a maximum length
+            for (; i < end; i++) {
+                rowData.push(this.table.getRowValues(telemetryObject,
+                    this.handle.makeDatum(telemetryObject, series, i)));
+            }
+
+            //Done processing all rows for this object.
+            if (end >= pointCount) {
+                offset++;
+                end = 0;
+            }
+
+            // Done processing either a batch or an object, yield process
+            // before continuing processing
+            this.timeoutHandle = this.$timeout(this.processTelemetryObjects.bind(this, objects, offset, end, rowData));
+        };
+
         /**
         * Populates historical data on scope when it becomes available from
         * the telemetry API
         */
         HistoricalTableController.prototype.addHistoricalData = function () {
-            var rowData = [],
+            if (this.timeoutHandle) {
-                self = this,
+                this.$timeout.cancel(this.timeoutHandle);
-                telemetryObjects = this.handle.getTelemetryObjects();
-
-            function processTelemetryObject(offset) {
-                var telemetryObject = telemetryObjects[offset],
-                    series = self.handle.getSeries(telemetryObject) || {},
-                    pointCount = series.getPointCount ? series.getPointCount() : 0;
-
-                function processBatch(start, end) {
-                    var i;
-                    end = Math.min(pointCount, end);
-
-                    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 {
-                            // 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;
-                            });
-                        }
-                    }
-                }
-                processBatch(0, self.batchSize);
-            }
-
-            if (telemetryObjects.length > 0) {
-                this.$scope.loading = true;
-                processTelemetryObject(0);
             }
 
+            this.timeoutHandle = this.$timeout(this.processTelemetryObjects.bind(this, this.handle.getTelemetryObjects(), 0, 0, []));
         };
 
         return HistoricalTableController;

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

@@ -91,6 +91,7 @@ define(
                         self.$scope.rows.length - 1);
                 }
             });
+            this.$scope.loading = false;
         };
 
         return RealtimeTableController;

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

@@ -140,6 +140,7 @@ define(
             if (this.handle) {
                 this.handle.unsubscribe();
             }
+            this.$scope.loading = true;
 
             this.handle = this.$scope.domainObject && this.telemetryHandler.handle(
                     this.$scope.domainObject,

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

@@ -35,6 +35,7 @@ define(
                 mockTable,
                 mockConfiguration,
                 mockAngularTimeout,
+                mockTimeoutHandle,
                 watches,
                 controller;
 
@@ -64,7 +65,10 @@ define(
                     watches[expression] = callback;
                 });
 
-                mockAngularTimeout = jasmine.createSpy('$timeout');
+                mockTimeoutHandle = jasmine.createSpy("timeoutHandle");
+                mockAngularTimeout = jasmine.createSpy("$timeout");
+                mockAngularTimeout.andReturn(mockTimeoutHandle);
+                mockAngularTimeout.cancel = jasmine.createSpy("cancelTimeout");
 
                 mockConfiguration = {
                     'range1': true,
@@ -166,8 +170,12 @@ define(
 
                 controller.addHistoricalData(mockDomainObject, mockSeries);
 
+                // Angular timeout is called a minumum of twice, regardless
+                // of batch size used.
                 expect(mockAngularTimeout).toHaveBeenCalled();
                 mockAngularTimeout.mostRecentCall.args[0]();
+                expect(mockAngularTimeout.calls.length).toEqual(2);
+                mockAngularTimeout.mostRecentCall.args[0]();
 
                 expect(controller.$scope.rows.length).toBe(5);
                 expect(controller.$scope.rows[0]).toBe(mockRow);
@@ -227,8 +235,7 @@ define(
             });
             describe('Yields thread', function () {
                 var mockSeries,
-                    mockRow,
+                    mockRow;
-                    mockWindowTimeout = {};
 
                 beforeEach(function () {
                     mockSeries = {
@@ -250,36 +257,23 @@ define(
                     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();
+                    //Timeout is called a minimum of two times
-                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
-
                     expect(mockAngularTimeout).toHaveBeenCalled();
                     mockAngularTimeout.mostRecentCall.args[0]();
 
+                    expect(mockAngularTimeout.calls.length).toEqual(2);
+                    mockAngularTimeout.mostRecentCall.args[0]();
+
+                    //Because it yields, timeout will have been called a
+                    // third time for the batch.
+                    expect(mockAngularTimeout.calls.length).toEqual(3);
+                    mockAngularTimeout.mostRecentCall.args[0]();
+
                     expect(controller.$scope.rows.length).toBe(5);
                     expect(controller.$scope.rows[0]).toBe(mockRow);
                 });
@@ -287,24 +281,27 @@ define(
                     controller.batchSize = 3;
                     controller.addHistoricalData(mockDomainObject, mockSeries);
 
-                    expect(jasmine.getGlobal().setTimeout).toHaveBeenCalled();
+                    expect(mockAngularTimeout).toHaveBeenCalled();
-                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
+                    mockAngularTimeout.mostRecentCall.args[0]();
 
                     controller.addHistoricalData(mockDomainObject, mockSeries);
 
-                    expect(jasmine.getGlobal().clearTimeout).toHaveBeenCalledWith(mockWindowTimeout);
+                    expect(mockAngularTimeout.cancel).toHaveBeenCalledWith(mockTimeoutHandle);
                 });
                 it('cancels timeout on scope destruction', function () {
                     controller.batchSize = 3;
                     controller.addHistoricalData(mockDomainObject, mockSeries);
 
-                    expect(jasmine.getGlobal().setTimeout).toHaveBeenCalled();
+                    //Destroy is used by parent class as well, so multiple
-                    jasmine.getGlobal().setTimeout.mostRecentCall.args[0]();
+                    // calls are made to scope.$on
-
+                    var destroyCalls = mockScope.$on.calls.filter(function (call) {
+                        return call.args[0] === '$destroy';
+                    });
                     //Call destroy function
-                    expect(mockScope.$on).toHaveBeenCalledWith("$destroy", jasmine.any(Function));
+                    expect(destroyCalls.length).toEqual(2);
-                    mockScope.$on.mostRecentCall.args[1]();
+
-                    expect(jasmine.getGlobal().clearTimeout).toHaveBeenCalledWith(mockWindowTimeout);
+                    destroyCalls[0].args[1]();
+                    expect(mockAngularTimeout.cancel).toHaveBeenCalledWith(mockTimeoutHandle);
 
                 });
             });