Skip to content

Commit

Permalink
Improve @CustomValueType annotation (better generic support, requir…
Browse files Browse the repository at this point in the history
…e Comparable for column class)
  • Loading branch information
nvamelichev committed Feb 23, 2024
1 parent f6c3f86 commit 224655d
Show file tree
Hide file tree
Showing 21 changed files with 200 additions and 309 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tech.ydb.yoj.databind;

import tech.ydb.yoj.ExperimentalApi;
import tech.ydb.yoj.databind.schema.Schema;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;
Expand All @@ -26,12 +27,12 @@
FieldValueType columnValueType();

/**
* Type of value that {@link #converter() converter's} {@link ValueConverter#toColumn(Object) toColumn()} method returns
* Type of value that {@link #converter() converter's} {@link ValueConverter#toColumn(Schema.JavaField, Object)} toColumn()} method returns
*/
Class<?> columnClass();
Class<? extends Comparable<?>> columnClass();

/**
* Converter class. Must have a no-args public constructor.
*/
Class<? extends ValueConverter<?, ?>> converter();
Class<? extends ValueConverter> converter();
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import tech.ydb.yoj.ExperimentalApi;
import tech.ydb.yoj.databind.schema.Column;
import tech.ydb.yoj.databind.schema.CustomConverterException;
import tech.ydb.yoj.databind.schema.Schema.JavaField;

import javax.annotation.Nullable;
import java.lang.reflect.InvocationTargetException;
Expand All @@ -19,9 +20,10 @@ public final class CustomValueTypes {
private CustomValueTypes() {
}

public static Object preconvert(@Nullable CustomValueType cvt, Object value) {
public static Object preconvert(@NonNull JavaField field, Object value) {
var cvt = field.getCustomValueType();
if (cvt != null) {
value = createCustomValueTypeConverter(cvt).toColumn(value);
value = createCustomValueTypeConverter(cvt).toColumn(field, value);

Preconditions.checkArgument(cvt.columnClass().isInstance(value),
"Custom value type converter %s must produce a non-null value of type columnClass()=%s but got value of type %s",
Expand All @@ -30,13 +32,16 @@ public static Object preconvert(@Nullable CustomValueType cvt, Object value) {
return value;
}

public static Object postconvert(@Nullable CustomValueType cvt, Object value) {
public static Object postconvert(@NonNull JavaField field, Object value) {
var cvt = field.getCustomValueType();
if (cvt != null) {
value = createCustomValueTypeConverter(cvt).toJava(value);
value = createCustomValueTypeConverter(cvt).toJava(field, value);
}
return value;
}

// TODO: Add caching to e.g. SchemaRegistry using @CustomValueType+[optionally JavaField if there is @Column annotation]+[type] as key,
// to avoid repetitive construction of ValueConverters
private static <V, C> ValueConverter<V, C> createCustomValueTypeConverter(CustomValueType cvt) {
try {
var ctor = cvt.converter().getDeclaredConstructor();
Expand All @@ -48,7 +53,6 @@ private static <V, C> ValueConverter<V, C> createCustomValueTypeConverter(Custom
}
}


@Nullable
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/24")
public static CustomValueType getCustomValueType(@NonNull Type type, @Nullable Column columnAnnotation) {
Expand Down
14 changes: 5 additions & 9 deletions databind/src/main/java/tech/ydb/yoj/databind/FieldValueType.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ public enum FieldValueType {
/**
* Binary value: just a stream of uninterpreted bytes.
* Java-side <strong>must</strong> be a byte array.
* <p>
* @deprecated It is strongly recommended to use a {@link ByteArray} that is properly {@code Comparable}
* and has a sane {@code equals()}.
*/
@Deprecated
BINARY,
/**
* Binary value: just a stream of uninterpreted bytes.
Expand Down Expand Up @@ -199,15 +203,7 @@ private static FieldValueType forJavaType(@NonNull Type type) {
}

private static boolean isStringValueType(Class<?> clazz) {
if (STRING_VALUE_TYPES.contains(clazz)) {
return true;
}
if (clazz.getAnnotation(StringValueType.class) != null) {
// FIXME: Move the Set of string-value types to SchemaRegistry
registerStringValueType(clazz);
return true;
}
return false;
return STRING_VALUE_TYPES.contains(clazz);
}

/**
Expand Down
106 changes: 106 additions & 0 deletions databind/src/main/java/tech/ydb/yoj/databind/StringValueConverter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package tech.ydb.yoj.databind;

import lombok.NonNull;
import lombok.SneakyThrows;
import tech.ydb.yoj.databind.schema.Schema.JavaField;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.function.Function;

import static java.lang.String.format;
import static java.lang.reflect.Modifier.isStatic;

/**
* Possible String value type replacement: a generic converter that can be applied to your
* type/your columns with
* <blockquote>
* <pre>
* &#64;Column(
* customValueType=&#64;CustomValueType(
* columnValueType=STRING,
* columnClass=&lt;your type&gt;,
* converter=StringValueConverter.class
* )
* )
* </pre>
* </blockquote>
*
* @param <J> Java type
*/
public final class StringValueConverter<J> implements ValueConverter<J, String> {
private volatile Function<String, J> deserializer;

private StringValueConverter() {
}

@Override
public @NonNull String toColumn(@NonNull JavaField field, @NonNull J value) {
return value.toString();
}

@Override
@SuppressWarnings("unchecked")
public @NonNull J toJava(@NonNull JavaField field, @NonNull String column) {
var clazz = field.getRawType();
if (String.class.equals(clazz)) {
return (J) column;
}

var f = deserializer;
if (deserializer == null) {
synchronized (this) {
f = deserializer;
if (f == null) {
deserializer = f = getStringValueDeserializerMethod(clazz);
}
}
}
return f.apply(column);
}

@SuppressWarnings("unchecked")
private static <J> ThrowingFunction<String, J> getStringValueDeserializerMethod(Class<?> clazz) {
for (String methodName : new String[]{"fromString", "valueOf"}) {
try {
Method method = clazz.getMethod(methodName, String.class);
if (isStatic(method.getModifiers())) {
return s -> (J) method.invoke(null, s);
}
} catch (NoSuchMethodException ignored) {
}
}

try {
var ctor = clazz.getConstructor(String.class);
return s -> (J) ctor.newInstance(s);
} catch (NoSuchMethodException ignored) {
}

throw new IllegalArgumentException(format(
"Type <%s> does not have a deserializer method: public static fromString(String)/valueOf(String) and" +
"doesn't have constructor public %s(String)",
clazz.getTypeName(),
clazz.getTypeName()
));
}

private interface ThrowingFunction<T, R> extends Function<T, R> {
R applyThrowing(T t) throws Exception;

@Override
@SneakyThrows
default R apply(T t) {
try {
return applyThrowing(t);
} catch (InvocationTargetException e) {
// Propagate the real exception thrown by the deserializer method.
// All unhandled getter exceptions are wrapped in ConversionException by the Repository, automatically,
// so we don't need to do any wrapping here.
throw e.getCause();
} catch (Exception e) {
throw new IllegalStateException("Reflection problem with deserializer method", e);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import lombok.NonNull;
import tech.ydb.yoj.ExperimentalApi;
import tech.ydb.yoj.databind.schema.Schema.JavaField;

/**
* Custom conversion logic between database column values and Java field values.
Expand All @@ -13,23 +14,23 @@
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/24")
public interface ValueConverter<J, C> {
@NonNull
C toColumn(@NonNull J v);
C toColumn(@NonNull JavaField field, @NonNull J v);

@NonNull
J toJava(@NonNull C c);
J toJava(@NonNull JavaField field, @NonNull C c);

class NoConverter implements ValueConverter<Void, Void> {
private NoConverter() {
throw new UnsupportedOperationException("Not instantiable");
}

@Override
public @NonNull Void toColumn(@NonNull Void v) {
public @NonNull Void toColumn(@NonNull JavaField field, @NonNull Void v) {
throw new UnsupportedOperationException();
}

@Override
public @NonNull Void toJava(@NonNull Void unused) {
public @NonNull Void toJava(@NonNull JavaField field, @NonNull Void unused) {
throw new UnsupportedOperationException();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import lombok.RequiredArgsConstructor;
import lombok.Value;
import tech.ydb.yoj.databind.ByteArray;
import tech.ydb.yoj.databind.CustomValueType;
import tech.ydb.yoj.databind.CustomValueTypes;
import tech.ydb.yoj.databind.FieldValueType;
import tech.ydb.yoj.databind.schema.Column;
Expand Down Expand Up @@ -82,7 +81,7 @@ public static FieldValue ofObj(@NonNull Object obj, @NonNull JavaField jf) {

@NonNull
@SuppressWarnings({"unchecked", "rawtypes"})
public static FieldValue ofObj(@NonNull Object obj, @Nullable Column column) {
private static FieldValue ofObj(@NonNull Object obj, @Nullable Column column) {
switch (FieldValueType.forJavaType(obj.getClass(), column)) {
case STRING -> {
return ofStr(obj.toString());
Expand Down Expand Up @@ -159,10 +158,8 @@ public boolean isByteArray() {
public static Comparable<?> getComparable(@NonNull Map<String, Object> values,
@NonNull JavaField field) {
if (field.isFlat()) {
Type fieldType = field.getFlatFieldType();
Object rawValue = values.get(field.getName());
Column column = field.getField().getColumn();
return rawValue == null ? null : ofObj(rawValue, column).getComparable(fieldType, column);
return rawValue == null ? null : ofObj(rawValue, field).getComparable(field);
} else {
List<JavaFieldValue> components = field.flatten()
.map(jf -> new JavaFieldValue(jf, values.get(jf.getName())))
Expand All @@ -172,22 +169,25 @@ public static Comparable<?> getComparable(@NonNull Map<String, Object> values,
}

@NonNull
public Object getRaw(@NonNull Type fieldType, @Nullable Column column) {
public Object getRaw(@NonNull JavaField field) {
Type fieldType = field.isFlat() ? field.getFlatFieldType() : field.getType();
Column column = field.getField().getColumn();
if (FieldValueType.forJavaType(fieldType, column) == FieldValueType.COMPOSITE) {
Preconditions.checkState(isTuple(), "Value is not a tuple: %s", this);
Preconditions.checkState(tuple.getType().equals(fieldType),
"Tuple value cannot be converted to a composite of type %s: %s", fieldType, this);
return tuple.asComposite();
}

Comparable<?> cmp = getComparable(fieldType, column);
CustomValueType cvt = CustomValueTypes.getCustomValueType(fieldType, column);
return CustomValueTypes.postconvert(cvt, cmp);
Comparable<?> cmp = getComparable(field);
return CustomValueTypes.postconvert(field, cmp);
}

@NonNull
@SuppressWarnings({"unchecked", "rawtypes"})
public Comparable<?> getComparable(@NonNull Type fieldType, @Nullable Column column) {
public Comparable<?> getComparable(@NonNull JavaField field) {
Type fieldType = field.isFlat() ? field.getFlatFieldType() : field.getType();
Column column = field.getField().getColumn();
switch (FieldValueType.forJavaType(fieldType, column)) {
case STRING -> {
Preconditions.checkState(isString(), "Value is not a string: " + this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

import com.google.common.collect.ImmutableList;
import lombok.NonNull;
import tech.ydb.yoj.databind.schema.Column;
import tech.ydb.yoj.databind.schema.Schema.JavaField;

import javax.annotation.Nullable;
import java.util.List;

public abstract class LeafExpression<T> implements FilterExpression<T> {
Expand All @@ -13,14 +12,20 @@ public final List<FilterExpression<T>> getChildren() {
return List.of();
}

public abstract java.lang.reflect.Type getFieldType();
public final java.lang.reflect.Type getFieldType() {
var field = getField();
return field.isFlat() ? field.getFlatFieldType() : field.getType();
}

@Nullable
public abstract Column getColumnAnnotation();
public final String getFieldName() {
return getField().getName();
}

public abstract String getFieldName();
public final String getFieldPath() {
return getField().getPath();
}

public abstract String getFieldPath();
public abstract JavaField getField();

public abstract boolean isGenerated();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import lombok.AllArgsConstructor;
import lombok.NonNull;
import lombok.Value;
import tech.ydb.yoj.databind.schema.Column;
import tech.ydb.yoj.databind.schema.Schema;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -38,36 +37,14 @@ public FilterExpression.Type getType() {
return Type.LIST;
}

@Override
public java.lang.reflect.Type getFieldType() {
return field.isFlat() ? field.getFlatFieldType() : field.getType();
}

@Override
public String getFieldName() {
return field.getName();
}

@Override
public String getFieldPath() {
return field.getPath();
}

@Override
public Column getColumnAnnotation() {
return field.getField().getColumn();
}

@Nullable
public Comparable<?> getActualValue(@NonNull T obj) {
return FieldValue.getComparable(schema.flatten(obj), field);
}

@NonNull
public List<Comparable<?>> getExpectedValues() {
java.lang.reflect.Type fieldType = getFieldType();
Column column = field.getField().getColumn();
return values.stream().map(v -> v.getComparable(fieldType, column)).collect(toList());
return values.stream().map(v -> v.getComparable(field)).collect(toList());
}

@Override
Expand Down
Loading

0 comments on commit 224655d

Please sign in to comment.