Skip to content

Commit b9b2edf

Browse files
authored
fix: idempotent user creation (#73)
* feat: enhance user synchronization and update logic in PadService and UserService - Refactored user creation logic in PadService to utilize a failsafe mechanism for user creation based on user session data. - Introduced a new method in UserService to update user data only if changes are detected, improving data consistency. - Enhanced error handling during user synchronization to manage race conditions and ensure robust user data management. * refactor: remove print statement from AuthDependency error handling - Eliminated the print statement for authentication errors in AuthDependency to streamline error handling and reduce console clutter.
1 parent a59dc29 commit b9b2edf

File tree

3 files changed

+65
-44
lines changed

3 files changed

+65
-44
lines changed

src/backend/database/service/pad_service.py

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,34 +35,33 @@ async def create_pad(self, owner_id: UUID, display_name: str, data: Dict[str, An
3535
# Check if owner exists
3636
owner = await self.user_repository.get_by_id(owner_id)
3737
if not owner and user_session:
38-
# User doesn't exist, create a user record from user session
39-
print(f"ANOMALY DETECTED: User with ID '{owner_id}' does not exist but has valid authentication. Creating user as failsafe.")
38+
# Reaching here is an anomaly, this is a failsafe
39+
# User should already exist in the database
4040

4141
# Create a UserService instance
4242
user_service = UserService(self.session)
4343

44-
# Create user with data from UserSession
44+
# Create token data dictionary from UserSession properties
45+
token_data = {
46+
"username": user_session.username,
47+
"email": user_session.email,
48+
"email_verified": user_session.email_verified,
49+
"name": user_session.name,
50+
"given_name": user_session.given_name,
51+
"family_name": user_session.family_name,
52+
"roles": user_session.roles
53+
}
54+
55+
# Use sync_user_with_token_data which handles race conditions
4556
try:
46-
await user_service.create_user(
47-
user_id=user_session.id,
48-
username=user_session.username,
49-
email=user_session.email,
50-
email_verified=user_session.email_verified,
51-
name=user_session.name,
52-
given_name=user_session.given_name,
53-
family_name=user_session.family_name,
54-
roles=user_session.roles
55-
)
56-
print(f"Successfully created user with ID '{owner_id}' as failsafe.")
57-
except ValueError as e:
57+
await user_service.sync_user_with_token_data(owner_id, token_data)
58+
# Get the user again to confirm it exists
59+
owner = await self.user_repository.get_by_id(owner_id)
60+
if not owner:
61+
raise ValueError(f"Failed to create user with ID '{owner_id}'")
62+
except Exception as e:
5863
print(f"Error creating user as failsafe: {str(e)}")
59-
# If user creation fails due to race condition, try to get the user again
60-
if "already exists" in str(e):
61-
owner = await self.user_repository.get_by_id(owner_id)
62-
if not owner:
63-
raise ValueError(f"Failed to create user with ID '{owner_id}'")
64-
else:
65-
raise
64+
raise ValueError(f"Failed to create user with ID '{owner_id}': {str(e)}")
6665

6766
# Check if pad with same name already exists for this owner
6867
existing_pad = await self.repository.get_by_name(owner_id, display_name)

src/backend/database/service/user_service.py

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ async def delete_user(self, user_id: UUID) -> bool:
9898
"""Delete a user"""
9999
return await self.repository.delete(user_id)
100100

101+
async def update_user_if_needed(self, user_id: UUID, token_data: Dict[str, Any], user_data: Dict[str, Any]) -> Dict[str, Any]:
102+
"""
103+
Update user only if data has changed
104+
105+
Args:
106+
user_id: The user's UUID
107+
token_data: Dictionary containing user data from the authentication token
108+
user_data: Current user data from the database
109+
110+
Returns:
111+
The updated user data dictionary or the original if no update was needed
112+
"""
113+
# Check if user data needs to be updated
114+
update_data = {}
115+
fields_to_check = [
116+
"username", "email", "email_verified",
117+
"name", "given_name", "family_name"
118+
]
119+
120+
for field in fields_to_check:
121+
token_value = token_data.get(field)
122+
if token_value is not None and user_data.get(field) != token_value:
123+
update_data[field] = token_value
124+
125+
# Handle roles separately as they might have a different structure
126+
if "roles" in token_data and user_data.get("roles") != token_data["roles"]:
127+
update_data["roles"] = token_data["roles"]
128+
129+
# Update user if any field has changed
130+
if update_data:
131+
return await self.update_user(user_id, update_data)
132+
133+
return user_data
134+
101135
async def sync_user_with_token_data(self, user_id: UUID, token_data: Dict[str, Any]) -> Optional[Dict[str, Any]]:
102136
"""
103137
Synchronize user data in the database with data from the authentication token.
@@ -117,6 +151,7 @@ async def sync_user_with_token_data(self, user_id: UUID, token_data: Dict[str, A
117151
# If user doesn't exist, create a new one
118152
if not user_data:
119153
try:
154+
print(f"User with ID '{user_id}' does not exist. Creating user from token data.")
120155
return await self.create_user(
121156
user_id=user_id,
122157
username=token_data.get("username", ""),
@@ -131,28 +166,16 @@ async def sync_user_with_token_data(self, user_id: UUID, token_data: Dict[str, A
131166
print(f"Error creating user: {e}")
132167
# Handle case where user might have been created in a race condition
133168
if "already exists" in str(e):
169+
print(f"Race condition detected: User with ID '{user_id}' was created by another process.")
134170
user_data = await self.get_user(user_id)
171+
if user_data:
172+
# User exists now, proceed with update if needed
173+
return await self.update_user_if_needed(user_id, token_data, user_data)
174+
else:
175+
# This shouldn't happen - user creation failed but user doesn't exist
176+
raise ValueError(f"Failed to create or find user with ID '{user_id}'")
135177
else:
136178
raise e
137179

138-
# Check if user data needs to be updated
139-
update_data = {}
140-
fields_to_check = [
141-
"username", "email", "email_verified",
142-
"name", "given_name", "family_name"
143-
]
144-
145-
for field in fields_to_check:
146-
token_value = token_data.get(field)
147-
if token_value is not None and user_data.get(field) != token_value:
148-
update_data[field] = token_value
149-
150-
# Handle roles separately as they might have a different structure
151-
if "roles" in token_data and user_data.get("roles") != token_data["roles"]:
152-
update_data["roles"] = token_data["roles"]
153-
154-
# Update user if any field has changed
155-
if update_data:
156-
return await self.update_user(user_id, update_data)
157-
158-
return user_data
180+
# User exists, update if needed
181+
return await self.update_user_if_needed(user_id, token_data, user_data)

src/backend/dependencies.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ def _handle_auth_error(self, detail: str, status_code: int = 401) -> Optional[No
137137
"""Handle authentication errors based on auto_error setting"""
138138
if self.auto_error:
139139
headers = {"WWW-Authenticate": "Bearer"} if status_code == 401 else None
140-
print(f"Authentication error: {detail}")
141140
raise HTTPException(
142141
status_code=status_code,
143142
detail=detail,

0 commit comments

Comments
 (0)