Skip to content

Routing: Added method to IDocumentUrlService for retrieving document key from URI (closes #20666)#20673

Merged
nikolajlauridsen merged 1 commit intov17/devfrom
v17/feature/get-document-key-by-url
Oct 29, 2025
Merged

Routing: Added method to IDocumentUrlService for retrieving document key from URI (closes #20666)#20673
nikolajlauridsen merged 1 commit intov17/devfrom
v17/feature/get-document-key-by-url

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Addresses: #20666

Description

This PR looks to provide a solution to the concern raised in the linked issue that it's not so easy with the IDocumentUrlService to get a document's key from it's path when domains are involved.

See the discussion #20666 (comment) and #20666 (comment) for why that's currently the case and a suggestion to resolve, which is what I've done in this PR.

IDocumentUrlService now has a GetDocumentKeyByUri (to go with the existing GetDocumentKeyByRoute) that can be called providing a Uri. The domain will be parsed as is done during routing to then be able to delegate to GetDocumentKeyByRoute.

Testing

The following code for a document with or without domains associated should now successfully retrieve the document's key.

@using Microsoft.AspNetCore.Http.Extensions
@using Umbraco.Cms.Core.Services
@using Umbraco.Cms.Web.Common.PublishedModels;
@inject IDocumentUrlService DocumentUrlService
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage
@{
	Layout = null;
}

@{
    var currentRequest = Context.Request;
    var currentUri = new Uri(currentRequest.GetDisplayUrl());
}
<p>Current path: @currentRequest.Path</p>
<p>Found page guid: @DocumentUrlService.GetDocumentKeyByRoute(currentRequest.Path, null, null, false)</p>
<p>Current URL: @currentUri</p>
<p>Found page guid: @DocumentUrlService.GetDocumentKeyByUri(currentUri, false)</p>

Integration tests have also been added to verify the functionality.

Copilot AI review requested due to automatic review settings October 28, 2025 14:17
Copy link
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 adds a new GetDocumentKeyByUri method to the IDocumentUrlService interface to enable retrieval of document keys from URIs when domains are involved. This addresses the limitation where GetDocumentKeyByRoute alone was insufficient when working with domains.

Key Changes:

  • Added GetDocumentKeyByUri method that parses domains from URIs and delegates to GetDocumentKeyByRoute
  • Updated constructor to inject WebRoutingSettings and IDefaultCultureAccessor dependencies needed for domain resolution
  • Added comprehensive integration tests covering both domain and non-domain scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Umbraco.Core/Services/IDocumentUrlService.cs Adds GetDocumentKeyByUri method signature with default implementation
src/Umbraco.Core/Services/DocumentUrlService.cs Implements GetDocumentKeyByUri method and updates constructor with required dependencies
src/Umbraco.Core/Routing/DomainUtilities.cs Removes unused imports
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentUrlServiceTests.cs Adds integration tests for the new method covering domain and non-domain scenarios
tests/Umbraco.Tests.Common/Builders/TemplateBuilder.cs Makes template name parameter configurable

Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good, tests good 👍

@nikolajlauridsen nikolajlauridsen merged commit e7ccfaa into v17/dev Oct 29, 2025
26 checks passed
@nikolajlauridsen nikolajlauridsen deleted the v17/feature/get-document-key-by-url branch October 29, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants