Merge pull request #3947 from TheTechmage/feat/auto-remove-failed

Author: swcurran

acapy_agent/config/argparse.py

@@ -1292,6 +1292,13 @@ def add_arguments(self, parser: ArgumentParser):
             help="Keep credential and presentation exchange records after "
             "exchange has completed.",
         )
+        parser.add_argument(
+            "--no-preserve-failed-exchange-records",
+            action="store_true",
+            env_var="ACAPY_NO_PRESERVE_FAILED_EXCHANGE_RECORDS",
+            help="Remove failed credential and presentation exchange records "
+            "upon failure.",
+        )
         parser.add_argument(
             "--emit-new-didcomm-prefix",
             action="store_true",

acapy_agent/protocols/issue_credential/v2_0/manager.py

@@ -54,6 +54,7 @@ async def prepare_send(
         cred_proposal: V20CredProposal,
         verification_method: Optional[str] = None,
         auto_remove: Optional[bool] = None,
+        auto_remove_on_failure: Optional[bool] = None,
         replacement_id: Optional[str] = None,
     ) -> Tuple[V20CredExRecord, V20CredOffer]:
         """Set up a new credential exchange record for an automated send.
@@ -63,6 +64,7 @@ async def prepare_send(
             cred_proposal: credential proposal with preview
             verification_method: an optional verification method to be used when issuing
             auto_remove: flag to remove the record automatically on completion
+            auto_remove_on_failure: flag to remove the record automatically on failure
             replacement_id: identifier to help coordinate credential replacement
 
         Returns:
@@ -71,6 +73,10 @@ async def prepare_send(
         """
         if auto_remove is None:
             auto_remove = not self._profile.settings.get("preserve_exchange_records")
+        if auto_remove_on_failure is None:
+            auto_remove_on_failure = bool(
+                self._profile.settings.get("no_preserve_failed_exchange_records")
+            )
         cred_ex_record = V20CredExRecord(
             connection_id=connection_id,
             verification_method=verification_method,
@@ -79,6 +85,7 @@ async def prepare_send(
             cred_proposal=cred_proposal,
             auto_issue=True,
             auto_remove=auto_remove,
+            auto_remove_on_failure=auto_remove_on_failure,
             trace=(cred_proposal._trace is not None),
         )
         return await self.create_offer(
@@ -766,4 +773,7 @@ async def receive_problem_report(
             cred_ex_record.error_msg = f"{code}: {message.description.get('en', code)}"
             await cred_ex_record.save(session, reason="received problem report")
 
+        if cred_ex_record.auto_remove_on_failure:
+            await self.delete_cred_ex_record(cred_ex_record.cred_ex_id)
+
         return cred_ex_record

acapy_agent/protocols/issue_credential/v2_0/models/cred_ex_record.py

@@ -69,6 +69,7 @@ def __init__(
         auto_offer: bool = False,
         auto_issue: bool = False,
         auto_remove: bool = True,
+        auto_remove_on_failure: bool = False,
         error_msg: Optional[str] = None,
         trace: bool = False,  # backward compat: BaseRecord.from_storage()
         cred_id_stored: Optional[
@@ -95,6 +96,7 @@ def __init__(
         self.auto_offer = auto_offer
         self.auto_issue = auto_issue
         self.auto_remove = auto_remove
+        self.auto_remove_on_failure = auto_remove_on_failure
         self.error_msg = error_msg
 
     @property
@@ -225,6 +227,7 @@ def record_value(self) -> Mapping:
                     "auto_offer",
                     "auto_issue",
                     "auto_remove",
+                    "auto_remove_on_failure",
                     "error_msg",
                     "trace",
                 )
@@ -428,6 +431,16 @@ class Meta:
             "example": False,
         },
     )
+    auto_remove_on_failure = fields.Bool(
+        required=False,
+        dump_default=True,
+        metadata={
+            "description": (
+                "Issuer choice to remove this credential exchange record when failed"
+            ),
+            "example": False,
+        },
+    )
     error_msg = fields.Str(
         required=False,
         metadata={"description": "Error message", "example": "The front fell off"},

acapy_agent/protocols/issue_credential/v2_0/routes.py

@@ -332,6 +332,15 @@ class V20IssueCredSchemaCore(AdminAPIMessageTracingSchema):
             )
         },
     )
+    auto_remove_on_failure = fields.Bool(
+        required=False,
+        metadata={
+            "description": (
+                "Whether to remove the credential exchange record on failure"
+                " (overrides --no-preserve-failed-exchange-records configuration setting)"
+            )
+        },
+    )
     comment = fields.Str(
         required=False,
         allow_none=True,
@@ -393,6 +402,15 @@ class V20CredRequestFreeSchema(AdminAPIMessageTracingSchema):
             )
         },
     )
+    auto_remove_on_failure = fields.Bool(
+        required=False,
+        metadata={
+            "description": (
+                "Whether to remove the credential exchange record on failure"
+                " (overrides --no-preserve-failed-exchange-records configuration setting)"
+            )
+        },
+    )
     comment = fields.Str(
         required=False,
         allow_none=True,
@@ -512,6 +530,16 @@ class V20CredRequestRequestSchema(OpenAPISchema):
             )
         },
     )
+    auto_remove_on_failure = fields.Bool(
+        required=False,
+        dump_default=False,
+        metadata={
+            "description": (
+                "Whether to remove the credential exchange record on failure"
+                " (overrides --no-preserve-failed-exchange-records configuration setting)"
+            )
+        },
+    )
 
 
 class V20CredIssueRequestSchema(OpenAPISchema):
@@ -735,6 +763,10 @@ async def credential_exchange_create(request: web.BaseRequest):
     auto_remove = body.get(
         "auto_remove", not profile.settings.get("preserve_exchange_records")
     )
+    auto_remove_on_failure = body.get(
+        "auto_remove_on_failure",
+        profile.settings.get("no_preserve_failed_exchange_records"),
+    )
     if not filt_spec:
         raise web.HTTPBadRequest(reason="Missing filter")
     trace_msg = body.get("trace")
@@ -763,6 +795,7 @@ async def credential_exchange_create(request: web.BaseRequest):
             connection_id=None,
             cred_proposal=cred_proposal,
             auto_remove=auto_remove,
+            auto_remove_on_failure=auto_remove_on_failure,
         )
     except (StorageError, BaseModelError) as err:
         raise web.HTTPBadRequest(reason=err.roll_up) from err
@@ -1338,6 +1371,10 @@ async def credential_exchange_send_free_request(request: web.BaseRequest):
     auto_remove = body.get(
         "auto_remove", not profile.settings.get("preserve_exchange_records")
     )
+    auto_remove_on_failure = body.get(
+        "auto_remove_on_failure",
+        profile.settings.get("no_preserve_failed_exchange_records"),
+    )
     trace_msg = body.get("trace")
     holder_did = body.get("holder_did")
 
@@ -1362,6 +1399,7 @@ async def credential_exchange_send_free_request(request: web.BaseRequest):
         cred_ex_record = V20CredExRecord(
             connection_id=connection_id,
             auto_remove=auto_remove,
+            auto_remove_on_failure=auto_remove_on_failure,
             cred_proposal=cred_proposal.serialize(),
             initiator=V20CredExRecord.INITIATOR_SELF,
             role=V20CredExRecord.ROLE_HOLDER,
@@ -1433,9 +1471,16 @@ async def credential_exchange_send_bound_request(request: web.BaseRequest):
         auto_remove = body.get(
             "auto_remove", not profile.settings.get("preserve_exchange_records")
         )
+        auto_remove_on_failure = body.get(
+            "auto_remove_on_failure",
+            profile.settings.get("no_preserve_failed_exchange_records"),
+        )
     except JSONDecodeError:
         holder_did = None
         auto_remove = not profile.settings.get("preserve_exchange_records")
+        auto_remove_on_failure = profile.settings.get(
+            "no_preserve_failed_exchange_records"
+        )
 
     cred_ex_id = request.match_info["cred_ex_id"]
 
@@ -1479,6 +1524,7 @@ async def credential_exchange_send_bound_request(request: web.BaseRequest):
 
         # assign the auto_remove flag from above...
         cred_ex_record.auto_remove = auto_remove
+        cred_ex_record.auto_remove_on_failure = auto_remove_on_failure
 
         cred_manager = V20CredManager(profile)
         cred_ex_record, cred_request_message = await cred_manager.create_request(

acapy_agent/protocols/issue_credential/v2_0/tests/test_manager.py

@@ -1466,6 +1466,48 @@ async def test_receive_problem_report_x(self):
             with self.assertRaises(test_module.StorageNotFoundError):
                 await self.manager.receive_problem_report(problem, connection_id)
 
+    async def test_receive_problem_report_removal(self):
+        connection_id = "connection-id"
+        stored_exchange = V20CredExRecord(
+            cred_ex_id="dummy-cxid",
+            connection_id=connection_id,
+            initiator=V20CredExRecord.INITIATOR_SELF,
+            role=V20CredExRecord.ROLE_ISSUER,
+            auto_remove_on_failure=True,
+        )
+        problem = V20CredProblemReport(
+            description={
+                "code": test_module.ProblemReportReason.ISSUANCE_ABANDONED.value,
+                "en": "Insufficient privilege",
+            }
+        )
+
+        with (
+            mock.patch.object(V20CredExRecord, "save", autospec=True) as save_ex,
+            mock.patch.object(
+                V20CredExRecord,
+                "retrieve_by_conn_and_thread",
+                mock.CoroutineMock(),
+            ) as retrieve_ex,
+            mock.patch.object(
+                V20CredExRecord, "retrieve_by_id", mock.CoroutineMock()
+            ) as mock_retrieve,
+            mock.patch.object(
+                V20CredExRecord, "delete_record", autospec=True
+            ) as delete_ex,
+        ):
+            retrieve_ex.return_value = stored_exchange
+            mock_retrieve.return_value = stored_exchange
+
+            ret_exchange = await self.manager.receive_problem_report(
+                problem, connection_id
+            )
+            retrieve_ex.assert_called()
+            save_ex.assert_called_once()
+            delete_ex.assert_called_once()
+
+            assert ret_exchange.state == V20CredExRecord.STATE_ABANDONED
+
     async def test_retrieve_records(self):
         self.profile.context.injector.bind_instance(InMemoryCache, InMemoryCache())
 

acapy_agent/protocols/present_proof/v2_0/manager.py

@@ -41,6 +41,7 @@ async def create_exchange_for_proposal(
         pres_proposal_message: V20PresProposal,
         auto_present: Optional[bool] = None,
         auto_remove: Optional[bool] = None,
+        auto_remove_on_failure: Optional[bool] = None,
     ):
         """Create a presentation exchange record for input presentation proposal.
 
@@ -51,13 +52,19 @@ async def create_exchange_for_proposal(
             auto_present: whether to present proof upon receiving proof request
                 (default to configuration setting)
             auto_remove: whether to remove this presentation exchange upon completion
+            auto_remove_on_failure: whether to remove this presentation exchange upon
+                failure
 
         Returns:
             Presentation exchange record, created
 
         """
         if auto_remove is None:
             auto_remove = not self._profile.settings.get("preserve_exchange_records")
+        if auto_remove_on_failure is None:
+            auto_remove_on_failure = bool(
+                self._profile.settings.get("no_preserve_failed_exchange_records")
+            )
         pres_ex_record = V20PresExRecord(
             connection_id=connection_id,
             thread_id=pres_proposal_message._thread_id,
@@ -68,6 +75,7 @@ async def create_exchange_for_proposal(
             auto_present=auto_present,
             trace=(pres_proposal_message._trace is not None),
             auto_remove=auto_remove,
+            auto_remove_on_failure=auto_remove_on_failure,
         )
 
         async with self._profile.session() as session:
@@ -162,6 +170,7 @@ async def create_exchange_for_request(
         pres_request_message: V20PresRequest,
         auto_verify: Optional[bool] = None,
         auto_remove: Optional[bool] = None,
+        auto_remove_on_failure: Optional[bool] = None,
     ):
         """Create a presentation exchange record for input presentation request.
 
@@ -171,13 +180,19 @@ async def create_exchange_for_request(
                 exchange record, extracting indy proof request and thread id
             auto_verify: whether to auto-verify presentation exchange
             auto_remove: whether to remove this presentation exchange upon completion
+            auto_remove_on_failure: whether to remove this presentation exchange upon
+                failure
 
         Returns:
             Presentation exchange record, updated
 
         """
         if auto_remove is None:
             auto_remove = not self._profile.settings.get("preserve_exchange_records")
+        if auto_remove_on_failure is None:
+            auto_remove_on_failure = bool(
+                self._profile.settings.get("no_preserve_failed_exchange_records")
+            )
         pres_ex_record = V20PresExRecord(
             connection_id=connection_id,
             thread_id=pres_request_message._thread_id,
@@ -188,6 +203,7 @@ async def create_exchange_for_request(
             auto_verify=auto_verify,
             trace=(pres_request_message._trace is not None),
             auto_remove=auto_remove,
+            auto_remove_on_failure=auto_remove_on_failure,
         )
         async with self._profile.session() as session:
             await pres_ex_record.save(
@@ -494,4 +510,8 @@ async def receive_problem_report(
             pres_ex_record.error_msg = f"{code}: {message.description.get('en', code)}"
             await pres_ex_record.save(session, reason="received problem report")
 
+            # all done: delete
+            if pres_ex_record.auto_remove_on_failure:
+                await pres_ex_record.delete_record(session)
+
         return pres_ex_record

acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py

@@ -67,6 +67,7 @@ def __init__(
         trace: bool = False,  # backward compat: BaseRecord.FromStorage()
         by_format: Optional[Mapping] = None,  # backward compat: BaseRecord.FromStorage()
         auto_remove: bool = False,
+        auto_remove_on_failure: bool = False,
         **kwargs,
     ):
         """Initialize a new PresExRecord."""
@@ -85,6 +86,7 @@ def __init__(
         self.auto_verify = auto_verify
         self.error_msg = error_msg
         self.auto_remove = auto_remove
+        self.auto_remove_on_failure = auto_remove_on_failure
 
     @property
     def pres_ex_id(self) -> str:
@@ -225,6 +227,7 @@ def record_value(self) -> Mapping:
                     "error_msg",
                     "trace",
                     "auto_remove",
+                    "auto_remove_on_failure",
                 )
             },
             **{
@@ -366,3 +369,13 @@ class Meta:
             "example": False,
         },
     )
+    auto_remove_on_failure = fields.Bool(
+        required=False,
+        dump_default=True,
+        metadata={
+            "description": (
+                "Verifier choice to remove this presentation exchange record when failed"
+            ),
+            "example": False,
+        },
+    )

acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py

@@ -109,6 +109,7 @@ async def test_record(self):
             "error_msg": "error",
             "trace": False,
             "auto_remove": True,
+            "auto_remove_on_failure": False,
         }
 
         bx_record = BasexRecordImpl()