Problem/Motivation
It would be nice to use current Meilisearch versions.
Updated Meilisearch Server to v1.2.0, meilisearch/meilisearch-php to v1.2.1
Whenever I try to index fields that end in "id", the indexing process seems to succeed but no items get indexed.
Steps to reproduce
Add Meilisearch Backend
Add an index
Add any field with its system name ending in "id".
Start indexing.
No nodes will get indexed, although the process finishes successfully.
Only looking at the task info in Meilisearch reveals the problem:
xidel -e '$json' --method=get -H 'Authorization: Bearer THEKEY' -H 'Content-Type: application/json' 'http://localhost:7700/tasks/8509'
**** Retrieving (get): http://localhost:7700/tasks/8509 ****
**** Processing: http://localhost:7700/tasks/8509 ****
{
"uid": 8509,
"indexUid": "theindex",
"status": "failed",
"type": "documentAdditionOrUpdate",
"canceledBy": null,
"details": {
"receivedDocuments": 1,
"indexedDocuments": 0
},
"error": {
"message": "The primary key inference failed as the engine found 2 fields ending with `id` in their names: 'id' and 'field_project_id'. Please specify the primary key manually using the `primaryKey` query parameter.",
"code": "index_primary_key_multiple_candidates_found",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#index_primary_key_multiple_candidates_found"
},
"duration": "PT0.010622596S",
"enqueuedAt": "2023-06-07T16:09:54.253299545Z",
"startedAt": "2023-06-07T16:09:54.258113396Z",
"finishedAt": "2023-06-07T16:09:54.268735992Z"
}Proposed resolution
1. Make all errors of tasks visible to the user or at least log them.
2. Make indexing fail.
3a. Probably add an option in the field config (is this search_api's domain and not search_api_meilisearch's?) for manually selecting a field as primary key.
3b. As the entity-ID seems to be one of the fields ending in "id" it may be enough to just explicity specify this field as the primary key during the indexing-call. In fact, it should suffice as a unique ID.
The function that's used offers a parameter for passing that:
meilisearch-php/src/Endpoints/Delegates/HandlesDocuments.php: public function addDocuments(array $documents, string $primaryKey = null)
It might be enough to just do that.
Remaining tasks
User interface changes
In case of 3a. above, a checkbox needs to be added.
API changes
See 3a. above, maybe API changes are also needed.
Data model changes
See 3a. above, maybe data model changes are also needed.
Issue fork search_api_meilisearch-3365408
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
tgoeg commentedPatch for now fixing it the 3b. way, I'll try to create a PR later.
This seems to be backwards compatible until the pinned version meilisearch/meilisearch-php ^0.24.0 of drupal/search_api_meilisearch 1.0, so could be directly integrated without needing to update to Meilisearch 1.x (and meilisearch/meilisearch-php 1.x).
https://github.com/meilisearch/meilisearch-php/blob/v0.24.0/src/Endpoint... already has that parameter.
Comment #3
deaom commentedThis is more of a future request then a bug report, since it seems this is working for the meilisearch php version that module states (v0.24.0). If the module will upgrade the version of meilisearch-php this can be revisited in the case it will not work. For now closing as it works as designed for supported version as there is currently no support for newer versions. Feel free to re-open if that is not the case.
Comment #4
tgoeg commented> this can be revisited in the case it will not work.
I can tell for sure it will not work :-)
0.24 has a few open security issues (mostly in dependencies) so I would recommend to upgrade a.s.a.p.
This is a very simple change that does not hurt (read: sets the node ID as the explicit primary key which would otherwise be automatically inferred, i.e. does not change anything in the index at all) in the current setup but will save a few hours of debugging once Meilisearch gets updated.
You could define it as a bug in the current version as well, as not explicitly setting the primary key at all could lead to hard to track down problems anyway. Why not take the safe route now already?
Just wanted to be nice for anyone working on this to be prepared :-)
I know this is probably not enough coverage, but Meilisearch 1.2.0 runs smoothly in our setup apart from this issue.
Comment #5
deaom commentedI just wanted to clean up the issue queue and it seems I failed miserably, my apologies to you.
I'm re-opening this as a bug for this dev version and marking it as needs work (as the code is not yet pushed to branch) and will assign it to myself, so the code you provided goes into MR (as soon as I fin some time). Not sure how fast it will be merged, as I assume test will also be needed.
Comment #7
deaom commentedAs per @tgoeg instructions to reproducing the issue I tested first how this behaves with the supported version (v0.28) without the code change, and it automatically sets the primary key as id. Then I added the code change @tgoeg provided and tested to see if anything changes on the v0.28 version of the meilisearch-php and everything works as expected.
Then I upgraded the meilisearch-php version to 1.2 to test the mentioned behaviour and could reproduce the issue. The primary key was not set and stayed as null, which resulted in no nodes being indexed which can be easily tested by searching for something and getting no results.
Then after adding the primary key as id to the code, I re-indexed the index and nodes were indexed which again could be tested by searching for something and getting desired results.
I'm marking this as needs review, even tough basically I reviewed the code form @tgoeg, but because I'm making a MR, best to have it set that way, I suppose.
I assume this will be covered with news tests from #3188421: Providing tests, so can probably be merged, once reviewed/approved.
Comment #9
bcizej commentedLooks ok, thanks @tgoeg @DeaOm
Comment #10
bcizej commentedAdded credits
Comment #11
tgoeg commentedThanks from my side as well!
The fact that there's no error visible in Drupal is still open, but this is not strictly related to this issue alone.
I opened https://www.drupal.org/project/search_api_meilisearch/issues/3380493 for it.