Make storage pluggable, and prove it with a folder backend
`RemoteBackend` existed from the first release and bought nothing it was
designed for. Seven files in `dr-ui` constructed a `NextcloudBackend`
directly, an account *was* a server URL beside a DAV user id, the local
cache directory was named after a hostname, and the launch screen knew
that signing in meant a browser handshake. The trait was real; the seam
was documentation.
A trait over operations is only a quarter of it. Pluggable storage needs
four things, and this adds the other three:
- **Capabilities** — already there, and the reason the engine can drive
two backends at the speed each actually runs at.
- **Configuration** — `dr_sync::Account`: where a library lives, in
whatever form its connector addresses, with no server in it. Loads
every existing config unchanged (`backend` defaults to `nextcloud`,
`endpoint` is stored under its historical `server` key), and
`Account::namespace()` reproduces the old catalog directory byte for
byte, because changing it would abandon a catalog, its thumbnail
shards, and the sidecars holding unsynced offline work.
- **Registration** — `BackendProvider` and `BackendRegistry`.
`ui/dr-ui/src/remote.rs` is now the only file above `dr-sync` that
names a connector.
`Connection` (an account plus an optional `Secret`) replaces the
credentials-and-user-id pair that was threaded through fifteen
signatures in an order that could be swapped. `Secret`'s inner string is
reachable only through `expose()` and its `Debug` prints `Secret(***)`,
so the indirect leak — a `{:?}` on anything holding one — no longer
compiles into a leak.
Nextcloud is unchanged and keeps every peculiarity: propagating ETags,
chunked upload v2, `oc:fileid`, the `oc:permissions` probe on a refused
PUT, the 423 retry classification, Login Flow v2. Those are what the
capability model exists to serve, not something to hide.
`dr-sync-folder` is the second connector: a local disk, a network mount,
an external drive, or a folder a Nextcloud client already syncs. No
account, no credential — the route that works where no secrets daemon
does. It declares `LocalEtags` rather than claiming propagation a POSIX
directory cannot provide, which costs nothing because 50k `stat` calls
are not 50k PROPFINDs. Identity is a path hash, not an inode: an inode
survives a rename but differs between devices and is reused after a
delete, so two machines would disagree about which photograph a
thumbnail belonged to. Re-deriving a thumbnail is a cost; showing the
wrong one is a bug.
docs/storage.md is the contract — the traits, the four steps to add a
backend, and what each connector declares. ARCH §8.0 and §8.4a, and
FR-NC-13, say why.
This commit is contained in:
+72
-77
@@ -25,8 +25,7 @@ use std::path::PathBuf;
|
||||
use std::sync::mpsc::{Receiver, Sender};
|
||||
|
||||
use dr_catalog::{Catalog, JobKind, Priority};
|
||||
use dr_sync::{RemoteBackend, RemoteId, RemotePath};
|
||||
use dr_sync_nextcloud::AppCredentials;
|
||||
use dr_sync::{Account, Connection, RemoteBackend, RemoteId, RemotePath};
|
||||
use dr_thumbs::ThumbStore;
|
||||
|
||||
use crate::sidecar_cache::SidecarCache;
|
||||
@@ -573,8 +572,7 @@ pub fn sidecar_path(image_path: &str) -> String {
|
||||
/// about it. Interrupting a cull with an error dialog per frame would be far
|
||||
/// worse than the risk. The counts are reported once, at the end.
|
||||
pub fn spawn_sidecar_writes(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
writes: Vec<SidecarWrite>,
|
||||
cache_dir: PathBuf,
|
||||
offline: bool,
|
||||
@@ -626,7 +624,7 @@ pub fn spawn_sidecar_writes(
|
||||
let report = match rt {
|
||||
None => queue_all(),
|
||||
Some(rt) => rt.block_on(async {
|
||||
match crate::remote::connect(&creds, &user_id) {
|
||||
match crate::remote::connect(&conn) {
|
||||
Ok(b) => {
|
||||
let mut report = SidecarReport::default();
|
||||
for w in &writes {
|
||||
@@ -860,11 +858,7 @@ async fn write_one_sidecar_online(
|
||||
/// The marker is cleared only after the server has taken the bytes. A drain
|
||||
/// interrupted halfway leaves the rest of the outbox exactly as it was, so
|
||||
/// nothing depends on this running to completion.
|
||||
pub fn spawn_outbox_drain(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
cache_dir: PathBuf,
|
||||
) -> Receiver<SidecarMessage> {
|
||||
pub fn spawn_outbox_drain(conn: Connection, cache_dir: PathBuf) -> Receiver<SidecarMessage> {
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
|
||||
std::thread::spawn(move || {
|
||||
@@ -895,7 +889,7 @@ pub fn spawn_outbox_drain(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
let _ = tx.send(SidecarMessage::Finished {
|
||||
@@ -1001,20 +995,17 @@ fn merge_into(local: &mut dr_pipeline::Sidecar, remote: &dr_pipeline::Sidecar) {
|
||||
|
||||
/// Where the catalog for an account lives.
|
||||
///
|
||||
/// Keyed by server and user so two accounts do not share an index. Under the
|
||||
/// XDG data directory, not cache: the catalog is rebuildable but rebuilding it
|
||||
/// costs a full rescan, so it is not something to discard on a cache sweep.
|
||||
pub fn catalog_path(server: &str, user_id: &str) -> PathBuf {
|
||||
let slug: String = server
|
||||
.trim_start_matches("https://")
|
||||
.trim_start_matches("http://")
|
||||
.chars()
|
||||
.map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
|
||||
.collect();
|
||||
|
||||
data_root()
|
||||
.join(format!("{slug}-{user_id}"))
|
||||
.join("catalog.sqlite")
|
||||
/// Keyed by [`Account::namespace`] so two accounts do not share an index —
|
||||
/// two servers, two logins on one server, or two folders on one disk. Under
|
||||
/// the XDG data directory, not cache: the catalog is rebuildable but
|
||||
/// rebuilding it costs a full rescan, so it is not something to discard on a
|
||||
/// cache sweep.
|
||||
///
|
||||
/// The namespace is the account's to compute, not this function's, because it
|
||||
/// is also frozen: it names the directory an existing install's catalog,
|
||||
/// thumbnail shards and un-uploaded sidecars are already in.
|
||||
pub fn catalog_path(account: &Account) -> PathBuf {
|
||||
data_root().join(account.namespace()).join("catalog.sqlite")
|
||||
}
|
||||
|
||||
/// The directory every account's data hangs off.
|
||||
@@ -1032,7 +1023,7 @@ pub fn catalog_path(server: &str, user_id: &str) -> PathBuf {
|
||||
/// reached the server, is the worst failure this application can have, and
|
||||
/// it would be silent.
|
||||
///
|
||||
/// `SessionStore::data_dir()` is the persistent per-app directory the
|
||||
/// `AccountStore::data_dir()` is the persistent per-app directory the
|
||||
/// Android entry point establishes before anything opens a store. On a
|
||||
/// desktop it is the XDG config directory, and the two lines below keep the
|
||||
/// established XDG *data* location there rather than moving anyone's
|
||||
@@ -1041,7 +1032,7 @@ fn data_root() -> PathBuf {
|
||||
let base = std::env::var_os("XDG_DATA_HOME")
|
||||
.map(PathBuf::from)
|
||||
.or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".local/share")))
|
||||
.unwrap_or_else(dr_sync_nextcloud::session::SessionStore::data_dir);
|
||||
.unwrap_or_else(dr_sync::AccountStore::data_dir);
|
||||
|
||||
base.join("darkroom")
|
||||
}
|
||||
@@ -1064,15 +1055,12 @@ fn data_root() -> PathBuf {
|
||||
/// one filesystem, so it is atomic and cannot half-finish. If the destination
|
||||
/// already exists this does nothing — the migration has run, or this is a
|
||||
/// fresh install, and in neither case may it overwrite live data.
|
||||
pub fn migrate_legacy_cache_data(server: &str, user_id: &str) {
|
||||
pub fn migrate_legacy_cache_data(account: &Account) {
|
||||
// Only meaningful where the old fallback and the new one differ, which is
|
||||
// exactly the platform that had the problem. On a desktop with XDG set,
|
||||
// both resolve to the same place and this returns immediately.
|
||||
let legacy_base = std::env::temp_dir();
|
||||
let Some(current) = catalog_path(server, user_id)
|
||||
.parent()
|
||||
.map(|p| p.to_path_buf())
|
||||
else {
|
||||
let Some(current) = catalog_path(account).parent().map(|p| p.to_path_buf()) else {
|
||||
return;
|
||||
};
|
||||
let Some(account) = current.file_name() else {
|
||||
@@ -1117,8 +1105,7 @@ fn move_account_dir(legacy: &std::path::Path, current: &std::path::Path) {
|
||||
/// Returns the receiver the UI drains. The worker owns its own tokio runtime
|
||||
/// and backend; nothing here touches the Slint event loop.
|
||||
pub fn spawn_scan(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
root: String,
|
||||
filter: FormatFilter,
|
||||
catalog_path: PathBuf,
|
||||
@@ -1127,7 +1114,7 @@ pub fn spawn_scan(
|
||||
|
||||
std::thread::spawn(move || {
|
||||
let started = std::time::Instant::now();
|
||||
if let Err(e) = run_scan(&tx, creds, user_id, root, filter, catalog_path, started) {
|
||||
if let Err(e) = run_scan(&tx, conn, root, filter, catalog_path, started) {
|
||||
let _ = tx.send(ScanMessage::Failed {
|
||||
message: e.message,
|
||||
offline: e.offline,
|
||||
@@ -1169,8 +1156,7 @@ impl From<dr_sync::RemoteError> for ScanFailure {
|
||||
|
||||
fn run_scan(
|
||||
tx: &Sender<ScanMessage>,
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
root: String,
|
||||
filter: FormatFilter,
|
||||
catalog_path: PathBuf,
|
||||
@@ -1185,7 +1171,7 @@ fn run_scan(
|
||||
let rt = crate::net_runtime::build().map_err(ScanFailure::local)?;
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = crate::remote::connect(&creds, &user_id).map_err(ScanFailure::local)?;
|
||||
let backend = crate::remote::connect(&conn).map_err(ScanFailure::local)?;
|
||||
|
||||
// Stored folder ETags, so an unchanged subtree is skipped whole. On a
|
||||
// first run this is empty and the walk is complete; on every run after
|
||||
@@ -1469,8 +1455,7 @@ pub enum PinMessage {
|
||||
/// memory — and it is the same contention that produced 423 Locked in the
|
||||
/// sweep.
|
||||
pub fn spawn_pin_fetch(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
catalog_path: PathBuf,
|
||||
cache_dir: PathBuf,
|
||||
budget: dr_catalog::Budget,
|
||||
@@ -1531,7 +1516,7 @@ pub fn spawn_pin_fetch(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
let _ = tx.send(PinMessage::Failed {
|
||||
@@ -1678,8 +1663,7 @@ pub struct CacheContext {
|
||||
/// not read, so an edit this build failed to understand is never destroyed by
|
||||
/// having been opened.
|
||||
pub fn spawn_sidecar_fetch(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
image_path: String,
|
||||
cache_dir: PathBuf,
|
||||
offline: bool,
|
||||
@@ -1720,7 +1704,7 @@ pub fn spawn_sidecar_fetch(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
log::debug!("sidecar fetch backend: {e}");
|
||||
@@ -1769,8 +1753,7 @@ pub fn spawn_sidecar_fetch(
|
||||
}
|
||||
|
||||
pub fn spawn_full_fetch(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
path: String,
|
||||
cache: Option<CacheContext>,
|
||||
) -> Receiver<Result<Vec<u8>, FetchFailure>> {
|
||||
@@ -1808,7 +1791,7 @@ pub fn spawn_full_fetch(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
let _ = tx.send(Err(FetchFailure::local(e)));
|
||||
@@ -1851,8 +1834,7 @@ pub fn spawn_full_fetch(
|
||||
/// or a second device that synced the shards — fills the grid with no transfer
|
||||
/// at all. Only genuine misses reach the network.
|
||||
pub fn spawn_thumbnails(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
wanted: Vec<ThumbnailRequest>,
|
||||
store_dir: PathBuf,
|
||||
catalog_path: PathBuf,
|
||||
@@ -1958,7 +1940,7 @@ pub fn spawn_thumbnails(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
for req in &to_fetch {
|
||||
@@ -2487,11 +2469,7 @@ const SWEEP_LANES: usize = 6;
|
||||
///
|
||||
/// Runs at the back of the queue by design: it holds no lock the grid needs,
|
||||
/// and its chunked commits keep write transactions short.
|
||||
pub fn spawn_sweep(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
catalog_path: PathBuf,
|
||||
) -> Receiver<SweepMessage> {
|
||||
pub fn spawn_sweep(conn: Connection, catalog_path: PathBuf) -> Receiver<SweepMessage> {
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
|
||||
std::thread::spawn(move || {
|
||||
@@ -2529,7 +2507,7 @@ pub fn spawn_sweep(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let Ok(backend) = crate::remote::connect(&creds, &user_id) else {
|
||||
let Ok(backend) = crate::remote::connect(&conn) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -2862,8 +2840,7 @@ fn faces_without_proxy(
|
||||
/// the images in flight and nothing else.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn spawn_face_sweep(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
catalog_path: PathBuf,
|
||||
store_dir: PathBuf,
|
||||
detector_model: PathBuf,
|
||||
@@ -2991,7 +2968,7 @@ pub fn spawn_face_sweep(
|
||||
rt.block_on(async {
|
||||
// Through `remote::connect`, which is the only place in the
|
||||
// interface that knows whose backend this is.
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
log::warn!("face sweep: {e}");
|
||||
@@ -3226,8 +3203,7 @@ pub enum ThumbSweepMessage {
|
||||
/// the alternative is a second piece of state that has to be invalidated when
|
||||
/// a file is replaced.
|
||||
pub fn spawn_thumbnail_sweep(
|
||||
creds: AppCredentials,
|
||||
user_id: String,
|
||||
conn: Connection,
|
||||
catalog_path: PathBuf,
|
||||
store_dir: PathBuf,
|
||||
) -> Receiver<ThumbSweepMessage> {
|
||||
@@ -3300,7 +3276,7 @@ pub fn spawn_thumbnail_sweep(
|
||||
};
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = match crate::remote::connect(&creds, &user_id) {
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
log::warn!("thumbnail sweep: {e}");
|
||||
@@ -3457,8 +3433,8 @@ fn thumbnails_outstanding(
|
||||
/// Beside the catalog rather than in the cache directory: these sync to the
|
||||
/// server and are shared with other clients, so discarding them on a cache
|
||||
/// sweep would cost a re-download for everyone.
|
||||
pub fn thumbs_dir(server: &str, user_id: &str) -> PathBuf {
|
||||
catalog_path(server, user_id)
|
||||
pub fn thumbs_dir(account: &Account) -> PathBuf {
|
||||
catalog_path(account)
|
||||
.parent()
|
||||
.map(|p| p.join("thumbs"))
|
||||
.unwrap_or_else(|| std::env::temp_dir().join("darkroom-thumbs"))
|
||||
@@ -3471,8 +3447,8 @@ pub fn thumbs_dir(server: &str, user_id: &str) -> PathBuf {
|
||||
/// project's licence, so the user obtains them and the app loads them from here
|
||||
/// (docs/faces.md §2). An absent directory is the ordinary state of a fresh
|
||||
/// install, not an error.
|
||||
pub fn face_models_dir(server: &str, user_id: &str) -> PathBuf {
|
||||
catalog_path(server, user_id)
|
||||
pub fn face_models_dir(account: &Account) -> PathBuf {
|
||||
catalog_path(account)
|
||||
.parent()
|
||||
.map(|p| p.join("models"))
|
||||
.unwrap_or_else(|| std::env::temp_dir().join("darkroom-models"))
|
||||
@@ -3511,13 +3487,13 @@ pub fn shared_face_models_dir() -> PathBuf {
|
||||
/// 3. **The system directories.** Where a package installs them — the Arch
|
||||
/// package puts the pair in `/usr/share/darkroom/models`. Last, so anything
|
||||
/// the user placed themselves outranks what the package shipped.
|
||||
pub fn face_models(server: &str, user_id: &str) -> Option<(PathBuf, PathBuf)> {
|
||||
pub fn face_models(account: &Account) -> Option<(PathBuf, PathBuf)> {
|
||||
fn pair(dir: PathBuf) -> Option<(PathBuf, PathBuf)> {
|
||||
let detector = dir.join("scrfd_500m_640.onnx");
|
||||
let embedder = dir.join("arcface_mbf_b1.onnx");
|
||||
(detector.is_file() && embedder.is_file()).then_some((detector, embedder))
|
||||
}
|
||||
pair(face_models_dir(server, user_id))
|
||||
pair(face_models_dir(account))
|
||||
.or_else(|| pair(shared_face_models_dir()))
|
||||
.or_else(|| system_face_models_dirs().into_iter().find_map(pair))
|
||||
}
|
||||
@@ -4302,17 +4278,36 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// An account for the path tests, defaulting to the connector every
|
||||
/// existing install uses.
|
||||
fn account(endpoint: &str, user: &str) -> Account {
|
||||
Account::new("nextcloud", endpoint).with_login(user, user)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_paths_separate_accounts() {
|
||||
// Two accounts on one machine must not share an index, or one
|
||||
// library's images appear in the other.
|
||||
let a = catalog_path("https://cloud.example", "duncan");
|
||||
let b = catalog_path("https://cloud.example", "someone");
|
||||
let c = catalog_path("https://other.example", "duncan");
|
||||
let a = catalog_path(&account("https://cloud.example", "duncan"));
|
||||
let b = catalog_path(&account("https://cloud.example", "someone"));
|
||||
let c = catalog_path(&account("https://other.example", "duncan"));
|
||||
assert_ne!(a, b);
|
||||
assert_ne!(a, c);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_folder_library_gets_its_own_catalog() {
|
||||
// The same rule across backends: a folder library on this machine
|
||||
// must not land in the directory a server account is already using.
|
||||
let server = catalog_path(&account("https://cloud.example", "duncan"));
|
||||
let folder = catalog_path(&Account::new("folder", "/mnt/photos"));
|
||||
assert_ne!(server, folder);
|
||||
assert_ne!(
|
||||
folder,
|
||||
catalog_path(&Account::new("folder", "/mnt/other-photos"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_legacy_cache_directory_is_moved_rather_than_abandoned() {
|
||||
// The upgrade hazard: `sidecars/` and `outbox/` hold work that exists
|
||||
@@ -4372,7 +4367,7 @@ mod tests {
|
||||
// and edit, and `outbox/`, holding exports the user was told had
|
||||
// succeeded. Losing a day of culling to an OS housekeeping pass, with
|
||||
// no error and no trace, is the worst outcome this application has.
|
||||
let path = catalog_path("https://cloud.example", "duncan");
|
||||
let path = catalog_path(&account("https://cloud.example", "duncan"));
|
||||
let text = path.to_string_lossy().to_lowercase();
|
||||
assert!(
|
||||
!text.contains("/cache/") && !text.contains("/tmp/"),
|
||||
@@ -4386,17 +4381,17 @@ mod tests {
|
||||
// Stated as a test because three separate call sites derive their
|
||||
// location by taking this path's parent, and a change here moves all
|
||||
// of them at once — including the two holding unsynced user work.
|
||||
let catalog = catalog_path("https://cloud.example", "duncan");
|
||||
let catalog = catalog_path(&account("https://cloud.example", "duncan"));
|
||||
let parent = catalog.parent().expect("a parent");
|
||||
assert_eq!(
|
||||
crate::export::outbox_dir("https://cloud.example", "duncan"),
|
||||
crate::export::outbox_dir(&account("https://cloud.example", "duncan")),
|
||||
parent.join("outbox")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_path_is_filesystem_safe() {
|
||||
let p = catalog_path("https://cloud.example.com:8443/nc", "duncan");
|
||||
let p = catalog_path(&account("https://cloud.example.com:8443/nc", "duncan"));
|
||||
let s = p.to_string_lossy();
|
||||
assert!(!s.contains("://"));
|
||||
assert!(!s.contains(':') || cfg!(windows));
|
||||
@@ -5029,8 +5024,8 @@ mod tests {
|
||||
fn thumbs_live_beside_the_catalog_not_in_the_cache() {
|
||||
// They sync to the server and are shared with other clients, so a
|
||||
// cache sweep must not discard them.
|
||||
let cat = catalog_path("https://cloud.example", "duncan");
|
||||
let thumbs = thumbs_dir("https://cloud.example", "duncan");
|
||||
let cat = catalog_path(&account("https://cloud.example", "duncan"));
|
||||
let thumbs = thumbs_dir(&account("https://cloud.example", "duncan"));
|
||||
assert_eq!(thumbs.parent(), cat.parent());
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user