mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-07-01 02:33:26 +00:00
refactor: collapse per-env Configs into ClientEnv + ServerEnv (#3906)
## Description: This is a refactor to simplify config handling. Replaces the per-environment DevConfig/PreprodConfig/ProdConfig class hierarchy with two static classes: ClientEnv (browser main thread, reads from window.BOOTSTRAP_CONFIG) and ServerEnv (Node server, reads from process.env). The four config classes are deleted, the abstract DefaultServerConfig is gone, and DefaultConfig is renamed to Config. The values that flow server → client (gameEnv, numWorkers, turnstileSiteKey, jwtAudience, instanceId) used to be baked into the hardcoded per-env classes. They're now real env vars on the server, embedded into a single window.BOOTSTRAP_CONFIG object in index.html at request time (alongside the existing gitCommit/assetManifest/cdnBase globals, which moved into the same object), and read back by ClientEnv on the client. The dev defaults previously hidden inside DevServerConfig are now explicit in start:server-dev (NUM_WORKERS=2, TURNSTILE_SITE_KEY=1x..., JWT_AUDIENCE=localhost, etc.) and in vite.config.ts's html plugin inject.data. Production deploys plumb NUM_WORKERS and TURNSTILE_SITE_KEY through deploy.yml (GitHub vars) into the remote env file; JWT_AUDIENCE is derived from DOMAIN in deploy.sh. The dynamic /api/instance endpoint is gone — INSTANCE_ID rides along in BOOTSTRAP_CONFIG now. ServerEnv is the only thing server code touches; ClientEnv is browser-only. The two classes have intentional overlap (env, numWorkers, jwtIssuer, gameCreationRate, workerIndex, etc.) since they derive identical logic from different sources — there's a TODO in each to consolidate via a shared helper later. The game-logic Config no longer stores a ServerConfig/ClientEnv reference and its serverConfig() getter is gone; the one caller (MultiTabModal) now reads ClientEnv.env() directly. Worker init no longer carries server-config values since nothing in the worker actually reads them. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: evan
This commit is contained in:
@@ -1,13 +1,13 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../../src/core/configuration/ConfigLoader", () => ({
|
||||
getServerConfigFromServer: () => ({
|
||||
vi.mock("../../src/server/ServerEnv", () => ({
|
||||
ServerEnv: {
|
||||
jwtIssuer: () => "https://archive.test.invalid",
|
||||
apiKey: () => "test-key",
|
||||
gitCommit: () => "DEV",
|
||||
subdomain: () => "test",
|
||||
domain: () => "test",
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("../../src/server/Logger", () => ({
|
||||
|
||||
@@ -1,17 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../../src/core/configuration/ConfigLoader", () => ({
|
||||
getServerConfigFromServer: () => ({
|
||||
otelEnabled: () => false,
|
||||
otelAuthHeader: () => "",
|
||||
otelEndpoint: () => "",
|
||||
env: () => 0, // GameEnv.Dev
|
||||
}),
|
||||
getServerConfig: () => ({
|
||||
otelEnabled: () => false,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../../src/core/Schemas", async () => {
|
||||
const actual = (await vi.importActual("../../src/core/Schemas")) as any;
|
||||
return {
|
||||
@@ -25,13 +13,11 @@ vi.mock("../../src/core/Schemas", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
import { GameEnv } from "../../src/core/configuration/Config";
|
||||
import { GameType } from "../../src/core/game/Game";
|
||||
import { GameServer } from "../../src/server/GameServer";
|
||||
|
||||
describe("GameLifecycle", () => {
|
||||
let mockLogger: any;
|
||||
let mockConfig: any;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
@@ -41,11 +27,6 @@ describe("GameLifecycle", () => {
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
};
|
||||
mockConfig = {
|
||||
turnIntervalMs: () => 100,
|
||||
gameCreationRate: () => 1000,
|
||||
env: () => GameEnv.Dev,
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -54,13 +35,9 @@ describe("GameLifecycle", () => {
|
||||
});
|
||||
|
||||
it("should not start turn interval if game has ended", async () => {
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{ gameType: GameType.Private } as any,
|
||||
);
|
||||
const game = new GameServer("test-game", mockLogger, Date.now(), {
|
||||
gameType: GameType.Private,
|
||||
} as any);
|
||||
|
||||
// Call end() first - this should set _hasEnded
|
||||
await game.end();
|
||||
@@ -77,17 +54,11 @@ describe("GameLifecycle", () => {
|
||||
|
||||
it("should clear turn interval and set _hasEnded on end()", async () => {
|
||||
// We need to initialize the game such that start() can succeed
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{
|
||||
gameType: GameType.Private,
|
||||
gameMap: "plains",
|
||||
gameMapSize: 100,
|
||||
} as any,
|
||||
);
|
||||
const game = new GameServer("test-game", mockLogger, Date.now(), {
|
||||
gameType: GameType.Private,
|
||||
gameMap: "plains",
|
||||
gameMapSize: 100,
|
||||
} as any);
|
||||
|
||||
// Manually trigger prestart to fulfill some internal checks if necessary
|
||||
game.prestart();
|
||||
@@ -103,13 +74,9 @@ describe("GameLifecycle", () => {
|
||||
});
|
||||
|
||||
it("should be resilient to multiple end() calls", async () => {
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{ gameType: GameType.Private } as any,
|
||||
);
|
||||
const game = new GameServer("test-game", mockLogger, Date.now(), {
|
||||
gameType: GameType.Private,
|
||||
} as any);
|
||||
|
||||
await game.end();
|
||||
expect((game as any)._hasEnded).toBe(true);
|
||||
|
||||
@@ -1,17 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../../src/core/configuration/ConfigLoader", () => ({
|
||||
getServerConfigFromServer: () => ({
|
||||
otelEnabled: () => false,
|
||||
otelAuthHeader: () => "",
|
||||
otelEndpoint: () => "",
|
||||
env: () => 0, // GameEnv.Dev
|
||||
}),
|
||||
getServerConfig: () => ({
|
||||
otelEnabled: () => false,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../../src/core/Schemas", async () => {
|
||||
const actual = (await vi.importActual("../../src/core/Schemas")) as any;
|
||||
return {
|
||||
@@ -69,7 +57,6 @@ function makeClient(
|
||||
|
||||
describe("GameServer - kick_player authorization", () => {
|
||||
let mockLogger: any;
|
||||
let mockConfig: any;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
@@ -79,11 +66,6 @@ describe("GameServer - kick_player authorization", () => {
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
};
|
||||
mockConfig = {
|
||||
turnIntervalMs: () => 100,
|
||||
gameCreationRate: () => 1000,
|
||||
env: () => 0,
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -96,7 +78,6 @@ describe("GameServer - kick_player authorization", () => {
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{ gameType: GameType.Private } as any,
|
||||
creatorPersistentID,
|
||||
);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import EventEmitter from "events";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { MasterLobbyService } from "../../src/server/MasterLobbyService";
|
||||
import { TestServerConfig } from "../util/TestServerConfig";
|
||||
import { ServerEnv } from "../../src/server/ServerEnv";
|
||||
|
||||
vi.mock("../../src/server/Logger", () => ({
|
||||
logger: {
|
||||
@@ -27,10 +27,9 @@ function sendWorkerReady(worker: EventEmitter, workerId: number) {
|
||||
}
|
||||
|
||||
function createService(numWorkers: number): MasterLobbyService {
|
||||
const config = new TestServerConfig();
|
||||
vi.spyOn(config, "numWorkers").mockReturnValue(numWorkers);
|
||||
vi.spyOn(ServerEnv, "numWorkers").mockReturnValue(numWorkers);
|
||||
const log = { info: vi.fn(), error: vi.fn() } as any;
|
||||
return new MasterLobbyService(config, {} as any, log);
|
||||
return new MasterLobbyService({} as any, log);
|
||||
}
|
||||
|
||||
function startAllWorkers(
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from "fs/promises";
|
||||
import os from "os";
|
||||
import path from "path";
|
||||
import { afterEach, describe, expect, test } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
||||
import {
|
||||
clearAppShellContentCache,
|
||||
getAppShellContent,
|
||||
@@ -12,7 +12,14 @@ describe("RenderHtml", () => {
|
||||
const originalGitCommit = process.env.GIT_COMMIT;
|
||||
let tempDir: string | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.stubEnv("NUM_WORKERS", "1");
|
||||
vi.stubEnv("TURNSTILE_SITE_KEY", "test-key");
|
||||
vi.stubEnv("DOMAIN", "localhost");
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.unstubAllEnvs();
|
||||
process.env.GIT_COMMIT = originalGitCommit;
|
||||
clearAppShellContentCache();
|
||||
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
import { afterEach, describe, expect, test, vi } from "vitest";
|
||||
import { ServerEnv } from "../../src/server/ServerEnv";
|
||||
|
||||
describe("ServerEnv.numWorkers", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("returns parsed value when valid", () => {
|
||||
vi.stubEnv("NUM_WORKERS", "4");
|
||||
expect(ServerEnv.numWorkers()).toBe(4);
|
||||
});
|
||||
|
||||
test("throws when unset", () => {
|
||||
vi.stubEnv("NUM_WORKERS", "");
|
||||
expect(() => ServerEnv.numWorkers()).toThrow(/NUM_WORKERS not set/);
|
||||
});
|
||||
|
||||
test("throws on non-numeric", () => {
|
||||
vi.stubEnv("NUM_WORKERS", "abc");
|
||||
expect(() => ServerEnv.numWorkers()).toThrow(/Invalid NUM_WORKERS/);
|
||||
});
|
||||
|
||||
test("throws on zero", () => {
|
||||
vi.stubEnv("NUM_WORKERS", "0");
|
||||
expect(() => ServerEnv.numWorkers()).toThrow(/Invalid NUM_WORKERS/);
|
||||
});
|
||||
|
||||
test("throws on negative", () => {
|
||||
vi.stubEnv("NUM_WORKERS", "-2");
|
||||
expect(() => ServerEnv.numWorkers()).toThrow(/Invalid NUM_WORKERS/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ServerEnv.turnstileSiteKey", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("returns value when set", () => {
|
||||
vi.stubEnv("TURNSTILE_SITE_KEY", "site-key");
|
||||
expect(ServerEnv.turnstileSiteKey()).toBe("site-key");
|
||||
});
|
||||
|
||||
test("throws when unset", () => {
|
||||
vi.stubEnv("TURNSTILE_SITE_KEY", "");
|
||||
expect(() => ServerEnv.turnstileSiteKey()).toThrow(
|
||||
/TURNSTILE_SITE_KEY not set/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ServerEnv.jwtAudience", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("returns DOMAIN when set", () => {
|
||||
vi.stubEnv("DOMAIN", "openfront.io");
|
||||
expect(ServerEnv.jwtAudience()).toBe("openfront.io");
|
||||
});
|
||||
|
||||
test("throws when DOMAIN unset", () => {
|
||||
vi.stubEnv("DOMAIN", "");
|
||||
expect(() => ServerEnv.jwtAudience()).toThrow(/DOMAIN not set/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ServerEnv.jwtIssuer", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("maps 'localhost' to http://localhost:8787", () => {
|
||||
vi.stubEnv("DOMAIN", "localhost");
|
||||
expect(ServerEnv.jwtIssuer()).toBe("http://localhost:8787");
|
||||
});
|
||||
|
||||
test("derives api.<audience> for non-localhost", () => {
|
||||
vi.stubEnv("DOMAIN", "openfront.io");
|
||||
expect(ServerEnv.jwtIssuer()).toBe("https://api.openfront.io");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ServerEnv.allowedFlares", () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("returns undefined when unset", () => {
|
||||
vi.stubEnv("ALLOWED_FLARES", "");
|
||||
expect(ServerEnv.allowedFlares()).toBeUndefined();
|
||||
});
|
||||
|
||||
test("parses a single value", () => {
|
||||
vi.stubEnv("ALLOWED_FLARES", "admin");
|
||||
expect(ServerEnv.allowedFlares()).toEqual(["admin"]);
|
||||
});
|
||||
|
||||
test("parses CSV", () => {
|
||||
vi.stubEnv("ALLOWED_FLARES", "admin,beta,internal");
|
||||
expect(ServerEnv.allowedFlares()).toEqual(["admin", "beta", "internal"]);
|
||||
});
|
||||
|
||||
test("trims whitespace and drops empties", () => {
|
||||
vi.stubEnv("ALLOWED_FLARES", " admin , , beta ");
|
||||
expect(ServerEnv.allowedFlares()).toEqual(["admin", "beta"]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user