Merge pull request #33487 from dimagi/dm/repeater-refactors

Refactor repeaters code and tests

corehq/apps/accounting/tests/utils.py

@@ -47,7 +47,10 @@ def teardown_subscriptions(cls):
 
     @classmethod
     def teardown_subscription(cls, domain):
-        SubscriptionAdjustment.objects.all().delete()
-        Subscription.visible_and_suppressed_objects.all().delete()
-        cls.__subscriptions[domain].delete()
-        cls.__accounts[domain].delete()
+        try:
+            SubscriptionAdjustment.objects.all().delete()
+            Subscription.visible_and_suppressed_objects.all().delete()
+            cls.__subscriptions[domain].delete()
+            cls.__accounts[domain].delete()
+        finally:
+            Subscription._get_active_subscription_by_domain.clear(Subscription, domain)

corehq/apps/data_interfaces/tests/test_auto_case_updates.py

@@ -592,7 +592,8 @@ def test_location_filter_criteria_does_include_child_locations(self):
         from corehq.apps.domain.shortcuts import create_domain
         from corehq.apps.locations.models import LocationType, SQLLocation
 
-        create_domain(self.domain)
+        domain_obj = create_domain(self.domain)
+        self.addCleanup(domain_obj.delete)
 
         location_type_provice = LocationType(domain=self.domain, name='Province')
         location_type_provice.save()
@@ -1178,11 +1179,7 @@ def setUpClass(cls):
         super(CaseRuleEndToEndTests, cls).setUpClass()
         cls.domain_object = Domain(name=cls.domain)
         cls.domain_object.save()
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.domain_object.delete()
-        super(CaseRuleEndToEndTests, cls).tearDownClass()
+        cls.addClassCleanup(cls.domain_object.delete)
 
     def test_get_rules_from_domain(self):
         rule1 = _create_empty_rule(self.domain, case_type='person-1')

corehq/ex-submodules/dimagi/utils/couch/migration.py

@@ -150,18 +150,6 @@ def _migration_sync_to_sql(self, sql_object, save=True):
         if save:
             sql_object.save(sync_to_couch=False)
 
-    @classmethod
-    def _migration_bulk_sync_to_sql(cls, couch_docs, **kw):
-        sql_class = cls._migration_get_sql_model_class()
-        id_name = sql_class._migration_couch_id_name
-        new_sql_docs = []
-        for doc in couch_docs:
-            assert doc._id, doc
-            obj = sql_class(**{id_name: doc._id})
-            doc._migration_sync_to_sql(obj, save=False)
-            new_sql_docs.append(obj)
-        sql_class.objects.bulk_create(new_sql_docs, **kw)
-
     def _migration_sync_submodels_to_sql(self, sql_object):
         """Migrate submodels from the Couch model to the SQL model. This is called
         as part of ``_migration_sync_to_sql``"""
@@ -182,8 +170,7 @@ def _migration_do_sync(self):
         self._migration_sync_to_sql(sql_object)
         return sql_object
 
-    def save(self, *args, **kwargs):
-        sync_to_sql = kwargs.pop('sync_to_sql', True)
+    def save(self, *args, sync_to_sql=True, **kwargs):
         super(SyncCouchToSQLMixin, self).save(*args, **kwargs)
         if sync_to_sql:
             try:
@@ -197,8 +184,7 @@ def save(self, *args, **kwargs):
                     message='Could not sync %s SQL object from %s %s' % (sql_class_name,
                         couch_class_name, self._id))
 
-    def delete(self, *args, **kwargs):
-        sync_to_sql = kwargs.pop('sync_to_sql', True)
+    def delete(self, sync_to_sql=True, *args, **kwargs):
         if sync_to_sql:
             sql_object = self._migration_get_sql_object()
             if sql_object is not None:
@@ -236,7 +222,7 @@ def _migration_get_submodels(cls):
         Should return a list of SubModelSpec tuples, one for each SchemaListProperty
         in the couch class. Should be identical in the couch and sql mixins.
         """
-        return []
+        return cls._migration_get_couch_model_class()._migration_get_submodels()
 
     @classmethod
     def _migration_get_custom_sql_to_couch_functions(cls):
@@ -300,8 +286,7 @@ def _migration_do_sync(self):
         couch_object = self._migration_get_or_create_couch_object()
         self._migration_sync_to_couch(couch_object)
 
-    def save(self, *args, **kwargs):
-        sync_to_couch = kwargs.pop('sync_to_couch', True)
+    def save(self, *args, sync_to_couch=True, **kwargs):
         super(SyncSQLToCouchMixin, self).save(*args, **kwargs)
         if sync_to_couch and sync_to_couch_enabled(self.__class__):
             try:
@@ -315,8 +300,7 @@ def save(self, *args, **kwargs):
                     message='Could not sync %s Couch object from %s %s' % (couch_class_name,
                         sql_class_name, self.pk))
 
-    def delete(self, *args, **kwargs):
-        sync_to_couch = kwargs.pop('sync_to_couch', True)
+    def delete(self, *args, sync_to_couch=True, **kwargs):
         if sync_to_couch and sync_to_couch_enabled(self.__class__):
             couch_object = self._migration_get_couch_object()
             if couch_object is not None:

corehq/motech/repeaters/dbaccessors.py

@@ -183,9 +183,6 @@ def _iter_repeat_records_by_repeater(domain, repeater_id, chunk_size,
 
 
 def get_repeat_records_by_payload_id(domain, payload_id):
-    repeat_records = get_sql_repeat_records_by_payload_id(domain, payload_id)
-    if repeat_records:
-        return repeat_records
     return get_couch_repeat_records_by_payload_id(domain, payload_id)
 
 
@@ -261,14 +258,13 @@ def get_domains_that_have_repeat_records():
 @unit_testing_only
 def delete_all_repeat_records():
     from .models import RepeatRecord
-    results = RepeatRecord.get_db().view('repeaters/repeat_records', reduce=False).all()
-    for result in results:
-        try:
-            repeat_record = RepeatRecord.get(result['id'])
-        except Exception:
-            pass
-        else:
-            repeat_record.delete()
+    db = RepeatRecord.get_db()
+    results = db.view(
+        'repeaters/repeat_records_by_payload_id',
+        reduce=False,
+        include_docs=True,
+    ).all()
+    db.bulk_delete([r["doc"] for r in results], empty_on_delete=False)
 
 
 @unit_testing_only

corehq/motech/repeaters/expression/tests.py

@@ -25,7 +25,10 @@ def setUpClass(cls):
 
         cls.domain = 'test'
         cls.domain_obj = create_domain(cls.domain)
+        cls.addClassCleanup(clear_plan_version_cache)
+        cls.addClassCleanup(cls.domain_obj.delete)
         cls.setup_subscription(cls.domain, SoftwarePlanEdition.PRO)
+        cls.addClassCleanup(cls.teardown_subscriptions)
 
         cls.factory = CaseFactory(cls.domain)
 
@@ -81,15 +84,6 @@ def setUpClass(cls):
 
         cls.repeater.save()
 
-    @classmethod
-    def tearDownClass(cls):
-        cls.repeater.delete()
-        cls.connection.delete()
-        cls.teardown_subscriptions()
-        cls.domain_obj.delete()
-        clear_plan_version_cache()
-        super().tearDownClass()
-
     def tearDown(self):
         delete_all_repeat_records()
 

corehq/motech/repeaters/models.py

@@ -116,7 +116,6 @@
 from corehq.privileges import DATA_FORWARDING, ZAPIER_INTEGRATION
 from corehq.util.metrics import metrics_counter
 from corehq.util.models import ForeignObject, foreign_init
-from corehq.util.quickcache import quickcache
 from corehq.util.urlvalidate.ip_resolver import CannotResolveHost
 from corehq.util.urlvalidate.urlvalidate import PossibleSSRFAttempt
 
@@ -936,7 +935,7 @@ def repeater(self):
 
     def is_repeater_deleted(self):
         try:
-            return Repeater.all_objects.get(repeater_id=self.repeater_id).is_deleted
+            return Repeater.all_objects.values_list("is_deleted", flat=True).get(repeater_id=self.repeater_id)
         except Repeater.DoesNotExist:
             return True
 
@@ -1446,15 +1445,8 @@ def is_response(duck):
     return hasattr(duck, 'status_code') and hasattr(duck, 'reason')
 
 
-@quickcache(['domain'], timeout=5 * 60)
 def are_repeat_records_migrated(domain) -> bool:
-    """
-    Returns True if ``domain`` has SQLRepeatRecords.
-
-    .. note:: Succeeded and cancelled RepeatRecords may not have been
-              migrated to SQLRepeatRecords.
-    """
-    return SQLRepeatRecord.objects.filter(domain=domain).exists()
+    return False
 
 
 def domain_can_forward(domain):

corehq/motech/repeaters/tests/test_data_registry_case_update_repeater.py

@@ -31,9 +31,12 @@ class DataRegistryCaseUpdateRepeaterTest(TestCase, TestXmlMixin, DomainSubscript
     def setUpClass(cls):
         super().setUpClass()
         cls.domain_obj = create_domain(cls.domain)
+        cls.addClassCleanup(clear_plan_version_cache)
+        cls.addClassCleanup(cls.domain_obj.delete)
 
         # DATA_FORWARDING is on PRO and above
         cls.setup_subscription(cls.domain, SoftwarePlanEdition.PRO)
+        cls.addClassCleanup(cls.teardown_subscriptions)
 
         cls.connx = ConnectionSettings.objects.create(
             domain=cls.domain,
@@ -63,6 +66,7 @@ def setUpClass(cls):
         ).slug
 
         cls.mobile_user = CommCareUser.create(cls.domain, "user1", "123", None, None, is_admin=True)
+        cls.addClassCleanup(cls.mobile_user.delete, None, None)
 
         cls.target_case_id_1 = uuid.uuid4().hex
         cls.target_case_id_2 = uuid.uuid4().hex
@@ -81,16 +85,6 @@ def setUpClass(cls):
     def tearDown(self):
         delete_all_repeat_records()
 
-    @classmethod
-    def tearDownClass(cls):
-        cls.repeater.delete()
-        cls.connx.delete()
-        cls.mobile_user.delete(None, None)
-        cls.teardown_subscriptions()
-        cls.domain_obj.delete()
-        clear_plan_version_cache()
-        super().tearDownClass()
-
     def test_update_cases(self):
         builder1 = (
             IntentCaseBuilder(self.registry_slug)

corehq/motech/repeaters/tests/test_dbaccessors.py

@@ -101,16 +101,11 @@ def setUpClass(cls):
             empty,
             other_id,
         ]
+        cls.addClassCleanup(RepeatRecord.bulk_delete, cls.records)
 
         for record in cls.records:
             record.save()
 
-    @classmethod
-    def tearDownClass(cls):
-        for record in cls.records:
-            record.delete()
-        super(TestRepeatRecordDBAccessors, cls).tearDownClass()
-
     def test_get_pending_repeat_record_count(self):
         count = get_pending_repeat_record_count(self.domain, self.repeater_id)
         self.assertEqual(count, 2)
@@ -199,11 +194,7 @@ def setUpClass(cls):
             RepeatRecord(domain='c'),
         ]
         RepeatRecord.bulk_save(cls.records)
-
-    @classmethod
-    def tearDownClass(cls):
-        RepeatRecord.bulk_delete(cls.records)
-        super(TestOtherDBAccessors, cls).tearDownClass()
+        cls.addClassCleanup(RepeatRecord.bulk_delete, cls.records)
 
     def test_get_domains_that_have_repeat_records(self):
         self.assertEqual(get_domains_that_have_repeat_records(), ['a', 'b', 'c'])

corehq/motech/repeaters/tests/test_models.py

@@ -28,7 +28,6 @@
     FormRepeater,
     Repeater,
     RepeatRecord,
-    are_repeat_records_migrated,
     format_response,
     get_all_repeater_types,
     is_response,
@@ -400,22 +399,6 @@ def test_add_payload_exception_attempt(self):
         self.assertEqual(self.repeat_record.attempts[0].traceback, tb_str)
 
 
-class TestAreRepeatRecordsMigrated(RepeaterTestCase):
-
-    def setUp(self):
-        super().setUp()
-        are_repeat_records_migrated.clear(DOMAIN)
-
-    def test_no(self):
-        is_migrated = are_repeat_records_migrated(DOMAIN)
-        self.assertFalse(is_migrated)
-
-    def test_yes(self):
-        with make_repeat_record(self.repeater, RECORD_PENDING_STATE):
-            is_migrated = are_repeat_records_migrated(DOMAIN)
-        self.assertTrue(is_migrated)
-
-
 class TestConnectionSettingsUsedBy(TestCase):
 
     def setUp(self):

corehq/motech/repeaters/tests/test_repeater.py

@@ -114,16 +114,12 @@ def setUpClass(cls):
         )
 
         cls.domain_obj = create_domain(cls.domain)
+        cls.addClassCleanup(clear_plan_version_cache)
+        cls.addClassCleanup(cls.domain_obj.delete)
 
         # DATA_FORWARDING is on PRO and above
         cls.setup_subscription(cls.domain, SoftwarePlanEdition.PRO)
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.teardown_subscriptions()
-        cls.domain_obj.delete()
-        clear_plan_version_cache()
-        super().tearDownClass()
+        cls.addClassCleanup(cls.teardown_subscriptions)
 
     @classmethod
     def post_xml(cls, xml, domain_name):