Merge pull request #189 from disneystreaming/external-member-refs

fix handling of external refs targeting member shapes

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,

modules/openapi/src/internals/postprocess/ExternalMemberRefTransformer.scala

@@ -0,0 +1,110 @@
+/* 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 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) {
+    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"))
+
+  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 Components(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 Components(Schemas.named((name, Properties(rest)))) =>
+          Some(
+            dId.copy(name =
+              Name(
+                NonEmptyChain
+                  .of(Components.segment, Schemas.segment, 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)
+
+  }
+
+}

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)
+  }
+
 }