diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index 42e7784d3ca44a92277efd18c3f20cddf16a7bbb..429805b5bb222830b5b3cd0b65bebe508712b74a 100644 --- a/src/_zkapauthorizer/controller.py +++ b/src/_zkapauthorizer/controller.py @@ -30,6 +30,10 @@ from functools import ( from json import ( dumps, ) +from datetime import ( + timedelta, +) + import attr from zope.interface import ( @@ -37,6 +41,9 @@ from zope.interface import ( implementer, ) +from twisted.python.reflect import ( + namedAny, +) from twisted.logger import ( Logger, ) @@ -50,6 +57,9 @@ from twisted.internet.defer import ( inlineCallbacks, returnValue, ) +from twisted.internet.task import ( + LoopingCall, +) from twisted.web.client import ( Agent, ) @@ -76,6 +86,8 @@ from .model import ( # The number of tokens to submit with a voucher redemption. NUM_TOKENS = 100 +RETRY_INTERVAL = timedelta(minutes=3) + class AlreadySpent(Exception): """ An attempt was made to redeem a voucher which has already been redeemed. @@ -511,6 +523,10 @@ class PaymentController(object): store = attr.ib() redeemer = attr.ib() + _clock = attr.ib( + default=attr.Factory(partial(namedAny, "twisted.internet.reactor")), + ) + _error = attr.ib(default=attr.Factory(dict)) _unpaid = attr.ib(default=attr.Factory(dict)) _active = attr.ib(default=attr.Factory(dict)) @@ -522,6 +538,29 @@ class PaymentController(object): This is an initialization-time hook called by attrs. """ self._check_pending_vouchers() + # Also start a time-based polling loop to retry redemption of vouchers + # in retryable error states. + self._schedule_retries() + + def _schedule_retries(self): + # TODO: should not eagerly schedule calls. If there are no vouchers + # in an error state we shouldn't wake up at all. + # + # TODO: should schedule retries on a bounded exponential backoff + # instead, perhaps on a per-voucher basis. + self._retry_task = LoopingCall(self._retry_redemption) + self._retry_task.clock = self._clock + self._retry_task.start( + RETRY_INTERVAL.total_seconds(), + now=False, + ) + + def _retry_redemption(self): + for voucher in self._error.keys() + self._unpaid.keys(): + if voucher in self._active: + continue + if self.get_voucher(voucher).state.should_start_redemption(): + self.redeem(voucher) def _check_pending_vouchers(self): """ @@ -535,8 +574,7 @@ class PaymentController(object): "Controller found voucher ({}) at startup that needs redemption.", voucher=voucher.number, ) - random_tokens = self._get_random_tokens_for_voucher(voucher.number, NUM_TOKENS) - self._perform_redeem(voucher.number, random_tokens) + self.redeem(voucher.number) else: self._log.info( "Controller found voucher ({}) at startup that does not need redemption.", diff --git a/src/_zkapauthorizer/tests/test_controller.py b/src/_zkapauthorizer/tests/test_controller.py index b36a78c7fc35942329a46c0ecb974b44c30210b3..672c95fa682d25dbe8ed90e484a2f6596ecc7d89 100644 --- a/src/_zkapauthorizer/tests/test_controller.py +++ b/src/_zkapauthorizer/tests/test_controller.py @@ -27,6 +27,10 @@ from json import ( from functools import ( partial, ) +from datetime import ( + datetime, + timedelta, +) from zope.interface import ( implementer, ) @@ -60,6 +64,9 @@ from hypothesis.strategies import ( from twisted.python.url import ( URL, ) +from twisted.internet.task import ( + Clock, +) from twisted.internet.defer import ( fail, ) @@ -120,6 +127,7 @@ from .fixtures import ( TemporaryVoucherStore, ) +POSIX_EPOCH = datetime.utcfromtimestamp(0) class PaymentControllerTests(TestCase): """ @@ -222,6 +230,69 @@ class PaymentControllerTests(TestCase): IsInstance(model_Redeemed), ) + @given( + tahoe_configs(), + datetimes( + # I don't know that time-based parts of the system break down + # before the POSIX epoch but I don't know that they work, either. + # Don't time travel with this code. + min_value=POSIX_EPOCH, + # Once we get far enough into the future we lose the ability to + # represent a timestamp with microsecond precision in a floating + # point number, which we do with Clock. So don't go far enough + # into the future. + max_value=datetime(2200, 1, 1), + ), + vouchers(), + ) + def test_redeem_error_after_delay(self, get_config, now, voucher): + """ + When ``PaymentController`` receives a non-terminal error trying to redeem + a voucher, after some time passes it tries to redeem the voucher + again. + """ + clock = Clock() + clock.advance((now - POSIX_EPOCH).total_seconds()) + + store = self.useFixture( + TemporaryVoucherStore( + get_config, + lambda: datetime.utcfromtimestamp(clock.seconds()), + ), + ).store + controller = PaymentController( + store, + UnpaidRedeemer(), + clock, + ) + controller.redeem(voucher) + # It fails this time. + self.assertThat( + controller.get_voucher(voucher).state, + MatchesAll( + IsInstance(model_Unpaid), + MatchesStructure( + finished=Equals(now), + ), + ) + ) + + # Some time passes. + interval = timedelta(hours=1) + clock.advance(interval.total_seconds()) + + # It failed again. + self.assertThat( + controller.get_voucher(voucher).state, + MatchesAll( + IsInstance(model_Unpaid), + MatchesStructure( + # At the new time, demonstrating the retry was performed. + finished=Equals(now + interval), + ), + ), + ) + NOWHERE = URL.from_text(u"https://127.0.0.1/")