Re: proposal for filter_init implementation (was: Re: rfc: new filtering APIs)

Stas Bekman wrote:
> As we now have most of filtering API completed, it's time to finish off 
> the filter_init API. 

:)

> Here is a proposal for filter_init implementation:
>
> Extend the struct modperl_filter_ctx_t, to include:
>
>   int init_done;
>   modperl_handle_t *init_handler;
>
> now we can register a new sub-routine to perform init for a given
> filter and have a flag to make sure that this is done once.

cool.  does this mean that the code will be available (somehow) via $f->ctx? 
 I think that maybe there might be value in being able to do

$f->ctx->{init}->();

someday :)

> The only troublesome issue is how to configure the addition of the init
> function. We have discussed this earlier and at this moment I'm thinking 
> about the following two options:
>
> 1) I haven't tried yet, but according to the attributes manpage
> it's possible to pass arguments to the attributes. If that works to
> add an init handler:
>
> sub my_init {
>     my $filter = shift;
>     # ...
>     return OK;
> }
>
> to the existing filter handler:
>
> sub handler : FilterRequestHandler {}
>
> one needs to do:
>
> sub handler : FilterRequestHandler FilterInitHandler('my_init') {}
>
> even if this works, we probably might have issues with resolving 
> 'my_init' and anonymous callbacks won't be possible.

well, personally I like this approach the best I think.  at the very least, 
we need this (in addition to other approaches) in order to allow module 
writers to make init routines mandatory (see below).

as for anonymous subs, we won't be able to use them now or ever?   I know 
there are some incomplete issues with using anonymous subs in certain areas 
(such as inlined in httpd.conf) but I was assuming that they would be 
non-issues eventually.

> 2) Another alternative is to change the way PerlInputFilterHandler is
> configured and let it be TAKE12 instead of ITERATE and then you can just 
> say:
>
> PerlInputFilterHandler handler [init_handler]
>
> but then filter handlers will differ from other handlers. I don't know
> if it's a good thing or not. But it's certainly the simplest solution.

I don't think this is bad per se, but it cannot be othe only way.  as I've 
said before, the init handler will be an integral part of the filter itself 
- in most cases you will not want to separate it from the module, since you 
will need it in order for the module to function properly (think managing 
conditional GETs).  so, asking module users to do more than

PerlOutputFilterHandler My::Filter

seems excessive from an EU standpoint for modules that require the hook - it 
should be transparent.  it also suggests that the [init_handler] is optional 
where it may not be for a given module.

the flip side of this is that if you give the module writer no interface to 
filter_init (such as solution #1) then module writers have no way to make 
the init routine required.  yucko.

> The perl API is easy, just change:
>
> void mpxs_Apache__RequestRec_add_input_filter(pTHX_ request_rec *r,
>                                               SV *callback)
>
> to:
>
> void mpxs_Apache__RequestRec_add_input_filter(pTHX_ request_rec *r,
>                                               SV *callback, SV 
> *init_callback)

as an aside, is there any reason you keep using input filters as the 
example?  I haven't looked over the input filter design as much as with 
output filters, but filter_init was added specifically for handling 
conditional GET requests with output filters - I'm not sure what value there 
is for input filters, or even if the input filter Apache mechanism supports 
it.  but if it does, cool :)

--Geoff