From 64c53155e302311961ef475f997de7d003e7a57e Mon Sep 17 00:00:00 2001 From: Nikolai Amelichev Date: Thu, 14 Mar 2024 16:59:04 +0100 Subject: [PATCH] #24: FIX: Using query() DSL with a custom value type caused ClassCastException --- .../ydb/yoj/databind/CustomValueTypes.java | 14 ++++++-- .../yoj/databind/expression/FieldValue.java | 31 ++++++++---------- .../test/inmemory/TestInMemoryRepository.java | 6 ++++ .../yoj/repository/test/RepositoryTest.java | 24 ++++++++++++++ .../repository/test/entity/TestEntities.java | 8 ++--- .../test/sample/TestEntityOperations.java | 3 ++ .../test/sample/model/TypeFreak.java | 5 +++ .../repository/test/sample/model/Version.java | 19 +++++++++++ .../test/sample/model/VersionedEntity.java | 32 +++++++++++++++++++ .../yoj/repository/ydb/TestYdbRepository.java | 6 ++++ .../yoj/repository/ydb/TestYdbRepository.java | 6 ++++ 11 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/Version.java create mode 100644 repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/VersionedEntity.java diff --git a/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java b/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java index ab33f667..ccad0cfa 100644 --- 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 index 8a8ef2ec..807f7128 100644 --- 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,39 +75,36 @@ 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 flatFields = schema.flattenFields(); - Map flattenedObj = schema.flatten(obj); + Map flattenedObj = schema.flatten(postconverted); List allFieldValues = flatFields.stream() .map(jf -> new JavaFieldValue(jf, flattenedObj.get(jf.getName()))) @@ -115,7 +112,7 @@ private static FieldValue ofObj(@NonNull Object obj, @Nullable Column column) { 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/repository-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java b/repository-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java index 4af28131..40b1ddac 100644 --- a/repository-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java +++ b/repository-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 updateFeedEntries() { public Table networkAppliances() { return table(NetworkAppliance.class); } + + @Override + public Table versionedEntities() { + return table(VersionedEntity.class); + } } private static class Supabubble2InMemoryTable extends InMemoryTable 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 index 746f6fc2..b69865a7 100644 --- 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 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 index 8f5a7429..e5159102 100644 --- 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 index 1ce2ae81..39ee35ee 100644 --- 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 bytePkEntities() { Table networkAppliances(); + Table versionedEntities(); + class ProjectTable extends AbstractDelegatingTable { public ProjectTable(Table 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 index ad85595f..c3746112 100644 --- 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 { 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. *

E.g., {@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 new file mode 100644 index 00000000..855edee2 --- /dev/null +++ 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 { + @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 new file mode 100644 index 00000000..4cf162ef --- /dev/null +++ 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 { + public record Id( + String value, + @Column( + customValueType = @CustomValueType( + columnValueType = FieldValueType.INTEGER, + columnClass = Long.class, + converter = Version.Converter.class + ) + ) + Version version + ) implements Entity.Id { + } +} 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 index 5bd7df15..8e67aaa6 100644 --- 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 updateFeedEntries() { public Table networkAppliances() { return table(NetworkAppliance.class); } + + @Override + public Table versionedEntities() { + return table(VersionedEntity.class); + } } private static class YdbSupabubble2Table extends YdbTable 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 index 193b0fd4..b22c8fc9 100644 --- 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 updateFeedEntries() { public Table networkAppliances() { return table(NetworkAppliance.class); } + + @Override + public Table versionedEntities() { + return table(VersionedEntity.class); + } } private static class YdbSupabubble2Table extends YdbTable implements TestEntityOperations.Supabubble2Table {