dia.Graph: storing graph reference on cell only when the cell is not …

…member of another graph (#457)

plugins/layout/DirectedGraph/joint.layout.DirectedGraph.js

@@ -18,7 +18,8 @@ joint.layout.DirectedGraph = {
         if (graphOrCells instanceof joint.dia.Graph) {
             graph = graphOrCells;
         } else {
-            graph = (new joint.dia.Graph()).resetCells(graphOrCells);
+            // Reset cells in dry mode so the graph reference is not stored on the cells.
+            graph = (new joint.dia.Graph()).resetCells(graphOrCells, { dry: true });
         }
 
         // This is not needed anymore.

src/joint.dia.graph.js

@@ -267,12 +267,16 @@ joint.dia.Graph = Backbone.Model.extend({
         return this;
     },
 
-    _prepareCell: function(cell) {
+    _prepareCell: function(cell, opt) {
 
         var attrs;
         if (cell instanceof Backbone.Model) {
             attrs = cell.attributes;
-            cell.graph = this;
+            if (!cell.graph && (!opt || !opt.dry)) {
+                // An element can not be member of more than one graph.
+                // A cell stops being the member of the graph after it's explicitely removed.
+                cell.graph = this;
+            }
         } else {
             // In case we're dealing with a plain JS object, we have to set the reference
             // to the `graph` right after the actual model is created. This happens in the `model()` function
@@ -293,11 +297,11 @@ joint.dia.Graph = Backbone.Model.extend({
         return lastCell ? (lastCell.get('z') || 0) : 0;
     },
 
-    addCell: function(cell, options) {
+    addCell: function(cell, opt) {
 
         if (_.isArray(cell)) {
 
-            return this.addCells(cell, options);
+            return this.addCells(cell, opt);
         }
 
         if (cell instanceof Backbone.Model) {
@@ -311,7 +315,7 @@ joint.dia.Graph = Backbone.Model.extend({
             cell.z = this.maxZIndex() + 1;
         }
 
-        this.get('cells').add(this._prepareCell(cell), options || {});
+        this.get('cells').add(this._prepareCell(cell, opt), opt || {});
 
         return this;
     },
@@ -339,7 +343,8 @@ joint.dia.Graph = Backbone.Model.extend({
     // Useful for bulk operations and optimizations.
     resetCells: function(cells, opt) {
 
-        this.get('cells').reset(_.map(cells, this._prepareCell, this), opt);
+        var preparedCells = _.map(cells, _.bind(this._prepareCell, this, _, opt));
+        this.get('cells').reset(preparedCells, opt);
 
         return this;
     },
@@ -379,7 +384,10 @@ joint.dia.Graph = Backbone.Model.extend({
         // would be triggered on the graph model.
         this.get('cells').remove(cell, { silent: true });
 
-        delete cell.graph;
+        if (cell.graph === this) {
+            // Remove the element graph reference only if the cell is the member of this graph.
+            cell.graph = null;
+        }
     },
 
     // Get a cell by `id`.

test/jointjs/graph.js

@@ -350,20 +350,52 @@ QUnit.module('graph', function(hooks) {
         var fromInstance = new joint.shapes.basic.Rect({ id: 'a' });
         var fromPlainObject = { id: 'b', type: 'basic.Rect' };
 
-        this.graph.addCell(fromInstance);
-        this.graph.addCell(fromPlainObject);
+        var graph1 = this.graph;
+        var graph2 = new joint.dia.Graph;
+
+        graph1.addCell(fromInstance);
+        graph1.addCell(fromPlainObject);
 
         assert.equal(
-            this.graph.getCell('a').graph,
-            this.graph,
+            graph1.getCell('a').graph,
+            graph1,
             'The graph reference was stored on the model when created from an instance.'
         );
 
         assert.equal(
-            this.graph.getCell('b').graph,
-            this.graph,
+            graph1.getCell('b').graph,
+            graph1,
             'The graph reference was stored on the model when created from a plain JS object.'
         );
+
+        var a = graph1.getCell('a');
+        var b = graph1.getCell('b');
+
+        a.remove();
+        assert.ok(a.graph === null, 'The graph reference is nulled when the model is removed from the graph.');
+
+        graph2.addCell(a);
+        assert.equal(
+            a.graph,
+            graph2,
+            'The graph reference was stored after the element was added to a graph.'
+        );
+
+        graph2.addCell(b);
+        assert.equal(
+            b.graph,
+            graph1,
+            'The graph reference was not stored after the model was added to a graph while it\'s still member of another graph.'
+        );
+
+        // Dry mode
+
+        b.remove();
+        graph1.addCell(b, { dry: true });
+        assert.ok(b.graph === null, 'The graph reference is not stored after element added when the `dry` flag is passed.');
+
+        graph1.resetCells([b], { dry: true });
+        assert.ok(b.graph === null, 'The graph reference is not stored after graph reset when the `dry` flag is passed.');
     });
 
     QUnit.test('graph.clear()', function(assert) {