Skip to content

Commit 64b2ab9

Browse files
authored
[Configuration] Initialize optional properties with defaults (#1684)
## Why make this change? - As per the JSON config schema, not all properties in the config file are mandatory. - With #1402, in versions 0.8.49+ those properties that are not explicitly set, are initialized with defaults before writing to the file system. - However, config files generated using version 0.7.6 or lesser, may not have these properties initialized to their defaults when writing to the file system. Using config files generated with <= 0.7.6, to start engine with 0.8.49 would therefore lead to null reference exceptions when the optional properties are dereferenced. - This PR is to ensure such null reference exceptions are avoided when starting engine with configs that don't have optional properties ## What is this change? - Modify object model constructors to accept nullable arguments for optional properties - represented as fields in the record types. If passed in argument is null, initialize the field with default values. Do this recursively for sub-properties. - To force JSON deserializer to pick up the custom constructor, add `IncludeFields = true` to `JsonSerializerOptions` - Default value of host mode is changed to `development` since `allow-introspection` has a default value of `true` ## How was this tested? - Removed optional properties from config and ensured engine could be started without them. - [X] Unit Tests ## Sample Request(s) - remove `runtime` section from config, starting engine with 0.8.49 leads to NullReferenceException. - After the fix, starting engine is successful even when `runtime` section is absent in the config.
1 parent b77e6f5 commit 64b2ab9

11 files changed

+173
-91
lines changed

schemas/dab.draft.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@
161161
"mode": {
162162
"description": "Set if running in Development or Production mode",
163163
"type": "string",
164-
"default": "production",
164+
"default": "development",
165165
"enum": [
166166
"production",
167167
"development"

src/Cli.Tests/TestHelper.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ public static Process ExecuteDabCommand(string command, string flags)
868868
""graphql"": {
869869
""path"": ""/graphql"",
870870
""enabled"": true,
871-
""allow-introspection"": true
871+
""allow-introspection"": false
872872
},
873873
""host"": {
874874
""mode"": ""production"",
@@ -915,11 +915,6 @@ public static Process ExecuteDabCommand(string command, string flags)
915915
""connection-string"": ""localhost:5000;User ID={USER_NAME};Password={USER_PASSWORD};MultipleActiveResultSets=False;""
916916
},
917917
""runtime"": {
918-
""graphql"": {
919-
""path"": ""/graphql"",
920-
""enabled"": true,
921-
""allow-introspection"": true
922-
},
923918
""host"": {
924919
""mode"": ""production"",
925920
""cors"": {
@@ -988,7 +983,7 @@ public static Process ExecuteDabCommand(string command, string flags)
988983
""graphql"": {
989984
""path"": ""/graphql"",
990985
""enabled"": true,
991-
""allow-introspection"": true
986+
""allow-introspection"": false
992987
},
993988
""host"": {
994989
""mode"": ""production"",

src/Cli/Utils.cs

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -204,50 +204,6 @@ public static bool IsURIComponentValid(string? uriComponent)
204204
return !DoesUriComponentContainReservedChars(uriComponent);
205205
}
206206

207-
/// <summary>
208-
/// Returns the default host Global Settings
209-
/// If the user doesn't specify host mode. Default value to be used is Production.
210-
/// Sample:
211-
// "host": {
212-
// "mode": "production",
213-
// "cors": {
214-
// "origins": [],
215-
// "allow-credentials": true
216-
// },
217-
// "authentication": {
218-
// "provider": "StaticWebApps"
219-
// }
220-
// }
221-
/// </summary>
222-
public static HostOptions GetDefaultHostOptions(
223-
HostMode hostMode,
224-
IEnumerable<string>? corsOrigin,
225-
string authenticationProvider,
226-
string? audience,
227-
string? issuer)
228-
{
229-
string[]? corsOriginArray = corsOrigin is null ? new string[] { } : corsOrigin.ToArray();
230-
CorsOptions cors = new(Origins: corsOriginArray);
231-
AuthenticationOptions AuthenticationOptions;
232-
if (Enum.TryParse<EasyAuthType>(authenticationProvider, ignoreCase: true, out _)
233-
|| AuthenticationOptions.SIMULATOR_AUTHENTICATION.Equals(authenticationProvider))
234-
{
235-
AuthenticationOptions = new(Provider: authenticationProvider, null);
236-
}
237-
else
238-
{
239-
AuthenticationOptions = new(
240-
Provider: authenticationProvider,
241-
Jwt: new(audience, issuer)
242-
);
243-
}
244-
245-
return new(
246-
Mode: hostMode,
247-
Cors: cors,
248-
Authentication: AuthenticationOptions);
249-
}
250-
251207
/// <summary>
252208
/// Returns an object of type Policy
253209
/// If policyRequest or policyDatabase is provided. Otherwise, returns null.

src/Config/ObjectModel/AuthenticationOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace Azure.DataApiBuilder.Config.ObjectModel;
1111
/// </param>
1212
/// <param name="Jwt">Settings enabling validation of the received JWT token.
1313
/// Required only when Provider is other than EasyAuth.</param>
14-
public record AuthenticationOptions(string Provider, JwtOptions? Jwt)
14+
public record AuthenticationOptions(string Provider = nameof(EasyAuthType.StaticWebApps), JwtOptions? Jwt = null)
1515
{
1616
public const string SIMULATOR_AUTHENTICATION = "Simulator";
1717
public const string CLIENT_PRINCIPAL_HEADER = "X-MS-CLIENT-PRINCIPAL";

src/Config/ObjectModel/CorsOptions.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System.Text.Json.Serialization;
5+
46
namespace Azure.DataApiBuilder.Config.ObjectModel;
57

68
/// <summary>
@@ -9,4 +11,16 @@ namespace Azure.DataApiBuilder.Config.ObjectModel;
911
/// <param name="Origins">List of allowed origins.</param>
1012
/// <param name="AllowCredentials">
1113
/// Whether to set Access-Control-Allow-Credentials CORS header.</param>
12-
public record CorsOptions(string[] Origins, bool AllowCredentials = false);
14+
public record CorsOptions
15+
{
16+
public string[] Origins;
17+
18+
public bool AllowCredentials;
19+
20+
[JsonConstructor]
21+
public CorsOptions(string[]? Origins, bool AllowCredentials = false)
22+
{
23+
this.Origins = Origins is null ? new string[] { } : Origins.ToArray();
24+
this.AllowCredentials = AllowCredentials;
25+
}
26+
}

src/Config/ObjectModel/HostOptions.cs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,49 @@
33

44
namespace Azure.DataApiBuilder.Config.ObjectModel;
55

6-
public record HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, HostMode Mode = HostMode.Development);
6+
public record HostOptions(CorsOptions? Cors, AuthenticationOptions? Authentication, HostMode Mode = HostMode.Development)
7+
{
8+
/// <summary>
9+
/// Returns the default host Global Settings
10+
/// If the user doesn't specify host mode. Default value to be used is Development.
11+
/// Sample:
12+
// "host": {
13+
// "mode": "development",
14+
// "cors": {
15+
// "origins": [],
16+
// "allow-credentials": true
17+
// },
18+
// "authentication": {
19+
// "provider": "StaticWebApps"
20+
// }
21+
// }
22+
/// </summary>
23+
public static HostOptions GetDefaultHostOptions(
24+
HostMode hostMode,
25+
IEnumerable<string>? corsOrigin,
26+
string authenticationProvider,
27+
string? audience,
28+
string? issuer)
29+
{
30+
string[]? corsOriginArray = corsOrigin is null ? new string[] { } : corsOrigin.ToArray();
31+
CorsOptions cors = new(Origins: corsOriginArray);
32+
AuthenticationOptions AuthenticationOptions;
33+
if (Enum.TryParse<EasyAuthType>(authenticationProvider, ignoreCase: true, out _)
34+
|| AuthenticationOptions.SIMULATOR_AUTHENTICATION.Equals(authenticationProvider))
35+
{
36+
AuthenticationOptions = new(Provider: authenticationProvider, Jwt: null);
37+
}
38+
else
39+
{
40+
AuthenticationOptions = new(
41+
Provider: authenticationProvider,
42+
Jwt: new(audience, issuer)
43+
);
44+
}
45+
46+
return new(
47+
Mode: hostMode,
48+
Cors: cors,
49+
Authentication: AuthenticationOptions);
50+
}
51+
}

src/Config/ObjectModel/RuntimeConfig.cs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,34 @@
66

77
namespace Azure.DataApiBuilder.Config.ObjectModel;
88

9-
public record RuntimeConfig(
10-
[property: JsonPropertyName("$schema")] string Schema,
11-
DataSource DataSource,
12-
RuntimeOptions Runtime,
13-
RuntimeEntities Entities)
9+
public record RuntimeConfig
1410
{
11+
[JsonPropertyName("$schema")]
12+
public string Schema;
13+
14+
public const string DEFAULT_CONFIG_SCHEMA_LINK = "https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json";
15+
16+
public DataSource DataSource;
17+
18+
public RuntimeOptions Runtime;
19+
20+
public RuntimeEntities Entities;
21+
22+
[JsonConstructor]
23+
public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Entities, RuntimeOptions? Runtime = null)
24+
{
25+
this.Schema = Schema ?? DEFAULT_CONFIG_SCHEMA_LINK;
26+
27+
this.DataSource = DataSource;
28+
this.Runtime = Runtime ??
29+
new RuntimeOptions(
30+
new RestRuntimeOptions(Enabled: DataSource.DatabaseType != DatabaseType.CosmosDB_NoSQL),
31+
GraphQL: null, // even though we pass null here, the constructor will take care of initializing with defaults.
32+
Host: null);
33+
34+
this.Entities = Entities;
35+
}
36+
1537
/// <summary>
1638
/// Serializes the RuntimeConfig object to JSON for writing to file.
1739
/// </summary>
Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,27 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System.Text.Json.Serialization;
5+
46
namespace Azure.DataApiBuilder.Config.ObjectModel;
57

6-
public record RuntimeOptions(RestRuntimeOptions Rest, GraphQLRuntimeOptions GraphQL, HostOptions Host, string? BaseRoute = null);
8+
public record RuntimeOptions
9+
{
10+
public RestRuntimeOptions Rest;
11+
public GraphQLRuntimeOptions GraphQL;
12+
public HostOptions Host;
13+
public string? BaseRoute;
14+
15+
[JsonConstructor]
16+
public RuntimeOptions(RestRuntimeOptions? Rest, GraphQLRuntimeOptions? GraphQL, HostOptions? Host, string? BaseRoute = null)
17+
{
18+
this.Rest = Rest ?? new RestRuntimeOptions();
19+
this.GraphQL = GraphQL ?? new GraphQLRuntimeOptions();
20+
this.Host = Host ?? HostOptions.GetDefaultHostOptions(
21+
HostMode.Development,
22+
corsOrigin: null,
23+
EasyAuthType.StaticWebApps.ToString(),
24+
audience: null, issuer: null);
25+
this.BaseRoute = BaseRoute;
26+
}
27+
}

src/Config/RuntimeConfigLoader.cs

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -64,32 +64,6 @@ public static bool TryParseConfig(string json,
6464
{
6565
config = config with { DataSource = config.DataSource with { ConnectionString = connectionString } };
6666
}
67-
68-
// For Cosmos DB NoSQL database type, DAB CLI v0.8.49+ generates a REST property within the Runtime section of the config file. However
69-
// v0.7.6- does not generate this property. So, when the config file generated using v0.7.6- is used to start the engine with v0.8.49+, the absence
70-
// of the REST property causes the engine to throw exceptions. This is the only difference in the way Runtime section of the config file is created
71-
// between these two versions.
72-
// To avoid the NullReference Exceptions, the REST property is added when absent in the config file.
73-
// Other properties within the Runtime section are also populated with default values to account for the cases where
74-
// the properties could be removed manually from the config file.
75-
if (config.Runtime is not null)
76-
{
77-
if (config.Runtime.Rest is null)
78-
{
79-
config = config with { Runtime = config.Runtime with { Rest = (config.DataSource.DatabaseType is DatabaseType.CosmosDB_NoSQL) ? new RestRuntimeOptions(Enabled: false) : new RestRuntimeOptions(Enabled: false) } };
80-
}
81-
82-
if (config.Runtime.GraphQL is null)
83-
{
84-
config = config with { Runtime = config.Runtime with { GraphQL = new GraphQLRuntimeOptions(AllowIntrospection: false) } };
85-
}
86-
87-
if (config.Runtime.Host is null)
88-
{
89-
config = config with { Runtime = config.Runtime with { Host = new HostOptions(Cors: null, Authentication: new AuthenticationOptions(Provider: EasyAuthType.StaticWebApps.ToString(), Jwt: null), Mode: HostMode.Production) } };
90-
}
91-
}
92-
9367
}
9468
catch (JsonException ex)
9569
{
@@ -127,7 +101,8 @@ public static JsonSerializerOptions GetSerializationOptions(bool replaceEnvVar =
127101
PropertyNamingPolicy = new HyphenatedNamingPolicy(),
128102
ReadCommentHandling = JsonCommentHandling.Skip,
129103
WriteIndented = true,
130-
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
104+
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
105+
IncludeFields = true
131106
};
132107
options.Converters.Add(new EnumMemberJsonEnumConverterFactory());
133108
options.Converters.Add(new RestRuntimeOptionsConverterFactory());

src/Service.Tests/Configuration/ConfigurationTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1994,7 +1994,8 @@ private static JsonContent GetPostStartupConfigParams(string environment, Runtim
19941994
}
19951995
else if (configurationEndpoint == CONFIGURATION_ENDPOINT_V2)
19961996
{
1997-
RuntimeConfig overrides = new(null, new DataSource(DatabaseType.MSSQL, connectionString, new()), null, null);
1997+
RuntimeConfig overrides = new(null, new DataSource(DatabaseType.MSSQL, connectionString, new()),
1998+
Entities: null, runtimeConfig.Runtime);
19981999

19992000
ConfigurationPostParametersV2 returnParams = new(
20002001
Configuration: serializedConfiguration,

src/Service.Tests/Unittests/RuntimeConfigLoaderJsonDeserializerTests.cs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Data;
66
using System.IO;
77
using System.IO.Abstractions.TestingHelpers;
8+
using System.Text;
89
using System.Text.Json;
910
using Azure.DataApiBuilder.Config;
1011
using Azure.DataApiBuilder.Config.Converters;
@@ -15,8 +16,7 @@
1516
namespace Azure.DataApiBuilder.Service.Tests.UnitTests
1617
{
1718
/// <summary>
18-
/// Unit tests for the environment variable
19-
/// parser for the runtime configuration. These
19+
/// Unit tests for deserializing the runtime configuration. These
2020
/// tests verify that we parse the config correctly
2121
/// when replacing environment variables. Also verify
2222
/// we throw the right exception when environment
@@ -114,6 +114,34 @@ public void CheckCommentParsingInConfigFile()
114114
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(actualJson, out RuntimeConfig _), "Should not fail to parse with comments");
115115
}
116116

117+
/// <summary>
118+
/// Method to validate that comments are skipped in config file (and are ignored during deserialization).
119+
/// </summary>
120+
[TestMethod]
121+
public void TestDefaultsCreatedForOptionalProps()
122+
{
123+
// Test with no runtime property
124+
StringBuilder minJson = new(@"
125+
""data-source"": {
126+
""database-type"": ""mssql"",
127+
""connection-string"": ""@env('test-connection-string')""
128+
},
129+
""entities"": { }");
130+
TryParseAndAssertOnDefaults("{" + minJson + "}");
131+
132+
// Test with an empty runtime property
133+
minJson.Append(@", ""runtime"": ");
134+
TryParseAndAssertOnDefaults("{" + minJson + "{ }}");
135+
136+
// Test with empty rest, graphql, host properties
137+
minJson.Append(@"{ ""rest"": { }, ""graphql"": { }, ""host"" : ");
138+
TryParseAndAssertOnDefaults("{" + minJson + "{ } } }", isHostSpecifiedButEmpty: true);
139+
140+
// Test with empty rest, graphql, and empty host sub-properties
141+
minJson.Append(@"{ ""cors"": { }, ""authentication"": { } } }");
142+
TryParseAndAssertOnDefaults("{" + minJson + "}", isHostSpecifiedButEmpty: false);
143+
}
144+
117145
#endregion Positive Tests
118146

119147
#region Negative Tests
@@ -288,6 +316,31 @@ public static string GetModifiedJsonString(string[] reps, string enumString)
288316
";
289317
}
290318

319+
private static void TryParseAndAssertOnDefaults(string json, bool isHostSpecifiedButEmpty = false)
320+
{
321+
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(json, out RuntimeConfig parsedConfig));
322+
Assert.AreEqual(RuntimeConfig.DEFAULT_CONFIG_SCHEMA_LINK, parsedConfig.Schema);
323+
Assert.IsTrue(parsedConfig.Runtime.Rest.Enabled);
324+
Assert.AreEqual(RestRuntimeOptions.DEFAULT_PATH, parsedConfig.Runtime.Rest.Path);
325+
Assert.IsTrue(parsedConfig.Runtime.GraphQL.Enabled);
326+
Assert.AreEqual(GraphQLRuntimeOptions.DEFAULT_PATH, parsedConfig.Runtime.GraphQL.Path);
327+
Assert.IsTrue(parsedConfig.Runtime.GraphQL.AllowIntrospection);
328+
Assert.IsNull(parsedConfig.Runtime.BaseRoute);
329+
Assert.AreEqual(HostMode.Development, parsedConfig.Runtime.Host.Mode);
330+
if (isHostSpecifiedButEmpty)
331+
{
332+
Assert.IsNull(parsedConfig.Runtime.Host.Cors);
333+
Assert.IsNull(parsedConfig.Runtime.Host.Authentication);
334+
}
335+
else
336+
{
337+
Assert.AreEqual(0, parsedConfig.Runtime.Host.Cors.Origins.Length);
338+
Assert.IsFalse(parsedConfig.Runtime.Host.Cors.AllowCredentials);
339+
Assert.AreEqual(EasyAuthType.StaticWebApps.ToString(), parsedConfig.Runtime.Host.Authentication.Provider);
340+
Assert.IsNull(parsedConfig.Runtime.Host.Authentication.Jwt);
341+
}
342+
}
343+
291344
#endregion Helper Functions
292345

293346
record StubJsonType(string Foo);

0 commit comments

Comments
 (0)