Merge pull request #345 from nasa/open32open169

[Layout] Layout rebuilds after resize/reposition #32

3 files changed, 139 insertions, 56 deletions

diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json
index df5bd9413..3f710d437 100644
--- a/platform/features/layout/bundle.json
+++ b/platform/features/layout/bundle.json
@@ -9,7 +9,7 @@
                 "glyph": "L",
                 "type": "layout",
                 "templateUrl": "templates/layout.html",
-                "uses": [ "composition" ],
+                "uses": [],
                 "gestures": [ "drop" ]
             },
             {
diff --git a/platform/features/layout/src/LayoutController.js b/platform/features/layout/src/LayoutController.js
index d7906dd0f..017793d2e 100644
--- a/platform/features/layout/src/LayoutController.js
+++ b/platform/features/layout/src/LayoutController.js
@@ -45,43 +45,8 @@ define(
          * @param {Scope} $scope the controller's Angular scope
          */
         function LayoutController($scope) {
-            var self = this;
-
-            // Utility function to copy raw positions from configuration,
-            // without writing directly to configuration (to avoid triggering
-            // persistence from watchers during drags).
-            function shallowCopy(obj, keys) {
-                var copy = {};
-                keys.forEach(function (k) {
-                    copy[k] = obj[k];
-                });
-                return copy;
-            }
-
-            // Compute panel positions based on the layout's object model
-            function lookupPanels(ids) {
-                var configuration = $scope.configuration || {};
-
-                // ids is read from model.composition and may be undefined;
-                // fall back to an array if that occurs
-                ids = ids || [];
-
-                // Pull panel positions from configuration
-                self.rawPositions =
-                    shallowCopy(configuration.panels || {}, ids);
-
-                // Clear prior computed positions
-                self.positions = {};
-
-                // Update width/height that we are tracking
-                self.gridSize =
-                    ($scope.model || {}).layoutGrid || DEFAULT_GRID_SIZE;
-
-                // Compute positions and add defaults where needed
-                ids.forEach(function (id, index) {
-                    self.populatePosition(id, index);
-                });
-            }
+            var self = this,
+                callbackCount = 0;
 
             // Update grid size when it changed
             function updateGridSize(layoutGrid) {
@@ -92,7 +57,7 @@ define(
                 // Only update panel positions if this actually changed things
                 if (self.gridSize[0] !== oldSize[0] ||
                         self.gridSize[1] !== oldSize[1]) {
-                    lookupPanels(Object.keys(self.positions));
+                    self.layoutPanels(Object.keys(self.positions));
                 }
             }
 
@@ -127,6 +92,28 @@ define(
                 e.preventDefault();
             }
 
+            //Will fetch fully contextualized composed objects, and populate
+            // scope with them.
+            function refreshComposition() {
+                //Keep a track of how many composition callbacks have been made
+                var thisCount = ++callbackCount;
+
+                $scope.domainObject.useCapability('composition').then(function(composition){
+                    var ids;
+
+                    //Is this callback for the most recent composition
+                    // request? If not, discard it. Prevents race condition
+                    if (thisCount === callbackCount){
+                        ids = composition.map(function (object) {
+                                return object.getId();
+                            }) || [];
+
+                        $scope.composition = composition;
+                        self.layoutPanels(ids);
+                    }
+                });
+            }
+
             // End drag; we don't want to put $scope into this
             // because it triggers "cpws" (copy window or scope)
             // errors in Angular.
@@ -156,8 +143,8 @@ define(
             // Watch for changes to the grid size in the model
             $scope.$watch("model.layoutGrid", updateGridSize);
 
-            // Position panes when the model field changes
-            $scope.$watch("model.composition", lookupPanels);
+            // Update composed objects on screen, and position panes
+            $scope.$watchCollection("model.composition", refreshComposition);
 
             // Position panes where they are dropped
             $scope.$on("mctDrop", handleDrop);
@@ -263,6 +250,43 @@ define(
             }
         };
 
+        // Utility function to copy raw positions from configuration,
+        // without writing directly to configuration (to avoid triggering
+        // persistence from watchers during drags).
+        function shallowCopy(obj, keys) {
+            var copy = {};
+            keys.forEach(function (k) {
+                copy[k] = obj[k];
+            });
+            return copy;
+        }
+
+        /**
+         * Compute panel positions based on the layout's object model.
+         * Defined as member function to facilitate testing.
+         * @private
+         */
+        LayoutController.prototype.layoutPanels = function (ids) {
+            var configuration = this.$scope.configuration || {},
+                self = this;
+
+            // Pull panel positions from configuration
+            this.rawPositions =
+                shallowCopy(configuration.panels || {}, ids);
+
+            // Clear prior computed positions
+            this.positions = {};
+
+            // Update width/height that we are tracking
+            this.gridSize =
+                (this.$scope.model || {}).layoutGrid || DEFAULT_GRID_SIZE;
+
+            // Compute positions and add defaults where needed
+            ids.forEach(function (id, index) {
+                self.populatePosition(id, index);
+            });
+        };
+
         /**
          * End the active drag gesture. This will update the
          * view configuration.
diff --git a/platform/features/layout/test/LayoutControllerSpec.js b/platform/features/layout/test/LayoutControllerSpec.js
index 085f7766a..bbed271d2 100644
--- a/platform/features/layout/test/LayoutControllerSpec.js
+++ b/platform/features/layout/test/LayoutControllerSpec.js
@@ -19,7 +19,7 @@
  * this source code distribution or the Licensing information page available
  * at runtime from the About dialog for additional information.
  *****************************************************************************/
-/*global define,describe,it,expect,beforeEach,jasmine*/
+/*global define,describe,it,expect,beforeEach,jasmine,spyOn*/
 
 define(
     ["../src/LayoutController"],
@@ -31,21 +31,44 @@ define(
                 mockEvent,
                 testModel,
                 testConfiguration,
-                controller;
+                controller,
+                mockCompositionCapability,
+                mockComposition,
+                mockCompositionObjects;
+
+            function mockPromise(value){
+                return {
+                    then: function (thenFunc) {
+                        return mockPromise(thenFunc(value));
+                    }
+                };
+            }
+
+            function mockDomainObject(id){
+                return {
+                    getId: function() {
+                        return id;
+                    },
+                    useCapability: function() {
+                        return mockCompositionCapability;
+                    }
+                };
+            }
 
             beforeEach(function () {
                 mockScope = jasmine.createSpyObj(
                     "$scope",
-                    [ "$watch", "$on", "commit" ]
+                    [ "$watch", "$watchCollection", "$on", "commit" ]
                 );
                 mockEvent = jasmine.createSpyObj(
                     'event',
                     [ 'preventDefault' ]
                 );
 
-                testModel = {
-                    composition: [ "a", "b", "c" ]
-                };
+                testModel = {};
+
+                mockComposition = ["a", "b", "c"];
+                mockCompositionObjects = mockComposition.map(mockDomainObject);
 
                 testConfiguration = {
                     panels: {
@@ -56,23 +79,62 @@ define(
                     }
                 };
 
+                mockCompositionCapability = mockPromise(mockCompositionObjects);
+
+                mockScope.domainObject = mockDomainObject("mockDomainObject");
                 mockScope.model = testModel;
                 mockScope.configuration = testConfiguration;
+                spyOn(mockScope.domainObject, "useCapability").andCallThrough();
 
                 controller = new LayoutController(mockScope);
+                spyOn(controller, "layoutPanels").andCallThrough();
             });
 
             // Model changes will indicate that panel positions
             // may have changed, for instance.
             it("watches for changes to composition", function () {
-                expect(mockScope.$watch).toHaveBeenCalledWith(
+                expect(mockScope.$watchCollection).toHaveBeenCalledWith(
                     "model.composition",
                     jasmine.any(Function)
                 );
             });
 
+            it("Retrieves updated composition from composition capability", function () {
+                mockScope.$watchCollection.mostRecentCall.args[1]();
+                expect(mockScope.domainObject.useCapability).toHaveBeenCalledWith(
+                    "composition"
+                );
+                expect(controller.layoutPanels).toHaveBeenCalledWith(
+                    mockComposition
+                );
+            });
+
+            it("Is robust to concurrent changes to composition", function () {
+                var secondMockComposition = ["a", "b", "c", "d"],
+                    secondMockCompositionObjects = secondMockComposition.map(mockDomainObject),
+                    firstCompositionCB,
+                    secondCompositionCB;
+
+                spyOn(mockCompositionCapability, "then");
+                mockScope.$watchCollection.mostRecentCall.args[1]();
+                mockScope.$watchCollection.mostRecentCall.args[1]();
+
+                firstCompositionCB = mockCompositionCapability.then.calls[0].args[0];
+                secondCompositionCB = mockCompositionCapability.then.calls[1].args[0];
+
+                //Resolve promises in reverse order
+                secondCompositionCB(secondMockCompositionObjects);
+                firstCompositionCB(mockCompositionObjects);
+
+                //Expect the promise call that was initiated most recently to
+                // be the one used to populate scope, irrespective of order that
+                // it was eventually resolved
+                expect(mockScope.composition).toBe(secondMockCompositionObjects);
+            });
+
+
             it("provides styles for frames, from configuration", function () {
-                mockScope.$watch.mostRecentCall.args[1](testModel.composition);
+                mockScope.$watchCollection.mostRecentCall.args[1]();
                 expect(controller.getFrameStyle("a")).toEqual({
                     top: "320px",
                     left: "640px",
@@ -85,7 +147,7 @@ define(
                 var styleB, styleC;
 
                 // b and c do not have configured positions
-                mockScope.$watch.mostRecentCall.args[1](testModel.composition);
+                mockScope.$watchCollection.mostRecentCall.args[1]();
 
                 styleB = controller.getFrameStyle("b");
                 styleC = controller.getFrameStyle("c");
@@ -102,7 +164,7 @@ define(
 
             it("allows panels to be dragged", function () {
                 // Populate scope
-                mockScope.$watch.mostRecentCall.args[1](testModel.composition);
+                mockScope.$watchCollection.mostRecentCall.args[1]();
 
                 // Verify precondtion
                 expect(testConfiguration.panels.b).not.toBeDefined();
@@ -121,7 +183,7 @@ define(
 
             it("invokes commit after drag", function () {
                 // Populate scope
-                mockScope.$watch.mostRecentCall.args[1](testModel.composition);
+                mockScope.$watchCollection.mostRecentCall.args[1]();
 
                 // Do a drag
                 controller.startDrag("b", [1, 1], [0, 0]);
@@ -147,7 +209,6 @@ define(
                 expect(testConfiguration.panels.d).not.toBeDefined();
 
                 // Notify that a drop occurred
-                testModel.composition.push('d');
                 mockScope.$on.mostRecentCall.args[1](
                     mockEvent,
                     'd',
@@ -167,7 +228,6 @@ define(
                 mockEvent.defaultPrevented = true;
 
                 // Notify that a drop occurred
-                testModel.composition.push('d');
                 mockScope.$on.mostRecentCall.args[1](
                     mockEvent,
                     'd',
@@ -184,7 +244,7 @@ define(
 
                 // White-boxy; we know which watch is which
                 mockScope.$watch.calls[0].args[1](testModel.layoutGrid);
-                mockScope.$watch.calls[1].args[1](testModel.composition);
+                mockScope.$watchCollection.calls[0].args[1](testModel.composition);
 
                 styleB = controller.getFrameStyle("b");
 
@@ -201,7 +261,6 @@ define(
                 mockScope.$watch.calls[0].args[1](testModel.layoutGrid);
 
                 // Notify that a drop occurred
-                testModel.composition.push('d');
                 mockScope.$on.mostRecentCall.args[1](
                     mockEvent,
                     'd',