Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Updated: Comment #0
Problem/Motivation
Hal's entity normalizer assumes that the entity bundle key is 'type' during denormalizing.
Proposed resolution
Inject the entity manager, get the info, use the real bundle key
Remaining tasks
Review
Tests - it looks like there are no tests at all for denormalizing of entities so not sure where to put them.
User interface changes
None
API changes
None
Related Issues
None
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | interdiff-2135573-13.txt | 775 bytes | damiankloip |
| #13 | 2135573-13.patch | 6.03 KB | damiankloip |
Comments
Comment #1
larowlanActually, we have the entity manager, lets use it instead of entity_create.
Comment #3
larowlanNeeded to set the entityManager on the normalizer in the tests.
Comment #4
BerdirRelated: #1867228: Make EntityTypeManager provide an entity factory and #1892320: Add deserialize for JSON/AJAX
Comment #5
damiankloip CreditAttribution: damiankloip commentedYes, patch berdir mentions above already does this approach, so it's good they are aligned.
Comment #6
linclark CreditAttribution: linclark commentedI'm not sure I understand correctly. What about DenormalizeTest?
Comment #7
damiankloip CreditAttribution: damiankloip commentedThat's what I was thinking! :)
Comment #8
larowlanWhoops, how'd I miss that!
Comment #9
Berdir3: entity-normalizer.patch queued for re-testing.
Comment #11
damiankloip CreditAttribution: damiankloip commentedLet's start this again. The above patch does not apply anymore, but pretty much none of it is the same anymore, since we now inject the deps into these services. Also making the change to inject module handler while we are changing the constructors..
Let's get this ok again, then maybe add a test case for this.
Comment #12
BerdirLooks good.
A test could be done by adding a test using terms similar to NodeTest? that explodes right now if you try to denormalize it.
maybe also unset the type from $data then, to avoid setting it twice, similar to the langcode in the ConfigField issue?
Comment #13
damiankloip CreditAttribution: damiankloip commentedI have added the unset for the bundle in $data, that makes alot of sense :)
Do we need to add an additional test for this, the fact that we have made these changes to handle the bundle name generically and our current tests work cover this?
Comment #14
BerdirI think we do, given the amount of broken things in hal, I wouldn't be surprised if more about terms is broken :)
That said, I'm adding tests for normalizing nodes and terms and back in #2219795: Config entity references broken and other bugs, do we want to merge the patches maybe? There's also @yched's issue about simplfying the field normalizer classes further than I did referenced there, we could merge that in too. Would make it easy to get that stuff in with proper test coverage, although the patches are getting bigger...
Comment #15
BerdirOk, the latest patch is now in #2219795: Config entity references broken and other bugs, which made that pass :)