Skip to content

Hosting: Make IHostingEnvironment.ApplicationMainUrl nullable#22558

Merged
AndyButland merged 3 commits into
v18/devfrom
v18/bugfix/make-application-main-url-nullable
Apr 23, 2026
Merged

Hosting: Make IHostingEnvironment.ApplicationMainUrl nullable#22558
AndyButland merged 3 commits into
v18/devfrom
v18/bugfix/make-application-main-url-nullable

Conversation

@AndyButland
Copy link
Copy Markdown
Contributor

@AndyButland AndyButland commented Apr 21, 2026

Description

In Umbraco 18 we can make IHostingEnvironment.ApplicationMainUrl nullable to correctly reflect that it can be null when ApplicationUrlDetection is None and no explicit URL is configured. This comes from the changes in Follows on from #22307.

The only compile error to resolve was in the HttpsCheck health check, which I've done, and also took the opportunity to add unit tests for these checks.

Update usage in HttpsCheck healthcheck and add unit tests to verify refactor.
Copilot AI review requested due to automatic review settings April 21, 2026 15:41
@claude
Copy link
Copy Markdown

claude Bot commented Apr 21, 2026

Claude finished @AndyButland's task in 2m 51s —— View job


PR Review

Target: origin/v18/dev · Based on commit: 9720f5ee

Makes IHostingEnvironment.ApplicationMainUrl correctly nullable in v18, removes the ! null-forgiving operator that was papering over a Uri? backing field, and refactors HttpsCheck to handle the nullable URL cleanly with [NotNullWhen] attributes.

  • Modified public API: IHostingEnvironment.ApplicationMainUrl — return type changed UriUri?
  • Affected implementations (outside this PR): AspNetCoreHostingEnvironment (updated in PR); all 14 other src/ consumers already use ?. null-conditional operators or explicit null checks, so no compile-time or runtime impact
  • Breaking changes: Property return type widened to Uri? on a public interface. This is intentional for v18 (major version) and resolves the pre-existing TODO (V18) comment. External implementors returning a non-nullable Uri remain compatible (covariant return); consumers using the property without null-checking will receive nullable warnings which is the correct behavior.

Suggestions

  • tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs:23: Test method names use MethodName_WhenCondition_Result style rather than the Can_/Cannot_ convention documented in coding preferences. The pattern used here is consistent with the sibling ImagingHMACSecretKeyCheckTests.cs in the same folder, so this is very low priority — but worth noting in case the convention is being standardized. No action needed if the team is happy with health-check tests using the condition-based style.

Approved

Clean, correct implementation. The TryGetApplicationUrl/[NotNullWhen] refactor is the right approach for surfacing the nullable URL without code duplication across the three checks. Widening the three methods to internal with documented justification via <remarks> is a reasonable tradeoff. Test coverage is solid for the null-URL path and both scheme/config branches, and correctly avoids making outbound HTTPS requests in the unit test suite.

@claude claude Bot added the area/backend label Apr 21, 2026
@AndyButland AndyButland changed the title Hosting: Make IHostingEnvironment.ApplicationMainUrl nullable Hosting: Make IHostingEnvironment.ApplicationMainUrl nullable Apr 21, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Umbraco’s hosting abstraction to reflect that the application’s “main URL” is not always available, aligning the public IHostingEnvironment API with the existing nullable backing field behavior and adjusting the HTTPS health check + unit tests accordingly.

Changes:

  • Change IHostingEnvironment.ApplicationMainUrl from Uri to Uri? and update AspNetCoreHostingEnvironment to return the nullable value without !.
  • Refactor HttpsCheck to short-circuit cleanly when the application URL is unavailable, and widen per-check methods to internal for isolated unit testing.
  • Add HttpsCheckTests covering null-URL behavior and scheme / UseHttps branches.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/Umbraco.Tests.UnitTests/Umbraco.Core/HealthChecks/HttpsCheckTests.cs Adds unit tests for the nullable URL behavior and per-branch HTTPS check results.
src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs Updates ApplicationMainUrl implementation to return Uri? without null-forgiving.
src/Umbraco.Core/Hosting/IHostingEnvironment.cs Makes ApplicationMainUrl nullable in the public interface.
src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs Refactors URL availability handling and exposes internal methods for test isolation.

Comment thread src/Umbraco.Core/Hosting/IHostingEnvironment.cs Outdated
Copy link
Copy Markdown
Contributor

@Migaroez Migaroez left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@AndyButland AndyButland enabled auto-merge (squash) April 23, 2026 05:41
@AndyButland AndyButland merged commit 0bf2d04 into v18/dev Apr 23, 2026
26 of 27 checks passed
@AndyButland AndyButland deleted the v18/bugfix/make-application-main-url-nullable branch April 23, 2026 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants