feat(gateway): configure greptime storage for conversations#313
Draft
turisanapo wants to merge 3 commits intomainfrom
Draft
feat(gateway): configure greptime storage for conversations#313turisanapo wants to merge 3 commits intomainfrom
turisanapo wants to merge 3 commits intomainfrom
Conversation
- Add SqlStorage with GrepTimeDialect using the shared greptimeSqlClient - Create OrgScopedStorage wrapper for multi-tenant isolation via organization_id metadata - Expose full /conversations CRUD routes with Elysia response type annotations - Include conversation items routes (/conversations/:id/items) - Use AsyncLocalStorage to propagate org context to the storage layer Closes #312 Co-authored-by: Davide Selvaggio <turisanapo@users.noreply.github.com>
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces SQL-backed conversation storage for the gateway using GrepTime, initialized during startup with database migrations. It adds a multi-tenant organization-scoped storage wrapper that enforces isolation by injecting and validating organization IDs. New REST endpoints are registered to manage conversations and conversation items with organization-scoped request handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant OrgContext as orgContext<br/>(AsyncLocalStorage)
participant OrgScopedStorage
participant SqlStorage as SqlStorage<br/>(GrepTime)
Note over Gateway: Startup Phase
Gateway->>SqlStorage: new SqlStorage(client)
SqlStorage->>SqlStorage: migrate()
Gateway->>OrgScopedStorage: wrap(sqlStorage)
Note over Client,SqlStorage: Request Handling Phase
Client->>Gateway: POST /conversations<br/>(organizationId, body)
Gateway->>OrgContext: orgContext.run(orgId, ...)
OrgContext->>OrgScopedStorage: createConversation(params)
OrgScopedStorage->>OrgScopedStorage: getOrgId() from context
OrgScopedStorage->>OrgScopedStorage: inject metadata.organization_id
OrgScopedStorage->>SqlStorage: createConversation(...)
SqlStorage->>SqlStorage: persist to GrepTime
SqlStorage-->>OrgScopedStorage: ConversationEntity
OrgScopedStorage-->>Gateway: ConversationEntity
Gateway-->>Client: 201 + response schema
Note over Client,SqlStorage: Read with Ownership Validation
Client->>Gateway: GET /conversations/{id}<br/>(organizationId)
Gateway->>OrgContext: orgContext.run(orgId, ...)
OrgContext->>OrgScopedStorage: getConversation(id)
OrgScopedStorage->>SqlStorage: fetch conversation
SqlStorage-->>OrgScopedStorage: ConversationEntity
OrgScopedStorage->>OrgScopedStorage: verify org_id matches context
alt Organization matches
OrgScopedStorage-->>Gateway: ConversationEntity
Gateway-->>Client: 200 + conversation
else Organization mismatch
OrgScopedStorage-->>Gateway: undefined
Gateway-->>Client: 404
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
@claude, are we exposing the GET conversation with the by-metadata option? |
|
Claude finished @turisanapo's task in 35s —— View job Analyzing the PR for by-metadata query support
Short answer: it depends on the The Looking at the storage layer in this PR:
However, there's no explicit
If the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/gateway/src/services/org-scoped-storage.ts (1)
53-63: Minor: CachegetOrgId()result to avoid duplicate call.
getOrgId()is called twice—once for the ownership check (line 58) and once when building the update payload (line 61). While functionally correct, caching the value avoids the redundantAsyncLocalStorage.getStore()lookup.♻️ Suggested improvement
async updateConversation( id: string, metadata: ConversationMetadata, ): Promise<ConversationEntity | undefined> { + const orgId = this.getOrgId(); const conv = await this.storage.getConversation(id); - if (!conv || conv.metadata?.organization_id !== this.getOrgId()) return undefined; + if (!conv || conv.metadata?.organization_id !== orgId) return undefined; return this.storage.updateConversation(id, { ...metadata, - organization_id: this.getOrgId(), + organization_id: orgId, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/services/org-scoped-storage.ts` around lines 53 - 63, In updateConversation, cache the result of getOrgId() into a local const (e.g., const orgId = this.getOrgId()) and use that cached orgId for both the ownership check (currently comparing conv.metadata?.organization_id) and the update payload passed to this.storage.updateConversation so you avoid calling getOrgId() twice (and the duplicate AsyncLocalStorage.getStore() lookup).apps/gateway/src/index.ts (1)
110-125: Consider type assertion fororganizationId.The
isSignedIn: truemacro guaranteesorganizationIdis defined at runtime, but TypeScript still types it asstring | undefined. While safe (becausegetOrgId()throws if undefined), a non-null assertion would document this contract and catch any future misuse at compile time.♻️ Example for one route (apply to all)
.post( "/conversations", ({ request, prismaClient, organizationId }) => - orgContext.run(organizationId, () => + orgContext.run(organizationId!, () => gw.handler(request, { prismaClient, organizationId }), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/index.ts` around lines 110 - 125, The route callback is passing organizationId typed as string | undefined to orgContext.run and gw.handler; add a non-null assertion to document the isSignedIn contract and satisfy TypeScript (e.g., use organizationId! where passed into orgContext.run and into gw.handler) so the compiler knows organizationId is defined — update all similar routes that rely on getOrgId() throwing when undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/gateway/src/index.ts`:
- Around line 110-125: The route callback is passing organizationId typed as
string | undefined to orgContext.run and gw.handler; add a non-null assertion to
document the isSignedIn contract and satisfy TypeScript (e.g., use
organizationId! where passed into orgContext.run and into gw.handler) so the
compiler knows organizationId is defined — update all similar routes that rely
on getOrgId() throwing when undefined.
In `@apps/gateway/src/services/org-scoped-storage.ts`:
- Around line 53-63: In updateConversation, cache the result of getOrgId() into
a local const (e.g., const orgId = this.getOrgId()) and use that cached orgId
for both the ownership check (currently comparing
conv.metadata?.organization_id) and the update payload passed to
this.storage.updateConversation so you avoid calling getOrgId() twice (and the
duplicate AsyncLocalStorage.getStore() lookup).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80266d89-250c-48a9-8ed0-cbb66d038386
📒 Files selected for processing (3)
apps/gateway/src/gateway-config.tsapps/gateway/src/index.tsapps/gateway/src/services/org-scoped-storage.ts
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configure the gateway's SQL storage for
/conversationsusing GreptimeDB with multi-tenant isolation.SqlStoragewithGrepTimeDialectusing the sharedgreptimeSqlClientOrgScopedStoragewrapper for org-level isolation viaorganization_idmetadata/conversationsCRUD + items routes with Elysia response typesAsyncLocalStorageto propagate org context to the storage layerCloses #312
Generated with Claude Code
Summary by CodeRabbit