Storage: SqlBlobStore no longer needs Language object

Constructing a Language object in order to initialize the
BlobStoreFactory service causes a circular dependency
(see T231866).

SqlBlobStore was using the Language object to all iconv.
But nothing language specific is done in Language::iconv,
so we can just inline the call.

Bug: T231866
Change-Id: I90c25decbcff10ea762a2c7474a12fd2041b3abc

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index e83f625..626c14f 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -109,8 +109,7 @@ return [
                        $services->getExternalStoreAccess(),
                        $services->getMainWANObjectCache(),
                        new ServiceOptions( BlobStoreFactory::CONSTRUCTOR_OPTIONS,
-                               $services->getMainConfig() ),
-                       $services->getContentLanguage()
+                               $services->getMainConfig() )
                );
        },
 

diff --git a/includes/Storage/BlobStoreFactory.php b/includes/Storage/BlobStoreFactory.php
index c1371c9..7345a62 100644 (file)
--- a/includes/Storage/BlobStoreFactory.php
+++ b/includes/Storage/BlobStoreFactory.php
@@ -20,7 +20,6 @@
 
 namespace MediaWiki\Storage;
 
-use Language;
 use MediaWiki\Config\ServiceOptions;
 use WANObjectCache;
 use Wikimedia\Rdbms\ILBFactory;
@@ -55,11 +54,6 @@ class BlobStoreFactory {
         */
        private $options;
 
-       /**
-        * @var Language
-        */
-       private $contLang;
-
        /**
         * @var array
         * @since 1.34
@@ -75,8 +69,7 @@ class BlobStoreFactory {
                ILBFactory $lbFactory,
                ExternalStoreAccess $extStoreAccess,
                WANObjectCache $cache,
-               ServiceOptions $options,
-               Language $contLang
+               ServiceOptions $options
        ) {
                $options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
 
@@ -84,7 +77,6 @@ class BlobStoreFactory {
                $this->extStoreAccess = $extStoreAccess;
                $this->cache = $cache;
                $this->options = $options;
-               $this->contLang = $contLang;
        }
 
        /**
@@ -119,7 +111,7 @@ class BlobStoreFactory {
                $store->setUseExternalStore( $this->options->get( 'DefaultExternalStore' ) !== false );
 
                if ( $this->options->get( 'LegacyEncoding' ) ) {
-                       $store->setLegacyEncoding( $this->options->get( 'LegacyEncoding' ), $this->contLang );
+                       $store->setLegacyEncoding( $this->options->get( 'LegacyEncoding' ) );
                }
 
                return $store;

diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php
index bcbc9e8..d408edb 100644 (file)
--- a/includes/Storage/SqlBlobStore.php
+++ b/includes/Storage/SqlBlobStore.php
@@ -31,12 +31,12 @@ use DBAccessObjectUtils;
 use IDBAccessObject;
 use IExpiringStore;
 use InvalidArgumentException;
-use Language;
 use MWException;
 use StatusValue;
 use WANObjectCache;
 use ExternalStoreAccess;
 use Wikimedia\Assert\Assert;
+use Wikimedia\AtEase\AtEase;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\ILoadBalancer;
 
@@ -88,11 +88,6 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         */
        private $legacyEncoding = false;
 
-       /**
-        * @var Language|null
-        */
-       private $legacyEncodingConversionLang = null;
-
        /**
         * @var boolean
         */
@@ -160,23 +155,26 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
        }
 
        /**
-        * @return Language|null The locale to use when decoding from a legacy encoding, or null
-        *         if handling of legacy encoding is disabled.
+        * @deprecated since 1.34 No longer needed
+        * @return null
         */
        public function getLegacyEncodingConversionLang() {
-               return $this->legacyEncodingConversionLang;
+               wfDeprecated( __METHOD__ );
+               return null;
        }
 
        /**
+        * Set the legacy encoding to assume for blobs that do not have the utf-8 flag set.
+        *
+        * @note The second parameter, Language $language, was removed in 1.34.
+        *
         * @param string $legacyEncoding The legacy encoding to assume for blobs that are
         *        not marked as utf8.
-        * @param Language $language The locale to use when decoding from a legacy encoding.
         */
-       public function setLegacyEncoding( $legacyEncoding, Language $language ) {
+       public function setLegacyEncoding( $legacyEncoding ) {
                Assert::parameterType( 'string', $legacyEncoding, '$legacyEncoding' );
 
                $this->legacyEncoding = $legacyEncoding;
-               $this->legacyEncodingConversionLang = $language;
        }
 
        /**
@@ -606,14 +604,20 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                }
 
                // Needed to support old revisions left over from from the 1.4 / 1.5 migration.
-               if ( $blob !== false && $this->legacyEncoding && $this->legacyEncodingConversionLang
+               if ( $blob !== false && $this->legacyEncoding
                        && !in_array( 'utf-8', $blobFlags ) && !in_array( 'utf8', $blobFlags )
                ) {
                        # Old revisions kept around in a legacy encoding?
                        # Upconvert on demand.
                        # ("utf8" checked for compatibility with some broken
                        #  conversion scripts 2008-12-30)
-                       $blob = $this->legacyEncodingConversionLang->iconv( $this->legacyEncoding, 'UTF-8', $blob );
+                       # Even with //IGNORE iconv can whine about illegal characters in
+                       # *input* string. We just ignore those too.
+                       # REF: https://bugs.php.net/bug.php?id=37166
+                       # REF: https://phabricator.wikimedia.org/T18885
+                       AtEase::suppressWarnings();
+                       $blob = iconv( $this->legacyEncoding, 'UTF-8//IGNORE', $blob );
+                       AtEase::restoreWarnings();
                }
 
                return $blob;

diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
index 4248650..0eb07a7 100644 (file)
--- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
@@ -9,7 +9,6 @@ use Exception;
 use HashBagOStuff;
 use IDBAccessObject;
 use InvalidArgumentException;
-use Language;
 use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\IncompleteRevisionException;
@@ -1754,7 +1753,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                $lb = $services->getDBLoadBalancer();
                $access = $services->getExternalStoreAccess();
                $blobStore = new SqlBlobStore( $lb, $access, $cache );
-               $blobStore->setLegacyEncoding( 'windows-1252', Language::factory( 'en' ) );
+               $blobStore->setLegacyEncoding( 'windows-1252' );
 
                $factory = $this->getMockBuilder( BlobStoreFactory::class )
                        ->setMethods( [ 'newBlobStore', 'newSqlBlobStore' ] )

diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index 249fa78..bdb34a9 100644 (file)
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -487,7 +487,6 @@ class RevisionTest extends MediaWikiTestCase {
        public function provideGetRevisionTextWithLegacyEncoding() {
                yield 'Utf8Native' => [
                        "Wiki est l'\xc3\xa9cole superieur !",
-                       'fr',
                        'iso-8859-1',
                        (object)[
                                'old_flags' => 'utf-8',
@@ -496,7 +495,6 @@ class RevisionTest extends MediaWikiTestCase {
                ];
                yield 'Utf8Legacy' => [
                        "Wiki est l'\xc3\xa9cole superieur !",
-                       'fr',
                        'iso-8859-1',
                        (object)[
                                'old_flags' => '',
@@ -509,9 +507,9 @@ class RevisionTest extends MediaWikiTestCase {
         * @covers Revision::getRevisionText
         * @dataProvider provideGetRevisionTextWithLegacyEncoding
         */
-       public function testGetRevisionWithLegacyEncoding( $expected, $lang, $encoding, $rowData ) {
+       public function testGetRevisionWithLegacyEncoding( $expected, $encoding, $rowData ) {
                $blobStore = $this->getBlobStore();
-               $blobStore->setLegacyEncoding( $encoding, Language::factory( $lang ) );
+               $blobStore->setLegacyEncoding( $encoding );
                $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) );
 
                $this->testGetRevisionText( $expected, $rowData );
@@ -525,7 +523,6 @@ class RevisionTest extends MediaWikiTestCase {
                 */
                yield 'Utf8NativeGzip' => [
                        "Wiki est l'\xc3\xa9cole superieur !",
-                       'fr',
                        'iso-8859-1',
                        (object)[
                                'old_flags' => 'gzip,utf-8',
@@ -534,7 +531,6 @@ class RevisionTest extends MediaWikiTestCase {
                ];
                yield 'Utf8LegacyGzip' => [
                        "Wiki est l'\xc3\xa9cole superieur !",
-                       'fr',
                        'iso-8859-1',
                        (object)[
                                'old_flags' => 'gzip',
@@ -547,11 +543,11 @@ class RevisionTest extends MediaWikiTestCase {
         * @covers Revision::getRevisionText
         * @dataProvider provideGetRevisionTextWithGzipAndLegacyEncoding
         */
-       public function testGetRevisionWithGzipAndLegacyEncoding( $expected, $lang, $encoding, $rowData ) {
+       public function testGetRevisionWithGzipAndLegacyEncoding( $expected, $encoding, $rowData ) {
                $this->checkPHPExtension( 'zlib' );
 
                $blobStore = $this->getBlobStore();
-               $blobStore->setLegacyEncoding( $encoding, Language::factory( $lang ) );
+               $blobStore->setLegacyEncoding( $encoding );
                $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) );
 
                $this->testGetRevisionText( $expected, $rowData );
@@ -728,7 +724,7 @@ class RevisionTest extends MediaWikiTestCase {
        public function testDecompressRevisionText( $legacyEncoding, $text, $flags, $expected ) {
                $blobStore = $this->getBlobStore();
                if ( $legacyEncoding ) {
-                       $blobStore->setLegacyEncoding( $legacyEncoding, Language::factory( 'en' ) );
+                       $blobStore->setLegacyEncoding( $legacyEncoding );
                }
 
                $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) );

diff --git a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
index 1d88122..9bac308 100644 (file)
--- a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
+++ b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
@@ -3,7 +3,6 @@
 namespace MediaWiki\Tests\Storage;
 
 use InvalidArgumentException;
-use Language;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\BlobAccessException;
 use MediaWiki\Storage\SqlBlobStore;
@@ -33,7 +32,7 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
                        $store->setCompressBlobs( $compressRevisions );
                }
                if ( $legacyEncoding ) {
-                       $store->setLegacyEncoding( $legacyEncoding, Language::factory( 'en' ) );
+                       $store->setLegacyEncoding( $legacyEncoding );
                }
 
                return $store;
@@ -58,11 +57,11 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
        public function testGetSetLegacyEncoding() {
                $store = $this->getBlobStore();
                $this->assertFalse( $store->getLegacyEncoding() );
-               $this->assertNull( $store->getLegacyEncodingConversionLang() );
-               $en = Language::factory( 'en' );
-               $store->setLegacyEncoding( 'foo', $en );
+               $store->setLegacyEncoding( 'foo' );
                $this->assertSame( 'foo', $store->getLegacyEncoding() );
-               $this->assertSame( $en, $store->getLegacyEncodingConversionLang() );
+
+               $this->hideDeprecated( SqlBlobStore::class . '::getLegacyEncodingConversionLang' );
+               $this->assertNull( $store->getLegacyEncodingConversionLang() );
        }
 
        /**