Smithy4s refinement types and range combination issue

[lenguyenthanh]

Smithy4s refinement types and range combination issue

Everything works if I have implicit RefinementProvider in refined type companion object

I have an issue related to refinement range combination in Smithy4s. I have a model like this:

@trait(selector: "string")
structure PageFormat {}

@trait(selector: "integer")
structure PageSizeFormat { }

apply fide.spec#PageFormat@refinement(
   targetType: "fide.types.Natural"
)

apply fide.spec#PageSizeFormat@refinement(
   targetType: "fide.types.Natural"
)

@PageFormat
@unwrap
string PageNumber

@PageSizeFormat
@unwrap
integer PageSize

@readonly
@paginated(inputToken: "page", outputToken: "nextPage", pageSize: "pageSize")
@http(method: "GET", uri: "/api/players", code: 200)
operation GetPlayers {

  input := {
    @httpQuery("page")
    page: PageNumber = "1"
    @httpQuery("page_size")
    @range(min: 1, max: 100)
    pageSize: PageSize = 30
  }

  output := {
    @required
    items: Players
    nextPage: PageNumber
  }

  errors: [InternalServerError]
}

It works completely fine as I provide implicit RefinementProvider for all types like this:

opaque type Natural <: Int :| Positive = Int :| Positive

object Natural extends RefinedTypeOps[Int, Positive, Natural]:
  def fromString(value: String): Either[String, Natural] =
    value.toIntOption.toRight(s"$value is not an int") >>= Natural.either

  extension (self: Natural) def succ: Natural = Natural.applyUnsafe(self + 1)

  given RefinementProvider[PageFormat, String, Natural] =
    Refinement.drivenBy(Natural.fromString, _.toString)

  given RefinementProvider.Simple[smithy.api.Range, Natural] =
    RefinementProvider.rangeConstraint(x => x: Int)

  given RefinementProvider[PageSizeFormat, Int, Natural] =
    Refinement.drivenBy(Natural.either, identity)

Please notice RefinementProvider.Simple[smithy.api.Range, Natural] is necessary to provide range constraint that I use in the PageSize field.

Problem arises when I need to move RefinementProvider out of refined type companion object

Now I want to move Natural type into a separate module which is independent from smithy module. The error I get is from here:

package fide.spec

import fide.types.Natural
import smithy4s.Hints
import smithy4s.Schema
import smithy4s.ShapeId
import smithy4s.ShapeTag
import smithy4s.schema.Schema.boolean
import smithy4s.schema.Schema.string
import smithy4s.schema.Schema.struct

final case class GetPlayersInput(page: Natural, pageSize: Natural)

object GetPlayersInput extends ShapeTag.Companion[GetPlayersInput] {
  val id: ShapeId = ShapeId("fide.spec", "GetPlayersInput")

  val hints: Hints = Hints(
    smithy.api.Input(),
  ).lazily

  // constructor using the original order from the spec
  private def make(page: Natural, pageSize: Natural): GetPlayersInput = GetPlayersInput(page, pageSize)

  implicit val schema: Schema[GetPlayersInput] = struct(
    PageNumber.underlyingSchema.field[GetPlayersInput]("page", _.page).addHints(smithy.api.Default(smithy4s.Document.fromString("1")), smithy.api.HttpQuery("page")),
    PageSize.underlyingSchema.validated(smithy.api.Range(min = Some(scala.math.BigDecimal(1.0)), max = Some(scala.math.BigDecimal(100.0)))).field[GetPlayersInput]("pageSize", _.pageSize).addHints(smithy.api.Default(smithy4s.Document.fromDouble(30.0d)), smithy.api.HttpQuery("page_size")),
  )(make).withId(id).addHints(hints) // the problem is this line
}

In this file, We don't have implicit provider for Range to Natural because it's not in Natural companion object any more.

You can find all the code here: https://github.com/lenguyenthanh/fide
And changes that causes the issue: lenguyenthanh/fide#100

Question

Currently I don't see how to include the implicit provider for Range to Natural in the GetPlayersInput companion object. How can I solve this issue?

proposed solutions

Add mechanism to provide implicit RefinementProvider for refinement type in Input object
or
include all implicit RefinementProvider in refinement object some how (in this case PageSize object)

[Baccata]

Not gonna lie : I hadn't envisioned this usecase. Refinements were designed with the goal of incorporating validated types that are not quite provided OOTB by the smithy standard library (things like phone numbers, email addresses, urls, etc), and I didn't really think of combining them with existing validators.

So with your context in mind, the feature was designed so that you could define PageSize as a Scala type, handcraft all the validation necessary there (including the 1 to 100 check), and inject it in Smithy4s generated code by means of a refinement. I can appreciate that intuition would lead users to believe they they should be able to combine features, but from a implementation standpoint it'd be annoyingly complex to keep track of the fact that some arbitrary implicit should only ever be imported when you have a PageSize value that also becomes subject of further validation (like @range(1, 100))

However, we could potentially meet you halfway, and add a @scalaImports that would allow users to manually add imports to files of generated shapes. This would imply that you'd have to use on all shapes that would require it to compile, as opposed to use it once when you define the refinement and for the rendering logic to magically pull it when needed.

If you're interested to contribute this, the PR would roughly involve the following :

1. define the scalaImports trait (search for the smithy4s.meta.smithy file in the smithy4s )
2. define a ScalaImportsTrait POJO here, and add the corresponding line here
3. define a ScalaImports member it in the Hint ADT here
4. define logic to go from ScalaImportsTrait POJO to ScalaImports ADT member here
5. extract the imports and add them to the allImports value here
6. write unit tests / smoke tests.

I'd like @kubukoz' opinion on the matter though.

[lenguyenthanh]

Thanks @Baccata for not dismissing my use case right away (and for your detail answer). tbh, I didn't expect this work (at all), because it's mentioned in the documentation, but I tried it anyway because I though it could be nice way to combine these two features (refinements and validators). And that combination will give users much more flexibility like in my use case.

And yes, I'm interested in contributing this feature. I'll need to read your guideline a bit more and following up with a PR.

[lenguyenthanh]

@Baccata I looked into your suggestion and wanted to clarify if I understand you correctly.

So, our goal for this is implement a scalaImports trait like this:

@trait(selector: "* [trait|trait]")
structure scalaImports {
    @required
    import: Import
}

And use it like:

use smithy4s.meta#scalaImports
apply fide.spec#GetPlayersInput @scalaImports(
   import: "fide.spec.providers.given"
)

@readonly
@paginated(inputToken: "page", outputToken: "nextPage", pageSize: "pageSize")
@http(method: "GET", uri: "/api/players", code: 200)
operation GetPlayers {

  input := {
    @httpQuery("page")
    page: PageNumber = "1"
    @httpQuery("page_size")
    @range(min: 1, max: 100)
    pageSize: PageSize = 30
  }

  output := {
    @required
    items: Players
    nextPage: PageNumber
  }

  errors: [InternalServerError]
}

So that, the generated GetPlayersInput.scala file will include one extra line import fide.spec.providers.given?

If I understand correctly, I can start working on a PR soon.

[Baccata]

That's pretty much it indeed

[kubukoz]

I'm not following... @refinement has a providerImport param, why doesn't that work here?

[lenguyenthanh]

because it's not enough 😅. So, let me reiterate, in my examples I have a refinement types and I want to combine it with @range trait like this:

@trait(selector: "integer")
structure PageSizeFormat { }

// note that RefinementProvider are not in `Natural` scope
apply fide.spec#PageSizeFormat@refinement(
   targetType: "fide.types.Natural"
   providerImports: "fide.spec.providers.given"
)

@PageSizeFormat
@unwrap
integer PageSize

@readonly
@paginated(inputToken: "page", outputToken: "nextPage", pageSize: "pageSize")
@http(method: "GET", uri: "/api/players", code: 200)
operation GetPlayers {


  input := {
    
    // Here is where I combine `range` and refinement type `PageSize`
    
    @httpQuery("page_size")
    @range(min: 1, max: 100)
    pageSize: PageSize = 30
  }
  // ignore stuff for clarity
}

The generated code for GetPlayersInput look like this:

package fide.spec

import fide.types.Natural
import smithy4s.Hints
import smithy4s.Schema
import smithy4s.ShapeId
import smithy4s.ShapeTag
import smithy4s.schema.Schema.boolean
import smithy4s.schema.Schema.string
import smithy4s.schema.Schema.struct

final case class GetPlayersInput(page: Natural, pageSize: Natural)

object GetPlayersInput extends ShapeTag.Companion[GetPlayersInput] {
  val id: ShapeId = ShapeId("fide.spec", "GetPlayersInput")

  val hints: Hints = Hints(
    smithy.api.Input(),
  ).lazily

  // constructor using the original order from the spec
  private def make(page: Natural, pageSize: Natural): GetPlayersInput = GetPlayersInput(page, pageSize)

  implicit val schema: Schema[GetPlayersInput] = struct(
    PageNumber.underlyingSchema.field[GetPlayersInput]("page", _.page).addHints(smithy.api.Default(smithy4s.Document.fromString("1")), smithy.api.HttpQuery("page")),
    PageSize.underlyingSchema.validated(smithy.api.Range(min = Some(scala.math.BigDecimal(1.0)), max = Some(scala.math.BigDecimal(100.0)))).field[GetPlayersInput]("pageSize", _.pageSize).addHints(smithy.api.Default(smithy4s.Document.fromDouble(30.0d)), smithy.api.HttpQuery("page_size")),
  )(make).withId(id).addHints(hints) // the problem is this line
}

And it's failed to compile because of this line: PageSize.underlyingSchema.validated(smithy.api.Range(min = Some(scala.math.BigDecimal(1.0)), max = Some(scala.math.BigDecimal(100.0)))).field[GetPlayersInput]("pageSize", _.pageSize).addHints(smithy.api.Default(smithy4s.Document.fromDouble(30.0d)), smithy.api.HttpQuery("page_size")),

PageSize.underlyingSchema.validated required an implict conversion from Range to Natural like this: given RefinementProvider.Simple[smithy.api.Range, Natural] = ??? which is not available here.

So that why We think that providing a mechanism to add implicit import to make this works.