From f72fd961cc94f5525e9add4bd9d11c0c38feaf49 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 24 Apr 2019 10:52:28 -0400 Subject: [PATCH 01/13] Update to d3 ~5.9.2 --- package.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 6c74bc06c..c27643493 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "chai": "^4.1.0", "colors": "^1.1.2", "concat-files": "^0.1.1", - "d3": "4.13.0", + "d3": "~5.9.2", "ecstatic": "^4.1.1", "editor-layer-index": "github:osmlab/editor-layer-index#gh-pages", "eslint": "^5.16.0", @@ -99,9 +99,7 @@ }, "greenkeeper": { "label": "chore-greenkeeper", - "ignore": [ - "d3" - ] + "ignore": [] }, "engines": { "node": ">=6.0.0", From b6cd13508a74b1e0b4af019a8d688fcedb95a096 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 24 Apr 2019 16:23:33 -0400 Subject: [PATCH 02/13] Add abortcontroller-polyfill to support abortable fetch calls --- modules/id.js | 2 ++ package.json | 1 + 2 files changed, 3 insertions(+) diff --git a/modules/id.js b/modules/id.js index df93f321e..2c1394ea0 100644 --- a/modules/id.js +++ b/modules/id.js @@ -1,3 +1,5 @@ import 'browser-polyfills'; +import 'abortcontroller-polyfill/dist/polyfill-patch-fetch'; + import * as iD from './index'; window.iD = iD; diff --git a/package.json b/package.json index c27643493..158d3cdfa 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "@mapbox/togeojson": "0.16.0", "@mapbox/vector-tile": "^1.3.1", "@turf/bbox-clip": "^6.0.0", + "abortcontroller-polyfill": "~1.3.0", "alif-toolkit": "^1.2.5", "browser-polyfills": "~1.5.0", "diacritics": "1.3.0", From 48b1ddc92512f45f3cb34d93e89711aec8de37c0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 24 Apr 2019 16:24:36 -0400 Subject: [PATCH 03/13] Rename `fetch` to `fetchComboData` to not conflict with fetch API --- modules/ui/combobox.js | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/modules/ui/combobox.js b/modules/ui/combobox.js index 4f61df9d2..427f2eafd 100644 --- a/modules/ui/combobox.js +++ b/modules/ui/combobox.js @@ -1,12 +1,5 @@ -import { - dispatch as d3_dispatch -} from 'd3-dispatch'; - -import { - event as d3_event, - select as d3_select -} from 'd3-selection'; - +import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { event as d3_event, select as d3_select } from 'd3-selection'; import { utilGetSetValue, utilRebind, utilTriggerEvent } from '../util'; @@ -107,7 +100,7 @@ export function uiCombobox(context, klass) { var tOrig = _tDown; window.setTimeout(function() { if (tOrig !== _tDown) return; // exit if user double clicked - fetch('', function() { + fetchComboData('', function() { show(); render(); }); @@ -120,7 +113,7 @@ export function uiCombobox(context, klass) { function focus() { - fetch(''); // prefetch values (may warm taginfo cache) + fetchComboData(''); // prefetch values (may warm taginfo cache) } @@ -225,7 +218,7 @@ export function uiCombobox(context, klass) { // Called whenever the input value is changed (e.g. on typing) function change() { - fetch(value(), function() { + fetchComboData(value(), function() { _selected = null; var val = input.property('value'); @@ -310,7 +303,7 @@ export function uiCombobox(context, klass) { } - function fetch(v, cb) { + function fetchComboData(v, cb) { _cancelFetch = false; _fetcher.call(input, v, function(results) { From e6bc9d9e8f7368e6bd64951caa5c406478e66297 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 24 Apr 2019 16:25:25 -0400 Subject: [PATCH 04/13] Swap out d3-request, swap in d3-fetch --- ARCHITECTURE.md | 5 +- modules/core/context.js | 31 ++--- modules/presets/index.js | 20 +-- modules/renderer/background_source.js | 168 +++++++++++++------------- modules/services/improveOSM.js | 85 ++++++++----- modules/services/keepRight.js | 49 +++++--- modules/services/mapillary.js | 44 ++++--- modules/services/nominatim.js | 67 ++++++---- modules/services/openstreetcam.js | 44 ++++--- modules/services/osm.js | 27 +++-- modules/services/osm_wikibase.js | 20 ++- modules/services/taginfo.js | 21 +++- modules/services/vector_tile.js | 28 +++-- modules/services/wikidata.js | 86 ++++++------- modules/services/wikipedia.js | 86 ++++++++----- modules/svg/data.js | 26 ++-- modules/svg/defs.js | 13 +- 17 files changed, 477 insertions(+), 343 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e88be04cc..f7294379e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -11,9 +11,8 @@ rendering the map data as well as many sorts of general DOM manipulation tasks for which jQuery would often be used. Notable features of d3 that are used by iD include -[d3.request](https://github.com/d3/d3/blob/master/API.md#requests-d3-request), which is -used to make the API requests to download data from openstreetmap.org and save -changes; +[d3.fetch](https://github.com/d3/d3/blob/master/API.md#fetches-d3-fetch), which is +used to make the API requests to download data from openstreetmap.org and save changes; [d3.dispatch](https://github.com/d3/d3/blob/master/API.md#dispatches-d3-dispatch), which provides a callback-based [Observer pattern](http://en.wikipedia.org/wiki/Observer_pattern) between different diff --git a/modules/core/context.js b/modules/core/context.js index 08e9b02dc..9022c6351 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -1,7 +1,7 @@ import _debounce from 'lodash-es/debounce'; import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { select as d3_select } from 'd3-selection'; import { t, currentLocale, addTranslation, setLocale } from '../util/locale'; @@ -434,16 +434,16 @@ export function coreContext() { context.loadLocale = function(callback) { if (locale && locale !== 'en' && dataLocales.hasOwnProperty(locale)) { localePath = localePath || context.asset('locales/' + locale + '.json'); - d3_json(localePath, function(err, result) { - if (!err) { + d3_json(localePath) + .then(function(result) { addTranslation(locale, result[locale]); setLocale(locale); utilDetect(true); - } - if (callback) { - callback(err); - } - }); + if (callback) callback(); + }) + .catch(function(err) { + if (callback) callback(err); + }); } else { if (locale) { setLocale(locale); @@ -520,13 +520,16 @@ export function coreContext() { if (services.maprules && utilStringQs(window.location.hash).maprules) { var maprules = utilStringQs(window.location.hash).maprules; - d3_json(maprules, function (err, mapcss) { - if (err) return; - services.maprules.init(); - mapcss.forEach(function(mapcssSelector) { - return services.maprules.addRule(mapcssSelector); + d3_json(maprules) + .then(function(mapcss) { + services.maprules.init(); + mapcss.forEach(function(mapcssSelector) { + return services.maprules.addRule(mapcssSelector); + }); + }) + .catch(function() { + /* ignore */ }); - }); } map = rendererMap(context); diff --git a/modules/presets/index.js b/modules/presets/index.js index 35d77bcb6..a0611263b 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -1,5 +1,5 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { data } from '../../data/index'; import { presetCategory } from './category'; @@ -251,15 +251,17 @@ export function presetIndex(context) { all.fromExternal = function(external, done) { all.reset(); - d3_json(external, function(err, externalPresets) { - if (err) { + d3_json(external) + .then(function(externalPresets) { + all.build(data.presets, false); // make default presets hidden to begin + all.build(externalPresets, true); // make the external visible + }) + .catch(function() { all.init(); - } else { - all.build(data.presets, false); // make default presets hidden to begin - all.build(externalPresets, true); // make the external visible - } - done(all); - }); + }) + .finally(function() { + done(all); + }); }; all.field = function(id) { diff --git a/modules/renderer/background_source.js b/modules/renderer/background_source.js index 0ee86e797..affbb6c37 100644 --- a/modules/renderer/background_source.js +++ b/modules/renderer/background_source.js @@ -1,9 +1,5 @@ -import { - geoArea as d3_geoArea, - geoMercatorRaw as d3_geoMercatorRaw -} from 'd3-geo'; - -import { json as d3_json } from 'd3-request'; +import { geoArea as d3_geoArea, geoMercatorRaw as d3_geoMercatorRaw } from 'd3-geo'; +import { json as d3_json } from 'd3-fetch'; import { t } from '../util/locale'; import { geoExtent, geoSphericalDistance } from '../geo'; @@ -218,27 +214,29 @@ rendererBackgroundSource.Bing = function(data, dispatch) { var bing = rendererBackgroundSource(data); var key = 'Arzdiw4nlOJzRwOz__qailc8NiR31Tt51dN2D7cm57NrnceZnCpgOkmJhNpGoppU'; // Same as P2 and JOSM - var url = 'https://dev.virtualearth.net/REST/v1/Imagery/Metadata/Aerial?include=ImageryProviders&key=' + - key; + var url = 'https://dev.virtualearth.net/REST/v1/Imagery/Metadata/Aerial?include=ImageryProviders&key=' + key; var cache = {}; var inflight = {}; var providers = []; - d3_json(url, function(err, json) { - if (err) return; - providers = json.resourceSets[0].resources[0].imageryProviders.map(function(provider) { - return { - attribution: provider.attribution, - areas: provider.coverageAreas.map(function(area) { - return { - zoom: [area.zoomMin, area.zoomMax], - extent: geoExtent([area.bbox[1], area.bbox[0]], [area.bbox[3], area.bbox[2]]) - }; - }) - }; + d3_json(url) + .then(function(json) { + providers = json.resourceSets[0].resources[0].imageryProviders.map(function(provider) { + return { + attribution: provider.attribution, + areas: provider.coverageAreas.map(function(area) { + return { + zoom: [area.zoomMin, area.zoomMax], + extent: geoExtent([area.bbox[1], area.bbox[0]], [area.bbox[3], area.bbox[2]]) + }; + }) + }; + }); + dispatch.call('change'); + }) + .catch(function() { + /* ignore */ }); - dispatch.call('change'); - }); bing.copyrightNotices = function(zoom, extent) { @@ -256,34 +254,28 @@ rendererBackgroundSource.Bing = function(data, dispatch) { bing.getMetadata = function(center, tileCoord, callback) { - var tileId = tileCoord.slice(0, 3).join('/'); + var tileID = tileCoord.slice(0, 3).join('/'); var zoom = Math.min(tileCoord[2], 21); var centerPoint = center[1] + ',' + center[0]; // lat,lng var url = 'https://dev.virtualearth.net/REST/v1/Imagery/Metadata/Aerial/' + centerPoint + '?zl=' + zoom + '&key=' + key; - if (inflight[tileId]) return; + if (inflight[tileID]) return; - if (!cache[tileId]) { - cache[tileId] = {}; + if (!cache[tileID]) { + cache[tileID] = {}; } - if (cache[tileId] && cache[tileId].metadata) { - return callback(null, cache[tileId].metadata); + if (cache[tileID] && cache[tileID].metadata) { + return callback(null, cache[tileID].metadata); } - inflight[tileId] = true; - d3_json(url, function(error, result) { - delete inflight[tileId]; - - var err; - if (error) { - err = error; - } else if (!result && 'Unknown Error') { - err = result.errorDetails; - } - if (err) { - return callback(err); - } else { + inflight[tileID] = true; + d3_json(url) + .then(function(result) { + delete inflight[tileID]; + if (!result) { + throw new Error('Unknown Error'); + } var vintage = { start: localeDateString(result.resourceSets[0].resources[0].vintageStart), end: localeDateString(result.resourceSets[0].resources[0].vintageEnd) @@ -291,10 +283,13 @@ rendererBackgroundSource.Bing = function(data, dispatch) { vintage.range = vintageRange(vintage); var metadata = { vintage: vintage }; - cache[tileId].metadata = metadata; - return callback(null, metadata); - } - }); + cache[tileID].metadata = metadata; + if (callback) callback(null, metadata); + }) + .catch(function(err) { + delete inflight[tileID]; + if (callback) callback(err); + }); }; @@ -338,25 +333,31 @@ rendererBackgroundSource.Esri = function(data) { var tilemapUrl = dummyUrl.replace(/tile\/[0-9]+\/[0-9]+\/[0-9]+\?blankTile=false/, 'tilemap') + '/' + z + '/' + y + '/' + x + '/8/8'; // make the request and introspect the response from the tilemap server - d3_json(tilemapUrl, function (err, tilemap) { - if (err || !tilemap) return; - - var hasTiles = true; - for (var i = 0; i < tilemap.data.length; i++) { - // 0 means an individual tile in the grid doesn't exist - if (!tilemap.data[i]) { - hasTiles = false; - break; + d3_json(tilemapUrl) + .then(function(tilemap) { + if (!tilemap) { + throw new Error('Unknown Error'); + } + var hasTiles = true; + for (var i = 0; i < tilemap.data.length; i++) { + // 0 means an individual tile in the grid doesn't exist + if (!tilemap.data[i]) { + hasTiles = false; + break; + } } - } - // if any tiles are missing at level 20 we restrict maxZoom to 19 - esri.zoomExtent[1] = (hasTiles ? 22 : 19); - }); + // if any tiles are missing at level 20 we restrict maxZoom to 19 + esri.zoomExtent[1] = (hasTiles ? 22 : 19); + }) + .catch(function() { + /* ignore */ + }); }; + esri.getMetadata = function(center, tileCoord, callback) { - var tileId = tileCoord.slice(0, 3).join('/'); + var tileID = tileCoord.slice(0, 3).join('/'); var zoom = Math.min(tileCoord[2], esri.zoomExtent[1]); var centerPoint = center[0] + ',' + center[1]; // long, lat (as it should be) var unknown = t('info_panels.background.unknown'); @@ -364,7 +365,7 @@ rendererBackgroundSource.Esri = function(data) { var vintage = {}; var metadata = {}; - if (inflight[tileId]) return; + if (inflight[tileID]) return; switch (true) { case (zoom >= 20 && esri.id === 'EsriWorldImageryClarity'): @@ -393,11 +394,11 @@ rendererBackgroundSource.Esri = function(data) { url += metadataLayer + '/query?returnGeometry=false&geometry=' + centerPoint + '&inSR=4326&geometryType=esriGeometryPoint&outFields=*&f=json'; - if (!cache[tileId]) { - cache[tileId] = {}; + if (!cache[tileID]) { + cache[tileID] = {}; } - if (cache[tileId] && cache[tileId].metadata) { - return callback(null, cache[tileId].metadata); + if (cache[tileID] && cache[tileID].metadata) { + return callback(null, cache[tileID].metadata); } // accurate metadata is only available >= 13 @@ -418,24 +419,18 @@ rendererBackgroundSource.Esri = function(data) { callback(null, metadata); } else { - inflight[tileId] = true; - d3_json(url, function(error, result) { - delete inflight[tileId]; + inflight[tileID] = true; + d3_json(url) + .then(function(result) { + delete inflight[tileID]; + if (!result) { + throw new Error('Unknown Error'); + } else if (result.features && result.features.length < 1) { + throw new Error('No Results'); + } else if (result.error && result.error.message) { + throw new Error(result.error.message); + } - var err; - if (error) { - err = error; - } else if (!result) { - err = 'Unknown Error'; - } else if (result.features && result.features.length < 1) { - err = 'No Results'; - } else if (result.error && result.error.message) { - err = result.error.message; - } - - if (err) { - return callback(err); - } else { // pass through the discrete capture date from metadata var captureDate = localeDateString(result.features[0].attributes.SRC_DATE2); vintage = { @@ -459,10 +454,13 @@ rendererBackgroundSource.Esri = function(data) { metadata.accuracy += ' m'; } - cache[tileId].metadata = metadata; - return callback(null, metadata); - } - }); + cache[tileID].metadata = metadata; + if (callback) callback(null, metadata); + }) + .catch(function(error) { + delete inflight[tileID]; + if (callback) callback(error); + }); } diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index a4e714238..26cc86a55 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -1,8 +1,7 @@ import rbush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { json as d3_json } from 'd3-request'; -import { request as d3_request } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { geoExtent, geoVecAdd, geoVecScale } from '../geo'; import { qaError } from '../osm'; @@ -24,9 +23,9 @@ var _impOsmUrls = { }; function abortRequest(i) { - Object.values(i).forEach(function(v) { - if (v) { - v.abort(); + Object.values(i).forEach(function(controller) { + if (controller) { + controller.abort(); } }); } @@ -177,12 +176,16 @@ export default { ); var url = v + '/search?' + utilQsString(kParams); - requests[k] = d3_json(url, - function(err, data) { - delete _erCache.inflightTile[tile.id]; + var controller = new AbortController(); + requests[k] = controller; - if (err) return; - _erCache.loadedTile[tile.id] = true; + d3_json(url, { signal: controller.signal }) + .then(function(data) { + delete _erCache.inflightTile[tile.id][k]; + if (!Object.keys(_erCache.inflightTile[tile.id]).length) { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + } // Road segments at high zoom == oneways if (data.roadSegments) { @@ -317,20 +320,29 @@ export default { _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); + dispatch.call('loaded'); }); } - } - ); + }) + .catch(function() { + delete _erCache.inflightTile[tile.id][k]; + if (!Object.keys(_erCache.inflightTile[tile.id]).length) { + delete _erCache.inflightTile[tile.id]; + _erCache.loadedTile[tile.id] = true; + } + }); }); _erCache.inflightTile[tile.id] = requests; - dispatch.call('loaded'); }); }, getComments: function(d, callback) { // If comments already retrieved no need to do so again - if (d.comments !== undefined) { return callback({}, d); } + if (d.comments !== undefined) { + if (callback) callback({}, d); + return; + } var key = d.error_key; var qParams = {}; @@ -347,15 +359,16 @@ export default { var url = _impOsmUrls[key] + '/retrieveComments?' + utilQsString(qParams); var that = this; - d3_json(url, function(err, data) { - // comments are served newest to oldest - var comments = data.comments ? data.comments.reverse() : []; - - that.replaceError(d.update({ - comments: comments - })); - return callback(err, d); - }); + d3_json(url) + .then(function(data) { + // comments are served newest to oldest + var comments = data.comments ? data.comments.reverse() : []; + that.replaceError(d.update({ comments: comments })); + if (callback) callback(null, d); + }) + .catch(function(err) { + if (callback) callback(err); + }); }, postUpdate: function(d, callback) { @@ -399,13 +412,18 @@ export default { payload.text = d.newComment; } - _erCache.inflightPost[d.id] = d3_request(url) - .header('Content-Type', 'application/json') - .post(JSON.stringify(payload), function(err) { - delete _erCache.inflightPost[d.id]; + var controller = new AbortController(); + _erCache.inflightPost[d.id] = controller; - // Unsuccessful response status, keep issue open - if (err.status !== 200) { return callback(err, d); } + var options = { + method: 'POST', + signal: controller.signal, + body: JSON.stringify(payload) + }; + + d3_json(url, options) + .then(function() { + delete _erCache.inflightPost[d.id]; // Just a comment, update error in cache if (d.newStatus === undefined) { @@ -424,19 +442,22 @@ export default { })); } else { that.removeError(d); - if (d.newStatus === 'SOLVED') { // No pretty identifier, so we just use coordinates var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); _erCache.closed[key + ':' + closedID] = true; } } - - return callback(err, d); + if (callback) callback(null, d); + }) + .catch(function(err) { + delete _erCache.inflightPost[d.id]; + if (callback) callback(err); }); } }, + // get all cached errors covering the viewport getErrors: function(projection) { var viewport = projection.clipExtent(); diff --git a/modules/services/keepRight.js b/modules/services/keepRight.js index 6c40b3dcf..9c297d7b3 100644 --- a/modules/services/keepRight.js +++ b/modules/services/keepRight.js @@ -1,8 +1,7 @@ import rbush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { json as d3_json } from 'd3-request'; -import { request as d3_request } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; @@ -30,9 +29,9 @@ var _krRuleset = [ ]; -function abortRequest(i) { - if (i) { - i.abort(); +function abortRequest(controller) { + if (controller) { + controller.abort(); } } @@ -308,14 +307,16 @@ export default { var params = Object.assign({}, options, { left: rect[0], bottom: rect[3], right: rect[2], top: rect[1] }); var url = _krUrlRoot + 'export.php?' + utilQsString(params) + '&ch=' + rules; - _krCache.inflightTile[tile.id] = d3_json(url, - function(err, data) { + var controller = new AbortController(); + _krCache.inflightTile[tile.id] = controller; + + d3_json(url, { signal: controller.signal }) + .then(function(data) { delete _krCache.inflightTile[tile.id]; - - if (err) return; _krCache.loadedTile[tile.id] = true; - - if (!data.features || !data.features.length) return; + if (!data || !data.features || !data.features.length) { + throw new Error('No Data'); + } data.features.forEach(function(feature) { var loc = feature.geometry.coordinates; @@ -396,8 +397,12 @@ export default { }); dispatch.call('loaded'); - } - ); + }) + .catch(function() { + delete _krCache.inflightTile[tile.id]; + _krCache.loadedTile[tile.id] = true; + }); + }); }, @@ -420,9 +425,16 @@ export default { // NOTE: This throws a CORS err, but it seems successful. // We don't care too much about the response, so this is fine. var url = _krUrlRoot + 'comment.php?' + utilQsString(params); - _krCache.inflightPost[d.id] = d3_request(url) - .post(function(err) { + + var controller = new AbortController(); + _krCache.inflightPost[d.id] = controller; + + fetch(url, { method: 'POST', signal: controller.signal }) + .then(function(response) { delete _krCache.inflightPost[d.id]; + if (!response.ok) { + throw new Error(response.status + ' ' + response.statusText); + } if (d.state === 'ignore') { // ignore permanently (false positive) that.removeError(d); @@ -439,9 +451,12 @@ export default { })); } - return callback(err, d); + if (callback) callback(null, d); + }) + .catch(function(err) { + delete _krCache.inflightPost[d.id]; + if (callback) callback(err); }); - }, diff --git a/modules/services/mapillary.js b/modules/services/mapillary.js index 1369b0e30..101728703 100644 --- a/modules/services/mapillary.js +++ b/modules/services/mapillary.js @@ -1,10 +1,6 @@ /* global Mapillary:false */ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { request as d3_request } from 'd3-request'; -import { - select as d3_select, - selectAll as d3_selectAll -} from 'd3-selection'; +import { select as d3_select, selectAll as d3_selectAll } from 'd3-selection'; import rbush from 'rbush'; @@ -28,8 +24,8 @@ var _mlySelectedImage; var _mlyViewer; -function abortRequest(i) { - i.abort(); +function abortRequest(controller) { + controller.abort(); } @@ -80,22 +76,36 @@ function loadNextTilePage(which, currZoom, url, tile) { var id = tile.id + ',' + String(nextPage); if (cache.loaded[id] || cache.inflight[id]) return; - cache.inflight[id] = d3_request(nextURL) - .mimeType('application/json') - .response(function(xhr) { - var linkHeader = xhr.getResponseHeader('Link'); + + var controller = new AbortController(); + cache.inflight[id] = controller; + + var options = { + method: 'GET', + signal: controller.signal, + headers: { 'Content-Type': 'application/json' } + }; + + fetch(nextURL, options) + .then(function(response) { + if (!response.ok) { + throw new Error(response.status + ' ' + response.statusText); + } + var linkHeader = response.headers.Link; if (linkHeader) { - var pagination = parsePagination(xhr.getResponseHeader('Link')); + var pagination = parsePagination(linkHeader); if (pagination.next) { cache.nextURL[tile.id] = pagination.next; } } - return JSON.parse(xhr.responseText); + return response.json(); }) - .get(function(err, data) { + .then(function(data) { cache.loaded[id] = true; delete cache.inflight[id]; - if (err || !data.features || !data.features.length) return; + if (!data || !data.features || !data.features.length) { + throw new Error('No Data'); + } var features = data.features.map(function(feature) { var loc = feature.geometry.coordinates; @@ -182,6 +192,10 @@ function loadNextTilePage(which, currZoom, url, tile) { } else { cache.nextPage[tile.id] = Infinity; // no more pages to load } + }) + .catch(function() { + cache.loaded[id] = true; + delete cache.inflight[id]; }); } diff --git a/modules/services/nominatim.js b/modules/services/nominatim.js index 37130e196..5d98c9176 100644 --- a/modules/services/nominatim.js +++ b/modules/services/nominatim.js @@ -1,4 +1,4 @@ -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import rbush from 'rbush'; import { geoExtent } from '../geo'; @@ -18,7 +18,7 @@ export default { }, reset: function() { - Object.values(_inflight).forEach(function(req) { req.abort(); }); + Object.values(_inflight).forEach(function(controller) { controller.abort(); }); _inflight = {}; _nominatimCache = rbush(); }, @@ -37,45 +37,62 @@ export default { }, - reverse: function (location, callback) { + reverse: function (loc, callback) { var cached = _nominatimCache.search( - { minX: location[0], minY: location[1], maxX: location[0], maxY: location[1] } + { minX: loc[0], minY: loc[1], maxX: loc[0], maxY: loc[1] } ); if (cached.length > 0) { - return callback(null, cached[0].data); + if (callback) callback(null, cached[0].data); + return; } - var params = { zoom: 13, format: 'json', addressdetails: 1, lat: location[1], lon: location[0] }; + var params = { zoom: 13, format: 'json', addressdetails: 1, lat: loc[1], lon: loc[0] }; var url = apibase + 'reverse?' + utilQsString(params); + if (_inflight[url]) return; + var controller = new AbortController(); + _inflight[url] = controller; - _inflight[url] = d3_json(url, function(err, result) { - delete _inflight[url]; - - if (err) { - return callback(err); - } else if (result && result.error) { - return callback(result.error); - } - - var extent = geoExtent(location).padByMeters(200); - _nominatimCache.insert(Object.assign(extent.bbox(), {data: result})); - - callback(null, result); - }); + d3_json(url, { signal: controller.signal }) + .then(function(result) { + delete _inflight[url]; + if (result && result.error) { + throw new Error(result.error); + } + var extent = geoExtent(loc).padByMeters(200); + _nominatimCache.insert(Object.assign(extent.bbox(), {data: result})); + if (callback) callback(null, result); + }) + .catch(function(err) { + delete _inflight[url]; + if (err.name === 'AbortError') return; + if (callback) callback(err); + }); }, search: function (val, callback) { var searchVal = encodeURIComponent(val); var url = apibase + 'search/' + searchVal + '?limit=10&format=json'; - if (_inflight[url]) return; - _inflight[url] = d3_json(url, function(err, result) { - delete _inflight[url]; - callback(err, result); - }); + if (_inflight[url]) return; + var controller = new AbortController(); + _inflight[url] = controller; + + d3_json(url, { signal: controller.signal }) + .then(function(result) { + delete _inflight[url]; + if (result && result.error) { + throw new Error(result.error); + } + if (callback) callback(null, result); + }) + .catch(function(err) { + delete _inflight[url]; + if (err.name === 'AbortError') return; + if (callback) callback(err); + }); } }; diff --git a/modules/services/openstreetcam.js b/modules/services/openstreetcam.js index 58c1f4225..366632047 100644 --- a/modules/services/openstreetcam.js +++ b/modules/services/openstreetcam.js @@ -1,16 +1,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { request as d3_request } from 'd3-request'; - -import { - event as d3_event, - select as d3_select, - selectAll as d3_selectAll -} from 'd3-selection'; - -import { - zoom as d3_zoom, - zoomIdentity as d3_zoomIdentity -} from 'd3-zoom'; +import { json as d3_json } from 'd3-fetch'; +import { event as d3_event, select as d3_select, selectAll as d3_selectAll } from 'd3-selection'; +import { zoom as d3_zoom, zoomIdentity as d3_zoomIdentity } from 'd3-zoom'; import rbush from 'rbush'; @@ -33,8 +24,8 @@ var _oscCache; var _oscSelectedImage; -function abortRequest(i) { - i.abort(); +function abortRequest(controller) { + controller.abort(); } @@ -86,14 +77,23 @@ function loadNextTilePage(which, currZoom, url, tile) { var id = tile.id + ',' + String(nextPage); if (cache.loaded[id] || cache.inflight[id]) return; - cache.inflight[id] = d3_request(url) - .mimeType('application/json') - .header('Content-type', 'application/x-www-form-urlencoded') - .response(function(xhr) { return JSON.parse(xhr.responseText); }) - .post(params, function(err, data) { + var controller = new AbortController(); + cache.inflight[id] = controller; + + var options = { + method: 'POST', + signal: controller.signal, + body: params, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' } + }; + + d3_json(url, options) + .then(function(data) { cache.loaded[id] = true; delete cache.inflight[id]; - if (err || !data.currentPageItems || !data.currentPageItems.length) return; + if (!data || !data.currentPageItems || !data.currentPageItems.length) { + throw new Error('No Data'); + } function localeDateString(s) { if (!s) return null; @@ -146,6 +146,10 @@ function loadNextTilePage(which, currZoom, url, tile) { } else { cache.nextPage[tile.id] = Infinity; // no more pages to load } + }) + .catch(function() { + cache.loaded[id] = true; + delete cache.inflight[id]; }); } diff --git a/modules/services/osm.js b/modules/services/osm.js index 747e747a0..0c8c7c5a1 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -1,7 +1,7 @@ import _throttle from 'lodash-es/throttle'; import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { xml as d3_xml } from 'd3-request'; +import { xml as d3_xml } from 'd3-fetch'; import osmAuth from 'osm-auth'; import rbush from 'rbush'; @@ -51,9 +51,9 @@ function authDone() { } -function abortRequest(i) { - if (i) { - i.abort(); +function abortRequest(controllerOrXHR) { + if (controllerOrXHR) { + controllerOrXHR.abort(); } } @@ -468,7 +468,16 @@ export default { return oauth.xhr({ method: 'GET', path: path }, done); } else { var url = urlroot + path; - return d3_xml(url).get(done); + var controller = new AbortController(); + d3_xml(url, { signal: controller.signal }) + .then(function(data) { + done(null, data); + }) + .catch(function(err) { + if (err.name === 'AbortError') return; + done(err); + }); + return controller; } }, @@ -743,9 +752,11 @@ export default { // Fetch the status of the OSM API // GET /api/capabilities status: function(callback) { - d3_xml(urlroot + '/api/capabilities').get( - wrapcb(this, done, _connectionID) - ); + var url = urlroot + '/api/capabilities'; + var errback = wrapcb(this, done, _connectionID); + d3_xml(url) + .then(function(data) { errback(null, data); }) + .catch(function(err) { errback(err); }); function done(err, xml) { if (err) { return callback(err); } diff --git a/modules/services/osm_wikibase.js b/modules/services/osm_wikibase.js index 05504559a..edcc91df8 100644 --- a/modules/services/osm_wikibase.js +++ b/modules/services/osm_wikibase.js @@ -1,6 +1,6 @@ import _debounce from 'lodash-es/debounce'; -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { utilDetect } from '../util/detect'; import { utilQsString } from '../util'; @@ -16,11 +16,19 @@ var debouncedRequest = _debounce(request, 500, { leading: false }); function request(url, callback) { if (_inflight[url]) return; + var controller = new AbortController(); + _inflight[url] = controller; - _inflight[url] = d3_json(url, function (err, data) { - delete _inflight[url]; - callback(err, data); - }); + d3_json(url, { signal: controller.signal }) + .then(function(result) { + delete _inflight[url]; + if (callback) callback(null, result); + }) + .catch(function(err) { + delete _inflight[url]; + if (err.name === 'AbortError') return; + if (callback) callback(err); + }); } @@ -50,7 +58,7 @@ export default { reset: function() { - Object.values(_inflight).forEach(function(req) { req.abort(); }); + Object.values(_inflight).forEach(function(controller) { controller.abort(); }); _inflight = {}; }, diff --git a/modules/services/taginfo.js b/modules/services/taginfo.js index b1f849e49..a28c405c2 100644 --- a/modules/services/taginfo.js +++ b/modules/services/taginfo.js @@ -1,6 +1,6 @@ import _debounce from 'lodash-es/debounce'; -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { utilObjectOmit, utilQsString } from '../util'; import { currentLocale } from '../util/locale'; @@ -142,10 +142,19 @@ function request(url, params, exactMatch, callback, loaded) { if (checkCache(url, params, exactMatch, callback)) return; - _inflight[url] = d3_json(url, function (err, data) { - delete _inflight[url]; - loaded(err, data); - }); + var controller = new AbortController(); + _inflight[url] = controller; + + d3_json(url, { signal: controller.signal }) + .then(function(result) { + delete _inflight[url]; + if (loaded) loaded(null, result); + }) + .catch(function(err) { + delete _inflight[url]; + if (err.name === 'AbortError') return; + if (loaded) loaded(err); + }); } @@ -207,7 +216,7 @@ export default { reset: function() { - Object.values(_inflight).forEach(function(request) { request.abort(); }); + Object.values(_inflight).forEach(function(controller) { controller.abort(); }); _inflight = {}; }, diff --git a/modules/services/vector_tile.js b/modules/services/vector_tile.js index dfcbf970e..79928f6a6 100644 --- a/modules/services/vector_tile.js +++ b/modules/services/vector_tile.js @@ -1,5 +1,4 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { request as d3_request } from 'd3-request'; import deepEqual from 'fast-deep-equal'; import turf_bboxClip from '@turf/bbox-clip'; @@ -17,8 +16,8 @@ var dispatch = d3_dispatch('loadedData'); var _vtCache; -function abortRequest(i) { - i.abort(); +function abortRequest(controller) { + controller.abort(); } @@ -105,12 +104,23 @@ function loadTile(source, tile) { return subdomains[(tile.xyz[0] + tile.xyz[1]) % subdomains.length]; }); - source.inflight[tile.id] = d3_request(url) - .responseType('arraybuffer') - .get(function(err, data) { + + var controller = new AbortController(); + source.inflight[tile.id] = controller; + + fetch(url, { signal: controller.signal }) + .then(function(response) { + if (!response.ok) { + throw new Error(response.status + ' ' + response.statusText); + } source.loaded[tile.id] = []; delete source.inflight[tile.id]; - if (err || !data) return; + return response.arrayBuffer(); + }) + .then(function(data) { + if (!data) { + throw new Error('No Data'); + } var z = tile.xyz[2]; if (!source.canMerge[z]) { @@ -119,6 +129,10 @@ function loadTile(source, tile) { source.loaded[tile.id] = vtToGeoJSON(data, tile, source.canMerge[z]); dispatch.call('loadedData'); + }) + .catch(function() { + source.loaded[tile.id] = []; + delete source.inflight[tile.id]; }); } diff --git a/modules/services/wikidata.js b/modules/services/wikidata.js index 0e88ec66b..81299a628 100644 --- a/modules/services/wikidata.js +++ b/modules/services/wikidata.js @@ -1,4 +1,4 @@ -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { utilArrayUniq, utilQsString } from '../util'; import { currentLocale } from '../util/locale'; @@ -19,11 +19,11 @@ export default { // Search for Wikidata items matching the query itemsForSearchQuery: function(query, callback) { if (!query) { - callback('No query', {}); + if (callback) callback('No query', {}); return; } - d3_json(apibase + utilQsString({ + var url = apibase + utilQsString({ action: 'wbsearchentities', format: 'json', formatversion: 2, @@ -32,30 +32,31 @@ export default { language: this.languagesToQuery()[0], limit: 10, origin: '*' - }), function(err, data) { - if (data && data.error) { - err = data.error; - } - if (err) { - callback(err, {}); - } else { - callback(null, data.search || {}); - } }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } + if (callback) callback(null, result.search || {}); + }) + .catch(function(err) { + if (callback) callback(err, {}); + }); }, - // Given a Wikipedia language and article title, return an array of - // corresponding Wikidata entities. + // Given a Wikipedia language and article title, + // return an array of corresponding Wikidata entities. itemsByTitle: function(lang, title, callback) { if (!title) { - callback('No title', {}); + if (callback) callback('No title', {}); return; } lang = lang || 'en'; - - d3_json(apibase + utilQsString({ + var url = apibase + utilQsString({ action: 'wbgetentities', format: 'json', formatversion: 2, @@ -63,18 +64,21 @@ export default { titles: title, languages: 'en', // shrink response by filtering to one language origin: '*' - }), function(err, data) { - if (data && data.error) { - err = data.error; - } - if (err) { - callback(err, {}); - } else { - callback(null, data.entities || {}); - } }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } + if (callback) callback(null, result.entities || {}); + }) + .catch(function(err) { + if (callback) callback(err, {}); + }); }, + languagesToQuery: function() { return utilArrayUniq([ currentLocale.toLowerCase(), @@ -83,19 +87,19 @@ export default { ]); }, + entityByQID: function(qid, callback) { if (!qid) { callback('No qid', {}); return; } if (_wikidataCache[qid]) { - callback(null, _wikidataCache[qid]); + if (callback) callback(null, _wikidataCache[qid]); return; } var langs = this.languagesToQuery(); - - d3_json(apibase + utilQsString({ + var url = apibase + utilQsString({ action: 'wbgetentities', format: 'json', formatversion: 2, @@ -105,17 +109,18 @@ export default { languages: langs.join('|'), languagefallback: 1, origin: '*' - }), function(err, data) { - if (data && data.error) { - err = data.error; - } - if (err) { - callback(err, {}); - } else { - _wikidataCache[qid] = data.entities[qid]; - callback(null, data.entities[qid] || {}); - } }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } + if (callback) callback(null, result.entities[qid] || {}); + }) + .catch(function(err) { + if (callback) callback(err, {}); + }); }, @@ -134,9 +139,7 @@ export default { // } // getDocs: function(params, callback) { - var langs = this.languagesToQuery(); - this.entityByQID(params.qid, function(err, entity) { if (err || !entity) { callback(err || 'No entity'); @@ -144,7 +147,6 @@ export default { } var i; - var description; if (entity.descriptions && Object.keys(entity.descriptions).length > 0) { description = entity.descriptions[Object.keys(entity.descriptions)[0]].value; diff --git a/modules/services/wikipedia.js b/modules/services/wikipedia.js index 980af8741..aa33052a8 100644 --- a/modules/services/wikipedia.js +++ b/modules/services/wikipedia.js @@ -1,4 +1,4 @@ -import { json as d3_json } from 'd3-request'; +import { json as d3_json } from 'd3-fetch'; import { utilQsString } from '../util'; @@ -13,12 +13,12 @@ export default { search: function(lang, query, callback) { if (!query) { - callback('', []); + if (callback) callback('No Query', []); return; } lang = lang || 'en'; - d3_json(endpoint.replace('en', lang) + + var url = endpoint.replace('en', lang) + utilQsString({ action: 'query', list: 'search', @@ -27,26 +27,34 @@ export default { format: 'json', origin: '*', srsearch: query - }), function(err, data) { - if (err || !data || !data.query || !data.query.search || data.error) { - callback('', []); - } else { - var results = data.query.search.map(function(d) { return d.title; }); - callback(query, results); + }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } else if (!result || !result.query || !result.query.search) { + throw new Error('No Results'); } - } - ); + if (callback) { + var titles = result.query.search.map(function(d) { return d.title; }); + callback(null, titles); + } + }) + .catch(function(err) { + if (callback) callback(err, []); + }); }, suggestions: function(lang, query, callback) { if (!query) { - callback('', []); + if (callback) callback('', []); return; } lang = lang || 'en'; - d3_json(endpoint.replace('en', lang) + + var url = endpoint.replace('en', lang) + utilQsString({ action: 'opensearch', namespace: 0, @@ -54,24 +62,30 @@ export default { format: 'json', origin: '*', search: query - }), function(err, data) { - if (err || !data || data.error) { - callback('', []); - } else { - callback(data[0], data[1] || []); + }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } else if (!result || result.length < 2) { + throw new Error('No Results'); } - } - ); + if (callback) callback(null, result[1] || []); + }) + .catch(function(err) { + if (callback) callback(err, []); + }); }, translations: function(lang, title, callback) { if (!title) { - callback({}); + if (callback) callback({}); return; } - d3_json(endpoint.replace('en', lang) + + var url = endpoint.replace('en', lang) + utilQsString({ action: 'query', prop: 'langlinks', @@ -79,21 +93,27 @@ export default { origin: '*', lllimit: 500, titles: title - }), function(err, data) { - if (err || !data || !data.query || !data.query.pages || data.error) { - callback({}); - } else { - var list = data.query.pages[Object.keys(data.query.pages)[0]], - translations = {}; + }); + + d3_json(url) + .then(function(result) { + if (result && result.error) { + throw new Error(result.error); + } else if (!result || !result.query || !result.query.pages) { + throw new Error('No Results'); + } + if (callback) { + var list = result.query.pages[Object.keys(result.query.pages)[0]]; + var translations = {}; if (list && list.langlinks) { - list.langlinks.forEach(function(d) { - translations[d.lang] = d['*']; - }); + list.langlinks.forEach(function(d) { translations[d.lang] = d['*']; }); } callback(translations); } - } - ); + }) + .catch(function() { + if (callback) callback({}); + }); } }; diff --git a/modules/svg/data.js b/modules/svg/data.js index 64a0aacc4..5738d1f94 100644 --- a/modules/svg/data.js +++ b/modules/svg/data.js @@ -1,16 +1,8 @@ import _throttle from 'lodash-es/throttle'; -import { - geoBounds as d3_geoBounds, - geoPath as d3_geoPath -} from 'd3-geo'; - -import { text as d3_text } from 'd3-request'; - -import { - event as d3_event, - select as d3_select -} from 'd3-selection'; +import { geoBounds as d3_geoBounds, geoPath as d3_geoPath } from 'd3-geo'; +import { text as d3_text } from 'd3-fetch'; +import { event as d3_event, select as d3_select } from 'd3-selection'; import stringify from 'fast-json-stable-stringify'; import toGeoJSON from '@mapbox/togeojson'; @@ -480,10 +472,14 @@ export function svgData(projection, context, dispatch) { var extension = getExtension(testUrl) || defaultExtension; if (extension) { _template = null; - d3_text(url, function(err, data) { - if (err) return; - drawData.setFile(extension, data); - }); + d3_text(url) + .then(function(data) { + drawData.setFile(extension, data); + }) + .catch(function() { + /* ignore */ + }); + } else { drawData.template(url); } diff --git a/modules/svg/defs.js b/modules/svg/defs.js index 7e8fb264b..31c98967a 100644 --- a/modules/svg/defs.js +++ b/modules/svg/defs.js @@ -1,4 +1,4 @@ -import { request as d3_request } from 'd3-request'; +import { svg as d3_svg } from 'd3-fetch'; import { select as d3_select } from 'd3-selection'; import { utilArrayUniq } from '../util'; @@ -191,11 +191,9 @@ export function svgDefs(context) { .each(function(d) { var url = context.imagePath(d + '.svg'); var node = d3_select(this).node(); - d3_request(url) - .mimeType('image/svg+xml') - .response(function(xhr) { return xhr.responseXML; }) - .get(function(err, svg) { - if (err) return; + + d3_svg(url) + .then(function(svg) { node.appendChild( d3_select(svg.documentElement).attr('id', d).node() ); @@ -203,6 +201,9 @@ export function svgDefs(context) { d3_select(node).selectAll('path') .attr('fill', 'currentColor'); } + }) + .catch(function() { + /* ignore */ }); }); }; From 6f31062d32c28f2ecfb9a4bf6023cd442017236f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 25 Apr 2019 13:35:46 -0400 Subject: [PATCH 05/13] Add support for sinon-stubbing `fetch` API to spec_helpers --- test/.eslintrc | 2 +- test/spec/spec_helpers.js | 46 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/test/.eslintrc b/test/.eslintrc index d1f7eda1f..213cc998d 100644 --- a/test/.eslintrc +++ b/test/.eslintrc @@ -4,8 +4,8 @@ "mocha": true }, "globals": { - "_": false, "expect": true, + "fakeFetch": true, "happen": false, "iD": false, "sinon": false diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index 1453e023f..cb3f89d0e 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -38,8 +38,7 @@ expect = chai.expect; window.d3 = iD.d3; // TODO: remove if we can avoid exporting all of d3.js - -// workaround for `Array.from` polyfill in PhantomJS +// Workaround for `Array.from` polyfill in PhantomJS // https://github.com/openstreetmap/iD/issues/6087#issuecomment-476219308 var __arrayfrom = Array.from; Array.from = function(what) { @@ -51,3 +50,46 @@ Array.from = function(what) { return __arrayfrom.apply(null, arguments); } }; + + +// Add support for sinon-stubbing `fetch` API +// (sinon fakeServer works only on `XMLHttpRequest`) +// see https://github.com/sinonjs/nise/issues/7 +// +// None of the alternatives really worked well, +// so I'm just pasting the `fake-fetch` methods here. +// - https://github.com/msn0/fake-fetch +// - https://github.com/wheresrhys/fetch-mock + +window.fakeFetch = { + install: function () { + sinon.stub(window, 'fetch'); + }, + restore: function () { + window.fetch.restore(); + }, + getUrl: function () { + return window.fetch.firstCall.args[0]; + }, + getOptions: function () { + return window.fetch.firstCall.args[1] || {}; + }, + getMethod: function () { + return this.getOptions().method || 'get'; + }, + getBody: function () { + return this.getOptions().body || ''; + }, + getRequestHeaders: function () { + return this.getOptions().headers || {}; + }, + respondWith: function (data, options) { + return window.fetch.returns(Promise.resolve(new Response(data, options))); + } +}; + +Object.defineProperty( + window.fakeFetch, 'called', { + get: function() { return !!window.fetch.firstCall; } + } +); From 3e363fed8b2051d91a1a1b81175eed3b9c98fe59 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 25 Apr 2019 19:22:11 -0400 Subject: [PATCH 06/13] Make sure to return a new Response each time, not the same one You can only consume a response once - thereafter the response body becomes "locked" and will throw an error if you try. https://stackoverflow.com/questions/53511974/javascript-fetch-failed-to-execute-json-on-response-body-stream-is-locked --- test/spec/spec_helpers.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index cb3f89d0e..aaf859894 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -84,7 +84,9 @@ window.fakeFetch = { return this.getOptions().headers || {}; }, respondWith: function (data, options) { - return window.fetch.returns(Promise.resolve(new Response(data, options))); + return window.fetch.callsFake(function() { + return Promise.resolve(new Response(data, options)); + }); } }; From b99be671693d411fe9742e414691aa874eeff177 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 25 Apr 2019 21:58:36 -0400 Subject: [PATCH 07/13] When calling an errback from a Promise.catch, pass err.message --- modules/core/context.js | 2 +- modules/renderer/background_source.js | 6 +++--- modules/services/improveOSM.js | 4 ++-- modules/services/keepRight.js | 2 +- modules/services/nominatim.js | 4 ++-- modules/services/osm.js | 4 ++-- modules/services/osm_wikibase.js | 2 +- modules/services/taginfo.js | 2 +- modules/services/wikidata.js | 6 +++--- modules/services/wikipedia.js | 10 +++++----- modules/ui/fields/localized.js | 5 ++++- 11 files changed, 25 insertions(+), 22 deletions(-) diff --git a/modules/core/context.js b/modules/core/context.js index 9022c6351..4a6da757e 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -442,7 +442,7 @@ export function coreContext() { if (callback) callback(); }) .catch(function(err) { - if (callback) callback(err); + if (callback) callback(err.message); }); } else { if (locale) { diff --git a/modules/renderer/background_source.js b/modules/renderer/background_source.js index affbb6c37..883cc76dc 100644 --- a/modules/renderer/background_source.js +++ b/modules/renderer/background_source.js @@ -288,7 +288,7 @@ rendererBackgroundSource.Bing = function(data, dispatch) { }) .catch(function(err) { delete inflight[tileID]; - if (callback) callback(err); + if (callback) callback(err.message); }); }; @@ -457,9 +457,9 @@ rendererBackgroundSource.Esri = function(data) { cache[tileID].metadata = metadata; if (callback) callback(null, metadata); }) - .catch(function(error) { + .catch(function(err) { delete inflight[tileID]; - if (callback) callback(error); + if (callback) callback(err.message); }); } diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 26cc86a55..50ae62a19 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -367,7 +367,7 @@ export default { if (callback) callback(null, d); }) .catch(function(err) { - if (callback) callback(err); + if (callback) callback(err.message); }); }, @@ -452,7 +452,7 @@ export default { }) .catch(function(err) { delete _erCache.inflightPost[d.id]; - if (callback) callback(err); + if (callback) callback(err.message); }); } }, diff --git a/modules/services/keepRight.js b/modules/services/keepRight.js index 9c297d7b3..afe644de8 100644 --- a/modules/services/keepRight.js +++ b/modules/services/keepRight.js @@ -455,7 +455,7 @@ export default { }) .catch(function(err) { delete _krCache.inflightPost[d.id]; - if (callback) callback(err); + if (callback) callback(err.message); }); }, diff --git a/modules/services/nominatim.js b/modules/services/nominatim.js index 5d98c9176..053a8ba70 100644 --- a/modules/services/nominatim.js +++ b/modules/services/nominatim.js @@ -67,7 +67,7 @@ export default { .catch(function(err) { delete _inflight[url]; if (err.name === 'AbortError') return; - if (callback) callback(err); + if (callback) callback(err.message); }); }, @@ -91,7 +91,7 @@ export default { .catch(function(err) { delete _inflight[url]; if (err.name === 'AbortError') return; - if (callback) callback(err); + if (callback) callback(err.message); }); } diff --git a/modules/services/osm.js b/modules/services/osm.js index 0c8c7c5a1..ecef84dc1 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -475,7 +475,7 @@ export default { }) .catch(function(err) { if (err.name === 'AbortError') return; - done(err); + done(err.message); }); return controller; } @@ -756,7 +756,7 @@ export default { var errback = wrapcb(this, done, _connectionID); d3_xml(url) .then(function(data) { errback(null, data); }) - .catch(function(err) { errback(err); }); + .catch(function(err) { errback(err.message); }); function done(err, xml) { if (err) { return callback(err); } diff --git a/modules/services/osm_wikibase.js b/modules/services/osm_wikibase.js index edcc91df8..47254c2c0 100644 --- a/modules/services/osm_wikibase.js +++ b/modules/services/osm_wikibase.js @@ -27,7 +27,7 @@ function request(url, callback) { .catch(function(err) { delete _inflight[url]; if (err.name === 'AbortError') return; - if (callback) callback(err); + if (callback) callback(err.message); }); } diff --git a/modules/services/taginfo.js b/modules/services/taginfo.js index a28c405c2..099b864a7 100644 --- a/modules/services/taginfo.js +++ b/modules/services/taginfo.js @@ -153,7 +153,7 @@ function request(url, params, exactMatch, callback, loaded) { .catch(function(err) { delete _inflight[url]; if (err.name === 'AbortError') return; - if (loaded) loaded(err); + if (loaded) loaded(err.message); }); } diff --git a/modules/services/wikidata.js b/modules/services/wikidata.js index 81299a628..e23f2edaf 100644 --- a/modules/services/wikidata.js +++ b/modules/services/wikidata.js @@ -42,7 +42,7 @@ export default { if (callback) callback(null, result.search || {}); }) .catch(function(err) { - if (callback) callback(err, {}); + if (callback) callback(err.message, {}); }); }, @@ -74,7 +74,7 @@ export default { if (callback) callback(null, result.entities || {}); }) .catch(function(err) { - if (callback) callback(err, {}); + if (callback) callback(err.message, {}); }); }, @@ -119,7 +119,7 @@ export default { if (callback) callback(null, result.entities[qid] || {}); }) .catch(function(err) { - if (callback) callback(err, {}); + if (callback) callback(err.message, {}); }); }, diff --git a/modules/services/wikipedia.js b/modules/services/wikipedia.js index aa33052a8..c373ef922 100644 --- a/modules/services/wikipedia.js +++ b/modules/services/wikipedia.js @@ -74,14 +74,14 @@ export default { if (callback) callback(null, result[1] || []); }) .catch(function(err) { - if (callback) callback(err, []); + if (callback) callback(err.message, []); }); }, translations: function(lang, title, callback) { if (!title) { - if (callback) callback({}); + if (callback) callback('No Title'); return; } @@ -108,11 +108,11 @@ export default { if (list && list.langlinks) { list.langlinks.forEach(function(d) { translations[d.lang] = d['*']; }); } - callback(translations); + callback(null, translations); } }) - .catch(function() { - if (callback) callback({}); + .catch(function(err) { + if (callback) callback(err.message); }); } diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 14b2868ea..2d8faa8c1 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -480,7 +480,10 @@ export function uiFieldLocalized(field, context) { _wikiTitles = {}; var wm = tags.wikipedia.match(/([^:]+):(.+)/); if (wm && wm[0] && wm[1]) { - wikipedia.translations(wm[1], wm[2], function(d) { _wikiTitles = d; }); + wikipedia.translations(wm[1], wm[2], function(err, d) { + if (err || !d) return; + _wikiTitles = d; + }); } } From b48a7d1e1ba931e89e7d0267a0743a7fc307d93f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 26 Apr 2019 22:23:08 -0400 Subject: [PATCH 08/13] Workround for status errors thrown by d3-xml Because the done callback is expecting something that has a `status` property, (like an XHR would), we need to extract the status out of the error message. d3-xml includes status in the error message, but we can't access the response itself. --- modules/services/osm.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/modules/services/osm.js b/modules/services/osm.js index ecef84dc1..79bfa7f6d 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -440,7 +440,8 @@ export default { // 400 Bad Request, 401 Unauthorized, 403 Forbidden // Logout and retry the request.. - if (isAuthenticated && err && (err.status === 400 || err.status === 401 || err.status === 403)) { + if (isAuthenticated && err && err.status && + (err.status === 400 || err.status === 401 || err.status === 403)) { that.logout(); that.loadFromAPI(path, callback, options); @@ -448,7 +449,7 @@ 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 && (err.status === 509 || err.status === 429)) { _rateLimitError = err; dispatch.call('change'); @@ -475,7 +476,15 @@ export default { }) .catch(function(err) { if (err.name === 'AbortError') return; - done(err.message); + // d3-fetch includes status in the error message, + // but we can't access the response itself + // https://github.com/d3/d3-fetch/issues/27 + var match = err.message.match(/^\d{3}/); + if (match) { + done({ status: +match[0], statusText: err.message }); + } else { + done(err.message); + } }); return controller; } From 552ea46c436c63a63bf588054f98824b68c3b7e7 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 26 Apr 2019 22:25:57 -0400 Subject: [PATCH 09/13] Wrap the fake-fetch methods with sinon.fakeServer-like interface This mimics `respondWith` and `respond` to keep the tests mostly similar between `FakeFetch` and `sinon.fakeServer` Major difference is that all the fakeServer tests which were sync are now async and this is probably unavoidable. --- test/spec/spec_helpers.js | 133 +++++++++++++++++++++++++++++--------- 1 file changed, 101 insertions(+), 32 deletions(-) diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index aaf859894..26769ef52 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -57,41 +57,110 @@ Array.from = function(what) { // see https://github.com/sinonjs/nise/issues/7 // // None of the alternatives really worked well, -// so I'm just pasting the `fake-fetch` methods here. +// so I'm just wrapping the `fake-fetch` methods in here. // - https://github.com/msn0/fake-fetch // - https://github.com/wheresrhys/fetch-mock -window.fakeFetch = { - install: function () { - sinon.stub(window, 'fetch'); - }, - restore: function () { - window.fetch.restore(); - }, - getUrl: function () { - return window.fetch.firstCall.args[0]; - }, - getOptions: function () { - return window.fetch.firstCall.args[1] || {}; - }, - getMethod: function () { - return this.getOptions().method || 'get'; - }, - getBody: function () { - return this.getOptions().body || ''; - }, - getRequestHeaders: function () { - return this.getOptions().headers || {}; - }, - respondWith: function (data, options) { - return window.fetch.callsFake(function() { - return Promise.resolve(new Response(data, options)); +window.fakeFetch = function() { + var _responders = []; + var _requests = []; + + function fake(url, options) { + options = Object.assign({ method: 'get', headers: {}, body: '' }, options); + return new Promise(function(resolve, reject) { + _requests.push({ + url: url, options: options, resolve: resolve, reject: reject + }); }); } -}; -Object.defineProperty( - window.fakeFetch, 'called', { - get: function() { return !!window.fetch.firstCall; } - } -); + return { + requests: function() { + return _requests; + }, + + create: function () { + _responders = []; + _requests = []; + sinon.stub(window, 'fetch').callsFake(fake); + return this; + }, + + restore: function () { + window.fetch.restore(); + }, + + getUrl: function () { + return window.fetch.firstCall.args[0]; + }, + + getOptions: function () { + return window.fetch.firstCall.args[1] || {}; + }, + + getMethod: function () { + return this.getOptions().method || 'get'; + }, + + getBody: function () { + return this.getOptions().body || ''; + }, + + getRequestHeaders: function () { + return this.getOptions().headers || {}; + }, + + respondWith: function(method, match, response) { + var status = 200; + var headers = { 'Content-Type': 'text/html' }; + var body = 'OK'; + + if (typeof response === 'string') { + body = response; + } else if (Array.isArray(response) && response.length === 3) { + status = response[0]; + headers = Object.assign(headers, response[1] || {}); + body = response[2]; + } + + headers['Content-Length'] = body.length; + var data = new Blob([body], { type: headers['Content-Type'] }); + var options = { status: status, headers: headers }; + + _responders.push({ + method: method, + match: match, + respond: function() { return new Response(data, options); } + }); + }, + + respond: function () { + _requests.forEach(function(request) { + var didMatch = false; + for (var i = 0; i < _responders.length; i++) { + var responder = _responders[i]; + if (responder.method.toLowerCase() !== request.options.method.toLowerCase()) { + continue; // skip if method doesn't match (get/post) + } + + if (responder.match.constructor.name === 'RegExp') { + didMatch = responder.match.test(request.url); + } else if (typeof responder.match === 'string') { + didMatch = (request.url.indexOf(responder.match) !== -1); + } + + if (didMatch) { + request.resolve(responder.respond()); + break; + } + } + if (!didMatch) { + request.reject(new Response( + new Blob(['404'], { type: 'text/plain' }), + { status: 404, statusText: 'Not Found' } + )); + } + }); + } + }; +}; From d0452e6be418ab1e0830b9b072f5d339cd07d1e3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 26 Apr 2019 22:29:48 -0400 Subject: [PATCH 10/13] Convert fakeServer tests to use fakeFetch - Many text expects are now wrapped in setTimeout, as the fetch promises settle async now. - This makes the tests somewhat brittle, and we should maybe consider reworking some of them. For example it is very hard to perform a test like `expect(spy).to.have.not.been.called` in an async way. (We could instead inspect the fakeServer requests() to know this.) - Also includes some trickery for osm.js, which uses d3-xml (fetch) now for unauthenticated calls and osmauth (xhr) for authenticated calls --- test/spec/presets/index.js | 2 +- test/spec/services/mapillary.js | 45 +++-- test/spec/services/nominatim.js | 110 +++++++----- test/spec/services/openstreetcam.js | 23 ++- test/spec/services/osm.js | 267 ++++++++++------------------ test/spec/services/osm_wikibase.js | 37 ++-- test/spec/services/streetside.js | 18 +- test/spec/services/taginfo.js | 241 +++++++++++++++---------- test/spec/ui/fields/wikipedia.js | 6 +- 9 files changed, 389 insertions(+), 360 deletions(-) diff --git a/test/spec/presets/index.js b/test/spec/presets/index.js index c874b314b..3da1a6fab 100644 --- a/test/spec/presets/index.js +++ b/test/spec/presets/index.js @@ -245,7 +245,7 @@ describe('iD.presetIndex', function () { }; beforeEach(function () { - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); }); afterEach(function () { diff --git a/test/spec/services/mapillary.js b/test/spec/services/mapillary.js index 8642b9b51..a6f21c344 100644 --- a/test/spec/services/mapillary.js +++ b/test/spec/services/mapillary.js @@ -18,7 +18,7 @@ describe('iD.serviceMapillary', function() { .translate([-116508, 0]) // 10,0 .clipExtent([[0,0], dimensions]); - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); mapillary = iD.services.mapillary; mapillary.reset(); }); @@ -54,7 +54,7 @@ describe('iD.serviceMapillary', function() { }); describe('#loadImages', function() { - it('fires loadedImages when images are loaded', function() { + it('fires loadedImages when images are loaded', function(done) { var spy = sinon.spy(); mapillary.on('loadedImages', spy); mapillary.loadImages(context.projection); @@ -71,10 +71,13 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - expect(spy).to.have.been.calledOnce; + window.setTimeout(function() { + expect(spy).to.have.been.calledOnce; + done(); + }, 50); }); - it('does not load images around null island', function() { + it('does not load images around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); mapillary.on('loadedImages', spy); @@ -92,10 +95,13 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - expect(spy).to.have.been.not.called; + window.setTimeout(function() { + expect(spy).to.have.been.not.called; + done(); + }, 50); }); - it.skip('loads multiple pages of image results', function() { + it.skip('loads multiple pages of image results', function(done) { var spy = sinon.spy(); mapillary.on('loadedImages', spy); mapillary.loadImages(context.projection); @@ -130,12 +136,16 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response1) ]); server.respond(); - expect(spy).to.have.been.calledTwice; + window.setTimeout(function() { + expect(spy).to.have.been.calledTwice; + done(); + }, 50); }); }); + describe('#loadSigns', function() { - it('fires loadedSigns when signs are loaded', function() { + it('fires loadedSigns when signs are loaded', function(done) { var spy = sinon.spy(); mapillary.on('loadedSigns', spy); mapillary.loadSigns(context, context.projection); @@ -156,10 +166,13 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - expect(spy).to.have.been.calledOnce; + window.setTimeout(function() { + expect(spy).to.have.been.calledOnce; + done(); + }, 50); }); - it('does not load signs around null island', function() { + it('does not load signs around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); mapillary.on('loadedSigns', spy); @@ -181,10 +194,13 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - expect(spy).to.have.been.not.called; + window.setTimeout(function() { + expect(spy).to.have.been.not.called; + done(); + }, 50); }); - it.skip('loads multiple pages of signs results', function() { + it.skip('loads multiple pages of signs results', function(done) { var spy = sinon.spy(); mapillary.on('loadedSigns', spy); mapillary.loadSigns(context, context.projection); @@ -226,7 +242,10 @@ describe('iD.serviceMapillary', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(response1) ]); server.respond(); - expect(spy).to.have.been.calledTwice; + window.setTimeout(function() { + expect(spy).to.have.been.calledTwice; + done(); + }, 50); }); }); diff --git a/test/spec/services/nominatim.js b/test/spec/services/nominatim.js index fb984039b..8773018a9 100644 --- a/test/spec/services/nominatim.js +++ b/test/spec/services/nominatim.js @@ -11,7 +11,7 @@ describe('iD.serviceNominatim', function() { }); beforeEach(function() { - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); nominatim = iD.services.geocoder; nominatim.reset(); }); @@ -26,7 +26,7 @@ describe('iD.serviceNominatim', function() { describe('#countryCode', function() { - it('calls the given callback with the results of the country code query', function() { + it('calls the given callback with the results of the country code query', function(done) { var callback = sinon.spy(); nominatim.countryCode([16, 48], callback); @@ -35,69 +35,83 @@ describe('iD.serviceNominatim', function() { '{"address":{"country_code":"at"}}']); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'}); - expect(callback).to.have.been.calledWithExactly(null, 'at'); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'} + ); + expect(callback).to.have.been.calledWithExactly(null, 'at'); + done(); + }, 50); }); }); describe('#reverse', function() { - it('should not cache distant result', function() { + it('should not cache distant result', function(done) { var callback = sinon.spy(); nominatim.reverse([16, 48], callback); server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), - [200, { 'Content-Type': 'application/json' }, - '{"address":{"country_code":"at"}}']); + [200, { 'Content-Type': 'application/json' }, '{"address":{"country_code":"at"}}']); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'}); - expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'} + ); + expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); - server.restore(); - server = sinon.fakeServer.create(); + server.restore(); + server = window.fakeFetch().create(); - callback = sinon.spy(); - nominatim.reverse([17, 49], callback); + callback = sinon.spy(); + nominatim.reverse([17, 49], callback); - server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), - [200, { 'Content-Type': 'application/json' }, - '{"address":{"country_code":"cz"}}']); - server.respond(); + server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), + [200, { 'Content-Type': 'application/json' }, '{"address":{"country_code":"cz"}}']); + server.respond(); - expect(query(server.requests[0].url)).to.eql( - {zoom: '13', format: 'json', addressdetails: '1', lat: '49', lon: '17'}); - expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'cz'}}); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {zoom: '13', format: 'json', addressdetails: '1', lat: '49', lon: '17'} + ); + expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'cz'}}); + done(); + }, 50); + }, 50); }); - it('should cache nearby result', function() { + it('should cache nearby result', function(done) { var callback = sinon.spy(); nominatim.reverse([16, 48], callback); server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), - [200, { 'Content-Type': 'application/json' }, - '{"address":{"country_code":"at"}}']); + [200, { 'Content-Type': 'application/json' }, '{"address":{"country_code":"at"}}']); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'}); - expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {zoom: '13', format: 'json', addressdetails: '1', lat: '48', lon: '16'} + ); + expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); - server.restore(); - server = sinon.fakeServer.create(); + server.restore(); + server = window.fakeFetch().create(); - callback = sinon.spy(); - nominatim.reverse([16.000001, 48.000001], callback); + callback = sinon.spy(); + nominatim.reverse([16.000001, 48.000001], callback); - server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), - [200, { 'Content-Type': 'application/json' }, - '{"address":{"country_code":"cz"}}']); - server.respond(); - expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); + server.respondWith('GET', new RegExp('https://nominatim.openstreetmap.org/reverse'), + [200, { 'Content-Type': 'application/json' }, '{"address":{"country_code":"cz"}}']); + server.respond(); + + window.setTimeout(function() { + expect(callback).to.have.been.calledWithExactly(null, {address: {country_code:'at'}}); + done(); + }, 50); + }, 50); }); - it('calls the given callback with an error', function() { + it('calls the given callback with an error', function(done) { var callback = sinon.spy(); nominatim.reverse([1000, 1000], callback); @@ -106,16 +120,19 @@ describe('iD.serviceNominatim', function() { '{"error":"Unable to geocode"}']); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {zoom: '13', format: 'json', addressdetails: '1', lat: '1000', lon: '1000'}); - expect(callback).to.have.been.calledWithExactly('Unable to geocode'); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {zoom: '13', format: 'json', addressdetails: '1', lat: '1000', lon: '1000'} + ); + expect(callback).to.have.been.calledWithExactly('Unable to geocode'); + done(); + }, 50); }); }); describe('#search', function() { - - it('calls the given callback with the results of the search query', function() { + it('calls the given callback with the results of the search query', function(done) { var callback = sinon.spy(); nominatim.search('philadelphia', callback); @@ -125,8 +142,11 @@ describe('iD.serviceNominatim', function() { ]); server.respond(); - expect(query(server.requests[0].url)).to.eql({format: 'json', limit: '10'}); - expect(callback).to.have.been.calledOnce; + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql({format: 'json', limit: '10'}); + expect(callback).to.have.been.calledOnce; + done(); + }, 50); }); }); diff --git a/test/spec/services/openstreetcam.js b/test/spec/services/openstreetcam.js index 13d03b56a..a12c48291 100644 --- a/test/spec/services/openstreetcam.js +++ b/test/spec/services/openstreetcam.js @@ -17,7 +17,7 @@ describe('iD.serviceOpenstreetcam', function() { .translate([-116508, 0]) // 10,0 .clipExtent([[0,0], dimensions]); - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); openstreetcam = iD.services.openstreetcam; openstreetcam.reset(); }); @@ -51,7 +51,7 @@ describe('iD.serviceOpenstreetcam', function() { }); describe('#loadImages', function() { - it('fires loadedImages when images are loaded', function() { + it('fires loadedImages when images are loaded', function(done) { var spy = sinon.spy(); openstreetcam.on('loadedImages', spy); openstreetcam.loadImages(context.projection); @@ -102,10 +102,13 @@ describe('iD.serviceOpenstreetcam', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(data) ]); server.respond(); - expect(spy).to.have.been.calledOnce; + window.setTimeout(function() { + expect(spy).to.have.been.calledOnce; + done(); + }, 50); }); - it('does not load images around null island', function() { + it('does not load images around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); openstreetcam.on('loadedImages', spy); @@ -157,10 +160,13 @@ describe('iD.serviceOpenstreetcam', function() { [200, { 'Content-Type': 'application/json' }, JSON.stringify(data) ]); server.respond(); - expect(spy).to.have.been.not.called; + window.setTimeout(function() { + expect(spy).to.have.been.not.called; + done(); + }, 50); }); - it.skip('loads multiple pages of image results', function() { + it.skip('loads multiple pages of image results', function(done) { var spy = sinon.spy(); openstreetcam.on('loadedImages', spy); openstreetcam.loadImages(context.projection); @@ -222,7 +228,10 @@ describe('iD.serviceOpenstreetcam', function() { }); server.respond(); - expect(spy).to.have.been.calledTwice; + window.setTimeout(function() { + expect(spy).to.have.been.calledTwice; + done(); + }, 50); }); }); diff --git a/test/spec/services/osm.js b/test/spec/services/osm.js index cfccfd0b6..45c438f5f 100644 --- a/test/spec/services/osm.js +++ b/test/spec/services/osm.js @@ -1,8 +1,8 @@ describe('iD.serviceOsm', function () { - var context, connection, server, spy; + var context, connection, spy; + var serverFetch, serverXHR; function login() { - if (!connection) return; connection.switch({ urlroot: 'http://www.openstreetmap.org', oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT', @@ -13,7 +13,6 @@ describe('iD.serviceOsm', function () { } function logout() { - if (!connection) return; connection.logout(); } @@ -26,7 +25,8 @@ describe('iD.serviceOsm', function () { }); beforeEach(function () { - server = sinon.fakeServer.create(); + serverFetch = window.fakeFetch().create(); // unauthenticated calls use d3-fetch + serverXHR = sinon.fakeServer.create(); // authenticated calls use XHR via osm-auth context = iD.coreContext(); connection = context.connection(); connection.switch({ urlroot: 'http://www.openstreetmap.org' }); @@ -35,7 +35,8 @@ describe('iD.serviceOsm', function () { }); afterEach(function() { - server.restore(); + serverFetch.restore(); + serverXHR.restore(); }); @@ -139,43 +140,33 @@ describe('iD.serviceOsm', function () { describe('#loadFromAPI', function () { var path = '/api/0.6/map?bbox=-74.542,40.655,-74.541,40.656'; var response = '' + - '' + - ' ' + - ' ' + - ' ' + - ' ' + - ' ' + - ' ' + - ' ' + - ' ' + - ' ' + - ''; + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ''; - beforeEach(function() { - connection.reset(); - server = sinon.fakeServer.create(); - spy = sinon.spy(); - }); - - afterEach(function() { - server.restore(); - }); - - - it('returns an object', function (done) { + it('returns an object', function(done) { connection.loadFromAPI(path, function (err, xml) { expect(err).to.not.be.ok; expect(typeof xml).to.eql('object'); done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, [200, { 'Content-Type': 'text/xml' }, response]); - server.respond(); + serverFetch.respond(); }); it('retries an authenticated call unauthenticated if 400 Bad Request', function (done) { login(); + connection.loadFromAPI(path, function (err, xml) { expect(err).to.be.not.ok; expect(typeof xml).to.eql('object'); @@ -183,17 +174,13 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, - function(request) { - if (connection.authenticated()) { - return request.respond(400, {}); - } else { - return request.respond(200, { 'Content-Type': 'text/xml' }, response); - } - } - ); - server.respond(); - server.respond(); + serverXHR.respondWith('GET', 'http://www.openstreetmap.org' + path, + [400, { 'Content-Type': 'text/plain' }, 'Bad Request']); + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, + [200, { 'Content-Type': 'text/xml' }, response]); + + serverXHR.respond(); + serverFetch.respond(); }); it('retries an authenticated call unauthenticated if 401 Unauthorized', function (done) { @@ -205,17 +192,13 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, - function(request) { - if (connection.authenticated()) { - return request.respond(401, {}); - } else { - return request.respond(200, { 'Content-Type': 'text/xml' }, response); - } - } - ); - server.respond(); - server.respond(); + serverXHR.respondWith('GET', 'http://www.openstreetmap.org' + path, + [401, { 'Content-Type': 'text/plain' }, 'Unauthorized']); + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, + [200, { 'Content-Type': 'text/xml' }, response]); + + serverXHR.respond(); + serverFetch.respond(); }); it('retries an authenticated call unauthenticated if 403 Forbidden', function (done) { @@ -227,17 +210,13 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, - function(request) { - if (connection.authenticated()) { - return request.respond(403, {}); - } else { - return request.respond(200, { 'Content-Type': 'text/xml' }, response); - } - } - ); - server.respond(); - server.respond(); + serverXHR.respondWith('GET', 'http://www.openstreetmap.org' + path, + [403, { 'Content-Type': 'text/plain' }, 'Forbidden']); + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, + [200, { 'Content-Type': 'text/xml' }, response]); + + serverXHR.respond(); + serverFetch.respond(); }); @@ -250,20 +229,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, - function(request) { - if (!connection.authenticated()) { - // workaround: sinon.js seems to call error handler with a - // sinon.Event instead of the target XMLHttpRequest object.. - var orig = request.onreadystatechange; - request.onreadystatechange = function(o) { orig((o && o.target) || o); }; - return request.respond(509, {}); - } else { - return request.respond(200, { 'Content-Type': 'text/xml' }, response); - } - } - ); - server.respond(); + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, + [509, { 'Content-Type': 'text/plain' }, 'Bandwidth Limit Exceeded']); + serverFetch.respond(); }); it('dispatches change event if 429 Too Many Requests', function (done) { @@ -275,20 +243,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org' + path, - function(request) { - if (!connection.authenticated()) { - // workaround: sinon.js seems to call error handler with a - // sinon.Event instead of the target XMLHttpRequest object.. - var orig = request.onreadystatechange; - request.onreadystatechange = function(o) { orig((o && o.target) || o); }; - return request.respond(429, {}); - } else { - return request.respond(200, { 'Content-Type': 'text/xml' }, response); - } - } - ); - server.respond(); + serverFetch.respondWith('GET', 'http://www.openstreetmap.org' + path, + [429, { 'Content-Type': 'text/plain' }, '429 Too Many Requests']); + serverFetch.respond(); }); }); @@ -320,28 +277,28 @@ describe('iD.serviceOsm', function () { var spy = sinon.spy(); connection.loadTiles(context.projection, spy); - server.respondWith('GET', /map\?bbox/, + serverFetch.respondWith('GET', /map\?bbox/, [200, { 'Content-Type': 'text/xml' }, tileXML]); - server.respond(); + serverFetch.respond(); window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 20); + }, 50); }); it('#isDataLoaded', function(done) { expect(connection.isDataLoaded([-74.0444216, 40.6694299])).to.be.not.ok; connection.loadTiles(context.projection); - server.respondWith('GET', /map\?bbox/, + serverFetch.respondWith('GET', /map\?bbox/, [200, { 'Content-Type': 'text/xml' }, tileXML]); - server.respond(); + serverFetch.respond(); window.setTimeout(function() { expect(connection.isDataLoaded([-74.0444216, 40.6694299])).to.be.ok; done(); - }, 20); + }, 50); }); }); @@ -356,14 +313,6 @@ describe('iD.serviceOsm', function () { '' + ''; - beforeEach(function() { - server = sinon.fakeServer.create(); - }); - - afterEach(function() { - server.restore(); - }); - it('loads a node', function(done) { var id = 'n1'; connection.loadEntity(id, function(err, result) { @@ -372,9 +321,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1', [200, { 'Content-Type': 'text/xml' }, nodeXML]); - server.respond(); + serverFetch.respond(); }); it('loads a way', function(done) { @@ -385,9 +334,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/way/1/full', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/way/1/full', [200, { 'Content-Type': 'text/xml' }, wayXML]); - server.respond(); + serverFetch.respond(); }); it('does not ignore repeat requests', function(done) { @@ -400,12 +349,12 @@ describe('iD.serviceOsm', function () { expect(entity2).to.be.an.instanceOf(iD.osmNode); done(); }); - server.respond(); + serverFetch.respond(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1', [200, { 'Content-Type': 'text/xml' }, nodeXML]); - server.respond(); + serverFetch.respond(); }); }); @@ -420,14 +369,6 @@ describe('iD.serviceOsm', function () { '' + ''; - beforeEach(function() { - server = sinon.fakeServer.create(); - }); - - afterEach(function() { - server.restore(); - }); - it('loads a node', function(done) { var id = 'n1'; connection.loadEntityVersion(id, 1, function(err, result) { @@ -436,9 +377,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1/1', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1/1', [200, { 'Content-Type': 'text/xml' }, nodeXML]); - server.respond(); + serverFetch.respond(); }); it('loads a way', function(done) { @@ -449,9 +390,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/way/1/1', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/way/1/1', [200, { 'Content-Type': 'text/xml' }, wayXML]); - server.respond(); + serverFetch.respond(); }); it('does not ignore repeat requests', function(done) { @@ -464,25 +405,17 @@ describe('iD.serviceOsm', function () { expect(entity2).to.be.an.instanceOf(iD.osmNode); done(); }); - server.respond(); + serverFetch.respond(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1/1', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/node/1/1', [200, { 'Content-Type': 'text/xml' }, nodeXML]); - server.respond(); + serverFetch.respond(); }); }); describe('#loadMultiple', function () { - beforeEach(function() { - server = sinon.fakeServer.create(); - }); - - afterEach(function() { - server.restore(); - }); - it('loads nodes'); it('loads ways'); it('does not ignore repeat requests'); @@ -493,7 +426,6 @@ describe('iD.serviceOsm', function () { var userDetailsFn; beforeEach(function() { - server = sinon.fakeServer.create(); userDetailsFn = connection.userDetails; connection.userDetails = function (callback) { callback(undefined, { id: 1, displayName: 'Steve' }); @@ -501,7 +433,6 @@ describe('iD.serviceOsm', function () { }); afterEach(function() { - server.restore(); connection.userDetails = userDetailsFn; }); @@ -527,9 +458,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', + serverXHR.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', [200, { 'Content-Type': 'text/xml' }, changesetsXML]); - server.respond(); + serverXHR.respond(); }); it('excludes changesets without comment tag', function(done) { @@ -556,9 +487,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', + serverXHR.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', [200, { 'Content-Type': 'text/xml' }, changesetsXML]); - server.respond(); + serverXHR.respond(); }); it('excludes changesets with empty comment', function(done) { @@ -586,34 +517,31 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', + serverXHR.respondWith('GET', 'http://www.openstreetmap.org/api/0.6/changesets?user=1', [200, { 'Content-Type': 'text/xml' }, changesetsXML]); - server.respond(); + serverXHR.respond(); }); - }); describe('#caches', function() { - it('loads reset caches', function (done) { + it('loads reset caches', function () { var caches = connection.caches(); expect(caches.tile).to.have.all.keys(['toLoad','loaded','inflight','seen','rtree']); expect(caches.note).to.have.all.keys(['toLoad','loaded','inflight','inflightPost','note','closed','rtree']); expect(caches.user).to.have.all.keys(['toLoad','user']); - done(); }); describe('sets/gets caches', function() { - it('sets/gets a tile', function (done) { + it('sets/gets a tile', function () { var obj = { tile: { loaded: { '1,2,16': true, '3,4,16': true } } }; connection.caches(obj); expect(connection.caches().tile.loaded['1,2,16']).to.eql(true); expect(Object.keys(connection.caches().tile.loaded).length).to.eql(2); - done(); }); - it('sets/gets a note', function (done) { + it('sets/gets a note', function () { var note = iD.osmNote({ id: 1, loc: [0, 0] }); var note2 = iD.osmNote({ id: 2, loc: [0, 0] }); var obj = { @@ -622,10 +550,9 @@ describe('iD.serviceOsm', function () { connection.caches(obj); expect(connection.caches().note.note[note.id]).to.eql(note); expect(Object.keys(connection.caches().note.note).length).to.eql(2); - done(); }); - it('sets/gets a user', function (done) { + it('sets/gets a user', function () { var user = { id: 1, display_name: 'Name' }; var user2 = { id: 2, display_name: 'Name' }; var obj = { @@ -634,7 +561,6 @@ describe('iD.serviceOsm', function () { connection.caches(obj); expect(connection.caches().user.user[user.id]).to.eql(user); expect(Object.keys(connection.caches().user.user).length).to.eql(2); - done(); }); }); @@ -676,14 +602,14 @@ describe('iD.serviceOsm', function () { connection.on('loadedNotes', spy); connection.loadNotes(context.projection, {}); - server.respondWith('GET', /notes\?/, + serverFetch.respondWith('GET', /notes\?/, [200, { 'Content-Type': 'text/xml' }, notesXML ]); - server.respond(); + serverFetch.respond(); window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 20); + }, 50); }); }); @@ -696,6 +622,7 @@ describe('iD.serviceOsm', function () { .translate([-116508, 0]) // 10,0 .clipExtent([[0,0], dimensions]); }); + it('returns notes in the visible map area', function() { var notes = [ { minX: 10, minY: 0, maxX: 10, maxY: 0, data: { key: '0', loc: [10,0] } }, @@ -715,7 +642,7 @@ describe('iD.serviceOsm', function () { describe('#getNote', function() { - it('returns a note', function (done) { + it('returns a note', function () { var note = iD.osmNote({ id: 1, loc: [0, 0], }); var obj = { note: { note: { 1: note } } @@ -723,24 +650,22 @@ describe('iD.serviceOsm', function () { connection.caches(obj); var result = connection.getNote(1); expect(result).to.deep.equal(note); - done(); }); }); describe('#removeNote', function() { - it('removes a note that is new', function(done) { + it('removes a note that is new', function() { var note = iD.osmNote({ id: -1, loc: [0, 0], }); connection.replaceNote(note); connection.removeNote(note); var result = connection.getNote(-1); expect(result).to.eql(undefined); - done(); }); }); describe('#replaceNote', function() { - it('returns a new note', function (done) { + it('returns a new note', function () { var note = iD.osmNote({ id: 2, loc: [0, 0], }); var result = connection.replaceNote(note); expect(result.id).to.eql(2); @@ -749,10 +674,9 @@ describe('iD.serviceOsm', function () { var result_rtree = rtree.search({ 'minX': -1, 'minY': -1, 'maxX': 1, 'maxY': 1 }); expect(result_rtree.length).to.eql(1); expect(result_rtree[0].data).to.eql(note); - done(); }); - it('replaces a note', function (done) { + it('replaces a note', function () { var note = iD.osmNote({ id: 2, loc: [0, 0], }); connection.replaceNote(note); note.status = 'closed'; @@ -763,8 +687,6 @@ describe('iD.serviceOsm', function () { var result_rtree = rtree.search({ 'minX': -1, 'minY': -1, 'maxX': 1, 'maxY': 1 }); expect(result_rtree.length).to.eql(1); expect(result_rtree[0].data.status).to.eql('closed'); - - done(); }); }); @@ -787,15 +709,6 @@ describe('iD.serviceOsm', function () { '' + ''; - - beforeEach(function() { - server = sinon.fakeServer.create(); - }); - - afterEach(function() { - server.restore(); - }); - describe('#status', function() { it('gets API status', function(done) { connection.status(function(err, val) { @@ -803,9 +716,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/capabilities', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/capabilities', [200, { 'Content-Type': 'text/xml' }, capabilitiesXML]); - server.respond(); + serverFetch.respond(); }); }); @@ -817,9 +730,9 @@ describe('iD.serviceOsm', function () { done(); }); - server.respondWith('GET', 'http://www.openstreetmap.org/api/capabilities', + serverFetch.respondWith('GET', 'http://www.openstreetmap.org/api/capabilities', [200, { 'Content-Type': 'text/xml' }, capabilitiesXML]); - server.respond(); + serverFetch.respond(); }); }); diff --git a/test/spec/services/osm_wikibase.js b/test/spec/services/osm_wikibase.js index dc58ae094..ae05015d3 100644 --- a/test/spec/services/osm_wikibase.js +++ b/test/spec/services/osm_wikibase.js @@ -10,9 +10,9 @@ describe('iD.serviceOsmWikibase', function () { }); beforeEach(function () { + server = window.fakeFetch().create(); wikibase = iD.services.osmWikibase; wikibase.init(); - server = sinon.fakeServer.create(); }); afterEach(function () { @@ -273,7 +273,7 @@ describe('iD.serviceOsmWikibase', function () { }; describe('#getEntity', function () { - it('calls the given callback with the results of the getEntity data item query', function () { + it('calls the given callback with the results of the getEntity data item query', function (done) { var callback = sinon.spy(); wikibase.getEntity({key: 'amenity', value: 'parking', langCode: 'fr'}, callback); @@ -289,21 +289,24 @@ describe('iD.serviceOsmWikibase', function () { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - { - action: 'wbgetentities', - sites: 'wiki', - titles: 'Locale:fr|Key:amenity|Tag:amenity=parking', - languages: 'fr', - languagefallback: '1', - origin: '*', - format: 'json', - } - ); - expect(callback).to.have.been.calledWith(null, { - key: keyData({norm: true}), - tag: tagData({norm: true}) - }); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + { + action: 'wbgetentities', + sites: 'wiki', + titles: 'Locale:fr|Key:amenity|Tag:amenity=parking', + languages: 'fr', + languagefallback: '1', + origin: '*', + format: 'json', + } + ); + expect(callback).to.have.been.calledWith(null, { + key: keyData({norm: true}), + tag: tagData({norm: true}) + }); + done(); + }, 50); }); }); diff --git a/test/spec/services/streetside.js b/test/spec/services/streetside.js index 8221718d1..73d46f3cb 100644 --- a/test/spec/services/streetside.js +++ b/test/spec/services/streetside.js @@ -17,7 +17,7 @@ describe('iD.serviceStreetside', function() { .translate([-116508, 0]) // 10,0 .clipExtent([[0,0], dimensions]); - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); streetside = iD.services.streetside; streetside.reset(); }); @@ -49,7 +49,7 @@ describe('iD.serviceStreetside', function() { }); describe('#loadBubbles', function() { - it('fires loadedBubbles when bubbles are loaded', function() { + it('fires loadedBubbles when bubbles are loaded', function(done) { // adjust projection so that only one tile is fetched // (JSONP hack will return the same data for every fetch) context.projection @@ -79,10 +79,14 @@ describe('iD.serviceStreetside', function() { ]; streetside.loadBubbles(context.projection, 0); // 0 = don't fetch margin tiles - expect(spy).to.have.been.calledOnce; + + window.setTimeout(function() { + expect(spy).to.have.been.calledOnce; + done(); + }, 50); }); - it('does not load bubbles around null island', function() { + it('does not load bubbles around null island', function(done) { context.projection .scale(iD.geoZoomToScale(18)) .translate([0, 0]) @@ -110,7 +114,11 @@ describe('iD.serviceStreetside', function() { ]; streetside.loadBubbles(context.projection, 0); // 0 = don't fetch margin tiles - expect(spy).to.have.been.not.called; + + window.setTimeout(function() { + expect(spy).to.have.been.not.called; + done(); + }, 50); }); }); diff --git a/test/spec/services/taginfo.js b/test/spec/services/taginfo.js index 6f310e3bc..0f6843e88 100644 --- a/test/spec/services/taginfo.js +++ b/test/spec/services/taginfo.js @@ -11,7 +11,7 @@ describe('iD.serviceTaginfo', function() { }); beforeEach(function() { - server = sinon.fakeServer.create(); + server = window.fakeFetch().create(); taginfo = iD.services.taginfo; // prepopulate popular keys list with "name" @@ -22,7 +22,8 @@ describe('iD.serviceTaginfo', function() { '{"data":[{"count_all":56136034,"key":"name","count_all_fraction":0.0132}]}'] ); server.respond(); - server = sinon.fakeServer.create(); + server.restore(); + server = window.fakeFetch().create(); }); afterEach(function() { @@ -35,7 +36,7 @@ describe('iD.serviceTaginfo', function() { describe('#keys', function() { - it('calls the given callback with the results of the keys query', function() { + it('calls the given callback with the results of the keys query', function(done) { var callback = sinon.spy(); taginfo.keys({query: 'amen'}, callback); @@ -45,15 +46,18 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {query: 'amen', page: '1', rp: '10', sortname: 'count_all', sortorder: 'desc', lang: 'en'} - ); - expect(callback).to.have.been.calledWith( - null, [{'title':'amenity', 'value':'amenity'}] - ); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {query: 'amen', page: '1', rp: '10', sortname: 'count_all', sortorder: 'desc', lang: 'en'} + ); + expect(callback).to.have.been.calledWith( + null, [{'title':'amenity', 'value':'amenity'}] + ); + done(); + }, 50); }); - it('includes popular keys', function() { + it('includes popular keys', function(done) { var callback = sinon.spy(); taginfo.keys({query: 'amen'}, callback); @@ -64,12 +68,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'title':'amenity', 'value':'amenity'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'title':'amenity', 'value':'amenity'}] + ); + done(); + }, 50); }); - it('includes popular keys with an entity type filter', function() { + it('includes popular keys with an entity type filter', function(done) { var callback = sinon.spy(); taginfo.keys({query: 'amen', filter: 'nodes'}, callback); @@ -80,12 +87,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'title':'amenity', 'value':'amenity'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'title':'amenity', 'value':'amenity'}] + ); + done(); + }, 50); }); - it('includes unpopular keys with a wiki page', function() { + it('includes unpopular keys with a wiki page', function(done) { var callback = sinon.spy(); taginfo.keys({query: 'amen'}, callback); @@ -96,13 +106,16 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith(null, [ - {'title':'amenity', 'value':'amenity'}, - {'title':'amenityother', 'value':'amenityother'} - ]); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith(null, [ + {'title':'amenity', 'value':'amenity'}, + {'title':'amenityother', 'value':'amenityother'} + ]); + done(); + }, 50); }); - it('sorts keys with \':\' below keys without \':\'', function() { + it('sorts keys with \':\' below keys without \':\'', function(done) { var callback = sinon.spy(); taginfo.keys({query: 'ref'}, callback); @@ -113,14 +126,17 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'title':'ref', 'value':'ref'},{'title':'ref:bag', 'value':'ref:bag'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'title':'ref', 'value':'ref'},{'title':'ref:bag', 'value':'ref:bag'}] + ); + done(); + }, 50); }); }); describe('#multikeys', function() { - it('calls the given callback with the results of the multikeys query', function() { + it('calls the given callback with the results of the multikeys query', function(done) { var callback = sinon.spy(); taginfo.multikeys({query: 'recycling:'}, callback); @@ -130,15 +146,18 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {query: 'recycling:', page: '1', rp: '25', sortname: 'count_all', sortorder: 'desc', lang: 'en'} - ); - expect(callback).to.have.been.calledWith( - null, [{'title':'recycling:glass', 'value':'recycling:glass'}] - ); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {query: 'recycling:', page: '1', rp: '25', sortname: 'count_all', sortorder: 'desc', lang: 'en'} + ); + expect(callback).to.have.been.calledWith( + null, [{'title':'recycling:glass', 'value':'recycling:glass'}] + ); + done(); + }, 50); }); - it('excludes multikeys with extra colons', function() { + it('excludes multikeys with extra colons', function(done) { var callback = sinon.spy(); taginfo.multikeys({query: 'service:bicycle:'}, callback); @@ -149,12 +168,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'title':'service:bicycle:retail', 'value':'service:bicycle:retail'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'title':'service:bicycle:retail', 'value':'service:bicycle:retail'}] + ); + done(); + }, 50); }); - it('excludes multikeys with wrong prefix', function() { + it('excludes multikeys with wrong prefix', function(done) { var callback = sinon.spy(); taginfo.multikeys({query: 'service:bicycle:'}, callback); @@ -165,14 +187,17 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'title':'service:bicycle:retail', 'value':'service:bicycle:retail'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'title':'service:bicycle:retail', 'value':'service:bicycle:retail'}] + ); + done(); + }, 50); }); }); describe('#values', function() { - it('calls the given callback with the results of the values query', function() { + it('calls the given callback with the results of the values query', function(done) { var callback = sinon.spy(); taginfo.values({key: 'amenity', query: 'par'}, callback); @@ -182,15 +207,18 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {key: 'amenity', query: 'par', page: '1', rp: '25', sortname: 'count_all', sortorder: 'desc', lang: 'en'} - ); - expect(callback).to.have.been.calledWith( - null, [{'value':'parking','title':'A place for parking cars'}] - ); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {key: 'amenity', query: 'par', page: '1', rp: '25', sortname: 'count_all', sortorder: 'desc', lang: 'en'} + ); + expect(callback).to.have.been.calledWith( + null, [{'value':'parking','title':'A place for parking cars'}] + ); + done(); + }, 50); }); - it('includes popular values', function() { + it('includes popular values', function(done) { var callback = sinon.spy(); taginfo.values({key: 'amenity', query: 'par'}, callback); @@ -201,12 +229,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'value':'parking','title':'A place for parking cars'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'value':'parking','title':'A place for parking cars'}] + ); + done(); + }, 50); }); - it('does not get values for extremely unpopular keys', function() { + it('does not get values for extremely unpopular keys', function(done) { var callback = sinon.spy(); taginfo.values({key: 'name', query: 'ste'}, callback); @@ -217,10 +248,13 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith(null, []); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith(null, []); + done(); + }, 50); }); - it('excludes values with capital letters and some punctuation', function() { + it('excludes values with capital letters and some punctuation', function(done) { var callback = sinon.spy(); taginfo.values({key: 'amenity', query: 'par'}, callback); @@ -234,12 +268,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'value':'parking','title':'A place for parking cars'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'value':'parking','title':'A place for parking cars'}] + ); + done(); + }, 50); }); - it('includes network values with capital letters and some punctuation', function() { + it('includes network values with capital letters and some punctuation', function(done) { var callback = sinon.spy(); taginfo.values({key: 'network', query: 'us'}, callback); @@ -253,16 +290,19 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith(null, [ - {'value':'US:TX:FM','title':'Farm to Market Roads in the U.S. state of Texas.'}, - {'value':'US:KY','title':'Primary and secondary state highways in the U.S. state of Kentucky.'}, - {'value':'US:US','title':'U.S. routes in the United States.'}, - {'value':'US:I','title':'Interstate highways in the United States.'}, - {'value':'US:MD','title':'State highways in the U.S. state of Maryland.'} - ]); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith(null, [ + {'value':'US:TX:FM','title':'Farm to Market Roads in the U.S. state of Texas.'}, + {'value':'US:KY','title':'Primary and secondary state highways in the U.S. state of Kentucky.'}, + {'value':'US:US','title':'U.S. routes in the United States.'}, + {'value':'US:I','title':'Interstate highways in the United States.'}, + {'value':'US:MD','title':'State highways in the U.S. state of Maryland.'} + ]); + done(); + }, 50); }); - it('includes biological genus values with capital letters', function() { + it('includes biological genus values with capital letters', function(done) { var callback = sinon.spy(); taginfo.values({key: 'genus', query: 'qu'}, callback); @@ -272,12 +312,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'value':'Quercus','title':'Oak'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'value':'Quercus','title':'Oak'}] + ); + done(); + }, 50); }); - it('includes biological taxon values with capital letters', function() { + it('includes biological taxon values with capital letters', function(done) { var callback = sinon.spy(); taginfo.values({key: 'taxon', query: 'qu'}, callback); @@ -287,12 +330,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'value':'Quercus robur','title':'Oak'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'value':'Quercus robur','title':'Oak'}] + ); + done(); + }, 50); }); - it('includes biological species values with capital letters', function() { + it('includes biological species values with capital letters', function(done) { var callback = sinon.spy(); taginfo.values({key: 'species', query: 'qu'}, callback); @@ -302,14 +348,17 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(callback).to.have.been.calledWith( - null, [{'value':'Quercus robur','title':'Oak'}] - ); + window.setTimeout(function() { + expect(callback).to.have.been.calledWith( + null, [{'value':'Quercus robur','title':'Oak'}] + ); + done(); + }, 50); }); }); describe('#roles', function() { - it('calls the given callback with the results of the roles query', function() { + it('calls the given callback with the results of the roles query', function(done) { var callback = sinon.spy(); taginfo.roles({rtype: 'route', query: 's', geometry: 'relation'}, callback); @@ -320,18 +369,21 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {rtype: 'route', query: 's', page: '1', rp: '25', sortname: 'count_relation_members', sortorder: 'desc', lang: 'en'} - ); - expect(callback).to.have.been.calledWith(null, [ - {'value': 'stop', 'title': 'stop'}, - {'value': 'south', 'title': 'south'} - ]); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {rtype: 'route', query: 's', page: '1', rp: '25', sortname: 'count_relation_members', sortorder: 'desc', lang: 'en'} + ); + expect(callback).to.have.been.calledWith(null, [ + {'value': 'stop', 'title': 'stop'}, + {'value': 'south', 'title': 'south'} + ]); + done(); + }, 50); }); }); describe('#docs', function() { - it('calls the given callback with the results of the docs query', function() { + it('calls the given callback with the results of the docs query', function(done) { var callback = sinon.spy(); taginfo.docs({key: 'amenity', value: 'parking'}, callback); @@ -341,12 +393,15 @@ describe('iD.serviceTaginfo', function() { ); server.respond(); - expect(query(server.requests[0].url)).to.eql( - {key: 'amenity', value: 'parking'} - ); - expect(callback).to.have.been.calledWith( - null, [{'on_way':false,'lang':'en','on_area':true,'image':'File:Car park2.jpg'}] - ); + window.setTimeout(function() { + expect(query(server.requests()[0].url)).to.eql( + {key: 'amenity', value: 'parking'} + ); + expect(callback).to.have.been.calledWith( + null, [{'on_way':false,'lang':'en','on_area':true,'image':'File:Car park2.jpg'}] + ); + done(); + }, 50); }); }); diff --git a/test/spec/ui/fields/wikipedia.js b/test/spec/ui/fields/wikipedia.js index 393d5c6e2..b8c5719d3 100644 --- a/test/spec/ui/fields/wikipedia.js +++ b/test/spec/ui/fields/wikipedia.js @@ -22,8 +22,10 @@ describe('iD.uiFieldWikipedia', function() { } } - function createServer(options) { - var server = sinon.fakeServer.create(options); + function createServer(options) { // eslint-disable-line no-unused-vars + // note - currently skipping the tests that use `options` to delay responses + // var server = sinon.fakeServer.create(options); + var server = window.fakeFetch().create(); server.respondWith('GET', new RegExp('\/w\/api\.php.*action=wbgetentities'), [200, { 'Content-Type': 'application/json' }, From 452eb8b43ae238bbac48170dbcffeefcc34c0672 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 27 Apr 2019 17:41:58 -0400 Subject: [PATCH 11/13] Bump timeouts on streetlevel loadimage-type tests This isn't perfect but might avoid spurious test failures. --- test/spec/services/mapillary.js | 12 ++++++------ test/spec/services/openstreetcam.js | 6 +++--- test/spec/services/streetside.js | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/test/spec/services/mapillary.js b/test/spec/services/mapillary.js index a6f21c344..94bc15121 100644 --- a/test/spec/services/mapillary.js +++ b/test/spec/services/mapillary.js @@ -74,7 +74,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 50); + }, 200); }); it('does not load images around null island', function(done) { @@ -98,7 +98,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.not.called; done(); - }, 50); + }, 200); }); it.skip('loads multiple pages of image results', function(done) { @@ -139,7 +139,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.calledTwice; done(); - }, 50); + }, 200); }); }); @@ -169,7 +169,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 50); + }, 200); }); it('does not load signs around null island', function(done) { @@ -197,7 +197,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.not.called; done(); - }, 50); + }, 200); }); it.skip('loads multiple pages of signs results', function(done) { @@ -245,7 +245,7 @@ describe('iD.serviceMapillary', function() { window.setTimeout(function() { expect(spy).to.have.been.calledTwice; done(); - }, 50); + }, 200); }); }); diff --git a/test/spec/services/openstreetcam.js b/test/spec/services/openstreetcam.js index a12c48291..f64f27142 100644 --- a/test/spec/services/openstreetcam.js +++ b/test/spec/services/openstreetcam.js @@ -105,7 +105,7 @@ describe('iD.serviceOpenstreetcam', function() { window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 50); + }, 200); }); it('does not load images around null island', function(done) { @@ -163,7 +163,7 @@ describe('iD.serviceOpenstreetcam', function() { window.setTimeout(function() { expect(spy).to.have.been.not.called; done(); - }, 50); + }, 200); }); it.skip('loads multiple pages of image results', function(done) { @@ -231,7 +231,7 @@ describe('iD.serviceOpenstreetcam', function() { window.setTimeout(function() { expect(spy).to.have.been.calledTwice; done(); - }, 50); + }, 200); }); }); diff --git a/test/spec/services/streetside.js b/test/spec/services/streetside.js index 73d46f3cb..9231f0a31 100644 --- a/test/spec/services/streetside.js +++ b/test/spec/services/streetside.js @@ -83,7 +83,7 @@ describe('iD.serviceStreetside', function() { window.setTimeout(function() { expect(spy).to.have.been.calledOnce; done(); - }, 50); + }, 200); }); it('does not load bubbles around null island', function(done) { @@ -118,7 +118,7 @@ describe('iD.serviceStreetside', function() { window.setTimeout(function() { expect(spy).to.have.been.not.called; done(); - }, 50); + }, 200); }); }); From af31b84b1d01824177f1f610fa353e0b2b2fb8e6 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 29 Apr 2019 13:03:01 -0400 Subject: [PATCH 12/13] Keep track of whether requests have been processed This lets us call `respond` multiple times, which is useful for the tests which fetch data in pages. --- test/spec/spec_helpers.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/spec/spec_helpers.js b/test/spec/spec_helpers.js index 26769ef52..830c59a2e 100644 --- a/test/spec/spec_helpers.js +++ b/test/spec/spec_helpers.js @@ -69,7 +69,7 @@ window.fakeFetch = function() { options = Object.assign({ method: 'get', headers: {}, body: '' }, options); return new Promise(function(resolve, reject) { _requests.push({ - url: url, options: options, resolve: resolve, reject: reject + url: url, options: options, resolve: resolve, reject: reject, processed: false }); }); } @@ -136,6 +136,8 @@ window.fakeFetch = function() { respond: function () { _requests.forEach(function(request) { + if (request.processed) return; + var didMatch = false; for (var i = 0; i < _responders.length; i++) { var responder = _responders[i]; @@ -150,11 +152,13 @@ window.fakeFetch = function() { } if (didMatch) { + request.processed = true; request.resolve(responder.respond()); break; } } if (!didMatch) { + request.processed = true; request.reject(new Response( new Blob(['404'], { type: 'text/plain' }), { status: 404, statusText: 'Not Found' } From e981cd5dd569689fa451b56cf1dec48c2c78bf36 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 29 Apr 2019 15:31:08 -0400 Subject: [PATCH 13/13] Switch mapillary and openstreetcam tests to work async - can't reliably use sinon.spy to tell whether a thing has been called, so we listen for events instead and check server.requests() - make sure to request the next page before dispatching `loadedImages` so we can `server.respond()` to the request in the event handler if we want to - also moves `localeDateString` in the openstreetcam service from parsing code to display code, because it's very slow (we can just do this for the images we look at, instead of all images we fetch) --- modules/services/mapillary.js | 14 +-- modules/services/openstreetcam.js | 32 +++---- modules/svg/mapillary_signs.js | 2 +- test/spec/services/mapillary.js | 132 +++++++++++++--------------- test/spec/services/openstreetcam.js | 90 ++++++------------- 5 files changed, 115 insertions(+), 155 deletions(-) diff --git a/modules/services/mapillary.js b/modules/services/mapillary.js index 101728703..df4f2a9ed 100644 --- a/modules/services/mapillary.js +++ b/modules/services/mapillary.js @@ -180,18 +180,18 @@ function loadNextTilePage(which, currZoom, url, tile) { cache.rtree.load(features); } - if (which === 'images' || which === 'sequences') { - dispatch.call('loadedImages'); - } else if (which === 'map_features') { - dispatch.call('loadedSigns'); - } - if (data.features.length === maxResults) { // more pages to load cache.nextPage[tile.id] = nextPage + 1; loadNextTilePage(which, currZoom, url, tile); } else { cache.nextPage[tile.id] = Infinity; // no more pages to load } + + if (which === 'images' || which === 'sequences') { + dispatch.call('loadedImages'); + } else if (which === 'map_features') { + dispatch.call('loadedSigns'); + } }) .catch(function() { cache.loaded[id] = true; @@ -326,7 +326,7 @@ export default { }, - loadSigns: function(context, projection) { + loadSigns: function(projection) { // if we are looking at signs, we'll actually need to fetch images too loadTiles('images', apibase + 'images?', projection); loadTiles('map_features', apibase + 'map_features?layers=trafficsigns&min_nbr_image_detections=1&', projection); diff --git a/modules/services/openstreetcam.js b/modules/services/openstreetcam.js index 366632047..768d41da3 100644 --- a/modules/services/openstreetcam.js +++ b/modules/services/openstreetcam.js @@ -95,15 +95,6 @@ function loadNextTilePage(which, currZoom, url, tile) { throw new Error('No Data'); } - function localeDateString(s) { - if (!s) return null; - var detected = utilDetect(); - var options = { day: 'numeric', month: 'short', year: 'numeric' }; - var d = new Date(s); - if (isNaN(d.getTime())) return null; - return d.toLocaleDateString(detected.locale, options); - } - var features = data.currentPageItems.map(function(item) { var loc = [+item.lng, +item.lat]; var d; @@ -113,7 +104,7 @@ function loadNextTilePage(which, currZoom, url, tile) { loc: loc, key: item.id, ca: +item.heading, - captured_at: localeDateString(item.shot_date || item.date_added), + captured_at: (item.shot_date || item.date_added), captured_by: item.username, imagePath: item.lth_name, sequence_id: item.sequence_id, @@ -136,16 +127,16 @@ function loadNextTilePage(which, currZoom, url, tile) { cache.rtree.load(features); - if (which === 'images') { - dispatch.call('loadedImages'); - } - if (data.currentPageItems.length === maxResults) { // more pages to load cache.nextPage[tile.id] = nextPage + 1; loadNextTilePage(which, currZoom, url, tile); } else { cache.nextPage[tile.id] = Infinity; // no more pages to load } + + if (which === 'images') { + dispatch.call('loadedImages'); + } }) .catch(function() { cache.loaded[id] = true; @@ -439,7 +430,7 @@ export default { attribution .append('span') .attr('class', 'captured_at') - .text(d.captured_at); + .text(localeDateString(d.captured_at)); attribution .append('span') @@ -453,7 +444,18 @@ export default { .attr('href', 'https://openstreetcam.org/details/' + d.sequence_id + '/' + d.sequence_index) .text('openstreetcam.org'); } + return this; + + + function localeDateString(s) { + if (!s) return null; + var detected = utilDetect(); + var options = { day: 'numeric', month: 'short', year: 'numeric' }; + var d = new Date(s); + if (isNaN(d.getTime())) return null; + return d.toLocaleDateString(detected.locale, options); + } }, diff --git a/modules/svg/mapillary_signs.js b/modules/svg/mapillary_signs.js index cffffdce7..b846fb538 100644 --- a/modules/svg/mapillary_signs.js +++ b/modules/svg/mapillary_signs.js @@ -142,7 +142,7 @@ export function svgMapillarySigns(projection, context, dispatch) { if (service && ~~context.map().zoom() >= minZoom) { editOn(); update(); - service.loadSigns(context, projection); + service.loadSigns(projection); } else { editOff(); } diff --git a/test/spec/services/mapillary.js b/test/spec/services/mapillary.js index 94bc15121..03d741d83 100644 --- a/test/spec/services/mapillary.js +++ b/test/spec/services/mapillary.js @@ -55,11 +55,13 @@ describe('iD.serviceMapillary', function() { describe('#loadImages', function() { it('fires loadedImages when images are loaded', function(done) { - var spy = sinon.spy(); - mapillary.on('loadedImages', spy); + mapillary.on('loadedImages', function() { + expect(server.requests().length).to.eql(2); // 1 images, 1 sequences + done(); + }); + mapillary.loadImages(context.projection); - var match = /images/; var features = [{ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, @@ -67,23 +69,18 @@ describe('iD.serviceMapillary', function() { }]; var response = { type: 'FeatureCollection', features: features }; - server.respondWith('GET', match, + server.respondWith('GET', /images/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledOnce; - done(); - }, 200); }); it('does not load images around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); + mapillary.on('loadedImages', spy); mapillary.loadImages(context.projection); - var match = /images/; var features = [{ type: 'Feature', geometry: { type: 'Point', coordinates: [0,0] }, @@ -91,70 +88,72 @@ describe('iD.serviceMapillary', function() { }]; var response = { type: 'FeatureCollection', features: features }; - server.respondWith('GET', match, + server.respondWith('GET', /images/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); window.setTimeout(function() { expect(spy).to.have.been.not.called; + expect(server.requests().length).to.eql(0); // no tile requests of any kind done(); }, 200); }); - it.skip('loads multiple pages of image results', function(done) { - var spy = sinon.spy(); - mapillary.on('loadedImages', spy); + it('loads multiple pages of image results', function(done) { + var calls = 0; + mapillary.on('loadedImages', function() { + server.respond(); // respond to new fetches + if (++calls === 2) { + expect(server.requests().length).to.eql(3); // 2 images, 1 sequences + done(); + } + }); + mapillary.loadImages(context.projection); var features0 = []; var features1 = []; - var i; + var i, key; for (i = 0; i < 1000; i++) { + key = String(i); features0.push({ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, - properties: { ca: 90, key: String(i) } + properties: { ca: 90, key: key } }); } for (i = 0; i < 500; i++) { + key = String(1000 + i); features1.push({ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, - properties: { ca: 90, key: String(1000 + i) } + properties: { ca: 90, key: key } }); } - var match0 = /page=0/; var response0 = { type: 'FeatureCollection', features: features0 }; - var match1 = /page=1/; var response1 = { type: 'FeatureCollection', features: features1 }; - server.respondWith('GET', match0, + server.respondWith('GET', /\/images\?.*&page=0/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response0) ]); - server.respondWith('GET', match1, + server.respondWith('GET', /\/images\?.*&page=1/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response1) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledTwice; - done(); - }, 200); }); }); describe('#loadSigns', function() { it('fires loadedSigns when signs are loaded', function(done) { - var spy = sinon.spy(); - mapillary.on('loadedSigns', spy); - mapillary.loadSigns(context, context.projection); + mapillary.on('loadedSigns', function() { + expect(server.requests().length).to.eql(3); // 1 images, 1 map_features, 1 image_detections + done(); + }); - var match = /map_features/; - var detections = [{ - detection_key: '0', - image_key: '0' - }]; + mapillary.loadSigns(context.projection); + + var detections = [{ detection_key: '0', image_key: '0' }]; var features = [{ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, @@ -162,27 +161,19 @@ describe('iD.serviceMapillary', function() { }]; var response = { type: 'FeatureCollection', features: features }; - server.respondWith('GET', match, + server.respondWith('GET', /map_features/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledOnce; - done(); - }, 200); }); it('does not load signs around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); - mapillary.on('loadedSigns', spy); - mapillary.loadSigns(context, context.projection); - var match = /map_features/; - var detections = [{ - detection_key: '0', - image_key: '0' - }]; + mapillary.on('loadedSigns', spy); + mapillary.loadSigns(context.projection); + + var detections = [{ detection_key: '0', image_key: '0' }]; var features = [{ type: 'Feature', geometry: { type: 'Point', coordinates: [0,0] }, @@ -190,62 +181,60 @@ describe('iD.serviceMapillary', function() { }]; var response = { type: 'FeatureCollection', features: features }; - server.respondWith('GET', match, + server.respondWith('GET', /map_features/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); window.setTimeout(function() { expect(spy).to.have.been.not.called; + expect(server.requests().length).to.eql(0); // no tile requests of any kind done(); }, 200); }); - it.skip('loads multiple pages of signs results', function(done) { - var spy = sinon.spy(); - mapillary.on('loadedSigns', spy); - mapillary.loadSigns(context, context.projection); + it('loads multiple pages of signs results', function(done) { + var calls = 0; + mapillary.on('loadedSigns', function() { + server.respond(); // respond to new fetches + if (++calls === 2) { + expect(server.requests().length).to.eql(4); // 2 images, 1 map_features, 1 image_detections + done(); + } + }); + + mapillary.loadSigns(context.projection); - var rects = [{ - package: 'trafficsign', - rect: [ 0.805, 0.463, 0.833, 0.502 ], - length: 4, - score: '1.27', - type: 'regulatory--maximum-speed-limit-65--us' - }]; var features0 = []; var features1 = []; - var i; + var i, key, detections; for (i = 0; i < 1000; i++) { + key = String(i); + detections = [{ detection_key: key, image_key: key }]; features0.push({ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, - properties: { rects: rects, key: String(i) } + properties: { detections: detections, key: key, value: 'not-in-set' } }); } for (i = 0; i < 500; i++) { + key = String(1000 + i); + detections = [{ detection_key: key, image_key: key }]; features1.push({ type: 'Feature', geometry: { type: 'Point', coordinates: [10,0] }, - properties: { rects: rects, key: String(1000 + i) } + properties: { detections: detections, key: key, value: 'not-in-set' } }); } - var match0 = /page=0/; var response0 = { type: 'FeatureCollection', features: features0 }; - var match1 = /page=1/; var response1 = { type: 'FeatureCollection', features: features1 }; - server.respondWith('GET', match0, + server.respondWith('GET', /\/map_features\?.*&page=0/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response0) ]); - server.respondWith('GET', match1, + server.respondWith('GET', /\/map_features\?.*&page=1/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(response1) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledTwice; - done(); - }, 200); }); }); @@ -283,6 +272,7 @@ describe('iD.serviceMapillary', function() { }); }); + describe('#signs', function() { it('returns signs in the visible map area', function() { var detections = [{ diff --git a/test/spec/services/openstreetcam.js b/test/spec/services/openstreetcam.js index f64f27142..5553c4a49 100644 --- a/test/spec/services/openstreetcam.js +++ b/test/spec/services/openstreetcam.js @@ -52,8 +52,11 @@ describe('iD.serviceOpenstreetcam', function() { describe('#loadImages', function() { it('fires loadedImages when images are loaded', function(done) { - var spy = sinon.spy(); - openstreetcam.on('loadedImages', spy); + openstreetcam.on('loadedImages', function() { + expect(server.requests().length).to.eql(1); // 1 nearby-photos + done(); + }); + openstreetcam.loadImages(context.projection); var data = { @@ -101,16 +104,12 @@ describe('iD.serviceOpenstreetcam', function() { server.respondWith('POST', /nearby-photos/, [200, { 'Content-Type': 'application/json' }, JSON.stringify(data) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledOnce; - done(); - }, 200); }); it('does not load images around null island', function(done) { var spy = sinon.spy(); context.projection.translate([0,0]); + openstreetcam.on('loadedImages', spy); openstreetcam.loadImages(context.projection); @@ -162,76 +161,45 @@ describe('iD.serviceOpenstreetcam', function() { window.setTimeout(function() { expect(spy).to.have.been.not.called; + expect(server.requests().length).to.eql(0); // no tile requests of any kind done(); }, 200); }); - it.skip('loads multiple pages of image results', function(done) { - var spy = sinon.spy(); - openstreetcam.on('loadedImages', spy); + it('loads multiple pages of image results', function(done) { + openstreetcam.on('loadedImages', function() { + expect(server.requests().length).to.eql(2); // 2 nearby-photos + done(); + }); + openstreetcam.loadImages(context.projection); - var features0 = [], - features1 = [], - i; - - for (i = 0; i < 1000; i++) { - features0.push({ - id: String(i), + var features = []; + for (var i = 0; i < 1000; i++) { + var key = String(i); + features.push({ + id: key, sequence_id: '100', - sequence_index: String(i), + sequence_index: key, lat: '10', lng: '0', - name: 'storage6\/files\/photo\/foo' + String(i) +'.jpg', - lth_name: 'storage6\/files\/photo\/lth\/foo' + String(i) +'.jpg', - th_name: 'storage6\/files\/photo\/th\/foo' + String(i) +'.jpg', - shot_date: '2017-09-24 23:58:07', - heading: '90', - username: 'test' - }); - } - for (i = 0; i < 500; i++) { - features1.push({ - id: String(i), - sequence_id: '100', - sequence_index: String(1000 + i), - lat: '10', - lng: '0', - name: 'storage6\/files\/photo\/foo' + String(1000 + i) +'.jpg', - lth_name: 'storage6\/files\/photo\/lth\/foo' + String(1000 + i) +'.jpg', - th_name: 'storage6\/files\/photo\/th\/foo' + String(1000 + i) +'.jpg', + name: 'storage6\/files\/photo\/foo' + key + '.jpg', + lth_name: 'storage6\/files\/photo\/lth\/foo' + key + '.jpg', + th_name: 'storage6\/files\/photo\/th\/foo' + key + '.jpg', shot_date: '2017-09-24 23:58:07', heading: '90', username: 'test' }); } + var response = { + status: { apiCode: '600', httpCode: 200, httpMessage: 'Success' }, + currentPageItems: features, + totalFilteredItems: ['1000'] + }; - var response0 = { - status: { apiCode: '600', httpCode: 200, httpMessage: 'Success' }, - currentPageItems: [features0], - totalFilteredItems: ['1000'] - }, - response1 = { - status: { apiCode: '600', httpCode: 200, httpMessage: 'Success' }, - currentPageItems: [features1], - totalFilteredItems: ['500'] - }; - - server.respondWith('POST', /nearby-photos/, function (request) { - var response; - if (request.requestBody.match(/page=1/) !== null) { - response = JSON.stringify(response0); - } else if (request.requestBody.match(/page=2/) !== null) { - response = JSON.stringify(response1); - } - request.respond(200, {'Content-Type': 'application/json'}, response); - }); + server.respondWith('POST', /nearby-photos/, + [200, { 'Content-Type': 'application/json' }, JSON.stringify(response) ]); server.respond(); - - window.setTimeout(function() { - expect(spy).to.have.been.calledTwice; - done(); - }, 200); }); });