dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Let the interface hold a backend without knowing whose it is

Browse Source 
`dr-sync` defines `RemoteBackend` and a capability model the engine adapts
to, so a second backend can be added without touching the code that uses
one. That boundary was documentation. Seven files in `dr-ui` constructed a
`NextcloudBackend` directly, ten functions took one by concrete type, and
exactly two call sites in the tree — both inside `dr-sync` itself — ever held
the trait object. A WebDAV or local-folder backend would have had a
well-written trait to implement and nowhere to go afterwards.

The change is smaller than the finding suggests, because the trait was
already right. Every method the UI has ever called on a backend — `get`,
`put`, `list`, `delete`, `create_dir`, `move_to` — was already on it, so
nothing had to be added and no behaviour moved. Ten signatures widened to
`&dyn RemoteBackend`, sixteen constructions became `remote::connect`, and
`remote.rs` is now the only file in the interface that names a connector.

`connect` returns `Result<Box<dyn RemoteBackend>, RemoteError>`. The error
type is `dr-sync`'s rather than the connector's, which is why every call site
kept its shape — the `match`, the `let Ok(..) else`, and
`.map_err(ScanFailure::local)?` all still read as they did.

One wrinkle worth recording: `&Box<dyn Trait>` does not reach `&dyn Trait` on
its own. The compiler reaches for unsizing, which wants
`Box<dyn RemoteBackend>: RemoteBackend`, and reports a confusing missing impl
rather than suggesting a deref. Twelve call sites therefore say `&*backend`,
and two say `let backend: &dyn RemoteBackend = &*backend` where a borrow is
shared across lanes.

What this does *not* do is abstract credentials. `AppCredentials` is an app
password from Login Flow v2 — a Nextcloud protocol, not a general notion of
authenticating to a remote — and seven files still name it. An OAuth token, a
bucket key pair and an app password have no useful common shape, so deciding
what an account is across backends before a second one exists would be a
confident guess. code-health.md CH-2 now records that as the remaining half,
and it should wait for the backend that forces it.

Verified: fmt clean, clippy clean at -D warnings, 2041 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-27 19:51:23 +02:00
co-authored by Claude Opus 5
parent 617262b4da
commit 242374fd0f
11 changed files with 127 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/derived_sync.rs
+7 -7
View File
@@ -35,7 +35,7 @@
use std::path::{Path, PathBuf};
use dr_sync::{RemoteBackend, RemoteId, RemotePath};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
use dr_sync_nextcloud::AppCredentials;
use dr_thumbs::ThumbStore;
/// Folder under the library root holding derived state.
@@ -117,7 +117,7 @@ pub fn spawn_sync(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(SyncMessage::Failed(e.to_string()));
@@ -125,7 +125,7 @@ pub fn spawn_sync(
}
};
match run(&backend, &root, &thumbs_dir, &catalog_path, &scratch, &tx).await {
match run(&*backend, &root, &thumbs_dir, &catalog_path, &scratch, &tx).await {
Ok(report) => {
let _ = tx.send(SyncMessage::Finished(Box::new(report)));
}
@@ -140,7 +140,7 @@ pub fn spawn_sync(
}
async fn run(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
root: &str,
thumbs_dir: &Path,
catalog_path: &Path,
@@ -193,7 +193,7 @@ fn derived_path(root: &str) -> RemotePath {
/// namespace, and [`ThumbStore::adopted`] then tracks what has been merged by
/// remote name instead of by our own ids.
async fn sync_shards(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
base: &RemotePath,
thumbs_dir: &Path,
scratch: &Path,
@@ -323,7 +323,7 @@ async fn sync_shards(
/// carrier. So this exports the catalog's new faces into the local shard store
/// first, syncs the shards, and imports whatever arrived back into the catalog.
async fn sync_face_shards(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
base: &RemotePath,
catalog_path: &Path,
scratch: &Path,
@@ -482,7 +482,7 @@ fn legacy_upload_of_ours(store: &ThumbStore, id: u32, remote_size: u64) -> bool
/// describes local state (folder ETags, cache paths, job rows) and importing
/// another device's version would be actively wrong.
async fn sync_catalog(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
base: &RemotePath,
catalog_path: &Path,
scratch: &Path,
ui/dr-ui/src/export.rs
+3 -3
View File
@@ -61,8 +61,8 @@ use std::sync::Arc;
use std::time::Duration;
use dr_export::{Encoded, NameContext};
use dr_sync::{RemoteBackend, RemotePath};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
use dr_sync::RemotePath;
use dr_sync_nextcloud::AppCredentials;
use dr_types::{ExportSettings, ExportTarget};
use crate::AppWindow;
@@ -339,7 +339,7 @@ pub fn spawn_upload(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(UploadMessage::Finished {
ui/dr-ui/src/import.rs
+5 -5
View File
@@ -56,7 +56,7 @@ use std::sync::Arc;
use dr_ingest::{Candidate, DupKey, Imported, Ingest, Options, Report, Shot, TransferMode};
use dr_plat::{DirRef, LocalStorage, Storage, WritableStorage};
use dr_sync::{RemoteBackend, RemotePath};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
use dr_sync_nextcloud::AppCredentials;
use dr_types::{FormatFilter, RootId};
/// Which root the card is granted as, and which the library is.
@@ -385,7 +385,7 @@ fn upload_all(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&upload.credentials, &upload.user_id) {
let backend = match crate::remote::connect(&upload.credentials, &upload.user_id) {
Ok(b) => b,
Err(e) => {
log::warn!("connecting to upload: {e}");
@@ -423,7 +423,7 @@ fn upload_all(
// The same folder segments the local copy went into, so the two
// libraries have the same shape (FR-NC-7a).
match dr_sync::upload_original(&backend, &root, &image.folders, &image.name, bytes)
match dr_sync::upload_original(&*backend, &root, &image.folders, &image.name, bytes)
.await
{
Ok(placed) => {
@@ -445,7 +445,7 @@ fn upload_all(
Err(e) => log::warn!("uploading {}: {e}", image.name),
}
}
let filed = file_thumbnails(&backend, &root, imported, &sent, &upload.thumbs).await;
let filed = file_thumbnails(&*backend, &root, imported, &sent, &upload.thumbs).await;
(sent, filed)
})
}
@@ -502,7 +502,7 @@ fn make_thumbnail(bytes: &[u8]) -> Option<dr_thumbs::Thumbnail> {
/// Best-effort throughout. A thumbnail that cannot be filed costs the grid one
/// preview fetch later; failing the import over it would be the wrong trade.
async fn file_thumbnails(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
library: &RemotePath,
imported: &[Imported],
sent: &[Sent],
ui/dr-ui/src/launch_ui.rs
+3 -3
View File
@@ -8,8 +8,8 @@ use std::cell::RefCell;
use std::rc::Rc;
use dr_plat::PlatformSecretStore;
use dr_sync::{RemoteBackend, RemotePath};
use dr_sync_nextcloud::{auth, NextcloudBackend, Session, SessionStore};
use dr_sync::RemotePath;
use dr_sync_nextcloud::{auth, Session, SessionStore};
use slint::ComponentHandle;
@@ -608,7 +608,7 @@ fn spawn_folder_list(weak: slint::Weak<AppWindow>, ctl: Rc<LaunchController>, pa
return;
};
rt.block_on(async {
match NextcloudBackend::new(&creds, &user_id) {
match crate::remote::connect(&creds, &user_id) {
Ok(b) => match b.list(&RemotePath::new(&path), None).await {
Ok(entries) => {
let mut dirs: Vec<String> = entries
ui/dr-ui/src/lib.rs
+1
View File
@@ -39,6 +39,7 @@ mod live_style;
mod masks_ui;
mod net_runtime;
mod presets;
mod remote;
mod segmentation;
mod settings_store;
mod settings_ui;
ui/dr-ui/src/library.rs
+22 -22
View File
@@ -26,7 +26,7 @@ use std::sync::mpsc::{Receiver, Sender};
use dr_catalog::{Catalog, JobKind, Priority};
use dr_sync::{RemoteBackend, RemoteId, RemotePath};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
use dr_sync_nextcloud::AppCredentials;
use dr_thumbs::ThumbStore;
use crate::sidecar_cache::SidecarCache;
@@ -530,11 +530,11 @@ pub fn spawn_sidecar_writes(
let report = match rt {
None => queue_all(),
Some(rt) => rt.block_on(async {
match NextcloudBackend::new(&creds, &user_id) {
match crate::remote::connect(&creds, &user_id) {
Ok(b) => {
let mut report = SidecarReport::default();
for w in &writes {
match write_one_sidecar_online(&b, &cache, w).await {
match write_one_sidecar_online(&*b, &cache, w).await {
Ok(Outcome::Uploaded) => report.written += 1,
Ok(Outcome::Queued) => report.queued += 1,
Err(e) => {
@@ -696,7 +696,7 @@ fn write_one_sidecar(cache: &SidecarCache, w: &SidecarWrite) -> Result<Outcome,
/// TRACES: FR-CAT-8 | FR-CAT-9
/// Read-modify-write one sidecar, with a server to read from and send to.
async fn write_one_sidecar_online(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
cache: &SidecarCache,
w: &SidecarWrite,
) -> Result<Outcome, String> {
@@ -799,7 +799,7 @@ pub fn spawn_outbox_drain(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(SidecarMessage::Finished {
@@ -814,7 +814,7 @@ pub fn spawn_outbox_drain(
let mut report = SidecarReport::default();
for path_str in &queued {
match drain_one(&backend, &cache, path_str).await {
match drain_one(&*backend, &cache, path_str).await {
Ok(()) => report.written += 1,
Err(e) => {
// Warn, for the reason the write path does: this is
@@ -843,7 +843,7 @@ pub fn spawn_outbox_drain(
/// Reconcile one queued sidecar with the server and upload it.
async fn drain_one(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
cache: &SidecarCache,
path_str: &str,
) -> Result<(), String> {
@@ -1089,14 +1089,14 @@ fn run_scan(
let rt = crate::net_runtime::build().map_err(ScanFailure::local)?;
rt.block_on(async {
let backend = NextcloudBackend::new(&creds, &user_id).map_err(ScanFailure::local)?;
let backend = crate::remote::connect(&creds, &user_id).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
// it is what keeps cost proportional to what changed (ARCH §8.4).
let known = load_folder_etags(&catalog, &root);
let result = dr_sync::scan(&backend, &RemotePath::new(&root), &filter, &known, |p| {
let result = dr_sync::scan(&*backend, &RemotePath::new(&root), &filter, &known, |p| {
let _ = tx.send(ScanMessage::Progress {
directories: p.directories_listed,
pruned: p.directories_pruned,
@@ -1426,7 +1426,7 @@ pub fn spawn_pin_fetch(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(PinMessage::Failed {
@@ -1615,7 +1615,7 @@ pub fn spawn_sidecar_fetch(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
log::debug!("sidecar fetch backend: {e}");
@@ -1703,7 +1703,7 @@ pub fn spawn_full_fetch(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(Err(FetchFailure::local(e)));
@@ -1853,7 +1853,7 @@ pub fn spawn_thumbnails(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
for req in &to_fetch {
@@ -1878,7 +1878,7 @@ pub fn spawn_thumbnails(
let mut offline = false;
for req in to_fetch {
let msg = fetch_one(&backend, store.as_mut(), &req, &mut found).await;
let msg = fetch_one(&*backend, store.as_mut(), &req, &mut found).await;
offline = matches!(msg, ThumbnailMessage::Offline { .. });
// A closed channel means the window went away mid-fetch.
if tx.send(msg).is_err() || offline {
@@ -1903,7 +1903,7 @@ pub fn spawn_thumbnails(
if tx.send(ThumbnailMessage::DateProgress).is_err() {
break;
}
read_metadata_only(&backend, &req, &mut found).await;
read_metadata_only(&*backend, &req, &mut found).await;
if found.len() >= FLUSH_EVERY {
flush_metadata(&catalog_path, &mut found, &tx);
@@ -1922,7 +1922,7 @@ pub fn spawn_thumbnails(
}
async fn fetch_one(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
store: Option<&mut ThumbStore>,
req: &ThumbnailRequest,
found_metadata: &mut Vec<MetadataFound>,
@@ -1979,7 +1979,7 @@ enum PreviewOutcome {
/// the embedded JPEG partway through, and decoders render a truncated JPEG as
/// the top fraction of the frame rather than reporting an error.
async fn fetch_preview(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
req: &ThumbnailRequest,
found_metadata: &mut Vec<MetadataFound>,
) -> PreviewOutcome {
@@ -2185,7 +2185,7 @@ fn flush_metadata(
/// happened both leave `found` untouched, and recording the second as "this
/// image has no date" would let one lock mark it dateless for good.
async fn read_metadata_only(
backend: &NextcloudBackend,
backend: &dyn RemoteBackend,
req: &ThumbnailRequest,
found: &mut Vec<MetadataFound>,
) -> bool {
@@ -2410,7 +2410,7 @@ pub fn spawn_sweep(
};
rt.block_on(async {
let Ok(backend) = NextcloudBackend::new(&creds, &user_id) else {
let Ok(backend) = crate::remote::connect(&creds, &user_id) else {
return;
};
@@ -2449,7 +2449,7 @@ pub fn spawn_sweep(
.collect();
let results = futures_join_all(lanes.into_iter().map(|lane| {
let backend = &backend;
let backend: &dyn RemoteBackend = &*backend;
async move {
let mut found = Vec::new();
let mut reached = Vec::new();
@@ -2735,7 +2735,7 @@ pub fn spawn_thumbnail_sweep(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
log::warn!("thumbnail sweep: {e}");
@@ -2757,7 +2757,7 @@ pub fn spawn_thumbnail_sweep(
.collect();
let results = futures_join_all(lanes.into_iter().map(|lane| {
let backend = &backend;
let backend: &dyn RemoteBackend = &*backend;
async move {
let mut made: Vec<(u64, dr_thumbs::Thumbnail)> = Vec::new();
let mut found = Vec::new();
ui/dr-ui/src/remote.rs
+40
View File
@@ -0,0 +1,40 @@
// TRACES: FR-NC-12
//! The one place the interface names a backend.
//!
//! `dr-sync` defines [`RemoteBackend`] and a capability model the engine adapts
//! to, so that a second backend can be added without touching the code that
//! uses one (ARCH §8.1). Until this module existed that boundary was
//! documentation: seven files in `dr-ui` constructed a `NextcloudBackend`
//! directly and ten functions took one by concrete type, so the abstraction
//! bought nothing it was designed for and a WebDAV or local-folder backend
//! would have had nowhere to go.
//!
//! Everything above this module now works through `&dyn RemoteBackend`. Adding
//! a backend is implementing the trait and changing [`connect`] — not editing
//! seven files.
//!
//! ## What is deliberately still Nextcloud-shaped
//!
//! Credentials. [`AppCredentials`] is an app password obtained through Login
//! Flow v2, which is a Nextcloud protocol rather than a general notion of
//! "how one authenticates to a remote". Abstracting it needs a decision about
//! what an account *is* across backends — an OAuth token, a bucket key pair
//! and an app password have no useful common shape — and inventing one before
//! a second backend exists would produce a wrong answer confidently. That is
//! the remaining half of this seam, and it is a design problem rather than a
//! mechanical one.
use dr_sync::{RemoteBackend, RemoteError};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
/// Open a connection to the configured remote.
///
/// Returns the trait object every caller should hold. The error type is
/// `dr-sync`'s rather than the connector's, so a caller handles a failure
/// without learning which backend produced it.
pub(crate) fn connect(
creds: &AppCredentials,
user_id: &str,
) -> Result<Box<dyn RemoteBackend>, RemoteError> {
Ok(Box::new(NextcloudBackend::new(creds, user_id)?))
}
ui/dr-ui/src/settings_ui.rs
+2 -3
View File
@@ -619,8 +619,7 @@ pub fn spawn_folder_list(
user_id: String,
path: String,
) {
use dr_sync::{RemoteBackend, RemotePath};
use dr_sync_nextcloud::NextcloudBackend;
use dr_sync::RemotePath;
let (tx, rx) = std::sync::mpsc::channel::<Result<Vec<String>, String>>();
@@ -638,7 +637,7 @@ pub fn spawn_folder_list(
return;
};
rt.block_on(async {
match NextcloudBackend::new(&creds, &user_id) {
match crate::remote::connect(&creds, &user_id) {
Ok(b) => match b.list(&RemotePath::new(&path), None).await {
Ok(entries) => {
let mut dirs: Vec<String> = entries
ui/dr-ui/src/trash.rs
+4 -4
View File
@@ -31,8 +31,8 @@ use std::path::PathBuf;
use std::sync::mpsc::Receiver;
use dr_catalog::{trash, Catalog};
use dr_sync::{RemoteBackend, RemoteError, RemoteId, RemotePath};
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
use dr_sync::{RemoteError, RemoteId, RemotePath};
use dr_sync_nextcloud::AppCredentials;
use dr_types::ImageId;
/// What a trash operation reports back to the UI.
@@ -184,7 +184,7 @@ pub fn spawn_move(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(TrashMessage::Done {
@@ -297,7 +297,7 @@ pub fn spawn_purge(
};
rt.block_on(async {
let backend = match NextcloudBackend::new(&creds, &user_id) {
let backend = match crate::remote::connect(&creds, &user_id) {
Ok(b) => b,
Err(e) => {
let _ = tx.send(TrashMessage::Done {