Opened 2 years ago

Closed 2 years ago

Last modified 7 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: serdaroncode 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 claudep)

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

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 years ago by claudep

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

comment:2 Changed 2 years ago by claudep

  • Easy pickings set
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 2 years ago by ddavies@…

Patched to make delete() a pre_delete signal

Changed 2 years ago by ddavies@…

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

Changed 2 years ago by ddavies@…

Sorry, forgot to take out decorator on previous patch.

Changed 2 years ago by serdaroncode

comment:3 Changed 2 years ago by serdaroncode

  • Cc serdaroncode added
  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin
  • Version changed from 1.4 to 1.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 2 years ago by DHessing

  • Version changed from 1.5-rc-1 to 1.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 2 years ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 7 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