Bug 509053: Implement Bugzilla->feature (feature_enabled in the templates), and use it to detect when PatchReader is available.

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

9 files changed, 55 insertions, 36 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 671cda38d..43a9b39ae 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -42,6 +42,7 @@ use Bugzilla::Auth::Persist::Cookie;
 use Bugzilla::CGI;
 use Bugzilla::DB;
 use Bugzilla::Install::Localconfig qw(read_localconfig);
+use Bugzilla::Install::Requirements qw(OPTIONAL_MODULES);
 use Bugzilla::JobQueue;
 use Bugzilla::Template;
 use Bugzilla::User;
@@ -187,6 +188,40 @@ sub template_inner {
     return $class->request_cache->{"template_inner_$lang"};
 }
 
+sub feature {
+    my ($class, $feature) = @_;
+    my $cache = $class->request_cache;
+    return $cache->{feature}->{$feature}
+        if exists $cache->{feature}->{$feature};
+
+    my $feature_map = $cache->{feature_map};
+    if (!$feature_map) {
+        foreach my $package (@{ OPTIONAL_MODULES() }) {
+            foreach my $f (@{ $package->{feature} }) {
+                $feature_map->{$f} ||= [];
+                push(@{ $feature_map->{$f} }, $package->{module});
+            }
+        }
+        $cache->{feature_map} = $feature_map;
+    }
+
+    if (!$feature_map->{$feature}) {
+        ThrowCodeError('invalid_feature', { feature => $feature });
+    }
+
+    my $success = 1;
+    foreach my $module (@{ $feature_map->{$feature} }) {
+        # We can't use a string eval and "use" here (it kills Template-Toolkit,
+        # see https://rt.cpan.org/Public/Bug/Display.html?id=47929), so we have
+        # to do a block eval.
+        $module =~ s{::}{/}g;
+        $module .= ".pm";
+        eval { require $module; 1; } or $success = 0;
+    }
+    $cache->{feature}->{$feature} = $success;
+    return $success;
+}
+
 sub cgi {
     my $class = shift;
     $class->request_cache->{cgi} ||= new Bugzilla::CGI();
@@ -759,4 +794,9 @@ Returns a L<Bugzilla::JobQueue> that you can use for queueing jobs.
 Will throw an error if job queueing is not correctly configured on
 this Bugzilla installation.
 
+=item C<feature>
+
+Tells you whether or not a specific feature is enabled. For names
+of features, see C<OPTIONAL_MODULES> in C<Bugzilla::Install::Requirements>.
+
 =back
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index d6f665952..bea1639f3 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -281,12 +281,8 @@ sub get_attachment_link {
         # If the attachment is a patch, try to link to the diff rather
         # than the text, by default.
         my $patchlink = "";
-        if ($is_patch) {
-            # Determine if PatchReader is installed
-            my $patchviewer_installed = eval { require PatchReader; };
-            if ($patchviewer_installed) {
-                $patchlink = '&amp;action=diff';
-            }
+        if ($is_patch and Bugzilla->feature('patch_viewer')) {
+            $patchlink = '&amp;action=diff';
         }
 
         # Whitespace matters here because these links are in <pre> tags.
@@ -745,6 +741,8 @@ sub create {
                 return \@bug_list;
             },
 
+            'feature_enabled' => sub { return Bugzilla->feature(@_); },
+
             # These don't work as normal constants.
             DB_MODULE        => \&Bugzilla::Constants::DB_MODULE,
             REQUIRED_MODULES => 
diff --git a/attachment.cgi b/attachment.cgi
index 2e952746e..be82294d7 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -86,12 +86,6 @@ if ($action ne 'view') {
     Bugzilla->login();
 }
 
-# Determine if PatchReader is installed
-eval {
-    require PatchReader;
-    $vars->{'patchviewerinstalled'} = 1;
-};
-
 # When viewing an attachment, do not request credentials if we are on
 # the alternate host. Let view() decide when to call Bugzilla->login.
 if ($action eq "view")
diff --git a/post_bug.cgi b/post_bug.cgi
index f3b31e9a8..323e005f4 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -240,12 +240,6 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
     else {
         $vars->{'message'} = 'attachment_creation_failed';
     }
-
-    # Determine if Patch Viewer is installed, for Diff link
-    eval {
-        require PatchReader;
-        $vars->{'patchviewerinstalled'} = 1;
-    };
 }
 
 # Set bug flags.
diff --git a/process_bug.cgi b/process_bug.cgi
index c0ee54938..36091b892 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -634,13 +634,6 @@ foreach my $bug (@bug_objects) {
     }
 }
 
-# Determine if Patch Viewer is installed, for Diff link
-# (NB: Duplicate code with show_bug.cgi.)
-eval {
-    require PatchReader;
-    $vars->{'patchviewerinstalled'} = 1;
-};
-
 if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) {
     # Do nothing.
 }
diff --git a/show_bug.cgi b/show_bug.cgi
index 4a530269e..ddb41ffec 100755
--- a/show_bug.cgi
+++ b/show_bug.cgi
@@ -91,12 +91,6 @@ if ($single) {
     }
 }
 
-# Determine if Patch Viewer is installed, for Diff link
-eval {
-  require PatchReader;
-  $vars->{'patchviewerinstalled'} = 1;
-};
-
 $vars->{'bugs'} = \@bugs;
 $vars->{'marks'} = \%marks;
 
diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl
index fadf3e308..bbdf24866 100644
--- a/template/en/default/attachment/edit.html.tmpl
+++ b/template/en/default/attachment/edit.html.tmpl
@@ -40,7 +40,7 @@
 %]
 
 [%# No need to display the Diff button and iframe if the attachment is not a patch. %]
-[% patchviewerinstalled = (patchviewerinstalled && attachment.ispatch) %]
+[% use_patchviewer = (feature_enabled('patch_viewer') && attachment.ispatch) %]
 
 <form method="post" action="attachment.cgi" onsubmit="normalizeComments();">
   <input type="hidden" name="id" value="[% attachment.id %]">
@@ -127,7 +127,7 @@
         <input type="submit" value="Submit" id="update"><br><br>
         <strong>Actions:</strong>
         <a href="attachment.cgi?id=[% attachment.id %]">View</a>
-        [% IF attachment.ispatch && patchviewerinstalled %]
+        [% IF use_patchviewer %]
          | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=diff">Diff</a>
         [% END %]
         [% IF Param("allow_attachment_deletion")
@@ -183,14 +183,14 @@
             var patchviewerinstalled = 0;
             var attachment_id = [% attachment.id %];
             if (typeof document.getElementById == "function") {
-[% IF patchviewerinstalled %]
+[% IF use_patchviewer %]
               var patchviewerinstalled = 1;
               document.write('<iframe id="viewDiffFrame" style="height: 400px; width: 100%; display: none;"><\/iframe>');
 [% END %]
               document.write('<button type="button" id="editButton" onclick="editAsComment(patchviewerinstalled);">Edit Attachment As Comment<\/button>');
               document.write('<button type="button" id="undoEditButton" onclick="undoEditAsComment(patchviewerinstalled);" style="display: none;">Undo Edit As Comment<\/button>');
               document.write('<button type="button" id="redoEditButton" onclick="redoEditAsComment(patchviewerinstalled);" style="display: none;">Redo Edit As Comment<\/button>');
-[% IF patchviewerinstalled %]
+[% IF use_patchviewer %]
               document.write('<button type="button" id="viewDiffButton" onclick="viewDiff(attachment_id, patchviewerinstalled);">View Attachment As Diff<\/button>');
 [% END %]
               document.write('<button type="button" id="viewRawButton" onclick="viewRaw(patchviewerinstalled);" style="display: none;">View Attachment As Raw<\/button>');
diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl
index 04894ff8d..bd597e48b 100644
--- a/template/en/default/attachment/list.html.tmpl
+++ b/template/en/default/attachment/list.html.tmpl
@@ -117,7 +117,7 @@ function toggle_display(link) {
 
         <td valign="top">
           <a href="attachment.cgi?id=[% attachment.id %]&amp;action=edit">Details</a>
-          [% IF attachment.ispatch && patchviewerinstalled %]
+          [% IF attachment.ispatch && feature_enabled('patch_viewer') %]
             | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=diff">Diff</a>
           [% END %]
           [% Hook.process("action") %]
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index d75ca4432..8ad0c4ae9 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -198,6 +198,12 @@
     [% title = "Invalid Dimensions" %]
     The width or height specified is not a positive integer.
 
+  [% ELSIF error == "invalid_feature" %]
+    [% title = "Invalid Feature Name" %]
+    [% feature FILTER html %] is not a valid feature name. See
+    <code>OPTIONAL_MODULES</code> in 
+    <code>Bugzilla::Install::Requirements</code> for valid names.
+
   [% ELSIF error == "invalid_flag_association" %]
     [% title = "Invalid Flag Association" %]
     Some flags do not belong to