Fix display plugin not being available right after a view or display is created

PHP Fatal error: Call to a member function setConjunction() on a non-object in /var/www/docroot/modules/contrib/search_api/src/Plugin/views/filter/SearchApiFulltext.php on line 214, referer: http://192.168.99.100:32781/solr-search/content

I can confirm the error. It seems to be introduced lately before the release by the commit of #2064637: Add a plugin system for query parsing

Thomas, do you an idea about what's going on here or an advice about something we need to change in the Solr backend?

i confirm the bug too
i tested with search_api_solr and search_api_db
it seems that every view based on an index throws the error

This appears not to be limited to Solr, as I am getting the error on a View created against a Content index with search_api_db backend.

Debugging now but I bet somebody else will patch it soon ;)

-S

See #2760953-8: Notice: Undefined index: roles in ...RenderedItem->addFieldValues() – clearing the cache should help to fix a site broken that way, and a patch like the one there should fix the module in general. Pretty stupid, but seems to be necessary. (I can explain in more detail, if someone wants to know.)

Actually I would be really interested in knowing more details about this issue. Does it appear randomly? or only once after you first enable the module?

Hello,

I encountered the same issue using search_api-8.x-1.0-alpha17 and search_api_solr-8.x-1.0-alpha6

In search_api/src/Plugin/views/query/SearchApiQuery.php line 204, I debugged

$display_plugin_manager = \Drupal::service('plugin.manager.search_api.display');
$test = $display_plugin_manager->getDefinitions();

$test was effectively empty.

In search_api/src/Plugin/search_api/display/ViewsPageDisplay.php, I changed the namespace just to test (I forgot why I thought there was a problem with the namespace...).

namespace Drupal\search_api\search_api\display;

instead of

namespace Drupal\search_api\Plugin\search_api\display;

I cleared the cache.

It does not work either.

I undid the change in the namespace and cleared the cache.

Now $test is not empty and the plugin derived from the view is detected and it works...

I haven't used plugin deriver yet, but I suspect that it is not triggered when creating a new view which can be problematic.

The attached patch should fix this problem.
I don't know anymore what I meant to say with #6. It's possible I posted this to the wrong issue, or linked the wrong issue.
The problem here is exactly as Grimreaper states: when you create/update/delete a view, the display plugin definitions won't be re-computed but loaded from cache, so the display plugin associated with the Views page cannot be found. (With #2772829: Make "search id" a query property instead of option, this would at least not crash the whole view anymore, but still lead to problems – for example, with facets.)

The attached patch should fix this: just implement the entity CRUD hooks for views in the Search API and clear the display plugin cache in them.

This is a good fix, I believe, not sure if we'd want to have tests for this either, but that would mean we'd have to create a view trough the UI or code in the test and that seems like a lot of trouble to get this right.

Tests are a good idea, and would be far less complicated than you thought: it seems between a Kernel test's setUp() method and the test methods, the cache isn't cleared automatically, so there we can easily check whether the display plugin is there.

Or, rather: "could". The static caching of derivatives in the deriver class, and of derivers in the discovery class, makes this impossible without a dirty hack like the one in the attached patch.
A much better solution would be if we were able to rebuild the container (or just our display plugin manager service), but I don't see any easy method to do this.
On the other hand, adding this method override just for the sake of a test is also a bit ugly. (Although it might also prevent problems in non-test edge cases, where display plugins are retrieved in the same page request as a view CRUD event.) In summary: I'm not sure.

Something else entirely, though: Should we maybe, before clearing the cache in the CRUD hook, check whether the view is even one of ours?

This would add the afore-mentioned check.

Every single time …

Tested this with alpha16 and the patch works for me.

Thanks a lot for testing and reviewing!
Committed.

This broke our tests, seems we need to re-visit.

The patch not only works for me for this issue, but it also fixes another cache-related issue due to which "rendered entity" are not shown as a display option #2771271: Add Views support for "users" similiar to rendered entity

Both test-only patches pass, and I don't think they should?

Oops, seems I accidentally added the tests-only patch a second time instead of adding the proper patch.
Also, as you say, it's of course not ideal if the tests-only patch passes. But I see the problem: if we don't fill the search displays cache first, the check for the display can't possibly use stale cache data.
The attached revision should now work.

KernelTest looks great. However, I just applied this locally and reran the failing test in #2799475: Support block and rest displays in the display deriver and this doesn't resolve that.

KernelTest looks great. However, I just applied this locally and reran the failing test in #2799475: Support block and rest displays in the display deriver and this doesn't resolve that.

I'd say that's a problem of the other issue.
And I think I know what that is: the plugin manager has both a static and a persistent cache. When you delete the Views display with a web request, the persistent cache will be flushed, but the static cache in the test case process can't be affected, so will continue to serve the stale data.
You'd either have to rebuild the plugin manager service (or complete container – no idea, though, how to do either of those), or manually call clearCachedDefinitions(), or delete the Views display programmatically instead of via a web request.

If that's something that should be fixed in the other issue - I feel this can be committed.

OK, great. Thanks for reviewing!
Committed.
Thanks again, everyone!

Thanks, that will save sitebuilders a lot of headache.