Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
38,549 Commits 28 Branches 143 Tags 615 MiB
Commit Graph

29 Commits

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