Page MenuHomePhabricator

Remove global service container access from Block class hierarchy
Open, Needs TriagePublic

Description

In the MediaWiki core classes that implement the Block interface, calls to MediaWikiServices::getInstance() should be avoided, for testability and architectural cleanliness.

Plans:

  • New convenience factory methods in DatabaseBlockStore, for the benefit of the many DatabaseBlock::__construct() callers in tests.
  • The DatabaseBlock constructor parameter expiry, which requires IDatabase for its interpretation, will be replaced by decodedExpiry. We could consider some other interpretation for expiry which doesn't depend on services, since any caller that is actually getting rows from the database should be using DatabaseBlockStore::newFromRow(), which already passes decodedExpiry to DatabaseBlock.
  • The AbstractBlock constructor parameter address, which requires BlockUtils for its interpretation, will be replaced by target, which will be a BlockTarget object (T382106). The original parameter will still be available in a factory.
  • Calling setTarget()with UserIdentity|string will be deprecated in favour of BlockTarget|null, to avoid a BlockUtils dependency.
  • Already hard-deprecated: newFromID, newFromRow, delete, insert, update, isExemptedFromAutoblocks, doAutoblock, updateTimestamp, getAutoblockExpiry, newFromTarget, newListFromTarget and getBlocksForIPList

Problems without plans:

  • appliesToRight needs config, BlockActionInfo and PermissionManager
  • appliesToUsertalk needs config
  • equals, setId and getRestrictions need BlockRestrictionStore
  • setBlocker needs UserNameUtils
  • setReason can take a Message parameter and formats it using CommentStoreComment::newUnsavedComment, which depends on services

Event Timeline

Change #1118000 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] block: DatabaseBlock constructor caller migration

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

Change #1118259 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] block: Deprecate UserIdentity|string block target specifications

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

Change #1118000 merged by jenkins-bot:

[mediawiki/core@master] block: DatabaseBlock constructor caller migration

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

Change #1124904 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/CheckUser@master] Updates for multiblocks

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

Change #1126213 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/CentralAuth@master] Use DatabaseBlockStore::newUnsaved()

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

Change #1126214 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/EntitySchema@master] Avoid direct construction of DatabaseBlock

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

Change #1126218 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/EventBus@master] Avoid direct construction of DatabaseBlock

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

Change #1126228 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/Thanks@master] Avoid DatabaseBlock address parameter

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

Change #1126213 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Use DatabaseBlockStore::newUnsaved()

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

Change #1124904 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Updates for multiblocks

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

Change #1126218 merged by jenkins-bot:

[mediawiki/extensions/EventBus@master] Avoid direct construction of DatabaseBlock

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

Change #1126214 merged by jenkins-bot:

[mediawiki/extensions/EntitySchema@master] Avoid direct construction of DatabaseBlock

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

Change #1126228 merged by jenkins-bot:

[mediawiki/extensions/Thanks@master] Avoid DatabaseBlock address parameter

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

Change #1140563 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/UploadWizard@master] Avoid DatabaseBlock::setTarget() with UserIdentity

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

Change #1140563 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@master] Avoid DatabaseBlock::setTarget() with UserIdentity

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

Change #1141559 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/GrowthExperiments@master] Avoid calling DatabaseBlock::setTarget() with UserIdentity

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

Change #1141561 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/WikibaseLexeme@master] Avoid direct construction of DatabaseBlock

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

Change #1141563 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/Wikibase@master] Avoid direct construction of DatabaseBlock

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

Change #1141563 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Avoid direct construction of DatabaseBlock

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

Change #1141559 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Avoid calling DatabaseBlock::setTarget() with UserIdentity

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

Change #1141561 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Avoid direct construction of DatabaseBlock

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

Change #1141925 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/extensions/WikimediaMaintenance@master] Pass in UserBlockTarget object to `AbstractBlock::setTarget()`

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

Change #1141925 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMaintenance@master] Pass in UserBlockTarget object to `AbstractBlock::setTarget()`

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

Change #1118259 merged by jenkins-bot:

[mediawiki/core@master] block: Deprecate UserIdentity|string block target specifications

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

Change #1143488 had a related patch set uploaded (by D3r1ck01; author: Tim Starling):

[mediawiki/core@REL1_44] block: Deprecate UserIdentity|string block target specifications

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

Change #1143488 merged by jenkins-bot:

[mediawiki/core@REL1_44] block: Deprecate UserIdentity|string block target specifications

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