[CalendarServer-changes] [14094] CalendarServer/trunk

source_changes at macosforge.org source_changes at macosforge.org
Tue Oct 21 09:51:27 PDT 2014


Revision: 14094
          http://trac.calendarserver.org//changeset/14094
Author:   cdaboo at apple.com
Date:     2014-10-21 09:51:27 -0700 (Tue, 21 Oct 2014)
Log Message:
-----------
Push more of the proxy/delegate logic into the store from the app-layer.

Modified Paths:
--------------
    CalendarServer/trunk/twistedcaldav/directory/calendaruserproxy.py
    CalendarServer/trunk/twistedcaldav/directory/principal.py
    CalendarServer/trunk/twistedcaldav/directory/test/test_principal.py
    CalendarServer/trunk/txdav/caldav/icalendardirectoryservice.py
    CalendarServer/trunk/txdav/who/delegates.py
    CalendarServer/trunk/txdav/who/directory.py

Modified: CalendarServer/trunk/twistedcaldav/directory/calendaruserproxy.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/calendaruserproxy.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/twistedcaldav/directory/calendaruserproxy.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -424,26 +424,24 @@
         return succeed([])
 
 
-    @inlineCallbacks
     def containsPrincipal(self, principal):
         """
-        Uses proxyFor information to turn the "contains principal" question around;
-        rather than expanding this principal's groups to see if the other principal
-        is a member, ask the other principal if they are a proxy for this principal's
-        parent resource, since this principal is a proxy principal.
+        Optimize this to ignore the owner principal when doing this test. This is a common occurrence as the owner
+        is the one most likely to be looking at their own resources, each of which will have the owner's two proxy
+        principals listed in the ACLs (and this method is called during each ACL L{checkPrivileges} call.
 
         @param principal: The principal to check
         @type principal: L{DirectoryCalendarPrincipalResource}
-        @return: True if principal is a proxy (of the correct type) of our parent
+        @return: L{True} if principal is a proxy (of the correct type) of our parent
         @rtype: C{boolean}
         """
-        readWrite = self.isProxyType(True)  # is read-write
-        if principal and self.parent in (yield principal.proxyFor(readWrite)):
-            returnValue(True)
-        returnValue(False)
 
+        if principal == self.parent:
+            return succeed(False)
+        return super(CalendarUserProxyPrincipalResource, self).containsPrincipal(principal)
 
 
+
 class ProxyDB(AbstractADBAPIDatabase):
     """
     A database to maintain calendar user proxy group memberships.

Modified: CalendarServer/trunk/twistedcaldav/directory/principal.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/principal.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/twistedcaldav/directory/principal.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -55,7 +55,6 @@
 from twistedcaldav.extensions import DirectoryElement
 from twistedcaldav.resource import CalendarPrincipalCollectionResource, CalendarPrincipalResource
 from txdav.caldav.datastore.scheduling.utils import normalizeCUAddr
-from txdav.who.delegates import RecordType as DelegateRecordType
 from txdav.who.directory import CalendarDirectoryRecordMixin
 from txdav.xml import element as davxml
 from txweb2 import responsecode
@@ -936,41 +935,27 @@
         return self.principalURL()
 
 
-    @inlineCallbacks
     def isProxyFor(self, principal):
         """
         Determine whether this principal is a read-only or read-write proxy for the
         specified principal.
+
+        @param principal: principal resource for the possible user proxying to this principal
+        @type principal: L{DirectoryPrincipalResource}
         """
+        return self.record.isProxyFor(principal.record)
 
-        read_uids = (yield self.proxyFor(False))
-        if principal in read_uids:
-            returnValue(True)
 
-        write_uids = (yield self.proxyFor(True))
-        if principal in write_uids:
-            returnValue(True)
-
-        returnValue(False)
-
-
-    @inlineCallbacks
     def proxyMode(self, principal):
         """
-        Determine whether what proxy mode this principal has in relation to the one specified.
+        Determine whether proxy mode this principal has in relation to the one specified.
+
+        @param principal: principal resource for the possible user proxying to this principal
+        @type principal: L{DirectoryPrincipalResource}
         """
+        return self.record.proxyMode(principal.record)
 
-        read_uids = (yield self.proxyFor(False))
-        if principal in read_uids:
-            returnValue("read")
 
-        write_uids = (yield self.proxyFor(True))
-        if principal in write_uids:
-            returnValue("write")
-
-        returnValue("none")
-
-
     @inlineCallbacks
     def proxyFor(self, readWrite, ignoreDisabled=True):
         """
@@ -990,28 +975,13 @@
         proxyFors = set()
 
         if config.EnableProxyPrincipals:
-            proxyRecordType = (
-                DelegateRecordType.writeDelegatorGroup if readWrite else
-                DelegateRecordType.readDelegatorGroup
-            )
-            proxyGroupRecord = yield self.record.service.recordWithShortName(
-                proxyRecordType, self.record.uid
-            )
-            if proxyGroupRecord is not None:
-                proxyForRecords = yield proxyGroupRecord.members()
+            proxyForRecords = yield self.record.proxyFor(readWrite, ignoreDisabled)
 
-                uids = set()
-                for record in tuple(proxyForRecords):
-                    if record.uid in uids:
-                        proxyForRecords.remove(record)
-                    else:
-                        uids.add(record.uid)
+            for record in proxyForRecords:
+                principal = yield self.parent.principalForRecord(record)
+                if principal is not None:
+                    proxyFors.add(principal)
 
-                for record in proxyForRecords:
-                    principal = yield self.parent.principalForRecord(record)
-                    if principal is not None and (not ignoreDisabled or principal.record.hasCalendars):
-                        proxyFors.add(principal)
-
         returnValue(proxyFors)
 
 

Modified: CalendarServer/trunk/twistedcaldav/directory/test/test_principal.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/test/test_principal.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/twistedcaldav/directory/test/test_principal.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -1096,3 +1096,118 @@
 
         self.assertTrue(len((yield principal01.proxyFor(False))) == 1)
         self.assertTrue(len((yield principal01.proxyFor(True))) == 0)
+
+
+    @inlineCallbacks
+    def test_isProxyFor(self):
+        """
+        Test that L{DirectoryPrincipalResource.proxyFor} returns the correct results.
+        """
+        principal01 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user01")))
+        principal02 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user02")))
+        principal03 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user03")))
+
+        # Check proxy for
+        self.assertFalse((yield principal01.isProxyFor(principal01)))
+        self.assertFalse((yield principal01.isProxyFor(principal02)))
+        self.assertFalse((yield principal01.isProxyFor(principal03)))
+        self.assertFalse((yield principal02.isProxyFor(principal01)))
+        self.assertFalse((yield principal02.isProxyFor(principal02)))
+        self.assertFalse((yield principal02.isProxyFor(principal03)))
+        self.assertFalse((yield principal03.isProxyFor(principal01)))
+        self.assertFalse((yield principal03.isProxyFor(principal02)))
+        self.assertFalse((yield principal03.isProxyFor(principal03)))
+
+        # Make user02 a read-only proxy for user01, and user03 a read-write proxy for user01
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal02.record, False)
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal03.record, True)
+        yield self.commit()
+
+        # Check proxy for
+        self.assertFalse((yield principal01.isProxyFor(principal01)))
+        self.assertFalse((yield principal01.isProxyFor(principal02)))
+        self.assertFalse((yield principal01.isProxyFor(principal03)))
+        self.assertTrue((yield principal02.isProxyFor(principal01)))
+        self.assertFalse((yield principal02.isProxyFor(principal02)))
+        self.assertFalse((yield principal02.isProxyFor(principal03)))
+        self.assertTrue((yield principal03.isProxyFor(principal01)))
+        self.assertFalse((yield principal03.isProxyFor(principal02)))
+        self.assertFalse((yield principal03.isProxyFor(principal03)))
+
+
+    @inlineCallbacks
+    def test_proxyMode(self):
+        """
+        Test that L{DirectoryPrincipalResource.proxyMode} returns the correct results.
+        """
+        principal01 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user01")))
+        principal02 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user02")))
+        principal03 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user03")))
+
+        # Check proxy mode
+        mode = yield principal02.proxyMode(principal01)
+        self.assertEqual(mode, "none")
+        mode = yield principal01.proxyMode(principal02)
+        self.assertEqual(mode, "none")
+        mode = yield principal03.proxyMode(principal01)
+        self.assertEqual(mode, "none")
+        mode = yield principal01.proxyMode(principal03)
+        self.assertEqual(mode, "none")
+
+        # Make user02 a read-only proxy for user01, and user03 a read-write proxy for user01
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal02.record, False)
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal03.record, True)
+        yield self.commit()
+
+        # Check proxy mode
+        mode = yield principal02.proxyMode(principal01)
+        self.assertEqual(mode, "read")
+        mode = yield principal01.proxyMode(principal02)
+        self.assertEqual(mode, "none")
+        mode = yield principal03.proxyMode(principal01)
+        self.assertEqual(mode, "write")
+        mode = yield principal01.proxyMode(principal03)
+        self.assertEqual(mode, "none")
+
+
+    @inlineCallbacks
+    def test_proxyFor(self):
+        """
+        Test that L{DirectoryPrincipalResource.proxyFor} returns the correct results.
+        """
+        principal01 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user01")))
+        principal02 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user02")))
+        principal03 = yield self.principalRootResource.principalForUID((yield self.userUIDFromShortName("user03")))
+
+        # Check proxy for
+        proxies = yield principal01.proxyFor(False)
+        self.assertEqual(proxies, set())
+        proxies = yield principal01.proxyFor(True)
+        self.assertEqual(proxies, set())
+        proxies = yield principal02.proxyFor(False)
+        self.assertEqual(proxies, set())
+        proxies = yield principal02.proxyFor(True)
+        self.assertEqual(proxies, set())
+        proxies = yield principal03.proxyFor(False)
+        self.assertEqual(proxies, set())
+        proxies = yield principal03.proxyFor(True)
+        self.assertEqual(proxies, set())
+
+        # Make user02 a read-only proxy for user01, and user03 a read-write proxy for user01
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal02.record, False)
+        yield addDelegate(self.transactionUnderTest(), principal01.record, principal03.record, True)
+        yield self.commit()
+
+        # Check proxy for
+        proxies = yield principal01.proxyFor(False)
+        self.assertEqual(proxies, set())
+        proxies = yield principal01.proxyFor(True)
+        self.assertEqual(proxies, set())
+        proxies = yield principal02.proxyFor(False)
+        self.assertEqual(proxies, set((principal01,)))
+        proxies = yield principal02.proxyFor(True)
+        self.assertEqual(proxies, set())
+        proxies = yield principal03.proxyFor(False)
+        self.assertEqual(proxies, set())
+        proxies = yield principal03.proxyFor(True)
+        self.assertEqual(proxies, set((principal01,)))

Modified: CalendarServer/trunk/txdav/caldav/icalendardirectoryservice.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/icalendardirectoryservice.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/txdav/caldav/icalendardirectoryservice.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -116,3 +116,27 @@
         @return: C{True} if it is a proxy.
         @rtype: C{bool}
         """
+
+    def proxyMode(other): #@NoSelf
+        """
+        Determine the proxy mode this record has in relation to the one specified.
+
+        @param other: record for the possible user proxying to this record
+        @type other: L{CalendarDirectoryRecordMixin}
+        """
+
+    def proxyFor(readWrite, ignoreDisabled=True): #@NoSelf
+        """
+        Returns the set of records currently delegating to this record
+        with the access indicated by the readWrite argument.  If readWrite is
+        True, then write-access delegators are returned, otherwise the read-
+        only-access delegators are returned.
+
+        @param readWrite: Whether to look up read-write delegators, or
+            read-only delegators
+        @type readWrite: L{bool}
+        @param ignoreDisabled: If L{True} disabled delegators are not returned
+        @type ignoreDisabled: L{bool}
+
+        @return: A Deferred firing with a set of records
+        """

Modified: CalendarServer/trunk/txdav/who/delegates.py
===================================================================
--- CalendarServer/trunk/txdav/who/delegates.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/txdav/who/delegates.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -99,6 +99,10 @@
         returnValue(records)
 
 
+    def expandedMembers(self):
+        return self.members(expanded=True)
+
+
     @inlineCallbacks
     def setMembers(self, memberRecords):
         """

Modified: CalendarServer/trunk/txdav/who/directory.py
===================================================================
--- CalendarServer/trunk/txdav/who/directory.py	2014-10-20 19:21:40 UTC (rev 14093)
+++ CalendarServer/trunk/txdav/who/directory.py	2014-10-21 16:51:27 UTC (rev 14094)
@@ -630,18 +630,70 @@
     # For scheduling/freebusy
     @inlineCallbacks
     def isProxyFor(self, other):
-        for recordType in (
-            DelegateRecordType.readDelegatorGroup,
-            DelegateRecordType.writeDelegatorGroup,
+        proxymode = yield self.proxyMode(other)
+        returnValue(proxymode != "none")
+
+
+    @inlineCallbacks
+    def proxyMode(self, other):
+        """
+        Determine the proxy mode this record has in relation to the one specified. Note that L{proxyFor} is
+        expensive when there are multiple server pods, so we should expand the proxy record members instead
+        (and that is cheaper).
+
+        @param other: record for the possible user proxying to this record
+        @type other: L{CalendarDirectoryRecordMixin}
+        """
+
+        for recordType, result in (
+            (DelegateRecordType.writeDelegateGroup, "write",),
+            (DelegateRecordType.readDelegateGroup, "read", ),
         ):
-            delegatorGroup = yield self.service.recordWithShortName(
-                recordType, self.uid
+            delegateGroup = yield other.service.recordWithShortName(
+                recordType, other.uid
             )
-            if delegatorGroup:
-                if other in (yield delegatorGroup.members()):
-                    returnValue(True)
+            if delegateGroup:
+                if self in (yield delegateGroup.expandedMembers()):
+                    returnValue(result)
 
+        returnValue("none")
 
+
+    @inlineCallbacks
+    def proxyFor(self, readWrite, ignoreDisabled=True):
+        """
+        Returns the set of records currently delegating to this record
+        with the access indicated by the readWrite argument.  If readWrite is
+        True, then write-access delegators are returned, otherwise the read-
+        only-access delegators are returned.
+
+        @param readWrite: Whether to look up read-write delegators, or
+            read-only delegators
+        @type readWrite: L{bool}
+        @param ignoreDisabled: If L{True} disabled delegators are not returned
+        @type ignoreDisabled: L{bool}
+
+        @return: A Deferred firing with a set of records
+        """
+        proxyFors = set()
+
+        proxyRecordType = (
+            DelegateRecordType.writeDelegatorGroup if readWrite else
+            DelegateRecordType.readDelegatorGroup
+        )
+        proxyGroupRecord = yield self.service.recordWithShortName(
+            proxyRecordType, self.uid
+        )
+        if proxyGroupRecord is not None:
+            proxyForRecords = yield proxyGroupRecord.members()
+
+            for record in proxyForRecords:
+                if not ignoreDisabled or record.hasCalendars:
+                    proxyFors.add(record)
+
+        returnValue(proxyFors)
+
+
     def attendeeProperty(self, params={}):
         """
         Returns a pycalendar ATTENDEE property for this record.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20141021/2a03ab50/attachment-0001.html>


More information about the calendarserver-changes mailing list