✨ save_exchange_record should be Optional (#1195)

* ✨ `save_exchange_record` should be Optional

* ✨ Deduplicate `save_exchange_record`

Defines a helper class `SaveExchangeRecordField`, with `auto_remove` property, and is extended where needed

* 🐛 Fix class inheritance

app/models/issuer.py

@@ -4,6 +4,7 @@
 from aries_cloudcontroller import LDProofVCDetail, TxnOrPublishRevocationsResult
 from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator
 
+from app.util.save_exchange_record import SaveExchangeRecordField
 from shared.exceptions import CloudApiValueError
 
 
@@ -18,14 +19,10 @@ class IndyCredential(BaseModel):
     attributes: Dict[str, str]
 
 
-class CredentialBase(BaseModel):
+class CredentialBase(SaveExchangeRecordField):
     type: CredentialType = CredentialType.INDY
     indy_credential_detail: Optional[IndyCredential] = None
     ld_credential_detail: Optional[LDProofVCDetail] = None
-    save_exchange_record: bool = Field(
-        default=False,
-        description="Whether the credential exchange record should be saved on completion",
-    )
 
     @field_validator("indy_credential_detail", mode="before")
     @classmethod

app/models/verifier.py

@@ -5,6 +5,7 @@
 from aries_cloudcontroller import IndyProofRequest as AcaPyIndyProofRequest
 from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator
 
+from app.util.save_exchange_record import SaveExchangeRecordField
 from shared.exceptions import CloudApiValueError
 
 
@@ -62,11 +63,10 @@ class ProofRequestMetadata(BaseModel):
     comment: Optional[str] = None
 
 
-class CreateProofRequest(ProofRequestBase, ProofRequestMetadata):
-    save_exchange_record: bool = Field(
-        default=False,
-        description="Whether the presentation exchange record should be saved on completion",
-    )
+class CreateProofRequest(
+    ProofRequestBase, ProofRequestMetadata, SaveExchangeRecordField
+):
+    pass
 
 
 class SendProofRequest(CreateProofRequest):
@@ -77,14 +77,10 @@ class ProofId(BaseModel):
     proof_id: str
 
 
-class AcceptProofRequest(ProofId):
+class AcceptProofRequest(ProofId, SaveExchangeRecordField):
     type: ProofRequestType = ProofRequestType.INDY
     indy_presentation_spec: Optional[IndyPresSpec] = None
     dif_presentation_spec: Optional[DIFPresSpec] = None
-    save_exchange_record: bool = Field(
-        default=False,
-        description="Whether the presentation exchange record should be saved on completion",
-    )
 
     @field_validator("indy_presentation_spec", mode="before")
     @classmethod

app/routes/issuer.py

@@ -409,8 +409,8 @@ async def get_credential(
     NB: An issuer and a holder will have distinct credential exchange ids, despite referring to the same exchange.
     The `thread_id` is the only record attribute that will be the same for the holder and the issuer.
 
-    An exchange record will automatically be deleted after a flow completes (i.e. when state is 'done'),
-    unless the `save_exchange_record` was set to true.
+    An exchange record will, by default, automatically be deleted after a flow completes (i.e. when state is 'done'),
+    unless the `save_exchange_record` was set to true, or the wallet is configured to preserve records by default.
 
     The following parameters can be set to filter the fetched exchange records: connection_id, role, state, thread_id.
 

app/services/issuer/acapy_issuer_v2.py

@@ -66,7 +66,7 @@ async def send_credential(
 
         bound_logger.debug("Issue v2 credential (automated)")
         request_body = V20CredExFree(
-            auto_remove=not credential.save_exchange_record,
+            auto_remove=credential.auto_remove,
             connection_id=credential.connection_id,
             filter=cred_filter,
             credential_preview=credential_preview,
@@ -113,7 +113,7 @@ async def create_offer(
 
         bound_logger.debug("Creating v2 credential offer")
         request_body = V20CredOfferConnFreeRequest(
-            auto_remove=not credential.save_exchange_record,
+            auto_remove=credential.auto_remove,
             credential_preview=credential_preview,
             filter=cred_filter,
         )

app/services/verifier/acapy_verifier_v2.py

@@ -53,7 +53,7 @@ async def create_proof_request(
         bound_logger = logger.bind(body=create_proof_request)
         bound_logger.debug("Creating v2 proof request")
         request_body = V20PresCreateRequestRequest(
-            auto_remove=not create_proof_request.save_exchange_record,
+            auto_remove=create_proof_request.auto_remove,
             presentation_request=presentation_request,
             auto_verify=True,
             comment=create_proof_request.comment,
@@ -95,7 +95,7 @@ async def send_proof_request(
 
         bound_logger = logger.bind(body=send_proof_request)
         request_body = V20PresSendRequestRequest(
-            auto_remove=not send_proof_request.save_exchange_record,
+            auto_remove=send_proof_request.auto_remove,
             connection_id=send_proof_request.connection_id,
             presentation_request=presentation_request,
             auto_verify=True,
@@ -121,7 +121,8 @@ async def send_proof_request(
     async def accept_proof_request(
         cls, controller: AcaPyClient, accept_proof_request: AcceptProofRequest
     ) -> PresentationExchange:
-        auto_remove = not accept_proof_request.save_exchange_record
+        auto_remove = accept_proof_request.auto_remove
+
         if accept_proof_request.type == ProofRequestType.INDY:
             presentation_spec = V20PresSpecByFormatRequest(
                 auto_remove=auto_remove,

app/tests/e2e/issuer/test_save_exchange_record.py

@@ -1,5 +1,6 @@
 import asyncio
 import json
+from typing import Optional
 
 import pytest
 from fastapi import HTTPException
@@ -15,13 +16,13 @@
 
 
 @pytest.mark.anyio
-@pytest.mark.parametrize("save_exchange_record", [False, True])
+@pytest.mark.parametrize("save_exchange_record", [None, False, True])
 async def test_issue_credential_with_save_exchange_record(
     faber_client: RichAsyncClient,
     credential_definition_id: str,
     faber_and_alice_connection: FaberAliceConnect,
     alice_member_client: RichAsyncClient,
-    save_exchange_record: bool,
+    save_exchange_record: Optional[bool],
 ) -> CredentialExchange:
     credential = {
         "connection_id": faber_and_alice_connection.faber_connection_id,

app/tests/e2e/verifier/test_verifier.py

@@ -1,6 +1,7 @@
 import asyncio
 import json
 import time
+from typing import Optional
 
 import pytest
 from aries_cloudcontroller import IndyPresSpec, IndyRequestedCredsRequestedAttr
@@ -544,16 +545,16 @@ async def test_accept_proof_request_verifier_has_issuer_role(
 
 
 @pytest.mark.anyio
-@pytest.mark.parametrize("acme_save_exchange_record", [False, True])
-@pytest.mark.parametrize("alice_save_exchange_record", [False, True])
+@pytest.mark.parametrize("acme_save_exchange_record", [None, False, True])
+@pytest.mark.parametrize("alice_save_exchange_record", [None, False, True])
 async def test_saving_of_presentation_exchange_records(
     issue_credential_to_alice: CredentialExchange,  # pylint: disable=unused-argument
     credential_definition_id: str,
     alice_member_client: RichAsyncClient,
     acme_client: RichAsyncClient,
     acme_and_alice_connection: AcmeAliceConnect,
-    acme_save_exchange_record: bool,
-    alice_save_exchange_record: bool,
+    acme_save_exchange_record: Optional[bool],
+    alice_save_exchange_record: Optional[bool],
 ):
     request_body = {
         "connection_id": acme_and_alice_connection.acme_connection_id,

app/util/save_exchange_record.py

@@ -0,0 +1,22 @@
+from typing import Optional
+
+from pydantic import BaseModel, Field
+
+save_exchange_record_field = Field(
+    default=None,
+    description=(
+        "Controls exchange record retention after exchange is complete. None uses "
+        "wallet default (typically to delete), true forces save, false forces delete."
+    ),
+)
+
+
+class SaveExchangeRecordField(BaseModel):
+    save_exchange_record: Optional[bool] = save_exchange_record_field
+
+    @property
+    def auto_remove(self) -> Optional[bool]:
+        """Returns the inverse of save_exchange_record if set, otherwise None."""
+        if self.save_exchange_record is None:
+            return None
+        return not self.save_exchange_record