From bfb16874b1f1844725bebfd3a7fd3f6a4aae0310 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Thu, 30 Dec 2021 20:41:45 +0100 Subject: [PATCH] Further experimentation --- Logic/FeatureSource/FeaturePipeline.ts | 4 +- .../FullNodeDatabaseSource.ts | 31 +++- Logic/Osm/Actions/ReplaceGeometryAction.ts | 49 ++++-- Logic/Web/MangroveReviews.ts | 3 +- test/ReplaceGeometry.spec.ts | 165 +++++++++++++++++- test/TestAll.ts | 111 ++++++------ test/TestHelper.ts | 8 +- 7 files changed, 284 insertions(+), 87 deletions(-) diff --git a/Logic/FeatureSource/FeaturePipeline.ts b/Logic/FeatureSource/FeaturePipeline.ts index ca704536a5..d0f03b03bf 100644 --- a/Logic/FeatureSource/FeaturePipeline.ts +++ b/Logic/FeatureSource/FeaturePipeline.ts @@ -92,7 +92,7 @@ export default class FeaturePipeline { if (location?.zoom === undefined) { return false; } - let minzoom = Math.min(...state.layoutToUse.layers.map(layer => layer.minzoom ?? 18)); + let minzoom = Math.min(...state.filteredLayers.data.map(layer => layer.layerDef.minzoom ?? 18)); return location.zoom >= minzoom; } ); @@ -312,7 +312,7 @@ export default class FeaturePipeline { // Whenever fresh data comes in, we need to update the metatagging - self.newDataLoadedSignal.stabilized(250).addCallback(src => { + self.newDataLoadedSignal.stabilized(250).addCallback(_ => { self.updateAllMetaTagging() }) diff --git a/Logic/FeatureSource/TiledFeatureSource/FullNodeDatabaseSource.ts b/Logic/FeatureSource/TiledFeatureSource/FullNodeDatabaseSource.ts index d2e9ec95be..b56b1eb149 100644 --- a/Logic/FeatureSource/TiledFeatureSource/FullNodeDatabaseSource.ts +++ b/Logic/FeatureSource/TiledFeatureSource/FullNodeDatabaseSource.ts @@ -3,6 +3,7 @@ import FeatureSource, {FeatureSourceForLayer, Tiled} from "../FeatureSource"; import {OsmNode, OsmObject, OsmWay} from "../../Osm/OsmObject"; import SimpleFeatureSource from "../Sources/SimpleFeatureSource"; import FilteredLayer from "../../../Models/FilteredLayer"; +import {UIEventSource} from "../../UIEventSource"; export default class FullNodeDatabaseSource implements TileHierarchy { @@ -10,6 +11,7 @@ export default class FullNodeDatabaseSource implements TileHierarchy void; private readonly layer: FilteredLayer private readonly nodeByIds = new Map(); + private readonly parentWays = new Map>() constructor( layer: FilteredLayer, @@ -35,7 +37,6 @@ export default class FullNodeDatabaseSource implements TileHierarchy() for (const osmObj of allObjects) { if (osmObj.type !== "way") { continue @@ -43,16 +44,20 @@ export default class FullNodeDatabaseSource implements TileHierarchyosmObj; for (const nodeId of osmWay.nodes) { - if (!parentWaysByNodeId.has(nodeId)) { - parentWaysByNodeId.set(nodeId, []) + if (!this.parentWays.has(nodeId)) { + const src = new UIEventSource([]) + this.parentWays.set(nodeId,src) + src.addCallback(parentWays => { + const tgs = nodesById.get(nodeId).tags + tgs ["parent_ways"] = JSON.stringify(parentWays.map(w => w.tags)) + tgs["parent_way_ids"] = JSON.stringify(parentWays.map(w => w.id)) + }) } - parentWaysByNodeId.get(nodeId).push(osmWay) + const src = this.parentWays.get(nodeId) + src.data.push(osmWay) + src.ping(); } } - parentWaysByNodeId.forEach((allWays, nodeId) => { - nodesById.get(nodeId).tags["parent_ways"] = JSON.stringify(allWays.map(w => w.tags)) - nodesById.get(nodeId).tags["parent_way_ids"] = JSON.stringify(allWays.map(w => w.id)) - }) const now = new Date() const asGeojsonFeatures = Array.from(nodesById.values()).map(osmNode => ({ feature: osmNode.asGeoJson(), freshness: now @@ -71,10 +76,18 @@ export default class FullNodeDatabaseSource implements TileHierarchy { + return this.parentWays.get(nodeId) + } } diff --git a/Logic/Osm/Actions/ReplaceGeometryAction.ts b/Logic/Osm/Actions/ReplaceGeometryAction.ts index 651676dc58..a6026f96e5 100644 --- a/Logic/Osm/Actions/ReplaceGeometryAction.ts +++ b/Logic/Osm/Actions/ReplaceGeometryAction.ts @@ -216,16 +216,16 @@ export default class ReplaceGeometryAction extends OsmChangeAction { throw "PANIC: replaceGeometryAction needs the FullNodeDatabase, which is undefined. This should be initialized by having the 'type_node'-layer enabled in your theme. (NB: the replacebutton has type_node as dependency)" } for (const nodeId of detachedNodeIds) { - const osmNode = nodeDb.GetNode(nodeId) - const parentWayIds: number[] = JSON.parse(osmNode.tags["parent_way_ids"]) - const index = parentWayIds.indexOf(osmWay.id) + const parentWays = nodeDb.GetParentWays(nodeId) + const index = parentWays.data.map(w => w.id).indexOf(osmWay.id) if(index < 0){ console.error("ReplaceGeometryAction is trying to detach node "+nodeId+", but it isn't listed as being part of way "+osmWay.id) continue; } - parentWayIds.splice(index, 1) - osmNode.tags["parent_way_ids"] = JSON.stringify(parentWayIds) - if(parentWayIds.length == 0){ + // We detachted this node - so we unregister + parentWays.data.splice(index, 1) + parentWays.ping(); + if(parentWays.data.length == 0){ // This point has no other ways anymore - lets clean it! console.log("Removing node "+nodeId, "as it isn't needed anymore by any way") @@ -254,7 +254,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction { * This method contains the main logic for this module, as it decides which node gets moved where. * */ - private async GetClosestIds(): Promise<{ + public async GetClosestIds(): Promise<{ // A list of the same length as targetCoordinates, containing which OSM-point to move. If undefined, a new point will be created closestIds: number[], @@ -282,11 +282,33 @@ export default class ReplaceGeometryAction extends OsmChangeAction { const allNodes = parsed.filter(o => o.type === "node") /** - * For every already existing OSM-point, we calculate the distance to every target point + * For every already existing OSM-point, we calculate: + * + * - the distance to every target point. + * - Wether this node has (other) parent ways, which might restrict movement + * - Wether this node has tags set + * + * Having tags and/or being connected to another way indicate that there is some _relation_ with objects in the neighbourhood. + * + * The Replace-geometry action should try its best to honour these. Some 'wiggling' is allowed (e.g. moving an entrance a bit), but these relations should not be broken.l */ - - const distances = new Map distance (or undefined if a duplicate)*/>(); + const distances = new Map distance (or undefined if a duplicate)*/ + number[]>(); for (const node of allNodes) { + const nodesWithAllParents = this.state.featurePipeline.fullNodeDatabase + if (nodesWithAllParents === undefined) { + throw "PANIC: replaceGeometryAction needs the FullNodeDatabase, which is undefined. This should be initialized by having the 'type_node'-layer enabled in your theme. (NB: the replacebutton has type_node as dependency)" + } + + let parentWayIds = nodesWithAllParents.GetParentWays(node.id) + + + + + + + const nodeDistances = this.targetCoordinates.map(_ => undefined) for (let i = 0; i < this.targetCoordinates.length; i++) { if (this.identicalTo[i] !== undefined) { @@ -299,17 +321,18 @@ export default class ReplaceGeometryAction extends OsmChangeAction { distances.set(node.id, nodeDistances) } + const closestIds = this.targetCoordinates.map(_ => undefined) + const unusedIds = [] + { /** * Then, we search the node that has to move the least distance and add this as mapping. * We do this until no points are left */ let candidate: number; let moveDistance: number; - const closestIds = this.targetCoordinates.map(_ => undefined) /** * The list of nodes that are _not_ used anymore, typically if there are less targetCoordinates then source coordinates */ - const unusedIds = [] do { candidate = undefined; moveDistance = Infinity; @@ -353,7 +376,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction { } } } while (candidate !== undefined) - + } // If there are still unused values in 'distances', they are definitively unused distances.forEach((_, nodeId) => { diff --git a/Logic/Web/MangroveReviews.ts b/Logic/Web/MangroveReviews.ts index 0362282480..328e6d6af6 100644 --- a/Logic/Web/MangroveReviews.ts +++ b/Logic/Web/MangroveReviews.ts @@ -1,6 +1,7 @@ import * as mangrove from 'mangrove-reviews' import {UIEventSource} from "../UIEventSource"; import {Review} from "./Review"; +import {Utils} from "../../Utils"; export class MangroveIdentity { public keypair: any = undefined; @@ -23,7 +24,7 @@ export class MangroveIdentity { }) }) try { - if ((mangroveIdentity.data ?? "") === "") { + if (!Utils.runningFromConsole && (mangroveIdentity.data ?? "") === "") { this.CreateIdentity(); } } catch (e) { diff --git a/test/ReplaceGeometry.spec.ts b/test/ReplaceGeometry.spec.ts index 4919914cc7..eed8d79764 100644 --- a/test/ReplaceGeometry.spec.ts +++ b/test/ReplaceGeometry.spec.ts @@ -1,14 +1,22 @@ import T from "./TestHelper"; import {Utils} from "../Utils"; +import ReplaceGeometryAction from "../Logic/Osm/Actions/ReplaceGeometryAction"; +import FeaturePipeline from "../Logic/FeatureSource/FeaturePipeline"; +import {Tag} from "../Logic/Tags/Tag"; +import MapState from "../Logic/State/MapState"; +import * as grb from "../assets/themes/grb_import/grb.json" +import LayoutConfig from "../Models/ThemeConfig/LayoutConfig"; +import * as Assert from "assert"; export default class ReplaceGeometrySpec extends T { constructor() { super("ReplaceGeometry", [ ["Simple house replacement", async () => { - const coordinates = <[number, number][]>[[ - 3.216690793633461, - 51.21474084112525 - ], + const coordinates = <[number, number][]>[ + [ + 3.216690793633461, + 51.21474084112525 + ], [ 3.2167256623506546, 51.214696737309964 @@ -51,6 +59,15 @@ export default class ReplaceGeometrySpec extends T { ] ] + const targetFeature = { + type: "Feature", + properties: {}, + geometry: { + type: "Polygon", + coordinates + } + } + Utils.injectJsonDownloadForTests( "https://www.openstreetmap.org/api/0.6/way/160909312/full", { @@ -170,12 +187,146 @@ export default class ReplaceGeometrySpec extends T { } ) - const wayId = "way/160909312" - const url = `https://www.openstreetmap.org/api/0.6/${wayId}/full`; - const rawData = await Utils.downloadJsonCached(url, 1000) + + const state = new MapState( + new LayoutConfig(grb, true, "ReplaceGeometrySpec.grbtheme") + ) + const featurePipeline = new FeaturePipeline( + _ => { + }, + state + ) + + const action = new ReplaceGeometryAction({ + osmConnection: undefined, + featurePipeline + }, targetFeature, wayId, { + theme: "test" + } + ) + const info = await action.GetClosestIds() + console.log(info) + Assert.equal(coordinates.length, 11) + }], + ["Advanced merge case with connections and tags", async () => { + const osmWay = "way/323230330"; + const grb_data = { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [ + 4.483118100000016, + 51.028366499999706 + ], + [ + 4.483135099999986, + 51.028325800000005 + ], + [ + 4.483137700000021, + 51.02831960000019 + ], + [ + 4.4831429000000025, + 51.0283205 + ], + [ + 4.483262199999987, + 51.02834059999982 + ], + [ + 4.483276700000019, + 51.028299999999746 + ], + [ + 4.483342100000037, + 51.02830730000009 + ], + [ + 4.483340700000012, + 51.028331299999934 + ], + [ + 4.483346499999953, + 51.02833189999984 + ], + [ + 4.483290600000001, + 51.028500699999846 + ], + [ + 4.4833335999999635, + 51.02851150000015 + ], + [ + 4.4833433000000475, + 51.028513999999944 + ], + [ + 4.483312899999958, + 51.02857759999998 + ], + [ + 4.483141100000033, + 51.02851780000015 + ], + [ + 4.483193100000022, + 51.028409999999894 + ], + [ + 4.483206100000019, + 51.02838310000014 + ], + [ + 4.483118100000016, + 51.028366499999706 + ] + ] + ] + }, + "id": "https://betadata.grbosm.site/grb?bbox=498980.9206456306,6626173.107985358,499133.7947022009,6626325.98204193/30", + "bbox": { + "maxLat": 51.02857759999998, + "maxLon": 4.483346499999953, + "minLat": 51.028299999999746, + "minLon": 4.483118100000016 + }, + "_lon": 4.483232299999985, + "_lat": 51.02843879999986 + } + + const config = new LayoutConfig(grb, true, "ReplaceGeometrySpec.grbtheme") + const state = new MapState( + config, { + attemptLogin: false + } + ) + const featurepipeline = new FeaturePipeline( + _ => { + }, + state + ) + + + const action = new ReplaceGeometryAction({ + osmConnection: undefined, + featurePipeline: featurepipeline + }, grb_data, + osmWay, + { + theme: "test", + newTags: [new Tag("test", "yes")] + }) + + const info = await action.GetClosestIds() + console.log(info) }] ]); } diff --git a/test/TestAll.ts b/test/TestAll.ts index f53e1beb46..dabfc8e689 100644 --- a/test/TestAll.ts +++ b/test/TestAll.ts @@ -17,59 +17,66 @@ import ReplaceGeometrySpec from "./ReplaceGeometry.spec"; import LegacyThemeLoaderSpec from "./LegacyThemeLoader.spec"; -ScriptUtils.fixUtils() -const allTests = [ - new OsmObjectSpec(), - new TagSpec(), - new ImageAttributionSpec(), - new GeoOperationsSpec(), - new ThemeSpec(), - new UtilsSpec(), - new UnitsSpec(), - new RelationSplitHandlerSpec(), - new SplitActionSpec(), - new TileFreshnessCalculatorSpec(), - new WikidataSpecTest(), - new ImageProviderSpec(), - new ActorsSpec(), - new ReplaceGeometrySpec(), - new LegacyThemeLoaderSpec() -] +async function main() { -Utils.externalDownloadFunction = async (url) => { - console.error("Fetching ", url, "blocked in tests, use Utils.injectJsonDownloadForTests") - const data = await ScriptUtils.DownloadJSON(url) - console.log("\n\n ----------- \nBLOCKED DATA\n Utils.injectJsonDownloadForTests(\n" + - " ", JSON.stringify(url), ", \n", - " ", JSON.stringify(data), "\n )\n------------------\n\n") - throw "Detected internet access for URL " + url + ", please inject it with Utils.injectJsonDownloadForTests" -} + ScriptUtils.fixUtils() + const allTests = [ + new OsmObjectSpec(), + new TagSpec(), + new ImageAttributionSpec(), + new GeoOperationsSpec(), + new ThemeSpec(), + new UtilsSpec(), + new UnitsSpec(), + new RelationSplitHandlerSpec(), + new SplitActionSpec(), + new TileFreshnessCalculatorSpec(), + new WikidataSpecTest(), + new ImageProviderSpec(), + new ActorsSpec(), + new ReplaceGeometrySpec(), + new LegacyThemeLoaderSpec() + ] -let args = [...process.argv] -args.splice(0, 2) -args = args.map(a => a.toLowerCase()) - -const allFailures: { testsuite: string, name: string, msg: string } [] = [] -let testsToRun = allTests -if (args.length > 0) { - args = args.map(a => a.toLowerCase()) - testsToRun = allTests.filter(t => args.indexOf(t.name.toLowerCase()) >= 0) -} - -if (testsToRun.length == 0) { - throw "No tests found. Try one of " + allTests.map(t => t.name).join(", ") -} - -for (let i = 0; i < testsToRun.length; i++) { - const test = testsToRun[i]; - console.log(" Running test", i, "/", testsToRun.length, test.name) - allFailures.push(...(test.Run() ?? [])) - console.log("OK!") -} -if (allFailures.length > 0) { - for (const failure of allFailures) { - console.error(" !! " + failure.testsuite + "." + failure.name + " failed due to: " + failure.msg) + Utils.externalDownloadFunction = async (url) => { + console.error("Fetching ", url, "blocked in tests, use Utils.injectJsonDownloadForTests") + const data = await ScriptUtils.DownloadJSON(url) + console.log("\n\n ----------- \nBLOCKED DATA\n Utils.injectJsonDownloadForTests(\n" + + " ", JSON.stringify(url), ", \n", + " ", JSON.stringify(data), "\n )\n------------------\n\n") + throw "Detected internet access for URL " + url + ", please inject it with Utils.injectJsonDownloadForTests" } - throw "Some test failed" + + let args = [...process.argv] + args.splice(0, 2) + args = args.map(a => a.toLowerCase()) + + const allFailures: { testsuite: string, name: string, msg: string } [] = [] + let testsToRun = allTests + if (args.length > 0) { + args = args.map(a => a.toLowerCase()) + testsToRun = allTests.filter(t => args.indexOf(t.name.toLowerCase()) >= 0) + } + + if (testsToRun.length == 0) { + throw "No tests found. Try one of " + allTests.map(t => t.name).join(", ") + } + + for (let i = 0; i < testsToRun.length; i++) { + const test = testsToRun[i]; + console.log(" Running test", i, "/", testsToRun.length, test.name) + + allFailures.push(...(await test.Run() ?? [])) + console.log("OK!") + } + if (allFailures.length > 0) { + for (const failure of allFailures) { + console.error(" !! " + failure.testsuite + "." + failure.name + " failed due to: " + failure.msg) + } + throw "Some test failed" + } + console.log("All tests successful: ", testsToRun.map(t => t.name).join(", ")) + } -console.log("All tests successful: ", testsToRun.map(t => t.name).join(", ")) + +main() \ No newline at end of file diff --git a/test/TestHelper.ts b/test/TestHelper.ts index f8a6714cb8..b5e803bcd2 100644 --- a/test/TestHelper.ts +++ b/test/TestHelper.ts @@ -56,16 +56,18 @@ export default class T { * Returns an empty list if successful * @constructor */ - public Run(): { testsuite: string, name: string, msg: string } [] { + public async Run(): Promise<{ testsuite: string, name: string, msg: string } []> { const failures: { testsuite: string, name: string, msg: string } [] = [] for (const [name, test] of this._tests) { try { const r = test() if (r instanceof Promise) { - r.catch(e => { + try { + await r + } catch (e) { console.log("ASYNC ERROR: ", e, e.stack) failures.push({testsuite: this.name, name: name, msg: "" + e}); - }); + } } } catch (e) {