doctrine / DoctrineModule

Doctrine Module for Laminas
http://www.doctrine-project.org/projects/doctrine-module.html
MIT License
398 stars 269 forks source link

UniqueObject doesn't work anymore after upgrade when adding new entites #632

Closed BigMichi1 closed 3 years ago

BigMichi1 commented 6 years ago

today we upgraded from 1.2.0 to 2.1.1 and we are now seeing an issue with the UniqueObject validator that worked before (1.2.0)

we have this in our fieldset

                    [
                        'name' => UniqueObject::class,
                        'options' => [
                            'object_manager' => $this->objectManager,
                            'object_repository' => $this->objectManager->getRepository(Operationtype::class),
                            'use_context' => true,
                            'fields' => [
                                'shortname'
                            ],
                            'messages' => [
                                'objectNotUnique' => 'message operationtype already exists with this shortname',
                            ],
                        ],
                    ]

this will now fail when we are adding a new entry because of the change in the method getExpectedIdentifiers() from !array_key_exists($identifierField, $context) to ! isset($context[$identifierField]) and we got an exception about the missing identifier but it is in the array but null as it is correct for new entries, changing this one back to the old check everthing is now fine

TomHAnderson commented 6 years ago

Because you have a change which works will you push it in a PR?

TomHAnderson commented 6 years ago

The old version was patched with array_key_exists here: https://github.com/doctrine/DoctrineModule/commit/9cef26c8a360be9b4a10cecd43f092a128f5599a

This was changed in a mass 'general code improvement' here: https://github.com/doctrine/DoctrineModule/commit/6706e63632359dc43ac4745fc3b2246e0666cccc#diff-23ce8187a6240f02a6ec84502c38c849L164

And that is the last edit. There should be no harm moving this to the fix you suggested.

TomHAnderson commented 6 years ago

@BigMichi1 I'd like to put a unit test in to ensure this doesn't happen again. Can you provide the $context which breaks with this change?

TomHAnderson commented 6 years ago

https://github.com/doctrine/DoctrineModule/releases/tag/2.1.2

I'm going to leave this issue open in hopes you'll help develop a unit test to prevent this in the future.