The goal of typed_endpoint is to replicate most features supported by
has_request_variables, and to improve on top of it. There are some
unresolved issues that we don't plan to work on currently. For example,
typed_endpoint does not support ignored_parameters_supported for 400
responses, and it does not run validators on path-only arguments.
Unlike has_request_variables, typed_endpoint supports error handling by
processing validation errors from Pydantic.
Most features supported by has_request_variables are supported by
typed_endpoint in various ways.
To define a function, use a syntax like this with Annotated if there is
any metadata you want to associate with a parameter, do note that
parameters that are not keyword-only are ignored from the request:
```
@typed_endpoint
def view(
request: HttpRequest,
user_profile: UserProfile,
*,
foo: Annotated[int, ApiParamConfig(path_only=True)],
bar: Json[int],
other: Annotated[
Json[int],
ApiParamConfig(
whence="lorem",
documentation_status=NTENTIONALLY_UNDOCUMENTED
)
] = 10,
) -> HttpResponse:
....
```
There are also some shorthands for the commonly used annotated types,
which are encouraged when applicable for better readability and less
typing:
```
WebhookPayload = Annotated[Json[T], ApiParamConfig(argument_type_is_body=True)]
PathOnly = Annotated[T, ApiParamConfig(path_only=True)]
```
Then the view function above can be rewritten as:
```
@typed_endpoint
def view(
request: HttpRequest,
user_profile: UserProfile,
*,
foo: PathOnly[int],
bar: Json[int],
other: Annotated[
Json[int],
ApiParamConfig(
whence="lorem",
documentation_status=INTENTIONALLY_UNDOCUMENTED
)
] = 10,
) -> HttpResponse:
....
```
There are some intentional restrictions:
- A single parameter cannot have more than one ApiParamConfig
- Path-only parameters cannot have default values
- argument_type_is_body is incompatible with whence
- Arguments of name "request", "user_profile", "args", and "kwargs" and
etc. are ignored by typed_endpoint.
- positional-only arguments are not supported by typed_endpoint. Only
keyword-only parameters are expected to be parsed from the request.
- Pydantic's strict mode is always enabled, because we don't want to
coerce input parsed from JSON into other types unnecessarily.
- Using strict mode all the time also means that we should always use
Json[int] instead of int, because it is only possible for the request
to have data of type str, and a type annotation of int will always
reject such data.
typed_endpoint's handling of ignored_parameters_unsupported is mostly
identical to that of has_request_variables.
**Background**
User groups are expected to comply with the DAG constraint for the
many-to-many inter-group membership. The check for this constraint has
to be performed recursively so that we can find all direct and indirect
subgroups of the user group to be added.
This kind of check is vulnerable to phantom reads which is possible at
the default read committed isolation level because we cannot guarantee
that the check is still valid when we are adding the subgroups to the
user group.
**Solution**
To avoid having another transaction concurrently update one of the
to-be-subgroup after the recursive check is done, and before the subgroup
is added, we use SELECT FOR UPDATE to lock the user group rows.
The lock needs to be acquired before a group membership change is about
to occur before any check has been conducted.
Suppose that we are adding subgroup B to supergroup A, the locking protocol
is specified as follows:
1. Acquire a lock for B and all its direct and indirect subgroups.
2. Acquire a lock for A.
For the removal of user groups, we acquire a lock for the user group to
be removed with all its direct and indirect subgroups. This is the special
case A=B, which is still complaint with the protocol.
**Error handling**
We currently rely on Postgres' deadlock detection to abort transactions
and show an error for the users. In the future, we might need some
recovery mechanism or at least better error handling.
**Notes**
An important note is that we need to reuse the recursive CTE query that
finds the direct and indirect subgroups when applying the lock on the
rows. And the lock needs to be acquired the same way for the addition and
removal of direct subgroups.
User membership change (as opposed to user group membership) is not
affected. Read-only queries aren't either. The locks only protect
critical regions where the user group dependency graph might violate
the DAG constraint, where users are not participating.
**Testing**
We implement a transaction test case targeting some typical scenarios
when an internal server error is expected to happen (this means that the
user group view makes the correct decision to abort the transaction when
something goes wrong with locks).
To achieve this, we add a development view intended only for unit tests.
It has a global BARRIER that can be shared across threads, so that we
can synchronize them to consistently reproduce certain potential race
conditions prevented by the database locks.
The transaction test case lanuches pairs of threads initiating possibly
conflicting requests at the same time. The tests are set up such that exactly N
of them are expected to succeed with a certain error message (while we don't
know each one).
**Security notes**
get_recursive_subgroups_for_groups will no longer fetch user groups from
other realms. As a result, trying to add/remove a subgroup from another
realm results in a UserGroup not found error response.
We also implement subgroup-specific checks in has_user_group_access to
keep permission managing in a single place. Do note that the API
currently don't have a way to violate that check because we are only
checking the realm ID now.
Dropping support for url_prefix for RealmPlayground, the server now uses
url_template instead only for playground creation, retrieval and audit
logging upon removal.
This does the necessary handling so that url_template is expanded with
the extracted code.
Fixes#25723.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Creates a custom linter rule for `zerver/openapi/zulip.yaml` to
only allow lowercase versions of "true", "false" and "null".
Updates existing documentation for new rules.
If the ID of the scheduled message is passed by the client, we
edit the existing scheduled message instead of creating a new one.
However, this will soon be moved into its own API endpoint.
Primary goal of library replacement is improving execution speed.
This commit should not affect the functionality of the system
or make any changes to it.
Created `WidgetConfig` type which has all the parameter that can be
passed launch(). Except html_heading, html_body, and on_click all
other fields are optional. Therefore, removed checking of undefined
mandatory fields as it will be done by typescript.
Co-authored-by: Hardik Dharmani <Ddharmani99@gmail.com>
This commit extracts realm creation form html in a new file
realm_creation_form.html as we are reworking the organization
flow as per #24307 and this change would allow us to avoid
duplicating the code.
This commit adds 'zerver/lib/email_notifications.py'
to the FILES_WITH_LEGACY_SUBJECT set.
Because the file can have 'subject' in the email sense,
it should be exempted from the 'avoid subject as a var' lint rule.
This lets us simplify the long-ish ‘../../static/js’ paths, and will
remove the need for the ‘zrequire’ wrapper.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Ever since we started bundling the app with webpack, there’s been less
and less overlap between our ‘static’ directory (files belonging to
the frontend app) and Django’s interpretation of the ‘static’
directory (files served directly to the web).
Split the app out to its own ‘web’ directory outside of ‘static’, and
remove all the custom collectstatic --ignore rules. This makes it
much clearer what’s actually being served to the web, and what’s being
bundled by webpack. It also shrinks the release tarball by 3%.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
- Updates `.prettierignore` for the new directory.
- Updates any reference to the API documentation directory for
markdown files to be `api_docs/` instead of `zerver/api/`.
- Removes a reference link from `docs/documentation/api.md` that
hasn't referenced anything in the text since commit 0542c60.
- Update rendering of API documentation for new directory.
These files are not Jinja2 templates, so there's no reason that they needed
to be inside `templates/zerver`. Moving them to the top level reflects their
importance and also makes it feel nicer to work on editing the help center content,
without it being unnecessary buried deep in the codebase.
The module has been renamed, but also the original linter rule was
likely ineffective because of the length of compose_banner calls
anyway.
I'm not particularly worried about future internationalization bugs
here, so removing seems better than replacing it.
We should rearrange Zulip's developer docs to make it easier to
find the documentation that new contributors need.
Name changes
Rename "Code contribution guide" section -> "Contributing to Zulip".
Rename "Contributing to Zulip" page -> "Contributing guide".
Organizational changes to the newly-named "Contributing to Zulip":
Move up "Contributing to Zulip", as the third link in sidebar index.
Move up renamed "Contributing guide" page to the top of this section.
Move up "Zulip code of Conduct", as the second link of this section.
Move down "Licensing", as the last link of this section.
Move "Accessibility" just below "HTML and CSS" in Subsystems section.
Update all links according to the changes above.
Redirects should be added as needed.
Fixes: #22517.
The error message a user gets from the linter when using the
data-tippy-allowHTML attribute now conveys the fact that the
<template> tag is supposed to hold the tooltip content. This
might make understanding the correct workflow easier for
someone who encounters this error.
Removes `LEGACY_PREV_TOPIC` which is no longer needed due to the
message edit history migration.
Also remove additions to the linter exclude list that were added
earlier in this commit series.
We fix the mutation of caller and other bad patterns, as well as
adding explicit typing to make the code readable.
We also update the OpenAPI documentation for previously
undocumented `prev_strem` field in the `/get-message-history`
endpoint for API validation testing.
Co-authored-by: Lauryn Menard <lauryn.menard@gmail.com>
These types will help make iteration on this code easier.
Note that `user_id` can be null due to the fact that
edit history entries before March 2017 did not log
the user that made the edit, which was years after
supporting topic edits (discovered in test deployment
of migration on chat.zulip.org).
Co-authored-by: Lauryn Menard <lauryn.menard@gmail.com>
Zixuan James Li
Lalit Kumar Singh
Sahil Batra
Alex Vandiver
Anders Kaseorg
Lauryn Menard
Aman Agrawal
Tim Abbott
Daniil Fadeev
Hardik Dharmani
Prakhar Pratyush
Lauryn Menard
Yuri Sulyma
David Rosa
Purushottam Tiwari
Julia Bichler
Raghav Luthra
Lauryn Menard