HTTP recorder

[dmcrodrigues]

Vinyl should provide a mechanism to automatically record all http traffic (request+response) for posterior replay.

[RuiAAPeres]

The "automatically" is something that bothers me a bit, should we make this explicit? Should we simply record if there are no tracks?

[dmcrodrigues]

I think this should be configurable. I was thinking something like this:

Record modes

• None
• New tracks (track not available on vinyl)
• All

[RuiAAPeres]

I think they would sit well here then: TurntableConfiguration.

[dmcrodrigues]

Maybe instead of All we should have a Once mode to simply create a vinyl and then use it without any new recording, that way you can reason about the requests expected to be handled.

[dmcrodrigues]

I agree, TurntableConfiguration seems the right place to configure this.

[RuiAAPeres]

I didn't understand the reasoning behind the Once. So let's look at the cases:

enum RecordingMode {
case None
case NewTracks
case All/Once

• .None: If a track is not found, it will raise a fatalError (current behaviour)
• .NewTracks: For every track not found, a request will be made and stored
• .All/Once: For every track, a request will be made and stored

Is this what you have in mind?

[dmcrodrigues]

I don't know if we really need/want a .All.

• .Once: For every vinyl (file) not found, record and store every request performed to create that vinyl.

Makes any sense?

[RuiAAPeres]

Makes sense, I am just not sold with the naming. 😁

[hugocf]

Suggestion❓

enum RecordingMode {
case None
case NewTracks
case NewVinyl
}

And btw, looks like the plural of “vinyl” is “vinyl” 😄

[RuiAAPeres]

With your suggestion, you actually raised an interesting question which is: on a test with multiple requests, how should those requests be treated? As individual vinyl with a single track, or one vinyl with multiple tracks? I am 👍 for the later.

[dmcrodrigues]

I think the 2nd approach is a better one otherwise we may need to support having multiple vinyl loaded (thank you for clarifying this @hugocf 😃), 1 per request.

[RuiAAPeres]

Initial draft

Let's start with the new entities that will be added:

• Network: A class that serves as a gateway for making the requests. This takes us to two possible scenarios:
  • Use a protocol to abstract the implementation: this will help us testing it via mocked objects. (I find this approach simpler)
  • Initialize this class with a NSURLSession: this will help us testing it via a Turntable. So we are essentially testing our lib with itself. 🆒
• Persistence: A class that will allow us to store the Network response on disk. In this particular case, we could test it via its outcome. So we would check if the file was stored in disk.
• Recorder: This what will orchestrate the work between the Network and the Persistence. It will be used by the Turntable.

The second part is the enum:

enum RecordingMode {
  case None
  case MissingTracks
  case Vinyl
}

• None: As expected, if a track is not found in the vinyl it will raise a fatalError (what we currently have)
• MissingTracks: It will only download the missing tracks from a Vinyl
• Vinyl: It will download an entire Vinyl

I think MissingTracks jells better, if you read the whole thing: "Recording Missing Tracks", rather than "Recording New Tracks".

We would probably go with MissingTracks as the default configuration.

The final higher level item would be when to signal the recording to stop. DVR does this explicitly with the session.beginRecording() and session.endRecording(). VCR does it automatically. We can keep it simple for now (manual signalling) and do something more advance later.

@dmcrodrigues let me know what you think

[dmcrodrigues]

I really like the option of use a NSURLSession that can be tested via a Turntable, like you said, test our own lib is like pure gold ✨

I think enum is perfect! 👍

About the default configuration I'm hesitating between MissingTracks and Vinyl, this last one is more strict which I think may be a good option.

[RuiAAPeres]

between MissingTracks and Vinyl, this last one is more strict which I think may be a good option.

If we go with the Vinyl option, you will be recording a new Vinyl every time, which basically means that if you don't internet on your server, the tests will fail. Not only that, we never use our recorded tracks.

Let's look at the flow with Vinyl (from my POV):

• A new request is made and hits the Turntable.
• We check the recording mode.
  • It's Vinyl recording mode, so we will make a new request and return a real NSURLSessionTask.
  • As a side effect, we will store the NSURLSessionTask's eventual response on disk and warn the user, to add it to its project later
• At no point we are using our recorded tracks.

[dmcrodrigues]

I was assuming that Vinyl mode does in fact an entire download but only if that vinyl doesn't exist in the bundle. Basically, in the first execution the tests create the vinyl to be stored and used in the follow executions.

I think you have described an All mode which can be something that we can consider but I think we need a mechanism where you record something to be strictly used in the following executions.

@RuiAAPeres what do you think?

[RuiAAPeres]

Ah good call! I contemplated the idea of:

• We have the Vinyl, but we are missing a track. So we have .MissingTracks.

But I forgot about:

• We are missing the Vinyl. (the file)

So maybe:

enum RecordingMode {
  case None
  case MissingTracks
  case MissingVinyl
}

With .MissingVinyl if there is a missing vinyl, it will try to download it, if the Vinyl is present but not the track, it will crash. This is the same behaviour that DVR has by default.

[dmcrodrigues]

👍

[RuiAAPeres]

I haven't had much time to start working on this, so I will remove myself from it, so anyone can tackle this. (if I have time, I will work on it tho) .

[dmcrodrigues]

I'll start working on this 🚴🏼

[mluisbrown]

Since @dmcrodrigues hasn't had time to work on this, I've decided to take it on 🙈