Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Helper Methods for adding Transform at Runtime #317

Closed
Kahbazi opened this issue Jul 9, 2020 · 5 comments · Fixed by #324
Closed

Add Helper Methods for adding Transform at Runtime #317

Kahbazi opened this issue Jul 9, 2020 · 5 comments · Fixed by #324
Assignees
Labels
Type: Idea This issue is a high-level idea for discussion.

Comments

@Kahbazi
Copy link
Collaborator

Kahbazi commented Jul 9, 2020

What should we add or change to make your life better?

Add helper methods for each Transform.

public static class TransformHelper
{
    public static void AddPathPrefixTransform(this ProxyRoute proxyRoute, PathString prefix)
    {
        proxyRoute.Transforms.Add(new Dictionary<string, string>
        {
            ["PathPrefix"] = prefix,
        });
    }

    public static void AddPathRemovePrefixTransform(this ProxyRoute proxyRoute, PathString prefix)
    {
        proxyRoute.Transforms.Add(new Dictionary<string, string>
        {
            ["PathRemovePrefix"] = prefix,
        });
    }
}

Why is this important to you?

This methods can be used in IProxyConfigFilter.

@Kahbazi Kahbazi added the Type: Idea This issue is a high-level idea for discussion. label Jul 9, 2020
@Tratcher
Copy link
Member

Tratcher commented Jul 9, 2020

This is right in line with #60, though a big part of that one is to let you define new custom transforms as well.

@samsp-msft samsp-msft added this to the 1.0.0 milestone Jul 9, 2020
@samsp-msft samsp-msft changed the title Add Helper Methods for adding Transform in Runtime Add Helper Methods for adding Transform at Runtime Jul 9, 2020
@samsp-msft
Copy link
Contributor

If you want to contribute this - lets schedule an ad-hoc design meeting before working on the PR. Ping me to get that setup.

@Kahbazi
Copy link
Collaborator Author

Kahbazi commented Jul 9, 2020

@samsp-msft yeah, I would like to contribute this.

@Kahbazi
Copy link
Collaborator Author

Kahbazi commented Jul 13, 2020

For adding Transforms with parameters, we could either have two separate methods for each or use an Enum in input.
1. Multiple methods

public static void AddSetRequestHeaderTransform(this ProxyRoute proxyRoute, string headerName, string value)
{
    proxyRoute.Transforms.Add(new Dictionary<string, string>
    {
        ["RequestHeader"] = headerName,
        ["Set"] = value
    });
}

public static void AddAppendRequestHeaderTransform(this ProxyRoute proxyRoute, string headerName, string value)
{
    proxyRoute.Transforms.Add(new Dictionary<string, string>
    {
        ["RequestHeader"] = headerName,
        ["Append"] = value
    });
}

2. Multiple parameters

public static void AddSetRequestHeaderTransform(this ProxyRoute proxyRoute, string headerName, string value, TransformHeaderType transformHeaderType)
{
    var type = transformHeaderType switch
    {
        TransformHeaderType.Set => "Set",
        TransformHeaderType.Append => "Append",
        _ => throw new System.ArgumentException(nameof(transformHeaderType));
    };

    proxyRoute.Transforms.Add(new Dictionary<string, string>
    {
        ["RequestHeader"] = headerName,
        [type] = value
    });
}

Which one do you prefer?

@Tratcher
Copy link
Member

I'd prefer multiple overloads (option 1), that avoids introducing more enum types that aren't widely used. Alternatively it could be a bool parameter, that's what we use for the internal implementation.

public RequestHeaderValueTransform(string value, bool append)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Idea This issue is a high-level idea for discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants