Skip to content

Commit 38c83b1

Browse files
authored
chore: Remove unused code, refactor in protocol tests (#776)
1 parent 785e53a commit 38c83b1

File tree

9 files changed

+32
-114
lines changed

9 files changed

+32
-114
lines changed

smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolTestGenerator.kt

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ import software.amazon.smithy.protocoltests.traits.HttpMessageTestCase
1313
import software.amazon.smithy.protocoltests.traits.HttpRequestTestsTrait
1414
import software.amazon.smithy.protocoltests.traits.HttpResponseTestsTrait
1515
import software.amazon.smithy.swift.codegen.SwiftDependency
16-
import software.amazon.smithy.swift.codegen.integration.middlewares.OperationInputUrlHostMiddleware
17-
import software.amazon.smithy.swift.codegen.integration.middlewares.OperationInputUrlPathMiddleware
18-
import software.amazon.smithy.swift.codegen.integration.middlewares.RequestTestEndpointResolverMiddleware
19-
import software.amazon.smithy.swift.codegen.middleware.MiddlewareStep
20-
import software.amazon.smithy.swift.codegen.middleware.OperationMiddleware
21-
import software.amazon.smithy.swift.codegen.model.hasTrait
2216
import software.amazon.smithy.swift.codegen.model.toUpperCamelCase
2317
import software.amazon.smithy.swift.codegen.testModuleName
2418
import java.util.TreeSet
@@ -33,7 +27,6 @@ class HttpProtocolTestGenerator(
3327
private val responseTestBuilder: HttpProtocolUnitTestResponseGenerator.Builder,
3428
private val errorTestBuilder: HttpProtocolUnitTestErrorGenerator.Builder,
3529
private val httpProtocolCustomizable: HTTPProtocolCustomizable,
36-
private val operationMiddleware: OperationMiddleware,
3730
private val httpBindingResolver: HttpBindingResolver,
3831
// list of test IDs to ignore/skip
3932
private val testsToIgnore: Set<String> = setOf(),
@@ -46,48 +39,16 @@ class HttpProtocolTestGenerator(
4639
*/
4740
fun generateProtocolTests(): Int {
4841
val topDownIndex: TopDownIndex = TopDownIndex.of(ctx.model)
49-
val operationMiddleware = updateRequestTestMiddleware()
5042
var numTests = 0
5143
for (operation in TreeSet(topDownIndex.getContainedOperations(ctx.service).filterNot(::serverOnly))) {
52-
numTests += renderRequestTests(operation, operationMiddleware)
44+
numTests += renderRequestTests(operation)
5345
numTests += renderResponseTests(operation)
5446
numTests += renderErrorTestCases(operation)
5547
}
5648
return numTests
5749
}
5850

59-
private fun updateRequestTestMiddleware(): OperationMiddleware {
60-
val topDownIndex: TopDownIndex = TopDownIndex.of(ctx.model)
61-
val requestTestOperations = TreeSet(
62-
topDownIndex.getContainedOperations(ctx.service)
63-
.filter { it.hasTrait<HttpRequestTestsTrait>() }
64-
.filterNot(::serverOnly)
65-
)
66-
val cloned = operationMiddleware.clone()
67-
68-
for (operation in requestTestOperations) {
69-
cloned.removeMiddleware(operation, MiddlewareStep.INITIALIZESTEP, "OperationInputUrlPathMiddleware")
70-
cloned.removeMiddleware(operation, MiddlewareStep.INITIALIZESTEP, "OperationInputUrlHostMiddleware")
71-
cloned.removeMiddleware(operation, MiddlewareStep.BUILDSTEP, "EndpointResolverMiddleware")
72-
cloned.removeMiddleware(operation, MiddlewareStep.BUILDSTEP, "UserAgentMiddleware")
73-
cloned.removeMiddleware(operation, MiddlewareStep.BUILDSTEP, "AuthSchemeMiddleware")
74-
cloned.removeMiddleware(operation, MiddlewareStep.FINALIZESTEP, "RetryMiddleware")
75-
cloned.removeMiddleware(operation, MiddlewareStep.FINALIZESTEP, "SignerMiddleware")
76-
cloned.removeMiddleware(operation, MiddlewareStep.DESERIALIZESTEP, "DeserializeMiddleware")
77-
cloned.removeMiddleware(operation, MiddlewareStep.DESERIALIZESTEP, "LoggingMiddleware")
78-
79-
cloned.appendMiddleware(operation, RequestTestEndpointResolverMiddleware(ctx.model, ctx.symbolProvider))
80-
cloned.appendMiddleware(operation, OperationInputUrlPathMiddleware(ctx.model, ctx.symbolProvider, "urlPrefix: urlPrefix"))
81-
val hostMiddlewares = cloned.middlewares(operation, MiddlewareStep.INITIALIZESTEP)
82-
.filter { it.name.contains("HostMiddleware") }
83-
if (hostMiddlewares.isEmpty()) {
84-
cloned.appendMiddleware(operation, OperationInputUrlHostMiddleware(ctx.model, ctx.symbolProvider, operation, true))
85-
}
86-
}
87-
return cloned
88-
}
89-
90-
private fun renderRequestTests(operation: OperationShape, operationMiddleware: OperationMiddleware): Int {
51+
private fun renderRequestTests(operation: OperationShape): Int {
9152
val serviceSymbol = ctx.symbolProvider.toSymbol(ctx.service)
9253
val tempTestCases = operation.getTrait(HttpRequestTestsTrait::class.java)
9354
.orElse(null)
@@ -112,7 +73,6 @@ class HttpProtocolTestGenerator(
11273
.serviceName(serviceSymbol.name)
11374
.testCases(requestTestCases)
11475
.httpProtocolCustomizable(httpProtocolCustomizable)
115-
.operationMiddleware(operationMiddleware)
11676
.httpBindingResolver(httpBindingResolver)
11777
.build()
11878
.renderTestClass(testClassName)
@@ -146,7 +106,6 @@ class HttpProtocolTestGenerator(
146106
.serviceName(serviceSymbol.name)
147107
.testCases(responseTestCases)
148108
.httpProtocolCustomizable(httpProtocolCustomizable)
149-
.operationMiddleware(operationMiddleware)
150109
.httpBindingResolver(httpBindingResolver)
151110
.build()
152111
.renderTestClass(testClassName)
@@ -188,7 +147,6 @@ class HttpProtocolTestGenerator(
188147
.serviceName(serviceSymbol.name)
189148
.testCases(testCases)
190149
.httpProtocolCustomizable(httpProtocolCustomizable)
191-
.operationMiddleware(operationMiddleware)
192150
.httpBindingResolver(httpBindingResolver)
193151
.build()
194152
.renderTestClass(testClassName)

smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestErrorGenerator.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ open class HttpProtocolUnitTestErrorGenerator protected constructor(builder: Bui
2222
writer.openBlock("do {", "} catch {") {
2323
renderBuildHttpResponse(test)
2424
writer.write("")
25-
renderInitOperationError(test, operationErrorType)
25+
renderInitOperationError(operationErrorType)
2626
writer.write("")
2727
renderCompareActualAndExpectedErrors(test, it, operationErrorType)
2828
writer.write("")
@@ -34,8 +34,8 @@ open class HttpProtocolUnitTestErrorGenerator protected constructor(builder: Bui
3434
}
3535
}
3636

37-
private fun renderInitOperationError(test: HttpResponseTestCase, operationErrorType: String) {
38-
val operationErrorVariableName = operationErrorType.decapitalize()
37+
private fun renderInitOperationError(operationErrorType: String) {
38+
val operationErrorVariableName = operationErrorType.replaceFirstChar { it.lowercase() }
3939
val responseErrorClosure = ResponseErrorClosureUtils(ctx, writer, operation).render()
4040
writer.addImport(SwiftDependency.SMITHY_READ_WRITE.target)
4141
writer.write(
@@ -50,9 +50,8 @@ open class HttpProtocolUnitTestErrorGenerator protected constructor(builder: Bui
5050
errorShape: Shape,
5151
operationErrorType: String
5252
) {
53-
val operationErrorVariableName = operationErrorType.decapitalize()
53+
val operationErrorVariableName = operationErrorType.replaceFirstChar { it.lowercase() }
5454
val errorType = symbolProvider.toSymbol(errorShape).name
55-
val errorVariableName = errorType.decapitalize()
5655

5756
writer.openBlock("if let actual = \$L as? \$L {", "} else {", operationErrorVariableName, errorType) {
5857
renderExpectedOutput(test, errorShape)

smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestGenerator.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import software.amazon.smithy.model.shapes.OperationShape
1010
import software.amazon.smithy.model.shapes.ServiceShape
1111
import software.amazon.smithy.protocoltests.traits.HttpMessageTestCase
1212
import software.amazon.smithy.swift.codegen.SwiftWriter
13-
import software.amazon.smithy.swift.codegen.middleware.OperationMiddleware
1413

1514
/**
1615
* Abstract base implementation for protocol test generators to extend in order to generate HttpMessageTestCase
@@ -28,7 +27,6 @@ protected constructor(builder: Builder<T>) {
2827
protected val operation: OperationShape = builder.operation!!
2928
protected val writer: SwiftWriter = builder.writer!!
3029
protected val httpProtocolCustomizable = builder.httpProtocolCustomizable!!
31-
protected val operationMiddleware = builder.operationMiddleware!!
3230
protected val httpBindingResolver = builder.httpBindingResolver!!
3331
protected val serviceName: String = builder.serviceName!!
3432
abstract val baseTestClassName: String
@@ -75,7 +73,6 @@ protected constructor(builder: Builder<T>) {
7573
var writer: SwiftWriter? = null
7674
var serviceName: String? = null
7775
var httpProtocolCustomizable: HTTPProtocolCustomizable? = null
78-
var operationMiddleware: OperationMiddleware? = null
7976
var httpBindingResolver: HttpBindingResolver? = null
8077

8178
fun symbolProvider(provider: SymbolProvider): Builder<T> = apply { this.symbolProvider = provider }
@@ -86,7 +83,6 @@ protected constructor(builder: Builder<T>) {
8683
fun writer(writer: SwiftWriter): Builder<T> = apply { this.writer = writer }
8784
fun serviceName(serviceName: String): Builder<T> = apply { this.serviceName = serviceName }
8885
fun httpProtocolCustomizable(httpProtocolCustomizable: HTTPProtocolCustomizable): Builder<T> = apply { this.httpProtocolCustomizable = httpProtocolCustomizable }
89-
fun operationMiddleware(operationMiddleware: OperationMiddleware): Builder<T> = apply { this.operationMiddleware = operationMiddleware }
9086
fun httpBindingResolver(httpBindingResolver: HttpBindingResolver): Builder<T> = apply { this.httpBindingResolver = httpBindingResolver }
9187
abstract fun build(): HttpProtocolUnitTestGenerator<T>
9288
}

smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestRequestGenerator.kt

Lines changed: 24 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
*/
55
package software.amazon.smithy.swift.codegen.integration
66

7-
import software.amazon.smithy.codegen.core.Symbol
8-
import software.amazon.smithy.model.shapes.OperationShape
9-
import software.amazon.smithy.model.shapes.Shape
107
import software.amazon.smithy.model.shapes.StructureShape
118
import software.amazon.smithy.protocoltests.traits.HttpRequestTestCase
129
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait
@@ -17,7 +14,6 @@ import software.amazon.smithy.swift.codegen.integration.serde.readwrite.WireProt
1714
import software.amazon.smithy.swift.codegen.integration.serde.readwrite.requestWireProtocol
1815
import software.amazon.smithy.swift.codegen.model.RecursiveShapeBoxer
1916
import software.amazon.smithy.swift.codegen.model.toLowerCamelCase
20-
import software.amazon.smithy.swift.codegen.model.toUpperCamelCase
2117
import software.amazon.smithy.swift.codegen.swiftmodules.SmithyHTTPAPITypes
2218
import software.amazon.smithy.swift.codegen.swiftmodules.SmithyStreamsTypes
2319

@@ -33,8 +29,8 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
3329
}
3430

3531
private fun renderExpectedBlock(test: HttpRequestTestCase) {
36-
var resolvedHostValue = if (test.resolvedHost.isPresent && test.resolvedHost.get() != "") test.resolvedHost.get() else "example.com"
37-
var hostValue = if (test.host.isPresent && test.host.get() != "") test.host.get() else "example.com"
32+
val resolvedHostValue = if (test.resolvedHost.isPresent && test.resolvedHost.get() != "") test.resolvedHost.get() else "example.com"
33+
val hostValue = if (test.host.isPresent && test.host.get() != "") test.host.get() else "example.com"
3834

3935
// Normalize the URI
4036
val normalizedUri = when {
@@ -46,10 +42,8 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
4642
}
4743
}
4844

49-
writer.write("let urlPrefix = urlPrefixFromHost(host: \$S)", test.host)
50-
writer.write("let hostOnly = hostOnlyFromHost(host: \$S)", test.host)
5145
writer.openBlock("let expected = buildExpectedHttpRequest(")
52-
.write("method: .${test.method.toLowerCase()},")
46+
.write("method: .${test.method.lowercase()},")
5347
.write("path: \$S,", normalizedUri)
5448
.call { renderExpectedHeaders(test) }
5549
.call { renderExpectedQueryParams(test) }
@@ -86,7 +80,7 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
8680

8781
if (!serviceShape.getTrait(EndpointRuleSetTrait::class.java).isPresent) {
8882
val host: String? = test.host.orElse(null)
89-
val url: String = "http://${host ?: "example.com"}"
83+
val url = "http://${host ?: "example.com"}"
9084
writer.write("\nlet config = try await $clientName.${clientName}Configuration(endpointResolver: StaticEndpointResolver(endpoint: try \$N(urlString: \$S)))", SmithyHTTPAPITypes.Endpoint, url)
9185
} else {
9286
writer.write("\nlet config = try await $clientName.${clientName}Configuration()")
@@ -98,21 +92,14 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
9892
}
9993

10094
private fun renderOperationBlock(test: HttpRequestTestCase) {
101-
operation.input.ifPresent { it ->
102-
val clientName = "${ctx.settings.sdkId}Client"
95+
operation.input.ifPresent {
10396
val inputShape = model.expectShape(it)
10497
model = RecursiveShapeBoxer.transform(model)
10598
writer.writeInline("\nlet input = ")
10699
.call {
107100
ShapeValueGenerator(model, symbolProvider).writeShapeValueInline(writer, inputShape, test.params)
108101
}
109102
.write("")
110-
val inputSymbol = symbolProvider.toSymbol(inputShape)
111-
val outputShapeId = operation.output.get()
112-
val outputShape = model.expectShape(outputShapeId)
113-
val outputSymbol = symbolProvider.toSymbol(outputShape)
114-
val outputErrorName = "${operation.toUpperCamelCase()}OutputError"
115-
writer.addImport(SwiftDependency.SMITHY.target)
116103
writer.addImport(SwiftDependency.SMITHY.target)
117104
writer.write(
118105
"""
@@ -122,17 +109,12 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
122109
${'$'}{C|}
123110
}
124111
""".trimIndent(),
125-
Runnable { renderBodyAssert(test, inputSymbol, inputShape) },
112+
Runnable { renderBodyAssert(test) },
126113
)
127114
}
128115
}
129116

130-
private fun resolveHttpMethod(op: OperationShape): String {
131-
val httpTrait = httpBindingResolver.httpTrait(op)
132-
return httpTrait.method.toLowerCase()
133-
}
134-
135-
private fun renderBodyAssert(test: HttpRequestTestCase, inputSymbol: Symbol, inputShape: Shape) {
117+
private fun renderBodyAssert(test: HttpRequestTestCase) {
136118
if (test.body.isPresent && test.body.get().isNotBlank()) {
137119
writer.openBlock(
138120
"try await self.assertEqual(expected, actual, { (expectedHttpBody, actualHttpBody) -> Void in",
@@ -193,39 +175,26 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
193175
}
194176

195177
private fun renderExpectedHeaders(test: HttpRequestTestCase) {
196-
if (test.headers.isNotEmpty()) {
197-
writer.openBlock("headers: [")
198-
.call {
199-
for ((idx, hdr) in test.headers.entries.withIndex()) {
200-
val suffix = if (idx < test.headers.size - 1) "," else ""
201-
writer.write("\$S: \$S$suffix", hdr.key, hdr.value)
202-
}
203-
}
204-
.closeBlock("],")
205-
}
178+
writeHeaders("headers", test.headers)
179+
writeHeaders("forbiddenHeaders", test.forbidHeaders)
180+
writeHeaders("requiredHeaders", test.requireHeaders)
181+
}
206182

207-
if (test.forbidHeaders.isNotEmpty()) {
208-
val forbiddenHeaders = test.forbidHeaders
209-
writer.openBlock("forbiddenHeaders: [")
210-
.call {
211-
forbiddenHeaders.forEachIndexed { idx, value ->
212-
val suffix = if (idx < forbiddenHeaders.size - 1) "," else ""
213-
writer.write("\$S$suffix", value)
214-
}
215-
}
216-
.closeBlock("],")
183+
private fun writeHeaders(name: String, headers: Map<String, String>) {
184+
if (headers.isEmpty()) return
185+
writer.openBlock("\$L: [", "],", name) {
186+
val contents = headers.entries.joinToString(",\n") {
187+
writer.format("\$S: \$S", it.key, it.value)
188+
}
189+
writer.write(contents)
217190
}
191+
}
218192

219-
if (test.requireHeaders.isNotEmpty()) {
220-
val requiredHeaders = test.requireHeaders
221-
writer.openBlock("requiredHeaders: [")
222-
.call {
223-
requiredHeaders.forEachIndexed { idx, value ->
224-
val suffix = if (idx < requiredHeaders.size - 1) "," else ""
225-
writer.write("\$S$suffix", value)
226-
}
227-
}
228-
.closeBlock("],")
193+
private fun writeHeaders(name: String, headers: List<String>) {
194+
if (headers.isEmpty()) return
195+
writer.openBlock("\$L: [", "],", name) {
196+
val contents = headers.joinToString(",\n") { writer.format("\$S", it) }
197+
writer.write(contents)
229198
}
230199
}
231200

smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestResponseGenerator.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder:
4444
val symbol = symbolProvider.toSymbol(it)
4545
renderBuildHttpResponse(test)
4646
writer.write("")
47-
renderActualOutput(test, symbol)
47+
renderActualOutput(symbol)
4848
writer.write("")
4949
renderExpectedOutput(test, it)
5050
writer.write("")
@@ -103,7 +103,7 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder:
103103
}
104104
}
105105

106-
private fun renderActualOutput(test: HttpResponseTestCase, outputStruct: Symbol) {
106+
private fun renderActualOutput(outputStruct: Symbol) {
107107
val responseClosure = ResponseClosureUtils(ctx, writer, operation).render()
108108
writer.write("let actual: \$N = try await \$L(httpResponse)", outputStruct, responseClosure)
109109
}

smithy-swift-codegen/src/test/kotlin/mocks/MockHTTPAWSJson11ProtocolGenerator.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class MockHTTPAWSJson11ProtocolGenerator() : HTTPBindingProtocolGenerator(MockAW
5959
responseTestBuilder,
6060
errorTestBuilder,
6161
customizations,
62-
operationMiddleware,
6362
getProtocolHttpBindingResolver(ctx, defaultContentType),
6463
).generateProtocolTests()
6564
}

smithy-swift-codegen/src/test/kotlin/mocks/MockHTTPEC2QueryProtocolGenerator.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ class MockHTTPEC2QueryProtocolGenerator : HTTPBindingProtocolGenerator(MockEC2Qu
6262
responseTestBuilder,
6363
errorTestBuilder,
6464
customizations,
65-
operationMiddleware,
6665
getProtocolHttpBindingResolver(ctx, defaultContentType),
6766
).generateProtocolTests()
6867
}

smithy-swift-codegen/src/test/kotlin/mocks/MockHTTPRestJsonProtocolGenerator.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ class MockHTTPRestJsonProtocolGenerator : HTTPBindingProtocolGenerator(MockRestJ
3636
responseTestBuilder,
3737
errorTestBuilder,
3838
customizations,
39-
operationMiddleware,
4039
getProtocolHttpBindingResolver(ctx, defaultContentType),
4140
).generateProtocolTests()
4241
}

smithy-swift-codegen/src/test/kotlin/mocks/MockHTTPRestXMLProtocolGenerator.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ class MockHTTPRestXMLProtocolGenerator : HTTPBindingProtocolGenerator(MockRestXM
3737
responseTestBuilder,
3838
errorTestBuilder,
3939
customizations,
40-
operationMiddleware,
4140
getProtocolHttpBindingResolver(ctx, defaultContentType),
4241
).generateProtocolTests()
4342
}

0 commit comments

Comments
 (0)