fix(arch): l inscription est une operation, donc une transaction
ARCH-006, avec une requalification du constat.
**Le chiffre de l audit surestimait le probleme.** « 58 commit() en routes
pour un seul rollback() » lisait un ratio comme un defaut. Mesure plutot
que suppose :
- Flask-SQLAlchemy demonte la session a la fin de chaque requete, ce qui
annule tout ce qui n a pas ete commite ;
- l unique rollback est dans le gestionnaire 500, c est-a-dire au bon
endroit ;
- depuis la vague D, aucun module de routes ne contient `except
Exception` : les 34 releves sont dans discord_bot.py, les scripts et le
service de notification, ou avaler l erreur est le comportement voulu
et documente.
Ce que le decompte ne pouvait pas voir, c est le vrai defaut : une fonction
qui commite DEUX fois, ou un echec apres le premier commit laisse une
demi-operation persistee. Il y en avait deux dans tout le depot -- une
analyse AST le confirme. edit_user etait la grave, corrigee avec ARCH-008.
register est la seconde : le compte etait commite, puis les gamertags dans
une seconde transaction. Un echec entre les deux laissait un compte dont
les jeux declares etaient absents, l inscription etant annoncee reussie.
Le premier commit devient un flush -- l identifiant est necessaire pour les
lignes suivantes, pas la durabilite.
Les deux commit() de login ne sont pas concernes : ils sont dans des
branches mutuellement exclusives, succes et echec.
tests/test_transactions.py enonce la garantie plutot que de la supposer :
un echec en cours de requete ne laisse aucune ligne, et l inscription est
tout ou rien. Le second echoue sur le code d avant.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
"""What survives a failure mid-request — ARCH-006.
|
||||
|
||||
The audit counted 58 `db.session.commit()` in the routes against a single
|
||||
`db.session.rollback()` in the whole repository, and read that ratio as a
|
||||
problem. Measured rather than assumed, it mostly is not one:
|
||||
|
||||
- Flask-SQLAlchemy tears the session down at the end of every request,
|
||||
which rolls back anything uncommitted;
|
||||
- the one rollback is in the 500 handler, which is where it belongs;
|
||||
- after the wave-D work, no route module contains `except Exception` at
|
||||
all, so nothing swallows a failure and carries on.
|
||||
|
||||
What the count could not see is the real defect: a route that commits
|
||||
*twice*, so a failure after the first one leaves half an operation
|
||||
persisted. There were two such functions. `edit_user` was the serious one
|
||||
and was fixed with ARCH-008; `register` is fixed here.
|
||||
|
||||
These tests state the guarantee, so that removing the rollback or
|
||||
reintroducing a mid-operation commit fails loudly.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from app.models import User, UserGamertag
|
||||
|
||||
|
||||
class TestNothingPartialSurvives:
|
||||
def test_a_failure_after_add_leaves_no_row(self, app, client, as_role, monkeypatch):
|
||||
"""The session is torn down per request; an uncommitted add is gone."""
|
||||
as_role('admin')
|
||||
|
||||
from app.routes.users import accounts
|
||||
|
||||
def _explode(*args, **kwargs):
|
||||
raise RuntimeError('storage unavailable')
|
||||
|
||||
monkeypatch.setattr(accounts.db.session, 'commit', _explode)
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
client.post(
|
||||
'/users/create',
|
||||
data={
|
||||
'username': 'ghost',
|
||||
'email': '[email protected]',
|
||||
'password': 'Password123',
|
||||
'full_name': 'Ghost Account',
|
||||
'role': 'coach',
|
||||
},
|
||||
)
|
||||
|
||||
with app.app_context():
|
||||
assert User.query.filter_by(username='ghost').first() is None
|
||||
|
||||
def test_the_error_handler_still_rolls_back(self, app):
|
||||
"""The single rollback in the repository is the one in the 500
|
||||
handler. Removing it should break something visible."""
|
||||
import inspect
|
||||
|
||||
from app import app as app_module
|
||||
|
||||
source = inspect.getsource(app_module.create_app)
|
||||
assert 'db.session.rollback()' in source
|
||||
|
||||
|
||||
class TestRegistrationIsOneOperation:
|
||||
"""register() committed the account, then committed the gamertags. A
|
||||
failure in between left an account whose declared games were absent,
|
||||
with the sign-up reported as successful."""
|
||||
|
||||
FORM = {
|
||||
'username': 'newplayer',
|
||||
'email': '[email protected]',
|
||||
'password': 'Password123',
|
||||
'confirm_password': 'Password123',
|
||||
'full_name': 'New Player',
|
||||
'games': 'Valorant',
|
||||
'gamertag_Valorant': 'newplayer#1234',
|
||||
}
|
||||
|
||||
def _submit(self, client, app, **overrides):
|
||||
with client.session_transaction() as session:
|
||||
session['captcha_answer'] = 4
|
||||
payload = dict(self.FORM, captcha_answer='4')
|
||||
payload.update(overrides)
|
||||
return client.post('/auth/register', data=payload, follow_redirects=True)
|
||||
|
||||
def test_a_successful_sign_up_stores_both(self, app, client):
|
||||
self._submit(client, app)
|
||||
|
||||
with app.app_context():
|
||||
user = User.query.filter_by(username='newplayer').one()
|
||||
tags = UserGamertag.query.filter_by(user_id=user.id).all()
|
||||
assert [tag.gamertag for tag in tags] == ['newplayer#1234']
|
||||
|
||||
def test_a_failure_leaves_no_half_account(self, app, client, monkeypatch):
|
||||
from app.routes import auth as auth_module
|
||||
|
||||
real_add = auth_module.db.session.add
|
||||
|
||||
def _explode_on_gamertag(instance, *args, **kwargs):
|
||||
if isinstance(instance, UserGamertag):
|
||||
raise RuntimeError('storage unavailable')
|
||||
return real_add(instance, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(auth_module.db.session, 'add', _explode_on_gamertag)
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
self._submit(client, app)
|
||||
|
||||
with app.app_context():
|
||||
assert User.query.filter_by(username='newplayer').first() is None
|
||||
Reference in New Issue
Block a user