Skip to content

Commit

Permalink
Issue #951644 by catch, David_Rothstein, aschiwi, xjm: Fixed Requirem…
Browse files Browse the repository at this point in the history 
…ent warnings (e.g. for PHP memory limit) are not shown on install or update unless there is a requirement error also.
  • Loading branch information
@webchick
webchick committed Nov 8, 2011
1 parent e8abf8f commit 5646753
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 19 deletions.
22 changes: 16 additions & 6 deletions includes/install.core.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,10 @@ function install_display_output($output, $install_state) {
*
* @return
* A themed status report, or an exception if there are requirement errors.
* Otherwise, no output is returned, so that the next task can be run
* in the same page request.
* If there are only requirement warnings, a themed status report is shown
* initially, but the user is allowed to bypass it by providing 'continue=1'
* in the URL. Otherwise, no output is returned, so that the next task can be
* run in the same page request.
*/
function install_verify_requirements(&$install_state) {
// Check the installation requirements for Drupal and this profile.
Expand All @@ -723,22 +725,30 @@ function install_verify_requirements(&$install_state) {
// Check the severity of the requirements reported.
$severity = drupal_requirements_severity($requirements);

if ($severity == REQUIREMENT_ERROR) {
// If there are errors, always display them. If there are only warnings, skip
// them if the user has provided a URL parameter acknowledging the warnings
// and indicating a desire to continue anyway. See drupal_requirements_url().
if ($severity == REQUIREMENT_ERROR || ($severity == REQUIREMENT_WARNING && empty($install_state['parameters']['continue']))) {
if ($install_state['interactive']) {
drupal_set_title(st('Requirements problem'));
$status_report = theme('status_report', array('requirements' => $requirements));
$status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(request_uri())));
$status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(drupal_requirements_url($severity))));
return $status_report;
}
else {
// Throw an exception showing all unmet requirements.
// Throw an exception showing any unmet requirements.
$failures = array();
foreach ($requirements as $requirement) {
// Skip warnings altogether for non-interactive installations; these
// proceed in a single request so there is no good opportunity (and no
// good method) to warn the user anyway.
if (isset($requirement['severity']) && $requirement['severity'] == REQUIREMENT_ERROR) {
$failures[] = $requirement['title'] . ': ' . $requirement['value'] . "\n\n" . $requirement['description'];
}
}
throw new Exception(implode("\n\n", $failures));
if (!empty($failures)) {
throw new Exception(implode("\n\n", $failures));
}
}
}
}
Expand Down
63 changes: 62 additions & 1 deletion includes/install.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,6 @@ function drupal_install_fix_file($file, $mask, $message = TRUE) {
}
}


/**
* Send the user to a different installer page.
*
Expand All @@ -1016,6 +1015,68 @@ function install_goto($path) {
drupal_exit();
}

/**
* Returns the URL of the current script, with modified query parameters.
*
* This function can be called by low-level scripts (such as install.php and
* update.php) and returns the URL of the current script. Existing query
* parameters are preserved by default, but new ones can optionally be merged
* in.
*
* This function is used when the script must maintain certain query parameters
* over multiple page requests in order to work correctly. In such cases (for
* example, update.php, which requires the 'continue=1' parameter to remain in
* the URL throughout the update process if there are any requirement warnings
* that need to be bypassed), using this function to generate the URL for links
* to the next steps of the script ensures that the links will work correctly.
*
* @param $query
* (optional) An array of query parameters to merge in to the existing ones.
*
* @return
* The URL of the current script, with query parameters modified by the
* passed-in $query. The URL is not sanitized, so it still needs to be run
* through check_url() if it will be used as an HTML attribute value.
*
* @see drupal_requirements_url()
*/
function drupal_current_script_url($query = array()) {
$uri = $_SERVER['SCRIPT_NAME'];
$query = array_merge(drupal_get_query_parameters(), $query);
if (!empty($query)) {
$uri .= '?' . drupal_http_build_query($query);
}
return $uri;
}

/**
* Returns a URL for proceeding to the next page after a requirements problem.
*
* This function can be called by low-level scripts (such as install.php and
* update.php) and returns a URL that can be used to attempt to proceed to the
* next step of the script.
*
* @param $severity
* The severity of the requirements problem, as returned by
* drupal_requirements_severity().
*
* @return
* A URL for attempting to proceed to the next step of the script. The URL is
* not sanitized, so it still needs to be run through check_url() if it will
* be used as an HTML attribute value.
*
* @see drupal_current_script_url()
*/
function drupal_requirements_url($severity) {
$query = array();
// If there are no errors, only warnings, append 'continue=1' to the URL so
// the user can bypass this screen on the next page load.
if ($severity == REQUIREMENT_WARNING) {
$query['continue'] = 1;
}
return drupal_current_script_url($query);
}

/**
* Functional equivalent of t(), used when some systems are not available.
*
Expand Down
45 changes: 44 additions & 1 deletion modules/system/system.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2086,7 +2086,7 @@ class UpdateScriptFunctionalTest extends DrupalWebTestCase {
}

function setUp() {
parent::setUp();
parent::setUp('update_script_test');
$this->update_url = $GLOBALS['base_url'] . '/update.php';
$this->update_user = $this->drupalCreateUser(array('administer software updates'));
}
Expand Down Expand Up @@ -2122,6 +2122,49 @@ class UpdateScriptFunctionalTest extends DrupalWebTestCase {
$this->assertResponse(200);
}

/**
* Tests that requirements warnings and errors are correctly displayed.
*/
function testRequirements() {
$this->drupalLogin($this->update_user);

// If there are no requirements warnings or errors, we expect to be able to
// go through the update process uninterrupted.
$this->drupalGet($this->update_url, array('external' => TRUE));
$this->drupalPost(NULL, array(), t('Continue'));
$this->assertText(t('No pending updates.'), t('End of update process was reached.'));

// If there is a requirements warning, we expect it to be initially
// displayed, but clicking the link to proceed should allow us to go
// through the rest of the update process uninterrupted. (First run this
// test with pending updates to make sure they can be run successfully;
// then try again without pending updates to make sure that works too.)
variable_set('update_script_test_requirement_type', REQUIREMENT_WARNING);
drupal_set_installed_schema_version('update_script_test', drupal_get_installed_schema_version('update_script_test') - 1);
$this->drupalGet($this->update_url, array('external' => TRUE));
$this->assertText('This is a requirements warning provided by the update_script_test module.');
$this->clickLink('try again');
$this->assertNoText('This is a requirements warning provided by the update_script_test module.');
$this->drupalPost(NULL, array(), t('Continue'));
$this->drupalPost(NULL, array(), t('Apply pending updates'));
$this->assertText(t('The update_script_test_update_7000() update was executed successfully.'), t('End of update process was reached.'));
$this->drupalGet($this->update_url, array('external' => TRUE));
$this->assertText('This is a requirements warning provided by the update_script_test module.');
$this->clickLink('try again');
$this->assertNoText('This is a requirements warning provided by the update_script_test module.');
$this->drupalPost(NULL, array(), t('Continue'));
$this->assertText(t('No pending updates.'), t('End of update process was reached.'));

// If there is a requirements error, it should be displayed even after
// clicking the link to proceed (since the problem that triggered the error
// has not been fixed).
variable_set('update_script_test_requirement_type', REQUIREMENT_ERROR);
$this->drupalGet($this->update_url, array('external' => TRUE));
$this->assertText('This is a requirements error provided by the update_script_test module.');
$this->clickLink('try again');
$this->assertText('This is a requirements error provided by the update_script_test module.');
}

/**
* Tests the effect of using the update script on the theme system.
*/
Expand Down
39 changes: 28 additions & 11 deletions update.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ function update_info_page() {
$output .= "<li>Install your new files in the appropriate location, as described in the handbook.</li>\n";
$output .= "</ol>\n";
$output .= "<p>When you have performed the steps above, you may proceed.</p>\n";
$output .= '<form method="post" action="update.php?op=selection&amp;token=' . $token . '"><p><input type="submit" value="Continue" class="form-submit" /></p></form>';
$form_action = check_url(drupal_current_script_url(array('op' => 'selection', 'token' => $token)));
$output .= '<form method="post" action="' . $form_action . '"><p><input type="submit" value="Continue" class="form-submit" /></p></form>';
$output .= "\n";
return $output;
}
Expand Down Expand Up @@ -316,20 +317,26 @@ function update_extra_requirements($requirements = NULL) {
}

/**
* Check update requirements and report any errors.
* Check update requirements and report any errors or (optionally) warnings.
*
* @param $skip_warnings
* (optional) If set to TRUE, requirement warnings will be ignored, and a
* report will only be issued if there are requirement errors. Defaults to
* FALSE.
*/
function update_check_requirements() {
function update_check_requirements($skip_warnings = FALSE) {
// Check requirements of all loaded modules.
$requirements = module_invoke_all('requirements', 'update');
$requirements += update_extra_requirements();
$severity = drupal_requirements_severity($requirements);

// If there are issues, report them.
if ($severity == REQUIREMENT_ERROR) {
// If there are errors, always display them. If there are only warnings, skip
// them if the caller has indicated they should be skipped.
if ($severity == REQUIREMENT_ERROR || ($severity == REQUIREMENT_WARNING && !$skip_warnings)) {
update_task_list('requirements');
drupal_set_title('Requirements problem');
$status_report = theme('status_report', array('requirements' => $requirements));
$status_report .= 'Check the error messages and <a href="' . check_url(request_uri()) . '">try again</a>.';
$status_report .= 'Check the error messages and <a href="' . check_url(drupal_requirements_url($severity)) . '">try again</a>.';
print theme('update_page', array('content' => $status_report));
exit();
}
Expand Down Expand Up @@ -385,8 +392,9 @@ function update_check_requirements() {
// Set up theme system for the maintenance page.
drupal_maintenance_theme();

// Check the update requirements for Drupal.
update_check_requirements();
// Check the update requirements for Drupal. Only report on errors at this
// stage, since the real requirements check happens further down.
update_check_requirements(TRUE);

// Redirect to the update information page if all requirements were met.
install_goto('update.php?op=info');
Expand Down Expand Up @@ -418,8 +426,12 @@ function update_check_requirements() {

update_fix_compatibility();

// Check the update requirements for all modules.
update_check_requirements();
// Check the update requirements for all modules. If there are warnings, but
// no errors, skip reporting them if the user has provided a URL parameter
// acknowledging the warnings and indicating a desire to continue anyway. See
// drupal_requirements_url().
$skip_warnings = !empty($_GET['continue']);
update_check_requirements($skip_warnings);

$op = isset($_REQUEST['op']) ? $_REQUEST['op'] : '';
switch ($op) {
Expand All @@ -433,7 +445,12 @@ function update_check_requirements() {

case 'Apply pending updates':
if (isset($_GET['token']) && $_GET['token'] == drupal_get_token('update')) {
update_batch($_POST['start'], $base_url . '/update.php?op=results', $base_url . '/update.php');
// Generate absolute URLs for the batch processing (using $base_root),
// since the batch API will pass them to url() which does not handle
// update.php correctly by default.
$batch_url = $base_root . drupal_current_script_url();
$redirect_url = $base_root . drupal_current_script_url(array('op' => 'results'));
update_batch($_POST['start'], $redirect_url, $batch_url);
break;
}

Expand Down

0 comments on commit 5646753

Please sign in to comment.