Do not create export register when Instance is saved (#2760)

* do not create register when Instance is saved

creating a register if it does not exist when an Instance is saved resulted in some performance problems. This change ensures the only time we create a new register if it does not exist is when a CSV export is triggred

* create export register when form is published

* fix failing tests

* fix failing test

* refactor tests

* refactor test

* refactor tests

* refactor tests

* refactor tests

* fix ModelSignal.connect() missing 1 required positional argument: 'receiver'

* reconnect signal tearDown

* refactor test

* disable signal in setUp

onadata/apps/api/tests/viewsets/test_abstract_viewset.py

@@ -2,6 +2,7 @@
 """
 Test base class for API viewset tests.
 """
+
 import json
 import os
 import re
@@ -11,6 +12,7 @@
 from django.conf import settings
 from django.contrib.auth import authenticate, get_user_model
 from django.contrib.auth.models import Permission
+from django.db.models.signals import post_save
 from django.test import TestCase
 
 import requests
@@ -42,8 +44,10 @@
 from onadata.apps.logger.views import submission
 from onadata.apps.logger.xform_instance_parser import clean_and_parse_xml
 from onadata.apps.main import tests as main_tests
-from onadata.apps.main.tests.test_base import TestBase
 from onadata.apps.main.models import MetaData, UserProfile
+from onadata.apps.main.tests.test_base import TestBase
+from onadata.apps.viewer.models import DataDictionary
+from onadata.apps.viewer.models.data_dictionary import create_export_repeat_register
 from onadata.libs.serializers.project_serializer import ProjectSerializer
 from onadata.libs.utils.common_tools import merge_dicts
 
@@ -133,6 +137,20 @@ def setUp(self):
         self.factory = APIRequestFactory()
         self._login_user_and_profile()
         self.maxDiff = None
+        # Disable signals
+        post_save.disconnect(
+            sender=DataDictionary, dispatch_uid="create_export_repeat_register"
+        )
+
+    def tearDown(self):
+        # Enable signals
+        post_save.connect(
+            sender=DataDictionary,
+            dispatch_uid="create_export_repeat_register",
+            receiver=create_export_repeat_register,
+        )
+
+        TestCase.tearDown(self)
 
     def user_profile_data(self):
         """Returns the user profile python object."""

onadata/apps/logger/tests/models/test_instance.py

@@ -1228,8 +1228,7 @@ def test_register_repeats(self):
         |         | form_title  | form_id         |                     |
         |         | Births      | births          |                     |
         """
-        self._publish_markdown(md, self.user, project)
-        xform = XForm.objects.all().order_by("-pk").first()
+        xform = self._publish_markdown(md, self.user, project)
         xml = (
             '<?xml version="1.0" encoding="UTF-8"?>'
             '<data xmlns:jr="http://openrosa.org/javarosa" xmlns:orx='

onadata/apps/main/tests/test_csv_export.py

@@ -2,17 +2,19 @@
 """
 Test CSV Exports
 """
+
 import csv
 import os
 
 from django.core.files.storage import get_storage_class
+from django.db.models.signals import post_save
 from django.utils.dateparse import parse_datetime
 
-from onadata.apps.viewer.models.data_dictionary import DataDictionary
-from onadata.apps.viewer.models.export import Export
 from onadata.apps.logger.models import XForm
-from onadata.libs.utils.export_tools import generate_export
 from onadata.apps.main.tests.test_base import TestBase
+from onadata.apps.viewer.models import DataDictionary, Export
+from onadata.apps.viewer.models.data_dictionary import create_export_repeat_register
+from onadata.libs.utils.export_tools import generate_export
 
 
 class TestCsvExport(TestBase):
@@ -23,150 +25,161 @@ class TestCsvExport(TestBase):
     def setUp(self):
         self._create_user_and_login()
 
-        self.fixture_dir = os.path.join(
-            self.this_directory, 'fixtures', 'csv_export')
-        self._submission_time = parse_datetime('2013-02-18 15:54:01Z')
+        self.fixture_dir = os.path.join(self.this_directory, "fixtures", "csv_export")
+        self._submission_time = parse_datetime("2013-02-18 15:54:01Z")
         self.options = {"extension": "csv"}
         self.xform = None
+        # Disable signals
+        post_save.disconnect(
+            sender=DataDictionary, dispatch_uid="create_export_repeat_register"
+        )
+
+    def tearDown(self):
+        # Reconnect signals
+        post_save.connect(
+            sender=DataDictionary,
+            dispatch_uid="create_export_repeat_register",
+            receiver=create_export_repeat_register,
+        )
+
+        super().tearDown()
 
     def test_csv_export_output(self):
         """
         Test CSV export output
         """
-        path = os.path.join(self.fixture_dir, 'tutorial_w_repeats.xlsx')
+        path = os.path.join(self.fixture_dir, "tutorial_w_repeats.xlsx")
         self._publish_xls_file_and_set_xform(path)
-        path = os.path.join(self.fixture_dir, 'tutorial_w_repeats.xml')
-        self._make_submission(
-            path, forced_submission_time=self._submission_time)
+        path = os.path.join(self.fixture_dir, "tutorial_w_repeats.xml")
+        self._make_submission(path, forced_submission_time=self._submission_time)
         # test csv
 
-        export = generate_export(
-            Export.CSV_EXPORT,
-            self.xform,
-            None,
-            self.options)
+        export = generate_export(Export.CSV_EXPORT, self.xform, None, self.options)
         storage = get_storage_class()()
         self.assertTrue(storage.exists(export.filepath))
         path, ext = os.path.splitext(export.filename)
-        self.assertEqual(ext, '.csv')
-        test_file_path = os.path.join(
-            self.fixture_dir, 'tutorial_w_repeats.csv')
-        with storage.open(export.filepath, 'r') as csv_file:
+        self.assertEqual(ext, ".csv")
+        test_file_path = os.path.join(self.fixture_dir, "tutorial_w_repeats.csv")
+        with storage.open(export.filepath, "r") as csv_file:
             self._test_csv_files(csv_file, test_file_path)
 
     def test_csv_nested_repeat_output(self):
         """
         Test CSV export with nested repeats
         """
-        path = os.path.join(self.fixture_dir, 'double_repeat.xlsx')
+        path = os.path.join(self.fixture_dir, "double_repeat.xlsx")
         self._publish_xls_file(path)
-        self.xform = XForm.objects.get(id_string='double_repeat')
-        path = os.path.join(self.fixture_dir, 'instance.xml')
-        self._make_submission(
-            path, forced_submission_time=self._submission_time)
+        self.xform = XForm.objects.get(id_string="double_repeat")
+        path = os.path.join(self.fixture_dir, "instance.xml")
+        self._make_submission(path, forced_submission_time=self._submission_time)
         self.maxDiff = None
         data_dictionary = DataDictionary.objects.all()[0]
         xpaths = [
-            u'/data/bed_net[1]/member[1]/name',
-            u'/data/bed_net[1]/member[2]/name',
-            u'/data/bed_net[2]/member[1]/name',
-            u'/data/bed_net[2]/member[2]/name',
-            u'/data/meta/instanceID'
+            "/data/bed_net[1]/member[1]/name",
+            "/data/bed_net[1]/member[2]/name",
+            "/data/bed_net[2]/member[1]/name",
+            "/data/bed_net[2]/member[2]/name",
+            "/data/meta/instanceID",
         ]
         self.assertEqual(data_dictionary.xpaths(repeat_iterations=2), xpaths)
         # test csv
-        export = generate_export(
-            Export.CSV_EXPORT,
-            self.xform,
-            None,
-            self.options)
+        export = generate_export(Export.CSV_EXPORT, self.xform, None, self.options)
         storage = get_storage_class()()
         self.assertTrue(storage.exists(export.filepath))
         path, ext = os.path.splitext(export.filename)
-        self.assertEqual(ext, '.csv')
-        test_file_path = os.path.join(self.fixture_dir, 'export.csv')
-        with storage.open(export.filepath, 'r') as csv_file:
+        self.assertEqual(ext, ".csv")
+        test_file_path = os.path.join(self.fixture_dir, "export.csv")
+        with storage.open(export.filepath, "r") as csv_file:
             self._test_csv_files(csv_file, test_file_path)
 
     def test_dotted_fields_csv_export(self):
         """
         Test CSV export with dotted field names
         """
-        path = os.path.join(os.path.dirname(__file__), 'fixtures', 'userone',
-                            'userone_with_dot_name_fields.xlsx')
+        path = os.path.join(
+            os.path.dirname(__file__),
+            "fixtures",
+            "userone",
+            "userone_with_dot_name_fields.xlsx",
+        )
         self._publish_xls_file_and_set_xform(path)
-        path = os.path.join(os.path.dirname(__file__), 'fixtures', 'userone',
-                            'userone_with_dot_name_fields.xml')
-        self._make_submission(
-            path, forced_submission_time=self._submission_time)
+        path = os.path.join(
+            os.path.dirname(__file__),
+            "fixtures",
+            "userone",
+            "userone_with_dot_name_fields.xml",
+        )
+        self._make_submission(path, forced_submission_time=self._submission_time)
         # test csv
-        self.options['id_string'] = 'userone'
-        export = generate_export(
-            Export.CSV_EXPORT,
-            self.xform,
-            None,
-            self.options)
+        self.options["id_string"] = "userone"
+        export = generate_export(Export.CSV_EXPORT, self.xform, None, self.options)
         storage = get_storage_class()()
         self.assertTrue(storage.exists(export.filepath))
         path, ext = os.path.splitext(export.filename)
-        self.assertEqual(ext, '.csv')
+        self.assertEqual(ext, ".csv")
         test_file_path = os.path.join(
-            os.path.dirname(__file__), 'fixtures', 'userone',
-            'userone_with_dot_name_fields.csv')
-        with storage.open(export.filepath, 'r') as csv_file:
+            os.path.dirname(__file__),
+            "fixtures",
+            "userone",
+            "userone_with_dot_name_fields.csv",
+        )
+        with storage.open(export.filepath, "r") as csv_file:
             self._test_csv_files(csv_file, test_file_path)
 
     def test_csv_truncated_titles(self):
         """
         Test CSV export with removed_group_name = True
         """
-        path = os.path.join(self.fixture_dir, 'tutorial_w_repeats.xlsx')
+        path = os.path.join(self.fixture_dir, "tutorial_w_repeats.xlsx")
         self._publish_xls_file_and_set_xform(path)
-        path = os.path.join(self.fixture_dir, 'tutorial_w_repeats.xml')
-        self._make_submission(
-            path, forced_submission_time=self._submission_time)
+        path = os.path.join(self.fixture_dir, "tutorial_w_repeats.xml")
+        self._make_submission(path, forced_submission_time=self._submission_time)
         # test csv
-        self.options['remove_group_name'] = True
-        export = generate_export(
-            Export.CSV_EXPORT,
-            self.xform,
-            None,
-            self.options)
+        self.options["remove_group_name"] = True
+        export = generate_export(Export.CSV_EXPORT, self.xform, None, self.options)
         storage = get_storage_class()()
         self.assertTrue(storage.exists(export.filepath))
         path, ext = os.path.splitext(export.filename)
-        self.assertEqual(ext, '.csv')
+        self.assertEqual(ext, ".csv")
         test_file_path = os.path.join(
-            self.fixture_dir, 'tutorial_w_repeats_truncate_titles.csv')
-        with storage.open(export.filepath, 'r') as csv_file:
+            self.fixture_dir, "tutorial_w_repeats_truncate_titles.csv"
+        )
+        with storage.open(export.filepath, "r") as csv_file:
             self._test_csv_files(csv_file, test_file_path)
 
     def test_csv_repeat_with_note(self):
         """
         Test that note field in repeat is not in csv export
         """
-        path = os.path.join(self.fixture_dir, 'repeat_w_note.xlsx')
+        path = os.path.join(self.fixture_dir, "repeat_w_note.xlsx")
         self._publish_xls_file_and_set_xform(path)
-        path = os.path.join(self.fixture_dir, 'repeat_w_note.xml')
-        self._make_submission(
-            path, forced_submission_time=self._submission_time)
-        export = generate_export(
-            Export.CSV_EXPORT,
-            self.xform,
-            None,
-            self.options)
+        path = os.path.join(self.fixture_dir, "repeat_w_note.xml")
+        self._make_submission(path, forced_submission_time=self._submission_time)
+        export = generate_export(Export.CSV_EXPORT, self.xform, None, self.options)
         storage = get_storage_class()()
         self.assertTrue(storage.exists(export.filepath))
         path, ext = os.path.splitext(export.filename)
-        self.assertEqual(ext, '.csv')
-        with storage.open(export.filepath, 'r') as csv_file:
+        self.assertEqual(ext, ".csv")
+        with storage.open(export.filepath, "r") as csv_file:
             reader = csv.reader(csv_file)
             rows = [row for row in reader]
             actual_headers = [h for h in rows[0]]
             expected_headers = [
-                'chnum', 'chrepeat[1]/chname', 'chrepeat[2]/chname',
-                'meta/instanceID', '_id', '_uuid', '_submission_time',
-                '_date_modified', '_tags', '_notes', '_version',
-                '_duration', '_submitted_by', '_total_media',
-                '_media_count', '_media_all_received']
+                "chnum",
+                "chrepeat[1]/chname",
+                "chrepeat[2]/chname",
+                "meta/instanceID",
+                "_id",
+                "_uuid",
+                "_submission_time",
+                "_date_modified",
+                "_tags",
+                "_notes",
+                "_version",
+                "_duration",
+                "_submitted_by",
+                "_total_media",
+                "_media_count",
+                "_media_all_received",
+            ]
             self.assertEqual(sorted(expected_headers), sorted(actual_headers))

onadata/apps/viewer/models/data_dictionary.py

@@ -35,6 +35,7 @@
     PROJ_FORMS_CACHE,
     safe_delete,
 )
+from onadata.libs.utils.common_tags import EXPORT_REPEAT_REGISTER
 from onadata.libs.utils.model_tools import get_columns_with_hxl, set_uuid
 
 
@@ -438,3 +439,21 @@ def invalidate_caches(sender, instance=None, created=False, **kwargs):
     sender=DataDictionary,
     dispatch_uid="xform_invalidate_caches",
 )
+
+
+def create_export_repeat_register(sender, instance=None, created=False, **kwargs):
+    """Create export repeat register for the form"""
+    if created:
+        MetaData.objects.create(
+            content_type=ContentType.objects.get_for_model(instance),
+            object_id=instance.pk,
+            data_type=EXPORT_REPEAT_REGISTER,
+            data_value="",
+        )
+
+
+post_save.connect(
+    create_export_repeat_register,
+    sender=DataDictionary,
+    dispatch_uid="create_export_repeat_register",
+)

onadata/apps/viewer/models/tests/test_data_dictionary.py

@@ -2,6 +2,7 @@
 
 import json
 
+from django.contrib.contenttypes.models import ContentType
 from django.core.cache import cache
 
 from onadata.apps.logger.models.entity_list import EntityList
@@ -307,3 +308,15 @@ def test_cache_invalidated(self):
 
         for key in cache_keys:
             self.assertIsNone(cache.get(key))
+
+    def test_export_repeat_register_created(self):
+        """Export repeat register is created when form is published"""
+        xform = self._publish_markdown(self.registration_form, self.user)
+        content_type = ContentType.objects.get_for_model(xform)
+        exists = MetaData.objects.filter(
+            data_type="export_repeat_register",
+            object_id=xform.pk,
+            content_type=content_type,
+        ).exists()
+
+        self.assertTrue(exists)