From d2efc1a4feb936761006a3a9bed16193d8eda3ec Mon Sep 17 00:00:00 2001
From: Peter Hanecak <115141505+phanecak-maptiler@users.noreply.github.com>
Date: Thu, 22 Aug 2024 12:35:36 +0200
Subject: [PATCH] Fix handling of string typed null values (#987)

* Added unit test for handling of missing (e.g. null) values

- When missing item is NOT typed, than it is properly handld as null,
  e.g. it is NOT included in the generated feature
- When missing item is typed to 'string', it gets converted into "null"
  string value hence generated featire has attribute
  non_existent_typed=null

* Fixed handling of missing or null values for attributes with type 'string'

* clean-up: removed unused import
---
 .../planetiler/expression/DataType.java       |  3 +--
 .../com/onthegomap/planetiler/util/Parse.java |  5 +++++
 .../custommap/ConfiguredFeatureTest.java      |  9 +++++++++
 .../validSchema/tag_attribute_null.yml        | 19 +++++++++++++++++++
 4 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 planetiler-custommap/src/test/resources/validSchema/tag_attribute_null.yml

diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/DataType.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/DataType.java
index de3fdac9b8..04c3b3f229 100644
--- a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/DataType.java
+++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/DataType.java
@@ -2,7 +2,6 @@
 
 import com.onthegomap.planetiler.reader.WithTags;
 import com.onthegomap.planetiler.util.Parse;
-import java.util.Objects;
 import java.util.function.BiFunction;
 import java.util.function.UnaryOperator;
 
@@ -10,7 +9,7 @@
  * Destination data types for an attribute that link the type to functions that can parse the value from an input object
  */
 public enum DataType implements BiFunction<WithTags, String, Object> {
-  GET_STRING("string", WithTags::getString, Objects::toString),
+  GET_STRING("string", WithTags::getString, Parse::parseStringOrNull),
   GET_BOOLEAN("boolean", WithTags::getBoolean, Parse::bool),
   GET_DIRECTION("direction", WithTags::getDirection, Parse::direction),
   GET_LONG("long", WithTags::getLong, Parse::parseLongOrNull),
diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/Parse.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/Parse.java
index 1ddec7c115..13d7a0cbf3 100644
--- a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/Parse.java
+++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/Parse.java
@@ -31,6 +31,11 @@ public class Parse {
 
   private Parse() {}
 
+  /** Returns {@code tag} as a string or null if null. */
+  public static String parseStringOrNull(Object tag) {
+    return tag == null ? null : tag.toString();
+  }
+
   /** Returns {@code tag} as a long or null if invalid. */
   public static Long parseLongOrNull(Object tag) {
     return tag == null ? null : tag instanceof Number number ? Long.valueOf(number.longValue()) :
diff --git a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java
index fc743a8925..64bdc0056d 100644
--- a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java
+++ b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java
@@ -267,6 +267,15 @@ void testTagValueAttributeTest() {
     }, 1);
   }
 
+  @Test
+  void testTagNullValueAttributeTest() {
+    testPolygon(TEST_RESOURCE, "tag_attribute_null.yml", waterTags, f -> {
+      var attr = f.getAttrsAtZoom(14);
+      assertNull(attr.get("non_existent"));
+      assertNull(attr.get("non_existent_typed"));
+    }, 1);
+  }
+
   @Test
   void testTagIncludeAttributeTest() {
     testPolygon(TEST_RESOURCE, "tag_include.yml", waterTags, f -> {
diff --git a/planetiler-custommap/src/test/resources/validSchema/tag_attribute_null.yml b/planetiler-custommap/src/test/resources/validSchema/tag_attribute_null.yml
new file mode 100644
index 0000000000..3ea5257572
--- /dev/null
+++ b/planetiler-custommap/src/test/resources/validSchema/tag_attribute_null.yml
@@ -0,0 +1,19 @@
+schema_name: Test Case Schema
+schema_description: Test case tile schema
+attribution: Test attribution
+sources:
+  osm:
+    type: osm
+    url: geofabrik:rhode-island
+layers:
+- id: testLayer
+  features:
+  - source:
+    - osm
+    geometry: polygon
+    include_when:
+      natural: water
+    attributes:
+    - key: non_existent
+    - key: non_existent_typed
+      type: string