feat(request): add get_param_as_media() method#2556
feat(request): add get_param_as_media() method#2556MannXo wants to merge 8 commits intofalconry:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2556 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 7911 7920 +9
Branches 1086 1088 +2
=========================================
+ Hits 7911 7920 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Cover media_type=None fallback to default_media_type - Cover JSON handler fallback when no explicit handler found - Cover error case when no handler and non-JSON media type - Ensures 100% coverage for new get_param_as_media method
…to feat/get-param-as-media
| raise errors.HTTPInvalidParam(msg, name) | ||
|
|
||
| try: | ||
| # TODO(CaselIT): find a way to avoid encode + BytesIO if handlers |
There was a problem hiding this comment.
Please restore @CaselIT 's comment. This is still an unsolved issue in the framework.
|
|
||
| return self._cached_headers | ||
|
|
||
| def get_param_as_media( |
There was a problem hiding this comment.
Is there a reason for copypasting this here? Couldn't we get the exact same copy (sans extra call overhead) via inheritance?
| name (str): Parameter name, case-sensitive (e.g., 'payload'). | ||
|
|
||
| Keyword Args: | ||
| media_type (str|None): Media type to use for deserialization. If |
There was a problem hiding this comment.
Maybe we could use the same form (str | None) here as in the annotation (if we even need to duplicate it here? 🤔)?
| if handler is None: | ||
| handler = _DEFAULT_JSON_HANDLER | ||
| # Fall back to JSON handler when requested media is JSON | ||
| if media_type and MEDIA_JSON in media_type: |
There was a problem hiding this comment.
We are missing the case here when media_type is omitted, and the default_media_type is JSON. It should also use the default handler for JSON according to this logic.
| if handler is None: | ||
| handler = _DEFAULT_JSON_HANDLER | ||
| # Fall back to JSON handler when requested media is JSON | ||
| if media_type and MEDIA_JSON in media_type: |
There was a problem hiding this comment.
This is a bit sloppy way to check MEDIA_JSON in media_type, this way it could match an unrelated media type such as application/jsonbinary or whatever.
However, we do cut corners in Falcon in some edge cases like this one. But then we should add a NOTE(...): ... comment explaining this reasoning (along the lines "good enough until proven otherwise").
2 participants
Summary of Changes
Adds a new public helper:
Request.get_param_as_media(name, media_type=None, required=False, store=None, default=None):1- Deserializes a single query-string parameter using the app's configured media handlers.
2- If media_type is omitted, falls back to the app's
default_media_type.3- Stores the parsed value in
storewhen provided.4- Raises
HTTPInvalidParamon parse errors (or when no suitable handler exists for non-JSON media).ASGI support:
1-
falcon.asgi.Request.get_param_as_mediadelegates to the synchronous WSGI implementation (parameter values are already in memory).2-
get_param_as_json()now delegates toget_param_as_media(..., MEDIA_JSON)(thin wrapper).Related Issues
closes: #2549
Pull Request Checklist
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. Reading our contribution guide at least once will save you a few review cycles!
If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.
tox.docs/.docs/.versionadded,versionchanged, ordeprecateddirectives.docs/_newsfragments/, with the file name format{issue_number}.{fragment_type}.rst. (Runtox -e towncrier, and inspectdocs/_build/html/changes/in the browser to ensure it renders correctly.)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
PR template inspired by the attrs project.