Skip to content

Conversation

@soyuka
Copy link
Member

@soyuka soyuka commented Oct 21, 2025

Q A
Branch? 4.1
Tickets Closes #2253
License MIT
Doc PR todo

We may want the denormalizeRelation not to throw an ItemNotFoundException but there's no way of knowing that we're denormalizing. This adds a context flag so that users can return null instead of throwing. Ping @rvanlaak another use case where you'd like null from the IriConverter (relates to #7401).

@rvanlaak
Copy link
Contributor

Thanks for tagging @soyuka 👏

@VincentLanglet
Copy link
Contributor

Should it target 4.2 instead ?

@rvanlaak
Copy link
Contributor

Denormalizing for a resource's relation still isn't a valid case where you'd expect an IRI to be null. What actually is the root cause that the IRI would be null? The item not being flushed yet? The item not being a resource? In that situation @id shouldn't be included on normalization at all.

When the intention is to denormalize (assuming: write a newly persisted object), you should be able to expect the item is there, and throw an exception if the item isn't there. An IRI not being there (yet) means that persistence / flush didn't finish yet, and the IRI is a read concern that should to be generated post-flush (during post-write normalization).

By allowing null as return value in any of these cases, we're moving the nullable check responsibility a layer higher every time. This formally would require userland to check whether the return value actually is usable.

Regarding the related discussion #5136 about filtering subresources, when a relation (e.g. OneToMany) is filtered the collection of results shouldn't contain or be null, but should be [] when it's empty right?

In the end the expectation imho would be: when you request the IriConverter for an IRI, you always should be able to expect to get one, or handle the exceptions otherwise.

@soyuka
Copy link
Member Author

soyuka commented Oct 24, 2025

I agree about IRI generation but there are some cases where you know an item will exist but for now doesn't. I think it may be a valid use case where you'd like to just ignore that IRI generation. Still, I don't want to break that interface and the IriConverter::getResourceFromIri return type is object. I changed the approach (which aligns with handling the exception correctly).

@soyuka soyuka force-pushed the fix/resilient-denormalize-relation branch from 71fa539 to 989f24c Compare October 28, 2025 09:12
@soyuka soyuka force-pushed the fix/resilient-denormalize-relation branch from 989f24c to b5dee8a Compare October 28, 2025 09:39
@soyuka soyuka merged commit 99718d9 into api-platform:4.1 Oct 28, 2025
110 of 111 checks passed
@soyuka soyuka deleted the fix/resilient-denormalize-relation branch October 28, 2025 10:28
soyuka added a commit that referenced this pull request Oct 31, 2025
Co-authored-by: Nicolas LAURENT <aegypius@users.noreply.github.com>
Co-authored-by: Javier Sampedro <jsampedro77@gmail.com>
fix(laravel): serializer attributes on Eloquent methods (#7416)
fixes #7289
fixes #7338
fix(validator): custom message was not translated (#7424)
fixes #7336
fix(serializer): resilient denormalizeRelation capability (#7474)
fix(doctrine): properly set properties according to interface (#7487)
fix(graphql): stateOptions to get filter class (#7485)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants