refactor(game): lock-free storage, remove /command, flatten engine wrapper
Three-stage refactor of the game-engine plumbing (game logic untouched): Stage 1 — lock-free persistence + admin serialisation. Remove the file lock from repo/fs (the .lock file, the Read/Write-vs-*Safe duality and the dead ReadSafe polling) and replace the two-step rename with a single atomic rename so concurrent reads are torn-free without a lock. Serialise the state-mutating admin writers (init/turn/banish) with one shared router LimitMiddleware, rewritten to block on the request context instead of a racy shared 100ms timer. Stage 2 — remove the obsolete immediate-command path end to end. Players submit through PUT /api/v1/order; the legacy PUT /api/v1/command path is deleted across game (route, handler, 24 command factories, Ctrl), backend (Commands handler/route, engineclient.ExecuteCommands), gateway (dispatch + executeUserGamesCommand + routing entry), the FlatBuffers/model contract (UserGamesCommand[Response]) and transcoder, plus every affected OpenAPI/README/FUNCTIONAL/ARCHITECTURE doc. The integration proxy test is converted to the order path. Stage 3 — flatten the REST->engine wrapper. Replace the executor adapter, the controller package functions and RepoController with one concrete controller.Service; drop the single-implementation Repo and Storage interfaces (repo.Repo / fs.FS are now concrete). Handlers depend on a thin handler.Engine seam and own the domain->REST projection; storage is resolved once at startup instead of per request. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,9 +1,11 @@
|
||||
package fs_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"galaxy/game/internal/repo/fs"
|
||||
@@ -12,10 +14,6 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
const (
|
||||
lockFile = ".lock"
|
||||
)
|
||||
|
||||
type sampleData struct {
|
||||
data []byte
|
||||
}
|
||||
@@ -36,20 +34,6 @@ func TestNewFileStorageSuccess(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestLock(t *testing.T) {
|
||||
root, cleanup := util.CreateWorkDir(t)
|
||||
defer cleanup()
|
||||
fs, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err, "create file storage")
|
||||
unlock, err := fs.Lock()
|
||||
assert.NoError(t, err, "acquire lock")
|
||||
lockPath := filepath.Join(root, lockFile)
|
||||
assert.FileExists(t, lockPath, "lock file should be created")
|
||||
err = unlock()
|
||||
assert.NoError(t, err, "unlocking existing lock")
|
||||
assert.NoFileExists(t, lockPath, "lock file must be removed")
|
||||
}
|
||||
|
||||
func TestExist(t *testing.T) {
|
||||
root, cleanup := util.CreateWorkDir(t)
|
||||
defer cleanup()
|
||||
@@ -78,9 +62,6 @@ func TestWrite(t *testing.T) {
|
||||
fs, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err, "create file storage: %s", err)
|
||||
|
||||
unlock, err := fs.Lock()
|
||||
assert.NoError(t, err, "acquire lock: %s", err)
|
||||
|
||||
dirName := "some-dir"
|
||||
if err := os.Mkdir(filepath.Join(root, dirName), os.ModePerm); err != nil {
|
||||
t.Fatal(err)
|
||||
@@ -93,9 +74,8 @@ func TestWrite(t *testing.T) {
|
||||
{path: "file-1.ext"},
|
||||
{path: "/dir/file-2.ext"},
|
||||
{path: "dir/subdir/file-3.ext"},
|
||||
{path: lockFile, err: "write to the lock file"},
|
||||
{path: dirName, err: "wrong type"},
|
||||
{path: "/" + dirName, err: "wrong type"},
|
||||
{path: dirName, err: "file exists"},
|
||||
{path: "/" + dirName, err: "file exists"},
|
||||
} {
|
||||
t.Run(tc.path, func(t *testing.T) {
|
||||
sd := &sampleData{[]byte{0, 1, 2, 3}}
|
||||
@@ -103,13 +83,26 @@ func TestWrite(t *testing.T) {
|
||||
if tc.err == "" {
|
||||
assert.NoError(t, err)
|
||||
assert.FileExists(t, filepath.Join(root, tc.path), "the written file should exist")
|
||||
} else if tc.err != "" {
|
||||
} else {
|
||||
assert.ErrorContains(t, err, tc.err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
assert.NoError(t, unlock(), "unlocking existing lock")
|
||||
func TestWriteLeavesNoTempLeftovers(t *testing.T) {
|
||||
root, cleanup := util.CreateWorkDir(t)
|
||||
defer cleanup()
|
||||
|
||||
s, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err)
|
||||
|
||||
assert.NoError(t, s.Write("state.bin", &sampleData{[]byte{1, 2, 3}}))
|
||||
|
||||
entries, err := os.ReadDir(root)
|
||||
assert.NoError(t, err)
|
||||
assert.Len(t, entries, 1, "a successful write must leave only the target file, no temporaries")
|
||||
assert.Equal(t, "state.bin", entries[0].Name())
|
||||
}
|
||||
|
||||
func TestRead(t *testing.T) {
|
||||
@@ -121,11 +114,6 @@ func TestRead(t *testing.T) {
|
||||
fs, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err, "create file storage: %s", err)
|
||||
|
||||
assert.EqualError(t, fs.Read("some.file", sd), "lock must be acquired before read")
|
||||
|
||||
unlock, err := fs.Lock()
|
||||
assert.NoError(t, err, "acquire lock: %s", err)
|
||||
|
||||
dirName := "some-dir"
|
||||
if err := os.Mkdir(filepath.Join(root, dirName), os.ModePerm); err != nil {
|
||||
t.Fatal(err)
|
||||
@@ -142,33 +130,82 @@ func TestRead(t *testing.T) {
|
||||
}{
|
||||
{path: fileName},
|
||||
{path: "/" + fileName},
|
||||
{path: lockFile, err: "read from the lock file"},
|
||||
{path: "dir/subdir/file-3.ext", err: "no such file"},
|
||||
{path: lockFile, err: "read from the lock file"},
|
||||
{path: dirName, err: "is a directory"},
|
||||
} {
|
||||
t.Run(tc.path, func(t *testing.T) {
|
||||
err = fs.Read(tc.path, sd)
|
||||
if tc.err == "" {
|
||||
assert.NoError(t, err)
|
||||
assert.FileExists(t, filepath.Join(root, tc.path), "the written file should exist")
|
||||
} else if tc.err != "" {
|
||||
} else {
|
||||
assert.ErrorContains(t, err, tc.err)
|
||||
}
|
||||
})
|
||||
}
|
||||
assert.NoError(t, unlock(), "unlocking existing lock")
|
||||
}
|
||||
|
||||
func TestWriteErrorWithoutLock(t *testing.T) {
|
||||
// TestReadAtomicUnderConcurrentWrites is the regression that guards the
|
||||
// lock-free contract: with Write swapping files in via a single rename, a
|
||||
// concurrent Read must always observe one previously written payload in full —
|
||||
// never a torn mix and never a missing file. The two payloads differ in length
|
||||
// so any partial read is detectable.
|
||||
func TestReadAtomicUnderConcurrentWrites(t *testing.T) {
|
||||
root, cleanup := util.CreateWorkDir(t)
|
||||
defer cleanup()
|
||||
fs, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err, "create file storage")
|
||||
sd := &sampleData{[]byte{0, 1, 2, 3}}
|
||||
err = fs.Write("some/path", sd)
|
||||
assert.Error(t, err, "should return error when no lock acquired")
|
||||
assert.EqualError(t, err, "lock must be acquired before write")
|
||||
|
||||
s, err := fs.NewFileStorage(root)
|
||||
assert.NoError(t, err)
|
||||
|
||||
const path = "state.bin"
|
||||
payloads := [][]byte{
|
||||
bytes.Repeat([]byte{0xAA}, 4096),
|
||||
bytes.Repeat([]byte{0xBB}, 8192),
|
||||
}
|
||||
assert.NoError(t, s.Write(path, &sampleData{slices.Clone(payloads[0])}))
|
||||
|
||||
stop := make(chan struct{})
|
||||
var writers sync.WaitGroup
|
||||
for w := range 4 {
|
||||
writers.Go(func() {
|
||||
for {
|
||||
select {
|
||||
case <-stop:
|
||||
return
|
||||
default:
|
||||
_ = s.Write(path, &sampleData{slices.Clone(payloads[w%len(payloads)])})
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
var readers sync.WaitGroup
|
||||
for range 8 {
|
||||
readers.Go(func() {
|
||||
for range 1000 {
|
||||
sd := new(sampleData)
|
||||
if err := s.Read(path, sd); err != nil {
|
||||
t.Errorf("read during concurrent write failed: %v", err)
|
||||
return
|
||||
}
|
||||
if !knownPayload(sd.data, payloads) {
|
||||
t.Errorf("read observed a torn payload (len=%d)", len(sd.data))
|
||||
return
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
readers.Wait()
|
||||
close(stop)
|
||||
writers.Wait()
|
||||
}
|
||||
|
||||
func knownPayload(got []byte, want [][]byte) bool {
|
||||
for _, w := range want {
|
||||
if bytes.Equal(got, w) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func TestNewFileStorageErrorNotExists(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user