import: Fix rendered_content in imported messages.

After the messages have been imported, set the rendered_content of the
messages instead of leaving its value to be 'None'.

This is important to ensure that:
(1) Performance for users is good after completing the import.
(2) The database's full-text indexes have all of the imported messages
(which only happens properly when Message rows have their
rendered_content field edited).

Fixes #9168.

tools/linter_lib/custom_check.py

@@ -420,6 +420,7 @@ def build_custom_checkers(by_lang):
         {'pattern': '[^r]Message.objects.get',
          'exclude': set(["zerver/tests",
                          "zerver/lib/onboarding.py",
+                         "zerver/lib/import_realm.py",
                          "zilencer/management/commands/add_mock_conversation.py",
                          "zerver/worker/queue_processors.py"]),
          'description': 'Please use access_message() to fetch Message objects',

zerver/lib/import_realm.py

@@ -17,6 +17,8 @@ from zerver.lib.avatar_hash import user_avatar_path_from_ids
 from zerver.lib.bulk_create import bulk_create_users
 from zerver.lib.export import DATE_FIELDS, realm_tables, \
     Record, TableData, TableName, Field, Path
+from zerver.lib.message import save_message_rendered_content
+from zerver.lib.bugdown import version as bugdown_version
 from zerver.lib.upload import random_name, sanitize_name, \
     S3UploadBackend, LocalUploadBackend
 from zerver.lib.utils import generate_api_key
@@ -196,6 +198,24 @@ def fix_customprofilefield(data: TableData) -> None:
                 old_id_list=old_user_id_list)
             item['value'] = ujson.dumps(new_id_list)
 
+def fix_message_rendered_content(data: TableData, field: TableName) -> None:
+    """
+    This function sets the rendered_content of all the messages
+    after the messages have been imported.
+    """
+    for message in data[field]:
+        message_object = Message.objects.get(id=message['id'])
+        try:
+            rendered_content = save_message_rendered_content(message_object, message['content'])  # type: Optional[str]
+        except Exception:
+            rendered_content = None
+
+        if rendered_content is None:
+            # This can happen with two possible causes:
+            # * rendering markdown failing with the exception being caught in bugdown
+            # * The explicit None clause from an exception escaping
+            logging.warning("Error in markdown rendering for message ID %s; continuing" % (message['id']))
+
 def current_table_ids(data: TableData, table: TableName) -> List[int]:
     """
     Returns the ids present in the current table
@@ -861,6 +881,9 @@ def import_message_data(import_dir: Path) -> None:
         re_map_foreign_keys(data, 'zerver_message', 'id', related_table='message', id_field=True)
         bulk_import_model(data, Message)
 
+        fix_message_rendered_content(data, 'zerver_message')
+        logging.info("Successfully rendered markdown for message batch")
+
         # Due to the structure of these message chunks, we're
         # guaranteed to have already imported all the Message objects
         # for this batch of UserMessage objects.

zerver/lib/message.py

@@ -151,6 +151,13 @@ def stringify_message_dict(message_dict: Dict[str, Any]) -> bytes:
 def message_to_dict_json(message: Message) -> bytes:
     return MessageDict.to_dict_uncached(message)
 
+def save_message_rendered_content(message: Message, content: str) -> str:
+    rendered_content = render_markdown(message, content, realm=message.get_realm())
+    message.rendered_content = rendered_content
+    message.rendered_content_version = bugdown.version
+    message.save_rendered_content()
+    return rendered_content
+
 class MessageDict:
     @staticmethod
     def wide_dict(message: Message) -> Dict[str, Any]:
@@ -347,10 +354,7 @@ class MessageDict:
             assert message is not None  # Hint for mypy.
             # It's unfortunate that we need to have side effects on the message
             # in some cases.
-            rendered_content = render_markdown(message, content, realm=message.get_realm())
-            message.rendered_content = rendered_content
-            message.rendered_content_version = bugdown.version
-            message.save_rendered_content()
+            rendered_content = save_message_rendered_content(message, content)
 
         if rendered_content is not None:
             obj['rendered_content'] = rendered_content

zerver/tests/test_gitter_importer.py

@@ -17,6 +17,8 @@ from zerver.lib.test_helpers import (
 from zerver.models import (
     Realm,
     get_realm,
+    UserProfile,
+    Message,
 )
 from zerver.data_import.gitter import (
     do_convert_data,
@@ -43,7 +45,7 @@ class GitterImporter(ZulipTestCase):
         return output_dir
 
     @mock.patch('zerver.data_import.gitter.process_avatars', return_value=[])
-    def test_gitter_import_to_existing_database(self, mock_process_avatars: mock.Mock) -> None:
+    def test_gitter_import_data_conversion(self, mock_process_avatars: mock.Mock) -> None:
         output_dir = self._make_output_dir()
         gitter_file = os.path.join(os.path.dirname(__file__), 'fixtures/gitter_data.json')
         do_convert_data(gitter_file, output_dir)
@@ -108,6 +110,21 @@ class GitterImporter(ZulipTestCase):
         exported_usermessage_message = get_set(messages['zerver_usermessage'], 'message')
         self.assertEqual(exported_usermessage_message, exported_messages_id)
 
+    @mock.patch('zerver.data_import.gitter.process_avatars', return_value=[])
+    def test_gitter_import_to_existing_database(self, mock_process_avatars: mock.Mock) -> None:
+        output_dir = self._make_output_dir()
+        gitter_file = os.path.join(os.path.dirname(__file__), 'fixtures/gitter_data.json')
+        do_convert_data(gitter_file, output_dir)
+
+        do_import_realm(output_dir, 'test-gitter-import')
+        realm = get_realm('test-gitter-import')
+
+        # test rendered_messages
+        realm_users = UserProfile.objects.filter(realm=realm)
+        messages = Message.objects.filter(sender__in=realm_users)
+        for message in messages:
+            self.assertIsNotNone(message.rendered_content, None)
+
     def test_get_usermentions(self) -> None:
         user_map = {'57124a4': 3, '57124b4': 5, '57124c4': 8}
         user_short_name_to_full_name = {'user': 'user name', 'user2': 'user2',