Optimize ActivityRegistry.Find to prefer tenant-specific descriptors without performance regression

src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs

@@ -35,7 +35,27 @@ public IEnumerable<ActivityDescriptor> ListByProvider(Type providerType)
     }
 
     /// <inheritdoc />
-    public ActivityDescriptor? Find(string type) => _activityDescriptors.Values.Where(x => (x.TenantId == tenantAccessor.TenantId || x.TenantId == null) && x.TypeName == type).MaxBy(x => x.Version);
+    public ActivityDescriptor? Find(string type)
+    {
+        var tenantId = tenantAccessor.TenantId;
+        ActivityDescriptor? tenantSpecific = null;
+        ActivityDescriptor? tenantAgnostic = null;
+
+        // Single-pass iteration to find both tenant-specific and tenant-agnostic descriptors
+        foreach (var descriptor in _activityDescriptors.Values)
+        {
+            if (descriptor.TypeName != type)
+                continue;
+
+            if (descriptor.TenantId == tenantId && (tenantSpecific == null || descriptor.Version > tenantSpecific.Version))
+                tenantSpecific = descriptor;
+            else if (descriptor.TenantId == null && (tenantAgnostic == null || descriptor.Version > tenantAgnostic.Version))
+                tenantAgnostic = descriptor;
+        }
+
+        // Prefer tenant-specific over tenant-agnostic
+        return tenantSpecific ?? tenantAgnostic;
+    }
 
     /// <inheritdoc />
     public ActivityDescriptor? Find(string type, int version) => _activityDescriptors.GetValueOrDefault((tenantAccessor.TenantId, type, version)) ?? _activityDescriptors.GetValueOrDefault((null, type, version));

test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityRegistryTests.cs

@@ -0,0 +1,160 @@
+using Elsa.Common.Multitenancy;
+using Elsa.Workflows;
+using Elsa.Workflows.Models;
+using Microsoft.Extensions.Logging;
+using NSubstitute;
+
+namespace Elsa.Workflows.Core.UnitTests.Services;
+
+/// <summary>
+/// Unit tests for ActivityRegistry, specifically testing multi-tenant descriptor resolution logic.
+/// </summary>
+public class ActivityRegistryTests
+{
+    private const string TestActivityType = "TestActivity";
+    private const string CurrentTenant = "tenant1";
+
+    private readonly ITenantAccessor _tenantAccessor;
+    private readonly IActivityDescriber _activityDescriber;
+    private readonly ILogger<ActivityRegistry> _logger;
+    private readonly ActivityRegistry _registry;
+
+    public ActivityRegistryTests()
+    {
+        _tenantAccessor = Substitute.For<ITenantAccessor>();
+        _activityDescriber = Substitute.For<IActivityDescriber>();
+        _logger = Substitute.For<ILogger<ActivityRegistry>>();
+        _registry = new ActivityRegistry(_activityDescriber, Array.Empty<IActivityDescriptorModifier>(), _tenantAccessor, _logger);
+
+        // Set default tenant for all tests
+        _tenantAccessor.TenantId.Returns(CurrentTenant);
+    }
+
+    private ActivityDescriptor CreateDescriptor(string typeName, int version, string? tenantId) =>
+        new()
+        {
+            TypeName = typeName,
+            Version = version,
+            TenantId = tenantId,
+            Kind = ActivityKind.Action
+        };
+
+    private void RegisterDescriptors(params ActivityDescriptor[] descriptors)
+    {
+        foreach (var descriptor in descriptors)
+            _registry.Register(descriptor);
+    }
+
+    private static void AssertDescriptor(ActivityDescriptor? result, string? expectedTenantId, int expectedVersion)
+    {
+        Assert.NotNull(result);
+        Assert.Equal(expectedTenantId, result.TenantId);
+        Assert.Equal(expectedVersion, result.Version);
+    }
+
+    [Fact]
+    public void Find_TenantSpecificPreferredOverTenantAgnostic_WhenBothExist()
+    {
+        // Arrange
+        var tenantSpecific = CreateDescriptor(TestActivityType, 1, CurrentTenant);
+        var tenantAgnostic = CreateDescriptor(TestActivityType, 2, null); // Higher version
+        RegisterDescriptors(tenantSpecific, tenantAgnostic);
+
+        // Act
+        var result = _registry.Find(TestActivityType);
+
+        // Assert - tenant-specific should be preferred even though it has a lower version
+        AssertDescriptor(result, CurrentTenant, 1);
+    }
+
+    [Fact]
+    public void Find_ReturnsTenantAgnostic_WhenNoTenantSpecificExists()
+    {
+        // Arrange
+        var tenantAgnostic = CreateDescriptor(TestActivityType, 1, null);
+        RegisterDescriptors(tenantAgnostic);
+
+        // Act
+        var result = _registry.Find(TestActivityType);
+
+        // Assert
+        AssertDescriptor(result, null, 1);
+    }
+
+    [Theory]
+    [InlineData(1, 2, 3, 3)] // Multiple versions, expect highest
+    [InlineData(3, 1, 2, 3)] // Out of order registration
+    [InlineData(1, 1, 1, 1)] // Same version multiple times
+    public void Find_ReturnsHighestVersionTenantSpecific_WhenMultipleTenantSpecificExist(int v1, int v2, int v3, int expectedVersion)
+    {
+        // Arrange
+        var descriptors = new[]
+        {
+            CreateDescriptor(TestActivityType, v1, CurrentTenant),
+            CreateDescriptor(TestActivityType, v2, CurrentTenant),
+            CreateDescriptor(TestActivityType, v3, CurrentTenant)
+        };
+        RegisterDescriptors(descriptors);
+
+        // Act
+        var result = _registry.Find(TestActivityType);
+
+        // Assert
+        AssertDescriptor(result, CurrentTenant, expectedVersion);
+    }
+
+    [Theory]
+    [InlineData(1, 2, 3, 3)] // Multiple versions, expect highest
+    [InlineData(3, 1, 2, 3)] // Out of order registration
+    [InlineData(1, 1, 1, 1)] // Same version multiple times
+    public void Find_ReturnsHighestVersionTenantAgnostic_WhenMultipleTenantAgnosticExist(int v1, int v2, int v3, int expectedVersion)
+    {
+        // Arrange
+        var descriptors = new[]
+        {
+            CreateDescriptor(TestActivityType, v1, null),
+            CreateDescriptor(TestActivityType, v2, null),
+            CreateDescriptor(TestActivityType, v3, null)
+        };
+        RegisterDescriptors(descriptors);
+
+        // Act
+        var result = _registry.Find(TestActivityType);
+
+        // Assert
+        AssertDescriptor(result, null, expectedVersion);
+    }
+
+    [Fact]
+    public void Find_ReturnsNull_WhenNoMatchingDescriptorsExist()
+    {
+        // Arrange
+        var otherDescriptor = CreateDescriptor("OtherActivity", 1, CurrentTenant);
+        RegisterDescriptors(otherDescriptor);
+
+        // Act
+        var result = _registry.Find("NonExistentActivity");
+
+        // Assert
+        Assert.Null(result);
+    }
+
+    [Fact]
+    public void Find_IgnoresOtherTenantDescriptors_OnlyReturnsCurrentTenantOrAgnostic()
+    {
+        // Arrange
+        var descriptors = new[]
+        {
+            CreateDescriptor(TestActivityType, 1, CurrentTenant),
+            CreateDescriptor(TestActivityType, 5, "tenant2"), // Much higher version but wrong tenant
+            CreateDescriptor(TestActivityType, 2, null)
+        };
+        RegisterDescriptors(descriptors);
+
+        // Act
+        var result = _registry.Find(TestActivityType);
+
+        // Assert - should return tenant1 descriptor (not tenant2, even though it has higher version)
+        AssertDescriptor(result, CurrentTenant, 1);
+    }
+}