PluginNotFoundException exception in Views when using an index with multiple entity types

Thanks a lot for reporting this problem, and already investigating thoroughly!
Trying this myself, I can indeed reproduce the problem: creating a new view based on a search index results in an uncaught exception.

Seems we introduced this problem in #2826822: The "text" plugin does not exist., by changing the property type of the "Rendered item" field added by the processor of the same name. The problem is that our Views integration code treats everything with field_item: prefix as an entity field, without checking further. While we could arguably "just" change the "Rendered item" type back and solve the original problem in some other way, I think the better (more future-proof) solution is to check whether a Search API field is actually based on an entity field before assigning the entity field-specific field handler.

Patch attached, please test/review!
It changes the value passed in hook_search_api_views_field_handler_mapping_alter() slightly, so people implementing that hook would need to update it, but otherwise this should be fine and seems to fix the problem exactly (when I compare search_api_views_data() output before and after the patch). It also adds the correct sanitization for the field as a by-product.

That's related enough, I'd say. And a bit of a problem, I'd say – it seems we have no real way of specifying a processor-added property as "fulltext" content by default. All the mappings we currently have use a parent/child type combination, which we use internally but can't use for a real data definition (as we see here).

I think this is the proper fix (which we should probably have done right from the start): define our own data type(s) just for this purpose. Once at it, I went all the way and added two: one for normal text and one for HTML, so it's easily possible to differentiate between the two. (As we now do for our Views field data definitions.)

Please test/review! Just applying the patch and clearing the cache should be enough to make a view with a "Rendered item" field work fine.

The patch in #15 fixes this issue for me (I have only given cursory manual testing so far). Setting to needs review for testbot.

@ bmcclure: Thanks a lot for re-rolling! Don't know what happened there, my patch doesn't apply cleanly to any module version I can find.

+++ b/src/Plugin/views/field/SearchApiFieldTrait.php
@@ -438,7 +438,8 @@ trait SearchApiFieldTrait {
-                    $typed_data = \Drupal::service('typed_data_manager')
+                    $typed_data = \Drupal::getContainer()
+                      ->get('typed_data_manager')

I don't understand this change. Let's not do this. \Drupal::service and \Drupal::getContainer()->get() suck just as hard. With the first being shorter.

This removes the change in SearchApiFieldTrait as per #18.

When applying the patch and clearing caches, when I add "Rendered Item" from the index to the view, I get a fatal error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "search_api_html" plugin does not exist. in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 52 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).
Drupal\Core\Plugin\DefaultPluginManager->getDefinition('search_api_html') (Line: 78)
Drupal\Core\TypedData\TypedDataManager->createInstance('search_api_html', Array) (Line: 104)
Drupal\Core\TypedData\TypedDataManager->create(Object, Object) (Line: 444)
Drupal\search_api\Plugin\views\field\SearchApiStandard->preRender(Array) (Line: 1490)
Drupal\views\ViewExecutable->render() (Line: 171)
Drupal\views\Plugin\views\display\Page->execute() (Line: 1617)
Drupal\views\ViewExecutable->executeDisplay('results', Array) (Line: 78)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func(Array, Array) (Line: 376)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 226)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 227)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 149)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 64)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 652)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I don't understand this change. Let's not do this. \Drupal::service and \Drupal::getContainer()->get() suck just as hard. With the first being shorter.

And the second leading to correct type hints in PhpStorm. I don't like it either, but I'd rather have type hinting than shorter code. (Should ultimately be fixed in PhpStorm, resp. the Symfony (or Drupal-Symfony bridge?) plugin, of course.)
That being said, the change doesn't really belong into this patch, so I'm fine with skipping it here.

Attached: a tentative try for a test for this. (Interdiff == tests-only.)

@ kevinquillen: That doesn't really seem possible, the patch adds exactly that plugin. Maybe try clearing the caches again? Or maybe you didn't apply the patch properly?

That didn't get far …

I'm very good at this.

Hallo Thomas,

I successfully applied your pacth in #27. Issue resolved for me.

Thanks!

Chris

Hello,

I applied #27 patch (2839932-27--rendered_item_views_field_handler.patch). This patch fix my issue.

Thanks,

@drunken monkey,

Did I miss anything or everyone's else set up is somewhat different?

#27 patch (2839932-27--rendered_item_views_field_handler.patch) does work for me to some point (though it doesn't apply cleanly on Search API 8.x-1.0-beta4, I adjusted it).

With the patch applied I was able to index successfully the FULL HTML output of the whole entity. I can see when the search is run, it returns the full output (with all of the markup - divs, comments, other elements - produced by Twig file) - SUCCESS !! .

The issue I am seeing, is when I try to display the RENDERED field in the View (Search API based view), because the value of the field is sanitized in class SearchApiStandard render_item() function, all of the markup of this HTML output is stripped by XSS filter (only tags remain in my case, if any.

  public function render_item($count, $item) {
    $type = !empty($this->definition['filter_type']) ? $this->definition['filter_type'] : 'plain';
      return $this->sanitizeValue($item['value'], $type);
  }

All divs, spans, article tags and others are gone).

If I skip sanitization in class SearchApiStandard render_item() function and change it to this:

  public function render_item($count, $item) {
    $type = !empty($this->definition['filter_type']) ? $this->definition['filter_type'] : 'plain';
    if ($this->realField == 'rendered_item') {
      return ViewsRenderPipelineMarkup::create($item['value']);
    }
    else {
      return $this->sanitizeValue($item['value'], $type);
    }
  }

it will print all of the markup as it should, with all of the data correctly. Otherwise - it's pretty much blank value.

Did anyone else see this issue ?

@veronicaSeveryn

Yes you are right, i had the same problem.

This patch is only for getting the correct theme for the entity to be rendered in while indexing.

The escaping of the html when displaying this field in a view is actually intentional.
It is discussed here:
https://www.drupal.org/node/2742027

In my case, I looks a bit different, but does the same.
With this I avoided modyfing the search_api module code directly.

// Somewhere in some Module..
function ao_utility_search_api_views_field_handler_mapping_alter(array &$mapping) {
  // Dont filter the HTML for our HTML fields, we need to display them like that.
  if ( !empty($mapping['search_api_html']) ) {
    $mapping['search_api_html']['id'] = 'search_api_my_html';
  }
}

and under [your_module]/src/Plugin/view/field
SearchApiMyHtml.php

namespace Drupal\search_api\Plugin\views\field;
namespace Drupal\ao_quicksearch\Plugin\views\field;

use Drupal\views\Plugin\views\field\FieldPluginBase;
use Drupal\views\Plugin\views\field\MultiItemsFieldHandlerInterface;
use Drupal\search_api\Plugin\views\field\SearchApiFieldTrait;

/**
 * Provides a default handler for fields in Search API Views.
 *
 * @ingroup views_field_handlers
 *
 * @ViewsField("search_api_my_html")
 */
class SearchApiMyHtml extends FieldPluginBase implements MultiItemsFieldHandlerInterface {

  use SearchApiFieldTrait;

  /**
   * {@inheritdoc}
   */
  public function render_item($count, $item) {
    return check_markup($item['value'], 'full_html');
  }

}

You probably can use your render_item Code here too :)

Thanks @vierlex, good tip !

Good to hear it works, thanks a lot for testing, everyone! And sorry for the long delay!
The attached should/might fix the tests, otherwise I need to investigate more. Would probably nice to get this in before RC.

Wow, did that really just get set back to "Needs work" because of coding standards problems? Awesome! (Could be communicated a bit more clearly, but still.)
This patch would fix those (though, I think, none of them was introduced by this patch), let's see if that helps.

Looks good.

Great, thanks for reviewing!
Committed.
Thanks again to everyone here!