Mirror/Erupe
1
0
Fork 0
mirror of https://github.com/Mezeporta/Erupe.git synced 2026-03-22 07:32:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

docs: update anti-patterns #9 to reflect current repo migration status

Browse Source 
All guild subsystem tables are now migrated into repo_guild.go.
21 repository files cover all major subsystems. Only 5 inline SQL
queries remain across 3 handler files. Updated summary table and
refactoring priority list to mark completed items.
This commit is contained in:
Houmgaor
2026-02-21 13:34:26 +01:00
parent f689770e94
commit 4fbc955774
1 changed files with 19 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
docs/anti-patterns.md
View File

@@ -240,9 +240,17 @@ The same table is queried in different handlers with slightly different column s
**Recommendation:** At minimum, define query constants. Ideally, introduce a repository layer that encapsulates all queries for a given entity.
**Status (substantial):** A `CharacterRepository` layer in `repo_character.go` now centralizes nearly all `characters` table access (27 methods). The initial PR introduced 9 core methods and rewired the 4 helpers + 6 direct queries (~70%). A second pass migrated ~56 additional inline queries across 13 handler files (cafe, misc, clients, plate, rengoku, mercenary, gacha, guild_board, guild_scout, data, items, house, session), bringing coverage to ~95% of character queries. Remaining unmigrated queries are cross-table JOINs (house+user_binary, mercenary+guild_characters, session auth), the bulk `CharacterSaveData` read/write, and a `handlers_commands.go` subquery through `users`.
**Status:** ~~Substantially fixed.~~ **Nearly complete.** 21 repository files now cover all major subsystems: character, guild, user, house, tower, festa, mail, rengoku, stamp, distribution, session, gacha, event, achievement, shop, cafe, goocoo, diva, misc, scenario, mercenary. All guild subsystem tables (`guild_posts`, `guild_adventures`, `guild_meals`, `guild_hunts`, `guild_hunts_claimed`, `guild_alliances`) are fully migrated into `repo_guild.go`.
A `GuildRepository` layer in `repo_guild.go` now centralizes all `guilds`, `guild_characters`, and `guild_applications` table access (~30 methods). All guild handler files plus cross-cutting callers (`handlers_festa.go`, `handlers_mail.go`, `handlers_house.go`, `handlers_mercenary.go`, `handlers_rengoku.go`) have been migrated. Remaining unmigrated guild subsystem tables: `guild_posts`, `guild_adventures`, `guild_meals`, `guild_hunts`, `guild_hunts_claimed`, `guild_alliances`. Next steps: column allowlist for SQL injection hardening.
Only 5 inline SQL queries remain across 3 handler files:
| File | Table | Query | Why unmigrated |
|------|-------|-------|----------------|
| `handlers_character.go:14` | `characters` | SELECT savedata by ID | Bulk `CharacterSaveData` read |
| `handlers_commands.go:107` | `bans` | UPSERT permanent ban | Admin command, no `bans` repo yet |
| `handlers_commands.go:113` | `bans` | UPSERT temporary ban | Admin command, no `bans` repo yet |
| `handlers_quest.go:352` | `event_quests` | SELECT all event quests for rotation | Event quest rotation logic |
| `handlers_quest.go:389` | `event_quests` | UPDATE rotation start_time | Event quest rotation logic |
---
@@ -306,7 +314,7 @@ Database operations use raw `database/sql` with PostgreSQL-specific syntax throu
| Severity | Anti-patterns |
|----------|--------------|
| **High** | ~~Missing ACK responses / softlocks (#2)~~ **Fixed**, no architectural layering (#3), tight DB coupling (#13) |
| **Medium** | ~~Magic numbers (#4)~~ **Fixed**, ~~inconsistent binary I/O (#5)~~ **Resolved**, Session god object (#6), ~~copy-paste handlers (#8)~~ **Fixed**, ~~raw SQL duplication (#9)~~ **Substantially fixed** (characters ~95%, guild core tables 100% migrated) |
| **Medium** | ~~Magic numbers (#4)~~ **Fixed**, ~~inconsistent binary I/O (#5)~~ **Resolved**, Session god object (#6), ~~copy-paste handlers (#8)~~ **Fixed**, ~~raw SQL duplication (#9)~~ **Nearly complete** (21 repos, 5 inline queries remain) |
| **Low** | God files (#1), ~~`init()` registration (#10)~~ **Fixed**, ~~inconsistent logging (#12)~~ **Fixed**, mutex granularity (#7), ~~panic-based flow (#11)~~ **Fixed** |
### Root Cause
@@ -315,9 +323,12 @@ Most of these anti-patterns stem from a single root cause: **the codebase grew o
### Recommended Refactoring Priority
1. **Add fail ACKs to silent error paths** — prevents player softlocks, ~70 existing doAckFail calls prove safety, low risk, can be done handler-by-handler
2. **Extract a character repository layer** — 152 queries across 26 files touch the `characters` table, highest SQL duplication
3. **Extract load/save helpers** — 38 handlers repeat the same ~10-15 line template, mechanical extraction
4. ~~**Extract a guild repository layer**~~ — **Done.** `repo_guild.go` with ~30 methods covers `guilds`, `guild_characters`, `guild_applications` tables
5. **Define protocol constants** — 1,052 hex literals with 174 unique values, improves documentation
1. ~~**Add fail ACKs to silent error paths**~~ — **Done** (see #2)
2. ~~**Extract a character repository layer**~~ — **Done.** `repo_character.go` covers ~95%+ of character queries
3. ~~**Extract load/save helpers**~~ — **Done.** `loadCharacterData`/`saveCharacterData` in `handlers_helpers.go`
4. ~~**Extract a guild repository layer**~~ — **Done.** `repo_guild.go` covers all guild tables including subsystem tables
5. ~~**Define protocol constants**~~ — **Done** (see #4)
6. ~~**Standardize binary I/O**~~ — already standardized on `byteframe`; remaining `encoding/binary` uses are correct (see #5)
7. **Migrate last 5 inline queries** — `handlers_character.go` (bulk save read), `handlers_commands.go` (bans UPSERT ×2), `handlers_quest.go` (event quest rotation ×2)
8. **Introduce repository interfaces** (#13) — enables mocking/testing without PostgreSQL
9. **Reduce mutex contention** (#7) — per-stage locks instead of global stage map lock