Re: [mp2 patch] introducing Apache::compat "scoping"

On Sun, 2003-12-14 at 18:33, Stas Bekman wrote:
> The following is yet another attempt to avoid collisions between 
> Apache::compat and the real mp2 APIs.

That is indeed an annoying problem.

> Similar to 'use' and 'no' pragma, I'm suggesting to introduce the new 
> functions override_mp2_api and restore_mp2_api for those APIs in 
> Apache::compat that collide with 2.0 API. The only difference is that one 
> needs an explicit call to restore_mp2_api, which I suppose could be done by 
> returning an object, which if made a lexically scoped will call 
> restore_mp2_api on its own (i.e. DESTROY), but since it's going to be used 
> very infrequently and eventually won't be needed at all, I don't see a reason 
> to bother. See the compat/request test change for an example in the patch 
> below.
> 
> I decided not to use the import() method, but have an explicit function call, 
> since import() may make some people think that it imports the overriden 
> methods, which it doesn't.

I would probably prefer to use import() just to make things as easy as
possible for the potential users of Apache::compat(), but as long as
it's documented, I don't think I'd mind much.

> I think there are only two functions at the moment that collide with mp2 API:
> Apache::RequestRec::notes and Apache::RequestRec::finfo. There is also 
> APR::URI::unparse, which introduces a special case but I think it's harmless.
> 
> in June I posted an implementation for 2 more colliding functions:
> Apache::Connection::local_addr
> Apache::Connection::remote_addr
> http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=105452446932154&w=2
> which I will now be able to commit using this new functionality.
> 
> The cool thing is that we can introduce a sub-class of Registry which will do 
> the wrapping into a handler like so:

Coolness indeed !

>   'sub handler {' .
>   'Apache::compat::override_mp2_api('Apache::RequestRec::notes');' .
>    $code .
>   'Apache::compat::restore_mp2_api('Apache::RequestRec::notes');' .
>   '}';
> 
> or something like that (overriding other subs as well).
> 
> Here is the patch:
> 
> Index: lib/Apache/compat.pm
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/lib/Apache/compat.pm,v
> retrieving revision 1.90
> diff -u -r1.90 compat.pm
> --- lib/Apache/compat.pm      19 Nov 2003 19:30:11 -0000      1.90
> +++ lib/Apache/compat.pm      15 Dec 2003 02:02:10 -0000
> @@ -50,6 +50,86 @@
>       $INC{'Apache/Table.pm'} = __FILE__;
>   }
> 
> +# api => "overriding code"
> +# the overriding code, needs to "return" the original CODE reference
> +# when eval'ed , so that it can be restored later
> +my %overridable_mp2_api = (
> +    'Apache::RequestRec::notes' => <<'EOI',
> +{
> +    require Apache::RequestRec;
> +    my $notes_sub = *Apache::RequestRec::notes{CODE};
> +    *Apache::RequestRec::notes = sub {
> +        my $r = shift;
> +        return wantarray()
> +            ?       ($r->table_get_set(scalar($r->$notes_sub), @_))
> +            : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
> +    };
> +    $notes_sub;
> +}
> +EOI
> +
> +    'Apache::RequestRec::finfo' => <<'EOI',
> +{
> +    require APR::Finfo;
> +    my $finfo_sub = *APR::Finfo::finfo{CODE};
> +    sub Apache::RequestRec::finfo {
> +        my $r = shift;
> +        stat $r->filename;
> +        \*_;
> +    }
> +    $finfo_sub;
> +}
> +EOI
> +);

Why make this code a SCALAR, instead of creating an anonymous sub right
there?

> +my %overridden_mp2_api = ();
> +
> +# this function enables back-compatible APIs which can't coexist with
> +# mod_perl 2.0 APIs with the same name and therefore it should be
> +# avoided if possible.
> +#
> +# it expects a list of fully qualified functions, like
> +# "Apache::RequestRec::finfo"
> +sub override_mp2_api {
> +    my (@subs) = @_;
> +
> +    for my $sub (@subs) {
> +        unless (exists $overridable_mp2_api{$sub}) {
> +            die __PACKAGE__ . ": $sub is not overridable";
> +        }
> +        if(exists $overridden_mp2_api{$sub}) {
> +            warn __PACKAGE__ . ": $sub has been already overridden";
> +            next;
> +        }
> +        $overridden_mp2_api{$sub} = eval $overridable_mp2_api{$sub};
> +        unless (exists $overridden_mp2_api{$sub} &&
> +                ref($overridden_mp2_api{$sub}) eq 'CODE') {
> +            die "overriding $sub didn't return a CODE ref";
> +        }
> +    }
> +}
> +
> +# restore_mp2_api does the opposite of override_mp2_api(), it removes
> +# the overriden API and restores the original mod_perl 2.0 API
> +sub restore_mp2_api {
> +    my (@subs) = @_;
> +
> +    for my $sub (@subs) {
> +        unless (exists $overridable_mp2_api{$sub}) {
> +            die __PACKAGE__ . ": $sub is not overridable";
> +        }
> +        unless (exists $overridden_mp2_api{$sub}) {
> +            warn __PACKAGE__ . ": can't restore $sub, " .
> +                "as it has not been overridden";
> +            next;
> +        }
> +        my $original_sub = delete $overridden_mp2_api{$sub};
> +        no warnings 'redefine';
> +        no strict 'refs';
> +        *$sub = $original_sub;
> +    }
> +}
>   sub request {
>       my $what = shift;
> 
> @@ -249,15 +329,6 @@
>           : scalar($r->table_get_set(scalar($r->err_headers_out), @_));
>   }
> 
> -{
> -    my $notes_sub = *Apache::RequestRec::notes{CODE};
> -    *Apache::RequestRec::notes = sub {
> -        my $r = shift;
> -        return wantarray()
> -            ?       ($r->table_get_set(scalar($r->$notes_sub), @_))
> -            : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
> -    }
> -}
> 
>   sub register_cleanup {
>       shift->pool->cleanup_register(@_);
> @@ -345,12 +416,6 @@
> 
>   sub chdir_file {
>       #XXX resolve '.' in @INC to basename $r->filename
> -}
> -
> -sub finfo {
> -    my $r = shift;
> -    stat $r->filename;
> -    \*_;
>   }
> 
>   *log_reason = \&log_error;
> Index: t/response/TestCompat/request.pm
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/t/response/TestCompat/request.pm,v
> retrieving revision 1.3
> diff -u -r1.3 request.pm
> --- t/response/TestCompat/request.pm  11 Apr 2003 07:34:03 -0000      1.3
> +++ t/response/TestCompat/request.pm  15 Dec 2003 02:02:10 -0000
> @@ -75,6 +75,8 @@
> 
>       # $r->notes
>       {
> +        Apache::compat::override_mp2_api('Apache::RequestRec::notes');
> +
>           my $key = 'notes-test';
>           # get/set scalar context
>           {
> @@ -98,6 +100,10 @@
>               $r->notes->add($key => $_) for @exp;
>               ok t_cmp(\@exp, [ $r->notes($key) ], "\$r->notes in list 
> context");
>           }
> +
> +        # restore the real 2.0 notes() method, now that we are done
> +        # with the compat one
> +        Apache::compat::restore_mp2_api('Apache::RequestRec::notes');
>       }
> 
>       # get_remote_host()
> 
> 
> __________________________________________________________________
> Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/     mod_perl Guide ---> http://perl.apache.org
> mailto:stas@xxxxxxxxxx http://use.perl.org http://apacheweek.com
> http://modperlbook.org http://apache.org   http://ticketmaster.com
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxx
> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxx
-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

signature.asc
Description: This is a digitally signed message part