alert_words: Fix cache flushing behavior and add tests.

The post_init cache-flushing behavior in the original alert words
migration was subtly wrong; while it may have passed tests, it didn't
have the right ordering for unlikely races.

We use post_save rather than post_init hooks precisely because they
ensure that we flush the cache after we know the database has been
updated and any future reads from the database will have the latest
state.

zerver/lib/alert_words.py

@@ -1,6 +1,6 @@
 from django.db import transaction
 
-from zerver.models import UserProfile, Realm, AlertWord
+from zerver.models import UserProfile, Realm, AlertWord, flush_realm_alert_words
 from zerver.lib.cache import cache_with_key, realm_alert_words_cache_key, \
     realm_alert_words_automaton_cache_key
 import ahocorasick
@@ -56,6 +56,8 @@ def add_user_alert_words(user_profile: UserProfile, new_words: Iterable[str]) ->
         AlertWord(user_profile=user_profile, word=word, realm=user_profile.realm)
         for word in word_dict.values()
     )
+    # Django bulk_create operations don't flush caches, so we need to do this ourselves.
+    flush_realm_alert_words(user_profile.realm)
 
     return user_alert_words(user_profile)
 
@@ -63,6 +65,7 @@ def add_user_alert_words(user_profile: UserProfile, new_words: Iterable[str]) ->
 def remove_user_alert_words(user_profile: UserProfile, delete_words: Iterable[str]) -> List[str]:
     # TODO: Ideally, this would be a bulk query, but Django doesn't have a `__iexact`.
     # We can clean this up if/when Postgres has more native support for case-insensitive fields.
+    # If we turn this into a bulk operation, we will need to call flush_realm_alert_words() here.
     for delete_word in delete_words:
         AlertWord.objects.filter(user_profile=user_profile, word__iexact=delete_word).delete()
     return user_alert_words(user_profile)

zerver/models.py

@@ -25,7 +25,7 @@ from zerver.lib.utils import make_safe_digest, generate_random_token
 from django.db import transaction
 from django.utils.timezone import now as timezone_now
 from zerver.lib.timestamp import datetime_to_timestamp
-from django.db.models.signals import post_save, post_delete, post_init
+from django.db.models.signals import post_save, post_delete
 from django.utils.translation import ugettext_lazy as _
 from zerver.lib import cache
 from zerver.lib.validator import check_int, \
@@ -2900,11 +2900,13 @@ class AlertWord(models.Model):
     class Meta:
         unique_together = ("user_profile", "word")
 
-def flush_alert_word(sender: Any, **kwargs: Any) -> None:
-    realm = kwargs['instance'].realm
+def flush_realm_alert_words(realm: Realm) -> None:
     cache_delete(realm_alert_words_cache_key(realm))
     cache_delete(realm_alert_words_automaton_cache_key(realm))
 
+def flush_alert_word(sender: Any, **kwargs: Any) -> None:
+    realm = kwargs['instance'].realm
+    flush_realm_alert_words(realm)
 
-post_init.connect(flush_alert_word, sender=AlertWord)
+post_save.connect(flush_alert_word, sender=AlertWord)
 post_delete.connect(flush_alert_word, sender=AlertWord)

zerver/tests/test_alert_words.py

@@ -44,27 +44,37 @@ class AlertWordTests(ZulipTestCase):
         words = user_alert_words(user)
         self.assertEqual(words, [])
 
-    def test_add_word(self) -> None:
+    def test_basics(self) -> None:
         """
-        add_user_alert_words can add multiple alert words at once.
+        Verifies the basic behavior of modifying alert words.
+
+        Also verifies the cache-flushing behavior.
         """
         user = self.example_user('cordelia')
+        realm_alert_words = alert_words_in_realm(user.realm)
+        self.assert_length(realm_alert_words.get(user.id, []), 0)
 
         # Add several words, including multi-word and non-ascii words.
         add_user_alert_words(user, self.interesting_alert_word_list)
 
         words = user_alert_words(user)
         self.assertEqual(set(words), set(self.interesting_alert_word_list))
+        realm_alert_words = alert_words_in_realm(user.realm)
+        self.assert_length(realm_alert_words[user.id], 3)
 
         # Test the case-insensitivity of adding words
         add_user_alert_words(user, set(["ALert", "ALERT"]))
         words = user_alert_words(user)
         self.assertEqual(set(words), set(self.interesting_alert_word_list))
+        realm_alert_words = alert_words_in_realm(user.realm)
+        self.assert_length(realm_alert_words[user.id], 3)
 
         # Test the case-insensitivity of removing words
         remove_user_alert_words(user, set(["ALert"]))
         words = user_alert_words(user)
         self.assertEqual(set(words), set(self.interesting_alert_word_list) - {'alert'})
+        realm_alert_words = alert_words_in_realm(user.realm)
+        self.assert_length(realm_alert_words[user.id], 2)
 
     def test_remove_word(self) -> None:
         """