Skip to content

OAK-12125: throw IllegalStateException for property index missing propertyNames#2784

Open
tmvlad wants to merge 1 commit intoapache:trunkfrom
tmvlad:OAK-12125
Open

OAK-12125: throw IllegalStateException for property index missing propertyNames#2784
tmvlad wants to merge 1 commit intoapache:trunkfrom
tmvlad:OAK-12125

Conversation

@tmvlad
Copy link
Copy Markdown

@tmvlad tmvlad commented Mar 6, 2026

  • A property index definition with `type=property` but missing the required `propertyNames` property caused a `NullPointerException` in `PropertyIndexEditor` that was not caught by `IndexUpdate`, breaking the entire async indexing cycle for all indexes
  • Fix: add a null check in `PropertyIndexEditor` constructor and throw an `IllegalStateException` with a clear message; `IndexUpdate` already catches `IllegalStateException` to gracefully skip bad index definitions and continue

Links

…pertyNames

A property index definition without the required 'propertyNames' property
caused a NullPointerException in PropertyIndexEditor that was not caught by
IndexUpdate, breaking the entire async indexing cycle for all indexes.

Fix: add a null check in PropertyIndexEditor constructor and throw an
IllegalStateException with a clear message. IndexUpdate already catches
IllegalStateException to gracefully skip bad index definitions and continue.
PropertyState names = definition.getProperty(PROPERTY_NAMES);
if (names.count() == 1) {
if (names == null) {
throw new IllegalStateException("Missing required property '" + PROPERTY_NAMES + "' in property index definition");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So instead of a NPE we now get an IllegalStateException... It might currently resolve the problem, but I think it is not a good solution, because still an exception is thrown. There is a performance overhead in throwing an exception, but more importantly exception handing is not part of the API, and so changes in the future might break this logic. Also, there is no logging.

An alternative solution would be:

  • Use a default value if no propertyNames are set, eg. use the value "propertyNamesIsMissing". A property name that is so unlikely to occur in practice that there are no repositories with this property. (And if there are, then actually it wouldn't be a problem.)
  • Log a warning, but only once. Only once such that the log file is not filled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants