From 072acaa9dc027ffd01df08ecd3f65da938946ef9 Mon Sep 17 00:00:00 2001 From: jamshale Date: Thu, 18 Jul 2024 17:54:50 +0000 Subject: [PATCH 1/4] Fix publishing multiple rev reg defs with endorsement Signed-off-by: jamshale --- aries_cloudagent/revocation/manager.py | 18 +++-- aries_cloudagent/revocation/routes.py | 93 +++++++++++--------------- 2 files changed, 49 insertions(+), 62 deletions(-) diff --git a/aries_cloudagent/revocation/manager.py b/aries_cloudagent/revocation/manager.py index e5505ea37f..e90b1faa63 100644 --- a/aries_cloudagent/revocation/manager.py +++ b/aries_cloudagent/revocation/manager.py @@ -283,7 +283,7 @@ async def publish_pending_revocations( """ result = {} issuer = self._profile.inject(IndyIssuer) - rev_entry_resp = None + rev_entry_responses = [] async with self._profile.session() as session: issuer_rr_recs = await IssuerRevRegRecord.query_by_pending(session) @@ -331,20 +331,24 @@ async def publish_pending_revocations( session, "endorser_info" ) endorser_did = endorser_info["endorser_did"] - rev_entry_resp = await issuer_rr_upd.send_entry( - self._profile, - write_ledger=write_ledger, - endorser_did=endorser_did, + rev_entry_responses.append( + await issuer_rr_upd.send_entry( + self._profile, + write_ledger=write_ledger, + endorser_did=endorser_did, + ) ) else: - rev_entry_resp = await issuer_rr_upd.send_entry(self._profile) + rev_entry_responses.append( + await issuer_rr_upd.send_entry(self._profile) + ) await notify_revocation_published_event( self._profile, issuer_rr_rec.revoc_reg_id, crids ) published = sorted(crid for crid in crids if crid not in failed_crids) result[issuer_rr_rec.revoc_reg_id] = published - return rev_entry_resp, result + return rev_entry_responses, result async def clear_pending_revocations( self, purge: Mapping[Text, Sequence[Text]] = None diff --git a/aries_cloudagent/revocation/routes.py b/aries_cloudagent/revocation/routes.py index c2e0c13782..256a215176 100644 --- a/aries_cloudagent/revocation/routes.py +++ b/aries_cloudagent/revocation/routes.py @@ -118,9 +118,7 @@ class TxnOrRevRegResultSchema(OpenAPISchema): txn = fields.Nested( TransactionRecordSchema(), required=False, - metadata={ - "description": "Revocation registry definition transaction to endorse" - }, + metadata={"description": "Revocation registry definition transaction to endorse"}, ) @@ -213,13 +211,9 @@ def validate_fields(self, data, **kwargs): notify_version = data.get("notify_version", "v1_0") if notify and not connection_id: - raise ValidationError( - "Request must specify connection_id if notify is true" - ) + raise ValidationError("Request must specify connection_id if notify is true") if notify and not notify_version: - raise ValidationError( - "Request must specify notify_version if notify is true" - ) + raise ValidationError("Request must specify notify_version if notify is true") publish = fields.Boolean( required=False, @@ -652,7 +646,7 @@ async def publish_revocations(request: web.BaseRequest): if not endorser_conn_id: raise web.HTTPBadRequest(reason="No endorser connection found") try: - rev_reg_resp, result = await rev_manager.publish_pending_revocations( + rev_reg_responses, result = await rev_manager.publish_pending_revocations( rrid2crid=rrid2crid, write_ledger=write_ledger, connection_id=endorser_conn_id, @@ -660,30 +654,37 @@ async def publish_revocations(request: web.BaseRequest): except (RevocationError, StorageError, IndyIssuerError, LedgerError) as err: raise web.HTTPBadRequest(reason=err.roll_up) from err - if create_transaction_for_endorser and rev_reg_resp: - transaction_mgr = TransactionManager(profile) - try: - transaction = await transaction_mgr.create_record( - messages_attach=rev_reg_resp["result"], connection_id=endorser_conn_id - ) - except StorageError as err: - raise web.HTTPBadRequest(reason=err.roll_up) from err - - # if auto-request, send the request to the endorser - if context.settings.get_value("endorser.auto_request"): + txn_responses = [] + for response in rev_reg_responses: + if create_transaction_for_endorser: + transaction_mgr = TransactionManager(profile) try: - ( - transaction, - transaction_request, - ) = await transaction_mgr.create_request( - transaction=transaction, + transaction = await transaction_mgr.create_record( + messages_attach=response["result"], connection_id=endorser_conn_id ) - except (StorageError, TransactionManagerError) as err: + except StorageError as err: raise web.HTTPBadRequest(reason=err.roll_up) from err - await outbound_handler(transaction_request, connection_id=endorser_conn_id) + # if auto-request, send the request to the endorser + if context.settings.get_value("endorser.auto_request"): + try: + ( + transaction, + transaction_request, + ) = await transaction_mgr.create_request( + transaction=transaction, + ) + except (StorageError, TransactionManagerError) as err: + raise web.HTTPBadRequest(reason=err.roll_up) from err + + await outbound_handler( + transaction_request, connection_id=endorser_conn_id + ) + + txn_responses.append({"txn": transaction.serialize()}) + if txn_responses: + return web.json_response(txn_responses) - return web.json_response({"txn": transaction.serialize()}) return web.json_response({"rrid2crid": result}) @@ -824,9 +825,7 @@ async def rev_regs_created(request: web.BaseRequest): is_anoncreds_profile_raise_web_exception(context.profile) search_tags = list(vars(RevRegsCreatedQueryStringSchema)["_declared_fields"]) - tag_filter = { - tag: request.query[tag] for tag in search_tags if tag in request.query - } + tag_filter = {tag: request.query[tag] for tag in search_tags if tag in request.query} async with context.profile.session() as session: found = await IssuerRevRegRecord.query( session, @@ -835,11 +834,7 @@ async def rev_regs_created(request: web.BaseRequest): ) return web.json_response( - { - "rev_reg_ids": [ - record.revoc_reg_id for record in found if record.revoc_reg_id - ] - } + {"rev_reg_ids": [record.revoc_reg_id for record in found if record.revoc_reg_id]} ) @@ -1279,9 +1274,7 @@ async def send_rev_reg_def(request: web.BaseRequest): raise web.HTTPBadRequest(reason=err.roll_up) from err async with profile.session() as session: - endorser_info = await connection_record.metadata_get( - session, "endorser_info" - ) + endorser_info = await connection_record.metadata_get(session, "endorser_info") if not endorser_info: raise web.HTTPForbidden( reason=( @@ -1399,9 +1392,7 @@ async def send_rev_reg_entry(request: web.BaseRequest): except BaseModelError as err: raise web.HTTPBadRequest(reason=err.roll_up) from err - endorser_info = await connection_record.metadata_get( - session, "endorser_info" - ) + endorser_info = await connection_record.metadata_get(session, "endorser_info") if not endorser_info: raise web.HTTPForbidden( reason=( @@ -1566,9 +1557,7 @@ async def on_revocation_registry_init_event(profile: Profile, event: Event): # TODO error handling - for now just let exceptions get raised endorser_connection_id = meta_data["endorser"]["connection_id"] async with profile.session() as session: - connection = await ConnRecord.retrieve_by_id( - session, endorser_connection_id - ) + connection = await ConnRecord.retrieve_by_id(session, endorser_connection_id) endorser_info = await connection.metadata_get(session, "endorser_info") endorser_did = endorser_info["endorser_did"] write_ledger = False @@ -1640,9 +1629,7 @@ async def generate(rr_record: IssuerRevRegRecord) -> dict: registry_record = await IssuerRevRegRecord.retrieve_by_id(session, record_id) await shield(generate(registry_record)) - create_pending_rev_reg = meta_data["processing"].get( - "create_pending_rev_reg", False - ) + create_pending_rev_reg = meta_data["processing"].get("create_pending_rev_reg", False) if write_ledger and create_pending_rev_reg: revoc = IndyRevocation(profile) await revoc.init_issuer_registry( @@ -1730,17 +1717,13 @@ async def on_revocation_registry_endorsed_event(profile: Profile, event: Event): await registry_record.upload_tails_file(profile) # Post the initial revocation entry - await notify_revocation_entry_event( - profile, registry_record.record_id, meta_data - ) + await notify_revocation_entry_event(profile, registry_record.record_id, meta_data) # create a "pending" registry if one is requested # (this is done automatically when creating a credential definition, so that when a # revocation registry fills up, we can continue to issue credentials without a # delay) - create_pending_rev_reg = meta_data["processing"].get( - "create_pending_rev_reg", False - ) + create_pending_rev_reg = meta_data["processing"].get("create_pending_rev_reg", False) if create_pending_rev_reg: endorser_connection_id = ( meta_data["endorser"].get("connection_id", None) From 4fefadab12d0343a4d493fc59a0dd4f0081d5885 Mon Sep 17 00:00:00 2001 From: jamshale Date: Thu, 18 Jul 2024 18:37:45 +0000 Subject: [PATCH 2/4] Refactor / Unit test Signed-off-by: jamshale --- aries_cloudagent/revocation/routes.py | 63 +++++++++++-------- .../revocation/tests/test_routes.py | 51 ++++++--------- 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/aries_cloudagent/revocation/routes.py b/aries_cloudagent/revocation/routes.py index 256a215176..9901062fb1 100644 --- a/aries_cloudagent/revocation/routes.py +++ b/aries_cloudagent/revocation/routes.py @@ -654,38 +654,51 @@ async def publish_revocations(request: web.BaseRequest): except (RevocationError, StorageError, IndyIssuerError, LedgerError) as err: raise web.HTTPBadRequest(reason=err.roll_up) from err + if create_transaction_for_endorser: + return web.json_response( + ( + await _process_publish_response_for_endorsement( + profile, rev_reg_responses, outbound_handler, endorser_conn_id + ) + ) + ) + + return web.json_response({"rrid2crid": result}) + + +async def _process_publish_response_for_endorsement( + profile: Profile, + responses: dict, + outbound_handler: BaseResponder.send_outbound, + endorser_conn_id: str, +): txn_responses = [] - for response in rev_reg_responses: - if create_transaction_for_endorser: - transaction_mgr = TransactionManager(profile) + for response in responses: + transaction_mgr = TransactionManager(profile) + try: + transaction = await transaction_mgr.create_record( + messages_attach=response["result"], connection_id=endorser_conn_id + ) + except StorageError as err: + raise web.HTTPBadRequest(reason=err.roll_up) from err + + # if auto-request, send the request to the endorser + if profile.context.settings.get_value("endorser.auto_request"): try: - transaction = await transaction_mgr.create_record( - messages_attach=response["result"], connection_id=endorser_conn_id + ( + transaction, + transaction_request, + ) = await transaction_mgr.create_request( + transaction=transaction, ) - except StorageError as err: + except (StorageError, TransactionManagerError) as err: raise web.HTTPBadRequest(reason=err.roll_up) from err - # if auto-request, send the request to the endorser - if context.settings.get_value("endorser.auto_request"): - try: - ( - transaction, - transaction_request, - ) = await transaction_mgr.create_request( - transaction=transaction, - ) - except (StorageError, TransactionManagerError) as err: - raise web.HTTPBadRequest(reason=err.roll_up) from err - - await outbound_handler( - transaction_request, connection_id=endorser_conn_id - ) + await outbound_handler(transaction_request, connection_id=endorser_conn_id) - txn_responses.append({"txn": transaction.serialize()}) - if txn_responses: - return web.json_response(txn_responses) + txn_responses.append({"txn": transaction.serialize()}) - return web.json_response({"rrid2crid": result}) + return txn_responses @docs(tags=["revocation"], summary="Clear pending revocations") diff --git a/aries_cloudagent/revocation/tests/test_routes.py b/aries_cloudagent/revocation/tests/test_routes.py index 95e4eab0b5..f979fd5810 100644 --- a/aries_cloudagent/revocation/tests/test_routes.py +++ b/aries_cloudagent/revocation/tests/test_routes.py @@ -75,9 +75,7 @@ async def test_validate_cred_rev_rec_qs_and_revoke_req(self): with self.assertRaises(test_module.ValidationError): req.validate_fields({"rev_reg_id": test_module.INDY_REV_REG_ID_EXAMPLE}) with self.assertRaises(test_module.ValidationError): - req.validate_fields( - {"cred_rev_id": test_module.INDY_CRED_REV_ID_EXAMPLE} - ) + req.validate_fields({"cred_rev_id": test_module.INDY_CRED_REV_ID_EXAMPLE}) with self.assertRaises(test_module.ValidationError): req.validate_fields( { @@ -136,9 +134,7 @@ async def test_revoke_endorser_no_conn_id_by_cred_ex_id(self): test_module, "get_endorser_connection_id", mock.CoroutineMock(return_value="dummy-conn-id"), - ), mock.patch.object( - test_module.web, "json_response" - ): + ), mock.patch.object(test_module.web, "json_response"): mock_mgr.return_value.revoke_credential = mock.CoroutineMock( return_value={"result": "..."} ) @@ -185,9 +181,7 @@ async def test_revoke_endorser_no_conn_id(self): test_module, "get_endorser_connection_id", mock.CoroutineMock(return_value="dummy-conn-id"), - ), mock.patch.object( - test_module.web, "json_response" - ): + ), mock.patch.object(test_module.web, "json_response"): mock_mgr.return_value.revoke_credential = mock.CoroutineMock( return_value={"result": "..."} ) @@ -294,9 +288,7 @@ async def test_publish_revocations(self): await test_module.publish_revocations(self.request) - mock_response.assert_called_once_with( - {"rrid2crid": pub_pending.return_value} - ) + mock_response.assert_called_once_with({"rrid2crid": pub_pending.return_value}) async def test_publish_revocations_x(self): self.request.json = mock.CoroutineMock() @@ -319,19 +311,20 @@ async def test_publish_revocations_endorser(self): test_module, "get_endorser_connection_id", mock.CoroutineMock(return_value="dummy-conn-id"), - ), mock.patch.object( - test_module.web, "json_response" - ) as mock_response: + ): pub_pending = mock.CoroutineMock() mock_mgr.return_value.publish_pending_revocations = mock.CoroutineMock( - return_value=({}, pub_pending.return_value) + return_value=( + [ + {"result": "..."}, + {"result": "..."}, + ], + pub_pending.return_value, + ) ) - await test_module.publish_revocations(self.author_request) - - mock_response.assert_called_once_with( - {"rrid2crid": pub_pending.return_value} - ) + result = await test_module.publish_revocations(self.author_request) + assert result.status == 200 async def test_publish_revocations_endorser_x(self): self.author_request.json = mock.CoroutineMock() @@ -342,9 +335,7 @@ async def test_publish_revocations_endorser_x(self): test_module, "get_endorser_connection_id", mock.CoroutineMock(return_value=None), - ), mock.patch.object( - test_module.web, "json_response" - ) as mock_response: + ), mock.patch.object(test_module.web, "json_response") as mock_response: pub_pending = mock.CoroutineMock() mock_mgr.return_value.publish_pending_revocations = pub_pending with self.assertRaises(test_module.web.HTTPBadRequest): @@ -923,9 +914,7 @@ async def test_update_rev_reg(self): ) self.request.match_info = {"rev_reg_id": REV_REG_ID} self.request.json = mock.CoroutineMock( - return_value={ - "tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}" - } + return_value={"tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}"} ) with mock.patch.object( @@ -953,9 +942,7 @@ async def test_update_rev_reg_not_found(self): ) self.request.match_info = {"rev_reg_id": REV_REG_ID} self.request.json = mock.CoroutineMock( - return_value={ - "tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}" - } + return_value={"tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}"} ) with mock.patch.object( @@ -979,9 +966,7 @@ async def test_update_rev_reg_x(self): ) self.request.match_info = {"rev_reg_id": REV_REG_ID} self.request.json = mock.CoroutineMock( - return_value={ - "tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}" - } + return_value={"tails_public_uri": f"http://sample.ca:8181/tails/{REV_REG_ID}"} ) with mock.patch.object( From 4375c80ee7ad09aa7f43bde21d4c6030f8674e20 Mon Sep 17 00:00:00 2001 From: jamshale Date: Thu, 18 Jul 2024 19:41:42 +0000 Subject: [PATCH 3/4] Add more thourough unit testing Signed-off-by: jamshale --- .../revocation/tests/test_routes.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/aries_cloudagent/revocation/tests/test_routes.py b/aries_cloudagent/revocation/tests/test_routes.py index f979fd5810..44714eb76c 100644 --- a/aries_cloudagent/revocation/tests/test_routes.py +++ b/aries_cloudagent/revocation/tests/test_routes.py @@ -11,6 +11,8 @@ from ...admin.request_context import AdminRequestContext from ...askar.profile_anon import AskarAnoncredsProfile +from ...protocols.endorse_transaction.v1_0.manager import TransactionManagerError +from ...storage.error import StorageError from ...storage.in_memory import InMemoryStorage from .. import routes as test_module @@ -326,6 +328,40 @@ async def test_publish_revocations_endorser(self): result = await test_module.publish_revocations(self.author_request) assert result.status == 200 + # Auto endorsement + self.author_request_dict["context"].settings["endorser.auto_request"] = True + self.author_request = mock.MagicMock( + app={}, + match_info={}, + query={}, + __getitem__=lambda _, k: self.author_request_dict[k], + headers={"x-api-key": "author-key"}, + ) + self.author_request.json = mock.CoroutineMock() + result = await test_module.publish_revocations(self.author_request) + assert result.status == 200 + + # exceptions + with mock.patch.object( + test_module, "TransactionManager", autospec=True + ) as mock_txn_mgr: + mock_txn_mgr.return_value.create_record = mock.CoroutineMock( + side_effect=StorageError() + ) + with self.assertRaises(test_module.web.HTTPBadRequest): + result = await test_module.publish_revocations(self.author_request) + + with mock.patch.object( + test_module, "TransactionManager", autospec=True + ) as mock_txn_mgr: + mock_txn_mgr.return_value.create_request = mock.CoroutineMock( + side_effect=[StorageError(), TransactionManagerError()] + ) + with self.assertRaises(test_module.web.HTTPBadRequest): + await test_module.publish_revocations(self.author_request) + with self.assertRaises(test_module.web.HTTPBadRequest): + await test_module.publish_revocations(self.author_request) + async def test_publish_revocations_endorser_x(self): self.author_request.json = mock.CoroutineMock() From bbde3a42a6f2d09bdfe6ef3cc9ac8daf544605a6 Mon Sep 17 00:00:00 2001 From: jamshale Date: Thu, 18 Jul 2024 19:55:51 +0000 Subject: [PATCH 4/4] Revert auto formatting to avoid doing coverage Signed-off-by: jamshale --- aries_cloudagent/revocation/routes.py | 48 ++++++++++++++++++++------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/aries_cloudagent/revocation/routes.py b/aries_cloudagent/revocation/routes.py index 9901062fb1..83505db7d4 100644 --- a/aries_cloudagent/revocation/routes.py +++ b/aries_cloudagent/revocation/routes.py @@ -118,7 +118,9 @@ class TxnOrRevRegResultSchema(OpenAPISchema): txn = fields.Nested( TransactionRecordSchema(), required=False, - metadata={"description": "Revocation registry definition transaction to endorse"}, + metadata={ + "description": "Revocation registry definition transaction to endorse" + }, ) @@ -211,9 +213,13 @@ def validate_fields(self, data, **kwargs): notify_version = data.get("notify_version", "v1_0") if notify and not connection_id: - raise ValidationError("Request must specify connection_id if notify is true") + raise ValidationError( + "Request must specify connection_id if notify is true" + ) if notify and not notify_version: - raise ValidationError("Request must specify notify_version if notify is true") + raise ValidationError( + "Request must specify notify_version if notify is true" + ) publish = fields.Boolean( required=False, @@ -838,7 +844,9 @@ async def rev_regs_created(request: web.BaseRequest): is_anoncreds_profile_raise_web_exception(context.profile) search_tags = list(vars(RevRegsCreatedQueryStringSchema)["_declared_fields"]) - tag_filter = {tag: request.query[tag] for tag in search_tags if tag in request.query} + tag_filter = { + tag: request.query[tag] for tag in search_tags if tag in request.query + } async with context.profile.session() as session: found = await IssuerRevRegRecord.query( session, @@ -847,7 +855,11 @@ async def rev_regs_created(request: web.BaseRequest): ) return web.json_response( - {"rev_reg_ids": [record.revoc_reg_id for record in found if record.revoc_reg_id]} + { + "rev_reg_ids": [ + record.revoc_reg_id for record in found if record.revoc_reg_id + ] + } ) @@ -1287,7 +1299,9 @@ async def send_rev_reg_def(request: web.BaseRequest): raise web.HTTPBadRequest(reason=err.roll_up) from err async with profile.session() as session: - endorser_info = await connection_record.metadata_get(session, "endorser_info") + endorser_info = await connection_record.metadata_get( + session, "endorser_info" + ) if not endorser_info: raise web.HTTPForbidden( reason=( @@ -1405,7 +1419,9 @@ async def send_rev_reg_entry(request: web.BaseRequest): except BaseModelError as err: raise web.HTTPBadRequest(reason=err.roll_up) from err - endorser_info = await connection_record.metadata_get(session, "endorser_info") + endorser_info = await connection_record.metadata_get( + session, "endorser_info" + ) if not endorser_info: raise web.HTTPForbidden( reason=( @@ -1570,7 +1586,9 @@ async def on_revocation_registry_init_event(profile: Profile, event: Event): # TODO error handling - for now just let exceptions get raised endorser_connection_id = meta_data["endorser"]["connection_id"] async with profile.session() as session: - connection = await ConnRecord.retrieve_by_id(session, endorser_connection_id) + connection = await ConnRecord.retrieve_by_id( + session, endorser_connection_id + ) endorser_info = await connection.metadata_get(session, "endorser_info") endorser_did = endorser_info["endorser_did"] write_ledger = False @@ -1642,7 +1660,9 @@ async def generate(rr_record: IssuerRevRegRecord) -> dict: registry_record = await IssuerRevRegRecord.retrieve_by_id(session, record_id) await shield(generate(registry_record)) - create_pending_rev_reg = meta_data["processing"].get("create_pending_rev_reg", False) + create_pending_rev_reg = meta_data["processing"].get( + "create_pending_rev_reg", False + ) if write_ledger and create_pending_rev_reg: revoc = IndyRevocation(profile) await revoc.init_issuer_registry( @@ -1730,13 +1750,17 @@ async def on_revocation_registry_endorsed_event(profile: Profile, event: Event): await registry_record.upload_tails_file(profile) # Post the initial revocation entry - await notify_revocation_entry_event(profile, registry_record.record_id, meta_data) + await notify_revocation_entry_event( + profile, registry_record.record_id, meta_data + ) # create a "pending" registry if one is requested # (this is done automatically when creating a credential definition, so that when a # revocation registry fills up, we can continue to issue credentials without a # delay) - create_pending_rev_reg = meta_data["processing"].get("create_pending_rev_reg", False) + create_pending_rev_reg = meta_data["processing"].get( + "create_pending_rev_reg", False + ) if create_pending_rev_reg: endorser_connection_id = ( meta_data["endorser"].get("connection_id", None) @@ -1902,4 +1926,4 @@ def post_process_routes(app: web.Application): methods["get"]["responses"]["200"]["schema"] = { "type": "string", "format": "binary", - } + } \ No newline at end of file