Refactoring: simplify preferences handling

scripts/Script.ts

@@ -24,7 +24,7 @@ export default abstract class Script {
             })
             .catch((e) => {
                 console.log(`ERROR in script ${process.argv[1]}:`, e)
-                process.exit(1)
+               // process.exit(1)
             })
     }
 

src/Logic/Osm/OsmConnection.ts

@@ -154,7 +154,6 @@ export class OsmConnection {
             console.log("Not authenticated")
         }
     }
-
     public GetPreference<T extends string = string>(
         key: string,
         defaultValue: string = undefined,
@@ -162,12 +161,20 @@ export class OsmConnection {
             prefix?: string
         }
     ): UIEventSource<T | undefined> {
-        options ??= {prefix: "mapcomplete-"}
+        const prefix =options?.prefix ?? "mapcomplete-"
-        return <UIEventSource<T>>this.preferencesHandler.GetPreference(key, defaultValue, options)
+        return <UIEventSource<T>>this.preferencesHandler.getPreference(key, defaultValue, prefix)
+
+    }
+    public getPreference<T extends string = string>(
+        key: string,
+        defaultValue: string = undefined,
+        prefix: string = "mapcomplete-"
+    ): UIEventSource<T | undefined> {
+        return <UIEventSource<T>>this.preferencesHandler.getPreference(key, defaultValue, prefix)
     }
 
     public GetLongPreference(key: string, prefix: string = "mapcomplete-"): UIEventSource<string> {
-        return this.preferencesHandler.GetLongPreference(key, prefix)
+        return this.preferencesHandler.getPreference(key, prefix)
     }
 
     public OnLoggedIn(action: (userDetails: UserDetails) => void) {

src/Logic/Osm/OsmPreferences.ts

@@ -2,12 +2,18 @@ import { Store, UIEventSource } from "../UIEventSource"
 import { OsmConnection } from "./OsmConnection"
 import { LocalStorageSource } from "../Web/LocalStorageSource"
 import OSMAuthInstance = OSMAuth.osmAuth
+import { Utils } from "../../Utils"
 
 export class OsmPreferences {
 
-    private normalPreferences: Record<string, UIEventSource<string>> = {}
+    private preferences: Record<string, UIEventSource<string>> = {}
-    private longPreferences: Record<string, UIEventSource<string>> = {}
+
     private localStorageInited: Set<string> = new Set()
+    /**
+     * Contains all the keys as returned by the OSM-preferences.
+     * Used to clean up old preferences
+     */
+    private seenKeys: string[] = []
 
     private readonly _allPreferences: UIEventSource<Record<string, string>> = new UIEventSource({})
     public readonly allPreferences: Store<Readonly<Record<string, string>>> = this._allPreferences
@@ -25,15 +31,6 @@ export class OsmPreferences {
         })
     }
 
-    private getLongValue(allPrefs: Record<string, string>, key: string): string {
-        const count = Number(allPrefs[key + "-length"])
-        let str = ""
-        for (let i = 0; i < count; i++) {
-            str += allPrefs[key + i]
-        }
-        return str
-
-    }
 
     private setPreferencesAll(key: string, value: string) {
         if (this._allPreferences.data[key] !== value) {
@@ -42,98 +39,50 @@ export class OsmPreferences {
         }
     }
 
-    private initPreference(key: string, value: string = "", excludeFromAll: boolean = false): UIEventSource<string> {
+    private initPreference(key: string, value: string = ""): UIEventSource<string> {
-        if (this.normalPreferences[key] !== undefined) {
+        if (this.preferences[key] !== undefined) {
-            return this.normalPreferences[key]
+            return this.preferences[key]
         }
-        const pref = this.normalPreferences[key] = new UIEventSource(value, "preference: " + key)
+        const pref = this.preferences[key] = new UIEventSource(value, "preference: " + key)
-        if(value && !excludeFromAll){
+        if (value) {
             this.setPreferencesAll(key, value)
         }
         pref.addCallback(v => {
-            this.UploadPreference(key, v)
+            this.uploadKvSplit(key, v)
-            if(!excludeFromAll){
-                this.setPreferencesAll(key, v)
-            }
-        })
-        return pref
-    }
-
-    private initLongPreference(key: string, initialValue: string): UIEventSource<string> {
-        if (this.longPreferences[key] !== undefined) {
-            return this.longPreferences[key]
-        }
-        const pref = this.longPreferences[key] = new UIEventSource<string>(initialValue, "long-preference-"+key)
-        const maxLength = 255
-        const length = UIEventSource.asInt(this.initPreference(key + "-length", "0", true))
-        if(initialValue){
-            this.setPreferencesAll(key, initialValue)
-        }
-        pref.addCallback(v => {
-            if(v === null || v === undefined || v === ""){
-                length.set(null)
-                return
-            }
-            length.set(Math.ceil((v?.length ?? 1) / maxLength))
-            let i = 0
-            while (v.length > 0) {
-                this.UploadPreference(key + "-" + i, v.substring(0, maxLength))
-                i++
-                v = v.substring(maxLength)
-            }
             this.setPreferencesAll(key, v)
         })
         return pref
     }
 
     private async loadBulkPreferences() {
-        const prefs = await this.getPreferencesDict()
+        const prefs = await this.getPreferencesDictDirectly()
-        const isCombined = /-combined-/
+        this.seenKeys = Object.keys(prefs)
-        for (const key in prefs) {
+        const legacy = OsmPreferences.getLegacyCombinedItems(prefs)
-            if (key.endsWith("-combined-length")) {
+        const merged = OsmPreferences.mergeDict(prefs)
-                const v = this.getLongValue(prefs, key.substring(0, key.length - "-length".length))
+        for (const key in merged) {
-                this.initLongPreference(key, v)
-            }
-            if (key.match(isCombined)) {
-                continue
-            }
             this.initPreference(key, prefs[key])
         }
+        for (const key in legacy) {
+            this.initPreference(key, legacy[key])
+        }
     }
 
 
-    /**
+    public getPreference(
-     * OSM preferences can be at most 255 chars.
-     * This method chains multiple together.
-     * Values written into this key will be erased when the user logs in
-     */
-    public GetLongPreference(key: string, prefix: string = "mapcomplete-"): UIEventSource<string> {
-        return this.getPreferenceSeedFromlocal(key, true, undefined, { prefix })
-    }
-
-    public GetPreference(
         key: string,
         defaultValue: string = undefined,
-        options?: {
+        prefix?: string,
-            documentation?: string
-            prefix?: string
-        },
     ) {
-        return this.getPreferenceSeedFromlocal(key, false, defaultValue, options)
+        return this.getPreferenceSeedFromlocal(key, defaultValue, { prefix })
     }
 
     /**
      * Gets a OSM-preference.
      * The OSM-preference is cached in local storage and updated from the OSM.org as soon as those values come in.
      * THis means that values written before being logged in might be erased by the cloud settings
-     * @param key
-     * @param defaultValue
-     * @param options
-     * @constructor
      */
     private getPreferenceSeedFromlocal(
         key: string,
-        long: boolean,
         defaultValue: string = undefined,
         options?: {
             prefix?: string,
@@ -146,17 +95,11 @@ export class OsmPreferences {
         key = key.replace(/[:/"' {}.%\\]/g, "")
 
 
-        let pref : UIEventSource<string>
         const localStorage = LocalStorageSource.Get(key)
-        if(localStorage.data === "null" || localStorage.data === "undefined"){
+        if (localStorage.data === "null" || localStorage.data === "undefined") {
             localStorage.set(undefined)
         }
-        if(long){
+        let pref: UIEventSource<string> = this.initPreference(key, localStorage.data ?? defaultValue)
-            pref = this.initLongPreference(key, localStorage.data ?? defaultValue)
-        }else{
-            pref = this.initPreference(key, localStorage.data  ?? defaultValue)
-        }
-
         if (this.localStorageInited.has(key)) {
             return pref
         }
@@ -173,12 +116,67 @@ export class OsmPreferences {
         this.removeAllWithPrefix("")
     }
 
+    /**
+     *
+     * OsmPreferences.mergeDict({abc: "123", def: "123", "def:0": "456", "def:1":"789"}) // => {abc: "123", def: "123456789"}
+     */
+    private static mergeDict(dict: Record<string, string>): Record<string, string> {
+        const newDict = {}
+
+        const allKeys: string[] = Object.keys(dict)
+        const normalKeys = allKeys.filter(k => !k.match(/[a-z-_0-9A-Z]*:[0-9]+/))
+        for (const normalKey of normalKeys) {
+            if(normalKey.match(/-combined-[0-9]*$/) || normalKey.match(/-combined-length$/)){
+                // Ignore legacy keys
+                continue
+            }
+            const partKeys = OsmPreferences.keysStartingWith(allKeys, normalKey)
+            const parts = partKeys.map(k => dict[k])
+            newDict[normalKey] = parts.join("")
+        }
+        return newDict
+    }
+
+    /**
+     * Gets all items which have a 'combined'-string, the legacy long preferences
+     *
+     *   const input = {
+     *             "extra-noncombined-key":"xyz",
+     *             "mapcomplete-unofficial-theme-httpsrawgithubusercontentcomosm-catalanwikidataimgmainwikidataimgjson-combined-0":
+     *                 "{\"id\":\"https://raw.githubusercontent.com/osm-catalan/wikidataimg/main/wikidataimg.json\",\"icon\":\"https://upload.wikimedia.org/wikipedia/commons/5/50/Yes_Check_Circle.svg\",\"title\":{\"ca\":\"wikidataimg\",\"_context\":\"themes:wikidataimg.title\"},\"shortDescription\"",
+     *             "mapcomplete-unofficial-theme-httpsrawgithubusercontentcomosm-catalanwikidataimgmainwikidataimgjson-combined-1":
+     *                 ":{\"ca\":\"Afegeix imatges d'articles de wikimedia\",\"_context\":\"themes:wikidataimg\"}}",
+     *         }
+     *         const merged = OsmPreferences.getLegacyCombinedItems(input)
+     *         const data = merged["mapcomplete-unofficial-theme-httpsrawgithubusercontentcomosm-catalanwikidataimgmainwikidataimgjson"]
+     *         JSON.parse(data) // =>  {"id": "https://raw.githubusercontent.com/osm-catalan/wikidataimg/main/wikidataimg.json", "icon": "https://upload.wikimedia.org/wikipedia/commons/5/50/Yes_Check_Circle.svg","title": { "ca": "wikidataimg", "_context": "themes:wikidataimg.title" }, "shortDescription": {"ca": "Afegeix imatges d'articles de wikimedia","_context": "themes:wikidataimg"}}
+     *         merged["extra-noncombined-key"] // => undefined
+     */
+    public static getLegacyCombinedItems(dict: Record<string, string>): Record<string, string> {
+        const merged: Record<string, string> = {}
+        const keys = Object.keys(dict)
+        const toCheck =Utils.NoNullInplace( Utils.Dedup(keys.map(k => k.match(/(.*)-combined-[0-9]+$/)?.[1])))
+        for (const key of toCheck) {
+            let i = 0
+            let str = ""
+            let v: string
+            do {
+                v = dict[key + "-combined-" + i]
+                str += v ?? ""
+                i++
+            } while (v !== undefined)
+            merged[key] = str
+        }
+
+
+        return merged
+    }
 
     /**
      * Bulk-downloads all preferences
      * @private
      */
-    private getPreferencesDict(): Promise<Record<string, string>> {
+    private getPreferencesDictDirectly(): Promise<Record<string, string>> {
         return new Promise<Record<string, string>>((resolve, reject) => {
             this.auth.xhr(
                 {
@@ -206,10 +204,51 @@ export class OsmPreferences {
     }
 
     /**
-     * UPloads the given k=v to the OSM-server
+     * Returns all keys matching `k:[number]`
-     * Deletes it if 'v' is undefined, null or empty
+     * Split separately for test
+     *
+     * const keys = ["abc", "def", "ghi", "ghi:0", "ghi:1"]
+     * OsmPreferences.keysStartingWith(keys, "xyz") // => []
+     * OsmPreferences.keysStartingWith(keys, "abc") // => ["abc"]
+     * OsmPreferences.keysStartingWith(keys, "ghi") // => ["ghi", "ghi:0", "ghi:1"]
+     *
      */
-    private UploadPreference(k: string, v: string) {
+    private static keysStartingWith(allKeys: string[], key: string): string[] {
+        const keys = allKeys.filter(k =>  k === key || k.match(new RegExp(key + ":[0-9]+")))
+        keys.sort()
+        return keys
+    }
+
+    /**
+     * Smart 'upload', which splits the value into `k`, `k:0`, `k:1` if needed.
+     * If `v` is null, undefined, empty, "undefined" (literal string) or "null" (literal string), will delete `k` and `k:[number]`
+     *
+     */
+    private async uploadKvSplit(k: string, v: string) {
+
+        if (v === null || v === undefined || v === "" || v === "undefined" || v === "null") {
+            const keysToDelete = OsmPreferences.keysStartingWith(this.seenKeys, k)
+            await Promise.all(keysToDelete.map(k => this.deleteKeyDirectly(k)))
+            return
+        }
+
+
+        await this.uploadKeyDirectly(k, v.slice(0, 255))
+        v = v.slice(255)
+        let i = 0
+        while (v.length > 0) {
+            await this.uploadKeyDirectly(`${k}:${i}`, v.slice(0, 255))
+            v = v.slice(255)
+        }
+
+    }
+
+    /**
+     * Directly deletes this key
+     * @param k
+     * @private
+     */
+    private deleteKeyDirectly(k: string) {
         if (!this.osmConnection.userDetails.data.loggedIn) {
             console.debug(`Not saving preference ${k}: user not logged in`)
             return
@@ -218,7 +257,8 @@ export class OsmPreferences {
         if (this._fakeUser) {
             return
         }
-        if (v === undefined || v === "" || v === null) {
+        return new Promise<void>((resolve, reject) => {
+
                 this.auth.xhr(
                     {
                         method: "DELETE",
@@ -228,14 +268,42 @@ export class OsmPreferences {
                     (error) => {
                         if (error) {
                             console.warn("Could not remove preference", error)
+                            reject(error)
                             return
                         }
                         console.debug("Preference ", k, "removed!")
+                        resolve()
                     },
                 )
+            },
+        )
+    }
+
+    /**
+     * Uploads the given k=v to the OSM-server
+     * Deletes it if 'v' is undefined, null or empty
+     */
+    private async uploadKeyDirectly(k: string, v: string) {
+        if (!this.osmConnection.userDetails.data.loggedIn) {
+            console.debug(`Not saving preference ${k}: user not logged in`)
             return
         }
 
+        if (this._fakeUser) {
+            return
+        }
+        if (v === undefined || v === "" || v === null) {
+            await this.deleteKeyDirectly(k)
+            return
+        }
+
+        if (v.length > 255) {
+            console.error("Preference too long, max 255 chars", { k, v })
+            throw "Preference too long, at most 255 characters are supported"
+        }
+
+        return new Promise<void>((resolve, reject) => {
+
             this.auth.xhr(
                 {
                     method: "PUT",
@@ -246,35 +314,28 @@ export class OsmPreferences {
                 (error) => {
                     if (error) {
                         console.warn(`Could not set preference "${k}"'`, error)
+                        reject(error)
                         return
                     }
+                    resolve()
                 },
             )
+        })
     }
 
-    removeAllWithPrefix(prefix: string) {
+    async removeAllWithPrefix(prefix: string) {
-        for (const key in this.normalPreferences) {
+        const keys = this.seenKeys
-            if(key.startsWith(prefix)){
+        for (const key in keys) {
-                this.normalPreferences[key].set(null)
+            await this.deleteKeyDirectly(key)
-            }
-        }
-        for (const key in this.longPreferences) {
-            if(key.startsWith(prefix)){
-                this.longPreferences[key].set(null)
-            }
         }
     }
 
-    getExistingPreference(key: string, defaultValue: undefined, prefix: string ): UIEventSource<string> {
+    getExistingPreference(key: string, defaultValue: undefined, prefix: string): UIEventSource<string> {
         if (prefix) {
             key = prefix + key
         }
         key = key.replace(/[:/"' {}.%\\]/g, "")
-
+        return this.preferences[key]
-        if(this.normalPreferences[key]){
-            return this.normalPreferences[key]
-        }
-        return this.longPreferences[key]
 
     }
 }

src/Logic/State/FeatureSwitchState.ts

@@ -6,7 +6,6 @@ import { UIEventSource } from "../UIEventSource"
 import { QueryParameters } from "../Web/QueryParameters"
 import Constants from "../../Models/Constants"
 import { Utils } from "../../Utils"
-import { Query } from "pg"
 import { eliCategory } from "../../Models/RasterLayerProperties"
 import { AvailableRasterLayers } from "../../Models/RasterLayers"
 import MarkdownUtils from "../../Utils/MarkdownUtils"

src/Logic/State/UserRelatedState.ts

@@ -38,7 +38,7 @@ export class OptionallySyncedHistory<T> {
         this.osmconnection = osmconnection
         this._maxHistory = maxHistory
         this._isSame = isSame
-        this.syncPreference = osmconnection.GetPreference(
+        this.syncPreference = osmconnection.getPreference(
             "preference-" + key + "-history",
             "sync",
         )
@@ -189,28 +189,28 @@ export default class UserRelatedState {
         this._mapProperties = mapProperties
 
         this.showAllQuestionsAtOnce = UIEventSource.asBoolean(
-            this.osmConnection.GetPreference("show-all-questions", "false"),
+            this.osmConnection.getPreference("show-all-questions", "false"),
         )
-        this.language = this.osmConnection.GetPreference("language")
+        this.language = this.osmConnection.getPreference("language")
-        this.showTags = this.osmConnection.GetPreference("show_tags")
+        this.showTags = this.osmConnection.getPreference("show_tags")
-        this.showCrosshair = this.osmConnection.GetPreference("show_crosshair")
+        this.showCrosshair = this.osmConnection.getPreference("show_crosshair")
-        this.fixateNorth = this.osmConnection.GetPreference("fixate-north")
+        this.fixateNorth = this.osmConnection.getPreference("fixate-north")
-        this.morePrivacy = this.osmConnection.GetPreference("more_privacy", "no")
+        this.morePrivacy = this.osmConnection.getPreference("more_privacy", "no")
 
-        this.a11y = this.osmConnection.GetPreference("a11y")
+        this.a11y = this.osmConnection.getPreference("a11y")
 
         this.mangroveIdentity = new MangroveIdentity(
-            this.osmConnection.GetLongPreference("identity", "mangrove"),
+            this.osmConnection.getPreference("identity", undefined,"mangrove"),
-            this.osmConnection.GetPreference("identity-creation-date", "mangrove"),
+            this.osmConnection.getPreference("identity-creation-date", undefined,"mangrove"),
         )
-        this.preferredBackgroundLayer = this.osmConnection.GetPreference("preferred-background-layer")
+        this.preferredBackgroundLayer = this.osmConnection.getPreference("preferred-background-layer")
 
-        this.addNewFeatureMode = this.osmConnection.GetPreference(
+        this.addNewFeatureMode = this.osmConnection.getPreference(
             "preferences-add-new-mode",
             "button_click_right",
         )
 
-        this.imageLicense = this.osmConnection.GetPreference("pictures-license", "CC0")
+        this.imageLicense = this.osmConnection.getPreference("pictures-license", "CC0")
         this.installedUserThemes = UserRelatedState.initInstalledUserThemes(osmConnection)
         this.translationMode = this.initTranslationMode()
         this.homeLocation = this.initHomeLocation()
@@ -242,7 +242,7 @@ export default class UserRelatedState {
 
     private initTranslationMode(): UIEventSource<"false" | "true" | "mobile" | undefined | string> {
         const translationMode: UIEventSource<undefined | "true" | "false" | "mobile" | string> =
-            this.osmConnection.GetPreference("translation-mode", "false")
+            this.osmConnection.getPreference("translation-mode", "false")
         translationMode.addCallbackAndRunD((mode) => {
             mode = mode.toLowerCase()
             if (mode === "true" || mode === "yes") {
@@ -301,7 +301,7 @@ export default class UserRelatedState {
             this.osmConnection.isLoggedIn.addCallbackAndRunD((loggedIn) => {
                 if (loggedIn) {
                     this.osmConnection
-                        .GetPreference("hidden-theme-" + layout?.id + "-enabled")
+                        .getPreference("hidden-theme-" + layout?.id + "-enabled")
                         .setData("true")
                     return true
                 }
@@ -516,10 +516,8 @@ export default class UserRelatedState {
                 if(tags[key] === null){
                     continue
                 }
-                let pref = this.osmConnection.preferencesHandler.getExistingPreference(key, undefined, "")
+                let pref = this.osmConnection.GetPreference(key, undefined, {prefix: ""})
-                if (!pref) {
+
-                    pref = this.osmConnection.GetPreference(key, undefined, {prefix: ""})
-                }
                 pref.set(tags[key])
             }
         })

src/Logic/Web/MangroveReviews.ts

@@ -22,6 +22,9 @@ export class MangroveIdentity {
         this.mangroveIdentity = mangroveIdentity
         this._mangroveIdentityCreationDate = mangroveIdentityCreationDate
         mangroveIdentity.addCallbackAndRunD(async (data) => {
+            if(data === ""){
+                return
+            }
             await this.setKeypair(data)
         })
     }

src/UI/AllThemesGui.svelte

@@ -51,17 +51,17 @@
       knownIds.indexOf(theme.id) >= 0 || state.osmConnection.userDetails.data.name === "Pieter Vander Vennet"
     ))
 
-
   const customThemes: Store<MinimalLayoutInformation[]> = Stores.ListStabilized<string>(state.installedUserThemes)
-    .mapD(stableIds => stableIds.map(id => state.getUnofficialTheme(id)))
+    .mapD(stableIds => Utils.NoNullInplace(stableIds.map(id => state.getUnofficialTheme(id))))
-
 
   function filtered(themes: MinimalLayoutInformation[]): Store<MinimalLayoutInformation[]> {
+    const prefiltered = themes.filter(th => th.id !== "personal")
     return searchStable.map(search => {
       if (!search) {
         return themes
       }
-      const scores = ThemeSearch.sortedByLowestScores(search, themes)
+
+      const scores = ThemeSearch.sortedByLowestScores(search, prefiltered)
       const strict = scores.filter(sc => sc.lowest < 2)
       if (strict.length > 0) {
         return strict.map(sc => sc.theme)

test/Logic/OSM/OsmObject.spec.ts

@@ -1,4 +1,3 @@
-import { OsmObject } from "../../../src/Logic/Osm/OsmObject"
 import { Utils } from "../../../src/Utils"
 import ScriptUtils from "../../../scripts/ScriptUtils"
 import { readFileSync } from "fs"