From 437893ebb8b31e033e6544f0cc343725c4d6a0fd Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 4 Jan 2018 23:27:00 -0500 Subject: [PATCH] Don't reenter putChangeset, allow reuse of open changeset id We are going to start trying an opportunistic save, then only start the conflict resolution stuff if the server returns a 409. Reusing an already open changeset makes sense in this situation. --- modules/services/osm.js | 230 +++++++++++++++++++++------------------- 1 file changed, 123 insertions(+), 107 deletions(-) diff --git a/modules/services/osm.js b/modules/services/osm.js index af96685a3..d284d72f2 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -25,25 +25,26 @@ import { import { utilRebind, utilIdleWorker } from '../util'; -var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'), - urlroot = 'https://www.openstreetmap.org', - blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*'], - inflight = {}, - loadedTiles = {}, - entityCache = {}, - connectionId = 1, - tileZoom = 16, - oauth = osmAuth({ - url: urlroot, - oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT', - oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL', - loading: authLoading, - done: authDone - }), - rateLimitError, - userChangesets, - userDetails, - off; +var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'); +var urlroot = 'https://www.openstreetmap.org'; +var oauth = osmAuth({ + url: urlroot, + oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT', + oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL', + loading: authLoading, + done: authDone +}); + +var _blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*']; +var _tiles = { loaded: {}, inflight: {} }; +var _changeset = {}; +var _entityCache = {}; +var _connectionID = 1; +var _tileZoom = 16; +var _rateLimitError; +var _userChangesets; +var _userDetails; +var _off; function authLoading() { @@ -64,15 +65,15 @@ function abortRequest(i) { function getLoc(attrs) { - var lon = attrs.lon && attrs.lon.value, - lat = attrs.lat && attrs.lat.value; + var lon = attrs.lon && attrs.lon.value; + var lat = attrs.lat && attrs.lat.value; return [parseFloat(lon), parseFloat(lat)]; } function getNodes(obj) { - var elems = obj.getElementsByTagName('nd'), - nodes = new Array(elems.length); + var elems = obj.getElementsByTagName('nd'); + var nodes = new Array(elems.length); for (var i = 0, l = elems.length; i < l; i++) { nodes[i] = 'n' + elems[i].attributes.ref.value; } @@ -81,8 +82,8 @@ function getNodes(obj) { function getTags(obj) { - var elems = obj.getElementsByTagName('tag'), - tags = {}; + var elems = obj.getElementsByTagName('tag'); + var tags = {}; for (var i = 0, l = elems.length; i < l; i++) { var attrs = elems[i].attributes; tags[attrs.k.value] = attrs.v.value; @@ -93,8 +94,8 @@ function getTags(obj) { function getMembers(obj) { - var elems = obj.getElementsByTagName('member'), - members = new Array(elems.length); + var elems = obj.getElementsByTagName('member'); + var members = new Array(elems.length); for (var i = 0, l = elems.length; i < l; i++) { var attrs = elems[i].attributes; members[i] = { @@ -164,14 +165,14 @@ function parse(xml, callback, options) { options = _extend({ cache: true }, options); if (!xml || !xml.childNodes) return; - var root = xml.childNodes[0], - children = root.childNodes; + var root = xml.childNodes[0]; + var children = root.childNodes; function parseChild(child) { var parser = parsers[child.nodeName]; if (parser) { var uid = osmEntity.id.fromOSM(child.nodeName, child.attributes.id.value); - if (options.cache && entityCache[uid]) { + if (options.cache && _entityCache[uid]) { return null; } return parser(child, uid); @@ -190,20 +191,21 @@ export default { reset: function() { - connectionId++; - userChangesets = undefined; - userDetails = undefined; - rateLimitError = undefined; - _forEach(inflight, abortRequest); - entityCache = {}; - loadedTiles = {}; - inflight = {}; + _connectionID++; + _userChangesets = undefined; + _userDetails = undefined; + _rateLimitError = undefined; + _forEach(_tiles.inflight, abortRequest); + if (_changeset.inflight) abortRequest(_changeset.inflight); + _tiles = { loaded: {}, inflight: {} }; + _changeset = {}; + _entityCache = {}; return this; }, getConnectionId: function() { - return connectionId; + return _connectionID; }, @@ -239,7 +241,7 @@ export default { loadFromAPI: function(path, callback, options) { options = _extend({ cache: true }, options); var that = this; - var cid = connectionId; + var cid = _connectionID; function done(err, xml) { if (that.getConnectionId() !== cid) { @@ -260,9 +262,9 @@ export default { } else { // 509 Bandwidth Limit Exceeded, 429 Too Many Requests // Set the rateLimitError flag and trigger a warning.. - if (!isAuthenticated && !rateLimitError && err && + if (!isAuthenticated && !_rateLimitError && err && (err.status === 509 || err.status === 429)) { - rateLimitError = err; + _rateLimitError = err; dispatch.call('change'); } @@ -271,7 +273,7 @@ export default { parse(xml, function (entities) { if (options.cache) { for (var i in entities) { - entityCache[entities[i].id] = true; + _entityCache[entities[i].id] = true; } } callback(null, entities); @@ -290,9 +292,9 @@ export default { loadEntity: function(id, callback) { - var type = osmEntity.id.type(id), - osmID = osmEntity.id.toOSM(id), - options = { cache: false }; + var type = osmEntity.id.type(id); + var osmID = osmEntity.id.toOSM(id); + var options = { cache: false }; this.loadFromAPI( '/api/0.6/' + type + '/' + osmID + (type !== 'node' ? '/full' : ''), @@ -305,9 +307,9 @@ export default { loadEntityVersion: function(id, version, callback) { - var type = osmEntity.id.type(id), - osmID = osmEntity.id.toOSM(id), - options = { cache: false }; + var type = osmEntity.id.type(id); + var osmID = osmEntity.id.toOSM(id); + var options = { cache: false }; this.loadFromAPI( '/api/0.6/' + type + '/' + osmID + '/' + version, @@ -323,9 +325,9 @@ export default { var that = this; _forEach(_groupBy(_uniq(ids), osmEntity.id.type), function(v, k) { - var type = k + 's', - osmIDs = _map(v, osmEntity.id.toOSM), - options = { cache: false }; + var type = k + 's'; + var osmIDs = _map(v, osmEntity.id.toOSM); + var options = { cache: false }; _forEach(_chunk(osmIDs, 150), function(arr) { that.loadFromAPI( @@ -346,19 +348,28 @@ export default { putChangeset: function(changeset, changes, callback) { + if (_changeset.inflight) { + return callback({ message: 'Changeset already inflight', status: -2 }); + } + var that = this; - var cid = connectionId; + var cid = _connectionID; - // Create the changeset.. - oauth.xhr({ - method: 'PUT', - path: '/api/0.6/changeset/create', - options: { header: { 'Content-Type': 'text/xml' } }, - content: JXON.stringify(changeset.asJXON()) - }, createdChangeset); + if (_changeset.open) { // reuse existing open changeset.. + createdChangeset(null, _changeset.open); + } else { // open a new changeset.. + _changeset.inflight = oauth.xhr({ + method: 'PUT', + path: '/api/0.6/changeset/create', + options: { header: { 'Content-Type': 'text/xml' } }, + content: JXON.stringify(changeset.asJXON()) + }, createdChangeset); + } - function createdChangeset(err, changeset_id) { + function createdChangeset(err, changesetID) { + _changeset.inflight = null; + if (err) { return callback(err); } @@ -366,12 +377,13 @@ export default { return callback({ message: 'Connection Switched', status: -1 }); } - changeset = changeset.update({ id: changeset_id }); + _changeset.open = changesetID; + changeset = changeset.update({ id: changesetID }); // Upload the changeset.. - oauth.xhr({ + _changeset.inflight = oauth.xhr({ method: 'POST', - path: '/api/0.6/changeset/' + changeset_id + '/upload', + path: '/api/0.6/changeset/' + changesetID + '/upload', options: { header: { 'Content-Type': 'text/xml' } }, content: JXON.stringify(changeset.osmChangeJXON(changes)) }, uploadedChangeset); @@ -379,6 +391,8 @@ export default { function uploadedChangeset(err) { + _changeset.inflight = null; + if (err) return callback(err); // Upload was successful, safe to call the callback. @@ -387,6 +401,8 @@ export default { callback(null, changeset); }, 2500); + _changeset.open = null; + // 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) { @@ -402,13 +418,13 @@ export default { userDetails: function(callback) { - if (userDetails) { - callback(undefined, userDetails); + if (_userDetails) { + callback(undefined, _userDetails); return; } var that = this; - var cid = connectionId; + var cid = _connectionID; function done(err, user_details) { if (err) { @@ -418,29 +434,29 @@ export default { return callback({ message: 'Connection Switched', status: -1 }); } - var u = user_details.getElementsByTagName('user')[0], - img = u.getElementsByTagName('img'), - image_url = ''; + var u = user_details.getElementsByTagName('user')[0]; + var img = u.getElementsByTagName('img'); + var image_url = ''; if (img && img[0] && img[0].getAttribute('href')) { image_url = img[0].getAttribute('href'); } - var changesets = u.getElementsByTagName('changesets'), - changesets_count = 0; + var changesets = u.getElementsByTagName('changesets'); + var changesets_count = 0; if (changesets && changesets[0] && changesets[0].getAttribute('count')) { changesets_count = changesets[0].getAttribute('count'); } - userDetails = { + _userDetails = { id: u.attributes.id.value, display_name: u.attributes.display_name.value, image_url: image_url, changesets_count: changesets_count }; - callback(undefined, userDetails); + callback(undefined, _userDetails); } oauth.xhr({ method: 'GET', path: '/api/0.6/user/details' }, done); @@ -448,13 +464,13 @@ export default { userChangesets: function(callback) { - if (userChangesets) { - callback(undefined, userChangesets); + if (_userChangesets) { + callback(undefined, _userChangesets); return; } var that = this; - var cid = connectionId; + var cid = _connectionID; this.userDetails(function(err, user) { if (err) { @@ -472,7 +488,7 @@ export default { return callback({ message: 'Connection Switched', status: -1 }); } - userChangesets = Array.prototype.map.call( + _userChangesets = Array.prototype.map.call( changesets.getElementsByTagName('changeset'), function (changeset) { return { tags: getTags(changeset) }; @@ -482,7 +498,7 @@ export default { return comment && comment !== ''; }); - callback(undefined, userChangesets); + callback(undefined, _userChangesets); } oauth.xhr({ method: 'GET', path: '/api/0.6/changesets?user=' + user.id }, done); @@ -492,7 +508,7 @@ export default { status: function(callback) { var that = this; - var cid = connectionId; + var cid = _connectionID; function done(xml) { if (that.getConnectionId() !== cid) { @@ -509,12 +525,12 @@ export default { } } if (regexes.length) { - blacklists = regexes; + _blacklists = regexes; } - if (rateLimitError) { - callback(rateLimitError, 'rateLimited'); + if (_rateLimitError) { + callback(_rateLimitError, 'rateLimited'); } else { var apiStatus = xml.getElementsByTagName('status'); var val = apiStatus[0].getAttribute('api'); @@ -530,31 +546,31 @@ export default { imageryBlacklists: function() { - return blacklists; + return _blacklists; }, tileZoom: function(_) { - if (!arguments.length) return tileZoom; - tileZoom = _; + if (!arguments.length) return _tileZoom; + _tileZoom = _; return this; }, loadTiles: function(projection, dimensions, callback) { - if (off) return; + if (_off) return; var that = this; var s = projection.scale() * 2 * Math.PI; var z = Math.max(Math.log(s) / Math.log(2) - 8, 0); - var ts = 256 * Math.pow(2, z - tileZoom); + var ts = 256 * Math.pow(2, z - _tileZoom); var origin = [ s / 2 - projection.translate()[0], s / 2 - projection.translate()[1] ]; var tiles = d3_geoTile() - .scaleExtent([tileZoom, tileZoom]) + .scaleExtent([_tileZoom, _tileZoom]) .scale(s) .size(dimensions) .translate(projection.translate())() @@ -570,36 +586,36 @@ export default { }; }); - _filter(inflight, function(v, i) { + _filter(_tiles.inflight, function(v, i) { var wanted = _find(tiles, function(tile) { return i === tile.id; }); - if (!wanted) delete inflight[i]; + if (!wanted) delete _tiles.inflight[i]; return !wanted; }).map(abortRequest); tiles.forEach(function(tile) { var id = tile.id; - if (loadedTiles[id] || inflight[id]) return; + if (_tiles.loaded[id] || _tiles.inflight[id]) return; - if (_isEmpty(inflight)) { + if (_isEmpty(_tiles.inflight)) { dispatch.call('loading'); } - inflight[id] = that.loadFromAPI( + _tiles.inflight[id] = that.loadFromAPI( '/api/0.6/map?bbox=' + tile.extent.toParam(), function(err, parsed) { - delete inflight[id]; + delete _tiles.inflight[id]; if (!err) { - loadedTiles[id] = true; + _tiles.loaded[id] = true; } if (callback) { callback(err, _extend({ data: parsed }, tile)); } - if (_isEmpty(inflight)) { + if (_isEmpty(_tiles.inflight)) { dispatch.call('loaded'); } } @@ -625,21 +641,21 @@ export default { toggle: function(_) { - off = !_; + _off = !_; return this; }, loadedTiles: function(_) { - if (!arguments.length) return loadedTiles; - loadedTiles = _; + if (!arguments.length) return _tiles.loaded; + _tiles.loaded = _; return this; }, logout: function() { - userChangesets = undefined; - userDetails = undefined; + _userChangesets = undefined; + _userDetails = undefined; oauth.logout(); dispatch.call('change'); return this; @@ -648,9 +664,9 @@ export default { authenticate: function(callback) { var that = this; - var cid = connectionId; - userChangesets = undefined; - userDetails = undefined; + var cid = _connectionID; + _userChangesets = undefined; + _userDetails = undefined; function done(err, res) { if (err) { @@ -661,10 +677,10 @@ export default { if (callback) callback({ message: 'Connection Switched', status: -1 }); return; } - rateLimitError = undefined; + _rateLimitError = undefined; dispatch.call('change'); if (callback) callback(err, res); - that.userChangesets(function() {}); // eagerly load user details/changesets + that._userChangesets(function() {}); // eagerly load user details/changesets } return oauth.authenticate(done);