diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index a0fa9fc2b23a11164acb1e2ca9b7f9527d76caaf..b16473f367d5be09920f3f399c33ecdba1fc95d7 100644 --- a/src/_zkapauthorizer/controller.py +++ b/src/_zkapauthorizer/controller.py @@ -651,18 +651,11 @@ class PaymentController(object): """ Generate or load random tokens for a redemption attempt of a voucher. """ - self._log.info("Generating random tokens for a voucher ({voucher}).", voucher=voucher) - tokens = self.redeemer.random_tokens_for_voucher(Voucher(voucher), num_tokens) - - self._log.info("Persisting random tokens for a voucher ({voucher}).", voucher=voucher) - self.store.add(voucher, tokens) - - # XXX If the voucher is already in the store then the tokens passed to - # `add` are dropped and we need to get the tokens already persisted in - # the store. add should probably return them so we can use them. Or - # maybe we should pass the generator in so it can be called only if - # necessary. - return tokens + def get_tokens(): + self._log.info("Generating random tokens for a voucher ({voucher}).", voucher=voucher) + return self.redeemer.random_tokens_for_voucher(Voucher(voucher), num_tokens) + + return self.store.add(voucher, get_tokens) def redeem(self, voucher, num_tokens=None): """ diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py index 18ce4f043a42d77161cdf2dd67b481af88636044..a208fbc9cfc6e1db9a7a1b7bd8594e4da264d5fa 100644 --- a/src/_zkapauthorizer/model.py +++ b/src/_zkapauthorizer/model.py @@ -45,6 +45,9 @@ import attr from aniso8601 import ( parse_datetime, ) +from twisted.logger import ( + Logger, +) from twisted.python.filepath import ( FilePath, ) @@ -241,6 +244,8 @@ class VoucherStore(object): :ivar now: A no-argument callable that returns the time of the call as a ``datetime`` instance. """ + _log = Logger() + database_path = attr.ib(validator=attr.validators.instance_of(FilePath)) now = attr.ib() @@ -296,7 +301,7 @@ class VoucherStore(object): return Voucher.from_row(refs[0]) @with_cursor - def add(self, cursor, voucher, tokens): + def add(self, cursor, voucher, get_tokens): """ Add a new voucher and associated random tokens to the database. If a voucher with the given text value is already present, do nothing. @@ -309,17 +314,40 @@ class VoucherStore(object): if not isinstance(now, datetime): raise TypeError("{} returned {}, expected datetime".format(self.now, now)) + cursor.execute("BEGIN IMMEDIATE TRANSACTION") cursor.execute( """ - INSERT OR IGNORE INTO [vouchers] ([number], [created]) VALUES (?, ?) + SELECT ([text]) + FROM [tokens] + WHERE [voucher] = ? """, - (voucher, self.now()) + (voucher,), ) - if cursor.rowcount: - # Something was inserted. Insert the tokens, too. It's okay to - # drop the tokens in the other case. They've never been used. - # What's *already* in the database, on the other hand, may already - # have been submitted in a redeem attempt and must not change. + rows = cursor.fetchall() + if len(rows) > 0: + self._log.info( + "Loaded {count} random tokens for a voucher ({voucher}).", + count=len(rows), + voucher=voucher, + ) + tokens = list( + RandomToken(token_value) + for (token_value,) + in rows + ) + else: + tokens = get_tokens() + self._log.info( + "Persisting {count} random tokens for a voucher ({voucher}).", + count=len(tokens), + voucher=voucher, + ) + cursor.execute( + """ + INSERT OR IGNORE INTO [vouchers] ([number], [created]) VALUES (?, ?) + """, + (voucher, self.now()) + ) cursor.executemany( """ INSERT INTO [tokens] ([voucher], [text]) VALUES (?, ?) @@ -330,6 +358,8 @@ class VoucherStore(object): in tokens ), ) + cursor.connection.commit() + return tokens @with_cursor def list(self, cursor): diff --git a/src/_zkapauthorizer/tests/test_model.py b/src/_zkapauthorizer/tests/test_model.py index 348c0cf0f2258389d9516eb0fa1da1b5e9bb86be..00dc6eca2395628b38bc56c1d1a7ef7a20ddb321 100644 --- a/src/_zkapauthorizer/tests/test_model.py +++ b/src/_zkapauthorizer/tests/test_model.py @@ -141,7 +141,7 @@ class VoucherStoreTests(TestCase): previously added to the store with ``VoucherStore.add``. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher, tokens) + store.add(voucher, lambda: tokens) self.assertThat( store.get(voucher), MatchesStructure( @@ -158,8 +158,8 @@ class VoucherStoreTests(TestCase): in the same state as a single call. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher, tokens) - store.add(voucher, []) + first_tokens = store.add(voucher, lambda: tokens) + second_tokens = store.add(voucher, lambda: []) self.assertThat( store.get(voucher), MatchesStructure( @@ -168,7 +168,14 @@ class VoucherStoreTests(TestCase): state=Equals(Pending()), ), ) - + self.assertThat( + first_tokens, + Equals(tokens), + ) + self.assertThat( + second_tokens, + Equals(tokens), + ) @given(tahoe_configs(), datetimes(), lists(vouchers(), unique=True)) def test_list(self, get_config, now, vouchers): @@ -178,7 +185,7 @@ class VoucherStoreTests(TestCase): """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store for voucher in vouchers: - store.add(voucher, []) + store.add(voucher, lambda: []) self.assertThat( store.list(), @@ -389,7 +396,7 @@ class UnblindedTokenStoreTests(TestCase): ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, random) + store.add(voucher_value, lambda: random) store.insert_unblinded_tokens_for_voucher(voucher_value, unblinded) loaded_voucher = store.get(voucher_value) self.assertThat( @@ -414,7 +421,7 @@ class UnblindedTokenStoreTests(TestCase): such. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, random_tokens) + store.add(voucher_value, lambda: random_tokens) store.mark_voucher_double_spent(voucher_value) voucher = store.get(voucher_value) self.assertThat( @@ -454,7 +461,7 @@ class UnblindedTokenStoreTests(TestCase): ), ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, random) + store.add(voucher_value, lambda: random) store.insert_unblinded_tokens_for_voucher(voucher_value, unblinded) try: result = store.mark_voucher_double_spent(voucher_value) diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py index 41c2dd43f4b3bd240ec9bbf852fe19e93c328de0..a8fff0d78b28dd5a6adcde78467d2b9bdb1178a8 100644 --- a/src/_zkapauthorizer/tests/test_plugin.py +++ b/src/_zkapauthorizer/tests/test_plugin.py @@ -419,7 +419,7 @@ class ClientPluginTests(TestCase): ) store = VoucherStore.from_node_config(node_config, lambda: now) - store.add(voucher, [token]) + store.add(voucher, lambda: [token]) store.insert_unblinded_tokens_for_voucher(voucher, [unblinded_token]) storage_client = storage_server.get_storage_client(