Deprecate node_reindex_node_search() and move reindexing logic to a hook

There is only 1 usage of this function according to http://codcontrib.hank.vps-private.net/search?text=node_reindex_node_sea... and that module doesn't have a release, looks unmaintained.

I don't think we need a replacement here, we'll just do all the work in a new Hooks file. Have updated the proposed solution.

Hey, in #4 I have done the following changes:

1. Deprecate node_reindex_node_search()
2. Move the logic into a new public method on a new NodeEntityHooks class
3. Move the following methods from NodeHooks1 into NodeEntityHooks: commentInsert, commentUpdate, commentDelete
4. Remove the call from Node.php, move into a #[Hook('node_update')] hook on NodeEntityHooks

Let me know if everything looks good or if any changes are needed, or if I’ve missed anything. I'm changing the status to "Needs Work" for now so the work can continue in case the pipeline fails or test cases need to be added.

Actioned feedback and fixed up linting.

Only thing I'm not sure on is if we should even mention the new replacement in the CR, it's in a Hooks class so it's a bit weird but it does technically mean there's a way to do it without us having to add yet another service.

I'm generally not very fond of modules implementing their own entity hooks but I think this is a good example for when it makes sense: Optional integration with another module, this allows us to encapsulate and group all related hooks for that in one service. And with #3524377: Allow to skip OOP hooks and services for modules that are not installed, we plan to have a means for dynamically registering this hooks service only when the search module is installed, something that for now would either require a node_search glue module or custom ServiceProvider code.

And then with this we also have a real use case in core that we can test it with.

I think mentioning the replacement in the CR is fine, personally I wouldn't really have bothered with the Why section. Realistically speaking, we plan to deprecate basically all runtime procedural code by deprecating .module files, doesn't really matter where it's called from.

The replacement doesn't seem right to me - asked a question on the MR. fwiw I'd also be fine with 'deprecated with no replacement'.

Agreed, also made the hooks method protected.

Committed/pushed to 11.x, thanks!