Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lenient numeric decoding #1606

Merged
merged 4 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Thank you!

# 0.18.25

* Add A flag to allow for numerics to be decoded from JSON strings (in smithy4s-json).
* Fixes issues in which applications of some Smithy traits would be incorrectly rendered in Scala code (see [#1602](https://github.com/disneystreaming/smithy4s/pull/1602)).
* Fixes an issue in which refinements wouldn't work on custom simple shapes (newtypes) (see [#1595](https://github.com/disneystreaming/smithy4s/pull/1595))
* Fixes a regression from 0.18.4 which incorrectly rendered default values for certain types (see [#1593](https://github.com/disneystreaming/smithy4s/pull/1593))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ See the section about [unions](../../04-codegen/02-unions.md) for a detailed des

By default, optional properties (headers, query parameters, structure fields) that are set to `None` and optional properties that are set to default value will be excluded during encoding process. If you wish to change this so that instead they are included and set to `null` explicitly, you can do so by calling `.withExplicitDefaultsEncoding(true)`.

## Other customisations of JSON codec behaviour

The underlying JSON codecs can be configured with a number of options to cater to niche usecases, via the `.transformJsonCodecs` method, which takes a function that takes in and returns a
`JsonPayloadCodecCompiler`. For instance, by default, the `NaN` and `Infinity` values are not considered valid during parsing `Float` or `Double` values. This can be amended via
`.transformJsonCodecs(_.configureJsoniterCodecCompiler(_.withInfinitySupport(true)))`.

The customisations are bound to evolve as we uncover new niche cases that warrant adding new pieces of opt-in behaviour. The default behaviour is kept rather strict as it helps keep competitive performance and safety.

## Supported traits

Here is the list of traits supported by `SimpleRestJson`
Expand Down
42 changes: 31 additions & 11 deletions modules/http4s/src/smithy4s/http4s/SimpleRestJsonBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,47 +17,67 @@
package smithy4s
package http4s

object SimpleRestJsonBuilder extends SimpleRestJsonBuilder(1024, false, true)
import smithy4s.json.Json
import smithy4s.json.JsonPayloadCodecCompiler

object SimpleRestJsonBuilder
extends SimpleRestJsonBuilder(
new internals.SimpleRestJsonCodecs(
jsonCodecs = Json.payloadCodecs,
explicitDefaultsEncoding = false,
hostPrefixInjection = true
)
)

class SimpleRestJsonBuilder private (
simpleRestJsonCodecs: internals.SimpleRestJsonCodecs
) extends SimpleProtocolBuilder[alloy.SimpleRestJson](
simpleRestJsonCodecs
) {

@deprecated(message = "Use .withXXX methods instead", since = "0.18.25")
def this(
maxArity: Int,
explicitDefaultsEncoding: Boolean,
hostPrefixInjection: Boolean
) =
this(
new internals.SimpleRestJsonCodecs(
maxArity,
Json.payloadCodecs
.withJsoniterCodecCompiler(
Json.jsoniter
.withMaxArity(maxArity)
.withExplicitDefaultsEncoding(explicitDefaultsEncoding)
),
explicitDefaultsEncoding,
hostPrefixInjection
)
)

def withMaxArity(maxArity: Int): SimpleRestJsonBuilder =
new SimpleRestJsonBuilder(
maxArity,
simpleRestJsonCodecs.explicitDefaultsEncoding,
simpleRestJsonCodecs.hostPrefixInjection
simpleRestJsonCodecs.transformJsonCodecs(
_.configureJsoniterCodecCompiler(_.withMaxArity(maxArity))
)
)

def withExplicitDefaultsEncoding(
explicitDefaultsEncoding: Boolean
): SimpleRestJsonBuilder =
new SimpleRestJsonBuilder(
simpleRestJsonCodecs.maxArity,
explicitDefaultsEncoding,
simpleRestJsonCodecs.hostPrefixInjection
simpleRestJsonCodecs.withExplicitDefaultEncoding(explicitDefaultsEncoding)
)

def disableHostPrefixInjection(): SimpleRestJsonBuilder =
new SimpleRestJsonBuilder(
simpleRestJsonCodecs.maxArity,
simpleRestJsonCodecs.explicitDefaultsEncoding,
false
simpleRestJsonCodecs.withHostPrefixInjection(false)
)

/**
* Transforms the underlying JSON codec compiler to change its behaviour.
*/
def transformJsonCodecs(
f: JsonPayloadCodecCompiler => JsonPayloadCodecCompiler
): SimpleRestJsonBuilder =
new SimpleRestJsonBuilder(simpleRestJsonCodecs.transformJsonCodecs(f))
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,43 @@ import smithy4s.http.HttpDiscriminator
import smithy4s.http.Metadata
import smithy4s.http._
import smithy4s.http4s.kernel._
import smithy4s.json.Json
import smithy4s.client._
import smithy4s.codecs.BlobEncoder
import cats.syntax.all._
import org.http4s.Response
import org.http4s.Request
import org.http4s.Uri
import smithy4s.http.HttpMethod
import smithy4s.json.JsonPayloadCodecCompiler

// scalafmt: {maxColumn = 120}
private[http4s] class SimpleRestJsonCodecs(
val maxArity: Int,
val jsonCodecs: JsonPayloadCodecCompiler,
val explicitDefaultsEncoding: Boolean,
val hostPrefixInjection: Boolean
) extends SimpleProtocolCodecs {
private val hintMask =
alloy.SimpleRestJson.protocol.hintMask

private val jsonCodecs = Json.payloadCodecs
.withJsoniterCodecCompiler(
Json.jsoniter
.withHintMask(hintMask)
.withMaxArity(maxArity)
.withExplicitDefaultsEncoding(explicitDefaultsEncoding)
def transformJsonCodecs(f: JsonPayloadCodecCompiler => JsonPayloadCodecCompiler): SimpleRestJsonCodecs =
new SimpleRestJsonCodecs(f(jsonCodecs), explicitDefaultsEncoding, hostPrefixInjection)

def withExplicitDefaultEncoding(newExplicitDefaultsEncoding: Boolean): SimpleRestJsonCodecs =
new SimpleRestJsonCodecs(
jsonCodecs.configureJsoniterCodecCompiler(_.withExplicitDefaultsEncoding(newExplicitDefaultsEncoding)),
newExplicitDefaultsEncoding,
hostPrefixInjection
)

def withHostPrefixInjection(newHostPrefixInjection: Boolean): SimpleRestJsonCodecs =
new SimpleRestJsonCodecs(jsonCodecs, explicitDefaultsEncoding, newHostPrefixInjection)

// val mediaType = HttpMediaType("application/json")
private val payloadEncoders: BlobEncoder.Compiler =
jsonCodecs.encoders
jsonCodecs.configureJsoniterCodecCompiler(_.withHintMask(hintMask)).encoders

private val payloadDecoders =
jsonCodecs.decoders
jsonCodecs.configureJsoniterCodecCompiler(_.withHintMask(hintMask)).decoders

// Adding X-Amzn-Errortype as well to facilitate interop with Amazon-issued code-generators.
private val errorHeaders = List(
Expand Down
6 changes: 6 additions & 0 deletions modules/json/src/smithy4s/json/JsoniterCodecCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ trait JsoniterCodecCompiler extends CachedSchemaCompiler[JsonCodec] {
*/
def withLenientTaggedUnionDecoding: JsoniterCodecCompiler

/**
* Enables lenient decoding of numeric values, where numbers may be carried by JSON strings
* as well as JSON numbers.
*/
def withLenientNumericDecoding: JsoniterCodecCompiler
Comment on lines +81 to +85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: shall we just call it lenient decoding, in case we add more cases of leniency? such as, case-insensitivity (which might not be doable if there are members with names like foo and FOO, but might be possible in many cases)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we'd just mention number handling because it happens to be the only thing for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have more granular options than a "one size fits all" option.


}

object JsoniterCodecCompiler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ private[smithy4s] case class JsoniterCodecCompilerImpl(
infinitySupport: Boolean,
preserveMapOrder: Boolean,
hintMask: Option[HintMask],
lenientTaggedUnionDecoding: Boolean
lenientTaggedUnionDecoding: Boolean,
lenientNumericDecoding: Boolean
) extends CachedSchemaCompiler.Impl[JCodec]
with JsoniterCodecCompiler {

Expand Down Expand Up @@ -59,6 +60,9 @@ private[smithy4s] case class JsoniterCodecCompilerImpl(
def withLenientTaggedUnionDecoding: JsoniterCodecCompiler =
copy(lenientTaggedUnionDecoding = true)

def withLenientNumericDecoding: JsoniterCodecCompiler =
copy(lenientNumericDecoding = true)

def fromSchema[A](schema: Schema[A], cache: Cache): JCodec[A] = {
val visitor = new SchemaVisitorJCodec(
maxArity,
Expand All @@ -67,6 +71,7 @@ private[smithy4s] case class JsoniterCodecCompilerImpl(
flexibleCollectionsSupport,
preserveMapOrder,
lenientTaggedUnionDecoding,
lenientNumericDecoding,
cache
)
val amendedSchema =
Expand All @@ -88,6 +93,7 @@ private[smithy4s] object JsoniterCodecCompilerImpl {
flexibleCollectionsSupport = false,
preserveMapOrder = false,
lenientTaggedUnionDecoding = false,
lenientNumericDecoding = false,
hintMask = Some(JsoniterCodecCompiler.defaultHintMask)
)

Expand Down
Loading