EDIT

This generally seems to work. One issue that is kind of unclear to me:

• is there a reason why you use the treeindex as key for TaxonNode in e.g.

private String cacheKey(TaxonNode taxonNode, String predicate) {
    return taxonNode.treeIndex() + predicate;
}

The treeindex is not necessarily as stable as the uuid. But I see that this guarantees that if the position of the node in the tree changes the cached value is not valid anymore which might be good. However, it is generally not clear if caching of taxon node related preferences works. The documentation of find(TaxonNode taxonNode, String predicate) says "Returns the best matching preference that matches the given predicate and the taxon node filter". That means if in the meanwhile a better matching preference was stored this preference is not correct anymore.
A solution could be to not cache this case like we do for "list(IPreferencePredicate<?> predicate)". Another solution could be to recompute the full cache or all taxon node related cache values once a taxon node related preference is stored.

Which solution to take also depends on how often the cache is currently used for these cases (I guess it is not often used and therefore we can choose solution 1)

I would suggest to add a separator in the cacheKey(key) method to clearly distinguish the subject and the predicate. Otherwise in rare cases it could happen that the computed key is maybe not unique. I did this with 5301f5b54d8 . Please revert if you think this is not correct.

Andreas Müller wrote:

This generally seems to work. One issue that is kind of unclear to me:

• is there a reason why you use the treeindex as key for TaxonNode in e.g.

private String cacheKey(TaxonNode taxonNode, String predicate) {
    return taxonNode.treeIndex() + predicate;
}

The treeindex is not necessarily as stable as the uuid. But I see that this guarantees that if the position of the node in the tree changes the cached value is not valid anymore which might be good. However, it is generally not clear if caching of taxon node related preferences works. The documentation of find(TaxonNode taxonNode, String predicate) says "Returns the best matching preference that matches the given predicate and the taxon node filter". That means if in the meanwhile a better matching preference was stored this preference is not correct anymore.

I implemented it in this way since the PreferenceDaoImpl.find(TaxonNode taxonNode, String predicate) also uses the taxon node index as key. But you are right there is a risk of missing recently added better matching preferences. BTW: It would be good to extend the documentation of this find method so that the strategy of finding matching pref becomes clear without reading the whole code. THe doc should also answer the question why the last split is omitted. Is this by purpose?

A solution could be to not cache this case like we do for "list(IPreferencePredicate<?> predicate)". Another solution could be to recompute the full cache or all taxon node related cache values once a taxon node related preference is stored.

Which solution to take also depends on how often the cache is currently used for these cases (I guess it is not often used and therefore we can choose solution 1)

I would also choose this solution for the same reason.

Andreas Kohlbecker wrote:

Andreas Müller wrote:

...

A solution could be to not cache this case like we do for "list(IPreferencePredicate<?> predicate)". Another solution could be to recompute the full cache or all taxon node related cache values once a taxon node related preference is stored.

Which solution to take also depends on how often the cache is currently used for these cases (I guess it is not often used and therefore we can choose solution 1)

I would also choose this solution for the same reason.

Done, see db9e9aab

Andreas Kohlbecker wrote:

Andreas Müller wrote:

The treeindex is not necessarily as stable as the uuid. But I see that this guarantees that if the position of the node in the tree changes the cached value is not valid anymore which might be good. However, it is generally not clear if caching of taxon node related preferences works. The documentation of find(TaxonNode taxonNode, String predicate) says "Returns the best matching preference that matches the given predicate and the taxon node filter". That means if in the meanwhile a better matching preference was stored this preference is not correct anymore.

I implemented it in this way since the PreferenceDaoImpl.find(TaxonNode taxonNode, String predicate) also uses the taxon node index as key. But you are right there is a risk of missing recently added better matching preferences. BTW: It would be good to extend the documentation of this find method so that the strategy of finding matching pref becomes clear without reading the whole code. THe doc should also answer the question why the last split is omitted. Is this by purpose?

Hmm, I can't really see what you mean by "last split omitted". Can you explain a bit more?

I implemented it in this way since the PreferenceDaoImpl.find(TaxonNode taxonNode, String predicate) also uses the taxon node index as key. But you are right there is a risk of missing recently added better matching preferences. BTW: It would be good to extend the documentation of this find method so that the strategy of finding matching pref becomes clear without reading the whole code.

I think I haven't done this yet because the final strategy is not yet fully clear. The current implementation is still a bit preliminary as you can see from the FIXME.

I added "The rules how to do this best is still under discussion (see FIXME in implementing code)" to the doc (not committed yet)