* looking for nirs@freeshell.org--2005/moin--fix--1.3--patch-23 to compare with
* comparing to nirs@freeshell.org--2005/moin--fix--1.3--patch-23
M  MoinMoin/user.py

* modified files

--- orig/MoinMoin/user.py
+++ mod/MoinMoin/user.py
@@ -6,7 +6,7 @@
     @license: GNU GPL, see COPYING for details.
 """
 
-import os, string, time, Cookie, sha, codecs
+import os, time, Cookie, sha, codecs
 
 try:
     import cPickle as pickle
@@ -259,7 +259,8 @@
         
         # attrs not saved to profile
         self._request = request
-        self._trail = []
+        # This means trail is not known yet, must read from disk
+        self._trail = None
 
         # create checkbox fields (with default 0)
         for key, label in self._checkbox_fields:
@@ -410,8 +411,8 @@
         if -24 <= self.tz_offset and self.tz_offset <= 24:
             self.tz_offset = self.tz_offset * 3600
 
-        # clear trail
-        self._trail = []
+        # clear trail, must read from disk
+        self._trail = None
 
         if not self.disabled:
             self.valid = 1
@@ -686,47 +687,63 @@
                 return 1
         return 0
 
+    def shouldAddTrail(self, pageName):
+        """ Return true if name should be added to trail """
+        if not self.valid:
+            return False
+
+        # Don't update if trail is not used currently.
+        # TODO: This cause a little problem when activating 'show page
+        # trail' since its starts with empty trail. If we remove this we
+        # will have to pay one disk access per request for valid users.
+        if not (self.show_page_trail or self.remember_last_visit):
+            return False
+        
+        # Don't add missing pages or those the user may not read
+        if self._request:
+            from MoinMoin.Page import Page
+            page = Page(self._request, pageName)
+            if not (page.exists() and
+                    self._request.user.may.read(page.page_name)):
+                return False
+        return True
 
     def addTrail(self, pagename):
-        """
-        Add page to trail.
+        """ Add page to trail.
         
         @param pagename: the page name to add to the trail
         """
-        # TODO: aquire lock here, so multiple processes don't clober
-        # each one trail.
-
-        if self.valid and (self.show_page_trail or self.remember_last_visit):
-            # load trail if not known
-            self.getTrail()      
-            
-            # don't append tail to trail ;)
-            if self._trail and self._trail[-1] == pagename: return
+        if self.shouldAddTrail(pagename):
+            # TODO: aquire lock so another process may not read or write
+            # trail until we release the lock.
+
+            # Becuase one user can use multiple browsers, and each
+            # session might add pages to the trail, we must read trail
+            # from disk before adding items to the trail.
+            self._loadTrail()      
+
+            # Don't append tail to trail
+            if not pagename in self._trail[-1:]:
+                # Remove pagename duplicates (from multiple sessions?)
+                self._trail = [name for name in self._trail
+                               if name != pagename]
+                self._trail.append(pagename)
+                self._trail = self._trail[-self._cfg.trail_size:]
+                self._saveTrail()
 
-            # Add only existing pages that the user may read
-            if self._request:
-                from MoinMoin.Page import Page
-                page = Page(self._request, pagename)
-                if not (page.exists() and
-                        self._request.user.may.read(page.page_name)):
-                    return
-
-            # append new page, limiting the length
-            self._trail = filter(lambda p, pn=pagename: p != pn, self._trail)
-            self._trail = self._trail[-(self._cfg.trail_size-1):]
-            self._trail.append(pagename)
-            self.saveTrail()
+            # TODO: release lock
 
-            ## TODO: release lock here
-            
-    def saveTrail(self):
+    def _trailPath(self):
+        return self.__filename() + ".trail"
+        
+    def _saveTrail(self):
         """ Save trail file
 
         Save using one write call, which should be fine in most cases,
         but will fail in rare cases without real file locking.
         """
         data = '\n'.join(self._trail) + '\n'
-        path = self.__filename() + ".trail"
+        path = self._trailPath()
         try:
             file = codecs.open(path, "w", config.charset)
             try:
@@ -745,22 +762,45 @@
             self._request.log("Can't save trail file: %s" % str(err))
 
     def getTrail(self):
-        """
-        Return list of recently visited pages.
+        """ Return list of recently visited pages.
+
+        Get the trail from disk once per user life span, which is one
+        request. Later return cached trail.
         
         @rtype: list
         @return: pages in trail
         """
-        if self.valid and (self.show_page_trail or self.remember_last_visit) \
-                and not self._trail \
-                and os.path.exists(self.__filename() + ".trail"):
-            try:
-                self._trail = codecs.open(self.__filename() + ".trail", 'r', config.charset).readlines()
-            except (OSError, ValueError):
-                self._trail = []
+        if self._trail is None:
+            # Initialize trail from disk if used, or to empty list
+            if (self.valid and
+                (self.show_page_trail or self.remember_last_visit)):
+                self._loadTrail()
             else:
-                self._trail = filter(None, map(string.strip, self._trail))
-                self._trail = self._trail[-self._cfg.trail_size:]
-
+                self._trail = []  
         return self._trail
 
+    def _loadTrail(self):
+        """ Load trail from disk or set to empty list
+
+        Should be safe for normal use, even safer if we add file
+        locking.
+        """
+        path = self._trailPath()
+        # TODO: aquire lock
+        try:
+            file = codecs.open(path, 'r', config.charset)
+            try:
+                trail = file.readlines()
+            finally:
+                file.close()
+            trail = [name.strip() for name in trail
+                     if name != '' and not name.isspace()]
+            self._trail = trail[-self._cfg.trail_size:]
+        except (IOError, OSError, ValueError), err:
+            # Don't log this because its ok if the trail was deleted,
+            # its a problem only if we can't create it later.
+            self._trail = []
+        # TODO: release lock
+            
+                    
+