r79989 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79988‎ | r79989 | r79990 >
Date:03:07, 11 January 2011
Author:overlordq
Status:resolved (Comments)
Tags:
Comment:
I have no clue how the callbacks work, this at least sets up a database, everything after that is fubar
Modified paths:
  • /trunk/phase3/includes/installer/PostgresInstaller.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/installer/PostgresInstaller.php
——@@ -81,6 +81,12 @@
8282 return $status;
8383 }
8484
 85+/* //Make sure install user can create
 86+ $status->merge( $this->canCreateAccounts() );
 87+ if ( !$status->isOK() ) {
 88+ return $status;
 89+ } */
 90+
8591 // Check version
8692 $version = $this->db->getServerVersion();
8793 if ( version_compare( $version, $this->minimumVersion ) < 0 ) {
——@@ -92,7 +98,7 @@
9399 return $status;
94100 }
95101
96 - function getConnection() {
 102+ function getConnection($database = 'template1') {
97103 $status = Status::newGood();
98104
99105 try {
——@@ -100,7 +106,7 @@
101107 $this->getVar( 'wgDBserver' ),
102108 $this->getVar( '_InstallUser' ),
103109 $this->getVar( '_InstallPassword' ),
104 - false );
 110+ $database );
105111 $status->value = $this->db;
106112 } catch ( DBConnectionError $e ) {
107113 $status->fatal( 'config-connection-error', $e->getMessage() );
——@@ -187,20 +193,25 @@
188194 'name' => 'pg-commit',
189195 'callback' => array( $this, 'commitChanges' ),
190196 );
191 - $userCB = array(
 197+/* $userCB = array(
192198 'name' => 'user',
193199 'callback' => array( $this, 'setupUser' ),
194 - );
 200+ ); */
195201 $ts2CB = array(
196202 'name' => 'pg-ts2',
197203 'callback' => array( $this, 'setupTs2' ),
198204 );
199205 $this->parent->addInstallStep( $commitCB, 'interwiki' );
200 - $this->parent->addInstallStep( $userCB );
 206+// $this->parent->addInstallStep( $userCB );
201207 $this->parent->addInstallStep( $ts2CB, 'setupDatabase' );
202208 }
203209
204210 function setupDatabase() {
 211+ $status = $this->setupUser();
 212+ if (!$status->isOK() ) {
 213+ return $status;
 214+ }
 215+
205216 $status = $this->getConnection();
206217 if ( !$status->isOK() ) {
207218 return $status;
——@@ -208,26 +219,49 @@
209220 $conn = $status->value;
210221
211222 $dbName = $this->getVar( 'wgDBname' );
212 - if( !$conn->selectDB( $dbName ) ) {
213 - // Make sure that we can write to the correct schema
214 - // If not, Postgres will happily and silently go to the next search_path item
 223+ $SQL = "SELECT 1 FROM pg_catalog.pg_database WHERE datname = " . $conn->addQuotes( $wgDBname );
 224+ $rows = $conn->numRows( $conn->doQuery( $SQL ) );
 225+ if( !$rows ) {
215226 $schema = $this->getVar( 'wgDBmwschema' );
216 - $ctest = 'mediawiki_test_table';
 227+ $user = $this->getVar( 'wgDBuser' );
 228+
217229 $safeschema = $conn->addIdentifierQuotes( $schema );
218 - if ( $conn->tableExists( $ctest, $schema ) ) {
219 - $conn->query( "DROP TABLE $safeschema.$ctest" );
220 - }
221 - $res = $conn->query( "CREATE TABLE $safeschema.$ctest(a int)" );
222 - if ( !$res ) {
223 - $status->fatal( 'config-install-pg-schema-failed',
224 - $this->getVar( 'wgDBuser'), $schema );
225 - }
226 - $conn->query( "DROP TABLE $safeschema.$ctest" );
 230+ $safeuser = $conn->addIdentifierQuotes( $user );
227231
228232 $safedb = $conn->addIdentifierQuotes( $dbName );
229 - $safeuser = $conn->addQuotes( $this->getVar( 'wgDBuser' ) );
 233+
230234 $conn->query( "CREATE DATABASE $safedb OWNER $safeuser", __METHOD__ );
231 - $conn->selectDB( $dbName );
 235+
 236+ $conn = new DatabasePostgres(
 237+ $this->getVar( 'wgDBserver' ),
 238+ $this->getVar( 'wgDBuser' ),
 239+ $this->getVar( 'wgDBpassword' ),
 240+ $dbName,
 241+ false,
 242+ 0,
 243+ $this->getVar( 'wgDBprefix' )
 244+ );
 245+
 246+ $result = $conn->schemaExists( $schema );
 247+ if( !$result ) {
 248+ $result = $conn->doQuery( "CREATE SCHEMA $safeschema AUTHORIZATION $safeuser" );
 249+ if( !$result ) {
 250+ $status->fatal( 'config-install-pg-schema-failed', $user, $schema );
 251+ }
 252+ } else {
 253+ $safeschema2 = $conn->addQuotes( $schema );
 254+ $SQL = "SELECT 'GRANT ALL ON '||pg_catalog.quote_ident(relname)||' TO $safeuser;'\n".
 255+ "FROM pg_catalog.pg_class p, pg_catalog.pg_namespace n\n".
 256+ "WHERE relnamespace = n.oid AND n.nspname = $safeschema2\n".
 257+ "AND p.relkind IN ('r','S','v')\n";
 258+ $SQL .= "UNION\n";
 259+ $SQL .= "SELECT 'GRANT ALL ON FUNCTION '||pg_catalog.quote_ident(proname)||'('||\n".
 260+ "pg_catalog.oidvectortypes(p.proargtypes)||') TO $safeuser;'\n".
 261+ "FROM pg_catalog.pg_proc p, pg_catalog.pg_namespace n\n".
 262+ "WHERE p.pronamespace = n.oid AND n.nspname = $safeschema2";
 263+ $res = $conn->doQuery( $SQL );
 264+ $conn->doQuery( "SET search_path = $safeschema" );
 265+ }
232266 }
233267 return $status;
234268 }
——@@ -274,9 +308,11 @@
275309
276310 $db = $this->getVar( 'wgDBname' );
277311 $this->db->selectDB( $db );
278 - $safeuser = $this->db->addQuotes( $this->getVar( 'wgDBuser' ) );
 312+ $safeuser = $this->db->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) );
279313 $safepass = $this->db->addQuotes( $this->getVar( 'wgDBpassword' ) );
280314 $res = $this->db->query( "CREATE USER $safeuser NOCREATEDB PASSWORD $safepass", __METHOD__ );
 315+ return $status;
 316+
281317 if ( $res !== true ) {
282318 $status->fatal( 'config-install-user-failed', $this->getVar( 'wgDBuser' ) );
283319 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r80006Fix callback issue noted in r79989 (broken in r78774)....demon13:54, 11 January 2011
r80007Fix copy+paste mistake from r79989demon14:00, 11 January 2011
r81182MFT a bunch of installer fixes. r80238, r80128, r80124, r80083, r80080, r800...demon01:59, 29 January 2011

Comments

#Comment by 😂 (talk | contribs)   03:32, 11 January 2011
  • The changes in setupDatabase() look ok.
  • In setupUser(), it looks like you're returning $status too early and not checking the result of $res like I did originally. Was this intended?
  • We shouldn't call setupUser() from setupDatabase(), it's its own step (the main point of breaking these into steps is to prevent the huge walls of unmaintainable code like we had in the old installer). By passing $userCB to addInstallStep() without a second parameter, it should've added it to the beginning (before setupDatabase(), functionally identical to how you changed it). If it's not doing that, that's my fault and needs fixing.
#Comment by OverlordQ (talk | contribs)   05:54, 11 January 2011

Yes, I had a comment in there, but the $res !== true check was failing even though the query completed successfully so I left it for later.

And yes, setupDatabase was being called before setupUser which is why I changed it like that.

#Comment by Reedy (talk | contribs)   05:52, 11 January 2011

Looks like it's got a mix of tabs and spaces (like I usually do..)

Status & tagging log