Switch read paths to the true-instant date contract (ticket #119)
- New src/date_view.rs: pure read-side functions (row_true_instant, row_display_wall, row_display_day, filename_true_instant) applying the per-source convention table for unconverted rows and the new-contract triple for converted rows (ADR-0001). - /memories: grouping on the row's true instant rendered in the owner's display zone (prefs -> app default), not the client device offset. SQL is a coarse interval pre-filter over the full years-back window; exact month-day/week/month matching across past years happens in Rust (memories::day_in_span, sqlite_w_week). - Reels: ReelSelector::Memories carries an IANA display_tz instead of a fixed minute offset; pre-gen params read REEL_PREGEN_DISPLAY_TZ; the tz_offset_minutes ledger column stores the zone offset at render time. - Insights: prompt-facing dates render as the display wall; the proximity timestamp and chat context block use the true instant; InsightGenerator reads the owner's display zone from prefs per call. - GpsPhotoSummary carries the new-contract triple alongside the frozen legacy date_taken. Closes #119
This commit is contained in:
1 parent
cd375f18db
commit
af87765bbb
13 files changed
+1590
-301
No files matched your search
+179
-124
@@ -9,21 +9,6 @@ use crate::database::models::{
|
||||
};
|
||||
use crate::otel::trace_db_call;
|
||||
|
||||
/// Decoded shape for `get_memories_in_window`'s raw `sql_query`. Diesel's
|
||||
/// query DSL doesn't expose strftime, so the memories filter is hand-
|
||||
/// written SQL — but the returned columns are simple enough that a small
|
||||
/// `QueryableByName` struct suffices, kept private to this module.
|
||||
#[derive(diesel::QueryableByName)]
|
||||
#[allow(dead_code)] // fields read via Diesel's QueryableByName derive
|
||||
struct MemoriesWindowRow {
|
||||
#[diesel(sql_type = diesel::sql_types::Text)]
|
||||
rel_path: String,
|
||||
#[diesel(sql_type = diesel::sql_types::BigInt)]
|
||||
date_taken: i64,
|
||||
#[diesel(sql_type = diesel::sql_types::BigInt)]
|
||||
last_modified: i64,
|
||||
}
|
||||
|
||||
/// Wire shape for a single member of a duplicate group, returned by
|
||||
/// `list_duplicates_*` and `lookup_duplicate_row`. Carries everything
|
||||
/// the Apollo modal needs to render a member tile and its meta line —
|
||||
@@ -441,7 +426,18 @@ pub trait ExifDao: Sync + Send {
|
||||
context: &opentelemetry::Context,
|
||||
base_path: &str,
|
||||
recursive: bool,
|
||||
) -> Result<Vec<(String, f64, f64, Option<i64>)>, DbError>;
|
||||
) -> Result<
|
||||
Vec<(
|
||||
String,
|
||||
f64,
|
||||
f64,
|
||||
Option<i64>,
|
||||
Option<i64>,
|
||||
Option<i32>,
|
||||
Option<String>,
|
||||
)>,
|
||||
DbError,
|
||||
>;
|
||||
|
||||
/// Return rows that still lack a `content_hash`, oldest first. Used by
|
||||
/// the `backfill_hashes` binary to batch through the historical
|
||||
@@ -652,34 +648,34 @@ pub trait ExifDao: Sync + Send {
|
||||
rel_path: &str,
|
||||
) -> Result<ImageExif, DbError>;
|
||||
|
||||
/// Single-query backend for `/memories`. Returns
|
||||
/// `(rel_path, date_taken, last_modified)` for rows in `library_id`
|
||||
/// whose `date_taken` falls within `[now - years_back y, now]` and
|
||||
/// whose calendar position matches the request's span:
|
||||
/// - `"day"` — same month + day-of-month (any year)
|
||||
/// - `"week"` — same week-of-year (SQLite `%W`, Monday-anchored —
|
||||
/// close to but not exactly ISO week 8601; the boundary cases
|
||||
/// at year-start/end can shift by ±1 vs the prior request-time
|
||||
/// `iso_week()` filter)
|
||||
/// - `"month"` — same month (any year)
|
||||
/// Coarse pre-filter for the `/memories` span query (ADR-0001).
|
||||
///
|
||||
/// `tz_offset_minutes` is applied to both sides of the strftime
|
||||
/// comparison so the calendar match is in the user's local time.
|
||||
/// Backed by the `(library_id, date_taken)` index.
|
||||
/// The span feature matches the same calendar position (month-day /
|
||||
/// week-of-year / month) across every past year, so this query only
|
||||
/// bounds the rows by epoch windows; the exact span-day match happens
|
||||
/// in the caller (`memories::day_in_span`), because it needs the
|
||||
/// owner's display zone.
|
||||
///
|
||||
/// This is the single-SQL replacement for the EXIF-loop +
|
||||
/// WalkDir-fallback that powered `/memories` previously; it's
|
||||
/// correct only because the canonical-date waterfall at ingest
|
||||
/// (`crate::date_resolver`) populates `date_taken` for every row
|
||||
/// it can resolve.
|
||||
/// Eligibility:
|
||||
/// - `date_taken` must be set and sit inside the unconverted band
|
||||
/// `[unconv_lo, unconv_hi]` (a ±15 h margin around the years-back
|
||||
/// window that covers every per-source legacy convention, and which
|
||||
/// also caps how far into the future a row may sit);
|
||||
/// - converted rows (`date_utc IS NOT NULL`) additionally need
|
||||
/// `date_utc` inside `[conv_lo, conv_hi]` — the true-instant
|
||||
/// window; unconverted rows pass on the band alone.
|
||||
///
|
||||
/// Returns full rows so the caller can read the date triple
|
||||
/// (`date_utc` / `camera_offset_minutes` / `date_taken_tz_source`).
|
||||
fn get_memories_in_window(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
library_id: i32,
|
||||
span_token: &str,
|
||||
years_back: i32,
|
||||
tz_offset_minutes: i32,
|
||||
) -> Result<Vec<(String, i64, i64)>, DbError>;
|
||||
conv_lo: i64,
|
||||
conv_hi: i64,
|
||||
unconv_lo: i64,
|
||||
unconv_hi: i64,
|
||||
) -> Result<Vec<ImageExif>, DbError>;
|
||||
|
||||
/// Return image rows that have a `content_hash` but no `phash_64`,
|
||||
/// oldest first. Used by the `backfill_perceptual_hash` binary.
|
||||
@@ -1293,12 +1289,24 @@ impl ExifDao for SqliteExifDao {
|
||||
.map_err(|e| DbError::log(DbErrorKind::QueryError, e))
|
||||
}
|
||||
|
||||
#[allow(clippy::type_complexity)]
|
||||
fn get_all_with_gps(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
base_path: &str,
|
||||
recursive: bool,
|
||||
) -> Result<Vec<(String, f64, f64, Option<i64>)>, DbError> {
|
||||
) -> Result<
|
||||
Vec<(
|
||||
String,
|
||||
f64,
|
||||
f64,
|
||||
Option<i64>,
|
||||
Option<i64>,
|
||||
Option<i32>,
|
||||
Option<String>,
|
||||
)>,
|
||||
DbError,
|
||||
> {
|
||||
trace_db_call(context, "query", "get_all_with_gps", |span| {
|
||||
use opentelemetry::KeyValue;
|
||||
use opentelemetry::trace::Span;
|
||||
@@ -1334,10 +1342,19 @@ impl ExifDao for SqliteExifDao {
|
||||
.load::<ImageExif>(connection)
|
||||
.map_err(|e| anyhow::anyhow!("GPS query error: {}", e))?;
|
||||
|
||||
// Convert to tuple format (path, lat, lon, date_taken)
|
||||
// Convert to tuple format (path, lat, lon, legacy date_taken,
|
||||
// date_utc, camera_offset_minutes, date_taken_tz_source).
|
||||
// Filter out any rows where GPS is still None (shouldn't happen due to filter)
|
||||
// Cast f32 GPS values to f64 for API compatibility
|
||||
let filtered: Vec<(String, f64, f64, Option<i64>)> = results
|
||||
let filtered: Vec<(
|
||||
String,
|
||||
f64,
|
||||
f64,
|
||||
Option<i64>,
|
||||
Option<i64>,
|
||||
Option<i32>,
|
||||
Option<String>,
|
||||
)> = results
|
||||
.into_iter()
|
||||
.filter_map(|exif| {
|
||||
if let (Some(lat_val), Some(lon_val)) = (exif.gps_latitude, exif.gps_longitude)
|
||||
@@ -1347,6 +1364,9 @@ impl ExifDao for SqliteExifDao {
|
||||
lat_val as f64,
|
||||
lon_val as f64,
|
||||
exif.date_taken,
|
||||
exif.date_utc,
|
||||
exif.camera_offset_minutes,
|
||||
exif.date_taken_tz_source,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
@@ -1937,51 +1957,27 @@ impl ExifDao for SqliteExifDao {
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
library_id: i32,
|
||||
span_token: &str,
|
||||
years_back: i32,
|
||||
tz_offset_minutes: i32,
|
||||
) -> Result<Vec<(String, i64, i64)>, DbError> {
|
||||
conv_lo: i64,
|
||||
conv_hi: i64,
|
||||
unconv_lo: i64,
|
||||
unconv_hi: i64,
|
||||
) -> Result<Vec<ImageExif>, DbError> {
|
||||
trace_db_call(context, "query", "get_memories_in_window", |_span| {
|
||||
// strftime pattern is span-dependent; the rest of the WHERE
|
||||
// clause is shared. Only `%m-%d`, `%W`, `%m` are accepted —
|
||||
// anything else is a programmer error.
|
||||
let pattern = match span_token {
|
||||
"day" => "%m-%d",
|
||||
"week" => "%W",
|
||||
"month" => "%m",
|
||||
_ => return Err(anyhow::anyhow!("invalid span token: {}", span_token)),
|
||||
};
|
||||
|
||||
// SQLite's date modifiers want a string like `'-480 minutes'`
|
||||
// (signed) or `'-15 years'`. Use the `+` flag so positive
|
||||
// offsets render as `+480 minutes`.
|
||||
let tz_modifier = format!("{:+} minutes", tz_offset_minutes);
|
||||
let years_modifier = format!("-{} years", years_back);
|
||||
|
||||
let sql = format!(
|
||||
"SELECT rel_path, date_taken, last_modified \
|
||||
FROM image_exif \
|
||||
WHERE library_id = ?1 \
|
||||
AND date_taken IS NOT NULL \
|
||||
AND date_taken <= unixepoch('now') \
|
||||
AND date_taken >= unixepoch('now', ?2) \
|
||||
AND strftime('{p}', date_taken, 'unixepoch', ?3) \
|
||||
= strftime('{p}', 'now', ?3)",
|
||||
p = pattern,
|
||||
);
|
||||
use crate::database::schema::image_exif::dsl as d;
|
||||
|
||||
let mut connection = self.connection.lock().expect("Unable to get ExifDao");
|
||||
|
||||
diesel::sql_query(sql)
|
||||
.bind::<diesel::sql_types::Integer, _>(library_id)
|
||||
.bind::<diesel::sql_types::Text, _>(years_modifier)
|
||||
.bind::<diesel::sql_types::Text, _>(tz_modifier)
|
||||
.load::<MemoriesWindowRow>(connection.deref_mut())
|
||||
.map(|rows| {
|
||||
rows.into_iter()
|
||||
.map(|r| (r.rel_path, r.date_taken, r.last_modified))
|
||||
.collect()
|
||||
})
|
||||
d::image_exif
|
||||
.filter(d::library_id.eq(library_id))
|
||||
.filter(d::date_taken.is_not_null())
|
||||
.filter(d::date_taken.ge(unconv_lo))
|
||||
.filter(d::date_taken.le(unconv_hi))
|
||||
.filter(
|
||||
d::date_utc
|
||||
.is_null()
|
||||
.or(d::date_utc.ge(conv_lo).and(d::date_utc.le(conv_hi))),
|
||||
)
|
||||
.load::<ImageExif>(connection.deref_mut())
|
||||
.map_err(|e| anyhow::anyhow!("Query error: {}", e))
|
||||
})
|
||||
.map_err(|e| DbError::log(DbErrorKind::QueryError, e))
|
||||
@@ -2966,25 +2962,21 @@ mod exif_dao_tests {
|
||||
assert_eq!(row.size_bytes, Some(1024));
|
||||
}
|
||||
|
||||
/// Wide intervals that admit everything in the years-back window —
|
||||
/// span-day matching happens in the caller, so tests that don't care
|
||||
/// about the span pass these.
|
||||
/// Mirrors the production caller (`memories::memories_span_windows`):
|
||||
/// the converted window spans the full years-back horizon and the
|
||||
/// unconverted band pads it by ±15 h — so far-future legacy values
|
||||
/// are dropped at the DAO, not by the band.
|
||||
fn wide_windows(now_ts: i64, years_back: i64) -> (i64, i64, i64, i64) {
|
||||
let lo = now_ts - years_back * 366 * 86_400;
|
||||
(lo, now_ts, lo - 15 * 3600, now_ts + 15 * 3600)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_memories_in_window_day_matches_only_same_md_in_year_window() {
|
||||
fn get_memories_in_window_filters_future_no_date_and_years_back() {
|
||||
let mut dao = setup_two_libraries();
|
||||
|
||||
// Anchor on a known date so the test is timezone-stable: insert
|
||||
// rows whose date_taken IS the same wall-clock time as `now()`
|
||||
// would have been some N years ago, and verify the day-span
|
||||
// filter returns them. We can't bind 'now' from Rust, so instead
|
||||
// we insert rows for the *current* day (offset by 365 days * N
|
||||
// years) and rely on SQLite computing the same `%m-%d` for both
|
||||
// sides of the equality. Using the unix-now-minus-365*N seconds
|
||||
// approximation is good enough — leap years drift by ~one day
|
||||
// every four years, but the test only checks day-of-year match
|
||||
// for rows inserted "today minus N years (no leap correction)".
|
||||
// To dodge the leap-year drift entirely, we use rows whose
|
||||
// calendar date is read back from SQLite and we just check
|
||||
// membership.
|
||||
|
||||
// 1y, 5y, 10y, 21y back from 'now':
|
||||
let now_ts = chrono::Utc::now().timestamp();
|
||||
let year_secs: i64 = 365 * 86_400;
|
||||
insert_row_with_source(
|
||||
@@ -2994,13 +2986,6 @@ mod exif_dao_tests {
|
||||
Some(now_ts - year_secs),
|
||||
Some("exif"),
|
||||
);
|
||||
insert_row_with_source(
|
||||
&mut dao,
|
||||
1,
|
||||
"y5.jpg",
|
||||
Some(now_ts - 5 * year_secs),
|
||||
Some("exif"),
|
||||
);
|
||||
insert_row_with_source(
|
||||
&mut dao,
|
||||
1,
|
||||
@@ -3027,20 +3012,15 @@ mod exif_dao_tests {
|
||||
// No date — never returned regardless of source.
|
||||
insert_row_with_source(&mut dao, 1, "nodate.jpg", None, None);
|
||||
|
||||
// Month span returns rows from the same calendar month over the
|
||||
// window — y1, y5, y10 should all qualify (same month any year),
|
||||
// y21 trims (out of years_back), future trims (> now), nodate
|
||||
// never qualifies. Day-of-month leap drift means even with 365-
|
||||
// day approximation a row may shift by one in either direction;
|
||||
// month is the safer assertion under that approximation.
|
||||
let (clo, chi, ulo, uhi) = wide_windows(now_ts, 20);
|
||||
let rows = dao
|
||||
.get_memories_in_window(&ctx(), 1, "month", 20, 0)
|
||||
.get_memories_in_window(&ctx(), 1, clo, chi, ulo, uhi)
|
||||
.unwrap();
|
||||
let paths: std::collections::HashSet<String> =
|
||||
rows.into_iter().map(|(p, _, _)| p).collect();
|
||||
rows.into_iter().map(|r| r.file_path).collect();
|
||||
assert!(
|
||||
paths.contains("y1.jpg") && paths.contains("y5.jpg") && paths.contains("y10.jpg"),
|
||||
"month span should include all in-window rows: {:?}",
|
||||
paths.contains("y1.jpg") && paths.contains("y10.jpg"),
|
||||
"in-window rows should be returned: {:?}",
|
||||
paths
|
||||
);
|
||||
assert!(
|
||||
@@ -3067,23 +3047,98 @@ mod exif_dao_tests {
|
||||
Some(now_ts - year),
|
||||
Some("exif"),
|
||||
);
|
||||
|
||||
let (clo, chi, ulo, uhi) = wide_windows(now_ts, 20);
|
||||
let lib1 = dao
|
||||
.get_memories_in_window(&ctx(), 1, "month", 20, 0)
|
||||
.get_memories_in_window(&ctx(), 1, clo, chi, ulo, uhi)
|
||||
.unwrap();
|
||||
let lib2 = dao
|
||||
.get_memories_in_window(&ctx(), 2, "month", 20, 0)
|
||||
.get_memories_in_window(&ctx(), 2, clo, chi, ulo, uhi)
|
||||
.unwrap();
|
||||
assert_eq!(lib1.len(), 1);
|
||||
assert_eq!(lib1[0].0, "main/x.jpg");
|
||||
assert_eq!(lib1[0].file_path, "main/x.jpg");
|
||||
assert_eq!(lib2.len(), 1);
|
||||
assert_eq!(lib2[0].0, "archive/x.jpg");
|
||||
assert_eq!(lib2[0].file_path, "archive/x.jpg");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_memories_in_window_rejects_unknown_span_token() {
|
||||
fn get_memories_in_window_branches_converted_on_date_utc() {
|
||||
let mut dao = setup_two_libraries();
|
||||
let err = dao.get_memories_in_window(&ctx(), 1, "decade", 20, 0);
|
||||
assert!(err.is_err());
|
||||
let now_ts = chrono::Utc::now().timestamp();
|
||||
let year = 365 * 86_400i64;
|
||||
|
||||
// A converted row whose `date_utc` sits INSIDE the conv interval
|
||||
// but whose legacy `date_taken` sits OUTSIDE the unconv interval:
|
||||
// it must still be returned (proof the converted branch won).
|
||||
insert_row_with_source(&mut dao, 1, "conv.jpg", Some(now_ts - year), Some("exif"));
|
||||
dao.backfill_date_taken(
|
||||
&ctx(),
|
||||
1,
|
||||
"conv.jpg",
|
||||
now_ts - year,
|
||||
"exif",
|
||||
Some(now_ts - year - 9000), // shifted true instant
|
||||
None,
|
||||
Some("assumed_server"),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// An unconverted row: judged on legacy `date_taken` only.
|
||||
insert_row_with_source(
|
||||
&mut dao,
|
||||
1,
|
||||
"unconv.jpg",
|
||||
Some(now_ts - year),
|
||||
Some("fs_time"),
|
||||
);
|
||||
|
||||
// A converted row OUTSIDE the conv interval: excluded even though
|
||||
// its legacy value is inside the unconv band.
|
||||
insert_row_with_source(
|
||||
&mut dao,
|
||||
1,
|
||||
"conv-out.jpg",
|
||||
Some(now_ts - year),
|
||||
Some("exif"),
|
||||
);
|
||||
dao.backfill_date_taken(
|
||||
&ctx(),
|
||||
1,
|
||||
"conv-out.jpg",
|
||||
now_ts - year,
|
||||
"exif",
|
||||
Some(now_ts - 30 * year),
|
||||
None,
|
||||
Some("assumed_server"),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// conv interval: a tight band around (now - year - 9000) that
|
||||
// excludes (now - 30*year) and excludes the unconv row's legacy
|
||||
// (now - year) is irrelevant — it's unconverted. The unconv band
|
||||
// is wide for unconv.jpg.
|
||||
let clo = now_ts - year - 86_400;
|
||||
let chi = now_ts - year;
|
||||
let ulo = now_ts - year - 86_400;
|
||||
let uhi = now_ts;
|
||||
let rows = dao
|
||||
.get_memories_in_window(&ctx(), 1, clo, chi, ulo, uhi)
|
||||
.unwrap();
|
||||
let paths: std::collections::HashSet<String> =
|
||||
rows.into_iter().map(|r| r.file_path).collect();
|
||||
assert!(
|
||||
paths.contains("conv.jpg"),
|
||||
"converted row in conv band: {:?}",
|
||||
paths
|
||||
);
|
||||
assert!(
|
||||
paths.contains("unconv.jpg"),
|
||||
"unconverted row in legacy band: {:?}",
|
||||
paths
|
||||
);
|
||||
assert!(
|
||||
!paths.contains("conv-out.jpg"),
|
||||
"converted row outside conv band must be excluded: {:?}",
|
||||
paths
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user