Merge pull request #145 from robamu-org/simplify-dest-handler

Bugfix for destination handler: Transfer cancellation

CHANGELOG.md

@@ -9,6 +9,10 @@ Starting from v4.0.0, this project adheres to [Semantic Versioning](http://semve
 
 # [unreleased]
 
+## Fixed
+
+- Fixed bug for acknowledged file transfer cancellation in the destination handler.
+
 # [v8.0.0rc0] 2023-11-29
 
 ## Added

tests/cfdp/test_dest_handler.py

@@ -11,6 +11,7 @@
     ConditionCode,
     Direction,
     DirectiveType,
+    FinishedParams,
     PduConfig,
     PduType,
     TransactionId,
@@ -258,6 +259,7 @@ def _generic_no_error_finished_pdu_check(
         fsm_res: FsmResult,
         expected_step: TransactionStep = TransactionStep.SENDING_FINISHED_PDU,
         expected_file_status: FileStatus = FileStatus.FILE_RETAINED,
+        expected_condition_code: ConditionCode = ConditionCode.NO_ERROR,
     ) -> FinishedPdu:
         self._state_checker(fsm_res, 1, CfdpState.BUSY, expected_step)
         self.assertTrue(fsm_res.states.packets_ready)
@@ -267,9 +269,12 @@ def _generic_no_error_finished_pdu_check(
         self.assertEqual(next_pdu.pdu_directive_type, DirectiveType.FINISHED_PDU)
 
         finished_pdu = next_pdu.to_finished_pdu()
-        self.assertEqual(finished_pdu.condition_code, ConditionCode.NO_ERROR)
+        self.assertEqual(finished_pdu.condition_code, expected_condition_code)
         self.assertEqual(finished_pdu.file_status, expected_file_status)
-        self.assertEqual(finished_pdu.delivery_code, DeliveryCode.DATA_COMPLETE)
+        if expected_condition_code == ConditionCode.NO_ERROR:
+            self.assertEqual(finished_pdu.delivery_code, DeliveryCode.DATA_COMPLETE)
+        else:
+            self.assertEqual(finished_pdu.delivery_code, DeliveryCode.DATA_INCOMPLETE)
         self.assertEqual(finished_pdu.direction, Direction.TOWARDS_SENDER)
         self.assertIsNone(finished_pdu.fault_location)
         self.assertEqual(len(finished_pdu.file_store_responses), 0)
@@ -279,10 +284,14 @@ def _generic_verify_transfer_completion(
         self,
         fsm_res: FsmResult,
         expected_file_data: Optional[bytes],
-        expected_file_status: FileStatus = FileStatus.FILE_RETAINED,
+        expected_finished_params: FinishedParams = FinishedParams(
+            condition_code=ConditionCode.NO_ERROR,
+            file_status=FileStatus.FILE_RETAINED,
+            delivery_code=DeliveryCode.DATA_COMPLETE,
+        ),
     ):
-        self._generic_transfer_finished_indication_success_check(
-            fsm_res, expected_file_status
+        self._generic_transfer_finished_indication_check(
+            fsm_res, expected_finished_params
         )
         if expected_file_data is not None:
             self.assertTrue(self.dest_file_path.exists())
@@ -299,26 +308,20 @@ def _generic_verify_transfer_completion(
                 fsm_res, 0, CfdpState.BUSY, TransactionStep.WAITING_FOR_FINISHED_ACK
             )
 
-    def _generic_transfer_finished_indication_success_check(
-        self,
-        fsm_res: FsmResult,
-        expected_file_status: FileStatus = FileStatus.FILE_RETAINED,
+    def _generic_transfer_finished_indication_check(
+        self, fsm_res: FsmResult, expected_finished_params: FinishedParams
     ):
         self.cfdp_user.transaction_finished_indication.assert_called_once()
-        finished_params = cast(
+        finished_params_from_callback = cast(
             TransactionFinishedParams,
             self.cfdp_user.transaction_finished_indication.call_args.args[0],
         )
-        self.assertEqual(finished_params.transaction_id, self.transaction_id)
-        self.assertEqual(fsm_res.states.transaction_id, self.transaction_id)
-        self.assertEqual(
-            finished_params.finished_params.condition_code, ConditionCode.NO_ERROR
-        )
         self.assertEqual(
-            finished_params.finished_params.delivery_code, DeliveryCode.DATA_COMPLETE
+            finished_params_from_callback.transaction_id, self.transaction_id
         )
+        self.assertEqual(fsm_res.states.transaction_id, self.transaction_id)
         self.assertEqual(
-            finished_params.finished_params.file_status, expected_file_status
+            finished_params_from_callback.finished_params, expected_finished_params
         )
 
     def _generate_put_response_opts(self) -> TlvList:

tests/cfdp/test_dest_handler_acked.py

@@ -11,6 +11,7 @@
 )
 from spacepackets.cfdp.pdu import (
     AckPdu,
+    EofPdu,
     FinishedPdu,
     TransactionStatus,
     DeliveryCode,
@@ -57,6 +58,40 @@ def test_acked_small_file_transfer(self):
         self._generic_verify_transfer_completion(fsm_res, file_content)
         self._generic_insert_finished_pdu_ack(finished_pdu)
 
+    def test_cancelled_file_transfer(self):
+        file_content = "Hello World!".encode()
+        with open(self.src_file_path, "wb") as of:
+            of.write(file_content)
+        # Basic acknowledged empty file transfer.
+        self._generic_regular_transfer_init(len(file_content))
+        # Cancel the transfer by sending an EOF PDU with the appropriate parameters.
+        eof_pdu = EofPdu(
+            file_size=0,
+            file_checksum=NULL_CHECKSUM_U32,
+            pdu_conf=self.src_pdu_conf,
+            condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED,
+        )
+        self.dest_handler.insert_packet(eof_pdu)
+        fsm_res = self.dest_handler.state_machine()
+        # Should contain an ACK PDU now.
+        self._generic_verify_eof_ack_packet(
+            fsm_res, condition_code_of_acked_pdu=ConditionCode.CANCEL_REQUEST_RECEIVED
+        )
+        fsm_res = self.dest_handler.state_machine()
+        finished_pdu = self._generic_no_error_finished_pdu_check(
+            fsm_res, expected_condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED
+        )
+        self._generic_verify_transfer_completion(
+            fsm_res,
+            expected_file_data=None,
+            expected_finished_params=FinishedParams(
+                condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED,
+                delivery_code=DeliveryCode.DATA_INCOMPLETE,
+                file_status=FileStatus.FILE_RETAINED,
+            ),
+        )
+        self._generic_insert_finished_pdu_ack(finished_pdu)
+
     def test_deferred_missing_file_segment_handling(self):
         file_content = "Hello World!".encode()
         with open(self.src_file_path, "wb") as of:
@@ -459,7 +494,11 @@ def test_metadata_only_transfer(self):
         self._generic_verify_transfer_completion(
             fsm_res,
             expected_file_data=None,
-            expected_file_status=FileStatus.FILE_STATUS_UNREPORTED,
+            expected_finished_params=FinishedParams(
+                condition_code=ConditionCode.NO_ERROR,
+                file_status=FileStatus.FILE_STATUS_UNREPORTED,
+                delivery_code=DeliveryCode.DATA_COMPLETE,
+            ),
         )
         self._generic_insert_finished_pdu_ack(finished_pdu)
 
@@ -497,7 +536,11 @@ def _generic_verify_missing_segment_requested(
         self.assertEqual(nak_pdu.end_of_scope, end_of_scope)
         self.assertEqual(nak_pdu.segment_requests, segment_reqs)
 
-    def _generic_verify_eof_ack_packet(self, fsm_res: FsmResult):
+    def _generic_verify_eof_ack_packet(
+        self,
+        fsm_res: FsmResult,
+        condition_code_of_acked_pdu: ConditionCode = ConditionCode.NO_ERROR,
+    ):
         self._state_checker(
             fsm_res,
             1,
@@ -510,7 +553,9 @@ def _generic_verify_eof_ack_packet(self, fsm_res: FsmResult):
         self.assertEqual(next_pdu.pdu_directive_type, DirectiveType.ACK_PDU)
         ack_pdu = next_pdu.to_ack_pdu()
         self.assertEqual(ack_pdu.directive_code_of_acked_pdu, DirectiveType.EOF_PDU)
-        self.assertEqual(ack_pdu.condition_code_of_acked_pdu, ConditionCode.NO_ERROR)
+        self.assertEqual(
+            ack_pdu.condition_code_of_acked_pdu, condition_code_of_acked_pdu
+        )
         self.assertEqual(ack_pdu.transaction_status, TransactionStatus.ACTIVE)
 
     def _generic_insert_finished_pdu_ack(self, finished_pdu: FinishedPdu):

tests/cfdp/test_dest_handler_naked.py

@@ -155,6 +155,37 @@ def test_check_timer_mechanism(self):
             TransactionStep.IDLE,
         )
 
+    def test_cancelled_transfer(self):
+        data = "Hello World\n".encode()
+        with open(self.src_file_path, "wb") as of:
+            of.write(data)
+        file_size = self.src_file_path.stat().st_size
+        self._generic_regular_transfer_init(
+            file_size=file_size,
+        )
+        self._insert_file_segment(segment=data, offset=0)
+        # Cancel the transfer by sending an EOF PDU with the appropriate parameters.
+        eof_pdu = EofPdu(
+            file_size=0,
+            file_checksum=NULL_CHECKSUM_U32,
+            pdu_conf=self.src_pdu_conf,
+            condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED,
+        )
+        self.dest_handler.insert_packet(eof_pdu)
+        fsm_res = self.dest_handler.state_machine()
+        self._generic_eof_recv_indication_check(fsm_res)
+        if self.closure_requested:
+            self._generic_no_error_finished_pdu_check(fsm_res)
+        self._generic_verify_transfer_completion(
+            fsm_res,
+            expected_file_data=None,
+            expected_finished_params=FinishedParams(
+                condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED,
+                delivery_code=DeliveryCode.DATA_INCOMPLETE,
+                file_status=FileStatus.FILE_RETAINED,
+            ),
+        )
+
     def test_check_limit_reached(self):
         data = "Hello World\n".encode()
         self._generic_check_limit_test(data)
@@ -235,7 +266,7 @@ def test_file_data_pdu_before_metadata_is_discarded(self):
         )
         # At least one segment was stored
         self.assertEqual(
-            finished_args.finished_params.delivery_status,
+            finished_args.finished_params.file_status,
             FileStatus.FILE_RETAINED,
         )
         self.assertEqual(