Merge remote-tracking branch 'origin/open473' into open-master

platform/commonUI/edit/src/capabilities/EditableCompositionCapability.js

@@ -0,0 +1,36 @@
+/*global define*/
+
+
+define(
+    ['./EditableLookupCapability'],
+    function (EditableLookupCapability) {
+        'use strict';
+
+        /**
+         * Wrapper for the "composition" capability;
+         * ensures that any domain objects reachable in Edit mode
+         * are also wrapped as EditableDomainObjects.
+         *
+         * Meant specifically for use by EditableDomainObject and the
+         * associated cache; the constructor signature is particular
+         * to a pattern used there and may contain unused arguments.
+         */
+        return function EditableCompositionCapability(
+            contextCapability,
+            editableObject,
+            domainObject,
+            cache
+        ) {
+            // This is a "lookup" style capability (it looks up other
+            // domain objects), but we do not want to return the same
+            // specific value every time (composition may change)
+            return new EditableLookupCapability(
+                contextCapability,
+                editableObject,
+                domainObject,
+                cache,
+                false // Not idempotent
+            );
+        };
+    }
+);

platform/commonUI/edit/src/capabilities/EditableContextCapability.js

@@ -2,12 +2,12 @@
 
 
 define(
-    [],
+    ['./EditableLookupCapability'],
-    function () {
+    function (EditableLookupCapability) {
         'use strict';
 
         /**
-         * Wrapper for both "context" and "composition" capabilities;
+         * Wrapper for the "context" capability;
          * ensures that any domain objects reachable in Edit mode
          * are also wrapped as EditableDomainObjects.
          *
@@ -21,55 +21,15 @@ define(
             domainObject,
             cache
         ) {
-            var capability = Object.create(contextCapability);
+            // This is a "lookup" style capability (it looks up other
-
+            // domain objects), and it should be idempotent
-            // Check for domain object interface. If something has these
+            return new EditableLookupCapability(
-            // three methods, we assume it's a domain object.
+                contextCapability,
-            function isDomainObject(obj) {
+                editableObject,
-                return obj !== undefined &&
+                domainObject,
-                        typeof obj.getId === 'function' &&
+                cache,
-                        typeof obj.getModel === 'function' &&
+                true // Not idempotent
-                        typeof obj.getCapability === 'function';
+            );
-            }
-
-            // Check an object returned by the wrapped capability; if it
-            // is a domain object, we want to make it editable and/or get
-            // it from the cache of editable domain objects. This will
-            // prevent changes made in edit mode from modifying the actual
-            // underlying domain object.
-            function makeEditableObject(obj) {
-                return isDomainObject(obj) ?
-                        cache.getEditableObject(obj) :
-                        obj;
-            }
-
-            // Wrap a returned value (see above); if it's an array, wrap
-            // all elements.
-            function makeEditable(returnValue) {
-                return Array.isArray(returnValue) ?
-                        returnValue.map(makeEditableObject) :
-                        makeEditableObject(returnValue);
-            }
-
-            // Wrap a returned value (see above); if it's a promise, wrap
-            // the resolved value.
-            function wrapResult(result) {
-                return result.then ? // promise-like
-                        result.then(makeEditable) :
-                        makeEditable(result);
-            }
-
-            // Wrap all methods; return only editable domain objects.
-            Object.keys(contextCapability).forEach(function (k) {
-                capability[k] = function () {
-                    return wrapResult(contextCapability[k].apply(
-                        capability,
-                        arguments
-                    ));
-                };
-            });
-
-            return capability;
         };
     }
 );

platform/commonUI/edit/src/capabilities/EditableLookupCapability.js

@@ -0,0 +1,97 @@
+/*global define*/
+
+
+define(
+    [],
+    function () {
+        'use strict';
+
+        /**
+         * Wrapper for both "context" and "composition" capabilities;
+         * ensures that any domain objects reachable in Edit mode
+         * are also wrapped as EditableDomainObjects.
+         *
+         * Meant specifically for use by EditableDomainObject and the
+         * associated cache; the constructor signature is particular
+         * to a pattern used there and may contain unused arguments.
+         */
+        return function EditableLookupCapability(
+            contextCapability,
+            editableObject,
+            domainObject,
+            cache,
+            idempotent
+        ) {
+            var capability = Object.create(contextCapability);
+
+            // Check for domain object interface. If something has these
+            // three methods, we assume it's a domain object.
+            function isDomainObject(obj) {
+                return obj !== undefined &&
+                        typeof obj.getId === 'function' &&
+                        typeof obj.getModel === 'function' &&
+                        typeof obj.getCapability === 'function';
+            }
+
+            // Check an object returned by the wrapped capability; if it
+            // is a domain object, we want to make it editable and/or get
+            // it from the cache of editable domain objects. This will
+            // prevent changes made in edit mode from modifying the actual
+            // underlying domain object.
+            function makeEditableObject(obj) {
+                return isDomainObject(obj) ?
+                        cache.getEditableObject(obj) :
+                        obj;
+            }
+
+            // Wrap a returned value (see above); if it's an array, wrap
+            // all elements.
+            function makeEditable(returnValue) {
+                return Array.isArray(returnValue) ?
+                        returnValue.map(makeEditableObject) :
+                        makeEditableObject(returnValue);
+            }
+
+            // Wrap a returned value (see above); if it's a promise, wrap
+            // the resolved value.
+            function wrapResult(result) {
+                return result.then ? // promise-like
+                        result.then(makeEditable) :
+                        makeEditable(result);
+            }
+
+            // Return a wrapped version of a function, which ensures
+            // all results are editable domain objects.
+            function wrapFunction(fn) {
+                return function () {
+                    return wrapResult(contextCapability[fn].apply(
+                        capability,
+                        arguments
+                    ));
+                };
+            }
+
+            // Wrap a method such that it only delegates once.
+            function oneTimeFunction(fn) {
+                return function () {
+                    var result = wrapFunction(fn).apply(this, arguments);
+                    capability[fn] = function () {
+                        return result;
+                    };
+                    return result;
+                };
+            }
+
+            // Wrap a method of this capability
+            function wrapMethod(fn) {
+                capability[fn] =
+                    (idempotent ? oneTimeFunction : wrapFunction)(fn);
+            }
+
+            // Wrap all methods; return only editable domain objects.
+            Object.keys(contextCapability).forEach(wrapMethod);
+
+            return capability;
+        };
+    }
+);

platform/commonUI/edit/src/objects/EditableDomainObject.js

@@ -13,12 +13,14 @@ define(
     [
         '../capabilities/EditablePersistenceCapability',
         '../capabilities/EditableContextCapability',
+        '../capabilities/EditableCompositionCapability',
         '../capabilities/EditorCapability',
         './EditableDomainObjectCache'
     ],
     function (
         EditablePersistenceCapability,
         EditableContextCapability,
+        EditableCompositionCapability,
         EditorCapability,
         EditableDomainObjectCache
     ) {
@@ -27,7 +29,7 @@ define(
         var capabilityFactories = {
             persistence: EditablePersistenceCapability,
             context: EditableContextCapability,
-            composition: EditableContextCapability,
+            composition: EditableCompositionCapability,
             editor: EditorCapability
         };
 
@@ -53,9 +55,8 @@ define(
 
             // Constructor for EditableDomainObject, which adheres
             // to the same shared cache.
-            function EditableDomainObjectImpl(domainObject) {
+            function EditableDomainObjectImpl(domainObject, model) {
-                var model = JSON.parse(JSON.stringify(domainObject.getModel())),
+                var editableObject = Object.create(domainObject);
-                    editableObject = Object.create(domainObject);
 
                 // Only provide the cloned model.
                 editableObject.getModel = function () { return model; };

platform/commonUI/edit/src/objects/EditableDomainObjectCache.js

@@ -15,7 +15,8 @@
  * @module editor/object/editable-domain-object-cache
  */
 define(
-    function () {
+    ["./EditableModelCache"],
+    function (EditableModelCache) {
         'use strict';
 
         /**
@@ -32,7 +33,7 @@ define(
          * @memberof module:editor/object/editable-domain-object-cache
          */
         function EditableDomainObjectCache(EditableDomainObject) {
-            var cache = {},
+            var cache = new EditableModelCache(),
                 dirty = {};
 
             return {
@@ -44,9 +45,10 @@ define(
                  * @returns {DomainObject} the domain object in an editable form
                  */
                 getEditableObject: function (domainObject) {
-                    var id = domainObject.getId();
+                    return new EditableDomainObject(
-                    return (cache[id] =
+                        domainObject,
-                        cache[id] || new EditableDomainObject(domainObject));
+                        cache.getCachedModel(domainObject)
+                    );
                 },
                 /**
                  * Mark an editable domain object (presumably already cached)

platform/commonUI/edit/src/objects/EditableModelCache.js

@@ -0,0 +1,42 @@
+/*global define*/
+
+define(
+    [],
+    function () {
+        "use strict";
+
+        /**
+         * An editable model cache stores domain object models that have been
+         * made editable, to support a group that can be saved all-at-once.
+         * This is useful in Edit mode, which is launched for a specific
+         * object but may contain changes across many objects.
+         * @constructor
+         */
+        function EditableModelCache() {
+            var cache = {};
+
+            // Deep-copy a model. Models are JSONifiable, so this can be
+            // done by stringification then destringification
+            function clone(model) {
+                return JSON.parse(JSON.stringify(model));
+            }
+
+            return {
+                /**
+                 * Get this domain object's model from the cache (or
+                 * place it in the cache if it isn't in the cache yet)
+                 * @returns a clone of the domain object's model
+                 */
+                getCachedModel: function (domainObject) {
+                    var id = domainObject.getId();
+
+                    return (cache[id] =
+                        cache[id] || clone(domainObject.getModel()));
+                }
+            };
+
+        }
+
+        return EditableModelCache;
+    }
+);

platform/commonUI/edit/test/capabilities/EditableCompositionCapabilitySpec.js

@@ -0,0 +1,54 @@
+/*global define,describe,it,expect,beforeEach,jasmine*/
+
+define(
+    ["../../src/capabilities/EditableCompositionCapability"],
+    function (EditableCompositionCapability) {
+        "use strict";
+
+        describe("An editable composition capability", function () {
+            var mockContext,
+                mockEditableObject,
+                mockDomainObject,
+                mockTestObject,
+                someValue,
+                mockFactory,
+                capability;
+
+            beforeEach(function () {
+                // EditableContextCapability should watch ALL
+                // methods for domain objects, so give it an
+                // arbitrary interface to wrap.
+                mockContext =
+                    jasmine.createSpyObj("context", [ "getDomainObject" ]);
+                mockTestObject = jasmine.createSpyObj(
+                    "domainObject",
+                    [ "getId", "getModel", "getCapability" ]
+                );
+                mockFactory =
+                    jasmine.createSpyObj("factory", ["getEditableObject"]);
+
+                someValue = { x: 42 };
+
+                mockContext.getDomainObject.andReturn(mockTestObject);
+                mockFactory.getEditableObject.andReturn(someValue);
+
+                capability = new EditableCompositionCapability(
+                    mockContext,
+                    mockEditableObject,
+                    mockDomainObject,
+                    mockFactory
+                );
+
+            });
+
+            // Most behavior is tested for EditableLookupCapability,
+            // so just verify that this isse
+            it("presumes non-idempotence of its wrapped capability", function () {
+                expect(capability.getDomainObject())
+                    .toEqual(capability.getDomainObject());
+                expect(mockContext.getDomainObject.calls.length).toEqual(2);
+            });
+
+        });
+    }
+);

platform/commonUI/edit/test/capabilities/EditableContextCapabilitySpec.js

@@ -11,63 +11,40 @@ define(
                 mockDomainObject,
                 mockTestObject,
                 someValue,
-                factory,
+                mockFactory,
                 capability;
 
             beforeEach(function () {
                 // EditableContextCapability should watch ALL
                 // methods for domain objects, so give it an
                 // arbitrary interface to wrap.
-                mockContext = jasmine.createSpyObj(
+                mockContext =
-                    "context",
+                    jasmine.createSpyObj("context", [ "getDomainObject" ]);
-                    [
-                        "getSomething",
-                        "getDomainObject",
-                        "getDomainObjectArray"
-                    ]
-                );
                 mockTestObject = jasmine.createSpyObj(
                     "domainObject",
                     [ "getId", "getModel", "getCapability" ]
                 );
-                factory = {
+                mockFactory =
-                    getEditableObject: function (v) {
+                    jasmine.createSpyObj("factory", ["getEditableObject"]);
-                        return {
-                            isFromTestFactory: true,
-                            calledWith: v
-                        };
-                    }
-                };
 
                 someValue = { x: 42 };
 
-                mockContext.getSomething.andReturn(someValue);
                 mockContext.getDomainObject.andReturn(mockTestObject);
-                mockContext.getDomainObjectArray.andReturn([mockTestObject]);
+                mockFactory.getEditableObject.andReturn(someValue);
 
                 capability = new EditableContextCapability(
                     mockContext,
                     mockEditableObject,
                     mockDomainObject,
-                    factory
+                    mockFactory
                 );
 
             });
 
-            it("wraps retrieved domain objects", function () {
+            it("presumes idempotence of its wrapped capability", function () {
-                var object = capability.getDomainObject();
+                expect(capability.getDomainObject())
-                expect(object.isFromTestFactory).toBe(true);
+                    .toEqual(capability.getDomainObject());
-                expect(object.calledWith).toEqual(mockTestObject);
+                expect(mockContext.getDomainObject.calls.length).toEqual(1);
-            });
-
-            it("wraps retrieved domain object arrays", function () {
-                var object = capability.getDomainObjectArray()[0];
-                expect(object.isFromTestFactory).toBe(true);
-                expect(object.calledWith).toEqual(mockTestObject);
-            });
-
-            it("does not wrap non-domain-objects", function () {
-                expect(capability.getSomething()).toEqual(someValue);
             });
 
         });

platform/commonUI/edit/test/capabilities/EditableLookupCapabilitySpec.js

@@ -0,0 +1,102 @@
+/*global define,describe,it,expect,beforeEach,jasmine*/
+
+define(
+    ["../../src/capabilities/EditableLookupCapability"],
+    function (EditableLookupCapability) {
+        "use strict";
+
+        describe("An editable lookup capability", function () {
+            var mockContext,
+                mockEditableObject,
+                mockDomainObject,
+                mockTestObject,
+                someValue,
+                factory,
+                capability;
+
+            beforeEach(function () {
+                // EditableContextCapability should watch ALL
+                // methods for domain objects, so give it an
+                // arbitrary interface to wrap.
+                mockContext = jasmine.createSpyObj(
+                    "context",
+                    [
+                        "getSomething",
+                        "getDomainObject",
+                        "getDomainObjectArray"
+                    ]
+                );
+                mockTestObject = jasmine.createSpyObj(
+                    "domainObject",
+                    [ "getId", "getModel", "getCapability" ]
+                );
+                factory = {
+                    getEditableObject: function (v) {
+                        return {
+                            isFromTestFactory: true,
+                            calledWith: v
+                        };
+                    }
+                };
+
+                someValue = { x: 42 };
+
+                mockContext.getSomething.andReturn(someValue);
+                mockContext.getDomainObject.andReturn(mockTestObject);
+                mockContext.getDomainObjectArray.andReturn([mockTestObject]);
+
+                capability = new EditableLookupCapability(
+                    mockContext,
+                    mockEditableObject,
+                    mockDomainObject,
+                    factory,
+                    false
+                );
+
+            });
+
+            it("wraps retrieved domain objects", function () {
+                var object = capability.getDomainObject();
+                expect(object.isFromTestFactory).toBe(true);
+                expect(object.calledWith).toEqual(mockTestObject);
+            });
+
+            it("wraps retrieved domain object arrays", function () {
+                var object = capability.getDomainObjectArray()[0];
+                expect(object.isFromTestFactory).toBe(true);
+                expect(object.calledWith).toEqual(mockTestObject);
+            });
+
+            it("does not wrap non-domain-objects", function () {
+                expect(capability.getSomething()).toEqual(someValue);
+            });
+
+            it("caches idempotent lookups", function () {
+                capability = new EditableLookupCapability(
+                    mockContext,
+                    mockEditableObject,
+                    mockDomainObject,
+                    factory,
+                    true // idempotent
+                );
+                expect(capability.getDomainObject())
+                    .toEqual(capability.getDomainObject());
+                expect(mockContext.getDomainObject.calls.length).toEqual(1);
+            });
+
+            it("does not cache non-idempotent lookups", function () {
+                capability = new EditableLookupCapability(
+                    mockContext,
+                    mockEditableObject,
+                    mockDomainObject,
+                    factory,
+                    false // Not idempotent
+                );
+                expect(capability.getDomainObject())
+                    .toEqual(capability.getDomainObject());
+                expect(mockContext.getDomainObject.calls.length).toEqual(2);
+            });
+
+        });
+    }
+);

platform/commonUI/edit/test/objects/EditableDomainObjectCacheSpec.js

@@ -24,9 +24,10 @@ define(
                 };
             }
 
-            function WrapObject(domainObject) {
+            function WrapObject(domainObject, model) {
                 var result = Object.create(domainObject);
                 result.wrapped = true;
+                result.wrappedModel = model;
                 captured.wraps = (captured.wraps || 0) + 1;
                 return result;
             }
@@ -49,24 +50,30 @@ define(
                 expect(wrappedObject.getId()).toEqual(domainObject.getId());
             });
 
-            it("only wraps objects once", function () {
+            it("wraps objects repeatedly, wraps models once", function () {
                 var domainObject = new TestObject('test-id'),
-                    wrappedObject;
+                    wrappedObjects = [];
 
                 // Verify precondition
                 expect(captured.wraps).toBeUndefined();
 
                 // Invoke a few more times; expect count not to increment
-                wrappedObject = cache.getEditableObject(domainObject);
+                wrappedObjects.push(cache.getEditableObject(domainObject));
-                expect(captured.wraps).toEqual(1);
-                wrappedObject = cache.getEditableObject(domainObject);
-                expect(captured.wraps).toEqual(1);
-                wrappedObject = cache.getEditableObject(domainObject);
                 expect(captured.wraps).toEqual(1);
+                wrappedObjects.push(cache.getEditableObject(domainObject));
+                expect(captured.wraps).toEqual(2);
+                wrappedObjects.push(cache.getEditableObject(domainObject));
+                expect(captured.wraps).toEqual(3);
 
                 // Verify that the last call still gave us a wrapped object
-                expect(wrappedObject.wrapped).toBeTruthy();
+                expect(wrappedObjects[0].wrapped).toBeTruthy();
-                expect(wrappedObject.getId()).toEqual(domainObject.getId());
+                expect(wrappedObjects[0].getId()).toEqual(domainObject.getId());
+
+                // Verify that objects are distinct but models are identical
+                expect(wrappedObjects[0].wrappedModel)
+                    .toBe(wrappedObjects[1].wrappedModel);
+                expect(wrappedObjects[0]).not
+                    .toBe(wrappedObjects[1]);
             });
 
             it("saves objects that have been marked dirty", function () {

platform/commonUI/edit/test/objects/EditableModelCacheSpec.js

@@ -0,0 +1,60 @@
+/*global define,describe,it,expect,beforeEach,jasmine*/
+
+define(
+    ["../../src/objects/EditableModelCache"],
+    function (EditableModelCache) {
+        "use strict";
+
+        describe("The editable model cache", function () {
+            var mockObject,
+                mockOtherObject,
+                testModel,
+                testId,
+                otherModel,
+                otherId,
+                cache;
+
+            beforeEach(function () {
+                testId = "test";
+                testModel = { someKey: "some value" };
+                otherId = "other";
+                otherModel = { someKey: "some other value" };
+
+                mockObject = jasmine.createSpyObj(
+                    "domainObject",
+                    [ "getId", "getModel" ]
+                );
+                mockOtherObject = jasmine.createSpyObj(
+                    "domainObject",
+                    [ "getId", "getModel" ]
+                );
+
+                mockObject.getId.andReturn(testId);
+                mockObject.getModel.andReturn(testModel);
+                mockOtherObject.getId.andReturn(otherId);
+                mockOtherObject.getModel.andReturn(otherModel);
+
+                cache = new EditableModelCache();
+            });
+
+            it("provides clones of domain object models", function () {
+                var model = cache.getCachedModel(mockObject);
+                // Should be identical...
+                expect(model).toEqual(testModel);
+                // ...but not pointer-identical
+                expect(model).not.toBe(testModel);
+            });
+
+            it("provides only one clone per object", function () {
+                var model = cache.getCachedModel(mockObject);
+                expect(cache.getCachedModel(mockObject)).toBe(model);
+            });
+
+            it("maintains separate caches per-object", function () {
+                expect(cache.getCachedModel(mockObject))
+                    .not.toEqual(cache.getCachedModel(mockOtherObject));
+            });
+        });
+
+    }
+);

platform/commonUI/edit/test/suite.json

@@ -8,9 +8,12 @@
     "actions/PropertiesDialog",
     "actions/RemoveAction",
     "actions/SaveAction",
+    "capabilities/EditableCompositionCapability",
     "capabilities/EditableContextCapability",
+    "capabilities/EditableLookupCapability",
     "capabilities/EditablePersistenceCapability",
     "capabilities/EditorCapability",
     "objects/EditableDomainObject",
-    "objects/EditableDomainObjectCache"
+    "objects/EditableDomainObjectCache",
+    "objects/EditableModelCache"
 ]