Repository navigation
fix(aspire): detach kept resources from removed resources in ResourcesToRemove #7295
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d21f444
fix(aspire): detach kept resources from removed resources in Resource…
thomhurst ae8f347
fix(aspire): drop option name with a removed argument value
thomhurst cbda71d
fix(aspire): keep '--' separator when dropping removed argument values
thomhurst d32c81c
fix(aspire): scan whole argument list for removed references
thomhurst 926aeb7
fix(aspire): pair options with values in rebuilt argument lists
thomhurst b26c2a2
fix(aspire): pair only options the current args callback added
thomhurst 609e4b3
fix(aspire): count argument occurrences when pairing options
thomhurst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,229 @@ | ||
| using Aspire.Hosting.ApplicationModel; | ||
|
|
||
| namespace TUnit.Aspire; | ||
|
|
||
| /// <summary> | ||
| /// Removes resources from an Aspire app model and detaches the edges that kept resources still | ||
| /// hold on them. Removing a resource from the model alone leaves <c>WaitFor</c> and | ||
| /// <c>WithReference</c> edges behind: a kept resource then waits forever for a resource that never | ||
| /// starts, or blocks resolving an endpoint or connection string that is never allocated (#7284). | ||
| /// </summary> | ||
| internal static class ResourceRemoval | ||
| { | ||
| // Bounds recursion through nested ReferenceExpressions; real value graphs are a few levels deep. | ||
| private const int MaxReferenceDepth = 32; | ||
|
|
||
| public static void Remove(IList<IResource> resources, IEnumerable<string> names, Action<string> log) | ||
| { | ||
| var removed = new HashSet<IResource>(ReferenceEqualityComparer.Instance); | ||
|
|
||
| foreach (var name in names) | ||
| { | ||
| var resource = resources.SingleOrDefault(r => string.Equals(r.Name, name, StringComparison.Ordinal)); | ||
|
|
||
| if (resource is not null) | ||
| { | ||
| removed.Add(resource); | ||
| } | ||
| else | ||
| { | ||
| log($"ResourcesToRemove: resource '{name}' not found (skipped)."); | ||
| } | ||
| } | ||
|
|
||
| if (removed.Count == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| AddOrphanedChildren(resources, removed, log); | ||
|
|
||
| foreach (var resource in removed) | ||
| { | ||
| resources.Remove(resource); | ||
| } | ||
|
|
||
| foreach (var resource in resources) | ||
| { | ||
| DetachEdges(resource, removed, log); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// A child (for example a database on a removed server) cannot run without its parent, so it | ||
| /// goes too. Repeats until stable so grandchildren follow. | ||
| /// </summary> | ||
| private static void AddOrphanedChildren(IList<IResource> resources, HashSet<IResource> removed, Action<string> log) | ||
| { | ||
| bool added; | ||
| do | ||
| { | ||
| added = false; | ||
| foreach (var resource in resources) | ||
| { | ||
| if (resource is IResourceWithParent child | ||
| && removed.Contains(child.Parent) | ||
| && removed.Add(resource)) | ||
| { | ||
| log($"ResourcesToRemove: also removing '{resource.Name}' because its parent '{child.Parent.Name}' is removed."); | ||
| added = true; | ||
| } | ||
| } | ||
| } while (added); | ||
| } | ||
|
|
||
| private static void DetachEdges(IResource resource, HashSet<IResource> removed, Action<string> log) | ||
| { | ||
| var annotations = resource.Annotations; | ||
|
|
||
| for (var i = annotations.Count - 1; i >= 0; i--) | ||
| { | ||
| switch (annotations[i]) | ||
| { | ||
| case WaitAnnotation wait when removed.Contains(wait.Resource): | ||
| annotations.RemoveAt(i); | ||
| log($"ResourcesToRemove: dropped wait on removed resource '{wait.Resource.Name}' from '{resource.Name}'."); | ||
| break; | ||
| case EndpointReferenceAnnotation endpointReference when removed.Contains(endpointReference.Resource): | ||
| annotations.RemoveAt(i); | ||
| log($"ResourcesToRemove: dropped endpoint reference to removed resource '{endpointReference.Resource.Name}' from '{resource.Name}'."); | ||
| break; | ||
| case ResourceRelationshipAnnotation relationship when removed.Contains(relationship.Resource): | ||
| annotations.RemoveAt(i); | ||
| log($"ResourcesToRemove: dropped relationship to removed resource '{relationship.Resource.Name}' from '{resource.Name}'."); | ||
| break; | ||
| case EnvironmentCallbackAnnotation environment: | ||
| annotations[i] = WrapEnvironmentCallback(environment, resource, removed, log); | ||
| break; | ||
| case CommandLineArgsCallbackAnnotation args: | ||
| annotations[i] = WrapArgsCallback(args, resource, removed, log); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// <c>WithReference</c> and <c>WithEnvironment</c> callbacks capture the removed resource in a | ||
| /// closure, so they cannot be detached by annotation. Run them, then drop each value that | ||
| /// references a removed resource before Aspire tries to resolve it. | ||
| /// </summary> | ||
| private static EnvironmentCallbackAnnotation WrapEnvironmentCallback( | ||
| EnvironmentCallbackAnnotation original, IResource resource, HashSet<IResource> removed, Action<string> log) | ||
| { | ||
| return new EnvironmentCallbackAnnotation(async context => | ||
| { | ||
| await original.Callback(context).ConfigureAwait(false); | ||
|
|
||
| List<string>? toRemove = null; | ||
| foreach (var (key, value) in context.EnvironmentVariables) | ||
| { | ||
| if (ReferencesRemoved(value, removed, 0)) | ||
| { | ||
| (toRemove ??= []).Add(key); | ||
| } | ||
| } | ||
|
|
||
| if (toRemove is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| foreach (var key in toRemove) | ||
| { | ||
| context.EnvironmentVariables.Remove(key); | ||
| log($"ResourcesToRemove: dropped environment variable '{key}' from '{resource.Name}' because it references a removed resource."); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private static CommandLineArgsCallbackAnnotation WrapArgsCallback( | ||
| CommandLineArgsCallbackAnnotation original, IResource resource, HashSet<IResource> removed, Action<string> log) | ||
| { | ||
| return new CommandLineArgsCallbackAnnotation(async context => | ||
| { | ||
| // A callback can replace or rebuild earlier arguments, so the whole list is scanned. An option is | ||
| // only paired with a value when this callback added it: an argument object that was in the list | ||
| // before the callback ran belongs to an earlier WithArgs call, wherever it now sits. | ||
| var before = new HashSet<object>(context.Args, ReferenceEqualityComparer.Instance); | ||
| await original.Callback(context).ConfigureAwait(false); | ||
|
|
||
| for (var i = context.Args.Count - 1; i >= 0; i--) | ||
| { | ||
| var argument = context.Args[i]; | ||
| if (!ReferencesRemoved(argument, removed, 0)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| context.Args.RemoveAt(i); | ||
|
|
||
| // "--name value": dropping only the value would make the next argument the option's value. | ||
| // A dropped "--name=value" is an option itself, so it is never the previous option's value, | ||
| // and after a "--" separator every argument is positional. | ||
| if (i > 0 | ||
| && !before.Contains(context.Args[i - 1]) | ||
|
thomhurst marked this conversation as resolved.
Resolved
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| && !IsOption(argument) | ||
| && IsOptionName(context.Args[i - 1]) | ||
| && !HasSeparatorBefore(context.Args, i - 1)) | ||
| { | ||
| var option = (string)context.Args[i - 1]; | ||
| context.Args.RemoveAt(i - 1); | ||
| i--; | ||
| log($"ResourcesToRemove: dropped argument '{option}' and its value from '{resource.Name}' because the value references a removed resource."); | ||
| } | ||
| else | ||
| { | ||
| log($"ResourcesToRemove: dropped argument {i} from '{resource.Name}' because it references a removed resource."); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // "--url=value" is one argument and is dropped whole, so only a bare option name pairs with the next argument. | ||
| private static bool IsOptionName(object? argument) | ||
| => argument is string { Length: > 1 } text && text[0] == '-' && text != "--" && !text.Contains('='); | ||
|
|
||
| private static bool HasSeparatorBefore(IList<object> args, int index) | ||
| { | ||
| for (var i = 0; i < index; i++) | ||
| { | ||
| if (args[i] is "--") | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private static bool IsOption(object? argument) => argument switch | ||
| { | ||
| string text => text.StartsWith('-'), | ||
| ReferenceExpression expression => expression.Format.StartsWith('-'), | ||
| _ => false, | ||
| }; | ||
|
|
||
| private static bool ReferencesRemoved(object? value, HashSet<IResource> removed, int depth) | ||
| { | ||
| switch (value) | ||
| { | ||
| // Stop at resources: a kept resource is valid even if it was built from other values. | ||
| case IResource referenced: | ||
| return removed.Contains(referenced); | ||
| case ConnectionStringReference connectionString when removed.Contains(connectionString.Resource): | ||
| return true; | ||
| case IValueWithReferences withReferences when depth < MaxReferenceDepth: | ||
| foreach (var reference in withReferences.References) | ||
| { | ||
| if (ReferencesRemoved(reference, removed, depth + 1)) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| default: | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.