From ef2da4b0e7d23a2ed8a8c8448d7fecb42985d6ad Mon Sep 17 00:00:00 2001 From: Jeff Lewis Date: Tue, 12 Sep 2023 16:48:37 -0600 Subject: [PATCH 1/4] fix handling of external refs targeting member shapes --- .../src/internals/IModelPostProcessor.scala | 1 + .../ExternalMemberRefTransformer.scala | 89 ++++++++++++ modules/openapi/tests/src/MultiFileSpec.scala | 130 ++++++++++++++++++ 3 files changed, 220 insertions(+) create mode 100644 modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala diff --git a/modules/openapi/src/internals/IModelPostProcessor.scala b/modules/openapi/src/internals/IModelPostProcessor.scala index 2ce1795..189fede 100644 --- a/modules/openapi/src/internals/IModelPostProcessor.scala +++ b/modules/openapi/src/internals/IModelPostProcessor.scala @@ -21,6 +21,7 @@ trait IModelPostProcessor extends (IModel => IModel) object IModelPostProcessor { private[this] val defaultTransformers: List[IModelPostProcessor] = List( + ExternalMemberRefTransformer, NewtypeTransformer, FixMissingTargetsTransformer, AllOfTransformer, diff --git a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala new file mode 100644 index 0000000..818824d --- /dev/null +++ b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala @@ -0,0 +1,89 @@ +/* Copyright 2022 Disney Streaming + * + * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://disneystreaming.github.io/TOST-1.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package smithytranslate.openapi.internals +package postprocess + +import org.typelevel.ci._ +import cats.data.NonEmptyChain +import cats.data.Chain +import scala.annotation.tailrec + +/** This transformer is for the specific case where an external reference + * targets the member of a structure from another file. In this case, we need + * to update the target type to be whatever the target type of the referenced + * member is in the target structure. For example, if there is structure A$a + * that references B$b in another file, we would update A$a to instead target + * the same type that B$b targets, rather than targeting B$b directly. + */ +object ExternalMemberRefTransformer extends IModelPostProcessor { + + def apply(model: IModel): IModel = { + val allDefs = model.definitions.map(d => d.id.toString -> d).toMap + + def process(d: Definition): Definition = d match { + case s @ Structure(_, localFields, _, _) => + val newFields = localFields.map { f => + f.tpe.name.segments.toChain.toList match { + case Segment.Arbitrary(ci"components") :: Segment.Arbitrary( + ci"schemas" + ) :: _ => + f.copy(tpe = updatedDefId(f.tpe)) + case _ => f + } + } + s.copy(localFields = newFields) + case other => other + } + + def removeProperties(dId: DefId): Option[DefId] = { + dId.name.segments.toChain.toList match { + case Segment.Arbitrary(ci"components") :: + Segment.Arbitrary(ci"schemas") :: + name :: + Segment.Arbitrary(ci"properties") :: + rest => + Some( + dId.copy(name = + Name( + NonEmptyChain + .of( + Segment.Arbitrary(ci"components"), + Segment.Arbitrary(ci"schemas"), + name + ) + .appendChain(Chain.fromSeq(rest)) + ) + ) + ) + case _ => None + } + } + + @tailrec + def updatedDefId(dId: DefId, isParentProperty: Boolean = false): DefId = + allDefs.get(dId.toString) match { + case Some(Newtype(_, target, _)) => + removeProperties(target) match { + case None => if (isParentProperty) target else dId + case Some(id) => updatedDefId(id, true) + } + case _ => dId + } + IModel(model.definitions.map(process), model.suppressions) + + } + +} diff --git a/modules/openapi/tests/src/MultiFileSpec.scala b/modules/openapi/tests/src/MultiFileSpec.scala index f023a8d..63a3106 100644 --- a/modules/openapi/tests/src/MultiFileSpec.scala +++ b/modules/openapi/tests/src/MultiFileSpec.scala @@ -409,4 +409,134 @@ final class MultiFileSpec extends munit.FunSuite { assertEquals(errors, expectedErrors) assertEquals(output, expectedModel) } + + /* . + * \|-- foo.yaml + * \|-- bar.yaml + */ + test("multiple files - property ref") { + val fooYml = """|openapi: '3.0.' + |info: + | title: test + | version: '1.0' + |paths: {} + |components: + | schemas: + | Object: + | type: object + | properties: + | l: + | $ref: bar.yaml#/components/schemas/Test/properties/s + |""".stripMargin + val barYml = """|openapi: '3.0.' + |info: + | title: test + | version: '1.0' + |paths: {} + |components: + | schemas: + | Test: + | type: object + | properties: + | s: + | type: string + |""".stripMargin + + val expectedFoo = """|namespace foo + | + |structure Object { + | l: String + |} + |""".stripMargin + + val expectedBar = """|namespace bar + | + |structure Test { + | s: String + |} + |""".stripMargin + + val inOne = TestUtils.ConversionTestInput( + NonEmptyList.of("foo.yaml"), + fooYml, + expectedFoo + ) + val inTwo = TestUtils.ConversionTestInput( + NonEmptyList.of("bar.yaml"), + barYml, + expectedBar + ) + TestUtils.runConversionTest(inOne, inTwo) + } + + /* . + * \|-- foo.yaml + * \|-- bar.yaml + */ + test("multiple files - property ref object type") { + val fooYml = """|openapi: '3.0.' + |info: + | title: test + | version: '1.0' + |paths: {} + |components: + | schemas: + | Object: + | type: object + | properties: + | l: + | $ref: bar.yaml#/components/schemas/Test/properties/s + |""".stripMargin + val barYml = """|openapi: '3.0.' + |info: + | title: test + | version: '1.0' + |paths: {} + |components: + | schemas: + | Test: + | type: object + | properties: + | s: + | $ref: '#/components/schemas/Bar' + | Bar: + | type: object + | properties: + | b: + | type: string + |""".stripMargin + + val expectedFoo = """|namespace foo + | + |use bar#Bar + | + |structure Object { + | l: Bar + |} + |""".stripMargin + + val expectedBar = """|namespace bar + | + |structure Bar { + | b: String + |} + | + |structure Test { + | s: Bar + |} + |""".stripMargin + + val inOne = TestUtils.ConversionTestInput( + NonEmptyList.of("foo.yaml"), + fooYml, + expectedFoo + ) + val inTwo = TestUtils.ConversionTestInput( + NonEmptyList.of("bar.yaml"), + barYml, + expectedBar + ) + TestUtils.runConversionTest(inOne, inTwo) + } + } From cab2c05f69bccc336177435d4210ec46a9f26680 Mon Sep 17 00:00:00 2001 From: David Francoeur Date: Wed, 13 Sep 2023 15:03:01 -0400 Subject: [PATCH 2/4] Add some helper to extract from list --- .../ExternalMemberRefTransformer.scala | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala index 818824d..fe3923a 100644 --- a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala +++ b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala @@ -29,6 +29,34 @@ import scala.annotation.tailrec * the same type that B$b targets, rather than targeting B$b directly. */ object ExternalMemberRefTransformer extends IModelPostProcessor { + private abstract class MatchesOne(val segment: Segment) { + def unapply(segments: List[Segment]): Option[List[Segment]] = + segments match { + case `segment` :: rest => Some(rest) + case _ => None + } + } + + private trait MatchesOneNamed { self: MatchesOne => + object named { + def unapply(segments: List[Segment]): Option[(Segment, List[Segment])] = + segments match { + case `segment` :: name :: rest => Some((name, rest)) + case _ => None + } + } + } + + private object Components + extends MatchesOne(Segment.Arbitrary(ci"components")) + + private object Schemas + extends MatchesOne(Segment.Arbitrary(ci"schemas")) + with MatchesOneNamed + + private object Properties + extends MatchesOne(Segment.Arbitrary(ci"properties")) + with MatchesOneNamed def apply(model: IModel): IModel = { val allDefs = model.definitions.map(d => d.id.toString -> d).toMap @@ -37,9 +65,7 @@ object ExternalMemberRefTransformer extends IModelPostProcessor { case s @ Structure(_, localFields, _, _) => val newFields = localFields.map { f => f.tpe.name.segments.toChain.toList match { - case Segment.Arbitrary(ci"components") :: Segment.Arbitrary( - ci"schemas" - ) :: _ => + case Components(Schemas(_)) => f.copy(tpe = updatedDefId(f.tpe)) case _ => f } @@ -50,20 +76,12 @@ object ExternalMemberRefTransformer extends IModelPostProcessor { def removeProperties(dId: DefId): Option[DefId] = { dId.name.segments.toChain.toList match { - case Segment.Arbitrary(ci"components") :: - Segment.Arbitrary(ci"schemas") :: - name :: - Segment.Arbitrary(ci"properties") :: - rest => + case Components(Schemas.named((name, Properties(rest)))) => Some( dId.copy(name = Name( NonEmptyChain - .of( - Segment.Arbitrary(ci"components"), - Segment.Arbitrary(ci"schemas"), - name - ) + .of(Components.segment, Schemas.segment, name) .appendChain(Chain.fromSeq(rest)) ) ) From 21cc5ad977152c4c656db054e4a34728c5be909d Mon Sep 17 00:00:00 2001 From: David Francoeur Date: Wed, 13 Sep 2023 15:05:41 -0400 Subject: [PATCH 3/4] Properties does not need MatchesOneNames --- .../src/internals/postprocess/ExternalMemberRefTransformer.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala index fe3923a..79aedb3 100644 --- a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala +++ b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala @@ -56,7 +56,6 @@ object ExternalMemberRefTransformer extends IModelPostProcessor { private object Properties extends MatchesOne(Segment.Arbitrary(ci"properties")) - with MatchesOneNamed def apply(model: IModel): IModel = { val allDefs = model.definitions.map(d => d.id.toString -> d).toMap From 358169eee3a918715b2ea6b17a3cce4adbdabe75 Mon Sep 17 00:00:00 2001 From: Jeff Lewis Date: Wed, 13 Sep 2023 13:25:08 -0600 Subject: [PATCH 4/4] update docs --- .../postprocess/ExternalMemberRefTransformer.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala index 79aedb3..608eb40 100644 --- a/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala +++ b/modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala @@ -22,11 +22,15 @@ import cats.data.Chain import scala.annotation.tailrec /** This transformer is for the specific case where an external reference - * targets the member of a structure from another file. In this case, we need - * to update the target type to be whatever the target type of the referenced - * member is in the target structure. For example, if there is structure A$a - * that references B$b in another file, we would update A$a to instead target - * the same type that B$b targets, rather than targeting B$b directly. + * targets the primitive member of a structure from another file. In this case, + * we need to update the target type to be whatever the target type of the + * referenced member is in the target structure. For example, if there is + * structure A$a that references B$b in another file, we would update A$a to + * instead target the same type that B$b targets, rather than targeting B$b + * directly. Note that this transformation only takes place if B$b targets a + * primitive. If it targets a structure, or another type, then there would be + * no issue. The reason for this is that Structures get rendered as separate + * types already that can be referenced from another file. */ object ExternalMemberRefTransformer extends IModelPostProcessor { private abstract class MatchesOne(val segment: Segment) {