From 9c5b9f1ba97adfceeaeaff630091f57082bb13ca Mon Sep 17 00:00:00 2001
From: Ian Streeter <ian@snowplowanalytics.com>
Date: Fri, 10 Mar 2023 21:35:03 +0000
Subject: [PATCH] BigQuery final type suggestion should always allow null
 (close #153)

---
 .../iglu.schemaddl/bigquery/Field.scala       |  8 +-
 .../iglu/schemaddl/bigquery/FieldSpec.scala   | 82 +++++++++++++++++++
 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/bigquery/Field.scala b/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/bigquery/Field.scala
index 65076d8b..e6c641cb 100644
--- a/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/bigquery/Field.scala
+++ b/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/bigquery/Field.scala
@@ -42,27 +42,27 @@ object Field {
       case Some(types) if types.possiblyWithNull(CommonProperties.Type.Object) =>
         val subfields = topSchema.properties.map(_.value).getOrElse(Map.empty)
         if (subfields.isEmpty) {
-          Suggestion.finalSuggestion(topSchema, required)(name)
+          Suggestion.finalSuggestion(topSchema, required && !types.nullable)(name)
         } else {
           val requiredKeys = topSchema.required.toList.flatMap(_.value)
           val fields = subfields.map { case (key, schema) =>
             build(key, schema, requiredKeys.contains(key))
           }
           val subFields = fields.toList.sortBy(field => (Mode.sort(field.mode), field.name))
-          Field(name, Type.Record(subFields), Mode.required(required))
+          Field(name, Type.Record(subFields), Mode.required(required && !types.nullable))
         }
       case Some(types) if types.possiblyWithNull(CommonProperties.Type.Array) =>
         topSchema.items match {
           case Some(ArrayProperty.Items.ListItems(schema)) =>
             build(name, schema, false).copy(mode = Mode.Repeated)
           case _ =>
-            Suggestion.finalSuggestion(topSchema, required)(name)
+            Suggestion.finalSuggestion(topSchema, required && !types.nullable)(name)
         }
       case _ =>
         Suggestion.suggestions
           .find(suggestion => suggestion(topSchema, required).isDefined)
           .flatMap(_.apply(topSchema, required))
-          .getOrElse(Suggestion.finalSuggestion(topSchema, required))
+          .getOrElse(Suggestion.finalSuggestion(topSchema, required = false))
           .apply(name)
     }
   }
diff --git a/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/bigquery/FieldSpec.scala b/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/bigquery/FieldSpec.scala
index 39a2795b..f6cb9acb 100644
--- a/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/bigquery/FieldSpec.scala
+++ b/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/bigquery/FieldSpec.scala
@@ -23,6 +23,9 @@ class FieldSpec extends org.specs2.Specification { def is = s2"""
   build generates repeated string for empty schema in items $e7
   build generates repeated record for nullable array $e8
   normalName handles camel case and disallowed characters $e9
+  build generates nullable field for oneOf types $e10
+  build generates nullable field for nullable object without nested keys $e11
+  build generates nullable field for nullable array without items $e12
   """
 
   def e1 = {
@@ -231,5 +234,84 @@ class FieldSpec extends org.specs2.Specification { def is = s2"""
       (fieldNormalName("1test1,Test2Test3Test4.test5;test6") must beEqualTo("_1test1_test2_test3_test4_test5_test6"))
   }
 
+  def e10 = {
+    val input = SpecHelpers.parseSchema(
+      """
+        |  {
+        |    "type": "object",
+        |    "required": ["xyz"],
+        |    "properties": {
+        |      "xyz": {
+        |        "oneOf": [
+        |           {"type": "string"},
+        |           {"type": "number"},
+        |           {"type": "null"}
+        |        ]
+        |      }
+        |    }
+        |  }
+      """.stripMargin)
+
+    val expected = Field(
+      "foo",
+      Type.Record(List(
+        Field("xyz", Type.String, Mode.Nullable)
+      )),
+      Mode.Nullable
+    )
+
+    Field.build("foo", input, false) must beEqualTo(expected)
+  }
+
+  def e11 = {
+    val input = SpecHelpers.parseSchema(
+      """
+        |  {
+        |    "type": "object",
+        |    "required": ["xyz"],
+        |    "properties": {
+        |      "xyz": {
+        |        "type": ["object", "null"]
+        |      }
+        |    }
+        |  }
+      """.stripMargin)
+
+    val expected = Field(
+      "foo",
+      Type.Record(List(
+        Field("xyz", Type.String, Mode.Nullable)
+      )),
+      Mode.Nullable
+    )
+
+    Field.build("foo", input, false) must beEqualTo(expected)
+  }
+
+  def e12 = {
+    val input = SpecHelpers.parseSchema(
+      """
+        |  {
+        |    "type": "object",
+        |    "required": ["xyz"],
+        |    "properties": {
+        |      "xyz": {
+        |        "type": ["array", "null"]
+        |      }
+        |    }
+        |  }
+      """.stripMargin)
+
+    val expected = Field(
+      "foo",
+      Type.Record(List(
+        Field("xyz", Type.String, Mode.Nullable)
+      )),
+      Mode.Nullable
+    )
+
+    Field.build("foo", input, false) must beEqualTo(expected)
+  }
+
   private def fieldNormalName(name: String) = Field(name, Type.String, Mode.Nullable).normalName
 }