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

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

Open
janopae opened this issue Nov 29, 2023 · 9 comments
Open

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

janopae opened this issue Nov 29, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@janopae
Copy link

janopae commented Nov 29, 2023

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
Copy link
Author

janopae commented Nov 29, 2023

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.

@nikophil
Copy link
Member

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 🤔

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

@nikophil
Copy link
Member

nikophil commented Dec 1, 2023

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);
            }
        );
    }

@OskarStark
Copy link
Contributor

This should be added to the docs I guess 🙏

@nikophil
Copy link
Member

nikophil commented Dec 1, 2023

Hi Oskar,

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

@janopae
Copy link
Author

janopae commented Dec 2, 2023

@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
Copy link
Author

janopae commented Dec 2, 2023

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?

@nikophil
Copy link
Member

nikophil commented Dec 2, 2023

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

@janopae janopae changed the title How to make sure Entity Association is valid from both sides? How to make sure Entity association is valid from both sides? Dec 4, 2023
@nikophil
Copy link
Member

nikophil commented Dec 7, 2023

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()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

3 participants