feat(core): add deletion support and limit checks

src/command/describe.rs

@@ -2,9 +2,11 @@ use crate::command::Command;
 use crate::domain::hash::Hash;
 use crate::domain::object::Object;
 use crate::error::VctrlError;
-use crate::storage::traits::{ObjectStore, RefStore};
+use crate::storage::traits::{ObjectStore, ObjectStoreExt, RefStore};
 use std::collections::{HashSet, VecDeque};
 
+const MAX_DESCRIBE_SEARCH: usize = 100_000;
+
 pub struct Describe {
     pub commit_hash: Hash,
     pub max_commits_to_search: usize,
@@ -18,6 +20,15 @@ impl Command for Describe {
         store: &mut dyn ObjectStore,
         refs: &mut dyn RefStore,
     ) -> Result<Option<String>, VctrlError> {
+        if self.max_commits_to_search > MAX_DESCRIBE_SEARCH {
+            return Err(VctrlError::Other(format!(
+                "max_commits_to_search must be <= {}",
+                MAX_DESCRIBE_SEARCH
+            )));
+        }
+
+        store.get_commit(&self.commit_hash)?;
+
         let tag_refs = refs.list_refs("refs/tags/")?;
         let mut tag_map: Vec<(Hash, String)> = Vec::new();
         for ref_name in &tag_refs {

src/command/log_graph.rs

@@ -19,8 +19,6 @@ pub struct GraphCommit {
 
 pub struct LogGraph {
     pub head: Hash,
-    pub encoder: Box<dyn crate::codec::Encoder>,
-    pub hasher: Box<dyn crate::hashing::Hasher>,
 }
 
 impl Command for LogGraph {
@@ -32,30 +30,24 @@ impl Command for LogGraph {
         _refs: &mut dyn RefStore,
     ) -> Result<Vec<GraphCommit>, VctrlError> {
         let walk = RevWalk::new(store, &[self.head])?;
-        let commits: Vec<Commit> = walk.collect::<Result<Vec<_>, _>>()?;
+        let commit_pairs: Vec<(Hash, Commit)> = walk.collect::<Result<Vec<_>, _>>()?;
 
-        let mut hashes = Vec::with_capacity(commits.len());
-        for c in &commits {
-            let mut buf = Vec::new();
-            self.encoder.encode_commit(c, &mut buf)?;
-            let hash = self.hasher.hash_commit_encoded(&buf);
-            hashes.push(hash);
-        }
-
-        let hash_to_idx: HashMap<Hash, usize> =
-            hashes.iter().enumerate().map(|(i, h)| (*h, i)).collect();
+        let hash_to_idx: HashMap<Hash, usize> = commit_pairs
+            .iter()
+            .enumerate()
+            .map(|(i, (hash, _))| (*hash, i))
+            .collect();
 
-        let graph_commits = commits
+        let graph_commits = commit_pairs
             .into_iter()
-            .enumerate()
-            .map(|(i, c)| {
+            .map(|(hash, c)| {
                 let parent_indices = c
                     .parents
                     .iter()
                     .filter_map(|p| hash_to_idx.get(p).copied())
                     .collect();
                 GraphCommit {
-                    hash: hashes[i],
+                    hash,
                     message: c.message,
                     author: c.author,
                     timestamp: c.timestamp,

src/command/octopus_merge.rs

@@ -32,6 +32,14 @@ impl Command for OctopusMerge {
             ));
         }
 
+        let total_parents = 1 + self.branch_names.len();
+        if total_parents > 255 {
+            return Err(VctrlError::Other(format!(
+                "too many parents: {} (max 255)",
+                total_parents
+            )));
+        }
+
         let head_hash = refs
             .head()?
             .ok_or_else(|| VctrlError::Other("no HEAD".into()))?;

src/command/rebase.rs

@@ -10,6 +10,8 @@ use crate::merge::{ConflictResolver, ThreeWayMerge};
 use crate::storage::traits::{ObjectStore, ObjectStoreExt, RefStore};
 use std::collections::HashSet;
 
+const MAX_REBASE_COMMITS: usize = 10_000;
+
 pub struct Rebase {
     pub upstream: Hash,
     pub onto: Hash,
@@ -46,6 +48,13 @@ impl Command for Rebase {
             let commit = store.get_commit(&h)?;
             current = commit.parents.first().copied();
             to_rebase.push(commit);
+
+            if to_rebase.len() > MAX_REBASE_COMMITS {
+                return Err(VctrlError::Other(format!(
+                    "too many commits to rebase (limit {})",
+                    MAX_REBASE_COMMITS
+                )));
+            }
         }
         to_rebase.reverse();
 

src/command/stash.rs

@@ -38,9 +38,10 @@ impl Command for StashPush {
         let hash = self.hasher.hash_commit_encoded(&buf);
         store.put(&hash, &Object::Commit(Box::new(commit)))?;
 
-        let existing = refs.list_refs("refs/stash/")?;
-        let next_index = existing.len();
-        let ref_name = format!("refs/stash/{}", next_index);
+        let nanos = chrono::Utc::now()
+            .timestamp_nanos_opt()
+            .ok_or_else(|| VctrlError::Other("invalid system clock".into()))?;
+        let ref_name = format!("refs/stash/{}", nanos);
         refs.set_ref(&ref_name, &hash)?;
         Ok(hash)
     }
@@ -56,20 +57,15 @@ impl Command for StashPop {
         store: &mut dyn ObjectStore,
         refs: &mut dyn RefStore,
     ) -> Result<Option<Hash>, VctrlError> {
-        let stash_refs = refs.list_refs("refs/stash/")?;
+        let mut stash_refs = refs.list_refs("refs/stash/")?;
         if stash_refs.is_empty() {
             return Ok(None);
         }
-        let mut indices: Vec<usize> = stash_refs
-            .iter()
-            .filter_map(|r| r.trim_start_matches("refs/stash/").parse::<usize>().ok())
-            .collect();
-        indices.sort();
-        let last = indices.last().unwrap();
-        let ref_name = format!("refs/stash/{}", last);
-        let commit_hash = refs.get_ref(&ref_name)?.unwrap();
+        stash_refs.sort();
+        let last = stash_refs.last().unwrap().clone();
+        let commit_hash = refs.get_ref(&last)?.unwrap();
         let commit = store.get_commit(&commit_hash)?;
-        refs.delete_ref(&ref_name)?;
+        refs.delete_ref(&last)?;
         Ok(Some(commit.tree))
     }
 }

src/revwalk.rs

@@ -59,7 +59,7 @@ impl<'a> RevWalk<'a> {
 }
 
 impl<'a> Iterator for RevWalk<'a> {
-    type Item = Result<Commit, VctrlError>;
+    type Item = Result<(Hash, Commit), VctrlError>;
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
@@ -72,7 +72,7 @@ impl<'a> Iterator for RevWalk<'a> {
                             return Some(Err(e));
                         }
                     }
-                    return Some(Ok(*commit));
+                    return Some(Ok((hash, *commit)));
                 }
                 Ok(_) => continue,
                 Err(e) => return Some(Err(e)),

src/storage/file_store.rs

@@ -5,7 +5,7 @@ use crate::domain::object::Object;
 use crate::error::VctrlError;
 use crate::storage::traits::{ObjectStore, RefStore};
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::fs::{File, OpenOptions};
 use std::io::{self, BufReader, BufWriter, Read, Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
@@ -20,6 +20,7 @@ const REC_TAG: u8 = 0x04;
 const REC_SET_REF: u8 = 0x10;
 const REC_DEL_REF: u8 = 0x11;
 const REC_SET_HEAD: u8 = 0x12;
+const REC_DEL_OBJECT: u8 = 0x20;
 
 struct ObjectInfo {
     rec_type: u8,
@@ -32,6 +33,7 @@ pub struct FileStore {
     objects: HashMap<Hash, ObjectInfo>,
     refs: HashMap<String, Hash>,
     head: Option<String>,
+    deleted: HashSet<Hash>,
     encoder: BinaryEncoder,
     decoder: BinaryDecoder,
     writer: Option<BufWriter<File>>,
@@ -45,6 +47,7 @@ impl FileStore {
             objects: HashMap::new(),
             refs: HashMap::new(),
             head: None,
+            deleted: HashSet::new(),
             encoder: BinaryEncoder,
             decoder: BinaryDecoder,
             writer: None,
@@ -77,7 +80,9 @@ impl FileStore {
                 .map_err(VctrlError::Io)?;
             self.writer = Some(BufWriter::new(file));
         }
-        Ok(self.writer.as_mut().unwrap())
+        self.writer
+            .as_mut()
+            .ok_or_else(|| VctrlError::Backend("no writer".into()))
     }
 
     fn load(&mut self) -> Result<(), VctrlError> {
@@ -96,6 +101,7 @@ impl FileStore {
                 version
             )));
         }
+        let mut deleted_hashes = HashSet::new();
         loop {
             let rec_type = match file.read_u8() {
                 Ok(t) => t,
@@ -111,14 +117,16 @@ impl FileStore {
                     let offset = file.stream_position().map_err(VctrlError::Io)?;
                     file.seek(SeekFrom::Current(length as i64))
                         .map_err(VctrlError::Io)?;
-                    self.objects.insert(
-                        hash,
-                        ObjectInfo {
-                            rec_type,
-                            offset,
-                            length,
-                        },
-                    );
+                    if !deleted_hashes.contains(&hash) {
+                        self.objects.insert(
+                            hash,
+                            ObjectInfo {
+                                rec_type,
+                                offset,
+                                length,
+                            },
+                        );
+                    }
                 }
                 REC_SET_REF => {
                     let name_len = file.read_u16::<BigEndian>().map_err(VctrlError::Io)?;
@@ -146,6 +154,11 @@ impl FileStore {
                         String::from_utf8(target).map_err(|e| VctrlError::Other(e.to_string()))?;
                     self.head = Some(target);
                 }
+                REC_DEL_OBJECT => {
+                    let mut h = [0u8; 64];
+                    file.read_exact(&mut h).map_err(VctrlError::Io)?;
+                    deleted_hashes.insert(Hash::from_bytes(h));
+                }
                 _ => {
                     return Err(VctrlError::Other(format!(
                         "unknown record type {}",
@@ -154,6 +167,10 @@ impl FileStore {
                 }
             }
         }
+        for hash in &deleted_hashes {
+            self.objects.remove(hash);
+        }
+        self.deleted = deleted_hashes;
         Ok(())
     }
 
@@ -203,6 +220,7 @@ impl ObjectStore for FileStore {
         if self.objects.contains_key(hash) {
             return Ok(());
         }
+        self.deleted.remove(hash);
         let (rec_type, data) = self.encode_object(obj)?;
         let writer = self.ensure_writer()?;
         writer.write_u8(rec_type).map_err(VctrlError::Io)?;
@@ -226,6 +244,9 @@ impl ObjectStore for FileStore {
     }
 
     fn get(&self, hash: &Hash) -> Result<Option<Object>, VctrlError> {
+        if self.deleted.contains(hash) {
+            return Ok(None);
+        }
         match self.objects.get(hash) {
             Some(info) => {
                 let mut file = File::open(&self.path).map_err(VctrlError::Io)?;
@@ -241,13 +262,25 @@ impl ObjectStore for FileStore {
     }
 
     fn exists(&self, hash: &Hash) -> Result<bool, VctrlError> {
-        Ok(self.objects.contains_key(hash))
+        Ok(self.objects.contains_key(hash) && !self.deleted.contains(hash))
     }
+
     fn all_hashes(&self) -> Result<Vec<Hash>, VctrlError> {
-        Ok(self.objects.keys().copied().collect())
+        Ok(self
+            .objects
+            .keys()
+            .filter(|h| !self.deleted.contains(h))
+            .copied()
+            .collect())
     }
+
     fn remove(&mut self, hash: &Hash) -> Result<(), VctrlError> {
         if self.objects.remove(hash).is_some() {
+            self.deleted.insert(*hash);
+            let writer = self.ensure_writer()?;
+            writer.write_u8(REC_DEL_OBJECT).map_err(VctrlError::Io)?;
+            writer.write_all(hash.as_bytes()).map_err(VctrlError::Io)?;
+            writer.flush().map_err(VctrlError::Io)?;
             Ok(())
         } else {
             Err(VctrlError::NotFound(format!("object '{}' not found", hash)))