venmo / DVR

Network testing for Swift
http://blog.soff.es/network-testing-in-swift-with-dvr
MIT License
651 stars 86 forks source link

Plain text serialization for JSON #5

Closed czechboy0 closed 9 years ago

czechboy0 commented 9 years ago

Added different serialization to different data types. JSON is now serialized in plain text, so that it's more readable. Other types fall back to base64 as before. Includes tests.

soffes commented 9 years ago

Amazing. I've been thinking about this all week. Can you add a plain text case? So any context type with text/will appear as only plain text. We don't need a body format. We can simply use the content type :wink:

czechboy0 commented 9 years ago

Hmm but if we don't have the body format, how do you know how to deserialize it from JSON? Both base64 string and normal string will be loaded as String, but when converting back to NSData, we need to know whether it's base64 encoded or not.

Let me try adding a text case first and see if that's what you meant.

czechboy0 commented 9 years ago

Ok, I added a plain_text format as well and now we rely on the returned Content-Type of the response. So now e.g. HTML will also be shown in plain text in the Cassette on disk. I still don't know how to get around saving the body_format though, so if you can think of one, let me know.

czechboy0 commented 9 years ago

Oh of course, I didnt realize we also have all the headers saved to disk. Do you want me to remove the body_format then?

soffes commented 9 years ago

Ya let's remove it. I have an idea to not regenerate all of the cassettes. Thanks for you work on this so far! Going to chat with some other folks here today about this.

czechboy0 commented 9 years ago

Aight, I'll fix it in the next couple of days. Thanks for creating it, it's super useful for API tests. I wanted to kill myself when I thought about the hoops I'd have to jump through to do old-school object mocking etc.

soffes commented 9 years ago

:heart: by the way, some of your work inspired something we're working on

czechboy0 commented 9 years ago

Oh nice! I'll link to it from XcodeServerSDK in case someone's looking for a Ruby version.

But... Good luck. The API is still a beast, especially since Apple still hasn't released any documentation. I spent the last half a year reverse engineering it and working with it, so I have an idea of how to make it work. But there's still enough magic involved to make it a PITA to work with.

czechboy0 commented 9 years ago

@soffes I was going to change this to just use Content-Type, but I think there is still value to saving the format of the response body in a separate key. Let's assume we build logic that purely from Content-Type infers a method of serialization. Let's say that we now save image data as Base64 encoded-string, which is fine. But what if we decide that suddenly we want to do more, save it as an ASCII-art image (a ridiculous example, but you get the gist). Then, since we change that logic in code, all the old Cassettes will have to be regenerated, otherwise they'll try to parse the images as ASCII-art, even though they'll still be encoded in Base64, like we used to do.

My point is, having the body and next to it also its format of encoding is a future-proof approach. For now, if the key is not there, we'll assume base64 (which is correct). From now on, even if we change our mind of how we encode certain Content-Types, we will always correctly load old Cassettes (in the previous example, even if we decide to now encode images as ASCII-art, the old images still will have their format as base64, thus will be parsed correctly. Just the new images will be saved as ASCII-art).

On the other hand, if we just rely on Content-Type, we will most likely need to ask people to regenerate their Cassettes whenever we add a nice new serialization option. So I'd suggest to keep it this way for the sake of our future bright ideas.

(Just to be clear, I don't have a problem with changing it to rely on Content-Type, in fact I started doing that, but in the process realized the issue with future upgrades to our serialization logic and how it will always break old Cassettes).

Just my two cents, let me know what you want to do with this.

czechboy0 commented 9 years ago

Just FYI, I also added the same serialization rules to requests, not just responses.

czechboy0 commented 9 years ago

Hello?

soffes commented 9 years ago

Been busy. I think about this every day though :) I'll try to take a look tomorrow

czechboy0 commented 9 years ago

No problem, I know how it works :+1:

soffes commented 9 years ago

Implemented in #15