Handle potential null on Index::getServerInstance() in Index::postSave()

Shouldn’t the server be set to null instead of ''? The former means “no server”, the latter looks to most code like a server with ID ''.
In your setup, there also seems to be something else going wrong as an index without server cannot be enabled.

If you try to import invalid configuration it is clear that errors will occur, I wouldn’t say that is a bug in the code.

Shouldn’t the server be set to null instead of ''? The former means “no server”, the latter looks to most code like a server with ID ''.

Good question, I was also thinking about that, but the configuration that I have tried to import via Recipe was created by selecting " - No server - " in a search index and that lead to server: '' in the config instead of server: null.

Please also note that my proposed fix also already occurs in the postSave() method, so my theory was that it was just forgotten to be added to this specific place as a guarding condition.

      elseif (!$this->status() && $original->status()) {
        if ($this->hasValidTracker()) {
          $index_task_manager->stopTracking($original);
        }
        if ($original->isServerEnabled()) {
          $original->getServerInstance()->removeIndex($original);
        }
      }

I tested what happens when an index’s server assignment changes, and wanted to clarify the resulting config values:

- If an index is explicitly set to “- No server -” in the UI, the config is saved as server: ''.
- If the server that an index is assigned to is deleted, the index entity ends up with server: null.
--- If you then edit and re-save such an index (now with no server) and again select “- No server -”, the value becomes server: ''.

Huh, you’re right, good catch! Then I think that is the real bug here, the problem in Index::postSave() is just a symptom. “No server” should definitely be represented as null, not ''.
I think we should make sure an index is never saved with '' as the server ID and probably also provide an update hook that sanitizes existing indexes.

Would it be OK with you if we converted this issue accordingly, or would you prefer a new issue and would be in favor of still adding this check (which I’d argue against, but am prepared to be convinced/outvoted)?