Move HTTP 304 check from performRequest to ViewAction
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 25 Aug 2016 00:19:58 +0000 (17:19 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 29 Aug 2016 21:23:16 +0000 (14:23 -0700)
* Follow-up to 8b141886edebc
* The method is now called after the setCdnMaxage() call
  in performAction.
* Allow any CDN urls for the title now, check $wgDebugToolbar,
  and allows caching redirects. The multi-step redirect case does
  not cache however, for simplicity.
* Removed now-unused code in Article that calculated $timestamp.

Change-Id: Ic4f4e3a79d7d386c2f15ca5b11dddf5c57ff9e9f

includes/MediaWiki.php
includes/actions/ViewAction.php
includes/page/Article.php

index 2a00900..77ac76a 100644 (file)
@@ -286,16 +286,6 @@ class MediaWiki {
                                // may still be a wikipage redirect to another article or URL.
                                $article = $this->initializeArticle();
                                if ( is_object( $article ) ) {
-                                       $url = $request->getFullRequestURL(); // requested URL
-                                       if (
-                                               $request->getMethod() === 'GET' &&
-                                               $url === $article->getTitle()->getCanonicalURL() &&
-                                               $article->checkTouched() &&
-                                               $output->checkLastModified( $article->getTouched() )
-                                       ) {
-                                               wfDebug( __METHOD__ . ": done 304\n" );
-                                               return;
-                                       }
                                        $this->performAction( $article, $requestTitle );
                                } elseif ( is_string( $article ) ) {
                                        $output->redirect( $article );
index 3a24565..55507f5 100644 (file)
@@ -41,6 +41,30 @@ class ViewAction extends FormlessAction {
        }
 
        public function show() {
+               $config = $this->context->getConfig();
+
+               if (
+                       $config->get( 'DebugToolbar' ) == false && // don't let this get stuck on pages
+                       $this->page->checkTouched() // page exists and is not a redirect
+               ) {
+                       // Include any redirect in the last-modified calculation
+                       $redirFromTitle = $this->page->getRedirectedFrom();
+                       if ( !$redirFromTitle ) {
+                               $touched = $this->page->getTouched();
+                       } elseif ( $config->get( 'MaxRedirects' ) <= 1 ) {
+                               $touched = max( $this->page->getTouched(), $redirFromTitle->getTouched() );
+                       } else {
+                               // Don't bother following the chain and getting the max mtime
+                               $touched = null;
+                       }
+
+                       // Send HTTP 304 if the IMS matches or otherwise set expiry/last-modified headers
+                       if ( $touched && $this->getOutput()->checkLastModified( $touched ) ) {
+                               wfDebug( __METHOD__ . ": done 304\n" );
+                               return;
+                       }
+               }
+
                $this->page->view();
        }
 }
index b3a97f7..e299f7e 100644 (file)
@@ -149,6 +149,15 @@ class Article implements Page {
                return $article;
        }
 
+       /**
+        * Get the page this view was redirected from
+        * @return Title|null
+        * @since 1.28
+        */
+       public function getRedirectedFrom() {
+               return $this->mRedirectedFrom;
+       }
+
        /**
         * Tell the page view functions that this view was redirected
         * from another page on the wiki.
@@ -467,7 +476,7 @@ class Article implements Page {
         * page of the given title.
         */
        public function view() {
-               global $wgUseFileCache, $wgDebugToolbar, $wgMaxRedirects;
+               global $wgUseFileCache, $wgDebugToolbar;
 
                # Get variables from query string
                # As side effect this will load the revision and update the title
@@ -520,29 +529,6 @@ class Article implements Page {
 
                # Try client and file cache
                if ( !$wgDebugToolbar && $oldid === 0 && $this->mPage->checkTouched() ) {
-                       # Use the greatest of the page's timestamp or the timestamp of any
-                       # redirect in the chain (bug 67849)
-                       $timestamp = $this->mPage->getTouched();
-                       if ( isset( $this->mRedirectedFrom ) ) {
-                               $timestamp = max( $timestamp, $this->mRedirectedFrom->getTouched() );
-
-                               # If there can be more than one redirect in the chain, we have
-                               # to go through the whole chain too in case an intermediate
-                               # redirect was changed.
-                               if ( $wgMaxRedirects > 1 ) {
-                                       $titles = Revision::newFromTitle( $this->mRedirectedFrom )
-                                               ->getContent( Revision::FOR_THIS_USER, $user )
-                                               ->getRedirectChain();
-                                       $thisTitle = $this->getTitle();
-                                       foreach ( $titles as $title ) {
-                                               if ( Title::compare( $title, $thisTitle ) === 0 ) {
-                                                       break;
-                                               }
-                                               $timestamp = max( $timestamp, $title->getTouched() );
-                                       }
-                               }
-                       }
-
                        # Try to stream the output from file cache
                        if ( $wgUseFileCache && $this->tryFileCache() ) {
                                wfDebug( __METHOD__ . ": done file cache\n" );