From 4bf46274a262dd745a57de3a3669b5636fd079d1 Mon Sep 17 00:00:00 2001 From: Nikolai Amelichev Date: Fri, 8 Aug 2025 20:54:53 +0200 Subject: [PATCH] noticket: Deprecate `Entity.Id.{getType(), isPartial()}` for removal, and use Guava `TypeToken` only where it's absolutely needed --- .../ydb/yoj/databind/CustomValueTypes.java | 4 ++-- .../expression/values/StringFieldValue.java | 6 ++--- .../reflect/KotlinDataClassComponent.java | 11 +++++----- .../databind/schema/reflect/StdReflector.java | 3 +-- .../yoj/databind/schema/reflect/Types.java | 18 +++++++++++++++ .../test/inmemory/InMemoryTable.java | 8 +++---- .../yoj/repository/ydb/table/YdbTable.java | 15 +++++++++++-- .../db/AbstractDelegatingTable.java | 16 ++++++++++++++ .../tech/ydb/yoj/repository/db/Entity.java | 22 +++++++++++++++---- .../ydb/yoj/repository/db/TableQueryImpl.java | 9 +++++++- .../repository/db/cache/DbValueUpdater.java | 8 +++++++ .../db/projection/RwProjectionCache.java | 16 ++++++++++---- .../ydb/yoj/repository/db/PojoEntityTest.java | 6 ++--- .../yoj/repository/db/RecordEntityTest.java | 5 +++-- 14 files changed, 114 insertions(+), 33 deletions(-) create mode 100644 databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/Types.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 d6cb7f18..507c01a4 100644 --- a/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java +++ b/databind/src/main/java/tech/ydb/yoj/databind/CustomValueTypes.java @@ -1,7 +1,6 @@ package tech.ydb.yoj.databind; import com.google.common.base.Preconditions; -import com.google.common.reflect.TypeToken; import lombok.NonNull; import tech.ydb.yoj.ExperimentalApi; import tech.ydb.yoj.InternalApi; @@ -10,6 +9,7 @@ import tech.ydb.yoj.databind.schema.CustomConverterException; import tech.ydb.yoj.databind.schema.CustomValueTypeInfo; import tech.ydb.yoj.databind.schema.Schema.JavaField; +import tech.ydb.yoj.databind.schema.reflect.Types; import tech.ydb.yoj.util.lang.Annotations; import javax.annotation.Nullable; @@ -71,7 +71,7 @@ private static > ValueConverter createC public static > CustomValueTypeInfo getCustomValueTypeInfo( @NonNull Type type, @Nullable Column columnAnnotation ) { - Class rawType = type instanceof Class ? (Class) type : TypeToken.of(type).getRawType(); + Class rawType = Types.getRawType(type); CustomValueType cvt = getCustomValueType(rawType, columnAnnotation); if (cvt == null) { return null; diff --git a/databind/src/main/java/tech/ydb/yoj/databind/expression/values/StringFieldValue.java b/databind/src/main/java/tech/ydb/yoj/databind/expression/values/StringFieldValue.java index 5e47fb69..702d2504 100644 --- a/databind/src/main/java/tech/ydb/yoj/databind/expression/values/StringFieldValue.java +++ b/databind/src/main/java/tech/ydb/yoj/databind/expression/values/StringFieldValue.java @@ -1,11 +1,11 @@ package tech.ydb.yoj.databind.expression.values; -import com.google.common.reflect.TypeToken; import lombok.NonNull; import tech.ydb.yoj.databind.FieldValueType; import tech.ydb.yoj.databind.expression.IllegalExpressionException.FieldTypeError.StringFieldExpected; import tech.ydb.yoj.databind.expression.IllegalExpressionException.FieldTypeError.UnknownEnumConstant; import tech.ydb.yoj.databind.expression.IllegalExpressionException.FieldTypeError.UuidFieldExpected; +import tech.ydb.yoj.databind.schema.reflect.Types; import java.lang.reflect.Type; import java.util.Optional; @@ -23,7 +23,7 @@ public Optional> getComparableByType(Type fieldType, FieldValueTyp return switch (valueType) { case STRING -> Optional.of(str); case ENUM -> { - @SuppressWarnings({"rawtypes", "unchecked"}) var enumType = (Class) TypeToken.of(fieldType).getRawType(); + @SuppressWarnings({"rawtypes", "unchecked"}) var enumType = (Class) Types.getRawType(fieldType); @SuppressWarnings("unchecked") var enumValue = (Comparable) Enum.valueOf(enumType, str); yield Optional.of(enumValue); } @@ -45,7 +45,7 @@ public Optional> getComparableByType(Type fieldType, FieldValueTyp public ValidationResult isValidValueOfType(Type fieldType, FieldValueType valueType) { return switch (valueType) { case STRING -> validFieldValue(); - case ENUM -> enumHasConstant(TypeToken.of(fieldType).getRawType(), str) + case ENUM -> enumHasConstant(Types.getRawType(fieldType), str) ? validFieldValue() : invalidFieldValue(p -> new UnknownEnumConstant(p, str), p -> format("Unknown enum constant for field \"%s\": \"%s\"", p, str)); case UUID -> isValidUuid() diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java index e9959aec..443bfeb1 100644 --- a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java +++ b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/KotlinDataClassComponent.java @@ -1,7 +1,6 @@ package tech.ydb.yoj.databind.schema.reflect; import com.google.common.base.Preconditions; -import com.google.common.reflect.TypeToken; import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KClass; import kotlin.reflect.KClassifier; @@ -22,18 +21,18 @@ private final KProperty1.Getter getter; public KotlinDataClassComponent(Reflector reflector, String name, KProperty1 property) { - super(reflector, name, genericType(property), rawType(property), field(property)); + super(reflector, name, genericJavaType(property), rawJavaClass(property), field(property)); this.getter = property.getGetter(); KCallablesJvm.setAccessible(this.getter, true); } - private static Type genericType(KProperty1 property) { + private static Type genericJavaType(KProperty1 property) { return ReflectJvmMapping.getJavaType(property.getReturnType()); } - private static Class rawType(KProperty1 property) { - Type genericType = genericType(property); + private static Class rawJavaClass(KProperty1 property) { + Type javaType = genericJavaType(property); KType kPropertyType = property.getReturnType(); KClassifier kClassifier = kPropertyType.getClassifier(); @@ -41,7 +40,7 @@ private static Class rawType(KProperty1 property) { return JvmClassMappingKt.getJavaClass(kClass); } else { // fallback to Guava's TypeToken if kotlin-reflect returns unpredictable results ;-) - return TypeToken.of(genericType).getRawType(); + return Types.getRawType(javaType); } } diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/StdReflector.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/StdReflector.java index 8b78c03f..59a4a251 100644 --- a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/StdReflector.java +++ b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/StdReflector.java @@ -1,6 +1,5 @@ package tech.ydb.yoj.databind.schema.reflect; -import com.google.common.reflect.TypeToken; import lombok.NonNull; import tech.ydb.yoj.databind.FieldValueType; import tech.ydb.yoj.databind.schema.configuration.SchemaRegistry; @@ -49,7 +48,7 @@ public ReflectType reflectFieldType(Type genericType, FieldValueType bindingT } private ReflectType reflectFor(Type type, FieldValueType fvt) { - Class rawType = type instanceof Class clazz ? clazz : TypeToken.of(type).getRawType(); + Class rawType = Types.getRawType(type); for (TypeFactory m : matchers) { if (m.matches(rawType, fvt)) { return m.create(this, rawType, fvt); diff --git a/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/Types.java b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/Types.java new file mode 100644 index 00000000..35748c83 --- /dev/null +++ b/databind/src/main/java/tech/ydb/yoj/databind/schema/reflect/Types.java @@ -0,0 +1,18 @@ +package tech.ydb.yoj.databind.schema.reflect; + +import com.google.common.reflect.TypeToken; +import lombok.NonNull; +import tech.ydb.yoj.InternalApi; + +import java.lang.reflect.Type; + +@InternalApi +public final class Types { + private Types() { + } + + @NonNull + public static Class getRawType(@NonNull Type genericType) { + return genericType instanceof Class clazz ? clazz : TypeToken.of(genericType).getRawType(); + } +} diff --git a/repository-inmemory/src/main/java/tech/ydb/yoj/repository/test/inmemory/InMemoryTable.java b/repository-inmemory/src/main/java/tech/ydb/yoj/repository/test/inmemory/InMemoryTable.java index 3953a331..bf08e46f 100644 --- a/repository-inmemory/src/main/java/tech/ydb/yoj/repository/test/inmemory/InMemoryTable.java +++ b/repository-inmemory/src/main/java/tech/ydb/yoj/repository/test/inmemory/InMemoryTable.java @@ -175,8 +175,8 @@ public TableDescriptor getTableDescriptor() { @Override public T find(Entity.Id id) { - if (id.isPartial()) { - throw new IllegalArgumentException("Cannot use partial id in find method"); + if (TableQueryImpl.isPartialId(id, schema)) { + throw new IllegalArgumentException("Cannot use partial ID in Table.find(ID) method"); } return transaction.getTransactionLocal().firstLevelCache(tableDescriptor).get(id, __ -> { markKeyRead(id); @@ -192,8 +192,8 @@ public > List find(Set ids) { @Override public V find(Class viewType, Entity.Id id) { - if (id.isPartial()) { - throw new IllegalArgumentException("Cannot use partial id in find method"); + if (TableQueryImpl.isPartialId(id, schema)) { + throw new IllegalArgumentException("Cannot use partial ID in Table.find(Class, ID) method"); } FirstLevelCache cache = transaction.getTransactionLocal().firstLevelCache(tableDescriptor); diff --git a/repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/table/YdbTable.java b/repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/table/YdbTable.java index 494f1908..252957b8 100644 --- a/repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/table/YdbTable.java +++ b/repository-ydb-v2/src/main/java/tech/ydb/yoj/repository/ydb/table/YdbTable.java @@ -4,6 +4,7 @@ import com.google.common.collect.Sets; import com.google.common.reflect.TypeToken; import lombok.NonNull; +import tech.ydb.yoj.DeprecationWarnings; import tech.ydb.yoj.InternalApi; import tech.ydb.yoj.databind.expression.FilterExpression; import tech.ydb.yoj.databind.expression.OrderExpression; @@ -77,11 +78,20 @@ public YdbTable(Class type, QueryExecutor executor) { this.tableDescriptor = TableDescriptor.from(schema); } + /** + * @deprecated This {@code YdbTable} constructor uses reflection tricks to determine table entity type. + * It will be removed in YOJ 2.7.0. + */ + @Deprecated(forRemoval = true) protected YdbTable(QueryExecutor executor) { this.type = resolveEntityType(); this.executor = new CheckingQueryExecutor(executor); this.schema = EntitySchema.of(type); this.tableDescriptor = TableDescriptor.from(schema); + + DeprecationWarnings.warnOnce("new YdbTable(QueryExecutor)[" + type.getName() + "]", + "new YdbTable(QueryExecutor) constructor will be removed in YOJ 2.7.0. Please use the new YdbTable(TableDescriptor, QueryExecutor) " + + "or the new YdbTable(TableDescriptor, QueryExecutor) constructor instead"); } public YdbTable(TableDescriptor tableDescriptor, QueryExecutor executor) { @@ -110,6 +120,7 @@ protected final QueryExecutor getExecutor() { return executor; } + @Deprecated(forRemoval = true) @SuppressWarnings("unchecked") private Class resolveEntityType() { return (Class) (new TypeToken(getClass()) { @@ -261,7 +272,7 @@ private Stream readTableStream(ReadTableMapper mapper, ReadTable @Override public T find(Entity.Id id) { - if (id.isPartial()) { + if (TableQueryImpl.isPartialId(id, schema)) { throw new IllegalArgumentException("Cannot use partial id in find method"); } return executor.getTransactionLocal().firstLevelCache(tableDescriptor).get(id, __ -> { @@ -372,7 +383,7 @@ public > List find( if (ids.isEmpty()) { return List.of(); } - var isPartialIdMode = ids.iterator().next().isPartial(); + var isPartialIdMode = TableQueryImpl.isPartialId(ids.iterator().next(), schema); List found = postLoad(findUncached(ids, filter, orderBy, limit)); if (!isPartialIdMode && ids.size() > found.size()) { Set> foundIds = found.stream().map(Entity::getId).collect(toSet()); diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/AbstractDelegatingTable.java b/repository/src/main/java/tech/ydb/yoj/repository/db/AbstractDelegatingTable.java index 2d8cdc8e..1f7e8e48 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/AbstractDelegatingTable.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/AbstractDelegatingTable.java @@ -3,6 +3,7 @@ import com.google.common.reflect.TypeToken; import lombok.AccessLevel; import lombok.Getter; +import tech.ydb.yoj.DeprecationWarnings; import tech.ydb.yoj.ExperimentalApi; import tech.ydb.yoj.databind.expression.FilterExpression; import tech.ydb.yoj.databind.expression.OrderExpression; @@ -23,15 +24,30 @@ protected AbstractDelegatingTable(Table target) { this.target = target; } + /** + * @deprecated This constructor uses reflection tricks to guess entity type for the table, + * and will be removed in YOJ 3.0.0. + * Please use {@link AbstractDelegatingTable#AbstractDelegatingTable(Class)} or + * {@link AbstractDelegatingTable#AbstractDelegatingTable(TableDescriptor)} instead. + */ + @Deprecated(forRemoval = true) protected AbstractDelegatingTable() { + DeprecationWarnings.warnOnce("new AbstractDelegatingTable()", + "Nullary AbstractDelegatingTable constructor will be removed in YOJ 3.0.0. " + + "Please use 1-arg Class or TableDescriptor constructor instead"); this.target = BaseDb.current(BaseDb.class).table(resolveEntityType()); } + protected AbstractDelegatingTable(Class entityType) { + this.target = BaseDb.current(BaseDb.class).table(entityType); + } + @ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/32") protected AbstractDelegatingTable(TableDescriptor tableDescriptor) { this.target = BaseDb.current(BaseDb.class).table(tableDescriptor); } + @Deprecated(forRemoval = true) @SuppressWarnings("unchecked") private Class resolveEntityType() { return (Class) (new TypeToken(getClass()) { diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/Entity.java b/repository/src/main/java/tech/ydb/yoj/repository/db/Entity.java index 4a8b4f61..e0566eba 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/Entity.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/Entity.java @@ -60,17 +60,31 @@ default E resolve( return Tx.Current.get().getRepositoryTransaction().table(getType()).find(this, throwIfAbsent); } + /** + * @deprecated This method will be removed in YOJ 3.0.0. Please stop using it. + */ @SuppressWarnings("unchecked") + @Deprecated(forRemoval = true) default Class getType() { + DeprecationWarnings.warnOnce( + "Entity.Id.getType()", + "You are using Entity.Id.getType() which will be removed in YOJ 3.0.0. Please stop using this method" + ); return (Class) new TypeToken(getClass()) { }.getRawType(); } + /** + * @deprecated This method will be removed in YOJ 3.0.0. Please use other ways to check if ID is partial + * (i.e., has some of its trailing components set to {@code null} to implicitly indicate an ID range.) + */ + @Deprecated(forRemoval = true) default boolean isPartial() { - var schema = EntitySchema.of(getType()).getIdSchema(); - var columns = schema.flattenFields(); - var nonNullFields = schema.flatten(this); - return columns.size() > nonNullFields.size(); + DeprecationWarnings.warnOnce( + "Entity.Id.isPartial()", + "You are using Entity.Id.isPartial() which will be removed in YOJ 3.0.0. Please use other ways to check if ID represents a range" + ); + return TableQueryImpl.isPartialId(this, EntitySchema.of(getType())); } } } diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/TableQueryImpl.java b/repository/src/main/java/tech/ydb/yoj/repository/db/TableQueryImpl.java index 4f3ee950..80e93a2b 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/TableQueryImpl.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/TableQueryImpl.java @@ -33,7 +33,7 @@ public static , ID extends Entity.Id> List find( } var orderBy = EntityExpressions.defaultOrder(table.getType()); - var isPartialIdMode = ids.iterator().next().isPartial(); + var isPartialIdMode = TableQueryImpl.isPartialId(ids.iterator().next(), table.getSchema()); var foundInCache = ids.stream() .filter(cache::containsKey) @@ -82,4 +82,11 @@ public static > TableQueryBuilder toQueryBuilder(Table .offset(request.getOffset()) .limit(request.getPageSize() + 1); } + + public static , ID extends Entity.Id> boolean isPartialId(ID id, EntitySchema schema) { + var idSchema = schema.getIdSchema(); + var columns = idSchema.flattenFields(); + var nonNullFields = idSchema.flatten(id); + return columns.size() > nonNullFields.size(); + } } diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/cache/DbValueUpdater.java b/repository/src/main/java/tech/ydb/yoj/repository/db/cache/DbValueUpdater.java index 8f3c6bd8..7a1ceb1e 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/cache/DbValueUpdater.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/cache/DbValueUpdater.java @@ -8,6 +8,7 @@ import lombok.Value; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tech.ydb.yoj.DeprecationWarnings; import tech.ydb.yoj.repository.db.exception.QueryCancelledException; import tech.ydb.yoj.repository.db.exception.QueryInterruptedException; import tech.ydb.yoj.util.lang.Interrupts; @@ -87,11 +88,18 @@ public DbValueUpdater(@NonNull ThreadFactoryCreator threadFactorySupplier) { this(DEFAULT_CACHE_TIMEOUT, DEFAULT_SHUTDOWN_TIMEOUT, DEFAULT_MAX_LAG, DEFAULT_MAX_READ_DURATION, threadFactorySupplier); } + /** + * @deprecated This constructor uses reflection tricks to determine the name of the entity being updated. + * This constructor will be removed in YOJ 2.7.0. + */ + @Deprecated(forRemoval = true) public DbValueUpdater(@NonNull Duration pollInterval, @NonNull Duration shutdownTimeout, @NonNull Duration maxAge, @NonNull Duration maxReadDuration, @NonNull ThreadFactoryCreator threadFactorySupplier) { this(pollInterval, shutdownTimeout, maxAge, maxReadDuration, vu -> new TypeToken(vu.getClass()) { }.getRawType().getSimpleName(), threadFactorySupplier); + DeprecationWarnings.warnOnce("DbValueUpdater/TypeToken", + "DbValueUpdater constructor without explicit `name` will be removed in YOJ 2.7.0. Please use the constructor with explicit name"); } public DbValueUpdater(@NonNull String name, diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/projection/RwProjectionCache.java b/repository/src/main/java/tech/ydb/yoj/repository/db/projection/RwProjectionCache.java index 01012661..a17c74d1 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/projection/RwProjectionCache.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/projection/RwProjectionCache.java @@ -7,6 +7,7 @@ import tech.ydb.yoj.InternalApi; import tech.ydb.yoj.repository.db.Entity; import tech.ydb.yoj.repository.db.RepositoryTransaction; +import tech.ydb.yoj.repository.db.Table; import java.util.LinkedHashMap; import java.util.List; @@ -60,21 +61,28 @@ public void applyProjectionChanges(RepositoryTransaction transaction) { oldProjections.values().stream() .filter(e -> !newProjections.containsKey(e.getId())) - .forEach(e -> deleteEntity(transaction, e.getId())); + .forEach(e -> deleteEntity(transaction, e)); newProjections.values().stream() .filter(e -> !e.equals(oldProjections.get(e.getId()))) .forEach(e -> saveEntity(transaction, e)); } - private > void deleteEntity(RepositoryTransaction transaction, Entity.Id entityId) { - transaction.table(entityId.getType()).delete(entityId); + private > void deleteEntity(RepositoryTransaction transaction, Entity entity) { + table(transaction, entity).delete(entity.getId()); } private > void saveEntity(RepositoryTransaction transaction, Entity entity) { @SuppressWarnings("unchecked") T castedEntity = (T) entity; - transaction.table(entity.getId().getType()).save(castedEntity); + table(transaction, entity).save(castedEntity); + } + + private > Table table(RepositoryTransaction transaction, Entity entity) { + @SuppressWarnings("unchecked") + Class entityType = (Class) entity.getClass(); + + return transaction.table(entityType); } private Entity mergeOldProjections(Entity p1, Entity p2) { diff --git a/repository/src/test/java/tech/ydb/yoj/repository/db/PojoEntityTest.java b/repository/src/test/java/tech/ydb/yoj/repository/db/PojoEntityTest.java index 3704b7ca..a38ba5fb 100644 --- a/repository/src/test/java/tech/ydb/yoj/repository/db/PojoEntityTest.java +++ b/repository/src/test/java/tech/ydb/yoj/repository/db/PojoEntityTest.java @@ -8,7 +8,6 @@ import static org.junit.Assert.assertTrue; public class PojoEntityTest { - @Value private static class Ent implements Entity { @NonNull @@ -24,10 +23,11 @@ private static class Id implements Entity.Id { @Test public void testPartialId() { + var schema = EntitySchema.of(Ent.class); var completeId = new Ent.Id("a", "b"); var partialId = new Ent.Id("a", null); - assertTrue(partialId.isPartial()); - assertFalse(completeId.isPartial()); + assertTrue(TableQueryImpl.isPartialId(partialId, schema)); + assertFalse(TableQueryImpl.isPartialId(completeId, schema)); } } diff --git a/repository/src/test/java/tech/ydb/yoj/repository/db/RecordEntityTest.java b/repository/src/test/java/tech/ydb/yoj/repository/db/RecordEntityTest.java index 36a08417..237e3f1b 100644 --- a/repository/src/test/java/tech/ydb/yoj/repository/db/RecordEntityTest.java +++ b/repository/src/test/java/tech/ydb/yoj/repository/db/RecordEntityTest.java @@ -14,10 +14,11 @@ private record Id(String part1, String parts) implements Entity.Id