orm icon indicating copy to clipboard operation
orm copied to clipboard

Add regression test for #8108

Open Jean85 opened this issue 1 year ago • 5 comments

As reported in https://github.com/doctrine/orm/issues/8108#issuecomment-2188230072, this is a regression test for a bug that I encounter when upgrading from ORM 2 to 3. I'm not sure how this should be fixed.

[EDIT] I should add that this doesn't happen if the relation is coming from a trait instead of an abstract class.

Jean85 avatar Jun 25 '24 12:06 Jean85

Ok I've debugged the issue, that lies in \Doctrine\ORM\Mapping\Driver\ReflectionBasedDriver::isRepeatedPropertyDeclaration:

  • that method should return true, skipping the whole "double-mapping"
  • ...but it doesn't because:
    • the reflection says that the property is defined in an (abstract) class which is not declared as an entity (Issue8108WithRelation)
    • while the mapping says that the property is declared in the first entity of the chain (Issue8108Base)

Any suggestion on how we should fix this?

[EDIT] In short, it seems that the issue stems from the fact that I'm having this chain of inheritance:

  1. class A (NOT declared as an entity) with a relation / field declared on a property
  2. class B extends A, declared as mapped superclass
  3. class C extends B, declared as child entity of B

Jean85 avatar Jul 01 '24 15:07 Jean85

Bisecting and using this as the reproducer, I've discovered that my chain of inheritance was considered "bad" but accepted in 2.x only thanks to the reportFieldsWhereDeclared set to false.

Should I consider this as invalid and close?

Jean85 avatar Jul 02 '24 09:07 Jean85

I've discovered that my chain of inheritance was considered "bad

By what? The schema validator? If yes, then we should probably close, and I think we might want to work on making it impossible to write regression tests for an invalid schema :thinking:

greg0ire avatar Jul 02 '24 10:07 greg0ire

Locally, the schema validator is what led me to this reproducer, but it's an exception, not a validation error.

This reproducer, ported to the 2.x branch, fails if reportFieldsWhereDeclared is true (with the same exception), passes otherwise. I don't know if it's a validation error or the bug itself, but this is the stack trace:

Doctrine\ORM\Mapping\MappingException: Property "createdBy" in "TmpTest\Issue8108Extending" was already declared, but it must be declared only once                                                         
                                                                                                                                                                                                            
doctrine2/src/Mapping/MappingException.php:395                                                                                                                                     
doctrine2/src/Mapping/ClassMetadataInfo.php:3826                                                                                                                                   
doctrine2/src/Mapping/ClassMetadataInfo.php:3012                                                                                                                                   
doctrine2/src/Mapping/ClassMetadataInfo.php:2980                                                                                                                                   
doctrine2/src/Mapping/Driver/AnnotationDriver.php:619                        
doctrine2/src/Mapping/Driver/AnnotationDriver.php:442                                                                                                                              
doctrine2/src/Mapping/ClassMetadataFactory.php:149                           
doctrine2/vendor/doctrine/persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php:343                                                                                 
doctrine2/vendor/doctrine/persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php:207
doctrine2/src/EntityManager.php:318                                                                                                                                                
doctrine2/tests/Tests/OrmFunctionalTestCase.php:381                          
doctrine2/tests/Tests/OrmFunctionalTestCase.php:379                          
doctrine2/tests/Tests/OrmFunctionalTestCase.php:351                                                                                                                                
doctrine2/Issue8108Test.php:32

Jean85 avatar Jul 02 '24 13:07 Jean85

but it's an exception, not a validation error

Then it might be to severe to merely be marked as invalid only to be allowed later on.

greg0ire avatar Jul 02 '24 15:07 greg0ire

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.

github-actions[bot] avatar Nov 18 '24 03:11 github-actions[bot]

This pull request was closed due to inactivity.

github-actions[bot] avatar Nov 26 '24 03:11 github-actions[bot]