Skip to content

Log undeclared plugin only if it is not disabled #40081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@
<item name="primary" xsi:type="string">primary</item>
<item name="first" xsi:type="string">global</item>
</argument>
<argument name="appMode" xsi:type="init_parameter">Magento\Framework\App\State::PARAM_MODE</argument>
</arguments>
</type>
<type name="Magento\Framework\App\ResourceConnection">
Expand Down
135 changes: 36 additions & 99 deletions lib/internal/Magento/Framework/Interception/PluginListGenerator.php
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
* Copyright 2020 Adobe
* All Rights Reserved.
*/
declare(strict_types=1);

namespace Magento\Framework\Interception;

use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\State;
use Magento\Framework\Config\ReaderInterface;
use Magento\Framework\Config\ScopeInterface;
use Magento\Framework\Exception\FileSystemException;
use Magento\Framework\Interception\ObjectManager\ConfigInterface;
use Magento\Framework\ObjectManager\DefinitionInterface as ClassDefinitions;
use Magento\Framework\ObjectManager\RelationsInterface;
Expand All @@ -20,94 +23,37 @@
*/
class PluginListGenerator implements ConfigWriterInterface, ConfigLoaderInterface
{
/**
* @var ScopeInterface
*/
private $scopeConfig;

/**
* Configuration reader
*
* @var ReaderInterface
*/
private $reader;

/**
* Cache tag
*
* @var string
*/
private $cacheId = 'plugin-list';

/**
* @var array
*/
private $loadedScopes = [];

/**
* Type config
*
* @var ConfigInterface
*/
private $omConfig;

/**
* Class relations information provider
*
* @var RelationsInterface
*/
private $relations;

/**
* List of interception methods per plugin
*
* @var DefinitionInterface
*/
private $definitions;

/**
* List of interceptable application classes
*
* @var ClassDefinitions
*/
private $classDefinitions;
private string $cacheId = 'plugin-list';

/**
* @var LoggerInterface
*/
private $logger;

/**
* @var DirectoryList
* @var string[]
*/
private $directoryList;
private array $loadedScopes = [];

/**
* @var array
*/
private $pluginData;
private array $pluginData = [];

/**
* @var array
*/
private $inherited = [];
private array $inherited = [];

/**
* @var array
*/
private $processed;

/**
* Scope priority loading scheme
*
* @var string[]
*/
private $scopePriorityScheme;
private array $processed = [];

/**
* @var array
*/
private $globalScopePluginData = [];
private array $globalScopePluginData = [];

/**
* @param ReaderInterface $reader
Expand All @@ -118,28 +64,21 @@ class PluginListGenerator implements ConfigWriterInterface, ConfigLoaderInterfac
* @param ClassDefinitions $classDefinitions
* @param LoggerInterface $logger
* @param DirectoryList $directoryList
* @param array $scopePriorityScheme
* @param array $scopePriorityScheme [optional]
* @param string $appMode [optional]
*/
public function __construct(
ReaderInterface $reader,
ScopeInterface $scopeConfig,
ConfigInterface $omConfig,
RelationsInterface $relations,
DefinitionInterface $definitions,
ClassDefinitions $classDefinitions,
LoggerInterface $logger,
DirectoryList $directoryList,
array $scopePriorityScheme = ['global']
private ReaderInterface $reader,
private ScopeInterface $scopeConfig,
private ConfigInterface $omConfig,
private RelationsInterface $relations,
private DefinitionInterface $definitions,
private ClassDefinitions $classDefinitions,
private LoggerInterface $logger,
private DirectoryList $directoryList,
private array $scopePriorityScheme = ['global'],
private string $appMode = State::MODE_DEFAULT
) {
$this->reader = $reader;
$this->scopeConfig = $scopeConfig;
$this->omConfig = $omConfig;
$this->relations = $relations;
$this->definitions = $definitions;
$this->classDefinitions = $classDefinitions;
$this->logger = $logger;
$this->directoryList = $directoryList;
$this->scopePriorityScheme = $scopePriorityScheme;
}

/**
Expand Down Expand Up @@ -245,10 +184,8 @@ public function loadScopedVirtualTypes($scopePriorityScheme, $loadedScopes, $plu

/**
* Returns class definitions
*
* @return array
*/
private function getClassDefinitions()
private function getClassDefinitions(): array
{
return $this->classDefinitions->getClasses();
}
Expand All @@ -259,7 +196,7 @@ private function getClassDefinitions()
* @param string $scopeCode
* @return bool
*/
private function isCurrentScope($scopeCode)
private function isCurrentScope(string $scopeCode): bool
{
return $this->scopeConfig->getCurrentScope() === $scopeCode;
}
Expand Down Expand Up @@ -366,9 +303,12 @@ public function trimInstanceStartingBackslash(&$plugins)
public function filterPlugins(array &$plugins)
{
foreach ($plugins as $name => $plugin) {
if (empty($plugin['instance'])) {
if (!isset($plugin['instance'])) {
unset($plugins[$name]);
$this->logger->info("Reference to undeclared plugin with name '{$name}'.");
// Log the undeclared plugin when it is not disabled or when the app is in Developer mode.
if ($this->appMode === State::MODE_DEVELOPER || !($plugin['disabled'] ?? false)) {
$this->logger->debug("Reference to undeclared plugin with name '{$name}'.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to change info to debug

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it does not have any effect on the platform and it's related to development only.
@lbajsarowicz would argue better than me 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@engcom-Hotel Because this does not provide any valuable information outside of Development / Debugging environment, only consumes gigabytes of NewRelic / Sentry, just like the Adminhtml Menu logger.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree on this @thomas-kl1 & @lbajsarowicz :-)

}
}
}
}
Expand Down Expand Up @@ -401,26 +341,23 @@ public function merge(array $config, $pluginData)
*
* @param string $key
* @param array $config
* @return void
* @throws \Magento\Framework\Exception\FileSystemException
* @throws FileSystemException
*/
private function writeConfig(string $key, array $config)
private function writeConfig(string $key, array $config): void
{
$this->initialize();
$configuration = sprintf('<?php return %s;', var_export($config, true));
file_put_contents(
$this->directoryList->getPath(DirectoryList::GENERATED_METADATA) . '/' . $key . '.php',
$configuration
sprintf('<?php return %s;', var_export($config, true))
);
}

/**
* Initializes writer
*
* @return void
* @throws \Magento\Framework\Exception\FileSystemException
* @throws FileSystemException
*/
private function initialize()
private function initialize(): void
{
if (!file_exists($this->directoryList->getPath(DirectoryList::GENERATED_METADATA))) {
mkdir($this->directoryList->getPath(DirectoryList::GENERATED_METADATA));
Expand Down