[uss_qualifier/scenarios/utm/data_exchange_validation] Refactor filtering of mock_uss_interactions #831
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
mickmis
force-pushed
the
797/interactions
branch
3 times, most recently
from
979a813
to
8085fe3
Compare
Shastick
reviewed
Nov 4, 2024
Comment on lines
187
to
207
| def operation_filter( | ||
| op_id: OperationID, | ||
| **query_params: str, | ||
| ) -> Callable[[Interaction], bool]: | ||
| """ | ||
| Returns an `is_applicable` filter that filters according to operation ID. | ||
|
|
||
| Raises: | ||
| KeyError: if query_params contains a non-existing parameter | ||
| IndexError: if query_params is missing a parameter | ||
| """ | ||
| op = api.OPERATIONS[op_id] | ||
| op_path = op.path.format(**query_params) # raises KeyError, IndexError | ||
|
|
||
| def is_applicable(interaction: Interaction) -> bool: | ||
| return interaction.query.request.method == op.verb and re.search( | ||
| op_path, interaction.query.request.url | ||
| ) | ||
|
|
||
| return is_applicable |
There was a problem hiding this comment.
The documentation and/or name of this method could be improved: at first glance it looks like it might allow one to return "all queries for this operation ID regardless of parameters, and you can optionally specify parameters to narrow it down". The docstring 'Raises' part does hint at the fact that this is incorrect, but is a bit obscure.
(Ie, the passed query_params MUST be all the parameters required for the passed operation ID)
There was a problem hiding this comment.
Updated doc to clarify.
BenjaminPelletier
approved these changes
Nov 5, 2024
…ring of mock_uss_interactions
mickmis
force-pushed
the
797/interactions
branch
from
8085fe3
to
6c69eb3
Compare
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.
Issue #797
Stack on top of #829, please consider only last commit.
This refactors the way
mock_uss_interactionsfilters interactions by generalizing the filtering by functions that already existed. On top of simplifying the logic, this enables invoking that function without any filters, which we will need in an upcoming PR.op_idandquery_paramsis replaced by filter functionoperation_filterdirectionis replaced by filter functiondirection_filteris_op_intent_notification_with_idis renamed intonotif_op_intent_id_filter_is_notification_sent_to_url_with_op_intent_idis replaced by newbase_url_filtercombined with existingnotif_op_intent_id_filter