GEOMESA-3556 Fix SimpleFeatureTypes geometry index flag parsing

docs/user/datastores/index_creation.rst

@@ -96,11 +96,11 @@ Examples
         import org.locationtech.geomesa.utils.interop.SimpleFeatureTypes;
 
         // creates a default attribute index on name and an implicit default z3 and z2 index on geom
-        String spec = "name:String:index=true,dtg:Date,*geom:Point:srid=4325";
+        String spec = "name:String:index=true,dtg:Date,*geom:Point:srid=4326";
         // creates an attribute index on name (with a secondary date index), and a z3 index on geom and dtg
-        spec = "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4325:index='z3:dtg'";
+        spec = "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4326:index='z3:dtg'";
         // creates an attribute index on name (with a secondary date index), and a z3 index on geom and dtg and disables the ID index
-        spec = "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4325:index='z3:dtg';id.index.enabled=false";
+        spec = "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4326:index='z3:dtg';id.index.enabled=false";
 
         SimpleFeatureType sft = SimpleFeatureTypes.createType("myType", spec);
         // alternatively, set user data after parsing the type string (but before calling "createSchema")

geomesa-accumulo/geomesa-accumulo-datastore/src/test/scala/org/locationtech/geomesa/accumulo/index/ConfigurableIndexesTest.scala

@@ -79,7 +79,7 @@ class ConfigurableIndexesTest extends Specification with TestWithFeatureType {
 
     "add another empty index" >> {
       import org.locationtech.geomesa.utils.geotools.RichSimpleFeatureType.RichSimpleFeatureType
-      val updated = SimpleFeatureTypes.mutable(sft)
+      val updated = SimpleFeatureTypes.copy(sft)
       updated.setIndices(sft.getIndices :+ IndexId(Z2Index.name, Z2Index.version, Seq("geom"), IndexMode.ReadWrite))
       ds.updateSchema(sftName, updated)
       val indices = ds.manager.indices(updated)

geomesa-index-api/src/main/scala/org/locationtech/geomesa/index/geotools/GeoMesaDataStore.scala

@@ -11,7 +11,6 @@ package org.locationtech.geomesa.index.geotools
 import com.github.benmanes.caffeine.cache.{AsyncCacheLoader, AsyncLoadingCache, CacheLoader, Caffeine}
 import com.typesafe.scalalogging.LazyLogging
 import org.geotools.api.data._
-import org.geotools.api.feature.`type`.AttributeDescriptor
 import org.geotools.api.feature.simple.SimpleFeatureType
 import org.geotools.api.filter.Filter
 import org.locationtech.geomesa.index.FlushableFeatureWriter
@@ -20,7 +19,6 @@ import org.locationtech.geomesa.index.conf.QueryHints
 import org.locationtech.geomesa.index.conf.partition.TablePartition
 import org.locationtech.geomesa.index.geotools.GeoMesaDataStore.{SchemaCompatibility, VersionKey}
 import org.locationtech.geomesa.index.geotools.GeoMesaDataStoreFactory.GeoMesaDataStoreConfig
-import org.locationtech.geomesa.index.index.attribute.AttributeIndex
 import org.locationtech.geomesa.index.index.id.IdIndex
 import org.locationtech.geomesa.index.planning.DataStoreQueryRunner
 import org.locationtech.geomesa.index.stats.HasGeoMesaStats
@@ -37,7 +35,7 @@ import org.locationtech.geomesa.utils.index.IndexMode
 import org.locationtech.geomesa.utils.io.CloseWithLogging
 
 import java.io.IOException
-import java.util.Collections
+import java.util.{Collections, Locale}
 import java.util.concurrent.{ConcurrentHashMap, TimeUnit}
 import scala.util.Try
 import scala.util.control.NonFatal
@@ -503,42 +501,29 @@ abstract class GeoMesaDataStore[DS <: GeoMesaDataStore[DS]](val config: GeoMesaD
 
     def remapCol(name: String): String = colMap.getOrElse(name, name)
 
-    // check for attributes flagged 'index' and convert them to sft-level user data
-    def indexed(d: AttributeDescriptor): Boolean = {
-      d.getUserData.get(AttributeOptions.OptIndex) match {
-        case i: String if Seq("true", "full").exists(_.equalsIgnoreCase(i)) => true
-        case i if i == null || Seq("false", "none").exists(_.equalsIgnoreCase(i.toString)) => false
-        case i => throw new IllegalArgumentException(s"Configured index coverage '$i' is not valid: expected 'true'")
-      }
-    }
-    sft.getAttributeDescriptors.asScala.foreach { d =>
-      if (indexed(d)) {
-        val existing = {
-          val explicit = sft.getIndices
-          if (explicit.nonEmpty) { explicit } else { previous.getIndices }
+    val indices = {
+      // set directly in the sft
+      val explicitIndices = (previous.getIndices ++ sft.getIndices).map(i => i.copy(attributes = i.attributes.map(remapCol)))
+      // set in index flags or geomesa.indices.enabled
+      val configuredIndices =
+        // don't add in a fid index unless explicitly enabled
+        if (sft.getUserData.get(Configs.EnableFidIndex) == null &&
+            Option(sft.getUserData.get(Configs.EnabledIndices))
+              .forall(!_.toString.split(",").map(_.trim.toLowerCase(Locale.US)).exists(_.matches(s"^${IdIndex.name}:?")))) {
+          GeoMesaFeatureIndexFactory.indices(sft).filterNot(_.name == IdIndex.name)
+        } else {
+          GeoMesaFeatureIndexFactory.indices(sft)
         }
-        if (!existing.exists(e => e.name == AttributeIndex.name && remapCol(e.attributes.head) == d.getLocalName)) {
-          val fields = Seq(d.getLocalName) ++ Option(sft.getGeomField) ++ sft.getDtgField
-          val id = IndexId(AttributeIndex.name, AttributeIndex.version, fields, IndexMode.ReadWrite)
-          sft.setIndices(existing :+ id)
+      // get the unique indices - note: ignores index version
+      (explicitIndices ++ configuredIndices).foldLeft(Seq.empty[IndexId]) { (sum, next) =>
+        if (sum.exists(i => i.name == next.name && i.attributes == next.attributes)) {
+          sum
+        } else {
+          sum :+ next
         }
       }
     }
-
-    // check for new indices and 'enabled indices' changes
-    val indices = {
-      val enabled = if (!sft.getUserData.containsKey(Configs.EnabledIndices)) { Seq.empty } else {
-        GeoMesaFeatureIndexFactory.indices(sft)
-      }
-      val remapped = (previous.getIndices ++ sft.getIndices).map(i => i.copy(attributes = i.attributes.map(remapCol)))
-      (remapped ++ enabled).foldLeft(Seq.empty[IndexId]) { (sum, next) =>
-        // note: ignore index version
-        if (sum.exists(i => i.name == next.name && i.attributes == next.attributes)) { sum } else { sum :+ next }
-      }
-    }
-    if (indices != previous.getIndices) {
-      sft.setIndices(indices)
-    }
+    sft.setIndices(indices)
 
     // preserve any existing user data but overwrite any keys we redefine
     val userData = new java.util.HashMap[AnyRef, AnyRef](previous.getUserData)

geomesa-index-api/src/test/scala/org/locationtech/geomesa/index/index/ConfigurableIndicesTest.scala

@@ -0,0 +1,85 @@
+/***********************************************************************
+ * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Apache License, Version 2.0
+ * which accompanies this distribution and is available at
+ * https://www.apache.org/licenses/LICENSE-2.0
+ ***********************************************************************/
+
+package org.locationtech.geomesa.index.index
+
+import com.typesafe.config.ConfigFactory
+import com.typesafe.scalalogging.LazyLogging
+import org.geotools.api.feature.simple.SimpleFeatureType
+import org.junit.runner.RunWith
+import org.locationtech.geomesa.index.TestGeoMesaDataStore
+import org.locationtech.geomesa.utils.geotools.{SchemaBuilder, SimpleFeatureTypes}
+import org.locationtech.geomesa.utils.io.WithClose
+import org.specs2.mutable.Specification
+import org.specs2.runner.JUnitRunner
+
+@RunWith(classOf[JUnitRunner])
+class ConfigurableIndicesTest extends Specification with LazyLogging {
+
+  import org.locationtech.geomesa.utils.geotools.RichSimpleFeatureType.RichSimpleFeatureType
+
+  def getIndexConfig(sft: SimpleFeatureType): Seq[String] =
+    sft.getIndices.map(i => s"${i.name}=${i.attributes.mkString(":")}").sorted
+
+  "GeoMesaDataStore" should {
+
+    // tests here are the examples from the docs
+
+    "support configurable indices in a spec string" in {
+      WithClose(new TestGeoMesaDataStore(true)) { ds =>
+        // creates a default attribute index on name and an implicit default z3 and z2 index on geom
+        ds.createSchema(SimpleFeatureTypes.createType("test1", "name:String:index=true,dtg:Date,*geom:Point:srid=4326"))
+        // creates an attribute index on name (with a secondary date index), and a z3 index on geom and dtg
+        ds.createSchema(SimpleFeatureTypes.createType("test2", "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4326:index='z3:dtg'"))
+        // creates an attribute index on name (with a secondary date index), and a z3 index on geom and dtg and disables the ID index
+        ds.createSchema(SimpleFeatureTypes.createType("test3", "name:String:index='attr:dtg',dtg:Date,*geom:Point:srid=4326:index='z3:dtg';id.index.enabled=false"))
+        getIndexConfig(ds.getSchema("test1")) mustEqual Seq("attr=name:geom:dtg", "id=", "z2=geom", "z3=geom:dtg")
+        getIndexConfig(ds.getSchema("test2")) mustEqual Seq("attr=name:dtg", "id=", "z3=geom:dtg")
+        getIndexConfig(ds.getSchema("test3")) mustEqual Seq("attr=name:dtg", "z3=geom:dtg")
+      }
+    }
+
+    "support configurable indices with SchemaBuilder" in {
+      val sft =
+        SchemaBuilder.builder()
+          .addString("name").withIndex("attr:dtg") // creates an attribute index on name, with a secondary date index
+          .addInt("age").withIndex() // creates an attribute index on age, with a default secondary index
+          .addDate("dtg") // not a primary index
+          .addPoint("geom", default = true).withIndices("z3:dtg", "z2") // creates a z3 index with dtg, and a z2 index
+          .userData
+          .disableIdIndex() // disables the ID index
+          .build("test1")
+      WithClose(new TestGeoMesaDataStore(true)) { ds =>
+        ds.createSchema(sft)
+        getIndexConfig(ds.getSchema("test1")) mustEqual Seq("attr=age:geom:dtg", "attr=name:dtg", "z2=geom", "z3=geom:dtg")
+      }
+    }
+
+    "support configurable indices with config" in {
+      val config =
+        s"""{
+           |  type-name = test1
+           |  attributes = [
+           |    { name = "name", type = "String", index = "attr:dtg" } // creates an attribute index on name, with a secondary date index
+           |    { name = "age", type = "Int", index = "true" } // creates an attribute index on age, with a default secondary index
+           |    { name = "dtg", type = "Date" } // not a primary index
+           |    { name = "geom", type = "Point", srid = "4326", index = "z3:dtg,z2" } // creates a z3 index with dtg, and a z2 index
+           |  ]
+           |  user-data = {
+           |    "id.index.enabled" = "false" // disables the default ID index
+           |  }
+           |}
+           |""".stripMargin
+      val sft = SimpleFeatureTypes.createType(ConfigFactory.parseString(config), path = None)
+      WithClose(new TestGeoMesaDataStore(true)) { ds =>
+        ds.createSchema(sft)
+        getIndexConfig(ds.getSchema("test1")) mustEqual Seq("attr=age:geom:dtg", "attr=name:dtg", "z2=geom", "z3=geom:dtg")
+      }
+    }
+  }
+}

geomesa-utils-parent/geomesa-utils/src/main/scala/org/locationtech/geomesa/utils/geotools/sft/SimpleFeatureSpec.scala

@@ -121,7 +121,7 @@ object SimpleFeatureSpec {
     protected def builderHook(builder: AttributeTypeBuilder): Unit = {}
   }
 
-  import org.locationtech.geomesa.utils.geotools.SimpleFeatureTypes.AttributeOptions.{OptDefault, OptIndex, OptSrid}
+  import org.locationtech.geomesa.utils.geotools.SimpleFeatureTypes.AttributeOptions.{OptDefault, OptSrid}
 
   private val simpleOptionPattern = Pattern.compile("[a-zA-Z0-9_]+")
 
@@ -189,10 +189,9 @@ object SimpleFeatureSpec {
     }
 
     // default geoms are indicated by the *
-    // we don't allow attribute indexing for geometries
-    override protected def specOptions: Map[String, String] = options - OptDefault - OptIndex
-    override protected def configOptions: Map[String, String] = options - OptIndex
-    override protected def descriptorOptions: Map[String, String] = options - OptIndex
+    override protected def specOptions: Map[String, String] = options - OptDefault
+    override protected def configOptions: Map[String, String] = options
+    override protected def descriptorOptions: Map[String, String] = options
   }
 
   /**

geomesa-utils-parent/geomesa-utils/src/test/scala/org/locationtech/geomesa/utils/geotools/SimpleFeatureTypesTest.scala

@@ -34,7 +34,7 @@ class SimpleFeatureTypesTest extends Specification {
 
   "SimpleFeatureTypes" should {
     "create an sft that" >> {
-      val sft = SimpleFeatureTypes.createType("testing", "id:Integer,dtg:Date,*geom:Point:srid=4326:index=true")
+      val sft = SimpleFeatureTypes.createType("testing", "id:Integer,dtg:Date,*geom:Point:srid=4326")
       "has name \'test\'"  >> { sft.getTypeName mustEqual "testing" }
       "has three attributes" >> { sft.getAttributeCount must be_==(3) }
       "has an id attribute which is " >> {
@@ -46,9 +46,10 @@ class SimpleFeatureTypesTest extends Specification {
         val geomDescriptor = sft.getGeometryDescriptor
         geomDescriptor.getLocalName must be equalTo "geom"
       }
-      "not include index flag for geometry" >> {
+      "include index flag for geometry" >> {
+        val sft = SimpleFeatureTypes.createType("testing", "id:Integer,dtg:Date,*geom:Point:srid=4326:index=true")
         val geomDescriptor = sft.getGeometryDescriptor
-        geomDescriptor.getUserData.get("index") must beNull
+        geomDescriptor.getUserData.get("index") mustEqual "true"
       }
       "encode an sft properly" >> {
         SimpleFeatureTypes.encodeType(sft) must be equalTo s"id:Integer,dtg:Date,*geom:Point:srid=4326"
@@ -139,7 +140,7 @@ class SimpleFeatureTypesTest extends Specification {
     }
 
     "return the indexed attributes (not including the default geometry)" >> {
-      val sft = SimpleFeatureTypes.createType("testing", "id:Integer:index=false,dtg:Date:index=true,*geom:Point:srid=4326:index=true")
+      val sft = SimpleFeatureTypes.createType("testing", "id:Integer:index=false,dtg:Date:index=true,*geom:Point:srid=4326")
       val indexed = sft.getAttributeDescriptors.asScala.collect {
         case d if java.lang.Boolean.valueOf(d.getUserData.get(AttributeOptions.OptIndex).asInstanceOf[String]) => d.getLocalName
       }