Skip to main content

max / goingson

GO-33-2: dedupe IN-clause placeholder building into utils::bind_placeholders Add bind_placeholders(n) -> "?,?,...,?" (empty for n==0) with a unit test, and replace all 16 hand-rolled placeholder idioms across contact/email/task/event/ time_session/annotation/subtask repos (both the iter().map().collect().join() and vec!["?"; n].join() forms, including the two two-step Vec<String> sites). vacation_days serialization is left as-is (not a placeholder list). 869 tests green, clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-06 15:03 UTC
Commit: 205bc480b4843a36cec91daa57c8e1f5de72808c
Parent: 1528b9e
8 files changed, +45 insertions, -25 deletions
M crates/db-sqlite/src/repository/annotation_repo.rs +2 -3
@@ -8,7 +8,7 @@ use std::collections::HashMap;
8 8
9 9 use goingson_core::{AnnotationId, Annotation, CoreError, Result, TaskId, UserId};
10 10
11 - use crate::utils::{format_datetime_now, parse_datetime, parse_uuid};
11 + use crate::utils::{bind_placeholders, format_datetime_now, parse_datetime, parse_uuid};
12 12
13 13 /// Row struct for annotations from SQLite.
14 14 #[derive(Debug, Clone, sqlx::FromRow)]
@@ -42,7 +42,6 @@ pub(crate) async fn get_annotations_for_tasks(
42 42 }
43 43
44 44 // SQLite doesn't have ANY(), use IN with placeholder generation
45 - let placeholders: Vec<String> = task_ids.iter().map(|_| "?".to_string()).collect();
46 45 let query = format!(
47 46 r#"
48 47 SELECT id, task_id, timestamp, note
@@ -50,7 +49,7 @@ pub(crate) async fn get_annotations_for_tasks(
50 49 WHERE task_id IN ({})
51 50 ORDER BY timestamp DESC
52 51 "#,
53 - placeholders.join(",")
52 + bind_placeholders(task_ids.len())
54 53 );
55 54
56 55 let mut q = sqlx::query_as::<_, AnnotationRow>(&query);
M crates/db-sqlite/src/repository/contact_repo.rs +8 -8
@@ -13,7 +13,7 @@ use goingson_core::{
13 13 SocialHandle, SocialHandleId, UpdateContact, UserId,
14 14 };
15 15
16 - use crate::utils::{escape_like, format_datetime, format_datetime_now, parse_datetime, parse_tags, parse_uuid};
16 + use crate::utils::{bind_placeholders, escape_like, format_datetime, format_datetime_now, parse_datetime, parse_tags, parse_uuid};
17 17
18 18 // ============ Row Structs ============
19 19
@@ -174,7 +174,7 @@ impl SqliteContactRepository {
174 174 return Ok(HashMap::new());
175 175 }
176 176
177 - let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
177 + let placeholders = bind_placeholders(ids.len());
178 178 let sql = format!(
179 179 "SELECT id, contact_id, address, label, is_primary FROM contact_emails WHERE contact_id IN ({}) ORDER BY is_primary DESC, rowid ASC",
180 180 placeholders
@@ -204,7 +204,7 @@ impl SqliteContactRepository {
204 204 return Ok(HashMap::new());
205 205 }
206 206
207 - let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
207 + let placeholders = bind_placeholders(ids.len());
208 208 let sql = format!(
209 209 "SELECT id, contact_id, number, label, is_primary FROM contact_phones WHERE contact_id IN ({}) ORDER BY is_primary DESC, rowid ASC",
210 210 placeholders
@@ -234,7 +234,7 @@ impl SqliteContactRepository {
234 234 return Ok(HashMap::new());
235 235 }
236 236
237 - let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
237 + let placeholders = bind_placeholders(ids.len());
238 238 let sql = format!(
239 239 "SELECT id, contact_id, platform, handle, url FROM contact_social_handles WHERE contact_id IN ({}) ORDER BY rowid ASC",
240 240 placeholders
@@ -264,7 +264,7 @@ impl SqliteContactRepository {
264 264 return Ok(HashMap::new());
265 265 }
266 266
267 - let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
267 + let placeholders = bind_placeholders(ids.len());
268 268 let sql = format!(
269 269 "SELECT id, contact_id, label, value, url FROM contact_custom_fields WHERE contact_id IN ({}) ORDER BY rowid ASC",
270 270 placeholders
@@ -496,7 +496,7 @@ impl ContactRepository for SqliteContactRepository {
496 496 return Ok(0);
497 497 }
498 498 let user_id_str = user_id.to_string();
499 - let placeholders = vec!["?"; ids.len()].join(",");
499 + let placeholders = bind_placeholders(ids.len());
500 500 let sql = format!("DELETE FROM contacts WHERE user_id = ? AND id IN ({placeholders})");
501 501 let mut query = sqlx::query(&sql).bind(&user_id_str);
502 502 for id in ids {
@@ -513,7 +513,7 @@ impl ContactRepository for SqliteContactRepository {
513 513 }
514 514 let user_id_str = user_id.to_string();
515 515 let like_pattern = format!("%\"{}\"%" , escape_like(tag));
516 - let placeholders = vec!["?"; ids.len()].join(",");
516 + let placeholders = bind_placeholders(ids.len());
517 517 // Append tag to JSON array where not already present.
518 518 let sql = format!(
519 519 r#"UPDATE contacts
@@ -664,7 +664,7 @@ impl ContactRepository for SqliteContactRepository {
664 664 return Ok(HashSet::new());
665 665 }
666 666
667 - let placeholders = addresses.iter().map(|_| "?").collect::<Vec<_>>().join(",");
667 + let placeholders = bind_placeholders(addresses.len());
668 668 let query = format!(
669 669 "SELECT DISTINCT LOWER(ce.address) FROM contact_emails ce JOIN contacts c ON ce.contact_id = c.id WHERE c.user_id = ? AND LOWER(ce.address) IN ({})",
670 670 placeholders
M crates/db-sqlite/src/repository/email_repo.rs +5 -5
@@ -17,7 +17,7 @@ use goingson_core::{
17 17 };
18 18 use std::collections::HashMap;
19 19
20 - use crate::utils::{format_datetime, format_datetime_now, format_datetime_opt, parse_datetime, parse_uuid, parse_uuid_opt};
20 + use crate::utils::{bind_placeholders, format_datetime, format_datetime_now, format_datetime_opt, parse_datetime, parse_uuid, parse_uuid_opt};
21 21
22 22 /// Column list for SELECT queries - avoids duplication across methods.
23 23 const EMAIL_SELECT_COLUMNS: &str = r#"e.id, e.project_id, p.name as project_name, e.from_address, e.to_address,
@@ -239,7 +239,7 @@ impl EmailRepository for SqliteEmailRepository {
239 239
240 240 // Query 3: Fetch full emails for the page's most-recent-email IDs
241 241 let email_ids: Vec<String> = summaries.iter().map(|s| s.latest_email_id.clone()).collect();
242 - let placeholders = email_ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
242 + let placeholders = bind_placeholders(email_ids.len());
243 243 let emails_sql = format!(
244 244 "SELECT {} FROM emails e LEFT JOIN projects p ON e.project_id = p.id AND p.user_id = ? WHERE e.id IN ({}) AND e.user_id = ?",
245 245 EMAIL_SELECT_COLUMNS, placeholders
@@ -293,7 +293,7 @@ impl EmailRepository for SqliteEmailRepository {
293 293 if addresses.is_empty() {
294 294 return Ok(Vec::new());
295 295 }
296 - let placeholders = addresses.iter().map(|_| "?").collect::<Vec<_>>().join(",");
296 + let placeholders = bind_placeholders(addresses.len());
297 297 let query = format!(
298 298 "SELECT {} FROM emails e LEFT JOIN projects p ON e.project_id = p.id AND p.user_id = ? WHERE e.user_id = ? AND (LOWER(e.from_address) IN ({placeholders}) OR LOWER(e.to_address) IN ({placeholders})) ORDER BY e.received_at DESC LIMIT 200",
299 299 EMAIL_SELECT_COLUMNS
@@ -508,7 +508,7 @@ impl EmailRepository for SqliteEmailRepository {
508 508 return Ok(HashSet::new());
509 509 }
510 510
511 - let placeholders = message_ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
511 + let placeholders = bind_placeholders(message_ids.len());
512 512 let query = format!(
513 513 "SELECT message_id FROM emails WHERE user_id = ? AND message_id IN ({})",
514 514 placeholders
@@ -531,7 +531,7 @@ impl EmailRepository for SqliteEmailRepository {
531 531 return Ok(HashSet::new());
532 532 }
533 533
534 - let placeholders = addresses.iter().map(|_| "?").collect::<Vec<_>>().join(",");
534 + let placeholders = bind_placeholders(addresses.len());
535 535 let query = format!(
536 536 "SELECT DISTINCT LOWER(from_address) FROM emails WHERE user_id = ? AND LOWER(from_address) IN ({})",
537 537 placeholders
M crates/db-sqlite/src/repository/event_repo.rs +2 -2
@@ -14,7 +14,7 @@ use goingson_core::{
14 14 ProjectId, Recurrence, RecurrenceRule, Result, TaskId, UpdateEvent, UserId,
15 15 };
16 16
17 - use crate::utils::{format_datetime, format_datetime_opt, parse_datetime, parse_uuid, parse_uuid_opt};
17 + use crate::utils::{bind_placeholders, format_datetime, format_datetime_opt, parse_datetime, parse_uuid, parse_uuid_opt};
18 18
19 19 /// Column list for SELECT queries - avoids duplication across methods.
20 20 const EVENT_SELECT_COLUMNS: &str = r#"e.id, e.user_id, e.project_id, p.name as project_name,
@@ -319,7 +319,7 @@ impl EventRepository for SqliteEventRepository {
319 319 return Ok(0);
320 320 }
321 321 let user_id_str = user_id.to_string();
322 - let placeholders = vec!["?"; ids.len()].join(",");
322 + let placeholders = bind_placeholders(ids.len());
323 323 let sql = format!("DELETE FROM events WHERE user_id = ? AND id IN ({placeholders})");
324 324 let mut query = sqlx::query(&sql).bind(&user_id_str);
325 325 for id in ids {
M crates/db-sqlite/src/repository/subtask_repo.rs +2 -3
@@ -8,7 +8,7 @@ use std::collections::HashMap;
8 8
9 9 use goingson_core::{CoreError, Result, SubtaskId, Subtask, TaskId, TaskStatus, UserId};
10 10
11 - use crate::utils::{parse_uuid, parse_uuid_opt};
11 + use crate::utils::{bind_placeholders, parse_uuid, parse_uuid_opt};
12 12
13 13 /// Row struct for subtasks from SQLite.
14 14 #[derive(Debug, Clone, sqlx::FromRow)]
@@ -45,7 +45,6 @@ pub(crate) async fn get_subtasks_for_tasks(
45 45 return Ok(HashMap::new());
46 46 }
47 47
48 - let placeholders: Vec<String> = task_ids.iter().map(|_| "?".to_string()).collect();
49 48 let query = format!(
50 49 r#"
51 50 SELECT id, task_id, text, linked_task_id, is_completed, position
@@ -53,7 +52,7 @@ pub(crate) async fn get_subtasks_for_tasks(
53 52 WHERE task_id IN ({})
54 53 ORDER BY position ASC, created_at ASC
55 54 "#,
56 - placeholders.join(",")
55 + bind_placeholders(task_ids.len())
57 56 );
58 57
59 58 let mut q = sqlx::query_as::<_, SubtaskRow>(&query);
M crates/db-sqlite/src/repository/task_repo.rs +2 -2
@@ -19,7 +19,7 @@ use goingson_core::{
19 19 TimeTrackingSummary, UpdateTask, UserId,
20 20 };
21 21
22 - use crate::utils::{format_datetime, format_datetime_now, format_datetime_opt, parse_datetime, parse_tags, parse_uuid, parse_uuid_opt};
22 + use crate::utils::{bind_placeholders, format_datetime, format_datetime_now, format_datetime_opt, parse_datetime, parse_tags, parse_uuid, parse_uuid_opt};
23 23
24 24 use super::annotation_repo;
25 25 use super::subtask_repo;
@@ -604,7 +604,7 @@ impl TaskRepository for SqliteTaskRepository {
604 604 if ids.is_empty() {
605 605 return Ok(0);
606 606 }
607 - let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
607 + let placeholders = bind_placeholders(ids.len());
608 608 let sql = format!(
609 609 "UPDATE tasks SET project_id = ? WHERE user_id = ? AND id IN ({placeholders})"
610 610 );
M crates/db-sqlite/src/repository/time_session_repo.rs +2 -2
@@ -11,7 +11,7 @@ use goingson_core::{
11 11 CoreError, PositiveMinutes, Result, TaskId, TimeSession, TimeSessionId, TimeTrackingSummary, UserId,
12 12 };
13 13
14 - use crate::utils::{format_datetime, format_datetime_now, parse_datetime, parse_uuid};
14 + use crate::utils::{bind_placeholders, format_datetime, format_datetime_now, parse_datetime, parse_uuid};
15 15
16 16 /// Row struct for time session queries.
17 17 #[derive(Debug, sqlx::FromRow)]
@@ -49,7 +49,7 @@ pub(crate) async fn get_active_sessions_for_tasks(
49 49 return Ok(HashMap::new());
50 50 }
51 51
52 - let placeholders = task_ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
52 + let placeholders = bind_placeholders(task_ids.len());
53 53 let sql = format!(
54 54 "SELECT id, task_id, user_id, started_at, ended_at, duration_minutes, created_at
55 55 FROM time_sessions WHERE task_id IN ({}) AND ended_at IS NULL",
M crates/db-sqlite/src/utils.rs +22
@@ -72,6 +72,19 @@ pub fn escape_like(value: &str) -> String {
72 72 value.replace('\\', "\\\\").replace('%', "\\%").replace('_', "\\_")
73 73 }
74 74
75 + /// Build a comma-separated list of `n` SQLite bind placeholders (`?,?,...,?`)
76 + /// for an `IN (...)` clause.
77 + ///
78 + /// Callers guard the empty case before building the query (an empty `IN ()`
79 + /// is invalid SQL); this returns `""` for `n == 0` so the guard stays their
80 + /// responsibility, matching every existing call site.
81 + #[inline]
82 + pub fn bind_placeholders(n: usize) -> String {
83 + let mut s = "?,".repeat(n);
84 + s.pop(); // drop the trailing comma ("" when n == 0)
85 + s
86 + }
87 +
75 88 /// Validate email address format (RFC 5321/5322 compliant).
76 89 ///
77 90 /// Validates the basic structure of an email address:
@@ -146,6 +159,15 @@ mod tests {
146 159 }
147 160
148 161 #[test]
162 + fn test_bind_placeholders() {
163 + assert_eq!(bind_placeholders(0), "");
164 + assert_eq!(bind_placeholders(1), "?");
165 + assert_eq!(bind_placeholders(3), "?,?,?");
166 + // count of placeholders matches n for a realistic IN-clause size
167 + assert_eq!(bind_placeholders(10).split(',').count(), 10);
168 + }
169 +
170 + #[test]
149 171 fn test_parse_datetime_sqlite_format() {
150 172 let result = parse_datetime("2024-01-15 10:30:00");
151 173 assert!(result.is_ok());