From 255e3aaabdb704d18da4e33f13e7ca45e50c1411 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 7 Dec 2012 11:54:22 -0500 Subject: [PATCH] Modify Graph so it can track deleted ids --- js/id/format/xml.js | 6 +-- js/id/graph/graph.js | 48 +++++++++++------ js/id/graph/history.js | 33 +++++------- test/spec/graph/graph.js | 106 +++++++++++++++++++++++-------------- test/spec/graph/history.js | 25 +++++++++ 5 files changed, 137 insertions(+), 81 deletions(-) diff --git a/js/id/format/xml.js b/js/id/format/xml.js index 1c0fe8ad6..ec036120f 100644 --- a/js/id/format/xml.js +++ b/js/id/format/xml.js @@ -52,17 +52,17 @@ iD.format.XML = { '@version': 0.3, '@generator': 'iD', // TODO: copy elements first - create: nest(changes.create.map(function(c) { + create: nest(changes.created.map(function(c) { var x = iD.Entity(c); x.changeset = changeset_id; return x; }).map(iD.format.XML.rep)), - modify: changes.modify.map(function(c) { + modify: changes.modified.map(function(c) { var x = iD.Entity(c); x.changeset = changeset_id; return x; }).map(iD.format.XML.rep), - 'delete': changes['delete'].map(function(c) { + 'delete': changes.deleted.map(function(c) { var x = iD.Entity(c); x.changeset = changeset_id; return x; diff --git a/js/id/graph/graph.js b/js/id/graph/graph.js index 0626a492f..b05129aee 100644 --- a/js/id/graph/graph.js +++ b/js/id/graph/graph.js @@ -24,14 +24,14 @@ iD.Graph.prototype = { parentWays: function(id) { // This is slow and a bad hack. return _.filter(this.entities, function(e) { - return e.type === 'way' && e.nodes.indexOf(id) !== -1; + return e && e.type === 'way' && e.nodes.indexOf(id) !== -1; }); }, parentRelations: function(id) { // This is slow and a bad hack. return _.filter(this.entities, function(e) { - return e.type === 'relation' && e.members.indexOf(id) !== -1; + return e && e.type === 'relation' && e.members.indexOf(id) !== -1; }); }, @@ -49,7 +49,13 @@ iD.Graph.prototype = { remove: function(entity) { var entities = _.clone(this.entities); - delete entities[entity.id]; + + if (entity.created()) { + delete entities[entity.id]; + } else { + entities[entity.id] = undefined; + } + return iD.Graph(entities); }, @@ -58,7 +64,7 @@ iD.Graph.prototype = { var items = []; for (var i in this.entities) { var entity = this.entities[i]; - if (entity.intersects(extent, this)) { + if (entity && entity.intersects(extent, this)) { items.push(this.fetch(entity.id)); } } @@ -68,26 +74,34 @@ iD.Graph.prototype = { // Resolve the id references in a way, replacing them with actual objects. fetch: function(id) { var entity = this.entities[id], nodes = []; - if (!entity.nodes || !entity.nodes.length) return entity; + if (!entity || !entity.nodes || !entity.nodes.length) return entity; for (var i = 0, l = entity.nodes.length; i < l; i++) { nodes[i] = this.fetch(entity.nodes[i]); } return iD.Entity(entity, {nodes: nodes}); }, - modifications: function() { - return _.filter(this.entities, function(entity) { - return entity.modified(); - }).map(function(e) { - return this.fetch(e.id); - }.bind(this)); + modified: function() { + var result = []; + _.each(this.entities, function(entity, id) { + if (entity && entity.modified()) result.push(id); + }); + return result; }, - creations: function() { - return _.filter(this.entities, function(entity) { - return entity.created(); - }).map(function(e) { - return this.fetch(e.id); - }.bind(this)); + created: function() { + var result = []; + _.each(this.entities, function(entity, id) { + if (entity && entity.created()) result.push(id); + }); + return result; + }, + + deleted: function() { + var result = []; + _.each(this.entities, function(entity, id) { + if (!entity) result.push(id); + }); + return result; } }; diff --git a/js/id/graph/history.js b/js/id/graph/history.js index eec93f3e8..683b0af16 100644 --- a/js/id/graph/history.js +++ b/js/id/graph/history.js @@ -81,29 +81,20 @@ iD.History = function() { } }, - // generate reports of changes for changesets to use - modify: function () { - return stack[index].graph.modifications(); - }, - - create: function () { - return stack[index].graph.creations(); - }, - - 'delete': function () { - return _.difference( - _.pluck(stack[0].graph.entities, 'id'), - _.pluck(stack[index].graph.entities, 'id') - ).map(function (id) { - return stack[0].graph.fetch(id); - }); - }, - changes: function () { + var initial = stack[0].graph, + current = stack[index].graph; + return { - modify: this.modify(), - create: this.create(), - 'delete': this['delete']() + modified: current.modified().map(function (id) { + return current.fetch(id); + }), + created: current.created().map(function (id) { + return current.fetch(id); + }), + deleted: current.deleted().map(function (id) { + return initial.fetch(id); + }) }; }, diff --git a/test/spec/graph/graph.js b/test/spec/graph/graph.js index 248056c0d..1341996d8 100644 --- a/test/spec/graph/graph.js +++ b/test/spec/graph/graph.js @@ -21,35 +21,46 @@ describe('iD.Graph', function() { }); } - describe('operations', function() { - it('#remove', function() { - var entities = { 'n-1': { - type: 'node', - loc: [-80, 30], - id: 'n-1' - } - }; - var graph = iD.Graph(entities, 'first graph'); - var g2 = graph.remove(entities['n-1'], 'Removed node'); - expect(graph.entity('n-1')).to.equal(entities['n-1']); - expect(g2.entity('n-1')).to.equal(undefined); + describe("#remove", function () { + it("returns a new graph", function () { + var node = iD.Node(), + graph = iD.Graph([node]); + expect(graph.remove(node)).not.to.equal(graph); }); - it('#replace', function() { - var entities = { 'n-1': { - type: 'node', - loc: [-80, 30], - id: 'n-1' - } - }; - var replacement = { - type: 'node', - loc: [-80, 40], - id: 'n-1' - }; - var graph = iD.Graph(entities, 'first graph'); - var g2 = graph.replace(replacement, 'Removed node'); - expect(graph.entity('n-1').loc[1]).to.equal(30); - expect(g2.entity('n-1').loc[1]).to.equal(40); + + it("doesn't modify the receiver", function () { + var node = iD.Node(), + graph = iD.Graph([node]); + graph.remove(node); + expect(graph.entity(node.id)).to.equal(node); + }); + + it("removes the entity from the result", function () { + var node = iD.Node(), + graph = iD.Graph([node]); + expect(graph.remove(node).entity(node.id)).to.be.undefined; + }); + }); + + describe("#replace", function () { + it("returns a new graph", function () { + var node = iD.Node(), + graph = iD.Graph([node]); + expect(graph.replace(node)).not.to.equal(graph); + }); + + it("doesn't modify the receiver", function () { + var node = iD.Node(), + graph = iD.Graph([node]); + graph.replace(node); + expect(graph.entity(node.id)).to.equal(node); + }); + + it("replaces the entity in the result", function () { + var node1 = iD.Node(), + node2 = node1.update({}), + graph = iD.Graph([node1]); + expect(graph.replace(node2).entity(node2.id)).to.equal(node2); }); }); @@ -82,21 +93,36 @@ describe('iD.Graph', function() { }); }); - describe("#modifications", function () { - it("filters entities by modified", function () { - var a = {id: 'a', modified: function () { return true; }}, - b = {id: 'b', modified: function () { return false; }}, - graph = iD.Graph({ 'a': a, 'b': b }); - expect(graph.modifications()).to.eql([graph.fetch('a')]); + describe("#modified", function () { + it("returns an Array of ids of modified entities", function () { + var node1 = iD.Node({id: 'n1', _updated: true}), + node2 = iD.Node({id: 'n2'}), + graph = iD.Graph([node1, node2]); + expect(graph.modified()).to.eql([node1.id]); }); }); - describe("#creations", function () { - it("filters entities by created", function () { - var a = {id: 'a', created: function () { return true; }}, - b = {id: 'b', created: function () { return false; }}, - graph = iD.Graph({ 'a': a, 'b': b }); - expect(graph.creations()).to.eql([graph.fetch('a')]); + describe("#created", function () { + it("returns an Array of ids of created entities", function () { + var node1 = iD.Node({id: 'n-1', _updated: true}), + node2 = iD.Node({id: 'n2'}), + graph = iD.Graph([node1, node2]); + expect(graph.created()).to.eql([node1.id]); + }); + }); + + describe("#deleted", function () { + it("returns an Array of ids of deleted entities", function () { + var node1 = iD.Node({id: "n1"}), + node2 = iD.Node(), + graph = iD.Graph([node1, node2]).remove(node1); + expect(graph.deleted()).to.eql([node1.id]); + }); + + it("doesn't include created entities that were subsequently deleted", function () { + var node = iD.Node(), + graph = iD.Graph([node]).remove(node); + expect(graph.deleted()).to.eql([]); }); }); }); diff --git a/test/spec/graph/history.js b/test/spec/graph/history.js index efabcc32a..be7237292 100644 --- a/test/spec/graph/history.js +++ b/test/spec/graph/history.js @@ -101,6 +101,31 @@ describe("iD.History", function () { }); }); + describe("#changes", function () { + it("includes created entities", function () { + var node = iD.Node(); + history.perform(function (graph) { return graph.replace(node); }); + expect(history.changes().created).to.eql([node]); + }); + + it("includes modified entities", function () { + var node1 = iD.Node({id: "n1"}), + node2 = node1.update({}), + graph = iD.Graph([node1]); + history.merge(graph); + history.perform(function (graph) { return graph.replace(node2); }); + expect(history.changes().modified).to.eql([node2]); + }); + + it("includes deleted entities", function () { + var node = iD.Node({id: "n1"}), + graph = iD.Graph([node]); + history.merge(graph); + history.perform(function (graph) { return graph.remove(node); }); + expect(history.changes().deleted).to.eql([node]); + }); + }); + describe("#reset", function () { it("clears the version stack", function () { history.perform(action, "annotation");