Modify Graph so it can track deleted ids

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;

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;
     }
 };

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);
+                })
             };
         },
 

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([]);
         });
     });
 });

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");