Skip to content

Commit

Permalink
Fix unclosed SQLAlchemy sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
bfbachmann committed Feb 5, 2019
1 parent a50ad15 commit 5d00a6e
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 47 deletions.
11 changes: 8 additions & 3 deletions bounce/server/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,14 @@ async def handle_request(self, request, *args, **kwargs):
Args:
request (Request): the incoming request to route
"""
# Create a SQLAlchemy session for handling DB transactions in this
# request
session = self.server.db_session
result = None
try:
# Call the handler with the same name as the request method
result = await getattr(self, request.method.lower())(
request, *args, **kwargs)
session, request, *args, **kwargs)
except APIError as err:
# An error was raised by the handler because there was something
# wrong with the request
Expand All @@ -140,6 +143,8 @@ async def handle_request(self, request, *args, **kwargs):
result.headers, 'Access-Control-Allow-Origin'):
result.headers[
'Access-Control-Allow-Origin'] = self._allowed_origin
# Make sure the session is closed
session.close()

return result

Expand All @@ -156,7 +161,7 @@ def verify_token():
# pylint: disable=missing-docstring
def decorator(coro):
@wraps(coro)
async def wrapper(endpoint, request, *args, **kwargs):
async def wrapper(endpoint, session, request, *args, **kwargs):
if not request.token:
logger.error('No token provided in request')
return response.json({'error': 'Unauthorized'}, status=401)
Expand All @@ -169,7 +174,7 @@ async def wrapper(endpoint, request, *args, **kwargs):

# Call the request handler
try:
return await coro(endpoint, request, *args, **kwargs)
return await coro(endpoint, session, request, *args, **kwargs)
except APIError as err:
return response.json({'error': err.message}, status=err.status)
except Exception:
Expand Down
4 changes: 2 additions & 2 deletions bounce/server/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ class LoginEndpoint(Endpoint):
__uri__ = '/auth/login'

@validate(AuthenticateUserRequest, AuthenticateUserResponse)
async def post(self, request):
async def post(self, session, request):
"""Handles a POST /auth/login request by validating the user's
credentials and issuing them a JSON Web Token."""
body = request.json

# Fetch the user's info from the DB
user_row = user.select(self.server.db_session, body['username'])
user_row = user.select(session, body['username'])
if not user_row:
raise APIError('Unauthorized', status=401)

Expand Down
30 changes: 15 additions & 15 deletions bounce/server/api/clubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,27 @@ class ClubEndpoint(Endpoint):
__uri__ = "/clubs/<name:string>"

@validate(None, GetClubResponse)
async def get(self, _, name):
async def get(self, session, _, name):
"""Handles a GET /clubs/<name> request by returning the club with
the given name."""
# Decode the name, since special characters will be URL-encoded
name = unquote(name)
# Fetch club data from DB
club_data = club.select(self.server.db_session, name)
club_data = club.select(session, name)
if not club_data:
# Failed to find a club with that name
raise APIError('No such club', status=404)
return response.json(club_data, status=200)

@validate(PutClubRequest, GetClubResponse)
async def put(self, request, name):
async def put(self, session, request, name):
"""Handles a PUT /clubs/<name> request by updating the club with
the given name and returning the updated club info."""
# Decode the name, since special characters will be URL-encoded
name = unquote(name)
body = util.strip_whitespace(request.json)
updated_club = club.update(
self.server.db_session,
session,
name,
new_name=body.get('name', None),
description=body.get('description', None),
Expand All @@ -51,12 +51,12 @@ async def put(self, request, name):
twitter_url=body.get('twitter_url', None))
return response.json(updated_club, status=200)

async def delete(self, _, name):
async def delete(self, session, _, name):
"""Handles a DELETE /clubs/<name> request by deleting the club with
the given name. """
# Decode the name, since special characters will be URL-encoded
name = unquote(name)
club.delete(self.server.db_session, name)
club.delete(session, name)
return response.text('', status=204)


Expand All @@ -66,13 +66,13 @@ class ClubsEndpoint(Endpoint):
__uri__ = '/clubs'

@validate(PostClubsRequest, None)
async def post(self, request):
async def post(self, session, request):
"""Handles a POST /clubs request by creating a new club."""
# Put the club in the DB
body = util.strip_whitespace(request.json)
try:
club.insert(
self.server.db_session, body['name'].strip(),
session, body['name'].strip(),
body['description'].strip(), body['website_url'].strip(),
body['facebook_url'].strip(), body['instagram_url'].strip(),
body['twitter_url'].strip())
Expand All @@ -87,7 +87,7 @@ class SearchClubsEndpoint(Endpoint):
__uri__ = '/clubs/search'

@validate(SearchClubsRequest, SearchClubsResponse)
async def get(self, request):
async def get(self, session, request):
"""Handles a GET /club/search request by returning
clubs that contain content from the query."""

Expand All @@ -102,7 +102,7 @@ async def get(self, request):
raise APIError('size too low', status=400)

queried_clubs, result_count, total_pages = club.search(
self.server.db_session, query, page, size)
session, query, page, size)
if not queried_clubs:
# Failed to find clubs that match the query
raise APIError('No clubs match your query', status=404)
Expand All @@ -124,7 +124,7 @@ class ClubImagesEndpoint(Endpoint):

__uri__ = '/clubs/<club_name>/images/<image_name>'

async def get(self, _, club_name, image_name):
async def get(self, session, _, club_name, image_name):
"""
Handles a GET /clubs/<club_name>/images/<image_name> request
by returning the club's image with the given name.
Expand All @@ -140,7 +140,7 @@ async def get(self, _, club_name, image_name):
raise APIError('No such image', status=404)

# @verify_token()
async def put(self, request, club_name, image_name):
async def put(self, session, request, club_name, image_name):
"""
Handles a PUT /clubs/<club_name>/images/<image_name> request
by updating the image at the given path.
Expand All @@ -150,7 +150,7 @@ async def put(self, request, club_name, image_name):
raise APIError('Invalid image name', status=400)

# Make sure the user is updating an image they own
club_info = club.select(self.server.db_session, club_name)
club_info = club.select(session, club_name)
if not club_info:
raise APIError('No such image', status=404)

Expand All @@ -171,13 +171,13 @@ async def put(self, request, club_name, image_name):
return response.text('', status=200)

# @verify_token()
async def delete(self, _, club_name, image_name):
async def delete(self, session, _, club_name, image_name):
"""Handles a DETELE by deleting the club's image by the given name."""

if not util.check_image_name(image_name):
raise APIError('Invalid image name', status=400)
# Make sure the user is deleting their own image
club_info = club.select(self.server.db_session, club_name)
club_info = club.select(session, club_name)
if not club_info:
raise APIError('No such image', status=404)
try:
Expand Down
16 changes: 8 additions & 8 deletions bounce/server/api/membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class MembershipEndpoint(Endpoint):
__uri__ = "/memberships/<club_name:string>"

@validate(GetMembershipsRequest, GetMembershipsResponse)
async def get(self, request, club_name):
async def get(self, session, request, club_name):
"""
Handles a GET /memberships/<club_name>?user_id=<user_id> request
by returning the membership that associates the given user with the
Expand All @@ -31,20 +31,20 @@ async def get(self, request, club_name):
user_id = request.args.get('user_id', None)

# Make sure the club exists
club_row = club.select(self.server.db_session, club_name)
club_row = club.select(session, club_name)
if not club_row:
raise APIError('No such club', status=404)

# Fetch the club's memberships
results = membership.select(
self.server.db_session, club_name, user_id=user_id)
session, club_name, user_id=user_id)
info = {'results': results}
return response.json(info, status=200)

# pylint: disable=unused-argument
@verify_token()
@validate(PutMembershipRequest, None)
async def put(self, request, club_name, id_from_token=None):
async def put(self, session, request, club_name, id_from_token=None):
"""Handles a PUT /memberships/<club_name>?user_id=<user_id> request by
creating or updating the membership for the given user and club."""
# Decode the club name
Expand All @@ -54,7 +54,7 @@ async def put(self, request, club_name, id_from_token=None):
except Exception:
raise APIError('Invalid user ID', status=400)
try:
membership.insert(self.server.db_session, club_name, user_id)
membership.insert(session, club_name, user_id)
except IntegrityError:
raise APIError('Invalid user or club ID', status=400)
return response.text('', status=201)
Expand All @@ -63,7 +63,7 @@ async def put(self, request, club_name, id_from_token=None):

@verify_token()
@validate(DeleteMembershipRequest, None)
async def delete(self, request, club_name, id_from_token=None):
async def delete(self, session, request, club_name, id_from_token=None):
"""
Handles a DELETE /memberships/<club_name>?user_id=<user_id> request
by deleting the membership that associates the given user with the
Expand All @@ -85,10 +85,10 @@ async def delete(self, request, club_name, id_from_token=None):
raise APIError('Forbidden', status=403)

# Make sure the club exists
club_row = club.select(self.server.db_session, club_name)
club_row = club.select(session, club_name)
if not club_row:
raise APIError('No such club', status=404)

# Delete the memberships
membership.delete(self.server.db_session, club_name, user_id=user_id)
membership.delete(session, club_name, user_id=user_id)
return response.text('', status=204)
34 changes: 17 additions & 17 deletions bounce/server/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,26 @@ class UserEndpoint(Endpoint):
__uri__ = "/users/<username:string>"

@validate(None, GetUserResponse)
async def get(self, _, username):
async def get(self, session, _, username):
"""Handles a GET /users/<username> request by returning the user with
the given membership."""
# Fetch user data from DB
user_row = user.select(self.server.db_session, username)
user_row = user.select(session, username)
if not user_row:
# Failed to find a user with that username
raise APIError('No such user', status=404)
return response.json(user_row.to_dict(), status=200)

@verify_token()
@validate(PutUserRequest, GetUserResponse)
async def put(self, request, username, id_from_token=None):
async def put(self, session, request, username, id_from_token=None):
"""Handles a PUT /users/<username> request by updating the user with
the given username and returning the updated user info. """
body = util.strip_whitespace(request.json)
secret = None
email = None
# Make sure the ID from the token is for the user we're updating
user_row = user.select(self.server.db_session, username)
user_row = user.select(session, username)
if not user_row:
raise APIError('No such user', status=404)
if user_row.identifier != id_from_token:
Expand Down Expand Up @@ -74,7 +74,7 @@ async def put(self, request, username, id_from_token=None):
secret = util.hash_password(body['new_password'])
# Update the user
updated_user = user.update(
self.server.db_session,
session,
username,
secret=secret,
full_name=body.get('full_name', None),
Expand All @@ -83,17 +83,17 @@ async def put(self, request, username, id_from_token=None):
return response.json(updated_user.to_dict(), status=200)

@verify_token()
async def delete(self, _, username, id_from_token=None):
async def delete(self, session, _, username, id_from_token=None):
"""Handles a DELETE /users/<username> request by deleting the user with
the given username. """
# Make sure the ID from the token is for the user we're deleting
user_row = user.select(self.server.db_session, username)
user_row = user.select(session, username)
if not user_row:
raise APIError('No such user', status=404)
elif user_row.identifier != id_from_token:
raise APIError('Forbidden', status=403)
# Delete the user
user.delete(self.server.db_session, username)
user.delete(session, username)
# Delete the user's images
image.delete_dir(self.server.config.image_dir, EntityType.USER,
user_row.identifier)
Expand All @@ -106,7 +106,7 @@ class UsersEndpoint(Endpoint):
__uri__ = '/users'

@validate(PostUsersRequest, None)
async def post(self, request):
async def post(self, session, request):
"""Handles a POST /users request by creating a new user."""
body = util.strip_whitespace(request.json)
# Make sure the username is valid
Expand All @@ -121,7 +121,7 @@ async def post(self, request):
secret = util.hash_password(body['password'])
# Put the user in the DB
try:
user.insert(self.server.db_session, body['full_name'],
user.insert(session, body['full_name'],
body['username'], secret, body['email'])
except IntegrityError:
raise APIError('User already exists', status=409)
Expand All @@ -133,7 +133,7 @@ class UserImagesEndpoint(Endpoint):

__uri__ = '/users/<user_id>/images/<image_name>'

async def get(self, _, user_id, image_name):
async def get(self, session, _, user_id, image_name):
"""
Handles a GET /users/<user_id>/images/<image_name> request
by returning the user's image with the given name.
Expand All @@ -148,7 +148,7 @@ async def get(self, _, user_id, image_name):
raise APIError('No such image', status=404)

@verify_token()
async def put(self, request, user_id, image_name, id_from_token=None):
async def put(self, session, request, user_id, image_name, id_from_token=None):
"""
Handles a PUT /users/<user_id>/images/<image_name> request
by updating the image at the given path.
Expand All @@ -158,7 +158,7 @@ async def put(self, request, user_id, image_name, id_from_token=None):
raise APIError('Invalid image name', status=400)

# Make sure the user is updating an image they own
user_info = user.select_by_id(self.server.db_session, user_id)
user_info = user.select_by_id(session, user_id)
if not user_info:
raise APIError('No such image', status=404)
if not user_info.identifier == id_from_token:
Expand All @@ -183,12 +183,12 @@ async def put(self, request, user_id, image_name, id_from_token=None):
return response.text('', status=200)

@verify_token()
async def delete(self, _, user_id, image_name, id_from_token=None):
async def delete(self, session, _, user_id, image_name, id_from_token=None):
"""Handles a DETELE by deleting the user's image by the given name."""
if not util.check_image_name(image_name):
raise APIError('Invalid image name', status=400)
# Make sure the user is deleting their own image
user_info = user.select_by_id(self.server.db_session, user_id)
user_info = user.select_by_id(session, user_id)
if not user_info:
raise APIError('No such image', status=404)
if not user_info.identifier == id_from_token:
Expand All @@ -211,7 +211,7 @@ class SearchUsersEndpoint(Endpoint):
__uri__ = '/users/search'

@validate(SearchUsersRequest, SearchUsersResponse)
async def get(self, request):
async def get(self, session, request):
"""Handles a GET /club/search request by returning
users that contain content from the query."""

Expand All @@ -226,7 +226,7 @@ async def get(self, request):
raise APIError('size too low', status=400)

queried_users, result_count, total_pages = user.search(
self.server.db_session, query, page, size)
session, query, page, size)
if not queried_users:
# Failed to find users that match the query
raise APIError('No users match your query', status=404)
Expand Down
Loading

0 comments on commit 5d00a6e

Please sign in to comment.