Standardize Site/RequestSite access; looser coupling of sites framework in contrib

[Gabriel Hurley]

A few days ago I started ​this thread on the Django Developers list in order to solve a number of tickets at the same time. The gist:

All of these tickets relate to needing access to the current domain, whether that's through a Site instance, or through a RequestSite instance. To facilitate this, the provided patch adds a utility function, django.contrib.sites.models.get_current_site which takes a request as an argument and returns either a Site or RequestSite instance.

Using that utility function, a lot of repetitious code could be removed and streamlined, and several tickets were trivial to resolve.

Included in this patch:

• Adds the django.contrib.sites.models.get_current_site function.
• Provides a note in the contrib.sites docs regarding the function.
• Converts to contrib.sites tests from doctests to unit tests and adds a test for get_current_site
• Resolves #8960 by allowing django.contrib.contenttypes.views.shortcut to use either a Site or RequesSite object.
• Converts contrib.contenttypes tests from doctests to unit tests and adds a test for #8960.
• Resolves #10235 and #10909 by using get_current_site in the Sitemaps framework.
• Adds test case for #10235/#10909.
• Updates django.contrib.syndication.views to use `get_current_site.
• Updates django.contrib.gis.sitemaps.views to use get_current_site as per #10608.
• Updates django.contrib.auth.views.login to use get_current_site.

The patch passes the full test suite under WinXP/Py2.6/SQLite.

This patch would completely fix #8960, #10235, and #10909. The remainder of #10608 is actually invalid, because the remaining places where contrib apps rely on contrib.sites the model has a direct relation to Site, or there is no way to access a request object to use RequestSite. Any further decoupling in contrib would require backwards-incompatible rewrites.

[Gabriel Hurley]

I'm not sure why Trac isn't displaying the diff for this patch, but the code *is* in there... ::sigh::

[Gabriel Hurley]

Updated to fix #14377 as well. Uses get_current_site() in contrib.auth.views.logout to make site accessible to logout template as in login view.

[Gabriel Hurley]

Since apparently overwriting an old patch file in Trac overwrites messages added with it, I'll add a permanent note that this patch now solves #13845 (using get_current_site in contrib.auth.forms.PasswordResetForm.save), and #14377 (using get_current_site in contrib.auth.views.logout to make the current site accessible in the logout template as it is in the login view/template).

Complete list of tickets affected currently: #8960, #10235, #10909, #10608, #13845, #14377.

[Luke Plant]

I will commit this shortly. I've made some tweaks to the patch, which I'll document here for your benefit:

1. In the tests, settings.DEBUG should be restored to it's original value in a tearDown method, rather than in the test and rather than to the value 'False'. This is to avoid relying on assumptions about settings files or other code - it might be safe now, but that could change.

2. Similarly, in the tests, Site._meta.installed should also be restored in the tearDown method, and again to the original value, and the test should set it to True if it is assuming that it is installed.

3. According to PEP8, comments should start with a # followed by a space - the space was missing in some of the comments. Also, according to PEP8 there should be be two lines between top level definitions in modules - a convention we haven't followed properly so far, but are trying to do with new files.

4. I moved a tearDown method to after the setup, since they belong together.

5. There was an unneeded import in one test, and a remaining unneeded import of Site/RequestSite in auth.views, and one in FlatPageSitemap

6. Technically I think #13845 and #14377 should have had separate patches with tests. I added a test for #14377 since it was easy to do.

7. Whitespace - we like trailing spaces removed in the Django code base. It's admittedly hard to see in some editors, Emacs has an option for seeing it.

BTW, Trac didn't show the patch because of a bug where it fails to parse "\ No newline at end of file" in the patch, which wasn't your fault.

Notwithstanding those comments, thanks for putting all this together, great work.

[Luke Plant]

(In [13980]) Fixed #14386, #8960, #10235, #10909, #10608, #13845, #14377 - standardize Site/RequestSite usage in various places.

Many thanks to gabrielhurley for putting most of this together. Also to
bmihelac, arthurk, qingfeng, hvendelbo, petr.pulc@…, Hraban for
reports and some initial patches.

The patch also contains some whitespace/PEP8 fixes.

[Gabriel Hurley]

Thanks for the final cleanup (and commit) on the patch, Luke. The inconsistent whitespace on comments and the like are what I get when writing too much code too late at night. Good point on TearDown, too.

As for rolling the other two tickets into it later, I did that simply because the solution to them relied on the same get_current_site method, and I wasn't sure if this would get committed soon or days/weeks from now. Didn't want to write a patch for those that wouldn't apply cleanly until this one was in and have it end up getting messy. I certainly see your point though.

Thanks!

[Luke Plant]

(In [13987]) [1.2.X] Fixed #14386, #8960, #10235, #10909, #10608, #13845, #14377 - standardize Site/RequestSite usage in various places.

Many thanks to gabrielhurley for putting most of this together. Also to
bmihelac, arthurk, qingfeng, hvendelbo, petr.pulc@…, Hraban for
reports and some initial patches.

The patch also contains some whitespace/PEP8 fixes.

Backport of [13980] from trunk. Some items could be considered features
(i.e. supporting RequestSite in various contrib apps), others are definite
bugs, but it was much more robust to backport all these tightly related
changes together.

[Luke Plant]

(In [14141]) Fixed #14433 - replaced a thread-unsafe solution to #10235 introduced in [13980]

This patch also addresses sitemap code found in contrib/gis, which [13980]
did not.

Thanks to gabrielhurley for the initial patch.

Refs #10235, #14386

[Luke Plant]

(In [14142]) [1.2.X] Fixed #14433 - replaced a thread-unsafe solution to #10235 introduced in [13980]

This patch also addresses sitemap code found in contrib/gis, which [13980]
did not.

Thanks to gabrielhurley for the initial patch.

Refs #10235, #14386

Backport of [14141] from trunk.

[Jacob]

Milestone 1.3 deleted