Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MRG: panic when FSStorage::load_sig encounters more than one Signature in a JSON record #3333

Merged
merged 51 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
6b1d1bb
make error catchable
ctb Aug 21, 2024
7e73e8f
flag another problematic unwrap
ctb Aug 21, 2024
2896c49
more
ctb Aug 21, 2024
45b1a8f
more
ctb Aug 21, 2024
e8d7999
upd log message
ctb Aug 21, 2024
53bcf02
provide correct error
ctb Aug 21, 2024
d467d9f
switch Manifest from paths to TryFrom
ctb Aug 22, 2024
5f1eef6
Revert "provide correct error"
ctb Aug 22, 2024
eb46ecd
Revert "switch Manifest from paths to TryFrom"
ctb Aug 22, 2024
923af44
poor person's picklist?
ctb Aug 23, 2024
315dfff
add picklist select
ctb Aug 23, 2024
1fb658a
ok
ctb Aug 23, 2024
a122982
picklist by ref
ctb Aug 23, 2024
39816ab
add manifest.is_empty()
ctb Aug 23, 2024
61416fb
update revindex indexing message
ctb Aug 24, 2024
3a7abe9
propagate error on bad directory when opening RocksDB
ctb Aug 24, 2024
07e3a09
Merge branch 'update_revindex' into remove_unwrap
ctb Aug 24, 2024
fe75c6c
do we not need len?
ctb Aug 24, 2024
5fb20fc
nope, don't need em
ctb Aug 24, 2024
2c59010
revert for now
ctb Aug 24, 2024
c8477df
Merge branch 'latest' of github.com:sourmash-bio/sourmash into remove…
ctb Aug 27, 2024
6fee403
adjust select_picklist per luiz
ctb Aug 27, 2024
0675402
simplify and encapsulate
ctb Aug 27, 2024
17f50ef
cargo fmt
ctb Aug 27, 2024
39c140b
add a test of intersect_manifest
ctb Sep 15, 2024
16f72c5
impl PartialEq/Eq for Record, ignoring internal_location
ctb Sep 15, 2024
2146f30
switch to using full Record for intersect_manifest
ctb Sep 16, 2024
32839ae
round out comparison & hashing for Record
ctb Sep 16, 2024
43ee757
cargo fmt
ctb Sep 16, 2024
31aa378
Merge branch 'latest' into remove_unwrap
ctb Sep 16, 2024
1594dc4
remove identity closure
ctb Sep 16, 2024
2465c02
add in a print for debugging
ctb Sep 16, 2024
5af5f45
more print
ctb Sep 16, 2024
b46565d
remove prints
ctb Sep 16, 2024
b7a9850
Merge branch 'latest' of github.com:sourmash-bio/sourmash into remove…
ctb Sep 17, 2024
9c26752
add a test for Collection::intersect_manifest
ctb Sep 17, 2024
63df7a8
panic in bad circumstances
ctb Sep 17, 2024
7f9c1d1
Merge branch 'latest' of github.com:sourmash-bio/sourmash into debug_…
ctb Sep 21, 2024
4c29e9e
add unimplemented when there are multiple Signatures
ctb Sep 24, 2024
70b1c5d
upd
ctb Sep 24, 2024
b95ea9a
??
ctb Sep 24, 2024
075efde
Merge branch 'latest' of github.com:sourmash-bio/sourmash into debug_…
ctb Sep 24, 2024
de16118
Merge branch 'latest' of github.com:sourmash-bio/sourmash into debug_…
ctb Oct 16, 2024
39dd3a7
Merge branch 'latest' of github.com:sourmash-bio/sourmash into debug_…
ctb Oct 27, 2024
c5b2d4d
document InnerStorage a bit
ctb Oct 27, 2024
f370ed7
document InnerStorage a bit
ctb Oct 27, 2024
874a9a8
finally, a test that works (well, fails appropriately)
ctb Oct 27, 2024
0745990
cleanup
ctb Oct 27, 2024
d68d8d4
a much cleaner test
ctb Oct 27, 2024
abc01c1
cargo fmt
ctb Oct 27, 2024
e0726c8
Merge branch 'latest' into debug_multisigfile
ctb Nov 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/core/src/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,23 @@ mod test {
}
}

#[test]
#[should_panic] // for now...
fn sigstore_sig_from_record_2() {
let mut filename = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
filename.push("../../tests/test-data/short.sig.gz");
let v = [filename];
let collection = Collection::from_paths(&v).expect("no sigs!?");

// pull off first record
let v: Vec<_> = collection.iter().collect();
let (_idx, rec) = v.first().expect("no records in collection?!");

// this will panic with "unimplemented" because there are two
// sketches and that is not supported.
let _first_sig = collection.sig_from_record(rec).expect("no sig!?");
}

#[test]
fn sigstore_selection_moltype_zip() {
// load test sigs
Expand Down
35 changes: 26 additions & 9 deletions src/core/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@
/// Load signature from internal path
fn load_sig(&self, path: &str) -> Result<SigStore> {
let raw = self.load(path)?;
let sig = Signature::from_reader(&mut &raw[..])?
// TODO: select the right sig?
.swap_remove(0);
let mut vs = Signature::from_reader(&mut &raw[..])?;
if vs.len() > 1 {
unimplemented!("only one Signature currently allowed");
}
let sig = vs.swap_remove(0);

Check warning on line 41 in src/core/src/storage/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/storage/mod.rs#L41

Added line #L41 was not covered by tests

Ok(sig.into())
}
Expand Down Expand Up @@ -70,6 +72,16 @@
MissingFeature(String, String),
}

/// InnerStorage: a catch-all type that allows using any Storage in
/// parallel contexts.
///
/// Arc allows ref counting to share it between threads;
/// RwLock makes sure there is only one writer possible (and a lot of readers);
/// dyn Storage so we can init with anything that implements the Storage trait.

// Send + Sync + 'static is kind of a cheat to avoid lifetimes issues: we
// should get rid of that 'static if possible... -- Luiz.

#[derive(Clone)]
pub struct InnerStorage(Arc<RwLock<dyn Storage + Send + Sync + 'static>>);

Expand Down Expand Up @@ -299,9 +311,12 @@

fn load_sig(&self, path: &str) -> Result<SigStore> {
let raw = self.load(path)?;
let sig = Signature::from_reader(&mut &raw[..])?
// TODO: select the right sig?
.swap_remove(0);

let mut vs = Signature::from_reader(&mut &raw[..])?;
if vs.len() > 1 {
unimplemented!("only one Signature currently allowed when using 'load_sig'");
}
let sig = vs.swap_remove(0);

Ok(sig.into())
}
Expand Down Expand Up @@ -369,9 +384,11 @@

fn load_sig(&self, path: &str) -> Result<SigStore> {
let raw = self.load(path)?;
let sig = Signature::from_reader(&mut &raw[..])?
// TODO: select the right sig?
.swap_remove(0);
let mut vs = Signature::from_reader(&mut &raw[..])?;
if vs.len() > 1 {
unimplemented!("only one Signature currently allowed");
}
let sig = vs.swap_remove(0);

Ok(sig.into())
}
Expand Down
Binary file added tests/test-data/short.sig.gz
Binary file not shown.
Loading