User deactivation didn't include code to persist to the database (e.g. db.session.merge(user)
). Why did the user deactivation test pass?
-
User deactivation code (resources): https://github.com/WildMeOrg/houston/blob/c35500bec2d324775935009fe5451aef683023fb/app/modules/users/resources.py#L195-L210
-
User deactivation code (model): https://github.com/WildMeOrg/houston/blob/c35500bec2d324775935009fe5451aef683023fb/app/modules/users/models.py#L720-L732
-
User deactivation test: https://github.com/WildMeOrg/houston/blob/c35500bec2d324775935009fe5451aef683023fb/tests/modules/users/resources/test_signup.py#L286-L317
-
User.query.get(user_guid)
returns the same instance in the app and in the testSame instance meaning
hash(user)
is the same.When using
<model>.query.get(<primary-key>)
, sqlalchemy caches it in its "identity map". This means it doesn't try to fetch from the database if it is already in the identity map.So when the app modifies the user, then the test tries to assert the changes to the user, the test will pass even if the changes are not in the database.
It is possible to force sqlalchemy to fetch from the database by doing
User.query.filter(User.guid == user_guid).first()
. -
The deactivated changes was actually persisted to the test database
Surprisingly even though there is no code to persist the changes to the database, when I looked at the
houston_test
database, the changes were there:karen@yukihira:~/src/wildme/houston$ docker-compose exec db psql -U houston houston_test -c 'select * from "user"' created | updated | viewed | guid | version | email | password | full_name | website | location | affiliation | forum_id | locale | accepted_user_agreement | use_usa_date_format | show_email_in_profile | receive_notification_emails | receive_newsletter_emails | shares_data | default_identification_catalogue | profile_fileupload_guid | static_role s ----------------------------+----------------------------+----------------------------+--------------------------------------+---------+----------- -----------------------+--------------------------------------------------------------------------------------------------------------------------- -+-------------------+---------+----------+-------------+----------+--------+-------------------------+---------------------+---------------------- -+-----------------------------+---------------------------+-------------+----------------------------------+-------------------------+------------ -- 2021-10-15 15:30:26.738675 | 2021-10-15 15:30:26.738684 | 2021-10-15 15:30:26.73881 | c4dc08e1-f8b8-4971-b282-d57f227451af | | useradmin@ localhost | \x243262243132246c462e37735252736d66376e59444635384f58456d4f6970677045396d325676306b62737631706c2f474f48612f32355671526f6d | First Middle Last | | | | | EN | f | t | f | t | f | t | | | 79104 0 2021-10-15 15:30:27.048615 | 2021-10-15 15:30:27.048631 | 2021-10-15 15:30:27.048638 | a77915ed-0135-4d09-8702-7cd4f0b69f8b | | oauth-user @wildme.org | \x2432622431322442325452394e47434664626e62736462692f436561754765764957533177776d57326f4377354e3430305a5548422e46397a574a69 | | | | | | EN | f | t | f | t | f | t | | | 29900 8 2021-10-15 15:30:27.573834 | 2021-10-15 15:30:27.573851 | 2021-10-15 15:30:27.573858 | 61ad59de-8188-4bb3-9294-e00f170d3316 | | -631943609 8885476203@deactivated | \x24326224313224446a746d355930434352423339582f734448724f5665613239422e53514d677a4368624533797447304c764c38305971382f533243 | Inactivated User | | | | | EN | f | t | f | t | f | t | | | 0 (3 rows)
At what point was the deactivation changes persisted? It's actually in our test client code https://github.com/WildMeOrg/houston/blob/c35500bec2d324775935009fe5451aef683023fb/tests/utils.py#L92
with db.session.begin():
-
with db.session.begin():
persists objects that have changedEven if you do:
with db.session.begin(): pass
The changed user would still persist to the database because (I think, not tested) we set sqlalchemy to auto commit in https://github.com/WildMeOrg/houston/blob/c35500bec2d324775935009fe5451aef683023fb/app/extensions/flask_sqlalchemy/init.py#L52:
kwargs['session_options']['autocommit'] = True
-
User.query.get(user_guid)
also persists changes to the databaseI commented out the code in the test client to not do any
db.session
stuff, and still the deactivation changes got persisted. This time it was this line:User.query.get(user_guid)
Doing the above line in the test persisted the deactivation changes.
But even if we don't do this in the test and use the
GET /api/v1/users/<user-guid>
api, the deactivation changes still got persisted after the API call.
There is no way to check in our current test set up to check that we forgot to persist data to the database.
-
Use actual integration tests with
requests
orselenium
against an actual running houston instance (which I'm working on)I wrote a test using
requests
with the user deactivation stuff and checked the user instances for the DELETE and GET apis and they are different, so the test failed as expected. -
Add some middleware code to check there's no unpersisted changes when response is returned?