search

disneystreaming / smithy-translate

Other
54 stars 11 forks source link

NPE in OpenAPI conversion (broken on main) #162

Closed keynmol closed 1 year ago

keynmol commented 1 year ago

This works on 0.3.8, but

  1. Take this file: https://machines-api-spec.fly.dev/swagger/doc.json
  2. Checkout 625d537fa5e474bee6494e9cc6f6de1fcc24e96b
  3. run mill -i cli.run openapi-to-smithy --input doc.json flyway-machines-out/

Exception:

in thread "main" java.lang.NullPointerException
        at java.base/java.util.Objects.requireNonNull(Objects.java:208)
        at software.amazon.smithy.model.traits.ExternalDocumentationTrait$Builder.addUrl(ExternalDocumentationTrait.java:93)
        at smithytranslate.openapi.internals.IModelToSmithy.$anonfun$hintsToTraits$1(IModelToSmithy.scala:310)
        at scala.collection.immutable.List.flatMap(List.scala:293)
        at smithytranslate.openapi.internals.IModelToSmithy.smithytranslate$openapi$internals$IModelToSmithy$$hintsToTraits(IModelToSmithy.scala:295)
        at smithytranslate.openapi.internals.IModelToSmithy$ShapeBuilderOps.addHints(IModelToSmithy.scala:390)
        at smithytranslate.openapi.internals.IModelToSmithy.$anonfun$apply$1(IModelToSmithy.scala:160)
        at scala.collection.immutable.Vector2.map(Vector.scala:2152)
        at scala.collection.immutable.Vector2.map(Vector.scala:441)
        at smithytranslate.openapi.internals.IModelToSmithy.apply(IModelToSmithy.scala:49)
        at smithytranslate.openapi.internals.IModelToSmithy.apply(IModelToSmithy.scala:45)
        at cats.instances.NTupleMonadInstances$$anon$1.$anonfun$map$1(NTupleMonadInstances.scala:18)
        at cats.instances.NTupleMonadInstances.$anonfun$catsStdInstancesForTuple2$1(NTupleMonadInstances.scala:25)
        at cats.instances.NTupleMonadInstances$$anon$1.coflatMap(NTupleMonadInstances.scala:14)
        at cats.instances.NTupleMonadInstances$$anon$1.map(NTupleMonadInstances.scala:18)
        at cats.instances.NTupleMonadInstances$$anon$1.map(NTupleMonadInstances.scala:12)
        at cats.Functor$Ops.map(Functor.scala:246)
        at cats.Functor$Ops.map$(Functor.scala:246)
        at cats.Functor$ToFunctorOps$$anon$4.map(Functor.scala:263)
        at smithytranslate.openapi.OpenApiCompiler$.compile(OpenApiCompiler.scala:153)
        at smithytranslate.openapi.OpenApiCompiler$.parseAndCompile(OpenApiCompiler.scala:142)
        at smithytranslate.cli.runners.openapi.ParseAndCompile$.openapi(ParseAndCompile.scala:43)
        at smithytranslate.cli.runners.OpenApi$.runOpenApi(OpenApi.scala:36)
        at smithytranslate.cli.Main$$anonfun$$lessinit$greater$1.apply(Main.scala:46)
        at smithytranslate.cli.Main$$anonfun$$lessinit$greater$1.apply(Main.scala:44)
        at scala.Function1.$anonfun$andThen$1(Function1.scala:87)
        at scala.Function1.$anonfun$andThen$1(Function1.scala:87)
        at cats.data.Validated.andThen(Validated.scala:727)
        at com.monovore.decline.Parser$Accumulator$Validate.$anonfun$mapValidated$1(Parser.scala:413)
        at scala.Function1.$anonfun$andThen$1(Function1.scala:87)
        at cats.data.Validated.andThen(Validated.scala:727)
        at com.monovore.decline.Result.$anonfun$mapValidated$2(Result.scala:13)
        at cats.instances.Function0Instances$$anon$4.$anonfun$map$1(function.scala:96)
        at com.monovore.decline.Parser.evalResult(Parser.scala:30)
        at com.monovore.decline.Parser.$anonfun$consumeAll$3(Parser.scala:107)
        at scala.util.Either.flatMap(Either.scala:352)
        at com.monovore.decline.Parser.$anonfun$consumeAll$1(Parser.scala:107)
        at scala.Option.map(Option.scala:242)
        at com.monovore.decline.Parser.consumeAll(Parser.scala:106)
        at com.monovore.decline.Parser.apply(Parser.scala:19)
        at com.monovore.decline.Command.parse(opts.scala:20)
        at smithytranslate.cli.CommandApp.main(CommandApp.scala:81)
        at smithytranslate.cli.Main.main(Main.scala)
Baccata commented 1 year ago

Here's the line to fix :

https://github.com/disneystreaming/smithy-translate/blob/0e5c3a09b3a45e5e0fff648697b33f599145a254/modules/openapi/src/internals/OpenApiToIModel.scala#L577-L578

The issue is that the getDescription returns null. The openapi specification states that this value is optional, so we need to give it a dummy description/name to avoid the NPE down the line when it's not provided.

daddykotex commented 1 year ago

I can fix that as well since I'm in the repo, what do you think of External Docs as a dummy?