feat: Generate service clients as modularized Swift packages #757
Conversation
| public extension URLRequest { | ||
| init(sdkRequest: SdkHttpRequest) async throws { | ||
| public extension SdkHttpRequest { | ||
| static func makeURLRequest(from sdkRequest: SdkHttpRequest) async throws -> URLRequest { |
There was a problem hiding this comment.
Changed this to an extension on SdkHttpRequest to prevent polluting URLRequest with it.
| @@ -83,101 +83,82 @@ public func timestampReadingClosure<Reader: SmithyReader>(format: TimestampForma | |||
| } | |||
| } | |||
|
|
|||
| public extension String { | |||
There was a problem hiding this comment.
Made the following changes on ReadingClosures & WritingClosures:
- Removed them as extensions on the types they read/write; instead they are static methods. This prevents them from polluting the namespace of Swift and Foundation types.
| @@ -1,13 +1,16 @@ | |||
| // Code generated by smithy-swift-codegen. DO NOT EDIT! | |||
|
|
|||
| import ClientRuntime | |||
There was a problem hiding this comment.
Many WeatherSDK files changed below due to codegen changes.
You can look to see how service clients are affected, or you can skip past WeatherSDK entirely.
| @@ -65,7 +65,7 @@ class HttpRequestTests: NetworkingTestUtils { | |||
|
|
|||
| let httpBody = ByteStream.data(expectedMockRequestData) | |||
| let mockHttpRequest = SdkHttpRequest(method: .get, endpoint: endpoint, body: httpBody) | |||
| let urlRequest = try await URLRequest(sdkRequest: mockHttpRequest) | |||
| let urlRequest = try await SdkHttpRequest.makeURLRequest(from: mockHttpRequest) | |||
There was a problem hiding this comment.
Calls the new method for converting SdkHttpRequest to URLRequest.
| try writer["FlattenedListArg"].writeList(value.flattenedListArg, memberWritingClosure: String.write(value:to:), memberNodeInfo: "member", isFlattened: true) | ||
| try writer["ListArg"].writeList(value.listArg, memberWritingClosure: String.write(value:to:), memberNodeInfo: "member", isFlattened: false) | ||
| try writer["FlattenedListArg"].writeList(value.flattenedListArg, memberWritingClosure: WritingClosures.writeString(value:to:), memberNodeInfo: "member", isFlattened: true) | ||
| try writer["ListArg"].writeList(value.listArg, memberWritingClosure: WritingClosures.writeString(value:to:), memberNodeInfo: "member", isFlattened: false) |
There was a problem hiding this comment.
Adapted to new writing closures
There was a problem hiding this comment.
Several changes like this in source files and tests below. Will not comment
| into("$sourcesDir/WeatherSDK") | ||
| exclude("Package.swift") | ||
| } | ||
| copy { | ||
| from("$outputDir/WeatherSDKTests") | ||
| from("$outputDir/Tests/WeatherSDKTests") |
There was a problem hiding this comment.
Copies WeatherSDK files to the correct locations for a Swift package.
| writePackageManifest(settings, fileManifest, dependencies, shouldGenerateTestTarget) | ||
| LOGGER.info("Flushing swift writers") | ||
| writers.flushWriters() | ||
| } |
There was a problem hiding this comment.
Writing the package manifest used to be done with a separate writer; IDK why. It is now done with a SwiftWriter from the same context as all other generated files.
| val separator = if (typesToConformMiddlewareTo.count() == 1) "" else ", " | ||
| return typesToConformMiddlewareTo.joinToString(separator) { it.toString() } | ||
| } | ||
|
|
| .map { writer.format("\$N", it) } | ||
| .joinToString(", ") | ||
| writer.openBlock("public struct \$L: \$L {", "}", middleware.typeName, inheritance) { | ||
| writer.write("public let id: \$N = \$S", SwiftTypes.String, middleware.id) |
There was a problem hiding this comment.
Inherited types are now written with a writer so they are properly imported.
| .name("${ClientRuntimeTypes.Middleware.OperationOutput}<$outputType>") | ||
| .addDependency(SwiftDependency.CLIENT_RUNTIME) | ||
| .addReference(ClientRuntimeTypes.Middleware.OperationOutput) | ||
| .addReference(outputType) |
There was a problem hiding this comment.
.addReference() allows a symbol to import another symbol along with itself. This is useful for when you want to have a symbol that has another in its definition, such as collection types.
| writer.write("// swift-tools-version:\$L", ctx.settings.swiftVersion) | ||
| writer.write("") | ||
| writer.write("import PackageDescription") | ||
| writer.write("") |
There was a problem hiding this comment.
Package.swift is an exception among generated files: it writes its own import statements, since it needs to have special content at the top of the file. Going forward, though, the only import in Package.swift should be PackageDescription.
| val target = dependency.expectProperty("target", String::class.java) | ||
| writer.write("name: \"${target}\",") | ||
| writer.write("package: \"${dependency.packageName}\"") | ||
| dependenciesByURL.forEach { dependency -> |
There was a problem hiding this comment.
All the old logic for reading env vars in Package.swift is removed.
For now, Package.swift will get dependencies from local folders... in the future, it will point to published package versions.
| writer.write(".library(name: \"${settings.moduleName}\", targets: [\"${settings.moduleName}\"])") | ||
| } | ||
| val externalDependencies = dependencies.filter { it.getProperty("url", String::class.java).get().isNotEmpty() } | ||
| val dependenciesByURL = externalDependencies.distinctBy { it.expectProperty("url", String::class.java) } |
There was a problem hiding this comment.
Dependencies are provided by the writer delegator based on the symbols that were imported, and are separated into lists for writing to Package.swift.
| ShapeType.BIG_DECIMAL -> { | ||
| writer.addImport(SwiftDependency.BIG.target) | ||
| writer.writeInline("Complex(\$L)", (node.value as Double).toBigDecimal()) | ||
| } |
There was a problem hiding this comment.
BigInteger and BigDecimal are unused codegen and are removed. Those types will use Swift Int and Double, respectively.
| ShapeType.BIG_DECIMAL -> { | ||
| writer.addImport(SwiftDependency.BIG.target) | ||
| writer.writeInline("Complex(\$L)", (node.value as Double).toBigDecimal()) | ||
| } | ||
| else -> throw CodegenException("unexpected shape type $currShape for numberNode") | ||
| } | ||
| } |
There was a problem hiding this comment.
The "types" files below were moved to the "swiftmodules" folder; Git just didn't track them
| var indirectOrNot = "" | ||
| if (indirect) { | ||
| writer.addImport(ClientRuntimeTypes.Core.Indirect) | ||
| indirectOrNot = "@Indirect " |
There was a problem hiding this comment.
ClientRuntimeTypes.Core.Indirect is imported directly when the @Indirect property wrapper is referenced
| "smithy-swift" | ||
| ); | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
These SwiftDependencies were changed from enum cases to static values
| .addDependency(SwiftDependency.BIG) | ||
| .build() | ||
| } | ||
| override fun bigDecimalShape(shape: BigDecimalShape): Symbol = numberShape(shape, "Double", "0.0") |
There was a problem hiding this comment.
BigInteger and BigDecimal changed to Int and Double, as previously mentioned
| @@ -97,14 +97,28 @@ class SwiftWriter(private val fullPackageName: String, swiftImportContainer: Swi | |||
| } | |||
|
|
|||
| fun addImport(symbol: Symbol) { | |||
| symbol.references.forEach { addImport(it.symbol) } | |||
There was a problem hiding this comment.
Imports the referenced symbols for a symbol that is imported
| } ?: run { | ||
| addImport(symbol.namespace, internalSPIName = spiName) | ||
| } | ||
| symbol.dependencies.forEach { addDependency(it) } |
There was a problem hiding this comment.
Adds the symbol's dependencies to the writer so they can be tracked for later writing to the Package.swift
|
|
||
| constructor( | ||
| name: String, | ||
| type: Symbol, | ||
| default: String, | ||
| default: (SwiftWriter) -> String, |
There was a problem hiding this comment.
default is changed to a closure that takes a SwiftWriter so that any symbols that are part of the default value can be properly recorded.
| // writer.write("self.logLevel = logLevel") | ||
| // } | ||
| // } | ||
| // writer.write("") |
There was a problem hiding this comment.
LogHandlerFactoy appears to be unused. If that's the case, I will remove this.
| ctx.delegator.useTestFileWriter(testFilename, ctx.settings.moduleName) { writer -> | ||
| LOGGER.fine("Generating request protocol test cases for ${operation.id}") | ||
| for (import in imports) { | ||
| writer.addImport(import) | ||
| } |
There was a problem hiding this comment.
No need to use this means of importing anymore so it is removed
| is MapShape -> { | ||
| private fun makeReadingClosure(shape: Shape, memberTimestampFormatTrait: TimestampFormatTrait? = null, isSparse: Boolean): String { | ||
| val base = when { | ||
| shape is MapShape -> { |
There was a problem hiding this comment.
Changed this logic a bit so that I can write the new reading closures correctly.
| @@ -54,19 +52,3 @@ val ServiceShape.isRPCBound: Boolean | |||
| AWSProtocol.AWS_JSON_1_0, AWSProtocol.AWS_JSON_1_1, AWSProtocol.AWS_QUERY, AWSProtocol.EC2_QUERY -> true | |||
| else -> false | |||
| } | |||
|
|
|||
| // Adds the imports needed for models of this protocol | |||
There was a problem hiding this comment.
This is no longer needed
| val base = when (shape) { | ||
| is MapShape -> { | ||
| val base = when { | ||
| shape is MapShape -> { |
There was a problem hiding this comment.
This is changed very similar to the ReadingClosure Utils above
| .dependencies(SwiftDependency.CLIENT_RUNTIME).build() | ||
| .addReference(inputType) | ||
| .dependencies(SwiftDependency.CLIENT_RUNTIME) | ||
| .build() |
There was a problem hiding this comment.
Using the Symbol .addReferences() feature again
| .putProperty("spiName", spiName) | ||
| .build() | ||
| } | ||
|
|
There was a problem hiding this comment.
This is removed, the symbol factory method does this for us now when the symbol is made
| "$this" | ||
| } | ||
| fun Symbol.renderSwiftType(writer: SwiftWriter): String { | ||
| return writer.format("\$N", this) |
There was a problem hiding this comment.
The writer has this same logic built-in and will do this for us
| @@ -0,0 +1,16 @@ | |||
| package software.amazon.smithy.swift.codegen.swiftmodules | |||
There was a problem hiding this comment.
Several "types" files below. All of them have SwiftDeclaration added to all symbols, and use the new helper method to construct the symbols.
| @@ -10,14 +10,14 @@ class AuthSchemeResolverGeneratorTests { | |||
| @Test | |||
| fun `test auth scheme resolver generation`() { | |||
| val context = setupTests("auth-scheme-resolver-generator-test.smithy", "com.test#Example") | |||
| val contents = getFileContents(context.manifest, "/Example/AuthSchemeResolver.swift") | |||
| val contents = getFileContents(context.manifest, "Sources/Example/AuthSchemeResolver.swift") | |||
There was a problem hiding this comment.
Many tests follow. In general, file locations are updated, and code is updated with namespaced symbols. No further comments on tests.
Issue #
awslabs/aws-sdk-swift#1222
Description of changes
Sources/{service name}.Tests/{service name}Tests.SwiftWriterautomatically uses written symbols to assemble a list of the needed dependencies for a service: if a referenced symbol requires a dependency, then it is added to Package.swift.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.