Page MenuHomePhabricator

Replace all calls to Revision::getRevisionText()
Closed, ResolvedPublic2 Story Points

Description

Since Revision::getRevisionText() uses the old_text field which is obtained by joining rev_text_id or ar_text_id against text.old_id, it can no longer be used with queries based on getQueryInfo() or getSelectFields(), once rev_text_id and ar_text_id go away.

Code that interacts with the text table directly should use SqlBobStore::expandBlob instead. Code that operates on the revision table should use the appropriate methods in RevisionStore and RevisionRecord to access content.

See Uses of Revision::getRevisionText() in extensions.

DOD: This task should be considered done when any code calling Revision::getRevisionText() has been, in core and all extensions deployed by Wikimedia, as well as extensions bundled with MediaWiki releases. Other extensions and external tools may be left to be updated later.

Extensions to fix:

  • Translate (T228675)
  • ActiveAbstract

Related Objects

StatusAssignedTask
OpenNone
OpenNone
OpenNone
Resolveddaniel
ResolvedBPirkle
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedPchelolo
ResolvedPchelolo
ResolvedPchelolo
ResolvedPchelolo
OpenPchelolo
ResolvedPchelolo
Resolveddaniel
Resolveddaniel

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
daniel triaged this task as Normal priority.Jun 27 2018, 5:49 PM
Addshore moved this task from incoming to monitoring on the Wikidata board.Sep 19 2018, 7:09 AM
greg added a project: Multimedia.Mar 7 2019, 10:59 PM

Change 524572 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Remove usage of getRevisionText from maintenance scripts

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

Change 524591 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/extensions/ActiveAbstract@master] Remove usages of getRevisionText.

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

Change 524593 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/extensions/Translate@master] Remove direct access to text table

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

Addshore removed a subscriber: Addshore.Jul 20 2019, 3:09 AM
daniel updated the task description. (Show Details)Jul 22 2019, 2:59 PM
daniel updated the task description. (Show Details)
daniel updated the task description. (Show Details)Jul 22 2019, 3:04 PM
daniel updated the task description. (Show Details)Jul 22 2019, 3:23 PM
WDoranWMF raised the priority of this task from Normal to High.Jul 22 2019, 10:05 PM
WDoranWMF lowered the priority of this task from High to Normal.Jul 24 2019, 9:48 PM
WDoranWMF moved this task from MCR to mop on the Core Platform Team board.Jul 26 2019, 6:39 PM

Ran the following with old and new code for abstracts:

/usr/bin/php7.2 /srv/mediawiki/multiversion/MWScript.php dumpBackup.php --wiki=anwiki /srv/v/mediawiki/php-1.34.0-wmf.15/extensions/ActiveAbstract/includes/AbstractFilter.php  --full --report=1 --output=file:/mnt/dumpsdata/temp/dumpsgen/abstracts-anwiki-cr-testing.txt.test  --filter=noredirect --filter=abstract --skip-header --start=36142 --skip-footer --end 36150

/usr/bin/php7.2 /srv/mediawiki/multiversion/MWScript.php dumpBackup.php --wiki=enwiki /srv/mediawiki/php-1.34.0-wmf.15 --plugin=AbstractFilter:/srv/mediawiki/php-1.34.0-wmf.15/extensions/ActiveAbstract/includes/AbstractFilter.php  --full --report=1 --output=file:/mnt/dumpsdata/temp/dumpsgen/abstracts-enwiki-cr-testing.txt.prod  --filter=noredirect --filter=abstract --skip-header --start=1240149 --skip-footer --end 1240150

/usr/bin/php7.2 /srv/mediawiki/multiversion/MWScript.php dumpBackup.php --wiki=dewikiversity /srv/mediawiki/php-1.34.0-wmf.15 --plugin=AbstractFilter:/srv/mediawiki/php-1.34.0-wmf.15/extensions/ActiveAbstract/includes/AbstractFilter.php --current --report=1 --output=file:/mnt/dumpsdata/temp/dumpsgen/abstracts-dewv-cr-testing.txt.test  --filter=noredirect --filter=abstract --skip-header --start=47279 --skip-footer --end 47300

The first two have identical output with and without the change.
The last has the following diff:

dumpsgen@snapshot1008:~$ diff /mnt/dumpsdata/temp/dumpsgen/abstracts-dewv-cr-testing.txt.good /mnt/dumpsdata/temp/dumpsgen/abstracts-dewv-cr-testing.txt.test
4c4
< <abstract serialization-error="" />
---
> <abstract />
5a6
> <sublink linktype="nav"><anchor>Schulprojekt:Physik Sekundarstufe I</anchor><link>https://de.wikiversity.org/wiki/Kategorie:Schulprojekt:Physik_Sekundarstufe_I</link></sublink>

I think we can live with the empty tag; some pages produce that already. I need to look at the link data and see if that makes sense.

The category links are the fallback that was designed, so this is a net positive. Going to go update the CR on the patch.

WDoranWMF set the point value for this task to 2.Tue, Sep 17, 4:35 PM
Clarakosi assigned this task to daniel.Wed, Sep 18, 6:29 PM
Clarakosi moved this task from Ready to Doing on the Core Platform Team Workboards (Purple) board.
Clarakosi added a subscriber: Clarakosi.

Moving this task away from the ready column as @daniel already has patches for this that either need to be updated or needs review

The last usage of the method within core (apart from maintenance scripts) is in ApiQueryDeletedrevs which was deprecated since 1.25.

Quick search reveals that it's still used in pywikibot ( there's a ticket about that T75370) but not used anywhere else. We can either quick-fix it replacing it with smith like $revisionStore->newRevisionFromArchiveRow( $row )->getContent( SlotRecord::MAIN )->serialize() or just get bold and remove the API module, or fix up pywikibot and remove the API module.

It's been deprecated for a long time, so I'd be for removing the module. @Anomie what do you think?

Anomie added a comment.EditedWed, Sep 18, 9:14 PM

See T221869: Remove deprecated ApiQueryDeletedRevs for what's needed to remove the module from a Wikimedia usage perspective. The status seems unchanged since that task was created.

TL;DR: Tell 6–10 users that their stuff is going to break soon (ideally with a deadline), have me send a mailing list announcement with the deadline, then go ahead and break it.

Change 537759 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Remove Revision::getRevisionText from ApiQueryDeletedrevs

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

Change 537759 merged by jenkins-bot:
[mediawiki/core@master] Remove Revision::getRevisionText from ApiQueryDeletedrevs

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

Change 538083 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Remove Revision::getRevisionText from migrateArchiveText

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

daniel removed daniel as the assignee of this task.Fri, Sep 20, 12:12 PM

Change 538314 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Remove Revision::getRevisionText and gated pre-MCR schema access

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

Change 538083 merged by jenkins-bot:
[mediawiki/core@master] Remove Revision::getRevisionText from migrateArchiveText

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

Change 538314 merged by jenkins-bot:
[mediawiki/core@master] Remove Revision::getRevisionText and gated pre-MCR schema access

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

Change 539607 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Hard deprecate Revision::getRevisionText() method

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

daniel raised the priority of this task from Normal to High.Mon, Sep 30, 2:25 PM

Bumping to high, because this blocks 1.34

daniel updated the task description. (Show Details)Mon, Sep 30, 2:37 PM

Change 524572 abandoned by Daniel Kinzler:
Remove usage of getRevisionText from maintenance scripts

Reason:
obsolete

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

Pchelolo closed this task as Resolved.Mon, Oct 7, 7:07 PM
Pchelolo claimed this task.

Translate extension is done, all usages has been either removed or gated. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/539607 hard deprecates the method.

Change 539607 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecate Revision::getRevisionText() method

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