merge-the-resolvers patch

John Williams <[email protected]>
Newsgroups gmane.comp.web.mason.devel
Message-ID <[email protected]>
I apologize for not posting this patch sooner.  I've been too busy to
finish this properly.

The attached patch eliminates the HTML::Mason::Resolver::File::ApacheHandler
class, merging it with HTML::Mason::Resolver::File.
All tests currently pass.

What is left undone?

HTML::Mason::Resolver::File defaults comp_root to the current directory,
while HTML::Mason::Resolver::File::ApacheHandler had NO default comp_root.

There are no tests for this behavior currently, and I'm not sure what
changing the default comp_root would do to users.  Basically, I have
not had time to write the tests to verify the current behavior.


HTML::Mason::ApacheHandler->new tries assign a default comp_root, but
I think this only works if the ApacheHandler is instantiated inside
the handler() routine.

    if ( exists $allowed_params->{comp_root} and
         my $req = $r || Apache->request )  # DocumentRoot is only available inside requests
    {
        $defaults{comp_root} = $req->document_root;
    }

I tried this simple hack in Resolver::File->new, but it caused problems
because Module::Build imported Apache modules even when it wasn't using
them in tests.

    # default comp_root of cwd does not apply under mod_perl
    delete $package->valid_params->{comp_root}->{default} if (%Apache::);

Maybe ApacheHandler->new should enforce the default, something like this:

    if ( exists $allowed_params->{comp_root} ) {
      my $req = $r || Apache->request;
      if ($req) {
        $defaults{comp_root} = $req->document_root;
      } else {
        param_error "comp_root parameter is required"
            unless $params{comp_root};
      }
    }

I'll punt this patch to the other developers at this point.
Hopefully whoever put the current defaults for comp_root into place
can make a better decision about how to handle it.

~ John Williams
resolver.patch (text/plain, 6.8 KB)
Index: dist/lib/HTML/Mason/ApacheHandler.pm
===================================================================
RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/ApacheHandler.pm,v
retrieving revision 1.309
diff -u -r1.309 ApacheHandler.pm
--- dist/lib/HTML/Mason/ApacheHandler.pm	8 Feb 2004 05:19:38 -0000	1.309
+++ dist/lib/HTML/Mason/ApacheHandler.pm	11 Feb 2004 18:36:56 -0000
@@ -181,61 +181,6 @@
 
 #----------------------------------------------------------------------
 #
-# APACHE-SPECIFIC FILE RESOLVER OBJECT
-#
-package HTML::Mason::Resolver::File::ApacheHandler;
-
-use strict;
-
-use HTML::Mason::Tools qw(paths_eq);
-
-use HTML::Mason::Resolver::File;
-use base qw(HTML::Mason::Resolver::File);
-use Params::Validate qw(SCALAR ARRAYREF);
-
-BEGIN
-{
-    __PACKAGE__->valid_params
-	(
-	 comp_root =>   # This is optional in superclass, but required for us.
-	 { parse => 'list',
-	   type => SCALAR|ARRAYREF,
-	   descr => "A string or array of arrays indicating the search path for component calls" },
-	);
-}
-
-#
-# Given an apache request object, return the associated component
-# path or undef if none exists. This is called for top-level web
-# requests that resolve to a particular file.
-#
-sub apache_request_to_comp_path {
-    my ($self, $r) = @_;
-
-    my $file = $r->filename;
-    $file .= $r->path_info unless -f $file;
-
-    # Clear up any weirdness here so that paths_eq compares two
-    # 'canonical' paths (canonpath is called on comp roots when
-    # resolver object is created.  Seems to be needed on Win32 (see
-    # bug #356).
-    $file = File::Spec->canonpath($file);
-
-    foreach my $root (map $_->[1], $self->comp_root_array) {
-	if (paths_eq($root, substr($file, 0, length($root)))) {
-	    my $path = substr($file, length $root);
-            $path = length $path ? join '/', File::Spec->splitdir($path) : '/';
-            chop $path if $path ne '/' && substr($path, -1) eq '/';
-
-            return $path;
-	}
-    }
-    return undef;
-}
-
-
-#----------------------------------------------------------------------
-#
 # APACHEHANDLER OBJECT
 #
 package HTML::Mason::ApacheHandler;
@@ -575,8 +520,6 @@
     my %defaults;
     $defaults{request_class}  = 'HTML::Mason::Request::ApacheHandler'
         unless exists $params{request};
-    $defaults{resolver_class} = 'HTML::Mason::Resolver::File::ApacheHandler'
-        unless exists $params{resolver};
 
     my $allowed_params = $class->allowed_params(%defaults, %params);
 
Index: dist/lib/HTML/Mason/Request.pm
===================================================================
RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Request.pm,v
retrieving revision 1.323
diff -u -r1.323 Request.pm
--- dist/lib/HTML/Mason/Request.pm	31 Jan 2004 18:07:22 -0000	1.323
+++ dist/lib/HTML/Mason/Request.pm	11 Feb 2004 18:36:56 -0000
@@ -1537,6 +1537,11 @@
 L<Apache|HTML::Mason::ApacheHandler>, standard output is
 redirected to C<< $r->print >>.
 
+=item plugins
+
+  ## placeholder to make Mason::Build happy... ##
+  ## was giving error: could not find pod entry for plugins ##
+
 =back
 
 =head1 METHODS
Index: dist/lib/HTML/Mason/Resolver.pm
===================================================================
RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Resolver.pm,v
retrieving revision 1.48
diff -u -r1.48 Resolver.pm
--- dist/lib/HTML/Mason/Resolver.pm	4 Jun 2003 20:44:02 -0000	1.48
+++ dist/lib/HTML/Mason/Resolver.pm	11 Feb 2004 18:36:56 -0000
@@ -103,8 +103,7 @@
 
 If you are creating a new resolver that you intend to use with the
 L<HTML::Mason::ApacheHandler|HTML::Mason::ApacheHandler> module, then
-you must implement the following method as well, possibly in a
-different subclass.
+you must implement the following method as well.
 
 =over 4
 
@@ -119,12 +118,6 @@
 
 =back
 
-For example, Mason includes the
-L<HTML::Mason::Resolver::File|HTML::Mason::Resolver::File> and
-HTML::Mason::Resolver::File::ApacheHandler classes.  The latter simply
-adds an implementation of the C<apache_request_to_comp_path> method
-for file based components.
-
 =head1 SEE ALSO
 
 L<HTML::Mason|HTML::Mason>
Index: dist/lib/HTML/Mason/Subclassing.pod
===================================================================
RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Subclassing.pod,v
retrieving revision 1.14
diff -u -r1.14 Subclassing.pod
--- dist/lib/HTML/Mason/Subclassing.pod	15 May 2003 23:26:39 -0000	1.14
+++ dist/lib/HTML/Mason/Subclassing.pod	11 Feb 2004 18:36:56 -0000
@@ -294,7 +294,6 @@
   my $interp =
       My::Interp->new
           ( request_class  => 'HTML::Mason::Request::ApacheHandler',
-            resolver_class => 'HTML::Mason::Resolver::File::ApacheHandler',
             my_new_interp_param => 42,
           );
 
Index: dist/lib/HTML/Mason/Resolver/File.pm
===================================================================
RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Resolver/File.pm,v
retrieving revision 1.86
diff -u -r1.86 File.pm
--- dist/lib/HTML/Mason/Resolver/File.pm	8 Nov 2003 17:23:59 -0000	1.86
+++ dist/lib/HTML/Mason/Resolver/File.pm	11 Feb 2004 18:36:56 -0000
@@ -18,6 +18,8 @@
 
 use HTML::Mason::Exceptions (abbr => ['param_error']);
 
+# need to test for proper default comp_root
+# should get exception if instantiated outside a request under Apache
 __PACKAGE__->valid_params
     (
      comp_root =>
@@ -100,6 +102,7 @@
 
 #
 # Given a glob pattern of url_paths, return all existing url_paths for that glob.
+# glob_path is required for using the "preloads" parameter.
 #
 sub glob_path {
     my ($self,$pattern) = @_;
@@ -118,6 +121,37 @@
     return keys(%path_hash);
 }
 
+#
+# Given an apache request object, return the associated component
+# path or undef if none exists. This is called for top-level web
+# requests that resolve to a particular file.
+# apache_request_to_comp_path is required for running Mason under mod_perl.
+#
+sub apache_request_to_comp_path {
+    my ($self, $r) = @_;
+
+    my $file = $r->filename;
+    $file .= $r->path_info unless -f $file;
+
+    # Clear up any weirdness here so that paths_eq compares two
+    # 'canonical' paths (canonpath is called on comp roots when
+    # resolver object is created.  Seems to be needed on Win32 (see
+    # bug #356).
+    $file = File::Spec->canonpath($file);
+
+    foreach my $root (map $_->[1], $self->comp_root_array) {
+	if (paths_eq($root, substr($file, 0, length($root)))) {
+	    my $path = substr($file, length $root);
+            $path = length $path ? join '/', File::Spec->splitdir($path) : '/';
+            chop $path if $path ne '/' && substr($path, -1) eq '/';
+
+            return $path;
+	}
+    }
+    return undef;
+}
+
+
 1;
 
 __END__
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.