Skip to content

Commit

Permalink
refactor: add Tenant to ShareConsumer (#15142)
Browse files Browse the repository at this point in the history
  • Loading branch information
@drmingdrmer
drmingdrmer authored Mar 31, 2024
1 parent 4e894ae commit cab23ee
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 31 deletions.
14 changes: 7 additions & 7 deletions src/meta/api/src/share_api_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
for account in req.accounts.iter() {
if !share_meta.has_account(account) {
add_share_account_keys.push(ShareConsumer {
tenant: account.clone(),
tenant: Tenant::new_or_err(account, "add_share_tenants")?,
share_id,
});
}
Expand Down Expand Up @@ -358,7 +358,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
condition.push(txn_cond_seq(share_account_key, Eq, 0));

let share_account_meta = ShareAccountMeta::new(
share_account_key.tenant.clone(),
share_account_key.tenant.name().to_string(),
share_id,
req.share_on,
);
Expand All @@ -368,7 +368,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
serialize_struct(&share_account_meta)?,
)); /* (account, share_id) -> share_account_meta */

share_meta.add_account(share_account_key.tenant.clone());
share_meta.add_account(share_account_key.tenant.name().to_string());
}
if_then.push(txn_op_put(&id_key, serialize_struct(&share_meta)?)); /* (share_id) -> share_meta */

Expand Down Expand Up @@ -438,7 +438,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
}
if share_meta.has_account(account) {
let share_account_key = ShareConsumer {
tenant: account.clone(),
tenant: Tenant::new_or_err(account, "remove_share_tenants")?,
share_id,
};

Expand Down Expand Up @@ -485,7 +485,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {

if_then.push(txn_op_del(&share_account_key_and_seq.0)); // del (account, share_id)

share_meta.del_account(&share_account_key_and_seq.0.tenant);
share_meta.del_account(share_account_key_and_seq.0.tenant.name());
}
if_then.push(txn_op_put(&id_key, serialize_struct(&share_meta)?)); /* (share_id) -> share_meta */

Expand Down Expand Up @@ -1358,7 +1358,7 @@ async fn get_outbound_share_tenants_by_name(
let mut accounts = vec![];
for account in share_meta.get_accounts() {
let share_account_key = ShareConsumer {
tenant: account.clone(),
tenant: Tenant::new_or_err(&account, "get_outbound_share_tenants_by_name")?,
share_id,
};

Expand Down Expand Up @@ -1652,7 +1652,7 @@ async fn drop_accounts_granted_from_share(
// get all accounts seq from share_meta
for account in share_meta.get_accounts() {
let share_account_key = ShareConsumer {
tenant: account.clone(),
tenant: Tenant::new_or_err(&account, "drop_accounts_granted_from_share")?,
share_id,
};
let ret = get_share_account_meta_or_err(
Expand Down
8 changes: 4 additions & 4 deletions src/meta/api/src/share_api_test_suite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async fn is_all_share_data_removed(

for account in share_meta.get_accounts() {
let share_account_key = ShareConsumer {
tenant: account.clone(),
tenant: Tenant::new_or_err(account, "is_all_share_data_removed")?,
share_id,
};
let res = get_share_account_meta_or_err(kv_api, &share_account_key, "").await;
Expand Down Expand Up @@ -581,7 +581,7 @@ impl ShareApiTestSuite {

// get and check share account meta
let share_account_name = ShareConsumer {
tenant: account.to_string(),
tenant: Tenant::new_or_err(account, "share_add_remove_account")?,
share_id,
};
let (_share_account_meta_seq, share_account_meta) =
Expand Down Expand Up @@ -707,7 +707,7 @@ impl ShareApiTestSuite {

// check share account meta has been removed
let share_account_name = ShareConsumer {
tenant: account2.to_string(),
tenant: Tenant::new_or_err(account2, "share_add_remove_account")?,
share_id,
};
let res = get_share_account_meta_or_err(mt.as_kv_api(), &share_account_name, "").await;
Expand All @@ -730,7 +730,7 @@ impl ShareApiTestSuite {

// check share account meta has been removed
let share_account_name = ShareConsumer {
tenant: account.to_string(),
tenant: Tenant::new_or_err(account, "share_add_remove_account")?,
share_id,
};
let res = get_share_account_meta_or_err(mt.as_kv_api(), &share_account_name, "").await;
Expand Down
2 changes: 1 addition & 1 deletion src/meta/api/src/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ fn share_account_meta_has_to_exist(

Err(KVAppError::AppError(AppError::UnknownShareAccounts(
UnknownShareAccounts::new(
&[name_key.tenant.clone()],
&[name_key.tenant.name().to_string()],
name_key.share_id,
format!("{}: {}", msg, name_key),
),
Expand Down
33 changes: 21 additions & 12 deletions src/meta/app/src/share/share.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::schema::TableInfo;
use crate::schema::TableMeta;
use crate::share::ShareEndpointIdent;
use crate::tenant::Tenant;
use crate::tenant::ToTenant;

// serde is required by `DatabaseType`
/// A share that is created by `tenant` and is named with `share_name`.
Expand All @@ -48,15 +49,24 @@ impl Display for ShareNameIdent {

/// The share consuming key describes that the `tenant`, who is a consumer of a shared object,
/// which is created by another tenant and is identified by `share_id`.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct ShareConsumer {
pub tenant: String,
pub tenant: Tenant,
pub share_id: u64,
}

impl ShareConsumer {
pub fn new(tenant: impl ToTenant, share_id: u64) -> Self {
Self {
tenant: tenant.to_tenant(),
share_id,
}
}
}

impl Display for ShareConsumer {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "'{}'/'{}'", self.tenant, self.share_id)
write!(f, "'{}'/'{}'", self.tenant.name(), self.share_id)
}
}

Expand Down Expand Up @@ -608,7 +618,7 @@ impl ShareMeta {
self.accounts.insert(account);
}

pub fn del_account(&mut self, account: &String) {
pub fn del_account(&mut self, account: &str) {
self.accounts.remove(account);
}

Expand Down Expand Up @@ -885,33 +895,32 @@ mod kvapi_key_impl {

/// It belongs to a tenant
fn parent(&self) -> Option<String> {
Some(Tenant::new(&self.tenant).to_string_key())
Some(kvapi::Key::to_string_key(&self.tenant))
}

fn to_string_key(&self) -> String {
if self.share_id != 0 {
kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
.push_str(&self.tenant)
.push_str(self.tenant.name())
.push_u64(self.share_id)
.done()
} else {
kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
.push_str(&self.tenant)
.push_str(self.tenant.name())
.done()
}
}

fn from_str_key(s: &str) -> Result<Self, kvapi::KeyError> {
let mut p = kvapi::KeyParser::new_prefixed(s, Self::PREFIX)?;

let account = p.next_str()?;
let tenant = p.next_nonempty()?;
let share_id = p.next_u64()?;
p.done()?;

Ok(ShareConsumer {
tenant: account,
share_id,
})
let tenant = Tenant::new_nonempty(tenant);

Ok(ShareConsumer { tenant, share_id })
}
}

Expand Down
7 changes: 0 additions & 7 deletions src/meta/app/src/tenant/tenant.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ pub struct Tenant {
}

impl Tenant {
// #[deprecated]
pub fn new(tenant: impl ToString) -> Self {
Self {
tenant: tenant.to_string(),
}
}

pub fn new_or_err(tenant: impl ToString, ctx: impl Display) -> Result<Self, TenantIsEmpty> {
let non_empty =
NonEmptyString::new(tenant.to_string()).map_err(|_e| TenantIsEmpty::new(ctx))?;
Expand Down

0 comments on commit cab23ee

Please sign in to comment.