Skip to content

Code Tidy: Remove obsolete code scheduled for removal in Umbraco 18 (UrlSegment extension method)#22682

Merged
kjac merged 7 commits into
v18/devfrom
v18/task/further-todos-and-obsoletes-11
May 4, 2026
Merged

Code Tidy: Remove obsolete code scheduled for removal in Umbraco 18 (UrlSegment extension method)#22682
kjac merged 7 commits into
v18/devfrom
v18/task/further-todos-and-obsoletes-11

Conversation

@AndyButland

@AndyButland AndyButland commented May 4, 2026

Copy link
Copy Markdown
Contributor

Description

Continues the v18 obsolete-cleanup series. This PR removes the [Obsolete] UrlSegment extension method on PublishedContentExtensions and migrates its callers to IDocumentUrlService.GetUrlSegment().

It also starts a clean v20 deprecation cycle for the underlying IPublishedContent.UrlSegment property — that property on the interface and some of its implementations were never [Obsolete]-marked, so they can't be removed in v18 without violating the deprecation policy.

Removed

  • Umbraco.Cms.Core.Extensions.PublishedContentExtensions.UrlSegment(this IPublishedContent, IVariationContextAccessor?, string?)[Obsolete("...Scheduled for removal in Umbraco 18.")]. The three production call sites (ApiContentRouteBuilder, RequestStartItemProvider, HybridCache PublishedContent) are migrated to IDocumentUrlService.GetUrlSegment().

Migrated to IDocumentUrlService.GetUrlSegment()

  • src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs — service was already injected; only the call site changed.
  • src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs — constructor now takes IDocumentUrlService.
  • src/Umbraco.Cms.Api.Delivery/Services/RequestRedirectService.cs — constructor now takes IDocumentUrlService.

Re-implemented (no behaviour change for non-preview rendering)

  • src/Umbraco.Core/Models/PublishedContent/PublishedContentBase.csUrlSegment body now delegates to the new shared helper.
  • src/Umbraco.PublishedCache.HybridCache/PublishedContent.cs — same. Also corrected an unrelated stale obsolete message ("Scheduled for removal in V16" was never honoured) to align with the v20 target.
  • src/Umbraco.Web.Common/Extensions/FriendlyPublishedContentExtensions.cs — the Razor-facing UrlSegment(this IPublishedContent, string?) wrapper is kept and is not marked as [Obsolete] (I'm expecting we'll want to keep this as a convenience). Its body now routes through IDocumentUrlService.GetUrlSegment() with preview detection via IUmbracoContextAccessor — so it's consistent with the new deprecation messaging and survives the v20 removal of IPublishedContent.UrlSegment unchanged.

Shared helper

A new PublishedContentUrlSegmentResolver.Resolve() static helper consolidates the URL-segment lookup. It also routes through IDocumentUrlService.GetUrlSegment() (pulled via StaticServiceProvider) and detects preview state via IUmbracoContextAccessor.TryGetUmbracoContext(). Marked [Obsolete] because it only exists to satisfy the deprecated IPublishedContent.UrlSegment contract during the v18→v20 deprecation period — removed in v20 alongside the property itself.

Routing the resolver through IDocumentUrlService (rather than reading Cultures[culture].UrlSegment directly, as the deleted extension did) keeps the deprecation messaging consistent: every path that ever resolves a URL segment now reads from a single source of truth — the document-URL cache populated by IDocumentUrlService.

New [Obsolete] markers (v20 removal target)

Marked [Obsolete("...Scheduled for removal in Umbraco 20.")] to start a clean deprecation cycle:

  • IPublishedContent.UrlSegment (interface property)
  • PublishedContentWrapped.UrlSegment
  • InternalPublishedContent.UrlSegment (internal test harness)
  • PublishedExternalMember.UrlSegment (specific message — always returns null for external members)
  • HybridCache PublishedContent.UrlSegment — already obsolete; only the version target was corrected

Test refactoring

The Delivery API tests (tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/) previously used IPublishedContent.UrlSegment heavily as scaffolding — both as a mock-content identity in path-provider lambdas and as the implicit URL-provider return. This PR refactors those tests so:

  • Tests that use the route builder explicitly mock IDocumentUrlService (matches what the production code now reads).
  • The ConfigurePublishedContentMock helper no longer sets c.UrlSegment; the urlSegment parameter is dropped from its signature and its 14 call sites updated.
  • The redundant Assert.AreEqual("...", content.UrlSegment) lines in PublishedContentCacheTests are removed (the same tests already verify content identity via Key and ContentType.Alias).

End result: zero IPublishedContent.UrlSegment references in the Delivery API test folder. When the property is removed in v20, no further test changes are needed.

Behaviour note: preview mode

The new code passes isDraft (derived from preview state) to GetUrlSegment() consistently across all migrated paths:

  • ApiContentRouteBuilder and RequestStartItemProvider use IRequestPreviewService.IsPreview() (already injected).
  • PublishedContentUrlSegmentResolver (and therefore PublishedContentBase.UrlSegment, HybridCache PublishedContent.UrlSegment, and FriendlyPublishedContentExtensions.UrlSegment) detect preview state via the ambient IUmbracoContext.InPreviewMode.
  • RequestRedirectService correctly uses isDraft: false — URL tracking is published-only.

The deleted extension method always returned the published URL segment regardless of preview state (it read from IPublishedContent.Cultures, which only ever exposed published data). So in preview mode, route building and the Razor @Model.UrlSegment() helper now return the draft URL segment where they previously returned the published one. This is more correct (preview should preview the draft URL).

Breaking changes

  • PublishedContentExtensions.UrlSegment(this IPublishedContent, IVariationContextAccessor?, string?) extension method is removed. External callers should switch to IDocumentUrlService.GetUrlSegment(documentKey, culture, isDraft).
  • IPublishedContent.UrlSegment (interface property) and the four remaining implementations (PublishedContentWrapped, InternalPublishedContent, PublishedExternalMember, HybridCache PublishedContent) are now [Obsolete] and emit CS0618 warnings when consumed. The property remains functional in v18.
  • The Razor helper FriendlyPublishedContentExtensions.UrlSegment(this IPublishedContent, string?) is not obsolete — Razor templates calling @Model.UrlSegment(culture) keep working and require no migration.

Testing

Solution builds and tests pass.

Can also use the following Razor code to verify that the different, existing methods for retrieving URL segments reveal the same result:

@using Microsoft.Extensions.DependencyInjection
@using Umbraco.Cms.Core.Services
@using Umbraco.Cms.Core.Web

<div>
    @{
        // Pull services from the request scope.
        var documentUrlService = Context.RequestServices.GetRequiredService<IDocumentUrlService>();
        var variationContextAccessor = Context.RequestServices.GetRequiredService<IVariationContextAccessor>();
        var umbracoContextAccessor = Context.RequestServices.GetRequiredService<IUmbracoContextAccessor>();

        var ambientCulture = variationContextAccessor.VariationContext?.Culture ?? string.Empty;
        var inPreview = umbracoContextAccessor.TryGetUmbracoContext(out var umbCtx) && umbCtx.InPreviewMode;

        // 1. Friendly extension — NOT obsolete. Routes through IDocumentUrlService internally.
        var fromFriendlyExtension = Model.UrlSegment();
        var fromFriendlyExtensionWithCulture = Model.UrlSegment(ambientCulture);

        // 2. Obsolete property — still functional in v18, emits CS0618 at compile time.
#pragma warning disable CS0618
        var fromObsoleteProperty = Model.UrlSegment;
#pragma warning restore CS0618

        // 3. IDocumentUrlService directly — the strategic v20+ API.
        var fromServiceMatchingPreview = documentUrlService.GetUrlSegment(Model.Key, ambientCulture, inPreview);
        var fromServicePublished = documentUrlService.GetUrlSegment(Model.Key, ambientCulture, isDraft: false);
        var fromServiceDraft = documentUrlService.GetUrlSegment(Model.Key, ambientCulture, isDraft: true);
    }

    <h2>UrlSegment verification</h2>
    <dl>
        <dt>InPreviewMode</dt>
        <dd><strong>@inPreview</strong></dd>
        <dt>Ambient culture</dt>
        <dd><strong>@(string.IsNullOrEmpty(ambientCulture) ? "(invariant)" : ambientCulture)</strong></dd>
        <dt>Content key</dt>
        <dd><code>@Model.Key</code></dd>
    </dl>

    <table border="1" cellpadding="6" cellspacing="0">
        <thead>
            <tr><th>Source</th><th>Value</th><th>Expected</th></tr>
        </thead>
        <tbody>
            <tr>
                <td><code>@@Model.UrlSegment()</code></td>
                <td>@fromFriendlyExtension</td>
                <td>Equal to service-with-current-preview-state below</td>
            </tr>
            <tr>
                <td><code>@@Model.UrlSegment(ambientCulture)</code></td>
                <td>@fromFriendlyExtensionWithCulture</td>
                <td>Equal to the row above</td>
            </tr>
            <tr>
                <td><code>@@Model.UrlSegment</code> (obsolete property)</td>
                <td>@fromObsoleteProperty</td>
                <td>Equal to <code>@@Model.UrlSegment()</code> — emits CS0618 warning</td>
            </tr>
            <tr>
                <td><code>IDocumentUrlService.GetUrlSegment(key, c, isDraft: @inPreview)</code></td>
                <td>@fromServiceMatchingPreview</td>
                <td>Equal to <code>@@Model.UrlSegment()</code></td>
            </tr>
            <tr>
                <td><code>IDocumentUrlService.GetUrlSegment(key, c, isDraft: false)</code></td>
                <td>@fromServicePublished</td>
                <td>Always the published segment</td>
            </tr>
            <tr>
                <td><code>IDocumentUrlService.GetUrlSegment(key, c, isDraft: true)</code></td>
                <td>@fromServiceDraft</td>
                <td>Always the draft segment</td>
            </tr>
        </tbody>
    </table>
</div>

…ify obsoletion of UrlSegment property for Umbraco 19.
Copilot AI review requested due to automatic review settings May 4, 2026 05:37
@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 5m 4s —— View job


PR Review

Target: origin/v18/dev · Based on commit: 2a2556b4 · Skipped: 0 noise files

Removes the [Obsolete]-for-v18 UrlSegment extension method from PublishedContentExtensions and migrates its three Delivery API call sites to IDocumentUrlService.GetUrlSegment(). Simultaneously starts a fresh deprecation cycle for IPublishedContent.UrlSegment and its four implementations, sharing a new PublishedContentUrlSegmentResolver helper to maintain backward compatibility through the deprecation window.

  • Modified public API: PublishedContentExtensions.UrlSegment(this IPublishedContent, IVariationContextAccessor?, string?) removed (valid — was [Obsolete] for v18); IPublishedContent.UrlSegment, PublishedContentWrapped.UrlSegment, PublishedExternalMember.UrlSegment newly [Obsolete]; new public static class PublishedContentUrlSegmentResolver (also [Obsolete]); FriendlyPublishedContentExtensions.UrlSegment(this IPublishedContent, string?) now [Obsolete]
  • Affected implementations (outside this PR): Any external IPublishedContent implementation will now see CS0618 on UrlSegment override sites (warning only, not a compile error)
  • Breaking changes: Removal of PublishedContentExtensions.UrlSegment is valid per policy. RequestRedirectService and RequestStartItemProvider constructor signatures changed, but both are internal sealed — no external consumers
  • Other changes: In preview mode, ApiContentRouteBuilder and RequestStartItemProvider now resolve the draft URL segment via IDocumentUrlService.GetUrlSegment(..., isDraft: isPreview) rather than always the published one — a behavior change noted and justified in the PR description. RequestRedirectService correctly keeps isDraft: false.

Note

Complexity advisory — This PR may benefit from splitting.

  • Layer spread: 4 architectural layers touched (Core / PublishedCache.HybridCache / Web.Common / Api.Delivery) across 12 production files. In practice the deprecation chain requires all layers to change atomically to avoid a broken intermediate state, so splitting is unlikely to be practical here.

This is an observation, not a blocker. The full review follows below.


Important

  • src/Umbraco.Core/Models/PublishedContent/IPublishedContent.cs:15: [Obsolete] says "Scheduled for removal in Umbraco 19" but current major is 18 and CLAUDE.md §5.4 requires N+2 = 20. Same issue on every other newly-added or updated marker in this PR: PublishedContentBase.UrlSegment, PublishedContentWrapped.UrlSegment, InternalPublishedContent.UrlSegment, PublishedExternalMember.UrlSegment, HybridCache.PublishedContent.UrlSegment, FriendlyPublishedContentExtensions.UrlSegment, and PublishedContentUrlSegmentResolver (class + remarks). All should read "Scheduled for removal in Umbraco 20." → Update all eight occurrences.

Suggestions

  • tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs:864: ConfigurePublishedContentMock now passes null for the URL segment in PublishedCultureInfo. This is correct given the refactor, but it silently means any test that still asserts on content.Cultures[string.Empty].UrlSegment will now compare against null. Worth a comment in the helper noting that URL segments are intentionally omitted and should be wired up via IDocumentUrlService mock instead.

  • src/Umbraco.Core/Models/PublishedContent/PublishedContentUrlSegmentResolver.cs: Being public but [Obsolete] at birth is a reasonable pattern for a short-lived bridge helper, but having both a class-level [Obsolete] and an unlabelled public method means callers only see the class-level warning. The Resolve method could also carry [Obsolete] to make suppression explicit at each call site — consistent with how PublishedContentBase.UrlSegment suppresses at its single call site.


Request Changes

The [Obsolete] removal version is off by one across all eight newly-marked members — "Umbraco 19" should be "Umbraco 20" per the N+2 policy. Please update before merging.

Comment thread src/Umbraco.Core/Models/PublishedContent/IPublishedContent.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 continues the v18 obsolete-cleanup by removing the obsolete PublishedContentExtensions.UrlSegment(...) extension method and migrating production + unit test call sites to IDocumentUrlService.GetUrlSegment(...). It also starts deprecating the underlying IPublishedContent.UrlSegment property by adding new [Obsolete] markers and consolidating legacy URL-segment resolution logic into a shared helper.

Changes:

  • Removed the obsolete PublishedContentExtensions.UrlSegment(...) extension method and updated callers to use IDocumentUrlService.GetUrlSegment(...).
  • Added new [Obsolete] markers for IPublishedContent.UrlSegment and implementations, plus introduced PublishedContentUrlSegmentResolver to keep legacy behavior centralized.
  • Refactored Delivery API unit tests to avoid IPublishedContent.UrlSegment and to mock IDocumentUrlService instead.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs Removes UrlSegment-based assertions and updates mocks to align with IDocumentUrlService route resolution.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs Stops relying on IPublishedContent.UrlSegment and uses explicit URL return values in provider mocks.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs Removes UrlSegment setup from test scaffolding and updates URL provider mocks accordingly.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs Replaces content.UrlSegment usage with explicit URL segment values in test setup.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MediaBuilderTests.cs Removes reliance on media.UrlSegment and adjusts media URL provider test setup.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs Refactors shared mock helper to no longer set UrlSegment and adjusts cultures setup.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs Updates route-builder tests to mock IDocumentUrlService.GetUrlSegment(...) and threads the mock through helpers.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs Replaces content.UrlSegment usage with explicit URL segment values in test URL provider setup.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs Removes UrlSegment reliance in content path/route builder mocks and assertions.
src/Umbraco.Web.Common/Extensions/FriendlyPublishedContentExtensions.cs Keeps Razor-facing UrlSegment(...) but routes through the shared resolver and marks it obsolete.
src/Umbraco.PublishedCache.HybridCache/PublishedContent.cs Updates UrlSegment implementation to use shared resolver and refreshes obsolete messaging.
src/Umbraco.Core/Security/PublishedExternalMember.cs Adds [Obsolete] marker to UrlSegment implementation for external members.
src/Umbraco.Core/PublishedCache/Internal/InternalPublishedContent.cs Adds [Obsolete] marker to UrlSegment on internal published content test harness.
src/Umbraco.Core/Models/PublishedContent/PublishedContentWrapped.cs Adds [Obsolete] marker and warning suppression around UrlSegment passthrough.
src/Umbraco.Core/Models/PublishedContent/PublishedContentUrlSegmentResolver.cs Introduces shared legacy URL-segment resolution helper (marked obsolete).
src/Umbraco.Core/Models/PublishedContent/PublishedContentBase.cs Re-implements UrlSegment using shared resolver now that the extension method is removed.
src/Umbraco.Core/Models/PublishedContent/IPublishedContent.cs Marks IPublishedContent.UrlSegment obsolete to start deprecation cycle.
src/Umbraco.Core/Extensions/PublishedContentExtensions.cs Removes the obsolete UrlSegment(...) extension method implementation.
src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs Switches root-path segment resolution to IDocumentUrlService.GetUrlSegment(...) (supports draft in preview).
src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs Injects IDocumentUrlService and uses it for matching the start item by URL segment.
src/Umbraco.Cms.Api.Delivery/Services/RequestRedirectService.cs Injects IDocumentUrlService and uses it for resolving the start-item segment when forming redirect lookup paths.

Comment thread src/Umbraco.Core/Models/PublishedContent/IPublishedContent.cs Outdated
Comment thread src/Umbraco.Core/Models/PublishedContent/PublishedContentUrlSegmentResolver.cs Outdated
Comment thread src/Umbraco.Web.Common/Extensions/FriendlyPublishedContentExtensions.cs Outdated
Comment thread src/Umbraco.Core/Security/PublishedExternalMember.cs Outdated
Comment thread src/Umbraco.Core/Models/PublishedContent/PublishedContentWrapped.cs Outdated
Comment thread src/Umbraco.Core/PublishedCache/Internal/InternalPublishedContent.cs Outdated
Comment thread src/Umbraco.PublishedCache.HybridCache/PublishedContent.cs Outdated
@kjac kjac self-requested a review May 4, 2026 10:40
Comment thread src/Umbraco.Core/PublishedCache/Internal/InternalPublishedContent.cs Outdated

@kjac kjac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@kjac kjac enabled auto-merge (squash) May 4, 2026 11:30
@kjac kjac merged commit 1a87feb into v18/dev May 4, 2026
26 of 27 checks passed
@kjac kjac deleted the v18/task/further-todos-and-obsoletes-11 branch May 4, 2026 13:37
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.

4 participants