Skip to content

Revert #16058 for non-applicable settings#19229

Merged
AndyButland merged 5 commits intorelease/16.0from
v16/fix/revert-sets-for-configs
May 5, 2025
Merged

Revert #16058 for non-applicable settings#19229
AndyButland merged 5 commits intorelease/16.0from
v16/fix/revert-sets-for-configs

Conversation

@kjac
Copy link
Contributor

@kjac kjac commented May 5, 2025

Prerequisites

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

Description

#16058 had the unintended side effect of breaking a few of our configurations:

  • The LoginRedirectUrls and LogoutRedirectUrls of DeliveryApiSettings.MemberAuthorization.AuthorizationCodeFlow.
  • The UserDefinedCharCollection of RequestHandlerSettings.

For some reason, it seems that IOptions won't parse an ISet<T> unless T is a simple type like string. The affected configuration collections will simply remain empty, no matter what's in appsettings.json.

This fixes it up by replacing ISet<T> with IEnumerable<T>.

Testing this PR

The easiest way to test this is to add the affected configuration to appsettings.json, then create a custom controller that outputs the configuration. Something like this:

using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;

namespace Umbraco.Cms.Web.UI.Custom;

[ApiController]
public class MyTestController : ControllerBase
{
    private readonly RequestHandlerSettings _requestHandlerSettings;
    private readonly DeliveryApiSettings _deliveryApiSettings;

    public MyTestController(IOptions<RequestHandlerSettings> requestHandlerSettings, IOptions<DeliveryApiSettings> deliveryApiSettings)
    {
        _requestHandlerSettings = requestHandlerSettings.Value;
        _deliveryApiSettings = deliveryApiSettings.Value;
    }

    [HttpGet("my/test")]
    public IActionResult Get() => Ok(new
    {
        RequestHandlerSettings = _requestHandlerSettings,
        DeliveryApiSettings = _deliveryApiSettings
    });
}

@kjac kjac marked this pull request as ready for review May 5, 2025 11:58
@kjac kjac added status/regression A previously working feature that has broken or changed behavior unexpectedly release/16.0.0 labels May 5, 2025
Copy link
Contributor

@AndyButland AndyButland 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 and tests out as expected.

@AndyButland AndyButland merged commit 6518a26 into release/16.0 May 5, 2025
21 checks passed
@AndyButland AndyButland deleted the v16/fix/revert-sets-for-configs branch May 5, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/16.0.0 status/regression A previously working feature that has broken or changed behavior unexpectedly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants