GEOMESA-3347 Fix spurious warning for JSON type inference

* Remove unnecessary identity transforms
locationtech · Apr 10, 2024 · 304c8a6 · 304c8a6
1 parent f613646
commit 304c8a6
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/...vert-json/src/main/scala/org/locationtech/geomesa/convert/json/JsonConverterFactory.scala b/...vert-json/src/main/scala/org/locationtech/geomesa/convert/json/JsonConverterFactory.scala
@@ -19,7 +19,7 @@ import org.locationtech.geomesa.convert.json.JsonConverter._
 import org.locationtech.geomesa.convert.json.JsonConverterFactory.{JsonConfigConvert, JsonFieldConvert, PropNamer}
 import org.locationtech.geomesa.convert2.AbstractConverter.BasicOptions
 import org.locationtech.geomesa.convert2.AbstractConverterFactory.{BasicOptionsConvert, ConverterConfigConvert, FieldConvert, OptionConvert}
-import org.locationtech.geomesa.convert2.TypeInference.{Namer, PathWithValues, TypeWithPath}
+import org.locationtech.geomesa.convert2.TypeInference.{IdentityTransform, Namer, PathWithValues, TypeWithPath}
 import org.locationtech.geomesa.convert2.transforms.Expression
 import org.locationtech.geomesa.convert2.{AbstractConverterFactory, TypeInference}
 import org.locationtech.geomesa.utils.geotools.ObjectType
@@ -97,15 +97,18 @@ class JsonConverterFactory extends AbstractConverterFactory[JsonConverter, JsonC
     // track the 'properties', geometry type and 'id' in each feature
     // use linkedHashMap to retain insertion order
     val props = scala.collection.mutable.LinkedHashMap.empty[String, ListBuffer[Any]]
+    val geoms = ListBuffer.empty[Any]
     var hasId = true
 
     features.take(AbstractConverterFactory.inferSampleSize).foreach { feature =>
       feature.properties.foreach { case (k, v) => props.getOrElseUpdate(k, ListBuffer.empty) += v }
-      props.getOrElseUpdate(JsonConverterFactory.GeoJsonGeometryPath, ListBuffer.empty) += feature.geom
+      geoms += feature.geom
       hasId = hasId && feature.id.isDefined
     }
 
-    val pathsAndValues = props.toSeq.map { case (path, values) => PathWithValues(path, values) }
+    val pathsAndValues =
+      props.toSeq.map { case (path, values) => PathWithValues(path, values) } :+
+          PathWithValues(JsonConverterFactory.GeoJsonGeometryPath, geoms)
     val inferredTypes = TypeInference.infer(pathsAndValues, sft.toRight("inferred-json"), new PropNamer())
 
     val idJsonField = if (hasId) { Some(StringJsonField("id", "$.id", pathIsRoot = false, None)) } else { None }
@@ -185,22 +188,25 @@ class JsonConverterFactory extends AbstractConverterFactory[JsonConverter, JsonC
 
   private def createFieldConfig(typed: TypeWithPath): JsonField = {
     val TypeWithPath(path, inferredType) = typed
-    // account for optional nodes by wrapping transform with a try/null
-    val transform = Some(Expression(s"try(${inferredType.transform.apply(0)},null)"))
+    val transform = inferredType.transform match {
+      case IdentityTransform => None
+      // account for optional nodes by wrapping transform with a try/null
+      case t => Some(Expression(s"try(${t.apply(0)},null)"))
+    }
     if (path.isEmpty) {
       DerivedField(inferredType.name, transform)
     } else if (path == JsonConverterFactory.GeoJsonGeometryPath) {
       GeometryJsonField(inferredType.name, path, pathIsRoot = false, None)
     } else {
       inferredType.typed match {
-        case ObjectType.STRING  => StringJsonField(inferredType.name, path, pathIsRoot = false, transform)
-        case ObjectType.BOOLEAN => BooleanJsonField(inferredType.name, path, pathIsRoot = false, transform)
+        case ObjectType.BOOLEAN =>
+          BooleanJsonField(inferredType.name, path, pathIsRoot = false, transform)
         case ObjectType.LIST =>
           // if type is list, that means the transform is 'identity', but we need to replace it with jsonList.
           // this is due to GeoJsonParsing decoding the json array for us, above
           ArrayJsonField(inferredType.name, path, pathIsRoot = false, Some(Expression("try(jsonList('string',$0),null)")))
         case _ =>
-          logger.warn(s"Unhandled JSON type: $typed")
+          // all other types will be parsed as strings with appropriate transforms
           StringJsonField(inferredType.name, path, pathIsRoot = false, transform)
       }
     }

diff --git a/...convert-json/src/test/scala/org/locationtech/geomesa/convert/json/JsonConverterTest.scala b/...convert-json/src/test/scala/org/locationtech/geomesa/convert/json/JsonConverterTest.scala
@@ -1574,6 +1574,7 @@ class JsonConverterTest extends Specification {
           Seq("name", "demographics_age", "files", "geom")
       sft.getAttributeDescriptors.asScala.map(_.getType.getBinding) mustEqual
           Seq(classOf[String], classOf[Integer], classOf[java.util.List[_]], classOf[Point])
+
       WithClose(SimpleFeatureConverter(sft, inferred.get._2)) { converter =>
         converter must not(beNull)
 

diff --git a/...convert-xml/src/main/scala/org/locationtech/geomesa/convert/xml/XmlConverterFactory.scala b/...convert-xml/src/main/scala/org/locationtech/geomesa/convert/xml/XmlConverterFactory.scala
@@ -16,7 +16,7 @@ import org.locationtech.geomesa.convert.Modes.{ErrorMode, LineMode, ParseMode}
 import org.locationtech.geomesa.convert.xml.XmlConverter._
 import org.locationtech.geomesa.convert.xml.XmlConverterFactory.{XmlConfigConvert, XmlFieldConvert, XmlNamer, XmlOptionsConvert}
 import org.locationtech.geomesa.convert2.AbstractConverterFactory.{ConverterConfigConvert, ConverterOptionsConvert, FieldConvert, OptionConvert}
-import org.locationtech.geomesa.convert2.TypeInference.{Namer, PathWithValues, TypeWithPath}
+import org.locationtech.geomesa.convert2.TypeInference.{IdentityTransform, Namer, PathWithValues, TypeWithPath}
 import org.locationtech.geomesa.convert2.transforms.Expression
 import org.locationtech.geomesa.convert2.{AbstractConverterFactory, TypeInference}
 import org.locationtech.geomesa.utils.io.WithClose
@@ -109,8 +109,11 @@ class XmlConverterFactory extends AbstractConverterFactory[XmlConverter, XmlConf
             val inferredTypes = TypeInference.infer(pathsAndValues, sft.toRight("inferred-xml"), namer)
 
             val fieldConfig = inferredTypes.types.map { case TypeWithPath(path, inferredType) =>
-              // account for optional nodes by wrapping transform with a try/null
-              val transform = Some(Expression(s"try(${inferredType.transform.apply(0)},null)"))
+              val transform = inferredType.transform match {
+                case IdentityTransform => None
+                // account for optional nodes by wrapping transform with a try/null
+                case t => Some(Expression(s"try(${t.apply(0)},null)"))
+              }
               if (path.isEmpty) {
                 DerivedField(inferredType.name, transform)
               } else {