noticket: FIX: Unexpected NPE when trying to validate conversion result in CustomValueTypes.preconvert()

This happens when a custom-value converter erroneously returns `null` from `ValueConverter.toColumn()`,
e.g. because of a bad `null`-returning `toString()` implementation in a string-value type.

This should not happen in `CustomValueTypes.postconvert()` because the `null -> null` logic is applied
before that, and a null `value` should not get into `CustomValueTypes.postconvert()` **at all**.

Author: nvamelichev

databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java

@@ -37,10 +37,11 @@ public static Object preconvert(@NonNull JavaField field, @NonNull Object value)
 
         value = cvt.toColumn(field, value);
         Preconditions.checkArgument(cvt.getColumnClass().isInstance(value),
-                "Custom value type converter %s must produce a non-null value of type columnClass()=%s but got value of type %s",
+                "Custom value type converter %s must produce a non-null value of type columnClass()=%s but got a %s",
                 cvt.getConverter().getClass().getCanonicalName(),
                 cvt.getColumnClass().getCanonicalName(),
-                value.getClass().getCanonicalName());
+                // NB: do not simplify this condition to `false`! This is explicitly to catch custom value converters that return a `null` by mistake!
+                value == null ? "null value" : "value of type " + value.getClass().getCanonicalName());
         return value;
     }
 
@@ -50,7 +51,10 @@ public static <C extends Comparable<? super C>> Object postconvert(@NonNull Java
             return value;
         }
 
-        Preconditions.checkArgument(value instanceof Comparable, "postconvert() only takes Comparable values, but got value of %s", value.getClass());
+        Preconditions.checkArgument(value instanceof Comparable, "postconvert() only takes Comparable values, but got value of %s",
+                // NB: here we don't need to check `value` for `null`, because it got here from YDB/InMemory repository deserialization logic,
+                // which won't even call the `postconvert()` method if `value` is `null`
+                value.getClass().getCanonicalName());
 
         @SuppressWarnings("unchecked") C comparable = (C) value;
         return cvt.toJava(field, comparable);

repository-test/src/main/java/tech/ydb/yoj/repository/test/RepositoryTest.java

@@ -43,6 +43,8 @@
 import tech.ydb.yoj.repository.test.sample.TestDb;
 import tech.ydb.yoj.repository.test.sample.TestDbImpl;
 import tech.ydb.yoj.repository.test.sample.TestEntityOperations;
+import tech.ydb.yoj.repository.test.sample.model.BadlyWrappedEntity;
+import tech.ydb.yoj.repository.test.sample.model.BadlyWrappedEntity.BadStringValueWrapper;
 import tech.ydb.yoj.repository.test.sample.model.Book;
 import tech.ydb.yoj.repository.test.sample.model.Bubble;
 import tech.ydb.yoj.repository.test.sample.model.BytePkEntity;
@@ -2963,6 +2965,21 @@ public void multiWrapperEntity2WithCustomConverterFullyWrapped() {
         )).isEmpty();
     }
 
+    @Test
+    public void nullWrappingStringValueColumn() {
+        // must not be able to save a string-value field that returns null from toString()
+        var badWrapperWithNullInside = new BadlyWrappedEntity(new BadlyWrappedEntity.Id("xxx"), new BadStringValueWrapper(null));
+        assertThatExceptionOfType(ConversionException.class)
+                .isThrownBy(() -> db.tx(() -> db.table(BadlyWrappedEntity.class).save(badWrapperWithNullInside)))
+                .withCauseInstanceOf(IllegalArgumentException.class);
+
+        // null column value -> null string-value type field, without invoking the converter at all
+        var noWrapper = new BadlyWrappedEntity(new BadlyWrappedEntity.Id("yyy"), null);
+        db.tx(() -> db.table(BadlyWrappedEntity.class).save(noWrapper));
+        assertThat(db.tx(() -> db.table(BadlyWrappedEntity.class).find(new BadlyWrappedEntity.Id("yyy"))).badWrapper()).isNull();
+        assertThat(db.tx(() -> db.table(BadlyWrappedEntity.class).find(new BadlyWrappedEntity.Id("yyy")))).isEqualTo(noWrapper);
+    }
+
     @Test
     public void lenientEnumBehaviorExplicit() {
         try {

repository-test/src/main/java/tech/ydb/yoj/repository/test/entity/TestEntities.java

@@ -4,6 +4,7 @@
 import tech.ydb.yoj.repository.db.Entity;
 import tech.ydb.yoj.repository.db.Repository;
 import tech.ydb.yoj.repository.db.TableDescriptor;
+import tech.ydb.yoj.repository.test.sample.model.BadlyWrappedEntity;
 import tech.ydb.yoj.repository.test.sample.model.Book;
 import tech.ydb.yoj.repository.test.sample.model.Bubble;
 import tech.ydb.yoj.repository.test.sample.model.BytePkEntity;
@@ -62,6 +63,7 @@ private TestEntities() {
             DetachedEntity.class,
             MultiWrappedEntity.class,
             MultiWrappedEntity2.class,
+            BadlyWrappedEntity.class,
             UniqueEntity.class,
             UniqueEntityNative.class,
             EnumEntity.class

repository-test/src/main/java/tech/ydb/yoj/repository/test/sample/model/BadlyWrappedEntity.java

@@ -0,0 +1,34 @@
+package tech.ydb.yoj.repository.test.sample.model;
+
+import lombok.NonNull;
+import lombok.RequiredArgsConstructor;
+import tech.ydb.yoj.databind.converter.StringValueType;
+import tech.ydb.yoj.repository.db.Entity;
+import tech.ydb.yoj.repository.db.RecordEntity;
+
+import javax.annotation.Nullable;
+
+public record BadlyWrappedEntity(
+        @NonNull Id id,
+        @Nullable BadStringValueWrapper badWrapper
+) implements RecordEntity<BadlyWrappedEntity> {
+    public record Id(String value) implements Entity.Id<BadlyWrappedEntity> {
+    }
+
+    @StringValueType
+    @RequiredArgsConstructor
+    public static final class BadStringValueWrapper {
+        private final String value;
+
+        @NonNull
+        public static BadStringValueWrapper valueOf(@NonNull String str) {
+            return new BadStringValueWrapper(str);
+        }
+
+        @Override
+        public String toString() {
+            // MWAHAHAHAHAHAH!!! StringValueConverter doesn't expect that, for sure!
+            return value;
+        }
+    }
+}