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.

Sign up for GitHub

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

Jump to bottom

allow passing in the MaxArity for the JsonSchemaVisitor #569

Merged
merged 19 commits into from
Nov 16, 2022
Merged

allow passing in the MaxArity for the JsonSchemaVisitor #569

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
848cad8
allow passing in the MaxArity for the JsonSchemaVisitor
yisraelU Nov 3, 2022
1ee1876
revert changes to restJsonBuilder, rather provide capabilities in Cli…
yisraelU Nov 3, 2022
296711d
remove scala 3 wildcards
yisraelU Nov 3, 2022
ae94c4d
adds abstract SimpleRestJsonBuilder layer to type the codec as
yisraelU Nov 4, 2022
11ad06b
remove withJsonCodec method
yisraelU Nov 4, 2022
fe7dd01
remove codecApi variable
yisraelU Nov 4, 2022
046a4f9
formatting
yisraelU Nov 4, 2022
90af9d8
remove withMaxArity method as unnecessary
yisraelU Nov 8, 2022
30c8e1c
add withMaxArity method and remove case class
yisraelU Nov 8, 2022
8b2085c
Merge branch 'series/0.17' into maxArity
yisraelU Nov 9, 2022
2d9ba19
moved maxArity method to class
yisraelU Nov 10, 2022
31a737e
Merge remote-tracking branch 'origin/maxArity' into maxArity
yisraelU Nov 10, 2022
d28ed75
documentation for maxArity metod
yisraelU Nov 11, 2022
5485d41
add example for using max arity
yisraelU Nov 11, 2022
be2c463
Merge remote-tracking branch 'origin/series/0.17' into maxArity
Baccata Nov 12, 2022
0c112da
add deprecation warning to SimpleRestJsonBuilder extension method
yisraelU Nov 14, 2022
bdcdcbc
Merge remote-tracking branch 'origin/maxArity' into maxArity
yisraelU Nov 14, 2022
7237dd4
fmt
yisraelU Nov 14, 2022
f58aacf
remove use of deprecated extension method
yisraelU Nov 14, 2022
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
9 changes: 7 additions & 2 deletions modules/http4s/src/smithy4s/http4s/SimpleRestJsonBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,17 @@ package http4s

import smithy4s.internals.InputOutput

object SimpleRestJsonBuilder
object SimpleRestJsonBuilder extends SimpleRestJsonBuilder(1024) {
def withMaxArity(maxArity: Int): SimpleRestJsonBuilder =
Copy link
Contributor

Choose a reason for hiding this comment

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

the method ought to go on the class, not the object.

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 thought it a bit odd to be on the class ,being that the class has a maxArity parameter in the constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's the "fluent builder" pattern. Each withMethod is basically equivalent to a copy modifying just one field, but has the quality of being much more nicer wrt bincompat.

The point of having the method on the class is that we can add more methods later on if we need to amend the builder to carry more customisation.

new SimpleRestJsonBuilder(i)
}
class SimpleRestJsonBuilder(maxArity: Int)
extends SimpleProtocolBuilder[alloy.SimpleRestJson](
smithy4s.http.json.codecs(
alloy.SimpleRestJson.protocol.hintMask ++ HintMask(
InputOutput,
IntEnum
)
),
maxArity
)
)
3 changes: 1 addition & 2 deletions modules/http4s/src/smithy4s/http4s/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ package object http4s extends Compat.Package {
private[this] val serviceProvider: smithy4s.Service.Provider[Alg, Op]
) {

def simpleRestJson: SimpleRestJsonBuilder.ServiceBuilder[Alg, Op] =
def simpleRestJson: SimpleRestJsonBuilder#ServiceBuilder[Alg, Op] =
SimpleRestJsonBuilder(serviceProvider.service)

}

private[smithy4s] def toHttp4sMethod(method: SmithyMethod): Http4sMethod =
Expand Down
16 changes: 11 additions & 5 deletions modules/json/src/smithy4s/http/json/codecs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ import smithy4s.schema.SchemaVisitor
import smithy4s.schema.CompilationCache

final case class codecs(
hintMask: HintMask = codecs.defaultHintMask
) extends JsonCodecAPI(codecs.schemaVisitorJCodec(_), Some(hintMask))
hintMask: HintMask = codecs.defaultHintMask,
maxArity: Int = codecs.defaultMaxArity
) extends JsonCodecAPI(codecs.schemaVisitorJCodec(_, maxArity), Some(hintMask))

object codecs {

Expand All @@ -43,13 +44,18 @@ object codecs {
// TODO: add tests for `codecs` understanding int enums. Maybe pizza spec
IntEnum
)
val defaultMaxArity: Int = 1024

private[smithy4s] def schemaVisitorJCodec(
cache: CompilationCache[JCodec]
cache: CompilationCache[JCodec],
maxArity: Int = defaultMaxArity
): SchemaVisitor[JCodec] =
new SchemaVisitorJCodec(maxArity = 1024, cache)
new SchemaVisitorJCodec(maxArity, cache)

private[smithy4s] val schemaVisitorJCodec: SchemaVisitor[JCodec] =
new SchemaVisitorJCodec(maxArity = 1024, CompilationCache.nop[JCodec])
new SchemaVisitorJCodec(
maxArity = defaultMaxArity,
CompilationCache.nop[JCodec]
)
Comment on lines +56 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this version currently used for? From a quick look it seems like only in tests. Maybe we can remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im not sure what you mean by version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's add some documentation showing how to increase the maxArity and what that means

good point

Copy link
Contributor

Choose a reason for hiding this comment

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

Im not sure what you mean by version?

Yeah sorry that is super confusing! I meant the "Val" vs the "def" above. The val I'm not sure is used anywhere. Thought we could remove it if that is in fact the case.


}