Send Problem report when CredEx not found #2577
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
dbluhm
reviewed
Nov 2, 2023
Comment on lines
415
to
422
| format_handlers = [] | ||
| format_handlers_without_offer = [] | ||
| for format in cred_request_message.formats: | ||
| f = V20CredFormat.Format.get(format.format) | ||
| if f: | ||
| format_handlers.append(f) | ||
| if f.handler(self.profile).can_receive_request_without_offer(): | ||
| format_handlers_without_offer.append(f) |
There was a problem hiding this comment.
Good case for a list comprehension:
handlers = [
handler(self.profile)
for format in cred_request_message.formats
if (handler := V20CredFormat.Format.get(format.format))
]
handlers_without_offer = [
handler for handler in handlers
if handler.can_receive_request_without_offer()
]There was a problem hiding this comment.
i like it; will do.
| "debug.auto_respond_credential_request" | ||
| ), | ||
| ) | ||
| if len(format_handlers_without_offer): |
There was a problem hiding this comment.
Nitpick but I'd say it's more idiomatic to just do if format_handlers_without_offer:
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
|
Kudos, SonarCloud Quality Gate passed!
|
|
@dbluhm - just bumping this for another review. added your suggestions and then put a similar exception handling in the v1 code. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #2549
Address the situation when an issuer sends an offer and then deletes that offer (credential exchange) before the holder accepts it. When this is an
indycredential exchange, a Problem Report is sent; when this is ajson-ldexchange, the holder's accept actually becomes an unbound request for the credential and the flow carries on.A discussion is warranted to see if we want to implement something more global? Seems like at any point in any protocol one side may not find their record and the other side should be notified of the abandonment. Maybe it is already done and this was just a gap...
Added BDD tests to address the deletion and followup acceptance for
indyandjson-ldand also the holder sending a request for credential to issuer. Again,indyis not allowed,json-ldcan flow into a credential issuance.