From: Timo Tijhof <krinklemail@gmail.com>
Date: Sat, 31 Aug 2019 22:05:50 +0000 (+0100)
Subject: Setup: Move wgActionPath logic to PathRouter
X-Git-Tag: 1.34.0-rc.0~371^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=64281b6c524f54863174768c025dfe698948fc3f;p=lhc%2Fweb%2Fwiklou.git

Setup: Move wgActionPath logic to PathRouter

This is only relevant when processing page views or when constructing
Title urls with an 'action' query. Pretty important stuff, and worth
optimising for if we had to choose, but we can defer it in this case
without slowing it down, which is better for everything else.

It also means we don't mutate configuration (beyond setting whole values
as dynamic defaults), which seems desirable, and makes the overall behaviour
easier to test. Handling absence of 'view' should be PathRouter's
responsibility, not Setup.

Bug: T189966
Change-Id: I9c1eea2dcea74be0e283eb2b175268315ced1793
---

diff --git a/includes/PathRouter.php b/includes/PathRouter.php
index 2882e6632e..4d7bd38149 100644
--- a/includes/PathRouter.php
+++ b/includes/PathRouter.php
@@ -401,4 +401,22 @@ class PathRouter {
 
 		return $value;
 	}
+
+	/**
+	 * @internal For use by Title and WebRequest only.
+	 * @param array $actionPaths
+	 * @param string $articlePath
+	 * @return string[]|false
+	 */
+	public static function getActionPaths( array $actionPaths, $articlePath ) {
+		if ( !$actionPaths ) {
+			return false;
+		}
+		// Processing of urls for this feature requires that 'view' is set.
+		// By default, set it to the pretty article path.
+		if ( !isset( $actionPaths['view'] ) ) {
+			$actionPaths['view'] = $articlePath;
+		}
+		return $actionPaths;
+	}
 }
diff --git a/includes/Setup.php b/includes/Setup.php
index cc9a3f9b0b..b5540d4477 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -156,12 +156,6 @@ if ( $wgArticlePath === false ) {
 	}
 }
 
-if ( !empty( $wgActionPaths ) && !isset( $wgActionPaths['view'] ) ) {
-	// 'view' is assumed the default action path everywhere in the code
-	// but is rarely filled in $wgActionPaths
-	$wgActionPaths['view'] = $wgArticlePath;
-}
-
 if ( $wgResourceBasePath === null ) {
 	$wgResourceBasePath = $wgScriptPath;
 }
diff --git a/includes/Title.php b/includes/Title.php
index 547b28c0c3..b64144855e 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -2068,16 +2068,18 @@ class Title implements LinkTarget, IDBAccessObject {
 				$url = false;
 				$matches = [];
 
-				if ( !empty( $wgActionPaths )
+				$articlePaths = PathRouter::getActionPaths( $wgActionPaths, $wgArticlePath );
+
+				if ( $articlePaths
 					&& preg_match( '/^(.*&|)action=([^&]*)(&(.*)|)$/', $query, $matches )
 				) {
 					$action = urldecode( $matches[2] );
-					if ( isset( $wgActionPaths[$action] ) ) {
+					if ( isset( $articlePaths[$action] ) ) {
 						$query = $matches[1];
 						if ( isset( $matches[4] ) ) {
 							$query .= $matches[4];
 						}
-						$url = str_replace( '$1', $dbkey, $wgActionPaths[$action] );
+						$url = str_replace( '$1', $dbkey, $articlePaths[$action] );
 						if ( $query != '' ) {
 							$url = wfAppendQuery( $url, $query );
 						}
diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 9b8f5a69f5..7b14667d5a 100644
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -162,8 +162,9 @@ class WebRequest {
 			}
 
 			global $wgActionPaths;
-			if ( $wgActionPaths ) {
-				$router->add( $wgActionPaths, [ 'action' => '$key' ] );
+			$articlePaths = PathRouter::getActionPaths( $wgActionPaths, $wgArticlePath );
+			if ( $articlePaths ) {
+				$router->add( $articlePaths, [ 'action' => '$key' ] );
 			}
 
 			global $wgVariantArticlePath;