Remove ORM use from DBSiteStore

This loses something of the genericity of the former DBSiteStore (i.e.
it's now tied to MediaWiki's database and sites table, and subclasses
and users can't easily override that), but nothing in core or extensions
in Gerrit was using that genericity so it's probably no big loss.

Further, T113034 (an RFC to actually use this 'site' stuff for its
original purpose) proposes getting rid of SiteStore anyway.

Bug: T114538
Change-Id: I7e7ca257451e6307a7e5bb11fd393283d0d19e77

diff --git a/includes/site/DBSiteStore.php b/includes/site/DBSiteStore.php
index 1193bd6..17764a1 100644 (file)
--- a/includes/site/DBSiteStore.php
+++ b/includes/site/DBSiteStore.php
@@ -34,22 +34,16 @@ class DBSiteStore implements SiteStore {
         */
        protected $sites = null;
 
-       /**
-        * @var ORMTable
-        */
-       protected $sitesTable;
-
        /**
         * @since 1.25
-        *
-        * @param ORMTable|null $sitesTable
+        * @param null $sitesTable Unused since 1.27
         */
-       public function __construct( ORMTable $sitesTable = null ) {
-               if ( $sitesTable === null ) {
-                       $sitesTable = $this->newSitesTable();
+       public function __construct( $sitesTable = null ) {
+               if ( $sitesTable !== null ) {
+                       throw new InvalidArgumentException(
+                               __METHOD__ . ': $sitesTable parameter must be null'
+                       );
                }
-
-               $this->sitesTable = $sitesTable;
        }
 
        /**
@@ -65,86 +59,6 @@ class DBSiteStore implements SiteStore {
                return $this->sites;
        }
 
-       /**
-        * Returns a new Site object constructed from the provided ORMRow.
-        *
-        * @since 1.25
-        *
-        * @param ORMRow $siteRow
-        *
-        * @return Site
-        */
-       protected function siteFromRow( ORMRow $siteRow ) {
-
-               $site = Site::newForType( $siteRow->getField( 'type', Site::TYPE_UNKNOWN ) );
-
-               $site->setGlobalId( $siteRow->getField( 'global_key' ) );
-
-               $site->setInternalId( $siteRow->getField( 'id' ) );
-
-               if ( $siteRow->hasField( 'forward' ) ) {
-                       $site->setForward( $siteRow->getField( 'forward' ) );
-               }
-
-               if ( $siteRow->hasField( 'group' ) ) {
-                       $site->setGroup( $siteRow->getField( 'group' ) );
-               }
-
-               if ( $siteRow->hasField( 'language' ) ) {
-                       $site->setLanguageCode( $siteRow->getField( 'language' ) === ''
-                               ? null
-                               : $siteRow->getField( 'language' )
-                       );
-               }
-
-               if ( $siteRow->hasField( 'source' ) ) {
-                       $site->setSource( $siteRow->getField( 'source' ) );
-               }
-
-               if ( $siteRow->hasField( 'data' ) ) {
-                       $site->setExtraData( $siteRow->getField( 'data' ) );
-               }
-
-               if ( $siteRow->hasField( 'config' ) ) {
-                       $site->setExtraConfig( $siteRow->getField( 'config' ) );
-               }
-
-               return $site;
-       }
-
-       /**
-        * Get a new ORMRow from a Site object
-        *
-        * @since 1.25
-        *
-        * @param Site $site
-        *
-        * @return ORMRow
-        */
-       protected function getRowFromSite( Site $site ) {
-               $fields = array(
-                       // Site data
-                       'global_key' => $site->getGlobalId(), // TODO: check not null
-                       'type' => $site->getType(),
-                       'group' => $site->getGroup(),
-                       'source' => $site->getSource(),
-                       'language' => $site->getLanguageCode() === null ? '' : $site->getLanguageCode(),
-                       'protocol' => $site->getProtocol(),
-                       'domain' => strrev( $site->getDomain() ) . '.',
-                       'data' => $site->getExtraData(),
-
-                       // Site config
-                       'forward' => $site->shouldForward(),
-                       'config' => $site->getExtraConfig(),
-               );
-
-               if ( $site->getInternalId() !== null ) {
-                       $fields['id'] = $site->getInternalId();
-               }
-
-               return new ORMRow( $this->sitesTable, $fields );
-       }
-
        /**
         * Fetches the site from the database and loads them into the sites field.
         *
@@ -153,16 +67,46 @@ class DBSiteStore implements SiteStore {
        protected function loadSites() {
                $this->sites = new SiteList();
 
-               $siteRows = $this->sitesTable->select( null, array(), array(
-                       'ORDER BY' => 'site_global_key'
-               ) );
+               $dbr = wfGetDB( DB_SLAVE );
 
-               foreach ( $siteRows as $siteRow ) {
-                       $this->sites[] = $this->siteFromRow( $siteRow );
+               $res = $dbr->select(
+                       'sites',
+                       array(
+                               'site_id',
+                               'site_global_key',
+                               'site_type',
+                               'site_group',
+                               'site_source',
+                               'site_language',
+                               'site_protocol',
+                               'site_domain',
+                               'site_data',
+                               'site_forward',
+                               'site_config',
+                       ),
+                       '',
+                       __METHOD__,
+                       array( 'ORDER BY' => 'site_global_key' )
+               );
+
+               foreach ( $res as $row ) {
+                       $site = Site::newForType( $row->site_type );
+                       $site->setGlobalId( $row->site_global_key );
+                       $site->setInternalId( (int)$row->site_id );
+                       $site->setForward( (bool)$row->site_forward );
+                       $site->setGroup( $row->site_group );
+                       $site->setLanguageCode( $row->site_language === ''
+                               ? null
+                               : $row->site_language
+                       );
+                       $site->setSource( $row->site_source );
+                       $site->setExtraData( unserialize( $row->site_data ) );
+                       $site->setExtraConfig( unserialize( $row->site_config ) );
+                       $this->sites[] = $site;
                }
 
                // Batch load the local site identifiers.
-               $ids = wfGetDB( $this->sitesTable->getReadDb() )->select(
+               $ids = $dbr->select(
                        'site_identifiers',
                        array(
                                'si_site',
@@ -226,7 +170,7 @@ class DBSiteStore implements SiteStore {
                        return true;
                }
 
-               $dbw = $this->sitesTable->getWriteDbConnection();
+               $dbw = wfGetDB( DB_MASTER );
 
                $dbw->startAtomic( __METHOD__ );
 
@@ -240,12 +184,37 @@ class DBSiteStore implements SiteStore {
                                $internalIds[] = $site->getInternalId();
                        }
 
-                       $siteRow = $this->getRowFromSite( $site );
-                       $success = $siteRow->save( __METHOD__ ) && $success;
+                       $fields = array(
+                               // Site data
+                               'site_global_key' => $site->getGlobalId(), // TODO: check not null
+                               'site_type' => $site->getType(),
+                               'site_group' => $site->getGroup(),
+                               'site_source' => $site->getSource(),
+                               'site_language' => $site->getLanguageCode() === null ? '' : $site->getLanguageCode(),
+                               'site_protocol' => $site->getProtocol(),
+                               'site_domain' => strrev( $site->getDomain() ) . '.',
+                               'site_data' => serialize( $site->getExtraData() ),
+
+                               // Site config
+                               'site_forward' => $site->shouldForward() ? 1 : 0,
+                               'site_config' => serialize( $site->getExtraConfig() ),
+                       );
+
+                       $rowId = $site->getInternalId();
+                       if ( $rowId !== null ) {
+                               $success = $dbw->update(
+                                       'sites', $fields, array( 'site_id' => $rowId ), __METHOD__
+                               ) && $success;
+                       } else {
+                               $rowId = $dbw->nextSequenceValue( 'sites_site_id_seq' );
+                               $fields['site_id'] = $rowId;
+                               $success = $dbw->insert( 'sites', $fields, __METHOD__ ) && $success;
+                               $rowId = $dbw->insertId();
+                       }
 
                        foreach ( $site->getLocalIds() as $idType => $ids ) {
                                foreach ( $ids as $id ) {
-                                       $localIds[] = array( $siteRow->getId(), $idType, $id );
+                                       $localIds[] = array( $rowId, $idType, $id );
                                }
                        }
                }
@@ -294,7 +263,7 @@ class DBSiteStore implements SiteStore {
         * @return bool Success
         */
        public function clear() {
-               $dbw = $this->sitesTable->getWriteDbConnection();
+               $dbw = wfGetDB( DB_MASTER );
 
                $dbw->startAtomic( __METHOD__ );
                $ok = $dbw->delete( 'sites', '*', __METHOD__ );
@@ -306,44 +275,4 @@ class DBSiteStore implements SiteStore {
                return $ok;
        }
 
-       /**
-        * @since 1.25
-        *
-        * @return ORMTable
-        */
-       protected function newSitesTable() {
-               return new ORMTable(
-                       'sites',
-                       array(
-                               'id' => 'id',
-
-                               // Site data
-                               'global_key' => 'str',
-                               'type' => 'str',
-                               'group' => 'str',
-                               'source' => 'str',
-                               'language' => 'str',
-                               'protocol' => 'str',
-                               'domain' => 'str',
-                               'data' => 'array',
-
-                               // Site config
-                               'forward' => 'bool',
-                               'config' => 'array',
-                       ),
-                       array(
-                               'type' => Site::TYPE_UNKNOWN,
-                               'group' => Site::GROUP_NONE,
-                               'source' => Site::SOURCE_LOCAL,
-                               'data' => array(),
-
-                               'forward' => false,
-                               'config' => array(),
-                               'language' => '',
-                       ),
-                       'ORMRow',
-                       'site_'
-               );
-       }
-
 }

diff --git a/includes/site/SiteSQLStore.php b/includes/site/SiteSQLStore.php
index e3230ff..e61179b 100644 (file)
--- a/includes/site/SiteSQLStore.php
+++ b/includes/site/SiteSQLStore.php
@@ -34,12 +34,18 @@ class SiteSQLStore extends CachingSiteStore {
         * @since 1.21
         * @deprecated 1.25 Construct a SiteStore instance directly instead.
         *
-        * @param ORMTable|null $sitesTable
+        * @param null $sitesTable Unused
         * @param BagOStuff|null $cache
         *
         * @return SiteStore
         */
-       public static function newInstance( ORMTable $sitesTable = null, BagOStuff $cache = null ) {
+       public static function newInstance( $sitesTable = null, BagOStuff $cache = null ) {
+               if ( $sitesTable !== null ) {
+                       throw new InvalidArgumentException(
+                               __METHOD__ . ': $sitesTable parameter is unused and must be null'
+                       );
+               }
+
                if ( $cache === null ) {
                        $cache = wfGetCache( wfIsHHVM() ? CACHE_ACCEL : CACHE_ANYTHING );
                }