continue tests

src/dest.rs

@@ -624,7 +624,7 @@ impl<
             }
             FileDirectiveType::MetadataPdu => {
                 let metadata_pdu = MetadataPduReader::from_bytes(raw_packet)?;
-                self.handle_metadata_pdu(metadata_pdu, self.step() == TransactionStep::Idle)?
+                self.handle_metadata_pdu(metadata_pdu)?
             }
             FileDirectiveType::PromptPdu => self.handle_prompt_pdu(raw_packet)?,
         };
@@ -659,39 +659,46 @@ impl<
         Ok(())
     }
 
-    fn handle_metadata_pdu(
-        &mut self,
-        metadata_pdu: MetadataPduReader,
-        first_packet: bool,
-    ) -> Result<(), DestError> {
-        if self.state != State::Idle {
-            return Err(DestError::RecvdMetadataButIsBusy);
-        }
+    fn handle_metadata_pdu(&mut self, metadata_pdu: MetadataPduReader) -> Result<(), DestError> {
+        let first_packet = self.step() == TransactionStep::Idle;
         if first_packet {
             self.first_packet_handling(metadata_pdu.pdu_header().common_pdu_conf())?;
         }
-        self.reset_nak_activity_parameters_if_active();
-        if self.transmission_mode().unwrap() == TransmissionMode::Acknowledged {
-            match self.transaction_params.acked_params.as_mut() {
-                Some(acked_params) => {
-                    if acked_params.metadata_missing {
-                        if let Some(timer) =
-                            self.transaction_params.deferred_procedure_timer.as_mut()
-                        {
-                            acked_params.nak_activity_counter = 0;
-                            timer.reset();
+        match self.transmission_mode() {
+            Some(transmission_mode) => {
+                if !first_packet && transmission_mode == TransmissionMode::Unacknowledged {
+                    return Err(DestError::RecvdMetadataButIsBusy);
+                }
+                //if !self.check_for_deferred_lost_segment_completion(false) {
+                match self.transaction_params.acked_params.as_mut() {
+                    Some(acked_params) => {
+                        if acked_params.metadata_missing {
+                            if let Some(timer) =
+                                self.transaction_params.deferred_procedure_timer.as_mut()
+                            {
+                                acked_params.nak_activity_counter = 0;
+                                timer.reset();
+                            }
+                            acked_params.metadata_missing = false;
+                        } else {
+                            // Quietly ignore.
+                            return Ok(());
                         }
                     }
+                    None => {
+                        self.transaction_params.acked_params = Some(AcknowledgedModeParams {
+                            last_start_offset: 0,
+                            last_end_offset: 0,
+                            metadata_missing: false,
+                            nak_activity_counter: 0,
+                            deferred_procedure_active: false,
+                        });
+                    }
                 }
-                None => {
-                    self.transaction_params.acked_params = Some(AcknowledgedModeParams {
-                        last_start_offset: 0,
-                        last_end_offset: 0,
-                        metadata_missing: false,
-                        nak_activity_counter: 0,
-                        deferred_procedure_active: false,
-                    });
-                }
+                //}
+            }
+            None => {
+                return Err(DestError::RecvdMetadataButIsBusy);
             }
         }
 
@@ -978,12 +985,16 @@ impl<
         eof_pdu: EofPdu,
     ) -> Result<u32, DestError> {
         let sent_packets = 0;
+        let first_packet = self.step() == TransactionStep::Idle;
+        if first_packet {
+            self.first_packet_handling(eof_pdu.pdu_header().common_pdu_conf())?;
+        }
         if self.local_cfg.indication_cfg.eof_recv {
             // Unwrap is okay here, application logic ensures that transaction ID is valid here.
             cfdp_user
                 .eof_recvd_indication(self.transaction_params.transaction_id.as_ref().unwrap());
         }
-        if self.state == State::Idle {
+        if first_packet {
             if self.transmission_mode().unwrap() == TransmissionMode::Unacknowledged {
                 return Err(DestError::WrongStepForPdu {
                     pdu_type: PduType::FileDirective,
@@ -991,8 +1002,7 @@ impl<
                     step: self.step(),
                 });
             }
-            self.handle_eof_without_previous_metadata(&eof_pdu)?;
-            return Ok(sent_packets);
+            return self.handle_eof_without_previous_metadata_in_acked_mode(&eof_pdu);
         }
         if eof_pdu.condition_code() == ConditionCode::NoError {
             if !self.handle_eof_no_error(&eof_pdu)? {
@@ -1006,17 +1016,18 @@ impl<
             TransmissionMode::Acknowledged => {
                 self.acknowledge_eof_pdu(&eof_pdu)?;
                 sent_packets += 1;
-                if self
-                    .transaction_params
-                    .acked_params
-                    .as_ref()
-                    .unwrap()
-                    .metadata_missing
-                    || !self.lost_segment_tracker.is_empty()
-                {
-                    self.start_deferred_lost_segment_handling();
-                } else {
-                    self.set_step(TransactionStep::TransferCompletion);
+                match self.transaction_params.acked_params.as_ref() {
+                    Some(acked_params) => {
+                        if acked_params.metadata_missing || !self.lost_segment_tracker.is_empty() {
+                            self.start_deferred_lost_segment_handling();
+                        } else {
+                            self.set_step(TransactionStep::TransferCompletion);
+                        }
+                    }
+                    // This can happen when the EOF is the first packet to arrive.
+                    None => {
+                        self.start_deferred_lost_segment_handling();
+                    }
                 }
             }
             TransmissionMode::Unacknowledged => {
@@ -1026,8 +1037,11 @@ impl<
         Ok(sent_packets)
     }
 
-    fn handle_eof_without_previous_metadata(&mut self, eof_pdu: &EofPdu) -> Result<(), DestError> {
-        self.transaction_params.progress = eof_pdu.file_size();
+    fn handle_eof_without_previous_metadata_in_acked_mode(
+        &mut self,
+        eof_pdu: &EofPdu,
+    ) -> Result<u32, DestError> {
+        let mut sent_packets = 0;
         self.transaction_params.file_size = eof_pdu.file_size();
         self.transaction_params.acked_params = Some(AcknowledgedModeParams {
             last_start_offset: 0,
@@ -1046,19 +1060,15 @@ impl<
             self.handle_eof_cancel(eof_pdu);
         }
         self.acknowledge_eof_pdu(eof_pdu)?;
+        sent_packets += 1;
         if self.transaction_params.completion_disposition.get() == CompletionDisposition::Cancelled
         {
             self.set_step(TransactionStep::TransferCompletion);
-            return Ok(());
+            return Ok(sent_packets);
         }
-        if let Some(ack_params) = &self.transaction_params.acked_params {
-            if ack_params.metadata_missing || !self.lost_segment_tracker.is_empty() {
-                self.start_deferred_lost_segment_handling();
-                return Ok(());
-            }
-        }
-        self.set_step(TransactionStep::TransferCompletion);
-        Ok(())
+        self.set_step(TransactionStep::WaitingForMetadata);
+        self.start_deferred_lost_segment_handling();
+        Ok(sent_packets)
     }
 
     fn handle_eof_cancel(&mut self, eof_pdu: &EofPdu) {
@@ -1129,14 +1139,8 @@ impl<
         }
     }
 
-    fn deferred_lost_segment_handling(&mut self) -> Result<u32, DestError> {
-        let mut sent_packets = 0;
-        assert!(
-            self.transaction_params.acked_params.is_some(),
-            "acknowledged parameters unexpectedly None"
-        );
-        let acked_params = self.transaction_params.acked_params.unwrap();
-        if self.lost_segment_tracker.is_empty() && !acked_params.metadata_missing {
+    fn check_for_deferred_lost_segment_completion(&mut self, metadata_missing: bool) -> bool {
+        if self.lost_segment_tracker.is_empty() && !metadata_missing {
             // We are done and have received everything.
             match self.checksum_verify(
                 self.transaction_params.progress,
@@ -1169,9 +1173,21 @@ impl<
                 .unwrap()
                 .deferred_procedure_active = false;
             self.set_step(TransactionStep::TransferCompletion);
-            return Ok(sent_packets);
+            return true;
         }
+        false
+    }
 
+    fn deferred_lost_segment_handling(&mut self) -> Result<u32, DestError> {
+        assert!(
+            self.transaction_params.acked_params.is_some(),
+            "acknowledged parameters unexpectedly None"
+        );
+        let acked_params = self.transaction_params.acked_params.unwrap();
+        if self.check_for_deferred_lost_segment_completion(acked_params.metadata_missing) {
+            return Ok(0);
+        }
+        let mut sent_packets = 0;
         let first_nak_issuance = self.transaction_params.deferred_procedure_timer.is_none();
         if first_nak_issuance {
             self.transaction_params.deferred_procedure_timer = Some(
@@ -1246,7 +1262,7 @@ impl<
                 .write_to_nak_segment_list(&mut nak_pdu_creator, acked_params.metadata_missing)
                 .expect("unexpected lost segment write error");
             let written_len = nak_pdu_creator
-                .finish(0, self.transaction_params.progress)
+                .finish(0, self.transaction_params.file_size)
                 .map_err(PduError::from)?;
             self.pdu_sender.send_file_directive_pdu(
                 FileDirectiveType::NakPdu,
@@ -1405,6 +1421,7 @@ impl<
                 .add_lost_segment((self.transaction_params.progress, eof_pdu.file_size()))?;
         }
 
+        self.transaction_params.file_size = eof_pdu.file_size();
         self.transaction_params.checksum = eof_pdu.file_checksum();
         if self.transaction_params.transmission_mode() == TransmissionMode::Unacknowledged
             && !self.checksum_verify(
@@ -3305,12 +3322,97 @@ mod tests {
         assert_eq!(
             tb.generic_file_data_insert(&mut user, 0, file_data)
                 .expect("file data insertion failed"),
-            0
+            1
         );
         tb.state_check(State::Busy, TransactionStep::WaitingForMetadata);
-        // TODO: Metadata should now be re-requested.
-        let _transfer_info = tb
+        assert_eq!(tb.pdu_queue_len(), 1);
+        let pdu = tb.get_next_pdu().unwrap();
+        assert_eq!(pdu.pdu_type, PduType::FileDirective);
+        assert_eq!(pdu.file_directive_type.unwrap(), FileDirectiveType::NakPdu);
+        let nak_pdu = NakPduReader::new(&pdu.raw_pdu).unwrap();
+        assert_eq!(nak_pdu.start_of_scope(), 0);
+        assert_eq!(nak_pdu.end_of_scope(), file_size);
+        let seg_reqs: Vec<(u64, u64)> = nak_pdu.get_segment_requests_iterator().unwrap().collect();
+        assert_eq!(seg_reqs.len(), 2);
+        // Metadata is re-requested.
+        assert_eq!(seg_reqs[0], (0, 0));
+        // File data is re-requested. The destination handler can not do anything with the file
+        // data as long as no metadata has been received. Buffering of FD PDUs is not supported.
+        assert_eq!(seg_reqs[1], (0, 12));
+
+        let transfer_info = tb
             .generic_transfer_init(&mut user, file_size)
             .expect("transfer init failed");
+        // Re-insert missing file data.
+        assert_eq!(
+            tb.generic_file_data_insert(&mut user, 0, file_data)
+                .expect("file data insertion failed"),
+            0
+        );
+        assert_eq!(
+            tb.generic_eof_no_error(&mut user, file_data.to_vec())
+                .expect("EOF no error insertion failed"),
+            2
+        );
+        tb.check_completion_indication_success(&mut user);
+        assert_eq!(tb.pdu_queue_len(), 2);
+        tb.check_eof_ack_pdu(ConditionCode::NoError);
+        tb.check_finished_pdu_success();
+        tb.acknowledge_finished_pdu(&mut user, &transfer_info);
     }
+
+    /*
+    #[test]
+    fn test_eof_before_metadata() {
+        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,
+            TransmissionMode::Acknowledged,
+            false,
+        );
+        tb.remote_cfg_mut().immediate_nak_mode = true;
+        let mut user = tb.test_user_from_cached_paths(file_size);
+        // We expect the ACK for the EOF and a NAK packet re-requesting the metadata and file data.
+        assert_eq!(
+            tb.generic_eof_no_error(&mut user, file_data.to_vec())
+                .expect("file data insertion failed"),
+            2
+        );
+        tb.state_check(State::Busy, TransactionStep::WaitingForMetadata);
+        assert_eq!(tb.pdu_queue_len(), 2);
+        tb.check_eof_ack_pdu(ConditionCode::NoError);
+        let pdu = tb.get_next_pdu().unwrap();
+        assert_eq!(pdu.pdu_type, PduType::FileDirective);
+        assert_eq!(pdu.file_directive_type.unwrap(), FileDirectiveType::NakPdu);
+        let nak_pdu = NakPduReader::new(&pdu.raw_pdu).unwrap();
+        assert_eq!(nak_pdu.start_of_scope(), 0);
+        assert_eq!(nak_pdu.end_of_scope(), file_size);
+        let seg_reqs: Vec<(u64, u64)> = nak_pdu.get_segment_requests_iterator().unwrap().collect();
+        assert_eq!(seg_reqs.len(), 2);
+        // Metadata is re-requested.
+        assert_eq!(seg_reqs[0], (0, 0));
+        // File data is re-requested. The destination handler can not do anything with the file
+        // data as long as no metadata has been received. Buffering of FD PDUs is not supported.
+        assert_eq!(seg_reqs[1], (0, 12));
+
+        let transfer_info = tb
+            .generic_transfer_init(&mut user, file_size)
+            .expect("transfer init failed");
+        // Re-insert missing file data.
+        assert_eq!(
+            tb.generic_file_data_insert(&mut user, 0, file_data)
+                .expect("file data insertion failed"),
+            0
+        );
+        tb.check_completion_indication_success(&mut user);
+        assert_eq!(tb.pdu_queue_len(), 2);
+        tb.check_eof_ack_pdu(ConditionCode::NoError);
+        tb.check_finished_pdu_success();
+        tb.acknowledge_finished_pdu(&mut user, &transfer_info);
+    }
+    */
 }