Skip to content

Commit

Permalink
Fix dissappearing default tags, add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
pietervdvn committed Mar 14, 2022
1 parent a0e59e4 commit e4f062b
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 26 deletions.
2 changes: 1 addition & 1 deletion Logic/Osm/Changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ export class Changes {

}

private CreateChangesetObjects(changes: ChangeDescription[], downloadedOsmObjects: OsmObject[]): {
public CreateChangesetObjects(changes: ChangeDescription[], downloadedOsmObjects: OsmObject[]): {
newObjects: OsmObject[],
modifiedObjects: OsmObject[]
deletedObjects: OsmObject[]
Expand Down
58 changes: 34 additions & 24 deletions Logic/Osm/ChangesetHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,14 @@ export class ChangesetHandler {
}

/**
* Inplace rewrite of extraMetaTags
* If the metatags contain a special motivation of the format "<change-type>:node/-<number>", this method will rewrite this negative number to the actual ID
* The key is changed _in place_; true will be returned if a change has been applied
* @param extraMetaTags
* @param rewriteIds
* @private
*/
private static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map<string, string>) {
static rewriteMetaTags(extraMetaTags: ChangesetTag[], rewriteIds: Map<string, string>) {
let hasChange = false;
for (const tag of extraMetaTags) {
const match = tag.key.match(/^([a-zA-Z0-9_]+):(node\/-[0-9])$/)
Expand Down Expand Up @@ -92,6 +93,8 @@ export class ChangesetHandler {
if (!extraMetaTags.some(tag => tag.key === "comment") || !extraMetaTags.some(tag => tag.key === "theme")) {
throw "The meta tags should at least contain a `comment` and a `theme`"
}

extraMetaTags = [...extraMetaTags, ...this.defaultChangesetTags()]

if (this.userDetails.data.csCount == 0) {
// The user became a contributor!
Expand All @@ -112,7 +115,7 @@ export class ChangesetHandler {
openChangeset.setData(csId);
const changeset = generateChangeXML(csId);
console.trace("Opened a new changeset (openChangeset.data is undefined):", changeset);
const changes = await this.AddChange(csId, changeset)
const changes = await this.UploadChange(csId, changeset)
const hasSpecialMotivationChanges = ChangesetHandler.rewriteMetaTags(extraMetaTags, changes)
if(hasSpecialMotivationChanges){
// At this point, 'extraMetaTags' will have changed - we need to set the tags again
Expand All @@ -139,11 +142,12 @@ export class ChangesetHandler {
return;
}

const rewritings = await this.AddChange(
const rewritings = await this.UploadChange(
csId,
generateChangeXML(csId))

await this.RewriteTagsOf(extraMetaTags, rewritings, oldChangesetMeta)
const rewrittenTags = this.RewriteTagsOf(extraMetaTags, rewritings, oldChangesetMeta)
await this.UpdateTags(csId, rewrittenTags)

} catch (e) {
console.warn("Could not upload, changeset is probably closed: ", e);
Expand All @@ -153,23 +157,22 @@ export class ChangesetHandler {
}

/**
* Updates the metatag of a changeset -
* Given an existing changeset with metadata and extraMetaTags to add, will fuse them to a new set of metatags
* Does not yet send data
* @param extraMetaTags: new changeset tags to add/fuse with this changeset
* @param rewriteIds: the mapping of ids
* @param oldChangesetMeta: the metadata-object of the already existing changeset
* @constructor
* @private
*/
private async RewriteTagsOf(extraMetaTags: ChangesetTag[],
public RewriteTagsOf(extraMetaTags: ChangesetTag[],
rewriteIds: Map<string, string>,
oldChangesetMeta: {
open: boolean,
id: number
uid: number, // User ID
changes_count: number,
tags: any
}) {
}) : ChangesetTag[] {

const csId = oldChangesetMeta.id

// Note: extraMetaTags is where all the tags are collected into

Expand Down Expand Up @@ -214,10 +217,8 @@ export class ChangesetHandler {


ChangesetHandler.rewriteMetaTags(extraMetaTags, rewriteIds)

await this.UpdateTags(csId, extraMetaTags)


return extraMetaTags

}

private handleIdRewrite(node: any, type: string): [string, string] {
Expand Down Expand Up @@ -295,7 +296,7 @@ export class ChangesetHandler {
})
}

private async GetChangesetMeta(csId: number): Promise<{
async GetChangesetMeta(csId: number): Promise<{
id: number,
open: boolean,
uid: number,
Expand Down Expand Up @@ -340,21 +341,30 @@ export class ChangesetHandler {
});
})
}

private defaultChangesetTags() : ChangesetTag[]{
return [ ["created_by", `MapComplete ${Constants.vNumber}`],
["locale", Locale.language.data],
["host", `${window.location.origin}${window.location.pathname}`],
["source", this.changes.state["currentUserLocation"]?.features?.data?.length > 0 ? "survey" : undefined],
["imagery", this.changes.state["backgroundLayer"]?.data?.id]].map(([key, value]) => ({
key, value, aggretage: false
}))
}

/**
* Opens a changeset with the specified tags
* @param changesetTags
* @constructor
* @private
*/
private OpenChangeset(
changesetTags: ChangesetTag[]
): Promise<number> {
const self = this;
return new Promise<number>(function (resolve, reject) {

const metadata = [
["created_by", `MapComplete ${Constants.vNumber}`],
["locale", Locale.language.data],
["host", `${window.location.origin}${window.location.pathname}`],
["source", self.changes.state["currentUserLocation"]?.features?.data?.length > 0 ? "survey" : undefined],
["imagery", self.changes.state["backgroundLayer"]?.data?.id],
...changesetTags.map(cstag => [cstag.key, cstag.value])
]
const metadata = changesetTags.map(cstag => [cstag.key, cstag.value])
.filter(kv => (kv[1] ?? "") !== "")
.map(kv => `<tag k="${kv[0]}" v="${escapeHtml(kv[1])}"/>`)
.join("\n")
Expand Down Expand Up @@ -382,7 +392,7 @@ export class ChangesetHandler {
/**
* Upload a changesetXML
*/
private AddChange(changesetId: number,
private UploadChange(changesetId: number,
changesetXML: string): Promise<Map<string, string>> {
const self = this;
return new Promise(function (resolve, reject) {
Expand Down
51 changes: 51 additions & 0 deletions test/Changes.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import T from "./TestHelper";
import {Changes} from "../Logic/Osm/Changes";
import {ChangeDescription, ChangeDescriptionTools} from "../Logic/Osm/Actions/ChangeDescription";

export default class ChangesSpec extends T {

constructor() {
super([
["Generate preXML from changeDescriptions", () => {
const changeDescrs: ChangeDescription[] = [
{
type: "node",
id: -1,
changes: {
lat: 42,
lon: -8
},
tags: [{k: "someKey", v: "someValue"}],
meta: {
changeType: "create",
theme: "test"
}
},
{
type: "node",
id: -1,
tags: [{k: 'foo', v: 'bar'}],
meta: {
changeType: "answer",
theme: "test"
}
}
]
const c = new Changes()
const descr = c.CreateChangesetObjects(
changeDescrs,
[]
)
T.equals(0, descr.modifiedObjects.length)
T.equals(0, descr.deletedObjects.length)
T.equals(1, descr.newObjects.length)
const ch = descr.newObjects[0]
T.equals("bar", ch.tags["foo"])
T.equals("someValue", ch.tags["someKey"])
}]
]);

}


}
181 changes: 181 additions & 0 deletions test/ChangesetHandler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
import T from "./TestHelper";
import {ChangesetHandler, ChangesetTag} from "../Logic/Osm/ChangesetHandler";
import {UIEventSource} from "../Logic/UIEventSource";
import {OsmConnection} from "../Logic/Osm/OsmConnection";
import {ElementStorage} from "../Logic/ElementStorage";
import {Changes} from "../Logic/Osm/Changes";

export default class ChangesetHandlerSpec extends T {

private static asDict(tags: {key: string, value: string | number}[]) : Map<string, string | number>{
const d= new Map<string, string | number>()

for (const tag of tags) {
d.set(tag.key, tag.value)
}

return d
}

constructor() {
super([
[
"Test rewrite tags", () => {
const cs = new ChangesetHandler(new UIEventSource<boolean>(true),
new OsmConnection({}),
new ElementStorage(),
new Changes(),
new UIEventSource(undefined)
);


const oldChangesetMeta = {
"type": "changeset",
"id": 118443748,
"created_at": "2022-03-13T19:52:10Z",
"closed_at": "2022-03-13T20:54:35Z",
"open": false,
"user": "Pieter Vander Vennet",
"uid": 3818858,
"minlat": 51.0361902,
"minlon": 3.7092939,
"maxlat": 51.0364194,
"maxlon": 3.7099520,
"comments_count": 0,
"changes_count": 3,
"tags": {
"answer": "5",
"comment": "Adding data with #MapComplete for theme #toerisme_vlaanderen",
"created_by": "MapComplete 0.16.6",
"host": "https://mapcomplete.osm.be/toerisme_vlaanderen.html",
"imagery": "osm",
"locale": "nl",
"source": "survey",
"source:node/-1":"note/1234",
"theme": "toerisme_vlaanderen",
}
}
let rewritten = cs.RewriteTagsOf(
[{
key: "newTag",
value: "newValue",
aggregate: false
}],
new Map<string, string>(),
oldChangesetMeta)
let d = ChangesetHandlerSpec.asDict(rewritten)
T.equals(10, d.size)
T.equals("5", d.get("answer"))
T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
T.equals("MapComplete 0.16.6", d.get("created_by"))
T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
T.equals("osm", d.get("imagery"))
T.equals("survey", d.get("source"))
T.equals("note/1234", d.get("source:node/-1"))
T.equals("toerisme_vlaanderen", d.get("theme"))

T.equals("newValue", d.get("newTag"))




rewritten = cs.RewriteTagsOf(
[{
key: "answer",
value: "37",
aggregate: true
}],
new Map<string, string>(),
oldChangesetMeta)
d = ChangesetHandlerSpec.asDict(rewritten)

T.equals(9, d.size)
T.equals("42", d.get("answer"))
T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
T.equals("MapComplete 0.16.6", d.get("created_by"))
T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
T.equals("osm", d.get("imagery"))
T.equals("survey", d.get("source"))
T.equals("note/1234", d.get("source:node/-1"))
T.equals("toerisme_vlaanderen", d.get("theme"))

rewritten = cs.RewriteTagsOf(
[],
new Map<string, string>([["node/-1", "node/42"]]),
oldChangesetMeta)
d = ChangesetHandlerSpec.asDict(rewritten)

T.equals(9, d.size)
T.equals("5", d.get("answer"))
T.equals("Adding data with #MapComplete for theme #toerisme_vlaanderen", d.get("comment"))
T.equals("MapComplete 0.16.6", d.get("created_by"))
T.equals("https://mapcomplete.osm.be/toerisme_vlaanderen.html", d.get("host"))
T.equals("osm", d.get("imagery"))
T.equals("survey", d.get("source"))
T.equals("note/1234", d.get("source:node/42"))
T.equals("toerisme_vlaanderen", d.get("theme"))

},
],[
"Test rewrite on special motivation", () => {


const cs = new ChangesetHandler(new UIEventSource<boolean>(true),
new OsmConnection({}),
new ElementStorage(),
new Changes(),
new UIEventSource(undefined)
);


const oldChangesetMeta = {
"type": "changeset",
"id": 118443748,
"created_at": "2022-03-13T19:52:10Z",
"closed_at": "2022-03-13T20:54:35Z",
"open": false,
"user": "Pieter Vander Vennet",
"uid": 3818858,
"minlat": 51.0361902,
"minlon": 3.7092939,
"maxlat": 51.0364194,
"maxlon": 3.7099520,
"comments_count": 0,
"changes_count": 3,
"tags": {
"answer": "5",
"comment": "Adding data with #MapComplete for theme #toerisme_vlaanderen",
"created_by": "MapComplete 0.16.6",
"host": "https://mapcomplete.osm.be/toerisme_vlaanderen.html",
"imagery": "osm",
"locale": "nl",
"source": "survey",
"source:-1":"note/1234",
"theme": "toerisme_vlaanderen",
}
}

const extraMetaTags : ChangesetTag[] = [
{
key: "created_by",
value:"mapcomplete"
},
{
key: "source:node/-1",
value:"note/1234"
}
]
const changes = new Map<string, string>([["node/-1","node/42"]])
const hasSpecialMotivationChanges = ChangesetHandler.rewriteMetaTags(extraMetaTags, changes)
T.isTrue(hasSpecialMotivationChanges, "Special rewrite did not trigger")
// Rewritten inline by rewriteMetaTags
T.equals("source:node/42", extraMetaTags[1].key)
T.equals("note/1234", extraMetaTags[1].value)
T.equals("created_by", extraMetaTags[0].key)
T.equals("mapcomplete", extraMetaTags[0].value)
}

]
]);
}
}
Loading

0 comments on commit e4f062b

Please sign in to comment.