Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thorough request object retrieval algorithm #45

Merged

Conversation

acrusage-iaik
Copy link
Contributor

Improved algorithm for retrieving AuthenticationResponseParameters as asked for in a-sit-plus/valera#61 (comment)

@acrusage-iaik acrusage-iaik self-assigned this Mar 22, 2024
Copy link
Collaborator

@JesusMcCloud JesusMcCloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks really good, but it would be nice to have both cases tested and not only the baseline one we hade before (even though it is important to keep it as regression test)

@acrusage-iaik
Copy link
Contributor Author

The implementation looks really good, but it would be nice to have both cases tested and not only the baseline one we hade before (even though it is important to keep it as regression test)

I might need help with that, I'm not sure how to provide a dummy response to a HttpClient.get request

@acrusage-iaik
Copy link
Contributor Author

The implementation looks really good, but it would be nice to have both cases tested and not only the baseline one we hade before (even though it is important to keep it as regression test)

fixed in commit 19b4614

@acrusage-iaik acrusage-iaik merged commit d1d1d85 into develop Mar 29, 2024
3 checks passed
@nodh
Copy link
Contributor

nodh commented Apr 2, 2024

I'm unhappy with the added dependency on HttpClient from ktor ... is this really necessary?

@JesusMcCloud
Copy link
Collaborator

how would you make an http request, without an http client?


typealias RequestObjectCandidateRetriever = suspend (Url) -> List<String>

private val HttpClient.asRequestObjectCandidateRetriever: RequestObjectCandidateRetriever
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho as as the prefix indicates a function, but this is a property

@nodh
Copy link
Contributor

nodh commented Apr 2, 2024

how would you make an http request, without an http client?

Why don't we leave that up to the client, i.e. the caller of our library, as it has been the case up until now?

@acrusage-iaik
Copy link
Contributor Author

how would you make an http request, without an http client?

Why don't we leave that up to the client, i.e. the caller of our library, as it has been the case up until now?

fixed in commit 03a0cdd
prepared at pull request #46 along with a fix for #45 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants