diff --git a/modules/modes/save.js b/modules/modes/save.js index a2174c56b..27a70ebca 100644 --- a/modules/modes/save.js +++ b/modules/modes/save.js @@ -2,6 +2,17 @@ import * as d3 from 'd3'; import _ from 'lodash'; import { t } from '../util/locale'; +import { JXON } from '../util/jxon'; + +import { + actionDiscardTags, + actionMergeRemoteChanges, + actionNoop, + actionRevert +} from '../actions'; + +import { coreGraph } from '../core'; +import { modeBrowse } from './index'; import { uiConflicts, @@ -9,23 +20,13 @@ import { uiCommit, uiLoading, uiSuccess -} from '../ui/index'; - -import { - actionDiscardTags, - actionMergeRemoteChanges, - actionNoop, - actionRevert -} from '../actions/index'; +} from '../ui'; import { utilDisplayName, utilDisplayType -} from '../util/index'; +} from '../util'; -import { modeBrowse } from './index'; -import { coreGraph } from '../core/index'; -import { JXON } from '../util/jxon'; export function modeSave(context) { @@ -33,7 +34,7 @@ export function modeSave(context) { id: 'save' }; - var ui = uiCommit(context) + var commit = uiCommit(context) .on('cancel', cancel) .on('save', save); @@ -65,7 +66,7 @@ export function modeSave(context) { if (toCheck.length) { context.connection().loadMultiple(toLoad, loaded); } else { - finalize(); + upload(); } @@ -187,11 +188,11 @@ export function modeSave(context) { }); }); - finalize(); + upload(); } - function finalize() { + function upload() { if (conflicts.length) { conflicts.sort(function(a,b) { return b.id.localeCompare(a.id); }); showConflicts(); @@ -200,29 +201,7 @@ export function modeSave(context) { } else { var changes = history.changes(actionDiscardTags(history.difference())); if (changes.modified.length || changes.created.length || changes.deleted.length) { - context.connection().putChangeset( - changes, - context.version, - changeset.tags.comment, - history.imageryUsed(), - function(err, changeset_id) { - if (err) { - errors.push({ - msg: err.responseText, - details: [ t('save.status_code', { code: err.status }) ] - }); - showErrors(); - } else { - history.clearSaved(); - success(changeset, changeset_id); - // Add delay to allow for postgres replication #1646 #2678 - window.setTimeout(function() { - d3.select('.inspector-wrap *').remove(); - loading.close(); - context.flush(); - }, 2500); - } - }); + context.connection().putChangeset(changeset, changes, uploadCallback); } else { // changes were insignificant or reverted by user d3.select('.inspector-wrap *').remove(); loading.close(); @@ -233,6 +212,26 @@ export function modeSave(context) { } + function uploadCallback(err, changeset) { + if (err) { + errors.push({ + msg: err.responseText, + details: [ t('save.status_code', { code: err.status }) ] + }); + showErrors(); + } else { + history.clearSaved(); + success(changeset); + // Add delay to allow for postgres replication #1646 #2678 + window.setTimeout(function() { + d3.select('.inspector-wrap *').remove(); + loading.close(); + context.flush(); + }, 2500); + } + } + + function showConflicts() { var selection = context.container() .select('#sidebar') @@ -244,7 +243,7 @@ export function modeSave(context) { selection.call(uiConflicts(context) .list(conflicts) .on('download', function() { - var data = JXON.stringify(context.connection().osmChangeJXON('CHANGEME', origChanges)), + var data = JXON.stringify(changeset.update({ id: 'CHANGEME' }).osmChangeJXON(origChanges)), win = window.open('data:text/xml,' + encodeURIComponent(data), '_blank'); win.focus(); }) @@ -340,13 +339,11 @@ export function modeSave(context) { } - function success(changeset, changeset_id) { + function success(changeset) { + commit.reset(); context.enter(modeBrowse(context) .sidebar(uiSuccess(context) - .changeset({ - id: changeset_id, - comment: changeset.tags.comment - }) + .changeset(changeset) .on('cancel', function() { context.ui().sidebar.hide(); }) @@ -357,7 +354,7 @@ export function modeSave(context) { mode.enter = function() { function done() { - context.ui().sidebar.show(ui); + context.ui().sidebar.show(commit); } context.container().selectAll('#content') diff --git a/modules/osm/changeset.js b/modules/osm/changeset.js index b2ed36f11..26e940e24 100644 --- a/modules/osm/changeset.js +++ b/modules/osm/changeset.js @@ -46,6 +46,90 @@ _.extend(osmChangeset.prototype, { }, + // Generate [osmChange](http://wiki.openstreetmap.org/wiki/OsmChange) + // XML. Returns a string. + osmChangeJXON: function(changes) { + var changeset_id = this.id; + + function nest(x, order) { + var groups = {}; + for (var i = 0; i < x.length; i++) { + var tagName = Object.keys(x[i])[0]; + if (!groups[tagName]) groups[tagName] = []; + groups[tagName].push(x[i][tagName]); + } + var ordered = {}; + order.forEach(function(o) { + if (groups[o]) ordered[o] = groups[o]; + }); + return ordered; + } + + + // sort relations in a changeset by dependencies + function sort(changes) { + + // find a referenced relation in the current changeset + function resolve(item) { + return _.find(relations, function(relation) { + return item.keyAttributes.type === 'relation' + && item.keyAttributes.ref === relation['@id']; + }); + } + + // a new item is an item that has not been already processed + function isNew(item) { + return !sorted[ item['@id'] ] && !_.find(processing, function(proc) { + return proc['@id'] === item['@id']; + }); + } + + var processing = [], + sorted = {}, + relations = changes.relation; + + if (!relations) return changes; + + for (var i = 0; i < relations.length; i++) { + var relation = relations[i]; + + // skip relation if already sorted + if (!sorted[relation['@id']]) { + processing.push(relation); + } + + while (processing.length > 0) { + var next = processing[0], + deps = _.filter(_.compact(next.member.map(resolve)), isNew); + if (deps.length === 0) { + sorted[next['@id']] = next; + processing.shift(); + } else { + processing = deps.concat(processing); + } + } + } + + changes.relation = _.values(sorted); + return changes; + } + + function rep(entity) { + return entity.asJXON(changeset_id); + } + + return { + osmChange: { + '@version': 0.6, + '@generator': 'iD', + 'create': sort(nest(changes.created.map(rep), ['node', 'way', 'relation'])), + 'modify': nest(changes.modified.map(rep), ['node', 'way', 'relation']), + 'delete': _.extend(nest(changes.deleted.map(rep), ['relation', 'way', 'node']), { '@if-unused': true }) + } + }; + }, + + asGeoJSON: function() { return {}; } diff --git a/modules/services/osm.js b/modules/services/osm.js index 16077cdf3..de1d977e6 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -3,17 +3,15 @@ import _ from 'lodash'; import osmAuth from 'osm-auth'; import { JXON } from '../util/jxon'; import { d3geoTile } from '../lib/d3.geo.tile'; -import { geoExtent } from '../geo/index'; +import { geoExtent } from '../geo'; import { - osmChangeset, osmEntity, osmNode, osmRelation, osmWay -} from '../osm/index'; +} from '../osm'; -import { utilDetect } from '../util/detect'; -import { utilRebind } from '../util/rebind'; +import { utilRebind } from '../util'; var dispatch = d3.dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'), @@ -288,132 +286,47 @@ export default { }, - // Generate [osmChange](http://wiki.openstreetmap.org/wiki/OsmChange) - // XML. Returns a string. - osmChangeJXON: function(changeset_id, changes) { - function nest(x, order) { - var groups = {}; - for (var i = 0; i < x.length; i++) { - var tagName = Object.keys(x[i])[0]; - if (!groups[tagName]) groups[tagName] = []; - groups[tagName].push(x[i][tagName]); - } - var ordered = {}; - order.forEach(function(o) { - if (groups[o]) ordered[o] = groups[o]; - }); - return ordered; - } - // sort relations in a changeset by dependencies - function sort(changes) { - // find a referenced relation in the current changeset - function resolve(item){ - return _.find(relations, function(relation) { - return item.keyAttributes.type === 'relation' - && item.keyAttributes.ref === relation['@id']; - }); - } - // a new item is an item that has not been already processed - function isNew(item) { - return !sorted[ item['@id'] ] && !_.find(processing, function(proc){ - return proc['@id'] === item['@id']; - }); - } - var processing = [], - sorted = {}, - relations = changes.relation; - - if (!relations) return changes; - - for (var i = 0; i < relations.length; i++) { - - var relation = relations[i]; - - // skip relation if already sorted - if ( !sorted[relation['@id']] ) { - processing.push( relation ); - } - - while ( processing.length > 0 ){ - var next = processing[0], - deps = _.filter( _.compact(next.member.map(resolve)), isNew); - - if ( deps.length === 0 ){ - sorted[ next['@id'] ] = next; - processing.shift(); - } else { - processing = deps.concat( processing ); - } - } - - } - - changes.relation = _.values(sorted); - return changes; - } - - function rep(entity) { - return entity.asJXON(changeset_id); - } - - return { - osmChange: { - '@version': 0.6, - '@generator': 'iD', - 'create': sort(nest(changes.created.map(rep), ['node', 'way', 'relation'])), - 'modify': nest(changes.modified.map(rep), ['node', 'way', 'relation']), - 'delete': _.extend(nest(changes.deleted.map(rep), ['relation', 'way', 'node']), {'@if-unused': true}) - } - }; - }, - - - changesetTags: function(version, comment, imageryUsed) { - var detected = utilDetect(), - tags = { - created_by: ('iD ' + version).substr(0, 255), - imagery_used: imageryUsed.join(';').substr(0, 255), - host: detected.host.substr(0, 255), - locale: detected.locale.substr(0, 255) - }; - - if (comment) { - tags.comment = comment.substr(0, 255); - } - - return tags; - }, - - - putChangeset: function(changes, version, comment, imageryUsed, callback) { - var that = this, - changeset = new osmChangeset({ tags: this.changesetTags(version, comment, imageryUsed) }); + putChangeset: function(changeset, changes, callback) { + // Create the changeset.. oauth.xhr({ - method: 'PUT', - path: '/api/0.6/changeset/create', + 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) { + if (err) return callback(err); + changeset = changeset.update({ id: changeset_id }); + + // Upload the changeset.. + oauth.xhr({ + method: 'POST', + path: '/api/0.6/changeset/' + changeset_id + '/upload', options: { header: { 'Content-Type': 'text/xml' } }, - content: JXON.stringify(changeset.asJXON()) - }, function(err, changeset_id) { - if (err) return callback(err); - oauth.xhr({ - method: 'POST', - path: '/api/0.6/changeset/' + changeset_id + '/upload', - options: { header: { 'Content-Type': 'text/xml' } }, - content: JXON.stringify(that.osmChangeJXON(changeset_id, changes)) - }, function(err) { - if (err) return callback(err); - // POST was successful, safe to call the callback. - // Still attempt to close changeset, but ignore response because #2667 - // Add delay to allow for postgres replication #1646 #2678 - window.setTimeout(function() { callback(null, changeset_id); }, 2500); - oauth.xhr({ - method: 'PUT', - path: '/api/0.6/changeset/' + changeset_id + '/close', - options: { header: { 'Content-Type': 'text/xml' } } - }, function() { return true; }); - }); - }); + content: JXON.stringify(changeset.osmChangeJXON(changes)) + }, uploadedChangeset); + } + + + function uploadedChangeset(err) { + if (err) return callback(err); + + // Upload was successful, safe to call the callback. + // Add delay to allow for postgres replication #1646 #2678 + window.setTimeout(function() { + 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; }); + } }, diff --git a/modules/ui/commit.js b/modules/ui/commit.js index c0627a720..c6e41f835 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -214,7 +214,6 @@ export function uiCommit(context) { }) .on('click.save', function() { dispatch.call('save', this, changeset); - changeset = null; }); saveButton @@ -383,11 +382,11 @@ export function uiCommit(context) { var tags = _.clone(changeset.tags); _.forEach(changed, function(v, k) { - k = k.trim(); + k = k.trim().substr(0, 255); if (readOnlyTags.indexOf(k) !== -1) return; if (k !== '' && v !== undefined) { - tags[k] = v.trim(); + tags[k] = v.trim().substr(0, 255); } else { delete tags[k]; } @@ -402,5 +401,11 @@ export function uiCommit(context) { } + + commit.reset = function() { + changeset = null; + }; + + return utilRebind(commit, dispatch, 'on'); } diff --git a/modules/ui/success.js b/modules/ui/success.js index f182119aa..989363e3c 100644 --- a/modules/ui/success.js +++ b/modules/ui/success.js @@ -11,9 +11,6 @@ export function uiSuccess(context) { function success(selection) { - var message = (changeset.comment || t('success.edited_osm')).substring(0, 130) + - ' ' + context.connection().changesetURL(changeset.id); - var header = selection .append('div') .attr('class', 'header fillL'); @@ -55,6 +52,10 @@ export function uiSuccess(context) { .attr('href', changesetURL) .text(t('success.view_on_osm')); + + var message = (changeset.tags.comment || t('success.edited_osm')).substring(0, 130) + + ' ' + context.connection().changesetURL(changeset.id); + var sharing = { facebook: 'https://facebook.com/sharer/sharer.php?u=' + encodeURIComponent(changesetURL), twitter: 'https://twitter.com/intent/tweet?source=webclient&text=' + encodeURIComponent(message), diff --git a/test/spec/osm/changeset.js b/test/spec/osm/changeset.js index 566704218..16df504c0 100644 --- a/test/spec/osm/changeset.js +++ b/test/spec/osm/changeset.js @@ -12,6 +12,7 @@ describe('iD.osmChangeset', function () { expect(iD.osmChangeset({tags: {foo: 'bar'}}).tags).to.eql({foo: 'bar'}); }); + describe('#asJXON', function () { it('converts a node to jxon', function() { var node = iD.osmChangeset({tags: {'comment': 'hello'}}); @@ -26,4 +27,100 @@ describe('iD.osmChangeset', function () { }); }); }); + + + describe('#osmChangeJXON', function() { + it('converts change data to JXON', function() { + var changeset = iD.osmChangeset(), + jxon = changeset.osmChangeJXON({ created: [], modified: [], deleted: [] }); + + expect(jxon).to.eql({ + osmChange: { + '@version': 0.6, + '@generator': 'iD', + 'create': {}, + 'modify': {}, + 'delete': { '@if-unused': true } + } + }); + }); + + it('includes creations ordered by nodes, ways, relations', function() { + var n = iD.osmNode({ loc: [0, 0] }), + w = iD.osmWay(), + r = iD.osmRelation(), + c = iD.osmChangeset({ id: '1234' }), + changes = { created: [r, w, n], modified: [], deleted: [] }, + jxon = c.osmChangeJXON(changes); + + expect(d3.entries(jxon.osmChange.create)).to.eql([ + { key: 'node', value: [n.asJXON('1234').node] }, + { key: 'way', value: [w.asJXON('1234').way] }, + { key: 'relation', value: [r.asJXON('1234').relation] } + ]); + }); + + it('includes creations ordered by dependencies', function() { + var n = iD.osmNode({ loc: [0, 0] }), + w = iD.osmWay({nodes: [n.id]}), + r1 = iD.osmRelation({ members: [{ id: w.id, type: 'way' }] }), + r2 = iD.osmRelation({ members: [{ id: r1.id, type: 'relation' }] }), + c = iD.osmChangeset({ id: '1234' }), + changes = { created: [r2, r1, w, n], modified: [], deleted: [] }, + jxon = c.osmChangeJXON(changes); + + expect(d3.entries(jxon.osmChange.create)).to.eql([ + { key: 'node', value: [n.asJXON('1234').node] }, + { key: 'way', value: [w.asJXON('1234').way] }, + { key: 'relation', value: [r1.asJXON('1234').relation, r2.asJXON('1234').relation] }, + ]); + }); + + it('includes creations ignoring circular dependencies', function() { + var r1 = iD.osmRelation(), + r2 = iD.osmRelation(), + c = iD.osmChangeset({ id: '1234' }), + changes, jxon; + r1.addMember({ id: r2.id, type: 'relation' }); + r2.addMember({ id: r1.id, type: 'relation' }); + changes = { created: [r1,r2], modified: [], deleted: [] }; + jxon = c.osmChangeJXON(changes); + + expect(d3.entries(jxon.osmChange.create)).to.eql([ + { key: 'relation', value: [r1.asJXON('1234').relation, r2.asJXON('1234').relation] }, + ]); + }); + + it('includes modifications', function() { + var n = iD.osmNode({ loc: [0, 0] }), + w = iD.osmWay(), + r = iD.osmRelation(), + c = iD.osmChangeset({ id: '1234' }), + changes = { created: [], modified: [r, w, n], deleted: [] }, + jxon = c.osmChangeJXON(changes); + + expect(jxon.osmChange.modify).to.eql({ + node: [n.asJXON('1234').node], + way: [w.asJXON('1234').way], + relation: [r.asJXON('1234').relation] + }); + }); + + it('includes deletions ordered by relations, ways, nodes', function() { + var n = iD.osmNode({ loc: [0, 0] }), + w = iD.osmWay(), + r = iD.osmRelation(), + c = iD.osmChangeset({ id: '1234' }), + changes = { created: [], modified: [], deleted: [n, w, r] }, + jxon = c.osmChangeJXON(changes); + + expect(d3.entries(jxon.osmChange.delete)).to.eql([ + { key: 'relation', value: [r.asJXON('1234').relation] }, + { key: 'way', value: [w.asJXON('1234').way] }, + { key: 'node', value: [n.asJXON('1234').node] }, + { key: '@if-unused', value: true } + ]); + }); + }); + }); diff --git a/test/spec/services/osm.js b/test/spec/services/osm.js index bded77c43..4644b310b 100644 --- a/test/spec/services/osm.js +++ b/test/spec/services/osm.js @@ -333,95 +333,6 @@ describe('iD.serviceOsm', function () { }); - describe('#osmChangeJXON', function() { - it('converts change data to JXON', function() { - var jxon = connection.osmChangeJXON('1234', {created: [], modified: [], deleted: []}); - - expect(jxon).to.eql({ - osmChange: { - '@version': 0.6, - '@generator': 'iD', - 'create': {}, - 'modify': {}, - 'delete': {'@if-unused': true} - } - }); - }); - - it('includes creations ordered by nodes, ways, relations', function() { - var n = iD.Node({loc: [0, 0]}), - w = iD.Way(), - r = iD.Relation(), - changes = {created: [r, w, n], modified: [], deleted: []}, - jxon = connection.osmChangeJXON('1234', changes); - - expect(d3.entries(jxon.osmChange.create)).to.eql([ - {key: 'node', value: [n.asJXON('1234').node]}, - {key: 'way', value: [w.asJXON('1234').way]}, - {key: 'relation', value: [r.asJXON('1234').relation]} - ]); - }); - - it('includes creations ordered by dependencies', function() { - var n = iD.Node({loc: [0, 0]}), - w = iD.Way({nodes: [n.id]}), - r1 = iD.Relation({members: [{id: w.id, type: 'way'}]}), - r2 = iD.Relation({members: [{id: r1.id, type: 'relation'}]}), - changes = {created: [r2, r1, w, n], modified: [], deleted: []}, - jxon = connection.osmChangeJXON('1234', changes); - - expect(d3.entries(jxon.osmChange.create)).to.eql([ - {key: 'node', value: [n.asJXON('1234').node]}, - {key: 'way', value: [w.asJXON('1234').way]}, - {key: 'relation', value: [r1.asJXON('1234').relation, r2.asJXON('1234').relation]}, - ]); - }); - - it('includes creations ignoring circular dependencies', function() { - var r1 = iD.Relation(), - r2 = iD.Relation(), - changes, jxon; - r1.addMember({id: r2.id, type: 'relation'}); - r2.addMember({id: r1.id, type: 'relation'}); - changes = {created: [r1,r2], modified: [], deleted: []}; - jxon = connection.osmChangeJXON('1234', changes); - - expect(d3.entries(jxon.osmChange.create)).to.eql([ - {key: 'relation', value: [r1.asJXON('1234').relation, r2.asJXON('1234').relation]}, - ]); - }); - - it('includes modifications', function() { - var n = iD.Node({loc: [0, 0]}), - w = iD.Way(), - r = iD.Relation(), - changes = {created: [], modified: [r, w, n], deleted: []}, - jxon = connection.osmChangeJXON('1234', changes); - - expect(jxon.osmChange.modify).to.eql({ - node: [n.asJXON('1234').node], - way: [w.asJXON('1234').way], - relation: [r.asJXON('1234').relation] - }); - }); - - it('includes deletions ordered by relations, ways, nodes', function() { - var n = iD.Node({loc: [0, 0]}), - w = iD.Way(), - r = iD.Relation(), - changes = {created: [], modified: [], deleted: [n, w, r]}, - jxon = connection.osmChangeJXON('1234', changes); - - expect(d3.entries(jxon.osmChange.delete)).to.eql([ - {key: 'relation', value: [r.asJXON('1234').relation]}, - {key: 'way', value: [w.asJXON('1234').way]}, - {key: 'node', value: [n.asJXON('1234').node]}, - {key: '@if-unused', value: true} - ]); - }); - }); - - describe('#userChangesets', function() { var server, userDetailsFn; @@ -527,13 +438,6 @@ describe('iD.serviceOsm', function () { }); - describe('#changesetTags', function() { - it('omits comment when empty', function() { - expect(connection.changesetTags('2.0.0', '', [])).not.to.have.property('comment'); - }); - }); - - describe('API capabilities', function() { var server, capabilitiesXML = '' +