Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 67 additions & 5 deletions src/domain/opendatafabric/src/identity/dataset_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// by the Apache License, Version 2.0.

use std::convert::{AsRef, TryFrom};
use std::hash::Hash;
use std::sync::Arc;
use std::{cmp, fmt, ops};

Expand Down Expand Up @@ -104,7 +105,7 @@ macro_rules! impl_serde {
}

pub(crate) use impl_serde;
use like::Like;
use like::ILike;

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

Expand Down Expand Up @@ -221,6 +222,10 @@ impl DatasetName {
pub fn into_local_ref(self) -> DatasetRef {
DatasetRef::Alias(DatasetAlias::new(None, self))
}

pub fn lowercase_eq(&self, other: &Self) -> bool {
Comment thread
sergiimk marked this conversation as resolved.
Outdated
self.to_lowercase() == other.to_lowercase()
Comment thread
sergiimk marked this conversation as resolved.
Outdated
}
}

///////////////////////////////////////////////////////////////////////////////
Expand All @@ -233,7 +238,7 @@ newtype_str!(

impl DatasetNamePattern {
pub fn matches(&self, dataset_name: &DatasetName) -> bool {
Like::<false>::like(dataset_name.as_str(), self).unwrap()
ILike::<false>::ilike(dataset_name.as_str(), self).unwrap()
}
}

Expand All @@ -257,7 +262,14 @@ impl DatasetAliasPattern {
}

pub fn matches(&self, dataset_handle: &DatasetHandle) -> bool {
(self.account_name.is_none() || self.account_name == dataset_handle.alias.account_name)
((self.account_name.is_none() && dataset_handle.alias.account_name.is_none())
|| (self.account_name.is_some()
&& dataset_handle.alias.account_name.is_some()
&& self
Comment thread
zaychenko-sergei marked this conversation as resolved.
Outdated
.account_name
.as_ref()
.unwrap()
.lowercase_eq(dataset_handle.alias.account_name.as_ref().unwrap())))
&& self
.dataset_name_pattern
.matches(&dataset_handle.alias.dataset_name)
Expand Down Expand Up @@ -305,13 +317,19 @@ newtype_str!(
AccountNameSerdeVisitor
);

impl AccountName {
pub fn lowercase_eq(&self, other: &Self) -> bool {
self.to_lowercase() == other.to_lowercase()
}
}

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

newtype_str!(RepoName, Grammar::match_repo_name, RepoNameSerdeVisitor);

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

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone, Eq, PartialOrd, Ord)]
Comment thread
zaychenko-sergei marked this conversation as resolved.
Outdated
pub struct DatasetAlias {
pub account_name: Option<AccountName>,
pub dataset_name: DatasetName,
Expand Down Expand Up @@ -358,6 +376,27 @@ impl DatasetAlias {
}
}

impl PartialEq<Self> for DatasetAlias {
fn eq(&self, other: &Self) -> bool {
((self.account_name.is_none() && other.account_name.is_none())
|| (self.account_name.is_some()
&& other.account_name.is_some()
&& self
.account_name
.as_ref()
.unwrap()
.lowercase_eq(other.account_name.as_ref().unwrap())))
&& self.dataset_name.lowercase_eq(&other.dataset_name)
Comment thread
zaychenko-sergei marked this conversation as resolved.
Outdated
}
}

impl Hash for DatasetAlias {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.account_name.hash(state);
self.dataset_name.hash(state);
}
}

impl std::str::FromStr for DatasetAlias {
type Err = ParseError<Self>;
fn from_str(s: &str) -> Result<Self, Self::Err> {
Expand Down Expand Up @@ -388,7 +427,7 @@ impl_serde!(DatasetAlias, DatasetAliasSerdeVisitor);

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

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone, Eq, PartialOrd, Ord)]
Comment thread
zaychenko-sergei marked this conversation as resolved.
Outdated
pub struct DatasetAliasRemote {
pub repo_name: RepoName,
pub account_name: Option<AccountName>,
Expand Down Expand Up @@ -437,6 +476,29 @@ impl DatasetAliasRemote {
}
}

impl PartialEq<Self> for DatasetAliasRemote {
fn eq(&self, other: &Self) -> bool {
(self.repo_name == other.repo_name)
&& ((self.account_name.is_some()
&& other.account_name.is_some()
&& self
.account_name
.as_ref()
.unwrap()
.lowercase_eq(other.account_name.as_ref().unwrap()))
Comment thread
zaychenko-sergei marked this conversation as resolved.
Outdated
|| (self.account_name.is_none() && other.account_name.is_none()))
&& self.dataset_name.lowercase_eq(&other.dataset_name)
}
}

impl Hash for DatasetAliasRemote {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.repo_name.hash(state);
self.account_name.hash(state);
self.dataset_name.hash(state);
}
}

impl std::str::FromStr for DatasetAliasRemote {
type Err = ParseError<Self>;
fn from_str(s: &str) -> Result<Self, Self::Err> {
Expand Down
52 changes: 52 additions & 0 deletions src/domain/opendatafabric/tests/tests/test_dataset_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,55 @@ fn test_dataset_refs_conversions() {
alias: DatasetAlias::try_from("bar").unwrap(),
});
}

#[test]
fn test_dataset_alias_eq() {
assert_eq!(
DatasetAlias::from_str("account/net.example.com").unwrap(),
DatasetAlias::from_str("aCCouNt/net.ExaMplE.coM").unwrap(),
);
assert_eq!(
DatasetAlias::from_str("account/net.example.com").unwrap(),
DatasetAlias::from_str("account/net.example.com").unwrap(),
);
assert_eq!(
DatasetAlias::from_str("net.example.com").unwrap(),
DatasetAlias::from_str("net.ExaMplE.coM").unwrap(),
);
assert_ne!(
DatasetAlias::from_str("account/net.example.com").unwrap(),
DatasetAlias::from_str("aCCouNt1/net.eXamPle.cOm").unwrap(),
);
assert_ne!(
DatasetAlias::from_str("account1/net.example.com").unwrap(),
DatasetAlias::from_str("account/net.example.com").unwrap(),
);
assert_ne!(
DatasetAlias::from_str("net.example.com").unwrap(),
DatasetAlias::from_str("account/net.example.com").unwrap(),
);
}

#[test]
fn test_dataset_remote_alias_eq() {
assert_eq!(
DatasetAliasRemote::from_str("repository/net.example.com").unwrap(),
DatasetAliasRemote::from_str("repository/net.ExaMplE.coM").unwrap(),
);
assert_eq!(
DatasetAliasRemote::from_str("repository/net.example.com").unwrap(),
DatasetAliasRemote::from_str("repository/net.example.com").unwrap(),
);
assert_eq!(
DatasetAliasRemote::from_str("repository/account/net.example.com").unwrap(),
DatasetAliasRemote::from_str("repository/AccOuNt/net.ExaMplE.coM").unwrap(),
);
assert_ne!(
DatasetAliasRemote::from_str("repository/net.example.com").unwrap(),
DatasetAliasRemote::from_str("rEpoSitOry/net.ExaMplE.coM").unwrap(),
);
assert_ne!(
DatasetAliasRemote::from_str("repository/account/net.example.com").unwrap(),
DatasetAliasRemote::from_str("repository/net.example.com").unwrap(),
);
}
2 changes: 1 addition & 1 deletion src/domain/opendatafabric/tests/tests/test_dataset_refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn test_dataset_ref_pattern_match() {
},
};

assert!(pattern.matches(&dataset_handle));
assert!(!pattern.matches(&dataset_handle));

let dataset_account = "account1";
let dataset_name_pattern = "net%";
Expand Down
10 changes: 7 additions & 3 deletions src/infra/core/src/repos/dataset_repository_local_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ impl DatasetStorageStrategy for DatasetSingleTenantStorageStrategy {
}

fn get_datasets_by_owner(&self, account_name: AccountName) -> DatasetHandleStream<'_> {
if account_name == DEFAULT_ACCOUNT_NAME {
if account_name.to_lowercase() == DEFAULT_ACCOUNT_NAME {
self.get_all_datasets()
} else {
Box::pin(futures::stream::empty())
Expand All @@ -529,7 +529,8 @@ impl DatasetStorageStrategy for DatasetSingleTenantStorageStrategy {
) -> Result<DatasetHandle, ResolveDatasetError> {
assert!(
!dataset_alias.is_multi_tenant()
|| dataset_alias.account_name.as_ref().unwrap() == DEFAULT_ACCOUNT_NAME,
|| dataset_alias.account_name.as_ref().unwrap().to_lowercase()
== DEFAULT_ACCOUNT_NAME,
"Multi-tenant refs shouldn't have reached down to here with earlier validations"
);

Expand Down Expand Up @@ -809,7 +810,10 @@ impl DatasetStorageStrategy for DatasetMultiTenantStorageStrategy {
)
.await?;

if candidate_dataset_alias.dataset_name == dataset_alias.dataset_name {
if candidate_dataset_alias
.dataset_name
.lowercase_eq(&dataset_alias.dataset_name)
{
return Ok(DatasetHandle::new(dataset_id, candidate_dataset_alias));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/infra/core/src/repos/dataset_repository_s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,13 @@ impl DatasetRepository for DatasetRepositoryS3 {
}

fn get_datasets_by_owner(&self, account_name: AccountName) -> DatasetHandleStream<'_> {
if !self.is_multi_tenant() && account_name != DEFAULT_ACCOUNT_NAME {
if !self.is_multi_tenant() && account_name.to_lowercase() != DEFAULT_ACCOUNT_NAME {
return Box::pin(futures::stream::empty());
}

self.stream_datasets_if(move |dataset_alias| {
if let Some(dataset_account_name) = &dataset_alias.account_name {
dataset_account_name == &account_name
dataset_account_name.to_lowercase() == account_name.to_lowercase()
} else {
true
}
Expand Down
6 changes: 4 additions & 2 deletions src/infra/core/src/utils/datasets_filtering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ pub fn matches_remote_ref_pattern(
DatasetRefAnyPattern::PatternRemote(repo_name, account_name, dataset_name_pattern) => {
repo_name == &dataset_alias_remote.repo_name
&& (dataset_alias_remote.account_name.is_some()
&& account_name == dataset_alias_remote.account_name.as_ref().unwrap())
&& account_name
.lowercase_eq(dataset_alias_remote.account_name.as_ref().unwrap()))
&& dataset_name_pattern.matches(&dataset_alias_remote.dataset_name)
}
}
Expand Down Expand Up @@ -197,7 +198,8 @@ pub fn matches_local_ref_pattern(
}
DatasetRefAnyPattern::PatternAmbiguous(account_name, dataset_name_pattern) => {
let account_name = AccountName::from_str(&account_name.pattern).unwrap();
Some(account_name) == dataset_handle.alias.account_name
(dataset_handle.alias.account_name.is_some()
&& account_name.lowercase_eq(dataset_handle.alias.account_name.as_ref().unwrap()))
&& dataset_name_pattern.matches(&dataset_handle.alias.dataset_name)
}
}
Expand Down