Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Add simple repository service #9871

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,6 @@ public interface CompetencyRepository extends ArtemisJpaRepository<Competency, L
""")
Optional<Competency> findByIdWithLectureUnitsAndExercises(@Param("competencyId") long competencyId);

default Competency findByIdWithLectureUnitsAndExercisesElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithLectureUnitsAndExercises(competencyId), competencyId);
}

default Competency findByIdWithLectureUnitsElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithLectureUnits(competencyId), competencyId);
}

long countByCourse(Course course);

List<Competency> findByCourseIdOrderById(long courseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import de.tum.cit.aet.artemis.atlas.domain.LearningObject;
import de.tum.cit.aet.artemis.atlas.domain.competency.CourseCompetency;
import de.tum.cit.aet.artemis.atlas.dto.metrics.CompetencyExerciseMasteryCalculationDTO;
import de.tum.cit.aet.artemis.atlas.dto.metrics.CompetencyLectureUnitMasteryCalculationDTO;
Expand Down Expand Up @@ -80,10 +79,6 @@ public interface CourseCompetencyRepository extends ArtemisJpaRepository<CourseC
""")
Optional<CourseCompetency> findByIdWithExercisesAndLectureUnitsAndLectures(@Param("id") long id);

default CourseCompetency findByIdWithExercisesAndLectureUnitsAndLecturesElseThrow(long id) {
return getValueElseThrow(findByIdWithExercisesAndLectureUnitsAndLectures(id), id);
}

@Query("""
SELECT c
FROM CourseCompetency c
Expand Down Expand Up @@ -259,40 +254,6 @@ Page<CourseCompetency> findForImportAndUserHasAccessToCourse(@Param("partialTitl
""")
Optional<CourseCompetency> findByIdWithLectureUnitsAndExercises(@Param("competencyId") long competencyId);

default CourseCompetency findByIdWithLectureUnitsElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithLectureUnits(competencyId), competencyId);
}

default CourseCompetency findByIdWithExercisesAndLectureUnitsElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithExercisesAndLectureUnits(competencyId), competencyId);
}

default CourseCompetency findByIdWithExercisesAndLectureUnitsBidirectionalElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithExercisesAndLectureUnitsBidirectional(competencyId), competencyId);
}

/**
* Finds the set of ids of course competencies that are linked to a given learning object
*
* @param learningObject the learning object to find the course competencies for
* @return the set of ids of course competencies linked to the learning object
*/
default Set<Long> findAllIdsByLearningObject(LearningObject learningObject) {
return switch (learningObject) {
case LectureUnit lectureUnit -> findAllIdsByLectureUnit(lectureUnit);
case Exercise exercise -> findAllIdsByExercise(exercise);
default -> throw new IllegalArgumentException("Unknown LearningObject type: " + learningObject.getClass());
};
}

default CourseCompetency findByIdWithExercisesElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithExercises(competencyId), competencyId);
}

default CourseCompetency findByIdWithLectureUnitsAndExercisesElseThrow(long competencyId) {
return getValueElseThrow(findByIdWithLectureUnitsAndExercises(competencyId), competencyId);
}

List<CourseCompetency> findByCourseIdOrderById(long courseId);

boolean existsByIdAndCourseId(long competencyId, long courseId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package de.tum.cit.aet.artemis.atlas.repository.simple;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;

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

import de.tum.cit.aet.artemis.atlas.domain.competency.Competency;
import de.tum.cit.aet.artemis.atlas.repository.CompetencyRepository;
import de.tum.cit.aet.artemis.core.repository.base.AbstractSimpleService;

@Profile(PROFILE_CORE)
@Service
public class CompetencySimpleService extends AbstractSimpleService<Competency> {

private final static String ENTITY_NAME = "Competency";

private final CompetencyRepository competencyRepository;

public CompetencySimpleService(CompetencyRepository competencyRepository) {
this.competencyRepository = competencyRepository;
}

public Competency findByIdWithLectureUnitsAndExercisesElseThrow(long competencyId) {
return getValueElseThrow(competencyRepository.findByIdWithLectureUnitsAndExercises(competencyId), ENTITY_NAME);
}

public Competency findByIdWithLectureUnitsElseThrow(long competencyId) {
return getValueElseThrow(competencyRepository.findByIdWithLectureUnitsAndExercises(competencyId), ENTITY_NAME);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package de.tum.cit.aet.artemis.atlas.repository.simple;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;

import java.util.Set;

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

import de.tum.cit.aet.artemis.atlas.domain.LearningObject;
import de.tum.cit.aet.artemis.atlas.domain.competency.CourseCompetency;
import de.tum.cit.aet.artemis.atlas.repository.CourseCompetencyRepository;
import de.tum.cit.aet.artemis.core.repository.base.AbstractSimpleService;
import de.tum.cit.aet.artemis.exercise.domain.Exercise;
import de.tum.cit.aet.artemis.lecture.domain.LectureUnit;

@Profile(PROFILE_CORE)
@Service
public class CourseCompetencySimpleService extends AbstractSimpleService<CourseCompetency> {

private static final String ENTITY_NAME = "CourseCompetency";

private final CourseCompetencyRepository courseCompetencyRepository;

public CourseCompetencySimpleService(CourseCompetencyRepository courseCompetencyRepository) {
this.courseCompetencyRepository = courseCompetencyRepository;
}

public CourseCompetency findByIdWithExercisesAndLectureUnitsAndLecturesElseThrow(long id) {
return getValueElseThrow(courseCompetencyRepository.findByIdWithExercisesAndLectureUnitsAndLectures(id), ENTITY_NAME);
}

public CourseCompetency findByIdWithExercisesAndLectureUnitsElseThrow(long competencyId) {
return getValueElseThrow(courseCompetencyRepository.findByIdWithExercisesAndLectureUnits(competencyId), ENTITY_NAME);
}

public CourseCompetency findByIdWithExercisesAndLectureUnitsBidirectionalElseThrow(long competencyId) {
return getValueElseThrow(courseCompetencyRepository.findByIdWithExercisesAndLectureUnitsBidirectional(competencyId), ENTITY_NAME);
}

/**
* Finds the set of ids of course competencies that are linked to a given learning object
*
* @param learningObject the learning object to find the course competencies for
* @return the set of ids of course competencies linked to the learning object
*/
public Set<Long> findAllIdsByLearningObject(LearningObject learningObject) {
return switch (learningObject) {
case LectureUnit lectureUnit -> courseCompetencyRepository.findAllIdsByLectureUnit(lectureUnit);
case Exercise exercise -> courseCompetencyRepository.findAllIdsByExercise(exercise);
default -> throw new IllegalArgumentException("Unknown LearningObject type: " + learningObject.getClass());
};
}
Comment on lines +47 to +53
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential NullPointerException if learningObject is null

The method findAllIdsByLearningObject does not check if learningObject is null. Passing a null value will result in a NullPointerException at runtime.

Consider adding a null check at the beginning of the method:

+        if (learningObject == null) {
+            throw new IllegalArgumentException("LearningObject must not be null");
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Set<Long> findAllIdsByLearningObject(LearningObject learningObject) {
return switch (learningObject) {
case LectureUnit lectureUnit -> courseCompetencyRepository.findAllIdsByLectureUnit(lectureUnit);
case Exercise exercise -> courseCompetencyRepository.findAllIdsByExercise(exercise);
default -> throw new IllegalArgumentException("Unknown LearningObject type: " + learningObject.getClass());
};
}
public Set<Long> findAllIdsByLearningObject(LearningObject learningObject) {
if (learningObject == null) {
throw new IllegalArgumentException("LearningObject must not be null");
}
return switch (learningObject) {
case LectureUnit lectureUnit -> courseCompetencyRepository.findAllIdsByLectureUnit(lectureUnit);
case Exercise exercise -> courseCompetencyRepository.findAllIdsByExercise(exercise);
default -> throw new IllegalArgumentException("Unknown LearningObject type: " + learningObject.getClass());
};
}


public CourseCompetency findByIdWithExercisesElseThrow(long competencyId) {
return getValueElseThrow(courseCompetencyRepository.findByIdWithExercises(competencyId), ENTITY_NAME);
}

public CourseCompetency findByIdWithLectureUnitsAndExercisesElseThrow(long competencyId) {
return getValueElseThrow(courseCompetencyRepository.findByIdWithLectureUnitsAndExercises(competencyId), ENTITY_NAME);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import de.tum.cit.aet.artemis.atlas.dto.metrics.CompetencyLectureUnitMasteryCalculationDTO;
import de.tum.cit.aet.artemis.atlas.repository.CompetencyProgressRepository;
import de.tum.cit.aet.artemis.atlas.repository.CourseCompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.simple.CourseCompetencySimpleService;
import de.tum.cit.aet.artemis.atlas.service.learningpath.LearningPathService;
import de.tum.cit.aet.artemis.core.domain.Course;
import de.tum.cit.aet.artemis.core.domain.User;
Expand Down Expand Up @@ -62,6 +63,8 @@ public class CompetencyProgressService {

private final CourseCompetencyRepository courseCompetencyRepository;

private final CourseCompetencySimpleService courseCompetencySimpleRepository;

private static final int MIN_EXERCISES_RECENCY_CONFIDENCE = 3;

private static final int MAX_SUBMISSIONS_FOR_QUICK_SOLVE_HEURISTIC = 3;
Expand All @@ -75,13 +78,14 @@ public class CompetencyProgressService {
private static final double CONFIDENCE_REASON_DEADZONE = 0.05;

public CompetencyProgressService(CompetencyProgressRepository competencyProgressRepository, LearningPathService learningPathService,
ParticipantScoreService participantScoreService, LectureUnitCompletionRepository lectureUnitCompletionRepository,
CourseCompetencyRepository courseCompetencyRepository) {
ParticipantScoreService participantScoreService, LectureUnitCompletionRepository lectureUnitCompletionRepository, CourseCompetencyRepository courseCompetencyRepository,
CourseCompetencySimpleService courseCompetencySimpleRepository) {
this.competencyProgressRepository = competencyProgressRepository;
this.learningPathService = learningPathService;
this.participantScoreService = participantScoreService;
this.lectureUnitCompletionRepository = lectureUnitCompletionRepository;
this.courseCompetencyRepository = courseCompetencyRepository;
this.courseCompetencySimpleRepository = courseCompetencySimpleRepository;
}

/**
Expand All @@ -104,7 +108,7 @@ public void updateProgressByLearningObjectForParticipantAsync(LearningObject lea
@Async
public void updateProgressByLearningObjectAsync(LearningObject learningObject) {
SecurityUtils.setAuthorizationObject(); // Required for async
Set<Long> competencyIds = courseCompetencyRepository.findAllIdsByLearningObject(learningObject);
Set<Long> competencyIds = courseCompetencySimpleRepository.findAllIdsByLearningObject(learningObject);

for (long competencyId : competencyIds) {
Set<User> users = competencyProgressRepository.findAllByCompetencyId(competencyId).stream().map(CompetencyProgress::getUser).collect(Collectors.toSet());
Expand Down Expand Up @@ -183,7 +187,7 @@ private void updateProgressByCompetencyIdsAndLearningObject(Set<Long> competency
* @param users The users for which to update the progress
*/
public void updateProgressByLearningObjectSync(LearningObject learningObject, Set<User> users) {
Set<Long> competencyIds = courseCompetencyRepository.findAllIdsByLearningObject(learningObject);
Set<Long> competencyIds = courseCompetencySimpleRepository.findAllIdsByLearningObject(learningObject);

for (long competencyId : competencyIds) {
log.debug("Updating competency progress synchronously for {} users.", users.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import de.tum.cit.aet.artemis.atlas.repository.CompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.CourseCompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.StandardizedCompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.simple.CompetencySimpleService;
import de.tum.cit.aet.artemis.atlas.repository.simple.CourseCompetencySimpleService;
import de.tum.cit.aet.artemis.atlas.service.LearningObjectImportService;
import de.tum.cit.aet.artemis.atlas.service.learningpath.LearningPathService;
import de.tum.cit.aet.artemis.core.domain.Course;
Expand All @@ -42,17 +44,21 @@ public class CompetencyService extends CourseCompetencyService {

private final CompetencyRepository competencyRepository;

private final CompetencySimpleService competencySimpleRepository;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Rename field to match its type

The field is of type CompetencySimpleService but named competencySimpleRepository. This naming inconsistency could lead to confusion.

-    private final CompetencySimpleService competencySimpleRepository;
+    private final CompetencySimpleService competencySimpleService;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private final CompetencySimpleService competencySimpleRepository;
private final CompetencySimpleService competencySimpleService;


private final CompetencyExerciseLinkRepository competencyExerciseLinkRepository;

public CompetencyService(CompetencyRepository competencyRepository, AuthorizationCheckService authCheckService, CompetencyRelationRepository competencyRelationRepository,
LearningPathService learningPathService, CompetencyProgressService competencyProgressService, LectureUnitService lectureUnitService,
CompetencyProgressRepository competencyProgressRepository, LectureUnitCompletionRepository lectureUnitCompletionRepository,
CourseCompetencySimpleService courseCompetencySimpleRepository, LearningPathService learningPathService, CompetencyProgressService competencyProgressService,
LectureUnitService lectureUnitService, CompetencyProgressRepository competencyProgressRepository, LectureUnitCompletionRepository lectureUnitCompletionRepository,
StandardizedCompetencyRepository standardizedCompetencyRepository, CourseCompetencyRepository courseCompetencyRepository, ExerciseService exerciseService,
LearningObjectImportService learningObjectImportService, CompetencyLectureUnitLinkRepository competencyLectureUnitLinkRepository, CourseRepository courseRepository,
CompetencyExerciseLinkRepository competencyExerciseLinkRepository) {
super(competencyProgressRepository, courseCompetencyRepository, competencyRelationRepository, competencyProgressService, exerciseService, lectureUnitService,
learningPathService, authCheckService, standardizedCompetencyRepository, lectureUnitCompletionRepository, learningObjectImportService, courseRepository);
CompetencySimpleService competencySimpleRepository, CompetencyExerciseLinkRepository competencyExerciseLinkRepository) {
super(competencyProgressRepository, courseCompetencyRepository, courseCompetencySimpleRepository, competencyRelationRepository, competencyProgressService, exerciseService,
lectureUnitService, learningPathService, authCheckService, standardizedCompetencyRepository, lectureUnitCompletionRepository, learningObjectImportService,
courseRepository);
this.competencyRepository = competencyRepository;
this.competencySimpleRepository = competencySimpleRepository;
this.competencyExerciseLinkRepository = competencyExerciseLinkRepository;
}

Expand Down Expand Up @@ -100,7 +106,7 @@ public List<Competency> createCompetencies(List<Competency> competencies, Course
* @return The found competency
*/
public Competency findCompetencyWithExercisesAndLectureUnitsAndProgressForUser(Long competencyId, Long userId) {
Competency competency = competencyRepository.findByIdWithLectureUnitsAndExercisesElseThrow(competencyId);
Competency competency = competencySimpleRepository.findByIdWithLectureUnitsAndExercisesElseThrow(competencyId);
return findProgressAndLectureUnitCompletionsForUser(competency, userId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import de.tum.cit.aet.artemis.atlas.repository.CompetencyRelationRepository;
import de.tum.cit.aet.artemis.atlas.repository.CourseCompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.StandardizedCompetencyRepository;
import de.tum.cit.aet.artemis.atlas.repository.simple.CourseCompetencySimpleService;
import de.tum.cit.aet.artemis.atlas.service.LearningObjectImportService;
import de.tum.cit.aet.artemis.atlas.service.learningpath.LearningPathService;
import de.tum.cit.aet.artemis.core.domain.Course;
Expand Down Expand Up @@ -64,6 +65,8 @@ public class CourseCompetencyService {

protected final CourseCompetencyRepository courseCompetencyRepository;

private final CourseCompetencySimpleService courseCompetencySimpleRepository;

protected final CompetencyRelationRepository competencyRelationRepository;

protected final CompetencyProgressService competencyProgressService;
Expand All @@ -85,12 +88,13 @@ public class CourseCompetencyService {
private final CourseRepository courseRepository;

public CourseCompetencyService(CompetencyProgressRepository competencyProgressRepository, CourseCompetencyRepository courseCompetencyRepository,
CompetencyRelationRepository competencyRelationRepository, CompetencyProgressService competencyProgressService, ExerciseService exerciseService,
LectureUnitService lectureUnitService, LearningPathService learningPathService, AuthorizationCheckService authCheckService,
StandardizedCompetencyRepository standardizedCompetencyRepository, LectureUnitCompletionRepository lectureUnitCompletionRepository,
LearningObjectImportService learningObjectImportService, CourseRepository courseRepository) {
CourseCompetencySimpleService courseCompetencySimpleRepository, CompetencyRelationRepository competencyRelationRepository,
CompetencyProgressService competencyProgressService, ExerciseService exerciseService, LectureUnitService lectureUnitService, LearningPathService learningPathService,
AuthorizationCheckService authCheckService, StandardizedCompetencyRepository standardizedCompetencyRepository,
LectureUnitCompletionRepository lectureUnitCompletionRepository, LearningObjectImportService learningObjectImportService, CourseRepository courseRepository) {
this.competencyProgressRepository = competencyProgressRepository;
this.courseCompetencyRepository = courseCompetencyRepository;
this.courseCompetencySimpleRepository = courseCompetencySimpleRepository;
this.competencyRelationRepository = competencyRelationRepository;
this.competencyProgressService = competencyProgressService;
this.exerciseService = exerciseService;
Expand All @@ -113,7 +117,7 @@ public CourseCompetencyService(CompetencyProgressRepository competencyProgressRe
* @return The found competency
*/
public CourseCompetency findCompetencyWithExercisesAndLectureUnitsAndProgressForUser(Long competencyId, Long userId) {
CourseCompetency competency = courseCompetencyRepository.findByIdWithLectureUnitsAndExercisesElseThrow(competencyId);
CourseCompetency competency = courseCompetencySimpleRepository.findByIdWithLectureUnitsAndExercisesElseThrow(competencyId);
return findProgressAndLectureUnitCompletionsForUser(competency, userId);
}

Expand Down
Loading
Loading