Opened 4 years ago

Closed 4 years ago

Last modified 22 months ago

#19698 closed Bug (fixed)

Deleting Sites through a manager does not clear cache

Reported by: ddavies@… Owned by: nobody
Component: contrib.sites Version: 1.4
Severity: Normal Keywords:
Cc: Serdar Dalgic Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Claude Paroz)

When you delete a Site instance, i.e. Site.objects.get_current().delete(), the cache is cleared. However, if you delete all sites, Site.objects.all().delete(), the cache is not cleared.

    def test_delete_all_sites_clears_cache(self):
        """
        When all site objects are deleted the cache should  also
        be cleared and get_current should raise a DoesNotExist
        """
        from django.contrib.sites.models import Site
        site = Site.objects.create(domain='example.com', name='test')
        self.assertIsInstance(Site.objects.get_current(), Site)
        Site.objects.all().delete()
        self.assertRaises(Site.DoesNotExist, Site.objects.get_current)

Attachments (4)

19698.patch (7.6 KB) - added by ddavies@… 4 years ago.
Patched to make delete() a pre_delete signal
19698-2.patch (8.0 KB) - added by ddavies@… 4 years ago.
Refactored the previous patch as Site no longer needed a save() method either.
19698-3.patch (7.9 KB) - added by ddavies@… 4 years ago.
Sorry, forgot to take out decorator on previous patch.
19698-rebase-and-cleaned.diff (2.2 KB) - added by Serdar Dalgic 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by Claude Paroz

Description: modified (diff)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 4 years ago by Claude Paroz

Easy pickings: set
Triage Stage: UnreviewedAccepted

The delete() method of the Site class should probably be replaced by a pre_delete signal.

Changed 4 years ago by ddavies@…

Attachment: 19698.patch added

Patched to make delete() a pre_delete signal

Changed 4 years ago by ddavies@…

Attachment: 19698-2.patch added

Refactored the previous patch as Site no longer needed a save() method either.

Changed 4 years ago by ddavies@…

Attachment: 19698-3.patch added

Sorry, forgot to take out decorator on previous patch.

Changed 4 years ago by Serdar Dalgic

comment:3 Changed 4 years ago by Serdar Dalgic

Cc: Serdar Dalgic added
Has patch: set
Triage Stage: AcceptedReady for checkin
Version: 1.41.5-rc-1

Hi;

I've rebased the patch, checked and fixed some pep8 violations.

I've also moved the function to the end of the file, instead of writing in after the class Site.

The patch is ready for checking. I've also changed the version to 1.5-rc1, because it is a bug. If you don't agree, you can change it.

This is my first work in Django trac, so sorry if I did something wrong :) Thanks for your help, any comments are appreciated.

comment:4 Changed 4 years ago by DHessing

Version: 1.5-rc-11.4

You can't mark your own patches as Ready For Check-in and version is used for when the bug was identified. Otherwise the patch looks fine! Leaving as RFC.

comment:5 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 98ab8e8876e130de24e1de9473a4db716d5b002e:

Fixed #19698 -- Cleared sites cache with signals

Thanks ddavies at nomensa.com for the report and the patch and
serdaroncode for reviewing the patch.

comment:6 Changed 22 months ago by Tim Graham <timograham@…>

In 777b4c26e343d07764a35132462e92c07e4b0aec:

Removed a clear_cache statement in contrib.sites.create_default_site.

It was originally added to fix a test (refs #7514); but Site now has a
pre_save signal handler (refs #19698) to clear the cache which makes
this call redundant.

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