Introduce Fluent API for Shape Placement and Add Unit Tests

[MikeAlhayek]

No description provided.

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[gvkries]

I like this, but we should not start to replace our current Location calls with this, as it adds a lot of additional allocations.

There is small change required, because of PR #18867.

[gvkries]

I think all of this was already in the test. But more tests doesn't hurt anyway 😊

[gvkries]

Additionally, I think we should merge #18860 first. Okay?

[MikeAlhayek]

@gvkries the main places to use Fluent API is where we need to use Card, Tab or Column which improves code readability even at a very small additional allocations. So we should use these extensions even internally in this case but not by default when the string is easy to read like Content or Content:5. So I agree in most cases these extensions won't be needed internally but should still use them when we tab, card or columns are used

[gvkries]

Maybe we can simplify this a little bit, the builder seems overly complex for our simple location syntax. A single builder class should be sufficient.

Here is a alternative implementation Copilot came up with:

using System.Text;

namespace OrchardCore.DisplayManagement.Descriptors;

/// <summary>
/// Single-object fluent builder for placement location strings.
/// Format: [/]{Zone}[:{Position}][#{Tab}][@{Group}][%{Card}][|{Column}]
/// Validation is performed in Build(); method call order does not matter.
/// </summary>
public sealed class PlacementLocationBuilder
{
    private string _zone;
    private string _position;

    // Stored already in serialized form with optional ";{groupPosition}"
    private string _tab;

    private string _group;

    // Stored already in serialized form with optional ";{groupPosition}"
    private string _card;

    // Stored already in serialized form with optional "_{width}" and ";{columnPosition}"
    private string _column;

    private bool _isLayoutZone;

    // Optional "configured twice" detection. Remove if you want last-wins silently.
    private bool _zoneSet, _positionSet, _tabSet, _groupSet, _cardSet, _columnSet;

    public PlacementLocationBuilder Zone(string zone)
    {
        ArgumentException.ThrowIfNullOrEmpty(zone);

        if (_zoneSet) ThrowAlreadyConfigured(nameof(Zone));
        _zoneSet = true;

        _zone = zone;
        return this;
    }

    public PlacementLocationBuilder Position(string position)
    {
        // Position is optional; allow null/empty to mean "no explicit position".
        if (_positionSet) ThrowAlreadyConfigured(nameof(Position));
        _positionSet = true;

        _position = position;
        return this;
    }

    /// <summary>
    /// Convenience: sets Zone + optional Position in one call.
    /// </summary>
    public PlacementLocationBuilder Zone(string zone, string position)
        => Zone(zone).Position(position);

    public PlacementLocationBuilder AsLayoutZone()
    {
        _isLayoutZone = true;
        return this;
    }

    public PlacementLocationBuilder Group(string name)
    {
        ArgumentException.ThrowIfNullOrEmpty(name);

        if (_groupSet) ThrowAlreadyConfigured(nameof(Group));
        _groupSet = true;

        _group = name;
        return this;
    }

    public PlacementLocationBuilder Tab(string name, string position = null)
    {
        ArgumentException.ThrowIfNullOrEmpty(name);

        if (_tabSet) ThrowAlreadyConfigured(nameof(Tab));
        _tabSet = true;

        _tab = string.IsNullOrEmpty(position) ? name : $"{name};{position}";
        return this;
    }

    public PlacementLocationBuilder Card(string name, string position = null)
    {
        ArgumentException.ThrowIfNullOrEmpty(name);

        if (_cardSet) ThrowAlreadyConfigured(nameof(Card));
        _cardSet = true;

        _card = string.IsNullOrEmpty(position) ? name : $"{name};{position}";
        return this;
    }

    public PlacementLocationBuilder Column(string name, string position = null, string width = null)
    {
        ArgumentException.ThrowIfNullOrEmpty(name);

        if (_columnSet) ThrowAlreadyConfigured(nameof(Column));
        _columnSet = true;

        var value = name;

        if (!string.IsNullOrEmpty(width))
        {
            value = $"{value}_{width}";
        }

        if (!string.IsNullOrEmpty(position))
        {
            value = $"{value};{position}";
        }

        _column = value;
        return this;
    }

    /// <summary>
    /// Validates and builds the placement string. Call order does not matter.
    /// </summary>
    public string Build()
    {
        // Validate required parts
        if (string.IsNullOrEmpty(_zone))
        {
            throw new InvalidOperationException("Zone is required. Call Zone() before Build().");
        }

        // Validate hierarchy per README (Zone → Tab → Card → Column).
        // With this builder, "skipping levels" is allowed:
        // - Card without Tab: OK
        // - Column without Card/Tab: OK
        // - Group anywhere: OK
        //
        // So what is invalid? Mostly: nothing structural beyond requiring a Zone.
        // (If you later add more constraints—e.g. "Tab cannot be used with layout zones"—enforce here.)

        // Serialize in canonical order required by the README:
        // [/]{Zone}[:{Position}][#{Tab}][@{Group}][%{Card}][|{Column}]
        var sb = new StringBuilder();

        if (_isLayoutZone)
        {
            sb.Append('/');
        }

        sb.Append(_zone);

        if (!string.IsNullOrEmpty(_position))
        {
            sb.Append(PlacementInfo.PositionDelimiter);
            sb.Append(_position);
        }

        if (!string.IsNullOrEmpty(_tab))
        {
            sb.Append(PlacementInfo.TabDelimiter);
            sb.Append(_tab);
        }

        if (!string.IsNullOrEmpty(_group))
        {
            sb.Append(PlacementInfo.GroupDelimiter);
            sb.Append(_group);
        }

        if (!string.IsNullOrEmpty(_card))
        {
            sb.Append(PlacementInfo.CardDelimiter);
            sb.Append(_card);
        }

        if (!string.IsNullOrEmpty(_column))
        {
            sb.Append(PlacementInfo.ColumnDelimiter);
            sb.Append(_column);
        }

        return sb.ToString();
    }

    public override string ToString() => Build();

    public void Reset()
    {
        _zone = null;
        _position = null;
        _tab = null;
        _group = null;
        _card = null;
        _column = null;
        _isLayoutZone = false;

        _zoneSet = _positionSet = _tabSet = _groupSet = _cardSet = _columnSet = false;
    }

    private static void ThrowAlreadyConfigured(string methodName)
        => throw new InvalidOperationException($"{methodName} was already configured on this builder instance.");
}

[gvkries]

@MikeAlhayek I updated your PR with the recent changes I did for placement.

[MikeAlhayek]

@gvkries I think using the current approach will allow us to use nesting without getting lost in the chaining levels.. So we can support Zone > Tab > Card > Column type of nesting. Feel free to make any modification to the PR.

[sebastienros]

Can we just build PlacementInfo instances and only build these strings just for storing or parsing them?

[sebastienros]

No need for a strinbuilder allocation. We can use a ZString or String.Create directly since we can compute the size of the final string

[sebastienros]

Why not ToString() ? Like StringBuilder does.

Also maybe should cache the result in case it's called multiple times.

[MikeAlhayek]

@gvkries @sebastienros I made more changes to address some of your feedback. let me know if you have anything else.