feat: add identity-based credits schema and database changes#4190
feat: add identity-based credits schema and database changes#4190
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a credits system into the database schema and refactors identity lookup to replace Changes
Sequence DiagramsequenceDiagram
participant Handler as API Handler
participant DB as Database
participant Result as Result Processing
rect rgb(200, 240, 255)
Note over Handler,Result: Old Flow (FindIdentityWithRatelimits)
Handler->>DB: FindIdentityWithRatelimits()
DB-->>Handler: []FindIdentityWithRatelimitsRow (slice)
Handler->>Result: Check len(results) == 0
alt Empty Results
Result-->>Handler: NotFound Error
else Results Present
Result->>Result: results[0]
Result-->>Handler: Identity + Ratelimits
end
end
rect rgb(240, 200, 255)
Note over Handler,Result: New Flow (FindIdentity)
Handler->>DB: FindIdentity()
DB-->>Handler: FindIdentityRow (single)
Handler->>Result: Check db.IsNotFound(err)
alt Not Found
Result-->>Handler: NotFound Fault
else Found
Result->>Result: Process row
Result-->>Handler: Identity + Ratelimits + Credits
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (29)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-10-30T15:10:52.743ZApplied to files:
📚 Learning: 2025-07-17T14:24:20.403ZApplied to files:
📚 Learning: 2025-09-11T14:24:40.988ZApplied to files:
📚 Learning: 2025-08-21T15:54:45.198ZApplied to files:
📚 Learning: 2025-09-12T08:01:20.792ZApplied to files:
📚 Learning: 2025-08-27T13:48:54.016ZApplied to files:
📚 Learning: 2024-10-20T07:05:55.471ZApplied to files:
🧬 Code graph analysis (11)go/pkg/db/bulk_credits_upsert.sql_generated.go (1)
go/apps/api/routes/v2_identities_delete_identity/handler.go (3)
go/apps/api/routes/v2_identities_update_identity/handler.go (4)
go/apps/api/routes/v2_identities_get_identity/handler.go (3)
go/pkg/db/credits_upsert.sql_generated.go (1)
go/pkg/db/bulk_credits_insert.sql_generated.go (1)
go/pkg/db/credits_find_by_identity_id.sql_generated.go (2)
go/pkg/db/querier_bulk_generated.go (3)
go/pkg/db/identity_list.sql_generated.go (1)
go/pkg/db/querier_generated.go (12)
go/pkg/db/key_find_live_by_id.sql_generated.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b943287 to
ae7385a
Compare
ae7385a to
3715c57
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
go/pkg/db/queries/credits_update_increment.sql (1)
1-5: Verify parameter syntax consistency.The query mixes
sqlc.arg('credits')for the increment value with?placeholders forupdated_atandid. While sqlc supports both syntaxes, consider using a consistent approach across all parameters for better readability.Additionally, the increment operation doesn't guard against overflow or validate bounds. Depending on your requirements, you might want to add constraints at the database level or validation at the application level.
Optional: Standardize to named parameters
-- name: UpdateCreditIncrement :exec UPDATE `credits` SET remaining = remaining + sqlc.arg('credits'), - updated_at = ? -WHERE id = ?; + updated_at = sqlc.arg('updated_at') +WHERE id = sqlc.arg('id');go/pkg/db/queries/credits_update_set.sql (1)
1-5: Consider consistent parameter syntax.The query mixes parameter styles:
sqlc.arg('remaining')for the remaining field but positional placeholders (?) for updated_at and id. While this may be valid sqlc syntax, using consistent parameter naming (e.g.,sqlc.arg('updated_at')andsqlc.arg('id')) could improve readability and maintainability.-- name: UpdateCreditSet :exec UPDATE `credits` SET remaining = sqlc.arg('remaining'), - updated_at = ? -WHERE id = ?; + updated_at = sqlc.arg('updated_at') +WHERE id = sqlc.arg('id');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
go/apps/api/routes/v2_identities_delete_identity/handler.go(1 hunks)go/apps/api/routes/v2_identities_get_identity/handler.go(1 hunks)go/apps/api/routes/v2_identities_update_identity/handler.go(1 hunks)go/apps/api/routes/v2_keys_verify_key/200_test.go(2 hunks)go/internal/services/keys/verifier.go(1 hunks)go/pkg/db/bulk_credits_insert.sql_generated.go(1 hunks)go/pkg/db/bulk_credits_upsert.sql_generated.go(1 hunks)go/pkg/db/credits_delete.sql_generated.go(1 hunks)go/pkg/db/credits_find_by_identity_id.sql_generated.go(1 hunks)go/pkg/db/credits_find_by_key_id.sql_generated.go(1 hunks)go/pkg/db/credits_find_remaining.sql_generated.go(1 hunks)go/pkg/db/credits_insert.sql_generated.go(1 hunks)go/pkg/db/credits_update_decrement.sql_generated.go(1 hunks)go/pkg/db/credits_update_increment.sql_generated.go(1 hunks)go/pkg/db/credits_update_set.sql_generated.go(1 hunks)go/pkg/db/credits_upsert.sql_generated.go(1 hunks)go/pkg/db/identity_delete_old_by_external_id.sql_generated.go(2 hunks)go/pkg/db/identity_delete_old_with_ratelimits.sql_generated.go(2 hunks)go/pkg/db/identity_find.sql_generated.go(1 hunks)go/pkg/db/identity_find_with_ratelimits.sql_generated.go(0 hunks)go/pkg/db/identity_list.sql_generated.go(4 hunks)go/pkg/db/key_data.go(2 hunks)go/pkg/db/key_find_for_verification.sql_generated.go(3 hunks)go/pkg/db/key_find_live_by_hash.sql_generated.go(4 hunks)go/pkg/db/key_find_live_by_id.sql_generated.go(4 hunks)go/pkg/db/key_find_remaining.sql_generated.go(1 hunks)go/pkg/db/keys_find_without_credits.sql_generated.go(1 hunks)go/pkg/db/models_generated.go(1 hunks)go/pkg/db/querier_bulk_generated.go(1 hunks)go/pkg/db/querier_generated.go(14 hunks)go/pkg/db/queries/credits_delete.sql(1 hunks)go/pkg/db/queries/credits_find_by_identity_id.sql(1 hunks)go/pkg/db/queries/credits_find_by_key_id.sql(1 hunks)go/pkg/db/queries/credits_find_remaining.sql(1 hunks)go/pkg/db/queries/credits_insert.sql(1 hunks)go/pkg/db/queries/credits_update_decrement.sql(1 hunks)go/pkg/db/queries/credits_update_increment.sql(1 hunks)go/pkg/db/queries/credits_update_set.sql(1 hunks)go/pkg/db/queries/credits_upsert.sql(1 hunks)go/pkg/db/queries/identity_delete_old_by_external_id.sql(1 hunks)go/pkg/db/queries/identity_delete_old_with_ratelimits.sql(1 hunks)go/pkg/db/queries/identity_find.sql(1 hunks)go/pkg/db/queries/identity_find_with_ratelimits.sql(0 hunks)go/pkg/db/queries/identity_list.sql(1 hunks)go/pkg/db/queries/key_find_for_verification.sql(1 hunks)go/pkg/db/queries/key_find_live_by_hash.sql(1 hunks)go/pkg/db/queries/key_find_live_by_id.sql(1 hunks)go/pkg/db/queries/key_find_remaining.sql(1 hunks)go/pkg/db/queries/keys_find_without_credits.sql(1 hunks)go/pkg/db/schema.sql(2 hunks)go/pkg/uid/uid.go(1 hunks)internal/db/src/schema/credits.ts(1 hunks)internal/db/src/schema/index.ts(1 hunks)internal/db/src/schema/keys.ts(4 hunks)internal/db/src/types.ts(1 hunks)internal/id/src/generate.ts(1 hunks)internal/metrics/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- go/pkg/db/identity_find_with_ratelimits.sql_generated.go
- go/pkg/db/queries/identity_find_with_ratelimits.sql
🧰 Additional context used
🧠 Learnings (16)
📚 Learning: 2025-08-27T13:48:54.016Z
Learnt from: Flo4604
PR: unkeyed/unkey#3841
File: go/pkg/db/queries/key_find_for_verification.sql:15-15
Timestamp: 2025-08-27T13:48:54.016Z
Learning: The pending_migration_id field in FindKeyForVerification is only used internally by get_migrated.go for migration logic and doesn't need to be exposed in KeyVerifier struct or API response DTOs since it's an internal implementation detail.
Applied to files:
go/internal/services/keys/verifier.gogo/pkg/db/key_find_for_verification.sql_generated.go
📚 Learning: 2025-08-21T15:54:45.198Z
Learnt from: chronark
PR: unkeyed/unkey#3825
File: go/internal/services/usagelimiter/limit.go:38-0
Timestamp: 2025-08-21T15:54:45.198Z
Learning: In go/internal/services/usagelimiter/limit.go, the UpdateKeyCreditsDecrement operation cannot be safely wrapped with db.WithRetry due to the lack of idempotency mechanisms in the current tech stack. Retrying this non-idempotent write operation risks double-charging users if the first attempt commits but the client sees a transient error.
Applied to files:
go/internal/services/keys/verifier.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/queries/credits_update_decrement.sqlgo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/queries/credits_upsert.sqlgo/pkg/db/credits_update_increment.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/bulk_credits_upsert.sql_generated.gogo/pkg/db/credits_find_by_key_id.sql_generated.gogo/pkg/db/queries/credits_update_increment.sqlgo/pkg/db/credits_update_set.sql_generated.gogo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/querier_generated.go
📚 Learning: 2025-07-17T14:24:20.403Z
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Applied to files:
go/pkg/db/querier_bulk_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/bulk_credits_insert.sql_generated.gogo/pkg/db/key_find_remaining.sql_generated.gogo/pkg/db/queries/credits_insert.sqlgo/pkg/db/credits_insert.sql_generated.gogo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/queries/credits_upsert.sqlgo/pkg/db/credits_update_increment.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/queries/credits_find_by_key_id.sqlgo/pkg/db/bulk_credits_upsert.sql_generated.gogo/pkg/db/queries/keys_find_without_credits.sqlgo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/key_find_for_verification.sql_generated.gogo/pkg/db/queries/key_find_remaining.sql
📚 Learning: 2025-09-12T08:01:20.792Z
Learnt from: Flo4604
PR: unkeyed/unkey#3944
File: go/pkg/db/acme_challenge_update_verified_with_expiry.sql_generated.go:31-39
Timestamp: 2025-09-12T08:01:20.792Z
Learning: Do not review or suggest changes to files with sql_generated.go suffix or other files marked as auto-generated (containing "Code generated by" comments), as these are generated by tools like sqlc and changes would be overwritten on regeneration.
Applied to files:
go/pkg/db/credits_find_remaining.sql_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/bulk_credits_insert.sql_generated.gogo/pkg/db/key_find_remaining.sql_generated.gogo/pkg/db/credits_delete.sql_generated.gogo/pkg/db/queries/credits_insert.sqlgo/pkg/db/credits_insert.sql_generated.gogo/pkg/db/queries/credits_delete.sqlgo/pkg/db/identity_find.sql_generated.gogo/pkg/db/credits_update_increment.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/credits_find_by_identity_id.sql_generated.gogo/pkg/db/queries/credits_update_set.sqlgo/pkg/db/credits_update_set.sql_generated.go
📚 Learning: 2024-10-20T07:05:55.471Z
Learnt from: chronark
PR: unkeyed/unkey#2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.
Applied to files:
go/pkg/db/queries/key_find_live_by_id.sqlgo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/queries/key_find_live_by_hash.sqlgo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/querier_generated.gointernal/db/src/schema/keys.ts
📚 Learning: 2025-08-18T08:41:37.689Z
Learnt from: Flo4604
PR: unkeyed/unkey#3804
File: go/pkg/db/key_update_credits_decrement.sql_generated.go:13-20
Timestamp: 2025-08-18T08:41:37.689Z
Learning: In MySQL queries for decrementing key credits, using GREATEST(remaining_requests - ?, 0) is slower than using a CASE expression with duplicate placeholders (CASE WHEN remaining_requests - ? > 0 THEN remaining_requests - ? ELSE 0 END), even though GREATEST reduces parameter count.
Applied to files:
go/pkg/db/queries/credits_update_decrement.sqlgo/pkg/db/queries/credits_find_remaining.sqlgo/pkg/db/queries/keys_find_without_credits.sql
📚 Learning: 2025-09-11T14:24:40.988Z
Learnt from: Flo4604
PR: unkeyed/unkey#3944
File: go/apps/ctrl/services/deployment/deploy_workflow.go:326-334
Timestamp: 2025-09-11T14:24:40.988Z
Learning: The InsertDomains method in the bulk queries uses ON DUPLICATE KEY UPDATE, making it an upsert operation that is idempotent and safe for retries, despite the "Insert" naming convention.
Applied to files:
go/pkg/db/bulk_credits_upsert.sql_generated.go
📚 Learning: 2025-08-14T16:25:48.167Z
Learnt from: Flo4604
PR: unkeyed/unkey#3785
File: go/apps/api/routes/v2_keys_reroll_key/401_test.go:52-61
Timestamp: 2025-08-14T16:25:48.167Z
Learning: User Flo4604 requested creation of a GitHub issue to track converting all test files to use table-driven test patterns as a broader codebase improvement, following the suggestion made during review of go/apps/api/routes/v2_keys_reroll_key/401_test.go.
Applied to files:
go/apps/api/routes/v2_keys_verify_key/200_test.go
📚 Learning: 2025-09-01T08:29:10.199Z
Learnt from: Flo4604
PR: unkeyed/unkey#3895
File: go/pkg/db/key_list_live_by_auth_id.sql_generated.go:98-105
Timestamp: 2025-09-01T08:29:10.199Z
Learning: In Unkey's ListLiveKeysByKeyAuthID query, adding ka.workspace_id = k.workspace_id constraint negatively impacts index performance. Workspace validation is handled upstream at the API level before the query is called, making the additional constraint unnecessary.
Applied to files:
go/pkg/db/queries/key_find_for_verification.sqlgo/pkg/db/schema.sql
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
go/pkg/db/queries/key_find_for_verification.sqlinternal/db/src/schema/keys.ts
📚 Learning: 2024-11-29T15:15:47.308Z
Learnt from: chronark
PR: unkeyed/unkey#2693
File: apps/api/src/routes/v1_keys_updateKey.ts:350-368
Timestamp: 2024-11-29T15:15:47.308Z
Learning: In `apps/api/src/routes/v1_keys_updateKey.ts`, the code intentionally handles `externalId` and `ownerId` separately for clarity. The `ownerId` field will be removed in the future, simplifying the code.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-08-21T12:37:40.996Z
Learnt from: Flo4604
PR: unkeyed/unkey#3821
File: apps/dashboard/lib/trpc/routers/key/updateRootKeyPermissions.ts:74-74
Timestamp: 2025-08-21T12:37:40.996Z
Learning: Root keys in Unkey have two workspace fields: `workspaceId` (always set to env().UNKEY_WORKSPACE_ID for the Unkey workspace that owns the key) and `forWorkspaceId` (set to ctx.workspace.id for the user's workspace that the key is for). When querying root keys, the system filters by forWorkspaceId to get keys for the current user's workspace, but the returned rootKey.workspaceId is always the Unkey workspace ID.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: When querying or updating namespaces in the Unkey dashboard, always scope the operations to the current workspace using `eq(table.workspaceId, ctx.workspace.id)` to prevent cross-workspace access.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2024-10-04T20:44:38.489Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:56-118
Timestamp: 2024-10-04T20:44:38.489Z
Learning: When typing the `workspace` parameter in functions like `createWorkspaceNavigation`, prefer importing the `Workspace` type from the database module and picking the necessary keys (e.g., `features`) instead of redefining the interface.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-08-25T17:40:51.836Z
Learnt from: mcstepp
PR: unkeyed/unkey#3830
File: go/pkg/db/project_find_by_workspace_slug.sql_generated.go:0-0
Timestamp: 2025-08-25T17:40:51.836Z
Learning: In database schema analysis: UNIQUE constraints automatically create composite indexes in most SQL databases (PostgreSQL, MySQL, etc.), so when analyzing query performance, check for both explicit CREATE INDEX statements and UNIQUE constraints that cover the query's filter columns.
Applied to files:
go/pkg/db/schema.sql
📚 Learning: 2025-04-18T20:01:33.812Z
Learnt from: Flo4604
PR: unkeyed/unkey#3151
File: go/apps/api/openapi/gen.go:221-233
Timestamp: 2025-04-18T20:01:33.812Z
Learning: For identity deletion operations in the Unkey API, identityId takes precedence over externalId when both are provided in the request body.
Applied to files:
go/pkg/db/queries/identity_delete_old_by_external_id.sqlgo/pkg/db/identity_delete_old_by_external_id.sql_generated.go
🧬 Code graph analysis (19)
go/internal/services/keys/verifier.go (1)
go/pkg/db/models_generated.go (1)
Credit(569-580)
go/pkg/db/querier_bulk_generated.go (3)
internal/db/src/types.ts (1)
InsertCredits(59-59)go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(42-56)
go/pkg/db/key_data.go (1)
go/pkg/db/models_generated.go (1)
Credit(569-580)
go/pkg/db/credits_update_decrement.sql_generated.go (1)
internal/db/src/types.ts (1)
Credits(58-58)
go/pkg/db/bulk_credits_insert.sql_generated.go (1)
go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)
internal/db/src/schema/credits.ts (3)
internal/db/src/schema/workspaces.ts (1)
workspaces(16-104)internal/db/src/schema/keys.ts (1)
keys(22-116)internal/db/src/schema/identity.ts (1)
identities(16-38)
go/apps/api/routes/v2_identities_delete_identity/handler.go (4)
go/pkg/db/identity_find.sql_generated.go (1)
FindIdentityParams(52-56)go/pkg/db/models_generated.go (1)
Identity(642-651)go/apps/api/openapi/gen.go (1)
Identity(161-173)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)
go/apps/api/routes/v2_identities_update_identity/handler.go (4)
go/pkg/db/identity_find.sql_generated.go (2)
FindIdentityParams(52-56)FindIdentityRow(58-73)go/apps/api/openapi/gen.go (1)
Identity(161-173)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/db/custom_types.go (1)
RatelimitInfo(23-31)
internal/db/src/types.ts (1)
internal/db/src/index.ts (1)
schema(8-8)
go/pkg/db/key_find_live_by_hash.sql_generated.go (1)
go/pkg/db/models_generated.go (5)
Environment(631-640)Api(514-525)KeyAuth(679-690)Workspace(828-846)EncryptedKey(622-629)
go/pkg/db/identity_find.sql_generated.go (3)
go/pkg/db/models_generated.go (2)
Identity(642-651)Environment(631-640)internal/db/src/types.ts (1)
Identity(46-46)go/apps/api/openapi/gen.go (2)
Identity(161-173)Meta(279-282)
go/pkg/db/models_generated.go (1)
go/pkg/db/types/null_string.go (1)
NullString(10-10)
go/apps/api/routes/v2_identities_get_identity/handler.go (5)
go/pkg/db/identity_find.sql_generated.go (1)
FindIdentityParams(52-56)go/pkg/db/models_generated.go (1)
Identity(642-651)go/apps/api/openapi/gen.go (1)
Identity(161-173)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/codes/unkey_data.go (1)
Data(95-139)
go/pkg/db/bulk_credits_upsert.sql_generated.go (1)
go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(42-56)
go/pkg/db/credits_find_by_key_id.sql_generated.go (2)
go/pkg/db/types/null_string.go (1)
NullString(10-10)go/pkg/db/models_generated.go (1)
Credit(569-580)
go/pkg/db/credits_find_by_identity_id.sql_generated.go (2)
go/pkg/db/types/null_string.go (1)
NullString(10-10)go/pkg/db/models_generated.go (1)
Credit(569-580)
go/pkg/db/identity_list.sql_generated.go (1)
go/pkg/db/models_generated.go (1)
Environment(631-640)
go/pkg/db/querier_generated.go (10)
go/pkg/db/identity_delete_old_by_external_id.sql_generated.go (1)
DeleteOldIdentityByExternalIDParams(23-27)go/pkg/db/models_generated.go (2)
Credit(569-580)Identity(642-651)go/pkg/db/identity_find.sql_generated.go (2)
FindIdentityParams(52-56)FindIdentityRow(58-73)go/pkg/db/keys_find_without_credits.sql_generated.go (2)
FindKeysWithoutCreditsParams(36-39)FindKeysWithoutCreditsRow(41-50)go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)go/pkg/db/identity_list.sql_generated.go (2)
ListIdentitiesParams(29-34)ListIdentitiesRow(36-49)go/pkg/db/credits_update_decrement.sql_generated.go (1)
UpdateCreditDecrementParams(23-27)go/pkg/db/credits_update_increment.sql_generated.go (1)
UpdateCreditIncrementParams(20-24)go/pkg/db/credits_update_set.sql_generated.go (1)
UpdateCreditSetParams(20-24)go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(42-56)
internal/db/src/schema/keys.ts (1)
internal/db/src/schema/credits.ts (1)
credits(16-64)
🔇 Additional comments (36)
go/apps/api/routes/v2_keys_verify_key/200_test.go (1)
223-236: LGTM - Formatting alignment.The spacing adjustments to header map literals improve readability without changing functionality.
go/pkg/uid/uid.go (1)
30-30: LGTM - Credit prefix follows established pattern.The new
CreditPrefixconstant is well-named and consistent with existing resource type prefixes in the system.go/pkg/db/queries/credits_find_by_key_id.sql (1)
1-2: LGTM - Standard query pattern.The query follows the established pattern for single-row lookups and correctly filters credits by key_id.
go/pkg/db/queries/credits_delete.sql (1)
1-2: LGTM - Standard delete pattern.The delete query correctly removes a credit by ID and follows the established pattern for sqlc exec operations.
internal/db/src/schema/index.ts (1)
5-5: LGTM - Credits schema export.The new re-export follows the established pattern and properly exposes the credits schema module.
internal/metrics/src/index.ts (1)
88-92: Schema allows both identifiers absent, but keyId is always present in practice.The metric.usagelimit schema now allows both
keyIdandcreditIdto be optional. However,LimitRequestrequireskeyIdas a mandatory field (not optional), so the metric will always includekeyIdwhen emitted.creditIdis never included in any emission point and has no usage in the usagelimit package. While the schema theoretically allows both absent, the code ensureskeyIdis always populated. IfcreditIdis not currently needed, consider removing it from the schema to avoid confusion.go/pkg/db/queries/identity_delete_old_by_external_id.sql (1)
1-9: LGTM! Credits cascade deletion implemented correctly.The addition of credits to the DELETE statement and LEFT JOIN ensures that associated credits are properly cleaned up when deleting old identities. The LEFT JOIN is appropriate since credits may not exist for all identities.
go/pkg/db/queries/credits_find_remaining.sql (1)
1-2: LGTM! Simple and correct query.The query correctly retrieves the remaining credits by ID. The
:oneannotation is appropriate sinceidshould be the primary key.go/pkg/db/queries/key_find_remaining.sql (1)
1-2: LGTM! Simple and correct query.The query correctly retrieves the remaining requests for a key by ID. The
:oneannotation is appropriate for a primary key lookup.go/pkg/db/identity_delete_old_with_ratelimits.sql_generated.go (1)
12-39: LGTM! Generated code correctly reflects the credits cascade deletion.The generated code properly extends the delete operation to include the credits table with a LEFT JOIN, maintaining consistency with the identity deletion pattern. Parameter bindings are correct.
go/pkg/db/identity_delete_old_by_external_id.sql_generated.go (1)
12-42: LGTM! Generated code correctly implements credits cascade deletion.The generated code properly extends the delete operation to include the credits table. The parameter bindings and LEFT JOIN logic are correct, ensuring credits are deleted when present.
internal/db/src/types.ts (1)
57-59: LGTM! Type exports follow established patterns.The Credits and InsertCredits type exports are consistent with the existing type definitions in the file, using the standard drizzle-orm inference utilities. The placement and naming conventions are appropriate.
go/pkg/db/queries/credits_find_by_identity_id.sql (1)
1-2: No issues found—unique constraint on identity_id is properly enforced.The schema verification confirms that the
creditstable has a UNIQUE constraint onidentity_id(line 139 in schema.sql). This constraint enforces the PR's stated requirement that credits exist either on an identity or on a key, never both, making the:oneannotation safe and appropriate.go/pkg/db/queries/identity_delete_old_with_ratelimits.sql (1)
2-5: LGTM! Credits cleanup integrated correctly.The addition of the credits table to the multi-table DELETE statement with a LEFT JOIN ensures that any credits associated with the identity are properly cleaned up during deletion.
internal/id/src/generate.ts (1)
7-7: LGTM! Credit prefix addition is correct.The new "cr" prefix enables credit ID generation consistent with the existing ID system.
go/pkg/db/queries/keys_find_without_credits.sql (1)
1-21: LGTM! Migration query correctly identifies keys without credits.The LEFT JOIN with
c.id IS NULLfilter correctly identifies keys that have legacyremaining_requestsvalues but haven't been migrated to the new credits table. The CASE expression properly handles NULL timestamps, and pagination support is appropriate for large-scale migrations.go/pkg/db/queries/identity_list.sql (1)
2-14: Database schema correctly enforces uniqueness—no action needed.The verification confirms that
go/pkg/db/schema.sqlincludes the constraintCONSTRAINTunique_per_identity_idxUNIQUE(identity_id)at line 139 on thecreditstable. The LEFT JOIN in the query is safe and will not produce duplicate rows for any identity, as the database enforces at most one credit record per identity_id.go/pkg/db/queries/credits_insert.sql (1)
1-13: Add CHECK constraint to enforce key_id and identity_id mutual exclusivity.The credits table schema is missing the required CHECK constraint. Current UNIQUE constraints on
key_idandidentity_idare insufficient—they allow both columns to be NULL or both to be populated simultaneously, violating the PR objective that credits must exist on either an identity or a key, never both.The schema file at
go/pkg/db/schema.sql:126–140must be updated to include:CONSTRAINT credits_check CHECK ((key_id IS NULL) <> (identity_id IS NULL))⛔ Skipped due to learnings
Learnt from: Flo4604 PR: unkeyed/unkey#3841 File: go/pkg/db/schema.sql:122-122 Timestamp: 2025-08-27T13:50:49.819Z Learning: The unkey project does not use foreign key constraints in their database schema. They handle referential integrity at the application level instead of relying on database-level foreign keys.go/pkg/db/credits_delete.sql_generated.go (1)
1-22: Auto-generated file - skipping review.Based on learnings, files with
sql_generated.gosuffix are auto-generated by sqlc and should not be reviewed as changes would be overwritten on regeneration.go/pkg/db/credits_find_remaining.sql_generated.go (1)
1-24: Auto-generated file - skipping review.Based on learnings, files with
sql_generated.gosuffix are auto-generated by sqlc and should not be reviewed as changes would be overwritten on regeneration.go/pkg/db/queries/credits_update_decrement.sql (1)
1-8: LGTM! Correct use of CASE expression for performance.The CASE expression correctly decrements credits and clamps to 0, aligning with the learning that CASE with duplicate placeholders is faster than GREATEST in MySQL for decrement operations.
go/pkg/db/credits_update_set.sql_generated.go (1)
1-35: Auto-generated file - skipping review.Based on learnings, files with
sql_generated.gosuffix are auto-generated by sqlc and should not be reviewed.go/pkg/db/key_find_live_by_hash.sql_generated.go (1)
1-359: Auto-generated file - skipping review.Based on learnings, files with
sql_generated.gosuffix are auto-generated by sqlc and should not be reviewed.go/pkg/db/key_data.go (2)
167-192: LGTM! Credit population logic correctly distinguishes key vs identity credits.The credit population logic correctly constructs separate
Creditinstances with appropriateKeyIDandIdentityIDvalues for key-level and identity-level credits respectively.
15-16: Enforce mutual exclusivity of KeyCredits and IdentityCredits via CHECK constraint or validation.The schema allows both
key_idandidentity_idto be non-NULL simultaneously. The existing UNIQUE constraints on individual columns only prevent duplicate credits per key/identity—they do not enforce the mutual exclusivity requirement. Add aCHECK (key_id IS NOT NULL XOR identity_id IS NOT NULL)constraint at the schema level, or implement validation during credit creation in the application layer to ensure credits exist only on an identity or a key, never both.⛔ Skipped due to learnings
Learnt from: Flo4604 PR: unkeyed/unkey#3841 File: go/pkg/db/schema.sql:122-122 Timestamp: 2025-08-27T13:50:49.819Z Learning: The unkey project does not use foreign key constraints in their database schema. They handle referential integrity at the application level instead of relying on database-level foreign keys.go/apps/api/routes/v2_identities_delete_identity/handler.go (1)
74-91: LGTM! Ratelimits properly included in refactored query.Verification confirms the
FindIdentityquery includes ratelimits as a JSON array via JSON_ARRAYAGG (go/pkg/db/queries/identity_find.sql:10-23), with all expected fields (id, name, key_id, identity_id, limit, duration, auto_apply). The refactoring fromFindIdentityWithRatelimitstoFindIdentitymaintains data completeness with no regression.go/pkg/db/queries/key_find_for_verification.sql (1)
69-92: Now I need to search for credit creation logic and any application-level validation:Based on the initial script output and web search findings, I can now provide a comprehensive rewritten review comment:
The credits table schema lacks database-level enforcement of the mutual exclusivity constraint.
As of MySQL 8.0.16, MySQL supports table and column CHECK constraints for all storage engines. However, the schema at
go/pkg/db/schema.sqllines 126-140 shows thecreditstable withkey_idandidentity_idas nullable columns with only UNIQUE constraints—no CHECK constraint enforcing mutual exclusivity exists.Currently, the schema permits invalid states:
- Both
key_idANDidentity_idpopulated- Both columns NULL
To align with the PR objective that credits exist either on an identity or a key (never both), add a CHECK constraint like:
CHECK ((key_id IS NOT NULL AND identity_id IS NULL) OR (key_id IS NULL AND identity_id IS NOT NULL))Verify that:
- Credit creation handlers validate this constraint at the application level if the database version predates 8.0.16
- Existing credits in the database comply with this mutual exclusivity requirement before deploying
go/pkg/db/schema.sql (1)
418-419: Index strategy looks appropriate.The indexes support the expected query patterns:
workspace_id_idxfor workspace-scoped queriesrefill_lookup_idxfor refill workflow operations- UNIQUE constraints on
key_idandidentity_idautomatically provide index coverage for lookupsgo/pkg/db/models_generated.go (1)
569-580: Auto-generated code follows expected patterns.The Credit struct correctly maps the schema with appropriate use of sql.Null* types for nullable columns.
go/pkg/db/credits_find_by_key_id.sql_generated.go (1)
1-36: Generated query implementation looks correct.Standard sqlc-generated query to find credits by key_id. No issues identified.
go/pkg/db/credits_find_by_identity_id.sql_generated.go (1)
1-36: Generated query implementation looks correct.Standard sqlc-generated query to find credits by identity_id, mirrors the find_by_key_id pattern.
go/pkg/db/credits_update_decrement.sql_generated.go (1)
13-21: Decrement logic correctly prevents negative values.The CASE statement ensures
remainingis clamped to 0 if the decrement would result in a negative value, which is appropriate defensive programming for credit management.Note: This operation is non-idempotent and should not be retried automatically. Based on learnings, the retry concern should be handled at call sites.
go/pkg/db/bulk_credits_insert.sql_generated.go (1)
1-47: Bulk insert follows established patterns.Standard implementation consistent with other bulk insert operations in the codebase. The 10 placeholders per row correctly match the 10 columns being inserted.
internal/db/src/schema/keys.ts (2)
52-90: Deprecation strategy preserves backwards compatibility.The existing credit-related fields (
refillDay,refillAmount,lastRefillAt,remaining) are marked as deprecated while remaining in the schema, allowing for a gradual migration to the newcreditstable. This is a sound approach for avoiding breaking changes.Consider documenting the migration timeline and whether these fields will eventually be removed in a future major version.
140-143: Credits relation correctly establishes one-to-one link.The new relation maps
keys.idtocredits.keyId, which aligns with the UNIQUE constraint oncredits.key_idin the schema, ensuring a proper one-to-one relationship.go/pkg/db/bulk_credits_upsert.sql_generated.go (1)
58-62: Verify intent with the developer or schema design documentation.The search found no call sites to
UpsertCreditin the repository, so the usage pattern cannot be verified through code inspection. This is a generated file, and the design constraint (uniformSpecifiedflags fromargs[0]applied to all rows) may reflect either:
- An intentional design for bulk operations (all rows update consistently)
- A limitation of the code generator (sqlc)
- Future usage not yet in the codebase
Without call sites or explicit documentation, it's unclear whether callers are expected to enforce uniform flags or if this constraint should be documented.
|
Thank you for following the naming conventions for pull request titles! 🙏 |
62afa56 to
7017b5a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
go/apps/api/routes/v2_identities_delete_identity/handler.go(1 hunks)go/apps/api/routes/v2_identities_get_identity/handler.go(1 hunks)go/apps/api/routes/v2_identities_update_identity/handler.go(1 hunks)go/apps/api/routes/v2_keys_verify_key/200_test.go(2 hunks)go/internal/services/keys/verifier.go(1 hunks)go/pkg/db/bulk_credits_insert.sql_generated.go(1 hunks)go/pkg/db/bulk_credits_upsert.sql_generated.go(1 hunks)go/pkg/db/credits_delete.sql_generated.go(1 hunks)go/pkg/db/credits_find_by_identity_id.sql_generated.go(1 hunks)go/pkg/db/credits_find_by_key_id.sql_generated.go(1 hunks)go/pkg/db/credits_find_remaining.sql_generated.go(1 hunks)go/pkg/db/credits_insert.sql_generated.go(1 hunks)go/pkg/db/credits_update_decrement.sql_generated.go(1 hunks)go/pkg/db/credits_update_increment.sql_generated.go(1 hunks)go/pkg/db/credits_update_set.sql_generated.go(1 hunks)go/pkg/db/credits_upsert.sql_generated.go(1 hunks)go/pkg/db/identity_delete_old_by_external_id.sql_generated.go(2 hunks)go/pkg/db/identity_delete_old_with_ratelimits.sql_generated.go(2 hunks)go/pkg/db/identity_find.sql_generated.go(1 hunks)go/pkg/db/identity_find_with_ratelimits.sql_generated.go(0 hunks)go/pkg/db/identity_list.sql_generated.go(4 hunks)go/pkg/db/key_data.go(2 hunks)go/pkg/db/key_find_for_verification.sql_generated.go(3 hunks)go/pkg/db/key_find_live_by_hash.sql_generated.go(4 hunks)go/pkg/db/key_find_live_by_id.sql_generated.go(4 hunks)go/pkg/db/key_find_remaining.sql_generated.go(1 hunks)go/pkg/db/keys_find_without_credits.sql_generated.go(1 hunks)go/pkg/db/models_generated.go(1 hunks)go/pkg/db/querier_bulk_generated.go(1 hunks)go/pkg/db/querier_generated.go(14 hunks)go/pkg/db/queries/credits_delete.sql(1 hunks)go/pkg/db/queries/credits_find_by_identity_id.sql(1 hunks)go/pkg/db/queries/credits_find_by_key_id.sql(1 hunks)go/pkg/db/queries/credits_find_remaining.sql(1 hunks)go/pkg/db/queries/credits_insert.sql(1 hunks)go/pkg/db/queries/credits_update_decrement.sql(1 hunks)go/pkg/db/queries/credits_update_increment.sql(1 hunks)go/pkg/db/queries/credits_update_set.sql(1 hunks)go/pkg/db/queries/credits_upsert.sql(1 hunks)go/pkg/db/queries/identity_delete_old_by_external_id.sql(1 hunks)go/pkg/db/queries/identity_delete_old_with_ratelimits.sql(1 hunks)go/pkg/db/queries/identity_find.sql(1 hunks)go/pkg/db/queries/identity_find_with_ratelimits.sql(0 hunks)go/pkg/db/queries/identity_list.sql(1 hunks)go/pkg/db/queries/key_find_for_verification.sql(1 hunks)go/pkg/db/queries/key_find_live_by_hash.sql(1 hunks)go/pkg/db/queries/key_find_live_by_id.sql(1 hunks)go/pkg/db/queries/key_find_remaining.sql(1 hunks)go/pkg/db/queries/keys_find_without_credits.sql(1 hunks)go/pkg/db/schema.sql(2 hunks)go/pkg/uid/uid.go(1 hunks)internal/db/src/schema/credits.ts(1 hunks)internal/db/src/schema/index.ts(1 hunks)internal/db/src/schema/keys.ts(4 hunks)internal/db/src/types.ts(1 hunks)internal/id/src/generate.ts(1 hunks)internal/metrics/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- go/pkg/db/identity_find_with_ratelimits.sql_generated.go
- go/pkg/db/queries/identity_find_with_ratelimits.sql
🚧 Files skipped from review as they are similar to previous changes (20)
- go/pkg/db/queries/credits_insert.sql
- go/pkg/db/queries/credits_find_remaining.sql
- go/pkg/uid/uid.go
- go/pkg/db/queries/identity_delete_old_with_ratelimits.sql
- go/pkg/db/credits_find_by_key_id.sql_generated.go
- go/pkg/db/queries/credits_find_by_identity_id.sql
- go/pkg/db/queries/key_find_live_by_hash.sql
- go/pkg/db/queries/key_find_remaining.sql
- go/apps/api/routes/v2_keys_verify_key/200_test.go
- go/pkg/db/queries/credits_update_decrement.sql
- go/pkg/db/credits_find_by_identity_id.sql_generated.go
- go/pkg/db/queries/key_find_for_verification.sql
- go/pkg/db/schema.sql
- go/pkg/db/credits_update_increment.sql_generated.go
- go/pkg/db/identity_delete_old_by_external_id.sql_generated.go
- go/pkg/db/queries/credits_upsert.sql
- go/pkg/db/queries/credits_find_by_key_id.sql
- go/pkg/db/queries/credits_delete.sql
- internal/id/src/generate.ts
- go/pkg/db/queries/identity_list.sql
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.710Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
📚 Learning: 2025-10-30T15:10:52.710Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.710Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Applied to files:
go/internal/services/keys/verifier.gointernal/db/src/types.tsgo/pkg/db/models_generated.gogo/pkg/db/credits_delete.sql_generated.gogo/pkg/db/queries/keys_find_without_credits.sqlgo/pkg/db/credits_insert.sql_generated.gointernal/db/src/schema/keys.tsgo/pkg/db/queries/key_find_live_by_id.sqlgo/pkg/db/queries/identity_find.sqlgo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/key_find_for_verification.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/identity_list.sql_generated.gogo/pkg/db/queries/credits_update_set.sqlgo/pkg/db/credits_find_remaining.sql_generated.gogo/pkg/db/key_find_remaining.sql_generated.gogo/pkg/db/key_data.gogo/pkg/db/identity_find.sql_generated.gointernal/db/src/schema/credits.tsgo/pkg/db/identity_delete_old_with_ratelimits.sql_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gointernal/metrics/src/index.tsgo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/bulk_credits_insert.sql_generated.gogo/pkg/db/credits_update_set.sql_generated.gogo/pkg/db/bulk_credits_upsert.sql_generated.gogo/pkg/db/querier_bulk_generated.gogo/pkg/db/queries/identity_delete_old_by_external_id.sqlgo/pkg/db/queries/credits_update_increment.sqlgo/pkg/db/querier_generated.go
📚 Learning: 2025-08-27T13:48:54.016Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3841
File: go/pkg/db/queries/key_find_for_verification.sql:15-15
Timestamp: 2025-08-27T13:48:54.016Z
Learning: The pending_migration_id field in FindKeyForVerification is only used internally by get_migrated.go for migration logic and doesn't need to be exposed in KeyVerifier struct or API response DTOs since it's an internal implementation detail.
Applied to files:
go/internal/services/keys/verifier.gogo/pkg/db/key_find_for_verification.sql_generated.go
📚 Learning: 2025-08-21T15:54:45.198Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 3825
File: go/internal/services/usagelimiter/limit.go:38-0
Timestamp: 2025-08-21T15:54:45.198Z
Learning: In go/internal/services/usagelimiter/limit.go, the UpdateKeyCreditsDecrement operation cannot be safely wrapped with db.WithRetry due to the lack of idempotency mechanisms in the current tech stack. Retrying this non-idempotent write operation risks double-charging users if the first attempt commits but the client sees a transient error.
Applied to files:
go/internal/services/keys/verifier.gogo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/credits_update_set.sql_generated.gogo/pkg/db/bulk_credits_upsert.sql_generated.gogo/pkg/db/queries/credits_update_increment.sqlgo/pkg/db/querier_generated.go
📚 Learning: 2025-04-22T14:43:11.724Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 3161
File: go/pkg/clickhouse/schema/databases/002_ratelimits/006_ratelimits_per_day_v1.sql:1-13
Timestamp: 2025-04-22T14:43:11.724Z
Learning: In the unkey project, the SQL files in clickhouse/schema/databases represent the current production schema and shouldn't be modified directly in PRs. Schema changes require dedicated migration scripts.
Applied to files:
go/internal/services/keys/verifier.go
📚 Learning: 2025-09-12T08:01:20.792Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3944
File: go/pkg/db/acme_challenge_update_verified_with_expiry.sql_generated.go:31-39
Timestamp: 2025-09-12T08:01:20.792Z
Learning: Do not review or suggest changes to files with sql_generated.go suffix or other files marked as auto-generated (containing "Code generated by" comments), as these are generated by tools like sqlc and changes would be overwritten on regeneration.
Applied to files:
go/pkg/db/credits_delete.sql_generated.gogo/pkg/db/credits_insert.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/key_find_remaining.sql_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/bulk_credits_insert.sql_generated.gogo/pkg/db/credits_update_set.sql_generated.gogo/pkg/db/queries/credits_update_increment.sql
📚 Learning: 2025-07-17T14:24:20.403Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Applied to files:
go/pkg/db/queries/keys_find_without_credits.sqlgo/pkg/db/credits_insert.sql_generated.gogo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/key_find_for_verification.sql_generated.gogo/pkg/db/credits_upsert.sql_generated.gogo/pkg/db/key_find_remaining.sql_generated.gogo/pkg/db/credits_update_decrement.sql_generated.gogo/pkg/db/keys_find_without_credits.sql_generated.gogo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/bulk_credits_insert.sql_generated.gogo/pkg/db/bulk_credits_upsert.sql_generated.gogo/pkg/db/querier_bulk_generated.go
📚 Learning: 2025-08-18T08:41:37.689Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3804
File: go/pkg/db/key_update_credits_decrement.sql_generated.go:13-20
Timestamp: 2025-08-18T08:41:37.689Z
Learning: In MySQL queries for decrementing key credits, using GREATEST(remaining_requests - ?, 0) is slower than using a CASE expression with duplicate placeholders (CASE WHEN remaining_requests - ? > 0 THEN remaining_requests - ? ELSE 0 END), even though GREATEST reduces parameter count.
Applied to files:
go/pkg/db/queries/keys_find_without_credits.sql
📚 Learning: 2024-10-20T07:05:55.471Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.
Applied to files:
internal/db/src/schema/keys.tsgo/pkg/db/queries/key_find_live_by_id.sqlgo/pkg/db/key_find_live_by_hash.sql_generated.gogo/pkg/db/key_find_live_by_id.sql_generated.gogo/pkg/db/querier_generated.go
📚 Learning: 2024-11-29T15:15:47.308Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2693
File: apps/api/src/routes/v1_keys_updateKey.ts:350-368
Timestamp: 2024-11-29T15:15:47.308Z
Learning: In `apps/api/src/routes/v1_keys_updateKey.ts`, the code intentionally handles `externalId` and `ownerId` separately for clarity. The `ownerId` field will be removed in the future, simplifying the code.
Applied to files:
internal/db/src/schema/keys.tsinternal/db/src/schema/credits.ts
📚 Learning: 2025-08-21T12:37:40.996Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3821
File: apps/dashboard/lib/trpc/routers/key/updateRootKeyPermissions.ts:74-74
Timestamp: 2025-08-21T12:37:40.996Z
Learning: Root keys in Unkey have two workspace fields: `workspaceId` (always set to env().UNKEY_WORKSPACE_ID for the Unkey workspace that owns the key) and `forWorkspaceId` (set to ctx.workspace.id for the user's workspace that the key is for). When querying root keys, the system filters by forWorkspaceId to get keys for the current user's workspace, but the returned rootKey.workspaceId is always the Unkey workspace ID.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: When querying or updating namespaces in the Unkey dashboard, always scope the operations to the current workspace using `eq(table.workspaceId, ctx.workspace.id)` to prevent cross-workspace access.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2024-10-04T20:44:38.489Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:56-118
Timestamp: 2024-10-04T20:44:38.489Z
Learning: When typing the `workspace` parameter in functions like `createWorkspaceNavigation`, prefer importing the `Workspace` type from the database module and picking the necessary keys (e.g., `features`) instead of redefining the interface.
Applied to files:
internal/db/src/schema/keys.ts
📚 Learning: 2025-08-22T12:49:20.668Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3775
File: apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts:1-1
Timestamp: 2025-08-22T12:49:20.668Z
Learning: The team at unkeyed/unkey is comfortable with keeping TRPC schema imports from app route folders as temporary technical debt during their migration from TRPC to calling their v2 API directly, since this code will be replaced rather than maintained long-term.
Applied to files:
internal/db/src/schema/credits.ts
📚 Learning: 2025-04-18T20:01:33.812Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3151
File: go/apps/api/openapi/gen.go:221-233
Timestamp: 2025-04-18T20:01:33.812Z
Learning: For identity deletion operations in the Unkey API, identityId takes precedence over externalId when both are provided in the request body.
Applied to files:
internal/db/src/schema/credits.tsgo/pkg/db/queries/identity_delete_old_by_external_id.sql
📚 Learning: 2025-08-27T13:50:49.819Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3841
File: go/pkg/db/schema.sql:122-122
Timestamp: 2025-08-27T13:50:49.819Z
Learning: The unkey project does not use foreign key constraints in their database schema. They handle referential integrity at the application level instead of relying on database-level foreign keys.
Applied to files:
internal/db/src/schema/credits.ts
📚 Learning: 2025-09-01T08:27:27.709Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3895
File: go/pkg/db/querier_generated.go:1293-1307
Timestamp: 2025-09-01T08:27:27.709Z
Learning: In the unkey codebase, for JSON_ARRAYAGG queries in ListLiveKeysByKeyAuthID and similar database queries, deterministic ordering is not a priority requirement. The team is fine with non-deterministic ordering for permissions and roles aggregation.
Applied to files:
internal/db/src/schema/credits.ts
📚 Learning: 2025-09-01T08:26:57.944Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3895
File: go/pkg/db/key_list_live_by_auth_id.sql_generated.go:53-67
Timestamp: 2025-09-01T08:26:57.944Z
Learning: For the ListLiveKeysByKeyAuthID query in the unkeyed/unkey repository, the user Flo4604 confirmed that ordering of role permissions in the JSON array output is not required.
Applied to files:
internal/db/src/schema/credits.ts
📚 Learning: 2025-09-11T14:24:40.988Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3944
File: go/apps/ctrl/services/deployment/deploy_workflow.go:326-334
Timestamp: 2025-09-11T14:24:40.988Z
Learning: The InsertDomains method in the bulk queries uses ON DUPLICATE KEY UPDATE, making it an upsert operation that is idempotent and safe for retries, despite the "Insert" naming convention.
Applied to files:
go/pkg/db/bulk_credits_upsert.sql_generated.go
🧬 Code graph analysis (17)
go/internal/services/keys/verifier.go (1)
go/pkg/db/models_generated.go (1)
Credit(569-580)
internal/db/src/types.ts (1)
internal/db/src/index.ts (1)
schema(8-8)
go/apps/api/routes/v2_identities_delete_identity/handler.go (4)
go/pkg/db/identity_find.sql_generated.go (1)
FindIdentityParams(52-56)go/apps/api/openapi/gen.go (1)
Identity(161-173)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/codes/unkey_data.go (1)
Data(95-139)
go/pkg/db/credits_insert.sql_generated.go (1)
go/pkg/db/types/null_string.go (1)
NullString(10-10)
internal/db/src/schema/keys.ts (1)
internal/db/src/schema/credits.ts (1)
credits(16-64)
go/apps/api/routes/v2_identities_update_identity/handler.go (4)
go/pkg/db/identity_find.sql_generated.go (2)
FindIdentityParams(52-56)FindIdentityRow(58-73)go/apps/api/openapi/gen.go (1)
Identity(161-173)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/db/custom_types.go (1)
RatelimitInfo(23-31)
go/apps/api/routes/v2_identities_get_identity/handler.go (3)
go/pkg/db/identity_find.sql_generated.go (1)
FindIdentityParams(52-56)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/codes/unkey_data.go (1)
Data(95-139)
go/pkg/db/key_find_live_by_hash.sql_generated.go (1)
go/pkg/db/models_generated.go (4)
Api(514-525)KeyAuth(679-690)Workspace(828-846)EncryptedKey(622-629)
go/pkg/db/key_find_for_verification.sql_generated.go (1)
go/pkg/db/types/null_string.go (1)
NullString(10-10)
go/pkg/db/identity_list.sql_generated.go (1)
go/pkg/db/models_generated.go (1)
Environment(631-640)
go/pkg/db/key_data.go (1)
go/pkg/db/models_generated.go (1)
Credit(569-580)
go/pkg/db/identity_find.sql_generated.go (1)
go/pkg/db/models_generated.go (2)
Identity(642-651)Environment(631-640)
internal/db/src/schema/credits.ts (3)
internal/db/src/schema/workspaces.ts (1)
workspaces(16-104)internal/db/src/schema/keys.ts (1)
keys(22-116)internal/db/src/schema/identity.ts (1)
identities(16-38)
go/pkg/db/bulk_credits_insert.sql_generated.go (1)
go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)
go/pkg/db/bulk_credits_upsert.sql_generated.go (1)
go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(46-61)
go/pkg/db/querier_bulk_generated.go (3)
internal/db/src/types.ts (1)
InsertCredits(59-59)go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(46-61)
go/pkg/db/querier_generated.go (10)
go/pkg/db/identity_delete_old_by_external_id.sql_generated.go (1)
DeleteOldIdentityByExternalIDParams(23-27)go/pkg/db/models_generated.go (2)
Credit(569-580)Identity(642-651)go/pkg/db/identity_find.sql_generated.go (2)
FindIdentityParams(52-56)FindIdentityRow(58-73)go/pkg/db/keys_find_without_credits.sql_generated.go (2)
FindKeysWithoutCreditsParams(36-39)FindKeysWithoutCreditsRow(41-50)go/pkg/db/credits_insert.sql_generated.go (1)
InsertCreditParams(28-39)go/pkg/db/identity_list.sql_generated.go (2)
ListIdentitiesParams(29-34)ListIdentitiesRow(36-49)go/pkg/db/credits_update_decrement.sql_generated.go (1)
UpdateCreditDecrementParams(23-27)go/pkg/db/credits_update_increment.sql_generated.go (1)
UpdateCreditIncrementParams(20-24)go/pkg/db/credits_update_set.sql_generated.go (1)
UpdateCreditSetParams(20-24)go/pkg/db/credits_upsert.sql_generated.go (1)
UpsertCreditParams(46-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
- GitHub Check: autofix
🔇 Additional comments (17)
go/pkg/db/queries/identity_delete_old_by_external_id.sql (1)
2-5: LGTM! Credits cleanup properly integrated.The expansion of the DELETE operation to include the credits table is correct. The LEFT JOIN ensures identities are deleted even when no credits exist, which is the right behavior for cleaning up soft-deleted identities and their associated data.
go/pkg/db/identity_delete_old_with_ratelimits.sql_generated.go (1)
13-16: LGTM! Generated code correctly reflects SQL changes.The sqlc-generated code properly includes the credits table in the DELETE statement and LEFT JOIN, matching the SQL source file changes. The function signature remains unchanged, maintaining backward compatibility.
internal/metrics/src/index.ts (1)
88-92: LGTM! Metric schema updated to support credit-based usage limits.The changes correctly reflect the new credits functionality by making
keyIdoptional and adding an optionalcreditIdfield. This allows themetric.usagelimitto track usage limits for both key-based and credit-based scenarios.go/pkg/db/queries/credits_update_set.sql (1)
1-5: LGTM! Credit update query is correctly structured.The query provides a clean path to set the
remainingcredits andupdated_attimestamp by ID. The use of parameterized queries andsqlc.argannotation follows best practices.internal/db/src/schema/index.ts (1)
5-5: LGTM! Credits schema properly exported.The new export follows the established pattern and correctly exposes the credits schema alongside other schema modules.
go/internal/services/keys/verifier.go (1)
51-53: LGTM! Credit fields added to support future priority enforcement.The new
IdentityCreditsandKeyCreditsfields correctly establish the data structure for the credits feature. As confirmed in previous review, this PR focuses solely on schema and query changes—the priority enforcement logic for these credits will be implemented in a subsequent PR in the stack.Based on learnings
internal/db/src/types.ts (1)
58-59: LGTM! Credit types follow established patterns.The new
CreditsandInsertCreditstype exports correctly mirror the pattern used for other entities (e.g.,Quotasabove) and properly leverage Drizzle ORM's type inference.go/apps/api/routes/v2_identities_delete_identity/handler.go (1)
74-91: LGTM! Identity query refactored with improved error handling.The migration from
FindIdentityWithRatelimits(multi-row) toFindIdentity(single-row) is well-executed. The explicit not-found handling usingdb.IsNotFound(err)provides clearer error semantics and returns an appropriate fault code before falling back to generic database errors.go/apps/api/routes/v2_identities_update_identity/handler.go (1)
116-134: LGTM: Clean refactor to single-row identity query.The migration from
FindIdentityWithRatelimitstoFindIdentityis well-executed, with proper not-found handling usingdb.IsNotFound(err)and appropriate fault wrapping.go/pkg/db/key_data.go (2)
15-16: LGTM: Credits fields added to KeyData.The new
KeyCreditsandIdentityCreditsfields properly extend KeyData to surface credit information for keys and their associated identities.
166-192: LGTM: Credit population logic is correct.The implementation correctly:
- Populates
KeyCreditswhenCreditID.Validis true, withKeyIDset andIdentityIDleft invalid- Populates
IdentityCreditswhenIdentityCreditID.Validis true, withIdentityIDset andKeyIDleft invalidThis aligns with the schema constraint that credits belong to either a key or an identity, not both.
go/pkg/db/bulk_credits_upsert.sql_generated.go (1)
1-72: Skipping review of generated file.This file is auto-generated by the sqlc bulk insert plugin and should not be manually modified.
go/pkg/db/queries/identity_find.sql (1)
1-37: LGTM: Well-structured identity query with credits and ratelimits.The query efficiently retrieves:
- Identity data from the main table
- Associated credit information via LEFT JOIN
- Ratelimits as a JSON array with proper fallback
The UNION ALL pattern for matching by either
idorexternal_idis appropriate, and the LIMIT 1 ensures a single-row result.go/apps/api/routes/v2_identities_get_identity/handler.go (1)
54-72: LGTM: Consistent migration to single-row identity query.The refactor properly uses
FindIdentitywith explicit not-found handling viadb.IsNotFound(err), matching the pattern in the update handler.go/pkg/db/credits_upsert.sql_generated.go (1)
1-113: Skipping review of generated file.This file is auto-generated by sqlc and should not be manually modified.
internal/db/src/schema/credits.ts (1)
16-64: LGTM: Credits schema is well-structured.The table definition includes:
- Appropriate primary key and indexes
- Unique constraints on both
keyIdandidentityIdto ensure at most one credit per owner- Proper documentation of the exclusivity constraint
- Refill-related indexes for efficient queries
The mutual exclusivity of
keyIdandidentityIdwill be enforced at the application level, as previously discussed.go/pkg/db/credits_find_remaining.sql_generated.go (1)
1-24: Skipping review of generated file.This file is auto-generated by sqlc and should not be manually modified.
| -- name: FindKeysWithoutCredits :many | ||
| SELECT | ||
| k.id, | ||
| k.workspace_id, | ||
| k.remaining_requests, | ||
| k.refill_day, | ||
| k.refill_amount, | ||
| CASE | ||
| WHEN k.last_refill_at IS NULL THEN NULL | ||
| ELSE UNIX_TIMESTAMP(k.last_refill_at) * 1000 | ||
| END as last_refill_at_unix, | ||
| k.created_at_m, | ||
| k.updated_at_m | ||
| FROM `keys` k | ||
| LEFT JOIN `credits` c ON c.key_id = k.id | ||
| WHERE k.deleted_at_m IS NULL | ||
| AND k.remaining_requests IS NOT NULL | ||
| AND c.id IS NULL | ||
| ORDER BY k.created_at_m DESC | ||
| LIMIT ? | ||
| OFFSET ?; No newline at end of file |
There was a problem hiding this comment.
Avoid returning keys whose identity already owns credits
As written, this query flags every key missing a key-scoped credit, even when its identity already has a credit row. Using it to backfill credits would create both key- and identity-level credits for the same identity, violating the “one or the other” rule this PR is introducing. Please exclude keys whose identity already has a credit record before they reach the migration.
[suggested fix]
FROM `keys` k
-LEFT JOIN `credits` c ON c.key_id = k.id
+LEFT JOIN `credits` c ON c.key_id = k.id
+LEFT JOIN `credits` ic ON ic.identity_id = k.identity_id
WHERE k.deleted_at_m IS NULL
AND k.remaining_requests IS NOT NULL
- AND c.id IS NULL
+ AND c.id IS NULL
+ AND (k.identity_id IS NULL OR ic.id IS NULL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- name: FindKeysWithoutCredits :many | |
| SELECT | |
| k.id, | |
| k.workspace_id, | |
| k.remaining_requests, | |
| k.refill_day, | |
| k.refill_amount, | |
| CASE | |
| WHEN k.last_refill_at IS NULL THEN NULL | |
| ELSE UNIX_TIMESTAMP(k.last_refill_at) * 1000 | |
| END as last_refill_at_unix, | |
| k.created_at_m, | |
| k.updated_at_m | |
| FROM `keys` k | |
| LEFT JOIN `credits` c ON c.key_id = k.id | |
| WHERE k.deleted_at_m IS NULL | |
| AND k.remaining_requests IS NOT NULL | |
| AND c.id IS NULL | |
| ORDER BY k.created_at_m DESC | |
| LIMIT ? | |
| OFFSET ?; | |
| -- name: FindKeysWithoutCredits :many | |
| SELECT | |
| k.id, | |
| k.workspace_id, | |
| k.remaining_requests, | |
| k.refill_day, | |
| k.refill_amount, | |
| CASE | |
| WHEN k.last_refill_at IS NULL THEN NULL | |
| ELSE UNIX_TIMESTAMP(k.last_refill_at) * 1000 | |
| END as last_refill_at_unix, | |
| k.created_at_m, | |
| k.updated_at_m | |
| FROM `keys` k | |
| LEFT JOIN `credits` c ON c.key_id = k.id | |
| LEFT JOIN `credits` ic ON ic.identity_id = k.identity_id | |
| WHERE k.deleted_at_m IS NULL | |
| AND k.remaining_requests IS NOT NULL | |
| AND c.id IS NULL | |
| AND (k.identity_id IS NULL OR ic.id IS NULL) | |
| ORDER BY k.created_at_m DESC | |
| LIMIT ? | |
| OFFSET ?; |
🤖 Prompt for AI Agents
In go/pkg/db/queries/keys_find_without_credits.sql around lines 1 to 21, the
query currently selects keys that lack a key-scoped credit but does not exclude
keys whose identity already has a credit; add a condition to exclude any key
whose identity_id has an existing (non-deleted) credit record. Implement this by
adding a NOT EXISTS subquery (or an additional LEFT JOIN + WHERE c2.id IS NULL)
that checks credits for c2.identity_id = k.identity_id (and c2.deleted_at_m IS
NULL) so keys belonging to identities with credits are filtered out before
migration.
| @@ -113,36 +113,34 @@ func (h *Handler) Handle(ctx context.Context, s *zen.Session) error { | |||
| } | |||
|
|
|||
| // Use UNION query to find identity + ratelimits in one query (fast!) | |||
There was a problem hiding this comment.
probably outdated comment
| @@ -0,0 +1,2 @@ | |||
| -- name: FindCreditsByKeyID :one | |||
There was a problem hiding this comment.
shouldn't this have a LIMIT 1?
There was a problem hiding this comment.
same for the one above btw
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
| ON DUPLICATE KEY UPDATE | ||
| remaining = CASE | ||
| WHEN CAST(sqlc.arg('remaining_specified') AS UNSIGNED) = 1 THEN VALUES(remaining) |
There was a problem hiding this comment.
because when we upsert and hit a race condition we only want to overwrite the data that we know has been given to us in the api.
for example you send updateIdentity at the same time, whoever writes last will win, but we shouldn't set data that wasnt sent to us in the first place. e.g only sets refill but not remaining etc
Since SQLC only has a state of NULL or not null value
| `remaining` int NOT NULL, | ||
| `refill_day` tinyint, | ||
| `refill_amount` int, | ||
| `refilled_at` bigint unsigned, |
There was a problem hiding this comment.
why is this one unsigned, yet the other dates are signed?
There was a problem hiding this comment.
yeah didnt think about it too much but we can change that
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/03/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
7017b5a to
84cf2ac
Compare
- Add new credits table supporting both key-based and identity-based credits - Update keys schema to include credits relation - Add credit type to types and ID generation - Add SQL queries for credit operations (find remaining, decrement) - Update identity list query to include credit information
84cf2ac to
8277f35
Compare


What does this PR do?
This adds a new table where our identity/key credits are stored.
You either have credits on an identity or a key but NOT NEVER EVER both.
This als refactors the findIdentity function to return a single row and not 2.
Re-generated the non formatted ts files
DO NOT MERGE THIS
Type of change
How should this be tested?
Check if you like the schema, check if the tests still pass
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated