response: Implement ignored parameters with MutableJsonResponse class.

Creates `MutableJsonResponse` as a subclass of Django's `HttpResponse`
that we can modify for ignored parameters in the response content.

Updates responses to include `ignored_parameters_unsupported` in
the response data through `has_request_variables`. Creates unit
test for this implementation in `test_decorators.py`.

The `method` parameter processed in `rest_dispatch` is not in the
`REQ` framework, so for any tests that pass that parameter, assert
for the ignored parameter with a comment.

Updates OpenAPI documentation for `ignored_parameters_unsupported`
being returned in the JSON success response for all endpoints.
Adds detailed documentation in the error handling article, and
links to that page in relevant locations throughout the API docs.

For the majority of endpoints, the documentation does not include
the array in any examples of return values, and instead links to
the error handling page. The exceptions are the three endpoints
that had previously supported this return value. The changes note
and example for these endpoints is also used in the error
handling page.
This commit is contained in:
Lauryn Menard 2022-08-25 18:41:46 +02:00 committed by Tim Abbott
parent 0f2472ed14
commit e9bfdd1bf2
13 changed files with 367 additions and 130 deletions

View File

@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 7.0
**Feature level 167**
* [All REST API endpoints](/api/rest-error-handling#ignored-parameters):
Implemented `ignored_parameters_unsupported` as a possible return value
in the JSON success response for all endpoints. This value is a array
of any parameters that were sent in the request by the client that are
not supported by the endpoint. Previously, unsupported parameters were
silently ignored, except in the subset of endpoints which already
supported this return value; see feature levels 111, 96 and 78.
**Feature level 166**
* [`POST /messages`](/api/send-message): Eliminated the undocumented

View File

@ -44,3 +44,17 @@ to a given request, the values returned will be for the strictest
limit.
[rate-limiting-rules]: https://zulip.readthedocs.io/en/latest/production/security-model.html#rate-limiting
## Ignored Parameters
In JSON success responses, all Zulip REST API endpoints may return
an array of parameters sent in the request that are not supported
by that specific endpoint.
While this can be expected, e.g. when sending both current and legacy
names for a parameter to a Zulip server of unknown version, this often
indicates either a bug in the client implementation or an attempt to
configure a new feature while connected to an older Zulip server that
does not support said feature.
{generate_code_example|/settings:patch|fixture}

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 166
API_FEATURE_LEVEL = 167
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

View File

@ -106,7 +106,7 @@ class APIReturnValuesTablePreprocessor(Preprocessor):
)
def render_table(self, return_values: Dict[str, Any], spacing: int) -> List[str]:
IGNORE = ["result", "msg"]
IGNORE = ["result", "msg", "ignored_parameters_unsupported"]
ans = []
for return_value in return_values:
if return_value in IGNORE:

View File

@ -30,6 +30,7 @@ from typing_extensions import Concatenate, ParamSpec
from zerver.lib import rate_limiter
from zerver.lib.exceptions import ErrorCode, InvalidJSONError, JsonableError
from zerver.lib.notes import BaseNotes
from zerver.lib.response import MutableJsonResponse
from zerver.lib.types import Validator
from zerver.lib.validator import check_anything
from zerver.models import Client, Realm
@ -70,7 +71,6 @@ class RequestNotes(BaseNotes[HttpRequest, "RequestNotes"]):
saved_response: Optional[HttpResponse] = None
tornado_handler_id: Optional[int] = None
processed_parameters: Set[str] = field(default_factory=set)
ignored_parameters: Set[str] = field(default_factory=set)
remote_server: Optional["RemoteZulipServer"] = None
is_webhook_view: bool = False
@ -456,7 +456,38 @@ def has_request_variables(
kwargs[func_var_name] = val
return req_func(request, *args, **kwargs)
return_value = req_func(request, *args, **kwargs)
if (
isinstance(return_value, MutableJsonResponse)
and not request_notes.is_webhook_view
# Implemented only for 200 responses.
# TODO: Implement returning unsupported ignored parameters for 400
# JSON error responses. This is complex because has_request_variables
# can be called multiple times, so when an error response is raised,
# there may be supported parameters that have not yet been processed,
# which could lead to inaccurate output.
and 200 <= return_value.status_code < 300
):
ignored_parameters = set(
list(request.POST.keys()) + list(request.GET.keys())
).difference(request_notes.processed_parameters)
# This will be called each time a function decorated with
# has_request_variables returns a MutableJsonResponse with a
# success status_code. Because a shared processed_parameters
# value is checked each time, the value for the
# ignored_parameters_unsupported key is either added/updated
# to the response data or it is removed in the case that all
# of the request parameters have been processed.
if ignored_parameters:
return_value.get_data()["ignored_parameters_unsupported"] = sorted(
ignored_parameters
)
else:
return_value.get_data().pop("ignored_parameters_unsupported", None)
return return_value
return _wrapped_req_func

View File

@ -1,4 +1,4 @@
from typing import Any, List, Mapping, Optional
from typing import Any, Dict, Iterator, List, Mapping, Optional
import orjson
from django.http import HttpRequest, HttpResponse, HttpResponseNotAllowed
@ -6,6 +6,72 @@ from django.http import HttpRequest, HttpResponse, HttpResponseNotAllowed
from zerver.lib.exceptions import JsonableError, UnauthorizedError
class MutableJsonResponse(HttpResponse):
def __init__(
self,
data: Dict[str, Any],
*,
content_type: str,
status: int,
) -> None:
# Mirror the behavior of Django's TemplateResponse and pass an
# empty string for the initial content value. Because that will
# set _needs_serialization to False, we initialize it to True
# after the call to super __init__.
super().__init__("", content_type=content_type, status=status)
self._data = data
self._needs_serialization = True
def get_data(self) -> Dict[str, Any]:
"""Get data for this MutableJsonResponse. Calling this method
after the response's content has already been serialized
will mean the next time the response's content is accessed
it will be reserialized because the caller may have mutated
the data."""
self._needs_serialization = True
return self._data
# This always returns bytes, but in Django's HttpResponse the return
# value can be bytes, an iterable of bytes or some other object. Any
# is used here to encompass all of those return values.
# See https://github.com/typeddjango/django-stubs/commit/799b41fe47cfe2e56be33eee8cfbaf89a9853a8e
# and https://github.com/python/mypy/issues/3004.
@property
def content(self) -> Any:
"""Get content for the response. If the content hasn't been
overridden by the property setter, it will be the response data
serialized lazily to JSON."""
if self._needs_serialization:
# Because we don't pass a default handler, OPT_PASSTHROUGH_DATETIME
# actually causes orjson to raise a TypeError on datetime objects. This
# helps us avoid relying on the particular serialization used by orjson.
self.content = orjson.dumps(
self._data,
option=orjson.OPT_APPEND_NEWLINE | orjson.OPT_PASSTHROUGH_DATETIME,
)
return super().content
# There are two ways this might be called. The first is in the getter when
# the response data is being serialized into JSON. The second is when it
# is called from some other part of the code. This happens for instance in
# the parent class constructor. In this case, the new content overrides the
# serialized JSON.
@content.setter
def content(self, value: Any) -> None:
"""Set the content for the response."""
assert isinstance(HttpResponse.content, property)
assert HttpResponse.content.fset is not None
HttpResponse.content.fset(self, value)
self._needs_serialization = False
# The superclass HttpResponse defines an iterator that doesn't access the content
# property, so in order to not break the implementation of the superclass with
# our lazy content generation, we override the iterator to access `self.content`
# through our getter.
def __iter__(self) -> Iterator[bytes]:
return iter([self.content])
def json_unauthorized(
message: Optional[str] = None, www_authenticate: Optional[str] = None
) -> HttpResponse:
@ -24,32 +90,26 @@ def json_method_not_allowed(methods: List[str]) -> HttpResponseNotAllowed:
def json_response(
res_type: str = "success", msg: str = "", data: Mapping[str, Any] = {}, status: int = 200
) -> HttpResponse:
) -> MutableJsonResponse:
content = {"result": res_type, "msg": msg}
content.update(data)
# Because we don't pass a default handler, OPT_PASSTHROUGH_DATETIME
# actually causes orjson to raise a TypeError on datetime objects. This
# helps us avoid relying on the particular serialization used by orjson.
return HttpResponse(
content=orjson.dumps(
content,
option=orjson.OPT_APPEND_NEWLINE | orjson.OPT_PASSTHROUGH_DATETIME,
),
return MutableJsonResponse(
data=content,
content_type="application/json",
status=status,
)
def json_success(request: HttpRequest, data: Mapping[str, Any] = {}) -> HttpResponse:
def json_success(request: HttpRequest, data: Mapping[str, Any] = {}) -> MutableJsonResponse:
return json_response(data=data)
def json_partial_success(request: HttpRequest, data: Mapping[str, Any] = {}) -> HttpResponse:
def json_partial_success(request: HttpRequest, data: Mapping[str, Any] = {}) -> MutableJsonResponse:
return json_response(res_type="partially_completed", data=data, status=200)
def json_response_from_error(exception: JsonableError) -> HttpResponse:
def json_response_from_error(exception: JsonableError) -> MutableJsonResponse:
"""
This should only be needed in middleware; in app code, just raise.

View File

@ -207,6 +207,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
events:
type: array
description: |
@ -4539,6 +4540,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
stream_id:
type: integer
description: |
@ -4667,6 +4669,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
attachments:
type: array
description: |
@ -4779,6 +4782,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
count:
type: integer
description: |
@ -4866,6 +4870,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
ids:
type: array
description: |
@ -5244,6 +5249,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
anchor:
type: integer
description: |
@ -5500,6 +5506,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
id:
type: integer
description: |
@ -5570,6 +5577,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
message_history:
type: array
items:
@ -5820,6 +5828,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
messages:
type: array
items:
@ -6049,6 +6058,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
rendered:
type: string
description: |
@ -6193,6 +6203,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
user_ids:
type: array
description: |
@ -6289,6 +6300,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
messages:
type: object
description: |
@ -6370,6 +6382,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
raw_content:
type: string
deprecated: true
@ -6718,6 +6731,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
uri:
type: string
description: |
@ -6767,6 +6781,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
url:
type: string
description: |
@ -6817,6 +6832,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
members:
type: array
description: |
@ -6966,6 +6982,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
user_id:
type: integer
description: |
@ -7047,6 +7064,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
presence:
type: object
description: |
@ -7114,6 +7132,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
avatar_url:
type: string
description: |
@ -7340,6 +7359,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
alert_words:
type: array
description: |
@ -7391,6 +7411,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
alert_words:
type: array
description: |
@ -7458,6 +7479,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
alert_words:
type: array
description: |
@ -7655,6 +7677,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
topics:
type: array
description: |
@ -7738,6 +7761,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
subscriptions:
type: array
description: |
@ -8017,6 +8041,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
subscribed:
type: object
description: |
@ -8137,6 +8162,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
not_removed:
type: array
items:
@ -8367,6 +8393,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
is_subscribed:
type: boolean
description: |
@ -8441,6 +8468,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
emoji:
type: object
description: |
@ -8489,6 +8517,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
server_timestamp:
type: number
description: |
@ -8553,6 +8582,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
custom_fields:
type: array
description: |
@ -8769,6 +8799,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
id:
type: integer
description: |
@ -8795,9 +8826,13 @@ paths:
Note that this endpoint cannot, at present, be used to modify
settings for existing users in any way.
**Changes**: New in Zulip 5.0 (feature level 96).
**Changes**: New in Zulip 5.0 (feature level 96). If any parameters
sent in the request are not supported by this endpoint, an
[`ignored_parameters_unsupported`][ignored-parameters] array will
be returned in the JSON success response.
[new-user-defaults]: /help/configure-default-new-user-settings
[ignored-parameters]: /api/rest-error-handling#ignored-parameters
x-curl-examples-parameters:
oneOf:
- type: include
@ -9181,26 +9216,7 @@ paths:
example: 1
responses:
"200":
description: Success
content:
application/json:
schema:
allOf:
- $ref: "#/components/schemas/JsonSuccessBase"
- $ref: "#/components/schemas/SuccessDescription"
- additionalProperties: false
properties:
result: {}
msg: {}
ignored_parameters_unsupported:
$ref: "#/components/schemas/IgnoredParametersUnsupported"
example:
{
"ignored_parameters_unsupported":
["desktop_notifications", "demote_streams"],
"msg": "",
"result": "success",
}
$ref: "#/components/responses/SuccessIgnoredParameters"
/users/me/subscriptions/properties:
post:
@ -9215,7 +9231,9 @@ paths:
**Changes**: Prior to Zulip 5.0 (feature level 111), response
object included the `subscription_data` in the the
request. The endpoint now returns the more ergonomic
`ignored_parameters_unsupported` field instead.
[`ignored_parameters_unsupported`][ignored-parameters] array instead.
[ignored-parameters]: /api/rest-error-handling#ignored-parameters
parameters:
- name: subscription_data
in: query
@ -9306,25 +9324,7 @@ paths:
required: true
responses:
"200":
description: Success.
content:
application/json:
schema:
allOf:
- $ref: "#/components/schemas/JsonSuccessBase"
- $ref: "#/components/schemas/SuccessDescription"
- additionalProperties: false
properties:
result: {}
msg: {}
ignored_parameters_unsupported:
$ref: "#/components/schemas/IgnoredParametersUnsupported"
example:
{
"ignored_parameters_unsupported": ["invalid_parameter"],
"result": "success",
"msg": "",
}
$ref: "#/components/responses/SuccessIgnoredParameters"
/users/{email}:
get:
operationId: get-user-by-email
@ -9380,6 +9380,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
user:
$ref: "#/components/schemas/User"
example:
@ -9468,6 +9469,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
user:
$ref: "#/components/schemas/User"
example:
@ -9667,6 +9669,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
linkifiers:
type: array
description: |
@ -9726,6 +9729,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
id:
type: integer
description: |
@ -9827,6 +9831,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
id:
type: integer
description: |
@ -10075,6 +10080,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
queue_id:
type: string
nullable: true
@ -13413,10 +13419,19 @@ paths:
- $ref: "#/components/schemas/JsonSuccessBase"
- additionalProperties: false
description: |
A typical successful JSON response for a single-organization server may look like:
**Changes**: As of Zulip 7.0 (feature level 167), if any
parameters sent in the request are not supported by this
endpoint, a successful JSON response will include an
[ignored_parameters_unsupported][ignored_params] array.
A typical successful JSON response for a single-organization server
may look like:
[ignored_params]: /api/rest-error-handling#ignored-parameters
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
authentication_methods:
type: object
additionalProperties: false
@ -13692,7 +13707,7 @@ paths:
processed by including in the response object `"key": value`
entries for values successfully changed by the request. That
was replaced by the more ergonomic
`ignored_parameters_unsupported` response parameter.
[`ignored_parameters_unsupported`][ignored-parameters] array.
The `/settings/notifications` and `/settings/display` endpoints
also had this behavior before they became aliases of `/settings`
@ -13700,6 +13715,8 @@ paths:
Before these changes, request parameters that were not supported
(or were unchanged) were silently ignored.
[ignored-parameters]: /api/rest-error-handling#ignored-parameters
x-curl-examples-parameters:
oneOf:
- type: include
@ -14259,25 +14276,7 @@ paths:
example: 1
responses:
"200":
description: Success
content:
application/json:
schema:
allOf:
- $ref: "#/components/schemas/JsonSuccessBase"
- $ref: "#/components/schemas/SuccessDescription"
- additionalProperties: false
properties:
result: {}
msg: {}
ignored_parameters_unsupported:
$ref: "#/components/schemas/IgnoredParametersUnsupported"
example:
{
"ignored_parameters_unsupported": ["name", "password"],
"msg": "",
"result": "success",
}
$ref: "#/components/responses/SuccessIgnoredParameters"
/streams/{stream_id}/members:
get:
operationId: get-subscribers
@ -14300,6 +14299,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
subscribers:
type: array
items:
@ -14409,6 +14409,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
streams:
description: |
A list of `stream` objects with details on the requested streams.
@ -14524,6 +14525,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
stream:
$ref: "#/components/schemas/BasicStream"
example:
@ -15040,6 +15042,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
members:
type: array
items:
@ -15150,6 +15153,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
user_groups:
type: array
items:
@ -15295,6 +15299,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
subgroups:
type: array
items:
@ -15329,6 +15334,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
is_user_group_member:
type: boolean
description: |
@ -15531,6 +15537,7 @@ paths:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
url:
description: |
The URL for the BigBlueButton video call.
@ -15563,18 +15570,10 @@ components:
type: string
description: |
An array of any parameters sent in the request that are not
supported by the endpoint. While this can be expected, e.g. when sending
both current and legacy names for a parameter to a Zulip server of
unknown version, this often indicates either a bug in the client
implementation or an attempt to configure a new feature while
connected to an older Zulip server that does not support said feature.
supported by the endpoint.
**Changes**: Added to `POST /users/me/subscriptions/properties` in
Zulip 5.0 (feature level 111).
Added to `PATCH /realm/user_settings_defaults` in Zulip 5.0 (feature level 96).
Introduced in `PATCH /settings` in Zulip 5.0 (feature level 78).
See [error handling](/api/rest-error-handling#ignored-parameters) documentation
for details on this and its change history.
EventIdSchema:
type: integer
description: |
@ -17107,7 +17106,14 @@ components:
type: string
SuccessDescription:
description: |
**Changes**: As of Zulip 7.0 (feature level 167), if any
parameters sent in the request are not supported by this
endpoint, a successful JSON response will include an
[`ignored_parameters_unsupported`][ignored_params] array.
A typical successful JSON response may look like:
[ignored_params]: /api/rest-error-handling#ignored-parameters
JsonSuccess:
allOf:
- $ref: "#/components/schemas/JsonSuccessBase"
@ -17115,6 +17121,7 @@ components:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
JsonSuccessBase:
allOf:
- $ref: "#/components/schemas/JsonResponseBase"
@ -17127,7 +17134,54 @@ components:
- success
msg:
type: string
ignored_parameters_unsupported:
$ref: "#/components/schemas/IgnoredParametersUnsupported"
example: {"msg": "", "result": "success"}
IgnoredParametersSuccess:
allOf:
- $ref: "#/components/schemas/IgnoredParametersBase"
- additionalProperties: false
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
description: |
**Changes**: The [`ignored_parameters_unsupported`][ignored_params]
array was added as a possible return value for all REST API endpoint
JSON success responses in Zulip 7.0 (feature level 167).
Previously, it was added to
[`POST /users/me/subscriptions/properties`](/api/update-subscription-settings)
in Zulip 5.0 (feature level 111) and to
[`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults)
in Zulip 5.0 (feature level 96). The feature was introduced in Zulip 5.0
(feature level 78) as a return value for the
[`PATCH /settings`](/api/update-settings) endpoint.
A typical successful JSON response with ignored parameters may look like:
[ignored_params]: /api/rest-error-handling#ignored-parameters
IgnoredParametersBase:
allOf:
- $ref: "#/components/schemas/JsonResponseBase"
- required:
- result
- msg
properties:
result:
enum:
- success
msg:
type: string
ignored_parameters_unsupported:
$ref: "#/components/schemas/IgnoredParametersUnsupported"
example:
{
"ignored_parameters_unsupported":
["invalid_param_1", "invalid_param_2"],
"msg": "",
"result": "success",
}
JsonError:
allOf:
- $ref: "#/components/schemas/JsonErrorBase"
@ -17174,6 +17228,7 @@ components:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
api_key:
type: string
description: |
@ -17265,6 +17320,7 @@ components:
properties:
result: {}
msg: {}
ignored_parameters_unsupported: {}
subscribed:
type: object
description: |
@ -17409,6 +17465,13 @@ components:
allOf:
- $ref: "#/components/schemas/JsonSuccess"
- $ref: "#/components/schemas/SuccessDescription"
SuccessIgnoredParameters:
description: Success.
content:
application/json:
schema:
allOf:
- $ref: "#/components/schemas/IgnoredParametersSuccess"
####################
# Shared parameters

View File

@ -1501,7 +1501,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
email = "hambot-bot@zulip.testserver"
# Important: We intentionally use the wrong method, post, here.
result = self.client_post(f"/json/bots/{self.get_bot_user(email).id}", bot_info)
response_dict = self.assert_json_success(result)
# TODO: The "method" parameter is not currently tracked as a processed parameter
# by has_request_variables. Assert it is returned as an ignored parameter.
response_dict = self.assert_json_success(result, ignored_parameters=["method"])
self.assertEqual("Fred", response_dict["full_name"])

View File

@ -53,7 +53,7 @@ from zerver.lib.request import (
RequestVariableMissingError,
has_request_variables,
)
from zerver.lib.response import json_response, json_success
from zerver.lib.response import MutableJsonResponse, json_response, json_success
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import HostRequestMock, dummy_handler, queries_captured
from zerver.lib.types import Validator
@ -2251,3 +2251,94 @@ class ClientTestCase(ZulipTestCase):
# client_name has the full name still, though
self.assertEqual(client_name, "very-long-name-goes-here-and-still-works")
self.assert_length(queries, 0)
class TestIgnoredParametersUnsupported(ZulipTestCase):
def test_ignored_parameters_json_success(self) -> None:
@has_request_variables
def test_view(
request: HttpRequest,
name: Optional[str] = REQ(default=None),
age: Optional[int] = 0,
) -> HttpResponse:
return json_success(request)
# ignored parameter (not processed through REQ)
request = HostRequestMock()
request.POST["age"] = "30"
result = test_view(request)
self.assert_json_success(result, ignored_parameters=["age"])
# valid parameter, returns no ignored parameters
request = HostRequestMock()
request.POST["name"] = "Hamlet"
result = test_view(request)
self.assert_json_success(result)
# both valid and ignored parameters
request = HostRequestMock()
request.POST["name"] = "Hamlet"
request.POST["age"] = "30"
request.POST["location"] = "Denmark"
request.POST["dies"] = "True"
result = test_view(request)
ignored_parameters = ["age", "dies", "location"]
json_result = self.assert_json_success(result, ignored_parameters=ignored_parameters)
# check that results are sorted
self.assertEqual(json_result["ignored_parameters_unsupported"], ignored_parameters)
# Because `has_request_variables` can be called multiple times on a request,
# here we test that parameters processed in separate, nested function calls
# are not returned in the `ignored parameters_unsupported` array.
def test_nested_has_request_variables(self) -> None:
@has_request_variables
def not_view_function_A(
request: HttpRequest, dies: bool = REQ(json_validator=check_bool)
) -> None:
return
@has_request_variables
def not_view_function_B(
request: HttpRequest, married: bool = REQ(json_validator=check_bool)
) -> None:
return
@has_request_variables
def view_B(request: HttpRequest, name: str = REQ()) -> MutableJsonResponse:
return json_success(request)
@has_request_variables
def view_A(
request: HttpRequest, age: int = REQ(json_validator=check_int)
) -> MutableJsonResponse:
not_view_function_A(request)
response = view_B(request)
not_view_function_B(request)
return response
# valid parameters, returns no ignored parameters
post_data = {"name": "Hamlet", "age": "30", "dies": "true", "married": "false"}
request = HostRequestMock(post_data)
result = view_A(request)
result_iter = list(iter(result))
self.assertEqual(result_iter, [b'{"result":"success","msg":""}\n'])
self.assert_json_success(result)
# ignored parameter
post_data = {
"name": "Hamlet",
"age": "30",
"dies": "true",
"married": "false",
"author": "William Shakespeare",
}
request = HostRequestMock(post_data)
result = view_A(request)
result_iter = list(iter(result))
self.assertEqual(
result_iter,
[b'{"result":"success","msg":"","ignored_parameters_unsupported":["author"]}\n'],
)
self.assert_json_success(result, ignored_parameters=["author"])

View File

@ -921,7 +921,7 @@ class AdminCreateUserTest(ZulipTestCase):
short_name="DEPRECATED",
),
)
self.assert_json_success(result)
self.assert_json_success(result, ignored_parameters=["short_name"])
result = self.client_post(
"/json/users",

View File

@ -484,16 +484,4 @@ def update_realm_user_settings_defaults(
if v is not None and getattr(realm_user_default, k) != v:
do_set_realm_user_default_setting(realm_user_default, k, v, acting_user=user_profile)
# TODO: Extract `ignored_parameters_unsupported` to be a common feature of the REQ framework.
from zerver.lib.request import RequestNotes
request_notes = RequestNotes.get_notes(request)
for req_var in request.POST:
if req_var not in request_notes.processed_parameters:
request_notes.ignored_parameters.add(req_var)
result: Dict[str, Any] = {}
if len(request_notes.ignored_parameters) > 0:
result["ignored_parameters_unsupported"] = list(request_notes.ignored_parameters)
return json_success(request, data=result)
return json_success(request)

View File

@ -1051,16 +1051,4 @@ def update_subscription_properties_backend(
user_profile, sub, stream, property, value, acting_user=user_profile
)
# TODO: Do this more generally, see update_realm_user_settings_defaults.realm.py
from zerver.lib.request import RequestNotes
request_notes = RequestNotes.get_notes(request)
for req_var in request.POST:
if req_var not in request_notes.processed_parameters:
request_notes.ignored_parameters.add(req_var)
result: Dict[str, Any] = {}
if len(request_notes.ignored_parameters) > 0:
result["ignored_parameters_unsupported"] = list(request_notes.ignored_parameters)
return json_success(request, data=result)
return json_success(request)

View File

@ -327,17 +327,6 @@ def json_change_settings(
if timezone is not None and user_profile.timezone != timezone:
do_change_user_setting(user_profile, "timezone", timezone, acting_user=user_profile)
# TODO: Do this more generally.
from zerver.lib.request import RequestNotes
request_notes = RequestNotes.get_notes(request)
for req_var in request.POST:
if req_var not in request_notes.processed_parameters:
request_notes.ignored_parameters.add(req_var)
if len(request_notes.ignored_parameters) > 0:
result["ignored_parameters_unsupported"] = list(request_notes.ignored_parameters)
return json_success(request, data=result)