cycle/orm

Heap does not return a new entity after persistState

KorDum opened this issue · 5 comments

KorDum commented

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

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

KorDum commented

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?

@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 commented

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.

Converted to #436