Bug 530960: Put hooks into template/default/hook instead of template/hook

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> (module owner) a=mkanat

3 files changed, 41 insertions, 56 deletions

diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm
index 254cc237b..107f91a9b 100644
--- a/Bugzilla/Install/Util.pm
+++ b/Bugzilla/Install/Util.pm
@@ -43,8 +43,6 @@ our @EXPORT_OK = qw(
     install_string
     include_languages
     template_include_path
-    template_base_directories
-    template_lang_directories
     vers_cmp
     get_console_locale
     init_console
@@ -300,20 +298,14 @@ sub include_languages {
     return @usedlanguages;
 }
 
-# Used by template_include_path and Bugzilla::Template::Plugin::Hook.
-sub template_lang_directories {
-    my ($languages, $templatedir, $subdir_name) = @_;
+# Used by template_include_path
+sub _template_lang_directories {
+    my ($languages, $templatedir) = @_;
     
-    my @add;
+    my @add = qw(custom default);
     my $project = bz_locations->{'project'};
-    if ($subdir_name) {
-        @add = ("$subdir_name.custom", $subdir_name);
-        unshift(@add, "$subdir_name.$project") if $project;
-    }
-    else {
-        @add = ("custom", "default");
-        unshift(@add, $project) if $project;
-    }
+    unshift(@add, $project) if $project;
+
     my @result;
     foreach my $lang (@$languages) {
         foreach my $dir (@add) {
@@ -327,8 +319,8 @@ sub template_lang_directories {
     return @result;
 }
 
-# Used by template_include_path and Bugzilla::Template::Plugin::Hook.
-sub template_base_directories {
+# Used by template_include_path.
+sub _template_base_directories {
     # First, we add extension template directories, because extension templates
     # override standard templates. Extensions may be localized in the same way
     # that Bugzilla templates are localized.
@@ -339,14 +331,23 @@ sub template_base_directories {
 }
 
 sub template_include_path {
+    my ($params) = @_;
     my @used_languages = include_languages(@_);
     # Now, we add template directories in the order they will be searched:
-    my $template_dirs = template_base_directories(); 
+    my $template_dirs = _template_base_directories(); 
 
     my @include_path;
     foreach my $template_dir (@$template_dirs) {
-        push(@include_path,
-             template_lang_directories(\@used_languages, $template_dir));
+        my @lang_dirs = _template_lang_directories(\@used_languages, 
+                                                   $template_dir);
+        # Hooks get each set of extension directories separately.
+        if ($params->{hook}) {
+            push(@include_path, \@lang_dirs);
+        }
+        # Whereas everything else just gets a whole INCLUDE_PATH.
+        else {
+            push(@include_path, @lang_dirs);
+        }
     }
     return \@include_path;
 }
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index c3c7a9fac..f714f04d1 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -475,6 +475,14 @@ sub create {
         PRE_CHOMP => 1,
         TRIM => 1,
 
+        # Bugzilla::Template::Plugin::Hook uses the absolute (in mod_perl)
+        # or relative (in mod_cgi) paths of hook files to explicitly compile
+        # a specific file. Also, these paths may be absolute at any time
+        # if a packager has modified bz_locations() to contain absolute
+        # paths.
+        ABSOLUTE => 1,
+        RELATIVE => $ENV{MOD_PERL} ? 0 : 1,
+
         COMPILE_DIR => bz_locations()->{'datadir'} . "/template",
 
         # Initialize templates (f.e. by loading plugins like Hook).
diff --git a/Bugzilla/Template/Plugin/Hook.pm b/Bugzilla/Template/Plugin/Hook.pm
index fb59a495b..9c292d726 100644
--- a/Bugzilla/Template/Plugin/Hook.pm
+++ b/Bugzilla/Template/Plugin/Hook.pm
@@ -27,8 +27,7 @@ use strict;
 use base qw(Template::Plugin);
 
 use Bugzilla::Constants;
-use Bugzilla::Install::Util qw(include_languages template_base_directories
-                               template_lang_directories);
+use Bugzilla::Install::Util qw(include_languages template_include_path); 
 use Bugzilla::Util;
 use Bugzilla::Error;
 
@@ -58,16 +57,18 @@ sub process {
     my $type = $2;
 
     # Hooks are named like this:
-    my $extension_template = "$path/$template_name-$hook_name.$type.tmpl";
+    my $extension_template = "$path$template_name-$hook_name.$type.tmpl";
 
     # Get the hooks out of the cache if they exist. Otherwise, read them
     # from the disk.
     my $cache = Bugzilla->request_cache->{template_plugin_hook_cache} ||= {};
-    $cache->{$extension_template} ||= $self->_get_hooks($extension_template);
+    my $lang = $cache->{language} || '';
+    $cache->{"${lang}__$extension_template"} 
+        ||= $self->_get_hooks($extension_template);
 
     # process() accepts an arrayref of templates, so we just pass the whole
     # arrayref.
-    return $context->process($cache->{$extension_template});
+    return $context->process($cache->{"${lang}__$extension_template"});
 }
 
 sub _get_hooks {
@@ -76,21 +77,10 @@ sub _get_hooks {
     my $template_sets = _template_hook_include_path();
     my @hooks;
     foreach my $dir_set (@$template_sets) {
-        foreach my $lang_dir (@$dir_set) {
-            my $file = File::Spec->catdir($lang_dir, $extension_template);
+        foreach my $template_dir (@$dir_set) {
+            my $file = "$template_dir/hook/$extension_template";
             if (-e $file) {
-                # TT won't accept a file that isn't in its include path. 
-                # So we open a file handle, compile it to a template, and 
-                # then pass the compiled template to process().
-                #
-                # This doesn't cache the hook template on disk
-                # (which is nearly impossible for us to do, with TT's
-                # architecture), but we do cache this compiled template
-                # per-request (in process() above) so that it doesn't have
-                # to be recompiled over and over within one request.
-                open(my $fh, '<', $file) or die "$file: $!";
-                my $template = $self->_context->template($fh);
-                close($fh);
+                my $template = $self->_context->template($file);
                 push(@hooks, $template);
                 # Don't run the hook for more than one language.
                 last;
@@ -105,25 +95,11 @@ sub _template_hook_include_path {
     my $cache = Bugzilla->request_cache;
     my $language = $cache->{language} || '';
     my $cache_key = "template_plugin_hook_include_path_$language";
-    return $cache->{$cache_key} if defined $cache->{$cache_key};
-    
-    my @used_languages = include_languages({
+    $cache->{$cache_key} ||= template_include_path({
         use_languages => Bugzilla->languages,
-        only_language => $language });
-    my $template_dirs = template_base_directories();
-
-    # We create an array of arrayrefs, with each arrayref being a single
-    # extension's "language" directories. In addition to the extensions/
-    # directory, this also includes a set for the base template/ directory. 
-    my @template_sets;
-    foreach my $template_dir (@$template_dirs) {
-        my @language_dirs = template_lang_directories(\@used_languages, 
-                                                      $template_dir, 'hook');
-        if (scalar @language_dirs) {
-            push(@template_sets, \@language_dirs);
-        }
-    }
-    $cache->{$cache_key} = \@template_sets;
+        only_language => $language,
+        hook          => 1,
+    });
     return $cache->{$cache_key};
 }