Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Commit

Permalink
Make finish_email_verification concurrency-safe
Browse files Browse the repository at this point in the history
  • Loading branch information
chadwhitacre committed Apr 24, 2017
1 parent eaa8699 commit 1d42094
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
9 changes: 6 additions & 3 deletions gratipay/models/package/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,13 @@ def ensure_team(self, cursor, owner):
, owner=owner
, _cursor=cursor
)
cursor.run('UPDATE packages SET team_id=%s WHERE id=%s', (team.id, self.id))
self.set_attributes(team_id=team.id)
cursor.run('INSERT INTO teams_to_packages (team_id, package_id) '
'VALUES (%s, %s)', (team.id, self.id))
return team


def _load_team(self, cursor):
return cursor.one('SELECT t.*::teams FROM teams t WHERE t.id=%s', (self.team_id,))
return cursor.one( 'SELECT t.*::teams FROM teams t WHERE t.id='
'(SELECT team_id FROM teams_to_packages tp WHERE tp.package_id=%s)'
, (self.id,)
)
20 changes: 12 additions & 8 deletions gratipay/models/participant/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def finish_email_verification(self, email, nonce):
if '' in (email.strip(), nonce.strip()):
return VERIFICATION_MISSING
with self.db.get_cursor() as cursor:
record = self.get_email(email, cursor)
record = self.get_email(email, cursor, and_lock=True)
if record is None:
return VERIFICATION_FAILED
packages = self.get_packages_claiming(cursor, nonce)
Expand Down Expand Up @@ -294,15 +294,19 @@ def finish_package_claims(self, cursor, nonce, *packages):
)


def get_email(self, address, cursor=None):
def get_email(self, address, cursor=None, and_lock=False):
"""Return a record for a single email address on file for this participant.
:param unicode address: the email address for which to get a record
:param Cursor cursor: a database cursor; if ``None``, we'll use ``self.db``
:param and_lock: if True, we will acquire a write-lock on the email record before returning
:returns: a database record (a named tuple)
"""
return (cursor or self.db).one("""
SELECT *
FROM emails
WHERE participant_id=%s
AND address=%s
""", (self.id, address))
sql = 'SELECT * FROM emails WHERE participant_id=%s AND address=%s'
if and_lock:
sql += ' FOR UPDATE'
return (cursor or self.db).one(sql, (self.id, address))


def get_emails(self):
Expand Down
5 changes: 4 additions & 1 deletion gratipay/models/team/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ def package(self):


def _load_package(self, cursor):
return cursor.one('SELECT p.*::packages FROM packages p WHERE p.team_id=%s', (self.id,))
return cursor.one( 'SELECT p.*::packages FROM packages p WHERE p.id='
'(SELECT package_id FROM teams_to_packages tp WHERE tp.team_id=%s)'
, (self.id,)
)
5 changes: 4 additions & 1 deletion sql/branch.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ BEGIN;
, UNIQUE(nonce, package_id)
);

ALTER TABLE packages ADD COLUMN team_id bigint UNIQUE REFERENCES teams(id) ON DELETE RESTRICT;
CREATE TABLE teams_to_packages
( team_id bigint UNIQUE REFERENCES teams(id) ON DELETE RESTRICT
, package_id bigint UNIQUE REFERENCES packages(id) ON DELETE RESTRICT
);

END;
11 changes: 10 additions & 1 deletion tests/py/test_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ def test_team_can_only_be_linked_from_one_package(self):
bar = self.make_package(name='bar')
raises( IntegrityError
, self.db.run
, 'UPDATE packages SET team_id=%s WHERE id=%s'
, 'INSERT INTO teams_to_packages (team_id, package_id) VALUES (%s, %s)'
, (team.id, bar.id)
)

def test_package_can_only_be_linked_from_one_team(self):
alice, package, team = self.test_can_link_to_a_new_team()
bar = self.make_team(name='Bar')
raises( IntegrityError
, self.db.run
, 'INSERT INTO teams_to_packages (team_id, package_id) VALUES (%s, %s)'
, (bar.id, package.id)
)

0 comments on commit 1d42094

Please sign in to comment.