Merge pull request #14 from waveaccounting/DLF-344-fix-participate

[JIRA](DLF-344) Fix participate() so it doesn't create duplicates in different buckets

djsixpack/__init__.py

@@ -1 +1 @@
-__version__ = "1.1.4-wave"
+__version__ = "1.1.5-wave"

djsixpack/djsixpack.py

@@ -60,6 +60,12 @@ def _get_experiment_name(self):
         name = RE_ALL_CAP.sub(r'\1_\2', s1).lower()
         return RE_TEST_NAME.sub('', name)
 
+    def _delete_duplicates(self, experiment_name):
+        # Check for duplicate records and delete them.
+        duplicates = SixpackParticipant.objects.filter(unique_attr=self.client_id, experiment_name=experiment_name).order_by('id')
+        for dup in duplicates[1:]:
+            dup.delete()
+
     def get_client_id(self, instance):
         if not self.unique_attr:
             raise ValueError('Need a unique_attr to compute the client ID')
@@ -111,13 +117,17 @@ def participate(self, force=None, user_agent=None, ip_address=None, prefetch=Fal
             chosen_alternative = resp['alternative']['name']
         finally:
             if self.local and chosen_alternative:
-                try:
-                    SixpackParticipant.objects.get_or_create(unique_attr=self.client_id, experiment_name=experiment_name, bucket=chosen_alternative)
-                except SixpackParticipant.MultipleObjectsReturned:
-                    # clean up duplicate entries
-                    duplicates = SixpackParticipant.objects.filter(unique_attr=self.client_id, experiment_name=experiment_name, bucket=chosen_alternative)
-                    for dup in duplicates[1:]:
-                        dup.delete()
+
+                # Record the bucket in the database if one doesn't already exist.
+                if not SixpackParticipant.objects.filter(unique_attr=self.client_id, experiment_name=experiment_name).exists():
+                    try:
+                        SixpackParticipant.objects.get_or_create(unique_attr=self.client_id, experiment_name=experiment_name, bucket=chosen_alternative)
+                    except SixpackParticipant.MultipleObjectsReturned:
+                        self._delete_duplicates(experiment_name)
+
+                # Scan and remove duplicates.
+                else:
+                    self._delete_duplicates(experiment_name)
 
         return chosen_alternative
 

djsixpack/tests/class_tests.py

@@ -193,6 +193,37 @@ class DefaultTest(SixpackTest):
             [call('default')]
         )
 
+    def test_participate_is_called_twice_for_local_test(self):
+        mock_user = Mock(pk=10)
+
+        class DefaultTest(SixpackTest):
+            alternatives = ('FIRST', 'SECOND')
+            local = True
+
+        class MockSession1(object):
+            def participate(self, experiment_name, alternatives, force, prefetch, bucket):
+                return {
+                    'alternative': {'name': 'SECOND'}
+                }
+        mock_session1 = MockSession1()
+
+        class MockSession2(object):
+            def participate(self, experiment_name, alternatives, force, prefetch, bucket):
+                return {
+                    'alternative': {'name': 'FIRST'}
+                }
+        mock_session2 = MockSession2()
+
+        with patch('djsixpack.djsixpack.sixpack'):
+            with self.settings(SIXPACK_HOST=None):
+                expt = DefaultTest(mock_user)
+                with patch.object(SixpackTest, '_get_session', return_value=mock_session1):
+                    expt.participate(force='SECOND', bucket='SECOND')
+                with patch.object(SixpackTest, '_get_session', return_value=mock_session2):
+                    expt.participate(force='FIRST', bucket='FIRST')
+
+        self.assertEquals(SixpackParticipant.objects.all().count(), 1)
+
 
 class ExperimentNameTest(TestCase):
 

setup.py

@@ -15,7 +15,7 @@
     description='A django-friendly wrapper for sixpack-py',
     long_description=open('README.rst').read(),
     install_requires=install_reqs,
-    tests_require=install_reqs + ["mock==1.0.1"],
+    tests_require=install_reqs + ["mock==1.0.1", "factory-boy==1.1.3"],
     test_suite='runtests.runtests',
     classifiers=(
         'Development Status :: 5 - Production/Stable',