-
Notifications
You must be signed in to change notification settings - Fork 296
REST Advanced Paths: Allow sub-directories in entity REST paths #2999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
40a7877
017aabb
127c3f2
d280125
36b8dc3
e3fffeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -438,6 +438,13 @@ public bool TryGetRestRouteFromConfig([NotNullWhen(true)] out string? configured | |
| /// as the substring following the '/'. | ||
| /// For example, a request route should be of the form | ||
| /// {EntityPath}/{PKColumn}/{PkValue}/{PKColumn}/{PKValue}... | ||
| /// or {SubDir}/.../{EntityPath}/{PKColumn}/{PkValue}/{PKColumn}/{PKValue}... | ||
| /// | ||
| /// Note: Uses shortest-prefix matching. When multiple entity paths could match, | ||
| /// the shortest matching path takes precedence. For example, if both "api" and | ||
| /// "api/books" are valid entity paths, a request to "/api/books/id/1" will match | ||
| /// "api" with primaryKeyRoute "books/id/1". Configure unique, non-overlapping | ||
| /// paths to avoid ambiguity. | ||
| /// </summary> | ||
| /// <param name="routeAfterPathBase">The request route (no '/' prefix) containing the entity path | ||
| /// (and optionally primary key).</param> | ||
|
|
@@ -448,26 +455,27 @@ public bool TryGetRestRouteFromConfig([NotNullWhen(true)] out string? configured | |
|
|
||
| RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); | ||
|
|
||
| // Split routeAfterPath on the first occurrence of '/', if we get back 2 elements | ||
| // this means we have a non-empty primary key route which we save. Otherwise, save | ||
| // primary key route as empty string. Entity Path will always be the element at index 0. | ||
| // ie: {EntityPath}/{PKColumn}/{PkValue}/{PKColumn}/{PKValue}... | ||
| // splits into [{EntityPath}] when there is an empty primary key route and into | ||
| // [{EntityPath}, {Primarykeyroute}] when there is a non-empty primary key route. | ||
| int maxNumberOfElementsFromSplit = 2; | ||
| string[] entityPathAndPKRoute = routeAfterPathBase.Split(new[] { '/' }, maxNumberOfElementsFromSplit); | ||
| string entityPath = entityPathAndPKRoute[0]; | ||
| string primaryKeyRoute = entityPathAndPKRoute.Length == maxNumberOfElementsFromSplit ? entityPathAndPKRoute[1] : string.Empty; | ||
|
|
||
| if (!runtimeConfig.TryGetEntityNameFromPath(entityPath, out string? entityName)) | ||
| // Split routeAfterPath to extract segments | ||
| string[] segments = routeAfterPathBase.Split('/'); | ||
|
|
||
| // Try progressively longer paths until we find a match | ||
| // Start with the first segment, then first two, etc. | ||
| for (int i = 1; i <= segments.Length; i++) | ||
| { | ||
| throw new DataApiBuilderException( | ||
| message: $"Invalid Entity path: {entityPath}.", | ||
| statusCode: HttpStatusCode.NotFound, | ||
| subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); | ||
| string entityPath = string.Join("/", segments.Take(i)); | ||
| if (runtimeConfig.TryGetEntityNameFromPath(entityPath, out string? entityName)) | ||
| { | ||
| // Found entity | ||
| string primaryKeyRoute = i < segments.Length ? string.Join("/", segments.Skip(i)) : string.Empty; | ||
| return (entityName!, primaryKeyRoute); | ||
| } | ||
| } | ||
|
Comment on lines
457
to
471
|
||
|
|
||
| return (entityName!, primaryKeyRoute); | ||
| // No entity found - show the full path for better debugging | ||
| throw new DataApiBuilderException( | ||
| message: $"Invalid Entity path: {routeAfterPathBase}.", | ||
| statusCode: HttpStatusCode.NotFound, | ||
| subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The shortest-prefix matching approach is unconventional compared to standard routing practices. Most routing frameworks (ASP.NET Core MVC, Express.js, etc.) use longest-prefix (most specific) matching to avoid ambiguity.
With the current implementation, if entities are configured with paths "cart" and "cart/item", a request to "/cart/item/id/123" will match "cart" (not "cart/item"), treating "item/id/123" as the primary key route.
While this is documented, it could surprise developers familiar with standard routing conventions and may lead to configuration errors. Consider whether longest-prefix matching would be more intuitive, or add validation to warn about overlapping entity paths during configuration validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 36b8dc3. Changed to longest-prefix (most-specific) matching to align with standard routing conventions. The loop now iterates from
segments.Lengthdown to1.