Issue #2548511 by David_Rothstein: Write tests for the Update Manager...

Issue #2548511 by David_Rothstein: Write tests for the Update Manager downloading and applying updates for a module

core/lib/Drupal/Core/Extension/InfoParser.php

@@ -7,14 +7,10 @@
 
 namespace Drupal\Core\Extension;
 
-use Drupal\Component\Serialization\Yaml;
-use Drupal\Component\Serialization\Exception\InvalidDataTypeException;
-use Drupal\Component\Utility\SafeMarkup;
-
 /**
  * Parses extension .info.yml files.
  */
-class InfoParser implements InfoParserInterface {
+class InfoParser extends InfoParserDynamic {
 
   /**
    * Array of all info keyed by filename.
@@ -28,38 +24,9 @@ class InfoParser implements InfoParserInterface {
    */
   public function parse($filename) {
     if (!isset(static::$parsedInfos[$filename])) {
-      if (!file_exists($filename)) {
-        static::$parsedInfos[$filename] = array();
-      }
-      else {
-        try {
-          static::$parsedInfos[$filename] = Yaml::decode(file_get_contents($filename));
-        }
-        catch (InvalidDataTypeException $e) {
-          $message = SafeMarkup::format("Unable to parse !file: !error", array('!file' => $filename, '!error' => $e->getMessage()));
-          throw new InfoParserException($message);
-        }
-        $missing_keys = array_diff($this->getRequiredKeys(), array_keys(static::$parsedInfos[$filename]));
-        if (!empty($missing_keys)) {
-          $message = SafeMarkup::format('Missing required keys (!missing_keys) in !file.', array('!missing_keys' => implode(', ', $missing_keys), '!file' => $filename));
-          throw new InfoParserException($message);
-        }
-        if (isset(static::$parsedInfos[$filename]['version']) && static::$parsedInfos[$filename]['version'] === 'VERSION') {
-          static::$parsedInfos[$filename]['version'] = \Drupal::VERSION;
-        }
-      }
+      static::$parsedInfos[$filename] = parent::parse($filename);
     }
     return static::$parsedInfos[$filename];
   }
 
-  /**
-   * Returns an array of keys required to exist in .info.yml file.
-   *
-   * @return array
-   *   An array of required keys.
-   */
-  protected function getRequiredKeys() {
-    return array('type', 'core', 'name');
-  }
-
 }

core/lib/Drupal/Core/Extension/InfoParserDynamic.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Extension\InfoParserDynamic.
+ */
+
+namespace Drupal\Core\Extension;
+
+use Drupal\Component\Serialization\Yaml;
+use Drupal\Component\Serialization\Exception\InvalidDataTypeException;
+use Drupal\Component\Utility\SafeMarkup;
+
+/**
+ * Parses dynamic .info.yml files that might change during the page request.
+ */
+class InfoParserDynamic implements InfoParserInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function parse($filename) {
+    if (!file_exists($filename)) {
+      $parsed_info = array();
+    }
+    else {
+      try {
+        $parsed_info = Yaml::decode(file_get_contents($filename));
+      }
+      catch (InvalidDataTypeException $e) {
+        $message = SafeMarkup::format("Unable to parse !file: !error", array('!file' => $filename, '!error' => $e->getMessage()));
+        throw new InfoParserException($message);
+      }
+      $missing_keys = array_diff($this->getRequiredKeys(), array_keys($parsed_info));
+      if (!empty($missing_keys)) {
+        $message = SafeMarkup::format('Missing required keys (!missing_keys) in !file.', array('!missing_keys' => implode(', ', $missing_keys), '!file' => $filename));
+        throw new InfoParserException($message);
+      }
+      if (isset($parsed_info['version']) && $parsed_info['version'] === 'VERSION') {
+        $parsed_info['version'] = \Drupal::VERSION;
+      }
+    }
+    return $parsed_info;
+  }
+
+  /**
+   * Returns an array of keys required to exist in .info.yml file.
+   *
+   * @return array
+   *   An array of required keys.
+   */
+  protected function getRequiredKeys() {
+    return array('type', 'core', 'name');
+  }
+
+}

core/lib/Drupal/Core/Updater/Module.php

@@ -18,26 +18,27 @@ class Module extends Updater implements UpdaterInterface {
   /**
    * Returns the directory where a module should be installed.
    *
-   * If the module is already installed, drupal_get_path() will return
-   * a valid path and we should install it there (although we need to use an
-   * absolute path, so we prepend the root path). If we're installing a new
-   * module, we always want it to go into /modules, since that's
-   * where all the documentation recommends users install their modules, and
-   * there's no way that can conflict on a multi-site installation, since
-   * the Update manager won't let you install a new module if it's already
-   * found on your system, and if there was a copy in the top-level we'd see it.
+   * If the module is already installed, drupal_get_path() will return a valid
+   * path and we should install it there. If we're installing a new module, we
+   * always want it to go into /modules, since that's where all the
+   * documentation recommends users install their modules, and there's no way
+   * that can conflict on a multi-site installation, since the Update manager
+   * won't let you install a new module if it's already found on your system,
+   * and if there was a copy in the top-level we'd see it.
    *
    * @return string
-   *   A directory path.
+   *   The absolute path of the directory.
    */
   public function getInstallDirectory() {
     if ($this->isInstalled() && ($relative_path = drupal_get_path('module', $this->name))) {
-      $relative_path = dirname($relative_path);
+      // The return value of drupal_get_path() is always relative to the site,
+      // so prepend DRUPAL_ROOT.
+      return DRUPAL_ROOT . '/' . dirname($relative_path);
     }
     else {
-      $relative_path = $this->getRootDirectoryRelativePath();
+      // When installing a new module, prepend the requested root directory.
+      return $this->root . '/' . $this->getRootDirectoryRelativePath();
     }
-    return $this->root . '/' . $relative_path;
   }
 
   /**

core/lib/Drupal/Core/Updater/Theme.php

@@ -18,26 +18,27 @@ class Theme extends Updater implements UpdaterInterface {
   /**
    * Returns the directory where a theme should be installed.
    *
-   * If the theme is already installed, drupal_get_path() will return
-   * a valid path and we should install it there (although we need to use an
-   * absolute path, so we prepend the root path). If we're installing a new
-   * theme, we always want it to go into /themes, since that's
-   * where all the documentation recommends users install their themes, and
-   * there's no way that can conflict on a multi-site installation, since
-   * the Update manager won't let you install a new theme if it's already
-   * found on your system, and if there was a copy in the top-level we'd see it.
+   * If the theme is already installed, drupal_get_path() will return a valid
+   * path and we should install it there. If we're installing a new theme, we
+   * always want it to go into /themes, since that's where all the
+   * documentation recommends users install their themes, and there's no way
+   * that can conflict on a multi-site installation, since the Update manager
+   * won't let you install a new theme if it's already found on your system,
+   * and if there was a copy in the top-level we'd see it.
    *
    * @return string
-   *   A directory path.
+   *   The absolute path of the directory.
    */
   public function getInstallDirectory() {
     if ($this->isInstalled() && ($relative_path = drupal_get_path('theme', $this->name))) {
-      $relative_path = dirname($relative_path);
+      // The return value of drupal_get_path() is always relative to the site,
+      // so prepend DRUPAL_ROOT.
+      return DRUPAL_ROOT . '/' . dirname($relative_path);
     }
     else {
-      $relative_path = $this->getRootDirectoryRelativePath();
+      // When installing a new theme, prepend the requested root directory.
+      return $this->root . '/' . $this->getRootDirectoryRelativePath();
     }
-    return $this->root . '/' . $relative_path;
   }
 
   /**

core/modules/update/src/Tests/UpdateUploadTest.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\update\Tests;
 
+use Drupal\Core\Extension\InfoParserDynamic;
 use Drupal\Core\Updater\Updater;
 use Drupal\Core\Url;
 
@@ -26,12 +27,12 @@ class UpdateUploadTest extends UpdateTestBase {
 
   protected function setUp() {
     parent::setUp();
-    $admin_user = $this->drupalCreateUser(array('administer software updates', 'administer site configuration'));
+    $admin_user = $this->drupalCreateUser(array('administer modules', 'administer software updates', 'administer site configuration'));
     $this->drupalLogin($admin_user);
   }
 
   /**
-   * Tests upload and extraction of a module.
+   * Tests upload, extraction, and update of a module.
    */
   public function testUploadModule() {
     // Images are not valid archives, so get one and try to install it. We
@@ -64,7 +65,7 @@ public function testUploadModule() {
     $moduleUpdater = $updaters['module']['class'];
     $installedInfoFilePath = $this->container->get('update.root') . '/' . $moduleUpdater::getRootDirectoryRelativePath() . '/update_test_new_module/update_test_new_module.info.yml';
     $this->assertFalse(file_exists($installedInfoFilePath), 'The new module does not exist in the filesystem before it is installed with the Update Manager.');
-    $validArchiveFile = drupal_get_path('module', 'update') . '/tests/update_test_new_module.tar.gz';
+    $validArchiveFile = drupal_get_path('module', 'update') . '/tests/update_test_new_module/8.x-1.0/update_test_new_module.tar.gz';
     $edit = array(
       'files[project_upload]' => $validArchiveFile,
     );
@@ -87,6 +88,43 @@ public function testUploadModule() {
     $this->clickLink(t('Install another module'));
     $this->assertResponse(200);
     $this->assertUrl('admin/modules/install');
+
+    // Check that the module has the correct version before trying to update
+    // it. Since the module is installed in sites/simpletest, which only the
+    // child site has access to, standard module API functions won't find it
+    // when called here. To get the version, the info file must be parsed
+    // directly instead.
+    $info_parser = new InfoParserDynamic();
+    $info = $info_parser->parse($installedInfoFilePath);
+    $this->assertEqual($info['version'], '8.x-1.0');
+
+    // Enable the module.
+    $this->drupalPostForm('admin/modules', array('modules[Testing][update_test_new_module][enable]' => TRUE), t('Install'));
+
+    // Define the update XML such that the new module downloaded above needs an
+    // update from 8.x-1.0 to 8.x-1.1.
+    $update_test_config = $this->config('update_test.settings');
+    $system_info = array(
+      'update_test_new_module' => array(
+        'project' => 'update_test_new_module',
+      ),
+    );
+    $update_test_config->set('system_info', $system_info)->save();
+    $xml_mapping = array(
+      'update_test_new_module' => '1_1',
+    );
+    $this->refreshUpdateStatus($xml_mapping);
+
+    // Run the updates for the new module.
+    $this->drupalPostForm('admin/reports/updates/update', array('projects[update_test_new_module]' => TRUE), t('Download these updates'));
+    $this->drupalPostForm(NULL, array('maintenance_mode' => FALSE), t('Continue'));
+    $this->assertText(t('Update was completed successfully.'));
+    $this->assertRaw(t('Installed %project_name successfully', array('%project_name' => 'update_test_new_module')));
+
+    // Parse the info file again to check that the module has been updated to
+    // 8.x-1.1.
+    $info = $info_parser->parse($installedInfoFilePath);
+    $this->assertEqual($info['version'], '8.x-1.1');
   }
 
   /**

core/modules/update/tests/modules/update_test/update_test_new_module.1_1.xml

+<?xml version="1.0" encoding="utf-8"?>
+<project xmlns:dc="http://purl.org/dc/elements/1.1/">
+<title>Update test new module</title>
+<short_name>update_test_new_module</short_name>
+<dc:creator>Drupal</dc:creator>
+<api_version>8.x</api_version>
+<recommended_major>1</recommended_major>
+<supported_majors>1</supported_majors>
+<default_major>1</default_major>
+<project_status>published</project_status>
+<link>http://example.com/project/update_test_new_module</link>
+  <terms>
+   <term><name>Projects</name><value>Modules</value></term>
+  </terms>
+<releases>
+ <release>
+  <name>update_test_new_module 8.x-1.1</name>
+  <version>8.x-1.1</version>
+  <tag>DRUPAL-8--1-1</tag>
+  <version_major>1</version_major>
+  <version_patch>1</version_patch>
+  <status>published</status>
+  <release_link>http://example.com/update_test_new_module-8-x-1-1-release</release_link>
+  <download_link>core/modules/update/tests/update_test_new_module/8.x-1.1/update_test_new_module.tar.gz</download_link>
+  <date>1300424521</date>
+  <mdhash>b966255555d9c9b86d480ca08cfaa98e</mdhash>
+  <filesize>1073741824</filesize>
+  <terms>
+   <term><name>Release type</name><value>New features</value></term>
+   <term><name>Release type</name><value>Bug fixes</value></term>
+  </terms>
+ </release>
+</releases>
+</project>

core/modules/update/update.manager.inc

@@ -206,7 +206,7 @@ function update_manager_archive_verify($project, $archive_file, $directory) {
 function update_manager_file_get($url) {
   $parsed_url = parse_url($url);
   $remote_schemes = array('http', 'https', 'ftp', 'ftps', 'smb', 'nfs');
-  if (!in_array($parsed_url['scheme'], $remote_schemes)) {
+  if (!isset($parsed_url['scheme']) || !in_array($parsed_url['scheme'], $remote_schemes)) {
     // This is a local file, just return the path.
     return drupal_realpath($url);
   }