fix(channelserver): post-RC1 stabilization sprint

Fix rasta_id=0 overwriting NULL in SaveMercenary, which prevented
game state saving for characters without a mercenary (#163).

Also includes:
- CHANGELOG updated with all 10 post-RC1 commits
- Setup wizard fmt.Printf replaced with zap structured logging
- technical-debt.md updated with 6 newly completed items
- Scenario binary format documented (docs/scenario-format.md)
- Tests: alliance nil-guard (#171), handler dispatch table,
  migrations (sorted/SQL/baseline), setup wizard (10 tests),
  protbot protocol sign/entrance/channel (23 tests)
This commit is contained in:
Houmgaor
2026-03-05 16:39:15 +01:00
parent 10ac803a45
commit 03adb21e99
13 changed files with 1314 additions and 6 deletions

View File

@@ -505,3 +505,49 @@ func TestOperateJoint_NilAlliance(t *testing.T) {
t.Error("No response packet queued — would softlock the client")
}
}
// --- scanAllianceWithGuilds nil guild tests (issue #171) ---
func TestInfoJoint_MissingSubGuild1(t *testing.T) {
// Verify that GetAllianceByID returns an error when sub guild 1 references
// a non-existent guild (nil return from GetByID). This is the scenario from
// issue #171 — a deleted guild causes a nil dereference in scanAllianceWithGuilds.
server := createMockServer()
guildMock := &mockGuildRepo{
// GetAllianceByID returns an error for missing guilds because
// scanAllianceWithGuilds calls GetByID for each sub guild.
// With guild=nil and SubGuild1ID > 0, GetByID returns nil,
// and scanAllianceWithGuilds should return an error rather than panic.
getAllianceErr: errNotFound,
}
server.guildRepo = guildMock
session := createMockSession(1, server)
pkt := &mhfpacket.MsgMhfInfoJoint{AckHandle: 100, AllianceID: 5}
handleMsgMhfInfoJoint(session, pkt)
// Handler should send a response even on error (not softlock)
select {
case <-session.sendPackets:
default:
t.Error("No response packet queued — would softlock the client")
}
}
func TestInfoJoint_MissingSubGuild2(t *testing.T) {
server := createMockServer()
guildMock := &mockGuildRepo{
getAllianceErr: errNotFound,
}
server.guildRepo = guildMock
session := createMockSession(1, server)
pkt := &mhfpacket.MsgMhfInfoJoint{AckHandle: 100, AllianceID: 6}
handleMsgMhfInfoJoint(session, pkt)
select {
case <-session.sendPackets:
default:
t.Error("No response packet queued — would softlock the client")
}
}

View File

@@ -194,7 +194,12 @@ func handleMsgMhfSaveMercenary(s *Session, p mhfpacket.MHFPacket) {
dumpSaveData(s, pkt.MercData, "mercenary")
if len(pkt.MercData) >= 4 {
temp := byteframe.NewByteFrameFromBytes(pkt.MercData)
if err := s.server.charRepo.SaveMercenary(s.charID, pkt.MercData, temp.ReadUint32()); err != nil {
rastaID := temp.ReadUint32()
if rastaID == 0 {
s.logger.Warn("Mercenary save with rasta_id=0, preserving existing value",
zap.Uint32("charID", s.charID))
}
if err := s.server.charRepo.SaveMercenary(s.charID, pkt.MercData, rastaID); err != nil {
s.logger.Error("Failed to save mercenary data", zap.Error(err))
}
}

View File

@@ -0,0 +1,59 @@
package channelserver
import (
"testing"
"erupe-ce/network"
)
func TestBuildHandlerTable_EntryCount(t *testing.T) {
table := buildHandlerTable()
// handlers_table.go has exactly 432 entries (one per packet ID).
// This test catches accidental deletions or duplicates.
const expectedCount = 432
if len(table) != expectedCount {
t.Errorf("handler table has %d entries, want %d", len(table), expectedCount)
}
}
func TestBuildHandlerTable_CriticalOpcodes(t *testing.T) {
table := buildHandlerTable()
critical := []struct {
name string
id network.PacketID
}{
{"MSG_SYS_LOGIN", network.MSG_SYS_LOGIN},
{"MSG_SYS_LOGOUT", network.MSG_SYS_LOGOUT},
{"MSG_SYS_PING", network.MSG_SYS_PING},
{"MSG_SYS_ACK", network.MSG_SYS_ACK},
{"MSG_SYS_CAST_BINARY", network.MSG_SYS_CAST_BINARY},
{"MSG_SYS_ENTER_STAGE", network.MSG_SYS_ENTER_STAGE},
{"MSG_SYS_LEAVE_STAGE", network.MSG_SYS_LEAVE_STAGE},
{"MSG_MHF_SAVEDATA", network.MSG_MHF_SAVEDATA},
{"MSG_MHF_LOADDATA", network.MSG_MHF_LOADDATA},
{"MSG_MHF_ENUMERATE_QUEST", network.MSG_MHF_ENUMERATE_QUEST},
{"MSG_MHF_CREATE_GUILD", network.MSG_MHF_CREATE_GUILD},
{"MSG_MHF_INFO_GUILD", network.MSG_MHF_INFO_GUILD},
{"MSG_MHF_GET_ACHIEVEMENT", network.MSG_MHF_GET_ACHIEVEMENT},
{"MSG_MHF_PLAY_NORMAL_GACHA", network.MSG_MHF_PLAY_NORMAL_GACHA},
{"MSG_MHF_SEND_MAIL", network.MSG_MHF_SEND_MAIL},
{"MSG_MHF_SAVE_RENGOKU_DATA", network.MSG_MHF_SAVE_RENGOKU_DATA},
{"MSG_MHF_LOAD_RENGOKU_DATA", network.MSG_MHF_LOAD_RENGOKU_DATA},
}
for _, tc := range critical {
if _, ok := table[tc.id]; !ok {
t.Errorf("critical opcode %s (0x%04X) is not mapped in handler table", tc.name, uint16(tc.id))
}
}
}
func TestBuildHandlerTable_NoNilHandlers(t *testing.T) {
table := buildHandlerTable()
for id, handler := range table {
if handler == nil {
t.Errorf("handler for opcode 0x%04X is nil", uint16(id))
}
}
}

View File

@@ -193,8 +193,15 @@ func (r *CharacterRepository) ReadGuildPostChecked(charID uint32) (time.Time, er
return t, err
}
// SaveMercenary updates savemercenary and rasta_id atomically.
// SaveMercenary updates savemercenary and optionally rasta_id.
// When rastaID is 0, only the mercenary blob is saved — the existing rasta_id
// (typically NULL for characters without a mercenary) is preserved. Writing 0
// would pollute GetMercenaryLoans queries that match on pact_id.
func (r *CharacterRepository) SaveMercenary(charID uint32, data []byte, rastaID uint32) error {
if rastaID == 0 {
_, err := r.db.Exec("UPDATE characters SET savemercenary=$1 WHERE id=$2", data, charID)
return err
}
_, err := r.db.Exec("UPDATE characters SET savemercenary=$1, rasta_id=$2 WHERE id=$3", data, rastaID, charID)
return err
}

View File

@@ -208,3 +208,72 @@ func TestReadMigrations(t *testing.T) {
t.Errorf("first migration filename = %q, want 0001_init.sql", migrations[0].filename)
}
}
func TestReadMigrations_Sorted(t *testing.T) {
migrations, err := readMigrations()
if err != nil {
t.Fatalf("readMigrations failed: %v", err)
}
for i := 1; i < len(migrations); i++ {
if migrations[i].version <= migrations[i-1].version {
t.Errorf("migrations not sorted: version %d at index %d follows version %d at index %d",
migrations[i].version, i, migrations[i-1].version, i-1)
}
}
}
func TestReadMigrations_AllHaveSQL(t *testing.T) {
migrations, err := readMigrations()
if err != nil {
t.Fatalf("readMigrations failed: %v", err)
}
for _, m := range migrations {
if m.sql == "" {
t.Errorf("migration %s has empty SQL", m.filename)
}
}
}
func TestReadMigrations_BaselineIsLargest(t *testing.T) {
migrations, err := readMigrations()
if err != nil {
t.Fatalf("readMigrations failed: %v", err)
}
if len(migrations) < 2 {
t.Skip("not enough migrations to compare sizes")
}
// The baseline (0001_init.sql) should be the largest migration.
baselineLen := len(migrations[0].sql)
for _, m := range migrations[1:] {
if len(m.sql) > baselineLen {
t.Errorf("migration %s (%d bytes) is larger than baseline (%d bytes)",
m.filename, len(m.sql), baselineLen)
}
}
}
func TestParseVersion_Comprehensive(t *testing.T) {
tests := []struct {
filename string
want int
wantErr bool
}{
{"0001_init.sql", 1, false},
{"0002_add_users.sql", 2, false},
{"0100_big_change.sql", 100, false},
{"9999_final.sql", 9999, false},
{"bad.sql", 0, true},
{"noseparator", 0, true},
{"abc_description.sql", 0, true},
}
for _, tt := range tests {
got, err := parseVersion(tt.filename)
if (err != nil) != tt.wantErr {
t.Errorf("parseVersion(%q) error = %v, wantErr %v", tt.filename, err, tt.wantErr)
continue
}
if got != tt.want {
t.Errorf("parseVersion(%q) = %d, want %d", tt.filename, got, tt.want)
}
}
}

View File

@@ -30,8 +30,9 @@ func Run(logger *zap.Logger, port int) error {
Handler: r,
}
logger.Info(fmt.Sprintf("Setup wizard available at http://localhost:%d", port))
fmt.Printf("\n >>> Open http://localhost:%d in your browser to configure Erupe <<<\n\n", port)
logger.Info("Setup wizard available",
zap.String("url", fmt.Sprintf("http://localhost:%d", port)))
logger.Warn("Open the URL above in your browser to configure Erupe")
// Start the HTTP server in a goroutine.
errCh := make(chan error, 1)

View File

@@ -6,6 +6,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"go.uber.org/zap"
@@ -195,3 +196,291 @@ func containsHelper(s, substr string) bool {
}
return false
}
func TestBuildDefaultConfig_EmptyLanguage(t *testing.T) {
req := FinishRequest{
DBHost: "localhost",
DBPort: 5432,
DBUser: "postgres",
DBPassword: "pass",
DBName: "erupe",
Host: "127.0.0.1",
ClientMode: "ZZ",
Language: "", // empty — should default to "jp"
}
cfg := buildDefaultConfig(req)
lang, ok := cfg["Language"].(string)
if !ok {
t.Fatal("Language is not a string")
}
if lang != "jp" {
t.Errorf("Language = %q, want %q", lang, "jp")
}
}
func TestHandleTestDB_InvalidJSON(t *testing.T) {
ws := &wizardServer{
logger: zap.NewNop(),
done: make(chan struct{}),
}
req := httptest.NewRequest("POST", "/api/setup/test-db", strings.NewReader("{invalid"))
w := httptest.NewRecorder()
ws.handleTestDB(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", w.Code, http.StatusBadRequest)
}
var resp map[string]string
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode error: %v", err)
}
if resp["error"] != "invalid JSON" {
t.Errorf("error = %q, want %q", resp["error"], "invalid JSON")
}
}
func TestHandleInitDB_InvalidJSON(t *testing.T) {
ws := &wizardServer{
logger: zap.NewNop(),
done: make(chan struct{}),
}
req := httptest.NewRequest("POST", "/api/setup/init-db", strings.NewReader("not json"))
w := httptest.NewRecorder()
ws.handleInitDB(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", w.Code, http.StatusBadRequest)
}
var resp map[string]string
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode error: %v", err)
}
if resp["error"] != "invalid JSON" {
t.Errorf("error = %q, want %q", resp["error"], "invalid JSON")
}
}
func TestHandleFinish_InvalidJSON(t *testing.T) {
ws := &wizardServer{
logger: zap.NewNop(),
done: make(chan struct{}),
}
req := httptest.NewRequest("POST", "/api/setup/finish", strings.NewReader("%%%"))
w := httptest.NewRecorder()
ws.handleFinish(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", w.Code, http.StatusBadRequest)
}
var resp map[string]string
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode error: %v", err)
}
if resp["error"] != "invalid JSON" {
t.Errorf("error = %q, want %q", resp["error"], "invalid JSON")
}
}
func TestHandleFinish_Success(t *testing.T) {
dir := t.TempDir()
origDir, _ := os.Getwd()
if err := os.Chdir(dir); err != nil {
t.Fatal(err)
}
defer func() { _ = os.Chdir(origDir) }()
done := make(chan struct{})
ws := &wizardServer{
logger: zap.NewNop(),
done: done,
}
body := `{"dbHost":"localhost","dbPort":5432,"dbUser":"postgres","dbPassword":"pw","dbName":"erupe","host":"10.0.0.5","clientMode":"G10","autoCreateAccount":false}`
req := httptest.NewRequest("POST", "/api/setup/finish", strings.NewReader(body))
w := httptest.NewRecorder()
ws.handleFinish(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want %d", w.Code, http.StatusOK)
}
// Verify config.json was written
data, err := os.ReadFile(filepath.Join(dir, "config.json"))
if err != nil {
t.Fatalf("config.json not written: %v", err)
}
var parsed map[string]interface{}
if err := json.Unmarshal(data, &parsed); err != nil {
t.Fatalf("config.json is not valid JSON: %v", err)
}
if parsed["Host"] != "10.0.0.5" {
t.Errorf("Host = %v, want 10.0.0.5", parsed["Host"])
}
if parsed["ClientMode"] != "G10" {
t.Errorf("ClientMode = %v, want G10", parsed["ClientMode"])
}
// Verify done channel was closed
select {
case <-done:
// expected
default:
t.Error("done channel was not closed after successful finish")
}
}
func TestWriteJSON(t *testing.T) {
tests := []struct {
name string
status int
payload interface{}
wantStatus int
}{
{
name: "OK with string map",
status: http.StatusOK,
payload: map[string]string{"key": "value"},
wantStatus: http.StatusOK,
},
{
name: "BadRequest with error",
status: http.StatusBadRequest,
payload: map[string]string{"error": "bad input"},
wantStatus: http.StatusBadRequest,
},
{
name: "InternalServerError",
status: http.StatusInternalServerError,
payload: map[string]string{"error": "something broke"},
wantStatus: http.StatusInternalServerError,
},
{
name: "OK with nested payload",
status: http.StatusOK,
payload: map[string]interface{}{"count": 42, "items": []string{"a", "b"}},
wantStatus: http.StatusOK,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
w := httptest.NewRecorder()
writeJSON(w, tc.status, tc.payload)
if w.Code != tc.wantStatus {
t.Errorf("status = %d, want %d", w.Code, tc.wantStatus)
}
ct := w.Header().Get("Content-Type")
if ct != "application/json" {
t.Errorf("Content-Type = %q, want application/json", ct)
}
// Verify body is valid JSON
var decoded interface{}
if err := json.NewDecoder(w.Body).Decode(&decoded); err != nil {
t.Errorf("response body is not valid JSON: %v", err)
}
})
}
}
func TestClientModesContainsExpected(t *testing.T) {
modes := clientModes()
expected := []string{"ZZ", "G10", "FW.4", "S1.0", "Z2", "GG"}
modeSet := make(map[string]bool, len(modes))
for _, m := range modes {
modeSet[m] = true
}
for _, exp := range expected {
if !modeSet[exp] {
t.Errorf("clientModes() missing expected mode %q", exp)
}
}
}
func TestHandleInitDB_NoOps(t *testing.T) {
ws := &wizardServer{
logger: zap.NewNop(),
done: make(chan struct{}),
}
// All flags false — no DB operations, should succeed immediately.
body := `{"host":"localhost","port":5432,"user":"test","password":"test","dbName":"test","createDB":false,"applySchema":false,"applyBundled":false}`
req := httptest.NewRequest("POST", "/api/setup/init-db", strings.NewReader(body))
w := httptest.NewRecorder()
ws.handleInitDB(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", w.Code)
}
var resp map[string]interface{}
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode error: %v", err)
}
if resp["success"] != true {
t.Errorf("success = %v, want true", resp["success"])
}
log, ok := resp["log"].([]interface{})
if !ok {
t.Fatal("log should be an array")
}
// Should contain the "complete" message
found := false
for _, entry := range log {
if s, ok := entry.(string); ok && strings.Contains(s, "complete") {
found = true
break
}
}
if !found {
t.Error("expected completion message in log")
}
}
func TestBuildDefaultConfig_WithLanguage(t *testing.T) {
req := FinishRequest{
DBHost: "localhost",
DBPort: 5432,
DBUser: "postgres",
DBPassword: "pass",
DBName: "erupe",
Host: "127.0.0.1",
ClientMode: "ZZ",
Language: "en",
}
cfg := buildDefaultConfig(req)
if cfg["Language"] != "en" {
t.Errorf("Language = %v, want en", cfg["Language"])
}
}
func TestWriteConfig_Permissions(t *testing.T) {
dir := t.TempDir()
origDir, _ := os.Getwd()
if err := os.Chdir(dir); err != nil {
t.Fatal(err)
}
defer func() { _ = os.Chdir(origDir) }()
cfg := buildDefaultConfig(FinishRequest{
DBHost: "localhost",
DBPort: 5432,
DBUser: "postgres",
DBPassword: "pass",
DBName: "erupe",
Host: "127.0.0.1",
ClientMode: "ZZ",
})
if err := writeConfig(cfg); err != nil {
t.Fatalf("writeConfig failed: %v", err)
}
info, err := os.Stat(filepath.Join(dir, "config.json"))
if err != nil {
t.Fatalf("stat config.json: %v", err)
}
// File should be 0600 (owner read/write only)
if perm := info.Mode().Perm(); perm != 0600 {
t.Errorf("config.json permissions = %o, want 0600", perm)
}
}