Skip to content

Replace Model trait with Format trait (applied to Response instead) #2559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions sdk/core/azure_core/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ pub use options::*;
pub use pager::*;
pub use pipeline::*;
pub use request::{Body, Request, RequestContent};
pub use response::{Model, Response};
pub use response::Response;

pub use typespec_client_core::http::response;
pub use typespec_client_core::http::{
new_http_client, AppendToUrlQuery, Context, HttpClient, Method, StatusCode, Url,
new_http_client, AppendToUrlQuery, Context, Format, HttpClient, JsonFormat, Method, StatusCode,
Url,
};

#[cfg(feature = "xml")]
pub use typespec_client_core::http::XmlFormat;
7 changes: 2 additions & 5 deletions sdk/core/azure_core/src/http/pager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ enum State<T> {
mod tests {
use std::collections::HashMap;

use crate::http::Model;
use futures::StreamExt;
use serde::Deserialize;

Expand All @@ -161,8 +160,7 @@ mod tests {

#[tokio::test]
pub async fn standard_pagination() {
#[derive(Model, Deserialize, Debug, PartialEq, Eq)]
#[typespec(crate = "crate")]
#[derive(Deserialize, Debug, PartialEq, Eq)]
struct Page {
pub page: usize,
}
Expand Down Expand Up @@ -233,8 +231,7 @@ mod tests {

#[tokio::test]
pub async fn error_stops_pagination() {
#[derive(Model, Deserialize, Debug, PartialEq, Eq)]
#[typespec(crate = "crate")]
#[derive(Deserialize, Debug, PartialEq, Eq)]
struct Page {
pub page: usize,
}
Expand Down
13 changes: 8 additions & 5 deletions sdk/core/azure_core/src/http/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
ops::Deref,
sync::Arc,
};
use typespec_client_core::http::{self, policies::Policy};
use typespec_client_core::http::{self, policies::Policy, Format, JsonFormat};

/// Execution pipeline.
///
Expand All @@ -34,14 +34,17 @@ use typespec_client_core::http::{self, policies::Policy};
/// cannot be enforced by code). All policies except Transport policy can assume there is another following policy (so
/// `self.pipeline[0]` is always valid).
#[derive(Debug, Clone)]
pub struct Pipeline(http::Pipeline);
pub struct Pipeline<F: Format = JsonFormat>(http::Pipeline<F>);

impl Pipeline {
impl<F: Format> Pipeline<F> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Was this a necessary viral change? Though, it's congruent with our discussion offline about requiring separate clients for JSON, XML, or whatever future format we might support. @JeffreyRichter, thoughts? I can catch you up offline. Basically, the whole point of this PR is to simplify callers because the client knows what the format is, so there's no reason to pass that responsibility off to callers as we have been.

I guess what I wasn't expecting is that we couldn't do this only in client methods' impl, but is that even a practical concern? Would we have any scenarios where - using the same client instance - we want to send e.g., both JSON and XML? Seems incredibly unlikely, though not impossible. (Unbranding might be more of a concern.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring specifically to the trait bound on F? The type parameter is certainly viral though, as only the client itself knows what format to use for a Pipeline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss offline

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put something on our calendars, but we can chat about it as well if people would rather.

/// Creates a new pipeline given the client library crate name and version,
/// alone with user-specified and client library-specified policies.
///
/// Crates can simply pass `option_env!("CARGO_PKG_NAME")` and `option_env!("CARGO_PKG_VERSION")` for the
/// `crate_name` and `crate_version` arguments respectively.
///
/// In addition, this constructor allows for specifying the response format (e.g. JSON, XML) to be used
/// when deserializing the response body.
pub fn new(
crate_name: Option<&'static str>,
crate_version: Option<&'static str>,
Expand Down Expand Up @@ -75,8 +78,8 @@ impl Pipeline {
}

// TODO: Should we instead use the newtype pattern?
impl Deref for Pipeline {
type Target = http::Pipeline;
impl<F: Format> Deref for Pipeline<F> {
type Target = http::Pipeline<F>;
fn deref(&self) -> &Self::Target {
&self.0
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/azure_core_test/src/proxy/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl Client {
.send::<PlaybackStartResult>(&ctx, &mut request)
.await?;
let recording_id = resp.headers().get_str(&RECORDING_ID)?.to_string();
let mut result: PlaybackStartResult = resp.into_json_body().await?;
let mut result: PlaybackStartResult = resp.into_body().await?;
result.recording_id = recording_id;
Ok(result)
}
Expand Down Expand Up @@ -212,7 +212,7 @@ impl Client {
self.pipeline
.send::<RemovedSanitizers>(&ctx, &mut request)
.await?
.into_json_body()
.into_body()
.await
}

Expand Down
15 changes: 8 additions & 7 deletions sdk/cosmos/azure_data_cosmos/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Cosmos DB SDK for Rust.
# Azure Cosmos DB SDK for Rust

This client library enables client applications to connect to Azure Cosmos DB via the NoSQL API. Azure Cosmos DB is a globally distributed, multi-model database service.

Expand All @@ -19,7 +19,7 @@ cargo add azure_data_cosmos
* An [Azure subscription] or free Azure Cosmos DB trial account.

Note: If you don't have an Azure subscription, create a free account before you begin.
You can Try Azure Cosmos DB for free without an Azure subscription, free of charge and commitments, or create an Azure Cosmos DB free tier account, with the first 400 RU/s and 5 GB of storage for free. You can also use the Azure Cosmos DB Emulator with a URI of https://localhost:8081. For the key to use with the emulator, see [how to develop with the emulator](https://learn.microsoft.com/azure/cosmos-db/how-to-develop-emulator).
You can Try Azure Cosmos DB for free without an Azure subscription, free of charge and commitments, or create an Azure Cosmos DB free tier account, with the first 400 RU/s and 5 GB of storage for free. You can also use the Azure Cosmos DB Emulator with a URI of <https://localhost:8081>. For the key to use with the emulator, see [how to develop with the emulator](https://learn.microsoft.com/azure/cosmos-db/how-to-develop-emulator).

### Create an Azure Cosmos DB account

Expand All @@ -36,6 +36,7 @@ In order to interact with the Azure Cosmos DB service you'll need to create an i
## Examples

The following section provides several code snippets covering some of the most common Azure Cosmos DB NoSQL API tasks, including:

* [Create Client](#create-cosmos-db-client "Create Cosmos DB client")
* [CRUD operation on Items](#crud-operation-on-items "CRUD operation on Items")

Expand Down Expand Up @@ -103,7 +104,7 @@ async fn example(cosmos_client: CosmosClient) -> Result<(), Box<dyn std::error::

// Read an item
let item_response = container.read_item("partition1", "1", None).await?;
let mut item: Item = item_response.into_json_body().await?;
let mut item: Item = item_response.into_body().await?;

item.value = "3".into();

Expand All @@ -125,10 +126,10 @@ async fn example(cosmos_client: CosmosClient) -> Result<(), Box<dyn std::error::

## Next steps

- [Resource Model of Azure Cosmos DB Service](https://learn.microsoft.com/azure/cosmos-db/sql-api-resources)
- [Azure Cosmos DB Resource URI](https://learn.microsoft.com/rest/api/documentdb/documentdb-resource-uri-syntax-for-rest)
- [Partitioning](https://learn.microsoft.com/azure/cosmos-db/partition-data)
- [Using emulator](https://github.com/Azure/azure-documentdb-dotnet/blob/master/docs/documentdb-nosql-local-emulator.md)
* [Resource Model of Azure Cosmos DB Service](https://learn.microsoft.com/azure/cosmos-db/sql-api-resources)
* [Azure Cosmos DB Resource URI](https://learn.microsoft.com/rest/api/documentdb/documentdb-resource-uri-syntax-for-rest)
* [Partitioning](https://learn.microsoft.com/azure/cosmos-db/partition-data)
* [Using emulator](https://github.com/Azure/azure-documentdb-dotnet/blob/master/docs/documentdb-nosql-local-emulator.md)

## Next steps

Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/examples/cosmos/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl CreateCommand {
println!("Created item successfully");

if show_updated {
let created = response.into_json_body::<serde_json::Value>().await?;
let created: serde_json::Value = response.into_raw_body().json().await?;
println!("Newly created item:");
println!("{:#?}", created);
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/examples/cosmos/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl PatchCommand {
match response {
Err(e) if e.http_status() == Some(StatusCode::NotFound) => println!("Item not found!"),
Ok(r) => {
let item: serde_json::Value = r.into_json_body().await?;
let item: serde_json::Value = r.into_raw_body().json().await?;
println!("Patched item:");
println!("{:#?}", item);
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/examples/cosmos/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl ReadCommand {
println!("Item not found!")
}
Ok(r) => {
let item: serde_json::Value = r.into_json_body().await?;
let item: serde_json::Value = r.into_body().await?;
println!("Found item:");
println!("{:#?}", item);
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/examples/cosmos/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl ReplaceCommand {
println!("Replaced item successfully");

if show_updated {
let created: serde_json::Value = r.into_json_body().await?;
let created: serde_json::Value = r.into_raw_body().json().await?;
println!("Newly replaced item:");
println!("{:#?}", created);
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/examples/cosmos/upsert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl UpsertCommand {
println!("Item updated successfully");

if self.show_updated {
let created: serde_json::Value = response.into_json_body().await?;
let created: serde_json::Value = response.into_raw_body().json().await?;
println!("Updated item:");
println!("{:#?}", created);
}
Expand Down
22 changes: 11 additions & 11 deletions sdk/cosmos/azure_data_cosmos/src/clients/container_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl ContainerClient {
///
/// By default, the newly created item is *not* returned in the HTTP response.
/// If you want the new item to be returned, set the [`ItemOptions::enable_content_response_on_write`] option to `true`.
/// You can deserialize the returned item using [`Response::into_json_body`], like this:
/// You can deserialize the returned item using [`ResponseBody::json`](azure_core::http::response::ResponseBody::json), like this:
///
/// ```rust,no_run
/// use azure_data_cosmos::ItemOptions;
Expand All @@ -242,7 +242,7 @@ impl ContainerClient {
/// let created_item = container_client
/// .create_item("category1", p, Some(options))
/// .await?
/// .into_json_body::<Product>()
/// .into_raw_body().json::<Product>()
/// .await?;
/// # Ok(())
/// # }
Expand Down Expand Up @@ -306,7 +306,7 @@ impl ContainerClient {
///
/// By default, the replaced item is *not* returned in the HTTP response.
/// If you want the replaced item to be returned, set the [`ItemOptions::enable_content_response_on_write`] option to `true`.
/// You can deserialize the returned item using [`Response::into_json_body`], like this:
/// You can deserialize the returned item using [`ResponseBody::json`](azure_core::http::response::ResponseBody::json), like this:
///
/// ```rust,no_run
/// use azure_data_cosmos::ItemOptions;
Expand All @@ -332,7 +332,7 @@ impl ContainerClient {
/// let updated_product: Product = container_client
/// .replace_item("category1", "product1", p, Some(options))
/// .await?
/// .into_json_body()
/// .into_raw_body().json::<Product>()
/// .await?;
/// # }
/// ```
Expand Down Expand Up @@ -396,7 +396,7 @@ impl ContainerClient {
///
/// By default, the created/replaced item is *not* returned in the HTTP response.
/// If you want the created/replaced item to be returned, set the [`ItemOptions::enable_content_response_on_write`] option to `true`.
/// You can deserialize the returned item using [`Response::into_json_body`], like this:
/// You can deserialize the returned item using [`ResponseBody::json`](azure_core::http::response::ResponseBody::json), like this:
///
/// ```rust,no_run
/// use azure_data_cosmos::ItemOptions;
Expand All @@ -422,7 +422,7 @@ impl ContainerClient {
/// let updated_product = container_client
/// .upsert_item("category1", p, Some(options))
/// .await?
/// .into_json_body::<Product>()
/// .into_raw_body().json::<Product>()
/// .await?;
/// Ok(())
/// # }
Expand Down Expand Up @@ -477,18 +477,18 @@ impl ContainerClient {
/// let item: Product = container_client
/// .read_item("partition1", "item1", None)
/// .await?
/// .into_json_body()
/// .into_body()
/// .await?;
/// println!("Read Item: {:#?}", item);
/// # Ok(())
/// # }
/// ```
pub async fn read_item(
pub async fn read_item<T>(
&self,
partition_key: impl Into<PartitionKey>,
item_id: &str,
options: Option<ItemOptions<'_>>,
) -> azure_core::Result<Response> {
) -> azure_core::Result<Response<T>> {
let options = options.unwrap_or_default();
let link = self.items_link.item(item_id);
let url = self.pipeline.url(&link);
Expand Down Expand Up @@ -562,7 +562,7 @@ impl ContainerClient {
///
/// By default, the patched item is *not* returned in the HTTP response.
/// If you want the patched item to be returned, set the [`ItemOptions::enable_content_response_on_write`] option to `true`.
/// You can deserialize the returned item using [`Response::into_json_body`], like this:
/// You can deserialize the returned item using [`ResponseBody::json`](azure_core::http::response::ResponseBody::json), like this:
///
/// For example:
///
Expand All @@ -586,7 +586,7 @@ impl ContainerClient {
/// let patched_item = client
/// .patch_item("partition1", "item1", patch, Some(options))
/// .await?
/// .into_json_body::<Product>()
/// .into_raw_body().json::<Product>()
/// .await?;
/// # Ok(())
/// # }
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure_data_cosmos/src/feed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<T: DeserializeOwned> FeedPage<T> {
pub(crate) async fn from_response(response: Response) -> azure_core::Result<Self> {
let headers = response.headers().clone();
let continuation = headers.get_optional_string(&constants::CONTINUATION);
let body: FeedBody<T> = response.into_json_body::<FeedBody<T>>().await?;
let body: FeedBody<T> = response.into_raw_body().json().await?;

Ok(Self {
items: body.items,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use std::{borrow::Cow, time::Duration};

use azure_core::http::response::Model;
use serde::{Deserialize, Deserializer, Serialize, Serializer};

use crate::models::{IndexingPolicy, PartitionKeyDefinition, SystemProperties};
Expand Down Expand Up @@ -48,7 +47,7 @@ where
/// Also, note that the `id` and `partition_key` values are **required** by the server. You will get an error from the server if you omit them.
///
/// [Struct Update]: https://doc.rust-lang.org/stable/book/ch05-01-defining-structs.html?highlight=Struct#creating-instances-from-other-instances-with-struct-update-syntax
#[derive(Model, Clone, Default, Debug, Deserialize, Serialize, PartialEq, Eq)]
#[derive(Clone, Default, Debug, Deserialize, Serialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
pub struct ContainerProperties {
/// The ID of the container.
Expand Down
22 changes: 5 additions & 17 deletions sdk/cosmos/azure_data_cosmos/src/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,8 @@

//! Model types sent to and received from the Azure Cosmos DB API.

use azure_core::{
date::OffsetDateTime,
http::{
response::{Model, ResponseBody},
Etag,
},
};
use serde::{de::DeserializeOwned, Deserialize, Deserializer, Serialize};
use azure_core::{date::OffsetDateTime, http::Etag};
use serde::{Deserialize, Deserializer, Serialize};

mod container_properties;
mod indexing_policy;
Expand Down Expand Up @@ -51,23 +45,17 @@ pub struct QueryResults<T> {
pub items: Vec<T>,
}

impl<T: DeserializeOwned> Model for QueryResults<T> {
async fn from_response_body(body: ResponseBody) -> typespec_client_core::Result<Self> {
body.json().await
}
}

/// A page of results from [`CosmosClient::query_databases`](crate::CosmosClient::query_databases())
#[non_exhaustive]
#[derive(Clone, Default, Debug, Deserialize, Model)]
#[derive(Clone, Default, Debug, Deserialize)]
pub struct DatabaseQueryResults {
#[serde(alias = "Databases")]
pub databases: Vec<DatabaseProperties>,
}

/// A page of results from [`DatabaseClient::query_containers`](crate::clients::DatabaseClient::query_containers())
#[non_exhaustive]
#[derive(Clone, Default, Debug, Deserialize, Model)]
#[derive(Clone, Default, Debug, Deserialize)]
pub struct ContainerQueryResults {
#[serde(alias = "DocumentCollections")]
pub containers: Vec<ContainerProperties>,
Expand Down Expand Up @@ -108,7 +96,7 @@ pub struct SystemProperties {
///
/// Returned by [`DatabaseClient::read()`](crate::clients::DatabaseClient::read()).
#[non_exhaustive]
#[derive(Model, Clone, Default, Debug, Deserialize, PartialEq, Eq)]
#[derive(Clone, Default, Debug, Deserialize, PartialEq, Eq)]
pub struct DatabaseProperties {
/// The ID of the database.
pub id: String,
Expand Down
4 changes: 2 additions & 2 deletions sdk/cosmos/azure_data_cosmos/src/models/patch_operations.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::borrow::Cow;

use azure_core::{error::ErrorKind, http::response::Model, Error};
use azure_core::{error::ErrorKind, Error};
use serde::{Deserialize, Serialize};

// Cosmos' patch operations are _similar_ to JSON Patch (RFC 6902) in structure, but have different operations.
Expand Down Expand Up @@ -37,7 +37,7 @@ use serde::{Deserialize, Serialize};
/// # Ok(())
/// # }
/// ```
#[derive(Model, Default, Debug, Serialize, Deserialize, PartialEq, Eq)]
#[derive(Default, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct PatchDocument {
pub operations: Vec<PatchOperation>,
}
Expand Down
Loading
Loading