Details
Description
Currently, PropertyIndexLookup#getIndexNode [0] uses NodeState#getNames for propertyNames [1] and declaringNodeTypes [2]. That means that these properties need to be either of Name or Name[] types. While that's ok and probably useful as well - this can potentially be used for validation and javadoc for getNames says that the implementation is free to use an optimized path.
That being said, at least in one case (issue 768 in ensure oak index [3]) the values get set as String/String[] instead which can very easily render useful indices like nodetype useless for the running system.
I see 2 ways around it:
- PropertyIndexLookup can be more resilient and accept non-Name-type too. For optimal case, we can probably try getNames() first and then fallback to getProperties with potential to cast (and may be log a warning for mistake in property type)
- Proactively validate that properties such as declNodeTypes etc must have Name/Name[] during commit
I'm ok with any of the approach... but current scenario is too silent in failing... saving the change doesn't cause any issue... cost calculation while picking index simply ignore such indices.
[0]: https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L159
[1]: https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L171
[2]: https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L179
[3]: https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/768