FIX: KotlinDataClassComponent ignored @Column(flatten=false) annota…

…tion on data class properties Fixes #16
ydb-platform · Feb 1, 2024 · b2cd742 · b2cd742
1 parent d6dd53e
commit b2cd742
Show file tree

Hide file tree

Showing 10 changed files with 81 additions and 59 deletions.
diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java
@@ -1,9 +1,10 @@
 package tech.ydb.yoj.databind.schema.reflect;
 
+import com.google.common.base.Preconditions;
 import com.google.common.reflect.TypeToken;
 import kotlin.jvm.JvmClassMappingKt;
-import kotlin.reflect.KCallable;
 import kotlin.reflect.KClass;
+import kotlin.reflect.KProperty1;
 import kotlin.reflect.jvm.KCallablesJvm;
 import kotlin.reflect.jvm.ReflectJvmMapping;
 import tech.ydb.yoj.databind.FieldValueType;
@@ -17,7 +18,7 @@
  * Represents a Kotlin data class component for the purposes of YOJ data-binding.
  */
 public final class KotlinDataClassComponent implements ReflectField {
-    private final KCallable<?> callable;
+    private final KProperty1.Getter<?, ?> getter;
 
     private final String name;
     private final Type genericType;
@@ -27,24 +28,29 @@ public final class KotlinDataClassComponent implements ReflectField {
 
     private final ReflectType<?> reflectType;
 
-    public KotlinDataClassComponent(Reflector reflector, String name, KCallable<?> callable) {
-        this.callable = callable;
-        KCallablesJvm.setAccessible(this.callable, true);
+    public KotlinDataClassComponent(Reflector reflector, String name,
+                                    KProperty1<?, ?> property) {
+        this.getter = property.getGetter();
+        KCallablesJvm.setAccessible(this.getter, true);
 
         this.name = name;
 
-        var kReturnType = callable.getReturnType();
-        this.genericType = ReflectJvmMapping.getJavaType(kReturnType);
+        var kPropertyType = property.getReturnType();
+        this.genericType = ReflectJvmMapping.getJavaType(kPropertyType);
 
-        var kClassifier = kReturnType.getClassifier();
+        var kClassifier = kPropertyType.getClassifier();
         if (kClassifier instanceof KClass<?> kClass) {
             this.type = JvmClassMappingKt.getJavaClass(kClass);
         } else {
             // fallback to Guava's TypeToken if kotlin-reflect returns unpredictable results ;-)
             this.type = TypeToken.of(genericType).getRawType();
         }
 
-        this.column = type.getAnnotation(Column.class);
+        var field = ReflectJvmMapping.getJavaField(property);
+        Preconditions.checkArgument(field != null, "Could not get Java field for property '%s' of '%s'",
+                property.getName(), kPropertyType);
+
+        this.column = field.getAnnotation(Column.class);
         this.valueType = FieldValueType.forJavaType(genericType, column);
         this.reflectType = reflector.reflectFieldType(genericType, valueType);
     }
@@ -53,7 +59,7 @@ public KotlinDataClassComponent(Reflector reflector, String name, KCallable<?> c
     @Override
     public Object getValue(Object containingObject) {
         try {
-            return callable.call(containingObject);
+            return getter.call(containingObject);
         } catch (Exception e) {
             throw new FieldValueException(e, getName(), containingObject);
         }

diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassType.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassType.java
@@ -5,6 +5,7 @@
 import kotlin.reflect.KCallable;
 import kotlin.reflect.KMutableProperty;
 import kotlin.reflect.KParameter;
+import kotlin.reflect.KProperty1;
 import kotlin.reflect.full.KClasses;
 import kotlin.reflect.jvm.ReflectJvmMapping;
 
@@ -53,22 +54,23 @@ public KotlinDataClassType(Reflector reflector, Class<T> type) {
         var mutableProperties = KClasses.getDeclaredMemberProperties(kClass).stream()
                 .filter(p -> p instanceof KMutableProperty)
                 .collect(toMap(KCallable::getName, m -> m));
+        var immutableProperties = KClasses.getDeclaredMemberProperties(kClass).stream()
+                .filter(p -> !(p instanceof KMutableProperty))
+                .collect(toMap(KCallable::getName, m -> (KProperty1<T, ?>) m));
 
         List<ReflectField> fields = new ArrayList<>();
-        int n = 1;
         for (KParameter param : primaryKtConstructor.getParameters()) {
             var paramName = param.getName();
-
             Preconditions.checkArgument(!mutableProperties.containsKey(paramName),
                     "Mutable constructor arguments are not allowed in '%s', but got: '%s'",
                     kClassName, paramName);
 
-            var callable = functions.get("component" + n);
-            Preconditions.checkState(callable != null,
-                    "Could not find component%s() in '%s'",
-                    n, kClassName);
-            fields.add(new KotlinDataClassComponent(reflector, paramName, callable));
-            n++;
+            var property = immutableProperties.get(paramName);
+            Preconditions.checkArgument(property != null,
+                    "Could not find immutable property in '%s' corresponding to constructor argument '%s'",
+                    kClassName, paramName);
+
+            fields.add(new KotlinDataClassComponent(reflector, paramName, property));
         }
         this.fields = List.copyOf(fields);
     }

diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassTypeFactory.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassTypeFactory.java
@@ -24,6 +24,10 @@ public int priority() {
 
     @Override
     public boolean matches(Class<?> rawType, FieldValueType fvt) {
+        if (!fvt.isComposite()) {
+            return false;
+        }
+
         return KotlinReflectionDetector.kotlinAvailable
                 && stream(rawType.getDeclaredAnnotations()).anyMatch(this::isKotlinClassMetadata)
                 && stream(rawType.getDeclaredMethods()).anyMatch(this::isComponentGetter);

diff --git a/repository/src/test/java/tech/ydb/yoj/repository/hybrid/AccountSnapshotMetadata.java b/repository/src/test/java/tech/ydb/yoj/repository/hybrid/AccountSnapshotMetadata.java
@@ -0,0 +1,6 @@
+package tech.ydb.yoj.repository.hybrid;
+
+import java.time.Instant;
+
+public record AccountSnapshotMetadata(Instant at, String description) {
+}
diff --git a/repository/src/test/java/tech/ydb/yoj/repository/hybrid/ChangeMetadata.java b/repository/src/test/java/tech/ydb/yoj/repository/hybrid/ChangeMetadata.java
diff --git a/repository/src/test/java/tech/ydb/yoj/repository/hybrid/Chronology.java b/repository/src/test/java/tech/ydb/yoj/repository/hybrid/Chronology.java
diff --git a/repository/src/test/java/tech/ydb/yoj/repository/hybrid/JobArgs.java b/repository/src/test/java/tech/ydb/yoj/repository/hybrid/JobArgs.java
@@ -0,0 +1,4 @@
+package tech.ydb.yoj.repository.hybrid;
+
+public record JobArgs(Object request) {
+}
diff --git a/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/Account.kt b/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/Account.kt
@@ -1,22 +1,21 @@
 package tech.ydb.yoj.repository.hybrid
 
 import tech.ydb.yoj.repository.db.Entity
+import java.time.Instant
 
 data class Account(
     private val id: Id,
     private val version: Long,
-    private val chronology: Chronology,
     val login: String,
-    val description: String? = null,
 ) : Entity<Account> {
     override fun getId() = id
 
     data class Id(val value: String) : Entity.Id<Account>
 
     data class Snapshot(
         private val id: Id,
-        private val entity: Account?,
-        val meta: ChangeMetadata,
+        private val account: Account?,
+        val meta: AccountSnapshotMetadata
     ) : Entity<Snapshot> {
         override fun getId() = id
 

diff --git a/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/HybridSchemaTest.kt b/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/HybridSchemaTest.kt
@@ -15,14 +15,12 @@ class HybridSchemaTest {
         val account = Account(
             id = Account.Id("hahaha"),
             version = 1L,
-            chronology = Chronology(now, now),
-            login = "logIn",
-            description = null
+            login = "logIn"
         )
         val accountSnapshot = Account.Snapshot(
             id = Account.Snapshot.Id(account.id, 1L),
-            entity = account,
-            meta = ChangeMetadata(ChangeMetadata.ChangeKind.CREATE, now, 1L)
+            account = account,
+            meta = AccountSnapshotMetadata(now, "Created account '${account.login}'")
         )
 
         val schema = EntitySchema.of(Account.Snapshot::class.java)
@@ -31,15 +29,29 @@ class HybridSchemaTest {
         assertThat(flattened).isEqualTo(mapOf(
             "id_id" to "hahaha",
             "id_version" to 1L,
-            "entity_id" to "hahaha",
-            "entity_version" to 1L,
-            "entity_chronology_createdAt" to now,
-            "entity_chronology_updatedAt" to now,
-            "entity_login" to "logIn",
-            "meta_kind" to ChangeMetadata.ChangeKind.CREATE,
-            "meta_time" to now,
-            "meta_version" to 1L
+            "account_id" to "hahaha",
+            "account_version" to 1L,
+            "account_login" to "logIn",
+            "meta_at" to now,
+            "meta_description" to "Created account 'logIn'"
         ))
         assertThat(schema.newInstance(flattened)).isEqualTo(accountSnapshot)
     }
+
+    @Test
+    fun json() {
+        val op = Job(
+            id = Job.Id("smth"),
+            args = JobArgs("abc")
+        )
+
+        val schema = EntitySchema.of(Job::class.java)
+
+        val flattened = schema.flatten(op)
+        assertThat(flattened).isEqualTo(mapOf(
+            "id" to "smth",
+            "args" to JobArgs("abc")
+        ))
+        assertThat(schema.newInstance(flattened)).isEqualTo(op)
+    }
 }
diff --git a/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/Job.kt b/repository/src/test/kotlin/tech/ydb/yoj/repository/hybrid/Job.kt
@@ -0,0 +1,13 @@
+package tech.ydb.yoj.repository.hybrid
+
+import tech.ydb.yoj.databind.schema.Column
+import tech.ydb.yoj.repository.db.Entity
+
+data class Job(
+    private val id: Id,
+    @Column(flatten = false) val args: JobArgs?,
+) : Entity<Job> {
+    override fun getId() = id
+
+    data class Id(val value: String) : Entity.Id<Job>
+}