Page MenuHomePhabricator

GraphViz repeatedly reuploads GraphVizExtensionDummy dot files
Closed, ResolvedPublic

Description

Recent changes is getting populated by various users repeatedly deleting and re-uploading File:File graph GraphVizExtensionDummy dot.svg ‎

Event Timeline

Prod assigned this task to Welterkj.
Prod raised the priority of this task from to Needs Triage.
Prod updated the task description. (Show Details)
Prod added a project: GraphViz.
Prod subscribed.

+1
in Mediawiki 1.27.1-3 + Graphviz 1.6.1 (6281429) 00:36, 5 May 2016

What can we do to help debug this problem

Thanks

I am afraid not much. This is a design problem, not a traceable bug, setting $wgShowExceptionDetails = true;
$wgShowDBErrorBacktrace = true; does not produce anything. Until Welterkj finds time for it or somebody else with deep knowledge of extension programming (I am not the one) who has time and is allowed to produce fix by the developer, we are stuck. It is the only extension that produces kind of relations between entities I need so I installed, tested, rejoiced and disabled (unfortunately such polution of recent changes and categories is not acceptable). On top of that under MW 1.28 this extension is a victim of Incompatibility with AbuseFilter and SpamBlackList extensions (see Extension_talk:GraphViz)

We are using this extension and it's great except for this issue which is cluttering our change history. I have tracked the issue down to the function called initDummyFilePages within GraphViz_body.php. When saving any changes to any page (that don't even use GraphViz) this function is executed, and the dummy file is removed and recreated. The problem is I don't understand the purpose of this initDummyFilePages function, so not sure about making changes to it. Would be great if someone with knowledge of what this function does could help.

+1

Setup

  • MediaWiki 1.27.1 (8c593e7) 22:49, 26 October 2016
  • PHP 7.0.15-0ubuntu0.16.04.4 (apache2handler)
  • MySQL 5.7.17-0ubuntu0.16.04.1
  • Graphviz 1.6.1 (1515b85) 19:26, 16 June 2015

I've written a fix that is actually a design change. The fix performs the upload of graph or message sequence chart images without creating a file page. This approach eliminates the need for dummy file pages and other awkward techniques. Creation of these file pages may be considered a feature of the extension that will be lost. On balance, they seem to have caused more pain than good (both in the implementation and in the user community) and I now think it is best to abandon that feature. The fix needs some cleanup and additional testing so stay tuned.

Background: When I re-wrote this extension for version 1.0.0 I made a conscious design decision to use core upload functionality in the most unadulterated way possible. However, such uploads necessarily created file pages which were extremely tricky to handle in the context of a parser function which the GraphViz extension essentially is. For example, I had to create "dummy" file pages at "safe" points (outside the parser function) so that I could upload real graph images on top of them when inside the parser function. The alternative, which I'm now taking, is to customize the upload so that graph images are stored in the wiki file repository and image properties are stored in the database but corresponding file pages are not created. A consequence of this is that users will lose the ability to directly manage (e.g. link or delete) graph images via the file page. The extension has logic to detect when a graph image is no longer in use and delete it (e.g. when a page with graph source is deleted). That will have to be sufficient.

@Welterkj Great news. Thanks for authoring the upcoming 2.0 release. Very much appreciated! File handling via the extension without involving the File namespace is fine with me since the embedded graphs will still be downloadable.

I may be late with the idea; it might be useful to check the extension:Score. It has a very similar work flow: it takes as an input metalanguage (lilypond or abc), it processes the input by using the external packages (lilypond/abc) and displays the image created by them. On top of that it creates the page property (suits me a lot) score for a page using the lilypond/abc.

Change 344483 had a related patch set uploaded (by Welterkj):
[mediawiki/extensions/GraphViz@master] Redesign to eliminate creation of file pages for graph images.

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

Please try the provided patch set and let me know if the redesign solves your problem.

I've installed the patch, but now when I try to save a page that contains GraphViz, the following error is generated:
Fatal error: Call to undefined method UploadLocalFile::getName() in /opt/bitnami/apps/mediawiki/htdocs/includes/upload/UploadBase.php on line 395

I'm using MediaWiki v1.28.0

Does it work for anyone else?

Unfortunately, retest on both 1.28 and 1.27 gives the same result
... includes/upload/UploadBase.php: Call to undefined method UploadLocalFile::getName()
on line 395 for 1.28 and line 386 for 1.27.

I switched to 1.28, recreated the problem and updated the patch to address it. Please try again.

Retested MW 1.27.1 - OK, works like a charm. Not a trace in Recent changes except the changed page.

MW 1.28 - MWException from line 6012 of /usr/share/webapps/mediawiki/includes/parser/Parser.php: Parser state cleared while parsing. Did you call Parser::parse recursively?
But that might be connected with MW core as I had problems with quite a few extensions when testing migration from 1.27 to 1,28, that's why stayed in the production with 1.27.

There shouldn't be any recursive calls to Parser::parse. Can you provide
the exception traceback and the wikitext?

This new version appears to be working perfectly for us (on 1.28), no errors and it has resolved the issue. Thank you very much.

Wikitext:
<graphviz border='frame' format='png' caption='Teacher - pupil'>
digraph Teacher_pupils {teacher->pupil1; teacher->pupil2; teacher->pupil3; teacher->pupil4; t->p;}
</graphviz>

<graphviz border='frame' format='png' caption='Teacher - pupil relationships'>
digraph Teacher_pupil_2 {Corelli->Paganini; Corelli->Lully; Lully->Martini; Paganini->Martini; Bach->All;}
</graphviz>

Traceback:

[ebd73692ca1fb6233c91297b] /mediawiki/index.php?title=User:Sysop/sandbox/tests&action=submit MWException from line 6012 of .../mediawiki/includes/parser/Parser.php: Parser state cleared while parsing. Did you call Parser::parse recursively?

Backtrace:

#0 .../mediawiki/includes/parser/Parser.php(414): Parser->lock()
#1 .../mediawiki/includes/content/WikitextContent.php(330): Parser->parse(string, Title, ParserOptions, boolean, boolean, NULL)
#2 .../mediawiki/includes/content/AbstractContent.php(497): WikitextContent->fillParserOutput(Title, NULL, ParserOptions, boolean, ParserOutput)
#3 .../mediawiki/extensions/SpamBlacklist/SpamBlacklistHooks.php(224): AbstractContent->getParserOutput(Title, NULL, ParserOptions)
#4 .../mediawiki/includes/Hooks.php(195): SpamBlacklistHooks::onUploadVerifyUpload(UploadFromLocalFile, User, array, string, string, NULL)
#5 .../mediawiki/extensions/GraphViz/UploadLocalFile.php(522): Hooks::run(string, array)
#6 .../mediawiki/extensions/GraphViz/UploadLocalFile.php(247): UploadFromLocalFile->performUpload2(string)
#7 .../mediawiki/extensions/GraphViz/GraphViz_body.php(686): UploadLocalFile::uploadWithoutFilePage(UploadFromLocalFile, string, string, boolean)
#8 .../mediawiki/extensions/GraphViz/GraphViz_body.php(378): GraphViz::render(string, array, Parser, PPFrame_DOM)
#9 .../mediawiki/includes/parser/Parser.php(3851): GraphViz::graphvizParserHook(string, array, Parser, PPFrame_DOM)
#10 .../mediawiki/includes/parser/Preprocessor_DOM.php(1260): Parser->extensionSubstitution(array, PPFrame_DOM)
#11 .../mediawiki/includes/parser/Parser.php(2917): PPFrame_DOM->expand(DOMElement, integer)
#12 .../mediawiki/includes/parser/Parser.php(1265): Parser->replaceVariables(string)
#13 .../mediawiki/includes/parser/Parser.php(441): Parser->internalParse(string)
#14 .../mediawiki/includes/content/WikitextContent.php(330): Parser->parse(string, Title, ParserOptions, boolean, boolean, NULL)
#15 .../mediawiki/includes/content/AbstractContent.php(497): WikitextContent->fillParserOutput(Title, NULL, ParserOptions, boolean, ParserOutput)
#16 .../mediawiki/includes/page/WikiPage.php(2198): AbstractContent->getParserOutput(Title, NULL, ParserOptions)
#17 .../mediawiki/extensions/SpamBlacklist/SpamBlacklistHooks.php(37): WikiPage->prepareContentForEdit(WikitextContent)
#18 .../mediawiki/includes/Hooks.php(195): SpamBlacklistHooks::filterMergedContent(RequestContext, WikitextContent, Status, string, User, boolean)
#19 .../mediawiki/includes/EditPage.php(1634): Hooks::run(string, array)
#20 .../mediawiki/includes/EditPage.php(2048): EditPage->runPostMergeFilters(WikitextContent, Status, User)
#21 .../mediawiki/includes/EditPage.php(1467): EditPage->internalAttemptSave(NULL, boolean)
#22 .../mediawiki/includes/EditPage.php(612): EditPage->attemptSave(NULL)
#23 .../mediawiki/includes/actions/EditAction.php(59): EditPage->edit()
#24 .../mediawiki/includes/actions/SubmitAction.php(38): EditAction->show()
#25 .../mediawiki/includes/MediaWiki.php(495): SubmitAction->show()
#26 .../mediawiki/includes/MediaWiki.php(289): MediaWiki->performAction(Article, Title)
#27 .../mediawiki/includes/MediaWiki.php(851): MediaWiki->performRequest()
#28 .../mediawiki/includes/MediaWiki.php(512): MediaWiki->main()
#29 .../mediawiki/index.php(43): MediaWiki->run()
#30 {main}

Run on:
a)
MediaWiki 1.28.0
PHP 7.1.3 (apache2handler)
MariaDB 10.1.22-MariaDB
ICU 58.2
b)
MediaWiki 1.28.0
PHP 7.0.14 (apache2handler)
MariaDB 10.1.22-MariaDB
ICU 58.1

c) where it runs OK.
MediaWiki 1.27.1
PHP 7.0.14 (apache2handler)
MariaDB 10.1.21-MariaDB
ICU 58.1

The recursive parse exception is due to an interaction with the
SpamBlacklist extension and the UploadVerifyFile hook. Since GraphViz no
longer creates a file page there is no wiki text for the SpamBlacklist
extension (or any other to examine). The only reliable way to prevent this
exception is for the custom GraphViz upload code to not call the
UploadVerifyFile hook. I've updated the patch accordingly.

Retests: all passed. Thanks.
a) OK.
MediaWiki 1.27.1
PHP 7.0.14 (apache2handler)
MariaDB 10.1.22-MariaDB
ICU 58.1

b) OK.
MediaWiki 1.28.0
PHP 7.0.14 (apache2handler)
MariaDB 10.1.22-MariaDB
ICU 58.1

c) OK
MediaWiki 1.28.0
PHP 7.1.3 (apache2handler)
MariaDB 10.1.22-MariaDB
ICU 58.2

Just a question. SpamBlacklist extension is installed in all three environments yet the test passed at MW 1.27 with the previous patch. How is that possible. MW core at play?

Change 344483 merged by jenkins-bot:
[mediawiki/extensions/GraphViz@master] Redesign to eliminate creation of file pages for graph images.

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

I just successully tested on the following environment:

  • MediaWiki 1.28.1 (819c0d2) 00:51, 7 April 2017
  • PHP 5.6.30-0+deb8u1 (apache2handler)
  • MariaDB 10.0.30-MariaDB-1~jessie
  • ICU 52.1
  • Graphviz 2.0.0 (78f8598) 20:50, 10 April 2017

Thanks a lot @Welterkj for authoring the new release and for the others testing.