Sync develop presets through the library
Presets were the one piece of the photographer's work that never left the device: faces, sidecars, albums, collections, keywords and camera profiles all travel with the sync pass, the preset library did not. It now goes to <derived>/presets/library.drpl. PresetLibrary::merge decides each name against the base the last exchange left (kept per library beside place.json), so presets added on two devices both survive, a deletion reaches the other device instead of being restored by it, and an edit outlives a deletion made elsewhere. The upload is If-Match / If-None-Match on the server's copy, and a 412 reads and merges again, so two devices exchanging at once cannot save over each other. A server copy that will not parse (a newer build's) is left alone, and a local file that will not read stops the exchange rather than being taken for an empty library. The develop view's save merges with the file when the sync changed it since the view read it, and a sync that brought presets reloads and redraws the list. Also corrects the register, which still said camera profiles do not sync.
This commit is contained in:
@@ -96,6 +96,11 @@ pub struct SyncReport {
|
||||
/// (camera-profiles.md §13).
|
||||
pub profiles_uploaded: usize,
|
||||
pub profiles_downloaded: usize,
|
||||
/// TRACES: FR-DEV-6
|
||||
/// The develop presets: whether another device's changes reached this
|
||||
/// one's library, and whether this one's reached the server.
|
||||
pub presets_adopted: bool,
|
||||
pub presets_uploaded: bool,
|
||||
}
|
||||
|
||||
impl SyncReport {
|
||||
@@ -109,6 +114,7 @@ impl SyncReport {
|
||||
|| self.face_shards_downloaded > 0
|
||||
|| self.place_adopted
|
||||
|| self.profiles_downloaded > 0
|
||||
|| self.presets_adopted
|
||||
}
|
||||
}
|
||||
|
||||
@@ -124,12 +130,15 @@ pub enum SyncMessage {
|
||||
///
|
||||
/// Runs on its own thread with its own runtime, like every other network path
|
||||
/// here — the Slint loop must never block (NFR-P9).
|
||||
// One argument per thing the pass touches; see `run` below.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn spawn_sync(
|
||||
conn: Connection,
|
||||
root: String,
|
||||
thumbs_dir: PathBuf,
|
||||
catalog_path: PathBuf,
|
||||
place_path: PathBuf,
|
||||
presets: PresetFiles,
|
||||
scratch: PathBuf,
|
||||
// TRACES: FR-CULL-8
|
||||
// Which face pipeline's shards to export and adopt. From the settings
|
||||
@@ -165,6 +174,7 @@ pub fn spawn_sync(
|
||||
&thumbs_dir,
|
||||
&catalog_path,
|
||||
&place_path,
|
||||
&presets,
|
||||
&scratch,
|
||||
&face_model_id,
|
||||
&tx,
|
||||
@@ -184,7 +194,7 @@ pub fn spawn_sync(
|
||||
rx
|
||||
}
|
||||
|
||||
// Eight, because a sync touches eight distinct things — the same reason
|
||||
// Nine, because a sync touches nine distinct things — the same reason
|
||||
// `repairs::spawn` carries the allow: bundling them into a struct would name
|
||||
// nothing that exists.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
@@ -194,6 +204,7 @@ async fn run(
|
||||
thumbs_dir: &Path,
|
||||
catalog_path: &Path,
|
||||
place_path: &Path,
|
||||
presets: &PresetFiles,
|
||||
scratch: &Path,
|
||||
face_model_id: &str,
|
||||
tx: &std::sync::mpsc::Sender<SyncMessage>,
|
||||
@@ -240,6 +251,12 @@ async fn run(
|
||||
sync_profiles(backend, &base, &dir, &mut report).await;
|
||||
}
|
||||
|
||||
// TRACES: FR-DEV-6
|
||||
// The develop presets, a few kilobytes. Like the profiles, never fails
|
||||
// the pass.
|
||||
let _ = tx.send(SyncMessage::Status("checking presets…".into()));
|
||||
sync_presets(backend, &base, presets, &mut report).await;
|
||||
|
||||
// TRACES: FR-UI-8
|
||||
// Last, and it costs one small GET plus at most one small PUT. Last because
|
||||
// it is the only thing here that is not derived state and so the only thing
|
||||
@@ -1122,6 +1139,156 @@ async fn sync_profiles(
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-6
|
||||
/// Where this device keeps the preset library, and what the last exchange
|
||||
/// of it with this library left both sides holding.
|
||||
///
|
||||
/// The library is the device's, shared by every library it opens; the base
|
||||
/// is per library, because each library's server holds its own copy and has
|
||||
/// its own history of exchanges with this device.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PresetFiles {
|
||||
pub library: PathBuf,
|
||||
pub base: PathBuf,
|
||||
}
|
||||
|
||||
/// The preset library on the server, in its own folder so finding it is a
|
||||
/// listing of one file rather than of every shard beside it.
|
||||
const PRESETS_DIR: &str = "presets";
|
||||
const PRESETS_NAME: &str = "library.drpl";
|
||||
|
||||
/// TRACES: FR-DEV-6
|
||||
/// Exchange the develop presets with `<derived>/presets/library.drpl`.
|
||||
///
|
||||
/// Unlike the place, this is merged rather than replaced: a preset saved on
|
||||
/// the tablet and another saved on the desktop between two passes must both
|
||||
/// survive, and a preset deleted on one must not come back from the other.
|
||||
/// [`PresetLibrary::merge`](dr_pipeline::PresetLibrary::merge) decides each
|
||||
/// name against the base the last exchange left, which is what tells a
|
||||
/// deletion here from an addition there.
|
||||
///
|
||||
/// The write is conditional on the server still holding what was read, so
|
||||
/// two devices exchanging at once cannot each save over the other's
|
||||
/// additions; the one that loses the race reads again and merges again.
|
||||
/// Never fails the pass.
|
||||
async fn sync_presets(
|
||||
backend: &dyn RemoteBackend,
|
||||
base: &RemotePath,
|
||||
files: &PresetFiles,
|
||||
report: &mut SyncReport,
|
||||
) {
|
||||
for _ in 0..3 {
|
||||
match exchange_presets(backend, base, files, report).await {
|
||||
Err(RemoteError::PreconditionFailed) => {
|
||||
log::debug!("presets: another device wrote first; reading again");
|
||||
}
|
||||
Err(e) => {
|
||||
log::debug!("not exchanging presets: {e}");
|
||||
return;
|
||||
}
|
||||
Ok(()) => return,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn exchange_presets(
|
||||
backend: &dyn RemoteBackend,
|
||||
base: &RemotePath,
|
||||
files: &PresetFiles,
|
||||
report: &mut SyncReport,
|
||||
) -> Result<(), RemoteError> {
|
||||
use crate::preset_store::PresetStore;
|
||||
use dr_pipeline::PresetLibrary;
|
||||
|
||||
let dir = RemotePath::new(format!("{}/{PRESETS_DIR}", base.as_str()));
|
||||
let target = RemotePath::new(format!("{}/{PRESETS_NAME}", dir.as_str()));
|
||||
|
||||
// A listing that fails reads as "not there". That is safe because the
|
||||
// write below is then `IfAbsent`, which a server holding one refuses.
|
||||
let listed = backend.list(&dir, None).await.ok().and_then(|entries| {
|
||||
entries
|
||||
.into_iter()
|
||||
.find(|e| e.kind == dr_sync::EntryKind::File && e.path.name() == PRESETS_NAME)
|
||||
});
|
||||
|
||||
let mut theirs = match &listed {
|
||||
None => PresetLibrary::default(),
|
||||
Some(_) => {
|
||||
let bytes = read_derived(backend, &target).await?;
|
||||
match std::str::from_utf8(&bytes)
|
||||
.map_err(|e| e.to_string())
|
||||
.and_then(|t| PresetLibrary::parse(t).map_err(|e| e.to_string()))
|
||||
{
|
||||
Ok(library) => library,
|
||||
Err(e) => {
|
||||
// A newer build's format, or damage. Either way not ours
|
||||
// to write over.
|
||||
log::warn!("the preset library on the server will not read ({e}); left alone");
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let store = PresetStore::open_at(files.library.clone());
|
||||
let read = match store.try_load() {
|
||||
Ok(library) => library.unwrap_or_default(),
|
||||
Err(e) => {
|
||||
// An empty library here would read as every preset deleted.
|
||||
log::warn!("not exchanging presets: {}: {e}", store.path().display());
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let base_store = PresetStore::open_at(files.base.clone());
|
||||
let last = base_store.try_load().ok().flatten().unwrap_or_default();
|
||||
|
||||
// Copies an older build seeded of the shipped presets are not the
|
||||
// photographer's, and are dropped on both sides before anything travels.
|
||||
let mut ours = read.clone();
|
||||
dr_pipeline::bundled::forget_unchanged_copies(&mut ours);
|
||||
dr_pipeline::bundled::forget_unchanged_copies(&mut theirs);
|
||||
|
||||
let merged = PresetLibrary::merge(&last, &ours, &theirs);
|
||||
|
||||
if listed.is_none() && merged.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
if listed.is_none() || merged != theirs {
|
||||
let _ = backend.create_dir(&dir).await;
|
||||
let precondition = match &listed {
|
||||
Some(entry) => dr_sync::Precondition::IfMatch(entry.validator.clone()),
|
||||
None => dr_sync::Precondition::IfAbsent,
|
||||
};
|
||||
backend
|
||||
.put(&target, merged.to_text().into_bytes(), Some(precondition))
|
||||
.await?;
|
||||
report.presets_uploaded = true;
|
||||
}
|
||||
|
||||
if merged != ours {
|
||||
// The develop view saves this file too. A preset it saved since the
|
||||
// read above is merged in rather than written over; it reaches the
|
||||
// server on the next pass, as an addition against the base below.
|
||||
let local = match store.try_load() {
|
||||
Ok(Some(now)) if now != read => PresetLibrary::merge(&read, &merged, &now),
|
||||
_ => merged.clone(),
|
||||
};
|
||||
match store.save(&local) {
|
||||
Ok(()) => report.presets_adopted = true,
|
||||
Err(e) => {
|
||||
log::warn!("saving presets from the library: {e}");
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
}
|
||||
if merged != last {
|
||||
if let Err(e) = base_store.save(&merged) {
|
||||
log::debug!("recording the preset exchange: {e}");
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// TRACES: FR-UI-8
|
||||
/// Fetch just the place, for the handover at launch.
|
||||
///
|
||||
@@ -1934,6 +2101,99 @@ mod derived_guard_tests {
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
/// One device's preset files, under `root`.
|
||||
fn preset_device(root: &Path, name: &str) -> PresetFiles {
|
||||
PresetFiles {
|
||||
library: root.join(name).join("presets.drpl"),
|
||||
base: root.join(name).join("presets.base.drpl"),
|
||||
}
|
||||
}
|
||||
|
||||
fn preset_names(files: &PresetFiles) -> Vec<String> {
|
||||
crate::preset_store::PresetStore::open_at(files.library.clone())
|
||||
.load()
|
||||
.names()
|
||||
.map(str::to_string)
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn save_presets(files: &PresetFiles, names: &[&str]) {
|
||||
let mut library = dr_pipeline::PresetLibrary::default();
|
||||
for name in names {
|
||||
library
|
||||
.insert(name, dr_pipeline::Preset::default())
|
||||
.unwrap();
|
||||
}
|
||||
crate::preset_store::PresetStore::open_at(files.library.clone())
|
||||
.save(&library)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn presets_reach_every_device_and_so_do_their_deletions() {
|
||||
// TRACES: FR-DEV-6
|
||||
let root = std::env::temp_dir().join(format!("dr-preset-sync-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
let server = root.join("server");
|
||||
std::fs::create_dir_all(server.join(".darkroom-derived")).unwrap();
|
||||
let backend = dr_sync_folder::FolderBackend::new(&server).unwrap();
|
||||
let base = RemotePath::new(".darkroom-derived");
|
||||
let (desk, tablet) = (preset_device(&root, "desk"), preset_device(&root, "tablet"));
|
||||
|
||||
save_presets(&desk, &["Warm"]);
|
||||
save_presets(&tablet, &["Mono"]);
|
||||
for files in [&desk, &tablet, &desk] {
|
||||
sync_presets(&backend, &base, files, &mut SyncReport::default()).await;
|
||||
}
|
||||
assert_eq!(preset_names(&desk), vec!["Mono", "Warm"]);
|
||||
assert_eq!(preset_names(&tablet), vec!["Mono", "Warm"]);
|
||||
|
||||
// Deleted on the desk: gone from the tablet, not back on the desk.
|
||||
save_presets(&desk, &["Mono"]);
|
||||
for files in [&desk, &tablet, &desk] {
|
||||
sync_presets(&backend, &base, files, &mut SyncReport::default()).await;
|
||||
}
|
||||
assert_eq!(preset_names(&desk), vec!["Mono"]);
|
||||
assert_eq!(preset_names(&tablet), vec!["Mono"]);
|
||||
|
||||
// Settled: a pass with nothing new writes nothing.
|
||||
let mut quiet = SyncReport::default();
|
||||
sync_presets(&backend, &base, &tablet, &mut quiet).await;
|
||||
assert!(!quiet.presets_uploaded && !quiet.presets_adopted);
|
||||
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_preset_library_the_server_cannot_read_is_left_alone() {
|
||||
// TRACES: FR-DEV-6
|
||||
// A newer build's format reads as unreadable here, and is that build's
|
||||
// presets: neither written over nor taken as an empty library.
|
||||
let root = std::env::temp_dir().join(format!("dr-preset-unread-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
let server = root.join("server");
|
||||
let held = server.join(".darkroom-derived/presets/library.drpl");
|
||||
std::fs::create_dir_all(held.parent().unwrap()).unwrap();
|
||||
std::fs::write(&held, "drpl 9999\n").unwrap();
|
||||
let backend = dr_sync_folder::FolderBackend::new(&server).unwrap();
|
||||
let desk = preset_device(&root, "desk");
|
||||
save_presets(&desk, &["Warm"]);
|
||||
|
||||
let mut report = SyncReport::default();
|
||||
sync_presets(
|
||||
&backend,
|
||||
&RemotePath::new(".darkroom-derived"),
|
||||
&desk,
|
||||
&mut report,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(!report.presets_uploaded);
|
||||
assert_eq!(std::fs::read_to_string(&held).unwrap(), "drpl 9999\n");
|
||||
assert_eq!(preset_names(&desk), vec!["Warm"]);
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_place_that_could_not_be_read_is_never_written_over() {
|
||||
// A dehydrated placeholder, and the record on the server may well be
|
||||
|
||||
Reference in New Issue
Block a user