Loading request/response from external file is broken

[lukasgraf]

With the change from #38 the loading requests/responses from filesystem files got broken.

Steps to reproduce

• Include a request from the filesystem using the :request: option => rendered request content is completely empty

---

Specifically it's 05035394 that breaks it: Before that change, the HTTPExample directive processed it's data like this:

• Try to split the literal content of the directive into request and response, by using newlines and HTTP/ for splitting
• Store the results of that split in request_content and response_content respectively
• If the directive contains either a :request: or :response: option, treat it as a filename, read the file's contents from disk, and store them in the respective variable

After that, self.content was set to request_content (wherever it came from, file on disk or literal directive content):

        # reset the content to just the request
        self.content = request_content

With the change from 05035394, the stripping of fields got mangled into the logic for splitting request and response. The loading from disk parts were still left in place, but completely negated by ignoring request_contents, and always setting self.content to request_content_no_fields:

        # reset the content to the request, stripped of the reST fields
        self.content = request_content_no_fields

---

Unfortunately this bug slipped through because there's no test that really exercises loading from files (or makes any assertion on the result).

The fix in my opinion would be to separate loading of request/response (from disk, or from directive content + split) from any post-processing (like stripping fields) that's being done to it.

[datakurre]

@lukasgraf I’m sorry about this. Thanks for such a detailed report. Obviously the previous release should work as long as this remains unfixed.

[lukasgraf]

@datakurre no worries.

I tried to write a failing test for the filesystem loading, and create a PR, but honestly I didn't quite see how I could easily test this. The fixtures + snapshots work great for testing the builders (that get passed in an HTTP request), but because the loading from FS happens in the directive, I didn't quite know where to start.

When trying to carefully refactor the code introduced with #38, I noticed that I could comment out this block without any tests failing. So I couldn't even be sure my fix wouldn't break that feature, that's why I just stuck with creating an issue for now 😉

[datakurre]

Should be fixed in 0.10.3

[lukasgraf]

@datakurre thank you! Just tested with the plone.restapi docs, it is indeed fixed for our use case in 0.10.3 👍