Fix unclosed SQLAlchemy sessions (#131)

bounce/server/api/__init__.py

@@ -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
@@ -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
 
@@ -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)
@@ -168,7 +173,7 @@ async def wrapper(endpoint, request, *args, **kwargs):
             kwargs['id_from_token'] = user_id
 
             # Call the request handler
-            return await coro(endpoint, request, *args, **kwargs)
+            return await coro(endpoint, session, request, *args, **kwargs)
 
         return wrapper
 

bounce/server/api/auth.py

@@ -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)
 

bounce/server/api/clubs.py

@@ -22,33 +22,32 @@ 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)
 
     @verify_token()
     @validate(PutClubRequest, GetClubResponse)
-    async def put(self, request, name, id_from_token=None):
+    async def put(self, session, request, name, id_from_token=None):
         """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)
         try:
-            editor_attr = membership.select(self.server.db_session, name,
-                                            id_from_token,
+            editor_attr = membership.select(session, name, id_from_token,
                                             Roles.president.value)
             editors_role = editor_attr[0]['role']
             updated_club = club.update(
-                self.server.db_session,
+                session,
                 name,
                 editors_role,
                 new_name=body.get('name', None),
@@ -63,18 +62,17 @@ async def put(self, request, name, id_from_token=None):
 
     @verify_token()
     @validate(DeleteClubRequest, None)
-    async def delete(self, _, name, id_from_token=None):
+    async def delete(self, session, _, name, id_from_token=None):
         """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)
         try:
-            membership_attr = membership.select(self.server.db_session, name,
-                                                id_from_token,
+            membership_attr = membership.select(session, name, id_from_token,
                                                 Roles.president.value)
             editors_role = membership_attr[0]['role']
-            club.delete(self.server.db_session, name, editors_role)
+            club.delete(session, name, editors_role)
         except PermissionError:
             raise APIError('Forbidden', status=403)
         return response.text('', status=204)
@@ -87,13 +85,13 @@ class ClubsEndpoint(Endpoint):
 
     @verify_token()
     @validate(PostClubsRequest, None)
-    async def post(self, request, id_from_token=None):
+    async def post(self, session, request, id_from_token=None):
         """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,
+                session,
                 name=body.get('name', None),
                 description=body.get('description', None),
                 website_url=body.get('website_url', None),
@@ -103,9 +101,9 @@ async def post(self, request, id_from_token=None):
         except IntegrityError:
             raise APIError('Club already exists', status=409)
         # Give the creator of the club a President membership
-        membership.insert(self.server.db_session, body.get('name', None),
-                          id_from_token, Roles.president.value,
-                          Roles.president.value, 'Owner')
+        membership.insert(session, body.get('name', None), id_from_token,
+                          Roles.president.value, Roles.president.value,
+                          'Owner')
         return response.text('', status=201)
 
 
@@ -115,7 +113,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."""
 
@@ -130,7 +128,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)
@@ -152,7 +150,8 @@ class ClubImagesEndpoint(Endpoint):
 
     __uri__ = '/clubs/<name>/images/<image_name>'
 
-    async def get(self, _, name, image_name):
+    # pylint: disable=unused-argument
+    async def get(self, session, _, name, image_name):
         """
         Handles a GET /clubs/<name>/images/<image_name> request
         by returning the club's image with the given name.
@@ -167,8 +166,10 @@ async def get(self, _, name, image_name):
         except FileNotFoundError:
             raise APIError('No such image', status=404)
 
+    # pylint: enable=unused-argument
+
     # @verify_token()
-    async def put(self, request, name, image_name):
+    async def put(self, session, request, name, image_name):
         """
         Handles a PUT /clubs/<name>/images/<image_name> request
         by updating the image at the given path.
@@ -178,7 +179,7 @@ async def put(self, request, 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, name)
+        club_info = club.select(session, name)
         if not club_info:
             raise APIError('No such image', status=404)
 
@@ -199,13 +200,13 @@ async def put(self, request, name, image_name):
         return response.text('', status=200)
 
     # @verify_token()
-    async def delete(self, _, name, image_name):
+    async def delete(self, session, _, 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, name)
+        club_info = club.select(session, name)
         if not club_info:
             raise APIError('No such image', status=404)
         try:

bounce/server/api/membership.py

@@ -20,7 +20,7 @@ class MembershipEndpoint(Endpoint):
 
     @verify_token()
     @validate(GetMembershipsRequest, GetMembershipsResponse)
-    async def get(self, request, club_name, id_from_token=None):
+    async def get(self, session, request, club_name, id_from_token=None):
         """
         Handles a GET /memberships/<club_name> request
         by returning the membership that associates the given user with the
@@ -32,7 +32,7 @@ async def get(self, request, club_name, id_from_token=None):
         club_name = unquote(club_name)
 
         # 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)
         try:
@@ -57,8 +57,8 @@ async def get(self, request, club_name, id_from_token=None):
 
     @verify_token()
     @validate(PutMembershipRequest, None)
-    async def put(self, request, club_name, id_from_token=None):
-        """Handles a PUT /memberships/<club_name>
+    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
         club_name = unquote(club_name)
@@ -75,39 +75,36 @@ async def put(self, request, club_name, id_from_token=None):
         # get the editors role using id_from_token
         # to see if the editor has access to insert/update
         # the memberships table.
-        editor_attr = membership.select(self.server.db_session, club_name,
-                                        id_from_token, Roles.president)
+        editor_attr = membership.select(session, club_name, id_from_token,
+                                        Roles.president)
         if not editor_attr:
             # Either the club doesn't exist, or the user is not a member of
             # the club
             raise APIError('Bad request', status=400)
 
         editors_role = editor_attr[0]['role']
-        membership_attr = membership.select(self.server.db_session, club_name,
-                                            user_id, Roles.president)
+        membership_attr = membership.select(session, club_name, user_id,
+                                            Roles.president)
 
         try:
             # If the membership exists already in the table, update entry
             if membership_attr:
                 current_members_role = membership_attr[0]['role']
-                membership.update(self.server.db_session, club_name, user_id,
-                                  editors_role, current_members_role, position,
-                                  members_role)
+                membership.update(session, club_name, user_id, editors_role,
+                                  current_members_role, position, members_role)
             # Otherwise, insert new entry
             else:
-                membership.insert(self.server.db_session, club_name, user_id,
-                                  editors_role, members_role, position)
+                membership.insert(session, club_name, user_id, editors_role,
+                                  members_role, position)
         except PermissionError:
             raise APIError('Forbidden', status=403)
         except IntegrityError:
             raise APIError('Bad request', status=400)
         return response.text('', status=201)
 
-    # pylint: enable=unused-argument
-
     @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> request
         by deleting the membership that associates the given user with the
@@ -118,31 +115,28 @@ async def delete(self, request, club_name, id_from_token=None):
         club_name = unquote(club_name)
 
         # 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)
 
         try:
-            editor_attr = membership.select(self.server.db_session, club_name,
-                                            id_from_token, Roles.president)
+            editor_attr = membership.select(session, club_name, id_from_token,
+                                            Roles.president)
 
             editors_role = editor_attr[0]['role']
 
             if 'user_id' in request.args:
                 user_id = int(request.args['user_id'])
-                member_attr = membership.select(self.server.db_session,
-                                                club_name, user_id,
+                member_attr = membership.select(session, club_name, user_id,
                                                 Roles.president)
                 if not member_attr:
                     raise APIError('Bad request', status=400)
 
                 members_role = member_attr[0]['role']
-                membership.delete(self.server.db_session, club_name,
-                                  id_from_token, user_id, editors_role,
-                                  members_role)
+                membership.delete(session, club_name, id_from_token, user_id,
+                                  editors_role, members_role)
             else:
-                membership.delete_all(self.server.db_session, club_name,
-                                      editors_role)
+                membership.delete_all(session, club_name, editors_role)
         except PermissionError:
             raise APIError('Forbidden', status=403)
         return response.text('', status=201)