enhance upsert validation and add unit tests for alternate key handling

Author: Samson Gebre

src/PowerPlatform/Dataverse/data/_odata.py

@@ -358,11 +358,20 @@ def _build_alternate_key_str(self, alternate_key: Dict[str, Any]) -> str:
         String values are single-quoted and escaped; all other values are rendered as-is.
 
         :param alternate_key: Mapping of alternate key attribute names to their values.
+            Must be a non-empty dict with string keys.
         :type alternate_key: ``dict[str, Any]``
 
         :return: Comma-separated key=value pairs suitable for use in a URL segment.
         :rtype: ``str``
+
+        :raises ValueError: If ``alternate_key`` is empty.
+        :raises TypeError: If any key in ``alternate_key`` is not a string.
         """
+        if not alternate_key:
+            raise ValueError("alternate_key must be a non-empty dict")
+        bad_keys = [k for k in alternate_key if not isinstance(k, str)]
+        if bad_keys:
+            raise TypeError(f"alternate_key keys must be strings; got: {bad_keys!r}")
         parts = []
         for k, v in alternate_key.items():
             k_lower = k.lower() if isinstance(k, str) else k
@@ -433,20 +442,26 @@ def _upsert_multiple(
         :return: ``None``
         :rtype: ``None``
 
-        :raises ValueError: If ``alternate_keys`` and ``records`` differ in length.
+        :raises ValueError: If ``alternate_keys`` and ``records`` differ in length, or if
+            any record payload contains an alternate key field with a conflicting value.
         """
         if len(alternate_keys) != len(records):
             raise ValueError(
-                f"alternate_keys and records must have the same length " f"({len(alternate_keys)} != {len(records)})"
+                f"alternate_keys and records must have the same length "
+                f"({len(alternate_keys)} != {len(records)})"
             )
         logical_name = table_schema_name.lower()
         targets: List[Dict[str, Any]] = []
         for alt_key, record in zip(alternate_keys, records):
-            combined: Dict[str, Any] = {}
-            combined.update(self._lowercase_keys(alt_key))
+            alt_key_lower = self._lowercase_keys(alt_key)
             record_processed = self._lowercase_keys(record)
             record_processed = self._convert_labels_to_ints(table_schema_name, record_processed)
-            combined.update(record_processed)
+            conflicting = {k for k in set(alt_key_lower) & set(record_processed) if alt_key_lower[k] != record_processed[k]}
+            if conflicting:
+                raise ValueError(
+                    f"record payload conflicts with alternate_key on fields: {sorted(conflicting)!r}"
+                )
+            combined: Dict[str, Any] = {**alt_key_lower, **record_processed}
             if "@odata.type" not in combined:
                 combined["@odata.type"] = f"Microsoft.Dynamics.CRM.{logical_name}"
             key_str = self._build_alternate_key_str(alt_key)

src/PowerPlatform/Dataverse/operations/records.py

@@ -343,7 +343,11 @@ def upsert(self, table: str, items: List[Union[UpsertItem, Dict[str, Any]]]) ->
         for i in items:
             if isinstance(i, UpsertItem):
                 normalized.append(i)
-            elif isinstance(i, dict) and isinstance(i.get("alternate_key"), dict) and isinstance(i.get("record"), dict):
+            elif (
+                isinstance(i, dict)
+                and isinstance(i.get("alternate_key"), dict)
+                and isinstance(i.get("record"), dict)
+            ):
                 normalized.append(UpsertItem(alternate_key=i["alternate_key"], record=i["record"]))
             else:
                 raise TypeError("Each item must be a UpsertItem or a dict with 'alternate_key' and 'record' keys")

tests/unit/test_odata_internal.py tests/unit/data/test_odata_internal.pytests/unit/test_odata_internal.py renamed to tests/unit/data/test_odata_internal.py

@@ -57,6 +57,26 @@ def test_equal_lengths_does_not_raise(self):
         self.assertEqual(len(post_calls), 1)
         self.assertIn("UpsertMultiple", post_calls[0].args[1])
 
+    def test_record_conflicts_with_alternate_key_raises_value_error(self):
+        """_upsert_multiple raises ValueError when a record field contradicts its alternate key."""
+        with self.assertRaises(ValueError) as ctx:
+            self.od._upsert_multiple(
+                "accounts",
+                "account",
+                [{"accountnumber": "ACC-001"}],
+                [{"accountnumber": "ACC-WRONG", "name": "Contoso"}],
+            )
+        self.assertIn("accountnumber", str(ctx.exception))
+
+    def test_record_matching_alternate_key_field_does_not_raise(self):
+        """_upsert_multiple does not raise when a record field matches its alternate key value."""
+        self.od._upsert_multiple(
+            "accounts",
+            "account",
+            [{"accountnumber": "ACC-001"}],
+            [{"accountnumber": "ACC-001", "name": "Contoso"}],
+        )
+
 
 class TestBuildAlternateKeyStr(unittest.TestCase):
     """Unit tests for _ODataClient._build_alternate_key_str."""
@@ -76,7 +96,9 @@ def test_single_int_value(self):
 
     def test_composite_key_string_and_string(self):
         """Composite key with two string values produces comma-separated pairs."""
-        result = self.od._build_alternate_key_str({"accountnumber": "ACC-001", "address1_postalcode": "98052"})
+        result = self.od._build_alternate_key_str(
+            {"accountnumber": "ACC-001", "address1_postalcode": "98052"}
+        )
         self.assertEqual(result, "accountnumber='ACC-001',address1_postalcode='98052'")
 
     def test_composite_key_string_and_int(self):
@@ -94,6 +116,16 @@ def test_single_quote_in_value_is_escaped(self):
         result = self.od._build_alternate_key_str({"name": "O'Brien"})
         self.assertEqual(result, "name='O''Brien'")
 
+    def test_empty_dict_raises_value_error(self):
+        """Empty alternate_key raises ValueError."""
+        with self.assertRaises(ValueError):
+            self.od._build_alternate_key_str({})
+
+    def test_non_string_key_raises_type_error(self):
+        """Non-string key raises TypeError."""
+        with self.assertRaises(TypeError):
+            self.od._build_alternate_key_str({1: "ACC-001"})
+
 
 class TestUpsert(unittest.TestCase):
     """Unit tests for _ODataClient._upsert."""
@@ -128,7 +160,8 @@ def test_url_contains_composite_alternate_key(self):
             {"name": "Contoso"},
         )
         call = self._patch_call()
-        self.assertIn("accountnumber='ACC-001',address1_postalcode='98052'", call.args[1])
+        expected_key = "accountnumber='ACC-001',address1_postalcode='98052'"
+        self.assertIn(expected_key, call.args[1])
 
     def test_record_keys_lowercased(self):
         """Record field names are lowercased before sending."""

tests/unit/test_records_operations.py

@@ -191,11 +191,17 @@ def test_upsert_multiple_calls_upsert_multiple(self):
                 "UpsertItem",
                 [
                     UpsertItem(
-                        alternate_key={"accountnumber": "ACC-001", "address1_postalcode": "98052"},
+                        alternate_key={
+                            "accountnumber": "ACC-001",
+                            "address1_postalcode": "98052",
+                        },
                         record={"name": "Contoso Ltd"},
                     ),
                     UpsertItem(
-                        alternate_key={"accountnumber": "ACC-002", "address1_postalcode": "10001"},
+                        alternate_key={
+                            "accountnumber": "ACC-002",
+                            "address1_postalcode": "10001",
+                        },
                         record={"name": "Fabrikam Inc"},
                     ),
                 ],
@@ -204,11 +210,17 @@ def test_upsert_multiple_calls_upsert_multiple(self):
                 "dict",
                 [
                     {
-                        "alternate_key": {"accountnumber": "ACC-001", "address1_postalcode": "98052"},
+                        "alternate_key": {
+                            "accountnumber": "ACC-001",
+                            "address1_postalcode": "98052",
+                        },
                         "record": {"name": "Contoso Ltd"},
                     },
                     {
-                        "alternate_key": {"accountnumber": "ACC-002", "address1_postalcode": "10001"},
+                        "alternate_key": {
+                            "accountnumber": "ACC-002",
+                            "address1_postalcode": "10001",
+                        },
                         "record": {"name": "Fabrikam Inc"},
                     },
                 ],