Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-5178] Fix sending and receiving of message headers and metadata #196

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions Sources/AblyChat/ChatAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ internal final class ChatAPI: Sendable {
}

let endpoint = "\(apiVersionV2)/rooms/\(roomId)/messages"
var body: [String: Any] = ["text": params.text]
var body: [String: JSONValue] = ["text": .string(params.text)]

// (CHA-M3b) A message may be sent without metadata or headers. When these are not specified by the user, they must be omitted from the REST payload.
if let metadata = params.metadata {
body["metadata"] = metadata
body["metadata"] = .object(metadata.mapValues(\.toJSONValue))
}

if let headers = params.headers {
body["headers"] = headers
body["headers"] = .object(headers.mapValues(\.toJSONValue))
}

let response: SendMessageResponse = try await makeRequest(endpoint, method: "POST", body: body)
Expand All @@ -63,10 +63,16 @@ internal final class ChatAPI: Sendable {
}

// TODO: https://github.com/ably-labs/ably-chat-swift/issues/84 - Improve how we're decoding via `JSONSerialization` within the `DictionaryDecoder`
private func makeRequest<Response: Codable>(_ url: String, method: String, body: [String: Any]? = nil) async throws -> Response {
try await withCheckedThrowingContinuation { continuation in
private func makeRequest<Response: Codable>(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response {
let ablyCocoaBody: Any? = if let body {
JSONValue.object(body).toAblyCocoaData
} else {
nil
}

return try await withCheckedThrowingContinuation { continuation in
do {
try realtime.request(method, path: url, params: [:], body: body, headers: [:]) { paginatedResponse, error in
try realtime.request(method, path: url, params: [:], body: ablyCocoaBody, headers: [:]) { paginatedResponse, error in
if let error {
// (CHA-M3e) If an error is returned from the REST API, its ErrorInfo representation shall be thrown as the result of the send call.
continuation.resume(throwing: ARTErrorInfo.create(from: error))
Expand All @@ -93,8 +99,7 @@ internal final class ChatAPI: Sendable {

private func makePaginatedRequest<Response: Codable & Sendable & Equatable>(
_ url: String,
params: [String: String]? = nil,
body: [String: Any]? = nil
params: [String: String]? = nil
) async throws -> any PaginatedResult<Response> {
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<PaginatedResultWrapper<Response>, _>) in
do {
Expand Down
21 changes: 16 additions & 5 deletions Sources/AblyChat/DefaultMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,16 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
channel.subscribe(RealtimeMessageName.chatMessage.rawValue) { message in
Task {
// TODO: Revisit errors thrown as part of https://github.com/ably-labs/ably-chat-swift/issues/32
guard let data = message.data as? [String: Any],
let text = data["text"] as? String
guard let ablyCocoaData = message.data,
let data = JSONValue(ablyCocoaData: ablyCocoaData).objectValue,
let text = data["text"]?.stringValue
else {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data or text")
}

guard let extras = try message.extras?.toJSON() else {
guard let ablyCocoaExtras = message.extras,
let extras = try JSONValue(ablyCocoaData: ablyCocoaExtras.toJSON()).objectValue
else {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without extras")
}

Expand All @@ -74,8 +77,16 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without clientId")
}

let metadata = data["metadata"] as? Metadata
let headers = extras["headers"] as? Headers
let metadata: Metadata? = if let metadataJSONObject = data["metadata"]?.objectValue {
try metadataJSONObject.mapValues { try MetadataValue(jsonValue: $0) }
} else {
nil
}
let headers: Headers? = if let headersJSONObject = extras["headers"]?.objectValue {
try headersJSONObject.mapValues { try HeadersValue(jsonValue: $0) }
} else {
nil
}

guard let action = MessageAction.fromRealtimeAction(message.action) else {
return
Expand Down
8 changes: 4 additions & 4 deletions Sources/AblyChat/DefaultPresence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
let dto = PresenceDataDTO(userCustomData: data)

return try await withCheckedThrowingContinuation { continuation in
channel.presence.enterClient(clientID, data: JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
channel.presence.enterClient(clientID, data: dto.toJSONValue.toAblyCocoaData) { [logger] error in
if let error {
logger.log(message: "Error entering presence: \(error)", level: .error)
continuation.resume(throwing: error)
Expand Down Expand Up @@ -149,7 +149,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
let dto = PresenceDataDTO(userCustomData: data)

return try await withCheckedThrowingContinuation { continuation in
channel.presence.update(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
channel.presence.update(dto.toJSONValue.toAblyCocoaData) { [logger] error in
if let error {
logger.log(message: "Error updating presence: \(error)", level: .error)
continuation.resume(throwing: error)
Expand Down Expand Up @@ -183,7 +183,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
let dto = PresenceDataDTO(userCustomData: data)

return try await withCheckedThrowingContinuation { continuation in
channel.presence.leave(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
channel.presence.leave(dto.toJSONValue.toAblyCocoaData) { [logger] error in
if let error {
logger.log(message: "Error leaving presence: \(error)", level: .error)
continuation.resume(throwing: error)
Expand Down Expand Up @@ -237,7 +237,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
throw error
}

let jsonValue = JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData)
let jsonValue = JSONValue(ablyCocoaData: ablyCocoaPresenceData)

do {
return try PresenceDataDTO(jsonValue: jsonValue)
Expand Down
38 changes: 37 additions & 1 deletion Sources/AblyChat/Headers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,47 @@

public enum HeadersValue: Sendable, Codable, Equatable {
case string(String)
case number(Int) // Changed from NSNumber to Int to conform to Codable. Address in linked issue above.
case number(Double) // Changed from NSNumber to Double to conform to Codable. Address in linked issue above.
case bool(Bool)
case null
}

extension HeadersValue: JSONDecodable {
internal enum JSONDecodingError: Error {
case unsupportedJSONValue(JSONValue)
}

internal init(jsonValue: JSONValue) throws {
self = switch jsonValue {
case let .string(value):
.string(value)
case let .number(value):
.number(value)
case let .bool(value):
.bool(value)
case .null:
.null
default:
throw JSONDecodingError.unsupportedJSONValue(jsonValue)
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved
}

extension HeadersValue: JSONEncodable {
internal var toJSONValue: JSONValue {
switch self {
case let .string(value):
.string(value)
case let .number(value):
.number(Double(value))
case let .bool(value):
.bool(value)
case .null:
.null
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved
}

// The corresponding type in TypeScript is
// Record<string, number | string | boolean | null | undefined>
// There may be a better way to represent it in Swift; this will do for now. Have omitted `undefined` because I don’t know how that would occur.
Expand Down
9 changes: 9 additions & 0 deletions Sources/AblyChat/JSONCodable.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
internal protocol JSONEncodable {
var toJSONValue: JSONValue { get }
}

internal protocol JSONDecodable {
init(jsonValue: JSONValue) throws
}

internal typealias JSONCodable = JSONDecodable & JSONEncodable
maratal marked this conversation as resolved.
Show resolved Hide resolved
29 changes: 21 additions & 8 deletions Sources/AblyChat/JSONValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,19 @@ extension JSONValue: ExpressibleByBooleanLiteral {
// MARK: - Bridging with ably-cocoa

internal extension JSONValue {
init(ablyCocoaPresenceData: Any) {
switch ablyCocoaPresenceData {
/// Creates a `JSONValue` from an ably-cocoa deserialized JSON object.
///
/// Specifically, `ablyCocoaData` can be:
///
/// - a non-`nil` value of `ARTPresenceMessage`’s `data` property
/// - a non-`nil` value of `ARTMessage`’s `data` property
/// - the return value of the `toJSON()` method of a non-`nil` value of `ARTMessage`’s `extras` property
init(ablyCocoaData: Any) {
switch ablyCocoaData {
case let dictionary as [String: Any]:
self = .object(dictionary.mapValues { .init(ablyCocoaPresenceData: $0) })
self = .object(dictionary.mapValues { .init(ablyCocoaData: $0) })
case let array as [Any]:
self = .array(array.map { .init(ablyCocoaPresenceData: $0) })
self = .array(array.map { .init(ablyCocoaData: $0) })
case let string as String:
self = .string(string)
case let number as NSNumber:
Expand All @@ -149,16 +156,22 @@ internal extension JSONValue {
self = .null
default:
// ably-cocoa is not conforming to our assumptions; either its behaviour is wrong or our assumptions are wrong. Either way, bring this loudly to our attention instead of trying to carry on
preconditionFailure("JSONValue(ablyCocoaPresenceData:) was given \(ablyCocoaPresenceData)")
preconditionFailure("JSONValue(ablyCocoaData:) was given \(ablyCocoaData)")
}
}

var toAblyCocoaPresenceData: Any {
/// Creates an ably-cocoa deserialized JSON object from a `JSONValue`.
///
/// Specifically, the value of this property can be used as:
///
/// - `ARTPresenceMessage`’s `data` property
/// - the `data` argument that’s passed to `ARTRealtime`’s `request(…)` method
var toAblyCocoaData: Any {
switch self {
case let .object(underlying):
underlying.mapValues(\.toAblyCocoaPresenceData)
underlying.mapValues(\.toAblyCocoaData)
case let .array(underlying):
underlying.map(\.toAblyCocoaPresenceData)
underlying.map(\.toAblyCocoaData)
case let .string(underlying):
underlying
case let .number(underlying):
Expand Down
40 changes: 38 additions & 2 deletions Sources/AblyChat/Metadata.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,45 @@

public enum MetadataValue: Sendable, Codable, Equatable {
case string(String)
case number(Int) // Changed from NSNumber to Int to conform to Codable. Address in linked issue above.
case number(Double)
case bool(Bool)
case null
}

public typealias Metadata = [String: MetadataValue?]
public typealias Metadata = [String: MetadataValue]

extension MetadataValue: JSONDecodable {
internal enum JSONDecodingError: Error {
case unsupportedJSONValue(JSONValue)
}

internal init(jsonValue: JSONValue) throws {
self = switch jsonValue {
case let .string(value):
.string(value)
case let .number(value):
.number(value)
case let .bool(value):
.bool(value)
case .null:
.null
default:
throw JSONDecodingError.unsupportedJSONValue(jsonValue)
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved
}

extension MetadataValue: JSONEncodable {
internal var toJSONValue: JSONValue {
switch self {
case let .string(value):
.string(value)
case let .number(value):
.number(Double(value))
case let .bool(value):
.bool(value)
case .null:
.null
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved
}
14 changes: 7 additions & 7 deletions Sources/AblyChat/PresenceDataDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,32 @@ internal struct PresenceDataDTO: Equatable {
internal var userCustomData: PresenceData?
}

// MARK: - Conversion to and from JSONValue
// MARK: - JSONCodable

internal extension PresenceDataDTO {
enum JSONKey: String {
extension PresenceDataDTO: JSONCodable {
internal enum JSONKey: String {
case userCustomData
}

enum DecodingError: Error {
internal enum DecodingError: Error {
case valueHasWrongType(key: JSONKey)
}

init(jsonValue: JSONValue) throws {
internal init(jsonValue: JSONValue) throws {
guard case let .object(jsonObject) = jsonValue else {
throw DecodingError.valueHasWrongType(key: .userCustomData)
}

userCustomData = jsonObject[JSONKey.userCustomData.rawValue]
}

var toJSONObjectValue: [String: JSONValue] {
internal var toJSONValue: JSONValue {
var result: [String: JSONValue] = [:]

if let userCustomData {
result[JSONKey.userCustomData.rawValue] = userCustomData
}

return result
return .object(result)
}
}
46 changes: 46 additions & 0 deletions Tests/AblyChatTests/ChatAPITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,52 @@ struct ChatAPITests {
#expect(message == expectedMessage)
}

@Test
func sendMessage_includesHeadersInBody() async throws {
// Given
let realtime = MockRealtime.create {
(MockHTTPPaginatedResponse.successSendMessage, nil)
}
let chatAPI = ChatAPI(realtime: realtime)

// When
_ = try await chatAPI.sendMessage(
roomId: "", // arbitrary
params: .init(
text: "", // arbitrary
// The exact value here is arbitrary, just want to check it gets serialized
headers: ["numberKey": .number(10), "stringKey": .string("hello")]
maratal marked this conversation as resolved.
Show resolved Hide resolved
)
)

// Then
let requestBody = try #require(realtime.requestArguments.first?.body as? NSDictionary)
#expect(try #require(requestBody["headers"] as? NSObject) == ["numberKey": 10, "stringKey": "hello"] as NSObject)
}

@Test
func sendMessage_includesMetadataInBody() async throws {
// Given
let realtime = MockRealtime.create {
(MockHTTPPaginatedResponse.successSendMessage, nil)
}
let chatAPI = ChatAPI(realtime: realtime)

// When
_ = try await chatAPI.sendMessage(
roomId: "", // arbitrary
params: .init(
text: "", // arbitrary
// The exact value here is arbitrary, just want to check it gets serialized
metadata: ["numberKey": .number(10), "stringKey": .string("hello")]
)
)

// Then
let requestBody = try #require(realtime.requestArguments.first?.body as? NSDictionary)
#expect(try #require(requestBody["metadata"] as? NSObject) == ["numberKey": 10, "stringKey": "hello"] as NSObject)
}

// MARK: getMessages Tests

// @specOneOf(1/2) CHA-M6
Expand Down
Loading
Loading