[CalendarServer-changes] [5733] CalendarServer/branches/new-store

source_changes at macosforge.org source_changes at macosforge.org
Mon Jun 14 17:35:52 PDT 2010


Revision: 5733
          http://trac.macosforge.org/projects/calendarserver/changeset/5733
Author:   glyph at apple.com
Date:     2010-06-14 17:35:49 -0700 (Mon, 14 Jun 2010)
Log Message:
-----------
Stub out as much as possible to eliminate order-of-operations problems between file creation and transaction commit.

Modified Paths:
--------------
    CalendarServer/branches/new-store/twext/web2/dav/method/put.py
    CalendarServer/branches/new-store/twistedcaldav/static.py
    CalendarServer/branches/new-store/twistedcaldav/storebridge.py
    CalendarServer/branches/new-store/txcaldav/calendarstore/file.py
    CalendarServer/branches/new-store/txcaldav/calendarstore/test/test_file.py
    CalendarServer/branches/new-store/txdav/propertystore/xattr.py

Modified: CalendarServer/branches/new-store/twext/web2/dav/method/put.py
===================================================================
--- CalendarServer/branches/new-store/twext/web2/dav/method/put.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/twext/web2/dav/method/put.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -58,20 +58,6 @@
         yield x
         x.getResult()
 
-    if self.fp.exists():
-        if not self.fp.isfile():
-            log.err("Unable to PUT to non-file: %s" % (self.fp.path,))
-            raise HTTPError(StatusResponse(
-                responsecode.FORBIDDEN,
-                "The requested resource exists but is not backed by a regular file."
-            ))
-    else:
-        if not self.fp.parent().isdir():
-            log.err("No such directory: %s" % (self.fp.path,))
-            raise HTTPError(StatusResponse(
-                responsecode.CONFLICT,
-                "Parent collection resource does not exist."
-            ))
 
     #
     # HTTP/1.1 (RFC 2068, section 9.6) requires that we respond with a Not

Modified: CalendarServer/branches/new-store/twistedcaldav/static.py
===================================================================
--- CalendarServer/branches/new-store/twistedcaldav/static.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/twistedcaldav/static.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -687,6 +687,7 @@
 
         @return: an L{Deferred} with a C{int} result containing the size of the resource.
         """
+        raise RuntimeError("quotaSize called on %r" % (self,))
         if self.isCollection():
             @inlineCallbacks
             def walktree(top):
@@ -1004,17 +1005,33 @@
         # TODO: when calendar home gets a resourceID( ) method, remove
         # the "id=record.uid" keyword from this call:
         self.clientNotifier = ClientNotifier(self, id=record.uid)
-
+        txn = parent.parent._newStore.newTransaction()
+        self._newStoreCalendarHome = (
+            txn.calendarHomeWithUID(record.uid, create=True)
+        )
         CalDAVFile.__init__(self, path)
         DirectoryCalendarHomeResource.__init__(self, parent, record)
-        txn = self.parent.parent._newStore.newTransaction()
-        self._newStoreCalendarHome = (
-            txn.calendarHomeWithUID(self.record.uid, create=True)
+
+        from twistedcaldav.storebridge import _NewStorePropertiesWrapper
+        self._dead_properties = _NewStorePropertiesWrapper(
+            self._newStoreCalendarHome.properties()
         )
+
         self.associateWithTransaction(txn)
 
 
+    def exists(self):
+        # FIXME: tests
+        return True
+    
+    
+    def quotaSize(self, request):
+        # FIXME: tests, workingness
+        return succeed(0)
+
+
     def provision(self):
+        return
         result = super(CalendarHomeFile, self).provision()
         if config.Sharing.Enabled and config.Sharing.Calendars.Enabled:
             self.provisionShares()
@@ -1036,8 +1053,9 @@
         else:
             NotificationCollectionFileClass = None
 
+        from twistedcaldav.storebridge import StoreScheduleInboxFile
         cls = {
-            "inbox"        : ScheduleInboxFile,
+            "inbox"        : StoreScheduleInboxFile,
             "outbox"       : ScheduleOutboxFile,
             "dropbox"      : DropBoxHomeFileClass,
             "freebusy"     : FreeBusyURLFileClass,
@@ -1259,8 +1277,10 @@
         ScheduleOutboxResource.__init__(self, parent)
 
     def provision(self):
-        self.provisionFile()
-        return super(ScheduleOutboxFile, self).provision()
+        """
+        Schedule outboxes do not need to be provisioned; they shouldn't store
+        anything.
+        """
 
     def __repr__(self):
         return "<%s (calendar outbox collection): %s>" % (self.__class__.__name__, self.fp.path)

Modified: CalendarServer/branches/new-store/twistedcaldav/storebridge.py
===================================================================
--- CalendarServer/branches/new-store/twistedcaldav/storebridge.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/twistedcaldav/storebridge.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -30,11 +30,12 @@
 from twext.python import vcomponent
 
 from twext.web2.http_headers import ETag
-from twext.web2.responsecode import FORBIDDEN, NO_CONTENT, NOT_FOUND, CREATED
+from twext.web2.responsecode import FORBIDDEN, NO_CONTENT, NOT_FOUND, CREATED, \
+    CONFLICT
 from twext.web2.dav.util import parentForURL, allDataFromStream
 from twext.web2.http import HTTPError, StatusResponse
 
-from twistedcaldav.static import CalDAVFile
+from twistedcaldav.static import CalDAVFile, ScheduleInboxFile
 
 from txdav.propertystore.base import PropertyName
 from txcaldav.icalendarstore import NoSuchCalendarObjectError
@@ -105,21 +106,11 @@
                 self._newPropertyStore.keys()]
 
 
-
-class CalendarCollectionFile(CalDAVFile):
+class _CalendarChildHelper(object):
     """
-    Wrapper around a L{txcaldav.icalendar.ICalendar}.
+    Methods for things which are like calendars.
     """
 
-    def __init__(self, calendar, home, *args, **kw):
-        """
-        Create a CalendarCollectionFile from a L{txcaldav.icalendar.ICalendar}
-        and the arguments required for L{CalDAVFile}.
-        """
-        super(CalendarCollectionFile, self).__init__(*args, **kw)
-        self._initializeWithCalendar(calendar, home)
-
-
     def _initializeWithCalendar(self, calendar, home):
         """
         Initialize with a calendar.
@@ -137,6 +128,18 @@
         )
 
 
+    def index(self):
+        """
+        Retrieve the new-style index wrapper.
+        """
+        return self._newStoreCalendar.retrieveOldIndex()
+
+
+    def exists(self):
+        # FIXME: tests
+        return True
+
+
     @classmethod
     def transform(cls, self, calendar, home):
         """
@@ -174,6 +177,46 @@
         return similar
 
 
+    def quotaSize(self, request):
+        # FIXME: tests, workingness
+        return succeed(0)
+
+
+
+class StoreScheduleInboxFile(_CalendarChildHelper, ScheduleInboxFile):
+
+    def __init__(self, *a, **kw):
+        super(StoreScheduleInboxFile, self).__init__(*a, **kw)
+        home = self.parent._newStoreCalendarHome
+        storage = home.calendarWithName("inbox")
+        if storage is None:
+            # FIXME: spurious error, sanity check, should not be needed
+            raise RuntimeError("backend should be handling this for us")
+            storage = home.calendarWithName("inbox")
+        self._initializeWithCalendar(
+            storage,
+            self.parent._newStoreCalendarHome
+        )
+
+
+    def provisionFile(self):
+        pass
+
+
+class CalendarCollectionFile(_CalendarChildHelper, CalDAVFile):
+    """
+    Wrapper around a L{txcaldav.icalendar.ICalendar}.
+    """
+
+    def __init__(self, calendar, home, *args, **kw):
+        """
+        Create a CalendarCollectionFile from a L{txcaldav.icalendar.ICalendar}
+        and the arguments required for L{CalDAVFile}.
+        """
+        super(CalendarCollectionFile, self).__init__(*args, **kw)
+        self._initializeWithCalendar(calendar, home)
+
+
     def http_COPY(self, request):
         """
         Copying of calendar collections isn't allowed.
@@ -206,6 +249,16 @@
 
 
 
+class NoParent(CalDAVFile):
+    def http_MKCALENDAR(self, request):
+        return CONFLICT
+
+
+    def http_PUT(self, request):
+        return CONFLICT
+
+
+
 class ProtoCalendarCollectionFile(CalDAVFile):
     """
     A resource representing a calendar collection which hasn't yet been created.
@@ -230,9 +283,18 @@
         # twistedcaldav.test.test_mkcalendar.
         #     MKCALENDAR.test_make_calendar_no_parent - there should be a more
         # structured way to refuse creation with a non-existent parent.
-        return CalDAVFile(path)
+        return NoParent(path)
 
 
+    def provisionFile(self):
+        """
+        Create a calendar collection.
+        """
+        # FIXME: this should be done in the backend; provisionDefaultCalendars
+        # should go away.
+        return self.createCalendarCollection()
+
+
     def createCalendarCollection(self):
         """
         Override C{createCalendarCollection} to actually do the work.
@@ -250,7 +312,24 @@
         return d
 
 
+    def exists(self):
+        # FIXME: tests
+        return False
 
+
+    def provision(self):
+        """
+        This resource should do nothing if it's provisioned.
+        """
+        # FIXME: should be deleted, or raise an exception
+
+
+    def quotaSize(self, request):
+        # FIXME: tests, workingness
+        return succeed(0)
+
+
+
 class CalendarObjectFile(CalDAVFile):
     """
     A resource wrapping a calendar object.
@@ -267,6 +346,11 @@
         self._initializeWithObject(calendarObject)
 
 
+    def exists(self):
+        # FIXME: Tests
+        return True
+
+
     def etag(self):
         # FIXME: far too slow to be used for real, but I needed something to
         # placate the etag computation in the case where the file doesn't exist
@@ -290,7 +374,8 @@
 
 
     def quotaSize(self, request):
-        return len(self._newStoreObject.iCalendarText())
+        # FIXME: tests
+        return succeed(len(self._newStoreObject.iCalendarText()))
 
 
     def iCalendarText(self, ignored=None):
@@ -354,3 +439,11 @@
         returnValue(CREATED)
 
 
+    def exists(self):
+        # FIXME: tests
+        return False
+
+
+    def quotaSize(self, request):
+        # FIXME: tests, workingness
+        return succeed(0)

Modified: CalendarServer/branches/new-store/txcaldav/calendarstore/file.py
===================================================================
--- CalendarServer/branches/new-store/txcaldav/calendarstore/file.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/txcaldav/calendarstore/file.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -35,6 +35,7 @@
 
 from twisted.internet.defer import succeed
 
+from twisted.python import log
 from twext.python.log import LoggingMixIn
 from twext.python.vcomponent import VComponent
 from twext.python.vcomponent import InvalidICalendarDataError
@@ -195,12 +196,13 @@
                 undo = operation()
                 if undo is not None:
                     undos.append(undo)
-            except Exception, e:
+            except:
+                log.err()
                 for undo in undos:
                     try:
                         undo()
-                    except Exception, e:
-                        self.log_error("Exception while undoing transaction: %s" % (e,))
+                    except:
+                        log.err()
                 raise
 
 
@@ -216,31 +218,47 @@
         childPath1 = self._calendarStore._path.child(uid[0:2])
         childPath2 = childPath1.child(uid[2:4])
         childPath3 = childPath2.child(uid)
+        def createDirectory(path):
+            try:
+                path.createDirectory()
+            except (IOError, OSError), e:
+                if e.errno != EEXIST:
+                    # Ignore, in case someone else created the
+                    # directory while we were trying to as well.
+                    raise
 
-        def do():
-            def createDirectory(path):
-                try:
-                    path.createDirectory()
-                except (IOError, OSError), e:
-                    if e.errno != EEXIST:
-                        # Ignore, in case someone else created the
-                        # directory while we were trying to as well.
-                        raise
+        creating = False
+        if create:
+            if not childPath2.isdir():
+                if not childPath1.isdir():
+                    createDirectory(childPath1)
+                createDirectory(childPath2)
+            if childPath3.isdir():
+                calendarPath = childPath3
+            else:
+                creating = True
+                calendarPath = childPath3.temporarySibling()
+                createDirectory(calendarPath)
+                def do():
+                    def lastly():
+                        calendarPath.moveTo(childPath3)
+                        # calendarHome._path = calendarPath
+                        # do this _after_ all other file operations
+                        return lambda : None
+                    self.addOperation(lastly)
+                    return lambda : None
+                self.addOperation(do)
 
-            if not childPath3.isdir():
-                if not childPath2.isdir():
-                    if not childPath1.isdir():
-                        createDirectory(childPath1)
-                    createDirectory(childPath2)
-                createDirectory(childPath3)
-
-        if create:
-            self.addOperation(do)
         elif not childPath3.isdir():
             return None
+        else:
+            calendarPath = childPath3
 
-        calendarHome = CalendarHome(childPath3, self._calendarStore, self)
+        calendarHome = CalendarHome(calendarPath, self._calendarStore, self)
         self._calendarHomes[(uid, self)] = calendarHome
+        if creating:
+            calendarHome.createCalendarWithName("calendar")
+            calendarHome.createCalendarWithName("inbox")
         return calendarHome
 
 
@@ -288,10 +306,11 @@
 
         childPath = self._path.child(name)
         if childPath.isdir():
-            return Calendar(childPath, self)
+            return Calendar(name, self)
         else:
             return None
 
+
     def createCalendarWithName(self, name):
         if name.startswith("."):
             raise CalendarNameNotAllowedError(name)
@@ -301,19 +320,30 @@
         if name not in self._removedCalendars and childPath.isdir():
             raise CalendarAlreadyExistsError(name)
 
-        c = self._newCalendars[name] = Calendar(childPath, self)
+        temporary = childPath.temporarySibling()
+        temporary.createDirectory()
+        # In order for the index to work (which is doing real file ops on disk
+        # via SQLite) we need to create a real directory _immediately_.
+
+        # FIXME: some way to roll this back.
+
+        c = self._newCalendars[name] = Calendar(temporary.basename(), self, name)
+        c.retrieveOldIndex().create()
         def do():
             try:
-                childPath.createDirectory()
-                # FIXME: direct tests, undo for index creation
-                Index(c)._oldIndex.create()
-
-                # Return undo
-                return lambda: childPath.remove()
+                props = c.properties()
+                temporary.moveTo(childPath)
+                c._name = name
+                # FIXME: _lots_ of duplication of work here.
+                props.path = childPath
+                props.flush()
             except (IOError, OSError), e:
                 if e.errno == EEXIST and childPath.isdir():
                     raise CalendarAlreadyExistsError(name)
                 raise
+            # FIXME: direct tests, undo for index creation
+            # Return undo
+            return lambda: childPath.remove()
 
         self._transaction.addOperation(do)
         props = c.properties()
@@ -331,6 +361,7 @@
         # c.properties().participateInTxn(txn)
         # FIXME: return c # maybe ?
 
+
     def removeCalendarWithName(self, name):
         if name.startswith(".") or name in self._removedCalendars:
             raise NoSuchCalendarError(name)
@@ -368,6 +399,7 @@
 
             return undo
 
+
     # @_cached
     def properties(self):
         # FIXME: needs tests for actual functionality
@@ -385,17 +417,45 @@
     """
     implements(ICalendar)
 
-    compareAttributes = '_path _calendarHome _transaction'.split()
+    compareAttributes = '_name _calendarHome _transaction'.split()
 
-    def __init__(self, path, calendarHome):
-        self._path = path
+    def __init__(self, name, calendarHome, realName=None):
+        """
+        Initialize a calendar pointing at a path on disk.
+
+        @param name: the subdirectory of calendarHome where this calendar
+            resides.
+        @type name: C{str}
+
+        @param calendarHome: the home containing this calendar.
+        @type calendarHome: L{CalendarHome}
+
+        @param realName: If this calendar was just created, the name which it
+        will eventually have on disk.
+        @type realName: C{str}
+        """
+        self._name = name
         self._calendarHome = calendarHome
         self._transaction = calendarHome._transaction
         self._newCalendarObjects = {}
         self._cachedCalendarObjects = {}
         self._removedCalendarObjects = set()
+        self._index = Index(self)
+        self._renamedName = realName
 
 
+    @property
+    def _path(self):
+        return self._calendarHome._path.child(self._name)
+
+
+    def retrieveOldIndex(self):
+        """
+        Retrieve the old Index object.
+        """
+        return self._index._oldIndex
+
+
     def __repr__(self):
         return "<%s: %s>" % (self.__class__.__name__, self._path.path)
 
@@ -411,7 +471,6 @@
     def rename(self, name):
         oldName = self.name()
         self._renamedName = name
-        self._calendarHome
         self._calendarHome._newCalendars[name] = self
         self._calendarHome._removedCalendars.add(oldName)
         def doIt():
@@ -429,7 +488,7 @@
             self.calendarObjectWithName(name)
             for name in (
                 set(self._newCalendarObjects.iterkeys()) |
-                set(name for name in self._path.listdir() 
+                set(name for name in self._path.listdir()
                     if not name.startswith(".")) -
                 set(self._removedCalendarObjects)
             )),
@@ -447,7 +506,7 @@
 
         calendarObjectPath = self._path.child(name)
         if calendarObjectPath.isfile():
-            obj = CalendarObject(calendarObjectPath, self)
+            obj = CalendarObject(name, self)
             self._cachedCalendarObjects[name] = obj
             return obj
         else:
@@ -460,7 +519,7 @@
         for calendarObjectPath in self._path.children():
             if not _isValidName(calendarObjectPath.basename()):
                 continue
-            obj = CalendarObject(calendarObjectPath, self)
+            obj = CalendarObject(calendarObjectPath.basename(), self)
             if obj.component().resourceUID() == uid:
                 if obj.name() in self._removedCalendarObjects:
                     return None
@@ -475,7 +534,7 @@
         if calendarObjectPath.exists():
             raise CalendarObjectNameAlreadyExistsError(name)
 
-        calendarObject = CalendarObject(calendarObjectPath, self)
+        calendarObject = CalendarObject(name, self)
         calendarObject.setComponent(component)
         self._cachedCalendarObjects[name] = calendarObject
 
@@ -535,6 +594,14 @@
         props = PropertyStore(self._path)
         self._transaction.addOperation(props.flush)
         return props
+    
+    
+    def _doValidate(self, component):
+        # FIXME: should be separate class, not separate case!
+        if self.name() == 'inbox':
+            component.validateComponentsForCalDAV(True)
+        else:
+            component.validateForCalDAV()
 
 
 
@@ -546,12 +613,17 @@
     """
     implements(ICalendarObject)
 
-    def __init__(self, path, calendar):
-        self._path = path
+    def __init__(self, name, calendar):
+        self._name = name
         self._calendar = calendar
         self._component = None
 
 
+    @property
+    def _path(self):
+        return self._calendar._path.child(self._name)
+
+
     def __repr__(self):
         return "<%s: %s>" % (self.__class__.__name__, self._path.path)
 
@@ -575,7 +647,7 @@
             pass
 
         try:
-            component.validateForCalDAV()
+            self._calendar._doValidate(component)
         except InvalidICalendarDataError, e:
             raise InvalidCalendarComponentError(e)
 
@@ -677,7 +749,7 @@
 
 
 
-class Index (object):
+class Index(object):
     #
     # OK, here's where we get ugly.
     # The index code needs to be rewritten also, but in the meantime...
@@ -721,6 +793,7 @@
         self.calendar = calendar
         self._oldIndex = OldIndex(Index.StubResource(calendar))
 
+
     def calendarObjects(self):
         calendar = self.calendar
         for name, uid, componentType in self._oldIndex.bruteForceSearch():

Modified: CalendarServer/branches/new-store/txcaldav/calendarstore/test/test_file.py
===================================================================
--- CalendarServer/branches/new-store/txcaldav/calendarstore/test/test_file.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/txcaldav/calendarstore/test/test_file.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -25,7 +25,6 @@
 
 from txcaldav.icalendarstore import CalendarNameNotAllowedError
 from txcaldav.icalendarstore import CalendarObjectNameNotAllowedError
-from txcaldav.icalendarstore import CalendarAlreadyExistsError
 from txcaldav.icalendarstore import CalendarObjectUIDAlreadyExistsError
 from txcaldav.icalendarstore import NoSuchCalendarError
 from txcaldav.icalendarstore import NoSuchCalendarObjectError
@@ -34,8 +33,7 @@
 from txcaldav.calendarstore.file import Calendar, CalendarObject
 
 from txcaldav.calendarstore.test.common import (
-    CommonTests, calendar1_objectNames, event4_text, event1modified_text,
-    home1_calendarNames)
+    CommonTests, event4_text, event1modified_text)
 
 storePath = FilePath(__file__).parent().child("calendar_store")
 
@@ -176,6 +174,29 @@
         )
 
 
+    def test_useIndexImmediately(self):
+        """
+        L{Calendar._index} is usable in the same transaction it is created, with
+        a temporary filename.
+        """
+        self.home1.createCalendarWithName("calendar2")
+        calendar = self.home1.calendarWithName("calendar2")
+        index = calendar._index
+        self.assertEquals(set(index.calendarObjects()),
+                          set(calendar.calendarObjects()))
+        self.txn.commit()
+        self.txn = self.calendarStore.newTransaction()
+        self.home1 = self.txn.calendarHomeWithUID("home1")
+        calendar = self.home1.calendarWithName("calendar2")
+        # FIXME: we should be curating our own index here, but in order to fix
+        # that the code in the old implicit scheduler needs to change.  This
+        # test would be more effective if there were actually some objects in
+        # this list.
+        index = calendar._index
+        self.assertEquals(set(index.calendarObjects()),
+                          set(calendar.calendarObjects()))
+
+
     def test_calendarObjectWithName_dot(self):
         """
         Filenames starting with "." are reserved by this
@@ -298,6 +319,7 @@
             raise RuntimeError("oops")
         self.txn.addOperation(fail)
         self.assertRaises(RuntimeError, self.txn.commit)
+        self.assertEquals(len(self.flushLoggedErrors(RuntimeError)), 1)
         self._refresh()
 
 

Modified: CalendarServer/branches/new-store/txdav/propertystore/xattr.py
===================================================================
--- CalendarServer/branches/new-store/txdav/propertystore/xattr.py	2010-06-14 21:41:08 UTC (rev 5732)
+++ CalendarServer/branches/new-store/txdav/propertystore/xattr.py	2010-06-15 00:35:49 UTC (rev 5733)
@@ -90,9 +90,14 @@
         @type path: L{CachingFilePath}
         """
         self.path = path
-        self.attrs = xattr(path.path)
+        # self.attrs = xattr(path.path)
         self.removed = set()
         self.modified = {}
+        
+        
+    @property
+    def attrs(self):
+        return xattr(self.path.path)
 
     def __str__(self):
         return "<%s %s>" % (self.__class__.__name__, self.path.path)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20100614/d583c1fe/attachment-0001.html>


More information about the calendarserver-changes mailing list