Server and Index are Config Entities. That's a good design approach!
Drupal 8 itself defines a documented way for config overrides, see https://www.drupal.org/node/1928898
An absolutely valid pattern is to use such overrides for example to modify the connection URL of solr in a development or staging environment using settings.php or settings.local.php:
$config['search_api.server.my_solr_server'] = [
'backend_config' => [
'path' => '/solr/staging',
],
];
That works perfectly well for indexing and searching, but there's an important exception:
Drupal 8 introduces a configuration override system that maintains these overrides as temporary layers on top of the standard configuration values, does not use them for configuration forms ...
The message is that the overrides will not be applied if a config entity will be edited using a form. In other words if you use or derive from EntityForm.
Unfortunately Search API uses EntityForms to implement features that are not about editing an entity. The most critical one is ServerClearConfirmForm.
While the standard features like indexing and searching use the staging server in the override example above, a click on 'Delete all indexed data on this server' on the server status page triggers the ServerClearConfirmForm and deletes the production index!
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2684977.patch | 906 bytes | mkalkbrenner |
Comments
Comment #2
mkalkbrennerHere's a quick patch to prevent the accidental deletion of a random search index.
The good solution will be to not use EntityForms for features.
Comment #3
borisson_Setting to NR, to see what the tests think about this. Not sure how I feel about this fix yet.
Comment #4
drunken monkeyThanks for reporting this problem! You're right, that is indeed quite serious.
However, to me it seems the approach in #2682369: Fix problems with overridden config entities is more sensible and clean.
Would the patch there already solve this issue, too? Or don't the forms pick up that change in the loaded entity?
Comment #5
mkalkbrennerThere're too many issues in the queue ;-)
I searched through it but I didn't find that one.
Comment #6
mkalkbrennerSeems like the documentation about forms is miss-leading. Maybe the documented behavior of EntityForms is just a side effect of having them in the administration path :-(
Anyway, we need to ensure the right behavior that is in sync with any existing documentation:
Comment #7
mkalkbrennerJust want to confirm that
solves the issue as well.
So it is an option to extend the patch in #2682369: Fix problems with overridden config entities.
Nevertheless I'm interested in some more opinions here because we might hate to add a lot of route parameters. And in search_api_solr and search_api_solr_multilingual as well.
Comment #8
Alumei commentedI absolutely think that we only see this result because we are using an EntityForm on an admin-path for non-edit operations.
It seems that core only focuses on the edit use-case as with the current state as that one completely resembles the config-form intention:
Edit non-overriden values.
I think we should either keep using EntityForms and need to add request parameters OR in this case could use ConfirmFormBase instead, remember the config-entity id & load it in submitForm like @mkalkbrenner did in his initial patch.
I feel that using a regular form instead of an EntityForm with that logic would be cleaner as it does not circumvent the actual EntityForm logic.
Comment #9
Alumei commentedBTW if my thinking from #2682369: Fix problems with overridden config entities is correct, then 'Clear all indexed data' on the index-status page should clear the data on the correct server.
This is a result of the indexes clear() method loading the server configuration using the storage which results in an "overriden" config-entity being loaded.
The index data should be 'non-overriden' at that point though.
Comment #10
drunken monkeyLet's solve all of this in #2682369: Fix problems with overridden config entities.