refactor: architectural audit — UriRouter SSOT, spec coverage, code cleanliness#54
Merged
Merged
Conversation
UriRouter was defined but never imported — ReferenceResolver absorbed its responsibilities (pattern matching, parseUri, URI resolution) and became a god-class handling both URI routing and reference resolution. This refactoring: - Enhances UriRouter with wildcard pattern matching, URN prefix mapping - ReferenceResolver now takes UriRouter as constructor dependency and delegates URI pattern matching to it - parseUri/URI_REGISTER_RE defined in exactly one place (UriRouter) - All callers updated: GraphEngine, GraphDataSource, vocabulary store - AdapterFactory creates and exposes UriRouter alongside ReferenceResolver Fixes: MECE violation between UriRouter/ReferenceResolver Fixes: DRY violation (parseUri defined 3x) Fixes: UriRouter was dead code
- Add createResolverPair() helper to test-helpers.ts - All tests now create UriRouter first, pass to ReferenceResolver - registerDataset calls moved to uriRouter.registerDataset() - UriRouter tests updated for new 4-arg registerDataset signature - Added tests for URN resolution, uriBase lookup, wildcard patterns
GraphDataSource (8 tests): - resolveRefTarget with null ref, concept-level and localization-level edges - self-reference filtering, domain edge extraction, section tree mapping escape.ts (16 tests): - escapeHtml with <, >, &, combined, empty, unicode, long strings - escapeAttr with quotes, combined special characters, plain text
The HTML stripping regex appeared 3 times inline. Extracted to a single stripHtml() function and applied it to all designation values in the index summary (previously only eng was stripped). This also incorporates the intent of PR #25 (multilingual designations in index) — the designations map in index.json is now fully HTML-stripped.
- AppSidebar.vue: remove as any casts for siteConfig.branding access (RuntimeSiteConfig already types branding with ownerName/ownerUrl) - AppSidebar.vue: type navTitle parameter with optional title field - markdown-lite.ts: move import to top of file
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Strip HTML tags and normalize whitespace for plain-text display. */ | ||
| function stripHtml(s) { | ||
| return s.replace(/<[^>]+>/g, '').replace(/\s+/g, ' ').trim(); |
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
2 participants
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.
Summary
Architectural audit and cleanup addressing MECE/DRY/OCP violations discovered during codebase review.
1. Revitalize UriRouter as URI Routing SSOT
Problem:
UriRouterwas defined but never imported —ReferenceResolverabsorbed its responsibilities (pattern matching,parseUri, URI resolution) and became a god-class handling both URI routing and reference resolution.Fix:
UriRouterenhanced with wildcard pattern matching, URN prefix mapping, andresolveUri()ReferenceResolvertakesUriRouteras constructor dependency, delegates URI matching to itparseUri/URI_REGISTER_REdefined in exactly one place (UriRouter)GraphEngine,GraphDataSource,vocabularystoreAdapterFactorycreates and exposesUriRouteralongsideReferenceResolver2. Spec Coverage Additions (27 new tests)
3. Code Cleanliness
stripHtml()helper (was duplicated 3×). Now strips HTML from ALL designation values in index (not justeng), incorporating PR fix: include multilingual designations in index summary #25 intent.as anycasts —RuntimeSiteConfigalready properly typesbrandingTest Results
vue-tsc --noEmit)Commits
refactor: revitalize UriRouter as URI routing SSOT— core architectural fixtest: update all resolver tests for UriRouter DI— test infrastructuretest: add GraphDataSource and escape.ts specs— new coveragerefactor: extract stripHtml() helper in generate-data.mjs— DRYfix: remove unnecessary as any casts, fix import order— cleanliness