silverstripe / silverstripe-search-service

A service-agnostic search module for Silverstripe CMS
BSD 3-Clause "New" or "Revised" License
5 stars 18 forks source link

Force live object when adding to the index #91

Closed blueo closed 7 months ago

blueo commented 7 months ago

This change tries to make a consistent way of ensuring a dataobject can only be 'Live' when adding to the index but may be in another state for when it needs to be removed. It does this by using the BEFORE_ADD event to re-fetch a dataobject in "live" mode. This means a non-live dataobject can be given to DataObjectDocument for use when removing or otherwise checking if an object should be indexed - however when adding to an index, it will always use a live object.

Key Changes

Notes

Initially I thought this would allow indexing of draft content. Queues are normally run via a "dev task" which is run in the DevelopmentAdmin context. This controller will set the reading mode to DRAFT. The upshot of this is that an Index job will unserialise a DataObject in DRAFT reading mode (saved as a Data Query param) and the subsequent toArray call could get draft content. This is often seen as a link indexed with ?stage=Stage parameters. However because shouldIndex was checking that it was the Live version - this content would always have been the same as the published object.

ssmarco commented 7 months ago

@blueo Since we got 2 PRs for this module, I wonder which one should go to the finish line first and that might mean the other one will have to rebase as I also used the tests/Fake/PageFake.php in my PR.

blueo commented 7 months ago

I think I'll update the title for this - after a bit of discussion with @ssmarco it looks like there is a better way to solve this. It looks as though the logic for passing a live object to the indexer should live with the DataObjectDocument but (unlike changing the _unserialize function) we need the context of if an object is being added or removed from an index. This is to accommodate removing deleted items - as they need to present the 'old' version of a data object where as when adding to the index, we want to ensure an item is live.

A possible fix is via the indexer eg

if ($document instanceof DataObjectDocumentInterface) {
                        // Making sure we get the Live version of the DataObject before indexing the document
                        Versioned::withVersionedMode(static function () use ($document): void {
                            Versioned::set_stage(Versioned::LIVE);
                            $dataObject = $document->getDataObject();
                            $liveDataObject = DataObject::get($dataObject->ClassName)->byID($dataObject->ID);
                            $document->setDataObject($liveDataObject);
                        });
                    }

could be added prior to adding a document in src/Service/Indexer.php

Another is to update the onAddToSearchIndexes function which is called prior to adding a document to the index.

ssmarco commented 7 months ago

@blueo Have retested this again manually and working as expected.