From f3a10ba331d195913f6e8d747d3ef894b45820a5 Mon Sep 17 00:00:00 2001 From: jamshale <31809382+jamshale@users.noreply.github.com> Date: Thu, 18 Jul 2024 14:45:38 -0700 Subject: [PATCH] Fix publishing multiple rev reg defs with endorsement (#3107) * Fix publishing multiple rev reg defs with endorsement Signed-off-by: jamshale * Refactor / Unit test Signed-off-by: jamshale * Add more thourough unit testing Signed-off-by: jamshale * Revert auto formatting to avoid doing coverage Signed-off-by: jamshale --------- Signed-off-by: jamshale --- aries_cloudagent/revocation/manager.py | 18 ++-- aries_cloudagent/revocation/routes.py | 34 ++++++-- .../revocation/tests/test_routes.py | 85 ++++++++++++------- 3 files changed, 91 insertions(+), 46 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..83505db7d4 100644 --- a/aries_cloudagent/revocation/routes.py +++ b/aries_cloudagent/revocation/routes.py @@ -652,7 +652,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,17 +660,36 @@ 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: + 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 responses: transaction_mgr = TransactionManager(profile) try: transaction = await transaction_mgr.create_record( - messages_attach=rev_reg_resp["result"], connection_id=endorser_conn_id + 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 context.settings.get_value("endorser.auto_request"): + if profile.context.settings.get_value("endorser.auto_request"): try: ( transaction, @@ -683,8 +702,9 @@ async def publish_revocations(request: web.BaseRequest): await outbound_handler(transaction_request, connection_id=endorser_conn_id) - return web.json_response({"txn": transaction.serialize()}) - return web.json_response({"rrid2crid": result}) + txn_responses.append({"txn": transaction.serialize()}) + + return txn_responses @docs(tags=["revocation"], summary="Clear pending revocations") @@ -1906,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 diff --git a/aries_cloudagent/revocation/tests/test_routes.py b/aries_cloudagent/revocation/tests/test_routes.py index 95e4eab0b5..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 @@ -75,9 +77,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 +136,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 +183,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 +290,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 +313,54 @@ 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 + + # 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() @@ -342,9 +371,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 +950,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 +978,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 +1002,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(