Merge pull request #2339 from PolicyEngine/test_set_policy_service

Refactored tests for set policy service (Fixes: #2392)

Author: anth-volk

changelog_entry.yaml

@@ -0,0 +1,4 @@
+- bump: minor
+  changes:
+    added:
+    - Refactored tests for set_policy_service

policyengine_api/services/policy_service.py

@@ -96,6 +96,13 @@ def set_policy(
         print("Setting new policy")
 
         try:
+            # Convert country_id to lowercase
+            if not country_id.islower():
+                country_id = country_id.lower()
+
+            # Validate country_id
+            if country_id not in COUNTRY_PACKAGE_VERSIONS:
+                raise ValueError(f"Invalid country_id: {country_id}")
 
             policy_hash = hash_object(policy_json)
             api_version = COUNTRY_PACKAGE_VERSIONS.get(country_id)

tests/fixtures/services/policy_service.py

@@ -21,6 +21,13 @@
 }
 
 
+@pytest.fixture
+def mock_database():
+    """Mock the database module."""
+    with patch("policyengine_api.services.policy_service.database") as mock_db:
+        yield mock_db
+
+
 @pytest.fixture
 def mock_hash_object():
     """Mock the hash_object function."""

tests/to_refactor/python/test_policy_service_old.py

@@ -90,7 +90,7 @@ def test_set_policy_new(self, policy_service, mock_database):
 
         test_policy = {"param": "value"}
         test_label = "new_policy"
-        test_country_id = "US"
+        test_country_id = "us"
 
         expected_calls = [
             # First call - check if policy exists

tests/unit/services/test_policy_service.py

@@ -1,10 +1,13 @@
 import pytest
 import json
-
+from unittest.mock import call
 from policyengine_api.services.policy_service import PolicyService
 
 from tests.fixtures.services.policy_service import (
     valid_policy_data,
+    valid_hash_value,
+    mock_hash_object,
+    mock_database,
     existing_policy_record,
 )
 
@@ -167,3 +170,156 @@ def test_get_policy_json_given_negative_int_id(self, test_db):
             # WHEN we call get_policy_json with the invalid ID...
             # THEN an exception should be raised
             service.get_policy_json("us", INVALID_RECORD_ID)
+
+
+class TestSetPolicy:
+    def test_set_policy_new(self, mock_database, mock_hash_object):
+        # GIVEN a new policy to insert
+        new_policy_id = 12  # Different from existing fixture ID
+        test_policy = {"param": "value"}
+        test_label = "new_policy"
+        test_country_id = "us"
+
+        # Get current API version dynamically
+        from policyengine_api.constants import COUNTRY_PACKAGE_VERSIONS
+
+        current_api_version = COUNTRY_PACKAGE_VERSIONS.get(test_country_id)
+
+        # Setup mocks
+        mock_database.query.return_value.fetchone.side_effect = [
+            None,  # First call: policy does not exist
+            {"id": new_policy_id},  # Second call: fetch newly inserted policy
+        ]
+
+        # Define expected database calls
+        expected_calls = [
+            # First call - check if policy exists
+            call(
+                "SELECT * FROM policy WHERE country_id = ? AND policy_hash = ? AND label = ?",
+                (test_country_id, valid_hash_value, test_label),
+            ),
+            # Second call - insert new policy
+            call(
+                "INSERT INTO policy (country_id, policy_json, policy_hash, label, api_version) VALUES (?, ?, ?, ?, ?)",
+                (
+                    test_country_id,
+                    json.dumps(test_policy),
+                    valid_hash_value,
+                    test_label,
+                    current_api_version,  # From COUNTRY_PACKAGE_VERSIONS for 'us'
+                ),
+            ),
+            # Third call - fetch the newly created policy
+            call(
+                "SELECT * FROM policy WHERE country_id = ? AND policy_hash = ? AND label = ?",
+                (test_country_id, valid_hash_value, test_label),
+            ),
+        ]
+
+        # WHEN we call set_policy
+        policy_id, message, exists = service.set_policy(
+            test_country_id, test_label, test_policy
+        )
+
+        # THEN the result should indicate a new policy was created
+        assert policy_id == new_policy_id
+        assert message == "Policy created"
+        assert exists is False
+
+        # Verify the database queries were called as expected
+        assert mock_database.query.call_args_list == expected_calls
+
+    def test_set_policy_existing(
+        self, mock_database, mock_hash_object, existing_policy_record
+    ):
+        # GIVEN an existing policy record
+        existing_policy = existing_policy_record
+
+        # Setup mock
+        mock_database.query.return_value.fetchone.return_value = (
+            existing_policy
+        )
+
+        # Define expected database calls - matches actual implementation
+        expected_calls = [
+            call(
+                "SELECT * FROM policy WHERE country_id = ? AND policy_hash = ? AND label IS NULL",
+                (
+                    existing_policy["country_id"],
+                    valid_hash_value,
+                ),  # No None parameter for IS NULL
+            ),
+        ]
+
+        # WHEN we call set_policy with existing policy data
+        policy_id, message, exists = service.set_policy(
+            existing_policy["country_id"],
+            existing_policy["label"],
+            json.loads(existing_policy["policy_json"]),
+        )
+
+        # THEN the result should indicate the policy already exists
+        assert policy_id == existing_policy["id"]
+        assert message == "Policy already exists"
+        assert exists is True
+
+        # Verify the database query was called as expected
+        assert mock_database.query.call_args_list == expected_calls
+
+    def test_set_policy_given_database_insert_failure(
+        self, mock_database, mock_hash_object
+    ):
+        # GIVEN a database insertion failure
+        test_policy = {"param": "value"}
+        test_label = "test_policy"
+        test_country_id = "us"
+
+        # Setup mock to raise exception on insert
+        mock_database.query.return_value.fetchone.side_effect = [
+            None,  # First call: policy does not exist
+            Exception(
+                "Database insertion failed"
+            ),  # Second call: insertion fails
+        ]
+
+        # WHEN we call set_policy
+        with pytest.raises(Exception, match="Database insertion failed"):
+            # THEN an exception should be raised
+            service.set_policy(test_country_id, test_label, test_policy)
+
+    def test_set_policy_given_invalid_country_id(self, mock_hash_object):
+        # GIVEN an invalid country_id
+        INVALID_COUNTRY_ID = "xx"  # Unsupported country code
+        test_policy = {"param": "value"}
+        test_label = "test_policy"
+
+        # WHEN we call set_policy with an invalid country_id
+        with pytest.raises(
+            ValueError, match=f"Invalid country_id: {INVALID_COUNTRY_ID}"
+        ):
+            # THEN an exception should be raised
+            service.set_policy(INVALID_COUNTRY_ID, test_label, test_policy)
+
+    def test_set_policy_given_empty_label(
+        self, mock_database, mock_hash_object
+    ):
+        # GIVEN an empty label
+        EMPTY_LABEL = ""
+        test_policy = {"param": "value"}
+        test_country_id = "us"
+
+        # Setup mock
+        mock_database.query.return_value.fetchone.side_effect = [
+            None,  # Policy does not exist
+            {"id": 13},  # Return mock policy after creation
+        ]
+
+        # WHEN we call set_policy with an empty label
+        policy_id, message, exists = service.set_policy(
+            test_country_id, EMPTY_LABEL, test_policy
+        )
+
+        # THEN the result should indicate a new policy was created
+        assert policy_id == 13
+        assert message == "Policy created"
+        assert exists is False