Fatal error when using search_api_solr view

Created on 27 February 2023, over 1 year ago
Updated 30 January 2024, 5 months ago

Problem/Motivation

When a search_api_solr view is used as the content source view for rest_oai_pmh, RestOaiPmhViewsCacheBase::processItem calls $view->getBaseEntityType()->id(), but with a search_api_solr view there is no base entity type, so $view->getBaseEntityType() returns false, and then chaining that to ->id() throws the fatal error.

Steps to reproduce

  1. Install search_api_solr; create and configure an index
  2. Create a view based on the search_api_solr view, add and configure an entity reference display.
  3. Configure your rest_oai_pmh endpoint to use that view:display
  4. Try to view anything on the rest_oai_pmh endpoint. You will get a fatal error.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States pdcarto

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @pdcarto
  • πŸ‡ΊπŸ‡ΈUnited States pdcarto

    Thinking about this further, I feel I must be missing something, or else there is some fundamental disconnect between views, search_api, search_api_solr, and how this module operates. I'm further confused since it seems that other issues seem to suggest that search_api_solr does work with this module. So I wonder mine is an edge case. Could it be that $view->getBaseEntityType only returns false if your search api index is configured to index multiple entity types (data sources)?

    That said, even if it's not usual, a given solr index can contain records for any number of entity types. Nodes, media, and files, for example. When you create a view from that search_api_solr index, nowhere are you required to specify an entity type, though you can filter by datasource. The datasource for a given record in the solr index may not correspond to an entity type at all, and the datasource can be different from one search api view result record to the next. So the assumption in $this->member_entity_type = $view->getBaseEntityType()->id(); seems wrong to me.

    On the other hand, the $id for search_api views result row here (example: "entity:node/109229:en") provides all the information needed to identify the entity if it exists. So if we must have a loaded entity in \Drupal\rest_oai_pmh\Plugin\QueueWorker\RestOaiPmhViewsCacheBase::indexViewRecords, we have the information needed to get it if $view->getBaseEntityType() returns false.

    I'll be testing some code to do this.

  • @pdcarto opened merge request.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States pdcarto
  • πŸ‡ΊπŸ‡ΈUnited States pdcarto

    MR created.

    The main points:

    • In \Drupal\rest_oai_pmh\Plugin\QueueWorker\RestOaiPmhViewsCacheBase::processItem, it checks to see if $view->getBaseEntityType() returns non-empty before it tries to get the id. If it returns false, it does not try to fill in $this->member_entity_type or $this->member_entity_storage
    • In \Drupal\rest_oai_pmh\Plugin\QueueWorker\RestOaiPmhViewsCacheBase::indexViewRecords, it checks to see if $this->member_entity_type is empty.
    • If so it looks to see if the member $id matches the view result identifier pattern used by search_api views: ^entity:(?<entity_type>[a-z\-_]+)\/(?<entity_id>[a-z0-9]+):(?<entity_language>[a-z]+)$
    • If the $id matches that pattern, then it sets the entity type and entity id from that string, and then proceeds to try to load the entity storage entity and then the entity.
    • If the pattern does not match, then the entity is not loaded, causing the rest of the loop to be skipped and move on to the next member. Should we do more here? E.g. Log the skipped item?
    • This last step is inside a try/catch. If there is an error (e.g. the entity storage entity cannot be loaded), it logs an error and leaves the entity undefined, thereby skipping this record and moving on to the next member. I'm not sure if this is the desired behavior.
    • If $this->member_entity_type is not empty, then it proceeds as normal, loading the entity storage and then the entity.
  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States joecorall

    Thanks for the MR. I merged it into the dev version. I am going to test this for the known node case. The code was designed to possibly support entity types other that nodes, but to my knowledge that use case never presented itself until what looks like now. So the other entity type support with solr was more theoretical. I am going to make sure this doesn't cause any regressions then cut a new release.

  • Status changed to Fixed over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States joecorall

    LGTM πŸ‘

  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States joecorall

    Thanks @pdcarto. Given these changes haven't been tested, I reverted the MR. Happy to merge changes for your use case if another MR is submitted.

  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΉπŸ‡³Tunisia adel-by

    Hi,
    Following the suggestion in #10, here's another MR that adds support to search_api views.
    It also makes sure that the sets feature work properly and introduces a hook that allows other modules to alter the possible contextual filter ids.
    I'm using this module on media entities BTW and it works just fine.

  • πŸ‡ΊπŸ‡ΈUnited States pdcarto

    Thanks for picking this up adel-by β†’ !

    I have tested merge request !10 with a search api solr view with entity reference display, and I get good OAI results without errors.

    I also tested with a standard drupal content view with entity reference display, and likewise saw expected results and did not see any errors.

    I have not tested using a non-solr search_api indexed view. It seems to me that if someone can do that test, then we will have covered all the likely places where this MR could introduce a problem.

  • πŸ‡ΉπŸ‡³Tunisia adel-by

    The patch in #14 no longer applies, here is a re-roll.
    BTW we are using this patch on a production site for several months now and no issues were reported.

Production build 0.69.0 2024