thomasmueller commented on code in PR #2784:
URL: https://github.com/apache/jackrabbit-oak/pull/2784#discussion_r3071405260
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java:
##########
@@ -132,7 +132,10 @@ public PropertyIndexEditor(NodeBuilder definition,
NodeState root,
// get property names
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");
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
