[Checkins] SVN: zope.locking/branches/patricks-fix/src/zope/locking/ Bug fixes, better test coverage
Patrick Strawderman
patrick at zope.com
Fri Mar 11 19:46:51 EST 2011
Log message for revision 120881:
Bug fixes, better test coverage
Changed:
U zope.locking/branches/patricks-fix/src/zope/locking/CHANGES.txt
U zope.locking/branches/patricks-fix/src/zope/locking/README.txt
U zope.locking/branches/patricks-fix/src/zope/locking/adapters.py
U zope.locking/branches/patricks-fix/src/zope/locking/annoying.txt
U zope.locking/branches/patricks-fix/src/zope/locking/cleanup.txt
U zope.locking/branches/patricks-fix/src/zope/locking/generations.py
U zope.locking/branches/patricks-fix/src/zope/locking/tokens.py
U zope.locking/branches/patricks-fix/src/zope/locking/utility.py
-=-
Modified: zope.locking/branches/patricks-fix/src/zope/locking/CHANGES.txt
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/CHANGES.txt 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/CHANGES.txt 2011-03-12 00:46:51 UTC (rev 120881)
@@ -6,7 +6,17 @@
1.3 (unreleased)
----------------
+- Bug fix: fix an issue with tokens; they are no longer stored as keys
+ in BTrees to avoid ordering issues after conflict resolution.
+ zope.keyreference handles this properly, so use key references
+ instead.
+- Bug fix: TokenHandler's _checkInteraction method would raise an
+ incorrect exception when the token was ended.
+
+- 100% test coverage.
+
+
------------------
1.2.2 (2011-01-31)
------------------
Modified: zope.locking/branches/patricks-fix/src/zope/locking/README.txt
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/README.txt 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/README.txt 2011-03-12 00:46:51 UTC (rev 120881)
@@ -355,6 +355,13 @@
>>> ev.old == lock.started + four
True
+ >>> remaining = lock.remaining_duration
+ >>> lock.remaining_duration = None
+ >>> lock.expiration is None
+ True
+
+ >>> lock.remaining_duration = remaining
+
Now, we'll hack our code to make it think that it's a day later. It is very
important to remember that a lock ending with a timeout ends silently--that
is, no event is fired.
@@ -394,6 +401,11 @@
in a state that is not fully initialized.
>>> lock = tokens.ExclusiveLock(demo, 'john')
+ >>> lock.duration is None
+ True
+
+ >>> lock.duration = datetime.timedelta(1)
+
>>> lock.started # doctest: +ELLIPSIS
Traceback (most recent call last):
...
@@ -403,6 +415,40 @@
...
UnregisteredError: ...
+ >>> lock.expiration # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ UnregisteredError: ...
+
+ >>> lock.remaining_duration # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ UnregisteredError: ...
+
+ >>> t = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)
+ >>> lock.expiration = t # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ UnregisteredError: ...
+
+ >>> lock.remaining_duration = datetime.timedelta(1) # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ UnregisteredError: ...
+
+ >>> lock = util.register(lock)
+ >>> lock.end()
+ >>> lock.expiration = t # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ EndedError
+
+ >>> lock.remaining_duration = datetime.timedelta(1) # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ EndedError
+
+
------------
Shared Locks
------------
@@ -688,6 +734,9 @@
...
ParticipationError
+ >>> broker.get() is None
+ True
+
If we set up an interaction with one participation, the lock will have a
better chance.
@@ -723,9 +772,12 @@
True
>>> util.get(demo) is token
True
+ >>> token is broker.get()
+ True
>>> token.end()
You can only specify principals that are in the current interaction.
+# doctest + ELLIPSIS
>>> token = broker.lock('joe')
>>> sorted(token.principal_ids)
@@ -741,6 +793,19 @@
>>> token = broker.lock(duration=two)
>>> token.duration == two
True
+
+ >>> token.expiration is not None
+ True
+
+ >>> token.expiration == token.started + token.duration
+ True
+
+The duration can be set to `None`.
+
+ >>> token.duration = None
+ >>> token.expiration is None
+ True
+
>>> token.end()
If the interaction has more than one principal, a principal (in the
@@ -962,6 +1027,22 @@
ParticipationError: ...
>>> lock.end()
+Attempting to modify a token that has ended results in an EndedError
+being raised.
+
+ >>> handler.expiration = handler.started + three
+ Traceback (most recent call last):
+ ...
+ EndedError
+ >>> handler.duration = remaining_duration = two
+ Traceback (most recent call last):
+ ...
+ EndedError
+ >>> handler.release()
+ Traceback (most recent call last):
+ ...
+ EndedError
+
SharedLockHandlers
------------------
@@ -1010,6 +1091,11 @@
...
ParticipationError: ...
+ >>> handler.release(("joe",)) # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ParticipationError: ...
+
The shared lock handler adds two additional methods to a standard handler:
`join` and `add`. They do similar jobs, but are separate to allow separate
security settings for each. The `join` method lets some or all of the
@@ -1034,9 +1120,36 @@
Traceback (most recent call last):
...
ParticipationError: ...
- >>> lock.end()
+ >>> handler.join()
+ >>> sorted(handler.principal_ids)
+ ['joe', 'mary', 'susan']
+
+ >>> handler.release()
+ >>> sorted(handler.principal_ids)
+ ['mary', 'susan']
+
+ >>> handler.join()
+ >>> handler.release(('mary', 'susan',))
+ Traceback (most recent call last):
+ ...
+ ValueError: mary
+
+ >>> handler.release(('joe',))
+ >>> sorted(handler.principal_ids)
+ ['mary', 'susan']
+
>>> zope.security.management.endInteraction()
+Calling `join` without an interaction results in a ValueError being
+raised.
+
+ >>> handler.join()
+ Traceback (most recent call last):
+ ...
+ ValueError
+
+ >>> lock.end()
+
--------
Warnings
--------
Modified: zope.locking/branches/patricks-fix/src/zope/locking/adapters.py
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/adapters.py 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/adapters.py 2011-03-12 00:46:51 UTC (rev 120881)
@@ -71,7 +71,7 @@
def _checkInteraction(self):
if self.token.ended is not None:
- raise interfaces.ExpirationChangedEvent
+ raise interfaces.EndedError
interaction_principals = getInteractionPrincipals()
token_principals = frozenset(self.token.principal_ids)
if interaction_principals is not None:
Modified: zope.locking/branches/patricks-fix/src/zope/locking/annoying.txt
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/annoying.txt 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/annoying.txt 2011-03-12 00:46:51 UTC (rev 120881)
@@ -343,8 +343,79 @@
...
EndedError
+A token cannot be registered with multiple utilities.
+
+ >>> util2 = zope.locking.utility.TokenUtility()
+ >>> conn.add(util2)
+ >>> token = tokens.EndableFreeze(demo)
+ >>> _ = util.register(token)
+ >>> _ = util2.register(token)
+ Traceback (most recent call last):
+ ...
+ ValueError: Lock is already registered with another utility
+
+ >>> token.utility = util2
+ Traceback (most recent call last):
+ ...
+ ValueError: cannot reset utility
+
+ >>> token.end()
+ >>> token = tokens.Token(demo)
+ >>> token.utility = util2
+ >>> token.utility = util
+ Traceback (most recent call last):
+ ...
+ ValueError: cannot reset utility
+
+
+ >>> token = tokens.SharedLock(demo, ("Henry",))
+ >>> _ = util2.register(token)
+ >>> del events[:]
+ >>> token.remove([])
+ >>> len(events)
+ 0
+
+The TokenHandler base class does not implement its `release` method.
+
+ >>> import zope.locking.adapters
+ >>> handler = zope.locking.adapters.TokenHandler(token)
+ >>> handler.release()
+ Traceback (most recent call last):
+ ...
+ NotImplementedError
+
+ >>> token.duration = "" # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: duration must be datetime.timedelta
+
+ >>> token.remaining_duration = "" # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: duration must be datetime.timedelta
+
+ >>> token.duration = datetime.timedelta(-1) # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: duration may not be negative
+
+ >>> token.remaining_duration = datetime.timedelta(-1) # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: duration may not be negative
+
+ >>> token.expiration = "" # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: expiration must be datetime.datetime
+
+ >>> token.expiration = datetime.datetime.utcnow() # doctest: +ELLIPSIS
+ Traceback (most recent call last):
+ ...
+ ValueError: expiration must be timezone-aware
+
We'll undo the hacks, and also end the token (that is no longer ended once
the hack is finished).
+ >>> token.end()
>>> zope.locking.utils.now = oldNow # undo the hack
- >>> token.end()
Modified: zope.locking/branches/patricks-fix/src/zope/locking/cleanup.txt
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/cleanup.txt 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/cleanup.txt 2011-03-12 00:46:51 UTC (rev 120881)
@@ -95,9 +95,9 @@
>>> sorted(util._principal_ids)
['john', 'mary']
- >>> list(util._principal_ids['john']) == [lock]
+ >>> list(util._principal_ids['john']) == [IKeyReference(lock)]
True
- >>> list(util._principal_ids['mary']) == [lock]
+ >>> list(util._principal_ids['mary']) == [IKeyReference(lock)]
True
And `_expirations` has a single entry: the one hour duration, mapped to a set
@@ -107,7 +107,7 @@
1
>>> iter(util._expirations).next() == lock.expiration
True
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
Token Modification
@@ -140,7 +140,7 @@
>>> sorted(util._principal_ids)
['susan']
- >>> list(util._principal_ids['susan']) == [lock]
+ >>> list(util._principal_ids['susan']) == [IKeyReference(lock)]
True
And `_expirations` has a single entry: the two hour duration, mapped to a set
@@ -150,7 +150,7 @@
1
>>> iter(util._expirations).next() == lock.expiration
True
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
Adding a Freeze
@@ -180,7 +180,7 @@
['susan']
>>> len(util._expirations)
1
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
Expiration
@@ -217,7 +217,7 @@
['susan']
>>> len(util._expirations)
1
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
The changes won't be made for the expired lock until we register a new lock.
@@ -236,7 +236,7 @@
['john']
>>> len(util._expirations)
1
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
We just looked at adding a token for one object that removed the index of
@@ -264,7 +264,7 @@
['john']
>>> len(util._expirations)
1
- >>> list(util._expirations[lock.expiration]) == [lock]
+ >>> list(util._expirations[lock.expiration]) == [IKeyReference(lock)]
True
Now, when we create a new token for the same object, the indexes are again
@@ -286,7 +286,8 @@
['mary']
>>> len(util._expirations)
1
- >>> list(util._expirations[new_lock.expiration]) == [new_lock]
+ >>> list(util._expirations[new_lock.expiration]) == [
+ ... IKeyReference(new_lock)]
True
An issue arose when two or more expired locks are stored in the utility. When
@@ -313,7 +314,8 @@
>>> len(util._expirations)
1
- >>> list(util._expirations[third_lock.expiration]) == [third_lock]
+ >>> list(util._expirations[third_lock.expiration]) == [
+ ... IKeyReference(third_lock)]
True
Explicit Ending
Modified: zope.locking/branches/patricks-fix/src/zope/locking/generations.py
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/generations.py 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/generations.py 2011-03-12 00:46:51 UTC (rev 120881)
@@ -1,6 +1,7 @@
import BTrees.OOBTree
import zope.app.generations.interfaces
import zope.interface
+import zope.keyreference.interfaces
import zope.locking.interfaces
import zope.locking.utils
@@ -10,8 +11,8 @@
zope.interface.implements(
zope.app.generations.interfaces.IInstallableSchemaManager)
- minimum_generation = 2
- generation = 2
+ minimum_generation = 3
+ generation = 3
def install(self, context):
# Clean up cruft in any existing token utilities.
@@ -20,7 +21,7 @@
clean_locks(context)
def evolve(self, context, generation):
- if generation == 2:
+ if generation <= 2:
# Going from generation 1 -> 2, we need to run the token
# utility fixer again because of a deficiency it had in 1.2.
clean_locks(context)
@@ -62,7 +63,9 @@
for pid in list(util._principal_ids):
# iterForPrincipalId only returns non-ended locks, so we know
# they're still good.
- new_tree = BTrees.OOBTree.OOTreeSet(util.iterForPrincipalId(pid))
+ new_tree = BTrees.OOBTree.OOTreeSet()
+ for token in util.iterForPrincipalId(pid):
+ new_tree.add(zope.keyreference.interfaces.IKeyReference(token))
if new_tree:
util._principal_ids[pid] = new_tree
else:
@@ -70,7 +73,10 @@
now = zope.locking.utils.now()
for dt, tree in list(util._expirations.items()):
if dt > now:
- util._expirations[dt] = BTrees.OOBTree.OOTreeSet(tree)
+ new_tree = BTrees.OOBTree.OOTreeSet()
+ for token in tree:
+ new_tree.add(zope.keyreference.interfaces.IKeyReference(token))
+ util._expirations[dt] = new_tree
else:
del util._expirations[dt]
for token in tree:
Modified: zope.locking/branches/patricks-fix/src/zope/locking/tokens.py
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/tokens.py 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/tokens.py 2011-03-12 00:46:51 UTC (rev 120881)
@@ -47,9 +47,6 @@
self._started = utils.now()
return property(get, set)
- def __cmp__(self, other):
- return cmp((self._p_jar.db().database_name, self._p_oid),
- (other._p_jar.db().database_name, other._p_oid))
class EndableToken(Token):
Modified: zope.locking/branches/patricks-fix/src/zope/locking/utility.py
===================================================================
--- zope.locking/branches/patricks-fix/src/zope/locking/utility.py 2011-03-12 00:41:14 UTC (rev 120880)
+++ zope.locking/branches/patricks-fix/src/zope/locking/utility.py 2011-03-12 00:46:51 UTC (rev 120881)
@@ -22,7 +22,7 @@
def _del(self, tree, token, value):
"""remove a token for a value within either of the two index trees"""
reg = tree[value]
- reg.remove(token)
+ reg.remove(IKeyReference(token))
if not reg:
del tree[value]
@@ -31,18 +31,19 @@
reg = tree.get(value)
if reg is None:
reg = tree[value] = OOTreeSet()
- reg.insert(token)
+ reg.insert(IKeyReference(token))
def _cleanup(self):
"clean out expired keys"
expiredkeys = []
for k in self._expirations.keys(max=utils.now()):
- for token in self._expirations[k]:
+ for key_ref in self._expirations[k]:
+ token = key_ref()
assert token.ended
for p in token.principal_ids:
self._del(self._principal_ids, token, p)
key_ref = IKeyReference(token.context)
- del self._locks[key_ref]
+ del self._locks[IKeyReference(token.context)]
expiredkeys.append(k)
for k in expiredkeys:
del self._expirations[k]
@@ -122,7 +123,8 @@
def iterForPrincipalId(self, principal_id):
locks = self._principal_ids.get(principal_id, ())
- for l in locks:
+ for key_ref in locks:
+ l = key_ref()
assert principal_id in frozenset(l.principal_ids)
if not l.ended:
yield l
More information about the checkins
mailing list