Skip to content

Commit be47e9e

Browse files
author
Saurabh Badenkal
committed
Address PR review comments from abelmilash-msft
- Fix pip install command in batch example (PowerPlatform-Dataverse-Client) - Hoist entity_set call above if-check in _resolve_record_create - Extract _require_entity_metadata helper for duplicate table lookups - Add inline comment explaining 400 in expected status codes - Add annotation explaining why example 5 is commented out - Populate __all__ in operations/batch.py with public classes - Use _GUID_RE from _odata.py for consistent GUID regex - Add _resolve_record_update tests (single, multiple, invalid changes) - Add delete tests (multiple IDs, no-bulk, empty list, empty strings)
1 parent e7a3ee0 commit be47e9e

4 files changed

Lines changed: 116 additions & 28 deletions

File tree

examples/advanced/batch.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
HTTP request to the Dataverse Web API.
99
1010
Requirements:
11-
pip install "PowerPlatform.Dataverse" azure-identity
11+
pip install PowerPlatform-Dataverse-Client azure-identity
1212
"""
1313

1414
from __future__ import annotations
@@ -144,7 +144,8 @@
144144

145145
print("\n[INFO] Example 5: Mixed batch")
146146

147-
# Assume account_id exists
147+
# NOTE: Commented out because it requires a pre-existing account_id.
148+
# Uncomment and set account_id to run this example.
148149
# batch = client.batch.new()
149150
#
150151
# with batch.changeset() as cs:

src/PowerPlatform/Dataverse/data/_batch.py

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ def execute(
276276
url,
277277
data=body.encode("utf-8"),
278278
headers=headers,
279+
# 400 is expected: Dataverse returns 400 for top-level batch
280+
# errors (e.g. malformed body). We parse the response body to
281+
# surface the service error via _parse_batch_response /
282+
# _raise_top_level_batch_error rather than letting _request raise.
279283
expected=(200, 202, 207, 400),
280284
)
281285
return self._parse_batch_response(response)
@@ -353,10 +357,9 @@ def _resolve_one(self, item: Any) -> _RawRequest:
353357
# ------------------------------------------------------------------
354358

355359
def _resolve_record_create(self, op: _RecordCreate) -> List[_RawRequest]:
360+
entity_set = self._od._entity_set_from_schema_name(op.table)
356361
if isinstance(op.data, dict):
357-
entity_set = self._od._entity_set_from_schema_name(op.table)
358362
return [self._od._build_create(entity_set, op.table, op.data, content_id=op.content_id)]
359-
entity_set = self._od._entity_set_from_schema_name(op.table)
360363
return [self._od._build_create_multiple(entity_set, op.table, op.data)]
361364

362365
def _resolve_record_update(self, op: _RecordUpdate) -> List[_RawRequest]:
@@ -398,17 +401,22 @@ def _resolve_record_upsert(self, op: _RecordUpsert) -> List[_RawRequest]:
398401
# specific lookups needed before the relevant _build_* call)
399402
# ------------------------------------------------------------------
400403

401-
def _resolve_table_create(self, op: _TableCreate) -> List[_RawRequest]:
402-
return [self._od._build_create_entity(op.table, op.columns, op.solution, op.primary_column)]
403-
404-
def _resolve_table_delete(self, op: _TableDelete) -> List[_RawRequest]:
405-
ent = self._od._get_entity_by_table_schema_name(op.table)
404+
def _require_entity_metadata(self, table: str) -> str:
405+
"""Look up MetadataId for *table*, raising MetadataError if not found."""
406+
ent = self._od._get_entity_by_table_schema_name(table)
406407
if not ent or not ent.get("MetadataId"):
407408
raise MetadataError(
408-
f"Table '{op.table}' not found.",
409+
f"Table '{table}' not found.",
409410
subcode=METADATA_TABLE_NOT_FOUND,
410411
)
411-
return [self._od._build_delete_entity(ent["MetadataId"])]
412+
return ent["MetadataId"]
413+
414+
def _resolve_table_create(self, op: _TableCreate) -> List[_RawRequest]:
415+
return [self._od._build_create_entity(op.table, op.columns, op.solution, op.primary_column)]
416+
417+
def _resolve_table_delete(self, op: _TableDelete) -> List[_RawRequest]:
418+
metadata_id = self._require_entity_metadata(op.table)
419+
return [self._od._build_delete_entity(metadata_id)]
412420

413421
def _resolve_table_get(self, op: _TableGet) -> List[_RawRequest]:
414422
return [self._od._build_get_entity(op.table)]
@@ -417,25 +425,12 @@ def _resolve_table_list(self, op: _TableList) -> List[_RawRequest]:
417425
return [self._od._build_list_entities(filter=op.filter, select=op.select)]
418426

419427
def _resolve_table_add_columns(self, op: _TableAddColumns) -> List[_RawRequest]:
420-
ent = self._od._get_entity_by_table_schema_name(op.table)
421-
if not ent or not ent.get("MetadataId"):
422-
raise MetadataError(
423-
f"Table '{op.table}' not found.",
424-
subcode=METADATA_TABLE_NOT_FOUND,
425-
)
426-
return [
427-
self._od._build_create_column(ent["MetadataId"], col_name, dtype) for col_name, dtype in op.columns.items()
428-
]
428+
metadata_id = self._require_entity_metadata(op.table)
429+
return [self._od._build_create_column(metadata_id, col_name, dtype) for col_name, dtype in op.columns.items()]
429430

430431
def _resolve_table_remove_columns(self, op: _TableRemoveColumns) -> List[_RawRequest]:
431432
columns = [op.columns] if isinstance(op.columns, str) else list(op.columns)
432-
ent = self._od._get_entity_by_table_schema_name(op.table)
433-
if not ent or not ent.get("MetadataId"):
434-
raise MetadataError(
435-
f"Table '{op.table}' not found.",
436-
subcode=METADATA_TABLE_NOT_FOUND,
437-
)
438-
metadata_id = ent["MetadataId"]
433+
metadata_id = self._require_entity_metadata(op.table)
439434
requests: List[_RawRequest] = []
440435
for col_name in columns:
441436
attr_meta = self._od._get_attribute_metadata(

src/PowerPlatform/Dataverse/operations/batch.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,16 @@
4444
if TYPE_CHECKING:
4545
from ..client import DataverseClient
4646

47-
__all__ = []
47+
__all__ = [
48+
"BatchRecordOperations",
49+
"BatchTableOperations",
50+
"BatchQueryOperations",
51+
"BatchDataFrameOperations",
52+
"BatchRequest",
53+
"BatchOperations",
54+
"ChangeSet",
55+
"ChangeSetRecordOperations",
56+
]
4857

4958

5059
# ---------------------------------------------------------------------------

tests/unit/data/test_batch_serialization.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
_RecordCreate,
1515
_RecordDelete,
1616
_RecordGet,
17+
_RecordUpdate,
1718
_RecordUpsert,
1819
_TableGet,
1920
_TableList,
@@ -258,6 +259,88 @@ def test_resolve_record_delete_single(self):
258259
od._build_delete.assert_called_once_with("account", "guid-1", content_id=None)
259260
self.assertEqual(result, [mock_req])
260261

262+
def test_resolve_record_update_single(self):
263+
client, od = self._client_and_od()
264+
mock_req = MagicMock()
265+
od._build_update.return_value = mock_req
266+
267+
op = _RecordUpdate(table="account", ids="guid-1", changes={"name": "Updated"})
268+
result = client._resolve_record_update(op)
269+
270+
od._build_update.assert_called_once_with("account", "guid-1", {"name": "Updated"}, content_id=None)
271+
self.assertEqual(result, [mock_req])
272+
273+
def test_resolve_record_update_multiple(self):
274+
client, od = self._client_and_od()
275+
mock_req = MagicMock()
276+
od._build_update_multiple.return_value = mock_req
277+
278+
op = _RecordUpdate(
279+
table="account",
280+
ids=["guid-1", "guid-2"],
281+
changes=[{"name": "A"}, {"name": "B"}],
282+
)
283+
result = client._resolve_record_update(op)
284+
285+
od._build_update_multiple.assert_called_once()
286+
self.assertEqual(result, [mock_req])
287+
288+
def test_resolve_record_update_single_with_list_changes_raises(self):
289+
client, od = self._client_and_od()
290+
from PowerPlatform.Dataverse.core.errors import ValidationError
291+
292+
op = _RecordUpdate(table="account", ids="guid-1", changes=[{"name": "A"}])
293+
with self.assertRaises(ValidationError):
294+
client._resolve_record_update(op)
295+
296+
def test_resolve_record_delete_multiple_ids(self):
297+
client, od = self._client_and_od()
298+
mock_req = MagicMock()
299+
od._build_delete_multiple.return_value = mock_req
300+
301+
op = _RecordDelete(table="account", ids=["guid-1", "guid-2", "guid-3"])
302+
result = client._resolve_record_delete(op)
303+
304+
od._build_delete_multiple.assert_called_once_with("account", ["guid-1", "guid-2", "guid-3"])
305+
self.assertEqual(result, [mock_req])
306+
307+
def test_resolve_record_delete_multiple_no_bulk(self):
308+
client, od = self._client_and_od()
309+
mock_req = MagicMock()
310+
od._build_delete.return_value = mock_req
311+
312+
op = _RecordDelete(table="account", ids=["guid-1", "guid-2"], use_bulk_delete=False)
313+
result = client._resolve_record_delete(op)
314+
315+
self.assertEqual(od._build_delete.call_count, 2)
316+
self.assertEqual(len(result), 2)
317+
318+
def test_resolve_record_delete_empty_ids_returns_empty(self):
319+
client, od = self._client_and_od()
320+
321+
op = _RecordDelete(table="account", ids=[])
322+
result = client._resolve_record_delete(op)
323+
324+
self.assertEqual(result, [])
325+
326+
def test_resolve_record_delete_filters_empty_strings(self):
327+
client, od = self._client_and_od()
328+
mock_req = MagicMock()
329+
od._build_delete_multiple.return_value = mock_req
330+
331+
op = _RecordDelete(table="account", ids=["guid-1", "", "guid-2", ""])
332+
result = client._resolve_record_delete(op)
333+
334+
od._build_delete_multiple.assert_called_once_with("account", ["guid-1", "guid-2"])
335+
336+
def test_resolve_record_delete_all_empty_strings_returns_empty(self):
337+
client, od = self._client_and_od()
338+
339+
op = _RecordDelete(table="account", ids=["", "", ""])
340+
result = client._resolve_record_delete(op)
341+
342+
self.assertEqual(result, [])
343+
261344
def test_resolve_table_get(self):
262345
client, od = self._client_and_od()
263346
mock_req = MagicMock()

0 commit comments

Comments
 (0)