feat: Support block previews without ModelsBuilder

[rickbutterfield]

Summary

• Remove the requirement for strongly-typed ModelsBuilder models on disk — BlockPreview now falls back to base block item types (BlockGridItem, BlockListItem, RichTextBlockItem) using IPublishedElement directly when no generated model exists
• Remove CheckGeneratedModelsExist() gate and ITypeFinder dependency from the API controller; replace with IContentService for blueprint resolution
• Add Document Blueprint (Template) resolution so previews work for blueprints not in the published content cache
• Add NoModelsTestSite example project demonstrating usage without ModelsBuilder
• Add unit tests for the untyped fallback path in BlockModelFactory

Test plan

• Verify block previews render correctly on a site with ModelsBuilder (existing TestSite)
• Verify block previews render correctly on a site without ModelsBuilder (new NoModelsTestSite)
• Verify Document Blueprint previews work in the backoffice
• Run BlockModelFactoryTests — all tests should pass

🤖 Generated with Claude Code

[claude]

PR Review: feat: Support block previews without ModelsBuilder

This is a solid, well-motivated change. Removing the hard dependency on generated ModelsBuilder models significantly improves the package's accessibility. The overall approach is clean and the fallback logic is easy to follow.

---

✅ Positives

• The CreateUntypedBlockItem factory method is clean and well-placed — the switch expression is a natural fit here.
• Marking old constants [Obsolete] instead of deleting them immediately is good API hygiene for a library.
• Making GetPublishedContent async is the right call now that it may need to await cache operations.
• Unit tests for the untyped fallback path in BlockModelFactory are exactly what's needed.
• Removing the CheckGeneratedModelsExist() cache gate (which relied on ITypeFinder) simplifies the hot path meaningfully.

---

⚠️ Concerns

1. Blueprint cache refresh as a side effect of a read (medium priority)

In GetPublishedContent, each preview request for a blueprint calls _documentCacheService.RefreshContentAsync(blueprint) unconditionally. This mutates Umbraco's document cache as a side effect of what is conceptually a read. On a site with frequent blueprint previews, this will hammer the cache refresh path.

Consider checking whether the content is already in the cache before refreshing:

content = await _documentCacheService.GetByKeyAsync(nodeKey.Value, preview: true);
if (content == null)
{
    IContent? blueprint = _contentService.GetBlueprintById(nodeKey.Value);
    if (blueprint != null)
    {
        await _documentCacheService.RefreshContentAsync(blueprint);
        content = await _documentCacheService.GetByKeyAsync(nodeKey.Value, preview: true);
    }
}

This also has the benefit of only hitting IContentService (database) when the content isn't cached yet.

---

2. IContentService scope (low priority, but worth verifying)

IContentService.GetBlueprintById requires an active database scope. The controller already holds IScopeProvider _scopeProvider — is there a scope active for this call path? If Umbraco's middleware creates a per-request scope before hitting the controller, this is fine. But if not, the call to GetBlueprintById may fail silently or throw when accessing the database outside a scope.

It may be worth adding a using var scope = _scopeProvider.CreateScope(autoComplete: true); guard around the IContentService call to be explicit.

---

3. content! null-forgiving after async GetPublishedContent (low priority)

All three action methods use content! (null-forgiving):

markup = await _blockPreviewService.RenderGridBlock(blockData, content!, ControllerContext, ...);

This was safe before because the old GetPublishedContent (sync) had a well-understood return contract. Now that it's async and has additional fallback paths, an NullReferenceException here will be caught by the outer catch (Exception ex) and shown as a generic render error — which is acceptable behaviour, but a more explicit content == null guard with a clearer message (e.g. Constants.ErrorMessages.ModelsNotConfigured) would improve debuggability for end users.

---

4. Mixed typed/untyped model edge case in BlockModelFactory (low priority)

The guard in CreateBlockInstance:

if (contentType == typeof(IPublishedElement))
{
    return CreateUntypedBlockItem(...);
}

...takes the untyped path when contentType is IPublishedElement, regardless of whether settingsType is a real typed model. In theory, you could have a block with a typed settings model but no typed content model (or vice versa). The current implementation would silently ignore the typed settings model in that case. This is probably an edge case that won't occur in practice (if no model exists, both would be IPublishedElement), but it may be worth a code comment to document this assumption.

---

5. GetNoModelsErrorMessage still exists in BlockPreviewService

A private method GetNoModelsErrorMessage appears to be referenced in the pre-PR code — is it still reachable? If GetModelType now passes through typeof(IPublishedElement) instead of returning null, and all callers have been updated to handle it, this method may now be dead code. Worth checking and removing if so.

---

🔍 Test Coverage Gap

There are no tests for the blueprint resolution path in GetPublishedContent. I understand this would require integration testing against Umbraco services, which is a higher bar. At a minimum, a comment in the code noting this gap (or a follow-up issue) would be helpful.

---

Summary

The approach is sound and the implementation is clean. The main thing I'd address before merging is the unconditional blueprint cache refresh (concern #1) — the others are lower severity. Nice work!

In general, to fix log-forging issues, any user-controlled data included in log messages should be sanitized or encoded before being logged. For plain-text logs, the main concern is removing or normalizing newline and other control characters so that an attacker cannot inject apparent extra log lines or break the log format. For HTML-rendered logs, HTML encoding would be used instead.

In this case, the best minimal fix without changing existing functionality is to sanitize contentElementAlias right before it is used in the log message in the catch block of PreviewGridBlock. We can create a local sanitized variable derived from contentElementAlias which strips \r and \n (and optionally other line separators) using string.Replace, then pass that sanitized value into string.Format(Constants.ErrorMessages.LoggerError, ...). This keeps the meaning of the log entry intact while preventing newline-based log forging. The change is local to the PreviewGridBlock method in src/Umbraco.Community.BlockPreview/Controllers/BlockPreviewApiController.cs and does not require new imports or changes to other methods.

In general, to fix log-forging issues you should sanitize or encode any user-supplied data before including it in log messages. For plain-text logs, the safest minimal approach is to strip newline and carriage-return characters (and optionally other control characters) from user input. This preserves most of the content while preventing attackers from injecting additional fake log lines.

For this specific case in src/Umbraco.Community.BlockPreview/Controllers/BlockPreviewApiController.cs, the best fix with minimal behavior change is to sanitize contentElementAlias before using it in the error log. We can introduce a local sanitized variable in the catch block that removes \r and \n from contentElementAlias and then use that sanitized value in string.Format. This avoids altering the behavior of other code paths that may legitimately require the raw contentElementAlias, and it requires no new dependencies. All changes stay within the shown method PreviewListBlock, and we only touch the lines around the logging call.

Concretely:

• In the catch (Exception ex) block of PreviewListBlock, add a new local string safeContentElementAlias that is computed from contentElementAlias by replacing newline and carriage-return characters with empty strings.
• Use safeContentElementAlias instead of contentElementAlias when calling string.Format for the log message.
• No additional imports or methods are required; we use string.Replace which is in System and already available.

To fix the issue, user-controlled data should be sanitized before being included in log messages. For plain text logs, the key defense is to strip or replace newline and other control characters so a user cannot break the log’s line structure. We should apply such normalization as close as possible to the logging call and only affect the logged representation, not the value used elsewhere, to avoid changing application behavior.

The best targeted fix here is to sanitize contentElementAlias only for logging inside the catch block of PreviewRichTextMarkup. We can create a local variable (e.g., safeContentElementAlias) that derives from contentElementAlias with problematic characters removed. For example, we can call Replace to remove \r and \n, and optionally any other special characters we consider risky, and then use that sanitized variable in the string.Format call. This confines the change to the logging behavior while keeping the rest of the method and controller unchanged.

Concretely, in src/Umbraco.Community.BlockPreview/Controllers/BlockPreviewApiController.cs, within the catch (Exception ex) block of PreviewRichTextMarkup, we should:

• Introduce a local safeContentElementAlias that is derived from contentElementAlias with Environment.NewLine, \r, and \n removed (or replaced with spaces).
• Update the _logger.LogError line to use safeContentElementAlias instead of the raw contentElementAlias.

No new imports are needed, as we can use string.Replace and Environment.NewLine from the existing BCL.