Page MenuHomePhabricator

Replace all calls to Revision::getRevisionText()
Open, NormalPublic

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
Declineddchen
OpenNone
OpenNone
DuplicateNone
OpenNone
ResolvedAbit
OpenNone
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedppelberg
ResolvedKrinkle
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedBstorm
OpenNone
OpenNone
StalledNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolveddaniel
ResolvedBPirkle
OpenNone
ResolvedAnomie
ResolvedAnomie
Resolvedtstarling
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
Resolveddaniel
ResolvedAnomie
ResolvedAnomie
ResolvedMarostegui
ResolvedAnomie
Resolvedtstarling
ResolvedAnomie
Resolveddaniel
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
Resolveddaniel
Resolveddaniel
Resolveddaniel
ResolvedNone

Event Timeline

daniel triaged this task as Normal priority.Jun 27 2018, 5:49 PM
daniel created this task.
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

daniel updated the task description. (Show Details)Mon, Jul 22, 2:59 PM
daniel updated the task description. (Show Details)
daniel updated the task description. (Show Details)Mon, Jul 22, 3:04 PM
daniel updated the task description. (Show Details)Mon, Jul 22, 3:23 PM
WDoranWMF raised the priority of this task from Normal to High.Mon, Jul 22, 10:05 PM
WDoranWMF lowered the priority of this task from High to Normal.Wed, Jul 24, 9:48 PM
WDoranWMF moved this task from MCR to mop on the Core Platform Team board.Fri, Jul 26, 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.