r81440 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81439‎ | r81440 | r81441 >
Date:05:17, 3 February 2011
Author:overlordq
Status:ok (Comments)
Tags:
Comment:
Follow-up to r81439, works up to installing the admin user then dies with:

Creating administrator user account...

Mediawiki tried to access the database via wfGetDB(). This is not allowed.

Backtrace:

#0 /var/www/testsite.com/w/includes/GlobalFunctions.php(3020): LBFactory_Fake->getMainLB(false)
#1 /var/www/testsite.com/w/includes/GlobalFunctions.php(3010): wfGetLB(false)
#2 /var/www/testsite.com/w/includes/User.php(2558): wfGetDB(-1)
#3 /var/www/testsite.com/w/includes/installer/Installer.php(1383): User->idForName()
#4 [internal function]: Installer->createSysop(Object(PostgresInstaller))
#5 /var/www/testsite.com/w/includes/installer/Installer.php(1293): call_user_func(Array, Object(PostgresInstaller))
#6 /var/www/testsite.com/w/includes/installer/WebInstallerPage.php(1022): Installer->performInstallation(Array, Array)
#7 /var/www/testsite.com/w/includes/installer/WebInstaller.php(243): WebInstaller_Install->execute()
#8 /var/www/testsite.com/w/config/index.php(46): WebInstaller->execute(Array)
#9 /var/www/testsite.com/w/config/index.php(14): wfInstallerMain()
#10 {main}
Modified paths:
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/installer/PostgresInstaller.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -151,6 +151,7 @@
152152 if ( !strlen( $user ) ) { # e.g. the class is being loaded
153153 return;
154154 }
 155+
155156 $this->close();
156157 $this->mServer = $server;
157158 $this->mPort = $port = $wgDBport;
@@ -1014,6 +1015,7 @@
10151016 if ( !$schema ) {
10161017 $schema = $wgDBmwschema;
10171018 }
 1019+ $table = $this->tableName( $table );
10181020 $etable = $this->addQuotes( $table );
10191021 $eschema = $this->addQuotes( $schema );
10201022 $SQL = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
Index: trunk/phase3/includes/installer/PostgresInstaller.php
@@ -24,6 +24,7 @@
2525 );
2626
2727 var $minimumVersion = '8.3';
 28+ var $useAdmin = FALSE;
2829
2930 function getName() {
3031 return 'postgres';
@@ -69,17 +70,18 @@
7071 return $status;
7172 }
7273
 74+ $this->useAdmin = TRUE;
7375 // Try to connect
74 - $status->merge( $this->getConnection( 'template1' ) );
 76+ $status->merge( $this->getConnection() );
7577 if ( !$status->isOK() ) {
7678 return $status;
7779 }
7880
79 -/* //Make sure install user can create
 81+ //Make sure install user can create
8082 $status->merge( $this->canCreateAccounts() );
8183 if ( !$status->isOK() ) {
8284 return $status;
83 - } */
 85+ }
8486
8587 // Check version
8688 $version = $this->db->getServerVersion();
@@ -92,14 +94,22 @@
9395 return $status;
9496 }
9597
96 - function openConnection( $database = 'template1' ) {
 98+ public function openConnection() {
9799 $status = Status::newGood();
98100 try {
99 - $db = new DatabasePostgres(
100 - $this->getVar( 'wgDBserver' ),
101 - $this->getVar( '_InstallUser' ),
102 - $this->getVar( '_InstallPassword' ),
103 - $database );
 101+ if ( $this->useAdmin ) {
 102+ $db = new DatabasePostgres(
 103+ $this->getVar( 'wgDBserver' ),
 104+ $this->getVar( '_InstallUser' ),
 105+ $this->getVar( '_InstallPassword' ),
 106+ 'template1' );
 107+ } else {
 108+ $db = new DatabasePostgres(
 109+ $this->getVar( 'wgDBserver' ),
 110+ $this->getVar( 'wgDBuser' ),
 111+ $this->getVar( 'wgDBpassword' ),
 112+ $this->getVar( 'wgDBname' ) );
 113+ }
104114 $status->value = $db;
105115 } catch ( DBConnectionError $e ) {
106116 $status->fatal( 'config-connection-error', $e->getMessage() );
@@ -107,32 +117,9 @@
108118 return $status;
109119 }
110120
111 - function getConnection($database = null) {
112 - $status = Status::newGood();
113 -
114 - if( is_null( $database ) ) {
115 - $dbname = $this->getVar( 'wgDBname' );
116 - $dbuser = $this->getVar( 'wgDBuser' );
117 - $dbpass = $this->getVar( 'wgDBpassword' );
118 - } else {
119 - $dbname = $database;
120 - $dbuser = $this->getVar( '_InstallUser' );
121 - $dbpass = $this->getVar( '_InstallPassword' );
122 - }
123 -
124 - try {
125 - $this->db = new DatabasePostgres(
126 - $this->getVar( 'wgDBserver' ),
127 - $dbuser, $dbpass, $dbname );
128 - $status->value = $this->db;
129 - } catch ( DBConnectionError $e ) {
130 - $status->fatal( 'config-connection-error', $e->getMessage() );
131 - }
132 - return $status;
133 - }
134 -
135121 protected function canCreateAccounts() {
136 - $status = $this->getConnection( 'template1' );
 122+ $this->useAdmin = TRUE;
 123+ $status = $this->getConnection();
137124 if ( !$status->isOK() ) {
138125 return false;
139126 }
@@ -224,7 +211,8 @@
225212 }
226213
227214 function setupDatabase() {
228 - $status = $this->getConnection( 'template1' );
 215+ $this->useAdmin = TRUE;
 216+ $status = $this->getConnection();
229217 if ( !$status->isOK() ) {
230218 return $status;
231219 }
@@ -273,7 +261,6 @@
274262 "FROM pg_catalog.pg_proc p, pg_catalog.pg_namespace n\n" .
275263 "WHERE p.pronamespace = n.oid AND n.nspname = $safeschema2";
276264 $res = $conn->query( $SQL );
277 - $conn->query( "SET search_path = $safeschema" );
278265 }
279266 }
280267 return $status;
@@ -289,27 +276,31 @@
290277 return Status::newGood();
291278 }
292279
293 - $status = $this->getConnection( 'template1' );
 280+ $this->useAdmin = TRUE;
 281+ $status = $this->getConnection();
 282+
294283 if ( !$status->isOK() ) {
295284 return $status;
296285 }
297286
298287 $db = $this->getVar( 'wgDBname' );
 288+ $schema = $this->getVar( 'wgDBmwschema' );
299289 $this->db->selectDB( $db );
300290 $safeuser = $this->db->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
301291 $safeusercheck = $this->db->addQuotes( $this->getVar( 'wgDBuser' ) );
302292 $safepass = $this->db->addQuotes( $this->getVar( 'wgDBpassword' ) );
 293+ $safeschema = $this->db->addIdentifierQuotes( $schema );
303294
304295 $rows = $this->db->numRows(
305296 $this->db->query( "SELECT 1 FROM pg_catalog.pg_shadow WHERE usename = $safeusercheck" )
306297 );
307298 if ( $rows < 1 ) {
308 - $res = $this->db->query( "CREATE USER $safeuser NOCREATEDB PASSWORD $safepass", __METHOD__ );
309 -
 299+ $res = $this->db->query( "CREATE USER $safeuser NOCREATEDB PASSWORD $safepass", __METHOD__ );
310300 if ( $res !== true && !( $res instanceOf ResultWrapper ) ) {
311301 $status->fatal( 'config-install-user-failed', $this->getVar( 'wgDBuser' ), $res );
 302+ }
 303+ $this->db->query("ALTER USER $safeuser SET search_path = $safeschema");
312304 }
313 - }
314305
315306 return $status;
316307 }
@@ -332,7 +323,41 @@
333324 $wgDBpassword = $this->getVar( '_InstallPassword' );
334325 }
335326
 327+ public function createTables() {
 328+ $this->db = NULL;
 329+ $this->useAdmin = FALSE;
 330+ $status = $this->getConnection();
 331+ var_export($status);
 332+ if ( !$status->isOK() ) {
 333+ return $status;
 334+ }
 335+ $this->db->selectDB( $this->getVar( 'wgDBname' ) );
 336+
 337+ if( $this->db->tableExists( 'user' ) ) {
 338+ $status->warning( 'config-install-tables-exist' );
 339+ return $status;
 340+ }
 341+
 342+ $this->db->setFlag( DBO_DDLMODE ); // For Oracle's handling of schema files
 343+ $this->db->begin( __METHOD__ );
 344+
 345+ $error = $this->db->sourceFile( $this->db->getSchema() );
 346+ if( $error !== true ) {
 347+ $this->db->reportQueryError( $error, 0, '', __METHOD__ );
 348+ $this->db->rollback( __METHOD__ );
 349+ $status->fatal( 'config-install-tables-failed', $error );
 350+ } else {
 351+ $this->db->commit( __METHOD__ );
 352+ }
 353+ // Resume normal operations
 354+ if( $status->isOk() ) {
 355+ $this->enableLB();
 356+ }
 357+ return $status;
 358+ }
 359+
336360 public function setupPLpgSQL() {
 361+ $this->useAdmin = TRUE;
337362 $status = $this->getConnection();
338363 if ( !$status->isOK() ) {
339364 return $status;

Follow-up revisions

RevisionCommit summaryAuthorDate
r89821PostgreSQL install fixes:...tstarling11:32, 10 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r81439Results of my work on new-installer and Pg....mah04:06, 3 February 2011

Comments

#Comment by OverlordQ (talk | contribs)   05:18, 3 February 2011

I got no clue about LoadBalancer black magic.

#Comment by Nikerabbit (talk | contribs)   08:45, 3 February 2011

Write false, true and null with lowercase please. Isn't setting member variable which controls what getConnection returns a bit ugly and error prone?

#Comment by OverlordQ (talk | contribs)   08:54, 3 February 2011

Probably but the installer rewrite slightly forced it as it's the only way to tell it to use the right connection.

#Comment by 😂 (talk | contribs)   14:32, 4 February 2011

This is a good point.

What if we made a getUnprivilegedConnection() that returns the non-installer user (if different). Would also be helpful for generally testing the account we've just created.

#Comment by OverlordQ (talk | contribs)   09:02, 3 February 2011

To elaborate, all the inserts and table creations should be done as the wikiuser not as the install user. Without completely re-implementing every single callback and install step in PostgresInstaller this is the only way I've found to get the installer to actually do that. I didn't write it that way.

If there's a more graceful solution feel free to fix.

#Comment by Reedy (talk | contribs)   11:00, 3 February 2011

You've also got a mix of tabs and spaces for indenting...

#Comment by Brion VIBBER (talk | contribs)   03:16, 4 February 2011

Adding nodeploy tag -- this is an installer issue and doesn't block WMF deployment of 1.17

#Comment by 😂 (talk | contribs)   17:38, 25 March 2011

This has been largely superseded, marking ok. wfGetDB() issues still exist, but it's not the fault of this rev.

Status & tagging log