From 1b94fea566d62e0ed70dd937be840e137f1ee153 Mon Sep 17 00:00:00 2001 From: Hunter Wittenborn Date: Mon, 23 Oct 2023 15:48:11 -0500 Subject: [PATCH] Only sync files when they're changed locally/remotely (#164) Previously we'd be syncing files in an endless loop, which wasn't great from a UI/UX perspective. This new implementation checks the local and remote files against those recorded since the last sync, and uses those to figure out if a directory sync needs to be done. Currently we sync the entire directory pair when a change is detected, but this is still a fair amount better than the implementation we had previously. We'll fine-tune out the implementation at a later time. --- CHANGELOG.md | 4 + Cargo.lock | 2 +- Cargo.toml | 2 +- .../com.hunterwittenborn.Celeste.metainfo.xml | 12 ++ makedeb/PKGBUILD | 2 +- po/com.hunterwittenborn.Celeste.pot | 61 +++--- src/launch.rs | 196 +++++++++++++++++- src/tray.rs | 4 + 8 files changed, 241 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77282bc..bccfc5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.8.0] - 2023-10-23 +### Changed +- Added functionality to only run sync checks when files are changed. + ## [0.7.0] - 2023-10-07 ### Changed - Remove reliance on GTK3 and libappindicator. diff --git a/Cargo.lock b/Cargo.lock index 3849988..bd0c3ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -602,7 +602,7 @@ dependencies = [ [[package]] name = "celeste" -version = "0.7.0" +version = "0.8.0" dependencies = [ "base64 0.20.0", "blocking", diff --git a/Cargo.toml b/Cargo.toml index d867663..8daab5a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "celeste" -version = "0.7.0" +version = "0.8.0" edition = "2021" [dependencies] diff --git a/assets/com.hunterwittenborn.Celeste.metainfo.xml b/assets/com.hunterwittenborn.Celeste.metainfo.xml index 23daf5e..a6a0e5a 100644 --- a/assets/com.hunterwittenborn.Celeste.metainfo.xml +++ b/assets/com.hunterwittenborn.Celeste.metainfo.xml @@ -41,6 +41,18 @@ + + +

+ Changes in this release: +

+
    +
  • + Added functionality to only run sync checks when files are changed. +
    • +
+ +

diff --git a/makedeb/PKGBUILD b/makedeb/PKGBUILD index f29abb5..1fdda10 100644 --- a/makedeb/PKGBUILD +++ b/makedeb/PKGBUILD @@ -1,6 +1,6 @@ # Maintainer: Hunter Wittenborn pkgname=celeste -pkgver=0.7.0 +pkgver=0.8.0 pkgrel=1 pkgdesc='Sync your cloud files' arch=('any') diff --git a/po/com.hunterwittenborn.Celeste.pot b/po/com.hunterwittenborn.Celeste.pot index 4348efd..8df07f1 100644 --- a/po/com.hunterwittenborn.Celeste.pot +++ b/po/com.hunterwittenborn.Celeste.pot @@ -6,9 +6,9 @@ #, fuzzy msgid "" msgstr "" -"Project-Id-Version: Celeste 0.7.0\n" +"Project-Id-Version: Celeste 0.8.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2023-10-08 00:55+0000\n" +"POT-Creation-Date: 2023-10-23 20:35+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -120,97 +120,96 @@ msgstr "" msgid "All the directories associated with this remote will also stop syncing." msgstr "" -#. Notify the tray app that we're syncing this remote now. -#: src/launch.rs:1160 +#: src/launch.rs:1342 src/launch.rs:2509 +msgid "Files are synced." +msgstr "" + +#: src/launch.rs:1352 msgid "Syncing '{}'..." msgstr "" -#: src/launch.rs:1194 +#: src/launch.rs:1373 msgid "Checking for changes..." msgstr "" #. Add an error for reporting in the UI. -#: src/launch.rs:1201 +#: src/launch.rs:1380 msgid "Please resolve the reported syncing issues." msgstr "" -#: src/launch.rs:1228 +#: src/launch.rs:1407 msgid "{} errors found. " msgstr "" -#: src/launch.rs:1242 +#: src/launch.rs:1421 msgid "Would you like to dismiss this error?" msgstr "" -#: src/launch.rs:1269 +#: src/launch.rs:1448 msgid "Failed to sync '{}' to '{}' on remote." msgstr "" -#: src/launch.rs:1277 +#: src/launch.rs:1456 msgid "Failed to sync '{}' on remote to '{}'." msgstr "" -#: src/launch.rs:1302 +#: src/launch.rs:1481 msgid "Unable to fetch data for '{}' from the remote." msgstr "" -#: src/launch.rs:1311 src/launch.rs:1316 src/launch.rs:1324 +#: src/launch.rs:1490 src/launch.rs:1495 src/launch.rs:1503 msgid "File Update" msgstr "" -#: src/launch.rs:1311 +#: src/launch.rs:1490 msgid "Neither the local item or remote item exists anymore. This error will now be removed." msgstr "" -#: src/launch.rs:1316 +#: src/launch.rs:1495 msgid "Only the local item exists now, so it will be synced to the remote." msgstr "" -#: src/launch.rs:1324 +#: src/launch.rs:1503 msgid "Only the remote item exists now, so it will be synced to the local machine." msgstr "" -#: src/launch.rs:1334 +#: src/launch.rs:1513 msgid "Both the local item '{}' and remote item '{}' have been updated since the last sync." msgstr "" -#: src/launch.rs:1336 +#: src/launch.rs:1515 msgid "Which item would you like to keep?" msgstr "" -#: src/launch.rs:1338 +#: src/launch.rs:1517 msgid "Local" msgstr "" -#: src/launch.rs:1339 +#: src/launch.rs:1518 msgid "Remote" msgstr "" -#: src/launch.rs:1388 +#: src/launch.rs:1567 msgid "1 error found." msgstr "" -#: src/launch.rs:1390 +#: src/launch.rs:1569 msgid "{} errors found." msgstr "" -#: src/launch.rs:1522 +#: src/launch.rs:1701 msgid "Checking '{}' for changes..." msgstr "" -#: src/launch.rs:1938 +#: src/launch.rs:2117 msgid "Checking '{}' on remote for changes..." msgstr "" -#: src/launch.rs:2329 -msgid "Directory has finished sync checks." -msgstr "" - -#: src/launch.rs:2350 +#: src/launch.rs:2530 msgid "Finished sync checks with {} errors." msgstr "" -#: src/launch.rs:2355 +#: src/launch.rs:2535 msgid "Finished sync checks." msgstr "" @@ -318,11 +317,11 @@ msgstr "" msgid "Awaiting sync checks..." msgstr "" -#: src/tray.rs:57 +#: src/tray.rs:60 msgid "Open" msgstr "" -#: src/tray.rs:64 +#: src/tray.rs:67 msgid "Close" msgstr "" diff --git a/src/launch.rs b/src/launch.rs index beb8f88..237ed46 100644 --- a/src/launch.rs +++ b/src/launch.rs @@ -31,7 +31,7 @@ use std::{ boxed, cell::RefCell, collections::HashMap, - fs::{self, OpenOptions}, + fs::{self, File, OpenOptions}, io::Write, path::{Path, PathBuf}, rc::Rc, @@ -1109,9 +1109,6 @@ pub fn launch(app: &Application, background: bool) { } util::run_in_background(|| thread::sleep(Duration::from_millis(500))); - if sync_errors_count() == 0 { - handle.update(|tray| tray.set_syncing()); - } for remote in remotes { // Process any remote deletion requests. @@ -1156,9 +1153,6 @@ pub fn launch(app: &Application, background: bool) { } } - // Notify the tray app that we're syncing this remote now. - handle.update(|tray| tray.set_msg(tr::tr!("Syncing '{}'...", remote.name))); - let sync_dirs = util::await_future( SyncDirsEntity::find() .filter(SyncDirsColumn::RemoteId.eq(remote.id)) @@ -1167,6 +1161,176 @@ pub fn launch(app: &Application, background: bool) { .unwrap(); for sync_dir in sync_dirs { + // Get the list of local and remote files for this sync + // directory, and if they don't match the last sync state + // in the database, then continue with syncing. + let mut should_sync = false; + + // Local file checks. + let local_glob = format!("{}/**/*", sync_dir.local_path); + if let Ok(paths) = glob::glob(&local_glob) { + for maybe_path in paths { + if let Ok(path) = maybe_path { + let file = match File::open(&path) { + Ok(file) => file, + Err(_) => { + should_sync = true; + break; + } + }; + let current_timestamp = file + .metadata() + .unwrap() + .modified() + .unwrap() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + let maybe_db_sync_item = util::await_future( + SyncItemsEntity::find() + .filter(SyncItemsColumn::SyncDirId.eq(sync_dir.id)) + .filter( + SyncItemsColumn::LocalPath.eq(path.display().to_string()), + ) + .one(&db), + ) + .unwrap(); + + let db_timestamp: u64 = if let Some(db_sync_item) = maybe_db_sync_item { + db_sync_item.last_local_timestamp.try_into().unwrap() + } else { + should_sync = true; + break; + }; + + if current_timestamp != db_timestamp { + should_sync = true; + break; + } + } else { + should_sync = true; + break; + } + } + } else { + // TODO: We should show the user an error instead of trying to sync again. + should_sync = true; + } + + // Remote file checks. + if let Ok(paths) = rclone::sync::list( + &remote.name, + &sync_dir.remote_path, + true, + RcloneListFilter::All, + ) { + for path in paths { + let stripped_path = match path.name.contains('/') { + true => path + .name + .strip_suffix(&format!("{}/", sync_dir.remote_path)) + .unwrap() + .to_string(), + false => path.name.clone(), + }; + let maybe_db_sync_item = util::await_future( + SyncItemsEntity::find() + .filter(SyncItemsColumn::SyncDirId.eq(sync_dir.id)) + .filter(SyncItemsColumn::RemotePath.eq(stripped_path)) + .one(&db), + ) + .unwrap(); + let db_timestamp: i64 = if let Some(db_sync_item) = maybe_db_sync_item { + db_sync_item.last_remote_timestamp.into() + } else { + should_sync = true; + break; + }; + let remote_timestamp = path.mod_time.unix_timestamp(); + + if remote_timestamp != db_timestamp { + should_sync = true; + break; + } + } + } else { + // TODO: We should show the disconnected icon instead of trying to sync again. + should_sync = true; + } + + // DB file checks. This covers files that got deleted locally or on the remote, + // as those changes wouldn't neccesarily be visible above. + let sync_items = util::await_future( + SyncItemsEntity::find() + .filter(SyncItemsColumn::SyncDirId.eq(sync_dir.id)) + .all(&db), + ) + .unwrap(); + + for sync_item in sync_items { + let remote_path = if !sync_dir.remote_path.is_empty() { + format!("{}/{}", sync_dir.remote_path, sync_item.remote_path) + } else { + sync_item.remote_path.clone() + }; + let maybe_remote_timestamp: Option = + rclone::sync::stat(&remote.name, &remote_path) + .ok() + .flatten() + .map(|remote_item| { + remote_item.mod_time.unix_timestamp().try_into().unwrap() + }); + + // If the path doesn't exist both locally and on the remote, then we need to + // delete the DB entry. + if !Path::new(&sync_item.local_path).exists() + && maybe_remote_timestamp.is_none() + { + util::await_future(async { + SyncItemsEntity::find() + .filter(SyncItemsColumn::Id.eq(sync_item.id)) + .one(&db) + .await + .unwrap() + .unwrap() + .delete(&db) + .await + .unwrap(); + }); + continue; + } + + let remote_timestamp = match maybe_remote_timestamp { + Some(timestamp) => timestamp, + None => { + should_sync = true; + break; + } + }; + + let local_timestamp: i32 = match fs::metadata(&sync_item.local_path) { + Ok(metadata) => metadata + .modified() + .unwrap() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs() + .try_into() + .unwrap(), + Err(_) => { + should_sync = true; + break; + } + }; + + if local_timestamp != sync_item.last_local_timestamp + || remote_timestamp != sync_item.last_remote_timestamp + { + should_sync = true; + break; + } + } + let item_ptr = directory_map.get_ref(); let item = item_ptr .get(&remote.name) @@ -1174,6 +1338,21 @@ pub fn launch(app: &Application, background: bool) { .get(&(sync_dir.local_path.clone(), sync_dir.remote_path.clone())) .unwrap(); + if !should_sync { + item.status_text.set_label(&tr::tr!("Files are synced.")); + item.status_icon + .set_child(Some(&get_image("object-select-symbolic"))); + continue; + } + + // Notify the tray app that we're syncing this remote now. We + // do this here instead for each remote, because the above + // sync dir check might make us not change anything on this remote. + handle.update(|tray| { + tray.set_msg(tr::tr!("Syncing '{}'...", remote.name)); + tray.set_syncing(); + }); + // If we have pending errors that need resolved, don't sync this directory. if item.error_status_text.text().len() != 0 { continue; @@ -1985,6 +2164,7 @@ pub fn launch(app: &Application, background: bool) { item.path.strip_prefix(&sync_dir.remote_path).unwrap() ); update_ui_progress(&remote_path_string); + // If we've already synced this directory from `fn sync_local_directory` // above, don't sync it again. if synced_items @@ -2326,7 +2506,7 @@ pub fn launch(app: &Application, background: bool) { .unwrap(); item.status_icon .set_child(Some(&get_image("object-select-symbolic"))); - let mut finished_text = tr::tr!("Directory has finished sync checks."); + let mut finished_text = tr::tr!("Files are synced."); if item.error_status_text.text().len() != 0 { finished_text += &please_resolve_msg; item.status_icon diff --git a/src/tray.rs b/src/tray.rs index 45b7e1a..824b93f 100644 --- a/src/tray.rs +++ b/src/tray.rs @@ -30,6 +30,10 @@ impl Tray { pub fn set_done(&mut self) { self.icon = "com.hunterwittenborn.Celeste.CelesteTrayDone-symbolic".to_owned(); } + + pub fn set_disconnected(&mut self) { + self.icon = "com.hunterwittenborn.Celeste.CelesteTrayDisconnected-symbolic".to_owned(); + } } impl KsniTray for Tray {