Currently, we have several classes representing blocks (DatabaseBlock, SystemBlock, ...), all based on the AbstractBlock class, which doesn't involve the Database at all. Hence, I think this task should be updated. We could:

1. Have a general-purpose BlockStore service, suitable for all implementations of AbstractBlock
2. Have only a DatabaseBlockStore service
3. Have different services for different AbstractBlock implementations; we'd likely have a base interface for all services

I think (3) might be the most sensible, but I can't say for sure.

Thanks for the summary @Daimona. Of those options, (2) makes the most sense to me. The thing that sets DatabaseBlocks apart is that they are stored in the ipblocks table, and the BlockService would manage this. On the other hand, SystemBlocks and CompositeBlocks should not be stored in the database, since they're calculated on the fly.

I think we could start by moving the following methods from DatabaseBlock to a DatabaseBlockStore:

• delete
• insert
• update
• getDatabaseArray
• getQueryInfo (see T255433)
• purgeExpired

Do methods like newFromID, newFromTarget, etc also belong in the store? If not, then perhaps another service? Having these as static methods on DatabaseBlock makes unit testing for a lot of the block-related classes awkward; we often insert blocks for real when we shouldn't need to, which makes for slow tests. Would be interested to hear @daniel's thoughts.

Change 602785 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add a DatabaseBlockStore service

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

Change 602785 merged by jenkins-bot:
[mediawiki/core@master] Add a DatabaseBlockStore service

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

On beta, tested inserting (including an autoblock), updating, deleting and purging an expired block. All things the new DatabaseBlockStore is responsible for. Checked the correct blocks were created by looking at Special:BlockList and Special:Log.

Repeated this on vagrant to check that page and namespace restrictions are correctly added/removed from database.

Also tested that the autoblock hooks fired from DatabaseBlockStore when you create a block are handled correctly by CheckUser, and correctly inserts autoblocks.

Also tested autoblocks with CheckUser disabled, which also works fine.

Test environment: https://en.wikipedia.beta.wmflabs.org MediaWiki 1.36.0-alpha (0fed18b) 06:34, 30 July 2020 and vagrant MediaWiki 1.36.0-alpha (0fed18b).