HARP-11013: Support GeoJson feature ids (#2104)

* HARP-11013: Support string feature ids and deprecate number ids.

* HARP-11013: Get GeoJSON feature id on decoding.

@here/harp-datasource-protocol/lib/DecodedTile.ts

@@ -87,9 +87,10 @@ export interface PathGeometry {
 
 /**
  * Attributes corresponding to some decoded geometry. It may be either a map
- * of multiple attributes or just a number with the geometry's feature id.
+ * of multiple attributes or just a string with the geometry's feature id (id numbers are
+ * deprecated).
  */
-export type AttributeMap = {} | number;
+export type AttributeMap = {} | string | number;
 
 /**
  * This object keeps textual data together with metadata to place it on the map.
@@ -339,18 +340,21 @@ export function getProjectionName(projection: Projection): string | never {
 
 /**
  * @returns Feature id from the provided attribute map.
+ * @internal
  */
-export function getFeatureId(attributeMap: AttributeMap | undefined): number {
+export function getFeatureId(attributeMap: AttributeMap | undefined): string | number {
     if (attributeMap === undefined) {
         return 0;
     }
 
-    if (typeof attributeMap === "number") {
+    if (typeof attributeMap === "string" || typeof attributeMap === "number") {
         return attributeMap;
-    }
+    } else if (attributeMap.hasOwnProperty("$id")) {
+        const id = (attributeMap as any).$id;
 
-    if (attributeMap.hasOwnProperty("$id")) {
-        return (attributeMap as any).$id as number;
+        if (typeof id === "string" || typeof id === "number") {
+            return id;
+        }
     }
 
     return 0;

@here/harp-mapview-decoder/lib/GeoJsonTiler.ts

@@ -69,7 +69,7 @@ export class GeoJsonTiler implements ITiler {
             buffer: BUFFER, // tile buffer on each side
             lineMetrics: false, // whether to calculate line metrics
             promoteId: null, // name of a feature property to be promoted to feature.id
-            generateId: true, // whether to generate feature ids. Cannot be used with promoteId
+            generateId: false, // whether to generate feature ids. Cannot be used with promoteId
             debug: 0 // logging level (0, 1 or 2)
         });
         index.geojson = input;
@@ -85,27 +85,7 @@ export class GeoJsonTiler implements ITiler {
         const tile = index.getTile(tileKey.level, tileKey.column, tileKey.row);
         if (tile !== null) {
             tile.layer = indexId;
-            for (const feature of tile.features) {
-                feature.originalGeometry = this.getOriginalGeometry(feature, index.geojson);
-            }
         }
         return tile || {};
     }
-
-    private getOriginalGeometry(feature: any, geojson: GeoJson): any {
-        switch (geojson.type) {
-            case "Point":
-            case "MultiPoint":
-            case "LineString":
-            case "MultiLineString":
-            case "Polygon":
-            case "MultiPolygon":
-            case "GeometryCollection":
-                return geojson;
-            case "Feature":
-                return geojson.geometry;
-            case "FeatureCollection":
-                return geojson.features[feature.id].geometry;
-        }
-    }
 }

@here/harp-mapview-decoder/test/GeoJsonTilerTest.ts

@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2021 HERE Europe B.V.
+ * Licensed under Apache 2.0, see full license in LICENSE
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { FeatureCollection } from "@here/harp-datasource-protocol";
+import { TileKey } from "@here/harp-geoutils";
+import { expect } from "chai";
+
+import { GeoJsonTiler } from "../lib/GeoJsonTiler";
+
+const featureCollection: FeatureCollection = {
+    type: "FeatureCollection",
+    features: [
+        {
+            type: "Feature",
+            id: "point id",
+            properties: {},
+
+            geometry: {
+                type: "Point",
+                coordinates: [1, 2]
+            }
+        },
+        {
+            type: "Feature",
+            id: "line id",
+            properties: {},
+
+            geometry: {
+                type: "LineString",
+                coordinates: [
+                    [1, 2],
+                    [3, 4]
+                ]
+            }
+        },
+        {
+            type: "Feature",
+            id: "polygon id",
+            properties: {},
+
+            geometry: {
+                type: "Polygon",
+                coordinates: [
+                    [
+                        [1, 2],
+                        [3, 4],
+                        [5, 6]
+                    ]
+                ]
+            }
+        }
+    ]
+};
+
+describe("GeoJsonTiler", function () {
+    let tiler: GeoJsonTiler;
+
+    beforeEach(function () {
+        tiler = new GeoJsonTiler();
+    });
+
+    it("returns features with their original geojson ids", async function () {
+        const indexId = "dummy";
+        await tiler.registerIndex(indexId, featureCollection);
+
+        const tile = (await tiler.getTile(indexId, new TileKey(0, 0, 1))) as any;
+
+        expect(tile.features).has.lengthOf(3);
+        const expectedFeatureIds = featureCollection.features.map(feature => feature.id);
+        const actualFeatureIds: string[] = tile.features.map(
+            (feature: { id: string }) => feature.id
+        );
+        expect(actualFeatureIds).has.members(expectedFeatureIds);
+    });
+});

@here/harp-mapview/lib/DataSource.ts

@@ -225,7 +225,7 @@ export abstract class DataSource extends THREE.EventDispatcher {
      */
     private m_storageLevelOffset: number = 0;
 
-    private readonly m_featureStateMap = new Map<number, ValueMap>();
+    private readonly m_featureStateMap = new Map<number | string, ValueMap>();
 
     /**
      *  An array of ISO 639-1 language codes.
@@ -310,9 +310,9 @@ export abstract class DataSource extends THREE.EventDispatcher {
     /**
      * Gets the state of the given feature id.
      *
-     * @param featureId - The id of the feature.
+     * @param featureId - The id of the feature. Id numbers are deprecated in favor of strings.
      */
-    getFeatureState(featureId: number): ValueMap | undefined {
+    getFeatureState(featureId: number | string): ValueMap | undefined {
         return this.m_featureStateMap.get(featureId);
     }
 
@@ -330,19 +330,19 @@ export abstract class DataSource extends THREE.EventDispatcher {
      * dataSource.setFeatureState(featureId, { enabled: true });
      * ```
      *
-     * @param featureId - The id of the feature.
+     * @param featureId - The id of the feature. Id numbers are deprecated in favor of strings.
      * @param state - The new state of the feature.
      */
-    setFeatureState(featureId: number, state: ValueMap) {
+    setFeatureState(featureId: number | string, state: ValueMap) {
         this.m_featureStateMap.set(featureId, state);
     }
 
     /**
      * Removes the state associated to the given feature.
      *
-     * @param featureId - The id of the feature.
+     * @param featureId - The id of the feature. Id numbers are deprecated in favor of strings.
      */
-    removeFeatureState(featureId: number) {
+    removeFeatureState(featureId: number | string) {
         this.m_featureStateMap.delete(featureId);
     }
 

@here/harp-mapview/lib/PickHandler.ts

@@ -83,10 +83,11 @@ export interface PickResult {
     renderOrder?: number;
 
     /**
-     * An optional feature ID of the picked object; typically applies to the Optimized Map
-     * Vector (OMV) format.
+     * An optional feature ID of the picked object.
+     * @remarks The ID may be assigned by the object's {@link DataSource}, for example in case of
+     * Optimized Map Vector (OMV) and GeoJSON datata sources.
      */
-    featureId?: number;
+    featureId?: number | string;
 
     /**
      * Defined for geometry only.

@here/harp-mapview/lib/geometry/TileDataAccessor.ts

@@ -31,56 +31,68 @@ export interface ITileDataVisitor {
     /**
      * Should return `true` if the visitor wants to visit the object with the specified
      * `featureId`. This function is called before the type of the object is even known.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    wantsFeature(featureId: number | undefined): boolean;
+    wantsFeature(featureId: number | string | undefined): boolean;
 
     /**
      * Should return `true` if the visitor wants to visit the point with the specified
      * `featureId`.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    wantsPoint(featureId: number | undefined): boolean;
+    wantsPoint(featureId: number | string | undefined): boolean;
 
     /**
      * Should return `true` if the visitor wants to visit the line with the specified
      * `featureId`.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    wantsLine(featureId: number | undefined): boolean;
+    wantsLine(featureId: number | string | undefined): boolean;
 
     /**
      * Should return `true` if the visitor wants to visit the area object with the specified
      * `featureId`.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    wantsArea(featureId: number | undefined): boolean;
+    wantsArea(featureId: number | string | undefined): boolean;
 
     /**
      * Should return `true` if the visitor wants to visit the object with the specified
      * `featureId`.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    wantsObject3D(featureId: number | undefined): boolean;
+    wantsObject3D(featureId: number | string | undefined): boolean;
 
     /**
      * Visits a point object with the specified `featureId`; use `pointAccessor` to get the
      * object's properties.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    visitPoint(featureId: number | undefined): void;
+    visitPoint(featureId: number | string | undefined): void;
 
     /**
      * Visits a line object with the specified `featureId`; use `pointAccessor` to get the
      * object's properties.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    visitLine(featureId: number | undefined, lineAccessor: ILineAccessor): void;
+    visitLine(featureId: number | string | undefined, lineAccessor: ILineAccessor): void;
 
     /**
      * Visit an area object with the specified `featureId`; use `pointAccessor` to get the
      * object's properties.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    visitArea(featureId: number | undefined): void;
+    visitArea(featureId: number | string | undefined): void;
 
     /**
      * Visits a 3D object with the specified `featureId`; use `pointAccessor` to get the
      * object's properties.
+     * @remarks Number ids are deprecated in favor of strings.
      */
-    visitObject3D(featureId: number | undefined, object3dAccessor: IObject3dAccessor): void;
+    visitObject3D(
+        featureId: number | string | undefined,
+        object3dAccessor: IObject3dAccessor
+    ): void;
 }
 
 /**

@here/harp-mapview/lib/poi/PoiBuilder.ts

@@ -146,7 +146,6 @@ export class PoiBuilder {
             renderTextDuringMovements,
             mayOverlap: iconMayOverlap,
             reserveSpace: iconReserveSpace,
-            featureId: textElement.featureId,
             iconBrightness: technique.iconBrightness,
             iconColor,
             iconMinZoomLevel: this.m_iconMinZoomLevel,

@here/harp-mapview/lib/text/TextElement.ts

@@ -30,6 +30,7 @@ import { TextElementType } from "./TextElementType";
 
 /**
  * Additional information for an icon that is to be rendered along with a {@link TextElement}.
+ * @internal
  */
 export interface PoiInfo {
     /**
@@ -121,11 +122,6 @@ export interface PoiInfo {
      */
     isValid?: boolean;
 
-    /**
-     * ID to identify the (POI) icon.
-     */
-    featureId?: number;
-
     /**
      * Reference back to owning {@link TextElement}.
      */
@@ -216,6 +212,7 @@ export enum LoadingState {
 
 /**
  * `TextElement` is used to create 2D text elements (for example, labels).
+ * @internal
  */
 export class TextElement {
     /**
@@ -375,7 +372,8 @@ export class TextElement {
      *              negative value are always rendered, ignoring priorities and allowing overrides.
      * @param xOffset - Optional X offset of this `TextElement` in screen coordinates.
      * @param yOffset - Optional Y offset of this `TextElement` in screen coordinates.
-     * @param featureId - Optional number to identify feature (originated from `OmvDataSource`).
+     * @param featureId - Optional string to identify feature (originated from {@link DataSource}).
+     *                  Number ids are deprecated in favor of strings.
      * @param fadeNear - Distance to the camera (0.0 = camera position, 1.0 = farPlane) at which the
      *              label starts fading out (opacity decreases).
      * @param fadeFar - Distance to the camera (0.0 = camera position, 1.0 = farPlane) at which the
@@ -391,7 +389,7 @@ export class TextElement {
         public priority = 0,
         public xOffset: number = 0,
         public yOffset: number = 0,
-        public featureId?: number,
+        public featureId?: string | number,
         public style?: string,
         public fadeNear?: number,
         public fadeFar?: number,
@@ -506,8 +504,19 @@ export class TextElement {
         this.m_layoutStyle = style;
     }
 
+    /**
+     * @returns Whether this text element has a valid feature id.
+     */
     hasFeatureId(): boolean {
-        return this.featureId !== undefined && this.featureId !== 0;
+        if (this.featureId === undefined) {
+            return false;
+        }
+
+        if (typeof this.featureId === "number") {
+            return this.featureId !== 0;
+        }
+
+        return this.featureId.length > 0;
     }
 
     /**