oharaandrew314 / dynamodb-kotlin-module

Kotlin Module for the dynamodb-enhanced SDk
Apache License 2.0
23 stars 3 forks source link

Functionality of createTable changed from version 0.4.0 on for DynamoDbEnhancedAsyncClient #16

Closed PoisonedYouth closed 6 months ago

PoisonedYouth commented 6 months ago

With version 0.4.0 the createTableWithIndices functions were removed, because the underlying DefaultDynamoDbTable already is doing the job.

This is working for the DefaultDynamoDbTable but not for the DynamoDbEnhancedAsyncClient as I can see. The tests in CreateTableTest should verify the correct behavior after the removal of the createTableWithIndices - function are not using the async variant.

After replacing the createTableWithIndices - function calls with the recommended createTable - function calls in my application tests are failing because of missing indices.

Below you can find an updated test that I use to reproduce the issue. It is showing me that there are no indices available.

class CreateTableTest {

    private data class Person(
        @DynamoKtPartitionKey val id: Int,
        @DynamoKtSecondaryPartitionKey(indexNames = ["names"]) val name: String,
        @DynamoKtSecondarySortKey(indexNames = ["names"]) val dob: Instant
    )

    private val storage = Storage.InMemory<DynamoTable>()

    private val personTable = DynamoDbEnhancedAsyncClient.builder()
        .dynamoDbClient(
            DynamoDbAsyncClient.builder()
                .httpClient(AwsSdkAsyncClient(FakeDynamoDb(storage)))
                .credentialsProvider { AwsBasicCredentials.create("key", "id") }
                .region(Region.CA_CENTRAL_1)
                .build()
        )
        .build()
        .table("people", DataClassTableSchema(Person::class))

    @Test
    fun createTable() = runBlocking {
        personTable.createTable().await()

        val table = storage["people"].shouldNotBeNull().table

        table.GlobalSecondaryIndexes.shouldContainExactlyInAnyOrder(
            GlobalSecondaryIndexResponse(
                IndexName = "names",
                KeySchema = listOf(
                    KeySchema(AttributeName.of("name"), KeyType.HASH),
                    KeySchema(AttributeName.of("dob"), KeyType.RANGE)
                ),
                Projection = Projection(ProjectionType = ProjectionType.ALL)
            )
        )

        table.LocalSecondaryIndexes.shouldBeNull()
    }
}

Can you please verify if this is correct?

oharaandrew314 commented 6 months ago

If this is the case on the latest AWS SDK, it would be a curious oversight. What I would do is open an issue on the AWS SDK, and reintroduce a helper method in the meantime. I'll perform a test soon.

If you haven't done so already, you can make your own extension method. This is the old one.

private val defaultProjectionBuilder = { _: IndexMetadata ->
    Projection.builder().projectionType(ProjectionType.ALL).build()
}

fun <T: Any> DynamoDbAsyncTable<T>.createTableWithIndices(
    createProjection: (IndexMetadata) -> Projection = defaultProjectionBuilder
): CompletableFuture<Void> {
    val request = tableSchema().tableMetadata().createTableEnhancedRequestWithIndices(createProjection)
    return createTable(request)
}
PoisonedYouth commented 6 months ago

Thanks for the fast response.

Yes I already added my own extension function by just copying the code of version 0.3.0.

This is sufficient as a temporary solution but will be good to not need to keep code additionally in my projects.

oharaandrew314 commented 6 months ago

I've restored DynamoDbAsyncTable.createTableWithIndices in release 0.8 via 330e5f15cd9dd24838ca7d80ce516248a03d4709. I've verified the feature was never added to the DynamoDbAsyncTable, and I am attempting to add the missing feature myself. Just need to figure out how to get the project to build locally 😅 .

PoisonedYouth commented 6 months ago

That was super fast. Thank you very much.