Skip to content

fix(base-query): only register in objectByClass when type opts in via @Indexable #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deer merged 1 commit into from
May 19, 2026
Merged

fix(base-query): only register in objectByClass when type opts in via @Indexable #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions base-query/src/main/java/build/base/query/AbstractHeapBasedIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
});
}
}


Expand Down
66 changes: 66 additions & 0 deletions base-query/src/test/java/build/base/query/IndexCompatibilityTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <em>not</em> registered in class-membership when indexed — only the function-indexed maps are used.
*
* <p>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)
* <em>is</em> registered in class-membership when indexed.
*
* <p>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

/**
Expand Down Expand Up @@ -843,6 +902,13 @@ record NullFallbackColorful() {
public static final Function<NullFallbackColorful, Color> 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.
*
Expand Down