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

* start building Format/DeserializeWith trait

* remove Model and replace with Format type parameter

* fix issues in azure_core, azure_identity, and azure_data_cosmos

* fix tests in hand-written crates

* updates to generated crates

* make send_format more clearly a short-cut method

* fix async_trait on wasm32

* fix tyop

* initial pr feedback

* rename DefaultFormat to JsonFormat

* paramerize pipeline on format

* undo all generated code changes

* revert keyvault generated changes

* some updates to copilot instructions

* split Response into Response and RawResponse

* remove extraneous helpers, let's focus on simple APIs and be ok with boilerplate

* api doc updates

* remove some extraneous copilot instructions

* refactor cosmos for new Response types

* fix typespec client examples

* fix issues in non-generated clients

* fix issues in generated clients

* a few other hand-written client fixes

* update changelog

* pr feedback

* refmt

* fix doctest

* fix more doctests

* add `F` parameter to `Pager`

Author: analogrelay

.github/copilot-instructions.md

@@ -16,6 +16,9 @@ Use these instructions for test generation as well.
     -   Field and function names are snake_case.
     -   Parameter names are snake_case.
     -   Crate and module names are snake_case.
+-   Keep `use` directives at the top of the module in which they are used, and avoid placing them inside functions or blocks unless absolutely necessary.
+-   Prefer using `crate` in `use` directives to refer to types anywhere in the current crate instead of using it's name, or relative paths like `super` or `self`.
+-   Prefer merging new `use` directives into existing ones rather than creating new `use` blocks.
 -   Prioritize safety, efficiency, and correctness.
 -   Respect Rust's ownership and borrowing rules.
 -   Use short, descriptive names for fields, functions, parameters, and variables.
@@ -29,6 +32,7 @@ Use these instructions for test generation as well.
 -   Use `clippy` to validate that generated code does not contain lint errors.
 -   If you have trouble generating safe, efficient, maintainable, and lint-free code, insert a `TODO` comment describing what should happen.
 -   All imported types, constants, functions, modules, and macros should be imported explicitly. Never import `*`.
+-   Do not modify generated code, found in `generated` subdirectories. These files are generated by external tools and should not be edited manually.
 
 ## Test Generation
 
@@ -38,3 +42,4 @@ Use these instructions for test generation as well.
 -   The `tests` module should be conditioned on `#[cfg(test)]`.
 -   The `tests` module should always import APIs from `super`.
 -   Do not begin test function names with "test" unless necessary to disambiguate from the function being tested.
+-   Test functions do not need to be public.

sdk/core/azure_core/benches/benchmarks.rs

@@ -1,5 +1,5 @@
 use azure_core::http::{
-    headers::Headers, response::Response, HttpClient, Method, Request, StatusCode, Url,
+    headers::Headers, HttpClient, Method, RawResponse, Request, StatusCode, Url,
 };
 use azure_core_test::http::MockHttpClient;
 use criterion::{black_box, criterion_group, criterion_main, Criterion};
@@ -30,7 +30,14 @@ fn http_transport_test(c: &mut Criterion) {
 
     // client to be used in the benchmark
     let mock_client = Arc::new(MockHttpClient::new(move |_| {
-        async move { Ok(Response::from_bytes(StatusCode::Ok, Headers::new(), vec![])) }.boxed()
+        async move {
+            Ok(RawResponse::from_bytes(
+                StatusCode::Ok,
+                Headers::new(),
+                vec![],
+            ))
+        }
+        .boxed()
     })) as Arc<dyn HttpClient>;
 
     // requests to be used in the benchmark

sdk/core/azure_core/src/http/mod.rs

@@ -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::{RawResponse, 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;

sdk/core/azure_core/src/http/pager.rs

@@ -5,6 +5,7 @@ use crate::http::{headers::HeaderName, response::Response};
 use futures::{stream::unfold, Stream};
 use std::{future::Future, pin::Pin};
 use typespec::Error;
+use typespec_client_core::http::JsonFormat;
 
 /// The result of fetching a single page from a [`Pager`], whether the `Pager` should continue or is complete.
 #[derive(Debug)]
@@ -15,12 +16,12 @@ pub enum PagerResult<T, C> {
     Complete { response: T },
 }
 
-impl<T> PagerResult<Response<T>, String> {
+impl<T, F> PagerResult<Response<T, F>, String> {
     /// Creates a [`PagerResult<T, C>`] from the provided response, extracting the continuation value from the provided header.
     ///
     /// If the provided response has a header with the matching name, this returns [`PagerResult::Continue`], using the value from the header as the continuation.
     /// If the provided response does not have a header with the matching name, this returns [`PagerResult::Complete`].
-    pub fn from_response_header(response: Response<T>, header_name: &HeaderName) -> Self {
+    pub fn from_response_header(response: Response<T, F>, header_name: &HeaderName) -> Self {
         match response.headers().get_optional_string(header_name) {
             Some(continuation) => PagerResult::Continue {
                 response,
@@ -34,7 +35,7 @@ impl<T> PagerResult<Response<T>, String> {
 /// Represents a paginated stream of results generated through HTTP requests to a service.
 ///
 /// Specifically, this is a [`PageStream`] that yields [`Response<T>`] values.
-pub type Pager<T> = PageStream<Response<T>>;
+pub type Pager<T, F = JsonFormat> = PageStream<Response<T, F>>;
 
 /// Represents a paginated stream of results from a service.
 #[pin_project::pin_project]
@@ -78,7 +79,8 @@ impl<T> PageStream<T> {
     ///         }
     ///         let resp: Response<MyModel> = pipeline
     ///           .send(&Context::new(), &mut req)
-    ///           .await?;
+    ///           .await?
+    ///           .into();
     ///         Ok(PagerResult::from_response_header(resp, &HeaderName::from_static("x-next-continuation")))
     ///     }
     /// });
@@ -162,59 +164,60 @@ enum State<T> {
 mod tests {
     use std::collections::HashMap;
 
-    use crate::http::Model;
     use futures::StreamExt;
     use serde::Deserialize;
 
     use crate::http::{
         headers::{HeaderName, HeaderValue},
-        Pager, PagerResult, Response, StatusCode,
+        Pager, PagerResult, RawResponse, StatusCode,
     };
 
     #[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,
         }
 
         let pager: Pager<Page> = Pager::from_callback(|continuation| async move {
             match continuation {
                 None => Ok(PagerResult::Continue {
-                    response: Response::from_bytes(
+                    response: RawResponse::from_bytes(
                         StatusCode::Ok,
                         HashMap::from([(
                             HeaderName::from_static("x-test-header"),
                             HeaderValue::from_static("page-1"),
                         )])
                         .into(),
                         r#"{"page":1}"#,
-                    ),
+                    )
+                    .into(),
                     continuation: "1",
                 }),
                 Some("1") => Ok(PagerResult::Continue {
-                    response: Response::from_bytes(
+                    response: RawResponse::from_bytes(
                         StatusCode::Ok,
                         HashMap::from([(
                             HeaderName::from_static("x-test-header"),
                             HeaderValue::from_static("page-2"),
                         )])
                         .into(),
                         r#"{"page":2}"#,
-                    ),
+                    )
+                    .into(),
                     continuation: "2",
                 }),
                 Some("2") => Ok(PagerResult::Complete {
-                    response: Response::from_bytes(
+                    response: RawResponse::from_bytes(
                         StatusCode::Ok,
                         HashMap::from([(
                             HeaderName::from_static("x-test-header"),
                             HeaderValue::from_static("page-3"),
                         )])
                         .into(),
                         r#"{"page":3}"#,
-                    ),
+                    )
+                    .into(),
                 }),
                 _ => {
                     panic!("Unexpected continuation value")
@@ -245,24 +248,24 @@ 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,
         }
 
         let pager: Pager<Page> = Pager::from_callback(|continuation| async move {
             match continuation {
                 None => Ok(PagerResult::Continue {
-                    response: Response::from_bytes(
+                    response: RawResponse::from_bytes(
                         StatusCode::Ok,
                         HashMap::from([(
                             HeaderName::from_static("x-test-header"),
                             HeaderValue::from_static("page-1"),
                         )])
                         .into(),
                         r#"{"page":1}"#,
-                    ),
+                    )
+                    .into(),
                     continuation: "1",
                 }),
                 Some("1") => Err(typespec::Error::message(

sdk/core/azure_core/src/http/pipeline.rs

@@ -90,14 +90,14 @@ mod tests {
             headers::{self, HeaderName, Headers},
             policies::Policy,
             request::options::ClientRequestId,
-            Context, Method, Request, Response, StatusCode, TransportOptions,
+            Context, Method, Request, StatusCode, TransportOptions,
         },
         Bytes,
     };
     use azure_core_test::http::MockHttpClient;
     use futures::FutureExt as _;
     use std::sync::Arc;
-    use typespec_client_core::http::ClientOptions;
+    use typespec_client_core::http::{ClientOptions, RawResponse};
 
     #[tokio::test]
     async fn pipeline_with_custom_client_request_id_policy() {
@@ -121,7 +121,7 @@ mod tests {
                     "Custom header value should match the client request ID"
                 );
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     Headers::new(),
                     Bytes::new(),
@@ -153,7 +153,7 @@ mod tests {
 
         // Act
         pipeline
-            .send::<()>(&ctx, &mut request)
+            .send(&ctx, &mut request)
             .await
             .expect("Pipeline execution failed");
     }
@@ -178,7 +178,7 @@ mod tests {
                     "Default header value should match the client request ID"
                 );
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     Headers::new(),
                     Bytes::new(),
@@ -206,7 +206,7 @@ mod tests {
 
         // Act
         pipeline
-            .send::<()>(&ctx, &mut request)
+            .send(&ctx, &mut request)
             .await
             .expect("Pipeline execution failed");
     }

sdk/core/azure_core/src/http/policies/bearer_token_policy.rs

@@ -125,7 +125,7 @@ mod tests {
         http::{
             headers::{Headers, AUTHORIZATION},
             policies::Policy,
-            Request, Response, StatusCode,
+            Request, StatusCode,
         },
         Bytes, Result,
     };
@@ -138,7 +138,9 @@ mod tests {
     };
     use std::time::Duration;
     use time::OffsetDateTime;
-    use typespec_client_core::http::{policies::TransportPolicy, Method, TransportOptions};
+    use typespec_client_core::http::{
+        policies::TransportPolicy, Method, RawResponse, TransportOptions,
+    };
 
     #[derive(Debug, Clone)]
     struct MockCredential {
@@ -217,7 +219,7 @@ mod tests {
 
                 assert_eq!(format!("Bearer {}", expected.token.secret()), authz);
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     Headers::new(),
                     Bytes::new(),

sdk/core/azure_core/src/http/policies/client_request_id.rs

@@ -78,7 +78,7 @@ mod tests {
     use azure_core_test::http::MockHttpClient;
     use futures::FutureExt;
     use std::sync::Arc;
-    use typespec_client_core::http::{policies::TransportPolicy, Response, TransportOptions};
+    use typespec_client_core::http::{policies::TransportPolicy, RawResponse, TransportOptions};
 
     #[tokio::test]
     async fn header_already_present() {
@@ -100,7 +100,7 @@ mod tests {
                     "Header value should not change"
                 );
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     headers::Headers::new(),
                     Bytes::new(),
@@ -133,7 +133,7 @@ mod tests {
                     .expect("Header should be present");
                 assert!(!header_value.is_empty(), "Header value should be generated");
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     headers::Headers::new(),
                     Bytes::new(),
@@ -174,7 +174,7 @@ mod tests {
                     "Custom header value should not change"
                 );
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     headers::Headers::new(),
                     Bytes::new(),
@@ -214,7 +214,7 @@ mod tests {
                     "Header value should match the client request ID from the context"
                 );
 
-                Ok(Response::from_bytes(
+                Ok(RawResponse::from_bytes(
                     StatusCode::Ok,
                     headers::Headers::new(),
                     Bytes::new(),

sdk/core/azure_core_test/src/http/clients.rs

@@ -3,19 +3,19 @@
 
 use async_trait::async_trait;
 use azure_core::{
-    http::{request::Request, response::Response, HttpClient},
+    http::{request::Request, HttpClient, RawResponse},
     Result,
 };
 use futures::{future::BoxFuture, lock::Mutex};
 use std::fmt;
 
-/// An [`HttpClient`] from which you can assert [`Request`]s and return mock [`Response`]s.
+/// An [`HttpClient`] from which you can assert [`Request`]s and return mock [`RawResponse`]s.
 ///
 /// # Examples
 ///
 /// ```
 /// use azure_core::{
-///     http::{headers::Headers, ClientOptions, Response, StatusCode, TransportOptions},
+///     http::{headers::Headers, ClientOptions, RawResponse, StatusCode, TransportOptions},
 ///     Bytes,
 /// };
 /// use azure_core_test::http::MockHttpClient;
@@ -28,7 +28,7 @@ use std::fmt;
 /// # async fn main() -> Result<(), Box<dyn std::error::Error>> {
 /// let mock_client = Arc::new(MockHttpClient::new(|req| async {
 ///     assert_eq!(req.url().host_str(), Some("my-vault.vault.azure.net"));
-///     Ok(Response::from_bytes(
+///     Ok(RawResponse::from_bytes(
 ///         StatusCode::Ok,
 ///         Headers::new(),
 ///         Bytes::from_static(br#"{"value":"secret"}"#),
@@ -54,7 +54,7 @@ pub struct MockHttpClient<C>(Mutex<C>);
 
 impl<C> MockHttpClient<C>
 where
-    C: FnMut(&Request) -> BoxFuture<'_, Result<Response>> + Send + Sync,
+    C: FnMut(&Request) -> BoxFuture<'_, Result<RawResponse>> + Send + Sync,
 {
     /// Creates a new `MockHttpClient` using a capture.
     ///
@@ -75,9 +75,9 @@ impl<C> fmt::Debug for MockHttpClient<C> {
 #[cfg_attr(not(target_arch = "wasm32"), async_trait)]
 impl<C> HttpClient for MockHttpClient<C>
 where
-    C: FnMut(&Request) -> BoxFuture<'_, Result<Response>> + Send + Sync,
+    C: FnMut(&Request) -> BoxFuture<'_, Result<RawResponse>> + Send + Sync,
 {
-    async fn execute_request(&self, req: &Request) -> Result<Response> {
+    async fn execute_request(&self, req: &Request) -> Result<RawResponse> {
         let mut client = self.0.lock().await;
         (client)(req).await
     }
@@ -109,7 +109,11 @@ mod tests {
                     *count += 1;
                 }
 
-                Ok(Response::from_bytes(StatusCode::Ok, Headers::new(), vec![]))
+                Ok(RawResponse::from_bytes(
+                    StatusCode::Ok,
+                    Headers::new(),
+                    vec![],
+                ))
             }
             .boxed()
         })) as Arc<dyn HttpClient>;