Introduce Shell\CommandFactory

Bug: T177038
Change-Id: Id875e68ea1fa72b44a463f977ab52270fe1e7088

autoload.php

@@ -930,6 +930,7 @@ $wgAutoloadLocalClasses = [
 	'MediaWiki\\Session\\UserInfo' => __DIR__ . '/includes/session/UserInfo.php',
 	'MediaWiki\\ShellDisabledError' => __DIR__ . '/includes/exception/ShellDisabledError.php',
 	'MediaWiki\\Shell\\Command' => __DIR__ . '/includes/shell/Command.php',
+	'MediaWiki\\Shell\\CommandFactory' => __DIR__ . '/includes/shell/CommandFactory.php',
 	'MediaWiki\\Shell\\Result' => __DIR__ . '/includes/shell/Result.php',
 	'MediaWiki\\Shell\\Shell' => __DIR__ . '/includes/shell/Shell.php',
 	'MediaWiki\\Site\\MediaWikiPageNameNormalizer' => __DIR__ . '/includes/site/MediaWikiPageNameNormalizer.php',

includes/MediaWikiServices.php

@@ -10,6 +10,7 @@ use GenderCache;
 use GlobalVarConfig;
 use Hooks;
 use IBufferingStatsdDataFactory;
+use MediaWiki\Shell\CommandFactory;
 use Wikimedia\Rdbms\LBFactory;
 use LinkCache;
 use Wikimedia\Rdbms\LoadBalancer;
@@ -681,6 +682,14 @@ class MediaWikiServices extends ServiceContainer {
 		return $this->getService( 'ReadOnlyMode' );
 	}
 
+	/**
+	 * @since 1.30
+	 * @return CommandFactory
+	 */
+	public function getShellCommandFactory() {
+		return $this->getService( 'ShellCommandFactory' );
+	}
+
 	///////////////////////////////////////////////////////////////////////////
 	// NOTE: When adding a service getter here, don't forget to add a test
 	// case for it in MediaWikiServicesTest::provideGetters() and in

includes/ServiceWiring.php

@@ -41,6 +41,7 @@ use MediaWiki\Interwiki\ClassicInterwikiLookup;
 use MediaWiki\Linker\LinkRendererFactory;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Shell\CommandFactory;
 
 return [
 	'DBLoadBalancerFactory' => function ( MediaWikiServices $services ) {
@@ -428,6 +429,23 @@ return [
 		);
 	},
 
+	'ShellCommandFactory' => function ( MediaWikiServices $services ) {
+		$config = $services->getMainConfig();
+
+		$limits = [
+			'time' => $config->get( 'MaxShellTime' ),
+			'walltime' => $config->get( 'MaxShellWallClockTime' ),
+			'memory' => $config->get( 'MaxShellMemory' ),
+			'filesize' => $config->get( 'MaxShellFileSize' ),
+		];
+		$cgroup = $config->get( 'ShellCgroup' );
+
+		$factory = new CommandFactory( $limits, $cgroup );
+		$factory->setLogger( LoggerFactory::getInstance( 'exec' ) );
+
+		return $factory;
+	},
+
 	///////////////////////////////////////////////////////////////////////////
 	// NOTE: When adding a service here, don't forget to add a getter function
 	// in the MediaWikiServices class. The convenience getter should just call

includes/shell/Command.php

@@ -63,7 +63,7 @@ class Command {
 	private $everExecuted = false;
 
 	/** @var string|false */
-	private $cGroup = false;
+	private $cgroup = false;
 
 	/**
 	 * Constructor. Don't call directly, instead use Shell::command()
@@ -133,7 +133,8 @@ class Command {
 	/**
 	 * Sets execution limits
 	 *
-	 * @param array $limits Optional array with limits(filesize, memory, time, walltime).
+	 * @param array $limits Associative array of limits. Keys (all optional):
+	 *   filesize (for ulimit -f), memory, time, walltime.
 	 * @return $this
 	 */
 	public function limits( array $limits ) {
@@ -187,11 +188,11 @@ class Command {
 	/**
 	 * Sets cgroup for this command
 	 *
-	 * @param string|false $cgroup
+	 * @param string|false $cgroup Absolute file path to the cgroup, or false to not use a cgroup
 	 * @return $this
 	 */
 	public function cgroup( $cgroup ) {
-		$this->cGroup = $cgroup;
+		$this->cgroup = $cgroup;
 
 		return $this;
 	}
@@ -243,7 +244,7 @@ class Command {
 					   escapeshellarg(
 						   "MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' .
 						   "MW_CPU_LIMIT=$time; " .
-						   'MW_CGROUP=' . escapeshellarg( $this->cGroup ) . '; ' .
+						   'MW_CGROUP=' . escapeshellarg( $this->cgroup ) . '; ' .
 						   "MW_MEM_LIMIT=$mem; " .
 						   "MW_FILE_SIZE_LIMIT=$filesize; " .
 						   "MW_WALL_CLOCK_LIMIT=$wallTime; " .

includes/shell/CommandFactory.php

@@ -0,0 +1,65 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Shell;
+
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+
+/**
+ * Factory facilitating dependency injection for Command
+ *
+ * @since 1.30
+ */
+class CommandFactory {
+	use LoggerAwareTrait;
+
+	/** @var array */
+	private $limits;
+
+	/** @var string|bool */
+	private $cgroup;
+
+	/**
+	 * Constructor
+	 *
+	 * @param array $limits See {@see Command::limits()}
+	 * @param string|bool $cgroup See {@see Command::cgroup()}
+	 */
+	public function __construct( array $limits, $cgroup ) {
+		$this->limits = $limits;
+		$this->cgroup = $cgroup;
+		$this->setLogger( new NullLogger() );
+	}
+
+	/**
+	 * Instantiates a new Command
+	 *
+	 * @return Command
+	 */
+	public function create() {
+		$command = new Command();
+		$command->setLogger( $this->logger );
+
+		return $command
+			->limits( $this->limits )
+			->cgroup( $this->cgroup );
+	}
+}

includes/shell/Shell.php

@@ -22,7 +22,6 @@
 
 namespace MediaWiki\Shell;
 
-use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -57,18 +56,9 @@ class Shell {
 			// treat it as a list of arguments
 			$args = reset( $args );
 		}
-		$command = new Command();
-		$config = MediaWikiServices::getInstance()->getMainConfig();
-
-		$limits = [
-			'time' => $config->get( 'MaxShellTime' ),
-			'walltime' => $config->get( 'MaxShellWallClockTime' ),
-			'memory' => $config->get( 'MaxShellMemory' ),
-			'filesize' => $config->get( 'MaxShellFileSize' ),
-		];
-		$command->limits( $limits );
-		$command->cgroup( $config->get( 'ShellCgroup' ) );
-		$command->setLogger( LoggerFactory::getInstance( 'exec' ) );
+		$command = MediaWikiServices::getInstance()
+			->getShellCommandFactory()
+			->create();
 
 		return $command->params( $args );
 	}

tests/phpunit/includes/MediaWikiServicesTest.php

@@ -6,6 +6,7 @@ use MediaWiki\MediaWikiServices;
 use MediaWiki\Services\DestructibleService;
 use MediaWiki\Services\SalvageableService;
 use MediaWiki\Services\ServiceDisabledException;
+use MediaWiki\Shell\CommandFactory;
 
 /**
  * @covers MediaWiki\MediaWikiServices
@@ -328,7 +329,8 @@ class MediaWikiServicesTest extends MediaWikiTestCase {
 			'MainObjectStash' => [ 'MainObjectStash', BagOStuff::class ],
 			'MainWANObjectCache' => [ 'MainWANObjectCache', WANObjectCache::class ],
 			'LocalServerObjectCache' => [ 'LocalServerObjectCache', BagOStuff::class ],
-			'VirtualRESTServiceClient' => [ 'VirtualRESTServiceClient', VirtualRESTServiceClient::class ]
+			'VirtualRESTServiceClient' => [ 'VirtualRESTServiceClient', VirtualRESTServiceClient::class ],
+			'ShellCommandFactory' => [ 'ShellCommandFactory', CommandFactory::class ],
 		];
 	}
 

tests/phpunit/includes/shell/CommandFactoryTest.php

@@ -0,0 +1,33 @@
+<?php
+
+use MediaWiki\Shell\CommandFactory;
+use Psr\Log\NullLogger;
+use Wikimedia\TestingAccessWrapper;
+
+/**
+ * @group Shell
+ */
+class CommandFactoryTest extends PHPUnit_Framework_TestCase {
+	/**
+	 * @covers MediaWiki\Shell\CommandFactory::create
+	 */
+	public function testCreate() {
+		$logger = new NullLogger();
+		$cgroup = '/sys/fs/cgroup/memory/mygroup';
+		$limits = [
+			'filesize' => 1000,
+			'memory' => 1000,
+			'time' => 30,
+			'walltime' => 40,
+		];
+
+		$factory = new CommandFactory( $limits, $cgroup );
+		$factory->setLogger( $logger );
+		$command = $factory->create();
+
+		$wrapper = TestingAccessWrapper::newFromObject( $command );
+		$this->assertSame( $logger, $wrapper->logger );
+		$this->assertSame( $cgroup, $wrapper->cgroup );
+		$this->assertSame( $limits, $wrapper->limits );
+	}
+}