diff --git a/js/id/graph/graph.js b/js/id/graph/graph.js index 120fa2fe7..3eea6a59a 100644 --- a/js/id/graph/graph.js +++ b/js/id/graph/graph.js @@ -1,13 +1,15 @@ -iD.Graph = function(entities, mutable) { - if (!(this instanceof iD.Graph)) return new iD.Graph(entities, mutable); +iD.Graph = function(other, mutable) { + if (!(this instanceof iD.Graph)) return new iD.Graph(other, mutable); - if (_.isArray(entities)) { - this.entities = {}; - for (var i = 0; i < entities.length; i++) { - this.entities[entities[i].id] = entities[i]; + if (_.isArray(other)) { + this.entities = Object.create({}); + for (var i = 0; i < other.length; i++) { + this.entities[other[i].id] = other[i]; } + } else if (other instanceof iD.Graph) { + this.rebase(other.base(), other.entities); } else { - this.entities = entities || {}; + this.entities = Object.create(other || {}); } this.transients = {}; @@ -97,10 +99,18 @@ iD.Graph.prototype = { return (this._childNodes[entity.id] = nodes); }, - merge: function(graph) { - return this.update(function () { - _.defaults(this.entities, graph.entities); - }); + base: function() { + return Object.getPrototypeOf ? + Object.getPrototypeOf(this.entities) : + this.entities.__proto__; + }, + + // Unlike other graph methods, rebase mutates in place. This is because it + // is used only during the history operation that merges newly downloaded + // data into each state. To external consumers, it should appear as if the + // graph always contained the newly downloaded data. + rebase: function(base, entities) { + this.entities = _.assign(Object.create(base), entities || this.entities); }, replace: function(entity) { @@ -120,7 +130,7 @@ iD.Graph.prototype = { }, update: function() { - var graph = this.frozen ? iD.Graph(_.clone(this.entities), true) : this; + var graph = this.frozen ? iD.Graph(this, true) : this; for (var i = 0; i < arguments.length; i++) { arguments[i].call(graph, graph); @@ -133,7 +143,6 @@ iD.Graph.prototype = { this.frozen = true; if (iD.debug) { - Object.freeze(this); Object.freeze(this.entities); } diff --git a/js/id/graph/history.js b/js/id/graph/history.js index 29b652554..c9e3e19f5 100644 --- a/js/id/graph/history.js +++ b/js/id/graph/history.js @@ -30,8 +30,12 @@ iD.History = function() { }, merge: function (graph) { - for (var i = 0; i < stack.length; i++) { - stack[i].graph = stack[i].graph.merge(graph); + var base = stack[0].graph.base(); + + _.defaults(base, graph.entities); + + for (var i = 1; i < stack.length; i++) { + stack[i].graph.rebase(base); } }, diff --git a/test/spec/graph/graph.js b/test/spec/graph/graph.js index 25382e7c9..fa83a3a03 100644 --- a/test/spec/graph/graph.js +++ b/test/spec/graph/graph.js @@ -1,25 +1,110 @@ describe('iD.Graph', function() { - it("can be constructed with an entities Object", function () { - var entity = iD.Entity(), - graph = iD.Graph({'n-1': entity}); - expect(graph.entity('n-1')).to.equal(entity); - }); - - it("can be constructed with an entities Array", function () { - var entity = iD.Entity(), - graph = iD.Graph([entity]); - expect(graph.entity(entity.id)).to.equal(entity); - }); - - if (iD.debug) { - it("is frozen", function () { - expect(Object.isFrozen(iD.Graph())).to.be.true; + describe("constructor", function () { + it("accepts an entities Object", function () { + var entity = iD.Entity(), + graph = iD.Graph({'n-1': entity}); + expect(graph.entity('n-1')).to.equal(entity); }); - it("freezes entities", function () { - expect(Object.isFrozen(iD.Graph().entities)).to.be.true; + it("accepts an entities Array", function () { + var entity = iD.Entity(), + graph = iD.Graph([entity]); + expect(graph.entity(entity.id)).to.equal(entity); }); - } + + it("accepts a Graph", function () { + var entity = iD.Entity(), + graph = iD.Graph(iD.Graph([entity])); + expect(graph.entity(entity.id)).to.equal(entity); + }); + + it("copies other's entities", function () { + var entity = iD.Entity(), + base = iD.Graph([entity]), + graph = iD.Graph(base); + expect(graph.entities).not.to.equal(base.entities); + }); + + it("rebases on other's base", function () { + var base = iD.Graph(), + graph = iD.Graph(base); + expect(graph.base()).to.equal(base.base()); + }); + + it("freezes by default", function () { + expect(iD.Graph().frozen).to.be.true; + }); + + it("remains mutable if passed true as second argument", function () { + expect(iD.Graph([], true).frozen).not.to.be.true; + }); + }); + + describe("#freeze", function () { + it("sets the frozen flag", function () { + expect(iD.Graph([], true).freeze().frozen).to.be.true; + }); + + if (iD.debug) { + it("freezes entities", function () { + expect(Object.isFrozen(iD.Graph().entities)).to.be.true; + }); + } + }); + + describe("#rebase", function () { + it("preserves existing entities", function () { + var node = iD.Node({id: 'n'}), + graph = iD.Graph([node]); + graph.rebase({}); + expect(graph.entity('n')).to.equal(node); + }); + + it("includes new entities", function () { + var node = iD.Node({id: 'n'}), + graph = iD.Graph(); + graph.rebase({'n': node}); + expect(graph.entity('n')).to.equal(node); + }); + + it("gives precedence to existing entities", function () { + var a = iD.Node({id: 'n'}), + b = iD.Node({id: 'n'}), + graph = iD.Graph([a]); + graph.rebase({'n': b}); + expect(graph.entity('n')).to.equal(a); + }); + + it("inherits entities from base prototypally", function () { + var graph = iD.Graph(); + graph.rebase({'n': iD.Node()}); + expect(graph.entities).not.to.have.ownProperty('n'); + }); + + xit("updates parentWays", function () { + var n = iD.Node({id: 'n'}), + w1 = iD.Way({id: 'w1', nodes: ['n']}), + w2 = iD.Way({id: 'w2', nodes: ['n']}), + graph = iD.Graph([n, w1]); + + graph.parentWays(n); + graph.rebase({'w2': w2}); + + expect(graph.parentWays(n)).to.eql([w1, w2]); + }); + + xit("updates parentRelations", function () { + var n = iD.Node({id: 'n'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n'}]}), + r2 = iD.Relation({id: 'r2', members: [{id: 'n'}]}), + graph = iD.Graph([n, r1]); + + graph.parentRelations(n); + graph.rebase({'r2': r2}); + + expect(graph.parentRelations(n)).to.eql([r1, r2]); + }); + }); describe("#remove", function () { it("returns a new graph", function () {