[ResourceBundle] Fix EntityMerger and cascade-merging

[dpfaffenbauer]

Q	A
Bug fix?	yes
New feature?	yes/no
BC breaks?	no
Deprecations?	no
Fixed tickets	#2621

@solverat Please check if this works.

We need to refactor these things completely. It is so hard to debug and understand. I remember why we have the EntityMerger, but I am certain there are easier ways.

Main reason for the Entity Merger is:

• In preparation of the unsaved data-object, we don't use the EntityManager to load the data.
• Reason is: If you do changes and decide to not persist them, EntityManager holds the wrong state. Any flush on it, will cause the "temporary" changes to be flushed to the DB.
• This is one big disadvantages of Entity Management. Every state change that gets persisted, gets flushed.
• Since we don't know if a change gets persisted or not, we can't even use refresh to get the "original" data.
• So the merger gets the data from the actual Entity Manager, and merges them with our changes.

One workaround might be to use DTOs. So we don't actually work with real entities in combination with Pimcore DataObjects, but with DTOs that get converted to Entity on persisting. Not sure about it though.

[dpfaffenbauer]

It also seems to break other stuff...

[dpfaffenbauer]

@solverat this works, but might break versions. Can you check please?

[solverat]

@dpfaffenbauer Thanks for your investigation! It is simply brutal. Getting rid of the EntityMerger would be a good thing - especially for DX!

I just added your changes and removing / updating / adding units works flawless.

If I'm remembering it right, the EntityMerger exists because of Versions to handle drafts. With these changes, creating / updating / publishing versions are not working anymore (Which is quite obvious).

---

So I tried to go further without using the Entity Manager. I did two things:

• get entity from tmp manager if it's a version task (I don't like this tmp manager, but I guess there is no other solution as you explained above)
• using unmarshalVersion like a new form submission

First, if we're storing a version, do not use the default entity manager in getDataFromEditmode:

public function getDataFromEditmode(mixed $data, Concrete $object = null, array $params = []): mixed
{
    if (!is_array($data)) {
        return null;
    }

    $errors = [];

    // example code here!
    $onlyVersion = \Pimcore::getContainer()->get('request_stack')->getMainRequest()->query->get('task') === 'version';

    if ($onlyVersion === false) {
        $unitDefinitionsEntity = $this->getProductUnitDefinitionsRepository()->findOneForProduct($object);
    } else {
        $tempEntityManager = $this->createTempEntityManager($this->getEntityManager());
        $tempStoreValuesRepository = $this->getProductUnitDefinitionsRepositoryFactory()->createNewRepository($tempEntityManager);
        $unitDefinitionsEntity = $tempStoreValuesRepository->findOneForProduct($object);
    }

    // [...]

Second, we need to adjust the unmarshal method. PLEASE! Do not judge the example code here, I just modified the serialized data to simulate the form data:

public function unmarshalVersion(Concrete $object, mixed $data): mixed
    {
        if (!is_array($data)) {
            return null;
        }

        $unitDefinitionsEntity = $this->getProductUnitDefinitionsRepository()->findOneForProduct($object);
        $form = $this->getFormFactory()->createNamed('', ProductUnitDefinitionsType::class, $unitDefinitionsEntity);

        // @todo: use marshalVersion to store data in a right manner so we don't have to do this mess:

        $data['product'] = $object->getId();

        $additional = [];
        foreach($data['unitDefinitions'] as $index => $unitDefinition) {
            if($unitDefinition['id'] === $data['defaultUnitDefinition']['id']) {
                continue;
            }

            $unitDefinition['unit'] = $unitDefinition['unit']['id'];

            $additional[$index] = $unitDefinition;
        }

        $data['additionalUnitDefinitions'] = $additional;
        $data['defaultUnitDefinition'] = [
            'unit' => $data['defaultUnitDefinition']['unit']['id'],
            'precision' => $data['defaultUnitDefinition']['precision'],
            'conversionRate' => $data['defaultUnitDefinition']['conversionRate'],
        ];

        unset($data['unitDefinitions']);

        $form->submit($data);

        return $form->getData();
    }

The EntityManager is required by following data types and if this is a suitable way, we should also take care about them:

• ProductSpecificPriceRules
• ProductQuantityPriceRules
• StoreValues
• ProductUnitDefinitions

[dpfaffenbauer]

@solverat Main issue is how Pimcore persists and in what order. We do run the flush multiple times due to how Pimcore works. We run it for StoreValues, UnitDefinitions and Price Rules.

In this scenario, the unit definitions are the first. They actually get deleted, but then the StoreValues one comes and runs and re-adds it. But not completely right.

There is 2 fixes in here, one for only merge cascades that are actually configured to fix the issue with the ordering and old "entity" state. I had the issue that the Unit Definition never got deleted, the Unit Definitions Persister deleted it, the StoreValues added it back again. Since that one has a relation to it via Unit Definition Pricing. If we don't cascade merge that relation, we don't get the issue.

It is all a bit buggy and tricky...

[solverat]

There seems to be an issue if you're trying to remove an additional unit.

• add two units to product
• save
• remove one unit
• save

=> checkout db:

• unit is still there (has been readded, check ID)

=> next:

• change quantity of remaining unit
• save
• the removed unit is finally gone

[dpfaffenbauer]

@solverat this is killing me... Maybe, only maybe, I got it this time.

[solverat]

Improved. Next (Versions):

• Change unit
• Create draft
• Publish draft

Exception: Warning: Attempt to read property "reflFields" on string

[solverat]

Also: QPR Ranges can't be added anymore (add a qpr with a range and unit and save).

[dpfaffenbauer]

@solverat please try again