Don't call async callbacks after connection resets/switches

(closes #4288)

modules/core/context.js

@@ -112,10 +112,6 @@ export function coreContext() {
 
 
     /* Connection */
-    function entitiesLoaded(err, result) {
-        if (!err) history.merge(result.data, result.extent);
-    }
-
     context.preauth = function(options) {
         if (connection) {
             connection.switch(options);
@@ -124,39 +120,51 @@ export function coreContext() {
     };
 
     context.loadTiles = utilCallWhenIdle(function(projection, dimensions, callback) {
+        var cid;
         function done(err, result) {
-            entitiesLoaded(err, result);
+            if (connection.getConnectionId() !== cid) {
+                if (callback) callback({ message: 'Connection Switched', status: -1 });
+                return;
+            }
+            if (!err) history.merge(result.data, result.extent);
             if (callback) callback(err, result);
         }
         if (connection) {
+            cid = connection.getConnectionId();
             connection.loadTiles(projection, dimensions, done);
         }
     });
 
-    context.loadEntity = function(id, callback) {
+    context.loadEntity = function(entityId, callback) {
+        var cid;
         function done(err, result) {
-            entitiesLoaded(err, result);
+            if (connection.getConnectionId() !== cid) {
+                if (callback) callback({ message: 'Connection Switched', status: -1 });
+                return;
+            }
+            if (!err) history.merge(result.data, result.extent);
             if (callback) callback(err, result);
         }
         if (connection) {
-            connection.loadEntity(id, done);
+            cid = connection.getConnectionId();
+            connection.loadEntity(entityId, done);
         }
     };
 
-    context.zoomToEntity = function(id, zoomTo) {
+    context.zoomToEntity = function(entityId, zoomTo) {
         if (zoomTo !== false) {
-            this.loadEntity(id, function(err, result) {
+            this.loadEntity(entityId, function(err, result) {
                 if (err) return;
-                var entity = _find(result.data, function(e) { return e.id === id; });
+                var entity = _find(result.data, function(e) { return e.id === entityId; });
                 if (entity) { map.zoomTo(entity); }
             });
         }
 
         map.on('drawn.zoomToEntity', function() {
-            if (!context.hasEntity(id)) return;
+            if (!context.hasEntity(entityId)) return;
             map.on('drawn.zoomToEntity', null);
             context.on('enter.zoomToEntity', null);
-            context.enter(modeSelect(context, [id]));
+            context.enter(modeSelect(context, [entityId]));
         });
 
         context.on('enter.zoomToEntity', function() {

modules/renderer/map.js

@@ -74,10 +74,10 @@ export function rendererMap(context) {
         mousemove;
 
     var zoom = d3_zoom()
-            .scaleExtent([ztok(2), ztok(24)])
-            .interpolate(d3_interpolate)
-            .filter(zoomEventFilter)
-            .on('zoom', zoomPan);
+        .scaleExtent([ztok(2), ztok(24)])
+        .interpolate(d3_interpolate)
+        .filter(zoomEventFilter)
+        .on('zoom', zoomPan);
 
     var _selection = d3_select(null);
     var isRedrawScheduled = false;

modules/services/osm.js

@@ -31,6 +31,7 @@ var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'load
     inflight = {},
     loadedTiles = {},
     entityCache = {},
+    connectionId = 1,
     tileZoom = 16,
     oauth = osmAuth({
         url: urlroot,
@@ -189,6 +190,7 @@ export default {
 
 
     reset: function() {
+        connectionId++;
         userChangesets = undefined;
         userDetails = undefined;
         rateLimitError = undefined;
@@ -200,6 +202,11 @@ export default {
     },
 
 
+    getConnectionId: function() {
+        return connectionId;
+    },
+
+
     changesetURL: function(changesetId) {
         return urlroot + '/changeset/' + changesetId;
     },
@@ -232,8 +239,14 @@ export default {
     loadFromAPI: function(path, callback, options) {
         options = _extend({ cache: true }, options);
         var that = this;
+        var cid = connectionId;
 
         function done(err, xml) {
+            if (that.getConnectionId() !== cid) {
+                if (callback) callback({ message: 'Connection Switched', status: -1 });
+                return;
+            }
+
             var isAuthenticated = that.authenticated();
 
             // 400 Bad Request, 401 Unauthorized, 403 Forbidden
@@ -333,6 +346,8 @@ export default {
 
 
     putChangeset: function(changeset, changes, callback) {
+        var that = this;
+        var cid = connectionId;
 
         // Create the changeset..
         oauth.xhr({
@@ -344,7 +359,13 @@ export default {
 
 
         function createdChangeset(err, changeset_id) {
-            if (err) return callback(err);
+            if (err) {
+                return callback(err);
+            }
+            if (that.getConnectionId() !== cid) {
+                return callback({ message: 'Connection Switched', status: -1 });
+            }
+
             changeset = changeset.update({ id: changeset_id });
 
             // Upload the changeset..
@@ -366,12 +387,16 @@ export default {
                 callback(null, changeset);
             }, 2500);
 
-            // Still attempt to close changeset, but ignore response because #2667
-            oauth.xhr({
-                method: 'PUT',
-                path: '/api/0.6/changeset/' + changeset.id + '/close',
-                options: { header: { 'Content-Type': 'text/xml' } }
-            }, function() { return true; });
+            // At this point, we don't really care if the connection was switched..
+            // Only try to close the changeset if we're still talking to the same server.
+            if (that.getConnectionId() === cid) {
+                // Still attempt to close changeset, but ignore response because #2667
+                oauth.xhr({
+                    method: 'PUT',
+                    path: '/api/0.6/changeset/' + changeset.id + '/close',
+                    options: { header: { 'Content-Type': 'text/xml' } }
+                }, function() { return true; });
+            }
         }
     },
 
@@ -382,8 +407,16 @@ export default {
             return;
         }
 
+        var that = this;
+        var cid = connectionId;
+
         function done(err, user_details) {
-            if (err) return callback(err);
+            if (err) {
+                return callback(err);
+            }
+            if (that.getConnectionId() !== cid) {
+                return callback({ message: 'Connection Switched', status: -1 });
+            }
 
             var u = user_details.getElementsByTagName('user')[0],
                 img = u.getElementsByTagName('img'),
@@ -420,27 +453,36 @@ export default {
             return;
         }
 
+        var that = this;
+        var cid = connectionId;
+
         this.userDetails(function(err, user) {
             if (err) {
-                callback(err);
-                return;
+                return callback(err);
+            }
+            if (that.getConnectionId() !== cid) {
+                return callback({ message: 'Connection Switched', status: -1 });
             }
 
             function done(err, changesets) {
                 if (err) {
-                    callback(err);
-                } else {
-                    userChangesets = Array.prototype.map.call(
-                        changesets.getElementsByTagName('changeset'),
-                        function (changeset) {
-                            return { tags: getTags(changeset) };
-                        }
-                    ).filter(function (changeset) {
-                        var comment = changeset.tags.comment;
-                        return comment && comment !== '';
-                    });
-                    callback(undefined, userChangesets);
+                    return callback(err);
                 }
+                if (that.getConnectionId() !== cid) {
+                    return callback({ message: 'Connection Switched', status: -1 });
+                }
+
+                userChangesets = Array.prototype.map.call(
+                    changesets.getElementsByTagName('changeset'),
+                    function (changeset) {
+                        return { tags: getTags(changeset) };
+                    }
+                ).filter(function (changeset) {
+                    var comment = changeset.tags.comment;
+                    return comment && comment !== '';
+                });
+
+                callback(undefined, userChangesets);
             }
 
             oauth.xhr({ method: 'GET', path: '/api/0.6/changesets?user=' + user.id }, done);
@@ -449,7 +491,14 @@ export default {
 
 
     status: function(callback) {
+        var that = this;
+        var cid = connectionId;
+
         function done(xml) {
+            if (that.getConnectionId() !== cid) {
+                return callback({ message: 'Connection Switched', status: -1 }, 'connectionSwitched');
+            }
+
             // update blacklists
             var elements = xml.getElementsByTagName('blacklist'),
                 regexes = [];
@@ -568,9 +617,9 @@ export default {
             done: authDone
         }, options));
 
-        dispatch.call('change');
         this.reset();
         this.userChangesets(function() {});  // eagerly load user details/changesets
+        dispatch.call('change');
         return this;
     },
 
@@ -599,10 +648,19 @@ export default {
 
     authenticate: function(callback) {
         var that = this;
+        var cid = connectionId;
         userChangesets = undefined;
         userDetails = undefined;
 
         function done(err, res) {
+            if (err) {
+                if (callback) callback(err);
+                return;
+            }
+            if (that.getConnectionId() !== cid) {
+                if (callback) callback({ message: 'Connection Switched', status: -1 });
+                return;
+            }
             rateLimitError = undefined;
             dispatch.call('change');
             if (callback) callback(err, res);

modules/ui/source_switch.js

@@ -13,22 +13,28 @@ export function uiSourceSwitch(context) {
 
     function click() {
         d3_event.preventDefault();
+
+        var osm = context.connection();
+        if (!osm) return;
+
         if (context.inIntro()) return;
 
         if (context.history().hasChanges() &&
             !window.confirm(t('source_switch.lose_changes'))) return;
 
-        var live = d3_select(this)
+        var isLive = d3_select(this)
             .classed('live');
 
-        context.history().clearSaved();
-        context.connection().switch(live ? keys[1] : keys[0]);
+        isLive = !isLive;
         context.enter(modeBrowse(context));
-        context.flush();
+        context.history().clearSaved();          // remove saved history
+        context.flush();                         // remove stored data
 
         d3_select(this)
-            .text(live ? t('source_switch.dev') : t('source_switch.live'))
-            .classed('live', !live);
+            .text(isLive ? t('source_switch.live') : t('source_switch.dev'))
+            .classed('live', isLive);
+
+        osm.switch(isLive ? keys[0] : keys[1]);  // switch connection (warning: dispatches 'change' event)
     }
 
     var sourceSwitch = function(selection) {

modules/ui/status.js

@@ -16,7 +16,12 @@ export function uiStatus(context) {
                 selection.html('');
 
                 if (err) {
-                    if (apiStatus === 'rateLimited') {
+                    if (apiStatus === 'connectionSwitched') {
+                        // if the connection was just switched, we can't rely on
+                        // the status (we're getting the status of the previous api)
+                        return;
+
+                    } else if (apiStatus === 'rateLimited') {
                         selection
                             .text(t('status.rateLimit'))
                             .append('a')

test/spec/services/osm.js

@@ -52,6 +52,21 @@ describe('iD.serviceOsm', function () {
         expect(connection.changesetURL(2)).to.match(/^https:/);
     });
 
+    describe('#getConnectionId', function() {
+        it('changes the connection id every time connection is reset', function() {
+            var cid1 = connection.getConnectionId();
+            connection.reset();
+            var cid2 = connection.getConnectionId();
+            expect(cid2).to.be.above(cid1);
+        });
+        it('changes the connection id every time connection is switched', function() {
+            var cid1 = connection.getConnectionId();
+            connection.switch({ urlroot: 'https://api06.dev.openstreetmap.org' });
+            var cid2 = connection.getConnectionId();
+            expect(cid2).to.be.above(cid1);
+        });
+    });
+
     describe('#changesetURL', function() {
         it('provides a changeset url', function() {
             expect(connection.changesetURL(2)).to.eql('http://www.openstreetmap.org/changeset/2');