MaxSem has uploaded a new change for review. https://gerrit.wikimedia.org/r/312406
Change subject: Allow readable queries for externaldata in geojson ...................................................................... Allow readable queries for externaldata in geojson TBD: Should we do more validation in schema rather than Map.js? Bug: T145047 Change-Id: I033a515e3561850fccaba8ab2be9457e040fcfe9 (cherry picked from commit 90c849d84645e8efdad32f06a53db41cfa06a502) --- M modules/box/Map.js M schemas/geojson.json A tests/phpunit/data/bad-schemas/42-externaldata-7.json A tests/phpunit/data/bad-schemas/43-externaldata-8.json A tests/phpunit/data/bad-schemas/44-externaldata-9.json A tests/phpunit/data/bad-schemas/45-externaldata-10.json A tests/phpunit/data/bad-schemas/46-externaldata-11.json A tests/phpunit/data/bad-schemas/47-externaldata-12.json A tests/phpunit/data/bad-schemas/48-externaldata-13.json A tests/phpunit/data/bad-schemas/49-externaldata-14.json A tests/phpunit/data/bad-schemas/50-externaldata-15.json M tests/phpunit/data/good-schemas/08-externaldata.json 12 files changed, 160 insertions(+), 13 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Kartographer refs/changes/06/312406/1 diff --git a/modules/box/Map.js b/modules/box/Map.js index 587b5b6..eade691 100644 --- a/modules/box/Map.js +++ b/modules/box/Map.js @@ -216,15 +216,54 @@ * For a given ExternalData object, gets it via XHR and expands it in place * * @param {Object} data - * @param {string} data.href URL to external data + * @param {string} [data.href] optional URL to external data + * @param {string} [data.service] optional name of the service (same as protocol without the ':') + * @param {string} [data.host] optional host of the service + * @param {string|string[]} [data.query] optional geoshape query + * @param {string} [data.ids] optional geoshape ids * @return {Promise} resolved when done with geojson expansion */ function loadExternalDataAsync( data ) { - var uri = new mw.Uri( data.href ); - // If url begins with protocol:///... mark it as having relative host - if ( /^[a-z]+:\/\/\//.test( data.href ) ) { - uri.isRelativeHost = true; + var uri; + if ( data.href ) { + uri = new mw.Uri( data.href ); + // If url begins with protocol:///... mark it as having relative host + if ( /^[a-z]+:\/\/\//.test( data.href ) ) { + uri.isRelativeHost = true; + } + } else if ( data.service ) { + // Construct URI out of the parameters in the externalData object + uri = new mw.Uri( { + protocol: data.service, + host: data.host, + path: '/' + } ); + uri.isRelativeHost = !data.host; + uri.query = {}; + switch ( data.service ) { + case 'geoshape': + if ( data.query ) { + if ( typeof data.query === 'string' ) { + uri.query.query = data.query; + } else { + throw new Error( 'Invalid "query" parameter in ExternalData' ); + } + } + if ( data.ids ) { + if ( $.isArray( data.ids ) ) { + uri.query.ids = data.ids.join( ',' ); + } else if ( typeof data.ids === 'string' ) { + uri.query.ids = data.ids.replace( /\s*,\s*/, ',' ); + } else { + throw new Error( 'Invalid "ids" parameter in ExternalData' ); + } + } + break; + default: + throw new Error( 'Unknown externalData protocol ' + data.service ); + } } + switch ( uri.protocol ) { case 'geoshape': // geoshape:///?ids=Q16,Q30 @@ -232,10 +271,10 @@ // Get geo shapes data from OSM database by supplying Wikidata IDs or query // https://maps.wikimedia.org/shape?ids=Q16,Q30 if ( !uri.query || ( !uri.query.ids && !uri.query.query ) ) { - throw new Error( 'geoshape: missing ids or query parameter in: ' + data.href ); + throw new Error( 'geoshape: missing ids or query parameter in externalData' ); } if ( !uri.isRelativeHost && uri.host !== 'maps.wikimedia.org' ) { - throw new Error( 'geoshape: hostname must be missing or "maps.wikimedia.org": ' + data.href ); + throw new Error( 'geoshape: hostname must be missing or "maps.wikimedia.org"' ); } uri.protocol = 'https'; uri.host = 'maps.wikimedia.org'; @@ -272,7 +311,7 @@ } ); default: - throw new Error( 'Unknown protocol ' + data.href ); + throw new Error( 'Unknown externalData protocol ' + uri.protocol ); } } diff --git a/schemas/geojson.json b/schemas/geojson.json index 0063cae..98091e3 100644 --- a/schemas/geojson.json +++ b/schemas/geojson.json @@ -155,13 +155,46 @@ "externalData": { "title": "ExternalData", "description": "WMF extension - reference to external geometries", - "required": [ "href" ], + "required": [ "type" ], + "oneOf": [ + { + "required": [ "href" ], + "properties": { + "href": { + "type": "string", + "pattern": "^[a-z0-9-]+://[a-z0-9\\.-]*/([^\\s/\\?]+?(/[^\\s/\\?]+)*/?)?(\\?[^\\s]*)?$" + } + } + }, + { + "required": [ "service" ], + "properties": { + "service": { "enum": [ "geoshape" ] }, + "query": { "type": "string" }, + "ids": { + "oneOf": [ + { + "type": "array", + "items": { + "type": "string", + "pattern": "^Q[1-9]\\d{0,19}$" + } + }, + { + "type": "string", + "pattern": "^Q[1-9]\\d{0,19}(\\s*,\\s*Q[1-9]\\d{0,19})*$" + } + ] + } + }, + "anyOf": [ + { "required": [ "query" ] }, + { "required": [ "ids" ] } + ] + } + ], "properties": { "type": { "enum": [ "ExternalData" ] }, - "href": { - "type": "string", - "pattern": "^[a-z0-9-]+://[a-z0-9\\.-]*/([^\\s/\\?]+?(/[^\\s/\\?]+)*/?)?(\\?[^\\s]*)?$" - }, "properties": { "$ref": "#/definitions/simplestyle" } } }, diff --git a/tests/phpunit/data/bad-schemas/42-externaldata-7.json b/tests/phpunit/data/bad-schemas/42-externaldata-7.json new file mode 100644 index 0000000..483ee60 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/42-externaldata-7.json @@ -0,0 +1,4 @@ +{ + "type": "ExternalData", + "service": "fail" +} diff --git a/tests/phpunit/data/bad-schemas/43-externaldata-8.json b/tests/phpunit/data/bad-schemas/43-externaldata-8.json new file mode 100644 index 0000000..d0a7b83 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/43-externaldata-8.json @@ -0,0 +1,4 @@ +{ + "type": "ExternalData", + "service": "geoshapes" +} diff --git a/tests/phpunit/data/bad-schemas/44-externaldata-9.json b/tests/phpunit/data/bad-schemas/44-externaldata-9.json new file mode 100644 index 0000000..cface51 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/44-externaldata-9.json @@ -0,0 +1,4 @@ +{ + "type": "ExternalData", + "query": false +} diff --git a/tests/phpunit/data/bad-schemas/45-externaldata-10.json b/tests/phpunit/data/bad-schemas/45-externaldata-10.json new file mode 100644 index 0000000..9deebaa --- /dev/null +++ b/tests/phpunit/data/bad-schemas/45-externaldata-10.json @@ -0,0 +1,4 @@ +{ + "type": "ExternalData", + "query": "FOO BAR" +} diff --git a/tests/phpunit/data/bad-schemas/46-externaldata-11.json b/tests/phpunit/data/bad-schemas/46-externaldata-11.json new file mode 100644 index 0000000..c4d432c --- /dev/null +++ b/tests/phpunit/data/bad-schemas/46-externaldata-11.json @@ -0,0 +1,5 @@ +{ + "type": "ExternalData", + "service": "geoshape", + "ids": "1,2,3" +} diff --git a/tests/phpunit/data/bad-schemas/47-externaldata-12.json b/tests/phpunit/data/bad-schemas/47-externaldata-12.json new file mode 100644 index 0000000..7a68ea7 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/47-externaldata-12.json @@ -0,0 +1,6 @@ +{ + "type": "ExternalData", + "service": "geoshape", + "query": "FOO BAR", + "ids": [ "Q1", "Q2", "3" ] +} diff --git a/tests/phpunit/data/bad-schemas/48-externaldata-13.json b/tests/phpunit/data/bad-schemas/48-externaldata-13.json new file mode 100644 index 0000000..87923a0 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/48-externaldata-13.json @@ -0,0 +1,5 @@ +{ + "type": "ExternalData", + "service": "geoshape", + "ids": "fail" +} diff --git a/tests/phpunit/data/bad-schemas/49-externaldata-14.json b/tests/phpunit/data/bad-schemas/49-externaldata-14.json new file mode 100644 index 0000000..6662634 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/49-externaldata-14.json @@ -0,0 +1,5 @@ +{ + "type": "ExternalData", + "service": "geoshape", + "ids": "Q0123" +} diff --git a/tests/phpunit/data/bad-schemas/50-externaldata-15.json b/tests/phpunit/data/bad-schemas/50-externaldata-15.json new file mode 100644 index 0000000..9bd4195 --- /dev/null +++ b/tests/phpunit/data/bad-schemas/50-externaldata-15.json @@ -0,0 +1,5 @@ +{ + "type": "ExternalData", + "service": "geoshape", + "ids": [ "Q0123" ] +} diff --git a/tests/phpunit/data/good-schemas/08-externaldata.json b/tests/phpunit/data/good-schemas/08-externaldata.json index a17cb76..f9e1118 100644 --- a/tests/phpunit/data/good-schemas/08-externaldata.json +++ b/tests/phpunit/data/good-schemas/08-externaldata.json @@ -22,5 +22,38 @@ }, "custom fields": "are allowed", "sadly": true + }, + { + "type": "ExternalData", + "service": "geoshape", + "query": "SELECT ?foo\nbar" + }, + { + "type": "ExternalData", + "service": "geoshape", + "ids": [] + }, + { + "type": "ExternalData", + "service": "geoshape", + "ids": [ "Q1", "Q2", "Q3" ] + }, + { + "type": "ExternalData", + "service": "geoshape", + "query": "ugh", + "ids": [ "Q1", "Q2", "Q3" ] + }, + { + "type": "ExternalData", + "service": "geoshape", + "query": "lalala", + "ids": "Q1" + }, + { + "type": "ExternalData", + "service": "geoshape", + "query": "lalala", + "ids": "Q1,Q2 , Q3" } ] -- To view, visit https://gerrit.wikimedia.org/r/312406 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I033a515e3561850fccaba8ab2be9457e040fcfe9 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Kartographer Gerrit-Branch: wmf/1.28.0-wmf.20 Gerrit-Owner: MaxSem <maxsem.w...@gmail.com> Gerrit-Reviewer: Yurik <yu...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits