Skip to content

Commit f43577d

Browse files
author
Samson Gebre
committed
feat: implement shared Content-ID counter for batch changesets and enhance related tests
1 parent b19da41 commit f43577d

4 files changed

Lines changed: 186 additions & 46 deletions

File tree

examples/basic/functional_testing.py

Lines changed: 140 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -352,11 +352,13 @@ def test_sql_encoding(
352352
batch = client.batch.new()
353353
batch.query.sql(basic_sql)
354354
result = batch.execute()
355-
batch_count = len(result.responses[0].data.get("value", [])) if result.responses and result.responses[0].is_success and result.responses[0].data else 0
356-
357-
assert direct_count == batch_count, (
358-
f"Row count mismatch: client={direct_count}, batch={batch_count}"
355+
batch_count = (
356+
len(result.responses[0].data.get("value", []))
357+
if result.responses and result.responses[0].is_success and result.responses[0].data
358+
else 0
359359
)
360+
361+
assert direct_count == batch_count, f"Row count mismatch: client={direct_count}, batch={batch_count}"
360362
print(f" [OK] Both paths returned {direct_count} rows")
361363

362364
# ------------------------------------------------------------------
@@ -383,16 +385,12 @@ def test_sql_encoding(
383385
else 0
384386
)
385387

386-
assert direct_where_count == batch_where_count, (
387-
f"Row count mismatch on WHERE query: client={direct_where_count}, batch={batch_where_count}"
388-
)
389-
assert direct_where_count == 1, (
390-
f"Expected exactly 1 row for known record name, got {direct_where_count}"
391-
)
388+
assert (
389+
direct_where_count == batch_where_count
390+
), f"Row count mismatch on WHERE query: client={direct_where_count}, batch={batch_where_count}"
391+
assert direct_where_count == 1, f"Expected exactly 1 row for known record name, got {direct_where_count}"
392392
direct_name = direct_rows_where[0].get(name_col)
393-
assert direct_name == known_name, (
394-
f"Returned name '{direct_name}' does not match expected '{known_name}'"
395-
)
393+
assert direct_name == known_name, f"Returned name '{direct_name}' does not match expected '{known_name}'"
396394
print(f" [OK] Both paths found the record: '{direct_name}'")
397395
else:
398396
print(" [2/3] Skipped WHERE test — record name not available in retrieved_record")
@@ -422,12 +420,10 @@ def test_sql_encoding(
422420
else 0
423421
)
424422

425-
assert direct_eq_count == batch_eq_count, (
426-
f"Row count mismatch on '=' query: client={direct_eq_count}, batch={batch_eq_count}"
427-
)
428-
assert direct_eq_count == 1, (
429-
f"Expected 1 row for '=' record, got {direct_eq_count}"
430-
)
423+
assert (
424+
direct_eq_count == batch_eq_count
425+
), f"Row count mismatch on '=' query: client={direct_eq_count}, batch={batch_eq_count}"
426+
assert direct_eq_count == 1, f"Expected 1 row for '=' record, got {direct_eq_count}"
431427
print(f" [OK] Both paths found record with '=' in name: '{direct_eq_rows[0].get(name_col)}'")
432428
finally:
433429
client.records.delete(table_schema_name, eq_id)
@@ -452,9 +448,15 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
452448
records.delete (multi, use_bulk_delete=False)
453449
records.upsert (graceful — requires configured alternate key)
454450
tables.get, tables.list
451+
tables.add_columns + tables.remove_columns (two requests, each adding
452+
one column, verified then removed in a second batch)
455453
query.sql
456454
changeset happy path (create + update via content-ID ref + delete)
457455
changeset rollback (failing op rolls back entire changeset)
456+
two changesets in one batch (Content-IDs are globally unique across
457+
the batch via a shared counter)
458+
content-ID reference chaining ($n refs) across multiple creates in one
459+
changeset — regression guard for the shared counter fix
458460
execute(continue_on_error=True) — mixed success/failure
459461
"""
460462
print("\n-> Batch Operations Test (All Operations)")
@@ -467,9 +469,9 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
467469

468470
try:
469471
# -------------------------------------------------------------------
470-
# [1/8] CREATE — single record + CreateMultiple (list) in one batch
472+
# [1/11] CREATE — single record + CreateMultiple (list) in one batch
471473
# -------------------------------------------------------------------
472-
print("\n[1/8] Create — single + CreateMultiple (2 ops, 1 POST $batch)")
474+
print("\n[1/11] Create — single + CreateMultiple (2 ops, 1 POST $batch)")
473475
batch = client.batch.new()
474476
batch.records.create(
475477
table_schema_name,
@@ -503,11 +505,11 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
503505
print(f"[OK] {len(result.succeeded)} ops → {len(all_ids)} records created: {all_ids}")
504506

505507
# -------------------------------------------------------------------
506-
# [2/8] READ — get by ID + tables.get + tables.list + query.sql
508+
# [2/11] READ — get by ID + tables.get + tables.list + query.sql
507509
# All 4 reads in one batch request
508510
# -------------------------------------------------------------------
509511
if all_ids:
510-
print("\n[2/8] Read — records.get + tables.get + tables.list + query.sql (4 ops, 1 POST $batch)")
512+
print("\n[2/11] Read — records.get + tables.get + tables.list + query.sql (4 ops, 1 POST $batch)")
511513
batch = client.batch.new()
512514
batch.records.get(
513515
table_schema_name,
@@ -537,22 +539,22 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
537539
print(f" query.sql → {len(resp.data.get('value', []))} rows returned")
538540

539541
# -------------------------------------------------------------------
540-
# [3/8] UPDATE — single PATCH + UpdateMultiple (broadcast) in one batch
542+
# [3/11] UPDATE — single PATCH + UpdateMultiple (broadcast) in one batch
541543
# -------------------------------------------------------------------
542544
if len(all_ids) >= 3:
543-
print(f"\n[3/8] Update — single PATCH + UpdateMultiple ({len(all_ids)} records, 2 ops, 1 POST $batch)")
545+
print(f"\n[3/11] Update — single PATCH + UpdateMultiple ({len(all_ids)} records, 2 ops, 1 POST $batch)")
544546
batch = client.batch.new()
545547
batch.records.update(table_schema_name, all_ids[0], {f"{attr_prefix}_count": 10})
546548
batch.records.update(table_schema_name, all_ids[1:], {f"{attr_prefix}_count": 20})
547549
result = batch.execute()
548550
print(f"[OK] {len(result.succeeded)} updates succeeded, {len(result.failed)} failed")
549551

550552
# -------------------------------------------------------------------
551-
# [4/8] CHANGESET (happy path) — create + update via content-ID + delete
553+
# [4/11] CHANGESET (happy path) — create + update via content-ID + delete
552554
# All three changeset operation types committed atomically
553555
# -------------------------------------------------------------------
554556
if len(all_ids) >= 1:
555-
print("\n[4/8] Changeset (happy path) — cs.create + cs.update(ref) + cs.delete (1 transaction)")
557+
print("\n[4/11] Changeset (happy path) — cs.create + cs.update(ref) + cs.delete (1 transaction)")
556558
batch = client.batch.new()
557559
with batch.changeset() as cs:
558560
ref = cs.records.create(
@@ -576,9 +578,9 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
576578
print(f"[OK] {len(result.succeeded)} ops committed atomically (create + update + delete)")
577579

578580
# -------------------------------------------------------------------
579-
# [5/8] CHANGESET (rollback) — failing update rolls back the create
581+
# [5/11] CHANGESET (rollback) — failing update rolls back the create
580582
# -------------------------------------------------------------------
581-
print("\n[5/8] Changeset (rollback) — cs.create + cs.update(nonexistent) → full rollback")
583+
print("\n[5/11] Changeset (rollback) — cs.create + cs.update(nonexistent) → full rollback")
582584
nonexistent_id = "00000000-0000-0000-0000-000000000001"
583585
batch = client.batch.new()
584586
with batch.changeset() as cs:
@@ -591,7 +593,10 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
591593
},
592594
)
593595
cs.records.update(table_schema_name, nonexistent_id, {f"{attr_prefix}_count": 999})
594-
result = batch.execute()
596+
# continue_on_error=True ensures Dataverse returns a 200 multipart response
597+
# with the changeset failure embedded, rather than propagating the inner
598+
# 404 to the outer batch HTTP status (which some environments do).
599+
result = batch.execute(continue_on_error=True)
595600
if result.has_errors:
596601
leaked = list(result.created_ids)
597602
if not leaked:
@@ -604,10 +609,109 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
604609
all_ids.extend(result.created_ids)
605610

606611
# -------------------------------------------------------------------
607-
# [6/8] UPSERT — requires an alternate key configured on the table.
612+
# [6/11] TWO CHANGESETS — Content-IDs are unique across the entire batch
613+
# (shared counter). Verifies both changesets commit atomically.
614+
# -------------------------------------------------------------------
615+
print("\n[6/11] Two changesets in one batch — globally unique Content-IDs across changesets")
616+
batch = client.batch.new()
617+
with batch.changeset() as cs1:
618+
ref1 = cs1.records.create(
619+
table_schema_name,
620+
{
621+
f"{attr_prefix}_name": f"CS1-E {datetime.now().strftime('%H:%M:%S')}",
622+
f"{attr_prefix}_count": 10,
623+
f"{attr_prefix}_is_active": False,
624+
},
625+
)
626+
cs1.records.update(table_schema_name, ref1, {f"{attr_prefix}_is_active": True})
627+
with batch.changeset() as cs2:
628+
ref2 = cs2.records.create(
629+
table_schema_name,
630+
{
631+
f"{attr_prefix}_name": f"CS2-F {datetime.now().strftime('%H:%M:%S')}",
632+
f"{attr_prefix}_count": 20,
633+
f"{attr_prefix}_is_active": False,
634+
},
635+
)
636+
cs2.records.update(table_schema_name, ref2, {f"{attr_prefix}_is_active": True})
637+
result = batch.execute()
638+
if result.has_errors:
639+
for item in result.failed:
640+
print(f"[WARN] Two-changeset error {item.status_code}: {item.error_message}")
641+
else:
642+
cs_ids = list(result.created_ids)
643+
all_ids.extend(cs_ids)
644+
print(
645+
f"[OK] Both changesets committed — {len(cs_ids)} records created "
646+
f"with globally unique Content-IDs across changesets: {cs_ids}"
647+
)
648+
649+
# -------------------------------------------------------------------
650+
# [7/11] CONTENT-ID REFERENCE CHAINING — two creates in one changeset,
651+
# each update references its own $n — regression guard for the
652+
# shared-counter fix (ensures references stay self-consistent).
653+
# -------------------------------------------------------------------
654+
print("\n[7/11] Content-ID reference chaining — two creates + two updates via $n refs")
655+
batch = client.batch.new()
656+
with batch.changeset() as cs:
657+
ref_a = cs.records.create(
658+
table_schema_name,
659+
{
660+
f"{attr_prefix}_name": f"Chain-A {datetime.now().strftime('%H:%M:%S')}",
661+
f"{attr_prefix}_count": 0,
662+
f"{attr_prefix}_is_active": False,
663+
},
664+
)
665+
ref_b = cs.records.create(
666+
table_schema_name,
667+
{
668+
f"{attr_prefix}_name": f"Chain-B {datetime.now().strftime('%H:%M:%S')}",
669+
f"{attr_prefix}_count": 0,
670+
f"{attr_prefix}_is_active": False,
671+
},
672+
)
673+
# Update both records via their content-ID references
674+
cs.records.update(table_schema_name, ref_a, {f"{attr_prefix}_count": 100})
675+
cs.records.update(table_schema_name, ref_b, {f"{attr_prefix}_count": 200})
676+
result = batch.execute()
677+
if result.has_errors:
678+
for item in result.failed:
679+
print(f"[WARN] Chaining error {item.status_code}: {item.error_message}")
680+
else:
681+
chain_ids = list(result.created_ids)
682+
all_ids.extend(chain_ids)
683+
print(f"[OK] Both records created and updated via content-ID refs " f"{ref_a} and {ref_b}: {chain_ids}")
684+
685+
# -------------------------------------------------------------------
686+
# [8/11] BATCH TABLES ADD COLUMNS — two batch.tables.add_columns()
687+
# requests in one batch, each adding one column. Verifies
688+
# that metadata write operations work inside a $batch request.
689+
# The two columns are removed via a follow-up batch after the
690+
# assertion so they do not accumulate on the test table.
691+
# -------------------------------------------------------------------
692+
col_a = f"{attr_prefix}_batch_extra_a"
693+
col_b = f"{attr_prefix}_batch_extra_b"
694+
print(f"\n[8/11] Batch tables.add_columns — two add-column requests in one batch")
695+
batch = client.batch.new()
696+
batch.tables.add_columns(table_schema_name, {col_a: "string"})
697+
batch.tables.add_columns(table_schema_name, {col_b: "int"})
698+
result = batch.execute()
699+
if result.has_errors:
700+
for item in result.failed:
701+
print(f"[WARN] add_columns error {item.status_code}: {item.error_message}")
702+
else:
703+
print(f"[OK] {len(result.succeeded)} column(s) added via batch: {col_a}, {col_b}")
704+
# Remove the two test columns so the table stays clean
705+
batch_rm = client.batch.new()
706+
batch_rm.tables.remove_columns(table_schema_name, [col_a, col_b])
707+
rm_result = batch_rm.execute(continue_on_error=True)
708+
print(f"[OK] Removed {len(rm_result.succeeded)} batch-added column(s) via batch.tables.remove_columns")
709+
710+
# -------------------------------------------------------------------
711+
# [9/11] UPSERT — requires an alternate key configured on the table.
608712
# The test table has none, so this is expected to fail (graceful).
609713
# -------------------------------------------------------------------
610-
print(f"\n[6/8] Upsert — UpsertItem with alternate key (expected to fail: no alt key on test table)")
714+
print(f"\n[9/11] Upsert — UpsertItem with alternate key (expected to fail: no alt key on test table)")
611715
try:
612716
batch = client.batch.new()
613717
batch.records.upsert(
@@ -630,11 +734,11 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
630734
print(f"[WARN] Upsert skipped due to exception: {e}")
631735

632736
# -------------------------------------------------------------------
633-
# [7/8] MIXED BATCH with continue_on_error
737+
# [10/11] MIXED BATCH with continue_on_error
634738
# One intentional 404 alongside a valid get — both attempted
635739
# -------------------------------------------------------------------
636740
if all_ids:
637-
print(f"\n[7/8] Mixed batch (continue_on_error=True) — 1 bad get + 1 good get")
741+
print(f"\n[10/11] Mixed batch (continue_on_error=True) — 1 bad get + 1 good get")
638742
batch = client.batch.new()
639743
batch.records.get(
640744
table_schema_name,
@@ -652,10 +756,10 @@ def test_batch_all_operations(client: DataverseClient, table_info: Dict[str, Any
652756
print(f" Expected failure: {item.status_code} {item.error_message}")
653757

654758
# -------------------------------------------------------------------
655-
# [8/8] DELETE — multi-delete (use_bulk_delete=False → individual DELETEs)
759+
# [11/11] DELETE — multi-delete (use_bulk_delete=False → individual DELETEs)
656760
# -------------------------------------------------------------------
657761
if all_ids:
658-
print(f"\n[8/8] Delete — {len(all_ids)} records via multi-delete (use_bulk_delete=False, 1 POST $batch)")
762+
print(f"\n[11/11] Delete — {len(all_ids)} records via multi-delete (use_bulk_delete=False, 1 POST $batch)")
659763
batch = client.batch.new()
660764
batch.records.delete(table_schema_name, all_ids, use_bulk_delete=False)
661765
result = batch.execute(continue_on_error=True)

src/PowerPlatform/Dataverse/data/_batch.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,28 +167,38 @@ class _QuerySql:
167167

168168
@dataclass
169169
class _ChangeSet:
170-
"""Ordered group of single-record write operations that execute atomically."""
170+
"""Ordered group of single-record write operations that execute atomically.
171+
172+
Content-IDs are allocated from ``_counter``, a single-element ``List[int]``
173+
that is shared across all changesets in the same batch. Passing the same
174+
list object to every ``_ChangeSet`` created by a :class:`BatchRequest`
175+
ensures Content-ID values are unique within the entire batch request, not
176+
just within an individual changeset, as required by the OData spec.
177+
178+
When constructed in isolation (e.g. in unit tests), ``_counter`` defaults
179+
to a fresh ``[1]`` so the class remains self-contained.
180+
"""
171181

172182
operations: List[Union[_RecordCreate, _RecordUpdate, _RecordDelete]] = field(default_factory=list)
173-
_next_content_id: int = field(default=1, init=False, repr=False)
183+
_counter: List[int] = field(default_factory=lambda: [1], repr=False)
174184

175185
def add_create(self, table: str, data: Dict[str, Any]) -> str:
176186
"""Add a single-record create; return its content-ID reference string."""
177-
cid = self._next_content_id
178-
self._next_content_id += 1
187+
cid = self._counter[0]
188+
self._counter[0] += 1
179189
self.operations.append(_RecordCreate(table=table, data=data, content_id=cid))
180190
return f"${cid}"
181191

182192
def add_update(self, table: str, record_id: str, changes: Dict[str, Any]) -> None:
183193
"""Add a single-record update (record_id may be a '$n' reference)."""
184-
cid = self._next_content_id
185-
self._next_content_id += 1
194+
cid = self._counter[0]
195+
self._counter[0] += 1
186196
self.operations.append(_RecordUpdate(table=table, ids=record_id, changes=changes, content_id=cid))
187197

188198
def add_delete(self, table: str, record_id: str) -> None:
189199
"""Add a single-record delete (record_id may be a '$n' reference)."""
190-
cid = self._next_content_id
191-
self._next_content_id += 1
200+
cid = self._counter[0]
201+
self._counter[0] += 1
192202
self.operations.append(_RecordDelete(table=table, ids=record_id, content_id=cid))
193203

194204

src/PowerPlatform/Dataverse/operations/batch.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@ class BatchRequest:
568568
def __init__(self, client: "DataverseClient") -> None:
569569
self._client = client
570570
self._items: list = []
571+
self._content_id_counter: List[int] = [1] # shared across all changesets
571572
self.records = BatchRecordOperations(self)
572573
self.tables = BatchTableOperations(self)
573574
self.query = BatchQueryOperations(self)
@@ -587,7 +588,7 @@ def changeset(self) -> ChangeSet:
587588
cs.records.create("account", {"name": "ACME"})
588589
cs.records.create("contact", {"firstname": "Bob"})
589590
"""
590-
internal = _ChangeSet()
591+
internal = _ChangeSet(_counter=self._content_id_counter)
591592
self._items.append(internal)
592593
return ChangeSet(internal)
593594

0 commit comments

Comments
 (0)