From 5fc069a88963214b44ad4d2cff8b397e1b01a5ad Mon Sep 17 00:00:00 2001 From: Reed von Redwitz Date: Tue, 19 May 2026 10:02:40 +0200 Subject: [PATCH] fix(base-query): only register in objectByClass when type opts in via @Indexable --- .../base/query/AbstractHeapBasedIndex.java | 69 +++++++++++-------- .../base/query/IndexCompatibilityTests.java | 66 ++++++++++++++++++ 2 files changed, 105 insertions(+), 30 deletions(-) diff --git a/base-query/src/main/java/build/base/query/AbstractHeapBasedIndex.java b/base-query/src/main/java/build/base/query/AbstractHeapBasedIndex.java index f57d726..698814e 100644 --- a/base-query/src/main/java/build/base/query/AbstractHeapBasedIndex.java +++ b/base-query/src/main/java/build/base/query/AbstractHeapBasedIndex.java @@ -126,21 +126,11 @@ protected AbstractHeapBasedIndex() { public void index(final Object object) { final var objectClass = object.getClass(); - // always register the object in objectByClass — an @Indexable function field is sufficient - // to make the type a class-membership participant; type-level @Indexable remains accepted - // but is no longer required - this.objectByClass.compute(objectClass, (_, existing) -> { - final var objects = existing == null - ? ConcurrentHashMap.newKeySet() - : existing; - - objects.add(object); - return objects; - }); + final var nonUniqueFunctionFields = this.indexableFunctionFieldsByClass.compute(objectClass); + final var uniqueFunctionFields = this.uniqueIndexableFunctionFieldsByClass.compute(objectClass); // index the values produced by public static final @Indexable Function fields - this.indexableFunctionFieldsByClass - .compute(objectClass) + nonUniqueFunctionFields .forEach(field -> this.objectsByClassIndexableFunctionAndValue .compute(objectClass, (_, existingFunctions) -> { final var functions = existingFunctions == null @@ -191,8 +181,7 @@ public void index(final Object object) { })); // index the values produced by public static final @Indexable @Unique Function fields - this.uniqueIndexableFunctionFieldsByClass - .compute(objectClass) + uniqueFunctionFields .forEach(field -> this.uniqueObjectsByClassFunctionAndKey .compute(objectClass, (_, existingFunctions) -> { final var functions = existingFunctions == null @@ -236,27 +225,32 @@ public void index(final Object object) { return functions; })); + + // register in objectByClass after all function fields are successfully indexed — this ensures + // that a thrown exception during function indexing leaves objectByClass clean + if (Introspection.hasDeclaredAnnotation(objectClass, Indexable.class) + || !nonUniqueFunctionFields.isEmpty() + || !uniqueFunctionFields.isEmpty()) { + this.objectByClass.compute(objectClass, (_, existing) -> { + final var objects = existing == null + ? ConcurrentHashMap.newKeySet() + : existing; + + objects.add(object); + return objects; + }); + } } @Override public void unindex(final Object object) { final var objectClass = object.getClass(); - // always remove from objectByClass, symmetrical with index() - this.objectByClass.compute(objectClass, (_, existing) -> { - if (existing == null) { - return null; - } else { - existing.remove(object); - return existing.isEmpty() - ? null - : existing; - } - }); + final var nonUniqueFunctionFields = this.indexableFunctionFieldsByClass.compute(objectClass); + final var uniqueFunctionFields = this.uniqueIndexableFunctionFieldsByClass.compute(objectClass); // unindex the values produced by public static final @Indexable Function fields - this.indexableFunctionFieldsByClass - .compute(objectClass) + nonUniqueFunctionFields .forEach(field -> this.objectsByClassIndexableFunctionAndValue .compute(objectClass, (_, existingFunctions) -> { if (existingFunctions == null) { @@ -304,8 +298,7 @@ public void unindex(final Object object) { })); // unindex the values produced by public static final @Indexable @Unique Function fields - this.uniqueIndexableFunctionFieldsByClass - .compute(objectClass) + uniqueFunctionFields .forEach(field -> this.uniqueObjectsByClassFunctionAndKey .compute(objectClass, (_, existingFunctions) -> { if (existingFunctions == null) { @@ -332,6 +325,22 @@ public void unindex(final Object object) { return existingFunctions.isEmpty() ? null : existingFunctions; })); + + // remove from objectByClass after all function maps are cleaned up — symmetric with index() + if (Introspection.hasDeclaredAnnotation(objectClass, Indexable.class) + || !nonUniqueFunctionFields.isEmpty() + || !uniqueFunctionFields.isEmpty()) { + this.objectByClass.compute(objectClass, (_, existing) -> { + if (existing == null) { + return null; + } else { + existing.remove(object); + return existing.isEmpty() + ? null + : existing; + } + }); + } } diff --git a/base-query/src/test/java/build/base/query/IndexCompatibilityTests.java b/base-query/src/test/java/build/base/query/IndexCompatibilityTests.java index 3f03ce8..e8b6bd2 100644 --- a/base-query/src/test/java/build/base/query/IndexCompatibilityTests.java +++ b/base-query/src/test/java/build/base/query/IndexCompatibilityTests.java @@ -647,6 +647,65 @@ default void shouldNotIndexNonIndexableClass() { () -> index.index(new NotIndexable(Color.RED))); } + /** + * Ensure that when {@link Index#index} throws, the object is not registered in class-membership — i.e., + * {@code objectByClass} is left clean because it is populated only after function-map indexing succeeds. + */ + @Test + default void shouldNotRegisterInClassMembershipWhenIndexingFails() { + final var index = createIndex(); + final var object = new NotIndexable(Color.RED); + + assertThrows(UnsupportedOperationException.class, () -> index.index(object)); + + assertThat(index.match(NotIndexable.class) + .scope(Scope.Indexed) + .findAll() + .toList() + ).isEmpty(); + } + + /** + * Ensure that a class with no {@link Indexable} annotation and no {@link Indexable} function fields is + * not registered in class-membership when indexed — only the function-indexed maps are used. + * + *

Regression for an over-eager fix that added every indexed object to {@code objectByClass} regardless + * of whether the class opted in via {@link Indexable}. + */ + @Test + default void shouldNotRegisterInClassMembershipWhenNoIndexableFieldOrAnnotation() { + final var index = createIndex(); + + index.index(new Plain("hello")); + + assertThat(index.match(Plain.class) + .scope(Scope.Indexed) + .findAll() + .toList() + ).isEmpty(); + } + + /** + * Ensure that a class with an {@link Indexable} function field (but no type-level {@link Indexable} annotation) + * is registered in class-membership when indexed. + * + *

This is the complement of {@link #shouldNotRegisterInClassMembershipWhenNoIndexableFieldOrAnnotation}: + * a function-field {@link Indexable} is sufficient to opt the type in to class-membership queries. + */ + @Test + default void shouldRegisterInClassMembershipWhenFunctionFieldIsIndexable() { + final var index = createIndex(); + + final var colorful = new Colorful(Color.RED); + index.index(colorful); + + assertThat(index.match(Colorful.class) + .scope(Scope.Indexed) + .findAll() + .toList() + ).containsExactly(colorful); + } + // ---- reindexDynamic /** @@ -843,6 +902,13 @@ record NullFallbackColorful() { public static final Function COLOR = _ -> null; } + /** + * A plain class with no {@link Indexable} annotation and no {@link Indexable} function fields — used to verify + * that {@link Index#index} does not register such types in class-membership. + */ + record Plain(String value) { + } + /** * A simple {@code record} for testing purposes that is not indexable. *