Access/Storage for multiEnums in Oro 6.1

[jimohalloran]

Summary
When an entity containing a multiEnum is updated from a pervious version to Oro 6.1 there is an issue where it seems as though different storage is used for get/set accessor methods vs add/remove methods. If you create a multiEnum field in Oro 6.1 there is no issue. Likewise if the same migration is run in Oro 6.0 there's no issue. However when the 6.0 entity is upgraded to 6.1 the following unit test case fails:

    public function testAddGetMultiEnumWithMultipleValues()
    {
        $demo = new Demo();
        $demo->addLetter($this->a);
        $demo->addLetter($this->b);

        $this->assertCount(2, $demo->getLetters());
    }

Steps to reproduce
I've packaged everything required to demonstrate this issue into a self contained bundle: https://github.com/jimohalloran/oro-serialized-data-multienum

• Obtain the bundle linked above, and place it into the src/ folder of an Oro 6.0 instance.
• Run migrations (oro:migration:load) and load fixtures (oro:migration:data:load)
• Enable the Oro 6.0 unit test by renaming src/Acme/Bundle/SerializeBundle/Tests/Unit/DemoEntity60Test.disabled to src/Acme/Bundle/SerializeBundle/Tests/Unit/DemoEntity60Test.php
• Run unit test (bin/phpunit --filter DemoEntity and note all four test cases pass.
• Upgrade to Oro 6.1.
• Disable the 6.0 unit test and enable the 6.1 unit test as above.
• Run the DemoEntity61Test and note the second two test cases fail. The 60 and 61 tests are the same, the difference is only in the setUp method where the enum values are loaded.

Actual Result
When run on Oro 6.1 (upgraded from 6.0) test cases 3 and 4 fail.

• Test Case 3: Expected count: 1, Actual Count: 0
• Test Case 4: Expected count: 2, Actual Count: 0

Expected Result
All test cases should pass. It shouldn't matter if I call setLetters and assign all of the enum values at once, or make multiple addLetter calls. All of the added/set enum values should be returned when I call getLetters. Tymofii Prysiazhniuk suggested on slack that this could be due to calling addLetter instead of addLetters however, there's a commented out fifth test case that uses addLetters, and this throws a Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException: There is no "addLetters" method in "Acme\Bundle\SerializeBundle\Entity\Demo" entity exception, so I think addLetter is the correct method.

Details about your environment

• OroPlatform version: 6.1.0
• PHP version: 8.4.6
• Database (MySQL, PostgreSQL) version: PostgreSQL 15.4
• [Optional] Server operating system: Ubuntu 24.04

Additional information
If you install this bundle in Oro 6.0 it works correctly and the unit tests pass. Likewise if you install it in Oro 6.1 it also works and the unit tests pass. It's only when the bundle is installed in Oro 6.0 and then Oro is upgraded to 6.1 that we experience issues.

Tracing the tests with a debugger, it looks like the addLetter method uses different storage within the ExtendEntityTrait to the setLetters method. If you always use setLetters and getLetters everything works as expected. But addLetters uses different storage so getLetters is returning a different object. This is that the unit tests demonstrate.

I've tried comparing the records in oro_entity_config for a "native" va "upgraded" entity. I've found entity config for the "upgraded" version has an ['extend']['schema']['addremove']['letters'] key after upgrade which is missing on the version created natively on 6.1. I'm not sure exactly what this does, but it seems to cause EntityExtendTrait to use a different extension for get/set calls vs add calls.

Ultimately I think this happens because the entity ExtendSchema has a relationship defined for each enum, which for multiEnums would be a many to many. The upgrade removes these relationships, but not before oro/vendor/oro/platform/src/Oro/Bundle/EntityExtendBundle/Tools/ExtendConfigDumper.php:454 inserts this addremove item into the extend schema. The ExtendConfigDumper is invoked via the command that's executed from \Oro\Bundle\EntityExtendBundle\Migration\UpdateExtendConfigMigration whenever migrations are run.

I've further confirmed that it's these entries causing issues by writing a migration to remove them. Uncomment the two services commented out in Acme/Bundle/SerializeBundle/Resources/config/services.yml:13 and run migrations again. This runs a migration (prioritised to run just after UpdateExtendConfigMigration) that will remove these entiries for entities not in the Oro namespace. Once these are uncommented, run the migrations again and the previously failing unit tests will pass.

I'm not really sure what these keys are doing, or whether they're really required, but they seem to be the cause of the issue. I've previously signed a CLA, so if you wanted to use my migration as the basis of a fix, I've not no issue with that. However, the better fix might be to update the ExtendConfigDumper to not create them when they're not necessary?

I've written and tested this bundle in an OroCRM instance. However I don't think there's any reason it wouldn't work on OroCommerce or Oro Platform, the migrations, unit tests, etc is pretty standard across all three. There is some navigation and UI in the bundle as well (which is required for issue #1124), which may cause issues on OroCommerce or Oro Platform but I don't know because I haven't tested it. In any case the CRUD isn't required for this issue.

[jimohalloran]

Note: I've just added testRemoveMultiEnum to the 6.1 test case, which demonstrates issue #1124 which may not be the same issue as this one.

[timofiyprisyazhnyuk]

Thanks for the detailed description. This issue is related to the configuration "['extend']['schema']['addremove']", which was relevant in version 6.0, and it was needed for "relation to multi-enum", which is not relevant in 6.1, since enums changed type to "serialized".
I will attach a patch with the fix below.

addremove_multi_enums.zip

fix_serialized_normalized_enum.zip

[jimohalloran]

I backed out my temporary fixes and installed the patches using the composer-patches plugin yesterday, then migrated a fresh database. The issues I reported have been resolved. Based on my review of the patches, the solution looks solid. Looking forward to seeing these fixes in a future release!