Skip to content

Commit ccf25ae

Browse files
drmingdrmerdrmingdrmer
authored
refactor: define ShareEndpointIdent with TIdent (#15136)
1 parent 34ce8e3 commit ccf25ae

File tree

12 files changed

+118
-152
lines changed

12 files changed

+118
-152
lines changed

‎src/meta/api/src/share_api_impl.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -988,10 +988,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
988988
match req.create_option {
989989
CreateOption::Create => {
990990
return Err(KVAppError::AppError(AppError::ShareEndpointAlreadyExists(
991-
ShareEndpointAlreadyExists::new(
992-
&name_key.endpoint,
993-
format!("create share endpoint: tenant: {}", name_key.tenant),
994-
),
991+
ShareEndpointAlreadyExists::new(name_key.name(), func_name!()),
995992
)));
996993
}
997994
CreateOption::CreateIfNotExists => {
@@ -1176,10 +1173,9 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
11761173
) -> Result<GetShareEndpointReply, KVAppError> {
11771174
let mut share_endpoint_meta_vec = vec![];
11781175

1179-
let tenant_share_endpoint_name_key = ShareEndpointIdent {
1180-
tenant: req.tenant.clone(),
1181-
endpoint: req.endpoint.clone().unwrap_or("".to_string()),
1182-
};
1176+
let tenant_share_endpoint_name_key =
1177+
ShareEndpointIdent::new(&req.tenant, req.endpoint.clone().unwrap_or("".to_string()));
1178+
11831179
let share_endpoints = list_keys(self, &tenant_share_endpoint_name_key).await?;
11841180
for share_endpoint in share_endpoints {
11851181
let (_seq, share_endpoint_id) = get_u64_value(self, &share_endpoint).await?;
@@ -1271,7 +1267,7 @@ async fn construct_drop_share_endpoint_txn_operations(
12711267
name_key,
12721268
format!(
12731269
"construct_drop_share_endpoint_txn_operations: {}",
1274-
&name_key
1270+
name_key.display()
12751271
),
12761272
)
12771273
.await;
@@ -1294,7 +1290,7 @@ async fn construct_drop_share_endpoint_txn_operations(
12941290
share_endpoint_id,
12951291
format!(
12961292
"construct_drop_share_endpoint_txn_operations: {}",
1297-
&name_key
1293+
name_key.display()
12981294
),
12991295
)
13001296
.await?;

‎src/meta/api/src/share_api_test_suite.rs

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -204,23 +204,22 @@ impl ShareApiTestSuite {
204204
&self,
205205
mt: &MT,
206206
) -> anyhow::Result<()> {
207-
let tenant = "tenant1";
208-
let tenant2 = "tenant2";
209-
let tenant3 = "tenant3";
207+
let tenant_name1 = "tenant1";
208+
let tenant_name2 = "tenant2";
209+
let tenant_name3 = "tenant3";
210210
let endpoint1 = "endpoint1";
211211
let endpoint2 = "endpoint2";
212212

213+
let tenant1 = Tenant::new_literal(tenant_name1);
214+
213215
info!("--- create share endpoints");
214216
let create_on = Utc::now();
215217
{
216218
let req = CreateShareEndpointReq {
217219
create_option: CreateOption::Create,
218-
endpoint: ShareEndpointIdent {
219-
tenant: tenant.to_string(),
220-
endpoint: endpoint1.to_string(),
221-
},
220+
endpoint: ShareEndpointIdent::new(&tenant1, endpoint1),
222221
url: "http://127.0.0.1:22222".to_string(),
223-
tenant: tenant2.to_string(),
222+
tenant: tenant_name2.to_string(),
224223
comment: None,
225224
create_on,
226225
args: BTreeMap::new(),
@@ -232,12 +231,9 @@ impl ShareApiTestSuite {
232231

233232
let req = CreateShareEndpointReq {
234233
create_option: CreateOption::Create,
235-
endpoint: ShareEndpointIdent {
236-
tenant: tenant.to_string(),
237-
endpoint: endpoint1.to_string(),
238-
},
234+
endpoint: ShareEndpointIdent::new(&tenant1, endpoint1),
239235
url: "http://127.0.0.1:21111".to_string(),
240-
tenant: tenant2.to_string(),
236+
tenant: tenant_name2.to_string(),
241237
comment: None,
242238
args: BTreeMap::new(),
243239
create_on,
@@ -254,12 +250,9 @@ impl ShareApiTestSuite {
254250

255251
let req = CreateShareEndpointReq {
256252
create_option: CreateOption::Create,
257-
endpoint: ShareEndpointIdent {
258-
tenant: tenant.to_string(),
259-
endpoint: endpoint2.to_string(),
260-
},
253+
endpoint: ShareEndpointIdent::new(&tenant1, endpoint2),
261254
url: "http://127.0.0.1:21111".to_string(),
262-
tenant: tenant3.to_string(),
255+
tenant: tenant_name3.to_string(),
263256
comment: None,
264257
create_on,
265258
args: BTreeMap::new(),
@@ -272,14 +265,13 @@ impl ShareApiTestSuite {
272265

273266
info!("--- upsert share endpoints");
274267
{
275-
let upsert_tenant = "upsert_tenant";
268+
let upsert_tenant_name = "upsert_tenant";
269+
let upsert_tenant = Tenant::new_literal(upsert_tenant_name);
270+
276271
let upsert_req = UpsertShareEndpointReq {
277-
endpoint: ShareEndpointIdent {
278-
tenant: upsert_tenant.to_string(),
279-
endpoint: endpoint2.to_string(),
280-
},
272+
endpoint: ShareEndpointIdent::new(&upsert_tenant, endpoint2),
281273
url: "http://127.0.0.1:21111".to_string(),
282-
tenant: tenant3.to_string(),
274+
tenant: tenant_name3.to_string(),
283275
create_on,
284276
args: BTreeMap::new(),
285277
};
@@ -288,7 +280,7 @@ impl ShareApiTestSuite {
288280
let upsert_share_endpoint_id = res.unwrap().share_endpoint_id;
289281

290282
let req = GetShareEndpointReq {
291-
tenant: upsert_tenant.to_string(),
283+
tenant: upsert_tenant.clone(),
292284
endpoint: None,
293285
to_tenant: None,
294286
};
@@ -305,12 +297,9 @@ impl ShareApiTestSuite {
305297
assert_eq!(upsert_share_endpoint_id, res.unwrap().share_endpoint_id);
306298

307299
let upsert_req = UpsertShareEndpointReq {
308-
endpoint: ShareEndpointIdent {
309-
tenant: upsert_tenant.to_string(),
310-
endpoint: endpoint2.to_string(),
311-
},
300+
endpoint: ShareEndpointIdent::new(&upsert_tenant, endpoint2),
312301
url: "http://127.0.0.1:22222".to_string(),
313-
tenant: tenant3.to_string(),
302+
tenant: tenant_name3.to_string(),
314303
create_on,
315304
args: BTreeMap::new(),
316305
};
@@ -319,7 +308,7 @@ impl ShareApiTestSuite {
319308
assert_eq!(upsert_share_endpoint_id, res.unwrap().share_endpoint_id);
320309

321310
let req = GetShareEndpointReq {
322-
tenant: upsert_tenant.to_string(),
311+
tenant: upsert_tenant.clone(),
323312
endpoint: None,
324313
to_tenant: None,
325314
};
@@ -334,7 +323,7 @@ impl ShareApiTestSuite {
334323
info!("--- get share endpoints");
335324
{
336325
let req = GetShareEndpointReq {
337-
tenant: tenant.to_string(),
326+
tenant: tenant1.clone(),
338327
endpoint: None,
339328
to_tenant: None,
340329
};
@@ -344,7 +333,7 @@ impl ShareApiTestSuite {
344333
assert_eq!(res.unwrap().share_endpoint_meta_vec.len(), 2);
345334

346335
let req = GetShareEndpointReq {
347-
tenant: tenant.to_string(),
336+
tenant: tenant1.clone(),
348337
endpoint: Some(endpoint1.to_string()),
349338
to_tenant: None,
350339
};
@@ -358,16 +347,13 @@ impl ShareApiTestSuite {
358347
{
359348
let req = DropShareEndpointReq {
360349
if_exists: true,
361-
endpoint: ShareEndpointIdent {
362-
tenant: tenant.to_string(),
363-
endpoint: endpoint1.to_string(),
364-
},
350+
endpoint: ShareEndpointIdent::new(&tenant1, endpoint1),
365351
};
366352
let res = mt.drop_share_endpoint(req).await;
367353
assert!(res.is_ok());
368354

369355
let req = GetShareEndpointReq {
370-
tenant: tenant.to_string(),
356+
tenant: tenant1.clone(),
371357
endpoint: None,
372358
to_tenant: None,
373359
};
@@ -377,7 +363,7 @@ impl ShareApiTestSuite {
377363
assert_eq!(res.unwrap().share_endpoint_meta_vec.len(), 1);
378364

379365
let req = GetShareEndpointReq {
380-
tenant: tenant.to_string(),
366+
tenant: tenant1.clone(),
381367
endpoint: Some(endpoint1.to_string()),
382368
to_tenant: None,
383369
};
@@ -390,16 +376,14 @@ impl ShareApiTestSuite {
390376
{
391377
info!("--- create or replace share endpoints");
392378
let endpoint_name = "replace_endpoint";
393-
let endpoint = ShareEndpointIdent {
394-
tenant: tenant.to_string(),
395-
endpoint: endpoint_name.to_string(),
396-
};
379+
let endpoint = ShareEndpointIdent::new(&tenant1, endpoint_name);
380+
397381
let url = "http://127.0.0.1:22222".to_string();
398382
let req = CreateShareEndpointReq {
399383
create_option: CreateOption::Create,
400384
endpoint: endpoint.clone(),
401385
url: url.clone(),
402-
tenant: tenant2.to_string(),
386+
tenant: tenant_name2.to_string(),
403387
comment: None,
404388
create_on,
405389
args: BTreeMap::new(),
@@ -421,7 +405,7 @@ impl ShareApiTestSuite {
421405
assert_eq!(name_key, endpoint);
422406

423407
let req = GetShareEndpointReq {
424-
tenant: tenant.to_string(),
408+
tenant: tenant1.clone(),
425409
endpoint: Some(endpoint_name.to_string()),
426410
to_tenant: None,
427411
};
@@ -435,7 +419,7 @@ impl ShareApiTestSuite {
435419
create_option: CreateOption::CreateOrReplace,
436420
endpoint: endpoint.clone(),
437421
url: url.clone(),
438-
tenant: tenant2.to_string(),
422+
tenant: tenant_name2.to_string(),
439423
comment: None,
440424
create_on,
441425
args: BTreeMap::new(),
@@ -445,7 +429,7 @@ impl ShareApiTestSuite {
445429
let share_endpoint_id = res.share_endpoint_id;
446430

447431
let req = GetShareEndpointReq {
448-
tenant: tenant.to_string(),
432+
tenant: tenant1.clone(),
449433
endpoint: Some(endpoint_name.to_string()),
450434
to_tenant: None,
451435
};

‎src/meta/api/src/util.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ fn share_endpoint_has_to_exist(
513513
debug!(seq = seq, name_key :? =(name_key); "share endpoint does not exist");
514514

515515
Err(KVAppError::AppError(AppError::UnknownShareEndpoint(
516-
UnknownShareEndpoint::new(&name_key.endpoint, format!("{}: {}", msg, name_key)),
516+
UnknownShareEndpoint::new(name_key.name(), format!("{}", msg)),
517517
)))
518518
} else {
519519
Ok(())

‎src/meta/app/src/app_error.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,10 +555,10 @@ pub struct ShareEndpointAlreadyExists {
555555
}
556556

557557
impl ShareEndpointAlreadyExists {
558-
pub fn new(endpoint: impl Into<String>, context: impl Into<String>) -> Self {
558+
pub fn new(endpoint: impl ToString, context: impl ToString) -> Self {
559559
Self {
560-
endpoint: endpoint.into(),
561-
context: context.into(),
560+
endpoint: endpoint.to_string(),
561+
context: context.to_string(),
562562
}
563563
}
564564
}

‎src/meta/app/src/share/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#[allow(clippy::module_inception)]
1616
mod share;
1717

18+
mod share_end_point_ident;
19+
1820
pub use share::AddShareAccountsReply;
1921
pub use share::AddShareAccountsReq;
2022
pub use share::CreateShareEndpointReply;
@@ -48,7 +50,6 @@ pub use share::ShareConsumer;
4850
pub use share::ShareDatabaseSpec;
4951
pub use share::ShareEndpointId;
5052
pub use share::ShareEndpointIdToName;
51-
pub use share::ShareEndpointIdent;
5253
pub use share::ShareEndpointMeta;
5354
pub use share::ShareGrantEntry;
5455
pub use share::ShareGrantObject;
@@ -70,3 +71,4 @@ pub use share::ShowSharesReq;
7071
pub use share::TableInfoMap;
7172
pub use share::UpsertShareEndpointReply;
7273
pub use share::UpsertShareEndpointReq;
74+
pub use share_end_point_ident::ShareEndpointIdent;

‎src/meta/app/src/share/share.rs

Lines changed: 2 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use crate::schema::CreateOption;
2929
use crate::schema::DatabaseMeta;
3030
use crate::schema::TableInfo;
3131
use crate::schema::TableMeta;
32+
use crate::share::ShareEndpointIdent;
3233
use crate::tenant::Tenant;
3334

3435
// serde is required by `DatabaseType`
@@ -59,18 +60,6 @@ impl Display for ShareConsumer {
5960
}
6061
}
6162

62-
#[derive(Clone, Debug, Default, Eq, PartialEq)]
63-
pub struct ShareEndpointIdent {
64-
pub tenant: String,
65-
pub endpoint: String,
66-
}
67-
68-
impl Display for ShareEndpointIdent {
69-
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
70-
write!(f, "'{}'/'{}'", self.tenant, self.endpoint)
71-
}
72-
}
73-
7463
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
7564
pub struct ShowSharesReq {
7665
pub tenant: String,
@@ -303,7 +292,7 @@ pub struct UpsertShareEndpointReply {
303292

304293
#[derive(Clone, Debug, PartialEq, Eq)]
305294
pub struct GetShareEndpointReq {
306-
pub tenant: String,
295+
pub tenant: Tenant,
307296
// If `endpoint` is not None, return the specified endpoint,
308297
// else return all share endpoints meta of tenant
309298
pub endpoint: Option<String>,
@@ -952,35 +941,6 @@ mod kvapi_key_impl {
952941
}
953942
}
954943

955-
/// __fd_share/<tenant>/<share_endpoint_name> -> ShareEndpointId
956-
impl kvapi::Key for ShareEndpointIdent {
957-
const PREFIX: &'static str = "__fd_share_endpoint";
958-
959-
type ValueType = ShareEndpointId;
960-
961-
/// It belongs to a tenant
962-
fn parent(&self) -> Option<String> {
963-
Some(Tenant::new(&self.tenant).to_string_key())
964-
}
965-
966-
fn to_string_key(&self) -> String {
967-
kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
968-
.push_str(&self.tenant)
969-
.push_str(&self.endpoint)
970-
.done()
971-
}
972-
973-
fn from_str_key(s: &str) -> Result<Self, kvapi::KeyError> {
974-
let mut p = kvapi::KeyParser::new_prefixed(s, Self::PREFIX)?;
975-
976-
let tenant = p.next_str()?;
977-
let endpoint = p.next_str()?;
978-
p.done()?;
979-
980-
Ok(ShareEndpointIdent { tenant, endpoint })
981-
}
982-
}
983-
984944
/// __fd_share_endpoint_id/<share_endpoint_id> -> <share_meta>
985945
impl kvapi::Key for ShareEndpointId {
986946
const PREFIX: &'static str = "__fd_share_endpoint_id";
@@ -1064,13 +1024,6 @@ mod kvapi_key_impl {
10641024
}
10651025
}
10661026

1067-
impl kvapi::Value for ShareEndpointId {
1068-
/// ShareEndpointId is id of the two level `name->id,id->value` mapping
1069-
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
1070-
[self.to_string_key()]
1071-
}
1072-
}
1073-
10741027
impl kvapi::Value for ShareEndpointMeta {
10751028
fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
10761029
[]

0 commit comments

Comments
 (0)