From 4191a167003be97e75da7988353a066006640fb8 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 6 Oct 2015 13:08:56 -0400 Subject: [PATCH] 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 --- includes/site/DBSiteStore.php | 215 +++++++++++---------------------- includes/site/SiteSQLStore.php | 10 +- 2 files changed, 80 insertions(+), 145 deletions(-) diff --git a/includes/site/DBSiteStore.php b/includes/site/DBSiteStore.php index 1193bd65f6..17764a1985 100644 --- 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 e3230fff84..e61179b02e 100644 --- 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 ); } -- 2.20.1