Implement server tasks system

This definitely needs to be done, and tested.

Looking into this...

I did complete a part of the job. Remaining is getting the execute() method of the ServerTasks class working and executing it on cron run, before indexing items and before executing any action that could create tasks.

Attaching a functional but not fully tested patch. Missing are tests.

This patch also includes a port of the hook_cron implementation and a change to ServiceSpecificInterface to make the index parameter of the deleteAllItems method required to prevent accidental deletion of read only indexes.

Re-rolling...

Attached is a slightly improved version.
One thing still missing is calling execute() before calling any of the backend methods that could use exceptions, and failing if that returned FALSE.
And, of course, tests.

Here are some remarks about my changes:

1. +++ a/search_api.module
   @@ -7,10 +7,8 @@
   +use Drupal\search_api\Exception\SearchApiException;
   

   You forgot to import the exception class in some cases. But you're using an IDE, right? That should normally report such problems, maybe check your settings.

2. +++ a/src/Backend/BackendSpecificInterface.php
   @@ -210,14 +210,13 @@ interface BackendSpecificInterface {
   -   * @param \Drupal\search_api\Index\IndexInterface|null $index
   -   *   The index for which items should be deleted, or NULL to delete all items
   -   *   on this server.
   +   * @param \Drupal\search_api\Index\IndexInterface $index
   +   *   The index for which items should be deleted.
   

   When changing the parameters of a method, please remember to also update the doc!

3. +++ a/src/Tasks/ServerTasks.php
   @@ -24,13 +25,23 @@ class ServerTasks implements ServerTasksInterface {
   +   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
   +   *   The entity manager.
       */
   -  function __construct(Connection $database) {
   +  function __construct(Connection $database, EntityManagerInterface $entity_manager) {
   

   Since you want to load entities and are in a service, you should also dependency-inject the entity manager.

Also, it's good practice to run the tests before posting a patch (or, even more, committing, of course). That would probably have caught some of these.

Started to look into finishing this, but I'm going to post another cleanup first. The most important change is that we don't usually use plurals in class/service names, and the ServerTask class introduced here is often called a manager, even if its not a plugin manager.

"ServerTaskManager" makes sense, thanks!

Regarding test scenarios: Didn't Lukas already start work on this? I remember dicussing it with him, at least.

The idea is to give our test backend plugin a setting which makes it throw exceptions for all/some methods. Then set that, call the various methods on the server class that have task integration and see if tasks are created. See whether the tasks are properly executed when one of the methods is called (probably by giving the test backend also the functionality to store which methods are called on it, so we can check whether the right ones were called). Check that while executing pending tasks fails it's not possible to execute any new operation (i.e., they get added to the pending tasks automatically without calling the backend method first). Maybe also check that tasks for all servers are executed during cron runs, but that's optional.
Finally, unset the test backend plugin setting and check whether executing tasks correctly deletes them from the table, and whether calling the methods afterwards works normally.

Come to think of it, when a task in ServerTaskManager::execute() currently fails, do we still try to execute the others for that server? If so, that should be changed, tasks should only be executed in the order they were created in, and thus only if all previously saved tasks were successfully completed (or made redundant by other tasks, as sometimes happens).
Tasks for other servers can of course still be executed, if a task on one server fails.

+++ b/search_api_db/src/Plugin/SearchApi/Backend/SearchApiDbBackend.php
@@ -1048,38 +1048,16 @@ class SearchApiDbBackend extends BackendPluginBase {
-    catch (\Exception $e) {
       throw new SearchApiException($e->getMessage());

Since I'm just seeing this: this (and possible other instances of this pattern) should also pass $e as the third parameter when constructing the exception, so we have a proper chain.
(But should probably be done in another issue/commit, of course.)

Re-rolled and pushed to 2230925-server-tasks.

OK, let's do this!

Everything seems to work fine – committed.