renderdata() enhancement

[jochemd]

In 3.1 devel a new renderData type "rawjson" is supported. I can think of at least 3 more types I would want:

• binary for images, PDFs etc.
• X-Sendfile for https://tn123.org/mod_xsendfile/
• X-Accell for http://wiki.nginx.org/X-accel

Compared to the existing renderData they all have the problem they need more arguments since you need to pass in a mime-type as well. So might it be an idea to somehow make this extensible?

Discussion at https://groups.google.com/forum/#!topic/framework-one/aY5Ct68IbdE

[seancorfield]

The current assumption is that "rendering" produces something that can be passed to writeOutput() to be written into the output stream. I'm not sure how these would sit with that machinery.

One way I can think of is to allow for renderData()s type argument to be a function, that takes the data and returns a struct with contentType, output, and optional writer fields. The logic of renderDataWithContentType() would change so that if type was a UDF/closure, it would call the type function on the data, set the content type based on the result and return the whole struct instead of just out (the normally transformed data).

onRequest() would then change to check whether out was a string or a struct and, if a string, continue to call writeOutput() on it, else it would assume the struct contained output and, if writer exists then call writer on the output, else call writeOutput on it.

This would also have the side effect of allowing an overridden onMissingView() function to return a struct containing output and writer so it could do something other than just outputting an HTML string. Hmm, that would speak toward setting the content type in onRequest() to allow onMissingView() to override the content type easily.

Thoughts? Is that too "clever"?

[jochemd]

If users have to write a closure, then what is the benefit over just uising cfcontent + cfabort directly? Also (and this is a somewhat new consideration), a user would either use X-Sendfile if he runs Apache, or X-Accell if he uses Nginx or cfcontent if neither is supported. The choice between them should be driven from the FW/1 config and not be put in the closure code.

So I am more thinking along the lines of just adding a third argument to renderData for the MIME type and then putting logic in to check the FW/1 (environment) configuration whether X-Sendfile or X-Accell are enabled. If one of them is, use it, else fall back to cfcontent.
What I see as an open issue with this design is that I would like to make it possible to supply either a path or a bytearray as the resultData parameter and let renderData figure out the rest. But other than doing an isString() plus a fileExists() I don't see a way to figure out whether the resultData is a path or a bytearray, and I don't like the performance overhead of doing so. But even without solving that adding both types "binary" for a bytearray and "file" for a path seems like a better solution to me than using a closure.

[seancorfield]

The reason I suggested the UDF/closure is because nothing in FW/1 uses cfcontent and there are no tags in the code so I'm not sure there's a portable way to deal with that in script -- plus everyone pays for more conditional logic which I'm trying to keep to a minimum by isolating behavior into user-supplied code.

Adding additional arguments that are conditionally used (what would type be in that case?) and ad hoc configuration and conditional logic sprinkled across the framework doesn't sound like very good design to me when what's wanted here is to abstract behavior.

If there are certain, standard, file-processing behaviors, then FW/1 could provide UDFs that encapsulated those, for use as type arguments.

I'd be interested to see a PR with the code changes for what you are proposing so we'd have something concrete to review (but right now I'm not 100% sure exactly what you are proposing, especially around the send file / accell config).

[aliaspooryorik]

I was discussing the idea of allowing the JSON serialisation to be switched out (as SerializeJSON has several known issues on ACF!). Adding a comment here as it's loosely related.

My thinking was to do a simple public SerializeAsJSON function which could be overridden by the developer if required. Something like this commit:
aliaspooryorik@cf67649

Looking through the code for 4.0 I can see that there is a move towards using a builder where you could pass in a function to handle this instead.

[seancorfield]

renderData()s type can now be a UDF/closure that must return a struct containing contentType and output and an optional writer function per my notes above. I have not yet updated onMissingView() to support a struct return.

[seancorfield]

onMissingView() can now return a struct with contentType, output, and an optional writer, and can affect the content type. Just documentation to update now.