Page MenuHomePhabricator

page_props needs an internal API
Closed, ResolvedPublic

Description

There doesn't seem to be a uniform way to query or set page properties in MW. Cindy has already developed an interface for SemanticTitle that she would like to include in core.

I've found a few extensions that use the page_props table and could probably benefit from a uniform way to access the information. I think this shows a use that needs to be met. The extensions are:

Event Timeline

MarkAHershberger updated the task description. (Show Details)
MarkAHershberger raised the priority of this task from to Needs Triage.
MarkAHershberger assigned this task to cicalese.
MarkAHershberger added a subscriber: MarkAHershberger.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 13 2015, 3:03 PM

There is ParserOutput::setProperty and getProperty which will save the props in the database

These functions are only useful for the current page and only if you have access to a ParserOutput object. If you would like to read a property for a page other than the page currently being parsed, you cannot do so.

Also, the get function does not access the database but only returns the value of a property already set for the page in the current parser invocation. This value is stored in a data structure in ParserOutput. It has been found in some extensions that this value does not always accurately return the property value, so extensions have gotten around this by directly accessing the database. The goal would be to add a class extensions could use for this database access to abstract the extensions from the page_props table structure and direct database access.

Finally, in SemanticTitle, we found that the write to the database was deferred, so although we do indirectly use the ParserOutput::setProperty function by calling ParserOutput::setDisplayTitle, the displaytitle value was not getting set soon enough. When saving the page, the value of displaytitle was not updated until some time later. So, we directly write displaytitle to the database. If there is a better way to accomplish this, I am certainly open to suggestions.

Change 246246 had a related patch set uploaded (by Cicalese):
added page_props access class; set link text from displaytitle page property

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

Change 250039 had a related patch set uploaded (by Cicalese):
added page property accessors to Title; cached access for displaytitle page property

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

Change 246246 abandoned by Cicalese:
added page_props access class; set link text from displaytitle page property

Reason:
This patch has been replaced by https://gerrit.wikimedia.org/r/#/c/250039/.

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

https://gerrit.wikimedia.org/r/#/c/250039/ replaces https://gerrit.wikimedia.org/r/#/c/246246/. Instead of creating an additional class to access the page properties, this new patch adds that functionality to the Title class. The new patch does not provide any way to set the page properties, since that is functionality owned by the parser. The new patch does not provide functionality to change link text from the displaytitle page property. If I decide to move forward with that suggestion, I will do so in an RFC. The new patch does, however, add efficient access to displaytitle property through the LinkCache.

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptNov 6 2015, 2:42 PM
Lydia_Pintscher added a subscriber: aude.

I'm still looking for code review of https://gerrit.wikimedia.org/r/#/c/250039/. Any input would be appreciated.

Lydia_Pintscher moved this task from incoming to monitoring on the Wikidata board.Nov 13 2015, 2:32 PM

Change 263581 had a related patch set uploaded (by Cicalese):
Add page_props table access class

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

Change 250039 abandoned by Cicalese:
added page property accessors to Title; cached access for displaytitle page property

Reason:
After discussions regarding this patch at the Developer Summit, I am abandoning this patch and have proposed a new patch (https://gerrit.wikimedia.org/r/#/c/263581) in its place that uses the DAO approach. I would appreciate review of the new patch. Thanks to all for your feedback and insights!

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

Change 263581 had a related patch set uploaded (by Cicalese):
Add page_props table access class

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

Change 263581 merged by jenkins-bot:
Add page_props table access class

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

I believe that this task can now be closed. Other tasks may be created to convert other core code and extensions to use PageProps, but PageProps has been merged.

cicalese closed this task as Resolved.Feb 17 2016, 10:56 PM