From 1bd98ccb2249acb7b686bfd8f6afc113d0621de5 Mon Sep 17 00:00:00 2001 From: David Hofer Date: Tue, 21 Jul 2026 14:55:21 -0400 Subject: [PATCH] Send default collection via push and pull --- gen-capnp-schemas/gen-models.capnp | 2 ++ gen-models/src/manifest.rs | 33 +++++++++++++++++++- gen-models/src/operations.rs | 12 ++++++++ gen-python/README.md | 1 + gen-python/src/python_api/repository/mod.rs | 31 +++++++++++++++++++ src/operation_management.rs | 34 ++++++++++++++++++--- 6 files changed, 107 insertions(+), 6 deletions(-) diff --git a/gen-capnp-schemas/gen-models.capnp b/gen-capnp-schemas/gen-models.capnp index 79cd8132f..d830b261e 100644 --- a/gen-capnp-schemas/gen-models.capnp +++ b/gen-capnp-schemas/gen-models.capnp @@ -251,11 +251,13 @@ struct Manifest { some @3 :List(UInt8); } operations @4 :List(ManifestOperation); + defaultCollectionName @5 :Text; } struct ManifestDiff { missingInManifest2 @0 :List(ManifestOperation); missingInManifest1 @1 :List(ManifestOperation); + defaultCollectionName @2 :Text; } struct AnnotationInterval { diff --git a/gen-models/src/manifest.rs b/gen-models/src/manifest.rs index a655fd4ee..09d191033 100644 --- a/gen-models/src/manifest.rs +++ b/gen-models/src/manifest.rs @@ -9,7 +9,7 @@ use crate::{ manifest, manifest_annotation_file_addition, manifest_diff, manifest_operation, manifest_operation_file_addition, }, - operations::{FileAddition, Operation, OperationFile, OperationSummary}, + operations::{Defaults, FileAddition, Operation, OperationFile, OperationSummary}, traits::Query, }; @@ -252,6 +252,7 @@ pub struct Manifest { pub branch_name: String, pub end_hash: Option, pub operations: Vec, + pub default_collection_name: String, } impl<'a> Capnp<'a> for Manifest { @@ -274,6 +275,8 @@ impl<'a> Capnp<'a> for Manifest { let mut operation_builder = operations_builder.reborrow().get(i as u32); operation.write_capnp(&mut operation_builder); } + + builder.set_default_collection_name(&self.default_collection_name); } fn read_capnp(reader: Self::Reader) -> Self { @@ -295,11 +298,18 @@ impl<'a> Capnp<'a> for Manifest { } }; + let default_collection_name = reader + .get_default_collection_name() + .unwrap() + .to_string() + .unwrap(); + Manifest { manifest_version, branch_name, end_hash, operations, + default_collection_name, } } } @@ -308,6 +318,7 @@ impl<'a> Capnp<'a> for Manifest { pub struct ManifestDiff { pub missing_in_manifest2: Vec, pub missing_in_manifest1: Vec, + pub default_collection_name: String, } impl<'a> Capnp<'a> for ManifestDiff { @@ -330,6 +341,8 @@ impl<'a> Capnp<'a> for ManifestDiff { let mut operation_builder = missing_in_manifest1_builder.reborrow().get(i as u32); operation.write_capnp(&mut operation_builder); } + + builder.set_default_collection_name(&self.default_collection_name); } fn read_capnp(reader: Self::Reader) -> Self { @@ -345,9 +358,16 @@ impl<'a> Capnp<'a> for ManifestDiff { missing_in_manifest1.push(ManifestOperation::read_capnp(operation_reader)); } + let default_collection_name = reader + .get_default_collection_name() + .unwrap() + .to_string() + .unwrap(); + ManifestDiff { missing_in_manifest2, missing_in_manifest1, + default_collection_name, } } } @@ -415,6 +435,9 @@ impl<'a> ManifestGenerator<'a> { branch_name: branch_name.to_string(), end_hash: end_hash.copied(), operations: manifest_operations, + default_collection_name: Defaults::get(self.conn) + .and_then(|defaults| defaults.collection_name) + .unwrap_or_else(|| "default".to_string()), }) } } @@ -473,6 +496,7 @@ impl ManifestComparer { Ok(ManifestDiff { missing_in_manifest2, missing_in_manifest1, + default_collection_name: manifest2.default_collection_name.clone(), }) } } @@ -584,6 +608,7 @@ mod tests { manifest_version: "1.0".to_string(), branch_name: "main".to_string(), end_hash: Some(operation.hash), + default_collection_name: "test collection".to_string(), operations: vec![ManifestOperation { operation, file_additions: vec![], @@ -631,6 +656,7 @@ mod tests { let manifest_diff = ManifestDiff { missing_in_manifest2: vec![manifest_operation.clone()], missing_in_manifest1: vec![manifest_operation], + default_collection_name: "test collection".to_string(), }; let mut message = TypedBuilder::::new_default(); @@ -649,6 +675,7 @@ mod tests { let db_uuid = crate::metadata::get_db_uuid(conn); crate::files::GenDatabase::create(op_conn, &db_uuid, "test_db", "test_db_path").unwrap(); + Defaults::set_default_collection(op_conn, "test collection").unwrap(); let mut session = start_operation(conn); crate::sequence::Sequence::new() @@ -680,6 +707,7 @@ mod tests { .unwrap(); assert_eq!(manifest.branch_name, "main"); + assert_eq!(manifest.default_collection_name, "test collection"); assert_eq!(manifest.operations.len(), 2); assert_eq!(manifest.operations[0].operation.hash, op1.hash); assert_eq!(manifest.operations[1].operation.hash, op2.hash); @@ -842,6 +870,7 @@ mod tests { manifest_version: "1.0".to_string(), branch_name: "main".to_string(), end_hash: Some(op2.hash), + default_collection_name: "local".to_string(), operations: vec![ ManifestOperation { operation: op1.clone(), @@ -862,6 +891,7 @@ mod tests { manifest_version: "1.0".to_string(), branch_name: "main".to_string(), end_hash: Some(op3.hash), + default_collection_name: "remote".to_string(), operations: vec![ ManifestOperation { operation: op2.clone(), @@ -885,6 +915,7 @@ mod tests { assert_eq!(diff.missing_in_manifest1.len(), 1); assert_eq!(diff.missing_in_manifest1[0].operation.hash, op3.hash); + assert_eq!(diff.default_collection_name, "remote"); } #[test] diff --git a/gen-models/src/operations.rs b/gen-models/src/operations.rs index 5fd20aac9..5af409ff7 100644 --- a/gen-models/src/operations.rs +++ b/gen-models/src/operations.rs @@ -1270,6 +1270,18 @@ impl Query for Defaults { } impl Defaults { + /// Set the default collection by name. + pub fn set_default_collection( + conn: &OperationsConnection, + collection_name: &str, + ) -> SQLResult<()> { + conn.execute( + "UPDATE defaults SET collection_name = ?1 WHERE id = 1", + params![collection_name], + )?; + Ok(()) + } + /// Set the default remote by name pub fn set_default_remote( conn: &OperationsConnection, diff --git a/gen-python/README.md b/gen-python/README.md index d46ae09e1..4dfeefcd1 100644 --- a/gen-python/README.md +++ b/gen-python/README.md @@ -16,6 +16,7 @@ looking things up afterward: import gen repo = gen.Repository("path/to/.gen") +repo.defaults("my_collection") # default for omitted collection= arguments sample = repo.import_fasta("path/to.fa") # -> Sample sg = sample[0] # -> SequenceGraph diff --git a/gen-python/src/python_api/repository/mod.rs b/gen-python/src/python_api/repository/mod.rs index 5422fa7fd..93c6e8e7f 100644 --- a/gen-python/src/python_api/repository/mod.rs +++ b/gen-python/src/python_api/repository/mod.rs @@ -192,6 +192,13 @@ impl PyRepository { path_to_py_path(py, &path) } + /// Set the collection used when a method's `collection` argument is omitted. + fn defaults(&self, collection: &str) -> PyResult<()> { + Defaults::set_default_collection(self.context.operations().conn(), collection) + .map_err(sqlite_err_to_pyerr)?; + Ok(()) + } + // Transaction context manager /// Returns self so that Python's `with` statement calls `__enter__`/`__exit__` @@ -383,6 +390,30 @@ mod python_tests { }); } + #[test] + fn test_defaults_changes_default_collection() { + pyo3::prepare_freethreaded_python(); + Python::with_gil(|py| { + let py_repo = make_repo(py); + let dir = tempdir().unwrap(); + let fasta = write_fasta(&dir, "test.fa", "chr1", "ACGTACGT"); + + py_repo.borrow(py).defaults("configured").unwrap(); + let sample = py_repo + .borrow(py) + .import_fasta( + fasta.to_str().unwrap().to_string(), + Some("test".to_string()), + false, + None, + ) + .unwrap(); + + assert_eq!(sample.collection_name, "configured"); + assert_eq!(py_repo.borrow(py).get_default_collection(), "configured"); + }); + } + #[test] fn test_import_fasta_creates_block_group() { pyo3::prepare_freethreaded_python(); diff --git a/src/operation_management.rs b/src/operation_management.rs index 272b78250..47522002b 100644 --- a/src/operation_management.rs +++ b/src/operation_management.rs @@ -777,13 +777,17 @@ fn push_to_file_remote( None }; - let diff = if let Some(remote_manifest) = remote_manifest { - ManifestComparer::diff_manifests(&local_manifest, &remote_manifest)? + let diff = if let Some(remote_manifest) = &remote_manifest { + ManifestComparer::diff_manifests(&local_manifest, remote_manifest)? } else { // Empty remote - all local operations are missing ManifestDiff { missing_in_manifest2: local_manifest.operations.clone(), missing_in_manifest1: vec![], + default_collection_name: remote_manifest.as_ref().map_or_else( + || "default".to_string(), + |manifest| manifest.default_collection_name.clone(), + ), } }; @@ -791,6 +795,8 @@ fn push_to_file_remote( return Err(RemoteOperationError::RemoteBranchAhead); } + Defaults::set_default_collection(remote_op_conn, &local_manifest.default_collection_name)?; + if !diff.missing_in_manifest2.is_empty() { apply_operations_to_remote( local_context, @@ -1023,18 +1029,23 @@ fn pull_from_file_remote( )) })?; - let diff = if let Some(remote_hash) = remote_branch.current_operation_hash { - let remote_manifest = ManifestGenerator::new(remote_op_conn) - .generate_manifest(¤t_branch.name, Some(&remote_hash))?; + let remote_manifest = ManifestGenerator::new(remote_op_conn).generate_manifest( + ¤t_branch.name, + remote_branch.current_operation_hash.as_ref(), + )?; + let diff = if remote_branch.current_operation_hash.is_some() { ManifestComparer::diff_manifests(&local_manifest, &remote_manifest)? } else { // There's nothing in the remote, so just make it empty since we have nothing to pull. ManifestDiff { missing_in_manifest2: vec![], missing_in_manifest1: vec![], + default_collection_name: remote_manifest.default_collection_name.clone(), } }; + Defaults::set_default_collection(operation_conn, &remote_manifest.default_collection_name)?; + if diff.missing_in_manifest1.is_empty() { return Ok(()); } @@ -1069,6 +1080,8 @@ fn pull_from_remote_server( )?; let diff = send_manifest_to_remote(remote_name, remote_url, &manifest)?; + Defaults::set_default_collection(operation_conn, &diff.default_collection_name)?; + if diff.missing_in_manifest1.is_empty() { return Ok(()); } @@ -2773,6 +2786,7 @@ mod tests { "remote operation", HashId::random_str(), ); + Defaults::set_default_collection(remote_op_conn, "remote collection").unwrap(); let remote_url = format!( "file://{}", @@ -2794,6 +2808,10 @@ mod tests { let local_ops = Operation::all(op_conn); let remote_ops = Operation::all(remote_op_conn); assert_eq!(local_ops, remote_ops); + assert_eq!( + Defaults::get(op_conn).and_then(|defaults| defaults.collection_name), + Some("remote collection".to_string()) + ); } #[test] @@ -2932,6 +2950,7 @@ mod tests { description: "second operation".to_string(), }; let op2 = end_operation(&context, &mut session, &op_info, "test2", None).unwrap(); + Defaults::set_default_collection(op_conn, "local collection").unwrap(); let remote_context = setup_gen_on_disk(); let remote_url = format!( @@ -2952,6 +2971,11 @@ mod tests { .join(op2.hash.to_string()); assert!(remote_op1_dir.exists()); assert!(remote_op2_dir.exists()); + assert_eq!( + Defaults::get(remote_context.operations().conn()) + .and_then(|defaults| defaults.collection_name), + Some("local collection".to_string()) + ); } #[test]