Add AdditionalEndpointDefinitions to DefineStaticWebAssetEndpoints#53593
Add AdditionalEndpointDefinitions to DefineStaticWebAssetEndpoints#53593
Conversation
|
Thanks for your PR, @@javiercn. |
| // Compute the suffix after the recursive wildcard so we can strip it | ||
| // from the matched route to get the portion captured by **. | ||
| // For "**/index.html" the suffix is "index.html". | ||
| // For "index.html" (no **) the suffix is empty. | ||
| var suffix = ""; | ||
| var rwcIndex = pattern.IndexOf("**", StringComparison.Ordinal); | ||
| if (rwcIndex >= 0) | ||
| { | ||
| var afterRwc = pattern.AsSpan().Slice(rwcIndex + 2); | ||
| if (afterRwc.Length > 0 && (afterRwc[0] == '/' || afterRwc[0] == '\\')) | ||
| { | ||
| afterRwc = afterRwc.Slice(1); | ||
| } | ||
| suffix = afterRwc.ToString(); | ||
| } |
There was a problem hiding this comment.
Is this a bug that should be fixed in the matcher instead?
There was a problem hiding this comment.
Fixed in 535c050. Added StemEndIndex tracking to MatchState in the glob matcher and a new ComputeCapturedStem method that returns only the portion matched exclusively by ** (without trailing literal segments). This is exposed as GlobMatch.CapturedStem. The suffix computation and string-manipulation workaround in DefineStaticWebAssetEndpoints have been removed.
|
|
||
| private void CreateAdditionalEndpoints(StaticWebAssetEndpoint sourceEndpoint) | ||
| { | ||
| var matchContext = StaticWebAssetGlobMatcher.CreateMatchContext(); |
There was a problem hiding this comment.
This should move outside of this method and the enclosing loop and be passed as a parameter
There was a problem hiding this comment.
Fixed in 535c050. CreateMatchContext() is now no longer called inside CreateAdditionalEndpoints — the method accepts a StaticWebAssetGlobMatcher.MatchContext parameter and reuses the one created in Process and passed through CreateAnAddEndpoints.
| // The glob matcher's Stem captures everything from the ** start to the | ||
| // end of the path, including the literal suffix of the pattern. | ||
| // For example, **/index.html matching admin/index.html produces | ||
| // stem="admin/index.html". We need to strip the suffix ("index.html") | ||
| // to get the actual ** captured portion ("admin"). |
There was a problem hiding this comment.
Check if this is a bug or if it's some additional value we could compute as part of a match.
There was a problem hiding this comment.
This was missing functionality in the matcher. The matcher's Stem included trailing literal segments (e.g., for **/index.html matching admin/index.html, Stem was admin/index.html). Fixed in 535c050 by adding CapturedStem to GlobMatch which returns only what ** captured (admin). CreateAdditionalEndpoints now uses match.CapturedStem directly instead of manual suffix stripping.
| var additional = endpoints.First(e => e.Route != "index.html"); | ||
| additional.AssetFile.Should().Be(original.AssetFile); | ||
| additional.ResponseHeaders.Should().BeEquivalentTo(original.ResponseHeaders); |
There was a problem hiding this comment.
assert on the route too
There was a problem hiding this comment.
Fixed in 535c050. Both DefaultDocument tests are now merged into a single Theory with [InlineData("index.html", "index.html", "/")] and [InlineData("admin/index.html", "admin/index.html", "admin")], with route assertions on the additional endpoint in both cases.
| // The stem is "admin", replacement is empty -> new route is "admin" | ||
| var additional = endpoints.First(e => e.Route != "admin/index.html"); | ||
| additional.Route.Should().Be("admin"); | ||
| additional.AssetFile.Should().Be(original.AssetFile); |
There was a problem hiding this comment.
Assert on the route, combine it with the test above into a theory
There was a problem hiding this comment.
Fixed in 535c050. The two DefaultDocument tests are now combined into a single Theory (AdditionalEndpointDefinitions_DefaultDocument_CreatesEndpointWithCapturedStem) with both root-level and nested path cases. Both cases assert on the route.
javiercn
left a comment
There was a problem hiding this comment.
@copilot address some of the feedback. Your goal is to reduce the boilerplate on the PR, while keeping tests readable. Avoid doing one-off work to accommodate for bugs or limitations on the matcher. Determine if there is a bug that needs to be fixed or missing functionality and add it there.
There was a problem hiding this comment.
Pull request overview
This PR extends Static Web Assets endpoint generation to support configurable “additional endpoints” (e.g., default document and SPA fallback) by introducing AdditionalEndpointDefinitions into DefineStaticWebAssetEndpoints, enhancing glob matching to expose the **-captured stem, and flowing a new optional endpoint Order through manifests and tasks.
Changes:
- Add MSBuild-driven
AdditionalEndpointDefinitionsto generate extra routes (default document"/"and SPA fallback"{**fallback:nonfile}") during endpoint creation. - Enhance glob matching with
CapturedStemso patterns like**/index.htmlcan map to routes likeadmin(instead ofadmin/index.html). - Introduce optional
StaticWebAssetEndpoint.Orderand propagate it through manifest comparison and compression negotiation; update integration baseline/tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetsBaselines/Build_DefaultDocumentAndSpaFallback_CreatesAdditionalEndpoints.Build.staticwebassets.json | Adds/updates build-manifest baseline to include generated additional endpoints and fallback order. |
| test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetsBaselineComparer.cs | Compares endpoint Order when validating baselines. |
| test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/DefineStaticWebAssetEndpointsTest.cs | Adds unit tests for additional endpoint definitions (default document + SPA fallback). |
| test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssetEndpointsIntegrationTest.cs | Adds integration coverage verifying additional endpoints appear in the build manifest when enabled. |
| src/StaticWebAssetsSdk/Tasks/Utils/Globbing/StaticWebAssetGlobMatcher.cs | Tracks recursive-wildcard capture boundaries and computes CapturedStem. |
| src/StaticWebAssetsSdk/Tasks/Utils/Globbing/GlobMatch.cs | Adds CapturedStem to glob match results. |
| src/StaticWebAssetsSdk/Tasks/DefineStaticWebAssetEndpoints.cs | Adds AdditionalEndpointDefinitions parsing + generation of extra endpoints. |
| src/StaticWebAssetsSdk/Tasks/Data/StaticWebAssetEndpoint.cs | Adds optional Order metadata and includes it in canonical metadata cloning. |
| src/StaticWebAssetsSdk/Tasks/ApplyCompressionNegotiation.cs | Propagates Order when creating negotiated compressed endpoints. |
| src/StaticWebAssetsSdk/Targets/Microsoft.NET.Sdk.StaticWebAssets.targets | Adds MSBuild toggles and item definitions for default document + SPA fallback endpoint generation; passes definitions into the task. |
Comments suppressed due to low confidence (1)
src/StaticWebAssetsSdk/Tasks/Utils/Globbing/StaticWebAssetGlobMatcher.cs:523
MatchState.NextComplex()does not propagateStemStartIndex/StemEndIndex. When a glob includes**and also uses complex segments, advancing to the next complex segment will reset the stem tracking to the default (-1), which can makeGlobMatch.CapturedStemincorrect/empty. Consider copying both stem indices into the state returned byNextComplex()(similar toNextSegment/NextStage/NextExtension).
internal readonly MatchState NextExtension(int extensionIndex) => new(Node, MatchStage.Extension, SegmentIndex, extensionIndex, ComplexSegmentIndex)
{
StemStartIndex = StemStartIndex,
StemEndIndex = StemEndIndex
};
internal readonly MatchState NextComplex() => new(Node, MatchStage.Complex, SegmentIndex, ExtensionSegmentIndex, ComplexSegmentIndex + 1);
| // Optional order for the endpoint in the routing table. | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] | ||
| public string Order | ||
| { | ||
| get | ||
| { | ||
| if (!_orderRead && _order == null && _originalItem != null) | ||
| { | ||
| var value = _originalItem.GetMetadata(nameof(Order)); | ||
| _order = string.IsNullOrEmpty(value) ? null : value; | ||
| _orderRead = true; | ||
| } | ||
| return _order; | ||
| } | ||
|
|
||
| set | ||
| { | ||
| _order = value; | ||
| _orderRead = true; | ||
| _modified = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Now that StaticWebAssetEndpoint has an Order property, Equals(...) / GetHashCode() / CompareTo(...) still ignore it. This can cause endpoints that differ only by order to be treated as equal (e.g., baseline comparisons use Except/HashSet on endpoints), potentially hiding real differences or producing incorrect set operations. Include Order in the equality/hash/sort logic (and keep RouteAndAssetComparer as-is if route+asset uniqueness is still desired).
Add support for defining additional endpoints based on pattern matching
against existing endpoint routes. This enables:
- Default document support: **/index.html pattern strips the suffix,
creating endpoints like / and /admin for index.html files.
- SPA fallback support: index.html pattern creates a {**fallback:nonfile}
catch-all endpoint with configurable Order (int.MaxValue for fallbacks).
Changes:
- Add Order property to StaticWebAssetEndpoint with JSON conditional
serialization (omitted when null).
- Add AdditionalEndpointDefinitions ITaskItem[] parameter to
DefineStaticWebAssetEndpoints task with Pattern, Replacement, and
Order metadata.
- Add StaticWebAssetDefaultDocumentEnabled and
StaticWebAssetSpaFallbackEnabled MSBuild properties.
- Fix ApplyCompressionNegotiation to copy Order when creating compressed
endpoint variants.
- Add Order to endpoint canonical metadata list.
- Add unit tests and integration test with baseline.
…neStaticWebAssetEndpoints, improve tests Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/be827c71-2f55-48ec-80f6-cdaf2841582d
535c050 to
df31db5
Compare
StaticWebAssetGlobMatcherto trackStemEndIndexand exposeCapturedSteminGlobMatchStemEndIndextoMatchStatestructStemEndIndexinMatchRecursiveWildCardalongsideStemStartIndexStemEndIndexthrough allNextSegment,NextStage, andNextExtensioncallsComputeCapturedStemmethod (returns only the**-captured portion)CapturedStemproperty toGlobMatchDefineStaticWebAssetEndpoints**/index.html→ suffixindex.html)SuffixfromAdditionalEndpointDefinitionmatch.CapturedStemfrom matcher instead of suffix stripping logicMatchContextcreation outsideCreateAdditionalEndpoints, passed as parameter fromCreateAnAddEndpointsDefaultDocument_CreatesEndpointWithoutSuffixandDefaultDocument_NestedPath_CapturesSteminto a singleTheorywithInlineDatafor root-level and nested paths"/"route inBothRulestest⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.