mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(core): keep pre-v30 photos on the page until a refresh groups them
A database written before the grouping column opens with `group_key` NULL on every row, and NULL matches nothing: the join dropped the whole timeline until the first refresh ran. `COALESCE(group_key, uid)` is what makes such a row its own group, which is how the gallery behaved before the column existed. Covered by a migration test that rewinds a fixture to v29.
This commit is contained in:
1 parent
c1df3b6cc2
commit
09c0684a5e
2 files changed
+87
-8
No files matched your search
@@ -203,10 +203,14 @@ fn group_photos(entries: &[Grouped]) -> Vec<String> {
|
||||
/// The columns every photo read selects, with the group aggregates joined in.
|
||||
/// `g.n` is how many photos the group holds and `g.raw` whether any of them is a
|
||||
/// camera raw file — one query rather than a second lookup per tile.
|
||||
///
|
||||
/// `COALESCE(group_key, uid)` is what makes a row written before schema v30 its
|
||||
/// own group: the column is `NULL` until the next refresh computes it, and a
|
||||
/// `NULL` would otherwise match nothing and drop the photo off the page.
|
||||
const PHOTO_SELECT: &str = "SELECT p.uid, p.capture_time, p.name, p.ratio, p.thumb_state, \
|
||||
p.kind, p.favorite, g.n, g.raw \
|
||||
FROM photos p JOIN (SELECT group_key, COUNT(*) AS n, MAX(kind = 2) AS raw \
|
||||
FROM photos GROUP BY group_key) g ON g.group_key = p.group_key";
|
||||
FROM photos p JOIN (SELECT COALESCE(group_key, uid) AS k, COUNT(*) AS n, \
|
||||
MAX(kind = 2) AS raw FROM photos GROUP BY k) g ON g.k = COALESCE(p.group_key, p.uid)";
|
||||
|
||||
/// Read one row of [`PHOTO_SELECT`].
|
||||
fn stored_photo(r: &rusqlite::Row<'_>) -> rusqlite::Result<StoredPhoto> {
|
||||
@@ -374,7 +378,7 @@ impl Db {
|
||||
let mut binds: Vec<i64> = Vec::new();
|
||||
let mut conds: Vec<String> = Vec::new();
|
||||
if kind != Some(crate::control::PhotoKind::Raw) {
|
||||
conds.push("p.uid = p.group_key".to_string());
|
||||
conds.push("p.uid = COALESCE(p.group_key, p.uid)".to_string());
|
||||
}
|
||||
if let Some(k) = kind {
|
||||
binds.push(k.as_i64());
|
||||
@@ -414,8 +418,8 @@ impl Db {
|
||||
pub fn photos_group(&self, uid: &str) -> Result<Vec<StoredPhoto>> {
|
||||
let conn = self.read();
|
||||
let sql = format!(
|
||||
"{PHOTO_SELECT} WHERE p.group_key = \
|
||||
(SELECT group_key FROM photos WHERE uid = ?1) ORDER BY p.seq"
|
||||
"{PHOTO_SELECT} WHERE COALESCE(p.group_key, p.uid) = \
|
||||
(SELECT COALESCE(group_key, uid) FROM photos WHERE uid = ?1) ORDER BY p.seq"
|
||||
);
|
||||
let mut stmt = conn.prepare(&sql)?;
|
||||
let rows = stmt
|
||||
@@ -451,8 +455,11 @@ impl Db {
|
||||
// the page shows: groups everywhere except on the Raw tab.
|
||||
let (filter, binds): (&str, Vec<i64>) = match kind {
|
||||
Some(crate::control::PhotoKind::Raw) => (" WHERE kind = 2", Vec::new()),
|
||||
Some(k) => (" WHERE uid = group_key AND kind = ?1", vec![k.as_i64()]),
|
||||
None => (" WHERE uid = group_key", Vec::new()),
|
||||
Some(k) => (
|
||||
" WHERE uid = COALESCE(group_key, uid) AND kind = ?1",
|
||||
vec![k.as_i64()],
|
||||
),
|
||||
None => (" WHERE uid = COALESCE(group_key, uid)", Vec::new()),
|
||||
};
|
||||
let sql = format!(
|
||||
"SELECT CAST(strftime('%Y', capture_time, 'unixepoch', 'localtime') AS INTEGER) AS y, \
|
||||
@@ -481,7 +488,8 @@ impl Db {
|
||||
// files it was stored as. Raw counts files, because the Raw tab lists
|
||||
// them individually.
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT kind, COUNT(*) FROM photos WHERE uid = group_key OR kind = 2 GROUP BY kind",
|
||||
"SELECT kind, COUNT(*) FROM photos \
|
||||
WHERE uid = COALESCE(group_key, uid) OR kind = 2 GROUP BY kind",
|
||||
)?;
|
||||
let rows = stmt.query_map([], |r| Ok((r.get::<_, i64>(0)?, r.get::<_, i64>(1)?)))?;
|
||||
let (mut photos, mut videos, mut raw) = (0usize, 0usize, 0usize);
|
||||
|
||||
@@ -4101,3 +4101,74 @@ fn the_servers_photo_relation_groups_a_live_photo() {
|
||||
.unwrap();
|
||||
assert_eq!(db.photos_group("related").unwrap().len(), 2);
|
||||
}
|
||||
|
||||
/// A database written before grouping existed opens, keeps its timeline, and
|
||||
/// shows every photo — a row with no `group_key` is its own group, which is how
|
||||
/// the gallery behaved before the column.
|
||||
#[test]
|
||||
fn migration_v30_leaves_a_v29_timeline_intact() {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"pdfs-db-v29-fixture-{}-{}.db",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_nanos()
|
||||
));
|
||||
{
|
||||
let db = Db::open(&path).unwrap();
|
||||
db.photos_replace(&[
|
||||
TimelineRow {
|
||||
name: Some("IMG_1234.JPG".into()),
|
||||
..TimelineRow::new("jpeg", 300)
|
||||
},
|
||||
TimelineRow {
|
||||
name: Some("IMG_1234.CR2".into()),
|
||||
..TimelineRow::new("raw", 300)
|
||||
},
|
||||
])
|
||||
.unwrap();
|
||||
db.photo_set_thumb("jpeg", THUMB_HAVE, Some(1.5)).unwrap();
|
||||
}
|
||||
{
|
||||
// Put the file back in the state a released V29 database was in.
|
||||
let conn = rusqlite::Connection::open(&path).unwrap();
|
||||
conn.execute_batch(
|
||||
"DROP INDEX idx_photos_group;
|
||||
ALTER TABLE photos DROP COLUMN group_key;
|
||||
ALTER TABLE photos DROP COLUMN main_uid;
|
||||
ALTER TABLE photos DROP COLUMN content_hash;
|
||||
UPDATE sync_state SET value = '29' WHERE key = 'schema_version';",
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
let db = Db::open(&path).unwrap();
|
||||
let page = db.photos_page(0, 10, None, None, false).unwrap();
|
||||
assert_eq!(
|
||||
page.iter().map(|p| p.uid.as_str()).collect::<Vec<_>>(),
|
||||
["jpeg", "raw"],
|
||||
"no photo goes missing before the next refresh groups them"
|
||||
);
|
||||
assert!(page.iter().all(|p| p.group_size == 1));
|
||||
assert_eq!(page[0].ratio, Some(1.5), "what was learned is still there");
|
||||
|
||||
// The next refresh fills the column in and the pair collapses.
|
||||
db.photos_replace(&[
|
||||
TimelineRow {
|
||||
name: Some("IMG_1234.JPG".into()),
|
||||
..TimelineRow::new("jpeg", 300)
|
||||
},
|
||||
TimelineRow {
|
||||
name: Some("IMG_1234.CR2".into()),
|
||||
..TimelineRow::new("raw", 300)
|
||||
},
|
||||
])
|
||||
.unwrap();
|
||||
let page = db.photos_page(0, 10, None, None, false).unwrap();
|
||||
assert_eq!(page.len(), 1);
|
||||
assert_eq!(page[0].uid, "jpeg");
|
||||
|
||||
let _ = std::fs::remove_file(&path);
|
||||
let _ = std::fs::remove_file(format!("{}.lock", path.display()));
|
||||
}
|
||||
Reference in new issue
Block a user