Page MenuHomePhabricator

Before 1.38: ParserOutput::getPageProperty() should return null, not false
Closed, ResolvedPublic

Description

The ::getPageProperty method, present in both ContentMetadataCollector and ParserOutput, was newly added in 1.38 (a rename of the old ::getProperty method) and it inherited the old "return false on error" behavior. That's a poor fit to modern PHP; we should change it to return null when a property is missing before 1.38 is released.

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+60 -16
mediawiki/extensions/JsonConfigmaster+2 -2
mediawiki/extensions/CollaborationKitmaster+1 -2
mediawiki/extensions/CollaborationKitmaster+2 -1
mediawiki/extensions/Wikibasemaster+17 -20
mediawiki/extensions/PageTriagemaster+1 -1
mediawiki/extensions/WikiSEOmaster+1 -2
mediawiki/extensions/WikiSEOmaster+2 -1
mediawiki/extensions/PageImagesmaster+3 -5
mediawiki/extensions/NoCatmaster+1 -2
mediawiki/extensions/OpenGraphMetamaster+5 -3
mediawiki/extensions/OpenGraphMetamaster+2 -1
mediawiki/extensions/PageLanguagemaster+6 -4
mediawiki/extensions/PageLanguagemaster+2 -1
mediawiki/extensions/TemplateDatamaster+8 -5
mediawiki/extensions/GeoCrumbsmaster+1 -2
mediawiki/extensions/MobileFrontendmaster+4 -5
mediawiki/extensions/Scoremaster+1 -2
mediawiki/skins/Cosmosmaster+1 -2
mediawiki/extensions/Description2master+6 -5
mediawiki/extensions/ExtraLanguageLinkmaster+8 -4
mediawiki/extensions/Interlanguagemaster+9 -6
mediawiki/extensions/MintyDocsmaster+11 -8
mediawiki/extensions/NoTitlemaster+1 -2
mediawiki/extensions/Wikibasemaster+4 -2
mediawiki/extensions/Scoremaster+2 -1
mediawiki/extensions/NoTitlemaster+2 -1
mediawiki/extensions/MobileFrontendmaster+2 -1
mediawiki/extensions/TemplateDatamaster+2 -1
mediawiki/extensions/PageImagesmaster+4 -2
mediawiki/extensions/GeoCrumbsmaster+2 -1
mediawiki/extensions/PageTriagemaster+1 -1
mediawiki/extensions/JsonConfigmaster+2 -1
mediawiki/extensions/NoCatmaster+2 -1
mediawiki/extensions/MintyDocsmaster+2 -1
mediawiki/extensions/Interlanguagemaster+4 -2
mediawiki/extensions/ExtraLanguageLinkmaster+4 -2
mediawiki/extensions/Description2master+4 -2
mediawiki/extensions/Genealogymaster+1 -1
mediawiki/skins/Cosmosmaster+2 -1
mediawiki/extensions/Genealogymaster+5 -7
mediawiki/extensions/Genealogymaster+4 -2
mediawiki/extensions/Wikibasemaster+13 -10
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 763356 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/JsonConfig@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763357 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/JsonConfig@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763358 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/MobileFrontend@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763359 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/MobileFrontend@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763360 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] WIP: Change return value of ParserOutput::getPageProperty() when property is missing

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

Change 763363 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageImages@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763364 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageImages@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763365 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageTriage@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763366 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageTriage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763367 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Score@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763368 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Score@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763371 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/TemplateData@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763372 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/TemplateData@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763375 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/CollaborationKit@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763376 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/CollaborationKit@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763378 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Description2@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763379 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Description2@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763385 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Interlanguage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763392 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/Genealogy@master] Handle null or false from ParserOutput::getPageProperty and ::getProperty

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

Change 763348 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763520 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/MintyDocs@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763521 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/MintyDocs@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763523 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/NoCat@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763524 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/NoCat@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763526 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/NoTitle@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763527 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/NoTitle@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763544 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/OpenGraphMeta@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763545 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/OpenGraphMeta@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763547 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageLanguage@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763548 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PageLanguage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763549 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/WikiSEO@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763550 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/WikiSEO@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763553 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/skins/Cosmos@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763554 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/skins/Cosmos@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763382 abandoned by C. Scott Ananian:

[mediawiki/extensions/Genealogy@master] Update uses of ParserOutput::getPageProperty() to handle new return value

Reason:

abandoned in favor of Icf65a589ab577620428b63ef21840adf4ddc6281

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

Change 763383 abandoned by C. Scott Ananian:

[mediawiki/extensions/Genealogy@master] ParserOutput::getPageProperty() now returns null when key is missing.

Reason:

Abandoned in favor of Icf65a589ab577620428b63ef21840adf4ddc6281

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

Change 763392 merged by jenkins-bot:

[mediawiki/extensions/Genealogy@master] Handle null or false from ParserOutput::getPageProperty and ::getProperty

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

Change 763553 merged by jenkins-bot:

[mediawiki/skins/Cosmos@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763356 merged by jenkins-bot:

[mediawiki/extensions/JsonConfig@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763351 merged by jenkins-bot:

[mediawiki/extensions/GeoCrumbs@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763365 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763378 merged by jenkins-bot:

[mediawiki/extensions/Description2@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763380 merged by jenkins-bot:

[mediawiki/extensions/ExtraLanguageLink@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763384 merged by jenkins-bot:

[mediawiki/extensions/Interlanguage@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763371 merged by jenkins-bot:

[mediawiki/extensions/TemplateData@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763363 merged by jenkins-bot:

[mediawiki/extensions/PageImages@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763358 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763520 merged by jenkins-bot:

[mediawiki/extensions/MintyDocs@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763523 merged by jenkins-bot:

[mediawiki/extensions/NoCat@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763843 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Wikibase@master] Update new uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763367 merged by jenkins-bot:

[mediawiki/extensions/Score@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763526 merged by jenkins-bot:

[mediawiki/extensions/NoTitle@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763843 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update new uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763360 merged by jenkins-bot:

[mediawiki/core@master] Change return value of ParserOutput::getPageProperty() when property is missing

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

Change 763359 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763527 merged by jenkins-bot:

[mediawiki/extensions/NoTitle@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763521 merged by jenkins-bot:

[mediawiki/extensions/MintyDocs@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763368 merged by jenkins-bot:

[mediawiki/extensions/Score@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763385 merged by jenkins-bot:

[mediawiki/extensions/Interlanguage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763381 merged by jenkins-bot:

[mediawiki/extensions/ExtraLanguageLink@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763379 merged by jenkins-bot:

[mediawiki/extensions/Description2@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763554 merged by jenkins-bot:

[mediawiki/skins/Cosmos@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763372 merged by jenkins-bot:

[mediawiki/extensions/TemplateData@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763352 merged by jenkins-bot:

[mediawiki/extensions/GeoCrumbs@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763364 merged by jenkins-bot:

[mediawiki/extensions/PageImages@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763549 merged by jenkins-bot:

[mediawiki/extensions/WikiSEO@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763550 merged by jenkins-bot:

[mediawiki/extensions/WikiSEO@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763547 merged by jenkins-bot:

[mediawiki/extensions/PageLanguage@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763548 merged by jenkins-bot:

[mediawiki/extensions/PageLanguage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763544 merged by jenkins-bot:

[mediawiki/extensions/OpenGraphMeta@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763545 merged by jenkins-bot:

[mediawiki/extensions/OpenGraphMeta@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763524 merged by jenkins-bot:

[mediawiki/extensions/NoCat@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763349 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763366 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763375 merged by Jforrester:

[mediawiki/extensions/CollaborationKit@master] Update uses of ParserOutput::getPageProperty() to handle new return value

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

Change 763376 merged by Jforrester:

[mediawiki/extensions/CollaborationKit@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

Change 763357 merged by jenkins-bot:

[mediawiki/extensions/JsonConfig@master] ParserOutput::getPageProperty() now returns null when key is missing.

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

/** You need to use getPageProperties() to an explicitly-set null value. */

@cscott This suggests that null, and previously booleans, are in fact supported to be stored and roundtripped as page properties. Upto this point I have understood this not to be the case. They are stored as strings with no built-in encoding (besides the adhoc sql encoding in PagePropsTable, which was recently factored out to there from a less obvious place). This suggests a null value or boolean false will not come out as such, but rather as empty string or string "0".

Is that still the case?

Removing MW-1.38 tag as the parent task has this already.

/** You need to use getPageProperties() to an explicitly-set null value. */

@cscott This suggests that null, and previously booleans, are in fact supported to be stored and roundtripped as page properties. Upto this point I have understood this not to be the case. They are stored as strings with no built-in encoding (besides the adhoc sql encoding in PagePropsTable, which was recently factored out to there from a less obvious place). This suggests a null value or boolean false will not come out as such, but rather as empty string or string "0".

Is that still the case?

One issue here is that is a lot of confusion between "page properties as stored in the database" and "page properties as stored in parsercache". The ParserCache stores all fields as JSON-serialized data (nowadays, used to use PHP serialization, but no difference for this point) -- that means if you set a non-string value in your ParserOutput, you can later get that same non-string value back out of the ParserOutput, whether the ParserOutput was generated on-demand or fetched from ParserCache. This is in fact part of the explicit tests in ParserCacheSerializationTestCases, where boolean/null/number/string/array/map are the types tested.

There's a separate process which takes the page properties from ParserOutput and actually stuffs them in a database. That's done in the deferred LinksUpdate task, AFAICT, and there is only rudimentary encoding: PagePropsTable.php::encodeValue() encodes booleans as '0' and '1' and null as '' and stringifies anything else. If you later fetch the page properties using PageProps.php (as opposed to from the ParserCache) that's what you'll see.

I'll make a patch to improve the documentation around this, especially since I noticed a typo in the comment you'd flagged on gerrit. :)

cscott claimed this task.

@Krinkle I've opened T305158 to cover improving current documentation and perhaps gradually tightening the API here.

I'm going to close this particular issue, however.