Commit graph

4 commits

Author SHA1 Message Date
Sipke Schoorstra 9461fd020b
Add unit tests for StringExtensions.NormalizeRoute and refactor ActivityRegistryTests for cleaner setup and improved provider simulations. 2026-02-08 12:04:20 +01:00
Sipke Schoorstra cc2e331da4
Merge remote-tracking branch 'origin/release/3.6.0' 2026-02-08 10:52:28 +01:00
Sipke Schoorstra 7bc9035f5e
Implement null TenantId for tenant-agnostic entities (#7226)
* Add ADR for Null Tenant ID, implement tenant-agnostic logic

Introduce ADR-0009 to document the use of `null` for tenant-agnostic entities, enhancing multitenancy handling. Update multitenancy features across the codebase, including EF Core query filters and ActivityRegistry, to handle null as a tenant ID, ensuring tenant-agnostic entities are accessible across all tenants.

* Add multitenancy support in `ActivityTestFixture` by registering `ITenantAccessor`.

* Update src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove unused `using Elsa.Common.Multitenancy;` from WorkflowDefinitionActivityDescriptorFactory (#7230)

* Initial plan

* Remove unused using Elsa.Common.Multitenancy statement

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Optimize ActivityRegistry.Find to prefer tenant-specific descriptors without performance regression (#7227)

* Initial plan

* Optimize Find(string type) to prefer tenant-specific descriptors with single-pass iteration

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Apply review feedback: combine if statements and add comprehensive unit tests

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Refactor tests for DRYness using theories and helper methods

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Clean up extra whitespace in test file

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Fix default tenant data visibility leak by removing NullIfEmpty conversion (#7229)

* Initial plan

* Remove NullIfEmpty conversion to align with ADR-0008 and ADR-0009

- Updated ElsaDbContextBase to use empty string for default tenant
- Updated ApplyTenantId to stop converting empty string to null
- Updated TenantAwareDbContextFactory to preserve empty string for default tenant
- Updated Store.cs to preserve empty string for default tenant
- This ensures: null = tenant-agnostic (visible to all), "" = default tenant

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Add database migration to convert null TenantId to empty string for SqlServer

- Added Management migration to convert null to "" for WorkflowDefinitions and WorkflowInstances
- Added Runtime migration to convert null to "" for all runtime entities
- This ensures existing default tenant data is properly migrated per ADR-0008
- Note: Similar migrations needed for PostgreSql, MySql, Sqlite, and Oracle providers

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Clarify tenant handling logic in `ElsaDbContextBase` with new ADR references.

* Add tenant ID convention analysis documents and quick reference guide

* Implement tenant-agnostic functionality across modules

- Introduce `AgnosticTenantId` constant to manage tenant-agnostic entities.
- Modify entity handling logic to respect tenant-agnostic designations.
- Adjust workflow processing to include tenant-agnostic workflows.
- Update caching and activity descriptor logic to accommodate the `AgnosticTenantId`.

* Refactor tenant management and registry logic in `ActivityRegistry` for improved clarity and separation of tenant-specific and tenant-agnostic activity descriptors. Remove `TestTenantResolver` and update workflow definition handling for tenant support.

* Refactor `ActivityRegistry`: prioritize tenant-specific descriptors over tenant-agnostic and simplify descriptor retrieval logic.

* Improve async handling in `CommandHandlerInvokerMiddleware` to await tasks without blocking

* Update ADR to use asterisk as sentinel value for tenant-agnostic entities

Replace the previous convention of using `null` for tenant-agnostic entities with an asterisk (`"*"`) for improved clarity and system architecture. Updated ADR documentation, TOC, and dependency graph accordingly.

* Remove migration `ConvertNullTenantIdToEmptyString` and its associated designer file to clean up the codebase.

* Refactor `ActivityRegistry`: streamline activity descriptor removal logic and simplify tenant ID checks.

* Update Elsa.sln

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Simplify `RefreshDescriptorsAsync` by removing unnecessary local variable `currentTenantId`.

* Remove unused `currentTenantId` variable from `ActivityRegistry`.

* Add detailed semantic flow and key points to ADR 0009

Document the tenant ID flow from entity creation to query, emphasizing normalization and tenant-agnostic workflows. Update semantic flow diagrams and provide testing considerations for preserving `"*"` values in multi-tenant scenarios.

* Remove outdated Tenant ID Analysis and associated documents

* Add security-by-default design for tenant-agnostic entities in ADR

Enhance Architecture Decision Record to detail explicit requirements for tenant-agnostic database entities, highlighting differences between in-memory activity descriptors and persistent entities. Emphasize importance of setting `TenantId = "*"` to prevent accidental data leakage.

* Normalize tenant ID grouping in `ActivityRegistry` to unify null and agnostic IDs, reducing redundant processing.

* Refactor `SignalManager`: improve timeout handling and streamline signal task cancellation.

* Update src/modules/Elsa.Workflows.Core/Models/TenantRegistryData.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Refactor tests to use `Tenant.AgnosticTenantId` instead of `null` for tenant-agnostic descriptors.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Enhance logging in recurring tasks: add error handling and logger support to prevent crashes in scheduled timers.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
2026-02-02 10:59:01 +01:00
Copilot 40a7583f62
Fix spurious duplicate activity registry warnings during startup (#7183)
* Initial plan

* Fix: Remove bogus duplicate activity registry warnings during refresh

The ActivityRegistry.RefreshDescriptorsAsync() method was starting with existing descriptors, causing false warnings when intentionally repopulating the registry. Now starts fresh but preserves manual descriptors.

Co-authored-by: KnibbsyMan <23156317+KnibbsyMan@users.noreply.github.com>

* Add tests for ActivityRegistry refresh behavior

Added comprehensive tests to verify:
1. No warnings logged during intentional registry refresh
2. Manual descriptors preserved during refresh
3. True duplicates from different providers still logged

Co-authored-by: KnibbsyMan <23156317+KnibbsyMan@users.noreply.github.com>

* Apply code review feedback: track manual descriptors in providersDictionary and clarify comments

- Track _manualActivityDescriptors in providersDictionary under GetType() key for consistency
- Clarify comments to be more specific about GetType() provider and RegisterAsync(Type activityType) method
- Avoid duplicate entries in provider list when descriptor already preserved

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Minor style improvement: use Count > 0 instead of !Count.Equals(0)

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: KnibbsyMan <23156317+KnibbsyMan@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
2026-01-16 12:14:24 +01:00