31 Aug
2017
31 Aug
'17
8:24 p.m.
On Thu, Aug 31, 2017 at 8:39 PM, Erik Huelsmann <ehuels(a)gmail.com> wrote:
Quick: does the csv and OOO adaptors return bodies with the same
interface? Or do I have to find out what kind of reference the body is and
handle it? And why does the template library have special knowledge of
HTTP status?
Yep. And the UI portion will probably eventually go away.
I guess that's the question, isn't it?
Here's what I would like to see (and I think this goes well with the
efforts to move to a more service-oriented front-end): I would like to see
the template infrastructure be a report and document generator which
abstracts generation of documents in a variety of formats for the
programmer. That's one responsibility. It is a narrow one. We can build
something that then works for the future by ignoring what happens after we
generate the document.
Put another way, what I see the module doing is providing an abstraction
around "I want a report" and "btw, I want it in pdf format" or "I
want it
as a csv." Or "I want an invoice" and "Please make it a PDF."
Now to do this it has to do two things:
1. Maintain knowledge of available formats on the system (and possibly be
able to do so on a per-template basis, something we are currently missing),
and
2. It has to provide a stable interface for format providers to use to
communicate with it and to the program as a whole.
This means that what we have now for Template.pm in my proposal is
basically a point of dependency injection, which maintains the
responsibility of being aware of what dependencies have used it (PDF, HTML,
etc). The Template::LaTeX, Template::HTML, etc. then take on the
responsibility of plugging in to allow document generation in a singularly
consistent API.
That sounds small and narrow and it is. But think about what that enables
as well:
1. Dynamic format detection (What formats do I know about? PDF, HTML, PS,
OOO, and Gnumeric*)
2. Dynamic template detection (What formats can give me an invoice with my
current template list?)
* hypothetical new format we just installed
Note we have to provide a shim for old code. But I think the stabilisation
and generalisation here would be helpful.
More tangibly, it would allow someone to write an HTML -> PDF template
engine third party, and just have it plugged in if that was still desired
by anyone, or for someone who wants to be able to generate OOO doc invoices
to be able to do so without demanding that everyone else support the
functionality.
At any rate that's what I had in mind.
Best Wishes,
Chris Travers
On Thu, Aug 31, 2017 at 7:40 AM, Chris Travers <chris.travers(a)gmail.com>
wrote:
Well, Plack already decides about a lot of headers for us:
Transfer-Encoding (in case of 'chunked') and 'Content-Length' are never
set
by the template nor by the the report "render_to_psgi" methods. They're
added by Plack.
That's one too many options for the body in my view. It means unneeded
complexity given that different formats will probably return differently
here and you always have to handle both. I would like to see, ideally, one
response format only. A file handle would be good if we standardised on
that.
Hi. First, yes it is good to focus on
definitions and consistent
vocabulary. Explanation to follow, along with and will modify RFC to match.
On Wed, Aug 30, 2017 at 8:57 PM, Erik Huelsmann <ehuels(a)gmail.com> wrote:
Ok. You're proposing the return value being a pair: the expanded template
and the mime-type. The current implementation is a list of headers (usually
just the mime-type), the expanded template and a success/fail indicator (a
triplet).
If we want to go with just the pair, how are we going to report the
success/fail indicator? (In other words, how do we report error
conditions?) I'll move the handling of the cache control headers currently
in "render_to_psgi()" to LedgerSMB::PSGI. That's a positive outcome of
this
discussion for sure.
Elsewhere in the issue you refer to "the main wrapper script". Could you
On Wed, Aug 30, 2017 at 8:42 AM, Chris Travers <chris.travers(a)gmail.com>
wrote:
Fair enough. Let's go with these terms.
I have set up an issue with a more concrete
proposal for better
separation of concerns in the templates.
https://github.com/ledgersmb/LedgerSMB/issues/3108
Thanks for the write-up. To be 100% sure we understand each other, let
me explain my vocabulary before proceeding:
* Template: the actual file holding the structure of the output and the
instructions for the template processor (i.e. these would be our files in
UI/*.html)
* Template processor: The software transforming the template to the
intended output (this would be Template Toolkit)
* LedgerSMB Template Format (or 'format' in short): the plugins into the
template processor wrapper which implement format specific operations like
format encoding
* LedgerSMB Template Engine (LedgerSMB::Template): the module which
wraps the template processor; the glue layer between the LedgerSMB
application and the Template Toolkit
In the referenced issue, you write
The template should be responsible *only* to render the content and
provide a file mime type back with it. This means that the output of
template->render() should ...
Assuming that "template" there corresponds to my definition of
"LedgerSMB Template Engine" (concluded from the fragment of the second
sentence), I really wonder what value the template engine is providing: our
template processor has a render method too; the *only* thing it lacks is
the content type return value. The content type can be easily added based
on the extension of the processed template in a generic handler elsewhere
though.
Right.
In the same paragraph, you continue
The Template MUST NOT assume HTTP, PSGI, or any other particular
input/output format.
Could you have a look and https://github.com/ledgersmb/L
edgerSMB/blob/master/lib/LedgerSMB/Template.pm#L540 ? I think that that
achieves what you want.
Yes, though more specifically, I think if we *do not use* PSGI or output
options we achieve something much better. What I want to achieve is a
clearer separation of responsibility and I think this has cascading
benefits through the rest of the application as well as making the logic
more general.
be a bit
more specific about that? Is that a wrapper to be created around
LedgerSMB::Template, is that LedgerSMB::Template itself, is that one of the
modules in LedgerSMB::Scripts or is that one of LedgerSMB::PSGI or
tools/starman.psgi ?
Yes, the LedgerSMB::PSGI or whatever replaces it in the future. In other
words the wrapper should be the one to decide what headers to send (and
should have all PSGI-related functionality).
I believe this would allow us to isolate PSGI and
CGI-related code in one
But looking at it, the PSGI.pm doesn't decide what headers to send does
it? It gets this from the workflow script which gets it from the template
engine, is that correct?
Only for cache control (which I'll move to LedgerSMB::PSGI due to this
review/discussion -- a good improvement); that leaves the content type and
the content-disposition headers. the latter are derived from the
content-type; so, the disposition can be moved to LedgerSMB::PSGI too. That
just leaves the mime type header (content-type). Which is the level we want
to end up with.
or two
places only (request parsing and the request wrappers) for new code
and break them entirely out of the template's responsibility.
We currently have what I call "PSGI or CGI related code" in
LedgerSMB::PSGI (a lot), in old/lib/LedgerSMB/old_code.pm (a wrapper
completely dedicated to the cgi/psgi difference and isolating running of
old code in a forked interpreter) and a little bit in lib/LedgerSMB.pm and
lib/LedgerSMB/Form.pm.
Other than that, the CGI assumption is
omni-present in old/bin/*.pl, as
Ok. What is the problem with the render_to_psgi() functions when you throw
away the success/fail indicator and convert that to a 'die()' when the
value isn't equal to HTTP_OK ? I mean, that amounts to the same thing as
the pair you're proposing? The body is one of two things: an array of
strings or a filehandle. That should be easy to convert to a string (as you
want it?).
you know. I don't think you're talking
about replacing that (other than
replacing the entire code in that directory, which is why it's in the old/
directory).
Can you be a bit more specific about what your proposal be changing to
all that?
I would like to work on getting rid of the render_to_psgi and the output
functions from the template engine, and return the file type with the
contents. I think to make this work, we would need to provide a
compatibility class for old code.
Here's a case in point: Suppose I want a
cron job which emails a
spreadsheet of overdue invoices (ageing detail report). I think this
should be a fairly easy thing to do even with the global state
requirements, but on 1.4 it goes something like "fork our reporting engine"
and the problem still is not fully fixed in master (worse, it is not
*obvious* that the problem is not fully fixed in master because of
assumptions in the templating engine).
I'm afraid I'm missing the point. In my mind you'd need to instantiate a
LedgerSMB::Report::Aging, call its "render_to_psgi()" method and take the
body out of the returned triple. Then you do with that body whatever you
want.
I'm not sure I see what the problem is that isn't completely fixed in
master.
(Note that I'm avoiding to say that we have
resolved the problem that
Yes. We completely agree there.
WAIT! While going over the response above, I'm suddenly realising your
use-case is with the aging report!
Then it's my time now to say that you're confusing one problem for
another. Our LedgerSMB::Template is conflating two completely unrelated
(although both "template" related) problems: LedgerSMB::Template generates
our UI *and* LedgerSMB::Template generates our templatized documents.
none of our modules behave as "good
libraries" and make lots of assumptions
about the context in which they're being called, like accessing and/or
assuming global state having been set up.)
Agreed. One of the point of trying to formalise things that can be
broken off is to make good libraries and a smaller footprint. But this is
about two things. The first is general interface and having an interface
which avoids assuming that every component is being accessed from the whole
framework.
While doing the latter, it even mistakenly sends them
as "UI responses"
(HTTP responses).
I've long wanted to separate the "UI templates processor" from the
"document templates processor". Having that separation resolves 99% of the
cases where people may want to run reports from outside the context of a
web request.
Exactly what I am trying to get to.
I'm not convinced that separating "render_to_psgi()" out from the "UI
templates processor" actually brings us anything: Dancer's
"template()"
function is very tightly nit to the response being sent too.
Right but Dancer's template engine assumes you are going to use it with
Dancer, to generate web pages with HTML and jquery. I guess I am
questioning that assumption for an accounting package.
I think these two are closely connected and global state is a part of
Right. So suggesting we a shim for old code and a clean separation of
concerns.
the second.
The case that again immediately occurred to me
here was the fact that
LedgerSMB::Report->render sends the result in CGI format, despite the fact
that this is a module introduced in 1.4.
We didn't "do" PSGI-native-responses until 1.6, so, both 1.4 and 1.5
return CGI responses which are being converted to PSGI responses on the
request-dispatch boundary.
And even in master, you have a choice of PSGI
or CGI only (because for
historical reasons we output to cgi by default). Yes, we can fix it in the
reporting engine by adding a line of code but I guess I see this as a
single responsibility problem because the templating engine takes on the
responsibility of output which I think is a problem.
The problem is that we have a single templating engine to generate our UI
output as well as our "documents" (invoices, reports,...) output. (And
because they were conflated, we stored the report templates in the UI/
tree, instead of in the templates/ tree (!), with the huge downside that
they're not translatable or per-locale modifiable.)
(But if it doesn't take on the responsibility
to output then I don't
think it should take on PSGI responsibility either.)
There is a related question here which is where should the HTTP status
messages come from but I think that's an additional discussion. I think
they should only come from the workflow scripts or from PSGI.pm but open to
other ideas.
Now we're getting back to the beginning of the discussion: if the UI
templating engine doesn't take care of generating the actual output, then
what role *does* it have? I mean, expanding the template is a function
already performed by Template Toolkit...
--
Bye,
Erik.
http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.
--
Best Wishes,
Chris Travers
Efficito: Hosted Accounting and ERP. Robust and Flexible. No vendor
lock-in.
http://www.efficito.com/learn_more