refactor: Deduplicate the block preview rendering pipeline - #329
Open
rickbutterfield wants to merge 7 commits into
Open
refactor: Deduplicate the block preview rendering pipeline#329rickbutterfield wants to merge 7 commits into
rickbutterfield wants to merge 7 commits into
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves GetPublishedContent, GetCurrentCulture, and SetupPublishedRequest into a standalone, Moq-testable PreviewContentResolver service. Behavior is preserved verbatim; the controller is not yet wired to use it (Task 4). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pulls the "check models exist -> resolve content/culture -> enrich request -> render -> enrich response -> catch and format error" pipeline shared by the four preview/* controller actions into a standalone, Moq-testable IPreviewRequestExecutor. The block-type-specific render call is injected as a delegate so the executor stays decoupled from IBlockPreviewService. Nothing wires into the controller yet; that's Task 4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nto IPreviewRequestExecutor Rewrites the controller's constructor and PreviewGridBlock/PreviewListBlock/ PreviewSingleBlock/PreviewRichTextMarkup to delegate to IPreviewRequestExecutor and IMarkupSanitizer (Tasks 1-3), dropping nine constructor parameters it only used to build those services directly. Deletes CleanUpMarkup, GetCurrentCulture, SetupPublishedRequest, CheckGeneratedModelsExist, and the 3-arg GetPublishedContent overload (logic moved to PreviewContentResolver/ PreviewRequestExecutor/MarkupSanitizer). The 2-arg GetPublishedContent overload is kept, now delegating to IPreviewContentResolver, so the seven stylesheet actions compile unchanged. Removes the obsolete StaticServiceProvider-forwarding constructor, which only existed to bridge a prior IBlockPreviewResponseEnricher migration and has no external callers since the controller is DI-activated. Adds a CP0002 APICompat suppression for the removed constructor, following the existing suppression pattern for intentional breaking changes in this package. Adds the controller's first unit tests.
… shared tail RenderGridBlock, RenderListBlock, RenderSingleBlock, and RenderRichTextBlock repeated an identical ~60-line tail (convert content data to element, find block types, create the block instance, build ViewData, render markup). Extracted that into a private generic RenderTypedBlockAsync<TBlockItem> helper; each public method now keeps only its block-type-specific deserialize/key-parsing/grid-config head. Grid's layout matching (GetMatchingGridLayout) previously mutated the block instance's RowSpan/ColumnSpan directly, but that instance no longer exists until inside the shared helper. Reworked GetMatchingGridLayout to match by content key instead of block instance and report the matched spans via out params, applied via the helper's `configure` callback (same formulas, same relative ordering as before, verified equivalent since CreateBlockInstance sets ContentKey directly from the same contentData.Key). Adds BlockPreviewService's first unit tests, covering all four render methods through the shared tail's guard clauses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…edup Closes the gaps found in the final cross-commit review of the preview rendering pipeline refactor: - Document IMarkupSanitizer, IPreviewContentResolver, and IPreviewRequestExecutor in docs/advanced-customization.md's Replaceable Services section, alongside the pipeline's other three extracted services. - Drop the dead `culture` parameter from IPreviewContentResolver.SetupPublishedRequestAsync (it was never read, and the doc comment claiming it affected downstream culture was inaccurate). - Add render-delegate wiring assertions for PreviewGridBlock, PreviewListBlock, and PreviewSingleBlock so a method-name swap between block types would fail a test instead of silently compiling. - Add test coverage for BlockPreviewService's Grid `configure` callback (RowSpan/ColumnSpan threading), by making the private FindBlockType protected virtual so a test subclass can bypass the unmockable BlockEditorConverter. - Restore XML doc comments lost during the controller action rewrite, and add the missing summary/param tags flagged on IPreviewRequestExecutor.ExecuteAsync, PreviewContentResolver's constructor, and PreviewRenderRequest. - Remove three genuinely-unused usings from the controller. - Assert the logger actually receives an error log in PreviewRequestExecutorTests, and reference Constants.ErrorMessages.RenderError instead of a hardcoded string. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- CustomPreviewContentResolver example now injects ContextCultureService and calls SetCulture() with a required comment - CustomPreviewRequestExecutor example now includes missing using statements (IPublishedContent and ControllerContext) - IPreviewContentResolver XML doc cref to ContextCultureService now fully qualified to resolve CS1574 warning - BlockPreviewServiceTests comment updated: six tests now (not five), sixth test uses TestableBlockPreviewService Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| [MapToApiVersion("1.0")] | ||
| [ProducesResponseType(typeof(string), 200)] | ||
| public async Task<IActionResult> PreviewGridBlock( | ||
| public Task<IActionResult> PreviewGridBlock( |
| [MapToApiVersion("1.0")] | ||
| [ProducesResponseType(typeof(string), 200)] | ||
| public async Task<IActionResult> PreviewListBlock( | ||
| public Task<IActionResult> PreviewListBlock( |
| [MapToApiVersion("1.0")] | ||
| [ProducesResponseType(typeof(string), 200)] | ||
| public async Task<IActionResult> PreviewRichTextMarkup( | ||
| public Task<IActionResult> PreviewSingleBlock( |
| [MapToApiVersion("1.0")] | ||
| [ProducesResponseType(typeof(string), 200)] | ||
| public async Task<IActionResult> PreviewSingleBlock( | ||
| public Task<IActionResult> PreviewRichTextMarkup( |
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, string.Format(Constants.ErrorMessages.LoggerError, request.ContentElementAlias)); |
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
Extracts the duplicated pipeline logic in
BlockPreviewApiControllerandBlockPreviewServiceinto three focused, independently testable services, without changing any public behavior.IMarkupSanitizer— the markup-cleanup step previously inlined in the controller.IPreviewContentResolver— content/culture/published-request resolution, previously duplicated across the controller's actions.IPreviewRequestExecutor— the shared "check models exist → resolve → enrich → render → enrich → catch" pipeline, previously repeated across all four preview endpoints.BlockPreviewService's fourRender*Blockmethods now share one internal tail (RenderTypedBlockAsync<TBlockItem>) instead of duplicating ~60 lines each.All three new services follow the existing "Replaceable Services" pattern already used by
IBlockModelFactory/IBlockViewRenderer/IBlockDataConverter, and are documented indocs/advanced-customization.mdalongside them.No public behavior changes.
IBlockPreviewService's interface and its three documented overridable methods (GetViewResult,CreateViewDataAsync,GetStylesheetPaths) are byte-identical to before. The controller's HTTP contract (routes, params, response shapes) is unchanged. The obsolete 14-parameter fallback constructor was removed (an intentional, suppressed APICompat break — seeCompatibilitySuppressions.xml, following the existing precedent forAddInternal's visibility change).Both the controller and the service have unit test coverage for the first time.
Test plan
dotnet test— 91/91 passing🤖 Generated with Claude Code