[CalendarServer-changes] [8331] CalendarServer/trunk
source_changes at macosforge.org
source_changes at macosforge.org
Fri Nov 18 10:38:51 PST 2011
Revision: 8331
http://trac.macosforge.org/projects/calendarserver/changeset/8331
Author: glyph at apple.com
Date: 2011-11-18 10:38:51 -0800 (Fri, 18 Nov 2011)
Log Message:
-----------
Restore proper error-handling behavior (404 on individual resource, not 500 on entire request) on multiget with concurrent delete.
Modified Paths:
--------------
CalendarServer/trunk/twext/web2/dav/method/prop_common.py
CalendarServer/trunk/twistedcaldav/method/report_multiget_common.py
CalendarServer/trunk/txdav/caldav/datastore/file.py
CalendarServer/trunk/txdav/caldav/datastore/sql.py
CalendarServer/trunk/txdav/caldav/datastore/test/common.py
CalendarServer/trunk/txdav/caldav/icalendarstore.py
CalendarServer/trunk/txdav/carddav/iaddressbookstore.py
CalendarServer/trunk/txdav/common/datastore/sql.py
CalendarServer/trunk/txdav/common/datastore/test/util.py
CalendarServer/trunk/txdav/common/icommondatastore.py
Property Changed:
----------------
CalendarServer/trunk/
Property changes on: CalendarServer/trunk
___________________________________________________________________
Modified: svn:mergeinfo
- /CalendarServer/branches/config-separation:4379-4443
/CalendarServer/branches/egg-info-351:4589-4625
/CalendarServer/branches/generic-sqlstore:6167-6191
/CalendarServer/branches/new-store:5594-5934
/CalendarServer/branches/new-store-no-caldavfile:5911-5935
/CalendarServer/branches/new-store-no-caldavfile-2:5936-5981
/CalendarServer/branches/users/cdaboo/batchupload-6699:6700-7198
/CalendarServer/branches/users/cdaboo/cached-subscription-calendars-5692:5693-5702
/CalendarServer/branches/users/cdaboo/directory-cache-on-demand-3627:3628-3644
/CalendarServer/branches/users/cdaboo/implicituidrace:8137-8141
/CalendarServer/branches/users/cdaboo/more-sharing-5591:5592-5601
/CalendarServer/branches/users/cdaboo/partition-4464:4465-4957
/CalendarServer/branches/users/cdaboo/pods:7297-7377
/CalendarServer/branches/users/cdaboo/pycalendar:7085-7206
/CalendarServer/branches/users/cdaboo/pycard:7227-7237
/CalendarServer/branches/users/cdaboo/queued-attendee-refreshes:7740-8287
/CalendarServer/branches/users/cdaboo/relative-config-paths-5070:5071-5105
/CalendarServer/branches/users/cdaboo/shared-calendars-5187:5188-5440
/CalendarServer/branches/users/cdaboo/timezones:7443-7699
/CalendarServer/branches/users/glyph/conn-limit:6574-6577
/CalendarServer/branches/users/glyph/contacts-server-merge:4971-5080
/CalendarServer/branches/users/glyph/dalify:6932-7023
/CalendarServer/branches/users/glyph/db-reconnect:6824-6876
/CalendarServer/branches/users/glyph/deploybuild:7563-7572
/CalendarServer/branches/users/glyph/disable-quota:7718-7727
/CalendarServer/branches/users/glyph/dont-start-postgres:6592-6614
/CalendarServer/branches/users/glyph/imip-and-admin-html:7866-7984
/CalendarServer/branches/users/glyph/linux-tests:6893-6900
/CalendarServer/branches/users/glyph/misc-portability-fixes:7365-7374
/CalendarServer/branches/users/glyph/more-deferreds-6:6322-6368
/CalendarServer/branches/users/glyph/more-deferreds-7:6369-6445
/CalendarServer/branches/users/glyph/new-export:7444-7485
/CalendarServer/branches/users/glyph/oracle:7106-7155
/CalendarServer/branches/users/glyph/oracle-nulls:7340-7351
/CalendarServer/branches/users/glyph/other-html:8062-8091
/CalendarServer/branches/users/glyph/parallel-sim:8240-8251
/CalendarServer/branches/users/glyph/quota:7604-7637
/CalendarServer/branches/users/glyph/sendfdport:5388-5424
/CalendarServer/branches/users/glyph/shared-pool-take2:8155-8174
/CalendarServer/branches/users/glyph/sharedpool:6490-6550
/CalendarServer/branches/users/glyph/sql-store:5929-6073
/CalendarServer/branches/users/glyph/subtransactions:7248-7258
/CalendarServer/branches/users/glyph/uidexport:7673-7676
/CalendarServer/branches/users/glyph/use-system-twisted:5084-5149
/CalendarServer/branches/users/glyph/xattrs-from-files:7757-7769
/CalendarServer/branches/users/sagen/applepush:8126-8184
/CalendarServer/branches/users/sagen/inboxitems:7380-7381
/CalendarServer/branches/users/sagen/locations-resources:5032-5051
/CalendarServer/branches/users/sagen/locations-resources-2:5052-5061
/CalendarServer/branches/users/sagen/purge_old_events:6735-6746
/CalendarServer/branches/users/sagen/resource-delegates-4038:4040-4067
/CalendarServer/branches/users/sagen/resource-delegates-4066:4068-4075
/CalendarServer/branches/users/sagen/resources-2:5084-5093
/CalendarServer/branches/users/wsanchez/transations:5515-5593
+ /CalendarServer/branches/config-separation:4379-4443
/CalendarServer/branches/egg-info-351:4589-4625
/CalendarServer/branches/generic-sqlstore:6167-6191
/CalendarServer/branches/new-store:5594-5934
/CalendarServer/branches/new-store-no-caldavfile:5911-5935
/CalendarServer/branches/new-store-no-caldavfile-2:5936-5981
/CalendarServer/branches/users/cdaboo/batchupload-6699:6700-7198
/CalendarServer/branches/users/cdaboo/cached-subscription-calendars-5692:5693-5702
/CalendarServer/branches/users/cdaboo/directory-cache-on-demand-3627:3628-3644
/CalendarServer/branches/users/cdaboo/implicituidrace:8137-8141
/CalendarServer/branches/users/cdaboo/more-sharing-5591:5592-5601
/CalendarServer/branches/users/cdaboo/partition-4464:4465-4957
/CalendarServer/branches/users/cdaboo/pods:7297-7377
/CalendarServer/branches/users/cdaboo/pycalendar:7085-7206
/CalendarServer/branches/users/cdaboo/pycard:7227-7237
/CalendarServer/branches/users/cdaboo/queued-attendee-refreshes:7740-8287
/CalendarServer/branches/users/cdaboo/relative-config-paths-5070:5071-5105
/CalendarServer/branches/users/cdaboo/shared-calendars-5187:5188-5440
/CalendarServer/branches/users/cdaboo/timezones:7443-7699
/CalendarServer/branches/users/glyph/conn-limit:6574-6577
/CalendarServer/branches/users/glyph/contacts-server-merge:4971-5080
/CalendarServer/branches/users/glyph/dalify:6932-7023
/CalendarServer/branches/users/glyph/db-reconnect:6824-6876
/CalendarServer/branches/users/glyph/deploybuild:7563-7572
/CalendarServer/branches/users/glyph/disable-quota:7718-7727
/CalendarServer/branches/users/glyph/dont-start-postgres:6592-6614
/CalendarServer/branches/users/glyph/imip-and-admin-html:7866-7984
/CalendarServer/branches/users/glyph/linux-tests:6893-6900
/CalendarServer/branches/users/glyph/misc-portability-fixes:7365-7374
/CalendarServer/branches/users/glyph/more-deferreds-6:6322-6368
/CalendarServer/branches/users/glyph/more-deferreds-7:6369-6445
/CalendarServer/branches/users/glyph/multiget-delete:8321-8330
/CalendarServer/branches/users/glyph/new-export:7444-7485
/CalendarServer/branches/users/glyph/oracle:7106-7155
/CalendarServer/branches/users/glyph/oracle-nulls:7340-7351
/CalendarServer/branches/users/glyph/other-html:8062-8091
/CalendarServer/branches/users/glyph/parallel-sim:8240-8251
/CalendarServer/branches/users/glyph/quota:7604-7637
/CalendarServer/branches/users/glyph/sendfdport:5388-5424
/CalendarServer/branches/users/glyph/shared-pool-take2:8155-8174
/CalendarServer/branches/users/glyph/sharedpool:6490-6550
/CalendarServer/branches/users/glyph/sql-store:5929-6073
/CalendarServer/branches/users/glyph/subtransactions:7248-7258
/CalendarServer/branches/users/glyph/uidexport:7673-7676
/CalendarServer/branches/users/glyph/use-system-twisted:5084-5149
/CalendarServer/branches/users/glyph/xattrs-from-files:7757-7769
/CalendarServer/branches/users/sagen/applepush:8126-8184
/CalendarServer/branches/users/sagen/inboxitems:7380-7381
/CalendarServer/branches/users/sagen/locations-resources:5032-5051
/CalendarServer/branches/users/sagen/locations-resources-2:5052-5061
/CalendarServer/branches/users/sagen/purge_old_events:6735-6746
/CalendarServer/branches/users/sagen/resource-delegates-4038:4040-4067
/CalendarServer/branches/users/sagen/resource-delegates-4066:4068-4075
/CalendarServer/branches/users/sagen/resources-2:5084-5093
/CalendarServer/branches/users/wsanchez/transations:5515-5593
Modified: CalendarServer/trunk/twext/web2/dav/method/prop_common.py
===================================================================
--- CalendarServer/trunk/twext/web2/dav/method/prop_common.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/twext/web2/dav/method/prop_common.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -93,10 +93,10 @@
properties_by_status[responsecode.OK].append(prop)
except:
f = Failure()
-
- log.err("Error reading property %r for resource %s: %s" % (qname, request.uri, f.value))
-
status = statusForFailure(f, "getting property: %s" % (qname,))
+ if status != responsecode.NOT_FOUND:
+ log.err("Error reading property %r for resource %s: %s" %
+ (qname, request.uri, f.value))
if status not in properties_by_status: properties_by_status[status] = []
properties_by_status[status].append(propertyName(qname))
else:
Modified: CalendarServer/trunk/twistedcaldav/method/report_multiget_common.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/method/report_multiget_common.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/twistedcaldav/method/report_multiget_common.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -38,6 +38,7 @@
from twistedcaldav.carddavxml import carddav_namespace
from twistedcaldav.config import config
from twistedcaldav.method import report_common
+from txdav.common.icommondatastore import ConcurrentModification
from twistedcaldav.method.report_common import COLLECTION_TYPE_CALENDAR,\
COLLECTION_TYPE_ADDRESSBOOK
from twistedcaldav.query import addressbookqueryfilter
@@ -217,18 +218,28 @@
# Get properties for all valid readable resources
for resource, href in ok_resources:
try:
- yield report_common.responseForHref(request, responses, davxml.HRef.fromString(href), resource, propertiesForResource, propertyreq, isowner=isowner)
+ yield report_common.responseForHref(
+ request, responses, davxml.HRef.fromString(href),
+ resource, propertiesForResource, propertyreq,
+ isowner=isowner
+ )
except ValueError:
- log.err("Invalid calendar resource during multiget: %s" % (href,))
- responses.append(davxml.StatusResponse(davxml.HRef.fromString(href), davxml.Status.fromResponseCode(responsecode.FORBIDDEN)))
- except IOError:
+ log.err("Invalid calendar resource during multiget: %s" %
+ (href,))
+ responses.append(davxml.StatusResponse(
+ davxml.HRef.fromString(href),
+ davxml.Status.fromResponseCode(responsecode.FORBIDDEN)))
+ except ConcurrentModification:
# This can happen because of a race-condition between the
# time we determine which resources exist and the deletion
# of one of these resources in another request. In this
# case, return a 404 for the now missing resource rather
# than raise an error for the entire report.
log.err("Missing resource during multiget: %s" % (href,))
- responses.append(davxml.StatusResponse(davxml.HRef.fromString(href), davxml.Status.fromResponseCode(responsecode.NOT_FOUND)))
+ responses.append(davxml.StatusResponse(
+ davxml.HRef.fromString(href),
+ davxml.Status.fromResponseCode(responsecode.NOT_FOUND)
+ ))
# Indicate error for all valid non-readable resources
for ignore_resource, href in bad_resources:
Modified: CalendarServer/trunk/txdav/caldav/datastore/file.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/datastore/file.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/caldav/datastore/file.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -62,8 +62,8 @@
CommonObjectResource, CommonStubResource)
from txdav.caldav.icalendarstore import QuotaExceeded
-from txdav.common.icommondatastore import (NoSuchObjectResourceError,
- InternalDataStoreError)
+from txdav.common.icommondatastore import ConcurrentModification
+from txdav.common.icommondatastore import InternalDataStoreError
from txdav.base.datastore.file import writeOperation, hidden, FileMetaDataMixin
from txdav.base.propertystore.base import PropertyName
@@ -375,7 +375,7 @@
fh = self._path.open()
except IOError, e:
if e[0] == ENOENT:
- raise NoSuchObjectResourceError(self)
+ raise ConcurrentModification()
else:
raise
Modified: CalendarServer/trunk/txdav/caldav/datastore/sql.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/datastore/sql.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/caldav/datastore/sql.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -647,9 +647,10 @@
@inlineCallbacks
def component(self):
"""
- Read calendar data and validate/fix it. Do not raise a store error here if there are unfixable
- errors as that could prevent the overall request to fail. Instead we will hand bad data off to
- the caller - that is not ideal but in theory we should have checked everything on the way in and
+ Read calendar data and validate/fix it. Do not raise a store error here
+ if there are unfixable errors as that could prevent the overall request
+ to fail. Instead we will hand bad data off to the caller - that is not
+ ideal but in theory we should have checked everything on the way in and
only allowed in good data.
"""
text = yield self._text()
@@ -667,10 +668,12 @@
fixed, unfixed = component.validCalendarData(doFix=True, doRaise=False)
if unfixed:
- self.log_error("Calendar data id=%s had unfixable problems:\n %s" % (self._resourceID, "\n ".join(unfixed),))
-
+ self.log_error("Calendar data id=%s had unfixable problems:\n %s" %
+ (self._resourceID, "\n ".join(unfixed),))
+
if fixed:
- self.log_error("Calendar data id=%s had fixable problems:\n %s" % (self._resourceID, "\n ".join(fixed),))
+ self.log_error("Calendar data id=%s had fixable problems:\n %s" %
+ (self._resourceID, "\n ".join(fixed),))
returnValue(component)
Modified: CalendarServer/trunk/txdav/caldav/datastore/test/common.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/datastore/test/common.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/caldav/datastore/test/common.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -55,6 +55,7 @@
from txdav.caldav.icalendarstore import QuotaExceeded
from txdav.common.datastore.test.util import deriveQuota
from txdav.common.datastore.test.util import withSpecialQuota
+from txdav.common.icommondatastore import ConcurrentModification
from twistedcaldav.ical import Component
storePath = FilePath(__file__).parent().child("calendar_store")
@@ -257,34 +258,33 @@
@inlineCallbacks
- def homeUnderTest(self):
+ def homeUnderTest(self, txn=None):
"""
Get the calendar home detailed by C{requirements['home1']}.
"""
- returnValue(
- (yield self.transactionUnderTest().calendarHomeWithUID("home1"))
- )
+ if txn is None:
+ txn = self.transactionUnderTest()
+ returnValue((yield txn.calendarHomeWithUID("home1")))
@inlineCallbacks
- def calendarUnderTest(self):
+ def calendarUnderTest(self, txn=None):
"""
Get the calendar detailed by C{requirements['home1']['calendar_1']}.
"""
returnValue((yield
- (yield self.homeUnderTest()).calendarWithName("calendar_1"))
+ (yield self.homeUnderTest(txn)).calendarWithName("calendar_1"))
)
@inlineCallbacks
- def calendarObjectUnderTest(self, name="1.ics"):
+ def calendarObjectUnderTest(self, name="1.ics", txn=None):
"""
Get the calendar detailed by
C{requirements['home1']['calendar_1'][name]}.
"""
- returnValue(
- (yield (yield self.calendarUnderTest())
- .calendarObjectWithName(name)))
+ returnValue((yield (yield self.calendarUnderTest(txn))
+ .calendarObjectWithName(name)))
def test_calendarStoreProvides(self):
@@ -731,6 +731,29 @@
@inlineCallbacks
+ def test_calendarObjectRemoveConcurrent(self):
+ """
+ If a transaction, C{A}, is examining an L{ICalendarObject} C{O} while
+ another transaction, C{B}, deletes O, L{O.component()} should raise
+ L{ConcurrentModification}. (This assumes that we are in the default
+ serialization level, C{READ COMMITTED}. This test might fail if
+ something changes that.)
+ """
+ calendarObject = yield self.calendarObjectUnderTest()
+ ctxn = self.concurrentTransaction()
+ calendar1prime = yield self.calendarUnderTest(ctxn)
+ yield calendar1prime.removeCalendarObjectWithName("1.ics")
+ yield ctxn.commit()
+ try:
+ retrieval = yield calendarObject.component()
+ except ConcurrentModification:
+ pass
+ else:
+ self.fail("ConcurrentModification not raised, %r returned." %
+ (retrieval,))
+
+
+ @inlineCallbacks
def test_ownerCalendarHome(self):
"""
L{ICalendar.ownerCalendarHome} should match the home UID.
Modified: CalendarServer/trunk/txdav/caldav/icalendarstore.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/icalendarstore.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/caldav/icalendarstore.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -368,6 +368,10 @@
"""
Retrieve the calendar component for this calendar object.
+ @raise ConcurrentModification: if this L{ICalendarObject} has been
+ deleted and committed by another transaction between its creation
+ and the first call to this method.
+
@return: a C{VCALENDAR} L{VComponent}.
"""
Modified: CalendarServer/trunk/txdav/carddav/iaddressbookstore.py
===================================================================
--- CalendarServer/trunk/txdav/carddav/iaddressbookstore.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/carddav/iaddressbookstore.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -254,6 +254,10 @@
"""
Retrieve the addressbook component for this addressbook object.
+ @raise ConcurrentModification: if this L{IAddressBookObject} has been
+ deleted and committed by another transaction between its creation
+ and the first call to this method.
+
@return: a C{VCARD} L{VComponent}.
"""
Modified: CalendarServer/trunk/txdav/common/datastore/sql.py
===================================================================
--- CalendarServer/trunk/txdav/common/datastore/sql.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/common/datastore/sql.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -75,6 +75,7 @@
from txdav.base.propertystore.none import PropertyStore as NonePropertyStore
from txdav.base.propertystore.sql import PropertyStore
+from txdav.common.icommondatastore import ConcurrentModification
from twistedcaldav.customxml import NotificationType
from twistedcaldav.dateops import datetimeMktime, parseSQLTimestamp,\
pyCalendarTodatetime
@@ -2590,12 +2591,16 @@
@inlineCallbacks
def _text(self):
if self._objectText is None:
- text = (
+ texts = (
yield self._textByIDQuery.on(self._txn,
resourceID=self._resourceID)
- )[0][0]
- self._objectText = text
- returnValue(text)
+ )
+ if texts:
+ text = texts[0][0]
+ self._objectText = text
+ returnValue(text)
+ else:
+ raise ConcurrentModification()
else:
returnValue(self._objectText)
Modified: CalendarServer/trunk/txdav/common/datastore/test/util.py
===================================================================
--- CalendarServer/trunk/txdav/common/datastore/test/util.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/common/datastore/test/util.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -44,6 +44,7 @@
from twext.enterprise.adbapi2 import ConnectionPool
from twisted.internet.defer import returnValue
from twistedcaldav.notify import Notifier, NodeCreationException
+from twext.enterprise.ienterprise import AlreadyFinishedError
from twistedcaldav.vcard import Component as ABComponent
md5key = PropertyName.fromElement(TwistedGETContentMD5)
@@ -400,7 +401,6 @@
lastTransaction = None
savedStore = None
assertProvides = assertProvides
- lastCommitSetUp = False
def transactionUnderTest(self):
"""
@@ -408,17 +408,29 @@
C[lastTransaction}. Also makes sure to use the same store, saving the
value from C{storeUnderTest}.
"""
- if not self.lastCommitSetUp:
- self.lastCommitSetUp = True
- self.addCleanup(self.commitLast)
- if self.lastTransaction is not None:
- return self.lastTransaction
+ if self.lastTransaction is None:
+ self.lastTransaction = self.concurrentTransaction()
+ return self.lastTransaction
+
+
+ def concurrentTransaction(self):
+ """
+ Create a transaction from C{storeUnderTest} and save it for later
+ clean-up.
+ """
if self.savedStore is None:
self.savedStore = self.storeUnderTest()
self.counter += 1
- txn = self.lastTransaction = self.savedStore.newTransaction(
+ txn = self.savedStore.newTransaction(
self.id() + " #" + str(self.counter)
)
+ @inlineCallbacks
+ def maybeCommitThis():
+ try:
+ yield txn.commit()
+ except AlreadyFinishedError:
+ pass
+ self.addCleanup(maybeCommitThis)
return txn
Modified: CalendarServer/trunk/txdav/common/icommondatastore.py
===================================================================
--- CalendarServer/trunk/txdav/common/icommondatastore.py 2011-11-18 18:14:39 UTC (rev 8330)
+++ CalendarServer/trunk/txdav/common/icommondatastore.py 2011-11-18 18:38:51 UTC (rev 8331)
@@ -35,6 +35,7 @@
"NotFoundError",
"NoSuchHomeChildError",
"NoSuchObjectResourceError",
+ "ConcurrentModification",
"InvalidObjectResourceError",
"InternalDataStoreError",
]
@@ -98,6 +99,18 @@
The requested object resource does not exist.
"""
+class ConcurrentModification(NotFoundError):
+ """
+ Despite being loaded in the current transaction, the object whose data is
+ being requested has been deleted or modified in another transaction, and
+ therefore that data can no longer be retrieved.
+
+ (Note: in the future we should be able to avoid these types of errors with
+ more usage of locking, but until the impact of that on performance is
+ determined, callers of C{component()} need to be aware that this can
+ happen.)
+ """
+
class InvalidObjectResourceError(CommonStoreError):
"""
Invalid object resource data.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20111118/85cd18ac/attachment-0001.html>
More information about the calendarserver-changes
mailing list