Conversation
AndyButland
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the Management API where reference endpoints (ReferencedBy and ReferencedDescendants) would return all items instead of a 404 Not Found response when an invalid entity ID was provided. The change adds entity existence validation using IEntityService before processing reference queries.
Key changes:
- Added 404 Not Found validation for all reference endpoints across Document, Media, and Member controllers
- Introduced IEntityService dependency injection with backward-compatible obsolete constructors
- Added ProducesResponseType attributes documenting the new 404 responses
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ReferencedDescendantsMemberController.cs | Added entity validation to return 404 for non-existent member IDs |
| ReferencedByMemberController.cs | Added entity validation to return 404 for non-existent member IDs |
| ReferencedDescendantsMediaController.cs | Added entity validation to return 404 for non-existent media IDs |
| ReferencedByMediaController.cs | Added entity validation to return 404 for non-existent media IDs |
| ReferencedDescendantsDocumentController.cs | Added entity validation to return 404 for non-existent document IDs |
| ReferencedByDocumentController.cs | Added entity validation to return 404 for non-existent document IDs |
...co.Cms.Api.Management/Controllers/Member/References/ReferencedDescendantsMemberController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Member/References/ReferencedByMemberController.cs
Outdated
Show resolved
Hide resolved
...raco.Cms.Api.Management/Controllers/Media/References/ReferencedDescendantsMediaController.cs
Outdated
Show resolved
Hide resolved
...raco.Cms.Api.Management/Controllers/Media/References/ReferencedDescendantsMediaController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Media/References/ReferencedByMediaController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Media/References/ReferencedByMediaController.cs
Outdated
Show resolved
Hide resolved
...ms.Api.Management/Controllers/Document/References/ReferencedDescendantsDocumentController.cs
Outdated
Show resolved
Hide resolved
...Umbraco.Cms.Api.Management/Controllers/Document/References/ReferencedByDocumentController.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Zeegaan
requested changes
Dec 1, 2025
Comment on lines
+61
to
+65
| IEntitySlim? entity = _entityService.Get(id, UmbracoObjectTypes.Document); | ||
| if (entity is null) | ||
| { | ||
| return NotFound(); | ||
| } |
Member
There was a problem hiding this comment.
I think I disagree on this approach, sorry 😢
I think this logic doesn't belong in the controller itself, but in the service.
I think we should update the ITrackedReferencesService
to instead adopt the Attempt<T> pattern, which most of our services have adopted at this point, and then it will return a failed attempt, if the IEntitySlim? entity = _entityService.Get(id, UmbracoObjectTypes.Document); returns null. I guess you also need to pass the ObjectType, but I don't see the problem in that.
We actually already inject the IEntityService in the service, but it's not used for now 🙈
Contributor
Author
There was a problem hiding this comment.
No need to apologies, you are right in my view. I've refactored to implement this in the way you have suggested. Would you mind having another look please? Thanks.
src/Umbraco.Cms.Api.Management/Controllers/Media/References/ReferencedByMediaController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Media/References/ReferencedByMediaController.cs
Outdated
Show resolved
Hide resolved
...Umbraco.Cms.Api.Management/Controllers/Document/References/ReferencedByDocumentController.cs
Outdated
Show resolved
Hide resolved
...co.Cms.Api.Management/Controllers/Member/References/ReferencedDescendantsMemberController.cs
Outdated
Show resolved
Hide resolved
Zeegaan
deleted the
v17/bugfix/return-not-found-for-references-requests
branch
Zeegaan
pushed a commit
that referenced
this pull request
Dec 2, 2025
…when entity does not exist (closes #20997) (#20999) * Return not found when request for content references when entity does not exist. * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Move check for entity existence from controller to the service. * Update OpenApi.json. * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Addressed points raised in code review. * Update OpenApi.json * Resolved breaking changes. --------- Co-authored-by: Copilot <[email protected]> (cherry picked from commit da94e09)
Member
|
Cherry picked for 17.0.1 🚀 |
1 task
This was referenced Dec 8, 2025
Bump Umbraco.Cms.Persistence.EFCore from 17.0.0 to 17.0.1
karl-sjogren/umbraco-extend-everything#134
Closed
Merged
This was referenced Jan 26, 2026
This was referenced Feb 23, 2026
This was referenced Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prerequisites
Addresses: #20997
Description
Between 16 and 17 it is reported that these reference endpoints would return all items instead of an empty collection when the provided ID was not recognised. It seems better really that these would return "not found", which is what I've done in this PR.
Testing
Using the management API, make requests to e.g.
umbraco/management/api/v1/media/{id}/referenced-descendantsand verify that providing an ID that doesn't exist, or isn't of the right type, returns a 404 response.