Merge pull request #6286 from readthedocs/proxito-cleanup

Port additional features to proxito

readthedocs/proxito/middleware.py

@@ -8,14 +8,11 @@
 import logging
 
 from django.conf import settings
-from django.http import HttpResponseBadRequest
 from django.shortcuts import render
 from django.utils.deprecation import MiddlewareMixin
-from django.utils.translation import ugettext_lazy as _
 
 from readthedocs.projects.models import Domain
 
-
 log = logging.getLogger(__name__)  # noqa
 
 
@@ -55,7 +52,9 @@ def map_host_to_project_slug(request):
             # TODO: This can catch some possibly valid domains (docs.readthedocs.io.com) for example
             # But these feel like they might be phishing, etc. so let's block them for now.
             log.warning('Weird variation on our hostname: host=%s', host)
-            return HttpResponseBadRequest(_('Invalid hostname'))
+            return render(
+                request, 'core/dns-404.html', context={'host': host}, status=400
+            )
 
     # Serve CNAMEs
     else:

readthedocs/proxito/tests/test_full.py

@@ -2,10 +2,18 @@
 
 import os
 
+import django_dynamic_fixture as fixture
 import mock
 from django.conf import settings
+from django.core.files.storage import Storage
 from django.http import HttpResponse
 from django.test.utils import override_settings
+from django.urls import reverse
+
+from readthedocs.builds.constants import EXTERNAL, INTERNAL
+from readthedocs.builds.models import Version
+from readthedocs.projects import constants
+from readthedocs.projects.models import Project
 
 from .base import BaseDocServing
 
@@ -144,3 +152,212 @@ def test_project_nginx_serving_unicode_filename(self):
             resp['x-accel-redirect'],
             '/proxito/html/project/latest/%C3%BA%C3%B1%C3%AD%C4%8D%C3%B3d%C3%A9.html',
         )
+
+
+@override_settings(
+    PYTHON_MEDIA=False,
+    PUBLIC_DOMAIN='readthedocs.io',
+)
+class TestAdditionalDocViews(BaseDocServing):
+    # Test that robots.txt and sitemap.xml work
+
+    def test_default_robots_txt(self):
+        self.project.versions.update(active=True, built=True)
+        response = self.client.get(
+            reverse('robots_txt'),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.content,
+            b'User-agent: *\nAllow: /\nSitemap: https://project.readthedocs.io/sitemap.xml\n'
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_custom_robots_txt(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+        storage_mock()().exists.return_value = True
+        response = self.client.get(
+            reverse('robots_txt'),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(
+            response['x-accel-redirect'], '/proxito/html/project/latest/robots.txt',
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_directory_indexes(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+        storage_mock()().exists.return_value = True
+        storage_mock()().open().read.return_value = 'foo'
+        # Confirm we've serving from storage for the `index-exists/index.html` file
+        response = self.client.get(
+            reverse('serve_error_404', kwargs={'proxito_path': '/en/latest/index-exists'}),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(
+            response.content, b'foo'
+        )
+        self.assertEqual(
+            response.status_code, 200
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_404_storage_serves_404(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+
+        storage_mock()().exists.side_effect = [False, False, True]
+        response = self.client.get(
+            reverse('serve_error_404', kwargs={'proxito_path': '/en/fancy-version/not-found'}),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        storage_mock()().exists.assert_has_calls(
+            [
+                mock.call('html/project/fancy-version/not-found/index.html'),
+                mock.call('html/project/fancy-version/not-found/README.html'),
+                mock.call('html/project/fancy-version/404.html'),
+            ]
+        )
+        self.assertEqual(
+            response.status_code, 404
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_404_storage_paths_checked(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+        storage_mock()().exists.return_value = False
+        self.client.get(
+            reverse('serve_error_404', kwargs={'proxito_path': '/en/fancy-version/not-found'}),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        storage_mock()().exists.assert_has_calls(
+            [
+                mock.call('html/project/fancy-version/not-found/index.html'),
+                mock.call('html/project/fancy-version/not-found/README.html'),
+                mock.call('html/project/fancy-version/404.html'),
+                mock.call('html/project/fancy-version/404/index.html'),
+                mock.call('html/project/latest/404.html'),
+                mock.call('html/project/latest/404/index.html')
+            ]
+        )
+
+    def test_sitemap_xml(self):
+        self.project.versions.update(active=True)
+        private_version = fixture.get(
+            Version,
+            privacy_level=constants.PRIVATE,
+            project=self.project,
+        )
+        not_translated_public_version = fixture.get(
+            Version,
+            identifier='not-translated-version',
+            verbose_name='not-translated-version',
+            slug='not-translated-version',
+            privacy_level=constants.PUBLIC,
+            project=self.project,
+            active=True
+        )
+        stable_version = fixture.get(
+            Version,
+            identifier='stable',
+            verbose_name='stable',
+            slug='stable',
+            privacy_level=constants.PUBLIC,
+            project=self.project,
+            active=True
+        )
+        # This is a EXTERNAL Version
+        external_version = fixture.get(
+            Version,
+            identifier='pr-version',
+            verbose_name='pr-version',
+            slug='pr-9999',
+            project=self.project,
+            active=True,
+            type=EXTERNAL
+        )
+        # This also creates a Version `latest` Automatically for this project
+        translation = fixture.get(
+            Project,
+            main_language_project=self.project,
+            language='translation-es'
+        )
+        # sitemap hreflang should follow correct format.
+        # ref: https://en.wikipedia.org/wiki/Hreflang#Common_Mistakes
+        hreflang_test_translation_project = fixture.get(
+            Project,
+            main_language_project=self.project,
+            language='zh_CN'
+        )
+        response = self.client.get(
+            reverse('sitemap_xml'),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response['Content-Type'], 'application/xml')
+        for version in self.project.versions(manager=INTERNAL).filter(privacy_level=constants.PUBLIC):
+            self.assertContains(
+                response,
+                self.project.get_docs_url(
+                    version_slug=version.slug,
+                    lang_slug=self.project.language,
+                    private=False,
+                ),
+            )
+
+        # PRIVATE version should not appear here
+        self.assertNotContains(
+            response,
+            self.project.get_docs_url(
+                version_slug=private_version.slug,
+                lang_slug=self.project.language,
+                private=True,
+            ),
+        )
+        # The `translation` project doesn't have a version named `not-translated-version`
+        # so, the sitemap should not have a doc url for
+        # `not-translated-version` with `translation-es` language.
+        # ie: http://project.readthedocs.io/translation-es/not-translated-version/
+        self.assertNotContains(
+            response,
+            self.project.get_docs_url(
+                version_slug=not_translated_public_version.slug,
+                lang_slug=translation.language,
+                private=False,
+            ),
+        )
+        # hreflang should use hyphen instead of underscore
+        # in language and country value. (zh_CN should be zh-CN)
+        self.assertContains(response, 'zh-CN')
+
+        # External Versions should not be in the sitemap_xml.
+        self.assertNotContains(
+            response,
+            self.project.get_docs_url(
+                version_slug=external_version.slug,
+                lang_slug=self.project.language,
+                private=True,
+            ),
+        )
+
+        # Check if STABLE version has 'priority of 1 and changefreq of weekly.
+        self.assertEqual(
+            response.context['versions'][0]['loc'],
+            self.project.get_docs_url(
+                version_slug=stable_version.slug,
+                lang_slug=self.project.language,
+                private=False,
+            ),)
+        self.assertEqual(response.context['versions'][0]['priority'], 1)
+        self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly')
+
+        # Check if LATEST version has priority of 0.9 and changefreq of daily.
+        self.assertEqual(
+            response.context['versions'][1]['loc'],
+            self.project.get_docs_url(
+                version_slug='latest',
+                lang_slug=self.project.language,
+                private=False,
+            ),)
+        self.assertEqual(response.context['versions'][1]['priority'], 0.9)
+        self.assertEqual(response.context['versions'][1]['changefreq'], 'daily')

readthedocs/proxito/urls.py

@@ -30,18 +30,28 @@
 """
 
 from django.conf.urls import url
+from django.views import defaults
 
 from readthedocs.constants import pattern_opts
-from readthedocs.proxito.views import redirect_page_with_filename, serve_docs
+from readthedocs.proxito import views
 
 
 urlpatterns = [
+    # Serve custom 404 pages
+    url(
+        r'^_proxito_404_(?P<proxito_path>.*)$',
+        views.serve_error_404,
+        name='serve_error_404',
+    ),
+    url(r'robots\.txt$', views.robots_txt, name='robots_txt'),
+    url(r'sitemap\.xml$', views.sitemap_xml, name='sitemap_xml'),
+
     # # TODO: Support this?
     # (Sub)project `page` redirect
     url(
         r'^(?:projects/(?P<subproject_slug>{project_slug})/)?'
         r'page/(?P<filename>.*)$'.format(**pattern_opts),
-        redirect_page_with_filename,
+        views.redirect_page_with_filename,
         name='redirect_page_with_filename',
     ),
 
@@ -53,7 +63,7 @@
             r'(?P<version_slug>{version_slug})/'
             r'(?P<filename>{filename_slug})$'.format(**pattern_opts)
         ),
-        serve_docs,
+        views.serve_docs,
         name='docs_detail',
     ),
 
@@ -75,7 +85,11 @@
             r'^(?:projects/(?P<subproject_slug>{project_slug})/)?'
             r'(?P<filename>{filename_slug})$'.format(**pattern_opts)
         ),
-        serve_docs,
+        views.serve_docs,
         name='docs_detail_singleversion_subproject',
     ),
 ]
+
+# Use Django default error handlers to make things simpler
+handler404 = views.fast_404
+handler500 = defaults.server_error