From 8e66313ef1a731826d2452cbf0774b40b70fe673 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Tue, 9 Nov 2021 01:49:07 +0100 Subject: [PATCH] Add metadata in changeset with (binned) distance to changed feature --- Logic/Actors/GeoLocationHandler.ts | 15 ++- Logic/Osm/Actions/ChangeDescription.ts | 6 +- Logic/Osm/Actions/ChangeLocationAction.ts | 4 +- Logic/Osm/Actions/ChangeTagAction.ts | 4 +- Logic/Osm/Actions/CreateNewNodeAction.ts | 2 +- Logic/Osm/Actions/CreateNewWayAction.ts | 6 +- .../Actions/CreateWayWithPointReuseAction.ts | 7 +- Logic/Osm/Actions/DeleteAction.ts | 2 +- Logic/Osm/Actions/OsmChangeAction.ts | 14 +- Logic/Osm/Actions/RelationSplitHandler.ts | 3 +- Logic/Osm/Actions/ReplaceGeometryAction.ts | 2 +- Logic/Osm/Actions/SplitAction.ts | 2 +- Logic/Osm/Changes.ts | 122 ++++++++++++++++-- Logic/Osm/ChangesetHandler.ts | 1 + Logic/State/ElementsState.ts | 2 +- Logic/State/MapState.ts | 5 +- Logic/State/UserRelatedState.ts | 1 + Models/Constants.ts | 15 ++- UI/AllThemesGui.ts | 2 +- UI/Popup/SplitRoadWizard.ts | 2 +- test/Actors.spec.ts | 2 +- 21 files changed, 178 insertions(+), 41 deletions(-) diff --git a/Logic/Actors/GeoLocationHandler.ts b/Logic/Actors/GeoLocationHandler.ts index de883d2f7..4d6561391 100644 --- a/Logic/Actors/GeoLocationHandler.ts +++ b/Logic/Actors/GeoLocationHandler.ts @@ -6,6 +6,18 @@ import LayoutConfig from "../../Models/ThemeConfig/LayoutConfig"; import {QueryParameters} from "../Web/QueryParameters"; import FeatureSource from "../FeatureSource/FeatureSource"; +export interface GeoLocationPointProperties { + id: "gps", + "user:location": "yes", + "date": string, + "latitude": number + "longitude":number, + "speed": number, + "accuracy": number + "heading": number + "altitude":number +} + export default class GeoLocationHandler extends VariableUiElement { private readonly currentLocation: FeatureSource @@ -184,10 +196,9 @@ export default class GeoLocationHandler extends VariableUiElement { this.currentLocation = state.currentUserLocation this._currentGPSLocation.addCallback((location) => { self._previousLocationGrant.setData("granted"); - console.log("Location is", location,) const feature = { "type": "Feature", - properties: { + properties: { id: "gps", "user:location": "yes", "date": new Date().toISOString(), diff --git a/Logic/Osm/Actions/ChangeDescription.ts b/Logic/Osm/Actions/ChangeDescription.ts index b3b2e9bbf..0f03caf0b 100644 --- a/Logic/Osm/Actions/ChangeDescription.ts +++ b/Logic/Osm/Actions/ChangeDescription.ts @@ -20,7 +20,11 @@ export interface ChangeDescription { /** * THe motivation for the change, e.g. 'deleted because does not exist anymore' */ - specialMotivation?: string + specialMotivation?: string, + /** + * Added by Changes.ts + */ + distanceToObject?: number }, /** diff --git a/Logic/Osm/Actions/ChangeLocationAction.ts b/Logic/Osm/Actions/ChangeLocationAction.ts index 9bb53b427..54141d548 100644 --- a/Logic/Osm/Actions/ChangeLocationAction.ts +++ b/Logic/Osm/Actions/ChangeLocationAction.ts @@ -11,7 +11,7 @@ export default class ChangeLocationAction extends OsmChangeAction { theme: string, reason: string }) { - super(); + super(id,true); if (!id.startsWith("node/")) { throw "Invalid ID: only 'node/number' is accepted" } @@ -19,7 +19,7 @@ export default class ChangeLocationAction extends OsmChangeAction { this._newLonLat = newLonLat; this._meta = meta; } - + protected async CreateChangeDescriptions(changes: Changes): Promise { const d: ChangeDescription = { diff --git a/Logic/Osm/Actions/ChangeTagAction.ts b/Logic/Osm/Actions/ChangeTagAction.ts index 359d733e4..013b90a46 100644 --- a/Logic/Osm/Actions/ChangeTagAction.ts +++ b/Logic/Osm/Actions/ChangeTagAction.ts @@ -13,13 +13,13 @@ export default class ChangeTagAction extends OsmChangeAction { theme: string, changeType: "answer" | "soft-delete" | "add-image" | string }) { - super(); + super(elementId, true); this._elementId = elementId; this._tagsFilter = tagsFilter; this._currentTags = currentTags; this._meta = meta; } - + /** * Doublechecks that no stupid values are added */ diff --git a/Logic/Osm/Actions/CreateNewNodeAction.ts b/Logic/Osm/Actions/CreateNewNodeAction.ts index bf9255709..79f64c14c 100644 --- a/Logic/Osm/Actions/CreateNewNodeAction.ts +++ b/Logic/Osm/Actions/CreateNewNodeAction.ts @@ -31,7 +31,7 @@ export default class CreateNewNodeAction extends OsmChangeAction { reusePointWithinMeters?: number, theme: string, changeType: "create" | "import" | null }) { - super() + super(null,basicTags !== undefined && basicTags.length > 0) this._basicTags = basicTags; this._lat = lat; this._lon = lon; diff --git a/Logic/Osm/Actions/CreateNewWayAction.ts b/Logic/Osm/Actions/CreateNewWayAction.ts index 48b7ec7fb..ef10d417f 100644 --- a/Logic/Osm/Actions/CreateNewWayAction.ts +++ b/Logic/Osm/Actions/CreateNewWayAction.ts @@ -24,13 +24,13 @@ export default class CreateNewWayAction extends OsmChangeAction { options: { theme: string }) { - super() + super(null,true) this.coordinates = coordinates; this.tags = tags; this._options = options; } - protected async CreateChangeDescriptions(changes: Changes): Promise { + public async CreateChangeDescriptions(changes: Changes): Promise { const newElements: ChangeDescription[] = [] @@ -46,7 +46,7 @@ export default class CreateNewWayAction extends OsmChangeAction { changeType: null, theme: this._options.theme }) - await changes.applyAction(newPoint) + newElements.push(...await newPoint.CreateChangeDescriptions(changes)) pointIds.push(newPoint.newElementIdNumber) } diff --git a/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts b/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts index dcbc3501f..a0bc84241 100644 --- a/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts +++ b/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts @@ -46,7 +46,7 @@ export default class CreateWayWithPointReuseAction extends OsmChangeAction { state: FeaturePipelineState, config: MergePointConfig[] ) { - super(); + super(null,true); this._tags = tags; this._state = state; this._config = config; @@ -194,9 +194,8 @@ export default class CreateWayWithPointReuseAction extends OsmChangeAction { const newWay = new CreateNewWayAction(this._tags, nodeIdsToUse, { theme }) - - allChanges.push(...(await newWay.Perform(changes))) - + + allChanges.push(...(await newWay.CreateChangeDescriptions(changes))) return allChanges } diff --git a/Logic/Osm/Actions/DeleteAction.ts b/Logic/Osm/Actions/DeleteAction.ts index 34adc50e7..a5be01448 100644 --- a/Logic/Osm/Actions/DeleteAction.ts +++ b/Logic/Osm/Actions/DeleteAction.ts @@ -27,7 +27,7 @@ export default class DeleteAction extends OsmChangeAction { specialMotivation: string }, hardDelete: boolean) { - super() + super(id,true) this._id = id; this._hardDelete = hardDelete; this.meta = {...meta, changeType: "deletion"}; diff --git a/Logic/Osm/Actions/OsmChangeAction.ts b/Logic/Osm/Actions/OsmChangeAction.ts index 784b192ba..13a31a76a 100644 --- a/Logic/Osm/Actions/OsmChangeAction.ts +++ b/Logic/Osm/Actions/OsmChangeAction.ts @@ -8,6 +8,18 @@ import {ChangeDescription} from "./ChangeDescription"; export default abstract class OsmChangeAction { private isUsed = false + public readonly trackStatistics: boolean; + /** + * The ID of the object that is the center of this change. + * Null if the action creates a new object + * Undefined if such an id does not make sense + */ + public readonly mainObjectId: string; + + constructor(mainObjectId: string, trackStatistics: boolean = true) { + this.trackStatistics = trackStatistics; + this.mainObjectId = mainObjectId + } public Perform(changes: Changes) { if (this.isUsed) { @@ -18,6 +30,4 @@ export default abstract class OsmChangeAction { } protected abstract CreateChangeDescriptions(changes: Changes): Promise - - } \ No newline at end of file diff --git a/Logic/Osm/Actions/RelationSplitHandler.ts b/Logic/Osm/Actions/RelationSplitHandler.ts index 1f0da8bcd..4cc5a0d0e 100644 --- a/Logic/Osm/Actions/RelationSplitHandler.ts +++ b/Logic/Osm/Actions/RelationSplitHandler.ts @@ -16,11 +16,10 @@ abstract class AbstractRelationSplitHandler extends OsmChangeAction { protected readonly _theme: string; constructor(input: RelationSplitInput, theme: string) { - super() + super("relation/"+input.relation.id, false) this._input = input; this._theme = theme; } - /** * Returns which node should border the member at the given index */ diff --git a/Logic/Osm/Actions/ReplaceGeometryAction.ts b/Logic/Osm/Actions/ReplaceGeometryAction.ts index f1d03fb40..1bd90869e 100644 --- a/Logic/Osm/Actions/ReplaceGeometryAction.ts +++ b/Logic/Osm/Actions/ReplaceGeometryAction.ts @@ -41,7 +41,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction { newTags?: Tag[] } ) { - super(); + super(wayToReplaceId, false); this.state = state; this.feature = feature; this.wayToReplaceId = wayToReplaceId; diff --git a/Logic/Osm/Actions/SplitAction.ts b/Logic/Osm/Actions/SplitAction.ts index f7e95dedf..3928ed405 100644 --- a/Logic/Osm/Actions/SplitAction.ts +++ b/Logic/Osm/Actions/SplitAction.ts @@ -26,7 +26,7 @@ export default class SplitAction extends OsmChangeAction { * @param toleranceInMeters: if a splitpoint closer then this amount of meters to an existing point, the existing point will be used to split the line instead of a new point */ constructor(wayId: string, splitPointCoordinates: [number, number][], meta: { theme: string }, toleranceInMeters = 5) { - super() + super(wayId,true) this.wayId = wayId; this._splitPointsCoordinates = splitPointCoordinates this._toleranceInMeters = toleranceInMeters; diff --git a/Logic/Osm/Changes.ts b/Logic/Osm/Changes.ts index 72e5434c5..fd904e260 100644 --- a/Logic/Osm/Changes.ts +++ b/Logic/Osm/Changes.ts @@ -8,6 +8,11 @@ import {Utils} from "../../Utils"; import {LocalStorageSource} from "../Web/LocalStorageSource"; import SimpleMetaTagger from "../SimpleMetaTagger"; import CreateNewNodeAction from "./Actions/CreateNewNodeAction"; +import FeatureSource from "../FeatureSource/FeatureSource"; +import {ElementStorage} from "../ElementStorage"; +import {GeoLocationPointProperties} from "../Actors/GeoLocationHandler"; +import {GeoOperations} from "../GeoOperations"; +import {ChangesetTag} from "./ChangesetHandler"; /** * Handles all changes made to OSM. @@ -27,6 +32,8 @@ export class Changes { private readonly previouslyCreated: OsmObject[] = [] private readonly _leftRightSensitive: boolean; + + private _state : { allElements: ElementStorage; historicalUserLocations: FeatureSource } constructor(leftRightSensitive: boolean = false) { this._leftRightSensitive = leftRightSensitive; @@ -113,14 +120,71 @@ export class Changes { }) } - public async applyAction(action: OsmChangeAction): Promise { - this.applyChanges(await action.Perform(this)) - } + private calculateDistanceToChanges(change: OsmChangeAction, changeDescriptions: ChangeDescription[]){ - public async applyActions(actions: OsmChangeAction[]) { - for (const action of actions) { - await this.applyAction(action) + if (this._state === undefined) { + // No state loaded -> we can't calculate... + return; } + if(!change.trackStatistics){ + // Probably irrelevant, such as a new helper node + return; + } + const now = new Date() + const recentLocationPoints = this._state.historicalUserLocations.features.data.map(ff => ff.feature) + .filter(feat => feat.geometry.type === "Point") + .filter(feat => { + const visitTime = new Date((feat.properties).date) + // In seconds + const diff = (now.getTime() - visitTime.getTime()) / 1000 + return diff < Constants.nearbyVisitTime; + }) + if(recentLocationPoints.length === 0){ + // Probably no GPS enabled/no fix + return; + } + + // The applicable points, contain information in their properties about location, time and GPS accuracy + // They are all GeoLocationPointProperties + // We walk every change and determine the closest distance possible + // Only if the change itself does _not_ contain any coordinates, we fall back and search the original feature in the state + + const changedObjectCoordinates : [number, number][] = [] + + const feature = this._state.allElements.ContainingFeatures.get(change.mainObjectId) + if(feature !== undefined){ + changedObjectCoordinates.push(GeoOperations.centerpointCoordinates(feature)) + } + + for (const changeDescription of changeDescriptions) { + const chng : {lat: number, lon: number} | {coordinates : [number,number][]} | {members} = changeDescription.changes + if(chng === undefined){ + continue + } + if(chng["lat"] !== undefined){ + changedObjectCoordinates.push([chng["lat"],chng["lon"]]) + } + if(chng["coordinates"] !== undefined){ + changedObjectCoordinates.push(...chng["coordinates"]) + } + } + + const leastDistance = Math.min(...changedObjectCoordinates.map(coor => + Math.min(...recentLocationPoints.map(gpsPoint => { + const otherCoor = GeoOperations.centerpointCoordinates(gpsPoint) + const dist = GeoOperations.distanceBetween(coor, otherCoor) * 1000; + console.log("Comparing ", coor, "and ", otherCoor, " --> ", dist) + return dist + })) + )) + return leastDistance + } + + public async applyAction(action: OsmChangeAction): Promise { + const changeDescriptions = await action.Perform(this) + const distanceToObject = this.calculateDistanceToChanges(action, changeDescriptions) + changeDescriptions[0].meta.distanceToObject = distanceToObject + this.applyChanges(changeDescriptions) } public applyChanges(changes: ChangeDescription[]) { @@ -130,6 +194,13 @@ export class Changes { this.allChanges.data.push(...changes) this.allChanges.ping() } + + public useLocationHistory(state: { + allElements: ElementStorage, + historicalUserLocations: FeatureSource + }){ + this._state= state + } public registerIdRewrites(mappings: Map): void { CreateNewNodeAction.registerIdRewrites(mappings) @@ -162,7 +233,6 @@ export class Changes { return true } - const meta = pending[0].meta const perType = Array.from( Utils.Hist(pending.filter(descr => descr.meta.changeType !== undefined && descr.meta.changeType !== null) @@ -177,16 +247,46 @@ export class Changes { key: descr.meta.changeType + ":" + descr.type + "/" + descr.id, value: descr.meta.specialMotivation })) - const metatags = [{ + + const distances = Utils.NoNull(pending.map(descr => descr.meta.distanceToObject)); + distances.sort((a, b) => a - b) + const perBinCount = Constants.distanceToChangeObjectBins.map(_ => 0) + + let j = 0; + const maxDistances = Constants.distanceToChangeObjectBins + for (let i = 0; i < maxDistances.length; i++){ + const maxDistance = maxDistances[i]; + // distances is sorted in ascending order, so as soon as one is to big, all the resting elements will be bigger too + while(j < distances.length && distances[j] < maxDistance){ + perBinCount[i] ++ + j++ + } + } + + const perBinMessage = Utils.NoNull(perBinCount.map((count, i) => { + if(count === 0){ + return undefined + } + return { + key: "change_within_"+maxDistances[i]+"m", + value: count, + aggregate:true + } + })) + + // This method is only called with changedescriptions for this theme + const theme = pending[0].meta.theme + const metatags : ChangesetTag[] = [{ key: "comment", - value: "Adding data with #MapComplete for theme #" + meta.theme + value: "Adding data with #MapComplete for theme #" + theme }, { key: "theme", - value: meta.theme + value: theme }, ...perType, - ...motivations + ...motivations, + ...perBinMessage ] await State.state.osmConnection.changesetHandler.UploadChangeset( diff --git a/Logic/Osm/ChangesetHandler.ts b/Logic/Osm/ChangesetHandler.ts index 08884e6d8..866c61faa 100644 --- a/Logic/Osm/ChangesetHandler.ts +++ b/Logic/Osm/ChangesetHandler.ts @@ -78,6 +78,7 @@ export class ChangesetHandler { } if (this._dryRun) { const changesetXML = generateChangeXML(123456); + console.log("Metatags are", extraMetaTags) console.log(changesetXML); return; } diff --git a/Logic/State/ElementsState.ts b/Logic/State/ElementsState.ts index 637482e60..345ada244 100644 --- a/Logic/State/ElementsState.ts +++ b/Logic/State/ElementsState.ts @@ -11,6 +11,7 @@ import {Utils} from "../../Utils"; import ChangeToElementsActor from "../Actors/ChangeToElementsActor"; import PendingChangesUploader from "../Actors/PendingChangesUploader"; import TitleHandler from "../Actors/TitleHandler"; +import FeatureSource from "../FeatureSource/FeatureSource"; /** * The part of the state keeping track of where the elements, loading them, configuring the feature pipeline etc @@ -50,7 +51,6 @@ export default class ElementsState extends FeatureSwitchState { super(layoutToUse); this.changes = new Changes(layoutToUse?.isLeftRightSensitive() ?? false) - { // -- Location control initialization const zoom = UIEventSource.asFloat( diff --git a/Logic/State/MapState.ts b/Logic/State/MapState.ts index 892783dc7..b57461699 100644 --- a/Logic/State/MapState.ts +++ b/Logic/State/MapState.ts @@ -14,7 +14,7 @@ import {QueryParameters} from "../Web/QueryParameters"; import * as personal from "../../assets/themes/personal/personal.json"; import FilterConfig from "../../Models/ThemeConfig/FilterConfig"; import ShowOverlayLayer from "../../UI/ShowDataLayer/ShowOverlayLayer"; -import FeatureSource, {FeatureSourceForLayer, Tiled} from "../FeatureSource/FeatureSource"; +import {FeatureSourceForLayer, Tiled} from "../FeatureSource/FeatureSource"; import SimpleFeatureSource from "../FeatureSource/Sources/SimpleFeatureSource"; /** @@ -209,7 +209,6 @@ export default class MapState extends UserRelatedState { const feature = JSON.parse(JSON.stringify(location.feature)) feature.properties.id = "gps/"+i i++ - console.log("New location: ", feature) features.data.push({feature, freshness: new Date()}) histCoordinates.push(feature.geometry.coordinates) @@ -224,7 +223,7 @@ export default class MapState extends UserRelatedState { let gpsLayerDef: FilteredLayer = this.filteredLayers.data.filter(l => l.layerDef.id === "gps_track")[0] this.historicalUserLocations = new SimpleFeatureSource(gpsLayerDef, Tiles.tile_index(0, 0, 0), features); - + this.changes.useLocationHistory(this) } private initHomeLocation() { diff --git a/Logic/State/UserRelatedState.ts b/Logic/State/UserRelatedState.ts index e75db0e42..19d1eba85 100644 --- a/Logic/State/UserRelatedState.ts +++ b/Logic/State/UserRelatedState.ts @@ -11,6 +11,7 @@ import ElementsState from "./ElementsState"; import SelectedElementTagsUpdater from "../Actors/SelectedElementTagsUpdater"; import StaticFeatureSource from "../FeatureSource/Sources/StaticFeatureSource"; import FeatureSource from "../FeatureSource/FeatureSource"; +import {Feature} from "@turf/turf"; /** * The part of the state which keeps track of user-related stuff, e.g. the OSM-connection, diff --git a/Models/Constants.ts b/Models/Constants.ts index c431c4344..46ac92998 100644 --- a/Models/Constants.ts +++ b/Models/Constants.ts @@ -2,7 +2,7 @@ import {Utils} from "../Utils"; export default class Constants { - public static vNumber = "0.12.3"; + public static vNumber = "0.12.4"; public static ImgurApiKey = '7070e7167f0a25a' public static readonly mapillary_client_token_v3 = 'TXhLaWthQ1d4RUg0czVxaTVoRjFJZzowNDczNjUzNmIyNTQyYzI2' public static readonly mapillary_client_token_v4 = "MLY|4441509239301885|b40ad2d3ea105435bd40c7e76993ae85" @@ -39,6 +39,19 @@ export default class Constants { * (Note that pendingChanges might upload sooner if the popup is closed or similar) */ static updateTimeoutSec: number = 30; + /** + * If the contributor has their GPS location enabled and makes a change, + * the points visited less then `nearbyVisitTime`-seconds ago will be inspected. + * The point closest to the changed feature will be considered and this distance will be tracked. + * ALl these distances are used to calculate a nearby-score + */ + static nearbyVisitTime: number= 30 * 60; + /** + * If a user makes a change, the distance to the changed object is calculated. + * If a user makes multiple changes, all these distances are put into multiple bins, depending on this distance. + * For every bin, the totals are uploaded as metadata + */ + static distanceToChangeObjectBins = [25,50,100,500,1000,5000] private static isRetina(): boolean { if (Utils.runningFromConsole) { diff --git a/UI/AllThemesGui.ts b/UI/AllThemesGui.ts index fe65c8d09..0113ddbc1 100644 --- a/UI/AllThemesGui.ts +++ b/UI/AllThemesGui.ts @@ -15,7 +15,7 @@ export default class AllThemesGui { try { new FixedUiElement("").AttachTo("centermessage") - const state = new UserRelatedState(undefined); + const state = new UserRelatedState(undefined, undefined); const intro = new Combine([ LanguagePicker.CreateLanguagePicker(Translations.t.index.title.SupportedLanguages()) .SetClass("absolute top-2 right-3"), diff --git a/UI/Popup/SplitRoadWizard.ts b/UI/Popup/SplitRoadWizard.ts index 0490f8479..e95d89573 100644 --- a/UI/Popup/SplitRoadWizard.ts +++ b/UI/Popup/SplitRoadWizard.ts @@ -95,7 +95,7 @@ export default class SplitRoadWizard extends Toggle { const points = splitPoints.data.map((f, i) => [f.feature, i]) .filter(p => GeoOperations.distanceBetween(p[0].geometry.coordinates, coordinates) * 1000 < 5) .map(p => p[1]) - .sort() + .sort((a, b) => a - b) .reverse() if (points.length > 0) { for (const point of points) { diff --git a/test/Actors.spec.ts b/test/Actors.spec.ts index 38c2a7c17..17fcee3f1 100644 --- a/test/Actors.spec.ts +++ b/test/Actors.spec.ts @@ -52,7 +52,7 @@ export default class ActorsSpec extends T { [ "download latest version", () => { - const state = new UserRelatedState(AllKnownLayouts.allKnownLayouts.get("bookcases")) + const state = new UserRelatedState(AllKnownLayouts.allKnownLayouts.get("bookcases"), undefined) const feature = { "type": "Feature", "id": "node/5568693115",