From 47409353c7322c168dc2d486d55530a5c576a06c Mon Sep 17 00:00:00 2001 From: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com> Date: Mon, 23 Sep 2024 16:27:01 -0700 Subject: [PATCH] Fix bugs in auth router, js sdk auto refresh (#1255) * JS testing fixes, bugs in auth * More --- .../r2r-js-sdk-integration-tests.yml | 4 +- .../r2rClientIntegrationSuperUser.test.ts | 106 +++++++++++++++--- .../r2rClientIntegrationUser.test.ts | 88 +++++++++++++-- js/sdk/package-lock.json | 12 +- js/sdk/package.json | 3 +- js/sdk/pnpm-lock.yaml | 13 ++- js/sdk/src/r2rClient.ts | 30 ++--- js/sdk/tsconfig.json | 5 +- py/core/main/api/auth_router.py | 2 +- 9 files changed, 207 insertions(+), 56 deletions(-) diff --git a/.github/workflows/r2r-js-sdk-integration-tests.yml b/.github/workflows/r2r-js-sdk-integration-tests.yml index 0a33dbb93..6ce5241fd 100644 --- a/.github/workflows/r2r-js-sdk-integration-tests.yml +++ b/.github/workflows/r2r-js-sdk-integration-tests.yml @@ -41,8 +41,8 @@ jobs: POSTGRES_PROJECT_NAME: ${{ secrets.POSTGRES_PROJECT_NAME }} OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | - r2r serve --port=7272 > r2r_server.log 2>&1 & - sleep 15 + r2r serve --docker + sleep 60 - name: Use Node.js uses: actions/setup-node@v2 diff --git a/js/sdk/__tests__/r2rClientIntegrationSuperUser.test.ts b/js/sdk/__tests__/r2rClientIntegrationSuperUser.test.ts index 5ecb5d13e..a0d6ec431 100644 --- a/js/sdk/__tests__/r2rClientIntegrationSuperUser.test.ts +++ b/js/sdk/__tests__/r2rClientIntegrationSuperUser.test.ts @@ -1,5 +1,6 @@ import { r2rClient } from "../src/index"; const fs = require("fs"); +import { describe, test, beforeAll, expect } from "@jest/globals"; const baseUrl = "http://localhost:7272"; @@ -9,6 +10,61 @@ const baseUrl = "http://localhost:7272"; * myshkin.txt should have an id of `2e05b285-2746-5778-9e4a-e293db92f3be` */ +/** + * Coverage + * - health + * Auth: + * X register + * X verifyEmail + * - login + * - logout + * X user + * X updateUser + * - refreshAccessToken + * X changePassword + * X requestPasswordReset + * X confirmPasswordReset + * X deleteUser + * Ingestion: + * - ingestFiles + * - updateFiles + * Management: + * - serverStats + * X updatePrompt + * - analytics + * - logs + * - appSettings + * - scoreCompletion + * - usersOverview + * - delete + * X downloadFile + * - documentsOverview + * - documentChunks + * X inspectKnowledgeGraph + * X collectionsOverview + * X createCollection + * X getCollection + * X updateCollection + * X deleteCollection + * X listCollections + * X addUserToCollection + * X removeUserFromCollection + * X getUsersInCollection + * X getCollectionsForUser + * X assignDocumentToCollection + * X removeDocumentFromCollection + * X getDocumentCollections + * X getDocumentsInCollection + * Restructure: + * X enrichGraph + * Retrieval: + * - search + * - rag + * X streamingRag + * - agent + * X streamingAgent + */ + describe("r2rClient Integration Tests", () => { let client: r2rClient; @@ -94,32 +150,34 @@ describe("r2rClient Integration Tests", () => { }); // TOOD: Fix in R2R, table logs has no column named run_id - // test("Agentic RAG response with streaming", async () => { - // const messages = [ - // { role: "system", content: "You are a helpful assistant." }, - // { role: "user", content: "Tell me about Raskolnikov." }, - // ]; + test("Agentic RAG response with streaming", async () => { + const messages = [ + { role: "system", content: "You are a helpful assistant." }, + { role: "user", content: "Tell me about Raskolnikov." }, + ]; - // const stream = await client.agent(messages, undefined, undefined, { - // stream: true, - // }); + const stream = await client.agent(messages, undefined, undefined, { + stream: true, + }); - // expect(stream).toBeDefined(); + expect(stream).toBeDefined(); - // let fullResponse = ""; + let fullResponse = ""; - // for await (const chunk of stream) { - // fullResponse += chunk; - // } + for await (const chunk of stream) { + fullResponse += chunk; + } - // expect(fullResponse.length).toBeGreaterThan(0); - // }, 30000); + expect(fullResponse.length).toBeGreaterThan(0); + }, 30000); // Deletes raskolnikov.txt test("Delete document", async () => { await expect( client.delete({ - document_id: "f9f61fc8-079c-52d0-910a-c657958e385b", + document_id: { + $eq: "f9f61fc8-079c-52d0-910a-c657958e385b", + }, }), ).resolves.toBe(""); }); @@ -132,6 +190,10 @@ describe("r2rClient Integration Tests", () => { await expect(client.appSettings()).resolves.not.toThrow(); }); + test("Refresh access token", async () => { + await expect(client.refreshAccessToken()).resolves.not.toThrow(); + }); + test("Get analytics", async () => { const filterCriteria: Record | string = { search_latencies: "search_latency", @@ -163,12 +225,20 @@ describe("r2rClient Integration Tests", () => { test("Clean up remaining documents", async () => { // Deletes karamozov.txt await expect( - client.delete({ document_id: "73749580-1ade-50c6-8fbe-a5e9e87783c8" }), + client.delete({ + document_id: { + $eq: "73749580-1ade-50c6-8fbe-a5e9e87783c8", + }, + }), ).resolves.toBe(""); // Deletes myshkin.txt await expect( - client.delete({ document_id: "2e05b285-2746-5778-9e4a-e293db92f3be" }), + client.delete({ + document_id: { + $eq: "2e05b285-2746-5778-9e4a-e293db92f3be", + }, + }), ).resolves.toBe(""); }); diff --git a/js/sdk/__tests__/r2rClientIntegrationUser.test.ts b/js/sdk/__tests__/r2rClientIntegrationUser.test.ts index 2ee920403..7df880f84 100644 --- a/js/sdk/__tests__/r2rClientIntegrationUser.test.ts +++ b/js/sdk/__tests__/r2rClientIntegrationUser.test.ts @@ -9,6 +9,61 @@ const baseUrl = "http://localhost:7272"; * myshkin.txt should have an id of `0b80081e-a37a-579f-a06d-7d2032435d65` */ +/** + * Coverage + * - health + * Auth: + * - register + * X verifyEmail + * - login + * - logout + * X user + * X updateUser + * - refreshAccessToken + * - changePassword + * X requestPasswordReset + * X confirmPasswordReset + * - deleteUser + * Ingestion: + * - ingestFiles + * - updateFiles + * Management: + * X serverStats + * X updatePrompt + * X analytics + * X logs + * - appSettings + * X scoreCompletion + * X usersOverview + * - delete + * X downloadFile + * - documentsOverview + * X documentChunks + * X inspectKnowledgeGraph + * X collectionsOverview + * X createCollection + * X getCollection + * X updateCollection + * X deleteCollection + * X listCollections + * X addUserToCollection + * X removeUserFromCollection + * X getUsersInCollection + * X getCollectionsForUser + * X assignDocumentToCollection + * X removeDocumentFromCollection + * X getDocumentCollections + * X getDocumentsInCollection + * Restructure: + * X enrichGraph + * Retrieval: + * - search + * X rag + * X streamingRag + * X agent + * X streamingAgent + */ + describe("r2rClient Integration Tests", () => { let client: r2rClient; @@ -68,7 +123,11 @@ describe("r2rClient Integration Tests", () => { // Deletes rasolnikov.txt test("Delete document", async () => { await expect( - client.delete({ document_id: "91662726-7271-51a5-a0ae-34818509e1fd" }), + client.delete({ + document_id: { + $eq: "91662726-7271-51a5-a0ae-34818509e1fd", + }, + }), ).resolves.not.toThrow(); }); @@ -78,6 +137,10 @@ describe("r2rClient Integration Tests", () => { ); }); + test("Refresh access token", async () => { + await expect(client.refreshAccessToken()).resolves.not.toThrow(); + }); + test("Get documents overview", async () => { await expect(client.documentsOverview()).resolves.not.toThrow(); }); @@ -95,12 +158,20 @@ describe("r2rClient Integration Tests", () => { test("Clean up remaining documents", async () => { // Deletes karamozov.txt await expect( - client.delete({ document_id: "00f69fa0-c947-5f5f-a374-1837a1283366" }), + client.delete({ + document_id: { + $eq: "00f69fa0-c947-5f5f-a374-1837a1283366", + }, + }), ).resolves.not.toThrow(); // Deletes myshkin.txt await expect( - client.delete({ document_id: "0b80081e-a37a-579f-a06d-7d2032435d65" }), + client.delete({ + document_id: { + $eq: "0b80081e-a37a-579f-a06d-7d2032435d65", + }, + }), ).resolves.not.toThrow; }); @@ -110,9 +181,10 @@ describe("r2rClient Integration Tests", () => { ).resolves.not.toThrow(); }); - // TODO: Fix this test - // test("Delete User", async () => { - // const currentUser = await client.user(); - // await expect(client.deleteUser(currentUser.id, "new_password")).resolves.not.toThrow(); - // }); + test("Delete User", async () => { + const currentUser = await client.user(); + await expect( + client.deleteUser(currentUser.results.id, "new_password"), + ).resolves.not.toThrow(); + }); }); diff --git a/js/sdk/package-lock.json b/js/sdk/package-lock.json index dd9159003..855a0f592 100644 --- a/js/sdk/package-lock.json +++ b/js/sdk/package-lock.json @@ -1,12 +1,12 @@ { "name": "r2r-js", - "version": "0.3.3", + "version": "0.3.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "r2r-js", - "version": "0.3.3", + "version": "0.3.4", "license": "ISC", "dependencies": { "axios": "^1.7.4", @@ -16,7 +16,7 @@ "uuid": "^10.0.0" }, "devDependencies": { - "@types/jest": "^29.5.12", + "@types/jest": "^29.5.13", "@types/node": "^20.14.15", "@types/uuid": "^10.0.0", "jest": "^29.7.0", @@ -1141,9 +1141,9 @@ } }, "node_modules/@types/jest": { - "version": "29.5.12", - "resolved": "https://registry.npmjs.org/@types/jest/-/jest-29.5.12.tgz", - "integrity": "sha512-eDC8bTvT/QhYdxJAulQikueigY5AsdBRH2yDKW3yveW7svY3+DzN84/2NUgkw10RTiJbWqZrTtoGVdYlvFJdLw==", + "version": "29.5.13", + "resolved": "https://registry.npmjs.org/@types/jest/-/jest-29.5.13.tgz", + "integrity": "sha512-wd+MVEZCHt23V0/L642O5APvspWply/rGY5BcW4SUETo2UzPU3Z26qr8jC2qxpimI2jjx9h7+2cj2FwIr01bXg==", "dev": true, "license": "MIT", "dependencies": { diff --git a/js/sdk/package.json b/js/sdk/package.json index 10de99b40..37a834f9e 100644 --- a/js/sdk/package.json +++ b/js/sdk/package.json @@ -24,6 +24,7 @@ "author": "", "license": "ISC", "dependencies": { + "@jest/globals": "^29.7.0", "axios": "^1.7.4", "form-data": "^4.0.0", "posthog-js": "^1.155.4", @@ -31,7 +32,7 @@ "uuid": "^10.0.0" }, "devDependencies": { - "@types/jest": "^29.5.12", + "@types/jest": "^29.5.13", "@types/node": "^20.14.15", "@types/uuid": "^10.0.0", "jest": "^29.7.0", diff --git a/js/sdk/pnpm-lock.yaml b/js/sdk/pnpm-lock.yaml index 9c23c72b6..d9dfad6ce 100644 --- a/js/sdk/pnpm-lock.yaml +++ b/js/sdk/pnpm-lock.yaml @@ -7,6 +7,9 @@ settings: importers: .: dependencies: + "@jest/globals": + specifier: ^29.7.0 + version: 29.7.0 axios: specifier: ^1.7.4 version: 1.7.4 @@ -24,8 +27,8 @@ importers: version: 10.0.0 devDependencies: "@types/jest": - specifier: ^29.5.12 - version: 29.5.12 + specifier: ^29.5.13 + version: 29.5.13 "@types/node": specifier: ^20.14.15 version: 20.14.15 @@ -591,10 +594,10 @@ packages: integrity: sha512-pk2B1NWalF9toCRu6gjBzR69syFjP4Od8WRAX+0mmf9lAjCRicLOWc+ZrxZHx/0XRjotgkF9t6iaMJ+aXcOdZQ==, } - "@types/jest@29.5.12": + "@types/jest@29.5.13": resolution: { - integrity: sha512-eDC8bTvT/QhYdxJAulQikueigY5AsdBRH2yDKW3yveW7svY3+DzN84/2NUgkw10RTiJbWqZrTtoGVdYlvFJdLw==, + integrity: sha512-wd+MVEZCHt23V0/L642O5APvspWply/rGY5BcW4SUETo2UzPU3Z26qr8jC2qxpimI2jjx9h7+2cj2FwIr01bXg==, } "@types/node@20.14.15": @@ -2775,7 +2778,7 @@ snapshots: dependencies: "@types/istanbul-lib-report": 3.0.3 - "@types/jest@29.5.12": + "@types/jest@29.5.13": dependencies: expect: 29.7.0 pretty-format: 29.7.0 diff --git a/js/sdk/src/r2rClient.ts b/js/sdk/src/r2rClient.ts index abb2d3ac2..b5af49e68 100644 --- a/js/sdk/src/r2rClient.ts +++ b/js/sdk/src/r2rClient.ts @@ -202,10 +202,6 @@ export class r2rClient { // } } - async health(): Promise { - return await this._makeRequest("GET", "health"); - } - // ----------------------------------------------------------------------------- // // Auth @@ -332,7 +328,12 @@ export class r2rClient { const response = await this._makeRequest( "POST", "refresh_access_token", - { data: { refresh_token: this.refreshToken } }, + { + data: this.refreshToken, + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + }, ); if (response && response.results) { @@ -403,14 +404,9 @@ export class r2rClient { @feature("deleteUser") async deleteUser(userId: string, password?: string): Promise { this._ensureAuthenticated(); - - const data: Record = { user_id: userId }; - - if (password) { - data.password = password; - } - - return await this._makeRequest("DELETE", "user", { data }); + return await this._makeRequest("DELETE", `user/${userId}`, { + data: { password }, + }); } // ----------------------------------------------------------------------------- @@ -606,6 +602,14 @@ export class r2rClient { // // ----------------------------------------------------------------------------- + /** + * Check the health of the R2R deployment. + * @returns A promise that resolves to the response from the server. + */ + async health(): Promise { + return await this._makeRequest("GET", "health"); + } + /** * Get statistics about the server, including the start time, uptime, CPU usage, and memory usage. * @returns A promise that resolves to the response from the server. diff --git a/js/sdk/tsconfig.json b/js/sdk/tsconfig.json index df060141f..c7c319776 100644 --- a/js/sdk/tsconfig.json +++ b/js/sdk/tsconfig.json @@ -13,8 +13,9 @@ "forceConsistentCasingInFileNames": true, "jsx": "react", "lib": ["es2017", "dom"], - "sourceMap": true + "sourceMap": true, + "types": ["node", "jest", "@types/jest"] }, "include": ["src/**/*"], - "exclude": ["node_modules", "**/__tests__/*"] + "exclude": ["node_modules", "**/__tests__/*", "**/*.spec.ts"] } diff --git a/py/core/main/api/auth_router.py b/py/core/main/api/auth_router.py index 5b9d5aeb5..1a3bd7851 100644 --- a/py/core/main/api/auth_router.py +++ b/py/core/main/api/auth_router.py @@ -228,7 +228,7 @@ async def delete_user_app( This endpoint allows users to delete their own account or, for superusers, to delete any user account. """ - if auth_user.id != user_id and not auth_user.is_superuser: + if str(auth_user.id) != user_id and not auth_user.is_superuser: raise Exception("User ID does not match authenticated user") if not auth_user.is_superuser and not password: raise Exception("Password is required for non-superusers")