Consistent naming of local trait implementations (#300)

Call them LocalBlacklist, etc., in contrast to possible remote
implementations.

Use dynamic dispatch in SchedulerData instead of implementations. No
significant performance degradation observed in actual testing.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "trane"
-version = "0.19.2"
+version = "0.20.0"
 edition = "2021"
 description = "An automated system for learning complex skills"
 license = "AGPL-3.0"

src/blacklist.rs

@@ -37,15 +37,15 @@ pub trait Blacklist {
 }
 
 /// An implementation of [Blacklist] backed by `SQLite`.
-pub struct BlacklistDB {
+pub struct LocalBlacklist {
     /// A cache of the blacklist entries used to avoid unnecessary queries to the database.
     cache: RwLock<UstrMap<bool>>,
 
     /// A pool of connections to the database.
     pool: Pool<SqliteConnectionManager>,
 }
 
-impl BlacklistDB {
+impl LocalBlacklist {
     /// Returns all the migrations needed to set up the database.
     fn migrations() -> Migrations<'static> {
         Migrations::new(vec![
@@ -68,10 +68,10 @@ impl BlacklistDB {
     }
 
     /// A constructor taking a connection manager.
-    fn new(connection_manager: SqliteConnectionManager) -> Result<BlacklistDB> {
+    fn new(connection_manager: SqliteConnectionManager) -> Result<LocalBlacklist> {
         // Initialize the pool and the blacklist database.
         let pool = Pool::new(connection_manager)?;
-        let mut blacklist = BlacklistDB {
+        let mut blacklist = LocalBlacklist {
             cache: RwLock::new(UstrMap::default()),
             pool,
         };
@@ -86,7 +86,7 @@ impl BlacklistDB {
     }
 
     /// A constructor taking the path to the database file.
-    pub fn new_from_disk(db_path: &str) -> Result<BlacklistDB> {
+    pub fn new_from_disk(db_path: &str) -> Result<LocalBlacklist> {
         Self::new(db_utils::new_connection_manager(db_path))
     }
 
@@ -175,7 +175,7 @@ impl BlacklistDB {
     }
 }
 
-impl Blacklist for BlacklistDB {
+impl Blacklist for LocalBlacklist {
     fn add_to_blacklist(&mut self, unit_id: Ustr) -> Result<(), BlacklistError> {
         self.add_to_blacklist_helper(unit_id)
             .map_err(|e| BlacklistError::AddUnit(unit_id, e))
@@ -209,11 +209,11 @@ mod test {
     use tempfile::tempdir;
     use ustr::Ustr;
 
-    use crate::blacklist::{Blacklist, BlacklistDB};
+    use crate::blacklist::{Blacklist, LocalBlacklist};
 
     fn new_test_blacklist() -> Result<Box<dyn Blacklist>> {
         let connection_manager = SqliteConnectionManager::memory();
-        let blacklist = BlacklistDB::new(connection_manager)?;
+        let blacklist = LocalBlacklist::new(connection_manager)?;
         Ok(Box::new(blacklist))
     }
 
@@ -315,13 +315,13 @@ mod test {
     fn reopen_blacklist() -> Result<()> {
         let dir = tempdir()?;
         let mut blacklist =
-            BlacklistDB::new_from_disk(dir.path().join("blacklist.db").to_str().unwrap())?;
+            LocalBlacklist::new_from_disk(dir.path().join("blacklist.db").to_str().unwrap())?;
         let unit_id = Ustr::from("unit_id");
         blacklist.add_to_blacklist(unit_id)?;
         assert!(blacklist.blacklisted(unit_id)?);
 
         let new_blacklist =
-            BlacklistDB::new_from_disk(dir.path().join("blacklist.db").to_str().unwrap())?;
+            LocalBlacklist::new_from_disk(dir.path().join("blacklist.db").to_str().unwrap())?;
         assert!(new_blacklist.blacklisted(unit_id)?);
         Ok(())
     }

src/ffi.rs

@@ -30,10 +30,10 @@ pub struct TraneFFI {
 }
 
 impl TraneFFI {
-    /// Creates and wraps a new instance of Trane for use with FFI.
-    pub fn new(working_dir: &Path, library_root: &Path) -> Result<TraneFFI> {
+    /// Creates and wraps a new local instance of Trane for use with FFI.
+    pub fn new_local(working_dir: &Path, library_root: &Path) -> Result<TraneFFI> {
         Ok(TraneFFI {
-            trane: Trane::new(working_dir, library_root)?,
+            trane: Trane::new_local(working_dir, library_root)?,
         })
     }
 }

src/lib.rs

@@ -65,13 +65,13 @@ pub mod testutil;
 use anyhow::Result;
 use error::*;
 use parking_lot::RwLock;
-use review_list::{ReviewList, ReviewListDB};
+use review_list::{LocalReviewList, ReviewList};
 use std::{path::Path, sync::Arc};
 use study_session_manager::{LocalStudySessionManager, StudySessionManager};
 use ustr::{Ustr, UstrMap, UstrSet};
 
 use crate::mantra_miner::TraneMantraMiner;
-use blacklist::{Blacklist, BlacklistDB};
+use blacklist::{Blacklist, LocalBlacklist};
 use course_library::{CourseLibrary, GetUnitGraph, LocalCourseLibrary};
 use data::{
     filter::{ExerciseFilter, SavedFilter},
@@ -80,7 +80,7 @@ use data::{
 };
 use filter_manager::{FilterManager, LocalFilterManager};
 use graph::UnitGraph;
-use practice_stats::{PracticeStats, PracticeStatsDB};
+use practice_stats::{LocalPracticeStats, PracticeStats};
 use repository_manager::{LocalRepositoryManager, RepositoryManager};
 use scheduler::{data::SchedulerData, DepthFirstScheduler, ExerciseScheduler};
 
@@ -169,10 +169,10 @@ impl Trane {
         options
     }
 
-    /// Creates a new instance of the library given the path to the root of a course library. The
-    /// user data will be stored in a directory named `.trane` inside the library root directory.
-    /// The working directory will be used to resolve relative paths.
-    pub fn new(working_dir: &Path, library_root: &Path) -> Result<Trane> {
+    /// Creates a new local instance of the Trane given the path to the root of a course library.
+    /// The user data will be stored in a directory named `.trane` inside the library root
+    /// directory. The working directory will be used to resolve relative paths.
+    pub fn new_local(working_dir: &Path, library_root: &Path) -> Result<Trane> {
         let config_path = library_root.join(Path::new(TRANE_CONFIG_DIR_PATH));
 
         // The course library must be created first because it makes sure to initialize all the
@@ -183,13 +183,13 @@ impl Trane {
 
         // Build all the components needed to create a Trane instance.
         let unit_graph = course_library.write().get_unit_graph();
-        let practice_stats = Arc::new(RwLock::new(PracticeStatsDB::new_from_disk(
+        let practice_stats = Arc::new(RwLock::new(LocalPracticeStats::new_from_disk(
             config_path.join(PRACTICE_STATS_PATH).to_str().unwrap(),
         )?));
-        let blacklist = Arc::new(RwLock::new(BlacklistDB::new_from_disk(
+        let blacklist = Arc::new(RwLock::new(LocalBlacklist::new_from_disk(
             config_path.join(BLACKLIST_PATH).to_str().unwrap(),
         )?));
-        let review_list = Arc::new(RwLock::new(ReviewListDB::new_from_disk(
+        let review_list = Arc::new(RwLock::new(LocalReviewList::new_from_disk(
             config_path.join(REVIEW_LIST_PATH).to_str().unwrap(),
         )?));
         let filter_manager = Arc::new(RwLock::new(LocalFilterManager::new(
@@ -561,7 +561,7 @@ mod test {
     #[test]
     fn library_root() -> Result<()> {
         let dir = tempfile::tempdir()?;
-        let trane = Trane::new(dir.path(), dir.path())?;
+        let trane = Trane::new_local(dir.path(), dir.path())?;
         assert_eq!(trane.library_root(), dir.path().to_str().unwrap());
         Ok(())
     }
@@ -570,7 +570,7 @@ mod test {
     #[test]
     fn mantra_count() -> Result<()> {
         let dir = tempfile::tempdir()?;
-        let trane = Trane::new(dir.path(), dir.path())?;
+        let trane = Trane::new_local(dir.path(), dir.path())?;
         thread::sleep(Duration::from_millis(1000));
         assert!(trane.mantra_count() > 0);
         Ok(())
@@ -583,7 +583,7 @@ mod test {
         let dir = tempfile::tempdir()?;
         let trane_path = dir.path().join(".trane");
         File::create(&trane_path)?;
-        assert!(Trane::new(dir.path(), dir.path()).is_err());
+        assert!(Trane::new_local(dir.path(), dir.path()).is_err());
         Ok(())
     }
 
@@ -592,7 +592,7 @@ mod test {
     fn bad_dir_permissions() -> Result<()> {
         let dir = tempfile::tempdir()?;
         set_permissions(&dir, Permissions::from_mode(0o000))?;
-        assert!(Trane::new(dir.path(), dir.path()).is_err());
+        assert!(Trane::new_local(dir.path(), dir.path()).is_err());
         Ok(())
     }
 
@@ -603,15 +603,15 @@ mod test {
         let config_dir_path = dir.path().join(".trane");
         create_dir(&config_dir_path)?;
         set_permissions(&config_dir_path, Permissions::from_mode(0o000))?;
-        assert!(Trane::new(dir.path(), dir.path()).is_err());
+        assert!(Trane::new_local(dir.path(), dir.path()).is_err());
         Ok(())
     }
 
     /// Verifies retrieving the scheduler data from the library.
     #[test]
     fn scheduler_data() -> Result<()> {
         let dir = tempfile::tempdir()?;
-        let trane = Trane::new(dir.path(), dir.path())?;
+        let trane = Trane::new_local(dir.path(), dir.path())?;
         trane.get_scheduler_data();
         Ok(())
     }

src/practice_stats.rs

@@ -47,12 +47,12 @@ pub trait PracticeStats {
 }
 
 /// An implementation of [`PracticeStats`] backed by `SQLite`.
-pub struct PracticeStatsDB {
+pub struct LocalPracticeStats {
     /// A pool of connections to the database.
     pool: Pool<SqliteConnectionManager>,
 }
 
-impl PracticeStatsDB {
+impl LocalPracticeStats {
     /// Returns all the migrations needed to set up the database.
     fn migrations() -> Migrations<'static> {
         Migrations::new(vec![
@@ -98,16 +98,16 @@ impl PracticeStatsDB {
     }
 
     /// A constructor taking a `SQLite` connection manager.
-    fn new(connection_manager: SqliteConnectionManager) -> Result<PracticeStatsDB> {
+    fn new(connection_manager: SqliteConnectionManager) -> Result<LocalPracticeStats> {
         // Create a connection pool and initialize the database.
         let pool = Pool::new(connection_manager)?;
-        let mut stats = PracticeStatsDB { pool };
+        let mut stats = LocalPracticeStats { pool };
         stats.init()?;
         Ok(stats)
     }
 
     /// A constructor taking the path to a database file.
-    pub fn new_from_disk(db_path: &str) -> Result<PracticeStatsDB> {
+    pub fn new_from_disk(db_path: &str) -> Result<LocalPracticeStats> {
         Self::new(db_utils::new_connection_manager(db_path))
     }
 
@@ -213,7 +213,7 @@ impl PracticeStatsDB {
     }
 }
 
-impl PracticeStats for PracticeStatsDB {
+impl PracticeStats for LocalPracticeStats {
     fn get_scores(
         &self,
         exercise_id: Ustr,
@@ -252,12 +252,12 @@ mod test {
 
     use crate::{
         data::{ExerciseTrial, MasteryScore},
-        practice_stats::{PracticeStats, PracticeStatsDB},
+        practice_stats::{LocalPracticeStats, PracticeStats},
     };
 
     fn new_tests_stats() -> Result<Box<dyn PracticeStats>> {
         let connection_manager = SqliteConnectionManager::memory();
-        let practice_stats = PracticeStatsDB::new(connection_manager)?;
+        let practice_stats = LocalPracticeStats::new(connection_manager)?;
         Ok(Box::new(practice_stats))
     }
 

src/review_list.rs

@@ -27,12 +27,12 @@ pub trait ReviewList {
 }
 
 /// An implementation of [`ReviewList`] backed by `SQLite`.
-pub struct ReviewListDB {
+pub struct LocalReviewList {
     /// A pool of connections to the database.
     pool: Pool<SqliteConnectionManager>,
 }
 
-impl ReviewListDB {
+impl LocalReviewList {
     /// Returns all the migrations needed to set up the database.
     fn migrations() -> Migrations<'static> {
         Migrations::new(vec![
@@ -56,16 +56,16 @@ impl ReviewListDB {
     }
 
     /// A constructor taking a connection manager.
-    fn new(connection_manager: SqliteConnectionManager) -> Result<ReviewListDB> {
+    fn new(connection_manager: SqliteConnectionManager) -> Result<LocalReviewList> {
         // Initialize the pool and the review list database.
         let pool = Pool::new(connection_manager)?;
-        let mut review_list = ReviewListDB { pool };
+        let mut review_list = LocalReviewList { pool };
         review_list.init()?;
         Ok(review_list)
     }
 
     /// A constructor taking the path to the database file.
-    pub fn new_from_disk(db_path: &str) -> Result<ReviewListDB> {
+    pub fn new_from_disk(db_path: &str) -> Result<LocalReviewList> {
         let connection_manager = SqliteConnectionManager::file(db_path).with_init(
             |connection: &mut Connection| -> Result<(), rusqlite::Error> {
                 // The following pragma statements are set to improve the read and write performance
@@ -114,7 +114,7 @@ impl ReviewListDB {
     }
 }
 
-impl ReviewList for ReviewListDB {
+impl ReviewList for LocalReviewList {
     fn add_to_review_list(&mut self, unit_id: Ustr) -> Result<(), ReviewListError> {
         self.add_to_review_list_helper(unit_id)
             .map_err(|e| ReviewListError::AddUnit(unit_id, e))
@@ -137,11 +137,11 @@ mod test {
     use r2d2_sqlite::SqliteConnectionManager;
     use ustr::Ustr;
 
-    use crate::review_list::{ReviewList, ReviewListDB};
+    use crate::review_list::{LocalReviewList, ReviewList};
 
     fn new_test_review_list() -> Result<Box<dyn ReviewList>> {
         let connection_manager = SqliteConnectionManager::memory();
-        let review_list = ReviewListDB::new(connection_manager)?;
+        let review_list = LocalReviewList::new(connection_manager)?;
         Ok(Box::new(review_list))
     }
 

src/scheduler.rs

@@ -35,9 +35,6 @@ use crate::{
         ExerciseManifest, MasteryScore, SchedulerOptions, UnitType,
     },
     error::ExerciseSchedulerError,
-    graph::UnitGraph,
-    practice_stats::PracticeStats,
-    review_list::ReviewList,
     scheduler::{cache::ScoreCache, data::SchedulerData, filter::CandidateFilter},
 };
 

src/scheduler/cache.rs

@@ -11,10 +11,7 @@ use std::cell::RefCell;
 use ustr::{Ustr, UstrMap, UstrSet};
 
 use crate::{
-    blacklist::Blacklist,
     data::{SchedulerOptions, UnitType},
-    graph::UnitGraph,
-    practice_stats::PracticeStats,
     scheduler::SchedulerData,
     scorer::{ExerciseScorer, SimpleScorer},
 };