Merge pull request #53 from fgallina/django-16-db-patches-fixes

Django 16 db patches fixes

.travis.yml

@@ -3,6 +3,6 @@ python:
  - "2.6"
  - "2.7"
 install: pip install -r requirements.txt -r optional.txt --use-mirrors
-script: nosetests
+script: DJANGO_SETTINGS_MODULE='django_statsd.test_settings' nosetests
 notifications:
   irc: "irc.mozilla.org#amo-bots"

django_statsd/patches/__init__.py

@@ -1,11 +1,7 @@
 from django.conf import settings
 from django.utils.importlib import import_module
 
-# Workaround for tests.
+patches = getattr(settings, 'STATSD_PATCHES', [])
-try:
-    patches = getattr(settings, 'STATSD_PATCHES', [])
-except ImportError:
-    patches = []
 
 for patch in patches:
     import_module(patch).patch()

django_statsd/patches/db.py

@@ -7,6 +7,7 @@ from django_statsd.clients import statsd
 def key(db, attr):
     return 'db.%s.%s.%s' % (db.client.executable_name, db.alias, attr)
 
+
 def pre_django_1_6_cursorwrapper_getattr(self, attr):
     """
     The CursorWrapper is a pretty small wrapper around the cursor.
@@ -22,33 +23,39 @@ def pre_django_1_6_cursorwrapper_getattr(self, attr):
             return wrap(getattr(self.cursor, attr), key(self.db, attr))
         return getattr(self.cursor, attr)
 
+
+def patched_execute(orig_execute, self, *args, **kwargs):
+    with statsd.timer(key(self.db, 'execute')):
+        return orig_execute(self, *args, **kwargs)
+
+
+def patched_executemany(orig_executemany, self, *args, **kwargs):
+    with statsd.timer(key(self.db, 'executemany')):
+        return orig_executemany(self, *args, **kwargs)
+
+
+def patched_callproc(orig_callproc, self, *args, **kwargs):
+    with statsd.timer(key(self.db, 'callproc')):
+        return orig_callproc(self, *args, **kwargs)
+
+
 def patch():
     """
-    The CursorWrapper is a pretty small wrapper around the cursor.
+    The CursorWrapper is a pretty small wrapper around the cursor.  If
-    If you are NOT in debug mode, this is the wrapper that's used.
+    you are NOT in debug mode, this is the wrapper that's used.  Sadly
-    Sadly if it's in debug mode, we get a different wrapper for version earlier than 1.6.
+    if it's in debug mode, we get a different wrapper for version
+    earlier than 1.6.
     """
 
-    def execute(orig_execute, self, *args, **kwargs):
-        with statsd.timer(key(self.db, 'execute')):
-            return orig_execute(self, *args, **kwargs)
-
-    def executemany(orig_executemany, self, *args, **kwargs):
-        with statsd.timer(key(self.db, 'executemany')):
-            return orig_executemany(self, *args, **kwargs)
-
-    def callproc(orig_callproc, self, *args, **kwargs):
-        with statsd.timer(key(self.db, 'callproc')):
-            return orig_callproc(self, *args, **kwargs)
-
     if django.VERSION > (1, 6):
         # In 1.6+ util.CursorDebugWrapper just makes calls to CursorWrapper
         # As such, we only need to instrument CursorWrapper.
         # Instrumenting both will result in duplicated metrics
-        patch_method(util.CursorWrapper, 'execute')(execute)
+        patch_method(util.CursorWrapper, 'execute')(patched_execute)
-        patch_method(util.CursorWrapper, 'executemany')(executemany)
+        patch_method(util.CursorWrapper, 'executemany')(patched_executemany)
-        patch_method(util.CursorWrapper, 'callproc')(callproc)
+        patch_method(util.CursorWrapper, 'callproc')(patched_callproc)
     else:
         util.CursorWrapper.__getattr__ = pre_django_1_6_cursorwrapper_getattr
-        patch_method(util.CursorDebugWrapper, 'execute')(execute)
+        patch_method(util.CursorDebugWrapper, 'execute')(patched_execute)
-        patch_method(util.CursorDebugWrapper, 'executemany')(executemany)
+        patch_method(
+            util.CursorDebugWrapper, 'executemany')(patched_executemany)

django_statsd/test_settings.py

@@ -0,0 +1,13 @@
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': 'mydatabase'
+    }
+}
+
+ROOT_URLCONF = ''
+STATSD_CLIENT = 'django_statsd.clients.null'
+STATSD_PREFIX = None
+METLOG = None
+
+SECRET_KEY = 'secret'

django_statsd/tests.py

@@ -5,23 +5,9 @@ import sys
 from django.conf import settings
 from nose.exc import SkipTest
 from nose import tools as nose_tools
+from unittest2 import skipUnless
 
-minimal = {
+from django import VERSION
-    'DATABASES': {
-        'default': {
-            'ENGINE': 'django.db.backends.sqlite3',
-            'NAME': 'mydatabase'
-        }
-    },
-    'ROOT_URLCONF': '',
-    'STATSD_CLIENT': 'django_statsd.clients.null',
-    'STATSD_PREFIX': None,
-    'METLOG': None
-}
-
-if not settings.configured:
-    settings.configure(**minimal)
-
 from django.core.urlresolvers import reverse
 from django.http import HttpResponse, HttpResponseForbidden
 from django.test import TestCase
@@ -31,7 +17,13 @@ from django.utils import unittest
 
 import mock
 from nose.tools import eq_
-from django_statsd.clients import get_client
+from django_statsd.clients import get_client, statsd
+from django_statsd.patches import utils
+from django_statsd.patches.db import (
+    patched_callproc,
+    patched_execute,
+    patched_executemany,
+)
 from django_statsd import middleware
 
 cfg = {
@@ -261,7 +253,7 @@ class TestMetlogClient(TestCase):
                            STATSD_CLIENT='django_statsd.clients.moz_metlog'):
             client = get_client()
             client.incr('foo', 2)
-            
+
     def test_metlog_prefixes(self):
         metlog = self._create_client()
 
@@ -407,3 +399,159 @@ class TestErrorLog(TestCase):
     def test_not_emit(self, incr):
         self.log.error('blargh!')
         assert not incr.called
+
+
+class TestPatchMethod(TestCase):
+
+    def setUp(self):
+        super(TestPatchMethod, self).setUp()
+
+        class DummyClass(object):
+
+            def sumargs(self, a, b, c=3, d=4):
+                return a + b + c + d
+
+            def badfn(self, a, b=2):
+                raise ValueError
+
+        self.cls = DummyClass
+
+    def test_late_patching(self):
+        """
+        Objects created before patching should get patched as well.
+        """
+        def patch_fn(original_fn, self, *args, **kwargs):
+            return original_fn(self, *args, **kwargs) + 10
+
+        obj = self.cls()
+        self.assertEqual(obj.sumargs(1, 2, 3, 4), 10)
+        utils.patch_method(self.cls, 'sumargs')(patch_fn)
+        self.assertEqual(obj.sumargs(1, 2, 3, 4), 20)
+
+    def test_doesnt_call_original_implicitly(self):
+        """
+        Original fn must be called explicitly from patched to be
+        executed.
+        """
+        def patch_fn(original_fn, self, *args, **kwargs):
+            return 10
+
+        with self.assertRaises(ValueError):
+            obj = self.cls()
+            obj.badfn(1, 2)
+
+        utils.patch_method(self.cls, 'badfn')(patch_fn)
+        self.assertEqual(obj.badfn(1, 2), 10)
+
+    def test_args_kwargs_are_honored(self):
+        """
+        Args and kwargs must be honored between calls from the patched to
+        the original version.
+        """
+        def patch_fn(original_fn, self, *args, **kwargs):
+            return original_fn(self, *args, **kwargs)
+
+        utils.patch_method(self.cls, 'sumargs')(patch_fn)
+        obj = self.cls()
+        self.assertEqual(obj.sumargs(1, 2), 10)
+        self.assertEqual(obj.sumargs(1, 1, d=1), 6)
+        self.assertEqual(obj.sumargs(1, 1, 1, 1), 4)
+
+    def test_patched_fn_can_receive_arbitrary_arguments(self):
+        """
+        Args and kwargs can be received arbitrarily with no contraints on
+        the patched fn, even if the original_fn had a fixed set of
+        allowed args and kwargs.
+        """
+        def patch_fn(original_fn, self, *args, **kwargs):
+            return args, kwargs
+
+        utils.patch_method(self.cls, 'badfn')(patch_fn)
+        obj = self.cls()
+        self.assertEqual(obj.badfn(1, d=2), ((1,), {'d': 2}))
+        self.assertEqual(obj.badfn(1, d=2), ((1,), {'d': 2}))
+        self.assertEqual(obj.badfn(1, 2, c=1, d=2), ((1, 2), {'c': 1, 'd': 2}))
+
+
+class TestCursorWrapperPatching(TestCase):
+
+    def test_patched_callproc_calls_timer(self):
+        with mock.patch.object(statsd, 'timer') as timer:
+            db = mock.Mock(executable_name='name', alias='alias')
+            instance = mock.Mock(db=db)
+            patched_callproc(lambda *args, **kwargs: None, instance)
+            self.assertEqual(timer.call_count, 1)
+
+    def test_patched_execute_calls_timer(self):
+        with mock.patch.object(statsd, 'timer') as timer:
+            db = mock.Mock(executable_name='name', alias='alias')
+            instance = mock.Mock(db=db)
+            patched_execute(lambda *args, **kwargs: None, instance)
+            self.assertEqual(timer.call_count, 1)
+
+    def test_patched_executemany_calls_timer(self):
+        with mock.patch.object(statsd, 'timer') as timer:
+            db = mock.Mock(executable_name='name', alias='alias')
+            instance = mock.Mock(db=db)
+            patched_executemany(lambda *args, **kwargs: None, instance)
+            self.assertEqual(timer.call_count, 1)
+
+    @mock.patch(
+        'django_statsd.patches.db.pre_django_1_6_cursorwrapper_getattr')
+    @mock.patch('django_statsd.patches.db.patched_executemany')
+    @mock.patch('django_statsd.patches.db.patched_execute')
+    @mock.patch('django.db.backends.util.CursorDebugWrapper')
+    @skipUnless(VERSION < (1, 6, 0), "CursorWrapper Patching for Django<1.6")
+    def test_cursorwrapper_patching(self,
+                                    CursorDebugWrapper,
+                                    execute,
+                                    executemany,
+                                    _getattr):
+        try:
+            from django.db.backends import util
+
+            # We need to patch CursorWrapper like this because setting
+            # __getattr__ on Mock instances raises AttributeError.
+            class CursorWrapper(object):
+                pass
+
+            _CursorWrapper = util.CursorWrapper
+            util.CursorWrapper = CursorWrapper
+
+            from django_statsd.patches.db import patch
+            execute.__name__ = 'execute'
+            executemany.__name__ = 'executemany'
+            _getattr.__name__ = '_getattr'
+            execute.return_value = 'execute'
+            executemany.return_value = 'executemany'
+            _getattr.return_value = 'getattr'
+            patch()
+
+            self.assertEqual(CursorDebugWrapper.execute(), 'execute')
+            self.assertEqual(CursorDebugWrapper.executemany(), 'executemany')
+            self.assertEqual(CursorWrapper.__getattr__(), 'getattr')
+        finally:
+            util.CursorWrapper = _CursorWrapper
+
+    @mock.patch('django_statsd.patches.db.patched_callproc')
+    @mock.patch('django_statsd.patches.db.patched_executemany')
+    @mock.patch('django_statsd.patches.db.patched_execute')
+    @mock.patch('django.db.backends.util.CursorWrapper')
+    @skipUnless(VERSION >= (1, 6, 0), "CursorWrapper Patching for Django>=1.6")
+    def test_cursorwrapper_patching16(self,
+                                      CursorWrapper,
+                                      execute,
+                                      executemany,
+                                      callproc):
+        from django_statsd.patches.db import patch
+        execute.__name__ = 'execute'
+        executemany.__name__ = 'executemany'
+        callproc.__name__ = 'callproc'
+        execute.return_value = 'execute'
+        executemany.return_value = 'executemany'
+        callproc.return_value = 'callproc'
+        patch()
+
+        self.assertEqual(CursorWrapper.execute(), 'execute')
+        self.assertEqual(CursorWrapper.executemany(), 'executemany')
+        self.assertEqual(CursorWrapper.callproc(), 'callproc')

docs/index.rst

@@ -308,6 +308,13 @@ do this by adding in the handler. For example in your logging configuration::
         },
     }
 
+Testing
+=======
+
+You can run tests with the following command:
+
+    DJANGO_SETTINGS_MODULE='django_statsd.test_settings' nosetests
+
 Nose
 ====
 

requirements.txt

@@ -1,4 +1,5 @@
 mock
 nose
+unittest2
 statsd==1.0.0
 django<1.5