bug 1431259: remove compression middleware and its tests (#4740)

* bug 1431259: remove compression middleware and its tests * bug 1431259: remove Brotli package and remove/simplify tests
mdn · Apr 23, 2018 · 8a9f4b5 · 8a9f4b5
1 parent 4753c71
commit 8a9f4b5
Show file tree

Hide file tree

Showing 7 changed files with 4 additions and 234 deletions.
diff --git a/kuma/attachments/tests/test_views.py b/kuma/attachments/tests/test_views.py
@@ -246,21 +246,13 @@ def test_raw_file_requires_attachment_host(client, settings, file_attachment):
     assert response['Location'] == url
     assert 'Vary' not in response
 
-    response = client.get(
-        url,
-        HTTP_ACCEPT_ENCODING='gzip',
-        HTTP_HOST=settings.ATTACHMENT_HOST
-    )
+    response = client.get(url, HTTP_HOST=settings.ATTACHMENT_HOST)
     assert response.status_code == 200
     assert response.streaming
     assert response['x-frame-options'] == 'ALLOW-FROM %s' % settings.DOMAIN
-    assert 'Last-Modified' in response
     assert response['Last-Modified'] == convert_to_http_date(created)
-    assert 'Cache-Control' in response
     assert 'public' in response['Cache-Control']
     assert 'max-age=900' in response['Cache-Control']
-    assert response['Content-Encoding'] == 'gzip'
-    assert response['Vary'] == 'Accept-Encoding'
 
 
 def test_raw_file_if_modified_since(client, settings, file_attachment):

diff --git a/kuma/core/middleware.py b/kuma/core/middleware.py
@@ -1,18 +1,14 @@
 import contextlib
-import re
 import urllib
 from urlparse import urljoin
 
-import brotli
-import django.middleware.gzip
 from django.conf import settings
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.core import urlresolvers
 from django.http import (HttpResponseForbidden,
                          HttpResponsePermanentRedirect,
                          HttpResponseRedirect)
 from django.utils import translation
-from django.utils.cache import patch_vary_headers
 from django.utils.encoding import iri_to_uri, smart_str
 from whitenoise.middleware import WhiteNoiseMiddleware
 
@@ -217,65 +213,3 @@ def process_request(self, request):
                 urljoin(settings.SITE_URL, request.get_full_path())
             )
         return None
-
-
-class GZipMiddleware(django.middleware.gzip.GZipMiddleware):
-    """
-    This is identical to Django's GZipMiddleware, except that it will not
-    modify the ETag header.
-
-    TODO: When moving to Django 1.11, this code and its tests can be deleted,
-          and django.middleware.gzip.GZipMiddleware should be used instead.
-    """
-
-    def process_response(self, request, response):
-        original_etag = response.get('etag')
-        response_out = super(GZipMiddleware, self).process_response(
-            request,
-            response
-        )
-        if (original_etag is not None) and response_out.has_header('etag'):
-            response_out['etag'] = original_etag
-        return response_out
-
-
-class BrotliMiddleware(object):
-    """
-    This middleware enables Brotli compression
-
-    This code is inspired by https://github.com/illagrenan/django-brotli
-    """
-
-    MIN_LEN_RESPONSE_TO_PROCESS = 200
-    RE_ACCEPT_ENCODING_BROTLI = re.compile(r'\bbr\b')
-
-    def _accepts_brotli_encoding(self, request):
-        return bool(self.RE_ACCEPT_ENCODING_BROTLI.search(
-            request.META.get('HTTP_ACCEPT_ENCODING', '')))
-
-    def process_response(self, request, response):
-        if (response.streaming or
-                response.has_header('Content-Encoding') or
-                not self._accepts_brotli_encoding(request) or
-                len(response.content) < self.MIN_LEN_RESPONSE_TO_PROCESS):
-            # ---------
-            # 1) Skip streaming content, GZipMiddleware will compress it
-            #    (supported, see https://github.com/google/brotli/issues/191).
-            # 2) Skip if the content is already encoded.
-            # 3) Skip if client didn't request brotli.
-            # 4) Skip if the content is short, compressing isn't worth it
-            #    (same logic as Django's GZipMiddleware).
-            # ---------
-            return response
-
-        compressed_content = brotli.compress(response.content, quality=5)
-
-        # Return the uncompressed content if compression didn't help
-        if len(compressed_content) >= len(response.content):
-            return response
-
-        response.content = compressed_content
-        patch_vary_headers(response, ('Accept-Encoding',))
-        response['Content-Length'] = str(len(compressed_content))
-        response['Content-Encoding'] = 'br'
-        return response
diff --git a/kuma/core/tests/test_middleware.py b/kuma/core/tests/test_middleware.py
@@ -1,13 +1,10 @@
 import pytest
-from django.http import HttpResponse
 from django.test import RequestFactory
 from mock import MagicMock, patch
 
 from . import eq_, KumaTestCase
 from ..middleware import (
-    BrotliMiddleware,
     ForceAnonymousSessionMiddleware,
-    GZipMiddleware,
     LegacyDomainRedirectsMiddleware,
     RestrictedEndpointsMiddleware,
     RestrictedWhiteNoiseMiddleware,
@@ -121,73 +118,3 @@ def test_legacy_domain_redirects_middleware(rf, settings, site_url, host):
         assert response['Location'] == site_url + path
     else:
         assert response is None
-
-
-@pytest.mark.parametrize(
-    'etag_header',
-    (None, '"7ac66c0f148de9519b8bd264312c4d64"')
-)
-def test_gzip_middleware(rf, etag_header):
-    """
-    Test that GZipMiddleware does not modify the ETag header unlike
-    Django's GZipMiddleware.
-
-    TODO: When moving to Django 1.11, this test code and the GZipMiddleware
-          code in kuma.core.middleware can be deleted, and Django's
-          GZipMiddleware should be used instead.
-    """
-    request = rf.get('/foo/bar', HTTP_ACCEPT_ENCODING='gzip')
-    response = HttpResponse(50 * 'yada ')
-    if etag_header:
-        response['etag'] = etag_header
-
-    response_out = GZipMiddleware().process_response(request, response)
-
-    if etag_header:
-        # The ETag header is still there and hasn't been modified.
-        assert 'etag' in response_out
-        assert response_out['etag'] == etag_header
-    else:
-        assert 'etag' not in response
-
-
-def test_gzip_middleware_content_encoding_set(rf):
-    """
-    Test that our GZip middleware doesn't encode already encoded response
-
-    TODO: When moving to Django 1.11, this test code and the GZipMiddleware
-          code in kuma.core.middleware can be deleted, and Django's
-          GZipMiddleware should be used instead.
-    """
-    request = rf.get('/foo/bar', HTTP_ACCEPT_ENCODING='gzip')
-    response = HttpResponse(50 * 'yada ')
-    response['Content-Encoding'] = 'br'
-
-    response_out = GZipMiddleware().process_response(request, response)
-
-    eq_(response, response_out)
-
-
-def test_brotli_middleware(rf):
-    """
-    Test that our brotli middleware returns a brotli encoded response
-    """
-    request = rf.get('/foo/bar', HTTP_ACCEPT_ENCODING='br')
-    response = HttpResponse(50 * 'yada ')
-
-    response_out = BrotliMiddleware().process_response(request, response)
-
-    assert response_out['Content-Encoding'] is 'br'
-
-
-def test_brotli_middleware_content_encoding_set(rf):
-    """
-    Test that our brotli middleware doesn't encode already encoded response
-    """
-    request = rf.get('/foo/bar', HTTP_ACCEPT_ENCODING='br')
-    response = HttpResponse(50 * 'yada ')
-    response['Content-Encoding'] = 'gzip'
-
-    response_out = BrotliMiddleware().process_response(request, response)
-
-    eq_(response, response_out)
diff --git a/kuma/settings/common.py b/kuma/settings/common.py
@@ -475,9 +475,6 @@ def _get_locales():
     # LocaleURLMiddleware must be before any middleware that uses
     # kuma.core.urlresolvers.reverse() to add locale prefixes to URLs:
     'kuma.core.middleware.SetRemoteAddrFromForwardedFor',
-    # TODO: When moving to Django 1.11, replace with Django's GZipMiddleware.
-    'kuma.core.middleware.GZipMiddleware',
-    'kuma.core.middleware.BrotliMiddleware',
     ('kuma.core.middleware.ForceAnonymousSessionMiddleware'
      if MAINTENANCE_MODE else
      'django.contrib.sessions.middleware.SessionMiddleware'),

diff --git a/kuma/wiki/tests/test_views_code.py b/kuma/wiki/tests/test_views_code.py
@@ -40,7 +40,6 @@ def test_code_sample(code_sample_doc, constance_config, client, settings):
     )
     assert response.status_code == 200
     assert response['Access-Control-Allow-Origin'] == '*'
-    assert response['Vary'] == 'Accept-Encoding'
     assert 'Last-Modified' not in response
     assert 'ETag' in response
     assert 'public' in response['Cache-Control']
@@ -57,24 +56,6 @@ def test_code_sample(code_sample_doc, constance_config, client, settings):
         % settings.STATIC_URL)
     assert normalized == expected
 
-    # Get the ETag header value when using gzip to test that GZipMiddleware
-    # plays nicely with ConditionalGetMiddleware when making the following
-    # conditional request.
-    response = client.get(
-        url,
-        HTTP_HOST='testserver',
-        HTTP_ACCEPT_ENCODING='gzip',
-    )
-    assert 'ETag' in response
-    current_etag = response['ETag']
-
-    response = client.get(
-        url,
-        HTTP_HOST='testserver',
-        HTTP_IF_NONE_MATCH=current_etag
-    )
-    assert response.status_code == 304
-
 
 def test_code_sample_host_restriction(code_sample_doc, constance_config,
                                       client):

diff --git a/kuma/wiki/tests/test_views_document.py b/kuma/wiki/tests/test_views_document.py
@@ -13,7 +13,6 @@
 import requests_mock
 from django.test.client import BOUNDARY, encode_multipart, MULTIPART_CONTENT
 from django.utils.six.moves.urllib.parse import urlparse
-from django.utils.text import compress_string
 from pyquery import PyQuery as pq
 from waffle.models import Switch
 
@@ -73,7 +72,7 @@ def section_doc(root_doc, wiki_user):
     The content in this document's current revision contains multiple HTML
     elements with an "id" attribute (or "sections"), and also has a length
     greater than or equal to 200, which meets the compression threshold of
-    the GZipMiddleware.
+    the GZipMiddleware, if used.
     """
     root_doc.current_revision = Revision.objects.create(
         document=root_doc, creator=wiki_user, content=SECTIONS)
@@ -139,7 +138,7 @@ def test_api_safe(client, section_doc, section_case, if_none_match, method):
     if section_id:
         url += '?section={}'.format(section_id)
 
-    headers = dict(HTTP_ACCEPT_ENCODING='gzip')
+    headers = {}
 
     if if_none_match == 'match':
         response = getattr(client, method.lower())(url, **headers)
@@ -164,8 +163,6 @@ def test_api_safe(client, section_doc, section_case, if_none_match, method):
                 str(section_doc.current_revision_id))
 
     if method == 'GET':
-        if response.get('content-encoding') == 'gzip':
-            exp_content = compress_string(exp_content)
         assert response.content == exp_content
 
 
@@ -259,7 +256,7 @@ def test_api_put_existing(client, section_doc, authkey, section_case,
     headers = dict(HTTP_AUTHORIZATION=authkey.header)
 
     if if_match == 'match':
-        response = client.get(url, HTTP_ACCEPT_ENCODING='gzip')
+        response = client.get(url)
         assert 'etag' in response
         headers['HTTP_IF_MATCH'] = response['etag']
     elif if_match == 'mismatch':
@@ -386,37 +383,6 @@ def test_api_put_new(settings, client, root_doc, authkey, section_case,
                     set(data['review_tags'].split(',')))
 
 
-def test_conditional_get(client, section_doc):
-    """
-    Test conditional GET to document view (ETag only currently).
-    """
-    url = section_doc.get_absolute_url() + '$api'
-
-    # Ensure the ETag value is based on the entire content of the response.
-    response = client.get(url)
-    assert response.status_code == 200
-    assert_shared_cache_header(response)
-    assert 'etag' in response
-    assert 'last-modified' not in response
-    assert '"{}"'.format(calculate_etag(response.content)) in response['etag']
-
-    # Get the ETag header value when using gzip to test that GZipMiddleware
-    # plays nicely with ConditionalGetMiddleware when making the following
-    # conditional request.
-    response = client.get(url, HTTP_ACCEPT_ENCODING='gzip')
-    assert response.status_code == 200
-    assert_shared_cache_header(response)
-    assert 'etag' in response
-
-    response = client.get(
-        url,
-        HTTP_ACCEPT_ENCODING='gzip',
-        HTTP_IF_NONE_MATCH=response['etag']
-    )
-
-    assert response.status_code == 304
-
-
 def test_apply_content_experiment_no_experiment(ce_settings, rf):
     """If not under a content experiment, use the original Document."""
     doc = mock.Mock(spec_set=['locale', 'slug'])

diff --git a/requirements/default.txt b/requirements/default.txt
@@ -12,33 +12,6 @@ bleach==2.1.3 \
     --hash=sha256:eb7386f632349d10d9ce9d4a838b134d4731571851149f9cc2c05a9a837a9a44 \
     --hash=sha256:b8fa79e91f96c2c2cd9fd1f9eda906efb1b88b483048978ba62fef680e962b34
 
-# Provide Brotli compression
-# Code: https://github.com/google/brotli/tree/master/python
-Brotli==1.0.1 \
-    --hash=sha256:0d880ed398aa8e8499ec501188db474b7fd3045fb2d4e82b2afbb42972478e54 \
-    --hash=sha256:6e2e169e25ba5118b718f186a203b3362fcd3a4604c7374483814d4115dfad75 \
-    --hash=sha256:9ef2dc4b179a8cd0b656fcd0505e173cfd87c17002c5961e5d8844b1dcd10572 \
-    --hash=sha256:915dd46a46bcb7a1e34187834f9115cdaf2e3dda16f7421759050020721546d3 \
-    --hash=sha256:0596e1388e2ed34b33bdacd54276410cb2e9c487120cefaf1a7df4e4360910ce \
-    --hash=sha256:598401d93fb333550eff1412a6e38fa52344978d175fbc3f9514d5b0217c395c \
-    --hash=sha256:3f1a8ab7ee8c2ddf217748e543f0ffcbeb63df9b1a4ae3f1f2ef830f2987e8e0 \
-    --hash=sha256:76ef1f7ebe4b180dad1e79d545cc20917a89971421ab3ffd2b422eef797d624b \
-    --hash=sha256:e879795cb8d6a84e5bdb84a93c04aab46e0323e08dc143022aedac11e495502d \
-    --hash=sha256:59d5c6d325013888344c8e41c71232e06d0149bb7417eb86ee816db35222c13a \
-    --hash=sha256:257b685bdefad2c5e7005b7b8c3fc6515d3034bdc38cc99925285b8c1074a41e \
-    --hash=sha256:e8cdfbdadeee1cdb7305ba3744f9e862e07a80fc290c3b1ffb291854d05b2e74 \
-    --hash=sha256:e569f4efef30dc0443a9f9ea78f7dc731dced7241ecb90f7ad51f10571280497 \
-    --hash=sha256:025ef123c661094ad7f110b9ea9cdceb1e6b7958cc7123f6e7a774a8a8d62c39 \
-    --hash=sha256:6e9682969bcf9508ae4b52c94ef48aff90fdb6cbe2f90fdf6925f6ad90cdcc06 \
-    --hash=sha256:4046ccb4c60ed2420a501594356ff70ab608255205736c30226bb369d9e5e41a \
-    --hash=sha256:d99d383c5fd7c75dd555e29a96f7632a558e35a33134a289bddba88971ac7dfa \
-    --hash=sha256:330b612dbd3f8f675cf8a22f330ff14d166b4ac266381fdfe0cbe46219080c81 \
-    --hash=sha256:466b72d0564aa527430b140c0f07280d2ca1416d1352e22a88a694cfcd4bdcfc \
-    --hash=sha256:fce4ac55853ad1ac06a44451a21f71d23c33943f8cacc880eeedda198014d26d \
-    --hash=sha256:0cd0d4325de85f8aed5d99efbe308c580a60d0354a598fff643f282205101695 \
-    --hash=sha256:b45299fb73189e5c4c96b76a77c5a155f48092704e0e5e846d7dbfde66285350 \
-    --hash=sha256:36c5d56f4f20c952ed998b0d2f9839a282081fd1f18e82bc3b3e9348b27fbe6d
-
 # Process tasks in the background
 # Code: https://github.com/celery/celery
 # Changes: http://docs.celeryproject.org/en/latest/history/index.html