Merge "rdbms: Increase coverage for Database::selectSQLText()"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 25 Jul 2017 10:20:35 +0000 (10:20 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 25 Jul 2017 10:20:35 +0000 (10:20 +0000)
includes/config/EtcdConfig.php
includes/libs/objectcache/MultiWriteBagOStuff.php
includes/page/WikiPage.php
tests/phpunit/includes/config/EtcdConfigTest.php
tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php

index c57eba7..6605c38 100644 (file)
@@ -243,7 +243,7 @@ class EtcdConfig implements Config, LoggerAwareInterface {
 
                $info = json_decode( $rbody, true );
                if ( $info === null || !isset( $info['node']['nodes'] ) ) {
-                       return [ null, $rcode, "Unexpected JSON response; missing 'nodes' list.", false ];
+                       return [ null, "Unexpected JSON response; missing 'nodes' list.", false ];
                }
 
                $config = [];
index 687c67c..d94578d 100644 (file)
@@ -181,6 +181,12 @@ class MultiWriteBagOStuff extends BagOStuff {
                $ret = true;
                $args = array_slice( func_get_args(), 3 );
 
+               if ( $count > 1 && $asyncWrites ) {
+                       // Deep-clone $args to prevent misbehavior when something writes an
+                       // object to the BagOStuff then modifies it afterwards, e.g. T168040.
+                       $args = unserialize( serialize( $args ) );
+               }
+
                foreach ( $this->caches as $i => $cache ) {
                        if ( $i >= $count ) {
                                break; // ignore the lower tiers
index 9168482..c05ba45 100644 (file)
@@ -50,7 +50,7 @@ class WikiPage implements Page, IDBAccessObject {
        public $mLatest = false;             // !< Integer (false means "not loaded")
        /**@}}*/
 
-       /** @var stdClass Map of cache fields (text, parser output, ect) for a proposed/new edit */
+       /** @var PreparedEdit Map of cache fields (text, parser output, ect) for a proposed/new edit */
        public $mPreparedEdit = false;
 
        /**
@@ -782,7 +782,7 @@ class WikiPage implements Page, IDBAccessObject {
         * Determine whether a page would be suitable for being counted as an
         * article in the site_stats table based on the title & its content
         *
-        * @param object|bool $editInfo (false): object returned by prepareTextForEdit(),
+        * @param PreparedEdit|bool $editInfo (false): object returned by prepareTextForEdit(),
         *   if false, the current database state will be used
         * @return bool
         */
@@ -1607,7 +1607,7 @@ class WikiPage implements Page, IDBAccessObject {
                $meta = [
                        'bot' => ( $flags & EDIT_FORCE_BOT ),
                        'minor' => ( $flags & EDIT_MINOR ) && $user->isAllowed( 'minoredit' ),
-                       'serialized' => $editInfo->pst,
+                       'serialized' => $pstContent->serialize( $serialFormat ),
                        'serialFormat' => $serialFormat,
                        'baseRevId' => $baseRevId,
                        'oldRevision' => $old_revision,
@@ -1961,7 +1961,9 @@ class WikiPage implements Page, IDBAccessObject {
 
        /**
         * Prepare content which is about to be saved.
-        * Returns a stdClass with source, pst and output members
+        *
+        * Prior to 1.30, this returned a stdClass object with the same class
+        * members.
         *
         * @param Content $content
         * @param Revision|int|null $revision Revision object. For backwards compatibility, a
index 8e57a01..19cffa2 100644 (file)
@@ -364,22 +364,125 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
 
        public static function provideFetchFromServer() {
                return [
-                       [
+                       '200 OK - Success' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'node' => [ 'nodes' => [
+                                               [
+                                                       'key' => '/example/foo',
+                                                       'value' => json_encode( [ 'val' => true ] )
+                                               ],
+                                       ] ] ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       [ 'foo' => true ], // data
+                                       null,
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Skip dir' => [
                                'http' => [
                                        'code' => 200,
                                        'reason' => 'OK',
-                                       'headers' => [
-                                               'content-length' => 0,
-                                       ],
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'node' => [ 'nodes' => [
+                                               [
+                                                       'key' => '/example/foo',
+                                                       'value' => json_encode( [ 'val' => true ] )
+                                               ],
+                                               [
+                                                       'key' => '/example/sub',
+                                                       'dir' => true
+                                               ],
+                                               [
+                                                       'key' => '/example/bar',
+                                                       'value' => json_encode( [ 'val' => false ] )
+                                               ],
+                                       ] ] ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       [ 'foo' => true, 'bar' => false ], // data
+                                       null,
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Bad value' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => json_encode( [ 'node' => [ 'nodes' => [
+                                               [
+                                                       'key' => '/example/foo',
+                                                       'value' => ';"broken{value'
+                                               ]
+                                       ] ] ] ),
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       null, // data
+                                       "Failed to parse value for 'foo'.",
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Empty node list' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [],
+                                       'body' => '{"node":{"nodes":[]}}',
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       [], // data
+                                       null,
+                                       false // retry
+                               ],
+                       ],
+                       '200 OK - Invalid JSON' => [
+                               'http' => [
+                                       'code' => 200,
+                                       'reason' => 'OK',
+                                       'headers' => [ 'content-length' => 0 ],
                                        'body' => '',
                                        'error' => '(curl error: no status set)',
                                ],
                                'expect' => [
-                                       // FIXME: Returning 4 values instead of 3
-                                       null,
-                                       200,
+                                       null, // data
                                        "Unexpected JSON response; missing 'nodes' list.",
-                                       false
+                                       false // retry
+                               ],
+                       ],
+                       '404 Not Found' => [
+                               'http' => [
+                                       'code' => 404,
+                                       'reason' => 'Not Found',
+                                       'headers' => [ 'content-length' => 0 ],
+                                       'body' => '',
+                                       'error' => '',
+                               ],
+                               'expect' => [
+                                       null, // data
+                                       'HTTP 404 (Not Found)',
+                                       false // retry
+                               ],
+                       ],
+                       '400 Bad Request - custom error' => [
+                               'http' => [
+                                       'code' => 400,
+                                       'reason' => 'Bad Request',
+                                       'headers' => [ 'content-length' => 0 ],
+                                       'body' => '',
+                                       'error' => 'No good reason',
+                               ],
+                               'expect' => [
+                                       null, // data
+                                       'No good reason',
+                                       true // retry
                                ],
                        ],
                ];
@@ -387,6 +490,7 @@ class EtcConfigTest extends PHPUnit_Framework_TestCase {
 
        /**
         * @covers EtcdConfig::fetchAllFromEtcdServer
+        * @covers EtcdConfig::unserialize
         * @dataProvider provideFetchFromServer
         */
        public function testFetchFromServer( array $httpResponse, array $expected ) {
index 775709f..4a9f6cc 100644 (file)
@@ -81,22 +81,26 @@ class MultiWriteBagOStuffTest extends MediaWikiTestCase {
         */
        public function testSetDelayed() {
                $key = wfRandomString();
-               $value = wfRandomString();
+               $value = (object)[ 'v' => wfRandomString() ];
+               $expectValue = clone $value;
 
                // XXX: DeferredUpdates bound to transactions in CLI mode
                $dbw = wfGetDB( DB_MASTER );
                $dbw->begin();
                $this->cache->set( $key, $value );
 
+               // Test that later changes to $value don't affect the saved value (e.g. T168040)
+               $value->v = 'bogus';
+
                // Set in tier 1
-               $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' );
+               $this->assertEquals( $expectValue, $this->cache1->get( $key ), 'Written to tier 1' );
                // Not yet set in tier 2
                $this->assertEquals( false, $this->cache2->get( $key ), 'Not written to tier 2' );
 
                $dbw->commit();
 
                // Set in tier 2
-               $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' );
+               $this->assertEquals( $expectValue, $this->cache2->get( $key ), 'Written to tier 2' );
        }
 
        /**