From: Lawrence A. <la...@20...> - 2002-02-26 15:14:41
|
Any views from the floor? Marcel is using the PostNukeModule (in effect, phpWiki 1.3.2), but his comments apply generally. >Date: Tue, 26 Feb 2002 15:28:20 +0100 >From: Marcel van der Boom <ma...@hs...> >Organization: HS-Development BV >User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8+) >Gecko/20020222 >X-Accept-Language: en-us >To: Lawrence Akka <la...@us...> >Subject: Re: phpWiki patch - concurrent updates > >Lawrence, > >I think I disagree. The user is indeed presented with an error now. The >patch turns this error effectively into a warning. The modifications of >the user are saved under a new (higher than expected from the users point >of view) revision and the warning says that the user *should* look at the >diff. This looks like the normal "flow" when editting documents, storing >the latest edit under the highest revision.. It is also independent of >users settings of caching and browser. I agree that the patch is a tempory >fix. The user should really be presented with a choice of: >* merge documents >* revert to older revision (discard edits) >* apply my edits. > >During the warning it might be wise to lock the page to prevent second or >higher order effects to occur (see below). > >The code with the patch applied is still not save, or not concurrent >enough. As the number of concurrent edits increase, the "cumbersomeness" >increases as well. With "n" edits applied *in order* the user has to look >at "n"diffs to decide what to do, which is not really comfortable. The >risk of this obviously depends on the "edit-time" and the number of users >working on the same document. >This is the simplest situation. It could get worse. What happens for >example when multiple edits are done when the user is reading the >warning? By the time he has examined the diff a higher version may >exist. This can create rather complicated sessions with cascading warnings >and our users will be confused the first time, let alone the second >time. I guess this is a problem which really should be dealt with in a >broader context (if it is a problem and I'm not mistaken). > >For our purpose the safety net for a concurrency of 2 of the first order >is probably enough, but we haven't taken the phpWiki to implementations at >customers sites yet. > >Am I seeing things too complicated here? > >To summarize: >The fact that the createRevision will return false is "a choice". I think >the call should return true and warn the user that the flow is >interrupted. This applies only if phpWiki wants to allow concurrent >editting. Another strategy would be the locking of pagesduring an edit >sessions, but this is quite another discussion, and will look somewhat >more unfriendly. > >The problem, as far as I can see now, is not related to caching (at least >not in the PN version), but I might be wrong. I tend to choose solutions >which do not depend on client settings. > >Marcel > > >>I am not sure at the moment that your patch is necessary. If during the >>time that the code has been executing another user has edited and saved >>the page, the call to $page->createRevision($editversion + 1 .... will >>return false. The createRevision function will not be able to create the >>desired revision, because there will exist a revision with a later >>version number. Accordingly, if (!is_object($newrevision)) ... is true >>and an error is reported to the user. >> >>Let me know if you disagree >> >>Lawrence >> >>At 13:25 26/02/2002, you wrote: >> >>>Lawrence, >>> >>>>This might be something to do with caching problems in the >>>>browser. phpWiki 1.3.2 merges conflicting edits (in a similar way to >>>>cvs), and gives the user the chance to edit the merged result before saving. >>>I checked to savepage.php file and noticed that it is assumed that the >>>editted version is still the current version, which is in this case wrong. >>> >>>I've included a little patch for this. As I'm (yet) unfamiliar with the >>>phpWiki code and didn't know if this was PostNuke related I thought I >>>send the patch first to you. If you want me to take other actions, let >>>me know. The change is almost to small to apply the patch file but if >>>you want it let me know what your groups standard cmdline for diff is >>>and i'll send it to you. >>> >>>Cheers, >>>Marcel >>> >>>--- ./savepage.php Tue Feb 26 14:20:54 2002 >>> >>>+++ /home/httpd/hsd/modules/phpWiki/lib/savepage.php Tue Nov 27 >>>11:29:02 2001 >>>@@ -129,10 +129,7 @@ >>> >>> >>>// >>> >>> // From here on, we're actually saving. >>> >>> >>>// >>> >>>- // Don't assume we're still editting the current revision >>> >>>- // This section is now critical (in terms of concurrency) >>> >>>- $currentversion = $current->getVersion(); >>> >>>- $newrevision = $page->createRevision($currentversion + 1, >>>+ $newrevision = $page->createRevision($editversion + 1, >>> >>> $content, $meta, >>> >>> >>>ExtractWikiPageLinks($content)); >>> >>> if (!is_object($newrevision)) { >>> >>>@@ -145,13 +142,6 @@ >>> >>> >>>$cleaner->cleanPageRevisions($page); >>> >>> >>> >>> >>> $warnings = $dbi->GenericWarnings(); >>> >>>- >>> >>>- // Have we skipped a revision? >>> >>>- if(($currentversion > $editversion)){ >>> >>>- // Somehow an intermediate revision was created warn user >>>- $warnings ="While you were editting another revision was >>>created. You might want to check a diff between your revision and the >>>previous one"; >>> >>>- >>>} >>> >>>- >>> >>> if (empty($warnings)) { >>> >>> // Do redirect to browse page. >>> >>> // In this case, the user will most likely not see the rest of >>>@@ -165,7 +155,6 @@ >>> >>> $html .= gettext ("Your careful attention to detail is much >>> appreciated."); >>> $html .= "\n"; >>> >>> >>> >>> >>>- >>> >>> if ($warnings) { >>> >>> $html .= Element('p', "<b>Warning!</b> " >>> >>> . htmlspecialchars($warnings) >>> >>>-- >>>Marcel van der Boom >>>HS-Development BV >>>Kwartiersedijk 14B >>>Fijnaart, The Netherlands >>>Tel. : 0168-468822 >>>Fax. : 0168-468823 >>>Email: ma...@hs... >>> >>> >> > >-- >Marcel van der Boom >HS-Development BV >Kwartiersedijk 14B >Fijnaart, The Netherlands >Tel. : 0168-468822 >Fax. : 0168-468823 >Email: ma...@hs... > > > ==========NOTICE========== Internet e-mail is not necessarily secure or reliable. Please let us know if you would like to establish a secure channel of communication. This e-mail and any attachments are confidential and may be legally privileged. They are intended only for the use of the named recipient. If you are not the named or intended recipient, please notify us immediately. In such an event, you should not disclose the contents of this e-mail or any attachments to any other person, nor copy, print, store or use them in any manner whatsoever. Thank you for your co-operation. Although we have taken precautions to minimize the risk of transmitting software viruses, you are advised to carry out your own virus checks on any attachments to this message. Tel: +44 (0)207 842 1200 http://www.20essexst.com pos...@20... |