Skip to content

Commit

Permalink
Deprecate /users/register for /users, retrieval bug in list chunks
Browse files Browse the repository at this point in the history
  • Loading branch information
NolanTrem committed Dec 13, 2024
1 parent 6d50084 commit 16434c0
Show file tree
Hide file tree
Showing 26 changed files with 269 additions and 67 deletions.
34 changes: 32 additions & 2 deletions js/sdk/__tests__/DocumentsAndCollectionsIntegrationUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe("r2rClient V3 System Integration Tests User", () => {
});

test("Register user 1", async () => {
const response = await client.users.register({
const response = await client.users.create({
email: "user_1@example.com",
password: "change_me_immediately",
});
Expand All @@ -52,7 +52,7 @@ describe("r2rClient V3 System Integration Tests User", () => {
});

test("Register user 2", async () => {
const response = await client.users.register({
const response = await client.users.create({
email: "user_2@example.com",
password: "change_me_immediately",
});
Expand Down Expand Up @@ -160,6 +160,36 @@ describe("r2rClient V3 System Integration Tests User", () => {
expect(Array.isArray(response.results)).toBe(true);
});

test("List document chunks as user 1", async () => {
const response = await user1Client.documents.listChunks({
id: user1DocumentId,
});

expect(response.results).toBeDefined();
expect(Array.isArray(response.results)).toBe(true);
});

test("List document chunks as user 2", async () => {
const response = await user2Client.documents.listChunks({
id: user2DocumentId,
});

expect(response.results).toBeDefined();
expect(Array.isArray(response.results)).toBe(true);
});

test("User 2 should not be able to list user 1's document chunks", async () => {
await expect(
user2Client.documents.listChunks({ id: user1DocumentId }),
).rejects.toThrow(/Status 403/);
});

test("User 1 should not be able to list user 2's document chunks", async () => {
await expect(
user1Client.documents.listChunks({ id: user2DocumentId }),
).rejects.toThrow(/Status 403/);
});

test("Delete document as user 1", async () => {
const response = await user1Client.documents.delete({
id: user1DocumentId,
Expand Down
4 changes: 3 additions & 1 deletion js/sdk/__tests__/GraphsIntegrationSuperUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ describe("r2rClient V3 Graphs Integration Tests", () => {
expect(response.results.subject).toBe("Razumikhin");
expect(response.results.object).toBe("Dunia");
expect(response.results.predicate).toBe("falls in love with");
expect(response.results.description).toBe("Razumikhn and Dunia are central to the story");
expect(response.results.description).toBe(
"Razumikhn and Dunia are central to the story",
);
});

test("Retrieve the relationship", async () => {
Expand Down
7 changes: 5 additions & 2 deletions js/sdk/__tests__/SystemIntegrationUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,19 @@ describe("r2rClient V3 System Integration Tests User", () => {
});

test("Register a new user", async () => {
const response = await client.users.register({
const response = await client.users.create({
email: "system_integration_test_user@example.com",
password: "change_me_immediately",
name: "Test User",
bio: "This is the bio of the test user.",
});

userId = response.results.id;
name = response.results.name;
expect(response.results).toBeDefined();
expect(response.results.is_superuser).toBe(false);
expect(response.results.name).toBe(null);
expect(response.results.name).toBe("Test User");
expect(response.results.bio).toBe("This is the bio of the test user.");
});

test("Login as a user", async () => {
Expand Down
2 changes: 1 addition & 1 deletion js/sdk/__tests__/UsersIntegrationSuperUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("r2rClient V3 Users Integration Tests", () => {
});

test("Register a new user", async () => {
const response = await client.users.register({
const response = await client.users.create({
email: "new_user@example.com",
password: "change_me_immediately",
});
Expand Down
35 changes: 34 additions & 1 deletion js/sdk/src/v3/clients/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,44 @@ import {
export class UsersClient {
constructor(private client: r2rClient) {}

/**
* Create a new user.
* @param email User's email address
* @param password User's password
* @param name The name for the new user
* @param bio The bio for the new user
* @param profilePicture The profile picture for the new user
* @returns WrappedUserResponse
*/
@feature("users.create")
async create(options: {
email: string;
password: string;
name?: string;
bio?: string;
profilePicture?: string;
}): Promise<WrappedUserResponse> {
const data = {
...(options.email && { email: options.email }),
...(options.password && { password: options.password }),
...(options.name && { name: options.name }),
...(options.bio && { bio: options.bio }),
...(options.profilePicture && {
profile_picture: options.profilePicture,
}),
};

return this.client.makeRequest("POST", "users", {
data: data,
});
}

/**
* Register a new user.
* @param email User's email address
* @param password User's password
* @returns
* @returns WrappedUserResponse
* @deprecated Use `client.users.create` instead.
*/
@feature("users.register")
async register(options: {
Expand Down
4 changes: 2 additions & 2 deletions py/cli/commands/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ def users():
@click.argument("email", required=True, type=str)
@click.argument("password", required=True, type=str)
@pass_context
async def register(ctx, email, password):
async def create(ctx, email, password):
"""Create a new user."""
client: R2RAsyncClient = ctx.obj

with timer():
response = await client.users.register(email=email, password=password)
response = await client.users.create(email=email, password=password)

click.echo(json.dumps(response, indent=2))

Expand Down
2 changes: 1 addition & 1 deletion py/core/examples/scripts/test_v3_sdk/test_v3_sdk_chunks.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def generate_random_email():

# First create and authenticate a user if not already done
try:
new_user = client.users.register(
new_user = client.users.create(
email=user_email, password="new_secure_password123"
)
print("New user created:", new_user)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

# First create and authenticate a user if not already done
try:
new_user = client.users.register(
new_user = client.users.create(
email=user_email, password="new_secure_password123"
)
print("New user created:", new_user)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

# First create and authenticate a user if not already done
try:
new_user = client.users.register(
new_user = client.users.create(
email=user_email, password="new_secure_password123"
)
print("New user created:", new_user)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

# First create and authenticate a user if not already done
try:
new_user = client.users.register(
new_user = client.users.create(
email=user_email, password="new_secure_password123"
)
print("New user created:", new_user)
Expand Down
2 changes: 1 addition & 1 deletion py/core/examples/scripts/test_v3_sdk/test_v3_sdk_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def setup_prerequisites():

# # Login
# try:
# client.users.register(email=user_email, password="new_secure_password123")
# client.users.create(email=user_email, password="new_secure_password123")
# except Exception as e:
# print("User might already exist:", str(e))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

# # First create and authenticate a user if not already done
# try:
# new_user = client.users.register(
# new_user = client.users.create(
# email=user_email, password="new_secure_password123"
# )
# print("New user created:", new_user)
Expand Down
2 changes: 1 addition & 1 deletion py/core/examples/scripts/test_v3_sdk/test_v3_sdk_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def generate_random_email():

# Test 1: Register user
print("\n=== Test 1: Register User ===")
register_result = client.users.register(
register_result = client.users.create(
email=user_email, password="secure_password123"
)
print("Registered user:", register_result)
Expand Down
5 changes: 1 addition & 4 deletions py/core/main/api/v3/documents_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -823,10 +823,7 @@ async def list_chunks(
user_has_access = (
is_owner
or set(auth_user.collection_ids).intersection(
{
ele.collection_id
for ele in document_collections["results"]
}
{ele.id for ele in document_collections["results"]}
)
!= set()
)
Expand Down
92 changes: 91 additions & 1 deletion py/core/main/api/v3/users_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,97 @@ def __init__(

def _setup_routes(self):

# New authentication routes
@self.router.post(
"/users",
response_model=WrappedUserResponse,
openapi_extra={
"x-codeSamples": [
{
"lang": "Python",
"source": textwrap.dedent(
"""
from r2r import R2RClient
client = R2RClient("http://localhost:7272")
new_user = client.users.create(
email="jane.doe@example.com",
password="secure_password123"
)"""
),
},
{
"lang": "JavaScript",
"source": textwrap.dedent(
"""
const { r2rClient } = require("r2r-js");
const client = new r2rClient("http://localhost:7272");
function main() {
const response = await client.users.create({
email: "jane.doe@example.com",
password: "secure_password123"
});
}
main();
"""
),
},
{
"lang": "CLI",
"source": textwrap.dedent(
"""
r2r users create jane.doe@example.com secure_password123
"""
),
},
{
"lang": "cURL",
"source": textwrap.dedent(
"""
curl -X POST "https://api.example.com/v3/users" \\
-H "Content-Type: application/json" \\
-d '{
"email": "jane.doe@example.com",
"password": "secure_password123"
}'"""
),
},
]
},
)
@self.base_endpoint
async def register(
email: EmailStr = Body(..., description="User's email address"),
password: str = Body(..., description="User's password"),
name: str | None = Body(
None, description="The name for the new user"
),
bio: str | None = Body(
None, description="The bio for the new user"
),
profile_picture: str | None = Body(
None, description="Updated user profile picture"
),
auth_user=Depends(self.providers.auth.auth_wrapper),
) -> WrappedUserResponse:
"""Register a new user with the given email and password."""
registration_response = await self.services["auth"].register(
email, password
)

if name or bio or profile_picture:
return await self.services["auth"].update_user(
user_id=registration_response.id,
name=name,
bio=bio,
profile_picture=profile_picture,
)

return registration_response

# TODO: deprecated, remove in next release
@self.router.post(
"/users/register",
response_model=WrappedUserResponse,
Expand Down
7 changes: 6 additions & 1 deletion py/core/main/services/auth_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,13 @@ async def delete_user(
# Delete user's default collection
# TODO: We need to better define what happens to the user's data when they are deleted
collection_id = generate_default_user_collection_id(user_id)

await self.providers.database.graph_handler.delete_graph_for_collection(
collection_id=collection_id,
)

await self.providers.database.delete_collection_relational(
collection_id
collection_id=collection_id,
)

if delete_vector_data:
Expand Down
23 changes: 12 additions & 11 deletions py/core/providers/database/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -2067,7 +2067,7 @@ async def delete_graph_for_collection(

# don't delete if status is PROCESSING.
QUERY = f"""
SELECT graph_cluster_status FROM {self._get_table_name("collections")} WHERE collection_id = $1
SELECT graph_cluster_status FROM {self._get_table_name("collections")} WHERE id = $1
"""
status = (
await self.connection_manager.fetch_query(QUERY, [collection_id])
Expand Down Expand Up @@ -2108,19 +2108,20 @@ async def delete_graph_for_collection(
QUERY, [KGExtractionStatus.PENDING, collection_id]
)

for query in DELETE_QUERIES:
if "community" in query or "graphs_entities" in query:
await self.connection_manager.execute_query(
query, [collection_id]
)
else:
await self.connection_manager.execute_query(
query, [document_ids]
)
if document_ids:
for query in DELETE_QUERIES:
if "community" in query or "graphs_entities" in query:
await self.connection_manager.execute_query(
query, [collection_id]
)
else:
await self.connection_manager.execute_query(
query, [document_ids]
)

# set status to PENDING for this collection.
QUERY = f"""
UPDATE {self._get_table_name("collections")} SET graph_cluster_status = $1 WHERE collection_id = $2
UPDATE {self._get_table_name("collections")} SET graph_cluster_status = $1 WHERE id = $2
"""
await self.connection_manager.execute_query(
QUERY, [KGExtractionStatus.PENDING, collection_id]
Expand Down
2 changes: 1 addition & 1 deletion py/sdk/v2/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@


class AuthMixins:
@deprecated("Use client.users.register() instead")
@deprecated("Use client.users.create() instead")
async def register(self, email: str, password: str) -> User:
"""
Registers a new user with the given email and password.
Expand Down
Loading

0 comments on commit 16434c0

Please sign in to comment.