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.

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

fix handling of external refs targeting member shapes #189

Merged
merged 5 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions modules/openapi/src/internals/IModelPostProcessor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ trait IModelPostProcessor extends (IModel => IModel)

object IModelPostProcessor {
private[this] val defaultTransformers: List[IModelPostProcessor] = List(
ExternalMemberRefTransformer,
NewtypeTransformer,
FixMissingTargetsTransformer,
AllOfTransformer,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

as I recall from our discussion, this only happens if B$b is a primitive right?
can we state that in the doc or we don't want to because it may happen in other instances and we're not sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I will add it to the doc. It is that way since we match on the NewType on line 95 below and at this point in the postprocessing, all primitives are wrapped in newtypes

* 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)

}

}
130 changes: 130 additions & 0 deletions modules/openapi/tests/src/MultiFileSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

}