diff --git a/core/modules/update/src/Plugin/migrate/source/UpdateSettings.php b/core/modules/update/src/Plugin/migrate/source/UpdateSettings.php index 845baf5d36..4001338476 100644 --- a/core/modules/update/src/Plugin/migrate/source/UpdateSettings.php +++ b/core/modules/update/src/Plugin/migrate/source/UpdateSettings.php @@ -5,7 +5,7 @@ use Drupal\migrate_drupal\Plugin\migrate\source\Variable; /** - * Update setings source plugin. + * Update settings source plugin. * * @MigrateSource( * id = "update_settings", diff --git a/core/modules/update/src/UpdateFetcher.php b/core/modules/update/src/UpdateFetcher.php index 63574e0c7e..015b354a2c 100644 --- a/core/modules/update/src/UpdateFetcher.php +++ b/core/modules/update/src/UpdateFetcher.php @@ -63,7 +63,7 @@ public function __construct(ConfigFactoryInterface $config_factory, ClientInterf $this->httpClient = $http_client; $this->updateSettings = $config_factory->get('update.settings'); if (is_null($settings)) { - @trigger_error('The settings service should be passed to UpdateFetcherr::__construct() since 9.1.0. This will be required in Drupal 10.0.0. See https://www.drupal.org/node/3179315', E_USER_DEPRECATED); + @trigger_error('The settings service should be passed to UpdateFetcher::__construct() since 9.1.0. This will be required in Drupal 10.0.0. See https://www.drupal.org/node/3179315', E_USER_DEPRECATED); $settings = \Drupal::service('settings'); } $this->withHttpFallback = $settings->get('update_fetch_with_http_fallback', FALSE); @@ -93,7 +93,7 @@ public function fetchProjectData(array $project, $site_key = '') { * @return string * The body of the HTTP(S) request, or an empty string on failure. */ - protected function doRequest(string $url, array $options, bool $with_http_fallback) : string { + protected function doRequest(string $url, array $options, bool $with_http_fallback): string { $data = ''; try { $data = (string) $this->httpClient diff --git a/core/modules/update/tests/src/Unit/UpdateFetcherTest.php b/core/modules/update/tests/src/Unit/UpdateFetcherTest.php index 0a68971848..c530848a2d 100644 --- a/core/modules/update/tests/src/Unit/UpdateFetcherTest.php +++ b/core/modules/update/tests/src/Unit/UpdateFetcherTest.php @@ -2,6 +2,8 @@ namespace Drupal\Tests\update\Unit; +use Drupal\Core\Logger\LoggerChannelFactory; +use Drupal\Core\Logger\RfcLoggerTrait; use Drupal\Core\Site\Settings; use Drupal\Tests\UnitTestCase; use Drupal\update\UpdateFetcher; @@ -10,6 +12,7 @@ use GuzzleHttp\HandlerStack; use GuzzleHttp\Middleware; use GuzzleHttp\Psr7\Response; +use Psr\Log\LoggerInterface; /** * Tests update functionality unrelated to the database. @@ -18,7 +21,8 @@ * * @group update */ -class UpdateFetcherTest extends UnitTestCase { +class UpdateFetcherTest extends UnitTestCase implements LoggerInterface { + use RfcLoggerTrait; /** * The update fetcher to use. @@ -55,6 +59,11 @@ class UpdateFetcherTest extends UnitTestCase { */ protected $testProject; + /** + * @var array + */ + protected $logMessages = []; + /** * {@inheritdoc} */ @@ -73,6 +82,17 @@ protected function setUp(): void { ], 'includes' => ['module1' => 'Module 1', 'module2' => 'Module 2'], ]; + + // Set up logger factory so that watchdog_exception() does not break and + // register this class as the logger so we can test messages. + $container = $this->createMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $logger_factory = new LoggerChannelFactory(); + $logger_factory->addLogger($this); + $container->expects($this->any()) + ->method('get') + ->with('logger.factory') + ->will($this->returnValue($logger_factory)); + \Drupal::setContainer($container); } /** @@ -93,6 +113,7 @@ protected function setUp(): void { public function testUpdateBuildFetchUrl(array $project, $site_key, $expected) { $url = $this->updateFetcher->buildFetchUrl($project, $site_key); $this->assertEquals($url, $expected); + $this->assertSame([], $this->logMessages); } /** @@ -147,7 +168,7 @@ public function providerTestUpdateBuildFetchUrl() { /** * Mocks the HTTP client. * - * @param GuzzleHttp\Psr7\Response ...$responses + * @param \GuzzleHttp\Psr7\Response ... * Variable number of Response objects that the mocked client should return. */ protected function mockClient(Response ...$responses) { @@ -167,7 +188,7 @@ public function testUpdateFetcherNoFallback() { // First, try without the HTTP fallback setting, and HTTPS mocked to fail. $settings = new Settings([]); $this->mockClient( - new Response('500', [], 'https failed'), + new Response('500', [], 'HTTPS failed'), ); $update_fetcher = new UpdateFetcher($this->mockConfigFactory, $this->mockHttpClient, $settings); @@ -176,12 +197,13 @@ public function testUpdateFetcherNoFallback() { $this->assertCount(1, $this->history); $request = $this->history[0]['request']; $this->assertNotEmpty($request); - // It should have only been an https request. + // It should have only been an HTTPS request. $this->assertEquals('https', $request->getUri()->getScheme()); // And it should have failed. $response = $this->history[0]['response']; $this->assertEquals(500, $response->getStatusCode()); $this->assertEmpty($data); + $this->assertSame(["Server error: `GET https://www.example.com/update_test/current` resulted in a `500 Internal Server Error` response:\nHTTPS failed\n"], $this->logMessages); } /** @@ -191,8 +213,8 @@ public function testUpdateFetcherNoFallback() { public function testUpdateFetcherHttpFallback() { $settings = new Settings(['update_fetch_with_http_fallback' => TRUE]); $this->mockClient( - new Response('500', [], 'https failed'), - new Response('200', [], 'http worked'), + new Response('500', [], 'HTTPS failed'), + new Response('200', [], 'HTTP worked'), ); $update_fetcher = new UpdateFetcher($this->mockConfigFactory, $this->mockHttpClient, $settings); @@ -201,37 +223,28 @@ public function testUpdateFetcherHttpFallback() { // There should be two request / response pairs. $this->assertCount(2, $this->history); - // The first should have been https and should have failed. + // The first should have been HTTPS and should have failed. $first_try = $this->history[0]; $this->assertNotEmpty($first_try); $this->assertEquals('https', $first_try['request']->getUri()->getScheme()); $this->assertEquals(500, $first_try['response']->getStatusCode()); - // The second should have been the http fallback and should have worked. + // The second should have been the HTTP fallback and should have worked. $second_try = $this->history[1]; $this->assertNotEmpty($second_try); $this->assertEquals('http', $second_try['request']->getUri()->getScheme()); $this->assertEquals(200, $second_try['response']->getStatusCode()); // Although this is a bogus mocked response, it's what fetchProjectData() // should return in this case. - $this->assertEquals('http worked', $data); + $this->assertEquals('HTTP worked', $data); + $this->assertSame(["Server error: `GET https://www.example.com/update_test/current` resulted in a `500 Internal Server Error` response:\nHTTPS failed\n"], $this->logMessages); } -} + /** + * {@inheritdoc} + */ + public function log($level, $message, array $context = []) { + $this->logMessages[] = $context['@message']; + } -namespace Drupal\update; -use Drupal\Core\Logger\RfcLogLevel; -/** - * Mock version of watchdog_exception(). - * - * This is unfortunate, but UpdateFetcher::doRequest() calls - * watchdog_exception(), which is defined in includes/bootstrap.inc, which - * cannot be included for Unit tests or we break unit test encapsulation. - * Therefore, we define our own copy in the \Drupal\update namespace that - * doesn't do anything (we're not trying to test the exception logging). - * - * @todo Remove in https://www.drupal.org/project/drupal/issues/2932518 - */ -function watchdog_exception($type, \Exception $exception, $message = NULL, $variables = [], $severity = RfcLogLevel::ERROR, $link = NULL) { - // No-op. }