Skip to content

Commit 1fa13ce

Browse files
committed
[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 f8373b8 commit 1fa13ce

11 files changed

+155
-64
lines changed

schemas/dab.draft.schema.json

+1-1
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

+2-7
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ public static Process ExecuteDabCommand(string command, string flags)
870870
""graphql"": {
871871
""path"": ""/graphql"",
872872
""enabled"": true,
873-
""allow-introspection"": true
873+
""allow-introspection"": false
874874
},
875875
""host"": {
876876
""mode"": ""production"",
@@ -917,11 +917,6 @@ public static Process ExecuteDabCommand(string command, string flags)
917917
""connection-string"": ""localhost:5000;User ID={USER_NAME};Password={USER_PASSWORD};MultipleActiveResultSets=False;""
918918
},
919919
""runtime"": {
920-
""graphql"": {
921-
""path"": ""/graphql"",
922-
""enabled"": true,
923-
""allow-introspection"": true
924-
},
925920
""host"": {
926921
""mode"": ""production"",
927922
""cors"": {
@@ -990,7 +985,7 @@ public static Process ExecuteDabCommand(string command, string flags)
990985
""graphql"": {
991986
""path"": ""/graphql"",
992987
""enabled"": true,
993-
""allow-introspection"": true
988+
""allow-introspection"": false
994989
},
995990
""host"": {
996991
""mode"": ""production"",

src/Cli/Utils.cs

-44
Original file line numberDiff line numberDiff line change
@@ -203,50 +203,6 @@ public static bool IsURIComponentValid(string? uriComponent)
203203
return !DoesUriComponentContainReservedChars(uriComponent);
204204
}
205205

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

src/Config/ObjectModel/AuthenticationOptions.cs

+1-1
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

+15-1
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

+46-1
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

+9-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ public record RuntimeConfig
1313
[JsonPropertyName("$schema")]
1414
public string Schema { get; init; }
1515

16+
public const string DEFAULT_CONFIG_SCHEMA_LINK = "https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json";
17+
1618
public DataSource DataSource { get; init; }
1719

1820
public RuntimeOptions Runtime { get; init; }
@@ -50,11 +52,15 @@ public IEnumerable<KeyValuePair<string, DataSource>> GetDataSourceNamesToDataSou
5052
/// <param name="Runtime">Runtime settings.</param>
5153
/// <param name="Entities">Entities</param>
5254
[JsonConstructor]
53-
public RuntimeConfig(string Schema, DataSource DataSource, RuntimeOptions Runtime, RuntimeEntities Entities)
55+
public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Entities, RuntimeOptions? Runtime = null)
5456
{
55-
this.Schema = Schema;
57+
this.Schema = Schema ?? DEFAULT_CONFIG_SCHEMA_LINK;
5658
this.DataSource = DataSource;
57-
this.Runtime = Runtime;
59+
this.Runtime = Runtime ??
60+
new RuntimeOptions(
61+
new RestRuntimeOptions(Enabled: DataSource.DatabaseType != DatabaseType.CosmosDB_NoSQL),
62+
GraphQL: null, // even though we pass null here, the constructor will take care of initializing with defaults.
63+
Host: null);
5864
this.Entities = Entities;
5965
this._dataSourceNameToDataSource = new Dictionary<string, DataSource>();
6066
this._defaultDataSourceName = Guid.NewGuid().ToString();
+22-1
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

+2-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ public static JsonSerializerOptions GetSerializationOptions(bool replaceEnvVar =
155155
PropertyNamingPolicy = new HyphenatedNamingPolicy(),
156156
ReadCommentHandling = JsonCommentHandling.Skip,
157157
WriteIndented = true,
158-
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
158+
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
159+
IncludeFields = true
159160
};
160161
options.Converters.Add(new EnumMemberJsonEnumConverterFactory());
161162
options.Converters.Add(new RestRuntimeOptionsConverterFactory());

src/Service.Tests/Configuration/ConfigurationTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2270,8 +2270,8 @@ private static JsonContent GetPostStartupConfigParams(string environment, Runtim
22702270
RuntimeConfig overrides = new(
22712271
Schema: null,
22722272
DataSource: new DataSource(DatabaseType.MSSQL, connectionString, new()),
2273-
Runtime: null,
2274-
Entities: new(new Dictionary<string, Entity>()));
2273+
Entities: new(new Dictionary<string, Entity>()),
2274+
Runtime: null);
22752275

22762276
ConfigurationPostParametersV2 returnParams = new(
22772277
Configuration: serializedConfiguration,

src/Service.Tests/Unittests/RuntimeConfigLoaderJsonDeserializerTests.cs

+55-2
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
@@ -127,6 +127,34 @@ public void CheckCommentParsingInConfigFile()
127127
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(actualJson, out RuntimeConfig _), "Should not fail to parse with comments");
128128
}
129129

130+
/// <summary>
131+
/// Method to validate that comments are skipped in config file (and are ignored during deserialization).
132+
/// </summary>
133+
[TestMethod]
134+
public void TestDefaultsCreatedForOptionalProps()
135+
{
136+
// Test with no runtime property
137+
StringBuilder minJson = new(@"
138+
""data-source"": {
139+
""database-type"": ""mssql"",
140+
""connection-string"": ""@env('test-connection-string')""
141+
},
142+
""entities"": { }");
143+
TryParseAndAssertOnDefaults("{" + minJson + "}");
144+
145+
// Test with an empty runtime property
146+
minJson.Append(@", ""runtime"": ");
147+
TryParseAndAssertOnDefaults("{" + minJson + "{ }}");
148+
149+
// Test with empty rest, graphql, host properties
150+
minJson.Append(@"{ ""rest"": { }, ""graphql"": { }, ""host"" : ");
151+
TryParseAndAssertOnDefaults("{" + minJson + "{ } } }", isHostSpecifiedButEmpty: true);
152+
153+
// Test with empty rest, graphql, and empty host sub-properties
154+
minJson.Append(@"{ ""cors"": { }, ""authentication"": { } } }");
155+
TryParseAndAssertOnDefaults("{" + minJson + "}", isHostSpecifiedButEmpty: false);
156+
}
157+
130158
#endregion Positive Tests
131159

132160
#region Negative Tests
@@ -301,6 +329,31 @@ public static string GetModifiedJsonString(string[] reps, string enumString)
301329
";
302330
}
303331

332+
private static void TryParseAndAssertOnDefaults(string json, bool isHostSpecifiedButEmpty = false)
333+
{
334+
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(json, out RuntimeConfig parsedConfig));
335+
Assert.AreEqual(RuntimeConfig.DEFAULT_CONFIG_SCHEMA_LINK, parsedConfig.Schema);
336+
Assert.IsTrue(parsedConfig.Runtime.Rest.Enabled);
337+
Assert.AreEqual(RestRuntimeOptions.DEFAULT_PATH, parsedConfig.Runtime.Rest.Path);
338+
Assert.IsTrue(parsedConfig.Runtime.GraphQL.Enabled);
339+
Assert.AreEqual(GraphQLRuntimeOptions.DEFAULT_PATH, parsedConfig.Runtime.GraphQL.Path);
340+
Assert.IsTrue(parsedConfig.Runtime.GraphQL.AllowIntrospection);
341+
Assert.IsNull(parsedConfig.Runtime.BaseRoute);
342+
Assert.AreEqual(HostMode.Development, parsedConfig.Runtime.Host.Mode);
343+
if (isHostSpecifiedButEmpty)
344+
{
345+
Assert.IsNull(parsedConfig.Runtime.Host.Cors);
346+
Assert.IsNull(parsedConfig.Runtime.Host.Authentication);
347+
}
348+
else
349+
{
350+
Assert.AreEqual(0, parsedConfig.Runtime.Host.Cors.Origins.Length);
351+
Assert.IsFalse(parsedConfig.Runtime.Host.Cors.AllowCredentials);
352+
Assert.AreEqual(EasyAuthType.StaticWebApps.ToString(), parsedConfig.Runtime.Host.Authentication.Provider);
353+
Assert.IsNull(parsedConfig.Runtime.Host.Authentication.Jwt);
354+
}
355+
}
356+
304357
#endregion Helper Functions
305358

306359
record StubJsonType(string Foo);

0 commit comments

Comments
 (0)