Page MenuHomePhabricator

Replace all calls to Revision::getRevisionText()
Closed, ResolvedPublic2 Estimated 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

StatusSubtypeAssignedTask
ResolvedNone
ResolvedZabe
Resolveddaniel
Resolveddaniel
ResolvedBPirkle
ResolvedCCicalese_WMF
Resolveddaniel
ResolvedNone
ResolvedNone
ResolvedCCicalese_WMF
Resolveddaniel
Resolved Pchelolo
Resolved Pchelolo
Resolved Pchelolo
Resolved Pchelolo
Resolved Pchelolo
Resolveddaniel
Resolveddaniel
Resolveddaniel
ResolvedNone

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
daniel triaged this task as Medium priority.Jun 27 2018, 5:49 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

daniel updated the task description. (Show Details)
WDoranWMF raised the priority of this task from Medium to High.Jul 22 2019, 10:05 PM
WDoranWMF lowered the priority of this task from High to Medium.Jul 24 2019, 9:48 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.

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?

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

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 Medium to High.Sep 30 2019, 2:25 PM

Bumping to high, because this blocks 1.34

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

Reason:
obsolete

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

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