Efficient removing of QueryString parameters

[jmezach]

Is your feature request related to a problem? Please describe.

I'm currently working on pull request Microsoft/reverse-proxy#301 for YARP to add support for transforming query parameters on the incoming request to downstream services. One thing that seems useful is to allow YARP to remove query parameters on the incoming request when they flow to the downstream by configuring a transformation. Currently this involves parsing the entire query string into a Dictionary using QueryHelpers.Parse(), removing the parameter we want and then using the QueryBuilder class to put it all back into a QueryString instance.

This feels a little bit wasteful in terms of performance since we're allocating a dictionary and doing a whole bunch of string parsing, only to remove a single query parameter.

Describe the solution you'd like

Since the QueryString class already has an Add method it makes sense to add a Remove (or Delete if that's more appropriate) method to the QueryString that strips a query parameter from the query string in the most efficient way and returns a new QueryString.

Additional context

[lawrencek76]

I ran into the same situation in a tag helper.

I would also need to add Update to the list of operations. This proves very useful in tag helpers that want to modify the querystrings of urls.

I have an implementation of both Remove and Update that are inspired by the existing ParseNullableQuery method but is based on string builder instead of the dictionary.

@BrennanConroy I see the help wanted tag is removed does that mean submitting a pr to add these would be a waste of time?

[BrennanConroy]

I don't think we're against it, it's just probably a larger work item than other help wanted issues.

Can you first propose an API here?

[lawrencek76]

API Proposal

    public static class QueryHelpers
    {
        /// <summary>
        /// Remove the given <paramref name="paramName"/> from the URI if it exists.  
        /// If the <paramref name="paramName"/> appears multiple times in the querystring of <paramref name="uri"/> all instances will be removed.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="paramName">The name of the param key to remove.</param>
        /// <returns>The reduced uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramName"/> is <c>null</c>.</exception>
        public static string RemoveQueryString(string uri, string paramName) {}

        /// <summary>
        /// Remove any query parameters given in <paramref name="queryParamsToRemove"/> from the URI if they exist.  
        /// All occurances of any given parameter will be removed from the querystring of <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="queryParamsToRemove">A collection of query names to remove</param>
        /// <returns>The reduced uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="queryParamsToRemove"/> is <c>null</c>.</exception>
        public static string RemoveQueryString(string uri, IEnumerable<string> queryParamsToRemove) {}

        /// <summary>
        /// Remove any query parameters given in <paramref name="queryParamsToRemove"/> from the URI if they exist.  
        /// All occurances of any given parameter will be removed from the querystring of <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="queryParamsToRemove">A HashSet of query names to remove</param>
        /// <returns>The reduced uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="queryParamsToRemove"/> is <c>null</c>.</exception>
        public static string RemoveQueryString(string uri, HashSet<string> queryParamsToRemove) {} // Ultimately called by above overloads of RemoveQueryString

        /// <summary>
        /// Updates the given <paramref name="paramName"/> with the value in <paramref name="paramValue"/> if it exists or appends it to the <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="paramName">The name of the param key to update.</param>
        /// <returns>The updated uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramName"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramValue"/> is <c>null</c>.</exception>
        public static string UpdateQueryString(string uri, string paramName, string paramValue) {}

        /// <summary>
        /// Updates the given query parameter keys and values with those provided in <paramref name="paramsToUpdate"/>,  
        /// matching existing keys and values are replaced by those provided new keys are appended to the <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="paramsToUpdate">A dictionary of query keys and values to update.</param>
        /// <returns>The updated uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramsToUpdate"/> is <c>null</c>.</exception>
        public static string UpdateQueryString(string uri, IDictionary<string, string?> paramsToUpdate) {}

        /// <summary>
        /// Updates the given query parameter keys and values with those provided in <paramref name="paramsToUpdate"/>,  
        /// matching existing keys and values are replaced by those provided new keys are appended to the <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="paramsToUpdate">A collection of query names and values to update.</param>
        /// <returns>The updated uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramsToUpdate"/> is <c>null</c>.</exception>
        public static string UpdateQueryString(string uri, IEnumerable<KeyValuePair<string, StringValues>> paramsToUpdate) {}

        /// <summary>
        /// Updates the given query parameter keys and values with those provided in <paramref name="paramsToUpdate"/>,  
        /// matching existing keys and values are replaced by those provided new keys are appended to the <paramref name="uri"/>.
        /// </summary>
        /// <param name="uri">The base URI.</param>
        /// <param name="paramsToUpdate">A collection of query names and values to update.</param>
        /// <returns>The updated uri</returns>
        /// <exception cref="ArgumentNullException"><paramref name="uri"/> is <c>null</c>.</exception>
        /// <exception cref="ArgumentNullException"><paramref name="paramsToUpdate"/> is <c>null</c>.</exception>
        public static string UpdateQueryString(string uri, IEnumerable<KeyValuePair<string, string?>> paramsToUpdate) {}  // Ultimately called by above overloads of UpdateQueryString
     }

[Tratcher]

FYI YARP ended up using a different approach with a temporary collection so that it could batch changes. QueryHelpers working on raw strings are in-efficient if you need to do multiple operations. https://github.com/microsoft/reverse-proxy/blob/cee49090bc214c2ab3617837f32e47e350935925/src/ReverseProxy/Service/RuntimeModel/Transforms/QueryTransformContext.cs

@lawrencek76 would this approach make sense for your scenario?

[pranavkm]

We are going to wait until we have additional feedback for this API. At this point we're not considering adding this. @Tratcher could you follow up with them to figure out a suitable alternative?

[lawrencek76]

@Tratcher it does not really help as best I can tell since I have uris that are coming in as a string type and need to leave as string. Detecting a no-op situation would be an optimization I could use. Thank you for pointing me to that example.

@pranavkm I am not sure if it was clear that I already have these api's implemented. I was hoping to contribute the code for them if possible since I looked in QueryHelpers for something like these and it seems the best place for them if they were to exist. In any case, no alternative is necessary. Thank you for considering the api.

[Tratcher]

Given how inefficient the QueryHelpers APIs are, especially for multiple operations, we want to discourage their use by not expanding them. Thanks for being willing to share though.