After Autopromote logic is moved into UserGroupManager, it would require PermissionManager in order to find out if the user is a bot. PermissionManager would depend on UserGroupManager for getting user groups for permission verification.
I propose to split group permission methods into GroupPermissionLookup service, separate from PermissionManager. That would allow to break the circular dependency.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • Pchelolo | T234921 Factor group membership management out of User class | |||
| Resolved | • Pchelolo | T252621 Move Autopromotion logic to UserGroupManager service | |||
| Resolved | • Pchelolo | T254537 UserGroupManager/PermissionManager cyclic dependency | |||
| Resolved | Peter.ovchyn | T271593 Replace specific PermissionManager calls with GroupPermissionsLookup calls in all deployed extensions | |||
| Resolved | Peter.ovchyn | T271596 Migrate relevant User method usages to GroupPermissionsLookup and hard-deprecate User methods. |
UserGroupManager also needs access to PermissionManager's cache reset method, which is currently passed as a callback to avoid such a dependency, but that should be handled separately (eg with a dedicated central caching service)
Change 638637 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Add todo comment about UserGroupManager/PermissionManager dependency
Change 638637 merged by jenkins-bot:
[mediawiki/core@master] Add todo comment about UserGroupManager/PermissionManager dependency
Change 654528 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Introduce GroupPermissionsLookup
Change 654931 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Use GroupPermissionsLookup in various places
Change 654528 merged by jenkins-bot:
[mediawiki/core@master] Introduce GroupPermissionsLookup
Change 654931 merged by jenkins-bot:
[mediawiki/core@master] Enhance GroupPermissionsLookup and use it.
@Pchelolo in ServiceWiring the UserGroupManager is still passed a closure that invokes PermissionManager::invalidateUsersRightsCache(), and PermissionManager is still created with UserGroupManager injected - why is this resolved? They still depend on each other, right?
PermissionManager depending on UserGroupManager is fine, it's how the dependency is supposed to go.
UserGroupManager lazy dependency on PermissionManager for invalidating the user rights cache is fine as well - we don't have events yet, so we don't have a better mechanism for invalidation callbacks like this.
The task was about direct dependency on permission manager for figuring out whether the user is bot or not. That's now solved.