Skip to content

Commit

Permalink
Fixes and enhancements in JSON Configuration schema generator (#4441)
Browse files Browse the repository at this point in the history 
* Add tests to identify the current behavior (all tests are green).

* Fix: respect [ConfigurationKeyName] override for property names

This initially broke the integration test, which loads reference assemblies from the 'refs' subdirectory.
But Microsoft.Extensions.Configuration.Abstractions.dll (which contains ConfigurationKeyNameAttribute) isn't copied there during build,
which is why I changed the test to not use the refs subdirectory.

* Ignore leading/trailing whitespace and line breaks in doc-comments, because they have no semantic meaning. IDEs auto-break lines when rendering the text, depending on screen space. However, do convert <br/> and <para/> tags into visible line breaks.

While this doesn't have much effect on Aspire itself, it matters greatly to my team because we have configured Resharper to auto-format doc-comments, which takes the configured maximum line length into account. So it merges and breaks lines in doc-comments all the time.

The changes on Aspire .json files are the following, which can be seen after replacing all \n with a single space in the diff.
- ` ,` becomes `,`
- `( 'SomeType' )` becomes `('SomeType')`
- `Some.  Other` becomes `Some. Other`
- `Some. Other` becomes `Some.\n\nOther` (because of <para> usage)
- `"Some ."` becomes `"Some."`

* Fix crash on property of type byte[]. The ConfigurationBinder supports an array of numbers, as well as a base64-encoded string. See dotnet/runtime#43150.

* Allow settings to appear top-level

* Add support for using <inheritdoc /> in code and external assemblies.

Because roslyn provides no public API to expand inherited doc-comments (see dotnet/csharplang#313), I'm using the internal Microsoft.CodeAnalysis.Shared.Extensions.ISymbolExtensions.GetDocumentationComment method. The method is made accessible by using reflection.

This method behaves a bit odd though: If there's no doc-comment on a member, it internally assumes that the member contains "<doc><inheritdoc/></doc>" (which is completely invalid) and feeds that to itself. As a consequence, the method may return something wrapped in <doc>, instead of the expected <member> element. I didn't want to write a test for this undocumented behavior, but it explains the fallback to <doc> when <member> does not exist.

* Refactor recursion to support collections and recursive/circular types, allow all types as component, preserve existing JSON structure on empty nodes.

Note this commit includes a fix in the RuntimeSource directory to prevent a StackOverflowException.

* Fix broken build after rebase on latest main

* Review feedback: use Queue instead of ImmutableQueue

* Preserve blank lines in doc-comments

* Remove IgnoresAccessChecksToGenerator usage and just use normal reflection to reduce dependencies.

* Remove unnecessary #ifs

* Add explaining comments for node backup strategy

* Fix normalization of whitespace characters, such as tabs

* Add explaining comments for line break handling in doc-comments

* Rename GenerateComponent to GeneratePathSegment

* Provide compiled sample usage instead of appsettings.json comment

* Replace minimal assertions with full JSON comparison

* PR feedback

- rename variables to remove "component"

---------

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
  • Loading branch information
@bart-vmware @eerhardt
bart-vmware and eerhardt committed Jun 28, 2024
1 parent 38f987e commit f79f9f0
Show file tree
Hide file tree
Showing 35 changed files with 2,215 additions and 592 deletions.
28 changes: 21 additions & 7 deletions src/Components/Aspire.Azure.AI.OpenAI/ConfigurationSchema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
}
}
},
"type": "object",
"properties": {
"Aspire": {
"type": "object",
Expand Down Expand Up @@ -43,7 +44,7 @@
},
"IsDistributedTracingEnabled": {
"type": "boolean",
"description": "Gets or sets value indicating whether distributed tracing activities ( 'System.Diagnostics.Activity' ) are going to be created for the clients methods calls and HTTP calls."
"description": "Gets or sets value indicating whether distributed tracing activities ('System.Diagnostics.Activity') are going to be created for the clients methods calls and HTTP calls."
},
"IsLoggingContentEnabled": {
"type": "boolean",
Expand All @@ -55,11 +56,25 @@
},
"IsTelemetryEnabled": {
"type": "boolean",
"description": "Gets or sets value indicating whether the \"User-Agent\" header containing 'Azure.Core.DiagnosticsOptions.ApplicationId' , client library package name and version, 'System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription' and 'System.Runtime.InteropServices.RuntimeInformation.OSDescription' should be sent.\nThe default value can be controlled process wide by setting AZURE_TELEMETRY_DISABLED to true , false , 1 or 0."
"description": "Gets or sets value indicating whether the \"User-Agent\" header containing 'Azure.Core.DiagnosticsOptions.ApplicationId', client library package name and version, 'System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription' and 'System.Runtime.InteropServices.RuntimeInformation.OSDescription' should be sent. The default value can be controlled process wide by setting AZURE_TELEMETRY_DISABLED to true, false, 1 or 0."
},
"LoggedContentSizeLimit": {
"type": "integer",
"description": "Gets or sets value indicating maximum size of content to log in bytes. Defaults to 4096."
},
"LoggedHeaderNames": {
"type": "array",
"items": {
"type": "string"
},
"description": "Gets a list of header names that are not redacted during logging."
},
"LoggedQueryParameters": {
"type": "array",
"items": {
"type": "string"
},
"description": "Gets a list of query parameter names that are not redacted during logging."
}
},
"description": "Gets the client diagnostic options."
Expand All @@ -70,12 +85,12 @@
"Delay": {
"type": "string",
"pattern": "^-?(\\d{1,7}|((\\d{1,7}[\\.:])?(([01]?\\d|2[0-3]):[0-5]?\\d|([01]?\\d|2[0-3]):[0-5]?\\d:[0-5]?\\d)(\\.\\d{1,7})?))$",
"description": "The delay between retry attempts for a fixed approach or the delay\non which to base calculations for a backoff-based approach.\nIf the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
"description": "The delay between retry attempts for a fixed approach or the delay on which to base calculations for a backoff-based approach. If the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
},
"MaxDelay": {
"type": "string",
"pattern": "^-?(\\d{1,7}|((\\d{1,7}[\\.:])?(([01]?\\d|2[0-3]):[0-5]?\\d|([01]?\\d|2[0-3]):[0-5]?\\d:[0-5]?\\d)(\\.\\d{1,7})?))$",
"description": "The maximum permissible delay between retry attempts when the service does not provide a Retry-After response header.\nIf the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
"description": "The maximum permissible delay between retry attempts when the service does not provide a Retry-After response header. If the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
},
"MaxRetries": {
"type": "integer",
Expand Down Expand Up @@ -107,7 +122,7 @@
"Endpoint": {
"type": "string",
"format": "uri",
"description": "Gets or sets a 'System.Uri' referencing the Azure OpenAI endpoint.\nThis is likely to be similar to \"https://{account_name}.openai.azure.com\"."
"description": "Gets or sets a 'System.Uri' referencing the Azure OpenAI endpoint. This is likely to be similar to \"https://{account_name}.openai.azure.com\"."
},
"Key": {
"type": "string",
Expand All @@ -122,6 +137,5 @@
}
}
}
},
"type": "object"
}
}
30 changes: 22 additions & 8 deletions src/Components/Aspire.Azure.Data.Tables/ConfigurationSchema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
}
}
},
"type": "object",
"properties": {
"Aspire": {
"type": "object",
Expand Down Expand Up @@ -43,7 +44,7 @@
},
"IsDistributedTracingEnabled": {
"type": "boolean",
"description": "Gets or sets value indicating whether distributed tracing activities ( 'System.Diagnostics.Activity' ) are going to be created for the clients methods calls and HTTP calls."
"description": "Gets or sets value indicating whether distributed tracing activities ('System.Diagnostics.Activity') are going to be created for the clients methods calls and HTTP calls."
},
"IsLoggingContentEnabled": {
"type": "boolean",
Expand All @@ -55,31 +56,45 @@
},
"IsTelemetryEnabled": {
"type": "boolean",
"description": "Gets or sets value indicating whether the \"User-Agent\" header containing 'Azure.Core.DiagnosticsOptions.ApplicationId' , client library package name and version, 'System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription' and 'System.Runtime.InteropServices.RuntimeInformation.OSDescription' should be sent.\nThe default value can be controlled process wide by setting AZURE_TELEMETRY_DISABLED to true , false , 1 or 0."
"description": "Gets or sets value indicating whether the \"User-Agent\" header containing 'Azure.Core.DiagnosticsOptions.ApplicationId', client library package name and version, 'System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription' and 'System.Runtime.InteropServices.RuntimeInformation.OSDescription' should be sent. The default value can be controlled process wide by setting AZURE_TELEMETRY_DISABLED to true, false, 1 or 0."
},
"LoggedContentSizeLimit": {
"type": "integer",
"description": "Gets or sets value indicating maximum size of content to log in bytes. Defaults to 4096."
},
"LoggedHeaderNames": {
"type": "array",
"items": {
"type": "string"
},
"description": "Gets a list of header names that are not redacted during logging."
},
"LoggedQueryParameters": {
"type": "array",
"items": {
"type": "string"
},
"description": "Gets a list of query parameter names that are not redacted during logging."
}
},
"description": "Gets the client diagnostic options."
},
"EnableTenantDiscovery": {
"type": "boolean",
"description": "Enables tenant discovery through the authorization challenge when the client is configured to use a TokenCredential.\nWhen true , the client will attempt an initial un-authorized request to prompt an OAuth challenge in order to discover the correct tenant for the resource."
"description": "Enables tenant discovery through the authorization challenge when the client is configured to use a TokenCredential. When true, the client will attempt an initial un-authorized request to prompt an OAuth challenge in order to discover the correct tenant for the resource."
},
"Retry": {
"type": "object",
"properties": {
"Delay": {
"type": "string",
"pattern": "^-?(\\d{1,7}|((\\d{1,7}[\\.:])?(([01]?\\d|2[0-3]):[0-5]?\\d|([01]?\\d|2[0-3]):[0-5]?\\d:[0-5]?\\d)(\\.\\d{1,7})?))$",
"description": "The delay between retry attempts for a fixed approach or the delay\non which to base calculations for a backoff-based approach.\nIf the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
"description": "The delay between retry attempts for a fixed approach or the delay on which to base calculations for a backoff-based approach. If the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
},
"MaxDelay": {
"type": "string",
"pattern": "^-?(\\d{1,7}|((\\d{1,7}[\\.:])?(([01]?\\d|2[0-3]):[0-5]?\\d|([01]?\\d|2[0-3]):[0-5]?\\d:[0-5]?\\d)(\\.\\d{1,7})?))$",
"description": "The maximum permissible delay between retry attempts when the service does not provide a Retry-After response header.\nIf the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
"description": "The maximum permissible delay between retry attempts when the service does not provide a Retry-After response header. If the service provides a Retry-After response header, the next retry will be delayed by the duration specified by the header value."
},
"MaxRetries": {
"type": "integer",
Expand Down Expand Up @@ -120,7 +135,7 @@
"ServiceUri": {
"type": "string",
"format": "uri",
"description": "A 'System.Uri' referencing the table service account.\nThis is likely to be similar to \"https://{account_name}.table.core.windows.net/\" or \"https://{account_name}.table.cosmos.azure.com/\"."
"description": "A 'System.Uri' referencing the table service account. This is likely to be similar to \"https://{account_name}.table.core.windows.net/\" or \"https://{account_name}.table.cosmos.azure.com/\"."
}
},
"description": "Provides the client configuration settings for connecting to Azure Tables."
Expand All @@ -131,6 +146,5 @@
}
}
}
},
"type": "object"
}
}
Loading

0 comments on commit f79f9f0

Please sign in to comment.