Item13586: modernize and fix
Priority: Enhancement
Current State: No Action Required
Released In: n/a
Target Release:
This is one of the oldest and most useful plugins. Yet still there are a couple of red herrings in there.
Foremost, this is all implemented in
DirectedGraphPlugin.pm
with a very heavy
initPlugin()
method. So
every request suffers from having to compile and initialize this plugin, if you use it or not. Modernizing it
means moving the bulk code into a
Core.pm
object which is only instantiated as required.
Also, there is a lot of code working on the filesystem directly - optionally - presumably for better performance.
This however terribly breaks any other Store implementation in use. Removing this direct-file operations reduced
the code by 50%.
Then, the LIBRARY features is not working.
Then, DOT graphics are recomputed again and again. Visiting a page with a DOT graphics on it requires authentication.
When INCLUDING a funtion that renders the DOT graph will attach the generated image to the
included topic, not the
base topic where the function was called.
The plugin uses the session to store interim values between multiple graphs being rendered. This bloats the session
object as well as interferes with the PageCache which uses the session content to render page variations.
--
MichaelDaum - 30 Jul 2015
Feedback on this ... commented in IRC as well.
- Heavy init. yes. needs to be rewritten.
- File system direct i/o. In some cases plugin cannot be used without it. It will time out. A page with a large number of large dot's can take a very long time. And the time to attach all of the rendered versions, pdf, png, dot, gif, ... is way too long.
- LIBRARY works fine. The Examples page includes a LIBRARY and it rendered fine. Can be tricky though to get it configured correctly. LIBRARY depends upon getting the paths correct, so that the external 3rd party tools can read the files directly from the file system.
- DOT graphics should ONLY be recomputed if the md5 of the <dot> source does not match the md5 in the working/workarea. If it matches they are not recomputed. This is also tested and was working fine last time I tested.
- Visit requires auth: Only if the working/workarea md5 do not match the <dot> md5. Also auth not needed if using direct file i/o.
- INCLUDE .. yeah that's an issue.
- Session variables. That was a horrible design decision you can blame directly on me. I did that to avoid globals. and didn't understand what I was doing.
--
GeorgeClark - 30 Jul 2015
I can confirm that this plugin is extremely popular with my software developer users. They even hacked up a modded version called
PlantUMLPlugin. Not released yet. Horrible piece of junk. Error handling is a crash. Syntax error is a crash. But they love it. They made it, they use it and they love it. I feel bad about releasing it because it has all the horror that Michael just listed above for this plugin.
--
KennethLavrsen - 30 Jul 2015
Also worth re-iterating that the initPlugin method should not be returning 0 just because it's in the raw context or whatever. Returning 0 indicates an error, using it to disable the plugin in irrelevant contexts is kinda sloppy.
--
JohnKnutson - 06 Aug 2015
... extremely hard to disentangle this code. There is so much processing that is simply not making much sense to me. Things that should be easy are somewhat over-complicated. I am afraid best would be to start from scratch to integrate graphviz as a new
GraphvizPlugin.
--
MichaelDaum - 07 Aug 2015