Merge pull request #18314 from edx/youngstrom/remove_django_18_shim

Remove temp django upgrade logic
This commit is contained in:
Michael Youngstrom
2018-06-06 11:09:17 -04:00
committed by GitHub
40 changed files with 100 additions and 465 deletions

View File

@@ -1,6 +1,5 @@
""" Form widget classes """
import django
from django.conf import settings
from django.urls import reverse
from django.forms.utils import flatatt
@@ -16,14 +15,9 @@ class TermsOfServiceCheckboxInput(CheckboxInput):
""" Renders a checkbox with a label linking to the terms of service. """
def render(self, name, value, attrs=None):
# TODO: Remove Django 1.11 upgrade shim
# SHIM: Compensate for behavior change of default authentication backend in 1.10
if django.VERSION < (1, 11):
final_attrs = self.build_attrs(attrs, type='checkbox', name=name)
else:
extra_attrs = attrs.copy()
extra_attrs.update({'type': 'checkbox', 'name': name})
final_attrs = self.build_attrs(self.attrs, extra_attrs=extra_attrs) # pylint: disable=redundant-keyword-arg
extra_attrs = attrs.copy()
extra_attrs.update({'type': 'checkbox', 'name': name})
final_attrs = self.build_attrs(self.attrs, extra_attrs=extra_attrs)
if self.check_test(value):
final_attrs['checked'] = 'checked'

View File

@@ -24,11 +24,8 @@ class SessionAuthenticationCrossDomainCsrf(authentication.SessionAuthentication)
Since this subclass overrides only the `enforce_csrf()` method,
it can be mixed in with other `SessionAuthentication` subclasses.
"""
# TODO: Remove Django 1.11 upgrade shim
# SHIM: Call new process_request in Django 1.11 to process CSRF token in cookie.
def _process_enforce_csrf(self, request):
if django.VERSION >= (1, 11):
CsrfViewMiddleware().process_request(request)
CsrfViewMiddleware().process_request(request)
return super(SessionAuthenticationCrossDomainCsrf, self).enforce_csrf(request)
def enforce_csrf(self, request):

View File

@@ -44,19 +44,12 @@ CSRF cookie.
import logging
import django
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed
from django.middleware.csrf import CsrfViewMiddleware
from .helpers import is_cross_domain_request_allowed, skip_cross_domain_referer_check
# TODO: Remove Django 1.11 upgrade shim
# SHIM: Remove birdcage references post-1.11 upgrade as it is only in place to help during that deployment
if django.VERSION < (1, 9):
from birdcage.v1_11.csrf import CsrfViewMiddleware
else:
from django.middleware.csrf import CsrfViewMiddleware
log = logging.getLogger(__name__)

View File

@@ -5,18 +5,11 @@ Tests for the CORS CSRF middleware
from mock import patch, Mock
import ddt
import django
from django.test import TestCase
from django.test.utils import override_settings
from django.core.exceptions import MiddlewareNotUsed, ImproperlyConfigured
from django.http import HttpResponse
# TODO: Remove Django 1.11 upgrade shim
# SHIM: Remove birdcage references post-1.11 upgrade as it is only in place to help during that deployment
if django.VERSION < (1, 9):
from birdcage.v1_11.csrf import CsrfViewMiddleware
else:
from django.middleware.csrf import CsrfViewMiddleware
from django.middleware.csrf import CsrfViewMiddleware
from ..middleware import CorsCSRFMiddleware, CsrfCrossDomainCookieMiddleware

View File

@@ -8,7 +8,6 @@ import unittest
from importlib import import_module
from urllib import urlencode
import django
import pytest
from ddt import ddt, data
from django.conf import settings
@@ -23,7 +22,6 @@ from openedx.core.djangoapps.external_auth.views import (
shib_login, course_specific_login, course_specific_register, _flatten_to_ascii
)
from openedx.core.djangoapps.user_api import accounts as accounts_settings
from openedx.tests.util import expected_redirect_url
from mock import patch
from nose.plugins.attrib import attr
from six import text_type
@@ -369,12 +367,7 @@ class ShibSPTest(CacheIsolationTestCase):
if len(external_name.strip()) < accounts_settings.NAME_MIN_LENGTH:
self.assertEqual(profile.name, postvars['name'])
else:
expected_name = external_name
# TODO: Remove Django 1.11 upgrade shim
# SHIM: form character fields strip leading and trailing whitespace by default in Django 1.9+
if django.VERSION >= (1, 9):
expected_name = expected_name.strip()
self.assertEqual(profile.name, expected_name)
self.assertEqual(profile.name, external_name.strip())
self.assertNotIn(u';', profile.name)
else:
self.assertEqual(profile.name, self.client.session['ExternalAuthMap'].external_name)
@@ -586,7 +579,7 @@ class ShibSPTestModifiedCourseware(ModuleStoreTestCase):
# successful login is a redirect to the URL that handles auto-enrollment
self.assertEqual(response.status_code, 302)
self.assertEqual(response['location'],
expected_redirect_url('/account/finish_auth?{}'.format(urlencode(params))))
'/account/finish_auth?{}'.format(urlencode(params)))
class ShibUtilFnTest(TestCase):

View File

@@ -20,7 +20,6 @@ from openedx.core.djangoapps.external_auth.models import ExternalAuthMap
import openedx.core.djangoapps.external_auth.views as external_auth_views
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_cms, skip_unless_lms
from openedx.tests.util import expected_redirect_url
from student.models import CourseEnrollment
from student.roles import CourseStaffRole
from student.tests.factories import UserFactory
@@ -183,7 +182,7 @@ class SSLClientTest(ModuleStoreTestCase):
response = self.client.get(
reverse('dashboard'), follow=True,
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
self.assertEquals((expected_redirect_url('/dashboard'), 302),
self.assertEquals(('/dashboard', 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@@ -197,7 +196,7 @@ class SSLClientTest(ModuleStoreTestCase):
response = self.client.get(
reverse('register_user'), follow=True,
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
self.assertEquals((expected_redirect_url('/dashboard'), 302),
self.assertEquals(('/dashboard', 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@@ -237,7 +236,7 @@ class SSLClientTest(ModuleStoreTestCase):
response = self.client.get(
reverse('signin_user'), follow=True,
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
self.assertEquals((expected_redirect_url('/dashboard'), 302),
self.assertEquals(('/dashboard', 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@@ -360,7 +359,7 @@ class SSLClientTest(ModuleStoreTestCase):
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL),
HTTP_ACCEPT='text/html'
)
self.assertEqual((expected_redirect_url(course_private_url), 302),
self.assertEqual((course_private_url, 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@@ -392,7 +391,7 @@ class SSLClientTest(ModuleStoreTestCase):
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL),
HTTP_ACCEPT='text/html'
)
self.assertEqual((expected_redirect_url(course_private_url), 302),
self.assertEqual((course_private_url, 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@@ -410,7 +409,7 @@ class SSLClientTest(ModuleStoreTestCase):
response = self.client.get(
reverse('dashboard'), follow=True,
SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
self.assertEquals((expected_redirect_url('/dashboard'), 302),
self.assertEquals(('/dashboard', 302),
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
response = self.client.get(

View File

@@ -5,8 +5,8 @@ from __future__ import unicode_literals
from datetime import datetime
import django
from django.contrib.auth import authenticate, get_user_model
from django.contrib.auth.backends import AllowAllUsersModelBackend as UserModelBackend
from django.db.models.signals import pre_save
from django.dispatch import receiver
from oauth2_provider.models import AccessToken
@@ -31,17 +31,6 @@ def on_access_token_presave(sender, instance, *args, **kwargs): # pylint: disab
RestrictedApplication.set_access_token_as_expired(instance)
# TODO: Remove Django 1.11 upgrade shim
# SHIM: Allow users that are inactive to still authenticate while keeping rate-limiting functionality.
if django.VERSION < (1, 10):
# Old backend which allowed inactive users to authenticate prior to Django 1.10.
from django.contrib.auth.backends import ModelBackend as UserModelBackend
else:
# Django 1.10+ ModelBackend disallows inactive users from authenticating, so instead we use
# AllowAllUsersModelBackend which is the closest alternative.
from django.contrib.auth.backends import AllowAllUsersModelBackend as UserModelBackend
class EdxRateLimitedAllowAllUsersModelBackend(RateLimitMixin, UserModelBackend):
"""
Authentication backend needed to incorporate rate limiting of login attempts - but also

View File

@@ -6,7 +6,6 @@ import os.path
import posixpath
import re
import django
from django.conf import settings
from django.contrib.staticfiles.finders import find
from django.contrib.staticfiles.storage import CachedFilesMixin, StaticFilesStorage
@@ -171,24 +170,6 @@ class ThemeCachedFilesMixin(CachedFilesMixin):
return asset_name
# TODO: Remove Django 1.11 upgrade shim
# SHIM: This override method modifies the name argument to contain a theme
# prefix when Django < 1.11. In Django >= 1.11, asset name processing is
# done in the _url function, so this method becomes a no-op passthrough.
# After the 1.11 upgrade, delete this method.
def url(self, name, force=False):
"""
This override method serves a similar function to _url, but this is
needed for Django < 1.11.
"""
if django.VERSION < (1, 11):
processed_asset_name = self._processed_asset_name(name)
return super(ThemeCachedFilesMixin, self).url(processed_asset_name, force)
else:
# Passthrough directly to the function we are overriding. _url in
# Django 1.11+ will take care of processing the asset name.
return super(ThemeCachedFilesMixin, self).url(name, force)
def _url(self, hashed_name_func, name, force=False, hashed_files=None):
"""
This override method swaps out `name` with a processed version.
@@ -198,153 +179,6 @@ class ThemeCachedFilesMixin(CachedFilesMixin):
processed_asset_name = self._processed_asset_name(name)
return super(ThemeCachedFilesMixin, self)._url(hashed_name_func, processed_asset_name, force, hashed_files)
# TODO: Remove Django 1.11 upgrade shim
# SHIM: This method implements url_converter for Django < 1.11.
# After the 1.11 upgrade, delete this method.
def _url_converter__lt_111(self, name, template=None):
"""
This is an override of url_converter from CachedFilesMixin.
There are two lines commented out in order to make the converter method
return absolute urls instead of relative urls. This behavior is
necessary for theme overrides, as we get 404 on assets with relative
urls on a themed site.
"""
if template is None:
template = self.default_template
def converter(matchobj):
"""
Converts the matched URL depending on the parent level (`..`)
and returns the normalized and hashed URL using the url method
of the storage.
"""
matched, url = matchobj.groups()
# Completely ignore http(s) prefixed URLs,
# fragments and data-uri URLs
if url.startswith(('#', 'http:', 'https:', 'data:', '//')):
return matched
name_parts = name.split(os.sep)
# Using posix normpath here to remove duplicates
url = posixpath.normpath(url)
url_parts = url.split('/')
parent_level, sub_level = url.count('..'), url.count('/')
if url.startswith('/'):
sub_level -= 1
url_parts = url_parts[1:]
if parent_level or not url.startswith('/'):
start, end = parent_level + 1, parent_level
else:
if sub_level:
if sub_level == 1:
parent_level -= 1
start, end = parent_level, 1
else:
start, end = 1, sub_level - 1
joined_result = '/'.join(name_parts[:-start] + url_parts[end:])
hashed_url = self.url(unquote(joined_result), force=True)
# NOTE:
# following two lines are commented out so that absolute urls are used instead of relative urls
# to make themed assets work correctly.
#
# The lines are commented and not removed to make future django upgrade easier and
# show exactly what is changed in this method override
#
# file_name = hashed_url.split('/')[-1:]
# relative_url = '/'.join(url.split('/')[:-1] + file_name)
# Return the hashed version to the file
return template % unquote(hashed_url)
return converter
# TODO: Remove Django 1.11 upgrade shim
# SHIM: This method implements url_converter for Django >= 1.11.
# After the 1.11 upgrade, rename this method to url_converter.
def _url_converter__gte_111(self, name, hashed_files, template=None):
"""
This is an override of url_converter from CachedFilesMixin.
It changes one line near the end of the method (see the NOTE) in order
to return absolute urls instead of relative urls. This behavior is
necessary for theme overrides, as we get 404 on assets with relative
urls on a themed site.
"""
if template is None:
template = self.default_template
def converter(matchobj):
"""
Convert the matched URL to a normalized and hashed URL.
This requires figuring out which files the matched URL resolves
to and calling the url() method of the storage.
"""
matched, url = matchobj.groups()
# Ignore absolute/protocol-relative and data-uri URLs.
if re.match(r'^[a-z]+:', url):
return matched
# Ignore absolute URLs that don't point to a static file (dynamic
# CSS / JS?). Note that STATIC_URL cannot be empty.
if url.startswith('/') and not url.startswith(settings.STATIC_URL):
return matched
# Strip off the fragment so a path-like fragment won't interfere.
url_path, fragment = urldefrag(url)
if url_path.startswith('/'):
# Otherwise the condition above would have returned prematurely.
assert url_path.startswith(settings.STATIC_URL)
target_name = url_path[len(settings.STATIC_URL):]
else:
# We're using the posixpath module to mix paths and URLs conveniently.
source_name = name if os.sep == '/' else name.replace(os.sep, '/')
target_name = posixpath.join(posixpath.dirname(source_name), url_path)
# Determine the hashed name of the target file with the storage backend.
hashed_url = self._url(
self._stored_name, unquote(target_name),
force=True, hashed_files=hashed_files,
)
# NOTE:
# The line below was commented out so that absolute urls are used instead of relative urls to make themed
# assets work correctly.
#
# The line is commented and not removed to make future django upgrade easier and show exactly what is
# changed in this method override
#
#transformed_url = '/'.join(url_path.split('/')[:-1] + hashed_url.split('/')[-1:])
transformed_url = hashed_url # This line was added.
# Restore the fragment that was stripped off earlier.
if fragment:
transformed_url += ('?#' if '?#' in url else '#') + fragment
# Return the hashed version to the file
return template % unquote(transformed_url)
return converter
# TODO: Remove Django 1.11 upgrade shim
# SHIM: This method switches the implementation of url_converter according
# to the Django version. After the 1.11 upgrade, do these things:
#
# 1. delete _url_converter__lt_111.
# 2. delete url_converter (below).
# 3. rename _url_converter__gte_111 to url_converter.
def url_converter(self, *args, **kwargs):
"""
An implementation selector for the url_converter method. This is in
place only for the Django 1.11 upgrade.
"""
if django.VERSION < (1, 11):
return self._url_converter__lt_111(*args, **kwargs)
else:
return self._url_converter__gte_111(*args, **kwargs)
class ThemePipelineMixin(PipelineMixin):
"""

View File

@@ -246,12 +246,7 @@ class Command(BaseCommand):
user_id, username, email, full_name, course_id, is_opted_in, pref_set_datetime = row
if pref_set_datetime:
# TODO: Remove Django 1.11 upgrade shim
# SHIM: pref_set_datetime.tzinfo should always be None here after the 1.11 upgrade
# As of Django 1.9 datetimes returned from raw sql queries are no longer coerced to being tz aware
# so we correct for that here.
if pref_set_datetime.tzinfo is None or pref_set_datetime.tzinfo.utcoffset(pref_set_datetime) is None:
pref_set_datetime = timezone.make_aware(pref_set_datetime, timezone.utc)
pref_set_datetime = timezone.make_aware(pref_set_datetime, timezone.utc)
else:
pref_set_datetime = self.DEFAULT_DATETIME_STR