diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py index cf81a15e8c526a728fd43ebca14020e5bc9f1f12..74f495f470b746ac75dabe0a417817a47a280195 100644 --- a/src/_zkapauthorizer/lease_maintenance.py +++ b/src/_zkapauthorizer/lease_maintenance.py @@ -47,7 +47,7 @@ from allmydata.util.hashutil import ( ) @inlineCallbacks -def visit_filesystem_nodes(root_node, visit): +def visit_storage_indexes(root_node, visit): """ Call a visitor with the storage index of ``root_node`` and that of all nodes reachable from it. @@ -65,7 +65,11 @@ def visit_filesystem_nodes(root_node, visit): elem = stack.pop() visit(elem.get_storage_index()) if IDirectoryNode.providedBy(elem): - for (child_node, child_metadata) in (yield elem.list()): + children = yield elem.list() + # Produce consistent results by forcing some consistent ordering + # here. This will sort by name. + stable_children = sorted(children.items()) + for (name, (child_node, child_metadata)) in stable_children: stack.append(child_node) @@ -325,6 +329,25 @@ def lease_maintenance_service( ) +def visit_storage_indexes_from_root(visitor, root_node): + """ + An operation for ``lease_maintenance_service`` which applies the given + visitor to ``root_node`` and all its children. + + :param visitor: A one-argument callable which takes the traversal function + and which should call it as desired. + + :param IFilesystemNode root_node: The filesystem node at which traversal + will begin. + + :return: A no-argument callable to perform the visits. + """ + return partial( + visitor, + partial(visit_storage_indexes, root_node), + ) + + def maintain_leases_from_root(root_node, storage_broker, secret_holder, min_lease_remaining, get_now): """ An operation for ``lease_maintenance_service`` which visits ``root_node`` @@ -347,14 +370,21 @@ def maintain_leases_from_root(root_node, storage_broker, secret_holder, min_leas :param get_now: A no-argument callable that returns the current time as a ``datetime`` instance. + + :return: A no-argument callable to perform the maintenance. """ - return partial( - renew_leases, - partial(visit_filesystem_nodes, root_node), - storage_broker, - secret_holder, - min_lease_remaining, - get_now, + def visitor(visit_assets): + return renew_leases( + visit_assets, + storage_broker, + secret_holder, + min_lease_remaining, + get_now, + ) + + return visit_storage_indexes_from_root( + visitor, + root_node, ) diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index 4011e8c96d6d67711bab8d88fe00b62738c920a9..b4d5daaca56d9a28e0c18bea6ff994580752c9ba 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -25,6 +25,10 @@ from datetime import ( import attr +from zope.interface import ( + implementer, +) + from hypothesis.strategies import ( one_of, just, @@ -40,8 +44,12 @@ from hypothesis.strategies import ( fixed_dictionaries, builds, datetimes, + recursive, ) +from twisted.internet.defer import ( + succeed, +) from twisted.internet.task import ( Clock, ) @@ -50,6 +58,8 @@ from twisted.web.test.requesthelper import ( ) from allmydata.interfaces import ( + IFilesystemNode, + IDirectoryNode, HASH_SIZE, ) @@ -588,13 +598,17 @@ def clocks(now=posix_safe_datetimes()): @implementer(IFilesystemNode) -@attr.ib +@attr.s class _LeafNode(object): _storage_index = attr.ib() def get_storage_index(self): return self._storage_index + # For testing + def flatten(self): + return [self] + def leaf_nodes(): return storage_indexes().map(_LeafNode) @@ -609,6 +623,16 @@ class _DirectoryNode(object): def list(self): return succeed(self._children) + def get_storage_index(self): + return self._storage_index + + # For testing + def flatten(self): + result = [self] + for (node, _) in self._children.values(): + result.extend(node.flatten()) + return result + def directory_nodes(child_strategy): """ diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py index 23f736b6129739e7170e5df1434c71cad90cbe6e..4db256baa94c5747e705856abd1f4fb3cdfe4cbd 100644 --- a/src/_zkapauthorizer/tests/test_lease_maintenance.py +++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py @@ -33,9 +33,13 @@ from testtools import ( ) from testtools.matchers import ( Equals, + Always, + HasLength, + MatchesAll, + AfterPreprocessing, ) from testtools.twistedsupport import ( - succeeds, + succeeded, ) from hypothesis import ( given, @@ -64,9 +68,9 @@ from twisted.application.service import ( from allmydata.util.hashutil import ( CRYPTO_VAL_SIZE, ) -from allmydata.client import ( - SecretHolder, -) +# from allmydata.client import ( +# SecretHolder, +# ) from ..foolscap import ( ShareStat, @@ -84,7 +88,8 @@ from .strategies import ( from ..lease_maintenance import ( lease_maintenance_service, - maintain_leases_from_root, + # maintain_leases_from_root, + visit_storage_indexes_from_root, ) @@ -312,9 +317,9 @@ class LeaseMaintenanceServiceTests(TestCase): ) -class MaintainLeasesFromRootTests(TestCase): +class VisitStorageIndexesFromRootTests(TestCase): """ - Tests for ``maintain_leases_from_root``. + Tests for ``visit_storage_indexes_from_root``. """ @given(node_hierarchies(), clocks()) def test_visits_all_nodes(self, root_node, clock): @@ -323,22 +328,30 @@ class MaintainLeasesFromRootTests(TestCase): its deepest children. """ visited = [] - def visitor(node): - visited.append(node) - - storage_broker = DummyStorageBroker(clock, []) - secret_holder = SecretHolder(lease_secret, convergence_secret) + def perform_visit(visit_assets): + return visit_assets(visited.append) - operation = maintain_leases_from_root( - visitor, + operation = visit_storage_indexes_from_root( + perform_visit, root_node, - storage_broker, - secret_holder, - timedelta(days=3), - lambda: datetime.utcfromtimestamp(clock.seconds()), ) self.assertThat( - operation(root_node), - succeeds(Always()), + operation(), + succeeded(Always()), + ) + expected = root_node.flatten() + self.assertThat( + visited, + MatchesAll( + HasLength(len(expected)), + AfterPreprocessing( + set, + Equals(set( + node.get_storage_index() + for node + in expected + )), + ), + ), )