From 0040aa53d433fbec75e016ebacbf555ea99a7ccb Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 31 May 2024 13:48:44 -0700 Subject: [PATCH 01/12] Add MaxResponse Size to runtime config. --- src/Config/ObjectModel/HostOptions.cs | 41 +++++++++++++++++- src/Config/ObjectModel/RuntimeConfig.cs | 5 +++ .../Unittests/ConfigValidationUnitTests.cs | 42 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index c8b2759419..02223757a2 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -1,6 +1,45 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Net; +using System.Text.Json.Serialization; +using Azure.DataApiBuilder.Service.Exceptions; + namespace Azure.DataApiBuilder.Config.ObjectModel; -public record HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, HostMode Mode = HostMode.Production); +public record HostOptions +{ + /// + /// Dab engine can at maximum handle 187 MB of data in a single response from a source. + /// Json deserialization of a response into a string has a limit of 197020041 bytes which when converted to MB is 187 MB. + /// + private const int MAX_RESPONSE_LENGTH_DAB_ENGINE_MB = 187; + + [JsonPropertyName("cors")] + public CorsOptions? Cors { get; init; } + + [JsonPropertyName("authentication")] + public AuthenticationOptions? Authentication { get; init; } + + [JsonPropertyName("mode")] + public HostMode Mode { get; init; } + + [JsonPropertyName("max-response-size-mb")] + public int? MaxResponseSizeMB { get; init; } + + public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, HostMode Mode = HostMode.Production, int? MaxResponseSizeMB = null) + { + this.Cors = Cors; + this.Authentication = Authentication; + this.Mode = Mode; + this.MaxResponseSizeMB = MaxResponseSizeMB; + + if (this.MaxResponseSizeMB is not null && (this.MaxResponseSizeMB < 1 || this.MaxResponseSizeMB > MAX_RESPONSE_LENGTH_DAB_ENGINE_MB)) + { + throw new DataApiBuilderException( + message: $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + } + } +} diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index fb8545e4e9..340515b6f7 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -495,6 +495,11 @@ public uint MaxPageSize() return (uint?)Runtime?.Pagination?.MaxPageSize ?? PaginationOptions.MAX_PAGE_SIZE; } + public int? MaxResponseSizeMB() + { + return Runtime?.Host?.MaxResponseSizeMB; + } + /// /// Get the pagination limit from the runtime configuration. /// diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 941b6ee8d8..59a39f8712 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -2414,6 +2414,48 @@ public void ValidatePaginationOptionsInConfig( } } + /// + /// Test to validate the max response size option in the runtime config. + /// + /// should there be an exception. + /// maxResponse size input + /// expected exception message in case there is exception. + /// expected value in config. + [DataTestMethod] + [DataRow(false, null, "", null, + DisplayName = "MaxDbResponseSizeMB should be null when no value provided in config.")] + [DataRow(false, 64, "", 64, + DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] + [DataRow(true, -1, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", null, + DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] + [DataRow(true, 188, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", null, + DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] + public void ValidateMaxResponseSizeInConfig( + bool exceptionExpected, + int? maxDbResponseSizeMB, + string expectedExceptionMessage, + int? expectedMaxResponseSize = 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, MaxResponseSizeMB: maxDbResponseSizeMB) + ), + Entities: new(new Dictionary())); + Assert.AreEqual(expectedMaxResponseSize, runtimeConfig.MaxResponseSizeMB()); + } + catch (DataApiBuilderException ex) + { + Assert.IsTrue(exceptionExpected); + Assert.AreEqual(expectedExceptionMessage, ex.Message); + } + } + private static RuntimeConfigValidator InitializeRuntimeConfigValidator() { MockFileSystem fileSystem = new(); From 51e4437e2ced43bd4d9bbdc4c0de09c9323d4154 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Fri, 31 May 2024 13:52:17 -0700 Subject: [PATCH 02/12] Adding maxResponseSizeMb to the runtimeconfig. --- src/Config/ObjectModel/HostOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index 02223757a2..4d76c90f33 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -37,7 +37,7 @@ public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, Hos if (this.MaxResponseSizeMB is not null && (this.MaxResponseSizeMB < 1 || this.MaxResponseSizeMB > MAX_RESPONSE_LENGTH_DAB_ENGINE_MB)) { throw new DataApiBuilderException( - message: $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", + message: $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= {MAX_RESPONSE_LENGTH_DAB_ENGINE_MB} MB.", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); } From 4168a3ccd126b3d078aeb0698ef9de95a14e4b86 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 12:25:37 -0700 Subject: [PATCH 03/12] Fixing comments. --- src/Config/ObjectModel/HostOptions.cs | 50 +++++++++++++++---- src/Config/ObjectModel/RuntimeConfig.cs | 6 +++ .../Unittests/ConfigValidationUnitTests.cs | 29 ++++++----- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index 4d76c90f33..21607313af 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Text.Json.Serialization; using Azure.DataApiBuilder.Service.Exceptions; @@ -10,10 +11,17 @@ namespace Azure.DataApiBuilder.Config.ObjectModel; public record HostOptions { /// - /// Dab engine can at maximum handle 187 MB of data in a single response from a source. - /// Json deserialization of a response into a string has a limit of 197020041 bytes which when converted to MB is 187 MB. + /// Dab engine can at maximum handle 158 MB of data in a single response from a source. + /// Json deserialization of a response into a string has a limit of 166,666,666 bytes which when converted to MB is 158 MB. + /// ref: enforcing code: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs#L103 + /// ref: Json constant: https://github.com/stephentoub/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs /// - private const int MAX_RESPONSE_LENGTH_DAB_ENGINE_MB = 187; + private const int MAX_RESPONSE_LENGTH_DAB_ENGINE_MB = 158; + + /// + /// Dab engine default response length. As of now this is same as max response length. + /// + private const int DEFAULT_RESPONSE_LENGTH_DAB_ENGINE_MB = 158; [JsonPropertyName("cors")] public CorsOptions? Cors { get; init; } @@ -25,7 +33,7 @@ public record HostOptions public HostMode Mode { get; init; } [JsonPropertyName("max-response-size-mb")] - public int? MaxResponseSizeMB { get; init; } + public int? MaxResponseSizeMB { get; init; } = null; public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, HostMode Mode = HostMode.Production, int? MaxResponseSizeMB = null) { @@ -34,12 +42,36 @@ public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, Hos this.Mode = Mode; this.MaxResponseSizeMB = MaxResponseSizeMB; - if (this.MaxResponseSizeMB is not null && (this.MaxResponseSizeMB < 1 || this.MaxResponseSizeMB > MAX_RESPONSE_LENGTH_DAB_ENGINE_MB)) + if (this.MaxResponseSizeMB is not null) { - throw new DataApiBuilderException( - message: $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= {MAX_RESPONSE_LENGTH_DAB_ENGINE_MB} MB.", - statusCode: HttpStatusCode.ServiceUnavailable, - subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + this.MaxResponseSizeMB = this.MaxResponseSizeMB == -1 ? MAX_RESPONSE_LENGTH_DAB_ENGINE_MB : (int)this.MaxResponseSizeMB; + if (this.MaxResponseSizeMB < 1 || this.MaxResponseSizeMB > MAX_RESPONSE_LENGTH_DAB_ENGINE_MB) + { + throw new DataApiBuilderException( + message: $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} cannot be 0, exceed {MAX_RESPONSE_LENGTH_DAB_ENGINE_MB}MB or be less than -1", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + } + + UserProvidedMaxResponseSizeMB = true; + } + else + { + this.MaxResponseSizeMB = DEFAULT_RESPONSE_LENGTH_DAB_ENGINE_MB; } } + + /// + /// Flag which informs CLI and JSON serializer whether to write MaxResponseSizeMB. + /// property and value to the runtime config file. + /// When user doesn't provide the MaxResponseSizeMBproperty/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, MaxResponseSizeMB + /// property/value specified would be interpreted by DAB as "user explicitly default-page-size." + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(MaxResponseSizeMB))] + public bool UserProvidedMaxResponseSizeMB { get; init; } = false; } diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index 340515b6f7..c09b1c05ec 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -500,6 +500,12 @@ public uint MaxPageSize() return Runtime?.Host?.MaxResponseSizeMB; } + public bool MaxResponseSizeLogicEnabled() + { + // If the user has provided a max response size, we should use new logic to enforce it. + return Runtime?.Host?.UserProvidedMaxResponseSizeMB ?? false; + } + /// /// Get the pagination limit from the runtime configuration. /// diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 59a39f8712..cceb7cef93 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -2416,25 +2416,28 @@ public void ValidatePaginationOptionsInConfig( /// /// Test to validate the max response size option in the runtime config. + /// Note:Changing the default values of max response size would be a breaking change. /// /// should there be an exception. /// maxResponse size input /// expected exception message in case there is exception. /// expected value in config. [DataTestMethod] - [DataRow(false, null, "", null, - DisplayName = "MaxDbResponseSizeMB should be null when no value provided in config.")] - [DataRow(false, 64, "", 64, - DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] - [DataRow(true, -1, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", null, - DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] - [DataRow(true, 188, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must be greater than 0 and <= 187 MB.", null, - DisplayName = "Valid inputs of MaxResponseSize must be accepted and set in the config.")] + [DataRow(null, 158, false, "", + DisplayName = $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} should be 158MB when no value provided in config.")] + [DataRow(64, 64, false, "", + DisplayName = $"Valid positive input of {nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} > 0 and <= 158MB must be accepted and set in the config.")] + [DataRow(-1, 158, false, "", + DisplayName = $"-1 user input for {nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} should result in a value of 158MB which is the max value supported by dab engine")] + [DataRow(0, null, true, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} cannot be 0, exceed 158MB or be less than -1", + DisplayName = $"Input of 0 for {nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} must throw exception.")] + [DataRow(159, null, true, $"{nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} cannot be 0, exceed 158MB or be less than -1", + DisplayName = $"Inputs of {nameof(RuntimeConfig.Runtime.Host.MaxResponseSizeMB)} greater than 158MB must throw exception.")] public void ValidateMaxResponseSizeInConfig( + int? providedMaxResponseSizeMB, + int? expectedMaxResponseSizeMB, bool exceptionExpected, - int? maxDbResponseSizeMB, - string expectedExceptionMessage, - int? expectedMaxResponseSize = null) + string expectedExceptionMessage) { try { @@ -2444,10 +2447,10 @@ public void ValidateMaxResponseSizeInConfig( Runtime: new( Rest: new(), GraphQL: new(), - Host: new(Cors: null, Authentication: null, MaxResponseSizeMB: maxDbResponseSizeMB) + Host: new(Cors: null, Authentication: null, MaxResponseSizeMB: providedMaxResponseSizeMB) ), Entities: new(new Dictionary())); - Assert.AreEqual(expectedMaxResponseSize, runtimeConfig.MaxResponseSizeMB()); + Assert.AreEqual(expectedMaxResponseSizeMB, runtimeConfig.MaxResponseSizeMB()); } catch (DataApiBuilderException ex) { From 9b1c3177ef621ddfd1bf8544f5d0a39fe2abe259 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 12:27:44 -0700 Subject: [PATCH 04/12] Nit fix on comments. --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index cceb7cef93..5f4ba2bb71 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -2436,7 +2436,7 @@ public void ValidatePaginationOptionsInConfig( public void ValidateMaxResponseSizeInConfig( int? providedMaxResponseSizeMB, int? expectedMaxResponseSizeMB, - bool exceptionExpected, + bool isExceptionExpected, string expectedExceptionMessage) { try @@ -2454,7 +2454,7 @@ public void ValidateMaxResponseSizeInConfig( } catch (DataApiBuilderException ex) { - Assert.IsTrue(exceptionExpected); + Assert.IsTrue(isExceptionExpected); Assert.AreEqual(expectedExceptionMessage, ex.Message); } } From dcabcbad6ab60343583d34c467931363059ba629 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 13:38:49 -0700 Subject: [PATCH 05/12] Fixing unit test issues. --- src/Cli.Tests/ModuleInitializer.cs | 4 ++++ src/Config/ObjectModel/HostOptions.cs | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Cli.Tests/ModuleInitializer.cs b/src/Cli.Tests/ModuleInitializer.cs index 73f31a3158..9a0be76f2e 100644 --- a/src/Cli.Tests/ModuleInitializer.cs +++ b/src/Cli.Tests/ModuleInitializer.cs @@ -55,6 +55,10 @@ public static void Init() VerifierSettings.IgnoreMember(dataSource => dataSource.DatabaseTypeNotSupportedMessage); // Ignore DefaultDataSourceName as that's not serialized in our config file. VerifierSettings.IgnoreMember(config => config.DefaultDataSourceName); + // Ignore MaxResponseSizeMB as as that's unimportant from a test standpoint. + VerifierSettings.IgnoreMember(options => options.MaxResponseSizeMB); + // Ignore UserProvidedMaxResponseSizeMB as that's not serialized in our config file. + VerifierSettings.IgnoreMember(options => options.UserProvidedMaxResponseSizeMB); // Customise the path where we store snapshots, so they are easier to locate in a PR review. VerifyBase.DerivePathInfo( (sourceFile, projectDirectory, type, method) => new( diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index 21607313af..5db6f0cf1b 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -64,12 +64,12 @@ public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, Hos /// /// Flag which informs CLI and JSON serializer whether to write MaxResponseSizeMB. /// property and value to the runtime config file. - /// When user doesn't provide the MaxResponseSizeMBproperty/value, which signals DAB to use the default, + /// When user doesn't provide the MaxResponseSizeMB 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, MaxResponseSizeMB - /// property/value specified would be interpreted by DAB as "user explicitly default-page-size." + /// property/value specified would be interpreted by DAB as "user explicitly set MaxResponseSizeMB." /// [JsonIgnore(Condition = JsonIgnoreCondition.Always)] [MemberNotNullWhen(true, nameof(MaxResponseSizeMB))] From d6b3ef8b3e6c8c00d9b0ef5149bbe0f407088564 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 14:36:49 -0700 Subject: [PATCH 06/12] MaxResponseSizeMB --- src/Service.Tests/ModuleInitializer.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Service.Tests/ModuleInitializer.cs b/src/Service.Tests/ModuleInitializer.cs index 5e718dc3f2..ace71be950 100644 --- a/src/Service.Tests/ModuleInitializer.cs +++ b/src/Service.Tests/ModuleInitializer.cs @@ -59,6 +59,10 @@ public static void Init() VerifierSettings.IgnoreMember(dataSource => dataSource.DatabaseTypeNotSupportedMessage); // Ignore DefaultDataSourceName as that's not serialized in our config file. VerifierSettings.IgnoreMember(config => config.DefaultDataSourceName); + // Ignore MaxResponseSizeMB as as that's unimportant from a test standpoint. + VerifierSettings.IgnoreMember(options => options.MaxResponseSizeMB); + // Ignore UserProvidedMaxResponseSizeMB as that's not serialized in our config file. + VerifierSettings.IgnoreMember(options => options.UserProvidedMaxResponseSizeMB); // Customise the path where we store snapshots, so they are easier to locate in a PR review. VerifyBase.DerivePathInfo( (sourceFile, projectDirectory, type, method) => new( From 7cd7bdee18ea219c280c27faf9076871cedf0dca Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 15:43:23 -0700 Subject: [PATCH 07/12] Adding convertor for host options. --- .../Converters/HostOptionsConverterFactory.cs | 64 +++++++++++++++++++ src/Config/RuntimeConfigLoader.cs | 1 + 2 files changed, 65 insertions(+) create mode 100644 src/Config/Converters/HostOptionsConverterFactory.cs diff --git a/src/Config/Converters/HostOptionsConverterFactory.cs b/src/Config/Converters/HostOptionsConverterFactory.cs new file mode 100644 index 0000000000..39d3dd6a6f --- /dev/null +++ b/src/Config/Converters/HostOptionsConverterFactory.cs @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using System.Text.Json.Serialization; +using Azure.DataApiBuilder.Config.ObjectModel; + +namespace Azure.DataApiBuilder.Config.Converters; + +/// +/// Defines how DAB reads and writes an entity's cache options (JSON). +/// +internal class HostOptionsConvertorFactory : JsonConverterFactory +{ + /// + public override bool CanConvert(Type typeToConvert) + { + return typeToConvert.IsAssignableTo(typeof(HostOptions)); + } + + /// + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) + { + return new HostOptionsConverter(); + } + + private class HostOptionsConverter : JsonConverter + { + /// + /// Defines how DAB reads host options and defines which values are + /// used to instantiate HostOptions. + /// + /// Thrown when improperly formatted cache options are provided. + public override HostOptions? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return JsonSerializer.Deserialize(ref reader, options); + } + + /// + /// When writing the HostOptions back to a JSON file, only write the MaxResponseSizeMB property + /// if the property is user provided. This avoids polluting the written JSON file with a property + /// the user most likely ommitted when writing the original DAB runtime config file. + /// This Write operation is only used when a RuntimeConfig object is serialized to JSON. + /// + public override void Write(Utf8JsonWriter writer, HostOptions value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WritePropertyName("cors"); + JsonSerializer.Serialize(writer, value.Cors, options); + writer.WritePropertyName("authentication"); + JsonSerializer.Serialize(writer, value.Authentication, options); + writer.WritePropertyName("mode"); + JsonSerializer.Serialize(writer, value.Mode, options); + + if (value?.UserProvidedMaxResponseSizeMB is true) + { + writer.WritePropertyName("max-response-size-mb"); + JsonSerializer.Serialize(writer, value.MaxResponseSizeMB, options); + } + + writer.WriteEndObject(); + } + } +} diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 2cc823a6bb..c3d15c5351 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -181,6 +181,7 @@ public static JsonSerializerOptions GetSerializationOptions( options.Converters.Add(new MultipleCreateOptionsConverter()); options.Converters.Add(new MultipleMutationOptionsConverter(options)); options.Converters.Add(new DataSourceConverterFactory(replaceEnvVar)); + options.Converters.Add(new HostOptionsConvertorFactory()); if (replaceEnvVar) { From 7134699e4ed08dce7e5904228b5862fd0f5f1e39 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 15:45:15 -0700 Subject: [PATCH 08/12] nit fixes. --- src/Config/Converters/HostOptionsConverterFactory.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Config/Converters/HostOptionsConverterFactory.cs b/src/Config/Converters/HostOptionsConverterFactory.cs index 39d3dd6a6f..d02a1b64f0 100644 --- a/src/Config/Converters/HostOptionsConverterFactory.cs +++ b/src/Config/Converters/HostOptionsConverterFactory.cs @@ -8,7 +8,7 @@ namespace Azure.DataApiBuilder.Config.Converters; /// -/// Defines how DAB reads and writes an entity's cache options (JSON). +/// Defines how DAB reads and writes host options. /// internal class HostOptionsConvertorFactory : JsonConverterFactory { @@ -29,8 +29,9 @@ private class HostOptionsConverter : JsonConverter /// /// Defines how DAB reads host options and defines which values are /// used to instantiate HostOptions. + /// Uses default deserialize. /// - /// Thrown when improperly formatted cache options are provided. + /// Thrown when improperly formatted host options are provided. public override HostOptions? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { return JsonSerializer.Deserialize(ref reader, options); From 68abba873813adca0204702cc04d39343d501de9 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 15:54:05 -0700 Subject: [PATCH 09/12] Fix recursing code. --- src/Config/Converters/HostOptionsConverterFactory.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Config/Converters/HostOptionsConverterFactory.cs b/src/Config/Converters/HostOptionsConverterFactory.cs index d02a1b64f0..aed131493f 100644 --- a/src/Config/Converters/HostOptionsConverterFactory.cs +++ b/src/Config/Converters/HostOptionsConverterFactory.cs @@ -34,6 +34,9 @@ private class HostOptionsConverter : JsonConverter /// Thrown when improperly formatted host options are provided. public override HostOptions? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { + // Remove the converter so we don't recurse. + JsonSerializerOptions innerOptions = new(options); + innerOptions.Converters.Remove(innerOptions.Converters.First(c => c is HostOptionsConvertorFactory)); return JsonSerializer.Deserialize(ref reader, options); } From aed7642bc5da2bf22cb7fc3dc7055b5a4cf8d267 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Tue, 4 Jun 2024 16:06:21 -0700 Subject: [PATCH 10/12] Fix recursing code. --- src/Config/Converters/HostOptionsConverterFactory.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Config/Converters/HostOptionsConverterFactory.cs b/src/Config/Converters/HostOptionsConverterFactory.cs index aed131493f..cf39b871fb 100644 --- a/src/Config/Converters/HostOptionsConverterFactory.cs +++ b/src/Config/Converters/HostOptionsConverterFactory.cs @@ -35,9 +35,9 @@ private class HostOptionsConverter : JsonConverter public override HostOptions? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { // Remove the converter so we don't recurse. - JsonSerializerOptions innerOptions = new(options); - innerOptions.Converters.Remove(innerOptions.Converters.First(c => c is HostOptionsConvertorFactory)); - return JsonSerializer.Deserialize(ref reader, options); + JsonSerializerOptions jsonSerializerOptions = new(options); + jsonSerializerOptions.Converters.Remove(jsonSerializerOptions.Converters.First(c => c is HostOptionsConvertorFactory)); + return JsonSerializer.Deserialize(ref reader, jsonSerializerOptions); } /// From c6aca6e096aafc633e98f8c649b13c0e615cbbc9 Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Wed, 5 Jun 2024 12:38:15 -0700 Subject: [PATCH 11/12] Addressing comments. --- src/Config/ObjectModel/HostOptions.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index 5db6f0cf1b..d37e67e147 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -64,12 +64,13 @@ public HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, Hos /// /// Flag which informs CLI and JSON serializer whether to write MaxResponseSizeMB. /// property and value to the runtime config file. - /// When user doesn't provide the MaxResponseSizeMB property/value, which signals DAB to use the default, - /// the DAB CLI should not write the default value to a serialized config. + /// When user doesn't provide the MaxResponseSizeMB property/value or provides a null value, which signals DAB to use the default, + /// the DAB CLI will 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, MaxResponseSizeMB - /// property/value specified would be interpreted by DAB as "user explicitly set MaxResponseSizeMB." + /// property/value specified would be interpreted by DAB as "user explicitly set MaxResponseSizeMB. + /// UserProvidedMaxResponseSizeMB is true only when a user provides a non-null value /// [JsonIgnore(Condition = JsonIgnoreCondition.Always)] [MemberNotNullWhen(true, nameof(MaxResponseSizeMB))] From 40e77c38c4cee806342173feaae9f7810705943e Mon Sep 17 00:00:00 2001 From: Rohan Khanna Date: Wed, 5 Jun 2024 15:08:29 -0700 Subject: [PATCH 12/12] fixing nit comments. --- src/Config/ObjectModel/HostOptions.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Config/ObjectModel/HostOptions.cs b/src/Config/ObjectModel/HostOptions.cs index d37e67e147..452c78ef17 100644 --- a/src/Config/ObjectModel/HostOptions.cs +++ b/src/Config/ObjectModel/HostOptions.cs @@ -13,8 +13,10 @@ public record HostOptions /// /// Dab engine can at maximum handle 158 MB of data in a single response from a source. /// Json deserialization of a response into a string has a limit of 166,666,666 bytes which when converted to MB is 158 MB. - /// ref: enforcing code: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs#L103 - /// ref: Json constant: https://github.com/stephentoub/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs + /// ref: enforcing code: + /// .net8: https://github.com/dotnet/runtime/blob/v6.0.0/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs#L80 + /// .net6: https://github.com/dotnet/runtime/blob/v8.0.0/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs#75 + /// ref: Json constant: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L80 /// private const int MAX_RESPONSE_LENGTH_DAB_ENGINE_MB = 158;