Commit Graph

353 Commits

Author SHA1 Message Date
Zixuan James Li 9d448e73d2 decorator: Remove cachify in favor of lru_cache.
`cachify` is essentially caching the return value of a function using only
the non-keyword-only arguments as the key.

The use case of the function in the backend can be sufficiently covered by
`functools.lru_cache` as an unbound cache. There is no signficant difference
apart from `cachify` overlooking keyword-only arguments, and
`functools.lru_cache` being conveniently typed.

Signed-off-by: Zixuan James Li <359101898@qq.com>
2022-04-14 12:44:35 -07:00
Anders Kaseorg 5f92078d07 request: Add a var_name parameter to converter.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-03-15 13:02:02 -07:00
Anders Kaseorg c3788ab847 validator: Replace converter=int with json_validator=check_int.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-03-15 13:02:02 -07:00
Anders Kaseorg b0ce4f1bce docs: Fix many spelling mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-02-07 18:51:06 -08:00
Lauryn Menard 3be622ffa7 backend: Add request as parameter to json_success.
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.

Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
2022-02-04 15:16:56 -08:00
Anders Kaseorg ee71650908 python: Replace requests.packages.urllib3 alias with urllib3.
requests stopped vendoring urllib3 in 2.16.0 (2017-05-26).

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-01-23 22:14:17 -08:00
Anders Kaseorg 18aa4f4df4 tornado: Replace deprecated urllib3 method_whitelist.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-01-23 22:14:17 -08:00
Abhijeet Prasad Bodas 15e8717847 notifications: Don't enqueue notifications for bots.
This replaces the temporary (and testless) fix in
24b1439e93 with a more permanent
fix.

Instead of checking if the user is a bot just before
sending the notifications, we now just don't enqueue
notifications for bots. This is done by sending a list
of bot IDs to the event_queue code, just like other
lists which are used for creating NotificationData objects.

Credit @andersk for the test code in `test_notification_data.py`.
2022-01-03 09:55:06 -08:00
Alex Vandiver bc5539d871 tornado: Move SIGTERM shutdown handler into a callback.
A SIGTERM can show up at any point in the ioloop, even in places which
are not prepared to handle it.  This results in the process ignoring
the `sys.exit` which the SIGTERM handler calls, with an uncaught
SystemExit exception:

```
2021-11-09 15:37:49.368 ERR  [tornado.application:9803] Uncaught exception
Traceback (most recent call last):
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/http1connection.py", line 238, in _read_message
    delegate.finish()
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/httpserver.py", line 314, in finish
    self.delegate.finish()
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/routing.py", line 251, in finish
    self.delegate.finish()
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 2097, in finish
    self.execute()
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 2130, in execute
    **self.path_kwargs)
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/gen.py", line 307, in wrapper
    yielded = next(result)
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 1510, in _execute
    result = method(*self.path_args, **self.path_kwargs)
  File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/handlers.py", line 150, in get
    request = self.convert_tornado_request_to_django_request()
  File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/handlers.py", line 113, in convert_tornado_request_to_django_request
    request = WSGIRequest(environ)
  File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/django/core/handlers/wsgi.py", line 66, in __init__
    script_name = get_script_name(environ)
  File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/event_queue.py", line 611, in <lambda>
    signal.signal(signal.SIGTERM, lambda signum, stack: sys.exit(1))
SystemExit: 1
```

Supervisor then terminates the process with a SIGKILL, which results
in dropping data held in the tornado process, as it does not dump its
queue.

The only command which is safe to run in the signal handler is
`ioloop.add_callback_from_signal`, which schedules the callback to run
during the course of the normal ioloop.  This callbacks does an
orderly shutdown of the server and the ioloop before exiting.
2021-11-12 09:57:23 -08:00
rht bb8504d925 lint: Fix typos found by codespell. 2021-10-19 16:51:13 -07:00
Tim Abbott 272e81988b tornado: Make process_notification hack avoid import loops.
This fixes a problem where we could not import zerver.lib.streams from
zerver.lib.message, which would otherwise be reasonable, because the
former implicitly imported many modules due to this issue.
2021-09-28 10:07:36 -07:00
Anders Kaseorg 646c04eff2 Rename default branch to ‘main’.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-09-06 12:56:35 -07:00
PIG208 53888e5a26 request: Refactor ZulipRequestNotes to RequestNotes.
This utilizes the generic `BaseNotes` we added for multipurpose
patching. With this migration as an example, we can further support
more types of notes to replace the monkey-patching approach we have used
throughout the codebase for type safety.
2021-09-03 08:48:45 -07:00
PIG208 aa9d73c9f6 typing: Improve typing with assertions.
This fixes some mypy errors discovered with django-stubs.
2021-08-20 05:54:19 -07:00
Anders Kaseorg 4206e5f00b python: Remove locally dead code.
These changes are all independent of each other; I just didn’t feel
like making dozens of commits for them.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-19 01:51:37 -07:00
Abhijeet Prasad Bodas ac70a2d2e1 notifications: Fix unnecessary wildcard mention notifications.
This fixes a bug where email notifications were sent for wildcard
mentions even if the `enable_offline_email_notifications` setting was
turned off.
This was because the `notification_data` class incorrectly considered
`wildcard_mentions_notify` as an indeoendent setting, instead of a wrapper
around `enable_offline_email_notifications` and `enable_offline_push_notifications`.

Also add a test for this case.
2021-08-13 09:48:18 -07:00
Anders Kaseorg 3665deb93a python: Remove unnecessary intermediate lists.
Generated automatically by pyupgrade.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-02 15:53:52 -07:00
Sahil Batra 0364d0c8ca events: Add "user_settings_object" to client_capabilities.
This commit adds "user_settings_object" field to
client_capabilities which will be used to determine
if the client needs 'update_display_settings' and
'update_global_notifications' event.
2021-08-01 15:30:17 -07:00
Gaurav Pandey d89b4dcd0b api: Return zulip_merge_base alongside zulip_version.
Return zulip_merge_base alongside zulip_version
in `/register`, `/event` and `/server_settings`
endpoint so that the value can be used by other
clients.
2021-07-30 12:03:41 -07:00
Abhijeet Prasad Bodas de04f0ad67 notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:

1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
    return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.

Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)

then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.

The buggy code was this in `handle_push_notifications`:
```
if not (
    receives_offline_push_notifications(user_profile)
    or receives_online_push_notifications(user_profile)
):
    return

    // send notifications
```

This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.

2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).

3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-28 13:55:25 -07:00
Anders Kaseorg fd0ab7c4ec tornado: Call close() on Django HttpResponse objects.
This is necessary to break the uncollectable reference cycle created
by our ‘request_notes.saved_response = json_response(…)’, Django’s
‘response._resource_closers.append(request.close)’, and Python’s
https://bugs.python.org/issue44680.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-20 11:07:36 -07:00
Anders Kaseorg 6564b258f1 request: Weaken ZulipRequestNotes.tornado_handler reference.
This prevents a memory leak arising from Python’s inability to collect
a reference cycle from a WeakKeyDictionary value to its key
(https://bugs.python.org/issue44680).

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-19 16:48:23 -07:00
Anders Kaseorg 7c32134fb5 Revert "Revert "request: Refactor to record rate limit data using ZulipRequestNotes.""
This reverts commit 49eab4efef.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-19 16:48:23 -07:00
Anders Kaseorg fb3ddf50d4 python: Fix mypy no_implicit_reexport errors.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-16 14:02:31 -07:00
PIG208 49eab4efef Revert "request: Refactor to record rate limit data using ZulipRequestNotes."
This reverts commit 3f9a5e1e17.
2021-07-16 09:01:20 -07:00
PIG208 c03b9c95ad request: Store client information using ZulipRequestNotes.
This concludes the HttpRequest migration to eliminate arbitrary
attributes (except private ones that are belong to django) attached
to the request object during runtime and migrated them to a
separate data structure dedicated for the purpose of adding
information (so called notes) to a HttpRequest.
2021-07-14 12:01:07 -07:00
PIG208 5167a93229 request: Move tornado_handler to ZulipRequestNotes. 2021-07-14 12:01:07 -07:00
PIG208 742c17399e request: Move miscellaneous attributes to ZulipRequestNotes.
This includes the migration of fields that require trivial changes
to be migrated to be stored with ZulipRequestNotes.

Specifically _requestor_for_logs, _set_language, _query, error_format,
placeholder_open_graph_description, saveed_response, which were all
previously set on the HttpRequest object at some point. This migration
allows them to be typed.
2021-07-14 12:01:07 -07:00
PIG208 5475334b16 request: Refactor to store requestor_for_logs in ZulipRequestNotes. 2021-07-14 12:01:07 -07:00
PIG208 3f9a5e1e17 request: Refactor to record rate limit data using ZulipRequestNotes.
We will no longer use the HttpRequest to store the rate limit data.
Using ZulipRequestNotes, we can access rate_limit and ratelimits_applied
with type hints support. We also save the process of initializing
ratelimits_applied by giving it a default value.
2021-07-14 12:01:07 -07:00
PIG208 da6e5ddcae request: Move log_data from HttpRequest to ZulipRequestNotes. 2021-07-14 12:01:05 -07:00
PIG208 03693cd27e request: Map HttpRequest to ZulipRequestNotes for typing.
We create a class called ZulipRequestNotes as a new home to all the
additional attributes that we add to the Django HttpRequest object.
This allows mypy to do the typecheck and also enforces type safety.

Most of the attributes are added in the middleware, and thus it is
generally safe to assert that they are not None in a code path that
goes through the middleware. The caller is obligated to do manual
the type check otherwise.

This also resolves some cyclic dependencies that zerver.lib.request
have with zerver.lib.rate_limiter and zerver.tornado.handlers.
2021-07-14 11:52:42 -07:00
Abhijeet Prasad Bodas bf15c0235a notifications: Remove unused fields in queue events.
* `stream_name`: This field is actually redundant. The email/push
  notifications handlers don't use that field from the dict, and they
  anyways query for the message, so we're safe in deleting this field,
  even if in the future we end up needing the stream name.

* `timestamp`: This is totally unused by the email/push notification
  handlers, and aren't sent to push clients either.

* `type` is used only for the push notifications handler, since only
  push notifications can be revoked, so we move them to only run there.
2021-07-08 11:22:45 -07:00
Anders Kaseorg 4a04cda956 tornado: Remove unused logger variable.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-07 15:13:40 -07:00
Abhijeet Prasad Bodas 167be7dbdc mentions: Send user group mention data to notification notices.
We will later use this data to include text like:
`<sender> mentioned @<user_group>` instead of the current
`<sender> mentioned you` when someone mentions a user group
the current user is a part of in email/push notification.

Part of #13080.
2021-07-05 14:23:59 -07:00
Tim Abbott 64aa8f80a0 events: Add heartbeat events to tests and documentation.
Heartbeat events are an important part of the API, even though they
are noops, so it's important to document them.
2021-07-01 17:14:32 -07:00
PIG208 dcbb2a78ca python: Migrate most json_error => JsonableError.
JsonableError has two major benefits over json_error:
* It can be raised from anywhere in the codebase, rather than
  being a return value, which is much more convenient for refactoring,
  as one doesn't potentially need to change error handling style when
  extracting a bit of view code to a function.
* It is guaranteed to contain the `code` property, which is helpful
  for API consistency.

Various stragglers are not updated because JsonableError requires
subclassing in order to specify custom data or HTTP status codes.
2021-06-30 16:22:38 -07:00
Abhijeet Prasad Bodas e196ea7e64 event_queue: Consistently user `user_notifications_data` as variable name.
This disambiguates this object from the those sent via the `send_event`
calls, which are also called "user_data".
2021-06-25 08:54:01 -07:00
Abhijeet Prasad Bodas 733e0ae75e notification_data: Rename `sender_id` -> `acting_user_id`.
This better shows the situation for message edits, where we use the same
class.
2021-06-25 08:54:00 -07:00
Abhijeet Prasad Bodas 1cf1d147aa event_queue: Move notification trigger logic to notification_data.
This removes some complexity from the event_queue module.
To avoid code duplication, we reduce the `is_notifiable` methods to
internally just call the `trigger` methods and check their return value.
2021-06-24 09:35:18 -07:00
Abhijeet Prasad Bodas 66192825c0 maybe_enqueue_notifications: Take in notification_data dataclass.
* Modify `maybe_enqueue_notifications` to take in an instance of the
dataclass introduced in 951b49c048.

* The `check_notify` tests tested the "when to notify" logic in a way
which involved `maybe_enqueue_notifications`. To simplify things, we've
earlier extracted this logic in 8182632d7e.
So, we just kill off the `check_notify` test, and keep only those parts
which verify the queueing and return value behavior of that funtion.

* We retain the the missedmessage_hook and message
message_edit_notifications since they are more integration-style.

* There's a slightly subtle change with the missedmessage_hook tests.
Before this commit, we short-circuited the hook if the sender was muted
(5a642cea11).
With this commit, we delegate the check to our dataclass methods.
So, `maybe_enqueue_notifications` will be called even if the sender was
muted, and the test needs to be updated.

* In our test helper `get_maybe_enqueue_notifications_parameters` which
generates default values for testing `maybe_enqueue_notifications` calls,
we keep `message_id`, `sender_id`, and `user_id` as required arguments,
so that the tests are super-clear and avoid accidental false positives.

* Because `do_update_embedded_data` also sends `update_message` events,
we deal with that case with some hacky code for now. See the comment
there.

This mostly completes the extraction of the "when to notify" logic into
our new `notification_data` module.
2021-06-24 09:35:17 -07:00
Abhijeet Prasad Bodas dedc39f139 notifications_data: Rename `id` -> `user_id`.
We also make this a mandatory named argument for our test helper
for clarity.
2021-06-24 17:34:50 +05:30
Abhijeet Prasad Bodas f6e705d477 maybe_enqueue_notifications: Require all keyword arguments.
This is a more readable way to call the function.
2021-06-24 17:34:50 +05:30
Abhijeet Prasad Bodas 226ac4bd3b tornado: Use `UserMessageNotificationsData` for update_message.
This lets us simplify what had previously been somewhat duplicated
logic between the send and edit code paths.
2021-06-22 10:27:59 -07:00
Abhijeet Prasad Bodas c3fb413119 message edit: Don't send mentioned user_ids in event dict.
We already have this data in the `flags` for each user, so no need to
send this set/list in the event dictionary.

The `flags` in the event dict represent the after-message-update state,
so we can't avoid sending `prior_mention_user_ids`.
2021-06-22 10:27:55 -07:00
Abhijeet Prasad Bodas c6d696cf61 process_message_event: Deduplicate `is_notifiable` logic.
We use the `UserMessageNotificationsData` methods to check if the
message is notifiable, and use a cleaner early continue pattern.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas aeb2ad5f46 notification_data: Annotate `flags` with `Collection`.
Since `flags` here could be iterated through multiple times
(to check for push/email notifiability), we use `Collection`.
Inspired by 871e73ab8f.

The other change here in the `event_queue` code is prep for using
the `UserMessageNotificationsData` class there.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas ed58393079 message send: Optimize how user data is sent to Tornado.
Before this commit, we used to pre-calculate flags for user data and send
it to Tornado, like so:
```
{
    "id": 10,
    "flags": ["mentioned"],
    "mentioned": true,
    "online_push_enabled": false,
    "stream_push_notify": false,
    "stream_email_notify": false,
    "wildcard_mention_notify": false,
    "sender_is_muted": false,
}
```

This has the benefit of simplifying the logic in the event_queue code a bit.

However, because we sent such an object for each user receiving the event,
the string keys (like "stream_email_notify") get duplicated in the JSON
blob that is sent to Tornado.

For 1000 users, this data may take up upto ~190KB of space, which can
cause performance degradation in large organisations.

Hence, as an alternative, we send just the list of user_ids fitting
each notification criteria, and then calculate the flags in Tornado.
This brings down the space to ~60KB for 1000 users.

This commit reverts parts of following commits:
- 2179275
- 40cd6b5

We will in the future, add helpers to create `UserMessageNotificationsData`
objects from these lists, so as to avoid code duplication.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas 6167c36adc event_queue: Translation code for user data migration.
This is separate from the next commit for ease of testing.
To verify that the compatibility code works correctly, all message send
and event_queue tests from our test suite should pass on just this commit.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas 3ff3ded1ae process_message_event: Always calculate `online_push_enabled`.
Unlike `receiver_is_off_zulip`, fetching from a dict is pretty cheap,
so we can calculate `online_push_enabled` along with the other
variables.

This is a prep change to start using the dataclass introduced in the
earlier commit in this code.
2021-06-15 12:30:31 -07:00