Page MenuHomePhabricator

Review namespacing of MCR classes
Closed, ResolvedPublic

Description

We've added a number of classes for MCR, and there are more that still need adding. I think we could use a review of the namespaces being used for these classes.

The current structure is:

  • MediaWiki\Storage
    • ---- blob access
    • BlobAccessException
    • BlobStoreFactory
    • BlobStore
    • SqlBlobStore
    • ---- name table access
    • NameTableAccessException
    • NameTableStoreFactory
    • NameTableStore
    • ---- revision storage
    • RevisionFactory
    • RevisionLookup
    • RevisionStoreFactory
    • RevisionStore
    • ---- revision records
    • RevisionRecord
    • MutableRevisionRecord
    • RevisionArchiveRecord
    • RevisionStoreRecord
    • RevisionSlots
    • MutableRevisionSlots
    • SlotRecord
    • ---- revision exceptions
    • IncompleteRevisionException
    • RevisionAccessException
    • SuppressedDataException
    • ---- page editing
    • DerivedPageDataUpdater
    • PageUpdater
    • PageUpdateException
    • RevisionSlotsUpdate
  • MediaWiki\Revision
    • ---- revision rendering
    • RevisionRenderer
    • RenderedRevision

That's a lot of stuff in MediaWiki\Storage, most of which is only tenuously related to "storage". Following the precedent, there's a lot more in the future that would be able to go into MediaWiki\Storage too: PageStore and all its value classes, UserStore and whatever value classes it might have, probably even EditController and so on.

Event Timeline

Anomie created this task.Sep 12 2018, 8:26 PM
Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptSep 12 2018, 8:26 PM
Anomie added a comment.EditedSep 12 2018, 8:31 PM

I might rearrange these as follows (๐Ÿ”ฎ indicates classes that aren't created yet):

  • MediaWiki\Storage
    • ---- blob access
    • BlobAccessException
    • BlobStoreFactory
    • BlobStore
    • SqlBlobStore
    • ---- name table access
    • NameTableAccessException
    • NameTableStoreFactory
    • NameTableStore
  • MediaWiki\Revision
    • ---- revision storage (on the border between MediaWiki\Storage and MediaWiki\Revision)
    • RevisionFactory
    • RevisionLookup
    • RevisionStoreFactory
    • RevisionStore
    • ---- revision records
    • RevisionRecord
    • MutableRevisionRecord
    • RevisionArchiveRecord
    • RevisionStoreRecord
    • RevisionSlots (or maybe in MediaWiki\Revision\Slots)
    • MutableRevisionSlots (same)
    • ---- revision exceptions
    • IncompleteRevisionException
    • RevisionAccessException
    • SuppressedDataException
    • ---- revision rendering
    • RevisionRenderer
    • RenderedRevision
  • MediaWiki\Revision\Slots
    • SlotRecord
    • ๐Ÿ”ฎ SlotRoleHandler โ€“ like ContentHandler but for slots
  • MediaWiki\Page
    • ---- page storage and records
    • ๐Ÿ”ฎ PageStore โ€“ like RevisionStore but for the page table
    • ๐Ÿ”ฎ PageStoreFactory โ€“ for the same reason
    • ๐Ÿ”ฎ PageRecord โ€“ value class, I don't know if we'll need subclasses
    • ๐Ÿ”ฎ PageEventEmitter โ€“ like DerivedPageDataUpdater without all the extra stuff
    • ๐Ÿ”ฎ Probably some access-related exceptions
    • ---- page type handling
    • ๐Ÿ”ฎ PageTypeHandler
    • ๐Ÿ”ฎ PageTypeHandler subclasses
  • MediaWiki\Edit
    • ๐Ÿ”ฎ EditController โ€“ logic for turning EditData into a RevisionRecord. Name kind of sucks.
    • ๐Ÿ”ฎ EditData โ€“ edit data to be turned into a RevisionRecord
    • ๐Ÿ”ฎ EditStash โ€“ cache mapping EditData to RenderedRevision
    • ๐Ÿ”ฎ Maybe some other class to have full-service methods like WikiPage::doEditContent(), if those don't wind up in EditController.
    • ๐Ÿ”ฎ Probably some exceptions
  • (deprecate)
    • DerivedPageDataUpdater โ€“ main functionality in MediaWiki\Page\PageDataUpdater, state in MediaWiki\Edit\EditData, "preparing" in MediaWiki\Edit\EditController
    • PageUpdater โ€“ state in MediaWiki\Edit\EditData, creation of the RevisionRecord in MediaWiki\Edit\EditController, saving in MediaWiki\Page\PageStore
    • PageUpdateException โ€“ only used in PageUpdater
    • RevisionSlotsUpdate โ€“ now part of MediaWiki\Edit\EditData

Here MediaWiki\Revisions seems big like MediaWiki\Storage is now, but the difference is that there doesn't seem to be anything else to add to it in the future.

+1 to the use of MediaWiki\Edit, that was my intention when I originally introduced it, I just never completed the patch that would use it even more.

I like splitting Storage into a Revision namespace, though I'm not sure about the value in third namespace level (MediaWiki\Revision\Slots).

MediaWiki\Page looks reasonable as well, though it's harder to comment on that one since none of those classes exist yet :)

Slightly related, the new namespaces should use upper case directory names (like Storage did) to be in compliance with PSR-4 to begin with.

Change 461714 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Re-namespace RevisionStore and RevisionRecord classes

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

Tgr added a subscriber: Tgr.Sep 20 2018, 7:43 PM

Slightly related, the new namespaces should use upper case directory names (like Storage did) to be in compliance with PSR-4 to begin with.

includes/page and includes/edit already exist.

I've argued elsewhere that we should use a new src directory as the PSR-4 root, to avoid exactly these kind of naming conflicts.

Tgr added a comment.Sep 20 2018, 7:45 PM

The new namespaces look fine to me (I don't think it matters much as people use autocomplete anyway, but they are certainly more intuitive than the old ones). How to split up / deprecate DerivedPageDataUpdater/PageUpdater/RevisionSlotsUpdate should be a separate discussion IMO, namespace-wise those are fine under Revision or Page.

Change 461714 merged by jenkins-bot:
[mediawiki/core@master] Re-namespace RevisionStore and RevisionRecord classes

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

Anomie closed this task as Resolved.Oct 10 2018, 3:12 PM
Anomie claimed this task.
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:38 PM