GenderCache needs UserNameUtils but that gives a circular dependency / Split MediaWikiTitleCodec into TitleParser and TitleFormatter for proper dependencies?
Closed, ResolvedPublic
Actions

Description

The GenderCache class is a service and calling the deprecated User::isValidUserName. The replacement is the UserNameUtils service.

But the UserNameUtils indirectly used the GenderCache and than a circular dependency is given and exception thrown.
The way is: UserNameUtils -> TitleParser -> _MediaWikiTitleCodec -> GenderCache -> UserNameUtils

From my understanding of the code, the GenderCache in class MediaWikiTitleCodec is only used for the TitleFormatter part (Used in getNamespaceName) and UserNameUtils needs only a TitleParser.
So MediaWikiTitleCodec needs a split or how to resolve this situation?
Creating own MediaWikiTitleCodec as TitleParser for the UserNameUtils without GenderCache because the function should not be called?

Details

	Subject	Repo	Branch	Lines +/-
	GenderCache: remove checking for valid usernames	mediawiki/core	master	+2 -5

Customize query in gerrit

Related Objects

Mentioned In: T315827: GenderCache could avoid one SQL request sometimes

Event Timeline

Umherirrender created this task.Nov 2 2020, 6:32 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 2 2020, 6:32 PM

DannyS712 added a project: Platform Engineering.Dec 14 2020, 7:39 PM

• holger.knust triaged this task as Medium priority.Dec 15 2020, 2:14 PM

• holger.knust edited projects, added Platform Team Workboards (Clinic Duty Team); removed Platform Engineering.

• holger.knust moved this task from Inbox to Later on the Platform Team Workboards (Clinic Duty Team) board.

• holger.knust added subscribers: daniel, • Pchelolo.

I think the User::isValidUserName() call can just be removed. The worst thing that can happen is that the DB will have to search for nonexistent rows. If that's a performance problem, the check can be moved to the caller of GenderCache. But some callers (e.g. ApiPageSet) are getting the inputs from an already validated source.

Change 689533 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] GenderCache: remove checking for valid usernames

https://gerrit.wikimedia.org/r/689533

gerritbot added a project: Patch-For-Review.May 12 2021, 6:36 AM

In T267054#6693436, @tstarling wrote:

I think the User::isValidUserName() call can just be removed. The worst thing that can happen is that the DB will have to search for nonexistent rows. If that's a performance problem, the check can be moved to the caller of GenderCache. But some callers (e.g. ApiPageSet) are getting the inputs from an already validated source.

Makes sense to me (we already can be searching for nonexistent rows since we don't verify that the username corresponds to a user that exists, just one that could exist) - sent a patch to remove the call entirely

Change 689533 merged by jenkins-bot:

[mediawiki/core@master] GenderCache: remove checking for valid usernames

https://gerrit.wikimedia.org/r/689533

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.6; 2021-05-18).May 13 2021, 1:00 AM

Maintenance_bot removed a project: Patch-For-Review.May 13 2021, 1:10 AM

DannyS712 closed this task as Resolved.May 13 2021, 3:36 AM

DannyS712 claimed this task.

Restricted Application added a project: User-DannyS712. · View Herald TranscriptMay 13 2021, 3:36 AM

Umherirrender mentioned this in T315827: GenderCache could avoid one SQL request sometimes.Aug 25 2022, 9:23 PM