SearchVariantSubsites alterDefinition issue

[timkung]

In the case where the silverstripe/subsites module is installed and there are multiple classes being indexed, e.g. both SiteTree and File, the alterDefinition method (and then ultimately the SolrIndex->_addAs() method) needs to be able to handle having the "_subsite" field added to multiple classes (in the case example mentioned above, both SiteTree and File).

The issue here is when Solr_Configure is called, it calls the SearchVariantSubsites->alterDefinition method for each of the classes, but each time it just overwrites the "base" and "origin" key to the class called, so only the last class added to the SolrIndex gets the "_subsite" field applied to it, e.g. If we add the "SiteTree" class to the SolrIndex and then the "File" class to the SolrIndex, only the "File" class ends up with the "_subsite" field (because of SolrIndex->_addAs() line 519 - if ($field['base'] == $base) {)

A couple of initial thoughts would be to either allow the $field['base'] value to be an array or it needs to set class specific fields for subsites (rather than the global "_subsite" field), e.g. go through the classes in the index and add {$Class}_SubsiteID to the respective Solr documents.

[tractorcow]

How about having a separate field for each class?

I.e. instead of $index->filterFields['_subsite'] = array( maybe $index->filterFields[$base . '_subsite'] = array( in SearchVariantSubsites?

[tractorcow]

On second thought, that would probably break a huge amount of things. :) Maybe the array-base is better, and we just make sure to merge any existing config for subsequent calls to alterDefinition.

This doesn't seem like an easy problem to solve...

[timkung]

Yes, definitely one that will require some decent planning =)

[tractorcow]

We're fixing this now. :D Internal issue tracking: https://silverstripe.atlassian.net/browse/OSS-1737

[tractorcow]

I can confirm this doesn't just affect subsites, but also versioned.

[tractorcow]

I've tested a fix locally that seems to do the job... tractorcow@401afc5

Managed to do this without breaking changes. :)

Note that there is still only ONE field for all objects, but when adding objects multiple types may write to it. That way existing code that filters on that field won't break.

[madmatt]

So this is fixed by still having a single _subsite field for example, which still only accepts a single int value - and the value might be overwritten by two different variants?

Makes sense for SearchVariantVersioned and SearchVariantSubsites, each document will only have one value for _subsite or _versionedstage.

[dhensby]

@tractorcow so is this fixed?

[tractorcow]

Yep seems to have been merged in a while back https://github.com/silverstripe/silverstripe-fulltextsearch/blob/master/src/Search/Variants/SearchVariant.php