aws / aws-sdk-java-v2

The official AWS SDK for Java - Version 2
Apache License 2.0
2.16k stars 835 forks source link

DynamoDB Enhanced to support immutable Kotlin data classes #2096

Open tekener opened 3 years ago

tekener commented 3 years ago

In Kotlin your goal is to use data classes as immutable. With the current mapper this is not possible. You have to add an parameter less constructor to the class (noargs plugin) and all fields has to be changeable (var's instead val's)

Example: data class Example(val name:String) and not data class Example(var name:String)

Describe the Feature

The DynamoDB Enhanced mapper should call the constructor with all attributes read from the dynamodb.

Is your Feature Request related to a problem?

You can't use immutable data classes and you have to use the norags plugin.

Describe alternatives you've considered

Use var's instead of val's and use norags plugin:

plugins{
   id("org.jetbrains.kotlin.plugin.noarg") version "1.4.10"
}
noArg {
  annotation("software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbBean")
}

Your Environment

debora-ito commented 3 years ago

This was also mentioned in https://github.com/aws/aws-sdk-java-v2/issues/1801#issuecomment-643711518.

tekener commented 3 years ago

@debora-ito but the immutable object support as it is implemented in #1801 requires to have an builder, right?!

debora-ito commented 3 years ago

Yes, as per the example in the overview.

tekener commented 3 years ago

Hi just as info for Kotlin developers having the same problem, the builder pattern is also not working out of the box because the data class has hidden destructing methods named "component1-N".

https://kotlinlang.org/docs/reference/multi-declarations.html

andrewparmet commented 3 years ago

It doesn't seem to work even without data:

package com.foo.BeanType

import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbImmutable
import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbPartitionKey
import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbSortKey

@DynamoDbImmutable(builder = BeanType.Builder::class)
class BeanType(
    @get:DynamoDbPartitionKey
    val partitionKey: String,

    @get:DynamoDbSortKey
    val sortKey: String
) {
    class Builder {
        var partitionKey: String? = null
        var sortKey: String? = null

        fun build() = BeanType(partitionKey!!, sortKey!!)
    }
}

ImmutableTableSchema.create(BeanType::class.java) throws:

java.lang.IllegalArgumentException: A method was found on the immutable class builder that does not appear
    to have a matching getter on the immutable class. Use the @DynamoDbIgnore annotation on the method if
    you do not want it to be included in the TableSchema introspection. [Method = "public final
    java.lang.String com.foo.BeanType$Builder.getPartitionKey()"]

I can't figure out why this happens, since the Kotlin compiler should be generating getters with the same name. (Note that nested classes in Kotlin are by default static.)

For kicks I tried lateinit var in the builder as well, although I wouldn't expect a difference, and got the same error.

cmaus-cs commented 3 years ago

The builder's set methods are supposed to return a builder instance. kotlin generated setters that return void.

The following works for me, but is quite tedious to write:

@DynamoDbImmutable(builder = TweetDocument.Builder::class)
class TweetDocument(
    @get:DynamoDbPartitionKey val partitionKey: String,
    @get:DynamoDbSortKey val sortKey: String,
    val tweetId: Long,
    val searchTerm: String,
    val text: String,
    val lang: String,
    val createdAt: String
) {

    companion object {
        @JvmStatic
        fun builder() = Builder()
    }

    class Builder {
        private var partitionKey: String? = null
        fun partitionKey(value: String) = apply { partitionKey = value }

        private var sortKey: String? = null
        fun sortKey(value: String) = apply { sortKey = value }

        private var tweetId: Long? = null
        fun tweetId(value: Long) = apply { tweetId = value }

        private var searchTerm: String? = null
        fun searchTerm(value: String) = apply { searchTerm = value }

        private var text: String? = null
        fun text(value: String) = apply { text = value }

        private var lang: String? = null
        fun lang(value: String) = apply { lang = value }

        private var createdAt: String? = null
        fun createdAt(value: String) = apply { createdAt = value }

        fun build() = TweetDocument(
            partitionKey!!,
            sortKey!!,
            tweetId!!,
            searchTerm!!,
            text!!,
            lang!!,
            createdAt!!
        )
    }
}
oharaandrew314 commented 2 years ago

@cmaus-cs, that's a great workaround. But as you said, it is quite tedious to write, and I noticed that the document class still cannot be a data class, which I think negates the benefit of an immutable document class.

I think I'll keep using the BeanTableSchema, and continue mapping my document classes to immutable data classes for use in my business loigic.

Considering some other annoyances around indices and annotations, I think the v1 mapper continues to be my preferred version; let's hope the new kotlin sdk's mapper is done better, but their complete disregard for nullability in favor of fancy DSLs has me worried.

scprek commented 2 years ago

@cmaus-cs workaround does not work for me on latest enhanced sdk 2.17.204 with kotlinVersion=1.5.31

I get complaints about missing setters even using TableSchema.fromImmutableClass

Caused by: java.lang.IllegalArgumentException: A method was found on the immutable class that does not appear to have a matching setter on the builder class. Use the @DynamoDbIgnore annotation on the method if you do not want it to be included in the TableSchema introspection. [Method = \"public final java.lang.Double com.test.TestModel.getName()\"]

I've tried with both class and data class (get similar error but it's getComponent1() instead of getName()

I followed all the same naming conventions, etc in https://github.com/aws/aws-sdk-java-v2/tree/master/services-custom/dynamodb-enhanced#working-with-immutable-data-classes

oharaandrew314 commented 2 years ago

I made a plugin that allows you to use a data class directly with the DataClassTableSchema; might be worth checking out. https://github.com/oharaandrew314/dynamodb-kotlin-module

scprek commented 2 years ago

@oharaandrew314 well that worked for me easy, thanks for doing this. Are you aware of additional issues besides what you created and if there's any plans in sdk to resolve this? I know there are a few comments in issue requests floating around, but not sure there's been any traction?

oharaandrew314 commented 2 years ago

The only outstanding issue I'm tracking is support for kotlin 1.5. value classes. I did make an attempt once, but it needs another.