Add tests and fix circular dependency and last wins

dotnet · Jan 31, 2025 · c7537a4 · c7537a4
1 parent c7745e6
commit c7537a4
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 22 deletions.
diff --git a/src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs b/src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs
@@ -253,44 +253,36 @@ static bool TryGetParent(IResource resource, [NotNullWhen(true)] out IResource?
             return false;
         }
 
-        static IResource? SelectRootResource(IResource? resource) => resource switch
+        static IResource? SelectParentResource(IResource? resource) => resource switch
         {
-            IResource r when TryGetParent(r, out var parent) => SelectRootResource(parent) ?? parent,
+            IResource r when TryGetParent(r, out var parent) => parent,
             _ => null
         };
 
-        var result = model.Resources.Select(x => (Child: x, Root: SelectRootResource(x)))
-                                    .Where(x => x.Root is not null)
+        var result = model.Resources.Select(x => (Child: x, Parent: SelectParentResource(x)))
+                                    .Where(x => x.Parent is not null)
                                     .ToArray();
 
         ValidateRelationships(result!);
 
-        return result;
+        static IResource? SelectRootResource(IResource? resource) => resource switch
+        {
+            IResource r when TryGetParent(r, out var parent) => SelectRootResource(parent) ?? parent,
+            _ => null
+        };
+
+        // translate the result to child -> root, which the dashboard expects
+        return result.Select(x => (x.Child, Root: SelectRootResource(x.Child)));
     }
 
-    private static void ValidateRelationships((IResource Child, IResource Root)[] relationships)
+    private static void ValidateRelationships((IResource Child, IResource Parent)[] relationships)
     {
         if (relationships.Length == 0)
         {
             return;
         }
 
-        // ensure each child only appears once (i.e. doesn't have multiple parents)
-        List<IResource>? duplicates = null;
-        var childToParentLookup = new Dictionary<IResource, IResource>();
-        foreach (var relation in relationships)
-        {
-            if (!childToParentLookup.TryAdd(relation.Child, relation.Root))
-            {
-                duplicates ??= [];
-                duplicates.Add(relation.Child);
-            }
-        }
-
-        if (duplicates is not null)
-        {
-            throw new InvalidOperationException($"The following resources have multiple parent relationships: {string.Join(", ", duplicates)}");
-        }
+        var childToParentLookup = relationships.ToDictionary(x => x.Child, x => x.Parent);
 
         // ensure no circular dependencies
         var visited = new Stack<IResource>();

diff --git a/tests/Aspire.Hosting.Tests/Orchestrator/ApplicationOrchestratorTests.cs b/tests/Aspire.Hosting.Tests/Orchestrator/ApplicationOrchestratorTests.cs
@@ -7,6 +7,7 @@
 using Aspire.Hosting.Lifecycle;
 using Aspire.Hosting.Orchestrator;
 using Aspire.Hosting.Tests.Utils;
+using Aspire.Hosting.Utils;
 using Microsoft.AspNetCore.InternalTesting;
 using Microsoft.Extensions.DependencyInjection;
 using Xunit;
@@ -61,6 +62,146 @@ public async Task ParentPropertySetOnChildResource()
         Assert.Equal(parentResourceId, childParentResourceId);
     }
 
+    [Fact]
+    public async Task WithParentRelationshipSetsParentPropertyCorrectly()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+
+        var parent = builder.AddContainer("parent", "image");
+        var child = builder.AddContainer("child", "image").WithParentRelationship(parent.Resource);
+        var child2 = builder.AddContainer("child2", "image").WithParentRelationship(parent.Resource);
+
+        var nestedChild = builder.AddContainer("nested-child", "image").WithParentRelationship(child.Resource);
+
+        using var app = builder.Build();
+        var distributedAppModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+
+        var events = new DcpExecutorEvents();
+        var resourceNotificationService = ResourceNotificationServiceTestHelpers.Create();
+
+        var appOrchestrator = CreateOrchestrator(distributedAppModel, notificationService: resourceNotificationService, dcpEvents: events);
+        await appOrchestrator.RunApplicationAsync();
+
+        string? parentResourceId = null;
+        string? childParentResourceId = null;
+        string? child2ParentResourceId = null;
+        string? nestedChildParentResourceId = null;
+        var watchResourceTask = Task.Run(async () =>
+        {
+            await foreach (var item in resourceNotificationService.WatchAsync())
+            {
+                if (item.Resource == parent.Resource)
+                {
+                    parentResourceId = item.ResourceId;
+                }
+                else if (item.Resource == child.Resource)
+                {
+                    childParentResourceId = item.Snapshot.Properties.SingleOrDefault(p => p.Name == KnownProperties.Resource.ParentName)?.Value?.ToString();
+                }
+                else if (item.Resource == nestedChild.Resource)
+                {
+                    nestedChildParentResourceId = item.Snapshot.Properties.SingleOrDefault(p => p.Name == KnownProperties.Resource.ParentName)?.Value?.ToString();
+                }
+                else if (item.Resource == child2.Resource)
+                {
+                    child2ParentResourceId = item.Snapshot.Properties.SingleOrDefault(p => p.Name == KnownProperties.Resource.ParentName)?.Value?.ToString();
+                }
+
+                if (parentResourceId != null && childParentResourceId != null && nestedChildParentResourceId != null && child2ParentResourceId != null)
+                {
+                    return;
+                }
+            }
+        });
+
+        await events.PublishAsync(new OnResourceStartingContext(CancellationToken.None, KnownResourceTypes.Container, parent.Resource, parent.Resource.Name));
+
+        await watchResourceTask.DefaultTimeout();
+
+        Assert.Equal(parentResourceId, childParentResourceId);
+        Assert.Equal(parentResourceId, child2ParentResourceId);
+
+        // Nested child should have parent set to the root parent, not direct parent
+        Assert.Equal(parentResourceId, nestedChildParentResourceId);
+    }
+
+    [Fact]
+    public async Task LastWithParentRelationshipWins()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+
+        var firstParent = builder.AddContainer("firstParent", "image");
+        var secondParent = builder.AddContainer("secondParent", "image");
+
+        var child = builder.AddContainer("child", "image");
+
+        child.WithParentRelationship(firstParent.Resource);
+        child.WithParentRelationship(secondParent.Resource);
+
+        using var app = builder.Build();
+        var distributedAppModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+
+        var events = new DcpExecutorEvents();
+        var resourceNotificationService = ResourceNotificationServiceTestHelpers.Create();
+
+        var appOrchestrator = CreateOrchestrator(distributedAppModel, notificationService: resourceNotificationService, dcpEvents: events);
+        await appOrchestrator.RunApplicationAsync();
+
+        string? firstParentResourceId = null;
+        string? secondParentResourceId = null;
+        string? childParentResourceId = null;
+        var watchResourceTask = Task.Run(async () =>
+        {
+            await foreach (var item in resourceNotificationService.WatchAsync())
+            {
+                if (item.Resource == firstParent.Resource)
+                {
+                    firstParentResourceId = item.ResourceId;
+                }
+                else if (item.Resource == secondParent.Resource)
+                {
+                    secondParentResourceId = item.ResourceId;
+                }
+                else if (item.Resource == child.Resource)
+                {
+                    childParentResourceId = item.Snapshot.Properties.SingleOrDefault(p => p.Name == KnownProperties.Resource.ParentName)?.Value?.ToString();
+                }
+
+                if (firstParentResourceId != null && secondParentResourceId != null && childParentResourceId != null)
+                {
+                    return;
+                }
+            }
+        });
+
+        await events.PublishAsync(new OnResourceStartingContext(CancellationToken.None, KnownResourceTypes.Container, firstParent.Resource, firstParent.Resource.Name));
+        await events.PublishAsync(new OnResourceStartingContext(CancellationToken.None, KnownResourceTypes.Container, secondParent.Resource, secondParent.Resource.Name));
+
+        await watchResourceTask.DefaultTimeout();
+
+        // child should be parented to the last parent set
+        Assert.Equal(secondParentResourceId, childParentResourceId);
+    }
+
+    [Fact]
+    public void DetectsCircularDependency()
+    {
+        using var builder = TestDistributedApplicationBuilder.Create();
+
+        var container1 = builder.AddContainer("container1", "image");
+        var container2 = builder.AddContainer("container2", "image2");
+        var container3 = builder.AddContainer("container3", "image3");
+
+        container1.WithParentRelationship(container2.Resource);
+        container2.WithParentRelationship(container3.Resource);
+        container3.WithParentRelationship(container1.Resource);
+
+        using var app = builder.Build();
+
+        var e = Assert.Throws<InvalidOperationException>(() => app.Services.GetService<ApplicationOrchestrator>());
+        Assert.Contains("Circular dependency detected", e.Message);
+    }
+
     private static ApplicationOrchestrator CreateOrchestrator(
         DistributedApplicationModel distributedAppModel,
         ResourceNotificationService notificationService,