-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect invalid indexBy
configuration in the SchemaValidator
#11610
base: 3.2.x
Are you sure you want to change the base?
Conversation
indexBy
configuration in the SchemaValidator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What Psalm reports seems to be a bug… I used psalm-trace
to troubleshoot it, and got a huge type definition. I was able to fix it with /* @psalm-var ToManyAssociationMapping $assoc */
, but I should not have to. If you could make a minimal reproducer with psalm.dev and report it, that would help.
|
||
/** | ||
* GH11608 Tests whether the Schema validator picks up on invalid mapping. The entities are intentionally | ||
* invalid, and so for the purpose of this test case, those entities should be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I'd avoid creating a model set entirely. There are plenty of tests that don't.
Cheers, will do re psalm. Do you know if there is another test case I could salvage to learn about how to test the SchemaValidator without a model set? |
|
||
$errors = $validator->validateClass($this->_em->getClassMetadata($className)); | ||
|
||
var_dump($errors); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops.
I suppose calling |
|
||
if ($assoc->isIndexed()) { | ||
$joinColumns = array_column($targetMetadata->getAssociationMappings(), 'joinColumns'); | ||
$joinColumns = array_column($joinColumns, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it take all join columns into account instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it should. I didn't realise/think there could be multiple join columns. Thanks.
$joinColumns = array_column($joinColumns, 'name'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using a foreach
loop instead of several successive array_columns
, allowing to loop once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow the allowing to loop once: $targetMetadata->getAssociationMappings()
returns a multi-dimensional array. I'll have to have [probably three] nested foreach loops to gather all the data anyway.
I tried that, but then the EntityManager loaded also other tables from some default model set. Is there a way to disable that? I found none. Hence, the custom model set. |
I find that extremely surprising. Can you maybe push a branch that I could inspect to see the error? |
Fixes #11608