Summary
The N+1 group_user fix from #4696 (shipped in v2.0.0-rc.3) does not cover the forum HTML page render path. When a discussion page is rendered server-side (e.g. for a guest / crawler), serializing post authors still issues one group_user query per serialized author instance via UserPolicy::editCredentials() → User::isAdmin() → $user->groups.
What #4696 fixed
#4696 made the UserResource groups getter read the eager-loaded relation, and the posts endpoints (Show/Index) ->eagerLoad(['user.groups']). The regression test tests/integration/api/posts/ListGroupsQueryCountTest.php verifies this — but it exercises the direct JSON:API endpoint (GET /api/posts) as an authenticated user:
$this->request('GET', '/api/posts', ['authenticatedAs' => 2])
->withQueryParams(['filter' => ['discussion' => 1]])
That path passes: groups are batch-loaded once.
What's still broken
When the forum content is rendered as HTML (Flarum\Forum\Content\Discussion), the same authors are serialized again through additional documents beyond the embedded /posts include (the discussion's own user, firstPost.user, etc.). Those User model instances do not carry the eager-loaded groups relation, so the per-user lazy load fires again.
The trigger is the email field's visible() callback and the canEditCredentials field in UserResource:
// UserResource — email field
->visible(function (User $user, Context $context) {
return $context->getActor()->id === $user->id
|| $context->getActor()->can('editCredentials', $user); // ← isAdmin() → $user->groups
})
editCredentials reads $user->isAdmin() (→ $user->groups) on a User instance whose groups relation was not eager-loaded on this path.
Trace (from a downstream extension's render test)
Flarum\Api\Endpoint\Index serialize
→ UserResource email field visible() (UserResource.php:185)
→ $actor->can('editCredentials', $user)
→ UserPolicy::editCredentials → $user->isAdmin() → $user->groups ← lazy, 1 query per author instance
Query-log classification on an 8-author discussion page rendered as a guest:
Both populations coexist, which proves the eager-loaded instances and the editCredentials-checked instances are not the same objects.
How it surfaced
Discovered downstream in FriendsOfFlarum/seo, whose QAPage structured-data render walks a discussion's posts. A query-count regression guard there fails on rc.3 (growth of 6 vs an expected ≤3) while green on the older CI baseline. Repro is environment-agnostic (reproduces on SQLite, which is in core's own CI matrix).
Suggested fixes (one of)
- Reorder
UserPolicy::editCredentials to check the cheap $actor permission / $actor->isAdmin() before reading $user->isAdmin() ($user->groups).
- Ensure the forum discussion content render eager-loads
user.groups on all serialized author instances, not only the embedded /posts include.
- Have
editCredentials avoid forcing a lazy relation load on unloaded User instances.
Repro environment
Related: #4695, #4696
Summary
The N+1
group_userfix from #4696 (shipped inv2.0.0-rc.3) does not cover the forum HTML page render path. When a discussion page is rendered server-side (e.g. for a guest / crawler), serializing post authors still issues onegroup_userquery per serialized author instance viaUserPolicy::editCredentials()→User::isAdmin()→$user->groups.What #4696 fixed
#4696 made the
UserResourcegroupsgetter read the eager-loaded relation, and the posts endpoints (Show/Index)->eagerLoad(['user.groups']). The regression testtests/integration/api/posts/ListGroupsQueryCountTest.phpverifies this — but it exercises the direct JSON:API endpoint (GET /api/posts) as an authenticated user:That path passes: groups are batch-loaded once.
What's still broken
When the forum content is rendered as HTML (
Flarum\Forum\Content\Discussion), the same authors are serialized again through additional documents beyond the embedded/postsinclude (the discussion's ownuser,firstPost.user, etc.). ThoseUsermodel instances do not carry the eager-loadedgroupsrelation, so the per-user lazy load fires again.The trigger is the
emailfield'svisible()callback and thecanEditCredentialsfield inUserResource:editCredentialsreads$user->isAdmin()(→$user->groups) on aUserinstance whosegroupsrelation was not eager-loaded on this path.Trace (from a downstream extension's render test)
Query-log classification on an 8-author discussion page rendered as a guest:
8 × [BATCH]group_user ... in (...)— the [2.x] Fix N+1 group_user queries when serializing users #4696 eager-load is firing10 × [SINGLE]group_user ... where user_id = ?— the residual N+1 (same author re-serialized as unloaded instances)Both populations coexist, which proves the eager-loaded instances and the
editCredentials-checked instances are not the same objects.How it surfaced
Discovered downstream in
FriendsOfFlarum/seo, whoseQAPagestructured-data render walks a discussion's posts. A query-count regression guard there fails onrc.3(growth of 6 vs an expected ≤3) while green on the older CI baseline. Repro is environment-agnostic (reproduces on SQLite, which is in core's own CI matrix).Suggested fixes (one of)
UserPolicy::editCredentialsto check the cheap$actorpermission /$actor->isAdmin()before reading$user->isAdmin()($user->groups).user.groupson all serialized author instances, not only the embedded/postsinclude.editCredentialsavoid forcing a lazy relation load on unloadedUserinstances.Repro environment
flarum/corev2.0.0-rc.3(contains [2.x] Fix N+1 group_user queries when serializing users #4696)Related: #4695, #4696