Skip to content

Commit 9025bb3

Browse files
simonsabinCopilotJerryNixonAniruddh25
authored
#3053 Fix by using custom bool parsing that if a string is found then… (#3054)
… it uses the environment replacement and looks for a true false 1 or 0. ## Why make this change? Fixes #3053 Boolean values can't be set using environment variables. This allows that ## What is this change? Using custom JsonConverter for bools that if a string is detected it uses the string serialiser that uses the environment replacement rules. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Jerry Nixon <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]>
1 parent 3cf09f9 commit 9025bb3

8 files changed

Lines changed: 325 additions & 18 deletions

File tree

‎.gitattributes‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,6 @@
1515
*.verified.txt text eol=lf working-tree-encoding=UTF-8
1616
*.verified.xml text eol=lf working-tree-encoding=UTF-8
1717
*.verified.json text eol=lf working-tree-encoding=UTF-8
18+
19+
# This can't be enabled as there are tests that rely on the CRLF endings in files'
20+
#*.cs text eol=lf

‎CONTRIBUTING.md‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ We use `dotnet format` to enforce code conventions. It is run automatically in C
9595

9696
#### Enforcing code style with git hooks
9797

98-
You can copy paste the following commands to install a git pre-commit hook. This will cause a commit to fail if you forgot to run `dotnet format`. If you have run on save enabled in your editor this is not necessary.
98+
You can copy paste the following commands to install a git pre-commit hook (creates a pre-commit file in your .git folder, which isn't shown in vs code). This will cause a commit to fail if you forgot to run `dotnet format`. If you have run on save enabled in your editor this is not necessary.
9999

100100
```bash
101101
cat > .git/hooks/pre-commit << __EOF__
@@ -112,17 +112,42 @@ if [ "\$(get_files)" = '' ]; then
112112
fi
113113
114114
get_files |
115-
xargs dotnet format src/Azure.DataApiBuilder.Service.sln \\
116-
--check \\
117-
--fix-whitespace --fix-style warn --fix-analyzers warn \\
115+
xargs dotnet format src/Azure.DataApiBuilder.sln \\
116+
--verify-no-changes
118117
--include \\
119118
|| {
120119
get_files |
121-
xargs dotnet format src/Azure.DataApiBuilder.Service.sln \\
122-
--fix-whitespace --fix-style warn --fix-analyzers warn \\
120+
xargs dotnet format src/Azure.DataApiBuilder.sln \\
123121
--include
124122
exit 1
125123
}
126124
__EOF__
127125
chmod +x .git/hooks/pre-commit
128126
```
127+
128+
The file should look like this
129+
130+
``` bash
131+
#!/bin/bash
132+
set -euo pipefail
133+
134+
get_files() {
135+
git diff --cached --name-only --diff-filter=ACMR | \
136+
grep '\.cs$'
137+
}
138+
139+
if [ "$(get_files)" = '' ]; then
140+
exit 0
141+
fi
142+
143+
get_files |
144+
xargs dotnet format src/Azure.DataApiBuilder.sln \
145+
--verify-no-changes \
146+
--include \
147+
|| {
148+
get_files |
149+
xargs dotnet format src/Azure.DataApiBuilder.sln \
150+
--include
151+
exit 1
152+
}
153+
```

‎schemas/dab.draft.schema.json‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
"additionalProperties": false,
4242
"properties": {
4343
"enabled": {
44-
"type": "boolean",
45-
"description": "Enable health check endpoint",
44+
"$ref": "#/$defs/boolean-or-string",
45+
"description": "Enable health check endpoint for something",
4646
"default": true,
4747
"additionalProperties": false
4848
},
@@ -186,7 +186,7 @@
186186
"type": "string"
187187
},
188188
"enabled": {
189-
"type": "boolean",
189+
"$ref": "#/$defs/boolean-or-string",
190190
"description": "Allow enabling/disabling REST requests for all entities."
191191
},
192192
"request-body-strict": {
@@ -210,7 +210,7 @@
210210
"type": "string"
211211
},
212212
"enabled": {
213-
"type": "boolean",
213+
"$ref": "#/$defs/boolean-or-string",
214214
"description": "Allow enabling/disabling GraphQL requests for all entities."
215215
},
216216
"depth-limit": {
@@ -438,7 +438,7 @@
438438
"description": "Application Insights connection string"
439439
},
440440
"enabled": {
441-
"type": "boolean",
441+
"$ref": "#/$defs/boolean-or-string",
442442
"description": "Allow enabling/disabling Application Insights telemetry.",
443443
"default": true
444444
}
@@ -481,7 +481,7 @@
481481
"additionalProperties": false,
482482
"properties": {
483483
"enabled": {
484-
"type": "boolean",
484+
"$ref": "#/$defs/boolean-or-string",
485485
"description": "Allow enabling/disabling Azure Log Analytics.",
486486
"default": false
487487
},
@@ -618,7 +618,7 @@
618618
"additionalProperties": false,
619619
"properties": {
620620
"enabled": {
621-
"type": "boolean",
621+
"$ref": "#/$defs/boolean-or-string",
622622
"description": "Enable health check endpoint globally",
623623
"default": true,
624624
"additionalProperties": false
@@ -1391,7 +1391,15 @@
13911391
"type": "string"
13921392
}
13931393
},
1394-
"required": ["singular"]
1394+
"required": [ "singular" ]
1395+
}
1396+
]
1397+
},
1398+
"boolean-or-string": {
1399+
"oneOf":[
1400+
{
1401+
"type": [ "boolean", "string" ],
1402+
"pattern": "^(?:true|false|1|0|@env\\('.*'\\)|@akv\\('.*'\\))$"
13951403
}
13961404
]
13971405
},

‎src/Config/Converters/BooleanJsonConverter.cs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Text.Json;
5+
using System.Text.Json.Serialization;
6+
7+
namespace Azure.DataApiBuilder.Config.Converters;
8+
9+
/// <summary>
10+
/// JSON converter for boolean values that also supports string representations such as
11+
/// "true", "false", "1", and "0". Any environment variable replacement is handled by
12+
/// other converters (for example, the string converter) before the value is parsed here.
13+
/// </summary>
14+
public class BoolJsonConverter : JsonConverter<bool>
15+
{
16+
17+
public override bool Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
18+
{
19+
if (reader.TokenType is JsonTokenType.Null)
20+
{
21+
22+
throw new JsonException("Unexpected null JSON token. Expected a boolean literal or a valid @expression.");
23+
}
24+
25+
if (reader.TokenType == JsonTokenType.String)
26+
{
27+
28+
string? tempBoolean = JsonSerializer.Deserialize<string>(ref reader, options);
29+
30+
bool result = tempBoolean?.ToLower() switch
31+
{
32+
//numeric values have to be checked here as they may come from string replacement
33+
"true" or "1" => true,
34+
"false" or "0" => false,
35+
_ => throw new JsonException($"Invalid boolean value: {tempBoolean}. Specify either true or 1 for true, false or 0 for false"),
36+
};
37+
38+
return result;
39+
}
40+
else if (reader.TokenType == JsonTokenType.Number)
41+
{
42+
bool result = reader.GetInt32() switch
43+
{
44+
1 => true,
45+
0 => false,
46+
_ => throw new JsonException($"Invalid value for boolean attribute. Specify either 1 or 0."),
47+
};
48+
return result;
49+
}
50+
else
51+
{
52+
return reader.GetBoolean();
53+
}
54+
}
55+
56+
public override void Write(Utf8JsonWriter writer, bool value, JsonSerializerOptions options)
57+
{
58+
writer.WriteBooleanValue(value);
59+
}
60+
}

‎src/Config/RuntimeConfigLoader.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ public static JsonSerializerOptions GetSerializationOptions(
327327
options.Converters.Add(new AKVRetryPolicyOptionsConverterFactory(replacementSettings));
328328
options.Converters.Add(new AzureLogAnalyticsOptionsConverterFactory(replacementSettings));
329329
options.Converters.Add(new AzureLogAnalyticsAuthOptionsConverter(replacementSettings));
330+
options.Converters.Add(new BoolJsonConverter());
330331
options.Converters.Add(new FileSinkConverter(replacementSettings));
331332

332333
// Add AzureKeyVaultOptionsConverterFactory to ensure AKV config is deserialized properly

‎src/Service.Tests/Caching/CachingConfigProcessingTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ public void GlobalCacheOptionsDeserialization_ValidValues(
161161
[DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 0 }", DisplayName = "EntityCacheOptions.TtlSeconds set to zero is invalid configuration.")]
162162
[DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": -1 }", DisplayName = "EntityCacheOptions.TtlSeconds set to negative number is invalid configuration.")]
163163
[DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 1.1 }", DisplayName = "EntityCacheOptions.TtlSeconds set to decimal is invalid configuration.")]
164-
[DataRow(@",""cache"": { ""enabled"": 1 }", DisplayName = "EntityCacheOptions.Enabled property set to 1 should fail because not a boolean.")]
165-
[DataRow(@",""cache"": { ""enabled"": 0 }", DisplayName = "EntityCacheOptions.Enabled property set to 0 should fail because not a boolean.")]
166164
[DataRow(@",""cache"": 1", DisplayName = "EntityCacheOptions property set to 1 should fail because it's not a JSON object.")]
167165
[DataRow(@",""cache"": 0", DisplayName = "EntityCacheOptions property set to 0 should fail because it's not a JSON object.")]
168166
[DataRow(@",""cache"": true", DisplayName = "EntityCacheOptions property set to true should fail because it's not a JSON object.")]

‎src/Service.Tests/Configuration/ConfigurationTests.cs‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,46 @@ type Moon {
664664
""entities"":{ }
665665
}";
666666

667+
public const string CONFIG_FILE_WITH_BOOLEAN_AS_ENV = @"{
668+
// Link for latest draft schema.
669+
""$schema"":""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch-alpha/dab.draft.schema.json"",
670+
""data-source"": {
671+
""database-type"": ""mssql"",
672+
""connection-string"": ""sample-conn-string"",
673+
""health"": {
674+
""enabled"": <REPLACE_VALUE>
675+
}
676+
},
677+
""runtime"": {
678+
""health"": {
679+
""enabled"": <REPLACE_VALUE>
680+
},
681+
""rest"": {
682+
""enabled"": <REPLACE_VALUE>,
683+
""path"": ""/api""
684+
},
685+
""graphql"": {
686+
""enabled"": <REPLACE_VALUE>,
687+
""path"": ""/graphql"",
688+
""allow-introspection"": true
689+
},
690+
""host"": {
691+
""authentication"": {
692+
""provider"": ""AppService""
693+
}
694+
},
695+
""telemetry"": {
696+
""application-insights"":{
697+
""enabled"": <REPLACE_VALUE>,
698+
""connection-string"":""sample-ai-connection-string""
699+
}
700+
701+
}
702+
703+
},
704+
""entities"":{ }
705+
}";
706+
667707
[TestCleanup]
668708
public void CleanupAfterEachTest()
669709
{
@@ -1820,8 +1860,44 @@ public void TestBasicConfigSchemaWithNoOptionalFieldsIsValid(string jsonData)
18201860
JsonConfigSchemaValidator jsonSchemaValidator = new(schemaValidatorLogger.Object, new MockFileSystem());
18211861

18221862
JsonSchemaValidationResult result = jsonSchemaValidator.ValidateJsonConfigWithSchema(jsonSchema, jsonData);
1823-
Assert.IsTrue(result.IsValid);
1863+
Assert.AreEqual("", String.Join('\n', result.ValidationErrors?.Select(s => $"{s.Message} at {s.Path} {s.LineNumber} {s.LinePosition}") ?? []), "Expected no validation errors.");
18241864
Assert.IsTrue(EnumerableUtilities.IsNullOrEmpty(result.ValidationErrors));
1865+
1866+
Assert.IsTrue(result.IsValid);
1867+
schemaValidatorLogger.Verify(
1868+
x => x.Log(
1869+
LogLevel.Information,
1870+
It.IsAny<EventId>(),
1871+
It.Is<It.IsAnyType>((o, t) => o.ToString()!.Contains($"The config satisfies the schema requirements.")),
1872+
It.IsAny<Exception>(),
1873+
(Func<It.IsAnyType, Exception, string>)It.IsAny<object>()),
1874+
Times.Once);
1875+
}
1876+
1877+
[DataTestMethod]
1878+
[DataRow("true", DisplayName = "Validates variable boolean schema for true value")]
1879+
[DataRow("false", DisplayName = "Validates variable boolean schema for false value.")]
1880+
[DataRow("\"true\"", DisplayName = "Validates variable boolean schema for true as string.")]
1881+
[DataRow("\"false\"", DisplayName = "Validates variable boolean schema for false as string.")]
1882+
[DataRow("\"1\"", DisplayName = "Validates variable boolean schema for 1 as string.")]
1883+
[DataRow("\"0\"", DisplayName = "Validates variable boolean schema for 0as string.")]
1884+
[DataRow("\"@env('SAMPLE')\"", DisplayName = "Validates variable boolean schema for environment variables.")]
1885+
[DataRow("\"@akv('SAMPLE')\"", DisplayName = "Validates variable boolean schema for keyvaul variables.")]
1886+
public void TestBasicConfigSchemaWithFlexibleBoolean(string Value)
1887+
{
1888+
Mock<ILogger<JsonConfigSchemaValidator>> schemaValidatorLogger = new();
1889+
1890+
string jsonSchema = File.ReadAllText("dab.draft.schema.json");
1891+
1892+
JsonConfigSchemaValidator jsonSchemaValidator = new(schemaValidatorLogger.Object, new MockFileSystem());
1893+
1894+
string jsonData = CONFIG_FILE_WITH_BOOLEAN_AS_ENV.Replace("<REPLACE_VALUE>", Value);
1895+
JsonSchemaValidationResult result = jsonSchemaValidator.ValidateJsonConfigWithSchema(jsonSchema, jsonData);
1896+
Assert.AreEqual("", String.Join('\n', result.ValidationErrors?.Select(s => $"{s.Message} at {s.Path} {s.LineNumber} {s.LinePosition}") ?? []), "Expected no validation errors.");
1897+
1898+
Assert.IsTrue(EnumerableUtilities.IsNullOrEmpty(result.ValidationErrors), "Validation Erros null of empty");
1899+
1900+
Assert.IsTrue(result.IsValid, "Result should be valid");
18251901
schemaValidatorLogger.Verify(
18261902
x => x.Log(
18271903
LogLevel.Information,
@@ -3368,7 +3444,7 @@ public async Task ValidateStrictModeAsDefaultForRestRequestBody(bool includeExtr
33683444
HttpMethod httpMethod = SqlTestHelper.ConvertRestMethodToHttpMethod(SupportedHttpVerb.Post);
33693445
string requestBody = @"{
33703446
""title"": ""Harry Potter and the Order of Phoenix"",
3371-
""publisher_id"": 1234";
3447+
""publisher_id"": 1234 ";
33723448

33733449
if (includeExtraneousFieldInRequestBody)
33743450
{

0 commit comments

Comments
 (0)