From 20e28ca6efb7165e748b08950760e954acf9640e Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 27 Aug 2018 16:42:57 -0700 Subject: [PATCH] Serve badges directly from local filesystem (#4561) * Serve badges directly from local filesystem * Remove call to static * Add a content type test * Autolint fix and small performance optimization * Log an error when the filesystem can't be read --- readthedocs/projects/views/public.py | 50 ++++++++++++------- .../rtd_tests/tests/test_privacy_urls.py | 2 +- .../rtd_tests/tests/test_project_views.py | 37 +++++++------- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index 6a6d7d95ddd..f50b968902a 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -15,7 +15,6 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth.models import User -from django.contrib.staticfiles.templatetags.staticfiles import static from django.core.cache import cache from django.core.urlresolvers import reverse from django.http import Http404, HttpResponse, HttpResponseRedirect @@ -115,25 +114,40 @@ def project_badge(request, project_slug): style = request.GET.get('style', 'flat') if style not in ("flat", "plastic", "flat-square", "for-the-badge", "social"): style = "flat" - badge_path = 'projects/badges/%s-' + style + '.svg' + + # Get the local path to the badge files + badge_path = os.path.join( + os.path.dirname(__file__), + '..', + 'static', + 'projects', + 'badges', + '%s-' + style + '.svg', + ) + version_slug = request.GET.get('version', LATEST) + file_path = badge_path % 'unknown' + + version = Version.objects.public(request.user).filter( + project__slug=project_slug, slug=version_slug).first() + + if version: + last_build = version.builds.filter(type='html', state='finished').order_by('-date').first() + if last_build: + if last_build.success: + file_path = badge_path % 'passing' + else: + file_path = badge_path % 'failing' + try: - version = Version.objects.public(request.user).get( - project__slug=project_slug, slug=version_slug) - except Version.DoesNotExist: - url = static(badge_path % 'unknown') - return HttpResponseRedirect(url) - version_builds = version.builds.filter(type='html', - state='finished').order_by('-date') - if not version_builds.exists(): - url = static(badge_path % 'unknown') - return HttpResponseRedirect(url) - last_build = version_builds[0] - if last_build.success: - url = static(badge_path % 'passing') - else: - url = static(badge_path % 'failing') - return HttpResponseRedirect(url) + with open(file_path) as fd: + return HttpResponse( + fd.read(), + content_type='image/svg+xml', + ) + except (IOError, OSError): + log.exception('Failed to read local filesystem while serving a docs badge') + return HttpResponse(status=503) def project_downloads(request, project_slug): diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index e02be755ed9..c3edf1c14d4 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -177,7 +177,7 @@ class PublicProjectMixin(ProjectMixin): response_data = { # Public '/projects/pip/downloads/pdf/latest/': {'status_code': 302}, - '/projects/pip/badge/': {'status_code': 302}, + '/projects/pip/badge/': {'status_code': 200}, } def test_public_urls(self): diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 0615bf4aed9..656dd103a86 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -4,13 +4,11 @@ from mock import patch from django.test import TestCase from django.contrib.auth.models import User -from django.contrib.staticfiles.templatetags.staticfiles import static from django.contrib.messages import constants as message_const from django.core.urlresolvers import reverse from django.http.response import HttpResponseRedirect from django.views.generic.base import ContextMixin -from django_dynamic_fixture import get -from django_dynamic_fixture import new +from django_dynamic_fixture import get, new import six @@ -424,10 +422,6 @@ def get_project_queryset(self): class TestBadges(TestCase): """Test a static badge asset is served for each build.""" - # To set `flat` as default style as done in code. - def get_badge_path(self, version, style='flat'): - return static(self.BADGE_PATH % (version, style)) - def setUp(self): self.BADGE_PATH = 'projects/badges/%s-%s.svg' self.project = get(Project, slug='badgey') @@ -436,32 +430,39 @@ def setUp(self): def test_unknown_badge(self): res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('unknown') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'unknown') + + # Unknown project + unknown_project_url = reverse('project_badge', args=['fake-project']) + res = self.client.get(unknown_project_url, {'version': 'latest'}) + self.assertContains(res, 'unknown') def test_passing_badge(self): get(Build, project=self.project, version=self.version, success=True) res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('passing') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'passing') + self.assertEqual(res['Content-Type'], 'image/svg+xml') def test_failing_badge(self): get(Build, project=self.project, version=self.version, success=False) res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('failing') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'failing') def test_plastic_failing_badge(self): get(Build, project=self.project, version=self.version, success=False) res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'plastic'}) - static_badge = self.get_badge_path('failing', 'plastic') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'failing') + + # The plastic badge has slightly more rounding + self.assertContains(res, 'rx="4"') def test_social_passing_badge(self): get(Build, project=self.project, version=self.version, success=True) - res = self.client.get(self.badge_url, {'version': self.version.slug , 'style': 'social'}) - static_badge = self.get_badge_path('passing', 'social') - self.assertEquals(res.url, static_badge) + res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'social'}) + self.assertContains(res, 'passing') + + # The social badge (but not the other badges) has this element + self.assertContains(res, 'rlink') class TestTags(TestCase):