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