#346494 by hswong3i, Pancho, Dave Reid, duellj, dalin: Fixed DB drivers need to be able to change the configure database form during install

4 files changed, 192 insertions, 94 deletions

diff --git a/INSTALL.sqlite.txt b/INSTALL.sqlite.txt
index 4081ffbf9..e81e6abcc 100644
--- a/INSTALL.sqlite.txt
+++ b/INSTALL.sqlite.txt
@@ -16,13 +16,13 @@ the database file resides. This directory (not just the database file) also has
 to remain writeable by the web server going forward for SQLite to continue to be
 able to operate.
 
-On the "Database configuration" form in the "Database name" field, you must
+On the "Database configuration" form in the "Database file" field, you must
 supply the exact path to where you wish your database file to reside. It is
 strongly suggested that you choose a path that is outside of the webroot, yet
 ensure that the directory is writeable by the web server.
 
 If you must place your database file in your webroot, you could try using the
-following in your "Database name" field:
+following in your "Database file" field:
 
   sites/default/files/.ht.sqlite
 
@@ -30,8 +30,3 @@ Note: The .ht in the name will tell Apache to prevent the database from being
 downloaded. Please check that the file is, indeed, protected by your webserver.
 If not, please consult the documentation of your webserver on how to protect a
 file from downloading.
-
-USERNAME, PASSWORD, and ADVANCED OPTIONS
-----------------------------------------
-No username, password, or advanced options are necessary, and they should not be
-used.
diff --git a/includes/database/sqlite/install.inc b/includes/database/sqlite/install.inc
index 82ed0a48d..a8a12410d 100644
--- a/includes/database/sqlite/install.inc
+++ b/includes/database/sqlite/install.inc
@@ -21,5 +21,32 @@ class DatabaseTasks_sqlite extends DatabaseTasks {
   public function minimumVersion() {
     return '3.3.7';
   }
+
+  public function getFormOptions($database) {
+    $form = parent::getFormOptions($database);
+
+    // Remove the options that only apply to client/server style databases.
+    unset($form['username'], $form['password'], $form['advanced_options']['host'], $form['advanced_options']['port']);
+
+    // Make the text more accurate for SQLite.
+    $form['database']['#title'] = st('Database file');
+    $form['database']['#description'] = st('The absolute path to the file where @drupal data will be stored. This must be writable by the web server and should exist outside of the web root.', array('@drupal' => drupal_install_profile_distribution_name()));
+    $default_database = conf_path(FALSE, TRUE) . '/files/.ht.sqlite';
+    $form['database']['#default_value'] = empty($database['database']) ? $default_database : $database['database'];
+    return $form;
+  }
+
+  public function validateDatabaseSettings($database) {
+    // Perform standard validation.
+    $errors = parent::validateDatabaseSettings($database);
+
+    // Verify the database is writable.
+    $db_directory = new SplFileInfo(dirname($database['database']));
+    if (!$db_directory->isWritable()) {
+      $errors[$database['driver'] . '][database'] = st('The directory you specified is not writable by the web server.');
+    }
+
+    return $errors;
+  }
 }
 
diff --git a/includes/install.core.inc b/includes/install.core.inc
index 179d6b84d..5b01eb634 100644
--- a/includes/install.core.inc
+++ b/includes/install.core.inc
@@ -850,14 +850,13 @@ function install_settings_form($form, &$form_state, &$install_state) {
 
   drupal_set_title(st('Database configuration'));
 
-  $drivers = drupal_detect_database_types();
+  $drivers = drupal_get_database_types();
   $drivers_keys = array_keys($drivers);
 
   $form['driver'] = array(
     '#type' => 'radios',
     '#title' => st('Database type'),
     '#required' => TRUE,
-    '#options' => $drivers,
     '#default_value' => !empty($database['driver']) ? $database['driver'] : current($drivers_keys),
     '#description' => st('The type of database your @drupal data will be stored in.', array('@drupal' => drupal_install_profile_distribution_name())),
   );
@@ -866,82 +865,35 @@ function install_settings_form($form, &$form_state, &$install_state) {
     $form['driver']['#description'] .= ' ' . st('Your PHP configuration only supports a single database type, so it has been automatically selected.');
   }
 
-  // Database name.
-  $form['database'] = array(
-    '#type' => 'textfield',
-    '#title' => st('Database name'),
-    '#default_value' => empty($database['database']) ? '' : $database['database'],
-    '#size' => 45,
-    '#required' => TRUE,
-    '#description' => st('The name of the database your @drupal data will be stored in. It must exist on your server before @drupal can be installed.', array('@drupal' => drupal_install_profile_distribution_name())),
-  );
-
-  // Database username.
-  $form['username'] = array(
-    '#type' => 'textfield',
-    '#title' => st('Database username'),
-    '#default_value' => empty($database['username']) ? '' : $database['username'],
-    '#size' => 45,
-  );
-
-  // Database password.
-  $form['password'] = array(
-    '#type' => 'password',
-    '#title' => st('Database password'),
-    '#default_value' => empty($database['password']) ? '' : $database['password'],
-    '#size' => 45,
-  );
-
-  $form['advanced_options'] = array(
-    '#type' => 'fieldset',
-    '#title' => st('Advanced options'),
-    '#collapsible' => TRUE,
-    '#collapsed' => TRUE,
-    '#description' => st("These options are only necessary for some sites. If you're not sure what you should enter here, leave the default settings or check with your hosting provider.")
-  );
-
-  // Database host.
-  $form['advanced_options']['host'] = array(
-    '#type' => 'textfield',
-    '#title' => st('Database host'),
-    '#default_value' => empty($database['host']) ? 'localhost' : $database['host'],
-    '#size' => 45,
-    // Hostnames can be 255 characters long.
-    '#maxlength' => 255,
-    '#required' => TRUE,
-    '#description' => st('If your database is located on a different server, change this.'),
-  );
-
-  // Database port.
-  $form['advanced_options']['port'] = array(
-    '#type' => 'textfield',
-    '#title' => st('Database port'),
-    '#default_value' => empty($database['port']) ? '' : $database['port'],
-    '#size' => 45,
-    // The maximum port number is 65536, 5 digits.
-    '#maxlength' => 5,
-    '#description' => st('If your database server is listening to a non-standard port, enter its number.'),
-  );
-
-  // Table prefix.
-  $db_prefix = ($profile == 'standard') ? 'drupal_' : $profile . '_';
-  $form['advanced_options']['db_prefix'] = array(
-    '#type' => 'textfield',
-    '#title' => st('Table prefix'),
-    '#default_value' => '',
-    '#size' => 45,
-    '#description' => st('If more than one application will be sharing this database, enter a table prefix such as %prefix for your @drupal site here.', array('@drupal' => drupal_install_profile_distribution_name(), '%prefix' => $db_prefix)),
-  );
+  // Add driver specific configuration options.
+  foreach ($drivers as $key => $driver) {
+    $form['driver']['#options'][$key] = $driver->name();
+
+    $form['settings'][$key] = $driver->getFormOptions($database);
+    $form['settings'][$key]['#prefix'] = '<h2 class="js-hide">' . st('@driver_name settings', array('@driver_name' => $driver->name())) . '</h2>';
+    $form['settings'][$key]['#type'] = 'container';
+    $form['settings'][$key]['#tree'] = TRUE;
+    $form['settings'][$key]['advanced_options']['#parents'] = array($key);
+    $form['settings'][$key]['#states'] = array(
+      'visible' => array(
+        ':input[name=driver]' => array('value' => $key),
+      )
+    );
+  }
 
   $form['actions'] = array('#type' => 'actions');
   $form['actions']['save'] = array(
     '#type' => 'submit',
     '#value' => st('Save and continue'),
+    '#limit_validation_errors' => array(
+      array('driver'),
+      array(isset($form_state['input']['driver']) ? $form_state['input']['driver'] : current($drivers_keys)),
+    ),
+    '#submit' => array('install_settings_form_submit'),
   );
 
   $form['errors'] = array();
   $form['settings_file'] = array('#type' => 'value', '#value' => $settings_file);
-  $form['_database'] = array('#type' => 'value');
 
   return $form;
 }
@@ -950,12 +902,17 @@ function install_settings_form($form, &$form_state, &$install_state) {
  * Form API validate for install_settings form.
  */
 function install_settings_form_validate($form, &$form_state) {
+  $driver = $form_state['values']['driver'];
+  $database = $form_state['values'][$driver];
+  $database['driver'] = $driver;
+
   // TODO: remove when PIFR will be updated to use 'db_prefix' instead of
   // 'prefix' in the database settings form.
-  $form_state['values']['prefix'] = $form_state['values']['db_prefix'];
+  $database['prefix'] = $database['db_prefix'];
+  unset($database['db_prefix']);
 
-  form_set_value($form['_database'], $form_state['values'], $form_state);
-  $errors = install_database_errors($form_state['values'], $form_state['values']['settings_file']);
+  $form_state['storage']['database'] = $database;
+  $errors = install_database_errors($database, $form_state['values']['settings_file']);
   foreach ($errors as $name => $message) {
     form_set_error($name, $message);
   }
@@ -967,22 +924,17 @@ function install_settings_form_validate($form, &$form_state) {
 function install_database_errors($database, $settings_file) {
   global $databases;
   $errors = array();
-  // Verify the table prefix.
-  if (!empty($database['prefix']) && is_string($database['prefix']) && !preg_match('/^[A-Za-z0-9_.]+$/', $database['prefix'])) {
-    $errors['prefix'] = st('The database table prefix you have entered, %prefix, is invalid. The table prefix can only contain alphanumeric characters, periods, or underscores.', array('%prefix' => $database['prefix']));
-  }
-
-  if (!empty($database['port']) && !is_numeric($database['port'])) {
-    $errors['db_port'] = st('Database port must be a number.');
-  }
 
   // Check database type.
-  $database_types = drupal_detect_database_types();
+  $database_types = drupal_get_database_types();
   $driver = $database['driver'];
   if (!isset($database_types[$driver])) {
-    $errors['driver'] = st("In your %settings_file file you have configured @drupal to use a %driver server, however your PHP installation currently does not support this database type.", array('%settings_file' => $settings_file, '@drupal' => drupal_install_profile_distribution_name(), '%driver' => $database['driver']));
+    $errors['driver'] = st("In your %settings_file file you have configured @drupal to use a %driver server, however your PHP installation currently does not support this database type.", array('%settings_file' => $settings_file, '@drupal' => drupal_install_profile_distribution_name(), '%driver' => $driver));
   }
   else {
+    // Run driver specific validation
+    $errors += $database_types[$driver]->validateDatabaseSettings($database);
+
     // Run tasks associated with the database type. Any errors are caught in the
     // calling function.
     $databases['default']['default'] = $database;
@@ -991,13 +943,13 @@ function install_database_errors($database, $settings_file) {
     Database::parseConnectionInfo();
 
     try {
-      db_run_tasks($database['driver']);
+      db_run_tasks($driver);
     }
     catch (DatabaseTaskException $e) {
       // These are generic errors, so we do not have any specific key of the
       // database connection array to attach them to; therefore, we just put
       // them in the error array with standard numeric keys.
-      $errors[] = $e->getMessage();
+      $errors[$driver . '][0'] = $e->getMessage();
     }
   }
   return $errors;
@@ -1009,10 +961,9 @@ function install_database_errors($database, $settings_file) {
 function install_settings_form_submit($form, &$form_state) {
   global $install_state;
 
-  $database = array_intersect_key($form_state['values']['_database'], array_flip(array('driver', 'database', 'username', 'password', 'host', 'port', 'prefix')));
   // Update global settings array and save.
   $settings['databases'] = array(
-    'value'    => array('default' => array('default' => $database)),
+    'value'    => array('default' => array('default' => $form_state['storage']['database'])),
     'required' => TRUE,
   );
   $settings['drupal_hash_salt'] = array(
diff --git a/includes/install.inc b/includes/install.inc
index 6e0b7dc7b..2c9810a08 100644
--- a/includes/install.inc
+++ b/includes/install.inc
@@ -230,6 +230,22 @@ function drupal_detect_baseurl($file = 'install.php') {
  *  An array of database types compiled into PHP.
  */
 function drupal_detect_database_types() {
+  $databases = drupal_get_database_types();
+
+  foreach ($databases as $driver => $installer) {
+    $databases[$driver] = $installer->name();
+  }
+
+  return $databases;
+}
+
+/**
+ * Return all supported database installer objects that are compiled into PHP.
+ *
+ * @return
+ *  An array of database installer objects compiled into PHP.
+ */
+function drupal_get_database_types() {
   $databases = array();
 
   // We define a driver as a directory in /includes/database that in turn
@@ -249,7 +265,7 @@ function drupal_detect_database_types() {
     $class = 'DatabaseTasks_' . $driver;
     $installer = new $class();
     if ($installer->installable()) {
-      $databases[$driver] = $installer->name();
+      $databases[$driver] = $installer;
     }
   }
 
@@ -445,7 +461,116 @@ abstract class DatabaseTasks {
       $this->fail(st("The database version %version is less than the minimum required version %minimum_version.", array('%version' => Database::getConnection()->version(), '%minimum_version' => $this->minimumVersion())));
     }
   }
+
+  /**
+   * Return driver specific configuration options.
+   *
+   * @param $database
+   *  An array of driver specific configuration options.
+   *
+   * @return
+   *   The options form array.
+   */
+  public function getFormOptions($database) {
+    $form['database'] = array(
+      '#type' => 'textfield',
+      '#title' => st('Database name'),
+      '#default_value' => empty($database['database']) ? '' : $database['database'],
+      '#size' => 45,
+      '#required' => TRUE,
+      '#description' => st('The name of the database your @drupal data will be stored in. It must exist on your server before @drupal can be installed.', array('@drupal' => drupal_install_profile_distribution_name())),
+    );
+
+    $form['username'] = array(
+      '#type' => 'textfield',
+      '#title' => st('Database username'),
+      '#default_value' => empty($database['username']) ? '' : $database['username'],
+      '#required' => TRUE,
+      '#size' => 45,
+    );
+
+    $form['password'] = array(
+      '#type' => 'password',
+      '#title' => st('Database password'),
+      '#default_value' => empty($database['password']) ? '' : $database['password'],
+      '#required' => FALSE,
+      '#size' => 45,
+    );
+
+    $form['advanced_options'] = array(
+      '#type' => 'fieldset',
+      '#title' => st('Advanced options'),
+      '#collapsible' => TRUE,
+      '#collapsed' => TRUE,
+      '#description' => st("These options are only necessary for some sites. If you're not sure what you should enter here, leave the default settings or check with your hosting provider."),
+      '#weight' => 10,
+    );
+
+    $profile = drupal_get_profile();
+    $db_prefix = ($profile == 'standard') ? 'drupal_' : $profile . '_';
+    $form['advanced_options']['db_prefix'] = array(
+      '#type' => 'textfield',
+      '#title' => st('Table prefix'),
+      '#default_value' => '',
+      '#size' => 45,
+      '#description' => st('If more than one application will be sharing this database, enter a table prefix such as %prefix for your @drupal site here.', array('@drupal' => drupal_install_profile_distribution_name(), '%prefix' => $db_prefix)),
+      '#weight' => 10,
+    );
+
+    $form['advanced_options']['host'] = array(
+      '#type' => 'textfield',
+      '#title' => st('Database host'),
+      '#default_value' => empty($database['host']) ? 'localhost' : $database['host'],
+      '#size' => 45,
+      // Hostnames can be 255 characters long.
+      '#maxlength' => 255,
+      '#required' => TRUE,
+      '#description' => st('If your database is located on a different server, change this.'),
+    );
+
+    $form['advanced_options']['port'] = array(
+      '#type' => 'textfield',
+      '#title' => st('Database port'),
+      '#default_value' => empty($database['port']) ? '' : $database['port'],
+      '#size' => 45,
+      // The maximum port number is 65536, 5 digits.
+      '#maxlength' => 5,
+      '#description' => st('If your database server is listening to a non-standard port, enter its number.'),
+    );
+
+    return $form;
+  }
+
+  /**
+   * Validates driver specific configuration settings.
+   *
+   * Checks to ensure correct basic database settings and that a proper
+   * connection to the database can be established.
+   *
+   * @param $database
+   *   An array of driver specific configuration options.
+   * 
+   * @return
+   *   An array of driver configuration errors, keyed by form element name.
+   */
+  public function validateDatabaseSettings($database) {
+    $errors = array();
+
+    // Verify the table prefix.
+    if (!empty($database['prefix']) && is_string($database['prefix']) && !preg_match('/^[A-Za-z0-9_.]+$/', $database['prefix'])) {
+      $errors[$database['driver'] . '][advanced_options][db_prefix'] = st('The database table prefix you have entered, %prefix, is invalid. The table prefix can only contain alphanumeric characters, periods, or underscores.', array('%prefix' => $database['prefix']));
+    }
+
+    // Verify the database port.
+    if (!empty($database['port']) && !is_numeric($database['port'])) {
+      $errors[$database['driver'] . '][advanced_options][port'] =  st('Database port must be a number.');
+    }
+
+    return $errors;
+  }
+
 }
+
 /**
  * @class Exception class used to throw error if the DatabaseInstaller fails.
  */