Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
analogrelay committed Sep 11, 2024
1 parent f356f15 commit d9b2683
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 62 deletions.
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
# ServiceOwner: @Pilchie
# ServiceLabel: %Cosmos
# PRLabel: %Cosmos
/sdk/cosmosdb/ @analogrelay @Pilchie
/sdk/cosmos/ @analogrelay @Pilchie

###########
# Eng Sys
Expand Down
15 changes: 8 additions & 7 deletions .vscode/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@
"noSuggest": true
},
{
"name": "cosmosdb",
"path": "../sdk/cosmosdb/cosmosdb.dict.txt",
"name": "cosmos",
"path": "../sdk/cosmos/.dict.txt",
"noSuggest": true
}
],
Expand Down Expand Up @@ -103,11 +103,12 @@
]
},
{
"filename": "sdk/cosmosdb/azure_data_cosmos/**",
"dictionaries": [
"crates",
"cosmosdb",
"rust-custom"
"filename": "sdk/cosmos/**",
"words": [
"colls",
"pkranges",
"sprocs",
"udfs",
]
}
]
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ members = [
"sdk/typespec/typespec_derive",
"sdk/core/azure_core",
"sdk/core/azure_core_amqp",
"sdk/cosmosdb/azure_data_cosmos",
"sdk/cosmos/azure_data_cosmos",
"sdk/identity/azure_identity",
"sdk/eventhubs/azure_messaging_eventhubs",
"eng/test/mock_transport",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ repository.workspace = true
rust-version.workspace = true
homepage = "https://github.com/azure/azure-sdk-for-rust"
documentation = "https://docs.rs/azure_data_cosmos"
keywords = ["sdk", "azure", "rest", "cloud", "cosmosdb", "database"]
keywords = ["sdk", "azure", "rest", "cloud", "cosmos", "database"]
categories = ["api-bindings"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
Expand All @@ -19,7 +19,6 @@ categories = ["api-bindings"]
async-trait.workspace = true
azure_core.workspace = true
typespec_client_core = { workspace = true, features = ["derive"] }
time.workspace = true
tracing.workspace = true
url.workspace = true
serde.workspace = true
Expand All @@ -37,7 +36,7 @@ workspace = true
default = ["hmac_rust"]
hmac_rust = ["azure_core/hmac_rust"]
hmac_openssl = ["azure_core/hmac_openssl"]
key-auth = [] # Enables support for key-based authentication (Primary Keys and Resource Tokens)
key_auth = [] # Enables support for key-based authentication (Primary Keys and Resource Tokens)

[package.metadata.docs.rs]
features = ["key-auth"]
features = ["key_auth"]
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
use azure_data_cosmos::{CosmosClient, CosmosClientMethods, DatabaseClientMethods};
use azure_data_cosmos::{clients::DatabaseClientMethods, CosmosClient, CosmosClientMethods};
use clap::Parser;

/// A simple example to show connecting to a Cosmos Account and retrieving the properties of a database.
#[derive(Parser)]
pub struct Args {
/// The CosmosDB endpoint to connect to.
/// The Cosmos endpoint to connect to.
endpoint: String,

/// The database to fetch information for.
database: String,

/// An authentication key to use when connecting to the Cosmos DB account. If omitted, the connection will use Entra ID.
#[clap(long)]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
key: Option<String>,
}

Expand All @@ -22,13 +22,13 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {

let client = create_client(&args);

let db_client = client.database(&args.database);
let db_client = client.database_client(&args.database);
let response = db_client.read(None).await?.deserialize_body().await?;
println!("{:?}", response);
Ok(())
}

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
fn create_client(args: &Args) -> CosmosClient {
if let Some(key) = args.key.as_ref() {
CosmosClient::with_key(&args.endpoint, key.clone(), None).unwrap()
Expand All @@ -38,7 +38,7 @@ fn create_client(args: &Args) -> CosmosClient {
}
}

#[cfg(not(feature = "key-auth"))]
#[cfg(not(feature = "key_auth"))]
fn create_client(args: &Args) -> CosmosClient {
let cred = azure_identity::create_default_credential().unwrap();
CosmosClient::new(&args.endpoint, cred, None).unwrap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
//! We implement that policy here, because we can't use any standard Azure SDK authentication policy.

use azure_core::auth::TokenCredential;
use azure_core::date::OffsetDateTime;
use azure_core::{
date,
headers::{HeaderValue, AUTHORIZATION, MS_DATE, VERSION},
Context, Policy, PolicyResult, Request, Url,
};
use std::sync::Arc;
use time::OffsetDateTime;
use tracing::trace;
use url::form_urlencoded;

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
use azure_core::{auth::Secret, hmac::hmac_sha256};

const AZURE_VERSION: &str = "2018-12-31";
Expand All @@ -42,11 +42,11 @@ enum Credential {
Token(Arc<dyn TokenCredential>),

/// The credential is a key to be used to sign the HTTP request (a shared key)
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
PrimaryKey(Secret),
}

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
struct SignatureTarget<'a> {
http_method: &'a azure_core::Method,
resource_type: &'a ResourceType,
Expand All @@ -66,7 +66,7 @@ impl AuthorizationPolicy {
}
}

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
pub(crate) fn from_shared_key(key: Secret) -> Self {
Self {
credential: Credential::PrimaryKey(key),
Expand Down Expand Up @@ -99,7 +99,7 @@ impl Policy for AuthorizationPolicy {
let auth = generate_authorization(
&self.credential,
request.url(),
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
SignatureTarget {
http_method: request.method(),
resource_type,
Expand Down Expand Up @@ -186,7 +186,7 @@ fn extract_resource_link(request: &Request) -> String {
async fn generate_authorization<'a>(
auth_token: &Credential,
url: &Url,
#[cfg(feature = "key-auth")] signature_target: SignatureTarget<'a>,
#[cfg(feature = "key_auth")] signature_target: SignatureTarget<'a>,
) -> azure_core::Result<String> {
let (authorization_type, signature) = match auth_token {
Credential::Token(token_credential) => (
Expand All @@ -199,7 +199,7 @@ async fn generate_authorization<'a>(
.to_string(),
),

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
Credential::PrimaryKey(key) => {
let string_to_sign = string_to_sign(signature_target);
("master", hmac_sha256(&string_to_sign, key)?)
Expand All @@ -222,7 +222,7 @@ fn scope_from_url(url: &Url) -> String {
/// This function generates a valid authorization string, according to the documentation.
/// In case of authorization problems we can compare the `string_to_sign` generated by Azure against
/// our own.
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
fn string_to_sign(signature_target: SignatureTarget) -> String {
// From official docs:
// StringToSign =
Expand Down Expand Up @@ -266,16 +266,16 @@ fn string_to_sign(signature_target: SignatureTarget) -> String {

#[cfg(test)]
mod tests {
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
use azure_core::auth::AccessToken;

use super::*;

#[derive(Debug)]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
struct TestTokenCredential(String);

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
impl TokenCredential for TestTokenCredential {
Expand All @@ -293,7 +293,7 @@ mod tests {
}

#[test]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
fn string_to_sign_generates_expected_string_for_signing() {
let time_nonce = date::parse_rfc3339("1900-01-01T01:00:00.000000000+00:00").unwrap();

Expand All @@ -315,7 +315,7 @@ mon, 01 jan 1900 01:00:00 gmt
}

#[tokio::test]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
async fn generate_authorization_for_token_credential() {
let time_nonce = date::parse_rfc3339("1900-01-01T01:00:00.000000000+00:00").unwrap();
let cred = Arc::new(TestTokenCredential("test_token".to_string()));
Expand Down Expand Up @@ -346,7 +346,7 @@ mon, 01 jan 1900 01:00:00 gmt
}

#[tokio::test]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
async fn generate_authorization_for_primary_key_0() {
let time_nonce = date::parse_rfc3339("1900-01-01T01:00:00.000000000+00:00").unwrap();

Expand Down Expand Up @@ -379,7 +379,7 @@ mon, 01 jan 1900 01:00:00 gmt
}

#[tokio::test]
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
async fn generate_authorization_for_primary_key_1() {
let time_nonce = date::parse_rfc3339("2017-04-27T00:51:12.000000000+00:00").unwrap();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::authorization_policy::AuthorizationPolicy;
use crate::{CosmosClientOptions, DatabaseClient};
use crate::clients::DatabaseClient;
use crate::CosmosClientOptions;
use azure_core::auth::TokenCredential;
use azure_core::{Pipeline, Url};
use std::sync::Arc;

#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
use azure_core::auth::Secret;

/// Client for Azure Cosmos DB.
Expand All @@ -23,7 +24,10 @@ pub struct CosmosClient {
/// Rather than depending on `CosmosClient`, you can depend on a generic parameter constrained by this trait, or an `impl CosmosClientMethods` type.
pub trait CosmosClientMethods {
/// Gets a [`DatabaseClient`] that can be used to access the database with the specified ID.
fn database(&self, name: impl Into<String>) -> DatabaseClient;
///
/// # Arguments
/// * `id` - The ID of the database.
fn database_client(&self, id: impl AsRef<str>) -> DatabaseClient;
}

impl CosmosClient {
Expand Down Expand Up @@ -72,7 +76,7 @@ impl CosmosClient {
/// ```rust,no_run
/// let client = CosmosClient::with_shared_key("https://myaccount.documents.azure.com/", "my_key", None)?;
/// ```
#[cfg(feature = "key-auth")]
#[cfg(feature = "key_auth")]
pub fn with_key(
endpoint: impl AsRef<str>,
key: impl Into<Secret>,
Expand All @@ -97,8 +101,11 @@ impl CosmosClient {

impl CosmosClientMethods for CosmosClient {
/// Gets a [`DatabaseClient`] that can be used to access the database with the specified ID.
fn database(&self, id: impl Into<String>) -> DatabaseClient {
DatabaseClient::new(self.clone(), id.into())
///
/// # Arguments
/// * `id` - The ID of the database.
fn database_client(&self, id: impl AsRef<str>) -> DatabaseClient {
DatabaseClient::new(self.clone(), id.as_ref())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,41 +22,42 @@ pub trait DatabaseClientMethods {
///
/// ```rust,no_run
/// # async fn doc() {
/// use azure_data_cosmos::{CosmosClient, CosmosClientMethods, DatabaseClientMethods};
/// use azure_data_cosmos::{CosmosClient, CosmosClientMethods, clients::DatabaseClientMethods};
///
/// let credential = azure_identity::create_default_credential().unwrap();
/// let client = CosmosClient::new("https://myaccount.documents.azure.com/", credential, None).unwrap();
/// let db_client = client.database("my_database");
/// let db_client = client.database_client("my_database");
/// let response = db_client.read(None)
/// .await.unwrap()
/// .deserialize_body()
/// .await.unwrap();
/// # }
/// ```
fn read(
#[allow(async_fn_in_trait)] // REASON: See https://github.com/Azure/azure-sdk-for-rust/issues/1796 for detailed justification
async fn read(
&self,
options: Option<ReadDatabaseOptions>,
) -> impl std::future::Future<Output = azure_core::Result<azure_core::Response<DatabaseProperties>>>;
) -> azure_core::Result<azure_core::Response<DatabaseProperties>>;
}

/// A client for working with a specific database in a Cosmos account.
///
/// You can get a `DatabaseClient` by calling [`CosmosClient::database()`](CosmosClient::database).
/// You can get a `DatabaseClient` by calling [`CosmosClient::database_client()`](CosmosClient::database_client()).
pub struct DatabaseClient {
base_url: Url,
root_client: CosmosClient,
}

impl DatabaseClient {
pub(crate) fn new(root_client: CosmosClient, database_id: String) -> Self {
pub(crate) fn new(root_client: CosmosClient, database_id: &str) -> Self {
let base_url = {
let mut u = root_client.endpoint().clone();
{
let mut segments = u
.path_segments_mut()
.expect("The root client should have validated the format of the URL");
segments.push("dbs");
segments.push(&database_id);
segments.push(database_id);
}
u
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
#![cfg_attr(docsrs, feature(doc_cfg_hide))]

mod authorization_policy;
mod clients;
pub mod clients;
mod options;

/// Model types sent to and received from the Cosmos API.
pub mod models;

pub use clients::*;
pub use clients::{CosmosClient, CosmosClientMethods};
pub use options::*;
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use azure_core::Model;
use azure_core::{
date::{ComponentRange, OffsetDateTime},
Model,
};
use serde::{Deserialize, Serialize};
use time::error::ComponentRange;
use time::OffsetDateTime;

#[cfg(doc)]
use crate::DatabaseClientMethods;
use crate::{clients::DatabaseClientMethods, CosmosClientMethods};

/// Represents a timestamp in the format expected by Cosmos DB.
///
/// Cosmos DB timestamps are represented as the number of seconds since the Unix epoch.
/// Use [`CosmosTimestamp::try_into`] implementation to convert this into a [`time::OffsetDateTime`].
/// Use [`CosmosTimestamp::try_into`] implementation to convert this into an [`OffsetDateTime`].
#[derive(Serialize, Deserialize, Debug)]
pub struct CosmosTimestamp(i64);

Expand All @@ -25,7 +26,7 @@ impl TryInto<OffsetDateTime> for CosmosTimestamp {

/// Properties of a Cosmos DB database.
///
/// Returned by [`DatabaseClient::read()`](crate::DatabaseClient::read()).
/// Returned by [`DatabaseClient::read()`](crate::clients::DatabaseClient::read()).
#[derive(Model, Debug, Deserialize)]
pub struct DatabaseProperties {
/// The ID of the database.
Expand Down
Loading

0 comments on commit d9b2683

Please sign in to comment.