cob: Fix recursion bug in TryFrom of Reference

This commit addresses the issue of recursion in the TryFrom
call of Reference by resolving the parent in a lazy way.

Due to constraints posed by the need to access `git2::Repository`
or `git2::Commit` to resolve the parent, this commit decides to
pull out the parent resolution logic from the `cob::object::Commit`
module and move it inside the `cob::store::Store` implementation by adding
a new method `parents_of`.

Suggested-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Tested-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

radicle-cob/src/backend/git/change.rs

@@ -139,6 +139,14 @@ impl change::Storage for git2::Repository {
         })
     }
 
+    fn parents_of(&self, id: &Oid) -> Result<Vec<Oid>, Self::LoadError> {
+        Ok(self
+            .find_commit(**id)?
+            .parent_ids()
+            .map(Oid::from)
+            .collect::<Vec<_>>())
+    }
+
     fn load(&self, id: Self::ObjectId) -> Result<Change, Self::LoadError> {
         let commit = Commit::read(self, id.into())?;
         let timestamp = git2::Time::from(commit.committer().time).seconds() as u64;

radicle-cob/src/change/store.rs

@@ -3,6 +3,7 @@
 use std::{error::Error, fmt};
 
 use nonempty::NonEmpty;
+use radicle_git_ext::Oid;
 use serde::{Deserialize, Serialize};
 
 use crate::{
@@ -37,6 +38,9 @@ pub trait Storage {
         &self,
         id: Self::ObjectId,
     ) -> Result<Change<Self::Parent, Self::ObjectId, Self::Signatures>, Self::LoadError>;
+
+    /// Returns the parents of the object with the specified ID.
+    fn parents_of(&self, id: &Oid) -> Result<Vec<Oid>, Self::LoadError>;
 }
 
 /// Change template, used to create a new change.

radicle-cob/src/change_graph.rs

@@ -32,15 +32,16 @@ impl ChangeGraph {
     {
         log::info!("loading object '{}' '{}'", typename, oid);
         let mut builder = GraphBuilder::default();
-        let mut edges_to_process: Vec<(object::Commit, Oid)> = Vec::new();
+        let mut edges_to_process: Vec<(Oid, Oid)> = Vec::new();
 
         // Populate the initial set of edges_to_process from the refs we have
         for reference in tip_refs {
             log::trace!("loading object from reference '{}'", reference.name);
             match storage.load(reference.target.id) {
                 Ok(change) => {
-                    let commit = reference.target.clone();
-                    let new_edges = builder.add_change(commit, change);
+                    let new_edges = builder
+                        .add_change(storage, reference.target.id, change)
+                        .ok()?;
                     edges_to_process.extend(new_edges);
                 }
                 Err(e) => {
@@ -55,23 +56,22 @@ impl ChangeGraph {
         }
 
         // Process edges until we have no more to process
-        while let Some((parent_commit, child_commit_id)) = edges_to_process.pop() {
+        while let Some((parent_commit_id, child_commit_id)) = edges_to_process.pop() {
             log::trace!(
                 "loading change parent='{}', child='{}'",
-                parent_commit.id,
+                parent_commit_id,
                 child_commit_id
             );
-            match storage.load(parent_commit.id) {
+            match storage.load(parent_commit_id) {
                 Ok(change) => {
-                    let parent_commit_id = parent_commit.id;
-                    let new_edges = builder.add_change(parent_commit, change);
+                    let new_edges = builder.add_change(storage, parent_commit_id, change).ok()?;
                     edges_to_process.extend(new_edges);
                     builder.add_edge(child_commit_id, parent_commit_id);
                 }
                 Err(e) => {
                     log::warn!(
                         "unable to load changetree from commit '{}', error '{}'",
-                        parent_commit.id,
+                        parent_commit_id,
                         e
                     );
                 }
@@ -122,24 +122,32 @@ impl Default for GraphBuilder {
 impl GraphBuilder {
     /// Add a change to the graph which we are building up, returning any edges
     /// corresponding to the parents of this node in the change graph
-    fn add_change(
+    fn add_change<S>(
         &mut self,
-        commit: object::Commit,
+        storage: &S,
+        commit_id: Oid,
         change: Change,
-    ) -> impl Iterator<Item = (object::Commit, Oid)> + '_ {
+    ) -> Result<Vec<(Oid, Oid)>, S::LoadError>
+    where
+        S: change::Storage<ObjectId = Oid, Parent = Oid, Signatures = ExtendedSignature>,
+    {
         let resource_commit = *change.resource();
-        let commit_id = commit.id;
 
         if !self.graph.contains(&commit_id) {
             self.graph.node(commit_id, change);
         }
-        commit.parents.into_iter().filter_map(move |parent| {
-            if parent.id != resource_commit && !self.graph.has_dependency(&commit_id, &parent.id) {
-                Some((parent, commit_id))
-            } else {
-                None
-            }
-        })
+
+        Ok(storage
+            .parents_of(&commit_id)?
+            .into_iter()
+            .filter_map(move |parent| {
+                if parent != resource_commit && !self.graph.has_dependency(&commit_id, &parent) {
+                    Some((parent, commit_id))
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<(Oid, Oid)>>())
     }
 
     fn add_edge(&mut self, child: Oid, parent: Oid) {

radicle-cob/src/object/storage.rs

@@ -50,8 +50,6 @@ pub struct Reference {
 pub struct Commit {
     /// The content identifier of the commit.
     pub id: Oid,
-    /// The parents of the commit.
-    pub parents: Vec<Commit>,
 }
 
 pub trait Storage {
@@ -129,10 +127,8 @@ pub mod convert {
 
     impl<'a> From<git2::Commit<'a>> for Commit {
         fn from(commit: git2::Commit<'a>) -> Self {
-            let parents = commit.parents().map(Commit::from).collect();
             Commit {
                 id: commit.id().into(),
-                parents,
             }
         }
     }

radicle-cob/src/test/storage.rs

@@ -91,6 +91,15 @@ impl change::Storage for Storage {
     > {
         self.as_raw().load(id)
     }
+
+    fn parents_of(&self, id: &git_ext::Oid) -> Result<Vec<git_ext::Oid>, Self::LoadError> {
+        Ok(self
+            .as_raw()
+            .find_commit(**id)?
+            .parent_ids()
+            .map(git_ext::Oid::from)
+            .collect::<Vec<_>>())
+    }
 }
 
 impl object::Storage for Storage {

radicle/src/storage/git/cob.rs

@@ -60,6 +60,10 @@ impl change::Storage for Repository {
     fn load(&self, id: Self::ObjectId) -> Result<cob::Change, Self::LoadError> {
         self.backend.load(id)
     }
+
+    fn parents_of(&self, id: &Oid) -> Result<Vec<Oid>, Self::LoadError> {
+        self.backend.parents_of(id)
+    }
 }
 
 impl cob::object::Storage for Repository {