Commit Graph

112 Commits

Author SHA1 Message Date
Steve Howell 7bb7f2943f event_schema: Finish extraction with realm_emoji/update.
We now no longer define any schemas in test_events--all
of them are in event_schema, which helps our tooling
cross-check schemas for openapi and node tests.
2020-09-25 11:43:20 -07:00
Steve Howell ae4d083a5a event_schema: Extract check_realm_domains_*. 2020-09-25 11:43:20 -07:00
Steve Howell 298bed9fa1 event_schema: Split check_update_message_flags. 2020-09-25 11:43:20 -07:00
Steve Howell f6e0171d02 event_schema: Split check_reaction into add/remove.
It happens that whether you add a reaction or remove
a reaction, we send the exact same fields, just using
a different op code.

This sort of symmetry is actually kind of rare, as
usually "add" events have more fields, and "remove" events
might just send an id of something to remove.

Our openapi schema treats these as two seperate events,
so we are more consistent with it, and it helps our
schema-checking tooling for node fixtures, too.

Note that we now have to exempt the two events from
our openapi checks, due to the is_mirror_dummy field
in the deprecated user block.  We can decide how to
handle this later--one possibility is to just add it
as an optional field on the event_schema side.
2020-09-25 11:43:20 -07:00
Steve Howell b7b2546f44 event_schema: Extract check_subscription_update.
Note that we use value_type for value instead of
bool, since properties can be non-bool things
like color, which we just don't test now.  We
should test them.

We more than compensate for this by checking
the actual value of the value in
check_subscription_update.
2020-09-25 11:43:20 -07:00
Steve Howell b920ebce81 event_schema: Extract check_has_zoom_token. 2020-09-25 11:43:20 -07:00
Steve Howell 0c4286222f event_schema: Extract check_realm_update_dict. 2020-09-25 11:43:20 -07:00
Steve Howell 6ec6525624 event_schema: Extract check_delete_message.
There is a legacy format where we send
singular "message_id" instead of plural
"message_ids".

Then there are different fields for "private"
and "stream" message types.
2020-09-25 11:43:20 -07:00
Steve Howell 88165aee6b event_schema: Extract check_user_group_update. 2020-09-25 11:43:20 -07:00
Steve Howell aaaac11661 event_schema: Extract check_user_group_remove. 2020-09-25 11:43:20 -07:00
Steve Howell 1b7af13f37 event_schema: Extract check_user_group_remove_members. 2020-09-25 11:43:20 -07:00
Steve Howell 19b7739065 event_schema: Extract check_user_group_add_members. 2020-09-25 11:43:20 -07:00
Steve Howell 4084f0b949 event_schema: Extract check_realm_user_add.
Note that we make the schema for profile_data
slightly more realistic, but it doesn't actually get
exercised by our current tests (apart from
making sure it's a dict), since we don't have
profile data for our test realm.

We also don't have the optional fields for bots,
since our tests don't exercise that, nor
delivery_email.

So we exempt realm_user_add_event from openapi
checks for now.

When we try to match the openapi specs better, we
will probably want to add a few tests to test_events.

Obviously getting good coverage for adding users
would be nice for all these scenarios:

    * delivery_email matters
    * bots
    * realm has profile fields
2020-09-25 11:43:19 -07:00
Steve Howell dc2176a965 event_schema: Extract check_presence. 2020-09-25 11:43:19 -07:00
Steve Howell 6c74a44697 data_types: Generalize StringDictType.
This is a prep commit for supporting "presence"
events, where the key of the dictionary is some
arbitrary string like "website" but the value
of the dictionary is another dictionary itself
with keys that are more like variable names.
2020-09-25 11:43:19 -07:00
Steve Howell 4f3d5f2d87 event_schema: Extract check_realm_filters.
We have some known issues with representing
tuples in openapi, so we exempt realm_filters
from the relevant check.
2020-09-25 11:43:19 -07:00
Steve Howell e40a5400e5 event_schema: Extract check_muted_topics.
This also forces us to create TupleType.

We exempt this from the openapi check,
since we haven't figured out how to model
tuples in openapi with the same precision
as event_schema (and it may be impossible).

Long term we just want to stop dealing in
tuples, of course.
2020-09-25 11:43:19 -07:00
orientor 91ca1afe98 data_type: Add StringDict data type.
StringDict is a data type for representing dictionaries where
all keys and values are strings. Add this data type to data_types.py
and edit other files so that this data type is put to use and tested.

(slightly tweaked by @showell to remove a comment and shorten
a var name now that we have a proper data type)
2020-09-25 11:43:19 -07:00
Steve Howell 78a2059b8d event schema: Extract attachment checkers. 2020-09-25 11:43:19 -07:00
Steve Howell 4a947c971d event_schema: Extract check_realm_export.
These are all trivial transformations.

Note that we don't insist timestamps are
floats; the NumberType class allows ints
too.
2020-09-25 11:43:19 -07:00
Steve Howell d28c01284c event_schema: Extract check_hotspots.
This forces us to introduce a NumberType.
2020-09-25 11:43:19 -07:00
Steve Howell cf26151cea event_schema: Use realm_user_person_types.
For realm_user events, we now structure the
person type as a union of dicts, which is
more consistent with how we model this in
our openapi spec.
2020-09-25 11:43:19 -07:00
Steve Howell 10952394b0 test_events: Use int value of message_retention_days.
We also make our schema in event_schema reflect this,
which in turn makes us match the already accurate
openapi spec, so we no longer need to exempt four
types of events from our sanity checks.
2020-09-25 11:43:19 -07:00
Steve Howell 2b76eb767f event_schema: Change propagate_mode to an enum. 2020-09-25 11:43:19 -07:00
Steve Howell aca641a4d1 refactor: Extract data_types module.
Defining types with an object hierarchy
of type classes will allow us to build
functionality that was impossible (or
really janky) with the validators.py
approach of composing functions.

Most of the changes to event_schema.py
were automated search/replaces.

This patch doesn't really yet take
advantage of the new FooType classes,
but we will use it soon to audit our
openapi specs.
2020-09-25 11:43:19 -07:00
Anders Kaseorg 1ded51aa9d python: Replace list literal concatenation with * unpacking.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-09-02 11:15:41 -07:00
orientor 148c375e5b events: Add documentation and tests for `typing: stop` event.
The `typing: stop` event did not have any tests in test_events
hence its documentation wasn't added. So add tests and relevant
documentation for the typing stop event. Also edit the documentation
of `typing: start` to include the fact that servers should use
their own timeout incase `stop` event event isn't received.

Fixes #16122.
2020-08-30 16:43:44 -07:00
orientor 372e010dbb events: Add `op` field to `update_message_flags` events.
`update_message_flags` events used `operation` instead of `op`, the
latter being the standard field used in other events. So add `op`
field to `update_message_flags` and mark `operation` as deprecated,
so that it can be removed later.
2020-08-24 12:42:03 -07:00
sahil839 f046c9c58a streams: Add role field to Subscription objects passed to clients.
This commit adds "role" field to the Subscription objects passed to
clients.  This is important preparation for being able to work on the
frontend for this feature.
2020-08-14 16:33:11 -07:00
Steve Howell 6f97e9dfa9 mypy: Use object, not Any, in event_schema.
This requires a few redundant runtime isinstance
checks, but the extra assertions arguably make
the code more readable, and isinstance checks
are extremely negligible.
2020-08-08 11:30:46 -04:00
Steve Howell 15ffd2b666 event_schema: Extract check_stream_delete. 2020-08-06 12:29:43 -07:00
Steve Howell 059e0bb81e event_schema: Extract check_default_streams. 2020-08-06 12:29:43 -07:00
Steve Howell 36ec1571d4 event_schema: Extract check_default_stream_groups. 2020-08-06 12:29:43 -07:00
Tim Abbott 00fd9afad5 embed: Remove useless 'sender' field.
The variant `update_message` events have this extra sender field not
present in normal update_message events; this field has no purpose, so
we remove it.
2020-08-03 18:04:38 -07:00
Tim Abbott 99a54ba67e tornado: Fix ID lists leaked to the events API.
Apparently, `update_message` events unexpectedly contained what were
intended to be internal data structures about which users were
mentioned in a given message.

The bug has been present and accumulating new data structures for
years.

Fixing this should improve the performance of handling update_message
events as well as cleaning up this API's interface.

This was discovered by our automated API documentation schema checking
tooling detecting these unexpected elements in these event
definitions; that same logic should prevent future bugs like this from
being introduced in the future.
2020-08-03 17:52:39 -07:00
Sumanth V Rao 9b6de63afe stream/docs: Add date_created to Stream.API_FIELDS.
The parameter Stream.date_created is now sent down to the clients
for both:

    - client.get_streams()
    - client.list_subscriptions()

API docs updated for stream and subscriptions.

Fixes #15410
2020-07-27 16:33:36 -07:00
Steve Howell f03605bd73 event_schema: Support plan_type in check_realm_update. 2020-07-24 09:38:34 -07:00
Steve Howell 33f173ae1b event_schema: Use check_realm_update in two more places.
We also have the caller pass in the property name for an
additional sanity check.

Note that we don't yet handle the possibility of extra_data;
that will be a subsequent commit.

Also, the stream_id fields aren't in Realm.property_types,
so we specify their types in the checker.
2020-07-24 09:38:34 -07:00
Steve Howell 176ab66fc7 event_schema: Extract check_realm_user_update.
This a pretty big commit, but I really wanted it
to be atomic.

All realm_user/update events look the same from
the top:

    _check_realm_user_update = check_events_dict(
        required_keys=[
            ("type", equals("realm_user")),
            ("op", equals("update")),
            ("person", _check_realm_user_person),
        ]
    )

And then we have a bunch of fields for person that
are optional, and we usually only send user_id plus
one other field, with the exception of avatar-related
events:

    _check_realm_user_person = check_dict_only(
        required_keys=[
            # vertical formatting
            ("user_id", check_int),
        ],
        optional_keys=[
            ("avatar_source", check_string),
            ("avatar_url", check_none_or(check_string)),
            ("avatar_url_medium", check_none_or(check_string)),
            ("avatar_version", check_int),
            ("bot_owner_id", check_int),
            ("custom_profile_field", _check_custom_profile_field),
            ("delivery_email", check_string),
            ("full_name", check_string),
            ("role", check_int_in(UserProfile.ROLE_TYPES)),
            ("email", check_string),
            ("user_id", check_int),
            ("timezone", check_string),
        ],
    )

I would start the code review by just skimming the changes
to event_schema.py, to get the big picture of the complexity
here.  Basically the schema is just the combined superset of
all the individual schemas that we remove from test_events.

Then I would read test_events.py.

The simplest diffs are basically of this form:

    -  schema_checker = check_events_dict([
    -      ('type', equals('realm_user')),
    -      ('op', equals('update')),
    -      ('person', check_dict_only([
    -          ('role', check_int_in(UserProfile.ROLE_TYPES)),
    -          ('user_id', check_int),
    -      ])),
    -  ])

    # ...
    -  schema_checker('events[0]', events[0])
    +  check_realm_user_update('events[0]', events[0], {'role'})

Instead of a custom schema checker, we use the "superset"
schema checker, but then we pass in the set of fields that we
expect to be there.  Note that 'user_id' is always there.

So most of the heavy lifting happens in this new function
in event_schema.py:

    def check_realm_user_update(
        var_name: str, event: Dict[str, Any], optional_fields: Set[str],
    ) -> None:
        _check_realm_user_update(var_name, event)

        keys = set(event["person"].keys()) - {"user_id"}
        assert optional_fields == keys

But we still do some more custom checks in test_events.py.

custom profile fields: check keys of custom_profile_field

     def test_custom_profile_field_data_events(self) -> None:
+        self.assertEqual(
+            events[0]['person']['custom_profile_field'].keys(),
+            {"id", "value", "rendered_value"}
+        )

+        check_realm_user_update('events[0]', events[0], {"custom_profile_field"})
+        self.assertEqual(
+            events[0]['person']['custom_profile_field'].keys(),
+            {"id", "value"}
+        )

avatar fields: check more specific types, since the superset
    schema has check_none_or(check_string)

     def test_change_avatar_fields(self) -> None:
+        check_realm_user_update('events[0]', events[0], avatar_fields)
+        assert isinstance(events[0]['person']['avatar_url'], str)
+        assert isinstance(events[0]['person']['avatar_url_medium'], str)

+        check_realm_user_update('events[0]', events[0], avatar_fields)
+        self.assertEqual(events[0]['person']['avatar_url'], None)
+        self.assertEqual(events[0]['person']['avatar_url_medium'], None)

Also note that avatar_fields is a set of four fields that
are set in event_schema.

full name: no extra work!

     def test_change_full_name(self) -> None:
-        schema_checker('events[0]', events[0])
+        check_realm_user_update('events[0]', events[0], {'full_name'})

test_change_user_delivery_email_email_address_visibilty_admins:

    no extra work for delivery_email
    check avatar fields more directly

roles (several examples) -- actually check the specific role

     def test_change_realm_authentication_methods(self) -> None:
-            schema_checker('events[0]', events[0])
+            check_realm_user_update('events[0]', events[0], {'role'})
+            self.assertEqual(events[0]['person']['role'], role)

bot_owner_id: no extra work!

-        change_bot_owner_checker_user('events[1]', events[1])
+        check_realm_user_update('events[1]', events[1], {"bot_owner_id"})

-        change_bot_owner_checker_user('events[1]', events[1])
+        check_realm_user_update('events[1]', events[1], {"bot_owner_id"})

-        change_bot_owner_checker_user('events[1]', events[1])
+        check_realm_user_update('events[1]', events[1], {"bot_owner_id"})

timezone: no extra work!

-                timezone_schema_checker('events[1]', events[1])
+                check_realm_user_update('events[1]', events[1], {"email", "timezone"})
2020-07-24 09:38:34 -07:00
Steve Howell a6519e7b8f event_schema: Extract check_user_group_add. 2020-07-22 16:48:19 -07:00
Steve Howell 3f25e52667 event_schema: Extract check_user_status. 2020-07-22 16:48:19 -07:00
Steve Howell 631adc5677 event_schema: Extract check_alert_words. 2020-07-22 16:48:19 -07:00
Steve Howell 0a9a9d8258 event_schema: Extract check_custom_profile_fields. 2020-07-22 16:48:19 -07:00
Steve Howell 7176b90882 event_schema: Extract check_typing_start. 2020-07-22 16:48:19 -07:00
Steve Howell 5f3ea0a659 event_schema: Extract check_invites_changed. 2020-07-22 16:48:19 -07:00
Steve Howell ec17091521 event_schema: Extract check_submessage. 2020-07-22 16:48:19 -07:00
Steve Howell 92136d738a event_schema: Extract check_reaction. 2020-07-22 16:48:19 -07:00
Steve Howell 5209de0261 event_schema: Extract check_update_message_flags. 2020-07-22 16:48:19 -07:00
Steve Howell f2bc22e869 event_schema: Extract check_update_message*. 2020-07-22 16:48:19 -07:00
Steve Howell b81f3433d8 event_schema: Extract check_message. 2020-07-22 16:48:19 -07:00
Steve Howell 385050de20 event_schema: Extract check_realm_bot_(delete/remove).
It is strange that we have both of these events.
2020-07-22 16:48:19 -07:00
Steve Howell 96f5ab1c87 event_schema: Extract check_realm_bot_update. 2020-07-22 16:48:19 -07:00
Steve Howell f5c4ee4477 event_schema: Extract check_realm_bot_add.
Note that we use the actual integer bot_type
value now to determine how we validate
services.
2020-07-22 16:48:19 -07:00
Steve Howell 0a6ce36ac9 event_schema: Extract check_update_global_notifications. 2020-07-22 16:48:19 -07:00
Steve Howell 96b821684b event_schema: Extract check_update_display_settings. 2020-07-22 16:48:19 -07:00
Steve Howell dd5949274d event_schema: Extract check_subscription_peer_*. 2020-07-22 16:48:19 -07:00
Steve Howell 502f1b9fe2 event_schema: Extract check_subscription_remove. 2020-07-22 16:48:19 -07:00
Steve Howell 055f1a590d event_schema: Extract check_subscription_add. 2020-07-22 16:48:19 -07:00
Steve Howell b116f1e911 event_schema: Extract check_stream_update. 2020-07-22 16:48:19 -07:00
Steve Howell 14aa87a168 event_schema: Extract check_stream_create. 2020-07-22 16:48:19 -07:00
Steve Howell a6796e9e86 event_schema: Extract check_realm_update. 2020-07-22 16:48:19 -07:00
Steve Howell e49acfa637 event_schema: Extract event_schema.py.
Obviously, this file will soon grow--this
was the easiest way to start without introducing
noise into other commits.

It will soon be structurally similar
to frontend_tests/node_tests/lib/events.js--I
have some ideas there.  But this should also
help for things like API docs.
2020-07-22 16:48:19 -07:00