rdbms: make DBMasterPos implement Serializable

ChronologyProtector uses these classes to briefly store positions
and everytime the fields change then errors can happen when old
values are unserialized and used. Use a simple two-element map
format for serialized positions. The fields are recomputed back
from the data map.

Values from before this change will issue the warning
"Erroneous data format for unserializing". To avoid that, bump
the ChronologyProtector key version. Future field changes will
not require this.

This change should be deployed on all wikis at once.

Bug: T187942
Change-Id: I71bbbc9b9d4c7e02ac02f1d8750b70bda08d4db1

diff --git a/includes/libs/rdbms/ChronologyProtector.php b/includes/libs/rdbms/ChronologyProtector.php
index 88e276f..e115286 100644 (file)
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -75,7 +75,7 @@ class ChronologyProtector implements LoggerAwareInterface {
        public function __construct( BagOStuff $store, array $client, $posIndex = null ) {
                $this->store = $store;
                $this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );
        public function __construct( BagOStuff $store, array $client, $posIndex = null ) {
                $this->store = $store;
                $this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );

-               $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v1' );
+               $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v2' );

                $this->waitForPosIndex = $posIndex;
                $this->logger = new NullLogger();
        }
                $this->waitForPosIndex = $posIndex;
                $this->logger = new NullLogger();
        }

diff --git a/includes/libs/rdbms/database/position/DBMasterPos.php b/includes/libs/rdbms/database/position/DBMasterPos.php
index 2f79ea9..28d2a1b 100644 (file)
--- a/includes/libs/rdbms/database/position/DBMasterPos.php
+++ b/includes/libs/rdbms/database/position/DBMasterPos.php
@@ -2,12 +2,14 @@
 
 namespace Wikimedia\Rdbms;
 
 
 namespace Wikimedia\Rdbms;
 

+use Serializable;
+

 /**
  * An object representing a master or replica DB position in a replicated setup.
  *
  * The implementation details of this opaque type are up to the database subclass.
  */
 /**
  * An object representing a master or replica DB position in a replicated setup.
  *
  * The implementation details of this opaque type are up to the database subclass.
  */

-interface DBMasterPos {
+interface DBMasterPos extends Serializable {

        /**
         * @return float UNIX timestamp
         * @since 1.25
        /**
         * @return float UNIX timestamp
         * @since 1.25

diff --git a/includes/libs/rdbms/database/position/MySQLMasterPos.php b/includes/libs/rdbms/database/position/MySQLMasterPos.php
index 2ee9068..cdcb79c 100644 (file)
--- a/includes/libs/rdbms/database/position/MySQLMasterPos.php
+++ b/includes/libs/rdbms/database/position/MySQLMasterPos.php
@@ -3,6 +3,7 @@
 namespace Wikimedia\Rdbms;
 
 use InvalidArgumentException;
 namespace Wikimedia\Rdbms;
 
 use InvalidArgumentException;

+use UnexpectedValueException;

 
 /**
  * DBMasterPos class for MySQL/MariaDB
 
 /**
  * DBMasterPos class for MySQL/MariaDB


@@ -27,6 +28,14 @@ class MySQLMasterPos implements DBMasterPos {
         * @param float $asOfTime UNIX timestamp
         */
        public function __construct( $position, $asOfTime ) {
         * @param float $asOfTime UNIX timestamp
         */
        public function __construct( $position, $asOfTime ) {

+               $this->init( $position, $asOfTime );
+       }
+
+       /**
+        * @param string $position
+        * @param float $asOfTime
+        */
+       protected function init( $position, $asOfTime ) {

                $m = [];
                if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', $position, $m ) ) {
                        $this->binlog = $m[1]; // ideally something like host name
                $m = [];
                if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', $position, $m ) ) {
                        $this->binlog = $m[1]; // ideally something like host name


@@ -34,7 +43,7 @@ class MySQLMasterPos implements DBMasterPos {
                } else {
                        $gtids = array_filter( array_map( 'trim', explode( ',', $position ) ) );
                        foreach ( $gtids as $gtid ) {
                } else {
                        $gtids = array_filter( array_map( 'trim', explode( ',', $position ) ) );
                        foreach ( $gtids as $gtid ) {

-                               if ( !$this->parseGTID( $gtid ) ) {
+                               if ( !self::parseGTID( $gtid ) ) {

                                        throw new InvalidArgumentException( "Invalid GTID '$gtid'." );
                                }
                                $this->gtids[] = $gtid;
                                        throw new InvalidArgumentException( "Invalid GTID '$gtid'." );
                                }
                                $this->gtids[] = $gtid;


@@ -192,4 +201,17 @@ class MySQLMasterPos implements DBMasterPos {
                        ? [ 'binlog' => $this->binlog, 'pos' => $this->pos ]
                        : false;
        }
                        ? [ 'binlog' => $this->binlog, 'pos' => $this->pos ]
                        : false;
        }

+
+       public function serialize() {
+               return serialize( [ 'position' => $this->__toString(), 'asOfTime' => $this->asOfTime ] );
+       }
+
+       public function unserialize( $serialized ) {
+               $data = unserialize( $serialized );
+               if ( !is_array( $data ) ) {
+                       throw new UnexpectedValueException( __METHOD__ . ": cannot unserialize position" );
+               }
+
+               $this->init( $data['position'], $data['asOfTime'] );
+       }

 }
 }

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
index b9f57b5..5fcca1a 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
@@ -495,4 +495,19 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase {
 
                $db->clearFlag( Database::DBO_IGNORE );
        }
 
                $db->clearFlag( Database::DBO_IGNORE );
        }

+
+       /**
+        * @covers Wikimedia\Rdbms\MySQLMasterPos
+        */
+       public function testSerialize() {
+               $pos = new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:99', 53636363 );
+               $roundtripPos = unserialize( serialize( $pos ) );
+
+               $this->assertEquals( $pos, $roundtripPos );
+
+               $pos = new MySQLMasterPos( '255-11-23', 53636363 );
+               $roundtripPos = unserialize( serialize( $pos ) );
+
+               $this->assertEquals( $pos, $roundtripPos );
+       }

 }
 }