The returns plugin hasn’t been updated for mypy ≥ 1.6. This
annotation is more limited in that it only supports a fixed number of
positional arguments and no keyword arguments, but is good enough for
our purposes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Collapsing was done incorrectly, as 65c400e06d added `zulip_version`
and `zulip_feature_level`, but did not update the virtual event logic
to copy those new values into the virtual event.
However, it is unlikely that a server will be upgraded multiple times
in quick enough succession for this to ever be relevant. Remove the
logic, which is additional complication for little or no gain.
As premonitioned in c741c527d7, it is
indeed possible for `get_handler_by_id` to error out by cause the
handler has been unset elsewhere.
Protect the callsites of `get_handler_by_id` to be able to gracefully
handle when the handler has already done away.
4af00f61a8 claimed that `on_finish` and
`on_connection_close` were mutually exclusive. In cases where a
`DELETE` is called on the queue while a longpoll is in progress, this
can cause _both_ to happen:
- The `DELETE` pushes a `cleanup_queue` event, which triggers
`finish_handler` to begin pushing out an empty event response to the
longpoll connection.
- In the midst of that, in an `await`, the longpoll connection drops,
and `on_connection_close` clears the handler.
- The `await` resumes, calls `finish`, and attempts to clear the
handler.
The easiest solution is to make `clear_handler_by_id` tolerant to
multiple attempts to clear it. Since these processes run in parallel,
it means that parts may have a `handler_id` but `get_handler_by_id`
may error in attempting to look it up. We have not observed this in
testing, and I cannot currently prove it is impossible.
This partially reverts 579bdc18f85ea8599c8cf1f53ddb02fd41d97993; it
assumed (based on its documentation) that `on_finish` was called for
all requests, even client-terminated ones. This is not accurate; it
is only called when the request calls `finish`, which only happens for
successful requests. This caused every client-closed connection to
leak a handler (ironically, exactly re-introducing the bug previously
fixed in 12a5a3a6e1).
This behaviour was obscured by the development environment's proxy;
see comment added in the previous commit.
Instead of replacing the `clear_handler_by_id` call into
`ClientDescriptor.disconnect_handler`, we instead place it on
`AsyncDjangoHandler.on_connection_close`. This is more correct for
a few reasons:
- `on_connection_close` will be called if the client goes away during
a request without a client descriptor. If the handler garbage
collection of handlers runs inside the ClientDescriptor, we leak
handlers.
- `disconnect_handler` also runs when successfully sending an event,
which already calls `on_finish`. We avoid double-calling
`clear_handler_by_id` by doing it in two clearly exclusive cases,
`on_finish` and `on_connection_close`.
- It combines the creation and garbage collection logic into one
file, decreasing action at a distance which causes memory leaks.
This commit adds code to not include original details of senders like
name, email and avatar url in the message objects sent through events
and in the response of endpoint used to fetch messages.
This is the last major commit for the project to add support for
limiting guest access to an entire organization.
Fixes#10970.
initialize() is called on every request, and stored the
`RequestHandler` (and thus `HTTPServerRequest`) in a global shared
dict. However, the object is only removed from that structure if the
request was successful. This means that failed requests (such as 405
Method Not Allowed) leaked `RequestHandler`s and
`HTTPServerRequest`s.
Move the cleanup to `on_finish`, which is called at the close of all
requests, async and not, successful or not.
These are not part of the API, and lead to moderately confusing
behaviour -- they block (or not) requested, but of course send no
actual data in the body.
This commit renames the keyword 'pm' to 'dm' in the
'pm_mention_email_disabled_user_ids' and
'pm_mention_push_disabled_user_ids' attributes of the
'RecipientInfoResult' dataclass.
'pm' and 'dm' are the acronyms for 'private message' and
'direct message' respectively.
It includes 'TODO/compatibility' code to support the old format
fields in the tornado queues during the Zulip server upgrades.
This commit completes the notifications part of the @topic
wildcard mention feature.
Notifications are sent to the topic participants for the
@topic wildcard mention.
This prep commit replaces the 'wildcard' keyword in the codebase
with 'stream_wildcard' at some places for better readability, as
we plan to introduce 'topic_wildcards' as a part of the
'@topic mention' project.
Currently, 'wildcards = ["all", "everyone", "stream"]' which is an
alias to mention everyone in the stream, hence better renamed as
'stream_wildcards'.
Eventually, we will have:
'stream_wildcard' as an alias to mention everyone in the stream.
'topic_wildcard' as an alias to mention everyone in the topic.
'wildcard' refers to 'stream_wildcard' and 'topic_wildcard' as a whole.
This is a first step toward two goals:
* support dictionary-like narrows when registering events
* use readable dataclasses internally
This is gonna be a somewhat complicated exercise due to how
events get serialized, but fortunately this interim step
doesn't require any serious shims, so it improves the codebase
even if the long-term goals may take a while to get sorted
out.
The two places where we have to use a helper to convert narrows
from tuples to dataclasses will eventually rely on their callers
to do the conversion, but I don't want to re-work the entire
codepath yet.
Note that the new NarrowTerm dataclass makes it more explicit
that the internal functions currently either don't care about
negated flags or downright don't support them. This way mypy
protects us from assuming that we can just add negated support
at the outer edges.
OTOH I do make a tiny effort here to slightly restructure
narrow_filter in a way that paves the way for negation support.
The bigger goal by far, though, is to at least support the
dictionary format.
We no longer pass in a big opaque event to narrow_filter
(which is inside build_narrow_filter). We instead explicitly
pass in message and flags. This leads to a bit more type
safety, and it's also more flexible. There's no reason to
build an entire event just to see if a message belongs to
a narrow.
The changes to the test work around the fact that the fixtures
are sloppy with types. I plan a subsequent commit to clean
up those tests significantly.
django-stubs 4.2.1 gives transaction.on_commit a more accurate type
annotation, but this exposed that mypy can’t handle the lambda default
parameters that we use to recapture loop variables such as
for stream_id in public_stream_ids:
peer_user_ids = …
event = …
transaction.on_commit(
lambda event=event, peer_user_ids=peer_user_ids: send_event(
realm, event, peer_user_ids
)
)
https://github.com/python/mypy/issues/15459
A workaround that mypy accepts is
transaction.on_commit(
(
lambda event, peer_user_ids: lambda: send_event(
realm, event, peer_user_ids
)
)(event, peer_user_ids)
)
But that’s kind of ugly and potentially error-prone, so let’s make a
helper function for this very common pattern.
send_event_on_commit(realm, event, peer_user_ids)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Anders Kaseorg
Tim Abbott
Alex Vandiver
Sahil Batra
Prakhar Pratyush
Steve Howell
Lauryn Menard