diff --git a/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs b/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs index cc42574e1..0ce371b41 100644 --- a/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs +++ b/src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs @@ -30,6 +30,11 @@ public class TenantRegistryData /// public ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> ActivityDescriptors { get; } = new(); + /// + /// Cached latest-version activity descriptors by type for fast lookups. + /// + internal ConcurrentDictionary LatestActivityDescriptors { get; } = new(); + /// /// Secondary index of activity descriptors grouped by their provider type. /// diff --git a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs index 043fc9c33..b0f1184b0 100644 --- a/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs +++ b/src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs @@ -24,7 +24,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< { var registry = GetOrCreateRegistry(descriptor.TenantId); var providerDescriptors = GetOrCreateProviderDescriptors(registry, providerType); - Add(descriptor, registry.ActivityDescriptors, providerDescriptors); + Add(descriptor, registry, providerDescriptors); } /// @@ -34,7 +34,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< if (registry.ProvidedActivityDescriptors.TryGetValue(providerType, out var providerDescriptors)) { providerDescriptors.Remove(descriptor); - registry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); + RemoveDescriptor(registry, descriptor); } } @@ -82,18 +82,14 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< // Get highest version from current tenant's registry if (_tenantRegistries.TryGetValue(currentTenantId, out var tenantRegistry)) { - var tenantDescriptor = tenantRegistry.ActivityDescriptors.Values - .Where(x => x.TypeName == type) - .MaxBy(x => x.Version); - - if (tenantDescriptor != null) + if (tenantRegistry.LatestActivityDescriptors.TryGetValue(type, out var tenantDescriptor)) return tenantDescriptor; } // Fall back to agnostic registry only if no tenant-specific descriptor exists - return _agnosticRegistry.ActivityDescriptors.Values - .Where(x => x.TypeName == type) - .MaxBy(x => x.Version); + return _agnosticRegistry.LatestActivityDescriptors.TryGetValue(type, out var agnosticDescriptor) + ? agnosticDescriptor + : null; } /// @@ -151,7 +147,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< { var registry = GetOrCreateRegistry(descriptor.TenantId); var providerDescriptors = GetOrCreateProviderDescriptors(registry, GetType()); - Add(descriptor, registry.ActivityDescriptors, providerDescriptors); + Add(descriptor, registry, providerDescriptors); } /// @@ -166,7 +162,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< var activityDescriptor = await activityDescriber.DescribeActivityAsync(activityType, cancellationToken); var registry = GetOrCreateRegistry(activityDescriptor.TenantId); - Add(activityDescriptor, registry.ActivityDescriptors, _manualActivityDescriptors); + Add(activityDescriptor, registry, _manualActivityDescriptors); _manualActivityDescriptors.Add(activityDescriptor); } @@ -208,7 +204,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< { foreach (var oldDescriptor in oldDescriptors.ToList()) { - registry.ActivityDescriptors.TryRemove((oldDescriptor.TypeName, oldDescriptor.Version), out _); + RemoveDescriptor(registry, oldDescriptor); } } @@ -216,7 +212,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< var providerDescriptors = new List(); foreach (var descriptor in group) { - Add(descriptor, registry.ActivityDescriptors, providerDescriptors); + Add(descriptor, registry, providerDescriptors); } // Update the provider's descriptor list in this registry @@ -224,7 +220,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< } } - private void Add(ActivityDescriptor? descriptor, ConcurrentDictionary<(string Type, int Version), ActivityDescriptor> activityDescriptors, ICollection providerDescriptors) + private void Add(ActivityDescriptor? descriptor, TenantRegistryData registry, ICollection providerDescriptors) { if (descriptor is null) { @@ -235,8 +231,11 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< foreach (var modifier in modifiers) modifier.Modify(descriptor); + var activityDescriptors = registry.ActivityDescriptors; + var descriptorKey = (descriptor.TypeName, descriptor.Version); + // If the descriptor already exists, replace it. But log a warning. - if (activityDescriptors.TryGetValue((descriptor.TypeName, descriptor.Version), out var existingDescriptor)) + if (activityDescriptors.TryGetValue(descriptorKey, out var existingDescriptor)) { // Remove the existing descriptor from the providerDescriptors collection. providerDescriptors.Remove(existingDescriptor); @@ -245,7 +244,8 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< logger.LogWarning("Activity descriptor {ActivityType} v{ActivityVersion} was already registered for tenant {TenantId}. Replacing with new descriptor", descriptor.TypeName, descriptor.Version, descriptor.TenantId); } - activityDescriptors[(descriptor.TypeName, descriptor.Version)] = descriptor; + activityDescriptors[descriptorKey] = descriptor; + UpdateLatestDescriptor(registry, descriptor); providerDescriptors.Add(descriptor); } @@ -254,6 +254,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< { _tenantRegistries.Clear(); _agnosticRegistry.ActivityDescriptors.Clear(); + _agnosticRegistry.LatestActivityDescriptors.Clear(); _agnosticRegistry.ProvidedActivityDescriptors.Clear(); } @@ -267,7 +268,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< && tenantRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var descriptors)) { foreach (var descriptor in descriptors.ToList()) - tenantRegistry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); + RemoveDescriptor(tenantRegistry, descriptor); tenantRegistry.ProvidedActivityDescriptors.TryRemove(providerType, out _); } @@ -276,7 +277,7 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< if (_agnosticRegistry.ProvidedActivityDescriptors.TryGetValue(providerType, out var agnosticDescriptors)) { foreach (var descriptor in agnosticDescriptors.ToList()) - _agnosticRegistry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out _); + RemoveDescriptor(_agnosticRegistry, descriptor); _agnosticRegistry.ProvidedActivityDescriptors.TryRemove(providerType, out _); } @@ -305,6 +306,35 @@ public class ActivityRegistry(IActivityDescriber activityDescriber, IEnumerable< return registry.ProvidedActivityDescriptors.GetOrAdd(providerType, _ => new List()); } + private static void UpdateLatestDescriptor(TenantRegistryData registry, ActivityDescriptor descriptor) + { + registry.LatestActivityDescriptors.AddOrUpdate( + descriptor.TypeName, + descriptor, + (_, latestDescriptor) => descriptor.Version >= latestDescriptor.Version ? descriptor : latestDescriptor); + } + + private static void RemoveDescriptor(TenantRegistryData registry, ActivityDescriptor descriptor) + { + if (!registry.ActivityDescriptors.TryRemove((descriptor.TypeName, descriptor.Version), out var removedDescriptor)) + return; + + if (registry.LatestActivityDescriptors.TryGetValue(removedDescriptor.TypeName, out var latestDescriptor) && latestDescriptor.Version == removedDescriptor.Version) + RecomputeLatestDescriptor(registry, removedDescriptor.TypeName); + } + + private static void RecomputeLatestDescriptor(TenantRegistryData registry, string typeName) + { + var latestDescriptor = registry.ActivityDescriptors.Values + .Where(x => x.TypeName == typeName) + .MaxBy(x => x.Version); + + if (latestDescriptor == null) + registry.LatestActivityDescriptors.TryRemove(typeName, out _); + else + registry.LatestActivityDescriptors[typeName] = latestDescriptor; + } + /// /// Normalizes tenant ID for grouping purposes. /// Converts null to "*" so that both null and "*" descriptors are grouped together, diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs index 0ae94391f..c32f0fb54 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs @@ -22,7 +22,7 @@ public class ActivityRegistryTests var activityDescriber = Substitute.For(); _logger = Substitute.For>(); _registry = new(activityDescriber, [], tenantAccessor, _logger); - + // Set default tenant for all tests tenantAccessor.TenantId.Returns(CurrentTenant); } @@ -114,6 +114,88 @@ public class ActivityRegistryTests Assert.Null(result); } + [Fact] + public void Find_ReturnsNextLatestVersion_WhenLatestDescriptorRemoved() + { + // Arrange + var v1 = CreateDescriptor(TestActivityType, 1, CurrentTenant); + var v2 = CreateDescriptor(TestActivityType, 2, CurrentTenant); + var v3 = CreateDescriptor(TestActivityType, 3, CurrentTenant); + RegisterDescriptors(v1, v2, v3); + + // Act + _registry.Remove(typeof(ActivityRegistry), v3); + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, CurrentTenant, 2); + } + + [Fact] + public void Find_KeepsLatestVersion_WhenNonLatestDescriptorRemoved() + { + // Arrange + var v1 = CreateDescriptor(TestActivityType, 1, CurrentTenant); + var v2 = CreateDescriptor(TestActivityType, 2, CurrentTenant); + var v3 = CreateDescriptor(TestActivityType, 3, CurrentTenant); + RegisterDescriptors(v1, v2, v3); + + // Act + _registry.Remove(typeof(ActivityRegistry), v1); + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, CurrentTenant, 3); + } + + [Fact] + public void Find_ReturnsNull_WhenProviderWithLatestDescriptorClearedAndNoDescriptorsRemain() + { + // Arrange + var descriptor = CreateDescriptor(TestActivityType, 1, CurrentTenant); + _registry.Add(typeof(Provider1), descriptor); + + // Act + _registry.ClearProvider(typeof(Provider1)); + var result = _registry.Find(TestActivityType); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Find_RecomputesLatestVersion_WhenProviderWithLatestDescriptorCleared() + { + // Arrange + var provider1V1 = CreateDescriptor(TestActivityType, 1, CurrentTenant); + var provider1V3 = CreateDescriptor(TestActivityType, 3, CurrentTenant); + var provider2V2 = CreateDescriptor(TestActivityType, 2, CurrentTenant); + _registry.Add(typeof(Provider1), provider1V1); + _registry.Add(typeof(Provider1), provider1V3); + _registry.Add(typeof(Provider2), provider2V2); + + // Act + _registry.ClearProvider(typeof(Provider1)); + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, CurrentTenant, 2); + } + + [Fact] + public void Find_ReturnsNull_WhenRegistryCleared() + { + // Arrange + RegisterDescriptors(CreateDescriptor(TestActivityType, 2, CurrentTenant)); + + // Act + _registry.Clear(); + var result = _registry.Find(TestActivityType); + + // Assert + Assert.Null(result); + } + [Fact] public void Find_IgnoresOtherTenantDescriptors_OnlyReturnsCurrentTenantOrAgnostic() { @@ -147,7 +229,7 @@ public class ActivityRegistryTests Description = "Test Activity 1", IsBrowsable = true }; - + var descriptor2 = new ActivityDescriptor { TypeName = "TestActivity2", @@ -157,18 +239,18 @@ public class ActivityRegistryTests Description = "Test Activity 2", IsBrowsable = true }; - + mockProvider.GetDescriptorsAsync(Arg.Any()) .Returns(new ValueTask>([descriptor1, descriptor2])); var providers = new[] { mockProvider }; - + // 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 _logger.DidNotReceive().Log( LogLevel.Warning, @@ -176,14 +258,14 @@ public class ActivityRegistryTests Arg.Is(v => v.ToString()!.Contains("was already registered")), Arg.Any(), Arg.Any>()); - + // 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() { @@ -198,9 +280,9 @@ public class ActivityRegistryTests Description = "Manually registered activity", IsBrowsable = true }; - + _registry.Register(manualDescriptor); - + // Create a provider descriptor var mockProvider = Substitute.For(); var providerDescriptor = new ActivityDescriptor @@ -212,21 +294,21 @@ public class ActivityRegistryTests Description = "Provider activity", IsBrowsable = true }; - + mockProvider.GetDescriptorsAsync(Arg.Any()) .Returns(new ValueTask>([providerDescriptor])); var providers = new[] { mockProvider }; - + // 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 _logger.DidNotReceive().Log( LogLevel.Warning, @@ -235,7 +317,7 @@ public class ActivityRegistryTests Arg.Any(), Arg.Any>()); } - + [Fact] public async Task RefreshDescriptorsAsync_LogsWarning_WhenDifferentProvidersRegisterSameActivity() { @@ -277,7 +359,32 @@ public class ActivityRegistryTests Arg.Any>()); } - + [Fact] + public async Task RefreshDescriptorsAsync_RecomputesLatestDescriptor_WhenProviderDropsLatestVersion() + { + // Arrange + var provider = new MutableProvider( + [ + CreateDescriptor(TestActivityType, 1, CurrentTenant), + CreateDescriptor(TestActivityType, 3, CurrentTenant) + ]); + + await _registry.RefreshDescriptorsAsync(provider); + + provider.Descriptors = + [ + CreateDescriptor(TestActivityType, 1, CurrentTenant) + ]; + + // Act + await _registry.RefreshDescriptorsAsync(provider); + var result = _registry.Find(TestActivityType); + + // Assert + AssertDescriptor(result, CurrentTenant, 1); + } + + private ActivityDescriptor CreateDescriptor(string typeName, int version, string? tenantId) => new() { @@ -308,5 +415,12 @@ public class ActivityRegistryTests { public ValueTask> GetDescriptorsAsync(CancellationToken cancellationToken = default) => new(descriptors); } - + + private sealed class MutableProvider(IEnumerable descriptors) : IActivityProvider + { + public IEnumerable Descriptors { get; set; } = descriptors; + + public ValueTask> GetDescriptorsAsync(CancellationToken cancellationToken = default) => new(Descriptors); + } + } \ No newline at end of file