AzerothCore-RealmMaster/docs/DEAD_CODE_ANALYSIS.md

# Dead Code Analysis - Module SQL Staging

**Date:** 2025-11-16
**Context:** Phase 1 SQL Staging Implementation
**Status:** 🔍 Analysis Complete

---

## Executive Summary

After implementing runtime SQL staging in `stage-modules.sh`, we discovered that the original build-time SQL staging system is **no longer functional** and creates dead code. The build-time system stages SQL to module directories that AzerothCore's DBUpdater **never scans**.

**Key Finding:** AzerothCore's `DBUpdater` ONLY scans `/azerothcore/data/sql/updates/` (core directory), NOT `/azerothcore/modules/*/data/sql/updates/` (module directories).

---

## Dead Code Identified

### 1. **Build-Time SQL Staging in `manage-modules.sh`**

**File:** `scripts/bash/manage-modules.sh`
**Lines:** 480-557
**Functions:**
- `stage_module_sql_files()` (lines 480-551)
- `execute_module_sql()` (lines 553-557)

**What it does:**
- Called during `build.sh` (image build process)
- Stages SQL to `/azerothcore/modules/*/data/sql/updates/db_world/`
- Creates properly named SQL files with timestamps
- Intended to let AzerothCore's native updater find them

**Why it's dead:**
- AzerothCore's DBUpdater does NOT scan module directories
- Files created here are NEVER read or executed
- Confirmed by checking worldserver logs - no module SQL from this location

**Evidence:**
```bash
$ docker exec ac-worldserver ls /azerothcore/modules/mod-npc-beastmaster/data/sql/updates/db_world/
2025_11_15_00_npc_beastmaster.sql           # ❌ NEVER PROCESSED
2025_11_15_01_beastmaster_tames.sql         # ❌ NEVER PROCESSED
2025_11_15_02_beastmaster_tames_inserts.sql # ❌ NEVER PROCESSED

$ docker logs ac-worldserver 2>&1 | grep "2025_11_15_00_npc_beastmaster"
# NO RESULTS - File was never found by DBUpdater
```

---

### 2. **Stand-alone `stage-module-sql.sh` Script**

**File:** `scripts/bash/stage-module-sql.sh`
**Lines:** 297 lines total
**Purpose:** Called by `manage-modules.sh` to stage individual module SQL

**What it does:**
- Takes module path and target path as arguments
- Discovers SQL files in module
- Copies them with proper naming to target directory
- Validates SQL files (security checks)

**Why it's potentially dead:**
- Only called by `manage-modules.sh:527` (which is dead code)
- NOT called by the working runtime staging in `stage-modules.sh`
- The runtime staging does direct docker exec copying instead

**Current usage:**
- ✅ Called by `manage-modules.sh` (build-time - **DEAD**)
- ❌ NOT called by `stage-modules.sh` (runtime - **ACTIVE**)
- ✅ Referenced by `test-phase1-integration.sh` (test script)

**Status:** **Potentially useful** - Could be refactored for runtime use, but currently not in active code path

---

### 3. **SQL Manifest System**

**Files:**
- `scripts/python/modules.py` - Generates `.sql-manifest.json`
- `local-storage/modules/.sql-manifest.json` - Generated manifest file

**What it does:**
- Scans all modules during state generation
- Creates JSON manifest of all module SQL files
- Includes metadata: file paths, database types, checksums
- Used by `manage-modules.sh` to know which SQL to stage

**Why it's potentially dead:**
- Created during build process
- Consumed by `manage-modules.sh:stage_module_sql_files()` (dead code)
- NOT used by runtime staging in `stage-modules.sh`

**Current usage:**
- ✅ Generated by `modules.py generate` command
- ✅ Read by `manage-modules.sh` (build-time - **DEAD**)
- ❌ NOT used by `stage-modules.sh` (runtime - **ACTIVE**)
- ✅ Checked by `test-phase1-integration.sh` (test script)

**Status:** **Potentially useful** - Contains valuable metadata but not in active deployment path

---

### 4. **Test Files in `/tmp`**

**Files:**
- `/tmp/test-discover.sh` - Testing SQL discovery logic
- `/tmp/test-sql-staging.log` - Deployment test output

**Status:** **Temporary test files** - Should be cleaned up

---

## Working System (NOT Dead Code)

### Runtime SQL Staging in `stage-modules.sh`

**File:** `scripts/bash/stage-modules.sh`
**Lines:** 372-450
**Function:** `stage_module_sql_to_core()`

**What it does:**
1. Starts containers (including worldserver)
2. Waits for worldserver to be running
3. Uses `docker exec` to scan `/azerothcore/modules/*/data/sql/db-world/` (source files)
4. Copies SQL to `/azerothcore/data/sql/updates/db_world/` (core directory)
5. Renames with timestamp prefix: `YYYY_MM_DD_HHMMSS_{counter}_MODULE_{module_name}_{original}.sql`
6. AzerothCore's DBUpdater automatically processes them on startup

**Evidence of success:**
```bash
$ docker logs ac-worldserver 2>&1 | grep "Applying update" | grep MODULE
>> Applying update "2025_11_16_010945_0_MODULE_data_arac.sql" '025553C'...
>> Applying update "2025_11_16_010945_6_MODULE_data_beastmaster_tames.sql" '8C65AB2'...
# ✅ 46 MODULE SQL files successfully applied
```

**Status:** ✅ **ACTIVE AND WORKING**

---

## Architecture Comparison

### Build-Time Staging (DEAD)
```
build.sh
  └─> manage-modules.sh
       └─> stage_module_sql_files()
            └─> stage-module-sql.sh
                 └─> Copies SQL to: /azerothcore/modules/*/data/sql/updates/db_world/
                      └─> ❌ DBUpdater never scans this location
```

### Runtime Staging (ACTIVE)
```
deploy.sh
  └─> stage-modules.sh
       └─> stage_module_sql_to_core()
            └─> Direct docker exec copying
                 └─> Copies SQL to: /azerothcore/data/sql/updates/db_world/
                      └─> ✅ DBUpdater scans and processes this location
```

---

## Recommended Actions

### Option 1: Complete Removal (Aggressive)

**Remove:**
1. `stage_module_sql_files()` function from `manage-modules.sh` (lines 480-551)
2. `execute_module_sql()` function from `manage-modules.sh` (lines 553-557)
3. `scripts/bash/stage-module-sql.sh` (entire file - 297 lines)
4. SQL manifest generation from `modules.py`
5. Test files: `/tmp/test-discover.sh`, `/tmp/test-sql-staging.log`

**Update:**
1. `test-phase1-integration.sh` - Remove SQL manifest checks
2. `build.sh` - Remove call to SQL staging (if present)

**Pros:**
- Removes ~400 lines of dead code
- Simplifies architecture to single SQL staging approach
- Eliminates confusion about which system is active

**Cons:**
- Loses standalone `stage-module-sql.sh` tool (could be useful for manual operations)
- Loses SQL manifest metadata (though not currently used)

---

### Option 2: Refactor and Reuse (Conservative)

**Keep but refactor:**
1. Keep `stage-module-sql.sh` as a standalone tool for manual SQL staging
2. Update it to stage to core directory (`/azerothcore/data/sql/updates/`) instead of module directory
3. Document that it's a manual tool, not part of automated deployment
4. Keep SQL manifest as optional metadata for debugging

**Remove:**
1. `stage_module_sql_files()` and `execute_module_sql()` from `manage-modules.sh`
2. Automated call to staging during build process
3. Test files in `/tmp`

**Update:**
1. Document `stage-module-sql.sh` as manual/utility tool
2. Update its target directory logic to match runtime approach
3. Add clear comments explaining the architecture

**Pros:**
- Preserves utility scripts for manual operations
- Maintains SQL discovery/validation logic
- More flexible for future use cases

**Cons:**
- Still carries some dead weight
- More complex to maintain

---

### Option 3: Hybrid Approach (Recommended)

**Phase 1 - Immediate Cleanup:**
1. Remove `stage_module_sql_files()` and `execute_module_sql()` from `manage-modules.sh`
2. Remove automated SQL staging from build process
3. Delete test files from `/tmp`
4. Update `test-phase1-integration.sh` to test runtime staging instead

**Phase 2 - Refactor for Future:**
1. Keep `stage-module-sql.sh` but mark it clearly as "UTILITY - Not in deployment path"
2. Update it to stage to core directory for manual use cases
3. Keep SQL manifest generation but make it optional
4. Document the runtime staging approach as the canonical implementation

**Pros:**
- Immediate removal of dead code from active paths
- Preserves potentially useful utilities for future
- Clear documentation of what's active vs. utility
- Flexibility for future enhancements

**Cons:**
- Still maintains some unused code
- Requires clear documentation to prevent confusion

---

## Impact Analysis

### If We Remove All Dead Code

**Build Process:**
- ✅ No impact - build doesn't need SQL staging
- ✅ Modules still built correctly with C++ code
- ✅ Source SQL files still included in module directories

**Deployment Process:**
- ✅ No impact - runtime staging handles everything
- ✅ All 46 module SQL files still applied correctly
- ✅ AzerothCore's autoupdater still works

**Testing:**
- ⚠️ Need to update `test-phase1-integration.sh`
- ⚠️ Remove SQL manifest checks
- ✅ Can add runtime staging verification instead

**Future Development:**
- ⚠️ Loses SQL discovery logic (but it's reimplemented in runtime staging)
- ⚠️ Loses SQL validation logic (security checks for shell commands)
- ✅ Simpler architecture is easier to maintain

---

## Decision Required

**Question for User:** Which cleanup approach should we take?

1. **Aggressive** - Remove all dead code completely
2. **Conservative** - Refactor and keep as utilities
3. **Hybrid** - Remove from active paths, keep utilities documented

**Recommendation:** **Hybrid approach** - Remove dead code from active deployment/build paths while preserving utility scripts for future manual operations.

---

## Files Summary

| File | Lines | Status | Recommendation |
|------|-------|--------|----------------|
| `manage-modules.sh:480-557` | 78 | Dead Code | Remove functions |
| `stage-module-sql.sh` | 297 | Not in active path | Refactor as utility |
| `modules.py` (SQL manifest) | ~50 | Generated but unused | Keep as optional |
| `/tmp/test-discover.sh` | ~30 | Test file | Delete |
| `/tmp/test-sql-staging.log` | N/A | Test output | Delete |
| `test-phase1-integration.sh` | N/A | Needs update | Update to test runtime staging |
| `stage-modules.sh:372-450` | 78 | ✅ ACTIVE | Keep (working code) |

**Total Dead Code:** ~450 lines across multiple files

---

**Next Step:** Await user decision on cleanup approach, then proceed with selected option.