Cosmos: Cut over replace_item, upsert_item, delete_item to driver#4128
Conversation
Route replace_item, upsert_item, and delete_item through the driver instead of the legacy gateway pipeline, following the same pattern established by create_item and read_item. - Change CosmosOperation::upsert_item to take (ContainerReference, PartitionKey) instead of ItemReference, since upsert is a POST to the collection feed (same as create). - Remove ItemWriteOptions::apply_headers() shim and the apply_precondition_headers helper, no longer needed now that all item operations use the driver. - Remove tests for the removed gateway shim code. Agent-Logs-Url: https://github.com/Azure/azure-sdk-for-rust/sessions/9397eee6-2f94-4a70-ac3e-253e5fff4f90 Co-authored-by: simorenoh <30335873+simorenoh@users.noreply.github.com>
…dition wiring Consolidates the repeated session_token and precondition wiring into a shared apply_item_options() function used by all five item operations (read, create, replace, upsert, delete). Agent-Logs-Url: https://github.com/Azure/azure-sdk-for-rust/sessions/1e363c06-e4bc-4fca-88d5-80527c2f5d92 Co-authored-by: simorenoh <30335873+simorenoh@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Completes the Cosmos item-operation cutover in azure_data_cosmos by routing replace_item, upsert_item, and delete_item through CosmosDriver, aligning them with the existing driver-based create_item/read_item path and removing now-dead header-shim code from the SDK pipeline.
Changes:
- Cut over
ContainerClient::{replace_item, upsert_item, delete_item}to buildCosmosOperation, apply session token/precondition, execute viaCosmosDriver, and bridge the response back to SDK types. - Updated driver API:
CosmosOperation::upsert_itemnow takes(ContainerReference, PartitionKey)to reflect REST semantics (POST to collection feed). - Removed legacy SDK-pipeline header application helpers for
ItemWriteOptionsand deleted the associated unit tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| sdk/cosmos/azure_data_cosmos/src/options/mod.rs | Removes ItemWriteOptions header-application shims and their tests now that item writes are driver-based. |
| sdk/cosmos/azure_data_cosmos/src/cosmos_request.rs | Removes a test that depended on the deleted ItemWriteOptions::apply_headers behavior. |
| sdk/cosmos/azure_data_cosmos/src/clients/container_client.rs | Routes replace/upsert/delete through the driver and factors session-token/precondition wiring into a shared helper. |
| sdk/cosmos/azure_data_cosmos_driver/src/models/cosmos_operation.rs | Updates upsert_item to accept container + partition key and build a feed reference for POST upsert. |
…-driver-migration
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…-driver-migration
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…-driver-migration
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
b50fb4c
into
release/azure_data_cosmos-previews
## What This PR makes `create_item` and `upsert_item` require an explicit item id, matching the pattern already used by `read_item`, `replace_item`, and `delete_item`. ### Breaking Changes **`azure_data_cosmos` (SDK):** - `ContainerClient::create_item()` signature changed from `(partition_key, item, options)` to `(partition_key, item_id, item, options)` - `ContainerClient::upsert_item()` signature changed from `(partition_key, item, options)` to `(partition_key, item_id, item, options)` **`azure_data_cosmos_driver` (internal):** - `CosmosOperation::create_item()` now takes `ItemReference` instead of `(ContainerReference, PartitionKey)` - `CosmosOperation::upsert_item()` now takes `ItemReference` instead of `(ContainerReference, PartitionKey)` All five point operations (`create_item`, `upsert_item`, `read_item`, `replace_item`, `delete_item`) now follow the same pattern in both the SDK and the driver. ## Why PR #4128 cut over more point operations to the driver but inadvertently removed the `ItemReference`-based overloads for `create_item` and `upsert_item`. This was a regression — the original design requires the caller to provide the item id explicitly so the driver never needs to parse the request body to extract the document id. Parsing the body in the driver is both unnecessary overhead and a layering violation (the SDK should own serialization concerns). ## How - **Driver (`cosmos_operation.rs`):** `create_item` and `upsert_item` factory methods now take `ItemReference` and call `Self::new(OperationType::Create/Upsert, item)`, identical to `read_item`/`delete_item`/`replace_item`. - **Driver (`operation_pipeline.rs`):** `build_transport_request` detects Create/Upsert Document operations and uses `compute_feed_paths()` instead of `compute_paths()`, since these operations POST to the collection feed URL even though the operation carries the item id. - **Driver (`cosmos_resource_reference.rs`):** Added `compute_feed_paths()` to compute feed-style paths (parent URL + signing link) for references that carry a leaf id. - **SDK (`container_client.rs`):** `create_item` and `upsert_item` now accept `item_id: &str` and construct an `ItemReference` before delegating to the driver. - All tests, examples, benchmarks, and perf tooling updated. ## Testing Existing test coverage is sufficient
Migrates `ContainerClient::execute_transactional_batch()` from the legacy `CosmosRequest` + `BatchOptions::apply_headers()` + `container_connection.send()` pipeline to `CosmosOperation` + `driver.execute_operation()`, matching the pattern established in #4147, #4174, and #4128. This was the last item-level operation using the old SDK pipeline for header manipulation, so the batch-specific helpers are removed as dead code. ## Changes **`azure_data_cosmos_driver/src/models/cosmos_operation.rs`** - Added `CosmosOperation::batch()` factory (POST to items feed with partition key, `OperationType::Batch`), following the `create_item` pattern. **`azure_data_cosmos/src/clients/container_client.rs`** - Rewrote `execute_transactional_batch` to build a `CosmosOperation::batch`, apply options, execute through the driver, and bridge the response. - Added `apply_batch_options` helper that takes `&BatchOptions` and wires session token onto the operation. - Removed the `items_link` field from `ContainerClient` — it was only used by the batch method's old `CosmosRequest` path. **`azure_data_cosmos/src/options/mod.rs`** - Removed `BatchOptions::apply_headers()`. - Removed the `apply_content_response_on_write_header()` helper (sole caller was `BatchOptions::apply_headers`). - Removed 4 batch header tests that validated the removed methods. - Cleaned up unused imports. **`azure_data_cosmos/src/constants.rs`** - Removed the `PREFER_MINIMAL` constant, which is no longer referenced in the SDK crate. The driver handles this header internally via its operation pipeline.
Route ContainerClient::read(), replace(), and delete() through CosmosDriver instead of the legacy CosmosRequest + ContainerConnection pipeline, following the pattern established in #4147 and #4128. - Add replace_container factory to CosmosOperation - Rewrite read(), replace(), delete() to use CosmosOperation + driver.execute_operation() - Remove unused link field from ContainerClient - Remove unused CosmosResponse import
…st, AC/test gaps) Addresses PR Deep Reviewer iteration 1 findings: - 1/2/3: Rewrite section 4.4 idempotency soundness. The driver retry surface has at least four pathways in retry_evaluation.rs and only two consult is_idempotent(); try_handle_server_error (5xx, line 457) and try_handle_write_forbidden (403/3, line 289) retry cross-region unconditionally, and PPAF was designed for non-idempotent retries. Soundness now rests on document-level wire-body idempotency, not on a // SAFETY: comment over a single retry gate. - 4/5: Section 4.2 PatchItemOptions concrete shape and ItemWriteOptions applicability table. - 6: Remove-op unit test now requires byte-exact assertion. - 7: Section 4.5 corrects PR Azure#4128 precedent characterization (added zero new variants; precedent is leave-legacy-alone, not only-add-to-driver). - 8: Adds 4 acceptance criteria + integration tests (etag x predicate conjunction, soft-delete-of-soft-deleted, partition split, PPCB). - 9: Section 11.2 open question rewritten for FabianMeiswinkel. - Polish 10/11/12: rename_all serde note, README example with filter_predicate + content_response_on_write=false, phase-2 constraints subsection in Out of Scope. - Section 7.2 Alternative B rejection extended to mention the 5xx and 403/3 retry paths that bypass is_idempotent(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Routes the remaining item-level operations (
replace_item,upsert_item,delete_item) through theCosmosDriver, completing the item operation cutover from the legacy gateway pipeline.Driver
CosmosOperation::upsert_itemnow takes(ContainerReference, PartitionKey)instead ofItemReference. Upsert is aPOSTto the collection feed (same as create), not aPUTto a document URL.SDK
replace_item,upsert_item,delete_iteminContainerClientfollow the same pattern ascreate_item/read_item: buildCosmosOperation→ wire session token/precondition →driver.execute_operation()→ bridge response.Cleanup
ItemWriteOptions::apply_headers()andapply_precondition_headers()— dead code now that all item writes use the driver.Updated state
read_itemcreate_itemreplace_itemupsert_itemdelete_itemquery_itemsWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
test.documents.azure.com/home/REDACTED/work/azure-sdk-for-rust/azure-sdk-for-rust/target/debug/deps/azure_data_cosmos-6ba76bd8313db34b /home/REDACTED/work/azure-sdk-for-rust/azure-sdk-for-rust/target/debug/deps/azure_data_cosmos-6ba76bd8313db34b /home/REDACTED/work/azure-sdk-for-rust/azure-sdk-for-rust/target/debug/build/azure_core_test-3ead228e4bac545f/build_script_build-3ead228e4bac545f.28rxmzcc53ieso0vhgnjhp0mb.1g0umtu.rcgu.o -Wl,--as-needed -Wl,-Bstatic get/debug/deps/rustcVORl3H/list /home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libpanic_unwind-e462f106b2b26a06.rlib /home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib ORl3H/symbols.o core_test_macros-a1ab7b6aba03e8ee.00uyiyx5pyx2v1ngfia649863.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.0wxt1muwk7ybp3n7mh0wllbni.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.10a2tbvi9x7z0h5lmbonpdh71.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.16kd4stujzqt4xf2jl2sbtd59.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.1kccwp8eolfs8k1p8t95mpyuj.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.1lmiqx49m2x5aacy6jphmlzgy.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.1zbl7otmqptz39v95knjdsqes.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.2f42o1vbvljajd8rp0o10wval.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.2zwx85bqwtramffcrv2xdiq6i.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.31h1vvrpiuwu05d0sfhyv10f4.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.34eawspeaufijc3hxiybm1m5s.1iuy4et.rcgu.o core_test_macros-a1ab7b6aba03e8ee.3oxva6e4m60m4raphd0wdn5rb.1iuy4et.rcgu.o(dns block)/home/REDACTED/work/azure-sdk-for-rust/azure-sdk-for-rust/target/debug/deps/azure_data_cosmos-6ba76bd8313db34b /home/REDACTED/work/azure-sdk-for-rust/azure-sdk-for-rust/target/debug/deps/azure_data_cosmos-6ba76bd8313db34b _data_cosmos-ef5244a14879f580/build_script_build-ef5244a14879f580.5hz7v9l3n3w76s28d4�� _data_cosmos-ef5244a14879f580/build_script_build-ef5244a14879f580.b3xwjkkkf035nfaknghx3bnrq.1v2nf23.rcgu.o _data_cosmos-ef5244a14879f580/build_script_build-ef5244a14879f580.c2fzye5iqbx61t8txhcw5znvn.1v2nf23.rcgu.o _data_cosmos-ef5244a14879f580/build_script_build-ef5244a14879f580.d6uyvyxfwkk1tmp94qt99wief.1v2nf23.rcgu.o rqab2.rcgu.o rqab2.rcgu.o 64-REDACTED-linux-gnu/lib/libstd-46d936097e8c5b85.rlib 64-REDACTED-linux-gnu/lib/libpanic_unwind-e462f106b2b26a06.rlib 64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib 64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib 64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib 64-u�� 64-REDACTED-linux-gnu/lib/libcfg_if-a5addfdc94c3bad3.rlib 64-REDACTED-linux-gnu/lib/librustc_demangle-789fb9c0cb1c7158.rlib 4et.rcgu.o 4et.rcgu.o 4et.rcgu.o 4et.rcgu.o 4et.rcgu.o(dns block)If you need me to access, download, or install something from one of these locations, you can either: