Merge "Fix "succesful" typo"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 14 Jun 2019 00:54:19 +0000 (00:54 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 14 Jun 2019 00:54:19 +0000 (00:54 +0000)
13 files changed:
.travis.yml
includes/Rest/CopyableStreamInterface.php
includes/Rest/Router.php
includes/Rest/StringStream.php
includes/Storage/BlobStoreFactory.php
includes/Storage/DerivedPageDataUpdater.php
includes/Storage/PageUpdater.php
includes/Storage/SqlBlobStore.php
includes/installer/Installer.php
tests/phpunit/MediaWikiTestCase.php
tests/phpunit/includes/Rest/Handler/HelloHandlerTest.php [new file with mode: 0644]
tests/phpunit/includes/Rest/Handler/testRoutes.json [new file with mode: 0644]
tests/phpunit/includes/Rest/HeaderContainerTest.php [new file with mode: 0644]

index ebe1631..bf905e0 100644 (file)
@@ -24,10 +24,10 @@ cache:
 matrix:
   fast_finish: true
   include:
-      php: 7.3
-      php: 7.2
-      php: 7.1
-      php: 7
+    - php: 7.3
+    - php: 7.2
+    - php: 7.1
+    - php: 7
   allow_failures:
     - php: 7.3
 
index d271db3..3e18e16 100644 (file)
@@ -10,6 +10,9 @@ interface CopyableStreamInterface extends \Psr\Http\Message\StreamInterface {
         * Copy this stream to a specified stream resource. For some streams,
         * this can be implemented without a tight loop in PHP code.
         *
+        * Equivalent to reading from the object until EOF and writing the
+        * resulting data to $stream. The position will be advanced to the end.
+        *
         * Note that $stream is not a StreamInterface object.
         *
         * @param resource $stream Destination
index ab54439..83cd0f0 100644 (file)
@@ -181,6 +181,20 @@ class Router {
                return $this->matchers;
        }
 
+       /**
+        * Remove the path prefix $this->rootPath. Return the part of the path with the
+        * prefix removed, or false if the prefix did not match.
+        *
+        * @param string $path
+        * @return false|string
+        */
+       private function getRelativePath( $path ) {
+               if ( substr_compare( $path, $this->rootPath, 0, strlen( $this->rootPath ) ) !== 0 ) {
+                       return false;
+               }
+               return substr( $path, strlen( $this->rootPath ) );
+       }
+
        /**
         * Find the handler for a request and execute it
         *
@@ -188,20 +202,37 @@ class Router {
         * @return ResponseInterface
         */
        public function execute( RequestInterface $request ) {
-               $matchers = $this->getMatchers();
-               $matcher = $matchers[$request->getMethod()] ?? null;
-               if ( $matcher === null ) {
-                       return $this->responseFactory->createHttpError( 404 );
-               }
                $path = $request->getUri()->getPath();
-               if ( substr_compare( $path, $this->rootPath, 0, strlen( $this->rootPath ) ) !== 0 ) {
+               $relPath = $this->getRelativePath( $path );
+               if ( $relPath === false ) {
                        return $this->responseFactory->createHttpError( 404 );
                }
-               $relPath = substr( $path, strlen( $this->rootPath ) );
-               $match = $matcher->match( $relPath );
+
+               $matchers = $this->getMatchers();
+               $matcher = $matchers[$request->getMethod()] ?? null;
+               $match = $matcher ? $matcher->match( $relPath ) : null;
+
                if ( !$match ) {
-                       return $this->responseFactory->createHttpError( 404 );
+                       // Check for 405 wrong method
+                       $allowed = [];
+                       foreach ( $matchers as $allowedMethod => $allowedMatcher ) {
+                               if ( $allowedMethod === $request->getMethod() ) {
+                                       continue;
+                               }
+                               if ( $allowedMatcher->match( $relPath ) ) {
+                                       $allowed[] = $allowedMethod;
+                               }
+                       }
+                       if ( $allowed ) {
+                               $response = $this->responseFactory->createHttpError( 405 );
+                               $response->setHeader( 'Allow', $allowed );
+                               return $response;
+                       } else {
+                               // Did not match with any other method, must be 404
+                               return $this->responseFactory->createHttpError( 404 );
+                       }
                }
+
                $request->setAttributes( $match['params'] );
                $spec = $match['userData'];
                $objectFactorySpec = array_intersect_key( $spec,
index 18fb6b1..10ec42d 100644 (file)
@@ -35,6 +35,7 @@ class StringStream implements CopyableStreamInterface {
                } else {
                        $block = $this->contents;
                }
+               $this->offset = strlen( $this->contents );
                fwrite( $stream, $block );
        }
 
index 5e99454..368ca48 100644 (file)
@@ -23,7 +23,7 @@ namespace MediaWiki\Storage;
 use Language;
 use MediaWiki\Config\ServiceOptions;
 use WANObjectCache;
-use Wikimedia\Rdbms\LBFactory;
+use Wikimedia\Rdbms\ILBFactory;
 
 /**
  * Service for instantiating BlobStores
@@ -35,7 +35,7 @@ use Wikimedia\Rdbms\LBFactory;
 class BlobStoreFactory {
 
        /**
-        * @var LBFactory
+        * @var ILBFactory
         */
        private $lbFactory;
 
@@ -68,7 +68,7 @@ class BlobStoreFactory {
        ];
 
        public function __construct(
-               LBFactory $lbFactory,
+               ILBFactory $lbFactory,
                WANObjectCache $cache,
                ServiceOptions $options,
                Language $contLang
index 53fe615..0008ef7 100644 (file)
@@ -60,7 +60,7 @@ use SiteStatsUpdate;
 use Title;
 use User;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Rdbms\LBFactory;
+use Wikimedia\Rdbms\ILBFactory;
 use WikiPage;
 
 /**
@@ -132,7 +132,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
        private $messageCache;
 
        /**
-        * @var LBFactory
+        * @var ILBFactory
         */
        private $loadbalancerFactory;
 
@@ -268,7 +268,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         * @param JobQueueGroup $jobQueueGroup
         * @param MessageCache $messageCache
         * @param Language $contLang
-        * @param LBFactory $loadbalancerFactory
+        * @param ILBFactory $loadbalancerFactory
         */
        public function __construct(
                WikiPage $wikiPage,
@@ -279,7 +279,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                JobQueueGroup $jobQueueGroup,
                MessageCache $messageCache,
                Language $contLang,
-               LBFactory $loadbalancerFactory
+               ILBFactory $loadbalancerFactory
        ) {
                $this->wikiPage = $wikiPage;
 
index e25f0f0..7246238 100644 (file)
@@ -51,7 +51,7 @@ use Wikimedia\Assert\Assert;
 use Wikimedia\Rdbms\DBConnRef;
 use Wikimedia\Rdbms\DBUnexpectedError;
 use Wikimedia\Rdbms\IDatabase;
-use Wikimedia\Rdbms\LoadBalancer;
+use Wikimedia\Rdbms\ILoadBalancer;
 use WikiPage;
 
 /**
@@ -87,7 +87,7 @@ class PageUpdater {
        private $derivedDataUpdater;
 
        /**
-        * @var LoadBalancer
+        * @var ILoadBalancer
         */
        private $loadBalancer;
 
@@ -151,7 +151,7 @@ class PageUpdater {
         * @param User $user
         * @param WikiPage $wikiPage
         * @param DerivedPageDataUpdater $derivedDataUpdater
-        * @param LoadBalancer $loadBalancer
+        * @param ILoadBalancer $loadBalancer
         * @param RevisionStore $revisionStore
         * @param SlotRoleRegistry $slotRoleRegistry
         */
@@ -159,7 +159,7 @@ class PageUpdater {
                User $user,
                WikiPage $wikiPage,
                DerivedPageDataUpdater $derivedDataUpdater,
-               LoadBalancer $loadBalancer,
+               ILoadBalancer $loadBalancer,
                RevisionStore $revisionStore,
                SlotRoleRegistry $slotRoleRegistry
        ) {
index 467a8ac..e0e14b0 100644 (file)
@@ -36,7 +36,7 @@ use MWException;
 use WANObjectCache;
 use Wikimedia\Assert\Assert;
 use Wikimedia\Rdbms\IDatabase;
-use Wikimedia\Rdbms\LoadBalancer;
+use Wikimedia\Rdbms\ILoadBalancer;
 
 /**
  * Service for storing and loading Content objects.
@@ -52,7 +52,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
        const TEXT_CACHE_GROUP = 'revisiontext:10';
 
        /**
-        * @var LoadBalancer
+        * @var ILoadBalancer
         */
        private $dbLoadBalancer;
 
@@ -92,7 +92,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
        private $useExternalStore = false;
 
        /**
-        * @param LoadBalancer $dbLoadBalancer A load balancer for acquiring database connections
+        * @param ILoadBalancer $dbLoadBalancer A load balancer for acquiring database connections
         * @param WANObjectCache $cache A cache manager for caching blobs. This can be the local
         *        wiki's default instance even if $wikiId refers to a different wiki, since
         *        makeGlobalKey() is used to constructed a key that allows cached blobs from the
@@ -102,7 +102,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         * @param bool|string $wikiId The ID of the target wiki database. Use false for the local wiki.
         */
        public function __construct(
-               LoadBalancer $dbLoadBalancer,
+               ILoadBalancer $dbLoadBalancer,
                WANObjectCache $cache,
                $wikiId = false
        ) {
@@ -186,7 +186,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
        }
 
        /**
-        * @return LoadBalancer
+        * @return ILoadBalancer
         */
        private function getDBLoadBalancer() {
                return $this->dbLoadBalancer;
index 26f9bf0..33d4fcc 100644 (file)
@@ -1803,7 +1803,7 @@ abstract class Installer {
        /**
         * Add an installation step following the given step.
         *
-        * @param callable $callback A valid installation callback array, in this form:
+        * @param array $callback A valid installation callback array, in this form:
         *    [ 'name' => 'some-unique-name', 'callback' => [ $obj, 'function' ] ];
         * @param string $findStep The step to find. Omit to put the step at the beginning
         */
index f9416be..6c8b51f 100644 (file)
@@ -1596,6 +1596,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * Stub. If a test suite needs to test against a specific database schema, it should
         * override this method and return the appropriate information from it.
         *
+        * 'create', 'drop' and 'alter' in the returned array should list all the tables affected
+        * by the 'scripts', even if the test is only interested in a subset of them, otherwise
+        * the overrides may not be fully cleaned up, leading to errors later.
+        *
         * @param IMaintainableDatabase $db The DB connection to use for the mock schema.
         *        May be used to check the current state of the schema, to determine what
         *        overrides are needed.
diff --git a/tests/phpunit/includes/Rest/Handler/HelloHandlerTest.php b/tests/phpunit/includes/Rest/Handler/HelloHandlerTest.php
new file mode 100644 (file)
index 0000000..9719f9e
--- /dev/null
@@ -0,0 +1,81 @@
+<?php
+
+namespace MediaWiki\Tests\Rest\Handler;
+
+use EmptyBagOStuff;
+use GuzzleHttp\Psr7\Uri;
+use MediaWiki\Rest\RequestData;
+use MediaWiki\Rest\ResponseFactory;
+use MediaWiki\Rest\Router;
+use MediaWikiTestCase;
+
+/**
+ * @covers \MediaWiki\Rest\Handler\HelloHandler
+ */
+class HelloHandlerTest extends MediaWikiTestCase {
+       public static function provideTestViaRouter() {
+               return [
+                       'normal' => [
+                               [
+                                       'method' => 'GET',
+                                       'uri' => self::makeUri( '/user/Tim/hello' ),
+                               ],
+                               [
+                                       'statusCode' => 200,
+                                       'reasonPhrase' => 'OK',
+                                       'protocolVersion' => '1.1',
+                                       'body' => '{"message":"Hello, Tim!"}',
+                               ],
+                       ],
+                       'method not allowed' => [
+                               [
+                                       'method' => 'POST',
+                                       'uri' => self::makeUri( '/user/Tim/hello' ),
+                               ],
+                               [
+                                       'statusCode' => 405,
+                                       'reasonPhrase' => 'Method Not Allowed',
+                                       'protocolVersion' => '1.1',
+                                       'body' => '{"httpCode":405,"httpReason":"Method Not Allowed"}',
+                               ],
+                       ],
+               ];
+       }
+
+       private static function makeUri( $path ) {
+               return new Uri( "http://www.example.com/rest$path" );
+       }
+
+       /** @dataProvider provideTestViaRouter */
+       public function testViaRouter( $requestInfo, $responseInfo ) {
+               $router = new Router(
+                       [ __DIR__ . '/testRoutes.json' ],
+                       [],
+                       '/rest',
+                       new EmptyBagOStuff(),
+                       new ResponseFactory() );
+               $request = new RequestData( $requestInfo );
+               $response = $router->execute( $request );
+               if ( isset( $responseInfo['statusCode'] ) ) {
+                       $this->assertSame( $responseInfo['statusCode'], $response->getStatusCode() );
+               }
+               if ( isset( $responseInfo['reasonPhrase'] ) ) {
+                       $this->assertSame( $responseInfo['reasonPhrase'], $response->getReasonPhrase() );
+               }
+               if ( isset( $responseInfo['protocolVersion'] ) ) {
+                       $this->assertSame( $responseInfo['protocolVersion'], $response->getProtocolVersion() );
+               }
+               if ( isset( $responseInfo['body'] ) ) {
+                       $this->assertSame( $responseInfo['body'], $response->getBody()->getContents() );
+               }
+               $this->assertSame(
+                       [],
+                       array_diff( array_keys( $responseInfo ), [
+                               'statusCode',
+                               'reasonPhrase',
+                               'protocolVersion',
+                               'body'
+                       ] ),
+                       '$responseInfo may not contain unknown keys' );
+       }
+}
diff --git a/tests/phpunit/includes/Rest/Handler/testRoutes.json b/tests/phpunit/includes/Rest/Handler/testRoutes.json
new file mode 100644 (file)
index 0000000..6b440f7
--- /dev/null
@@ -0,0 +1,6 @@
+[
+       {
+               "path": "/user/{name}/hello",
+               "class": "MediaWiki\\Rest\\Handler\\HelloHandler"
+       }
+]
diff --git a/tests/phpunit/includes/Rest/HeaderContainerTest.php b/tests/phpunit/includes/Rest/HeaderContainerTest.php
new file mode 100644 (file)
index 0000000..d0fae9f
--- /dev/null
@@ -0,0 +1,171 @@
+<?php
+
+namespace MediaWiki\Tests\Rest;
+
+use MediaWikiTestCase;
+use MediaWiki\Rest\HeaderContainer;
+
+/**
+ * @covers \MediaWiki\Rest\HeaderContainer
+ */
+class HeaderContainerTest extends MediaWikiTestCase {
+       public static function provideSetHeader() {
+               return [
+                       'simple' => [
+                               [
+                                       [ 'Test', 'foo' ]
+                               ],
+                               [ 'Test' => [ 'foo' ] ],
+                               [ 'Test' => 'foo' ]
+                       ],
+                       'replace' => [
+                               [
+                                       [ 'Test', 'foo' ],
+                                       [ 'Test', 'bar' ],
+                               ],
+                               [ 'Test' => [ 'bar' ] ],
+                               [ 'Test' => 'bar' ],
+                       ],
+                       'array value' => [
+                               [
+                                       [ 'Test', [ '1', '2' ] ],
+                                       [ 'Test', [ '3', '4' ] ],
+                               ],
+                               [ 'Test' => [ '3', '4' ] ],
+                               [ 'Test' => '3, 4' ]
+                       ],
+                       'preserve most recent case' => [
+                               [
+                                       [ 'test', 'foo' ],
+                                       [ 'tesT', 'bar' ],
+                               ],
+                               [ 'tesT' => [ 'bar' ] ],
+                               [ 'tesT' => 'bar' ]
+                       ],
+               ];
+       }
+
+       /** @dataProvider provideSetHeader */
+       public function testSetHeader( $setOps, $headers, $lines ) {
+               $hc = new HeaderContainer;
+               foreach ( $setOps as list( $name, $value ) ) {
+                       $hc->setHeader( $name, $value );
+               }
+               $this->assertSame( $headers, $hc->getHeaders() );
+               $this->assertSame( $lines, $hc->getHeaderLines() );
+       }
+
+       public static function provideAddHeader() {
+               return [
+                       'simple' => [
+                               [
+                                       [ 'Test', 'foo' ]
+                               ],
+                               [ 'Test' => [ 'foo' ] ],
+                               [ 'Test' => 'foo' ]
+                       ],
+                       'add' => [
+                               [
+                                       [ 'Test', 'foo' ],
+                                       [ 'Test', 'bar' ],
+                               ],
+                               [ 'Test' => [ 'foo', 'bar' ] ],
+                               [ 'Test' => 'foo, bar' ],
+                       ],
+                       'array value' => [
+                               [
+                                       [ 'Test', [ '1', '2' ] ],
+                                       [ 'Test', [ '3', '4' ] ],
+                               ],
+                               [ 'Test' => [ '1', '2', '3', '4' ] ],
+                               [ 'Test' => '1, 2, 3, 4' ]
+                       ],
+                       'preserve original case' => [
+                               [
+                                       [ 'Test', 'foo' ],
+                                       [ 'tesT', 'bar' ],
+                               ],
+                               [ 'Test' => [ 'foo', 'bar' ] ],
+                               [ 'Test' => 'foo, bar' ]
+                       ],
+               ];
+       }
+
+       /** @dataProvider provideAddHeader */
+       public function testAddHeader( $addOps, $headers, $lines ) {
+               $hc = new HeaderContainer;
+               foreach ( $addOps as list( $name, $value ) ) {
+                       $hc->addHeader( $name, $value );
+               }
+               $this->assertSame( $headers, $hc->getHeaders() );
+               $this->assertSame( $lines, $hc->getHeaderLines() );
+       }
+
+       public static function provideRemoveHeader() {
+               return [
+                       'simple' => [
+                               [ [ 'Test', 'foo' ] ],
+                               [ 'Test' ],
+                               [],
+                               []
+                       ],
+                       'case mismatch' => [
+                               [ [ 'Test', 'foo' ] ],
+                               [ 'tesT' ],
+                               [],
+                               []
+                       ],
+                       'remove nonexistent' => [
+                               [ [ 'A', '1' ] ],
+                               [ 'B' ],
+                               [ 'A' => [ '1' ] ],
+                               [ 'A' => '1' ]
+                       ],
+               ];
+       }
+
+       /** @dataProvider provideRemoveHeader */
+       public function testRemoveHeader( $addOps, $removeOps, $headers, $lines ) {
+               $hc = new HeaderContainer;
+               foreach ( $addOps as list( $name, $value ) ) {
+                       $hc->addHeader( $name, $value );
+               }
+               foreach ( $removeOps as $name ) {
+                       $hc->removeHeader( $name );
+               }
+               $this->assertSame( $headers, $hc->getHeaders() );
+               $this->assertSame( $lines, $hc->getHeaderLines() );
+       }
+
+       public function testHasHeader() {
+               $hc = new HeaderContainer;
+               $hc->addHeader( 'A', '1' );
+               $hc->addHeader( 'B', '2' );
+               $hc->addHeader( 'C', '3' );
+               $hc->removeHeader( 'B' );
+               $hc->removeHeader( 'c' );
+               $this->assertTrue( $hc->hasHeader( 'A' ) );
+               $this->assertTrue( $hc->hasHeader( 'a' ) );
+               $this->assertFalse( $hc->hasHeader( 'B' ) );
+               $this->assertFalse( $hc->hasHeader( 'c' ) );
+               $this->assertFalse( $hc->hasHeader( 'C' ) );
+       }
+
+       public function testGetRawHeaderLines() {
+               $hc = new HeaderContainer;
+               $hc->addHeader( 'A', '1' );
+               $hc->addHeader( 'a', '2' );
+               $hc->addHeader( 'b', '3' );
+               $hc->addHeader( 'Set-Cookie', 'x' );
+               $hc->addHeader( 'SET-cookie', 'y' );
+               $this->assertSame(
+                       [
+                               'A: 1, 2',
+                               'b: 3',
+                               'Set-Cookie: x',
+                               'Set-Cookie: y',
+                       ],
+                       $hc->getRawHeaderLines()
+               );
+       }
+}