From 4460aae75ae5d69d3e2ac5ed78aede5b27213dc3 Mon Sep 17 00:00:00 2001
From: Brian Picciano <mediocregopher@gmail.com>
Date: Mon, 8 May 2023 17:18:21 +0200
Subject: [PATCH] keep refactoring git store to make this iterator work nice

---
 src/lib.rs          |  2 +-
 src/origin/store.rs | 56 +++++++++++++++++++++++----------------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 1ad5afd..4e6989c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,2 +1,2 @@
-pub mod origin;
 pub mod domain;
+pub mod origin;
diff --git a/src/origin/store.rs b/src/origin/store.rs
index 22189fa..67796ee 100644
--- a/src/origin/store.rs
+++ b/src/origin/store.rs
@@ -1,4 +1,5 @@
 use super::Descr;
+use mockall::automock;
 use std::error::Error;
 
 #[derive(Clone, Copy)]
@@ -54,9 +55,7 @@ impl<E: Error + 'static> From<E> for AllDescrsError {
     }
 }
 
-/// Used in the return from all_descrs from Store.
-type AllDescrsResult<T> = Result<T, AllDescrsError>;
-
+#[automock]
 pub trait Origin {
     fn descr(&self) -> &Descr;
     fn read_file_into(
@@ -66,14 +65,19 @@ pub trait Origin {
     ) -> Result<(), ReadFileIntoError>;
 }
 
+/// Used in the return from all_descrs from Store.
+pub type AllDescrsResult<T> = Result<T, AllDescrsError>;
+
 /// Describes a storage mechanism for Origins. Each Origin is uniquely identified by its Descr.
-pub trait Store {
+pub trait Store<'a> {
+    type AllDescrsIter: IntoIterator<Item = AllDescrsResult<Descr>>;
+
     /// If the origin is of a kind which can be updated, sync will pull down the latest version of
     /// the origin into the storage.
-    fn sync(&self, descr: &Descr, limits: Limits) -> Result<(), SyncError>;
+    fn sync(&'a self, descr: &Descr, limits: Limits) -> Result<(), SyncError>;
 
-    fn get(&self, descr: Descr) -> Result<Box<dyn Origin>, GetError>;
-    fn all_descrs(&self) -> AllDescrsResult<Box<dyn Iterator<Item = AllDescrsResult<Descr>> + '_>>;
+    fn get(&'a self, descr: Descr) -> Result<Box<dyn Origin>, GetError>;
+    fn all_descrs(&'a self) -> AllDescrsResult<Self::AllDescrsIter>;
 }
 
 pub mod git {
@@ -117,13 +121,13 @@ pub mod git {
 
     /// git::Store implements the Store trait for any Descr::Git based Origins. If any non-git
     /// Descrs are used then this implementation will panic.
-    pub struct Store<'a> {
-        dir_path: &'a Path,
+    pub struct Store {
+        dir_path: PathBuf,
     }
 
-    impl<'a> Store<'a> {
-        pub fn new(dir_path: &Path) -> io::Result<Store> {
-            fs::create_dir_all(dir_path)?;
+    impl Store {
+        pub fn new(dir_path: PathBuf) -> io::Result<Store> {
+            fs::create_dir_all(&dir_path)?;
             Ok(Store { dir_path })
         }
 
@@ -135,13 +139,15 @@ pub mod git {
             self.dir_path.join(descr_id).join("descr.json")
         }
 
-        fn branch_ref(branch_name: &str) -> String {
+        fn branch_ref(&self, branch_name: &str) -> String {
             format!("origin/{branch_name}")
         }
     }
 
-    impl<'a> super::Store for Store<'a> {
-        fn sync(&self, descr: &Descr, _limits: Limits) -> Result<(), SyncError> {
+    impl<'a> super::Store<'a> for Store {
+        type AllDescrsIter = Box<dyn Iterator<Item = AllDescrsResult<Descr>> + 'a>;
+
+        fn sync(&'a self, descr: &Descr, _limits: Limits) -> Result<(), SyncError> {
             use gix::clone::Error as gixCloneErr;
             use gix::progress::Discard;
 
@@ -170,7 +176,7 @@ pub mod git {
 
                 // Check to make sure the branch name exists
                 // TODO if this fails we should delete repo_path
-                repo.try_find_reference(&Self::branch_ref(branch_name))?
+                repo.try_find_reference(&self.branch_ref(branch_name))?
                     .ok_or(SyncError::InvalidBranchName)?;
 
                 // Add the descr to the repo directory, so we can know the actual descr later
@@ -198,7 +204,7 @@ pub mod git {
             Ok(())
         }
 
-        fn get(&self, descr: Descr) -> Result<Box<dyn super::Origin>, GetError> {
+        fn get(&'a self, descr: Descr) -> Result<Box<dyn super::Origin>, GetError> {
             let repo_path = self.repo_path(&descr);
             let Descr::Git {
                 ref branch_name, ..
@@ -212,7 +218,7 @@ pub mod git {
             let repo = gix::open(&repo_path)?;
 
             let commit_object_id = repo
-                .find_reference(&Self::branch_ref(branch_name))?
+                .find_reference(&self.branch_ref(branch_name))?
                 .peel_to_id_in_place()?
                 .detach();
 
@@ -228,11 +234,9 @@ pub mod git {
             }));
         }
 
-        fn all_descrs(
-            &self,
-        ) -> AllDescrsResult<Box<dyn Iterator<Item = AllDescrsResult<Descr>> + '_>> {
-            let inner_iter = fs::read_dir(self.dir_path)?.map(
-                |dir_entry_res: io::Result<fs::DirEntry>| -> Result<Descr, AllDescrsError> {
+        fn all_descrs(&'a self) -> AllDescrsResult<Self::AllDescrsIter> {
+            Ok(Box::from(fs::read_dir(&self.dir_path)?.map(
+                |dir_entry_res: io::Result<fs::DirEntry>| -> AllDescrsResult<Descr> {
                     let descr_id: String = dir_entry_res?
                         .file_name()
                         .to_str()
@@ -251,9 +255,7 @@ pub mod git {
 
                     Ok(descr)
                 },
-            );
-
-            Ok(Box::from(inner_iter))
+            )))
         }
     }
 
@@ -281,7 +283,7 @@ pub mod git {
 
             let limits = super::Limits {};
 
-            let store = super::Store::new(tmp_dir.path()).expect("store created");
+            let store = super::Store::new(tmp_dir.path().to_path_buf()).expect("store created");
 
             store.sync(&descr, limits).expect("sync should succeed");
             store