Page MenuHomePhabricator

Make RevisionRecord return a ProperPageIdentity
Open, MediumPublic

Description

Anything that has a revision is a "proper" page by definition,
so RevisionRecord::getPage() should return a ProperPageIdentity.

Event Timeline

Change 666136 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Make RevisionRecord return a ProperPageIdentity

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

Change 665661 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Make RevisionRecord require a ProperPageIdentity

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

Change 666519 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Deprecate constructing revision with non-proper page

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

daniel lowered the priority of this task from High to Medium.Mar 9 2021, 5:50 PM

Change 666519 merged by jenkins-bot:
[mediawiki/core@master] Deprecate constructing revision with non-proper page

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

Change 672462 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] RevisionStore: getRevisionbyId should take a PageIdentity

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

Change 672499 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] RevisionStore: create getPage, deprecate getTitle

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

As https://gerrit.wikimedia.org/r/672462 and https://gerrit.wikimedia.org/r/672499 do not seem to be finished, and the code change in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/666519 causes issues in Wikibase (T277362), WMDE would be inclined to revert https://gerrit.wikimedia.org/r/c/mediawiki/core/+/666519. If getting the proper fix (tm) is just minutes away and you're actively getting those fixes ready and merged @daniel and team, we'd hold with reverting, but would like to have a clarity on the status.

Change 672810 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Revert "Deprecate constructing revision with non-proper page"

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

Change 672811 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@wmf/1.36.0-wmf.35] Revert "Deprecate constructing revision with non-proper page"

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

Change 672810 merged by jenkins-bot:
[mediawiki/core@master] Revert "Deprecate constructing revision with non-proper page"

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

Change 672811 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.35] Revert "Deprecate constructing revision with non-proper page"

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

Mentioned in SAL (#wikimedia-operations) [2021-03-16T23:56:32Z] <krinkle@deploy1002> Synchronized php-1.36.0-wmf.35/includes/Revision/: I8619ab9e92b, T277362, T275531 (duration: 00m 58s)

Change 672819 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Re-apply "Deprecate constructing revision with non-proper page"

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

Change 672499 merged by jenkins-bot:
[mediawiki/core@master] RevisionStore: create getPage, deprecate getTitle

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

Change 672462 merged by jenkins-bot:
[mediawiki/core@master] RevisionStore: getRevisionbyId should take a PageIdentity

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

Change 672819 merged by jenkins-bot:
[mediawiki/core@master] Re-apply "Deprecate constructing revision with non-proper page"

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

@daniel I was in looking at the production errors with @hashar today and it looks that the problem still happens - not very often though

See the stacktrace below:

from /srv/mediawiki/php-1.36.0-wmf.36/includes/user/UserIdentityValue.php(98)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-1.36.0-wmf.36/includes/debug/MWDebug.php(376): trigger_error(string, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.36/includes/debug/MWDebug.php(352): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-1.36.0-wmf.36/includes/GlobalFunctions.php(1068): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.36/includes/dao/WikiAwareEntityTrait.php(78): wfDeprecatedMsg(string, string)
#5 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/UserIdentityValue.php(98): MediaWiki\User\UserIdentityValue->deprecateInvalidCrossWiki(string, string)
#6 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/ActorStore.php(489): MediaWiki\User\UserIdentityValue->getId(string)
#7 /srv/mediawiki/php-1.36.0-wmf.36/includes/ActorMigration.php(319): MediaWiki\User\ActorStore->acquireActorId(MediaWiki\User\UserIdentityValue, Wikimedia\Rdbms\DBConnRef)
#8 /srv/mediawiki/php-1.36.0-wmf.36/includes/block/DatabaseBlockStore.php(379): ActorMigration->getInsertValues(Wikimedia\Rdbms\DBConnRef, string, MediaWiki\User\UserIdentityValue)
#9 /srv/mediawiki/php-1.36.0-wmf.36/includes/block/DatabaseBlockStore.php(165): MediaWiki\Block\DatabaseBlockStore->getArrayForDatabaseBlock(MediaWiki\Block\DatabaseBlock, Wikimedia\Rdbms\DBConnRef)
#10 /srv/mediawiki/php-1.36.0-wmf.36/includes/block/DatabaseBlock.php(524): MediaWiki\Block\DatabaseBlockStore->insertBlock(MediaWiki\Block\DatabaseBlock, Wikimedia\Rdbms\DBConnRef)
#11 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/CentralAuthUser.php(1950): MediaWiki\Block\DatabaseBlock->insert(Wikimedia\Rdbms\DBConnRef)
#12 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/CentralAuthUser.php(1881): CentralAuthUser->doLocalSuppression(boolean, string, string, string)
#13 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/CentralAuthUser.php(1859): CentralAuthUser->doCrosswikiSuppression(boolean, string, string)
#14 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/CentralAuthUser.php(1810): CentralAuthUser->suppress(string, string)
#15 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/specials/SpecialCentralAuth.php(245): CentralAuthUser->adminLockHide(boolean, string, string, RequestContext)
#16 /srv/mediawiki/php-1.36.0-wmf.36/extensions/CentralAuth/includes/specials/SpecialCentralAuth.php(143): SpecialCentralAuth->doSubmit()
#17 /srv/mediawiki/php-1.36.0-wmf.36/includes/specialpage/SpecialPage.php(646): SpecialCentralAuth->execute(NULL)
#18 /srv/mediawiki/php-1.36.0-wmf.36/includes/specialpage/SpecialPageFactory.php(1382): SpecialPage->run(NULL)
#19 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(309): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#20 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(925): MediaWiki->performRequest()
#21 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(547): MediaWiki->main()
#22 /srv/mediawiki/php-1.36.0-wmf.36/index.php(53): MediaWiki->run()
#23 /srv/mediawiki/php-1.36.0-wmf.36/index.php(46): wfIndexMain()
#24 /srv/mediawiki/w/index.php(3): require(string)
#25 {main}

See: https://logstash.wikimedia.org/goto/e7fff370b4f79f729b1410d1a24f1a16
reqId: 0d095dc2-9959-498b-bdff-4f16aa3cf231

@daniel I was in looking at the production errors with @hashar today and it looks that the problem still happens - not very often though

That indeed looks like an issue, but unrelated to this ticket. It has nothing to do with PageRecord or ProperPageIdentity. It has to do with a UserIdentity object being used in the context of the wrong wiki. I'll file a ticket.

Apologies for the noise, got tricked by them identities!

Apologies for the noise, got tricked by them identities!

Not at all, thanks for noticing! Looks like we have a ticket for it already: T277687: Deprecated cross-wiki access to User. Expected: 'eswiki', Actual: the local wiki. Pass expected $wikiId. [Called from User::getId]