diff --git a/Cargo.lock b/Cargo.lock index 7f3dd4ba..ff548b3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7967,6 +7967,7 @@ dependencies = [ "eventsource-stream", "futures-lite", "jsonwebtoken 9.3.1", + "lazy_static", "parking_lot", "reqwest", "serde", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 87f581ec..e497c247 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -25,6 +25,7 @@ url = "2.5.4" [dev-dependencies] base64 = { version = "0.22.1", default-features = false, features = ["alloc"] } +lazy_static = "1.4.0" reqwest = { version = "0.12.8", features = ["stream", "multipart", "json"] } temp-dir = "0.1.13" tokio = { version = "1.43.0", features = ["macros", "rt-multi-thread"] } diff --git a/crates/client/tests/integration_test.rs b/crates/client/tests/integration_test.rs index 9bd288d6..e51ed48a 100644 --- a/crates/client/tests/integration_test.rs +++ b/crates/client/tests/integration_test.rs @@ -1,5 +1,6 @@ use base64::prelude::*; use futures_lite::StreamExt; +use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; use serde_json::json; use temp_dir::TempDir; @@ -18,6 +19,9 @@ impl Drop for Server { } const PORT: u16 = 4057; +lazy_static! { + static ref SITE: String = format!("http://127.0.0.1:{PORT}"); +} fn start_server() -> Result { let cwd = std::env::current_dir()?; @@ -52,7 +56,7 @@ fn start_server() -> Result { runtime.block_on(async { let client = reqwest::Client::new(); - let url = format!("http://127.0.0.1:{PORT}/api/healthcheck"); + let url = format!("{site}/api/healthcheck", site = *SITE); for _ in 0..100 { let response = client.get(&url).send().await; @@ -120,7 +124,7 @@ struct Comment { } async fn connect() -> Client { - let client = Client::new(&format!("http://127.0.0.1:{PORT}"), None).unwrap(); + let client = Client::new(&SITE, None).unwrap(); let _ = client.login("admin@localhost", "secret").await.unwrap(); return client; } @@ -483,10 +487,12 @@ async fn file_upload_json_base64_test() { let single_file_fname = single_file.filename.as_deref().unwrap(); for single_file_url in [ format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table/{record_id}/file/single_file" + "{site}/api/records/v1/file_upload_table/{record_id}/file/single_file", + site = *SITE ), format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table/{record_id}/files/single_file/{single_file_fname}" + "{site}/api/records/v1/file_upload_table/{record_id}/files/single_file/{single_file_fname}", + site = *SITE ), ] { let single_file_response = http_client.get(&single_file_url).send().await.unwrap(); @@ -501,7 +507,9 @@ async fn file_upload_json_base64_test() { let multi_file_1_response = http_client .get(&format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table/{record_id}/files/multiple_files/0", + "{site}/api/records/v1/file_upload_table/{record_id}/files/multiple_files/{filename}", + site = *SITE, + filename = multiple_files[0].filename.as_ref().unwrap() )) .send() .await @@ -509,10 +517,11 @@ async fn file_upload_json_base64_test() { let multi_file_1_bytes = multi_file_1_response.bytes().await.unwrap(); assert_eq!(multi_file_1_bytes, test_bytes2); - let filename = multiple_files[1].filename.as_ref().unwrap(); let multi_file_2_response = http_client .get(&format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table/{record_id}/files/multiple_files/{filename}" + "{site}/api/records/v1/file_upload_table/{record_id}/files/multiple_files/{filename}", + site = *SITE, + filename = multiple_files[1].filename.as_ref().unwrap(), )) .send() .await @@ -553,7 +562,8 @@ async fn file_upload_multipart_form_test() { let response: RecordIdResponse = http_client .post(&format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table" + "{site}/api/records/v1/file_upload_table", + site = *SITE )) .multipart(form) .send() @@ -567,7 +577,8 @@ async fn file_upload_multipart_form_test() { let single_file_response = http_client .get(&format!( - "http://127.0.0.1:{PORT}/api/records/v1/file_upload_table/{record_id}/file/single_file", + "{site}/api/records/v1/file_upload_table/{record_id}/file/single_file", + site = *SITE )) .send() .await diff --git a/crates/core/src/admin/rows/read_files.rs b/crates/core/src/admin/rows/read_files.rs index 8a28afa6..27a03a92 100644 --- a/crates/core/src/admin/rows/read_files.rs +++ b/crates/core/src/admin/rows/read_files.rs @@ -37,20 +37,23 @@ pub async fn read_files_handler( "Table {table_name:?} not found" ))); }; - let pk_col = &query.pk_column; - - let Some((_index, col)) = schema_metadata.column_by_name(pk_col) else { - return Err(Error::Precondition(format!("Missing column: {pk_col}"))); + let Some((_index, pk_col)) = schema_metadata.column_by_name(&query.pk_column) else { + return Err(Error::Precondition(format!( + "Missing PK column: {}", + query.pk_column + ))); }; - if !col.is_primary() { - return Err(Error::Precondition(format!("Not a primary key: {pk_col}"))); + if !pk_col.is_primary() { + return Err(Error::Precondition(format!( + "Not a primary key: {pk_col:?}" + ))); } let Some((index, file_col_metadata)) = schema_metadata.column_by_name(&query.file_column_name) else { return Err(Error::Precondition(format!( - "Missing column: {}", + "Missing file column: {}", query.file_column_name ))); }; @@ -61,7 +64,7 @@ pub async fn read_files_handler( ))); }; - let pk_value = flat_json_to_value(col.data_type, query.pk_value, true)?; + let pk_value = flat_json_to_value(pk_col.data_type, query.pk_value, true)?; let FileUploads(mut file_uploads) = run_get_files_query( &state, diff --git a/crates/core/src/records/mod.rs b/crates/core/src/records/mod.rs index f238e329..098f018a 100644 --- a/crates/core/src/records/mod.rs +++ b/crates/core/src/records/mod.rs @@ -74,7 +74,7 @@ pub(crate) fn router(enable_transactions: bool) -> Router { get(read_record::get_uploaded_file_from_record_handler), ) .route( - &format!("/{RECORD_API_PATH}/{{name}}/{{record}}/files/{{column_name}}/{{file_id}}"), + &format!("/{RECORD_API_PATH}/{{name}}/{{record}}/files/{{column_name}}/{{file_name}}"), get(read_record::get_uploaded_files_from_record_handler), ) .route( diff --git a/crates/core/src/records/read_record.rs b/crates/core/src/records/read_record.rs index 4686d6c0..0b9d964c 100644 --- a/crates/core/src/records/read_record.rs +++ b/crates/core/src/records/read_record.rs @@ -4,6 +4,7 @@ use axum::{ response::Response, }; use serde::Deserialize; +use trailbase_schema::FileUploads; use crate::auth::user::User; use crate::records::expand::expand_tables; @@ -210,13 +211,13 @@ type GetUploadedFilesFromRecordPath = Path<( String, // Column name // NOTE: We may want to remove index-based access in the future. A stable, unique identifier // makes a lot more sense in the context of mutations, caching, ... . - String, // Index or filename + String, // Filename )>; /// Read single file from list associated with record. #[utoipa::path( get, - path = "/{name}/{record}/files/{column_name}/{file_id}", + path = "/{name}/{record}/files/{column_name}/{file_name}", tag = "records", responses( (status = 200, description = "File contents.") @@ -224,7 +225,7 @@ type GetUploadedFilesFromRecordPath = Path<( )] pub async fn get_uploaded_files_from_record_handler( State(state): State, - Path((api_name, record, column_name, file_id)): GetUploadedFilesFromRecordPath, + Path((api_name, record, column_name, file_name)): GetUploadedFilesFromRecordPath, user: Option, ) -> Result { let Some(api) = state.lookup_record_api(&api_name) else { @@ -240,7 +241,7 @@ pub async fn get_uploaded_files_from_record_handler( return Err(RecordError::BadRequest("Invalid field/column name")); }; - let mut file_uploads = run_get_files_query( + let FileUploads(file_uploads) = run_get_files_query( &state, api.table_name(), column, @@ -251,22 +252,14 @@ pub async fn get_uploaded_files_from_record_handler( .await .map_err(|err| RecordError::Internal(err.into()))?; - let file_upload = if let Ok(index) = file_id.parse::() { - if index < file_uploads.0.len() { - Some(file_uploads.0.remove(index)) - } else { - None - } - } else { - file_uploads.0.into_iter().find(|f| f.filename() == file_id) - }; + let file_upload = file_uploads + .into_iter() + .find(|f| f.filename() == file_name) + .ok_or_else(|| RecordError::RecordNotFound)?; - return read_file_into_response( - &state, - file_upload.ok_or_else(|| RecordError::RecordNotFound)?, - ) - .await - .map_err(|err| RecordError::Internal(err.into())); + return read_file_into_response(&state, file_upload) + .await + .map_err(|err| RecordError::Internal(err.into())); } #[inline] @@ -784,7 +777,7 @@ mod test { API_NAME.to_string(), record_id.clone(), "files".to_string(), - 0.to_string(), + files[0].filename().to_string(), )), None, ) @@ -799,7 +792,7 @@ mod test { API_NAME.to_string(), record_id.clone(), "files".to_string(), - 1.to_string(), + files[1].filename().to_string(), )), None, ) diff --git a/docs/openapi/schema.json b/docs/openapi/schema.json index 6845c137..c363bc11 100644 --- a/docs/openapi/schema.json +++ b/docs/openapi/schema.json @@ -730,7 +730,7 @@ } } }, - "/api/records/v1/{name}/{record}/files/{column_name}/{file_id}": { + "/api/records/v1/{name}/{record}/files/{column_name}/{file_name}": { "get": { "tags": ["records"], "summary": "Read single file from list associated with record.",