While saving an entity (for example a node), all translations will be indexed immediately if the index is configured to do so. That's implemented in function search_api_entity_update(EntityInterface $entity).
If you reindex only the default translations will be re-indexed. All other translations are missing!
I guess that the same problem occurs if you clean the index.
This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.
Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.
Value : 5
Story Points: 3
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | reindex_misses-2540668-13.patch | 907 bytes | borisson_ |
| #1 | 2540668.patch | 853 bytes | mkalkbrenner |
Comments
Comment #1
mkalkbrennerHere's a quick and dirty patch. I'm sure it needs some polishing.
The better solution will be if function search_api_entity_update(EntityInterface $entity) and the queue based indexing will share more code!
Comment #2
mkalkbrennerComment #3
drunken monkeyThanks for reporting this issue!
You're right, that's really bad. However, your patch is a bit too dirty, I'm afraid – the structure of the item IDs (and the whole language handling) is completely up to the datasource plugin (and the
hook_entity_*()implementations, as undesirable as that is), that's definitely not something theIndexclass should take care of.So, we need to either patch the datasource plugin or the hook implementation(s).
I'd be very surprised if it would, that's a completely different mechanism.
Comment #4
nick_vhComment #5
nick_vhComment #6
borisson_I tried to reproduce this in a test but I don't think I understand the problem fully, it looks like the test agrees that this is pretty much ok.
Comment #7
drunken monkeyMarkus, I know you explained it at least twice to me already, but without it being in here I just keep forgetting it. Could you please explain the problem again, and how to trigger it?
And, did we actually already find something wrong? I vaguely remember debugging it with you in Vienna, but I might be mistaken.
@ Joris: A test would be handy in any case, I'd say, thanks for going to that trouble! I think we should just integrate it into the normal
IntegrationTest, but then it could really be helpful.Comment #8
borisson_@drunken monkey: I definitely see the value of adding a seperate test for the language specific features, not only can this test be run a lot faster (which I like for debugging), we should also add an integration test for #2573457: Make stopwords translatable and that can go in this class as well.
Attached is a reroll of the test; I was able to remove some unneeded things.
The only relevant test method
checkMultilingualContentEntityTrackingcan easily be ported to theIntegrationTestthough.Comment #9
borisson_Actual patch from #8.
Comment #10
mkalkbrennerSorry for the late reply but I'm very busy at the moment. Due to the same reason I didn't follow the latest changes to search_api. But I try to explain what we discovered early and what led to my initial patch.
I discovered a fundamental difference between indexing a single entity triggered by Entity::save() and the re-index job.
While Entity::save() iterates over all translations of an entity and indexes them separately, the re-index job just updates the index entry for the default translation of an entity.
My initial quick and dirty patch adds all the translations to the array of items to be re-indexed. The ugly part is the regex to create the item ids. This needs to leverage the id generation "service" - whatever that is.
Comment #11
borisson_If I understand it correctly, the test in the patch proves that is no longer a problem.
I added more comments in the test.
Comment #12
mkalkbrennerI'll update our test systems and try to verify if the issue is gone ...
Comment #13
borisson_The entire testcase that was added here is also in the new patch introduced trough #2256219: Allow the content entity datasource to specify which languages to handle.
We could add the attached patch, but that probably isn't worth it.
Comment #14
drunken monkeyOK, then let's just wait for Markus' feedback on whether he can still reproduce the problem and close otherwise.
Comment #15
drunken monkeyAny news on this?
Would be great to either verify or close this.
Comment #16
drunken monkey@ Markus: Please just re-open if you can still reproduce this, with steps explaining how.
Comment #17
mkalkbrennerSorry for not updating the issue. It didn't happen anymore in our environments :-)