Prefix now validates all constraints and errors on violation (#298)

That way, min and max lengths is assumed to be correct.

git-hash/src/lib.rs

@@ -11,7 +11,7 @@ use std::{convert::TryFrom, str::FromStr};
 pub use borrowed::oid;
 
 mod owned;
-pub use owned::{ObjectId, Prefix};
+pub use owned::{prefix, ObjectId, Prefix};
 
 #[allow(missing_docs)]
 pub mod decode {

git-hash/src/owned.rs

@@ -11,17 +11,40 @@ pub struct Prefix {
     hex_len: usize,
 }
 
+///
+pub mod prefix {
+    use quick_error::quick_error;
+
+    quick_error! {
+        /// TODO:
+        #[derive(Debug)]
+        #[allow(missing_docs)]
+        pub enum Error {
+            TooShort { hex_len: usize } {
+                display("The minimum hex length of a short object id is 4, got {}", hex_len)
+            }
+            TooLong { object_kind: crate::Kind, hex_len: usize } {
+                display("An object of kind {} cannot be larger than {} in hex, but {} was requested", object_kind, object_kind.len_in_hex(), hex_len)
+            }
+        }
+    }
+}
+
 impl Prefix {
     /// Create a new instance by taking a full `id` as input and truncating it to `hex_len`.
     ///
     /// For instance, with `hex_len` of 7 the resulting prefix is 3.5 bytes, or 3 bytes and 4 bits
     /// wide, with all other bytes and bits set to zero.
-    pub fn from_id(id: impl AsRef<oid>, hex_len: usize) -> Self {
+    pub fn try_from_id(id: impl AsRef<oid>, hex_len: usize) -> Result<Self, prefix::Error> {
         let id = id.as_ref();
-        assert!(
-            hex_len <= id.kind().len_in_hex(),
-            "hex_len must not be larger than the maximum hex len of the input id"
-        );
+        if hex_len > id.kind().len_in_hex() {
+            return Err(prefix::Error::TooLong {
+                object_kind: id.kind(),
+                hex_len,
+            });
+        } else if hex_len < 4 {
+            return Err(prefix::Error::TooShort { hex_len });
+        }
 
         let mut prefix = ObjectId::null(id.kind());
         let b = prefix.as_mut_slice();
@@ -31,7 +54,7 @@ impl Prefix {
             b[hex_len / 2] &= 0xf0;
         }
 
-        Prefix { bytes: prefix, hex_len }
+        Ok(Prefix { bytes: prefix, hex_len })
     }
 
     /// Returns the prefix as object id.

git-hash/tests/oid/mod.rs

@@ -8,23 +8,32 @@ mod prefix {
             let oid_hex = "abcdefabcdefabcdefabcdefabcdefabcdefabcd";
             let oid = hex_to_id(oid_hex);
 
-            assert_eq!(git_hash::Prefix::from_id(oid, 0).as_oid(), ObjectId::null(oid.kind()));
-
-            for hex_len in 1..oid.kind().len_in_hex() {
+            for hex_len in 4..oid.kind().len_in_hex() {
                 let mut expected = String::from(&oid_hex[..hex_len]);
                 let num_of_zeros = oid.kind().len_in_hex() - hex_len;
                 expected.extend(std::iter::repeat('0').take(num_of_zeros));
-                let prefix = git_hash::Prefix::from_id(oid, hex_len);
+                let prefix = git_hash::Prefix::try_from_id(oid, hex_len).unwrap();
                 assert_eq!(prefix.as_oid().to_hex().to_string(), expected, "{}", hex_len);
                 assert_eq!(prefix.hex_len(), hex_len);
             }
         }
 
         #[test]
-        #[should_panic]
-        fn panics_if_hex_len_is_longer_than_oid_len_in_hex() {
+        fn errors_if_hex_len_is_longer_than_oid_len_in_hex() {
+            let kind = Kind::Sha1;
+            assert!(matches!(
+                git_hash::Prefix::try_from_id(ObjectId::null(kind), kind.len_in_hex() + 1),
+                Err(git_hash::prefix::Error::TooLong { .. })
+            ));
+        }
+
+        #[test]
+        fn errors_if_hex_len_is_too_short() {
             let kind = Kind::Sha1;
-            git_hash::Prefix::from_id(ObjectId::null(kind), kind.len_in_hex() + 1);
+            assert!(matches!(
+                git_hash::Prefix::try_from_id(ObjectId::null(kind), 3),
+                Err(git_hash::prefix::Error::TooShort { .. })
+            ));
         }
     }
 }

git-pack/src/index/mod.rs

@@ -99,6 +99,10 @@ impl Version {
     }
 }
 
+/// A way to indicate if a lookup, despite successful, was ambiguous or yielded exactly
+/// one result in the particular index.
+pub type PrefixLookupResult = Result<EntryIndex, ()>;
+
 /// The type for referring to indices of an entry within the index file.
 pub type EntryIndex = u32;
 

git-pack/src/multi_index/access.rs

@@ -1,5 +1,6 @@
 use std::path::{Path, PathBuf};
 
+use crate::index::PrefixLookupResult;
 use crate::{
     data,
     multi_index::{EntryIndex, File, PackIndex, Version},
@@ -67,7 +68,7 @@ impl File {
     }
 
     /// TODO
-    pub fn lookup_prefix(&self, _id: impl AsRef<git_hash::oid>, _hex_len: usize) -> Option<EntryIndex> {
+    pub fn lookup_prefix(&self, prefix: git_hash::Prefix) -> Option<PrefixLookupResult> {
         todo!()
     }
 

git-pack/tests/pack/multi_index/mod.rs

@@ -16,15 +16,23 @@ mod access {
     use std::path::PathBuf;
 
     #[test]
-    fn lookup_abbrev() {
+    #[ignore]
+    fn lookup_with_ambiguity() {}
+
+    #[test]
+    fn lookup_prefix() {
         let (file, _path) = multi_index();
 
         for (idx, entry) in file.iter().enumerate() {
             let hex_len = (idx % file.object_hash().len_in_hex()).min(7);
             let hex_oid = entry.oid.to_hex_with_len(hex_len).to_string();
             assert_eq!(hex_oid.len(), hex_len);
-            let _oid_prefix = git_hash::Prefix::from_id(&entry.oid, hex_len);
-            // file.lookup_prefix(oid_prefix, hex_len).expect("non-ambiguous")
+            let oid_prefix = git_hash::Prefix::try_from_id(&entry.oid, hex_len);
+            let entry_index = file
+                .lookup_prefix(oid_prefix)
+                .expect("object found")
+                .expect("non-ambiguous");
+            assert_eq!(file.oid_at_index(entry_index), entry.oid);
         }
     }