From efb4ca7c2f50fc5a17a9f926feccc21e35c0ecf6 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 5 Nov 2026 02:06:12 +0000 Subject: [PATCH 1/8] =?UTF-8?q?Refactor:=20factory/api=20=E3=81=AE=20requi?= =?UTF-8?q?reEnv=20=E3=82=92=20env.ts=20=E3=81=AE=201=20=E3=81=A4=E3=81=AB?= =?UTF-8?q?=E3=81=97=E3=80=81handlers/=20=E3=82=92=20handler=20=E3=81=AE?= =?UTF-8?q?=E9=85=8D=E7=B7=9A=E3=81=A0=E3=81=91=E3=81=AB=E3=81=99=E3=82=8B?= =?UTF-8?q?=EF=BC=88#2199=20=E3=81=AE=20R20=E3=83=BBR02=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - requireEnv の 5 つの写しを src/env.ts へ寄せる - createApiApp・jsonLogger・webOriginFromEnv を app.ts、depsFromEnv を auth/deps.ts、 progressStoreFromEnv を progress/deps.ts へ移し、index.ts・runs/deps.ts・handlers/ どうしが handlers/ を読まない形にする - テストは createApiApp の import 元だけを替える（期待値は変えない） Co-Authored-By: Claude Opus 5.5 --- factory/api/src/app.ts | 82 ++++++++++++++ factory/api/src/auth/deps.ts | 28 +++++ factory/api/src/env.ts | 6 + factory/api/src/handlers/api.test.ts | 2 +- factory/api/src/handlers/api.ts | 116 +------------------- factory/api/src/handlers/github-oidc.ts | 7 +- factory/api/src/handlers/ingest.ts | 20 +--- factory/api/src/handlers/issue-requests.ts | 9 +- factory/api/src/index.ts | 6 +- factory/api/src/progress/deps.ts | 14 +++ factory/api/src/routes/factory.test.ts | 2 +- factory/api/src/routes/sprint-views.test.ts | 2 +- factory/api/src/runs/deps.ts | 9 +- factory/api/src/testing/app.ts | 2 +- 14 files changed, 147 insertions(+), 158 deletions(-) create mode 100644 factory/api/src/app.ts create mode 100644 factory/api/src/auth/deps.ts create mode 100644 factory/api/src/env.ts create mode 100644 factory/api/src/progress/deps.ts diff --git a/factory/api/src/app.ts b/factory/api/src/app.ts new file mode 100644 index 000000000..b0a1c7e48 --- /dev/null +++ b/factory/api/src/app.ts @@ -0,0 +1,82 @@ +// API の app を組む（ログ・CORS・操作の記録・認証と解決の中間処理・経路）。Lambda の handler は handlers/api.ts が配線する +import { Hono } from "hono"; +import { cors } from "hono/cors"; +import { ApiError, AccountUnavailableError } from "./errors.ts"; +import { requireCaller, type AuthDeps, type AuthEnv } from "./auth/resolve.ts"; +import { registerMe } from "./routes/me.ts"; +import { registerTeams } from "./routes/teams.ts"; +import { registerAccounts } from "./routes/accounts.ts"; +import { registerDelegations } from "./routes/delegations.ts"; +import { recordOperations, requestIdOf } from "./audit/middleware.ts"; +import { registerMcp } from "./mcp/server.ts"; +import { registerFactory, type FactoryDeps } from "./routes/factory.ts"; + +export interface LogEntry { + // 操作の記録の record_id と同じ値（同じ要求の記録とログの行をたどる） + request_id: string; + user_id: string | null; + method: string; + path: string; + status: number; +} + +export type Logger = (entry: LogEntry) => void; + +// ログは利用者の ID・経路・状態コードだけを出す。token とメールアドレスは出さない +export const jsonLogger: Logger = (entry) => console.log(JSON.stringify(entry)); + +// 画面のオリジン（FACTORY_WEB_URL のスキーム・ホスト・ポート）。無いか URL でなければ null で、どのオリジンも許さない +export function webOriginFromEnv(): string | null { + const url = process.env.FACTORY_WEB_URL; + if (!url) return null; + try { + return new URL(url).origin; + } catch { + return null; + } +} + +// API の入口。#2171・#2186・#2174 は、返った app に経路を足し、同じ認証と解決の中間処理を共有する（決定 11） +export function createApiApp( + deps: () => AuthDeps, + log: Logger = jsonLogger, + webOrigin: () => string | null = webOriginFromEnv, + factory: (() => FactoryDeps) | null = null, +): Hono { + const app = new Hono(); + app.use("*", async (c, next) => { + c.set("requestId", requestIdOf(c)); + await next(); + log({ + request_id: c.get("requestId")!, + user_id: c.get("caller")?.userId ?? null, + method: c.req.method, + path: c.req.path, + status: c.res.status, + }); + }); + // 画面と API は別のオリジン。preflight（Authorization を持たない OPTIONS）を認証より先に返す + app.use( + "*", + cors({ + origin: (origin) => (origin === webOrigin() ? origin : null), + allowHeaders: ["Authorization", "Content-Type"], + allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE"], + }), + ); + // 操作の記録は認証より外に置き、401 で断った要求も記録する（I7） + app.use("*", recordOperations(() => deps().audit, (line) => console.log(JSON.stringify(line)))); + app.use("*", requireCaller(deps)); + app.onError((err, c) => { + const known = err instanceof ApiError ? err : new AccountUnavailableError(); + return c.json(known.body(), known.status); + }); + registerMe(app); + registerTeams(app); + registerAccounts(app); + registerDelegations(app); + if (factory) registerFactory(app, factory); + // MCP の受け口。依存は同じアプリへの内部の要求だけ（#2384 の I15） + registerMcp(app, (path, init, env) => app.request(path, init, env), webOrigin); + return app; +} diff --git a/factory/api/src/auth/deps.ts b/factory/api/src/auth/deps.ts new file mode 100644 index 000000000..854d4ee29 --- /dev/null +++ b/factory/api/src/auth/deps.ts @@ -0,0 +1,28 @@ +// API の認証と解決の依存を環境変数から作る +import { AccountService } from "../accounts/service.ts"; +import { DynamoAccountStore } from "../accounts/store.ts"; +import { DynamoAuditLog } from "../audit/dynamo-log.ts"; +import { StructuredLogAudit } from "../audit/structured-log.ts"; +import { DelegationService } from "../delegations/service.ts"; +import { DynamoDelegationStore } from "../delegations/store.ts"; +import { requireEnv } from "../env.ts"; +import type { AuthDeps } from "./resolve.ts"; +import { createAccessTokenVerifier } from "./verify.ts"; + +let cached: AuthDeps | null = null; + +// 環境変数から依存を作る。起動のたびに 1 回だけ作る +export function depsFromEnv(): AuthDeps { + cached ??= { + verifier: createAccessTokenVerifier({ + userPoolId: requireEnv("COGNITO_USER_POOL_ID"), + clientId: requireEnv("COGNITO_CLIENT_ID"), + }), + accounts: new AccountService({ store: new DynamoAccountStore(requireEnv("ACCOUNTS_TABLE_NAME")) }), + // 委任の資格情報の表（インフラが DELEGATIONS_TABLE_NAME で渡す）。無い間は委任の資格情報を 401 にし、委任の操作を 503 にする + delegations: process.env.DELEGATIONS_TABLE_NAME ? new DelegationService({ store: new DynamoDelegationStore(process.env.DELEGATIONS_TABLE_NAME) }) : null, + // 操作の記録の表（インフラが AUDIT_TABLE_NAME で渡す）。無い間は記録を構造化ログへ出す + audit: process.env.AUDIT_TABLE_NAME ? new DynamoAuditLog(process.env.AUDIT_TABLE_NAME) : new StructuredLogAudit(), + }; + return cached; +} diff --git a/factory/api/src/env.ts b/factory/api/src/env.ts new file mode 100644 index 000000000..ebbffeae3 --- /dev/null +++ b/factory/api/src/env.ts @@ -0,0 +1,6 @@ +// 環境変数を読む。無ければ起動の誤りとして投げる（API・受け口・GitHub の仲立ち・起票の代行・実行の制御が使う） +export function requireEnv(name: string): string { + const value = process.env[name]; + if (!value) throw new Error(`環境変数 ${name} がありません`); + return value; +} diff --git a/factory/api/src/handlers/api.test.ts b/factory/api/src/handlers/api.test.ts index 6c7bdca01..d32c0a5f7 100644 --- a/factory/api/src/handlers/api.test.ts +++ b/factory/api/src/handlers/api.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it } from "vitest"; import { createTestApi, firstResult, type TestApi } from "../testing/app.ts"; -import { createApiApp } from "./api.ts"; +import { createApiApp } from "../app.ts"; let api: TestApi; let keys = 0; diff --git a/factory/api/src/handlers/api.ts b/factory/api/src/handlers/api.ts index d6b306326..fa1fcf881 100644 --- a/factory/api/src/handlers/api.ts +++ b/factory/api/src/handlers/api.ts @@ -1,117 +1,7 @@ -import { Hono } from "hono"; +// API Gateway の HTTP API（payload 2.0）から呼ばれる Lambda の handler import { handle } from "hono/aws-lambda"; -import { cors } from "hono/cors"; -import { ApiError, AccountUnavailableError } from "../errors.ts"; -import { AccountService } from "../accounts/service.ts"; -import { DynamoAccountStore } from "../accounts/store.ts"; -import { createAccessTokenVerifier } from "../auth/verify.ts"; -import { requireCaller, type AuthDeps, type AuthEnv } from "../auth/resolve.ts"; -import { registerMe } from "../routes/me.ts"; -import { registerTeams } from "../routes/teams.ts"; -import { registerAccounts } from "../routes/accounts.ts"; -import { registerDelegations } from "../routes/delegations.ts"; -import { DelegationService } from "../delegations/service.ts"; -import { DynamoDelegationStore } from "../delegations/store.ts"; -import { recordOperations, requestIdOf } from "../audit/middleware.ts"; -import { DynamoAuditLog } from "../audit/dynamo-log.ts"; -import { StructuredLogAudit } from "../audit/structured-log.ts"; -import { registerMcp } from "../mcp/server.ts"; -import { registerFactory, type FactoryDeps } from "../routes/factory.ts"; +import { createApiApp, jsonLogger, webOriginFromEnv } from "../app.ts"; +import { depsFromEnv } from "../auth/deps.ts"; import { factoryDepsFromEnv } from "../runs/deps.ts"; -export interface LogEntry { - // 操作の記録の record_id と同じ値（同じ要求の記録とログの行をたどる） - request_id: string; - user_id: string | null; - method: string; - path: string; - status: number; -} - -export type Logger = (entry: LogEntry) => void; - -// ログは利用者の ID・経路・状態コードだけを出す。token とメールアドレスは出さない -export const jsonLogger: Logger = (entry) => console.log(JSON.stringify(entry)); - -// 画面のオリジン（FACTORY_WEB_URL のスキーム・ホスト・ポート）。無いか URL でなければ null で、どのオリジンも許さない -export function webOriginFromEnv(): string | null { - const url = process.env.FACTORY_WEB_URL; - if (!url) return null; - try { - return new URL(url).origin; - } catch { - return null; - } -} - -// API の入口。#2171・#2186・#2174 は、返った app に経路を足し、同じ認証と解決の中間処理を共有する（決定 11） -export function createApiApp( - deps: () => AuthDeps, - log: Logger = jsonLogger, - webOrigin: () => string | null = webOriginFromEnv, - factory: (() => FactoryDeps) | null = null, -): Hono { - const app = new Hono(); - app.use("*", async (c, next) => { - c.set("requestId", requestIdOf(c)); - await next(); - log({ - request_id: c.get("requestId")!, - user_id: c.get("caller")?.userId ?? null, - method: c.req.method, - path: c.req.path, - status: c.res.status, - }); - }); - // 画面と API は別のオリジン。preflight（Authorization を持たない OPTIONS）を認証より先に返す - app.use( - "*", - cors({ - origin: (origin) => (origin === webOrigin() ? origin : null), - allowHeaders: ["Authorization", "Content-Type"], - allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE"], - }), - ); - // 操作の記録は認証より外に置き、401 で断った要求も記録する（I7） - app.use("*", recordOperations(() => deps().audit, (line) => console.log(JSON.stringify(line)))); - app.use("*", requireCaller(deps)); - app.onError((err, c) => { - const known = err instanceof ApiError ? err : new AccountUnavailableError(); - return c.json(known.body(), known.status); - }); - registerMe(app); - registerTeams(app); - registerAccounts(app); - registerDelegations(app); - if (factory) registerFactory(app, factory); - // MCP の受け口。依存は同じアプリへの内部の要求だけ（#2384 の I15） - registerMcp(app, (path, init, env) => app.request(path, init, env), webOrigin); - return app; -} - -function requireEnv(name: string): string { - const value = process.env[name]; - if (!value) throw new Error(`環境変数 ${name} がありません`); - return value; -} - -let cached: AuthDeps | null = null; - -// 環境変数から依存を作る。起動のたびに 1 回だけ作る -export function depsFromEnv(): AuthDeps { - cached ??= { - verifier: createAccessTokenVerifier({ - userPoolId: requireEnv("COGNITO_USER_POOL_ID"), - clientId: requireEnv("COGNITO_CLIENT_ID"), - }), - accounts: new AccountService({ store: new DynamoAccountStore(requireEnv("ACCOUNTS_TABLE_NAME")) }), - // 委任の資格情報の表（インフラが DELEGATIONS_TABLE_NAME で渡す）。無い間は委任の資格情報を 401 にし、委任の操作を 503 にする - delegations: process.env.DELEGATIONS_TABLE_NAME ? new DelegationService({ store: new DynamoDelegationStore(process.env.DELEGATIONS_TABLE_NAME) }) : null, - // 操作の記録の表（インフラが AUDIT_TABLE_NAME で渡す）。無い間は記録を構造化ログへ出す - audit: process.env.AUDIT_TABLE_NAME ? new DynamoAuditLog(process.env.AUDIT_TABLE_NAME) : new StructuredLogAudit(), - }; - return cached; -} - -// API Gateway の HTTP API（payload 2.0）から呼ばれる Lambda の handler export const handler = handle(createApiApp(depsFromEnv, jsonLogger, webOriginFromEnv, factoryDepsFromEnv)); diff --git a/factory/api/src/handlers/github-oidc.ts b/factory/api/src/handlers/github-oidc.ts index 10ee35b99..8ef28f435 100644 --- a/factory/api/src/handlers/github-oidc.ts +++ b/factory/api/src/handlers/github-oidc.ts @@ -1,14 +1,9 @@ import { handle } from "hono/aws-lambda"; import { Hono } from "hono"; +import { requireEnv } from "../env.ts"; import { createGithubOidcApp } from "../github-oidc/app.ts"; import { loadSigningKeys, type SigningKeys } from "../github-oidc/keys.ts"; -function requireEnv(name: string): string { - const value = process.env[name]; - if (!value) throw new Error(`環境変数 ${name} がありません`); - return value; -} - // 署名の鍵は起動のたびに 1 回だけ読む let keys: Promise | null = null; let app: Hono | null = null; diff --git a/factory/api/src/handlers/ingest.ts b/factory/api/src/handlers/ingest.ts index 59e3ac681..127d96a4f 100644 --- a/factory/api/src/handlers/ingest.ts +++ b/factory/api/src/handlers/ingest.ts @@ -5,24 +5,8 @@ import { handle } from "hono/aws-lambda"; import { createIngestApp, type IngestDeps } from "../progress/app.ts"; import { runJobTokenVerifierFromEnv } from "../progress/run-job-token.ts"; import { savedAnswersFromEnv } from "../progress/saved-answers.ts"; -import { DynamoProgressEventStore, type ProgressEventStore } from "../progress/store.ts"; - -function requireEnv(name: string): string { - const value = process.env[name]; - if (!value) throw new Error(`環境変数 ${name} がありません`); - return value; -} - -let store: ProgressEventStore | null = null; - -// 進捗の記録の保存。受け口と appendControlEvent が同じものを使う -export function progressStoreFromEnv(): ProgressEventStore { - store ??= new DynamoProgressEventStore( - requireEnv("PROGRESS_EVENTS_TABLE_NAME"), - requireEnv("PROGRESS_IDEMPOTENCY_TABLE_NAME"), - ); - return store; -} +import { requireEnv } from "../env.ts"; +import { progressStoreFromEnv } from "../progress/deps.ts"; let webhookSecret: Promise | null = null; diff --git a/factory/api/src/handlers/issue-requests.ts b/factory/api/src/handlers/issue-requests.ts index ac94df141..18ab1b4f5 100644 --- a/factory/api/src/handlers/issue-requests.ts +++ b/factory/api/src/handlers/issue-requests.ts @@ -4,18 +4,13 @@ import type { AttributeValue } from "@aws-sdk/client-dynamodb"; import { unmarshall } from "@aws-sdk/util-dynamodb"; import type { DynamoDBBatchResponse, DynamoDBStreamEvent } from "aws-lambda"; import type { IssueRequestedEvent } from "@ndf-factory/contracts"; +import { requireEnv } from "../env.ts"; import { DynamoIssueLedger } from "../issue-requests/ledger.ts"; import { DEFAULT_ISSUE_REQUEST_LIMIT_PER_RUN, resolveIssueRequest, type IssueRequestDeps } from "../issue-requests/resolve.ts"; import { defaultFilingStyle } from "../issue-requests/style.ts"; import { appendIssueRequestResolvedTo } from "../progress/append.ts"; import { factoryDepsFromEnv } from "../runs/deps.ts"; -import { progressStoreFromEnv } from "./ingest.ts"; - -function requireEnv(name: string): string { - const value = process.env[name]; - if (!value) throw new Error(`環境変数 ${name} がありません`); - return value; -} +import { progressStoreFromEnv } from "../progress/deps.ts"; // 上限は ISSUE_REQUEST_LIMIT_PER_RUN（1 以上の整数）。無い・形が違えば既定の 10 export function issueRequestLimit(env: Record): number { diff --git a/factory/api/src/index.ts b/factory/api/src/index.ts index 65ec4f550..1973cfe5e 100644 --- a/factory/api/src/index.ts +++ b/factory/api/src/index.ts @@ -1,14 +1,14 @@ // 他の子課題（#2171・#2186・#2174・#2189）へ公開する入口。使うのはここから export したものだけ import type { Hono } from "hono"; import { AccountService } from "./accounts/service.ts"; -import { depsFromEnv } from "./handlers/api.ts"; +import { depsFromEnv } from "./auth/deps.ts"; import { verifyAuthorization } from "./auth/verify.ts"; import type { AuthEnv } from "./auth/resolve.ts"; import type { AccountContext, Caller, Plan } from "./accounts/model.ts"; import type { SprintRun } from "./runs/model.ts"; import type { IssueRequestResolvedEvent, JobFinishedEvent } from "@ndf-factory/contracts"; import { appendControlEventTo, appendIssueRequestResolvedTo } from "./progress/append.ts"; -import { progressStoreFromEnv } from "./handlers/ingest.ts"; +import { progressStoreFromEnv } from "./progress/deps.ts"; import { runJobTokenVerifierFromEnv, type VerifiedRunJob } from "./progress/run-job-token.ts"; import { factoryDepsFromEnv } from "./runs/deps.ts"; @@ -16,7 +16,7 @@ export { getLimits, judgeLimits, LIMITS_TABLE } from "./accounts/limits.ts"; export type { CurrentUsage, Judgment, LimitReason, Limits } from "./accounts/limits.ts"; export type { AccountContext, Caller, ExecutionMode, Plan, SeatType } from "./accounts/model.ts"; export { ApiError, AuthError, ForbiddenError, PlanError } from "./errors.ts"; -export { createApiApp } from "./handlers/api.ts"; +export { createApiApp } from "./app.ts"; export { requireCaller, resolveFromRequest } from "./auth/resolve.ts"; export type { AuthEnv }; export type ApiApp = Hono; diff --git a/factory/api/src/progress/deps.ts b/factory/api/src/progress/deps.ts new file mode 100644 index 000000000..ce4a68436 --- /dev/null +++ b/factory/api/src/progress/deps.ts @@ -0,0 +1,14 @@ +// 進捗の記録の保存を環境変数から作る +import { requireEnv } from "../env.ts"; +import { DynamoProgressEventStore, type ProgressEventStore } from "./store.ts"; + +let store: ProgressEventStore | null = null; + +// 進捗の記録の保存。受け口と appendControlEvent が同じものを使う +export function progressStoreFromEnv(): ProgressEventStore { + store ??= new DynamoProgressEventStore( + requireEnv("PROGRESS_EVENTS_TABLE_NAME"), + requireEnv("PROGRESS_IDEMPOTENCY_TABLE_NAME"), + ); + return store; +} diff --git a/factory/api/src/routes/factory.test.ts b/factory/api/src/routes/factory.test.ts index eea325c60..b9654b4f2 100644 --- a/factory/api/src/routes/factory.test.ts +++ b/factory/api/src/routes/factory.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it } from "vitest"; import { AccountService } from "../accounts/service.ts"; import { MemoryAccountStore } from "../accounts/memory-store.ts"; import { CredentialService, type SecretsPort } from "../credentials/service.ts"; -import { createApiApp, type LogEntry } from "../handlers/api.ts"; +import { createApiApp, type LogEntry } from "../app.ts"; import { RepositoryService } from "../repositories/service.ts"; import { memoryProgressReader } from "../progress/read.ts"; import { CANDIDATE_FALLBACK_NOTE, type CandidateModel } from "../runs/candidates.ts"; diff --git a/factory/api/src/routes/sprint-views.test.ts b/factory/api/src/routes/sprint-views.test.ts index c463e8cbb..bace032ea 100644 --- a/factory/api/src/routes/sprint-views.test.ts +++ b/factory/api/src/routes/sprint-views.test.ts @@ -3,7 +3,7 @@ import type { ProgressEvent } from "@ndf-factory/contracts"; import { AccountService } from "../accounts/service.ts"; import { MemoryAccountStore } from "../accounts/memory-store.ts"; import { CredentialService, type SecretsPort } from "../credentials/service.ts"; -import { createApiApp } from "../handlers/api.ts"; +import { createApiApp } from "../app.ts"; import type { GitHubPort } from "../github/port.ts"; import { appendEvent } from "../progress/append.ts"; import { memoryProgressReader } from "../progress/read.ts"; diff --git a/factory/api/src/runs/deps.ts b/factory/api/src/runs/deps.ts index 750f4118d..38c5e12a2 100644 --- a/factory/api/src/runs/deps.ts +++ b/factory/api/src/runs/deps.ts @@ -8,7 +8,8 @@ import { OrganizerControl } from "../organizer/control.ts"; import { appendControlEventTo, appendReleaseAnsweredTo } from "../progress/append.ts"; import { DynamoProgressReader } from "../progress/read.ts"; import type { RunJobSigningKey } from "../progress/run-job-token.ts"; -import { progressStoreFromEnv } from "../handlers/ingest.ts"; +import { progressStoreFromEnv } from "../progress/deps.ts"; +import { requireEnv } from "../env.ts"; import { RepositoryService } from "../repositories/service.ts"; import type { FactoryDeps } from "../routes/factory.ts"; import { bedrockCandidateModel } from "./candidates.ts"; @@ -19,12 +20,6 @@ import { AccountService } from "../accounts/service.ts"; import { DynamoAccountStore } from "../accounts/store.ts"; import { accountContextResolver, LoopControl } from "./loops.ts"; -function requireEnv(name: string): string { - const value = process.env[name]; - if (!value) throw new Error(`環境変数 ${name} がありません`); - return value; -} - const list = (name: string) => requireEnv(name).split(",").map((s) => s.trim()).filter(Boolean); // 秘密を 1 回だけ読む。読めなければ次の呼び出しで読み直す。値をログと応答に出さない diff --git a/factory/api/src/testing/app.ts b/factory/api/src/testing/app.ts index 292ba14be..aa967de2a 100644 --- a/factory/api/src/testing/app.ts +++ b/factory/api/src/testing/app.ts @@ -4,7 +4,7 @@ import { MemoryAccountStore } from "../accounts/memory-store.ts"; import { MemoryDelegationStore } from "../delegations/memory-store.ts"; import { MemoryAuditLog } from "../audit/memory-log.ts"; import { DelegationService } from "../delegations/service.ts"; -import { createApiApp, type LogEntry } from "../handlers/api.ts"; +import { createApiApp, type LogEntry } from "../app.ts"; import { createTokenKit, type TokenKit } from "./tokens.ts"; import type { FactoryDeps } from "../routes/factory.ts"; import { CredentialService, type SecretsPort } from "../credentials/service.ts"; From b6a20863b7ae4219a8a3c9cd2047221dc0ae6735 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 5 Nov 2026 02:08:24 +0000 Subject: [PATCH 2/8] =?UTF-8?q?Refactor:=20=E3=82=A2=E3=82=AB=E3=82=A6?= =?UTF-8?q?=E3=83=B3=E3=83=88=E3=81=AE=E4=BF=9D=E5=AD=98=E3=81=AE=E5=A5=91?= =?UTF-8?q?=E7=B4=84=E3=82=92=20port.ts=20=E3=81=AB=E5=88=86=E3=81=91?= =?UTF-8?q?=E3=80=81=E6=9B=B8=E3=81=8D=E8=BE=BC=E3=81=BF=E3=81=AE=E9=87=8D?= =?UTF-8?q?=E8=A4=87=E3=82=92=E5=AF=84=E3=81=9B=E3=82=8B=EF=BC=88#2199=20?= =?UTF-8?q?=E3=81=AE=20R04=E3=83=BBR06=E3=83=BBR08=E3=83=BBR09=E3=83=BBR10?= =?UTF-8?q?=E3=83=BBR11=E3=83=BBR17=E3=83=BBR18=E3=83=BBR19=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AccountStore・AccountPatch・ConditionFailedError と新しい AccountUpdate を accounts/port.ts へ移す - updateAccount の 5 つの位置引数を AccountUpdate 1 つにする - DynamoAccountStore.transact が履歴を受け取り、items → 履歴 → after の順に並べる - AccountPatch の列を PATCH_COLUMNS の表で回す（プレースホルダは :plan・:mode のまま） - 鍵の頭を USER_PREFIX・ACCOUNT_PREFIX と userPk・accountPk にまとめる - AccountService の 4 つの再試行を retryOnConflict、3 つの条件の失敗の読み替えを writeOr にし、hist を消す - store.test.ts は import 元と updateAccount の呼び出しの形だけを替える（期待値は変えない） Co-Authored-By: Claude Opus 5.5 --- factory/api/src/accounts/memory-store.ts | 10 +- factory/api/src/accounts/port.ts | 50 +++++ factory/api/src/accounts/service.ts | 245 +++++++++++++---------- factory/api/src/accounts/store.test.ts | 5 +- factory/api/src/accounts/store.ts | 158 ++++++--------- 5 files changed, 252 insertions(+), 216 deletions(-) create mode 100644 factory/api/src/accounts/port.ts diff --git a/factory/api/src/accounts/memory-store.ts b/factory/api/src/accounts/memory-store.ts index b3eb4d659..a35cf3d75 100644 --- a/factory/api/src/accounts/memory-store.ts +++ b/factory/api/src/accounts/memory-store.ts @@ -1,4 +1,4 @@ -import { ConditionFailedError, type AccountPatch, type AccountStore } from "./store.ts"; +import { ConditionFailedError, type AccountStore, type AccountUpdate } from "./port.ts"; import type { AccountRecord, HistoryRecord, SeatRecord, UserRecord } from "./model.ts"; import { epochSeconds, idempotencyPk, liveCreatedId, type IdempotencyClaim, type IdempotencyRef } from "../idempotency.ts"; @@ -109,13 +109,7 @@ export class MemoryAccountStore implements AccountStore { await this.write("removeSeat", () => this.seatUnchanged(expected), () => this.seats.delete(key), history); } - async updateAccount( - accountId: string, - expectedVersion: number, - patch: AccountPatch, - updatedAt: string, - history: HistoryRecord[], - ) { + async updateAccount({ accountId, expectedVersion, patch, updatedAt, history }: AccountUpdate) { await this.write( "updateAccount", () => this.accounts.get(accountId)?.version === expectedVersion, diff --git a/factory/api/src/accounts/port.ts b/factory/api/src/accounts/port.ts new file mode 100644 index 000000000..3c2c190bf --- /dev/null +++ b/factory/api/src/accounts/port.ts @@ -0,0 +1,50 @@ +// アカウントの保存の契約（AccountStore）と、その引数・失敗の型。DynamoDB とメモリの実装が満たす。AWS SDK を読まない +import type { AccountRecord, HistoryRecord, SeatRecord, UserRecord } from "./model.ts"; +import type { IdempotencyClaim, IdempotencyRef } from "../idempotency.ts"; + +// 条件つきの書き込みが条件で失敗した。呼ぶ側が意味（I12・I15・版の競合など）へ読み替える +export class ConditionFailedError extends Error { + constructor() { + super("condition failed"); + this.name = "ConditionFailedError"; + } +} + +export interface AccountPatch { + plan?: AccountRecord["plan"]; + executionMode?: AccountRecord["executionMode"]; +} + +// アカウントの更新の 1 回分。version が expectedVersion と等しいときだけ patch を当て、version を 1 増やす +export interface AccountUpdate { + accountId: string; + expectedVersion: number; + patch: AccountPatch; + updatedAt: string; + history: HistoryRecord[]; +} + +// アカウントの表への読み書き。どの書き込みも履歴の行を同じトランザクションで足す（I20） +export interface AccountStore { + getUser(userId: string): Promise; + // 利用者の行が無いときだけ、利用者・個人のアカウント・履歴を書く（I12） + createUserWithPersonalAccount(user: UserRecord, account: AccountRecord, history: HistoryRecord[]): Promise; + getAccount(accountId: string): Promise; + getSeat(accountId: string, userId: string): Promise; + listSeats(accountId: string): Promise; + // 利用者が席を持つチームのアカウントの ID（GSI gsi1）。GSI は結果整合のため、 + // 返すのは候補であり、呼ぶ側は getSeat で元の席を確かめてから使う + listMemberships(userId: string): Promise; + // idempotency があれば作成の冪等の鍵の行も同じトランザクションで書く。期限の中の同じ鍵があれば ConditionFailedError（#2384 の I14） + createTeam(account: AccountRecord, ownerSeat: SeatRecord, history: HistoryRecord[], idempotency?: IdempotencyClaim): Promise; + // 作成の冪等の鍵の行が作った識別子。期限（ttl）を過ぎた行は、残っていても無いものとして扱う + getIdempotent(ref: IdempotencyRef, now: string): Promise; + // 席が無いときだけ足す（I15） + addSeat(seat: SeatRecord, history: HistoryRecord[]): Promise; + // 席が読んだときのまま（種類と足した時刻が expected と等しい）のときだけ変える + updateSeat(expected: SeatRecord, seatType: SeatRecord["seatType"], history: HistoryRecord[]): Promise; + // 席が読んだときのまま（種類と足した時刻が expected と等しい）のときだけ外す + removeSeat(expected: SeatRecord, history: HistoryRecord[]): Promise; + // version が expectedVersion と等しいときだけ変え、version を 1 増やす + updateAccount(update: AccountUpdate): Promise; +} diff --git a/factory/api/src/accounts/service.ts b/factory/api/src/accounts/service.ts index 85de493e0..497188c04 100644 --- a/factory/api/src/accounts/service.ts +++ b/factory/api/src/accounts/service.ts @@ -18,13 +18,12 @@ import { type Caller, type ExecutionMode, type HistoryRecord, - type HistoryType, type Plan, type SeatRecord, type SeatType, type UserRecord, } from "./model.ts"; -import { ConditionFailedError, type AccountPatch, type AccountStore } from "./store.ts"; +import { ConditionFailedError, type AccountPatch, type AccountStore } from "./port.ts"; import { claimOf, type IdempotencyRef } from "../idempotency.ts"; // 版の競合で読み直して書き直す回数の上限（docs/specifications/ndf-factory-accounts-and-plans.md の「常に成り立つ条件」） @@ -51,10 +50,6 @@ export class AccountService { return this.now().toISOString(); } - private hist(accountId: string, type: HistoryType, at: string, actor: string, before: unknown, after: unknown): HistoryRecord { - return { accountId, type, at, actor, before, after }; - } - // 新しいアカウントの初期値（I24）。形態は INITIAL_EXECUTION_MODE、版は 1 から始める private newAccount(kind: AccountKind, plan: Plan, ownerUserId: string, name: string | null, at: string): AccountRecord { return { @@ -71,12 +66,14 @@ export class AccountService { } private accountCreated(account: AccountRecord, actor: string, extra?: object): HistoryRecord { - return this.hist(account.accountId, "account.created", account.createdAt, actor, null, { - kind: account.kind, - plan: account.plan, - execution_mode: account.executionMode, - ...extra, - }); + return { + accountId: account.accountId, + type: "account.created", + at: account.createdAt, + actor, + before: null, + after: { kind: account.kind, plan: account.plan, execution_mode: account.executionMode, ...extra }, + }; } // 利用者の行を読み、無ければ利用者と個人のアカウント（Free・local）を 1 つのトランザクションで作る（I12・I24、E2） @@ -91,16 +88,15 @@ export class AccountService { personalAccountId: account.accountId, createdAt: at, }; - try { - await this.store.createUserWithPersonalAccount(user, account, [this.accountCreated(account, caller.userId)]); - return user; - } catch (err) { - if (err instanceof ConditionFailedError) { - const raced = await this.store.getUser(caller.userId); - if (raced) return raced; - } - throw new AccountUnavailableError(); - } + const raced = await writeOr( + () => this.store.createUserWithPersonalAccount(user, account, [this.accountCreated(account, caller.userId)]), + async () => { + const found = await this.store.getUser(caller.userId); + if (!found) throw new AccountUnavailableError(); + return found; + }, + ); + return raced ?? user; } // 要求の利用者と経路のアカウントの ID から、権限つきの文脈を返す（決定 4、I18） @@ -171,23 +167,25 @@ export class AccountService { if (replayed) return this.resolveAccount(caller, replayed); const account = this.newAccount("team", "team", caller.userId, name, at); const seat: SeatRecord = { accountId: account.accountId, userId: caller.userId, seatType, addedAt: at }; - try { - await this.store.createTeam( - account, - seat, - [ - this.accountCreated(account, caller.userId, { name }), - this.hist(account.accountId, "seat.added", at, caller.userId, null, { user_id: caller.userId, seat_type: seatType }), - ], - ref && claimOf(ref, account.accountId, at), - ); - } catch (err) { - // 同じ鍵の作成が同時に走り、先に書いた方が勝った - const won = ref && err instanceof ConditionFailedError ? await this.store.getIdempotent(ref, at) : null; - if (won) return this.resolveAccount(caller, won); - throw new AccountUnavailableError(); - } - return this.context(account, caller.userId, seatType); + const won = await writeOr( + () => + this.store.createTeam( + account, + seat, + [ + this.accountCreated(account, caller.userId, { name }), + { accountId: account.accountId, type: "seat.added", at, actor: caller.userId, before: null, after: { user_id: caller.userId, seat_type: seatType } }, + ], + ref && claimOf(ref, account.accountId, at), + ), + async () => { + // 同じ鍵の作成が同時に走り、先に書いた方が勝った + const id = ref ? await this.store.getIdempotent(ref, at) : null; + if (!id) throw new AccountUnavailableError(); + return this.resolveAccount(caller, id); + }, + ); + return won ?? this.context(account, caller.userId, seatType); } // 席の操作は持ち主だけ（I17）。持ち主でなければ席の有無にかかわらず先に 403 @@ -203,39 +201,35 @@ export class AccountService { } const at = this.stamp(); const seat: SeatRecord = { accountId: ctx.accountId, userId, seatType, addedAt: at }; - try { - await this.store.addSeat(seat, [ - this.hist(ctx.accountId, "seat.added", at, ctx.userId, null, { user_id: userId, seat_type: seatType }), - ]); - } catch (err) { - if (err instanceof ConditionFailedError) { + await writeOr( + () => + this.store.addSeat(seat, [ + { accountId: ctx.accountId, type: "seat.added", at, actor: ctx.userId, before: null, after: { user_id: userId, seat_type: seatType } }, + ]), + () => { throw new ConflictError("seat_exists", "その利用者は既にこのチームの席を持っています"); - } - throw new AccountUnavailableError(); - } + }, + ); return seat; } async changeSeat(ctx: AccountContext, userId: string, seatType: SeatType): Promise { this.requireTeamOwner(ctx); // 読んだ席のままのときだけ書く。競合したら読み直し、履歴の before を作り直す - for (let attempt = 0; attempt < VERSION_RETRIES; attempt++) { + return retryOnConflict(async () => { const current = await this.store.getSeat(ctx.accountId, userId); if (!current) throw seatNotFound(); const at = this.stamp(); - try { - await this.store.updateSeat(current, seatType, [ - this.hist(ctx.accountId, "seat.changed", at, ctx.userId, { user_id: userId, seat_type: current.seatType }, { - user_id: userId, - seat_type: seatType, - }), - ]); - return { ...current, seatType }; - } catch (err) { - if (!(err instanceof ConditionFailedError)) throw new AccountUnavailableError(); - } - } - throw new AccountUnavailableError(); + const history: HistoryRecord = { + accountId: ctx.accountId, + type: "seat.changed", + at, + actor: ctx.userId, + before: { user_id: userId, seat_type: current.seatType }, + after: { user_id: userId, seat_type: seatType }, + }; + return { write: () => this.store.updateSeat(current, seatType, [history]), result: { ...current, seatType } }; + }); } async removeSeat(ctx: AccountContext, userId: string): Promise { @@ -245,72 +239,111 @@ export class AccountService { throw new ConflictError("owner_seat_protected", "チームの持ち主の席は外せません"); } // 読んだ席のままのときだけ外す。外して足し直した新しい席を古い要求で消さない - for (let attempt = 0; attempt < VERSION_RETRIES; attempt++) { + await retryOnConflict(async () => { const current = await this.store.getSeat(ctx.accountId, userId); if (!current) throw seatNotFound(); const at = this.stamp(); - try { - await this.store.removeSeat(current, [ - this.hist(ctx.accountId, "seat.removed", at, ctx.userId, { user_id: userId, seat_type: current.seatType }, null), - ]); - return; - } catch (err) { - if (!(err instanceof ConditionFailedError)) throw new AccountUnavailableError(); - } - } - throw new AccountUnavailableError(); + const history: HistoryRecord = { + accountId: ctx.accountId, + type: "seat.removed", + at, + actor: ctx.userId, + before: { user_id: userId, seat_type: current.seatType }, + after: null, + }; + return { write: () => this.store.removeSeat(current, [history]), result: undefined }; + }); } // 実行の形態を変える。持ち主だけ（I22）。Free で managed は断る（I21） async setExecutionMode(ctx: AccountContext, mode: ExecutionMode): Promise { if (ctx.role !== "owner") throw new ForbiddenError(); - for (let attempt = 0; attempt < VERSION_RETRIES; attempt++) { + return retryOnConflict(async () => { const account = await this.store.getAccount(ctx.accountId); if (!account) throw new ForbiddenError(); - if (account.executionMode === mode) return account; + if (account.executionMode === mode) return { done: account }; if (mode === "managed" && account.plan === "free") { throw new ConflictError("managed_not_allowed_on_free", "Free の料金プランでは managed（サービスの Bedrock）を選べません"); } const at = this.stamp(); - try { - await this.store.updateAccount(account.accountId, account.version, { executionMode: mode }, at, [ - this.hist(account.accountId, "execution_mode.changed", at, ctx.userId, account.executionMode, mode), - ]); - return { ...account, executionMode: mode, version: account.version + 1, updatedAt: at }; - } catch (err) { - if (!(err instanceof ConditionFailedError)) throw new AccountUnavailableError(); - } - } - throw new AccountUnavailableError(); + const history: HistoryRecord = { + accountId: account.accountId, + type: "execution_mode.changed", + at, + actor: ctx.userId, + before: account.executionMode, + after: mode, + }; + return { + write: () => + this.store.updateAccount({ accountId: account.accountId, expectedVersion: account.version, patch: { executionMode: mode }, updatedAt: at, history: [history] }), + result: { ...account, executionMode: mode, version: account.version + 1, updatedAt: at }, + }; + }); } // アカウントの料金プランを変える。#2174 が決済の後に呼ぶ内側の関数（前提 8、E6） // free へ下げるとき形態が managed なら、同じトランザクションで local へ変える（I21） async setPlan(accountId: string, plan: Plan): Promise { if (!isPlan(plan)) throw new PlanError("unknown_plan", `知らない料金プランです: ${String(plan)}`); - for (let attempt = 0; attempt < VERSION_RETRIES; attempt++) { - const account = await this.store.getAccount(accountId); - if (!account) throw new PlanError("account_not_found", "アカウントがありません"); - if (!planFitsKind(account.kind, plan)) { - throw new PlanError("plan_kind_mismatch", `${account.kind} のアカウントに ${plan} は置けません`); - } - if (account.plan === plan) return; - const at = this.stamp(); - const patch: AccountPatch = { plan }; - const history = [this.hist(accountId, "plan.changed", at, "system", account.plan, plan)]; - if (plan === "free" && account.executionMode === "managed") { - patch.executionMode = "local"; - history.push(this.hist(accountId, "execution_mode.changed", at, "system", "managed", "local")); - } - try { - await this.store.updateAccount(accountId, account.version, patch, at, history); - return; - } catch (err) { - if (!(err instanceof ConditionFailedError)) throw err; - } + // 条件以外の書き込みの失敗は包まずにそのまま返す（呼ぶ側の #2174 が受ける） + await retryOnConflict( + async () => { + const account = await this.store.getAccount(accountId); + if (!account) throw new PlanError("account_not_found", "アカウントがありません"); + if (!planFitsKind(account.kind, plan)) { + throw new PlanError("plan_kind_mismatch", `${account.kind} のアカウントに ${plan} は置けません`); + } + if (account.plan === plan) return { done: undefined }; + const at = this.stamp(); + const patch: AccountPatch = { plan }; + const history: HistoryRecord[] = [{ accountId, type: "plan.changed", at, actor: "system", before: account.plan, after: plan }]; + if (plan === "free" && account.executionMode === "managed") { + patch.executionMode = "local"; + history.push({ accountId, type: "execution_mode.changed", at, actor: "system", before: "managed", after: "local" }); + } + return { + write: () => this.store.updateAccount({ accountId, expectedVersion: account.version, patch, updatedAt: at, history }), + result: undefined, + }; + }, + (err) => err, + ); + } +} + +// 読み直しと判定の 1 回分の結果。書かずに終わる（done）か、書いて終わる（write が成功したら result） +type ConflictStep = { done: T } | { write: () => Promise; result: T }; + +// 版の競合（条件の失敗）なら step から読み直して上限の回数まで書き直す。step の中で投げた誤りはそのまま届く。 +// 条件以外の書き込みの失敗は onOther の結果を投げる（既定は AccountUnavailableError） +async function retryOnConflict( + step: () => Promise>, + onOther: (err: unknown) => unknown = () => new AccountUnavailableError(), +): Promise { + for (let attempt = 0; attempt < VERSION_RETRIES; attempt++) { + const next = await step(); + if ("done" in next) return next.done; + try { + await next.write(); + return next.result; + } catch (err) { + if (!(err instanceof ConditionFailedError)) throw onOther(err); } - throw new AccountUnavailableError(); } + throw new AccountUnavailableError(); +} + +// 再試行しない書き込み。成功なら undefined、条件の失敗なら onCondition の結果を返す（onCondition は投げてもよい）。 +// 条件以外の失敗は AccountUnavailableError +async function writeOr(write: () => Promise, onCondition: () => T | Promise): Promise { + try { + await write(); + return undefined; + } catch (err) { + if (!(err instanceof ConditionFailedError)) throw new AccountUnavailableError(); + } + return onCondition(); } function seatNotFound(): NotFoundError { diff --git a/factory/api/src/accounts/store.test.ts b/factory/api/src/accounts/store.test.ts index 9cea02796..abcf325e9 100644 --- a/factory/api/src/accounts/store.test.ts +++ b/factory/api/src/accounts/store.test.ts @@ -2,7 +2,8 @@ import { beforeEach, describe, expect, it } from "vitest"; import { mockClient } from "aws-sdk-client-mock"; import { DynamoDBClient, TransactionCanceledException } from "@aws-sdk/client-dynamodb"; import { DynamoDBDocumentClient, GetCommand, QueryCommand, TransactWriteCommand } from "@aws-sdk/lib-dynamodb"; -import { ConditionFailedError, DynamoAccountStore } from "./store.ts"; +import { ConditionFailedError } from "./port.ts"; +import { DynamoAccountStore } from "./store.ts"; import type { AccountRecord, SeatRecord } from "./model.ts"; const ddb = mockClient(DynamoDBDocumentClient); @@ -53,7 +54,7 @@ describe("DynamoAccountStore（決定 8）", () => { it("アカウントの更新は版を条件にし、版を 1 増やす", async () => { ddb.on(TransactWriteCommand).resolves({}); - await store.updateAccount("a-1", 4, { plan: "free", executionMode: "local" }, account.updatedAt, []); + await store.updateAccount({ accountId: "a-1", expectedVersion: 4, patch: { plan: "free", executionMode: "local" }, updatedAt: account.updatedAt, history: [] }); const update = ddb.commandCalls(TransactWriteCommand)[0]!.args[0].input.TransactItems![0]!.Update!; expect(update.ConditionExpression).toBe("attribute_exists(#pk) AND #version = :expected"); expect(update.ExpressionAttributeValues).toMatchObject({ ":expected": 4, ":next": 5, ":plan": "free", ":mode": "local" }); diff --git a/factory/api/src/accounts/store.ts b/factory/api/src/accounts/store.ts index 30e626b18..85f43eb5e 100644 --- a/factory/api/src/accounts/store.ts +++ b/factory/api/src/accounts/store.ts @@ -8,55 +8,18 @@ import { type TransactWriteCommandInput, } from "@aws-sdk/lib-dynamodb"; import type { AccountRecord, HistoryRecord, SeatRecord, UserRecord } from "./model.ts"; +import { ConditionFailedError, type AccountPatch, type AccountStore, type AccountUpdate } from "./port.ts"; import { epochSeconds, idempotencyPk, liveCreatedId, type IdempotencyClaim, type IdempotencyRef } from "../idempotency.ts"; -// 条件つきの書き込みが条件で失敗した。呼ぶ側が意味（I12・I15・版の競合など）へ読み替える -export class ConditionFailedError extends Error { - constructor() { - super("condition failed"); - this.name = "ConditionFailedError"; - } -} +// 鍵の頭。表の pk と GSI gsi1 の鍵は、利用者とアカウントの ID にこの頭を付ける +const USER_PREFIX = "USER#"; +const ACCOUNT_PREFIX = "ACCOUNT#"; +const userPk = (userId: string) => `${USER_PREFIX}${userId}`; +const accountPk = (accountId: string) => `${ACCOUNT_PREFIX}${accountId}`; -export interface AccountPatch { - plan?: AccountRecord["plan"]; - executionMode?: AccountRecord["executionMode"]; -} - -// アカウントの表への読み書き。どの書き込みも履歴の行を同じトランザクションで足す（I20） -export interface AccountStore { - getUser(userId: string): Promise; - // 利用者の行が無いときだけ、利用者・個人のアカウント・履歴を書く（I12） - createUserWithPersonalAccount(user: UserRecord, account: AccountRecord, history: HistoryRecord[]): Promise; - getAccount(accountId: string): Promise; - getSeat(accountId: string, userId: string): Promise; - listSeats(accountId: string): Promise; - // 利用者が席を持つチームのアカウントの ID（GSI gsi1）。GSI は結果整合のため、 - // 返すのは候補であり、呼ぶ側は getSeat で元の席を確かめてから使う - listMemberships(userId: string): Promise; - // idempotency があれば作成の冪等の鍵の行も同じトランザクションで書く。期限の中の同じ鍵があれば ConditionFailedError（#2384 の I14） - createTeam(account: AccountRecord, ownerSeat: SeatRecord, history: HistoryRecord[], idempotency?: IdempotencyClaim): Promise; - // 作成の冪等の鍵の行が作った識別子。期限（ttl）を過ぎた行は、残っていても無いものとして扱う - getIdempotent(ref: IdempotencyRef, now: string): Promise; - // 席が無いときだけ足す（I15） - addSeat(seat: SeatRecord, history: HistoryRecord[]): Promise; - // 席が読んだときのまま（種類と足した時刻が expected と等しい）のときだけ変える - updateSeat(expected: SeatRecord, seatType: SeatRecord["seatType"], history: HistoryRecord[]): Promise; - // 席が読んだときのまま（種類と足した時刻が expected と等しい）のときだけ外す - removeSeat(expected: SeatRecord, history: HistoryRecord[]): Promise; - // version が expectedVersion と等しいときだけ変え、version を 1 増やす - updateAccount( - accountId: string, - expectedVersion: number, - patch: AccountPatch, - updatedAt: string, - history: HistoryRecord[], - ): Promise; -} - -const userKey = (userId: string) => ({ pk: `USER#${userId}`, sk: "USER" }); -const accountKey = (accountId: string) => ({ pk: `ACCOUNT#${accountId}`, sk: "ACCOUNT" }); -const seatKey = (accountId: string, userId: string) => ({ pk: `ACCOUNT#${accountId}`, sk: `SEAT#${userId}` }); +const userKey = (userId: string) => ({ pk: userPk(userId), sk: "USER" }); +const accountKey = (accountId: string) => ({ pk: accountPk(accountId), sk: "ACCOUNT" }); +const seatKey = (accountId: string, userId: string) => ({ pk: accountPk(accountId), sk: `SEAT#${userId}` }); type Item = Record; @@ -88,8 +51,8 @@ function accountItem(a: AccountRecord): Item { function seatItem(s: SeatRecord): Item { return { ...seatKey(s.accountId, s.userId), - gsi1pk: `USER#${s.userId}`, - gsi1sk: `ACCOUNT#${s.accountId}`, + gsi1pk: userPk(s.userId), + gsi1sk: accountPk(s.accountId), account_id: s.accountId, user_id: s.userId, seat_type: s.seatType, @@ -99,7 +62,7 @@ function seatItem(s: SeatRecord): Item { function historyItem(h: HistoryRecord): Item { return { - pk: `ACCOUNT#${h.accountId}`, + pk: accountPk(h.accountId), sk: `HIST#${h.at}#${randomUUID()}`, account_id: h.accountId, type: h.type, @@ -148,6 +111,14 @@ const SEAT_UNCHANGED = "attribute_exists(#pk) AND #seat_type = :expected_type AN const SEAT_NAMES = { "#pk": "pk", "#seat_type": "seat_type", "#added_at": "added_at" }; const seatExpected = (s: SeatRecord) => ({ ":expected_type": s.seatType, ":expected_added_at": s.addedAt }); +// AccountPatch の項目ごとの列の名前とプレースホルダ。SET には表の並びで足す +const PATCH_COLUMNS: { field: keyof AccountPatch; column: string; placeholder: string }[] = [ + { field: "plan", column: "plan", placeholder: ":plan" }, + { field: "executionMode", column: "execution_mode", placeholder: ":mode" }, +]; + +type TransactItem = NonNullable[number]; + export class DynamoAccountStore implements AccountStore { private readonly doc: DynamoDBDocumentClient; @@ -163,13 +134,15 @@ export class DynamoAccountStore implements AccountStore { return (out.Item as Item | undefined) ?? null; } - private put(item: Item, condition?: string): NonNullable[number] { + private put(item: Item, condition?: string): TransactItem { return { Put: { TableName: this.tableName, Item: item, ConditionExpression: condition } }; } - private async transact(items: NonNullable): Promise { + // どの書き込みも履歴の行を同じトランザクションで足す（I20）。並びは items → 履歴 → after + private async transact(items: TransactItem[], history: HistoryRecord[], after: TransactItem[] = []): Promise { + const all = [...items, ...history.map((h) => this.put(historyItem(h))), ...after]; try { - await this.doc.send(new TransactWriteCommand({ TransactItems: items })); + await this.doc.send(new TransactWriteCommand({ TransactItems: all })); } catch (err) { if ( err instanceof TransactionCanceledException && @@ -187,11 +160,7 @@ export class DynamoAccountStore implements AccountStore { } async createUserWithPersonalAccount(user: UserRecord, account: AccountRecord, history: HistoryRecord[]) { - await this.transact([ - this.put(userItem(user), NOT_EXISTS), - this.put(accountItem(account), NOT_EXISTS), - ...history.map((h) => this.put(historyItem(h))), - ]); + await this.transact([this.put(userItem(user), NOT_EXISTS), this.put(accountItem(account), NOT_EXISTS)], history); } async getAccount(accountId: string) { @@ -208,7 +177,7 @@ export class DynamoAccountStore implements AccountStore { const items = await this.queryAll({ TableName: this.tableName, KeyConditionExpression: "pk = :pk AND begins_with(sk, :seat)", - ExpressionAttributeValues: { ":pk": `ACCOUNT#${accountId}`, ":seat": "SEAT#" }, + ExpressionAttributeValues: { ":pk": accountPk(accountId), ":seat": "SEAT#" }, ConsistentRead: true, }); return items.map(toSeat); @@ -219,10 +188,10 @@ export class DynamoAccountStore implements AccountStore { TableName: this.tableName, IndexName: "gsi1", KeyConditionExpression: "gsi1pk = :u", - ExpressionAttributeValues: { ":u": `USER#${userId}` }, + ExpressionAttributeValues: { ":u": userPk(userId) }, }); // 射影（KEYS_ONLY など）によらず必ず載る表の鍵 pk（ACCOUNT#）から ID を取る - return items.map((i) => (i.pk as string).slice("ACCOUNT#".length)); + return items.map((i) => (i.pk as string).slice(ACCOUNT_PREFIX.length)); } private async queryAll(input: ConstructorParameters[0]): Promise { @@ -237,11 +206,10 @@ export class DynamoAccountStore implements AccountStore { } async createTeam(account: AccountRecord, ownerSeat: SeatRecord, history: HistoryRecord[], idempotency?: IdempotencyClaim) { - await this.transact([ - this.put(accountItem(account), NOT_EXISTS), - this.put(seatItem(ownerSeat), NOT_EXISTS), - ...history.map((h) => this.put(historyItem(h))), - ...(idempotency + await this.transact( + [this.put(accountItem(account), NOT_EXISTS), this.put(seatItem(ownerSeat), NOT_EXISTS)], + history, + idempotency ? [ { Put: { @@ -253,8 +221,8 @@ export class DynamoAccountStore implements AccountStore { }, }, ] - : []), - ]); + : [], + ); } async getIdempotent(ref: IdempotencyRef, now: string) { @@ -263,7 +231,7 @@ export class DynamoAccountStore implements AccountStore { } async addSeat(seat: SeatRecord, history: HistoryRecord[]) { - await this.transact([this.put(seatItem(seat), NOT_EXISTS), ...history.map((h) => this.put(historyItem(h)))]); + await this.transact([this.put(seatItem(seat), NOT_EXISTS)], history); } async updateSeat(expected: SeatRecord, seatType: SeatRecord["seatType"], history: HistoryRecord[]) { @@ -278,8 +246,7 @@ export class DynamoAccountStore implements AccountStore { ExpressionAttributeValues: { ":t": seatType, ...seatExpected(expected) }, }, }, - ...history.map((h) => this.put(historyItem(h))), - ]); + ], history); } async removeSeat(expected: SeatRecord, history: HistoryRecord[]) { @@ -293,43 +260,34 @@ export class DynamoAccountStore implements AccountStore { ExpressionAttributeValues: seatExpected(expected), }, }, - ...history.map((h) => this.put(historyItem(h))), - ]); + ], history); } - async updateAccount( - accountId: string, - expectedVersion: number, - patch: AccountPatch, - updatedAt: string, - history: HistoryRecord[], - ) { + async updateAccount({ accountId, expectedVersion, patch, updatedAt, history }: AccountUpdate) { // 予約語と重ならないよう、列の名前は常に #名前 で書く const names: Record = { "#pk": "pk", "#version": "version", "#updated_at": "updated_at" }; const sets = ["#version = :next", "#updated_at = :at"]; const values: Record = { ":expected": expectedVersion, ":next": expectedVersion + 1, ":at": updatedAt }; - if (patch.plan !== undefined) { - names["#plan"] = "plan"; - sets.push("#plan = :plan"); - values[":plan"] = patch.plan; + for (const { field, column, placeholder } of PATCH_COLUMNS) { + if (patch[field] === undefined) continue; + names[`#${column}`] = column; + sets.push(`#${column} = ${placeholder}`); + values[placeholder] = patch[field]; } - if (patch.executionMode !== undefined) { - names["#execution_mode"] = "execution_mode"; - sets.push("#execution_mode = :mode"); - values[":mode"] = patch.executionMode; - } - await this.transact([ - { - Update: { - TableName: this.tableName, - Key: accountKey(accountId), - UpdateExpression: `SET ${sets.join(", ")}`, - ConditionExpression: "attribute_exists(#pk) AND #version = :expected", - ExpressionAttributeNames: names, - ExpressionAttributeValues: values, + await this.transact( + [ + { + Update: { + TableName: this.tableName, + Key: accountKey(accountId), + UpdateExpression: `SET ${sets.join(", ")}`, + ConditionExpression: "attribute_exists(#pk) AND #version = :expected", + ExpressionAttributeNames: names, + ExpressionAttributeValues: values, + }, }, - }, - ...history.map((h) => this.put(historyItem(h))), - ]); + ], + history, + ); } } From 068c56df2abb757d7bcf4ed245737823789c512f Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 5 Nov 2026 02:09:12 +0000 Subject: [PATCH 3/8] =?UTF-8?q?Refactor:=20=E5=B8=AD=E3=81=AE=E7=A8=AE?= =?UTF-8?q?=E9=A1=9E=E3=81=AE=E6=A4=9C=E8=A8=BC=E3=83=BB=E8=A9=B3=E7=B4=B0?= =?UTF-8?q?=E3=81=AE=E6=9C=AC=E6=96=87=E3=83=BBGitHub=20=E3=81=AE=E5=88=A9?= =?UTF-8?q?=E7=94=A8=E8=80=85=E3=81=AE=E5=8F=96=E5=BE=97=E3=83=BBIdP=20?= =?UTF-8?q?=E3=81=AE=E5=88=A4=E5=AE=9A=E3=82=92=201=20=E3=81=8B=E6=89=80?= =?UTF-8?q?=E3=81=AB=E3=81=99=E3=82=8B=EF=BC=88#2199=20=E3=81=AE=20R01?= =?UTF-8?q?=E3=83=BBR13=E3=83=BBR03=E3=83=BBR12=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - validateSeatType を accounts/service.ts に置き、teams.ts と accounts.ts の 3 つの経路が使う（文言は同じ） - 「チームなら席を読んで詳細の本文を作る」を views.ts の accountDetailBody にし、3 つの経路が使う - github-oidc/app.ts の /token と /userinfo が githubUser を使う（判定の条件は同じ） - auth/verify.ts の IdP の判定を idpFromUsername に出す（判定の順序は同じ） Co-Authored-By: Claude Opus 5.5 --- factory/api/src/accounts/service.ts | 8 ++++++++ factory/api/src/auth/verify.ts | 9 +++++++-- factory/api/src/github-oidc/app.ts | 14 ++++++++++---- factory/api/src/routes/accounts.ts | 22 +++++++++------------- factory/api/src/routes/teams.ts | 13 +++++-------- factory/api/src/routes/views.ts | 7 +++++++ 6 files changed, 46 insertions(+), 27 deletions(-) diff --git a/factory/api/src/accounts/service.ts b/factory/api/src/accounts/service.ts index 497188c04..a414ce8ec 100644 --- a/factory/api/src/accounts/service.ts +++ b/factory/api/src/accounts/service.ts @@ -11,6 +11,7 @@ import { import { INITIAL_EXECUTION_MODE, isPlan, + isSeatType, planFitsKind, type AccountContext, type AccountKind, @@ -353,6 +354,13 @@ function seatNotFound(): NotFoundError { const TEAM_NAME_MAX_LENGTH = 64; const TEAM_NAME_MESSAGE = `1〜${TEAM_NAME_MAX_LENGTH} 字で入力してください`; +const SEAT_TYPE_MESSAGE = "席の種類は standard か premium です"; + +export function validateSeatType(value: unknown): SeatType { + if (!isSeatType(value)) throw new InvalidRequestError(SEAT_TYPE_MESSAGE); + return value; +} + export function validateTeamName(name: unknown): string { if (typeof name !== "string") throw new InvalidRequestError(TEAM_NAME_MESSAGE); const length = [...name].length; diff --git a/factory/api/src/auth/verify.ts b/factory/api/src/auth/verify.ts index 47ab11888..0822bcad5 100644 --- a/factory/api/src/auth/verify.ts +++ b/factory/api/src/auth/verify.ts @@ -14,6 +14,12 @@ const IDP_PREFIXES: ReadonlyArray<[string, Idp]> = [ ["GitHub_", "github"], ]; +// token の username の頭から IdP を決める。文字列でない・知らない頭なら undefined +function idpFromUsername(username: unknown): Idp | undefined { + const name = typeof username === "string" ? username : ""; + return IDP_PREFIXES.find(([prefix]) => name.startsWith(prefix))?.[1]; +} + export type AccessTokenVerifier = ReturnType; export function createAccessTokenVerifier(config: VerifierConfig) { @@ -36,8 +42,7 @@ export async function verifyAuthorization(verifier: AccessTokenVerifier, authori if (err instanceof JwtBaseError) throw new AuthError(); throw err; } - const username = typeof payload.username === "string" ? payload.username : ""; - const idp = IDP_PREFIXES.find(([prefix]) => username.startsWith(prefix))?.[1]; + const idp = idpFromUsername(payload.username); if (!idp || typeof payload.sub !== "string" || payload.sub === "") throw new AuthError(); return { userId: payload.sub, idp }; } diff --git a/factory/api/src/github-oidc/app.ts b/factory/api/src/github-oidc/app.ts index 64f5681ea..20e8f2ab9 100644 --- a/factory/api/src/github-oidc/app.ts +++ b/factory/api/src/github-oidc/app.ts @@ -120,6 +120,12 @@ export function createGithubOidcApp(config: GithubOidcConfig, deps: GithubOidcDe return (await res.json()) as T; } + // GitHub の利用者。取れない・数の ID を持たなければ null + async function githubUser(accessToken: string): Promise { + const user = await githubJson("/user", accessToken); + return user && typeof user.id === "number" ? user : null; + } + async function exchangeGithubCode( clientId: string, clientSecret: string, @@ -212,8 +218,8 @@ export function createGithubOidcApp(config: GithubOidcConfig, deps: GithubOidcDe } const token = await exchangeGithubCode(clientId, clientSecret, code.gh_code); if (!token.access_token) return oauthError(c, "invalid_grant"); - const user = await githubJson("/user", token.access_token); - if (!user || typeof user.id !== "number") return oauthError(c, "invalid_grant"); + const user = await githubUser(token.access_token); + if (!user) return oauthError(c, "invalid_grant"); // sub は GitHub の数の利用者 ID。利用者が変えられるログイン名とメールを使わない const idToken = await issueIdToken(String(user.id), clientId, code.nonce); return c.json({ @@ -227,8 +233,8 @@ export function createGithubOidcApp(config: GithubOidcConfig, deps: GithubOidcDe app.on(["GET", "POST"], "/userinfo", async (c) => { const bearer = /^Bearer (.+)$/.exec(c.req.header("Authorization") ?? ""); if (!bearer) return oauthError(c, "invalid_token", 401); - const user = await githubJson("/user", bearer[1]!); - if (!user || typeof user.id !== "number") return oauthError(c, "invalid_token", 401); + const user = await githubUser(bearer[1]!); + if (!user) return oauthError(c, "invalid_token", 401); const emails = (await githubJson("/user/emails", bearer[1]!)) ?? []; const primary = emails.find((e) => e.primary && e.verified); return c.json({ diff --git a/factory/api/src/routes/accounts.ts b/factory/api/src/routes/accounts.ts index 30b594bb5..909785128 100644 --- a/factory/api/src/routes/accounts.ts +++ b/factory/api/src/routes/accounts.ts @@ -1,11 +1,10 @@ import type { Context, Hono } from "hono"; import { ForbiddenError, InvalidRequestError } from "../errors.ts"; -import { isExecutionMode, isSeatType, type AccountContext } from "../accounts/model.ts"; +import { isExecutionMode, type AccountContext } from "../accounts/model.ts"; +import { validateSeatType } from "../accounts/service.ts"; import { resolveFromRequest, type AuthEnv } from "../auth/resolve.ts"; import { registerOperation } from "./operation.ts"; -import { accountDetail, seatView } from "./views.ts"; - -const SEAT_TYPE_MESSAGE = "席の種類は standard か premium です"; +import { accountDetailBody, seatView } from "./views.ts"; function userIdOf(raw: unknown): string { if (typeof raw !== "string" || raw === "" || raw.length > 128) throw new InvalidRequestError("利用者の ID の形ではありません"); @@ -25,8 +24,7 @@ export function registerAccounts(app: Hono): void { const accounts = c.get("auth").accounts; const account = await accounts.getAccount(ctx.accountId); if (!account) throw new ForbiddenError(); - const seats = ctx.kind === "team" ? await accounts.listSeats(ctx) : null; - return accountDetail(ctx, account, seats); + return accountDetailBody(accounts, ctx, account); }, }); @@ -38,25 +36,23 @@ export function registerAccounts(app: Hono): void { } const accounts = c.get("auth").accounts; const account = await accounts.setExecutionMode(ctx, e.execution_mode); - const next = { ...ctx, executionMode: account.executionMode, plan: account.plan }; - const seats = next.kind === "team" ? await accounts.listSeats(next) : null; - return accountDetail(next, account, seats); + return accountDetailBody(accounts, { ...ctx, executionMode: account.executionMode, plan: account.plan }, account); }, }); registerOperation(app, "addSeat", { run: async (e, ctx, _s, c) => { const userId = userIdOf(e.user_id); - if (!isSeatType(e.seat_type)) throw new InvalidRequestError(SEAT_TYPE_MESSAGE); - return seatView(await c.get("auth").accounts.addSeat(ctx!, userId, e.seat_type)); + const seatType = validateSeatType(e.seat_type); + return seatView(await c.get("auth").accounts.addSeat(ctx!, userId, seatType)); }, }); registerOperation(app, "changeSeat", { run: async (e, ctx, _s, c) => { const userId = userIdOf(e.user_id); - if (!isSeatType(e.seat_type)) throw new InvalidRequestError(SEAT_TYPE_MESSAGE); - return seatView(await c.get("auth").accounts.changeSeat(ctx!, userId, e.seat_type)); + const seatType = validateSeatType(e.seat_type); + return seatView(await c.get("auth").accounts.changeSeat(ctx!, userId, seatType)); }, }); diff --git a/factory/api/src/routes/teams.ts b/factory/api/src/routes/teams.ts index 77c0d275c..0094e2a9d 100644 --- a/factory/api/src/routes/teams.ts +++ b/factory/api/src/routes/teams.ts @@ -1,24 +1,21 @@ import type { Hono } from "hono"; -import { InvalidRequestError } from "../errors.ts"; -import { isSeatType } from "../accounts/model.ts"; -import { validateTeamName } from "../accounts/service.ts"; +import { validateSeatType, validateTeamName } from "../accounts/service.ts"; import type { AuthEnv } from "../auth/resolve.ts"; import { parseIdempotencyKey } from "../idempotency.ts"; import { registerOperation } from "./operation.ts"; -import { accountDetail } from "./views.ts"; +import { accountDetailBody } from "./views.ts"; export function registerTeams(app: Hono): void { // 同じ冪等の鍵の 2 回目は 1 回目のチームを返す（#2384 の I14） registerOperation(app, "createTeam", { run: async (e, _ctx, _s, c) => { const name = validateTeamName(e.name); - if (!isSeatType(e.seat_type)) throw new InvalidRequestError("席の種類は standard か premium です"); + const seatType = validateSeatType(e.seat_type); const key = parseIdempotencyKey(e.idempotency_key); const accounts = c.get("auth").accounts; - const ctx = await accounts.createTeam(c.get("caller"), name, e.seat_type, key); + const ctx = await accounts.createTeam(c.get("caller"), name, seatType, key); const account = await accounts.getAccount(ctx.accountId); - const seats = await accounts.listSeats(ctx); - return accountDetail(ctx, { name: account?.name ?? name }, seats); + return accountDetailBody(accounts, ctx, { name: account?.name ?? name }); }, }); } diff --git a/factory/api/src/routes/views.ts b/factory/api/src/routes/views.ts index e5c1364ab..7fada73a3 100644 --- a/factory/api/src/routes/views.ts +++ b/factory/api/src/routes/views.ts @@ -1,5 +1,6 @@ import { getLimits } from "../accounts/limits.ts"; import type { AccountContext, AccountRecord, SeatRecord, UserRecord } from "../accounts/model.ts"; +import type { AccountService } from "../accounts/service.ts"; // 応答の本文の形（factory/contracts/openapi/accounts.yaml） export function accountSummary(ctx: AccountContext, name: string | null) { @@ -30,3 +31,9 @@ export function accountDetail( seats: seats && seats.map(seatView), }; } + +// アカウントの詳細の本文。チームなら席を読んで載せ、個人なら seats を null にする +export async function accountDetailBody(accounts: AccountService, ctx: AccountContext, account: Pick) { + const seats = ctx.kind === "team" ? await accounts.listSeats(ctx) : null; + return accountDetail(ctx, account, seats); +} From ccce758bc5805c6c02267f7ab0292318d55ac154 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 5 Nov 2026 02:11:23 +0000 Subject: [PATCH 4/8] =?UTF-8?q?Refactor:=20=E7=94=BB=E9=9D=A2=E3=81=AE?= =?UTF-8?q?=E3=83=AD=E3=82=B0=E3=82=A4=E3=83=B3=E3=81=AE=E7=A2=BA=E8=AA=8D?= =?UTF-8?q?=E3=83=BB=E5=A4=B1=E6=95=97=E3=81=AE=E8=A1=A8=E7=A4=BA=E3=83=BB?= =?UTF-8?q?=E5=B8=AD=E3=81=AE=E7=A8=AE=E9=A1=9E=E3=81=AE=E9=81=B8=E6=8A=9E?= =?UTF-8?q?=E8=82=A2=E3=82=92=201=20=E3=81=8B=E6=89=80=E3=81=AB=E3=81=99?= =?UTF-8?q?=E3=82=8B=EF=BC=88#2199=20=E3=81=AE=20R05=E3=83=BBR14=E3=83=BBR?= =?UTF-8?q?15=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - RequireLogin（context.tsx）で App.tsx のログインの後の枠を包み、12 画面の hasToken と /login への転送を消す - usePolled の enabled の引数を消す（呼ぶ画面はすべて token の有無を渡していただけ） - 「呼んで、失敗なら画面の文を出す」を useAttempt（context.tsx）にし、判定表の対象の try/catch を置き換える - 席の種類の