[Tables] Maintain correct insertion order with duplicates. Fixes #1465

index.html

@@ -25,16 +25,8 @@
     <meta charset="utf-8">
     <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">
     <title></title>
-    <script src="openmct-tutorial/lib/http.js">
-    </script>
     <script src="bower_components/requirejs/require.js">
     </script>
-    <script src="openmct-tutorial/dictionary-plugin.js">
-    </script>
-    <script src="openmct-tutorial/realtime-telemetry-plugin.js">
-    </script>
-    <script src="openmct-tutorial/historical-telemetry-plugin.js">
-    </script>
     <script>
         require(['openmct'], function (openmct) {
             [
@@ -48,9 +40,6 @@
             openmct.install(openmct.plugins.Espresso());
             openmct.install(openmct.plugins.Generator());
             openmct.install(openmct.plugins.UTCTimeSystem());
-            openmct.install(DictionaryPlugin());
-            openmct.install(RealtimeTelemetryPlugin());
-            openmct.install(HistoricalTelemetryPlugin());
             openmct.start();
         });
     </script>

platform/features/conductor/compatibility/bundle.js

@@ -21,9 +21,11 @@
  *****************************************************************************/
 
 define([
+    "./src/ConductorTelemetryDecorator",
     "./src/ConductorRepresenter",
     'legacyRegistry'
 ], function (
+    ConductorTelemetryDecorator,
     ConductorRepresenter,
     legacyRegistry
 ) {
@@ -37,6 +39,16 @@ define([
                         "openmct"
                     ]
                 }
+            ],
+            "components": [
+                {
+                    "type": "decorator",
+                    "provides": "telemetryService",
+                    "implementation": ConductorTelemetryDecorator,
+                    "depends": [
+                        "openmct"
+                    ]
+                }
             ]
         }
     });

platform/features/conductor/compatibility/src/ConductorTelemetryDecorator.js

@@ -0,0 +1,87 @@
+/*****************************************************************************
+ * Open MCT Web, Copyright (c) 2014-2015, United States Government
+ * as represented by the Administrator of the National Aeronautics and Space
+ * Administration. All rights reserved.
+ *
+ * Open MCT Web is 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.
+ *
+ * Open MCT Web includes source code licensed under additional open source
+ * licenses. See the Open Source Licenses file (LICENSES.md) included with
+ * this source code distribution or the Licensing information page available
+ * at runtime from the About dialog for additional information.
+ *****************************************************************************/
+
+define(
+    function () {
+
+        /**
+         * Decorates the `telemetryService` such that requests are
+         * mediated by the time conductor. This is a modified version of the
+         * decorator used in the old TimeConductor that integrates with the
+         * new TimeConductor API.
+         *
+         * @constructor
+         * @memberof platform/features/conductor
+         * @implements {TelemetryService}
+         * @param {platform/features/conductor.TimeConductor} conductor
+         *        the service which exposes the global time conductor
+         * @param {TelemetryService} telemetryService the decorated service
+         */
+        function ConductorTelemetryDecorator(openmct, telemetryService) {
+            this.conductor = openmct.conductor;
+            this.telemetryService = telemetryService;
+
+            this.amendRequests = ConductorTelemetryDecorator.prototype.amendRequests.bind(this);
+        }
+
+        function amendRequest(request, bounds, timeSystem) {
+            request = request || {};
+            request.start = bounds.start;
+            request.end = bounds.end;
+            request.domain = timeSystem.metadata.key;
+
+            return request;
+        }
+
+        ConductorTelemetryDecorator.prototype.amendRequests = function (requests) {
+            var bounds = this.conductor.bounds(),
+                timeSystem = this.conductor.timeSystem();
+
+            return (requests || []).map(function (request) {
+                return amendRequest(request, bounds, timeSystem);
+            });
+        };
+
+        ConductorTelemetryDecorator.prototype.requestTelemetry = function (requests) {
+            return this.telemetryService
+                .requestTelemetry(this.amendRequests(requests));
+        };
+
+        ConductorTelemetryDecorator.prototype.subscribe = function (callback, requests) {
+            var unsubscribeFunc = this.telemetryService.subscribe(callback, this.amendRequests(requests)),
+                conductor = this.conductor,
+                self = this;
+
+            function amendRequests() {
+                return self.amendRequests(requests);
+            }
+
+            conductor.on('bounds', amendRequests);
+            return function () {
+                unsubscribeFunc();
+                conductor.off('bounds', amendRequests);
+            };
+        };
+
+        return ConductorTelemetryDecorator;
+    }
+);

platform/features/table/src/TelemetryCollection.js

@@ -112,22 +112,6 @@ define(
             this.lastBounds = bounds;
         };
 
-        /**
-         * Determines is a given telemetry datum is within the bounds currently
-         * defined for this telemetry collection.
-         * @private
-         * @param datum
-         * @returns {boolean}
-         */
-        TelemetryCollection.prototype.inBounds = function (datum) {
-            var noBoundsDefined = !this.lastBounds || (this.lastBounds.start === undefined && this.lastBounds.end === undefined);
-            var withinBounds =
-                _.get(datum, this.sortField) >= this.lastBounds.start &&
-                _.get(datum, this.sortField) <= this.lastBounds.end;
-
-            return noBoundsDefined || withinBounds;
-        };
-
         /**
          * Adds an individual item to the collection. Used internally only
          * @private
@@ -173,9 +157,10 @@ define(
                 // based on time stamp because the array is guaranteed ordered due
                 // to sorted insertion.
                 var startIx = _.sortedIndex(array, item, this.sortField);
+                var endIx;
 
                 if (startIx !== array.length) {
-                    var endIx = _.sortedLastIndex(array, item, this.sortField);
+                    endIx = _.sortedLastIndex(array, item, this.sortField);
 
                     // Create an array of potential dupes, based on having the
                     // same time stamp
@@ -185,7 +170,7 @@ define(
                 }
 
                 if (!isDuplicate) {
-                    array.splice(startIx, 0, item);
+                    array.splice(endIx || startIx, 0, item);
 
                     //Return true if it was added and in bounds
                     return array === this.telemetry;

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

@@ -425,6 +425,38 @@ define(
             }
         };
 
+        /**
+         * Finds the correct insertion point for a new row, which takes into
+         * account duplicates to make sure new rows are inserted in a way that
+         * maintains arrival order.
+         *
+         * @private
+         * @param {Array} searchArray
+         * @param {Object} searchElement Object to find the insertion point for
+         */
+        MCTTableController.prototype.findInsertionPoint = function (searchArray, searchElement) {
+            //First, use a binary search to find the correct insertion point
+            var index = this.binarySearch(searchArray, searchElement, 0, searchArray.length - 1);
+            var testIndex = index;
+
+            //It's possible that the insertion point is a duplicate of the element to be inserted
+            var isDupe = function () {
+                return this.sortComparator(searchElement,
+                        searchArray[testIndex][this.$scope.sortColumn].text) === 0;
+            }.bind(this);
+
+            // In the event of a duplicate, scan left or right (depending on
+            // sort order) to find an insertion point that maintains order received
+            while (testIndex >= 0 && testIndex < searchArray.length && isDupe()) {
+                if (this.$scope.sortDirection === 'asc') {
+                    index = ++testIndex;
+                } else {
+                    index = testIndex--;
+                }
+            }
+            return index;
+        };
+
         /**
          * @private
          */
@@ -439,9 +471,9 @@ define(
                 case -1:
                     return this.binarySearch(searchArray, searchElement, min,
                         sampleAt - 1);
-                case 0 :
+                case 0:
                     return sampleAt;
-                case 1 :
+                case 1:
                     return this.binarySearch(searchArray, searchElement,
                         sampleAt + 1, max);
             }
@@ -458,7 +490,7 @@ define(
                 index = array.length;
             } else {
                 //Sort is enabled, perform binary search to find insertion point
-                index = this.binarySearch(array, element[this.$scope.sortColumn].text, 0, array.length - 1);
+                index = this.findInsertionPoint(array, element[this.$scope.sortColumn].text);
             }
             if (index === -1) {
                 array.unshift(element);

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

@@ -364,11 +364,8 @@ define(
             var telemetryApi = this.openmct.telemetry;
             var telemetryCollection = this.telemetry;
             //Set table max length to avoid unbounded growth.
-            //var maxRows = 100000;
-            var maxRows = Number.MAX_VALUE;
             var limitEvaluator;
             var added = false;
-            var scope = this.$scope;
             var table = this.table;
 
             this.subscriptions.forEach(function (subscription) {
@@ -379,16 +376,6 @@ define(
             function newData(domainObject, datum) {
                 limitEvaluator = telemetryApi.limitEvaluator(domainObject);
                 added = telemetryCollection.add([table.getRowValues(limitEvaluator, datum)]);
-
-                //Inform table that a new row has been added
-                if (scope.rows.length > maxRows) {
-                    scope.$broadcast('remove:rows', scope.rows[0]);
-                    scope.rows.shift();
-                }
-                if (!scope.loading && added) {
-                    scope.$broadcast('add:row',
-                        scope.rows.length - 1);
-                }
             }
 
             objects.forEach(function (object) {

platform/features/table/test/TelemetryCollectionSpec.js

@@ -138,6 +138,27 @@ define(
                         };
                         collection.add([addedObjectB, addedObjectA]);
 
+                        expect(collection.telemetry[11]).toBe(addedObjectB);
+                    }
+                );
+                it("maintains insertion order in the case of duplicate time stamps",
+                    function () {
+                        var addedObjectA = {
+                            timestamp: 10000,
+                            value: {
+                                integer: 10,
+                                text: integerTextMap[10]
+                            }
+                        };
+                        var addedObjectB = {
+                            timestamp: 10000,
+                            value: {
+                                integer: 11,
+                                text: integerTextMap[11]
+                            }
+                        };
+                        collection.add([addedObjectA, addedObjectB]);
+
                         expect(collection.telemetry[11]).toBe(addedObjectB);
                     }
                 );

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

@@ -459,14 +459,14 @@ define(
 
                         beforeEach(function () {
                             row4 = {
-                                'col1': {'text': 'row5 col1'},
+                                'col1': {'text': 'row4 col1'},
                                 'col2': {'text': 'xyz'},
-                                'col3': {'text': 'row5 col3'}
+                                'col3': {'text': 'row4 col3'}
                             };
                             row5 = {
-                                'col1': {'text': 'row6 col1'},
+                                'col1': {'text': 'row5 col1'},
                                 'col2': {'text': 'aaa'},
-                                'col3': {'text': 'row6 col3'}
+                                'col3': {'text': 'row5 col3'}
                             };
                             row6 = {
                                 'col1': {'text': 'row6 col1'},
@@ -490,6 +490,71 @@ define(
                             expect(mockScope.displayRows[3].col2.text).toEqual('ggg');
                         });
 
+                        it('Inserts duplicate values for sort column in order received when sorted descending', function () {
+                            mockScope.sortColumn = 'col2';
+                            mockScope.sortDirection = 'desc';
+
+                            mockScope.displayRows = controller.sortRows(testRows.slice(0));
+
+                            var row6b = {
+                                'col1': {'text': 'row6b col1'},
+                                'col2': {'text': 'ggg'},
+                                'col3': {'text': 'row6b col3'}
+                            };
+                            var row6c = {
+                                'col1': {'text': 'row6c col1'},
+                                'col2': {'text': 'ggg'},
+                                'col3': {'text': 'row6c col3'}
+                            };
+
+                            controller.addRows(undefined, [row4, row5]);
+                            controller.addRows(undefined, [row6, row6b, row6c]);
+                            expect(mockScope.displayRows[0].col2.text).toEqual('xyz');
+                            expect(mockScope.displayRows[7].col2.text).toEqual('aaa');
+
+                            // Added duplicate rows
+                            expect(mockScope.displayRows[2].col2.text).toEqual('ggg');
+                            expect(mockScope.displayRows[3].col2.text).toEqual('ggg');
+                            expect(mockScope.displayRows[4].col2.text).toEqual('ggg');
+
+                            // Check that original order is maintained with dupes
+                            expect(mockScope.displayRows[2].col3.text).toEqual('row6c col3');
+                            expect(mockScope.displayRows[3].col3.text).toEqual('row6b col3');
+                            expect(mockScope.displayRows[4].col3.text).toEqual('row6 col3');
+                        });
+
+                        it('Inserts duplicate values for sort column in order received when sorted ascending', function () {
+                            mockScope.sortColumn = 'col2';
+                            mockScope.sortDirection = 'asc';
+
+                            mockScope.displayRows = controller.sortRows(testRows.slice(0));
+
+                            var row6b = {
+                                'col1': {'text': 'row6b col1'},
+                                'col2': {'text': 'ggg'},
+                                'col3': {'text': 'row6b col3'}
+                            };
+                            var row6c = {
+                                'col1': {'text': 'row6c col1'},
+                                'col2': {'text': 'ggg'},
+                                'col3': {'text': 'row6c col3'}
+                            };
+
+                            controller.addRows(undefined, [row4, row5, row6]);
+                            controller.addRows(undefined, [row6b, row6c]);
+                            expect(mockScope.displayRows[0].col2.text).toEqual('aaa');
+                            expect(mockScope.displayRows[7].col2.text).toEqual('xyz');
+
+                            // Added duplicate rows
+                            expect(mockScope.displayRows[3].col2.text).toEqual('ggg');
+                            expect(mockScope.displayRows[4].col2.text).toEqual('ggg');
+                            expect(mockScope.displayRows[5].col2.text).toEqual('ggg');
+                            // Check that original order is maintained with dupes
+                            expect(mockScope.displayRows[3].col3.text).toEqual('row6 col3');
+                            expect(mockScope.displayRows[4].col3.text).toEqual('row6b col3');
+                            expect(mockScope.displayRows[5].col3.text).toEqual('row6c col3');
+                        });
+
                         it('Adds new rows at the correct sort position when' +
                             ' sorted and filtered', function () {
                             mockScope.sortColumn = 'col2';

platform/telemetry/src/TelemetryCapability.js

@@ -139,8 +139,7 @@ define(
                 type = domainObject.getCapability("type"),
                 typeRequest = (type && type.getDefinition().telemetry) || {},
                 modelTelemetry = domainObject.getModel().telemetry,
-                fullRequest = Object.create(typeRequest),
-                bounds;
+                fullRequest = Object.create(typeRequest);
 
             // Add properties from the telemetry field of this
             // specific domain object.
@@ -161,12 +160,6 @@ define(
                 fullRequest.key = domainObject.getId();
             }
 
-            if (request.start === undefined && request.end === undefined) {
-                bounds = this.openmct.conductor.bounds();
-                fullRequest.start = bounds.start;
-                fullRequest.end = bounds.end;
-            }
-
             return fullRequest;
         };
 

platform/telemetry/test/TelemetryCapabilitySpec.js

@@ -97,15 +97,7 @@ define(
                 });
 
                 mockAPI = {
-                    telemetry: mockTelemetryAPI,
-                    conductor: {
-                        bounds: function () {
-                            return {
-                                start: 0,
-                                end: 1
-                            };
-                        }
-                    }
+                    telemetry: mockTelemetryAPI
                 };
 
                 telemetry = new TelemetryCapability(
@@ -158,9 +150,7 @@ define(
                 expect(telemetry.getMetadata()).toEqual({
                     id: "testId", // from domain object
                     source: "testSource",
-                    key: "testKey",
-                    start: 0,
-                    end: 1
+                    key: "testKey"
                 });
             });
 
@@ -174,9 +164,7 @@ define(
                 expect(telemetry.getMetadata()).toEqual({
                     id: "testId", // from domain object
                     source: "testSource", // from model
-                    key: "testId", // from domain object
-                    start: 0,
-                    end: 1
+                    key: "testId" // from domain object
                 });
             });
 
@@ -255,9 +243,7 @@ define(
                     [{
                         id: "testId", // from domain object
                         source: "testSource",
-                        key: "testKey",
-                        start: 0,
-                        end: 1
+                        key: "testKey"
                     }]
                 );