From bdfaf74ea69e82a75f07c6f0e5e6f33826cbdb0f Mon Sep 17 00:00:00 2001 From: Mourits de Beer <31511766+ff137@users.noreply.github.com> Date: Fri, 22 Nov 2024 23:47:53 +0200 Subject: [PATCH] :sparkles: `save_exchange_record` should be Optional (#1195) * :sparkles: `save_exchange_record` should be Optional * :sparkles: Deduplicate `save_exchange_record` Defines a helper class `SaveExchangeRecordField`, with `auto_remove` property, and is extended where needed * :bug: Fix class inheritance --- app/models/issuer.py | 7 ++---- app/models/verifier.py | 16 +++++--------- app/routes/issuer.py | 4 ++-- app/services/issuer/acapy_issuer_v2.py | 4 ++-- app/services/verifier/acapy_verifier_v2.py | 7 +++--- .../e2e/issuer/test_save_exchange_record.py | 5 +++-- app/tests/e2e/verifier/test_verifier.py | 9 ++++---- app/util/save_exchange_record.py | 22 +++++++++++++++++++ 8 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 app/util/save_exchange_record.py diff --git a/app/models/issuer.py b/app/models/issuer.py index ea47169c1..5564b1491 100644 --- a/app/models/issuer.py +++ b/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 diff --git a/app/models/verifier.py b/app/models/verifier.py index 69c015733..c7748cd75 100644 --- a/app/models/verifier.py +++ b/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 diff --git a/app/routes/issuer.py b/app/routes/issuer.py index 0167d344e..1203c86aa 100644 --- a/app/routes/issuer.py +++ b/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. diff --git a/app/services/issuer/acapy_issuer_v2.py b/app/services/issuer/acapy_issuer_v2.py index f9df4e351..569a6c8b5 100644 --- a/app/services/issuer/acapy_issuer_v2.py +++ b/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, ) diff --git a/app/services/verifier/acapy_verifier_v2.py b/app/services/verifier/acapy_verifier_v2.py index 5eceaf5a5..a4ce86a69 100644 --- a/app/services/verifier/acapy_verifier_v2.py +++ b/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, diff --git a/app/tests/e2e/issuer/test_save_exchange_record.py b/app/tests/e2e/issuer/test_save_exchange_record.py index 32edf6843..4c59d2dbb 100644 --- a/app/tests/e2e/issuer/test_save_exchange_record.py +++ b/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, diff --git a/app/tests/e2e/verifier/test_verifier.py b/app/tests/e2e/verifier/test_verifier.py index c97c4b387..0fd465bcb 100644 --- a/app/tests/e2e/verifier/test_verifier.py +++ b/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, diff --git a/app/util/save_exchange_record.py b/app/util/save_exchange_record.py new file mode 100644 index 000000000..71ea564e9 --- /dev/null +++ b/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