Skip to content
Snippets Groups Projects

Bezpečné transakce při zakládání uživatelů/účastníků/účastí

Merged Martin Mareš requested to merge mj/transakce into devel
1 file
+ 103
47
Compare changes
  • Side-by-side
  • Inline
  • Closes #314:
    
    Pokud organizátor odešle POST na přidání nového účastníka soutěže
    dvakrát rychle po sobě (třeba kvůli nějakému automatickému retry
    po rozpadu spojení), dvě různé DB transakce se snaží založit uživatele
    se stejným loginem. Jedna z nich selže na unikátnosti sloupce email.
    
    V defaultní úrovni izolace transakcí (READ COMMITTED) to nemá žádné
    hezké řešení. Nepomůže SELECT ... FOR UPDATE, jelikož ten zamyká pouze
    nalezené řádky, nikoliv neexistenci dalších řádků vyhovujících podmínce.
    
    Co by se dalo dělat:
    
    (1) Zvýšit úroveň izolace aspoň na READ REPEATABLE. To vyřeší problém,
        ale současně může začít víceméně jakákoliv zapisující transakce
        failovat. Vyžadovalo by dopsat retry do prakticky všech míst v OSMO,
        kde je nějaký commit.
    
    (2) Retryovat specificky transakce na zakládání užívatelů (a účastí apod.).
        Tohle nejde snadno, jelikoz jsou i součástí dlouho běžících transakcí
        v importech (zatím jsme se snažíli, aby byl celý import atomický
        a v případě selhání se celý rollbackoval). To by možná mohly vyřešit
        subtransakce.
    
    (3) Zamykat celou tabulku s uživateli, než na ní provedeme první SELECT.
        To by asi vyřešilo problém, ale byl by potřeba zápisový zámek, takže
        by paralelně nemohla běžet žádná čtení. A také by se to potenciálně
        mohlo deadlockovat (potřebujeme v jedné transakci postupně lock na
        uživatele, účastníky a účasti a locky platí až do konce transakce).
    
    (4) Používat INSERT ... ON CONFLICT <něco>. To vypadá bezpečně, jen to není
        moc pohodlné, zejména proto, že s tím nepočítá ORM, takže je potřeba
        dělat všechno ručně.
    
    Zatím jsem zvolil (4), protože mi přijde, že to změny udržuje lokální
    a funguje i s dlouhými transakcemi při importu. Výhledově bych se ale
    chtěl zamyslet nad tím, jak takové věci řešit co nejuniverzálněji
    a nejpohodlněji.
+ 103
47
@@ -7,6 +7,7 @@ import email.errors
@@ -7,6 +7,7 @@ import email.errors
import email.headerregistry
import email.headerregistry
import re
import re
import secrets
import secrets
 
from sqlalchemy.dialects.postgresql import insert as pgsql_insert
from typing import Optional, Tuple
from typing import Optional, Tuple
import mo
import mo
@@ -96,40 +97,57 @@ def change_user_to_org(user, reason: str):
@@ -96,40 +97,57 @@ def change_user_to_org(user, reason: str):
def find_or_create_user(email: str, krestni: Optional[str], prijmeni: Optional[str], is_org: bool, reason: str, allow_change_user_to_org=False) -> Tuple[db.User, bool, bool]:
def find_or_create_user(email: str, krestni: Optional[str], prijmeni: Optional[str], is_org: bool, reason: str, allow_change_user_to_org=False) -> Tuple[db.User, bool, bool]:
sess = db.get_session()
sess = db.get_session()
user = sess.query(db.User).with_for_update().filter_by(email=email).one_or_none()
user = sess.query(db.User).filter_by(email=email).one_or_none()
is_new = user is None
is_new = False
is_change_user_to_org = False
is_change_user_to_org = False
 
if user is None: # HACK: Podmínku je nutné zapsat znovu místo užití is_new, jinak si s tím mypy neporadí
if user is None: # HACK: Podmínku je nutné zapsat znovu místo užití is_new, jinak si s tím mypy neporadí
if not krestni or not prijmeni:
if not krestni or not prijmeni:
raise mo.CheckError('Osoba s daným e-mailem zatím neexistuje, je nutné uvést její jméno.')
raise mo.CheckError('Osoba s daným e-mailem zatím neexistuje, je nutné uvést její jméno.')
user = db.User(email=email, first_name=krestni, last_name=prijmeni, is_org=is_org)
sess.add(user)
res = sess.connection().execute(
sess.flush() # Aby uživatel dostal user_id
pgsql_insert(db.User.__table__)
logger.info(f'{reason.title()}: Založen uživatel user=#{user.user_id} email=<{user.email}>')
.values(
mo.util.log(
email=email,
type=db.LogType.user,
first_name=krestni,
what=user.user_id,
last_name=prijmeni,
details={'action': 'create-user', 'reason': reason, 'new': db.row2dict(user)},
is_org=is_org,
 
)
 
.on_conflict_do_nothing()
 
.returning(db.User.user_id)
)
)
else:
if (krestni and user.first_name != krestni) or (prijmeni and user.last_name != prijmeni):
user = sess.query(db.User).filter_by(email=email).one()
raise mo.CheckError(f'Osoba již registrována s odlišným jménem {user.full_name()}')
if (user.is_admin or user.is_org) != is_org:
if res.fetchall():
if is_org:
is_new = True
if allow_change_user_to_org:
logger.info(f'{reason.title()}: Založen uživatel user=#{user.user_id} email=<{user.email}>')
change_user_to_org(user, reason)
mo.util.log(
is_change_user_to_org = True
type=db.LogType.user,
else:
what=user.user_id,
raise CheckErrorOrgIsUser('Nelze předefinovat účastníka na organizátora.')
details={'action': 'create-user', 'reason': reason, 'new': db.row2dict(user)},
 
)
 
 
if (krestni and user.first_name != krestni) or (prijmeni and user.last_name != prijmeni):
 
raise mo.CheckError(f'Osoba již registrována s odlišným jménem {user.full_name()}')
 
if (user.is_admin or user.is_org) != is_org:
 
if is_org:
 
if allow_change_user_to_org:
 
change_user_to_org(user, reason)
 
is_change_user_to_org = True
else:
else:
raise mo.CheckError('Nelze předefinovat organizátora na účastníka.')
raise CheckErrorOrgIsUser('Nelze předefinovat účastníka na organizátora.')
 
else:
 
raise mo.CheckError('Nelze předefinovat organizátora na účastníka.')
 
return user, is_new, is_change_user_to_org
return user, is_new, is_change_user_to_org
def find_or_create_participant(user: db.User, year: int, school_id: Optional[int], birth_year: Optional[int], grade: Optional[str], reason: str) -> Tuple[db.Participant, bool]:
def find_or_create_participant(user: db.User, year: int, school_id: Optional[int], birth_year: Optional[int], grade: Optional[str], reason: str) -> Tuple[db.Participant, bool]:
sess = db.get_session()
sess = db.get_session()
part = sess.query(db.Participant).get((user.user_id, year))
part = sess.query(db.Participant).get((user.user_id, year))
is_new = part is None
is_new = False
 
if part is None:
if part is None:
prev_part = sess.query(db.Participant).filter_by(user_id=user.user_id).order_by(db.Participant.year.desc()).limit(1).one_or_none()
prev_part = sess.query(db.Participant).filter_by(user_id=user.user_id).order_by(db.Participant.year.desc()).limit(1).one_or_none()
if not school_id:
if not school_id:
@@ -144,19 +162,37 @@ def find_or_create_participant(user: db.User, year: int, school_id: Optional[int
@@ -144,19 +162,37 @@ def find_or_create_participant(user: db.User, year: int, school_id: Optional[int
raise mo.CheckError('Osoba s daným e-mailem zatím není zaregistrovaná do ročníku, je nutné uvést rok narození.')
raise mo.CheckError('Osoba s daným e-mailem zatím není zaregistrovaná do ročníku, je nutné uvést rok narození.')
if not grade:
if not grade:
raise mo.CheckError('Osoba s daným e-mailem zatím není zaregistrovaná do ročníku, je nutné uvést ročník.')
raise mo.CheckError('Osoba s daným e-mailem zatím není zaregistrovaná do ročníku, je nutné uvést ročník.')
part = db.Participant(user=user, year=year, school=school_id, birth_year=birth_year, grade=grade)
sess.add(part)
res = sess.connection().execute(
logger.info(f'{reason.title()}: Založen účastník #{user.user_id}')
pgsql_insert(db.Participant.__table__)
mo.util.log(
.values(
type=db.LogType.participant,
user_id=user.user_id,
what=user.user_id,
year=year,
details={'action': 'create-participant', 'reason': reason, 'new': db.row2dict(part)},
school=school_id,
 
birth_year=birth_year,
 
grade=grade,
 
)
 
.on_conflict_do_nothing()
 
.returning(db.Participant.user_id)
)
)
else:
if ((school_id and part.school != school_id)
part = sess.query(db.Participant).get((user.user_id, year))
or (grade and part.grade != grade)
assert part is not None
or (birth_year and part.birth_year != birth_year)):
raise mo.CheckError('Účastník již zaregistrován s odlišnou školou/ročníkem/rokem narození')
if res.fetchall():
 
is_new = True
 
logger.info(f'{reason.title()}: Založen účastník #{user.user_id}')
 
mo.util.log(
 
type=db.LogType.participant,
 
what=user.user_id,
 
details={'action': 'create-participant', 'reason': reason, 'new': db.row2dict(part)},
 
)
 
 
if ((school_id and part.school != school_id)
 
or (grade and part.grade != grade)
 
or (birth_year and part.birth_year != birth_year)):
 
raise mo.CheckError('Účastník již zaregistrován s odlišnou školou/ročníkem/rokem narození')
 
return part, is_new
return part, is_new
@@ -165,27 +201,47 @@ def find_or_create_participation(user: db.User, contest: db.Contest, place: Opti
@@ -165,27 +201,47 @@ def find_or_create_participation(user: db.User, contest: db.Contest, place: Opti
place = contest.place
place = contest.place
sess = db.get_session()
sess = db.get_session()
pions = (sess.query(db.Participation)
pion = None
.filter_by(user=user)
is_new = False
.filter(db.Participation.contest.has(db.Contest.round == contest.round))
retry = False
.all())
 
while pion is None:
 
pions = (sess.query(db.Participation)
 
.filter_by(user=user)
 
.filter(db.Participation.contest.has(db.Contest.round == contest.round))
 
.all())
 
 
if len(pions) == 0:
 
assert not retry
 
retry = True
 
res = sess.connection().execute(
 
pgsql_insert(db.Participation.__table__)
 
.values(
 
user_id=user.user_id,
 
contest_id=contest.contest_id,
 
place_id=place.place_id,
 
state=db.PartState.active,
 
)
 
.on_conflict_do_nothing()
 
.returning(db.Participation.user_id)
 
)
 
if res.fetchall():
 
is_new = True
 
elif len(pions) == 1:
 
pion = pions[0]
 
else:
 
raise mo.CheckError('Již se tohoto kola účastní ve více oblastech, což by nemělo být možné')
 
 
if pion.place != place:
 
raise mo.CheckError(f'Již se tohoto kola účastní v {contest.round.get_level().name_locative("jiném", "jiné", "jiném")} ({pion.place.get_code()})')
is_new = pions == []
if is_new:
if is_new:
pion = db.Participation(user=user, contest=contest, place_id=place.place_id, state=db.PartState.active)
sess.add(pion)
logger.info(f'{reason.title()}: Založena účast user=#{user.user_id} contest=#{contest.contest_id} place=#{place.place_id}')
logger.info(f'{reason.title()}: Založena účast user=#{user.user_id} contest=#{contest.contest_id} place=#{place.place_id}')
mo.util.log(
mo.util.log(
type=db.LogType.participant,
type=db.LogType.participant,
what=user.user_id,
what=user.user_id,
details={'action': 'add-to-contest', 'reason': reason, 'new': db.row2dict(pion)},
details={'action': 'add-to-contest', 'reason': reason, 'new': db.row2dict(pion)},
)
)
elif len(pions) == 1:
pion = pions[0]
if pion.place != place:
raise mo.CheckError(f'Již se tohoto kola účastní v {contest.round.get_level().name_locative("jiném", "jiné", "jiném")} ({pion.place.get_code()})')
else:
raise mo.CheckError('Již se tohoto kola účastní ve více oblastech, což by nemělo být možné')
return pion, is_new
return pion, is_new
Loading