The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
ContentEntityBase::createDuplicate() erases the bundle value from ContentEntityBase::entityKeys, which causes infinite loops:
ContentEntityBase::bundle()ContentEntityBase::getEntityKey()ContentEntityBase::getFieldDefinition()ContentEntityBase::getFieldDefinitions()ContentEntityBase::bundle()- ...
Proposed resolution
Do not erase the bundle value when creating a duplicate. A similar check already exists in ContentEntityBase::onChange().
Remaining tasks
None.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | drupal_2326377_11.patch | 1.31 KB | xano |
| #7 | drupal_2326377_7.patch | 1.35 KB | xano |
| #7 | interdiff.txt | 828 bytes | xano |
| #1 | drupal_2326377_1.patch | 554 bytes | xano |
Comments
Comment #1
xanoComment #3
berdirThat's unfortunate, can we add a simple assertion for this in an existing createDuplicate() test?
Comment #4
xanoI was already wondering why
createDuplicate()did not have PHPUnit coverage, but now I remember that the reason this method is uncovered was that when I worked on adding the PHPUnit tests, we concluded that adding coverage for some methods would be too hard to do because of typed data.What we can do is mock the typed data methods of the SUT and test that
createDuplicate()calls those correctly. This prevents us from having to create crazy complex mocks. This has of course the usual drawbacks of mocking the SUT. What do you think?Comment #5
berdirI'm not too sure about those kind of unit tests. There actually are some tests that mock the TypedDataManager there, but I'm not sure how useful it would really be (as in, being able to catch bugs). Just adding a single line assertEqual() in one of the existing kernel tests would at least be a lot easier to write.
Comment #6
berdirNeeds work for tests.
Comment #7
xanoComment #8
xanoComment #9
berdirtest-only patch would be good but looks good.
Comment #11
xanoRTBC as per #9 if the tests pass.
Comment #12
alexpottI ran the test without the change in
ContentEntityBase- it didn't fail. So back to needs work.Comment #13
xanoI have no idea what changed since I first worked on this, but I removed the workaround from Payment in #2350593: Remove @todo in Payment::createDuplicate() and the problem no longer occurred.