From bcd650e84468d747d5cba0053a3daf596bdefb06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 16:32:18 +0100 Subject: [PATCH 1/4] chore: add ids on features --- umap/static/umap/js/modules/global.js | 13 ++++- umap/static/umap/js/modules/utils.js | 13 +++++ umap/static/umap/js/umap.features.js | 13 +++++ .../test_features_id_generation.py | 51 +++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 umap/static/umap/js/modules/utils.js create mode 100644 umap/tests/integration/test_features_id_generation.py diff --git a/umap/static/umap/js/modules/global.js b/umap/static/umap/js/modules/global.js index c7fce53f..2d579187 100644 --- a/umap/static/umap/js/modules/global.js +++ b/umap/static/umap/js/modules/global.js @@ -1,10 +1,21 @@ import * as L from '../../vendors/leaflet/leaflet-src.esm.js' import URLs from './urls.js' import Browser from './browser.js' +import * as Utils from './utils.js' import { Request, ServerRequest, RequestError, HTTPError, NOKError } from './request.js' + // Import modules and export them to the global scope. // For the not yet module-compatible JS out there. // Copy the leaflet module, it's expected by leaflet plugins to be writeable. window.L = { ...L } -window.U = { URLs, Request, ServerRequest, RequestError, HTTPError, NOKError, Browser } +window.U = { + URLs, + Request, + ServerRequest, + RequestError, + HTTPError, + NOKError, + Browser, + Utils, +} diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js new file mode 100644 index 00000000..03081308 --- /dev/null +++ b/umap/static/umap/js/modules/utils.js @@ -0,0 +1,13 @@ +/** + * Generate a pseudo-unique identifier (4 chars long) + * + * Using uppercase + lowercase + digits, here's the collision risk: + * - for 6 chars, 1 in 100 000 + * - for 5 chars, 5 in 100 000 + * - for 4 chars, 500 in 100 000 + * + * @returns string + */ +export function generateId() { + return (((1 + Math.random()) * 0x10000) | 0).toString(16).substring(1) +} diff --git a/umap/static/umap/js/umap.features.js b/umap/static/umap/js/umap.features.js index fecbba57..b8dd0013 100644 --- a/umap/static/umap/js/umap.features.js +++ b/umap/static/umap/js/umap.features.js @@ -9,8 +9,17 @@ U.FeatureMixin = { // DataLayer the marker belongs to this.datalayer = options.datalayer || null this.properties = { _umap_options: {} } + let geojson_id if (options.geojson) { this.populate(options.geojson) + geojson_id = options.geojson.id + } + + // Each feature needs an unique identifier + if (this._checkId(geojson_id)) { + this.id = geojson_id + } else { + this.id = U.Utils.generateId() } let isDirty = false const self = this @@ -36,6 +45,9 @@ U.FeatureMixin = { this.addInteractions() this.parentClass.prototype.initialize.call(this, latlng, options) }, + _checkId: function () { + return typeof string !== 'undefined' + }, preInit: function () {}, @@ -344,6 +356,7 @@ U.FeatureMixin = { toGeoJSON: function () { const geojson = this.parentClass.prototype.toGeoJSON.call(this) geojson.properties = this.cloneProperties() + geojson.id = this.id delete geojson.properties._storage_options return geojson }, diff --git a/umap/tests/integration/test_features_id_generation.py b/umap/tests/integration/test_features_id_generation.py new file mode 100644 index 00000000..ca4558a5 --- /dev/null +++ b/umap/tests/integration/test_features_id_generation.py @@ -0,0 +1,51 @@ +import json +from pathlib import Path + + +def test_ids_generation(page, live_server, tilelayer): + page.goto(f"{live_server.url}/en/map/new/") + + # Click on the Draw a line button on a new map. + create_polyline = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polyline" + ) + create_polyline.click() + + map = page.locator("#map") + map.click(position={"x": 200, "y": 200}) + map.click(position={"x": 100, "y": 100}) + # Click again to finish + map.click(position={"x": 100, "y": 100}) + + # Click on the Draw a polygon button on a new map. + create_polygon = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polygon" + ) + create_polygon.click() + + map = page.locator("#map") + map.click(position={"x": 300, "y": 300}) + map.click(position={"x": 300, "y": 400}) + map.click(position={"x": 350, "y": 450}) + # Click again to finish + map.click(position={"x": 350, "y": 450}) + + download_panel = page.get_by_title("Share and download") + download_panel.click() + + button = page.get_by_role("button", name="geojson") + + with page.expect_download() as download_info: + button.click() + + download = download_info.value + + path = Path("/tmp/") / download.suggested_filename + download.save_as(path) + downloaded = json.loads(path.read_text()) + + assert "features" in downloaded + features = downloaded["features"] + assert len(features) == 2 + assert "id" in features[0] + assert "id" in features[1] From f9175e98b965c789d59dcbf4714b900040c15f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 16:45:21 +0100 Subject: [PATCH 2/4] chore: change `generateID()` implementation --- umap/static/umap/js/modules/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js index 03081308..aa681602 100644 --- a/umap/static/umap/js/modules/utils.js +++ b/umap/static/umap/js/modules/utils.js @@ -1,7 +1,7 @@ /** - * Generate a pseudo-unique identifier (4 chars long) + * Generate a pseudo-unique identifier (5 chars long, mixed-case alphanumeric) * - * Using uppercase + lowercase + digits, here's the collision risk: + * Here's the collision risk: * - for 6 chars, 1 in 100 000 * - for 5 chars, 5 in 100 000 * - for 4 chars, 500 in 100 000 @@ -9,5 +9,5 @@ * @returns string */ export function generateId() { - return (((1 + Math.random()) * 0x10000) | 0).toString(16).substring(1) + return btoa(Math.random().toString()).substring(10, 15) } From 702713e768615b2fea70fc2d135fdc1a193faa95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 17:10:52 +0100 Subject: [PATCH 3/4] chore: add missing string parameter --- umap/static/umap/js/umap.features.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/umap/static/umap/js/umap.features.js b/umap/static/umap/js/umap.features.js index b8dd0013..3baacc53 100644 --- a/umap/static/umap/js/umap.features.js +++ b/umap/static/umap/js/umap.features.js @@ -45,7 +45,7 @@ U.FeatureMixin = { this.addInteractions() this.parentClass.prototype.initialize.call(this, latlng, options) }, - _checkId: function () { + _checkId: function (string) { return typeof string !== 'undefined' }, From b0334a027db069d1473e6b69ea101fabc10d9190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 29 Feb 2024 11:38:18 +0100 Subject: [PATCH 4/4] feat: Ensure features ids match the requested format --- umap/static/umap/js/modules/utils.js | 11 +++++++++++ umap/static/umap/js/umap.features.js | 5 +---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js index aa681602..a5116616 100644 --- a/umap/static/umap/js/modules/utils.js +++ b/umap/static/umap/js/modules/utils.js @@ -11,3 +11,14 @@ export function generateId() { return btoa(Math.random().toString()).substring(10, 15) } + +/** + * Ensure the ID matches the expected format. + * + * @param {string} string + * @returns {boolean} + */ +export function checkId(string) { + if (typeof string !== 'string') return false + return /^[A-Za-z0-9]{5}$/.test(string) +} diff --git a/umap/static/umap/js/umap.features.js b/umap/static/umap/js/umap.features.js index 3baacc53..57caecce 100644 --- a/umap/static/umap/js/umap.features.js +++ b/umap/static/umap/js/umap.features.js @@ -16,7 +16,7 @@ U.FeatureMixin = { } // Each feature needs an unique identifier - if (this._checkId(geojson_id)) { + if (U.Utils.checkId(geojson_id)) { this.id = geojson_id } else { this.id = U.Utils.generateId() @@ -45,9 +45,6 @@ U.FeatureMixin = { this.addInteractions() this.parentClass.prototype.initialize.call(this, latlng, options) }, - _checkId: function (string) { - return typeof string !== 'undefined' - }, preInit: function () {},