django-currentuser icon indicating copy to clipboard operation
django-currentuser copied to clipboard

IntegrityError when running django tests

Open FinnGu opened this issue 3 years ago • 5 comments

In my tests, I create an object that has an CurrentUserField. Up until recently everything worked, but now it fails with an SQLdb.IntegrityError after upgrading to python 3.9, django 3.2 and django-currentuser 0.5.3.

I was able to resolve this by explicitly overriding the user in the setUp() method of my test class as follows: _set_current_user(my_user). Is that the recommended approach? If so, we should document it somewhere.

Stacktrace:

ERROR: test_donation_bill (app.fairgeben.tests.test_donation.DonationInternalsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/mysql/base.py", line 73, in execute
    return self.cursor.execute(query, args)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 206, in execute
    res = self._query(query)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 312, in _query
    db.query(q)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/connections.py", line 224, in query
    _mysql.connection.query(self, query)
MySQLdb._exceptions.IntegrityError: (1452, 'Cannot add or update a child row: a foreign key constraint fails (`test_edonapi`.`fairgeben_donationcomment`, CONSTRAINT `fairgeben_donationco_created_by_id_5e9223c0_fk_users_use` FOREIGN KEY (`created_by_id`) REFERENCES `users_user` (`id`))')

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/src/app/fairgeben/tests/test_donation.py", line 290, in test_donation_bill
    self.assertRaises(
  File "/usr/local/lib/python3.9/unittest/case.py", line 733, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/usr/local/lib/python3.9/unittest/case.py", line 201, in handle
    callable_obj(*args, **kwargs)
  File "/usr/src/app/fairgeben/models.py", line 631, in add_to_accounting
    self._log_invalid_status_change(
  File "/usr/src/app/fairgeben/models.py", line 553, in _log_invalid_status_change
    DonationComment.objects.create(
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/query.py", line 453, in create
    obj.save(force_insert=True, using=self.db)
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/base.py", line 726, in save
    self.save_base(using=using, force_insert=force_insert,
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/base.py", line 763, in save_base
    updated = self._save_table(
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/base.py", line 868, in _save_table
    results = self._do_insert(cls._base_manager, using, fields, returning_fields, raw)
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/base.py", line 906, in _do_insert
    return manager._insert(
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/query.py", line 1270, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File "/opt/venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1416, in execute_sql
    cursor.execute(sql, params)
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/opt/venv/lib/python3.9/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/opt/venv/lib/python3.9/site-packages/django/db/backends/mysql/base.py", line 73, in execute
    return self.cursor.execute(query, args)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 206, in execute
    res = self._query(query)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 312, in _query
    db.query(q)
  File "/opt/venv/lib/python3.9/site-packages/MySQLdb/connections.py", line 224, in query
    _mysql.connection.query(self, query)
django.db.utils.IntegrityError: (1452, 'Cannot add or update a child row: a foreign key constraint fails (`test_edonapi`.`fairgeben_donationcomment`, CONSTRAINT `fairgeben_donationco_created_by_id_5e9223c0_fk_users_use` FOREIGN KEY (`created_by_id`) REFERENCES `users_user` (`id`))')

FinnGu avatar Aug 27 '21 13:08 FinnGu

Hi there

I believe i ran into a variant of this as well, the function get_current_authenticated_user() works fine but our tests fail with errors such as:

E               psycopg2.errors.ForeignKeyViolation: insert or update on table "core_country" violates foreign key constraint "core_country_created_by_id_df63df11_fk_core_user_id"
E               DETAIL:  Key (created_by_id)=(6) is not present in table "core_user".

it seems that during the setup of subsequent tests the 'current user' is still set, and the call to get_current_authenticated_user() in the model save() method is trying to reference a non-existing user.

Our fix is to clear the current user after the response = self.get_response(request) line here:

https://github.com/PaesslerAG/django-currentuser/blob/master/django_currentuser/middleware.py#L34

e.g. _set_current_user(None).

Is there a reason why this is not cleared in the code already ?

otherwise thanks for the module :)

-ivan

ivan-price-acted avatar Dec 30 '21 15:12 ivan-price-acted

@ivan-price-acted did you modify the middleware or how exactly did you do that?

Having the exact same issue when running tests. Somebody else has suggested a fixture here https://github.com/PaesslerAG/django-currentuser/issues/47

alfonsrv avatar Jan 04 '22 21:01 alfonsrv

@alfonsrv indeed we run a version of the middleware where just after the line:

https://github.com/PaesslerAG/django-currentuser/blob/master/django_currentuser/middleware.py#L34

, i.e. once we have the response, we set the user to None.

We've not observed any bad side effects, and it avoids having to use special fixtures for the tests.

To be mega explicit:

    def __call__(self, request):
        # request.user closure; asserts laziness;
        # memorization is implemented in
        # request.user (non-data descriptor)
        _do_set_current_user(lambda self: getattr(request, 'user', None))
        response = self.get_response(request)
        # our modification here to prevent tests from failing.
        _set_current_user(None)
        # end modif
        return response

I'd be interested to know what dangers may befall us, for now it works.

-i

ivan-price-acted avatar Jan 05 '22 17:01 ivan-price-acted

Yeah well it feels like it should be the default way to me too that the user is unset after requests. To be thread-safe and avoid the possibility of leaking information.

There already seems to be a PR regarding the topic, but it's stalled https://github.com/PaesslerAG/django-currentuser/pull/49 - tests...

alfonsrv avatar Jan 05 '22 18:01 alfonsrv

ah ok i hadn't found that PR.

looking at the test that fails for me it looks like the test is not good, it is asserting the 'current user' after the request is finished.

https://github.com/PaesslerAG/django-currentuser/blob/master/tests/testapp/tests.py#L46

the assert should be inside the handling for the view, and client.login() could probably be used to avoid the call to the login endpoint which maybe adds complexity for not much gain.

ivan-price-acted avatar Jan 07 '22 11:01 ivan-price-acted