Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace JSONP with XHR where possible #5123

Merged
merged 4 commits into from
Jul 5, 2018
Merged

Replace JSONP with XHR where possible #5123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4cc080b
Use XHR to fetch imagery metadata instead of JSONP
tomhughes Jul 4, 2018
1ab7cf7
Use XHR to access wikipedia API instead of JSONP
tomhughes Jul 4, 2018
3819fd6
Use XHR to access wikidata API instead of JSONP
tomhughes Jul 4, 2018
d617d9c
Fix wikipedia UI tests to mock XHR instead of JSONP
tomhughes Jul 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions modules/renderer/background_source.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
geoSphericalDistance
} from '../geo';

import { jsonpRequest } from '../util/jsonp_request';
import { utilDetect } from '../util/detect';


Expand Down Expand Up @@ -217,12 +216,12 @@ 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 + '&jsonp={callback}';
key;
var cache = {};
var inflight = {};
var providers = [];

jsonpRequest(url, function(json) {
d3_json(url, function(err, json) {
providers = json.resourceSets[0].resources[0].imageryProviders.map(function(provider) {
return {
attribution: provider.attribution,
Expand Down Expand Up @@ -257,7 +256,7 @@ rendererBackgroundSource.Bing = function(data, dispatch) {
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 + '&jsonp={callback}';
'?zl=' + zoom + '&key=' + key;

if (inflight[tileId]) return;

Expand All @@ -269,10 +268,15 @@ rendererBackgroundSource.Bing = function(data, dispatch) {
}

inflight[tileId] = true;
jsonpRequest(url, function(result) {
d3_json(url, function(error, result) {
delete inflight[tileId];

var err = (!result && 'Unknown Error') || result.errorDetails;
var err;
if (error) {
err = error;
} else if (!result && 'Unknown Error') {
err = result.errorDetails;
}
if (err) {
return callback(err);
} else {
Expand Down Expand Up @@ -383,7 +387,7 @@ rendererBackgroundSource.Esri = function(data) {
url = 'https://serviceslab.arcgisonline.com/arcgis/rest/services/Clarity_World_Imagery/MapServer/';
}

url += metadataLayer + '/query?returnGeometry=false&geometry=' + centerPoint + '&inSR=4326&geometryType=esriGeometryPoint&outFields=*&f=json&callback={callback}';
url += metadataLayer + '/query?returnGeometry=false&geometry=' + centerPoint + '&inSR=4326&geometryType=esriGeometryPoint&outFields=*&f=json';

if (!cache[tileId]) {
cache[tileId] = {};
Expand Down Expand Up @@ -411,11 +415,13 @@ rendererBackgroundSource.Esri = function(data) {

} else {
inflight[tileId] = true;
jsonpRequest(url, function(result) {
d3_json(url, function(error, result) {
delete inflight[tileId];

var err;
if (!result) {
if (error) {
err = error;
} else if (!result) {
err = 'Unknown Error';
} else if (result.features && result.features.length < 1) {
err = 'No Results';
Expand Down
11 changes: 6 additions & 5 deletions modules/services/wikidata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { jsonpRequest } from '../util/jsonp_request';
import { json as d3_json } from 'd3-request';

import { utilQsString } from '../util';


Expand All @@ -19,15 +20,15 @@ export default {
}

lang = lang || 'en';
jsonpRequest(endpoint + utilQsString({
d3_json(endpoint + utilQsString({
action: 'wbgetentities',
format: 'json',
sites: lang.replace(/-/g, '_') + 'wiki',
titles: title,
languages: 'en', // shrink response by filtering to one language
callback: '{callback}'
}), function(data) {
if (!data || data.error) {
origin: '*'
}), function(err, data) {
if (err || !data || data.error) {
callback('', {});
} else {
callback(title, data.entities || {});
Expand Down
27 changes: 14 additions & 13 deletions modules/services/wikipedia.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { jsonpRequest } from '../util/jsonp_request';
import { json as d3_json } from 'd3-request';

import { utilQsString } from '../util';


Expand All @@ -17,17 +18,17 @@ export default {
}

lang = lang || 'en';
jsonpRequest(endpoint.replace('en', lang) +
d3_json(endpoint.replace('en', lang) +
utilQsString({
action: 'query',
list: 'search',
srlimit: '10',
srinfo: 'suggestion',
format: 'json',
callback: '{callback}',
origin: '*',
srsearch: query
}), function(data) {
if (!data || !data.query || !data.query.search || data.error) {
}), 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; });
Expand All @@ -45,16 +46,16 @@ export default {
}

lang = lang || 'en';
jsonpRequest(endpoint.replace('en', lang) +
d3_json(endpoint.replace('en', lang) +
utilQsString({
action: 'opensearch',
namespace: 0,
suggest: '',
format: 'json',
callback: '{callback}',
origin: '*',
search: query
}), function(data) {
if (!data || data.error) {
}), function(err, data) {
if (err || !data || data.error) {
callback('', []);
} else {
callback(data[0], data[1] || []);
Expand All @@ -70,16 +71,16 @@ export default {
return;
}

jsonpRequest(endpoint.replace('en', lang) +
d3_json(endpoint.replace('en', lang) +
utilQsString({
action: 'query',
prop: 'langlinks',
format: 'json',
callback: '{callback}',
origin: '*',
lllimit: 500,
titles: title
}), function(data) {
if (!data || !data.query || !data.query.pages || data.error) {
}), 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]],
Expand Down
33 changes: 22 additions & 11 deletions test/spec/ui/fields/wikipedia.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
describe('iD.uiFieldWikipedia', function() {
var entity, context, selection, field;
var entity, context, selection, field, server;

function changeTags(changed) {
var e = context.entity(entity.id),
Expand All @@ -22,6 +22,16 @@ describe('iD.uiFieldWikipedia', function() {
}
}

function createServer(options) {
var server = sinon.fakeServer.create(options);
server.respondWith('GET',
new RegExp('\/w\/api\.php.*action=wbgetentities'),
[200, { 'Content-Type': 'application/json' },
'{"entities":{"Q216353":{"id":"Q216353"}}}']
);
return server;
}

before(function() {
iD.services.wikipedia = iD.serviceWikipedia;
iD.services.wikidata = iD.serviceWikidata;
Expand All @@ -38,16 +48,11 @@ describe('iD.uiFieldWikipedia', function() {
context.history().merge([entity]);
selection = d3.select(document.createElement('div'));
field = context.presets().field('wikipedia');
window.JSONP_DELAY = 0;
window.JSONP_FIX = {
entities: {
Q216353: { id: 'Q216353' }
}
};
server = createServer({ respondImmediately: true });
});

afterEach(function() {
window.JSONP_FIX = undefined;
server.restore();
});

it('recognizes lang:title format', function() {
Expand Down Expand Up @@ -109,11 +114,13 @@ describe('iD.uiFieldWikipedia', function() {
var wikipedia = iD.uiFieldWikipedia(field, context).entity(entity);
wikipedia.on('change', changeTags);
selection.call(wikipedia);
window.JSONP_DELAY = 60;

var spy = sinon.spy();
wikipedia.on('change.spy', spy);

// Create an XHR server that will respond after 60ms
createServer({ autoRespond: true, autoRespondAfter: 60 });

// Set title to "Skip"
iD.utilGetSetValue(selection.selectAll('.wiki-lang'), 'Deutsch');
iD.utilGetSetValue(selection.selectAll('.wiki-title'), 'Skip');
Expand All @@ -123,21 +130,25 @@ describe('iD.uiFieldWikipedia', function() {
// t0
expect(context.entity(entity.id).tags.wikidata).to.be.undefined;

// Create a new XHR server that will respond after 60ms to
// separate requests after this point from those before
createServer({ autoRespond: true, autoRespondAfter: 60 });

// t30: graph change - Set title to "Title"
window.setTimeout(function() {
iD.utilGetSetValue(selection.selectAll('.wiki-title'), 'Title');
happen.once(selection.selectAll('.wiki-title').node(), { type: 'change' });
happen.once(selection.selectAll('.wiki-title').node(), { type: 'blur' });
}, 30);

// t60: at t0 + 60ms (JSONP_DELAY), wikidata SHOULD NOT be set because graph has changed.
// t60: at t0 + 60ms (delay), wikidata SHOULD NOT be set because graph has changed.

// t70: check that wikidata unchanged
window.setTimeout(function() {
expect(context.entity(entity.id).tags.wikidata).to.be.undefined;
}, 70);

// t90: at t30 + 60ms (JSONP_DELAY), wikidata SHOULD be set because graph is unchanged.
// t90: at t30 + 60ms (delay), wikidata SHOULD be set because graph is unchanged.

// t100: check that wikidata has changed
window.setTimeout(function() {
Expand Down