Item8314: DirectedGraphPlugin rdiff and rev= topic access updates attachments

-- GeorgeClark - 28 Oct 2009

The plugin updates and attaches old versions of the generated graphics when a topic is viewed using a revision number, or when the topic is viewed using rdiff. Updating current attachments while viewing old revisions is probably not a good idea.

Not sure of the best way to resolve this. I think adding an expert setting of contexts to be excluded from generating attachments, and a flag to disable generating attachments for back revisions would be better than simply turning off the feature. This way anyone who wants the old behaviour can enable it. I'm not sure if there would be some way to use viewfile to look at older revisions of attachments when viewing an old revision of the topic. Since graph versions are asynchronous to topic revisions, there may not be a reliable way to determine which attachment revisions went with older topic revisions.

---

(12:11:11 PM) gac410: using view  rev=   or rdiff  regenerates all old attachments.
(12:11:22 PM) gac410: I think thats a bad thing.
(12:13:14 PM) MTempest: I have seen odd things with history and DGP before.
(12:14:13 PM) gac410: I'm thinking that at a minimum, the plugin should only generate attachments for 
current view equivalents  (view and genpdf and ???)
(12:14:17 PM) MTempest: Regenerating old attachments is not a problem unless it interferes with 
subsequent views of other revisions e.g. the latest revision
(12:14:48 PM) gac410: But it does "update" the current topic when it attaches the old versions, 
and then updates again when the current version is viewed.
(12:15:53 PM) gac410: I'm not sure what would happen if a view   and a view;rev=  occurs simultaneously.  
(12:16:00 PM) MTempest: Updating the topic is bad.
(12:16:14 PM) gac410: Agree.  
(12:16:27 PM) MTempest: If the attachment API is bypassed, is the topic still updated.
(12:16:29 PM) MTempest: ?
(12:16:55 PM) MTempest: I would like rdiff to show changes to graphs. That did not work 
on my twiki installation. I have not tried it on foswiki.
(12:17:10 PM) MTempest: Perhaps DGP should not use attachments at all.
(12:17:15 PM) gac410: No, the topic is not updated, but the attachments are updated.  -   
If someone were depending upon attachment names accessing them directly, it would be ugly.
(12:18:08 PM) MTempest: I have cases where attachments are accessed directly.
(12:18:26 PM) gac410: It has to cache the output somewhere so that the browser can fetch 
them.   If it doesn't attach, then overhead of rebuilding the attachments happens for every view.
(12:19:19 PM) MTempest: It has to cache them - yes. I do not think attachments is the only way to do that.
(12:19:32 PM) gac410: so if user A goes and views rev=2 - a really old rev and never returns 
to view the current version, then user B accesses an attachment directly  they will get the 
wrong one or find it missing.
(12:20:12 PM) MTempest: Yes.
(12:20:20 PM) gac410: Maybe a rev= view should put the attachments in a "scratch" in pub, but 
not actually attach them.   Not sure how to clean them up thought.
(12:20:24 PM) gac410: though.
(12:21:27 PM) MTempest: I was just looking at MathModePlugin - it stores images at attachments, 
it does cleanup, and it uses hashes to differentiate between images.
(12:21:58 PM) MTempest: MathModePlugin images may go missing if there are simultaneous requests 
for two different revisions.
(12:22:16 PM) gac410: So does DGP - uses hashes to differentiate, and does cleanup. 
(12:22:53 PM) gac410: It only regenerates an attachment if the hash of the <dot> source is different.
(12:25:02 PM) MTempest: What if DGP only generated images for a plain view (or genpdf - list them 
in configure), so it would not generate images for rdiff or if a revision is specified, as you suggested,
(12:25:16 PM) MTempest: but also provide a way to force generation.
(12:25:38 PM) gac410: That is the easiest fix.    
(12:25:43 PM) MTempest: e.g. ...bin/view/Web/MyTopic?rev=2&dotimages=1
(12:26:38 PM) MTempest: I prefer your suggestion :) An additional parameter could be added later
(12:26:50 PM) gac410: Good idea
(12:27:53 PM) gac410: So if I add an exclude  (for rdiff and rev=)  then everything like genpdf, 
or other alternate view contexts will work without needing config intervention.
(12:28:28 PM) ArthurClemens left the room (quit: "Leaving...").
(12:28:34 PM) MTempest: Also exclude for history
(12:28:43 PM) MichaelDaum left the room (quit: Remote closed the connection).
(12:29:04 PM) gac410: And document that there is a significant change in behavior in that rev= 
will no longer show the diagrams.
(12:29:16 PM) gac410: Ah - history though is rdiff.   
(12:29:48 PM) gac410: And of course old revisions of diagrams can still be found by using the 
attachments dialog with viewfile 
(12:30:33 PM) ***MTempest is checking what HistoryPlugin and CompareRevisionsAddonPlugin do to the urls
(12:31:33 PM) gac410: Also wondering if it would make sense for "view equivalent" contexts to 
also set the view context.   genpdf is view as well as genpdf.
(12:32:37 PM) gac410: I believe that CompareRevisions  adds a "compare" script.   Which can slip through 
authentication unless one updates the apache config.
(12:38:53 PM) MTempest: CDot: [18:33]   <gac410>   Also wondering if it would make sense 
for "view equivalent" contexts to also set the view context. genpdf is view as well as genpdf.
(12:39:15 PM) CDot: oh, right. Yes, I hit that problem too
(12:39:17 PM) gac410: No - the question is should 'view equivalent' scripts like genpdf  ....  
MTempest knows copy/paste faster than typing.
(12:39:18 PM) MTempest: I was referring to the contexts question
(12:39:53 PM) CDot: gac410: I understand the question; had to do something similar for 
the old versions of some contribs
(12:40:04 PM) CDot: until I worked out how to work smarter
(12:41:19 PM) CDot: gac410: you can try that, but if it all goes pear shaped, me no speakee englishee
(12:41:32 PM) gac410: :-D
(12:42:38 PM) MTempest: gac410: your suggestion for 8314 looks good.

---

I plan to add the following configuration options and defaults:

# **BOOLEAN**
# Flag specifying if the plugin should generate graphs when viewing previous topic revisions<br>
#  If the plugin generates attachments when viewing old revisions, this can result
#  in out of date attachments, and significant overhead regenerating the attachments.
#  Enable this flag to restore previous behavior of the plugin
#  Note that by default, old revisions of attachments can be viewed using the attach dialog.
$Foswiki::cfg{Plugins}{DirectedGraphPlugin}{allowRevGeneration} = '0';

# **STRING 100**
# List of scripts that should bypass graph generation <br>
#  If the plugin generates attachments during compare operations, this can result
#  in out of date attachments, and significant overhead regenerating the attachments
#  Set this to 'none' to restore old behavior.   If this is not set, then rdiff and
#  compare will excluded.
$Foswiki::cfg{Plugins}{DirectedGraphPlugin}{bypassScriptGeneration} = 'rdiff,compare';

It's a pity rdiff can't display an attachment revision that is appropriate for the topic revision being examined, as this is a general problem with rdiff and attachments - not just DG.

Anyway, your fix here sounds reasonable. Thank you for your efforts on this excellent plugin.

-- PaulHarvey - 29 Oct 2009

I've made some minor changes to the above. The scripts are not readily available, but context should be consistent. So it now checks for the "diff" context. I've also modified the CompareRevisionsAddOn to set the diff context, as it provides for an alternative to diff/diffauth scripts.

-- GeorgeClark - 04 Nov 2009

I'm going to add one more change to this update. The afterCommonTagsHandler is probably overkill for doing the attachment cleanup. Change to a cleanup routine at the end of the commonTagsHandler that runs only when dot tags were found.

-- GeorgeClark - 04 Nov 2009

Commenting is disabled while not logged in