add support for Mapper initialization #1237

ianbotsf · 2024-02-29T00:16:55Z

Issue #

Description of changes

Initialization

This change adds initialization methods for creating a new DynamoDbMapper instance:

val mapper = DynamoDbMapper(lowLevelDdbClient) {
    // dsl block
    interceptors += MyCoolInterceptor(…)
}

Open to ideas about the method signature here, especially because a low-level client is a required parameter with no default but every other config setting will be optional and/or have sensible defaults.

Interface extraction

This change also takes several public classes from the POC and turns them into interfaces with private implementations and various factory methods. This should unit testing easier for users who don't have/want a mocking library.

KDocs

Added a bunch of documentation comments for public types. Not everything has docs yet but I focused on the types that are mostly stable and not planned to be heavily revised in future tasks.

Miscellany

Some reorganization of a few namespaces
Fixed a typo in DynamodDbAttribute
Changed the name of type param for high-level objects from I to T. I originally chose I to stand for "item" but realized that's the term for the low-level representation. The high-level data are regular Kotlin objects so I went with the more generic T.
Changed various references of attribute "keys" to attribute "names", which mirrors the DynamoDB service documentation and clarifies confusion with item keys

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

… classes and hide implementations; add KDocs for more types

lauzadis · 2024-02-29T16:38:03Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/items/ItemSchema.kt

+     * Represents a schema with a primary key that is a composite of a partition key and a sort key
+     * @param T The type of objects described by this schema
+     * @param PK The type of the partition key property, either [String], [Number], or [ByteArray]
+     * @param PK The type of the partition key property, either [String], [Number], or [ByteArray]


typo: @param SK

lauzadis · 2024-02-29T16:46:09Z

...-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/pipeline/Interceptor.kt

-    public fun readAfterDeserialization(ctx: HResContext<I, HReq, LReq, LRes, HRes>)
-    public fun modifyBeforeCompletion(ctx: HResContext<I, HReq, LReq, LRes, HRes>): HReq
-    public fun readAfterExecution(ctx: HResContext<I, HReq, LReq, LRes, HRes>)
+public interface Interceptor<T, HReq, LReq, LRes, HRes> {


naming suggestion: it might be easier to tell these are response types if they are named LResp and HResp. same for LRespContext and HRespContext

You may be right but I'll defer this to the later task of implementing the request pipeline which will necessarily include finalizing the design of interceptors.

lauzadis · 2024-02-29T16:48:55Z

...namodb-mapper/common/test/aws/sdk/kotlin/hll/dynamodbmapper/items/SimpleItemConverterTest.kt

+
+class SimpleItemConverterTest {
+    @Test
+    fun testSomething() {


github-actions · 2024-02-29T19:04:06Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

github-actions · 2024-02-29T19:10:56Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

aajtodd · 2024-03-01T14:18:09Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/DynamoDbMapper.kt

+         * @param client The low-level DynamoDB client to use for underlying calls to the service
+         * @param config A DSL configuration block
+         */
+        public operator fun invoke(client: DynamoDbClient, config: Config.Builder.() -> Unit = { }): DynamoDbMapper =


Question: Any reason to default this to {} instead of nullable?

Simply wanting to avoid nullability when a unit function accomplishes the same thing.

aajtodd · 2024-03-01T14:18:45Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/DynamoDbMapper.kt

+         * A list of [Interceptor] instances which will be applied to operations as they move through the request
+         * pipeline.
+         */
+        public val interceptors: List<Interceptor<*, *, *, *, *>>


Question: Will the types here become more specific than * or is this the expected type?

I'll take a second look at this when I'm implementing the request pipeline but my gut feeling is that this is the expected type. Each of these type params will vary based on the high-level object type (T) and the operation type (HReq, LReq, LRes, HRes). We could possibly make a common base type for all high-level request/response types but I think at most it'd be a marker interface. The low-level request/response types have no common supertype and we can't enforce one for the high-level object type either.

aajtodd · 2024-03-01T14:19:27Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/DynamoDbMapper.kt

+    }
+}
+
+private data class DynamoDbMapperImpl(


suggestion: Move to it's own file, the interface should exist on it's own.

Good idea, I can do that here and elsewhere. In general, how would we like to separate private implementations? In the same namespace as ./FooImpl.kt? In a nested namespace such as ./internal/FooImpl.kt? Other?

After offline discussion, going with ./internal/FooImpl.kt as our convention.

aajtodd · 2024-03-01T14:21:58Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/Table.kt

+
+    public suspend fun putItem(obj: T)
+
+    public fun scan(): Flow<T>


Question: How will this take into account filtering, exclusive start key, etc?

Sorry for not clarifying more in the PR description but all the operation methods below the // TODO are out of scope for this PR. I rearranged them slightly as part of extracting the interface from the implementation but none of these are final.

The eventual answer to your question will be that each high-level operation will have a canonical method that accepts a high-level request object and returns a high-level response object. Then there will be convenience methods for a handful of APIs which deal in more primitive types. The eventual scan operation will definitely be able to specify filters, exclusive start keys, etc.

aajtodd · 2024-03-01T14:22:35Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/Table.kt


-    internal suspend fun getItem(key: Item): I? {
-        val resp = client.getItem {
+internal abstract class TableImpl<T>(override val mapper: DynamoDbMapper, override val name: String) : Table<T> {


suggestion: Move backing implementations to different files separate from the interface.

aajtodd · 2024-03-01T14:23:08Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/Table.kt

+ * operations after mapping objects to items and vice versa.
+ * @param T The type of objects which will be read from and/or written to this table
+ */
+public interface Table<T> {


Question: Should this be a sealed interface?

It could be sealed but that might complicate users' unit testing. My goal is to maintain maximum flexibility in the public API by leaving as many types as possible/sensible open interfaces which can be mocked without a library.

Right now I don't have a use case which requires knowing all the concrete implementations of Table (e.g., for when matching). If one arises we may revisit this.

aajtodd · 2024-03-01T14:23:39Z

hll/ddb-mapper/dynamodb-mapper/common/src/aws/sdk/kotlin/hll/dynamodbmapper/Table.kt

+    override suspend fun putItem(obj: T) {
+        mapper.client.putItem {
+            tableName = name
+            this.item = schema.converter.toItem(obj)


style: unnecessary qualified this

…PR feedback

github-actions · 2024-03-08T19:29:44Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

sonarqubecloud · 2024-03-08T21:32:29Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-03-08T21:36:22Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

…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>

add support for Mapper initialization; extract interfaces from public…

c7f1d00

… classes and hide implementations; add KDocs for more types

ianbotsf requested a review from a team as a code owner February 29, 2024 00:16

lauzadis approved these changes Feb 29, 2024

View reviewed changes

Merge remote-tracking branch 'origin/feat-ddb-mapper' into mapper-init

32006a1

lint

b9b6ed1

aajtodd approved these changes Mar 1, 2024

View reviewed changes

refactor private implementations to separate files, minor fixes from …

569face

…PR feedback

ianbotsf added 2 commits March 8, 2024 21:28

lint

a8fff00

spleling

6b617df

ianbotsf merged commit 4ddf278 into feat-ddb-mapper Mar 8, 2024

ianbotsf deleted the mapper-init branch March 8, 2024 22:07


		public suspend fun putItem(obj: T)

		public fun scan(): Flow<T>

add support for Mapper initialization #1237

add support for Mapper initialization #1237

Uh oh!

Conversation

ianbotsf commented Feb 29, 2024

Issue #

Description of changes

Initialization

Interface extraction

KDocs

Miscellany

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 29, 2024

Uh oh!

github-actions bot commented Feb 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 8, 2024

Uh oh!

sonarqubecloud bot commented Mar 8, 2024

Quality Gate passed

Uh oh!

github-actions bot commented Mar 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants