refactor: deprecate stuff in Instantiator

zenstruck · Nov 6, 2023 · 1f042a6 · 1f042a6
1 parent 8dac262
commit 1f042a6
Show file tree

Hide file tree

Showing 10 changed files with 108 additions and 44 deletions.
diff --git a/phpunit.dama.xml.dist b/phpunit.dama.xml.dist
@@ -11,7 +11,7 @@
         <ini name="error_reporting" value="-1"/>
         <env name="KERNEL_CLASS" value="Zenstruck\Foundry\Tests\Fixtures\Kernel"/>
         <env name="USE_DAMA_DOCTRINE_TEST_BUNDLE" value="1"/>
-        <env name="SYMFONY_DEPRECATIONS_HELPER" value="max[self]=0&amp;max[direct]=0&amp;quiet[]=indirect&amp;quiet[]=other"/>
+        <env name="SYMFONY_DEPRECATIONS_HELPER" value="/will be private in Foundry/"/>
         <env name="SHELL_VERBOSITY" value="0"/>
     </php>
 

diff --git a/src/Bundle/DependencyInjection/ZenstruckFoundryExtension.php b/src/Bundle/DependencyInjection/ZenstruckFoundryExtension.php
@@ -22,6 +22,7 @@
 use Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension;
 use Zenstruck\Foundry\Bundle\Command\StubMakeFactory;
 use Zenstruck\Foundry\Bundle\Command\StubMakeStory;
+use Zenstruck\Foundry\Instantiator;
 use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory;
 use Zenstruck\Foundry\Story;
 use Zenstruck\Foundry\Test\ORMDatabaseResetter;
@@ -87,7 +88,7 @@ private function configureDefaultInstantiator(array $config, ContainerBuilder $c
         $definition = $container->getDefinition('.zenstruck_foundry.default_instantiator');
 
         if ($config['without_constructor']) {
-            $definition->addMethodCall('withoutConstructor');
+            $definition->setFactory([Instantiator::class, 'withoutConstructor']);
         }
 
         if ($config['allow_extra_attributes']) {

diff --git a/src/Bundle/Resources/config/services.xml b/src/Bundle/Resources/config/services.xml
@@ -5,7 +5,9 @@
         https://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <services>
-        <service id=".zenstruck_foundry.default_instantiator" class="Zenstruck\Foundry\Instantiator" />
+        <service id=".zenstruck_foundry.default_instantiator" class="Zenstruck\Foundry\Instantiator">
+            <factory class="Zenstruck\Foundry\Instantiator" method="withConstructor" />
+        </service>
         <service id=".zenstruck_foundry.faker" class="Faker\Generator">
             <factory class="Faker\Factory" method="create" />
         </service>

diff --git a/src/Configuration.php b/src/Configuration.php
@@ -55,7 +55,7 @@ public function __construct(private array $ormConnectionsToReset, private array
         $this->stories = new StoryManager([]);
         $this->factories = new ModelFactoryManager([]);
         $this->faker = Faker\Factory::create();
-        $this->instantiator = new Instantiator();
+        $this->instantiator = Instantiator::withConstructor();
     }
 
     public function stories(): StoryManager

diff --git a/src/Instantiator.php b/src/Instantiator.php
@@ -19,6 +19,9 @@
 
 /**
  * @author Kevin Bond <[email protected]>
+ *
+ * @method static self withoutConstructor()
+ * @method self withoutConstructor()
  */
 final class Instantiator
 {
@@ -35,6 +38,13 @@ final class Instantiator
     /** @var string[] */
     private array $forceProperties = [];
 
+    public function __construct(bool $calledInternally = false)
+    {
+        if (!$calledInternally) {
+            trigger_deprecation('zenstruck\foundry', '1.37.0', sprintf('%1$s constructor will be private in Foundry 2.0. Use either "%1$s::withConstructor()" or "%1$s::withoutConstructor()"', self::class));
+        }
+    }
+
     /**
      * @param class-string $class
      */
@@ -90,16 +100,37 @@ public function __invoke(array $attributes, string $class): object
         return $object;
     }
 
-    /**
-     * Instantiate objects without calling the constructor.
-     */
-    public function withoutConstructor(): self
+    public function __call(string $name, array $arguments): self
     {
+        if ('withoutConstructor' !== $name) {
+            throw new \BadMethodCallException(\sprintf('Call to undefined method "%s::%s".', static::class, $name));
+        }
+
+        trigger_deprecation('zenstruck/foundry', '1.37.0', 'Calling instance method "%1$s::withoutConstructor()" is deprecated and will be removed in 2.0. Use static call instead: "%1$s::withoutConstructor()" instead.', static::class);
+
         $this->withoutConstructor = true;
 
         return $this;
     }
 
+    public static function __callStatic(string $name, array $arguments): self
+    {
+        if ('withoutConstructor' !== $name) {
+            throw new \BadMethodCallException(\sprintf('Call to undefined method "%s::%s".', static::class, $name));
+        }
+
+        $instance = new self(calledInternally: true);
+
+        $instance->withoutConstructor = true;
+
+        return $instance;
+    }
+
+    public static function withConstructor(): self
+    {
+        return new self(calledInternally: true);
+    }
+
     /**
      * Ignore attributes that can't be set to object.
      *
@@ -240,6 +271,10 @@ private function instantiate(string $class, array &$attributes): object
         $constructor = $class->getConstructor();
 
         if ($this->withoutConstructor || !$constructor || !$constructor->isPublic()) {
+            if (!$this->withoutConstructor && $constructor && !$constructor->isPublic()) {
+                trigger_deprecation('zenstruck\foundry', '1.37.0', 'Instantiator was created to instantiate "%s" by calling the constructor whereas the constructor is not public. This is deprecated and will throw an exception in Foundry 2.0. Use "%s::withoutConstructor()" instead or make constructor public.', $class->getName(), self::class);
+            }
+
             return $class->newInstanceWithoutConstructor();
         }
 

diff --git a/tests/Fixtures/Factories/CategoryFactory.php b/tests/Fixtures/Factories/CategoryFactory.php
@@ -25,7 +25,7 @@ protected function initialize(): static
     {
         return $this
             ->instantiateWith(
-                (new Instantiator())->allowExtraAttributes(['extraPostsBeforeInstantiate', 'extraPostsAfterInstantiate'])
+                Instantiator::withConstructor()->allowExtraAttributes(['extraPostsBeforeInstantiate', 'extraPostsAfterInstantiate'])
             )
             ->beforeInstantiate(function (array $attributes): array {
                 if (isset($attributes['extraPostsBeforeInstantiate'])) {

diff --git a/tests/Fixtures/Factories/PostFactory.php b/tests/Fixtures/Factories/PostFactory.php
@@ -38,7 +38,7 @@ protected function initialize(): static
     {
         return $this
             ->instantiateWith(
-                (new Instantiator())->allowExtraAttributes(['extraCategoryBeforeInstantiate', 'extraCategoryAfterInstantiate'])
+                Instantiator::withConstructor()->allowExtraAttributes(['extraCategoryBeforeInstantiate', 'extraCategoryAfterInstantiate'])
             )
             ->beforeInstantiate(function (array $attributes): array {
                 if (isset($attributes['extraCategoryBeforeInstantiate'])) {

diff --git a/tests/Functional/FactoryDoctrineCascadeTest.php b/tests/Functional/FactoryDoctrineCascadeTest.php
@@ -50,7 +50,7 @@ public function many_to_one_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             $this->assertNull($attributes['brand']->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertNotNull($product->getBrand()->getId());
@@ -68,7 +68,7 @@ public function one_to_many_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             //            $this->assertNull($attributes['products'][0]->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertCount(2, $brand->getProducts());
@@ -87,7 +87,7 @@ public function many_to_many_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             $this->assertNull($attributes['tags'][0]->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertCount(1, $product->getTags());
@@ -106,7 +106,7 @@ public function many_to_many_reverse_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             $this->assertNull($attributes['categories'][0]->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertCount(1, $product->getCategories());
@@ -125,7 +125,7 @@ public function one_to_one_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             $this->assertNull($attributes['review']->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertNotNull($product->getReview()->getId());
@@ -143,7 +143,7 @@ public function one_to_one_reverse_relationship(): void
         ])->instantiateWith(function(array $attributes, string $class): object {
             $this->assertNull($attributes['review']->getId());
 
-            return (new Instantiator())($attributes, $class);
+            return Instantiator::withConstructor()($attributes, $class);
         })->create();
 
         $this->assertNotNull($product->getReview()->getId());

diff --git a/tests/Unit/Bundle/DependencyInjection/ZenstruckFoundryExtensionTest.php b/tests/Unit/Bundle/DependencyInjection/ZenstruckFoundryExtensionTest.php
@@ -124,9 +124,15 @@ public function custom_instantiator_config(): void
             'always_force_properties' => true,
         ]]);
 
-        $this->assertContainerBuilderHasServiceDefinitionWithMethodCall('.zenstruck_foundry.default_instantiator', 'withoutConstructor');
         $this->assertContainerBuilderHasServiceDefinitionWithMethodCall('.zenstruck_foundry.default_instantiator', 'allowExtraAttributes');
         $this->assertContainerBuilderHasServiceDefinitionWithMethodCall('.zenstruck_foundry.default_instantiator', 'alwaysForceProperties');
+
+        $instantiator = $this->container->get('.zenstruck_foundry.default_instantiator');
+
+        // matthiasnoback/symfony-dependency-injection-test cannot assert if a service is created through a factory.
+        // so, we're checking that private property "Instantiator::$withoutConstructor" was set to true.
+        $withoutConstructor = \Closure::bind(static function (Instantiator $instantiator){return $instantiator->withoutConstructor;}, null, Instantiator::class)($instantiator);
+        self::assertTrue($withoutConstructor);
     }
 
     /**