Skip to content

Commit

Permalink
Avoid run of Request handle() and use of container at compiler pass t…
Browse files Browse the repository at this point in the history
…o fix issues 3091 and 3553 (#3558)

* Fixes of Drupal Console issues due missing session and required contexts of global displayed blocks when a general request is processed at Bootstrap/DrupalKernel. Also fixed major exceptions caused by AddServicesCompilerPass compiler pass class because is using container services when container is still on build phase, here is reduced the use of container but still there is code to refactor.

* Fixed problems of undefined modules and legacy libraries functions that was not loaded and many commands as: 'user:login:url, site:status or cache:rebuild' was failing.
  • Loading branch information
citlacom authored and jmolivas committed Nov 10, 2017
1 parent 9b0e7fe commit 3c4395b
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 11 deletions.
34 changes: 32 additions & 2 deletions src/Bootstrap/AddServicesCompilerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use Drupal\Console\Utils\TranslatorManager;
use Drupal\Console\Extension\Extension;
use Drupal\Console\Extension\Manager;
use Drupal\Core\Cache\ListCacheBinsPass;
use GuzzleHttp\Client;

/**
* FindCommandsCompilerPass
Expand Down Expand Up @@ -77,15 +79,35 @@ public function process(ContainerBuilder $container)
* @var Site $site
*/
$site = $container->get('console.site');
\Drupal::getContainer()->set(
$container->set(
'console.root',
$this->root
);

// The AddServicesCompilerPass cache pass is executed before the
// ListCacheBinsPass causing exception: ParameterNotFoundException: You
// have requested a non-existent parameter "cache_default_bin_backends"
$cache_pass = new ListCacheBinsPass();
$cache_pass->process($container);

// Fix ContainerNotInitializedException: \Drupal::$container is not initialized yet.
// \Drupal::setContainer() must be called with a real container. The use of stream_wrapper.temporary
// service at cachedServicesFileExists method uses the container that in compiler pass
// is not ready to use: https://github.com/symfony/symfony/issues/22125#issuecomment-288734689
// This is workaroud works but seems that we cannot depend on services at this stage.
\Drupal::setContainer($container);

// Avoid Error: Call to undefined function
// Drupal\Core\StreamWrapper\file_directory_temp() in TemporaryStream
// that seems to be needed to the cache services and when no other
// stream wrapper is available defaults ont TemporaryStream.
$site->loadLegacyFile('/core/includes/file.inc');

if (!$this->rebuild && $site->cachedServicesFileExists()) {
$loader->load($site->getCachedServicesFile());
} else {
$site->removeCachedServicesFile();

$finder = new Finder();
$finder->files()
->name('*.yml')
Expand All @@ -105,10 +127,18 @@ public function process(ContainerBuilder $container)
);
}

// Avoid to use the container to get the console.extension_manager due
// container is not ready and cause exceptions.

/**
* @var GuzzleHttp\Client $httpClient
*/
$httpClient = new Client;
/**
* @var Manager $extensionManager
*/
$extensionManager = $container->get('console.extension_manager');
$extensionManager = new Manager($site, $httpClient, $this->appRoot);

/**
* @var Extension[] $modules
*/
Expand Down
21 changes: 20 additions & 1 deletion src/Bootstrap/Drupal.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use Drupal\Console\Core\Utils\ArgvInputReader;
use Drupal\Console\Core\Bootstrap\DrupalConsoleCore;
use Drupal\Console\Core\Utils\DrupalFinder;
use Drupal\Component\FileCache\FileCacheFactory;
use Drupal\Core\Site\Settings;

class Drupal
{
Expand Down Expand Up @@ -143,9 +145,26 @@ public function boot()
$io->writeln("\r\033[K\033[1A\r<info>✔</info>");
$io->writeln('➤ Rebuilding container');
}

// Fix an exception of FileCacheFactory not prefix not set when
// container is build and looks that as we depend on cache for
// AddServicesCompilerPass but container is not ready this prefix
// needs to be set manually to allow use of the cache files.
FileCacheFactory::setPrefix($this->drupalFinder->getDrupalRoot());

// Invalidate container to ensure rebuild of any cached state
// when boot is processed.
$drupalKernel->invalidateContainer();
$drupalKernel->rebuildContainer();

// Looks that the boot process is handling an initializeContainer
// so looks that rebuildContainer repeats what we finally do in boot().
//$drupalKernel->rebuildContainer();

// Load legacy libraries, modules, register stream wrapper, and push
// request to request stack but without trigger processing of '/'
// request that invokes hooks like hook_page_attachments().
$drupalKernel->boot();
$drupalKernel->preHandle($request);

if ($debug) {
$io->writeln("\r\033[K\033[1A\r<info>✔</info>");
Expand Down
12 changes: 11 additions & 1 deletion src/Bootstrap/DrupalKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,17 @@ public static function createFromRequest(Request $request, $class_loader, $envir
$kernel = new static($environment, $class_loader, $allow_dumping, $app_root);
static::bootEnvironment($app_root);
$kernel->initializeSettings($request);
$kernel->handle($request);
// Calling the request handle causes that a page request "/" is
// processed for any console execution even: help or --version and
// with sites that have globally displayed blocks contexts are not
// ready for blocks plugins so this causes lot of problems like:
// https://github.com/hechoendrupal/drupal-console/issues/3091 and
// https://github.com/hechoendrupal/drupal-console/issues/3553 Also
// handle does a initializeContainer which originally was invalidated
// and rebuild at Console Drupal Bootstrap. By disabling handle
// and processing the boot() at Bootstrap commands that do not
// depend on requests works well.
//$kernel->handle($request);
return $kernel;
}

Expand Down
29 changes: 22 additions & 7 deletions src/Extension/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ private function getExtensions(
$extensions[$name] = $extension;
}


return $nameOnly?array_keys($extensions):$extensions;
}

Expand All @@ -221,11 +220,6 @@ private function getExtensions(
*/
private function discoverExtensions($type)
{
if ($type === 'module') {
$this->site->loadLegacyFile('/core/modules/system/system.module');
system_rebuild_module_data();
}

if ($type === 'theme') {
$themeHandler = \Drupal::service('theme_handler');
$themeHandler->rebuildThemeData();
Expand All @@ -237,8 +231,29 @@ private function discoverExtensions($type)
*/
$discovery = new Discovery($this->appRoot);
$discovery->reset();
$extensions = $discovery->scan($type);

if ($type === 'module') {
// Using system_rebuild_module_data causes an error:
// Constructing service "logger.factory" from a parent definition is not supported at build time.
//$this->site->loadLegacyFile('/core/modules/system/system.module');
//system_rebuild_module_data();

// Looks that dependency on rebuild module data is just to determine
// the installed status so alternatively we can just look on installed
// modules config and apply to discovered extensions.
$installed_modules = \Drupal::config('core.extension')->get('module') ?: [];

/**
* @var \Drupal\Core\Extension\Extension $extension
*/
foreach ($extensions as $name => $extension) {
$extensions[$name]->weight = isset($installed_modules[$name]) ? $installed_modules[$name] : 0;
$extensions[$name]->status = (int) isset($installed_modules[$name]);
}
}

return $discovery->scan($type);
return $extensions;
}

/**
Expand Down

0 comments on commit 3c4395b

Please sign in to comment.