From 381e41b32592b96847ab32b245f5fc9e652d0c7b Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sat, 9 May 2026 10:55:55 +0200 Subject: [PATCH] fix: game order api & tests --- game/README.md | 1 + game/internal/router/handler/order.go | 15 +- game/internal/router/order_test.go | 168 +++++++++++++++++++++ game/internal/router/router_helper_test.go | 29 +++- game/openapi.yaml | 70 ++++++++- game/openapi_contract_test.go | 94 ++++++++++++ 6 files changed, 361 insertions(+), 16 deletions(-) diff --git a/game/README.md b/game/README.md index 3d8ad8c..bf3baa7 100644 --- a/game/README.md +++ b/game/README.md @@ -49,6 +49,7 @@ described below. Endpoints split into two route classes: | Admin (GM-only) | `POST /api/v1/admin/race/banish` | `Game Master` | Deactivate a race after a permanent platform removal. | | Player | `PUT /api/v1/command` | `Game Master` (forwarded from `Edge Gateway`) | Execute a batch of player commands. | | Player | `PUT /api/v1/order` | `Game Master` | Validate and store a batch of player orders. | +| Player | `GET /api/v1/order` | `Game Master` | Fetch the previously stored player order for a turn. | | Player | `GET /api/v1/report` | `Game Master` | Fetch the per-player turn report. | | Probe | `GET /healthz` | `Runtime Manager` | Technical liveness probe. | diff --git a/game/internal/router/handler/order.go b/game/internal/router/handler/order.go index b70c8e2..9a31b3c 100644 --- a/game/internal/router/handler/order.go +++ b/game/internal/router/handler/order.go @@ -46,23 +46,22 @@ type orderParam struct { func GetOrderHandler(c *gin.Context, executor CommandExecutor) { p := &orderParam{} - err := c.ShouldBindQuery(p) - if errorResponse(c, err) { + // ShouldBindQuery surfaces both validator failures and strconv parse + // errors; both are client-side faults, so 400 is the correct mapping. + if err := c.ShouldBindQuery(p); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - order, ok, err := executor.FetchOrder(p.Player, uint(p.Turn)) + o, ok, err := executor.FetchOrder(p.Player, uint(p.Turn)) if errorResponse(c, err) { return } if !ok { - // there was no order previously sent by player + // no order has been previously stored by the player for this turn c.Status(http.StatusNoContent) - } - var cmd rest.Command - if errorResponse(c, c.ShouldBindJSON(&cmd)) { return } - c.JSON(http.StatusOK, order) + c.JSON(http.StatusOK, o) } diff --git a/game/internal/router/order_test.go b/game/internal/router/order_test.go index 93844c6..4e3acba 100644 --- a/game/internal/router/order_test.go +++ b/game/internal/router/order_test.go @@ -2,6 +2,7 @@ package router_test import ( "encoding/json" + "errors" "net/http" "net/http/httptest" "testing" @@ -9,7 +10,9 @@ import ( "galaxy/model/order" "galaxy/model/rest" + "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestOrderRaceQuit(t *testing.T) { @@ -940,3 +943,168 @@ func TestMultipleCommandOrder(t *testing.T) { assert.Equal(t, 2, e.(*dummyExecutor).CommandsExecuted) } + +func TestPutOrderResponseBody(t *testing.T) { + e := &dummyExecutor{ + ValidateOrderResult: &order.UserGamesOrder{ + GameID: uuid.New(), + UpdatedAt: 1700, + Commands: []order.DecodableCommand{ + &order.CommandRaceVote{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeRaceVote}, + Acceptor: "Opponent", + }, + }, + }, + } + r := setupRouterExecutor(e) + + payload := &rest.Command{ + Actor: commandDefaultActor, + Commands: []json.RawMessage{ + encodeCommand(&order.CommandRaceVote{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeRaceVote}, + Acceptor: "Opponent", + }), + }, + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest(apiCommandMethod, apiOrderPath, asBody(payload)) + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusAccepted, w.Code, w.Body) + + var got struct { + GameID uuid.UUID `json:"game_id"` + UpdatedAt int64 `json:"updatedAt"` + Commands []json.RawMessage `json:"cmd"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) + assert.Equal(t, e.ValidateOrderResult.GameID, got.GameID) + assert.Equal(t, e.ValidateOrderResult.UpdatedAt, got.UpdatedAt) + assert.Len(t, got.Commands, 1) +} + +func TestPutOrderEngineError(t *testing.T) { + e := &dummyExecutor{ValidateOrderErr: errors.New("engine boom")} + r := setupRouterExecutor(e) + + payload := &rest.Command{ + Actor: commandDefaultActor, + Commands: []json.RawMessage{ + encodeCommand(&order.CommandRaceVote{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeRaceVote}, + Acceptor: "Opponent", + }), + }, + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest(apiCommandMethod, apiOrderPath, asBody(payload)) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code, w.Body) +} + +func TestGetOrderQueryValidation(t *testing.T) { + for _, tc := range []struct { + description string + query string + expectStatus int + }{ + {"Missing player param", "", http.StatusBadRequest}, + {"Empty player", "?player=", http.StatusBadRequest}, + {"Blank player", "?player=%20%20%20", http.StatusBadRequest}, + {"Negative turn", "?player=Race_01&turn=-1", http.StatusBadRequest}, + {"Non-numeric turn", "?player=Race_01&turn=abc", http.StatusBadRequest}, + } { + t.Run(tc.description, func(t *testing.T) { + e := &dummyExecutor{} + r := setupRouterExecutor(e) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, apiOrderPath+tc.query, nil) + r.ServeHTTP(w, req) + + assert.Equal(t, tc.expectStatus, w.Code, w.Body) + assert.Empty(t, e.FetchOrderActor, "FetchOrder must not be called on validation error") + }) + } +} + +func TestGetOrderFound(t *testing.T) { + stored := &order.UserGamesOrder{ + GameID: uuid.New(), + UpdatedAt: 4242, + Commands: []order.DecodableCommand{ + &order.CommandRaceVote{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeRaceVote}, + Acceptor: "Opponent", + }, + }, + } + e := &dummyExecutor{ + FetchOrderResult: stored, + FetchOrderOK: true, + } + r := setupRouterExecutor(e) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, apiOrderPath+"?player=Race_01&turn=3", nil) + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code, w.Body) + assert.Equal(t, "Race_01", e.FetchOrderActor) + assert.Equal(t, uint(3), e.FetchOrderTurn) + + var got struct { + GameID uuid.UUID `json:"game_id"` + UpdatedAt int64 `json:"updatedAt"` + Commands []json.RawMessage `json:"cmd"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) + assert.Equal(t, stored.GameID, got.GameID) + assert.Equal(t, stored.UpdatedAt, got.UpdatedAt) + assert.Len(t, got.Commands, 1) +} + +func TestGetOrderTurnDefaultsToZero(t *testing.T) { + e := &dummyExecutor{ + FetchOrderResult: &order.UserGamesOrder{GameID: uuid.New(), UpdatedAt: 1, Commands: []order.DecodableCommand{}}, + FetchOrderOK: true, + } + r := setupRouterExecutor(e) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, apiOrderPath+"?player=Race_01", nil) + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code, w.Body) + assert.Equal(t, uint(0), e.FetchOrderTurn) +} + +func TestGetOrderNotFound(t *testing.T) { + e := &dummyExecutor{FetchOrderOK: false} + r := setupRouterExecutor(e) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, apiOrderPath+"?player=Race_01&turn=2", nil) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNoContent, w.Code, w.Body) + assert.Empty(t, w.Body.Bytes(), "204 response must carry no body") + assert.Equal(t, "Race_01", e.FetchOrderActor) + assert.Equal(t, uint(2), e.FetchOrderTurn) +} + +func TestGetOrderEngineError(t *testing.T) { + e := &dummyExecutor{FetchOrderErr: errors.New("engine boom")} + r := setupRouterExecutor(e) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, apiOrderPath+"?player=Race_01&turn=0", nil) + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code, w.Body) +} diff --git a/game/internal/router/router_helper_test.go b/game/internal/router/router_helper_test.go index 98520e9..4ab9b98 100644 --- a/game/internal/router/router_helper_test.go +++ b/game/internal/router/router_helper_test.go @@ -32,15 +32,40 @@ func id() string { type dummyExecutor struct { CommandsExecuted int + + // ValidateOrderResult, when non-nil, is returned from ValidateOrder. + // When nil, ValidateOrder synthesises an order from the received args + // so the response body is non-empty for status assertions. + ValidateOrderResult *order.UserGamesOrder + ValidateOrderErr error + + // FetchOrder controls and observes calls to FetchOrder. + FetchOrderActor string + FetchOrderTurn uint + FetchOrderResult *order.UserGamesOrder + FetchOrderOK bool + FetchOrderErr error } func (e *dummyExecutor) ValidateOrder(actor string, cmd ...order.DecodableCommand) (*order.UserGamesOrder, error) { e.CommandsExecuted = len(cmd) - return nil, nil + if e.ValidateOrderErr != nil { + return nil, e.ValidateOrderErr + } + if e.ValidateOrderResult != nil { + return e.ValidateOrderResult, nil + } + return &order.UserGamesOrder{ + GameID: uuid.New(), + UpdatedAt: 1, + Commands: append([]order.DecodableCommand(nil), cmd...), + }, nil } func (e *dummyExecutor) FetchOrder(actor string, turn uint) (*order.UserGamesOrder, bool, error) { - return nil, true, nil + e.FetchOrderActor = actor + e.FetchOrderTurn = turn + return e.FetchOrderResult, e.FetchOrderOK, e.FetchOrderErr } func (e *dummyExecutor) Execute(command ...handler.Command) error { diff --git a/game/openapi.yaml b/game/openapi.yaml index 51ba5ee..37f8b36 100644 --- a/game/openapi.yaml +++ b/game/openapi.yaml @@ -136,8 +136,9 @@ paths: description: | Applies one or more game commands for the specified actor. Serialized to one concurrent execution; requests that cannot acquire the execution - slot within 100 ms return `504 Gateway Timeout`. Returns `204 No - Content` on success. + slot within 100 ms return `504 Gateway Timeout`. Returns `202 Accepted` + with no body on success. Reserved for future use; player order + submissions go through `/api/v1/order`. requestBody: required: true content: @@ -145,8 +146,8 @@ paths: schema: $ref: "#/components/schemas/CommandRequest" responses: - "204": - description: All commands applied successfully. + "202": + description: All commands accepted. "400": $ref: "#/components/responses/ValidationError" "504": @@ -161,7 +162,9 @@ paths: summary: Validate and store a player order without executing it description: | Validates and stores the game commands structurally without executing them. - Returns `204 No Content` if the order is valid and accepted. + On success returns `202 Accepted` with the stored order, including the + engine-assigned `updatedAt` timestamp used by clients to detect stale + submissions. requestBody: required: true content: @@ -169,8 +172,37 @@ paths: schema: $ref: "#/components/schemas/CommandRequest" responses: + "202": + description: Order is structurally valid and stored. + content: + application/json: + schema: + $ref: "#/components/schemas/UserGamesOrder" + "400": + $ref: "#/components/responses/ValidationError" + "500": + $ref: "#/components/responses/InternalError" + get: + tags: + - PlayerActions + operationId: getOrder + summary: Fetch the stored order for a player and turn + description: | + Returns the order previously stored by `PUT /api/v1/order` for the + specified player and turn. Responds `204 No Content` when no order + has been stored for that turn. + parameters: + - $ref: "#/components/parameters/PlayerParam" + - $ref: "#/components/parameters/TurnParam" + responses: + "200": + description: Stored player order for the requested turn. + content: + application/json: + schema: + $ref: "#/components/schemas/UserGamesOrder" "204": - description: Order is structurally valid. + description: No order has been stored for this player on this turn. "400": $ref: "#/components/responses/ValidationError" "500": @@ -362,6 +394,32 @@ components: minItems: 1 items: $ref: "#/components/schemas/Command" + UserGamesOrder: + type: object + description: | + Stored player order. Returned by `PUT /api/v1/order` after successful + validation and by `GET /api/v1/order` when fetching a previously stored + batch. `cmd` mirrors the command list submitted by the player; entries + carry per-command result fields (`cmdApplied`, `cmdErrorCode`) once the + order has been processed during turn generation. + required: + - game_id + - updatedAt + - cmd + properties: + game_id: + type: string + format: uuid + description: Identifier of the game this order belongs to. + updatedAt: + type: integer + format: int64 + description: Engine-assigned UTC millisecond timestamp of the last write. + cmd: + type: array + description: Commands stored as part of this order, in submission order. + items: + $ref: "#/components/schemas/Command" Command: type: object description: | diff --git a/game/openapi_contract_test.go b/game/openapi_contract_test.go index 0e93535..12446da 100644 --- a/game/openapi_contract_test.go +++ b/game/openapi_contract_test.go @@ -58,6 +58,20 @@ func TestGameOpenAPISpecFreezesResponseSchemas(t *testing.T) { status: http.StatusOK, wantRef: "#/components/schemas/StateResponse", }, + { + name: "put order", + path: "/api/v1/order", + method: http.MethodPut, + status: http.StatusAccepted, + wantRef: "#/components/schemas/UserGamesOrder", + }, + { + name: "get order", + path: "/api/v1/order", + method: http.MethodGet, + status: http.StatusOK, + wantRef: "#/components/schemas/UserGamesOrder", + }, { name: "healthz probe", path: "/healthz", @@ -77,6 +91,86 @@ func TestGameOpenAPISpecFreezesResponseSchemas(t *testing.T) { } } +func TestGameOpenAPISpecFreezesEmptyResponses(t *testing.T) { + t.Parallel() + + doc := loadOpenAPISpec(t) + + tests := []struct { + name string + path string + method string + status int + }{ + { + name: "command accepted", + path: "/api/v1/command", + method: http.MethodPut, + status: http.StatusAccepted, + }, + { + name: "get order no content", + path: "/api/v1/order", + method: http.MethodGet, + status: http.StatusNoContent, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + operation := getOpenAPIOperation(t, doc, tt.path, tt.method) + require.NotNil(t, operation.Responses, "operation must declare responses") + response := operation.Responses.Status(tt.status) + require.NotNil(t, response, "operation must declare %d response", tt.status) + require.NotNil(t, response.Value, "%d response must have a value", tt.status) + require.Empty(t, response.Value.Content, "%d response must carry no body", tt.status) + }) + } +} + +func TestGameOpenAPISpecFreezesUserGamesOrder(t *testing.T) { + t.Parallel() + + doc := loadOpenAPISpec(t) + schema := componentSchemaRef(t, doc, "UserGamesOrder") + + assertRequiredFields(t, schema, "game_id", "updatedAt", "cmd") + + gameIDSchema := schema.Value.Properties["game_id"] + require.NotNil(t, gameIDSchema, "UserGamesOrder.game_id schema must exist") + require.Equal(t, "uuid", gameIDSchema.Value.Format, "UserGamesOrder.game_id format must be uuid") + + updatedAtSchema := schema.Value.Properties["updatedAt"] + require.NotNil(t, updatedAtSchema, "UserGamesOrder.updatedAt schema must exist") + require.True(t, updatedAtSchema.Value.Type.Is("integer"), "UserGamesOrder.updatedAt must be integer") + require.Equal(t, "int64", updatedAtSchema.Value.Format, "UserGamesOrder.updatedAt format must be int64") + + cmdSchema := schema.Value.Properties["cmd"] + require.NotNil(t, cmdSchema, "UserGamesOrder.cmd schema must exist") + require.True(t, cmdSchema.Value.Type.Is("array"), "UserGamesOrder.cmd must be array") + require.NotNil(t, cmdSchema.Value.Items, "UserGamesOrder.cmd items must be defined") + assertSchemaRef(t, cmdSchema.Value.Items, "#/components/schemas/Command", "UserGamesOrder.cmd items schema") +} + +func TestGameOpenAPISpecFreezesGetOrderOperation(t *testing.T) { + t.Parallel() + + doc := loadOpenAPISpec(t) + operation := getOpenAPIOperation(t, doc, "/api/v1/order", http.MethodGet) + + require.Equal(t, "getOrder", operation.OperationID, "GET /api/v1/order operation id") + + paramRefs := make(map[string]bool) + for _, p := range operation.Parameters { + require.NotNil(t, p.Value, "parameter must have value") + paramRefs[p.Ref] = true + } + require.True(t, paramRefs["#/components/parameters/PlayerParam"], "GET /api/v1/order must reference PlayerParam") + require.True(t, paramRefs["#/components/parameters/TurnParam"], "GET /api/v1/order must reference TurnParam") +} + func TestGameOpenAPISpecFreezesInitRequest(t *testing.T) { t.Parallel()