diff --git a/examples/advanced/walkthrough.py b/examples/advanced/walkthrough.py index 52c4d7a3..7740bff7 100644 --- a/examples/advanced/walkthrough.py +++ b/examples/advanced/walkthrough.py @@ -445,6 +445,16 @@ def _run_walkthrough(client): print(f" new_Priority stored as integer: {retrieved.get('new_priority')}") print(f" new_Priority@FormattedValue: {retrieved.get('new_priority@OData.Community.Display.V1.FormattedValue')}") + # Update with a string label + log_call(f"client.records.update('{table_name}', label_id, {{'new_Priority': 'Low'}})") + backoff(lambda: client.records.update(table_name, label_id, {"new_Priority": "Low"})) + updated_label = backoff(lambda: client.records.get(table_name, label_id)) + print(f"[OK] Updated record with string label 'Low' for new_Priority") + print(f" new_Priority stored as integer: {updated_label.get('new_priority')}") + print( + f" new_Priority@FormattedValue: {updated_label.get('new_priority@OData.Community.Display.V1.FormattedValue')}" + ) + # ============================================================================ # 11. COLUMN MANAGEMENT # ============================================================================ diff --git a/src/PowerPlatform/Dataverse/data/_odata.py b/src/PowerPlatform/Dataverse/data/_odata.py index aca42f3d..a3ccaefe 100644 --- a/src/PowerPlatform/Dataverse/data/_odata.py +++ b/src/PowerPlatform/Dataverse/data/_odata.py @@ -171,8 +171,7 @@ def __init__( self._logical_to_entityset_cache: dict[str, str] = {} # Cache: normalized table_schema_name (lowercase) -> primary id attribute (e.g. accountid) self._logical_primaryid_cache: dict[str, str] = {} - # Picklist label cache: (normalized_table_schema_name, normalized_attribute) -> {'map': {...}, 'ts': epoch_seconds} - self._picklist_label_cache = {} + self._picklist_label_cache: dict[str, dict] = {} self._picklist_cache_ttl_seconds = 3600 # 1 hour TTL @contextmanager @@ -1134,141 +1133,118 @@ def _normalize_picklist_label(self, label: str) -> str: norm = re.sub(r"\s+", " ", norm).strip().lower() return norm - def _optionset_map(self, table_schema_name: str, attr_logical: str) -> Optional[Dict[str, int]]: - """Build or return cached mapping of normalized label -> value for a picklist attribute. - - Returns empty dict if attribute is not a picklist or has no options. Returns None only - for invalid inputs or unexpected metadata parse failures. - - Notes - ----- - - This method calls the Web API twice per attribute so it could have perf impact when there are lots of columns on the entity. - """ - if not table_schema_name or not attr_logical: - return None - # Normalize cache key for case-insensitive lookups - cache_key = (self._normalize_cache_key(table_schema_name), self._normalize_cache_key(attr_logical)) - now = time.time() - entry = self._picklist_label_cache.get(cache_key) - if isinstance(entry, dict) and "map" in entry and (now - entry.get("ts", 0)) < self._picklist_cache_ttl_seconds: - return entry["map"] - - # LogicalNames in Dataverse are stored in lowercase, so we need to lowercase for filters - attr_esc = self._escape_odata_quotes(attr_logical.lower()) - table_schema_name_esc = self._escape_odata_quotes(table_schema_name.lower()) - - # Step 1: lightweight fetch (no expand) to determine attribute type - url_type = ( - f"{self.api}/EntityDefinitions(LogicalName='{table_schema_name_esc}')/Attributes" - f"?$filter=LogicalName eq '{attr_esc}'&$select=LogicalName,AttributeType" - ) - # Retry on 404 (metadata not yet published) before surfacing the error. - r_type = None + def _request_metadata_with_retry(self, method: str, url: str, **kwargs): + """Fetch metadata with retries on transient errors.""" max_attempts = 5 backoff_seconds = 0.4 for attempt in range(1, max_attempts + 1): try: - r_type = self._request("get", url_type) - break + return self._request(method, url, **kwargs) except HttpError as err: if getattr(err, "status_code", None) == 404: if attempt < max_attempts: - # Exponential backoff: 0.4s, 0.8s, 1.6s, 3.2s time.sleep(backoff_seconds * (2 ** (attempt - 1))) continue - raise RuntimeError( - f"Picklist attribute metadata not found after retries: entity='{table_schema_name}' attribute='{attr_logical}' (404)" - ) from err + raise RuntimeError(f"Metadata request failed after {max_attempts} retries (404): {url}") from err raise - if r_type is None: - raise RuntimeError("Failed to retrieve attribute metadata due to repeated request failures.") - body_type = r_type.json() - items = body_type.get("value", []) if isinstance(body_type, dict) else [] - if not items: - return None - attr_md = items[0] - if attr_md.get("AttributeType") not in ("Picklist", "PickList"): - self._picklist_label_cache[cache_key] = {"map": {}, "ts": now} - return {} - - # Step 2: fetch with expand only now that we know it's a picklist - # Need to cast to the derived PicklistAttributeMetadata type; OptionSet is not a nav on base AttributeMetadata. - cast_url = ( - f"{self.api}/EntityDefinitions(LogicalName='{table_schema_name_esc}')/Attributes(LogicalName='{attr_esc}')/" - "Microsoft.Dynamics.CRM.PicklistAttributeMetadata?$select=LogicalName&$expand=OptionSet($select=Options)" + def _bulk_fetch_picklists(self, table_schema_name: str) -> None: + """Fetch all picklist attributes and their options for a table in one API call. + + Uses collection-level PicklistAttributeMetadata cast to retrieve every picklist + attribute on the table, including its OptionSet options. Populates the nested + cache so that ``_convert_labels_to_ints`` resolves labels without further API calls. + The Dataverse metadata API does not page results. + """ + table_key = self._normalize_cache_key(table_schema_name) + now = time.time() + table_entry = self._picklist_label_cache.get(table_key) + if isinstance(table_entry, dict) and (now - table_entry.get("ts", 0)) < self._picklist_cache_ttl_seconds: + return + + table_esc = self._escape_odata_quotes(table_schema_name.lower()) + url = ( + f"{self.api}/EntityDefinitions(LogicalName='{table_esc}')" + f"/Attributes/Microsoft.Dynamics.CRM.PicklistAttributeMetadata" + f"?$select=LogicalName&$expand=OptionSet($select=Options)" ) - # Step 2 fetch with retries: expanded OptionSet (cast form first) - r_opts = None - for attempt in range(1, max_attempts + 1): - try: - r_opts = self._request("get", cast_url) - break - except HttpError as err: - if getattr(err, "status_code", None) == 404: - if attempt < max_attempts: - time.sleep(backoff_seconds * (2 ** (attempt - 1))) - continue - raise RuntimeError( - f"Picklist OptionSet metadata not found after retries: entity='{table_schema_name}' attribute='{attr_logical}' (404)" - ) from err - raise - if r_opts is None: - raise RuntimeError("Failed to retrieve picklist OptionSet metadata due to repeated request failures.") + response = self._request_metadata_with_retry("get", url) + body = response.json() + items = body.get("value", []) if isinstance(body, dict) else [] - attr_full = {} - try: - attr_full = r_opts.json() if r_opts.text else {} - except ValueError: - return None - option_set = attr_full.get("OptionSet") or {} - options = option_set.get("Options") if isinstance(option_set, dict) else None - if not isinstance(options, list): - return None - mapping: Dict[str, int] = {} - for opt in options: - if not isinstance(opt, dict): + picklists: Dict[str, Dict[str, int]] = {} + for item in items: + if not isinstance(item, dict): continue - val = opt.get("Value") - if not isinstance(val, int): + ln = item.get("LogicalName", "").lower() + if not ln: continue - label_def = opt.get("Label") or {} - locs = label_def.get("LocalizedLabels") - if isinstance(locs, list): - for loc in locs: - if isinstance(loc, dict): - lab = loc.get("Label") - if isinstance(lab, str) and lab.strip(): - normalized = self._normalize_picklist_label(lab) - mapping.setdefault(normalized, val) - if mapping: - self._picklist_label_cache[cache_key] = {"map": mapping, "ts": now} - return mapping - # No options available - self._picklist_label_cache[cache_key] = {"map": {}, "ts": now} - return {} + option_set = item.get("OptionSet") or {} + options = option_set.get("Options") if isinstance(option_set, dict) else None + mapping: Dict[str, int] = {} + if isinstance(options, list): + for opt in options: + if not isinstance(opt, dict): + continue + val = opt.get("Value") + if not isinstance(val, int): + continue + label_def = opt.get("Label") or {} + locs = label_def.get("LocalizedLabels") + if isinstance(locs, list): + for loc in locs: + if isinstance(loc, dict): + lab = loc.get("Label") + if isinstance(lab, str) and lab.strip(): + normalized = self._normalize_picklist_label(lab) + mapping.setdefault(normalized, val) + picklists[ln] = mapping + + self._picklist_label_cache[table_key] = {"ts": now, "picklists": picklists} def _convert_labels_to_ints(self, table_schema_name: str, record: Dict[str, Any]) -> Dict[str, Any]: """Return a copy of record with any labels converted to option ints. Heuristic: For each string value, attempt to resolve against picklist metadata. If attribute isn't a picklist or label not found, value left unchanged. + + On first encounter of a table, bulk-fetches all picklist attributes and + their options in a single API call, then resolves labels from the warm cache. """ - out = record.copy() - for k, v in list(out.items()): + resolved_record = record.copy() + + # Check if there are any string-valued candidates worth resolving + has_candidates = any( + isinstance(v, str) and v.strip() and isinstance(k, str) and "@odata." not in k + for k, v in resolved_record.items() + ) + if not has_candidates: + return resolved_record + + # Bulk-fetch all picklists for this table (1 API call, cached for TTL) + self._bulk_fetch_picklists(table_schema_name) + + # Resolve labels from the nested cache + table_key = self._normalize_cache_key(table_schema_name) + table_entry = self._picklist_label_cache.get(table_key) + if not isinstance(table_entry, dict): + return resolved_record + picklists = table_entry.get("picklists", {}) + + for k, v in resolved_record.items(): if not isinstance(v, str) or not v.strip(): continue - # Skip OData annotations — they are not attribute names if isinstance(k, str) and "@odata." in k: continue - mapping = self._optionset_map(table_schema_name, k) - if not mapping: + attr_key = self._normalize_cache_key(k) + mapping = picklists.get(attr_key) + if not isinstance(mapping, dict) or not mapping: continue norm = self._normalize_picklist_label(v) val = mapping.get(norm) if val is not None: - out[k] = val - return out + resolved_record[k] = val + return resolved_record def _attribute_payload( self, column_schema_name: str, dtype: Any, *, is_primary_name: bool = False diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index b5b4dd79..2ed49791 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -3,7 +3,7 @@ import json import unittest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from PowerPlatform.Dataverse.core.errors import ValidationError from PowerPlatform.Dataverse.data._odata import _ODataClient @@ -494,23 +494,24 @@ def test_odata_bind_keys_preserve_case(self): def test_convert_labels_skips_odata_keys(self): """_convert_labels_to_ints should skip @odata.bind keys (no metadata lookup).""" - # Patch _optionset_map to track calls - calls = [] - original = self.od._optionset_map + import time - def tracking_optionset_map(table, attr): - calls.append(attr) - return original(table, attr) + # Pre-populate cache so no API call needed + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {}, + } - self.od._optionset_map = tracking_optionset_map record = { "name": "Contoso", "new_CustomerId@odata.bind": "/contacts(00000000-0000-0000-0000-000000000001)", "@odata.type": "Microsoft.Dynamics.CRM.account", } - self.od._convert_labels_to_ints("account", record) - # Only "name" should be checked, not the @odata keys - self.assertEqual(calls, ["name"]) + result = self.od._convert_labels_to_ints("account", record) + # @odata keys must be left unchanged + self.assertEqual(result["new_CustomerId@odata.bind"], "/contacts(00000000-0000-0000-0000-000000000001)") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.account") + self.assertEqual(result["name"], "Contoso") def test_returns_none(self): """_upsert always returns None.""" @@ -518,6 +519,758 @@ def test_returns_none(self): self.assertIsNone(result) +class TestPicklistLabelResolution(unittest.TestCase): + """Tests for picklist label-to-integer resolution. + + Covers _bulk_fetch_picklists, _request_metadata_with_retry, + _convert_labels_to_ints, and their integration through _create / _update / _upsert. + + Cache structure (nested): + _picklist_label_cache = { + "table_key": {"ts": epoch, "picklists": {"attr": {norm_label: int}}} + } + """ + + def setUp(self): + self.od = _make_odata_client() + + # ---- Helper to build a bulk-fetch API response ---- + @staticmethod + def _bulk_response(*picklists): + """Build a mock response for _bulk_fetch_picklists. + + Each picklist is (logical_name, [(value, label), ...]). + """ + items = [] + for ln, options in picklists: + opts = [{"Value": val, "Label": {"LocalizedLabels": [{"Label": lab}]}} for val, lab in options] + items.append({"LogicalName": ln, "OptionSet": {"Options": opts}}) + resp = MagicMock() + resp.json.return_value = {"value": items} + return resp + + # ---- _bulk_fetch_picklists ---- + + def test_bulk_fetch_populates_nested_cache(self): + """Bulk fetch stores picklists in nested {table: {ts, picklists: {...}}} format.""" + import time + + resp = self._bulk_response( + ("industrycode", [(6, "Technology"), (12, "Consulting")]), + ) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + entry = self.od._picklist_label_cache.get("account") + self.assertIsNotNone(entry) + self.assertIn("ts", entry) + self.assertIn("picklists", entry) + self.assertEqual(entry["picklists"]["industrycode"], {"technology": 6, "consulting": 12}) + + def test_bulk_fetch_multiple_picklists(self): + """Multiple picklist attributes are all stored under the same table entry.""" + resp = self._bulk_response( + ("industrycode", [(6, "Technology")]), + ("statuscode", [(1, "Active"), (2, "Inactive")]), + ) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + picklists = self.od._picklist_label_cache["account"]["picklists"] + self.assertEqual(picklists["industrycode"], {"technology": 6}) + self.assertEqual(picklists["statuscode"], {"active": 1, "inactive": 2}) + + def test_bulk_fetch_no_picklists_caches_empty(self): + """Table with no picklist attributes gets cached with empty picklists dict.""" + resp = MagicMock() + resp.json.return_value = {"value": []} + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + entry = self.od._picklist_label_cache.get("account") + self.assertIsNotNone(entry) + self.assertEqual(entry["picklists"], {}) + + def test_bulk_fetch_skips_when_cache_fresh(self): + """Warm cache within TTL should skip the API call.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {"industrycode": {"technology": 6}}, + } + + self.od._bulk_fetch_picklists("account") + self.od._request.assert_not_called() + + def test_bulk_fetch_refreshes_when_cache_expired(self): + """Expired cache should trigger a new API call.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time() - 7200, # 2 hours ago, beyond 1h TTL + "picklists": {"industrycode": {"technology": 6}}, + } + + resp = self._bulk_response(("industrycode", [(6, "Tech"), (12, "Consulting")])) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + self.od._request.assert_called_once() + self.assertEqual( + self.od._picklist_label_cache["account"]["picklists"]["industrycode"], + {"tech": 6, "consulting": 12}, + ) + + def test_bulk_fetch_case_insensitive_table_key(self): + """Table key is normalized to lowercase.""" + resp = self._bulk_response(("industrycode", [(6, "Tech")])) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("Account") + + self.assertIn("account", self.od._picklist_label_cache) + self.assertNotIn("Account", self.od._picklist_label_cache) + + def test_bulk_fetch_uses_picklist_cast_url(self): + """API call uses PicklistAttributeMetadata cast segment.""" + resp = self._bulk_response() + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + call_url = self.od._request.call_args.args[1] + self.assertIn("PicklistAttributeMetadata", call_url) + self.assertIn("OptionSet", call_url) + + def test_bulk_fetch_makes_single_api_call(self): + """Bulk fetch uses exactly one API call regardless of picklist count.""" + resp = self._bulk_response( + ("a", [(1, "X")]), + ("b", [(2, "Y")]), + ("c", [(3, "Z")]), + ) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + self.assertEqual(self.od._request.call_count, 1) + + def test_bulk_fetch_stress_large_workload(self): + """Bulk fetch correctly parses a response with a large number of picklist attributes.""" + num_picklists = 5000 + picklists = [(f"new_pick{i}", [(100000000 + j, f"Option {j}") for j in range(4)]) for i in range(num_picklists)] + resp = self._bulk_response(*picklists) + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + self.assertEqual(self.od._request.call_count, 1) + cached = self.od._picklist_label_cache["account"]["picklists"] + self.assertEqual(len(cached), num_picklists) + self.assertEqual(cached["new_pick0"]["option 0"], 100000000) + self.assertEqual(cached[f"new_pick{num_picklists - 1}"]["option 3"], 100000003) + + # ---- _request_metadata_with_retry ---- + + def test_retry_succeeds_on_first_try(self): + """No retry needed when first call succeeds.""" + mock_resp = MagicMock() + self.od._request.return_value = mock_resp + + result = self.od._request_metadata_with_retry("get", "https://example.com/test") + self.assertIs(result, mock_resp) + self.assertEqual(self.od._request.call_count, 1) + + @patch("PowerPlatform.Dataverse.data._odata.time.sleep") + def test_retry_retries_on_404(self, mock_sleep): + """Should retry on 404 and succeed on later attempt.""" + from PowerPlatform.Dataverse.core.errors import HttpError + + err_404 = HttpError("Not Found", status_code=404) + mock_resp = MagicMock() + self.od._request.side_effect = [err_404, mock_resp] + + result = self.od._request_metadata_with_retry("get", "https://example.com/test") + self.assertIs(result, mock_resp) + self.assertEqual(self.od._request.call_count, 2) + mock_sleep.assert_called_once() + + @patch("PowerPlatform.Dataverse.data._odata.time.sleep") + def test_retry_raises_after_max_attempts(self, mock_sleep): + """Should raise RuntimeError after all retries exhausted.""" + from PowerPlatform.Dataverse.core.errors import HttpError + + err_404 = HttpError("Not Found", status_code=404) + self.od._request.side_effect = err_404 + + with self.assertRaises(RuntimeError) as ctx: + self.od._request_metadata_with_retry("get", "https://example.com/test") + self.assertIn("404", str(ctx.exception)) + self.assertTrue(mock_sleep.called) + + def test_retry_does_not_retry_non_404(self): + """Non-404 errors should be raised immediately without retry.""" + from PowerPlatform.Dataverse.core.errors import HttpError + + err_500 = HttpError("Server Error", status_code=500) + self.od._request.side_effect = err_500 + + with self.assertRaises(HttpError): + self.od._request_metadata_with_retry("get", "https://example.com/test") + self.assertEqual(self.od._request.call_count, 1) + + # ---- _convert_labels_to_ints ---- + + def test_convert_no_string_values_skips_fetch(self): + """Record with no string values should not trigger any API call.""" + record = {"quantity": 5, "amount": 99.99, "completed": False} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result, record) + self.od._request.assert_not_called() + + def test_convert_empty_record_returns_copy(self): + """Empty record returns empty dict without API calls.""" + result = self.od._convert_labels_to_ints("account", {}) + self.assertEqual(result, {}) + self.od._request.assert_not_called() + + def test_convert_whitespace_only_string_skipped(self): + """String values that are only whitespace should not be candidates.""" + record = {"name": " ", "description": ""} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result, record) + self.od._request.assert_not_called() + + def test_convert_odata_keys_skipped(self): + """@odata.bind keys must not be resolved.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {}, + } + + record = { + "name": "Contoso", + "new_CustomerId@odata.bind": "/contacts(guid)", + "@odata.type": "Microsoft.Dynamics.CRM.account", + } + result = self.od._convert_labels_to_ints("account", record) + # @odata keys left unchanged + self.assertEqual(result["new_CustomerId@odata.bind"], "/contacts(guid)") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.account") + + def test_convert_warm_cache_no_api_calls(self): + """Warm cache should resolve labels without any API calls.""" + import time + + now = time.time() + self.od._picklist_label_cache["account"] = { + "ts": now, + "picklists": { + "industrycode": {"technology": 6}, + }, + } + + record = {"name": "Contoso", "industrycode": "Technology"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["name"], "Contoso") + self.od._request.assert_not_called() + + def test_convert_resolves_picklist_label_to_int(self): + """Full flow: bulk fetch returns picklists, label resolved to int.""" + resp = self._bulk_response( + ("industrycode", [(6, "Technology")]), + ) + self.od._request.return_value = resp + + record = {"name": "Contoso", "industrycode": "Technology"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["name"], "Contoso") + # Single bulk fetch call + self.assertEqual(self.od._request.call_count, 1) + + def test_convert_non_picklist_leaves_string_unchanged(self): + """Non-picklist string fields are left as strings (no picklist entry in cache).""" + resp = self._bulk_response() # no picklists on table + self.od._request.return_value = resp + + record = {"name": "Contoso", "telephone1": "555-0100"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["name"], "Contoso") + self.assertEqual(result["telephone1"], "555-0100") + + def test_convert_unmatched_label_left_unchanged(self): + """If a picklist label doesn't match any option, value stays as string.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": { + "industrycode": {"technology": 6, "consulting": 12}, + }, + } + + record = {"industrycode": "UnknownIndustry"} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result["industrycode"], "UnknownIndustry") + + def test_convert_does_not_mutate_original_record(self): + """_convert_labels_to_ints must return a copy, not mutate the input.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {"industrycode": {"technology": 6}}, + } + + original = {"industrycode": "Technology"} + result = self.od._convert_labels_to_ints("account", original) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(original["industrycode"], "Technology") + + def test_convert_multiple_picklists_in_one_record(self): + """Multiple picklist fields in the same record are all resolved.""" + resp = self._bulk_response( + ("industrycode", [(6, "Tech")]), + ("statuscode", [(1, "Active")]), + ) + self.od._request.return_value = resp + + record = {"industrycode": "Tech", "statuscode": "Active"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["statuscode"], 1) + # Single bulk fetch call + self.assertEqual(self.od._request.call_count, 1) + + def test_convert_mixed_picklists_and_non_picklists(self): + """Picklists resolved, non-picklist strings left unchanged, 1 API call.""" + resp = self._bulk_response( + ("industrycode", [(6, "Tech")]), + ("statuscode", [(1, "Active")]), + ) + self.od._request.return_value = resp + + record = { + "name": "Contoso", + "industrycode": "Tech", + "description": "A company", + "statuscode": "Active", + } + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["statuscode"], 1) + self.assertEqual(result["name"], "Contoso") + self.assertEqual(result["description"], "A company") + self.assertEqual(self.od._request.call_count, 1) + + def test_convert_all_non_picklist_makes_one_api_call(self): + """All non-picklist string fields: 1 bulk fetch call, labels unchanged.""" + resp = self._bulk_response() # no picklists + self.od._request.return_value = resp + + record = {"name": "Contoso", "description": "A company", "telephone1": "555-0100"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(self.od._request.call_count, 1) + self.assertEqual(result["name"], "Contoso") + + def test_convert_no_string_values_makes_zero_api_calls(self): + """All non-string values: 0 API calls total.""" + record = {"revenue": 1000000, "quantity": 5, "active": True} + self.od._convert_labels_to_ints("account", record) + + self.assertEqual(self.od._request.call_count, 0) + + def test_convert_bulk_fetch_failure_propagates(self): + """Server error during bulk fetch propagates to caller.""" + from PowerPlatform.Dataverse.core.errors import HttpError + + self.od._request.side_effect = HttpError("Server Error", status_code=500) + + with self.assertRaises(HttpError): + self.od._convert_labels_to_ints("account", {"name": "Contoso"}) + + def test_convert_single_picklist_makes_one_api_call(self): + """Single picklist field (cold cache): 1 bulk fetch total.""" + resp = self._bulk_response(("industrycode", [(6, "Tech")])) + self.od._request.return_value = resp + + record = {"industrycode": "Tech"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(self.od._request.call_count, 1) + + def test_convert_integer_values_passed_through(self): + """Integer values (already resolved) are left unchanged.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {"industrycode": {"technology": 6}}, + } + + record = {"industrycode": 6, "name": "Contoso"} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result["industrycode"], 6) + + def test_convert_case_insensitive_label_matching(self): + """Picklist label matching is case-insensitive.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": {"industrycode": {"technology": 6}}, + } + + record = {"industrycode": "TECHNOLOGY"} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result["industrycode"], 6) + + def test_convert_second_call_same_table_no_api(self): + """Second convert call for same table uses cached bulk fetch, no API call.""" + resp = self._bulk_response(("industrycode", [(6, "Tech")])) + self.od._request.return_value = resp + + self.od._convert_labels_to_ints("account", {"industrycode": "Tech"}) + self.assertEqual(self.od._request.call_count, 1) + + # Second call -- cache warm + self.od._request.reset_mock() + result = self.od._convert_labels_to_ints("account", {"industrycode": "Tech"}) + self.assertEqual(result["industrycode"], 6) + self.od._request.assert_not_called() + + def test_convert_different_tables_separate_fetches(self): + """Different tables each get their own bulk fetch.""" + resp1 = self._bulk_response(("industrycode", [(6, "Tech")])) + resp2 = self._bulk_response(("new_status", [(100, "Open")])) + self.od._request.side_effect = [resp1, resp2] + + r1 = self.od._convert_labels_to_ints("account", {"industrycode": "Tech"}) + r2 = self.od._convert_labels_to_ints("new_ticket", {"new_status": "Open"}) + + self.assertEqual(r1["industrycode"], 6) + self.assertEqual(r2["new_status"], 100) + self.assertEqual(self.od._request.call_count, 2) + + def test_convert_only_odata_and_non_strings_skips_fetch(self): + """Record with only @odata keys and non-string values should skip fetch entirely.""" + record = { + "@odata.type": "Microsoft.Dynamics.CRM.account", + "new_CustomerId@odata.bind": "/contacts(guid)", + "quantity": 5, + "active": True, + } + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result, record) + self.od._request.assert_not_called() + + def test_convert_partial_picklist_match(self): + """Some picklists match, some don't -- matched ones resolved, unmatched left as string.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": { + "industrycode": {"technology": 6, "consulting": 12}, + "statuscode": {"active": 1, "inactive": 2}, + }, + } + + record = {"industrycode": "Technology", "statuscode": "UnknownStatus"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["statuscode"], "UnknownStatus") + + def test_convert_mixed_int_and_label_in_same_record(self): + """One picklist already int, another is a label string -- only label resolved.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": { + "industrycode": {"technology": 6}, + "statuscode": {"active": 1}, + }, + } + + record = {"industrycode": 6, "statuscode": "Active"} + result = self.od._convert_labels_to_ints("account", record) + + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["statuscode"], 1) + + def test_convert_same_label_different_picklists(self): + """Same label text in two different picklist columns resolves to different values.""" + import time + + self.od._picklist_label_cache["new_ticket"] = { + "ts": time.time(), + "picklists": { + "new_priority": {"high": 3}, + "new_severity": {"high": 100}, + }, + } + + record = {"new_priority": "High", "new_severity": "High"} + result = self.od._convert_labels_to_ints("new_ticket", record) + + self.assertEqual(result["new_priority"], 3) + self.assertEqual(result["new_severity"], 100) + + def test_convert_picklist_with_empty_options(self): + """Picklist attribute with zero defined options: label stays as string.""" + import time + + self.od._picklist_label_cache["account"] = { + "ts": time.time(), + "picklists": { + "customcode": {}, # picklist exists but has no options + }, + } + + record = {"customcode": "SomeValue"} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result["customcode"], "SomeValue") + + def test_convert_full_realistic_record(self): + """Realistic record: mix of strings, ints, bools, @odata keys, and picklists.""" + resp = self._bulk_response( + ("industrycode", [(6, "Technology"), (12, "Consulting")]), + ("statuscode", [(1, "Active"), (2, "Inactive")]), + ) + self.od._request.return_value = resp + + record = { + "name": "Contoso Ltd", + "industrycode": "Technology", + "statuscode": "Active", + "revenue": 1000000, + "telephone1": "555-0100", + "emailaddress1": "info@contoso.com", + "new_completed": True, + "new_quantity": 42, + "description": "A technology company", + "@odata.type": "Microsoft.Dynamics.CRM.account", + "new_CustomerId@odata.bind": "/contacts(00000000-0000-0000-0000-000000000001)", + } + result = self.od._convert_labels_to_ints("account", record) + + # Picklists resolved + self.assertEqual(result["industrycode"], 6) + self.assertEqual(result["statuscode"], 1) + # Non-picklist strings unchanged + self.assertEqual(result["name"], "Contoso Ltd") + self.assertEqual(result["telephone1"], "555-0100") + self.assertEqual(result["emailaddress1"], "info@contoso.com") + self.assertEqual(result["description"], "A technology company") + # Non-strings unchanged + self.assertEqual(result["revenue"], 1000000) + self.assertEqual(result["new_completed"], True) + self.assertEqual(result["new_quantity"], 42) + # @odata keys unchanged + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.account") + self.assertEqual( + result["new_CustomerId@odata.bind"], + "/contacts(00000000-0000-0000-0000-000000000001)", + ) + self.assertEqual(self.od._request.call_count, 1) + + def test_bulk_fetch_skips_malformed_items(self): + """Bulk fetch ignores items that aren't dicts or lack LogicalName.""" + resp = MagicMock() + resp.json.return_value = { + "value": [ + "not-a-dict", + {"LogicalName": "", "OptionSet": {"Options": []}}, + { + "LogicalName": "industrycode", + "OptionSet": {"Options": [{"Value": 6, "Label": {"LocalizedLabels": [{"Label": "Tech"}]}}]}, + }, + {"no_logical_name_key": True}, + ] + } + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + picklists = self.od._picklist_label_cache["account"]["picklists"] + self.assertEqual(len(picklists), 1) + self.assertEqual(picklists["industrycode"], {"tech": 6}) + + def test_bulk_fetch_first_label_wins_for_same_value(self): + """When multiple localized labels exist, first label wins via setdefault.""" + resp = MagicMock() + resp.json.return_value = { + "value": [ + { + "LogicalName": "industrycode", + "OptionSet": { + "Options": [ + { + "Value": 6, + "Label": { + "LocalizedLabels": [ + {"Label": "Technology"}, + {"Label": "Technologie"}, + ] + }, + } + ] + }, + } + ] + } + self.od._request.return_value = resp + + self.od._bulk_fetch_picklists("account") + + picklists = self.od._picklist_label_cache["account"]["picklists"] + # Both labels should be present, mapping to the same value + self.assertEqual(picklists["industrycode"]["technology"], 6) + self.assertEqual(picklists["industrycode"]["technologie"], 6) + + # ---- Integration: through _create ---- + + def test_create_resolves_picklist_in_payload(self): + """_create resolves a picklist label to its integer in the POST payload.""" + bulk_resp = self._bulk_response( + ("industrycode", [(6, "Technology")]), + ) + post_resp = MagicMock() + post_resp.headers = { + "OData-EntityId": "https://example.crm.dynamics.com/api/data/v9.2/accounts(00000000-0000-0000-0000-000000000001)" + } + self.od._request.side_effect = [bulk_resp, post_resp] + + result = self.od._create("accounts", "account", {"name": "Contoso", "industrycode": "Technology"}) + self.assertEqual(result, "00000000-0000-0000-0000-000000000001") + post_calls = [c for c in self.od._request.call_args_list if c.args[0] == "post"] + payload = json.loads(post_calls[0].kwargs["data"]) + self.assertEqual(payload["industrycode"], 6) + self.assertEqual(payload["name"], "Contoso") + + def test_create_warm_cache_skips_fetch(self): + """_create with warm cache makes only the POST call.""" + import time + + now = time.time() + self.od._picklist_label_cache["account"] = { + "ts": now, + "picklists": {"industrycode": {"technology": 6}}, + } + + post_resp = MagicMock() + post_resp.headers = { + "OData-EntityId": "https://example.crm.dynamics.com/api/data/v9.2/accounts(00000000-0000-0000-0000-000000000001)" + } + self.od._request.return_value = post_resp + + result = self.od._create("accounts", "account", {"name": "Contoso", "industrycode": "Technology"}) + self.assertEqual(result, "00000000-0000-0000-0000-000000000001") + self.assertEqual(self.od._request.call_count, 1) + payload = json.loads(self.od._request.call_args.kwargs["data"]) + self.assertEqual(payload["industrycode"], 6) + + # ---- Integration: through _update ---- + + def test_update_resolves_picklist_in_payload(self): + """_update resolves a picklist label to its integer in the PATCH payload.""" + self.od._entity_set_from_schema_name = MagicMock(return_value="new_tickets") + + bulk_resp = self._bulk_response( + ("new_status", [(100000001, "In Progress")]), + ) + patch_resp = MagicMock() + self.od._request.side_effect = [bulk_resp, patch_resp] + + self.od._update( + "new_ticket", + "00000000-0000-0000-0000-000000000001", + {"new_status": "In Progress"}, + ) + patch_calls = [c for c in self.od._request.call_args_list if c.args[0] == "patch"] + payload = json.loads(patch_calls[0].kwargs["data"]) + self.assertEqual(payload["new_status"], 100000001) + + def test_update_warm_cache_skips_fetch(self): + """_update with warm cache makes only the PATCH call.""" + import time + + self.od._entity_set_from_schema_name = MagicMock(return_value="new_tickets") + self.od._picklist_label_cache["new_ticket"] = { + "ts": time.time(), + "picklists": {"new_status": {"in progress": 100000001}}, + } + + self.od._update( + "new_ticket", + "00000000-0000-0000-0000-000000000001", + {"new_status": "In Progress"}, + ) + self.assertEqual(self.od._request.call_count, 1) + self.assertEqual(self.od._request.call_args.args[0], "patch") + payload = json.loads(self.od._request.call_args.kwargs["data"]) + self.assertEqual(payload["new_status"], 100000001) + + # ---- Integration: through _upsert ---- + + def test_upsert_resolves_picklist_in_payload(self): + """_upsert resolves a picklist label to its integer in the PATCH payload.""" + bulk_resp = self._bulk_response( + ("industrycode", [(6, "Technology")]), + ) + patch_resp = MagicMock() + self.od._request.side_effect = [bulk_resp, patch_resp] + + self.od._upsert( + "accounts", + "account", + {"accountnumber": "ACC-001"}, + {"name": "Contoso", "industrycode": "Technology"}, + ) + patch_calls = [c for c in self.od._request.call_args_list if c.args[0] == "patch"] + payload = patch_calls[0].kwargs["json"] + self.assertEqual(payload["industrycode"], 6) + self.assertEqual(payload["name"], "Contoso") + + def test_upsert_warm_cache_skips_fetch(self): + """_upsert with warm cache makes only the PATCH call.""" + import time + + now = time.time() + self.od._picklist_label_cache["account"] = { + "ts": now, + "picklists": {"industrycode": {"technology": 6}}, + } + + self.od._upsert( + "accounts", + "account", + {"accountnumber": "ACC-001"}, + {"name": "Contoso", "industrycode": "Technology"}, + ) + self.assertEqual(self.od._request.call_count, 1) + patch_calls = [c for c in self.od._request.call_args_list if c.args[0] == "patch"] + payload = patch_calls[0].kwargs["json"] + self.assertEqual(payload["industrycode"], 6) + + class TestBuildUpsertMultiple(unittest.TestCase): """Unit tests for _ODataClient._build_upsert_multiple (batch deferred build).""" diff --git a/tests/unit/test_context_manager.py b/tests/unit/test_context_manager.py index df916583..2f1aab9c 100644 --- a/tests/unit/test_context_manager.py +++ b/tests/unit/test_context_manager.py @@ -158,7 +158,7 @@ def test_close_clears_odata_caches(self): odata = client._get_odata() odata._logical_to_entityset_cache["test"] = "value" odata._logical_primaryid_cache["test"] = "value" - odata._picklist_label_cache[("test", "attr")] = {"map": {}, "ts": 0} + odata._picklist_label_cache["test"] = {"ts": 0, "picklists": {"attr": {}}} client.close()