-
Notifications
You must be signed in to change notification settings - Fork 55
misc: add DynamoDbAttributeConverter annotation
#1751
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
base: feat-ddb-mapper-main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -4,20 +4,28 @@ | |
| */ | ||
| package aws.sdk.kotlin.hll.dynamodbmapper | ||
|
|
||
| import aws.sdk.kotlin.hll.dynamodbmapper.values.ValueConverter | ||
| import kotlin.reflect.KClass | ||
|
|
||
| /** | ||
| * Specifies the attribute name for a property in a [DynamoDbItem]-annotated class/interface. If this annotation is not | ||
| * included then the attribute name matches the property name. | ||
| */ | ||
| @Target(AnnotationTarget.PROPERTY) | ||
| public annotation class DynamoDbAttribute(val name: String) | ||
|
|
||
| /** | ||
| * Specifies the type of [ValueConverter] to be used when processing this attribute. | ||
| */ | ||
| public annotation class DynamoDbAttributeConverter(val converter: KClass<out ValueConverter<*>>) | ||
|
|
||
|
Comment on lines
+17
to
+21
Contributor
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. Comment: I always seem to forget that annotation classes can only take |
||
| /** | ||
| * Specifies that this class/interface describes an item type in a table. All public properties of this type will be mapped to | ||
| * attributes unless they are explicitly ignored. | ||
| * @param converterName The fully qualified name of the item converter to be used for converting this class/interface. | ||
| * If not set, one will be automatically generated. | ||
| */ | ||
| // FIXME Update to take a KClass<ItemConverter>, which will require splitting codegen modules due to a circular dependency | ||
| // FIXME Update to take a KClass<ItemConverter>? | ||
| @Target(AnnotationTarget.CLASS) | ||
| public annotation class DynamoDbItem(val converterName: String = "") | ||
|
Comment on lines
-20
to
30
Contributor
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. Comment: Yes, we'll definitely still want to do this. I think that'll be far more convenient. |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ internal class HighLevelRenderer( | |
| attributes, | ||
| ) | ||
|
|
||
| val annotation = SchemaRenderer(annotated, renderCtx) | ||
| val annotation = SchemaRenderer(logger, annotated, renderCtx) | ||
| annotation.render() | ||
|
Comment on lines
-45
to
46
Contributor
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: Isn't the logger already available in |
||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -569,4 +569,41 @@ class SchemaGeneratorPluginTest { | |
| """.trimIndent(), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testDynamoDbAttributeConverter() { | ||
| createClassFile("attribute-converter/Employee") | ||
| createClassFile("attribute-converter/HealthcareConverter") | ||
|
|
||
| val result = runner.build() | ||
| assertContains(setOf(TaskOutcome.SUCCESS, TaskOutcome.UP_TO_DATE), result.task(":build")?.outcome) | ||
|
|
||
| val schemaFile = File(testProjectDir, "build/generated/ksp/main/kotlin/org/example/dynamodbmapper/generatedschemas/EmployeeSchema.kt") | ||
| assertTrue(schemaFile.exists()) | ||
|
|
||
| val schemaContents = schemaFile.readText() | ||
|
|
||
| assertContains(schemaContents, "import org.example.OccupationConverter") | ||
| assertContains( | ||
| schemaContents, | ||
| """ AttributeDescriptor( | ||
| "occupation", | ||
| Employee::occupation, | ||
| Employee::occupation::set, | ||
| org.example.OccupationConverter(), | ||
| ),""", | ||
| ) | ||
|
|
||
| // Test cross-package converter | ||
| assertContains(schemaContents, "import a.different.pkg.HealthcareConverter") | ||
| assertContains( | ||
| schemaContents, | ||
| """ AttributeDescriptor( | ||
| "healthcare", | ||
| Employee::healthcare, | ||
| Employee::healthcare::set, | ||
| a.different.pkg.HealthcareConverter(), | ||
| ),""", | ||
|
Comment on lines
+587
to
+606
Contributor
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. Nit: String indentation is a little wonky. |
||
| ) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.example | ||
|
|
||
| import a.different.pkg.HealthcareConverter | ||
| import aws.sdk.kotlin.hll.dynamodbmapper.DynamoDbAttributeConverter | ||
| import aws.sdk.kotlin.hll.dynamodbmapper.DynamoDbItem | ||
| import aws.sdk.kotlin.hll.dynamodbmapper.DynamoDbPartitionKey | ||
| import aws.sdk.kotlin.hll.dynamodbmapper.values.ValueConverter | ||
| import aws.sdk.kotlin.hll.mapping.core.converters.MonoConverter | ||
| import aws.sdk.kotlin.services.dynamodb.model.AttributeValue | ||
|
|
||
| @DynamoDbItem | ||
| data class Employee( | ||
| @DynamoDbPartitionKey | ||
| var id: Int = 1, | ||
| var givenName: String = "Johnny", | ||
| var surname: String = "Appleseed", | ||
|
|
||
| @property:DynamoDbAttributeConverter(OccupationConverter::class) | ||
| var occupation: Occupation = Occupation("Student", 0), | ||
|
|
||
| @property:DynamoDbAttributeConverter(HealthcareConverter::class) | ||
| var healthcare: Healthcare = Healthcare(false), | ||
|
Comment on lines
+23
to
+27
Contributor
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: Having to qualify the target in the annotation line seems kind of ugly and unintuitive. Is this still required if the annotation class is annotated with |
||
| ) | ||
|
|
||
| data class Occupation(val title: String, val salary: Int) | ||
| data class Healthcare(val enrolled: Boolean) | ||
|
|
||
| class OccupationConverter : ValueConverter<Occupation> { | ||
| override val right = MonoConverter<Occupation, AttributeValue> { AttributeValue.S(it.title + "#" + it.salary) } | ||
|
|
||
| override val left = MonoConverter<AttributeValue, Occupation> { | ||
| val content = it.asS() | ||
| val (title, salary) = content.split("#") | ||
| Occupation(title, salary.toInt()) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package a.different.pkg | ||
|
|
||
| import aws.sdk.kotlin.hll.dynamodbmapper.values.ValueConverter | ||
| import aws.sdk.kotlin.hll.mapping.core.converters.MonoConverter | ||
| import aws.sdk.kotlin.services.dynamodb.model.AttributeValue | ||
| import org.example.Healthcare | ||
|
|
||
| class HealthcareConverter : ValueConverter<Healthcare> { | ||
| override val right = MonoConverter<Healthcare, AttributeValue> { AttributeValue.S(it.enrolled.toString()) } | ||
|
|
||
| override val left = MonoConverter<AttributeValue, Healthcare> { | ||
| val content = it.asS() | ||
| val enrolled = (content == "true") | ||
| Healthcare(enrolled) | ||
| } | ||
| } |
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.
Comment: I'd hoped to keep the dependencies clean for the annotations package but we obviously need the
ValueConverter(and soon,ItemConverter) types. I wonder if we should extract a dynamodb-mapper-core-api package which contains some important interfaces but no implementations.