Update to smithy 2.0; add int enum support #352

lewisjkl · 2022-08-02T19:45:21Z

Ported over from #189.

Note: This PR is created against the latest published snapshot of smithy 2.0 (version 1.21.0-rc1). There have been a few improvements/changes since then that I will point out in the comments below.

lewisjkl · 2022-08-02T19:47:40Z

modules/core/src/smithy4s/http/internals/SchemaVisitorMetadataReader.scala

+    if (hints.get[IntEnum].isDefined) {
+      MetaDecode
+        .from(
+          s"Enum[${values.map(_.stringValue).mkString(",")}]"


Not sure if we want to be using the stringValue in cases like this.. probably not, but that's what I had in the other PR. The ordinal would look a bit more confusing, but would be more relevant

lewisjkl · 2022-08-02T19:48:52Z

modules/example/src/smithy4s/example/AgeFormat.scala

-    smithy.api.Trait(Some("integer"), None, None, None),
+    smithy.api.Trait(Some("integer"), None, None),


This change is caused by the difference between 1.22.0 and 1.21.0-rc1. Once we update to smithy 2 (the real one), this will change back.

modules/example/src/smithy4s/example/ArbitraryData.scala

modules/example/src/smithy4s/example/FancyListFormat.scala

lewisjkl · 2022-08-02T19:50:45Z

modules/example/src/smithy4s/example/LowHigh.scala

@@ -12,7 +12,9 @@ sealed abstract class LowHigh(_value: String, _name: String, _ordinal: Int) exte
 object LowHigh extends smithy4s.Enumeration[LowHigh] with smithy4s.ShapeTag.Companion[LowHigh] {
  val id: smithy4s.ShapeId = smithy4s.ShapeId("smithy4s.example", "LowHigh")

-  val hints : smithy4s.Hints = smithy4s.Hints.empty
+  val hints : smithy4s.Hints = smithy4s.Hints(
+    smithy.api.Enum(List(smithy.api.EnumDefinition(smithy.api.NonEmptyString("Low"), Some(smithy.api.EnumConstantBodyName("LOW")), None, None, None), smithy.api.EnumDefinition(smithy.api.NonEmptyString("High"), Some(smithy.api.EnumConstantBodyName("HIGH")), None, None, None))),


These are here since this enum is still using the v1 syntax. Not sure if we want to remove these or not.. probably doesn't impact usage, just readability.

lewisjkl · 2022-08-02T19:51:20Z

modules/example/src/smithy4s/example/StructureWithRefinedTypes.scala

+    Age.schema.optional[StructureWithRefinedTypes]("age", _.age).addHints(smithy.api.Default()),
+    PersonAge.schema.optional[StructureWithRefinedTypes]("personAge", _.personAge).addHints(smithy.api.Default()),


Going to see why these showed up... they shouldn't have

The reason these showed up is that smithy 2.0 does the following when loading v1 models:

Inject an appropriate `@default` trait on structure members that targeted shapes with a default zero value and were not marked with the `@box` trait

Not sure how we want to proceed here.. especially since the default seems to be empty? I guess that indicates a null default? ~~In that case, this could be problematic with required fields since they'll get the empty default trait as well.. which would indicate that a null default is valid, where it isn't.~~ Never mind about that point. required and default are mutually exclusive.

lewisjkl · 2022-08-02T19:52:29Z

sampleSpecs/enums.smithy

+intEnum FaceCard {
+    @enumValue(1)
+    JACK
+    @enumValue(2)
+    QUEEN
+    @enumValue(3)
+    KING
+    @enumValue(4)
+    ACE
+    @enumValue(5)
+    JOKER
+}
+


With the real smithy 2 release we can change these to:

intEnum { JACK = 1 QUEEN = 2 ... etc

lewisjkl · 2022-08-02T21:54:51Z

modules/codegen/src/smithy4s/codegen/CollisionAvoidance.scala

@@ -244,7 +244,8 @@ object CollisionAvoidance {
    "BigDecimal",
    "Map",
    "List",
-    "Set"
+    "Set",
+    "None"


Idk if this is the right solution here.. but it works. There was a collision between scala's Option#None and an enum member in the aws-http module

yeah that's fine

lewisjkl · 2022-08-02T22:39:36Z

Alright so the plugin tests are failing with:

[info] [error] java.lang.UnsupportedOperationException: Member `aws.protocols#ChecksumAlgorithm$SHA256` cannot be added to software.amazon.smithy.model.shapes.StringShape$Builder
[info] [error] 	at software.amazon.smithy.model.shapes.AbstractShapeBuilder.addMember(AbstractShapeBuilder.java:209)
[info] [error] 	at software.amazon.smithy.model.loader.AbstractMutableModelFile.createShapes(AbstractMutableModelFile.java:204)
[info] [error] 	at software.amazon.smithy.model.loader.CompositeModelFile.createShapes(CompositeModelFile.java:133)
[info] [error] 	at software.amazon.smithy.model.loader.ModelAssembler.assemble(ModelAssembler.java:571)
[info] [error] 	at smithy4s.codegen.ModelLoader$.load(ModelLoader.scala:84)
[info] [error] 	at smithy4s.codegen.Codegen$.processSpecs(Codegen.scala:36)
[info] [error] 	at smithy4s.codegen.Smithy4sCodegenPlugin$.$anonfun$cachedSmithyCodegen$5(Smithy4sCodegenPlugin.scala:169)
[info] [error] 	at sbt.util.Tracked$.$anonfun$lastOutput$1(Tracked.scala:73)
[info] [error] 	at scala.Function$.$anonfun$untupled$1(Function.scala:110)
[info] [error] 	at sbt.util.Tracked$.$anonfun$inputChangedW$1(Tracked.scala:219)
[info] [error] 	at smithy4s.codegen.Smithy4sCodegenPlugin$.$anonfun$cachedSmithyCodegen$1(Smithy4sCodegenPlugin.scala:182)
[info] [error] 	at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[info] [error] 	at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:62)
[info] [error] 	at sbt.std.Transform$$anon$4.work(Transform.scala:68)
[info] [error] 	at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
[info] [error] 	at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
[info] [error] 	at sbt.Execute.work(Execute.scala:291)
[info] [error] 	at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
[info] [error] 	at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[info] [error] 	at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
[info] [error] 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[info] [error] 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
[info] [error] 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[info] [error] 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[info] [error] 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[info] [error] 	at java.base/java.lang.Thread.run(Thread.java:833)

I've been trying to figure out why this might be, so far no luck. Any ideas are appreciated!

Baccata · 2022-08-03T07:52:16Z

@lewisjkl, the scripted tests fail because some of them pull an incompatible version of aws-issued smithy libraries :

https://github.com/disneystreaming/smithy4s/blob/main/modules/codegen-plugin/src/sbt-test/codegen-plugin/dependencies-only/build.sbt#L9

update to smithy v2, update enum handling/codegen

197a3db

lewisjkl mentioned this pull request Aug 2, 2022

update to smithy v2, update enum handling/codegen #189

Closed

lewisjkl commented Aug 2, 2022

View reviewed changes

modules/example/src/smithy4s/example/ArbitraryData.scala Show resolved Hide resolved

lewisjkl commented Aug 2, 2022

View reviewed changes

modules/example/src/smithy4s/example/FancyListFormat.scala Show resolved Hide resolved

lewisjkl commented Aug 2, 2022

View reviewed changes

lewisjkl added 3 commits August 2, 2022 13:59

rm extra newline

e98e62f

remove extra import

c26c635

add None to reserved types

746000b

lewisjkl commented Aug 2, 2022

View reviewed changes

update protocol to smithy v2

f990549

update sbt test aws trait versions

9534fa0

Baccata approved these changes Aug 6, 2022

View reviewed changes

Baccata merged commit 8b3ce8f into 0.0.15 Aug 6, 2022

Baccata deleted the 0.0.15-smithy2 branch August 6, 2022 06:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to smithy 2.0; add int enum support #352

Update to smithy 2.0; add int enum support #352

lewisjkl commented Aug 2, 2022

lewisjkl Aug 2, 2022

lewisjkl Aug 2, 2022

lewisjkl Aug 2, 2022

lewisjkl Aug 2, 2022

lewisjkl Aug 2, 2022 •

edited

Loading

lewisjkl Aug 2, 2022

lewisjkl Aug 2, 2022

Baccata Aug 3, 2022

lewisjkl commented Aug 2, 2022

Baccata commented Aug 3, 2022

		smithy.api.Trait(Some("integer"), None, None, None),
		smithy.api.Trait(Some("integer"), None, None),

		Age.schema.optional[StructureWithRefinedTypes]("age", _.age).addHints(smithy.api.Default()),
		PersonAge.schema.optional[StructureWithRefinedTypes]("personAge", _.personAge).addHints(smithy.api.Default()),

Update to smithy 2.0; add int enum support #352

Update to smithy 2.0; add int enum support #352

Conversation

lewisjkl commented Aug 2, 2022

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

lewisjkl Aug 2, 2022

Choose a reason for hiding this comment

Baccata Aug 3, 2022

Choose a reason for hiding this comment

lewisjkl commented Aug 2, 2022

Baccata commented Aug 3, 2022

lewisjkl Aug 2, 2022 •

edited

Loading