Skip to content

Commit

Permalink
Merge pull request #241 from disneystreaming/issue-240
Browse files Browse the repository at this point in the history
handle empty response component
  • Loading branch information
lewisjkl authored Apr 12, 2024
2 parents 2894b68 + e176382 commit 0a904c3
Show file tree
Hide file tree
Showing 8 changed files with 278 additions and 94 deletions.
1 change: 1 addition & 0 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ trait OpenApiModule
object tests extends this.ScalaTests with BaseMunitTests {
def ivyDeps = super.ivyDeps() ++ Agg(
buildDeps.smithy.build,
buildDeps.smithy.diff,
buildDeps.scalaJavaCompat
)
}
Expand Down
1 change: 1 addition & 0 deletions buildDeps.sc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ object smithy {
val smithyVersion = "1.41.1"
val model = ivy"software.amazon.smithy:smithy-model:$smithyVersion"
val build = ivy"software.amazon.smithy:smithy-build:$smithyVersion"
val diff = ivy"software.amazon.smithy:smithy-diff:$smithyVersion"
}
object cats {
val mtl = ivy"org.typelevel::cats-mtl:1.4.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ private[compiler] object IModelPostProcessor {
RequirementShiftTransformer,
ContentTypeShiftTransformer,
ReorientDefaultValueTransformer,
DropRequiredWhenDefaultValue
DropRequiredWhenDefaultValue,
EmptyStructureToUnitTransformer
)

private[this] def transform(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/* 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.compiler.internals
package postprocess

import org.typelevel.ci._

// Removes empty structures and changes shapes which target
// them to instead target Unit
private[compiler] object EmptyStructureToUnitTransformer
extends IModelPostProcessor {

def apply(model: IModel): IModel = {
val defs = model.definitions.map(d => d.id -> d).toMap

val structuresToRemove = model.definitions.flatMap {
case s: Structure if isEmptyStructure(defs, s) =>
Some(s.id)
case _ => None
}.toSet
val amendedDefs = model.definitions.flatMap {
case s: Structure if structuresToRemove(s.id) => None
case op: OperationDef =>
val changeInput: OperationDef => OperationDef = o =>
if (o.input.exists(structuresToRemove)) o.copy(input = Some(unit))
else o
val changeOutput: OperationDef => OperationDef = o =>
if (o.output.exists(structuresToRemove)) o.copy(output = Some(unit))
else o
Some(changeInput.andThen(changeOutput)(op))
case other => Some(other)
}
IModel(amendedDefs, model.suppressions)
}

private val unit =
DefId(
Namespace(List("smithy", "api")),
Name(Segment.StandardLib(ci"Unit"))
)

// consider empty if has no fields OR if has one field with Body hint (httpPayload)
private def isEmptyStructure(
defs: Map[DefId, Definition],
d: Definition
): Boolean =
d match {
case s: Structure =>
(s.localFields.isEmpty && s.parents.isEmpty && s.hints.isEmpty) || {
val isHttpPayload =
s.localFields.length == 1 && s.localFields.head.hints
.contains(Hint.Body)
val isHttpPayloadEmpty = s.localFields.headOption
.flatMap(f =>
defs
.get(f.tpe)
)
.exists(isEmptyStructure(defs, _))
isHttpPayload && isHttpPayloadEmpty
}
case _ => false
}
}
3 changes: 1 addition & 2 deletions modules/openapi/src/internals/OpenApiToIModel.scala
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ private[openapi] class OpenApiToIModel[F[_]: Parallel: TellShape: TellError](
}
}
.map { fields =>
if (fields.isEmpty) None
else Structure(defId, fields, Vector.empty, message.hints).some
Structure(defId, fields, Vector.empty, message.hints).some
}
.flatMap(_.traverse(recordDef).map(_.as(defId)))
}
Expand Down
158 changes: 158 additions & 0 deletions modules/openapi/tests/src/ModelWrapper.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/* 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.compiler.openapi

import software.amazon.smithy.model._
import software.amazon.smithy.model.node._
import scala.jdk.CollectionConverters._
import software.amazon.smithy.build.transforms.FilterSuppressions
import software.amazon.smithy.build.TransformContext
import software.amazon.smithy.model.shapes.SmithyIdlModelSerializer
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.model.traits.Trait
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.traits.BoxTrait
import software.amazon.smithy.diff.ModelDiff
import java.util.stream.Collectors

// In order to have nice comparisons from test reports.
class ModelWrapper(val model: Model) {

private final implicit class JavaStreamOps[A](
stream: java.util.stream.Stream[A]
) {
def asList: List[A] = stream.collect(Collectors.toList()).asScala.toList
}

override def equals(obj: Any): Boolean = obj match {
case wrapper: ModelWrapper =>
val one = reorderMetadata(reorderFields(model))
val two = reorderMetadata(reorderFields(wrapper.model))
val diff = ModelDiff
.builder()
.oldModel(one)
.newModel(two)
.compare()
.getDifferences()
val added = diff.addedShapes().asList
val hasChanges =
diff
.changedShapes()
.asList
.exists { changed =>
val addedTraits =
changed.addedTraits().asList
val removedTraits = changed
.removedTraits()
.asList
val changedTraits = changed
.changedTraits()
.asList
.filterNot { pair =>
// compare shapeId and node values to avoid issues with differing java classes
pair.getLeft.toShapeId == pair.getRight.toShapeId && pair.getLeft.toNode == pair.getRight.toNode
}
.filterNot { pair =>
// don't consider synthetic traits
pair.getLeft().toShapeId().getNamespace() == "smithy.synthetic"
}
addedTraits.nonEmpty || removedTraits.nonEmpty || changedTraits.nonEmpty
}
val removed =
diff.removedShapes().asList
added.isEmpty && !hasChanges && removed.isEmpty
case _ => false
}

private def reorderMetadata(model: Model): Model = {
implicit val nodeOrd: Ordering[Node] = (x: Node, y: Node) =>
x.hashCode() - y.hashCode()

implicit val nodeStringOrd: Ordering[StringNode] = {
val ord = Ordering[String]
(x: StringNode, y: StringNode) => ord.compare(x.getValue(), y.getValue())
}
def goNode(n: Node): Node = n match {
case array: ArrayNode =>
val elements = array.getElements().asScala.toList.sorted
Node.arrayNode(elements: _*)
case obj: ObjectNode =>
Node.objectNode(
obj.getMembers().asScala.toSeq.sortBy(_._1).toMap.asJava
)
case other => other
}
def go(metadata: Map[String, Node]): Map[String, Node] = {
val keys = metadata.keySet.toVector.sorted
keys.map { k =>
k -> goNode(metadata(k))
}.toMap
}

val builder = model.toBuilder()
val newMeta = go(model.getMetadata().asScala.toMap)
builder.clearMetadata()
builder.metadata(newMeta.asJava)
builder.build()
}

private val reorderFields: Model => Model = m => {
val structures = m.getStructureShapes().asScala.map { structShape =>
val sortedMembers =
structShape.members().asScala.toList.sortBy(_.getMemberName())
structShape.toBuilder().members(sortedMembers.asJava).build()
}
m.toBuilder().addShapes(structures.asJava).build()
}

private def update(model: Model): Model = {
val filterSuppressions: Model => Model = m =>
new FilterSuppressions().transform(
TransformContext
.builder()
.model(m)
.settings(
ObjectNode.builder().withMember("removeUnused", true).build()
)
.build()
)
(filterSuppressions andThen reorderFields)(model)
}

override def toString() =
SmithyIdlModelSerializer
.builder()
.build()
.serialize(update(model))
.asScala
.map(in => s"${in._1.toString.toUpperCase}:\n\n${in._2}")
.mkString("\n")
}

object ModelWrapper {
def apply(model: Model): ModelWrapper = {
// Remove all box traits because they are applied inconsistently depending on if you
// load from Java model or from unparsed string model
@annotation.nowarn("msg=class BoxTrait in package traits is deprecated")
val noBoxModel = ModelTransformer
.create()
.filterTraits(
model,
((_: Shape, trt: Trait) => trt.toShapeId() != BoxTrait.ID)
)
new ModelWrapper(noBoxModel)
}
}
39 changes: 39 additions & 0 deletions modules/openapi/tests/src/OperationSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,45 @@ final class OperationSpec extends munit.FunSuite {
TestUtils.runConversionTest(openapiString, expectedString)
}

test("operation - response reference 204") {
val openapiString = """|openapi: 3.0.0
|info:
| title: Sample API
|paths:
| /users:
| post:
| operationId: testOperationId
| responses:
| '204':
| $ref: '#/components/responses/NoContent'
|components:
| responses:
| NoContent:
| description: no content
|""".stripMargin

val expectedString = """|namespace foo
|
|service FooService {
| operations: [
| TestOperationId
| ]
|}
|
|@http(
| method: "POST",
| uri: "/users",
| code: 204,
|)
|operation TestOperationId {
| input: Unit,
| output: Unit,
|}
|""".stripMargin

TestUtils.runConversionTest(openapiString, expectedString)
}

test("operation - request reference") {
val openapiString = """|openapi: '3.0.'
|info:
Expand Down
Loading

0 comments on commit 0a904c3

Please sign in to comment.