From 442fb1ff098f57b40c5cea2f48d7d8b7bf57be60 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 12:09:49 +0200 Subject: [PATCH 01/20] DatasetEntryServiceExt: absorb DatasetOwnershipService::get_owned_datasets() --- Cargo.lock | 1 + .../flows_mut/account_flow_triggers_mut.rs | 8 ++--- .../queries/accounts/account_flow_triggers.rs | 10 +++--- src/domain/datasets/domain/Cargo.toml | 1 + .../src/services/dataset_entry_service.rs | 34 +++++++++++++++++++ .../src/services/dataset_ownership_service.rs | 5 --- .../src/dataset_entry_service_impl.rs | 17 ---------- .../src/flow/flow_query_service_impl.rs | 16 ++++----- 8 files changed, 53 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 045214aca4..066eee317e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6199,6 +6199,7 @@ dependencies = [ "async-trait", "chrono", "database-common", + "futures", "internal-error", "merge", "mockall", diff --git a/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs b/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs index 494f551f51..33085ccc2c 100644 --- a/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs +++ b/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs @@ -9,7 +9,7 @@ use chrono::Utc; use kamu_accounts::Account; -use kamu_datasets::DatasetOwnershipService; +use kamu_datasets::{DatasetEntryService, DatasetEntryServiceExt}; use kamu_flow_system::FlowTriggerService; use opendatafabric::DatasetID; @@ -30,9 +30,9 @@ impl AccountFlowTriggersMut { #[graphql(skip)] async fn get_account_dataset_ids(&self, ctx: &Context<'_>) -> Result> { - let dataset_ownership_service = from_catalog_n!(ctx, dyn DatasetOwnershipService); - let dataset_ids: Vec<_> = dataset_ownership_service - .get_owned_datasets(&self.account.id) + let dataset_entry_service = from_catalog_n!(ctx, dyn DatasetEntryService); + let dataset_ids: Vec<_> = dataset_entry_service + .get_owned_dataset_ids(&self.account.id) .await?; Ok(dataset_ids) diff --git a/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs b/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs index 882c01e1df..a0f0a83de5 100644 --- a/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs +++ b/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs @@ -9,7 +9,7 @@ use futures::StreamExt; use kamu_accounts::Account as AccountEntity; -use kamu_datasets::DatasetOwnershipService; +use kamu_datasets::{DatasetEntryService, DatasetEntryServiceExt}; use kamu_flow_system::FlowTriggerService; use crate::prelude::*; @@ -29,11 +29,11 @@ impl AccountFlowTriggers { /// Checks if all triggers of all datasets in account are disabled async fn all_paused(&self, ctx: &Context<'_>) -> Result { - let (dataset_ownership_service, flow_trigger_service) = - from_catalog_n!(ctx, dyn DatasetOwnershipService, dyn FlowTriggerService); + let (dataset_entry_service, flow_trigger_service) = + from_catalog_n!(ctx, dyn DatasetEntryService, dyn FlowTriggerService); - let owned_dataset_ids: Vec<_> = dataset_ownership_service - .get_owned_datasets(&self.account.id) + let owned_dataset_ids: Vec<_> = dataset_entry_service + .get_owned_dataset_ids(&self.account.id) .await?; let mut all_triggers = flow_trigger_service diff --git a/src/domain/datasets/domain/Cargo.toml b/src/domain/datasets/domain/Cargo.toml index 35e6dae87c..5909a08e14 100644 --- a/src/domain/datasets/domain/Cargo.toml +++ b/src/domain/datasets/domain/Cargo.toml @@ -35,6 +35,7 @@ opendatafabric = { workspace = true } aes-gcm = { version = "0.10.3" } async-trait = { version = "0.1", default-features = false } chrono = { version = "0.4", default-features = false } +futures = { version = "0.3", default-features = false } merge = "0.1" secrecy = "0.10" serde = "1" diff --git a/src/domain/datasets/domain/src/services/dataset_entry_service.rs b/src/domain/datasets/domain/src/services/dataset_entry_service.rs index a142981c87..8129b6c963 100644 --- a/src/domain/datasets/domain/src/services/dataset_entry_service.rs +++ b/src/domain/datasets/domain/src/services/dataset_entry_service.rs @@ -36,6 +36,40 @@ pub trait DatasetEntryService: Sync + Send { ) -> Result, ListDatasetEntriesError>; } +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[async_trait::async_trait] +pub trait DatasetEntryServiceExt: Sync + Send { + async fn get_owned_dataset_ids( + &self, + owner_id: &odf::AccountID, + ) -> Result, InternalError>; +} + +#[async_trait::async_trait] +impl DatasetEntryServiceExt for T +where + T: DatasetEntryService, + T: ?Sized, +{ + async fn get_owned_dataset_ids( + &self, + owner_id: &odf::AccountID, + ) -> Result, InternalError> { + use futures::TryStreamExt; + + let owned_dataset_ids = self + .entries_owned_by(owner_id) + .try_collect::>() + .await? + .into_iter() + .map(|dataset_entry| dataset_entry.id) + .collect::>(); + + Ok(owned_dataset_ids) + } +} + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Errors //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/datasets/domain/src/services/dataset_ownership_service.rs b/src/domain/datasets/domain/src/services/dataset_ownership_service.rs index fe68361f74..517fbf352f 100644 --- a/src/domain/datasets/domain/src/services/dataset_ownership_service.rs +++ b/src/domain/datasets/domain/src/services/dataset_ownership_service.rs @@ -19,11 +19,6 @@ pub trait DatasetOwnershipService: Sync + Send { dataset_id: &odf::DatasetID, ) -> Result; - async fn get_owned_datasets( - &self, - account_id: &odf::AccountID, - ) -> Result, InternalError>; - async fn is_dataset_owned_by( &self, dataset_id: &odf::DatasetID, diff --git a/src/domain/datasets/services/src/dataset_entry_service_impl.rs b/src/domain/datasets/services/src/dataset_entry_service_impl.rs index a7876a6387..345074d3c1 100644 --- a/src/domain/datasets/services/src/dataset_entry_service_impl.rs +++ b/src/domain/datasets/services/src/dataset_entry_service_impl.rs @@ -540,23 +540,6 @@ impl DatasetOwnershipService for DatasetEntryServiceImpl { Ok(dataset_entry.owner_id) } - async fn get_owned_datasets( - &self, - account_id: &odf::AccountID, - ) -> Result, InternalError> { - use futures::TryStreamExt; - - let owned_dataset_ids = self - .entries_owned_by(account_id) - .try_collect::>() - .await? - .into_iter() - .map(|dataset_entry| dataset_entry.id) - .collect::>(); - - Ok(owned_dataset_ids) - } - async fn is_dataset_owned_by( &self, dataset_id: &odf::DatasetID, diff --git a/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs b/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs index 1fff1f7bbb..1146ff079e 100644 --- a/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs +++ b/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs @@ -15,7 +15,7 @@ use database_common::PaginationOpts; use dill::{component, interface, Catalog}; use futures::TryStreamExt; use internal_error::ResultIntoInternal; -use kamu_datasets::DatasetOwnershipService; +use kamu_datasets::{DatasetEntryService, DatasetEntryServiceExt}; use kamu_flow_system::*; use opendatafabric::{AccountID, DatasetID}; @@ -27,7 +27,7 @@ use crate::{FlowAbortHelper, FlowSchedulingHelper}; pub struct FlowQueryServiceImpl { catalog: Catalog, flow_event_store: Arc, - dataset_ownership_service: Arc, + dataset_entry_service: Arc, agent_config: Arc, flow_state: Arc, } @@ -38,14 +38,14 @@ impl FlowQueryServiceImpl { pub fn new( catalog: Catalog, flow_event_store: Arc, - dataset_ownership_service: Arc, + dataset_entry_service: Arc, agent_config: Arc, flow_state: Arc, ) -> Self { Self { catalog, flow_event_store, - dataset_ownership_service, + dataset_entry_service, agent_config, flow_state, } @@ -110,8 +110,8 @@ impl FlowQueryService for FlowQueryServiceImpl { pagination: PaginationOpts, ) -> Result { let owned_dataset_ids = self - .dataset_ownership_service - .get_owned_datasets(account_id) + .dataset_entry_service + .get_owned_dataset_ids(account_id) .await .map_err(ListFlowsByDatasetError::Internal)?; @@ -162,8 +162,8 @@ impl FlowQueryService for FlowQueryServiceImpl { account_id: &AccountID, ) -> Result { let owned_dataset_ids = self - .dataset_ownership_service - .get_owned_datasets(account_id) + .dataset_entry_service + .get_owned_dataset_ids(account_id) .await .map_err(ListFlowsByDatasetError::Internal)?; From e6611fa792a37b98286946789f4e61795ac717df Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 12:33:45 +0200 Subject: [PATCH 02/20] DatasetEntryServiceExt: absorb all rest DatasetOwnershipService --- .../dataset_mut/dataset_mut_utils.rs | 9 ++-- .../flows_mut/account_flow_triggers_mut.rs | 3 +- .../queries/accounts/account_flow_triggers.rs | 3 +- .../src/services/dataset_entry_service.rs | 45 +++++++++++++++++-- .../src/services/dataset_ownership_service.rs | 29 ------------ .../datasets/domain/src/services/mod.rs | 2 - .../src/dataset_entry_service_impl.rs | 44 ++++-------------- .../src/flow/flow_query_service_impl.rs | 4 +- .../src/flow/flow_scheduling_helper.rs | 21 +++++---- 9 files changed, 72 insertions(+), 88 deletions(-) delete mode 100644 src/domain/datasets/domain/src/services/dataset_ownership_service.rs diff --git a/src/adapter/graphql/src/mutations/dataset_mut/dataset_mut_utils.rs b/src/adapter/graphql/src/mutations/dataset_mut/dataset_mut_utils.rs index c122a8721e..11a65fd1d3 100644 --- a/src/adapter/graphql/src/mutations/dataset_mut/dataset_mut_utils.rs +++ b/src/adapter/graphql/src/mutations/dataset_mut/dataset_mut_utils.rs @@ -7,7 +7,7 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use kamu_datasets::DatasetOwnershipService; +use kamu_datasets::{DatasetEntryService, DatasetEntryServiceExt}; use opendatafabric as odf; use crate::prelude::*; @@ -19,7 +19,7 @@ pub(crate) async fn ensure_account_owns_dataset( ctx: &Context<'_>, dataset_handle: &odf::DatasetHandle, ) -> Result<()> { - let dataset_ownership_service = from_catalog_n!(ctx, dyn DatasetOwnershipService); + let dataset_entry_service = from_catalog_n!(ctx, dyn DatasetEntryService); let logged_account = utils::get_logged_account(ctx)?; if logged_account.is_admin { @@ -27,9 +27,10 @@ pub(crate) async fn ensure_account_owns_dataset( return Ok(()); } - let not_owner = !dataset_ownership_service + let not_owner = !dataset_entry_service .is_dataset_owned_by(&dataset_handle.id, &logged_account.account_id) - .await?; + .await + .int_err()?; if not_owner { return Err(Error::new("Only the dataset owner can perform this action").into()); diff --git a/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs b/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs index 33085ccc2c..e884773f8c 100644 --- a/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs +++ b/src/adapter/graphql/src/mutations/flows_mut/account_flow_triggers_mut.rs @@ -33,7 +33,8 @@ impl AccountFlowTriggersMut { let dataset_entry_service = from_catalog_n!(ctx, dyn DatasetEntryService); let dataset_ids: Vec<_> = dataset_entry_service .get_owned_dataset_ids(&self.account.id) - .await?; + .await + .int_err()?; Ok(dataset_ids) } diff --git a/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs b/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs index a0f0a83de5..8f979e958f 100644 --- a/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs +++ b/src/adapter/graphql/src/queries/accounts/account_flow_triggers.rs @@ -34,7 +34,8 @@ impl AccountFlowTriggers { let owned_dataset_ids: Vec<_> = dataset_entry_service .get_owned_dataset_ids(&self.account.id) - .await?; + .await + .int_err()?; let mut all_triggers = flow_trigger_service .find_triggers_by_datasets(owned_dataset_ids) diff --git a/src/domain/datasets/domain/src/services/dataset_entry_service.rs b/src/domain/datasets/domain/src/services/dataset_entry_service.rs index 8129b6c963..f62a5d17f8 100644 --- a/src/domain/datasets/domain/src/services/dataset_entry_service.rs +++ b/src/domain/datasets/domain/src/services/dataset_entry_service.rs @@ -8,11 +8,11 @@ // by the Apache License, Version 2.0. use database_common::{EntityPageListing, PaginationOpts}; -use internal_error::InternalError; +use internal_error::{ErrorIntoInternal, InternalError}; use opendatafabric as odf; use thiserror::Error; -use crate::{DatasetEntry, DatasetEntryStream}; +use crate::{DatasetEntry, DatasetEntryStream, GetDatasetEntryError}; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -24,6 +24,11 @@ pub trait DatasetEntryService: Sync + Send { fn entries_owned_by(&self, owner_id: &odf::AccountID) -> DatasetEntryStream; + async fn get_entry( + &self, + dataset_id: &odf::DatasetID, + ) -> Result; + async fn list_all_entries( &self, pagination: PaginationOpts, @@ -43,7 +48,13 @@ pub trait DatasetEntryServiceExt: Sync + Send { async fn get_owned_dataset_ids( &self, owner_id: &odf::AccountID, - ) -> Result, InternalError>; + ) -> Result, GetOwnedDatasetIdsError>; + + async fn is_dataset_owned_by( + &self, + dataset_id: &odf::DatasetID, + account_id: &odf::AccountID, + ) -> Result; } #[async_trait::async_trait] @@ -55,7 +66,7 @@ where async fn get_owned_dataset_ids( &self, owner_id: &odf::AccountID, - ) -> Result, InternalError> { + ) -> Result, GetOwnedDatasetIdsError> { use futures::TryStreamExt; let owned_dataset_ids = self @@ -68,6 +79,20 @@ where Ok(owned_dataset_ids) } + + async fn is_dataset_owned_by( + &self, + dataset_id: &odf::DatasetID, + account_id: &odf::AccountID, + ) -> Result { + match self.get_entry(dataset_id).await { + Ok(entry) => Ok(entry.owner_id == *account_id), + Err(err) => match err { + GetDatasetEntryError::NotFound(_) => Ok(false), + unexpected_error => Err(unexpected_error.int_err().into()), + }, + } + } } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -80,4 +105,16 @@ pub enum ListDatasetEntriesError { Internal(#[from] InternalError), } +#[derive(Error, Debug)] +pub enum GetOwnedDatasetIdsError { + #[error(transparent)] + Internal(#[from] InternalError), +} + +#[derive(Error, Debug)] +pub enum IsDatasetOwnedByError { + #[error(transparent)] + Internal(#[from] InternalError), +} + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/datasets/domain/src/services/dataset_ownership_service.rs b/src/domain/datasets/domain/src/services/dataset_ownership_service.rs deleted file mode 100644 index 517fbf352f..0000000000 --- a/src/domain/datasets/domain/src/services/dataset_ownership_service.rs +++ /dev/null @@ -1,29 +0,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. - -use internal_error::InternalError; -use opendatafabric as odf; - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - -#[async_trait::async_trait] -pub trait DatasetOwnershipService: Sync + Send { - async fn get_dataset_owner( - &self, - dataset_id: &odf::DatasetID, - ) -> Result; - - async fn is_dataset_owned_by( - &self, - dataset_id: &odf::DatasetID, - account_id: &odf::AccountID, - ) -> Result; -} - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/datasets/domain/src/services/mod.rs b/src/domain/datasets/domain/src/services/mod.rs index 004333bdb6..a0b3ec926a 100644 --- a/src/domain/datasets/domain/src/services/mod.rs +++ b/src/domain/datasets/domain/src/services/mod.rs @@ -10,9 +10,7 @@ mod dataset_entry_service; mod dataset_env_var_service; mod dataset_key_value_service; -mod dataset_ownership_service; pub use dataset_entry_service::*; pub use dataset_env_var_service::*; pub use dataset_key_value_service::*; -pub use dataset_ownership_service::*; diff --git a/src/domain/datasets/services/src/dataset_entry_service_impl.rs b/src/domain/datasets/services/src/dataset_entry_service_impl.rs index 345074d3c1..d384b081af 100644 --- a/src/domain/datasets/services/src/dataset_entry_service_impl.rs +++ b/src/domain/datasets/services/src/dataset_entry_service_impl.rs @@ -12,7 +12,7 @@ use std::sync::Arc; use database_common::{EntityPageListing, EntityPageStreamer, PaginationOpts}; use dill::{component, interface, meta, Catalog}; -use internal_error::{ErrorIntoInternal, InternalError, ResultIntoInternal}; +use internal_error::{InternalError, ResultIntoInternal}; use kamu_accounts::{AccountRepository, CurrentAccountSubject}; use kamu_core::{ DatasetHandleStream, @@ -68,7 +68,6 @@ struct AccountsCache { #[component(pub)] #[interface(dyn DatasetEntryService)] #[interface(dyn DatasetRegistry)] -#[interface(dyn DatasetOwnershipService)] #[interface(dyn MessageConsumer)] #[interface(dyn MessageConsumerT)] #[meta(MessageConsumerMeta { @@ -350,6 +349,13 @@ impl DatasetEntryService for DatasetEntryServiceImpl { ) } + async fn get_entry( + &self, + dataset_id: &odf::DatasetID, + ) -> Result { + self.dataset_entry_repo.get_dataset_entry(dataset_id).await + } + async fn list_all_entries( &self, pagination: PaginationOpts, @@ -525,40 +531,6 @@ impl DatasetRegistry for DatasetEntryServiceImpl { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -#[async_trait::async_trait] -impl DatasetOwnershipService for DatasetEntryServiceImpl { - async fn get_dataset_owner( - &self, - dataset_id: &odf::DatasetID, - ) -> Result { - let dataset_entry = self - .dataset_entry_repo - .get_dataset_entry(dataset_id) - .await - .int_err()?; - - Ok(dataset_entry.owner_id) - } - - async fn is_dataset_owned_by( - &self, - dataset_id: &odf::DatasetID, - account_id: &odf::AccountID, - ) -> Result { - let get_res = self.dataset_entry_repo.get_dataset_entry(dataset_id).await; - - match get_res { - Ok(dataset_entry) => Ok(dataset_entry.owner_id == *account_id), - Err(err) => match err { - GetDatasetEntryError::NotFound(_) => Ok(false), - unexpected_error => Err(unexpected_error.int_err()), - }, - } - } -} - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - impl MessageConsumer for DatasetEntryServiceImpl {} //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs b/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs index 1146ff079e..41460a4e44 100644 --- a/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs +++ b/src/domain/flow-system/services/src/flow/flow_query_service_impl.rs @@ -113,7 +113,7 @@ impl FlowQueryService for FlowQueryServiceImpl { .dataset_entry_service .get_owned_dataset_ids(account_id) .await - .map_err(ListFlowsByDatasetError::Internal)?; + .int_err()?; let filtered_dataset_ids = if !filters.by_dataset_ids.is_empty() { owned_dataset_ids @@ -165,7 +165,7 @@ impl FlowQueryService for FlowQueryServiceImpl { .dataset_entry_service .get_owned_dataset_ids(account_id) .await - .map_err(ListFlowsByDatasetError::Internal)?; + .int_err()?; let matched_stream = Box::pin(async_stream::try_stream! { for dataset_id in &owned_dataset_ids { diff --git a/src/domain/flow-system/services/src/flow/flow_scheduling_helper.rs b/src/domain/flow-system/services/src/flow/flow_scheduling_helper.rs index 4db47cf5d8..ce74bd2e00 100644 --- a/src/domain/flow-system/services/src/flow/flow_scheduling_helper.rs +++ b/src/domain/flow-system/services/src/flow/flow_scheduling_helper.rs @@ -13,7 +13,7 @@ use chrono::{DateTime, Utc}; use dill::component; use internal_error::InternalError; use kamu_core::{DatasetChangesService, DependencyGraphService}; -use kamu_datasets::DatasetOwnershipService; +use kamu_datasets::{DatasetEntryService, DatasetEntryServiceExt}; use kamu_flow_system::*; use messaging_outbox::{Outbox, OutboxExt}; use time_source::SystemTimeSource; @@ -30,7 +30,7 @@ pub(crate) struct FlowSchedulingHelper { outbox: Arc, dataset_changes_service: Arc, dependency_graph_service: Arc, - dataset_ownership_service: Arc, + dataset_entry_service: Arc, time_source: Arc, agent_config: Arc, } @@ -44,7 +44,7 @@ impl FlowSchedulingHelper { outbox: Arc, dataset_changes_service: Arc, dependency_graph_service: Arc, - dataset_ownership_service: Arc, + dataset_entry_service: Arc, time_source: Arc, agent_config: Arc, ) -> Self { @@ -55,7 +55,7 @@ impl FlowSchedulingHelper { outbox, dataset_changes_service, dependency_graph_service, - dataset_ownership_service, + dataset_entry_service, time_source, agent_config, } @@ -215,15 +215,18 @@ impl FlowSchedulingHelper { DownstreamDependencyTriggerType::TriggerOwnHardCompaction => { let owner_account_id = self - .dataset_ownership_service - .get_dataset_owner(&fk_dataset.dataset_id) - .await?; + .dataset_entry_service + .get_entry(&fk_dataset.dataset_id) + .await + .int_err()? + .owner_id; for dependent_dataset_id in dependent_dataset_ids { let owned = self - .dataset_ownership_service + .dataset_entry_service .is_dataset_owned_by(&dependent_dataset_id, &owner_account_id) - .await?; + .await + .int_err()?; if owned { plans.push(DownstreamDependencyFlowPlan { From 8377f3d410a5c9363203799dde832cfaf71d94a0 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 15:10:49 +0200 Subject: [PATCH 03/20] kamu-adapter-auth-oso-rebac: remove duplicate dep --- src/adapter/auth-oso-rebac/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/src/adapter/auth-oso-rebac/Cargo.toml b/src/adapter/auth-oso-rebac/Cargo.toml index 78b9abe049..43de3683f9 100644 --- a/src/adapter/auth-oso-rebac/Cargo.toml +++ b/src/adapter/auth-oso-rebac/Cargo.toml @@ -44,7 +44,6 @@ tracing = { version = "0.1", default-features = false } kamu-accounts-inmem = { workspace = true } kamu-accounts-services = { workspace = true } kamu-auth-rebac-inmem = { workspace = true } -kamu-auth-rebac-services = { workspace = true } kamu-core = { workspace = true, default-features = false, features = ["oso", "testing"] } kamu-datasets-inmem = { workspace = true } kamu-datasets-services = { workspace = true } From df0f6c185dcb72d57c3939fddf50eabd9e1f7b45 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 15:12:25 +0200 Subject: [PATCH 04/20] DatasetActionAuthorizer: classify_datasets_by_allowance() -> classify_dataset_handles_by_allowance() --- src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs | 2 +- src/adapter/graphql/src/queries/datasets/datasets.rs | 2 +- src/domain/core/src/auth/dataset_action_authorizer.rs | 4 ++-- src/infra/core/src/testing/mock_dataset_action_authorizer.rs | 4 ++-- src/infra/core/src/use_cases/pull_dataset_use_case_impl.rs | 4 ++-- src/infra/core/src/use_cases/push_dataset_use_case_impl.rs | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs index 4ef045e30b..a27deea442 100644 --- a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs +++ b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs @@ -174,7 +174,7 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer { } #[tracing::instrument(level = "debug", skip_all, fields(dataset_handles=?dataset_handles, action=%action))] - async fn classify_datasets_by_allowance( + async fn classify_dataset_handles_by_allowance( &self, dataset_handles: Vec, action: DatasetAction, diff --git a/src/adapter/graphql/src/queries/datasets/datasets.rs b/src/adapter/graphql/src/queries/datasets/datasets.rs index 43509f1fd5..84f1b0f190 100644 --- a/src/adapter/graphql/src/queries/datasets/datasets.rs +++ b/src/adapter/graphql/src/queries/datasets/datasets.rs @@ -104,7 +104,7 @@ impl Datasets { account_owned_datasets_stream.try_next().await.int_err()? { let authorized_handles = dataset_action_authorizer - .classify_datasets_by_allowance( + .classify_dataset_handles_by_allowance( account_owned_dataset_handles_chunk, DatasetAction::Read, ) diff --git a/src/domain/core/src/auth/dataset_action_authorizer.rs b/src/domain/core/src/auth/dataset_action_authorizer.rs index f308a2e577..4edda84349 100644 --- a/src/domain/core/src/auth/dataset_action_authorizer.rs +++ b/src/domain/core/src/auth/dataset_action_authorizer.rs @@ -55,7 +55,7 @@ pub trait DatasetActionAuthorizer: Sync + Send { ) -> Result, InternalError>; // TODO: Private Datasets: tests - async fn classify_datasets_by_allowance( + async fn classify_dataset_handles_by_allowance( &self, dataset_handles: Vec, action: DatasetAction, @@ -250,7 +250,7 @@ impl DatasetActionAuthorizer for AlwaysHappyDatasetActionAuthorizer { Ok(dataset_handles) } - async fn classify_datasets_by_allowance( + async fn classify_dataset_handles_by_allowance( &self, dataset_handles: Vec, _action: DatasetAction, diff --git a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs index 5713c1250a..486584ac78 100644 --- a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs +++ b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs @@ -47,7 +47,7 @@ mockall::mock! { action: DatasetAction, ) -> Result, InternalError>; - async fn classify_datasets_by_allowance( + async fn classify_dataset_handles_by_allowance( &self, dataset_handles: Vec, action: DatasetAction, @@ -165,7 +165,7 @@ impl MockDatasetActionAuthorizer { times: usize, authorized: HashSet, ) -> Self { - self.expect_classify_datasets_by_allowance() + self.expect_classify_dataset_handles_by_allowance() .with(always(), eq(action)) .times(times) .returning(move |handles, action| { diff --git a/src/infra/core/src/use_cases/pull_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/pull_dataset_use_case_impl.rs index bc12edb642..6696c3a7ac 100644 --- a/src/infra/core/src/use_cases/pull_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/pull_dataset_use_case_impl.rs @@ -196,7 +196,7 @@ impl PullDatasetUseCaseImpl { unauthorized_handles_with_errors, } = self .dataset_action_authorizer - .classify_datasets_by_allowance(written_datasets, DatasetAction::Write) + .classify_dataset_handles_by_allowance(written_datasets, DatasetAction::Write) .await?; let mut okay_jobs = Vec::with_capacity(authorized_handles.len() + other_jobs.len()); @@ -269,7 +269,7 @@ impl PullDatasetUseCaseImpl { unauthorized_handles_with_errors, } = self .dataset_action_authorizer - .classify_datasets_by_allowance(read_datasets, DatasetAction::Read) + .classify_dataset_handles_by_allowance(read_datasets, DatasetAction::Read) .await?; if unauthorized_handles_with_errors.is_empty() { diff --git a/src/infra/core/src/use_cases/push_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/push_dataset_use_case_impl.rs index 0da57cded6..84af079811 100644 --- a/src/infra/core/src/use_cases/push_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/push_dataset_use_case_impl.rs @@ -76,7 +76,7 @@ impl PushDatasetUseCaseImpl { unauthorized_handles_with_errors, } = self .dataset_action_authorizer - .classify_datasets_by_allowance(dataset_handles, DatasetAction::Read) + .classify_dataset_handles_by_allowance(dataset_handles, DatasetAction::Read) .await?; let unauthorized_responses = unauthorized_handles_with_errors From 1ea937daa6c55d81d05da80897518f3adac3d40c Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 15:14:14 +0200 Subject: [PATCH 05/20] DatasetRegistry: remove an TODO --- src/domain/core/src/services/dataset_registry.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/domain/core/src/services/dataset_registry.rs b/src/domain/core/src/services/dataset_registry.rs index a413f79882..8953b6bf73 100644 --- a/src/domain/core/src/services/dataset_registry.rs +++ b/src/domain/core/src/services/dataset_registry.rs @@ -19,7 +19,6 @@ use crate::{DatasetHandleStream, GetDatasetError, ResolvedDataset}; pub trait DatasetRegistry: Send + Sync { fn all_dataset_handles(&self) -> DatasetHandleStream<'_>; - // TODO: Private Datasets: replace AccountName with AccountID? fn all_dataset_handles_by_owner(&self, owner_name: &AccountName) -> DatasetHandleStream<'_>; async fn resolve_dataset_handle_by_ref( From f75e125fcea9fbbaea1899a902ef9093ea743736 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 15:17:14 +0200 Subject: [PATCH 06/20] DatasetEntryRepository::get_dataset_entries(): use dataset_name column for sorting in implementations (as it was) --- ...790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json} | 4 ++-- ...72f0b9fa446ea3255ffca15f34c2af9aaeb8d31453ab364f97495.json | 0 .../postgres/src/repos/postgres_dataset_entry_repository.rs | 2 +- ...790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json} | 4 ++-- .../sqlite/src/repos/sqlite_dateset_entry_repository.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) rename src/infra/datasets/postgres/.sqlx/{query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json => query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json} (81%) delete mode 100644 src/infra/datasets/postgres/.sqlx/query-30c92efe33072f0b9fa446ea3255ffca15f34c2af9aaeb8d31453ab364f97495.json rename src/infra/datasets/sqlite/.sqlx/{query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json => query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json} (80%) diff --git a/src/infra/datasets/postgres/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json b/src/infra/datasets/postgres/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json similarity index 81% rename from src/infra/datasets/postgres/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json rename to src/infra/datasets/postgres/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json index 81679caa2a..128220e4a5 100644 --- a/src/infra/datasets/postgres/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json +++ b/src/infra/datasets/postgres/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n dataset_id as \"id: _\",\n owner_id as \"owner_id: _\",\n dataset_name as name,\n created_at as \"created_at: _\"\n FROM dataset_entries\n ORDER BY created_at ASC\n LIMIT $1 OFFSET $2\n ", + "query": "\n SELECT\n dataset_id as \"id: _\",\n owner_id as \"owner_id: _\",\n dataset_name as name,\n created_at as \"created_at: _\"\n FROM dataset_entries\n ORDER BY dataset_name ASC\n LIMIT $1 OFFSET $2\n ", "describe": { "columns": [ { @@ -37,5 +37,5 @@ false ] }, - "hash": "168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2" + "hash": "13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2" } diff --git a/src/infra/datasets/postgres/.sqlx/query-30c92efe33072f0b9fa446ea3255ffca15f34c2af9aaeb8d31453ab364f97495.json b/src/infra/datasets/postgres/.sqlx/query-30c92efe33072f0b9fa446ea3255ffca15f34c2af9aaeb8d31453ab364f97495.json deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/infra/datasets/postgres/src/repos/postgres_dataset_entry_repository.rs b/src/infra/datasets/postgres/src/repos/postgres_dataset_entry_repository.rs index cdc842cf7a..f2377e3928 100644 --- a/src/infra/datasets/postgres/src/repos/postgres_dataset_entry_repository.rs +++ b/src/infra/datasets/postgres/src/repos/postgres_dataset_entry_repository.rs @@ -104,7 +104,7 @@ impl DatasetEntryRepository for PostgresDatasetEntryRepository { dataset_name as name, created_at as "created_at: _" FROM dataset_entries - ORDER BY created_at ASC + ORDER BY dataset_name ASC LIMIT $1 OFFSET $2 "#, limit, diff --git a/src/infra/datasets/sqlite/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json b/src/infra/datasets/sqlite/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json similarity index 80% rename from src/infra/datasets/sqlite/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json rename to src/infra/datasets/sqlite/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json index 23640468a4..be47e2fc9b 100644 --- a/src/infra/datasets/sqlite/.sqlx/query-168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2.json +++ b/src/infra/datasets/sqlite/.sqlx/query-13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "\n SELECT\n dataset_id as \"id: _\",\n owner_id as \"owner_id: _\",\n dataset_name as name,\n created_at as \"created_at: _\"\n FROM dataset_entries\n ORDER BY created_at ASC\n LIMIT $1 OFFSET $2\n ", + "query": "\n SELECT\n dataset_id as \"id: _\",\n owner_id as \"owner_id: _\",\n dataset_name as name,\n created_at as \"created_at: _\"\n FROM dataset_entries\n ORDER BY dataset_name ASC\n LIMIT $1 OFFSET $2\n ", "describe": { "columns": [ { @@ -34,5 +34,5 @@ false ] }, - "hash": "168c5decfa4e1abb634750d661f3d811c12600aa6af7a06b226e7c5b7df64fb2" + "hash": "13fe35a7997b790566736b78e16c17cd7452d48887938a2a28cbd9a1408472e2" } diff --git a/src/infra/datasets/sqlite/src/repos/sqlite_dateset_entry_repository.rs b/src/infra/datasets/sqlite/src/repos/sqlite_dateset_entry_repository.rs index eecc4e200b..b655891c50 100644 --- a/src/infra/datasets/sqlite/src/repos/sqlite_dateset_entry_repository.rs +++ b/src/infra/datasets/sqlite/src/repos/sqlite_dateset_entry_repository.rs @@ -106,7 +106,7 @@ impl DatasetEntryRepository for SqliteDatasetEntryRepository { dataset_name as name, created_at as "created_at: _" FROM dataset_entries - ORDER BY created_at ASC + ORDER BY dataset_name ASC LIMIT $1 OFFSET $2 "#, limit, From f48701b2cdcdee04d8b5547a86d2e0cca1eaf5c0 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Wed, 15 Jan 2025 15:39:31 +0200 Subject: [PATCH 07/20] OsoResourceServiceImpl: state extraction to singleton component --- .../auth-oso-rebac/src/dependencies.rs | 1 + .../src/oso_resource_service_impl.rs | 30 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/adapter/auth-oso-rebac/src/dependencies.rs b/src/adapter/auth-oso-rebac/src/dependencies.rs index 7d88e90cb5..f10fb00d47 100644 --- a/src/adapter/auth-oso-rebac/src/dependencies.rs +++ b/src/adapter/auth-oso-rebac/src/dependencies.rs @@ -17,6 +17,7 @@ pub fn register_dependencies(catalog_builder: &mut CatalogBuilder) { catalog_builder.add::(); catalog_builder.add::(); catalog_builder.add::(); + catalog_builder.add::(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/adapter/auth-oso-rebac/src/oso_resource_service_impl.rs b/src/adapter/auth-oso-rebac/src/oso_resource_service_impl.rs index 73a344024d..3a51f0c6d1 100644 --- a/src/adapter/auth-oso-rebac/src/oso_resource_service_impl.rs +++ b/src/adapter/auth-oso-rebac/src/oso_resource_service_impl.rs @@ -32,32 +32,46 @@ use crate::{DatasetResource, UserActor}; type EntityId = String; #[derive(Debug, Default)] -struct State { +pub struct State { user_actor_cache_map: HashMap, } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +pub struct OsoResourceServiceImplStateHolder { + pub state: RwLock, +} + +#[component(pub)] +#[scope(Singleton)] +impl OsoResourceServiceImplStateHolder { + pub fn new() -> Self { + Self { + state: RwLock::new(State::default()), + } + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // TODO: Private Datasets: add Service trait? pub struct OsoResourceServiceImpl { - state: RwLock, + state_holder: Arc, dataset_entry_repo: Arc, rebac_service: Arc, account_repo: Arc, } #[component(pub)] -// TODO: Private Datasets: This service should be a singleton -// Alternative: put the state into a separate component -// #[scope(Singleton)] impl OsoResourceServiceImpl { pub fn new( + state_holder: Arc, dataset_entry_repo: Arc, rebac_service: Arc, account_repo: Arc, ) -> Self { Self { - state: RwLock::new(State::default()), + state_holder, dataset_entry_repo, rebac_service, account_repo, @@ -74,7 +88,7 @@ impl OsoResourceServiceImpl { // First, an attempt to get from the cache { - let readable_state = self.state.read().await; + let readable_state = self.state_holder.state.read().await; let account_id_stack = account_id.as_did_str().to_stack_string(); let maybe_cached_user_actor = readable_state @@ -107,7 +121,7 @@ impl OsoResourceServiceImpl { }; // Lastly, caching - let mut writable_state = self.state.write().await; + let mut writable_state = self.state_holder.state.write().await; writable_state .user_actor_cache_map From 6d4cf77642c766b0150e093ba784d1098a339ff9 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 00:05:25 +0200 Subject: [PATCH 08/20] DatasetActionAuthorizer::check_action_allowed(): use DatasetID instead of DatasetHandle --- .../src/oso_dataset_authorizer.rs | 14 +++-- .../tests/test_oso_dataset_authorizer.rs | 8 +-- .../src/queries/datasets/dataset_metadata.rs | 2 +- src/adapter/graphql/src/utils.rs | 33 +++++------- .../tests/test_gql_account_flow_triggers.rs | 1 + .../graphql/tests/tests/test_gql_data.rs | 1 + .../tests/tests/test_gql_dataset_env_vars.rs | 2 + .../tests/test_gql_dataset_flow_configs.rs | 2 + .../tests/tests/test_gql_dataset_flow_runs.rs | 2 + .../tests/test_gql_dataset_flow_triggers.rs | 2 + .../graphql/tests/tests/test_gql_datasets.rs | 1 + .../graphql/tests/tests/test_gql_metadata.rs | 1 + .../tests/tests/test_gql_metadata_chain.rs | 1 + .../tests/tests/test_gql_remote_statuses.rs | 1 + .../graphql/tests/tests/test_gql_search.rs | 1 + src/adapter/http/Cargo.toml | 1 + src/adapter/http/src/data/ingest_handler.rs | 2 +- .../middleware/dataset_authorization_layer.rs | 2 +- .../http/tests/harness/client_side_harness.rs | 2 + .../http/tests/harness/server_side_harness.rs | 4 +- .../harness/server_side_local_fs_harness.rs | 9 +++- .../tests/harness/server_side_s3_harness.rs | 3 +- .../tests/test_dataset_authorization_layer.rs | 37 ++++++++++---- src/adapter/http/tests/tests/test_routing.rs | 1 + .../odata/tests/tests/test_handlers.rs | 1 + src/app/cli/src/app.rs | 2 + .../cli/src/commands/inspect_query_command.rs | 2 +- src/app/cli/src/commands/log_command.rs | 2 +- .../src/auth/dataset_action_authorizer.rs | 6 +-- src/domain/core/src/services/did_generator.rs | 51 +++++++++++++++++++ src/domain/core/src/services/mod.rs | 2 + .../test_dependency_graph_service_impl.rs | 2 +- .../test_flow_configuration_service_impl.rs | 1 + .../tests/test_flow_trigger_service_impl.rs | 1 + .../tests/tests/utils/flow_harness_shared.rs | 1 + .../tests/tests/test_task_agent_impl.rs | 3 +- src/infra/core/Cargo.toml | 1 + .../src/repos/dataset_repository_helpers.rs | 8 +-- .../src/repos/dataset_repository_local_fs.rs | 11 +++- .../core/src/repos/dataset_repository_s3.rs | 11 +++- .../src/services/provenance_service_impl.rs | 2 +- .../core/src/services/query_service_impl.rs | 2 +- .../core/src/testing/base_repo_harness.rs | 39 ++++++++++---- .../core/src/testing/metadata_factory.rs | 7 +++ .../testing/mock_dataset_action_authorizer.rs | 30 ++++++----- .../commit_dataset_event_use_case_impl.rs | 2 +- .../compact_dataset_use_case_impl.rs | 2 +- .../use_cases/delete_dataset_use_case_impl.rs | 2 +- .../use_cases/rename_dataset_use_case_impl.rs | 2 +- .../use_cases/reset_dataset_use_case_impl.rs | 2 +- .../use_cases/set_watermark_use_case_impl.rs | 2 +- .../use_cases/verify_dataset_use_case_impl.rs | 4 +- .../core/tests/tests/engine/test_engine_io.rs | 2 + .../tests/engine/test_engine_transform.rs | 1 + .../tests/tests/ingest/test_polling_ingest.rs | 1 + .../tests/tests/ingest/test_push_ingest.rs | 1 + .../core/tests/tests/ingest/test_writer.rs | 1 + .../repos/test_dataset_repository_local_fs.rs | 3 +- .../tests/repos/test_dataset_repository_s3.rs | 8 ++- .../tests/test_compaction_services_impl.rs | 2 + .../test_dataset_changes_service_impl.rs | 2 +- .../tests/tests/test_datasets_filtering.rs | 2 +- .../tests/test_pull_request_planner_impl.rs | 3 +- .../tests/test_push_request_planner_impl.rs | 2 +- .../tests/tests/test_query_service_impl.rs | 2 + .../tests/tests/test_remote_status_service.rs | 2 +- .../tests/tests/test_reset_services_impl.rs | 2 +- .../tests/tests/test_search_service_impl.rs | 1 + .../tests/tests/test_sync_service_impl.rs | 2 + .../tests/test_transform_services_impl.rs | 1 + .../tests/test_verification_service_impl.rs | 2 +- .../tests/test_watermark_services_impl.rs | 2 +- .../tests/use_cases/base_use_case_harness.rs | 15 +++++- .../test_commit_dataset_event_use_case.rs | 37 ++++++++++---- .../test_compact_dataset_use_case.rs | 33 ++++++++---- .../use_cases/test_delete_dataset_use_case.rs | 41 +++++++++++---- .../use_cases/test_rename_dataset_use_case.rs | 28 +++++++--- .../use_cases/test_reset_dataset_use_case.rs | 17 +++++-- .../use_cases/test_set_watermark_use_case.rs | 17 +++++-- .../use_cases/test_verify_dataset_use_case.rs | 38 ++++++++++---- 80 files changed, 440 insertions(+), 162 deletions(-) create mode 100644 src/domain/core/src/services/did_generator.rs diff --git a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs index a27deea442..4819b6b757 100644 --- a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs +++ b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs @@ -61,10 +61,8 @@ impl OsoDatasetAuthorizer { async fn dataset_resource( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, ) -> Result { - let dataset_id = &dataset_handle.id; - let dataset_resource = self .oso_resource_service .dataset_resource(dataset_id) @@ -86,14 +84,14 @@ impl OsoDatasetAuthorizer { #[async_trait::async_trait] impl DatasetActionAuthorizer for OsoDatasetAuthorizer { - #[tracing::instrument(level = "debug", skip_all, fields(%dataset_handle, ?action))] + #[tracing::instrument(level = "debug", skip_all, fields(%dataset_id, ?action))] async fn check_action_allowed( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError> { let (user_actor, dataset_resource) = - try_join!(self.user_actor(), self.dataset_resource(dataset_handle))?; + try_join!(self.user_actor(), self.dataset_resource(dataset_id))?; match self .kamu_auth_oso @@ -104,7 +102,7 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer { AccessError::Forbidden( DatasetActionNotEnoughPermissionsError { action, - dataset_ref: dataset_handle.as_local_ref(), + dataset_ref: dataset_id.as_local_ref(), } .into(), ), @@ -119,7 +117,7 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer { dataset_handle: &odf::DatasetHandle, ) -> Result, InternalError> { let (user_actor, dataset_resource) = - try_join!(self.user_actor(), self.dataset_resource(dataset_handle))?; + try_join!(self.user_actor(), self.dataset_resource(&dataset_handle.id))?; self.kamu_auth_oso .get_allowed_actions(user_actor, dataset_resource) diff --git a/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs index b412b67719..c527230b6c 100644 --- a/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs +++ b/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs @@ -54,12 +54,12 @@ async fn test_owner_can_read_and_write_private_dataset() { let read_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, DatasetAction::Read) .await; let write_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await; let allowed_actions = harness @@ -87,12 +87,12 @@ async fn test_guest_can_read_but_not_write_public_dataset() { let read_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, DatasetAction::Read) .await; let write_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await; let allowed_actions = harness diff --git a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs index d36f75c6ed..28aec4bb41 100644 --- a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs +++ b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs @@ -179,7 +179,7 @@ impl DatasetMetadata { Ok(upstream_dependencies) } - // TODO: Convert to collection + // TODO: Convert to connection (page_based_connection!) // TODO: Private Datasets: tests /// Current downstream dependencies of a dataset async fn current_downstream_dependencies( diff --git a/src/adapter/graphql/src/utils.rs b/src/adapter/graphql/src/utils.rs index abf34168e3..394434e9d1 100644 --- a/src/adapter/graphql/src/utils.rs +++ b/src/adapter/graphql/src/utils.rs @@ -11,7 +11,7 @@ use async_graphql::{Context, ErrorExtensions}; use internal_error::*; use kamu_accounts::{CurrentAccountSubject, GetAccessTokenError, LoggedAccount}; use kamu_core::auth::DatasetActionUnauthorizedError; -use kamu_core::{DatasetRegistry, ResolvedDataset}; +use kamu_core::{auth, DatasetRegistry, ResolvedDataset}; use kamu_datasets::DatasetEnvVarsConfig; use {kamu_task_system as ts, opendatafabric as odf}; @@ -109,32 +109,27 @@ pub(crate) async fn check_dataset_read_access( ctx: &Context<'_>, dataset_handle: &odf::DatasetHandle, ) -> Result<(), GqlError> { - let dataset_action_authorizer = - from_catalog_n!(ctx, dyn kamu_core::auth::DatasetActionAuthorizer); - - dataset_action_authorizer - .check_action_allowed(dataset_handle, kamu_core::auth::DatasetAction::Read) - .await - .map_err(|e| match e { - DatasetActionUnauthorizedError::Access(_) => GqlError::Gql( - async_graphql::Error::new("Dataset access error") - .extend_with(|_, eev| eev.set("alias", dataset_handle.alias.to_string())), - ), - DatasetActionUnauthorizedError::Internal(e) => GqlError::Internal(e), - })?; - - Ok(()) + check_dataset_access(ctx, dataset_handle, auth::DatasetAction::Read).await } pub(crate) async fn check_dataset_write_access( ctx: &Context<'_>, dataset_handle: &odf::DatasetHandle, ) -> Result<(), GqlError> { - let dataset_action_authorizer = - from_catalog_n!(ctx, dyn kamu_core::auth::DatasetActionAuthorizer); + check_dataset_access(ctx, dataset_handle, auth::DatasetAction::Write).await +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +async fn check_dataset_access( + ctx: &Context<'_>, + dataset_handle: &odf::DatasetHandle, + action: auth::DatasetAction, +) -> Result<(), GqlError> { + let dataset_action_authorizer = from_catalog_n!(ctx, dyn auth::DatasetActionAuthorizer); dataset_action_authorizer - .check_action_allowed(dataset_handle, kamu_core::auth::DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, action) .await .map_err(|e| match e { DatasetActionUnauthorizedError::Access(_) => make_dataset_access_error(dataset_handle), diff --git a/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs b/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs index 97e4e7849b..17a2778d95 100644 --- a/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs +++ b/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs @@ -641,6 +641,7 @@ impl FlowTriggerHarness { .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), ) .bind::() + .add::() .add_value(TenancyConfig::MultiTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) .bind::() diff --git a/src/adapter/graphql/tests/tests/test_gql_data.rs b/src/adapter/graphql/tests/tests/test_gql_data.rs index 45fafe0f85..833f668cb6 100644 --- a/src/adapter/graphql/tests/tests/test_gql_data.rs +++ b/src/adapter/graphql/tests/tests/test_gql_data.rs @@ -59,6 +59,7 @@ async fn create_catalog_with_local_workspace( let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add::() .add_value(current_account_subject) .add_value(predefined_accounts_config) diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs index fccdd54487..daaf90662a 100644 --- a/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs +++ b/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs @@ -23,6 +23,7 @@ use kamu_core::{ CreateDatasetFromSnapshotUseCase, CreateDatasetResult, DatasetRepository, + DidGeneratorDefault, TenancyConfig, }; use kamu_datasets::DatasetEnvVarsConfig; @@ -336,6 +337,7 @@ impl DatasetEnvVarsHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_value(DatasetEnvVarsConfig::sample()) .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs index 48cd92a9c7..e6b630e9e2 100644 --- a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs +++ b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs @@ -24,6 +24,7 @@ use kamu_core::{ CreateDatasetFromSnapshotUseCase, CreateDatasetResult, DatasetRepository, + DidGeneratorDefault, TenancyConfig, }; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; @@ -581,6 +582,7 @@ impl FlowConfigHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) .bind::() diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs index 354123be1b..05ed69c0b9 100644 --- a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs +++ b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs @@ -40,6 +40,7 @@ use kamu_core::{ DatasetIntervalIncrement, DatasetLifecycleMessage, DatasetRepository, + DidGeneratorDefault, PullResult, ResetResult, TenancyConfig, @@ -3113,6 +3114,7 @@ impl FlowRunsHarness { .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), ) .bind::() + .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) .bind::() diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs index 2713f5b521..851065f88f 100644 --- a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs +++ b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs @@ -24,6 +24,7 @@ use kamu_core::{ CreateDatasetFromSnapshotUseCase, CreateDatasetResult, DatasetRepository, + DidGeneratorDefault, TenancyConfig, }; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; @@ -1063,6 +1064,7 @@ impl FlowTriggerHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) .bind::() diff --git a/src/adapter/graphql/tests/tests/test_gql_datasets.rs b/src/adapter/graphql/tests/tests/test_gql_datasets.rs index 86b59c86dc..6b4c6fbb4d 100644 --- a/src/adapter/graphql/tests/tests/test_gql_datasets.rs +++ b/src/adapter/graphql/tests/tests/test_gql_datasets.rs @@ -805,6 +805,7 @@ impl GraphQLDatasetsHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_builder( OutboxImmediateImpl::builder() .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), diff --git a/src/adapter/graphql/tests/tests/test_gql_metadata.rs b/src/adapter/graphql/tests/tests/test_gql_metadata.rs index 10f96732e8..8075602a9f 100644 --- a/src/adapter/graphql/tests/tests/test_gql_metadata.rs +++ b/src/adapter/graphql/tests/tests/test_gql_metadata.rs @@ -34,6 +34,7 @@ async fn test_current_push_sources() { let mut b = CatalogBuilder::new(); b.add_value(RunInfoDir::new(tempdir.path().join("run"))) + .add::() .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) diff --git a/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs b/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs index 4e459b6489..52da51de9d 100644 --- a/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs +++ b/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs @@ -529,6 +529,7 @@ impl GraphQLMetadataChainHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add::() .add::() .add::() diff --git a/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs b/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs index e925424687..206d726f6f 100644 --- a/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs +++ b/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs @@ -143,6 +143,7 @@ impl PushStatusesTestHarness { let mut b = CatalogBuilder::new(); b.add_value(RunInfoDir::new(tempdir.path().join("run"))) + .add::() .add::() .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) .bind::() diff --git a/src/adapter/graphql/tests/tests/test_gql_search.rs b/src/adapter/graphql/tests/tests/test_gql_search.rs index 9b59fa1f1d..065d34da24 100644 --- a/src/adapter/graphql/tests/tests/test_gql_search.rs +++ b/src/adapter/graphql/tests/tests/test_gql_search.rs @@ -28,6 +28,7 @@ async fn test_search_query() { std::fs::create_dir(&datasets_dir).unwrap(); let cat = dill::CatalogBuilder::new() + .add::() .add::() .add::() .add::() diff --git a/src/adapter/http/Cargo.toml b/src/adapter/http/Cargo.toml index a54184f270..2f0ce55c18 100644 --- a/src/adapter/http/Cargo.toml +++ b/src/adapter/http/Cargo.toml @@ -104,6 +104,7 @@ http-body-util = { optional = true, version = "0.1" } container-runtime = { workspace = true } init-on-startup = { workspace = true } kamu = { workspace = true, features = ["testing"] } +kamu-core = { workspace = true, features = ["testing"] } kamu-accounts = { workspace = true, features = ["testing"] } kamu-accounts-inmem = { workspace = true } kamu-accounts-services = { workspace = true } diff --git a/src/adapter/http/src/data/ingest_handler.rs b/src/adapter/http/src/data/ingest_handler.rs index 59a8c9cd88..239aa741bf 100644 --- a/src/adapter/http/src/data/ingest_handler.rs +++ b/src/adapter/http/src/data/ingest_handler.rs @@ -131,7 +131,7 @@ pub async fn dataset_ingest_handler( // Authorization check let authorizer = catalog.get_one::().unwrap(); authorizer - .check_action_allowed(target.get_handle(), auth::DatasetAction::Write) + .check_action_allowed(&target.get_handle().id, auth::DatasetAction::Write) .await .map_err(|e| match e { DatasetActionUnauthorizedError::Access(_) => ApiError::new_forbidden(), diff --git a/src/adapter/http/src/middleware/dataset_authorization_layer.rs b/src/adapter/http/src/middleware/dataset_authorization_layer.rs index e8f9534d11..f497461435 100644 --- a/src/adapter/http/src/middleware/dataset_authorization_layer.rs +++ b/src/adapter/http/src/middleware/dataset_authorization_layer.rs @@ -131,7 +131,7 @@ where { Ok(dataset_handle) => { if let Err(err) = dataset_action_authorizer - .check_action_allowed(&dataset_handle, action) + .check_action_allowed(&dataset_handle.id, action) .await { if let Err(err_result) = Self::check_logged_in(catalog) { diff --git a/src/adapter/http/tests/harness/client_side_harness.rs b/src/adapter/http/tests/harness/client_side_harness.rs index 2719ffa35c..fce481f8b8 100644 --- a/src/adapter/http/tests/harness/client_side_harness.rs +++ b/src/adapter/http/tests/harness/client_side_harness.rs @@ -75,6 +75,8 @@ impl ClientSideHarness { let mut b = dill::CatalogBuilder::new(); + b.add::(); + b.add_value(RunInfoDir::new(run_info_dir)); b.add_value(CacheDir::new(cache_dir)); b.add_value(RemoteReposDir::new(repos_dir)); diff --git a/src/adapter/http/tests/harness/server_side_harness.rs b/src/adapter/http/tests/harness/server_side_harness.rs index 0bec16e831..42377464a5 100644 --- a/src/adapter/http/tests/harness/server_side_harness.rs +++ b/src/adapter/http/tests/harness/server_side_harness.rs @@ -130,10 +130,10 @@ pub(crate) fn create_web_user_catalog( let mut mock_dataset_action_authorizer = MockDatasetActionAuthorizer::new(); mock_dataset_action_authorizer .expect_check_action_allowed() - .returning(|dataset_handle, action| { + .returning(|dataset_id, action| { if action == DatasetAction::Write { Err(MockDatasetActionAuthorizer::denying_error( - dataset_handle, + dataset_id.as_local_ref(), action, )) } else { diff --git a/src/adapter/http/tests/harness/server_side_local_fs_harness.rs b/src/adapter/http/tests/harness/server_side_local_fs_harness.rs index cdb620211d..936c487348 100644 --- a/src/adapter/http/tests/harness/server_side_local_fs_harness.rs +++ b/src/adapter/http/tests/harness/server_side_local_fs_harness.rs @@ -40,7 +40,13 @@ use kamu::{ }; use kamu_accounts::testing::MockAuthenticationService; use kamu_accounts::{Account, AuthenticationService}; -use kamu_core::{CompactionExecutor, CompactionPlanner, DatasetRegistry, TenancyConfig}; +use kamu_core::{ + CompactionExecutor, + CompactionPlanner, + DatasetRegistry, + DidGeneratorDefault, + TenancyConfig, +}; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; use kamu_datasets_services::DependencyGraphServiceImpl; use messaging_outbox::DummyOutboxImpl; @@ -99,6 +105,7 @@ impl ServerSideLocalFsHarness { let base_url_rest = format!("http://{}", listener.local_addr().unwrap()); b.add_value(RunInfoDir::new(run_info_dir)) + .add::() .add_value(CacheDir::new(cache_dir)) .add::() .add_value(time_source.clone()) diff --git a/src/adapter/http/tests/harness/server_side_s3_harness.rs b/src/adapter/http/tests/harness/server_side_s3_harness.rs index 4d0f7f2b49..f29d009583 100644 --- a/src/adapter/http/tests/harness/server_side_s3_harness.rs +++ b/src/adapter/http/tests/harness/server_side_s3_harness.rs @@ -44,7 +44,7 @@ use kamu::{ }; use kamu_accounts::testing::MockAuthenticationService; use kamu_accounts::{Account, AuthenticationService}; -use kamu_core::{DatasetRegistry, TenancyConfig}; +use kamu_core::{DatasetRegistry, DidGeneratorDefault, TenancyConfig}; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; use kamu_datasets_services::DependencyGraphServiceImpl; use messaging_outbox::DummyOutboxImpl; @@ -97,6 +97,7 @@ impl ServerSideS3Harness { let mut b = dill::CatalogBuilder::new(); b.add_value(time_source.clone()) + .add::() .add_value(RunInfoDir::new(run_info_dir)) .bind::() .add::() diff --git a/src/adapter/http/tests/tests/test_dataset_authorization_layer.rs b/src/adapter/http/tests/tests/test_dataset_authorization_layer.rs index 285ebbdda9..eb205e41b0 100644 --- a/src/adapter/http/tests/tests/test_dataset_authorization_layer.rs +++ b/src/adapter/http/tests/tests/test_dataset_authorization_layer.rs @@ -24,12 +24,13 @@ use kamu::{ }; use kamu_accounts::testing::MockAuthenticationService; use kamu_accounts::*; -use kamu_core::TenancyConfig; +use kamu_core::{DidGenerator, MockDidGenerator, TenancyConfig}; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; use kamu_datasets_services::DependencyGraphServiceImpl; use messaging_outbox::DummyOutboxImpl; use mockall::predicate::{eq, function}; -use opendatafabric::{DatasetAlias, DatasetHandle, DatasetKind, DatasetName, DatasetRef}; +use opendatafabric as odf; +use opendatafabric::{DatasetAlias, DatasetKind, DatasetName}; use time_source::SystemTimeSourceDefault; use url::Url; @@ -217,10 +218,15 @@ impl ServerHarness { let datasets_dir = temp_dir.path().join("datasets"); std::fs::create_dir(&datasets_dir).unwrap(); + let dataset_id = Self::dataset_id(); let base_catalog = { let mut b = dill::CatalogBuilder::new(); b.add::() + .add_value(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id.clone() + ])) + .bind::() .add::() .add::() .add::() @@ -257,8 +263,12 @@ impl ServerHarness { create_dataset .execute( &Self::dataset_alias(), - MetadataFactory::metadata_block(MetadataFactory::seed(DatasetKind::Root).build()) - .build_typed(), + MetadataFactory::metadata_block( + MetadataFactory::seed(DatasetKind::Root) + .id(dataset_id.clone()) + .build(), + ) + .build_typed(), Default::default(), ) .await @@ -281,7 +291,7 @@ impl ServerHarness { .layer( tower::ServiceBuilder::new() .layer(axum::Extension(catalog_test)) - .layer(axum::Extension(DatasetRef::from_str("mydataset").unwrap())) + .layer(axum::Extension(dataset_id.as_local_ref())) .layer(kamu_adapter_http::DatasetAuthorizationLayer::new( |request| { if !request.method().is_safe() || request.uri().path() == "/bar" { @@ -314,14 +324,17 @@ impl ServerHarness { mock_dataset_action_authorizer .expect_check_action_allowed() .with( - function(|dh: &DatasetHandle| dh.alias == ServerHarness::dataset_alias()), + function(|dataset_id: &odf::DatasetID| *dataset_id == ServerHarness::dataset_id()), eq(action), ) - .returning(move |dh, action| { + .returning(move |dataset_id, action| { if should_authorize { Ok(()) } else { - Err(MockDatasetActionAuthorizer::denying_error(dh, action)) + Err(MockDatasetActionAuthorizer::denying_error( + dataset_id.as_local_ref(), + action, + )) } }); @@ -354,7 +367,7 @@ impl ServerHarness { .await .unwrap(); - assert_eq!(response.status(), expected_status); + assert_eq!(expected_status, response.status()); } pub fn test_url(&self, path: &str) -> Url { @@ -371,7 +384,11 @@ impl ServerHarness { http::StatusCode::OK } - fn dataset_alias() -> DatasetAlias { + fn dataset_alias() -> odf::DatasetAlias { DatasetAlias::new(None, DatasetName::new_unchecked("mydataset")) } + + fn dataset_id() -> odf::DatasetID { + odf::DatasetID::new_seeded_ed25519(Self::dataset_alias().dataset_name.as_bytes()) + } } diff --git a/src/adapter/http/tests/tests/test_routing.rs b/src/adapter/http/tests/tests/test_routing.rs index d0e84a04b3..23cced6503 100644 --- a/src/adapter/http/tests/tests/test_routing.rs +++ b/src/adapter/http/tests/tests/test_routing.rs @@ -43,6 +43,7 @@ async fn setup_repo() -> RepoFixture { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add::() .add::() .add::() diff --git a/src/adapter/odata/tests/tests/test_handlers.rs b/src/adapter/odata/tests/tests/test_handlers.rs index 079bd079c1..7bc80ad72a 100644 --- a/src/adapter/odata/tests/tests/test_handlers.rs +++ b/src/adapter/odata/tests/tests/test_handlers.rs @@ -340,6 +340,7 @@ impl TestHarness { let mut b = dill::CatalogBuilder::new(); b.add_value(RunInfoDir::new(run_info_dir)) + .add::() .add_value(CacheDir::new(cache_dir)) .add::() .add::() diff --git a/src/app/cli/src/app.rs b/src/app/cli/src/app.rs index 0707892fb0..4203abca9b 100644 --- a/src/app/cli/src/app.rs +++ b/src/app/cli/src/app.rs @@ -386,6 +386,8 @@ pub fn configure_base_catalog( b.add::(); } + b.add::(); + b.add_builder( DatasetRepositoryLocalFs::builder().with_root(workspace_layout.datasets_dir.clone()), ); diff --git a/src/app/cli/src/commands/inspect_query_command.rs b/src/app/cli/src/commands/inspect_query_command.rs index 9c25d875e0..86feaa3979 100644 --- a/src/app/cli/src/commands/inspect_query_command.rs +++ b/src/app/cli/src/commands/inspect_query_command.rs @@ -179,7 +179,7 @@ impl Command for InspectQueryCommand { .await?; self.dataset_action_authorizer - .check_action_allowed(&dataset_handle, auth::DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, auth::DatasetAction::Read) .await .map_err(|e| match e { auth::DatasetActionUnauthorizedError::Access(e) => CLIError::failure(e), diff --git a/src/app/cli/src/commands/log_command.rs b/src/app/cli/src/commands/log_command.rs index 28ba2399e3..b88b8772a8 100644 --- a/src/app/cli/src/commands/log_command.rs +++ b/src/app/cli/src/commands/log_command.rs @@ -122,7 +122,7 @@ impl Command for LogCommand { .await?; self.dataset_action_authorizer - .check_action_allowed(&dataset_handle, auth::DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, auth::DatasetAction::Read) .await .map_err(|e| match e { auth::DatasetActionUnauthorizedError::Access(e) => CLIError::failure(e), diff --git a/src/domain/core/src/auth/dataset_action_authorizer.rs b/src/domain/core/src/auth/dataset_action_authorizer.rs index 4edda84349..05aa72ab10 100644 --- a/src/domain/core/src/auth/dataset_action_authorizer.rs +++ b/src/domain/core/src/auth/dataset_action_authorizer.rs @@ -24,7 +24,7 @@ pub trait DatasetActionAuthorizer: Sync + Send { async fn check_action_allowed( &self, // TODO: Private Datasets: migrate to use odf::DatasetID, here and below - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError>; @@ -33,7 +33,7 @@ pub trait DatasetActionAuthorizer: Sync + Send { dataset_handle: &odf::DatasetHandle, action: DatasetAction, ) -> Result { - match self.check_action_allowed(dataset_handle, action).await { + match self.check_action_allowed(&dataset_handle.id, action).await { Ok(()) => Ok(true), Err(DatasetActionUnauthorizedError::Access(_)) => Ok(false), Err(DatasetActionUnauthorizedError::Internal(err)) => Err(err), @@ -228,7 +228,7 @@ impl AlwaysHappyDatasetActionAuthorizer { impl DatasetActionAuthorizer for AlwaysHappyDatasetActionAuthorizer { async fn check_action_allowed( &self, - _dataset_handle: &odf::DatasetHandle, + _dataset_id: &odf::DatasetID, _action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError> { // Ignore rules diff --git a/src/domain/core/src/services/did_generator.rs b/src/domain/core/src/services/did_generator.rs new file mode 100644 index 0000000000..330e87f26a --- /dev/null +++ b/src/domain/core/src/services/did_generator.rs @@ -0,0 +1,51 @@ +// 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. + +use dill::{component, interface}; +use opendatafabric as odf; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[cfg_attr(any(feature = "testing", test), mockall::automock)] +pub trait DidGenerator: Send + Sync { + fn generate_dataset_id(&self) -> odf::DatasetID; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[component(pub)] +#[interface(dyn DidGenerator)] +pub struct DidGeneratorDefault; + +impl DidGenerator for DidGeneratorDefault { + fn generate_dataset_id(&self) -> odf::DatasetID { + let (_, dataset_id) = odf::DatasetID::new_generated_ed25519(); + dataset_id + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[cfg(any(feature = "testing", test))] +impl MockDidGenerator { + pub fn predefined_dataset_ids(dataset_ids: Vec) -> Self { + let mut mock = Self::default(); + + let dataset_ids_count = dataset_ids.len(); + let mut dataset_ids_it = dataset_ids.into_iter(); + + mock.expect_generate_dataset_id() + .times(dataset_ids_count) + .returning(move || dataset_ids_it.next().unwrap()); + + mock + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/core/src/services/mod.rs b/src/domain/core/src/services/mod.rs index 0be61469e9..94ecb0e6c7 100644 --- a/src/domain/core/src/services/mod.rs +++ b/src/domain/core/src/services/mod.rs @@ -25,6 +25,7 @@ pub use watermark::*; pub mod dataset_changes_service; pub mod dataset_registry; pub mod dependency_graph_service; +mod did_generator; pub mod engine_provisioner; pub mod export_service; pub mod metadata_query_service; @@ -45,6 +46,7 @@ pub mod verification_service; pub use dataset_changes_service::*; pub use dataset_registry::*; pub use dependency_graph_service::*; +pub use did_generator::*; pub use engine_provisioner::*; pub use export_service::*; pub use metadata_query_service::*; diff --git a/src/domain/datasets/services/tests/tests/test_dependency_graph_service_impl.rs b/src/domain/datasets/services/tests/tests/test_dependency_graph_service_impl.rs index a7c8bac44f..379a026255 100644 --- a/src/domain/datasets/services/tests/tests/test_dependency_graph_service_impl.rs +++ b/src/domain/datasets/services/tests/tests/test_dependency_graph_service_impl.rs @@ -616,7 +616,7 @@ struct DependencyGraphHarness { impl DependencyGraphHarness { fn new(tenancy_config: TenancyConfig) -> Self { - let base_repo_harness = BaseRepoHarness::new(tenancy_config); + let base_repo_harness = BaseRepoHarness::new(tenancy_config, None); let mut b = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()); b.add_builder( diff --git a/src/domain/flow-system/services/tests/tests/test_flow_configuration_service_impl.rs b/src/domain/flow-system/services/tests/tests/test_flow_configuration_service_impl.rs index 437e2cbb89..1bb3b273d4 100644 --- a/src/domain/flow-system/services/tests/tests/test_flow_configuration_service_impl.rs +++ b/src/domain/flow-system/services/tests/tests/test_flow_configuration_service_impl.rs @@ -217,6 +217,7 @@ impl FlowConfigurationHarness { .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), ) .bind::() + .add::() .add::() .add::() .add::() diff --git a/src/domain/flow-system/services/tests/tests/test_flow_trigger_service_impl.rs b/src/domain/flow-system/services/tests/tests/test_flow_trigger_service_impl.rs index b2311aef71..8f11e81001 100644 --- a/src/domain/flow-system/services/tests/tests/test_flow_trigger_service_impl.rs +++ b/src/domain/flow-system/services/tests/tests/test_flow_trigger_service_impl.rs @@ -372,6 +372,7 @@ impl FlowTriggerHarness { .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), ) .bind::() + .add::() .add::() .add::() .add::() diff --git a/src/domain/flow-system/services/tests/tests/utils/flow_harness_shared.rs b/src/domain/flow-system/services/tests/tests/utils/flow_harness_shared.rs index 1340099288..82fa116ed0 100644 --- a/src/domain/flow-system/services/tests/tests/utils/flow_harness_shared.rs +++ b/src/domain/flow-system/services/tests/tests/utils/flow_harness_shared.rs @@ -145,6 +145,7 @@ impl FlowHarness { .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), ) .bind::() + .add::() .add::() .add_value(FlowAgentConfig::new( awaiting_step, diff --git a/src/domain/task-system/services/tests/tests/test_task_agent_impl.rs b/src/domain/task-system/services/tests/tests/test_task_agent_impl.rs index 63b1d52a9a..1be3aed5e4 100644 --- a/src/domain/task-system/services/tests/tests/test_task_agent_impl.rs +++ b/src/domain/task-system/services/tests/tests/test_task_agent_impl.rs @@ -16,7 +16,7 @@ use kamu::utils::ipfs_wrapper::IpfsClient; use kamu::*; use kamu_accounts::CurrentAccountSubject; use kamu_core::auth::DummyOdfServerAccessTokenResolver; -use kamu_core::{DatasetRepository, TenancyConfig}; +use kamu_core::{DatasetRepository, DidGeneratorDefault, TenancyConfig}; use kamu_datasets::DatasetEnvVarsConfig; use kamu_datasets_inmem::InMemoryDatasetEnvVarRepository; use kamu_datasets_services::DatasetEnvVarServiceImpl; @@ -162,6 +162,7 @@ impl TaskAgentHarness { let mut b = CatalogBuilder::new(); b.add::() + .add::() .add::() .add::() .add::() diff --git a/src/infra/core/Cargo.toml b/src/infra/core/Cargo.toml index 928d58ad8e..8116575abe 100644 --- a/src/infra/core/Cargo.toml +++ b/src/infra/core/Cargo.toml @@ -149,6 +149,7 @@ libc = "0.2" # For getting uid:gid [dev-dependencies] kamu = { workspace = true, features = ["testing"] } +kamu-core = { workspace = true, features = ["testing"] } kamu-data-utils = { workspace = true, features = ["testing"] } kamu-datasets-services = { workspace = true } kamu-datasets-inmem = { workspace = true } diff --git a/src/infra/core/src/repos/dataset_repository_helpers.rs b/src/infra/core/src/repos/dataset_repository_helpers.rs index 6f2ec8c624..bf92ed784f 100644 --- a/src/infra/core/src/repos/dataset_repository_helpers.rs +++ b/src/infra/core/src/repos/dataset_repository_helpers.rs @@ -30,6 +30,7 @@ pub(crate) async fn create_dataset_from_snapshot_impl< dataset_repo: &TRepository, mut snapshot: DatasetSnapshot, system_time: DateTime, + new_dataset_id: DatasetID, ) -> Result { // Validate / resolve events for event in &mut snapshot.metadata { @@ -98,11 +99,6 @@ pub(crate) async fn create_dataset_from_snapshot_impl< }?; } - // We are generating a key pair and deriving a dataset ID from it. - // The key pair is discarded for now, but in future can be used for - // proof of control over dataset and metadata signing. - let (_keypair, dataset_id) = DatasetID::new_generated_ed25519(); - let create_result = dataset_repo .create_dataset( &snapshot.name, @@ -110,7 +106,7 @@ pub(crate) async fn create_dataset_from_snapshot_impl< system_time, prev_block_hash: None, event: Seed { - dataset_id, + dataset_id: new_dataset_id, dataset_kind: snapshot.kind, }, sequence_number: 0, diff --git a/src/infra/core/src/repos/dataset_repository_local_fs.rs b/src/infra/core/src/repos/dataset_repository_local_fs.rs index 8d053ee9ff..65d2a8e6ca 100644 --- a/src/infra/core/src/repos/dataset_repository_local_fs.rs +++ b/src/infra/core/src/repos/dataset_repository_local_fs.rs @@ -27,6 +27,7 @@ pub struct DatasetRepositoryLocalFs { storage_strategy: Box, thrash_lock: tokio::sync::Mutex<()>, system_time_source: Arc, + did_generator: Arc, } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -39,6 +40,7 @@ impl DatasetRepositoryLocalFs { current_account_subject: Arc, tenancy_config: Arc, system_time_source: Arc, + did_generator: Arc, ) -> Self { Self { storage_strategy: match *tenancy_config { @@ -52,6 +54,7 @@ impl DatasetRepositoryLocalFs { }, thrash_lock: tokio::sync::Mutex::new(()), system_time_source, + did_generator, } } @@ -278,7 +281,13 @@ impl DatasetRepositoryWriter for DatasetRepositoryLocalFs { &self, snapshot: DatasetSnapshot, ) -> Result { - create_dataset_from_snapshot_impl(self, snapshot, self.system_time_source.now()).await + create_dataset_from_snapshot_impl( + self, + snapshot, + self.system_time_source.now(), + self.did_generator.generate_dataset_id(), + ) + .await } #[tracing::instrument(level = "debug", skip_all, fields(%dataset_handle, %new_name))] diff --git a/src/infra/core/src/repos/dataset_repository_s3.rs b/src/infra/core/src/repos/dataset_repository_s3.rs index 080b008b93..7cd89ba301 100644 --- a/src/infra/core/src/repos/dataset_repository_s3.rs +++ b/src/infra/core/src/repos/dataset_repository_s3.rs @@ -32,6 +32,7 @@ pub struct DatasetRepositoryS3 { registry_cache: Option>, metadata_cache_local_fs_path: Option>, system_time_source: Arc, + did_generator: Arc, } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -54,6 +55,7 @@ impl DatasetRepositoryS3 { registry_cache: Option>, metadata_cache_local_fs_path: Option>, system_time_source: Arc, + did_generator: Arc, ) -> Self { Self { s3_context, @@ -62,6 +64,7 @@ impl DatasetRepositoryS3 { registry_cache, metadata_cache_local_fs_path, system_time_source, + did_generator, } } @@ -417,7 +420,13 @@ impl DatasetRepositoryWriter for DatasetRepositoryS3 { &self, snapshot: DatasetSnapshot, ) -> Result { - create_dataset_from_snapshot_impl(self, snapshot, self.system_time_source.now()).await + create_dataset_from_snapshot_impl( + self, + snapshot, + self.system_time_source.now(), + self.did_generator.generate_dataset_id(), + ) + .await } #[tracing::instrument(level = "debug", skip_all, fields(%dataset_handle, %new_name))] diff --git a/src/infra/core/src/services/provenance_service_impl.rs b/src/infra/core/src/services/provenance_service_impl.rs index e1963dd0d6..7a3e22c34d 100644 --- a/src/infra/core/src/services/provenance_service_impl.rs +++ b/src/infra/core/src/services/provenance_service_impl.rs @@ -44,7 +44,7 @@ impl ProvenanceServiceImpl { visitor: &mut dyn LineageVisitor, ) -> Result<(), GetLineageError> { self.dataset_action_authorizer - .check_action_allowed(dataset_handle, auth::DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, auth::DatasetAction::Read) .await?; let resolved_dataset = self.dataset_registry.get_dataset_by_handle(dataset_handle); diff --git a/src/infra/core/src/services/query_service_impl.rs b/src/infra/core/src/services/query_service_impl.rs index 6269828ab1..c6ad8e7b4f 100644 --- a/src/infra/core/src/services/query_service_impl.rs +++ b/src/infra/core/src/services/query_service_impl.rs @@ -320,7 +320,7 @@ impl QueryServiceImpl { .await?; self.dataset_action_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Read) + .check_action_allowed(&dataset_handle.id, DatasetAction::Read) .await?; Ok(self.dataset_registry.get_dataset_by_handle(&dataset_handle)) diff --git a/src/infra/core/src/testing/base_repo_harness.rs b/src/infra/core/src/testing/base_repo_harness.rs index 0320b42fae..30f06cce5b 100644 --- a/src/infra/core/src/testing/base_repo_harness.rs +++ b/src/infra/core/src/testing/base_repo_harness.rs @@ -17,8 +17,11 @@ use kamu_core::{ DatasetRegistry, DatasetRegistryExt, DatasetRepository, + DidGenerator, + DidGeneratorDefault, GetDatasetError, MetadataChainExt, + MockDidGenerator, ResolvedDataset, RunInfoDir, TenancyConfig, @@ -41,7 +44,10 @@ pub struct BaseRepoHarness { } impl BaseRepoHarness { - pub fn new(tenancy_config: TenancyConfig) -> Self { + pub fn new( + tenancy_config: TenancyConfig, + maybe_mock_did_generator: Option, + ) -> Self { let temp_dir = tempfile::tempdir().unwrap(); let datasets_dir = temp_dir.path().join("datasets"); @@ -50,16 +56,27 @@ impl BaseRepoHarness { let run_info_dir = temp_dir.path().join("run"); std::fs::create_dir(&run_info_dir).unwrap(); - let catalog = dill::CatalogBuilder::new() - .add_value(RunInfoDir::new(run_info_dir)) - .add_value(tenancy_config) - .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) - .bind::() - .bind::() - .add::() - .add_value(CurrentAccountSubject::new_test()) - .add::() - .build(); + let catalog = { + let mut b = dill::CatalogBuilder::new(); + + b.add_value(RunInfoDir::new(run_info_dir)) + .add_value(tenancy_config) + .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) + .bind::() + .bind::() + .add::() + .add_value(CurrentAccountSubject::new_test()) + .add::(); + + if let Some(mock_did_generator) = maybe_mock_did_generator { + b.add_value(mock_did_generator) + .bind::(); + } else { + b.add::(); + } + + b.build() + }; Self { temp_dir, diff --git a/src/infra/core/src/testing/metadata_factory.rs b/src/infra/core/src/testing/metadata_factory.rs index b65768497c..7879ee8412 100644 --- a/src/infra/core/src/testing/metadata_factory.rs +++ b/src/infra/core/src/testing/metadata_factory.rs @@ -14,6 +14,8 @@ use opendatafabric::*; use super::IDFactory; +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + pub struct MetadataFactory; impl MetadataFactory { @@ -84,6 +86,11 @@ impl SeedBuilder { } } + pub fn id(mut self, dataset_id: DatasetID) -> Self { + self.v.dataset_id = dataset_id; + self + } + pub fn id_random(mut self) -> Self { self.v.dataset_id = DatasetID::new_generated_ed25519().1; self diff --git a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs index 486584ac78..da6b1cb49b 100644 --- a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs +++ b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs @@ -32,7 +32,7 @@ mockall::mock! { impl DatasetActionAuthorizer for DatasetActionAuthorizer { async fn check_action_allowed( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError>; @@ -63,13 +63,13 @@ mockall::mock! { impl MockDatasetActionAuthorizer { pub fn denying_error( - dataset_handle: &odf::DatasetHandle, + dataset_ref: odf::DatasetRef, action: DatasetAction, ) -> DatasetActionUnauthorizedError { DatasetActionUnauthorizedError::Access(AccessError::Forbidden( DatasetActionNotEnoughPermissionsError { action, - dataset_ref: dataset_handle.as_local_ref(), + dataset_ref, } .into(), )) @@ -79,7 +79,9 @@ impl MockDatasetActionAuthorizer { let mut mock_dataset_action_authorizer = MockDatasetActionAuthorizer::new(); mock_dataset_action_authorizer .expect_check_action_allowed() - .returning(|dataset_handle, action| Err(Self::denying_error(dataset_handle, action))); + .returning(|dataset_id, action| { + Err(Self::denying_error(dataset_id.as_local_ref(), action)) + }); mock_dataset_action_authorizer } @@ -93,13 +95,13 @@ impl MockDatasetActionAuthorizer { pub fn expect_check_read_dataset( self, - dataset_alias: &odf::DatasetAlias, + expected_dataset_id: &odf::DatasetID, times: usize, success: bool, ) -> Self { - let dataset_alias = dataset_alias.clone(); + let expected_dataset_id = expected_dataset_id.clone(); self.expect_check_action_allowed_internal( - function(move |dh: &odf::DatasetHandle| dh.alias == dataset_alias), + function(move |dataset_id| *dataset_id == expected_dataset_id), DatasetAction::Read, times, success, @@ -108,13 +110,13 @@ impl MockDatasetActionAuthorizer { pub fn expect_check_write_dataset( self, - dataset_alias: &odf::DatasetAlias, + expected_dataset_id: &odf::DatasetID, times: usize, success: bool, ) -> Self { - let dataset_alias = dataset_alias.clone(); + let expected_dataset_id = expected_dataset_id.clone(); self.expect_check_action_allowed_internal( - function(move |dh: &odf::DatasetHandle| dh.alias == dataset_alias), + function(move |dataset_id| *dataset_id == expected_dataset_id), DatasetAction::Write, times, success, @@ -137,17 +139,17 @@ impl MockDatasetActionAuthorizer { success: bool, ) -> Self where - P: Predicate + Sync + Send + 'static, + P: Predicate + Sync + Send + 'static, { if times > 0 { self.expect_check_action_allowed() .with(dataset_handle_predicate, eq(action)) .times(times) - .returning(move |hdl, action| { + .returning(move |dataset_id, action| { if success { Ok(()) } else { - Err(Self::denying_error(hdl, action)) + Err(Self::denying_error(dataset_id.as_local_ref(), action)) } }); } else { @@ -176,7 +178,7 @@ impl MockDatasetActionAuthorizer { if authorized.contains(&handle.alias) { good.push(handle); } else { - let error = Self::denying_error(&handle, action); + let error = Self::denying_error(handle.as_local_ref(), action); bad.push((handle, error)); } } diff --git a/src/infra/core/src/use_cases/commit_dataset_event_use_case_impl.rs b/src/infra/core/src/use_cases/commit_dataset_event_use_case_impl.rs index 5c4478cb62..c4f0b38924 100644 --- a/src/infra/core/src/use_cases/commit_dataset_event_use_case_impl.rs +++ b/src/infra/core/src/use_cases/commit_dataset_event_use_case_impl.rs @@ -62,7 +62,7 @@ impl CommitDatasetEventUseCase for CommitDatasetEventUseCaseImpl { opts: CommitOpts<'_>, ) -> Result { self.dataset_action_authorizer - .check_action_allowed(dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; let resolved_dataset = self.dataset_registry.get_dataset_by_handle(dataset_handle); diff --git a/src/infra/core/src/use_cases/compact_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/compact_dataset_use_case_impl.rs index 8f496e3b38..548504c75e 100644 --- a/src/infra/core/src/use_cases/compact_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/compact_dataset_use_case_impl.rs @@ -69,7 +69,7 @@ impl CompactDatasetUseCase for CompactDatasetUseCaseImpl { ) -> Result { // Permission check self.dataset_action_authorizer - .check_action_allowed(dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; // Resolve dataset diff --git a/src/infra/core/src/use_cases/delete_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/delete_dataset_use_case_impl.rs index b46cff9d49..36525b2b43 100644 --- a/src/infra/core/src/use_cases/delete_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/delete_dataset_use_case_impl.rs @@ -125,7 +125,7 @@ impl DeleteDatasetUseCase for DeleteDatasetUseCaseImpl { ) -> Result<(), DeleteDatasetError> { // Permission check self.dataset_action_authorizer - .check_action_allowed(dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; // Validate against dangling ref diff --git a/src/infra/core/src/use_cases/rename_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/rename_dataset_use_case_impl.rs index e407cbca1c..d75def94f3 100644 --- a/src/infra/core/src/use_cases/rename_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/rename_dataset_use_case_impl.rs @@ -85,7 +85,7 @@ impl RenameDatasetUseCase for RenameDatasetUseCaseImpl { }?; self.dataset_action_authorizer - .check_action_allowed(&dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; let old_name = dataset_handle.alias.dataset_name.clone(); diff --git a/src/infra/core/src/use_cases/reset_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/reset_dataset_use_case_impl.rs index 74fbb79ba1..2add2c34b3 100644 --- a/src/infra/core/src/use_cases/reset_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/reset_dataset_use_case_impl.rs @@ -64,7 +64,7 @@ impl ResetDatasetUseCase for ResetDatasetUseCaseImpl { ) -> Result { // Permission check self.dataset_action_authorizer - .check_action_allowed(dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; // Resolve dataset diff --git a/src/infra/core/src/use_cases/set_watermark_use_case_impl.rs b/src/infra/core/src/use_cases/set_watermark_use_case_impl.rs index 2195ec1ffb..1085a488ae 100644 --- a/src/infra/core/src/use_cases/set_watermark_use_case_impl.rs +++ b/src/infra/core/src/use_cases/set_watermark_use_case_impl.rs @@ -57,7 +57,7 @@ impl SetWatermarkUseCase for SetWatermarkUseCaseImpl { ) -> Result { // Permission check self.dataset_action_authorizer - .check_action_allowed(dataset_handle, DatasetAction::Write) + .check_action_allowed(&dataset_handle.id, DatasetAction::Write) .await?; // Resolve dataset diff --git a/src/infra/core/src/use_cases/verify_dataset_use_case_impl.rs b/src/infra/core/src/use_cases/verify_dataset_use_case_impl.rs index bc9d352837..4d14aa8bab 100644 --- a/src/infra/core/src/use_cases/verify_dataset_use_case_impl.rs +++ b/src/infra/core/src/use_cases/verify_dataset_use_case_impl.rs @@ -63,7 +63,7 @@ impl VerifyDatasetUseCase for VerifyDatasetUseCaseImpl { // TODO: verification of derived datasets requires read permission for inputs match self .dataset_action_authorizer - .check_action_allowed(&request.target, DatasetAction::Read) + .check_action_allowed(&request.target.id, DatasetAction::Read) .await { Ok(_) => {} @@ -104,7 +104,7 @@ impl VerifyDatasetUseCase for VerifyDatasetUseCaseImpl { for request in requests { let res = self .dataset_action_authorizer - .check_action_allowed(&request.target, DatasetAction::Read) + .check_action_allowed(&request.target.id, DatasetAction::Read) .await; match res { Ok(_) => authorized_requests.push(VerificationRequest { diff --git a/src/infra/core/tests/tests/engine/test_engine_io.rs b/src/infra/core/tests/tests/engine/test_engine_io.rs index 5489919a15..4a145dca82 100644 --- a/src/infra/core/tests/tests/engine/test_engine_io.rs +++ b/src/infra/core/tests/tests/engine/test_engine_io.rs @@ -258,6 +258,7 @@ async fn test_engine_io_local_file_mount() { std::fs::create_dir(&cache_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add::() .add::() .add::() @@ -299,6 +300,7 @@ async fn test_engine_io_s3_to_local_file_mount_proxy() { let s3_context = kamu::utils::s3_context::S3Context::from_url(&s3.url).await; let catalog = dill::CatalogBuilder::new() + .add::() .add::() .add::() .add_value(CurrentAccountSubject::new_test()) diff --git a/src/infra/core/tests/tests/engine/test_engine_transform.rs b/src/infra/core/tests/tests/engine/test_engine_transform.rs index 0b71343736..59939f72ae 100644 --- a/src/infra/core/tests/tests/engine/test_engine_transform.rs +++ b/src/infra/core/tests/tests/engine/test_engine_transform.rs @@ -236,6 +236,7 @@ impl TestHarness { std::fs::create_dir(&datasets_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_value(ContainerRuntimeConfig::default()) .add_value(RunInfoDir::new(run_info_dir)) .add_value(CacheDir::new(cache_dir)) diff --git a/src/infra/core/tests/tests/ingest/test_polling_ingest.rs b/src/infra/core/tests/tests/ingest/test_polling_ingest.rs index 29d7aee1f7..bf8ee13b0f 100644 --- a/src/infra/core/tests/tests/ingest/test_polling_ingest.rs +++ b/src/infra/core/tests/tests/ingest/test_polling_ingest.rs @@ -1219,6 +1219,7 @@ impl IngestTestHarness { std::fs::create_dir(&datasets_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_value(RunInfoDir::new(run_info_dir)) .add_value(CacheDir::new(cache_dir)) .add_value(ContainerRuntimeConfig::default()) diff --git a/src/infra/core/tests/tests/ingest/test_push_ingest.rs b/src/infra/core/tests/tests/ingest/test_push_ingest.rs index 0809c2b41c..67d625e0a1 100644 --- a/src/infra/core/tests/tests/ingest/test_push_ingest.rs +++ b/src/infra/core/tests/tests/ingest/test_push_ingest.rs @@ -783,6 +783,7 @@ impl IngestTestHarness { std::fs::create_dir(&datasets_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_value(RunInfoDir::new(run_info_dir)) .add_value(CacheDir::new(cache_dir)) .add_value(CurrentAccountSubject::new_test()) diff --git a/src/infra/core/tests/tests/ingest/test_writer.rs b/src/infra/core/tests/tests/ingest/test_writer.rs index c2aeb7492c..9fca51fb4f 100644 --- a/src/infra/core/tests/tests/ingest/test_writer.rs +++ b/src/infra/core/tests/tests/ingest/test_writer.rs @@ -1219,6 +1219,7 @@ impl Harness { let system_time = Utc.with_ymd_and_hms(2010, 1, 1, 12, 0, 0).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add::() .add_value(CurrentAccountSubject::new_test()) .add_value(TenancyConfig::SingleTenant) diff --git a/src/infra/core/tests/tests/repos/test_dataset_repository_local_fs.rs b/src/infra/core/tests/tests/repos/test_dataset_repository_local_fs.rs index 30922dc34c..8fb22cb218 100644 --- a/src/infra/core/tests/tests/repos/test_dataset_repository_local_fs.rs +++ b/src/infra/core/tests/tests/repos/test_dataset_repository_local_fs.rs @@ -13,7 +13,7 @@ use dill::Component; use domain::DatasetRepository; use kamu::*; use kamu_accounts::{CurrentAccountSubject, DEFAULT_ACCOUNT_NAME}; -use kamu_core::{CreateDatasetFromSnapshotUseCase, TenancyConfig}; +use kamu_core::{CreateDatasetFromSnapshotUseCase, DidGeneratorDefault, TenancyConfig}; use messaging_outbox::{Outbox, OutboxImmediateImpl}; use tempfile::TempDir; use time_source::SystemTimeSourceDefault; @@ -35,6 +35,7 @@ impl LocalFsRepoHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_builder( messaging_outbox::OutboxImmediateImpl::builder() .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), diff --git a/src/infra/core/tests/tests/repos/test_dataset_repository_s3.rs b/src/infra/core/tests/tests/repos/test_dataset_repository_s3.rs index 6809ebbd06..ee2297d0a9 100644 --- a/src/infra/core/tests/tests/repos/test_dataset_repository_s3.rs +++ b/src/infra/core/tests/tests/repos/test_dataset_repository_s3.rs @@ -19,7 +19,12 @@ use kamu::{ S3RegistryCache, }; use kamu_accounts::{CurrentAccountSubject, DEFAULT_ACCOUNT_NAME}; -use kamu_core::{CreateDatasetFromSnapshotUseCase, DatasetRepository, TenancyConfig}; +use kamu_core::{ + CreateDatasetFromSnapshotUseCase, + DatasetRepository, + DidGeneratorDefault, + TenancyConfig, +}; use messaging_outbox::{Outbox, OutboxImmediateImpl}; use time_source::SystemTimeSourceDefault; @@ -44,6 +49,7 @@ impl S3RepoHarness { let mut b = dill::CatalogBuilder::new(); b.add::() + .add::() .add_builder( messaging_outbox::OutboxImmediateImpl::builder() .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers), diff --git a/src/infra/core/tests/tests/test_compaction_services_impl.rs b/src/infra/core/tests/tests/test_compaction_services_impl.rs index 0e50a0fdf5..d55d2a998c 100644 --- a/src/infra/core/tests/tests/test_compaction_services_impl.rs +++ b/src/infra/core/tests/tests/test_compaction_services_impl.rs @@ -1088,6 +1088,7 @@ impl CompactTestHarness { let current_date_time = Utc.with_ymd_and_hms(2050, 1, 1, 12, 0, 0).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_value(RunInfoDir::new(run_info_dir)) .add_value(CurrentAccountSubject::new_test()) .add_value(TenancyConfig::SingleTenant) @@ -1140,6 +1141,7 @@ impl CompactTestHarness { let current_date_time = Utc.with_ymd_and_hms(2050, 1, 1, 12, 0, 0).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_builder(run_info_dir.clone()) .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryS3::builder().with_s3_context(s3_context.clone())) diff --git a/src/infra/core/tests/tests/test_dataset_changes_service_impl.rs b/src/infra/core/tests/tests/test_dataset_changes_service_impl.rs index 471ccf7fc2..1b53f095f1 100644 --- a/src/infra/core/tests/tests/test_dataset_changes_service_impl.rs +++ b/src/infra/core/tests/tests/test_dataset_changes_service_impl.rs @@ -791,7 +791,7 @@ struct DatasetChangesHarness { impl DatasetChangesHarness { fn new() -> Self { - let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant); + let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant, None); let catalog = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()) .add::() diff --git a/src/infra/core/tests/tests/test_datasets_filtering.rs b/src/infra/core/tests/tests/test_datasets_filtering.rs index 255036e801..c00d406cde 100644 --- a/src/infra/core/tests/tests/test_datasets_filtering.rs +++ b/src/infra/core/tests/tests/test_datasets_filtering.rs @@ -246,7 +246,7 @@ struct DatasetFilteringHarness { impl DatasetFilteringHarness { fn new(tenancy_config: TenancyConfig) -> Self { - let base_repo_harness = BaseRepoHarness::new(tenancy_config); + let base_repo_harness = BaseRepoHarness::new(tenancy_config, None); Self { base_repo_harness } } diff --git a/src/infra/core/tests/tests/test_pull_request_planner_impl.rs b/src/infra/core/tests/tests/test_pull_request_planner_impl.rs index 5b2b5dff95..438b5fbc91 100644 --- a/src/infra/core/tests/tests/test_pull_request_planner_impl.rs +++ b/src/infra/core/tests/tests/test_pull_request_planner_impl.rs @@ -139,6 +139,7 @@ async fn create_graph_remote( Arc::new(CurrentAccountSubject::new_test()), Arc::new(TenancyConfig::SingleTenant), Arc::new(SystemTimeSourceDefault), + Arc::new(DidGeneratorDefault), ); create_graph(&remote_dataset_repo, datasets).await; @@ -660,7 +661,7 @@ struct PullTestHarness { impl PullTestHarness { fn new(tenancy_config: TenancyConfig) -> Self { - let base_repo_harness = BaseRepoHarness::new(tenancy_config); + let base_repo_harness = BaseRepoHarness::new(tenancy_config, None); let calls = Arc::new(Mutex::new(Vec::new())); diff --git a/src/infra/core/tests/tests/test_push_request_planner_impl.rs b/src/infra/core/tests/tests/test_push_request_planner_impl.rs index af5b044f4a..72a39ecda2 100644 --- a/src/infra/core/tests/tests/test_push_request_planner_impl.rs +++ b/src/infra/core/tests/tests/test_push_request_planner_impl.rs @@ -307,7 +307,7 @@ struct RemoteRepoData { impl PushTestHarness { fn new(tenancy_config: TenancyConfig, create_remote_repo: bool) -> Self { - let base_repo_harness = BaseRepoHarness::new(tenancy_config); + let base_repo_harness = BaseRepoHarness::new(tenancy_config, None); let repos_dir = base_repo_harness.temp_dir_path().join("repos"); std::fs::create_dir(&repos_dir).unwrap(); diff --git a/src/infra/core/tests/tests/test_query_service_impl.rs b/src/infra/core/tests/tests/test_query_service_impl.rs index 4b6451928e..ca239f0ed1 100644 --- a/src/infra/core/tests/tests/test_query_service_impl.rs +++ b/src/infra/core/tests/tests/test_query_service_impl.rs @@ -124,6 +124,7 @@ fn create_catalog_with_local_workspace( std::fs::create_dir(&datasets_dir).unwrap(); dill::CatalogBuilder::new() + .add::() .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir)) @@ -149,6 +150,7 @@ async fn create_catalog_with_s3_workspace( let s3_context = S3Context::from_items(endpoint.clone(), bucket, key_prefix).await; dill::CatalogBuilder::new() + .add::() .add::() .add_value(TenancyConfig::SingleTenant) .add_builder(DatasetRepositoryS3::builder().with_s3_context(s3_context.clone())) diff --git a/src/infra/core/tests/tests/test_remote_status_service.rs b/src/infra/core/tests/tests/test_remote_status_service.rs index 963bd6130e..8667df68cb 100644 --- a/src/infra/core/tests/tests/test_remote_status_service.rs +++ b/src/infra/core/tests/tests/test_remote_status_service.rs @@ -240,7 +240,7 @@ struct RemoteStatusTestHarness { impl RemoteStatusTestHarness { fn new() -> Self { - let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant); // TODO review + let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant, None); // TODO review let remote_repos_dir = base_repo_harness.temp_dir_path().join("remote_repos"); let catalog = CatalogBuilder::new_chained(base_repo_harness.catalog()) diff --git a/src/infra/core/tests/tests/test_reset_services_impl.rs b/src/infra/core/tests/tests/test_reset_services_impl.rs index d0f2d7e420..5d47991e61 100644 --- a/src/infra/core/tests/tests/test_reset_services_impl.rs +++ b/src/infra/core/tests/tests/test_reset_services_impl.rs @@ -171,7 +171,7 @@ struct ResetTestHarness { impl ResetTestHarness { fn new() -> Self { - let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant); + let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant, None); let catalog = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()) .add::() diff --git a/src/infra/core/tests/tests/test_search_service_impl.rs b/src/infra/core/tests/tests/test_search_service_impl.rs index 5b6008ceed..b8c00c7ed0 100644 --- a/src/infra/core/tests/tests/test_search_service_impl.rs +++ b/src/infra/core/tests/tests/test_search_service_impl.rs @@ -31,6 +31,7 @@ async fn do_test_search(tmp_workspace_dir: &Path, repo_url: Url) { std::fs::create_dir(&datasets_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add::() .add_value(CurrentAccountSubject::new_test()) .add_value(TenancyConfig::SingleTenant) diff --git a/src/infra/core/tests/tests/test_sync_service_impl.rs b/src/infra/core/tests/tests/test_sync_service_impl.rs index 1d29d1c6f2..c3c38b1183 100644 --- a/src/infra/core/tests/tests/test_sync_service_impl.rs +++ b/src/infra/core/tests/tests/test_sync_service_impl.rs @@ -70,6 +70,7 @@ async fn do_test_sync( std::fs::create_dir(&datasets_dir_bar).unwrap(); let catalog_foo = dill::CatalogBuilder::new() + .add::() .add::() .add_value(ipfs_gateway.clone()) .add_value(ipfs_client.clone()) @@ -92,6 +93,7 @@ async fn do_test_sync( .build(); let catalog_bar = dill::CatalogBuilder::new() + .add::() .add::() .add_value(ipfs_gateway.clone()) .add_value(ipfs_client.clone()) diff --git a/src/infra/core/tests/tests/test_transform_services_impl.rs b/src/infra/core/tests/tests/test_transform_services_impl.rs index f1d979e0bd..f5be8fe8d1 100644 --- a/src/infra/core/tests/tests/test_transform_services_impl.rs +++ b/src/infra/core/tests/tests/test_transform_services_impl.rs @@ -51,6 +51,7 @@ impl TransformTestHarness { std::fs::create_dir(&run_info_dir).unwrap(); let catalog = dill::CatalogBuilder::new() + .add::() .add_value(RunInfoDir::new(run_info_dir)) .add_value(CurrentAccountSubject::new_test()) .add_value(TenancyConfig::SingleTenant) diff --git a/src/infra/core/tests/tests/test_verification_service_impl.rs b/src/infra/core/tests/tests/test_verification_service_impl.rs index f0a4b17a52..9898686c38 100644 --- a/src/infra/core/tests/tests/test_verification_service_impl.rs +++ b/src/infra/core/tests/tests/test_verification_service_impl.rs @@ -193,7 +193,7 @@ struct VerifyHarness { impl VerifyHarness { fn new() -> Self { - let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant); + let base_repo_harness = BaseRepoHarness::new(TenancyConfig::SingleTenant, None); let catalog = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()) .add::() diff --git a/src/infra/core/tests/tests/test_watermark_services_impl.rs b/src/infra/core/tests/tests/test_watermark_services_impl.rs index 19f141b1f4..342a4e0752 100644 --- a/src/infra/core/tests/tests/test_watermark_services_impl.rs +++ b/src/infra/core/tests/tests/test_watermark_services_impl.rs @@ -169,7 +169,7 @@ struct WatermarkTestHarness { impl WatermarkTestHarness { fn new(tenancy_config: TenancyConfig) -> Self { - let base_repo_harness = BaseRepoHarness::new(tenancy_config); + let base_repo_harness = BaseRepoHarness::new(tenancy_config, None); let catalog = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()) .add::() diff --git a/src/infra/core/tests/tests/use_cases/base_use_case_harness.rs b/src/infra/core/tests/tests/use_cases/base_use_case_harness.rs index 2a5ad0c425..dc89e19115 100644 --- a/src/infra/core/tests/tests/use_cases/base_use_case_harness.rs +++ b/src/infra/core/tests/tests/use_cases/base_use_case_harness.rs @@ -10,7 +10,7 @@ use dill::Catalog; use kamu::testing::{BaseRepoHarness, MockDatasetActionAuthorizer}; use kamu_core::auth::DatasetActionAuthorizer; -use kamu_core::TenancyConfig; +use kamu_core::{MockDidGenerator, TenancyConfig}; use messaging_outbox::{MockOutbox, Outbox}; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -19,6 +19,7 @@ pub(crate) struct BaseUseCaseHarnessOptions { tenancy_config: TenancyConfig, mock_dataset_action_authorizer: MockDatasetActionAuthorizer, mock_outbox: MockOutbox, + maybe_mock_did_generator: Option, } impl BaseUseCaseHarnessOptions { @@ -38,6 +39,14 @@ impl BaseUseCaseHarnessOptions { self.mock_outbox = mock_outbox; self } + + pub(crate) fn with_maybe_mock_did_generator( + mut self, + mock_did_generator: Option, + ) -> Self { + self.maybe_mock_did_generator = mock_did_generator; + self + } } impl Default for BaseUseCaseHarnessOptions { @@ -46,6 +55,7 @@ impl Default for BaseUseCaseHarnessOptions { tenancy_config: TenancyConfig::SingleTenant, mock_dataset_action_authorizer: MockDatasetActionAuthorizer::new(), mock_outbox: MockOutbox::new(), + maybe_mock_did_generator: None, } } } @@ -60,7 +70,8 @@ pub(crate) struct BaseUseCaseHarness { impl BaseUseCaseHarness { pub(crate) fn new(options: BaseUseCaseHarnessOptions) -> Self { - let base_repo_harness = BaseRepoHarness::new(options.tenancy_config); + let base_repo_harness = + BaseRepoHarness::new(options.tenancy_config, options.maybe_mock_did_generator); let catalog = dill::CatalogBuilder::new_chained(base_repo_harness.catalog()) .add_value(options.mock_dataset_action_authorizer) diff --git a/src/infra/core/tests/tests/use_cases/test_commit_dataset_event_use_case.rs b/src/infra/core/tests/tests/use_cases/test_commit_dataset_event_use_case.rs index d68d41c077..44ebad7ed7 100644 --- a/src/infra/core/tests/tests/use_cases/test_commit_dataset_event_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_commit_dataset_event_use_case.rs @@ -12,9 +12,9 @@ use std::sync::Arc; use kamu::testing::{MetadataFactory, MockDatasetActionAuthorizer}; use kamu::CommitDatasetEventUseCaseImpl; -use kamu_core::{CommitDatasetEventUseCase, CommitError, CommitOpts}; +use kamu_core::{CommitDatasetEventUseCase, CommitError, CommitOpts, MockDidGenerator}; use messaging_outbox::MockOutbox; -use opendatafabric::{DatasetAlias, DatasetName, MetadataEvent}; +use opendatafabric::{DatasetAlias, DatasetID, DatasetName, MetadataEvent}; use crate::tests::use_cases::*; @@ -23,13 +23,18 @@ use crate::tests::use_cases::*; #[tokio::test] async fn test_commit_dataset_event() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true); + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true); let mock_outbox = MockOutbox::new(); - let harness = CommitDatasetEventUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = CommitDatasetEventUseCaseHarness::new( + mock_authorizer, + mock_outbox, + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), + ); let foo = harness.create_root_dataset(&alias_foo).await; let res = harness @@ -48,13 +53,18 @@ async fn test_commit_dataset_event() { #[tokio::test] async fn test_commit_event_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false); + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false); let mock_outbox = MockOutbox::new(); - let harness = CommitDatasetEventUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = CommitDatasetEventUseCaseHarness::new( + mock_authorizer, + mock_outbox, + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), + ); let foo = harness.create_root_dataset(&alias_foo).await; let res = harness @@ -74,14 +84,19 @@ async fn test_commit_event_unauthorized() { async fn test_commit_event_with_new_dependencies() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_bar) = DatasetID::new_generated_ed25519(); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_bar, 1, true); - + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_bar, 1, true); let mut mock_outbox = MockOutbox::new(); expect_outbox_dataset_dependencies_updated(&mut mock_outbox, 1); - let harness = CommitDatasetEventUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = CommitDatasetEventUseCaseHarness::new( + mock_authorizer, + mock_outbox, + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo, dataset_id_bar]), + ); let foo = harness.create_root_dataset(&alias_foo).await; let bar = harness .create_derived_dataset(&alias_bar, vec![foo.dataset_handle.as_local_ref()]) @@ -117,11 +132,13 @@ impl CommitDatasetEventUseCaseHarness { fn new( mock_dataset_action_authorizer: MockDatasetActionAuthorizer, mock_outbox: MockOutbox, + mock_did_generator: MockDidGenerator, ) -> Self { let base_harness = BaseUseCaseHarness::new( BaseUseCaseHarnessOptions::new() .with_authorizer(mock_dataset_action_authorizer) - .with_outbox(mock_outbox), + .with_outbox(mock_outbox) + .with_maybe_mock_did_generator(Some(mock_did_generator)), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_compact_dataset_use_case.rs b/src/infra/core/tests/tests/use_cases/test_compact_dataset_use_case.rs index 18c9cbacb6..2e67f62803 100644 --- a/src/infra/core/tests/tests/use_cases/test_compact_dataset_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_compact_dataset_use_case.rs @@ -13,7 +13,7 @@ use std::sync::Arc; use kamu::testing::MockDatasetActionAuthorizer; use kamu::*; use kamu_core::*; -use opendatafabric::{DatasetAlias, DatasetName}; +use opendatafabric::{DatasetAlias, DatasetID, DatasetName}; use crate::tests::use_cases::*; @@ -22,9 +22,11 @@ use crate::tests::use_cases::*; #[tokio::test] async fn test_compact_dataset_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = CompactUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -41,11 +43,14 @@ async fn test_compact_dataset_success() { async fn test_compact_multiple_datasets_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_bar) = DatasetID::new_generated_ed25519(); let harness = CompactUseCaseHarness::new( MockDatasetActionAuthorizer::new() - .expect_check_write_dataset(&alias_foo, 1, true) - .expect_check_write_dataset(&alias_bar, 1, true), + .expect_check_write_dataset(&dataset_id_foo, 1, true) + .expect_check_write_dataset(&dataset_id_bar, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo, dataset_id_bar]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -82,9 +87,11 @@ async fn test_compact_multiple_datasets_success() { #[tokio::test] async fn test_compact_dataset_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = CompactUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -101,11 +108,14 @@ async fn test_compact_dataset_unauthorized() { async fn test_compact_dataset_mixed_authorization_outcome() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_bar) = DatasetID::new_generated_ed25519(); let harness = CompactUseCaseHarness::new( MockDatasetActionAuthorizer::new() - .expect_check_write_dataset(&alias_foo, 1, false) - .expect_check_write_dataset(&alias_bar, 1, true), + .expect_check_write_dataset(&dataset_id_foo, 1, false) + .expect_check_write_dataset(&dataset_id_bar, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo, dataset_id_bar]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -146,9 +156,14 @@ struct CompactUseCaseHarness { } impl CompactUseCaseHarness { - fn new(mock_dataset_action_authorizer: MockDatasetActionAuthorizer) -> Self { + fn new( + mock_dataset_action_authorizer: MockDatasetActionAuthorizer, + mock_did_generator: MockDidGenerator, + ) -> Self { let base_harness = BaseUseCaseHarness::new( - BaseUseCaseHarnessOptions::new().with_authorizer(mock_dataset_action_authorizer), + BaseUseCaseHarnessOptions::new() + .with_authorizer(mock_dataset_action_authorizer) + .with_maybe_mock_did_generator(Some(mock_did_generator)), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_delete_dataset_use_case.rs b/src/infra/core/tests/tests/use_cases/test_delete_dataset_use_case.rs index 2e3594417e..64a129c7e8 100644 --- a/src/infra/core/tests/tests/use_cases/test_delete_dataset_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_delete_dataset_use_case.rs @@ -18,11 +18,12 @@ use kamu_core::{ DeleteDatasetError, DeleteDatasetUseCase, GetDatasetError, + MockDidGenerator, }; use kamu_datasets_inmem::InMemoryDatasetDependencyRepository; use kamu_datasets_services::{DependencyGraphIndexer, DependencyGraphServiceImpl}; use messaging_outbox::{consume_deserialized_message, ConsumerFilter, Message, MockOutbox}; -use opendatafabric::{DatasetAlias, DatasetName}; +use opendatafabric::{DatasetAlias, DatasetID, DatasetName}; use crate::tests::use_cases::*; @@ -31,14 +32,21 @@ use crate::tests::use_cases::*; #[tokio::test] async fn test_delete_dataset_success_via_ref() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let mut mock_outbox = MockOutbox::new(); expect_outbox_dataset_deleted(&mut mock_outbox, 1); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true); + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true); - let harness = DeleteUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = DeleteUseCaseHarness::new( + mock_authorizer, + mock_outbox, + Some(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + ])), + ); harness.create_root_dataset(&alias_foo).await; harness.reindex_dependency_graph().await; @@ -60,14 +68,21 @@ async fn test_delete_dataset_success_via_ref() { #[tokio::test] async fn test_delete_dataset_success_via_handle() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let mut mock_outbox = MockOutbox::new(); expect_outbox_dataset_deleted(&mut mock_outbox, 1); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true); + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true); - let harness = DeleteUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = DeleteUseCaseHarness::new( + mock_authorizer, + mock_outbox, + Some(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + ])), + ); let foo = harness.create_root_dataset(&alias_foo).await; harness.reindex_dependency_graph().await; @@ -88,7 +103,8 @@ async fn test_delete_dataset_success_via_handle() { #[tokio::test] async fn test_delete_dataset_not_found() { - let harness = DeleteUseCaseHarness::new(MockDatasetActionAuthorizer::new(), MockOutbox::new()); + let harness = + DeleteUseCaseHarness::new(MockDatasetActionAuthorizer::new(), MockOutbox::new(), None); let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); assert_matches!( @@ -105,10 +121,14 @@ async fn test_delete_dataset_not_found() { #[tokio::test] async fn test_delete_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = DeleteUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false), MockOutbox::new(), + Some(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + ])), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -135,7 +155,8 @@ async fn test_delete_dataset_respects_dangling_refs() { let mut mock_outbox = MockOutbox::new(); expect_outbox_dataset_deleted(&mut mock_outbox, 2); - let harness = DeleteUseCaseHarness::new(MockDatasetActionAuthorizer::allowing(), mock_outbox); + let harness = + DeleteUseCaseHarness::new(MockDatasetActionAuthorizer::allowing(), mock_outbox, None); let root = harness.create_root_dataset(&alias_foo).await; let derived = harness @@ -201,11 +222,13 @@ impl DeleteUseCaseHarness { fn new( mock_dataset_action_authorizer: MockDatasetActionAuthorizer, mock_outbox: MockOutbox, + maybe_mock_did_generator: Option, ) -> Self { let base_harness = BaseUseCaseHarness::new( BaseUseCaseHarnessOptions::new() .with_authorizer(mock_dataset_action_authorizer) - .with_outbox(mock_outbox), + .with_outbox(mock_outbox) + .with_maybe_mock_did_generator(maybe_mock_did_generator), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_rename_dataset_use_case.rs b/src/infra/core/tests/tests/use_cases/test_rename_dataset_use_case.rs index 6d4275f095..ed7f4c71b9 100644 --- a/src/infra/core/tests/tests/use_cases/test_rename_dataset_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_rename_dataset_use_case.rs @@ -12,9 +12,9 @@ use std::sync::Arc; use kamu::testing::MockDatasetActionAuthorizer; use kamu::RenameDatasetUseCaseImpl; -use kamu_core::{GetDatasetError, RenameDatasetError, RenameDatasetUseCase}; +use kamu_core::{GetDatasetError, MockDidGenerator, RenameDatasetError, RenameDatasetUseCase}; use messaging_outbox::MockOutbox; -use opendatafabric::{DatasetAlias, DatasetName}; +use opendatafabric::{DatasetAlias, DatasetID, DatasetName}; use crate::tests::use_cases::*; @@ -24,13 +24,20 @@ use crate::tests::use_cases::*; async fn test_rename_dataset_success_via_ref() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let mock_authorizer = - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true); + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true); let mut mock_outbox = MockOutbox::new(); expect_outbox_dataset_renamed(&mut mock_outbox, 1); - let harness = RenameUseCaseHarness::new(mock_authorizer, mock_outbox); + let harness = RenameUseCaseHarness::new( + mock_authorizer, + mock_outbox, + Some(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + ])), + ); harness.create_root_dataset(&alias_foo).await; assert_matches!(harness.check_dataset_exists(&alias_foo).await, Ok(_)); @@ -56,7 +63,8 @@ async fn test_rename_dataset_success_via_ref() { #[tokio::test] async fn test_rename_dataset_not_found() { - let harness = RenameUseCaseHarness::new(MockDatasetActionAuthorizer::new(), MockOutbox::new()); + let harness = + RenameUseCaseHarness::new(MockDatasetActionAuthorizer::new(), MockOutbox::new(), None); let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); assert_matches!( @@ -76,10 +84,14 @@ async fn test_rename_dataset_not_found() { #[tokio::test] async fn test_rename_dataset_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = RenameUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false), MockOutbox::new(), + Some(MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + ])), ); harness.create_root_dataset(&alias_foo).await; @@ -110,11 +122,13 @@ impl RenameUseCaseHarness { fn new( mock_dataset_action_authorizer: MockDatasetActionAuthorizer, mock_outbox: MockOutbox, + maybe_mock_did_generator: Option, ) -> Self { let base_harness = BaseUseCaseHarness::new( BaseUseCaseHarnessOptions::new() .with_authorizer(mock_dataset_action_authorizer) - .with_outbox(mock_outbox), + .with_outbox(mock_outbox) + .with_maybe_mock_did_generator(maybe_mock_did_generator), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_reset_dataset_use_case.rs b/src/infra/core/tests/tests/use_cases/test_reset_dataset_use_case.rs index 3459d7f946..ac4aee0bb6 100644 --- a/src/infra/core/tests/tests/use_cases/test_reset_dataset_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_reset_dataset_use_case.rs @@ -22,9 +22,11 @@ use super::{BaseUseCaseHarness, BaseUseCaseHarnessOptions}; #[tokio::test] async fn test_reset_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = ResetUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -53,9 +55,11 @@ async fn test_reset_success() { #[tokio::test] async fn test_reset_dataset_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = ResetUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -78,9 +82,14 @@ struct ResetUseCaseHarness { } impl ResetUseCaseHarness { - fn new(mock_dataset_action_authorizer: MockDatasetActionAuthorizer) -> Self { + fn new( + mock_dataset_action_authorizer: MockDatasetActionAuthorizer, + mock_did_generator: MockDidGenerator, + ) -> Self { let base_harness = BaseUseCaseHarness::new( - BaseUseCaseHarnessOptions::new().with_authorizer(mock_dataset_action_authorizer), + BaseUseCaseHarnessOptions::new() + .with_authorizer(mock_dataset_action_authorizer) + .with_maybe_mock_did_generator(Some(mock_did_generator)), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_set_watermark_use_case.rs b/src/infra/core/tests/tests/use_cases/test_set_watermark_use_case.rs index 99cef42103..e3324b902f 100644 --- a/src/infra/core/tests/tests/use_cases/test_set_watermark_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_set_watermark_use_case.rs @@ -23,9 +23,11 @@ use super::{BaseUseCaseHarness, BaseUseCaseHarnessOptions}; #[tokio::test] async fn test_set_watermark_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = SetWatermarkUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, true), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -46,9 +48,11 @@ async fn test_set_watermark_success() { #[tokio::test] async fn test_set_watermark_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = SetWatermarkUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_write_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_write_dataset(&dataset_id_foo, 1, false), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -73,9 +77,14 @@ struct SetWatermarkUseCaseHarness { } impl SetWatermarkUseCaseHarness { - fn new(mock_dataset_action_authorizer: MockDatasetActionAuthorizer) -> Self { + fn new( + mock_dataset_action_authorizer: MockDatasetActionAuthorizer, + mock_did_generator: MockDidGenerator, + ) -> Self { let base_harness = BaseUseCaseHarness::new( - BaseUseCaseHarnessOptions::new().with_authorizer(mock_dataset_action_authorizer), + BaseUseCaseHarnessOptions::new() + .with_authorizer(mock_dataset_action_authorizer) + .with_maybe_mock_did_generator(Some(mock_did_generator)), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) diff --git a/src/infra/core/tests/tests/use_cases/test_verify_dataset_use_case.rs b/src/infra/core/tests/tests/use_cases/test_verify_dataset_use_case.rs index 7f1e82aaf0..6f50c57a3b 100644 --- a/src/infra/core/tests/tests/use_cases/test_verify_dataset_use_case.rs +++ b/src/infra/core/tests/tests/use_cases/test_verify_dataset_use_case.rs @@ -22,9 +22,11 @@ use super::{BaseUseCaseHarness, BaseUseCaseHarnessOptions}; #[tokio::test] async fn test_verify_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = VerifyUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_read_dataset(&alias_foo, 1, true), + MockDatasetActionAuthorizer::new().expect_check_read_dataset(&dataset_id_foo, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -43,11 +45,14 @@ async fn test_verify_success() { async fn test_verify_multiple_success() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_bar) = DatasetID::new_generated_ed25519(); let harness = VerifyUseCaseHarness::new( MockDatasetActionAuthorizer::new() - .expect_check_read_dataset(&alias_foo, 1, true) - .expect_check_read_dataset(&alias_bar, 1, true), + .expect_check_read_dataset(&dataset_id_foo, 1, true) + .expect_check_read_dataset(&dataset_id_bar, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo, dataset_id_bar]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -87,9 +92,11 @@ async fn test_verify_multiple_success() { #[tokio::test] async fn test_verify_unauthorized() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); let harness = VerifyUseCaseHarness::new( - MockDatasetActionAuthorizer::new().expect_check_read_dataset(&alias_foo, 1, false), + MockDatasetActionAuthorizer::new().expect_check_read_dataset(&dataset_id_foo, 1, false), + MockDidGenerator::predefined_dataset_ids(vec![dataset_id_foo]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -109,12 +116,20 @@ async fn test_verify_mixed_authorization_outcome() { let alias_foo = DatasetAlias::new(None, DatasetName::new_unchecked("foo")); let alias_bar = DatasetAlias::new(None, DatasetName::new_unchecked("bar")); let alias_baz = DatasetAlias::new(None, DatasetName::new_unchecked("baz")); + let (_, dataset_id_foo) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_bar) = DatasetID::new_generated_ed25519(); + let (_, dataset_id_baz) = DatasetID::new_generated_ed25519(); let harness = VerifyUseCaseHarness::new( MockDatasetActionAuthorizer::new() - .expect_check_read_dataset(&alias_foo, 1, true) - .expect_check_read_dataset(&alias_bar, 1, false) - .expect_check_read_dataset(&alias_baz, 1, true), + .expect_check_read_dataset(&dataset_id_foo, 1, true) + .expect_check_read_dataset(&dataset_id_bar, 1, false) + .expect_check_read_dataset(&dataset_id_baz, 1, true), + MockDidGenerator::predefined_dataset_ids(vec![ + dataset_id_foo, + dataset_id_bar, + dataset_id_baz, + ]), ); let foo = harness.create_root_dataset(&alias_foo).await; @@ -169,9 +184,14 @@ struct VerifyUseCaseHarness { } impl VerifyUseCaseHarness { - fn new(mock_dataset_action_authorizer: MockDatasetActionAuthorizer) -> Self { + fn new( + mock_dataset_action_authorizer: MockDatasetActionAuthorizer, + mock_did_generator: MockDidGenerator, + ) -> Self { let base_harness = BaseUseCaseHarness::new( - BaseUseCaseHarnessOptions::new().with_authorizer(mock_dataset_action_authorizer), + BaseUseCaseHarnessOptions::new() + .with_authorizer(mock_dataset_action_authorizer) + .with_maybe_mock_did_generator(Some(mock_did_generator)), ); let catalog = dill::CatalogBuilder::new_chained(base_harness.catalog()) From 55aadf90adf8b42249f30d5e1fbae404a4fd96c0 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 13:29:48 +0200 Subject: [PATCH 09/20] DatasetActionAuthorizer::is_action_allowed(): use DatasetID instead of DatasetHandle --- .../src/auth/dataset_action_authorizer.rs | 30 +++++++++++-------- src/infra/core/src/services/query/mod.rs | 15 +++++----- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/domain/core/src/auth/dataset_action_authorizer.rs b/src/domain/core/src/auth/dataset_action_authorizer.rs index 05aa72ab10..976be5edd2 100644 --- a/src/domain/core/src/auth/dataset_action_authorizer.rs +++ b/src/domain/core/src/auth/dataset_action_authorizer.rs @@ -28,18 +28,6 @@ pub trait DatasetActionAuthorizer: Sync + Send { action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError>; - async fn is_action_allowed( - &self, - dataset_handle: &odf::DatasetHandle, - action: DatasetAction, - ) -> Result { - match self.check_action_allowed(&dataset_handle.id, action).await { - Ok(()) => Ok(true), - Err(DatasetActionUnauthorizedError::Access(_)) => Ok(false), - Err(DatasetActionUnauthorizedError::Internal(err)) => Err(err), - } - } - // TODO: Private Datasets: tests async fn get_allowed_actions( &self, @@ -157,6 +145,12 @@ pub struct ClassifyByAllowanceResponse { #[async_trait::async_trait] pub trait DatasetActionAuthorizerExt: DatasetActionAuthorizer { + async fn is_action_allowed( + &self, + dataset_id: &odf::DatasetID, + action: DatasetAction, + ) -> Result; + fn filtered_datasets_stream<'a>( &'a self, dataset_handles_stream: DatasetHandleStream<'a>, @@ -172,6 +166,18 @@ where T: DatasetActionAuthorizer, T: ?Sized, { + async fn is_action_allowed( + &self, + dataset_id: &odf::DatasetID, + action: DatasetAction, + ) -> Result { + match self.check_action_allowed(dataset_id, action).await { + Ok(()) => Ok(true), + Err(DatasetActionUnauthorizedError::Access(_)) => Ok(false), + Err(DatasetActionUnauthorizedError::Internal(err)) => Err(err), + } + } + fn filtered_datasets_stream<'a>( &'a self, dataset_handles_stream: DatasetHandleStream<'a>, diff --git a/src/infra/core/src/services/query/mod.rs b/src/infra/core/src/services/query/mod.rs index 99354c8a4f..aada24abb9 100644 --- a/src/infra/core/src/services/query/mod.rs +++ b/src/infra/core/src/services/query/mod.rs @@ -130,23 +130,22 @@ impl KamuSchema { if !name_resolution_enabled { for (id, opts) in &self.inner.options.input_datasets { - let hdl = self - .inner - .dataset_registry - .resolve_dataset_handle_by_ref(&id.as_local_ref()) - .await - .int_err()?; - if !self .inner .dataset_action_authorizer - .is_action_allowed(&hdl, auth::DatasetAction::Read) + .is_action_allowed(id, auth::DatasetAction::Read) .await? { // Ignore this alias and let the query fail with "not found" error continue; } + let hdl = self + .inner + .dataset_registry + .resolve_dataset_handle_by_ref(&id.as_local_ref()) + .await + .int_err()?; let resolved_dataset = self.inner.dataset_registry.get_dataset_by_handle(&hdl); tables.insert( From 89c362ebfc63cb85a0d18474115850ce47f59faa Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 13:42:08 +0200 Subject: [PATCH 10/20] DatasetActionAuthorizer::get_allowed_actions(): use DatasetID instead of DatasetHandle --- .../src/oso_dataset_authorizer.rs | 6 ++--- .../tests/test_oso_dataset_authorizer.rs | 24 +++++++++---------- .../graphql/src/queries/datasets/dataset.rs | 2 +- .../src/auth/dataset_action_authorizer.rs | 5 ++-- .../testing/mock_dataset_action_authorizer.rs | 2 +- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs index 4819b6b757..27f874a9a4 100644 --- a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs +++ b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs @@ -111,13 +111,13 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer { } } - #[tracing::instrument(level = "debug", skip_all, fields(%dataset_handle))] + #[tracing::instrument(level = "debug", skip_all, fields(%dataset_id))] async fn get_allowed_actions( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, ) -> Result, InternalError> { let (user_actor, dataset_resource) = - try_join!(self.user_actor(), self.dataset_resource(&dataset_handle.id))?; + try_join!(self.user_actor(), self.dataset_resource(dataset_id))?; self.kamu_auth_oso .get_allowed_actions(user_actor, dataset_resource) diff --git a/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs index c527230b6c..9aa54ae72d 100644 --- a/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs +++ b/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs @@ -48,23 +48,23 @@ use time_source::SystemTimeSourceDefault; #[test_log::test(tokio::test)] async fn test_owner_can_read_and_write_private_dataset() { let harness = DatasetAuthorizerHarness::new(logged("john")).await; - let dataset_handle = harness + let dataset_id = harness .create_private_dataset(dataset_alias("john/foo")) .await; let read_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle.id, DatasetAction::Read) + .check_action_allowed(&dataset_id, DatasetAction::Read) .await; let write_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle.id, DatasetAction::Write) + .check_action_allowed(&dataset_id, DatasetAction::Write) .await; let allowed_actions = harness .dataset_authorizer - .get_allowed_actions(&dataset_handle) + .get_allowed_actions(&dataset_id) .await; assert_matches!(read_result, Ok(())); @@ -81,23 +81,23 @@ async fn test_owner_can_read_and_write_private_dataset() { #[test_log::test(tokio::test)] async fn test_guest_can_read_but_not_write_public_dataset() { let harness = DatasetAuthorizerHarness::new(anonymous()).await; - let dataset_handle = harness + let dataset_id = harness .create_public_dataset(dataset_alias("john/foo")) .await; let read_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle.id, DatasetAction::Read) + .check_action_allowed(&dataset_id, DatasetAction::Read) .await; let write_result = harness .dataset_authorizer - .check_action_allowed(&dataset_handle.id, DatasetAction::Write) + .check_action_allowed(&dataset_id, DatasetAction::Write) .await; let allowed_actions = harness .dataset_authorizer - .get_allowed_actions(&dataset_handle) + .get_allowed_actions(&dataset_id) .await; assert_matches!(read_result, Ok(())); @@ -185,11 +185,11 @@ impl DatasetAuthorizerHarness { } } - async fn create_public_dataset(&self, alias: odf::DatasetAlias) -> odf::DatasetHandle { + async fn create_public_dataset(&self, alias: odf::DatasetAlias) -> odf::DatasetID { self.create_dataset(alias, DatasetVisibility::Public).await } - async fn create_private_dataset(&self, alias: odf::DatasetAlias) -> odf::DatasetHandle { + async fn create_private_dataset(&self, alias: odf::DatasetAlias) -> odf::DatasetID { self.create_dataset(alias, DatasetVisibility::Private).await } @@ -197,7 +197,7 @@ impl DatasetAuthorizerHarness { &self, alias: odf::DatasetAlias, visibility: DatasetVisibility, - ) -> odf::DatasetHandle { + ) -> odf::DatasetID { let dataset_id = dataset_id(&alias); self.outbox @@ -213,7 +213,7 @@ impl DatasetAuthorizerHarness { .await .unwrap(); - odf::DatasetHandle::new(dataset_id, alias) + dataset_id } } diff --git a/src/adapter/graphql/src/queries/datasets/dataset.rs b/src/adapter/graphql/src/queries/datasets/dataset.rs index f1a1cae68f..da7258a59f 100644 --- a/src/adapter/graphql/src/queries/datasets/dataset.rs +++ b/src/adapter/graphql/src/queries/datasets/dataset.rs @@ -160,7 +160,7 @@ impl Dataset { let dataset_action_authorizer = from_catalog_n!(ctx, dyn auth::DatasetActionAuthorizer); let allowed_actions = dataset_action_authorizer - .get_allowed_actions(&self.dataset_handle) + .get_allowed_actions(&self.dataset_handle.id) .await?; let can_read = allowed_actions.contains(&auth::DatasetAction::Read); let can_write = allowed_actions.contains(&auth::DatasetAction::Write); diff --git a/src/domain/core/src/auth/dataset_action_authorizer.rs b/src/domain/core/src/auth/dataset_action_authorizer.rs index 976be5edd2..ba008e1b08 100644 --- a/src/domain/core/src/auth/dataset_action_authorizer.rs +++ b/src/domain/core/src/auth/dataset_action_authorizer.rs @@ -23,7 +23,6 @@ use crate::{AccessError, DatasetHandleStream}; pub trait DatasetActionAuthorizer: Sync + Send { async fn check_action_allowed( &self, - // TODO: Private Datasets: migrate to use odf::DatasetID, here and below dataset_id: &odf::DatasetID, action: DatasetAction, ) -> Result<(), DatasetActionUnauthorizedError>; @@ -31,7 +30,7 @@ pub trait DatasetActionAuthorizer: Sync + Send { // TODO: Private Datasets: tests async fn get_allowed_actions( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, ) -> Result, InternalError>; // TODO: Private Datasets: tests @@ -243,7 +242,7 @@ impl DatasetActionAuthorizer for AlwaysHappyDatasetActionAuthorizer { async fn get_allowed_actions( &self, - _dataset_handle: &odf::DatasetHandle, + _dataset_id: &odf::DatasetID, ) -> Result, InternalError> { Ok(HashSet::from([DatasetAction::Read, DatasetAction::Write])) } diff --git a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs index da6b1cb49b..9c08ef7937 100644 --- a/src/infra/core/src/testing/mock_dataset_action_authorizer.rs +++ b/src/infra/core/src/testing/mock_dataset_action_authorizer.rs @@ -38,7 +38,7 @@ mockall::mock! { async fn get_allowed_actions( &self, - dataset_handle: &odf::DatasetHandle, + dataset_id: &odf::DatasetID, ) -> Result, InternalError>; async fn filter_datasets_allowing( From 4eb1b37fb6c8e99f43c9591f0764afe4757b5196 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 13:56:20 +0200 Subject: [PATCH 11/20] DatasetActionAuthorizer: finalization --- src/domain/core/src/auth/dataset_action_authorizer.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/domain/core/src/auth/dataset_action_authorizer.rs b/src/domain/core/src/auth/dataset_action_authorizer.rs index ba008e1b08..cb55711b29 100644 --- a/src/domain/core/src/auth/dataset_action_authorizer.rs +++ b/src/domain/core/src/auth/dataset_action_authorizer.rs @@ -36,7 +36,6 @@ pub trait DatasetActionAuthorizer: Sync + Send { // TODO: Private Datasets: tests async fn filter_datasets_allowing( &self, - // TODO: Private Datasets: use slice? here and above dataset_handles: Vec, action: DatasetAction, ) -> Result, InternalError>; @@ -49,8 +48,6 @@ pub trait DatasetActionAuthorizer: Sync + Send { ) -> Result; // TODO: Private Datasets: tests - // TODO: Private Datasets: use classify_datasets_by_allowance() name - // after migration async fn classify_dataset_ids_by_allowance( &self, dataset_ids: Vec, @@ -188,15 +185,15 @@ where use futures::TryStreamExt; // Page by page check... - let mut related_dataset_handles = dataset_handles_stream + let mut chunked_dataset_handles = dataset_handles_stream .try_chunks(STREAM_CHUNK_LEN); - while let Some(potentially_related_handles_chunk) = - related_dataset_handles.try_next().await.int_err()? + while let Some(datataset_handles_chunk) = + chunked_dataset_handles.try_next().await.int_err()? { // ... the datasets that are accessed. let hdls = self - .filter_datasets_allowing(potentially_related_handles_chunk, action) + .filter_datasets_allowing(datataset_handles_chunk, action) .await?; for hdl in hdls { From b1ee411863971f73ff7d8f0b36d7b2f29bda258a Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 14:08:11 +0200 Subject: [PATCH 12/20] ODataServiceContext::list_collections(): use DatasetActionAuthorizer::filtered_datasets_stream() --- src/adapter/odata/src/context.rs | 42 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/adapter/odata/src/context.rs b/src/adapter/odata/src/context.rs index bdf38673ca..686e77bf59 100644 --- a/src/adapter/odata/src/context.rs +++ b/src/adapter/odata/src/context.rs @@ -28,6 +28,7 @@ use datafusion_odata::context::{CollectionContext, OnUnsupported, ServiceContext use datafusion_odata::error::ODataError; use dill::Catalog; use internal_error::ResultIntoInternal; +use kamu_core::auth::DatasetActionAuthorizerExt; use kamu_core::*; use opendatafabric::*; @@ -76,32 +77,27 @@ impl ServiceContext for ODataServiceContext { } else { registry.all_dataset_handles() }; - - let dataset_handles: Vec<_> = dataset_handles - .try_collect() - .await - .map_err(ODataError::internal)?; - - let dataset_handles = authorizer - .filter_datasets_allowing(dataset_handles, DatasetAction::Read) + let readable_dataset_handles_stream = + authorizer.filtered_datasets_stream(dataset_handles, DatasetAction::Read); + + let collections = readable_dataset_handles_stream + .map_ok(|dataset_handle| { + let resolved_dataset = registry.get_dataset_by_handle(&dataset_handle); + let context: Arc = Arc::new(ODataCollectionContext { + catalog: self.catalog.clone(), + addr: CollectionAddr { + name: dataset_handle.alias.dataset_name.to_string(), + key: None, + }, + resolved_dataset, + service_base_url: self.service_base_url.clone(), + }); + context + }) + .try_collect::>() .await .map_err(ODataError::internal)?; - let mut collections: Vec> = Vec::new(); - for dataset_handle in dataset_handles { - let resolved_dataset = registry.get_dataset_by_handle(&dataset_handle); - - collections.push(Arc::new(ODataCollectionContext { - catalog: self.catalog.clone(), - addr: CollectionAddr { - name: dataset_handle.alias.dataset_name.to_string(), - key: None, - }, - resolved_dataset, - service_base_url: self.service_base_url.clone(), - })); - } - Ok(collections) } From 8f09f1b86d8ddd1eeefee39a08c8bd3c84b448d8 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 14:46:12 +0200 Subject: [PATCH 13/20] Datasets::by_account_impl(): use DatasetActionAuthorizer::filtered_datasets_stream() --- .../graphql/src/queries/datasets/datasets.rs | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/adapter/graphql/src/queries/datasets/datasets.rs b/src/adapter/graphql/src/queries/datasets/datasets.rs index 84f1b0f190..c8d70f8122 100644 --- a/src/adapter/graphql/src/queries/datasets/datasets.rs +++ b/src/adapter/graphql/src/queries/datasets/datasets.rs @@ -7,7 +7,7 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use kamu_core::auth::{DatasetAction, DatasetActionAuthorizer}; +use kamu_core::auth::{DatasetAction, DatasetActionAuthorizer, DatasetActionAuthorizerExt}; use kamu_core::{ DatasetRegistryExt, {self as domain}, @@ -25,7 +25,6 @@ pub struct Datasets; #[Object] impl Datasets { const DEFAULT_PER_PAGE: usize = 15; - const DATASETS_CHUNK_SIZE: usize = 100; #[graphql(skip)] async fn by_dataset_ref( @@ -95,24 +94,13 @@ impl Datasets { use futures::TryStreamExt; - let mut account_owned_datasets_stream = dataset_registry - .all_dataset_handles_by_owner(&account_name.clone().into()) - .try_chunks(Self::DATASETS_CHUNK_SIZE); - let mut accessible_datasets_handles = Vec::new(); - - while let Some(account_owned_dataset_handles_chunk) = - account_owned_datasets_stream.try_next().await.int_err()? - { - let authorized_handles = dataset_action_authorizer - .classify_dataset_handles_by_allowance( - account_owned_dataset_handles_chunk, - DatasetAction::Read, - ) - .await? - .authorized_handles; - - accessible_datasets_handles.extend(authorized_handles); - } + let account_owned_datasets_stream = + dataset_registry.all_dataset_handles_by_owner(&account_name.clone().into()); + let readable_dataset_handles_stream = dataset_action_authorizer + .filtered_datasets_stream(account_owned_datasets_stream, DatasetAction::Read); + let mut accessible_datasets_handles = readable_dataset_handles_stream + .try_collect::>() + .await?; let total_count = accessible_datasets_handles.len(); From 80cd2bd31c495f98572501ce7fd3ec2c51523d62 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 16:47:20 +0200 Subject: [PATCH 14/20] Search::query(): use DatasetActionAuthorizer::filtered_datasets_stream() --- src/adapter/graphql/src/queries/search.rs | 25 ++++++++--------- .../core/src/utils/datasets_filtering.rs | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/adapter/graphql/src/queries/search.rs b/src/adapter/graphql/src/queries/search.rs index f8aa5e3363..aeff92da6e 100644 --- a/src/adapter/graphql/src/queries/search.rs +++ b/src/adapter/graphql/src/queries/search.rs @@ -7,8 +7,8 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use futures::TryStreamExt; -use kamu_core::auth::DatasetAction; +use kamu::utils::datasets_filtering::filter_dataset_handle_stream; +use kamu_core::auth::{DatasetAction, DatasetActionAuthorizerExt}; use kamu_core::{self as domain, TryStreamExtExt}; use crate::prelude::*; @@ -33,6 +33,8 @@ impl Search { page: Option, per_page: Option, ) -> Result { + use futures::TryStreamExt; + let (dataset_registry, dataset_action_authorizer) = from_catalog_n!( ctx, dyn domain::DatasetRegistry, @@ -46,17 +48,16 @@ impl Search { // to filter, e.g.: // - Anonymous: get all the public // - Logged: all owned datasets and datasets with relations - let filtered_dataset_handles: Vec<_> = dataset_registry - .all_dataset_handles() + let filtered_all_dataset_handles_stream = + filter_dataset_handle_stream(dataset_registry.all_dataset_handles(), |hdl| { + hdl.alias.dataset_name.contains(&query) + }); + let readable_dataset_handles_stream = dataset_action_authorizer + .filtered_datasets_stream(filtered_all_dataset_handles_stream, DatasetAction::Read); + let readable_dataset_handles = readable_dataset_handles_stream .filter_ok(|hdl| hdl.alias.dataset_name.contains(&query)) - .try_collect() - .await - .int_err()?; - - let readable_dataset_handles = dataset_action_authorizer - .filter_datasets_allowing(filtered_dataset_handles, DatasetAction::Read) - .await - .int_err()?; + .try_collect::>() + .await?; let total_count = readable_dataset_handles.len(); diff --git a/src/infra/core/src/utils/datasets_filtering.rs b/src/infra/core/src/utils/datasets_filtering.rs index 37650ff5af..ce46f2d0f6 100644 --- a/src/infra/core/src/utils/datasets_filtering.rs +++ b/src/infra/core/src/utils/datasets_filtering.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use futures::{future, StreamExt, TryStreamExt}; use internal_error::InternalError; use kamu_core::{ + DatasetHandleStream, DatasetRegistry, GetDatasetError, SearchError, @@ -211,3 +212,29 @@ pub fn matches_local_ref_pattern( } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +/// A utility that allows you to filter an `DatasetHandleStream` without +/// changing its type (like `StreamExt::filter_ok()` does). +pub fn filter_dataset_handle_stream<'a, F>( + mut stream: DatasetHandleStream<'a>, + predicate: F, +) -> DatasetHandleStream<'a> +where + F: Fn(&DatasetHandle) -> bool, + F: Send + 'a, +{ + Box::pin(async_stream::stream! { + while let Some(item) = stream.next().await { + if let Ok(dataset_handle) = &item { + if predicate(dataset_handle) { + yield item; + } + } else { + // In case of an iteration error, it is not our responsibility to handle the error here. + yield item; + } + } + }) +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// From 7506f8427477b24816e6041e4fe77155f6538193 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 19:13:42 +0200 Subject: [PATCH 15/20] GetDatasetDownstreamDependenciesUseCase: extract --- .../src/queries/datasets/dataset_metadata.rs | 92 +++--------- src/app/cli/src/app.rs | 1 + ...ataset_downstream_dependencies_use_case.rs | 66 +++++++++ src/domain/core/src/use_cases/mod.rs | 2 + src/domain/core/src/utils/tenancy_config.rs | 15 ++ .../src/repos/dataset_entry_repository.rs | 13 ++ .../src/dataset_entry_service_impl.rs | 17 +-- ...t_downstream_dependencies_use_case_impl.rs | 135 ++++++++++++++++++ src/infra/core/src/use_cases/mod.rs | 2 + 9 files changed, 255 insertions(+), 88 deletions(-) create mode 100644 src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs create mode 100644 src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs diff --git a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs index 28aec4bb41..8106258940 100644 --- a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs +++ b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs @@ -14,6 +14,8 @@ use kamu_accounts::AccountService; use kamu_core::auth::{ClassifyByAllowanceIdsResponse, DatasetAction}; use kamu_core::{ self as domain, + DownstreamDependency, + GetDatasetDownstreamDependenciesUseCase, MetadataChainExt, SearchSetAttachmentsVisitor, SearchSetInfoVisitor, @@ -186,84 +188,24 @@ impl DatasetMetadata { &self, ctx: &Context<'_>, ) -> Result> { - let ( - dependency_graph_service, - dataset_action_authorizer, - dataset_entry_repository, - account_service, - ) = from_catalog_n!( - ctx, - dyn domain::DependencyGraphService, - dyn kamu_core::auth::DatasetActionAuthorizer, - dyn kamu_datasets::DatasetEntryRepository, - dyn AccountService - ); + let get_dataset_downstream_dependencies_use_case = + from_catalog_n!(ctx, dyn GetDatasetDownstreamDependenciesUseCase); - use tokio_stream::StreamExt; - - // TODO: PERF: chunk the stream - let downstream_dependency_ids = dependency_graph_service - .get_downstream_dependencies(&self.dataset_handle.id) + let downstream_dependencies = get_dataset_downstream_dependencies_use_case + .execute(&self.dataset_handle.id) .await .int_err()? - .collect::>() - .await; - - let mut downstream_dependencies = Vec::with_capacity(downstream_dependency_ids.len()); - - // Cut off datasets that we don't have access to - let authorized_ids = dataset_action_authorizer - .classify_dataset_ids_by_allowance(downstream_dependency_ids, DatasetAction::Read) - .await? - .authorized_ids; - - let DatasetEntriesResolution { - resolved_entries, - unresolved_entries, - } = dataset_entry_repository - .get_multiple_dataset_entries(&authorized_ids) - .await - .int_err()?; - - downstream_dependencies.extend( - unresolved_entries - .into_iter() - .map(DependencyDatasetResult::not_accessible), - ); - - let owner_ids = resolved_entries - .iter() - .fold(HashSet::new(), |mut acc, entry| { - acc.insert(entry.owner_id.clone()); - acc - }); - let account_map = account_service - .get_account_map(owner_ids.into_iter().collect()) - .await - .int_err()?; - - for dataset_entry in resolved_entries { - let maybe_account = account_map.get(&dataset_entry.owner_id); - if let Some(account) = maybe_account { - let dataset_handle = odf::DatasetHandle { - id: dataset_entry.id, - alias: odf::DatasetAlias::new( - Some(account.account_name.clone()), - dataset_entry.name, - ), - }; - let dataset = Dataset::new(Account::from_account(account.clone()), dataset_handle); - - downstream_dependencies.push(DependencyDatasetResult::accessible(dataset)); - } else { - tracing::warn!( - "Downstream owner's account not found for dataset: {:?}", - &dataset_entry - ); - downstream_dependencies - .push(DependencyDatasetResult::not_accessible(dataset_entry.id)); - } - } + .into_iter() + .map(|dependency| match dependency { + DownstreamDependency::Resolved(r) => { + let account = Account::new(r.owner_id.into(), r.owner_name.into()); + let dataset = Dataset::new(account, r.dataset_handle); + + DependencyDatasetResult::accessible(dataset) + } + DownstreamDependency::Unresolved(id) => DependencyDatasetResult::not_accessible(id), + }) + .collect::>(); Ok(downstream_dependencies) } diff --git a/src/app/cli/src/app.rs b/src/app/cli/src/app.rs index 4203abca9b..78deaf8b93 100644 --- a/src/app/cli/src/app.rs +++ b/src/app/cli/src/app.rs @@ -468,6 +468,7 @@ pub fn configure_base_catalog( b.add::(); b.add::(); b.add::(); + b.add::(); b.add::(); diff --git a/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs b/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs new file mode 100644 index 0000000000..719ab9454e --- /dev/null +++ b/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs @@ -0,0 +1,66 @@ +// 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. + +use internal_error::InternalError; +use opendatafabric as odf; +use thiserror::Error; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +// TODO: Private Datasets: tests +#[async_trait::async_trait] +pub trait GetDatasetDownstreamDependenciesUseCase: Send + Sync { + async fn execute( + &self, + dataset_id: &odf::DatasetID, + ) -> Result, GetDatasetDownstreamDependenciesError>; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[derive(Debug)] +pub struct ResolvedDownstreamDependency { + pub dataset_handle: odf::DatasetHandle, + pub owner_id: odf::AccountID, + pub owner_name: odf::AccountName, +} + +#[derive(Debug)] +pub enum DownstreamDependency { + Resolved(ResolvedDownstreamDependency), + Unresolved(odf::DatasetID), +} + +impl DownstreamDependency { + pub fn resolved( + dataset_handle: odf::DatasetHandle, + owner_id: odf::AccountID, + owner_name: odf::AccountName, + ) -> Self { + Self::Resolved(ResolvedDownstreamDependency { + dataset_handle, + owner_id, + owner_name, + }) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[derive(Error, Debug)] +pub enum GetDatasetDownstreamDependenciesError { + #[error(transparent)] + Internal( + #[from] + #[backtrace] + InternalError, + ), +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/core/src/use_cases/mod.rs b/src/domain/core/src/use_cases/mod.rs index 27f41b70b2..99d72d7357 100644 --- a/src/domain/core/src/use_cases/mod.rs +++ b/src/domain/core/src/use_cases/mod.rs @@ -13,6 +13,7 @@ mod compact_dataset_use_case; mod create_dataset_from_snapshot_use_case; mod create_dataset_use_case; mod delete_dataset_use_case; +mod get_dataset_downstream_dependencies_use_case; mod pull_dataset_use_case; mod push_dataset_use_case; mod rename_dataset_use_case; @@ -26,6 +27,7 @@ pub use compact_dataset_use_case::*; pub use create_dataset_from_snapshot_use_case::*; pub use create_dataset_use_case::*; pub use delete_dataset_use_case::*; +pub use get_dataset_downstream_dependencies_use_case::*; pub use pull_dataset_use_case::*; pub use push_dataset_use_case::*; pub use rename_dataset_use_case::*; diff --git a/src/domain/core/src/utils/tenancy_config.rs b/src/domain/core/src/utils/tenancy_config.rs index ce9d8c2b5d..306105ffe7 100644 --- a/src/domain/core/src/utils/tenancy_config.rs +++ b/src/domain/core/src/utils/tenancy_config.rs @@ -7,6 +7,8 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. +use opendatafabric as odf; + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// #[derive(Debug, Copy, Clone, Eq, PartialEq)] @@ -15,6 +17,19 @@ pub enum TenancyConfig { MultiTenant, } +impl TenancyConfig { + pub fn make_alias( + &self, + owner_name: odf::AccountName, + dataset_name: odf::DatasetName, + ) -> odf::DatasetAlias { + match *self { + TenancyConfig::MultiTenant => odf::DatasetAlias::new(Some(owner_name), dataset_name), + TenancyConfig::SingleTenant => odf::DatasetAlias::new(None, dataset_name), + } + } +} + impl Default for TenancyConfig { fn default() -> Self { Self::SingleTenant diff --git a/src/domain/datasets/domain/src/repos/dataset_entry_repository.rs b/src/domain/datasets/domain/src/repos/dataset_entry_repository.rs index 4809e33200..48acf4548a 100644 --- a/src/domain/datasets/domain/src/repos/dataset_entry_repository.rs +++ b/src/domain/datasets/domain/src/repos/dataset_entry_repository.rs @@ -7,6 +7,8 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. +use std::collections::HashSet; + use database_common::PaginationOpts; use internal_error::InternalError; use opendatafabric as odf; @@ -84,6 +86,17 @@ pub struct DatasetEntriesResolution { pub unresolved_entries: Vec, } +impl DatasetEntriesResolution { + pub fn resolved_entries_owner_ids(&self) -> HashSet { + self.resolved_entries + .iter() + .fold(HashSet::new(), |mut acc, entry| { + acc.insert(entry.owner_id.clone()); + acc + }) + } +} + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Errors //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/datasets/services/src/dataset_entry_service_impl.rs b/src/domain/datasets/services/src/dataset_entry_service_impl.rs index d384b081af..b17b57347d 100644 --- a/src/domain/datasets/services/src/dataset_entry_service_impl.rs +++ b/src/domain/datasets/services/src/dataset_entry_service_impl.rs @@ -204,7 +204,8 @@ impl DatasetEntryServiceImpl { // Form DatasetHandle handles.push(odf::DatasetHandle::new( entry.id.clone(), - self.make_alias(owner_name.clone(), entry.name.clone()), + self.tenancy_config + .make_alias(owner_name.clone(), entry.name.clone()), )); } } @@ -309,17 +310,6 @@ impl DatasetEntryServiceImpl { .int_err()?, }) } - - fn make_alias( - &self, - owner_name: odf::AccountName, - dataset_name: odf::DatasetName, - ) -> odf::DatasetAlias { - match *self.tenancy_config { - TenancyConfig::MultiTenant => odf::DatasetAlias::new(Some(owner_name), dataset_name), - TenancyConfig::SingleTenant => odf::DatasetAlias::new(None, dataset_name), - } - } } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -467,7 +457,8 @@ impl DatasetRegistry for DatasetEntryServiceImpl { let owner_name = self.resolve_account_name_by_id(&entry.owner_id).await?; Ok(odf::DatasetHandle::new( entry.id.clone(), - self.make_alias(owner_name, entry.name.clone()), + self.tenancy_config + .make_alias(owner_name, entry.name.clone()), )) } Err(GetDatasetEntryError::NotFound(_)) => { diff --git a/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs b/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs new file mode 100644 index 0000000000..71aeb04b38 --- /dev/null +++ b/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs @@ -0,0 +1,135 @@ +// 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. + +use std::sync::Arc; + +use dill::{component, interface}; +use internal_error::ResultIntoInternal; +use kamu_accounts::AccountService; +use kamu_core::auth::{DatasetAction, DatasetActionAuthorizer}; +use kamu_core::{ + DependencyGraphService, + DownstreamDependency, + GetDatasetDownstreamDependenciesError, + GetDatasetDownstreamDependenciesUseCase, + TenancyConfig, +}; +use kamu_datasets::DatasetEntryRepository; +use opendatafabric as odf; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[component(pub)] +#[interface(dyn GetDatasetDownstreamDependenciesUseCase)] +pub struct GetDatasetDownstreamDependenciesUseCaseImpl { + tenancy_config: Arc, + dependency_graph_service: Arc, + dataset_action_authorizer: Arc, + dataset_entry_repository: Arc, + account_service: Arc, +} + +impl GetDatasetDownstreamDependenciesUseCaseImpl { + pub fn new( + tenancy_config: Arc, + dependency_graph_service: Arc, + dataset_action_authorizer: Arc, + dataset_entry_repository: Arc, + account_service: Arc, + ) -> Self { + Self { + tenancy_config, + dependency_graph_service, + dataset_action_authorizer, + dataset_entry_repository, + account_service, + } + } +} + +#[async_trait::async_trait] +impl GetDatasetDownstreamDependenciesUseCase for GetDatasetDownstreamDependenciesUseCaseImpl { + #[tracing::instrument( + level = "info", + name = "GetDatasetDownstreamDependenciesUseCase::execute", + skip_all, + fields(dataset_id) + )] + async fn execute( + &self, + dataset_id: &odf::DatasetID, + ) -> Result, GetDatasetDownstreamDependenciesError> { + use tokio_stream::StreamExt; + + // TODO: PERF: chunk the stream + let downstream_dependency_ids = self + .dependency_graph_service + .get_downstream_dependencies(dataset_id) + .await + .int_err()? + .collect::>() + .await; + + let mut downstream_dependencies = Vec::with_capacity(downstream_dependency_ids.len()); + + // Cut off datasets that we don't have access to + let authorized_ids = self + .dataset_action_authorizer + .classify_dataset_ids_by_allowance(downstream_dependency_ids, DatasetAction::Read) + .await? + .authorized_ids; + + let dataset_entries_resolution = self + .dataset_entry_repository + .get_multiple_dataset_entries(&authorized_ids) + .await + .int_err()?; + + let owner_ids = dataset_entries_resolution.resolved_entries_owner_ids(); + + downstream_dependencies.extend( + dataset_entries_resolution + .unresolved_entries + .into_iter() + .map(DownstreamDependency::Unresolved), + ); + + let account_map = self + .account_service + .get_account_map(owner_ids.into_iter().collect()) + .await + .int_err()?; + + for dataset_entry in dataset_entries_resolution.resolved_entries { + let maybe_account = account_map.get(&dataset_entry.owner_id); + if let Some(account) = maybe_account { + let dataset_alias = self + .tenancy_config + .make_alias(account.account_name.clone(), dataset_entry.name); + let dataset_handle = odf::DatasetHandle::new(dataset_entry.id, dataset_alias); + + downstream_dependencies.push(DownstreamDependency::resolved( + dataset_handle, + account.id.clone(), + account.account_name.clone(), + )); + } else { + tracing::warn!( + "Downstream owner's account not found for dataset: {:?}", + &dataset_entry + ); + downstream_dependencies.push(DownstreamDependency::Unresolved(dataset_entry.id)); + } + } + + Ok(downstream_dependencies) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/infra/core/src/use_cases/mod.rs b/src/infra/core/src/use_cases/mod.rs index c9a04aecb4..b2ab035622 100644 --- a/src/infra/core/src/use_cases/mod.rs +++ b/src/infra/core/src/use_cases/mod.rs @@ -13,6 +13,7 @@ mod compact_dataset_use_case_impl; mod create_dataset_from_snapshot_use_case_impl; mod create_dataset_use_case_impl; mod delete_dataset_use_case_impl; +mod get_dataset_downstream_dependencies_use_case_impl; mod pull_dataset_use_case_impl; mod push_dataset_use_case_impl; mod rename_dataset_use_case_impl; @@ -26,6 +27,7 @@ pub use compact_dataset_use_case_impl::*; pub use create_dataset_from_snapshot_use_case_impl::*; pub use create_dataset_use_case_impl::*; pub use delete_dataset_use_case_impl::*; +pub use get_dataset_downstream_dependencies_use_case_impl::*; pub use pull_dataset_use_case_impl::*; pub use push_dataset_use_case_impl::*; pub use rename_dataset_use_case_impl::*; From 993e6adbf92ba023fcd6563800696a34541546ce Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 19:31:30 +0200 Subject: [PATCH 16/20] GetDatasetUpstreamDependenciesUseCase: extract --- .../src/queries/datasets/dataset_metadata.rs | 107 +++---------- src/app/cli/src/app.rs | 1 + ...ataset_downstream_dependencies_use_case.rs | 12 +- ..._dataset_upstream_dependencies_use_case.rs | 39 +++++ src/domain/core/src/use_cases/mod.rs | 2 + ...t_downstream_dependencies_use_case_impl.rs | 10 +- ...set_upstream_dependencies_use_case_impl.rs | 140 ++++++++++++++++++ src/infra/core/src/use_cases/mod.rs | 2 + 8 files changed, 213 insertions(+), 100 deletions(-) create mode 100644 src/domain/core/src/use_cases/get_dataset_upstream_dependencies_use_case.rs create mode 100644 src/infra/core/src/use_cases/get_dataset_upstream_dependencies_use_case_impl.rs diff --git a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs index 8106258940..e92b222589 100644 --- a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs +++ b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs @@ -7,22 +7,18 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use std::collections::HashSet; - use chrono::prelude::*; -use kamu_accounts::AccountService; -use kamu_core::auth::{ClassifyByAllowanceIdsResponse, DatasetAction}; use kamu_core::{ self as domain, - DownstreamDependency, + DatasetDependency, GetDatasetDownstreamDependenciesUseCase, + GetDatasetUpstreamDependenciesUseCase, MetadataChainExt, SearchSetAttachmentsVisitor, SearchSetInfoVisitor, SearchSetLicenseVisitor, SearchSetVocabVisitor, }; -use kamu_datasets::DatasetEntriesResolution; use opendatafabric as odf; use crate::prelude::*; @@ -92,91 +88,24 @@ impl DatasetMetadata { &self, ctx: &Context<'_>, ) -> Result> { - let ( - dependency_graph_service, - dataset_action_authorizer, - dataset_entry_repository, - account_service, - ) = from_catalog_n!( - ctx, - dyn domain::DependencyGraphService, - dyn kamu_core::auth::DatasetActionAuthorizer, - dyn kamu_datasets::DatasetEntryRepository, - dyn AccountService - ); + let get_dataset_upstream_dependencies_use_case = + from_catalog_n!(ctx, dyn GetDatasetUpstreamDependenciesUseCase); - use tokio_stream::StreamExt; - - // TODO: PERF: chunk the stream - let upstream_dependency_ids = dependency_graph_service - .get_upstream_dependencies(&self.dataset_handle.id) + let upstream_dependencies = get_dataset_upstream_dependencies_use_case + .execute(&self.dataset_handle.id) .await .int_err()? - .collect::>() - .await; - - let mut upstream_dependencies = Vec::with_capacity(upstream_dependency_ids.len()); - - let ClassifyByAllowanceIdsResponse { - authorized_ids, - unauthorized_ids_with_errors, - } = dataset_action_authorizer - .classify_dataset_ids_by_allowance(upstream_dependency_ids, DatasetAction::Read) - .await?; - - upstream_dependencies.extend(unauthorized_ids_with_errors.into_iter().map( - |(unauthorized_dataset_id, _)| { - DependencyDatasetResult::not_accessible(unauthorized_dataset_id) - }, - )); - - let DatasetEntriesResolution { - resolved_entries, - unresolved_entries, - } = dataset_entry_repository - .get_multiple_dataset_entries(&authorized_ids) - .await - .int_err()?; - - upstream_dependencies.extend( - unresolved_entries - .into_iter() - .map(DependencyDatasetResult::not_accessible), - ); - - let owner_ids = resolved_entries - .iter() - .fold(HashSet::new(), |mut acc, entry| { - acc.insert(entry.owner_id.clone()); - acc - }); - let account_map = account_service - .get_account_map(owner_ids.into_iter().collect()) - .await - .int_err()?; + .into_iter() + .map(|dependency| match dependency { + DatasetDependency::Resolved(r) => { + let account = Account::new(r.owner_id.into(), r.owner_name.into()); + let dataset = Dataset::new(account, r.dataset_handle); - for dataset_entry in resolved_entries { - let maybe_account = account_map.get(&dataset_entry.owner_id); - if let Some(account) = maybe_account { - let dataset_handle = odf::DatasetHandle { - id: dataset_entry.id, - alias: odf::DatasetAlias::new( - Some(account.account_name.clone()), - dataset_entry.name, - ), - }; - let dataset = Dataset::new(Account::from_account(account.clone()), dataset_handle); - - upstream_dependencies.push(DependencyDatasetResult::accessible(dataset)); - } else { - tracing::warn!( - "Upstream owner's account not found for dataset: {:?}", - &dataset_entry - ); - upstream_dependencies - .push(DependencyDatasetResult::not_accessible(dataset_entry.id)); - } - } + DependencyDatasetResult::accessible(dataset) + } + DatasetDependency::Unresolved(id) => DependencyDatasetResult::not_accessible(id), + }) + .collect::>(); Ok(upstream_dependencies) } @@ -197,13 +126,13 @@ impl DatasetMetadata { .int_err()? .into_iter() .map(|dependency| match dependency { - DownstreamDependency::Resolved(r) => { + DatasetDependency::Resolved(r) => { let account = Account::new(r.owner_id.into(), r.owner_name.into()); let dataset = Dataset::new(account, r.dataset_handle); DependencyDatasetResult::accessible(dataset) } - DownstreamDependency::Unresolved(id) => DependencyDatasetResult::not_accessible(id), + DatasetDependency::Unresolved(id) => DependencyDatasetResult::not_accessible(id), }) .collect::>(); diff --git a/src/app/cli/src/app.rs b/src/app/cli/src/app.rs index 78deaf8b93..54a2aed33b 100644 --- a/src/app/cli/src/app.rs +++ b/src/app/cli/src/app.rs @@ -469,6 +469,7 @@ pub fn configure_base_catalog( b.add::(); b.add::(); b.add::(); + b.add::(); b.add::(); diff --git a/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs b/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs index 719ab9454e..c30418fad3 100644 --- a/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs +++ b/src/domain/core/src/use_cases/get_dataset_downstream_dependencies_use_case.rs @@ -19,31 +19,31 @@ pub trait GetDatasetDownstreamDependenciesUseCase: Send + Sync { async fn execute( &self, dataset_id: &odf::DatasetID, - ) -> Result, GetDatasetDownstreamDependenciesError>; + ) -> Result, GetDatasetDownstreamDependenciesError>; } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// #[derive(Debug)] -pub struct ResolvedDownstreamDependency { +pub struct ResolvedDatasetDependency { pub dataset_handle: odf::DatasetHandle, pub owner_id: odf::AccountID, pub owner_name: odf::AccountName, } #[derive(Debug)] -pub enum DownstreamDependency { - Resolved(ResolvedDownstreamDependency), +pub enum DatasetDependency { + Resolved(ResolvedDatasetDependency), Unresolved(odf::DatasetID), } -impl DownstreamDependency { +impl DatasetDependency { pub fn resolved( dataset_handle: odf::DatasetHandle, owner_id: odf::AccountID, owner_name: odf::AccountName, ) -> Self { - Self::Resolved(ResolvedDownstreamDependency { + Self::Resolved(ResolvedDatasetDependency { dataset_handle, owner_id, owner_name, diff --git a/src/domain/core/src/use_cases/get_dataset_upstream_dependencies_use_case.rs b/src/domain/core/src/use_cases/get_dataset_upstream_dependencies_use_case.rs new file mode 100644 index 0000000000..f4546514be --- /dev/null +++ b/src/domain/core/src/use_cases/get_dataset_upstream_dependencies_use_case.rs @@ -0,0 +1,39 @@ +// 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. + +use internal_error::InternalError; +use opendatafabric as odf; +use thiserror::Error; + +use crate::DatasetDependency; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +// TODO: Private Datasets: tests +#[async_trait::async_trait] +pub trait GetDatasetUpstreamDependenciesUseCase: Send + Sync { + async fn execute( + &self, + dataset_id: &odf::DatasetID, + ) -> Result, GetDatasetUpstreamDependenciesError>; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[derive(Error, Debug)] +pub enum GetDatasetUpstreamDependenciesError { + #[error(transparent)] + Internal( + #[from] + #[backtrace] + InternalError, + ), +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/core/src/use_cases/mod.rs b/src/domain/core/src/use_cases/mod.rs index 99d72d7357..9ce84af135 100644 --- a/src/domain/core/src/use_cases/mod.rs +++ b/src/domain/core/src/use_cases/mod.rs @@ -14,6 +14,7 @@ mod create_dataset_from_snapshot_use_case; mod create_dataset_use_case; mod delete_dataset_use_case; mod get_dataset_downstream_dependencies_use_case; +mod get_dataset_upstream_dependencies_use_case; mod pull_dataset_use_case; mod push_dataset_use_case; mod rename_dataset_use_case; @@ -28,6 +29,7 @@ pub use create_dataset_from_snapshot_use_case::*; pub use create_dataset_use_case::*; pub use delete_dataset_use_case::*; pub use get_dataset_downstream_dependencies_use_case::*; +pub use get_dataset_upstream_dependencies_use_case::*; pub use pull_dataset_use_case::*; pub use push_dataset_use_case::*; pub use rename_dataset_use_case::*; diff --git a/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs b/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs index 71aeb04b38..ff005f2fce 100644 --- a/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs +++ b/src/infra/core/src/use_cases/get_dataset_downstream_dependencies_use_case_impl.rs @@ -14,8 +14,8 @@ use internal_error::ResultIntoInternal; use kamu_accounts::AccountService; use kamu_core::auth::{DatasetAction, DatasetActionAuthorizer}; use kamu_core::{ + DatasetDependency, DependencyGraphService, - DownstreamDependency, GetDatasetDownstreamDependenciesError, GetDatasetDownstreamDependenciesUseCase, TenancyConfig, @@ -64,7 +64,7 @@ impl GetDatasetDownstreamDependenciesUseCase for GetDatasetDownstreamDependencie async fn execute( &self, dataset_id: &odf::DatasetID, - ) -> Result, GetDatasetDownstreamDependenciesError> { + ) -> Result, GetDatasetDownstreamDependenciesError> { use tokio_stream::StreamExt; // TODO: PERF: chunk the stream @@ -97,7 +97,7 @@ impl GetDatasetDownstreamDependenciesUseCase for GetDatasetDownstreamDependencie dataset_entries_resolution .unresolved_entries .into_iter() - .map(DownstreamDependency::Unresolved), + .map(DatasetDependency::Unresolved), ); let account_map = self @@ -114,7 +114,7 @@ impl GetDatasetDownstreamDependenciesUseCase for GetDatasetDownstreamDependencie .make_alias(account.account_name.clone(), dataset_entry.name); let dataset_handle = odf::DatasetHandle::new(dataset_entry.id, dataset_alias); - downstream_dependencies.push(DownstreamDependency::resolved( + downstream_dependencies.push(DatasetDependency::resolved( dataset_handle, account.id.clone(), account.account_name.clone(), @@ -124,7 +124,7 @@ impl GetDatasetDownstreamDependenciesUseCase for GetDatasetDownstreamDependencie "Downstream owner's account not found for dataset: {:?}", &dataset_entry ); - downstream_dependencies.push(DownstreamDependency::Unresolved(dataset_entry.id)); + downstream_dependencies.push(DatasetDependency::Unresolved(dataset_entry.id)); } } diff --git a/src/infra/core/src/use_cases/get_dataset_upstream_dependencies_use_case_impl.rs b/src/infra/core/src/use_cases/get_dataset_upstream_dependencies_use_case_impl.rs new file mode 100644 index 0000000000..2b43f2bff4 --- /dev/null +++ b/src/infra/core/src/use_cases/get_dataset_upstream_dependencies_use_case_impl.rs @@ -0,0 +1,140 @@ +// 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. + +use std::sync::Arc; + +use dill::{component, interface}; +use internal_error::ResultIntoInternal; +use kamu_accounts::AccountService; +use kamu_core::auth::{ClassifyByAllowanceIdsResponse, DatasetAction, DatasetActionAuthorizer}; +use kamu_core::{ + DatasetDependency, + DependencyGraphService, + GetDatasetUpstreamDependenciesError, + GetDatasetUpstreamDependenciesUseCase, + TenancyConfig, +}; +use kamu_datasets::DatasetEntryRepository; +use opendatafabric as odf; + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#[component(pub)] +#[interface(dyn GetDatasetUpstreamDependenciesUseCase)] +pub struct GetDatasetUpstreamDependenciesUseCaseImpl { + tenancy_config: Arc, + dependency_graph_service: Arc, + dataset_action_authorizer: Arc, + dataset_entry_repository: Arc, + account_service: Arc, +} + +impl GetDatasetUpstreamDependenciesUseCaseImpl { + pub fn new( + tenancy_config: Arc, + dependency_graph_service: Arc, + dataset_action_authorizer: Arc, + dataset_entry_repository: Arc, + account_service: Arc, + ) -> Self { + Self { + tenancy_config, + dependency_graph_service, + dataset_action_authorizer, + dataset_entry_repository, + account_service, + } + } +} + +#[async_trait::async_trait] +impl GetDatasetUpstreamDependenciesUseCase for GetDatasetUpstreamDependenciesUseCaseImpl { + #[tracing::instrument( + level = "info", + name = "GetDatasetUpstreamDependenciesUseCase::execute", + skip_all, + fields(dataset_id) + )] + async fn execute( + &self, + dataset_id: &odf::DatasetID, + ) -> Result, GetDatasetUpstreamDependenciesError> { + use tokio_stream::StreamExt; + + // TODO: PERF: chunk the stream + let upstream_dependency_ids = self + .dependency_graph_service + .get_upstream_dependencies(dataset_id) + .await + .int_err()? + .collect::>() + .await; + + let mut upstream_dependencies = Vec::with_capacity(upstream_dependency_ids.len()); + + let ClassifyByAllowanceIdsResponse { + authorized_ids, + unauthorized_ids_with_errors, + } = self + .dataset_action_authorizer + .classify_dataset_ids_by_allowance(upstream_dependency_ids, DatasetAction::Read) + .await?; + + upstream_dependencies.extend(unauthorized_ids_with_errors.into_iter().map( + |(unauthorized_dataset_id, _)| DatasetDependency::Unresolved(unauthorized_dataset_id), + )); + + let dataset_entries_resolution = self + .dataset_entry_repository + .get_multiple_dataset_entries(&authorized_ids) + .await + .int_err()?; + + let owner_ids = dataset_entries_resolution.resolved_entries_owner_ids(); + + upstream_dependencies.extend( + dataset_entries_resolution + .unresolved_entries + .into_iter() + .map(DatasetDependency::Unresolved), + ); + + let account_map = self + .account_service + .get_account_map(owner_ids.into_iter().collect()) + .await + .int_err()?; + + for dataset_entry in dataset_entries_resolution.resolved_entries { + let maybe_account = account_map.get(&dataset_entry.owner_id); + if let Some(account) = maybe_account { + let dataset_alias = self + .tenancy_config + .make_alias(account.account_name.clone(), dataset_entry.name); + let dataset_handle = odf::DatasetHandle::new(dataset_entry.id, dataset_alias); + + upstream_dependencies.push(DatasetDependency::resolved( + dataset_handle, + account.id.clone(), + account.account_name.clone(), + )); + } else { + tracing::warn!( + "Upstream owner's account not found for dataset: {:?}", + &dataset_entry + ); + upstream_dependencies.push(DatasetDependency::Unresolved(dataset_entry.id)); + } + } + + Ok(upstream_dependencies) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/infra/core/src/use_cases/mod.rs b/src/infra/core/src/use_cases/mod.rs index b2ab035622..81bdde7a54 100644 --- a/src/infra/core/src/use_cases/mod.rs +++ b/src/infra/core/src/use_cases/mod.rs @@ -14,6 +14,7 @@ mod create_dataset_from_snapshot_use_case_impl; mod create_dataset_use_case_impl; mod delete_dataset_use_case_impl; mod get_dataset_downstream_dependencies_use_case_impl; +mod get_dataset_upstream_dependencies_use_case_impl; mod pull_dataset_use_case_impl; mod push_dataset_use_case_impl; mod rename_dataset_use_case_impl; @@ -28,6 +29,7 @@ pub use create_dataset_from_snapshot_use_case_impl::*; pub use create_dataset_use_case_impl::*; pub use delete_dataset_use_case_impl::*; pub use get_dataset_downstream_dependencies_use_case_impl::*; +pub use get_dataset_upstream_dependencies_use_case_impl::*; pub use pull_dataset_use_case_impl::*; pub use push_dataset_use_case_impl::*; pub use rename_dataset_use_case_impl::*; From 272b151e192a1d977d8c09b2b0de39ad1fd040b2 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 20:22:38 +0200 Subject: [PATCH 17/20] AccountServiceImpl::all_accounts(): absorb list_all_accounts() method --- .../domain/src/services/account_service.rs | 14 ------- .../services/src/account_service_impl.rs | 41 ++++++++----------- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/src/domain/accounts/domain/src/services/account_service.rs b/src/domain/accounts/domain/src/services/account_service.rs index fedd5b13cc..c65b7df2f8 100644 --- a/src/domain/accounts/domain/src/services/account_service.rs +++ b/src/domain/accounts/domain/src/services/account_service.rs @@ -9,7 +9,6 @@ use std::collections::HashMap; -use database_common::{EntityPageListing, PaginationOpts}; use internal_error::InternalError; use opendatafabric as odf; use thiserror::Error; @@ -25,11 +24,6 @@ pub trait AccountService: Sync + Send { // TODO: Private Datasets: extract to AccountRegistry? fn all_accounts(&self) -> AccountPageStream; - async fn list_all_accounts( - &self, - pagination: PaginationOpts, - ) -> Result, ListAccountError>; - async fn get_account_map( &self, account_ids: Vec, @@ -40,14 +34,6 @@ pub trait AccountService: Sync + Send { // Error //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -#[derive(Debug, Error)] -pub enum ListAccountError { - #[error(transparent)] - Internal(#[from] InternalError), -} - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - #[derive(Debug, Error)] pub enum GetAccountMapError { #[error(transparent)] diff --git a/src/domain/accounts/services/src/account_service_impl.rs b/src/domain/accounts/services/src/account_service_impl.rs index f4ef33913c..11507cff34 100644 --- a/src/domain/accounts/services/src/account_service_impl.rs +++ b/src/domain/accounts/services/src/account_service_impl.rs @@ -10,7 +10,7 @@ use std::collections::HashMap; use std::sync::Arc; -use database_common::{EntityPageListing, EntityPageStreamer, PaginationOpts}; +use database_common::{EntityPageListing, EntityPageStreamer}; use dill::*; use internal_error::ResultIntoInternal; use kamu_accounts::{ @@ -20,7 +20,6 @@ use kamu_accounts::{ AccountService, GetAccountByIdError, GetAccountMapError, - ListAccountError, }; use opendatafabric as odf; @@ -45,31 +44,23 @@ impl AccountService for AccountServiceImpl { fn all_accounts(&self) -> AccountPageStream { EntityPageStreamer::default().into_stream( || async { Ok(()) }, - |_, pagination| { - let list_fut = self.list_all_accounts(pagination); - async { list_fut.await.int_err() } - }, - ) - } + move |_, pagination| async move { + use futures::TryStreamExt; - async fn list_all_accounts( - &self, - pagination: PaginationOpts, - ) -> Result, ListAccountError> { - use futures::TryStreamExt; + let total_count = self.account_repo.accounts_count().await.int_err()?; + let entries = self + .account_repo + .get_accounts(pagination) + .await + .try_collect() + .await?; - let total_count = self.account_repo.accounts_count().await.int_err()?; - let entries = self - .account_repo - .get_accounts(pagination) - .await - .try_collect() - .await?; - - Ok(EntityPageListing { - list: entries, - total_count, - }) + Ok(EntityPageListing { + list: entries, + total_count, + }) + }, + ) } async fn get_account_map( From b628d966463e7f11e946a9521d1c0639c586f98c Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 20:43:05 +0200 Subject: [PATCH 18/20] ExpensiveAccountRepository: extract trait --- Cargo.lock | 1 + src/domain/accounts/domain/Cargo.toml | 1 + .../domain/src/repos/account_repository.rs | 52 ++++++-- .../domain/src/services/account_service.rs | 3 +- .../services/src/account_service_impl.rs | 24 ---- .../auth-rebac/services/src/rebac_indexer.rs | 11 +- .../src/repos/inmem_account_repository.rs | 56 ++++---- .../src/repos/mysql_account_repository.rs | 120 +++++++++--------- .../src/repos/postgres_account_repository.rs | 120 +++++++++--------- .../src/repos/sqlite_account_repository.rs | 120 +++++++++--------- 10 files changed, 273 insertions(+), 235 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 066eee317e..bf658ea394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5388,6 +5388,7 @@ dependencies = [ "chrono", "crc32fast", "database-common", + "futures", "internal-error", "jsonwebtoken", "lazy_static", diff --git a/src/domain/accounts/domain/Cargo.toml b/src/domain/accounts/domain/Cargo.toml index fe320c9a5d..1182067c4f 100644 --- a/src/domain/accounts/domain/Cargo.toml +++ b/src/domain/accounts/domain/Cargo.toml @@ -37,6 +37,7 @@ async-trait = { version = "0.1", default-features = false } base32 = { version = "0.5" } chrono = { version = "0.4", default-features = false } crc32fast = { version = "1.4.2" } +futures = { version = "0.3", default-features = false } jsonwebtoken = "9" lazy_static = { version = "1" } merge = "0.1" diff --git a/src/domain/accounts/domain/src/repos/account_repository.rs b/src/domain/accounts/domain/src/repos/account_repository.rs index cea6ad3aa6..4987444e79 100644 --- a/src/domain/accounts/domain/src/repos/account_repository.rs +++ b/src/domain/accounts/domain/src/repos/account_repository.rs @@ -9,8 +9,8 @@ use std::fmt::Display; -use database_common::{EntityPageStream, PaginationOpts}; -use internal_error::InternalError; +use database_common::{EntityPageListing, EntityPageStream, EntityPageStreamer, PaginationOpts}; +use internal_error::{InternalError, ResultIntoInternal}; use opendatafabric::{AccountID, AccountName}; use thiserror::Error; @@ -20,14 +20,8 @@ use crate::Account; #[async_trait::async_trait] pub trait AccountRepository: Send + Sync { - // TODO: Private Datasets: tests - async fn accounts_count(&self) -> Result; - async fn create_account(&self, account: &Account) -> Result<(), CreateAccountError>; - // TODO: Private Datasets: tests - async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream; - async fn get_account_by_id( &self, account_id: &AccountID, @@ -61,6 +55,48 @@ pub trait AccountRepository: Send + Sync { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// TODO: Private Datasets: tests +#[async_trait::async_trait] +pub trait ExpensiveAccountRepository: AccountRepository { + async fn accounts_count(&self) -> Result; + + async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +// TODO: Private Datasets: tests +#[async_trait::async_trait] +pub trait ExpensiveAccountRepositoryExt: ExpensiveAccountRepository { + fn all_accounts(&self) -> AccountPageStream; +} + +#[async_trait::async_trait] +impl ExpensiveAccountRepositoryExt for T +where + T: ExpensiveAccountRepository, + T: ?Sized, +{ + fn all_accounts(&self) -> AccountPageStream { + EntityPageStreamer::default().into_stream( + || async { Ok(()) }, + move |_, pagination| async move { + use futures::TryStreamExt; + + let total_count = self.accounts_count().await.int_err()?; + let entries = self.get_accounts(pagination).await.try_collect().await?; + + Ok(EntityPageListing { + list: entries, + total_count, + }) + }, + ) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + pub type AccountPageStream<'a> = EntityPageStream<'a, Account>; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/accounts/domain/src/services/account_service.rs b/src/domain/accounts/domain/src/services/account_service.rs index c65b7df2f8..32f4a87679 100644 --- a/src/domain/accounts/domain/src/services/account_service.rs +++ b/src/domain/accounts/domain/src/services/account_service.rs @@ -13,7 +13,7 @@ use internal_error::InternalError; use opendatafabric as odf; use thiserror::Error; -use crate::{Account, AccountPageStream}; +use crate::Account; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -22,7 +22,6 @@ use crate::{Account, AccountPageStream}; #[async_trait::async_trait] pub trait AccountService: Sync + Send { // TODO: Private Datasets: extract to AccountRegistry? - fn all_accounts(&self) -> AccountPageStream; async fn get_account_map( &self, diff --git a/src/domain/accounts/services/src/account_service_impl.rs b/src/domain/accounts/services/src/account_service_impl.rs index 11507cff34..a455698e59 100644 --- a/src/domain/accounts/services/src/account_service_impl.rs +++ b/src/domain/accounts/services/src/account_service_impl.rs @@ -10,12 +10,10 @@ use std::collections::HashMap; use std::sync::Arc; -use database_common::{EntityPageListing, EntityPageStreamer}; use dill::*; use internal_error::ResultIntoInternal; use kamu_accounts::{ Account, - AccountPageStream, AccountRepository, AccountService, GetAccountByIdError, @@ -41,28 +39,6 @@ impl AccountServiceImpl { #[async_trait::async_trait] impl AccountService for AccountServiceImpl { - fn all_accounts(&self) -> AccountPageStream { - EntityPageStreamer::default().into_stream( - || async { Ok(()) }, - move |_, pagination| async move { - use futures::TryStreamExt; - - let total_count = self.account_repo.accounts_count().await.int_err()?; - let entries = self - .account_repo - .get_accounts(pagination) - .await - .try_collect() - .await?; - - Ok(EntityPageListing { - list: entries, - total_count, - }) - }, - ) - } - async fn get_account_map( &self, account_ids: Vec, diff --git a/src/domain/auth-rebac/services/src/rebac_indexer.rs b/src/domain/auth-rebac/services/src/rebac_indexer.rs index c179f014d2..b03952ff5d 100644 --- a/src/domain/auth-rebac/services/src/rebac_indexer.rs +++ b/src/domain/auth-rebac/services/src/rebac_indexer.rs @@ -14,7 +14,8 @@ use dill::{component, interface, meta}; use init_on_startup::{InitOnStartup, InitOnStartupMeta}; use internal_error::{InternalError, ResultIntoInternal}; use kamu_accounts::{ - AccountService, + ExpensiveAccountRepository, + ExpensiveAccountRepositoryExt, PredefinedAccountsConfig, JOB_KAMU_ACCOUNTS_PREDEFINED_ACCOUNTS_REGISTRATOR, }; @@ -36,7 +37,7 @@ pub struct RebacIndexer { rebac_repo: Arc, rebac_service: Arc, dataset_entry_service: Arc, - account_service: Arc, + expensive_account_repo: Arc, predefined_accounts_config: Arc, } @@ -55,14 +56,14 @@ impl RebacIndexer { rebac_repo: Arc, rebac_service: Arc, dataset_entry_service: Arc, - account_service: Arc, + expensive_account_repo: Arc, predefined_accounts_config: Arc, ) -> Self { Self { rebac_repo, rebac_service, dataset_entry_service, - account_service, + expensive_account_repo, predefined_accounts_config, } } @@ -117,7 +118,7 @@ impl RebacIndexer { ) -> Result { use futures::TryStreamExt; - let mut accounts_stream = self.account_service.all_accounts(); + let mut accounts_stream = self.expensive_account_repo.all_accounts(); let predefined_accounts_map = self.predefined_accounts_config.predefined.iter().fold( HashMap::new(), diff --git a/src/infra/accounts/inmem/src/repos/inmem_account_repository.rs b/src/infra/accounts/inmem/src/repos/inmem_account_repository.rs index 829ecaf49f..d115335c74 100644 --- a/src/infra/accounts/inmem/src/repos/inmem_account_repository.rs +++ b/src/infra/accounts/inmem/src/repos/inmem_account_repository.rs @@ -47,6 +47,7 @@ impl State { #[component(pub)] #[interface(dyn AccountRepository)] +#[interface(dyn ExpensiveAccountRepository)] #[interface(dyn PasswordHashRepository)] #[scope(Singleton)] impl InMemoryAccountRepository { @@ -61,14 +62,6 @@ impl InMemoryAccountRepository { #[async_trait::async_trait] impl AccountRepository for InMemoryAccountRepository { - async fn accounts_count(&self) -> Result { - let readable_state = self.state.lock().unwrap(); - - let accounts_count = readable_state.accounts_by_id.len(); - - Ok(accounts_count) - } - async fn create_account(&self, account: &Account) -> Result<(), CreateAccountError> { let mut guard = self.state.lock().unwrap(); if guard.accounts_by_id.contains_key(&account.id) { @@ -114,23 +107,6 @@ impl AccountRepository for InMemoryAccountRepository { Ok(()) } - async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { - let dataset_entries_page = { - let readable_state = self.state.lock().unwrap(); - - readable_state - .accounts_by_id - .values() - .skip(pagination.offset) - .take(pagination.limit) - .cloned() - .map(Ok) - .collect::>() - }; - - Box::pin(futures::stream::iter(dataset_entries_page)) - } - async fn get_account_by_id( &self, account_id: &AccountID, @@ -213,6 +189,36 @@ impl AccountRepository for InMemoryAccountRepository { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +#[async_trait::async_trait] +impl ExpensiveAccountRepository for InMemoryAccountRepository { + async fn accounts_count(&self) -> Result { + let readable_state = self.state.lock().unwrap(); + + let accounts_count = readable_state.accounts_by_id.len(); + + Ok(accounts_count) + } + + async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { + let dataset_entries_page = { + let readable_state = self.state.lock().unwrap(); + + readable_state + .accounts_by_id + .values() + .skip(pagination.offset) + .take(pagination.limit) + .cloned() + .map(Ok) + .collect::>() + }; + + Box::pin(futures::stream::iter(dataset_entries_page)) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + #[async_trait::async_trait] impl PasswordHashRepository for InMemoryAccountRepository { async fn save_password_hash( diff --git a/src/infra/accounts/mysql/src/repos/mysql_account_repository.rs b/src/infra/accounts/mysql/src/repos/mysql_account_repository.rs index 355f9cb59c..5431c15d51 100644 --- a/src/infra/accounts/mysql/src/repos/mysql_account_repository.rs +++ b/src/infra/accounts/mysql/src/repos/mysql_account_repository.rs @@ -23,6 +23,7 @@ pub struct MySqlAccountRepository { #[component(pub)] #[interface(dyn AccountRepository)] +#[interface(dyn ExpensiveAccountRepository)] #[interface(dyn PasswordHashRepository)] impl MySqlAccountRepository { pub fn new(transaction: TransactionRef) -> Self { @@ -34,24 +35,6 @@ impl MySqlAccountRepository { #[async_trait::async_trait] impl AccountRepository for MySqlAccountRepository { - async fn accounts_count(&self) -> Result { - let mut tr = self.transaction.lock().await; - - let connection_mut = tr.connection_mut().await?; - - let accounts_count = sqlx::query_scalar!( - r#" - SELECT COUNT(*) - FROM accounts - "#, - ) - .fetch_one(connection_mut) - .await - .int_err()?; - - Ok(usize::try_from(accounts_count).unwrap_or(0)) - } - async fn create_account(&self, account: &Account) -> Result<(), CreateAccountError> { let mut tr = self.transaction.lock().await; @@ -110,45 +93,6 @@ impl AccountRepository for MySqlAccountRepository { Ok(()) } - async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { - Box::pin(async_stream::stream! { - let mut tr = self.transaction.lock().await; - let connection_mut = tr.connection_mut().await?; - - let limit = i64::try_from(pagination.limit).int_err()?; - let offset = i64::try_from(pagination.offset).int_err()?; - - let mut query_stream = sqlx::query_as!( - AccountRowModel, - r#" - SELECT id AS "id: _", - account_name, - email AS "email?", - display_name, - account_type AS "account_type: AccountType", - avatar_url, - registered_at AS "registered_at: _", - is_admin AS "is_admin: _", - provider, - provider_identity_key - FROM accounts - ORDER BY registered_at ASC - LIMIT ? OFFSET ? - "#, - limit, - offset, - ) - .fetch(connection_mut) - .map_err(ErrorIntoInternal::int_err); - - use futures::TryStreamExt; - - while let Some(entry) = query_stream.try_next().await? { - yield Ok(entry.into()); - } - }) - } - async fn get_account_by_id( &self, account_id: &AccountID, @@ -365,6 +309,68 @@ impl AccountRepository for MySqlAccountRepository { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +#[async_trait::async_trait] +impl ExpensiveAccountRepository for MySqlAccountRepository { + async fn accounts_count(&self) -> Result { + let mut tr = self.transaction.lock().await; + + let connection_mut = tr.connection_mut().await?; + + let accounts_count = sqlx::query_scalar!( + r#" + SELECT COUNT(*) + FROM accounts + "#, + ) + .fetch_one(connection_mut) + .await + .int_err()?; + + Ok(usize::try_from(accounts_count).unwrap_or(0)) + } + + async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { + Box::pin(async_stream::stream! { + let mut tr = self.transaction.lock().await; + let connection_mut = tr.connection_mut().await?; + + let limit = i64::try_from(pagination.limit).int_err()?; + let offset = i64::try_from(pagination.offset).int_err()?; + + let mut query_stream = sqlx::query_as!( + AccountRowModel, + r#" + SELECT id AS "id: _", + account_name, + email AS "email?", + display_name, + account_type AS "account_type: AccountType", + avatar_url, + registered_at AS "registered_at: _", + is_admin AS "is_admin: _", + provider, + provider_identity_key + FROM accounts + ORDER BY registered_at ASC + LIMIT ? OFFSET ? + "#, + limit, + offset, + ) + .fetch(connection_mut) + .map_err(ErrorIntoInternal::int_err); + + use futures::TryStreamExt; + + while let Some(entry) = query_stream.try_next().await? { + yield Ok(entry.into()); + } + }) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + #[async_trait::async_trait] impl PasswordHashRepository for MySqlAccountRepository { async fn save_password_hash( diff --git a/src/infra/accounts/postgres/src/repos/postgres_account_repository.rs b/src/infra/accounts/postgres/src/repos/postgres_account_repository.rs index 669e8cc0d0..9822d39c49 100644 --- a/src/infra/accounts/postgres/src/repos/postgres_account_repository.rs +++ b/src/infra/accounts/postgres/src/repos/postgres_account_repository.rs @@ -22,6 +22,7 @@ pub struct PostgresAccountRepository { #[component(pub)] #[interface(dyn AccountRepository)] +#[interface(dyn ExpensiveAccountRepository)] #[interface(dyn PasswordHashRepository)] impl PostgresAccountRepository { pub fn new(transaction: TransactionRef) -> Self { @@ -33,24 +34,6 @@ impl PostgresAccountRepository { #[async_trait::async_trait] impl AccountRepository for PostgresAccountRepository { - async fn accounts_count(&self) -> Result { - let mut tr = self.transaction.lock().await; - - let connection_mut = tr.connection_mut().await?; - - let accounts_count = sqlx::query_scalar!( - r#" - SELECT COUNT(*) - FROM accounts - "#, - ) - .fetch_one(connection_mut) - .await - .int_err()?; - - Ok(usize::try_from(accounts_count.unwrap()).unwrap()) - } - async fn create_account(&self, account: &Account) -> Result<(), CreateAccountError> { let mut tr = self.transaction.lock().await; @@ -104,45 +87,6 @@ impl AccountRepository for PostgresAccountRepository { Ok(()) } - async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { - Box::pin(async_stream::stream! { - let mut tr = self.transaction.lock().await; - let connection_mut = tr.connection_mut().await?; - - let limit = i64::try_from(pagination.limit).int_err()?; - let offset = i64::try_from(pagination.offset).int_err()?; - - let mut query_stream = sqlx::query_as!( - AccountRowModel, - r#" - SELECT id AS "id: _", - account_name, - email, - display_name, - account_type AS "account_type: AccountType", - avatar_url, - registered_at, - is_admin, - provider, - provider_identity_key - FROM accounts - ORDER BY registered_at ASC - LIMIT $1 OFFSET $2 - "#, - limit, - offset, - ) - .fetch(connection_mut) - .map_err(ErrorIntoInternal::int_err); - - use futures::TryStreamExt; - - while let Some(account_row_model) = query_stream.try_next().await? { - yield Ok(account_row_model.into()); - } - }) - } - async fn get_account_by_id( &self, account_id: &AccountID, @@ -336,6 +280,68 @@ impl AccountRepository for PostgresAccountRepository { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +#[async_trait::async_trait] +impl ExpensiveAccountRepository for PostgresAccountRepository { + async fn accounts_count(&self) -> Result { + let mut tr = self.transaction.lock().await; + + let connection_mut = tr.connection_mut().await?; + + let accounts_count = sqlx::query_scalar!( + r#" + SELECT COUNT(*) + FROM accounts + "#, + ) + .fetch_one(connection_mut) + .await + .int_err()?; + + Ok(usize::try_from(accounts_count.unwrap()).unwrap()) + } + + async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { + Box::pin(async_stream::stream! { + let mut tr = self.transaction.lock().await; + let connection_mut = tr.connection_mut().await?; + + let limit = i64::try_from(pagination.limit).int_err()?; + let offset = i64::try_from(pagination.offset).int_err()?; + + let mut query_stream = sqlx::query_as!( + AccountRowModel, + r#" + SELECT id AS "id: _", + account_name, + email, + display_name, + account_type AS "account_type: AccountType", + avatar_url, + registered_at, + is_admin, + provider, + provider_identity_key + FROM accounts + ORDER BY registered_at ASC + LIMIT $1 OFFSET $2 + "#, + limit, + offset, + ) + .fetch(connection_mut) + .map_err(ErrorIntoInternal::int_err); + + use futures::TryStreamExt; + + while let Some(account_row_model) = query_stream.try_next().await? { + yield Ok(account_row_model.into()); + } + }) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + #[async_trait::async_trait] impl PasswordHashRepository for PostgresAccountRepository { async fn save_password_hash( diff --git a/src/infra/accounts/sqlite/src/repos/sqlite_account_repository.rs b/src/infra/accounts/sqlite/src/repos/sqlite_account_repository.rs index dad5ef75b2..031cc82d1f 100644 --- a/src/infra/accounts/sqlite/src/repos/sqlite_account_repository.rs +++ b/src/infra/accounts/sqlite/src/repos/sqlite_account_repository.rs @@ -23,6 +23,7 @@ pub struct SqliteAccountRepository { #[component(pub)] #[interface(dyn AccountRepository)] +#[interface(dyn ExpensiveAccountRepository)] #[interface(dyn PasswordHashRepository)] impl SqliteAccountRepository { pub fn new(transaction: TransactionRef) -> Self { @@ -34,24 +35,6 @@ impl SqliteAccountRepository { #[async_trait::async_trait] impl AccountRepository for SqliteAccountRepository { - async fn accounts_count(&self) -> Result { - let mut tr = self.transaction.lock().await; - - let connection_mut = tr.connection_mut().await?; - - let accounts_count = sqlx::query_scalar!( - r#" - SELECT COUNT(*) - FROM accounts - "#, - ) - .fetch_one(connection_mut) - .await - .int_err()?; - - Ok(usize::try_from(accounts_count).unwrap_or(0)) - } - async fn create_account(&self, account: &Account) -> Result<(), CreateAccountError> { let mut tr = self.transaction.lock().await; @@ -122,45 +105,6 @@ impl AccountRepository for SqliteAccountRepository { Ok(()) } - async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { - Box::pin(async_stream::stream! { - let mut tr = self.transaction.lock().await; - let connection_mut = tr.connection_mut().await?; - - let limit = i64::try_from(pagination.limit).int_err()?; - let offset = i64::try_from(pagination.offset).int_err()?; - - let mut query_stream = sqlx::query_as!( - AccountRowModel, - r#" - SELECT id AS "id: _", - account_name, - email, - display_name, - account_type AS "account_type: AccountType", - avatar_url, - registered_at AS "registered_at: _", - is_admin AS "is_admin: _", - provider, - provider_identity_key - FROM accounts - ORDER BY registered_at ASC - LIMIT $1 OFFSET $2 - "#, - limit, - offset, - ) - .fetch(connection_mut) - .map_err(ErrorIntoInternal::int_err); - - use futures::TryStreamExt; - - while let Some(account_row_model) = query_stream.try_next().await? { - yield Ok(account_row_model.into()); - } - }) - } - async fn get_account_by_id( &self, account_id: &AccountID, @@ -407,6 +351,68 @@ impl AccountRepository for SqliteAccountRepository { //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +#[async_trait::async_trait] +impl ExpensiveAccountRepository for SqliteAccountRepository { + async fn accounts_count(&self) -> Result { + let mut tr = self.transaction.lock().await; + + let connection_mut = tr.connection_mut().await?; + + let accounts_count = sqlx::query_scalar!( + r#" + SELECT COUNT(*) + FROM accounts + "#, + ) + .fetch_one(connection_mut) + .await + .int_err()?; + + Ok(usize::try_from(accounts_count).unwrap_or(0)) + } + + async fn get_accounts(&self, pagination: PaginationOpts) -> AccountPageStream { + Box::pin(async_stream::stream! { + let mut tr = self.transaction.lock().await; + let connection_mut = tr.connection_mut().await?; + + let limit = i64::try_from(pagination.limit).int_err()?; + let offset = i64::try_from(pagination.offset).int_err()?; + + let mut query_stream = sqlx::query_as!( + AccountRowModel, + r#" + SELECT id AS "id: _", + account_name, + email, + display_name, + account_type AS "account_type: AccountType", + avatar_url, + registered_at AS "registered_at: _", + is_admin AS "is_admin: _", + provider, + provider_identity_key + FROM accounts + ORDER BY registered_at ASC + LIMIT $1 OFFSET $2 + "#, + limit, + offset, + ) + .fetch(connection_mut) + .map_err(ErrorIntoInternal::int_err); + + use futures::TryStreamExt; + + while let Some(account_row_model) = query_stream.try_next().await? { + yield Ok(account_row_model.into()); + } + }) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + #[async_trait::async_trait] impl PasswordHashRepository for SqliteAccountRepository { async fn save_password_hash( From 0869efaeeb2bde1dd9e35948a92d816995136744 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 20:53:57 +0200 Subject: [PATCH 19/20] RebacService::properties_count(): implement --- src/domain/auth-rebac/domain/src/services/rebac_service.rs | 3 +++ src/domain/auth-rebac/services/src/rebac_indexer.rs | 7 ++----- src/domain/auth-rebac/services/src/rebac_service_impl.rs | 5 +++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/domain/auth-rebac/domain/src/services/rebac_service.rs b/src/domain/auth-rebac/domain/src/services/rebac_service.rs index 8f77c3f0d0..1a69fbec92 100644 --- a/src/domain/auth-rebac/domain/src/services/rebac_service.rs +++ b/src/domain/auth-rebac/domain/src/services/rebac_service.rs @@ -19,6 +19,7 @@ use crate::{ DatasetPropertyName, EntityNotFoundError, EntityWithRelation, + PropertiesCountError, PropertyValue, SetEntityPropertyError, SubjectEntityRelationsError, @@ -28,6 +29,8 @@ use crate::{ #[async_trait::async_trait] pub trait RebacService: Send + Sync { + async fn properties_count(&self) -> Result; + // Account async fn set_account_property( &self, diff --git a/src/domain/auth-rebac/services/src/rebac_indexer.rs b/src/domain/auth-rebac/services/src/rebac_indexer.rs index b03952ff5d..96d2577337 100644 --- a/src/domain/auth-rebac/services/src/rebac_indexer.rs +++ b/src/domain/auth-rebac/services/src/rebac_indexer.rs @@ -19,7 +19,7 @@ use kamu_accounts::{ PredefinedAccountsConfig, JOB_KAMU_ACCOUNTS_PREDEFINED_ACCOUNTS_REGISTRATOR, }; -use kamu_auth_rebac::{AccountPropertyName, DatasetPropertyName, RebacRepository, RebacService}; +use kamu_auth_rebac::{AccountPropertyName, DatasetPropertyName, RebacService}; use kamu_core::DatasetVisibility; use kamu_datasets::DatasetEntryService; use kamu_datasets_services::JOB_KAMU_DATASETS_DATASET_ENTRY_INDEXER; @@ -34,7 +34,6 @@ type PredefinedAccountIdDatasetVisibilityMapping = HashMap, rebac_service: Arc, dataset_entry_service: Arc, expensive_account_repo: Arc, @@ -53,14 +52,12 @@ pub struct RebacIndexer { })] impl RebacIndexer { pub fn new( - rebac_repo: Arc, rebac_service: Arc, dataset_entry_service: Arc, expensive_account_repo: Arc, predefined_accounts_config: Arc, ) -> Self { Self { - rebac_repo, rebac_service, dataset_entry_service, expensive_account_repo, @@ -69,7 +66,7 @@ impl RebacIndexer { } async fn has_entities_indexed(&self) -> Result { - let properties_count = self.rebac_repo.properties_count().await.int_err()?; + let properties_count = self.rebac_service.properties_count().await.int_err()?; Ok(properties_count > 0) } diff --git a/src/domain/auth-rebac/services/src/rebac_service_impl.rs b/src/domain/auth-rebac/services/src/rebac_service_impl.rs index 1d2870cb99..193ad355bd 100644 --- a/src/domain/auth-rebac/services/src/rebac_service_impl.rs +++ b/src/domain/auth-rebac/services/src/rebac_service_impl.rs @@ -28,6 +28,7 @@ use kamu_auth_rebac::{ GetPropertiesError, InsertEntitiesRelationError, InsertRelationError, + PropertiesCountError, PropertyName, PropertyValue, RebacRepository, @@ -56,6 +57,10 @@ impl RebacServiceImpl { #[async_trait::async_trait] impl RebacService for RebacServiceImpl { + async fn properties_count(&self) -> Result { + self.rebac_repo.properties_count().await + } + async fn set_account_property( &self, account_id: &odf::AccountID, From 850df534481057ec95a9b359023250e6c1ad2708 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Thu, 16 Jan 2025 21:02:28 +0200 Subject: [PATCH 20/20] DatasetEntryService: move list-* operations within an implementation --- .../src/services/dataset_entry_service.rs | 15 +-- .../src/dataset_entry_service_impl.rs | 96 +++++++++---------- 2 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/domain/datasets/domain/src/services/dataset_entry_service.rs b/src/domain/datasets/domain/src/services/dataset_entry_service.rs index f62a5d17f8..9d50beebd2 100644 --- a/src/domain/datasets/domain/src/services/dataset_entry_service.rs +++ b/src/domain/datasets/domain/src/services/dataset_entry_service.rs @@ -7,7 +7,6 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use database_common::{EntityPageListing, PaginationOpts}; use internal_error::{ErrorIntoInternal, InternalError}; use opendatafabric as odf; use thiserror::Error; @@ -16,10 +15,9 @@ use crate::{DatasetEntry, DatasetEntryStream, GetDatasetEntryError}; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// TODO: Private Datasets: tests #[async_trait::async_trait] pub trait DatasetEntryService: Sync + Send { - // TODO: Private Datasets: tests - // TODO: Private Datasets: extract to DatasetEntryRegistry? fn all_entries(&self) -> DatasetEntryStream; fn entries_owned_by(&self, owner_id: &odf::AccountID) -> DatasetEntryStream; @@ -28,17 +26,6 @@ pub trait DatasetEntryService: Sync + Send { &self, dataset_id: &odf::DatasetID, ) -> Result; - - async fn list_all_entries( - &self, - pagination: PaginationOpts, - ) -> Result, ListDatasetEntriesError>; - - async fn list_entries_owned_by( - &self, - owner_id: &odf::AccountID, - pagination: PaginationOpts, - ) -> Result, ListDatasetEntriesError>; } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/domain/datasets/services/src/dataset_entry_service_impl.rs b/src/domain/datasets/services/src/dataset_entry_service_impl.rs index b17b57347d..5a77191ee4 100644 --- a/src/domain/datasets/services/src/dataset_entry_service_impl.rs +++ b/src/domain/datasets/services/src/dataset_entry_service_impl.rs @@ -277,6 +277,30 @@ impl DatasetEntryServiceImpl { } } + async fn list_all_entries( + &self, + pagination: PaginationOpts, + ) -> Result, ListDatasetEntriesError> { + use futures::TryStreamExt; + + let total_count = self + .dataset_entry_repo + .dataset_entries_count() + .await + .int_err()?; + let entries = self + .dataset_entry_repo + .get_dataset_entries(pagination) + .await + .try_collect() + .await?; + + Ok(EntityPageListing { + list: entries, + total_count, + }) + } + async fn list_all_dataset_handles( &self, pagination: PaginationOpts, @@ -292,6 +316,30 @@ impl DatasetEntryServiceImpl { }) } + async fn list_entries_owned_by( + &self, + owner_id: &odf::AccountID, + pagination: PaginationOpts, + ) -> Result, ListDatasetEntriesError> { + use futures::TryStreamExt; + + let total_count = self + .dataset_entry_repo + .dataset_entries_count_by_owner_id(owner_id) + .await?; + let entries = self + .dataset_entry_repo + .get_dataset_entries_by_owner_id(owner_id, pagination) + .await + .try_collect() + .await?; + + Ok(EntityPageListing { + list: entries, + total_count, + }) + } + async fn list_all_dataset_handles_by_owner_id( &self, owner_id: &odf::AccountID, @@ -345,54 +393,6 @@ impl DatasetEntryService for DatasetEntryServiceImpl { ) -> Result { self.dataset_entry_repo.get_dataset_entry(dataset_id).await } - - async fn list_all_entries( - &self, - pagination: PaginationOpts, - ) -> Result, ListDatasetEntriesError> { - use futures::TryStreamExt; - - let total_count = self - .dataset_entry_repo - .dataset_entries_count() - .await - .int_err()?; - let entries = self - .dataset_entry_repo - .get_dataset_entries(pagination) - .await - .try_collect() - .await?; - - Ok(EntityPageListing { - list: entries, - total_count, - }) - } - - async fn list_entries_owned_by( - &self, - owner_id: &odf::AccountID, - pagination: PaginationOpts, - ) -> Result, ListDatasetEntriesError> { - use futures::TryStreamExt; - - let total_count = self - .dataset_entry_repo - .dataset_entries_count_by_owner_id(owner_id) - .await?; - let entries = self - .dataset_entry_repo - .get_dataset_entries_by_owner_id(owner_id, pagination) - .await - .try_collect() - .await?; - - Ok(EntityPageListing { - list: entries, - total_count, - }) - } } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////