History

lookup_prefix for ODB (#298)

Sebastian Thiel committed 4 years ago

Commit a4ccd181 parent 6244c064 files · +90 −57Browse files

Unified · Split

4 files
  1. git-odb/src/find.rs +1 −0
  2. git-odb/src/store_impls/dynamic/find.rs +51 −18
  3. git-odb/src/store_impls/dynamic/handle.rs +9 −0
  4. git-odb/tests/odb/store/dynamic.rs +29 −39
git-odb/src/find.rs+1−0
@@ -1,5 +1,6 @@
11 /// A way to indicate if a lookup, despite successful, was ambiguous or yielded exactly
22 /// one result in the particular index.
3+// TODO: find better name, ambiguous with git_pack::index::PrefixLookupResult (entry_index inside)
34 pub type PrefixLookupResult = Result<git_hash::ObjectId, ()>;
45
56 ///
git-odb/src/store_impls/dynamic/find.rs+51−18
@@ -15,6 +15,8 @@ mod error { · 14 unmodified lines
1515 pub enum Error {
1616 #[error("An error occurred while obtaining an object from the loose object store")]
1717 Loose(#[from] loose::find::Error),
18+ #[error("An error occurred looking up a prefix which requires iteration")]
19+ LooseWalkDir(#[from] loose::iter::Error),
1820 #[error("An error occurred while obtaining an object from the packed object store")]
1921 Pack(#[from] pack::data::decode_entry::Error),
2022 #[error(transparent)]
@@ -32,8 +34,44 @@ where · 11 unmodified lines
3234 S: Deref<Target = super::Store> + Clone,
3335 {
3436 #[allow(missing_docs)] // TODO: docs
35− pub fn lookup_prefix(&self, _prefix: git_hash::Prefix) -> Result<Option<crate::find::PrefixLookupResult>, Error> {
36− todo!()
37+ pub fn lookup_prefix(&self, prefix: git_hash::Prefix) -> Result<Option<crate::find::PrefixLookupResult>, Error> {
38+ let mut candidate: Option<git_hash::ObjectId> = None;
39+ loop {
40+ let snapshot = self.snapshot.borrow();
41+ {
42+ for index in snapshot.indices.iter() {
43+ match (index.lookup_prefix(prefix), candidate) {
44+ (Some(Ok(oid)), Some(candidate)) if candidate != oid => return Ok(Some(Err(()))),
45+ (Some(Ok(_)), Some(_)) | (None, None) | (None, Some(_)) => continue,
46+ (Some(Err(())), _) => return Ok(Some(Err(()))),
47+ (Some(Ok(oid)), None) => {
48+ candidate = Some(oid);
49+ continue;
50+ }
51+ }
52+ }
53+ }
54+
55+ for lodb in snapshot.loose_dbs.iter() {
56+ match (lodb.lookup_prefix(prefix)?, candidate) {
57+ (Some(Ok(oid)), Some(candidate)) if candidate != oid => return Ok(Some(Err(()))),
58+ (Some(Ok(_)), Some(_)) | (None, None) | (None, Some(_)) => continue,
59+ (Some(Err(())), _) => return Ok(Some(Err(()))),
60+ (Some(Ok(oid)), None) => {
61+ candidate = Some(oid);
62+ continue;
63+ }
64+ }
65+ }
66+
67+ match self.store.load_one_index(self.refresh_mode, snapshot.marker)? {
68+ Some(new_snapshot) => {
69+ drop(snapshot);
70+ *self.snapshot.borrow_mut() = new_snapshot;
71+ }
72+ None => return Ok(candidate.map(Ok)),
73+ }
74+ }
3775 }
3876 }
3977
@@ -46,8 +84,8 @@ where · 6 unmodified lines
4684 // TODO: probably make this method fallible, but that would mean its own error type.
4785 fn contains(&self, id: impl AsRef<oid>) -> bool {
4886 let id = id.as_ref();
87+ let mut snapshot = self.snapshot.borrow_mut();
4988 loop {
50− let mut snapshot = self.snapshot.borrow_mut();
5189 {
5290 for (idx, index) in snapshot.indices.iter().enumerate() {
5391 if index.contains(id) {
@@ -67,8 +105,7 @@ where · 13 unmodified lines
67105
68106 match self.store.load_one_index(self.refresh_mode, snapshot.marker) {
69107 Ok(Some(new_snapshot)) => {
70− drop(snapshot);
71− *self.snapshot.borrow_mut() = new_snapshot;
108+ *snapshot = new_snapshot;
72109 }
73110 Ok(None) => return false, // nothing more to load, or our refresh mode doesn't allow disk refreshes
74111 Err(_) => return false, // something went wrong, nothing we can communicate here with this trait. TODO: Maybe that should change?
@@ -83,8 +120,8 @@ where · 8 unmodified lines
83120 pack_cache: &mut impl DecodeEntry,
84121 ) -> Result<Option<(Data<'a>, Option<Location>)>, Self::Error> {
85122 let id = id.as_ref();
123+ let mut snapshot = self.snapshot.borrow_mut();
86124 'outer: loop {
87− let mut snapshot = self.snapshot.borrow_mut();
88125 {
89126 let marker = snapshot.marker;
90127 for (idx, index) in snapshot.indices.iter_mut().enumerate() {
@@ -105,8 +142,7 @@ where · 14 unmodified lines
105142 // The pack wasn't available anymore so we are supposed to try another round with a fresh index
106143 match self.store.load_one_index(self.refresh_mode, snapshot.marker)? {
107144 Some(new_snapshot) => {
108− drop(snapshot);
109− *self.snapshot.borrow_mut() = new_snapshot;
145+ *snapshot = new_snapshot;
110146 continue 'outer;
111147 }
112148 None => {
@@ -166,8 +202,7 @@ where · 53 unmodified lines
166202
167203 match self.store.load_one_index(self.refresh_mode, snapshot.marker)? {
168204 Some(new_snapshot) => {
169− drop(snapshot);
170− *self.snapshot.borrow_mut() = new_snapshot;
205+ *snapshot = new_snapshot;
171206 }
172207 None => return Ok(None),
173208 }
@@ -180,8 +215,8 @@ where · 6 unmodified lines
180215 "BUG: handle must be configured to `prevent_pack_unload()` before using this method"
181216 );
182217 let id = id.as_ref();
218+ let mut snapshot = self.snapshot.borrow_mut();
183219 'outer: loop {
184− let mut snapshot = self.snapshot.borrow_mut();
185220 {
186221 let marker = snapshot.marker;
187222 for (idx, index) in snapshot.indices.iter_mut().enumerate() {
@@ -202,8 +237,7 @@ where · 14 unmodified lines
202237 // The pack wasn't available anymore so we are supposed to try another round with a fresh index
203238 match self.store.load_one_index(self.refresh_mode, snapshot.marker).ok()? {
204239 Some(new_snapshot) => {
205− drop(snapshot);
206− *self.snapshot.borrow_mut() = new_snapshot;
240+ *snapshot = new_snapshot;
207241 continue 'outer;
208242 }
209243 None => {
@@ -239,8 +273,7 @@ where · 29 unmodified lines
239273
240274 match self.store.load_one_index(self.refresh_mode, snapshot.marker).ok()? {
241275 Some(new_snapshot) => {
242− drop(snapshot);
243− *self.snapshot.borrow_mut() = new_snapshot;
276+ *snapshot = new_snapshot;
244277 }
245278 None => return None,
246279 }
@@ -279,9 +312,9 @@ where · 32 unmodified lines
279312 "BUG: handle must be configured to `prevent_pack_unload()` before using this method"
280313 );
281314 let pack_id = PackId::from_intrinsic_pack_id(location.pack_id);
315+ let mut snapshot = self.snapshot.borrow_mut();
316+ let marker = snapshot.marker;
282317 loop {
283− let mut snapshot = self.snapshot.borrow_mut();
284− let marker = snapshot.marker;
285318 {
286319 for index in snapshot.indices.iter_mut() {
287320 if let Some(possibly_pack) = index.pack(pack_id) {
@@ -289,7 +322,7 @@ where · 1 unmodified line
289322 Some(pack) => pack,
290323 None => {
291324 let pack = self.store.load_pack(pack_id, marker).ok()?.expect(
292− "BUG: pack must exist from previous call to locaion_by_oid() and must not be unloaded",
325+ "BUG: pack must exist from previous call to location_by_oid() and must not be unloaded",
293326 );
294327 *possibly_pack = Some(pack);
295328 possibly_pack.as_deref().expect("just put it in")
git-odb/src/store_impls/dynamic/handle.rs+9−0
@@ -138,6 +138,15 @@ pub(crate) mod index_lookup { · 137 unmodified lines
138138 }
139139 }
140140
141+ #[allow(missing_docs)] // TODO: docs
142+ pub(crate) fn lookup_prefix(&self, prefix: git_hash::Prefix) -> Option<crate::find::PrefixLookupResult> {
143+ let res = match &self.file {
144+ handle::SingleOrMultiIndex::Single { index, .. } => index.lookup_prefix(prefix),
145+ handle::SingleOrMultiIndex::Multi { index, .. } => index.lookup_prefix(prefix),
146+ }?;
147+ Some(res.map(|entry_index| self.oid_at_index(entry_index).to_owned()))
148+ }
149+
141150 /// See if the oid is contained in this index, and return its full id for lookup possibly alongside its data file if already
142151 /// loaded.
143152 /// Also return the index itself as it's needed to resolve intra-pack ref-delta objects. They are a possibility even though
git-odb/tests/odb/store/dynamic.rs+29−39
@@ -381,53 +381,43 @@ fn lookup() { · 380 unmodified lines
381381
382382 mod lookup_prefix {
383383 use crate::store::dynamic::db_with_all_object_sources;
384+ use git_testtools::hex_to_id;
384385
385386 #[test]
386− #[should_panic]
387387 fn returns_none_for_prefixes_without_any_match() {
388388 let (handle, _tmp) = db_with_all_object_sources().unwrap();
389389 let prefix = git_hash::Prefix::new(git_hash::ObjectId::null(git_hash::Kind::Sha1), 7).unwrap();
390390 assert!(handle.lookup_prefix(prefix).unwrap().is_none());
391391 }
392392
393− // #[test]
394− // fn returns_some_err_for_prefixes_with_more_than_one_match() {
395− // let objects_dir = git_testtools::tempfile::tempdir().unwrap();
396− // git_testtools::copy_recursively_into_existing_dir(fixture_path("objects"), &objects_dir).unwrap();
397− // std::fs::write(
398− // objects_dir
399− // .path()
400− // .join("37")
401− // .join("d4ffffffffffffffffffffffffffffffffffff"),
402− // b"fake",
403− // )
404− // .unwrap();
405− // let store = git_odb::loose::Store::at(objects_dir.path(), git_hash::Kind::Sha1);
406− // let prefix = git_hash::Prefix::new(hex_to_id("37d4e6c5c48ba0d245164c4e10d5f41140cab980"), 4).unwrap();
407− // assert_eq!(
408− // store.lookup_prefix(prefix).unwrap(),
409− // Some(Err(())),
410− // "there are two objects with that prefix"
411− // );
412− // }
413−
414− // #[test]
415− // fn iterable_objects_can_be_looked_up_with_varying_prefix_lengths() {
416− // let store = ldb();
417− // let hex_lengths = &[4, 7, 40];
418− // for (index, oid) in store.iter().map(Result::unwrap).enumerate() {
419− // let hex_len = hex_lengths[index % hex_lengths.len()];
420− // let prefix = git_hash::Prefix::new(oid, hex_len).unwrap();
421− // assert_eq!(
422− // store
423− // .lookup_prefix(prefix)
424− // .unwrap()
425− // .expect("object exists")
426− // .expect("unambiguous"),
427− // oid
428− // );
429− // }
430− // }
393+ #[test]
394+ fn returns_some_err_for_prefixes_with_more_than_one_match() {
395+ let (store, _tmp) = db_with_all_object_sources().unwrap();
396+ let prefix = git_hash::Prefix::new(hex_to_id("a7065b5e971a6d8b55875d8cf634a3a37202ab23"), 4).unwrap();
397+ assert_eq!(
398+ store.lookup_prefix(prefix).unwrap(),
399+ Some(Err(())),
400+ "there are two objects with that prefix"
401+ );
402+ }
403+
404+ #[test]
405+ fn iterable_objects_can_be_looked_up_with_varying_prefix_lengths() {
406+ let (store, _tmp) = db_with_all_object_sources().unwrap();
407+ let hex_lengths = &[5, 7, 40];
408+ for (index, oid) in store.iter().unwrap().map(Result::unwrap).enumerate() {
409+ let hex_len = hex_lengths[index % hex_lengths.len()];
410+ let prefix = git_hash::Prefix::new(oid, hex_len).unwrap();
411+ assert_eq!(
412+ store
413+ .lookup_prefix(prefix)
414+ .unwrap()
415+ .expect("object exists")
416+ .expect("unambiguous"),
417+ oid
418+ );
419+ }
420+ }
431421 }
432422
433423 #[test]