Allow items to have both time ranges and point in time datetimes (#405)

* switch item datetimes from ADT to cats Ior

* remove stale old print

* fix encoder

order is important to avoid accidentally overwriting the datetime
field

* handle case where two generated datetimes are equal

* Update changelog

* suppress warning

* update STAC versions in tests

Author: jisantuc

CHANGELOG.md

@@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
+### Fixed
+- Allowed items to have both point in time and time range datetimes [#405](https://github.com/azavea/stac4s/pull/405)
 
 ## [0.6.2] - 2021-07-29
 ### Fixed

modules/core-test/js/src/test/scala/com/azavea/stac4s/JsSerDeSpec.scala

@@ -16,16 +16,14 @@ class JsSerDeSpec extends AnyFunSuite with FunSuiteDiscipline with Checkers with
   checkAll("Codec.ItemCollection", CodecTests[ItemCollection].unserializableCodec)
   checkAll("Codec.StacItem", CodecTests[StacItem].unserializableCodec)
   checkAll("Codec.Geometry", CodecTests[Geometry].unserializableCodec)
-  checkAll("Codec.ItemDatetime", CodecTests[ItemDatetime].unserializableCodec)
-  checkAll("Codec.ItemProperties", CodecTests[ItemProperties].unserializableCodec)
 
   /** Ensure that the datetime field is present but null for time ranges
     *
     * Specification:
     * https://github.com/radiantearth/stac-spec/blob/v1.0.0-rc.4/item-spec/common-metadata.md#date-and-time-range
     */
   test("Encoded time ranges print null datetime") {
-    val tr = ItemDatetime.TimeRange(
+    val tr = TimeRange(
       Instant.parse("2021-01-01T00:00:00Z"),
       Instant.parse("2022-01-01T00:00:00Z")
     )

modules/core-test/jvm/src/test/scala/com/azavea/stac4s/JvmSerDeSpec.scala

@@ -29,16 +29,14 @@ class JvmSerDeSpec extends AnyFunSuite with FunSuiteDiscipline with Checkers wit
   checkAll("Codec.StacLayer", CodecTests[StacLayer].unserializableCodec)
   checkAll("Codec.PeriodDuration", CodecTests[PeriodDuration].unserializableCodec)
   checkAll("Codec.PeriodicExtent", CodecTests[PeriodicExtent].unserializableCodec)
-  checkAll("Codec.ItemDatetime", CodecTests[ItemDatetime].unserializableCodec)
-  checkAll("Codec.ItemProperties", CodecTests[ItemProperties].unserializableCodec)
 
   /** Ensure that the datetime field is present but null for time ranges
     *
     * Specification:
     * https://github.com/radiantearth/stac-spec/blob/v1.0.0-rc.4/item-spec/common-metadata.md#date-and-time-range
     */
   test("Encoded time ranges print null datetime") {
-    val tr = ItemDatetime.TimeRange(
+    val tr = TimeRange(
       Instant.parse("2021-01-01T00:00:00Z"),
       Instant.parse("2022-01-01T00:00:00Z")
     )

modules/core-test/shared/src/test/scala/com/azavea/stac4s/SerDeSpec.scala

@@ -6,6 +6,7 @@ import com.azavea.stac4s.extensions.label._
 import com.azavea.stac4s.extensions.layer._
 import com.azavea.stac4s.meta.ForeignImplicits._
 import com.azavea.stac4s.testing.TestInstances._
+import com.azavea.stac4s.types._
 
 import io.circe.Decoder
 import io.circe.parser._
@@ -33,6 +34,8 @@ class SerDeSpec extends AnyFunSuite with FunSuiteDiscipline with Checkers with M
   checkAll("Codec.StacProviderRole", CodecTests[StacProviderRole].unserializableCodec)
   checkAll("Codec.ThreeDimBbox", CodecTests[ThreeDimBbox].unserializableCodec)
   checkAll("Codec.TwoDimBbox", CodecTests[TwoDimBbox].unserializableCodec)
+  checkAll("Codec.ItemDatetime", CodecTests[ItemDatetime].unserializableCodec)
+  checkAll("Codec.ItemProperties", CodecTests[ItemProperties].unserializableCodec)
 
   // extensions
 
@@ -72,7 +75,6 @@ class SerDeSpec extends AnyFunSuite with FunSuiteDiscipline with Checkers with M
   private def accumulatingDecodeTest[T: Decoder]: Assertion =
     decodeAccumulating[T]("{}").fold(
       errs => {
-        println(s"Errs: $errs")
         errs.size should be > 1
       },
       _ => fail("Decoding succeeded but should not have")

modules/core/js/src/main/scala/com/azavea/stac4s/StacItem.scala

@@ -4,8 +4,8 @@ import com.azavea.stac4s.geometry.Geometry
 
 import cats.Eq
 import io.circe._
-import monocle.macros.{GenLens, GenPrism}
-import monocle.{Lens, Optional}
+import monocle.Lens
+import monocle.macros.GenLens
 
 final case class StacItem(
     id: String,
@@ -28,20 +28,8 @@ final case class StacItem(
 
 object StacItem {
 
-  private val datetimeField: Lens[StacItem, ItemDatetime] = GenLens[StacItem](_.properties.datetime)
-
-  private val toDatetime = GenPrism[ItemDatetime, ItemDatetime.PointInTime]
-
-  private val toTimeRange = GenPrism[ItemDatetime, ItemDatetime.TimeRange]
-
   val propertiesExtension: Lens[StacItem, JsonObject] = GenLens[StacItem](_.properties.extensionFields)
 
-  val datetimePrism: Optional[StacItem, ItemDatetime.PointInTime] =
-    datetimeField.composePrism(toDatetime)
-
-  val timeRangePrism: Optional[StacItem, ItemDatetime.TimeRange] =
-    datetimeField.composePrism(toTimeRange)
-
   implicit val eqStacItem: Eq[StacItem] = Eq.fromUniversalEquals
 
   implicit val encStacItem: Encoder[StacItem] = Encoder

modules/core/jvm/src/main/scala/com/azavea/stac4s/StacItem.scala

@@ -5,8 +5,8 @@ import com.azavea.stac4s.meta._
 import cats.Eq
 import geotrellis.vector.Geometry
 import io.circe._
-import monocle.macros.{GenLens, GenPrism}
-import monocle.{Lens, Optional}
+import monocle.Lens
+import monocle.macros.GenLens
 
 final case class StacItem(
     id: String,
@@ -29,20 +29,8 @@ final case class StacItem(
 
 object StacItem {
 
-  private val datetimeField: Lens[StacItem, ItemDatetime] = GenLens[StacItem](_.properties.datetime)
-
-  private val toDatetime = GenPrism[ItemDatetime, ItemDatetime.PointInTime]
-
-  private val toTimeRange = GenPrism[ItemDatetime, ItemDatetime.TimeRange]
-
   val propertiesExtension: Lens[StacItem, JsonObject] = GenLens[StacItem](_.properties.extensionFields)
 
-  val datetimePrism: Optional[StacItem, ItemDatetime.PointInTime] =
-    datetimeField.composePrism(toDatetime)
-
-  val timeRangePrism: Optional[StacItem, ItemDatetime.TimeRange] =
-    datetimeField.composePrism(toTimeRange)
-
   implicit val eqStacItem: Eq[StacItem] = Eq.fromUniversalEquals
 
   implicit val encStacItem: Encoder[StacItem] = Encoder

modules/core/shared/src/main/scala/com/azavea/stac4s/ItemDatetime.scala

@@ -1,42 +1,30 @@
 package com.azavea.stac4s
 
-import cats.kernel.Eq
 import cats.syntax.apply._
-import cats.syntax.functor._
-import io.circe.syntax._
 import io.circe.{Decoder, Encoder, HCursor}
 
 import java.time.Instant
 
-sealed abstract class ItemDatetime
+case class PointInTime(when: Instant)
 
-object ItemDatetime {
+case class TimeRange(start: Instant, end: Instant)
 
-  case class PointInTime(when: Instant) extends ItemDatetime
-
-  case class TimeRange(start: Instant, end: Instant) extends ItemDatetime
-
-  implicit val eqItemDatetime: Eq[ItemDatetime] = Eq.fromUniversalEquals
-
-  implicit val decPointInTime: Decoder[PointInTime] = { cursor: HCursor =>
-    cursor.get[Instant]("datetime") map { PointInTime }
-  }
+object TimeRange {
 
   implicit val decTimeRange: Decoder[TimeRange] = { cursor: HCursor =>
-    (cursor.get[Instant]("start_datetime"), cursor.get[Instant]("end_datetime")) mapN { TimeRange }
+    (cursor.get[Instant]("start_datetime"), cursor.get[Instant]("end_datetime")) mapN { TimeRange.apply }
   }
 
-  implicit val decItemDatetime: Decoder[ItemDatetime] = decPointInTime.widen or decTimeRange.widen
-
-  implicit val encPointInTime: Encoder[PointInTime] = Encoder.forProduct1("datetime")(_.when)
-
   implicit val encTimeRange: Encoder[TimeRange] =
     Encoder.forProduct3("datetime", "start_datetime", "end_datetime")(range =>
       (Option.empty[Instant], range.start, range.end)
     )
+}
 
-  implicit val encItemDateTime: Encoder[ItemDatetime] = {
-    case pit @ PointInTime(_) => pit.asJson
-    case tr @ TimeRange(_, _) => tr.asJson
+object PointInTime {
+
+  implicit val decPointInTime: Decoder[PointInTime] = { cursor: HCursor =>
+    cursor.get[Instant]("datetime") map { PointInTime.apply }
   }
+  implicit val encPointInTime: Encoder[PointInTime] = Encoder.forProduct1("datetime")(_.when)
 }

modules/core/shared/src/main/scala/com/azavea/stac4s/ItemProperties.scala

@@ -1,5 +1,7 @@
 package com.azavea.stac4s
 
+import com.azavea.stac4s.types._
+
 import cats.data.NonEmptyList
 import cats.kernel.Eq
 import cats.syntax.apply._

modules/core/shared/src/main/scala/com/azavea/stac4s/types.scala

@@ -1,11 +1,46 @@
 package com.azavea.stac4s
 
+import cats.data.Ior
+import cats.kernel.Eq
 import eu.timepit.refined.W
 import eu.timepit.refined.api.Refined
 import eu.timepit.refined.generic._
+import io.circe.syntax._
+import io.circe.{Decoder, DecodingFailure, Encoder, HCursor}
 
 package object types {
 
   type CatalogType    = String Refined Equal[W.`"Catalog"`.T]
   type CollectionType = String Refined Equal[W.`"Collection"`.T]
+
+  type ItemDatetime = Ior[PointInTime, TimeRange]
+
+  implicit val encItemDateTime: Encoder[ItemDatetime] = {
+    case Ior.Left(pit @ PointInTime(_))                       => pit.asJson
+    case Ior.Right(tr @ TimeRange(_, _))                      => tr.asJson
+    case Ior.Both(pit @ PointInTime(_), tr @ TimeRange(_, _)) =>
+      // order is important here! the time range encoder also writes a `null` value to the
+      // datetime field, which overwrites what the point-in-time encoder wants to write.
+      val out = tr.asJson.deepMerge(pit.asJson)
+      out
+  }
+
+  implicit val decItemDateTime: Decoder[ItemDatetime] = { c: HCursor =>
+    (c.as[PointInTime], c.as[TimeRange]) match {
+      case (Right(pit), Right(tr)) => Right(Ior.Both(pit, tr))
+      case (_, Right(tr))          => Right(Ior.Right(tr))
+      case (Right(pit), _)         => Right(Ior.Left(pit))
+      case (Left(err1), Left(err2)) =>
+        (err1, err2) match {
+          case (DecodingFailure(decFailure1, h1), DecodingFailure(decFailure2, h2)) =>
+            Left(DecodingFailure(s"${decFailure1}. ${decFailure2}", h1 ++ h2))
+          // since they're decoding the same cursor, if one of the errors is a ParsingFailure instead
+          // of a decoding failure, they _both_ should be, so we just need the first one
+          case _ =>
+            Left(err1)
+        }
+    }
+  }
+
+  implicit val eqItemDatetime: Eq[ItemDatetime] = Eq.fromUniversalEquals
 }

modules/testing/js/src/main/scala/JsInstances.scala

@@ -8,8 +8,6 @@ import com.azavea.stac4s.{
   Bbox,
   Interval,
   ItemCollection,
-  ItemDatetime,
-  ItemProperties,
   Proprietary,
   SpatialExtent,
   StacAsset,
@@ -62,29 +60,29 @@ trait JsInstances extends GenericInstances {
   private[testing] def stacItemGen: Gen[StacItem] =
     (
       nonEmptyStringGen,
-      Gen.const("1.0.0-rc2"),
+      Gen.const("1.0.0"),
       Gen.const(List.empty[String]),
       Gen.const("Feature"),
       geometryGen,
       TestInstances.twoDimBboxGen,
       nonEmptyListGen(TestInstances.stacLinkGen) map { _.toList },
       TestInstances.assetMapGen,
       Gen.option(nonEmptyStringGen),
-      itemPropertiesGen
+      TestInstances.itemPropertiesGen
     ).mapN(StacItem.apply)
 
   private[testing] def stacItemShortGen: Gen[StacItem] =
     (
       nonEmptyStringGen,
-      Gen.const("1.0.0-rc2"),
+      Gen.const("1.0.0"),
       Gen.const(List.empty[String]),
       Gen.const("Feature"),
       geometryGen,
       TestInstances.twoDimBboxGen,
       Gen.const(Nil),
       Gen.const(Map.empty[String, StacAsset]),
       Gen.option(nonEmptyStringGen),
-      itemPropertiesGen
+      TestInstances.itemPropertiesGen
     ).mapN(StacItem.apply)
 
   private[testing] def itemCollectionGen: Gen[ItemCollection] =
@@ -110,7 +108,7 @@ trait JsInstances extends GenericInstances {
   private[testing] def stacCollectionShortGen: Gen[StacCollection] =
     (
       Arbitrary.arbitrary[CollectionType],
-      Gen.const("1.0.0-rc2"),
+      Gen.const("1.0.0"),
       Gen.const(Nil),
       nonEmptyStringGen,
       nonEmptyStringGen.map(_.some),
@@ -137,30 +135,6 @@ trait JsInstances extends GenericInstances {
     StacLayer.apply
   )
 
-  private def itemDateTimeGen: Gen[ItemDatetime] = Gen.oneOf[ItemDatetime](
-    instantGen map { ItemDatetime.PointInTime },
-    (instantGen, instantGen) mapN {
-      case (i1, i2) if i2.isAfter(i1) => ItemDatetime.TimeRange(i1, i2)
-      case (i1, i2)                   => ItemDatetime.TimeRange(i2, i1)
-    }
-  )
-
-  private def itemPropertiesGen: Gen[ItemProperties] = (
-    itemDateTimeGen,
-    Gen.option(nonEmptyAlphaRefinedStringGen),
-    Gen.option(nonEmptyAlphaRefinedStringGen),
-    Gen.option(instantGen),
-    Gen.option(instantGen),
-    Gen.option(TestInstances.stacLicenseGen),
-    Gen.option(nonEmptyListGen(TestInstances.stacProviderGen)),
-    Gen.option(nonEmptyAlphaRefinedStringGen),
-    Gen.option(nonEmptyListGen(nonEmptyAlphaRefinedStringGen)),
-    Gen.option(nonEmptyAlphaRefinedStringGen),
-    Gen.option(nonEmptyAlphaRefinedStringGen),
-    Gen.option(TestInstances.finiteDoubleGen),
-    TestInstances.itemExtensionFieldsGen
-  ) mapN { ItemProperties.apply }
-
   implicit val arbItem: Arbitrary[StacItem] = Arbitrary { stacItemGen }
 
   val arbItemShort: Arbitrary[StacItem] = Arbitrary { stacItemShortGen }
@@ -181,13 +155,6 @@ trait JsInstances extends GenericInstances {
     stacLayerGen
   }
 
-  implicit val arbItemDatetime: Arbitrary[ItemDatetime] = Arbitrary {
-    itemDateTimeGen
-  }
-
-  implicit val arbItemProperties: Arbitrary[ItemProperties] = Arbitrary {
-    itemPropertiesGen
-  }
 }
 
 object JsInstances extends JsInstances {}