diff --git a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs index f5b4482ca..f92cb4fcc 100644 --- a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs +++ b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs @@ -75,7 +75,41 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< public async Task RefreshDescriptorsAsync(IEnumerable activityProviders, CancellationToken cancellationToken = default) { var providersDictionary = new ConcurrentDictionary>(); - var activityDescriptors = new ConcurrentDictionary<(string Type, int Version), ActivityDescriptor>(_activityDescriptors); + var activityDescriptors = new ConcurrentDictionary<(string Type, int Version), ActivityDescriptor>(); + + // First, preserve manually registered descriptors from Register/RegisterAsync(Type) calls, which are stored under the ActivityRegistry type (GetType()) as the provider key, without logging warnings (since we're starting fresh). + if (_providedActivityDescriptors.TryGetValue(GetType(), out var manualDescriptors)) + { + var preservedManualDescriptors = new List(); + providersDictionary[GetType()] = preservedManualDescriptors; + + foreach (var manualDescriptor in manualDescriptors) + { + activityDescriptors[(manualDescriptor.TypeName, manualDescriptor.Version)] = manualDescriptor; + preservedManualDescriptors.Add(manualDescriptor); + } + } + + // Also add descriptors from _manualActivityDescriptors (from RegisterAsync(Type activityType) calls). + // These should also be tracked under the GetType() provider key to keep providersDictionary consistent. + if (_manualActivityDescriptors.Count > 0) + { + if (!providersDictionary.TryGetValue(GetType(), out var manualProviderDescriptors)) + { + manualProviderDescriptors = new List(); + providersDictionary[GetType()] = manualProviderDescriptors; + } + + foreach (var manualDescriptor in _manualActivityDescriptors) + { + activityDescriptors[(manualDescriptor.TypeName, manualDescriptor.Version)] = manualDescriptor; + + // Avoid adding duplicates to the provider list if the descriptor was already preserved. + if (!manualProviderDescriptors.Contains(manualDescriptor)) + manualProviderDescriptors.Add(manualDescriptor); + } + } + foreach (var activityProvider in activityProviders) { var descriptors = (await activityProvider.GetDescriptorsAsync(cancellationToken)).ToList(); diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs new file mode 100644 index 000000000..ab5d27015 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs @@ -0,0 +1,187 @@ +using Elsa.Workflows; +using Elsa.Workflows.Models; +using Microsoft.Extensions.Logging; +using Moq; + +namespace Elsa.Workflows.Core.UnitTests.Services; + +public class ActivityRegistryTests +{ + [Fact] + public async Task RefreshDescriptorsAsync_CalledTwice_DoesNotLogWarnings() + { + // Arrange + var mockLogger = new Mock>(); + var mockActivityDescriber = new Mock(); + var modifiers = Array.Empty(); + + var registry = new ActivityRegistry(mockActivityDescriber.Object, modifiers, mockLogger.Object); + + var mockProvider = new Mock(); + var descriptor1 = new ActivityDescriptor + { + TypeName = "TestActivity1", + Version = 1, + Kind = ActivityKind.Task, + Category = "Test", + Description = "Test Activity 1", + IsBrowsable = true + }; + + var descriptor2 = new ActivityDescriptor + { + TypeName = "TestActivity2", + Version = 1, + Kind = ActivityKind.Task, + Category = "Test", + Description = "Test Activity 2", + IsBrowsable = true + }; + + mockProvider.Setup(p => p.GetDescriptorsAsync(It.IsAny())) + .ReturnsAsync(new[] { descriptor1, descriptor2 }); + + var providers = new[] { mockProvider.Object }; + + // Act - First refresh + await registry.RefreshDescriptorsAsync(providers); + + // Act - Second refresh (simulates the intentional repopulation in DefaultRegistriesPopulator) + await registry.RefreshDescriptorsAsync(providers); + + // Assert - Verify no warning logs were made + mockLogger.Verify( + x => x.Log( + LogLevel.Warning, + It.IsAny(), + It.Is((v, t) => v.ToString()!.Contains("was already registered")), + It.IsAny(), + It.IsAny>()), + Times.Never, + "Expected no duplicate registration warnings during refresh"); + + // Verify descriptors are still registered + var allDescriptors = registry.ListAll().ToList(); + Assert.Equal(2, allDescriptors.Count); + Assert.Contains(allDescriptors, d => d.TypeName == "TestActivity1"); + Assert.Contains(allDescriptors, d => d.TypeName == "TestActivity2"); + } + + [Fact] + public async Task RefreshDescriptorsAsync_PreservesManualDescriptors() + { + // Arrange + var mockLogger = new Mock>(); + var mockActivityDescriber = new Mock(); + var modifiers = Array.Empty(); + + var registry = new ActivityRegistry(mockActivityDescriber.Object, modifiers, mockLogger.Object); + + // Create a manual descriptor + var manualDescriptor = new ActivityDescriptor + { + TypeName = "ManualActivity", + Version = 1, + Kind = ActivityKind.Task, + Category = "Manual", + Description = "Manually registered activity", + IsBrowsable = true + }; + + registry.Register(manualDescriptor); + + // Create a provider descriptor + var mockProvider = new Mock(); + var providerDescriptor = new ActivityDescriptor + { + TypeName = "ProviderActivity", + Version = 1, + Kind = ActivityKind.Task, + Category = "Provider", + Description = "Provider activity", + IsBrowsable = true + }; + + mockProvider.Setup(p => p.GetDescriptorsAsync(It.IsAny())) + .ReturnsAsync(new[] { providerDescriptor }); + + var providers = new[] { mockProvider.Object }; + + // Act - Refresh with provider + await registry.RefreshDescriptorsAsync(providers); + + // Assert - Both manual and provider descriptors should be present + var allDescriptors = registry.ListAll().ToList(); + Assert.Equal(2, allDescriptors.Count); + Assert.Contains(allDescriptors, d => d.TypeName == "ManualActivity"); + Assert.Contains(allDescriptors, d => d.TypeName == "ProviderActivity"); + + // Verify no warnings about manual descriptor being replaced + mockLogger.Verify( + x => x.Log( + LogLevel.Warning, + It.IsAny(), + It.Is((v, t) => v.ToString()!.Contains("ManualActivity")), + It.IsAny(), + It.IsAny>()), + Times.Never, + "Expected no warnings about manual descriptor"); + } + + [Fact] + public async Task RefreshDescriptorsAsync_LogsWarning_WhenDifferentProvidersRegisterSameActivity() + { + // Arrange + var mockLogger = new Mock>(); + var mockActivityDescriber = new Mock(); + var modifiers = Array.Empty(); + + var registry = new ActivityRegistry(mockActivityDescriber.Object, modifiers, mockLogger.Object); + + // Create two different providers with the same activity + var mockProvider1 = new Mock(); + var mockProvider2 = new Mock(); + + var descriptor1 = new ActivityDescriptor + { + TypeName = "DuplicateActivity", + Version = 1, + Kind = ActivityKind.Task, + Category = "Test", + Description = "From Provider 1", + IsBrowsable = true + }; + + var descriptor2 = new ActivityDescriptor + { + TypeName = "DuplicateActivity", + Version = 1, + Kind = ActivityKind.Task, + Category = "Test", + Description = "From Provider 2", + IsBrowsable = true + }; + + mockProvider1.Setup(p => p.GetDescriptorsAsync(It.IsAny())) + .ReturnsAsync(new[] { descriptor1 }); + + mockProvider2.Setup(p => p.GetDescriptorsAsync(It.IsAny())) + .ReturnsAsync(new[] { descriptor2 }); + + var providers = new[] { mockProvider1.Object, mockProvider2.Object }; + + // Act + await registry.RefreshDescriptorsAsync(providers); + + // Assert - Should log a warning for the duplicate + mockLogger.Verify( + x => x.Log( + LogLevel.Warning, + It.IsAny(), + It.Is((v, t) => v.ToString()!.Contains("DuplicateActivity") && v.ToString()!.Contains("was already registered")), + It.IsAny(), + It.IsAny>()), + Times.Once, + "Expected one warning for duplicate activity from different providers"); + } +}