Re: [Django] #34137: model.refresh_from_db() doesn't clear cached generic foreign keys
Django
--------------------------------------+------------------------------------
Reporter: pascal chambon | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Simon Charette):
Bonjour Pascal,
It seems to be an oversight in `Model.refresh_from_db` as it should also
consider `_meta.private_fields` which is where `GenericForeignKey` and
`GenericRel` end up as opposed to `related_objects`. Something along these
lines should address the issue
{{{#!diff
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 2eb7ba7e9b..0f5f8d0881 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -737,6 +737,11 @@ def refresh_from_db(self, using=None, fields=None):
if field.is_cached(self):
field.delete_cached_value(self)
+ # Clear cached private relations.
+ for field in self._meta.private_fields:
+ if field.is_relation and field.is_cached(self):
+ field.delete_cached_value(self)
+
self._state.db = db_instance._state.db
async def arefresh_from_db(self, using=None, fields=None):
}}}
Would you be interested in submitting a patch whit these changes and
[https://github.com/django/django/blob/3dc9f3ac6960c83cd32058677eb0ddb5a5e5da43/tests/contenttypes_tests/test_fields.py#L12-L44
adding a regression test to the suite]?
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: pascal chambon | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Bhuvnesh):
Hi,
I was just working around to write a regression test for the issue, if not
pascal I would like to submit a patch with a test and changes proposed by
simon. Thanks :)
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: pascal chambon | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by pascal chambon):
Please proceed yes ^^
I'll be happy to backport/test the PR against my own project to validate
it in real conditions
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:5>
Django
--------------------------------------+------------------------------------
Type: Bug | Status: assigned
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: nobody => Bhuvnesh
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: pascal chambon | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Bhuvnesh):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16260 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:7>
Django
Reporter: pascal chambon | Owner: Bhuvnesh
Type: Bug | Status: assigned
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Accepted => Ready for checkin
Comment:
Patch is looking great, thanks for the tests Bhuvnesh.
Pascal, did you confirm [https://github.com/django/django/pull/16260 the
proposed patch] solves your issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: pascal chambon | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: | Version: 3.2
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by pascal chambon):
Juste tested the patch against my codebase, now the code works without
forcing a "authenticated_user.controlled_entity.refresh_from_db()" :)
So looks quite good to me !
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: pascal chambon | Owner: Bhuvnesh
Component: | Version: 3.2
contrib.contenttypes |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"123b1d3fcf79f091573c40be6da7113a6ef35b62" 123b1d3]:
{{{
#!CommitTicketReference repository=""
revision="123b1d3fcf79f091573c40be6da7113a6ef35b62"
Fixed #34137 -- Made Model.refresh_from_db() clear cached generic
relations.
Thanks Simon Charette for the implementation idea.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34137#comment:10>