From 05b4220e61c6fc4855042c599a7296b87981c647 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone <exarkun@twistedmatrix.com> Date: Mon, 25 May 2020 12:01:09 -0400 Subject: [PATCH] some database interaction improvements removing the operation-id may not actually be an improvement ... it is a simplification, though. However it requires us to load all the rows into python and then send them back to the database instead of only loading them. I thought removing the operation-id from the in-use table would be a win and maybe it is but not for the reason I thought - sqlite3 emits the same code whether you do `in [foo]` or `in (select x from [foo])` as far as I can tell. Still, I like the simplification so we'll go with it for now. --- src/_zkapauthorizer/model.py | 45 ++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py index 3b23a91..66dcb12 100644 --- a/src/_zkapauthorizer/model.py +++ b/src/_zkapauthorizer/model.py @@ -17,9 +17,6 @@ This module implements models (in the MVC sense) for the client side of the storage plugin. """ -from uuid import ( - uuid4, -) from functools import ( wraps, ) @@ -154,18 +151,23 @@ def open_and_initialize(path, connect=None): # upgrade logic because we re-create them on every connection. conn.execute( """ + -- Track tokens in use by the process holding this connection. CREATE TEMPORARY TABLE [in-use] ( [unblinded-token] text, -- The base64 encoded unblinded token. - [operation-id] text, -- A unique identifier for a group of tokens in-use together. PRIMARY KEY([unblinded-token]) -- A foreign key on unblinded-token to [unblinded-tokens]([token]) -- would be alright - however SQLite3 foreign key constraints -- can't cross databases (and temporary tables are considered to - -- be in a different database than normal tables). ) """, + -- be in a different database than normal tables). + ) + """, ) conn.execute( """ + -- Track tokens that we want to remove from the database. Mainly just + -- works around the awkward DB-API interface for dealing with deleting + -- many rows. CREATE TEMPORARY TABLE [to-discard] ( [unblinded-token] text ) @@ -173,18 +175,13 @@ def open_and_initialize(path, connect=None): ) conn.execute( """ + -- Track tokens that we want to remove from the [in-use] set. Similar + -- to [to-discard]. CREATE TEMPORARY TABLE [to-reset] ( [unblinded-token] text ) """, ) - conn.execute( - """ - CREATE TEMPORARY TABLE [extracting] ( - [token] text - ) - """, - ) return conn @@ -515,27 +512,25 @@ class VoucherStore(object): # provoke undesirable behavior from the database. raise NotEnoughTokens() - operation_id = unicode(uuid4()) cursor.execute( """ - INSERT INTO [in-use] - SELECT [token], ? + SELECT [token] FROM [unblinded-tokens] - WHERE [token] NOT IN (SELECT [unblinded-token] FROM [in-use]) + WHERE [token] NOT IN [in-use] LIMIT ? """, - (operation_id, count), + (count,), ) - if cursor.rowcount < count: + texts = cursor.fetchall() + if len(texts) < count: raise NotEnoughTokens() - cursor.execute( + cursor.executemany( """ - SELECT [unblinded-token] FROM [in-use] WHERE [operation-id] = ? + INSERT INTO [in-use] VALUES (?) """, - (operation_id,), + texts, ) - texts = cursor.fetchall() return list( UnblindedToken(t) for (t,) @@ -562,13 +557,13 @@ class VoucherStore(object): cursor.execute( """ DELETE FROM [in-use] - WHERE [unblinded-token] IN (SELECT [unblinded-token] FROM [to-discard]) + WHERE [unblinded-token] IN [to-discard] """, ) cursor.execute( """ DELETE FROM [unblinded-tokens] - WHERE [token] IN (SELECT [unblinded-token] FROM [to-discard]) + WHERE [token] IN [to-discard] """, ) cursor.execute( @@ -629,7 +624,7 @@ class VoucherStore(object): cursor.execute( """ DELETE FROM [in-use] - WHERE [unblinded-token] IN (SELECT [unblinded-token] FROM [to-reset]) + WHERE [unblinded-token] IN [to-reset] """, ) cursor.execute( -- GitLab