Introduce SqlBlobStore::makeAddressFromTextId

This introduces a convenience method for constructing a blob address
from a text ID. It's the inverse of SqlBlobStore::getTextIdFromAddress

Change-Id: I31b3ee5e40185c754fb2c119eb5edc50b903f5dc

diff --git a/includes/Revision.php b/includes/Revision.php
index 22eb115..c948d9e 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1069,7 +1069,9 @@ class Revision implements IDBAccessObject {
                        return false;
                }
 
-               $cacheKey = isset( $row->old_id ) ? ( 'tt:' . $row->old_id ) : null;
+               $cacheKey = isset( $row->old_id )
+                       ? SqlBlobStore::makeAddressFromTextId( $row->old_id )
+                       : null;
 
                return self::getBlobStore( $wiki )->expandBlob( $text, $flags, $cacheKey );
        }

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 584142b..dc363d2 100644 (file)
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -391,7 +391,7 @@ class RevisionStore
 
                // getTextIdFromAddress() is free to insert something into the text table, so $textId
                // may be a new value, not anything already contained in $blobAddress.
-               $blobAddress = 'tt:' . $textId;
+               $blobAddress = SqlBlobStore::makeAddressFromTextId( $textId );
 
                $comment = $this->failOnNull( $rev->getComment( RevisionRecord::RAW ), 'comment' );
                $user = $this->failOnNull( $rev->getUser( RevisionRecord::RAW ), 'user' );
@@ -769,7 +769,9 @@ class RevisionStore
 
                        if ( isset( $row->rev_text_id ) && $row->rev_text_id > 0 ) {
                                $mainSlotRow->slot_content_id = $row->rev_text_id;
-                               $mainSlotRow->content_address = 'tt:' . $row->rev_text_id;
+                               $mainSlotRow->content_address = SqlBlobStore::makeAddressFromTextId(
+                                       $row->rev_text_id
+                               );
                        }
 
                        // This is used by null-revisions
@@ -808,7 +810,7 @@ class RevisionStore
                                ? intval( $row['slot_origin'] )
                                : null;
                        $mainSlotRow->content_address = isset( $row['text_id'] )
-                               ? 'tt:' . intval( $row['text_id'] )
+                               ? SqlBlobStore::makeAddressFromTextId( intval( $row['text_id'] ) )
                                : null;
                        $mainSlotRow->content_size = isset( $row['len'] ) ? intval( $row['len'] ) : null;
                        $mainSlotRow->content_sha1 = isset( $row['sha1'] ) ? strval( $row['sha1'] ) : null;

diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php
index 0ff7c13..70d7d42 100644 (file)
--- a/includes/Storage/SqlBlobStore.php
+++ b/includes/Storage/SqlBlobStore.php
@@ -244,7 +244,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 
                        $textId = $dbw->insertId();
 
-                       return 'tt:' . $textId;
+                       return self::makeAddressFromTextId( $textId );
                } catch ( MWException $e ) {
                        throw new BlobAccessException( $e->getMessage(), 0, $e );
                }
@@ -542,11 +542,12 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         * Currently, $address must start with 'tt:' followed by a decimal integer representing
         * the old_id; if $address does not start with 'tt:', null is returned. However,
         * the implementation may change to insert rows into the text table on the fly.
+        * This implies that this method cannot be static.
         *
         * @note This method exists for use with the text table based storage schema.
         * It should not be assumed that is will function with all future kinds of content addresses.
         *
-        * @deprecated since 1.31, so not assume that all blob addresses refer to a row in the text
+        * @deprecated since 1.31, so don't assume that all blob addresses refer to a row in the text
         * table. This method should become private once the relevant refactoring in WikiPage is
         * complete.
         *
@@ -570,6 +571,22 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                return $textId;
        }
 
+       /**
+        * Returns an address referring to content stored in the text table row with the given ID.
+        * The address schema for blobs stored in the text table is "tt:" followed by an integer
+        * that corresponds to a value of the old_id field.
+        *
+        * @deprecated since 1.31. This method should become private once the relevant refactoring
+        * in WikiPage is complete.
+        *
+        * @param int $id
+        *
+        * @return string
+        */
+       public static function makeAddressFromTextId( $id ) {
+               return 'tt:' . $id;
+       }
+
        /**
         * Splits a blob address into three parts: the schema, the ID, and parameters/flags.
         *

diff --git a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
index dbbef11..07f1f82 100644 (file)
--- a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
+++ b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
@@ -2,6 +2,7 @@
 
 namespace MediaWiki\Tests\Storage;
 
+use InvalidArgumentException;
 use Language;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\SqlBlobStore;
@@ -238,4 +239,40 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
                $this->assertSame( $blob, $store->getBlob( $address ) );
        }
 
+       public function provideGetTextIdFromAddress() {
+               yield [ 'tt:17', 17 ];
+               yield [ 'xy:17', null ];
+               yield [ 'xy:xyzzy', null ];
+       }
+
+       /**
+        * @dataProvider provideGetTextIdFromAddress
+        */
+       public function testGetTextIdFromAddress( $address, $textId ) {
+               $store = $this->getBlobStore();
+               $this->assertSame( $textId, $store->getTextIdFromAddress( $address ) );
+       }
+
+       public function provideGetTextIdFromAddressInvalidArgumentException() {
+               yield [ 'tt:-17' ];
+               yield [ 'tt:xy' ];
+               yield [ 'tt:0' ];
+               yield [ 'tt:' ];
+               yield [ 'xy' ];
+               yield [ '' ];
+       }
+
+       /**
+        * @dataProvider provideGetTextIdFromAddressInvalidArgumentException
+        */
+       public function testGetTextIdFromAddressInvalidArgumentException( $address ) {
+               $this->setExpectedException( InvalidArgumentException::class );
+               $store = $this->getBlobStore();
+               $store->getTextIdFromAddress( $address );
+       }
+
+       public function testMakeAddressFromTextId() {
+               $this->assertSame( 'tt:17', SqlBlobStore::makeAddressFromTextId( 17 ) );
+       }
+
 }