Page MenuHomePhabricator

importTextFiles.php not setting content model and thus failing on MW 1.35
Closed, ResolvedPublicBUG REPORT

Description

When trying to import text files using maintenance/importTextFiles.php, I get the following error:

TypeError from line 95 of /path/to/includes/content/ContentHandlerFactory.php: Argument 1 passed to MediaWiki\Content\ContentHandlerFactory::getContentHandler() must be of the type string, null given, called in /path/to/includes/content/ContentHandler.php on line 272

After a bit of debugging, I could fix it for my use case by adding the following at line 140:

$rev->setModel( 'wikitext' )

I could send this as a patch, but I suspect the proper way to do it would be to auto-detect the content model out of the filetype of the imported files, or at least add a parameter for specifying the content model, rather than hardcoding 'wikitext', yes?

Event Timeline

Reedy added a subscriber: Reedy.

I could send this as a patch, but I suspect the proper way to do it would be to auto-detect the content model out of the filetype of the imported files, or at least adding a parameter for specifying the content model, rather than hardcoding 'wikitext', yes?

AFAIK MW doesn't have a way to "detect" NS. It's either set manually, or using a default content model for a NS

An option defaulting to wikitext probably makes sense though, yeah.

It would look like swapping setText for setContent would be better, as setModel is already deprecated (though not hard deprecated yet).

Full stack trace is

Importing 1 pages...
[0533533b8d8ca45ffa2c678d] [no req]   TypeError from line 95 of /var/www/wiki/mediawiki/core/includes/content/ContentHandlerFactory.php: Argument 1 passed to MediaWiki\Content\ContentHandlerFactory::getContentHandler() must be of the type string, null given, called in /var/www/wiki/mediawiki/core/includes/content/ContentHandler.php on line 272
Backtrace:
#0 /var/www/wiki/mediawiki/core/includes/content/ContentHandler.php(272): MediaWiki\Content\ContentHandlerFactory->getContentHandler()
#1 /var/www/wiki/mediawiki/core/includes/import/WikiRevision.php(295): ContentHandler::getForModelID()
#2 /var/www/wiki/mediawiki/core/maintenance/importTextFiles.php(137): WikiRevision->setText()
#3 /var/www/wiki/mediawiki/core/maintenance/doMaintenance.php(106): ImportTextFiles->execute()
#4 /var/www/wiki/mediawiki/core/maintenance/importTextFiles.php(213): require_once(string)
#5 {main}

By auto-detecting the content model out of the filetype, I meant that if the imported file is named foo.css, set the content model to CSS, if it's bar.json, set it to JSON, if it's baz.txt, set it to wikitext, etc. Thanks!

By auto-detecting the content model out of the filetype, I meant that if the imported file is named foo.css, set the content model to CSS, if it's bar.json, set it to JSON, if it's baz.txt, set it to wikitext, etc. Thanks!

I think that'd would probably means mapping all core and non core content models and trying to guess... maybe it can be done for core ones only though.

Considering MW core has a "TextContent" (which is the parent class of WikitextContent)... Can you really necessarily map txt to wikitext?

MW core only has a handful. You could add a hook or use an attribute for the mapping in extensions if we really wanted. But then numerous extension ones won't necessarily map to a known/common file type.

I guess it's one of those that does anyone really care/need this functionality? Is anyone likely to be importing a directory/list of mixed content type files? Is it just extra unnecessary complexity?

I think a parameter accepting a content type, some validation... And pass that through is sufficient

Indeed .txt would have to forgo 'textcontent' and go with 'wikitext' here... or the script will become unusable. I believe TextContent is rarely used used or even known to exist. So I don't think it will be a problem to just explain that.

A parameter option can be used like --guess-model to signify that one wants the script to make that guess with understanding that .txt and no extension files will default to wikitext. It would be slightly better than status quo where everything is defaulting to wikitext and no way to control that.

This comment was removed by Sophivorus.

Change 640273 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] importTextFiles.php: Replace deprecated WikiRevision:setText()

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

Change 640493 had a related patch set uploaded (by Reedy; owner: Ammarpad):
[mediawiki/core@REL1_35] importTextFiles.php: Replace deprecated WikiRevision:setText()

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

Change 640493 merged by jenkins-bot:
[mediawiki/core@REL1_35] importTextFiles.php: Replace deprecated WikiRevision:setText()

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

Change 640273 merged by jenkins-bot:
[mediawiki/core@master] importTextFiles.php: Replace deprecated WikiRevision:setText()

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

Ammarpad claimed this task.

Same error here when using maintenance/importDump.php with the --uploads switch and a dump file that includes files. Exact same command and dump file succeeds if --uploads is not used. Should this have its own issue?

Edit: I should add that the dump was exported on 1.27.5 and is being imported on 1.35.3.

Same error here when using maintenance/importDump.php with the --uploads switch and a dump file that includes files. Exact same command and dump file succeeds if --uploads is not used. Should this have its own issue?

Edit: I should add that the dump was exported on 1.27.5 and is being imported on 1.35.3.

Yes, this should be filed as a seperate issue. This was filed about importTextFiles.php, and has been fixed and resolved (months ago).

I can't see it obviously having been fixed on master, but it may have.

Please include full error messages/stack traces as appropriate