Merge pull request #193 from opbeat/feature/django-2.0

fix Django 2.0 incompatibilities

.travis.yml

@@ -19,6 +19,7 @@ env:
     - WEBFRAMEWORK=django-1.9
     - WEBFRAMEWORK=django-1.10
     - WEBFRAMEWORK=django-1.11
+    - WEBFRAMEWORK=django-2.0
     - WEBFRAMEWORK=django-master
     - WEBFRAMEWORK=flask-0.10
     - WEBFRAMEWORK=flask-0.11
@@ -28,6 +29,8 @@ env:
   - RUN_SCRIPT="./travis/run_tests.sh"
 matrix:
   exclude:
+  - python: 2.7
+    env: WEBFRAMEWORK=django-2.0
   - python: 2.7
     env: WEBFRAMEWORK=django-master
   - python: 3.3
@@ -38,10 +41,14 @@ matrix:
     env: WEBFRAMEWORK=django-1.10
   - python: 3.3
     env: WEBFRAMEWORK=django-1.11
+  - python: 3.3
+    env: WEBFRAMEWORK=django-2.0
   - python: 3.3
     env: WEBFRAMEWORK=django-master
   - python: 3.4
     env: WEBFRAMEWORK=django-1.4
+  - python: 3.4
+    env: WEBFRAMEWORK=django-master
   - python: 3.5
     env: WEBFRAMEWORK=django-1.4
   - python: 3.5
@@ -58,6 +65,10 @@ matrix:
     env: WEBFRAMEWORK=django-1.6
   - python: 3.6
     env: WEBFRAMEWORK=django-1.7
+  - python: pypy
+    env: WEBFRAMEWORK=django-2.0
+  - python: pypy
+    env: WEBFRAMEWORK=django-master
   - python: nightly
     env: WEBFRAMEWORK=django-1.4
   - python: nightly

conftest.py

@@ -28,12 +28,17 @@ def pytest_configure(config):
     if settings is not None and not settings.configured:
         import django
 
+        if django.VERSION >= (2, 0):
+            middleware_settings_name = 'MIDDLEWARE'
+        else:
+            middleware_settings_name = 'MIDDLEWARE_CLASSES'
+
         # django-celery does not work well with Django 1.8+
         if django.VERSION < (1, 8):
             djcelery = ['djcelery']
         else:
             djcelery = []
-        settings.configure(
+        settings_dict = dict(
             DATABASES={
                 'default': {
                     'ENGINE': 'django.db.backends.sqlite3',
@@ -91,6 +96,12 @@ def pytest_configure(config):
             ]
 
         )
+        settings_dict[middleware_settings_name] = [
+            'django.contrib.sessions.middleware.SessionMiddleware',
+            'django.contrib.auth.middleware.AuthenticationMiddleware',
+            'django.contrib.messages.middleware.MessageMiddleware',
+        ]
+        settings.configure(**settings_dict)
         if hasattr(django, 'setup'):
             django.setup()
         if django.VERSION < (1, 8):

opbeat/contrib/django/client.py

@@ -64,27 +64,27 @@ def __init__(self, **kwargs):
         super(DjangoClient, self).__init__(**kwargs)
 
     def get_user_info(self, request):
+        user_info = {}
+
+        if not hasattr(request, 'user'):
+            return user_info
         try:
-            if request.user.is_authenticated():
-                user_info = {
-                    'is_authenticated': True,
-                    'id': request.user.pk,
-                }
-                if hasattr(request.user, 'get_username'):
-                    user_info['username'] = request.user.get_username()
-                elif hasattr(request.user, 'username'):
-                    user_info['username'] = request.user.username
+            user = request.user
+            if hasattr(user, 'is_authenticated'):
+                if callable(user.is_authenticated):
+                    user_info['is_authenticated'] = user.is_authenticated()
                 else:
-                    # this only happens if the project uses custom user models, but
-                    # doesn't correctly inherit from AbstractBaseUser
-                    user_info['username'] = ''
-
-                if hasattr(request.user, 'email'):
-                    user_info['email'] = request.user.email
-            else:
-                user_info = {
-                    'is_authenticated': False,
-                }
+                    user_info['is_authenticated'] = bool(user.is_authenticated)
+            if hasattr(user, 'id'):
+                user_info['id'] = user.id
+            if hasattr(user, 'get_username'):
+                username = user.get_username()
+            elif hasattr(user, 'username'):
+                username = user.username
+            if username:
+                user_info['username'] = username
+            if hasattr(user, 'email'):
+                user_info['email'] = user.email
         except DatabaseError:
             # If the connection is closed or similar, we'll just skip this
             return {}

opbeat/contrib/django/middleware/__init__.py

@@ -14,6 +14,7 @@
 import logging
 import threading
 
+import django
 from django.conf import settings as django_settings
 
 from opbeat.contrib.django.models import client, get_client
@@ -33,6 +34,11 @@ class MiddlewareMixin(object):
         pass
 
 
+if django.VERSION >= (2, 0):
+    middleware_settings_name = 'MIDDLEWARE'
+else:
+    middleware_settings_name = 'MIDDLEWARE_CLASSES'
+
 def _is_ignorable_404(uri):
     """
     Returns True if the given request *shouldn't* notify the site managers.
@@ -122,8 +128,8 @@ def __init__(self, *args, **kwargs):
                     OpbeatAPMMiddleware._opbeat_instrumented = True
 
     def instrument_middlewares(self):
-        if getattr(django_settings, 'MIDDLEWARE_CLASSES', None):
-            for middleware_path in django_settings.MIDDLEWARE_CLASSES:
+        if getattr(django_settings, middleware_settings_name, None):
+            for middleware_path in getattr(django_settings, middleware_settings_name):
                 module_path, class_name = middleware_path.rsplit('.', 1)
                 try:
                     module = import_module(module_path)

test_requirements/requirements-base.txt

@@ -1,5 +1,5 @@
 py==1.4.30
-pytest==2.8.0
+pytest==3.0.6
 pytest-capturelog==0.7
 pytest-django==2.8.0
 pytest-benchmark==2.5.0

test_requirements/requirements-django-2.0.txt

@@ -0,0 +1,3 @@
+Django>=2.0,<2.1
+django-celery
+-r requirements-base.txt

tests/contrib/django/django_tests.py

@@ -14,7 +14,6 @@
 from django.core.handlers.wsgi import WSGIRequest
 from django.core.management import call_command
 from django.core.signals import got_request_exception
-from django.core.urlresolvers import reverse
 from django.db import DatabaseError
 from django.http import QueryDict
 from django.template import TemplateSyntaxError
@@ -37,6 +36,12 @@
 from opbeat.utils import six
 from opbeat.utils.lru import LRUCache
 
+try:
+    # Django 1.10+
+    from django.urls import reverse
+except ImportError:
+    from django.core.urlresolvers import reverse
+
 try:
     from celery.tests.utils import with_eager_tasks
     has_with_eager_tasks = True
@@ -48,6 +53,12 @@
 settings.OPBEAT = {'CLIENT': 'tests.contrib.django.django_tests.TempStoreClient'}
 
 
+if django.VERSION >= (2, 0):
+    middleware_settings_name = 'MIDDLEWARE'
+else:
+    middleware_settings_name = 'MIDDLEWARE_CLASSES'
+
+
 class MockClientHandler(_TestClientHandler):
     def __call__(self, environ, start_response=[]):
         # this pretends doesnt require start_response
@@ -246,7 +257,7 @@ def test_user_info_with_non_django_auth(self):
             self.assertFalse('user' in event)
 
     def test_request_middleware_exception(self):
-        with self.settings(MIDDLEWARE_CLASSES=['tests.contrib.django.testapp.middleware.BrokenRequestMiddleware']):
+        with self.settings(**{middleware_settings_name: ['tests.contrib.django.testapp.middleware.BrokenRequestMiddleware']}):
             self.assertRaises(ImportError, self.client.get, reverse('opbeat-raise-exc'))
 
             self.assertEquals(len(self.opbeat.events), 1)
@@ -263,7 +274,7 @@ def test_request_middleware_exception(self):
     def test_response_middlware_exception(self):
         if django.VERSION[:2] < (1, 3):
             return
-        with self.settings(MIDDLEWARE_CLASSES=['tests.contrib.django.testapp.middleware.BrokenResponseMiddleware']):
+        with self.settings(**{middleware_settings_name: ['tests.contrib.django.testapp.middleware.BrokenResponseMiddleware']}):
             self.assertRaises(ImportError, self.client.get, reverse('opbeat-no-error'))
 
             self.assertEquals(len(self.opbeat.events), 1)
@@ -282,7 +293,8 @@ def test_broken_500_handler_with_middleware(self):
             client = _TestClient(REMOTE_ADDR='127.0.0.1')
             client.handler = MockOpbeatMiddleware(MockClientHandler())
 
-            self.assertRaises(Exception, client.get, reverse('opbeat-raise-exc'))
+            with override_settings(**{middleware_settings_name: []}):
+                self.assertRaises(Exception, client.get, reverse('opbeat-raise-exc'))
 
             self.assertEquals(len(self.opbeat.events), 2)
             event = self.opbeat.events.pop(0)
@@ -306,7 +318,7 @@ def test_broken_500_handler_with_middleware(self):
             self.assertEquals(event['culprit'], 'tests.contrib.django.testapp.urls.handler500')
 
     def test_view_middleware_exception(self):
-        with self.settings(MIDDLEWARE_CLASSES=['tests.contrib.django.testapp.middleware.BrokenViewMiddleware']):
+        with self.settings(**{middleware_settings_name: ['tests.contrib.django.testapp.middleware.BrokenViewMiddleware']}):
             self.assertRaises(ImportError, self.client.get, reverse('opbeat-raise-exc'))
 
             self.assertEquals(len(self.opbeat.events), 1)
@@ -397,7 +409,7 @@ def test_record_none_exc_info(self):
         self.assertEquals(event['param_message'], {'message': 'test','params':()})
 
     def test_404_middleware(self):
-        with self.settings(MIDDLEWARE_CLASSES=['opbeat.contrib.django.middleware.Opbeat404CatchMiddleware']):
+        with self.settings(**{middleware_settings_name: ['opbeat.contrib.django.middleware.Opbeat404CatchMiddleware']}):
             resp = self.client.get('/non-existant-page')
             self.assertEquals(resp.status_code, 404)
 
@@ -435,6 +447,8 @@ def test_404_new_style_middleware(self):
             self.assertEquals(http['query_string'], '')
             self.assertEquals(http['data'], None)
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_404_middleware_with_debug(self):
         with self.settings(
                 MIDDLEWARE_CLASSES=[
@@ -460,6 +474,8 @@ def test_404_new_style_middleware_with_debug(self):
             self.assertEquals(resp.status_code, 404)
             self.assertEquals(len(self.opbeat.events), 0)
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_response_error_id_middleware(self):
         with self.settings(MIDDLEWARE_CLASSES=[
                 'opbeat.contrib.django.middleware.OpbeatResponseErrorIdMiddleware',
@@ -630,6 +646,8 @@ def test_request_capture(self):
         self.assertTrue('SERVER_PORT' in env, env.keys())
         self.assertEquals(env['SERVER_PORT'], '80')
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_transaction_metrics(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
         with self.settings(MIDDLEWARE_CLASSES=[
@@ -670,6 +688,8 @@ def test_transaction_metrics_new_style_middleware(self):
             self.assertEqual(timing['result'],
                              200)
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_request_metrics_301_append_slash(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
 
@@ -725,6 +745,8 @@ def test_request_metrics_301_append_slash_new_style_middleware(self):
             )
         )
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_request_metrics_301_prepend_www(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
 
@@ -772,6 +794,8 @@ def test_request_metrics_301_prepend_www_new_style_middleware(self):
             'GET django.middleware.common.CommonMiddleware.process_request'
         )
 
+    @pytest.mark.skipif(django.VERSION >= (2, 0),
+                        reason='new-style middlewares')
     def test_request_metrics_contrib_redirect(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
 
@@ -843,12 +867,12 @@ def test_ASYNC_config_raises_deprecation(self):
 
     def test_request_metrics_name_override(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
-        with self.settings(
-            MIDDLEWARE_CLASSES=[
+        with self.settings(**{
+            middleware_settings_name: [
                 'opbeat.contrib.django.middleware.OpbeatAPMMiddleware',
                 'tests.contrib.django.testapp.middleware.MetricsNameOverrideMiddleware',
             ]
-        ):
+        }):
             self.client.get(reverse('opbeat-no-error'))
         timed_requests, _traces = self.opbeat.instrumentation_store.get_all()
         timing = timed_requests[0]
@@ -859,11 +883,11 @@ def test_request_metrics_name_override(self):
 
     def test_request_metrics_404_resolve_error(self):
         self.opbeat.instrumentation_store.get_all()  # clear the store
-        with self.settings(
-                MIDDLEWARE_CLASSES=[
-                    'opbeat.contrib.django.middleware.OpbeatAPMMiddleware',
-                ]
-        ):
+        with self.settings(**{
+            middleware_settings_name: [
+                'opbeat.contrib.django.middleware.OpbeatAPMMiddleware',
+            ]
+        }):
             self.client.get('/i-dont-exist/')
         timed_requests, _traces = self.opbeat.instrumentation_store.get_all()
         timing = timed_requests[0]
@@ -1050,13 +1074,13 @@ def test_stacktraces_have_templates():
         should_collect.return_value = False
         TEMPLATES_copy = deepcopy(settings.TEMPLATES)
         TEMPLATES_copy[0]['OPTIONS']['debug'] = TEMPLATE_DEBUG
-        with override_settings(
-            MIDDLEWARE_CLASSES=[
+        with override_settings(**{
+            middleware_settings_name: [
                 'opbeat.contrib.django.middleware.OpbeatAPMMiddleware'
             ],
-            TEMPLATE_DEBUG=TEMPLATE_DEBUG,
-            TEMPLATES=TEMPLATES_copy
-        ):
+            'TEMPLATE_DEBUG': TEMPLATE_DEBUG,
+            'TEMPLATES': TEMPLATES_copy
+        }):
             resp = client.get(reverse("render-heavy-template"))
     assert resp.status_code == 200
 
@@ -1094,8 +1118,8 @@ def test_stacktrace_filtered_for_opbeat():
     with mock.patch(
             "opbeat.traces.RequestsStore.should_collect") as should_collect:
         should_collect.return_value = False
-        with override_settings(MIDDLEWARE_CLASSES=[
-            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']):
+        with override_settings(**{middleware_settings_name:[
+            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']}):
             resp = client.get(reverse("render-heavy-template"))
     assert resp.status_code == 200
 
@@ -1121,8 +1145,8 @@ def test_perf_template_render(benchmark):
     instrumentation.control.instrument()
     with mock.patch("opbeat.traces.RequestsStore.should_collect") as should_collect:
         should_collect.return_value = False
-        with override_settings(MIDDLEWARE_CLASSES=[
-            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']):
+        with override_settings(**{middleware_settings_name: [
+            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']}):
             resp = benchmark(client_get, client, reverse("render-heavy-template"))
     assert resp.status_code == 200
 
@@ -1161,8 +1185,8 @@ def test_perf_database_render(benchmark):
     with mock.patch("opbeat.traces.RequestsStore.should_collect") as should_collect:
         should_collect.return_value = False
 
-        with override_settings(MIDDLEWARE_CLASSES=[
-            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']):
+        with override_settings(**{middleware_settings_name: [
+            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']}):
             resp = benchmark(client_get, client, reverse("render-user-template"))
         assert resp.status_code == 200
 
@@ -1201,8 +1225,8 @@ def test_perf_transaction_with_collection(benchmark):
 
         client = _TestClient()
 
-        with override_settings(MIDDLEWARE_CLASSES=[
-            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']):
+        with override_settings(**{middleware_settings_name: [
+            'opbeat.contrib.django.middleware.OpbeatAPMMiddleware']}):
 
             for i in range(10):
                 resp = client_get(client, reverse("render-user-template"))