Skip to content

Commit 5e0ccd9

Browse files
rmn-boikormn-boiko
authored
Make dataset alias comparisons case-insensitive (#537)
* Make dataset alias comparisons case-insensitive * Add case insensetive comparing * Improve datasets lookup * Add repo support
1 parent ce5f8b7 commit 5e0ccd9

36 files changed

+751
-205
lines changed

‎CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99
- Support `ArrowJson` schema output format in QGL API and CLI commands
1010
- New `kamu system compact <dataset>` command that compacts dataslices for the given dataset
1111
### Changed
12-
- Next batch of optimizations of metadata chain traversal through API using Visitors
12+
- Case insensitive comparisons of `dataset`s, `account`s and `repo`s
1313

1414
## [0.170.0] - 2024-03-29
1515
### Added

‎src/adapter/graphql/src/queries/datasets/datasets.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,14 @@ impl Datasets {
3939
account_name: AccountName,
4040
dataset_name: DatasetName,
4141
) -> Result<Option<Dataset>> {
42-
let account = Account::from_account_name(account_name.clone().into());
43-
4442
let dataset_alias = odf::DatasetAlias::new(Some(account_name.into()), dataset_name.into());
4543

4644
let dataset_repo = from_catalog::<dyn domain::DatasetRepository>(ctx).unwrap();
4745
let hdl = dataset_repo
4846
.try_resolve_dataset_ref(&dataset_alias.into_local_ref())
4947
.await?;
50-
Ok(hdl.map(|h| Dataset::new(account, h)))
48+
49+
Ok(hdl.map(|h| Dataset::new(Account::from_dataset_alias(ctx, &h.alias), h)))
5150
}
5251

5352
#[graphql(skip)]

‎src/adapter/graphql/tests/tests/test_accounts.rs

+57-16
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,7 @@ use opendatafabric::AccountName;
1717

1818
#[test_log::test(tokio::test)]
1919
async fn test_account_by_name() {
20-
let mut mock_authentication_service = MockAuthenticationService::new();
21-
mock_authentication_service
22-
.expect_find_account_info_by_name()
23-
.with(eq(AccountName::new_unchecked(DEFAULT_ACCOUNT_NAME)))
24-
.returning(|_| Ok(Some(AccountInfo::dummy())));
25-
mock_authentication_service
26-
.expect_find_account_info_by_name()
27-
.with(eq(AccountName::new_unchecked("unknown")))
28-
.returning(|_| Ok(None));
29-
30-
let cat = dill::CatalogBuilder::new()
31-
.add_value(mock_authentication_service)
32-
.bind::<dyn kamu_core::auth::AuthenticationService, MockAuthenticationService>()
33-
.build();
20+
let harness = GraphQLAccountsHarness::new();
3421

3522
let schema = kamu_adapter_graphql::schema_quiet();
3623
let res = schema
@@ -46,7 +33,7 @@ async fn test_account_by_name() {
4633
}}
4734
"#,
4835
))
49-
.data(cat.clone()),
36+
.data(harness.catalog.clone()),
5037
)
5138
.await;
5239

@@ -76,7 +63,7 @@ async fn test_account_by_name() {
7663
"#,
7764
"unknown",
7865
))
79-
.data(cat),
66+
.data(harness.catalog.clone()),
8067
)
8168
.await;
8269

@@ -89,6 +76,60 @@ async fn test_account_by_name() {
8976
}
9077
})
9178
);
79+
80+
let res = schema
81+
.execute(
82+
async_graphql::Request::new(format!(
83+
r#"
84+
query {{
85+
accounts {{
86+
byName (name: "{}") {{
87+
accountName
88+
}}
89+
}}
90+
}}
91+
"#,
92+
DEFAULT_ACCOUNT_NAME.to_ascii_uppercase(),
93+
))
94+
.data(harness.catalog),
95+
)
96+
.await;
97+
98+
assert!(res.is_ok(), "{res:?}");
99+
assert_eq!(
100+
res.data,
101+
value!({
102+
"accounts": {
103+
"byName": {
104+
"accountName": DEFAULT_ACCOUNT_NAME
105+
}
106+
}
107+
})
108+
);
92109
}
93110

94111
////////////////////////////////////////////////////////////////////////////////////////
112+
113+
struct GraphQLAccountsHarness {
114+
catalog: dill::Catalog,
115+
}
116+
117+
impl GraphQLAccountsHarness {
118+
pub fn new() -> Self {
119+
let mut mock_authentication_service = MockAuthenticationService::new();
120+
mock_authentication_service
121+
.expect_find_account_info_by_name()
122+
.with(eq(AccountName::new_unchecked(DEFAULT_ACCOUNT_NAME)))
123+
.returning(|_| Ok(Some(AccountInfo::dummy())));
124+
mock_authentication_service
125+
.expect_find_account_info_by_name()
126+
.with(eq(AccountName::new_unchecked("unknown")))
127+
.returning(|_| Ok(None));
128+
let catalog = dill::CatalogBuilder::new()
129+
.add_value(mock_authentication_service)
130+
.bind::<dyn kamu_core::auth::AuthenticationService, MockAuthenticationService>()
131+
.build();
132+
133+
Self { catalog }
134+
}
135+
}

‎src/adapter/graphql/tests/tests/test_gql_data.rs

+18-11
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,18 @@ use opendatafabric::*;
2222

2323
/////////////////////////////////////////////////////////////////////////////////////////
2424

25-
fn create_catalog_with_local_workspace(tempdir: &Path) -> dill::Catalog {
25+
fn create_catalog_with_local_workspace(tempdir: &Path, is_multitenant: bool) -> dill::Catalog {
26+
let datasets_dir = tempdir.join("datasets");
27+
std::fs::create_dir(&datasets_dir).unwrap();
28+
2629
dill::CatalogBuilder::new()
2730
.add::<EventBus>()
2831
.add::<DependencyGraphServiceInMemory>()
2932
.add_builder(
3033
DatasetRepositoryLocalFs::builder()
31-
.with_root(tempdir.join("datasets"))
34+
.with_root(datasets_dir)
3235
.with_current_account_subject(Arc::new(CurrentAccountSubject::new_test()))
33-
.with_multi_tenant(false),
36+
.with_multi_tenant(is_multitenant),
3437
)
3538
.bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()
3639
.add::<QueryServiceImpl>()
@@ -42,12 +45,16 @@ fn create_catalog_with_local_workspace(tempdir: &Path) -> dill::Catalog {
4245

4346
/////////////////////////////////////////////////////////////////////////////////////////
4447

45-
async fn create_test_dataset(catalog: &dill::Catalog, tempdir: &Path) {
48+
async fn create_test_dataset(
49+
catalog: &dill::Catalog,
50+
tempdir: &Path,
51+
account_name: Option<AccountName>,
52+
) {
4653
let dataset_repo = catalog.get_one::<dyn DatasetRepository>().unwrap();
4754

4855
let dataset = dataset_repo
4956
.create_dataset(
50-
&DatasetAlias::new(None, DatasetName::new_unchecked("foo")),
57+
&DatasetAlias::new(account_name, DatasetName::new_unchecked("foo")),
5158
MetadataFactory::metadata_block(MetadataFactory::seed(DatasetKind::Root).build())
5259
.build_typed(),
5360
)
@@ -103,8 +110,8 @@ async fn create_test_dataset(catalog: &dill::Catalog, tempdir: &Path) {
103110
#[test_log::test(tokio::test)]
104111
async fn test_dataset_schema_local_fs() {
105112
let tempdir = tempfile::tempdir().unwrap();
106-
let catalog = create_catalog_with_local_workspace(tempdir.path());
107-
create_test_dataset(&catalog, tempdir.path()).await;
113+
let catalog = create_catalog_with_local_workspace(tempdir.path(), true);
114+
create_test_dataset(&catalog, tempdir.path(), None).await;
108115

109116
let schema = kamu_adapter_graphql::schema_quiet();
110117
let res = schema
@@ -162,8 +169,8 @@ async fn test_dataset_schema_local_fs() {
162169
#[test_log::test(tokio::test)]
163170
async fn test_dataset_tail_local_fs() {
164171
let tempdir = tempfile::tempdir().unwrap();
165-
let catalog = create_catalog_with_local_workspace(tempdir.path());
166-
create_test_dataset(&catalog, tempdir.path()).await;
172+
let catalog = create_catalog_with_local_workspace(tempdir.path(), true);
173+
create_test_dataset(&catalog, tempdir.path(), None).await;
167174

168175
let schema = kamu_adapter_graphql::schema_quiet();
169176
let res = schema
@@ -203,8 +210,8 @@ async fn test_dataset_tail_local_fs() {
203210
#[test_log::test(tokio::test)]
204211
async fn test_dataset_tail_empty_local_fs() {
205212
let tempdir = tempfile::tempdir().unwrap();
206-
let catalog = create_catalog_with_local_workspace(tempdir.path());
207-
create_test_dataset(&catalog, tempdir.path()).await;
213+
let catalog = create_catalog_with_local_workspace(tempdir.path(), true);
214+
create_test_dataset(&catalog, tempdir.path(), None).await;
208215

209216
let schema = kamu_adapter_graphql::schema_quiet();
210217
let res = schema

‎src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1099,12 +1099,14 @@ struct FlowConfigHarness {
10991099
impl FlowConfigHarness {
11001100
fn new() -> Self {
11011101
let tempdir = tempfile::tempdir().unwrap();
1102+
let datasets_dir = tempdir.path().join("datasets");
1103+
std::fs::create_dir(&datasets_dir).unwrap();
11021104

11031105
let catalog_base = dill::CatalogBuilder::new()
11041106
.add::<EventBus>()
11051107
.add_builder(
11061108
DatasetRepositoryLocalFs::builder()
1107-
.with_root(tempdir.path().join("datasets"))
1109+
.with_root(datasets_dir)
11081110
.with_multi_tenant(false),
11091111
)
11101112
.bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

‎src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1701,6 +1701,8 @@ impl FlowRunsHarness {
17011701
let tempdir = tempfile::tempdir().unwrap();
17021702
let run_info_dir = tempdir.path().join("run");
17031703
let cache_dir = tempdir.path().join("cache");
1704+
let datasets_dir = tempdir.path().join("datasets");
1705+
std::fs::create_dir(&datasets_dir).unwrap();
17041706
std::fs::create_dir(&run_info_dir).unwrap();
17051707
std::fs::create_dir(&cache_dir).unwrap();
17061708

@@ -1711,7 +1713,7 @@ impl FlowRunsHarness {
17111713
.add::<EventBus>()
17121714
.add_builder(
17131715
DatasetRepositoryLocalFs::builder()
1714-
.with_root(tempdir.path().join("datasets"))
1716+
.with_root(datasets_dir)
17151717
.with_multi_tenant(false),
17161718
)
17171719
.bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

0 commit comments

Comments
 (0)