Skip to content

MINOR: improve generic type safety for IQv2 in metered-store layer#21683

Open
mjsax wants to merge 4 commits intoapache:trunkfrom
mjsax:minor-iq-type-safety-metered-store
Open

MINOR: improve generic type safety for IQv2 in metered-store layer#21683
mjsax wants to merge 4 commits intoapache:trunkfrom
mjsax:minor-iq-type-safety-metered-store

Conversation

@mjsax
Copy link
Member

@mjsax mjsax commented Mar 9, 2026

No description provided.

@mjsax mjsax added the streams label Mar 9, 2026
Copy link
Member Author

@mjsax mjsax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighted the code changes -- other stuff is improved code formatting only.


@AfterEach
public void tearDown() {
verify(inner).init(context, store);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally unrelated side cleanup -- but doesn't make any sense to have a verify in @AfterEach -- this should just be part of the test, which is empty atm and "misuses" the teatDown() method to actually test what it should test...


@AfterEach
public void tearDown() {
verify(inner).init(context, store);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

final Query<?> query,
public interface QueryHandler<R> {
QueryResult<R> apply(
final Query<R> query,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change -- If we get a Query with gives us result R, and we apply it, we should get a QueryResult<R> back -- And we thy the result type R to the QueryHandler interface.

Not using any types here, is unnecessarily loose I believe.


@SuppressWarnings("rawtypes")
private final Map<Class, QueryHandler> queryHandlers =
private final Map<Class<?>, QueryHandler<?>> queryHandlers =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we cannot have safe types, make it explicit using <?> (similar elsewhere)

}
} else {
result = (QueryResult<R>) handler.apply(
result = ((QueryHandler<R>) handler).apply(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That we have improved type check on the the QueryHandler, we need to cast the handler, and get the correct result type automatically.

This is still cleaner. The issue is with the queryHandlers Map that introduces the missing types, so the "problem" is not at the right place in the code.

(similar elsewhere in this PR)

@mjsax mjsax force-pushed the minor-iq-type-safety-metered-store branch from ebff123 to 26df811 Compare March 9, 2026 20:01
@mjsax mjsax force-pushed the minor-iq-type-safety-metered-store branch from 26df811 to 0ef1fb1 Compare March 9, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant