Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
📝 Walkthrough## Walkthrough
The changes unify the way namespaces are referenced across rate limit API endpoints by replacing separate `namespaceId` and `namespaceName` fields with a single `namespace` field in request bodies, schemas, and database queries. Associated logic, tests, and SQL queries are updated to use this unified field, and redundant helper functions are removed.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|
| go/apps/api/openapi/gen.go,<br>go/apps/api/openapi/openapi-generated.yaml,<br>go/apps/api/openapi/spec/paths/v2/ratelimit/deleteOverride/V2RatelimitDeleteOverrideRequestBody.yaml,<br>go/apps/api/openapi/spec/paths/v2/ratelimit/getOverride/V2RatelimitGetOverrideRequestBody.yaml,<br>go/apps/api/openapi/spec/paths/v2/ratelimit/limit/V2RatelimitLimitRequestBody.yaml,<br>go/apps/api/openapi/spec/paths/v2/ratelimit/listOverrides/V2RatelimitListOverridesRequestBody.yaml,<br>go/apps/api/openapi/spec/paths/v2/ratelimit/setOverride/V2RatelimitSetOverrideRequestBody.yaml | OpenAPI schemas and generated Go structs updated to replace `namespaceId`/`namespaceName` with `namespace` string; descriptions and constraints updated accordingly. |
| go/apps/api/routes/v2_ratelimit_delete_override/handler.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/handler.go,<br>go/apps/api/routes/v2_ratelimit_set_override/handler.go | Refactored handler logic to remove helper functions for namespace lookup; now use a unified `namespace` field directly in DB queries and transactions. |
| go/apps/api/routes/v2_ratelimit_get_override/handler.go,<br>go/apps/api/routes/v2_ratelimit_limit/handler.go | Refactored namespace retrieval to use a single `namespace` field; updated cache and DB query logic accordingly. |
| go/pkg/db/queries/ratelimit_namespace_find.sql,<br>go/pkg/db/querier_generated.go,<br>go/pkg/db/ratelimit_namespace_find.sql_generated.go | SQL query and generated code updated: now filter by a single `namespace` parameter matching either ID or name; parameter structs updated. |
| go/apps/api/routes/v2_ratelimit_delete_override/200_test.go,<br>go/apps/api/routes/v2_ratelimit_delete_override/400_test.go,<br>go/apps/api/routes/v2_ratelimit_delete_override/401_test.go,<br>go/apps/api/routes/v2_ratelimit_delete_override/403_test.go,<br>go/apps/api/routes/v2_ratelimit_delete_override/404_test.go,<br>go/apps/api/routes/v2_ratelimit_get_override/200_test.go,<br>go/apps/api/routes/v2_ratelimit_get_override/400_test.go,<br>go/apps/api/routes/v2_ratelimit_get_override/401_test.go,<br>go/apps/api/routes/v2_ratelimit_get_override/403_test.go,<br>go/apps/api/routes/v2_ratelimit_get_override/404_test.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/200_test.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/400_test.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/401_test.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/403_test.go,<br>go/apps/api/routes/v2_ratelimit_list_overrides/404_test.go,<br>go/apps/api/routes/v2_ratelimit_set_override/200_test.go,<br>go/apps/api/routes/v2_ratelimit_set_override/400_test.go,<br>go/apps/api/routes/v2_ratelimit_set_override/401_test.go,<br>go/apps/api/routes/v2_ratelimit_set_override/403_test.go,<br>go/apps/api/routes/v2_ratelimit_set_override/404_test.go | Test code updated to use unified `namespace` field in all request structs; pointer usage and field distinctions removed; error message assertions updated as needed. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Client
participant API_Handler
participant DB
participant Cache
Client->>API_Handler: Send request with {namespace, identifier, ...}
API_Handler->>Cache: Lookup namespace by {workspace_id, namespace}
alt Cache Hit
Cache-->>API_Handler: Return namespace data
else Cache Miss
API_Handler->>DB: Query namespace by {workspace_id, namespace}
DB-->>API_Handler: Return namespace row or not found
API_Handler->>Cache: Store namespace data
end
API_Handler->>DB: Perform action (get/set/delete/list override) using namespace
DB-->>API_Handler: Return result
API_Handler-->>Client: Return responseEstimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Learnt from: chronark Learnt from: ogzhanolguncu |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
I love merging openapi |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
the openapi change introduced merge conflicts. Anyways instead of making this a breaking change, what if we just add the namespace field for now and keep the other 2 fields and mark them as deprecated and remove them later? |
|
yeah that’s what I had in mind, I just put this PR on ice until yours landed, so the conflicts were in this one, not yours do you have time to add the deprecated fields perhaps? |
|
yeah I am currently integrating the single permission field and once im done with that ill fix this pr up |
…at_update_openapi_and_accept_a_single_namespace_field_everywhere
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
go/apps/api/openapi/openapi-generated.yaml (1)
1942-1965: Inconsistent validation metadata across request bodiesFor
V2RatelimitListOverridesRequestBodythenamespaceproperty has length constraints but nopattern, whereas other schemas do include a pattern (see 1872-1876).
To keep client generators and documentation consistent, use a single source-of-truth schema (via$ref) fornamespaceinstead of duplicating inline definitions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (35)
go/apps/api/openapi/gen.go(5 hunks)go/apps/api/openapi/openapi-generated.yaml(10 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/deleteOverride/V2RatelimitDeleteOverrideRequestBody.yaml(2 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/getOverride/V2RatelimitGetOverrideRequestBody.yaml(2 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/V2RatelimitLimitRequestBody.yaml(1 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/listOverrides/V2RatelimitListOverridesRequestBody.yaml(2 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/setOverride/V2RatelimitSetOverrideRequestBody.yaml(2 hunks)go/apps/api/routes/v2_ratelimit_delete_override/200_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_delete_override/400_test.go(6 hunks)go/apps/api/routes/v2_ratelimit_delete_override/401_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_delete_override/403_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_delete_override/404_test.go(4 hunks)go/apps/api/routes/v2_ratelimit_delete_override/handler.go(1 hunks)go/apps/api/routes/v2_ratelimit_get_override/200_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_get_override/400_test.go(6 hunks)go/apps/api/routes/v2_ratelimit_get_override/401_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_get_override/403_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_get_override/404_test.go(5 hunks)go/apps/api/routes/v2_ratelimit_get_override/handler.go(2 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(2 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/200_test.go(3 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/400_test.go(4 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/401_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/403_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/404_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_list_overrides/handler.go(4 hunks)go/apps/api/routes/v2_ratelimit_set_override/200_test.go(5 hunks)go/apps/api/routes/v2_ratelimit_set_override/400_test.go(8 hunks)go/apps/api/routes/v2_ratelimit_set_override/401_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_set_override/403_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_set_override/404_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_set_override/handler.go(1 hunks)go/pkg/db/querier_generated.go(1 hunks)go/pkg/db/queries/ratelimit_namespace_find.sql(1 hunks)go/pkg/db/ratelimit_namespace_find.sql_generated.go(2 hunks)
🧰 Additional context used
🧠 Learnings (36)
📓 Common learnings
Learnt from: chronark
PR: unkeyed/unkey#2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., `apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts`), URL parameters like `namespaceId` and `identifier` do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
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.
go/apps/api/openapi/spec/paths/v2/ratelimit/limit/V2RatelimitLimitRequestBody.yaml (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: chronark
PR: #2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The cloudflareRatelimiter type definition in apps/api/src/pkg/env.ts should not have its interface changed; it should keep the limit method returning Promise<{ success: boolean }> without additional error properties.
go/apps/api/routes/v2_ratelimit_set_override/404_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_set_override/401_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_list_overrides/403_test.go (3)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_delete_override/403_test.go (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_get_override/404_test.go (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
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.
go/apps/api/openapi/spec/paths/v2/ratelimit/listOverrides/V2RatelimitListOverridesRequestBody.yaml (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_set_override/200_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_get_override/400_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_delete_override/401_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_list_overrides/401_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_list_overrides/200_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_get_override/401_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_get_override/200_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_list_overrides/400_test.go (4)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
Learnt from: MichaelUnkey
PR: #2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the v1/keys.updateKey endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.
Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:353-466
Timestamp: 2025-07-15T14:25:05.608Z
Learning: In the Unkey codebase, input validation for API endpoints is handled at the OpenAPI schema layer, which validates request fields like permission slugs (pattern: "^[a-zA-Z0-9_]+$", length: 1-100 characters) before requests reach the handler code. This validation occurs during the zen.BindBody call in handlers.
go/apps/api/routes/v2_ratelimit_delete_override/400_test.go (3)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
Learnt from: MichaelUnkey
PR: #2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the v1/keys.updateKey endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.
go/pkg/db/queries/ratelimit_namespace_find.sql (5)
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
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.
Learnt from: chronark
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.
Learnt from: chronark
PR: #3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
go/apps/api/routes/v2_ratelimit_delete_override/404_test.go (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_set_override/403_test.go (3)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_list_overrides/404_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/openapi/spec/paths/v2/ratelimit/setOverride/V2RatelimitSetOverrideRequestBody.yaml (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/pkg/db/ratelimit_namespace_find.sql_generated.go (4)
Learnt from: chronark
PR: #3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: ogzhanolguncu
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.
Learnt from: Flo4604
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.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_limit/handler.go (6)
Learnt from: chronark
PR: #3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: chronark
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.
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: Flo4604
PR: #3606
File: go/pkg/db/replica.go:8-11
Timestamp: 2025-07-16T15:38:53.491Z
Learning: For debugging database replica usage in go/pkg/db/replica.go, it's acceptable to mark QueryRowContext operations as "success" even though SQL errors only surface during row.Scan() calls. The timing metrics are the primary concern for debugging replica performance patterns.
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
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.
go/apps/api/openapi/spec/paths/v2/ratelimit/getOverride/V2RatelimitGetOverrideRequestBody.yaml (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: Flo4604
PR: #3151
File: go/apps/api/openapi/gen.go:221-233
Timestamp: 2025-04-18T20:01:33.812Z
Learning: For destructive operations like deletion in the Unkey API, oneOf validation is preferred over anyOf to enforce explicit targeting and prevent ambiguity.
go/apps/api/openapi/spec/paths/v2/ratelimit/deleteOverride/V2RatelimitDeleteOverrideRequestBody.yaml (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_get_override/403_test.go (2)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_get_override/handler.go (7)
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
Learnt from: Flo4604
PR: #3606
File: go/pkg/db/replica.go:8-11
Timestamp: 2025-07-16T15:38:53.491Z
Learning: For debugging database replica usage in go/pkg/db/replica.go, it's acceptable to mark QueryRowContext operations as "success" even though SQL errors only surface during row.Scan() calls. The timing metrics are the primary concern for debugging replica performance patterns.
Learnt from: chronark
PR: #3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: chronark
PR: #3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
Learnt from: Flo4604
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.
Learnt from: Flo4604
PR: #3606
File: go/pkg/prometheus/metrics/database.go:29-30
Timestamp: 2025-07-16T10:06:35.397Z
Learning: In Go packages, variables defined in one file within a package (like latencyBuckets and constLabels in go/pkg/prometheus/metrics/http.go) are accessible from other files in the same package without requiring imports. This is a common pattern for sharing configuration across multiple files within a package.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_set_override/400_test.go (4)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
Learnt from: MichaelUnkey
PR: #2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the v1/keys.updateKey endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.
Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:353-466
Timestamp: 2025-07-15T14:25:05.608Z
Learning: In the Unkey codebase, input validation for API endpoints is handled at the OpenAPI schema layer, which validates request fields like permission slugs (pattern: "^[a-zA-Z0-9_]+$", length: 1-100 characters) before requests reach the handler code. This validation occurs during the zen.BindBody call in handlers.
go/apps/api/routes/v2_ratelimit_set_override/handler.go (3)
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
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.
go/apps/api/routes/v2_ratelimit_list_overrides/handler.go (3)
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
go/apps/api/routes/v2_ratelimit_delete_override/200_test.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (3)
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
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.
Learnt from: ogzhanolguncu
PR: #2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts, when determining the maximum number of rate limit overrides (max), the intentional use of const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5; allows max to fall back to 5 when hasWorkspaceAccess returns 0 or false. This fallback behavior is expected and intended in the codebase.
go/pkg/db/querier_generated.go (2)
Learnt from: chronark
PR: #3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: ogzhanolguncu
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.
go/apps/api/openapi/gen.go (1)
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
go/apps/api/openapi/openapi-generated.yaml (3)
Learnt from: Flo4604
PR: #3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as description) alongside $ref in schema objects. Do not flag this as an error in future reviews.
Learnt from: chronark
PR: #2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts), URL parameters like namespaceId and identifier do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Learnt from: chronark
PR: #2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The cloudflareRatelimiter type definition in apps/api/src/pkg/env.ts should not have its interface changed; it should keep the limit method returning Promise<{ success: boolean }> without additional error properties.
🧬 Code Graph Analysis (2)
go/apps/api/routes/v2_ratelimit_get_override/200_test.go (2)
go/pkg/testutil/http.go (1)
CallRoute(257-291)go/apps/api/routes/v2_ratelimit_get_override/handler.go (2)
Request(22-22)Response(23-23)
go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
go/pkg/hydra/store/db.go (1)
DBTX(8-13)
⏰ 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)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
🔇 Additional comments (52)
go/apps/api/routes/v2_ratelimit_list_overrides/403_test.go (1)
66-66: LGTM: Clean simplification of namespace field usage.The change from pointer-based
NamespaceIdto directNamespacestring field correctly implements the unified namespace identification pattern. The test logic for workspace permissions remains intact.go/apps/api/openapi/spec/paths/v2/ratelimit/limit/V2RatelimitLimitRequestBody.yaml (1)
8-8: Good documentation simplification.The concise description clearly conveys that the namespace field accepts either an ID or name, which aligns perfectly with the unified namespace approach in this PR.
go/apps/api/routes/v2_ratelimit_set_override/401_test.go (1)
33-36: LGTM: Simplified request structure.The direct assignment to the
Namespacefield removes unnecessary pointer complexity while maintaining the same test behavior for unauthorized access scenarios.go/apps/api/routes/v2_ratelimit_set_override/404_test.go (2)
37-40: LGTM: Consistent namespace field usage for ID not found test.The unified
Namespacefield correctly handles the ID not found scenario with appropriate test data.
52-55: LGTM: Consistent namespace field usage for name not found test.The unified
Namespacefield correctly handles the name not found scenario, maintaining consistent test structure across both test cases.go/apps/api/routes/v2_ratelimit_set_override/403_test.go (1)
55-58: LGTM: Proper workspace permission testing with unified namespace field.The simplified request structure correctly implements the unified namespace approach while maintaining the important cross-workspace access control test.
go/apps/api/routes/v2_ratelimit_delete_override/403_test.go (2)
27-27: Good cleanup - removing unused variable.The direct assignment of
uid.New("test")eliminates the unnecessary intermediate variable while maintaining the same functionality.
68-69: LGTM! Unified namespace field usage.The request structure now correctly uses the single
Namespacefield instead of separateNamespaceIdandNamespaceNamefields, aligning with the API unification goals.go/apps/api/routes/v2_ratelimit_list_overrides/401_test.go (1)
30-30: Consistent with unified namespace approach.The direct string assignment to
Namespacefield properly reflects the API schema simplification.go/pkg/db/querier_generated.go (1)
384-384: Excellent query simplification.The updated SQL query
(ns.id = ? OR ns.name = ?)is much more readable than the previous CASE-based conditional logic while supporting the unified namespace parameter that can match either ID or name.go/apps/api/routes/v2_ratelimit_list_overrides/404_test.go (1)
51-51: Good test coverage maintenance.Both test cases now use the unified
Namespacefield while still covering the scenarios of non-existent namespace ID and name, ensuring comprehensive error handling validation.Also applies to: 64-64
go/apps/api/routes/v2_ratelimit_get_override/404_test.go (2)
27-27: Clean variable elimination.Direct assignment removes unnecessary intermediate variable while maintaining functionality.
50-51: Comprehensive test coverage with unified fields.All test scenarios now consistently use the unified
NamespaceandIdentifierfields, maintaining thorough coverage of various 404 error conditions while using the simplified API structure.Also applies to: 64-65, 77-78, 102-103
go/apps/api/routes/v2_ratelimit_get_override/401_test.go (1)
32-33: LGTM: Clean unification of namespace fieldsThe change successfully replaces the previous separate
NamespaceId/NamespaceNamefields with a singleNamespacefield. This simplifies the request structure while maintaining test coverage for unauthorized access scenarios.go/apps/api/routes/v2_ratelimit_delete_override/401_test.go (1)
33-34: LGTM: Consistent namespace field unificationThe change mirrors the approach used in other test files, correctly implementing the unified
Namespacefield pattern. The test maintains its purpose of verifying unauthorized access while using the simplified request structure.go/apps/api/routes/v2_ratelimit_get_override/400_test.go (5)
52-52: LGTM: Simplified namespace field usageDirect string assignment to the
Namespacefield is cleaner than the previous pointer-based approach.
70-71: LGTM: Consistent field usage across test casesThe unified
Namespacefield approach is applied consistently across different test scenarios.
89-90: LGTM: Clear test case for empty namespace validationUsing an empty string for the
Namespacefield effectively tests the validation logic for missing namespace values.
103-103: Good improvement: More flexible error count assertionChanging from an exact error count to "greater than 0" makes the test more resilient to schema validation changes while still ensuring errors are properly reported.
113-114: LGTM: Consistent unauthorized access test patternThe namespace field changes maintain consistency with other unauthorized access tests while using the simplified request structure.
Also applies to: 129-130
go/apps/api/routes/v2_ratelimit_set_override/200_test.go (3)
52-55: LGTM: Excellent test coverage for namespace name usageThis test demonstrates the unified
Namespacefield accepting a namespace name, which is a key requirement of the refactoring.
78-81: LGTM: Comprehensive coverage for namespace ID usageThis test proves the same
Namespacefield can handle namespace IDs, validating the dual-purpose nature of the unified field.
104-107: LGTM: Thorough test coverage for override scenariosThe tests cover wildcard identifiers and override updates, ensuring the unified namespace field works correctly across all use cases.
Also applies to: 130-133, 142-145
go/apps/api/routes/v2_ratelimit_limit/handler.go (2)
71-72: LGTM: Clean namespace handling implementationThe direct assignment with clear comments explains the dual-purpose nature of the unified field. This eliminates the need for conditional logic to handle separate name and ID fields.
75-80: LGTM: Simplified cache and database operationsUsing the unified
namespaceKeyfor both cache lookups and database queries streamlines the implementation while maintaining the same functionality.go/apps/api/routes/v2_ratelimit_get_override/403_test.go (1)
65-67: LGTM! Unified namespace field correctly implemented.The request struct properly uses the new unified
Namespacefield instead of separateNamespaceIdandNamespaceNamefields, aligning with the PR's objective of consolidating namespace identification.go/apps/api/routes/v2_ratelimit_list_overrides/200_test.go (3)
67-67: Excellent test coverage for unified namespace field.The test properly demonstrates that the unified
Namespacefield accepts namespace names, providing good coverage for this usage pattern.
84-84: Good coverage for namespace ID usage.The test correctly shows that the unified
Namespacefield also accepts namespace IDs, ensuring both usage patterns are validated.
112-112: Consistent usage in edge case testing.The empty namespace test correctly uses the unified field, maintaining consistency across all test scenarios.
go/apps/api/openapi/spec/paths/v2/ratelimit/listOverrides/V2RatelimitListOverridesRequestBody.yaml (2)
3-6: Well-designed schema simplification.The unified
namespacefield with clear documentation about accepting either ID or name is an excellent design choice. The field constraints are appropriate and the description clearly communicates the dual-purpose nature.
28-29: Correct required field specification.The
requiredarray properly specifies the unifiednamespacefield, ensuring API consumers must provide this essential parameter.go/apps/api/routes/v2_ratelimit_delete_override/404_test.go (1)
53-55: Consistent unified field usage across error scenarios.All 404 test cases properly use the unified
Namespacefield, ensuring comprehensive error handling validation with the new API structure.go/apps/api/routes/v2_ratelimit_delete_override/200_test.go (2)
66-68: Good update to use unified namespace field.The existing test correctly migrated to use the unified
Namespacefield with a namespace name, maintaining the original test's intent while conforming to the new API structure.
83-115: Excellent addition of namespace ID test coverage.The new test case provides valuable coverage for deletion using namespace ID through the unified field. The test setup, execution, and verification logic are well-implemented and consistent with the existing test pattern.
go/apps/api/openapi/spec/paths/v2/ratelimit/setOverride/V2RatelimitSetOverrideRequestBody.yaml (1)
12-16: Schema unification looks good, but consider backward compatibility.The consolidation of
namespaceIdandnamespaceNameinto a singlenamespacefield simplifies the API surface effectively. The field constraints and description are appropriate.However, based on the PR discussion, there was consideration for maintaining backward compatibility by keeping the deprecated fields alongside the new unified field to avoid breaking changes. Consider if this breaking change approach aligns with your deprecation strategy.
go/apps/api/routes/v2_ratelimit_get_override/200_test.go (1)
68-70: LGTM! Unified namespace field usage is correct.The test correctly uses the new unified
Namespacefield for both namespace name and ID scenarios, maintaining comprehensive test coverage.go/pkg/db/queries/ratelimit_namespace_find.sql (1)
16-17: SQL unification correctly implemented.The query properly handles the unified namespace approach by matching against both
ns.idandns.namewith a single parameter. The workspace scoping is correctly maintained for security.go/apps/api/routes/v2_ratelimit_delete_override/400_test.go (2)
51-51: Test updates correctly reflect unified namespace field.All test cases properly use the new unified
Namespacestring field instead of the previous separate pointer fields. The error assertions are appropriately standardized to generic schema validation messages.Also applies to: 69-70, 88-89, 112-113, 128-129
102-102: Improved error count assertion.Changing from expecting exactly 0 errors to expecting greater than 0 errors makes more sense for validation failure scenarios, as schema validation typically produces one or more specific error details.
go/apps/api/routes/v2_ratelimit_list_overrides/400_test.go (2)
49-49: Consistent test updates for unified namespace field.The test cases correctly use the new unified
Namespacestring field and maintain proper coverage for bad request scenarios. The standardized error message assertions align with the schema validation approach.Also applies to: 72-72, 87-87
62-62: Appropriate error count expectation.The change to expecting greater than 0 errors is correct for schema validation failures, which typically provide detailed error information.
go/apps/api/routes/v2_ratelimit_set_override/handler.go (1)
59-95: LGTM! Consolidated namespace handling improves API consistency.The refactoring successfully unifies namespace identification by:
- Using a single
Namespaceparameter instead of separate ID/name fields- Moving namespace lookup and authorization within the transaction for better atomicity
- Maintaining proper error handling for not found and deleted namespace cases
The transaction now includes authorization checks, which ensures consistency but may slightly impact performance if authorization fails after the database query. This trade-off is acceptable for the improved atomicity and simplified API surface.
go/apps/api/openapi/spec/paths/v2/ratelimit/deleteOverride/V2RatelimitDeleteOverrideRequestBody.yaml (1)
14-34: Excellent schema simplification!The consolidation from separate
namespaceIdandnamespaceNameproperties to a singlenamespacefield successfully:
- Reduces API complexity by eliminating the oneOf constraint
- Maintains clear documentation about accepting either ID or name
- Preserves appropriate validation constraints
This change aligns perfectly with the PR objective of unifying namespace identification across the API.
go/apps/api/routes/v2_ratelimit_set_override/400_test.go (1)
50-185: Comprehensive test updates align perfectly with schema changes.The test modifications successfully:
- Replace all instances of separate namespace fields with the unified
Namespacefield- Standardize error assertions to expect generic schema validation messages
- Remove unnecessary pointer handling (no more
ptrimport needed)- Maintain comprehensive coverage of all validation scenarios
The test logic remains robust while properly reflecting the simplified API schema.
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (1)
57-94: Consistent refactoring pattern maintains API coherence.The handler refactoring follows the same excellent pattern as the set override handler:
- Consolidates namespace lookup within the transaction using the unified
Namespaceparameter- Moves authorization checks inside the transaction for better atomicity
- Maintains proper error handling with appropriate fault codes
- Preserves audit logging and cache invalidation logic
This consistency across handlers ensures maintainable code and a coherent API experience.
go/apps/api/openapi/spec/paths/v2/ratelimit/getOverride/V2RatelimitGetOverrideRequestBody.yaml (1)
12-33: Consistent schema unification across override endpoints.The schema change mirrors the excellent simplification seen in other override endpoints, replacing dual namespace fields with a unified
namespaceproperty. The consistent approach across all rate limit override endpoints ensures API coherence and developer experience.go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
1-79: LGTM! Generated code correctly implements the unified namespace field.The changes properly implement the namespace unification by:
- Simplifying the SQL query to use a single
Namespaceparameter with OR condition- Updating the parameter struct to use a single non-nullable
Namespacefield- Correctly passing the namespace parameter twice for both ID and name matching
Since this is generated code from sqlc, the changes will be maintained through the source SQL file.
go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)
94-117: Well-structured error handling with clear distinction between error cases.The implementation correctly handles three distinct scenarios:
- Database errors (including not found)
- Cache null values
- Soft-deleted namespaces
This provides good observability for debugging cache and data issues.
go/apps/api/routes/v2_ratelimit_list_overrides/handler.go (2)
106-128: Clean implementation of response construction.The variable renaming from
responsetoresponseBodyimproves code clarity by distinguishing between the database query result and the HTTP response body.
50-72: Ensure consistent namespace caching and extract mapping helperIn v2_ratelimit_list_overrides/handler.go (lines 50–72), we query the namespace directly and manually map
FindRatelimitNamespaceRowtodb.RatelimitNamespace, whereas v2_ratelimit_get_override/handler.go (around lines 56–64) usesh.RatelimitNamespaceCache.SWR. This inconsistency can lead to uneven performance and duplication.• v2_ratelimit_list_overrides/handler.go (50–72):
- Direct call to
db.Query.FindRatelimitNamespacewithout cache- Inline mapping block:
namespace := db.RatelimitNamespace{ ID: response.ID, WorkspaceID: response.WorkspaceID, Name: response.Name, CreatedAtM: response.CreatedAtM, UpdatedAtM: response.UpdatedAtM, DeletedAtM: response.DeletedAtM, }• v2_ratelimit_get_override/handler.go (56–64):
- Uses
h.RatelimitNamespaceCache.SWRfor lookupsRecommendations:
- Align
list_overridesto use the same cache pattern, e.g.namespace, _, err := h.RatelimitNamespaceCache.SWR(ctx, key, func(ctx context.Context) (db.FindRatelimitNamespace, error) { return db.Query.FindRatelimitNamespace(ctx, h.DB.RO(), params) })- Extract the
FindRatelimitNamespaceRow → db.RatelimitNamespaceconversion into a shared mapper function to avoid duplication and ease future field changes.go/apps/api/openapi/gen.go (2)
1826-1827: Add explicit validation thatnamespaceis non-empty.
namespaceis now a requiredstring, but nothing prevents callers from sending""," ", or other obviously-invalid values.
Consider enforcing this directly in the OpenAPI schema (e.g.minLength: 1, an appropriatepattern, orformat) so that both generated code and middleware reject empty/blank input early.This avoids subtle bugs in the handler layer and keeps the contract self-documenting.
Would you like a quick patch to the OpenAPI YAML illustrating the
minLengthaddition?Also applies to: 1861-1862, 1900-1901, 1968-1969, 2029-2030
2029-2030: Response type is still exposingnamespaceId– confirm symmetry.All request structs now take
namespace, butRatelimitOverride(the object returned by*.getOverride,*.listOverrides, etc.) still containsNamespaceId string \json:"namespaceId"``.If the intent is full unification, the response object should expose the same field the client supplied (
namespace).
If the API should always return the canonical ID, document that explicitly to avoid confusion and ease client migrations.Please double-check the desired behaviour and adjust either:
- // NamespaceId ... - NamespaceId string `json:"namespaceId"` + // Namespace ... + Namespace string `json:"namespace"`or the docs accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
go/apps/api/openapi/openapi-generated.yaml (2)
1763-1765: Backward-compatibility gap – legacynamespaceId/namespaceNamestill missingThe agreed phased-deprecation strategy hasn’t been applied. Old clients that still send
namespaceId/namespaceNamewill break because onlynamespaceis now accepted.+ namespaceId: + $ref: '#/components/schemas/NamespaceId' + deprecated: true + namespaceName: + $ref: '#/components/schemas/NamespaceName' + deprecated: true namespace: description: The id or name of the namespace containing the override.Also update the surrounding
required:list so that onlynamespaceis mandatory.
1844-1847: Regex still rejects numeric / UUID-style IDsThe pattern
^[a-zA-Z][a-zA-Z0-9_./-]*$requires the first char to be a letter, blocking valid IDs like3e42f...or123.
Relax to allow digits, e.g.^[a-zA-Z0-9][a-zA-Z0-9_./-]*$, or drop the pattern and rely on server-side detection logic.go/apps/api/openapi/spec/paths/chproxy/metrics/index.yaml (1)
36-37: Clarify overload response as per ratelimits endpoint.Same observation:
529+InternalServerErrorResponse.yamlis semantically confusing. Recommend aligning with503or a dedicated overload schema name for clarity and future maintainability.go/apps/api/openapi/spec/paths/chproxy/verifications/index.yaml (1)
36-37: Overload status/schema naming – same concern.As with the other two specs, consider switching to HTTP
503or renaming the error schema to explicitly convey “service overloaded” instead of generic “internal server error”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
go/apps/api/openapi/openapi-generated.yaml(15 hunks)go/apps/api/openapi/spec/paths/chproxy/metrics/index.yaml(1 hunks)go/apps/api/openapi/spec/paths/chproxy/ratelimits/index.yaml(1 hunks)go/apps/api/openapi/spec/paths/chproxy/verifications/index.yaml(1 hunks)
⏰ 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). (6)
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
go/apps/api/openapi/openapi-generated.yaml (1)
48-50: SchemaBadRequestErrorDetailsis present
- The
BadRequestErrorDetailsschema is defined ingo/apps/api/openapi/openapi-generated.yamlat line 2037.- The reference
$ref: "#/components/schemas/BadRequestErrorDetails"is valid and requires no changes.Likely an incorrect or invalid review comment.
go/apps/api/openapi/spec/paths/chproxy/ratelimits/index.yaml (1)
30-31: Case-sensitivity fix looks correct.The
$refpath now matches the capitalized filename (BadRequestErrorResponse.yaml) used elsewhere in the tree.
No further action required.go/apps/api/openapi/spec/paths/chproxy/metrics/index.yaml (1)
30-31: Good consistency fix.The capitalized
BadRequestErrorResponse.yamlreference aligns with the renamed file.
No issues spotted.go/apps/api/openapi/spec/paths/chproxy/verifications/index.yaml (1)
30-31: Capitalization fix approved.Reference now matches actual file casing; tooling that is case-sensitive will resolve correctly.
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (07/28/25)1 gif was posted to this PR based on Andreas Thomas's automation. "Notify author when CI fails" took an action on this PR • (07/28/25)1 teammate was notified to this PR based on Andreas Thomas's automation. |
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
…e_namespace_field_everywhere
Flo4604
deleted the
07-18-feat_update_openapi_and_accept_a_single_namespace_field_everywhere
branch
2 participants
What does this PR do?
Simplifies the rate limit API by unifying namespace identification into a single
namespacefield that accepts either namespace names or IDs. This change:namespaceIdandnamespaceNamefields with a singlenamespacefield in all rate limit override endpointsType of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests