Skip to content

Commit 3fb0bc9

Browse files
JanZachmannJanZachmann
committed
refactor: extract HTTP response handling logic from macros to helper functions
Extract repetitive HTTP response handling logic from macros into reusable helper functions and a helper macro to improve maintainability and reduce code duplication. Changes: - Add build_api_url() helper for URL construction - Add handle_json_response() helper for extracting JSON response bodies - Add handle_status_response() helper for checking HTTP status codes - Add with_loading! helper macro for loading state management - Update unauth_post! macro to use new helpers (3 patterns) - Update auth_post! macro to use new helpers (3 patterns) - Reduce macro complexity by moving logic to testable functions This refactoring: - Reduces code duplication across macro patterns - Centralizes loading state and render command logic - Improves debuggability by moving logic to named functions - Makes response handling more testable and maintainable - Centralizes URL construction logic - Maintains existing functionality and API Addresses PR #69 finding r2563954064 Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>
1 parent d51890f commit 3fb0bc9

File tree

1 file changed

+100
-106
lines changed

1 file changed

+100
-106
lines changed

‎src/app/src/macros.rs‎

Lines changed: 100 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,50 @@ pub fn http_error(action: &str, status: impl std::fmt::Display) -> String {
6868
format!("{action} failed: HTTP {status}")
6969
}
7070

71+
/// Helper function to build full API URL from endpoint
72+
pub fn build_api_url(endpoint: &str) -> String {
73+
format!("{}{endpoint}", crate::API_BASE_URL)
74+
}
75+
76+
/// Helper function to handle HTTP response with JSON body extraction
77+
pub fn handle_json_response<T>(
78+
result: Result<crux_http::Response<T>, crux_http::HttpError>,
79+
) -> Result<T, String> {
80+
match result {
81+
Ok(mut response) => match response.take_body() {
82+
Some(data) => Ok(data),
83+
None => Err("Empty response body".to_string()),
84+
},
85+
Err(e) => Err(e.to_string()),
86+
}
87+
}
88+
89+
/// Helper function to handle HTTP response with status check
90+
pub fn handle_status_response(
91+
result: Result<crux_http::Response<Vec<u8>>, crux_http::HttpError>,
92+
action: &str,
93+
) -> Result<(), String> {
94+
match result {
95+
Ok(response) => {
96+
if response.status().is_success() {
97+
Ok(())
98+
} else {
99+
Err(http_error(action, response.status()))
100+
}
101+
}
102+
Err(e) => Err(e.to_string()),
103+
}
104+
}
105+
106+
/// Helper macro to set loading state and wrap command with render
107+
#[macro_export]
108+
macro_rules! with_loading {
109+
($model:expr, $command:expr) => {{
110+
$model.is_loading = true;
111+
crux_core::Command::all([crux_core::render::render(), $command])
112+
}};
113+
}
114+
71115
/// Macro for unauthenticated POST requests with standard error handling.
72116
/// Used for login, password setup, and other pre-authentication endpoints.
73117
///
@@ -98,73 +142,50 @@ pub fn http_error(action: &str, status: impl std::fmt::Display) -> String {
98142
#[macro_export]
99143
macro_rules! unauth_post {
100144
// Pattern 1: POST with JSON body expecting JSON response
101-
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr, expect_json: $response_type:ty) => {{
102-
$model.is_loading = true;
103-
crux_core::Command::all([
104-
crux_core::render::render(),
105-
$crate::HttpCmd::post(format!("{}{}", $crate::API_BASE_URL, $endpoint))
145+
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr, expect_json: $response_type:ty) => {
146+
$crate::with_loading!(
147+
$model,
148+
$crate::HttpCmd::post($crate::macros::build_api_url($endpoint))
106149
.header("Content-Type", "application/json")
107150
.body_json($body)
108151
.expect(&format!("Failed to serialize {} request", $action))
109152
.expect_json::<$response_type>()
110153
.build()
111-
.then_send(|result| match result {
112-
Ok(mut response) => match response.take_body() {
113-
Some(data) => $crate::Event::$response_event(Ok(data)),
114-
None => {
115-
$crate::Event::$response_event(Err("Empty response body".to_string()))
116-
}
117-
},
118-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
119-
}),
120-
])
121-
}};
154+
.then_send(|result| {
155+
$crate::Event::$response_event($crate::macros::handle_json_response(result))
156+
})
157+
)
158+
};
122159

123160
// Pattern 2: POST with JSON body expecting status only
124-
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr) => {{
125-
$model.is_loading = true;
126-
crux_core::Command::all([
127-
crux_core::render::render(),
128-
$crate::HttpCmd::post(format!("{}{}", $crate::API_BASE_URL, $endpoint))
161+
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr) => {
162+
$crate::with_loading!(
163+
$model,
164+
$crate::HttpCmd::post($crate::macros::build_api_url($endpoint))
129165
.header("Content-Type", "application/json")
130166
.body_json($body)
131167
.expect(&format!("Failed to serialize {} request", $action))
132168
.build()
133-
.then_send(|result| match result {
134-
Ok(response) => {
135-
if response.status().is_success() {
136-
$crate::Event::$response_event(Ok(()))
137-
} else {
138-
$crate::Event::$response_event(Err($crate::macros::http_error(
139-
$action,
140-
response.status(),
141-
)))
142-
}
143-
}
144-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
145-
}),
146-
])
147-
}};
169+
.then_send(|result| {
170+
$crate::Event::$response_event($crate::macros::handle_status_response(
171+
result, $action,
172+
))
173+
})
174+
)
175+
};
148176

149177
// Pattern 3: GET expecting JSON response
150-
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, method: get, expect_json: $response_type:ty) => {{
151-
$model.is_loading = true;
152-
crux_core::Command::all([
153-
crux_core::render::render(),
154-
$crate::HttpCmd::get(format!("{}{}", $crate::API_BASE_URL, $endpoint))
178+
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, method: get, expect_json: $response_type:ty) => {
179+
$crate::with_loading!(
180+
$model,
181+
$crate::HttpCmd::get($crate::macros::build_api_url($endpoint))
155182
.expect_json::<$response_type>()
156183
.build()
157-
.then_send(|result| match result {
158-
Ok(mut response) => match response.take_body() {
159-
Some(data) => $crate::Event::$response_event(Ok(data)),
160-
None => {
161-
$crate::Event::$response_event(Err("Empty response body".to_string()))
162-
}
163-
},
164-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
165-
}),
166-
])
167-
}};
184+
.then_send(|result| {
185+
$crate::Event::$response_event($crate::macros::handle_json_response(result))
186+
})
187+
)
188+
};
168189
}
169190

170191
/// Macro for authenticated POST requests with standard error handling.
@@ -194,88 +215,61 @@ macro_rules! unauth_post {
194215
macro_rules! auth_post {
195216
// Pattern 1: Simple POST without body
196217
($model:expr, $endpoint:expr, $response_event:ident, $action:expr) => {{
197-
$model.is_loading = true;
198218
if let Some(token) = &$model.auth_token {
199-
crux_core::Command::all([
200-
crux_core::render::render(),
201-
$crate::HttpCmd::post(format!("{}{}", $crate::API_BASE_URL, $endpoint))
219+
$crate::with_loading!(
220+
$model,
221+
$crate::HttpCmd::post($crate::macros::build_api_url($endpoint))
202222
.header("Authorization", format!("Bearer {token}"))
203223
.build()
204-
.then_send(|result| match result {
205-
Ok(response) => {
206-
if response.status().is_success() {
207-
$crate::Event::$response_event(Ok(()))
208-
} else {
209-
$crate::Event::$response_event(Err($crate::macros::http_error(
210-
$action,
211-
response.status(),
212-
)))
213-
}
214-
}
215-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
216-
}),
217-
])
224+
.then_send(|result| {
225+
$crate::Event::$response_event($crate::macros::handle_status_response(
226+
result, $action,
227+
))
228+
})
229+
)
218230
} else {
219231
crux_core::render::render()
220232
}
221233
}};
222234

223235
// Pattern 2: POST with JSON body
224236
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr) => {{
225-
$model.is_loading = true;
226237
if let Some(token) = &$model.auth_token {
227-
crux_core::Command::all([
228-
crux_core::render::render(),
229-
$crate::HttpCmd::post(format!("{}{}", $crate::API_BASE_URL, $endpoint))
238+
$crate::with_loading!(
239+
$model,
240+
$crate::HttpCmd::post($crate::macros::build_api_url($endpoint))
230241
.header("Authorization", format!("Bearer {token}"))
231242
.header("Content-Type", "application/json")
232243
.body_json($body)
233244
.expect(&format!("Failed to serialize {} request", $action))
234245
.build()
235-
.then_send(|result| match result {
236-
Ok(response) => {
237-
if response.status().is_success() {
238-
$crate::Event::$response_event(Ok(()))
239-
} else {
240-
$crate::Event::$response_event(Err($crate::macros::http_error(
241-
$action,
242-
response.status(),
243-
)))
244-
}
245-
}
246-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
247-
}),
248-
])
246+
.then_send(|result| {
247+
$crate::Event::$response_event($crate::macros::handle_status_response(
248+
result, $action,
249+
))
250+
})
251+
)
249252
} else {
250253
crux_core::render::render()
251254
}
252255
}};
253256

254257
// Pattern 3: POST with string body
255258
($model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_string: $body:expr) => {{
256-
$model.is_loading = true;
257259
if let Some(token) = &$model.auth_token {
258-
crux_core::Command::all([
259-
crux_core::render::render(),
260-
$crate::HttpCmd::post(format!("{}{}", $crate::API_BASE_URL, $endpoint))
260+
$crate::with_loading!(
261+
$model,
262+
$crate::HttpCmd::post($crate::macros::build_api_url($endpoint))
261263
.header("Authorization", format!("Bearer {token}"))
262264
.header("Content-Type", "application/json")
263265
.body_string($body)
264266
.build()
265-
.then_send(|result| match result {
266-
Ok(response) => {
267-
if response.status().is_success() {
268-
$crate::Event::$response_event(Ok(()))
269-
} else {
270-
$crate::Event::$response_event(Err($crate::macros::http_error(
271-
$action,
272-
response.status(),
273-
)))
274-
}
275-
}
276-
Err(e) => $crate::Event::$response_event(Err(e.to_string())),
277-
}),
278-
])
267+
.then_send(|result| {
268+
$crate::Event::$response_event($crate::macros::handle_status_response(
269+
result, $action,
270+
))
271+
})
272+
)
279273
} else {
280274
crux_core::render::render()
281275
}

0 commit comments

Comments
 (0)