Skip to content

Tie an '@' profile name and a non-text address to the right configuration field (O02) - #329

Merged
siebe41 merged 2 commits into
sprint/release/O02-coverage-testsfrom
sprint/release/O02-config-issue-paths
Oct 23, 2026
Merged

siebe41 merged 2 commits into
sprint/release/O02-coverage-testsfrom
sprint/release/O02-config-issue-paths

Conversation

@siebe41

@siebe41 siebe41 commented Oct 21, 2026

Copy link
Copy Markdown
Owner

What changed

Two fixes in ConfigurationIssues, the dashboard code that ties a configuration problem to the field it's about. The coverage tests in #328 found both. This PR is stacked on #328 because the regression tests go in the test file that PR adds to.

  1. A profile name with @ pointed at the wrong field. The validator's message is Profile names must not be empty or contain '@' ('work@home'). The path came from the first quoted text, which is '@', so the problem was tied to Profiles.@, a profile that doesn't exist. The name is now taken from the last quoted text, so work@home gives Profiles.work@home. An empty name (('')) gives Profiles, because an empty name has no field of its own.
  2. A connector address of the wrong type got the generic hint. For "BaseAddress": 5, the problem should say 5 is not an address; use one like http://localhost:1234. It said is not valid here instead. The type named in the JSON error (System.Uri) was resolved with Type.GetType(name), and without an assembly name that only looks in the calling assembly and CoreLib. Uri lives in System.Private.Uri, so the lookup returned null and the Uri case never matched. The lookup now also tries typeof(Uri).Assembly.

Regression tests in ConfigurationIssuesTests: A_bad_profile_name_points_at_that_profile_not_at_the_quoted_at_sign (both the @ and the empty name), and a new BaseAddress row in A_value_that_does_not_fit_its_field_says_the_format_it_needs. CHANGELOG has a line under Unreleased → Fixed.

Why

Overflow task O02 of the usage sprint says behaviour changes found while adding coverage go in separate PRs. Both bugs leave the dashboard's configuration editor pointing at the wrong field, or at no field, for a problem the validator reports correctly.

How it was verified

  • Without the fix (the file from Test the ten least-covered classes in Core and AspNetCore (O02) #328's head restored), the two new tests fail: A_bad_profile_name_points_at_that_profile_not_at_the_quoted_at_sign and the BaseAddress row of A_value_that_does_not_fit_its_field_says_the_format_it_needs. With it: dotnet test -c Release --filter FullyQualifiedName~ConfigurationIssuesTests gave 15/15.
  • dotnet build -c Release: 0 warnings, 0 errors.
  • Full suite, dotnet test -c Release: 1,564 / 1,564 passed.
  • dotnet format --verify-no-changes: clean.
  • dotnet pack -c Release src/SemanticRouter.AspNetCore: no warnings, package validation passes (only internal code changed).

Linux and macOS: neither fix touches paths, processes or shells. Uri is in System.Private.Uri on every platform the runtime supports, so typeof(Uri).Assembly is the right assembly on all three.

Not done

  • Nothing else from O02. The quoted-text rule still assumes the profile name itself has no ' in it. With one (it's@home), the last match is a fragment of the name. That only affects which field gets highlighted, so I left it.

Refs: usage-sprint overflow task O02 (no issue), stacked on #328.

The profile path came from the first quoted text in the validator's message, which is '@', not the name. And Type.GetType("System.Uri") returns null because Uri is not in CoreLib, so a bad BaseAddress got the generic hint.

@siebe41 siebe41 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right. Both fixes are correct and their tests go through the real validator and Locate, so they would fail on the old code (the PR says it checked that; the test shapes agree). Two small points.

  1. Low: the ' case left in "Not done" is a one-line fix. ConfigurationIssues.cs:131-135 takes the last '…' match, so a name containing an apostrophe (it's@home) yields Profiles.it. The validator's message always ends in ('<name>'). (SemanticRouterOptions.cs:234). Anchoring to that, e.g. \('(?<name>.*)'\)\.$, gets the whole name whatever it contains, and the "last match" rule isn't needed.
  2. nit: whitespace-only names. The validator also rejects whitespace-only names (IsNullOrWhiteSpace, SemanticRouterOptions.cs:232), and those still give Profiles.<spaces>, while the comment at ConfigurationIssues.cs:133 says an empty name has no field of its own. A whitespace-only key does exist in the dictionary, so pointing at it is defensible. Either make the comment say "empty" on purpose, or use string.IsNullOrWhiteSpace to send both to Profiles.

Checked: the Uri lookup. Type.GetType("System.Uri") without an assembly name searches only the calling assembly and CoreLib, and typeof(Uri).Assembly (System.Private.Uri) is the same assembly on all three OSes, so the typeof(Uri) case at :325 can now match. The CHANGELOG line is under Unreleased → Fixed. Nothing here is path-, process- or shell-specific.

@siebe41
siebe41 marked this pull request as ready for review October 23, 2026 14:45
@siebe41
siebe41 merged commit e15517e into sprint/release/O02-coverage-tests Oct 23, 2026
13 checks passed
@siebe41
siebe41 deleted the sprint/release/O02-config-issue-paths branch October 23, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

usage-sprint Opened by the Oct 2026 usage sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant