[UriBuilder] Add function to add OData query parameters with preserve…

…d $ signs (#503)

#### Summary
This PR tackles an issue with URL encoding of OData parameters.
Some servers, such as AppSource marketplace, don't interpret the encoded
OData query parameter keys (`%24filter`) as equivalent to their
unencoded form (`$filter`). Technically, this is by design given the
encoding rules that Business Central uses, but this fix enables the
caller of the AL code to decide whether to preserve the `$` signs or not
when adding URL query parameters.

#### Work Item(s)
Fixes
[AB#497036](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/497036)

src/System Application/App/URI/src/UriBuilder.Codeunit.al

@@ -175,6 +175,7 @@ codeunit 3061 "Uri Builder"
     /// <param name="Flag">A flag to add to the query string of this UriBuilder. This value will be encoded before being added to the URI query string. Cannot be empty.</param>
     /// <error>If the provided <paramref name="Flag"/> is empty.</error>
     /// <remarks>This function could alter the order of the existing query string parts. For example, if the previous URL was "https://microsoft.com?foo=bar&amp;john=doe" and the new flag is "contoso", the result could be "https://microsoft.com?john=doe&amp;foo=bar&amp;contoso".</remarks>
+    /// <remarks>This function will encode OData parameters (and will ensure encoding of the ones added to the <see cref="UriBuilder"/> before). For example "$skip=3" will result in "%24skip=3". Some servers don't support the encoded version. Use <see cref="AddODataQueryParameter"/> in these cases instead.</remarks>
     procedure AddQueryFlag(Flag: Text)
     begin
         UriBuilderImpl.AddQueryFlag(Flag, Enum::"Uri Query Duplicate Behaviour"::"Overwrite All Matching");
@@ -190,6 +191,7 @@ codeunit 3061 "Uri Builder"
     /// <error>If the provided <paramref name="DuplicateAction"/> is <c>"Throw Error"</c>.</error>
     /// <error>If the provided <paramref name="DuplicateAction"/> is not a valid value for the enum.</error>
     /// <remarks>This function could alter the order of the existing query string parts. For example, if the previous URL was "https://microsoft.com?foo=bar&amp;john=doe" and the new flag is "contoso=42", the result could be "https://microsoft.com?john=doe&amp;foo=bar&amp;contoso=42".</remarks>
+    /// <remarks>This function will encode OData parameters (and will ensure encoding of the ones added to the <see cref="UriBuilder"/> before). For example "$skip=3" will result in "%24skip=3". Some servers don't support the encoded version. Use <see cref="AddODataQueryParameter"/> in these cases instead.</remarks>
     procedure AddQueryParameter(ParameterKey: Text; ParameterValue: Text; DuplicateAction: Enum "Uri Query Duplicate Behaviour")
     begin
         UriBuilderImpl.AddQueryParameter(ParameterKey, ParameterValue, DuplicateAction);
@@ -202,11 +204,25 @@ codeunit 3061 "Uri Builder"
     /// <param name="ParameterValue">The value for the new query parameter. This value will be encoded before being added to the URI query string. Can be empty.</param>
     /// <error>If the provided <paramref name="ParameterKey"/> is empty.</error>
     /// <remarks>This function could alter the order of the existing query string parts. For example, if the previous URL was "https://microsoft.com?foo=bar&amp;john=doe" and the new flag is "contoso=42", the result could be "https://microsoft.com?john=doe&amp;foo=bar&amp;contoso=42".</remarks>
+    /// <remarks>This function will encode OData parameters (and will ensure encoding of the ones added to the <see cref="UriBuilder"/> before). For example "$skip=3" will result in "%24skip=3". Some servers don't support the encoded version. Use <see cref="AddODataQueryParameter"/> in these cases instead.</remarks>
     procedure AddQueryParameter(ParameterKey: Text; ParameterValue: Text)
     begin
         UriBuilderImpl.AddQueryParameter(ParameterKey, ParameterValue, Enum::"Uri Query Duplicate Behaviour"::"Overwrite All Matching");
     end;
 
+    /// <summary>
+    /// Adds an OData parameter key-value pair to the query string of this UriBuilder (in the form <c>ParameterKey=ParameterValue</c>), preserving the "$" sign in unencoded form. In case the same query key exists already, its value is overwritten.
+    /// </summary>
+    /// <param name="ParameterKey">The key for the new query parameter. This value will be encoded before being added to the URI query string, except for any "$" sign. Cannot be empty.</param>
+    /// <param name="ParameterValue">The value for the new query parameter. This value will be encoded before being added to the URI query string. Can be empty.</param>
+    /// <error>If the provided <paramref name="ParameterKey"/> is empty.</error>
+    /// <remarks>This function could alter the order of the existing query string parts. For example, if the previous URL was "https://microsoft.com?foo=bar&amp;john=doe" and the new flag is "contoso=42", the result could be "https://microsoft.com?john=doe&amp;foo=bar&amp;contoso=42".</remarks>
+    /// <remarks>This function might modify existing query parameters to apply the same encoding rules. For example if the URL is "https://microsoft.com?%24top=1" and the new flag is "$skip=42", the result could be "https://microsoft.com?$skip=42&amp;$top=1".</remarks>
+    procedure AddODataQueryParameter(ParameterKey: Text; ParameterValue: Text)
+    begin
+        UriBuilderImpl.AddODataQueryParameter(ParameterKey, ParameterValue);
+    end;
+
     var
         UriBuilderImpl: Codeunit "Uri Builder Impl.";
 }

src/System Application/App/URI/src/UriBuilderImpl.Codeunit.al

@@ -95,12 +95,22 @@ codeunit 3062 "Uri Builder Impl."
         QueryString := GetQuery();
         ParseParametersAndFlags(QueryString, KeysWithValueList, Flags);
         ProcessNewFlag(Flags, Flag, DuplicateAction);
-        QueryString := CreateNewQueryString(KeysWithValueList, Flags);
+        QueryString := CreateNewQueryString(KeysWithValueList, Flags, false);
 
         SetQuery(QueryString);
     end;
 
     procedure AddQueryParameter(ParameterKey: Text; ParameterValue: Text; DuplicateAction: Enum "Uri Query Duplicate Behaviour")
+    begin
+        AddQueryParameterInternal(ParameterKey, ParameterValue, DuplicateAction, false);
+    end;
+
+    procedure AddODataQueryParameter(ParameterKey: Text; ParameterValue: Text)
+    begin
+        AddQueryParameterInternal(ParameterKey, ParameterValue, Enum::"Uri Query Duplicate Behaviour"::"Overwrite All Matching", true);
+    end;
+
+    local procedure AddQueryParameterInternal(ParameterKey: Text; ParameterValue: Text; DuplicateAction: Enum "Uri Query Duplicate Behaviour"; UseODataEncoding: Boolean)
     var
         KeysWithValueList: Dictionary of [Text, List of [Text]];
         Flags: List of [Text];
@@ -112,7 +122,7 @@ codeunit 3062 "Uri Builder Impl."
         QueryString := GetQuery();
         ParseParametersAndFlags(QueryString, KeysWithValueList, Flags);
         ProcessNewParameter(KeysWithValueList, ParameterKey, ParameterValue, DuplicateAction);
-        QueryString := CreateNewQueryString(KeysWithValueList, Flags);
+        QueryString := CreateNewQueryString(KeysWithValueList, Flags, UseODataEncoding);
 
         SetQuery(QueryString);
     end;
@@ -209,7 +219,7 @@ codeunit 3062 "Uri Builder Impl."
         end;
     end;
 
-    local procedure CreateNewQueryString(KeysWithValueList: Dictionary of [Text, List of [Text]]; Flags: List of [Text]) FinalQuery: Text
+    local procedure CreateNewQueryString(KeysWithValueList: Dictionary of [Text, List of [Text]]; Flags: List of [Text]; UseODataEncoding: Boolean) FinalQuery: Text
     var
         Uri: Codeunit Uri;
         CurrentKey: Text;
@@ -219,7 +229,7 @@ codeunit 3062 "Uri Builder Impl."
         foreach CurrentKey in KeysWithValueList.Keys() do begin
             KeysWithValueList.Get(CurrentKey, CurrentValues);
             foreach CurrentValue in CurrentValues do
-                FinalQuery += '&' + Uri.EscapeDataString(CurrentKey) + '=' + Uri.EscapeDataString(CurrentValue);
+                FinalQuery += '&' + EncodeParameterKey(CurrentKey, UseODataEncoding) + '=' + Uri.EscapeDataString(CurrentValue);
         end;
 
         foreach CurrentKey in Flags do
@@ -228,6 +238,24 @@ codeunit 3062 "Uri Builder Impl."
         FinalQuery := DelChr(FinalQuery, '<', '&');
     end;
 
+    local procedure EncodeParameterKey(ParameterKey: Text; UseODataEncoding: Boolean) EncodedParameterKey: Text
+    var
+        Uri: Codeunit Uri;
+    begin
+        // Uri.EscapeDataString converts all characters except for RFC 3986 unreserved characters (alphanumeric and "-", ".", "_", "~") to their hex
+        // representation, as per: https://learn.microsoft.com/dotnet/api/system.uri.escapedatastring
+        // "$" is reserved but has currently no meaning in query strings (i.e. it's safe to leave unencoded). Some servers don't recognize encoded
+        // OData parameter keys (such as "%24filter"), hence this function.
+
+        // Notice: even though parameters such as "$filter" and "$expand" will include "$" (and "(", ")", " ", ...) in the parameter value as well, we
+        // assume the server will decode these correctly, and limit this special encoding only for the OData parameters keys.
+
+        EncodedParameterKey := Uri.EscapeDataString(ParameterKey);
+
+        if UseODataEncoding then
+            EncodedParameterKey := EncodedParameterKey.Replace('%24', '$');
+    end;
+
     var
         FlagCannotBeEmptyErr: Label 'The flag cannot be empty.';
         QueryParameterKeyCannotBeEmptyErr: Label 'The query parameter key cannot be empty.';

src/System Application/Test/URI/src/UriBuilderQueryTest.Codeunit.al

@@ -314,18 +314,94 @@ codeunit 135072 "Uri Builder Query Test"
         // [When] Adding a query parameter that needs encoding
         UriBuilder.AddQueryParameter('è', 'éAddedNotEncoded', Enum::"Uri Query Duplicate Behaviour"::"Keep All");
         UriBuilder.AddQueryParameter('%C3%A8', '%C3%A9AddedEncoded', Enum::"Uri Query Duplicate Behaviour"::"Keep All");
-        UriBuilder.AddQueryParameter('moreGarbledStuff', '&/\''"???%20', Enum::"Uri Query Duplicate Behaviour"::"Keep All");
+        UriBuilder.AddQueryParameter('moreGarbledStuff', '&/\''"*!???%20', Enum::"Uri Query Duplicate Behaviour"::"Keep All");
         UriBuilder.AddQueryParameter('고양이', ':30&고양이', Enum::"Uri Query Duplicate Behaviour"::"Throw Error");
 
         // [Then] The resulting URI has encoded query parameters
         UriBuilder.GetUri(Uri);
         Assert.AreEqual('https://microsoft.com/?%C3%A8=%C3%A9NotEncoded&%C3%A8=%C3%A9Encoded' // Initial parameters
             + '&%C3%A8=%C3%A9AddedNotEncoded&%25C3%25A8=%25C3%25A9AddedEncoded' // Added parameters (the one that was already encoded should be double encoded)
+            + '&moreGarbledStuff=%26%2F%5C%27%22%2A%21%3F%3F%3F%2520' // Ensure escape and special characters are encoded
+            + '&%EA%B3%A0%EC%96%91%EC%9D%B4=%3A30%26%EA%B3%A0%EC%96%91%EC%9D%B4', // Ensure character that use 3 bytes are also correctly encoded
+            Uri.GetAbsoluteUri(), 'Unexpected URL.');
+    end;
+
+    [Test]
+    [Scope('OnPrem')]
+    procedure TestExpectedODataEncoding_NonODataParameters()
+    var
+        Uri: Codeunit Uri;
+    begin
+        // [Given] A Url
+        UriBuilder.Init('https://microsoft.com?è=éNotEncoded&%C3%A8=%C3%A9Encoded');
+
+        // [When] Adding OData parameters that don't include the $ sign
+        UriBuilder.AddODataQueryParameter('è', 'éAddedNotEncoded');
+        UriBuilder.AddODataQueryParameter('%C3%A8', '%C3%A9AddedEncoded');
+        UriBuilder.AddODataQueryParameter('moreGarbledStuff', '&/\''"???%20');
+        UriBuilder.AddODataQueryParameter('고양이', ':30&고양이');
+
+        // [Then] The resulting URI has encoded query parameters (same as if using the regular non-OData function)
+        UriBuilder.GetUri(Uri);
+        Assert.AreEqual('https://microsoft.com/' // Initial parameters are overwritten
+            + '?%C3%A8=%C3%A9AddedNotEncoded&%25C3%25A8=%25C3%25A9AddedEncoded' // Added parameters (the one that was already encoded should be double encoded)
             + '&moreGarbledStuff=%26%2F%5C%27%22%3F%3F%3F%2520' // Ensure escape and special characters are encoded
             + '&%EA%B3%A0%EC%96%91%EC%9D%B4=%3A30%26%EA%B3%A0%EC%96%91%EC%9D%B4', // Ensure character that use 3 bytes are also correctly encoded
             Uri.GetAbsoluteUri(), 'Unexpected URL.');
     end;
 
+    [Test]
+    [Scope('OnPrem')]
+    procedure TestExpectedODataEncoding_ODataParameters()
+    var
+        Uri: Codeunit Uri;
+    begin
+        // [Given] A Url
+        UriBuilder.Init('https://microsoft.com?$top=33&%24skip=41&$filter=nothing&è=éNotEncoded&%C3%A8=%C3%A9Encoded');
+
+        // [When] Adding OData parameters that include the $ sign
+        UriBuilder.AddODataQueryParameter('$filter', 'Name eq ''&Contoso''');
+        UriBuilder.AddODataQueryParameter('$expand', 'Products($filter=DiscontinuedDate eq null)');
+        UriBuilder.AddODataQueryParameter('moreGarbledStuff😊', '&/\''"*!???%20');
+
+        // [Then] The resulting URI has encoded query parameters, except the $ sign in the parameter name
+        UriBuilder.GetUri(Uri);
+        Assert.AreEqual('https://microsoft.com/?$top=33&$skip=41' // Initial parameters
+            + '&$filter=Name%20eq%20%27%26Contoso%27' // Filter
+            + '&%C3%A8=%C3%A9NotEncoded&%C3%A8=%C3%A9Encoded' // Other initial parameters
+            + '&$expand=Products%28%24filter%3DDiscontinuedDate%20eq%20null%29' // $ is encoded in the value
+            + '&moreGarbledStuff%F0%9F%98%8A=%26%2F%5C%27%22%2A%21%3F%3F%3F%2520', // Non-OData parameter
+            Uri.GetAbsoluteUri(), 'Unexpected URL.');
+    end;
+
+    [Test]
+    [Scope('OnPrem')]
+    procedure TestExpectedODataEncoding_ODataParametersThenFlag()
+    var
+        Uri: Codeunit Uri;
+    begin
+        // [Given] A Url
+        UriBuilder.Init('https://microsoft.com?$top=33&%24skip=41&$filter=nothing&è=éNotEncoded&%C3%A8=%C3%A9Encoded');
+
+        // [When] Adding OData parameters that include the $ sign
+        UriBuilder.AddODataQueryParameter('$filter', 'Name eq ''&Contoso''');
+        UriBuilder.AddODataQueryParameter('$expand', 'Products($filter=DiscontinuedDate eq null)');
+        UriBuilder.AddODataQueryParameter('moreGarbledStuff😊', '&/\''"*!???%20');
+
+        // [When] Adding a flag afterwards that does not require OData encoding
+        UriBuilder.AddQueryFlag('$newschemaversion');
+
+        // [Then] The library honours the encoding of the last parameter or flag added, and hence the resulting URI has encoded query parameters, including the $ sign in the parameter name.
+        UriBuilder.GetUri(Uri);
+        Assert.AreEqual('https://microsoft.com/?%24top=33&%24skip=41' // Initial parameters
+            + '&%24filter=Name%20eq%20%27%26Contoso%27' // Filter
+            + '&%C3%A8=%C3%A9NotEncoded&%C3%A8=%C3%A9Encoded' // Other initial parameters
+            + '&%24expand=Products%28%24filter%3DDiscontinuedDate%20eq%20null%29' // $ is encoded in the value
+            + '&moreGarbledStuff%F0%9F%98%8A=%26%2F%5C%27%22%2A%21%3F%3F%3F%2520' // Non-OData parameter
+            + '&%24newschemaversion', // Flag
+            Uri.GetAbsoluteUri(), 'Unexpected URL.');
+    end;
+
     [Test]
     [Scope('OnPrem')]
     procedure TestExpectedEncoding_Flags()