From ab0b975211969ce44022de6d26a60e3cf1273d75 Mon Sep 17 00:00:00 2001 From: GGThed Date: Sat, 8 Aug 2026 18:35:38 -0400 Subject: [PATCH] fix(arch): l inscription est une operation, donc une transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- app/routes/auth.py | 6 +- tests/test_transactions.py | 111 +++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/test_transactions.py diff --git a/app/routes/auth.py b/app/routes/auth.py index c75e273..0ad9015 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -379,7 +379,11 @@ def register(): league_os_profile=league_os_profile, ) db.session.add(user) - db.session.commit() + # flush, not commit: the id is needed for the gamertag rows below, + # and signing up is one operation. Committing here made it two, so a + # failure while writing the gamertags left an account whose declared + # games were silently absent (ARCH-006). + db.session.flush() # Create UserGamertag records for each selected game from app.models import UserGamertag diff --git a/tests/test_transactions.py b/tests/test_transactions.py new file mode 100644 index 0000000..921b1cb --- /dev/null +++ b/tests/test_transactions.py @@ -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': 'ghost@example.test', + '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': 'newplayer@example.test', + '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