From b7b46514a2900c337f0775ad9f3a4a7afe57b1e6 Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Wed, 29 Oct 2025 11:56:07 +0100 Subject: [PATCH 1/6] Extended scope for admin and added/extended endpoints --- grader_service/auth/auth.py | 4 +- grader_service/handlers/__init__.py | 4 + grader_service/handlers/assignment.py | 109 ++++++++------ grader_service/handlers/base_handler.py | 65 +++++---- grader_service/handlers/lectures.py | 112 +++++++++------ grader_service/handlers/roles.py | 137 ++++++++++++++++++ grader_service/handlers/submissions.py | 180 ++++++++++++++++++------ grader_service/handlers/users.py | 108 ++++++++++++++ grader_service/orm/submission.py | 10 ++ grader_service/orm/takepart.py | 3 + 10 files changed, 584 insertions(+), 148 deletions(-) create mode 100644 grader_service/handlers/roles.py create mode 100644 grader_service/handlers/users.py diff --git a/grader_service/auth/auth.py b/grader_service/auth/auth.py index 61b04c449..3824aa937 100644 --- a/grader_service/auth/auth.py +++ b/grader_service/auth/auth.py @@ -10,6 +10,7 @@ from grader_service.utils import maybe_future, url_path_join from .login import LoginHandler +from ..handlers.base_handler import BaseHandler class Authenticator(LoggingConfigurable): @@ -511,7 +512,7 @@ async def get_authenticated_user(self, handler, data): self.log.warning("User %r not allowed.", username) return - async def refresh_user(self, user, handler=None): + async def refresh_user(self, user, handler:BaseHandler=None): """Refresh auth data for a given user Allows refreshing or invalidating auth data. @@ -537,6 +538,7 @@ async def refresh_user(self, user, handler=None): Any fields not present will be left unchanged. This can include updating `.admin` or `.auth_state` fields. """ + user.is_admin = handler.authenticator.is_admin(handler=self, authentication={'name': user.name}) return True def is_admin(self, handler, authentication): diff --git a/grader_service/handlers/__init__.py b/grader_service/handlers/__init__.py index f79e883e2..46cb93ea3 100644 --- a/grader_service/handlers/__init__.py +++ b/grader_service/handlers/__init__.py @@ -15,6 +15,8 @@ lectures, permission, submissions, + roles, + users ) from grader_service.handlers.handler_utils import GitRepoType @@ -29,4 +31,6 @@ "config", "base_handler", "GitRepoType", + "roles", + "users", ] diff --git a/grader_service/handlers/assignment.py b/grader_service/handlers/assignment.py index eff865098..1063bfde3 100644 --- a/grader_service/handlers/assignment.py +++ b/grader_service/handlers/assignment.py @@ -63,7 +63,7 @@ class AssignmentBaseHandler(GraderBaseHandler): route: /lectures/{lecture_id}/assignments.""" - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int): """Returns all assignments of a lecture. @@ -73,35 +73,44 @@ async def get(self, lecture_id: int): """ lecture_id = parse_ids(lecture_id) self.validate_parameters("include-submissions") - role = self.get_role(lecture_id) - if role.lecture.deleted == DeleteState.deleted: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") - if role.role == Scope.student: # students do not get assignments that are created - assignments = ( - self.session.query(Assignment) - .filter( - Assignment.lectid == role.lecture.id, - Assignment.deleted == DeleteState.active, - Assignment.status != "created", - Assignment.status != "pushed", + if not self.user.is_admin: + role = self.get_role(lecture_id) + if role.lecture.deleted == DeleteState.deleted: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") + + if role.role == Scope.student: # students do not get assignments that are created + assignments = ( + self.session.query(Assignment) + .filter( + Assignment.lectid == role.lecture.id, + Assignment.deleted == DeleteState.active, + Assignment.status != "created", + Assignment.status != "pushed", + ) + .all() ) - .all() - ) - + else: + assignments = [a for a in role.lecture.assignments if (a.deleted == DeleteState.active)] else: - assignments = [a for a in role.lecture.assignments if (a.deleted == DeleteState.active)] + lecture = self.get_lecture(lecture_id=lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") + assignments = lecture.assignments # Handle the case that the user wants to include submissions include_submissions = self.get_argument("include-submissions", "true") == "true" if include_submissions: assignids = [a.id for a in assignments] - user_id = self.user.id - results = ( - self.session.query(Submission) - .filter(Submission.assignid.in_(assignids), Submission.user_id == user_id) - .all() - ) + if not self.user.is_admin: + user_id = self.user.id + results = ( + self.session.query(Submission) + .filter(Submission.assignid.in_(assignids), Submission.user_id == user_id) + .all() + ) + else: + results = self.session.query(Submission).filter(Submission.assignid.in_(assignids)).all() # Create a combined list of assignments and submissions assignments = [a.serialize() for a in assignments] submissions = [s.serialize() for s in results] @@ -242,7 +251,7 @@ async def put(self, lecture_id: int, assignment_id: int): self.session.commit() self.write_json(assignment) - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int, assignment_id: int): """Returns a specific assignment of a lecture. @@ -256,9 +265,9 @@ async def get(self, lecture_id: int, assignment_id: int): assignment = self.get_assignment(lecture_id=lecture_id, assignment_id=assignment_id) self.write_json(assignment) - @authorize([Scope.instructor]) + @authorize([Scope.instructor, Scope.admin]) async def delete(self, lecture_id: int, assignment_id: int): - """Soft-Deletes a specific assignment. + """Soft or Hard-Deletes a specific assignment. :param lecture_id: id of the lecture :type lecture_id: int @@ -270,26 +279,46 @@ async def delete(self, lecture_id: int, assignment_id: int): self.validate_parameters() assignment = self.get_assignment(lecture_id, assignment_id) - if assignment.status in ["released", "complete"]: - msg = "Cannot delete released or completed assignments!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + if not self.user.is_admin: + if assignment.status in ["released", "complete"]: + msg = "Cannot delete released or completed assignments!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - previously_deleted = ( - self.session.query(Assignment) - .filter( - Assignment.lectid == lecture_id, - Assignment.name == assignment.name, - Assignment.deleted == DeleteState.deleted, + previously_deleted = ( + self.session.query(Assignment) + .filter( + Assignment.lectid == lecture_id, + Assignment.name == assignment.name, + Assignment.deleted == DeleteState.deleted, + ) + .one_or_none() ) - .one_or_none() - ) - if previously_deleted is not None: - self.session.delete(previously_deleted) + if previously_deleted is not None: + if len(previously_deleted.submissions) > 0: + msg = "Assignment cannot be deleted: submissions still exist. Delete submissions first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + + self.delete_assignment_files(previously_deleted) + self.session.delete(previously_deleted) + self.session.commit() + + assignment.deleted = DeleteState.deleted self.session.commit() + else: + if len(assignment.submissions) > 0: + msg = "Assignment cannot be deleted: submissions still exist. Delete submissions first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - assignment.deleted = DeleteState.deleted - self.session.commit() + self.delete_assignment_files(assignment) + self.session.delete(assignment) + self.session.commit() + def delete_assignment_files(self, assignment): + # delete all associated directories of the assignment + assignment_path = os.path.abspath(os.path.join(self.gitbase, assignment.lecture.code, str(assignment.id))) + tmp_assignment_path = os.path.abspath(os.path.join(self.tmpbase, assignment.lecture.code, str(assignment.id))) + shutil.rmtree(assignment_path, ignore_errors=True) + shutil.rmtree(tmp_assignment_path, ignore_errors=True) @register_handler( path=r"\/api\/lectures\/(?P\d*)\/assignments\/(?P\d*)\/reset\/?", diff --git a/grader_service/handlers/base_handler.py b/grader_service/handlers/base_handler.py index 4d2810fdd..82f7ca308 100644 --- a/grader_service/handlers/base_handler.py +++ b/grader_service/handlers/base_handler.py @@ -24,7 +24,7 @@ from sqlalchemy import func from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm import joinedload -from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound +from sqlalchemy.orm.exc import MultipleResultsFound from sqlalchemy.orm.session import Session from tornado import httputil, web from tornado.escape import json_decode @@ -59,22 +59,23 @@ def check_authorization( if ("/permissions" in self.request.path) or ("/config" in self.request.path): return True if lecture_id is None and "/lectures" in self.request.path and self.request.method == "POST": - # lecture name and semester is in post body + # lecture code is in post body try: data = json_decode(self.request.body) - lecture_id = self.session.query(Lecture).filter(Lecture.code == data["code"]).one().id + lecture = self.session.query(Lecture).filter(Lecture.code == data["code"]).one_or_none() + lecture_id = lecture.id if lecture else None except MultipleResultsFound: raise HTTPError(403) - except NoResultFound: - raise HTTPError(404, reason="Lecture not found") except json.decoder.JSONDecodeError: raise HTTPError(403) elif lecture_id is None and "/lectures" in self.request.path and self.request.method == "GET": return True + is_admin = self.authenticator.is_admin(handler=self, authentication={'name': self.user.name}) + role = self.session.get(Role, (self.user.id, lecture_id)) - if (role is None) or (role.role not in scopes): + if not ((is_admin and Scope.admin in scopes) or (role is not None and role.role in scopes)): self.log.warning( "User %s tried to access %s with insufficient privileges", self.user.name, @@ -89,7 +90,7 @@ def authorize(scopes: list[Scope]): :param scopes: the user's roles :return: wrapper function """ - if not set(scopes).issubset({Scope.student, Scope.tutor, Scope.instructor}): + if not set(scopes).issubset({Scope.student, Scope.tutor, Scope.instructor, Scope.admin}): return ValueError("Invalid scopes") def wrapper(handler_method): @@ -772,20 +773,20 @@ def write_error(self, status_code, **kwargs): return super().write_error(status_code, **kwargs) def get_role(self, lecture_id: int) -> Role: - role = self.session.get(Role, (self.user.id, lecture_id)) + role: Optional[Role] = self.session.get(Role, (self.user.id, lecture_id)) if role is None: raise HTTPError(403) return role def get_lecture(self, lecture_id: int) -> Lecture: - lecture: Lecture = self.session.get(Lecture, lecture_id) + lecture: Optional[Lecture] = self.session.get(Lecture, lecture_id) return lecture def get_assignment(self, lecture_id: int, assignment_id: int) -> Assignment: assignment: Optional[Assignment] = self.session.get(Assignment, assignment_id) if ( (assignment is None) - or (assignment.deleted == DeleteState.deleted) + or ((assignment.deleted == DeleteState.deleted) and (not self.user.is_admin)) or (int(assignment.lectid) != int(lecture_id)) ): msg = "Assignment with id " + str(assignment_id) + " was not found" @@ -793,12 +794,12 @@ def get_assignment(self, lecture_id: int, assignment_id: int) -> Assignment: return assignment def get_submission(self, lecture_id: int, assignment_id: int, submission_id: int) -> Submission: - submission = self.session.get(Submission, submission_id) + submission: Optional[Submission] = self.session.get(Submission, submission_id) if ( (submission is None) or (submission.assignid != assignment_id) or (int(submission.assignment.lectid) != lecture_id) - or (submission.deleted == DeleteState.deleted) + or ((submission.deleted == DeleteState.deleted) and (not self.user.is_admin)) ): msg = f"Submission with id {submission_id} was not found" raise HTTPError(HTTPStatus.NOT_FOUND, reason=msg) @@ -810,10 +811,12 @@ def get_latest_submissions( query = ( self.session.query(Submission.user_id, func.max(Submission.date).label("max_date")) .filter(Submission.assignid == assignment_id) - .filter(Submission.deleted == DeleteState.active) .group_by(Submission.user_id) ) + if not self.user.is_admin: + query = query.filter(Submission.deleted == DeleteState.active) + if must_have_feedback: query = query.filter(Submission.feedback_status != FeedbackStatus.NOT_GENERATED) @@ -823,7 +826,7 @@ def get_latest_submissions( subquery = query.subquery() # Build the main query - submissions = ( + submissions_query = ( self.session.query(Submission) .options(joinedload(Submission.user)) .join( @@ -831,21 +834,25 @@ def get_latest_submissions( (Submission.user_id == subquery.c.user_id) & (Submission.date == subquery.c.max_date) & (Submission.assignid == assignment_id) - & (Submission.deleted == DeleteState.active), ) .order_by(Submission.id) - .all() ) - return submissions + if not self.user.is_admin: + submissions_query = submissions_query.filter(Submission.deleted == DeleteState.active) + + return submissions_query.all() def get_all_submissions(self, assignment_id) -> List[Submission]: query = ( self.session.query(Submission) .options(joinedload(Submission.user)) .filter(Submission.assignid == assignment_id) - .filter(Submission.deleted == DeleteState.active) ) + + if not self.user.is_admin: + query = query.filter(Submission.deleted == DeleteState.active) + return query.all() def get_best_submissions( @@ -854,10 +861,12 @@ def get_best_submissions( query = ( self.session.query(Submission.user_id, func.max(Submission.score).label("max_score")) .filter(Submission.assignid == assignment_id) - .filter(Submission.deleted == DeleteState.active) .group_by(Submission.user_id) ) + if not self.user.is_admin: + query = query.filter(Submission.deleted == DeleteState.active) + if must_have_feedback: query = query.filter(Submission.feedback_status != FeedbackStatus.NOT_GENERATED) @@ -867,7 +876,7 @@ def get_best_submissions( subquery = query.subquery() # Build the main query - submissions = ( + submissions_query = ( self.session.query(Submission) .options(joinedload(Submission.user)) .join( @@ -875,18 +884,25 @@ def get_best_submissions( (Submission.user_id == subquery.c.user_id) & (Submission.score == subquery.c.max_score) & (Submission.assignid == assignment_id) - & (Submission.deleted == DeleteState.active), ) .order_by(Submission.id) - .all() ) - return submissions + + if not self.user.is_admin: + submissions_query = submissions_query.filter(Submission.deleted == DeleteState.active) + + return submissions_query.all() @property def gitbase(self): app: GraderServer = self.application return os.path.join(app.grader_service_dir, "git") + @property + def tmpbase(self): + app: GraderServer = self.application + return os.path.join(app.grader_service_dir, "tmp") + def construct_git_dir( self, repo_type: GitRepoType, @@ -911,11 +927,10 @@ def construct_git_dir( path = os.path.join(assignment_path, repo_type) if repo_type == GitRepoType.EDIT: path = os.path.join(path, str(submission.id)) - self.log.info(path) elif repo_type in {GitRepoType.AUTOGRADE, GitRepoType.FEEDBACK}: type_path = os.path.join(assignment_path, repo_type, "user") if repo_type == GitRepoType.AUTOGRADE: - if (submission is None) or (self.get_role(lecture.id).role < Scope.tutor): + if (submission is None) or (not self.user.is_admin and self.get_role(lecture.id).role < Scope.tutor): raise HTTPError(403) path = os.path.join(type_path, submission.user.name) else: diff --git a/grader_service/handlers/lectures.py b/grader_service/handlers/lectures.py index 8becd291b..b289d6e28 100644 --- a/grader_service/handlers/lectures.py +++ b/grader_service/handlers/lectures.py @@ -3,6 +3,8 @@ # # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. +import os +import shutil from http import HTTPStatus import tornado @@ -25,7 +27,7 @@ class LectureBaseHandler(GraderBaseHandler): Tornado Handler class for http requests to /lectures. """ - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self): """ Returns all lectures the user can access. @@ -45,22 +47,23 @@ async def get(self): self.write_json(lectures) - @authorize([Scope.instructor]) + @authorize([Scope.instructor, Scope.admin]) async def post(self): """ - Creates a new lecture from a "ghost"-lecture. - - :raises HTTPError: throws err if "ghost"-lecture was not found + Creates a new lecture or updates an existing one. """ self.validate_parameters() body = tornado.escape.json_decode(self.request.body) lecture_model = LectureModel.from_dict(body) + created = False lecture = ( self.session.query(Lecture).filter(Lecture.code == lecture_model.code).one_or_none() ) if lecture is None: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture template not found") + lecture = Lecture() + self.session.add(lecture) + created = True lecture.name = lecture_model.name lecture.code = lecture_model.code @@ -68,7 +71,10 @@ async def post(self): lecture.deleted = DeleteState.active self.session.commit() - self.set_status(HTTPStatus.CREATED) + if created: + self.set_status(HTTPStatus.CREATED) + else: + self.set_status(HTTPStatus.OK) self.write_json(lecture) @@ -78,7 +84,7 @@ class LectureObjectHandler(GraderBaseHandler): Tornado Handler class for http requests to /lectures/{lecture_id}. """ - @authorize([Scope.instructor]) + @authorize([Scope.instructor, Scope.admin]) async def put(self, lecture_id: int): """ Updates a lecture. @@ -97,7 +103,7 @@ async def put(self, lecture_id: int): self.session.commit() self.write_json(lecture) - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int): """ Finds lecture with the given lecture id. @@ -105,18 +111,24 @@ async def get(self, lecture_id: int): :return: lecture with given id """ self.validate_parameters() - role = self.get_role(lecture_id) - if role.lecture.deleted == DeleteState.deleted: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") - - self.write_json(role.lecture) + if not self.user.is_admin: + role = self.get_role(lecture_id) + lecture = role.lecture + if lecture.deleted == DeleteState.deleted: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") + else: + lecture = self.get_lecture(lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") + self.write_json(lecture) - @authorize([Scope.instructor]) + @authorize([Scope.admin]) async def delete(self, lecture_id: int): """ - "Soft"-delete a lecture. - Softdeleting: lecture is still saved in the datastore - but the users have not access to it. + Soft or Hard-Deletes a specific lecture. + Soft deleting: lecture is still saved in the datastore + but the users have no access to it. + Hard deleting: removes lecture from the datastore and all associated directories/files. :param lecture_id: id of the lecture :type lecture_id: int @@ -124,28 +136,48 @@ async def delete(self, lecture_id: int): or was not found """ - self.validate_parameters() + self.validate_parameters("hard_delete") + hard_delete = self.get_argument("hard_delete", "false") == "true" + try: - lecture = self.session.get(Lecture, lecture_id) - if lecture.deleted == 1: - raise HTTPError(404) - lecture.deleted = 1 - a: Assignment - for a in lecture.assignments: - if (len(a.submissions)) > 0: - self.session.rollback() - raise HTTPError( - HTTPStatus.CONFLICT, "Cannot delete assignment because it has submissions" - ) - if a.status in ["released", "complete"]: - self.session.rollback() - raise HTTPError( - HTTPStatus.CONFLICT, - "Cannot delete assignment because its status is not created", - ) - - a.deleted = 1 - self.session.commit() + lecture = self.get_lecture(lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") + if not hard_delete: + if lecture.deleted == 1: + raise HTTPError(404) + lecture.deleted = 1 + a: Assignment + for a in lecture.assignments: + if (len(a.submissions)) > 0: + self.session.rollback() + raise HTTPError( + HTTPStatus.CONFLICT, "Cannot delete assignment because it has submissions" + ) + if a.status in ["released", "complete"]: + self.session.rollback() + raise HTTPError( + HTTPStatus.CONFLICT, + "Cannot delete assignment because its status is not created", + ) + a.deleted = 1 + self.session.commit() + else: + if len(lecture.assignments) > 0: + msg = "Lecture cannot be deleted: assignments still exist. Delete assignments first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + if len(lecture.roles) > 0: + msg = "Lecture cannot be deleted: roles still exist. Delete roles first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + + # delete all associated directories of the lecture + lecture_path = os.path.abspath(os.path.join(self.gitbase, lecture.code)) + tmp_lecture_path = os.path.abspath(os.path.join(self.tmpbase, lecture.code)) + shutil.rmtree(lecture_path, ignore_errors=True) + shutil.rmtree(tmp_lecture_path, ignore_errors=True) + + self.session.delete(lecture) + self.session.commit() except ObjectDeletedError: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") self.write("OK") @@ -159,7 +191,7 @@ class LectureStudentsHandler(GraderBaseHandler): Tornado Handler class for http requests to /lectures/{lecture_id}/users. """ - @authorize([Scope.tutor, Scope.instructor]) + @authorize([Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int): """ Finds all users of a lecture and groups them by roles. diff --git a/grader_service/handlers/roles.py b/grader_service/handlers/roles.py new file mode 100644 index 000000000..8d0eab217 --- /dev/null +++ b/grader_service/handlers/roles.py @@ -0,0 +1,137 @@ +from http import HTTPStatus + +import tornado +from tornado.escape import json_decode +from tornado.web import HTTPError + +from grader_service.handlers.base_handler import GraderBaseHandler, authorize +from grader_service.handlers.handler_utils import parse_ids +from grader_service.orm import User +from grader_service.orm.takepart import Role, Scope +from grader_service.registry import VersionSpecifier, register_handler + +@register_handler(r"\/api\/users\/(?P[^\/]+)\/roles", VersionSpecifier.ALL) +class RoleUserHandler(GraderBaseHandler): + """ + Tornado Handler class for http requests to /user/{username}/roles. + """ + + @authorize([Scope.admin]) + async def get(self, username: str): + """ + Returns all roles for a specific user. + + :param username: name of the user + :type username: str + """ + self.validate_parameters() + + db_user = self.session.query(User).filter_by(name=username).first() + if db_user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + + roles = self.session.query(Role).filter(Role.user_id == db_user.id).all() + + self.set_status(HTTPStatus.OK) + self.write_json(roles) + +@register_handler(r"\/api\/lectures\/(?P\d*)\/roles", VersionSpecifier.ALL) +class RoleBaseHandler(GraderBaseHandler): + """ + Tornado Handler class for http requests to /lectures/{lecture_id}/roles. + """ + + @authorize([Scope.admin]) + async def get(self, lecture_id: int): + """ + Returns all roles for a specific lecture. + + :param lecture_id: id of the lecture + :type lecture_id: int + """ + lecture_id = parse_ids(lecture_id) + self.validate_parameters() + roles = self.session.query(Role).filter(Role.lectid == lecture_id).all() + + self.set_status(HTTPStatus.OK) + self.write_json(roles) + + @authorize([Scope.admin]) + async def post(self, lecture_id: int): + """ + Creates or update roles for a specific lecture. + + Request body example: + { + "users": [ + { "username": "alice", "role": 0 }, + { "username": "bob", "role": 1 } + ] + } + + :param lecture_id: id of the lecture + :type lecture_id: int + raises HTTPError: throws err if one user was not found + """ + lecture_id = parse_ids(lecture_id) + self.validate_parameters() + body = tornado.escape.json_decode(self.request.body) + + roles = [] + for user_req in body["users"]: + + user = self.session.query(User).filter(User.name == user_req["username"]).one_or_none() + if user is None: + self.session.rollback() + raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {user_req['username']} not found") + + role = self.session.query(Role) \ + .filter(Role.user_id == user.id) \ + .filter(Role.lectid == lecture_id) \ + .one_or_none() + if role is None: + role = Role() + role.user_id = user.id + role.lectid = lecture_id + self.session.add(role) + role.role = user_req["role"] + roles.append(role) + self.session.commit() + + self.set_status(HTTPStatus.CREATED) + self.write_json(roles) + + @authorize([Scope.admin]) + async def delete(self, lecture_id: int): + """ + Deletes roles for a specific lecture. + + Request body example: + { + "users": ["alice", "bob"] + } + + :param lecture_id: id of the lecture + :type lecture_id: int + raises HTTPError: throws err if one user was not found + """ + lecture_id = parse_ids(lecture_id) + self.validate_parameters() + body = tornado.escape.json_decode(self.request.body) + + roles = [] + for username in body['users']: + + user = self.session.query(User).filter(User.name == username).one_or_none() + if user is None: + self.session.rollback() + raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {username} not found") + + role = self.session.query(Role) \ + .filter(Role.user_id == user.id) \ + .filter(Role.lectid == lecture_id) \ + .one_or_none() + if role is not None: + roles.append(role) + self.session.delete(role) + self.session.commit() diff --git a/grader_service/handlers/submissions.py b/grader_service/handlers/submissions.py index b1b5f675b..9b649f782 100644 --- a/grader_service/handlers/submissions.py +++ b/grader_service/handlers/submissions.py @@ -62,7 +62,7 @@ class SubmissionLectureHandler(GraderBaseHandler): /lectures/{lecture_id}/submissions. """ - @authorize([Scope.tutor, Scope.instructor]) + @authorize([Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int): """Return the submissions of a specific lecture. @@ -94,26 +94,27 @@ async def get(self, lecture_id: int): ) if submission_filter == "latest": - subquery = ( + query = ( self.session.query(Submission.user_id, func.max(Submission.date).label("max_date")) .join(Assignment, Submission.assignid == Assignment.id) .filter(Assignment.lectid == lecture_id) - .filter(Submission.deleted == DeleteState.active) .group_by(Submission.user_id, Assignment.id) - .subquery() ) else: - subquery = ( + query = ( self.session.query( Submission.user_id, func.max(Submission.score).label("max_score") ) .join(Assignment, Submission.assignid == Assignment.id) .filter(Assignment.lectid == lecture_id) - .filter(Submission.deleted == DeleteState.active) .group_by(Submission.user_id, Assignment.id) - .subquery() ) + if not self.user.is_admin: + query = query.filter(Submission.deleted == DeleteState.active) + + subquery = query.subquery() + submissions_query = ( self.session.query( label("username", User.name), @@ -123,9 +124,11 @@ async def get(self, lecture_id: int): .join(Assignment, Submission.assignid == Assignment.id) .join(User, Submission.user_id == User.id) .filter(Assignment.lectid == lecture_id) - .filter(Submission.deleted == DeleteState.active) ) + if not self.user.is_admin: + submissions_query = submissions_query.filter(Submission.deleted == DeleteState.active) + if submission_filter == "latest": submissions = ( submissions_query.join( @@ -160,6 +163,67 @@ async def get(self, lecture_id: int): self.write(pivoted_df.to_json(orient="columns", force_ascii=False)) +@register_handler( + path=r"\/api\/users\/(?P[^\/]+)\/submissions\/?", + version_specifier=VersionSpecifier.ALL, +) +class SubmissionUserHandler(GraderBaseHandler): + """Tornado Handler class for http requests to + /users/{username}/submissions. + """ + + @authorize([Scope.tutor, Scope.instructor, Scope.admin]) + async def get(self, username: str): + """Return the submissions of a specific user. + + One query parameter: + 1 - format: + csv: return list as comma separated values + json: return list as JSON + + :param username: name of the user + :type username: str + :raises HTTPError: throws err if user is not authorized + """ + self.validate_parameters("format") + response_format = self.get_argument("format", "json") + if response_format not in ["json", "csv"]: + raise HTTPError(HTTPStatus.BAD_REQUEST, reason="Response format can either be 'json' or 'csv'") + + if not self.user.is_admin: + if username != self.user.name: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Forbidden") + + db_user = self.session.query(User).filter_by(name=username).first() + if db_user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + + submissions_query = ( + self.session.query(Submission) + .filter(Submission.user_id == db_user.id) + .order_by(Submission.id) + ) + + if not self.user.is_admin: + submissions_query = submissions_query.filter(Submission.deleted == DeleteState.active) + + submissions = submissions_query.all() + + if response_format == "csv": + self._write_csv(submissions) + else: + self.write_json([submission.serialize_with_lectid() for submission in submissions]) + self.session.close() + + def _write_csv(self, submissions): + self.set_header("Content-Type", "text/csv") + for i, s in enumerate(submissions): + d = s.model.serialize_with_lectid() + if i == 0: + self.write(",".join((k for k in d.keys() if k != "logs")) + "\n") + self.write(",".join((str(v) for k, v in d.items() if k != "logs")) + "\n") + + @register_handler( path=r"\/api\/lectures\/(?P\d*)\/assignments" + r"\/(?P\d*)\/submissions\/?", @@ -197,7 +261,7 @@ def validate_assignment(self, lecture_id, assignment_id): ) return True - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int, assignment_id: int): """Return the submissions of an assignment. @@ -229,23 +293,29 @@ async def get(self, lecture_id: int, assignment_id: int): HTTPStatus.BAD_REQUEST, reason="Response format can either be 'json' or 'csv'" ) - # check required scopes for instructor version - role: Role = self.get_role(lecture_id) - if instr_version and role.role < Scope.tutor: - raise HTTPError(HTTPStatus.FORBIDDEN, reason="Forbidden") + if not self.user.is_admin: + # check required scopes for instructor version + role: Role = self.get_role(lecture_id) + if instr_version and role.role < Scope.tutor: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Forbidden") # validate that assignment is part of lecture self.validate_assignment(lecture_id, assignment_id) # get list of submissions based on arguments - user_id = None if instr_version else role.user_id + user_id = None if instr_version else self.user.id + if submission_filter == "latest": submissions = self.get_latest_submissions(assignment_id, user_id=user_id) elif submission_filter == "best": submissions = self.get_best_submissions(assignment_id, user_id=user_id) else: - query = self.session.query(Submission).filter( - Submission.assignid == assignment_id, Submission.deleted == DeleteState.active - ) + if not self.user.is_admin: + query = self.session.query(Submission).filter( + Submission.assignid == assignment_id, Submission.deleted == DeleteState.active + ) + else: + query = self.session.query(Submission).filter(Submission.assignid == assignment_id) + if user_id: query = query.filter(Submission.user_id == user_id) submissions = query.order_by(Submission.id).all() @@ -448,7 +518,7 @@ class SubmissionObjectHandler(GraderBaseHandler): /{submission_id}. """ - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, lecture_id: int, assignment_id: int, submission_id: int): """Returns a specific submission. @@ -463,7 +533,7 @@ async def get(self, lecture_id: int, assignment_id: int, submission_id: int): lecture_id, assignment_id, submission_id ) submission = self.get_submission(lecture_id, assignment_id, submission_id) - if self.get_role(lecture_id).role == Scope.student and submission.user_id != self.user.id: + if not self.user.is_admin and self.get_role(lecture_id).role == Scope.student and submission.user_id != self.user.id: raise HTTPError(HTTPStatus.NOT_FOUND) self.write_json(submission) @@ -502,9 +572,9 @@ async def put(self, lecture_id: int, assignment_id: int, submission_id: int): self.session.commit() self.write_json(sub) - @authorize([Scope.student, Scope.tutor, Scope.instructor]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def delete(self, lecture_id: int, assignment_id: int, submission_id: int): - """Soft-deletes a specific submission. + """Soft or Hard-deletes a specific submission. :param lecture_id: id of the lecture :type lecture_id: int @@ -512,41 +582,67 @@ async def delete(self, lecture_id: int, assignment_id: int, submission_id: int): :type assignment_id: int :param submission_id: id of the submission :type submission_id: int - :raises HTTPError: if submission has feedback, or the deadline has passed, + :raises HTTPError: for soft-delete if submission has feedback, or the deadline has passed, or it has already been (soft-)deleted, or it belongs to another student, or it was not found in the given lecture and assignment. + for hard-delete if user is not an admin, + or it was not found in the given lecture and assignment. """ lecture_id, assignment_id, submission_id = parse_ids( lecture_id, assignment_id, submission_id ) - self.validate_parameters() + self.validate_parameters("hard_delete") + hard_delete = self.get_argument("hard_delete", "false") == "true" + submission = self.get_submission(lecture_id, assignment_id, submission_id) if submission is None: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") - # Do not allow students to delete other users' submissions - if self.get_role(lecture_id).role < Scope.instructor and submission.user_id != self.user.id: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") + if not hard_delete: + # Do not allow students to delete other users' submissions + if not self.user.is_admin and self.get_role(lecture_id).role < Scope.instructor and submission.user_id != self.user.id: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") - if submission.feedback_status != FeedbackStatus.NOT_GENERATED: - raise HTTPError( - HTTPStatus.FORBIDDEN, reason="Only submissions without feedback can be deleted." - ) + if submission.feedback_status != FeedbackStatus.NOT_GENERATED: + raise HTTPError( + HTTPStatus.FORBIDDEN, reason="Only submissions without feedback can be deleted." + ) - # if assignment has deadline and it has already passed - if ( - submission.assignment.settings.deadline - and submission.assignment.settings.deadline - < datetime.datetime.now(datetime.timezone.utc) - ): - raise HTTPError( - HTTPStatus.FORBIDDEN, - reason="Submission can't be deleted, due date of assigment has passed.", - ) + # if assignment has deadline and it has already passed + if ( + submission.assignment.settings.deadline + and submission.assignment.settings.deadline + < datetime.datetime.now(datetime.timezone.utc) + ): + raise HTTPError( + HTTPStatus.FORBIDDEN, + reason="Submission can't be deleted, due date of assigment has passed.", + ) - submission.deleted = DeleteState.deleted - self.session.commit() + submission.deleted = DeleteState.deleted + self.session.commit() + else: + if not self.current_user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete submission.") + + # delete all associated directories of the submission + assignment_path = os.path.abspath(os.path.join(self.gitbase, submission.assignment.lecture.code, str(submission.assignment.id))) + tmp_assignment_path = os.path.abspath(os.path.join(self.tmpbase, submission.assignment.lecture.code, str(submission.assignment.id))) + target_names = {submission.user.name, str(submission.id)} + matching_dirs = [] + for path in [assignment_path, tmp_assignment_path]: + for root, dirs, _ in os.walk(path): + for d in dirs: + if d in target_names: + matching_dirs.append(os.path.join(root, d)) + for path in matching_dirs: + shutil.rmtree(path, ignore_errors=True) + + self.session.delete(submission.logs) + self.session.delete(submission.properties) + self.session.delete(submission) + self.session.commit() @register_handler( diff --git a/grader_service/handlers/users.py b/grader_service/handlers/users.py new file mode 100644 index 000000000..9a30a80d5 --- /dev/null +++ b/grader_service/handlers/users.py @@ -0,0 +1,108 @@ +from http import HTTPStatus + +import tornado +from tornado.escape import json_decode +from tornado.web import HTTPError + +from grader_service.api.models.user import User as UserModel +from grader_service.handlers.base_handler import GraderBaseHandler, authorize +from grader_service.orm import User +from grader_service.orm.takepart import Scope +from grader_service.registry import VersionSpecifier, register_handler + + +@register_handler(r"\/api\/users", VersionSpecifier.ALL) +class UserBaseHandler(GraderBaseHandler): + """ + Tornado Handler class for http requests to /users. + """ + @authorize([Scope.admin]) + async def get(self): + """ + Returns all users. + """ + self.validate_parameters() + user = self.session.query(User).filter().all() + + self.set_status(HTTPStatus.OK) + self.write_json(user) + +@register_handler(r"\/api\/users\/(?P[^\/]+)\/?", VersionSpecifier.ALL) +class UserObjectBaseHandler(GraderBaseHandler): + """ + Tornado Handler class for http requests to /users/{username}. + """ + + @authorize([Scope.admin]) + async def get(self, username: str): + """ + Returns a specific user. + + :param username: the name of the user. + :type username: str + :raises HTTPError: throws err if user was not found + """ + self.validate_parameters() + + user = self.session.query(User).filter_by(name=username).first() + if user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + + self.set_status(HTTPStatus.OK) + self.write_json(user) + + @authorize([Scope.admin]) + async def put(self, username: str): + """ + Updates a specific user. + + :param username: the name of the user. + :type username: str + :raises HTTPError: throws err if user was not found + """ + self.validate_parameters() + body = tornado.escape.json_decode(self.request.body) + user_model = UserModel.from_dict(body) + + user = self.session.query(User).filter_by(name=username).first() + if user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + + user.display_name = user_model.display_name + self.session.add(user) + self.session.commit() + + self.set_status(HTTPStatus.OK) + self.write_json(user) + + @authorize([Scope.admin]) + async def delete(self, username: str): + """ + Hard-Deletes a specific user. + Hard deleting: removes user from datastore. + + :param username: the name of the user. + :type username: str + :raises HTTPError: throws err if user was not found, + or user has still submissions, + or user has still roles + """ + self.validate_parameters() + + user = self.session.query(User).filter_by(name=username).first() + if user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + + if len(user.submissions) > 0: + msg = "User cannot be deleted: submissions still exist. Delete submissions first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + if len(user.roles) > 0: + msg = "User cannot be deleted: roles still exist. Delete roles first!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + + for api_token in user.api_tokens: + self.session.delete(api_token) + for oauth_code in user.oauth_codes: + self.session.delete(oauth_code) + self.session.delete(user) + self.session.commit() diff --git a/grader_service/orm/submission.py b/grader_service/orm/submission.py index 36602028e..f7ce9a801 100644 --- a/grader_service/orm/submission.py +++ b/grader_service/orm/submission.py @@ -99,3 +99,13 @@ def serialize_with_user(self) -> dict: model = self.model.to_dict() model["user"] = self.user.serialize() return model + + def serialize_with_lectid(self) -> dict: + """Serialize the submission with lectid. + + Returns: + dict: The serialized submission data including lectid. + """ + model = self.model.to_dict() + model["lectid"] = self.assignment.lectid + return model diff --git a/grader_service/orm/takepart.py b/grader_service/orm/takepart.py index 1bf8e4dfb..93e6f4adc 100644 --- a/grader_service/orm/takepart.py +++ b/grader_service/orm/takepart.py @@ -27,3 +27,6 @@ class Role(Base, Serializable): lecture = relationship("Lecture", back_populates="roles") user = relationship("User", back_populates="roles") + + def serialize(self): + return {"user_id": self.user_id, "lectid": self.lectid, "role": self.role} From 5fc2b9acc6f721d6273a11b3de1a3d162120d90a Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Wed, 29 Oct 2025 12:05:06 +0100 Subject: [PATCH 2/6] Fix bug: instructor can't soft-delete lecture --- grader_service/handlers/lectures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grader_service/handlers/lectures.py b/grader_service/handlers/lectures.py index b289d6e28..0a13eafd1 100644 --- a/grader_service/handlers/lectures.py +++ b/grader_service/handlers/lectures.py @@ -122,7 +122,7 @@ async def get(self, lecture_id: int): raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") self.write_json(lecture) - @authorize([Scope.admin]) + @authorize([Scope.instructor, Scope.admin]) async def delete(self, lecture_id: int): """ Soft or Hard-Deletes a specific lecture. From ea682ba547ccbbd6bfb06095ee6a2845799af96a Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:02:15 +0100 Subject: [PATCH 3/6] Add ondelete cascade and enable SQLite foreign key --- grader_service/main.py | 12 +- .../28500016a3c3_add_user_display_name.py | 8 + .../4a88dacd888f_add_ondelete_cascade.py | 319 ++++++++++++++++++ .../versions/9983ef1fda76_add_user_pk_id.py | 15 + ...c153_added_oauth_provider_functionality.py | 15 +- grader_service/orm/__init__.py | 4 + grader_service/orm/assignment.py | 6 +- grader_service/orm/lecture.py | 8 +- grader_service/orm/oauthclient.py | 6 +- grader_service/orm/submission.py | 14 +- grader_service/orm/submission_logs.py | 2 +- grader_service/orm/submission_properties.py | 2 +- grader_service/orm/takepart.py | 14 +- grader_service/orm/user.py | 16 +- 14 files changed, 420 insertions(+), 21 deletions(-) create mode 100644 grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py diff --git a/grader_service/main.py b/grader_service/main.py index 18f1cf910..39f441298 100644 --- a/grader_service/main.py +++ b/grader_service/main.py @@ -11,13 +11,14 @@ import secrets import shutil import signal +import sqlite3 import subprocess import sys import tornado import uvloop as uvloop from jupyterhub.log import log_request -from sqlalchemy import create_engine +from sqlalchemy import create_engine, Engine, event from sqlalchemy.orm import scoped_session, sessionmaker from tornado.httpserver import HTTPServer from traitlets import ( @@ -40,7 +41,6 @@ from grader_service import __version__ from grader_service.auth.auth import Authenticator - # run __init__.py to register handlers from grader_service.auth.dummy import DummyAuthenticator from grader_service.autograding.celery.app import CeleryApp @@ -63,6 +63,14 @@ def get_session_maker(url) -> scoped_session: return scoped_session(sessionmaker(bind=engine)) +@event.listens_for(Engine, "connect") +def _set_sqlite_pragma(dbapi_connection, connection_record): + if isinstance(dbapi_connection, sqlite3.Connection): + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA foreign_keys=ON;") + cursor.close() + + class GraderService(config.Application): name = "grader-service" version = __version__ diff --git a/grader_service/migrate/versions/28500016a3c3_add_user_display_name.py b/grader_service/migrate/versions/28500016a3c3_add_user_display_name.py index 165d84002..cb29babc7 100644 --- a/grader_service/migrate/versions/28500016a3c3_add_user_display_name.py +++ b/grader_service/migrate/versions/28500016a3c3_add_user_display_name.py @@ -8,6 +8,7 @@ import sqlalchemy as sa from alembic import op +from sqlalchemy import text # revision identifiers, used by Alembic. revision = "28500016a3c3" @@ -17,6 +18,10 @@ def upgrade(): + conn = op.get_bind() + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) + # Step 1: Add column as nullable op.add_column("user", sa.Column("display_name", sa.String(), nullable=True)) @@ -31,6 +36,9 @@ def upgrade(): with op.batch_alter_table("user") as batch_op: batch_op.alter_column("display_name", nullable=False) + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) + def downgrade(): op.drop_column("user", "display_name") diff --git a/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py b/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py new file mode 100644 index 000000000..aaf1df974 --- /dev/null +++ b/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py @@ -0,0 +1,319 @@ +"""add ondelete cascade + +Revision ID: 4a88dacd888f +Revises: 9983ef1fda76 +Create Date: 2025-11-04 11:55:10.513853 + +""" +from typing import Dict, List + +from alembic import op +from sqlalchemy import text, inspect, Inspector, Table, MetaData, Column, String, select, and_, Connection + +# revision identifiers, used by Alembic. +revision = '4a88dacd888f' +down_revision = '9983ef1fda76' +branch_labels = None +depends_on = None + + +""" +- Standardizes the names of all foreign keys regardless of the database used. + - SQLite: None -> fk__ + - PostgreSQL:
__fkey -> fk_
_ +- Adds the new foreign key “fk_api_token_client_id”. +- Due to the new foreign key “fk_api_token_client_id,” invalid “api_token” entries are deleted. +- All foreign keys receive the option ondelete="CASCADE". +- The new table “alembic_fk_metadata” stores the old foreign key names so that they can be restored during downgrade. +The exception is SQLite, because SQLAlchemy does not allow “None” in names, the new foreign key names are retained. +""" + + +def _drop_all_foreign_keys(batch_op, connection, table_name: str): + """ + Remove all foreign key constraints from the specified table. + + Only drops constraints that have a defined name, since unnamed foreign keys + cannot be dropped explicitly. (SQLite) + """ + inspector = Inspector.from_engine(connection) + fks = inspector.get_foreign_keys(table_name) + for fk in fks: + if fk['name'] is not None: + batch_op.drop_constraint(fk["name"], type_="foreignkey") + + +def _get_fk_name(conn, table_name, local_cols, referred_table): + """Return the foreign key name if exists, else None (SQLite may be None)""" + inspector = inspect(conn) + for fk in inspector.get_foreign_keys(table_name): + if fk["constrained_columns"] == local_cols and fk["referred_table"] == referred_table: + return fk["name"] + return None + + +def _store_old_fk(conn, table_name, local_cols, referred_table, old_name): + """Store information about an existing foreign key in a helper table.""" + metadata = MetaData() + fk_table = Table( + "alembic_fk_metadata", metadata, + Column("table_name", String, primary_key=True), + Column("column_name", String, primary_key=True), + Column("referred_table", String), + Column("old_fk_name", String), + ) + fk_table.create(conn, checkfirst=True) + conn.execute( + fk_table.insert().values( + table_name=table_name, + column_name=local_cols[0], + referred_table=referred_table, + old_fk_name=old_name, + ) + ) + + +def _get_stored_old_fk(conn, table_name, local_cols, referred_table): + """Retrieve old foreign key name from a helper table""" + metadata = MetaData() + fk_table = Table("alembic_fk_metadata", metadata, autoload_with=conn) + stmt = select(fk_table.c.old_fk_name).where( + and_( + fk_table.c.table_name == table_name, + fk_table.c.column_name == local_cols[0], + fk_table.c.referred_table == referred_table, + ) + ) + result = conn.execute(stmt).first() + return result[0] if result else None + + +def _upgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk_definitions: List[Dict]) -> None: + """ + Recreate foreign key constraints on a table during a migration. + + Steps performed: + 1. Retrieve the current foreign key names for the given definitions. + 2. Store the old foreign key info in a helper table for reference. + 3. Drop all existing foreign key constraints on the table. + 4. Recreate the foreign keys using the new definitions with CASCADE on delete. + + :param connection: SQLAlchemy connection object. + :type connection: sqlalchemy.engine.Connection + :param table_name: Name of the table to alter. + :type table_name: str + :param fk_definitions: List of dictionaries describing foreign keys to recreate, each containing: + - new_constraint_name: Name for the new foreign key constraint. + - referred_table: Name of the referenced table. + - local_cols: List of columns in the local table. + - remote_cols: List of columns in the referenced table. + :type fk_definitions: List[Dict] + """ + with op.batch_alter_table(table_name) as batch_op: + for fk in fk_definitions: + old_name = _get_fk_name(connection, table_name, fk["local_cols"], fk["referred_table"]) + _store_old_fk(connection, table_name, fk["local_cols"], fk["referred_table"], old_name) + + _drop_all_foreign_keys(batch_op, connection, table_name) + + for fk in fk_definitions: + batch_op.create_foreign_key( + constraint_name=fk["new_constraint_name"], + referent_table=fk["referred_table"], + local_cols=fk["local_cols"], + remote_cols=fk["remote_cols"], + ondelete="CASCADE" + ) + + +def _downgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk_definitions: List[Dict]) -> None: + """ + Restore previous foreign key constraints during a downgrade migration. + + For each foreign key in fk_definitions: + 1. Retrieve the old foreign key name from the helper table. + 2. Drop the new foreign key constraint (except for SQLite, which handles it differently). + 3. Recreate the old foreign key using the stored name, or fallback to the new name if none was stored. + + :param connection: SQLAlchemy connection object. + :type connection: sqlalchemy.engine.Connection + :param table_name: Name of the table to alter. + :type table_name: str + :param fk_definitions: List of dictionaries describing foreign keys to restore, each containing: + - new_constraint_name: Name of the current foreign key constraint. + - referred_table: Name of the referenced table. + - local_cols: List of columns in the local table. + - remote_cols: List of columns in the referenced table. + :type fk_definitions: List[Dict] + """ + with op.batch_alter_table(table_name) as batch_op: + for fk in fk_definitions: + old_fk_name = _get_stored_old_fk(connection, table_name, fk["local_cols"], fk["referred_table"]) + if connection.dialect.name != "sqlite": + batch_op.drop_constraint(fk["new_constraint_name"], type_="foreignkey") + if old_fk_name is None: + if connection.dialect.name != "sqlite": + return + old_fk_name = fk["new_constraint_name"] + batch_op.create_foreign_key( + constraint_name=old_fk_name, + referent_table=fk["referred_table"], + local_cols=fk["local_cols"], + remote_cols=fk["remote_cols"], + ) + + +def upgrade(): + connection = op.get_bind() + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="assignment", + fk_definitions=[ + {"new_constraint_name": "fk_assignment_lectid", "referred_table": "lecture", + "local_cols": ["lectid"], "remote_cols": ["id"]} + ] + ) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="submission", + fk_definitions=[ + {"new_constraint_name": "fk_submission_assignid", "referred_table": "assignment", + "local_cols": ["assignid"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_submission_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]} + ] + ) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="submission_logs", + fk_definitions=[ + {"new_constraint_name": "fk_submission_logs_sub_id", "referred_table": "submission", + "local_cols": ["sub_id"], "remote_cols": ["id"]} + ] + ) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="submission_properties", + fk_definitions=[ + {"new_constraint_name": "fk_submission_properties_sub_id", "referred_table": "submission", + "local_cols": ["sub_id"], "remote_cols": ["id"]} + ] + ) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="takepart", + fk_definitions=[ + {"new_constraint_name": "fk_takepart_lectid", "referred_table": "lecture", + "local_cols": ["lectid"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_takepart_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]} + ] + ) + + # invalid “api_token” entries are deleted + connection.execute( + text(""" + DELETE + FROM api_token + WHERE client_id NOT IN (SELECT identifier FROM oauth_client) + """) + ) + _upgrade_recreate_foreign_keys( + connection=connection, table_name="api_token", + fk_definitions=[ + {"new_constraint_name": "fk_api_token_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_api_token_client_id", "referred_table": "oauth_client", + "local_cols": ["client_id"], "remote_cols": ["identifier"]} + ] + ) + + _upgrade_recreate_foreign_keys( + connection=connection, table_name="oauth_code", + fk_definitions=[ + {"new_constraint_name": "fk_oauth_code_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_oauth_code_client_id", "referred_table": "oauth_client", + "local_cols": ["client_id"], "remote_cols": ["identifier"]} + ] + ) + + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=ON;")) + + +def downgrade(): + connection = op.get_bind() + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="assignment", + fk_definitions=[ + {"new_constraint_name": "fk_assignment_lectid", "referred_table": "lecture", + "local_cols": ["lectid"], "remote_cols": ["id"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="submission", + fk_definitions=[ + {"new_constraint_name": "fk_submission_assignid", "referred_table": "assignment", + "local_cols": ["assignid"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_submission_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="submission_logs", + fk_definitions=[ + {"new_constraint_name": "fk_submission_logs_sub_id", "referred_table": "submission", + "local_cols": ["sub_id"], "remote_cols": ["id"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="submission_properties", + fk_definitions=[ + {"new_constraint_name": "fk_submission_properties_sub_id", "referred_table": "submission", + "local_cols": ["sub_id"], "remote_cols": ["id"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="takepart", + fk_definitions=[ + {"new_constraint_name": "fk_takepart_lectid", "referred_table": "lecture", + "local_cols": ["lectid"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_takepart_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="api_token", + fk_definitions=[ + {"new_constraint_name": "fk_api_token_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_api_token_client_id", "referred_table": "oauth_client", + "local_cols": ["client_id"], "remote_cols": ["identifier"]} + ] + ) + + _downgrade_recreate_foreign_keys( + connection=connection, table_name="oauth_code", + fk_definitions=[ + {"new_constraint_name": "fk_oauth_code_user_id", "referred_table": "user", + "local_cols": ["user_id"], "remote_cols": ["id"]}, + {"new_constraint_name": "fk_oauth_code_client_id", "referred_table": "oauth_client", + "local_cols": ["client_id"], "remote_cols": ["identifier"]} + ] + ) + + op.drop_table("alembic_fk_metadata") + + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=ON;")) diff --git a/grader_service/migrate/versions/9983ef1fda76_add_user_pk_id.py b/grader_service/migrate/versions/9983ef1fda76_add_user_pk_id.py index 7c13b9ee3..97d1e73cb 100644 --- a/grader_service/migrate/versions/9983ef1fda76_add_user_pk_id.py +++ b/grader_service/migrate/versions/9983ef1fda76_add_user_pk_id.py @@ -8,6 +8,7 @@ import sqlalchemy as sa from alembic import op +from sqlalchemy import text # revision identifiers, used by Alembic. revision = "9983ef1fda76" @@ -17,6 +18,10 @@ def upgrade(): + conn = op.get_bind() + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) + user_table = sa.table("user", sa.column("id"), sa.column("name")) # 0. Drop FKs referencing user.name @@ -86,8 +91,15 @@ def _add_user_id_col(table_name: str) -> None: batch_op.create_foreign_key("fk_oauth_code_user_id", "user", ["user_id"], ["id"]) batch_op.drop_column("username") + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) + def downgrade(): + conn = op.get_bind() + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) + user_table = sa.table("user", sa.column("id"), sa.column("name")) def _add_username_col(table_name: str) -> None: @@ -146,3 +158,6 @@ def _add_username_col(table_name: str) -> None: ]: with op.batch_alter_table(table) as batch_op: batch_op.create_foreign_key(fk, "user", ["username"], ["name"]) + + if conn.dialect.name == "sqlite": + conn.execute(text("PRAGMA foreign_keys=OFF;")) diff --git a/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py b/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py index 1b0a159bd..c928443cc 100644 --- a/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py +++ b/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py @@ -16,6 +16,7 @@ PrimaryKeyConstraint, Text, Unicode, + text ) from grader_service.utils import new_token @@ -28,6 +29,10 @@ def upgrade(): + connection = op.get_bind() + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) + op.create_table( "api_token", Column("username", Unicode(255)), @@ -74,7 +79,6 @@ def upgrade(): op.add_column("user", sa.Column("encrypted_auth_state", sa.types.LargeBinary, nullable=True)) op.add_column("user", sa.Column("cookie_id", Unicode(255), nullable=True)) - connection = op.get_bind() result = connection.execute(sa.text('select * from "user"')).mappings() for row in result: connection.execute( @@ -90,9 +94,15 @@ def upgrade(): batch_op.alter_column("cookie_id", nullable=False) batch_op.create_unique_constraint("uq_user_cookie", ["cookie_id"]) + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) + def downgrade(): connection = op.get_bind() + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) + op.drop_table("api_token") op.drop_table("oauth_code") op.drop_table("oauth_client") @@ -104,3 +114,6 @@ def downgrade(): batch_op.drop_constraint("uq_user_cookie") op.drop_column("user", "cookie_id") + + if connection.dialect.name == "sqlite": + connection.execute(text("PRAGMA foreign_keys=OFF;")) \ No newline at end of file diff --git a/grader_service/orm/__init__.py b/grader_service/orm/__init__.py index 80c156c13..00b142018 100644 --- a/grader_service/orm/__init__.py +++ b/grader_service/orm/__init__.py @@ -13,6 +13,8 @@ from grader_service.orm.submission import Submission from grader_service.orm.takepart import Role from grader_service.orm.user import User +from grader_service.orm.submission_logs import SubmissionLogs +from grader_service.orm.submission_properties import SubmissionProperties __all__ = [ "Lecture", @@ -24,4 +26,6 @@ "OAuthCode", "OAuthClient", "APIToken", + "SubmissionLogs", + "SubmissionProperties" ] diff --git a/grader_service/orm/assignment.py b/grader_service/orm/assignment.py index 788d7d700..57e52a726 100644 --- a/grader_service/orm/assignment.py +++ b/grader_service/orm/assignment.py @@ -33,7 +33,7 @@ class Assignment(Base, Serializable): id = Column(Integer, primary_key=True, autoincrement=True) name = Column(String(255), nullable=False) """Name of the assignment""" - lectid = Column(Integer, ForeignKey("lecture.id")) + lectid = Column(Integer, ForeignKey("lecture.id", ondelete="CASCADE")) points = Column(DECIMAL(10, 3), nullable=True) status = Column(Enum("created", "pushed", "released", "complete"), default="created") deleted = Column(Enum(DeleteState), nullable=False, unique=False) @@ -43,7 +43,9 @@ class Assignment(Base, Serializable): _settings = Column("settings", Text, server_default="", nullable=False) lecture = relationship("Lecture", back_populates="assignments") - submissions = relationship("Submission", back_populates="assignment") + submissions = relationship( + "Submission", back_populates="assignment", cascade="all, delete-orphan", passive_deletes=True + ) @property def settings(self) -> AssignmentSettings: diff --git a/grader_service/orm/lecture.py b/grader_service/orm/lecture.py index 5565d5065..2a5db7e54 100644 --- a/grader_service/orm/lecture.py +++ b/grader_service/orm/lecture.py @@ -32,8 +32,12 @@ class Lecture(Base, Serializable): DateTime, default=datetime.now(UTC), onupdate=datetime.now(UTC), nullable=False ) - assignments = relationship("Assignment", back_populates="lecture") - roles = relationship("Role", back_populates="lecture") + assignments = relationship( + "Assignment", back_populates="lecture", cascade="all, delete-orphan", passive_deletes=True + ) + roles = relationship( + "Role", back_populates="lecture", cascade="all, delete-orphan", passive_deletes=True + ) @property def model(self) -> lecture.Lecture: diff --git a/grader_service/orm/oauthclient.py b/grader_service/orm/oauthclient.py index de4e08cb2..c41fd6ae2 100644 --- a/grader_service/orm/oauthclient.py +++ b/grader_service/orm/oauthclient.py @@ -19,9 +19,11 @@ def client_id(self): return self.identifier access_tokens = relationship( - APIToken, back_populates="oauth_client", cascade="all, delete-orphan" + APIToken, back_populates="oauth_client", cascade="all, delete-orphan", passive_deletes=True + ) + codes = relationship( + OAuthCode, back_populates="client", cascade="all, delete-orphan", passive_deletes=True ) - codes = relationship(OAuthCode, back_populates="client", cascade="all, delete-orphan") def __repr__(self): return f"<{self.__class__.__name__}(identifier={self.identifier!r})>" diff --git a/grader_service/orm/submission.py b/grader_service/orm/submission.py index f7ce9a801..6406526ec 100644 --- a/grader_service/orm/submission.py +++ b/grader_service/orm/submission.py @@ -48,8 +48,8 @@ class Submission(Base, Serializable): auto_status = Column(Enum(AutoStatus), default=AutoStatus.NOT_GRADED, nullable=False) manual_status = Column(Enum(ManualStatus), default=ManualStatus.NOT_GRADED, nullable=False) score = Column(Float, nullable=True) - assignid = Column(Integer, ForeignKey("assignment.id")) - user_id = Column(Integer, ForeignKey("user.id"), nullable=False) + assignid = Column(Integer, ForeignKey("assignment.id", ondelete="CASCADE")) + user_id = Column(Integer, ForeignKey("user.id", ondelete="CASCADE"), nullable=False) commit_hash = Column(String(length=40), nullable=False) feedback_status = Column( Enum(FeedbackStatus), default=FeedbackStatus.NOT_GENERATED, nullable=False @@ -64,8 +64,14 @@ class Submission(Base, Serializable): assignment = relationship("Assignment", back_populates="submissions") user = relationship("User", back_populates="submissions") - logs = relationship("SubmissionLogs", back_populates="submission", uselist=False) - properties = relationship("SubmissionProperties", back_populates="submission", uselist=False) + logs = relationship( + "SubmissionLogs", back_populates="submission", uselist=False, + cascade="all, delete-orphan", passive_deletes=True + ) + properties = relationship( + "SubmissionProperties", back_populates="submission", uselist=False, + cascade="all, delete-orphan", passive_deletes=True + ) @hybrid_property def user_display_name(self) -> str: diff --git a/grader_service/orm/submission_logs.py b/grader_service/orm/submission_logs.py index c194038c8..6f8535049 100644 --- a/grader_service/orm/submission_logs.py +++ b/grader_service/orm/submission_logs.py @@ -6,7 +6,7 @@ class SubmissionLogs(Base, Serializable): __tablename__ = "submission_logs" - sub_id = Column(Integer, ForeignKey("submission.id"), primary_key=True) + sub_id = Column(Integer, ForeignKey("submission.id", ondelete="CASCADE"), primary_key=True) logs = Column(Text, nullable=True) submission = relationship("Submission", back_populates="logs") diff --git a/grader_service/orm/submission_properties.py b/grader_service/orm/submission_properties.py index 8528450b9..42d370070 100644 --- a/grader_service/orm/submission_properties.py +++ b/grader_service/orm/submission_properties.py @@ -6,7 +6,7 @@ class SubmissionProperties(Base, Serializable): __tablename__ = "submission_properties" - sub_id = Column(Integer, ForeignKey("submission.id"), primary_key=True) + sub_id = Column(Integer, ForeignKey("submission.id", ondelete="CASCADE"), primary_key=True) properties = Column(Text, nullable=True) submission = relationship("Submission", back_populates="properties") diff --git a/grader_service/orm/takepart.py b/grader_service/orm/takepart.py index 93e6f4adc..d7624a374 100644 --- a/grader_service/orm/takepart.py +++ b/grader_service/orm/takepart.py @@ -21,8 +21,8 @@ class Scope(enum.IntEnum): class Role(Base, Serializable): __tablename__ = "takepart" - user_id = Column(Integer, ForeignKey("user.id"), primary_key=True) - lectid = Column(Integer, ForeignKey("lecture.id"), primary_key=True) + user_id = Column(Integer, ForeignKey("user.id", ondelete="CASCADE"), primary_key=True) + lectid = Column(Integer, ForeignKey("lecture.id", ondelete="CASCADE"), primary_key=True) role = Column(Enum(Scope), nullable=False) lecture = relationship("Lecture", back_populates="roles") @@ -30,3 +30,13 @@ class Role(Base, Serializable): def serialize(self): return {"user_id": self.user_id, "lectid": self.lectid, "role": self.role} + + def serialize_with_user(self) -> dict: + """Serialize the role with user information. + + Returns: + dict: The serialized role data including user information. + """ + model = self.serialize() + model["user"] = self.user.serialize() + return model \ No newline at end of file diff --git a/grader_service/orm/user.py b/grader_service/orm/user.py index 5e7241c76..9d15596b8 100644 --- a/grader_service/orm/user.py +++ b/grader_service/orm/user.py @@ -25,10 +25,18 @@ class User(Base, Serializable): name = Column(String(255), nullable=False, unique=True) display_name = Column(String(255), nullable=False) - roles = relationship("Role", back_populates="user") - submissions = relationship("Submission", back_populates="user") - api_tokens = relationship("APIToken", back_populates="user") - oauth_codes = relationship("OAuthCode", back_populates="user") + roles = relationship( + "Role", back_populates="user", cascade="all, delete-orphan", passive_deletes=True + ) + submissions = relationship( + "Submission", back_populates="user", cascade="all, delete-orphan", passive_deletes=True + ) + api_tokens = relationship( + "APIToken", back_populates="user", cascade="all, delete-orphan", passive_deletes=True + ) + oauth_codes = relationship( + "OAuthCode", back_populates="user", cascade="all, delete-orphan", passive_deletes=True + ) encrypted_auth_state = Column(LargeBinary) cookie_id = Column(Unicode(255), default=new_token, nullable=False, unique=True) From 68395bb614bd5cd1dad68809e577ab2860fc5b78 Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:07:42 +0100 Subject: [PATCH 4/6] Refactor handlers, fix bugs, and remove inconsistencies --- grader_service/handlers/assignment.py | 116 ++++++++++++++---------- grader_service/handlers/base_handler.py | 38 ++++++++ grader_service/handlers/lectures.py | 85 ++++++++--------- grader_service/handlers/roles.py | 81 ++++++++++------- grader_service/handlers/submissions.py | 91 ++++++++----------- grader_service/handlers/users.py | 38 ++++---- 6 files changed, 251 insertions(+), 198 deletions(-) diff --git a/grader_service/handlers/assignment.py b/grader_service/handlers/assignment.py index 1063bfde3..684d3e6e6 100644 --- a/grader_service/handlers/assignment.py +++ b/grader_service/handlers/assignment.py @@ -9,6 +9,7 @@ import isodate import tornado from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm.exc import ObjectDeletedError from tornado.web import HTTPError from grader_service.api.models.assignment import Assignment as AssignmentModel @@ -74,7 +75,12 @@ async def get(self, lecture_id: int): lecture_id = parse_ids(lecture_id) self.validate_parameters("include-submissions") - if not self.user.is_admin: + if self.user.is_admin: + lecture = self.get_lecture(lecture_id=lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") + assignments = lecture.assignments + else: role = self.get_role(lecture_id) if role.lecture.deleted == DeleteState.deleted: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") @@ -92,25 +98,20 @@ async def get(self, lecture_id: int): ) else: assignments = [a for a in role.lecture.assignments if (a.deleted == DeleteState.active)] - else: - lecture = self.get_lecture(lecture_id=lecture_id) - if lecture is None: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") - assignments = lecture.assignments # Handle the case that the user wants to include submissions include_submissions = self.get_argument("include-submissions", "true") == "true" if include_submissions: assignids = [a.id for a in assignments] - if not self.user.is_admin: - user_id = self.user.id + if self.user.is_admin: + results = self.session.query(Submission).filter(Submission.assignid.in_(assignids)).all() + else: results = ( self.session.query(Submission) - .filter(Submission.assignid.in_(assignids), Submission.user_id == user_id) + .filter(Submission.assignid.in_(assignids), Submission.user_id == self.user.id) .all() ) - else: - results = self.session.query(Submission).filter(Submission.assignid.in_(assignids)).all() + # Create a combined list of assignments and submissions assignments = [a.serialize() for a in assignments] submissions = [s.serialize() for s in results] @@ -119,7 +120,7 @@ async def get(self, lecture_id: int): assignments = sorted(assignments, key=lambda o: o["id"]) self.write_json(assignments) - @authorize([Scope.instructor]) + @authorize([Scope.instructor, Scope.admin]) async def post(self, lecture_id: int): """Creates a new assignment. @@ -262,8 +263,18 @@ async def get(self, lecture_id: int, assignment_id: int): :raises HTTPError: throws err if assignment was not found """ lecture_id, assignment_id = parse_ids(lecture_id, assignment_id) + self.validate_parameters() assignment = self.get_assignment(lecture_id=lecture_id, assignment_id=assignment_id) - self.write_json(assignment) + # reflects the behavior from get assignments + if self.user.is_admin: + self.write_json(assignment) + else: + role: Role = self.get_role(lecture_id=lecture_id) + if role.role == Scope.student: + if assignment.status == "created" or assignment.status == "pushed": + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Assignment not found") + self.write_json(assignment) + @authorize([Scope.instructor, Scope.admin]) async def delete(self, lecture_id: int, assignment_id: int): @@ -276,49 +287,54 @@ async def delete(self, lecture_id: int, assignment_id: int): :raises HTTPError: throws err if assignment was not found or deleted """ lecture_id, assignment_id = parse_ids(lecture_id, assignment_id) - self.validate_parameters() - assignment = self.get_assignment(lecture_id, assignment_id) + self.validate_parameters("hard_delete") + hard_delete = self.get_argument("hard_delete", "false") == "true" - if not self.user.is_admin: - if assignment.status in ["released", "complete"]: - msg = "Cannot delete released or completed assignments!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - - previously_deleted = ( - self.session.query(Assignment) - .filter( - Assignment.lectid == lecture_id, - Assignment.name == assignment.name, - Assignment.deleted == DeleteState.deleted, - ) - .one_or_none() - ) - if previously_deleted is not None: - if len(previously_deleted.submissions) > 0: - msg = "Assignment cannot be deleted: submissions still exist. Delete submissions first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + try: + assignment = self.get_assignment(lecture_id, assignment_id) + if assignment is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Assignment was not found") + + if hard_delete: + if not self.user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete assignment.") - self.delete_assignment_files(previously_deleted) - self.session.delete(previously_deleted) + self.delete_assignment_files(assignment) + self.session.delete(assignment) self.session.commit() + else: + if assignment.deleted == DeleteState.deleted: + raise HTTPError(HTTPStatus.NOT_FOUND) - assignment.deleted = DeleteState.deleted - self.session.commit() - else: - if len(assignment.submissions) > 0: - msg = "Assignment cannot be deleted: submissions still exist. Delete submissions first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + if len(assignment.submissions) > 0: + msg = "Cannot delete assignments with submissions!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) + if assignment.status in ["released", "complete"]: + msg = "Cannot delete released or completed assignments!" + raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - self.delete_assignment_files(assignment) - self.session.delete(assignment) - self.session.commit() + previously_deleted = ( + self.session.query(Assignment) + .filter( + Assignment.lectid == lecture_id, + Assignment.name == assignment.name, + Assignment.deleted == DeleteState.deleted, + ) + .one_or_none() + ) + if previously_deleted is not None: + self.delete_assignment_files(previously_deleted) + self.session.delete(previously_deleted) + self.session.commit() + + # No need to soft-delete submissions, because an assignment with submissions + # cannot reach this point (checked above). + assignment.deleted = DeleteState.deleted + self.session.commit() + except ObjectDeletedError: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Assignment was not found") + self.write(f"OK {assignment.deleted}") - def delete_assignment_files(self, assignment): - # delete all associated directories of the assignment - assignment_path = os.path.abspath(os.path.join(self.gitbase, assignment.lecture.code, str(assignment.id))) - tmp_assignment_path = os.path.abspath(os.path.join(self.tmpbase, assignment.lecture.code, str(assignment.id))) - shutil.rmtree(assignment_path, ignore_errors=True) - shutil.rmtree(tmp_assignment_path, ignore_errors=True) @register_handler( path=r"\/api\/lectures\/(?P\d*)\/assignments\/(?P\d*)\/reset\/?", diff --git a/grader_service/handlers/base_handler.py b/grader_service/handlers/base_handler.py index 82f7ca308..df9bcf261 100644 --- a/grader_service/handlers/base_handler.py +++ b/grader_service/handlers/base_handler.py @@ -70,6 +70,8 @@ def check_authorization( raise HTTPError(403) elif lecture_id is None and "/lectures" in self.request.path and self.request.method == "GET": return True + if re.match(r"/api/users/(?P[^/]+)/submissions/?", self.request.path) and self.request.method == "GET": + return True is_admin = self.authenticator.is_admin(handler=self, authentication={'name': self.user.name}) @@ -893,6 +895,42 @@ def get_best_submissions( return submissions_query.all() + def delete_lecture_files(self, lecture: Lecture): + # delete all associated directories of the lecture + lecture_path = os.path.abspath(os.path.join(self.gitbase, lecture.code)) + tmp_lecture_path = os.path.abspath(os.path.join(self.tmpbase, lecture.code)) + shutil.rmtree(lecture_path, ignore_errors=True) + shutil.rmtree(tmp_lecture_path, ignore_errors=True) + + def delete_assignment_files(self, assignment: Assignment): + # delete all associated directories of the assignment + assignment_path = os.path.abspath( + os.path.join(self.gitbase, assignment.lecture.code, str(assignment.id)) + ) + tmp_assignment_path = os.path.abspath( + os.path.join(self.tmpbase, assignment.lecture.code, str(assignment.id)) + ) + shutil.rmtree(assignment_path, ignore_errors=True) + shutil.rmtree(tmp_assignment_path, ignore_errors=True) + + def delete_submission_files(self, submission: Submission): + # delete all associated directories of the submission + assignment_path = os.path.abspath( + os.path.join(self.gitbase, submission.assignment.lecture.code, str(submission.assignment.id)) + ) + tmp_assignment_path = os.path.abspath( + os.path.join(self.tmpbase, submission.assignment.lecture.code, str(submission.assignment.id)) + ) + target_names = {submission.user.name, str(submission.id)} + matching_dirs = [] + for path in [assignment_path, tmp_assignment_path]: + for root, dirs, _ in os.walk(path): + for d in dirs: + if d in target_names: + matching_dirs.append(os.path.join(root, d)) + for path in matching_dirs: + shutil.rmtree(path, ignore_errors=True) + @property def gitbase(self): app: GraderServer = self.application diff --git a/grader_service/handlers/lectures.py b/grader_service/handlers/lectures.py index 0a13eafd1..5cfa06157 100644 --- a/grader_service/handlers/lectures.py +++ b/grader_service/handlers/lectures.py @@ -3,8 +3,6 @@ # # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -import os -import shutil from http import HTTPStatus import tornado @@ -31,19 +29,24 @@ class LectureBaseHandler(GraderBaseHandler): async def get(self): """ Returns all lectures the user can access. + - For regular users: only lectures they have a role in, which match the requested state and are not deleted. + - For admins: all lectures, regardless of state or deletion. """ self.validate_parameters("complete") complete = self.get_argument("complete", "false") == "true" - state = LectureState.complete if complete else LectureState.active - lectures = sorted( - [ - role.lecture - for role in self.user.roles - if role.lecture.state == state and role.lecture.deleted == DeleteState.active - ], - key=lambda lecture: lecture.id, - ) + + if self.user.is_admin: + lectures = self.session.query(Lecture).order_by(Lecture.id.asc()).all() + else: + lectures = sorted( + [ + role.lecture + for role in self.user.roles + if role.lecture.state == state and role.lecture.deleted == DeleteState.active + ], + key=lambda lecture: lecture.id, + ) self.write_json(lectures) @@ -55,26 +58,21 @@ async def post(self): self.validate_parameters() body = tornado.escape.json_decode(self.request.body) lecture_model = LectureModel.from_dict(body) - created = False lecture = ( self.session.query(Lecture).filter(Lecture.code == lecture_model.code).one_or_none() ) if lecture is None: lecture = Lecture() - self.session.add(lecture) - created = True lecture.name = lecture_model.name lecture.code = lecture_model.code lecture.state = LectureState.complete if lecture_model.complete else LectureState.active lecture.deleted = DeleteState.active + self.session.add(lecture) self.session.commit() - if created: - self.set_status(HTTPStatus.CREATED) - else: - self.set_status(HTTPStatus.OK) + self.set_status(HTTPStatus.CREATED) self.write_json(lecture) @@ -111,15 +109,15 @@ async def get(self, lecture_id: int): :return: lecture with given id """ self.validate_parameters() - if not self.user.is_admin: + if self.user.is_admin: + lecture = self.get_lecture(lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") + else: role = self.get_role(lecture_id) lecture = role.lecture if lecture.deleted == DeleteState.deleted: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") - else: - lecture = self.get_lecture(lecture_id) - if lecture is None: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") self.write_json(lecture) @authorize([Scope.instructor, Scope.admin]) @@ -143,40 +141,33 @@ async def delete(self, lecture_id: int): lecture = self.get_lecture(lecture_id) if lecture is None: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") - if not hard_delete: - if lecture.deleted == 1: - raise HTTPError(404) - lecture.deleted = 1 + + if hard_delete: + if not self.user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete lecture.") + + self.delete_lecture_files(lecture) + self.session.delete(lecture) + self.session.commit() + else: + if lecture.deleted == DeleteState.deleted: + raise HTTPError(HTTPStatus.NOT_FOUND) + + lecture.deleted = DeleteState.deleted a: Assignment for a in lecture.assignments: - if (len(a.submissions)) > 0: + if len(a.submissions) > 0: self.session.rollback() raise HTTPError( - HTTPStatus.CONFLICT, "Cannot delete assignment because it has submissions" + HTTPStatus.CONFLICT, "Cannot delete assignments with submissions!" ) if a.status in ["released", "complete"]: self.session.rollback() raise HTTPError( HTTPStatus.CONFLICT, - "Cannot delete assignment because its status is not created", + "Cannot delete released or completed assignments!", ) - a.deleted = 1 - self.session.commit() - else: - if len(lecture.assignments) > 0: - msg = "Lecture cannot be deleted: assignments still exist. Delete assignments first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - if len(lecture.roles) > 0: - msg = "Lecture cannot be deleted: roles still exist. Delete roles first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - - # delete all associated directories of the lecture - lecture_path = os.path.abspath(os.path.join(self.gitbase, lecture.code)) - tmp_lecture_path = os.path.abspath(os.path.join(self.tmpbase, lecture.code)) - shutil.rmtree(lecture_path, ignore_errors=True) - shutil.rmtree(tmp_lecture_path, ignore_errors=True) - - self.session.delete(lecture) + a.deleted = DeleteState.deleted self.session.commit() except ObjectDeletedError: raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture was not found") diff --git a/grader_service/handlers/roles.py b/grader_service/handlers/roles.py index 8d0eab217..7889902e6 100644 --- a/grader_service/handlers/roles.py +++ b/grader_service/handlers/roles.py @@ -1,6 +1,6 @@ from http import HTTPStatus -import tornado +from sqlalchemy.orm.exc import ObjectDeletedError from tornado.escape import json_decode from tornado.web import HTTPError @@ -10,7 +10,8 @@ from grader_service.orm.takepart import Role, Scope from grader_service.registry import VersionSpecifier, register_handler -@register_handler(r"\/api\/users\/(?P[^\/]+)\/roles", VersionSpecifier.ALL) + +@register_handler(r"\/api\/users\/(?P[^\/]+)\/roles\/?", VersionSpecifier.ALL) class RoleUserHandler(GraderBaseHandler): """ Tornado Handler class for http requests to /user/{username}/roles. @@ -35,7 +36,8 @@ async def get(self, username: str): self.set_status(HTTPStatus.OK) self.write_json(roles) -@register_handler(r"\/api\/lectures\/(?P\d*)\/roles", VersionSpecifier.ALL) + +@register_handler(r"\/api\/lectures\/(?P\d*)\/roles\/?", VersionSpecifier.ALL) class RoleBaseHandler(GraderBaseHandler): """ Tornado Handler class for http requests to /lectures/{lecture_id}/roles. @@ -51,10 +53,11 @@ async def get(self, lecture_id: int): """ lecture_id = parse_ids(lecture_id) self.validate_parameters() + roles = self.session.query(Role).filter(Role.lectid == lecture_id).all() self.set_status(HTTPStatus.OK) - self.write_json(roles) + self.write_json([r.serialize_with_user() for r in roles]) @authorize([Scope.admin]) async def post(self, lecture_id: int): @@ -71,11 +74,15 @@ async def post(self, lecture_id: int): :param lecture_id: id of the lecture :type lecture_id: int - raises HTTPError: throws err if one user was not found + :raises HTTPError: throws err if one user was not found """ lecture_id = parse_ids(lecture_id) self.validate_parameters() - body = tornado.escape.json_decode(self.request.body) + body = json_decode(self.request.body) + + lecture = self.get_lecture(lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") roles = [] for user_req in body["users"]: @@ -99,39 +106,51 @@ async def post(self, lecture_id: int): self.session.commit() self.set_status(HTTPStatus.CREATED) - self.write_json(roles) + self.write_json([r.serialize_with_user() for r in roles]) @authorize([Scope.admin]) async def delete(self, lecture_id: int): """ Deletes roles for a specific lecture. - Request body example: - { - "users": ["alice", "bob"] - } + Query parameter example: + ?usernames=alice,boby :param lecture_id: id of the lecture :type lecture_id: int - raises HTTPError: throws err if one user was not found + :raises HTTPError: if the lecture does not exist, no usernames were provided, + or at least one user cannot be found """ lecture_id = parse_ids(lecture_id) - self.validate_parameters() - body = tornado.escape.json_decode(self.request.body) - - roles = [] - for username in body['users']: - - user = self.session.query(User).filter(User.name == username).one_or_none() - if user is None: - self.session.rollback() - raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {username} not found") - - role = self.session.query(Role) \ - .filter(Role.user_id == user.id) \ - .filter(Role.lectid == lecture_id) \ - .one_or_none() - if role is not None: - roles.append(role) - self.session.delete(role) - self.session.commit() + self.validate_parameters("usernames") + raw_usernames = self.get_argument("usernames", "") + + try: + # Roles can not be soft-deleted + if not self.user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can delete roles.") + + lecture = self.get_lecture(lecture_id) + if lecture is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Lecture not found") + + if len(raw_usernames.strip()) == 0: + raise HTTPError(HTTPStatus.BAD_REQUEST, reason="usernames cannot be empty") + usernames = raw_usernames.split(",") + + for username in usernames: + user = self.session.query(User).filter(User.name == username).one_or_none() + if user is None: + self.session.rollback() + raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {username} was not found") + + role = self.session.query(Role) \ + .filter(Role.user_id == user.id) \ + .filter(Role.lectid == lecture_id) \ + .one_or_none() + if role is not None: + self.session.delete(role) + self.session.commit() + except ObjectDeletedError: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Roles was not found") + self.write("OK") diff --git a/grader_service/handlers/submissions.py b/grader_service/handlers/submissions.py index 9b649f782..e4bb13b37 100644 --- a/grader_service/handlers/submissions.py +++ b/grader_service/handlers/submissions.py @@ -17,7 +17,7 @@ import tornado from celery import chain from sqlalchemy import label -from sqlalchemy.orm.exc import NoResultFound +from sqlalchemy.orm.exc import NoResultFound, ObjectDeletedError from sqlalchemy.sql.expression import func from tornado.web import HTTPError @@ -172,7 +172,7 @@ class SubmissionUserHandler(GraderBaseHandler): /users/{username}/submissions. """ - @authorize([Scope.tutor, Scope.instructor, Scope.admin]) + @authorize([Scope.student, Scope.tutor, Scope.instructor, Scope.admin]) async def get(self, username: str): """Return the submissions of a specific user. @@ -212,13 +212,13 @@ async def get(self, username: str): if response_format == "csv": self._write_csv(submissions) else: - self.write_json([submission.serialize_with_lectid() for submission in submissions]) + self.write_json([s.serialize_with_lectid() for s in submissions]) self.session.close() def _write_csv(self, submissions): self.set_header("Content-Type", "text/csv") for i, s in enumerate(submissions): - d = s.model.serialize_with_lectid() + d = s.serialize_with_lectid() if i == 0: self.write(",".join((k for k in d.keys() if k != "logs")) + "\n") self.write(",".join((str(v) for k, v in d.items() if k != "logs")) + "\n") @@ -309,12 +309,12 @@ async def get(self, lecture_id: int, assignment_id: int): elif submission_filter == "best": submissions = self.get_best_submissions(assignment_id, user_id=user_id) else: - if not self.user.is_admin: + if self.user.is_admin: + query = self.session.query(Submission).filter(Submission.assignid == assignment_id) + else: query = self.session.query(Submission).filter( Submission.assignid == assignment_id, Submission.deleted == DeleteState.active ) - else: - query = self.session.query(Submission).filter(Submission.assignid == assignment_id) if user_id: query = query.filter(Submission.user_id == user_id) @@ -594,55 +594,44 @@ async def delete(self, lecture_id: int, assignment_id: int, submission_id: int): self.validate_parameters("hard_delete") hard_delete = self.get_argument("hard_delete", "false") == "true" - submission = self.get_submission(lecture_id, assignment_id, submission_id) + try: + submission = self.get_submission(lecture_id, assignment_id, submission_id) + if submission is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission was not found") - if submission is None: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") + if hard_delete: + if not self.user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete submission.") - if not hard_delete: - # Do not allow students to delete other users' submissions - if not self.user.is_admin and self.get_role(lecture_id).role < Scope.instructor and submission.user_id != self.user.id: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") + self.delete_submission_files(submission) + self.session.delete(submission) + self.session.commit() + else: + # Do not allow students to delete other users' submissions + if not self.user.is_admin and self.get_role(lecture_id).role < Scope.instructor and submission.user_id != self.user.id: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") - if submission.feedback_status != FeedbackStatus.NOT_GENERATED: - raise HTTPError( - HTTPStatus.FORBIDDEN, reason="Only submissions without feedback can be deleted." - ) + if submission.feedback_status != FeedbackStatus.NOT_GENERATED: + raise HTTPError( + HTTPStatus.FORBIDDEN, reason="Only submissions without feedback can be deleted." + ) - # if assignment has deadline and it has already passed - if ( - submission.assignment.settings.deadline - and submission.assignment.settings.deadline - < datetime.datetime.now(datetime.timezone.utc) - ): - raise HTTPError( - HTTPStatus.FORBIDDEN, - reason="Submission can't be deleted, due date of assigment has passed.", - ) + # if assignment has deadline and it has already passed + if ( + submission.assignment.settings.deadline + and submission.assignment.settings.deadline + < datetime.datetime.now(datetime.timezone.utc) + ): + raise HTTPError( + HTTPStatus.FORBIDDEN, + reason="Submission can't be deleted, due date of assigment has passed.", + ) - submission.deleted = DeleteState.deleted - self.session.commit() - else: - if not self.current_user.is_admin: - raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete submission.") - - # delete all associated directories of the submission - assignment_path = os.path.abspath(os.path.join(self.gitbase, submission.assignment.lecture.code, str(submission.assignment.id))) - tmp_assignment_path = os.path.abspath(os.path.join(self.tmpbase, submission.assignment.lecture.code, str(submission.assignment.id))) - target_names = {submission.user.name, str(submission.id)} - matching_dirs = [] - for path in [assignment_path, tmp_assignment_path]: - for root, dirs, _ in os.walk(path): - for d in dirs: - if d in target_names: - matching_dirs.append(os.path.join(root, d)) - for path in matching_dirs: - shutil.rmtree(path, ignore_errors=True) - - self.session.delete(submission.logs) - self.session.delete(submission.properties) - self.session.delete(submission) - self.session.commit() + submission.deleted = DeleteState.deleted + self.session.commit() + except ObjectDeletedError: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission was not found") + self.write("OK") @register_handler( diff --git a/grader_service/handlers/users.py b/grader_service/handlers/users.py index 9a30a80d5..586d5ab3b 100644 --- a/grader_service/handlers/users.py +++ b/grader_service/handlers/users.py @@ -1,6 +1,6 @@ from http import HTTPStatus -import tornado +from sqlalchemy.orm.exc import ObjectDeletedError from tornado.escape import json_decode from tornado.web import HTTPError @@ -11,7 +11,7 @@ from grader_service.registry import VersionSpecifier, register_handler -@register_handler(r"\/api\/users", VersionSpecifier.ALL) +@register_handler(r"\/api\/users\/?", VersionSpecifier.ALL) class UserBaseHandler(GraderBaseHandler): """ Tornado Handler class for http requests to /users. @@ -61,7 +61,7 @@ async def put(self, username: str): :raises HTTPError: throws err if user was not found """ self.validate_parameters() - body = tornado.escape.json_decode(self.request.body) + body = json_decode(self.request.body) user_model = UserModel.from_dict(body) user = self.session.query(User).filter_by(name=username).first() @@ -89,20 +89,20 @@ async def delete(self, username: str): """ self.validate_parameters() - user = self.session.query(User).filter_by(name=username).first() - if user is None: - raise HTTPError(HTTPStatus.NOT_FOUND, reason="User not found") + try: + # User can not be soft-deleted + if not self.user.is_admin: + raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can delete users.") - if len(user.submissions) > 0: - msg = "User cannot be deleted: submissions still exist. Delete submissions first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - if len(user.roles) > 0: - msg = "User cannot be deleted: roles still exist. Delete roles first!" - raise HTTPError(HTTPStatus.CONFLICT, reason=msg) - - for api_token in user.api_tokens: - self.session.delete(api_token) - for oauth_code in user.oauth_codes: - self.session.delete(oauth_code) - self.session.delete(user) - self.session.commit() + user = self.session.query(User).filter_by(name=username).first() + if user is None: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User was not found") + + for submission in user.submissions: + self.delete_submission_files(submission) + + self.session.delete(user) + self.session.commit() + except ObjectDeletedError: + raise HTTPError(HTTPStatus.NOT_FOUND, reason="User was not found") + self.write("OK") From 7e1ba24e89c6289d4051d15ab6f0c3c9d8aab45e Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:10:42 +0100 Subject: [PATCH 5/6] Add new tests for handlers --- grader_service/tests/conftest.py | 26 +- grader_service/tests/handlers/db_util.py | 121 ++- .../tests/handlers/test_assignment_handler.py | 426 ++++++++--- grader_service/tests/handlers/test_git.py | 7 +- .../tests/handlers/test_lectures_handler.py | 434 ++++++++++- .../tests/handlers/test_permission_handler.py | 3 +- .../tests/handlers/test_roles_handler.py | 423 +++++++++++ .../handlers/test_submissions_handler.py | 711 ++++++++++++++++-- .../tests/handlers/test_users_handler.py | 374 +++++++++ grader_service/tests/migrate/test_migrate.py | 14 +- 10 files changed, 2317 insertions(+), 222 deletions(-) create mode 100644 grader_service/tests/handlers/test_roles_handler.py create mode 100644 grader_service/tests/handlers/test_users_handler.py diff --git a/grader_service/tests/conftest.py b/grader_service/tests/conftest.py index 0f4a337d2..318948824 100644 --- a/grader_service/tests/conftest.py +++ b/grader_service/tests/conftest.py @@ -26,6 +26,18 @@ def default_user_login(default_user, sql_alchemy_engine): engine = sql_alchemy_engine session: Session = sessionmaker(engine)() user = session.get(User, default_user.id) + user.is_admin = False + + with patch.object(handlers.base_handler.BaseHandler, "_grader_user", new=user, create=True): + yield + + +@pytest.fixture(scope="function") +def default_admin_login(default_admin, sql_alchemy_engine): + engine = sql_alchemy_engine + session: Session = sessionmaker(engine)() + user = session.get(User, default_admin.id) + user.is_admin = True with patch.object(handlers.base_handler.BaseHandler, "_grader_user", new=user, create=True): yield @@ -37,6 +49,7 @@ def default_roles_dict(): "20wle2": [{"members": ["ubuntu"], "role": "instructor"}], "21wle1": [{"members": ["ubuntu"], "role": "student"}], "22wle1": [{"members": ["ubuntu"], "role": "instructor"}], + "23wle1": [{"members": ["debian"], "role": "instructor"}], } @@ -72,14 +85,17 @@ def sql_alchemy_sessionmaker(db_test_config): @pytest.fixture(scope="function") -def app(tmpdir, sql_alchemy_sessionmaker): +def app(tmpdir, sql_alchemy_sessionmaker, default_admin): service_dir = str(tmpdir.mkdir("grader_service")) handlers = HandlerPathRegistry.handler_list() + authenticator = DummyAuthenticator() + authenticator.admin_users = [default_admin.name] + application = GraderServer( grader_service_dir=service_dir, base_url="/", - authenticator=DummyAuthenticator(), + authenticator=authenticator, handlers=handlers, oauth_provider=None, session_maker=sql_alchemy_sessionmaker, @@ -110,6 +126,12 @@ def default_user(): yield user +@pytest.fixture(scope="function") +def default_admin(): + user = User(id=2, name="debian") + yield user + + @pytest.fixture(scope="function") def default_token(): token = "token" diff --git a/grader_service/tests/handlers/db_util.py b/grader_service/tests/handlers/db_util.py index dddb80795..f5a76ac94 100644 --- a/grader_service/tests/handlers/db_util.py +++ b/grader_service/tests/handlers/db_util.py @@ -3,22 +3,28 @@ # # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. - +import os import secrets import subprocess from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Optional +from unittest.mock import Mock from sqlalchemy.engine import Engine from sqlalchemy.orm import Session, sessionmaker +from grader_service import orm from grader_service.api.models.assignment_settings import AssignmentSettings -from grader_service.orm import Assignment, Lecture, Role, Submission, User +from grader_service.handlers import GitRepoType +from grader_service.handlers.git.server import GitBaseHandler +from grader_service.orm import Assignment, Lecture, Role, Submission, User, SubmissionLogs from grader_service.orm.base import DeleteState from grader_service.orm.submission import AutoStatus, FeedbackStatus, ManualStatus from grader_service.orm.submission_properties import SubmissionProperties from grader_service.orm.takepart import Scope +from grader_service.server import GraderServer +from grader_service.tests.handlers.test_git import get_query_side_effect def add_role(engine: Engine, user_id: int, l_id: int, scope: Scope) -> Role: @@ -44,7 +50,8 @@ def insert_lectures(session: Engine): session: Session = sessionmaker(session)() session.add(_get_lecture(1, "lecture1", "21wle1")) session.add(_get_lecture(2, "lecture2", "20wle2")) - session.add(_get_lecture(3, "lecture2", "22wle1")) + session.add(_get_lecture(3, "lecture3", "22wle1")) + session.add(_get_lecture(4, "lecture4", "23wle1")) # session.add(_get_lecture("lecture3", "22sle3")) # session.add(_get_lecture("lecture4", "21sle4")) session.commit() @@ -63,6 +70,23 @@ def _get_assignment(name, lectid, points, status, settings): return a +def insert_assignment(ex, lecture_id=1): + session: Session = sessionmaker(ex)() + session.add( + _get_assignment( + "assignment_1", + lecture_id, + 20, + "created", + AssignmentSettings(deadline=datetime.now(tz=timezone.utc) + timedelta(weeks=2)), + ) + ) + session.commit() + session.flush() + num_inserts = 1 + return num_inserts + + def insert_assignments(ex, lecture_id=1): session: Session = sessionmaker(ex)() session.add( @@ -119,6 +143,7 @@ def insert_submission( with_properties: bool = True, score: float = None, commit_hash: Optional[str] = None, + with_logs: bool = False, ) -> Submission: # TODO Allows only one submission with properties per user because we do not have # the submission id @@ -132,6 +157,11 @@ def insert_submission( if with_properties: session.add(SubmissionProperties(sub_id=submission.id, properties=None)) session.commit() + + if with_logs: + session.add(SubmissionLogs(sub_id=submission.id, logs=None)) + session.commit() + session.refresh(submission) session.flush() return submission @@ -149,7 +179,7 @@ def insert_student(ex: Engine, username: str, lecture_id: int) -> User: def create_user_submission_with_repo( - engine: Engine, gitbase_dir: Path, student: User, assignment_id: int, lecture_code: str + engine: Engine, gitbase_dir: Path, student: User, assignment_id: int, lecture_code: str ) -> Submission: """Creates a submission for `student` and a user repo for storing it. @@ -192,3 +222,86 @@ def create_user_submission_with_repo( engine, assignment_id, student.name, user_id=student.id, commit_hash=commit_hash ) return submission + + +def check_assignment_and_status(engine: Engine, l_id: int, a_id: int, status: str, should_exist: bool = True): + session: Session = sessionmaker(engine)() + assignment = session.query(orm.Assignment).filter(orm.Assignment.id == a_id, orm.Assignment.lectid == l_id).first() + if should_exist: + assert assignment is not None, "assignment is None" + assert assignment.status == status, f"assert '{assignment.status}' == '{status}'" + else: + assert assignment is None, f"assignment exists (id={a_id}, lectid={l_id})" + + +def check_submission(engine: Engine, a_id: int, s_id: int, should_exist: bool = True): + session: Session = sessionmaker(engine)() + submission = session.query(orm.Submission).filter(orm.Submission.id == s_id, + orm.Submission.assignid == a_id).first() + if should_exist: + assert submission is not None, "submission is None" + else: + assert submission is None, f"submission exists (id={s_id}, assignid={a_id})" + + +def create_git_repository(app: GraderServer, l_id: int, code: str, a_id: int, s_id: int, repo_type: GitRepoType, + username: str): + git_dir = Path(app.grader_service_dir) / "git" + git_dir.mkdir(exist_ok=True) + path = f"/git/{code}/{a_id}/{repo_type}/{s_id}" + handler_mock = Mock() + handler_mock.request.path = path + handler_mock.gitbase = str(git_dir) + handler_mock.user.name = username + sf = get_query_side_effect(lid=l_id, code=code, scope=Scope.instructor, a_id=a_id, s_id=s_id, username=username) + handler_mock.session.query = Mock(side_effect=sf) + constructed_git_dir = GitBaseHandler.construct_git_dir( + handler_mock, + repo_type=repo_type, + lecture=sf(orm.Lecture).filter().one(), + assignment=sf(orm.Assignment).filter().one(), + submission=sf(orm.Submission).filter().one(), + ) + handler_mock.construct_git_dir = Mock(return_value=constructed_git_dir) + lookup_dir = GitBaseHandler.gitlookup(handler_mock, "send-pack") + assert os.path.exists(lookup_dir) + + +def create_all_git_repositories(app: GraderServer, user: User, l_id: int, l_code: str, a_id: int, s_id: int): + # create possible git repositories for submission + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.SOURCE, + username=user.name) + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.RELEASE, + username=user.name) + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.USER, + username=user.name) + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.EDIT, + username=user.name) + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.AUTOGRADE, + username=user.name) + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.FEEDBACK, + username=user.name) + + check_git_repositories(app, user, l_code, a_id, s_id, + True, True, True, True, True, True, True) + + +def check_git_repositories(app: GraderServer, user: User, l_code: str, a_id: int, s_id: int, + exits_assignment: bool, exits_source: bool, exits_release: bool, exits_user: bool, + exits_edit: bool, exits_feedback: bool, exits_autograde: bool): + assignment_path = Path(app.grader_service_dir) / "git" / l_code / str(a_id) + + source_path = assignment_path / GitRepoType.SOURCE + release_path = assignment_path / GitRepoType.RELEASE + user_path = assignment_path / GitRepoType.USER / user.name + edit_path = assignment_path / GitRepoType.EDIT / str(s_id) + feedback_path = assignment_path / GitRepoType.FEEDBACK / "user" / user.name + autograde_path = assignment_path / GitRepoType.AUTOGRADE / "user" / user.name + + assert assignment_path.exists() if exits_assignment else not assignment_path.exists() + assert source_path.exists() if exits_source else not source_path.exists() + assert release_path.exists() if exits_release else not release_path.exists() + assert user_path.exists() if exits_user else not user_path.exists() + assert edit_path.exists() if exits_edit else not edit_path.exists() + assert feedback_path.exists() if exits_feedback else not feedback_path.exists() + assert autograde_path.exists() if exits_autograde else not autograde_path.exists() diff --git a/grader_service/tests/handlers/test_assignment_handler.py b/grader_service/tests/handlers/test_assignment_handler.py index 150c18d4d..116e24c54 100644 --- a/grader_service/tests/handlers/test_assignment_handler.py +++ b/grader_service/tests/handlers/test_assignment_handler.py @@ -7,13 +7,15 @@ from http import HTTPStatus import pytest +from sqlalchemy.orm import Session, sessionmaker from tornado.httpclient import HTTPClientError from grader_service.api.models.assignment import Assignment from grader_service.api.models.assignment_settings import AssignmentSettings from grader_service.server import GraderServer - -from .db_util import insert_assignments, insert_submission +from .db_util import insert_assignments, insert_submission, check_assignment_and_status, insert_assignment, \ + create_all_git_repositories, check_git_repositories +from ... import orm async def test_get_assignments( @@ -29,10 +31,30 @@ async def test_get_assignments( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK + assignments = json.loads(response.body.decode()) + assert isinstance(assignments, list) + assert len(assignments) == 1 + [Assignment.from_dict(a) for a in assignments] # assert no errors + + +async def test_get_assignments_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures/1/assignments/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK assignments = json.loads(response.body.decode()) assert isinstance(assignments, list) - assert len(assignments) > 0 + assert len(assignments) == 2 [Assignment.from_dict(a) for a in assignments] # assert no errors @@ -54,7 +76,7 @@ async def test_get_assignments_instructor( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK assignments = json.loads(response.body.decode()) assert isinstance(assignments, list) assert len(assignments) == num_inserted @@ -76,7 +98,7 @@ async def test_get_assignments_lecture_deleted( delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK url = service_base_url + f"lectures/{l_id}/assignments/" @@ -85,7 +107,7 @@ async def test_get_assignments_lecture_deleted( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_post_assignment( @@ -101,7 +123,7 @@ async def test_post_assignment( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK assignments = json.loads(get_response.body.decode()) assert isinstance(assignments, list) orig_len = len(assignments) @@ -118,7 +140,7 @@ async def test_post_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) assert post_assignment.id != pre_assignment.id assert post_assignment.name == pre_assignment.name @@ -129,7 +151,7 @@ async def test_post_assignment( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK assignments = json.loads(get_response.body.decode()) assert len(assignments) == orig_len + 1 @@ -156,7 +178,7 @@ async def test_post_assignment_name_already_used( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(post_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -169,23 +191,6 @@ async def test_post_assignment_name_already_used( assert e.code == HTTPStatus.CONFLICT -async def test_delete_assignment_not_found( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, -): - url = service_base_url + "lectures/3/assignments/-5" - with pytest.raises(HTTPClientError) as exc_info: - await http_server_client.fetch( - url, method="DELETE", headers={"Authorization": f"Token {default_token}"} - ) - e = exc_info.value - assert e.code == HTTPStatus.NOT_FOUND - - async def test_put_assignment_name_already_used( app: GraderServer, service_base_url, @@ -256,7 +261,7 @@ async def test_post_assignment_lecture_deleted( delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK url = service_base_url + "lectures/3/assignments/" post_assignment = Assignment( @@ -273,7 +278,7 @@ async def test_post_assignment_lecture_deleted( body=json.dumps(post_assignment.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_post_assignment_decode_error( @@ -296,7 +301,7 @@ async def test_post_assignment_decode_error( body=json.dumps(pre_assignment.to_dict()), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST # no autograde type given pre_assignment = Assignment( @@ -310,7 +315,7 @@ async def test_post_assignment_decode_error( body=json.dumps(pre_assignment.to_dict()), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST async def test_post_assignment_missing_vars( @@ -354,7 +359,7 @@ async def test_post_no_status_error( body=json.dumps(pre_assignment.to_dict()), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST async def test_put_assignment( @@ -379,7 +384,7 @@ async def test_put_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) post_assignment.name = "new name" @@ -393,7 +398,7 @@ async def test_put_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(post_assignment.to_dict()), ) - assert put_response.code == 200 + assert put_response.code == HTTPStatus.OK put_assignment = Assignment.from_dict(json.loads(put_response.body.decode())) assert put_assignment.id == post_assignment.id assert put_assignment.name == "new name" @@ -423,7 +428,7 @@ async def test_put_assignment_wrong_lecture_id( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) # now with lecture id 2 because user would be instructor there too @@ -437,8 +442,7 @@ async def test_put_assignment_wrong_lecture_id( body=json.dumps(post_assignment.to_dict()), ) e = exc_info.value - print(e.response) - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_put_assignment_wrong_assignment_id( @@ -460,7 +464,7 @@ async def test_put_assignment_wrong_assignment_id( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) url = url + "99" @@ -472,7 +476,7 @@ async def test_put_assignment_wrong_assignment_id( body=json.dumps(post_assignment.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_put_assignment_deleted_assignment( @@ -497,7 +501,7 @@ async def test_put_assignment_deleted_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) url = url + str(post_assignment.id) @@ -505,7 +509,7 @@ async def test_put_assignment_deleted_assignment( delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -515,7 +519,7 @@ async def test_put_assignment_deleted_assignment( body=json.dumps(post_assignment.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == f"Assignment with id {post_assignment.id} was not found" @@ -541,7 +545,7 @@ async def test_put_assignment_no_point_changes( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) post_assignment.name = "new name" @@ -556,7 +560,7 @@ async def test_put_assignment_no_point_changes( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(post_assignment.to_dict()), ) - assert put_response.code == 200 + assert put_response.code == HTTPStatus.OK put_assignment = Assignment.from_dict(json.loads(put_response.body.decode())) assert put_assignment.id == post_assignment.id assert put_assignment.name == "new name" @@ -586,7 +590,7 @@ async def test_get_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) url = url + str(post_assignment.id) @@ -594,7 +598,7 @@ async def test_get_assignment( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK get_assignment = Assignment.from_dict(json.loads(get_response.body.decode())) assert get_assignment.id == post_assignment.id assert get_assignment.name == post_assignment.name @@ -610,16 +614,64 @@ async def test_get_assignment_created_student( default_token, default_roles, default_user_login, + sql_alchemy_engine, ): l_id = 1 # default user is student - a_id = 3 # assignment is created + a_id = 2 # assignment is created url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="created") + with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert exc_info.value.code == 404 + assert exc_info.value.code == HTTPStatus.NOT_FOUND + + +async def test_get_assignment_created_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, +): + l_id = 1 # default admin is admin + a_id = 2 # assignment is created + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="created") + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + Assignment.from_dict(json.loads(get_response.body.decode())) + + +async def test_get_assignment_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + sql_alchemy_engine, +): + l_id = 4 # default user has no role + a_id = 3 # assignment is released + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + + insert_assignments(sql_alchemy_engine, l_id) + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="released") + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert exc_info.value.code == HTTPStatus.FORBIDDEN async def test_get_assignment_wrong_lecture_id( @@ -645,7 +697,7 @@ async def test_get_assignment_wrong_lecture_id( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) l_id = 1 @@ -655,7 +707,7 @@ async def test_get_assignment_wrong_lecture_id( await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert exc_info.value.code == 404 + assert exc_info.value.code == HTTPStatus.NOT_FOUND async def test_get_assignment_wrong_assignment_id( @@ -681,7 +733,7 @@ async def test_get_assignment_wrong_assignment_id( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED url = service_base_url + f"lectures/{l_id}/assignments/99" @@ -689,10 +741,10 @@ async def test_get_assignment_wrong_assignment_id( await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert exc_info.value.code == 404 + assert exc_info.value.code == HTTPStatus.NOT_FOUND -async def test_get_assignment_instructor_version( +async def test_get_assignment_unknown_parameter( app: GraderServer, service_base_url, http_server_client, @@ -707,10 +759,11 @@ async def test_get_assignment_instructor_version( engine = sql_alchemy_engine insert_assignments(engine, 3) - get_response = await http_server_client.fetch( - url, method="GET", headers={"Authorization": f"Token {default_token}"} - ) - assert get_response.code == 200 + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert exc_info.value.code == HTTPStatus.BAD_REQUEST async def test_delete_assignment( @@ -720,8 +773,11 @@ async def test_delete_assignment( default_token, default_roles, default_user_login, + sql_alchemy_engine, ): - url = service_base_url + "lectures/3/assignments/" + l_id = 3 + + url = service_base_url + f"lectures/{l_id}/assignments/" pre_assignment = Assignment( id=-1, @@ -735,7 +791,7 @@ async def test_delete_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) url = url + str(post_assignment.id) @@ -743,7 +799,8 @@ async def test_delete_assignment( delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created") with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -751,7 +808,7 @@ async def test_delete_assignment( ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == f"Assignment with id {post_assignment.id} was not found" @@ -762,8 +819,11 @@ async def test_delete_assignment_deleted_assignment( default_token, default_roles, default_user_login, + sql_alchemy_engine, ): - url = service_base_url + "lectures/3/assignments/" + l_id = 3 + + url = service_base_url + f"lectures/{l_id}/assignments/" pre_assignment = Assignment( id=-1, @@ -777,7 +837,7 @@ async def test_delete_assignment_deleted_assignment( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) url = url + str(post_assignment.id) @@ -785,7 +845,8 @@ async def test_delete_assignment_deleted_assignment( delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created") with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -793,10 +854,27 @@ async def test_delete_assignment_deleted_assignment( ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == f"Assignment with id {post_assignment.id} was not found" +async def test_delete_assignment_not_found( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, +): + url = service_base_url + "lectures/3/assignments/-5" + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + async def test_delete_assignment_not_created( app: GraderServer, service_base_url, @@ -812,7 +890,7 @@ async def test_delete_assignment_not_created( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_delete_assignment_same_name_twice( @@ -822,8 +900,11 @@ async def test_delete_assignment_same_name_twice( default_token, default_roles, default_user_login, + sql_alchemy_engine, ): - url = service_base_url + "lectures/3/assignments/" + l_id = 3 + + url = service_base_url + f"lectures/{l_id}/assignments/" pre_assignment = Assignment( id=-1, @@ -837,15 +918,16 @@ async def test_delete_assignment_same_name_twice( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 - post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) + assert post_response.code == HTTPStatus.CREATED + first_post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) - url = url + str(post_assignment.id) + url = url + str(first_post_assignment.id) delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=first_post_assignment.id, status="created") url = service_base_url + "lectures/3/assignments/" @@ -855,15 +937,18 @@ async def test_delete_assignment_same_name_twice( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 - post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) + assert post_response.code == HTTPStatus.CREATED + second_post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) - url = url + str(post_assignment.id) + url = url + str(second_post_assignment.id) delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=first_post_assignment.id, status="created", + should_exist=False) + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=second_post_assignment.id, status="created") async def test_delete_released_assignment( @@ -936,6 +1021,154 @@ async def test_delete_complete_assignment( assert e.code == HTTPStatus.CONFLICT +async def test_delete_assignment_with_submissions( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_user, + sql_alchemy_engine, + default_roles, + default_user_login, +): + l_id = 3 # user has to be instructor + a_id = 3 + engine = sql_alchemy_engine + + insert_assignments(engine, l_id) + insert_submission(engine, a_id, default_user.name, default_user.id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.CONFLICT + + +async def test_delete_assignment_hard( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, +): + l_id = 3 + a_id = 3 + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + + insert_assignment(sql_alchemy_engine, l_id) + + delete_response = await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="created", should_exist=False) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + assert e.message == f"Assignment with id {a_id} was not found" + + session: Session = sessionmaker(sql_alchemy_engine)() + assignments = session.query(orm.Assignment).filter(orm.Assignment.lectid == l_id).all() + assert len(assignments) == 0 + + +async def test_delete_assignment_hard_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + sql_alchemy_engine, +): + l_id = 3 + a_id = 3 + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + + insert_assignment(sql_alchemy_engine, l_id) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_assignment_hard_with_submissions( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, + default_user +): + l_id = 4 + l_code = "23wle1" + a_id = 3 + s_id = 1 + + # create assignment + url = service_base_url + f"lectures/{l_id}/assignments" + pre_assignment = Assignment( + id=-1, + name="pytest", + status="released", + settings=AssignmentSettings(), + ) + post_response = await http_server_client.fetch( + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(pre_assignment.to_dict()), + ) + assert post_response.code == HTTPStatus.CREATED + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + create_all_git_repositories(app, default_user, l_id, l_code, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + delete_response = await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="released", should_exist=False) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + assert e.message == f"Assignment with id {a_id} was not found" + + session: Session = sessionmaker(sql_alchemy_engine)() + assignments = session.query(orm.Assignment).filter(orm.Assignment.lectid == l_id).all() + assert len(assignments) == 0 + + submissions = session.query(orm.Submission).filter(orm.Submission.assignid == a_id).all() + assert len(submissions) == 0 + + check_git_repositories(app, default_user, l_code, a_id, + False, False, False, False, False, False, False, False) + + async def test_assignment_properties_lecture_assignment_missmatch( app: GraderServer, service_base_url, @@ -961,14 +1194,14 @@ async def test_assignment_properties_lecture_assignment_missmatch( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_assignment_properties_wrong_assignment_id( @@ -996,14 +1229,14 @@ async def test_assignment_properties_wrong_assignment_id( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_assignment_properties_not_found( @@ -1027,7 +1260,7 @@ async def test_assignment_properties_not_found( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_assignment_properties_properties_wrong_for_autograde( @@ -1053,7 +1286,7 @@ async def test_assignment_properties_properties_wrong_for_autograde( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) assert post_assignment.settings.autograde_type == "full_auto" url = service_base_url + f"lectures/3/assignments/{post_assignment.id}/properties" @@ -1250,7 +1483,7 @@ async def test_assignment_properties_properties_manual_graded_with_auto_grading( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_assignment = Assignment.from_dict(json.loads(post_response.body.decode())) assert post_assignment.settings.autograde_type == "full_auto" url = service_base_url + f"lectures/3/assignments/{post_assignment.id}/properties" @@ -1401,30 +1634,3 @@ async def test_assignment_properties_properties_manual_graded_with_auto_grading( ) e = exc_info.value assert e.code == HTTPStatus.CONFLICT - - -async def test_delete_assignment_with_submissions( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_user, - sql_alchemy_engine, - default_roles, - default_user_login, -): - l_id = 3 # user has to be instructor - a_id = 3 - engine = sql_alchemy_engine - - insert_assignments(engine, l_id) - insert_submission(engine, a_id, default_user.name, default_user.id) - - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" - - with pytest.raises(HTTPClientError) as exc_info: - await http_server_client.fetch( - url, method="DELETE", headers={"Authorization": f"Token {default_token}"} - ) - e = exc_info.value - assert e.code == HTTPStatus.CONFLICT diff --git a/grader_service/tests/handlers/test_git.py b/grader_service/tests/handlers/test_git.py index ca020c802..d6334653a 100644 --- a/grader_service/tests/handlers/test_git.py +++ b/grader_service/tests/handlers/test_git.py @@ -20,7 +20,7 @@ def get_query_side_effect( - lid=1, code="ivs21s", scope: Scope = Scope.student, username="test_user", user_id=137, a_id=1 + lid=1, code="ivs21s", scope: Scope = Scope.student, username="test_user", user_id=137, a_id=1, s_id=1 ): def query_side_effect(input): m = Mock() @@ -39,9 +39,10 @@ def query_side_effect(input): m.get.return_value = role elif input is Submission: sub = Submission() + sub.id = s_id sub.user_id = user_id sub.user = User(id=user_id, name=username) - m.get.return_value = sub + m.filter.return_value.one.return_value = sub else: m.filter.return_value.one.return_value = None return m @@ -268,7 +269,7 @@ def test_git_lookup_pull_autograde_instructor(tmpdir): repo_type=GitRepoType.AUTOGRADE, lecture=sf(Lecture).filter().one(), assignment=sf(Assignment).filter().one(), - submission=sf(Submission).get(), + submission=sf(Submission).filter().one(), ) handler_mock.construct_git_dir = Mock(return_value=constructed_git_dir) diff --git a/grader_service/tests/handlers/test_lectures_handler.py b/grader_service/tests/handlers/test_lectures_handler.py index ce365f146..ecaf762e2 100644 --- a/grader_service/tests/handlers/test_lectures_handler.py +++ b/grader_service/tests/handlers/test_lectures_handler.py @@ -5,16 +5,20 @@ # LICENSE file in the root directory of this source tree. import json from http import HTTPStatus +from pathlib import Path import pytest +from sqlalchemy.orm import sessionmaker, Session from tornado.httpclient import HTTPClientError from grader_service.api.models.assignment import Assignment from grader_service.api.models.assignment_settings import AssignmentSettings from grader_service.api.models.lecture import Lecture from grader_service.server import GraderServer - -from .db_util import insert_assignments, insert_student, insert_submission +from .db_util import insert_assignment, insert_assignments, insert_student, insert_submission, create_git_repository +from ... import orm +from ...handlers import GitRepoType +from ...orm.base import DeleteState async def test_get_lectures( @@ -30,11 +34,33 @@ async def test_get_lectures( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK lectures = json.loads(response.body.decode()) assert isinstance(lectures, list) assert lectures [Lecture.from_dict(lec) for lec in lectures] # assert no errors + assert len(lectures) == 3 + + +async def test_get_lectures_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + lectures = json.loads(response.body.decode()) + assert isinstance(lectures, list) + assert lectures + [Lecture.from_dict(lec) for lec in lectures] + assert len(lectures) == 4 async def test_get_lectures_with_some_parameter( @@ -52,10 +78,10 @@ async def test_get_lectures_with_some_parameter( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST -async def test_post_lectures( +async def test_post_lectures_update( app: GraderServer, service_base_url, http_server_client, @@ -69,7 +95,7 @@ async def test_post_lectures( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK lectures = json.loads(get_response.body.decode()) assert isinstance(lectures, list) assert len(lectures) == 3 @@ -83,7 +109,7 @@ async def test_post_lectures( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_lecture.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED post_lecture = Lecture.from_dict(json.loads(post_response.body.decode())) assert post_lecture.id != pre_lecture.id assert post_lecture.name == pre_lecture.name @@ -93,12 +119,12 @@ async def test_post_lectures( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK lectures = json.loads(get_response.body.decode()) assert len(lectures) == orig_len -async def test_post_not_found( +async def test_post_lectures_update_unauthorized( app: GraderServer, service_base_url, http_server_client, @@ -109,8 +135,74 @@ async def test_post_not_found( ): url = service_base_url + "lectures" - pre_lecture = Lecture(id=-1, name="pytest_lecture", code="abc", complete=False) + pre_lecture = Lecture(id=-1, name="pytest_lecture", code="23wle1", complete=False) + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(pre_lecture.to_dict()), + ) + + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_post_lectures_update_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_admin, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures" + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + lectures = json.loads(get_response.body.decode()) + assert isinstance(lectures, list) + assert len(lectures) == 4 + orig_len = len(lectures) + + # same code as in group of user + pre_lecture = Lecture(id=-1, name="pytest_lecture", code="21wle1", complete=False) + post_response = await http_server_client.fetch( + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(pre_lecture.to_dict()), + ) + assert post_response.code == HTTPStatus.CREATED + post_lecture = Lecture.from_dict(json.loads(post_response.body.decode())) + assert post_lecture.id != pre_lecture.id + assert post_lecture.name == pre_lecture.name + assert post_lecture.code == pre_lecture.code + assert not post_lecture.complete + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + lectures = json.loads(get_response.body.decode()) + assert len(lectures) == orig_len + + +async def test_post_lectures_new_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + default_roles, + default_user_login, +): + url = service_base_url + "lectures" + pre_lecture = Lecture(id=-1, name="pytest_lecture_new", code="abc", complete=False) with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, @@ -118,9 +210,52 @@ async def test_post_not_found( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_lecture.to_dict()), ) + e = exc_info.value - print(e.response.error) - assert e.code == HTTPStatus.NOT_FOUND + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_post_lectures_new_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_admin, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures" + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + lectures = json.loads(get_response.body.decode()) + assert isinstance(lectures, list) + assert len(lectures) == 4 + orig_len = len(lectures) + + # same code as in group of user + pre_lecture = Lecture(id=-1, name="pytest_lecture", code="abc", complete=False) + post_response = await http_server_client.fetch( + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(pre_lecture.to_dict()), + ) + assert post_response.code == HTTPStatus.CREATED + post_lecture = Lecture.from_dict(json.loads(post_response.body.decode())) + assert post_lecture.id != pre_lecture.id + assert post_lecture.name == pre_lecture.name + assert post_lecture.code == pre_lecture.code + assert not post_lecture.complete + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + lectures = json.loads(get_response.body.decode()) + assert len(lectures) == orig_len + 1 async def test_post_unknown_parameter( @@ -143,7 +278,7 @@ async def test_post_unknown_parameter( body=json.dumps(pre_lecture.to_dict()), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST async def test_put_lecture( @@ -160,7 +295,7 @@ async def test_put_lecture( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK lecture = Lecture.from_dict(json.loads(get_response.body.decode())) lecture.name = "new name" lecture.complete = not lecture.complete @@ -174,7 +309,7 @@ async def test_put_lecture( body=json.dumps(lecture.to_dict()), ) - assert put_response.code == 200 + assert put_response.code == HTTPStatus.OK put_lecture = Lecture.from_dict(json.loads(put_response.body.decode())) assert put_lecture.name == lecture.name assert put_lecture.complete == lecture.complete @@ -195,7 +330,7 @@ async def test_put_lecture_unauthorized( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK lecture = Lecture.from_dict(json.loads(get_response.body.decode())) lecture.name = "new name" @@ -208,7 +343,7 @@ async def test_put_lecture_unauthorized( ) e = exc_info.value - assert e.code == 403 + assert e.code == HTTPStatus.FORBIDDEN async def test_get_lecture( @@ -224,11 +359,11 @@ async def test_get_lecture( get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK Lecture.from_dict(json.loads(get_response.body.decode())) -async def test_get_lecture_not_found( +async def test_get_lecture_unauthorized( app: GraderServer, service_base_url, http_server_client, @@ -243,7 +378,42 @@ async def test_get_lecture_not_found( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 403 + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_get_lecture_admin_not_found( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures/999" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_get_lecture_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, +): + url = service_base_url + "lectures/1" + + get_response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert get_response.code == HTTPStatus.OK + Lecture.from_dict(json.loads(get_response.body.decode())) async def test_delete_lecture( @@ -253,20 +423,50 @@ async def test_delete_lecture( default_token, default_roles, default_user_login, + sql_alchemy_engine, ): - url = service_base_url + "lectures/3" + l_id = 3 + url = service_base_url + f"lectures/{l_id}" delete_response = await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) - assert delete_response.code == 200 + assert delete_response.code == HTTPStatus.OK with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND + + session: Session = sessionmaker(sql_alchemy_engine)() + lectures = session.query(orm.Lecture).filter(orm.Lecture.id == l_id).all() + assert len(lectures) == 1 + assert lectures[0].deleted == DeleteState.deleted + + +async def test_delete_lecture_already_deleted( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, +): + url = service_base_url + "lectures/3" + + delete_response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND async def test_delete_lecture_unauthorized( @@ -284,7 +484,55 @@ async def test_delete_lecture_unauthorized( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 403 + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_lecture_assignment( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_user, +): + l_id = 3 + a_id = 3 + url = service_base_url + f"lectures/{l_id}" + + engine = sql_alchemy_engine + insert_assignment(engine, lecture_id=l_id) + + delete_response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + session: Session = sessionmaker(sql_alchemy_engine)() + lectures = session.query(orm.Lecture).filter(orm.Lecture.id == l_id).all() + assert len(lectures) == 1 + assert lectures[0].deleted == DeleteState.deleted + + session: Session = sessionmaker(sql_alchemy_engine)() + assignments = session.query(orm.Assignment).filter(orm.Assignment.lectid == l_id).all() + assert len(assignments) == 1 + assert assignments[0].deleted == DeleteState.deleted async def test_delete_lecture_assignment_with_submissions( @@ -298,11 +546,11 @@ async def test_delete_lecture_assignment_with_submissions( default_user, ): l_id = 3 - a_id = 2 + a_id = 3 url = service_base_url + f"lectures/{l_id}" engine = sql_alchemy_engine - insert_assignments(engine, lecture_id=3) + insert_assignment(engine, lecture_id=l_id) insert_submission(engine, a_id, default_user.name, default_user.id) with pytest.raises(HTTPClientError) as exc_info: @@ -359,7 +607,7 @@ async def test_delete_lecture_assignment_complete( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_assignment.to_dict()), ) - assert post_response.code == 201 + assert post_response.code == HTTPStatus.CREATED url = service_base_url + f"lectures/{l_id}" with pytest.raises(HTTPClientError) as exc_info: @@ -390,6 +638,136 @@ async def test_delete_lecture_not_found( assert e.code == HTTPStatus.NOT_FOUND +async def test_delete_lecture_hard( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, +): + l_id = 3 + + url = service_base_url + f"lectures/{l_id}" + delete_response = await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + session: Session = sessionmaker(sql_alchemy_engine)() + lectures = session.query(orm.Lecture).filter(orm.Lecture.id == l_id).all() + assert len(lectures) == 0 + + +async def test_delete_lecture_hard_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + sql_alchemy_engine, +): + l_id = 3 + + url = service_base_url + f"lectures/{l_id}" + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_lecture_hard_assignments_roles( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, + default_admin, +): + l_id = 4 + l_code = "23wle1" + a_id = 3 + + # create assignment + url = service_base_url + f"lectures/{l_id}/assignments" + pre_assignment = Assignment( + id=-1, + name="pytest", + status="released", + settings=AssignmentSettings(), + ) + post_response = await http_server_client.fetch( + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(pre_assignment.to_dict()), + ) + assert post_response.code == HTTPStatus.CREATED + + create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=1, repo_type=GitRepoType.SOURCE, username=default_admin.name) + + git_dir = Path(app.grader_service_dir) / "git" / l_code / str(a_id) + assert git_dir.exists() + + url = service_base_url + f"lectures/{l_id}" + delete_response = await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert delete_response.code == HTTPStatus.OK + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + session: Session = sessionmaker(sql_alchemy_engine)() + lectures = session.query(orm.Lecture).filter(orm.Lecture.id == l_id).all() + assert len(lectures) == 0 + + assignments = session.query(orm.Assignment).filter(orm.Assignment.lectid == l_id).all() + assert len(assignments) == 0 + + roles = session.query(orm.Role).filter(orm.Role.lectid == l_id).all() + assert len(roles) == 0 + + assert not git_dir.exists() + + +async def test_delete_lecture_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + default_roles, + default_user_login, +): + l_id = 3 + + url = service_base_url + f"lectures/{l_id}?some_param=asdf" + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"}, + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + async def test_get_lecture_users( service_base_url, http_server_client, @@ -411,4 +789,4 @@ async def test_get_lecture_users( data = json.loads(resp.body.decode()) assert data["instructors"] == [1] assert data["tutors"] == [] - assert data["students"] == [2, 3] + assert data["students"] == [3, 4] diff --git a/grader_service/tests/handlers/test_permission_handler.py b/grader_service/tests/handlers/test_permission_handler.py index fe721c90d..24cf87cb0 100644 --- a/grader_service/tests/handlers/test_permission_handler.py +++ b/grader_service/tests/handlers/test_permission_handler.py @@ -40,7 +40,8 @@ def get_scope(v): groups = set() for lecture_code, roles_list in default_roles_dict.items(): for role_entry in roles_list: - groups.add((lecture_code, role_entry["role"])) + if default_user.name in role_entry["members"]: + groups.add((lecture_code, role_entry["role"])) for p in permissions: t = (p["lecture_code"], get_scope(p["scope"])) diff --git a/grader_service/tests/handlers/test_roles_handler.py b/grader_service/tests/handlers/test_roles_handler.py new file mode 100644 index 000000000..ffaf679d3 --- /dev/null +++ b/grader_service/tests/handlers/test_roles_handler.py @@ -0,0 +1,423 @@ +import json +from http import HTTPStatus + +import pytest +from tornado.httpclient import HTTPClientError + +from grader_service.orm.takepart import Scope +from grader_service.server import GraderServer + + +async def test_get_roles_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_get_roles_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/?abc=123" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_get_roles_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 1 + assert roles[0]["user_id"] == default_user.id + + +async def test_get_roles_admin_wrong_lecture( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 999 + url = service_base_url + f"lectures/{l_id}/roles/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 0 + + +async def test_post_roles_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + ] + } + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_post_roles_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/?abc=123" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + ] + } + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_post_roles_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + roles = json.loads(response.body.decode()) + assert len(roles) == 1 + assert roles[0]["user_id"] == default_user.id + assert roles[0]["role"] == Scope.instructor + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor} # new role + ] + } + response = await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.CREATED + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 2 + assert roles[0]["user_id"] == default_user.id + assert roles[0]["role"] == Scope.student + assert roles[1]["user_id"] == default_admin.id + assert roles[1]["role"] == Scope.instructor + + +async def test_post_roles_admin_wrong_lecture( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 999 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + ] + } + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_delete_roles_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/?usernames={default_user.name}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_roles_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/?abc=123&usernames={default_user.name}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_delete_roles_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor} # new role + ] + } + response = await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.CREATED + + response = await http_server_client.fetch( + f"{url}?usernames={default_user.name}", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 1 + assert roles[0]["user_id"] == default_admin.id + assert roles[0]["role"] == Scope.instructor + + +async def test_delete_roles_admin_multiple( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor} # new role + ] + } + response = await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.CREATED + + response = await http_server_client.fetch( + f"{url}?usernames={default_user.name},{default_admin.name}", method="DELETE", + headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 0 + + +async def test_delete_roles_admin_multiple_wrong_user( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor} # new role + ] + } + response = await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.CREATED + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + f"{url}?usernames={default_user.name},{default_admin.name},windows", method="DELETE", + headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + roles = json.loads(response.body.decode()) + assert isinstance(roles, list) + assert len(roles) == 2 + + +async def test_delete_roles_admin_empty_usernames( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 2 + url = service_base_url + f"lectures/{l_id}/roles/" + + data = { + "users": [ + {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor} # new role + ] + } + response = await http_server_client.fetch( + url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.CREATED + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_delete_roles_admin_wrong_lecture( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + l_id = 999 + url = service_base_url + f"lectures/{l_id}/roles/usernames={default_user.name}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND diff --git a/grader_service/tests/handlers/test_submissions_handler.py b/grader_service/tests/handlers/test_submissions_handler.py index 13f6f555e..ac6a6832e 100644 --- a/grader_service/tests/handlers/test_submissions_handler.py +++ b/grader_service/tests/handlers/test_submissions_handler.py @@ -13,7 +13,7 @@ import isodate import pytest -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import sessionmaker, Session from tornado.httpclient import HTTPClientError from grader_service.api.models import AssignmentSettings, Submission @@ -29,24 +29,24 @@ from grader_service.orm.submission import Submission as SubmissionORM from grader_service.orm.takepart import Scope from grader_service.server import GraderServer - from .db_util import ( create_user_submission_with_repo, insert_assignments, insert_student, - insert_submission, + insert_submission, check_submission, create_all_git_repositories, check_git_repositories, ) +from ... import orm async def submission_test_setup(engine, default_user, a_id: int): insert_submission(engine, a_id, default_user.name, default_user.id) insert_submission(engine, a_id, default_user.name, default_user.id, with_properties=False) # should make no difference - insert_submission(engine, a_id, "user1", 2137) - insert_submission(engine, a_id, "user1", 2137, with_properties=False) + insert_submission(engine, a_id, "debian", 2) + insert_submission(engine, a_id, "debian", 2, with_properties=False) -async def test_get_submission_unauthorized( +async def test_get_submissions_lecture_unauthorized( service_base_url, http_server_client, default_user, @@ -57,8 +57,11 @@ async def test_get_submission_unauthorized( ): l_id = 1 # user is student a_id = 1 + s_id = 1 insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + check_submission(sql_alchemy_engine, a_id, s_id) + url = service_base_url + f"lectures/{l_id}/submissions/" with pytest.raises(HTTPClientError) as exc_info: @@ -66,7 +69,7 @@ async def test_get_submission_unauthorized( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 403 + assert e.code == HTTPStatus.FORBIDDEN @pytest.mark.parametrize( @@ -124,7 +127,7 @@ async def test_get_submissions( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submissions = json.loads(response.body.decode()) assert isinstance(submissions, list) assert len(submissions) == 2 @@ -150,7 +153,7 @@ async def test_get_submissions_format_csv( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK decoded_content = response.body.decode("utf-8") body_csv = csv.reader(decoded_content.splitlines(), delimiter=",") @@ -184,7 +187,7 @@ async def test_get_submissions_format_wrong( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST async def test_get_submissions_filter_wrong( @@ -207,7 +210,7 @@ async def test_get_submissions_filter_wrong( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST async def test_get_submissions_instructor_version( @@ -227,8 +230,8 @@ async def test_get_submissions_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" ) insert_submission(engine, a_id, default_user.name, user_id=default_user.id) @@ -241,7 +244,7 @@ async def test_get_submissions_instructor_version( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submissions = json.loads(response.body.decode()) assert isinstance(submissions, list) assert len(submissions) == 4 @@ -285,8 +288,8 @@ async def test_get_submissions_instructor_version_unauthorized( engine = sql_alchemy_engine url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" ) insert_submission(engine, a_id, username=default_user.name, user_id=default_user.id) @@ -294,12 +297,15 @@ async def test_get_submissions_instructor_version_unauthorized( engine, a_id, username=default_user.name, user_id=default_user.id, with_properties=False ) + check_submission(sql_alchemy_engine, a_id, 1) + check_submission(sql_alchemy_engine, a_id, 2) + with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 403 + assert e.code == HTTPStatus.FORBIDDEN async def test_get_submissions_latest_instructor_version( @@ -319,8 +325,8 @@ async def test_get_submissions_latest_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=latest" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=latest" ) insert_submission(engine, a_id, default_user.name, user_id=default_user.id) @@ -333,7 +339,7 @@ async def test_get_submissions_latest_instructor_version( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submissions = json.loads(response.body.decode()) assert isinstance(submissions, list) assert len(submissions) == 2 @@ -380,8 +386,8 @@ async def test_get_submissions_best_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=best" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=best" ) insert_submission( @@ -406,7 +412,7 @@ async def test_get_submissions_best_instructor_version( response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submissions = json.loads(response.body.decode()) assert isinstance(submissions, list) assert len(submissions) == 2 @@ -443,7 +449,7 @@ async def test_get_submissions_lecture_assignment_missmatch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_get_submissions_wrong_assignment_id( @@ -465,7 +471,116 @@ async def test_get_submissions_wrong_assignment_id( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_get_submissions_deleted( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_user, +): + l_id = 1 + a_id = 1 + await submission_test_setup(sql_alchemy_engine, default_user, a_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1" + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions" + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + submissions = json.loads(response.body.decode()) + assert isinstance(submissions, list) + assert len(submissions) == 1 + assert submissions[0]["user_id"] == default_user.id + Submission.from_dict(submissions[0]) + + +async def test_get_submissions_admin_deleted( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 1 + a_id = 1 + await submission_test_setup(sql_alchemy_engine, default_user, a_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1" + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions" + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + submissions = json.loads(response.body.decode()) + assert isinstance(submissions, list) + assert len(submissions) == 2 + assert submissions[0]["user_id"] == default_admin.id + assert submissions[1]["user_id"] == default_admin.id + Submission.from_dict(submissions[0]) + Submission.from_dict(submissions[1]) + + +async def test_get_submissions_admin_deleted_instructor_version( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, + default_user, + default_admin, +): + l_id = 1 + a_id = 1 + await submission_test_setup(sql_alchemy_engine, default_user, a_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1" + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions?instructor-version=true" + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + submissions = json.loads(response.body.decode()) + assert isinstance(submissions, list) + assert len(submissions) == 4 + assert submissions[0]["user_id"] == default_user.id + assert submissions[1]["user_id"] == default_user.id + assert submissions[2]["user_id"] == default_admin.id + assert submissions[3]["user_id"] == default_admin.id + Submission.from_dict(submissions[0]) + Submission.from_dict(submissions[1]) + Submission.from_dict(submissions[2]) + Submission.from_dict(submissions[3]) async def test_get_submission( @@ -490,14 +605,14 @@ async def test_get_submission( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND insert_submission(engine, a_id, default_user.name, default_user.id) response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submission_dict = json.loads(response.body.decode()) Submission.from_dict(submission_dict) @@ -524,7 +639,7 @@ async def test_get_submission_assignment_lecture_missmatch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_get_submission_assignment_submission_missmatch( @@ -543,6 +658,8 @@ async def test_get_submission_assignment_submission_missmatch( insert_assignments(engine, l_id) insert_submission(engine, a_id, default_user.name, default_user.id) + check_submission(engine, a_id, 1) + a_id = 1 # this assignment has no submissions url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/" @@ -551,7 +668,7 @@ async def test_get_submission_assignment_submission_missmatch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_get_submission_wrong_submission( @@ -570,6 +687,8 @@ async def test_get_submission_wrong_submission( insert_assignments(engine, l_id) insert_submission(engine, a_id, default_user.name, default_user.id) + check_submission(engine, a_id, 1) + a_id = 1 # this assignment has no submissions url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/99/" @@ -578,7 +697,65 @@ async def test_get_submission_wrong_submission( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_get_submission_student_from_another_student( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_admin, +): + l_id = 1 # user has to be student + a_id = 1 + s_id = 1 + engine = sql_alchemy_engine + insert_submission(engine, a_id, default_admin.name, default_admin.id) + + check_submission(engine, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}/" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_get_submission_admin_from_another_student( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, + default_admin, +): + l_id = 1 # admon has no role + a_id = 1 + s_id = 1 + engine = sql_alchemy_engine + insert_submission(engine, a_id, default_user.name, default_user.id) + + check_submission(engine, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + submission_dict = json.loads(response.body.decode()) + Submission.from_dict(submission_dict) async def test_put_submission( @@ -616,7 +793,7 @@ async def test_put_submission( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(pre_submission.to_dict()), ) - assert response.code == 200 + assert response.code == HTTPStatus.OK submission_dict = json.loads(response.body.decode()) submission = Submission.from_dict(submission_dict) assert submission.id == s_id @@ -664,7 +841,7 @@ async def test_put_submission_lecture_assignment_missmatch( body=json.dumps(pre_submission.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_put_submission_assignment_submission_missmatch( @@ -704,7 +881,7 @@ async def test_put_submission_assignment_submission_missmatch( body=json.dumps(pre_submission.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_put_submission_wrong_submission( @@ -744,7 +921,7 @@ async def test_put_submission_wrong_submission( body=json.dumps(pre_submission.to_dict()), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_delete_own_submission_by_student( @@ -778,7 +955,40 @@ async def test_delete_own_submission_by_student( assert submission.deleted == DeleteState.deleted -async def test_delete_submission_from_another_student_fails( +async def test_delete_submission( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + sql_alchemy_engine, + default_user, +): + l_id = 1 # default user is student + a_id = 1 + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + check_submission(sql_alchemy_engine, a_id, s_id) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_delete_submission_deleted_submission( service_base_url, http_server_client, default_user, @@ -789,10 +999,19 @@ async def test_delete_submission_from_another_student_fails( ): l_id = 1 # default user is student a_id = 1 - # The submission does NOT belong to the default user: - insert_submission(sql_alchemy_engine, a_id, "other_student", 2137) + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + check_submission(sql_alchemy_engine, a_id, s_id) - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/" with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} @@ -801,7 +1020,7 @@ async def test_delete_submission_from_another_student_fails( assert e.code == HTTPStatus.NOT_FOUND -async def test_delete_submission_with_feedback_fails( +async def test_delete_submission_not_found( service_base_url, http_server_client, default_user, @@ -812,23 +1031,107 @@ async def test_delete_submission_with_feedback_fails( ): l_id = 1 # default user is student a_id = 1 - insert_submission( - sql_alchemy_engine, a_id, default_user.name, feedback=FeedbackStatus.GENERATED + s_id = 999 + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + + check_submission(sql_alchemy_engine, a_id, s_id, should_exist=False) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_delete_submission_student_from_another_student( + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, +): + l_id = 1 # default user is student + a_id = 1 + s_id = 1 + + # The submission does NOT belong to the default user: + insert_submission(sql_alchemy_engine, a_id, "debian", 2) + check_submission(sql_alchemy_engine, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_delete_submission_admin_from_another_student( + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, +): + l_id = 1 # admin has no role + a_id = 1 + s_id = 1 + + # The submission does NOT belong to the admin: + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) + assert response.code == HTTPStatus.OK - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/" + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + submission_dict = json.loads(response.body.decode()) + Submission.from_dict(submission_dict) - with pytest.raises( - HTTPClientError, match="Only submissions without feedback can be deleted." - ) as exc_info: + session: Session = sessionmaker(sql_alchemy_engine)() + submission = session.query(orm.Submission).filter(orm.Submission.id == s_id).first() + assert submission.deleted == DeleteState.deleted + + +async def test_delete_submission_with_feedback( + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, +): + l_id = 1 # default user is student + a_id = 1 + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, feedback=FeedbackStatus.GENERATED) + check_submission(sql_alchemy_engine, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN + assert e.message == "Only submissions without feedback can be deleted." -async def test_delete_submission_after_deadline_fails( +async def test_delete_submission_after_deadline( service_base_url, http_server_client, default_user, @@ -839,6 +1142,7 @@ async def test_delete_submission_after_deadline_fails( ): l_id = 1 # default user is student a_id = 1 + s_id = 1 session = sessionmaker(sql_alchemy_engine)() assign = session.query(AssignmentORM).get(1) @@ -848,43 +1152,131 @@ async def test_delete_submission_after_deadline_fails( insert_submission(sql_alchemy_engine, a_id, default_user.name) - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/" - with pytest.raises( - HTTPClientError, match="Submission can't be deleted, due date of assigment has passed." - ) as exc_info: + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN + assert e.message == "Submission can't be deleted, due date of assigment has passed." -async def test_delete_submission_twice_fails( +async def test_delete_submission_hard( + app: GraderServer, service_base_url, http_server_client, - default_user, default_token, + default_roles, + default_admin_login, sql_alchemy_engine, + default_user, +): + l_id = 1 # admin has no role + a_id = 1 + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id, with_properties=True, + with_logs=True) + + session: Session = sessionmaker(sql_alchemy_engine)() + submissions = session.query(orm.Submission).filter(orm.Submission.id == s_id).all() + assert len(submissions) == 1 + submission_properties = session.query(orm.SubmissionProperties).filter( + orm.SubmissionProperties.sub_id == s_id).all() + assert len(submission_properties) == 1 + submission_logs = session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + assert len(submission_logs) == 1 + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + + response = await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + check_submission(sql_alchemy_engine, a_id, s_id, should_exist=False) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + submissions = session.query(orm.Submission).filter(orm.Submission.id == s_id).all() + assert len(submissions) == 0 + submission_properties = session.query(orm.SubmissionProperties).filter( + orm.SubmissionProperties.sub_id == s_id).all() + assert len(submission_properties) == 0 + submission_logs = session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + assert len(submission_logs) == 0 + + +async def test_delete_submission_hard_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, default_roles, default_user_login, + sql_alchemy_engine, + default_user, ): l_id = 1 # default user is student a_id = 1 - insert_submission(sql_alchemy_engine, a_id, default_user.name) + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id, with_properties=True, + with_logs=True) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_submission_hard_with_files( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + sql_alchemy_engine, + default_user, +): + l_id = 1 # admin has no role + l_code = "21wle1" + a_id = 1 + s_id = 1 + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + create_all_git_repositories(app, default_user, l_id, l_code, a_id, s_id) + + url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/" response = await http_server_client.fetch( - url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} ) assert response.code == HTTPStatus.OK + check_submission(sql_alchemy_engine, a_id, s_id, should_exist=False) + with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value assert e.code == HTTPStatus.NOT_FOUND + check_git_repositories(app, default_user, l_code, a_id, s_id, + True, True, True, False, False, False, False) + async def test_post_submission_by_student( service_base_url, @@ -988,7 +1380,7 @@ async def test_post_submission_git_repo_not_found( body=json.dumps({"commit_hash": secrets.token_hex(20)}), ) e = exc_info.value - assert e.code == 422 + assert e.code == HTTPStatus.UNPROCESSABLE_ENTITY assert e.message == "User git repository not found" @@ -1019,7 +1411,7 @@ async def test_post_submission_commit_hash_not_found( body=json.dumps(pre_submission), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST assert e.message == "Commit hash not found in body" @@ -1101,11 +1493,11 @@ async def test_submission_properties( headers={"Authorization": f"Token {default_token}"}, body=json.dumps(prop), ) - assert put_response.code == 200 + assert put_response.code == HTTPStatus.OK get_response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) - assert get_response.code == 200 + assert get_response.code == HTTPStatus.OK assignment_props = json.loads(get_response.body.decode()) assert assignment_props == prop @@ -1138,7 +1530,7 @@ async def test_submission_properties_not_correct( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 400 + assert e.code == HTTPStatus.BAD_REQUEST assert e.message == "Cannot parse properties file!" @@ -1170,14 +1562,14 @@ async def test_submission_properties_lecture_assignment_missmatch( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_submission_properties_assignment_submission_missmatch( @@ -1209,14 +1601,14 @@ async def test_submission_properties_assignment_submission_missmatch( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND async def test_submission_properties_wrong_submission( @@ -1249,7 +1641,7 @@ async def test_submission_properties_wrong_submission( body=json.dumps(prop), ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == f"Submission with id {sub_id} was not found" with pytest.raises(HTTPClientError) as exc_info: @@ -1257,7 +1649,7 @@ async def test_submission_properties_wrong_submission( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == "Properties of submission were not found" @@ -1284,7 +1676,7 @@ async def test_submission_properties_not_found( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value - assert e.code == 404 + assert e.code == HTTPStatus.NOT_FOUND assert e.message == "Properties of submission were not found" @@ -1379,8 +1771,8 @@ async def test_submission_cannot_edit_submission_created_by_instructor( url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/edit" with pytest.raises( - HTTPClientError, - match="This repo cannot be edited or reset, because it was created by instructor", + HTTPClientError, + match="This repo cannot be edited or reset, because it was created by instructor", ) as exc_info: await http_server_client.fetch( url, @@ -1390,3 +1782,182 @@ async def test_submission_cannot_edit_submission_created_by_instructor( ) e = exc_info.value assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_get_submissions_username( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_user, + default_admin, +): + a_id = 1 + url = service_base_url + f"users/{default_user.name}/submissions" + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + insert_submission(sql_alchemy_engine, a_id, default_admin.name, default_admin.id) + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + submissions = json.loads(response.body.decode()) + assert isinstance(submissions, list) + assert len(submissions) == 1 + assert submissions[0]["user_id"] == default_user.id + Submission.from_dict(submissions[0]) + + +async def test_get_submissions_username_student_from_another_student( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_user, + default_admin, +): + a_id = 1 + url = service_base_url + f"users/{default_admin.name}/submissions" + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + insert_submission(sql_alchemy_engine, a_id, default_admin.name, default_admin.id) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_get_submissions_username_admin_from_another_student( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, + default_user, + default_admin, +): + a_id = 1 + url = service_base_url + f"users/{default_user.name}/submissions" + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + insert_submission(sql_alchemy_engine, a_id, default_admin.name, default_admin.id) + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + submissions = json.loads(response.body.decode()) + assert isinstance(submissions, list) + assert len(submissions) == 1 + assert submissions[0]["user_id"] == default_user.id + Submission.from_dict(submissions[0]) + + +async def test_get_submissions_username_admin_user_not_found( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_admin_login, + default_user, + default_admin, +): + a_id = 1 + username = "windows" + url = service_base_url + f"users/{username}/submissions" + + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + insert_submission(sql_alchemy_engine, a_id, default_admin.name, default_admin.id) + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_get_submissions_username_format_csv( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, +): + a_id = 1 + url = service_base_url + f"users/{default_user.name}/submissions?format=csv" + await submission_test_setup(sql_alchemy_engine, default_user, a_id) + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + decoded_content = response.body.decode("utf-8") + + body_csv = csv.reader(decoded_content.splitlines(), delimiter=",") + submissions = list(body_csv) + # Delete column description + submissions.pop(0) + + assert len(submissions) == 2 + assert submissions[0][4] == str(default_user.id) + assert submissions[1][4] == str(default_user.id) + assert submissions[0][5] == default_user.name + assert submissions[1][5] == default_user.name + + +async def test_get_submissions_username_format_wrong( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}/submissions?format=abc" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_get_submissions_username_filter_wrong( + app: GraderServer, + service_base_url, + http_server_client, + default_user, + default_token, + sql_alchemy_engine, + default_roles, + default_user_login, +): + url = service_base_url + f"users/{default_user.name}/submissions?filter=abc" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST diff --git a/grader_service/tests/handlers/test_users_handler.py b/grader_service/tests/handlers/test_users_handler.py new file mode 100644 index 000000000..46ed23496 --- /dev/null +++ b/grader_service/tests/handlers/test_users_handler.py @@ -0,0 +1,374 @@ +import json +from http import HTTPStatus + +import pytest +from sqlalchemy.orm import Session, sessionmaker +from tornado.httpclient import HTTPClientError + +from grader_service import orm +from grader_service.server import GraderServer +from grader_service.tests.handlers.db_util import insert_submission, create_all_git_repositories, \ + check_git_repositories + + +async def test_get_users_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + url = service_base_url + "users/" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_get_users_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + "users/?abc=123" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_get_users_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + url = service_base_url + "users/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + users = json.loads(response.body.decode()) + assert isinstance(users, list) + assert len(users) == 2 + assert users[0]["name"] == default_user.name + assert users[1]["name"] == default_admin.name + + +async def test_get_user_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}/" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_get_user_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}/?abc=123" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_get_user_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + url = service_base_url + f"users/{default_user.name}" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + user = json.loads(response.body.decode()) + assert user["name"] == default_user.name + + +async def test_get_user_admin_wrong_user( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + "users/windows" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_put_user_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}" + + data = {"name": default_user.name, "display_name": "New Name"} + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_put_user_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}/?abc=123" + + data = {"name": default_user.name, "display_name": "New Name"} + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_put_user_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, +): + url = service_base_url + f"users/{default_user.name}" + + data = {"name": default_user.name, "display_name": "New Name"} + response = await http_server_client.fetch( + url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + assert response.code == HTTPStatus.OK + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + user = json.loads(response.body.decode()) + assert user["name"] == default_user.name + assert user["display_name"] == "New Name" + + +async def test_put_user_admin_wrong_user( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + "users/windows" + + data = {"name": default_user.name, "display_name": "New Name"} + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + +async def test_delete_user_unauthorized( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.FORBIDDEN + + +async def test_delete_user_admin_unknown_parameter( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + f"users/{default_user.name}/?abc=123" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.BAD_REQUEST + + +async def test_delete_user_admin( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, + sql_alchemy_engine, +): + username = default_user.name + url = service_base_url + f"users/{username}/" + + response = await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + session: Session = sessionmaker(sql_alchemy_engine)() + old_user = session.query(orm.User).filter(orm.User.name == username).first() + + orm.APIToken.new(user=old_user) + + auth_code = orm.OAuthCode( + code="abc", + expires_at=int(orm.OAuthCode.now() + 300), + scopes=[orm.takepart.Scope.student], + redirect_uri="redirect_uri", + session_id="1234", + user_id=old_user.id, + ) + session.add(auth_code) + session.commit() + + l_id = 1 # admin has no role + l_code = "21wle1" + a_id = 1 + s_id = 1 + insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) + create_all_git_repositories(app, default_user, l_id, l_code, a_id, s_id) + + old_submissions = session.query(orm.Submission).filter(orm.Submission.user_id == old_user.id).all() + old_roles = session.query(orm.Role).filter(orm.Role.user_id == old_user.id).all() + old_api_tokens = session.query(orm.APIToken).filter(orm.APIToken.user_id == old_user.id).all() + old_auth_codes = session.query(orm.OAuthCode).filter(orm.OAuthCode.user_id == old_user.id).all() + + assert old_user.name == username + assert len(old_submissions) == 1 + assert len(old_roles) == 3 + assert len(old_api_tokens) == 1 + assert len(old_auth_codes) == 1 + + response = await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + assert response.code == HTTPStatus.OK + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="GET", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND + + user = session.query(orm.User).filter(orm.User.name == username).first() + submissions = session.query(orm.Submission).filter(orm.Submission.user_id == old_user.id).all() + roles = session.query(orm.Role).filter(orm.Role.user_id == old_user.id).all() + api_tokens = session.query(orm.APIToken).filter(orm.APIToken.user_id == old_user.id).all() + auth_codes = session.query(orm.OAuthCode).filter(orm.OAuthCode.user_id == old_user.id).all() + + assert user is None + assert len(submissions) == 0 + assert len(roles) == 0 + assert len(api_tokens) == 0 + assert len(auth_codes) == 0 + + check_git_repositories(app, default_user, l_code, a_id, s_id, + True, True, True, False, False, False, False) + + +async def test_delete_user_admin_wrong_user( + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, +): + url = service_base_url + "users/windows/" + + with pytest.raises(HTTPClientError) as exc_info: + await http_server_client.fetch( + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} + ) + e = exc_info.value + assert e.code == HTTPStatus.NOT_FOUND diff --git a/grader_service/tests/migrate/test_migrate.py b/grader_service/tests/migrate/test_migrate.py index fdc44a1d1..1b56e7a09 100644 --- a/grader_service/tests/migrate/test_migrate.py +++ b/grader_service/tests/migrate/test_migrate.py @@ -98,6 +98,7 @@ def test_migration_upgrade_downgrade(alembic_cfg, migration): """ cfg, db_url = alembic_cfg engine = create_engine(db_url) + engine2 = None conn = engine.connect() trans = conn.begin() try: @@ -145,7 +146,8 @@ def test_migration_upgrade_downgrade(alembic_cfg, migration): assert not user_tables, "User tables not dropped after downgrade to base" finally: engine.dispose() - engine2.dispose() + if engine2 is not None: + engine2.dispose() @pytest.mark.parametrize("migration", get_migration_scripts()) @@ -212,6 +214,8 @@ def test_migration_upgrade_downgrade_with_data_from_prev_revision(alembic_cfg, m """ cfg, db_url = alembic_cfg engine = create_engine(db_url) + engine2 = None + engine3 = None conn = engine.connect() trans = conn.begin() @@ -280,9 +284,11 @@ def test_migration_upgrade_downgrade_with_data_from_prev_revision(alembic_cfg, m assert data_before_upgrade == data_after_downgrade, "Data changed after downgrade!" except AssertionError as e: # If the tested migration is part of the not lossless migration do not throw the error - if migration.revision not in ("f1ae66d52ad9", "fc5d2febe781"): + if migration.revision not in ("f1ae66d52ad9", "fc5d2febe781", "4a88dacd888f"): raise e finally: engine.dispose() - engine2.dispose() - engine3.dispose() + if engine2 is not None: + engine2.dispose() + if engine3 is not None: + engine3.dispose() From 836b3ca09d9b08e43b399f98eeaea3e6285c9a89 Mon Sep 17 00:00:00 2001 From: BigFlagBurito <102980482+BigFlagBurito@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:21:20 +0100 Subject: [PATCH 6/6] Apply code formatting and sort imports --- grader_service/auth/auth.py | 8 +- grader_service/handlers/__init__.py | 4 +- grader_service/handlers/assignment.py | 13 +- grader_service/handlers/base_handler.py | 23 +- grader_service/handlers/lectures.py | 7 +- grader_service/handlers/roles.py | 23 +- grader_service/handlers/submissions.py | 23 +- grader_service/handlers/users.py | 2 + grader_service/main.py | 7 +- .../4a88dacd888f_add_ondelete_cascade.py | 283 ++++++++++----- ...c153_added_oauth_provider_functionality.py | 4 +- grader_service/orm/__init__.py | 6 +- grader_service/orm/assignment.py | 5 +- grader_service/orm/submission.py | 14 +- grader_service/orm/takepart.py | 2 +- grader_service/tests/handlers/db_util.py | 132 +++++-- .../tests/handlers/test_assignment_handler.py | 68 ++-- grader_service/tests/handlers/test_git.py | 8 +- .../tests/handlers/test_lectures_handler.py | 40 ++- .../tests/handlers/test_roles_handler.py | 324 ++++++++++-------- .../handlers/test_submissions_handler.py | 97 ++++-- .../tests/handlers/test_users_handler.py | 256 +++++++------- 22 files changed, 875 insertions(+), 474 deletions(-) diff --git a/grader_service/auth/auth.py b/grader_service/auth/auth.py index 3824aa937..5175ac529 100644 --- a/grader_service/auth/auth.py +++ b/grader_service/auth/auth.py @@ -9,8 +9,8 @@ from grader_service.utils import maybe_future, url_path_join -from .login import LoginHandler from ..handlers.base_handler import BaseHandler +from .login import LoginHandler class Authenticator(LoggingConfigurable): @@ -512,7 +512,7 @@ async def get_authenticated_user(self, handler, data): self.log.warning("User %r not allowed.", username) return - async def refresh_user(self, user, handler:BaseHandler=None): + async def refresh_user(self, user, handler: BaseHandler = None): """Refresh auth data for a given user Allows refreshing or invalidating auth data. @@ -538,7 +538,9 @@ async def refresh_user(self, user, handler:BaseHandler=None): Any fields not present will be left unchanged. This can include updating `.admin` or `.auth_state` fields. """ - user.is_admin = handler.authenticator.is_admin(handler=self, authentication={'name': user.name}) + user.is_admin = handler.authenticator.is_admin( + handler=self, authentication={"name": user.name} + ) return True def is_admin(self, handler, authentication): diff --git a/grader_service/handlers/__init__.py b/grader_service/handlers/__init__.py index 46cb93ea3..878b97c96 100644 --- a/grader_service/handlers/__init__.py +++ b/grader_service/handlers/__init__.py @@ -14,9 +14,9 @@ health, lectures, permission, - submissions, roles, - users + submissions, + users, ) from grader_service.handlers.handler_utils import GitRepoType diff --git a/grader_service/handlers/assignment.py b/grader_service/handlers/assignment.py index 684d3e6e6..c01a1ae29 100644 --- a/grader_service/handlers/assignment.py +++ b/grader_service/handlers/assignment.py @@ -97,14 +97,18 @@ async def get(self, lecture_id: int): .all() ) else: - assignments = [a for a in role.lecture.assignments if (a.deleted == DeleteState.active)] + assignments = [ + a for a in role.lecture.assignments if (a.deleted == DeleteState.active) + ] # Handle the case that the user wants to include submissions include_submissions = self.get_argument("include-submissions", "true") == "true" if include_submissions: assignids = [a.id for a in assignments] if self.user.is_admin: - results = self.session.query(Submission).filter(Submission.assignid.in_(assignids)).all() + results = ( + self.session.query(Submission).filter(Submission.assignid.in_(assignids)).all() + ) else: results = ( self.session.query(Submission) @@ -275,7 +279,6 @@ async def get(self, lecture_id: int, assignment_id: int): raise HTTPError(HTTPStatus.NOT_FOUND, reason="Assignment not found") self.write_json(assignment) - @authorize([Scope.instructor, Scope.admin]) async def delete(self, lecture_id: int, assignment_id: int): """Soft or Hard-Deletes a specific assignment. @@ -297,7 +300,9 @@ async def delete(self, lecture_id: int, assignment_id: int): if hard_delete: if not self.user.is_admin: - raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete assignment.") + raise HTTPError( + HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete assignment." + ) self.delete_assignment_files(assignment) self.session.delete(assignment) diff --git a/grader_service/handlers/base_handler.py b/grader_service/handlers/base_handler.py index df9bcf261..2c274af54 100644 --- a/grader_service/handlers/base_handler.py +++ b/grader_service/handlers/base_handler.py @@ -70,10 +70,13 @@ def check_authorization( raise HTTPError(403) elif lecture_id is None and "/lectures" in self.request.path and self.request.method == "GET": return True - if re.match(r"/api/users/(?P[^/]+)/submissions/?", self.request.path) and self.request.method == "GET": + if ( + re.match(r"/api/users/(?P[^/]+)/submissions/?", self.request.path) + and self.request.method == "GET" + ): return True - is_admin = self.authenticator.is_admin(handler=self, authentication={'name': self.user.name}) + is_admin = self.authenticator.is_admin(handler=self, authentication={"name": self.user.name}) role = self.session.get(Role, (self.user.id, lecture_id)) @@ -835,7 +838,7 @@ def get_latest_submissions( subquery, (Submission.user_id == subquery.c.user_id) & (Submission.date == subquery.c.max_date) - & (Submission.assignid == assignment_id) + & (Submission.assignid == assignment_id), ) .order_by(Submission.id) ) @@ -885,7 +888,7 @@ def get_best_submissions( subquery, (Submission.user_id == subquery.c.user_id) & (Submission.score == subquery.c.max_score) - & (Submission.assignid == assignment_id) + & (Submission.assignid == assignment_id), ) .order_by(Submission.id) ) @@ -916,10 +919,14 @@ def delete_assignment_files(self, assignment: Assignment): def delete_submission_files(self, submission: Submission): # delete all associated directories of the submission assignment_path = os.path.abspath( - os.path.join(self.gitbase, submission.assignment.lecture.code, str(submission.assignment.id)) + os.path.join( + self.gitbase, submission.assignment.lecture.code, str(submission.assignment.id) + ) ) tmp_assignment_path = os.path.abspath( - os.path.join(self.tmpbase, submission.assignment.lecture.code, str(submission.assignment.id)) + os.path.join( + self.tmpbase, submission.assignment.lecture.code, str(submission.assignment.id) + ) ) target_names = {submission.user.name, str(submission.id)} matching_dirs = [] @@ -968,7 +975,9 @@ def construct_git_dir( elif repo_type in {GitRepoType.AUTOGRADE, GitRepoType.FEEDBACK}: type_path = os.path.join(assignment_path, repo_type, "user") if repo_type == GitRepoType.AUTOGRADE: - if (submission is None) or (not self.user.is_admin and self.get_role(lecture.id).role < Scope.tutor): + if (submission is None) or ( + not self.user.is_admin and self.get_role(lecture.id).role < Scope.tutor + ): raise HTTPError(403) path = os.path.join(type_path, submission.user.name) else: diff --git a/grader_service/handlers/lectures.py b/grader_service/handlers/lectures.py index 5cfa06157..28e8f5960 100644 --- a/grader_service/handlers/lectures.py +++ b/grader_service/handlers/lectures.py @@ -144,7 +144,9 @@ async def delete(self, lecture_id: int): if hard_delete: if not self.user.is_admin: - raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete lecture.") + raise HTTPError( + HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete lecture." + ) self.delete_lecture_files(lecture) self.session.delete(lecture) @@ -164,8 +166,7 @@ async def delete(self, lecture_id: int): if a.status in ["released", "complete"]: self.session.rollback() raise HTTPError( - HTTPStatus.CONFLICT, - "Cannot delete released or completed assignments!", + HTTPStatus.CONFLICT, "Cannot delete released or completed assignments!" ) a.deleted = DeleteState.deleted self.session.commit() diff --git a/grader_service/handlers/roles.py b/grader_service/handlers/roles.py index 7889902e6..07461d04c 100644 --- a/grader_service/handlers/roles.py +++ b/grader_service/handlers/roles.py @@ -86,16 +86,19 @@ async def post(self, lecture_id: int): roles = [] for user_req in body["users"]: - user = self.session.query(User).filter(User.name == user_req["username"]).one_or_none() if user is None: self.session.rollback() - raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {user_req['username']} not found") - - role = self.session.query(Role) \ - .filter(Role.user_id == user.id) \ - .filter(Role.lectid == lecture_id) \ + raise HTTPError( + HTTPStatus.NOT_FOUND, reason=f"User {user_req['username']} not found" + ) + + role = ( + self.session.query(Role) + .filter(Role.user_id == user.id) + .filter(Role.lectid == lecture_id) .one_or_none() + ) if role is None: role = Role() role.user_id = user.id @@ -144,10 +147,12 @@ async def delete(self, lecture_id: int): self.session.rollback() raise HTTPError(HTTPStatus.NOT_FOUND, reason=f"User {username} was not found") - role = self.session.query(Role) \ - .filter(Role.user_id == user.id) \ - .filter(Role.lectid == lecture_id) \ + role = ( + self.session.query(Role) + .filter(Role.user_id == user.id) + .filter(Role.lectid == lecture_id) .one_or_none() + ) if role is not None: self.session.delete(role) self.session.commit() diff --git a/grader_service/handlers/submissions.py b/grader_service/handlers/submissions.py index e4bb13b37..bfab0c5aa 100644 --- a/grader_service/handlers/submissions.py +++ b/grader_service/handlers/submissions.py @@ -188,7 +188,9 @@ async def get(self, username: str): self.validate_parameters("format") response_format = self.get_argument("format", "json") if response_format not in ["json", "csv"]: - raise HTTPError(HTTPStatus.BAD_REQUEST, reason="Response format can either be 'json' or 'csv'") + raise HTTPError( + HTTPStatus.BAD_REQUEST, reason="Response format can either be 'json' or 'csv'" + ) if not self.user.is_admin: if username != self.user.name: @@ -533,7 +535,11 @@ async def get(self, lecture_id: int, assignment_id: int, submission_id: int): lecture_id, assignment_id, submission_id ) submission = self.get_submission(lecture_id, assignment_id, submission_id) - if not self.user.is_admin and self.get_role(lecture_id).role == Scope.student and submission.user_id != self.user.id: + if ( + not self.user.is_admin + and self.get_role(lecture_id).role == Scope.student + and submission.user_id != self.user.id + ): raise HTTPError(HTTPStatus.NOT_FOUND) self.write_json(submission) @@ -601,19 +607,26 @@ async def delete(self, lecture_id: int, assignment_id: int, submission_id: int): if hard_delete: if not self.user.is_admin: - raise HTTPError(HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete submission.") + raise HTTPError( + HTTPStatus.FORBIDDEN, reason="Only Admins can hard-delete submission." + ) self.delete_submission_files(submission) self.session.delete(submission) self.session.commit() else: # Do not allow students to delete other users' submissions - if not self.user.is_admin and self.get_role(lecture_id).role < Scope.instructor and submission.user_id != self.user.id: + if ( + not self.user.is_admin + and self.get_role(lecture_id).role < Scope.instructor + and submission.user_id != self.user.id + ): raise HTTPError(HTTPStatus.NOT_FOUND, reason="Submission to delete not found.") if submission.feedback_status != FeedbackStatus.NOT_GENERATED: raise HTTPError( - HTTPStatus.FORBIDDEN, reason="Only submissions without feedback can be deleted." + HTTPStatus.FORBIDDEN, + reason="Only submissions without feedback can be deleted.", ) # if assignment has deadline and it has already passed diff --git a/grader_service/handlers/users.py b/grader_service/handlers/users.py index 586d5ab3b..4e55283c7 100644 --- a/grader_service/handlers/users.py +++ b/grader_service/handlers/users.py @@ -16,6 +16,7 @@ class UserBaseHandler(GraderBaseHandler): """ Tornado Handler class for http requests to /users. """ + @authorize([Scope.admin]) async def get(self): """ @@ -27,6 +28,7 @@ async def get(self): self.set_status(HTTPStatus.OK) self.write_json(user) + @register_handler(r"\/api\/users\/(?P[^\/]+)\/?", VersionSpecifier.ALL) class UserObjectBaseHandler(GraderBaseHandler): """ diff --git a/grader_service/main.py b/grader_service/main.py index 39f441298..e2b04db0e 100644 --- a/grader_service/main.py +++ b/grader_service/main.py @@ -18,7 +18,7 @@ import tornado import uvloop as uvloop from jupyterhub.log import log_request -from sqlalchemy import create_engine, Engine, event +from sqlalchemy import Engine, create_engine, event from sqlalchemy.orm import scoped_session, sessionmaker from tornado.httpserver import HTTPServer from traitlets import ( @@ -41,6 +41,7 @@ from grader_service import __version__ from grader_service.auth.auth import Authenticator + # run __init__.py to register handlers from grader_service.auth.dummy import DummyAuthenticator from grader_service.autograding.celery.app import CeleryApp @@ -406,7 +407,9 @@ def init_roles(self): user = db.query(User).filter(User.name == username).one_or_none() if user is None: - self.log.info(f"Adding new user with username {username} and display name {display_name}") + self.log.info( + f"Adding new user with username {username} and display name {display_name}" + ) user = User() user.name = username user.display_name = display_name diff --git a/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py b/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py index aaf1df974..e7b0574c1 100644 --- a/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py +++ b/grader_service/migrate/versions/4a88dacd888f_add_ondelete_cascade.py @@ -5,14 +5,26 @@ Create Date: 2025-11-04 11:55:10.513853 """ + from typing import Dict, List from alembic import op -from sqlalchemy import text, inspect, Inspector, Table, MetaData, Column, String, select, and_, Connection +from sqlalchemy import ( + Column, + Connection, + Inspector, + MetaData, + String, + Table, + and_, + inspect, + select, + text, +) # revision identifiers, used by Alembic. -revision = '4a88dacd888f' -down_revision = '9983ef1fda76' +revision = "4a88dacd888f" +down_revision = "9983ef1fda76" branch_labels = None depends_on = None @@ -39,7 +51,7 @@ def _drop_all_foreign_keys(batch_op, connection, table_name: str): inspector = Inspector.from_engine(connection) fks = inspector.get_foreign_keys(table_name) for fk in fks: - if fk['name'] is not None: + if fk["name"] is not None: batch_op.drop_constraint(fk["name"], type_="foreignkey") @@ -56,7 +68,8 @@ def _store_old_fk(conn, table_name, local_cols, referred_table, old_name): """Store information about an existing foreign key in a helper table.""" metadata = MetaData() fk_table = Table( - "alembic_fk_metadata", metadata, + "alembic_fk_metadata", + metadata, Column("table_name", String, primary_key=True), Column("column_name", String, primary_key=True), Column("referred_table", String), @@ -88,7 +101,9 @@ def _get_stored_old_fk(conn, table_name, local_cols, referred_table): return result[0] if result else None -def _upgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk_definitions: List[Dict]) -> None: +def _upgrade_recreate_foreign_keys( + connection: Connection, table_name: str, fk_definitions: List[Dict] +) -> None: """ Recreate foreign key constraints on a table during a migration. @@ -122,11 +137,13 @@ def _upgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk_d referent_table=fk["referred_table"], local_cols=fk["local_cols"], remote_cols=fk["remote_cols"], - ondelete="CASCADE" + ondelete="CASCADE", ) -def _downgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk_definitions: List[Dict]) -> None: +def _downgrade_recreate_foreign_keys( + connection: Connection, table_name: str, fk_definitions: List[Dict] +) -> None: """ Restore previous foreign key constraints during a downgrade migration. @@ -148,7 +165,9 @@ def _downgrade_recreate_foreign_keys(connection: Connection, table_name: str, fk """ with op.batch_alter_table(table_name) as batch_op: for fk in fk_definitions: - old_fk_name = _get_stored_old_fk(connection, table_name, fk["local_cols"], fk["referred_table"]) + old_fk_name = _get_stored_old_fk( + connection, table_name, fk["local_cols"], fk["referred_table"] + ) if connection.dialect.name != "sqlite": batch_op.drop_constraint(fk["new_constraint_name"], type_="foreignkey") if old_fk_name is None: @@ -169,47 +188,80 @@ def upgrade(): connection.execute(text("PRAGMA foreign_keys=OFF;")) _upgrade_recreate_foreign_keys( - connection=connection, table_name="assignment", + connection=connection, + table_name="assignment", fk_definitions=[ - {"new_constraint_name": "fk_assignment_lectid", "referred_table": "lecture", - "local_cols": ["lectid"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_assignment_lectid", + "referred_table": "lecture", + "local_cols": ["lectid"], + "remote_cols": ["id"], + } + ], ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="submission", + connection=connection, + table_name="submission", fk_definitions=[ - {"new_constraint_name": "fk_submission_assignid", "referred_table": "assignment", - "local_cols": ["assignid"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_submission_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_assignid", + "referred_table": "assignment", + "local_cols": ["assignid"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_submission_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + ], ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="submission_logs", + connection=connection, + table_name="submission_logs", fk_definitions=[ - {"new_constraint_name": "fk_submission_logs_sub_id", "referred_table": "submission", - "local_cols": ["sub_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_logs_sub_id", + "referred_table": "submission", + "local_cols": ["sub_id"], + "remote_cols": ["id"], + } + ], ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="submission_properties", + connection=connection, + table_name="submission_properties", fk_definitions=[ - {"new_constraint_name": "fk_submission_properties_sub_id", "referred_table": "submission", - "local_cols": ["sub_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_properties_sub_id", + "referred_table": "submission", + "local_cols": ["sub_id"], + "remote_cols": ["id"], + } + ], ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="takepart", + connection=connection, + table_name="takepart", fk_definitions=[ - {"new_constraint_name": "fk_takepart_lectid", "referred_table": "lecture", - "local_cols": ["lectid"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_takepart_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_takepart_lectid", + "referred_table": "lecture", + "local_cols": ["lectid"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_takepart_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + ], ) # invalid “api_token” entries are deleted @@ -221,23 +273,41 @@ def upgrade(): """) ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="api_token", + connection=connection, + table_name="api_token", fk_definitions=[ - {"new_constraint_name": "fk_api_token_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_api_token_client_id", "referred_table": "oauth_client", - "local_cols": ["client_id"], "remote_cols": ["identifier"]} - ] + { + "new_constraint_name": "fk_api_token_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_api_token_client_id", + "referred_table": "oauth_client", + "local_cols": ["client_id"], + "remote_cols": ["identifier"], + }, + ], ) _upgrade_recreate_foreign_keys( - connection=connection, table_name="oauth_code", + connection=connection, + table_name="oauth_code", fk_definitions=[ - {"new_constraint_name": "fk_oauth_code_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_oauth_code_client_id", "referred_table": "oauth_client", - "local_cols": ["client_id"], "remote_cols": ["identifier"]} - ] + { + "new_constraint_name": "fk_oauth_code_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_oauth_code_client_id", + "referred_table": "oauth_client", + "local_cols": ["client_id"], + "remote_cols": ["identifier"], + }, + ], ) if connection.dialect.name == "sqlite": @@ -250,67 +320,118 @@ def downgrade(): connection.execute(text("PRAGMA foreign_keys=OFF;")) _downgrade_recreate_foreign_keys( - connection=connection, table_name="assignment", + connection=connection, + table_name="assignment", fk_definitions=[ - {"new_constraint_name": "fk_assignment_lectid", "referred_table": "lecture", - "local_cols": ["lectid"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_assignment_lectid", + "referred_table": "lecture", + "local_cols": ["lectid"], + "remote_cols": ["id"], + } + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="submission", + connection=connection, + table_name="submission", fk_definitions=[ - {"new_constraint_name": "fk_submission_assignid", "referred_table": "assignment", - "local_cols": ["assignid"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_submission_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_assignid", + "referred_table": "assignment", + "local_cols": ["assignid"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_submission_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="submission_logs", + connection=connection, + table_name="submission_logs", fk_definitions=[ - {"new_constraint_name": "fk_submission_logs_sub_id", "referred_table": "submission", - "local_cols": ["sub_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_logs_sub_id", + "referred_table": "submission", + "local_cols": ["sub_id"], + "remote_cols": ["id"], + } + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="submission_properties", + connection=connection, + table_name="submission_properties", fk_definitions=[ - {"new_constraint_name": "fk_submission_properties_sub_id", "referred_table": "submission", - "local_cols": ["sub_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_submission_properties_sub_id", + "referred_table": "submission", + "local_cols": ["sub_id"], + "remote_cols": ["id"], + } + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="takepart", + connection=connection, + table_name="takepart", fk_definitions=[ - {"new_constraint_name": "fk_takepart_lectid", "referred_table": "lecture", - "local_cols": ["lectid"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_takepart_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]} - ] + { + "new_constraint_name": "fk_takepart_lectid", + "referred_table": "lecture", + "local_cols": ["lectid"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_takepart_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="api_token", + connection=connection, + table_name="api_token", fk_definitions=[ - {"new_constraint_name": "fk_api_token_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_api_token_client_id", "referred_table": "oauth_client", - "local_cols": ["client_id"], "remote_cols": ["identifier"]} - ] + { + "new_constraint_name": "fk_api_token_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_api_token_client_id", + "referred_table": "oauth_client", + "local_cols": ["client_id"], + "remote_cols": ["identifier"], + }, + ], ) _downgrade_recreate_foreign_keys( - connection=connection, table_name="oauth_code", + connection=connection, + table_name="oauth_code", fk_definitions=[ - {"new_constraint_name": "fk_oauth_code_user_id", "referred_table": "user", - "local_cols": ["user_id"], "remote_cols": ["id"]}, - {"new_constraint_name": "fk_oauth_code_client_id", "referred_table": "oauth_client", - "local_cols": ["client_id"], "remote_cols": ["identifier"]} - ] + { + "new_constraint_name": "fk_oauth_code_user_id", + "referred_table": "user", + "local_cols": ["user_id"], + "remote_cols": ["id"], + }, + { + "new_constraint_name": "fk_oauth_code_client_id", + "referred_table": "oauth_client", + "local_cols": ["client_id"], + "remote_cols": ["identifier"], + }, + ], ) op.drop_table("alembic_fk_metadata") diff --git a/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py b/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py index c928443cc..74a6b6f66 100644 --- a/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py +++ b/grader_service/migrate/versions/ba71c755c153_added_oauth_provider_functionality.py @@ -16,7 +16,7 @@ PrimaryKeyConstraint, Text, Unicode, - text + text, ) from grader_service.utils import new_token @@ -116,4 +116,4 @@ def downgrade(): op.drop_column("user", "cookie_id") if connection.dialect.name == "sqlite": - connection.execute(text("PRAGMA foreign_keys=OFF;")) \ No newline at end of file + connection.execute(text("PRAGMA foreign_keys=OFF;")) diff --git a/grader_service/orm/__init__.py b/grader_service/orm/__init__.py index 00b142018..f5c0711c7 100644 --- a/grader_service/orm/__init__.py +++ b/grader_service/orm/__init__.py @@ -11,10 +11,10 @@ from grader_service.orm.oauthclient import OAuthClient from grader_service.orm.oauthcode import OAuthCode from grader_service.orm.submission import Submission -from grader_service.orm.takepart import Role -from grader_service.orm.user import User from grader_service.orm.submission_logs import SubmissionLogs from grader_service.orm.submission_properties import SubmissionProperties +from grader_service.orm.takepart import Role +from grader_service.orm.user import User __all__ = [ "Lecture", @@ -27,5 +27,5 @@ "OAuthClient", "APIToken", "SubmissionLogs", - "SubmissionProperties" + "SubmissionProperties", ] diff --git a/grader_service/orm/assignment.py b/grader_service/orm/assignment.py index 57e52a726..5ca44da7c 100644 --- a/grader_service/orm/assignment.py +++ b/grader_service/orm/assignment.py @@ -44,7 +44,10 @@ class Assignment(Base, Serializable): lecture = relationship("Lecture", back_populates="assignments") submissions = relationship( - "Submission", back_populates="assignment", cascade="all, delete-orphan", passive_deletes=True + "Submission", + back_populates="assignment", + cascade="all, delete-orphan", + passive_deletes=True, ) @property diff --git a/grader_service/orm/submission.py b/grader_service/orm/submission.py index 6406526ec..7c6a68419 100644 --- a/grader_service/orm/submission.py +++ b/grader_service/orm/submission.py @@ -65,12 +65,18 @@ class Submission(Base, Serializable): assignment = relationship("Assignment", back_populates="submissions") user = relationship("User", back_populates="submissions") logs = relationship( - "SubmissionLogs", back_populates="submission", uselist=False, - cascade="all, delete-orphan", passive_deletes=True + "SubmissionLogs", + back_populates="submission", + uselist=False, + cascade="all, delete-orphan", + passive_deletes=True, ) properties = relationship( - "SubmissionProperties", back_populates="submission", uselist=False, - cascade="all, delete-orphan", passive_deletes=True + "SubmissionProperties", + back_populates="submission", + uselist=False, + cascade="all, delete-orphan", + passive_deletes=True, ) @hybrid_property diff --git a/grader_service/orm/takepart.py b/grader_service/orm/takepart.py index d7624a374..1860b4998 100644 --- a/grader_service/orm/takepart.py +++ b/grader_service/orm/takepart.py @@ -39,4 +39,4 @@ def serialize_with_user(self) -> dict: """ model = self.serialize() model["user"] = self.user.serialize() - return model \ No newline at end of file + return model diff --git a/grader_service/tests/handlers/db_util.py b/grader_service/tests/handlers/db_util.py index f5a76ac94..18051a8f5 100644 --- a/grader_service/tests/handlers/db_util.py +++ b/grader_service/tests/handlers/db_util.py @@ -18,7 +18,7 @@ from grader_service.api.models.assignment_settings import AssignmentSettings from grader_service.handlers import GitRepoType from grader_service.handlers.git.server import GitBaseHandler -from grader_service.orm import Assignment, Lecture, Role, Submission, User, SubmissionLogs +from grader_service.orm import Assignment, Lecture, Role, Submission, SubmissionLogs, User from grader_service.orm.base import DeleteState from grader_service.orm.submission import AutoStatus, FeedbackStatus, ManualStatus from grader_service.orm.submission_properties import SubmissionProperties @@ -179,7 +179,7 @@ def insert_student(ex: Engine, username: str, lecture_id: int) -> User: def create_user_submission_with_repo( - engine: Engine, gitbase_dir: Path, student: User, assignment_id: int, lecture_code: str + engine: Engine, gitbase_dir: Path, student: User, assignment_id: int, lecture_code: str ) -> Submission: """Creates a submission for `student` and a user repo for storing it. @@ -224,9 +224,15 @@ def create_user_submission_with_repo( return submission -def check_assignment_and_status(engine: Engine, l_id: int, a_id: int, status: str, should_exist: bool = True): +def check_assignment_and_status( + engine: Engine, l_id: int, a_id: int, status: str, should_exist: bool = True +): session: Session = sessionmaker(engine)() - assignment = session.query(orm.Assignment).filter(orm.Assignment.id == a_id, orm.Assignment.lectid == l_id).first() + assignment = ( + session.query(orm.Assignment) + .filter(orm.Assignment.id == a_id, orm.Assignment.lectid == l_id) + .first() + ) if should_exist: assert assignment is not None, "assignment is None" assert assignment.status == status, f"assert '{assignment.status}' == '{status}'" @@ -236,16 +242,26 @@ def check_assignment_and_status(engine: Engine, l_id: int, a_id: int, status: st def check_submission(engine: Engine, a_id: int, s_id: int, should_exist: bool = True): session: Session = sessionmaker(engine)() - submission = session.query(orm.Submission).filter(orm.Submission.id == s_id, - orm.Submission.assignid == a_id).first() + submission = ( + session.query(orm.Submission) + .filter(orm.Submission.id == s_id, orm.Submission.assignid == a_id) + .first() + ) if should_exist: assert submission is not None, "submission is None" else: assert submission is None, f"submission exists (id={s_id}, assignid={a_id})" -def create_git_repository(app: GraderServer, l_id: int, code: str, a_id: int, s_id: int, repo_type: GitRepoType, - username: str): +def create_git_repository( + app: GraderServer, + l_id: int, + code: str, + a_id: int, + s_id: int, + repo_type: GitRepoType, + username: str, +): git_dir = Path(app.grader_service_dir) / "git" git_dir.mkdir(exist_ok=True) path = f"/git/{code}/{a_id}/{repo_type}/{s_id}" @@ -253,7 +269,9 @@ def create_git_repository(app: GraderServer, l_id: int, code: str, a_id: int, s_ handler_mock.request.path = path handler_mock.gitbase = str(git_dir) handler_mock.user.name = username - sf = get_query_side_effect(lid=l_id, code=code, scope=Scope.instructor, a_id=a_id, s_id=s_id, username=username) + sf = get_query_side_effect( + lid=l_id, code=code, scope=Scope.instructor, a_id=a_id, s_id=s_id, username=username + ) handler_mock.session.query = Mock(side_effect=sf) constructed_git_dir = GitBaseHandler.construct_git_dir( handler_mock, @@ -267,28 +285,82 @@ def create_git_repository(app: GraderServer, l_id: int, code: str, a_id: int, s_ assert os.path.exists(lookup_dir) -def create_all_git_repositories(app: GraderServer, user: User, l_id: int, l_code: str, a_id: int, s_id: int): +def create_all_git_repositories( + app: GraderServer, user: User, l_id: int, l_code: str, a_id: int, s_id: int +): # create possible git repositories for submission - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.SOURCE, - username=user.name) - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.RELEASE, - username=user.name) - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.USER, - username=user.name) - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.EDIT, - username=user.name) - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.AUTOGRADE, - username=user.name) - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=s_id, repo_type=GitRepoType.FEEDBACK, - username=user.name) - - check_git_repositories(app, user, l_code, a_id, s_id, - True, True, True, True, True, True, True) - - -def check_git_repositories(app: GraderServer, user: User, l_code: str, a_id: int, s_id: int, - exits_assignment: bool, exits_source: bool, exits_release: bool, exits_user: bool, - exits_edit: bool, exits_feedback: bool, exits_autograde: bool): + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.SOURCE, + username=user.name, + ) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.RELEASE, + username=user.name, + ) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.USER, + username=user.name, + ) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.EDIT, + username=user.name, + ) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.AUTOGRADE, + username=user.name, + ) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=s_id, + repo_type=GitRepoType.FEEDBACK, + username=user.name, + ) + + check_git_repositories(app, user, l_code, a_id, s_id, True, True, True, True, True, True, True) + + +def check_git_repositories( + app: GraderServer, + user: User, + l_code: str, + a_id: int, + s_id: int, + exits_assignment: bool, + exits_source: bool, + exits_release: bool, + exits_user: bool, + exits_edit: bool, + exits_feedback: bool, + exits_autograde: bool, +): assignment_path = Path(app.grader_service_dir) / "git" / l_code / str(a_id) source_path = assignment_path / GitRepoType.SOURCE diff --git a/grader_service/tests/handlers/test_assignment_handler.py b/grader_service/tests/handlers/test_assignment_handler.py index 116e24c54..90ca6360e 100644 --- a/grader_service/tests/handlers/test_assignment_handler.py +++ b/grader_service/tests/handlers/test_assignment_handler.py @@ -13,9 +13,16 @@ from grader_service.api.models.assignment import Assignment from grader_service.api.models.assignment_settings import AssignmentSettings from grader_service.server import GraderServer -from .db_util import insert_assignments, insert_submission, check_assignment_and_status, insert_assignment, \ - create_all_git_repositories, check_git_repositories + from ... import orm +from .db_util import ( + check_assignment_and_status, + check_git_repositories, + create_all_git_repositories, + insert_assignment, + insert_assignments, + insert_submission, +) async def test_get_assignments( @@ -800,7 +807,9 @@ async def test_delete_assignment( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created") + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created" + ) with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -846,7 +855,9 @@ async def test_delete_assignment_deleted_assignment( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created") + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=post_assignment.id, status="created" + ) with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -927,7 +938,9 @@ async def test_delete_assignment_same_name_twice( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=first_post_assignment.id, status="created") + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=first_post_assignment.id, status="created" + ) url = service_base_url + "lectures/3/assignments/" @@ -946,9 +959,16 @@ async def test_delete_assignment_same_name_twice( url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=first_post_assignment.id, status="created", - should_exist=False) - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=second_post_assignment.id, status="created") + check_assignment_and_status( + sql_alchemy_engine, + l_id=l_id, + a_id=first_post_assignment.id, + status="created", + should_exist=False, + ) + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=second_post_assignment.id, status="created" + ) async def test_delete_released_assignment( @@ -1065,10 +1085,14 @@ async def test_delete_assignment_hard( insert_assignment(sql_alchemy_engine, l_id) delete_response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="created", should_exist=False) + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=a_id, status="created", should_exist=False + ) with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -1102,7 +1126,9 @@ async def test_delete_assignment_hard_unauthorized( with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN @@ -1116,7 +1142,7 @@ async def test_delete_assignment_hard_with_submissions( default_roles, default_admin_login, sql_alchemy_engine, - default_user + default_user, ): l_id = 4 l_code = "23wle1" @@ -1126,10 +1152,7 @@ async def test_delete_assignment_hard_with_submissions( # create assignment url = service_base_url + f"lectures/{l_id}/assignments" pre_assignment = Assignment( - id=-1, - name="pytest", - status="released", - settings=AssignmentSettings(), + id=-1, name="pytest", status="released", settings=AssignmentSettings() ) post_response = await http_server_client.fetch( url, @@ -1144,10 +1167,14 @@ async def test_delete_assignment_hard_with_submissions( url = service_base_url + f"lectures/{l_id}/assignments/{a_id}" delete_response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert delete_response.code == HTTPStatus.OK - check_assignment_and_status(sql_alchemy_engine, l_id=l_id, a_id=a_id, status="released", should_exist=False) + check_assignment_and_status( + sql_alchemy_engine, l_id=l_id, a_id=a_id, status="released", should_exist=False + ) with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( @@ -1165,8 +1192,9 @@ async def test_delete_assignment_hard_with_submissions( submissions = session.query(orm.Submission).filter(orm.Submission.assignid == a_id).all() assert len(submissions) == 0 - check_git_repositories(app, default_user, l_code, a_id, - False, False, False, False, False, False, False, False) + check_git_repositories( + app, default_user, l_code, a_id, False, False, False, False, False, False, False, False + ) async def test_assignment_properties_lecture_assignment_missmatch( diff --git a/grader_service/tests/handlers/test_git.py b/grader_service/tests/handlers/test_git.py index d6334653a..567e9d25c 100644 --- a/grader_service/tests/handlers/test_git.py +++ b/grader_service/tests/handlers/test_git.py @@ -20,7 +20,13 @@ def get_query_side_effect( - lid=1, code="ivs21s", scope: Scope = Scope.student, username="test_user", user_id=137, a_id=1, s_id=1 + lid=1, + code="ivs21s", + scope: Scope = Scope.student, + username="test_user", + user_id=137, + a_id=1, + s_id=1, ): def query_side_effect(input): m = Mock() diff --git a/grader_service/tests/handlers/test_lectures_handler.py b/grader_service/tests/handlers/test_lectures_handler.py index ecaf762e2..8973accfa 100644 --- a/grader_service/tests/handlers/test_lectures_handler.py +++ b/grader_service/tests/handlers/test_lectures_handler.py @@ -8,17 +8,24 @@ from pathlib import Path import pytest -from sqlalchemy.orm import sessionmaker, Session +from sqlalchemy.orm import Session, sessionmaker from tornado.httpclient import HTTPClientError from grader_service.api.models.assignment import Assignment from grader_service.api.models.assignment_settings import AssignmentSettings from grader_service.api.models.lecture import Lecture from grader_service.server import GraderServer -from .db_util import insert_assignment, insert_assignments, insert_student, insert_submission, create_git_repository + from ... import orm from ...handlers import GitRepoType from ...orm.base import DeleteState +from .db_util import ( + create_git_repository, + insert_assignment, + insert_assignments, + insert_student, + insert_submission, +) async def test_get_lectures( @@ -651,7 +658,9 @@ async def test_delete_lecture_hard( url = service_base_url + f"lectures/{l_id}" delete_response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert delete_response.code == HTTPStatus.OK @@ -681,7 +690,9 @@ async def test_delete_lecture_hard_unauthorized( url = service_base_url + f"lectures/{l_id}" with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN @@ -704,10 +715,7 @@ async def test_delete_lecture_hard_assignments_roles( # create assignment url = service_base_url + f"lectures/{l_id}/assignments" pre_assignment = Assignment( - id=-1, - name="pytest", - status="released", - settings=AssignmentSettings(), + id=-1, name="pytest", status="released", settings=AssignmentSettings() ) post_response = await http_server_client.fetch( url, @@ -717,14 +725,24 @@ async def test_delete_lecture_hard_assignments_roles( ) assert post_response.code == HTTPStatus.CREATED - create_git_repository(app=app, l_id=l_id, code=l_code, a_id=a_id, s_id=1, repo_type=GitRepoType.SOURCE, username=default_admin.name) + create_git_repository( + app=app, + l_id=l_id, + code=l_code, + a_id=a_id, + s_id=1, + repo_type=GitRepoType.SOURCE, + username=default_admin.name, + ) git_dir = Path(app.grader_service_dir) / "git" / l_code / str(a_id) assert git_dir.exists() url = service_base_url + f"lectures/{l_id}" delete_response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert delete_response.code == HTTPStatus.OK @@ -762,7 +780,7 @@ async def test_delete_lecture_unknown_parameter( url = service_base_url + f"lectures/{l_id}?some_param=asdf" with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="DELETE", headers={"Authorization": f"Token {default_token}"}, + url, method="DELETE", headers={"Authorization": f"Token {default_token}"} ) e = exc_info.value assert e.code == HTTPStatus.BAD_REQUEST diff --git a/grader_service/tests/handlers/test_roles_handler.py b/grader_service/tests/handlers/test_roles_handler.py index ffaf679d3..28d70a468 100644 --- a/grader_service/tests/handlers/test_roles_handler.py +++ b/grader_service/tests/handlers/test_roles_handler.py @@ -9,13 +9,13 @@ async def test_get_roles_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" @@ -29,13 +29,13 @@ async def test_get_roles_unauthorized( async def test_get_roles_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/?abc=123" @@ -49,13 +49,13 @@ async def test_get_roles_admin_unknown_parameter( async def test_get_roles_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" @@ -71,13 +71,13 @@ async def test_get_roles_admin( async def test_get_roles_admin_wrong_lecture( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 999 url = service_base_url + f"lectures/{l_id}/roles/" @@ -92,64 +92,76 @@ async def test_get_roles_admin_wrong_lecture( async def test_post_roles_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + { + "username": default_user.name, + "role": Scope.student, + } # change role from instructor to student ] } with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN async def test_post_roles_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/?abc=123" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + { + "username": default_user.name, + "role": Scope.student, + } # change role from instructor to student ] } with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.BAD_REQUEST async def test_post_roles_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" @@ -164,12 +176,18 @@ async def test_post_roles_admin( data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student - {"username": default_admin.name, "role": Scope.instructor} # new role + { + "username": default_user.name, + "role": Scope.student, + }, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor}, # new role ] } response = await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.CREATED @@ -187,38 +205,44 @@ async def test_post_roles_admin( async def test_post_roles_admin_wrong_lecture( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 999 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student + { + "username": default_user.name, + "role": Scope.student, + } # change role from instructor to student ] } with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.NOT_FOUND async def test_delete_roles_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/?usernames={default_user.name}" @@ -232,13 +256,13 @@ async def test_delete_roles_unauthorized( async def test_delete_roles_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/?abc=123&usernames={default_user.name}" @@ -252,31 +276,39 @@ async def test_delete_roles_admin_unknown_parameter( async def test_delete_roles_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student - {"username": default_admin.name, "role": Scope.instructor} # new role + { + "username": default_user.name, + "role": Scope.student, + }, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor}, # new role ] } response = await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.CREATED response = await http_server_client.fetch( - f"{url}?usernames={default_user.name}", method="DELETE", headers={"Authorization": f"Token {default_token}"} + f"{url}?usernames={default_user.name}", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert response.code == HTTPStatus.OK @@ -292,32 +324,39 @@ async def test_delete_roles_admin( async def test_delete_roles_admin_multiple( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student - {"username": default_admin.name, "role": Scope.instructor} # new role + { + "username": default_user.name, + "role": Scope.student, + }, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor}, # new role ] } response = await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.CREATED response = await http_server_client.fetch( - f"{url}?usernames={default_user.name},{default_admin.name}", method="DELETE", - headers={"Authorization": f"Token {default_token}"} + f"{url}?usernames={default_user.name},{default_admin.name}", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert response.code == HTTPStatus.OK @@ -331,33 +370,40 @@ async def test_delete_roles_admin_multiple( async def test_delete_roles_admin_multiple_wrong_user( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student - {"username": default_admin.name, "role": Scope.instructor} # new role + { + "username": default_user.name, + "role": Scope.student, + }, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor}, # new role ] } response = await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.CREATED with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - f"{url}?usernames={default_user.name},{default_admin.name},windows", method="DELETE", - headers={"Authorization": f"Token {default_token}"} + f"{url}?usernames={default_user.name},{default_admin.name},windows", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) e = exc_info.value assert e.code == HTTPStatus.NOT_FOUND @@ -372,26 +418,32 @@ async def test_delete_roles_admin_multiple_wrong_user( async def test_delete_roles_admin_empty_usernames( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): l_id = 2 url = service_base_url + f"lectures/{l_id}/roles/" data = { "users": [ - {"username": default_user.name, "role": Scope.student}, # change role from instructor to student - {"username": default_admin.name, "role": Scope.instructor} # new role + { + "username": default_user.name, + "role": Scope.student, + }, # change role from instructor to student + {"username": default_admin.name, "role": Scope.instructor}, # new role ] } response = await http_server_client.fetch( - url, method="POST", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="POST", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.CREATED @@ -404,13 +456,13 @@ async def test_delete_roles_admin_empty_usernames( async def test_delete_roles_admin_wrong_lecture( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): l_id = 999 url = service_base_url + f"lectures/{l_id}/roles/usernames={default_user.name}" diff --git a/grader_service/tests/handlers/test_submissions_handler.py b/grader_service/tests/handlers/test_submissions_handler.py index ac6a6832e..39780416e 100644 --- a/grader_service/tests/handlers/test_submissions_handler.py +++ b/grader_service/tests/handlers/test_submissions_handler.py @@ -13,7 +13,7 @@ import isodate import pytest -from sqlalchemy.orm import sessionmaker, Session +from sqlalchemy.orm import Session, sessionmaker from tornado.httpclient import HTTPClientError from grader_service.api.models import AssignmentSettings, Submission @@ -29,13 +29,17 @@ from grader_service.orm.submission import Submission as SubmissionORM from grader_service.orm.takepart import Scope from grader_service.server import GraderServer + +from ... import orm from .db_util import ( + check_git_repositories, + check_submission, + create_all_git_repositories, create_user_submission_with_repo, insert_assignments, insert_student, - insert_submission, check_submission, create_all_git_repositories, check_git_repositories, + insert_submission, ) -from ... import orm async def submission_test_setup(engine, default_user, a_id: int): @@ -230,8 +234,8 @@ async def test_get_submissions_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" ) insert_submission(engine, a_id, default_user.name, user_id=default_user.id) @@ -288,8 +292,8 @@ async def test_get_submissions_instructor_version_unauthorized( engine = sql_alchemy_engine url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true" ) insert_submission(engine, a_id, username=default_user.name, user_id=default_user.id) @@ -325,8 +329,8 @@ async def test_get_submissions_latest_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=latest" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=latest" ) insert_submission(engine, a_id, default_user.name, user_id=default_user.id) @@ -386,8 +390,8 @@ async def test_get_submissions_best_instructor_version( other_user = insert_student(engine, "student1", l_id) url = ( - service_base_url - + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=best" + service_base_url + + f"lectures/{l_id}/assignments/{a_id}/submissions/?instructor-version=true&filter=best" ) insert_submission( @@ -564,7 +568,9 @@ async def test_get_submissions_admin_deleted_instructor_version( ) assert response.code == HTTPStatus.OK - url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions?instructor-version=true" + url = ( + service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions?instructor-version=true" + ) response = await http_server_client.fetch( url, method="GET", headers={"Authorization": f"Token {default_token}"} ) @@ -1118,7 +1124,9 @@ async def test_delete_submission_with_feedback( a_id = 1 s_id = 1 - insert_submission(sql_alchemy_engine, a_id, default_user.name, feedback=FeedbackStatus.GENERATED) + insert_submission( + sql_alchemy_engine, a_id, default_user.name, feedback=FeedbackStatus.GENERATED + ) check_submission(sql_alchemy_engine, a_id, s_id) url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" @@ -1176,22 +1184,35 @@ async def test_delete_submission_hard( a_id = 1 s_id = 1 - insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id, with_properties=True, - with_logs=True) + insert_submission( + sql_alchemy_engine, + a_id, + default_user.name, + default_user.id, + with_properties=True, + with_logs=True, + ) session: Session = sessionmaker(sql_alchemy_engine)() submissions = session.query(orm.Submission).filter(orm.Submission.id == s_id).all() assert len(submissions) == 1 - submission_properties = session.query(orm.SubmissionProperties).filter( - orm.SubmissionProperties.sub_id == s_id).all() + submission_properties = ( + session.query(orm.SubmissionProperties) + .filter(orm.SubmissionProperties.sub_id == s_id) + .all() + ) assert len(submission_properties) == 1 - submission_logs = session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + submission_logs = ( + session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + ) assert len(submission_logs) == 1 url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert response.code == HTTPStatus.OK @@ -1206,10 +1227,15 @@ async def test_delete_submission_hard( submissions = session.query(orm.Submission).filter(orm.Submission.id == s_id).all() assert len(submissions) == 0 - submission_properties = session.query(orm.SubmissionProperties).filter( - orm.SubmissionProperties.sub_id == s_id).all() + submission_properties = ( + session.query(orm.SubmissionProperties) + .filter(orm.SubmissionProperties.sub_id == s_id) + .all() + ) assert len(submission_properties) == 0 - submission_logs = session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + submission_logs = ( + session.query(orm.SubmissionLogs).filter(orm.SubmissionLogs.sub_id == s_id).all() + ) assert len(submission_logs) == 0 @@ -1227,14 +1253,22 @@ async def test_delete_submission_hard_unauthorized( a_id = 1 s_id = 1 - insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id, with_properties=True, - with_logs=True) + insert_submission( + sql_alchemy_engine, + a_id, + default_user.name, + default_user.id, + with_properties=True, + with_logs=True, + ) url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN @@ -1261,7 +1295,9 @@ async def test_delete_submission_hard_with_files( url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/{s_id}" response = await http_server_client.fetch( - url + "?hard_delete=true", method="DELETE", headers={"Authorization": f"Token {default_token}"} + url + "?hard_delete=true", + method="DELETE", + headers={"Authorization": f"Token {default_token}"}, ) assert response.code == HTTPStatus.OK @@ -1274,8 +1310,9 @@ async def test_delete_submission_hard_with_files( e = exc_info.value assert e.code == HTTPStatus.NOT_FOUND - check_git_repositories(app, default_user, l_code, a_id, s_id, - True, True, True, False, False, False, False) + check_git_repositories( + app, default_user, l_code, a_id, s_id, True, True, True, False, False, False, False + ) async def test_post_submission_by_student( @@ -1771,8 +1808,8 @@ async def test_submission_cannot_edit_submission_created_by_instructor( url = service_base_url + f"lectures/{l_id}/assignments/{a_id}/submissions/1/edit" with pytest.raises( - HTTPClientError, - match="This repo cannot be edited or reset, because it was created by instructor", + HTTPClientError, + match="This repo cannot be edited or reset, because it was created by instructor", ) as exc_info: await http_server_client.fetch( url, diff --git a/grader_service/tests/handlers/test_users_handler.py b/grader_service/tests/handlers/test_users_handler.py index 46ed23496..28d7df6a1 100644 --- a/grader_service/tests/handlers/test_users_handler.py +++ b/grader_service/tests/handlers/test_users_handler.py @@ -7,18 +7,21 @@ from grader_service import orm from grader_service.server import GraderServer -from grader_service.tests.handlers.db_util import insert_submission, create_all_git_repositories, \ - check_git_repositories +from grader_service.tests.handlers.db_util import ( + check_git_repositories, + create_all_git_repositories, + insert_submission, +) async def test_get_users_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): url = service_base_url + "users/" @@ -31,13 +34,13 @@ async def test_get_users_unauthorized( async def test_get_users_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + "users/?abc=123" @@ -50,14 +53,14 @@ async def test_get_users_admin_unknown_parameter( async def test_get_users_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): url = service_base_url + "users/" @@ -73,13 +76,13 @@ async def test_get_users_admin( async def test_get_user_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): url = service_base_url + f"users/{default_user.name}/" @@ -92,13 +95,13 @@ async def test_get_user_unauthorized( async def test_get_user_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + f"users/{default_user.name}/?abc=123" @@ -111,14 +114,14 @@ async def test_get_user_admin_unknown_parameter( async def test_get_user_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): url = service_base_url + f"users/{default_user.name}" @@ -131,13 +134,13 @@ async def test_get_user_admin( async def test_get_user_admin_wrong_user( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + "users/windows" @@ -150,60 +153,69 @@ async def test_get_user_admin_wrong_user( async def test_put_user_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): url = service_base_url + f"users/{default_user.name}" data = {"name": default_user.name, "display_name": "New Name"} with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="PUT", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.FORBIDDEN async def test_put_user_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + f"users/{default_user.name}/?abc=123" data = {"name": default_user.name, "display_name": "New Name"} with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="PUT", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.BAD_REQUEST async def test_put_user_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, ): url = service_base_url + f"users/{default_user.name}" data = {"name": default_user.name, "display_name": "New Name"} response = await http_server_client.fetch( - url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="PUT", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) assert response.code == HTTPStatus.OK @@ -217,33 +229,36 @@ async def test_put_user_admin( async def test_put_user_admin_wrong_user( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + "users/windows" data = {"name": default_user.name, "display_name": "New Name"} with pytest.raises(HTTPClientError) as exc_info: await http_server_client.fetch( - url, method="PUT", headers={"Authorization": f"Token {default_token}"}, body=json.dumps(data) + url, + method="PUT", + headers={"Authorization": f"Token {default_token}"}, + body=json.dumps(data), ) e = exc_info.value assert e.code == HTTPStatus.NOT_FOUND async def test_delete_user_unauthorized( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_user_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_user_login, + default_user, ): url = service_base_url + f"users/{default_user.name}" @@ -256,13 +271,13 @@ async def test_delete_user_unauthorized( async def test_delete_user_admin_unknown_parameter( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + f"users/{default_user.name}/?abc=123" @@ -275,15 +290,15 @@ async def test_delete_user_admin_unknown_parameter( async def test_delete_user_admin( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, - default_admin, - sql_alchemy_engine, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, + default_admin, + sql_alchemy_engine, ): username = default_user.name url = service_base_url + f"users/{username}/" @@ -316,7 +331,9 @@ async def test_delete_user_admin( insert_submission(sql_alchemy_engine, a_id, default_user.name, default_user.id) create_all_git_repositories(app, default_user, l_id, l_code, a_id, s_id) - old_submissions = session.query(orm.Submission).filter(orm.Submission.user_id == old_user.id).all() + old_submissions = ( + session.query(orm.Submission).filter(orm.Submission.user_id == old_user.id).all() + ) old_roles = session.query(orm.Role).filter(orm.Role.user_id == old_user.id).all() old_api_tokens = session.query(orm.APIToken).filter(orm.APIToken.user_id == old_user.id).all() old_auth_codes = session.query(orm.OAuthCode).filter(orm.OAuthCode.user_id == old_user.id).all() @@ -351,18 +368,19 @@ async def test_delete_user_admin( assert len(api_tokens) == 0 assert len(auth_codes) == 0 - check_git_repositories(app, default_user, l_code, a_id, s_id, - True, True, True, False, False, False, False) + check_git_repositories( + app, default_user, l_code, a_id, s_id, True, True, True, False, False, False, False + ) async def test_delete_user_admin_wrong_user( - app: GraderServer, - service_base_url, - http_server_client, - default_token, - default_roles, - default_admin_login, - default_user, + app: GraderServer, + service_base_url, + http_server_client, + default_token, + default_roles, + default_admin_login, + default_user, ): url = service_base_url + "users/windows/"