fix(modules): Correct SQL staging directory structure for AzerothCore

Fixed critical bug in stage-module-sql.sh that prevented module SQL from
being applied by AzerothCore's native updater.

Problem:
- Script was stripping 'db_' prefix from directory names
- Created updates/world/ instead of updates/db_world/
- AzerothCore's dbimport couldn't find the SQL files
- Result: [1146] table doesn't exist errors on deployment

Solution:
- Preserve full database type name (db_world, db_auth, etc.)
- Stage SQL to correct AzerothCore directory structure
- Verified against AC source code conventions

Impact:
- Critical for Phase 1 module SQL refactor
- Enables proper SQL tracking in updates table
- Prevents module initialization failures

Testing:
- Verified correct path: updates/db_world/ (not updates/world/)
- Confirmed against AzerothCore source structure
- Documented in docs/BUGFIX_SQL_STAGING_PATH.md

Related: Phase 1 implementation (PHASE1_CONTEXT.md)
This commit is contained in:
uprightbass360
2025-11-15 17:44:20 -05:00
parent 0d83f01995
commit 17101ae3c5
2 changed files with 450 additions and 0 deletions

View File

@@ -0,0 +1,153 @@
# Bug Fix: SQL Staging Path Incorrect
**Date:** 2025-11-15
**Status:** ✅ FIXED
**Severity:** Critical (Prevented module SQL from being applied)
---
## Summary
Fixed critical bug in `scripts/bash/stage-module-sql.sh` that prevented module SQL files from being staged in the correct AzerothCore directory structure, causing database schema errors and module failures.
---
## The Bug
### Symptom
Deployment failed with error:
```
[1146] Table 'acore_world.beastmaster_tames' doesn't exist
Your database structure is not up to date.
```
### Root Cause
**File:** `scripts/bash/stage-module-sql.sh`
**Lines:** 259-261
The script was incorrectly removing the `db_` prefix from database types when creating target directories:
```bash
# WRONG (before fix)
local target_subdir="${current_db#db_}" # Strips "db_" → "world"
local target_dir="$acore_path/data/sql/updates/$target_subdir"
# Result: /azerothcore/modules/mod-name/data/sql/updates/world/ ❌
```
**Problem:** AzerothCore's `dbimport` tool expects SQL in `updates/db_world/` not `updates/world/`
### Impact
- **All module SQL failed to apply** via AzerothCore's native updater
- SQL files staged to wrong directory (`updates/world/` instead of `updates/db_world/`)
- `dbimport` couldn't find the files
- Modules requiring SQL failed to initialize
- Database integrity checks failed on startup
---
## The Fix
### Code Change
**File:** `scripts/bash/stage-module-sql.sh`
**Lines:** 259-261
```bash
# CORRECT (after fix)
# AzerothCore expects db_world, db_auth, etc. (WITH db_ prefix)
local target_dir="$acore_path/data/sql/updates/$current_db"
# Result: /azerothcore/modules/mod-name/data/sql/updates/db_world/ ✅
```
### Verification
AzerothCore source confirms the correct structure:
```bash
$ find local-storage/source -type d -name "db_world"
local-storage/source/azerothcore-playerbots/data/sql/archive/db_world
local-storage/source/azerothcore-playerbots/data/sql/updates/db_world ← Correct!
local-storage/source/azerothcore-playerbots/data/sql/base/db_world
```
---
## Testing
### Before Fix
```bash
$ docker exec ac-worldserver ls /azerothcore/modules/mod-npc-beastmaster/data/sql/updates/
world/ ❌ Wrong directory name
```
### After Fix
```bash
$ docker exec ac-worldserver ls /azerothcore/modules/mod-npc-beastmaster/data/sql/updates/
db_world/ ✅ Correct!
$ ls /azerothcore/modules/mod-npc-beastmaster/data/sql/updates/db_world/
20251115_22_1_mod-npc-beastmaster_beastmaster_tames.sql ✅
20251115_22_2_mod-npc-beastmaster_beastmaster_tames_inserts.sql ✅
```
---
## Why This Bug Existed
The original implementation likely assumed AzerothCore used simple directory names (`world`, `auth`, `characters`) without the `db_` prefix. However, AzerothCore's actual schema uses:
| Database Type | Directory Name |
|--------------|----------------|
| World | `db_world` (not `world`) |
| Auth | `db_auth` (not `auth`) |
| Characters | `db_characters` (not `characters`) |
| Playerbots | `db_playerbots` (not `playerbots`) |
The bug was introduced when adding support for multiple database types and attempting to "normalize" the names by stripping the prefix.
---
## Impact on Phase 1 Implementation
This bug would have completely broken the Phase 1 module SQL refactor:
-**Goal:** Use AzerothCore's native updater for module SQL
-**Reality:** SQL staged to wrong location, updater couldn't find it
-**Result:** Module SQL never applied, databases incomplete
**Critical that we caught this before merging!**
---
## Lessons Learned
1. **Verify directory structure** against source code, not assumptions
2. **Test with real deployment** before considering feature complete
3. **Check AzerothCore conventions** - they use `db_` prefixes everywhere
4. **Integration testing is essential** - unit tests wouldn't have caught this
---
## Related Files
- `scripts/bash/stage-module-sql.sh` - Fixed (lines 259-261)
- `scripts/bash/manage-modules.sh` - Calls staging (working correctly)
- `scripts/python/modules.py` - SQL discovery (uses `db_*` correctly)
---
## Commit
**Fix:** Correct SQL staging directory structure for AzerothCore compatibility
Details:
- Fixed `stage-module-sql.sh` to preserve `db_` prefix in directory names
- Changed from `updates/world/` to `updates/db_world/` (correct format)
- Verified against AzerothCore source code directory structure
- Prevents [1146] table doesn't exist errors on deployment
**Type:** Bug Fix
**Severity:** Critical
**Impact:** Phase 1 implementation
**Testing:** Code review + path verification
---
**Status:** ✅ Fixed and ready to commit

297
scripts/bash/stage-module-sql.sh Executable file
View File

@@ -0,0 +1,297 @@
#!/bin/bash
# Stage Module SQL Files
# Copies module SQL to AzerothCore's native update directory structure
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
# Colors
GREEN='\033[0;32m'
YELLOW='\033[1;33m'
RED='\033[0;31m'
BLUE='\033[0;34m'
NC='\033[0m'
# Icons
ICON_SUCCESS="✅"
ICON_WARNING="⚠️"
ICON_ERROR="❌"
ICON_INFO=""
# Default values
MODULE_NAME=""
MODULE_PATH=""
ACORE_PATH=""
MANIFEST_PATH=""
DRY_RUN=0
usage() {
cat <<'EOF'
Usage: ./stage-module-sql.sh [options]
Stage module SQL files into AzerothCore's native update directory structure.
Options:
--module-name NAME Module name (e.g., mod-aoe-loot)
--module-path PATH Path to module repository
--acore-path PATH Path to AzerothCore modules directory
--manifest PATH Path to SQL manifest JSON (optional)
--dry-run Show what would be staged without doing it
-h, --help Show this help
Examples:
./stage-module-sql.sh \
--module-name mod-aoe-loot \
--module-path /staging/mod-aoe-loot \
--acore-path /azerothcore/modules/mod-aoe-loot
EOF
}
# Parse arguments
while [[ $# -gt 0 ]]; do
case "$1" in
--module-name) MODULE_NAME="$2"; shift 2;;
--module-path) MODULE_PATH="$2"; shift 2;;
--acore-path) ACORE_PATH="$2"; shift 2;;
--manifest) MANIFEST_PATH="$2"; shift 2;;
--dry-run) DRY_RUN=1; shift;;
-h|--help) usage; exit 0;;
*) echo "Unknown option: $1"; usage; exit 1;;
esac
done
# Validate arguments
if [ -z "$MODULE_NAME" ] || [ -z "$MODULE_PATH" ] || [ -z "$ACORE_PATH" ]; then
echo -e "${RED}${ICON_ERROR} Missing required arguments${NC}"
usage
exit 1
fi
if [ ! -d "$MODULE_PATH" ]; then
echo -e "${RED}${ICON_ERROR} Module path does not exist: $MODULE_PATH${NC}"
exit 1
fi
# Logging functions
info() {
echo -e "${BLUE}${ICON_INFO}${NC} $*"
}
ok() {
echo -e "${GREEN}${ICON_SUCCESS}${NC} $*"
}
warn() {
echo -e "${YELLOW}${ICON_WARNING}${NC} $*"
}
err() {
echo -e "${RED}${ICON_ERROR}${NC} $*"
}
# Generate timestamp-based filename
generate_sql_timestamp() {
# Format: YYYYMMDD_HH
# Uses current time to ensure unique, sequential naming
date +"%Y%m%d_%H"
}
# Validate SQL file
validate_sql_file() {
local sql_file="$1"
if [ ! -f "$sql_file" ]; then
return 1
fi
# Basic validation - file should not be empty
if [ ! -s "$sql_file" ]; then
warn "SQL file is empty: $(basename "$sql_file")"
return 1
fi
# Check for shell commands (security check)
if grep -qE '^\s*(system|exec|shell)' "$sql_file"; then
err "SQL file contains suspicious shell commands: $(basename "$sql_file")"
return 1
fi
return 0
}
# Discover SQL files in module
discover_module_sql() {
local module_path="$1"
local sql_base="$module_path/data/sql"
if [ ! -d "$sql_base" ]; then
# No SQL directory, not an error
return 0
fi
# Search in base/, updates/, and custom/ directories
local -A sql_files
# Support both underscore (db_world) and hyphen (db-world) naming conventions
local -A db_variants=(
["db_auth"]="db_auth db-auth"
["db_world"]="db_world db-world"
["db_characters"]="db_characters db-characters"
["db_playerbots"]="db_playerbots db-playerbots"
)
for canonical_type in db_auth db_world db_characters db_playerbots; do
for variant in ${db_variants[$canonical_type]}; do
# Check base/
if [ -d "$sql_base/base/$variant" ]; then
while IFS= read -r -d '' file; do
sql_files["$canonical_type"]+="$file"$'\n'
done < <(find "$sql_base/base/$variant" -name "*.sql" -type f -print0 2>/dev/null)
fi
# Check updates/
if [ -d "$sql_base/updates/$variant" ]; then
while IFS= read -r -d '' file; do
sql_files["$canonical_type"]+="$file"$'\n'
done < <(find "$sql_base/updates/$variant" -name "*.sql" -type f -print0 2>/dev/null)
fi
# Check custom/
if [ -d "$sql_base/custom/$variant" ]; then
while IFS= read -r -d '' file; do
sql_files["$canonical_type"]+="$file"$'\n'
done < <(find "$sql_base/custom/$variant" -name "*.sql" -type f -print0 2>/dev/null)
fi
# ALSO check direct db-type directories (legacy format used by many modules)
if [ -d "$sql_base/$variant" ]; then
while IFS= read -r -d '' file; do
sql_files["$canonical_type"]+="$file"$'\n'
done < <(find "$sql_base/$variant" -name "*.sql" -type f -print0 2>/dev/null)
fi
done
done
# Print discovered files
for db_type in "${!sql_files[@]}"; do
echo "$db_type"
echo "${sql_files[$db_type]}"
done
}
# Stage single SQL file
stage_sql_file() {
local source_file="$1"
local target_dir="$2"
local module_name="$3"
local counter="$4"
# Validate source file
if ! validate_sql_file "$source_file"; then
return 1
fi
# Generate target filename
local timestamp
timestamp=$(generate_sql_timestamp)
local basename
basename=$(basename "$source_file" .sql)
local target_file="$target_dir/${timestamp}_${counter}_${module_name}_${basename}.sql"
# Create target directory
if [ "$DRY_RUN" = "0" ]; then
mkdir -p "$target_dir"
fi
# Copy file
if [ "$DRY_RUN" = "1" ]; then
info "Would stage: $(basename "$source_file") -> $(basename "$target_file")"
else
if cp "$source_file" "$target_file"; then
ok "Staged: $(basename "$target_file")"
else
err "Failed to stage: $(basename "$source_file")"
return 1
fi
fi
return 0
}
# Stage all SQL for module
stage_module_sql() {
local module_name="$1"
local module_path="$2"
local acore_path="$3"
info "Staging SQL for module: $module_name"
# Discover SQL files
local sql_discovery
sql_discovery=$(discover_module_sql "$module_path")
if [ -z "$sql_discovery" ]; then
info "No SQL files found in module"
return 0
fi
# Parse discovery output
local current_db=""
local counter=1
local staged_count=0
while IFS= read -r line; do
if [ -z "$line" ]; then
continue
fi
# Check if this is a database type line
if [[ "$line" =~ ^db_(auth|world|characters|playerbots)$ ]]; then
current_db="$line"
counter=1
continue
fi
# This is a file path
if [ -n "$current_db" ] && [ -f "$line" ]; then
# AzerothCore expects db_world, db_auth, etc. (WITH db_ prefix)
local target_dir="$acore_path/data/sql/updates/$current_db"
if stage_sql_file "$line" "$target_dir" "$module_name" "$counter"; then
((staged_count++))
((counter++))
fi
fi
done <<< "$sql_discovery"
if [ "$staged_count" -gt 0 ]; then
ok "Staged $staged_count SQL file(s) for $module_name"
else
warn "No SQL files staged for $module_name"
fi
return 0
}
# Main execution
main() {
echo
info "Module SQL Staging"
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
echo
if [ "$DRY_RUN" = "1" ]; then
warn "DRY RUN MODE - No files will be modified"
echo
fi
stage_module_sql "$MODULE_NAME" "$MODULE_PATH" "$ACORE_PATH"
echo
ok "SQL staging complete"
echo
}
main "$@"