From 7ead7a397576ad8fa263e0106a3e20dc4d9d76b8 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 18 Nov 2021 10:59:04 -0500
Subject: [PATCH] test for lease maint config read from config file, and
related refactoring
---
setup.cfg | 2 +-
src/_zkapauthorizer/lease_maintenance.py | 44 ++++++-
src/_zkapauthorizer/model.py | 14 +--
src/_zkapauthorizer/tests/strategies.py | 49 +++++++-
.../tests/test_client_resource.py | 2 +-
.../tests/test_lease_maintenance.py | 38 +++---
src/_zkapauthorizer/tests/test_plugin.py | 116 ++++++++++++++----
7 files changed, 204 insertions(+), 61 deletions(-)
diff --git a/setup.cfg b/setup.cfg
index ebbd1ac..272d035 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -34,7 +34,7 @@ install_requires =
attrs
zope.interface
eliot
- aniso8601
+ isodate
python-challenge-bypass-ristretto
# The pip resolver sometimes finds treq's dependencies first and these are
# incompatible with Tahoe-LAFS'. So duplicate them here (the ones that
diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py
index 9fe3186..d4f010d 100644
--- a/src/_zkapauthorizer/lease_maintenance.py
+++ b/src/_zkapauthorizer/lease_maintenance.py
@@ -29,7 +29,7 @@ from allmydata.util.hashutil import (
file_cancel_secret_hash,
file_renewal_secret_hash,
)
-from aniso8601 import parse_datetime
+from isodate import duration_isoformat, parse_datetime, parse_duration
from twisted.application.service import Service
from twisted.internet.defer import inlineCallbacks, maybeDeferred
from twisted.python.log import err
@@ -309,12 +309,17 @@ class _FuzzyTimerService(Service):
:ivar IReactorTime reactor: A Twisted reactor to use to schedule runs of
the operation.
+
+ :ivar get_config: A function to call to return the service's
+ configuration. The configuration is represented as a service-specific
+ object.
"""
name = attr.ib()
operation = attr.ib()
initial_interval = attr.ib()
sample_interval_distribution = attr.ib()
+ get_config = attr.ib() # type: () -> Any
reactor = attr.ib()
def startService(self):
@@ -414,6 +419,12 @@ def lease_maintenance_service(
timedelta(0),
)
+ def get_config():
+ return LeaseMaintenanceConfig(
+ crawl_interval_mean=interval_mean,
+ crawl_interval_range=interval_range,
+ )
+
return _FuzzyTimerService(
SERVICE_NAME,
lambda: bracket(
@@ -426,10 +437,41 @@ def lease_maintenance_service(
),
initial_interval,
sample_interval_distribution,
+ get_config,
reactor,
)
+@attr.s(frozen=True)
+class LeaseMaintenanceConfig(object):
+ """
+ Represent the configuration for a lease maintenance service.
+ """
+
+ crawl_interval_mean = attr.ib() # type: datetime.timedelta
+ crawl_interval_range = attr.ib() # type: datetime.timedelta
+
+
+def lease_maintenance_config_to_dict(lease_maint_config):
+ # type: (LeaseMaintenanceConfig) -> Dict[str, str]
+ return {
+ "lease.crawl-interval.mean": duration_isoformat(
+ lease_maint_config.crawl_interval_mean,
+ ),
+ "lease.crawl-interval.range": duration_isoformat(
+ lease_maint_config.crawl_interval_range,
+ ),
+ }
+
+
+def lease_maintenance_config_from_dict(d):
+ # type: (Dict[str, str]) -> LeaseMaintenanceConfig
+ return LeaseMaintenanceConfig(
+ crawl_interval_mean=parse_duration(d["lease.crawl-interval.mean"]),
+ crawl_interval_range=parse_duration(d["lease.crawl-interval.range"]),
+ )
+
+
def write_time_to_path(path, when):
"""
Write an ISO8601 datetime string to a file.
diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py
index 27e2fe9..1b39895 100644
--- a/src/_zkapauthorizer/model.py
+++ b/src/_zkapauthorizer/model.py
@@ -24,7 +24,7 @@ from sqlite3 import OperationalError
from sqlite3 import connect as _connect
import attr
-from aniso8601 import parse_datetime as _parse_datetime
+from isodate import parse_datetime
from twisted.logger import Logger
from twisted.python.filepath import FilePath
from zope.interface import Interface, implementer
@@ -39,18 +39,6 @@ from .storage_common import (
from .validators import greater_than, has_length, is_base64_encoded
-def parse_datetime(s, **kw):
- """
- Like ``aniso8601.parse_datetime`` but accept unicode as well.
- """
- if isinstance(s, unicode):
- s = s.encode("utf-8")
- assert isinstance(s, bytes)
- if "delimiter" in kw and isinstance(kw["delimiter"], unicode):
- kw["delimiter"] = kw["delimiter"].encode("utf-8")
- return _parse_datetime(s, **kw)
-
-
class ILeaseMaintenanceObserver(Interface):
"""
An object which is interested in receiving events related to the progress
diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py
index 567c944..ec952ec 100644
--- a/src/_zkapauthorizer/tests/strategies.py
+++ b/src/_zkapauthorizer/tests/strategies.py
@@ -17,7 +17,7 @@ Hypothesis strategies for property testing.
"""
from base64 import b64encode, urlsafe_b64encode
-from datetime import datetime
+from datetime import datetime, timedelta
from urllib import quote
import attr
@@ -30,6 +30,7 @@ from hypothesis.strategies import (
datetimes,
dictionaries,
fixed_dictionaries,
+ floats,
integers,
just,
lists,
@@ -39,6 +40,7 @@ from hypothesis.strategies import (
sampled_from,
sets,
text,
+ timedeltas,
tuples,
)
from twisted.internet.defer import succeed
@@ -47,6 +49,7 @@ from twisted.web.test.requesthelper import DummyRequest
from zope.interface import implementer
from ..configutil import config_string_from_sections
+from ..lease_maintenance import LeaseMaintenanceConfig, lease_maintenance_config_to_dict
from ..model import (
DoubleSpend,
Error,
@@ -347,6 +350,50 @@ def client_errorredeemer_configurations(details):
)
+def interval_means():
+ """
+ Build timedeltas representing the mean time between lease maintenance
+ runs.
+ """
+ return floats(
+ # It doesn't make sense to have a negative check interval mean.
+ min_value=0,
+ # We can't make this value too large or it isn't convertable to a
+ # timedelta. Also, even values as large as this one are of
+ # questionable value.
+ max_value=60 * 60 * 24 * 365,
+ ).map(
+ # By representing the result as a timedelta we avoid the cases where
+ # the lower precision of timedelta compared to float drops the whole
+ # value (anything between 0 and 1 microsecond). This is just one
+ # example of how working with timedeltas is nicer, in general.
+ lambda s: timedelta(seconds=s),
+ )
+
+
+def lease_maintenance_configurations():
+ """
+ Build LeaseMaintenanceConfig instances.
+ """
+ return builds(
+ LeaseMaintenanceConfig,
+ interval_means(),
+ timedeltas(min_value=timedelta(0)),
+ )
+
+
+def client_lease_maintenance_configurations(maint_configs=None):
+ """
+ Build dictionaries representing the lease maintenance options that go into
+ the ZKAPAuthorizer client plugin section.
+ """
+ if maint_configs is None:
+ maint_configs = lease_maintenance_configurations()
+ return maint_configs.map(
+ lambda lease_maint_config: lease_maintenance_config_to_dict(lease_maint_config),
+ )
+
+
def direct_tahoe_configs(
zkapauthz_v1_configuration=client_dummyredeemer_configurations(),
shares=just((None, None, None)),
diff --git a/src/_zkapauthorizer/tests/test_client_resource.py b/src/_zkapauthorizer/tests/test_client_resource.py
index 954ad66..4b1663d 100644
--- a/src/_zkapauthorizer/tests/test_client_resource.py
+++ b/src/_zkapauthorizer/tests/test_client_resource.py
@@ -26,7 +26,6 @@ from urllib import quote
import attr
from allmydata.client import config_from_string
-from aniso8601 import parse_datetime
from fixtures import TempDir
from hypothesis import given, note
from hypothesis.strategies import (
@@ -44,6 +43,7 @@ from hypothesis.strategies import (
text,
tuples,
)
+from isodate import parse_datetime
from testtools import TestCase
from testtools.content import text_content
from testtools.matchers import (
diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py
index 8734528..a0a9c3a 100644
--- a/src/_zkapauthorizer/tests/test_lease_maintenance.py
+++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py
@@ -60,6 +60,8 @@ from ..foolscap import ShareStat
from ..lease_maintenance import (
MemoryMaintenanceObserver,
NoopMaintenanceObserver,
+ lease_maintenance_config_from_dict,
+ lease_maintenance_config_to_dict,
lease_maintenance_service,
maintain_leases_from_root,
renew_leases,
@@ -68,6 +70,8 @@ from ..lease_maintenance import (
from .matchers import Provides, between, leases_current
from .strategies import (
clocks,
+ interval_means,
+ lease_maintenance_configurations,
node_hierarchies,
posix_timestamps,
sharenums,
@@ -75,23 +79,6 @@ from .strategies import (
)
-def interval_means():
- return floats(
- # It doesn't make sense to have a negative check interval mean.
- min_value=0,
- # We can't make this value too large or it isn't convertable to a
- # timedelta. Also, even values as large as this one are of
- # questionable value.
- max_value=60 * 60 * 24 * 365,
- ).map(
- # By representing the result as a timedelta we avoid the cases where
- # the lower precision of timedelta compared to float drops the whole
- # value (anything between 0 and 1 microsecond). This is just one
- # example of how working with timedeltas is nicer, in general.
- lambda s: timedelta(seconds=s),
- )
-
-
def dummy_maintain_leases():
pass
@@ -217,6 +204,23 @@ def storage_brokers(draw, clocks):
)
+class LeaseMaintenanceConfigTests(TestCase):
+ """
+ Tests related to ``LeaseMaintenanceConfig``.
+ """
+
+ @given(lease_maintenance_configurations())
+ def test_config_roundtrip(self, config):
+ """
+ ``LeaseMaintenanceConfig`` round-trips through
+ ``lease_maintenance_config_to_dict`` and
+ ``lease_maintenance_config_from_dict``.
+ """
+ dumped = lease_maintenance_config_to_dict(config)
+ loaded = lease_maintenance_config_from_dict(dumped)
+ self.assertThat(config, Equals(loaded))
+
+
class LeaseMaintenanceServiceTests(TestCase):
"""
Tests for the service returned by ``lease_maintenance_service``.
diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py
index 78959a5..ac4d342 100644
--- a/src/_zkapauthorizer/tests/test_plugin.py
+++ b/src/_zkapauthorizer/tests/test_plugin.py
@@ -74,8 +74,10 @@ from .matchers import Provides, raises
from .strategies import (
announcements,
client_dummyredeemer_configurations,
+ client_lease_maintenance_configurations,
dummy_ristretto_keys,
lease_cancel_secrets,
+ lease_maintenance_configurations,
lease_renew_secrets,
minimal_tahoe_configs,
pass_counts,
@@ -537,44 +539,38 @@ class LeaseMaintenanceServiceTests(TestCase):
Tests for the plugin's initialization of the lease maintenance service.
"""
- def _created_test(self, get_config, servers_yaml, rootcap):
- original_tempdir = tempfile.tempdir
+ def _create(self, get_config, servers_yaml, rootcap):
+ """
+ Create a client node using ``create_client_from_config``.
+
+ :param get_config: A function to call to get a Tahoe-LAFS config
+ object.
+
+ :param servers_yaml: ``None`` or a string giving the contents for the
+ node's ``servers.yaml`` file.
+ :param rootcap: ``True`` to write some bytes to the node's ``rootcap``
+ file, ``False`` otherwise.
+ """
tempdir = self.useFixture(TempDir())
nodedir = tempdir.join(b"node")
privatedir = tempdir.join(b"node", b"private")
makedirs(privatedir)
config = get_config(nodedir, b"tub.port")
- # Provide it a statically configured server to connect to.
- config.write_private_config(
- b"servers.yaml",
- servers_yaml,
- )
+ if servers_yaml is not None:
+ # Provide it a statically configured server to connect to.
+ config.write_private_config(
+ b"servers.yaml",
+ servers_yaml,
+ )
if rootcap:
config.write_private_config(
b"rootcap",
b"dddddddd",
)
- try:
- d = create_client_from_config(config)
- self.assertThat(
- d,
- succeeded(
- AfterPreprocessing(
- lambda client: client.getServiceNamed(SERVICE_NAME),
- Always(),
- ),
- ),
- )
- finally:
- # create_client_from_config (indirectly) rewrites tempfile.tempdir
- # in a destructive manner that fails most of the rest of the test
- # suite if we don't clean it up. We can't do this with a tearDown
- # or a fixture or an addCleanup because hypothesis doesn't run any
- # of those at the right time. :/
- tempfile.tempdir = original_tempdir
+ return create_client_from_config(config)
@settings(
deadline=None,
@@ -589,7 +585,8 @@ class LeaseMaintenanceServiceTests(TestCase):
maintenance service after it has at least one storage server to
connect to.
"""
- return self._created_test(get_config, servers_yaml, rootcap=True)
+ d = self._create(get_config, servers_yaml, rootcap=True)
+ self.assertThat(d, succeeded(has_lease_maintenance_service()))
@settings(
deadline=None,
@@ -603,7 +600,72 @@ class LeaseMaintenanceServiceTests(TestCase):
The lease maintenance service can be created even if no rootcap has yet
been written to the client's configuration directory.
"""
- return self._created_test(get_config, servers_yaml, rootcap=False)
+ d = self._create(get_config, servers_yaml, rootcap=False)
+ self.assertThat(d, succeeded(has_lease_maintenance_service()))
+
+ @given(
+ # First build the simple lease maintenance configuration object that
+ # represents the example to test.
+ lease_maintenance_configurations().flatmap(
+ # Then generate build a function that will get us a Tahoe
+ # configuration that includes at least that lease maintenance
+ # configuration.
+ lambda lease_maint_config: tahoe_configs(
+ zkapauthz_v1_configuration=client_lease_maintenance_configurations(
+ just(lease_maint_config),
+ ),
+ ).map(
+ # Then bundle up both pieces to pass to the function. By
+ # preserving the simple lease maintenance configuration and
+ # making it available to the test, the test logic is much
+ # simplified (eg, we don't have to read values out of the
+ # Tahoe configuration to figure out what example we're working
+ # on).
+ lambda get_config: (lease_maint_config, get_config),
+ ),
+ ),
+ )
+ def test_values_from_configuration(self, config_objs):
+ """
+ If values for lease maintenance parameters are supplied in the
+ configuration file then the lease maintenance service is created with
+ those values.
+ """
+ lease_maint_config, get_config = config_objs
+ d = self._create(get_config, servers_yaml=None, rootcap=False)
+ self.assertThat(
+ d,
+ succeeded(has_lease_maintenance_configuration(lease_maint_config)),
+ )
+
+
+def has_lease_maintenance_service():
+ """
+ Return a matcher for a Tahoe-LAFS client object that has a lease
+ maintenance service.
+ """
+ # type: () -> Matcher
+ return AfterPreprocessing(
+ lambda client: client.getServiceNamed(SERVICE_NAME),
+ Always(),
+ )
+
+
+def has_lease_maintenance_configuration(lease_maint_config):
+ """
+ Return a matcher for a Tahoe-LAFS client object that has a lease
+ maintenance service with the given configuration.
+ """
+ # type: (_LeaseMaintenanceConfig) -> Matcher
+ def get_lease_maintenance_config(lease_maint_service):
+ return lease_maint_service.get_config()
+
+ return AfterPreprocessing(
+ lambda client: get_lease_maintenance_config(
+ client.getServiceNamed(SERVICE_NAME),
+ ),
+ Equals(lease_maint_config),
+ )
class LoadSigningKeyTests(TestCase):
--
GitLab