# Scheduler Service — Deep Code Analysis

**Date:** 2026-03-02
**Analyst:** Claude Code
**Scope:** All scheduler sub-modules in `GB5Framework` — Scheduler, AutoScheduler, SchedulerTaskGenerator, JobDefine, SysJobRun, SystemJob

---

## Executive Summary

The Scheduler service is the most architecturally complex module in GB5Framework. It encompasses six interrelated sub-modules, two competing background service implementations, a Quartz.NET integration, a RabbitMQ publisher, Dapr pub/sub stubs, and EF Core DbContexts — sitting alongside Dapper, in direct violation of project standards. The most critical finding is that **all endpoints are unauthenticated**, **the transaction in `AutoSchedulerDAL.SaveAutoScheduler` is begun but never committed or rolled back** (data integrity failure), and **two duplicate BackgroundService loops** will double-publish every job if both are registered.

**Total issues found: 40**
Critical: 7 | High: 12 | Medium: 12 | Low: 9

---

## 1. VAPT / Security

### VAPT-01 — CRITICAL: AllowAnonymous on All Scheduler Endpoints
**Files:**
- [FrameworkSL/Endpoints/Scheduler/GetScheduler.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/Scheduler/GetScheduler.cs#L23)
- [FrameworkSL/Endpoints/Scheduler/SaveScheduler.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/Scheduler/SaveScheduler.cs#L24)
- [FrameworkSL/Endpoints/SchedulerTaskGenerator/GetSchedulerTasksEndpoint.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/GetSchedulerTasksEndpoint.cs#L32)
- [FrameworkSL/Endpoints/SchedulerTaskGenerator/PublishSchedulerTaskEndpoint.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/PublishSchedulerTaskEndpoint.cs#L31)

Every scheduler endpoint uses `AllowAnonymous()`. An unauthenticated attacker can:
- List all scheduled jobs and their configurations
- Create, update, delete any scheduler
- Mark arbitrary job executions as "in-progress" (disrupting execution tracking)
- Read full scheduler configurations including cron expressions, web service endpoints, and tenant data

```csharp
// WRONG — every scheduler endpoint
public override void Configure()
{
    Get("/Scheduler/GetScheduler");
    AllowAnonymous();  // ← no authentication whatsoever
}
```

**Fix:** Replace `AllowAnonymous()` with `Roles("Admin")` or appropriate role-based authorization.

---

### VAPT-02 — CRITICAL: No Tenant Filter in SchedulerQB Queries
**Files:**
- [FrameworkDAL/Query/Scheduler/SchedulerQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/Scheduler/SchedulerQB.cs#L11)

`GET_SCHEDULER`, `SAVE_SCHEDULER`, `UPDATE_SCHEDULER`, `DELETE_SCHEDULER` do not filter by `ClientId` or `DatabaseName`. `GET_SELECTLIST_SCHEDULER` has **no WHERE clause at all** — returns every scheduler record.

```sql
-- WRONG — no tenant filter, returns all tenants
SELECT ... FROM TSCHEDULER WHERE SCHEDULERID = @schedulerid;

-- WRONG — returns everything in the database
SELECT S.SCHEDULERID, S.SCHEDULERNAME FROM TSCHEDULER S;
```

**Fix:** Add `AND TENANTID = @TenantId` (or equivalent column) to every query.

---

### VAPT-03 — CRITICAL: LoadReadyJobs Has No Tenant Filter — Cross-Tenant Job Execution
**File:** [FrameworkDAL/Query/SchedulerTaskGenerator/SchedulerTaskGeneratorQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/SchedulerTaskGenerator/SchedulerTaskGeneratorQB.cs#L14)

The production-critical `LoadReadyJobs` query that drives the scheduler engine loads jobs from all tenants with only a STATUS filter:

```sql
-- WRONG — loads jobs from ALL tenants
FROM MJOBDEFINE j INNER JOIN TSCHEDULER s ...
WHERE j.STATUS = 1 AND s.STATUS = 1;
```

When the background service runs, it could execute jobs belonging to different tenants and publish them to the same queue. This is a **cross-tenant job execution vulnerability**.

---

### VAPT-04 — HIGH: HeaderLoginContextProvider Trusts Client-Supplied Headers
**File:** [FrameworkDAL/CustomCode/SchedulerTaskGenerator/HeaderLoginContextProvider.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/SchedulerTaskGenerator/HeaderLoginContextProvider.cs#L29)

The `HeaderLoginContextProvider` reads the `X-LoginDTO` header and deserializes the entire `LoginDTO` (including `UserId`, `ClientId`, `DatabaseName`) directly from a client-supplied HTTP header. An attacker can forge any identity:

```csharp
// WRONG — trusting client-supplied identity allows impersonation
if (ctx.Request.Headers.TryGetValue("X-LoginDTO", out var val))
{
    var login = JsonSerializer.Deserialize<LoginDTO>(val.ToString(), ...);
    if (login != null) return login;  // ← fully forged identity accepted
}
```

**Fix:** `LoginDTO` must always be reconstructed from verified server-side claims/tokens, never from client-provided headers.

---

### VAPT-05 — HIGH: `ex.Message` Leaked to HTTP Clients
**Files:**
- [FrameworkSL/Endpoints/SchedulerTaskGenerator/GetSchedulerTasksEndpoint.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/GetSchedulerTasksEndpoint.cs#L60)
- [FrameworkSL/Endpoints/SchedulerTaskGenerator/PublishSchedulerTaskEndpoint.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/PublishSchedulerTaskEndpoint.cs#L65)
- [FrameworkSL/Endpoints/Scheduler/GetScheduler.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/Scheduler/GetScheduler.cs#L52)
- [FrameworkSL/Endpoints/Scheduler/SaveScheduler.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/Scheduler/SaveScheduler.cs#L41)

All endpoints pass `ex.Message` directly into `CreateExceptionError`. SQL error messages (including table names, column names, constraint names) are sent to clients.

```csharp
// WRONG — internal details exposed
return await Response.CreateExceptionError<string>(ex, ..., ex.Message, 500);
```

---

### VAPT-06 — HIGH: AutoSchedulerDAL Post-Query Tenant Filter (IDOR Risk)
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L60)

`GetAutoScheduler` fetches ALL jobs matching a MenuId from any tenant, then filters in application code:

```csharp
// WRONG — cross-tenant rows fetched first, filtered after
foreach (var JobDefine in JobDefineList)
{
    if (JobDefine.BaseTenantId != LoginDTO.ClientId)
        continue;  // ← should be a SQL filter, not application filter
```

This leaks tenant data into application memory. If the application-level check is accidentally removed or bypassed, all tenant data is exposed.

---

### VAPT-07 — HIGH: RabbitMQ Default Credentials Used as Fallback
**File:** [FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs#L33)

```csharp
UserName = config["RabbitMQ:UserName"] ?? "guest",
Password = config["RabbitMQ:Password"] ?? "guest",  // ← default credentials
```

If `RabbitMQ:Password` is missing from config, the service connects with the default `guest:guest` credentials. In production, this will silently succeed against an unconfigured RabbitMQ instance and expose the message bus.

---

### VAPT-08 — HIGH: `null!` Passed as LoginDTO to DAL
**File:** [FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs#L25)

```csharp
// WRONG — null-forgiving operator overrides null safety
await _schedulerService.MarkTaskAsPublishedAsync(message.JobId, null!);
```

`MarkJobInProgressAsync` downstream calls `_queryExecutor.ExecuteAsync(login, ...)` where `login` is `null`. This will crash with `NullReferenceException` in the DAL, or if the executor doesn't validate it, will execute with no tenant context — touching any tenant's data.

---

## 2. Logic & Functionality

### LOGIC-01 — CRITICAL: Transaction Begun But Never Committed or Rolled Back
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L454)

`SaveAutoScheduler` calls `BeginTransactionAsync` but the `CommitAsync` and `RollbackAsync` calls are commented out:

```csharp
await _QueryExecutor.BeginTransactionAsync(LoginDTO);
transactionStarted = true;
// ... INSERT JobDefine, MAction, EventTypeAction, EventTypeActionDetail ...

//// 7️⃣ Commit Transaction
//if (transactionStarted)
//    await _QueryExecutor.CommitAsync();       // ← COMMENTED OUT

catch (Exception ex)
{
    //// 8️⃣ Rollback only if the transaction actually started
    //if (transactionStarted)
    //    await _QueryExecutor.RollbackAsync();  // ← COMMENTED OUT
    throw;
}
```

**Consequence:** Every `SaveAutoScheduler` call:
- Opens a transaction that is never committed (data may not persist, or auto-commits unreliably depending on connection settings)
- On failure, the transaction is never rolled back (holds DB locks, causes partial writes)

This is a **data corruption and lock starvation** issue in production.

---

### LOGIC-02 — CRITICAL: Dual BackgroundService Loop — Double Job Execution
**Files:**
- [FrameworkBLL/SchedulerTaskGenerator/SchedulerBackgroundService.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerBackgroundService.cs#L15)
- [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs#L32)

Two independent classes both extend `BackgroundService` and run the **same** `ExecuteReadyTasksAsync` loop with a 30-second poll:

- `SchedulerBackgroundService` → calls `taskManager.ExecuteReadyTasksAsync(login)`
- `SchedulerTaskManager` → also a `BackgroundService` that calls `ExecuteReadyTasksAsync(login)`

If both are registered in DI, every job is published **twice per cycle**. There is also a `SchedulerQuartzJob` (Quartz integration) that does the same thing. This could result in **triple job execution**.

---

### LOGIC-03 — HIGH: Hardcoded Database Names in Background Services
**Files:**
- [FrameworkBLL/SchedulerTaskGenerator/SchedulerBackgroundService.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerBackgroundService.cs#L96)
- [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs#L125)

Both background services hardcode the database name:

```csharp
// SchedulerBackgroundService
return new LoginDTO { DatabaseName = "DefaultDB", UserId = 0, UserName = "SchedulerService" };

// SchedulerTaskManager
var login = new LoginDTO { DatabaseName = "DEFAULT_DB" };
```

Even the values differ (`"DefaultDB"` vs `"DEFAULT_DB"`). In a multi-tenant ERP, the scheduler must iterate over all active tenants, not execute against a single hardcoded database.

---

### LOGIC-04 — HIGH: `ComputeNextRun` Silently Swallows Exceptions
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskServiceBLL.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskServiceBLL.cs#L154)

```csharp
catch
{
    // Scheduler must never crash  ← acceptable intent, wrong implementation
}
return null;
```

If a cron expression is malformed, the catch swallows the error silently. The job will never be scheduled again (returns `null` every cycle) with no alert, no log, no indication to operators. At minimum, `_logger.LogWarning` should be used here.

---

### LOGIC-05 — HIGH: `NormalizeQuartz` Missing `L` Replacement
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskServiceBLL.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskServiceBLL.cs#L162)

```csharp
private static string NormalizeQuartz(string quartz)
{
    ...
    return string.Join(" ", parts.Select(p => p.Replace("?", "*")));
    // ← "L" (last day of month) is NOT replaced — Cronos does not support L
}
```

Quartz cron expressions use `L` (e.g., `0 0 L * ?`). Cronos does not understand `L`, so these jobs will throw a `CronFormatException`, caught by the silent catch, and silently never execute. The commented-out `SchedulerTaskGeneratorBLL` correctly added `.Replace("L", "*")` but the active implementation omits it.

---

### LOGIC-06 — HIGH: AutoScheduler Update Path Commented Out — Always Inserts
**File:** [FrameworkBLL/AutoScheduler/AutoSchedulerBLL.cs](../gb5/GB5Framework/FrameworkBLL/AutoScheduler/AutoSchedulerBLL.cs#L38)

```csharp
// Insert/update branching is fully commented out — always calls Save
//if (initialId == 0) {
    await _AutoSchedulerDAL.SaveAutoScheduler(AutoSchedulerDTO, LoginDTO);
//} else {
//    await _AutoSchedulerDAL.UpdateAutoScheduler(AutoSchedulerDTO, LoginDTO);
//}
```

Any update request will attempt another INSERT, causing a primary key violation or duplicate record creation.

---

### LOGIC-07 — HIGH: N+1 Queries in AutoSchedulerDAL.GetAutoScheduler
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L53)

For each job returned, the code executes 3 additional queries:
1. `GET_SCHEDULER_BY_ID` — Scheduler lookup
2. `GET_CRITERIA_BY_ID` — CriteriaConfig lookup
3. `GET_MACTION_BY_JOBDEFINE` — MAction list

For N jobs → `1 + 3N` round trips to the database. For a menu with 10 jobs: 31 queries per request.

**Fix:** Use a single JOIN query or `QueryMultiMapAsync`.

---

### LOGIC-08 — HIGH: N+1 Queries in AutoSchedulerDAL.ContentFillingAsync
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L241)

Parses a comma-separated ID string and fires one DB query per ID:
```csharp
foreach (var id in ids)
{
    switch (deliveryType) {
        case 0: var employee = await QuerySingleAsync("...WHERE EMPLOYEEID = @EmployeeId", new { EmployeeId = id });
        // ...
    }
}
```

For a field with 20 recipients, this is 20 serial round trips. Use `WHERE EMPLOYEEID IN @Ids` and batch-fetch all IDs at once.

---

### LOGIC-09 — MEDIUM: `SchedulerTaskManager.LoadReadyTasksAsync` Creates a Nested Scope
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs#L55)

`ExecuteReadyTasksAsync` creates a scope at line 78, then calls `LoadReadyTasksAsync` which creates **another scope** at line 55:

```csharp
public async Task ExecuteReadyTasksAsync(LoginDTO login)
{
    using var scope = _scopeFactory.CreateScope();       // Scope A
    ...
    var tasks = await LoadReadyTasksAsync(login);        // Creates Scope B inside

public async Task<IReadOnlyList<SchedulerTaskDTO>> LoadReadyTasksAsync(LoginDTO login)
{
    using var scope = _scopeFactory.CreateScope();       // Scope B (nested)
```

Scoped services (like DAL) are resolved twice, causing double DB connections and loading tasks twice.

---

### LOGIC-10 — MEDIUM: `MarkJobInProgressAsync` Uses `DateTime.Now` (Local Time)
**File:** [FrameworkDAL/CustomCode/SchedulerTaskGenerator/SchedulerTaskDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/SchedulerTaskGenerator/SchedulerTaskDAL.cs#L97)

```csharp
await _queryExecutor.ExecuteAsync(login, SchedulerTaskGeneratorQB.MarkJobInProgress,
    new { JobId = jobId, RunOn = DateTime.Now });  // ← local time, not UTC
```

The scheduler logic uses `DateTime.UtcNow` converted to IST. Storing `DateTime.Now` (server local time) in `LASTRUNON` will cause cron next-run calculations to be incorrect when `job.LastRunOn` is read back and compared with IST time.

---

### LOGIC-11 — MEDIUM: `GET_SCHEDULER` Column Alias Mismatch
**File:** [FrameworkDAL/Query/Scheduler/SchedulerQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/Scheduler/SchedulerQB.cs#L16)

`GET_SCHEDULER` aliases `CRONEXPRESSION AS SchedulerCronExpression` but the `SchedulerDTO` property is `SchedulerCronExpression`. However, `SAVE_SCHEDULER` uses `@SchedulerCronExpression` as the parameter. This will work only if Dapper maps by name — if any column alias differs from the DTO property, rows will return `null` silently.

---

### LOGIC-12 — MEDIUM: Duplicate Column Alias in AutoSchedulerQB
**File:** [FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs#L54)

Both `ACTIONTYPE` and `TYPE` are aliased as `MActionType`:
```sql
ACTIONTYPE AS MActionType,   -- line 54
...
TYPE AS MActionType,          -- line 56: duplicate alias
```

In SQL Server, only the last alias takes effect. `ACTIONTYPE` is silently lost — always reads from `TYPE` column.

---

## 3. Performance

### PERF-01 — HIGH: RabbitMQ Channel + QueueDeclare on Every Publish
**File:** [FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs#L47)

```csharp
public async Task PublishAsync(string queue, object payload)
{
    await using var channel = await _connection.CreateChannelAsync();   // new channel every call
    await channel.QueueDeclareAsync(queue, durable: true, ...);         // declare every call
```

A new channel is created and the queue is declared on every single publish. `QueueDeclare` is idempotent but still adds a network round trip. Under high frequency (many tasks per scheduler cycle), this creates unnecessary channel churn.

**Fix:** Use a persistent channel or channel pool; call `QueueDeclare` once at startup.

---

### PERF-02 — HIGH: Sync-Over-Async in RabbitMqPublisher Constructor
**File:** [FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs#L41)

```csharp
// WRONG — blocks calling thread during DI resolution
_connection = factory.CreateConnectionAsync().GetAwaiter().GetResult();
```

Blocks the ASP.NET threadpool during app startup. Can cause deadlocks in certain async startup contexts and slows down service initialization.

---

### PERF-03 — MEDIUM: No CancellationToken Propagation in Scheduler Pipeline
**Files:** `ISchedulerBLL`, `ISchedulerDAL`, `ISchedulerTaskServiceBLL`, `ISchedulerTaskDAL`

None of the scheduler interfaces or implementations accept or propagate `CancellationToken`. When the host shuts down, `stoppingToken` is cancelled in the BackgroundService but the in-flight DB operations run to completion regardless.

---

### PERF-04 — MEDIUM: `SchedulerTaskServiceDAL` — Redundant Wrapper Without Interface
**File:** [FrameworkDAL/CustomCode/SchedulerTaskGenerator/SchedulerTaskServiceDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/SchedulerTaskGenerator/SchedulerTaskServiceDAL.cs#L11)

`SchedulerTaskServiceDAL` is a DAL class with no interface, not registered in DI, and duplicates the same logic already in `SchedulerTaskServiceBLL.GetReadyTasksAsync`. It is dead code.

---

### PERF-05 — LOW: `SchedulerDAL.GetSelectListScheduler` Passes `null!` as Parameters
**File:** [FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs#L103)

```csharp
var result = await _QueryExecutor.QueryAsync<SchedulerPicklistDTO>(LoginDTO, sql, null!);
```

Forcibly null-overrides the null check. Depending on the QueryExecutor implementation, this may cause an exception or silently skip parameter binding.

---

## 4. Memory

### MEM-01 — HIGH: RabbitMqPublisher Lifetime Risk
**File:** [FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGeneratorPublisher/DaprTaskPublisher.cs#L25)

`RabbitMqPublisher` implements `IAsyncDisposable` and holds a long-lived `IConnection`. If registered as `Scoped` (per-request), a new TCP connection to RabbitMQ is created per HTTP request and disposed at end of request — causing connection pool exhaustion.

**Fix:** Register as `Singleton` so one connection is shared, and ensure `DisposeAsync` is called on app shutdown.

---

### MEM-02 — MEDIUM: Two EF DbContexts Present Despite No-EF Policy
**Files:**
- [FrameworkDAL/CustomCode/SchedulerTaskGenerator/AppDbContext.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/SchedulerTaskGenerator/AppDbContext.cs)
- [FrameworkDAL/CustomCode/SchedulerTaskGenerator/CentralDbContext.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/SchedulerTaskGenerator/CentralDbContext.cs)

`Gb4DbContext` and `CentralDbContext` are defined and bring EF Core package dependencies into the DAL project. CLAUDE.md explicitly prohibits EF Core. These are only used in entirely commented-out code.

**Fix:** Remove both context files and their EF Core package references.

---

### MEM-03 — MEDIUM: SchedulerDTO Has 50+ Java-Era Backing Fields
**File:** [FrameworkDAL/DTO/Scheduler/SchedulerDTO.cs](../gb5/GB5Framework/FrameworkDAL/DTO/Scheduler/SchedulerDTO.cs)

`SchedulerDTO` uses private backing fields for every property:
```csharp
private int schedulerid;
public int SchedulerId { get { return schedulerid; } set { schedulerid = value; } }
```
55+ properties, each duplicated. This roughly doubles the per-object memory footprint. Use C# auto-properties (`public int SchedulerId { get; set; }`).

The only property with real setter logic is `SchedulerSelectedMonth` and `SchedulerSelectedDays` (bitmask expansion). These two are justified; the rest are not.

---

### MEM-04 — LOW: `GetAutoScheduler` Builds List Then Returns String
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L215)

The entire result set is materialized as `List<AutoSchedulerResponseDTO>` then serialized to JSON string in DAL. The caller (BLL/SL) then serializes it again for the HTTP response. Double serialization wastes CPU and creates two large string allocations.

---

## 5. Best Practices

### BP-01 — HIGH: `ValidationException` Wrapped in Base `Exception`
**File:** [FrameworkBLL/Scheduler/SchedulerBLL.cs](../gb5/GB5Framework/FrameworkBLL/Scheduler/SchedulerBLL.cs#L72)

```csharp
catch (ValidationException vex)
{
    throw new Exception(vex.Message);  // WRONG — loses type, stack trace
}
```

Wrapping `ValidationException` in `Exception` destroys the exception type. The caller cannot distinguish validation failures from system errors. Use `throw;` or re-throw with the original exception as inner.

---

### BP-02 — HIGH: `IValidation` Injected into DAL
**File:** [FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs#L25)

```csharp
public SchedulerDAL(IQueryExecutor QueryExecutor, IValidation IValidation)
```

DAL should only perform data retrieval and mutation. Validation belongs in BLL. Injecting `IValidation` into DAL violates the 3-tier layer contract.

---

### BP-03 — HIGH: `SchedulerTaskMessageConsumer` Injects Concrete Class
**File:** [FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs#L9)

```csharp
public SchedulerTaskMessageConsumer(SchedulerTaskServiceBLL _schedulerService)
// ↑ concrete class — not ISchedulerTaskServiceBLL
```

Bypasses the interface/DI pattern. Cannot be unit tested or swapped.

---

### BP-04 — MEDIUM: `JobDefineBLL` Entirely Commented Out
**File:** [FrameworkBLL/JobDefine/JobDefineBLL.cs](../gb5/GB5Framework/FrameworkBLL/JobDefine/JobDefineBLL.cs)

The entire `JobDefineBLL` implementation is commented out (all 108 lines). The endpoints for JobDefine exist, but the BLL implementation is dead code. Any JobDefine endpoint call will fail with DI resolution error at runtime.

---

### BP-05 — MEDIUM: `SchedulerTaskGeneratorBLL` Entirely Commented Out
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskGeneratorBLL.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskGeneratorBLL.cs)

The primary task generation BLL (cron parsing, `ComputeNextRunIst`, `MarkAsInProgressAsync`) is disabled. Active code in `SchedulerTaskServiceBLL` partially reimplements this without the `L` fix.

---

### BP-06 — MEDIUM: `MissingMethodException` Used for Validation Errors
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L44)

```csharp
throw new MissingMethodException("UNIB0172");
```

`MissingMethodException` is a CLR exception indicating a missing method signature. Using it for business validation is semantically wrong and misleading. Use `ArgumentException` or `ValidationException`.

---

### BP-07 — MEDIUM: Duplicate `using` Directive
**File:** [FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/Scheduler/SchedulerDAL.cs#L4)

```csharp
using FrameworkDAL.Query.Scheduler;
using FrameworkDAL.Query.Scheduler;  // duplicate
```

---

### BP-08 — MEDIUM: `SchedulerTelemetry.ActivitySource` Uses Wrong Source Name
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs#L20)

```csharp
public static readonly ActivitySource ActivitySource = new("FrameworkSL");
```

This is in the BLL project but uses `"FrameworkSL"` as the OpenTelemetry source name. The source name should match the actual service/component: `"FrameworkBLL.Scheduler"`.

---

### BP-09 — MEDIUM: `SysJobRunBLL.SaveSysJobRun` Has No AutoNumber for New Records
**File:** [FrameworkBLL/SysJobRun/SysJobRunBLL.cs](../gb5/GB5Framework/FrameworkBLL/SysJobRun/SysJobRunBLL.cs#L35)

```csharp
if (sysJobRunDTO.SysJobRunId == 0)
    await _sysJobRunDAL.SaveSysJobRun(sysJobRunDTO, loginDTO);  // ← no AutoNumber
```

Unlike `SchedulerBLL`, `SysJobRunBLL` does not generate an ID before saving. If the DAL's INSERT requires a non-zero PK, this will fail with a DB constraint error.

---

### BP-10 — LOW: `AutoSchedulerQB.INSERT_EVENTTYPEACTION` Double Semicolons
**File:** [FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs#L316)

```sql
VALUES (...);
;     -- ← trailing lone semicolon
```

Both `INSERT_EVENTTYPEACTION` and `INSERT_EVENTTYPEACTIONDETAIL` have a trailing lone `;` after the statement-ending `;`. While SQL Server tolerates this, it's a code hygiene issue.

---

### BP-11 — LOW: `AutoSchedulerQB.GET_NEXT_NUMBER` Uses Raw Stored Procedure Call
**File:** [FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs](../gb5/GB5Framework/FrameworkDAL/Query/AutoScheduler/AutoSchedulerQB.cs#L163)

```csharp
public const string GET_NEXT_NUMBER = @"EXEC dbo.sp_AutoScheduler @EntityName";
```

Uses a stored procedure for auto-numbering, while all other modules use `IAutoNumber.GetAutoNumber()`. Inconsistent ID generation strategy.

---

### BP-12 — LOW: All BLL `catch (Exception) { throw; }` Blocks Add No Value
**Files:** `SchedulerBLL`, `AutoSchedulerBLL`, `SysJobRunBLL`

```csharp
catch (Exception)
{
    throw;  // adds nothing — exception propagates identically without this block
}
```

These empty catch-rethrow blocks add indentation and maintenance cost with no benefit.

---

## 6. Architecture

### ARCH-01 — CRITICAL: Entire Codebase Duplicated (97 files × 2)
The scheduler module exists in both `GB5Framework/` and `gb5/GB5Framework/`. Both directories contain 97 identical files. There is no clear canonical source. Changes made in one location may not be reflected in the other, leading to divergence, conflicting fixes, and deployment confusion.

**Fix:** Establish a single source directory. Remove or archive the duplicate.

---

### ARCH-02 — HIGH: Mixed Data Access (EF Core + Dapper) Violates Project Rules
**Files:** `AppDbContext.cs`, `CentralDbContext.cs`

EF Core DbContexts are defined and registered in the DAL project alongside Dapper. CLAUDE.md explicitly states:
> "No Entity Framework. All data access uses Dapper with raw, parameterized SQL."

Even if the EF contexts are only in commented-out code, they introduce EF Core as a package dependency and create architectural confusion.

---

### ARCH-03 — HIGH: `AutoSchedulerDAL.SaveAutoScheduler` Performs Orchestration
**File:** [FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs](../gb5/GB5Framework/FrameworkDAL/CustomCode/AutoScheduler/AutoSchedulerDAL.cs#L322)

`SaveAutoScheduler` in the DAL:
- Validates input (checks `JobName` is not empty)
- Manages auto-number generation (`GetNextNumber` calls)
- Orchestrates inserts across 4 tables (JobDefine → MAction → EventTypeAction → EventTypeActionDetail)
- Contains business branching logic (if `JobDefineId == 0` → insert, else → update)

All of this belongs in BLL, not DAL. DAL should execute a single SQL operation per method call.

---

### ARCH-04 — HIGH: `SchedulerTaskManager` Violates Single Responsibility
**File:** [FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs](../gb5/GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs#L32)

`SchedulerTaskManager` simultaneously:
1. Implements `ISchedulerTaskManager` (business logic interface)
2. Extends `BackgroundService` (infrastructure concern)

The background loop and the execution orchestration should be two separate classes.

---

### ARCH-05 — MEDIUM: Three Competing Scheduler Execution Engines
The codebase has three independent implementations for the same responsibility (running scheduled tasks):
1. `SchedulerBackgroundService` (IHostedService + 30s poll)
2. `SchedulerTaskManager.ExecuteAsync` (BackgroundService + 30s poll)
3. `SchedulerQuartzJob` (Quartz.NET IJob)

No documentation or clear decision about which one is active in production. If all three are registered, jobs execute three times per cycle.

---

### ARCH-06 — MEDIUM: `SchedulerTaskMessageConsumer` in Wrong Layer/Folder
**File:** [FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs](../gb5/GB5Framework/FrameworkSL/Endpoints/SchedulerTaskGenerator/SchedulerTaskMessageConsumer.cs)

A MassTransit consumer is placed in `Endpoints/` alongside FastEndpoints. Consumers should be in a dedicated `Consumers/` folder.

---

## Summary Table

| ID | Category | Severity | Issue | File |
|----|----------|----------|-------|------|
| VAPT-01 | Security | CRITICAL | AllowAnonymous on all scheduler endpoints | GetScheduler.cs, SaveScheduler.cs, etc. |
| VAPT-02 | Security | CRITICAL | No tenant filter in SchedulerQB — cross-tenant data access | SchedulerQB.cs |
| VAPT-03 | Security | CRITICAL | LoadReadyJobs has no tenant filter — cross-tenant job execution | SchedulerTaskGeneratorQB.cs |
| VAPT-04 | Security | HIGH | Client-supplied X-LoginDTO header accepted as identity | HeaderLoginContextProvider.cs |
| VAPT-05 | Security | HIGH | `ex.Message` leaked to HTTP response | GetSchedulerTasksEndpoint.cs, etc. |
| VAPT-06 | Security | HIGH | Post-query tenant filter (IDOR risk) | AutoSchedulerDAL.cs |
| VAPT-07 | Security | HIGH | RabbitMQ default guest:guest credentials as fallback | DaprTaskPublisher.cs |
| VAPT-08 | Security | HIGH | `null!` passed as LoginDTO to DAL — no tenant context | SchedulerTaskMessageConsumer.cs |
| LOGIC-01 | Logic | CRITICAL | Transaction begun but never committed or rolled back | AutoSchedulerDAL.cs |
| LOGIC-02 | Logic | CRITICAL | Dual BackgroundService loops — double job execution | SchedulerBackgroundService.cs, SchedulerTaskManager.cs |
| LOGIC-03 | Logic | HIGH | Hardcoded "DefaultDB" / "DEFAULT_DB" in background services | SchedulerBackgroundService.cs, SchedulerTaskManager.cs |
| LOGIC-04 | Logic | HIGH | ComputeNextRun silently swallows parse errors — no logging | SchedulerTaskServiceBLL.cs |
| LOGIC-05 | Logic | HIGH | NormalizeQuartz missing `L` replacement — Quartz L-expressions silently never run | SchedulerTaskServiceBLL.cs |
| LOGIC-06 | Logic | HIGH | AutoScheduler update path commented out — always inserts | AutoSchedulerBLL.cs |
| LOGIC-07 | Logic | HIGH | N+1 queries in GetAutoScheduler | AutoSchedulerDAL.cs |
| LOGIC-08 | Logic | HIGH | N+1 queries in ContentFillingAsync | AutoSchedulerDAL.cs |
| LOGIC-09 | Logic | MEDIUM | Nested scope creation doubles DB connections | SchedulerTaskManager.cs |
| LOGIC-10 | Logic | MEDIUM | `DateTime.Now` (local) used in LASTRUNON — time zone mismatch | SchedulerTaskDAL.cs |
| LOGIC-11 | Logic | MEDIUM | GET_SCHEDULER column alias may not match DTO property | SchedulerQB.cs |
| LOGIC-12 | Logic | MEDIUM | Duplicate column alias `MActionType` in ACTIONTYPE and TYPE | AutoSchedulerQB.cs |
| PERF-01 | Performance | HIGH | New RabbitMQ channel + QueueDeclare per publish | DaprTaskPublisher.cs |
| PERF-02 | Performance | HIGH | Sync-over-async in RabbitMqPublisher constructor | DaprTaskPublisher.cs |
| PERF-03 | Performance | MEDIUM | No CancellationToken propagation in scheduler pipeline | ISchedulerBLL, ISchedulerDAL, etc. |
| PERF-04 | Performance | MEDIUM | SchedulerTaskServiceDAL is dead code without interface | SchedulerTaskServiceDAL.cs |
| PERF-05 | Performance | LOW | `null!` passed as SQL parameters | SchedulerDAL.cs |
| MEM-01 | Memory | HIGH | RabbitMqPublisher lifetime risk — new TCP connection per scope | DaprTaskPublisher.cs |
| MEM-02 | Memory | MEDIUM | Two EF DbContexts violate no-EF policy | AppDbContext.cs, CentralDbContext.cs |
| MEM-03 | Memory | MEDIUM | SchedulerDTO 50+ Java-era backing fields — double memory | SchedulerDTO.cs |
| MEM-04 | Memory | LOW | Double serialization: DAL → BLL → SL | AutoSchedulerDAL.cs |
| BP-01 | Best Practice | HIGH | ValidationException wrapped in Exception — loses type | SchedulerBLL.cs |
| BP-02 | Best Practice | HIGH | IValidation injected in DAL — belongs in BLL | SchedulerDAL.cs |
| BP-03 | Best Practice | HIGH | SchedulerTaskMessageConsumer injects concrete class | SchedulerTaskMessageConsumer.cs |
| BP-04 | Best Practice | MEDIUM | JobDefineBLL entirely commented out — DI failure at runtime | JobDefineBLL.cs |
| BP-05 | Best Practice | MEDIUM | SchedulerTaskGeneratorBLL entirely commented out | SchedulerTaskGeneratorBLL.cs |
| BP-06 | Best Practice | MEDIUM | MissingMethodException used for validation | AutoSchedulerDAL.cs |
| BP-07 | Best Practice | MEDIUM | Duplicate `using` directive | SchedulerDAL.cs |
| BP-08 | Best Practice | MEDIUM | ActivitySource uses wrong name `"FrameworkSL"` in BLL | SchedulerTaskManager.cs |
| BP-09 | Best Practice | MEDIUM | SysJobRunBLL.SaveSysJobRun has no AutoNumber | SysJobRunBLL.cs |
| BP-10 | Best Practice | LOW | Double semicolons in SQL constants | AutoSchedulerQB.cs |
| BP-11 | Best Practice | LOW | Inconsistent ID generation (stored proc vs AutoNumber) | AutoSchedulerQB.cs |
| BP-12 | Best Practice | LOW | Empty catch-rethrow blocks add no value | SchedulerBLL.cs, etc. |
| ARCH-01 | Architecture | CRITICAL | 97-file codebase duplicated in gb5/ and GB5Framework/ | All files |
| ARCH-02 | Architecture | HIGH | EF Core DbContexts violate no-EF project policy | AppDbContext.cs, CentralDbContext.cs |
| ARCH-03 | Architecture | HIGH | AutoSchedulerDAL.SaveAutoScheduler contains orchestration logic | AutoSchedulerDAL.cs |
| ARCH-04 | Architecture | HIGH | SchedulerTaskManager violates Single Responsibility | SchedulerTaskManager.cs |
| ARCH-05 | Architecture | MEDIUM | Three competing scheduler execution engines | SchedulerBackgroundService.cs, SchedulerTaskManager.cs, SchedulerQuartzJob.cs |
| ARCH-06 | Architecture | MEDIUM | SchedulerTaskMessageConsumer in wrong folder (Endpoints/) | SchedulerTaskMessageConsumer.cs |

---

## Recommended Fix Priority

### Immediate (Production-Breaking / Security)
1. **LOGIC-01** — Uncomment transaction `CommitAsync` and `RollbackAsync` in `AutoSchedulerDAL.SaveAutoScheduler`
2. **LOGIC-02** — Register only ONE BackgroundService engine; disable the others
3. **VAPT-01** — Add authentication to all scheduler endpoints
4. **VAPT-02 / VAPT-03** — Add `TENANTID = @TenantId` filter to all SchedulerQB and SchedulerTaskGeneratorQB queries
5. **VAPT-04** — Remove `HeaderLoginContextProvider` client-header-based identity; use verified claims instead
6. **VAPT-08** — Fix `null!` LoginDTO in `SchedulerTaskMessageConsumer`
7. **LOGIC-03** — Replace hardcoded `DatabaseName = "DefaultDB"` with real multi-tenant tenant enumeration

### Short Term (Data Integrity / Correctness)
8. **LOGIC-05** — Add `L` → `*` replacement in `NormalizeQuartz`
9. **LOGIC-04** — Add `_logger.LogWarning` in the `ComputeNextRun` silent catch block
10. **LOGIC-06** — Uncomment update path in `AutoSchedulerBLL.SaveAutoScheduler`
11. **LOGIC-10** — Replace `DateTime.Now` with `DateTime.UtcNow` in `MarkJobInProgressAsync`
12. **LOGIC-12** — Fix duplicate `MActionType` alias in `GET_MACTION_BY_JOBDEFINE` and `GET_MACTION_BY_EVENT_TYPE`
13. **BP-04** — Uncomment and restore `JobDefineBLL` implementation

### Medium Term (Performance / Security Hardening)
14. **LOGIC-07 / LOGIC-08** — Replace N+1 loops in `AutoSchedulerDAL` with JOIN-based queries
15. **PERF-02** — Replace sync-over-async in `RabbitMqPublisher` constructor with proper async initialization
16. **MEM-01** — Register `RabbitMqPublisher` as `Singleton`
17. **VAPT-05** — Replace `ex.Message` in all `CreateExceptionError` calls with generic error messages
18. **PERF-03** — Add `CancellationToken` to all BLL/DAL scheduler interfaces
19. **BP-01** — Fix `ValidationException` wrapping in `SchedulerBLL`
20. **BP-02** — Remove `IValidation` from `SchedulerDAL`

### Long Term (Architectural Cleanup)
21. **ARCH-01** — Consolidate duplicate codebase; establish single canonical directory
22. **ARCH-02 / MEM-02** — Remove `AppDbContext.cs` and `CentralDbContext.cs`
23. **ARCH-03** — Refactor `AutoSchedulerDAL.SaveAutoScheduler` orchestration into BLL
24. **ARCH-04** — Split `SchedulerTaskManager` into a BackgroundService + a plain BLL class
25. **ARCH-05** — Choose one scheduler engine (Quartz or BackgroundService), remove the others
26. **MEM-03** — Migrate `SchedulerDTO` to auto-properties (except `SelectedMonth`/`SelectedDays` setters)