foundry icon indicating copy to clipboard operation
foundry copied to clipboard

How to make sure Entity association is valid from both sides?

Open janopae opened this issue 2 years ago • 9 comments

If you have a bidirectional OneToMany/ManyToOne assoziation between two entities, you need to make sure it gets updated on both sides. Your code might look like this:

<?php

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping as ORM;

#[ORM\Entity()]
class Category
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'string')]
    private string $id;

    #[ORM\Column(type: 'string', length: 255)]
    private string $name;

    #[ORM\OneToMany(targetEntity: Post::class)]
    private Collection $posts;

    public function __construct(string $name)
    {
        $this->posts = new ArrayCollection();
        $this->name = $name;
    }

    public function addPost(Post $post): void
    {
        if ($this !== $post->getCategory()) {
            throw new InvalidArgumentException();
        }

        if ($this->posts->contains($post)) {
            return;
        }

        $this->posts->add($post);
    }
}

#[ORM\Entity()]
class Post
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'string')]
    private $id;

    #[ORM\ManyToOne(targetEntity: Category::class)]
    #[ORM\JoinColumn]
    private $category;

    public function __construct(Category $category)
    {
        $this->category = $category;
        $category->addPost($this);
    }

    public function getCategory(): Category
    {
        return $this->category;
    }
}

If you want to create an instance, just do

$category = new Category('test');
$post = new Post($category);

And both sides of the association will be up to date.

Now if you want to create a category in Foundry, you'd probably like to do something like this:

CategoryFactory::create([
    'posts' => [
        PostFactory::create(),
        PostFactory::create(),
    ],
]);

How can you write the Factories in such a way that they pass the right Category into the Post's constructor?

janopae avatar Nov 29 '23 12:11 janopae

I already thought about using the beforeInstantiate hook in the CategoryFactory, but you can't provide a reference to the Category there, as this is before the Category gets instantiated.

afterIntantiate is too late, as the Posts will also have already been instantiated at his point, using the wrong Category.

I think I'd need to hook in after the Category is created, but before its attributes get normalized.

As I think the attributes get normalized before the category gets created, maybe I need to prevent the posts attribute from being normalized, and normalise them myself in the afterIntantiate hook?

Sadly, setting them as extraAttributes does not prevent them from being normalised.

janopae avatar Nov 29 '23 14:11 janopae

Hello @janopae

indeed, unfortunately there is currently no easy solution to do it.

However, I'd thought you'de be able to do it with afterInstantiate and extraAttributes :thinking:

have you tried to use another name than posts? a name which is not a field in your entitiy?

nikophil avatar Nov 30 '23 10:11 nikophil

I recently had the case and resolved it like this:

class Program{}
class ProgramTranslation
{
	public function __construct(private Program $program)
	{
		$program->addProgramTranslation($program);
	}
}

// ProgramFactory
    public function withTranslations(Language ...$languages): static
    {
        return $this->addState([
            'programTranslationsFactory' => static fn (Program $program) => ProgramTranslationFactory::new()->sequence(
                \array_map(
                    static fn (Language $language) => ['language' => $language, 'program' => $program],
                    $languages
                )
            )->create(),
        ]);
    }

    public function initialize(): self
    {
        return $this->instantiateWith(
            (new Instantiator())
                ->alwaysForceProperties()
                ->allowExtraAttributes(['programTranslationsFactory'])
        )->afterInstantiate(
            static function (Program $p, array $attributes) {
                if (!isset($attributes['programTranslationsFactory'])) {
                    return;
                }

                ($attributes['programTranslationsFactory'])($p);
            }
        );
    }

nikophil avatar Dec 01 '23 14:12 nikophil

This should be added to the docs I guess 🙏

OskarStark avatar Dec 01 '23 15:12 OskarStark

Hi Oskar,

I think you're right. However, I think it will be fixed in foundry 2.0

nikophil avatar Dec 01 '23 16:12 nikophil

@nikophil Thanks for the solution/workaround! Yes, that works – by wrapping it in a callback, I can successfully keep the instance from being automatically created before afterInstantiate.

I generalised it a bit, so it handles every attempt to set the ‘translations' attribute correctly – the withTranslations method now only needs to

return $this->addState(['translations' => ProgramTranslationFactory::new()->sequence(
                \array_map(
                    static fn (Language $language) => ['language' => $language, 'program' => $program],
                    $languages
                )
            )
]);

And from the outside, you can assign any ProgramTranslationFactoryto 'translations'. It even works when setting it from getDefaults.

This version of the code looks something like this:


class ProgramFactory
{
    protected function initialize(): self
    {
        return $this
            ->instantiateWith((new Instantiator())->allowExtraAttributes(['translations']))
            ->beforeInstantiate(function (array $attributes) {
                if (!isset($attributes['translations'])) {
                    return $attributes;
                }

                $translationFactories = $attributes['translations'];

                $attributes['translations'] = function (Program $program) use ($translationFactories): void {
                    if ($translationFactories instanceof FactoryCollection) {
                        $translationFactories = $translationFactories->all();
                    }

                    foreach ($translationFactories as $translationFactory) {
                        if (!$translationFactory instanceof TranslationFactory) {
                            throw new \InvalidArgumentException('Got ' . get_debug_type($translationFactory));
                        }

                        $translationFactory->create(['program' => $program]);
                    }
                };

                return $attributes;
            })
            ->afterInstantiate(function (Program $program, array $attributes): void {
                if (!isset($attributes['translations'])) {
                    return;
                }

                $attributes['translations']($program);
            });
    }
}

Edit: I generalised the implementation even further:

/**
 * @see https://github.com/zenstruck/foundry/issues/531
 *
 * Usage:
 *
 * In the `initialize()` function of your factory:
 *
 * ```
 * return $this
 *          ->instantiateWith((new Instantiator())->allowExtraAttributes(['children']))
 *          ->beforeInstantiate(ChildRelationHelper::prepareCreationOfChildEntitiesWithAReferenceToParent('children', 'parent'))
 *          ->afterInstantiate(ChildRelationHelper::createChildEntitiesWithAReferenceToParent('children'))
 *      ;
 *```
 *
 */
class ChildRelationHelper
{
    /**
     * Prevents objects of a child relation to be created without a reference to their parent in a factory, and prepares
     * the creation using {@see self::createChildEntitiesWithAReferenceToParent() } if passed to {@see ModelFactory::$beforeInstantiate}.
     *
     * Requires the instantiator passed to {@see ModelFactory::instantiateWith()} to have {@see Instantiator::allowExtraAttributes()}
     * set with $childRelationNameOnParent.
     */
    final public static function prepareCreationOfChildEntitiesWithAReferenceToParent(string $childRelationNameOnParent, string $parentRelationNameOnChild): callable
    {
        return static function (array $attributes) use ($childRelationNameOnParent, $parentRelationNameOnChild) {
            if (!isset($attributes[$childRelationNameOnParent])) {
                return $attributes;
            }

            $childFactories = $attributes[$childRelationNameOnParent];

            $attributes[$childRelationNameOnParent] = static function ($parent) use ($parentRelationNameOnChild, $childFactories): void {
                if ($childFactories instanceof FactoryCollection) {
                    $childFactories = $childFactories->all();
                }

                foreach ($childFactories as $childFactory) {
                    if (!$childFactory instanceof ModelFactory) {
                        throw new \InvalidArgumentException('Got '.get_debug_type($childFactory));
                    }

                    $childFactory->create([$parentRelationNameOnChild => $parent]);
                }
            };

            return $attributes;
        };
    }

    /**
     * Creates instances of a child relation with a reference to its parent if provided to { @see ModelFactory::afterInstantiate() }.
     * Requires creation to be prepared using {@see self::prepareCreationOfChildEntitiesWithAReferenceToParent() }.
     */
    final public static function createChildEntitiesWithAReferenceToParent(string $childRelationNameOnParent): callable
    {
        return function ($parent, array $attributes) use ($childRelationNameOnParent): void {
            if (!isset($attributes[$childRelationNameOnParent])) {
                return;
            }

            $attributes[$childRelationNameOnParent]($parent);
        };
    }
}

janopae avatar Dec 02 '23 14:12 janopae

I think you're right. However, I think it will be fixed in foundry 2.0

Because I'm curious: How will this use case be addressed in foundry 2.0?

janopae avatar Dec 02 '23 14:12 janopae

I think the creation of children is delegated to a post persist callback.

nikophil avatar Dec 02 '23 17:12 nikophil

hey @janopae

it appears it is even easier to directly use a "after instantiate" callback without even adding an extra parameter:

// ProgramFactory
public function withTranslation(array $translationAttributes): static
{
    return $this->afterInstantiate(
        static function (Program $program) use ($translationAttributes) {
            ProgramTranslationFactory::new([
                'program' => $program,
                ...$translationAttributes
            ])
                ->create();
        }
    );
}

// in some test
ProgramFactory::new()
    ->withTranslation([/** some attributes for translation 1 */])
    ->withTranslation([/** some attributes for translation 2 */])
    ->create()

I like how it fits well with the fluent interfaces.

you can even test if array_is_list($translationAttributes) this way you can call ->sequence($translationAttributes)->create() over create()

nikophil avatar Dec 07 '23 13:12 nikophil