Deprecate callable loggers

UPGRADE.md

@@ -8,6 +8,13 @@ awareness about deprecated code.
 
 # Upgrade to 1.8
 
+## Deprecated closure loggers in favor of PSR-3
+
+* Passing a callable to `AbstractExecutor::setLogger()` is deprecated, pass a PSR-3 logger instead.
+* The method `AbstractExecutor::log()` is deprecated without replacement.
+
+## Finalized classes
+
 Executor and Purger classes are final, they cannot be extended.
 `AbstractExecutor` is internal. It cannot be extended or used as typehint.
 

composer.json

@@ -16,7 +16,8 @@
     "require": {
         "php": "^7.4 || ^8.0",
         "doctrine/deprecations": "^0.5.3 || ^1.0",
-        "doctrine/persistence": "^2.0|^3.0"
+        "doctrine/persistence": "^2.0 || ^3.0",
+        "symfony/polyfill-php80": "^1"
     },
     "conflict": {
         "doctrine/dbal": "<3.5 || >=5",
@@ -30,8 +31,10 @@
         "doctrine/dbal": "^3.5 || ^4",
         "doctrine/mongodb-odm": "^1.3.0 || ^2.0.0",
         "doctrine/orm": "^2.14 || ^3",
+        "fig/log-test": "^1",
         "phpstan/phpstan": "^1.10",
         "phpunit/phpunit": "^9.6.13 || ^10.4.2",
+        "psr/log": "^1.1 || ^2 || ^3",
         "symfony/cache": "^5.4 || ^6.3 || ^7",
         "symfony/var-exporter": "^5.4 || ^6.3 || ^7",
         "vimeo/psalm": "^5.9"

src/Executor/AbstractExecutor.php

@@ -4,15 +4,21 @@
 
 namespace Doctrine\Common\DataFixtures\Executor;
 
+use Closure;
 use Doctrine\Common\DataFixtures\FixtureInterface;
 use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
 use Doctrine\Common\DataFixtures\Purger\PurgerInterface;
 use Doctrine\Common\DataFixtures\ReferenceRepository;
 use Doctrine\Common\DataFixtures\SharedFixtureInterface;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\Persistence\ObjectManager;
 use Exception;
+use Psr\Log\AbstractLogger;
+use Psr\Log\LoggerInterface;
+use TypeError;
 
-use function get_class;
+use function get_debug_type;
+use function is_callable;
 use function sprintf;
 
 /**
@@ -25,14 +31,14 @@ abstract class AbstractExecutor
     /**
      * Purger instance for purging database before loading data fixtures
      *
-     * @var PurgerInterface
+     * @var PurgerInterface|null
      */
     protected $purger;
 
     /**
      * Logger callback for logging messages when loading data fixtures
      *
-     * @var callable
+     * @var (LoggerInterface&callable)|null
      */
     protected $logger;
 
@@ -78,22 +84,105 @@ public function getPurger()
     /**
      * Set the logger callable to execute with the log() method.
      *
+     * @param LoggerInterface|callable|null $logger
+     *
      * @return void
      */
-    public function setLogger(callable $logger)
+    public function setLogger($logger)
     {
+        if ($logger instanceof LoggerInterface) {
+            $logger = new class ($logger) extends AbstractLogger
+            {
+                private LoggerInterface $logger;
+
+                public function __construct(LoggerInterface $logger)
+                {
+                    $this->logger = $logger;
+                }
+
+                /** @inheritDoc */
+                public function log($level, $message, array $context = []): void
+                {
+                    $this->logger->log($level, $message, $context);
+                }
+
+                public function __invoke(string $message): void
+                {
+                    Deprecation::trigger(
+                        'doctrine/data-fixtures',
+                        'https://github.com/doctrine/data-fixtures/pull/462',
+                        'Invoking the logger is deprecated, call %s methods instead',
+                        LoggerInterface::class,
+                    );
+
+                    $this->logger->debug($message);
+                }
+            };
+        } elseif (is_callable($logger)) {
+            Deprecation::trigger(
+                'doctrine/data-fixtures',
+                'https://github.com/doctrine/data-fixtures/pull/462',
+                'Passing a callable to %s() is deprecated, pass an instance of %s instead',
+                __METHOD__,
+                LoggerInterface::class,
+            );
+
+            $logger = new class ($logger) extends AbstractLogger
+            {
+                private Closure $logger;
+
+                public function __construct(callable $logger)
+                {
+                    $this->logger = Closure::fromCallable($logger);
+                }
+
+                /** @inheritDoc */
+                public function log($level, $message, array $context = []): void
+                {
+                    ($this->logger)($message);
+                }
+
+                public function __invoke(string $message): void
+                {
+                    Deprecation::trigger(
+                        'doctrine/data-fixtures',
+                        'https://github.com/doctrine/data-fixtures/pull/462',
+                        'Invoking the logger is deprecated, call %s methods instead',
+                        LoggerInterface::class,
+                    );
+
+                    ($this->logger)($message);
+                }
+            };
+        } elseif ($logger !== null) {
+            throw new TypeError(sprintf(
+                '%s(): Parameter $logger is expected to be an instance of %s, %s given.',
+                __METHOD__,
+                LoggerInterface::class,
+                get_debug_type($logger),
+            ));
+        }
+
         $this->logger = $logger;
     }
 
     /**
      * Logs a message using the logger.
      *
+     * @deprecated without replacement
+     *
      * @return void
      */
     public function log(string $message)
     {
-        $logger = $this->logger;
-        $logger($message);
+        Deprecation::triggerIfCalledFromOutside(
+            'doctrine/data-fixtures',
+            'https://github.com/doctrine/data-fixtures/pull/462',
+            'Method %s() is deprecated',
+            __METHOD__,
+        );
+
+        $this->logger->debug($message);
     }
 
     /**
@@ -109,7 +198,7 @@ public function load(ObjectManager $manager, FixtureInterface $fixture)
                 $prefix = sprintf('[%d] ', $fixture->getOrder());
             }
 
-            $this->log('loading ' . $prefix . get_class($fixture));
+            $this->log('loading ' . $prefix . get_debug_type($fixture));
         }
 
         // additionally pass the instance of reference repository to shared fixtures

tests/Common/DataFixtures/Executor/AbstractExecutorTest.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\Common\DataFixtures\Executor;
+
+use Doctrine\Common\DataFixtures\Executor\AbstractExecutor;
+use Doctrine\Common\DataFixtures\FixtureInterface;
+use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
+use Doctrine\Common\DataFixtures\Purger\PurgerInterface;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
+use Doctrine\Persistence\ObjectManager;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\Test\TestLogger;
+
+final class AbstractExecutorTest extends TestCase
+{
+    use VerifyDeprecations;
+
+    public function testLogOnLoad(): void
+    {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/data-fixtures/pull/462');
+
+        $logger   = new TestLogger();
+        $executor = $this->bootstrapExecutor();
+        $executor->setLogger($logger);
+
+        $executor->load($this->createStub(ObjectManager::class), new DummyFixture());
+
+        self::assertTrue($logger->hasDebugThatContains('loading Doctrine\Tests\Common\DataFixtures\Executor\DummyFixture'));
+
+        $executor->load($this->createStub(ObjectManager::class), new DummyOrderedFixture());
+
+        self::assertTrue($logger->hasDebugThatContains('loading [42] Doctrine\Tests\Common\DataFixtures\Executor\DummyOrderedFixture'));
+    }
+
+    public function testLogOnPurge(): void
+    {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/data-fixtures/pull/462');
+
+        $logger   = new TestLogger();
+        $executor = $this->bootstrapExecutor();
+        $executor->setLogger($logger);
+        $executor->setPurger($this->createStub(PurgerInterface::class));
+
+        $executor->purge();
+
+        self::assertTrue($logger->hasDebugThatContains('purging database'));
+    }
+
+    public function testDeprecatedLoggerUsage(): void
+    {
+        $executor = $this->bootstrapExecutor();
+        $logs     = [];
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/data-fixtures/pull/462');
+
+        $executor->setLogger(static function (string $message) use (&$logs): void {
+            $logs[] = $message;
+        });
+
+        $executor->log('Something happend.');
+        $executor->log('Something else happend.');
+        $executor->log('And we\'re done.');
+
+        self::assertSame([
+            'Something happend.',
+            'Something else happend.',
+            'And we\'re done.',
+        ], $logs);
+    }
+
+    public function testLogToLegacyClosure(): void
+    {
+        $executor = $this->bootstrapExecutor();
+        $logs     = [];
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/data-fixtures/pull/462');
+
+        $executor->setLogger(static function (string $message) use (&$logs): void {
+            $logs[] = $message;
+        });
+
+        $executor->execute([]);
+
+        self::assertSame(['Executed!'], $logs);
+    }
+
+    private function bootstrapExecutor(): AbstractExecutor
+    {
+        return new class ($this->createStub(ObjectManager::class)) extends AbstractExecutor {
+            public function execute(array $fixtures, bool $append = false): void
+            {
+                $this->logger->debug('Executed!');
+            }
+        };
+    }
+}
+
+class DummyFixture implements FixtureInterface
+{
+    public function load(ObjectManager $manager): void
+    {
+    }
+}
+
+class DummyOrderedFixture implements FixtureInterface, OrderedFixtureInterface
+{
+    public function getOrder(): int
+    {
+        return 42;
+    }
+
+    public function load(ObjectManager $manager): void
+    {
+    }
+}