[1/6] api: Audit enums to handle unknown values where appropriate - #2339
[1/6] api: Audit enums to handle unknown values where appropriate#2339sm-sayedi wants to merge 11 commits into
Conversation
97db30e to
e02550e
Compare
8b2e321 to
611b6b6
Compare
|
Thanks for working on this @sm-sayedi! This is a large PR (61 commits), I think it'd easier for review if this was divided in multiple PRs instead. So could you please divide it into N PRs, and if you want you can label each with "X/Y" (where X would be the N-th PR of the group, and Y would be the total number of PR in the group, Y could be just left as "N" if total number of PR is not clear.) |
We'll soon change Message.type to use MessageType instead of String. This will make it compatible with the modern values "channel" and "direct" when the server starts sending them in the future. Having it next to Message seems more appropriate than its previous location.
…omJson And change MessageTypeConverter.fromJson to forward to MessageType.fromJson. In future commits, we can use the compatibility logic independent of MessageTypeConverter; simply by using MessageType.fromJson.
As of 2026-06, the server only sends "stream" type we already support. This change is for accepting the modern "channel" type that the server may start sending in the future.
This way, it can also accept "channel" and "direct" as the valid values for the message type when the server starts sending them in the future. As of 2026-06, the server only sends "stream" and "private" for message types. Now, we map them to "channel" and "direct" in deserialization.
The modern type "channel" was added in server-9 (FL-248).
611b6b6 to
e6bad69
Compare
sm-sayedi
changed the title
|
@rajveermalviya Done as requested, with a series of 6 PRs of around 10 commits each. PTAL. |
rajveermalviya
left a comment
rajveermalviya
left a comment
There was a problem hiding this comment.
Thanks @sm-sayedi! I've read through first 7 commits of this PR, comment below.
4983a7c dart [nfc]: Use dot shorthands for MessageType
80bed4b api [nfc]: Move MessageType next to Message
bb5b96a api [nfc]: Move MessageTypeConverter.fromJson logic to MessageType.fromJson
f854928 api: Replace stream with channel for MessageType
9c56c05 api: Change Message.type to MessageType, from String
1aa4aac api: Explain why MessageType cannot be unknown
945522d api: In sendMessage, use "channel" when supported
After reading through these commits, one PR separation feedback would be to include the enums that PR is migrating/auditing in the title or description. Or better yet, we could have one PR with related commits for each enum.
There was a problem hiding this comment.
api: In sendMessage, use "channel" when supported
…
This is nice to have, but let's move it into another PR, because this is unrelated to #1982 and this series of PRs is already pretty large.
| stream, | ||
| direct; | ||
|
|
||
| factory fromJson(String json) { |
There was a problem hiding this comment.
nit: Don't think we've used nameless constructors yet.
| factory fromJson(String json) { | |
| factory MessageType.fromJson(String json) { |
Also without the name, it's easy to confuse it as a method.
| // The server never actually sends "channel" here yet (it's "stream" instead), | ||
| // but we accept both forms for forward-compatibility. |
There was a problem hiding this comment.
Let's have a comment/dartdoc on MessageType explaining this. (For both channel and direct messages). Then we don't have to put this comment on every place MessageType is used.
| if (json == 'stream') json = 'channel'; // TODO(server-future) | ||
| if (json == 'private') json = 'direct'; // TODO(server-future) | ||
| return $enumDecode(_$MessageTypeEnumMap, json); |
There was a problem hiding this comment.
nit: Now that there are more than one cases, let's use switch pattern matching.
There was a problem hiding this comment.
api: Replace `stream` with `channel` for MessageType
…
This too is technically out of scope for #1982, but it's fine to have.
2 participants
Towards #1982.
Goes through all the enums defined in
lib/api(from top to bottom, both through the files and inside the files). Makes the enums accept unknown values or explains why we don't/can't accept them. For finding enum definitions inside a file, I usedenum [^{]+\{regex in VSCode "Find" feature (Cmd+Fshortcut).I have also taken the opportunity of going through all the API enums to use dot shorthands in their usages. To find all the usages of
FooEnum, I used\bFooEnum(\.\w+)regex in VSCode "Search: Find in Files" feature (Shift+Cmd+Fshortcut) with case-sensitivity on and used the capture group ($1) as the replacement.