Skip to content
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

Only use the custom Logger when explicitely running a build task #448

Merged
8 changes: 8 additions & 0 deletions _config/logging.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
Name: graphql-logging
after: '#logging'
---

SilverStripe\Core\Injector\Injector:
# This will be overridden by SilverStripe\GraphQL\Schema\Logger in SilverStripe\GraphQL\Dev\DevAdmin
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
Psr\Log\LoggerInterface.graphql-build: '%$Psr\Log\LoggerInterface.errorhandler'
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 7 additions & 12 deletions src/Dev/Build.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

namespace SilverStripe\GraphQL\Dev;

use Psr\Log\LoggerInterface;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\DebugView;
use SilverStripe\GraphQL\Schema\DataObject\FieldAccessor;
use SilverStripe\GraphQL\Schema\Exception\EmptySchemaException;
Expand Down Expand Up @@ -42,15 +44,8 @@ public function build(HTTPRequest $request)
echo "<div class=\"build\">";
}
$clear = true;
$verbosity = strtoupper($request->getVar('verbosity') ?? 'INFO');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to remove the verbosity because the default logger doesn't support this. Since the scope where Logger is used has been substantially curtail by this PR, this seems reasonable.

Copy link
Member

Choose a reason for hiding this comment

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

Verbosity is currently explicitly mentioned in the docs as something that can be controlled, so I think that section in the docs needs to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a related PR to fix the doc silverstripe/silverstripe-framework#10279

$constantRef = sprintf('%s::%s', Logger::class, $verbosity);
Schema::invariant(
defined($constantRef),
'Illegal verbosity: %s',
$verbosity
);
$level = constant($constantRef);
$this->buildSchema($request->getVar('schema'), $clear, $level);

$this->buildSchema($request->getVar('schema'), $clear);

if ($isBrowser) {
echo "</div>";
Expand All @@ -65,10 +60,10 @@ public function build(HTTPRequest $request)
* @throws SchemaNotFoundException
* @throws SchemaBuilderException
*/
public function buildSchema($key = null, $clear = true, int $level = Logger::INFO): void
public function buildSchema($key = null, $clear = true): void
{
$logger = Logger::singleton();
$logger->setVerbosity($level);
/** @var LoggerInterface $logger */
$logger = Injector::inst()->get(LoggerInterface::class . '.graphql-build');
$keys = $key ? [$key] : array_keys(Schema::config()->get('schemas'));
$keys = array_filter($keys, function ($key) {
return $key !== Schema::ALL;
Expand Down
8 changes: 8 additions & 0 deletions src/Dev/DevelopmentAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
use SilverStripe\Security\Permission;
use SilverStripe\Security\Security;
use Exception;
use Psr\Log\LoggerInterface;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\GraphQL\Schema\Logger;

class DevelopmentAdmin extends Controller
{
Expand Down Expand Up @@ -42,6 +45,11 @@ protected function init()
Security::permissionFailure($this);
return;
}

// Define custom logger
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
$logger = Logger::singleton();
$logger->setVerbosity(Logger::INFO);
Injector::inst()->registerService($logger, LoggerInterface::class . '.graphql-build');
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
}

public function index(HTTPRequest $request)
Expand Down
20 changes: 18 additions & 2 deletions src/Extensions/DevBuildExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

namespace SilverStripe\GraphQL\Extensions;

use Psr\Log\LoggerInterface;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\GraphQL\Dev\Build;
use SilverStripe\GraphQL\Schema\Logger;
use SilverStripe\ORM\DataExtension;

class DevBuildExtension extends DataExtension
Expand All @@ -31,8 +34,21 @@ public function onAfterBuild()
return;
}
if (!self::$done) {
Build::singleton()->buildSchema();
self::$done = true;
// Get the current graphQL logger
$defaultLogger = Injector::inst()->get(LoggerInterface::class . '.graphql-build');

try {
// Define custom logger
$logger = Logger::singleton();
$logger->setVerbosity(Logger::INFO);
Injector::inst()->registerService($logger, LoggerInterface::class . '.graphql-build');

Build::singleton()->buildSchema();
self::$done = true;
} finally {
// Restore default logger back to its starting state
Injector::inst()->registerService($defaultLogger, LoggerInterface::class . '.graphql-build');
}
}
}
}
4 changes: 3 additions & 1 deletion src/Schema/BulkLoader/BulkLoaderSet.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
namespace SilverStripe\GraphQL\Schema\BulkLoader;

use InvalidArgumentException;
use Psr\Log\LoggerInterface;
use SilverStripe\Core\ClassInfo;
use ReflectionException;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\GraphQL\Schema\Exception\SchemaBuilderException;
use SilverStripe\GraphQL\Schema\Interfaces\ConfigurationApplier;
use SilverStripe\GraphQL\Schema\Logger;
Expand Down Expand Up @@ -80,7 +82,7 @@ public function addLoader(AbstractBulkLoader $loader): self
*/
public function process(): Collection
{
$logger = Logger::singleton();
$logger = Injector::inst()->get(LoggerInterface::class . '.graphql-build');
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
$collection = $this->initialCollection;
$logger->debug(sprintf(
'Bulk loader initial collection size: %s',
Expand Down
3 changes: 2 additions & 1 deletion src/Schema/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use SilverStripe\GraphQL\Schema\Type\UnionType;
use SilverStripe\ORM\ArrayLib;
use Exception;
use SilverStripe\Core\Injector\Injector;
use TypeError;

/**
Expand Down Expand Up @@ -148,7 +149,7 @@ public function __construct(string $schemaKey, SchemaConfig $schemaConfig = null

$this->schemaConfig = $schemaConfig ?: SchemaConfig::create();
$this->state = Configuration::create();
$this->logger = Logger::singleton();
$this->logger = Injector::inst()->get(LoggerInterface::class . '.graphql-build');
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/Schema/Storage/CodeGenerationStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Exception;
use GraphQL\Type\Schema as GraphQLSchema;
use GraphQL\Type\SchemaConfig as GraphQLSchemaConfig;
use Psr\Log\LoggerInterface;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Injector\Injectable;
Expand Down Expand Up @@ -123,7 +124,7 @@ public function __construct(string $name, CacheInterface $cache)
*/
public function persistSchema(StorableSchema $schema): void
{
$logger = Logger::singleton();
$logger = Injector::inst()->get(LoggerInterface::class . '.graphql-build');
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
if (!$schema->exists()) {
throw new EmptySchemaException(sprintf(
'Schema %s is empty',
Expand Down