Cannot remove deleted un-versioned DataObject from Elastic index #1
Conversation
… for example when deleting un-versioned data objects.
adunn49
changed the title
… data object is available
| @@ -642,7 +693,7 @@ public function onAddToSearchIndexes(string $event): void | |||
|
|
|||
| $currentDataObject = $this->getDataObject(); | |||
|
|
|||
| $liveDataObject = DataObject::get($currentDataObject->ClassName)->byID($currentDataObject->ID); | |||
| $liveDataObject = $currentDataObject ? DataObject::get($currentDataObject->ClassName)->byID($currentDataObject->ID) : null; | |||
There was a problem hiding this comment.
hmm if your current object isn't LIVE - then this will put a DRAFT object into the $liveDataObject variable and allow non-live data objects to be added to the index
There was a problem hiding this comment.
These changes are to do with the failing test on the backport branch (see https://github.com/silverstripe/silverstripe-search-service/actions/runs/8748084849/job/24007554595?pr=93) where a data object hasn't been set, and probably more to do with testEvents unit test not setting the data object on the class.
With regards to this change, if $currentDataObject is null, then this sets $liveDataObject to null and the error is thrown. This seems to be doing as intended (and not setting draft objects onto $liveDataObject) but i'll add some further tests so that we are testing this code for unversioned, draft and published and no dataobject set scenarios. Also, perhaps in the case of no dataobject (though this is probably unlikely when adding documents) the code should throw a different error to avoid confusion:
$currentDataObject = $this->getDataObject();
if (!$currentDataObject) {
throw new IndexingServiceException('No data object set for indexing');
}
$liveDataObject = DataObject::get($currentDataObject->ClassName)->byID($currentDataObject->ID);
if (!$liveDataObject) {
// unlikely case as indexer calls 'shouldIndex' immediately prior to this
throw new IndexingServiceException('Only published DataObjects may be added to the index');
}
$this->setDataObject($liveDataObject);
There was a problem hiding this comment.
hmm yeah I didn't bring the tests over as it was a bit of effort to do so for the older branch. Ah yes looks like I read it too quickly and its not what I thought it was :)
Are you ok using this fork for now on your projects? I'll probably move these changes in to the latest v3 branch for future support
There was a problem hiding this comment.
in other words - will you be going to CMS5 soon?
There was a problem hiding this comment.
No, we won't be going over to CMS5 just yet, and for now will be using this fork.
| if (!$dataObject) { | ||
| throw new Exception(sprintf('DataObject %s : %s does not exist', $data['className'], $data['id'])); |
There was a problem hiding this comment.
could we keep the exception for things that don't have a fallback?
ishannz
force-pushed
the
bugfix/support-unversioned-dataobject-deletion
branch
from
92aaf2f
to
0c3f9d5
Compare
Description
When a data object or page is deleted from the CMS, a job is created (via the onAfterDelete method defined in the SearchServiceExtension) that should remove this item from the search index.
However, in the case of un-unversioned DataObjects this results in a broken job. This is because the unserialize method expects the DataObject to still exist but by then it has been deleted and no traces of it remain.
With this fix, we remove the need for the data object to exist by setting the 'identifier' and the ClassName on the DataObjectDocument when it is initiated. We then use these as a fallback when no DataObject is available.
Screenshot with fix