Introduce Shell\CommandFactory

Bug: T177038
Change-Id: Id875e68ea1fa72b44a463f977ab52270fe1e7088

diff --git a/autoload.php b/autoload.php
index 83f2519..cf4a115 100644 (file)
--- a/autoload.php
+++ b/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',

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 84fc959..0d010b4 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/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

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index d048007..75ce8ec 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/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

diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index a16f4af..bd44ef8 100644 (file)
--- a/includes/shell/Command.php
+++ b/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; " .

diff --git a/includes/shell/CommandFactory.php b/includes/shell/CommandFactory.php
new file mode 100644 (file)
index 0000000..c0b8f89
--- /dev/null
+++ b/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 );
+       }
+}

diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php
index e21d762..a660a22 100644 (file)
--- a/includes/shell/Shell.php
+++ b/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 );
        }

diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php
index f6bc209..a5c4688 100644 (file)
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/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 ],
                ];
        }
 

diff --git a/tests/phpunit/includes/shell/CommandFactoryTest.php b/tests/phpunit/includes/shell/CommandFactoryTest.php
new file mode 100644 (file)
index 0000000..aacfd43
--- /dev/null
+++ b/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 );
+       }
+}