-
Notifications
You must be signed in to change notification settings - Fork 49
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
chore: add scaffolding task #1202
Changes from all commits
6059781
99e08e4
40c6c6e
841de9a
60ee9fa
7758025
e170b84
574264a
0a67cac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package aws.sdk.kotlin.gradle.sdk | ||
|
||
import kotlinx.serialization.ExperimentalSerializationApi | ||
import kotlinx.serialization.Serializable | ||
import kotlinx.serialization.json.Json | ||
import kotlinx.serialization.json.decodeFromStream | ||
import java.io.File | ||
|
||
/** | ||
* Manifest containing additional metadata about services. | ||
*/ | ||
@OptIn(ExperimentalSerializationApi::class) | ||
@Serializable | ||
data class PackageManifest( | ||
val packages: List<PackageMetadata>, | ||
) { | ||
|
||
val bySdkId: Map<String, PackageMetadata> = packages.associateBy(PackageMetadata::sdkId) | ||
companion object { | ||
fun fromFile(file: File): PackageManifest = | ||
file.inputStream().use { | ||
Json.decodeFromStream<PackageManifest>(it) | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Validate the package manifest for errors throwing an exception if any exist. | ||
*/ | ||
fun PackageManifest.validate() { | ||
val distinct = mutableMapOf<String, PackageMetadata>() | ||
val errors = mutableListOf<String>() | ||
packages.forEach { | ||
val existing = distinct[it.sdkId] | ||
if (existing != null) { | ||
errors.add("multiple packages with same sdkId `${it.sdkId}`: first: $existing; second: $it") | ||
} | ||
distinct[it.sdkId] = it | ||
} | ||
|
||
check(errors.isEmpty()) { errors.joinToString(separator = "\n") } | ||
} | ||
|
||
/** | ||
* Per/package metadata stored with the repository. | ||
* | ||
* @param sdkId the unique SDK ID from the model this metadata applies to | ||
* @param namespace the package namespace to use as the root namespace when generating code for this package | ||
* @param artifactName the Maven artifact name (i.e. the 'A' in 'GAV' coordinates) | ||
* @param brazilName the internal Brazil package name for this package | ||
*/ | ||
@Serializable | ||
data class PackageMetadata( | ||
public val sdkId: String, | ||
public val namespace: String, | ||
public val artifactName: String, | ||
public val brazilName: String, | ||
) { | ||
companion object { | ||
|
||
/** | ||
* Create a new [PackageMetadata] from inferring values using the given sdkId | ||
*/ | ||
fun from(sdkId: String): PackageMetadata = | ||
PackageMetadata( | ||
sdkId, | ||
packageNamespaceForService(sdkId), | ||
sdkIdToArtifactName(sdkId), | ||
sdkIdToBrazilName(sdkId), | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package aws.sdk.kotlin.gradle.sdk.tasks | ||
|
||
import aws.sdk.kotlin.gradle.sdk.PackageManifest | ||
import aws.sdk.kotlin.gradle.sdk.PackageMetadata | ||
import aws.sdk.kotlin.gradle.sdk.orNull | ||
import aws.sdk.kotlin.gradle.sdk.validate | ||
import kotlinx.serialization.ExperimentalSerializationApi | ||
import kotlinx.serialization.encodeToString | ||
import kotlinx.serialization.json.Json | ||
import org.gradle.api.DefaultTask | ||
import org.gradle.api.file.DirectoryProperty | ||
import org.gradle.api.file.RegularFileProperty | ||
import org.gradle.api.provider.Property | ||
import org.gradle.api.tasks.* | ||
import org.gradle.api.tasks.options.Option | ||
import software.amazon.smithy.aws.traits.ServiceTrait | ||
import software.amazon.smithy.model.Model | ||
import software.amazon.smithy.model.shapes.ServiceShape | ||
import kotlin.streams.toList | ||
|
||
/** | ||
* Task to update the package manifest which is used by the bootstrap process to generate service clients. | ||
* New services are required to be scaffolded | ||
*/ | ||
abstract class UpdatePackageManifest : DefaultTask() { | ||
|
||
@get:Option(option = "model", description = "the path to a single model file to scaffold") | ||
@get:Optional | ||
@get:InputFile | ||
public abstract val modelFile: RegularFileProperty | ||
|
||
@get:Optional | ||
@get:Option(option = "model-dir", description = "the path to a directory of model files to scaffold") | ||
@get:InputDirectory | ||
public abstract val modelDir: DirectoryProperty | ||
|
||
@get:Optional | ||
@get:Option( | ||
option = "discover", | ||
description = "Flag to discover and process only new packages not currently in the manifest. Only applicable when used in conjunction with `model-dir`", | ||
) | ||
@get:Input | ||
public abstract val discover: Property<Boolean> | ||
|
||
@OptIn(ExperimentalSerializationApi::class) | ||
@TaskAction | ||
fun updatePackageManifest() { | ||
check(modelFile.isPresent != modelDir.isPresent) { "Exactly one of `model` or `model-dir` must be set" } | ||
|
||
val manifestFile = project.file("packages.json") | ||
|
||
val manifest = if (manifestFile.exists()) { | ||
val manifest = PackageManifest.fromFile(manifestFile) | ||
manifest.validate() | ||
manifest | ||
} else { | ||
PackageManifest(emptyList()) | ||
} | ||
|
||
val model = Model.assembler() | ||
.discoverModels() | ||
.apply { | ||
val import = if (modelFile.isPresent) modelFile else modelDir | ||
addImport(import.get().asFile.absolutePath) | ||
} | ||
.assemble() | ||
.result | ||
.get() | ||
|
||
val discoveredPackages = model | ||
.shapes(ServiceShape::class.java) | ||
.toList() | ||
.mapNotNull { it.getTrait(ServiceTrait::class.java).orNull()?.sdkId } | ||
.map { PackageMetadata.from(it) } | ||
|
||
val newPackages = validatedPackages(manifest, discoveredPackages) | ||
|
||
if (newPackages.isEmpty()) { | ||
logger.lifecycle("no new packages to scaffold") | ||
return | ||
} | ||
|
||
logger.lifecycle("scaffolding ${newPackages.size} new service packages") | ||
|
||
val updatedPackages = manifest.packages + newPackages | ||
val updatedManifest = manifest.copy(packages = updatedPackages.sortedBy { it.sdkId }) | ||
|
||
val json = Json { prettyPrint = true } | ||
val contents = json.encodeToString(updatedManifest) | ||
manifestFile.writeText(contents) | ||
} | ||
|
||
private fun validatedPackages(manifest: PackageManifest, discovered: List<PackageMetadata>): List<PackageMetadata> = | ||
if (modelDir.isPresent && discover.orNull == true) { | ||
val bySdkId = manifest.packages.associateBy(PackageMetadata::sdkId) | ||
discovered.filter { it.sdkId !in bySdkId } | ||
} else { | ||
discovered.forEach { pkg -> | ||
val existing = manifest.packages.find { it.sdkId == pkg.sdkId } | ||
check(existing == null) { "found existing package in manifest for sdkId `${pkg.sdkId}`: $existing" } | ||
} | ||
discovered | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,22 +8,20 @@ import org.gradle.kotlin.dsl.extra | |
import org.gradle.testfixtures.ProjectBuilder | ||
import org.junit.jupiter.api.io.TempDir | ||
import java.io.File | ||
import kotlin.test.Test | ||
import kotlin.test.assertEquals | ||
import kotlin.test.assertNull | ||
import kotlin.test.* | ||
|
||
class AwsServiceTest { | ||
|
||
val modelContents = """ | ||
${"$"}version: "2.0" | ||
${"$"}version: "2" | ||
namespace gradle.test | ||
Comment on lines
15
to
17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Why did this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've slept and hacked since then and I don't recall. I want to say something about the model discovery and validation triggering something here and so I just fixed it to make smithy happy again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. services still model this as probably not, because the CI which bootstraps services passes, still weird. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Smithy IDL, both of these statements are valid but seem to mean the exact same thing—tooling must support an IDL version of anywhere from 2.0 (inclusive) to 3.0 (exclusive). I'd be curious what broke and if there's maybe a bug somewhere else. |
||
|
||
use aws.api#service | ||
use aws.protocols#awsJson1_0 | ||
|
||
@service(sdkId: "Test Gradle") | ||
@awsJson1_0 | ||
service TestService{ | ||
service TestService { | ||
operations: [], | ||
version: "1-alpha" | ||
} | ||
|
@@ -34,9 +32,23 @@ class AwsServiceTest { | |
val actual: AwsService?, | ||
) | ||
|
||
private val defaultPackageManifest = PackageManifest( | ||
listOf( | ||
PackageMetadata( | ||
"Test Gradle", | ||
// namespace and artifact name intentionally don't match the sdkId derivations to verify we pull from | ||
// the metadata rather than inferring again | ||
"aws.sdk.kotlin.services.testgradle2", | ||
"test-gradle", | ||
"AwsSdkKotlinTestGradle", | ||
), | ||
), | ||
) | ||
|
||
private fun testWith( | ||
tempDir: File, | ||
bootstrap: BootstrapConfig, | ||
manifest: PackageManifest = defaultPackageManifest, | ||
): TestResult { | ||
val project = ProjectBuilder.builder() | ||
.build() | ||
|
@@ -46,7 +58,7 @@ class AwsServiceTest { | |
val model = tempDir.resolve("test-gradle.smithy") | ||
model.writeText(modelContents) | ||
|
||
val lambda = fileToService(project, bootstrap) | ||
val lambda = fileToService(project, bootstrap, manifest) | ||
val actual = lambda(model) | ||
return TestResult(model, actual) | ||
} | ||
|
@@ -69,12 +81,13 @@ class AwsServiceTest { | |
val result = testWith(tempDir, bootstrap) | ||
val expected = AwsService( | ||
"gradle.test#TestService", | ||
"aws.sdk.kotlin.services.testgradle", | ||
"aws.sdk.kotlin.services.testgradle2", | ||
"1.2.3", | ||
Comment on lines
82
to
85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Is this rename significant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's proving that the namespace is taken from the package manifest given vs the inferred namespace from |
||
result.model, | ||
"test-gradle", | ||
"Test Gradle", | ||
"1-alpha", | ||
"test-gradle", | ||
"The AWS SDK for Kotlin client for Test Gradle", | ||
) | ||
assertEquals(expected, result.actual) | ||
|
@@ -98,4 +111,13 @@ class AwsServiceTest { | |
assertNull(result.actual, "expected null for bootstrap with $bootstrap") | ||
} | ||
} | ||
|
||
// FIXME - re-enable after migration | ||
// @Test | ||
// fun testFileToServiceMissingPackageMetadata(@TempDir tempDir: File) { | ||
// val ex = assertFailsWith<IllegalStateException> { | ||
// testWith(tempDir, BootstrapConfig.ALL, PackageManifest(emptyList())) | ||
// } | ||
// assertContains(ex.message!!, "unable to find package metadata for sdkId: Test Gradle") | ||
// } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package aws.sdk.kotlin.gradle.sdk | ||
|
||
import kotlin.test.Test | ||
import kotlin.test.assertContains | ||
import kotlin.test.assertFailsWith | ||
|
||
class PackageManifestTest { | ||
@Test | ||
fun testValidate() { | ||
val manifest = PackageManifest( | ||
listOf( | ||
PackageMetadata("Package 1", "aws.sdk.kotlin.services.package1", "package1", "AwsSdkKotlinPackage1"), | ||
PackageMetadata("Package 2", "aws.sdk.kotlin.services.package2", "package2", "AwsSdkKotlinPackage2"), | ||
), | ||
) | ||
|
||
manifest.validate() | ||
|
||
val badManifest = manifest.copy( | ||
manifest.packages + listOf( | ||
PackageMetadata("Package 2", "aws.sdk.kotlin.services.package2", "package2", "AwsSdkKotlinPackage2"), | ||
), | ||
) | ||
|
||
val ex = assertFailsWith<IllegalStateException> { badManifest.validate() } | ||
|
||
assertContains(ex.message!!, "multiple packages with same sdkId `Package 2`") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Add KDocs. In particular, it's not clear that
artifactName
means "Maven artifact name (i.e., the 'A' in 'GAV')" solely by looking at the code.