# WorkFlow Engine — Detailed Analysis **Date:** 2026-03-01 **Scope:** `GB5Shared.WorkFlow.WorkFlowEngine` + all callers **Status:** Work-in-progress codebase review --- ## Architecture Overview The engine is structured in distinct layers: ``` Endpoint (HTTP) └── WorkFlowBLL → IWorkFlowBLL └── WorkFlowDAL → IWorkFlowDAL (action handling) └── WorkFlowEngine → IWorkFlowEngine (core logic) └── WorkFlowRunTime → IWorkFlowRunTime (DB access) BaseEntityAppService (universal save pipeline, injects IWorkFlowEngine directly) ↑ used by RoleBLL and all entity BLLs ``` ### Key Files | File | Role | |------|------| | `GB5Shared/WorkFlow/WorkFlowEngine/IWorkFlowEngine.cs` | Interface contract | | `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs` | Core engine (1017 lines) | | `GB5Shared/WorkFlow/WorkFlowRunTime/IWorkFlowRunTime.cs` | DB access interface | | `GB5Shared/WorkFlow/WorkFlowRunTime/WorkFlowRunTime.cs` | DB access implementation | | `GB5Shared/DTO/WorkFlow/DomainDTO.cs` | Domain models and enums | | `GB5Shared/EntityHandler/EventHandler.cs` | `BaseEntityAppService` — universal save pipeline | | `GB5Framework/FrameworkBLL/WorkFlow/WorkFlowBLL.cs` | BLL facade | | `GB5Framework/FrameworkDAL/CustomCode/WorkFlow/WorkFlowDAL.cs` | Action handling DAL | | `GB5Framework/FrameworkBLL/Role/RoleBLL.cs` | Example entity BLL using the engine | --- ## Call Flow ### Save (Entity Creation / Update) ``` HTTP POST //Save └── RoleBLL.SaveRole() └── BaseEntityAppService.ExecuteSaveAsync() ├── QualifierFacade.ValidateAsync() ├── WorkFlowEngine.CheckWorkFlowApplicability() ← evaluates conditions on DTO ├── persistFunc(tx) ← skipped in WIP mode └── WorkFlowEngine.StartWorkflowAsync() ← creates instance + tasks └── EnterStepAsync → CreateTasksForStepAsync ``` ### Approval Action ``` HTTP POST /WorkFlow/WorkFlowActions └── WorkFlowBLL.WorkFlowActions() └── WorkFlowDAL.Actions() ← opens own tx └── WorkFlowEngine.HandleActionsAsync() ← iterates actions └── HandleSingleAsync() ← per-task ├── GetTask → GetInstance → GetActiveDefinition ├── UpdateTask (status=Completed, ActionTaken) ├── InsertHistory ├── [Parallel] check pending siblings ├── Choose transition by action name match ├── UpdateInstance (new CurrentStepId / WorkflowStatus) ├── ProcessTenantWorkflows ← on final step only └── EnterStepAsync → CreateTasksForStepAsync ``` --- ## Domain Models (Key) ```csharp WorkflowDefinition // Master: workflow config per entity ├── List // Steps: Normal(1), ParallelGroup(0), Auto(2), System(3) └── List // Edges: trigger="Approve"/"Reject"/"Return" WorkflowInstance // Runtime: one per submitted object ├── EntityId, ObjectId, WorkflowId ├── CurrentStepId └── WorkflowStatus: Pending(0), Completed(1), Rejected(2), Returned(3) WorkflowTask // Inbox item per approver per step ├── AssignedToUserId / AssignedRoleId / AssignedUserGroupId ├── Status: Pending(0), Approved(1), Rejected(2), Completed(3) ├── ActionTaken: Submit(0), Approve(1), Reject(2), Return(3), Escalate(4) └── DueOn (calculated from SlaHours) WorkflowHistory // Immutable audit trail UserDelegation // Time-bounded delegation of user tasks ``` --- ## Critical Bugs ### BUG-1: `RoleBLL` injects `null!` for all three key dependencies — runtime crash **File:** `GB5Framework/FrameworkBLL/Role/RoleBLL.cs` lines 106–111 ```csharp var baseEntityAppService = new BaseEntityAppService( facade, null!, // your IWorkFlowEngine ← injected _IWorkFlowEngine is unused! null!, // your IOutBox ← injected _outBox is unused! null! // your IQueryExecutor ← injected _queryExecutor is unused! ); ``` **Impact:** `NullReferenceException` at runtime whenever `CheckWorkFlowApplicability`, `StartWorkflowAsync`, or `PublishEventAsync` is called. **Fix:** Pass `_IWorkFlowEngine`, `_outBox`, `_queryExecutor` instead of `null!`. --- ### BUG-2: Wrong parameter position — external transaction silently ignored **File:** `GB5Framework/FrameworkBLL/Role/RoleBLL.cs` lines 113–125 ```csharp await baseEntityAppService.ExecuteSaveAsync( EntityConstant.OBJECTROLE, EventTypeConstant.SAVEROLEEVENTTYPEID, roleDto, login, async tx => { ... }, transaction); // ← maps to Facts (6th param), NOT externalTransaction (7th param)! ``` **Signature** (`EventHandler.cs` lines 37–44): ```csharp public async Task ExecuteSaveAsync( int entityId, int eventTypeId, TDto dto, LoginDTO login, Func> persistFunc, Dictionary? Facts = null, // ← 6th DbTransaction? externalTransaction = null) // ← 7th ``` **Impact:** A second internal transaction is created; the caller's transaction is abandoned. Double-commit or orphaned transaction depending on runtime behaviour. **Fix:** Use named argument: `externalTransaction: transaction`. --- ### BUG-3: SQL Injection in `WorkFlowDAL` — string substitution into SQL **File:** `GB5Framework/FrameworkDAL/CustomCode/WorkFlow/WorkFlowDAL.cs` lines 168–172, 203–206 ```csharp SQL = SQL.Replace(":entityid", entityId.ToString()); SQL = SQL.Replace(":userid", loginDTO.UserId.ToString()); SQL = SQL.Replace(":fromdate", fromDate.HasValue ? $"'{fromDate.Value:yyyy-MM-dd}'" : "NULL"); SQL = SQL.Replace(":todate", toDate.HasValue ? $"'{toDate.Value:yyyy-MM-dd}'" : "NULL"); ``` Affects: `WorkflowHistory()` and `WorkflowStatus()`. **Impact:** SQL injection vulnerability. Date values are formatted directly into the SQL string. **Fix:** Use Dapper parameterized queries consistently (`new { fromdate = fromDate, ... }`). --- ## Serious Design Issues ### ISSUE-4: Dual + incomplete assignment strategy in `CreateTasksForStepAsync` **File:** `WorkFlowEngine.cs` lines 353–428 Two switch blocks on different fields — `StrategyType` (lines 353–394) and `AssignmentType` (lines 409–429) — with overlapping concerns. `AssignmentType` cases 1 (Role) and 2 (Pool) are commented out. It is unclear which field drives final assignment. **Recommendation:** Decide on one assignment model, remove or consolidate the other switch. --- ### ISSUE-5: `instance.Status` vs `instance.WorkflowStatus` mismatch **File:** `WorkFlowEngine.cs` line 895 (`AutoTransitionAsync`) ```csharp instance.Status = 2; // "Completed" ← wrong property ``` Every other location uses `instance.WorkflowStatus` (lines 669, 805, 810, 816, 838). **Impact:** Auto-transition completion may be silently discarded or write to the wrong column. --- ### ISSUE-6: Applicability check commented out inside `StartWorkflowAsync` **File:** `WorkFlowEngine.cs` lines 657–658 ```csharp //var applicable = await IsWorkflowApplicableAsync(ctx.EntityId, ctx.Facts, LoginDTO); //if (!applicable) return false; ``` `StartWorkflowAsync` is a public interface method. Any caller bypassing `BaseEntityAppService` can start a workflow without condition evaluation. **Recommendation:** Re-enable the check inside the engine (defence in depth). --- ### ISSUE-7: `ValidateWorkflowDefinition` not called at workflow start **File:** `WorkFlowEngine.cs` line 728 — called only in `HandleSingleAsync`. A misconfigured definition (multiple initial steps, steps without transitions) is detected only at the first approval action, not when the workflow is started. **Recommendation:** Also call `ValidateWorkflowDefinition` inside `StartWorkflowAsync`. --- ### ISSUE-8: Definition loaded by `EntityId` — breaks for in-flight instances when definition changes **File:** `WorkFlowEngine.cs` lines 724–726 ```csharp var def = await _IWorkFlowRunTime .GetActiveDefinitionAsync(instance.EntityId, login, dbTransaction) ``` `WorkflowInstance` stores `WorkflowId` at creation, but action processing fetches the **current active** definition by `EntityId`. If the definition is updated while an instance is in-flight, it is processed against the new definition — mismatched steps, broken transitions. **Recommendation:** Load definition by `instance.WorkflowId`, not `EntityId`. --- ### ISSUE-9: WIP mode exists but is not implemented **File:** `EventHandler.cs` line 78 WIP mode skips `persistFunc` but does nothing else. Commented-out `RoleBLL.cs` lines 171–174 confirms: `//Save Wip , Pending have to do`. This is a placeholder gap. --- ## Medium Issues ### ISSUE-10: `ActionTaken = 0` set at task creation — semantically wrong **File:** `WorkFlowEngine.cs` line 404 ```csharp ActionTaken = 0 // Submit — set at task CREATION before any approver acts ``` `ActionTaken` should be `null` until the task is completed. Pre-setting to `Submit(0)` corrupts reporting data. --- ### ISSUE-11: `Actions()` always returns `"Approved"` regardless of action **File:** `WorkFlowDAL.cs` line 45 ```csharp return "Approved"; ``` Returns `"Approved"` even for Reject or Return actions. API response is misleading. --- ### ISSUE-12: Cache is injected but unused — repeated definition DB reads **File:** `WorkFlowEngine.cs` lines 31–32 `_cache` and `CacheTtl` are declared. A caching implementation was written and then commented out (lines 611–628). `GetActiveDefinitionAsync` is called on every action with no cache hit. **Recommendation:** Re-enable definition caching with a short TTL. --- ### ISSUE-13: Stray unrelated imports in `WorkFlowEngine.cs` **File:** `WorkFlowEngine.cs` lines 8–20 ```csharp using DocumentFormat.OpenXml.Office2019.Excel.RichData2; using Google.Api; using Microsoft.JSInterop; ``` No relation to a workflow engine. Unnecessary assembly coupling. --- ### ISSUE-14: Namespace typo: `GB5Shared.EntitmyHandler` **File:** `EntityHandler/EventHandler.cs` line 13 ```csharp namespace GB5Shared.EntitmyHandler // should be GB5Shared.EntityHandler ``` --- ### ISSUE-15: `long` → `int` truncation in `ResolveEffectiveUserAsync` **File:** `WorkFlowEngine.cs` lines 447–462 ```csharp public async Task ResolveEffectiveUserAsync(long userId, ...) { return Convert.ToInt32(effectiveUserId); // overflow if userId > 2,147,483,647 } ``` Interface, implementation, and all callers should align on `long`. --- ### ISSUE-16: Zero logging throughout the engine Every catch block either re-throws bare or wraps in `InvalidOperationException` with no `ILogger` calls. For an approval workflow, every state transition, task assignment, and action should be traceable in production. --- ### ISSUE-17: Read queries wrapped in unnecessary transactions **File:** `WorkFlowDAL.cs` lines 58, 121, 158, 192 `WorkflowHistory`, `WorkflowStatus`, `WorkflowRequestToMe`, `RequestByMe` all call `BeginTransactionAsync` / `CommitAsync` for pure SELECT operations. Adds lock overhead. --- ## Issue Priority Summary | # | Issue | Severity | Impact | |---|-------|----------|--------| | BUG-1 | `null!` injection in RoleBLL | **Critical** | NullReferenceException in production | | BUG-2 | Transaction parameter mismatch | **Critical** | Silent transaction leak / orphan | | BUG-3 | SQL injection in WorkFlowDAL | **Critical** | Security vulnerability | | ISSUE-4 | Dual assignment strategy confusion | High | Incorrect task assignment | | ISSUE-5 | `Status` vs `WorkflowStatus` mismatch | High | Silent data corruption | | ISSUE-6 | Applicability check commented out in Start | High | Workflow starts without condition check | | ISSUE-7 | ValidateDefinition not called at Start | High | Bad configs caught too late | | ISSUE-8 | Definition loaded by EntityId not WorkflowId | High | In-flight instances break on definition update | | ISSUE-9 | WIP mode not implemented | Medium | Feature gap | | ISSUE-10 | ActionTaken=0 at task creation | Medium | Wrong reporting data | | ISSUE-11 | Always returns "Approved" | Medium | Misleading API response | | ISSUE-12 | Cache unused — repeated DB reads | Medium | Performance | | ISSUE-13 | Stray imports (OpenXml, Google.Api, JSInterop) | Low | Coupling/cleanliness | | ISSUE-14 | Namespace typo EntitmyHandler | Low | Maintainability | | ISSUE-15 | long→int truncation in ResolveEffectiveUser | Medium | Potential overflow | | ISSUE-16 | Zero logging | Medium | Undiagnosable production issues | | ISSUE-17 | Transactions on read queries | Low | Unnecessary DB lock overhead | --- ## Strengths (What Is Well Designed) - Clean `IWorkFlowEngine` interface — testable and replaceable - Transaction-aware throughout (`DbTransaction` passed explicitly) - `IUserDelegationResolver` properly separated as its own service - Condition evaluation engine (`EvaluateConditions`) supports AND/OR, reflection-based path traversal, multiple data types (INT, DECIMAL, STRING, DATE, BOOL) - Parallel step support (waits for all tasks in a `ParallelGroup` before transitioning) - Simulation support (`SimulateAsync`) enables dry-run without side effects - Graph export (`GetDefinitionGraph`) for tooling / UI visualization - SLA due-date calculation at task creation - Audit trail (`WorkflowHistory`) on every state change - DI registration is clean and scoped correctly in `Program.cs`