From 6c1a1d59f388e3b9e8c41024c5abb6d3948923d4 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Sun, 3 Apr 2022 02:37:31 +0200 Subject: [PATCH] Fix #716 --- Models/Constants.ts | 2 +- Models/ThemeConfig/Conversion/PrepareLayer.ts | 156 ++++++++++----- .../ThemeConfig/Json/RewritableConfigJson.ts | 9 +- .../layers/bicycle_rental/bicycle_rental.json | 2 +- .../layers/waste_disposal/waste_disposal.json | 6 +- langs/layers/nl.json | 24 +-- .../Conversion/PrepareLayer.spec.ts | 177 ++++++++++-------- 7 files changed, 221 insertions(+), 155 deletions(-) diff --git a/Models/Constants.ts b/Models/Constants.ts index cd587284ff..8220054fb3 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.17.1"; + public static vNumber = "0.17.2"; public static ImgurApiKey = '7070e7167f0a25a' public static readonly mapillary_client_token_v4 = "MLY|4441509239301885|b40ad2d3ea105435bd40c7e76993ae85" diff --git a/Models/ThemeConfig/Conversion/PrepareLayer.ts b/Models/ThemeConfig/Conversion/PrepareLayer.ts index 1958535774..79a2eaafe8 100644 --- a/Models/ThemeConfig/Conversion/PrepareLayer.ts +++ b/Models/ThemeConfig/Conversion/PrepareLayer.ts @@ -1,10 +1,9 @@ -import {Conversion, DesugaringContext, Fuse, OnEvery, OnEveryConcat, SetDefault} from "./Conversion"; +import {Conversion, DesugaringContext, Fuse, OnEveryConcat, SetDefault} from "./Conversion"; import {LayerConfigJson} from "../Json/LayerConfigJson"; import {TagRenderingConfigJson} from "../Json/TagRenderingConfigJson"; import {Utils} from "../../../Utils"; -import Translations from "../../../UI/i18n/Translations"; -import {Translation} from "../../../UI/i18n/Translation"; import RewritableConfigJson from "../Json/RewritableConfigJson"; +import Translations from "../../../UI/i18n/Translations"; class ExpandTagRendering extends Conversion { private readonly _state: DesugaringContext; @@ -268,76 +267,145 @@ class ExpandGroupRewrite extends Conversion<{ } -class ExpandRewrite extends Conversion, T[]> { +export class ExpandRewrite extends Conversion, T[]> { constructor() { super("Applies a rewrite", [], "ExpandRewrite"); } - /* Used for left|right group creation and replacement. - * Every 'keyToRewrite' will be replaced with 'target' recursively. This substitution will happen in place in the object 'tr' */ + /** + * Used for left|right group creation and replacement. + * Every 'keyToRewrite' will be replaced with 'target' recursively. This substitution will happen in place in the object 'tr' + * + * // should substitute strings + * const spec = { + * "someKey": "somevalue {xyz}" + * } + * ExpandRewrite.RewriteParts("{xyz}", "rewritten", spec) // => {"someKey": "somevalue rewritten"} + * + */ public static RewriteParts(keyToRewrite: string, target: string | any, tr: T): T { + + const targetIsTranslation = Translations.isProbablyATranslation(target) - function replaceRecursive(transl: string | any) { - - if(transl === keyToRewrite){ + function replaceRecursive(obj: string | any, target) { + + if (obj === keyToRewrite) { return target } - - if (typeof transl === "string") { + + if (typeof obj === "string") { // This is a simple string - we do a simple replace - return transl.replace(keyToRewrite, target) + return obj.replace(keyToRewrite, target) } - if (Array.isArray(transl)) { + if (Array.isArray(obj)) { // This is a list of items - return transl.map(o => replaceRecursive(o)) + return obj.map(o => replaceRecursive(o, target)) } - if(typeof transl === "object"){ - transl = {...transl} - for (const key in transl) { - transl[key] = replaceRecursive(transl[key]) + if (typeof obj === "object") { + obj = {...obj} + + const isTr = targetIsTranslation && Translations.isProbablyATranslation(obj) + + for (const key in obj) { + let subtarget = target + if(isTr && target[key] !== undefined){ + // The target is a translation AND the current object is a translation + // This means we should recursively replace with the translated value + subtarget = target[key] + } + + obj[key] = replaceRecursive(obj[key], subtarget) } - return transl + return obj } - return transl + return obj } - return replaceRecursive(tr) + return replaceRecursive(tr, target) } + /** + * // should convert simple strings + * const spec = >{ + * rewrite: { + * sourceString: ["xyz","abc"], + * into: [ + * ["X", "A"], + * ["Y", "B"], + * ["Z", "C"]], + * }, + * renderings: "The value of xyz is abc" + * } + * new ExpandRewrite().convertStrict(spec, "test") // => ["The value of X is A", "The value of Y is B", "The value of Z is C"] + * + * // should rewrite with translations + * const spec = >{ + * rewrite: { + * sourceString: ["xyz","abc"], + * into: [ + * ["X", {en: "value", nl: "waarde"}], + * ["Y", {en: "some other value", nl: "een andere waarde"}], + * }, + * renderings: {en: "The value of xyz is abc", nl: "De waarde van xyz is abc"} + * } + * const expected = [ + * { + * en: "The value of X is value", + * nl: "De waarde van X is waarde" + * }, + * { + * en: "The value of Y is some other value", + * nl: "De waarde van Y is een andere waarde" + * } + * ] + * new ExpandRewrite().convertStrict(spec, "test") // => expected + */ convert(json: T | RewritableConfigJson, context: string): { result: T[]; errors?: string[]; warnings?: string[]; information?: string[] } { - if(json === null || json === undefined){ + if (json === null || json === undefined) { return {result: []} } - + if (json["rewrite"] === undefined) { - + // not a rewrite return {result: [(json)]} } const rewrite = >json; - const keysToRewrite = rewrite.rewrite - const ts : T[] = [] + const keysToRewrite = rewrite.rewrite + const ts: T[] = [] - for (let i = 0; i < keysToRewrite.sourceString.length; i++){ - const guard = keysToRewrite.sourceString[i]; - for (let j = i + 1; j < keysToRewrite.sourceString.length; j++) { - const toRewrite = keysToRewrite.sourceString[j] - if(toRewrite.indexOf(guard) >= 0){ - throw `${context} Error in rewrite: sourcestring[${i}] is a substring of sourcestring[${j}]: ${guard} will be substituted away before ${toRewrite} is reached.` + {// sanity check: rewrite: ["xyz", "longer_xyz"] is not allowed as "longer_xyz" will never be triggered + for (let i = 0; i < keysToRewrite.sourceString.length; i++) { + const guard = keysToRewrite.sourceString[i]; + for (let j = i + 1; j < keysToRewrite.sourceString.length; j++) { + const toRewrite = keysToRewrite.sourceString[j] + if (toRewrite.indexOf(guard) >= 0) { + throw `${context} Error in rewrite: sourcestring[${i}] is a substring of sourcestring[${j}]: ${guard} will be substituted away before ${toRewrite} is reached.` + } } } } - for (let i = 0; i < keysToRewrite.into[0].length; i++){ + {// sanity check: {rewrite: ["a", "b"] should have the right amount of 'intos' in every case + for (let i = 0; i < rewrite.rewrite.into.length; i++) { + const into = keysToRewrite.into[i] + if(into.length !== rewrite.rewrite.sourceString.length){ + throw `${context}.into.${i} Error in rewrite: there are ${rewrite.rewrite.sourceString.length} keys to rewrite, but entry ${i} has only ${into.length} values` + + } + } + } + + for (let i = 0; i < keysToRewrite.into.length; i++) { let t = Utils.Clone(rewrite.renderings) - for (let i1 = 0; i1 < keysToRewrite.sourceString.length; i1++){ - const key = keysToRewrite.sourceString[i1]; - const target = keysToRewrite.into[i1][i] + for (let j = 0; j < keysToRewrite.sourceString.length; j++) { + const key = keysToRewrite.sourceString[j]; + const target = keysToRewrite.into[i][j] t = ExpandRewrite.RewriteParts(key, target, t) } ts.push(t) @@ -349,22 +417,6 @@ class ExpandRewrite extends Conversion, T[]> { } - -class ExpandRewriteWithFlatten extends Conversion, T[]> { - - private _rewrite = new ExpandRewrite() - - constructor() { - super("Applies a rewrite, the result is flattened if it is an array", [], "ExpandRewriteWithFlatten"); - } - - convert(json: RewritableConfigJson | T, context: string): { result: T[]; errors?: string[]; warnings?: string[]; information?: string[] } { - return undefined; - } - - -} - export class PrepareLayer extends Fuse { diff --git a/Models/ThemeConfig/Json/RewritableConfigJson.ts b/Models/ThemeConfig/Json/RewritableConfigJson.ts index 7efc900b32..eb15c40a7e 100644 --- a/Models/ThemeConfig/Json/RewritableConfigJson.ts +++ b/Models/ThemeConfig/Json/RewritableConfigJson.ts @@ -1,5 +1,3 @@ -import {TagRenderingConfigJson} from "./TagRenderingConfigJson"; - /** * Rewrites and multiplies the given renderings of type T. * @@ -11,8 +9,9 @@ import {TagRenderingConfigJson} from "./TagRenderingConfigJson"; * rewrite: { * sourceString: ["key", "a|b|c"], * into: [ - * ["X","Y", "Z"], - * [0,1,2] + * ["X", 0] + * ["Y", 1], + * ["Z", 2] * ], * renderings: { * "key":"a|b|c" @@ -36,7 +35,7 @@ import {TagRenderingConfigJson} from "./TagRenderingConfigJson"; * * ] * - * + * @see ExpandRewrite */ export default interface RewritableConfigJson { rewrite: { diff --git a/assets/layers/bicycle_rental/bicycle_rental.json b/assets/layers/bicycle_rental/bicycle_rental.json index f2b28a8d20..6166c9952a 100644 --- a/assets/layers/bicycle_rental/bicycle_rental.json +++ b/assets/layers/bicycle_rental/bicycle_rental.json @@ -218,7 +218,7 @@ "city_bike", { "en": "city bikes", - "nl": "Stadsfietsen" + "nl": "stadsfietsen" } ], [ diff --git a/assets/layers/waste_disposal/waste_disposal.json b/assets/layers/waste_disposal/waste_disposal.json index a589d75d1c..1ccf33db4e 100644 --- a/assets/layers/waste_disposal/waste_disposal.json +++ b/assets/layers/waste_disposal/waste_disposal.json @@ -1,7 +1,8 @@ { "id": "waste_disposal", "name": { - "en": "Waste Disposal Bins" + "en": "Waste Disposal Bins", + "nl": "Afvalcontainers voor huishoudelijk afval" }, "description": { "en": "Waste Disposal Bin, medium to large bin for disposal of (household) waste" @@ -109,7 +110,8 @@ "options": [ { "question": { - "en": "Only public access" + "en": "Only public access", + "nl": "Enkel publiek toegankelijke afvalcontainers" }, "osmTags": "access=yes" } diff --git a/langs/layers/nl.json b/langs/layers/nl.json index a9e4ad9d69..51183d4f43 100644 --- a/langs/layers/nl.json +++ b/langs/layers/nl.json @@ -5212,6 +5212,16 @@ } }, "waste_disposal": { + "filter": { + "0": { + "options": { + "0": { + "question": "Enkel publiek toegankelijke afvalcontainers" + } + } + } + }, + "name": "Afvalcontainers voor huishoudelijk afval", "tagRenderings": { "disposal-location": { "mappings": { @@ -5227,17 +5237,7 @@ }, "question": "Waar bevindt deze container zich?" } - }, - "filter": { - "0": { - "options": { - "0": { - "question": "Enkel publiek toegankelijke afvalcontainers" - } - } - } - }, - "name": "Afvalcontainers voor huishoudelijk afval" + } }, "watermill": { "description": "Watermolens", @@ -5292,4 +5292,4 @@ "render": "Watermolens" } } -} +} \ No newline at end of file diff --git a/tests/Models/ThemeConfig/Conversion/PrepareLayer.spec.ts b/tests/Models/ThemeConfig/Conversion/PrepareLayer.spec.ts index b87a683cd5..d85a542e02 100644 --- a/tests/Models/ThemeConfig/Conversion/PrepareLayer.spec.ts +++ b/tests/Models/ThemeConfig/Conversion/PrepareLayer.spec.ts @@ -1,93 +1,106 @@ import {describe} from 'mocha' import {expect} from 'chai' -import {LayoutConfigJson} from "../../../../Models/ThemeConfig/Json/LayoutConfigJson"; import {LayerConfigJson} from "../../../../Models/ThemeConfig/Json/LayerConfigJson"; -import {PrepareTheme} from "../../../../Models/ThemeConfig/Conversion/PrepareTheme"; import {TagRenderingConfigJson} from "../../../../Models/ThemeConfig/Json/TagRenderingConfigJson"; -import LayoutConfig from "../../../../Models/ThemeConfig/LayoutConfig"; -import * as bookcaseLayer from "../../../../assets/generated/layers/public_bookcase.json" -import LayerConfig from "../../../../Models/ThemeConfig/LayerConfig"; -import {ExtractImages} from "../../../../Models/ThemeConfig/Conversion/FixImages"; -import * as cyclofix from "../../../../assets/generated/themes/cyclofix.json" import LineRenderingConfigJson from "../../../../Models/ThemeConfig/Json/LineRenderingConfigJson"; -import {PrepareLayer} from "../../../../Models/ThemeConfig/Conversion/PrepareLayer"; +import {ExpandRewrite, PrepareLayer} from "../../../../Models/ThemeConfig/Conversion/PrepareLayer"; +import RewritableConfigJson from "../../../../Models/ThemeConfig/Json/RewritableConfigJson"; +describe("ExpandRewrite", () => { + it("should do simple substitution", () => { -describe("PrepareLayer", () => { - - it("should expand mappings in map renderings", () => { - const exampleLayer: LayerConfigJson = { - id: "testlayer", - source: { - osmTags: "key=value" - }, - mapRendering: [ - { - "rewrite": { - sourceString: ["left|right", "lr_offset"], - into: [ - ["left", "right"], - [-6, +6] - ] - }, - renderings: { - "color": { - "render": "#888", - "mappings": [ - { - "if": "parking:condition:left|right=free", - "then": "#299921" - }, - { - "if": "parking:condition:left|right=disc", - "then": "#219991" - } - ] - }, - "offset": "lr_offset" - } - } - ] - } - const prep = new PrepareLayer({ - tagRenderings: new Map(), - sharedLayers: new Map() - }) - const result = prep.convertStrict(exampleLayer, "test") - - const expected = { - "id": "testlayer", - "source": {"osmTags": "key=value"}, - "mapRendering": [{ - "color": { - "render": "#888", - "mappings": [{ - "if": "parking:condition:left=free", - "then": "#299921" - }, - {"if": "parking:condition:left=disc", - "then": "#219991"}] - }, - "offset": -6 - }, { - "color": { - "render": "#888", - "mappings": [{ - "if": "parking:condition:right=free", - "then": "#299921" - }, - {"if": "parking:condition:right=disc", - "then": "#219991"}] - }, - "offset": 6 - }], - "titleIcons": [{"render": "defaults", "id": "defaults"}] - } - - - expect(result).deep.eq(expected) + }) + it("should not allow overlapping keys", () => { + const spec = >{ + rewrite: { + sourceString: ["xyz", "longer_xyz"], + into: [["a", "b"], ["A, B"]], + }, + renderings: "The value of xyz is longer_xyz" } - ) + const rewrite = new ExpandRewrite() + expect(() => rewrite.convert(spec, "test")).to.throw + }) +}) + +describe("PrepareLayer", () => { + + it("should expand rewrites in map renderings", () => { + const exampleLayer: LayerConfigJson = { + id: "testlayer", + source: { + osmTags: "key=value" + }, + mapRendering: [ + { + "rewrite": { + sourceString: ["left|right", "lr_offset"], + into: [ + ["left", -6], + [ "right", +6], + ] + }, + renderings: { + "color": { + "render": "#888", + "mappings": [ + { + "if": "parking:condition:left|right=free", + "then": "#299921" + }, + { + "if": "parking:condition:left|right=disc", + "then": "#219991" + } + ] + }, + "offset": "lr_offset" + } + } + ] + } + const prep = new PrepareLayer({ + tagRenderings: new Map(), + sharedLayers: new Map() + }) + const result = prep.convertStrict(exampleLayer, "test") + + const expected = { + "id": "testlayer", + "source": {"osmTags": "key=value"}, + "mapRendering": [{ + "color": { + "render": "#888", + "mappings": [{ + "if": "parking:condition:left=free", + "then": "#299921" + }, + { + "if": "parking:condition:left=disc", + "then": "#219991" + }] + }, + "offset": -6 + }, { + "color": { + "render": "#888", + "mappings": [{ + "if": "parking:condition:right=free", + "then": "#299921" + }, + { + "if": "parking:condition:right=disc", + "then": "#219991" + }] + }, + "offset": 6 + }], + "titleIcons": [{"render": "defaults", "id": "defaults"}] + } + + + expect(result).deep.eq(expected) + }) })