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

feat: add Schema generator Gradle plugin #1385

Merged
merged 101 commits into from
Aug 22, 2024
Merged

Conversation

lauzadis
Copy link
Member

Issue #

Description of changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lauzadis lauzadis changed the title Add "Schema generator" Gradle plugin feat: add Schema generator Gradle plugin Aug 21, 2024
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@lauzadis lauzadis marked this pull request as ready for review August 21, 2024 23:17
@lauzadis lauzadis requested a review from a team as a code owner August 21, 2024 23:17
Comment on lines 35 to 36
// FIXME Remove hardcoded versioning
dependencies.add("ksp", "aws.sdk.kotlin:dynamodb-mapper-codegen:1.3.2-SNAPSHOT")
Copy link
Member Author

Choose a reason for hiding this comment

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

Looking for ideas / suggestions on this. I can't seem to find a way to grab the sdkVersion from build.gradle.kts and use it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

In our low-level codegen modules, we persist at build time a resource containing a version number and then later read it at runtime when we need the version. (Only works for JVM, obvs.) Would something similar work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is something I was considering and forgot we used this solution already. I think it would work here, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

It did end up working, thanks for the suggestion

Comment on lines 16 to 24
gradlePlugin {
plugins {
create("dynamodb-mapper-schema-generator") {
id = "aws.sdk.kotlin.hll.dynamodbmapper.schema.generator"
implementationClass = "aws.sdk.kotlin.hll.dynamodbmapper.plugins.SchemaGeneratorPlugin"
description = "Plugin used to generate DynamoDbMapper schemas from user classes"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: The Gradle documentation shows several additional metadata we could set about the plugin such as website URL, repository URL, display name, etc. Do we know how those are used? Is it worth setting them on this plugin?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's displayed on Maven Central ("External Resources") in the sidebar. I can add them!

KSP plugin, for example: https://central.sonatype.com/artifact/com.google.devtools.ksp/com.google.devtools.ksp.gradle.plugin

Comment on lines 35 to 36
// FIXME Remove hardcoded versioning
dependencies.add("ksp", "aws.sdk.kotlin:dynamodb-mapper-codegen:1.3.2-SNAPSHOT")
Copy link
Contributor

Choose a reason for hiding this comment

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

In our low-level codegen modules, we persist at build time a resource containing a version number and then later read it at runtime when we need the version. (Only works for JVM, obvs.) Would something similar work here?

Comment on lines +35 to +56
@Test
fun `applies the plugin`() {
val buildFileContent = """
plugins {
id("org.jetbrains.kotlin.jvm") version "2.0.0"
id("aws.sdk.kotlin.hll.dynamodbmapper.schema.generator")
}
configure<aws.sdk.kotlin.hll.dynamodbmapper.plugins.SchemaGeneratorPluginExtension>{ }
""".trimIndent()

buildFile.writeText(buildFileContent)

val result = GradleRunner.create()
.withProjectDir(testProjectDir)
.withArguments("--info", "build")
.withPluginClasspath()
.withGradleVersion("8.5")
.forwardOutput()
.build()

assertContains(setOf(TaskOutcome.SUCCESS, TaskOutcome.UP_TO_DATE), result.task(":build")?.outcome)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Opinion: This test/module may benefit from more dynamic versioning retrieved at runtime to avoid hardcoding things like the Gradle runner version, Kotlin version, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally we would have a parameterized test with multiple versions of Kotlin and Gradle. I think it should be added in follow-on PRs, I will add a TODO here.

import java.io.File
import kotlin.test.assertContains

class SchemaGeneratorPluginTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Oh interesting—I was expecting your plugin tests to be another module sort of like dynamodb-mapper-annotation-processor-test. This looks like it works but I wonder if we miss anything by not having actual test modules as part of the overall Gradle project. Do you know if this is the recommended style of testing Gradle plugins and, if so, what tradeoffs it might have?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is using the Gradle TestKit which seemed recommended for testing plugins. I think having another test module would be another good test to have, I'll look at adding it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added another test package which consumes the plugin. It's basically a copy of dynamodb-mapper-annotation-processor-test without the manual KSP configuration. We can expand this as more features are added like codegen configuration.

Comment on lines 42 to 48
publishing {
publications {
create<MavenPublication>("dynamodb-mapper-codegen") {
from(components["java"])
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Elsewhere where we manually create Maven publications we also add the sources JAR as an artifact. Should we be doing that here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think we should. It doesn't prevent using this package as a dependency but it might help with autocompletion/search in IDEs.

Copy link
Member Author

Choose a reason for hiding this comment

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

This worked for all of the new publications except the Gradle plugin, which seems to already publish the JAR.

Execution failed for task ':hll:dynamodb-mapper:dynamodb-mapper-schema-generator-plugin:publishDynamodb-mapper-schema-generator-pluginPublicationToMavenLocal'.
> Failed to publish publication 'dynamodb-mapper-schema-generator-plugin' to repository 'mavenLocal'
   > Invalid publication 'dynamodb-mapper-schema-generator-plugin': multiple artifacts with the identical extension and classifier ('jar', 'sources').

@@ -80,7 +80,7 @@ if (project.NATIVE_ENABLED) {
// Start by invoking the JVM-only KSP configuration
dependencies.kspJvm(project(":hll:dynamodb-mapper:dynamodb-mapper-codegen"))

// Then we need to move the generated source from jvm to common. Gradle lacks a move task so we roll our own!
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: I see we have a solution already but would this be useful here?

copy {
from("$projectionOutputDir/src")
into("${it.destinationDir}/generated-src")
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good find, that's a copy but we want to move the files in this case

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
6.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@lauzadis lauzadis merged commit 32108ae into feat-ddb-mapper Aug 22, 2024
10 of 11 checks passed
@lauzadis lauzadis deleted the feat-gp-anno branch August 22, 2024 21:01
ianbotsf added a commit that referenced this pull request Oct 29, 2024
…lin (#1451)

* initial poc commit of DynamoDB Mapper (#1232)

* add support for Mapper initialization (#1237)

* implement mapper pipeline (#1266)

* initial implementation of codegen for low-level operations/types (#1357)

* initial implementation of secondary index support (#1375)

* Create new codegen module and refactor annotation processor to use it (#1382)

* feat: add Schema generator Gradle plugin (#1385)

* Fix plugin test package

* add attribute converters for "standard" values (#1381)

* fix: schema generator plugin test module (#1394)

* feat: annotation processor codegen configuration (#1392)

* feat: add `@DynamoDbIgnore` annotation (#1402)

* DDB Mapper filter expressions (runtime components) (#1401)

* feat: basic annotation processing (#1399)

* add DSL overloads, paginators, and better builder integration for DDB Mapper ops codegen (#1409)

* chore: split dynamodb-mapper-codegen into two modules (#1414)

* emit DDB_MAPPER business metric (#1426)

* feat: setup DynamoDbMapper publication (#1419)

* DDB Mapper filter expressions (codegen components) (#1424)

* correct docs

* mark every HLL/DDBM API experimental (#1428)

* fix accidental inclusion of expression attribute members in high-level DynamoDB Mapper requests (#1432)

* Upgrade to latest build plugin version

* fix: various issues found during testing (#1450)

* chore: update Athena changelog notes for 1.3.57 (2024-10-18) release (#1449)

* feat: update AWS API models

* feat: update AWS service endpoints metadata

* chore: release 1.3.60

* chore: bump snapshot version to 1.3.61-SNAPSHOT

* feat: initial release of Developer Preview of DynamoDB Mapper for Kotlin

* Fix Kotlin gradle-plugin version

* fix: ddb mapper tests (#1453)

* Bump build plugin version

---------

Co-authored-by: Matas <lauzmata@amazon.com>
Co-authored-by: aws-sdk-kotlin-ci <aws-kotlin-sdk-automation@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants