From bd02348ae4ee06365eccbb3700c19012a099a5ee Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Tue, 7 Apr 2026 09:56:07 -0700 Subject: [PATCH 01/16] Add test coverage and JUnit XML reporting to CI pipelines --- .azdo/ci-pr.yaml | 10 ++++++++-- .github/workflows/python-package.yml | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.azdo/ci-pr.yaml b/.azdo/ci-pr.yaml index eeb64f83..c5d680ca 100644 --- a/.azdo/ci-pr.yaml +++ b/.azdo/ci-pr.yaml @@ -66,12 +66,18 @@ extends: displayName: 'Install wheel' - script: | - pytest + pytest --junitxml=test-results.xml --cov=PowerPlatform --cov-report=xml --cov-report=term-missing displayName: 'Test with pytest' - task: PublishTestResults@2 condition: succeededOrFailed() inputs: - testResultsFiles: '**/test-*.xml' + testResultsFiles: '**/test-results.xml' testRunTitle: 'Python 3.12' displayName: 'Publish test results' + + - task: PublishCodeCoverageResults@2 + condition: succeededOrFailed() + inputs: + summaryFileLocation: '**/coverage.xml' + displayName: 'Publish code coverage' diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 26209bf7..fe8dbc7d 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -51,4 +51,18 @@ jobs: - name: Test with pytest run: | - pytest + pytest --junitxml=test-results.xml --cov=PowerPlatform --cov-report=xml --cov-report=term-missing + + - name: Upload test results + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-results + path: test-results.xml + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage.xml From a9f0b7f6cba031658c0a9f4c89d3cd3de28531c6 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Tue, 7 Apr 2026 13:33:46 -0700 Subject: [PATCH 02/16] Add comprehensive unit tests for file upload (34 tests) --- tests/unit/data/test_upload.py | 412 +++++++++++++++++++++++++++++++++ 1 file changed, 412 insertions(+) create mode 100644 tests/unit/data/test_upload.py diff --git a/tests/unit/data/test_upload.py b/tests/unit/data/test_upload.py new file mode 100644 index 00000000..675dd264 --- /dev/null +++ b/tests/unit/data/test_upload.py @@ -0,0 +1,412 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +import os +import tempfile +import unittest +from unittest.mock import MagicMock + +from PowerPlatform.Dataverse.data._odata import _ODataClient + + +def _make_odata_client() -> _ODataClient: + """Return an _ODataClient with HTTP calls mocked out.""" + mock_auth = MagicMock() + mock_auth._acquire_token.return_value = MagicMock(access_token="token") + client = _ODataClient(mock_auth, "https://example.crm.dynamics.com") + client._request = MagicMock() + return client + + +def _make_temp_file(content: bytes = b"test content", suffix: str = ".bin") -> str: + """Create a temporary file and return its path. Caller must delete.""" + with tempfile.NamedTemporaryFile(suffix=suffix, delete=False) as f: + f.write(content) + return f.name + + +class TestUploadFileOrchestrator(unittest.TestCase): + """Tests for _upload_file() orchestration and mode selection.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-1", "LogicalName": "account"} + ) + self.od._get_attribute_metadata = MagicMock(return_value={"LogicalName": "new_document"}) + + def test_auto_mode_small_file(self): + """Auto mode routes files <128MB to _upload_file_small.""" + path = _make_temp_file() + try: + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="auto") + self.od._upload_file_small.assert_called_once() + finally: + os.unlink(path) + + def test_default_mode_is_auto(self): + """mode=None defaults to auto (small for small files).""" + path = _make_temp_file() + try: + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path) + self.od._upload_file_small.assert_called_once() + finally: + os.unlink(path) + + def test_auto_mode_file_not_found(self): + """Auto mode raises FileNotFoundError for missing file.""" + with self.assertRaises(FileNotFoundError): + self.od._upload_file("account", "guid-1", "new_Document", "/nonexistent/file.pdf") + + def test_explicit_small_mode(self): + """Explicit 'small' mode calls _upload_file_small.""" + path = _make_temp_file() + try: + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._upload_file_small.assert_called_once() + finally: + os.unlink(path) + + def test_explicit_chunk_mode(self): + """Explicit 'chunk' mode calls _upload_file_chunk.""" + path = _make_temp_file() + try: + self.od._upload_file_chunk = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="chunk") + self.od._upload_file_chunk.assert_called_once() + finally: + os.unlink(path) + + def test_invalid_mode_raises(self): + """Invalid mode raises ValueError.""" + path = _make_temp_file() + try: + with self.assertRaises(ValueError) as ctx: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="invalid") + self.assertIn("invalid", str(ctx.exception).lower()) + finally: + os.unlink(path) + + def test_column_auto_creation_when_missing(self): + """Creates file column when attribute metadata not found.""" + self.od._get_attribute_metadata = MagicMock(return_value=None) + self.od._create_columns = MagicMock() + self.od._wait_for_attribute_visibility = MagicMock() + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._create_columns.assert_called_once_with("account", {"new_Document": "file"}) + self.od._wait_for_attribute_visibility.assert_called_once_with("accounts", "new_Document") + finally: + os.unlink(path) + + def test_column_exists_skips_creation(self): + """Does not create column when attribute already exists.""" + self.od._create_columns = MagicMock() + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._create_columns.assert_not_called() + finally: + os.unlink(path) + + def test_no_entity_metadata_skips_column_check(self): + """Skips column check entirely when entity metadata is None.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + self.od._get_attribute_metadata = MagicMock() + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._get_attribute_metadata.assert_not_called() + finally: + os.unlink(path) + + def test_lowercases_attribute_name(self): + """File name attribute is lowercased for URL usage.""" + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + # Third positional arg is the logical_name (lowercased) + self.assertEqual(self.od._upload_file_small.call_args[0][2], "new_document") + finally: + os.unlink(path) + + def test_passes_mime_type_to_small(self): + """mime_type is forwarded as content_type to _upload_file_small.""" + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", mime_type="text/csv") + kwargs = self.od._upload_file_small.call_args[1] + self.assertEqual(kwargs["content_type"], "text/csv") + finally: + os.unlink(path) + + def test_passes_if_none_match_to_small(self): + """if_none_match is forwarded to _upload_file_small.""" + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + try: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", if_none_match=False) + kwargs = self.od._upload_file_small.call_args[1] + self.assertFalse(kwargs["if_none_match"]) + finally: + os.unlink(path) + + +class TestUploadFileSmall(unittest.TestCase): + """Tests for _upload_file_small() single PATCH upload.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_successful_upload(self): + """Sends PATCH with correct URL, headers and file data.""" + path = _make_temp_file(b"PDF file content here", suffix=".pdf") + try: + self.od._upload_file_small("accounts", "guid-1", "new_document", path) + self.od._request.assert_called_once() + args, kwargs = self.od._request.call_args + self.assertEqual(args[0], "patch") + self.assertIn("new_document", args[1]) + self.assertEqual(kwargs["data"], b"PDF file content here") + finally: + os.unlink(path) + + def test_url_contains_entity_set_and_record_id(self): + """URL is constructed from entity_set, record_id, and attribute.""" + path = _make_temp_file() + try: + self.od._upload_file_small("accounts", "guid-1", "new_document", path) + url = self.od._request.call_args[0][1] + self.assertIn("accounts", url) + self.assertIn("guid-1", url) + self.assertIn("new_document", url) + finally: + os.unlink(path) + + def test_if_none_match_header(self): + """if_none_match=True sends If-None-Match: null.""" + path = _make_temp_file() + try: + self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=True) + headers = self.od._request.call_args[1]["headers"] + self.assertEqual(headers["If-None-Match"], "null") + self.assertNotIn("If-Match", headers) + finally: + os.unlink(path) + + def test_if_match_overwrite_header(self): + """if_none_match=False sends If-Match: *.""" + path = _make_temp_file() + try: + self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=False) + headers = self.od._request.call_args[1]["headers"] + self.assertEqual(headers["If-Match"], "*") + self.assertNotIn("If-None-Match", headers) + finally: + os.unlink(path) + + def test_custom_mime_type(self): + """Custom content_type is used in Content-Type header.""" + path = _make_temp_file(b"{}", suffix=".json") + try: + self.od._upload_file_small("accounts", "guid-1", "col", path, content_type="application/json") + headers = self.od._request.call_args[1]["headers"] + self.assertEqual(headers["Content-Type"], "application/json") + finally: + os.unlink(path) + + def test_default_mime_type(self): + """Default Content-Type is application/octet-stream.""" + path = _make_temp_file() + try: + self.od._upload_file_small("accounts", "guid-1", "col", path) + headers = self.od._request.call_args[1]["headers"] + self.assertEqual(headers["Content-Type"], "application/octet-stream") + finally: + os.unlink(path) + + def test_file_not_found_raises(self): + """Raises FileNotFoundError for missing file.""" + with self.assertRaises(FileNotFoundError): + self.od._upload_file_small("accounts", "guid-1", "col", "/no/such/file.txt") + + def test_empty_record_id_raises(self): + """Raises ValueError for empty record_id.""" + with self.assertRaises(ValueError): + self.od._upload_file_small("accounts", "", "col", "/any/path") + + def test_file_name_in_header(self): + """x-ms-file-name header contains the basename of the file.""" + path = _make_temp_file(b"a,b,c", suffix=".csv") + try: + self.od._upload_file_small("accounts", "guid-1", "col", path) + headers = self.od._request.call_args[1]["headers"] + self.assertEqual(headers["x-ms-file-name"], os.path.basename(path)) + finally: + os.unlink(path) + + +class TestUploadFileChunk(unittest.TestCase): + """Tests for _upload_file_chunk() streaming chunked upload.""" + + def setUp(self): + self.od = _make_odata_client() + + def _mock_init_response(self, location="https://example.com/session?token=abc", chunk_size=None): + """Create a mock init PATCH response with Location and optional chunk-size headers.""" + resp = MagicMock() + headers = {"Location": location} + if chunk_size is not None: + headers["x-ms-chunk-size"] = str(chunk_size) + resp.headers = headers + return resp + + def test_init_patch_sends_chunked_header(self): + """Initial PATCH sends x-ms-transfer-mode: chunked.""" + self.od._request.return_value = self._mock_init_response() + path = _make_temp_file(b"x" * 100) + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + init_call = self.od._request.call_args_list[0] + self.assertEqual(init_call[1]["headers"]["x-ms-transfer-mode"], "chunked") + finally: + os.unlink(path) + + def test_init_url_contains_file_name(self): + """Init PATCH URL includes x-ms-file-name query parameter.""" + self.od._request.return_value = self._mock_init_response() + path = _make_temp_file(b"data", suffix=".pdf") + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + init_url = self.od._request.call_args_list[0][0][1] + self.assertIn("x-ms-file-name=", init_url) + finally: + os.unlink(path) + + def test_missing_location_header_raises(self): + """Raises RuntimeError when init response lacks Location header.""" + resp = MagicMock() + resp.headers = {} + self.od._request.return_value = resp + path = _make_temp_file() + try: + with self.assertRaises(RuntimeError) as ctx: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + self.assertIn("Location", str(ctx.exception)) + finally: + os.unlink(path) + + def test_uses_chunk_size_from_response(self): + """Uses x-ms-chunk-size from init response to determine chunk size.""" + self.od._request.return_value = self._mock_init_response(chunk_size=50) + path = _make_temp_file(b"x" * 120) # 120 bytes / 50-byte chunks = 3 chunks + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # 1 init + 3 chunk calls = 4 total + self.assertEqual(self.od._request.call_count, 4) + finally: + os.unlink(path) + + def test_default_chunk_size_when_header_missing(self): + """Falls back to 4MB chunk size when x-ms-chunk-size header missing.""" + self.od._request.return_value = self._mock_init_response() # no chunk_size + path = _make_temp_file(b"x" * 100) # 100 bytes < 4MB = single chunk + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # 1 init + 1 chunk = 2 total + self.assertEqual(self.od._request.call_count, 2) + finally: + os.unlink(path) + + def test_content_range_headers(self): + """Each chunk has correct Content-Range header.""" + self.od._request.return_value = self._mock_init_response(chunk_size=10) + path = _make_temp_file(b"A" * 10 + b"B" * 10 + b"C" * 5) # 25 bytes -> 3 chunks + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_calls = self.od._request.call_args_list[1:] # skip init + self.assertEqual(len(chunk_calls), 3) + self.assertEqual(chunk_calls[0][1]["headers"]["Content-Range"], "bytes 0-9/25") + self.assertEqual(chunk_calls[1][1]["headers"]["Content-Range"], "bytes 10-19/25") + self.assertEqual(chunk_calls[2][1]["headers"]["Content-Range"], "bytes 20-24/25") + finally: + os.unlink(path) + + def test_chunk_content_length_header(self): + """Each chunk includes correct Content-Length header.""" + self.od._request.return_value = self._mock_init_response(chunk_size=10) + path = _make_temp_file(b"A" * 10 + b"B" * 5) # 15 bytes -> 2 chunks (10 + 5) + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_calls = self.od._request.call_args_list[1:] + self.assertEqual(chunk_calls[0][1]["headers"]["Content-Length"], "10") + self.assertEqual(chunk_calls[1][1]["headers"]["Content-Length"], "5") + finally: + os.unlink(path) + + def test_chunk_sends_to_location_url(self): + """Chunk PATCHes go to the Location URL, not the original URL.""" + session_url = "https://example.com/upload?session=xyz" + self.od._request.return_value = self._mock_init_response(location=session_url) + path = _make_temp_file() + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_call = self.od._request.call_args_list[1] + self.assertEqual(chunk_call[0][1], session_url) + finally: + os.unlink(path) + + def test_if_none_match_on_init(self): + """if_none_match=True sends If-None-Match on init PATCH.""" + self.od._request.return_value = self._mock_init_response() + path = _make_temp_file() + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=True) + init_headers = self.od._request.call_args_list[0][1]["headers"] + self.assertEqual(init_headers["If-None-Match"], "null") + self.assertNotIn("If-Match", init_headers) + finally: + os.unlink(path) + + def test_if_match_overwrite_on_init(self): + """if_none_match=False sends If-Match on init PATCH.""" + self.od._request.return_value = self._mock_init_response() + path = _make_temp_file() + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=False) + init_headers = self.od._request.call_args_list[0][1]["headers"] + self.assertEqual(init_headers["If-Match"], "*") + self.assertNotIn("If-None-Match", init_headers) + finally: + os.unlink(path) + + def test_empty_record_id_raises(self): + """Raises ValueError for empty record_id.""" + with self.assertRaises(ValueError): + self.od._upload_file_chunk("accounts", "", "col", "/any/path") + + def test_file_not_found_raises(self): + """Raises FileNotFoundError for missing file.""" + with self.assertRaises(FileNotFoundError): + self.od._upload_file_chunk("accounts", "guid-1", "col", "/no/such/file.bin") + + def test_chunk_requests_accept_206_and_204(self): + """Chunk requests use expected=(206, 204).""" + self.od._request.return_value = self._mock_init_response(chunk_size=50) + path = _make_temp_file(b"x" * 100) + try: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + for chunk_call in self.od._request.call_args_list[1:]: + self.assertEqual(chunk_call[1]["expected"], (206, 204)) + finally: + os.unlink(path) From d2c5cc8cc3bc3a288301de75942a5965920762d1 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Tue, 7 Apr 2026 17:39:13 -0700 Subject: [PATCH 03/16] Add unit test cases for uncovered areas Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/core/test_auth.py | 32 ++ tests/unit/core/test_http_client.py | 123 ++++++++ tests/unit/core/test_http_errors.py | 39 +++ tests/unit/data/test_relationships.py | 13 + tests/unit/data/test_upload.py | 394 +++++++++++++----------- tests/unit/models/test_query_builder.py | 6 + tests/unit/models/test_table_info.py | 6 + tests/unit/test_client.py | 6 + tests/unit/test_context_manager.py | 11 + tests/unit/test_records_operations.py | 46 +++ 10 files changed, 488 insertions(+), 188 deletions(-) create mode 100644 tests/unit/core/test_auth.py create mode 100644 tests/unit/core/test_http_client.py diff --git a/tests/unit/core/test_auth.py b/tests/unit/core/test_auth.py new file mode 100644 index 00000000..93e7bf63 --- /dev/null +++ b/tests/unit/core/test_auth.py @@ -0,0 +1,32 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +import unittest +from unittest.mock import MagicMock + +from azure.core.credentials import TokenCredential + +from PowerPlatform.Dataverse.core._auth import _AuthManager, _TokenPair + + +class TestAuthManager(unittest.TestCase): + """Tests for _AuthManager credential validation and token acquisition.""" + + def test_non_token_credential_raises(self): + """_AuthManager raises TypeError when credential does not implement TokenCredential.""" + with self.assertRaises(TypeError): + _AuthManager("not-a-credential") + + def test_acquire_token_returns_token_pair(self): + """_acquire_token calls get_token and returns a _TokenPair with scope and token.""" + mock_credential = MagicMock(spec=TokenCredential) + mock_credential.get_token.return_value = MagicMock(token="my-access-token") + + manager = _AuthManager(mock_credential) + result = manager._acquire_token("https://org.crm.dynamics.com/.default") + + mock_credential.get_token.assert_called_once_with("https://org.crm.dynamics.com/.default") + self.assertIsInstance(result, _TokenPair) + self.assertEqual(result.resource, "https://org.crm.dynamics.com/.default") + self.assertEqual(result.access_token, "my-access-token") + diff --git a/tests/unit/core/test_http_client.py b/tests/unit/core/test_http_client.py new file mode 100644 index 00000000..d29f8abb --- /dev/null +++ b/tests/unit/core/test_http_client.py @@ -0,0 +1,123 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +import unittest +from unittest.mock import MagicMock, patch, call + +import requests + +from PowerPlatform.Dataverse.core._http import _HttpClient + + +class TestHttpClientTimeout(unittest.TestCase): + """Tests for automatic timeout selection in _HttpClient._request.""" + + def _make_response(self, status=200): + resp = MagicMock(spec=requests.Response) + resp.status_code = status + return resp + + def test_get_uses_10s_default_timeout(self): + """GET requests use 10s default when no timeout is specified.""" + client = _HttpClient(retries=1) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("get", "https://example.com/data") + _, kwargs = mock_req.call_args + self.assertEqual(kwargs["timeout"], 10) + + def test_post_uses_120s_default_timeout(self): + """POST requests use 120s default when no timeout is specified.""" + client = _HttpClient(retries=1) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("post", "https://example.com/data") + _, kwargs = mock_req.call_args + self.assertEqual(kwargs["timeout"], 120) + + def test_delete_uses_120s_default_timeout(self): + """DELETE requests use 120s default when no timeout is specified.""" + client = _HttpClient(retries=1) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("delete", "https://example.com/data") + _, kwargs = mock_req.call_args + self.assertEqual(kwargs["timeout"], 120) + + def test_default_timeout_overrides_per_method_default(self): + """Explicit default_timeout on the client overrides per-method defaults.""" + client = _HttpClient(retries=1, timeout=30.0) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("get", "https://example.com/data") + _, kwargs = mock_req.call_args + self.assertEqual(kwargs["timeout"], 30.0) + + def test_explicit_timeout_in_kwargs_is_not_overridden(self): + """If timeout is already in kwargs it is passed through unchanged.""" + client = _HttpClient(retries=1, timeout=30.0) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("get", "https://example.com/data", timeout=5) + _, kwargs = mock_req.call_args + self.assertEqual(kwargs["timeout"], 5) + + +class TestHttpClientRequester(unittest.TestCase): + """Tests for session vs direct requests.request routing.""" + + def _make_response(self): + resp = MagicMock(spec=requests.Response) + resp.status_code = 200 + return resp + + def test_uses_requests_request_when_no_session(self): + """Without a session, _request uses requests.request directly.""" + client = _HttpClient(retries=1) + with patch("requests.request", return_value=self._make_response()) as mock_req: + client._request("get", "https://example.com/data") + mock_req.assert_called_once() + + def test_uses_session_request_when_session_provided(self): + """With a session, _request uses session.request instead of requests.request.""" + mock_session = MagicMock(spec=requests.Session) + mock_session.request.return_value = self._make_response() + client = _HttpClient(retries=1, session=mock_session) + with patch("requests.request") as mock_req: + client._request("get", "https://example.com/data") + mock_session.request.assert_called_once() + mock_req.assert_not_called() + + +class TestHttpClientRetry(unittest.TestCase): + """Tests for retry behavior on RequestException.""" + + def test_retries_on_request_exception_and_succeeds(self): + """Retries after a RequestException and returns response on second attempt.""" + resp = MagicMock(spec=requests.Response) + resp.status_code = 200 + client = _HttpClient(retries=2, backoff=0) + with patch("requests.request", side_effect=[requests.exceptions.ConnectionError(), resp]) as mock_req: + with patch("time.sleep"): + result = client._request("get", "https://example.com/data") + self.assertEqual(mock_req.call_count, 2) + self.assertIs(result, resp) + + def test_raises_after_all_retries_exhausted(self): + """Raises RequestException after all retry attempts fail.""" + client = _HttpClient(retries=3, backoff=0) + with patch("requests.request", side_effect=requests.exceptions.ConnectionError("timeout")): + with patch("time.sleep"): + with self.assertRaises(requests.exceptions.RequestException): + client._request("get", "https://example.com/data") + + def test_backoff_delay_between_retries(self): + """Sleeps with exponential backoff between retry attempts.""" + resp = MagicMock(spec=requests.Response) + resp.status_code = 200 + client = _HttpClient(retries=3, backoff=1.0) + side_effects = [ + requests.exceptions.ConnectionError(), + requests.exceptions.ConnectionError(), + resp, + ] + with patch("requests.request", side_effect=side_effects): + with patch("time.sleep") as mock_sleep: + client._request("get", "https://example.com/data") + # First retry: delay = 1.0 * 2^0 = 1.0, second retry: 1.0 * 2^1 = 2.0 + mock_sleep.assert_has_calls([call(1.0), call(2.0)]) diff --git a/tests/unit/core/test_http_errors.py b/tests/unit/core/test_http_errors.py index 729ebae3..a5bbd0cb 100644 --- a/tests/unit/core/test_http_errors.py +++ b/tests/unit/core/test_http_errors.py @@ -180,3 +180,42 @@ def test_correlation_id_shared_inside_call_scope(): h1, h2 = recorder.recorded_headers assert h1["x-ms-client-request-id"] != h2["x-ms-client-request-id"] assert h1["x-ms-correlation-id"] == h2["x-ms-correlation-id"] + + +# --- ValidationError / SQLParseError / HttpError optional fields --- + +def test_validation_error_instantiates(): + """ValidationError can be raised and carries the correct code.""" + from PowerPlatform.Dataverse.core.errors import ValidationError + + err = ValidationError("bad input", subcode="missing_field", details={"field": "name"}) + assert err.code == "validation_error" + assert err.subcode == "missing_field" + assert err.details["field"] == "name" + assert err.source == "client" + + +def test_sql_parse_error_instantiates(): + """SQLParseError can be raised and carries the correct code.""" + from PowerPlatform.Dataverse.core.errors import SQLParseError + + err = SQLParseError("unexpected token", subcode="syntax_error") + assert err.code == "sql_parse_error" + assert err.subcode == "syntax_error" + assert err.source == "client" + + +def test_http_error_optional_diagnostic_fields(): + """HttpError stores correlation_id, service_request_id, and traceparent in details.""" + from PowerPlatform.Dataverse.core.errors import HttpError + + err = HttpError( + "Server error", + status_code=500, + correlation_id="corr-123", + service_request_id="svc-456", + traceparent="00-abc-def-01", + ) + assert err.details["correlation_id"] == "corr-123" + assert err.details["service_request_id"] == "svc-456" + assert err.details["traceparent"] == "00-abc-def-01" diff --git a/tests/unit/data/test_relationships.py b/tests/unit/data/test_relationships.py index c67b3f1e..6636b184 100644 --- a/tests/unit/data/test_relationships.py +++ b/tests/unit/data/test_relationships.py @@ -196,6 +196,19 @@ def test_create_m2m_relationship_returns_result(self): self.assertEqual(result["entity1_logical_name"], "account") self.assertEqual(result["entity2_logical_name"], "contact") + def test_create_m2m_relationship_with_solution(self): + """Solution name is added as MSCRM.SolutionUniqueName header.""" + mock_response = Mock() + mock_response.headers = { + "OData-EntityId": "https://example.crm.dynamics.com/api/data/v9.2/RelationshipDefinitions(abcd1234-abcd-1234-abcd-1234abcd5678)" + } + self.client._mock_request.return_value = mock_response + + self.client._create_many_to_many_relationship(self.relationship, solution="MySolution") + + headers = self.client._mock_request.call_args.kwargs["headers"] + self.assertEqual(headers["MSCRM.SolutionUniqueName"], "MySolution") + class TestDeleteRelationship(unittest.TestCase): """Tests for _delete_relationship method.""" diff --git a/tests/unit/data/test_upload.py b/tests/unit/data/test_upload.py index 675dd264..2cf3b751 100644 --- a/tests/unit/data/test_upload.py +++ b/tests/unit/data/test_upload.py @@ -4,7 +4,7 @@ import os import tempfile import unittest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from PowerPlatform.Dataverse.data._odata import _ODataClient @@ -25,8 +25,8 @@ def _make_temp_file(content: bytes = b"test content", suffix: str = ".bin") -> s return f.name -class TestUploadFileOrchestrator(unittest.TestCase): - """Tests for _upload_file() orchestration and mode selection.""" +class TestUploadFile(unittest.TestCase): + """Tests for _upload_file() mode selection, column auto-creation, and argument forwarding.""" def setUp(self): self.od = _make_odata_client() @@ -39,22 +39,27 @@ def setUp(self): def test_auto_mode_small_file(self): """Auto mode routes files <128MB to _upload_file_small.""" path = _make_temp_file() - try: - self.od._upload_file_small = MagicMock() + self.addCleanup(os.unlink, path) + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="auto") + self.od._upload_file_small.assert_called_once() + + def test_auto_mode_large_file_routes_to_chunk(self): + """Auto mode routes files >=128MB to _upload_file_chunk.""" + path = _make_temp_file() + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk = MagicMock() + with patch("os.path.getsize", return_value=128 * 1024 * 1024): self.od._upload_file("account", "guid-1", "new_Document", path, mode="auto") - self.od._upload_file_small.assert_called_once() - finally: - os.unlink(path) + self.od._upload_file_chunk.assert_called_once() def test_default_mode_is_auto(self): - """mode=None defaults to auto (small for small files).""" + """mode=None is treated as 'auto'.""" path = _make_temp_file() - try: - self.od._upload_file_small = MagicMock() - self.od._upload_file("account", "guid-1", "new_Document", path) - self.od._upload_file_small.assert_called_once() - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path) + self.od._upload_file_small.assert_called_once() def test_auto_mode_file_not_found(self): """Auto mode raises FileNotFoundError for missing file.""" @@ -64,32 +69,26 @@ def test_auto_mode_file_not_found(self): def test_explicit_small_mode(self): """Explicit 'small' mode calls _upload_file_small.""" path = _make_temp_file() - try: - self.od._upload_file_small = MagicMock() - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") - self.od._upload_file_small.assert_called_once() - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._upload_file_small.assert_called_once() def test_explicit_chunk_mode(self): """Explicit 'chunk' mode calls _upload_file_chunk.""" path = _make_temp_file() - try: - self.od._upload_file_chunk = MagicMock() - self.od._upload_file("account", "guid-1", "new_Document", path, mode="chunk") - self.od._upload_file_chunk.assert_called_once() - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk = MagicMock() + self.od._upload_file("account", "guid-1", "new_Document", path, mode="chunk") + self.od._upload_file_chunk.assert_called_once() def test_invalid_mode_raises(self): """Invalid mode raises ValueError.""" path = _make_temp_file() - try: - with self.assertRaises(ValueError) as ctx: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="invalid") - self.assertIn("invalid", str(ctx.exception).lower()) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + with self.assertRaises(ValueError) as ctx: + self.od._upload_file("account", "guid-1", "new_Document", path, mode="invalid") + self.assertIn("invalid", str(ctx.exception).lower()) def test_column_auto_creation_when_missing(self): """Creates file column when attribute metadata not found.""" @@ -98,23 +97,19 @@ def test_column_auto_creation_when_missing(self): self.od._wait_for_attribute_visibility = MagicMock() self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") - self.od._create_columns.assert_called_once_with("account", {"new_Document": "file"}) - self.od._wait_for_attribute_visibility.assert_called_once_with("accounts", "new_Document") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._create_columns.assert_called_once_with("account", {"new_Document": "file"}) + self.od._wait_for_attribute_visibility.assert_called_once_with("accounts", "new_Document") def test_column_exists_skips_creation(self): """Does not create column when attribute already exists.""" self.od._create_columns = MagicMock() self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") - self.od._create_columns.assert_not_called() - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._create_columns.assert_not_called() def test_no_entity_metadata_skips_column_check(self): """Skips column check entirely when entity metadata is None.""" @@ -122,44 +117,52 @@ def test_no_entity_metadata_skips_column_check(self): self.od._get_attribute_metadata = MagicMock() self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") - self.od._get_attribute_metadata.assert_not_called() - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._get_attribute_metadata.assert_not_called() + + def test_entity_metadata_without_metadata_id_skips_column_check(self): + """Skips attribute check when entity metadata has no MetadataId.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value={"LogicalName": "account"}) + self.od._get_attribute_metadata = MagicMock() + self.od._upload_file_small = MagicMock() + path = _make_temp_file() + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + self.od._get_attribute_metadata.assert_not_called() def test_lowercases_attribute_name(self): """File name attribute is lowercased for URL usage.""" self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") - # Third positional arg is the logical_name (lowercased) - self.assertEqual(self.od._upload_file_small.call_args[0][2], "new_document") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small") + # Third positional arg to _upload_file_small is the logical_name (lowercased) + self.assertEqual(self.od._upload_file_small.call_args.args[2], "new_document") def test_passes_mime_type_to_small(self): """mime_type is forwarded as content_type to _upload_file_small.""" self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", mime_type="text/csv") - kwargs = self.od._upload_file_small.call_args[1] - self.assertEqual(kwargs["content_type"], "text/csv") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", mime_type="text/csv") + self.assertEqual(self.od._upload_file_small.call_args.kwargs["content_type"], "text/csv") def test_passes_if_none_match_to_small(self): """if_none_match is forwarded to _upload_file_small.""" self.od._upload_file_small = MagicMock() path = _make_temp_file() - try: - self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", if_none_match=False) - kwargs = self.od._upload_file_small.call_args[1] - self.assertFalse(kwargs["if_none_match"]) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="small", if_none_match=False) + self.assertFalse(self.od._upload_file_small.call_args.kwargs["if_none_match"]) + + def test_passes_if_none_match_to_chunk(self): + """if_none_match is forwarded to _upload_file_chunk.""" + self.od._upload_file_chunk = MagicMock() + path = _make_temp_file() + self.addCleanup(os.unlink, path) + self.od._upload_file("account", "guid-1", "new_Document", path, mode="chunk", if_none_match=False) + self.assertFalse(self.od._upload_file_chunk.call_args.kwargs["if_none_match"]) class TestUploadFileSmall(unittest.TestCase): @@ -171,69 +174,57 @@ def setUp(self): def test_successful_upload(self): """Sends PATCH with correct URL, headers and file data.""" path = _make_temp_file(b"PDF file content here", suffix=".pdf") - try: - self.od._upload_file_small("accounts", "guid-1", "new_document", path) - self.od._request.assert_called_once() - args, kwargs = self.od._request.call_args - self.assertEqual(args[0], "patch") - self.assertIn("new_document", args[1]) - self.assertEqual(kwargs["data"], b"PDF file content here") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "new_document", path) + self.od._request.assert_called_once() + call = self.od._request.call_args + self.assertEqual(call.args[0], "patch") + self.assertIn("new_document", call.args[1]) + self.assertEqual(call.kwargs["data"], b"PDF file content here") def test_url_contains_entity_set_and_record_id(self): """URL is constructed from entity_set, record_id, and attribute.""" path = _make_temp_file() - try: - self.od._upload_file_small("accounts", "guid-1", "new_document", path) - url = self.od._request.call_args[0][1] - self.assertIn("accounts", url) - self.assertIn("guid-1", url) - self.assertIn("new_document", url) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "new_document", path) + url = self.od._request.call_args.args[1] + self.assertIn("accounts", url) + self.assertIn("guid-1", url) + self.assertIn("new_document", url) def test_if_none_match_header(self): """if_none_match=True sends If-None-Match: null.""" path = _make_temp_file() - try: - self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=True) - headers = self.od._request.call_args[1]["headers"] - self.assertEqual(headers["If-None-Match"], "null") - self.assertNotIn("If-Match", headers) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=True) + headers = self.od._request.call_args.kwargs["headers"] + self.assertEqual(headers["If-None-Match"], "null") + self.assertNotIn("If-Match", headers) def test_if_match_overwrite_header(self): """if_none_match=False sends If-Match: *.""" path = _make_temp_file() - try: - self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=False) - headers = self.od._request.call_args[1]["headers"] - self.assertEqual(headers["If-Match"], "*") - self.assertNotIn("If-None-Match", headers) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "col", path, if_none_match=False) + headers = self.od._request.call_args.kwargs["headers"] + self.assertEqual(headers["If-Match"], "*") + self.assertNotIn("If-None-Match", headers) def test_custom_mime_type(self): """Custom content_type is used in Content-Type header.""" path = _make_temp_file(b"{}", suffix=".json") - try: - self.od._upload_file_small("accounts", "guid-1", "col", path, content_type="application/json") - headers = self.od._request.call_args[1]["headers"] - self.assertEqual(headers["Content-Type"], "application/json") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "col", path, content_type="application/json") + headers = self.od._request.call_args.kwargs["headers"] + self.assertEqual(headers["Content-Type"], "application/json") def test_default_mime_type(self): """Default Content-Type is application/octet-stream.""" path = _make_temp_file() - try: - self.od._upload_file_small("accounts", "guid-1", "col", path) - headers = self.od._request.call_args[1]["headers"] - self.assertEqual(headers["Content-Type"], "application/octet-stream") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "col", path) + headers = self.od._request.call_args.kwargs["headers"] + self.assertEqual(headers["Content-Type"], "application/octet-stream") def test_file_not_found_raises(self): """Raises FileNotFoundError for missing file.""" @@ -248,12 +239,19 @@ def test_empty_record_id_raises(self): def test_file_name_in_header(self): """x-ms-file-name header contains the basename of the file.""" path = _make_temp_file(b"a,b,c", suffix=".csv") - try: - self.od._upload_file_small("accounts", "guid-1", "col", path) - headers = self.od._request.call_args[1]["headers"] - self.assertEqual(headers["x-ms-file-name"], os.path.basename(path)) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_small("accounts", "guid-1", "col", path) + headers = self.od._request.call_args.kwargs["headers"] + self.assertEqual(headers["x-ms-file-name"], os.path.basename(path)) + + def test_file_exceeds_small_upload_limit_raises(self): + """Raises ValueError when file exceeds 128MB single-upload limit.""" + path = _make_temp_file() + self.addCleanup(os.unlink, path) + with patch("os.path.getsize", return_value=128 * 1024 * 1024 + 1): + with self.assertRaises(ValueError) as ctx: + self.od._upload_file_small("accounts", "guid-1", "col", path) + self.assertIn("chunk", str(ctx.exception).lower()) class TestUploadFileChunk(unittest.TestCase): @@ -262,7 +260,8 @@ class TestUploadFileChunk(unittest.TestCase): def setUp(self): self.od = _make_odata_client() - def _mock_init_response(self, location="https://example.com/session?token=abc", chunk_size=None): + @staticmethod + def _mock_init_response(location="https://example.com/session?token=abc", chunk_size=None): """Create a mock init PATCH response with Location and optional chunk-size headers.""" resp = MagicMock() headers = {"Location": location} @@ -275,23 +274,19 @@ def test_init_patch_sends_chunked_header(self): """Initial PATCH sends x-ms-transfer-mode: chunked.""" self.od._request.return_value = self._mock_init_response() path = _make_temp_file(b"x" * 100) - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - init_call = self.od._request.call_args_list[0] - self.assertEqual(init_call[1]["headers"]["x-ms-transfer-mode"], "chunked") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + init_call = self.od._request.call_args_list[0] + self.assertEqual(init_call.kwargs["headers"]["x-ms-transfer-mode"], "chunked") def test_init_url_contains_file_name(self): """Init PATCH URL includes x-ms-file-name query parameter.""" self.od._request.return_value = self._mock_init_response() path = _make_temp_file(b"data", suffix=".pdf") - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - init_url = self.od._request.call_args_list[0][0][1] - self.assertIn("x-ms-file-name=", init_url) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + init_url = self.od._request.call_args_list[0].args[1] + self.assertIn("x-ms-file-name=", init_url) def test_missing_location_header_raises(self): """Raises RuntimeError when init response lacks Location header.""" @@ -299,96 +294,121 @@ def test_missing_location_header_raises(self): resp.headers = {} self.od._request.return_value = resp path = _make_temp_file() - try: - with self.assertRaises(RuntimeError) as ctx: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - self.assertIn("Location", str(ctx.exception)) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + with self.assertRaises(RuntimeError) as ctx: + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + self.assertIn("Location", str(ctx.exception)) + + def test_lowercase_location_header_accepted(self): + """Accepts lowercase 'location' header as fallback.""" + resp = MagicMock() + resp.headers = {"location": "https://example.com/session?token=abc"} + self.od._request.return_value = resp + path = _make_temp_file(b"data") + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # 1 init + 1 chunk = 2 total calls + self.assertEqual(self.od._request.call_count, 2) def test_uses_chunk_size_from_response(self): """Uses x-ms-chunk-size from init response to determine chunk size.""" self.od._request.return_value = self._mock_init_response(chunk_size=50) path = _make_temp_file(b"x" * 120) # 120 bytes / 50-byte chunks = 3 chunks - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - # 1 init + 3 chunk calls = 4 total - self.assertEqual(self.od._request.call_count, 4) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # 1 init + 3 chunk calls = 4 total + self.assertEqual(self.od._request.call_count, 4) def test_default_chunk_size_when_header_missing(self): """Falls back to 4MB chunk size when x-ms-chunk-size header missing.""" self.od._request.return_value = self._mock_init_response() # no chunk_size path = _make_temp_file(b"x" * 100) # 100 bytes < 4MB = single chunk - try: + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # 1 init + 1 chunk = 2 total + self.assertEqual(self.od._request.call_count, 2) + + def test_malformed_chunk_size_header_falls_back_to_default(self): + """Non-integer x-ms-chunk-size falls back to 4MB default.""" + resp = MagicMock() + resp.headers = {"Location": "https://example.com/session", "x-ms-chunk-size": "not-a-number"} + self.od._request.return_value = resp + path = _make_temp_file(b"x" * 100) # 100 bytes < 4MB = single chunk + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # Falls back to 4MB default → 100 bytes = 1 chunk → 2 total calls + self.assertEqual(self.od._request.call_count, 2) + + def test_negative_chunk_size_raises(self): + """Negative x-ms-chunk-size raises ValueError (zero falls back to 4MB default).""" + resp = MagicMock() + resp.headers = {"Location": "https://example.com/session", "x-ms-chunk-size": "-1"} + self.od._request.return_value = resp + path = _make_temp_file(b"data") + self.addCleanup(os.unlink, path) + with self.assertRaises(ValueError): self.od._upload_file_chunk("accounts", "guid-1", "col", path) - # 1 init + 1 chunk = 2 total - self.assertEqual(self.od._request.call_count, 2) - finally: - os.unlink(path) + + def test_empty_file_completes_without_chunk_requests(self): + """Zero-byte file sends only the init PATCH, no chunk PATCHes.""" + self.od._request.return_value = self._mock_init_response() + path = _make_temp_file(b"") + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + # Only the init PATCH is sent + self.assertEqual(self.od._request.call_count, 1) def test_content_range_headers(self): """Each chunk has correct Content-Range header.""" self.od._request.return_value = self._mock_init_response(chunk_size=10) path = _make_temp_file(b"A" * 10 + b"B" * 10 + b"C" * 5) # 25 bytes -> 3 chunks - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - chunk_calls = self.od._request.call_args_list[1:] # skip init - self.assertEqual(len(chunk_calls), 3) - self.assertEqual(chunk_calls[0][1]["headers"]["Content-Range"], "bytes 0-9/25") - self.assertEqual(chunk_calls[1][1]["headers"]["Content-Range"], "bytes 10-19/25") - self.assertEqual(chunk_calls[2][1]["headers"]["Content-Range"], "bytes 20-24/25") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_calls = self.od._request.call_args_list[1:] # skip init + self.assertEqual(len(chunk_calls), 3) + self.assertEqual(chunk_calls[0].kwargs["headers"]["Content-Range"], "bytes 0-9/25") + self.assertEqual(chunk_calls[1].kwargs["headers"]["Content-Range"], "bytes 10-19/25") + self.assertEqual(chunk_calls[2].kwargs["headers"]["Content-Range"], "bytes 20-24/25") def test_chunk_content_length_header(self): """Each chunk includes correct Content-Length header.""" self.od._request.return_value = self._mock_init_response(chunk_size=10) path = _make_temp_file(b"A" * 10 + b"B" * 5) # 15 bytes -> 2 chunks (10 + 5) - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - chunk_calls = self.od._request.call_args_list[1:] - self.assertEqual(chunk_calls[0][1]["headers"]["Content-Length"], "10") - self.assertEqual(chunk_calls[1][1]["headers"]["Content-Length"], "5") - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_calls = self.od._request.call_args_list[1:] + self.assertEqual(chunk_calls[0].kwargs["headers"]["Content-Length"], "10") + self.assertEqual(chunk_calls[1].kwargs["headers"]["Content-Length"], "5") def test_chunk_sends_to_location_url(self): """Chunk PATCHes go to the Location URL, not the original URL.""" session_url = "https://example.com/upload?session=xyz" self.od._request.return_value = self._mock_init_response(location=session_url) path = _make_temp_file() - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - chunk_call = self.od._request.call_args_list[1] - self.assertEqual(chunk_call[0][1], session_url) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + chunk_call = self.od._request.call_args_list[1] + self.assertEqual(chunk_call.args[1], session_url) def test_if_none_match_on_init(self): """if_none_match=True sends If-None-Match on init PATCH.""" self.od._request.return_value = self._mock_init_response() path = _make_temp_file() - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=True) - init_headers = self.od._request.call_args_list[0][1]["headers"] - self.assertEqual(init_headers["If-None-Match"], "null") - self.assertNotIn("If-Match", init_headers) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=True) + init_headers = self.od._request.call_args_list[0].kwargs["headers"] + self.assertEqual(init_headers["If-None-Match"], "null") + self.assertNotIn("If-Match", init_headers) def test_if_match_overwrite_on_init(self): """if_none_match=False sends If-Match on init PATCH.""" self.od._request.return_value = self._mock_init_response() path = _make_temp_file() - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=False) - init_headers = self.od._request.call_args_list[0][1]["headers"] - self.assertEqual(init_headers["If-Match"], "*") - self.assertNotIn("If-None-Match", init_headers) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path, if_none_match=False) + init_headers = self.od._request.call_args_list[0].kwargs["headers"] + self.assertEqual(init_headers["If-Match"], "*") + self.assertNotIn("If-None-Match", init_headers) def test_empty_record_id_raises(self): """Raises ValueError for empty record_id.""" @@ -404,9 +424,7 @@ def test_chunk_requests_accept_206_and_204(self): """Chunk requests use expected=(206, 204).""" self.od._request.return_value = self._mock_init_response(chunk_size=50) path = _make_temp_file(b"x" * 100) - try: - self.od._upload_file_chunk("accounts", "guid-1", "col", path) - for chunk_call in self.od._request.call_args_list[1:]: - self.assertEqual(chunk_call[1]["expected"], (206, 204)) - finally: - os.unlink(path) + self.addCleanup(os.unlink, path) + self.od._upload_file_chunk("accounts", "guid-1", "col", path) + for chunk_call in self.od._request.call_args_list[1:]: + self.assertEqual(chunk_call.kwargs["expected"], (206, 204)) diff --git a/tests/unit/models/test_query_builder.py b/tests/unit/models/test_query_builder.py index 47bb28fc..9f094912 100644 --- a/tests/unit/models/test_query_builder.py +++ b/tests/unit/models/test_query_builder.py @@ -344,6 +344,12 @@ def test_filter_raw_returns_self(self): qb = QueryBuilder("account") self.assertIs(qb.filter_raw("a eq 1"), qb) + def test_build_with_plain_string_filter_part(self): + """build() handles plain string entries in _filter_parts (internal path).""" + qb = QueryBuilder("account") + qb._filter_parts.append("name eq 'Contoso'") + self.assertEqual(qb.build()["filter"], "name eq 'Contoso'") + class TestWhere(unittest.TestCase): """Tests for the where() method with composable expressions.""" diff --git a/tests/unit/models/test_table_info.py b/tests/unit/models/test_table_info.py index b3da07fa..7842c266 100644 --- a/tests/unit/models/test_table_info.py +++ b/tests/unit/models/test_table_info.py @@ -65,9 +65,15 @@ def test_len(self): def test_keys_values_items(self): self.assertEqual(list(self.info.keys()), list(self.info._LEGACY_KEY_MAP.keys())) + self.assertEqual(self.info.values()[0], "new_Product") items = dict(self.info.items()) self.assertEqual(items["table_schema_name"], "new_Product") + def test_contains_non_string_key_returns_false(self): + """__contains__ returns False for non-string keys.""" + self.assertNotIn(42, self.info) + self.assertNotIn(None, self.info) + def test_to_dict(self): d = self.info.to_dict() self.assertIsInstance(d, dict) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index cfad101e..faeb9436 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -135,6 +135,12 @@ def test_get_multiple(self): self.assertEqual(results[0][1]["name"], "B") + def test_empty_base_url_raises(self): + """DataverseClient raises ValueError when base_url is empty.""" + with self.assertRaises(ValueError): + DataverseClient("", self.mock_credential) + + class TestCreateLookupField(unittest.TestCase): """Tests for client.tables.create_lookup_field convenience method.""" diff --git a/tests/unit/test_context_manager.py b/tests/unit/test_context_manager.py index df916583..511b2d0a 100644 --- a/tests/unit/test_context_manager.py +++ b/tests/unit/test_context_manager.py @@ -268,6 +268,17 @@ def test_close_available_without_context_manager(self): with self.assertRaises(RuntimeError): client.records.create("account", {"name": "test"}) + def test_flush_cache_delegates_to_odata(self): + """flush_cache() calls _flush_cache on the OData client and returns its result.""" + client = DataverseClient(self.base_url, self.mock_credential) + client._odata = MagicMock() + client._odata._flush_cache.return_value = 3 + + result = client.flush_cache("picklist") + + client._odata._flush_cache.assert_called_once_with("picklist") + self.assertEqual(result, 3) + class TestExceptionHandling(unittest.TestCase): """Tests for exception handling during context manager usage.""" diff --git a/tests/unit/test_records_operations.py b/tests/unit/test_records_operations.py index 97da5a40..09df6869 100644 --- a/tests/unit/test_records_operations.py +++ b/tests/unit/test_records_operations.py @@ -64,6 +64,27 @@ def test_create_single_returns_string(self): self.assertNotIsInstance(result, list) self.assertEqual(result, "single-guid") + def test_create_single_non_string_return_raises(self): + """create() raises TypeError if _create returns a non-string.""" + self.client._odata._entity_set_from_schema_name.return_value = "accounts" + self.client._odata._create.return_value = 12345 + + with self.assertRaises(TypeError): + self.client.records.create("account", {"name": "Contoso"}) + + def test_create_bulk_non_list_return_raises(self): + """create() raises TypeError if _create_multiple returns a non-list.""" + self.client._odata._entity_set_from_schema_name.return_value = "accounts" + self.client._odata._create_multiple.return_value = "not-a-list" + + with self.assertRaises(TypeError): + self.client.records.create("account", [{"name": "Contoso"}]) + + def test_create_invalid_data_type_raises(self): + """create() raises TypeError if data is neither dict nor list.""" + with self.assertRaises(TypeError): + self.client.records.create("account", "invalid") + # ------------------------------------------------------------------ update def test_update_single(self): @@ -96,6 +117,16 @@ def test_update_paired(self): self.client._odata._update_by_ids.assert_called_once_with("account", ids, changes) + def test_update_single_non_dict_changes_raises(self): + """update() raises TypeError if ids is str but changes is not a dict.""" + with self.assertRaises(TypeError): + self.client.records.update("account", "guid-1", ["not", "a", "dict"]) + + def test_update_invalid_ids_type_raises(self): + """update() raises TypeError if ids is neither str nor list.""" + with self.assertRaises(TypeError): + self.client.records.update("account", 12345, {"name": "X"}) + # ------------------------------------------------------------------ delete def test_delete_single(self): @@ -137,6 +168,16 @@ def test_delete_empty_list(self): self.client._odata._delete_multiple.assert_not_called() self.assertIsNone(result) + def test_delete_invalid_ids_type_raises(self): + """delete() raises TypeError if ids is neither str nor list.""" + with self.assertRaises(TypeError): + self.client.records.delete("account", 12345) + + def test_delete_list_with_non_string_guids_raises(self): + """delete() raises TypeError if the ids list contains non-string entries.""" + with self.assertRaises(TypeError): + self.client.records.delete("account", ["valid-guid", 42]) + # --------------------------------------------------------------------- get def test_get_single(self): @@ -158,6 +199,11 @@ def test_get_single_with_query_params_raises(self): with self.assertRaises(ValueError): self.client.records.get("account", "guid-1", filter="statecode eq 0") + def test_get_non_string_record_id_raises(self): + """get() raises TypeError if record_id is not a string.""" + with self.assertRaises(TypeError): + self.client.records.get("account", 12345) + def test_get_paginated(self): """get() without record_id should yield pages of Record objects.""" page_1 = [{"accountid": "1", "name": "A"}] From 29c9a5eeec936c64a0f2b42f63ce1e8640e0cee0 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Tue, 7 Apr 2026 23:10:30 -0700 Subject: [PATCH 04/16] Add unit tests for _ODataClient internal methods (148 new tests) --- tests/unit/data/test_odata_internal.py | 1515 +++++++++++++++++++++++- 1 file changed, 1514 insertions(+), 1 deletion(-) diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index bea5a3d6..2fb71e60 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -1,9 +1,12 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT license. +import time import unittest -from unittest.mock import MagicMock +from enum import Enum +from unittest.mock import MagicMock, patch +from PowerPlatform.Dataverse.core.errors import HttpError, MetadataError, ValidationError from PowerPlatform.Dataverse.data._odata import _ODataClient @@ -16,6 +19,33 @@ def _make_odata_client() -> _ODataClient: return client +def _mock_response(json_data=None, text="", status_code=200, headers=None): + """Create a mock HTTP response.""" + r = MagicMock() + r.status_code = status_code + r.text = text or (str(json_data) if json_data else "") + r.json.return_value = json_data or {} + r.headers = headers or {} + return r + + +def _entity_def_response(entity_set_name="accounts", primary_id="accountid", metadata_id="meta-001"): + """Simulate a successful EntityDefinitions response.""" + return _mock_response( + json_data={ + "value": [ + { + "LogicalName": "account", + "EntitySetName": entity_set_name, + "PrimaryIdAttribute": primary_id, + "MetadataId": metadata_id, + "SchemaName": "Account", + } + ] + } + ) + + class TestUpsertMultipleValidation(unittest.TestCase): """Unit tests for _ODataClient._upsert_multiple internal validation.""" @@ -530,5 +560,1488 @@ def test_returns_none(self): self.assertIsNone(result) +class TestStaticHelpers(unittest.TestCase): + """Unit tests for _ODataClient static helper methods and __init__ validation.""" + + def test_init_empty_base_url_raises(self): + """__init__ with empty base_url raises ValueError.""" + mock_auth = MagicMock() + mock_auth._acquire_token.return_value = MagicMock(access_token="t") + with self.assertRaises(ValueError): + _ODataClient(mock_auth, "") + + def test_normalize_cache_key_non_string_returns_empty(self): + """_normalize_cache_key with non-string returns empty string.""" + self.assertEqual(_ODataClient._normalize_cache_key(None), "") # type: ignore[arg-type] + self.assertEqual(_ODataClient._normalize_cache_key(42), "") # type: ignore[arg-type] + + def test_lowercase_list_none_returns_none(self): + """_lowercase_list(None) returns None.""" + self.assertIsNone(_ODataClient._lowercase_list(None)) + + def test_lowercase_list_empty_returns_empty(self): + """_lowercase_list([]) returns [].""" + self.assertFalse(_ODataClient._lowercase_list([])) + + def test_lowercase_keys_non_dict_returned_as_is(self): + """_lowercase_keys with non-dict input returns it unchanged.""" + self.assertEqual(_ODataClient._lowercase_keys("a string"), "a string") # type: ignore[arg-type] + self.assertIsNone(_ODataClient._lowercase_keys(None)) # type: ignore[arg-type] + + def test_to_pascal_basic(self): + """_to_pascal converts snake_case to PascalCase.""" + client = _make_odata_client() + self.assertEqual(client._to_pascal("hello_world"), "HelloWorld") + self.assertEqual(client._to_pascal("my_table_name"), "MyTableName") + self.assertEqual(client._to_pascal("single"), "Single") + + +class TestRequestErrorParsing(unittest.TestCase): + """Unit tests for _ODataClient._request error response handling.""" + + def setUp(self): + mock_auth = MagicMock() + mock_auth._acquire_token.return_value = MagicMock(access_token="token") + self.client = _ODataClient(mock_auth, "https://example.crm.dynamics.com") + + def _make_raw_response(self, status_code, json_data=None, headers=None): + r = MagicMock() + r.status_code = status_code + r.text = "body" + r.json.return_value = json_data or {} + r.headers = headers or {} + return r + + def test_message_key_fallback_used_when_no_error_key(self): + """_request uses 'message' key when 'error' key is absent.""" + r = self._make_raw_response(400, json_data={"message": "Bad input received"}) + self.client._raw_request = MagicMock(return_value=r) + with self.assertRaises(HttpError) as ctx: + self.client._request("get", "http://example.com/test") + self.assertIn("Bad input received", str(ctx.exception)) + + def test_retry_after_non_int_not_stored_in_details(self): + """Retry-After header that is non-numeric results in retry_after absent from details.""" + r = self._make_raw_response(429, headers={"Retry-After": "not-a-number"}) + self.client._raw_request = MagicMock(return_value=r) + with self.assertRaises(HttpError) as ctx: + self.client._request("get", "http://example.com/test") + self.assertIsNone(ctx.exception.details.get("retry_after")) + + def test_retry_after_int_stored_in_details(self): + """Retry-After header that is numeric is stored in exception details.""" + r = self._make_raw_response(429, headers={"Retry-After": "30"}) + self.client._raw_request = MagicMock(return_value=r) + with self.assertRaises(HttpError) as ctx: + self.client._request("get", "http://example.com/test") + self.assertEqual(ctx.exception.details.get("retry_after"), 30) + + +class TestCreateMultiple(unittest.TestCase): + """Unit tests for _ODataClient._create_multiple.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_non_dict_items_raise_type_error(self): + """_create_multiple raises TypeError for non-dict items.""" + with self.assertRaises(TypeError): + self.od._create_multiple("accounts", "account", ["not a dict"]) + + def test_odata_type_already_present_not_duplicated(self): + """If @odata.type already in record, it is preserved as-is.""" + self.od._request.return_value = _mock_response( + json_data={"Ids": ["id-1"]}, + text='{"Ids": ["id-1"]}', + ) + self.od._create_multiple( + "accounts", + "account", + [{"@odata.type": "Microsoft.Dynamics.CRM.account", "name": "Test"}], + ) + post_calls = [c for c in self.od._request.call_args_list if c.args[0] == "post"] + target = post_calls[0].kwargs["json"]["Targets"][0] + self.assertEqual(target["@odata.type"], "Microsoft.Dynamics.CRM.account") + + def test_body_not_dict_returns_empty_list(self): + """When response body is not a dict, returns empty list.""" + r = _mock_response(text='["id1", "id2"]') + r.json.return_value = ["id1", "id2"] + self.od._request.return_value = r + result = self.od._create_multiple("accounts", "account", [{"name": "A"}]) + self.assertEqual(result, []) + + def test_value_key_path_extracts_ids(self): + """Falls back to 'value' key to extract IDs via heuristic.""" + long_guid = "a" * 32 + r = _mock_response( + json_data={"value": [{"accountid": long_guid, "name": "Test"}]}, + text="...", + ) + self.od._request.return_value = r + result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) + self.assertEqual(result, [long_guid]) + + def test_value_key_with_non_dict_items_returns_empty(self): + """'value' list with non-dict items returns empty list.""" + r = _mock_response(json_data={"value": ["not-a-dict"]}, text="...") + self.od._request.return_value = r + self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) + result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) + self.assertEqual(result, []) + + def test_no_ids_or_value_key_returns_empty_list(self): + """When body has neither 'Ids' nor 'value' keys, returns empty list.""" + r = _mock_response(json_data={"something_else": "data"}, text="...") + self.od._request.return_value = r + self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) + result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) + self.assertEqual(result, []) + + def test_value_parse_error_returns_empty_list(self): + """ValueError in body.json() returns empty list.""" + r = MagicMock() + r.text = "invalid json" + r.json.side_effect = ValueError("bad json") + self.od._request.return_value = r + self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) + result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) + self.assertEqual(result, []) + + +class TestPrimaryIdAttr(unittest.TestCase): + """Unit tests for _ODataClient._primary_id_attr cache-miss behavior.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_cache_miss_resolves_via_entity_set_lookup(self): + """Cache miss triggers entity set lookup and populates primary ID cache.""" + def mock_entity_set(table_schema_name): + cache_key = table_schema_name.lower() + self.od._logical_to_entityset_cache[cache_key] = "accounts" + self.od._logical_primaryid_cache[cache_key] = "accountid" + return "accounts" + + self.od._entity_set_from_schema_name = MagicMock(side_effect=mock_entity_set) + result = self.od._primary_id_attr("account") + self.assertEqual(result, "accountid") + + def test_cache_miss_no_primary_id_raises_runtime_error(self): + """Cache miss with no PrimaryIdAttribute in metadata raises RuntimeError.""" + def mock_entity_set_no_pid(table_schema_name): + cache_key = table_schema_name.lower() + self.od._logical_to_entityset_cache[cache_key] = "accounts" + return "accounts" + + self.od._entity_set_from_schema_name = MagicMock(side_effect=mock_entity_set_no_pid) + with self.assertRaises(RuntimeError) as ctx: + self.od._primary_id_attr("account") + self.assertIn("PrimaryIdAttribute not resolved", str(ctx.exception)) + + def test_cache_hit_returns_without_lookup(self): + """Cache hit returns primary ID immediately without issuing any request.""" + self.od._logical_primaryid_cache["account"] = "accountid" + result = self.od._primary_id_attr("account") + self.assertEqual(result, "accountid") + self.od._request.assert_not_called() + + +class TestUpdateByIds(unittest.TestCase): + """Unit tests for _ODataClient._update_by_ids.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_non_list_ids_raises_type_error(self): + """_update_by_ids raises TypeError when ids is not a list.""" + with self.assertRaises(TypeError): + self.od._update_by_ids("account", "not-a-list", {"name": "X"}) # type: ignore[arg-type] + + def test_empty_ids_returns_none(self): + """_update_by_ids returns None immediately for empty ids list.""" + result = self.od._update_by_ids("account", [], {"name": "X"}) + self.assertIsNone(result) + self.od._request.assert_not_called() + + def test_non_list_non_dict_changes_raises_type_error(self): + """_update_by_ids raises TypeError for changes that is not dict or list.""" + self.od._primary_id_attr = MagicMock(return_value="accountid") + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + with self.assertRaises(TypeError) as ctx: + self.od._update_by_ids("account", ["id-1"], "bad-changes") # type: ignore[arg-type] + self.assertIn("changes must be dict or list[dict]", str(ctx.exception)) + + def test_list_changes_length_mismatch_raises_value_error(self): + """_update_by_ids raises ValueError when changes list length != ids length.""" + self.od._primary_id_attr = MagicMock(return_value="accountid") + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + with self.assertRaises(ValueError) as ctx: + self.od._update_by_ids("account", ["id-1", "id-2"], [{"name": "A"}]) + self.assertIn("Length of changes list must match", str(ctx.exception)) + + def test_non_dict_patch_in_list_raises_type_error(self): + """_update_by_ids raises TypeError when a patch in the list is not a dict.""" + self.od._primary_id_attr = MagicMock(return_value="accountid") + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + self.od._update_multiple = MagicMock() + with self.assertRaises(TypeError) as ctx: + self.od._update_by_ids("account", ["id-1"], ["not-a-dict"]) + self.assertIn("Each patch must be a dict", str(ctx.exception)) + + def test_dict_changes_broadcasts_to_all_ids(self): + """_update_by_ids with dict changes builds one batch record per ID.""" + self.od._primary_id_attr = MagicMock(return_value="accountid") + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + self.od._update_multiple = MagicMock() + self.od._update_by_ids("account", ["id-1", "id-2"], {"name": "X"}) + self.od._update_multiple.assert_called_once() + _, _, batch = self.od._update_multiple.call_args.args + self.assertEqual(len(batch), 2) + self.assertEqual(batch[0]["accountid"], "id-1") + self.assertEqual(batch[1]["accountid"], "id-2") + self.assertEqual(batch[0]["name"], "X") + self.assertEqual(batch[1]["name"], "X") + + def test_list_changes_merges_per_record(self): + """_update_by_ids with list changes merges each patch with its corresponding ID.""" + self.od._primary_id_attr = MagicMock(return_value="accountid") + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + self.od._update_multiple = MagicMock() + self.od._update_by_ids("account", ["id-1", "id-2"], [{"name": "A"}, {"name": "B"}]) + _, _, batch = self.od._update_multiple.call_args.args + self.assertEqual(batch[0], {"accountid": "id-1", "name": "A"}) + self.assertEqual(batch[1], {"accountid": "id-2", "name": "B"}) + + +class TestUpdateMultiple(unittest.TestCase): + """Unit tests for _ODataClient._update_multiple.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_non_list_records_raises_type_error(self): + """_update_multiple raises TypeError for non-list records.""" + with self.assertRaises(TypeError): + self.od._update_multiple("accounts", "account", "not-a-list") # type: ignore[arg-type] + + def test_empty_list_raises_type_error(self): + """_update_multiple raises TypeError for empty list.""" + with self.assertRaises(TypeError): + self.od._update_multiple("accounts", "account", []) + + def test_odata_type_already_present_not_overridden(self): + """If all records have @odata.type, it is preserved.""" + self.od._request.return_value = _mock_response() + records = [ + {"@odata.type": "Microsoft.Dynamics.CRM.CustomType", "accountid": "id-1", "name": "A"} + ] + self.od._update_multiple("accounts", "account", records) + payload = self.od._request.call_args.kwargs["json"] + self.assertEqual(payload["Targets"][0]["@odata.type"], "Microsoft.Dynamics.CRM.CustomType") + + def test_posts_to_update_multiple_endpoint(self): + """_update_multiple POSTs to {entity_set}/Microsoft.Dynamics.CRM.UpdateMultiple.""" + self.od._request.return_value = _mock_response() + self.od._update_multiple("accounts", "account", [{"accountid": "id-1", "name": "X"}]) + method, url = self.od._request.call_args.args + self.assertEqual(method, "post") + self.assertIn("accounts/Microsoft.Dynamics.CRM.UpdateMultiple", url) + + def test_payload_contains_targets_array(self): + """_update_multiple sends {"Targets": [...]} with @odata.type injected per record.""" + self.od._request.return_value = _mock_response() + self.od._update_multiple("accounts", "account", [{"accountid": "id-1", "name": "X"}]) + payload = self.od._request.call_args.kwargs["json"] + self.assertIn("Targets", payload) + self.assertEqual(len(payload["Targets"]), 1) + self.assertIn("@odata.type", payload["Targets"][0]) + + +class TestDelete(unittest.TestCase): + """Unit tests for _ODataClient._delete_multiple.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._primary_id_attr = MagicMock(return_value="accountid") + + def test_empty_ids_returns_none(self): + """_delete_multiple returns None for empty IDs.""" + result = self.od._delete_multiple("account", []) + self.assertIsNone(result) + self.od._request.assert_not_called() + + def test_filters_out_falsy_ids(self): + """_delete_multiple filters None/empty strings from ids.""" + result = self.od._delete_multiple("account", [None, "", None]) # type: ignore[list-item] + self.assertIsNone(result) + self.od._request.assert_not_called() + + def test_posts_bulk_delete_payload(self): + """_delete_multiple issues POST to BulkDelete with correct payload.""" + self.od._request.return_value = _mock_response( + json_data={"JobId": "job-001"}, text='{"JobId": "job-001"}' + ) + result = self.od._delete_multiple("account", ["id-1", "id-2"]) + self.assertEqual(result, "job-001") + call_args = self.od._request.call_args + self.assertEqual(call_args.args[0], "post") + self.assertIn("BulkDelete", call_args.args[1]) + payload = call_args.kwargs["json"] + self.assertIn("QuerySet", payload) + self.assertIn("JobName", payload) + query = payload["QuerySet"][0] + self.assertEqual(query["EntityName"], "account") + conditions = query["Criteria"]["Conditions"] + self.assertEqual(len(conditions), 1) + self.assertEqual(conditions[0]["AttributeName"], "accountid") + values = conditions[0]["Values"] + self.assertEqual(len(values), 2) + self.assertEqual({v["Value"] for v in values}, {"id-1", "id-2"}) + + def test_returns_none_when_no_job_id_in_body(self): + """_delete_multiple returns None when response body has no JobId.""" + self.od._request.return_value = _mock_response(json_data={}, text="{}") + result = self.od._delete_multiple("account", ["id-1"]) + self.assertIsNone(result) + + def test_handles_value_error_in_json_parsing(self): + """_delete_multiple handles ValueError in response JSON parsing gracefully.""" + r = MagicMock() + r.text = "invalid" + r.json.side_effect = ValueError + self.od._request.return_value = r + result = self.od._delete_multiple("account", ["id-1"]) + self.assertIsNone(result) + + +class TestFormatKey(unittest.TestCase): + """Unit tests for _ODataClient._format_key.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_guid_wrapped_in_parens(self): + """_format_key wraps 36-char GUID in parentheses.""" + guid = "11111111-2222-3333-4444-555555555555" + self.assertEqual(self.od._format_key(guid), f"({guid})") + + def test_already_wrapped_key_returned_as_is(self): + """_format_key returns already-parenthesized key unchanged.""" + key = "(some-key)" + self.assertEqual(self.od._format_key(key), key) + + def test_alternate_key_with_quotes_is_escaped(self): + """_format_key wraps alternate key with single-quoted value in parentheses.""" + result = self.od._format_key("mykey='it''s value'") + self.assertEqual(result, "(mykey='it''s value')") + + +class TestGetMultiple(unittest.TestCase): + """Unit tests for _ODataClient._get_multiple query parameter handling.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + + def _single_page_response(self, items=None): + data = {"value": items or [{"accountid": "id-1"}]} + r = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = r + + def test_filter_param_passed(self): + """_get_multiple passes $filter to params.""" + self._single_page_response() + list(self.od._get_multiple("account", filter="statecode eq 0")) + params = self.od._request.call_args.kwargs["params"] + self.assertEqual(params["$filter"], "statecode eq 0") + + def test_orderby_param_passed(self): + """_get_multiple passes $orderby to params.""" + self._single_page_response() + list(self.od._get_multiple("account", orderby=["name asc", "createdon desc"])) + params = self.od._request.call_args.kwargs["params"] + self.assertIn("$orderby", params) + + def test_expand_param_passed(self): + """_get_multiple passes $expand to params.""" + self._single_page_response() + list(self.od._get_multiple("account", expand=["contact_customer_accounts"])) + params = self.od._request.call_args.kwargs["params"] + self.assertIn("$expand", params) + + def test_top_param_passed(self): + """_get_multiple passes $top to params.""" + self._single_page_response() + list(self.od._get_multiple("account", top=5)) + params = self.od._request.call_args.kwargs["params"] + self.assertEqual(params["$top"], 5) + + def test_count_param_passed(self): + """_get_multiple passes $count=true when count=True.""" + self._single_page_response() + list(self.od._get_multiple("account", count=True)) + params = self.od._request.call_args.kwargs["params"] + self.assertEqual(params["$count"], "true") + + def test_include_annotations_sets_prefer_header(self): + """_get_multiple sets Prefer header with include-annotations.""" + self._single_page_response() + list(self.od._get_multiple("account", include_annotations="*")) + headers = self.od._request.call_args.kwargs.get("headers") or {} + self.assertIn("Prefer", headers) + self.assertIn("include-annotations", headers["Prefer"]) + + def test_page_size_sets_prefer_header(self): + """_get_multiple sets Prefer odata.maxpagesize when page_size > 0.""" + self._single_page_response() + list(self.od._get_multiple("account", page_size=50)) + headers = self.od._request.call_args.kwargs.get("headers") or {} + self.assertIn("odata.maxpagesize=50", headers.get("Prefer", "")) + + def test_value_error_in_json_returns_empty(self): + """ValueError in page JSON parsing yields nothing.""" + r = MagicMock() + r.text = "bad json" + r.json.side_effect = ValueError + self.od._request.return_value = r + pages = list(self.od._get_multiple("account")) + self.assertEqual(pages, []) + + +class TestQuerySql(unittest.TestCase): + """Unit tests for _ODataClient._query_sql.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") + + def test_non_string_sql_raises_validation_error(self): + """_query_sql raises ValidationError for non-string sql.""" + with self.assertRaises(ValidationError): + self.od._query_sql(123) # type: ignore[arg-type] + + def test_empty_sql_raises_validation_error(self): + """_query_sql raises ValidationError for empty sql.""" + with self.assertRaises(ValidationError): + self.od._query_sql(" ") + + def test_returns_value_list(self): + """_query_sql returns rows from response 'value' key.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"accountid": "id-1", "name": "Contoso"}]}, + text="...", + ) + result = self.od._query_sql("SELECT name FROM account") + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "Contoso") + + def test_filters_non_dict_rows(self): + """_query_sql filters out non-dict rows from 'value' list.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"name": "A"}, "not-a-dict", 42]}, text="..." + ) + result = self.od._query_sql("SELECT name FROM account") + self.assertEqual(len(result), 1) + + def test_body_as_list_fallback(self): + """_query_sql handles body being a list directly.""" + r = _mock_response(text="...") + r.json.return_value = [{"name": "A"}, {"name": "B"}] + self.od._request.return_value = r + result = self.od._query_sql("SELECT name FROM account") + self.assertEqual(len(result), 2) + + def test_value_error_in_json_returns_empty(self): + """_query_sql returns empty list when JSON parsing fails.""" + r = MagicMock() + r.text = "bad json" + r.json.side_effect = ValueError + self.od._request.return_value = r + result = self.od._query_sql("SELECT name FROM account") + self.assertEqual(result, []) + + def test_unexpected_body_returns_empty(self): + """_query_sql returns empty list for non-dict, non-list body.""" + r = _mock_response(text="...") + r.json.return_value = "unexpected" + self.od._request.return_value = r + result = self.od._query_sql("SELECT name FROM account") + self.assertEqual(result, []) + + def test_extract_non_string_raises_value_error(self): + """_extract_logical_table with non-string raises ValueError.""" + with self.assertRaises(ValueError): + _ODataClient._extract_logical_table(123) # type: ignore[arg-type] + + def test_extract_no_from_clause_raises_value_error(self): + """_extract_logical_table without FROM raises ValueError.""" + with self.assertRaises(ValueError) as ctx: + _ODataClient._extract_logical_table("SELECT name, surname") + self.assertIn("FROM", str(ctx.exception)) + + +class TestEntitySetFromSchemaName(unittest.TestCase): + """Unit tests for _ODataClient._entity_set_from_schema_name.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_empty_table_schema_name_raises_value_error(self): + """_entity_set_from_schema_name raises ValueError for empty input.""" + with self.assertRaises(ValueError) as ctx: + self.od._entity_set_from_schema_name("") + self.assertIn("table schema name required", str(ctx.exception)) + + def test_json_value_error_in_response_treated_as_empty(self): + """_entity_set_from_schema_name handles ValueError in JSON parsing.""" + r = MagicMock() + r.text = "invalid json" + r.json.side_effect = ValueError + self.od._request.return_value = r + with self.assertRaises(MetadataError): + self.od._entity_set_from_schema_name("account") + + def test_plural_hint_when_name_ends_with_s(self): + """Error message includes plural hint when name ends with 's' (not 'ss').""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + with self.assertRaises(MetadataError) as ctx: + self.od._entity_set_from_schema_name("accounts") + self.assertIn("plural", str(ctx.exception).lower()) + + def test_no_plural_hint_when_name_ends_with_ss(self): + """No plural hint when name ends with 'ss'.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + with self.assertRaises(MetadataError) as ctx: + self.od._entity_set_from_schema_name("address") + self.assertNotIn("plural", str(ctx.exception).lower()) + + def test_missing_entity_set_name_raises_metadata_error(self): + """MetadataError raised when EntitySetName is absent from metadata.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"LogicalName": "account", "EntitySetName": None, "PrimaryIdAttribute": "accountid"}]}, + text="...", + ) + with self.assertRaises(MetadataError) as ctx: + self.od._entity_set_from_schema_name("account") + self.assertIn("EntitySetName", str(ctx.exception)) + + def test_cache_hit_returns_without_request(self): + """Cache hit returns entity set name immediately without issuing any request.""" + self.od._logical_to_entityset_cache["account"] = "accounts" + result = self.od._entity_set_from_schema_name("account") + self.assertEqual(result, "accounts") + self.od._request.assert_not_called() + + +class TestGetEntityByTableSchemaName(unittest.TestCase): + """Unit tests for _ODataClient._get_entity_by_table_schema_name.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_returns_first_match(self): + """_get_entity_by_table_schema_name returns first entity when found.""" + self.od._request.return_value = _entity_def_response() + result = self.od._get_entity_by_table_schema_name("account") + self.assertIsNotNone(result) + self.assertEqual(result["EntitySetName"], "accounts") + + def test_returns_none_when_not_found(self): + """_get_entity_by_table_schema_name returns None when no match.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + result = self.od._get_entity_by_table_schema_name("nonexistent") + self.assertIsNone(result) + + +class TestCreateEntity(unittest.TestCase): + """Unit tests for _ODataClient._create_entity.""" + + def setUp(self): + self.od = _make_odata_client() + + def _setup_entity_creation(self, get_response=None): + """Mock _request: POST returns 201, GET returns entity definition.""" + def side_effect(method, url, **kwargs): + if method == "post": + return _mock_response(status_code=201) + else: + return get_response or _entity_def_response() + + self.od._request.side_effect = side_effect + + def test_successful_entity_creation(self): + """_create_entity returns metadata on success.""" + self._setup_entity_creation() + result = self.od._create_entity("new_TestTable", "Test Table", [], solution_unique_name=None) + self.assertEqual(result["EntitySetName"], "accounts") + + def test_entity_set_name_missing_raises_runtime_error(self): + """_create_entity raises RuntimeError when EntitySetName not available after create.""" + get_response = _mock_response(json_data={"value": []}, text="{}") + + def side_effect(method, url, **kwargs): + return _mock_response(status_code=201) if method == "post" else get_response + + self.od._request.side_effect = side_effect + with self.assertRaises(RuntimeError) as ctx: + self.od._create_entity("new_TestTable", "Test Table", []) + self.assertIn("EntitySetName not available", str(ctx.exception)) + + def test_metadata_id_missing_raises_runtime_error(self): + """_create_entity raises RuntimeError when MetadataId missing after create.""" + get_response = _mock_response( + json_data={"value": [{"EntitySetName": "new_testtables", "SchemaName": "new_TestTable"}]}, + text="...", + ) + + def side_effect(method, url, **kwargs): + return _mock_response(status_code=201) if method == "post" else get_response + + self.od._request.side_effect = side_effect + with self.assertRaises(RuntimeError) as ctx: + self.od._create_entity("new_TestTable", "Test Table", []) + self.assertIn("MetadataId missing", str(ctx.exception)) + + def test_solution_unique_name_passed_as_param(self): + """_create_entity passes SolutionUniqueName as query param when provided.""" + self._setup_entity_creation() + self.od._create_entity("new_TestTable", "Test Table", [], solution_unique_name="MySolution") + post_call = next(c for c in self.od._request.call_args_list if c.args[0] == "post") + self.assertEqual(post_call.kwargs.get("params"), {"SolutionUniqueName": "MySolution"}) + + +class TestGetAttributeMetadata(unittest.TestCase): + """Unit tests for _ODataClient._get_attribute_metadata.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_returns_attribute_when_found(self): + """_get_attribute_metadata returns attribute dict when found.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"MetadataId": "attr-001", "LogicalName": "name", "SchemaName": "Name"}]}, + text="...", + ) + result = self.od._get_attribute_metadata("meta-001", "name") + self.assertIsNotNone(result) + self.assertEqual(result["MetadataId"], "attr-001") + + def test_returns_none_when_not_found(self): + """_get_attribute_metadata returns None when attribute not in response.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + result = self.od._get_attribute_metadata("meta-001", "name") + self.assertIsNone(result) + + def test_extra_select_fields_included(self): + """_get_attribute_metadata appends extra_select fields to $select.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + self.od._get_attribute_metadata("meta-001", "name", extra_select="AttributeType,MaxLength") + params = self.od._request.call_args.kwargs["params"] + self.assertIn("AttributeType", params["$select"]) + self.assertIn("MaxLength", params["$select"]) + + def test_extra_select_skips_empty_pieces(self): + """_get_attribute_metadata skips empty pieces in extra_select.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + self.od._get_attribute_metadata("meta-001", "name", extra_select=",AttributeType,") + params = self.od._request.call_args.kwargs["params"] + self.assertIn("AttributeType", params["$select"]) + + def test_extra_select_skips_odata_annotation_pieces(self): + """_get_attribute_metadata skips pieces starting with @.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + self.od._get_attribute_metadata("meta-001", "name", extra_select="@odata.type,MaxLength") + params = self.od._request.call_args.kwargs["params"] + self.assertNotIn("@odata.type", params["$select"]) + self.assertIn("MaxLength", params["$select"]) + + def test_value_error_in_json_returns_none(self): + """_get_attribute_metadata returns None on JSON parse failure.""" + r = MagicMock() + r.text = "bad json" + r.json.side_effect = ValueError + self.od._request.return_value = r + result = self.od._get_attribute_metadata("meta-001", "name") + self.assertIsNone(result) + + +class TestWaitForAttributeVisibility(unittest.TestCase): + """Unit tests for _ODataClient._wait_for_attribute_visibility.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_returns_immediately_on_success(self): + """_wait_for_attribute_visibility returns immediately when first probe succeeds.""" + self.od._request.return_value = _mock_response() + self.od._wait_for_attribute_visibility("accounts", "name", delays=(0,)) + self.od._request.assert_called_once() + + def test_retries_on_failure_then_succeeds(self): + """_wait_for_attribute_visibility retries after initial failure.""" + self.od._request.side_effect = [RuntimeError("not ready"), _mock_response()] + self.od._wait_for_attribute_visibility("accounts", "name", delays=(0, 0)) + self.assertEqual(self.od._request.call_count, 2) + + def test_sleep_is_called_for_nonzero_delays(self): + """_wait_for_attribute_visibility calls time.sleep for non-zero delays.""" + self.od._request.side_effect = [RuntimeError("not ready"), _mock_response()] + with patch("PowerPlatform.Dataverse.data._odata.time.sleep") as mock_sleep: + self.od._wait_for_attribute_visibility("accounts", "name", delays=(0, 5)) + mock_sleep.assert_called_once_with(5) + + def test_raises_runtime_error_after_all_retries_exhausted(self): + """_wait_for_attribute_visibility raises RuntimeError when all retries fail.""" + self.od._request.side_effect = RuntimeError("not ready") + with self.assertRaises(RuntimeError) as ctx: + self.od._wait_for_attribute_visibility("accounts", "name", delays=(0, 0)) + self.assertIn("did not become visible", str(ctx.exception)) + + +class TestLocalizedLabelsPayload(unittest.TestCase): + """Unit tests for _ODataClient._build_localizedlabels_payload.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_non_int_lang_raises_value_error(self): + """_build_localizedlabels_payload raises ValueError for non-int language code.""" + with self.assertRaises(ValueError) as ctx: + self.od._build_localizedlabels_payload({"1033": "English"}) # type: ignore[arg-type] + self.assertIn("must be int", str(ctx.exception)) + + def test_non_string_label_raises_value_error(self): + """_build_localizedlabels_payload raises ValueError for non-string label.""" + with self.assertRaises(ValueError) as ctx: + self.od._build_localizedlabels_payload({1033: 42}) # type: ignore[arg-type] + self.assertIn("non-empty string", str(ctx.exception)) + + def test_empty_translations_raises_value_error(self): + """_build_localizedlabels_payload raises ValueError for empty translations.""" + with self.assertRaises(ValueError) as ctx: + self.od._build_localizedlabels_payload({}) + self.assertIn("At least one translation", str(ctx.exception)) + + def test_empty_string_label_raises_value_error(self): + """_build_localizedlabels_payload raises ValueError for empty string label.""" + with self.assertRaises(ValueError): + self.od._build_localizedlabels_payload({1033: " "}) + + +class TestEnumOptionSetPayload(unittest.TestCase): + """Unit tests for _ODataClient._enum_optionset_payload.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_empty_enum_raises_value_error(self): + """_enum_optionset_payload raises ValueError for enum with no members.""" + class EmptyEnum(Enum): + pass + + with self.assertRaises(ValueError) as ctx: + self.od._enum_optionset_payload("new_Status", EmptyEnum) + self.assertIn("no members", str(ctx.exception)) + + def test_int_key_in_labels_resolved_to_member_name(self): + """__labels__ with int keys (matching enum values) are resolved to member names.""" + class Status(Enum): + Active = 1 + Inactive = 2 + + Status.__labels__ = {1033: {1: "Active", 2: "Inactive"}} # type: ignore[attr-defined] + result = self.od._enum_optionset_payload("new_Status", Status) + self.assertEqual(len(result["OptionSet"]["Options"]), 2) + + def test_enum_member_object_as_labels_key(self): + """__labels__ with enum member objects as keys resolves member name.""" + class Status(Enum): + Active = 1 + Inactive = 2 + + Status.__labels__ = {1033: {Status.Active: "Active Label", Status.Inactive: "Inactive Label"}} # type: ignore[attr-defined] + result = self.od._enum_optionset_payload("new_Status", Status) + options = result["OptionSet"]["Options"] + self.assertEqual(len(options), 2) + active_opt = next(o for o in options if o["Value"] == 1) + active_label = next( + loc["Label"] for loc in active_opt["Label"]["LocalizedLabels"] if loc["LanguageCode"] == 1033 + ) + self.assertEqual(active_label, "Active Label") + + def test_int_key_not_matching_any_member_raises_value_error(self): + """__labels__ with int key not matching any member raises ValueError.""" + class Status(Enum): + Active = 1 + + Status.__labels__ = {1033: {99: "Unknown"}} # type: ignore[attr-defined] + with self.assertRaises(ValueError) as ctx: + self.od._enum_optionset_payload("new_Status", Status) + self.assertIn("int key", str(ctx.exception)) + + def test_duplicate_enum_values_raises_value_error(self): + """_enum_optionset_payload raises ValueError when two members share the same int value.""" + # Python treats second definition as an alias; __members__ exposes both names + class Status(Enum): + Active = 1 + DuplicateActive = 1 # alias for Active in Python Enum + + with self.assertRaises(ValueError) as ctx: + self.od._enum_optionset_payload("new_Status", Status) + self.assertIn("Duplicate", str(ctx.exception)) + + def test_non_int_enum_value_raises_value_error(self): + """_enum_optionset_payload raises ValueError for enum member with a non-int value.""" + class Status(Enum): + Active = "active" + + with self.assertRaises(ValueError) as ctx: + self.od._enum_optionset_payload("new_Status", Status) + self.assertIn("non-int", str(ctx.exception)) + + +class TestOptionSetMap(unittest.TestCase): + """Unit tests for _ODataClient._optionset_map and _normalize_picklist_label.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_normalize_non_string_returns_empty_string(self): + """_normalize_picklist_label returns '' for non-string input.""" + self.assertEqual(self.od._normalize_picklist_label(None), "") # type: ignore[arg-type] + self.assertEqual(self.od._normalize_picklist_label(42), "") # type: ignore[arg-type] + + def test_normalize_diacritics(self): + """_normalize_picklist_label strips diacritics and lowercases.""" + self.assertEqual(self.od._normalize_picklist_label("Actif"), "actif") + + def test_empty_inputs_return_none(self): + """_optionset_map returns None for empty table/attr inputs.""" + self.assertIsNone(self.od._optionset_map("", "attrname")) + self.assertIsNone(self.od._optionset_map("account", "")) + + def test_cache_hit_returns_cached_map(self): + """_optionset_map returns cached map on subsequent calls.""" + cache_key = ("account", "statuscode") + self.od._picklist_label_cache[cache_key] = {"map": {"active": 1}, "ts": time.time()} + result = self.od._optionset_map("account", "statuscode") + self.assertEqual(result, {"active": 1}) + self.od._request.assert_not_called() + + def test_non_picklist_attribute_returns_empty_dict(self): + """_optionset_map returns empty dict for non-picklist attribute.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"LogicalName": "name", "AttributeType": "String"}]}, + text="...", + ) + self.assertEqual(self.od._optionset_map("account", "name"), {}) + + def test_not_found_returns_none(self): + """_optionset_map returns None when attribute is not found in metadata.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + self.assertIsNone(self.od._optionset_map("account", "statuscode")) + + def test_step2_parses_options_and_builds_map(self): + """_optionset_map fetches OptionSet in step 2 and builds label->value map.""" + step1 = _mock_response( + json_data={"value": [{"LogicalName": "statuscode", "AttributeType": "Picklist"}]}, + text="...", + ) + step2 = _mock_response( + json_data={ + "OptionSet": { + "Options": [ + {"Value": 1, "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, + {"Value": 2, "Label": {"LocalizedLabels": [{"Label": "Inactive", "LanguageCode": 1033}]}}, + ] + } + }, + text="...", + ) + self.od._request.side_effect = [step1, step2] + result = self.od._optionset_map("account", "statuscode") + self.assertEqual(result["active"], 1) + self.assertEqual(result["inactive"], 2) + + def test_404_on_step1_retries_and_raises_runtime_error(self): + """_optionset_map retries on 404 and raises RuntimeError when all attempts fail.""" + err = HttpError("Not found", status_code=404) + err.status_code = 404 + self.od._request.side_effect = err + with patch("PowerPlatform.Dataverse.data._odata.time.sleep"): + with self.assertRaises(RuntimeError) as ctx: + self.od._optionset_map("account", "statuscode") + self.assertIn("not found after retries", str(ctx.exception).lower()) + + def _step1_picklist(self): + return _mock_response( + json_data={"value": [{"LogicalName": "statuscode", "AttributeType": "Picklist"}]}, + text="...", + ) + + def test_step2_404_raises_runtime_error_after_all_retries(self): + """_optionset_map step2 raises RuntimeError after all 404 retries.""" + err = HttpError("Not found", status_code=404) + err.status_code = 404 + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else (_ for _ in ()).throw(err) + + self.od._request.side_effect = side_effect + with patch("PowerPlatform.Dataverse.data._odata.time.sleep"): + with self.assertRaises(RuntimeError) as ctx: + self.od._optionset_map("account", "statuscode") + self.assertIn("not found after retries", str(ctx.exception).lower()) + + def test_step2_non_404_error_is_re_raised(self): + """_optionset_map step2 re-raises non-404 HttpError.""" + err = HttpError("Server Error", status_code=500) + err.status_code = 500 + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else (_ for _ in ()).throw(err) + + self.od._request.side_effect = side_effect + with self.assertRaises(HttpError): + self.od._optionset_map("account", "statuscode") + + def test_step2_value_error_in_json_returns_none(self): + """_optionset_map returns None when step2 JSON parsing fails.""" + r2 = MagicMock() + r2.text = "bad json" + r2.json.side_effect = ValueError + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else r2 + + self.od._request.side_effect = side_effect + self.assertIsNone(self.od._optionset_map("account", "statuscode")) + + def test_step2_no_option_set_returns_none(self): + """_optionset_map returns None when OptionSet key is missing.""" + r2 = _mock_response(json_data={}, text="{}") + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else r2 + + self.od._request.side_effect = side_effect + self.assertIsNone(self.od._optionset_map("account", "statuscode")) + + def test_step2_non_dict_option_skipped(self): + """_optionset_map skips non-dict items in Options list.""" + r2 = _mock_response( + json_data={ + "OptionSet": { + "Options": [ + "not-a-dict", + {"Value": 1, "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, + ] + } + }, + text="...", + ) + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else r2 + + self.od._request.side_effect = side_effect + result = self.od._optionset_map("account", "statuscode") + self.assertIn("active", result) + + def test_step2_option_with_non_int_value_skipped(self): + """_optionset_map skips options with non-int Value.""" + r2 = _mock_response( + json_data={ + "OptionSet": { + "Options": [ + {"Value": "not-an-int", "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, + {"Value": 2, "Label": {"LocalizedLabels": [{"Label": "Inactive", "LanguageCode": 1033}]}}, + ] + } + }, + text="...", + ) + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else r2 + + self.od._request.side_effect = side_effect + result = self.od._optionset_map("account", "statuscode") + self.assertNotIn("active", result) + self.assertIn("inactive", result) + + def test_step2_empty_options_list_returns_empty_dict(self): + """_optionset_map returns {} when options list yields no valid mapping.""" + r2 = _mock_response(json_data={"OptionSet": {"Options": []}}, text="...") + call_count = [0] + + def side_effect(*_): + call_count[0] += 1 + return self._step1_picklist() if call_count[0] == 1 else r2 + + self.od._request.side_effect = side_effect + self.assertEqual(self.od._optionset_map("account", "statuscode"), {}) + + +class TestConvertLabelsToInts(unittest.TestCase): + """Unit tests for _ODataClient._convert_labels_to_ints.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_label_string_converted_to_int(self): + """_convert_labels_to_ints replaces string labels with ints.""" + self.od._optionset_map = MagicMock(return_value={"active": 1, "inactive": 2}) + result = self.od._convert_labels_to_ints("account", {"statuscode": "Active"}) + self.assertEqual(result["statuscode"], 1) + + def test_non_matching_label_left_unchanged(self): + """Non-matching string values are left unchanged.""" + self.od._optionset_map = MagicMock(return_value={"active": 1}) + result = self.od._convert_labels_to_ints("account", {"statuscode": "unknown_status"}) + self.assertEqual(result["statuscode"], "unknown_status") + + def test_odata_annotation_keys_skipped(self): + """Keys containing '@odata.' are not processed.""" + self.od._optionset_map = MagicMock(return_value={"active": 1}) + record = {"new_field@odata.bind": "/contacts(id-1)"} + result = self.od._convert_labels_to_ints("account", record) + self.assertEqual(result["new_field@odata.bind"], "/contacts(id-1)") + self.od._optionset_map.assert_not_called() + + def test_no_mapping_leaves_value_unchanged(self): + """If optionset_map returns falsy, value is left unchanged.""" + self.od._optionset_map = MagicMock(return_value={}) + result = self.od._convert_labels_to_ints("account", {"statuscode": "Active"}) + self.assertEqual(result["statuscode"], "Active") + + +class TestAttributePayloadDtypes(unittest.TestCase): + """Unit tests for _ODataClient._attribute_payload dtype dispatching.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_int_dtype(self): + result = self.od._attribute_payload("new_Count", "int") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.IntegerAttributeMetadata") + + def test_integer_dtype_alias(self): + result = self.od._attribute_payload("new_Count", "integer") + self.assertIn("Integer", result["@odata.type"]) + + def test_decimal_dtype(self): + result = self.od._attribute_payload("new_Price", "decimal") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DecimalAttributeMetadata") + + def test_money_dtype_alias(self): + result = self.od._attribute_payload("new_Revenue", "money") + self.assertIn("Decimal", result["@odata.type"]) + + def test_float_dtype(self): + result = self.od._attribute_payload("new_Score", "float") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DoubleAttributeMetadata") + + def test_double_dtype_alias(self): + result = self.od._attribute_payload("new_Score", "double") + self.assertIn("Double", result["@odata.type"]) + + def test_datetime_dtype(self): + result = self.od._attribute_payload("new_CreatedDate", "datetime") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DateTimeAttributeMetadata") + + def test_date_dtype_alias(self): + result = self.od._attribute_payload("new_BirthDate", "date") + self.assertIn("DateTime", result["@odata.type"]) + + def test_bool_dtype(self): + result = self.od._attribute_payload("new_IsActive", "bool") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.BooleanAttributeMetadata") + + def test_boolean_dtype_alias(self): + result = self.od._attribute_payload("new_IsActive", "boolean") + self.assertIn("Boolean", result["@odata.type"]) + + def test_file_dtype(self): + result = self.od._attribute_payload("new_Attachment", "file") + self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.FileAttributeMetadata") + + def test_unsupported_dtype_returns_none(self): + self.assertIsNone(self.od._attribute_payload("new_Unknown", "unsupported_type")) + + def test_non_string_dtype_raises_value_error(self): + with self.assertRaises(ValueError): + self.od._attribute_payload("new_Field", 42) # type: ignore[arg-type] + + +class TestGetTableInfo(unittest.TestCase): + """Unit tests for _ODataClient._get_table_info.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_returns_none_when_entity_not_found(self): + """_get_table_info returns None when entity does not exist.""" + self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") + self.assertIsNone(self.od._get_table_info("new_NonExistent")) + + def test_returns_metadata_when_found(self): + """_get_table_info returns metadata dict when entity exists.""" + self.od._request.return_value = _entity_def_response() + result = self.od._get_table_info("account") + self.assertIsNotNone(result) + self.assertIn("entity_set_name", result) + + +class TestDeleteTable(unittest.TestCase): + """Unit tests for _ODataClient._delete_table.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_deletes_table_by_metadata_id(self): + """_delete_table issues DELETE to EntityDefinitions({MetadataId}).""" + self.od._request.return_value = _mock_response() + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "SchemaName": "new_Test"} + ) + self.od._delete_table("new_Test") + delete_call = next(c for c in self.od._request.call_args_list if c.args[0] == "delete") + self.assertIn("meta-001", delete_call.args[1]) + + def test_raises_metadata_error_when_not_found(self): + """_delete_table raises MetadataError when entity does not exist.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._delete_table("new_NonExistent") + + def test_raises_metadata_error_when_metadata_id_missing(self): + """_delete_table raises MetadataError when MetadataId is absent from entity.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value={"SchemaName": "new_Test"}) + with self.assertRaises(MetadataError): + self.od._delete_table("new_Test") + + +class TestCreateAlternateKey(unittest.TestCase): + """Unit tests for _ODataClient._create_alternate_key.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_creates_alternate_key(self): + """_create_alternate_key posts to Keys endpoint and returns metadata.""" + post_response = MagicMock() + post_response.headers = {"OData-EntityId": "https://example.com/Keys(key-meta-001)"} + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "LogicalName": "account", "SchemaName": "Account"} + ) + self.od._request.return_value = post_response + result = self.od._create_alternate_key("account", "new_AccountNumKey", ["accountnumber"]) + self.assertEqual(result["schema_name"], "new_AccountNumKey") + self.assertEqual(result["key_attributes"], ["accountnumber"]) + + def test_display_name_label_passed_to_payload(self): + """_create_alternate_key includes DisplayName when display_name_label is provided.""" + post_response = MagicMock() + post_response.headers = {"OData-EntityId": "https://example.com/Keys(key-id)"} + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "LogicalName": "account"} + ) + self.od._request.return_value = post_response + mock_label = MagicMock() + mock_label.to_dict.return_value = {"LocalizedLabels": [{"Label": "Account Number Key", "LanguageCode": 1033}]} + self.od._create_alternate_key("account", "new_AccNumKey", ["accountnumber"], display_name_label=mock_label) + payload = self.od._request.call_args.kwargs["json"] + self.assertIn("DisplayName", payload) + mock_label.to_dict.assert_called_once() + + def test_raises_metadata_error_when_table_not_found(self): + """_create_alternate_key raises MetadataError when table not found.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._create_alternate_key("nonexistent", "key", ["col"]) + + +class TestGetAlternateKeys(unittest.TestCase): + """Unit tests for _ODataClient._get_alternate_keys.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_returns_keys_list(self): + """_get_alternate_keys returns list of alternate keys.""" + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "LogicalName": "account"} + ) + self.od._request.return_value = _mock_response( + json_data={"value": [{"SchemaName": "new_AccountNumKey"}]}, + text="...", + ) + result = self.od._get_alternate_keys("account") + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["SchemaName"], "new_AccountNumKey") + + def test_raises_metadata_error_when_table_not_found(self): + """_get_alternate_keys raises MetadataError when table not found.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._get_alternate_keys("nonexistent") + + +class TestDeleteAlternateKey(unittest.TestCase): + """Unit tests for _ODataClient._delete_alternate_key.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_deletes_alternate_key(self): + """_delete_alternate_key issues DELETE to Keys({key_id}).""" + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "LogicalName": "account"} + ) + self.od._request.return_value = _mock_response() + self.od._delete_alternate_key("account", "key-meta-001") + delete_call = self.od._request.call_args + self.assertEqual(delete_call.args[0], "delete") + self.assertIn("key-meta-001", delete_call.args[1]) + + def test_raises_metadata_error_when_table_not_found(self): + """_delete_alternate_key raises MetadataError when table not found.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._delete_alternate_key("nonexistent", "key-id") + + +class TestCreateTable(unittest.TestCase): + """Unit tests for _ODataClient._create_table.""" + + def setUp(self): + self.od = _make_odata_client() + + def _setup_for_create(self, entity_exists=False): + """Mock helpers for _create_table.""" + existing = {"MetadataId": "meta-001", "EntitySetName": "accounts"} if entity_exists else None + created = { + "MetadataId": "meta-001", + "EntitySetName": "new_testtables", + "LogicalName": "new_testtable", + "SchemaName": "new_TestTable", + "PrimaryNameAttribute": "new_name", + "PrimaryIdAttribute": "new_testtableid", + } + call_count = [0] + + def mock_get_entity(table_schema_name, headers=None): + call_count[0] += 1 + if entity_exists: + return existing + return None if call_count[0] == 1 else created + + self.od._get_entity_by_table_schema_name = MagicMock(side_effect=mock_get_entity) + self.od._request.return_value = _mock_response(status_code=201) + + def test_creates_table_successfully(self): + """_create_table returns metadata dict on success.""" + self._setup_for_create() + result = self.od._create_table("new_TestTable", {"new_Name": "string", "new_Age": "int"}) + self.assertEqual(result["table_schema_name"], "new_TestTable") + self.assertIn("new_Name", result["columns_created"]) + self.assertIn("new_Age", result["columns_created"]) + + def test_raises_metadata_error_when_table_already_exists(self): + """_create_table raises MetadataError when table already exists.""" + self._setup_for_create(entity_exists=True) + with self.assertRaises(MetadataError): + self.od._create_table("new_TestTable", {"new_Name": "string"}) + + def test_raises_value_error_for_unsupported_column_type(self): + """_create_table raises ValueError for unsupported column type.""" + self._setup_for_create() + with self.assertRaises(ValueError) as ctx: + self.od._create_table("new_TestTable", {"new_Col": "unsupported_type"}) + self.assertIn("Unsupported column type", str(ctx.exception)) + + def test_raises_type_error_for_non_string_solution_name(self): + """_create_table raises TypeError when solution_unique_name is not str.""" + self._setup_for_create() + with self.assertRaises(TypeError): + self.od._create_table("new_TestTable", {}, solution_unique_name=123) # type: ignore[arg-type] + + def test_raises_value_error_for_empty_solution_name(self): + """_create_table raises ValueError when solution_unique_name is empty string.""" + self._setup_for_create() + with self.assertRaises(ValueError): + self.od._create_table("new_TestTable", {}, solution_unique_name="") + + def test_primary_column_schema_name_used_when_provided(self): + """_create_table uses provided primary_column_schema_name in the POST payload.""" + self._setup_for_create() + self.od._create_table("new_TestTable", {}, primary_column_schema_name="new_CustomName") + post_json = self.od._request.call_args.kwargs["json"] + attrs = post_json["Attributes"] + primary_attr = next((a for a in attrs if a.get("IsPrimaryName")), None) + self.assertIsNotNone(primary_attr) + self.assertEqual(primary_attr["SchemaName"], "new_CustomName") + + +class TestCreateColumns(unittest.TestCase): + """Unit tests for _ODataClient._create_columns.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "SchemaName": "new_Test"} + ) + self.od._request.return_value = _mock_response(status_code=201) + + def test_creates_columns_successfully(self): + """_create_columns returns list of created column names.""" + result = self.od._create_columns("new_Test", {"new_Name": "string", "new_Age": "int"}) + self.assertIn("new_Name", result) + self.assertIn("new_Age", result) + + def test_empty_columns_raises_type_error(self): + """_create_columns raises TypeError for empty columns dict.""" + with self.assertRaises(TypeError): + self.od._create_columns("new_Test", {}) + + def test_non_dict_columns_raises_type_error(self): + """_create_columns raises TypeError for non-dict columns.""" + with self.assertRaises(TypeError): + self.od._create_columns("new_Test", None) # type: ignore[arg-type] + + def test_table_not_found_raises_metadata_error(self): + """_create_columns raises MetadataError when table does not exist.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._create_columns("new_NonExistent", {"new_Col": "string"}) + + def test_unsupported_column_type_raises_value_error(self): + """_create_columns raises ValueError for unsupported column type.""" + with self.assertRaises(ValueError): + self.od._create_columns("new_Test", {"new_Col": "unsupported"}) + + def test_picklist_column_flushes_cache(self): + """_create_columns calls _flush_cache when a picklist column is created.""" + self.od._flush_cache = MagicMock(return_value=0) + + class Status(Enum): + Active = 1 + + self.od._create_columns("new_Test", {"new_Status": Status}) + self.od._flush_cache.assert_called_once_with("picklist") + + def test_posts_to_correct_endpoint(self): + """_create_columns POSTs each column to EntityDefinitions({metadata_id})/Attributes.""" + self.od._create_columns("new_Test", {"new_Name": "string"}) + call_args = self.od._request.call_args + self.assertEqual(call_args.args[0], "post") + self.assertIn("EntityDefinitions(meta-001)/Attributes", call_args.args[1]) + + +class TestDeleteColumns(unittest.TestCase): + """Unit tests for _ODataClient._delete_columns.""" + + def setUp(self): + self.od = _make_odata_client() + self.od._get_entity_by_table_schema_name = MagicMock( + return_value={"MetadataId": "meta-001", "SchemaName": "new_Test"} + ) + self.od._get_attribute_metadata = MagicMock( + return_value={"MetadataId": "attr-001", "LogicalName": "new_name", "@odata.type": "StringAttributeMetadata"} + ) + self.od._request.return_value = _mock_response(status_code=204) + + def test_deletes_single_column(self): + """_delete_columns accepts a string column name.""" + result = self.od._delete_columns("new_Test", "new_Name") + self.assertIn("new_Name", result) + + def test_deletes_list_of_columns(self): + """_delete_columns accepts a list of column names.""" + result = self.od._delete_columns("new_Test", ["new_Name1", "new_Name2"]) + self.assertEqual(len(result), 2) + + def test_non_string_non_list_raises_type_error(self): + """_delete_columns raises TypeError for invalid columns type.""" + with self.assertRaises(TypeError): + self.od._delete_columns("new_Test", 42) # type: ignore[arg-type] + + def test_empty_column_name_raises_value_error(self): + """_delete_columns raises ValueError for empty column name.""" + with self.assertRaises(ValueError): + self.od._delete_columns("new_Test", "") + + def test_table_not_found_raises_metadata_error(self): + """_delete_columns raises MetadataError when table not found.""" + self.od._get_entity_by_table_schema_name = MagicMock(return_value=None) + with self.assertRaises(MetadataError): + self.od._delete_columns("new_NonExistent", "new_Col") + + def test_column_not_found_raises_metadata_error(self): + """_delete_columns raises MetadataError when column not found.""" + self.od._get_attribute_metadata = MagicMock(return_value=None) + with self.assertRaises(MetadataError) as ctx: + self.od._delete_columns("new_Test", "new_Missing") + self.assertIn("not found", str(ctx.exception)) + + def test_missing_metadata_id_raises_runtime_error(self): + """_delete_columns raises RuntimeError when column MetadataId is missing.""" + self.od._get_attribute_metadata = MagicMock(return_value={"LogicalName": "new_name"}) + with self.assertRaises(RuntimeError) as ctx: + self.od._delete_columns("new_Test", "new_Name") + self.assertIn("MetadataId", str(ctx.exception)) + + def test_picklist_column_deletion_flushes_cache(self): + """_delete_columns flushes picklist cache when a picklist column is deleted.""" + self.od._get_attribute_metadata = MagicMock( + return_value={"MetadataId": "attr-001", "LogicalName": "new_status", "@odata.type": "PicklistAttributeMetadata"} + ) + self.od._flush_cache = MagicMock(return_value=0) + self.od._delete_columns("new_Test", "new_Status") + self.od._flush_cache.assert_called_once_with("picklist") + + +class TestFlushCache(unittest.TestCase): + """Unit tests for _ODataClient._flush_cache.""" + + def setUp(self): + self.od = _make_odata_client() + + def test_flush_picklist_clears_cache(self): + """_flush_cache('picklist') clears _picklist_label_cache.""" + self.od._picklist_label_cache = {("account", "statuscode"): {"map": {}, "ts": 0.0}} + removed = self.od._flush_cache("picklist") + self.assertEqual(removed, 1) + self.assertEqual(len(self.od._picklist_label_cache), 0) + + def test_flush_empty_cache_returns_zero(self): + """_flush_cache returns 0 when cache is already empty.""" + self.assertEqual(self.od._flush_cache("picklist"), 0) + + def test_unsupported_cache_kind_raises_validation_error(self): + """_flush_cache raises ValidationError for unsupported kind.""" + with self.assertRaises(ValidationError): + self.od._flush_cache("entityset") + + def test_none_kind_raises_validation_error(self): + """_flush_cache raises ValidationError for None kind.""" + with self.assertRaises(ValidationError): + self.od._flush_cache(None) # type: ignore[arg-type] + + if __name__ == "__main__": unittest.main() From ea3aaaf917626969ac34f6519b6cda83c7067513 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 09:23:14 -0700 Subject: [PATCH 05/16] Fix 5 broken tests and add docstrings to TestAttributePayloadDtypes --- tests/unit/data/test_odata_internal.py | 57 +++++++++++++++++++------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index 94a7964d..0a20e766 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -647,7 +647,7 @@ def test_odata_type_already_present_not_duplicated(self): [{"@odata.type": "Microsoft.Dynamics.CRM.account", "name": "Test"}], ) post_calls = [c for c in self.od._request.call_args_list if c.args[0] == "post"] - target = post_calls[0].kwargs["json"]["Targets"][0] + target = json.loads(post_calls[0].kwargs["data"])["Targets"][0] self.assertEqual(target["@odata.type"], "Microsoft.Dynamics.CRM.account") def test_body_not_dict_returns_empty_list(self): @@ -704,6 +704,7 @@ def setUp(self): def test_cache_miss_resolves_via_entity_set_lookup(self): """Cache miss triggers entity set lookup and populates primary ID cache.""" + def mock_entity_set(table_schema_name): cache_key = table_schema_name.lower() self.od._logical_to_entityset_cache[cache_key] = "accounts" @@ -716,6 +717,7 @@ def mock_entity_set(table_schema_name): def test_cache_miss_no_primary_id_raises_runtime_error(self): """Cache miss with no PrimaryIdAttribute in metadata raises RuntimeError.""" + def mock_entity_set_no_pid(table_schema_name): cache_key = table_schema_name.lower() self.od._logical_to_entityset_cache[cache_key] = "accounts" @@ -820,11 +822,9 @@ def test_empty_list_raises_type_error(self): def test_odata_type_already_present_not_overridden(self): """If all records have @odata.type, it is preserved.""" self.od._request.return_value = _mock_response() - records = [ - {"@odata.type": "Microsoft.Dynamics.CRM.CustomType", "accountid": "id-1", "name": "A"} - ] + records = [{"@odata.type": "Microsoft.Dynamics.CRM.CustomType", "accountid": "id-1", "name": "A"}] self.od._update_multiple("accounts", "account", records) - payload = self.od._request.call_args.kwargs["json"] + payload = json.loads(self.od._request.call_args.kwargs["data"]) self.assertEqual(payload["Targets"][0]["@odata.type"], "Microsoft.Dynamics.CRM.CustomType") def test_posts_to_update_multiple_endpoint(self): @@ -839,7 +839,7 @@ def test_payload_contains_targets_array(self): """_update_multiple sends {"Targets": [...]} with @odata.type injected per record.""" self.od._request.return_value = _mock_response() self.od._update_multiple("accounts", "account", [{"accountid": "id-1", "name": "X"}]) - payload = self.od._request.call_args.kwargs["json"] + payload = json.loads(self.od._request.call_args.kwargs["data"]) self.assertIn("Targets", payload) self.assertEqual(len(payload["Targets"]), 1) self.assertIn("@odata.type", payload["Targets"][0]) @@ -866,15 +866,13 @@ def test_filters_out_falsy_ids(self): def test_posts_bulk_delete_payload(self): """_delete_multiple issues POST to BulkDelete with correct payload.""" - self.od._request.return_value = _mock_response( - json_data={"JobId": "job-001"}, text='{"JobId": "job-001"}' - ) + self.od._request.return_value = _mock_response(json_data={"JobId": "job-001"}, text='{"JobId": "job-001"}') result = self.od._delete_multiple("account", ["id-1", "id-2"]) self.assertEqual(result, "job-001") call_args = self.od._request.call_args self.assertEqual(call_args.args[0], "post") self.assertIn("BulkDelete", call_args.args[1]) - payload = call_args.kwargs["json"] + payload = json.loads(call_args.kwargs["data"]) self.assertIn("QuerySet", payload) self.assertIn("JobName", payload) query = payload["QuerySet"][0] @@ -1149,6 +1147,7 @@ def setUp(self): def _setup_entity_creation(self, get_response=None): """Mock _request: POST returns 201, GET returns entity definition.""" + def side_effect(method, url, **kwargs): if method == "post": return _mock_response(status_code=201) @@ -1324,6 +1323,7 @@ def setUp(self): def test_empty_enum_raises_value_error(self): """_enum_optionset_payload raises ValueError for enum with no members.""" + class EmptyEnum(Enum): pass @@ -1333,6 +1333,7 @@ class EmptyEnum(Enum): def test_int_key_in_labels_resolved_to_member_name(self): """__labels__ with int keys (matching enum values) are resolved to member names.""" + class Status(Enum): Active = 1 Inactive = 2 @@ -1343,6 +1344,7 @@ class Status(Enum): def test_enum_member_object_as_labels_key(self): """__labels__ with enum member objects as keys resolves member name.""" + class Status(Enum): Active = 1 Inactive = 2 @@ -1359,6 +1361,7 @@ class Status(Enum): def test_int_key_not_matching_any_member_raises_value_error(self): """__labels__ with int key not matching any member raises ValueError.""" + class Status(Enum): Active = 1 @@ -1369,6 +1372,7 @@ class Status(Enum): def test_duplicate_enum_values_raises_value_error(self): """_enum_optionset_payload raises ValueError when two members share the same int value.""" + # Python treats second definition as an alias; __members__ exposes both names class Status(Enum): Active = 1 @@ -1380,6 +1384,7 @@ class Status(Enum): def test_non_int_enum_value_raises_value_error(self): """_enum_optionset_payload raises ValueError for enum member with a non-int value.""" + class Status(Enum): Active = "active" @@ -1552,7 +1557,10 @@ def test_step2_option_with_non_int_value_skipped(self): json_data={ "OptionSet": { "Options": [ - {"Value": "not-an-int", "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, + { + "Value": "not-an-int", + "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}, + }, {"Value": 2, "Label": {"LocalizedLabels": [{"Label": "Inactive", "LanguageCode": 1033}]}}, ] } @@ -1623,53 +1631,66 @@ def setUp(self): self.od = _make_odata_client() def test_int_dtype(self): + """'int' produces IntegerAttributeMetadata.""" result = self.od._attribute_payload("new_Count", "int") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.IntegerAttributeMetadata") def test_integer_dtype_alias(self): + """'integer' is an alias for 'int'.""" result = self.od._attribute_payload("new_Count", "integer") self.assertIn("Integer", result["@odata.type"]) def test_decimal_dtype(self): + """'decimal' produces DecimalAttributeMetadata.""" result = self.od._attribute_payload("new_Price", "decimal") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DecimalAttributeMetadata") def test_money_dtype_alias(self): + """'money' is an alias for 'decimal'.""" result = self.od._attribute_payload("new_Revenue", "money") self.assertIn("Decimal", result["@odata.type"]) def test_float_dtype(self): + """'float' produces DoubleAttributeMetadata.""" result = self.od._attribute_payload("new_Score", "float") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DoubleAttributeMetadata") def test_double_dtype_alias(self): + """'double' is an alias for 'float'.""" result = self.od._attribute_payload("new_Score", "double") self.assertIn("Double", result["@odata.type"]) def test_datetime_dtype(self): + """'datetime' produces DateTimeAttributeMetadata.""" result = self.od._attribute_payload("new_CreatedDate", "datetime") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.DateTimeAttributeMetadata") def test_date_dtype_alias(self): + """'date' is an alias for 'datetime'.""" result = self.od._attribute_payload("new_BirthDate", "date") self.assertIn("DateTime", result["@odata.type"]) def test_bool_dtype(self): + """'bool' produces BooleanAttributeMetadata.""" result = self.od._attribute_payload("new_IsActive", "bool") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.BooleanAttributeMetadata") def test_boolean_dtype_alias(self): + """'boolean' is an alias for 'bool'.""" result = self.od._attribute_payload("new_IsActive", "boolean") self.assertIn("Boolean", result["@odata.type"]) def test_file_dtype(self): + """'file' produces FileAttributeMetadata.""" result = self.od._attribute_payload("new_Attachment", "file") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.FileAttributeMetadata") def test_unsupported_dtype_returns_none(self): + """Unrecognized dtype string returns None.""" self.assertIsNone(self.od._attribute_payload("new_Unknown", "unsupported_type")) def test_non_string_dtype_raises_value_error(self): + """Non-string dtype raises ValueError.""" with self.assertRaises(ValueError): self.od._attribute_payload("new_Field", 42) # type: ignore[arg-type] @@ -1916,9 +1937,11 @@ def test_table_not_found_raises_metadata_error(self): with self.assertRaises(MetadataError): self.od._create_columns("new_NonExistent", {"new_Col": "string"}) - def test_unsupported_column_type_raises_value_error(self): - """_create_columns raises ValueError for unsupported column type.""" - with self.assertRaises(ValueError): + def test_unsupported_column_type_raises_validation_error(self): + """Raises ValidationError for unsupported column type.""" + from PowerPlatform.Dataverse.core.errors import ValidationError + + with self.assertRaises(ValidationError): self.od._create_columns("new_Test", {"new_Col": "unsupported"}) def test_picklist_column_flushes_cache(self): @@ -1995,7 +2018,11 @@ def test_missing_metadata_id_raises_runtime_error(self): def test_picklist_column_deletion_flushes_cache(self): """_delete_columns flushes picklist cache when a picklist column is deleted.""" self.od._get_attribute_metadata = MagicMock( - return_value={"MetadataId": "attr-001", "LogicalName": "new_status", "@odata.type": "PicklistAttributeMetadata"} + return_value={ + "MetadataId": "attr-001", + "LogicalName": "new_status", + "@odata.type": "PicklistAttributeMetadata", + } ) self.od._flush_cache = MagicMock(return_value=0) self.od._delete_columns("new_Test", "new_Status") From c208e5d5246b6dadf8709f8ee3d0b4d9c539ab32 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 10:46:05 -0700 Subject: [PATCH 06/16] test: review fixes and assertion strengthening --- tests/unit/core/test_auth.py | 7 +- tests/unit/core/test_http_client.py | 4 +- tests/unit/core/test_http_errors.py | 2 - tests/unit/data/test_odata_internal.py | 190 +++++++++++++++++++------ tests/unit/test_client.py | 1 - 5 files changed, 155 insertions(+), 49 deletions(-) diff --git a/tests/unit/core/test_auth.py b/tests/unit/core/test_auth.py index 93e7bf63..b82bb9bd 100644 --- a/tests/unit/core/test_auth.py +++ b/tests/unit/core/test_auth.py @@ -14,8 +14,12 @@ class TestAuthManager(unittest.TestCase): def test_non_token_credential_raises(self): """_AuthManager raises TypeError when credential does not implement TokenCredential.""" - with self.assertRaises(TypeError): + with self.assertRaises(TypeError) as ctx: _AuthManager("not-a-credential") + self.assertEqual( + str(ctx.exception), + "credential must implement azure.core.credentials.TokenCredential.", + ) def test_acquire_token_returns_token_pair(self): """_acquire_token calls get_token and returns a _TokenPair with scope and token.""" @@ -29,4 +33,3 @@ def test_acquire_token_returns_token_pair(self): self.assertIsInstance(result, _TokenPair) self.assertEqual(result.resource, "https://org.crm.dynamics.com/.default") self.assertEqual(result.access_token, "my-access-token") - diff --git a/tests/unit/core/test_http_client.py b/tests/unit/core/test_http_client.py index d29f8abb..200b8be9 100644 --- a/tests/unit/core/test_http_client.py +++ b/tests/unit/core/test_http_client.py @@ -49,7 +49,7 @@ def test_default_timeout_overrides_per_method_default(self): _, kwargs = mock_req.call_args self.assertEqual(kwargs["timeout"], 30.0) - def test_explicit_timeout_in_kwargs_is_not_overridden(self): + def test_explicit_timeout_kwarg_takes_precedence(self): """If timeout is already in kwargs it is passed through unchanged.""" client = _HttpClient(retries=1, timeout=30.0) with patch("requests.request", return_value=self._make_response()) as mock_req: @@ -66,7 +66,7 @@ def _make_response(self): resp.status_code = 200 return resp - def test_uses_requests_request_when_no_session(self): + def test_uses_direct_request_without_session(self): """Without a session, _request uses requests.request directly.""" client = _HttpClient(retries=1) with patch("requests.request", return_value=self._make_response()) as mock_req: diff --git a/tests/unit/core/test_http_errors.py b/tests/unit/core/test_http_errors.py index a5bbd0cb..39373e05 100644 --- a/tests/unit/core/test_http_errors.py +++ b/tests/unit/core/test_http_errors.py @@ -182,8 +182,6 @@ def test_correlation_id_shared_inside_call_scope(): assert h1["x-ms-correlation-id"] == h2["x-ms-correlation-id"] -# --- ValidationError / SQLParseError / HttpError optional fields --- - def test_validation_error_instantiates(): """ValidationError can be raised and carries the correct code.""" from PowerPlatform.Dataverse.core.errors import ValidationError diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index 0a20e766..0e0af173 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -87,6 +87,10 @@ def test_equal_lengths_does_not_raise(self): post_calls = [c for c in self.od._request.call_args_list if c.args[0] == "post"] self.assertEqual(len(post_calls), 1) self.assertIn("UpsertMultiple", post_calls[0].args[1]) + payload = post_calls[0].kwargs.get("json", {}) + self.assertEqual(len(payload["Targets"]), 2) + self.assertIn("@odata.type", payload["Targets"][0]) + self.assertIn("@odata.id", payload["Targets"][0]) def test_payload_excludes_alternate_key_fields(self): """Alternate key fields must NOT appear in the request body (only in @odata.id).""" @@ -330,7 +334,7 @@ def test_record_keys_lowercased(self): """Regular record field names are lowercased before sending.""" self.od._create("accounts", "account", {"Name": "Contoso", "AccountNumber": "ACC-001"}) call = self._post_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = json.loads(call.kwargs["data"]) self.assertIn("name", payload) self.assertIn("accountnumber", payload) self.assertNotIn("Name", payload) @@ -348,7 +352,7 @@ def test_odata_bind_keys_preserve_case(self): }, ) call = self._post_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = json.loads(call.kwargs["data"]) self.assertIn("new_name", payload) self.assertIn("new_CustomerId@odata.bind", payload) self.assertIn("new_AgentId@odata.bind", payload) @@ -414,7 +418,7 @@ def test_record_keys_lowercased(self): """Regular field names are lowercased in _update.""" self.od._update("new_ticket", "00000000-0000-0000-0000-000000000001", {"New_Status": 100000001}) call = self._patch_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = json.loads(call.kwargs["data"]) self.assertIn("new_status", payload) self.assertNotIn("New_Status", payload) @@ -429,7 +433,7 @@ def test_odata_bind_keys_preserve_case(self): }, ) call = self._patch_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = json.loads(call.kwargs["data"]) self.assertIn("new_status", payload) self.assertIn("new_CustomerId@odata.bind", payload) self.assertNotIn("new_customerid@odata.bind", payload) @@ -498,7 +502,7 @@ def test_record_keys_lowercased(self): """Record field names are lowercased before sending.""" self.od._upsert("accounts", "account", {"accountnumber": "ACC-001"}, {"Name": "Contoso"}) call = self._patch_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = call.kwargs["json"] self.assertIn("name", payload) self.assertNotIn("Name", payload) @@ -514,7 +518,7 @@ def test_odata_bind_keys_preserve_case(self): }, ) call = self._patch_call() - payload = json.loads(call.kwargs["data"]) if "data" in call.kwargs else call.kwargs["json"] + payload = call.kwargs["json"] # Regular field is lowercased self.assertIn("name", payload) # @odata.bind key preserves original casing @@ -548,19 +552,12 @@ def test_returns_none(self): class TestStaticHelpers(unittest.TestCase): - """Unit tests for _ODataClient static helper methods and __init__ validation.""" - - def test_init_empty_base_url_raises(self): - """__init__ with empty base_url raises ValueError.""" - mock_auth = MagicMock() - mock_auth._acquire_token.return_value = MagicMock(access_token="t") - with self.assertRaises(ValueError): - _ODataClient(mock_auth, "") + """Unit tests for _ODataClient static helper methods.""" def test_normalize_cache_key_non_string_returns_empty(self): """_normalize_cache_key with non-string returns empty string.""" - self.assertEqual(_ODataClient._normalize_cache_key(None), "") # type: ignore[arg-type] - self.assertEqual(_ODataClient._normalize_cache_key(42), "") # type: ignore[arg-type] + self.assertEqual(_ODataClient._normalize_cache_key(None), "") + self.assertEqual(_ODataClient._normalize_cache_key(42), "") def test_lowercase_list_none_returns_none(self): """_lowercase_list(None) returns None.""" @@ -572,8 +569,23 @@ def test_lowercase_list_empty_returns_empty(self): def test_lowercase_keys_non_dict_returned_as_is(self): """_lowercase_keys with non-dict input returns it unchanged.""" - self.assertEqual(_ODataClient._lowercase_keys("a string"), "a string") # type: ignore[arg-type] - self.assertIsNone(_ODataClient._lowercase_keys(None)) # type: ignore[arg-type] + self.assertEqual(_ODataClient._lowercase_keys("a string"), "a string") + self.assertIsNone(_ODataClient._lowercase_keys(None)) + + def test_lowercase_keys_preserves_odata_bind_casing(self): + """_lowercase_keys lowercases regular keys but preserves @odata.bind key casing.""" + result = _ODataClient._lowercase_keys( + { + "Name": "Contoso", + "new_CustomerId@odata.bind": "/contacts(id-1)", + "@odata.type": "Microsoft.Dynamics.CRM.account", + } + ) + self.assertIn("name", result) + self.assertNotIn("Name", result) + self.assertIn("new_CustomerId@odata.bind", result) + self.assertNotIn("new_customerid@odata.bind", result) + self.assertIn("@odata.type", result) def test_to_pascal_basic(self): """_to_pascal converts snake_case to PascalCase.""" @@ -745,7 +757,7 @@ def setUp(self): def test_non_list_ids_raises_type_error(self): """_update_by_ids raises TypeError when ids is not a list.""" with self.assertRaises(TypeError): - self.od._update_by_ids("account", "not-a-list", {"name": "X"}) # type: ignore[arg-type] + self.od._update_by_ids("account", "not-a-list", {"name": "X"}) def test_empty_ids_returns_none(self): """_update_by_ids returns None immediately for empty ids list.""" @@ -758,7 +770,7 @@ def test_non_list_non_dict_changes_raises_type_error(self): self.od._primary_id_attr = MagicMock(return_value="accountid") self.od._entity_set_from_schema_name = MagicMock(return_value="accounts") with self.assertRaises(TypeError) as ctx: - self.od._update_by_ids("account", ["id-1"], "bad-changes") # type: ignore[arg-type] + self.od._update_by_ids("account", ["id-1"], "bad-changes") self.assertIn("changes must be dict or list[dict]", str(ctx.exception)) def test_list_changes_length_mismatch_raises_value_error(self): @@ -812,7 +824,7 @@ def setUp(self): def test_non_list_records_raises_type_error(self): """_update_multiple raises TypeError for non-list records.""" with self.assertRaises(TypeError): - self.od._update_multiple("accounts", "account", "not-a-list") # type: ignore[arg-type] + self.od._update_multiple("accounts", "account", "not-a-list") def test_empty_list_raises_type_error(self): """_update_multiple raises TypeError for empty list.""" @@ -845,7 +857,7 @@ def test_payload_contains_targets_array(self): self.assertIn("@odata.type", payload["Targets"][0]) -class TestDelete(unittest.TestCase): +class TestDeleteMultiple(unittest.TestCase): """Unit tests for _ODataClient._delete_multiple.""" def setUp(self): @@ -860,7 +872,7 @@ def test_empty_ids_returns_none(self): def test_filters_out_falsy_ids(self): """_delete_multiple filters None/empty strings from ids.""" - result = self.od._delete_multiple("account", [None, "", None]) # type: ignore[list-item] + result = self.od._delete_multiple("account", [None, "", None]) self.assertIsNone(result) self.od._request.assert_not_called() @@ -946,14 +958,14 @@ def test_orderby_param_passed(self): self._single_page_response() list(self.od._get_multiple("account", orderby=["name asc", "createdon desc"])) params = self.od._request.call_args.kwargs["params"] - self.assertIn("$orderby", params) + self.assertEqual(params["$orderby"], "name asc,createdon desc") def test_expand_param_passed(self): """_get_multiple passes $expand to params.""" self._single_page_response() list(self.od._get_multiple("account", expand=["contact_customer_accounts"])) params = self.od._request.call_args.kwargs["params"] - self.assertIn("$expand", params) + self.assertEqual(params["$expand"], "contact_customer_accounts") def test_top_param_passed(self): """_get_multiple passes $top to params.""" @@ -993,6 +1005,58 @@ def test_value_error_in_json_returns_empty(self): pages = list(self.od._get_multiple("account")) self.assertEqual(pages, []) + def test_yields_value_items_as_page(self): + """_get_multiple yields the 'value' list as a page of dicts.""" + items = [{"accountid": "id-1", "name": "A"}, {"accountid": "id-2", "name": "B"}] + self._single_page_response(items) + pages = list(self.od._get_multiple("account")) + self.assertEqual(len(pages), 1) + self.assertEqual(pages[0], items) + + def test_follows_nextlink_pagination(self): + """_get_multiple follows @odata.nextLink across multiple pages.""" + page1 = _mock_response( + json_data={ + "value": [{"accountid": "id-1"}], + "@odata.nextLink": "https://example.crm.dynamics.com/next-page", + }, + text="...", + ) + page2 = _mock_response( + json_data={"value": [{"accountid": "id-2"}]}, + text="...", + ) + self.od._request.side_effect = [page1, page2] + pages = list(self.od._get_multiple("account")) + self.assertEqual(len(pages), 2) + self.assertEqual(pages[0][0]["accountid"], "id-1") + self.assertEqual(pages[1][0]["accountid"], "id-2") + + def test_stops_when_no_nextlink(self): + """_get_multiple stops after a page without nextLink.""" + self._single_page_response([{"accountid": "id-1"}]) + pages = list(self.od._get_multiple("account")) + self.assertEqual(len(pages), 1) + self.od._request.assert_called_once() + + def test_filters_non_dict_items_from_page(self): + """_get_multiple filters out non-dict items from each page.""" + data = {"value": [{"accountid": "id-1"}, "not-a-dict", 42]} + r = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = r + pages = list(self.od._get_multiple("account")) + self.assertEqual(len(pages), 1) + self.assertEqual(len(pages[0]), 1) + self.assertEqual(pages[0][0]["accountid"], "id-1") + + def test_empty_value_list_yields_nothing(self): + """_get_multiple yields nothing when value list is empty.""" + data = {"value": []} + r = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = r + pages = list(self.od._get_multiple("account")) + self.assertEqual(pages, []) + class TestQuerySql(unittest.TestCase): """Unit tests for _ODataClient._query_sql.""" @@ -1004,7 +1068,7 @@ def setUp(self): def test_non_string_sql_raises_validation_error(self): """_query_sql raises ValidationError for non-string sql.""" with self.assertRaises(ValidationError): - self.od._query_sql(123) # type: ignore[arg-type] + self.od._query_sql(123) def test_empty_sql_raises_validation_error(self): """_query_sql raises ValidationError for empty sql.""" @@ -1057,7 +1121,7 @@ def test_unexpected_body_returns_empty(self): def test_extract_non_string_raises_value_error(self): """_extract_logical_table with non-string raises ValueError.""" with self.assertRaises(ValueError): - _ODataClient._extract_logical_table(123) # type: ignore[arg-type] + _ODataClient._extract_logical_table(123) def test_extract_no_from_clause_raises_value_error(self): """_extract_logical_table without FROM raises ValueError.""" @@ -1118,6 +1182,28 @@ def test_cache_hit_returns_without_request(self): self.assertEqual(result, "accounts") self.od._request.assert_not_called() + def test_success_populates_entityset_cache(self): + """Successful API response populates _logical_to_entityset_cache.""" + self.od._request.return_value = _entity_def_response(entity_set_name="accounts", primary_id="accountid") + result = self.od._entity_set_from_schema_name("account") + self.assertEqual(result, "accounts") + self.assertEqual(self.od._logical_to_entityset_cache["account"], "accounts") + + def test_success_populates_primaryid_cache(self): + """Successful API response populates _logical_primaryid_cache.""" + self.od._request.return_value = _entity_def_response(entity_set_name="accounts", primary_id="accountid") + self.od._entity_set_from_schema_name("account") + self.assertEqual(self.od._logical_primaryid_cache["account"], "accountid") + + def test_success_without_primary_id_does_not_populate_primaryid_cache(self): + """When PrimaryIdAttribute is missing, _logical_primaryid_cache is not populated.""" + self.od._request.return_value = _mock_response( + json_data={"value": [{"LogicalName": "account", "EntitySetName": "accounts"}]}, + text="...", + ) + self.od._entity_set_from_schema_name("account") + self.assertNotIn("account", self.od._logical_primaryid_cache) + class TestGetEntityByTableSchemaName(unittest.TestCase): """Unit tests for _ODataClient._get_entity_by_table_schema_name.""" @@ -1294,13 +1380,13 @@ def setUp(self): def test_non_int_lang_raises_value_error(self): """_build_localizedlabels_payload raises ValueError for non-int language code.""" with self.assertRaises(ValueError) as ctx: - self.od._build_localizedlabels_payload({"1033": "English"}) # type: ignore[arg-type] + self.od._build_localizedlabels_payload({"1033": "English"}) self.assertIn("must be int", str(ctx.exception)) def test_non_string_label_raises_value_error(self): """_build_localizedlabels_payload raises ValueError for non-string label.""" with self.assertRaises(ValueError) as ctx: - self.od._build_localizedlabels_payload({1033: 42}) # type: ignore[arg-type] + self.od._build_localizedlabels_payload({1033: 42}) self.assertIn("non-empty string", str(ctx.exception)) def test_empty_translations_raises_value_error(self): @@ -1338,7 +1424,7 @@ class Status(Enum): Active = 1 Inactive = 2 - Status.__labels__ = {1033: {1: "Active", 2: "Inactive"}} # type: ignore[attr-defined] + Status.__labels__ = {1033: {1: "Active", 2: "Inactive"}} result = self.od._enum_optionset_payload("new_Status", Status) self.assertEqual(len(result["OptionSet"]["Options"]), 2) @@ -1349,7 +1435,7 @@ class Status(Enum): Active = 1 Inactive = 2 - Status.__labels__ = {1033: {Status.Active: "Active Label", Status.Inactive: "Inactive Label"}} # type: ignore[attr-defined] + Status.__labels__ = {1033: {Status.Active: "Active Label", Status.Inactive: "Inactive Label"}} result = self.od._enum_optionset_payload("new_Status", Status) options = result["OptionSet"]["Options"] self.assertEqual(len(options), 2) @@ -1365,7 +1451,7 @@ def test_int_key_not_matching_any_member_raises_value_error(self): class Status(Enum): Active = 1 - Status.__labels__ = {1033: {99: "Unknown"}} # type: ignore[attr-defined] + Status.__labels__ = {1033: {99: "Unknown"}} with self.assertRaises(ValueError) as ctx: self.od._enum_optionset_payload("new_Status", Status) self.assertIn("int key", str(ctx.exception)) @@ -1401,8 +1487,8 @@ def setUp(self): def test_normalize_non_string_returns_empty_string(self): """_normalize_picklist_label returns '' for non-string input.""" - self.assertEqual(self.od._normalize_picklist_label(None), "") # type: ignore[arg-type] - self.assertEqual(self.od._normalize_picklist_label(42), "") # type: ignore[arg-type] + self.assertEqual(self.od._normalize_picklist_label(None), "") + self.assertEqual(self.od._normalize_picklist_label(42), "") def test_normalize_diacritics(self): """_normalize_picklist_label strips diacritics and lowercases.""" @@ -1692,7 +1778,7 @@ def test_unsupported_dtype_returns_none(self): def test_non_string_dtype_raises_value_error(self): """Non-string dtype raises ValueError.""" with self.assertRaises(ValueError): - self.od._attribute_payload("new_Field", 42) # type: ignore[arg-type] + self.od._attribute_payload("new_Field", 42) class TestGetTableInfo(unittest.TestCase): @@ -1713,6 +1799,20 @@ def test_returns_metadata_when_found(self): self.assertIsNotNone(result) self.assertIn("entity_set_name", result) + def test_returns_full_dict_shape(self): + """_get_table_info returns all expected keys from metadata.""" + self.od._request.return_value = _entity_def_response( + entity_set_name="accounts", primary_id="accountid", metadata_id="meta-001" + ) + result = self.od._get_table_info("account") + self.assertEqual(result["table_schema_name"], "Account") + self.assertEqual(result["table_logical_name"], "account") + self.assertEqual(result["entity_set_name"], "accounts") + self.assertEqual(result["metadata_id"], "meta-001") + self.assertEqual(result["primary_id_attribute"], "accountid") + self.assertIsInstance(result["columns_created"], list) + self.assertEqual(result["columns_created"], []) + class TestDeleteTable(unittest.TestCase): """Unit tests for _ODataClient._delete_table.""" @@ -1886,7 +1986,7 @@ def test_raises_type_error_for_non_string_solution_name(self): """_create_table raises TypeError when solution_unique_name is not str.""" self._setup_for_create() with self.assertRaises(TypeError): - self.od._create_table("new_TestTable", {}, solution_unique_name=123) # type: ignore[arg-type] + self.od._create_table("new_TestTable", {}, solution_unique_name=123) def test_raises_value_error_for_empty_solution_name(self): """_create_table raises ValueError when solution_unique_name is empty string.""" @@ -1929,7 +2029,7 @@ def test_empty_columns_raises_type_error(self): def test_non_dict_columns_raises_type_error(self): """_create_columns raises TypeError for non-dict columns.""" with self.assertRaises(TypeError): - self.od._create_columns("new_Test", None) # type: ignore[arg-type] + self.od._create_columns("new_Test", None) def test_table_not_found_raises_metadata_error(self): """_create_columns raises MetadataError when table does not exist.""" @@ -1951,7 +2051,8 @@ def test_picklist_column_flushes_cache(self): class Status(Enum): Active = 1 - self.od._create_columns("new_Test", {"new_Status": Status}) + result = self.od._create_columns("new_Test", {"new_Status": Status}) + self.assertIn("new_Status", result) self.od._flush_cache.assert_called_once_with("picklist") def test_posts_to_correct_endpoint(self): @@ -1976,19 +2077,24 @@ def setUp(self): self.od._request.return_value = _mock_response(status_code=204) def test_deletes_single_column(self): - """_delete_columns accepts a string column name.""" + """_delete_columns accepts a string column name and issues DELETE.""" result = self.od._delete_columns("new_Test", "new_Name") self.assertIn("new_Name", result) + delete_calls = [c for c in self.od._request.call_args_list if c.args[0] == "delete"] + self.assertEqual(len(delete_calls), 1) + self.assertIn("attr-001", delete_calls[0].args[1]) def test_deletes_list_of_columns(self): - """_delete_columns accepts a list of column names.""" + """_delete_columns accepts a list of column names and issues DELETE for each.""" result = self.od._delete_columns("new_Test", ["new_Name1", "new_Name2"]) self.assertEqual(len(result), 2) + delete_calls = [c for c in self.od._request.call_args_list if c.args[0] == "delete"] + self.assertEqual(len(delete_calls), 2) def test_non_string_non_list_raises_type_error(self): """_delete_columns raises TypeError for invalid columns type.""" with self.assertRaises(TypeError): - self.od._delete_columns("new_Test", 42) # type: ignore[arg-type] + self.od._delete_columns("new_Test", 42) def test_empty_column_name_raises_value_error(self): """_delete_columns raises ValueError for empty column name.""" @@ -2054,7 +2160,7 @@ def test_unsupported_cache_kind_raises_validation_error(self): def test_none_kind_raises_validation_error(self): """_flush_cache raises ValidationError for None kind.""" with self.assertRaises(ValidationError): - self.od._flush_cache(None) # type: ignore[arg-type] + self.od._flush_cache(None) class TestBuildUpsertMultiple(unittest.TestCase): diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index faeb9436..440643a3 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -134,7 +134,6 @@ def test_get_multiple(self): self.assertEqual(results[0][0]["name"], "A") self.assertEqual(results[0][1]["name"], "B") - def test_empty_base_url_raises(self): """DataverseClient raises ValueError when base_url is empty.""" with self.assertRaises(ValueError): From 936162900487a304d3175e2ce97a50a913901522 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 16:24:30 -0700 Subject: [PATCH 07/16] Add unit test coverage enforcement and CI reporting Co-Authored-By: Claude Sonnet 4.6 --- .azdo/ci-pr.yaml | 15 +- .github/workflows/python-package.yml | 15 +- examples/advanced/walkthrough.py | 10 + src/PowerPlatform/Dataverse/data/_odata.py | 190 ++-- tests/unit/data/test_odata_internal.py | 1004 +++++++++++++++----- tests/unit/test_context_manager.py | 2 +- 6 files changed, 878 insertions(+), 358 deletions(-) diff --git a/.azdo/ci-pr.yaml b/.azdo/ci-pr.yaml index c5d680ca..a59aa8e0 100644 --- a/.azdo/ci-pr.yaml +++ b/.azdo/ci-pr.yaml @@ -42,7 +42,7 @@ extends: - script: | python -m pip install --upgrade pip - python -m pip install flake8 black build + python -m pip install flake8 black build diff-cover python -m pip install -e .[dev] displayName: 'Install dependencies' @@ -60,15 +60,20 @@ extends: - script: | python -m build displayName: 'Build package' - + - script: | python -m pip install dist/*.whl displayName: 'Install wheel' - + - script: | - pytest --junitxml=test-results.xml --cov=PowerPlatform --cov-report=xml --cov-report=term-missing + PYTHONPATH=src pytest --junitxml=test-results.xml --cov=src/PowerPlatform --cov-report=xml --cov-report=term-missing --cov-fail-under=90 displayName: 'Test with pytest' - + + - script: | + git fetch origin main + diff-cover coverage.xml --compare-branch=origin/main --fail-under=90 + displayName: 'Diff coverage (90% for new changes)' + - task: PublishTestResults@2 condition: succeededOrFailed() inputs: diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index fe8dbc7d..acd42139 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -18,6 +18,8 @@ jobs: steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Set up Python 3.12 uses: actions/setup-python@v5 @@ -27,7 +29,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install flake8 black build + python -m pip install flake8 black build diff-cover python -m pip install -e .[dev] - name: Check format with black @@ -44,14 +46,19 @@ jobs: - name: Build package run: | python -m build - + - name: Install wheel run: | python -m pip install dist/*.whl - + - name: Test with pytest run: | - pytest --junitxml=test-results.xml --cov=PowerPlatform --cov-report=xml --cov-report=term-missing + PYTHONPATH=src pytest --junitxml=test-results.xml --cov=src/PowerPlatform --cov-report=xml --cov-report=term-missing --cov-fail-under=90 + + - name: Diff coverage (90% for new changes) + run: | + git fetch origin ${{ github.base_ref }} + diff-cover coverage.xml --compare-branch=origin/${{ github.base_ref }} --fail-under=90 - name: Upload test results if: always() 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 0e0af173..6aa08d83 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -527,23 +527,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.""" @@ -1479,237 +1480,6 @@ class Status(Enum): self.assertIn("non-int", str(ctx.exception)) -class TestOptionSetMap(unittest.TestCase): - """Unit tests for _ODataClient._optionset_map and _normalize_picklist_label.""" - - def setUp(self): - self.od = _make_odata_client() - - def test_normalize_non_string_returns_empty_string(self): - """_normalize_picklist_label returns '' for non-string input.""" - self.assertEqual(self.od._normalize_picklist_label(None), "") - self.assertEqual(self.od._normalize_picklist_label(42), "") - - def test_normalize_diacritics(self): - """_normalize_picklist_label strips diacritics and lowercases.""" - self.assertEqual(self.od._normalize_picklist_label("Actif"), "actif") - - def test_empty_inputs_return_none(self): - """_optionset_map returns None for empty table/attr inputs.""" - self.assertIsNone(self.od._optionset_map("", "attrname")) - self.assertIsNone(self.od._optionset_map("account", "")) - - def test_cache_hit_returns_cached_map(self): - """_optionset_map returns cached map on subsequent calls.""" - cache_key = ("account", "statuscode") - self.od._picklist_label_cache[cache_key] = {"map": {"active": 1}, "ts": time.time()} - result = self.od._optionset_map("account", "statuscode") - self.assertEqual(result, {"active": 1}) - self.od._request.assert_not_called() - - def test_non_picklist_attribute_returns_empty_dict(self): - """_optionset_map returns empty dict for non-picklist attribute.""" - self.od._request.return_value = _mock_response( - json_data={"value": [{"LogicalName": "name", "AttributeType": "String"}]}, - text="...", - ) - self.assertEqual(self.od._optionset_map("account", "name"), {}) - - def test_not_found_returns_none(self): - """_optionset_map returns None when attribute is not found in metadata.""" - self.od._request.return_value = _mock_response(json_data={"value": []}, text="{}") - self.assertIsNone(self.od._optionset_map("account", "statuscode")) - - def test_step2_parses_options_and_builds_map(self): - """_optionset_map fetches OptionSet in step 2 and builds label->value map.""" - step1 = _mock_response( - json_data={"value": [{"LogicalName": "statuscode", "AttributeType": "Picklist"}]}, - text="...", - ) - step2 = _mock_response( - json_data={ - "OptionSet": { - "Options": [ - {"Value": 1, "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, - {"Value": 2, "Label": {"LocalizedLabels": [{"Label": "Inactive", "LanguageCode": 1033}]}}, - ] - } - }, - text="...", - ) - self.od._request.side_effect = [step1, step2] - result = self.od._optionset_map("account", "statuscode") - self.assertEqual(result["active"], 1) - self.assertEqual(result["inactive"], 2) - - def test_404_on_step1_retries_and_raises_runtime_error(self): - """_optionset_map retries on 404 and raises RuntimeError when all attempts fail.""" - err = HttpError("Not found", status_code=404) - err.status_code = 404 - self.od._request.side_effect = err - with patch("PowerPlatform.Dataverse.data._odata.time.sleep"): - with self.assertRaises(RuntimeError) as ctx: - self.od._optionset_map("account", "statuscode") - self.assertIn("not found after retries", str(ctx.exception).lower()) - - def _step1_picklist(self): - return _mock_response( - json_data={"value": [{"LogicalName": "statuscode", "AttributeType": "Picklist"}]}, - text="...", - ) - - def test_step2_404_raises_runtime_error_after_all_retries(self): - """_optionset_map step2 raises RuntimeError after all 404 retries.""" - err = HttpError("Not found", status_code=404) - err.status_code = 404 - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else (_ for _ in ()).throw(err) - - self.od._request.side_effect = side_effect - with patch("PowerPlatform.Dataverse.data._odata.time.sleep"): - with self.assertRaises(RuntimeError) as ctx: - self.od._optionset_map("account", "statuscode") - self.assertIn("not found after retries", str(ctx.exception).lower()) - - def test_step2_non_404_error_is_re_raised(self): - """_optionset_map step2 re-raises non-404 HttpError.""" - err = HttpError("Server Error", status_code=500) - err.status_code = 500 - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else (_ for _ in ()).throw(err) - - self.od._request.side_effect = side_effect - with self.assertRaises(HttpError): - self.od._optionset_map("account", "statuscode") - - def test_step2_value_error_in_json_returns_none(self): - """_optionset_map returns None when step2 JSON parsing fails.""" - r2 = MagicMock() - r2.text = "bad json" - r2.json.side_effect = ValueError - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else r2 - - self.od._request.side_effect = side_effect - self.assertIsNone(self.od._optionset_map("account", "statuscode")) - - def test_step2_no_option_set_returns_none(self): - """_optionset_map returns None when OptionSet key is missing.""" - r2 = _mock_response(json_data={}, text="{}") - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else r2 - - self.od._request.side_effect = side_effect - self.assertIsNone(self.od._optionset_map("account", "statuscode")) - - def test_step2_non_dict_option_skipped(self): - """_optionset_map skips non-dict items in Options list.""" - r2 = _mock_response( - json_data={ - "OptionSet": { - "Options": [ - "not-a-dict", - {"Value": 1, "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}}, - ] - } - }, - text="...", - ) - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else r2 - - self.od._request.side_effect = side_effect - result = self.od._optionset_map("account", "statuscode") - self.assertIn("active", result) - - def test_step2_option_with_non_int_value_skipped(self): - """_optionset_map skips options with non-int Value.""" - r2 = _mock_response( - json_data={ - "OptionSet": { - "Options": [ - { - "Value": "not-an-int", - "Label": {"LocalizedLabels": [{"Label": "Active", "LanguageCode": 1033}]}, - }, - {"Value": 2, "Label": {"LocalizedLabels": [{"Label": "Inactive", "LanguageCode": 1033}]}}, - ] - } - }, - text="...", - ) - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else r2 - - self.od._request.side_effect = side_effect - result = self.od._optionset_map("account", "statuscode") - self.assertNotIn("active", result) - self.assertIn("inactive", result) - - def test_step2_empty_options_list_returns_empty_dict(self): - """_optionset_map returns {} when options list yields no valid mapping.""" - r2 = _mock_response(json_data={"OptionSet": {"Options": []}}, text="...") - call_count = [0] - - def side_effect(*_): - call_count[0] += 1 - return self._step1_picklist() if call_count[0] == 1 else r2 - - self.od._request.side_effect = side_effect - self.assertEqual(self.od._optionset_map("account", "statuscode"), {}) - - -class TestConvertLabelsToInts(unittest.TestCase): - """Unit tests for _ODataClient._convert_labels_to_ints.""" - - def setUp(self): - self.od = _make_odata_client() - - def test_label_string_converted_to_int(self): - """_convert_labels_to_ints replaces string labels with ints.""" - self.od._optionset_map = MagicMock(return_value={"active": 1, "inactive": 2}) - result = self.od._convert_labels_to_ints("account", {"statuscode": "Active"}) - self.assertEqual(result["statuscode"], 1) - - def test_non_matching_label_left_unchanged(self): - """Non-matching string values are left unchanged.""" - self.od._optionset_map = MagicMock(return_value={"active": 1}) - result = self.od._convert_labels_to_ints("account", {"statuscode": "unknown_status"}) - self.assertEqual(result["statuscode"], "unknown_status") - - def test_odata_annotation_keys_skipped(self): - """Keys containing '@odata.' are not processed.""" - self.od._optionset_map = MagicMock(return_value={"active": 1}) - record = {"new_field@odata.bind": "/contacts(id-1)"} - result = self.od._convert_labels_to_ints("account", record) - self.assertEqual(result["new_field@odata.bind"], "/contacts(id-1)") - self.od._optionset_map.assert_not_called() - - def test_no_mapping_leaves_value_unchanged(self): - """If optionset_map returns falsy, value is left unchanged.""" - self.od._optionset_map = MagicMock(return_value={}) - result = self.od._convert_labels_to_ints("account", {"statuscode": "Active"}) - self.assertEqual(result["statuscode"], "Active") - - class TestAttributePayloadDtypes(unittest.TestCase): """Unit tests for _ODataClient._attribute_payload dtype dispatching.""" @@ -2163,6 +1933,758 @@ def test_none_kind_raises_validation_error(self): self.od._flush_cache(None) +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 511b2d0a..054f0395 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() From 44b8928f6323fab182c4fabce6315a4fdd508e93 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 16:51:39 -0700 Subject: [PATCH 08/16] Apply black formatting Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/data/test_odata_internal.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index e732cdb6..90b9dda5 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -1933,7 +1933,6 @@ def test_none_kind_raises_validation_error(self): self.od._flush_cache(None) - class TestPicklistLabelResolution(unittest.TestCase): """Tests for picklist label-to-integer resolution. From 400ca9593def98eb7c898a205570ad9cd6a6e990 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 17:35:12 -0700 Subject: [PATCH 09/16] Improve _batch.py coverage: add tests for dispatch, execute, and edge cases Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/data/test_batch_serialization.py | 323 +++++++++++++++++++- 1 file changed, 318 insertions(+), 5 deletions(-) diff --git a/tests/unit/data/test_batch_serialization.py b/tests/unit/data/test_batch_serialization.py index 561b48b0..d5df71b2 100644 --- a/tests/unit/data/test_batch_serialization.py +++ b/tests/unit/data/test_batch_serialization.py @@ -16,20 +16,27 @@ _RecordGet, _RecordUpdate, _RecordUpsert, + _TableCreate, + _TableDelete, _TableGet, _TableList, + _TableAddColumns, + _TableRemoveColumns, + _TableCreateOneToMany, + _TableCreateManyToMany, + _TableDeleteRelationship, + _TableGetRelationship, + _TableCreateLookupField, _QuerySql, _extract_boundary, _raise_top_level_batch_error, - _split_multipart, _parse_mime_part, _parse_http_response_part, _CRLF, ) -from PowerPlatform.Dataverse.core.errors import HttpError +from PowerPlatform.Dataverse.core.errors import HttpError, MetadataError, ValidationError from PowerPlatform.Dataverse.models.upsert import UpsertItem from PowerPlatform.Dataverse.data._raw_request import _RawRequest -from PowerPlatform.Dataverse.models.batch import BatchItemResponse def _make_od(): @@ -389,12 +396,36 @@ def test_exceeds_1000_raises(self): client = _BatchClient(od) items = [_RecordGet(table="account", record_id=f"guid-{i}") for i in range(1001)] - from PowerPlatform.Dataverse.core.errors import ValidationError - with self.assertRaises(ValidationError): client.execute(items) +class TestContinueOnError(unittest.TestCase): + """execute() sends Prefer: odata.continue-on-error when requested.""" + + def setUp(self): + self.od = _make_od() + self.od._build_get.return_value = _RawRequest(method="GET", url="https://x/accounts(g)") + mock_resp = MagicMock() + mock_resp.headers = {"Content-Type": 'multipart/mixed; boundary="batch_x"'} + mock_resp.status_code = 200 + mock_resp.text = "--batch_x\r\n\r\nHTTP/1.1 204 No Content\r\n\r\n\r\n--batch_x--" + self.od._request.return_value = mock_resp + self.client = _BatchClient(self.od) + + def test_continue_on_error_header_sent(self): + """Prefer: odata.continue-on-error header is included when continue_on_error=True.""" + self.client.execute([_RecordGet(table="account", record_id="guid-1")], continue_on_error=True) + _, kwargs = self.od._request.call_args + self.assertEqual(kwargs.get("headers", {}).get("Prefer"), "odata.continue-on-error") + + def test_no_continue_on_error_header_by_default(self): + """Prefer header is absent when continue_on_error is not set.""" + self.client.execute([_RecordGet(table="account", record_id="guid-1")]) + _, kwargs = self.od._request.call_args + self.assertNotIn("Prefer", kwargs.get("headers", {})) + + class TestChangeSetInternal(unittest.TestCase): def test_add_create_returns_dollar_n(self): cs = _ChangeSet() @@ -628,5 +659,287 @@ def test_parse_batch_response_raises_on_missing_boundary(self): client._parse_batch_response(resp) +class TestContinueOnError(unittest.TestCase): + """execute() sends Prefer: odata.continue-on-error when requested.""" + + def setUp(self): + self.od = _make_od() + self.od._build_get.return_value = _RawRequest(method="GET", url="https://x/accounts(g)") + mock_resp = MagicMock() + mock_resp.headers = {"Content-Type": 'multipart/mixed; boundary="batch_x"'} + mock_resp.status_code = 200 + mock_resp.text = "--batch_x\r\n\r\nHTTP/1.1 204 No Content\r\n\r\n\r\n--batch_x--" + self.od._request.return_value = mock_resp + self.client = _BatchClient(self.od) + + def test_continue_on_error_header_sent(self): + self.client.execute([_RecordGet(table="account", record_id="guid-1")], continue_on_error=True) + _, kwargs = self.od._request.call_args + self.assertEqual(kwargs.get("headers", {}).get("Prefer"), "odata.continue-on-error") + + def test_no_continue_on_error_header_by_default(self): + self.client.execute([_RecordGet(table="account", record_id="guid-1")]) + _, kwargs = self.od._request.call_args + self.assertNotIn("Prefer", kwargs.get("headers", {})) + + +class TestResolveItemDispatch(unittest.TestCase): + """_resolve_item() routes each intent type to the correct resolver.""" + + def _client_and_od(self): + od = _make_od() + client = _BatchClient(od) + return client, od + + def test_dispatch_record_update(self): + """_resolve_item routes _RecordUpdate to _resolve_record_update.""" + client, od = self._client_and_od() + od._build_update.return_value = MagicMock() + op = _RecordUpdate(table="account", ids="guid-1", changes={"name": "X"}) + result = client._resolve_item(op) + od._build_update.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_record_delete(self): + """_resolve_item routes _RecordDelete to _resolve_record_delete.""" + client, od = self._client_and_od() + od._build_delete.return_value = MagicMock() + op = _RecordDelete(table="account", ids="guid-1") + result = client._resolve_item(op) + od._build_delete.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_create(self): + """_resolve_item routes _TableCreate to _build_create_entity.""" + client, od = self._client_and_od() + od._build_create_entity.return_value = MagicMock() + op = _TableCreate(table="new_Widget", columns={"new_name": str}) + result = client._resolve_item(op) + od._build_create_entity.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_delete(self): + """_resolve_item routes _TableDelete, resolving MetadataId before calling _build_delete_entity.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._build_delete_entity.return_value = MagicMock() + op = _TableDelete(table="new_Widget") + result = client._resolve_item(op) + od._build_delete_entity.assert_called_once_with("meta-1") + self.assertEqual(len(result), 1) + + def test_dispatch_table_get(self): + """_resolve_item routes _TableGet to _build_get_entity.""" + client, od = self._client_and_od() + od._build_get_entity.return_value = MagicMock() + op = _TableGet(table="account") + result = client._resolve_item(op) + od._build_get_entity.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_list(self): + """_resolve_item routes _TableList to _build_list_entities.""" + client, od = self._client_and_od() + od._build_list_entities.return_value = MagicMock() + op = _TableList() + result = client._resolve_item(op) + od._build_list_entities.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_add_columns(self): + """_resolve_item routes _TableAddColumns, emitting one request per column.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._build_create_column.return_value = MagicMock() + op = _TableAddColumns(table="account", columns={"new_col": str}) + result = client._resolve_item(op) + od._build_create_column.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_remove_columns(self): + """_resolve_item routes _TableRemoveColumns, fetching attribute metadata before deleting.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._get_attribute_metadata.return_value = {"MetadataId": "attr-1"} + od._build_delete_column.return_value = MagicMock() + op = _TableRemoveColumns(table="account", columns="new_col") + result = client._resolve_item(op) + od._build_delete_column.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_create_one_to_many(self): + """_resolve_item routes _TableCreateOneToMany, merging lookup into relationship body.""" + client, od = self._client_and_od() + od._build_create_relationship.return_value = MagicMock() + lookup = MagicMock() + lookup.to_dict.return_value = {} + relationship = MagicMock() + relationship.to_dict.return_value = {} + op = _TableCreateOneToMany(lookup=lookup, relationship=relationship) + result = client._resolve_item(op) + od._build_create_relationship.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_create_many_to_many(self): + """_resolve_item routes _TableCreateManyToMany to _build_create_relationship.""" + client, od = self._client_and_od() + od._build_create_relationship.return_value = MagicMock() + relationship = MagicMock() + relationship.to_dict.return_value = {} + op = _TableCreateManyToMany(relationship=relationship) + result = client._resolve_item(op) + od._build_create_relationship.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_table_delete_relationship(self): + """_resolve_item routes _TableDeleteRelationship, passing relationship_id.""" + client, od = self._client_and_od() + od._build_delete_relationship.return_value = MagicMock() + op = _TableDeleteRelationship(relationship_id="rel-guid-1") + result = client._resolve_item(op) + od._build_delete_relationship.assert_called_once_with("rel-guid-1") + self.assertEqual(len(result), 1) + + def test_dispatch_table_get_relationship(self): + """_resolve_item routes _TableGetRelationship, passing schema_name.""" + client, od = self._client_and_od() + od._build_get_relationship.return_value = MagicMock() + op = _TableGetRelationship(schema_name="new_account_contact") + result = client._resolve_item(op) + od._build_get_relationship.assert_called_once_with("new_account_contact") + self.assertEqual(len(result), 1) + + def test_dispatch_table_create_lookup_field(self): + """_resolve_item routes _TableCreateLookupField, building lookup and relationship models.""" + client, od = self._client_and_od() + lookup = MagicMock() + lookup.to_dict.return_value = {} + relationship = MagicMock() + relationship.to_dict.return_value = {} + od._build_lookup_field_models.return_value = (lookup, relationship) + od._build_create_relationship.return_value = MagicMock() + op = _TableCreateLookupField( + referencing_table="new_Widget", + lookup_field_name="new_accountid", + referenced_table="account", + ) + result = client._resolve_item(op) + od._build_lookup_field_models.assert_called_once() + od._build_create_relationship.assert_called_once() + self.assertEqual(len(result), 1) + + def test_dispatch_query_sql(self): + """_resolve_item routes _QuerySql to _build_sql, passing the SQL string.""" + client, od = self._client_and_od() + od._build_sql.return_value = MagicMock() + op = _QuerySql(sql="SELECT name FROM account") + result = client._resolve_item(op) + od._build_sql.assert_called_once_with("SELECT name FROM account") + self.assertEqual(len(result), 1) + + +class TestResolveOneChangeset(unittest.TestCase): + """_resolve_one() raises ValidationError when operation produces != 1 request.""" + + def test_multi_request_op_in_changeset_raises(self): + """use_bulk_delete=False with 2 ids produces 2 requests — not allowed in a changeset.""" + od = _make_od() + client = _BatchClient(od) + od._build_delete.return_value = MagicMock() + op = _RecordDelete(table="account", ids=["guid-1", "guid-2"], use_bulk_delete=False) + with self.assertRaises(ValidationError): + client._resolve_one(op) + + +class TestRequireEntityMetadata(unittest.TestCase): + """_require_entity_metadata raises MetadataError when table not found.""" + + def test_missing_entity_raises_metadata_error(self): + """MetadataError raised when _get_entity_by_table_schema_name returns None.""" + od = _make_od() + od._get_entity_by_table_schema_name.return_value = None + client = _BatchClient(od) + with self.assertRaises(MetadataError): + client._require_entity_metadata("new_Missing") + + def test_entity_without_metadata_id_raises(self): + """MetadataError raised when entity exists but has no MetadataId field.""" + od = _make_od() + od._get_entity_by_table_schema_name.return_value = {"LogicalName": "new_missing"} + client = _BatchClient(od) + with self.assertRaises(MetadataError): + client._require_entity_metadata("new_Missing") + + def test_valid_entity_returns_metadata_id(self): + """Returns MetadataId string when entity is found and has a MetadataId.""" + od = _make_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-abc"} + client = _BatchClient(od) + result = client._require_entity_metadata("account") + self.assertEqual(result, "meta-abc") + + +class TestTableRemoveColumnsResolver(unittest.TestCase): + """_resolve_table_remove_columns covers string input and missing column error.""" + + def _client_and_od(self): + od = _make_od() + client = _BatchClient(od) + return client, od + + def test_single_string_column_resolved(self): + """A single string column name is accepted and resolved to one delete request.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._get_attribute_metadata.return_value = {"MetadataId": "attr-1"} + od._build_delete_column.return_value = MagicMock() + op = _TableRemoveColumns(table="account", columns="new_col") + result = client._resolve_table_remove_columns(op) + self.assertEqual(len(result), 1) + + def test_missing_column_raises_metadata_error(self): + """MetadataError raised when attribute metadata is not found for the column.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._get_attribute_metadata.return_value = None + op = _TableRemoveColumns(table="account", columns="new_missing") + with self.assertRaises(MetadataError): + client._resolve_table_remove_columns(op) + + def test_column_without_metadata_id_raises(self): + """MetadataError raised when attribute metadata exists but has no MetadataId.""" + client, od = self._client_and_od() + od._get_entity_by_table_schema_name.return_value = {"MetadataId": "meta-1"} + od._get_attribute_metadata.return_value = {"AttributeType": "String"} + op = _TableRemoveColumns(table="account", columns="new_col") + with self.assertRaises(MetadataError): + client._resolve_table_remove_columns(op) + + +class TestParseMimePartNoSeparator(unittest.TestCase): + """_parse_mime_part handles raw string with no blank-line separator.""" + + def test_no_double_newline_returns_empty_body(self): + """When raw part has no blank-line separator, headers are parsed and body is empty.""" + raw = "Content-Type: application/http" + headers, body = _parse_mime_part(raw) + self.assertEqual(headers.get("content-type"), "application/http") + self.assertEqual(body, "") + + +class TestParseHttpResponsePartMalformed(unittest.TestCase): + """_parse_http_response_part returns None for malformed status lines.""" + + def test_status_line_too_short_returns_none(self): + """Returns None when status line has fewer than 2 tokens (no status code).""" + result = _parse_http_response_part("HTTP/1.1\r\n\r\n", content_id=None) + self.assertIsNone(result) + + def test_non_integer_status_code_returns_none(self): + """Returns None when status code token is not a valid integer.""" + result = _parse_http_response_part("HTTP/1.1 XYZ OK\r\n\r\n", content_id=None) + self.assertIsNone(result) + + if __name__ == "__main__": unittest.main() From df693c1be4014203032d5617fc50f1584f84d36d Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 17:40:43 -0700 Subject: [PATCH 10/16] Strengthen batch test assertions and fix duplicate TestContinueOnError Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/data/test_batch_serialization.py | 74 +++++++++------------ 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/tests/unit/data/test_batch_serialization.py b/tests/unit/data/test_batch_serialization.py index d5df71b2..333bc3a9 100644 --- a/tests/unit/data/test_batch_serialization.py +++ b/tests/unit/data/test_batch_serialization.py @@ -659,30 +659,6 @@ def test_parse_batch_response_raises_on_missing_boundary(self): client._parse_batch_response(resp) -class TestContinueOnError(unittest.TestCase): - """execute() sends Prefer: odata.continue-on-error when requested.""" - - def setUp(self): - self.od = _make_od() - self.od._build_get.return_value = _RawRequest(method="GET", url="https://x/accounts(g)") - mock_resp = MagicMock() - mock_resp.headers = {"Content-Type": 'multipart/mixed; boundary="batch_x"'} - mock_resp.status_code = 200 - mock_resp.text = "--batch_x\r\n\r\nHTTP/1.1 204 No Content\r\n\r\n\r\n--batch_x--" - self.od._request.return_value = mock_resp - self.client = _BatchClient(self.od) - - def test_continue_on_error_header_sent(self): - self.client.execute([_RecordGet(table="account", record_id="guid-1")], continue_on_error=True) - _, kwargs = self.od._request.call_args - self.assertEqual(kwargs.get("headers", {}).get("Prefer"), "odata.continue-on-error") - - def test_no_continue_on_error_header_by_default(self): - self.client.execute([_RecordGet(table="account", record_id="guid-1")]) - _, kwargs = self.od._request.call_args - self.assertNotIn("Prefer", kwargs.get("headers", {})) - - class TestResolveItemDispatch(unittest.TestCase): """_resolve_item() routes each intent type to the correct resolver.""" @@ -697,7 +673,7 @@ def test_dispatch_record_update(self): od._build_update.return_value = MagicMock() op = _RecordUpdate(table="account", ids="guid-1", changes={"name": "X"}) result = client._resolve_item(op) - od._build_update.assert_called_once() + od._build_update.assert_called_once_with("account", "guid-1", {"name": "X"}, content_id=None) self.assertEqual(len(result), 1) def test_dispatch_record_delete(self): @@ -706,7 +682,7 @@ def test_dispatch_record_delete(self): od._build_delete.return_value = MagicMock() op = _RecordDelete(table="account", ids="guid-1") result = client._resolve_item(op) - od._build_delete.assert_called_once() + od._build_delete.assert_called_once_with("account", "guid-1", content_id=None) self.assertEqual(len(result), 1) def test_dispatch_table_create(self): @@ -715,7 +691,7 @@ def test_dispatch_table_create(self): od._build_create_entity.return_value = MagicMock() op = _TableCreate(table="new_Widget", columns={"new_name": str}) result = client._resolve_item(op) - od._build_create_entity.assert_called_once() + od._build_create_entity.assert_called_once_with("new_Widget", {"new_name": str}, None, None) self.assertEqual(len(result), 1) def test_dispatch_table_delete(self): @@ -734,16 +710,16 @@ def test_dispatch_table_get(self): od._build_get_entity.return_value = MagicMock() op = _TableGet(table="account") result = client._resolve_item(op) - od._build_get_entity.assert_called_once() + od._build_get_entity.assert_called_once_with("account") self.assertEqual(len(result), 1) def test_dispatch_table_list(self): - """_resolve_item routes _TableList to _build_list_entities.""" + """_resolve_item routes _TableList to _build_list_entities, passing filter and select.""" client, od = self._client_and_od() od._build_list_entities.return_value = MagicMock() op = _TableList() result = client._resolve_item(op) - od._build_list_entities.assert_called_once() + od._build_list_entities.assert_called_once_with(filter=None, select=None) self.assertEqual(len(result), 1) def test_dispatch_table_add_columns(self): @@ -753,7 +729,7 @@ def test_dispatch_table_add_columns(self): od._build_create_column.return_value = MagicMock() op = _TableAddColumns(table="account", columns={"new_col": str}) result = client._resolve_item(op) - od._build_create_column.assert_called_once() + od._build_create_column.assert_called_once_with("meta-1", "new_col", str) self.assertEqual(len(result), 1) def test_dispatch_table_remove_columns(self): @@ -764,7 +740,7 @@ def test_dispatch_table_remove_columns(self): od._build_delete_column.return_value = MagicMock() op = _TableRemoveColumns(table="account", columns="new_col") result = client._resolve_item(op) - od._build_delete_column.assert_called_once() + od._build_delete_column.assert_called_once_with("meta-1", "attr-1") self.assertEqual(len(result), 1) def test_dispatch_table_create_one_to_many(self): @@ -772,12 +748,15 @@ def test_dispatch_table_create_one_to_many(self): client, od = self._client_and_od() od._build_create_relationship.return_value = MagicMock() lookup = MagicMock() - lookup.to_dict.return_value = {} + lookup.to_dict.return_value = {"SchemaName": "new_account_contact"} relationship = MagicMock() - relationship.to_dict.return_value = {} + relationship.to_dict.return_value = {"ReferencedEntity": "account"} op = _TableCreateOneToMany(lookup=lookup, relationship=relationship) result = client._resolve_item(op) - od._build_create_relationship.assert_called_once() + od._build_create_relationship.assert_called_once_with( + {"ReferencedEntity": "account", "Lookup": {"SchemaName": "new_account_contact"}}, + solution=None, + ) self.assertEqual(len(result), 1) def test_dispatch_table_create_many_to_many(self): @@ -785,10 +764,10 @@ def test_dispatch_table_create_many_to_many(self): client, od = self._client_and_od() od._build_create_relationship.return_value = MagicMock() relationship = MagicMock() - relationship.to_dict.return_value = {} + relationship.to_dict.return_value = {"SchemaName": "new_account_contact"} op = _TableCreateManyToMany(relationship=relationship) result = client._resolve_item(op) - od._build_create_relationship.assert_called_once() + od._build_create_relationship.assert_called_once_with({"SchemaName": "new_account_contact"}, solution=None) self.assertEqual(len(result), 1) def test_dispatch_table_delete_relationship(self): @@ -813,9 +792,9 @@ def test_dispatch_table_create_lookup_field(self): """_resolve_item routes _TableCreateLookupField, building lookup and relationship models.""" client, od = self._client_and_od() lookup = MagicMock() - lookup.to_dict.return_value = {} + lookup.to_dict.return_value = {"SchemaName": "new_accountid"} relationship = MagicMock() - relationship.to_dict.return_value = {} + relationship.to_dict.return_value = {"ReferencedEntity": "account"} od._build_lookup_field_models.return_value = (lookup, relationship) od._build_create_relationship.return_value = MagicMock() op = _TableCreateLookupField( @@ -824,8 +803,20 @@ def test_dispatch_table_create_lookup_field(self): referenced_table="account", ) result = client._resolve_item(op) - od._build_lookup_field_models.assert_called_once() - od._build_create_relationship.assert_called_once() + od._build_lookup_field_models.assert_called_once_with( + referencing_table="new_Widget", + lookup_field_name="new_accountid", + referenced_table="account", + display_name=None, + description=None, + required=False, + cascade_delete="RemoveLink", + language_code=1033, + ) + od._build_create_relationship.assert_called_once_with( + {"ReferencedEntity": "account", "Lookup": {"SchemaName": "new_accountid"}}, + solution=None, + ) self.assertEqual(len(result), 1) def test_dispatch_query_sql(self): @@ -895,6 +886,7 @@ def test_single_string_column_resolved(self): od._build_delete_column.return_value = MagicMock() op = _TableRemoveColumns(table="account", columns="new_col") result = client._resolve_table_remove_columns(op) + od._build_delete_column.assert_called_once_with("meta-1", "attr-1") self.assertEqual(len(result), 1) def test_missing_column_raises_metadata_error(self): From 68e9804a3cbc33fb35f70702227fb7a6c0afbaac Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 22:25:35 -0700 Subject: [PATCH 11/16] Rename r1/r2 variables to req1/req2 for clarity Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/data/test_batch_serialization.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/data/test_batch_serialization.py b/tests/unit/data/test_batch_serialization.py index 333bc3a9..b194a341 100644 --- a/tests/unit/data/test_batch_serialization.py +++ b/tests/unit/data/test_batch_serialization.py @@ -193,19 +193,19 @@ def test_single_request_body_ends_with_closing_boundary(self): self.assertIn("--batch_bnd--", body) def test_multiple_requests_all_in_body(self): - r1 = _RawRequest(method="GET", url="https://org/api/data/v9.2/accounts") - r2 = _RawRequest( + req1 = _RawRequest(method="GET", url="https://org/api/data/v9.2/accounts") + req2 = _RawRequest( method="DELETE", url="https://org/api/data/v9.2/accounts(guid)", headers={"If-Match": "*"}, ) client = self._client() - body = client._build_batch_body([r1, r2], "bnd") + body = client._build_batch_body([req1, req2], "bnd") self.assertEqual(body.count("--bnd\r\n"), 2) def test_changeset_produces_nested_multipart(self): - r1 = _RawRequest(method="POST", url="https://org/api/data/v9.2/accounts", body="{}") - cs = _ChangeSetBatchItem(requests=[r1]) + req1 = _RawRequest(method="POST", url="https://org/api/data/v9.2/accounts", body="{}") + cs = _ChangeSetBatchItem(requests=[req1]) client = self._client() body = client._build_batch_body([cs], "outer_bnd") self.assertIn("Content-Type: multipart/mixed", body) From 6187c638016085a80f7c2d4ba4243ea3e26c1f01 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 22:50:49 -0700 Subject: [PATCH 12/16] Add UseDotNet prerequisite for PublishCodeCoverageResults@2 Co-Authored-By: Claude Sonnet 4.6 --- .azdo/ci-pr.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.azdo/ci-pr.yaml b/.azdo/ci-pr.yaml index a59aa8e0..21a267a0 100644 --- a/.azdo/ci-pr.yaml +++ b/.azdo/ci-pr.yaml @@ -81,6 +81,11 @@ extends: testRunTitle: 'Python 3.12' displayName: 'Publish test results' + - task: UseDotNet@2 + inputs: + version: '8.x' + displayName: 'Install .NET (required by PublishCodeCoverageResults@2)' + - task: PublishCodeCoverageResults@2 condition: succeededOrFailed() inputs: From caba60474233728f5e6fed0bd2fb7c45b4e6d3a9 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 22:52:21 -0700 Subject: [PATCH 13/16] Improve test_odata_internal: rename r vars, add multi-record tests Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/data/test_odata_internal.py | 149 +++++++++++++++---------- 1 file changed, 88 insertions(+), 61 deletions(-) diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index 90b9dda5..bb4640ac 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -605,33 +605,33 @@ def setUp(self): self.client = _ODataClient(mock_auth, "https://example.crm.dynamics.com") def _make_raw_response(self, status_code, json_data=None, headers=None): - r = MagicMock() - r.status_code = status_code - r.text = "body" - r.json.return_value = json_data or {} - r.headers = headers or {} - return r + response = MagicMock() + response.status_code = status_code + response.text = "body" + response.json.return_value = json_data or {} + response.headers = headers or {} + return response def test_message_key_fallback_used_when_no_error_key(self): """_request uses 'message' key when 'error' key is absent.""" - r = self._make_raw_response(400, json_data={"message": "Bad input received"}) - self.client._raw_request = MagicMock(return_value=r) + response = self._make_raw_response(400, json_data={"message": "Bad input received"}) + self.client._raw_request = MagicMock(return_value=response) with self.assertRaises(HttpError) as ctx: self.client._request("get", "http://example.com/test") self.assertIn("Bad input received", str(ctx.exception)) def test_retry_after_non_int_not_stored_in_details(self): """Retry-After header that is non-numeric results in retry_after absent from details.""" - r = self._make_raw_response(429, headers={"Retry-After": "not-a-number"}) - self.client._raw_request = MagicMock(return_value=r) + response = self._make_raw_response(429, headers={"Retry-After": "not-a-number"}) + self.client._raw_request = MagicMock(return_value=response) with self.assertRaises(HttpError) as ctx: self.client._request("get", "http://example.com/test") self.assertIsNone(ctx.exception.details.get("retry_after")) def test_retry_after_int_stored_in_details(self): """Retry-After header that is numeric is stored in exception details.""" - r = self._make_raw_response(429, headers={"Retry-After": "30"}) - self.client._raw_request = MagicMock(return_value=r) + response = self._make_raw_response(429, headers={"Retry-After": "30"}) + self.client._raw_request = MagicMock(return_value=response) with self.assertRaises(HttpError) as ctx: self.client._request("get", "http://example.com/test") self.assertEqual(ctx.exception.details.get("retry_after"), 30) @@ -665,49 +665,62 @@ def test_odata_type_already_present_not_duplicated(self): def test_body_not_dict_returns_empty_list(self): """When response body is not a dict, returns empty list.""" - r = _mock_response(text='["id1", "id2"]') - r.json.return_value = ["id1", "id2"] - self.od._request.return_value = r + response = _mock_response(text='["id1", "id2"]') + response.json.return_value = ["id1", "id2"] + self.od._request.return_value = response result = self.od._create_multiple("accounts", "account", [{"name": "A"}]) self.assertEqual(result, []) def test_value_key_path_extracts_ids(self): """Falls back to 'value' key to extract IDs via heuristic.""" long_guid = "a" * 32 - r = _mock_response( + response = _mock_response( json_data={"value": [{"accountid": long_guid, "name": "Test"}]}, text="...", ) - self.od._request.return_value = r + self.od._request.return_value = response result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) self.assertEqual(result, [long_guid]) def test_value_key_with_non_dict_items_returns_empty(self): """'value' list with non-dict items returns empty list.""" - r = _mock_response(json_data={"value": ["not-a-dict"]}, text="...") - self.od._request.return_value = r + response = _mock_response(json_data={"value": ["not-a-dict"]}, text="...") + self.od._request.return_value = response self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) self.assertEqual(result, []) def test_no_ids_or_value_key_returns_empty_list(self): """When body has neither 'Ids' nor 'value' keys, returns empty list.""" - r = _mock_response(json_data={"something_else": "data"}, text="...") - self.od._request.return_value = r + response = _mock_response(json_data={"something_else": "data"}, text="...") + self.od._request.return_value = response self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) self.assertEqual(result, []) def test_value_parse_error_returns_empty_list(self): """ValueError in body.json() returns empty list.""" - r = MagicMock() - r.text = "invalid json" - r.json.side_effect = ValueError("bad json") - self.od._request.return_value = r + response = MagicMock() + response.text = "invalid json" + response.json.side_effect = ValueError("bad json") + self.od._request.return_value = response self.od._convert_labels_to_ints = MagicMock(side_effect=lambda _, rec: rec) result = self.od._create_multiple("accounts", "account", [{"name": "Test"}]) self.assertEqual(result, []) + def test_multiple_records_returns_all_ids(self): + """All IDs from the Ids response key are returned for multiple input records.""" + self.od._request.return_value = _mock_response( + json_data={"Ids": ["id-1", "id-2", "id-3"]}, + text='{"Ids": ["id-1", "id-2", "id-3"]}', + ) + result = self.od._create_multiple( + "accounts", + "account", + [{"name": "A"}, {"name": "B"}, {"name": "C"}], + ) + self.assertEqual(result, ["id-1", "id-2", "id-3"]) + class TestPrimaryIdAttr(unittest.TestCase): """Unit tests for _ODataClient._primary_id_attr cache-miss behavior.""" @@ -857,6 +870,20 @@ def test_payload_contains_targets_array(self): self.assertEqual(len(payload["Targets"]), 1) self.assertIn("@odata.type", payload["Targets"][0]) + def test_multiple_records_all_in_targets(self): + """All records are included in the Targets payload for multiple inputs.""" + self.od._request.return_value = _mock_response() + records = [ + {"accountid": "id-1", "name": "A"}, + {"accountid": "id-2", "name": "B"}, + {"accountid": "id-3", "name": "C"}, + ] + self.od._update_multiple("accounts", "account", records) + payload = json.loads(self.od._request.call_args.kwargs["data"]) + self.assertEqual(len(payload["Targets"]), 3) + self.assertEqual(payload["Targets"][0]["accountid"], "id-1") + self.assertEqual(payload["Targets"][2]["accountid"], "id-3") + class TestDeleteMultiple(unittest.TestCase): """Unit tests for _ODataClient._delete_multiple.""" @@ -905,10 +932,10 @@ def test_returns_none_when_no_job_id_in_body(self): def test_handles_value_error_in_json_parsing(self): """_delete_multiple handles ValueError in response JSON parsing gracefully.""" - r = MagicMock() - r.text = "invalid" - r.json.side_effect = ValueError - self.od._request.return_value = r + response = MagicMock() + response.text = "invalid" + response.json.side_effect = ValueError + self.od._request.return_value = response result = self.od._delete_multiple("account", ["id-1"]) self.assertIsNone(result) @@ -944,8 +971,8 @@ def setUp(self): def _single_page_response(self, items=None): data = {"value": items or [{"accountid": "id-1"}]} - r = _mock_response(json_data=data, text=str(data)) - self.od._request.return_value = r + response = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = response def test_filter_param_passed(self): """_get_multiple passes $filter to params.""" @@ -999,10 +1026,10 @@ def test_page_size_sets_prefer_header(self): def test_value_error_in_json_returns_empty(self): """ValueError in page JSON parsing yields nothing.""" - r = MagicMock() - r.text = "bad json" - r.json.side_effect = ValueError - self.od._request.return_value = r + response = MagicMock() + response.text = "bad json" + response.json.side_effect = ValueError + self.od._request.return_value = response pages = list(self.od._get_multiple("account")) self.assertEqual(pages, []) @@ -1043,8 +1070,8 @@ def test_stops_when_no_nextlink(self): def test_filters_non_dict_items_from_page(self): """_get_multiple filters out non-dict items from each page.""" data = {"value": [{"accountid": "id-1"}, "not-a-dict", 42]} - r = _mock_response(json_data=data, text=str(data)) - self.od._request.return_value = r + response = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = response pages = list(self.od._get_multiple("account")) self.assertEqual(len(pages), 1) self.assertEqual(len(pages[0]), 1) @@ -1053,8 +1080,8 @@ def test_filters_non_dict_items_from_page(self): def test_empty_value_list_yields_nothing(self): """_get_multiple yields nothing when value list is empty.""" data = {"value": []} - r = _mock_response(json_data=data, text=str(data)) - self.od._request.return_value = r + response = _mock_response(json_data=data, text=str(data)) + self.od._request.return_value = response pages = list(self.od._get_multiple("account")) self.assertEqual(pages, []) @@ -1096,26 +1123,26 @@ def test_filters_non_dict_rows(self): def test_body_as_list_fallback(self): """_query_sql handles body being a list directly.""" - r = _mock_response(text="...") - r.json.return_value = [{"name": "A"}, {"name": "B"}] - self.od._request.return_value = r + response = _mock_response(text="...") + response.json.return_value = [{"name": "A"}, {"name": "B"}] + self.od._request.return_value = response result = self.od._query_sql("SELECT name FROM account") self.assertEqual(len(result), 2) def test_value_error_in_json_returns_empty(self): """_query_sql returns empty list when JSON parsing fails.""" - r = MagicMock() - r.text = "bad json" - r.json.side_effect = ValueError - self.od._request.return_value = r + response = MagicMock() + response.text = "bad json" + response.json.side_effect = ValueError + self.od._request.return_value = response result = self.od._query_sql("SELECT name FROM account") self.assertEqual(result, []) def test_unexpected_body_returns_empty(self): """_query_sql returns empty list for non-dict, non-list body.""" - r = _mock_response(text="...") - r.json.return_value = "unexpected" - self.od._request.return_value = r + response = _mock_response(text="...") + response.json.return_value = "unexpected" + self.od._request.return_value = response result = self.od._query_sql("SELECT name FROM account") self.assertEqual(result, []) @@ -1145,10 +1172,10 @@ def test_empty_table_schema_name_raises_value_error(self): def test_json_value_error_in_response_treated_as_empty(self): """_entity_set_from_schema_name handles ValueError in JSON parsing.""" - r = MagicMock() - r.text = "invalid json" - r.json.side_effect = ValueError - self.od._request.return_value = r + response = MagicMock() + response.text = "invalid json" + response.json.side_effect = ValueError + self.od._request.return_value = response with self.assertRaises(MetadataError): self.od._entity_set_from_schema_name("account") @@ -1331,10 +1358,10 @@ def test_extra_select_skips_odata_annotation_pieces(self): def test_value_error_in_json_returns_none(self): """_get_attribute_metadata returns None on JSON parse failure.""" - r = MagicMock() - r.text = "bad json" - r.json.side_effect = ValueError - self.od._request.return_value = r + response = MagicMock() + response.text = "bad json" + response.json.side_effect = ValueError + self.od._request.return_value = response result = self.od._get_attribute_metadata("meta-001", "name") self.assertIsNone(result) @@ -2374,11 +2401,11 @@ def test_convert_different_tables_separate_fetches(self): 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"}) + result1 = self.od._convert_labels_to_ints("account", {"industrycode": "Tech"}) + result2 = self.od._convert_labels_to_ints("new_ticket", {"new_status": "Open"}) - self.assertEqual(r1["industrycode"], 6) - self.assertEqual(r2["new_status"], 100) + self.assertEqual(result1["industrycode"], 6) + self.assertEqual(result2["new_status"], 100) self.assertEqual(self.od._request.call_count, 2) def test_convert_only_odata_and_non_strings_skips_fetch(self): From 3828c74cf82713122e775359a29872b9655fc58f Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Wed, 8 Apr 2026 23:43:31 -0700 Subject: [PATCH 14/16] Add pathToSources to PublishCodeCoverageResults@2 to fix coverage tab rendering Co-Authored-By: Claude Sonnet 4.6 --- .azdo/ci-pr.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.azdo/ci-pr.yaml b/.azdo/ci-pr.yaml index 21a267a0..10576b88 100644 --- a/.azdo/ci-pr.yaml +++ b/.azdo/ci-pr.yaml @@ -81,13 +81,9 @@ extends: testRunTitle: 'Python 3.12' displayName: 'Publish test results' - - task: UseDotNet@2 - inputs: - version: '8.x' - displayName: 'Install .NET (required by PublishCodeCoverageResults@2)' - - task: PublishCodeCoverageResults@2 condition: succeededOrFailed() inputs: summaryFileLocation: '**/coverage.xml' + pathToSources: '$(Build.SourcesDirectory)/src' displayName: 'Publish code coverage' From da288fd754d90eccaef504c388b17a526cdfea23 Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Thu, 9 Apr 2026 08:57:47 -0700 Subject: [PATCH 15/16] Move coverage config to pyproject.toml and simplify CI pytest command Co-Authored-By: Claude Sonnet 4.6 --- .azdo/ci-pr.yaml | 2 +- .github/workflows/python-package.yml | 2 +- pyproject.toml | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.azdo/ci-pr.yaml b/.azdo/ci-pr.yaml index 10576b88..80fc4b4d 100644 --- a/.azdo/ci-pr.yaml +++ b/.azdo/ci-pr.yaml @@ -66,7 +66,7 @@ extends: displayName: 'Install wheel' - script: | - PYTHONPATH=src pytest --junitxml=test-results.xml --cov=src/PowerPlatform --cov-report=xml --cov-report=term-missing --cov-fail-under=90 + PYTHONPATH=src pytest --junitxml=test-results.xml --cov --cov-report=xml displayName: 'Test with pytest' - script: | diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index acd42139..886bc72b 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -53,7 +53,7 @@ jobs: - name: Test with pytest run: | - PYTHONPATH=src pytest --junitxml=test-results.xml --cov=src/PowerPlatform --cov-report=xml --cov-report=term-missing --cov-fail-under=90 + PYTHONPATH=src pytest --junitxml=test-results.xml --cov --cov-report=xml - name: Diff coverage (90% for new changes) run: | diff --git a/pyproject.toml b/pyproject.toml index 3df59c9d..3e26347e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -93,6 +93,14 @@ select = [ [tool.pytest.ini_options] testpaths = ["tests/unit"] + +[tool.coverage.run] +source = ["src/PowerPlatform"] + +[tool.coverage.report] +fail_under = 90 +show_missing = true + markers = [ "e2e: end-to-end tests requiring a live Dataverse environment (DATAVERSE_URL)", ] From ff26a9826d6e4866bcaec47376004a838abeb17c Mon Sep 17 00:00:00 2001 From: Abel Milash Date: Thu, 9 Apr 2026 12:05:17 -0700 Subject: [PATCH 16/16] Align test names with main --- tests/unit/data/test_odata_internal.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/data/test_odata_internal.py b/tests/unit/data/test_odata_internal.py index 178ed03b..1aa50c0a 100644 --- a/tests/unit/data/test_odata_internal.py +++ b/tests/unit/data/test_odata_internal.py @@ -1568,10 +1568,6 @@ def test_file_dtype(self): result = self.od._attribute_payload("new_Attachment", "file") self.assertEqual(result["@odata.type"], "Microsoft.Dynamics.CRM.FileAttributeMetadata") - def test_unsupported_dtype_returns_none(self): - """Unrecognized dtype string returns None.""" - self.assertIsNone(self.od._attribute_payload("new_Unknown", "unsupported_type")) - def test_non_string_dtype_raises_value_error(self): """Non-string dtype raises ValueError.""" with self.assertRaises(ValueError): @@ -1599,6 +1595,11 @@ def test_string_type_max_length(self): self.assertEqual(result["MaxLength"], 200) self.assertEqual(result["FormatName"], {"Value": "Text"}) + def test_unsupported_type_returns_none(self): + """An unknown type string should return None.""" + result = self.od._attribute_payload("new_Col", "unknown_type") + self.assertIsNone(result) + class TestGetTableInfo(unittest.TestCase): """Unit tests for _ODataClient._get_table_info."""