Merge pull request #159 from udduttaatlinkedIn/feature/adding-proxy-o…

…bject-util

Adding ability to use proxy objects in order to avoid object mutations to original content object

addon/components/draggable-object.js

@@ -4,6 +4,7 @@ import { alias } from '@ember/object/computed';
 import { computed } from '@ember/object';
 import { scheduleOnce, next } from '@ember/runloop';
 import { set } from '@ember/object';
+import { wrapper } from 'ember-drag-drop/utils/proxy-unproxy-objects';
 
 export default Component.extend({
   dragCoordinator: service(),
@@ -22,6 +23,11 @@ export default Component.extend({
     return isDraggable || null;
   }),
 
+  proxyContent: computed('content', function() {
+    return wrapper(this.get('content'));
+  }),
+
+
   init() {
     this._super(...arguments);
     if (this.get('dragHandle')) {
@@ -68,7 +74,7 @@ export default Component.extend({
 
     let dataTransfer = event.dataTransfer;
 
-    let obj = this.get('content');
+    let obj = this.get('proxyContent');
     let id = null;
     let coordinator = this.get('coordinator');
     if (coordinator) {
@@ -106,7 +112,7 @@ export default Component.extend({
       return;
     }
 
-    let obj = this.get('content');
+    let obj = this.get('proxyContent');
 
     if (obj && typeof obj === 'object') {
       set(obj, 'isDraggingObject', false);
@@ -150,7 +156,7 @@ export default Component.extend({
 
   actions: {
     selectForDrag() {
-      let obj = this.get('content');
+      let obj = this.get('proxyContent');
       let hashId = this.get('coordinator').setObject(obj, { source: this });
       this.set('coordinator.clickedId', hashId);
     }

addon/utils/proxy-unproxy-objects.js

@@ -0,0 +1,48 @@
+import { isNone } from '@ember/utils';
+import { guidFor } from '@ember/object/internals';
+
+/**
+ * On drag action we need to provide a property `isDraggingObject` to the UI.
+ * This utility is used to create a wrapper object around the object passed to the proxy function.
+ * We use this wrapper to prevent the `draggable-object` from mutating the original object by appending
+ * `isDraggingObject` to the content.
+ * 
+ * This unexpected mutation causes problems when the targeted content is not prepared to handle
+ * the additional property, and potentially leaks local state onto an object that likely holds state
+ * for the route or application more generally.
+ */
+
+/**
+ * @access public
+ * Returns the passed object wrapped within new object.
+ * Used to proxy draggable objects.
+ * @param objectToProxy Object to proxy.
+ * @returns {Object} Proxy object.
+ */
+export function wrapper(objectToProxy) {
+  // If we do not have any content for the object to proxy, 
+  // we do not wish to render that item.
+  if (!isNone(objectToProxy)) {
+    const guidKey = guidFor(objectToProxy);
+    return {
+      [guidKey]: objectToProxy,
+      unwrappingKey: guidKey,
+      id: objectToProxy.id
+    };
+  }
+  return null;
+}
+
+/**
+ * @access public
+ * Returns the content of the passed object.
+ * Used to un-proxy draggable objects.
+ * @param objectToUnproxy Object to un-proxy.
+ * @returns {Object} Content of the proxy object.
+ */
+export function unwrapper(objectToUnproxy) {
+  if (!isNone(objectToUnproxy)) {
+    return objectToUnproxy[objectToUnproxy.unwrappingKey];
+  }
+  return null;
+}

app/models/coordinator.js

@@ -2,6 +2,7 @@ import EmberObject from '@ember/object';
 import Evented from '@ember/object/evented';
 import { computed } from '@ember/object';
 import ObjHash from './obj-hash';
+import { unwrapper } from 'ember-drag-drop/utils/proxy-unproxy-objects';
 
 export default EmberObject.extend(Evented, {
   objectMap: computed(function() {
@@ -20,9 +21,9 @@ export default EmberObject.extend(Evented, {
       payload.ops.target.sendAction('action',payload.obj);
     }
 
-    this.trigger("objectMoved", {obj: payload.obj, source: payload.ops.source, target: ops.target});
+    this.trigger("objectMoved", {obj: unwrapper(payload.obj), source: payload.ops.source, target: ops.target});
 
-    return payload.obj;
+    return unwrapper(payload.obj);
   },
 
   setObject: function(obj,ops) {

tests/unit/components/draggable-object-test.js

@@ -4,6 +4,7 @@ import { A } from '@ember/array';
 import { test, moduleForComponent } from 'ember-qunit';
 import Coordinator from '../../../models/coordinator';
 import MockEvent from '../../helpers/mock-event';
+import { wrapper } from 'ember-drag-drop/utils/proxy-unproxy-objects';
 
 const Thing = EmberObject.extend({});
 
@@ -62,7 +63,7 @@ test("drop callbacks", function(assert) {
   let component = this.subject({ coordinator });
 
   let hashId = run(function() {
-    return coordinator.setObject(thing, { source: component });
+    return coordinator.setObject(wrapper(thing), { source: component });
   });
 
   coordinator.getObject(hashId);

tests/unit/utils/proxy-unproxy-objects-test.js

@@ -0,0 +1,46 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import {
+  wrapper,
+  unwrapper,
+} from 'ember-drag-drop/utils/proxy-unproxy-objects';
+import { guidFor } from '@ember/object/internals';
+
+module(
+  'Unit | Util | ember-drag-drop/utils/proxy-unproxy-objects',
+  function(hooks) {
+    setupTest(hooks);
+
+    hooks.beforeEach(function() {
+      this.testObject = {
+        value: true,
+        id: 123,
+      };
+      this.testObjectGuid = guidFor(this.testObject);
+    });
+
+    test('wrapper returns a new object containing content and ID feilds', function testProxyObjAction(assert) {
+      assert.expect(2);
+      assert.equal(
+        wrapper(this.testObject)[this.testObjectGuid],
+        this.testObject,
+        'Wrapped object contains corresponding guid field containing the original object content'
+      );
+
+      assert.equal(
+        wrapper(this.testObject).id,
+        this.testObject.id,
+        'Object contains ID field'
+      );
+    });
+
+    test('unwrapper returns back the original object', function testUnproxyObjAction(assert) {
+      assert.expect(1);
+      assert.deepEqual(
+        unwrapper(wrapper(this.testObject)),
+        this.testObject,
+        'Returned object is same as the original test object'
+      );
+    });
+  }
+);