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.

Sign up for GitHub

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

Jump to bottom

Encode JSON message data as string. #459

Merged
merged 7 commits into from
Aug 11, 2016
Merged

Encode JSON message data as string. #459

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1626e27
Encode JSON message data as string.
tcard Aug 3, 2016
fff2732
Add messages fixtures test.
tcard Aug 8, 2016
e7f68e1
Rename "map" to "object" in messages encoding fixtures.
tcard Aug 8, 2016
318fcbb
Use expectedValue from messages encoding fixtures.
tcard Aug 8, 2016
3176c62
Update ably-common.
tcard Aug 9, 2016
1ee851c
Ensure interoperability with other libraries over MsgPack.
tcard Aug 10, 2016
235cfdc
Do message encoding tests in both MsgPack and JSON.
tcard Aug 11, 2016
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
198 changes: 149 additions & 49 deletions Spec/RealtimeClientConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2965,49 +2965,10 @@ class RealtimeClientConnection: QuickSpec {
expect(client.connection.errorReason).to(equal(protoMsg.error))
}

// https://github.com/ably/wiki/issues/22
it("should encode and decode fixture messages as expected") {
context("with fixture messages") {
let fixtures = JSON(data: NSData(contentsOfFile: pathForTestResource(testResourcesPath + "messages-encoding.json"))!, options: .MutableContainers)

let options = AblyTests.commonAppSetup()
let client = AblyTests.newRealtime(options)
defer { client.close() }
let channel = client.channels.get("test")
channel.attach()

expect(channel.state).toEventually(equal(ARTRealtimeChannelState.Attached), timeout: testTimeout)
if channel.state != .Attached {
return
}

for (_, fixtureMessage) in fixtures["messages"] {
var receivedMessage: ARTMessage?

waitUntil(timeout: testTimeout) { done in
channel.subscribe { message in
channel.unsubscribe()
receivedMessage = message
done()
}

let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(channel.name)/messages")!)
request.HTTPMethod = "POST"
request.HTTPBody = try! fixtureMessage.rawData()
request.allHTTPHeaderFields = [
"Accept" : "application/json",
"Content-Type" : "application/json"
]
client.rest.executeRequest(request, withAuthOption: .On, completion: { _, _, err in
if let err = err {
fail("\(err)")
}
})
}

guard let message = receivedMessage else {
continue
}

func expectDataToMatch(message: ARTMessage, _ fixtureMessage: JSON) {
switch fixtureMessage["expectedType"].string! {
case "string":
expect(message.data as? NSString).to(equal(fixtureMessage["expectedValue"].string!))
Expand All @@ -3028,26 +2989,165 @@ class RealtimeClientConnection: QuickSpec {
default:
fail("unhandled: \(fixtureMessage["expectedType"].string!)")
}
}

waitUntil(timeout: testTimeout) { done in
channel.publish([message]) { err in
if let err = err {
fail("\(err)")
// https://github.com/ably/wiki/issues/22
it("should encode and decode fixture messages as expected") {
let options = AblyTests.commonAppSetup()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not explicitly use JSON i.e. binaryProtocol = false and should the test description be updated to reflect that these tests are about JSON based interoperability?

Copy link
Contributor Author

@tcard tcard Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't see why. Whether this uses JSON or MsgPack doesn't matter, that's not the layer we're testing. We have two fully independent layers:

  • Message data encoding JSON-like to string and back again.
  • Whole protocol message encoding to JSON or MsgPack and back again.

This test is about the former. When the encoded message data hits the latter, it is already a string in all cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message data encoding JSON-like to string and back again.

Well you are never technically doing that because every time you use JSON on one end MsgPack on the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't see your point here. How am I never doing that? I am pretty literally doing that, concretely here. Then I'm turning that string (doesn't matter if it's JSON or not) into either MsgPack or a JSON blob or any other protocol-level encoding we might use in the future. Then I'm turning that into maybe a gzipped blob over HTTP and then encrypting that with TLS and then Wi-Fi signals or whatever, all those encodings that also don't affect the first one at all. Am I wrong here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point here is that tests don't know how the internals of a library work and should not care. The point of our interoperability tests was to check that a message when decoded/encoded via JSON or MsgPack are consistent with the types we expect to see. We agreed the only reliable way to get data into the Ably system was via a raw REST request to Ably, and then to check that it was as expected, was also via a raw REST history request to Ably and then to parse the JSON (or inspect the type if binary/string) and compare with what we see the library interprets that over JSON and MsgPack. So if we want to test how our Ably library encoded/decodes objects with a JSON transport it should use that, and if we want to see how our Ably library does that MsgPack it should use a MsgPack transport. This test uses MsgPack, so the JSON encoding / decoding of a message injected or received via a raw request is not being done. That is why in Ruby I have a set of tests using the JSON transport and then using the MsgPack transport but all the time using a raw JSON request to inject or check the fixture in Ably irrespective of the transport being tested for interoperability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of our interoperability tests was to check that a message when decoded/encoded via JSON or MsgPack are consistent with the types we expect to see.

That's the point of the second interoperability test. The point of this one is to check it the message data (not the message itself) when decoded/encoded as string, JSON or base64. We need both precisely because they test two completely independent things.

We agreed the only reliable way to get data into the Ably system was via a raw REST request to Ably, and then to check that it was as expected, was also via a raw REST history request to Ably and then to parse the JSON

That part I agree with, and if that's what you meant then OK. But since I'm doing a direct raw POST with a JSON payload (here) it doesn't matter what I set useBinaryProtocol to. Same for the raw GET for history. It still doesn't affect how the message gets transmitted via realtime.

This test uses MsgPack, so the JSON encoding / decoding of a message injected or received via a raw request is not being done.

Yes it is, because what we're encoding/decoding is the JSON inside that message, which comes as a string. These lines that decode the JSON from the message data are being executed both if the message came as JSON or as MsgPack. At that point, it's just an ARTMessage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I am misunderstanding still, but in these tests, at what point do we publish with a raw JSON message (i.e. getting the fixture into Ably in a guaranteed portable way) and get that message over a JSON transport and confirm the message.data is as expected then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, and where do we publish over JSON using the library, and then confirm with a raw HTTP request that the JSON in Ably is as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at what point do we publish with a raw JSON message

Here. That's a POST /channels/.../messages. fixtureMessage is a JSON object (from a library for easy JSON manipulation) and rawData() converts it into a JSON string. That's quite like cutting and pasting from the messages-encoding.json directly into the POST request body.

and get that message over a JSON transport

Here. data there is the HTTP response body, which the JSON library parses.

and confirm the message.data is as expected

Here. Both persistedMessage and fixtureMessage are JSON objects that don't go through the ARTRealtime instance at any point. The ARTRealtime instance is used to test that the iOS library encodes and decodes the data as expected (ie. as expectedValue or expectedHexValue tell us), be it JSON, string or base64.

As above, and where do we publish over JSON using the library, and then confirm with a raw HTTP request that the JSON in Ably is as expected?

That's in the second test, here.

Copy link
Member

@mattheworiordan mattheworiordan Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and get that message over a JSON transport

Here. data there is the HTTP response body, which the JSON library parses.

But that's using client.rest.executeRequest to get that data. So it's not passing through either a Realtime subscribe API or a history API request. The goal was end-to-end API test, yet that is not using an API that handles encoding & decoding implicitly i.e. publish/subscribe/history.

As above, and where do we publish over JSON using the library, and then confirm with a raw HTTP request that the JSON in Ably is as expected?

That's in the second test, here.

Against, I don't think that is doing the job. You are mixing two concerns into one. We explicitly don't couple publishing & subscribing (regardless of protocol) into a single interoperability test because then you are simply testing that the client library when encoding & then decoding gives you the answer you expect. But that's not the point of interoperability tests.

let client = AblyTests.newRealtime(options)
defer { client.close() }
let channel = client.channels.get("test")
channel.attach()

expect(channel.state).toEventually(equal(ARTRealtimeChannelState.Attached), timeout: testTimeout)
if channel.state != .Attached {
return
}

for (_, fixtureMessage) in fixtures["messages"] {
var receivedMessage: ARTMessage?

waitUntil(timeout: testTimeout) { done in
channel.subscribe { message in
channel.unsubscribe()
receivedMessage = message
done()
}

let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(channel.name)/messages")!)
request.HTTPMethod = "POST"
request.HTTPBody = try! fixtureMessage.rawData()
request.allHTTPHeaderFields = [
"Accept" : "application/json",
"Content-Type" : "application/json"
]
client.rest.executeRequest(request, withAuthOption: .On, completion: { _, _, err in
if let err = err {
fail("\(err)")
}
})
}

guard let message = receivedMessage else {
continue
}

expectDataToMatch(message, fixtureMessage)

waitUntil(timeout: testTimeout) { done in
channel.publish([message]) { err in
if let err = err {
fail("\(err)")
done()
return
}

let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(channel.name)/messages?limit=1")!)
request.HTTPMethod = "GET"
request.allHTTPHeaderFields = ["Accept" : "application/json"]
client.rest.executeRequest(request, withAuthOption: .On, completion: { _, data, err in
if let err = err {
fail("\(err)")
done()
return
}
let persistedMessage = JSON(data: data!).array!.first!
expect(persistedMessage["data"]).to(equal(fixtureMessage["data"]))
expect(persistedMessage["encoding"]).to(equal(fixtureMessage["encoding"]))
done()
})
}
}
}
}

let jsonOptions = AblyTests.commonAppSetup()
jsonOptions.useBinaryProtocol = false
let msgpackOptions = AblyTests.commonAppSetup()
msgpackOptions.useBinaryProtocol = true

it("should send messages through JSON and retrieve equal messages through MsgPack") {
let restPublishClient = ARTRest(options: jsonOptions)
let realtimeSubscribeClient = AblyTests.newRealtime(msgpackOptions)
defer { realtimeSubscribeClient.close() }

let realtimeSubscribeChannel = realtimeSubscribeClient.channels.get("test-subscribe")
realtimeSubscribeChannel.attach()

expect(realtimeSubscribeChannel.state).toEventually(equal(ARTRealtimeChannelState.Attached), timeout: testTimeout)
if realtimeSubscribeChannel.state != .Attached {
return
}

for (_, fixtureMessage) in fixtures["messages"] {
var receivedMessage: ARTMessage?

waitUntil(timeout: testTimeout) { done in
realtimeSubscribeChannel.subscribe { message in
realtimeSubscribeChannel.unsubscribe()
receivedMessage = message
done()
}

let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(realtimeSubscribeChannel.name)/messages")!)
request.HTTPMethod = "POST"
request.HTTPBody = try! fixtureMessage.rawData()
request.allHTTPHeaderFields = [
"Accept" : "application/json",
"Content-Type" : "application/json"
]
restPublishClient.executeRequest(request, withAuthOption: .On, completion: { _, _, err in
if let err = err {
fail("\(err)")
}
})
}

guard let message = receivedMessage else {
continue
}

expectDataToMatch(message, fixtureMessage)
}
}

it("should send messages through MsgPack and retrieve equal messages through JSON") {
let restPublishClient = ARTRest(options: msgpackOptions)
let restRetrieveClient = ARTRest(options: jsonOptions)

let restPublishChannel = restPublishClient.channels.get("test-publish")

for (_, fixtureMessage) in fixtures["messages"] {
var data: AnyObject
if fixtureMessage["expectedType"] == "binary" {
data = fixtureMessage["expectedHexValue"].string!.dataFromHexadecimalString()!
} else {
data = fixtureMessage["expectedValue"].object
}

waitUntil(timeout: testTimeout) { done in
restPublishChannel.publish("event", data: data) { err in
if let err = err {
fail("\(err)")
done()
return
}
done()
return
}
}

let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(channel.name)/messages?limit=1")!)
waitUntil(timeout: testTimeout) { done in
let request = NSMutableURLRequest(URL: NSURL(string: "/channels/\(restPublishChannel.name)/messages?limit=1")!)
request.HTTPMethod = "GET"
request.allHTTPHeaderFields = ["Accept" : "application/json"]
client.rest.executeRequest(request, withAuthOption: .On, completion: { _, data, err in
restRetrieveClient.executeRequest(request, withAuthOption: .On, completion: { _, data, err in
if let err = err {
fail("\(err)")
done()
return
}
let persistedMessage = JSON(data: data!).array!.first!
expect(persistedMessage["data"]).to(equal(fixtureMessage["data"]))
expect(persistedMessage["data"]).to(equal(persistedMessage["data"]))
expect(persistedMessage["encoding"]).to(equal(fixtureMessage["encoding"]))
done()
})
Expand Down