Read Order from endpoint manifest in StaticAssetEndpointFactory#65975
Read Order from endpoint manifest in StaticAssetEndpointFactory#65975
Conversation
github-actions
bot
added
the
needs-area-label
javiercn
added
area-routing
and removed
needs-area-label
There was a problem hiding this comment.
Pull request overview
Adds support for propagating endpoint routing Order from the static assets endpoint manifest into the generated RouteEndpoint instances, enabling SDK-controlled routing precedence (e.g., SPA fallbacks).
Changes:
- Introduced
StaticAssetDescriptor.Order(string-backed, JSON-serializable) with freeze support. - Updated
StaticAssetEndpointFactoryto parse and applyOrdertoRouteEndpointBuilder(defaulting to-100when not specified). - Added integration tests validating default and explicit manifest-driven endpoint order, and updated public API declarations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/StaticAssets/test/StaticAssetsIntegrationTests.cs | Adds integration tests verifying default order and manifest-specified order behavior. |
| src/StaticAssets/src/StaticAssetEndpointFactory.cs | Applies manifest-provided order to RouteEndpoint.Order with a default fallback. |
| src/StaticAssets/src/StaticAssetDescriptor.cs | Adds new Order property with JSON behavior and freeze semantics. |
| src/StaticAssets/src/PublicAPI.Unshipped.txt | Declares newly added public API surface for StaticAssetDescriptor.Order. |
| // If the endpoint has an explicit order (e.g., SPA fallback endpoints), use that instead. | ||
| !string.IsNullOrEmpty(resource.Order) && int.TryParse(resource.Order, CultureInfo.InvariantCulture, out var order) ? order : -100); |
There was a problem hiding this comment.
When resource.Order is present but not a valid int, this silently falls back to -100, which can mask a bad/corrupt manifest and produce unexpected routing precedence. Consider treating invalid non-empty values as an error (e.g., throw with route/manifest context, or at least log) and only default to -100 when the value is null/empty.
| @@ -24,7 +24,8 @@ public Endpoint Create(StaticAssetDescriptor resource, List<Action<EndpointBuild | |||
| // Static resources always take precedence over default routes to mimic the behavior of UseStaticFiles. | |||
| // We give a -100 order to ensure that they are selected under normal circumstances, but leave a small lee-way | |||
There was a problem hiding this comment.
Typo in comment: "lee-way" should be "leeway".
| // We give a -100 order to ensure that they are selected under normal circumstances, but leave a small lee-way | |
| // We give a -100 order to ensure that they are selected under normal circumstances, but leave a small leeway |
| /// <summary> | ||
| /// The order of the endpoint in the routing table. When null or empty, the default order of -100 is used. | ||
| /// </summary> | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] | ||
| public string? Order | ||
| { | ||
| get => _order; | ||
| set => _order = !_isFrozen ? value : throw new InvalidOperationException("StaticAssetDescriptor is frozen and doesn't accept further changes"); | ||
| } |
There was a problem hiding this comment.
Since this is a public API and the property type is string?, the XML docs should clarify the expected format/range (e.g., invariant-culture integer that must fit in int) and what happens when an invalid value is provided. Right now it's easy for consumers to set a non-numeric value without realizing it will be ignored/defaulted at endpoint creation time.
javiercn
force-pushed
the
javiercn/endpoint-order
branch
from
a70ac1f to
8dddabb
Compare
Add string? Order property to StaticAssetDescriptor with freeze support and JsonIgnore(WhenWritingNull). StaticAssetEndpointFactory parses it to int at point of use (consistent with how Quality is handled on selectors), falling back to -100 when absent. This enables SPA fallback endpoints (Order=int.MaxValue) and default document endpoints to have configurable routing priority. Includes two integration tests: - EndpointOrder_DefaultsToNegative100_WhenOrderNotSet - EndpointOrder_UsesValueFromManifest_WhenOrderIsSet
javiercn
force-pushed
the
javiercn/endpoint-order
branch
from
8dddabb to
bb414f8
Compare
Summary
Add support for reading the
Orderproperty from the static assets endpoint manifest and applying it to the generatedRouteEndpointinstances.Changes
StaticAssetDescriptor.cs— Addedstring? Orderproperty with freeze support andJsonIgnore(WhenWritingNull). Follows the same pattern asQualityon selectors (string in JSON, parsed to the target type at point of use).StaticAssetEndpointFactory.cs— ParseOrderfrom the descriptor and use it as theRouteEndpoint.Order. Falls back to-100when absent (preserving existing behavior).PublicAPI.Unshipped.txt— Declared the new public API surface.StaticAssetsIntegrationTests.cs— Two new integration tests:EndpointOrder_DefaultsToNegative100_WhenOrderNotSet— Verifies the default-100order when no Order is specified.EndpointOrder_UsesValueFromManifest_WhenOrderIsSet— Verifies that an explicit Order value (2147483647) is read and applied.Motivation
This enables the SDK to control endpoint routing priority for scenarios like SPA fallback endpoints (
Order = int.MaxValue) and default document endpoints, where the endpoint should match only after more specific routes have been evaluated.Companion SDK PR: dotnet/sdk#53593