fix: include files without EXIF when sorting by date
Date sorting previously used a DB-level query that acted as an inner join, silently dropping files with no image_exif row. Replace it with the existing in-memory sort which already falls back to filename-extracted and filesystem dates, so all files appear in sorted results. Also removes the now-unused get_files_sorted_by_date trait method and its SqliteExifDao implementation and test mock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Cameron
@@ -304,17 +304,6 @@ pub trait ExifDao: Sync + Send {
|
||||
context: &opentelemetry::Context,
|
||||
) -> Result<Vec<String>, DbError>;
|
||||
|
||||
/// Get files sorted by date with optional pagination
|
||||
/// Returns (sorted_file_paths, total_count)
|
||||
fn get_files_sorted_by_date(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
file_paths: &[String],
|
||||
ascending: bool,
|
||||
limit: Option<i64>,
|
||||
offset: i64,
|
||||
) -> Result<(Vec<String>, i64), DbError>;
|
||||
|
||||
/// Get all photos with GPS coordinates
|
||||
/// Returns Vec<(file_path, latitude, longitude, date_taken)>
|
||||
fn get_all_with_gps(
|
||||
@@ -609,66 +598,6 @@ impl ExifDao for SqliteExifDao {
|
||||
.map_err(|_| DbError::new(DbErrorKind::QueryError))
|
||||
}
|
||||
|
||||
fn get_files_sorted_by_date(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
file_paths: &[String],
|
||||
ascending: bool,
|
||||
limit: Option<i64>,
|
||||
offset: i64,
|
||||
) -> Result<(Vec<String>, i64), DbError> {
|
||||
trace_db_call(context, "query", "get_files_sorted_by_date", |span| {
|
||||
use diesel::dsl::count_star;
|
||||
use opentelemetry::KeyValue;
|
||||
use opentelemetry::trace::Span;
|
||||
use schema::image_exif::dsl::*;
|
||||
|
||||
span.set_attributes(vec![
|
||||
KeyValue::new("file_count", file_paths.len() as i64),
|
||||
KeyValue::new("ascending", ascending.to_string()),
|
||||
KeyValue::new("limit", limit.map(|l| l.to_string()).unwrap_or_default()),
|
||||
KeyValue::new("offset", offset.to_string()),
|
||||
]);
|
||||
|
||||
if file_paths.is_empty() {
|
||||
return Ok((Vec::new(), 0));
|
||||
}
|
||||
|
||||
let connection = &mut *self.connection.lock().unwrap();
|
||||
|
||||
// Get total count of files that have EXIF data
|
||||
let total_count: i64 = image_exif
|
||||
.filter(file_path.eq_any(file_paths))
|
||||
.select(count_star())
|
||||
.first(connection)
|
||||
.map_err(|_| anyhow::anyhow!("Count query error"))?;
|
||||
|
||||
// Build sorted query
|
||||
let mut query = image_exif.filter(file_path.eq_any(file_paths)).into_boxed();
|
||||
|
||||
// Apply sorting
|
||||
// Note: SQLite NULL handling varies - NULLs appear first for ASC, last for DESC by default
|
||||
if ascending {
|
||||
query = query.order(date_taken.asc());
|
||||
} else {
|
||||
query = query.order(date_taken.desc());
|
||||
}
|
||||
|
||||
// Apply pagination if requested
|
||||
if let Some(limit_val) = limit {
|
||||
query = query.limit(limit_val).offset(offset);
|
||||
}
|
||||
|
||||
// Execute and extract file paths
|
||||
let results: Vec<String> = query
|
||||
.select(file_path)
|
||||
.load::<String>(connection)
|
||||
.map_err(|_| anyhow::anyhow!("Query error"))?;
|
||||
|
||||
Ok((results, total_count))
|
||||
})
|
||||
.map_err(|_| DbError::new(DbErrorKind::QueryError))
|
||||
}
|
||||
|
||||
fn get_all_with_gps(
|
||||
&mut self,
|
||||
|
||||
59
src/files.rs
59
src/files.rs
@@ -61,45 +61,19 @@ fn apply_sorting_with_exif(
|
||||
|
||||
match sort_type {
|
||||
SortType::DateTakenAsc | SortType::DateTakenDesc => {
|
||||
info!("Date sorting requested, using database-level sorting");
|
||||
|
||||
// Collect file paths for batch EXIF query
|
||||
let file_paths: Vec<String> = files.iter().map(|f| f.file_name.clone()).collect();
|
||||
|
||||
// Try database-level sorting first (most efficient)
|
||||
let ascending = sort_type == SortType::DateTakenAsc;
|
||||
match exif_dao.get_files_sorted_by_date(
|
||||
info!("Date sorting requested, using in-memory sort with EXIF/filename fallback");
|
||||
// Use in-memory sort so files without EXIF dates are included via
|
||||
// filename extraction and filesystem metadata fallbacks.
|
||||
let (sorted, _) = in_memory_date_sort(
|
||||
files,
|
||||
sort_type,
|
||||
exif_dao,
|
||||
span_context,
|
||||
&file_paths,
|
||||
ascending,
|
||||
base_path,
|
||||
limit,
|
||||
offset,
|
||||
) {
|
||||
Ok((sorted_files, db_total)) => {
|
||||
info!(
|
||||
"Database-level date sorting succeeded, returned {} files",
|
||||
sorted_files.len()
|
||||
);
|
||||
(sorted_files, db_total)
|
||||
}
|
||||
Err(e) => {
|
||||
warn!(
|
||||
"Database-level sorting failed: {:?}, falling back to in-memory sort",
|
||||
e
|
||||
);
|
||||
// Fallback to in-memory sorting with date extraction
|
||||
let (sorted, _) = in_memory_date_sort(
|
||||
files,
|
||||
sort_type,
|
||||
exif_dao,
|
||||
span_context,
|
||||
base_path,
|
||||
limit,
|
||||
offset,
|
||||
);
|
||||
(sorted, total_count)
|
||||
}
|
||||
}
|
||||
);
|
||||
(sorted, total_count)
|
||||
}
|
||||
_ => {
|
||||
// Use regular sort for non-date sorting
|
||||
@@ -1352,19 +1326,6 @@ mod tests {
|
||||
Ok(Vec::new())
|
||||
}
|
||||
|
||||
fn get_files_sorted_by_date(
|
||||
&mut self,
|
||||
_context: &opentelemetry::Context,
|
||||
file_paths: &[String],
|
||||
_ascending: bool,
|
||||
_limit: Option<i64>,
|
||||
_offset: i64,
|
||||
) -> Result<(Vec<String>, i64), DbError> {
|
||||
// For tests, just return all files unsorted
|
||||
let count = file_paths.len() as i64;
|
||||
Ok((file_paths.to_vec(), count))
|
||||
}
|
||||
|
||||
fn get_all_with_gps(
|
||||
&mut self,
|
||||
_context: &opentelemetry::Context,
|
||||
|
||||
Reference in New Issue
Block a user