Skip to content

Commit

Permalink
Fix review comments. Iter 1
Browse files Browse the repository at this point in the history
  • Loading branch information
rmn-boiko committed Oct 8, 2024
1 parent 546ec2f commit d87782c
Show file tree
Hide file tree
Showing 27 changed files with 370 additions and 440 deletions.
3 changes: 0 additions & 3 deletions src/adapter/http/src/data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

mod account_handler;
mod dataset_info_handler;
mod ingest_handler;
pub mod metadata_handler;
mod query_handler;
pub mod query_types;
mod router;
mod tail_handler;
mod verify_handler;
mod workspace_info_handler;

pub use router::*;
12 changes: 0 additions & 12 deletions src/adapter/http/src/data/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,6 @@ pub fn root_router() -> axum::Router {
"/verify",
axum::routing::post(super::verify_handler::verify_handler),
)
.route(
"/workspace/info",
axum::routing::get(super::workspace_info_handler::workspace_info_handler),
)
.route(
"/me",
axum::routing::get(super::account_handler::account_handler),
)
.route(
"/datasets/:id",
axum::routing::get(super::dataset_info_handler::dataset_info_handler),
)
}

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,10 @@

use axum::extract::Extension;
use axum::response::Json;
use chrono::{DateTime, Utc};
use database_common_macros::transactional_handler;
use dill::Catalog;
use http_common::*;
use kamu_accounts::{
Account,
AccountDisplayName,
AccountType,
AuthenticationService,
CurrentAccountSubject,
};
use kamu_accounts::{Account, AuthenticationService, CurrentAccountSubject};
use opendatafabric::{AccountID, AccountName};

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -38,29 +31,13 @@ use opendatafabric::{AccountID, AccountName};
pub struct AccountResponse {
pub id: AccountID,
pub account_name: AccountName,
pub email: Option<String>,
pub display_name: AccountDisplayName,
pub account_type: AccountType,
pub avatar_url: Option<String>,
pub registered_at: DateTime<Utc>,
pub is_admin: bool,
pub provider: String,
pub provider_identity_key: String,
}

impl From<Account> for AccountResponse {
fn from(value: Account) -> Self {
Self {
id: value.id,
account_name: value.account_name,
email: value.email,
display_name: value.display_name,
account_type: value.account_type,
avatar_url: value.avatar_url,
registered_at: value.registered_at,
is_admin: value.is_admin,
provider: value.provider,
provider_identity_key: value.provider_identity_key,
}
}
}
Expand All @@ -82,12 +59,11 @@ async fn get_account(catalog: &Catalog) -> Result<Json<AccountResponse>, ApiErro
CurrentAccountSubject::Anonymous(_) => Err(ApiError::new_unauthorized()),
CurrentAccountSubject::Logged(account) => {
let auth_service = catalog.get_one::<dyn AuthenticationService>().unwrap();
let full_account_info_maybe = auth_service.account_by_id(&account.account_id).await?;
if let Some(full_account_info) = full_account_info_maybe {
return Ok(Json(full_account_info.into()));
}

Err(ApiError::not_found_without_body())
let full_account_info = auth_service
.account_by_id(&account.account_id)
.await?
.unwrap();
Ok(Json(full_account_info.into()))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ use http_common::*;
use kamu_core::{DatasetRepository, GetDatasetError};
use opendatafabric::{AccountName, DatasetHandle, DatasetID, DatasetName};

use crate::axum_utils::ensure_authenticated_account;

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

#[derive(Debug, serde::Serialize)]
Expand Down Expand Up @@ -60,6 +62,8 @@ async fn get_dataset_by_id(
catalog: &Catalog,
dataset_id: &DatasetID,
) -> Result<Json<DatasetInfoResponse>, ApiError> {
ensure_authenticated_account(catalog).api_err()?;

let dataset_repo = catalog.get_one::<dyn DatasetRepository>().unwrap();
let dataset_handle = dataset_repo
.resolve_dataset_ref(&dataset_id.clone().as_local_ref())
Expand Down
15 changes: 15 additions & 0 deletions src/adapter/http/src/general/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright Kamu Data, Inc. and contributors. All rights reserved.
//
// Use of this software is governed by the Business Source License
// included in the LICENSE file.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

mod account_handler;
mod dataset_info_handler;
mod node_info_handler;
mod router;

pub use router::*;
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,24 @@ use kamu_core::DatasetRepository;

#[derive(Debug, serde::Serialize)]
#[serde(rename_all = "camelCase")]
pub struct WorkspaceInfoResponse {
pub struct NodeInfoResponse {
pub is_multi_tenant: bool,
}

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

pub async fn workspace_info_handler(
pub async fn node_info_handler(
Extension(catalog): Extension<Catalog>,
) -> Result<Json<WorkspaceInfoResponse>, ApiError> {
let response = get_workspace_info(&catalog);
tracing::debug!(?response, "Get workspace info response");
) -> Result<Json<NodeInfoResponse>, ApiError> {
let response = get_node_info(&catalog);
tracing::debug!(?response, "Get node info response");
Ok(response)
}

fn get_workspace_info(catalog: &Catalog) -> Json<WorkspaceInfoResponse> {
fn get_node_info(catalog: &Catalog) -> Json<NodeInfoResponse> {
let dataset_repo = catalog.get_one::<dyn DatasetRepository>().unwrap();

Json(WorkspaceInfoResponse {
Json(NodeInfoResponse {
is_multi_tenant: dataset_repo.is_multi_tenant(),
})
}
Expand Down
37 changes: 37 additions & 0 deletions src/adapter/http/src/general/router.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright Kamu Data, Inc. and contributors. All rights reserved.
//
// Use of this software is governed by the Business Source License
// included in the LICENSE file.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

// Copyright Kamu Data, Inc. and contributors. All rights reserved.
//
// Use of this software is governed by the Business Source License
// included in the LICENSE file.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

pub fn root_router() -> axum::Router {
axum::Router::new()
.route(
"/info",
axum::routing::get(super::node_info_handler::node_info_handler),
)
.route(
"/accounts/me",
axum::routing::get(super::account_handler::account_handler),
)
.route(
"/datasets/:id",
axum::routing::get(super::dataset_info_handler::dataset_info_handler),
)
}

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
1 change: 1 addition & 0 deletions src/adapter/http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub mod smart_protocol;
mod upload;
mod ws_common;
pub use upload::*;
pub mod general;

pub type SmartTransferProtocolClientWs =
smart_protocol::ws_tungstenite_client::WsSmartTransferProtocolClient;
14 changes: 6 additions & 8 deletions src/adapter/http/tests/harness/client_side_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ use opendatafabric::{
AccountID,
AccountName,
DatasetID,
DatasetPushTarget,
DatasetRef,
DatasetRefAny,
RepoName,
TransferDatasetRef,
};
use tempfile::TempDir;
use time_source::SystemTimeSourceDefault;
Expand Down Expand Up @@ -243,23 +243,21 @@ impl ClientSideHarness {
pub async fn push_dataset(
&self,
dataset_local_ref: DatasetRef,
dataset_remote_ref: TransferDatasetRef,
dataset_remote_ref: DatasetPushTarget,
force: bool,
dataset_visibility: DatasetVisibility,
) -> Vec<PushResponse> {
self.push_service
.push_multi_ext(
vec![PushRequest {
local_ref: Some(dataset_local_ref),
remote_ref: Some(dataset_remote_ref),
}],
.push_multi(
vec![dataset_local_ref],
PushMultiOptions {
sync_options: SyncOptions {
create_if_not_exists: true,
force,
dataset_visibility,
..SyncOptions::default()
},
remote_target: Some(dataset_remote_ref),
..PushMultiOptions::default()
},
None,
Expand All @@ -270,7 +268,7 @@ impl ClientSideHarness {
pub async fn push_dataset_result(
&self,
dataset_local_ref: DatasetRef,
dataset_remote_ref: TransferDatasetRef,
dataset_remote_ref: DatasetPushTarget,
force: bool,
dataset_visibility: DatasetVisibility,
) -> SyncResult {
Expand Down
1 change: 1 addition & 0 deletions src/adapter/http/tests/harness/test_api_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl TestAPIServer {
axum::routing::post(kamu_adapter_http::platform_file_upload_post_handler),
)
.nest("/", kamu_adapter_http::data::root_router())
.nest("/", kamu_adapter_http::general::root_router())
.nest(
if multi_tenant {
"/:account_name/:dataset_name"
Expand Down
2 changes: 1 addition & 1 deletion src/adapter/http/tests/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ mod test_data_ingest;
mod test_data_query;
mod test_dataset_authorization_layer;
mod test_dataset_info;
mod test_node_info;
mod test_platform_login_validate;
mod test_protocol_dataset_helpers;
mod test_routing;
mod test_upload_local;
mod test_upload_s3;
mod test_workspace_info;
mod tests_pull;
mod tests_push;

Expand Down
12 changes: 2 additions & 10 deletions src/adapter/http/tests/tests/test_account_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async fn test_get_account_info_with_wrong_token() {
let cl = reqwest::Client::new();

let res = cl
.get(&format!("{}me", harness.root_url))
.get(&format!("{}accounts/me", harness.root_url))
.send()
.await
.unwrap();
Expand All @@ -43,7 +43,7 @@ async fn test_get_account_info() {
let cl = reqwest::Client::new();

let res = cl
.get(&format!("{}me", harness.root_url))
.get(&format!("{}accounts/me", harness.root_url))
.header("Authorization", format!("Bearer {DUMMY_ACCESS_TOKEN}"))
.send()
.await
Expand All @@ -53,15 +53,7 @@ async fn test_get_account_info() {
res.json::<serde_json::Value>().await.unwrap(),
json!({
"accountName": expected_account.account_name,
"accountType": expected_account.account_type,
"id": expected_account.id,
"avatarUrl": expected_account.avatar_url,
"displayName": expected_account.display_name,
"email": expected_account.email,
"isAdmin": expected_account.is_admin,
"provider": expected_account.provider,
"providerIdentityKey": expected_account.provider_identity_key,
"registeredAt": expected_account.registered_at
})
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ use crate::harness::*;
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

#[test_log::test(tokio::test)]
async fn test_workspace_info_single_tenant() {
let harness = WorkspaceInfoHarness::new(false).await;
async fn test_node_info_single_tenant() {
let harness = NodeInfoHarness::new(false).await;

let client = async move {
let cl = reqwest::Client::new();

let res = cl
.get(&format!("{}workspace/info", harness.root_url))
.get(&format!("{}info", harness.root_url))
.send()
.await
.unwrap()
Expand All @@ -40,14 +40,14 @@ async fn test_workspace_info_single_tenant() {
}

#[test_log::test(tokio::test)]
async fn test_workspace_info_multi_tenant() {
let harness = WorkspaceInfoHarness::new(true).await;
async fn test_node_info_multi_tenant() {
let harness = NodeInfoHarness::new(true).await;

let client = async move {
let cl = reqwest::Client::new();

let res = cl
.get(&format!("{}workspace/info", harness.root_url))
.get(&format!("{}info", harness.root_url))
.send()
.await
.unwrap()
Expand All @@ -67,12 +67,12 @@ async fn test_workspace_info_multi_tenant() {

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

struct WorkspaceInfoHarness {
struct NodeInfoHarness {
root_url: url::Url,
server_harness: ServerSideLocalFsHarness,
}

impl WorkspaceInfoHarness {
impl NodeInfoHarness {
async fn new(is_multi_tenant: bool) -> Self {
let server_harness = ServerSideLocalFsHarness::new(ServerSideHarnessOptions {
multi_tenant: is_multi_tenant,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub(crate) struct SmartPushNewDatasetViaRepoRefScenario<TServerHarness: ServerSi
pub server_harness: TServerHarness,
pub server_dataset_layout: DatasetLayout,
pub client_dataset_layout: DatasetLayout,
pub server_dataset_ref: TransferDatasetRef,
pub server_dataset_ref: DatasetPushTarget,
pub client_dataset_ref: DatasetRef,
pub client_commit_result: CommitResult,
}
Expand Down Expand Up @@ -81,7 +81,7 @@ impl<TServerHarness: ServerSideHarness> SmartPushNewDatasetViaRepoRefScenario<TS
&server_harness.api_server_addr(),
)
.unwrap();
let server_dataset_ref = TransferDatasetRef::Repository(RepoName::new_unchecked("foo"));
let server_dataset_ref = DatasetPushTarget::Repository(RepoName::new_unchecked("foo"));

Self {
client_harness,
Expand Down
Loading

0 comments on commit d87782c

Please sign in to comment.