EZP-24212: Translations are not deleted from Solr index

[crevillo]

Fixes https://jira.ez.no/browse/EZP-24212

See the issue in jira for detailed info. Basically the thing is when you delete a translation from the admin interface, that translation remains in the solr index.

The fix i'm proposing is merely a workaround to solve this problem, but to me this probably needs some re-thinking. Actually we are defining when we need to remove the object from the index when update a content at search engine level. But actually we can see that some operations may need removal and some others not.

So, i'm thinking if we should move this needRemoveWithUpdate logic to somewhere else and let the operations decide when is needed to delete before update or not

Thoughts?

[crevillo]

ping @yannickroger @paulborgermans @andrerom @bdunogier

[yannickroger]

Isn't it another limitation that needs to be to be taken care of with a cronjob (for performance issues). like the case we have in : https://doc.ez.no/Extensions/eZ-Publish-extensions/eZ-Find/eZ-Find-LS-5.2.0/Basic-Configuration/Updating-the-search-index

[crevillo]

@yannickroger i don't think so. imho leaving this to cron wouldn't fix this issue. or is delete of objects performer before the update in the ezfind cron?
that said, my patch indeed is not perfomance friendly because it will perform a delete operation in every update operation even there is no need to.
This is what i see that this check of needRemoveWithUpdate should be done in another way.

Actually we have it set to true in the provided ezsearchengine... https://github.com/ezsystems/ezpublish-legacy/blob/master/kernel/search/plugins/ezsearchengine/ezsearchengine.php#L39

[crevillo]

gentle ping @yannickroger @andrerom @bdunogier @paulborgermans

[andrerom]

I'm not the one to review this @crevillo, @yannickroger and @paulborgermans are the main pings here.

[bdunogier]

Is this for translations of one content, or site languages in general ?

If languages in general, we'd do that when we remove a language, period. It's not a daily task, and I'd really really rather not do something on every update to cover a development / maintenance event.

For this, we could maybe use an ezpEvent instead ?
https://github.com/ezsystems/ezpublish-legacy/blob/master/kernel/content/translations.php#L103

If it's about a content's languages, it's not as bad, but...

[crevillo]

@bdunogier it is for translation in one content. is going for translations tab of content view in the admin interface and delete one (or several) of it.

i agree that changing that needRemoveWithUpdate to true is not the best option. but on the other said i truly believe that when a translation is deleted solr index should note. Not a daily task but not a daily task to delete a published content, don't you think?

So, my proposal here is to have operations that doesn't need that remove with the update and some others that true for that i will (maybe) go for redifining the registerSearchObject function, something like...
static public function registerSearchObject( $objectID, $version = null, $isMoved = false, $needRemoveWithUpdate = false )

Then, change that if ( eZSearch::needRemoveWithUpdate() ) for if ( $needRemoveWithUpdate ) and finally change https://github.com/ezsystems/ezpublish-legacy/blob/master/kernel/content/ezcontentoperationcollection.php#L1341 to something like
eZContentOperationCollection::registerSearchObject( $objectID, null, false, true );

this way we could decide if we need to remove before update per operation and not per search engine...

[bdunogier]

One more of those... we should have added an options array ;-)

The approach sounds sane.

[crevillo]

what bothers me is in order to keep bc, as said, provided ezsearchengine has that needRemoveWithUpdate set to true https://github.com/ezsystems/ezpublish-legacy/blob/master/kernel/search/plugins/ezsearchengine/ezsearchengine.php#L39

so, could we go for something like if ( eZSearch::needRemoveWithUpdate() || $needRemoveWithUpdate )? or a bit dirty?