Uploaded image for project: 'Jackrabbit Oak'
  1. Jackrabbit Oak
  2. OAK-4636

PropertyIndexLookup#getIndexNode should be more tolerant towards property types

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • None
    • 1.0.33, 1.2.18, 1.4.6, 1.5.8, 1.6.0
    • query
    • None

    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.

      /cc tmueller, chetanm

      [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

      Attachments

        Activity

          People

            catholicon Vikas Saurabh
            catholicon Vikas Saurabh
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: