[Cherrypick] Fix query string contents in nextLink for REST (remove dupe $after query parameter)

src/Core/Resolvers/SqlPaginationUtil.cs

@@ -520,22 +520,40 @@ public static string Base64Decode(string base64EncodedData)
         /// <summary>
         /// Create the URL that will provide for the next page of results
         /// using the same query options.
+        /// Return value formatted as a JSON array: [{"nextLink":"[base]/api/[entity]?[queryParams_URIescaped]$after=[base64encodedPaginationToken]"}]
         /// </summary>
-        /// <param name="path">The request path.</param>
-        /// <param name="queryStringParameters">Collection of query string parameters.</param>
-        /// <param name="after">The values needed for next page.</param>
-        /// <returns>The string representing nextLink.</returns>
-        public static JsonElement CreateNextLink(string path, NameValueCollection? queryStringParameters, string after)
+        /// <param name="path">The request path excluding query parameters (e.g. https://localhost/api/myEntity)</param>
+        /// <param name="queryStringParameters">Collection of query string parameters that are URI escaped.</param>
+        /// <param name="newAfterPayload">The contents to add to the $after query parameter. Should be base64 encoded pagination token.</param>
+        /// <returns>JSON element - array with nextLink.</returns>
+        public static JsonElement CreateNextLink(string path, NameValueCollection? queryStringParameters, string newAfterPayload)
         {
+            if (queryStringParameters is null)
+            {
+                queryStringParameters = new();
+            }
+            else
+            {
+                // Purge old $after value so this function can replace it.
+                queryStringParameters.Remove("$after");
+            }
+
+            // To prevent regression of current behavior, retain the call to FormatQueryString
+            // which URI escapes other query parameters. Since $after has been removed,
+            // this will not affect the base64 encoded paging token.
             string queryString = FormatQueryString(queryStringParameters: queryStringParameters);
-            if (!string.IsNullOrWhiteSpace(after))
+
+            // When a new $after payload is provided, append it to the query string with the
+            // appropriate prefix: ? if $after is the only query parameter. & if $after is one of many query parameters.
+            if (!string.IsNullOrWhiteSpace(newAfterPayload))
             {
                 string afterPrefix = string.IsNullOrWhiteSpace(queryString) ? "?" : "&";
-                queryString += $"{afterPrefix}{RequestParser.AFTER_URL}={after}";
+                queryString += $"{afterPrefix}{RequestParser.AFTER_URL}={newAfterPayload}";
             }
 
             // ValueKind will be array so we can differentiate from other objects in the response
             // to be returned.
+            // [{"nextLink":"[base]/api/[entity]?[queryParams_URIescaped]$after=[base64encodedPaginationToken]"}]
             string jsonString = JsonSerializer.Serialize(new[]
             {
                 new
@@ -580,6 +598,7 @@ public static string FormatQueryString(NameValueCollection? queryStringParameter
 
             foreach (string key in queryStringParameters)
             {
+                // Whitespace or empty string query paramters are not supported.
                 if (string.IsNullOrWhiteSpace(key))
                 {
                     continue;

src/Service.Tests/Configuration/ConfigurationTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Specialized;
 using System.IdentityModel.Tokens.Jwt;
 using System.IO;
 using System.IO.Abstractions;
@@ -16,6 +17,7 @@
 using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Web;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Core.AuthenticationHelpers;
@@ -2717,6 +2719,95 @@ public async Task OpenApi_EntityLevelRestEndpoint()
             Assert.IsFalse(componentSchemasElement.TryGetProperty("Publisher", out _));
         }
 
+        /// <summary>
+        /// This test validates that DAB properly creates and returns a nextLink with a single $after
+        /// query parameter when sending paging requests.
+        /// The first request initiates a paging workload, meaning the response is expected to have a nextLink.
+        /// The validation occurs after the second request which uses the previously acquired nextLink
+        /// This test ensures that the second request's response body contains the expected nextLink which:
+        /// - is base64 encoded and NOT URI escaped e.g. the trailing "==" are not URI escaped to "%3D%3D"
+        /// - is not the same as the first response's nextLink -> DAB is properly injecting a new $after query param
+        /// and updating the new nextLink
+        /// - does not contain a comma (,) indicating that the URI namevaluecollection tracking the query parameters
+        /// did not come across two $after query parameters. This addresses a customer raised issue where two $after
+        /// query parameters were returned by DAB.
+        /// </summary>
+        [TestMethod]
+        [TestCategory(TestCategory.MSSQL)]
+        public async Task ValidateNextLinkUsage()
+        {
+            // Arrange - Setup test server with entity that has >1 record so that results can be paged.
+            // A short cut to using an entity with >100 records is to just include the $first=1 filter
+            // as done in this test, so that paging behavior can be invoked.
+
+            const string ENTITY_NAME = "Bookmark";
+
+            // At least one entity is required in the runtime config for the engine to start.
+            // Even though this entity is not under test, it must be supplied to the config
+            // file creation function.
+            Entity requiredEntity = new(
+                Source: new("bookmarks", EntitySourceType.Table, null, null),
+                Rest: new(Enabled: true),
+                GraphQL: new(Singular: "", Plural: "", Enabled: false),
+                Permissions: new[] { GetMinimalPermissionConfig(AuthorizationResolver.ROLE_ANONYMOUS) },
+                Relationships: null,
+                Mappings: null);
+
+            Dictionary<string, Entity> entityMap = new()
+            {
+                { ENTITY_NAME, requiredEntity }
+            };
+
+            CreateCustomConfigFile(globalRestEnabled: true, entityMap);
+
+            string[] args = new[]
+            {
+                $"--ConfigFileName={CUSTOM_CONFIG_FILENAME}"
+            };
+
+            using TestServer server = new(Program.CreateWebHostBuilder(args));
+            using HttpClient client = server.CreateClient();
+
+            // Setup and send GET request
+            HttpRequestMessage initialPaginationRequest = new(HttpMethod.Get, $"{RestRuntimeOptions.DEFAULT_PATH}/{ENTITY_NAME}?$first=1");
+            HttpResponseMessage initialPaginationResponse = await client.SendAsync(initialPaginationRequest);
+
+            // Process response body for first request and get the nextLink to use on subsequent request
+            // which represents what this test is validating.
+            string responseBody = await initialPaginationResponse.Content.ReadAsStringAsync();
+            Dictionary<string, JsonElement> responseProperties = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(responseBody);
+            string nextLinkUri = responseProperties["nextLink"].ToString();
+
+            // Act - Submit request with nextLink uri as target and capture response
+
+            HttpRequestMessage followNextLinkRequest = new(HttpMethod.Get, nextLinkUri);
+            HttpResponseMessage followNextLinkResponse = await client.SendAsync(followNextLinkRequest);
+
+            // Assert
+
+            Assert.AreEqual(HttpStatusCode.OK, followNextLinkResponse.StatusCode, message: "Expected request to succeed.");
+
+            // Process the response body and inspect the "nextLink" property for expected contents.
+            string followNextLinkResponseBody = await followNextLinkResponse.Content.ReadAsStringAsync();
+            Dictionary<string, JsonElement> followNextLinkResponseProperties = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(followNextLinkResponseBody);
+
+            string followUpResponseNextLink = followNextLinkResponseProperties["nextLink"].ToString();
+            Uri nextLink = new(uriString: followUpResponseNextLink);
+            NameValueCollection parsedQueryParameters = HttpUtility.ParseQueryString(query: nextLink.Query);
+            Assert.AreEqual(expected: false, actual: parsedQueryParameters["$after"].Contains(','), message: "nextLink erroneously contained two $after query parameters that were joined by HttpUtility.ParseQueryString(queryString).");
+            Assert.AreNotEqual(notExpected: nextLinkUri, actual: followUpResponseNextLink, message: "The follow up request erroneously returned the same nextLink value.");
+
+            // Do not use SqlPaginationUtils.Base64Encode()/Decode() here to eliminate test dependency on engine code to perform an assert.
+            try
+            {
+                Convert.FromBase64String(parsedQueryParameters["$after"]);
+            }
+            catch (FormatException)
+            {
+                Assert.Fail(message: "$after query parameter was not a valid base64 encoded value.");
+            }
+        }
+
         /// <summary>
         /// Helper function to write custom configuration file. with minimal REST/GraphQL global settings
         /// using the supplied entities.