Skip to content

Commit bb7313d

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(knowledge): align merged application boundaries
1 parent f9296a0 commit bb7313d

13 files changed

Lines changed: 369 additions & 295 deletions

File tree

‎apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts‎

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import {
44
updateKnowledgeConnectorContract,
55
} from '@/lib/api/contracts/knowledge'
66
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
7-
import { decryptApiKey } from '@/lib/api-key/crypto'
8-
import { resolveCredentialTokenIdentity } from '@/lib/credentials/access'
97
import {
108
toInternalKnowledgeConnector,
119
toInternalKnowledgeConnectorDetail,
@@ -20,69 +18,6 @@ import {
2018
updateKnowledgeConnector,
2119
} from '@/lib/knowledge/application/connectors'
2220
import { knowledgeOperations } from '@/lib/knowledge/application/operations'
23-
import type {
24-
KnowledgeConnectorRow,
25-
SourceConfigRejection,
26-
} from '@/lib/knowledge/orchestration/connectors'
27-
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
28-
import { CONNECTOR_REGISTRY } from '@/connectors/registry.server'
29-
30-
function makeSourceConfigValidator(actingUserId: string, workspaceId: string, connectorId: string) {
31-
return async (
32-
connector: KnowledgeConnectorRow,
33-
sourceConfig: Record<string, unknown>
34-
): Promise<SourceConfigRejection | null> => {
35-
const connectorConfig = CONNECTOR_REGISTRY[connector.connectorType]
36-
if (!connectorConfig) {
37-
return {
38-
message: `Unknown connector type: ${connector.connectorType}`,
39-
errorCode: 'validation',
40-
}
41-
}
42-
43-
let accessToken: string | null = null
44-
if (connectorConfig.auth.mode === 'apiKey') {
45-
if (!connector.encryptedApiKey) {
46-
return {
47-
message: 'API key not found. Please reconfigure the connector.',
48-
errorCode: 'validation',
49-
}
50-
}
51-
accessToken = (await decryptApiKey(connector.encryptedApiKey)).decrypted
52-
} else {
53-
if (!connector.credentialId) {
54-
return {
55-
message: 'OAuth credential not found. Please reconfigure the connector.',
56-
errorCode: 'validation',
57-
}
58-
}
59-
const identity = await resolveCredentialTokenIdentity(connector.credentialId, workspaceId)
60-
if (!identity) {
61-
return {
62-
message: 'Credential is no longer usable in this workspace. Please reconnect it.',
63-
errorCode: 'validation',
64-
}
65-
}
66-
accessToken = await refreshAccessTokenIfNeeded(
67-
connector.credentialId,
68-
identity.kind === 'oauth' ? identity.userId : actingUserId,
69-
`patch-${connectorId}`
70-
)
71-
}
72-
73-
if (!accessToken) {
74-
return {
75-
message: 'Failed to refresh access token. Please reconnect your account.',
76-
errorCode: 'unauthorized',
77-
}
78-
}
79-
80-
const validation = await connectorConfig.validateConfig(accessToken, sourceConfig)
81-
return validation.valid
82-
? null
83-
: { message: validation.error || 'Invalid source configuration', errorCode: 'validation' }
84-
}
85-
}
8621

8722
export const GET = defineInternalJsonRoute({
8823
contract: getKnowledgeConnectorContract,
@@ -115,8 +50,6 @@ export const PATCH = defineInternalJsonRoute({
11550
connectorId: params.connectorId,
11651
knowledgeBaseId: params.id,
11752
updates: body,
118-
createSourceConfigValidator: (workspaceId: string, actingUserId: string) =>
119-
makeSourceConfigValidator(actingUserId, workspaceId, params.connectorId),
12053
source: 'ui' as const,
12154
}),
12255
useCase: updateKnowledgeConnector,

‎apps/sim/app/api/knowledge/[id]/connectors/route.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
} from '@/lib/api/contracts/knowledge'
55
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
66
import {
7-
internalKnowledgeActorUserId,
87
resolveInternalKnowledgeBillingAttribution,
98
toInternalKnowledgeConnector,
109
} from '@/lib/knowledge/api/internal-route'
@@ -17,7 +16,6 @@ import {
1716
listKnowledgeConnectors,
1817
} from '@/lib/knowledge/application/connectors'
1918
import { knowledgeOperations } from '@/lib/knowledge/application/operations'
20-
import { getCredential } from '@/app/api/auth/oauth/utils'
2119

2220
export const GET = defineInternalJsonRoute({
2321
contract: listKnowledgeConnectorsContract,
@@ -52,9 +50,6 @@ export const POST = defineInternalJsonRoute({
5250
syncIntervalMinutes: body.syncIntervalMinutes,
5351
resolveBillingAttribution: (workspaceId: string) =>
5452
resolveInternalKnowledgeBillingAttribution(request, principal, workspaceId),
55-
resolveAccessToken: async (requestId: string, credentialId: string) =>
56-
(await getCredential(requestId, credentialId, internalKnowledgeActorUserId(principal)))
57-
?.accessToken ?? null,
5853
source: 'ui' as const,
5954
}),
6055
useCase: createKnowledgeConnector,

‎apps/sim/app/api/knowledge/[id]/documents/upsert/route.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
} from '@/lib/knowledge/api/route-policies'
1212
import { upsertKnowledgeDocument } from '@/lib/knowledge/application/documents'
1313
import { knowledgeOperations } from '@/lib/knowledge/application/operations'
14-
import { getProcessingConfig } from '@/lib/knowledge/documents/service'
1514
import {
1615
createKnowledgeProvenanceResponse,
1716
resolveKnowledgeDocumentWriteSecretProvenance,
@@ -59,25 +58,22 @@ export const POST = defineInternalJsonRoute({
5958
},
6059
}),
6160
useCase: upsertKnowledgeDocument,
62-
present: ({ document, isUpdate, previousDocumentId }) => {
63-
const { maxConcurrentDocuments, batchSize } = getProcessingConfig()
64-
return {
65-
success: true as const,
66-
data: {
67-
documentsCreated: [
68-
{
69-
documentId: document.documentId,
70-
filename: document.filename,
71-
status: 'pending' as const,
72-
},
73-
],
74-
isUpdate,
75-
previousDocumentId,
76-
processingMethod: 'background' as const,
77-
processingConfig: { maxConcurrentDocuments, batchSize },
78-
},
79-
}
80-
},
61+
present: ({ document, isUpdate, previousDocumentId, processingConfig }) => ({
62+
success: true as const,
63+
data: {
64+
documentsCreated: [
65+
{
66+
documentId: document.documentId,
67+
filename: document.filename,
68+
status: 'pending' as const,
69+
},
70+
],
71+
isUpdate,
72+
previousDocumentId,
73+
processingMethod: 'background' as const,
74+
processingConfig,
75+
},
76+
}),
8177
renderResponse: ({ request, principal, result, body }) =>
8278
createKnowledgeProvenanceResponse({
8379
request,

‎apps/sim/app/api/knowledge/migrated-routes.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ describe('migrated internal Knowledge routes', () => {
303303
document: { documentId: 'document-2', filename: 'new.txt' },
304304
isUpdate: false,
305305
previousDocumentId: null,
306+
processingConfig: { maxConcurrentDocuments: 5, batchSize: 10 },
306307
workspaceId: 'workspace-1',
307308
userId: 'user-1',
308309
})

‎apps/sim/app/api/v2/knowledge/[id]/documents/route.test.ts‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import {
5+
V2_OPERATION_RATE_LIMIT_ALLOWED,
6+
V2_PREAUTH_RATE_LIMIT_ALLOWED,
7+
v2ApiKeyAuthModuleMock,
8+
v2GateModuleMock,
9+
v2RateLimiterModuleMock,
10+
v2RouteMocks,
11+
} from '@sim/testing'
412
import { NextRequest } from 'next/server'
513
import { beforeEach, describe, expect, it, vi } from 'vitest'
614

715
const {
8-
mockAuthenticate,
9-
mockCheckPreAuth,
10-
mockCheckRateLimit,
1116
mockAdmitUpload,
1217
mockUploadDocument,
1318
mockReadFormData,
@@ -17,9 +22,6 @@ const {
1722
mockCapture,
1823
mockIsPayloadSizeLimitError,
1924
} = vi.hoisted(() => ({
20-
mockAuthenticate: vi.fn(),
21-
mockCheckPreAuth: vi.fn(),
22-
mockCheckRateLimit: vi.fn(),
2325
mockAdmitUpload: vi.fn(),
2426
mockUploadDocument: vi.fn(),
2527
mockReadFormData: vi.fn(),
@@ -30,25 +32,9 @@ const {
3032
mockIsPayloadSizeLimitError: vi.fn(),
3133
}))
3234

33-
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({
34-
authenticateV2ApiKey: mockAuthenticate,
35-
V2ApiKeyUnauthenticatedError: class V2ApiKeyUnauthenticatedError extends Error {},
36-
}))
37-
38-
vi.mock('@/lib/core/rate-limiter', () => ({
39-
getRateLimit: () => ({ maxTokens: 100, refillRate: 100, refillIntervalMs: 60_000 }),
40-
RateLimiter: class RateLimiter {
41-
checkRateLimitDirect(...args: unknown[]) {
42-
return mockCheckPreAuth(...args)
43-
}
44-
45-
checkRateLimitDirectOrThrow(...args: unknown[]) {
46-
return mockCheckRateLimit(...args)
47-
}
48-
},
49-
}))
50-
51-
vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: vi.fn().mockResolvedValue(null) }))
35+
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
36+
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
37+
vi.mock('@/app/api/v2/lib/gate', () => v2GateModuleMock)
5238

5339
vi.mock('@/lib/knowledge/application/documents', () => ({
5440
listKnowledgeDocuments: {
@@ -89,12 +75,6 @@ import { validateFileType } from '@/lib/uploads/utils/validation'
8975
import { POST } from '@/app/api/v2/knowledge/[id]/documents/route'
9076

9177
const WORKSPACE_ID = 'workspace-1'
92-
const RATE_LIMIT_OK = {
93-
allowed: true,
94-
remaining: 99,
95-
resetAt: new Date('2024-01-01T01:00:00Z'),
96-
retryAfterMs: 0,
97-
}
9878
const PRINCIPAL = { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' } as const
9979

10080
function buildRequest() {
@@ -107,10 +87,11 @@ function buildRequest() {
10787
describe('POST /api/v2/knowledge/[id]/documents', () => {
10888
beforeEach(() => {
10989
vi.clearAllMocks()
110-
mockCheckPreAuth.mockResolvedValue(RATE_LIMIT_OK)
111-
mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK)
90+
v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED)
91+
v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED)
92+
v2RouteMocks.gate.mockResolvedValue(null)
11293
mockIsPayloadSizeLimitError.mockReturnValue(false)
113-
mockAuthenticate.mockResolvedValue({
94+
v2RouteMocks.authenticate.mockResolvedValue({
11495
principal: PRINCIPAL,
11596
rolloutUserId: 'user-1',
11697
rateLimitSubjectIds: ['api-key:key-1', 'user:user-1'],
@@ -218,7 +199,7 @@ describe('POST /api/v2/knowledge/[id]/documents', () => {
218199
})
219200

220201
it('does not create human analytics for a workspace key', async () => {
221-
mockAuthenticate.mockResolvedValue({
202+
v2RouteMocks.authenticate.mockResolvedValue({
222203
principal: { kind: 'workspace_api_key', workspaceId: WORKSPACE_ID, keyId: 'key-2' },
223204
rolloutUserId: 'billing-owner',
224205
rateLimitSubjectIds: ['api-key:key-2', `workspace:${WORKSPACE_ID}`],

‎apps/sim/app/api/v2/knowledge/search/route.test.ts‎

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,24 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import {
5+
V2_OPERATION_RATE_LIMIT_ALLOWED,
6+
V2_PREAUTH_RATE_LIMIT_ALLOWED,
7+
v2ApiKeyAuthModuleMock,
8+
v2GateModuleMock,
9+
v2RateLimiterModuleMock,
10+
v2RouteMocks,
11+
} from '@sim/testing'
412
import { NextRequest } from 'next/server'
513
import { beforeEach, describe, expect, it, vi } from 'vitest'
614

7-
const { mockAuthenticate, mockCheckPreAuth, mockCheckRateLimit, mockSearch } = vi.hoisted(() => ({
8-
mockAuthenticate: vi.fn(),
9-
mockCheckPreAuth: vi.fn(),
10-
mockCheckRateLimit: vi.fn(),
15+
const { mockSearch } = vi.hoisted(() => ({
1116
mockSearch: vi.fn(),
1217
}))
1318

14-
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({
15-
authenticateV2ApiKey: mockAuthenticate,
16-
V2ApiKeyUnauthenticatedError: class V2ApiKeyUnauthenticatedError extends Error {},
17-
}))
18-
19-
vi.mock('@/lib/core/rate-limiter', () => ({
20-
getRateLimit: () => ({ maxTokens: 100, refillRate: 100, refillIntervalMs: 60_000 }),
21-
RateLimiter: class RateLimiter {
22-
checkRateLimitDirect(...args: unknown[]) {
23-
return mockCheckPreAuth(...args)
24-
}
25-
26-
checkRateLimitDirectOrThrow(...args: unknown[]) {
27-
return mockCheckRateLimit(...args)
28-
}
29-
},
30-
}))
31-
32-
vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: vi.fn().mockResolvedValue(null) }))
19+
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
20+
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
21+
vi.mock('@/app/api/v2/lib/gate', () => v2GateModuleMock)
3322

3423
vi.mock('@/lib/knowledge/application/search', () => ({
3524
searchKnowledge: { operation: { id: 'knowledge.search' }, execute: mockSearch },
@@ -41,13 +30,6 @@ import { POST } from '@/app/api/v2/knowledge/search/route'
4130

4231
const WORKSPACE_ID = 'workspace-1'
4332
const PRINCIPAL = { kind: 'workspace_api_key', workspaceId: WORKSPACE_ID, keyId: 'key-1' } as const
44-
const RATE_LIMIT_OK = {
45-
allowed: true,
46-
remaining: 99,
47-
resetAt: new Date('2024-01-01T01:00:00Z'),
48-
retryAfterMs: 0,
49-
}
50-
5133
function buildRequest(body: string, headers: Record<string, string> = {}) {
5234
return new NextRequest('http://localhost/api/v2/knowledge/search', {
5335
method: 'POST',
@@ -59,9 +41,10 @@ function buildRequest(body: string, headers: Record<string, string> = {}) {
5941
describe('POST /api/v2/knowledge/search', () => {
6042
beforeEach(() => {
6143
vi.clearAllMocks()
62-
mockCheckPreAuth.mockResolvedValue(RATE_LIMIT_OK)
63-
mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK)
64-
mockAuthenticate.mockResolvedValue({
44+
v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED)
45+
v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED)
46+
v2RouteMocks.gate.mockResolvedValue(null)
47+
v2RouteMocks.authenticate.mockResolvedValue({
6548
principal: PRINCIPAL,
6649
rolloutUserId: 'billing-owner',
6750
rateLimitSubjectIds: ['api-key:key-1', `workspace:${WORKSPACE_ID}`],
@@ -122,7 +105,7 @@ describe('POST /api/v2/knowledge/search', () => {
122105
const response = await POST(buildRequest('{'))
123106

124107
expect(response.status).toBe(400)
125-
expect(mockAuthenticate).toHaveBeenCalledOnce()
108+
expect(v2RouteMocks.authenticate).toHaveBeenCalledOnce()
126109
expect(mockSearch).not.toHaveBeenCalled()
127110
})
128111

0 commit comments

Comments
 (0)