From 2a7361432cda6946cd2c8d58c012282fb6fb1670 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Sep 2024 17:26:37 +0200 Subject: [PATCH] clean up a bit --- src/dest.rs | 289 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 246 insertions(+), 43 deletions(-) diff --git a/src/dest.rs b/src/dest.rs index 4456ad6..142e88b 100644 --- a/src/dest.rs +++ b/src/dest.rs @@ -92,7 +92,7 @@ struct TransferState { transaction_id: Option, metadata_params: MetadataGenericParams, progress: u64, - file_size_eof: u64, + // file_size_eof: u64, metadata_only: bool, condition_code: ConditionCode, delivery_code: DeliveryCode, @@ -110,7 +110,7 @@ impl Default for TransferState { transaction_id: None, metadata_params: Default::default(), progress: Default::default(), - file_size_eof: Default::default(), + // file_size_eof: Default::default(), metadata_only: false, condition_code: ConditionCode::NoError, delivery_code: DeliveryCode::Incomplete, @@ -367,8 +367,30 @@ impl< } } - pub fn cancel_request(&mut self, transaction_id: &TransactionId) { - // TODO: Implement. + /// This function models the Cancel.request CFDP primitive and is the recommended way + /// to cancel a transaction. It will cause a Notice Of Cancellation at this entity. + /// Please note that the state machine might still be active because a cancelled transfer + /// might still require some packets to be sent to the remote sender entity. + /// + /// If no unexpected errors occur, this function returns whether the current transfer + /// was cancelled. It returns [false] if the state machine is currently idle or if there + /// is a transaction ID missmatch. + pub fn cancel_request(&mut self, transaction_id: &TransactionId) -> bool { + if self.state() == super::State::Idle { + return false; + } + if let Some(active_id) = self.transaction_id() { + if active_id == *transaction_id { + self.trigger_notice_of_completion_cancelled( + ConditionCode::CancelRequestReceived, + EntityIdTlv::new(self.local_cfg.id), + ); + + self.step = TransactionStep::TransferCompletion; + return true; + } + } + false } /// Returns [None] if the state machine is IDLE, and the transmission mode of the current @@ -549,11 +571,16 @@ impl< } else { // This is an EOF (Cancel), perform Cancel Response Procedures according to chapter // 4.6.6 of the standard. - self.trigger_notice_of_completion_cancelled(eof_pdu.condition_code()); + self.trigger_notice_of_completion_cancelled( + eof_pdu.condition_code(), + EntityIdTlv::new(self.tparams.remote_cfg.unwrap().entity_id), + ); self.tparams.tstate.progress = eof_pdu.file_size(); - self.tparams.tstate.delivery_code = DeliveryCode::Incomplete; - self.tparams.tstate.fault_location_finished = - Some(EntityIdTlv::new(self.tparams.remote_cfg.unwrap().entity_id)); + if eof_pdu.file_size() > 0 { + self.tparams.tstate.delivery_code = DeliveryCode::Incomplete; + } else { + self.tparams.tstate.delivery_code = DeliveryCode::Complete; + } // TODO: The cancel EOF also supplies a checksum and a progress number. We could cross // check that checksum, but how would we deal with a checksum failure? The standard // does not specify anything for this case.. It could be part of the status report @@ -566,9 +593,19 @@ impl< Ok(()) } - fn trigger_notice_of_completion_cancelled(&mut self, cond_code: ConditionCode) { + fn trigger_notice_of_completion_cancelled( + &mut self, + cond_code: ConditionCode, + fault_location: EntityIdTlv, + ) { self.tparams.tstate.completion_disposition = CompletionDisposition::Cancelled; self.tparams.tstate.condition_code = cond_code; + self.tparams.tstate.fault_location_finished = Some(fault_location); + // For anything larger than 0, we'd have to do a checksum check to verify whether + // the delivery is really complete, and we need the EOF checksum for that.. + if self.tparams.tstate.progress == 0 { + self.tparams.tstate.delivery_code = DeliveryCode::Complete; + } } /// Returns whether the transfer can be completed regularly. @@ -819,10 +856,8 @@ impl< || self.tparams.metadata_params().closure_requested { sent_packets += self.send_finished_pdu()?; - self.step = TransactionStep::SendingFinishedPdu; - } else { - self.reset(); } + self.step = TransactionStep::SendingFinishedPdu; Ok(sent_packets) } @@ -837,6 +872,7 @@ impl< .disposition_on_cancellation && self.tstate().delivery_code == DeliveryCode::Incomplete { + // Safety: We already verified that the path is valid during the transaction start. let dest_path = unsafe { from_utf8_unchecked( &self.tparams.file_properties.dest_path_buf @@ -846,7 +882,6 @@ impl< if self.vfs.exists(dest_path)? && self.vfs.is_file(dest_path)? { self.vfs.remove_file(dest_path)?; } - // Safety: We already verified that the path is valid during the transaction start. self.tstate_mut().file_status = FileStatus::DiscardDeliberately; } let tstate = self.tstate(); @@ -970,14 +1005,14 @@ mod tests { pdu::{finished::FinishedPduReader, metadata::MetadataPduCreator, WritablePduPacket}, ChecksumType, TransmissionMode, }, - util::{UbfU16, UnsignedByteFieldU16}, + util::{UbfU16, UnsignedByteFieldU8}, }; use crate::{ filestore::NativeFilestore, tests::{ basic_remote_cfg_table, SentPdu, TestCfdpSender, TestCfdpUser, TestFaultHandler, - LOCAL_ID, + LOCAL_ID, REMOTE_ID, }, CountdownProvider, FaultHandler, IndicationConfig, PduRawWithInfo, StdRemoteEntityConfigProvider, TimerCreatorProvider, CRC_32, @@ -985,8 +1020,6 @@ mod tests { use super::*; - const REMOTE_ID: UnsignedByteFieldU16 = UnsignedByteFieldU16::new(2); - #[derive(Debug)] struct TestCheckTimer { counter: Cell, @@ -1054,23 +1087,24 @@ mod tests { dest_path: PathBuf, check_dest_file: bool, check_handler_idle_at_drop: bool, - expected_file_size: u64, closure_requested: bool, pdu_header: PduHeader, expected_full_data: Vec, + expected_file_size: u64, buf: [u8; 512], } impl DestHandlerTestbench { - fn new(fault_handler: TestFaultHandler, closure_requested: bool) -> Self { + fn new_with_fixed_paths(fault_handler: TestFaultHandler, closure_requested: bool) -> Self { let (src_path, dest_path) = init_full_filepaths_textfile(); assert!(!Path::exists(&dest_path)); - Self::new_with_custom_paths(fault_handler, closure_requested, src_path, dest_path) + Self::new(fault_handler, closure_requested, true, src_path, dest_path) } - fn new_with_custom_paths( + fn new( fault_handler: TestFaultHandler, closure_requested: bool, + check_dest_file: bool, src_path: PathBuf, dest_path: PathBuf, ) -> Self { @@ -1084,7 +1118,7 @@ mod tests { src_path, closure_requested, dest_path, - check_dest_file: false, + check_dest_file, check_handler_idle_at_drop: true, expected_file_size: 0, pdu_header: create_pdu_header(UbfU16::new(0)), @@ -1112,6 +1146,10 @@ mod tests { &mut self.handler.local_cfg.indication_cfg } + fn get_next_pdu(&mut self) -> Option { + self.handler.pdu_sender.retrieve_next_pdu() + } + fn indication_cfg(&mut self) -> &IndicationConfig { &self.handler.local_cfg.indication_cfg } @@ -1345,10 +1383,21 @@ mod tests { assert_eq!(dest_handler.step(), TransactionStep::Idle); } + #[test] + fn test_cancelling_idle_fsm() { + let fault_handler = TestFaultHandler::default(); + let test_sender = TestCfdpSender::default(); + let mut dest_handler = default_dest_handler(fault_handler, test_sender, Arc::default()); + assert!(!dest_handler.cancel_request(&TransactionId::new( + UnsignedByteFieldU8::new(0).into(), + UnsignedByteFieldU8::new(0).into() + ))); + } + #[test] fn test_empty_file_transfer_not_acked_no_closure() { let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = tb.test_user_from_cached_paths(0); tb.generic_transfer_init(&mut test_user, 0) .expect("transfer init failed"); @@ -1365,7 +1414,7 @@ mod tests { let file_size = file_data.len() as u64; let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = tb.test_user_from_cached_paths(file_size); tb.generic_transfer_init(&mut test_user, file_size) .expect("transfer init failed"); @@ -1386,7 +1435,7 @@ mod tests { let segment_len = 256; let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = tb.test_user_from_cached_paths(file_size); tb.generic_transfer_init(&mut test_user, file_size) .expect("transfer init failed"); @@ -1413,7 +1462,7 @@ mod tests { let segment_len = 256; let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = tb.test_user_from_cached_paths(file_size); let transaction_id = tb .generic_transfer_init(&mut test_user, file_size) @@ -1459,7 +1508,7 @@ mod tests { let segment_len = 256; let fault_handler = TestFaultHandler::default(); - let mut testbench = DestHandlerTestbench::new(fault_handler, false); + let mut testbench = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = testbench.test_user_from_cached_paths(file_size); let transaction_id = testbench .generic_transfer_init(&mut test_user, file_size) @@ -1538,7 +1587,7 @@ mod tests { #[test] fn test_file_transfer_with_closure() { let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, true); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); let mut test_user = tb.test_user_from_cached_paths(0); tb.generic_transfer_init(&mut test_user, 0) .expect("transfer init failed"); @@ -1556,15 +1605,11 @@ mod tests { tb.check_completion_indication_success(&mut test_user); } - #[test] - fn test_file_transfer_with_closure_check_limit_reached() { - // TODO: Implement test. - } - #[test] fn test_finished_pdu_insertion_rejected() { let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); + tb.check_dest_file = false; let mut user = tb.test_user_from_cached_paths(0); let finished_pdu = FinishedPduCreator::new_default( PduHeader::new_no_file_data(CommonPduConfig::default(), 0), @@ -1589,7 +1634,7 @@ mod tests { #[test] fn test_metadata_insertion_twice_fails() { let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, true); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); let mut user = tb.test_user_from_cached_paths(0); tb.generic_transfer_init(&mut user, 0) .expect("transfer init failed"); @@ -1618,7 +1663,7 @@ mod tests { let file_data = file_data_str.as_bytes(); let file_size = file_data.len() as u64; let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, true); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); let mut user = tb.test_user_from_cached_paths(file_size); tb.generic_transfer_init(&mut user, file_size) .expect("transfer init failed"); @@ -1707,9 +1752,10 @@ mod tests { let src_path = tempfile::TempPath::from_path("/tmp/test.txt").to_path_buf(); let dest_path = tempfile::TempDir::new().unwrap(); let mut dest_path_buf = dest_path.into_path(); - let mut tb = DestHandlerTestbench::new_with_custom_paths( + let mut tb = DestHandlerTestbench::new( fault_handler, false, + false, src_path.clone(), dest_path_buf.clone(), ); @@ -1727,7 +1773,7 @@ mod tests { #[test] fn test_tranfer_cancellation_empty_file_with_eof_pdu() { let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, false); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); let mut test_user = tb.test_user_from_cached_paths(0); tb.generic_transfer_init(&mut test_user, 0) .expect("transfer init failed"); @@ -1749,13 +1795,13 @@ mod tests { assert_eq!(packets, 0); } - fn generic_tranfer_cancellation_partial_file_with_eof_pdu_no_closure(with_closure: bool) { + fn generic_tranfer_cancellation_partial_file_with_eof_pdu(with_closure: bool) { let file_data_str = "Hello World!"; let file_data = file_data_str.as_bytes(); - let file_size = file_data.len() as u64; + let file_size = 5; let fault_handler = TestFaultHandler::default(); - let mut tb = DestHandlerTestbench::new(fault_handler, with_closure); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, with_closure); let mut test_user = tb.test_user_from_cached_paths(file_size); tb.generic_transfer_init(&mut test_user, file_size) .expect("transfer init failed"); @@ -1805,14 +1851,171 @@ mod tests { } else { assert_eq!(packets, 0); } + tb.expected_file_size = file_size; + tb.expected_full_data = file_data[0..file_size as usize].to_vec(); } #[test] fn test_tranfer_cancellation_partial_file_with_eof_pdu_no_closure() { - generic_tranfer_cancellation_partial_file_with_eof_pdu_no_closure(false); + generic_tranfer_cancellation_partial_file_with_eof_pdu(false); } #[test] fn test_tranfer_cancellation_partial_file_with_eof_pdu_with_closure() { - generic_tranfer_cancellation_partial_file_with_eof_pdu_no_closure(true); + generic_tranfer_cancellation_partial_file_with_eof_pdu(true); + } + + #[test] + fn test_tranfer_cancellation_empty_file_with_cancel_api() { + let fault_handler = TestFaultHandler::default(); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); + let mut test_user = tb.test_user_from_cached_paths(0); + let transaction_id = tb + .generic_transfer_init(&mut test_user, 0) + .expect("transfer init failed"); + tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); + tb.handler.cancel_request(&transaction_id); + let packets = tb + .handler + .state_machine_no_packet(&mut test_user) + .expect("state machine call with EOF insertion failed"); + assert_eq!(packets, 0); + } + + #[test] + fn test_tranfer_cancellation_empty_file_with_cancel_api_and_closure() { + let fault_handler = TestFaultHandler::default(); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); + let mut test_user = tb.test_user_from_cached_paths(0); + let transaction_id = tb + .generic_transfer_init(&mut test_user, 0) + .expect("transfer init failed"); + tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); + tb.handler.cancel_request(&transaction_id); + let packets = tb + .handler + .state_machine_no_packet(&mut test_user) + .expect("state machine call with EOF insertion failed"); + assert_eq!(packets, 1); + let next_pdu = tb.get_next_pdu().unwrap(); + assert_eq!(next_pdu.pdu_type, PduType::FileDirective); + assert_eq!( + next_pdu.file_directive_type.unwrap(), + FileDirectiveType::FinishedPdu + ); + let finished_pdu = + FinishedPduReader::new(&next_pdu.raw_pdu).expect("finished pdu read failed"); + assert_eq!( + finished_pdu.condition_code(), + ConditionCode::CancelRequestReceived + ); + assert_eq!(finished_pdu.file_status(), FileStatus::Retained); + // Empty file, so still complete. + assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Complete); + assert_eq!( + finished_pdu.fault_location(), + Some(EntityIdTlv::new(REMOTE_ID.into())) + ); + } + + #[test] + fn test_tranfer_cancellation_partial_file_with_cancel_api_and_closure() { + let file_data_str = "Hello World!"; + let file_data = file_data_str.as_bytes(); + let file_size = 5; + + let fault_handler = TestFaultHandler::default(); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); + let mut test_user = tb.test_user_from_cached_paths(file_size); + let transaction_id = tb + .generic_transfer_init(&mut test_user, file_size) + .expect("transfer init failed"); + tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); + tb.generic_file_data_insert(&mut test_user, 0, &file_data[0..5]) + .expect("file data insertion failed"); + + tb.handler.cancel_request(&transaction_id); + let packets = tb + .handler + .state_machine_no_packet(&mut test_user) + .expect("state machine call with EOF insertion failed"); + assert_eq!(packets, 1); + let next_pdu = tb.get_next_pdu().unwrap(); + assert_eq!(next_pdu.pdu_type, PduType::FileDirective); + assert_eq!( + next_pdu.file_directive_type.unwrap(), + FileDirectiveType::FinishedPdu + ); + let finished_pdu = + FinishedPduReader::new(&next_pdu.raw_pdu).expect("finished pdu read failed"); + assert_eq!( + finished_pdu.condition_code(), + ConditionCode::CancelRequestReceived + ); + assert_eq!(finished_pdu.file_status(), FileStatus::Retained); + assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Incomplete); + assert_eq!( + finished_pdu.fault_location(), + Some(EntityIdTlv::new(REMOTE_ID.into())) + ); + tb.expected_file_size = file_size; + tb.expected_full_data = file_data[0..file_size as usize].to_vec(); + } + + // Only incomplete received files will be removed. + #[test] + fn test_tranfer_cancellation_file_disposition_not_done_for_empty_file() { + let fault_handler = TestFaultHandler::default(); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); + let remote_cfg_mut = tb + .handler + .remote_cfg_table + .get_mut(LOCAL_ID.value()) + .unwrap(); + remote_cfg_mut.disposition_on_cancellation = true; + let mut test_user = tb.test_user_from_cached_paths(0); + let transaction_id = tb + .generic_transfer_init(&mut test_user, 0) + .expect("transfer init failed"); + tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); + + tb.handler.cancel_request(&transaction_id); + let packets = tb + .handler + .state_machine_no_packet(&mut test_user) + .expect("state machine call with EOF insertion failed"); + assert_eq!(packets, 0); + } + + #[test] + fn test_tranfer_cancellation_file_disposition_not_done_for_incomplete_file() { + let file_data_str = "Hello World!"; + let file_data = file_data_str.as_bytes(); + let file_size = file_data.len() as u64; + + let fault_handler = TestFaultHandler::default(); + let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); + tb.check_dest_file = false; + let remote_cfg_mut = tb + .handler + .remote_cfg_table + .get_mut(LOCAL_ID.value()) + .unwrap(); + remote_cfg_mut.disposition_on_cancellation = true; + let mut test_user = tb.test_user_from_cached_paths(file_size); + let transaction_id = tb + .generic_transfer_init(&mut test_user, file_size) + .expect("transfer init failed"); + tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); + tb.generic_file_data_insert(&mut test_user, 0, &file_data[0..5]) + .expect("file data insertion failed"); + + tb.handler.cancel_request(&transaction_id); + let packets = tb + .handler + .state_machine_no_packet(&mut test_user) + .expect("state machine call with EOF insertion failed"); + assert_eq!(packets, 0); + // File was disposed. + assert!(!Path::exists(tb.dest_path())); } }