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

Provide the ability to use existing entities in commands for cascading persistence of state aggregate. #307

Open
lifinsky opened this issue Mar 27, 2024 · 12 comments

Comments

@lifinsky
Copy link
Contributor

Description
In aggregates, it's possible to use, for instance, Doctrine entities with cascading persistence, but only for their insertion. Because ObjectManagerInterceptor clear object manager on transaction start.

Example
The only way to correctly save an aggregate with a one-to-many or many-to-many collection of entities is to inject the repository into the command handler of the aggregate or to use a "Before" interceptor that adds entities into the command before passing it to the handler.

@lifinsky
Copy link
Contributor Author

Current possible solution:
Add interceptor on #[Before(pointcut: CommandHandler::class)], that takes the command with an entity id, then uses the repository to find the entity and returns the same or a different command instance, but with the entity instead of the id.

@dgafka
Copy link
Member

dgafka commented Mar 28, 2024

Can you provide PR with failing scenario?

@lifinsky
Copy link
Contributor Author

Hi. I will add code snippets describing the problem. After clear all existing in database entities doctrines are perceived as new and their insertion and binding occurs, instead of just binding to the aggregate

@lifinsky
Copy link
Contributor Author

@dgafka
Copy link
Member

dgafka commented Mar 29, 2024

So the case is that when we've main Entity (Aggregate Root) like Post and we change it's child Entities Tag, those child Entities won't be saved in database. And this is because we've not have not called ->persist(Tag) on each of them, so Doctrine does not know that they are meant to be saved.

@lifinsky do I understand the problem correctly?

@lifinsky
Copy link
Contributor Author

lifinsky commented Mar 29, 2024

No, existing tag was saved but as new record (doctrine allows cascade: ['persist']):

// ObjectManagerInterceptor
            foreach ($managerRegistries as $managerRegistry) {
                /** @var EntityManagerInterface $objectManager */
                foreach ($managerRegistry->getManagers() as $name => $objectManager) {
                    if (! $objectManager->isOpen()) {
                        $managerRegistry->resetManager($name);
                    }
                    if ($this->depthCount === 1) {
                        $objectManager->clear();
                    }
                }
            }

I think we should clear only agregates in unit of work. Entity manager supports clear($entityName = null) - null|string $entityName – if given, only entities of this type will get detached.

@lifinsky
Copy link
Contributor Author

Currently for this example we should find tag in interceptor with pointcut "before" on CommandHandler (after transaction start)

@dgafka
Copy link
Member

dgafka commented Mar 30, 2024

So the problem is that we create Tag Entity before outside of Post Aggregate.
As Post Aggregate is created via Command, Unity of Work is cleared, therefore the reference to Tag is also cleared.
This makes Doctrine ORM think this is new Entity, which need to be inserted instead of updated.

@lifinsky is this understanding correct?

@lifinsky
Copy link
Contributor Author

Problem in using of existing entity, with current functionality of Ecotone we should inject tag repository to aggregate class or use command handler interceptor that find entity by id and fill attribute in command or return new command object (command is ugly with default arguments with null, for example int $tagId = null, Tag $tag = null. We should not clear entities in unit of work before command transaction

@dgafka
Copy link
Member

dgafka commented Mar 30, 2024

We should not clear entities in unit of work before command transaction

I think, it's fine to set up clearing after, as long as we can clean with each Message, we are good.
Feel free to provide a PR with a test case you mentioned.

@benr77
Copy link

benr77 commented Jul 30, 2024

I'm running into this same issue. Did anything get sorted in the end as the last comment is quite a while ago and I can't find any related PR. Thanks

@lifinsky
Copy link
Contributor Author

I'm running into this same issue. Did anything get sorted in the end as the last comment is quite a while ago and I can't find any related PR. Thanks

This issue is actual and not resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants