noticket: Allow to use any wrapper (and not only top-level wrapper/raw unwrapped value) in FieldValue.ofObj() for flat fields

Author: nvamelichev

databind/src/main/java/tech/ydb/yoj/databind/expression/values/FieldValue.java

@@ -6,8 +6,9 @@
 import tech.ydb.yoj.databind.CustomValueTypes;
 import tech.ydb.yoj.databind.FieldValueType;
 import tech.ydb.yoj.databind.expression.IllegalExpressionException;
-import tech.ydb.yoj.databind.schema.ObjectSchema;
+import tech.ydb.yoj.databind.schema.Schema;
 import tech.ydb.yoj.databind.schema.Schema.JavaField;
+import tech.ydb.yoj.databind.schema.naming.NamingStrategy;
 
 import javax.annotation.Nullable;
 import java.lang.reflect.Type;
@@ -33,6 +34,8 @@ public sealed interface FieldValue extends tech.ydb.yoj.databind.expression.Fiel
 
     @Override
     default Object getRaw(@NonNull JavaField field) {
+        field = field.isFlat() ? field.toFlatField() : field;
+
         Comparable<?> cmp = getComparable(field);
         return CustomValueTypes.postconvert(field, cmp);
     }
@@ -49,7 +52,8 @@ default Comparable<?> getComparable(@NonNull JavaField field) {
     }
 
     static FieldValue ofObj(@NonNull Object obj, @NonNull JavaField schemaField) {
-        FieldValueType fvt = FieldValueType.forJavaType(obj.getClass(), schemaField.getField());
+        Class<?> objRawType = obj.getClass();
+        FieldValueType fvt = FieldValueType.forJavaType(objRawType, schemaField.getField());
         obj = CustomValueTypes.preconvert(schemaField, obj);
 
         return switch (fvt) {
@@ -62,19 +66,37 @@ static FieldValue ofObj(@NonNull Object obj, @NonNull JavaField schemaField) {
             case TIMESTAMP -> new TimestampFieldValue((Instant) obj);
             case UUID -> new UuidFieldValue((UUID) obj);
             case COMPOSITE -> {
-                ObjectSchema<?> schema = ObjectSchema.of(obj.getClass());
+                JavaField innerField;
+                if (schemaField.isFlat()) {
+                    // For flat fields, walk the chain of wrappers to find a wrapper which matches the obj's raw type exactly
+                    innerField = schemaField.getFlatRoot().findFlatChild(child -> child.getRawType().equals(objRawType));
+                } else {
+                    // For non-flat fields, we just require an exact match to obj's raw type (or else Schema.flatten() will fail!)
+                    innerField = schemaField;
+                }
+                Preconditions.checkArgument(innerField != null && innerField.getRawType().equals(objRawType),
+                        "Composite schema field %s is not compatible with value of type %s", schemaField, objRawType);
+
+                class InnerSchema<T> extends Schema<T> {
+                    protected InnerSchema(JavaField subSchemaField) {
+                        super(subSchemaField, (NamingStrategy) null);
+                    }
+                }
+
+                Schema<?> schema = new InnerSchema<>(innerField);
                 List<JavaField> flatFields = schema.flattenFields();
 
                 @SuppressWarnings({"rawtypes", "unchecked"})
-                Map<String, Object> flattenedObj = ((ObjectSchema) schema).flatten(obj);
+                Map<String, Object> flattenedObj = ((Schema) schema).flatten(obj);
 
                 List<Tuple.FieldAndValue> allFieldValues = tupleValues(flatFields, flattenedObj);
                 if (allFieldValues.size() == 1) {
                     FieldValue singleValue = allFieldValues.iterator().next().value();
                     Preconditions.checkArgument(singleValue != null, "Wrappers must have a non-null value inside them");
                     yield singleValue;
+                } else {
+                    yield new TupleFieldValue(new Tuple(obj, allFieldValues));
                 }
-                yield new TupleFieldValue(new Tuple(obj, allFieldValues));
             }
             default -> throw new UnsupportedOperationException("Unsupported value type: not a string, integer, timestamp, UUID, enum, "
                     + "floating-point number, byte array, tuple or wrapper of the above");

databind/src/main/java/tech/ydb/yoj/databind/schema/Schema.java

@@ -27,6 +27,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -35,7 +36,6 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
 import java.util.regex.Pattern;
@@ -45,15 +45,10 @@
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
-import static java.util.stream.Collectors.toMap;
-import static lombok.AccessLevel.PROTECTED;
 
 public abstract class Schema<T> {
     public static final String PATH_DELIMITER = ".";
 
-    @Getter(PROTECTED)
-    private final SchemaKey<T> schemaKey;
-
     @Getter
     private final List<JavaField> fields;
 
@@ -69,6 +64,10 @@ public abstract class Schema<T> {
 
     protected final ReflectType<T> reflectType;
 
+    private final Class<T> type;
+    private final NamingStrategy namingStrategy;
+
+    @Deprecated
     private final String staticName;
 
     protected Schema(@NonNull Class<T> type) {
@@ -88,12 +87,11 @@ protected Schema(@NonNull Class<T> type, @NonNull NamingStrategy namingStrategy,
     }
 
     protected Schema(@NonNull SchemaKey<T> key, @NonNull Reflector reflector) {
-        Class<T> type = key.clazz();
-        NamingStrategy namingStrategy = key.namingStrategy();
+        this.type = key.clazz();
+        this.namingStrategy = key.namingStrategy();
 
         this.reflectType = reflector.reflectRootType(type);
 
-        this.schemaKey = key;
         this.staticName = namingStrategy.getNameForClass(type);
 
         this.fields = reflectType.getFields().stream().map(this::newRootJavaField).toList();
@@ -106,17 +104,24 @@ protected Schema(@NonNull SchemaKey<T> key, @NonNull Reflector reflector) {
         this.ttlModifier = prepareTtlModifier(extractTtlModifier(type));
         this.changefeeds = prepareChangefeeds(collectChangefeeds(type));
     }
-    
+
     protected Schema(Schema<?> schema, String subSchemaFieldPath) {
-        JavaField subSchemaField = schema.getField(subSchemaFieldPath);
+        this(schema.getField(subSchemaFieldPath), schema.getNamingStrategy());
+    }
 
+    protected Schema(JavaField subSchemaField, @Nullable NamingStrategy parentNamingStrategy) {
         @SuppressWarnings("unchecked") ReflectType<T> rt = (ReflectType<T>) subSchemaField.field.getReflectType();
-        this.reflectType = rt;
 
-        this.schemaKey = schema.schemaKey.withClazz(reflectType.getRawType());
+        this.reflectType = rt;
+        this.type = rt.getRawType();
+        this.namingStrategy = parentNamingStrategy == null ? SUBFIELD_SCHEMA_NAMING_STRATEGY : parentNamingStrategy;
 
-        this.staticName = schema.staticName;
-        this.globalIndexes = schema.globalIndexes;
+        // This is a subfield, *NOT* an Entity, so it has no table name, no TTL, no indexes and no changefeeds
+        // (And also, no useful naming strategy, because all field names have already been assigned by the moment you construct a subfield Schema!)
+        this.staticName = "";
+        this.ttlModifier = null;
+        this.globalIndexes = List.of();
+        this.changefeeds = List.of();
 
         if (subSchemaField.fields != null) {
             this.fields = subSchemaField.fields.stream().map(this::newRootJavaField).toList();
@@ -129,19 +134,22 @@ protected Schema(Schema<?> schema, String subSchemaFieldPath) {
                 this.fields = List.of();
             }
         }
-        this.ttlModifier = schema.ttlModifier;
-        this.changefeeds = schema.changefeeds;
     }
 
-    public String getTypeName() {
-        return getType().getSimpleName();
+    public final String getTypeName() {
+        return type.getSimpleName();
     }
 
     private void validateFieldNames() {
-        flattenFields().stream().collect(toMap(JavaField::getName, Function.identity(), ((x, y) -> {
-            throw new IllegalArgumentException("fields with same name `%s` detected: `{%s}` and `{%s}`"
-                    .formatted(x.getName(), x.getField(), y.getField()));
-        })));
+        Map<String, JavaField> fieldNames = new HashMap<>();
+        for (JavaField field : flattenFields()) {
+            String fieldName = field.getName();
+            JavaField existingField = fieldNames.putIfAbsent(fieldName, field);
+            if (existingField != null) {
+                throw new IllegalArgumentException("fields with same name \"%s\" detected: {%s} and {%s}"
+                        .formatted(fieldName, field.getField(), existingField.getField()));
+            }
+        }
     }
 
     private List<Index> prepareIndexes(List<GlobalIndex> indexes) {
@@ -276,23 +284,29 @@ protected boolean isFlattenable(ReflectField field) {
     }
 
     public final Class<T> getType() {
-        return schemaKey.clazz();
+        return type;
     }
 
     public final NamingStrategy getNamingStrategy() {
-        return schemaKey.namingStrategy();
+        return namingStrategy;
     }
 
     /**
-     * DEPRECATED: old method, use correct instance of {@link TableDescriptor}
-     * Returns the name of the table for data binding.
-     * <p>
-     * If the {@link Table} annotation is present, the field {@code name} should be used to
-     * specify the table name.
+     * @deprecated This method will be pulled down to {@code EntitySchema} in YOJ 3.0.0 or even earlier; and it might be removed in YOJ 3.x.
+     * <br>YOJ end-users <strong>should never</strong> use this method themselves. To customize table name, just add the {@link Table} annotation to
+     * an {@code Entity} and specify the desired name in the annotation's {@code name} field. To dynamically choose table name, use
+     * the {@code BaseDb.table(TableDescriptor)} method inside your transaction.
+     * <br>
+     * This method always had somewhat unclear semantics (it was never specified what it returns for anything that's not an {@code EntitySchema})
+     * and unnecessarily coupled the data-binding model ({@code Schema}s) to database concepts (tables, which have names, and implementation-defined
+     * syntax for names and paths).
      *
-     * @return the table name for data binding
+     * @return this {@code Schema}'s "name", as determined by {@code NamingStrategy}. For {@code EntitySchema}, this will be the <em>table name</em>
+     *         that's used if you don't obtain the table with an explicit {@code TableDescriptor}. Other instances of {@code Schema} are not
+     *         guaranteed to return anything meaningful and/or useful from this method, and might return an empty {@code String}
+     *         (but <em>not</em> {@code null}.)
      */
-    @Deprecated
+    @Deprecated(forRemoval = true)
     public final String getName() {
         return staticName;
     }
@@ -375,20 +389,15 @@ private Optional<JavaField> findField(String... pathComponents) {
 
     @Override
     public final int hashCode() {
-        return Objects.hashCode(staticName);
+        return Objects.hash(getClass(), getType(), getNamingStrategy());
     }
 
     @Override
     public final boolean equals(Object o) {
-        if (this == o) {
-            return true;
-        }
-        if (o == null || getClass() != o.getClass()) {
-            return false;
-        }
-
-        Schema<?> other = (Schema<?>) o;
-        return Objects.equals(staticName, other.staticName);
+        return o instanceof Schema<?> otherSchema
+                && otherSchema.getClass().equals(this.getClass())
+                && otherSchema.getType().equals(this.getType())
+                && otherSchema.getNamingStrategy().equals(this.getNamingStrategy());
     }
 
     @Override
@@ -398,7 +407,9 @@ public final String toString() {
             schemaName = getClass().getName();
         }
 
-        return schemaName + " \"" + staticName + "\" [type=" + getType().getName() + "]";
+        String staticTableName = staticName.isEmpty() ? "" : " \"" + staticName + "\"";
+
+        return schemaName + staticTableName + " [type=" + getTypeName() + "]";
     }
 
     private static final class DummyCustomValueSubField implements ReflectField {
@@ -626,6 +637,41 @@ public boolean isFlat() {
             return getSimpleFieldCardinality(this) == 1;
         }
 
+        /**
+         * @return the uppermost field that contains this flat field and is still {@link #isFlat() flat}; {@code this} if no such flat field exists
+         * @throws IllegalStateException if this field is not {@link #isFlat() flat}
+         */
+        @ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/pull/130")
+        public JavaField getFlatRoot() {
+            Preconditions.checkState(isFlat(), "Cannot get flat parent for a non-flat field");
+
+            JavaField flatRoot = this;
+            while (flatRoot.parent != null && flatRoot.parent.getChildren().size() == 1) {
+                flatRoot = flatRoot.parent;
+            }
+            return flatRoot;
+        }
+
+        /**
+         * @param condition the condition for matching the fields
+         * @return the outermost flat child field that {@code this} field contains (including {@code this} itself!) that matches the {@code condition}
+         * @throws IllegalStateException if this field is not {@link #isFlat() flat}
+         */
+        @Nullable
+        @ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/pull/130")
+        public JavaField findFlatChild(@NonNull Predicate<JavaField> condition) {
+            Preconditions.checkState(isFlat(), "Cannot get flat child for a non-flat field");
+
+            JavaField current = this;
+            while (current != null) {
+                if (condition.test(current)) {
+                    return current;
+                }
+                current = current.getChildren().isEmpty() ? null : current.getChildren().get(0);
+            }
+            return null;
+        }
+
         /**
          * Determining that a java field is mapped in more than one database field.
          *
@@ -843,4 +889,21 @@ public static class Consumer {
             boolean important;
         }
     }
+
+    private static final NamingStrategy SUBFIELD_SCHEMA_NAMING_STRATEGY = new NamingStrategy() {
+        @Override
+        public String getNameForClass(@NonNull Class<?> entityClass) {
+            throw new UnsupportedOperationException("Schema.SUBFIELD_SCHEMA_NAMING_STRATEGY.getNameForClass() must never be called");
+        }
+
+        @Override
+        public void assignFieldName(@NonNull JavaField javaField) {
+            throw new UnsupportedOperationException("Schema.SUBFIELD_SCHEMA_NAMING_STRATEGY.assignFieldName() must never be called");
+        }
+
+        @Override
+        public String toString() {
+            return "Schema.SUBFIELD_SCHEMA_NAMING_STRATEGY";
+        }
+    };
 }

repository-inmemory/src/test/java/tech/ydb/yoj/repository/test/inmemory/TestInMemoryRepository.java

@@ -18,6 +18,7 @@
 import tech.ydb.yoj.repository.test.sample.model.IndexedEntity;
 import tech.ydb.yoj.repository.test.sample.model.LogEntry;
 import tech.ydb.yoj.repository.test.sample.model.MultiWrappedEntity;
+import tech.ydb.yoj.repository.test.sample.model.MultiWrappedEntity2;
 import tech.ydb.yoj.repository.test.sample.model.NetworkAppliance;
 import tech.ydb.yoj.repository.test.sample.model.Primitive;
 import tech.ydb.yoj.repository.test.sample.model.Project;
@@ -136,6 +137,11 @@ public Table<DetachedEntity> detachedEntities() {
         public Table<MultiWrappedEntity> multiWrappedIdEntities() {
             return table(MultiWrappedEntity.class);
         }
+
+        @Override
+        public Table<MultiWrappedEntity2> multiWrappedEntities2() {
+            return table(MultiWrappedEntity2.class);
+        }
     }
 
     private static class Supabubble2InMemoryTable extends InMemoryTable<Supabubble2> implements TestEntityOperations.Supabubble2Table {