#157: Deprecate reflection-heavy EntityIdSchema.SORT_ENTITY_BY_ID in favor of EntitySchema.defaultOrder() Comparator

nvamelichev · nvamelichev · commit 6610aab60e73 · 2025-08-04T23:17:04.000+02:00
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
@@ -110,7 +110,7 @@ public List<T> find(
             @Nullable Long offset
     ) {
         // NOTE: InMemoryTable doesn't handle index.
-        return InMemoryQueries.find(() -> findAll().stream(), filter, orderBy, limit, offset);
+        return TableQueryImpl.find(() -> findAll().stream(), schema, filter, orderBy, limit, offset);
     }
 
     @Override
@@ -192,7 +192,7 @@ public T find(Entity.Id<T> id) {
 
     @Override
     public <ID extends Entity.Id<T>> List<T> find(Set<ID> ids) {
-        return TableQueryImpl.find(this, getFirstLevelCache(), ids);
+        return TableQueryImpl.find(this, schema, getFirstLevelCache(), ids);
     }
 
     @Override
@@ -218,7 +218,7 @@ public <ID extends Entity.Id<T>> List<T> find(Range<ID> range) {
         transaction.getWatcher().markRangeRead(tableDescriptor, range);
         return findAll0().stream()
                 .filter(e -> range.contains((ID) e.getId()))
-                .sorted(EntityIdSchema.SORT_ENTITY_BY_ID)
+                .sorted(schema.defaultOrder())
                 .collect(toList());
     }
 
@@ -239,7 +239,7 @@ public <V extends View, ID extends Entity.Id<T>> List<V> find(Class<V> viewType,
 
     @Override
     public <V extends View, ID extends Entity.Id<T>> List<V> find(Class<V> viewType, Set<ID> ids) {
-        return find(viewType, ids, null, EntityExpressions.defaultOrder(getType()), null);
+        return find(viewType, ids, null, EntityExpressions.defaultOrder(schema), null);
     }
 
     @Override
@@ -542,7 +542,7 @@ private <ID extends Entity.Id<T>> Stream<T> readTableStream(ReadTableParams<ID>
                 .stream()
                 .filter(e -> readTableFilter(e, params));
         if (params.isOrdered()) {
-            stream = stream.sorted(EntityIdSchema.SORT_ENTITY_BY_ID);
+            stream = stream.sorted(schema.defaultOrder());
         }
         if (params.getRowLimit() > 0) {
             stream = stream.limit(params.getRowLimit());
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
@@ -273,7 +273,7 @@ public T find(Entity.Id<T> id) {
 
     @Override
     public <ID extends Entity.Id<T>> List<T> find(Set<ID> ids) {
-        return TableQueryImpl.find(this, getFirstLevelCache(), ids);
+        return TableQueryImpl.find(this, schema, getFirstLevelCache(), ids);
     }
 
     @Override
diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/EntityIdSchema.java b/repository/src/main/java/tech/ydb/yoj/repository/db/EntityIdSchema.java
@@ -3,6 +3,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.reflect.TypeToken;
 import lombok.NonNull;
+import tech.ydb.yoj.DeprecationWarnings;
 import tech.ydb.yoj.databind.CustomValueTypes;
 import tech.ydb.yoj.databind.FieldValueType;
 import tech.ydb.yoj.databind.schema.Schema;
@@ -42,11 +43,33 @@ public final class EntityIdSchema<ID extends Entity.Id<?>> extends Schema<ID> im
     private static final String ID_SUBFIELD_PATH_PREFIX = ID_FIELD_NAME + PATH_DELIMITER;
     private static final String ID_SUBFIELD_NAME_PREFIX = ID_FIELD_NAME + NAME_DELIMITER;
 
+    /**
+     * @deprecated This constant will be removed in YOJ 2.7.0.
+     * <p>If you need a {@link Comparator} to manually sort entities by ID ascending,
+     * use {@link EntitySchema#defaultOrder()} if you have an entity schema,
+     * or {@code Comparator.comparing(Entity::getId, idSchema)} if you have an entity ID schema.
+     * <p>To obtain entity schema or entity ID schema, see {@code EntitySchema.of()},
+     * {@link EntitySchema#getIdSchema()} and {@code EntityIdSchema.{of, from, ofEntity}()}.
+     */
+    @Deprecated(forRemoval = true)
     public static final Comparator<Entity<?>> SORT_ENTITY_BY_ID = Comparator.comparing(
-            Entity::getId, (a, b) -> EntityIdSchema.ofEntity(a.getType()).compare(a, b)
+            Entity::getId, (a, b) -> {
+                DeprecationWarnings.warnOnce("EntityIdSchema[" + a.getClass().getName() + "].SORT_ENTITY_BY_ID",
+                        "EntityIdSchema.SORT_ENTITY_BY_ID constant will be removed in YOJ 2.7.0. "
+                                + "Please use EntitySchema.defaultOrder() if you have an entity schema, or "
+                                + "Comparator.comparing(Entity::getId, idSchema) if you have an entity ID schema.");
+                return EntityIdSchema.ofEntity(a.getType()).compare(a, b);
+            }
     );
 
+    /**
+     * @deprecated This method serves no useful purpose and will be removed in YOJ 2.7.0.
+     * <p>If you need a {@link Comparator} for entity IDs, use the {@code EntityIdSchema} instance as it already implements {@code Comparator<ID>}.
+     */
+    @Deprecated(forRemoval = true)
     public static <T extends Entity<T>> Comparator<Entity.Id<T>> getIdComparator(Class<T> type) {
+        DeprecationWarnings.warnOnce("EntityIdSchema.getIdComparator(" + type.getName() + ")",
+                "EntityIdSchema.getIdComparator(Class) method will be removed in YOJ 2.7.0. Please use the EntityIdSchema itself as the comparator");
         return Comparator.comparing(
                 id -> id, (a, b) -> EntityIdSchema.ofEntity(type).compare(a, b)
         );
diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/EntitySchema.java b/repository/src/main/java/tech/ydb/yoj/repository/db/EntitySchema.java
@@ -10,6 +10,7 @@
 import tech.ydb.yoj.databind.schema.reflect.Reflector;
 
 import java.lang.reflect.Type;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 
@@ -117,4 +118,12 @@ public Map<String, Object> flattenId(Entity.Id<T> idValue) {
     public <V extends Table.View> ViewSchema<V> getViewSchema(Class<V> viewClass) {
         return ViewSchema.of(getRegistry(), viewClass, getNamingStrategy());
     }
+
+    /**
+     * @return a comparator for sorting entities by ID ascending
+     * @since 2.6.24
+     */
+    public Comparator<T> defaultOrder() {
+        return Comparator.comparing(Entity::getId, getIdSchema());
+    }
 }
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
@@ -1,18 +1,26 @@
 package tech.ydb.yoj.repository.db;
 
 import com.google.common.collect.Sets;
+import lombok.NonNull;
 import tech.ydb.yoj.InternalApi;
+import tech.ydb.yoj.databind.expression.FilterExpression;
+import tech.ydb.yoj.databind.expression.OrderExpression;
 import tech.ydb.yoj.repository.db.cache.FirstLevelCache;
+import tech.ydb.yoj.repository.db.list.InMemoryQueries;
 import tech.ydb.yoj.repository.db.list.ListRequest;
+import tech.ydb.yoj.util.function.StreamSupplier;
 
+import javax.annotation.Nullable;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
+import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
 /**
@@ -25,14 +33,16 @@ public final class TableQueryImpl {
     private TableQueryImpl() {
     }
 
+    @NonNull
     public static <E extends Entity<E>, ID extends Entity.Id<E>> List<E> find(
-            Table<E> table, FirstLevelCache<E> cache, Set<ID> ids
+            @NonNull Table<E> table, @NonNull EntitySchema<E> schema, @NonNull FirstLevelCache<E> cache,
+            @NonNull Set<ID> ids
     ) {
         if (ids.isEmpty()) {
             return List.of();
         }
 
-        var orderBy = EntityExpressions.defaultOrder(table.getType());
+        var orderBy = EntityExpressions.defaultOrder(schema);
         var isPartialIdMode = ids.iterator().next().isPartial();
 
         var foundInCache = ids.stream()
@@ -71,15 +81,49 @@ public static <E extends Entity<E>, ID extends Entity.Id<E>> List<E> find(
             Sets.difference(Sets.difference(ids, foundInDbIds), foundInCacheIds).forEach(cache::putEmpty);
         }
 
-        return merged.values().stream().sorted(EntityIdSchema.SORT_ENTITY_BY_ID).collect(Collectors.toList());
+        return merged.values().stream()
+                .sorted(schema.defaultOrder())
+                .collect(toList());
     }
 
-    public static <E extends Entity<E>> TableQueryBuilder<E> toQueryBuilder(Table<E> table, ListRequest<E> request) {
+    @NonNull
+    public static <E extends Entity<E>> TableQueryBuilder<E> toQueryBuilder(@NonNull Table<E> table, @NonNull ListRequest<E> request) {
         return table.query()
                 .index(request.getIndex())
                 .filter(request.getFilter())
                 .orderBy(request.getOrderBy())
                 .offset(request.getOffset())
                 .limit(request.getPageSize() + 1);
     }
+
+    public static <T extends Entity<T>> List<T> find(@NonNull StreamSupplier<T> streamSupplier,
+                                                     @NonNull EntitySchema<T> schema,
+                                                     @Nullable FilterExpression<T> filter,
+                                                     @Nullable OrderExpression<T> orderBy,
+                                                     @Nullable Integer limit,
+                                                     @Nullable Long offset) {
+        if (limit == null && offset != null && offset > 0) {
+            throw new IllegalArgumentException("offset > 0 with limit=null is not supported");
+        }
+
+        try (Stream<T> stream = streamSupplier.stream()) {
+            Stream<T> foundStream = stream;
+            if (filter != null) {
+                foundStream = foundStream.filter(InMemoryQueries.toPredicate(filter));
+            }
+            if (orderBy != null) {
+                foundStream = foundStream.sorted(InMemoryQueries.toComparator(orderBy));
+            } else {
+                foundStream = foundStream.sorted(schema.defaultOrder());
+            }
+
+            foundStream = foundStream.skip(offset == null ? 0L : offset);
+
+            if (limit != null) {
+                foundStream = foundStream.limit(limit);
+            }
+
+            return foundStream.collect(toList());
+        }
+    }
 }
diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/list/InMemoryQueries.java b/repository/src/main/java/tech/ydb/yoj/repository/db/list/InMemoryQueries.java
@@ -13,6 +13,9 @@
 import tech.ydb.yoj.databind.schema.Schema;
 import tech.ydb.yoj.databind.schema.Schema.JavaField;
 import tech.ydb.yoj.repository.db.Entity;
+import tech.ydb.yoj.repository.db.EntityIdSchema;
+import tech.ydb.yoj.repository.db.EntitySchema;
+import tech.ydb.yoj.repository.db.TableQueryImpl;
 import tech.ydb.yoj.util.function.StreamSupplier;
 
 import javax.annotation.Nullable;
@@ -29,18 +32,26 @@
 import static java.util.function.Predicate.not;
 import static java.util.stream.Collectors.toList;
 import static tech.ydb.yoj.databind.expression.OrderExpression.SortOrder.ASCENDING;
-import static tech.ydb.yoj.repository.db.EntityIdSchema.SORT_ENTITY_BY_ID;
 
 /**
  * Utilities for in-memory filtering and sorting objects that have a {@link Schema} (mostly {@link Entity entities}).
  */
 public final class InMemoryQueries {
+    @Deprecated(forRemoval = true)
+    private static final Comparator<Entity<?>> ENTITY_DEFAULT_ORDER_VIA_REFLECTION = Comparator.comparing(
+            Entity::getId, (a, b) -> EntityIdSchema.ofEntity(a.getType()).compare(a, b)
+    );
+
     public static <T extends Entity<T>> ListResult<T> list(@NonNull StreamSupplier<T> streamSupplier,
                                                            @NonNull ListRequest<T> request) {
+        if (!(request.getSchema() instanceof EntitySchema<T> entitySchema)) {
+            throw new IllegalArgumentException("Expected ListRequest for an entity, but got a non-entity schema: " + request.getSchema());
+        }
         return ListResult.forPage(
                 request,
-                find(
+                TableQueryImpl.find(
                         streamSupplier,
+                        entitySchema,
                         request.getFilter(),
                         request.getOrderBy(),
                         request.getPageSize() + 1,
@@ -49,6 +60,10 @@ public static <T extends Entity<T>> ListResult<T> list(@NonNull StreamSupplier<T
         );
     }
 
+    /**
+     * @deprecated This method is an implementation detail leaked into the public YOJ API. It will be removed in YOJ 2.7.0.
+     */
+    @Deprecated(forRemoval = true)
     public static <T extends Entity<T>> List<T> find(@NonNull StreamSupplier<T> streamSupplier,
                                                      @Nullable FilterExpression<T> filter,
                                                      @Nullable OrderExpression<T> orderBy,
@@ -66,7 +81,7 @@ public static <T extends Entity<T>> List<T> find(@NonNull StreamSupplier<T> stre
             if (orderBy != null) {
                 foundStream = foundStream.sorted(toComparator(orderBy));
             } else {
-                foundStream = foundStream.sorted(SORT_ENTITY_BY_ID);
+                foundStream = foundStream.sorted(ENTITY_DEFAULT_ORDER_VIA_REFLECTION);
             }
 
             foundStream = foundStream.skip(offset == null ? 0L : offset);

Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,7 @@ public T find(Entity.Id<T> id) {`
`273`	`273`
`274`	`274`	`@Override`
`275`	`275`	`public <ID extends Entity.Id<T>> List<T> find(Set<ID> ids) {`
`276`		`- return TableQueryImpl.find(this, getFirstLevelCache(), ids);`
	`276`	`+ return TableQueryImpl.find(this, schema, getFirstLevelCache(), ids);`
`277`	`277`	`}`
`278`	`278`
`279`	`279`	`@Override`