- 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 4:27pm 2 March 2023 - πΊπΈ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.
- In
- 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 12:34pm 4 March 2023 - Status changed to Needs work
over 1 year ago 8:57am 18 March 2023 - πΊπΈ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.
- Merge request !10Issue #3344748: Fatal error when using search_api_solr view β (Open) created by adel-by
- Status changed to Needs review
over 1 year ago 4:56pm 19 March 2023 - πΉπ³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.