feat(database): unify resource ID generation in storage dialects - #663
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: da4e548 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
055e574 to
89db301
Compare
68c6161 to
a5fad3e
Compare
CreateSigningKey left ID empty after domain stopped minting, which broke project create. Mirror encryption-key Ensure, and restore CreateToken's reject-if-set check before dialect minting. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Purpose-scoped JWEs store the KEK id as kid; without a pre-minted KEK id, GetProjectCrypter failed with enc_key.not_found after project create. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Main claimed ADR 046 for claim lifecycle v2. Keep that number and move dialect-owned ID generation to ADR 047, updating cross-references. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Remove coerce*Identity aliases, IsNumeric/int Scan leftovers, and narrative comments; share prefix normalization; mint Spanner challenge IDs only on insert. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Resolve conflicts keeping dialect-owned string IDs and SQLite as the unconfigured local default. Drop embedded-postgres default path. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
…IDs (ULID) Replace all INTEGER AUTOINCREMENT primary/foreign keys in the SQLite dialect with TEXT ULIDs minted by ensureManagedID, matching the Postgres dialect pattern. Changes: - session.go: ensureManagedID for session + user-agent IDs; scan all IDs as strings; database.CoerceString in sessionSchema; remove strconv dependency - auth_attempt.go: ensureManagedID for attempt + challenge IDs; ON CONFLICT sets id = excluded.id; scan IDs as strings - token.go: ensureManagedID for token_id (TokenPrefix); string session FKs; CoerceString in tokenSchema; remove coerceTokenInt64 - user_password.go: ensureManagedID with PrefixUserPassword - user_totp.go: ensureManagedID with PrefixUserTOTP - user_recovery_codes.go: ensureManagedID with PrefixUserRecoveryCodes - user_passkey.go: ensureManagedID with PrefixUserPasskey - crypto_keys.go: ensureManagedID with PrefixEncryptionKey / PrefixSigningKey - project.go: ensureManagedID with PrefixProject - team.go: replace empty-ID error with ensureManagedID; drop errors import - user.go: ensureManagedID with PrefixUser - branding.go: ensureManagedID with PrefixBranding - flow_definition.go: ensureManagedID with PrefixFlowDefinition - passkey_registration.go: ensureManagedID with PrefixPasskeyRegistration - identity.go: deleted (parseIdentity had no remaining callers) Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Rewrite the SQLite init migration so resource PKs/FKs are TEXT (no AUTOINCREMENT), mint JSON schema URLs via ensureManagedID, and document SQLite ULID alongside Postgres in ADR 047 / 028 and the changeset. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Make sessions.token_id nullable across dialects and retire the "0" sentinel. Drop redundant Create-supplied SQL DEFAULTs, remove unused project_secret/preview_secret columns, and add a non-empty recovery codes check on SQLite. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
SeedProjectsTiedAt for Postgres/Spanner omitted preview_origins after the column DEFAULT was removed, so ListCursorTie failed with NOT NULL. Match the SQLite seed and always insert an empty origins value. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
livio-a
left a comment
There was a problem hiding this comment.
LGTM — zero-exception dialect-minted PKs (ADR 047), SQLite/Postgres ULID + Spanner UUID, ceremony pre-mint, create-user reject, and the token_id/DEFAULT cleanup all look coherent. Happy to merge after a rebase onto current main (still a few commits behind). Squash is fine.
Nits below are non-blocking. On ID prefixes: I'm fine deferring renames of existing tokens (att, upk, enc_key, brnd, …). What I don't want is merging without a short "how we pick prefixes" rule and then breaking the wire again a week later when the next u*/consonant prefix lands. So: document + freeze new additions now; rename legacy later in one intentional pass.
Also (no good diff anchor): FlowDefinitionPrefix and PrefixFlowDefinition are both "flowdef" — collapse to one name (dialects currently disagree on which they call).
Optional sanity note: only create-user needs an explicit client-id reject (open map). Other creates have closed bodies — looks correct.
Before merge: rebase onto main. Squash OK.
Add ADR 047 prefix selection rules, statements-only ID minting in AGENTS.md, soften ResourcePrefix docs, drop v2database aliases, and align usr_/htok_ fixtures with user_/handoff_. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Align Create empty-id with project_test now that team suites live in stmttest after the main merge. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
TL;DR
Every resource PK is a dialect-minted
prefix_<opaque>string (Postgres ULID, SQLite ULID, Spanner UUID v4). SQL has noIDENTITY/ uuid / AUTOINCREMENT defaults. Create APIs reject client resource PKs. Documented as ADR 047.Also cleaned review leftovers:
sessions.token_idis nullable (retired"0"sentinel), redundant Create-supplied DEFAULTs dropped across dialects, deadproject_secret/preview_secretremoved.Follow-up review pass: prefix selection rules + keep-any ceremony note in ADR 047; statements-only ID minting in root/storage
AGENTS.md;ResourcePrefixcomment softened;v2databasealiases dropped;usr_/htok_fixtures aligned touser_/handoff_.Synced
main: ListTeams + team statement suites instmttest(#728/#733); dialect Spannerteam_test.goremoved; empty-ID Create asserted in stmttest (dialect minting).How to review (order)
docs/adrs/047-dialect-id-generation.md(+ amended 011/012/028)internal/storage/v2/dialect/idgen/+ dialectmanaged_id.go(postgres/sqlite ULID, spanner UUID)postgres/,spanner/,sqlite/(TEXT PKs; nullablesessions.token_id; no redundant array/JSON/TTL DEFAULTs)token_iduntil token mintsess_/att_/ch_patterns; KEK pre-mint for JWEkidinternal/storage/v2/AGENTS.mdidentifier mintingapi/generated/**, stmttest HasPrefix fixturesBehavior changes
prefix_…; Spanner bodies are UUID v4.sessions.token_idis NULL until the real token is linked (no"0"placeholder).Validation
go test ./internal/service/ ./internal/domain/— PASSmoon run server:test-sqlite— PASS (includes stmttest team Create empty-ID)ci / full-prafter this tipRelease notes / changeset
.changeset/dialect-owned-id-generation.md— minor@zitadel/serverReview notes
Notes
HasPrefix, not ULID-only regex.team_id = ''remains the intentional project-scope uniqueness sentinel (not nullable).