Skip to content

Commit f127ad8

Browse files
DanCodedThisDanCodedThis
authored
Volumes search through query engine & removed unintended info (#859)
* Volumes search and #850 ready (rebased) * After rebase * Requested changes
1 parent a663e24 commit f127ad8

File tree

8 files changed

+291
-122
lines changed

8 files changed

+291
-122
lines changed

‎crates/api-ui/src/databases/handlers.rs

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use crate::OrderDirection;
21
use crate::state::AppState;
2+
use crate::{OrderDirection, apply_parameters};
33
use crate::{
44
SearchParameters,
55
databases::error::{DatabasesAPIError, DatabasesResult},
@@ -227,25 +227,7 @@ pub async fn list_databases(
227227
) -> DatabasesResult<Json<DatabasesResponse>> {
228228
let context = QueryContext::default();
229229
let sql_string = "SELECT * FROM slatedb.public.databases".to_string();
230-
let sql_string = parameters.search.map_or_else(|| sql_string.clone(), |search|
231-
format!("{sql_string} WHERE (database_name ILIKE '%{search}%' OR volume_name ILIKE '%{search}%')")
232-
);
233-
let sql_string = parameters.order_by.map_or_else(
234-
|| format!("{sql_string} ORDER BY database_name"),
235-
|order_by| format!("{sql_string} ORDER BY {order_by}"),
236-
);
237-
let sql_string = parameters.order_direction.map_or_else(
238-
|| format!("{sql_string} DESC"),
239-
|order_direction| format!("{sql_string} {order_direction}"),
240-
);
241-
let sql_string = parameters.offset.map_or_else(
242-
|| sql_string.clone(),
243-
|offset| format!("{sql_string} OFFSET {offset}"),
244-
);
245-
let sql_string = parameters.limit.map_or_else(
246-
|| sql_string.clone(),
247-
|limit| format!("{sql_string} LIMIT {limit}"),
248-
);
230+
let sql_string = apply_parameters(&sql_string, parameters, &["database_name", "volume_name"]);
249231
let QueryResultData { records, .. } = state
250232
.execution_svc
251233
.query(&session_id, sql_string.as_str(), context)

‎crates/api-ui/src/lib.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,54 @@ fn downcast_int64_column<'a>(
8585
source: DataFusionError::Internal(format!("Missing or invalid column: '{name}'")),
8686
})
8787
}
88+
89+
fn apply_parameters(
90+
sql_string: &str,
91+
parameters: SearchParameters,
92+
search_columns: &[&str],
93+
) -> String {
94+
let sql_string = parameters.search.map_or_else(
95+
|| sql_string.to_string(),
96+
|search| {
97+
let separator = format!(" ILIKE '%{search}%' OR ");
98+
let sql_string = sql_string.find("WHERE").map_or_else(
99+
|| {
100+
//if there is no WHERE clause
101+
let sql_string =
102+
format!("{sql_string} WHERE ({}", search_columns.join(&separator));
103+
sql_string
104+
},
105+
|_| {
106+
//if there is a WHERE clause
107+
let sql_string =
108+
format!("{sql_string} AND ({}", search_columns.join(&separator));
109+
sql_string
110+
},
111+
);
112+
format!("{sql_string} ILIKE '%{search}%')")
113+
},
114+
);
115+
//Default order by is the first search column or created at
116+
let sql_string = parameters.order_by.map_or_else(
117+
|| {
118+
format!(
119+
"{sql_string} ORDER BY {}",
120+
search_columns.first().unwrap_or(&"created_at")
121+
)
122+
},
123+
|order_by| format!("{sql_string} ORDER BY {order_by}"),
124+
);
125+
let sql_string = parameters.order_direction.map_or_else(
126+
|| format!("{sql_string} DESC"),
127+
|order_direction| format!("{sql_string} {order_direction}"),
128+
);
129+
let sql_string = parameters.offset.map_or_else(
130+
|| sql_string.clone(),
131+
|offset| format!("{sql_string} OFFSET {offset}"),
132+
);
133+
134+
parameters.limit.map_or_else(
135+
|| sql_string.clone(),
136+
|limit| format!("{sql_string} LIMIT {limit}"),
137+
)
138+
}

‎crates/api-ui/src/schemas/handlers.rs

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use crate::OrderDirection;
21
use crate::state::AppState;
2+
use crate::{OrderDirection, apply_parameters};
33
use crate::{
44
SearchParameters, downcast_string_column,
55
error::ErrorResponse,
@@ -269,25 +269,7 @@ pub async fn list_schemas(
269269
"SELECT * FROM slatedb.public.schemas WHERE database_name = '{}'",
270270
database_name.clone()
271271
);
272-
let sql_string = parameters.search.map_or_else(|| sql_string.clone(), |search|
273-
format!("{sql_string} AND (schema_name ILIKE '%{search}%' OR database_name ILIKE '%{search}%')")
274-
);
275-
let sql_string = parameters.order_by.map_or_else(
276-
|| format!("{sql_string} ORDER BY schema_name"),
277-
|order_by| format!("{sql_string} ORDER BY {order_by}"),
278-
);
279-
let sql_string = parameters.order_direction.map_or_else(
280-
|| format!("{sql_string} DESC"),
281-
|order_direction| format!("{sql_string} {order_direction}"),
282-
);
283-
let sql_string = parameters.offset.map_or_else(
284-
|| sql_string.clone(),
285-
|offset| format!("{sql_string} OFFSET {offset}"),
286-
);
287-
let sql_string = parameters.limit.map_or_else(
288-
|| sql_string.clone(),
289-
|limit| format!("{sql_string} LIMIT {limit}"),
290-
);
272+
let sql_string = apply_parameters(&sql_string, parameters, &["schema_name", "database_name"]);
291273
let QueryResultData { records, .. } = state
292274
.execution_svc
293275
.query(&session_id, sql_string.as_str(), context)

‎crates/api-ui/src/tables/handlers.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::tables::models::{
99
TablePreviewDataResponse, TablePreviewDataRow, TableStatistics, TableStatisticsResponse,
1010
TableUploadPayload, TableUploadResponse, TablesResponse, UploadParameters,
1111
};
12-
use crate::{SearchParameters, downcast_int64_column, downcast_string_column};
12+
use crate::{SearchParameters, apply_parameters, downcast_int64_column, downcast_string_column};
1313
use api_sessions::DFSessionId;
1414
use axum::extract::Query;
1515
use axum::{
@@ -388,24 +388,16 @@ pub async fn get_tables(
388388
schema_name.clone(),
389389
database_name.clone()
390390
);
391-
let sql_string = parameters.search.map_or_else(|| sql_string.clone(), |search|
392-
format!("{sql_string} AND (table_name ILIKE '%{search}%' OR volume_name ILIKE '%{search}%' OR table_type ILIKE '%{search}%' OR table_format ILIKE '%{search}%' OR owner ILIKE '%{search}%')")
393-
);
394-
let sql_string = parameters.order_by.map_or_else(
395-
|| format!("{sql_string} ORDER BY table_name"),
396-
|order_by| format!("{sql_string} ORDER BY {order_by}"),
397-
);
398-
let sql_string = parameters.order_direction.map_or_else(
399-
|| format!("{sql_string} DESC"),
400-
|order_direction| format!("{sql_string} {order_direction}"),
401-
);
402-
let sql_string = parameters.offset.map_or_else(
403-
|| sql_string.clone(),
404-
|offset| format!("{sql_string} OFFSET {offset}"),
405-
);
406-
let sql_string = parameters.limit.map_or_else(
407-
|| sql_string.clone(),
408-
|limit| format!("{sql_string} LIMIT {limit}"),
391+
let sql_string = apply_parameters(
392+
&sql_string,
393+
parameters,
394+
&[
395+
"table_name",
396+
"volume_name",
397+
"table_type",
398+
"table_format",
399+
"owner",
400+
],
409401
);
410402
let QueryResultData { records, .. } = state
411403
.execution_svc

‎crates/api-ui/src/tests/volumes.rs

Lines changed: 157 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,36 @@
11
#![allow(clippy::unwrap_used, clippy::expect_used)]
2-
use crate::tests::common::{Entity, Op, ui_test_op};
2+
3+
use crate::tests::common::{Entity, Op, req, ui_test_op};
34
use crate::tests::server::run_test_server;
4-
use crate::volumes::models::{Volume, VolumeCreatePayload, VolumeCreateResponse};
5+
use crate::volumes::models::{Volume, VolumeCreatePayload, VolumeCreateResponse, VolumesResponse};
56
use core_metastore::Volume as MetastoreVolume;
67
use core_metastore::{
78
AwsAccessKeyCredentials, AwsCredentials, FileVolume as MetastoreFileVolume,
89
S3Volume as MetastoreS3Volume, VolumeType as MetastoreVolumeType,
910
};
11+
use http::Method;
1012
use serde_json;
1113

1214
#[tokio::test]
1315
#[allow(clippy::too_many_lines)]
14-
async fn test_ui_volumes_memory() {
16+
async fn test_ui_volumes() {
1517
let addr = run_test_server().await;
18+
let client = reqwest::Client::new();
1619

1720
// memory volume with empty ident create Ok
1821
let expected = VolumeCreatePayload {
1922
data: Volume::from(MetastoreVolume {
20-
ident: String::new(),
23+
ident: "embucket1".to_string(),
2124
volume: MetastoreVolumeType::Memory,
2225
}),
2326
};
2427
let res = ui_test_op(addr, Op::Create, None, &Entity::Volume(expected.clone())).await;
2528
assert_eq!(200, res.status());
2629
let created = res.json::<VolumeCreateResponse>().await.unwrap();
2730
assert_eq!(expected.data, created.data);
28-
}
29-
30-
#[tokio::test]
31-
#[allow(clippy::too_many_lines)]
32-
async fn test_ui_volumes_file() {
33-
let addr = run_test_server().await;
3431

3532
// memory volume with empty ident create Ok
36-
let payload = r#"{"name":"","type": "file", "path":"/tmp/data"}"#;
33+
let payload = r#"{"name":"embucket2","type": "file", "path":"/tmp/data"}"#;
3734
let expected: VolumeCreatePayload = serde_json::from_str(payload).unwrap();
3835
let res = ui_test_op(addr, Op::Create, None, &Entity::Volume(expected.clone())).await;
3936
// let res = create_test_volume(addr, &expected).await;
@@ -43,7 +40,7 @@ async fn test_ui_volumes_file() {
4340

4441
let expected = VolumeCreatePayload {
4542
data: Volume::from(MetastoreVolume {
46-
ident: String::new(),
43+
ident: "embucket2".to_string(),
4744
volume: MetastoreVolumeType::File(MetastoreFileVolume {
4845
path: "/tmp/data".to_string(),
4946
}),
@@ -52,17 +49,11 @@ async fn test_ui_volumes_file() {
5249
let res = ui_test_op(addr, Op::Create, None, &Entity::Volume(expected.clone())).await;
5350
// let res = create_test_volume(addr, &expected).await;
5451
assert_eq!(409, res.status());
55-
}
56-
57-
#[tokio::test]
58-
#[allow(clippy::too_many_lines)]
59-
async fn test_ui_volumes_s3() {
60-
let addr = run_test_server().await;
6152

6253
// memory volume with empty ident create Ok
6354
let expected = VolumeCreatePayload {
6455
data: Volume::from(MetastoreVolume {
65-
ident: String::new(),
56+
ident: "embucket3".to_string(),
6657
volume: MetastoreVolumeType::S3(MetastoreS3Volume {
6758
region: Some("us-west-1".to_string()),
6859
bucket: Some("embucket".to_string()),
@@ -81,4 +72,151 @@ async fn test_ui_volumes_s3() {
8172
assert_eq!(200, res.status());
8273
let created = res.json::<VolumeCreateResponse>().await.unwrap();
8374
assert_eq!(expected.data, created.data);
75+
76+
//Get list volumes
77+
let res = req(
78+
&client,
79+
Method::GET,
80+
&format!("http://{addr}/ui/volumes",).to_string(),
81+
String::new(),
82+
)
83+
.await
84+
.unwrap();
85+
assert_eq!(http::StatusCode::OK, res.status());
86+
let volumes_response: VolumesResponse = res.json().await.unwrap();
87+
assert_eq!(3, volumes_response.items.len());
88+
assert_eq!(
89+
"embucket3".to_string(),
90+
volumes_response.items.first().unwrap().name
91+
);
92+
93+
//Get list volumes with parameters
94+
let res = req(
95+
&client,
96+
Method::GET,
97+
&format!("http://{addr}/ui/volumes?limit=2",).to_string(),
98+
String::new(),
99+
)
100+
.await
101+
.unwrap();
102+
assert_eq!(http::StatusCode::OK, res.status());
103+
let volumes_response: VolumesResponse = res.json().await.unwrap();
104+
assert_eq!(2, volumes_response.items.len());
105+
assert_eq!(
106+
"embucket2".to_string(),
107+
volumes_response.items.last().unwrap().name
108+
);
109+
110+
//Get list volumes with parameters
111+
let res = req(
112+
&client,
113+
Method::GET,
114+
&format!("http://{addr}/ui/volumes?offset=2",).to_string(),
115+
String::new(),
116+
)
117+
.await
118+
.unwrap();
119+
assert_eq!(http::StatusCode::OK, res.status());
120+
let volumes_response: VolumesResponse = res.json().await.unwrap();
121+
assert_eq!(1, volumes_response.items.len());
122+
assert_eq!(
123+
"embucket1".to_string(),
124+
volumes_response.items.first().unwrap().name
125+
);
126+
127+
//Create a volume with diffrent name
128+
let expected = VolumeCreatePayload {
129+
data: Volume::from(MetastoreVolume {
130+
ident: "icebucket1".to_string(),
131+
volume: MetastoreVolumeType::Memory,
132+
}),
133+
};
134+
let res = ui_test_op(addr, Op::Create, None, &Entity::Volume(expected.clone())).await;
135+
assert_eq!(200, res.status());
136+
let created = res.json::<VolumeCreateResponse>().await.unwrap();
137+
assert_eq!(expected.data, created.data);
138+
139+
//Get list volumes
140+
let res = req(
141+
&client,
142+
Method::GET,
143+
&format!("http://{addr}/ui/volumes",).to_string(),
144+
String::new(),
145+
)
146+
.await
147+
.unwrap();
148+
assert_eq!(http::StatusCode::OK, res.status());
149+
let volumes_response: VolumesResponse = res.json().await.unwrap();
150+
assert_eq!(4, volumes_response.items.len());
151+
152+
//Get list volumes with parameters
153+
let res = req(
154+
&client,
155+
Method::GET,
156+
&format!("http://{addr}/ui/volumes?search=embucket",).to_string(),
157+
String::new(),
158+
)
159+
.await
160+
.unwrap();
161+
assert_eq!(http::StatusCode::OK, res.status());
162+
let volumes_response: VolumesResponse = res.json().await.unwrap();
163+
assert_eq!(3, volumes_response.items.len());
164+
165+
//Get list volumes with parameters
166+
let res = req(
167+
&client,
168+
Method::GET,
169+
&format!("http://{addr}/ui/volumes?search=embucket&orderDirection=ASC",).to_string(),
170+
String::new(),
171+
)
172+
.await
173+
.unwrap();
174+
assert_eq!(http::StatusCode::OK, res.status());
175+
let volumes_response: VolumesResponse = res.json().await.unwrap();
176+
assert_eq!(3, volumes_response.items.len());
177+
assert_eq!(
178+
"embucket1".to_string(),
179+
volumes_response.items.first().unwrap().name
180+
);
181+
182+
//Get list volumes with parameters
183+
let res = req(
184+
&client,
185+
Method::GET,
186+
&format!("http://{addr}/ui/volumes?search=ice",).to_string(),
187+
String::new(),
188+
)
189+
.await
190+
.unwrap();
191+
assert_eq!(http::StatusCode::OK, res.status());
192+
let volumes_response: VolumesResponse = res.json().await.unwrap();
193+
assert_eq!(1, volumes_response.items.len());
194+
assert_eq!(
195+
"icebucket1".to_string(),
196+
volumes_response.items.first().unwrap().name
197+
);
198+
199+
//Delete volume
200+
let res = req(
201+
&client,
202+
Method::DELETE,
203+
&format!("http://{addr}/ui/volumes/embucket1",).to_string(),
204+
String::new(),
205+
)
206+
.await
207+
.unwrap();
208+
assert_eq!(http::StatusCode::OK, res.status());
209+
210+
//Get list volumes
211+
let res = req(
212+
&client,
213+
Method::GET,
214+
&format!("http://{addr}/ui/volumes",).to_string(),
215+
String::new(),
216+
)
217+
.await
218+
.unwrap();
219+
assert_eq!(http::StatusCode::OK, res.status());
220+
let volumes_response: VolumesResponse = res.json().await.unwrap();
221+
assert_eq!(3, volumes_response.items.len());
84222
}

0 commit comments

Comments
 (0)