Specify more details of chat presence data #256
Assignees
Labels
chat
Related to the Chat SDK.
Comments
lawrence-forooghian
added a commit
to ably/ably-chat-swift
that referenced
this issue
Dec 10, 2024
Public API improvements: - You can now pass any JSON value as presence data (previously, the top-level value had to be an object, and you could not pass arrays or nested objects). This brings us in line with CHA-PR2a. - Do not expose the `userCustomData` property of the presence data object in the public API; this is an implementation detail. - Conform to the `ExpressibleBy*Literal` protocols, making it easier to create a JSON value. I have deliberately chosen to make the data-arg variants of the presence operation methods take a non-optional PresenceData. This is to minimise confusion between the absence of presence data and a presence data with JSON value `null`. This is how I wrote this API in 20e7f5f, but I didn’t restore it properly in 4ee16bd. The JSONValue type introduced here is based on the example given in [1]. Internals improvements: - Fix the presence object that gets passed to ably-cocoa when the user specifies no presence data; we were previously passing an empty string as the presence data in this case. (See below for where I got the “correct” behaviour from.) - Simplify the way in which we decode the presence data received from ably-cocoa (i.e. don’t do a round-trip of JSON serialization and deserialization); this comes at the cost of not getting a little bit for free from Swift’s serialization mechanism, but I think it’s worth it The behaviour of how to map the chat presence data public API to the object exchanged with the core SDK is not currently fully specified. So, the behaviour that I’ve implemented here is based on the behaviour of the JS Chat SDK at 69ea478. I’ve created spec issue [2] in order to specify this stuff properly, but I’m in a slight rush to get this public API fixed before we release our first beta, so I’ll address this later. Resolves #178. [1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-literals/ [2] ably/specification#256
lawrence-forooghian
added a commit
to ably/ably-chat-swift
that referenced
this issue
Dec 10, 2024
Public API improvements: - You can now pass any JSON value as presence data (previously, the top-level value had to be an object, and you could not pass arrays or nested objects). This brings us in line with CHA-PR2a. - Do not expose the `userCustomData` property of the presence data object in the public API; it’s an implementation detail. - Conform to the `ExpressibleBy*Literal` protocols, making it easier to create a JSON value. I have deliberately chosen to make the data-arg variants of the presence operation methods take a non-optional PresenceData. This is to minimise confusion between the absence of presence data and a presence data with JSON value `null`. This is how I wrote this API in 20e7f5f, but I didn’t restore it properly in 4ee16bd. The JSONValue type introduced here is based on the example given in [1]. Internals improvements: - Fix the presence object that gets passed to ably-cocoa when the user specifies no presence data; we were previously passing an empty string as the presence data in this case. (See below for where I got the “correct” behaviour from.) - Simplify the way in which we decode the presence data received from ably-cocoa (i.e. don’t do a round-trip of JSON serialization and deserialization); this comes at the cost of not getting a little bit for free from Swift’s serialization mechanism, but I think it’s worth it The behaviour of how to map the chat presence data public API to the object exchanged with the core SDK is not currently fully specified. So, the behaviour that I’ve implemented here is based on the behaviour of the JS Chat SDK at 69ea478. I’ve created spec issue [2] in order to specify this stuff properly, but I’m in a slight rush to get this public API fixed before we release our first beta, so I’ll address this later. Resolves #178. [1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-literals/ [2] ably/specification#256
lawrence-forooghian
added a commit
that referenced
this issue
Dec 11, 2024
Based on the behaviour of JS at 69ea478. Note that the <pre> is causing Textile to render CHA-PR2b in a separate <ul> to CHA-PR2a. I briefly tried various combinations of indentation, didn’t get anywhere, and gave up trying to fix it. Resolves #256.
lawrence-forooghian
added a commit
to ably/ably-chat-swift
that referenced
this issue
Dec 17, 2024
Similar issue to the one which was fixed in 7fcab5c. Haven’t added tests for the handling of the incoming message, since there were no existing tests to update and I didn’t feel like writing them; will leave this for whoever writes these tests in #88. The approach that I’ve taken here of using a DTO is consistent with the approach that we use for presence data. I should have done this in 7fcab5c too; will do it separately. The spec doesn’t describe how to behave if the user doesn’t pass headers or metadata; I’ve taken the behaviour of passing an empty object from the JS Chat SDK at 6d1b04a. Have created spec issue [1] for specifying this. Resolves #198. [1] ably/specification#256
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is currently a placeholder issue, but off top of head here are some things that should be specified, which are based on looking at the JS codebase:
null(JSON value)userCustomDatakey is present)The text was updated successfully, but these errors were encountered: