Periodically retry failed voucher redemption

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.",

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/")