IBX-11842: Fixed SiteAccessAwareEntityManager::find() ignoring lock mode and lock version#777
IBX-11842: Fixed SiteAccessAwareEntityManager::find() ignoring lock mode and lock version#777alongosz wants to merge 3 commits into
Conversation
| * @template T of object | ||
| * | ||
| * @param class-string<T> $className | ||
| * @param int|null $lockMode one of the \Doctrine\DBAL\LockMode::* constants or null |
There was a problem hiding this comment.
To improve phpstan analisys you can use @param \Doctrine\DBAL\LockMode::*|null instead 😉
https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants
There was a problem hiding this comment.
To improve phpstan analisys you can use
@param \Doctrine\DBAL\LockMode::*|nullinstead 😉 https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants
Good catch, must've disappeared when I was testing different version of doctrine persistence.
Actually, this is PHPStan-specific syntax, so I need to use here phpstan-param. Made redundant param so PHPStorm analysis doesn't false-negative mark this doc block as incomplete (in some places we did sth similar, but mostly for phpDocumentor).
Aligned also method signature with doctrine persistence v3. Looks like it passes on both v2 and v2.
8ee1150
The decorator narrowed EntityManagerInterface::find() to two parameters, so callers requesting a pessimistic or optimistic lock (e.g. LockMode::PESSIMISTIC_WRITE) silently got no lock and no error. The arguments are now forwarded to the wrapped entity manager, the same way Doctrine\ORM\Decorator\EntityManagerDecorator does. Added a scoped PHPStan ignore, as ObjectManager::find() declares only two parameters in both doctrine/persistence v2 and v3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Made PHPStan $lockMode parameter type hint more specific. * Changed PHPStan specific syntax hints to be `@phpstan-param` * Added fallback `@param` syntax to satisfy signature completeness analysis
dde76aa to
8ee1150
Compare
|
5 participants
Related PRs:
Description:
Ibexa\Core\Persistence\Doctrine\SiteAccessAwareEntityManager::find()declared only($className, $id), silently dropping the$lockModeand$lockVersionparameters of Doctrine'sEntityManager::find(). Any caller requesting a pessimistic lock, e.g.$em->find(TaxonomyEntry::class, $id, LockMode::PESSIMISTIC_WRITE), got no lock and no error — the intendedSELECT … FOR UPDATEwas never executed. Discovered while investigating IBX-11842 (taxonomy nested-set insert deadlocks), where the pessimistic lock was silently skipped.The method now accepts and forwards both parameters to the wrapped entity manager, matching
Doctrine\ORM\Decorator\EntityManagerDecorator::find().The rest of the decorator was audited against the interface: no other method narrows its signature (
refresh()already forwards$lockMode,lock()forwards all arguments).A scoped PHPStan
ignoreErrorsentry was added, becauseDoctrine\Persistence\ObjectManager::find()declares only two parameters in both persistence v2 and v3 — the same false-positive class as the existingclear()entry inphpstan-baseline-doctrine-persistence-v3.neon.For QA:
Covered by unit tests. Functional check: according to IBX-11842 steps, together with ibexa/taxonomy#423 PR.
Regression run: