From 1aa7cd8ae0ea67a20786d05a0c03838b7bf115cf Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 11:33:04 -0700 Subject: [PATCH 01/27] Adding pagination limits. --- src/Config/ObjectModel/Entity.cs | 5 +- src/Config/ObjectModel/PaginationOptions.cs | 90 +++++++++++++++++++ src/Config/ObjectModel/RuntimeOptions.cs | 5 +- src/Core/Resolvers/BaseQueryStructure.cs | 41 +++++++++ src/Core/Resolvers/CosmosQueryEngine.cs | 4 +- src/Core/Resolvers/CosmosQueryStructure.cs | 9 +- .../Sql Query Structures/SqlQueryStructure.cs | 23 +---- .../Queries/QueryBuilder.cs | 2 + 8 files changed, 154 insertions(+), 25 deletions(-) create mode 100644 src/Config/ObjectModel/PaginationOptions.cs diff --git a/src/Config/ObjectModel/Entity.cs b/src/Config/ObjectModel/Entity.cs index ec92a1f5a4..1aedaa4f57 100644 --- a/src/Config/ObjectModel/Entity.cs +++ b/src/Config/ObjectModel/Entity.cs @@ -31,6 +31,7 @@ public record Entity public Dictionary? Mappings { get; init; } public Dictionary? Relationships { get; init; } public EntityCacheOptions? Cache { get; init; } + public int? PageLimit { get; init; } [JsonIgnore] public bool IsLinkingEntity { get; init; } @@ -44,7 +45,8 @@ public Entity( Dictionary? Mappings, Dictionary? Relationships, EntityCacheOptions? Cache = null, - bool IsLinkingEntity = false) + bool IsLinkingEntity = false, + int? PageLimit = null) { this.Source = Source; this.GraphQL = GraphQL; @@ -54,6 +56,7 @@ public Entity( this.Relationships = Relationships; this.Cache = Cache; this.IsLinkingEntity = IsLinkingEntity; + this.PageLimit = PageLimit; } /// diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs new file mode 100644 index 0000000000..125d34fc40 --- /dev/null +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -0,0 +1,90 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization; + +namespace Azure.DataApiBuilder.Config.ObjectModel; + +/// +/// Pagination options for the dab setup. +/// Properties are nullable to support DAB CLI merge config +/// expected behavior. +/// +public record PaginationOptions +{ + /// + /// Default page size. + /// + public const int DEFAULT_PAGE_SIZE = 100; + + /// + /// Max page size. + /// + public const int MAX_PAGE_SIZE = 100000; + + /// + /// The default page size for pagination. + /// + [JsonPropertyName("default-page-size")] + public int? DefaultPageSize { get; init; } = null; + + /// + /// The max page size for pagination. + /// + [JsonPropertyName("max-page-size")] + public int? MaxPageSize { get; init; } = null; + + [JsonConstructor] + public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) + { + if (DefaultPageSize is not null) + { + this.DefaultPageSize = DefaultPageSize; + UserProvidedDefaultPageSize = true; + } + else + { + this.DefaultPageSize = DEFAULT_PAGE_SIZE; + } + + if (MaxPageSize is not null) + { + this.MaxPageSize = MaxPageSize; + UserProvidedMaxPageSize = true; + } + else + { + this.MaxPageSize = MaxPageSize; + } + } + + /// + /// Flag which informs CLI and JSON serializer whether to write ttl-seconds + /// property and value to the runtime config file. + /// When user doesn't provide the default-page-size property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// This is because the user's intent is to use DAB's default value which could change + /// and DAB CLI writing the property and value would lose the user's intent. + /// This is because if the user were to use the CLI created config, a default-page-size + /// property/value specified would be interpreted by DAB as "user explicitly set ttl." + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(DefaultPageSize))] + public bool UserProvidedDefaultPageSize { get; init; } = false; + + /// + /// Flag which informs CLI and JSON serializer whether to write max-page-size + /// property and value to the runtime config file. + /// When user doesn't provide the max-page-size property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// This is because the user's intent is to use DAB's default value which could change + /// and DAB CLI writing the property and value would lose the user's intent. + /// This is because if the user were to use the CLI created config, a max-page-size + /// property/value specified would be interpreted by DAB as "user explicitly set ttl." + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(MaxPageSize))] + public bool UserProvidedMaxPageSize { get; init; } = false; + +} diff --git a/src/Config/ObjectModel/RuntimeOptions.cs b/src/Config/ObjectModel/RuntimeOptions.cs index 89af714461..e31d83cc5d 100644 --- a/src/Config/ObjectModel/RuntimeOptions.cs +++ b/src/Config/ObjectModel/RuntimeOptions.cs @@ -14,6 +14,7 @@ public record RuntimeOptions public string? BaseRoute { get; init; } public TelemetryOptions? Telemetry { get; init; } public EntityCacheOptions? Cache { get; init; } + public PaginationOptions? Pagination { get; init; } [JsonConstructor] public RuntimeOptions( @@ -22,7 +23,8 @@ public RuntimeOptions( HostOptions? Host, string? BaseRoute = null, TelemetryOptions? Telemetry = null, - EntityCacheOptions? Cache = null) + EntityCacheOptions? Cache = null, + PaginationOptions? Pagination = null) { this.Rest = Rest; this.GraphQL = GraphQL; @@ -30,6 +32,7 @@ public RuntimeOptions( this.BaseRoute = BaseRoute; this.Telemetry = Telemetry; this.Cache = Cache; + this.Pagination = Pagination; } /// diff --git a/src/Core/Resolvers/BaseQueryStructure.cs b/src/Core/Resolvers/BaseQueryStructure.cs index e8cb912770..ad2830dfff 100644 --- a/src/Core/Resolvers/BaseQueryStructure.cs +++ b/src/Core/Resolvers/BaseQueryStructure.cs @@ -1,10 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Net; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Services; +using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.GraphQLBuilder; using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; using HotChocolate.Language; @@ -181,5 +185,42 @@ internal static IObjectField ExtractItemsSchemaField(IObjectField connectionSche { return GraphQLUtils.UnderlyingGraphQLEntityType(connectionSchemaField.Type).Fields[QueryBuilder.PAGINATION_FIELD_NAME]; } + + internal static int GetPaginationLimit(RuntimeConfigProvider runtimeConfigProvider, int? userInputValue) + { + runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + RuntimeOptions? runtimeOptions = runtimeConfig?.Runtime; + int defaultPageSize; + int maxPageSize; + if (runtimeOptions is not null && runtimeOptions.Pagination is not null) + { + defaultPageSize = (int)runtimeOptions.Pagination.DefaultPageSize!; + maxPageSize = (int)runtimeOptions.Pagination.MaxPageSize!; + } + else + { + defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE; + maxPageSize = PaginationOptions.MAX_PAGE_SIZE; + } + + if (userInputValue is not null) + { + if (userInputValue < -1 || userInputValue == 0 || userInputValue > maxPageSize) + { + throw new DataApiBuilderException( + message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {userInputValue}", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); + } + else + { + return (int)(userInputValue == -1 ? maxPageSize : userInputValue); + } + } + else + { + return defaultPageSize; + } + } } } diff --git a/src/Core/Resolvers/CosmosQueryEngine.cs b/src/Core/Resolvers/CosmosQueryEngine.cs index 9412d63ee4..243394ae28 100644 --- a/src/Core/Resolvers/CosmosQueryEngine.cs +++ b/src/Core/Resolvers/CosmosQueryEngine.cs @@ -69,7 +69,7 @@ public async Task> ExecuteAsync( ISqlMetadataProvider metadataStoreProvider = _metadataProviderFactory.GetMetadataProvider(dataSourceName); - CosmosQueryStructure structure = new(context, parameters, metadataStoreProvider, _authorizationResolver, _gQLFilterParser); + CosmosQueryStructure structure = new(context, parameters, _runtimeConfigProvider, metadataStoreProvider, _authorizationResolver, _gQLFilterParser); RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); string queryString = _queryBuilder.Build(structure); @@ -201,7 +201,7 @@ public async Task, IMetadata>> ExecuteListAsync( // TODO: add support for TOP and Order-by push-down ISqlMetadataProvider metadataStoreProvider = _metadataProviderFactory.GetMetadataProvider(dataSourceName); - CosmosQueryStructure structure = new(context, parameters, metadataStoreProvider, _authorizationResolver, _gQLFilterParser); + CosmosQueryStructure structure = new(context, parameters, _runtimeConfigProvider, metadataStoreProvider, _authorizationResolver, _gQLFilterParser); CosmosClient client = _clientProvider.Clients[dataSourceName]; Container container = client.GetDatabase(structure.Database).GetContainer(structure.Container); QueryDefinition querySpec = new(_queryBuilder.Build(structure)); diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index 881d2b50eb..becb95e7ef 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Services; using Azure.DataApiBuilder.Service.GraphQLBuilder; @@ -29,6 +30,8 @@ public class CosmosQueryStructure : BaseQueryStructure public int? MaxItemCount { get; internal set; } public string? PartitionKeyValue { get; internal set; } public List OrderByColumns { get; internal set; } + + public RuntimeConfigProvider RuntimeConfigProvider { get; internal set; } // Order of the join matters public Stack? Joins { get; internal set; } @@ -43,6 +46,7 @@ public record CosmosJoinStructure(DatabaseObject DbObject, string TableAlias); public CosmosQueryStructure( IMiddlewareContext context, IDictionary parameters, + RuntimeConfigProvider provider, ISqlMetadataProvider metadataProvider, IAuthorizationResolver authorizationResolver, GQLFilterParser gQLFilterParser, @@ -52,6 +56,7 @@ public CosmosQueryStructure( _context = context; SourceAlias = _containerAlias; DatabaseObject.Name = _containerAlias; + RuntimeConfigProvider = provider; Init(parameters); } @@ -110,7 +115,6 @@ private void Init(IDictionary queryParams) IsPaginated = QueryBuilder.IsPaginationType(underlyingType); OrderByColumns = new(); - if (IsPaginated) { FieldNode? fieldNode = ExtractItemsQueryField(selection.SyntaxNode); @@ -142,7 +146,8 @@ private void Init(IDictionary queryParams) // TODO: Revisit 'first' while adding support for TOP queries if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { - MaxItemCount = (int?)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]; + MaxItemCount = GetPaginationLimit(RuntimeConfigProvider, (int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); + queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index d62595b581..a3593aaa52 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -17,7 +17,6 @@ using HotChocolate.Language; using HotChocolate.Resolvers; using Microsoft.AspNetCore.Http; - namespace Azure.DataApiBuilder.Core.Resolvers { /// @@ -59,15 +58,10 @@ public class SqlQueryStructure : BaseSqlQueryStructure /// public Dictionary ColumnLabelToParam { get; } - /// - /// Default limit when no first param is specified for list queries - /// - private const uint DEFAULT_LIST_LIMIT = 100; - /// /// The maximum number of results this query should return. /// - private uint? _limit = DEFAULT_LIST_LIMIT; + private uint? _limit = PaginationOptions.DEFAULT_PAGE_SIZE; /// /// If this query is built because of a GraphQL query (as opposed to @@ -198,7 +192,8 @@ public SqlQueryStructure( } AddColumnsForEndCursor(); - _limit = context.First is not null ? context.First + 1 : DEFAULT_LIST_LIMIT + 1; + _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int?)context.First); + ParametrizeColumns(); } @@ -341,17 +336,7 @@ private SqlQueryStructure( if (firstObject != null) { - int first = (int)firstObject; - - if (first <= 0) - { - throw new DataApiBuilderException( - message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be an integer greater than 0 for {schemaField.Name}. Actual value: {first.ToString()}", - statusCode: HttpStatusCode.BadRequest, - subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); - } - - _limit = (uint)first; + _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int)firstObject); } } diff --git a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs index b68eb642af..c4b0a0d34a 100644 --- a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs +++ b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs @@ -24,6 +24,8 @@ public static class QueryBuilder public const string ORDER_BY_FIELD_NAME = "orderBy"; public const string PARTITION_KEY_FIELD_NAME = "_partitionKeyValue"; public const string ID_FIELD_NAME = "id"; + public const int MAX_PAGE_LIMIT = 1000; + public const int DEFAULT_PAGE_LIMIT = 100; /// /// Creates a DocumentNode containing FieldDefinitionNodes representing the FindByPK and FindAll queries From 78587e94d0cfeb587faded71907be0831cff4dec Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 11:52:22 -0700 Subject: [PATCH 02/27] Reverting changes to entity.cs --- src/Config/ObjectModel/Entity.cs | 5 +---- src/Config/ObjectModel/PaginationOptions.cs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Config/ObjectModel/Entity.cs b/src/Config/ObjectModel/Entity.cs index 1aedaa4f57..ec92a1f5a4 100644 --- a/src/Config/ObjectModel/Entity.cs +++ b/src/Config/ObjectModel/Entity.cs @@ -31,7 +31,6 @@ public record Entity public Dictionary? Mappings { get; init; } public Dictionary? Relationships { get; init; } public EntityCacheOptions? Cache { get; init; } - public int? PageLimit { get; init; } [JsonIgnore] public bool IsLinkingEntity { get; init; } @@ -45,8 +44,7 @@ public Entity( Dictionary? Mappings, Dictionary? Relationships, EntityCacheOptions? Cache = null, - bool IsLinkingEntity = false, - int? PageLimit = null) + bool IsLinkingEntity = false) { this.Source = Source; this.GraphQL = GraphQL; @@ -56,7 +54,6 @@ public Entity( this.Relationships = Relationships; this.Cache = Cache; this.IsLinkingEntity = IsLinkingEntity; - this.PageLimit = PageLimit; } /// diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index 125d34fc40..6741fb7a74 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -50,7 +50,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) if (MaxPageSize is not null) { - this.MaxPageSize = MaxPageSize; + this.MaxPageSize = MaxPageSize == -1 ? Int32.MaxValue : MaxPageSize; UserProvidedMaxPageSize = true; } else From d2dda1ef91688ac1022303320fc13582ce95cbbf Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 12:03:31 -0700 Subject: [PATCH 03/27] Dotnet format. --- src/Core/Resolvers/CosmosQueryStructure.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index becb95e7ef..54fe11f834 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -147,7 +147,7 @@ private void Init(IDictionary queryParams) if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { MaxItemCount = GetPaginationLimit(RuntimeConfigProvider, (int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); - + queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } From ab731e4df42ea6f936893e222fe94ffa1b5d354a Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 12:30:06 -0700 Subject: [PATCH 04/27] Adding parser. --- src/Core/Models/GraphQLFilterParsers.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Models/GraphQLFilterParsers.cs b/src/Core/Models/GraphQLFilterParsers.cs index 1e07c20481..e783cf4f4d 100644 --- a/src/Core/Models/GraphQLFilterParsers.cs +++ b/src/Core/Models/GraphQLFilterParsers.cs @@ -334,6 +334,7 @@ private void HandleNestedFilterForCosmos( new( context: ctx, parameters: subParameters, + provider: _configProvider, metadataProvider: metadataProvider, authorizationResolver: queryStructure.AuthorizationResolver, gQLFilterParser: this, From ffa4501a50c267a1e7d4b005b0e649c230617f1d Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 14:05:11 -0700 Subject: [PATCH 05/27] Adding test cases. --- .../GraphQLPaginationTestBase.cs | 107 +++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs index 327b84abc6..5eb50c6867 100644 --- a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs @@ -64,6 +64,92 @@ public async Task RequestFullConnection() SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); } + /// + /// Request a full connection object {items, after, hasNextPage} + /// without providing any parameters + /// + [TestMethod] + public async Task RequestNoParamFullConnection() + { + string graphQLQueryName = "books"; + string graphQLQuery = @"{ + books (first: -1) { + items { + id + title + } + endCursor + hasNextPage + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + string expected = @"{ + ""items"": [ + { + ""id"": 1, + ""title"": ""Awesome book"" + }, + { + ""id"": 2, + ""title"": ""Also Awesome book"" + }, + { + ""id"": 3, + ""title"": ""Great wall of china explained"" + }, + { + ""id"": 4, + ""title"": ""US history in a nutshell"" + }, + { + ""id"": 5, + ""title"": ""Chernobyl Diaries"" + }, + { + ""id"": 6, + ""title"": ""The Palace Door"" + }, + { + ""id"": 7, + ""title"": ""The Groovy Bar"" + }, + { + ""id"": 8, + ""title"": ""Time to Eat"" + }, + { + ""id"": 9, + ""title"": ""Policy-Test-01"" + }, + { + ""id"": 10, + ""title"": ""Policy-Test-02"" + }, + { + ""id"": 11, + ""title"": ""Policy-Test-04"" + }, + { + ""id"": 12, + ""title"": ""Time to Eat 2"" + }, + { + ""id"": 13, + ""title"": ""Before Sunrise"" + }, + { + ""id"": 14, + ""title"": ""Before Sunset"" + } + ], + ""endCursor"": null, + ""hasNextPage"": false + }"; + + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + /// /// Request a full connection object {items, after, hasNextPage} /// without providing any parameters @@ -856,7 +942,7 @@ public async Task RequestInvalidFirst() { string graphQLQueryName = "books"; string graphQLQuery = @"{ - books(first: -1) { + books(first: -2) { items { id } @@ -886,6 +972,25 @@ public async Task RequestInvalidZeroFirst() SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), statusCode: $"{DataApiBuilderException.SubStatusCodes.BadRequest}"); } + /// + /// Request an invalid number of entries for a pagination page + /// + [TestMethod] + public async Task RequestInvalidMaxSize() + { + string graphQLQueryName = "books"; + string graphQLQuery = @"{ + books(first: 100001) { + items { + id + } + } + }"; + + JsonElement result = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.TestForErrorInGraphQLResponse(result.ToString(), statusCode: $"{DataApiBuilderException.SubStatusCodes.BadRequest}"); + } + /// /// Supply a non JSON after parameter /// From 15e99dc8a0feb1301ed637bac1086193b5d9ae47 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 14:11:22 -0700 Subject: [PATCH 06/27] Fix test case. --- .../GraphQLPaginationTests/GraphQLPaginationTestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs index 5eb50c6867..21ecf6d19d 100644 --- a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs @@ -69,7 +69,7 @@ public async Task RequestFullConnection() /// without providing any parameters /// [TestMethod] - public async Task RequestNoParamFullConnection() + public async Task RequestMaxUsingNegativeOne() { string graphQLQueryName = "books"; string graphQLQuery = @"{ From e6f19aba7082d2d4ef777ce4a97f7b94afc34347 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 14:20:12 -0700 Subject: [PATCH 07/27] Rest api tests. --- .../RestApiTests/Find/FindApiTestBase.cs | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index 76f82aae2d..f38c182d81 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -744,9 +744,7 @@ await SetupAndRunRestApiTest( /// /// Validates that a proper nextLink is created for FindMany requests which do not - /// restrict results with query parameters. Engine default paging mechanisms are used - /// when > 100 records will be present in result set. - /// expectedAfterQueryString starts with ?$, and not &$, because it is the only query parameter. + /// restrict results with query parameters. /// [TestMethod] public async Task FindTest_NoQueryParams_PaginationNextLink() @@ -754,7 +752,7 @@ public async Task FindTest_NoQueryParams_PaginationNextLink() string after = SqlPaginationUtil.Base64Encode($"[{{\"EntityName\":\"Bookmarks\",\"FieldName\":\"id\",\"FieldValue\":100,\"Direction\":0}}]"); await SetupAndRunRestApiTest( primaryKeyRoute: string.Empty, - queryString: string.Empty, + queryString: "?$first=1", entityNameOrPath: _integrationPaginationEntityName, sqlQuery: GetQuery(nameof(FindTest_NoQueryParams_PaginationNextLink)), expectedAfterQueryString: $"?$after={after}", @@ -762,6 +760,24 @@ await SetupAndRunRestApiTest( ); } + /// + /// Validates that a proper nextLink is created for FindMany requests which do not + /// restrict results with query parameters. Engine default paging mechanisms are used + /// when > 100 records will be present in result set. + /// expectedAfterQueryString starts with ?$, and not &$, because it is the only query parameter. + /// + [TestMethod] + public async Task FindTest_Negative1QueryParams_PaginationNextLink() + { + string after = SqlPaginationUtil.Base64Encode($"[{{\"EntityName\":\"Bookmarks\",\"FieldName\":\"id\",\"FieldValue\":100,\"Direction\":0}}]"); + await SetupAndRunRestApiTest( + primaryKeyRoute: string.Empty, + queryString: "?$first=-1", + entityNameOrPath: _integrationPaginationEntityName, + sqlQuery: GetQuery(nameof(FindTest_NoQueryParams_PaginationNextLink)) + ); + } + /// /// Validates that a proper nextLink is created for FindMany requests which do not /// restrict results with query parameters. Engine default paging mechanisms are used @@ -1558,6 +1574,25 @@ await SetupAndRunRestApiTest( ); } + /// + /// Tests the REST Api for Find operation using $first=100001 + /// to request > max records of 100000 records, which should throw a DataApiBuilder + /// Exception. + /// + [TestMethod] + public async Task FindTestWithMaxExceededSingleKeyPagination() + { + await SetupAndRunRestApiTest( + primaryKeyRoute: string.Empty, + queryString: "?$first=0", + entityNameOrPath: _integrationEntityName, + sqlQuery: string.Empty, + exceptionExpected: true, + expectedErrorMessage: "Invalid number of items requested, first argument must be either -1 or a positive number within the max page size limit of 100000. Actual value: 100001", + expectedStatusCode: HttpStatusCode.BadRequest + ); + } + /// /// Tests the REST Api for Find operations using keywords /// of which is not currently supported, verify From 5b8fae85ef054ab42dc1692670bda978273dc039 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 14:34:01 -0700 Subject: [PATCH 08/27] fixing test case. --- .../SqlTests/RestApiTests/Find/FindApiTestBase.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index f38c182d81..9a4e2b7d4b 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -744,7 +744,9 @@ await SetupAndRunRestApiTest( /// /// Validates that a proper nextLink is created for FindMany requests which do not - /// restrict results with query parameters. + /// restrict results with query parameters. Engine default paging mechanisms are used + /// when > 100 records will be present in result set. + /// expectedAfterQueryString starts with ?$, and not &$, because it is the only query parameter. /// [TestMethod] public async Task FindTest_NoQueryParams_PaginationNextLink() @@ -752,7 +754,7 @@ public async Task FindTest_NoQueryParams_PaginationNextLink() string after = SqlPaginationUtil.Base64Encode($"[{{\"EntityName\":\"Bookmarks\",\"FieldName\":\"id\",\"FieldValue\":100,\"Direction\":0}}]"); await SetupAndRunRestApiTest( primaryKeyRoute: string.Empty, - queryString: "?$first=1", + queryString: string.Empty, entityNameOrPath: _integrationPaginationEntityName, sqlQuery: GetQuery(nameof(FindTest_NoQueryParams_PaginationNextLink)), expectedAfterQueryString: $"?$after={after}", @@ -1584,7 +1586,7 @@ public async Task FindTestWithMaxExceededSingleKeyPagination() { await SetupAndRunRestApiTest( primaryKeyRoute: string.Empty, - queryString: "?$first=0", + queryString: "?$first=100001", entityNameOrPath: _integrationEntityName, sqlQuery: string.Empty, exceptionExpected: true, From 28b610b25afbe054db666ca703af1c2cb15ace40 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 4 Apr 2024 15:34:04 -0700 Subject: [PATCH 09/27] RuntimeConfigValidator. --- .../Configurations/RuntimeConfigValidator.cs | 23 +++++++++++++++++++ .../Sql Query Structures/SqlQueryStructure.cs | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 7bb3aa2f7e..b1c4d21f33 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -82,6 +82,7 @@ public void ValidateConfigProperties() ValidateAuthenticationOptions(runtimeConfig); ValidateGlobalEndpointRouteConfig(runtimeConfig); ValidateAppInsightsTelemetryConnectionString(runtimeConfig); + ValidatePaginationOptions(runtimeConfig); // Running these graphQL validations only in development mode to ensure // fast startup of engine in production mode. @@ -138,6 +139,28 @@ public void ValidateAppInsightsTelemetryConnectionString(RuntimeConfig runtimeCo } } + /// + /// Validating pagination options in the runtime config. + /// + public void ValidatePaginationOptions(RuntimeConfig runtimeConfig) + { + if (runtimeConfig.Runtime is not null && runtimeConfig.Runtime.Pagination is not null) + { + PaginationOptions paginationOptions = runtimeConfig.Runtime.Pagination; + if (paginationOptions.DefaultPageSize < -1 || + paginationOptions.DefaultPageSize == 0 || + paginationOptions.MaxPageSize < -1 || + paginationOptions.MaxPageSize == 0 || + paginationOptions.DefaultPageSize > paginationOptions.MaxPageSize) + { + HandleOrRecordException(new DataApiBuilderException( + message: "Pagination options invalid. Page size arguments cannot be 0, < -1 and default page size cannot be greater than max page size", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + } + } + } + /// /// This method runs several validations against the config file such as schema validation, /// validation of entities metadata, validation of permissions, validation of entity configuration. diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index a3593aaa52..b970515768 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -192,7 +192,7 @@ public SqlQueryStructure( } AddColumnsForEndCursor(); - _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int?)context.First); + _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int?)context.First) + 1; ParametrizeColumns(); } From bab107e63d0f1489477e9ecf5c9efd8b6b83e405 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 5 Apr 2024 11:55:02 -0700 Subject: [PATCH 10/27] Fixing test cases. --- src/Config/ObjectModel/PaginationOptions.cs | 20 +++++++++---------- .../RestRequestContexts/RestRequestContext.cs | 2 +- src/Core/Resolvers/SqlPaginationUtil.cs | 9 ++++++--- src/Core/Resolvers/SqlResponseHelpers.cs | 5 ++++- src/Core/Services/RequestValidator.cs | 8 ++++---- .../GraphQLQueryTests/GraphQLQueryTestBase.cs | 2 +- .../RestApiTests/Find/DwSqlFindApiTests.cs | 6 ++++++ .../RestApiTests/Find/FindApiTestBase.cs | 5 ++--- .../RestApiTests/Find/MsSqlFindApiTests.cs | 6 ++++++ .../RestApiTests/Find/MySqlFindApiTests.cs | 11 ++++++++++ .../Find/PostgreSqlFindApiTests.cs | 12 +++++++++++ 11 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index 6741fb7a74..32c587bfa5 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -27,35 +27,35 @@ public record PaginationOptions /// The default page size for pagination. /// [JsonPropertyName("default-page-size")] - public int? DefaultPageSize { get; init; } = null; + public uint? DefaultPageSize { get; init; } = null; /// /// The max page size for pagination. /// [JsonPropertyName("max-page-size")] - public int? MaxPageSize { get; init; } = null; + public uint? MaxPageSize { get; init; } = null; [JsonConstructor] public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) { - if (DefaultPageSize is not null) + if (MaxPageSize is not null) { - this.DefaultPageSize = DefaultPageSize; - UserProvidedDefaultPageSize = true; + this.MaxPageSize = MaxPageSize == -1 ? UInt32.MaxValue : (uint)MaxPageSize; + UserProvidedMaxPageSize = true; } else { - this.DefaultPageSize = DEFAULT_PAGE_SIZE; + this.MaxPageSize = MAX_PAGE_SIZE; } - if (MaxPageSize is not null) + if (DefaultPageSize is not null) { - this.MaxPageSize = MaxPageSize == -1 ? Int32.MaxValue : MaxPageSize; - UserProvidedMaxPageSize = true; + this.DefaultPageSize = DefaultPageSize == -1 ? (uint)this.MaxPageSize : (uint)DefaultPageSize; + UserProvidedDefaultPageSize = true; } else { - this.MaxPageSize = MaxPageSize; + this.DefaultPageSize = DEFAULT_PAGE_SIZE; } } diff --git a/src/Core/Models/RestRequestContexts/RestRequestContext.cs b/src/Core/Models/RestRequestContexts/RestRequestContext.cs index 0150c54e0f..70d6a371b5 100644 --- a/src/Core/Models/RestRequestContexts/RestRequestContext.cs +++ b/src/Core/Models/RestRequestContexts/RestRequestContext.cs @@ -88,7 +88,7 @@ protected RestRequestContext(string entityName, DatabaseObject dbo) /// Based on request this property may or may not be populated. /// - public uint? First { get; set; } + public int? First { get; set; } /// /// Is the result supposed to be multiple values or not. /// diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 1125445019..3fdb18fbb9 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -6,6 +6,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Parsers; @@ -577,12 +578,14 @@ public static JsonElement CreateNextLink(string path, NameValueCollection? query /// /// Results plus one extra record if more exist. /// Client provided limit if one exists, otherwise 0. + /// Default limit if none provided. /// Bool representing if more records are available. - public static bool HasNext(JsonElement jsonResult, uint? first) + public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE, uint maxPageSize = PaginationOptions.MAX_PAGE_SIZE ) { - // When first is 0 we use default limit of 100, otherwise we use first + // When first is null we use default limit from runtime config, otherwise we use first uint numRecords = (uint)jsonResult.GetArrayLength(); - uint? limit = first is not null ? first : 100; + first = first is not null ? first : (int)defaultPageSize; + uint? limit = first == -1 ? maxPageSize : (uint)first; return numRecords > limit; } diff --git a/src/Core/Resolvers/SqlResponseHelpers.cs b/src/Core/Resolvers/SqlResponseHelpers.cs index 7c2e3a852c..c4549c6b95 100644 --- a/src/Core/Resolvers/SqlResponseHelpers.cs +++ b/src/Core/Resolvers/SqlResponseHelpers.cs @@ -51,9 +51,12 @@ public static OkObjectResult FormatFindResult( ? DetermineExtraFieldsInResponse(findOperationResponse, context.FieldsToBeReturned) : DetermineExtraFieldsInResponse(findOperationResponse.EnumerateArray().First(), context.FieldsToBeReturned); + uint defaultPageSize = runtimeConfig.Runtime?.Pagination?.DefaultPageSize ?? PaginationOptions.DEFAULT_PAGE_SIZE; + uint maxPageSize = runtimeConfig.Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; + // If the results are not a collection or if the query does not have a next page // no nextLink is needed. So, the response is returned after removing the extra fields. - if (findOperationResponse.ValueKind is not JsonValueKind.Array || !SqlPaginationUtil.HasNext(findOperationResponse, context.First)) + if (findOperationResponse.ValueKind is not JsonValueKind.Array || !SqlPaginationUtil.HasNext(findOperationResponse, context.First, defaultPageSize, maxPageSize)) { // If there are no additional fields present, the response is returned directly. When there // are extra fields, they are removed before returning the response. diff --git a/src/Core/Services/RequestValidator.cs b/src/Core/Services/RequestValidator.cs index fff83c922f..25b7bad14e 100644 --- a/src/Core/Services/RequestValidator.cs +++ b/src/Core/Services/RequestValidator.cs @@ -546,17 +546,17 @@ private static StoredProcedureDefinition TryGetStoredProcedureDefinition(string /// /// String representing value associated with $first /// uint > 0 representing $first - public static uint CheckFirstValidity(string first) + public static int CheckFirstValidity(string first) { - if (!uint.TryParse(first, out uint firstAsUint) || firstAsUint == 0) + if (!int.TryParse(first, out int firstAsInt) || firstAsInt == 0) { throw new DataApiBuilderException( - message: $"Invalid number of items requested, $first must be an integer greater than 0. Actual value: {first}", + message: $"Invalid number of items requested, $first must be -1 or an integer greater than 0. Actual value: {first}", statusCode: HttpStatusCode.BadRequest, subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); } - return firstAsUint; + return firstAsInt; } /// diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs index 10b02158af..d176915a3d 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs @@ -1573,7 +1573,7 @@ public virtual async Task TestInvalidFirstParamQuery() { string graphQLQueryName = "books"; string graphQLQuery = @"{ - books(first: -1) { + books(first: -2) { items { id title diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs index 7b5a620a2d..e1a17bc4bc 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs @@ -238,6 +238,12 @@ public class DwSqlFindApiTests : FindApiTestBase $"ORDER BY id asc " + $"FOR JSON PATH, INCLUDE_NULL_VALUES" }, + { + "FindTest_Negative1QueryParams_Pagination", + $"SELECT TOP 100000 * FROM { _integrationPaginationTableName } " + + $"ORDER BY id asc " + + $"FOR JSON PATH, INCLUDE_NULL_VALUES" + }, { "FindTest_OrderByNotFirstQueryParam_PaginationNextLink", $"SELECT TOP 100 id FROM { _integrationPaginationTableName } " + diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index 9a4e2b7d4b..d30da02f27 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -769,14 +769,13 @@ await SetupAndRunRestApiTest( /// expectedAfterQueryString starts with ?$, and not &$, because it is the only query parameter. /// [TestMethod] - public async Task FindTest_Negative1QueryParams_PaginationNextLink() + public async Task FindTest_Negative1QueryParams_Pagination() { - string after = SqlPaginationUtil.Base64Encode($"[{{\"EntityName\":\"Bookmarks\",\"FieldName\":\"id\",\"FieldValue\":100,\"Direction\":0}}]"); await SetupAndRunRestApiTest( primaryKeyRoute: string.Empty, queryString: "?$first=-1", entityNameOrPath: _integrationPaginationEntityName, - sqlQuery: GetQuery(nameof(FindTest_NoQueryParams_PaginationNextLink)) + sqlQuery: GetQuery(nameof(FindTest_Negative1QueryParams_Pagination)) ); } diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs index 8f543486f9..cee5305a5a 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs @@ -245,6 +245,12 @@ public class MsSqlFindApiTests : FindApiTestBase $"ORDER BY id asc " + $"FOR JSON PATH, INCLUDE_NULL_VALUES" }, + { + "FindTest_Negative1QueryParams_Pagination", + $"SELECT TOP 100000 * FROM { _integrationPaginationTableName } " + + $"ORDER BY id asc " + + $"FOR JSON PATH, INCLUDE_NULL_VALUES" + }, { "FindTest_OrderByNotFirstQueryParam_PaginationNextLink", $"SELECT TOP 100 id FROM { _integrationPaginationTableName } " + diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs index 76c5af128f..db385edbcc 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs @@ -456,6 +456,17 @@ ORDER BY id asc LIMIT 100 ) AS subq" }, + { + "FindTest_Negative1QueryParams_Pagination", + @" + SELECT JSON_ARRAYAGG(JSON_OBJECT('id', id, 'bkname', bkname)) AS data + FROM ( + SELECT * + FROM " + _integrationPaginationTableName + @" + ORDER BY id asc + LIMIT 100000 + ) AS subq" + }, { "FindTest_OrderByNotFirstQueryParam_PaginationNextLink", @" diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs index b7aeea0265..ab99890579 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs @@ -448,6 +448,18 @@ LIMIT 100 ) AS subq " }, + { + "FindTest_Negative1QueryParams_Pagination", + @" + SELECT json_agg(to_jsonb(subq)) AS data + FROM ( + SELECT * + FROM " + _integrationPaginationTableName + @" + ORDER BY id asc + LIMIT 100000 + ) AS subq + " + }, { "FindTest_OrderByNotFirstQueryParam_PaginationNextLink", @" From 8f6d7256a158cd1f1be46bf954fd7a1e2eb41688 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 5 Apr 2024 11:58:24 -0700 Subject: [PATCH 11/27] dotnet format. --- src/Core/Resolvers/SqlPaginationUtil.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 3fdb18fbb9..85ba16a65a 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -580,7 +580,7 @@ public static JsonElement CreateNextLink(string path, NameValueCollection? query /// Client provided limit if one exists, otherwise 0. /// Default limit if none provided. /// Bool representing if more records are available. - public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE, uint maxPageSize = PaginationOptions.MAX_PAGE_SIZE ) + public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE, uint maxPageSize = PaginationOptions.MAX_PAGE_SIZE) { // When first is null we use default limit from runtime config, otherwise we use first uint numRecords = (uint)jsonResult.GetArrayLength(); From d6dc671ebbcc151dd07127ab4d70f031be10dcb4 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 5 Apr 2024 12:27:21 -0700 Subject: [PATCH 12/27] fixing test. --- src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index d30da02f27..565c7a58d2 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -1570,7 +1570,7 @@ await SetupAndRunRestApiTest( entityNameOrPath: _integrationEntityName, sqlQuery: string.Empty, exceptionExpected: true, - expectedErrorMessage: "Invalid number of items requested, $first must be an integer greater than 0. Actual value: 0", + expectedErrorMessage: "Invalid number of items requested, $first must be -1 or an integer greater than 0. Actual value: 0", expectedStatusCode: HttpStatusCode.BadRequest ); } From 7fa2a46c4f3091e3d889deb6246a0e042863f8ae Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Thu, 11 Apr 2024 08:26:12 -0700 Subject: [PATCH 13/27] Removed the default parameters. --- src/Core/Resolvers/SqlPaginationUtil.cs | 3 +-- src/Service.GraphQLBuilder/Queries/QueryBuilder.cs | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 85ba16a65a..c7ffb75e4a 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -6,7 +6,6 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Text.Json.Serialization; -using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Parsers; @@ -580,7 +579,7 @@ public static JsonElement CreateNextLink(string path, NameValueCollection? query /// Client provided limit if one exists, otherwise 0. /// Default limit if none provided. /// Bool representing if more records are available. - public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE, uint maxPageSize = PaginationOptions.MAX_PAGE_SIZE) + public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize, uint maxPageSize) { // When first is null we use default limit from runtime config, otherwise we use first uint numRecords = (uint)jsonResult.GetArrayLength(); diff --git a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs index c4b0a0d34a..b68eb642af 100644 --- a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs +++ b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs @@ -24,8 +24,6 @@ public static class QueryBuilder public const string ORDER_BY_FIELD_NAME = "orderBy"; public const string PARTITION_KEY_FIELD_NAME = "_partitionKeyValue"; public const string ID_FIELD_NAME = "id"; - public const int MAX_PAGE_LIMIT = 1000; - public const int DEFAULT_PAGE_LIMIT = 100; /// /// Creates a DocumentNode containing FieldDefinitionNodes representing the FindByPK and FindAll queries From a884bfe9a852d1386aa3096278f7f2fe03bd9eb7 Mon Sep 17 00:00:00 2001 From: rohkhann <124841904+rohkhann@users.noreply.github.com> Date: Fri, 12 Apr 2024 08:41:39 -0700 Subject: [PATCH 14/27] Apply suggestions from code review Co-authored-by: Sean Leonard --- src/Core/Configurations/RuntimeConfigValidator.cs | 2 +- src/Core/Resolvers/BaseQueryStructure.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index b1c4d21f33..42ea462e82 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -154,7 +154,7 @@ public void ValidatePaginationOptions(RuntimeConfig runtimeConfig) paginationOptions.DefaultPageSize > paginationOptions.MaxPageSize) { HandleOrRecordException(new DataApiBuilderException( - message: "Pagination options invalid. Page size arguments cannot be 0, < -1 and default page size cannot be greater than max page size", + message: "Pagination options invalid. Page size arguments cannot be 0 or less than -1. The default page size cannot be greater than max page size", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); } diff --git a/src/Core/Resolvers/BaseQueryStructure.cs b/src/Core/Resolvers/BaseQueryStructure.cs index ad2830dfff..4767d6ea23 100644 --- a/src/Core/Resolvers/BaseQueryStructure.cs +++ b/src/Core/Resolvers/BaseQueryStructure.cs @@ -208,7 +208,7 @@ internal static int GetPaginationLimit(RuntimeConfigProvider runtimeConfigProvid if (userInputValue < -1 || userInputValue == 0 || userInputValue > maxPageSize) { throw new DataApiBuilderException( - message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {userInputValue}", + message: $"Invalid page size requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {userInputValue}", statusCode: HttpStatusCode.BadRequest, subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); } From 02729a36c2ee01844e69eae190ce0d0d1ef6643c Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 12 Apr 2024 10:28:38 -0700 Subject: [PATCH 15/27] Addressing test cases --- src/Config/ObjectModel/PaginationOptions.cs | 33 +++++++++++--- src/Config/ObjectModel/RuntimeConfig.cs | 10 +++++ .../Configurations/RuntimeConfigProvider.cs | 44 +++++++++++++++++++ .../Configurations/RuntimeConfigValidator.cs | 23 ---------- src/Core/Resolvers/BaseQueryStructure.cs | 41 ----------------- src/Core/Resolvers/CosmosQueryStructure.cs | 4 +- .../Sql Query Structures/SqlQueryStructure.cs | 4 +- src/Core/Resolvers/SqlPaginationUtil.cs | 25 +++++++++-- src/Core/Resolvers/SqlResponseHelpers.cs | 4 +- .../GraphQLPaginationTestBase.cs | 4 +- .../GraphQLQueryTests/GraphQLQueryTestBase.cs | 4 ++ .../RestApiTests/Find/FindApiTestBase.cs | 9 ++-- src/Service.Tests/SqlTests/SqlTestBase.cs | 2 +- 13 files changed, 122 insertions(+), 85 deletions(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index 32c587bfa5..78edaf22d5 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -2,7 +2,9 @@ // Licensed under the MIT License. using System.Diagnostics.CodeAnalysis; +using System.Net; using System.Text.Json.Serialization; +using Azure.DataApiBuilder.Service.Exceptions; namespace Azure.DataApiBuilder.Config.ObjectModel; @@ -16,12 +18,12 @@ public record PaginationOptions /// /// Default page size. /// - public const int DEFAULT_PAGE_SIZE = 100; + public const uint DEFAULT_PAGE_SIZE = 100; /// /// Max page size. /// - public const int MAX_PAGE_SIZE = 100000; + public const uint MAX_PAGE_SIZE = 100000; /// /// The default page size for pagination. @@ -40,6 +42,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) { if (MaxPageSize is not null) { + ValidatePageSize((int)MaxPageSize); this.MaxPageSize = MaxPageSize == -1 ? UInt32.MaxValue : (uint)MaxPageSize; UserProvidedMaxPageSize = true; } @@ -50,6 +53,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) if (DefaultPageSize is not null) { + ValidatePageSize((int)DefaultPageSize); this.DefaultPageSize = DefaultPageSize == -1 ? (uint)this.MaxPageSize : (uint)DefaultPageSize; UserProvidedDefaultPageSize = true; } @@ -57,17 +61,25 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) { this.DefaultPageSize = DEFAULT_PAGE_SIZE; } + + if (this.DefaultPageSize > this.MaxPageSize) + { + throw new DataApiBuilderException( + message: "Pagination options invalid. Page size arguments cannot be 0, < -1, exceed max int value and default page size cannot be greater than max page size", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + } } /// - /// Flag which informs CLI and JSON serializer whether to write ttl-seconds + /// Flag which informs CLI and JSON serializer whether to write default page size. /// property and value to the runtime config file. /// When user doesn't provide the default-page-size property/value, which signals DAB to use the default, /// the DAB CLI should not write the default value to a serialized config. /// This is because the user's intent is to use DAB's default value which could change /// and DAB CLI writing the property and value would lose the user's intent. /// This is because if the user were to use the CLI created config, a default-page-size - /// property/value specified would be interpreted by DAB as "user explicitly set ttl." + /// property/value specified would be interpreted by DAB as "user explicitly default-page-size." /// [JsonIgnore(Condition = JsonIgnoreCondition.Always)] [MemberNotNullWhen(true, nameof(DefaultPageSize))] @@ -81,10 +93,21 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) /// This is because the user's intent is to use DAB's default value which could change /// and DAB CLI writing the property and value would lose the user's intent. /// This is because if the user were to use the CLI created config, a max-page-size - /// property/value specified would be interpreted by DAB as "user explicitly set ttl." + /// property/value specified would be interpreted by DAB as "user explicitly max-page-size." /// [JsonIgnore(Condition = JsonIgnoreCondition.Always)] [MemberNotNullWhen(true, nameof(MaxPageSize))] public bool UserProvidedMaxPageSize { get; init; } = false; + private static void ValidatePageSize(int pageSize) + { + if (pageSize < -1 || pageSize == 0 || (uint)pageSize > Int32.MaxValue) + { + throw new DataApiBuilderException( + message: "Pagination options invalid. Page size arguments cannot be 0, < -1, exceed max int value and default page size cannot be greater than max page size", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + } + } + } diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index b115517b1a..02ce066bd8 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -471,4 +471,14 @@ Runtime.GraphQL.MultipleMutationOptions is not null && Runtime.GraphQL.MultipleMutationOptions.MultipleCreateOptions is not null && Runtime.GraphQL.MultipleMutationOptions.MultipleCreateOptions.Enabled); } + + public uint DefaultPageSize() + { + return Runtime?.Pagination?.DefaultPageSize ?? PaginationOptions.DEFAULT_PAGE_SIZE; + } + + public uint MaxPageSize() + { + return Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; + } } diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 58ae65c60a..70ae228226 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -9,6 +9,7 @@ using Azure.DataApiBuilder.Config.NamingPolicies; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Service.Exceptions; +using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; namespace Azure.DataApiBuilder.Core.Configurations; @@ -278,6 +279,49 @@ public async Task Initialize( return false; } + /// + /// Get the pagination limit from the runtime configuration. + /// + /// The pagination input from the user. Example: $first=10 + /// + /// + public uint GetPaginationLimit(int? first) + { + this.TryGetConfig(out RuntimeConfig? runtimeConfig); + RuntimeOptions? runtimeOptions = runtimeConfig?.Runtime; + uint defaultPageSize; + uint maxPageSize; + if (runtimeOptions is not null && runtimeOptions.Pagination is not null) + { + defaultPageSize = (uint)runtimeOptions.Pagination.DefaultPageSize!; + maxPageSize = (uint)runtimeOptions.Pagination.MaxPageSize!; + } + else + { + defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE; + maxPageSize = PaginationOptions.MAX_PAGE_SIZE; + } + + if (first is not null) + { + if (first < -1 || first == 0 || first > maxPageSize) + { + throw new DataApiBuilderException( + message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {first}", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); + } + else + { + return (first == -1 ? maxPageSize : (uint)first); + } + } + else + { + return defaultPageSize; + } + } + private async Task InvokeConfigLoadedHandlersAsync() { List> configLoadedTasks = new(); diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index b1c4d21f33..7bb3aa2f7e 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -82,7 +82,6 @@ public void ValidateConfigProperties() ValidateAuthenticationOptions(runtimeConfig); ValidateGlobalEndpointRouteConfig(runtimeConfig); ValidateAppInsightsTelemetryConnectionString(runtimeConfig); - ValidatePaginationOptions(runtimeConfig); // Running these graphQL validations only in development mode to ensure // fast startup of engine in production mode. @@ -139,28 +138,6 @@ public void ValidateAppInsightsTelemetryConnectionString(RuntimeConfig runtimeCo } } - /// - /// Validating pagination options in the runtime config. - /// - public void ValidatePaginationOptions(RuntimeConfig runtimeConfig) - { - if (runtimeConfig.Runtime is not null && runtimeConfig.Runtime.Pagination is not null) - { - PaginationOptions paginationOptions = runtimeConfig.Runtime.Pagination; - if (paginationOptions.DefaultPageSize < -1 || - paginationOptions.DefaultPageSize == 0 || - paginationOptions.MaxPageSize < -1 || - paginationOptions.MaxPageSize == 0 || - paginationOptions.DefaultPageSize > paginationOptions.MaxPageSize) - { - HandleOrRecordException(new DataApiBuilderException( - message: "Pagination options invalid. Page size arguments cannot be 0, < -1 and default page size cannot be greater than max page size", - statusCode: HttpStatusCode.ServiceUnavailable, - subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); - } - } - } - /// /// This method runs several validations against the config file such as schema validation, /// validation of entities metadata, validation of permissions, validation of entity configuration. diff --git a/src/Core/Resolvers/BaseQueryStructure.cs b/src/Core/Resolvers/BaseQueryStructure.cs index ad2830dfff..e8cb912770 100644 --- a/src/Core/Resolvers/BaseQueryStructure.cs +++ b/src/Core/Resolvers/BaseQueryStructure.cs @@ -1,14 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Net; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; -using Azure.DataApiBuilder.Config.ObjectModel; -using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Services; -using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.GraphQLBuilder; using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; using HotChocolate.Language; @@ -185,42 +181,5 @@ internal static IObjectField ExtractItemsSchemaField(IObjectField connectionSche { return GraphQLUtils.UnderlyingGraphQLEntityType(connectionSchemaField.Type).Fields[QueryBuilder.PAGINATION_FIELD_NAME]; } - - internal static int GetPaginationLimit(RuntimeConfigProvider runtimeConfigProvider, int? userInputValue) - { - runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - RuntimeOptions? runtimeOptions = runtimeConfig?.Runtime; - int defaultPageSize; - int maxPageSize; - if (runtimeOptions is not null && runtimeOptions.Pagination is not null) - { - defaultPageSize = (int)runtimeOptions.Pagination.DefaultPageSize!; - maxPageSize = (int)runtimeOptions.Pagination.MaxPageSize!; - } - else - { - defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE; - maxPageSize = PaginationOptions.MAX_PAGE_SIZE; - } - - if (userInputValue is not null) - { - if (userInputValue < -1 || userInputValue == 0 || userInputValue > maxPageSize) - { - throw new DataApiBuilderException( - message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {userInputValue}", - statusCode: HttpStatusCode.BadRequest, - subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); - } - else - { - return (int)(userInputValue == -1 ? maxPageSize : userInputValue); - } - } - else - { - return defaultPageSize; - } - } } } diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index 54fe11f834..2537c19589 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -27,7 +27,7 @@ public class CosmosQueryStructure : BaseQueryStructure public string Container { get; internal set; } public string Database { get; internal set; } public string? Continuation { get; internal set; } - public int? MaxItemCount { get; internal set; } + public uint? MaxItemCount { get; internal set; } public string? PartitionKeyValue { get; internal set; } public List OrderByColumns { get; internal set; } @@ -146,7 +146,7 @@ private void Init(IDictionary queryParams) // TODO: Revisit 'first' while adding support for TOP queries if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { - MaxItemCount = GetPaginationLimit(RuntimeConfigProvider, (int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); + MaxItemCount = RuntimeConfigProvider.GetPaginationLimit((int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index b970515768..87c2269f29 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -192,7 +192,7 @@ public SqlQueryStructure( } AddColumnsForEndCursor(); - _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int?)context.First) + 1; + _limit = runtimeConfigProvider.GetPaginationLimit((int?)context.First) + 1; ParametrizeColumns(); } @@ -336,7 +336,7 @@ private SqlQueryStructure( if (firstObject != null) { - _limit = (uint?)GetPaginationLimit(runtimeConfigProvider, (int)firstObject); + _limit = runtimeConfigProvider.GetPaginationLimit((int)firstObject); } } diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index c7ffb75e4a..5d44dfd05b 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -577,14 +577,33 @@ public static JsonElement CreateNextLink(string path, NameValueCollection? query /// /// Results plus one extra record if more exist. /// Client provided limit if one exists, otherwise 0. - /// Default limit if none provided. + /// Default limit for page size. + /// Maximum limit for page size. /// Bool representing if more records are available. public static bool HasNext(JsonElement jsonResult, int? first, uint defaultPageSize, uint maxPageSize) { // When first is null we use default limit from runtime config, otherwise we use first uint numRecords = (uint)jsonResult.GetArrayLength(); - first = first is not null ? first : (int)defaultPageSize; - uint? limit = first == -1 ? maxPageSize : (uint)first; + + uint limit; + if (first.HasValue) + { + // first is not null. + if (first == -1) + { + // user has requested max value. + limit = maxPageSize; + } + else + { + limit = (uint)first; + } + } + else + { + limit = defaultPageSize; + } + return numRecords > limit; } diff --git a/src/Core/Resolvers/SqlResponseHelpers.cs b/src/Core/Resolvers/SqlResponseHelpers.cs index c4549c6b95..7701d662d3 100644 --- a/src/Core/Resolvers/SqlResponseHelpers.cs +++ b/src/Core/Resolvers/SqlResponseHelpers.cs @@ -51,8 +51,8 @@ public static OkObjectResult FormatFindResult( ? DetermineExtraFieldsInResponse(findOperationResponse, context.FieldsToBeReturned) : DetermineExtraFieldsInResponse(findOperationResponse.EnumerateArray().First(), context.FieldsToBeReturned); - uint defaultPageSize = runtimeConfig.Runtime?.Pagination?.DefaultPageSize ?? PaginationOptions.DEFAULT_PAGE_SIZE; - uint maxPageSize = runtimeConfig.Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; + uint defaultPageSize = runtimeConfig.DefaultPageSize(); + uint maxPageSize = runtimeConfig.MaxPageSize(); // If the results are not a collection or if the query does not have a next page // no nextLink is needed. So, the response is returned after removing the extra fields. diff --git a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs index 21ecf6d19d..8da154717c 100644 --- a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs @@ -66,7 +66,8 @@ public async Task RequestFullConnection() /// /// Request a full connection object {items, after, hasNextPage} - /// without providing any parameters + /// using a negative one for the first parameter. + /// This should return max items as we use -1 to allow user to get max allowed page size. /// [TestMethod] public async Task RequestMaxUsingNegativeOne() @@ -83,6 +84,7 @@ public async Task RequestMaxUsingNegativeOne() } }"; + // this resultset represents all books in the db. JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); string expected = @"{ ""items"": [ diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs index d176915a3d..0e7c31e48f 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs @@ -1568,6 +1568,10 @@ public async Task GraphQLQueryWithMultipleOfTheSameFieldReturnsFieldOnce() #region Negative Tests + /// + /// This test checks the failure on providing invalid nfirst parameter in graphQL Query. + /// We only allow -1 or positive integers for first parameter.-1 means max page size. + /// [TestMethod] public virtual async Task TestInvalidFirstParamQuery() { diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index 565c7a58d2..6651df62ee 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -763,10 +763,9 @@ await SetupAndRunRestApiTest( } /// - /// Validates that a proper nextLink is created for FindMany requests which do not - /// restrict results with query parameters. Engine default paging mechanisms are used - /// when > 100 records will be present in result set. - /// expectedAfterQueryString starts with ?$, and not &$, because it is the only query parameter. + /// Validates that when first is set to -1, we return the maximum records + /// as specified by max page size. In this case, the expected result is that + /// all the records are returned from the db. /// [TestMethod] public async Task FindTest_Negative1QueryParams_Pagination() @@ -1578,7 +1577,7 @@ await SetupAndRunRestApiTest( /// /// Tests the REST Api for Find operation using $first=100001 /// to request > max records of 100000 records, which should throw a DataApiBuilder - /// Exception. + /// Exception. This test depends on max page size configuration in runtimeconfig. /// [TestMethod] public async Task FindTestWithMaxExceededSingleKeyPagination() diff --git a/src/Service.Tests/SqlTests/SqlTestBase.cs b/src/Service.Tests/SqlTests/SqlTestBase.cs index 56fd892d4b..3d4b9b48cc 100644 --- a/src/Service.Tests/SqlTests/SqlTestBase.cs +++ b/src/Service.Tests/SqlTests/SqlTestBase.cs @@ -57,7 +57,7 @@ public abstract class SqlTestBase protected static string _defaultSchemaName; protected static string _defaultSchemaVersion; protected static IAuthorizationResolver _authorizationResolver; - private static WebApplicationFactory _application; + protected static WebApplicationFactory _application; protected static ILogger _sqlMetadataLogger; protected static ILogger _mutationEngineLogger; protected static ILogger _queryEngineLogger; From 3650a696800553f171fc9f09ecdfe7e0c28184af Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 12 Apr 2024 10:34:15 -0700 Subject: [PATCH 16/27] Addressing comments. --- src/Config/ObjectModel/PaginationOptions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index 78edaf22d5..fe7271f9d7 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -65,7 +65,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) if (this.DefaultPageSize > this.MaxPageSize) { throw new DataApiBuilderException( - message: "Pagination options invalid. Page size arguments cannot be 0, < -1, exceed max int value and default page size cannot be greater than max page size", + message: "Pagination options invalid. The default page size cannot be greater than max page size", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); } @@ -104,7 +104,7 @@ private static void ValidatePageSize(int pageSize) if (pageSize < -1 || pageSize == 0 || (uint)pageSize > Int32.MaxValue) { throw new DataApiBuilderException( - message: "Pagination options invalid. Page size arguments cannot be 0, < -1, exceed max int value and default page size cannot be greater than max page size", + message: "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); } From b232ae4b505be63fd020c0474333c587de5c1439 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 12 Apr 2024 11:34:55 -0700 Subject: [PATCH 17/27] ConfigValidationTests. --- src/Config/ObjectModel/PaginationOptions.cs | 4 +- .../Configurations/RuntimeConfigProvider.cs | 15 +---- .../Unittests/ConfigValidationUnitTests.cs | 61 +++++++++++++++++++ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index fe7271f9d7..62f5be9b3b 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -43,7 +43,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) if (MaxPageSize is not null) { ValidatePageSize((int)MaxPageSize); - this.MaxPageSize = MaxPageSize == -1 ? UInt32.MaxValue : (uint)MaxPageSize; + this.MaxPageSize = MaxPageSize == -1 ? Int32.MaxValue : (uint)MaxPageSize; UserProvidedMaxPageSize = true; } else @@ -101,7 +101,7 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) private static void ValidatePageSize(int pageSize) { - if (pageSize < -1 || pageSize == 0 || (uint)pageSize > Int32.MaxValue) + if (pageSize < -1 || pageSize == 0 || pageSize > Int32.MaxValue) { throw new DataApiBuilderException( message: "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 70ae228226..1bd4ab60d6 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -288,19 +288,8 @@ public async Task Initialize( public uint GetPaginationLimit(int? first) { this.TryGetConfig(out RuntimeConfig? runtimeConfig); - RuntimeOptions? runtimeOptions = runtimeConfig?.Runtime; - uint defaultPageSize; - uint maxPageSize; - if (runtimeOptions is not null && runtimeOptions.Pagination is not null) - { - defaultPageSize = (uint)runtimeOptions.Pagination.DefaultPageSize!; - maxPageSize = (uint)runtimeOptions.Pagination.MaxPageSize!; - } - else - { - defaultPageSize = PaginationOptions.DEFAULT_PAGE_SIZE; - maxPageSize = PaginationOptions.MAX_PAGE_SIZE; - } + uint defaultPageSize = runtimeConfig!.DefaultPageSize(); + uint maxPageSize = runtimeConfig.MaxPageSize(); if (first is not null) { diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 6d6065b866..755f2e1ea7 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -2179,6 +2179,67 @@ public void TestRuntimeConfigSetupWithNonJsonConstructor() $"Config does not have a cosmos datasource and member {nameof(runtimeConfig.CosmosDataSourceUsed)} must be marked as false."); } + /// + /// Test to validate pagination options. + /// + /// Should there be an exception. + /// default page size to go into config. + /// max page size to go into config. + /// expected exception message in case there is exception. + /// expected default page size from config. + /// expected max page size from config. + [DataTestMethod] + [DataRow(false, null, null, "", (int)PaginationOptions.DEFAULT_PAGE_SIZE, (int)PaginationOptions.MAX_PAGE_SIZE, + DisplayName = "MaxPageSize should be 100,000 and DefaultPageSize should be 100 when no value provided in config.")] + [DataRow(false, 1000, 10000, "", 1000, 10000, + DisplayName = "Valid inputs of MaxPageSize and DefaultPageSize must be accepted and set in the config.")] + [DataRow(false, -1, 10000, "", 10000, 10000, + DisplayName = "DefaultPageSize should be the same as MaxPageSize when input is -1")] + [DataRow(false, 100, -1, "", 100, Int32.MaxValue, + DisplayName = "MaxPageSize should be UInt32.MaxValue when input is -1")] + [DataRow(true, 100, -3, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", + DisplayName = "MaxPageSize cannot be negative")] + [DataRow(true, -3, 100, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", + DisplayName = "DefaultPageSize cannot be negative")] + [DataRow(true, 100, 0, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", + DisplayName = "MaxPageSize cannot be 0")] + [DataRow(true, 0, 100, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", + DisplayName = "DefaultPageSize cannot be 0")] + [DataRow(true, 101, 100, "Pagination options invalid. The default page size cannot be greater than max page size", + DisplayName = "DefaultPageSize cannot be greater than MaxPageSize")] + public void ValidatePaginationOptionsInConfig( + bool exceptionExpected, + int? defaultPageSize, + int? maxPageSize, + string expectedExceptionMessage, + int? expectedDefaultPageSize = null, + int? expectedMaxPageSize = null) + { + try + { + RuntimeConfig runtimeConfig = new( + Schema: "UnitTestSchema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Host: new(Cors: null, Authentication: null), + Pagination: new PaginationOptions(defaultPageSize, maxPageSize) + ), + Entities: new(new Dictionary())); + + Assert.AreEqual((uint)expectedDefaultPageSize, runtimeConfig.DefaultPageSize()); + Assert.AreEqual((uint)expectedMaxPageSize, runtimeConfig.MaxPageSize()); + } + catch (DataApiBuilderException dabException) + { + Assert.IsTrue(exceptionExpected); + Assert.AreEqual(expectedExceptionMessage, dabException.Message); + Assert.AreEqual(expected: HttpStatusCode.ServiceUnavailable, actual: dabException.StatusCode); + Assert.AreEqual(expected: DataApiBuilderException.SubStatusCodes.ConfigValidationError, actual: dabException.SubStatusCode); + } + } + private static RuntimeConfigValidator InitializeRuntimeConfigValidator() { MockFileSystem fileSystem = new(); From c38ee072c2aa5061cb4e20bd1d4e66d85e2f6283 Mon Sep 17 00:00:00 2001 From: rohkhann <124841904+rohkhann@users.noreply.github.com> Date: Mon, 15 Apr 2024 09:01:21 -0700 Subject: [PATCH 18/27] Update src/Core/Configurations/RuntimeConfigProvider.cs Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> --- src/Core/Configurations/RuntimeConfigProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 1bd4ab60d6..0f21e77378 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -296,7 +296,7 @@ public uint GetPaginationLimit(int? first) if (first < -1 || first == 0 || first > maxPageSize) { throw new DataApiBuilderException( - message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {first}", + message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive integer within the max page size limit of {maxPageSize}. Actual value: {first}", statusCode: HttpStatusCode.BadRequest, subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); } From b9d50e28b536400cc734fbc74785770c14103111 Mon Sep 17 00:00:00 2001 From: rohkhann <124841904+rohkhann@users.noreply.github.com> Date: Mon, 15 Apr 2024 09:06:37 -0700 Subject: [PATCH 19/27] Apply suggestions from code review Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> --- .../SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs index 0e7c31e48f..ac00e98b1e 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs @@ -1569,7 +1569,7 @@ public async Task GraphQLQueryWithMultipleOfTheSameFieldReturnsFieldOnce() #region Negative Tests /// - /// This test checks the failure on providing invalid nfirst parameter in graphQL Query. + /// This test checks the failure on providing invalid first parameter in graphQL Query. /// We only allow -1 or positive integers for first parameter.-1 means max page size. /// [TestMethod] From c2d4e51f71c3bf762a9b5a6eedb5a545fbfd5291 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 09:20:52 -0700 Subject: [PATCH 20/27] Addressing comments. --- src/Config/ObjectModel/RuntimeConfig.cs | 31 +++++++++++++++++ .../Configurations/RuntimeConfigProvider.cs | 33 ------------------- src/Core/Resolvers/CosmosQueryStructure.cs | 4 ++- .../Sql Query Structures/SqlQueryStructure.cs | 6 ++-- src/Core/Services/RequestValidator.cs | 6 ++-- 5 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index 02ce066bd8..def0eee7bb 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -481,4 +481,35 @@ public uint MaxPageSize() { return Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; } + + /// + /// Get the pagination limit from the runtime configuration. + /// + /// The pagination input from the user. Example: $first=10 + /// + /// + public uint GetPaginationLimit(int? first) + { + uint defaultPageSize = this.DefaultPageSize(); + uint maxPageSize = this.MaxPageSize(); + + if (first is not null) + { + if (first < -1 || first == 0 || first > maxPageSize) + { + throw new DataApiBuilderException( + message: $"Invalid number of items requested, {nameof(first)} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {first}", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); + } + else + { + return (first == -1 ? maxPageSize : (uint)first); + } + } + else + { + return defaultPageSize; + } + } } diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index 1bd4ab60d6..58ae65c60a 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -9,7 +9,6 @@ using Azure.DataApiBuilder.Config.NamingPolicies; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Service.Exceptions; -using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; namespace Azure.DataApiBuilder.Core.Configurations; @@ -279,38 +278,6 @@ public async Task Initialize( return false; } - /// - /// Get the pagination limit from the runtime configuration. - /// - /// The pagination input from the user. Example: $first=10 - /// - /// - public uint GetPaginationLimit(int? first) - { - this.TryGetConfig(out RuntimeConfig? runtimeConfig); - uint defaultPageSize = runtimeConfig!.DefaultPageSize(); - uint maxPageSize = runtimeConfig.MaxPageSize(); - - if (first is not null) - { - if (first < -1 || first == 0 || first > maxPageSize) - { - throw new DataApiBuilderException( - message: $"Invalid number of items requested, {QueryBuilder.PAGE_START_ARGUMENT_NAME} argument must be either -1 or a positive number within the max page size limit of {maxPageSize}. Actual value: {first}", - statusCode: HttpStatusCode.BadRequest, - subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest); - } - else - { - return (first == -1 ? maxPageSize : (uint)first); - } - } - else - { - return defaultPageSize; - } - } - private async Task InvokeConfigLoadedHandlersAsync() { List> configLoadedTasks = new(); diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index 2537c19589..e63b94f49c 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; using Azure.DataApiBuilder.Core.Services; @@ -146,7 +147,8 @@ private void Init(IDictionary queryParams) // TODO: Revisit 'first' while adding support for TOP queries if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { - MaxItemCount = RuntimeConfigProvider.GetPaginationLimit((int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); + RuntimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + MaxItemCount = runtimeConfig?.GetPaginationLimit((int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 87c2269f29..e692959d8b 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -192,7 +192,8 @@ public SqlQueryStructure( } AddColumnsForEndCursor(); - _limit = runtimeConfigProvider.GetPaginationLimit((int?)context.First) + 1; + runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + _limit = runtimeConfig?.GetPaginationLimit((int?)context.First) + 1; ParametrizeColumns(); } @@ -336,7 +337,8 @@ private SqlQueryStructure( if (firstObject != null) { - _limit = runtimeConfigProvider.GetPaginationLimit((int)firstObject); + runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + _limit = runtimeConfig?.GetPaginationLimit((int)firstObject); } } diff --git a/src/Core/Services/RequestValidator.cs b/src/Core/Services/RequestValidator.cs index 25b7bad14e..0da00d9d62 100644 --- a/src/Core/Services/RequestValidator.cs +++ b/src/Core/Services/RequestValidator.cs @@ -542,13 +542,13 @@ private static StoredProcedureDefinition TryGetStoredProcedureDefinition(string /// /// Helper method checks the $first query param - /// to be sure that it can parse to a uint > 0 + /// to be sure that it can parse to a int > 0 or -1. /// /// String representing value associated with $first - /// uint > 0 representing $first + /// int > 0 or int == -1 representing $first public static int CheckFirstValidity(string first) { - if (!int.TryParse(first, out int firstAsInt) || firstAsInt == 0) + if (!int.TryParse(first, out int firstAsInt) || firstAsInt == 0 || firstAsInt < -1) { throw new DataApiBuilderException( message: $"Invalid number of items requested, $first must be -1 or an integer greater than 0. Actual value: {first}", From 36b2c1b1239a4c6722c592c8679d7288813bcd88 Mon Sep 17 00:00:00 2001 From: rohkhann <124841904+rohkhann@users.noreply.github.com> Date: Mon, 15 Apr 2024 11:22:38 -0700 Subject: [PATCH 21/27] Apply suggestions from code review Co-authored-by: Sean Leonard --- src/Config/ObjectModel/PaginationOptions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index 62f5be9b3b..c0a8916d5d 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -109,5 +109,4 @@ private static void ValidatePageSize(int pageSize) subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); } } - } From 23bf680dae2a399b74ac49a1b651e505828a8b62 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 12:38:21 -0700 Subject: [PATCH 22/27] Addressing comments. --- src/Core/Resolvers/CosmosQueryStructure.cs | 9 +++++++-- .../GraphQLPaginationTests/GraphQLPaginationTestBase.cs | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index e63b94f49c..f0ef66cf2d 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -147,8 +147,13 @@ private void Init(IDictionary queryParams) // TODO: Revisit 'first' while adding support for TOP queries if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { - RuntimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - MaxItemCount = runtimeConfig?.GetPaginationLimit((int)queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]!); + object? firstArgument = queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]; + + if (firstArgument is not null) + { + RuntimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + MaxItemCount = runtimeConfig?.GetPaginationLimit((int)firstArgument); + } queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } diff --git a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs index 8da154717c..01db456a71 100644 --- a/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs @@ -975,7 +975,8 @@ public async Task RequestInvalidZeroFirst() } /// - /// Request an invalid number of entries for a pagination page + /// Request an invalid number of entries for a pagination page. + /// Default max page size of config is 100000. Requesting 100001 entries, should lead to an error. /// [TestMethod] public async Task RequestInvalidMaxSize() From 9492e4944bcbfbd0d8043dc6207147ea5a7970ca Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 13:01:43 -0700 Subject: [PATCH 23/27] Addressing comment. --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 97d2c51161..3d0cd3e030 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -2364,6 +2364,7 @@ public void TestRuntimeConfigSetupWithNonJsonConstructor() /// /// Test to validate pagination options. + /// NOTE: Changing the default values of default page size and max page size would be a breaking change. /// /// Should there be an exception. /// default page size to go into config. @@ -2377,9 +2378,9 @@ public void TestRuntimeConfigSetupWithNonJsonConstructor() [DataRow(false, 1000, 10000, "", 1000, 10000, DisplayName = "Valid inputs of MaxPageSize and DefaultPageSize must be accepted and set in the config.")] [DataRow(false, -1, 10000, "", 10000, 10000, - DisplayName = "DefaultPageSize should be the same as MaxPageSize when input is -1")] + DisplayName = "DefaultPageSize should be the same as MaxPageSize when DefaultPageSize is -1 in config.")] [DataRow(false, 100, -1, "", 100, Int32.MaxValue, - DisplayName = "MaxPageSize should be UInt32.MaxValue when input is -1")] + DisplayName = "MaxPageSize should be assigned UInt32.MaxValue when MaxPageSize is -1 in config.")] [DataRow(true, 100, -3, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", DisplayName = "MaxPageSize cannot be negative")] [DataRow(true, -3, 100, "Pagination options invalid. Page size arguments cannot be 0, exceed max int value or be less than -1", From 122cd1ceb9090bac94444c060611a1106a16588c Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 13:06:04 -0700 Subject: [PATCH 24/27] fixing comments. --- src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index e692959d8b..4dc34bb016 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -193,7 +193,7 @@ public SqlQueryStructure( AddColumnsForEndCursor(); runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - _limit = runtimeConfig?.GetPaginationLimit((int?)context.First) + 1; + _limit = runtimeConfig!.GetPaginationLimit((int?)context.First) + 1; ParametrizeColumns(); } @@ -338,7 +338,7 @@ private SqlQueryStructure( if (firstObject != null) { runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - _limit = runtimeConfig?.GetPaginationLimit((int)firstObject); + _limit = runtimeConfig!.GetPaginationLimit((int)firstObject); } } From dbd90b6c48f4be09c8fe80594b2a074057f1ed09 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 13:31:29 -0700 Subject: [PATCH 25/27] Fixing the ODataVisitorTests. --- src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 4dc34bb016..e692959d8b 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -193,7 +193,7 @@ public SqlQueryStructure( AddColumnsForEndCursor(); runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - _limit = runtimeConfig!.GetPaginationLimit((int?)context.First) + 1; + _limit = runtimeConfig?.GetPaginationLimit((int?)context.First) + 1; ParametrizeColumns(); } @@ -338,7 +338,7 @@ private SqlQueryStructure( if (firstObject != null) { runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - _limit = runtimeConfig!.GetPaginationLimit((int)firstObject); + _limit = runtimeConfig?.GetPaginationLimit((int)firstObject); } } From 691588098cd11ccd40eb57db01a135c65dfe9e6a Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 14:22:33 -0700 Subject: [PATCH 26/27] Fixing issues while parsing the runtimeconfig. --- src/Config/ObjectModel/PaginationOptions.cs | 12 ++++++------ src/Config/ObjectModel/RuntimeConfig.cs | 4 ++-- .../Sql Query Structures/SqlQueryStructure.cs | 18 +++++++++++------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/Config/ObjectModel/PaginationOptions.cs b/src/Config/ObjectModel/PaginationOptions.cs index c0a8916d5d..ab4bff29ff 100644 --- a/src/Config/ObjectModel/PaginationOptions.cs +++ b/src/Config/ObjectModel/PaginationOptions.cs @@ -29,13 +29,13 @@ public record PaginationOptions /// The default page size for pagination. /// [JsonPropertyName("default-page-size")] - public uint? DefaultPageSize { get; init; } = null; + public int? DefaultPageSize { get; init; } = null; /// /// The max page size for pagination. /// [JsonPropertyName("max-page-size")] - public uint? MaxPageSize { get; init; } = null; + public int? MaxPageSize { get; init; } = null; [JsonConstructor] public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) @@ -43,23 +43,23 @@ public PaginationOptions(int? DefaultPageSize = null, int? MaxPageSize = null) if (MaxPageSize is not null) { ValidatePageSize((int)MaxPageSize); - this.MaxPageSize = MaxPageSize == -1 ? Int32.MaxValue : (uint)MaxPageSize; + this.MaxPageSize = MaxPageSize == -1 ? Int32.MaxValue : (int)MaxPageSize; UserProvidedMaxPageSize = true; } else { - this.MaxPageSize = MAX_PAGE_SIZE; + this.MaxPageSize = (int)MAX_PAGE_SIZE; } if (DefaultPageSize is not null) { ValidatePageSize((int)DefaultPageSize); - this.DefaultPageSize = DefaultPageSize == -1 ? (uint)this.MaxPageSize : (uint)DefaultPageSize; + this.DefaultPageSize = DefaultPageSize == -1 ? (int)this.MaxPageSize : (int)DefaultPageSize; UserProvidedDefaultPageSize = true; } else { - this.DefaultPageSize = DEFAULT_PAGE_SIZE; + this.DefaultPageSize = (int)DEFAULT_PAGE_SIZE; } if (this.DefaultPageSize > this.MaxPageSize) diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index def0eee7bb..05c4f35ebc 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -474,12 +474,12 @@ Runtime.GraphQL.MultipleMutationOptions.MultipleCreateOptions is not null && public uint DefaultPageSize() { - return Runtime?.Pagination?.DefaultPageSize ?? PaginationOptions.DEFAULT_PAGE_SIZE; + return (uint?)Runtime?.Pagination?.DefaultPageSize ?? PaginationOptions.DEFAULT_PAGE_SIZE; } public uint MaxPageSize() { - return Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; + return (uint?)Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; } /// diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index e692959d8b..46e6aaf93d 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -330,15 +330,19 @@ private SqlQueryStructure( IsListQuery = outputType.IsListType(); } - if (IsListQuery && queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) + if (IsListQuery) { - // parse first parameter for all list queries - object? firstObject = queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]; - - if (firstObject != null) + runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) + { + // parse first parameter for all list queries + object? firstObject = queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]; + _limit = runtimeConfig?.GetPaginationLimit((int?)firstObject); + } + else { - runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - _limit = runtimeConfig?.GetPaginationLimit((int)firstObject); + // if first is not passed, we should use the default page size. + _limit = runtimeConfig?.DefaultPageSize(); } } From 833f041a1e79fcc901e1b63ffd5f8618c47cff03 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Mon, 15 Apr 2024 14:29:30 -0700 Subject: [PATCH 27/27] Set default page size cosmos. --- src/Core/Resolvers/CosmosQueryStructure.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index f0ef66cf2d..0f9f01d76c 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -143,20 +143,22 @@ private void Init(IDictionary queryParams) Container = MetadataProvider.GetDatabaseObjectName(entityName); } + RuntimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); + // first and after will not be part of query parameters. They will be going into headers instead. // TODO: Revisit 'first' while adding support for TOP queries if (queryParams.ContainsKey(QueryBuilder.PAGE_START_ARGUMENT_NAME)) { object? firstArgument = queryParams[QueryBuilder.PAGE_START_ARGUMENT_NAME]; - - if (firstArgument is not null) - { - RuntimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig); - MaxItemCount = runtimeConfig?.GetPaginationLimit((int)firstArgument); - } + MaxItemCount = runtimeConfig?.GetPaginationLimit((int?)firstArgument); queryParams.Remove(QueryBuilder.PAGE_START_ARGUMENT_NAME); } + else + { + // set max item count to default value. + MaxItemCount = runtimeConfig?.DefaultPageSize(); + } if (queryParams.ContainsKey(QueryBuilder.PAGINATION_TOKEN_ARGUMENT_NAME)) {