Fix tests for index_vcon_parties and no default vcon TTL

- Add mock_redis.sadd = AsyncMock() so index_vcon_parties can run in tests
- Patch index_vcon_parties in expiry tests; update assertions for no default
  VCON_REDIS_EXPIRY on vcon key (only index keys get expire)
- Rename expiry tests to reflect new behavior (stores without default expiry)
- Replace test_external_ingress_expire_called_before_rpush with
  test_external_ingress_adds_to_ingress_list

Made-with: Cursor

Author: howethomas

server/tests/test_external_ingress.py

@@ -47,12 +47,13 @@ def test_successful_submission_single_api_key(
         # Configure mocks
         mock_get_ingress_auth.return_value = {self.ingress_list: self.valid_api_key}
 
-        # Mock Redis client properly
+        # Mock Redis client properly (sadd/expire used by index_vcon_parties)
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         # Set the global redis_async directly in the api module
@@ -77,12 +78,10 @@ def test_successful_submission_single_api_key(
 
             # Verify Redis operations were called
             mock_json.set.assert_called_once()
-            mock_redis.expire.assert_called_once()  # Verify expiry was set
             mock_redis.rpush.assert_called_once_with(
                 self.ingress_list, self.test_vcon["uuid"]
             )
             mock_add_vcon_to_set.assert_called_once()
-            mock_index_vcon.assert_called_once()
 
         finally:
             # Clean up the global variable
@@ -100,12 +99,13 @@ def test_successful_submission_multiple_api_keys(
             self.ingress_list: ["partner-1-key", self.valid_api_key, "partner-3-key"]
         }
 
-        # Mock Redis client properly
+        # Mock Redis client properly (sadd/expire used by index_vcon_parties)
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         # Set the global redis_async directly in the api module
@@ -233,6 +233,7 @@ def test_redis_failure_handling(self, mock_get_ingress_auth):
         mock_json.set = AsyncMock(side_effect=Exception("Redis connection failed"))
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         # Set the global redis_async directly in the api module
@@ -266,12 +267,13 @@ def test_multiple_ingress_lists_isolation(
             "shared_ingress": ["partner-a-key", "partner-b-key-1", "shared-key"],
         }
 
-        # Mock Redis client properly
+        # Mock Redis client properly (sadd/expire used by index_vcon_parties)
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         # Set the global redis_async directly in the api module

server/tests/test_post_vcon_expiry.py

@@ -1,7 +1,10 @@
-"""Unit tests for POST vCon default expiry functionality.
+"""Unit tests for POST vCon expiry behavior.
 
-These tests verify that vCons created via POST endpoints are stored with
-the default VCON_REDIS_EXPIRY TTL (3600 seconds by default).
+These tests verify that vCons created via POST endpoints are stored and
+indexed. As of the optimize-ingest-indexing change, default VCON_REDIS_EXPIRY
+TTL is no longer set on newly stored vCons; retention is controlled by
+storage backends or explicit TTL. Indexing (index_vcon_parties) still uses
+Redis sadd/expire for party index keys.
 """
 
 from unittest.mock import AsyncMock, MagicMock, patch
@@ -25,46 +28,43 @@ def setup_method(self):
         self.test_vcon = generate_mock_vcon()
 
     @patch("api.add_vcon_to_set")
-    @patch("api.index_vcon")
-    def test_post_vcon_sets_default_expiry(
-        self, mock_index_vcon, mock_add_vcon_to_set
+    @patch("api.index_vcon_parties")
+    def test_post_vcon_stores_without_default_expiry(
+        self, mock_index_vcon_parties, mock_add_vcon_to_set
     ):
-        """Test that POST /vcon sets the default VCON_REDIS_EXPIRY TTL."""
-        # Mock Redis client
+        """Test that POST /vcon stores vCon and indexes parties; no default vcon TTL."""
+        # Mock Redis client (sadd/expire used by index_vcon_parties when not patched)
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
-        # Set the global redis_async directly in the api module
         api.redis_async = mock_redis
 
         try:
-            # Make request
             response = self.client.post(
                 "/vcon",
                 json=self.test_vcon,
             )
 
-            # Debug the response if it fails
             if response.status_code != 201:
                 print(f"Status: {response.status_code}")
                 print(f"Response: {response.text}")
 
-            # Assertions
             assert response.status_code == 201
-
-            # Verify Redis JSON set was called
             mock_json.set.assert_called_once()
-
-            # Verify expire was called with the default VCON_REDIS_EXPIRY
-            expected_key = f"vcon:{self.test_vcon['uuid']}"
-            mock_redis.expire.assert_called_once_with(expected_key, VCON_REDIS_EXPIRY)
+            # No default TTL on vcon key: expire is only used for index keys by index_vcon_parties
+            vcon_key = f"vcon:{self.test_vcon['uuid']}"
+            vcon_expire_calls = [
+                c for c in mock_redis.expire.call_args_list
+                if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
+            ]
+            assert len(vcon_expire_calls) == 0, "vCon key should not get default VCON_REDIS_EXPIRY TTL"
 
         finally:
-            # Clean up the global variable
             api.redis_async = None
 
     def test_post_vcon_expiry_value_is_3600(self):
@@ -73,36 +73,36 @@ def test_post_vcon_expiry_value_is_3600(self):
         assert VCON_REDIS_EXPIRY == 3600, "Default VCON_REDIS_EXPIRY should be 3600 seconds"
 
     @patch("api.add_vcon_to_set")
-    @patch("api.index_vcon")
-    def test_post_vcon_with_ingress_list_sets_expiry(
-        self, mock_index_vcon, mock_add_vcon_to_set
+    @patch("api.index_vcon_parties")
+    def test_post_vcon_with_ingress_list_stores_without_default_expiry(
+        self, mock_index_vcon_parties, mock_add_vcon_to_set
     ):
-        """Test that POST /vcon with ingress_lists still sets expiry."""
-        # Mock Redis client
+        """Test that POST /vcon with ingress_lists stores and adds to list; no default vcon TTL."""
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         api.redis_async = mock_redis
 
         try:
-            # Make request with ingress_lists parameter
             response = self.client.post(
                 "/vcon?ingress_lists=test_ingress",
                 json=self.test_vcon,
             )
 
             assert response.status_code == 201
-
-            # Verify expire was still called
-            expected_key = f"vcon:{self.test_vcon['uuid']}"
-            mock_redis.expire.assert_called_once_with(expected_key, VCON_REDIS_EXPIRY)
-
-            # Verify ingress list was also populated
-            mock_redis.rpush.assert_called_once_with("test_ingress", self.test_vcon['uuid'])
+            # No default TTL on vcon key
+            vcon_key = f"vcon:{self.test_vcon['uuid']}"
+            vcon_expire_calls = [
+                c for c in mock_redis.expire.call_args_list
+                if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
+            ]
+            assert len(vcon_expire_calls) == 0
+            mock_redis.rpush.assert_called_once_with("test_ingress", self.test_vcon["uuid"])
 
         finally:
             api.redis_async = None
@@ -120,72 +120,61 @@ def setup_method(self):
 
     @patch("config.Configuration.get_ingress_auth")
     @patch("api.add_vcon_to_set")
-    @patch("api.index_vcon")
-    def test_external_ingress_sets_default_expiry(
-        self, mock_index_vcon, mock_add_vcon_to_set, mock_get_ingress_auth
+    @patch("api.index_vcon_parties")
+    def test_external_ingress_stores_without_default_expiry(
+        self, mock_index_vcon_parties, mock_add_vcon_to_set, mock_get_ingress_auth
     ):
-        """Test that POST /vcon/external-ingress sets the default VCON_REDIS_EXPIRY TTL."""
-        # Configure ingress auth mock
+        """Test that POST /vcon/external-ingress stores vCon; no default VCON_REDIS_EXPIRY on vcon key."""
         mock_get_ingress_auth.return_value = {self.ingress_list: self.valid_api_key}
 
-        # Mock Redis client
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
         mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
         mock_redis.rpush = AsyncMock()
 
         api.redis_async = mock_redis
 
         try:
-            # Make request
             response = self.client.post(
                 f"/vcon/external-ingress?ingress_list={self.ingress_list}",
                 json=self.test_vcon,
                 headers={CONSERVER_HEADER_NAME: self.valid_api_key},
             )
 
-            # Debug the response if it fails
             if response.status_code != 204:
                 print(f"Status: {response.status_code}")
                 print(f"Response: {response.text}")
 
-            # Assertions
             assert response.status_code == 204
-
-            # Verify expire was called with the default VCON_REDIS_EXPIRY
-            expected_key = f"vcon:{self.test_vcon['uuid']}"
-            mock_redis.expire.assert_called_once_with(expected_key, VCON_REDIS_EXPIRY)
+            vcon_key = f"vcon:{self.test_vcon['uuid']}"
+            vcon_expire_calls = [
+                c for c in mock_redis.expire.call_args_list
+                if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
+            ]
+            assert len(vcon_expire_calls) == 0, "vCon key should not get default VCON_REDIS_EXPIRY TTL"
 
         finally:
             api.redis_async = None
 
     @patch("config.Configuration.get_ingress_auth")
     @patch("api.add_vcon_to_set")
-    @patch("api.index_vcon")
-    def test_external_ingress_expire_called_before_rpush(
-        self, mock_index_vcon, mock_add_vcon_to_set, mock_get_ingress_auth
+    @patch("api.index_vcon_parties")
+    def test_external_ingress_adds_to_ingress_list(
+        self, mock_index_vcon_parties, mock_add_vcon_to_set, mock_get_ingress_auth
     ):
-        """Test that expire is called before adding to ingress list."""
+        """Test that external ingress stores vCon and adds to ingress list."""
         mock_get_ingress_auth.return_value = {self.ingress_list: self.valid_api_key}
 
-        # Mock Redis client with call order tracking
         mock_redis = MagicMock()
         mock_json = MagicMock()
         mock_json.set = AsyncMock()
         mock_redis.json.return_value = mock_json
-        
-        call_order = []
-        
-        async def track_expire(key, ttl):
-            call_order.append(('expire', key, ttl))
-        
-        async def track_rpush(list_name, uuid):
-            call_order.append(('rpush', list_name, uuid))
-        
-        mock_redis.expire = AsyncMock(side_effect=track_expire)
-        mock_redis.rpush = AsyncMock(side_effect=track_rpush)
+        mock_redis.expire = AsyncMock()
+        mock_redis.sadd = AsyncMock()
+        mock_redis.rpush = AsyncMock()
 
         api.redis_async = mock_redis
 
@@ -197,12 +186,7 @@ async def track_rpush(list_name, uuid):
             )
 
             assert response.status_code == 204
-
-            # Verify call order: expire should come before rpush
-            assert len(call_order) >= 2
-            expire_index = next(i for i, c in enumerate(call_order) if c[0] == 'expire')
-            rpush_index = next(i for i, c in enumerate(call_order) if c[0] == 'rpush')
-            assert expire_index < rpush_index, "expire should be called before rpush"
+            mock_redis.rpush.assert_called_once_with(self.ingress_list, self.test_vcon["uuid"])
 
         finally:
             api.redis_async = None