Option to externalize response bodies #59

Closed
robfletcher opened this Issue Aug 7, 2012 · 2 comments

Projects

None yet

1 participant

@robfletcher
Collaborator

To make tidying of response bodies easier it would make sense to offer the option to externalize them from the tape file. For example the tape file itself would contain a body field with the file path of the data file with the body. The data file could then contain formatted data & use the correct file extension so it's easier to manually modify.

Collaborator

This may well help with memory problems as removing large response bodies from the YAML should make it significantly smaller to the point where it really shouldn't be a problem to read tape file into memory.

A couple of acceptance criteria for this:

  • Betamax should avoid ever copying the response body into memory. It should be possible to stream direct from the file to the HTTP response when playing back and direct from the upstream response to the file when recording.
  • The file should have an appropriate extension so it can be opened and edited easily. That means we'll need a mime type -> extension mapping and for everyone's sanity that should be extensible for people using less common content types. I'm thinking a resource based solution is the way to go.
Collaborator

I think to best implement this a reworking of some internals is required. This is an idea that's been in the back of my mind for some time but this requirement clarifies it a bit.

Currently implementations use a adapter pattern to make Netty / HttpClient request and response objects implement a common interface. A third implementation is the one used for storage on tape. This means there is a lot of copying going on, i.e. when playing back the stored response is copied to a real Netty / HttpClient response, when recording the wrapped Netty / HttpClient response is copied to a storable version.

What I'd like to have is a more layered approach. When recording the adapter class should be responsible for populating the recorded response. That would allow different strategies for response body storage and efficient writing of HTTP response content directly out to file rather than via an in-memory byte array as currently. It would be easy to support chunked responses by just appending to the file.

The current Request and Response interfaces are implemented by Netty and HttpClient adapters. The purpose of those interfaces needs to be clarified. Crucially Request is essentially read-only while Response is read-write. I don't think it makes sense for the tape representations (current RecordedRequest and RecordedResponse) to implement those interfaces – they're for the adapters to external models. It might make sense to separate a read model of response from a write model. The former is used to read a "real" response to create a recording and the latter for playing back an existing recording. Mixing those two functions up muddies the responsibilities of the adapter classes.

The play and record methods on Tape should be responsible for writing Betamax specific headers to the "real" response (at least the X-Betamax header, maybe not Via as that needs to go on requests as well which doesn't feel like the tape's responsibility). It should also handle content encoding and charsets which would remove some existing duplication from the code and ensure consistency across implementations. It's also a better separation of concerns – the response adapters shouldn't care whether the tape wants encoded or decoded content, that's up to the tape to handle.

@robfletcher robfletcher was assigned Nov 3, 2013
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 tinkering 9e4c885
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 dumb proof of concept implementation 988ba4e
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 write body to correct root directory 3edbb60
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 writes the body out to tmpdir rather than tape root fd06e00
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 refactor loader<->tape relationship so tape doesn't need unreason…
…able amount of context
bdc73ca
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 almost working
now writes to correct directory. Need to make filenames unique (indexed?)
f4e1147
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 refactored so much out of StorableTape there's no need for it any…
… more
527f7a6
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 extract MIME->file extension code out to utility class 5389df7
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 clean up some unnecessary code c4fe703
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 write file path to YAML relative to tape root 9856527
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 read files in as well 491af93
@robfletcher robfletcher pushed a commit that referenced this issue Nov 5, 2013
Rob Fletcher #59 failing test for remaining functionality
need a good strategy for consistently naming the body files
2323eae
@robfletcher robfletcher pushed a commit that referenced this issue Nov 8, 2013
Rob Fletcher #59 place response body files in a sub-directory 04c1c36
@robfletcher robfletcher pushed a commit that closed this issue Nov 9, 2013
Rob Fletcher Use interaction index in body file name
Fixes #59
1327cac
@robfletcher robfletcher pushed a commit that referenced this issue Nov 9, 2013
Rob Fletcher #59 zero pad filename index b2cfa30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment