Commit Graph

38 Commits

Author SHA1 Message Date
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