From 572ffdf25aed7c23a15e75c4eb828ced379abb23 Mon Sep 17 00:00:00 2001
From: Piotr Miazga <piotr@polishdeveloper.pl>
Date: Wed, 10 Apr 2019 14:52:34 +0200
Subject: [PATCH] Use MediaWikiServices to cache commonly used helpers

SkinMinerva cached the ContentHandler object for better performance.
In the future the ContentHandler will be also used in the Menu,
for better readability, store ContentHandler as Service.

MediaWikiServices will initialize service on first access and cache
it for future needs. Same applies to SkinUserPageHelper,

Bug: T216152
Change-Id: Ia98dc860862360a68556272714669f0c3a13eb1e
---
 includes/ServiceWiring.php                    | 13 +++++++++
 includes/skins/SkinMinerva.php                | 27 +++----------------
 skin.json                                     |  3 +++
 .../skins/SkinMinervaPageActionsTest.php      | 18 +++++--------
 4 files changed, 26 insertions(+), 35 deletions(-)
 create mode 100644 includes/ServiceWiring.php

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
new file mode 100644
index 0000000..8253fcb
--- /dev/null
+++ b/includes/ServiceWiring.php
@@ -0,0 +1,13 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+use MediaWiki\Minerva\SkinUserPageHelper;
+
+return [
+	'Minerva.ContentHandler' => function ( MediaWikiServices $services ) {
+		return ContentHandler::getForTitle( RequestContext::getMain()->getTitle() );
+	},
+	'Minerva.SkinUserPageHelper' => function ( MediaWikiServices $services ) {
+		return new SkinUserPageHelper( RequestContext::getMain()->getTitle() );
+	}
+];
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 268cbc5..9259d6d 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -20,7 +20,6 @@
 
 use MediaWiki\Minerva\MenuBuilder;
 use MediaWiki\MediaWikiServices;
-use MediaWiki\Minerva\SkinUserPageHelper;
 
 /**
  * Minerva: Born from the godhead of Jupiter with weapons!
@@ -48,12 +47,9 @@ class SkinMinerva extends SkinTemplate {
 	public $skinname = 'minerva';
 	/** @var string $template Name of this used template */
 	public $template = 'MinervaTemplate';
-	/** @var ContentHandler Content handler of page; only access through getContentHandler */
-	protected $contentHandler = null;
+
 	/** @var bool Whether the page is also available in other languages or variants */
 	protected $doesPageHaveLanguages = false;
-	/** @var SkinUserPageHelper Helper class for UserPage handling */
-	protected $userPageHelper;
 
 	/**
 	 * Returns the site name for the footer, either as a text or <img> tag
@@ -310,19 +306,6 @@ class SkinMinerva extends SkinTemplate {
 		return '';
 	}
 
-	/**
-	 * Gets content handler of current title
-	 *
-	 * @return ContentHandler
-	 */
-	protected function getContentHandler() {
-		if ( $this->contentHandler === null ) {
-			$this->contentHandler = ContentHandler::getForTitle( $this->getTitle() );
-		}
-
-		return $this->contentHandler;
-	}
-
 	/**
 	 * Takes a title and returns classes to apply to the body tag
 	 * @param Title $title
@@ -378,10 +361,7 @@ class SkinMinerva extends SkinTemplate {
 	 * @return SkinUserPageHelper
 	 */
 	public function getUserPageHelper() {
-		if ( $this->userPageHelper === null ) {
-			$this->userPageHelper = new SkinUserPageHelper( $this->getContext()->getTitle() );
-		}
-		return $this->userPageHelper;
+		return MediaWikiServices::getInstance()->getService( 'Minerva.SkinUserPageHelper' );
 	}
 
 	/**
@@ -1296,7 +1276,8 @@ class SkinMinerva extends SkinTemplate {
 	 * @return bool
 	 */
 	protected function isCurrentPageContentModelEditable() {
-		$contentHandler = $this->getContentHandler();
+		$contentHandler = MediaWikiServices::getInstance()
+			->getService( 'Minerva.ContentHandler' );
 
 		return $contentHandler->supportsDirectEditing()
 			&& $contentHandler->supportsDirectApiEditing();
diff --git a/skin.json b/skin.json
index 38a0c2f..b9a9810 100644
--- a/skin.json
+++ b/skin.json
@@ -548,5 +548,8 @@
 			]
 		}
 	},
+	"ServiceWiringFiles": [
+		"includes/ServiceWiring.php"
+	],
 	"manifest_version": 1
 }
diff --git a/tests/phpunit/skins/SkinMinervaPageActionsTest.php b/tests/phpunit/skins/SkinMinervaPageActionsTest.php
index 6dcca37..41f175c 100644
--- a/tests/phpunit/skins/SkinMinervaPageActionsTest.php
+++ b/tests/phpunit/skins/SkinMinervaPageActionsTest.php
@@ -6,28 +6,21 @@ use SkinMinerva;
 use MediaWikiTestCase;
 use Title;
 use RequestContext;
-use ContentHandler;
 use MediaWiki\Minerva\SkinUserPageHelper;
 
 // FIXME: That this class exists is an indicator that at least SkinMinerva#isAllowedPageAction
 // should be extracted from SkinMinerva.
 // phpcs:ignore MediaWiki.Files.ClassMatchesFilename.NotMatch
 class TestSkinMinerva extends SkinMinerva {
+
 	public function isAllowedPageAction( $action ) {
 		return parent::isAllowedPageAction( $action );
 	}
 
-	public function setContentHandler( ContentHandler $contentHandler ) {
-		$this->contentHandler = $contentHandler;
-	}
-
 	public function setDoesPageHaveLanguages( $doesPageHaveLanguages ) {
 		$this->doesPageHaveLanguages = $doesPageHaveLanguages;
 	}
 
-	public function overWriteUserPageHelper( $helper ) {
-		$this->userPageHelper = $helper;
-	}
 }
 
 /**
@@ -51,7 +44,7 @@ class SkinMinervaPageActionsTest extends MediaWikiTestCase {
 	 * @return TestSkinMinerva
 	 */
 	private function getSkin( Title $title ) {
-		$requestContext = new RequestContext();
+		$requestContext = RequestContext::getMain();
 		$requestContext->setTitle( $title );
 
 		$result = new TestSkinMinerva();
@@ -122,7 +115,7 @@ class SkinMinervaPageActionsTest extends MediaWikiTestCase {
 		$contentHandler->method( 'supportsDirectApiEditing' )
 			->will( $this->returnValue( $supportsDirectApiEditing ) );
 
-		$this->skin->setContentHandler( $contentHandler );
+		$this->setService( 'Minerva.ContentHandler', $contentHandler );
 
 		$this->assertEquals( $expected, $this->skin->isAllowedPageAction( 'edit' ) );
 	}
@@ -140,7 +133,8 @@ class SkinMinervaPageActionsTest extends MediaWikiTestCase {
 			->willReturn( true );
 
 		$skin = $this->getSkin( Title::newFromText( 'User:Admin' ) );
-		$skin->overWriteUserPageHelper( $userPageHelper );
+
+		$this->setService( 'Minerva.SkinUserPageHelper', $userPageHelper );
 
 		$this->assertFalse( $skin->isAllowedPageAction( 'talk' ) );
 	}
@@ -158,7 +152,7 @@ class SkinMinervaPageActionsTest extends MediaWikiTestCase {
 			->willReturn( false );
 
 		$skin = $this->getSkin( Title::newFromText( 'User:Admin' ) );
-		$skin->overWriteUserPageHelper( $userPageHelper );
+		$this->setService( 'Minerva.SkinUserPageHelper', $userPageHelper );
 
 		$this->assertTrue( $skin->isAllowedPageAction( 'talk' ) );
 	}