Skip to content

Commit dcbf722

Browse files
Fix upsert create item regression (#4280)
## 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
1 parent 16d8d08 commit dcbf722

24 files changed

Lines changed: 345 additions & 118 deletions

File tree

sdk/cosmos/.cspell.json

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"autoscale",
1414
"azurecosmos",
1515
"backoff",
16+
"backpressure",
1617
"bytearray",
1718
"canadacentral",
1819
"canadaeast",
@@ -26,9 +27,11 @@
2627
"cloneable",
2728
"colls",
2829
"cosmosclient",
30+
"cutover",
2931
"Daad",
3032
"dedicatedgateway",
3133
"derefs",
34+
"dhat",
3235
"Dmaster",
3336
"documentdb",
3437
"dotproduct",
@@ -40,10 +43,12 @@
4043
"enablepreview",
4144
"EUAP",
4245
"euclidian",
46+
"evals",
4347
"fabianm",
4448
"failback",
4549
"failovers",
4650
"FILETIME",
51+
"flamegraph",
4752
"fract",
4853
"francecentral",
4954
"francesouth",
@@ -54,6 +59,7 @@
5459
"germanywestcentral",
5560
"hostnames",
5661
"hotfixes",
62+
"idents",
5763
"IMDS",
5864
"inclusivity",
5965
"isquery",
@@ -62,7 +68,10 @@
6268
"keepalive",
6369
"koreacentral",
6470
"koreasouth",
71+
"Kusto",
6572
"linearizability",
73+
"LLZA",
74+
"memcheck",
6675
"MEMORYSTATUSEX",
6776
"moka",
6877
"multihash",
@@ -86,6 +95,11 @@
8695
"othercoll",
8796
"partitionkey",
8897
"partitionkeyrangeid",
98+
"perfcontainer",
99+
"perfdb",
100+
"perfresults",
101+
"PIDS",
102+
"Pigw",
89103
"Pkrange",
90104
"pkranges",
91105
"pksysdocs",
@@ -96,17 +110,20 @@
96110
"PPAF",
97111
"PPCB",
98112
"pushback",
113+
"pyroscope",
99114
"qname",
115+
"qself",
100116
"RAII",
101117
"readfeed",
102118
"replicaset",
103-
"reqs",
104119
"Replicaset",
120+
"reqs",
105121
"Retriable",
106122
"retryable",
107123
"rfind",
108124
"RNTBD",
109125
"roundtrips",
126+
"RUPM",
110127
"rwcache",
111128
"sess",
112129
"southafricanorth",
@@ -179,4 +196,4 @@
179196
]
180197
}
181198
]
182-
}
199+
}

sdk/cosmos/azure_data_cosmos/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
### Breaking Changes
1717

18+
- `ContainerClient::create_item()` and `ContainerClient::upsert_item()` now require an `item_id: &str` parameter (same pattern as `replace_item` and `read_item`). The item id is passed to the driver via `ItemReference` so the body never needs to be parsed to extract the document id.
1819
- Renamed `replace_throughput` to `begin_replace_throughput` on `ContainerClient` and `DatabaseClient`. The return type changed from `ResourceResponse<ThroughputProperties>` to `ThroughputPoller`. ([#4096](https://github.com/Azure/azure-sdk-for-rust/pull/4096))
1920
- Removed `CreateDatabaseOptions::with_throughput()`. Database-level shared throughput provisioning is no longer supported through the SDK. Use container-level throughput instead. ([#4147](https://github.com/Azure/azure-sdk-for-rust/pull/4147))
2021

sdk/cosmos/azure_data_cosmos/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ async fn example(cosmos_client: CosmosClient) -> Result<(), Box<dyn std::error::
108108
let container = cosmos_client.database_client("myDatabase").container_client("myContainer").await?;
109109

110110
// Create an item
111-
container.create_item("partition1", item, None).await?;
111+
container.create_item("partition1", "1", item, None).await?;
112112

113113
// Read an item
114114
let item_response = container.read_item("partition1", "1", None).await?;

sdk/cosmos/azure_data_cosmos/examples/cosmos/create.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ pub enum Subcommands {
3333
#[arg(long, short)]
3434
partition_key: String,
3535

36+
/// The id of the new item.
37+
#[arg(long, short)]
38+
item_id: String,
39+
3640
/// The JSON of the new item.
3741
#[arg(long, short)]
3842
json: String,
@@ -77,6 +81,7 @@ impl CreateCommand {
7781
database,
7882
container,
7983
partition_key,
84+
item_id,
8085
json,
8186
show_updated,
8287
} => {
@@ -94,7 +99,9 @@ impl CreateCommand {
9499
None
95100
};
96101

97-
let response = container_client.create_item(pk, item, options).await?;
102+
let response = container_client
103+
.create_item(pk, &item_id, item, options)
104+
.await?;
98105

99106
println!("Created item successfully");
100107

sdk/cosmos/azure_data_cosmos/examples/cosmos/upsert.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ pub struct UpsertCommand {
2121
#[arg(long, short)]
2222
partition_key: String,
2323

24+
/// The id of the new item.
25+
#[arg(long, short)]
26+
item_id: String,
27+
2428
/// The JSON of the new item.
2529
#[arg(long, short)]
2630
json: String,
@@ -46,7 +50,9 @@ impl UpsertCommand {
4650
None
4751
};
4852

49-
let response = container_client.upsert_item(pk, item, options).await?;
53+
let response = container_client
54+
.upsert_item(pk, &self.item_id, item, options)
55+
.await?;
5056
println!("Item updated successfully");
5157

5258
if self.show_updated {

sdk/cosmos/azure_data_cosmos/src/clients/container_client.rs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ impl ContainerClient {
254254
///
255255
/// # Arguments
256256
/// * `partition_key` - The partition key of the new item.
257+
/// * `item_id` - The id of the new item.
257258
/// * `item` - The item to create. The type must implement [`Serialize`] and [`Deserialize`](serde::Deserialize)
258259
/// * `options` - Optional parameters for the request
259260
///
@@ -276,7 +277,7 @@ impl ContainerClient {
276277
/// };
277278
/// # let container_client: azure_data_cosmos::clients::ContainerClient = panic!("this is a non-running example");
278279
/// container_client
279-
/// .create_item("category1", p, None)
280+
/// .create_item("category1", "product1", p, None)
280281
/// .await?;
281282
/// # }
282283
/// ```
@@ -308,7 +309,7 @@ impl ContainerClient {
308309
/// operation.content_response_on_write = Some(ContentResponseOnWrite::Enabled);
309310
/// let options = ItemWriteOptions::default().with_operation_options(operation);
310311
/// let created_item = container_client
311-
/// .create_item("category1", p, Some(options))
312+
/// .create_item("category1", "product1", p, Some(options))
312313
/// .await?
313314
/// .into_body().json::<Product>();
314315
/// # Ok(())
@@ -317,16 +318,22 @@ impl ContainerClient {
317318
pub async fn create_item<T: Serialize>(
318319
&self,
319320
partition_key: impl Into<PartitionKey>,
321+
item_id: &str,
320322
item: T,
321323
options: Option<ItemWriteOptions>,
322324
) -> azure_core::Result<ItemResponse<()>> {
323325
let options = options.unwrap_or_default();
324326
let body = serde_json::to_vec(&item)?;
325-
let driver_pk = partition_key.into().into_driver_partition_key();
327+
328+
// Build the driver's item reference from our stored container metadata.
329+
let item_ref = ItemReference::from_name(
330+
&self.container_ref,
331+
partition_key.into().into_driver_partition_key(),
332+
item_id.to_owned(),
333+
);
326334

327335
// Create the driver operation and apply ItemWriteOptions fields.
328-
let operation =
329-
CosmosOperation::create_item(self.container_ref.clone(), driver_pk).with_body(body);
336+
let operation = CosmosOperation::create_item(item_ref).with_body(body);
330337
let operation = apply_item_options(operation, options.session_token, options.precondition);
331338

332339
// Execute through the driver.
@@ -447,6 +454,7 @@ impl ContainerClient {
447454
///
448455
/// # Arguments
449456
/// * `partition_key` - The partition key of the item to create or replace.
457+
/// * `item_id` - The id of the item to create or replace.
450458
/// * `item` - The item to create. The type must implement [`Serialize`] and [`Deserialize`](serde::Deserialize)
451459
/// * `options` - Optional parameters for the request
452460
///
@@ -469,7 +477,7 @@ impl ContainerClient {
469477
/// };
470478
/// # let container_client: azure_data_cosmos::clients::ContainerClient = panic!("this is a non-running example");
471479
/// container_client
472-
/// .upsert_item("category1", p, None)
480+
/// .upsert_item("category1", "product1", p, None)
473481
/// .await?;
474482
/// # Ok(())
475483
/// # }
@@ -502,24 +510,30 @@ impl ContainerClient {
502510
/// operation.content_response_on_write = Some(ContentResponseOnWrite::Enabled);
503511
/// let options = ItemWriteOptions::default().with_operation_options(operation);
504512
/// let updated_product = container_client
505-
/// .upsert_item("category1", p, Some(options))
513+
/// .upsert_item("category1", "product1", p, Some(options))
506514
/// .await?
507515
/// .into_body().json::<Product>()?;
508516
/// Ok(())
509517
/// # }
510518
pub async fn upsert_item<T: Serialize>(
511519
&self,
512520
partition_key: impl Into<PartitionKey>,
521+
item_id: &str,
513522
item: T,
514523
options: Option<ItemWriteOptions>,
515524
) -> azure_core::Result<ItemResponse<()>> {
516525
let options = options.unwrap_or_default();
517526
let body = serde_json::to_vec(&item)?;
518-
let driver_pk = partition_key.into().into_driver_partition_key();
527+
528+
// Build the driver's item reference from our stored container metadata.
529+
let item_ref = ItemReference::from_name(
530+
&self.container_ref,
531+
partition_key.into().into_driver_partition_key(),
532+
item_id.to_owned(),
533+
);
519534

520535
// Create the driver operation and apply ItemWriteOptions fields.
521-
let operation =
522-
CosmosOperation::upsert_item(self.container_ref.clone(), driver_pk).with_body(body);
536+
let operation = CosmosOperation::upsert_item(item_ref).with_body(body);
523537
let operation = apply_item_options(operation, options.session_token, options.precondition);
524538

525539
// Execute through the driver.
@@ -954,9 +968,9 @@ mod tests {
954968
assert_send(client.read_throughput(todo!()));
955969
assert_send(client.begin_replace_throughput(todo!(), todo!()));
956970
assert_send(client.delete(todo!()));
957-
assert_send(client.create_item::<serde_json::Value>("", todo!(), todo!()));
971+
assert_send(client.create_item::<serde_json::Value>("", todo!(), todo!(), todo!()));
958972
assert_send(client.replace_item::<serde_json::Value>("", todo!(), todo!(), todo!()));
959-
assert_send(client.upsert_item::<serde_json::Value>("", todo!(), todo!()));
973+
assert_send(client.upsert_item::<serde_json::Value>("", todo!(), todo!(), todo!()));
960974
assert_send(client.read_item::<serde_json::Value>("", todo!(), todo!()));
961975
assert_send(client.delete_item("", todo!(), todo!()));
962976
assert_send(client.execute_transactional_batch(todo!(), todo!()));

sdk/cosmos/azure_data_cosmos/tests/emulator_tests/cosmos_batch.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ pub async fn batch_mixed_operations() -> Result<(), Box<dyn Error>> {
132132
};
133133

134134
container_client
135-
.create_item(&partition_key, &item1, None)
135+
.create_item(&partition_key, &item1.id, &item1, None)
136136
.await?;
137137
container_client
138-
.create_item(&partition_key, &item2, None)
138+
.create_item(&partition_key, &item2.id, &item2, None)
139139
.await?;
140140

141141
// Now execute a batch with mixed operations
@@ -206,7 +206,7 @@ pub async fn batch_atomicity_on_failure() -> Result<(), Box<dyn Error>> {
206206
};
207207

208208
container_client
209-
.create_item(&partition_key, &item1, None)
209+
.create_item(&partition_key, &item1.id, &item1, None)
210210
.await?;
211211

212212
// Try to create a batch that will fail (trying to delete a non-existent item)

0 commit comments

Comments
 (0)