This was a fun one to debug. We have two indexes, one is indexing content as anonymous users, another one as a user with admin permissions (for unpublished content) We use paragraphs, which check parent entity access.
We had a problem that it sometimes didn't work. It worked fine when manually indexing, but not when creating new content and letting it index through cron. That is because then it did index the same item in the same process for both indexes, first with anonymous access. Entity access control handlers statically cache access results by UID. And because the created UserSession always has uid 0, it shared the static cache, despite having different roles.
Set a UID, possibly based on the roles to still benefit from static caching across multiple items/fields if it's the same roles.
This could in theory have side effects, like something trying to load a user when its > 0, but I kept the existing 0 for the default anonymous configuration to minimize that.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3181863-9--rendered_item_processor_permission_cache_issue.patch | 1.48 KB | spleshka |
| #7 | 3181863-7--rendered_item_processor_permission_cache_issue.patch | 1.48 KB | drunken monkey |
Show commandsStart 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:
changes
Comments
Comment #2
berdirComment #3
mkalkbrennerI haven't tested the patch yet. But the issue description describes the issue of a customer. I already assumed that caching is involved.
Comment #4
mkalkbrennerWith this patch applied, the situation got better but the issue isn't solved completely.
If I reindex everything in a batch, the result toggles between success and access errors for some items.
It really toggles!
1. sucess 7488 items
2. between 100 and 3000 erroneous items
3. sucess 7488 items
4. between 100 and 3000 erroneous items
5. sucess 7488 items
6. between 100 and 3000 erroneous items
7. sucess 7488 items
8. between 100 and 3000 erroneous items
9. success 7488 items
...
Its one index with two datasources. I also have the impression that indexing speed varies between the successful and erroneous runs. So it seems like successful cache hits are involved.
Comment #5
berdirThis patch should only have an effect if you index the same item from the same data source in two different indexes. If you only have one index them I'm quite sure that this does not apply for you. Except maybe if there is custom code involved that is checking for $user->isAnonymous() in the render pipeline, but that too should only result in wrong/missing data being indexed, not errors.
Comment #6
drunken monkeyThanks a lot for reporting this problem and providing a patch! Your explanation does make sense and the fix looks good.
In such cases, I’m always worried about hitting an existing user ID and the user entity getting changed somehow, but that’s probably a very small risk.
In #2898334: Add a "Role-based access" processor we are working on something similar. There, to avoid entity loads, we opted for negative UIDs (at least for now – not sure whether it will get committed like that). Maybe an option here, too? However, it could cause problems with
UserSession::isAuthenticated(), which checks for$this->uid > 0.Another option would be to just use very high numbers, that can’t realistically be valid UIDs. Maybe then just use static caching to make sure we use the same UID for the same role?
In any case, this will require some test coverage. Would you be willing to provide a regression test for this in
RenderedItemTest, Sascha?@ Markus: Thanks for your feedback, good to hear it at least helped somewhat with your problem, too. However, it’s rather hard to see how that behavior would be caused by this patch, so please try and debug yourself to see what’s happening. (I don’t think this can easily be reproduced without your exact setup.)
In any case, if the existing patch already helps for your problem, then I’d just commit it regardless, unless you find something specific to improve, and we can continue to refine in a follow-up, once you find more info.
Comment #7
drunken monkeyAlso, two small cosmetic adjustments.
Comment #8
sokru commentedI'd say this is duplicate of #2979846: User session change causes runtime exception in other modules. We faced the same issue with multiple indexes, using Rendered HTML output field with different User role settings. Unfortunately none of the patches worked, I suspect the reason was usage of hook_entity_access() on our project. Instead of creating dedicated Search API user with desired user roles, we solved the issue by setting the indexes not to index items immediately and having cron batch size to zero. Indexing happens by separate Drush commands set on server's cronjob.
Comment #9
spleshkaRe-rolled #7 against latest version of search api.
Comment #12
benstallings commentedComment #13
benstallings commentedBehavior changes for sites using non-default "Rendered item" role configurations
When the "Rendered item" field is configured with any role other than just Anonymous, the transient account used for rendering now has a generated user ID (≥ 2^32, guaranteed not to collide with any real user) instead of UID 0. The default (anonymous-only) configuration is unaffected and keeps using UID 0.
Two observable consequences during indexing-time rendering:
1. \Drupal::currentUser()->isAuthenticated() now returns TRUE for these configurations (previously FALSE, even when the Authenticated role was selected). This is more correct, but rendered output may change — for example, "Log in to post comments" links will no longer appear in indexed content. Affected sites should reindex after updating so indexed content reflects the new rendering.
2. Code that loads the user entity for the current user during rendering (e.g., User::load(\Drupal::currentUser()->id())) will now get NULL instead of the anonymous user entity, since no user entity exists for the generated UID. This matches the existing behavior of the "Role-based access" processor, which also uses transient accounts with non-existent UIDs. Custom or contrib code used in indexed view modes should handle a NULL return there.
Comment #14
drunken monkeyI think this is a duplicate of #3110652: RenderedItem processor causes theme to think user is anonymous, please continue the discussion there.
(I’m not closing this as a duplicate yet so people with an interest in this issue have a better chance of seeing this comment.)