Skip to content

Commit

Permalink
Development: Fix a NullPointerException in RepositoryAccessService
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan Krusche committed Mar 11, 2024
1 parent d360314 commit 5dacbe2
Show file tree
Hide file tree
Showing 16 changed files with 44 additions and 39 deletions.
20 changes: 14 additions & 6 deletions src/main/java/de/tum/in/www1/artemis/domain/BaseExercise.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract class BaseExercise extends DomainObject {
@JsonView(QuizView.Before.class)
private String shortName;

@Column(name = "max_points")
@Column(name = "max_points", nullable = false)
private Double maxPoints;

@Column(name = "bonus_points")
Expand All @@ -41,23 +41,27 @@ public abstract class BaseExercise extends DomainObject {

@Column(name = "release_date")
@JsonView(QuizView.Before.class)
@Nullable
private ZonedDateTime releaseDate;

// TODO: Also use for quiz exercises
@Column(name = "start_date")
@JsonView(QuizView.Before.class)
@Nullable
private ZonedDateTime startDate;

@Column(name = "due_date")
@JsonView(QuizView.Before.class)
@Nullable
private ZonedDateTime dueDate;

@Column(name = "assessment_due_date")
@JsonView(QuizView.Before.class)
@Nullable
private ZonedDateTime assessmentDueDate;

@Nullable
@Column(name = "example_solution_publication_date")
@Nullable
private ZonedDateTime exampleSolutionPublicationDate;

@Enumerated(EnumType.STRING)
Expand Down Expand Up @@ -115,35 +119,39 @@ public void setAssessmentType(AssessmentType assessmentType) {
this.assessmentType = assessmentType;
}

@Nullable
public ZonedDateTime getReleaseDate() {
return releaseDate;
}

public void setReleaseDate(ZonedDateTime releaseDate) {
public void setReleaseDate(@Nullable ZonedDateTime releaseDate) {
this.releaseDate = releaseDate;
}

@Nullable
public ZonedDateTime getStartDate() {
return startDate;
}

public void setStartDate(ZonedDateTime startDate) {
public void setStartDate(@Nullable ZonedDateTime startDate) {
this.startDate = startDate;
}

@Nullable
public ZonedDateTime getDueDate() {
return dueDate;
}

public void setDueDate(ZonedDateTime dueDate) {
public void setDueDate(@Nullable ZonedDateTime dueDate) {
this.dueDate = dueDate;
}

@Nullable
public ZonedDateTime getAssessmentDueDate() {
return assessmentDueDate;
}

public void setAssessmentDueDate(ZonedDateTime assessmentDueDate) {
public void setAssessmentDueDate(@Nullable ZonedDateTime assessmentDueDate) {
this.assessmentDueDate = assessmentDueDate;
}

Expand Down
1 change: 1 addition & 0 deletions src/main/java/de/tum/in/www1/artemis/domain/Exercise.java
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,7 @@ public void checkCourseAndExerciseGroupExclusivity(String entityName) throws Bad
* @return the time from which on access to the participation is allowed, for exercises that are not part of an exam, this is just the release date or start date.
*/
@JsonIgnore
@Nullable
public ZonedDateTime getParticipationStartDate() {
if (isExamExercise()) {
return getExerciseGroup().getExam().getStartDate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@Repository
public interface CompetencyRelationRepository extends JpaRepository<CompetencyRelation, Long> {

@Transactional
@Transactional // ok because of delete
@Modifying
@Query("""
DELETE FROM CompetencyRelation relation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ public interface ExamLiveEventRepository extends JpaRepository<ExamLiveEvent, Lo
*
* @param examId the id of the exam
*/
@Transactional // delete
@Transactional // ok because of delete
void deleteAllByExamId(Long examId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ WHERE p.expirationDate > NOW()
/**
* Cleans up the old/expired push notifications device configurations
*/
@Transactional
@Transactional // ok because of delete
@Modifying
@Query("""
DELETE FROM PushNotificationDeviceConfiguration p
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ OR CONCAT_WS(' ', user.firstName, user.lastName) LIKE %:#{#loginOrName}%
void updateUserLanguageKey(@Param("userId") long userId, @Param("languageKey") String languageKey);

@Modifying
@Transactional
@Transactional // ok because of modifying query
@Query("""
UPDATE User user
SET user.irisAccepted = :acceptDatetime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ default ConversationParticipant findConversationParticipantByConversationIdAndUs
* @param senderId userId of the sender of the message(Post)
* @param conversationId conversationId id of the conversation with participants
*/
@Transactional
@Transactional // ok because of modifying query
@Modifying
@Query("""
UPDATE ConversationParticipant conversationParticipant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
@Repository
public interface TutorialGroupNotificationRepository extends JpaRepository<TutorialGroupNotification, Long> {

@Transactional
@Transactional // ok because of delete
@Modifying
void deleteAllByTutorialGroupId(Long tutorialGroupId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@ Optional<TutorialGroupRegistration> findTutorialGroupRegistrationByTutorialGroup

Set<TutorialGroupRegistration> findAllByTutorialGroup(TutorialGroup tutorialGroup);

@Transactional // ok because of delete
@Modifying
@Transactional
// ok because of delete
void deleteAllByStudent(User student);

@Transactional
@Transactional // ok because of delete
@Modifying
void deleteById(@NotNull Long tutorialGroupRegistrationId);

@Transactional // ok because of delete
@Modifying
@Transactional
void deleteAllByStudentIsInAndTypeAndTutorialGroupCourse(Set<User> students, TutorialGroupRegistrationType type, Course course);

boolean existsByTutorialGroupTitleAndStudentAndType(String title, User student, TutorialGroupRegistrationType type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
import java.util.Optional;
import java.util.Set;

import javax.transaction.Transactional;

import org.springframework.cache.annotation.Cacheable;
import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import de.tum.in.www1.artemis.domain.Course;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.metis.conversation.Channel;
import de.tum.in.www1.artemis.domain.tutorialgroups.TutorialGroup;
import de.tum.in.www1.artemis.web.rest.errors.EntityNotFoundException;
Expand Down Expand Up @@ -138,13 +134,6 @@ public interface TutorialGroupRepository extends JpaRepository<TutorialGroup, Lo

boolean existsByTitleAndCourseId(String title, Long courseId);

Set<TutorialGroup> findAllByTeachingAssistant(User teachingAssistant);

@Transactional
@Modifying
// ok because of delete
void deleteAllByCourse(Course course);

@Query("""
SELECT tutorialGroup
FROM TutorialGroup tutorialGroup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ default TutorialGroupSession findByIdElseThrow(long tutorialGroupSessionId) {
return findById(tutorialGroupSessionId).orElseThrow(() -> new EntityNotFoundException("TutorialGroupSession", tutorialGroupSessionId));
}

@Transactional // ok because of delete
@Modifying
@Transactional
void deleteByTutorialGroupCourse(Course course);

@Transactional // ok because of delete
@Modifying
@Transactional
void deleteByTutorialGroupSchedule(TutorialGroupSchedule tutorialGroupSchedule);

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import java.time.ZonedDateTime;

import javax.annotation.Nullable;

import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;

Expand Down Expand Up @@ -64,7 +66,10 @@ public void checkAccessRepositoryElseThrow(ProgrammingExerciseParticipation prog
boolean isStudent = authorizationCheckService.isOnlyStudentInCourse(programmingExercise.getCourseViaExerciseGroupOrCourseMember(), user);
boolean isTeachingAssistant = !isStudent && !isAtLeastEditor;
boolean isStudentParticipation = programmingParticipation instanceof StudentParticipation;
var exerciseStartDate = programmingExercise.getParticipationStartDate();

// NOTE: the exerciseStartDate can be null, e.g. if no release and no start is defined for a course exercise
@Nullable
ZonedDateTime exerciseStartDate = programmingExercise.getParticipationStartDate();

// Error case 1: The user does not have permissions to access the participation or the submission for plagiarism comparison.
checkHasParticipationPermissionsOrCanAccessSubmissionForPlagiarismComparisonElseThrow(programmingParticipation, user, programmingExercise);
Expand Down Expand Up @@ -111,9 +116,9 @@ public void checkAccessRepositoryElseThrow(ProgrammingExerciseParticipation prog
* @param exerciseStartDate The start date of the exercise.
*/
private void checkIsStudentAllowedToAccessRepositoryConcerningDates(ProgrammingExerciseParticipation programmingParticipation, RepositoryActionType repositoryActionType,
ZonedDateTime exerciseStartDate) {
@Nullable ZonedDateTime exerciseStartDate) {
// if the exercise has not started yet, the student is not allowed to access the repository
if (exerciseStartDate.isAfter(ZonedDateTime.now())) {
if (exerciseStartDate != null && exerciseStartDate.isAfter(ZonedDateTime.now())) {
throw new AccessForbiddenException();
}
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ private StudentExam generateIndividualStudentExam(Exam exam, User student) {
* @param exam with eagerly loaded registered users, exerciseGroups and exercises loaded
* @return the list of student exams with their corresponding users
*/
@Transactional
@Transactional // TODO: NOT OK --> remove @Transactional
public List<StudentExam> generateStudentExams(final Exam exam) {
final var existingStudentExams = studentExamRepository.findByExamId(exam.getId());
// https://jira.spring.io/browse/DATAJPA-1367 deleteInBatch does not work, because it does not cascade the deletion of existing exam sessions, therefore use deleteAll
Expand All @@ -827,7 +827,7 @@ public List<StudentExam> generateStudentExams(final Exam exam) {
* @param exam with eagerly loaded registered users, exerciseGroups and exercises loaded
* @return the list of student exams with their corresponding users
*/
@Transactional
@Transactional // TODO: NOT OK --> remove @Transactional
public List<StudentExam> generateMissingStudentExams(Exam exam) {

// Get all users who already have an individual exam
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.function.Predicate;
import java.util.stream.Stream;

import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;

import org.springframework.context.annotation.Profile;
Expand Down Expand Up @@ -59,7 +60,7 @@ public PlagiarismService(PlagiarismComparisonRepository plagiarismComparisonRepo
* @param userLogin the user login of the student asking to see his plagiarism comparison.
* @param exerciseDueDate due date of the exercise.
*/
public void checkAccessAndAnonymizeSubmissionForStudent(Submission submission, String userLogin, ZonedDateTime exerciseDueDate) {
public void checkAccessAndAnonymizeSubmissionForStudent(Submission submission, String userLogin, @Nullable ZonedDateTime exerciseDueDate) {
if (!hasAccessToSubmission(submission.getId(), userLogin, exerciseDueDate)) {
throw new AccessForbiddenException("This plagiarism submission is not related to the requesting user or the user has not been notified yet.");
}
Expand Down Expand Up @@ -87,17 +88,18 @@ public boolean hasPlagiarismComparison(long submissionId) {
* @param exerciseDueDate due date of the exercise.
* @return true is the user has access to the submission
*/
public boolean hasAccessToSubmission(Long submissionId, String userLogin, ZonedDateTime exerciseDueDate) {
public boolean hasAccessToSubmission(Long submissionId, String userLogin, @Nullable ZonedDateTime exerciseDueDate) {
var comparisonOptional = plagiarismComparisonRepository.findBySubmissionA_SubmissionIdOrSubmissionB_SubmissionId(submissionId, submissionId);
return comparisonOptional.filter(not(Set::isEmpty)).isPresent()
&& isOwnSubmissionOrIsAfterExerciseDueDate(submissionId, userLogin, comparisonOptional.get(), exerciseDueDate)
&& wasUserNotifiedByInstructor(userLogin, comparisonOptional.get());
}

private boolean isOwnSubmissionOrIsAfterExerciseDueDate(Long submissionId, String userLogin, Set<PlagiarismComparison<?>> comparisons, ZonedDateTime exerciseDueDate) {
private boolean isOwnSubmissionOrIsAfterExerciseDueDate(Long submissionId, String userLogin, Set<PlagiarismComparison<?>> comparisons,
@Nullable ZonedDateTime exerciseDueDate) {
var isOwnSubmission = comparisons.stream().flatMap(it -> Stream.of(it.getSubmissionA(), it.getSubmissionB())).filter(Objects::nonNull)
.filter(it -> it.getSubmissionId() == submissionId).findFirst().map(PlagiarismSubmission::getStudentLogin).filter(isEqual(userLogin)).isPresent();
return isOwnSubmission || exerciseDueDate.isBefore(ZonedDateTime.now());
return isOwnSubmission || exerciseDueDate == null || exerciseDueDate.isBefore(ZonedDateTime.now());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public ResponseEntity<TextSubmission> getTextSubmissionWithResults(@PathVariable
if (!authCheckService.isAtLeastTeachingAssistantForExercise(textSubmission.getParticipation().getExercise())) {
// anonymize and throw exception if not authorized to view submission
plagiarismService.checkAccessAndAnonymizeSubmissionForStudent(textSubmission, userRepository.getUser().getLogin(),
// TODO: might be better to use the individual due date here if available
textSubmission.getParticipation().getExercise().getDueDate());
return ResponseEntity.ok(textSubmission);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public interface ProgrammingExerciseStudentParticipationTestRepository extends J
*
* @param buildPlanId new build plan id to be set
*/
@Transactional
@Transactional // ok because of modifying query
@Modifying
@Query("""
UPDATE ProgrammingExerciseStudentParticipation p
Expand Down

0 comments on commit 5dacbe2

Please sign in to comment.