Commit Graph

7887 Commits

Author SHA1 Message Date
Alex Vandiver fd6091ad17 push_notifications: Provide message timestamp in both APNS and GCM notifications. 2023-09-24 15:24:02 -07:00
Alex Vandiver b8581e2895 push_notifications: Provide subsecond granularity on the sent time. 2023-09-24 15:24:02 -07:00
Lauryn Menard 48a1cf04d0 api-docs: Update the add and remove update_message_flags events.
Clarifies that the `all` field in the `op: "add"` event is only
relevant for the `"read"` message flag, and that it will be false
for all other specified flags in theses events.

Deprecates the `all` field in the `op: "remove"` event and document
that it is false for all specified flags.

Updates the deprecated `operation` field description and makes
a few other small revisions to the event text for clarity and
accuracy.
2023-09-22 11:28:09 -07:00
Hemant Umre be653dd5b4 org_settings: Add backend for `realm_jitsi_server_url` setting.
This commit adds a `jitsi_server_url` field to the Realm model, which
will be used to save the URL of the custom Jitsi Meet server. In
the database, `None` will encode the server-level default. We can't
readily use `None` in the API, as it could be confused with "field not
sent". Therefore, we will use the string "default" for this purpose.

We have also introduced `server_jitsi_server_url` in the `/register`
API. This will be used to display the server's default Jitsi server
URL in the settings UI.

The existing `jitsi_server_url` will now be calculated as
`realm_jitsi_server_url || server_jitsi_server_url`.

Fixes a part of #17914.

Co-authored-by: Gaurav Pandey <gauravguitarrocks@gmail.com>
2023-09-21 17:39:10 -07:00
Alex Vandiver 11a1cb9630 narrow: Skip the realm_id limit on joins across to usermessage.
The unique index on `(user_id, message_id)` that is the
`zerver_usermessage` table is rather specific, and even the PostgreSQL
extended statistics are not enough for it to realize there is a
correlation between the `realm_id` in the message table and the
`user_id` in the usermessage table.  This means that adding the
`realm_id` limit when there is a join to `zerver_usermessage` flips
the query plan from a nested loop of unique usermessage index-only
scan, with an index scan of the messages pkey -- to a parallel hash
join of the messages limit with a index scan of just the user_id limit
on usermessages.  It thinks this is necessary because it thinks that
the `realm_id` limit may remove a large number of messages from the
usermessage set -- which is totally untrue.

Remove the `realm_id` limit if we have a usermessage join.
2023-09-21 13:58:34 -07:00
David Rosa 6588190918 help: Delete "Start a new topic" page in favor of "Starting a new topic".
- Deletes page, updates references, and adds URL redirect.
2023-09-21 08:41:40 -07:00
Sahil Batra cb203fbe9a user_groups: Do not allow empty group names in backend.
We now raise error if a user tries to create a group with
empty name or tries to update a group name to be empty.
2023-09-20 15:35:26 -07:00
Alex Vandiver 5ee4b642ad views: Add a /health healthcheck endpoint.
This endpoint verifies that the services that Zulip needs to function
are running, and Django can talk to them.  It is designed to be used
as a readiness probe[^1] for Zulip, either by Kubernetes, or some other
reverse-proxy load-balancer in front of Zulip.  Because of this, it
limits access to only localhost and the IP addresses of configured
reverse proxies.

Tests are limited because we cannot stop running services (which would
impact other concurrent tests) and there would be extremely limited
utility to mocking the very specific methods we're calling to raising
the exceptions that we're looking for.

[^1]: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
2023-09-20 09:53:59 -07:00
Steve Howell a8f5836ee6 tests: Make soft-reactivation tests readable.
The `expected` flag was incredibly confusing, as you
couldn't tell from the calling code what you were
actually expecting to happen.

I avoid the context manager idiom in order to force
the callers to create simple helper functions, and
I de-duplicate some code in some places.

I also force the caller to explicitly soft-deactivate
the user with one simple line of code, so that the
person reading the test doesn't have to research
the side effects of the helper. (And I make it
very easy for new authors to follow the practice
going forward.)

This is also somewhat of a prep commit to avoid
the obfuscated use of refresh_from_db.
2023-09-18 16:55:06 -07:00
Steve Howell 0e261f6ec4 tests: Wrap get_user method.
The get_user function is poorly named, but I don't want to
sweep the entire codebase yet.

It's also nice to have a test wrapper for little experiments
like profiling tests or hunting down calls to refresh_from_db.

It's possible that we would also just change the new wrapper
to more directly call Django. The `get_user` function isn't
used in a ton of real-world places, so we might want the test
code to just bypass the cache.
2023-09-18 16:55:06 -07:00
Steve Howell df43f86cbc tests: Clean up check_has_permission_policies.
I add a bunch of cute helper methods to make
the test a bit more readable.

And then I make sure to get clean objects,
which precludes the need for our callback
functions to refresh the user objects.

And finally I make sure that our validation
functions don't cause any round trips (assuming
we have fetched objects using a standard
Zulip helper, which example_user ensures.)
2023-09-18 16:55:05 -07:00
Lauryn Menard 31daef7f79 response: Remove "result: partially_completed" for success responses.
In feature levels 153 and 154, a new value of "partially_completed"
for `result` in a success (HTTP status code 200) was added for two
endpoints that process messages in batches: /api/delete-topic and
/api/mark-all-as-read.

Prior to these changes, `result` was either "success" or "error" for
all responses, which was a useful API invariant to have for clients.

So, here we remove "partially_completed" as a potential value for
"result" in a response. And instead, for the two endpoints noted
above, we return a boolean field "complete" to indicate if the
response successfully deleted/marked as read all the targeted
messages (complete: true) or if only some of the targeted messages
were processed (complete: false).

The "code" field for an error string that was also returned as part
of a partially completed response is removed in these changes as
well.

The web app does not currently use the /api/mark-all-as-read
endpoint, but it does use the /api/delete-topic endpoint, so these
changes update that to check the `complete` boolean instead of the
string value for `result`.
2023-09-18 13:18:24 -07:00
Mateusz Mandera 3e15ea3f3f scim: Add supporting for syncing the user role.
This adds support for syncing user role via the newly added "role"
attribute, which can be set to either of
['owner', 'administrator', 'moderator', 'member', 'guest'].

Removes durable=True from the atomic decorator of do_change_user_role,
as django-scim2 runs PATCH operations in an atomic block.
2023-09-15 13:15:45 -07:00
Alex Vandiver 4f4627b79b send_email: Use a consistent order when sending custom emails to users. 2023-09-14 17:16:36 -07:00
Anders Kaseorg 28597365da python: Delete superfluous parens.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-13 13:40:19 -07:00
Alex Vandiver e0d3176098 digest: Increase size of stream cache.
Since the cache is flushed when the cutoff or realm changes, the
maximum size of the cache should cap out at the number of streams in
the realm.  Raise the max cache size, now that this will not simply
lead to useless cache space for smaller servers.
2023-09-13 13:25:59 -07:00
Alex Vandiver a8a1f10f3c digest: Clear the cache once we move to a new realm / cutoff value. 2023-09-13 13:25:59 -07:00
Alex Vandiver 39358f77dd digest: Enqueue emails as we generate the contexts.
There is now no longer any reason to have the scheduled_email
enqueuing wait until all of the users' contexts have been generated.
Switch to returning the contexts as an iterator, and send them as we
compute them.
2023-09-13 13:25:59 -07:00
Alex Vandiver b9f72bdd68 digest: Switch loop to early-abort for clarity. 2023-09-13 13:25:59 -07:00
Alex Vandiver b555d3f553 digest: Cache per-stream recent topics, rather than batching.
The query plan for fetching recent messages from the arbitrary set of
streams formed by the intersection of 30 random users can be quite
bad, and can descend into a sequential scan on `zerver_recipient`.
Worse, this work of pulling recent messages out is redone if the
stream appears in the next batch of 30 users.

Instead, pull the recent messages for a stream on a one-by-one basis,
but cache them in an in-memory cache.  Since digests are enqueued in
30-user batches but still one-realm-at-a-time, work will be saved both
in terms of faster query plans whose results can also be reused across
batches.

This requires that we pull the stream-id to stream-name mapping for
_all_ streams in the realm at once, but that is well-indexed and
unlikely to cause performance issues -- in fact, it may be faster
than pulling a random subset of the streams in the realm.
2023-09-13 13:25:59 -07:00
Alex Vandiver ffb6c95bba email_notifications: Make stream_id_map optional to build_message_list.
This feels cleaner than passing an empty dict.
2023-09-13 13:25:59 -07:00
Alex Vandiver f8a9779b54 digest: Rename get_slim_stream_map slightly and explain its name more. 2023-09-13 13:25:59 -07:00
Alex Vandiver bca9821c89 digest: Rename get_recent_streams for clarity. 2023-09-13 13:25:59 -07:00
Alex Vandiver 524d4913b3 digest: Filter out users who have joined recently in SQL. 2023-09-13 13:25:59 -07:00
Alex Vandiver d8668ab242 digest: Narrow the query by only fetching the sender full name. 2023-09-13 13:25:59 -07:00
Alex Vandiver 058a168bfe digest: Rewrite target-user algorithm as one query.
There is no reason to do this set manipulation in Python.
2023-09-13 13:25:59 -07:00
Alex Vandiver 584c202d36 digest: Remove unnecessary should_process_digest function. 2023-09-13 13:25:59 -07:00
David Rosa 6505583b7e help: Update user management pages.
- Documents how to access the "Manage user" tab via a user's profile.
2023-09-13 13:06:04 -07:00
Anders Kaseorg 2665a3ce2b python: Elide unnecessary list wrappers.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-13 12:41:23 -07:00
Anders Kaseorg 8efa418205 typed_endpoint: Add missing tuple comma for OptionalTopic aliases.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-12 11:49:11 -07:00
Anders Kaseorg 6988622fe8 ruff: Enable B023 Function definition does not bind loop variable.
Python’s loop scoping is misdesigned, resulting in a very common
gotcha for functions that close over loop variables [1].  The general
problem is so bad that even the Go developers plan to break
compatibility in order to fix the same design mistake in their
language [2].

Enable the Ruff rule function-uses-loop-variable (B023) [3], which
conservatively prohibits functions from binding loop variables at all.

[1] https://docs.python-guide.org/writing/gotchas/#late-binding-closures
[2] https://go.dev/s/loopvar-design
[3] https://beta.ruff.rs/docs/rules/function-uses-loop-variable/

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-11 18:03:45 -07:00
Anders Kaseorg cf4791264c python: Replace functools.partial with type-safe returns.curry.partial.
The type annotation for functools.partial uses unchecked Any for all
the function parameters (both early and late).  returns.curry.partial
uses a mypy plugin to check the parameters safely.

https://returns.readthedocs.io/en/latest/pages/curry.html

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-11 18:03:45 -07:00
Alex Vandiver b94402152d models: Always search Messages with a realm_id or id limit.
Unless there is a limit on `id`, always provide a `realm_id` limit as
well.  We also notate which index is expected to be used in each
query.
2023-09-11 15:00:37 -07:00
Alex Vandiver f9dd2549eb narrow: Set a realm_id limit on messages in user searches. 2023-09-11 15:00:37 -07:00
Alex Vandiver 067de6f948 coverage: Skip zerver.lib.migrate coverage.
It is only covered when we run migration tests, which we are not
guaranteed to always be able to do.
2023-09-11 15:00:37 -07:00
Alex Vandiver d6745209f2 django: Use .exists() instead of .count() when possible. 2023-09-11 15:00:37 -07:00
Alex Vandiver 9d3d57e786 message_send: Inline single use of filter_by_exact_message_topic.
Matching the topic exactly, as opposed to case-insensitively, is not a
common operation, and one that we want to make difficult to do
accidentally.  Inline the single use case of it.
2023-09-11 15:00:37 -07:00
Alex Vandiver 5a0f4a1a22 messages: Limit to "id" column for max-message-id computation.
This lets PostgreSQL use an "Index Only Scan" which is slightly faster
than an "Index scan".
2023-09-11 15:00:37 -07:00
Alex Vandiver 631868a05b users: Refactor and optimize max_message_id_for_user by removing a join.
This algorithm existed in multiple places, with different queries.
Since we only access properties in the UserMessage table, we
standardize on the much simpler and faster Index Only Scan, rather
than a merge join.
2023-09-11 15:00:37 -07:00
Anders Kaseorg 1905df2342 requirements: Upgrade Python requirements.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-09-09 12:53:39 -07:00
Adrián Oliva 732ad89f3d markdown: Fix URL link topic skipping query.
When searching for links inside a topic name, the question mark (?)
was used to split the topic. If a URL had a query after the URL
(e.g., "?foo=bar"), then the query was trimmed from the URL.

Removing the question mark from `basic_link_splitter` is sufficient
to fix this issue. The `get_web_link_regex` function then removes
the trailing punctuation if any, including literal question marks.

Fixes #26368.
2023-09-08 16:17:11 -07:00
N-Shar-ma 8c91c91d86 widgets: Fix bug where a new line right after /todo broke rendering.
When there was no space right after `/todo` but there was content on a
new line, the message would be rendered plainly, not as a todo widget.
This was because we split on only the space character to then check if
the first token was a valid widget.

Now we split on both spaces and newlines to extract the widget name,
irrespective of whether it is followed by a space or a newline. This
results in the message being rendered as a todo widget as expected.
2023-09-08 15:39:07 -07:00
Lauryn Menard 11adc0f37d demo-organizations: Rename shortend versions of 'demo organization'.
Rename existing shortened references to demo organizations, like
`is_demo_org` or `demo-org-warning`, that have been used in the
codebase so far and replace them to be like the `models.py`
variable: `Realm.demo_organization_scheduled_deletion_date`.
2023-09-08 15:17:23 -07:00
Alex Vandiver 61262c7b9a tabbed_sections: Fix a backtrack-able regex.
This REDOS was not exploitable, as its content is only read from
checked-in files; regardless, simplify it to not backtrack.  We also
do not actually have any location which use leading or trailing
whitespace, so remove those optional bits.
2023-09-08 14:51:51 -07:00
Zixuan James Li 574740dda4 webhooks: Migrate check_send_webhook_message to use @typed_endpoint.
This function is used by almost all webhooks.

To support it, we use the "api_ignore_parameter" flag so that positional
arguments like topic and body that are not intended to be parsed from
the request can be ignored.
2023-09-08 08:20:17 -07:00
Zixuan James Li 910f69465c drafts: Migrate drafts to use @typed_endpoint.
This demonstrates the use of BaseModel to replace a check_dict_only
validator.

We also add support to referring to $defs in the OpenAPI tests. In the
future, we can descend down each object instead of mapping them to dict
for more accurate checks.
2023-09-08 08:20:17 -07:00
Zixuan James Li 6201914fd3 message_edit: Migrate message_edit to use @typed_endpoint.
This demonstrates how an alias is created and its suitable use case, the
use of PathOnly, NonNegativeInt, and Literal.
2023-09-08 08:20:17 -07:00
Zixuan James Li c336bf0398 api: Avoid programming errors due to nested Annotated types.
We want to reject ambiguous type annotations that set ApiParamConfig
inside a Union. If a parameter is Optional and has a default of None, we
prefer Annotated[Optional[T], ...] over Optional[Annotated[T, ...]].

This implements a check that detects Optional[Annotated[T, ...]] and
raise an assertion error if ApiParamConfig is in the annotation. It also
checks if the type annotation contains any ApiParamConfig objects that
are ignored, which can happen if the Annotated type is nested inside
another type like List, Union, etc.

Note that because
param: Annotated[Optional[T], ...] = None
and
param: Optional[Annotated[Optional[T], ...]] = None
are equivalent in runtime prior to Python 3.11, there is no way for us
to distinguish the two. So we cannot detect that in runtime.
See also: https://github.com/python/cpython/issues/90353
2023-09-08 08:20:17 -07:00
Zixuan James Li 5a7b1065e5 api: Rewrite argument type test for clarity.
We refactor HostRequestMock so that it now proper populates the request
body given the post data, assuming that the request is JSON encoded.
2023-09-08 08:20:17 -07:00
Zixuan James Li f4caf9dd79 api: Add new typed_endpoint decorators.
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.
2023-09-08 08:20:17 -07:00