Bug 1350909 - Make index.cgi cache-friendly for logged out requests

7 files changed, 158 insertions, 155 deletions

diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 2887f0138..1f0892a32 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -38,6 +38,7 @@ use File::Spec;
 use IO::Dir;
 use List::MoreUtils qw(firstidx);
 use Scalar::Util qw(blessed);
+use JSON::XS qw(encode_json);
 
 use base qw(Template);
 
@@ -976,6 +977,10 @@ sub create {
             # Function for retrieving global parameters.
             'Param' => sub { return Bugzilla->params->{$_[0]}; },
 
+            json_encode => sub {
+                return encode_json($_[0]);
+            },
+
             # Function to create date strings
             'time2str' => \&Date::Format::time2str,
 
diff --git a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
index 801d3d5fa..a472156f9 100644
--- a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
+++ b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
@@ -8,21 +8,11 @@
 [% USE Bugzilla %]
 
 [% IF Param('user_info_class').split(',').contains('GitHubAuth') %]
-  <script [% script_nonce FILTER none %] type="text/javascript">
-   YAHOO.util.Event.addListener('login_link[% qs_suffix FILTER js %]','click', function () {
-     var login_link = YAHOO.util.Dom.get('github_mini_login[% qs_suffix FILTER js %]');
-     YAHOO.util.Dom.removeClass(login_link, 'bz_default_hidden');
-   });
-   YAHOO.util.Event.addListener('hide_mini_login[% qs_suffix FILTER js %]','click', function () {
-     var login_link = YAHOO.util.Dom.get('github_mini_login[% qs_suffix FILTER js %]');
-     YAHOO.util.Dom.addClass(login_link, 'bz_default_hidden');
-   });
-  </script>
   <span id="github_mini_login[% qs_suffix FILTER html %]" class="bz_default_hidden">
     <form method="post" action="[% urlbase FILTER html %]github.cgi">
       <input type="hidden" name="github_secret" value="[% Bugzilla.github_secret FILTER html %]">
       <input type="hidden" name="target_uri" value="[% Bugzilla.cgi.target_uri FILTER html %]">
-        <input type="image" src="extensions/GitHubAuth/web/images/sign_in.png" height="22" width="75" align="absmiddle"
+      <input type="image" src="extensions/GitHubAuth/web/images/sign_in.png" height="22" width="75" align="absmiddle"
              alt="Sign in with GitHub"
              title="Sign in with GitHub"> or
     </form>
diff --git a/index.cgi b/index.cgi
index 56513aff7..420a162b6 100755
--- a/index.cgi
+++ b/index.cgi
@@ -16,43 +16,72 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Update;
+use Digest::MD5 qw(md5_hex);
+use List::MoreUtils qw(any);
 
 # Check whether or not the user is logged in
 my $user = Bugzilla->login(LOGIN_OPTIONAL);
 my $cgi = Bugzilla->cgi;
-my $template = Bugzilla->template;
 my $vars = {};
 
+# Yes, I really want to avoid two calls to the id method.
+my $user_id = $user->id;
+
+# We only cache unauthenticated requests now, because invalidating is harder for logged in users.
+my $can_cache = $user_id == 0;
+
 # And log out the user if requested. We do this first so that nothing
 # else accidentally relies on the current login.
 if ($cgi->param('logout')) {
     Bugzilla->logout();
     $user = Bugzilla->user;
+    $user_id = 0;
+    $can_cache = 0;
     $vars->{'message'} = "logged_out";
     # Make sure that templates or other code doesn't get confused about this.
     $cgi->delete('logout');
 }
 
-$cgi->content_security_policy(script_src  => ['self', 'nonce']);
+# our weak etag is based on the bugzilla version parameter (BMO customization) and the announcehtml
+# if either change, the cache will be considered invalid.
+my @etag_parts = (Bugzilla->params->{bugzilla_version}, Bugzilla->params->{announcehtml});
+my $weak_etag = q{W/"} . md5_hex(@etag_parts) . q{"};
+my $if_none_match = $cgi->http('If-None-Match');
+
+# load balancer (or client) will check back with us after max-age seconds
+# If the etag in If-None-Match is unchanged, we quickly respond without doing much work.
+my @cache_control = (
+    $can_cache ? 'public' : 'no-cache',
+    sprintf('max-age=%d', 60 * 5),
+);
 
-###############################################################################
-# Main Body Execution
-###############################################################################
+if ($can_cache && $if_none_match && any { $_ eq $weak_etag } split(/,\s*/, $if_none_match)) {
+    print $cgi->header(-status => '304 Not Modified', -ETag => $weak_etag);
+}
+else {
+    my $template = Bugzilla->template;
+    $cgi->content_security_policy(script_src  => ['self']);
 
-# Return the appropriate HTTP response headers.
-print $cgi->header();
+    # Return the appropriate HTTP response headers.
+    print $cgi->header(
+        -Cache_Control => join(', ', @cache_control),
+        $can_cache ? (-ETag => $weak_etag) : (),
+    );
 
-if ($user->in_group('admin')) {
-    # If 'urlbase' is not set, display the Welcome page.
-    unless (Bugzilla->params->{'urlbase'}) {
-        $template->process('welcome-admin.html.tmpl')
-          || ThrowTemplateError($template->error());
-        exit;
+    if ($user_id && $user->in_group('admin')) {
+        # If 'urlbase' is not set, display the Welcome page.
+        unless (Bugzilla->params->{'urlbase'}) {
+            $template->process('welcome-admin.html.tmpl')
+                or ThrowTemplateError($template->error());
+            exit;
+        }
+        # Inform the administrator about new releases, if any.
+        $vars->{'release'} = Bugzilla::Update::get_notifications();
     }
-    # Inform the administrator about new releases, if any.
-    $vars->{'release'} = Bugzilla::Update::get_notifications();
-}
 
-# Generate and return the UI (HTML page) from the appropriate template.
-$template->process("index.html.tmpl", $vars)
-  || ThrowTemplateError($template->error());
+    $vars->{use_login_page} = 1;
+
+    # Generate and return the UI (HTML page) from the appropriate template.
+    $template->process("index.html.tmpl", $vars)
+        or ThrowTemplateError( $template->error() );
+}
\ No newline at end of file
diff --git a/js/global.js b/js/global.js
index a997821f4..651d10241 100644
--- a/js/global.js
+++ b/js/global.js
@@ -16,6 +16,8 @@
 *                 
 */
 
+var BUGZILLA = $('head').data('bugzilla');
+
 $(function () {
   $('.show_mini_login_form').on("click", function (event) {
     return show_mini_login_form($(this).data('qs-suffix'));
@@ -32,15 +34,55 @@ $(function () {
   $('.check_mini_login_fields').on("click", function (event) {
     return check_mini_login_fields($(this).data('qs-suffix'));
   });
-  $('form .quicksearch_check_empty').on("submit", function (event) {
+  $('.quicksearch_check_empty').on("submit", function (event) {
       if (this.quicksearch.value == '') {
-        alert('Please enter one or more search terms first.');
-        return false;
+          alert('Please enter one or more search terms first.');
+          event.preventDefault();
       }
-      return true;
   });
+
+  unhide_language_selector();
+  $("#lob_action").on("change", update_text);
+  $("#lob_newqueryname").on("keyup", manage_old_lists);
 });
 
+function unhide_language_selector() {
+    $('#lang_links_container').removeClass('bz_default_hidden');
+}
+
+function update_text() {
+    // 'lob' means list_of_bugs.
+    var lob_action = document.getElementById('lob_action');
+    var action = lob_action.options[lob_action.selectedIndex].value;
+    var text = document.getElementById('lob_direction');
+    var new_query_text = document.getElementById('lob_new_query_text');
+
+    if (action == "add") {
+        text.innerHTML = "to";
+        new_query_text.style.display = 'inline';
+    }
+    else {
+        text.innerHTML = "from";
+        new_query_text.style.display = 'none';
+    }
+}
+
+function manage_old_lists() {
+    var old_lists = document.getElementById('lob_oldqueryname');
+    // If there is no saved searches available, returns.
+    if (!old_lists) return;
+
+    var new_query = document.getElementById('lob_newqueryname').value;
+
+    if (new_query != "") {
+        old_lists.disabled = true;
+    }
+    else {
+        old_lists.disabled = false;
+    }
+}
+
+
 function show_mini_login_form( suffix ) {
     $('#login_link' + suffix).addClass('bz_default_hidden');
     $('#mini_login' + suffix).removeClass('bz_default_hidden');
@@ -148,4 +190,4 @@ $().ready(function() {
     $(window).on('pageshow', function(event) {
         $('.bz_autocomplete').attr('autocomplete', 'off');
     });
-});
+});
\ No newline at end of file
diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl
index 2e08a461d..07a980050 100644
--- a/template/en/default/global/header.html.tmpl
+++ b/template/en/default/global/header.html.tmpl
@@ -38,6 +38,7 @@
   # generate_api_token: generate a token which can be used to make authenticated webservice calls
   # no_body: if true the body element will not be generated
   # allow_mobile: allow special CSS and viewport for detected mobile useragents
+  # use_login_page: display a link to the full login page, rather than an inline login.
   #%]
 
 [% IF message %]
@@ -116,11 +117,37 @@
   # value of title anyway.  To get around that problem we explicitly
   # set header's default value here only if it is undefined. %]
 [% IF !header.defined %][% header = title %][% END %]
-
+[%- js_BUGZILLA = {
+        param => {
+            cookiepath => Param('cookiepath'),
+            maxusermatches => Param('maxusermatches'),
+        },
+        constant => {
+            COMMENT_COLS => constants.COMMENT_COLS,
+        },
+        string => {
+            # Please keep these in alphabetical order.
+
+            attach_desc_required =>
+                'You must enter a Description for this attachment.',
+            component_required => 
+                "You must select a Component for this $terms.bug",
+            description_required => 
+                "You must enter a Description for this $terms.bug",
+            short_desc_required =>
+                "You must enter a Summary for this $terms.bug",
+            version_required =>
+                "You must select a Version for this $terms.bug"
+        }
+    };
+    IF generate_api_token;
+        js_BUGZILLA.api_token = get_api_token();
+    END; 
+%]
 <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
                       "http://www.w3.org/TR/html4/loose.dtd">
 <html lang="en">
-  <head>
+  <head data-bugzilla='[% json_encode(js_BUGZILLA) FILTER html %]'>
     [% Hook.process("start") %]
     <title>[% title %]</title>
 
@@ -181,65 +208,18 @@
       [% PROCESS format_js_link %]
     [% END %]
 
-    <script [% script_nonce FILTER none %] type="text/javascript">
-    <!--
+    [%# Make some Bugzilla information available to all scripts. 
+      # We don't import every parameter and constant because we
+      # don't want to add a lot of uncached JS to every page. 
+      # %]
+    
+    [% inline_javascript = BLOCK %]
         [% IF NOT no_yui %]
           YAHOO.namespace('bugzilla');
-          [% IF 0 %]
-          YAHOO.util.Event.addListener = function (el, sType, fn, obj, overrideContext) {
-              if ( ("onpagehide" in window || YAHOO.env.ua.gecko) && sType === "unload") { sType = "pagehide"; };
-              var capture = ((sType == "focusin" || sType == "focusout") && !YAHOO.env.ua.ie) ? true : false;
-              return this._addListener(el, this._getType(sType), fn, obj, overrideContext, capture);
-          };
-          [% END %]
           if ( "onpagehide" in window || YAHOO.env.ua.gecko) {
               YAHOO.util.Event._simpleRemove(window, "unload",
                                              YAHOO.util.Event._unload);
           }
-        [% END %]
-
-        [%# The language selector needs javascript to set its cookie,
-          # so it is hidden in HTML/CSS by the "bz_default_hidden" class.
-          # If the browser can run javascript, it will then "unhide"
-          # the language selector using the following code.
-          #%]
-        function unhide_language_selector() {
-            $('#lang_links_container').removeClass('bz_default_hidden');
-        } 
-        $(document).ready(unhide_language_selector);
-
-        [%# Make some Bugzilla information available to all scripts. 
-          # We don't import every parameter and constant because we
-          # don't want to add a lot of uncached JS to every page. 
-          #%]
-        var BUGZILLA = {
-            param: {
-                cookiepath: '[% Param('cookiepath') FILTER js %]',
-                maxusermatches: [% Param('maxusermatches') FILTER js %]
-            },
-            constant: {
-                COMMENT_COLS: [% constants.COMMENT_COLS FILTER js %]
-            },
-            string: {
-                [%# Please keep these in alphabetical order. %]
-
-                attach_desc_required:
-                    'You must enter a Description for this attachment.',
-                component_required:
-                    'You must select a Component for this [% terms.bug %].',
-                description_required:
-                    'You must enter a Description for this [% terms.bug %].',
-                short_desc_required:
-                    'You must enter a Summary for this [% terms.bug %].',
-                version_required:
-                    'You must select a Version for this [% terms.bug %].'
-            }
-            [% IF generate_api_token %]
-              , api_token: '[% get_api_token FILTER js FILTER html %]'
-            [% END %]
-        };
-
-        [% IF NOT no_yui %]
           [% FOREACH yui_name = yui %]
             [% FOREACH yui_template = yui_templates.$yui_name %]
               [% INCLUDE $yui_template %]
@@ -250,8 +230,12 @@
         [% IF javascript %]
           [% javascript %]
         [% END %]
-    // -->
-    </script>
+    [% END %]
+    [% IF inline_javascript.search("\\S") %]
+      <script [% script_nonce FILTER none %]>
+        [% inline_javascript FILTER none %]
+      </script>
+    [% END %]
 
     [% FOREACH asset_url = concatenate_js(javascript_urls) %]
       [% PROCESS format_js_link %]
@@ -347,14 +331,21 @@
               <li id="moz_new_account_container_top"><a href="createaccount.cgi">New&nbsp;Account</a></li>
             [% END %]
 
-            [%# Only display one login form when we're on a LOGIN_REQUIRED page. That
-              # way, we're guaranteed that the user will use the form that has 
-              # hidden_fields in it (the center form) instead of this one. Also, it's
-              # less confusing to have one form (as opposed to  three) when you're 
-              # required to log in.
-              #%]
-            [% IF user.authorizer.can_login && !Bugzilla.page_requires_login %]
-              [% PROCESS "account/auth/login-small.html.tmpl" qs_suffix = "_top" %]
+            [% IF use_login_page %]
+              <li>
+                <span class="separator">| </span>
+                <a href="[% urlbase %]login">Log In</a>
+              </li>
+            [% ELSE %]
+                [%# Only display one login form when we're on a LOGIN_REQUIRED page. That
+                # way, we're guaranteed that the user will use the form that has 
+                # hidden_fields in it (the center form) instead of this one. Also, it's
+                # less confusing to have one form (as opposed to  three) when you're 
+                # required to log in.
+                #%]
+                [% IF user.authorizer.can_login && !Bugzilla.page_requires_login %]
+                  [% PROCESS "account/auth/login-small.html.tmpl" qs_suffix = "_top" %]
+                [% END %]
             [% END %]
           </ul>
         [% END %]
diff --git a/template/en/default/global/per-bug-queries.html.tmpl b/template/en/default/global/per-bug-queries.html.tmpl
index 71723c178..b5c2431e1 100644
--- a/template/en/default/global/per-bug-queries.html.tmpl
+++ b/template/en/default/global/per-bug-queries.html.tmpl
@@ -15,46 +15,6 @@
 
 [% IF user.id && user.settings.per_bug_queries.value == "on" %]
   <li id="links-special">
-    <script [% script_nonce FILTER none %] type="text/javascript">
-      <!--
-      function update_text() {
-        // 'lob' means list_of_bugs.
-        var lob_action = document.getElementById('lob_action');
-        var action = lob_action.options[lob_action.selectedIndex].value;
-        var text = document.getElementById('lob_direction');
-        var new_query_text = document.getElementById('lob_new_query_text');
-
-        if (action == "add") {
-          text.innerHTML = "to";
-          new_query_text.style.display = 'inline';
-        }
-        else {
-          text.innerHTML = "from";
-          new_query_text.style.display = 'none';
-        }
-      }
-
-      function manage_old_lists() {
-        var old_lists = document.getElementById('lob_oldqueryname');
-        // If there is no saved searches available, returns.
-        if (!old_lists) return;
-
-        var new_query = document.getElementById('lob_newqueryname').value;
-
-        if (new_query != "") {
-          old_lists.disabled = true;
-        }
-        else {
-          old_lists.disabled = false;
-        }
-      }
-      $(function() {
-        $("#lob_action").on("change", update_text);
-        $("#lob_newqueryname").on("keyup", manage_old_lists);
-      });
-      //-->
-    </script>
-
     <div class="label"></div>
     <ul class="links"><li class="form">
       <form id="list_of_bugs" action="buglist.cgi" method="get">
diff --git a/template/en/default/index.html.tmpl b/template/en/default/index.html.tmpl
index a3fa0a906..c79be57ca 100644
--- a/template/en/default/index.html.tmpl
+++ b/template/en/default/index.html.tmpl
@@ -30,23 +30,9 @@
    header = "Main Page" 
    header_addl_info = "version " _ (Bugzilla.params.bugzilla_version || constants.BUGZILLA_VERSION)
    style_urls = [ 'skins/standard/index.css' ]
+   no_yui = 1
 %]
 
-<script [% script_nonce FILTER none %] type="text/javascript">
-function checkQuicksearch( form ) {
-  if (form.quicksearch.value == '') {
-    alert('Please enter one or more search terms first.');
-    return false;
-  }
-  return true;
-}
-$(function () {
-    $("#quicksearchForm").on("submit", function (event) {
-        return checkQuicksearch(this);
-    });
-});
-</script>
-
 <div id="page-index">
   <table>
     <tr>