From 819f65e18d698f9a7b6e466063852b5c2dca1ed0 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Sun, 1 May 2022 04:16:17 +0200 Subject: [PATCH] Add extra optimization on And, add test --- Logic/Tags/And.ts | 140 +++++++++++++++++---------- Logic/Tags/Tag.ts | 6 +- test/Logic/Tags/OptimizeTags.spec.ts | 6 ++ 3 files changed, 97 insertions(+), 55 deletions(-) diff --git a/Logic/Tags/And.ts b/Logic/Tags/And.ts index 340c2161f..719952519 100644 --- a/Logic/Tags/And.ts +++ b/Logic/Tags/And.ts @@ -1,16 +1,19 @@ import {TagsFilter} from "./TagsFilter"; import {Or} from "./Or"; import {TagUtils} from "./TagUtils"; +import {Tag} from "./Tag"; +import {RegexTag} from "./RegexTag"; export class And extends TagsFilter { public and: TagsFilter[] + constructor(and: TagsFilter[]) { super(); this.and = and } - - public static construct(and: TagsFilter[]): TagsFilter{ - if(and.length === 1){ + + public static construct(and: TagsFilter[]): TagsFilter { + if (and.length === 1) { return and[0] } return new And(and) @@ -50,7 +53,7 @@ export class And extends TagsFilter { * * import {Tag} from "./Tag"; * import {RegexTag} from "./RegexTag"; - * + * * const and = new And([new Tag("boundary","protected_area"), new RegexTag("protect_class","98",true)]) * and.asOverpass() // => [ "[\"boundary\"=\"protected_area\"][\"protect_class\"!=\"98\"]" ] */ @@ -142,7 +145,7 @@ export class And extends TagsFilter { usedKeys(): string[] { return [].concat(...this.and.map(subkeys => subkeys.usedKeys())); } - + usedTags(): { key: string; value: string }[] { return [].concat(...this.and.map(subkeys => subkeys.usedTags())); } @@ -161,97 +164,134 @@ export class And extends TagsFilter { * ^---------^ * When the evaluation hits (A=B & X=Y), we know _for sure_ that X=Y does _not_ match, as it would have matched the first clause otherwise. * This means that the entire 'AND' is considered FALSE - * + * * new And([ new Tag("key","value") ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), true) // => new Tag("other_key","value") * new And([ new Tag("key","value") ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), false) // => false * new And([ new RegexTag("key",/^..*$/) ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), true) // => new Tag("other_key","value") * new And([ new Tag("key","value") ]).removePhraseConsideredKnown(new Tag("key","value"), true) // => true - * + * * // should remove 'club~*' if we know that 'club=climbing' * const expr = TagUtils.Tag({and: ["sport=climbing", {or:["club~*", "office~*"]}]} ) * expr.removePhraseConsideredKnown(new Tag("club","climbing"), true) // => new Tag("sport","climbing") - * + * * const expr = TagUtils.Tag({and: ["sport=climbing", {or:["club~*", "office~*"]}]} ) * expr.removePhraseConsideredKnown(new Tag("club","climbing"), false) // => expr */ removePhraseConsideredKnown(knownExpression: TagsFilter, value: boolean): TagsFilter | boolean { const newAnds: TagsFilter[] = [] for (const tag of this.and) { - if(tag instanceof And){ + if (tag instanceof And) { throw "Optimize expressions before using removePhraseConsideredKnown" } - if(tag instanceof Or){ + if (tag instanceof Or) { const r = tag.removePhraseConsideredKnown(knownExpression, value) - if(r === true){ + if (r === true) { continue } - if(r === false){ + if (r === false) { return false; } newAnds.push(r) continue } - if(value && knownExpression.shadows(tag)){ + if (value && knownExpression.shadows(tag)) { /** * At this point, we do know that 'knownExpression' is true in every case * As `shadows` does define that 'tag' MUST be true if 'knownExpression' is true, * we can be sure that 'tag' is true as well. - * + * * "True" is the neutral element in an AND, so we can skip the tag */ continue } - if(!value && tag.shadows(knownExpression)){ + if (!value && tag.shadows(knownExpression)) { /** * We know that knownExpression is unmet. * if the tag shadows 'knownExpression' (which is the case when control flows gets here), * then tag CANNOT be met too, as known expression is not met. - * + * * This implies that 'tag' must be false too! */ // false is the element which absorbs all return false } - + newAnds.push(tag) } - if(newAnds.length === 0){ + if (newAnds.length === 0) { return true } return And.construct(newAnds) } optimize(): TagsFilter | boolean { - if(this.and.length === 0){ + if (this.and.length === 0) { return true } const optimizedRaw = this.and.map(t => t.optimize()) - .filter(t => t !== true /* true is the neutral element in an AND, we drop them*/ ) - if(optimizedRaw.some(t => t === false)){ + .filter(t => t !== true /* true is the neutral element in an AND, we drop them*/) + if (optimizedRaw.some(t => t === false)) { // We have an AND with a contained false: this is always 'false' return false; } - const optimized = optimizedRaw; - - const newAnds : TagsFilter[] = [] - - let containedOrs : Or[] = [] + const optimized = optimizedRaw; + + { + // Conflicting keys do return false + const properties: object = {} + for (const opt of optimized) { + if (opt instanceof Tag) { + properties[opt.key] = opt.value + } + } + for (const opt of optimized) { + if(opt instanceof Tag ){ + const k = opt.key + const v = properties[k] + if(v === undefined){ + continue + } + if(v !== opt.value){ + // detected an internal conflict + return false + } + } + if(opt instanceof RegexTag ){ + const k = opt.key + if(typeof k !== "string"){ + continue + } + const v = properties[k] + if(v === undefined){ + continue + } + if(v !== opt.value){ + // detected an internal conflict + return false + } + } + } + } + + const newAnds: TagsFilter[] = [] + + let containedOrs: Or[] = [] for (const tf of optimized) { - if(tf instanceof And){ + if (tf instanceof And) { newAnds.push(...tf.and) - }else if(tf instanceof Or){ + } else if (tf instanceof Or) { containedOrs.push(tf) } else { newAnds.push(tf) } } - + { let dirty = false; do { - const cleanedContainedOrs : Or[] = [] + const cleanedContainedOrs: Or[] = [] outer: for (let containedOr of containedOrs) { for (const known of newAnds) { // input for optimazation: (K=V & (X=Y | K=V)) @@ -278,10 +318,10 @@ export class And extends TagsFilter { cleanedContainedOrs.push(containedOr) } containedOrs = cleanedContainedOrs - } while(dirty) + } while (dirty) } - - + + containedOrs = containedOrs.filter(ca => { const isShadowed = TagUtils.containsEquivalents(newAnds, ca.or) // If 'isShadowed', then at least one part of the 'OR' is matched by the outer and, so this means that this OR isn't needed at all @@ -290,51 +330,51 @@ export class And extends TagsFilter { }) // Extract common keys from the OR - if(containedOrs.length === 1){ + if (containedOrs.length === 1) { newAnds.push(containedOrs[0]) - }else if(containedOrs.length > 1){ - let commonValues : TagsFilter [] = containedOrs[0].or - for (let i = 1; i < containedOrs.length && commonValues.length > 0; i++){ + } else if (containedOrs.length > 1) { + let commonValues: TagsFilter [] = containedOrs[0].or + for (let i = 1; i < containedOrs.length && commonValues.length > 0; i++) { const containedOr = containedOrs[i]; commonValues = commonValues.filter(cv => containedOr.or.some(candidate => candidate.shadows(cv))) } - if(commonValues.length === 0){ + if (commonValues.length === 0) { newAnds.push(...containedOrs) - }else{ + } else { const newOrs: TagsFilter[] = [] for (const containedOr of containedOrs) { const elements = containedOr.or .filter(candidate => !commonValues.some(cv => cv.shadows(candidate))) newOrs.push(Or.construct(elements)) } - + commonValues.push(And.construct(newOrs)) const result = new Or(commonValues).optimize() - if(result === false){ + if (result === false) { return false - }else if(result === true){ + } else if (result === true) { // neutral element: skip - }else{ + } else { newAnds.push(result) } } } - if(newAnds.length === 0){ + if (newAnds.length === 0) { return true } - if(TagUtils.ContainsOppositeTags(newAnds)){ + if (TagUtils.ContainsOppositeTags(newAnds)) { return false } - + TagUtils.sortFilters(newAnds, true) - + return And.construct(newAnds) } - + isNegative(): boolean { return !this.and.some(t => !t.isNegative()); } - - + + } \ No newline at end of file diff --git a/Logic/Tags/Tag.ts b/Logic/Tags/Tag.ts index 1294c6a53..f8ee64aa0 100644 --- a/Logic/Tags/Tag.ts +++ b/Logic/Tags/Tag.ts @@ -68,7 +68,7 @@ export class Tag extends TagsFilter { if (shorten) { v = Utils.EllipsesAfter(v, 25); } - if (v === "" || v === undefined) { + if (v === "" || v === undefined && currentProperties !== undefined) { // This tag will be removed if in the properties, so we indicate this with special rendering if (currentProperties !== undefined && (currentProperties[this.key] ?? "") === "") { // This tag is not present in the current properties, so this tag doesn't change anything @@ -122,10 +122,6 @@ export class Tag extends TagsFilter { return [{k: this.key, v: this.value}]; } - AsJson() { - return this.asHumanString(false, false) - } - optimize(): TagsFilter | boolean { return this; } diff --git a/test/Logic/Tags/OptimizeTags.spec.ts b/test/Logic/Tags/OptimizeTags.spec.ts index 5229c94a0..2261cb0eb 100644 --- a/test/Logic/Tags/OptimizeTags.spec.ts +++ b/test/Logic/Tags/OptimizeTags.spec.ts @@ -30,6 +30,12 @@ describe("Tag optimalization", () => { const opt = t.optimize() expect(opt).eq(true) }) + + it("should return false on conflicting tags", () => { + const t = new And([new Tag("key","a"), new Tag("key","b")]) + const opt = t.optimize() + expect(opt).eq(false) + }) it("with nested ors and common property should be extracted", () => {