#24: FIX: Using query() DSL with a custom value type caused ClassCast…

…Exception
ydb-platform · Mar 14, 2024 · 64c5315 · 64c5315
1 parent 060b9a9
commit 64c5315
Show file tree

Hide file tree

Showing 11 changed files with 130 additions and 24 deletions.
diff --git a/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java b/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java
@@ -21,9 +21,14 @@ public final class CustomValueTypes {
     private CustomValueTypes() {
     }
 
-    public static Object preconvert(@NonNull JavaField field, Object value) {
+    public static Object preconvert(@NonNull JavaField field, @NonNull Object value) {
         var cvt = field.getCustomValueType();
         if (cvt != null) {
+            if (cvt.columnClass().equals(value.getClass())) {
+                // Already preconverted
+                return value;
+            }
+
             value = createCustomValueTypeConverter(cvt).toColumn(field, value);
 
             Preconditions.checkArgument(cvt.columnClass().isInstance(value),
@@ -33,9 +38,14 @@ public static Object preconvert(@NonNull JavaField field, Object value) {
         return value;
     }
 
-    public static Object postconvert(@NonNull JavaField field, Object value) {
+    public static Object postconvert(@NonNull JavaField field, @NonNull Object value) {
         var cvt = field.getCustomValueType();
         if (cvt != null) {
+            if (field.getRawType().equals(value.getClass())) {
+                // Already postconverted
+                return value;
+            }
+
             value = createCustomValueTypeConverter(cvt).toJava(field, value);
         }
         return value;

diff --git a/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java b/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java
@@ -75,47 +75,44 @@ private static FieldValue ofByteArray(@NonNull ByteArray byteArray) {
     }
 
     @NonNull
-    public static FieldValue ofObj(@NonNull Object obj, @NonNull JavaField jf) {
-        return ofObj(obj, jf.getField().getColumn());
-    }
+    public static FieldValue ofObj(@NonNull Object obj, @NonNull JavaField javaField) {
+        FieldValueType fvt = FieldValueType.forJavaType(obj.getClass(), javaField.getField().getColumn());
+        Object postconverted = CustomValueTypes.preconvert(javaField, obj);
 
-    @NonNull
-    @SuppressWarnings({"unchecked", "rawtypes"})
-    private static FieldValue ofObj(@NonNull Object obj, @Nullable Column column) {
-        switch (FieldValueType.forJavaType(obj.getClass(), column)) {
+        switch (fvt) {
             case STRING -> {
-                return ofStr(obj.toString());
+                return ofStr((String) postconverted);
             }
             case ENUM -> {
-                return ofStr(((Enum<?>) obj).name());
+                return ofStr(((Enum<?>) postconverted).name());
             }
             case INTEGER -> {
-                return ofNum(((Number) obj).longValue());
+                return ofNum(((Number) postconverted).longValue());
             }
             case REAL -> {
-                return ofReal(((Number) obj).doubleValue());
+                return ofReal(((Number) postconverted).doubleValue());
             }
             case BOOLEAN -> {
-                return ofBool((Boolean) obj);
+                return ofBool((Boolean) postconverted);
             }
             case BYTE_ARRAY -> {
-                return ofByteArray((ByteArray) obj);
+                return ofByteArray((ByteArray) postconverted);
             }
             case TIMESTAMP -> {
-                return ofTimestamp((Instant) obj);
+                return ofTimestamp((Instant) postconverted);
             }
             case COMPOSITE -> {
-                ObjectSchema schema = ObjectSchema.of(obj.getClass());
+                ObjectSchema schema = ObjectSchema.of(postconverted.getClass());
                 List<JavaField> flatFields = schema.flattenFields();
-                Map<String, Object> flattenedObj = schema.flatten(obj);
+                Map<String, Object> flattenedObj = schema.flatten(postconverted);
 
                 List<JavaFieldValue> allFieldValues = flatFields.stream()
                         .map(jf -> new JavaFieldValue(jf, flattenedObj.get(jf.getName())))
                         .collect(collectingAndThen(toList(), Collections::unmodifiableList));
                 if (allFieldValues.size() == 1) {
                     JavaFieldValue singleValue = allFieldValues.iterator().next();
                     Preconditions.checkArgument(singleValue.getValue() != null, "Wrappers must have a non-null value inside them");
-                    return ofObj(singleValue.getValue(), column);
+                    return ofObj(singleValue.getValue(), singleValue.getField());
                 }
                 return ofTuple(new Tuple(obj, allFieldValues));
             }

diff --git a/...-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java b/...-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java
@@ -25,6 +25,7 @@
 import tech.ydb.yoj.repository.test.sample.model.Team;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 
 import java.util.Set;
 
@@ -112,6 +113,11 @@ public Table<UpdateFeedEntry> updateFeedEntries() {
         public Table<NetworkAppliance> networkAppliances() {
             return table(NetworkAppliance.class);
         }
+
+        @Override
+        public Table<VersionedEntity> versionedEntities() {
+            return table(VersionedEntity.class);
+        }
     }
 
     private static class Supabubble2InMemoryTable extends InMemoryTable<Supabubble2> implements TestEntityOperations.Supabubble2Table {

diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/RepositoryTest.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/RepositoryTest.java
@@ -55,6 +55,8 @@
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak.B;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak.Embedded;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.Version;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 import tech.ydb.yoj.repository.test.sample.model.WithUnflattenableField;
 
 import java.time.Instant;
@@ -2701,6 +2703,28 @@ public void customValueType() {
         assertThat(db.tx(() -> db.networkAppliances().find(app1.id()))).isEqualTo(app1);
     }
 
+    @Test
+    public void customValueTypeInFilter() {
+        var ve = new VersionedEntity(new VersionedEntity.Id("heyhey", new Version(100L)), new Version(100_500L));
+        db.tx(() -> db.versionedEntities().insert(ve));
+        assertThat(db.tx(() -> db.versionedEntities().find(ve.id()))).isEqualTo(ve);
+        assertThat(db.tx(() -> db.versionedEntities().query()
+                .where("id.version").eq(ve.id().version())
+                .and("version2").eq(ve.version2())
+                .findOne()
+        )).isEqualTo(ve);
+        assertThat(db.tx(() -> db.versionedEntities().query()
+                .where("id.version").eq(100L)
+                .and("version2").eq(100_500L)
+                .findOne()
+        )).isEqualTo(ve);
+        assertThat(db.tx(() -> db.versionedEntities().query()
+                .where("id.version").eq(100L)
+                .and("version2").eq(null)
+                .findOne()
+        )).isNull();
+    }
+
     protected void runInTx(Consumer<RepositoryTransaction> action) {
         // We do not retry transactions, because we do not expect conflicts in our test scenarios.
         RepositoryTransaction transaction = startTransaction();

diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/entity/TestEntities.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/entity/TestEntities.java
@@ -1,7 +1,6 @@
 package tech.ydb.yoj.repository.test.entity;
 
 import lombok.NonNull;
-import tech.ydb.yoj.databind.FieldValueType;
 import tech.ydb.yoj.repository.db.Entity;
 import tech.ydb.yoj.repository.db.Repository;
 import tech.ydb.yoj.repository.test.sample.model.Book;
@@ -21,6 +20,7 @@
 import tech.ydb.yoj.repository.test.sample.model.Team;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 import tech.ydb.yoj.repository.test.sample.model.WithUnflattenableField;
 
 import java.util.List;
@@ -43,14 +43,12 @@ private TestEntities() {
             NonDeserializableEntity.class,
             WithUnflattenableField.class,
             UpdateFeedEntry.class,
-            NetworkAppliance.class
+            NetworkAppliance.class,
+            VersionedEntity.class
     );
 
     @SuppressWarnings("unchecked")
     public static Repository init(@NonNull Repository repository) {
-        FieldValueType.registerStringValueType(TypeFreak.Ticket.class);
-        FieldValueType.registerStringValueType(TypeFreak.StringValueWrapper.class);
-
         repository.createTablespace();
         ALL.forEach(entityClass -> repository.schema(entityClass).create());
 

diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/TestEntityOperations.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/TestEntityOperations.java
@@ -21,6 +21,7 @@
 import tech.ydb.yoj.repository.test.sample.model.Team;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -63,6 +64,8 @@ default Table<BytePkEntity> bytePkEntities() {
 
     Table<NetworkAppliance> networkAppliances();
 
+    Table<VersionedEntity> versionedEntities();
+
     class ProjectTable extends AbstractDelegatingTable<Project> {
         public ProjectTable(Table<Project> target) {
             super(target);

diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/TypeFreak.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/TypeFreak.java
@@ -4,7 +4,10 @@
 import lombok.NonNull;
 import lombok.Value;
 import lombok.With;
+import tech.ydb.yoj.databind.CustomValueType;
 import tech.ydb.yoj.databind.DbType;
+import tech.ydb.yoj.databind.FieldValueType;
+import tech.ydb.yoj.databind.converter.StringValueConverter;
 import tech.ydb.yoj.databind.schema.Column;
 import tech.ydb.yoj.repository.db.Entity;
 import tech.ydb.yoj.repository.db.Table;
@@ -115,6 +118,7 @@ public static class StringView implements Table.ViewId<TypeFreak> {
         String stringEmbedded;
     }
 
+    @CustomValueType(columnValueType = FieldValueType.STRING, columnClass = String.class, converter = StringValueConverter.class)
     public static final class StringValueWrapper {
         private final String value;
 
@@ -143,6 +147,7 @@ public String toString() {
      * instance method.
      * <p><em>E.g.,</em> {@code "XYZ-100500" <=> new Ticket(queue="XYZ", num=100500)}
      */
+    @CustomValueType(columnValueType = FieldValueType.STRING, columnClass = String.class, converter = StringValueConverter.class)
     public record Ticket(@NonNull String queue, int num) {
         public Ticket {
             Preconditions.checkArgument(num >= 1, "ticket number must be >= 1");

diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/Version.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/Version.java
@@ -0,0 +1,19 @@
+package tech.ydb.yoj.repository.test.sample.model;
+
+import lombok.NonNull;
+import tech.ydb.yoj.databind.converter.ValueConverter;
+import tech.ydb.yoj.databind.schema.Schema.JavaField;
+
+public record Version(long value) {
+    public static final class Converter implements ValueConverter<Version, Long> {
+        @Override
+        public @NonNull Long toColumn(@NonNull JavaField field, @NonNull Version v) {
+            return v.value();
+        }
+
+        @Override
+        public @NonNull Version toJava(@NonNull JavaField field, @NonNull Long value) {
+            return new Version(value);
+        }
+    }
+}
diff --git a/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/VersionedEntity.java b/repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/VersionedEntity.java
@@ -0,0 +1,32 @@
+package tech.ydb.yoj.repository.test.sample.model;
+
+import tech.ydb.yoj.databind.CustomValueType;
+import tech.ydb.yoj.databind.FieldValueType;
+import tech.ydb.yoj.databind.schema.Column;
+import tech.ydb.yoj.repository.db.Entity;
+import tech.ydb.yoj.repository.db.RecordEntity;
+
+public record VersionedEntity(
+        Id id,
+        @Column(
+                customValueType = @CustomValueType(
+                        columnValueType = FieldValueType.INTEGER,
+                        columnClass = Long.class,
+                        converter = Version.Converter.class
+                )
+        )
+        Version version2
+) implements RecordEntity<VersionedEntity> {
+    public record Id(
+            String value,
+            @Column(
+                    customValueType = @CustomValueType(
+                            columnValueType = FieldValueType.INTEGER,
+                            columnClass = Long.class,
+                            converter = Version.Converter.class
+                    )
+            )
+            Version version
+    ) implements Entity.Id<VersionedEntity> {
+    }
+}
diff --git a/repository-ydb-v1/src/test/java/tech/ydb/yoj/repository/ydb/TestYdbRepository.java b/repository-ydb-v1/src/test/java/tech/ydb/yoj/repository/ydb/TestYdbRepository.java
@@ -30,6 +30,7 @@
 import tech.ydb.yoj.repository.test.sample.model.Team;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 import tech.ydb.yoj.repository.ydb.table.YdbTable;
 import tech.ydb.yoj.repository.ydb.yql.YqlPredicate;
 
@@ -132,6 +133,11 @@ public Table<UpdateFeedEntry> updateFeedEntries() {
         public Table<NetworkAppliance> networkAppliances() {
             return table(NetworkAppliance.class);
         }
+
+        @Override
+        public Table<VersionedEntity> versionedEntities() {
+            return table(VersionedEntity.class);
+        }
     }
 
     private static class YdbSupabubble2Table extends YdbTable<Supabubble2> implements Supabubble2Table {

diff --git a/repository-ydb-v2/src/test/java/tech/ydb/yoj/repository/ydb/TestYdbRepository.java b/repository-ydb-v2/src/test/java/tech/ydb/yoj/repository/ydb/TestYdbRepository.java
@@ -28,6 +28,7 @@
 import tech.ydb.yoj.repository.test.sample.model.Team;
 import tech.ydb.yoj.repository.test.sample.model.TypeFreak;
 import tech.ydb.yoj.repository.test.sample.model.UpdateFeedEntry;
+import tech.ydb.yoj.repository.test.sample.model.VersionedEntity;
 import tech.ydb.yoj.repository.ydb.table.YdbTable;
 
 import java.util.List;
@@ -131,6 +132,11 @@ public Table<UpdateFeedEntry> updateFeedEntries() {
         public Table<NetworkAppliance> networkAppliances() {
             return table(NetworkAppliance.class);
         }
+
+        @Override
+        public Table<VersionedEntity> versionedEntities() {
+            return table(VersionedEntity.class);
+        }
     }
 
     private static class YdbSupabubble2Table extends YdbTable<Supabubble2> implements TestEntityOperations.Supabubble2Table {