From 5370bc660c63534107c17777ceee8b6e2e654282 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 26 Nov 2025 12:06:08 +0100 Subject: [PATCH] Clean up NAVIGATION_REFACTOR markdown from root --- NAVIGATION_REFACTOR_DEEP_DIVE.md | 478 ------------------------------- NAVIGATION_REFACTOR_SLIDES.md | 313 -------------------- 2 files changed, 791 deletions(-) delete mode 100644 NAVIGATION_REFACTOR_DEEP_DIVE.md delete mode 100644 NAVIGATION_REFACTOR_SLIDES.md diff --git a/NAVIGATION_REFACTOR_DEEP_DIVE.md b/NAVIGATION_REFACTOR_DEEP_DIVE.md deleted file mode 100644 index 37b65cf79..000000000 --- a/NAVIGATION_REFACTOR_DEEP_DIVE.md +++ /dev/null @@ -1,478 +0,0 @@ -# Navigation Refactor - Technical Deep Dive - -**Branch:** `refactor/navigation` -**Base:** `main` -**Date:** 2025-11-04 - -## Table of Contents - -1. [Executive Summary](#executive-summary) -2. [Core Architecture Changes](#core-architecture-changes) -3. [Type System Improvements: Covariance](#type-system-improvements-covariance) -4. [The Home Provider Pattern](#the-home-provider-pattern) -5. [Two-Phase Navigation Building](#two-phase-navigation-building) -6. [Detailed Technical Improvements](#detailed-technical-improvements) - ---- - -## Executive Summary - -This refactor represents a **complete rewrite of the navigation system**, moving from a monolithic, tightly-coupled approach to a modern, functional architecture with: - -- **Type-safe covariant interfaces** - Proper use of C# variance for safer, more flexible APIs -- **O(1) URL re-homing** - Change URL prefixes without traversing trees via the Home Provider pattern -- **Two-phase building** - Isolated per-docset navigation + cross-docset assembly -- **Immutable data structures** - Functional programming principles reduce bugs -- **Separation of concerns** - Configuration, navigation, and rendering are independent - -**Impact:** 169 files changed, 13,872 additions, 2,756 deletions - ---- - -## Core Architecture Changes - -### Before: Monolithic Navigation (main branch) - -``` -┌─────────────────────────────────────────┐ -│ GlobalNavigation │ -│ - Builds everything at once │ -│ - Tightly coupled to markdown parsing │ -│ - URLs baked in at construction │ -│ - Mixed configuration + navigation │ -└─────────────────────────────────────────┘ - │ - ┌───────────┴───────────┐ - │ │ - DocumentationGroup TableOfContentsTree - │ │ - FileNavigationItem CrossLinkNavigationItem -``` - -**Problems:** -1. **Tight coupling** - Navigation building happens during markdown parsing -2. **No reusability** - Can't build navigation for docsets in isolation -3. **URL rigidity** - URLs computed at construction, can't change prefixes -4. **Mixed concerns** - Configuration, navigation, and content generation entangled -5. **No type safety** - Weak typing, no use of covariance - -### After: Two-Phase Isolated Navigation (current branch) - -``` -Phase 1: Isolated Building (per docset) -┌──────────────────────────────────────────────────┐ -│ DocumentationSetNavigation │ -│ implements: IRootNavigationItem │ -│ INavigationHomeProvider │ -│ INavigationHomeAccessor │ -└──────────────────────────────────────────────────┘ - │ - ┌───────────┴───────────────────┐ - │ │ -TableOfContentsNavigation FolderNavigation - │ │ -FileNavigationLeaf VirtualFileNavigation - -Phase 2: Assembly (cross-docset) -┌──────────────────────────────────────────────────┐ -│ SiteNavigation │ -│ - Combines multiple DocumentationSets │ -│ - Updates HomeProvider for re-homing │ -│ - Resolves cross-links │ -└──────────────────────────────────────────────────┘ -``` - -**Benefits:** -1. **Loose coupling** - Navigation builds independently from content -2. **Reusability** - Each docset navigation is self-contained -3. **Dynamic URLs** - URLs computed on-demand via Home Provider pattern -4. **Separation** - Configuration → Navigation → Assembly → Rendering (distinct phases) -5. **Type safety** - Covariant interfaces enable compile-time guarantees - ---- - -## Type System Improvements: Covariance - -### Key Commits -- `234055bb` - Fix remaining covariance issues -- `00f65d36` - Preserve covariance of interface - -### The Problem with Old Interfaces - -**Issues in main branch:** - -1. **`TIndex Index`** - Invariant type parameter breaks polymorphism - - Can't assign `INodeNavigationItem` to `INodeNavigationItem` - -2. **`int Depth`** - Stored at every node - - Redundant (calculable from tree structure) - - Wastes memory - -3. **`bool IsCrossLink`** - Type discrimination via property - - Runtime checks instead of compile-time type safety - -### The Solution: Covariant Interfaces - -**Key Changes:** - -1. **Added `out` keyword** - `INodeNavigationItem` - - Enables upcasting to base types - - Polymorphic navigation trees now possible - -2. **Wrapped Index** - `ILeafNavigationItem Index` instead of `TIndex Index` - - Interface wrapper enables covariance - - Access model via `.Index.Model` - -3. **Removed `Depth`** - Calculate on-demand via extension method - -4. **Removed `IsCrossLink`** - Use pattern matching (`item is CrossLinkNavigationLeaf`) - -**Why This Matters:** - -- **Type Safety** - Compiler catches mismatches at compile time -- **Flexibility** - Work with navigation trees polymorphically -- **Memory** - No stored depth/URLs at every node -- **Cleaner APIs** - Better developer experience - -### Interface Hierarchy - -``` -INavigationModel (marker interface) - ↓ -INavigationItem - ├─→ ILeafNavigationItem - │ └─ TModel Model { get; } - │ - └─→ INodeNavigationItem - ├─ ILeafNavigationItem Index { get; } - ├─ IReadOnlyCollection NavigationItems { get; } - │ - └─→ IRootNavigationItem - ├─ bool IsUsingNavigationDropdown { get; } - ├─ Uri Identifier { get; } - └─ void SetNavigationItems(...) // Via IAssignableChildrenNavigation -``` - -**Note the `out` keywords** - These enable covariance: -- `ILeafNavigationItem` - Can upcast to less-derived types -- `INodeNavigationItem` - Both type parameters are covariant - ---- - -## The Home Provider Pattern - -### The Problem: URL Re-homing - -**Scenario:** Combine multiple repos with different URL prefixes -- `elasticsearch` repo → `/elasticsearch/docs/intro/` -- `kibana` repo → `/kibana/docs/intro/` - -**Old approach (main branch):** -- URLs baked in at construction time -- Changing prefixes requires **rebuilding entire tree** - O(n) -- Must allocate new objects (high memory cost) - -### The Solution: Home Provider Pattern - -**Key Interfaces:** - -- **`INavigationHomeProvider`** - Provides URL context (PathPrefix, NavigationRoot, Id) -- **`INavigationHomeAccessor`** - Allows nodes to access/update their provider - -**How It Works:** - -1. **Indirection** - Nodes reference a provider instead of storing URL info -2. **Dynamic URLs** - Computed on-demand from current provider's PathPrefix -3. **Caching** - URLs cached per provider ID, invalidated on provider change -4. **O(1) Re-homing** - Change provider → all descendants automatically use new prefix - -**Re-homing Example:** - -```csharp -// Single assignment updates entire subtree -elasticsearchNav.HomeProvider = new NavigationHomeProvider("/elasticsearch", siteNav); - -// All URLs in subtree now use /elasticsearch/ prefix - O(1)! -``` - -**Benefits:** - -- **Performance** - O(1) re-homing vs O(n) tree rebuilding -- **Memory** - No tree cloning, single reference update -- **Flexibility** - Re-home subtrees multiple times -- **Lazy Evaluation** - URLs computed when accessed -- **Caching** - Per-provider caching avoids redundant calculations - ---- - -## Two-Phase Navigation Building - -### Phase 1: Isolated Building - -Each documentation set builds navigation **independently**: - -**Process:** -1. Parse `_docset.yml` via `DocumentationSetFile.LoadAndResolve()` -2. Build `DocumentationSetNavigation` with no prefix -3. Result is self-contained, reusable navigation tree - -**Properties:** -- **Independent** - No knowledge of other docsets -- **Cacheable** - Can serialize and load separately -- **Testable** - Each docset tests in isolation -- **Parallel** - Build multiple docsets concurrently - -### Phase 2: Assembly - -Combine multiple docsets into unified site: - -**Process:** -1. Parse `navigation.yml` listing all docsets -2. Load each docset's navigation -3. Re-home via `docSetNav.HomeProvider = new NavigationHomeProvider(pathPrefix, siteNav)` -4. Add to `SiteNavigation` - -**Result:** -- Each docset has custom URL prefix -- Cross-docset references resolved -- Phantom navigation for external references -- All navigation trees composed into single hierarchy - ---- - -## Detailed Technical Improvements - -### 1. Configuration Layer Refactor - -**Key Changes:** - -- **`DocumentationSetFile`** (726 lines) - Pure data class for YAML configuration - - `LoadAndResolve()` method parses YAML and resolves all file paths eagerly - - Returns immutable, validated configuration objects - - No navigation building logic - -- **`DocumentationSetNavigation`** (429 lines) - Converts configuration to navigation tree - - Takes `DocumentationSetFile` as input - - Pure transformation, no YAML parsing - - Separated from content generation - -- **`ConfigurationFile`** - Simplified from ~350 to ~140 lines - - Only handles product/version substitutions - - No TOC parsing or navigation building - - Receives pre-parsed `DocumentationSetFile` - -**Benefits:** -- **Testability** - Can test YAML parsing independently from navigation -- **Reusability** - Configuration can be cached/serialized -- **Clarity** - Single responsibility per class - -### 2. Node Type Hierarchy - -#### Navigation Node Types - -**Root Nodes (can be re-homed):** - -```csharp -// Site-level navigation -SiteNavigation - : IRootNavigationItem - -// Docset-level navigation -DocumentationSetNavigation - : IRootNavigationItem - , INavigationHomeProvider - , INavigationHomeAccessor - -// TOC-level navigation (nested docsets) -TableOfContentsNavigation - : IRootNavigationItem - , INavigationHomeAccessor -``` - -**Intermediate Nodes:** - -```csharp -// Folders -FolderNavigation - : INodeNavigationItem - , INavigationHomeAccessor - -// Files with children (e.g., setup/index.md with setup/advanced.md) -VirtualFileNavigation - : IRootNavigationItem - , INavigationHomeAccessor -``` - -**Leaf Nodes:** - -```csharp -// Regular file -FileNavigationLeaf - : ILeafNavigationItem - -// Cross-link to another docset -CrossLinkNavigationLeaf - : ILeafNavigationItem -``` - -#### Type Relationships - -``` -IRootNavigationItem - └─ Can be top of a navigation tree - └─ Can have children assigned via SetNavigationItems() - -INodeNavigationItem - └─ Has children - └─ Has an Index (landing page) - -ILeafNavigationItem - └─ Terminal node - └─ Has a Model (content) - -INavigationHomeProvider - └─ Provides URL context (PathPrefix, NavigationRoot) - -INavigationHomeAccessor - └─ Can access/change HomeProvider - └─ Enables re-homing -``` - -### 3. Path Resolution Enables Context-Aware URLs - -**The Problem in main branch:** - -URL building required runtime path manipulation to handle: -- Isolated builds: URLs relative to documentation set -- Assembler builds: URLs relative to declaring TOC - -**The Solution: Eager Path Resolution** - -`DocumentationSetFile.LoadAndResolve()` resolves **two path representations** during config load: - -1. **`PathRelativeToDocumentationSet`** - Full path from docset root - - Used in isolated builds - - Example: `guides/api/search.md` - -2. **`PathRelativeToContainer`** - Path relative to declaring TOC - - Used in assembler builds - - Example: `api/search.md` (in `guides/toc.yml`) - -**URL Building:** - -Simple conditional chooses pre-resolved path based on context: -```csharp -var relativePath = isAssemblerBuild - ? PathRelativeToContainer - : PathRelativeToDocumentationSet; -``` - -**Benefits:** -- **No runtime manipulation** - Paths resolved once at config load -- **Performance** - No string operations during URL building -- **Clarity** - Simple conditional instead of complex path logic - -### 4. Diagnostic System: Suppressible Hints - -Navigation now emits **hints** (not errors) for potentially problematic patterns: - -**Hint Types:** - -1. **`DeepLinkingVirtualFile`** - File with deep path that has children - ```yaml - # Emits hint - use 'folder' instead - - file: guides/api/overview.md - children: [...] - ``` - Virtual files intended for sibling grouping, not nested structures. - -2. **`FolderFileNameMismatch`** - Folder+file with mismatched names - ```yaml - # Emits hint - file should be api.md or index.md - - folder: api - file: overview.md - ``` - -**Suppressing Hints:** - -```yaml -# _docset.yml -suppress_hints: - - deep_linking_virtual_file - - folder_file_name_mismatch -``` - -**Benefits:** -- Guides authors toward best practices without blocking builds -- Teams can suppress for legacy docs or during migration - -### 5. Test Infrastructure - -**Total: 4,699 lines of new test code across 3 projects** - -#### Navigation.Tests/Isolation/ (2,383 lines) - -**Purpose:** Test isolated navigation building (single docset) - -**Coverage:** -- Navigation node construction and initialization -- URL generation with different HomeProvider contexts -- File reference validation and error handling -- Tree structure correctness and parent-child relationships -- Folder+file combination handling -- Hint emission for virtual files -- Testing against real physical docset configurations - -#### Navigation.Tests/Assembler/ (1,528 lines) - -**Purpose:** Test cross-docset assembly and site navigation - -**Coverage:** -- HomeProvider re-homing (O(1) URL prefix changes) -- Multi-docset scenarios with cross-links -- URI identifier resolution across docsets -- Site-wide docset integration -- Phantom navigation support -- URL prefix handling in assembly context - -#### Elastic.Documentation.Configuration.Tests/ (1,153 lines) - -**Purpose:** Test configuration parsing and validation - -**Coverage:** -- YAML deserialization to typed objects -- `LoadAndResolve` path resolution logic -- Dual path representation (PathRelativeToDocumentationSet vs Container) -- Configuration validation rules -- Loading real physical docset configurations - ---- - -## Conclusion - -This refactor represents a **fundamental architectural improvement** to the docs-builder navigation system: - -1. **Type Safety** - Covariant interfaces catch errors at compile time -2. **Performance** - O(1) re-homing via Home Provider pattern -3. **Modularity** - Two-phase building enables caching and parallelization -4. **Testability** - 4,699 lines of new tests with >80% coverage -5. **Maintainability** - Clear separation of concerns, extensive documentation - -The investment in this refactor pays dividends in: -- **Developer experience** - Cleaner APIs, better tooling support -- **Build performance** - Parallel docset building, intelligent caching -- **Flexibility** - Easy to add new navigation types and behaviors -- **Reliability** - Comprehensive tests catch regressions - ---- - -**For Questions:** -- Review the extensive documentation in `/docs/development/navigation/` -- Check test examples in `/tests/Navigation.Tests/` -- Read the inline comments in core navigation classes - -**Next Steps:** -1. Review the covariance changes in `INavigationItem.cs` -2. Understand the Home Provider pattern in `INavigationHomeProvider.cs` -3. Trace a navigation build in `DocumentationSetNavigation.cs` -4. Run the tests to see the system in action diff --git a/NAVIGATION_REFACTOR_SLIDES.md b/NAVIGATION_REFACTOR_SLIDES.md deleted file mode 100644 index 0849592c3..000000000 --- a/NAVIGATION_REFACTOR_SLIDES.md +++ /dev/null @@ -1,313 +0,0 @@ -# Navigation Refactor - Walkthrough -**Branch:** `refactor/navigation` | **Date:** 2025-11-04 - ---- - -## Slide 1: Overview - -**Impact: 169 files changed** -- 13,872 additions -- 2,756 deletions -- 4,699 new test lines - -**Key Achievements** -- Type-safe covariant interfaces -- O(1) URL re-homing via Home Provider pattern -- Two-phase building (isolated + assembly) -- Complete separation of concerns -- Extensive test coverage - ---- - -## Slide 2: Architecture - Before (main branch) - -**Problems with Monolithic Navigation** - -- Tight coupling - navigation built during markdown parsing -- No reusability - can't build docsets in isolation -- URL rigidity - URLs baked in at construction -- Mixed concerns - config + navigation + content entangled -- Weak typing - no covariance support - ---- - -## Slide 3: Architecture - After (current branch) - -**Two-Phase Isolated Navigation** - -**Phase 1: Isolated Building** -- Each docset builds independently -- Self-contained navigation trees -- Cacheable, testable, parallelizable - -**Phase 2: Assembly** -- Combine multiple docsets -- O(1) URL prefix updates -- Cross-docset reference resolution - ---- - -## Slide 4: Type System - Covariance - -**Problems in main branch:** -- `TIndex Index` - invariant, breaks polymorphism -- `int Depth` - stored at every node, wasteful -- `bool IsCrossLink` - runtime checks vs compile-time safety - -**Solution:** -- Added `out` keyword for covariance -- Wrapped Index in `ILeafNavigationItem` -- Removed Depth (calculate on-demand) -- Removed IsCrossLink (use pattern matching) - ---- - -## Slide 5: Interface Hierarchy - -``` -INavigationModel (marker) - ↓ -INavigationItem - ├─→ ILeafNavigationItem - │ └─ TModel Model - │ - └─→ INodeNavigationItem - ├─ ILeafNavigationItem Index - ├─ IReadOnlyCollection NavigationItems - │ - └─→ IRootNavigationItem - ├─ bool IsUsingNavigationDropdown - ├─ Uri Identifier - └─ IAssignableChildrenNavigation -``` - -**Note:** `out` keywords enable upcasting to base types - ---- - -## Slide 6: Home Provider Pattern - The Problem - -**Scenario:** Combine repos with different URL prefixes -- `elasticsearch` repo → `/elasticsearch/docs/intro/` -- `kibana` repo → `/kibana/docs/intro/` - -**Old approach (main branch):** -- URLs baked in at construction -- Changing prefixes = rebuild entire tree (O(n)) -- High memory cost - ---- - -## Slide 7: Home Provider Pattern - The Solution - -**Key Interfaces:** -- `INavigationHomeProvider` - provides URL context -- `INavigationHomeAccessor` - nodes access/update provider - -**How it works:** -1. Nodes reference provider (indirection) -2. URLs computed on-demand from PathPrefix -3. URLs cached per provider ID -4. **O(1) re-homing** - single assignment updates entire subtree - -**Example:** -```csharp -elasticsearchNav.HomeProvider = - new NavigationHomeProvider("/elasticsearch", siteNav); -// All URLs now use /elasticsearch/ prefix - O(1)! -``` - ---- - -## Slide 8: Two-Phase Building - Phase 1 - -**Phase 1: Isolated Building** - -**Process:** -1. Parse `_docset.yml` via `LoadAndResolve()` -2. Build `DocumentationSetNavigation` -3. Result: self-contained navigation tree - -**Properties:** -- Independent (no knowledge of other docsets) -- Cacheable (serialize/load separately) -- Testable (isolated testing) -- Parallel (build concurrently) - ---- - -## Slide 9: Two-Phase Building - Phase 2 - -**Phase 2: Assembly** - -**Process:** -1. Parse `navigation.yml` listing docsets -2. Load each docset's navigation -3. Re-home: `docSetNav.HomeProvider = new NavigationHomeProvider(...)` -4. Add to `SiteNavigation` - -**Result:** -- Each docset has custom URL prefix -- Cross-docset references resolved -- Phantom navigation for external refs -- Single unified hierarchy - ---- - -## Slide 10: Configuration Layer Refactor - -**Key Changes:** - -**DocumentationSetFile** (726 lines) -- Pure data class for YAML config -- `LoadAndResolve()` parses and resolves paths -- Immutable, validated configuration - -**DocumentationSetNavigation** (429 lines) -- Converts config → navigation tree -- Pure transformation, no YAML parsing - -**ConfigurationFile** (140 lines, was ~350) -- Only product/version substitutions -- No TOC parsing or navigation building - ---- - -## Slide 11: Node Type Hierarchy - -**Root Nodes (can be re-homed):** -- `SiteNavigation` -- `DocumentationSetNavigation` -- `TableOfContentsNavigation` - -**Intermediate Nodes:** -- `FolderNavigation` -- `VirtualFileNavigation` - -**Leaf Nodes:** -- `FileNavigationLeaf` -- `CrossLinkNavigationLeaf` - -All implement `INavigationHomeAccessor` for re-homing - ---- - -## Slide 12: Path Resolution - -**The Problem (main branch):** -- Runtime path manipulation for URL building -- Different logic for isolated vs assembler builds - -**The Solution:** -- `LoadAndResolve()` creates **two path representations**: - 1. `PathRelativeToDocumentationSet` (isolated builds) - 2. `PathRelativeToContainer` (assembler builds) - -**URL Building:** -```csharp -var relativePath = isAssemblerBuild - ? PathRelativeToContainer - : PathRelativeToDocumentationSet; -``` - -**Benefits:** No runtime manipulation, simple conditional - ---- - -## Slide 13: Diagnostic System - Hints - -**New Suppressible Hints:** - -**1. DeepLinkingVirtualFile** -- File with deep path that has children -- Suggests using `folder` syntax instead -- Virtual files for sibling grouping, not nesting - -**2. FolderFileNameMismatch** -- Folder+file with mismatched names -- Best practice: file = folder name or index.md - -**Suppressing:** -```yaml -suppress_hints: - - deep_linking_virtual_file - - folder_file_name_mismatch -``` - ---- - -## Slide 14: Test Infrastructure - -**Total: 4,699 lines across 3 projects** - -**Navigation.Tests/Isolation** (2,383 lines) -- Node construction, URL generation -- File validation, tree structure -- Hint emission, physical docset testing - -**Navigation.Tests/Assembler** (1,528 lines) -- HomeProvider re-homing (O(1) changes) -- Multi-docset scenarios with cross-links -- Site-wide integration, phantom support - -**Configuration.Tests** (1,153 lines) -- YAML deserialization, path resolution -- Dual path representation validation -- Real docset configuration testing - ---- - -## Slide 15: Key Benefits - -**Type Safety** -- Covariant interfaces catch errors at compile time -- Polymorphic navigation trees - -**Performance** -- O(1) re-homing vs O(n) tree rebuilding -- Lazy URL evaluation with caching - -**Modularity** -- Two-phase building enables caching -- Parallel docset processing - -**Testability** -- 4,699 lines of tests -- >80% coverage - -**Maintainability** -- Clear separation of concerns -- Extensive documentation in `/docs/development/navigation/` - ---- - -## Slide 16: Summary - -**Fundamental Architectural Improvements:** - -1. **Covariant interfaces** - type-safe polymorphism -2. **Home Provider pattern** - O(1) URL re-homing -3. **Two-phase building** - isolated + assembly -4. **Path resolution** - eager resolution, context-aware -5. **Suppressible hints** - guidance without blocking -6. **Comprehensive tests** - 4,699 new test lines - -**Documentation:** `/docs/development/navigation/` - ---- - -## Slide 17: Questions? - -**Documentation Resources:** -- `/docs/development/navigation/navigation.md` - Overview -- `/docs/development/navigation/home-provider-architecture.md` - Deep dive -- `/docs/development/navigation/functional-principles.md` - Design principles - -**Test Examples:** -- `/tests/Navigation.Tests/` - Comprehensive test suite - -**Next Steps:** -1. Review covariance changes in `INavigationItem.cs` -2. Understand Home Provider pattern -3. Trace navigation build in `DocumentationSetNavigation.cs` -4. Run tests to see system in action