Page MenuHomePhabricator
Create Task
Maniphest T155139

'ContentHandlerForModelID' hook allows creating of handlers that aren't registered
Closed, ResolvedPublic
Actions

Description

Content handlers are supposed to be registered in $wgContentHandlers, so they can be enumerated by ContentHandler::getContentModels() (for the API and for other purposes). But the 'ContentHandlerForModelID' hook allows for creating handlers in ContentHandler::getForModelID() that are not registered.

The end result is a handler that mostly exists, but can't be used with the API or Special:ChangeContentModel, doesn't get tested by ContentHandlerSanityTest, and doesn't get its getFieldsForSearchIndex() method called by SearchEngine.

One possible solution would be to only call the hook for entries in $wgContentHandlers where the value is null, to allow the handlers to be created on demand with dependency injection while still being properly registered.

Extensions (mis)using this hook appear to be MediaWiki-extensions-WikibaseRepository and JsonConfig.

Details

SubjectRepoBranchLines +/-
Register ContentHandlers in $wgContentHandlers.mediawiki/extensions/Wikibasemaster+16 -34
Add GetContentModels hook for Wikibasemediawiki/extensions/Wikibasemaster+11 -0
Add GetContentModels hook to allow extensions to enumerate dynamic content models.mediawiki/coremaster+17 -1
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedWikidata-bugsT77898 query taking 10s: TermSqlIndex::getMatchingIDs
 OpenNoneT46529 Wikidata search problems (tracking)
 ResolvedaudeT110648 [Bug] high-ranking items seemed to have dropped significantly in Special:Search results for wikidata
 ResolvedSmalyshevT119067 Adjust rescoring config for Wikidata to consider sitelink count
 ResolvedaudeT119066 Add sitelink count to search index for Wikidata
 ResolvedNoneT120217 [Task] Add CirrusSearch and Elastica as dependencies of Wikibase on Jenkins
 InvalidNoneT161334 Stemming for item suggestions, e.g. "the" vs. no "the" on Wikidata
 ResolvedSmalyshevT78157 [Story] Use ElasticSearch for entity search on wikidata.org
 ResolvedLydia_PintscherT88535 [Task] Investigate preconditions & constraints for using Elastic directly
 DuplicateSmalyshevT117520 Index Wikidata labels, aliases and descriptions as separate fields in ElasticSearch
 DuplicateNoneT117522 Create standard "completion suggestion" API for Search
 DeclinedNoneT120089 Add an internal completion or suggestions API to core SearchEngine
 DeclinedNoneT170392 Create gadget that enables the use of the elastic search backend for the entity selector
 ResolveddanielT170400 Define metrics for search result quality for the entity selector widget on wikidata.
 ResolvedLydia_PintscherT170405 Manually evaluate cirrus based entity search on test system
 OpenNoneT170547 Metrics to evaluate new search for item suggestor
 DeclinedNoneT170549 Provide A/B test for item suggestor
ResolvedSmalyshevT162292 Reindex wikidata to pick up labels/descriptions mappings
 ResolvedSmalyshevT175199 Index certain statements for Wikidata items
 DeclinedNoneT141813 Add full-text search support to Query Service
 ResolvedSmalyshevT125500 [Epic] Index Wikidata labels and descriptions as separate fields in ElasticSearch
 ResolvedSmalyshevT155139 'ContentHandlerForModelID' hook allows creating of handlers that aren't registered
 ResolvedSmalyshevT157604 Make Wikibase register all its content handlers
ResolvedSmalyshevT157626 ContentHandlerSanityTest assumes every handler can create empty content, but Wikibase can't
 ResolveddcausseT160926 Make noop script be able replace whole fields with nested subfields
 ResolveddcausseT166589 Update wikidata code to take advantage of nested fields noop script
 ResolvedSmalyshevT178851 Use label & description index for fulltext search
 ResolvedSmalyshevT176903 Index wikidata descriptions
 ResolvedSmalyshevT180169 Make list of languages where using stemmed analyzer for Wikibase is beneficial
 ResolvedSmalyshevT182271 Wikidata fulltext search should handle search syntax properly
 ResolveddcausseT182293 Tune wikidata fulltext search similarity parameters

Event Timeline

Anomie created this task.Jan 11 2017, 11:15 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 11 2017, 11:15 PM
Anomie added subscribers: Yurik, TheDJ.Jan 11 2017, 11:20 PM
daniel added a project: User-Daniel.Jan 12 2017, 7:49 AM
TheDJ added a subtask: T155144: unable to save json datasets using api.Jan 12 2017, 7:31 PM
daniel added a comment.Jan 12 2017, 8:57 PM

One thing to consider is that we may support legacy content models for old revisions that we do want to allow for new revisions.

In any case, an alternative to putting null entries into $wgContentHandlers would be to add another hook, GetContentModels, which is called from ContentHandler::getContentModels(). This isn't very pretty either, but if an extension is implementing one hook, it can just as well implement another.

On a higher level, this problem is caused by the fact that extension.json cannot add closures to $wgContentHandlers. But it could add a static factory method. We should think about introducing a syntax into extension.json that lets us reference callbacks defined in some kind of wiring file. On the other hand - such a wiring file could just as well be a php lass with static callback methods, which could easily be used from extension.json.

TheDJ added a comment.Jan 13 2017, 1:21 AM

@daniel BTW: this seems to have some similarities to [[mw:Manual:Hooks/ResourceLoaderRegisterModules]], which allows extensions to do conditional registration of resourceloader modules.

daniel added a subscriber: Smalyshev.Jan 26 2017, 1:14 AM
Smalyshev awarded a token.Jan 26 2017, 1:16 AM
Smalyshev added a comment.Jan 26 2017, 1:20 AM

I don't care too much if legacy ones do not show up in wgContentHandlers. In fact, I'd be completely happy if they didn't since then I don't need to keep search fields for them (even though empty mapped fields aren't super-expensive AFAIK). But it's be very nice if ContentHandler::getContentModels() showed currently used ones.

On a higher level, this problem is caused by the fact that extension.json cannot add closures to $wgContentHandlers. But it could add a static factory method.

Yes, Wikibase creates them by a static hook anyway, so it's just a question of who calls the hook and when.

Smalyshev added a parent task: T125500: [Epic] Index Wikidata labels and descriptions as separate fields in ElasticSearch.Jan 26 2017, 1:22 AM
daniel triaged this task as High priority.Jan 30 2017, 11:26 AM
daniel moved this task from Inbox to To Do on the User-Daniel board.
gerritbot subscribed.Jan 31 2017, 5:32 AM

Change 335188 had a related patch set uploaded (by Smalyshev):
Add GetContentModels hook to allow extensions to enumerate dynamic content models.

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

gerritbot added a project: Patch-For-Review.Jan 31 2017, 5:32 AM
gerritbot added a comment.Jan 31 2017, 5:36 AM

Change 335189 had a related patch set uploaded (by Smalyshev):
Add GetContentModels hook for Wikibase

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

daniel moved this task from To Do to Doing on the User-Daniel board.Jan 31 2017, 6:01 PM
gerritbot added a comment.Jan 31 2017, 7:47 PM

Change 335270 had a related patch set uploaded (by Daniel Kinzler):
Register ContentHandlers in $wgContentHandlers.

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

gerritbot added a comment.Feb 8 2017, 8:55 AM

Change 335188 merged by jenkins-bot:
Add GetContentModels hook to allow extensions to enumerate dynamic content models.

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

ReleaseTaggerBot added projects: MW-1.29-release-notes, MW-1.29-release (WMF-deploy-2017-02-14_(1.29.0-wmf.12)).Feb 8 2017, 9:00 AM
Smalyshev closed this task as Resolved.Feb 8 2017, 7:55 PM
Smalyshev claimed this task.
Smalyshev mentioned this in T157604: Make Wikibase register all its content handlers .EditedFeb 8 2017, 7:58 PM
Smalyshev created subtask T157604: Make Wikibase register all its content handlers .

I've split off Wikibase part to a new ticket. T155144 is for JsonConfig part of the same.

Smalyshev created subtask T157626: ContentHandlerSanityTest assumes every handler can create empty content, but Wikibase can't.Feb 8 2017, 10:03 PM
Smalyshev removed a subtask: T157626: ContentHandlerSanityTest assumes every handler can create empty content, but Wikibase can't.Feb 8 2017, 10:14 PM
gerritbot added a comment.Feb 16 2017, 6:32 AM

Change 335189 abandoned by Smalyshev:
Add GetContentModels hook for Wikibase

Reason:
Replaced by I73ab3f156427abe2

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

gerritbot added a comment.Feb 16 2017, 10:44 PM

Change 335270 merged by jenkins-bot:
Register ContentHandlers in $wgContentHandlers.

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

Smalyshev closed subtask T157604: Make Wikibase register all its content handlers as Resolved.Feb 17 2017, 9:45 PM
thiemowmde added projects: Wikidata, MediaWiki-extensions-WikibaseRepository.Feb 20 2017, 9:41 AM
thiemowmde moved this task from incoming to in progress on the Wikidata board.
Smalyshev removed a subtask: T155144: unable to save json datasets using api.Dec 13 2017, 12:54 AM
• Mholloway mentioned this in T259126: Warning: Locally stored wiki page has unsupported content model (from JsonConfig).Jul 29 2020, 9:20 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:40 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL