Opened 4 years ago

Closed 4 years ago

#27406 closed Uncategorized (invalid)

Signals pre_delete and post_delete break logical relationship of models and behave in diffrently with different backends

Reported by: Seti Owned by: nobody
Component: Database layer (models, ORM) Version: 1.10
Severity: Normal Keywords: Databases
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The mini-project with details https://bitbucket.org/setivolkylany/test

If need more info, feel free and write here setivolkylany@ gmail.com

Change History (5)

comment:1 Changed 4 years ago by Tim Graham

Maybe #24228 ("Signals have unclear & inconsistent transaction handling") is related? It would be nice to update the ticket description with a high level description of the problem so that it's not required to view the link to understand the basics.

comment:2 in reply to:  1 Changed 4 years ago by Seti

Replying to Tim Graham:

Maybe #24228 ("Signals have unclear & inconsistent transaction handling") is related? It would be nice to update the ticket description with a high level description of the problem so that it's not required to view the link to understand the basics.

I made description of the problem in REAMDE of repository. Did you at least looked to repository?

The problem exists as with the setting ATOMIC_REQUESTS = True or ATOMIC_REQUESTS = False (on the PostgreSQL exactly).

I will try describe the problem:

Custom the app 'users' contains three models: User, Vote and Notification.

User - my own implementation of a user model;

Vote - keep votes of a user by date; Has CASCADE related with the model User by field user;

Notification - keep notification about actions of a user by field 'user'; Namely register, unregister and add, update, delete vote. A user may be null in the field 'user'.

Database schema
http://www.tiikoni.com/tis/view/

Task is next:
Save all actions of a user in the model Notification.

I am use for it signals pre_delete, post_delete, post_save and own signal for write new records in the the model Notification.

The signal post_save worked, but pre_delete, post_delete signals on PostgreSQL and MySQL. But all worked on the SQLite.

Problem:
if user will be deleted, it the field 'user' in the model Notification must be set to null. After user was deleted notifications about it must not be deleted.
But this does not happen on on PostgreSQL and MySQL (see traceback https://bitbucket.org/setivolkylany/test/src/318d38381d86cdc1421fec500c03a5a7ffa16b42/README.md?at=default&fileviewer=file-view-default). If I made disabled the signals pre_delete, post_delete - all worked: a user is deleted, but notifications for him still exists in the model Notification.

If something is does not clean, see code:
https://bitbucket.org/setivolkylany/test/src/318d38381d86cdc1421fec500c03a5a7ffa16b42/apps/users/models.py?at=default&fileviewer=file-view-default;]
https://bitbucket.org/setivolkylany/test/src/318d38381d86cdc1421fec500c03a5a7ffa16b42/apps/users/signals.py?at=default&fileviewer=file-view-default;

or tell me

Sorry for English grammar mistakes

comment:3 Changed 4 years ago by Tim Graham

There's probably no issue on SQLite because we don't have support for foreign key constraints there (#14204). I haven't finished investigating to see if there's a bug with the other databases.

comment:4 in reply to:  3 Changed 4 years ago by Seti

Replying to Tim Graham:

Resolved.

If a field has attributes null=True and blank=True, and using signals pre_delete (as example) attribute "on_delete" does not working at all.
I tried on_delete=models.CASCADE and models.SET_NULL, it is does not working.
I made dived deep in SQL generate from the Django ORM.

On my code (with activate signal "pre_delete")

User.objects.filter().delete()

it generated a next code (a main part):

DELETE FROM "users_user" WHERE "users_user"."id" IN ('87420c3a-dae9-4c87-9095-064d8806530b'::uuid)
DELETE FROM "users_profile" WHERE "users_profile"."id" IN ('f4d4aa39-e645-4da0-aef3-c2853c5d5e91'::uuid)
DELETE FROM "diaries_diary" WHERE "diaries_diary"."id" IN ('b12f5507-3957-4384-ab32-9ffbf30acd87'::uuid)
INSERT INTO "notifications_notification" ("id", "level", "action", "is_read", "is_deleted", "is_public", "is_emailed", "created", "is_anonimuos", "user_id", "user_display_text", "target_content_type_id", "target_object_id", "target_display_text", "action_target_content_type_id", "action_target_object_id") VALUES ('1d2b02d2-aec0-45fd-a23f-ef01262f4960'::uuid, 'success', 'deleted_user', false, false, true, true, '2016-11-08T09:38:08.317519+00:00'::timestamptz, false, '87420c3a-dae9-4c87-9095-064d8806530b'::uuid, 'uкузнецов (savannah14@lee.com)', NULL, NULL, NULL, NULL, NULL)
UPDATE "notifications_notification" SET "level" = 'success', "action" = 'deleted_user', "is_read" = false, "is_deleted" = false, "is_public" = true, "is_emailed" = true, "created" = NULL, "is_anonimuos" = false, "user_id" = '87420c3a-dae9-4c87-9095-064d8806530b'::uuid, "user_display_text" = 'uкузнецов (savannah14@lee.com)'', "target_content_type_id" = NULL, "target_object_id" = NULL, "target_display_text" = NULL, "action_target_content_type_id" = NULL, "action_target_object_id" = NULL WHERE "notifications_notification"."id" = '1d2b02d2-aec0-45fd-a23f-ef01262f4960'::uuid

An error is here

"user_id" = '87420c3a-dae9-4c87-9095-064d8806530b'::uuid, "user_display_text" = 'uкузнецов (savannah14@lee.com)''

"user_id" must be NULL, because a user must be deleted

On a python side it is resolved very simple

class Notification(models.Model):

    ..........................

    def save(self, *args, **kwargs):
        ..................................
        user = kwargs.pop('user', None)
        if user is not None:
            
            # user will be deleted, so we keep it full name for display
            self.user_display_text = user.get_full_name()

            # manualy set field "user" to None (NULL in SQL), because the Django does not made it
            self.user = None

        self.full_clean()
        super(Notification, self).save(*args, **kwargs)

So, dear the Django developer team, please point this caution about using signals "pre_delete" and "post_delete" in the docs.

comment:5 Changed 4 years ago by Tim Graham

Resolution: invalid
Status: newclosed

Feel free to propose some text, but the behavior doesn't seem surprising to me. The post_delete docs already have a note that says, "Note that the object will no longer be in the database, so be very careful what you do with this instance."

Note: See TracTickets for help on using tickets.
Back to Top