-
Notifications
You must be signed in to change notification settings - Fork 769
Refactor user secrets to DI-based factory with thread-safe synchronization #12692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e8712f7
4d2cfac
b5cd119
81d3417
50647b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,6 @@ | |
| <Compile Include="$(SharedDir)LoggingHelpers.cs" Link="Utils\LoggingHelpers.cs" /> | ||
| <Compile Include="$(SharedDir)StringUtils.cs" Link="Utils\StringUtils.cs" /> | ||
| <Compile Include="$(SharedDir)SchemaUtils.cs" Link="Utils\SchemaUtils.cs" /> | ||
| <Compile Include="$(SharedDir)SecretsStore.cs" Link="Utils\SecretsStore.cs" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the file be deleted now? This appears to be the only project that uses it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot delete this file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in commit 81d3417. The file has been deleted as it's no longer used after the refactoring. |
||
| <Compile Include="$(SharedDir)ConsoleLogs\LogEntries.cs" Link="Utils\ConsoleLogs\LogEntries.cs" /> | ||
| <Compile Include="$(SharedDir)ConsoleLogs\LogEntry.cs" Link="Utils\ConsoleLogs\LogEntry.cs" /> | ||
| <Compile Include="$(SharedDir)ConsoleLogs\LogPauseViewModel.cs" Link="Utils\ConsoleLogs\LogPauseViewModel.cs" /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Text.Json.Nodes; | ||
|
|
||
| namespace Aspire.Hosting.Pipelines.Internal; | ||
|
|
||
| /// <summary> | ||
| /// Provides utility methods for flattening and unflattening JSON objects using colon-separated keys. | ||
| /// </summary> | ||
| internal static class JsonFlattener | ||
| { | ||
| /// <summary> | ||
| /// Flattens a JsonObject using colon-separated keys for configuration compatibility. | ||
| /// Handles both nested objects and arrays with indexed keys. | ||
| /// </summary> | ||
| /// <param name="source">The source JsonObject to flatten.</param> | ||
| /// <returns>A flattened JsonObject.</returns> | ||
| public static JsonObject FlattenJsonObject(JsonObject source) | ||
| { | ||
| var result = new JsonObject(); | ||
| FlattenJsonObjectRecursive(source, string.Empty, result); | ||
| return result; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Unflattens a JsonObject that uses colon-separated keys back into a nested structure. | ||
| /// Handles both nested objects and arrays with indexed keys. | ||
| /// </summary> | ||
| /// <param name="source">The flattened JsonObject to unflatten.</param> | ||
| /// <returns>An unflattened JsonObject with nested structure.</returns> | ||
| public static JsonObject UnflattenJsonObject(JsonObject source) | ||
| { | ||
| var result = new JsonObject(); | ||
|
|
||
| foreach (var kvp in source) | ||
| { | ||
| var keys = kvp.Key.Split(':'); | ||
| var current = result; | ||
|
|
||
| for (var i = 0; i < keys.Length - 1; i++) | ||
| { | ||
| var key = keys[i]; | ||
| if (!current.TryGetPropertyValue(key, out var existing) || existing is not JsonObject) | ||
| { | ||
| var newObject = new JsonObject(); | ||
| current[key] = newObject; | ||
| current = newObject; | ||
| } | ||
| else | ||
| { | ||
| current = existing.AsObject(); | ||
| } | ||
| } | ||
|
|
||
| current[keys[^1]] = kvp.Value?.DeepClone(); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| private static void FlattenJsonObjectRecursive(JsonObject source, string prefix, JsonObject result) | ||
| { | ||
| foreach (var kvp in source) | ||
| { | ||
| var key = string.IsNullOrEmpty(prefix) ? kvp.Key : $"{prefix}:{kvp.Key}"; | ||
|
|
||
| if (kvp.Value is JsonObject nestedObject) | ||
| { | ||
| FlattenJsonObjectRecursive(nestedObject, key, result); | ||
| } | ||
| else if (kvp.Value is JsonArray array) | ||
| { | ||
| for (var i = 0; i < array.Count; i++) | ||
| { | ||
| var arrayKey = $"{key}:{i}"; | ||
| if (array[i] is JsonObject arrayObject) | ||
| { | ||
| FlattenJsonObjectRecursive(arrayObject, arrayKey, result); | ||
| } | ||
| else | ||
| { | ||
| result[arrayKey] = array[i]?.DeepClone(); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| result[key] = kvp.Value?.DeepClone(); | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Text.Json.Nodes; | ||
| using Microsoft.Extensions.Configuration; | ||
|
|
||
| namespace Aspire.Hosting.UserSecrets; | ||
|
|
||
| /// <summary> | ||
| /// Defines an interface for managing user secrets with support for read and write operations. | ||
| /// </summary> | ||
| internal interface IUserSecretsManager | ||
| { | ||
| /// <summary> | ||
| /// Gets the path to the user secrets file. | ||
| /// </summary> | ||
| string FilePath { get; } | ||
|
|
||
| /// <summary> | ||
| /// Attempts to set a user secret value synchronously. | ||
| /// </summary> | ||
| /// <param name="name">The name of the secret.</param> | ||
| /// <param name="value">The value of the secret.</param> | ||
| /// <returns>True if the secret was set successfully; otherwise, false.</returns> | ||
| bool TrySetSecret(string name, string value); | ||
|
|
||
| /// <summary> | ||
| /// Gets a secret value if it exists in configuration, or sets it using the value generator if it doesn't. | ||
| /// </summary> | ||
| /// <param name="configuration">The configuration manager to check and update.</param> | ||
| /// <param name="name">The name of the secret.</param> | ||
| /// <param name="valueGenerator">Function to generate the value if it doesn't exist.</param> | ||
| void GetOrSetSecret(IConfigurationManager configuration, string name, Func<string> valueGenerator); | ||
|
|
||
| /// <summary> | ||
| /// Saves state to user secrets asynchronously (for deployment state manager). | ||
| /// If multiple callers save state concurrently, the last write wins. | ||
| /// </summary> | ||
| /// <param name="state">The state to save as a JSON object.</param> | ||
| /// <param name="cancellationToken">Cancellation token.</param> | ||
| Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who uses the ctor that takes a factory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot write tests that use this overload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructor taking a factory is used only by tests to enable test isolation. Tests create their own
UserSecretsManagerFactoryinstances to avoid cross-contamination between test runs. Production code uses the parameterless constructor which delegates to the factory-based constructor withUserSecretsManagerFactory.Instance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 3 tests in commit 50647b4 that specifically use the constructor overload with factory parameter:
UserSecretsParameterDefault_WithCustomFactory_UsesProvidedFactory- Verifies the custom factory is usedUserSecretsParameterDefault_WithCustomFactory_IsolatesFromGlobalInstance- Tests isolation between custom factory and singleton instanceUserSecretsParameterDefault_WithCustomFactory_ConcurrentAccess- Validates thread safety with custom factory under concurrent access