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

Heap does not return a new entity after persistState #392

Closed
1 task done
KorDum opened this issue Jan 14, 2023 · 5 comments
Closed
1 task done

Heap does not return a new entity after persistState #392

KorDum opened this issue Jan 14, 2023 · 5 comments
Assignees
Labels
type:question Further information is requested

Comments

@KorDum
Copy link

KorDum commented Jan 14, 2023

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

Hello!
Everything works excellent in simple scenarios, but there is something that does not work in a complex scenario with DDD, Command Bus, Domain Events and sync subscribers, etc.

Unfortunately, if you call EntityManagerInterface#persistState() and get persisted entity from ORM at once (I mean EntityProvider of course), then you will get null and even SQL-query. This doesn't allow separating logic by aggregate entities and persist to database atomically.

For example simple code:

$entity = new User(id: 1);
$entityManager->persistState($entity);

// There will be a search in the Heap, then a SQL query.
// Result will be null
$persistedEntity = $orm->get(User::class, ['id' => 1]);

I expect the behavior to be similar to that of Doctrine ORM, where the repository returns an entity from the unit of work cache. I also see entity being attach to the storage of Heap, but not in "paths" property. So searching for an entity in the Heap returns a empty result.

As an alternative I found a solution to get the Storage and iterate over all the entities in it.

Version

ORM 2.2.1
PHP 8.1
@KorDum KorDum added status:to be verified Needs to be reproduced and validated. type:bug Bug labels Jan 14, 2023
@KorDum KorDum changed the title 🐛 🐛 Heap does not return a new entity after persistState Jan 14, 2023
@roxblnfk
Copy link
Member

When you call EntityManagerInterface#persist() or EntityManagerInterface#persistState() you are working wiith separated Unit of Work that just collects entities to store.
ORM know nothing about that entities (if the are new) before you run EntityManagerInterface#run()

@roxblnfk roxblnfk added type:question Further information is requested and removed type:bug Bug status:to be verified Needs to be reproduced and validated. labels Jan 14, 2023
@roxblnfk roxblnfk added this to Cycle Jan 14, 2023
@roxblnfk roxblnfk moved this to Discuss in Cycle Jan 14, 2023
@KorDum
Copy link
Author

KorDum commented Jan 15, 2023

Thanks for answer!
Eventually, I need to make an atomic transaction, so I don't want to call run method until all important business logic has finished.
So there is no other way, but to take the entity iterator directly from Heap (instead HeapInterface)?

public class UserRepository implements UserRepositoryInterface
{
    public function get(UserId $id): User
    {
        /** @var Heap $heap */
        $heap = $this->orm->getHeap();
    
        foreach ($heap->getIterator() as $entity) {
            if (!$entity instanceof User) {
                continue;
            }
    
            if ($entity->id->equals($id)) {
                return $entity;
            }
        }
    
        /** @var null|User $entity */
        $entity = $this->orm->get(User::class, [
            'id' => $id,
        ]);
    
        return $entity ?? throw new DomainException();
    }
}

Of course, you can make your own storage for new entities, but you have to keep it clean after run/clean EntityManager. Or put entirely entities in domain events, not their IDs.

Honestly, all variations look like architectural overcomplication. Can you explain why it was designed this way please?

@KorDum KorDum changed the title 🐛 Heap does not return a new entity after persistState Heap does not return a new entity after persistState Jan 15, 2023
@wolfy-j
Copy link
Contributor

wolfy-j commented Jan 15, 2023

@roxblnfk I think we can review the idea of working with iterators/heaps based on local UoW. Since it's localized it will be much safer to predict collisions. In this particular case, the ID has not been confirmed by the database, so the local index for it does not exist.

@KorDum It was not indendend as overcomplication. Instead, the design proposes to work with local, non-intercepting transactions instead of one huge and messy heap + uow + em like in Doctrine.

The main focus is on avoiding edge cases when data can not be combined properly, for example, you created a used with the given ID and then refer it from other parts of the application. You now creating references for the object which has NOT been stored, this is OK within a single transaction, but once you span it outside of it - you can create a whole mess of side-effects when user can't be properly committed.

One of the options I can suggest - we can build local indexes on a level of UoW/EM itself, which seems much more logical instead of going back to Heap.

@KorDum
Copy link
Author

KorDum commented Jan 15, 2023

Thanks!
So I thought it was for separation.
It would be nice to have a cleaner and more "high level" API for getting an entity from a local UoW.

@roxblnfk
Copy link
Member

roxblnfk commented Sep 7, 2023

Converted to #436

@roxblnfk roxblnfk closed this as completed Sep 7, 2023
@github-project-automation github-project-automation bot moved this from Discuss to Done in Cycle Sep 7, 2023
@roxblnfk roxblnfk moved this from Done to Released in Cycle Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Further information is requested
Projects
Status: Released
Development

No branches or pull requests

3 participants