Skip to content

Commit

Permalink
Development: Implement small server code and log improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
krusche committed Jan 4, 2025
1 parent 22d38a8 commit 4e56238
Show file tree
Hide file tree
Showing 14 changed files with 45 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public interface DataExportRepository extends ArtemisJpaRepository<DataExport, L
Set<DataExport> findAllToBeCreated();

/**
* Find all data exports that need to be deleted. This includes all data exports that have a creation date older than 7 days
* Find all data exports that need to be deleted. This includes all data exports that have a creation date older than 7 days and that have not been deleted or failed before
*
* @param thresholdDate the date to filter data exports, typically 7 days before today.
* @return a set of data exports that need to be deleted
Expand All @@ -47,6 +47,7 @@ public interface DataExportRepository extends ArtemisJpaRepository<DataExport, L
FROM DataExport dataExport
WHERE dataExport.creationFinishedDate IS NOT NULL
AND dataExport.creationFinishedDate < :thresholdDate
AND dataExport.dataExportState < 4
""")
Set<DataExport> findAllToBeDeleted(@Param("thresholdDate") ZonedDateTime thresholdDate);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
* @param tokenProvider the Artemis token provider used to generate and validate jwt's
* @return the valid jwt or null if not found or invalid
*/
public static @Nullable String extractValidJwt(HttpServletRequest httpServletRequest, TokenProvider tokenProvider) {
@Nullable
public static String extractValidJwt(HttpServletRequest httpServletRequest, TokenProvider tokenProvider) {
var cookie = WebUtils.getCookie(httpServletRequest, JWT_COOKIE_NAME);
var authHeader = httpServletRequest.getHeader(AUTHORIZATION_HEADER);

Expand All @@ -76,8 +77,9 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
}

String jwtToken = cookie != null ? getJwtFromCookie(cookie) : getJwtFromBearer(authHeader);
String source = cookie != null ? "cookie" : "bearer";

if (!isJwtValid(tokenProvider, jwtToken)) {
if (!isJwtValid(tokenProvider, jwtToken, source)) {
return null;
}

Expand All @@ -90,7 +92,8 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
* @param jwtCookie the cookie with Key "jwt"
* @return the jwt or null if not found
*/
private static @Nullable String getJwtFromCookie(@Nullable Cookie jwtCookie) {
@Nullable
private static String getJwtFromCookie(@Nullable Cookie jwtCookie) {
if (jwtCookie == null) {
return null;
}
Expand All @@ -103,7 +106,8 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
* @param jwtBearer the content of the Authorization header
* @return the jwt or null if not found
*/
private static @Nullable String getJwtFromBearer(@Nullable String jwtBearer) {
@Nullable
private static String getJwtFromBearer(@Nullable String jwtBearer) {
if (!StringUtils.hasText(jwtBearer) || !jwtBearer.startsWith(BEARER_PREFIX)) {
return null;
}
Expand All @@ -116,10 +120,11 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
* Checks if the jwt is valid
*
* @param tokenProvider the Artemis token provider used to generate and validate jwt's
* @param jwtToken the jwt
* @param jwtToken the jwt token which should be validated
* @param source the source of the jwt token
* @return true if the jwt is valid, false if missing or invalid
*/
private static boolean isJwtValid(TokenProvider tokenProvider, @Nullable String jwtToken) {
return StringUtils.hasText(jwtToken) && tokenProvider.validateTokenForAuthority(jwtToken);
private static boolean isJwtValid(TokenProvider tokenProvider, @Nullable String jwtToken, @Nullable String source) {
return StringUtils.hasText(jwtToken) && tokenProvider.validateTokenForAuthority(jwtToken, source);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.List;
import java.util.stream.Collectors;

import jakarta.annotation.Nullable;
import jakarta.annotation.PostConstruct;

import javax.crypto.SecretKey;
Expand Down Expand Up @@ -125,19 +126,21 @@ public Authentication getAuthentication(String token) {
* Validate an JWT Authorization Token
*
* @param authToken JWT Authorization Token
* @param source the source of the token
* @return boolean indicating if token is valid
*/
public boolean validateTokenForAuthority(String authToken) {
return validateJwsToken(authToken);
public boolean validateTokenForAuthority(String authToken, @Nullable String source) {
return validateJwsToken(authToken, source);
}

/**
* Validate an JWT Authorization Token
*
* @param authToken JWT Authorization Token
* @param source the source of the token
* @return boolean indicating if token is valid
*/
private boolean validateJwsToken(String authToken) {
private boolean validateJwsToken(String authToken, @Nullable String source) {
try {
parseClaims(authToken);
return true;
Expand All @@ -161,21 +164,15 @@ private boolean validateJwsToken(String authToken) {
catch (IllegalArgumentException e) {
log.error("Token validation error {}", e.getMessage());
}
log.info("Invalid JWT token: {}", authToken);
log.info("Invalid JWT token: {} from source {}", authToken, source);
return false;
}

private Claims parseClaims(String authToken) {
return Jwts.parser().verifyWith(key).build().parseSignedClaims(authToken).getPayload();
}

public <T> T getClaim(String token, String claimName, Class<T> claimType) {
Claims claims = parseClaims(token);
return claims.get(claimName, claimType);
}

public Date getExpirationDate(String authToken) {
return parseClaims(authToken).getExpiration();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ public ContinuousPlagiarismControlService(ExerciseRepository exerciseRepository,
*/
@Scheduled(cron = "${artemis.scheduling.continuous-plagiarism-control-trigger-time:0 0 5 * * *}")
public void executeChecks() {
log.info("Starting continuous plagiarism control...");

var exercises = exerciseRepository.findAllExercisesWithDueDateOnOrAfterYesterdayAndContinuousPlagiarismControlEnabledIsTrue();
log.info("Starting scheduled continuous plagiarism control for {} exercises: {}", exercises.size(), exercises.stream().map(Exercise::getId).toList());
exercises.stream().filter(isBeforeDueDateOrAfterWithPostDueDateChecksEnabled).forEach(exercise -> {
log.info("Started continuous plagiarism control for exercise: exerciseId={}, type={}.", exercise.getId(), exercise.getExerciseType());
final long startTime = System.nanoTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;

import de.jplag.exceptions.ExitException;
import de.tum.cit.aet.artemis.modeling.domain.ModelingExercise;
import de.tum.cit.aet.artemis.plagiarism.domain.PlagiarismResult;
import de.tum.cit.aet.artemis.plagiarism.domain.modeling.ModelingPlagiarismResult;
Expand Down Expand Up @@ -60,7 +59,7 @@ public PlagiarismDetectionService(TextPlagiarismDetectionService textPlagiarismD
* @param exercise exercise to check plagiarism
* @return result of plagiarism checks
*/
public TextPlagiarismResult checkTextExercise(TextExercise exercise) throws ExitException {
public TextPlagiarismResult checkTextExercise(TextExercise exercise) {
var plagiarismResult = textPlagiarismDetectionService.checkPlagiarism(exercise, exercise.getPlagiarismDetectionConfig().getSimilarityThreshold(),
exercise.getPlagiarismDetectionConfig().getMinimumScore(), exercise.getPlagiarismDetectionConfig().getMinimumSize());
log.info("Finished textPlagiarismDetectionService.checkPlagiarism for exercise {} with {} comparisons,", exercise.getId(), plagiarismResult.getComparisons().size());
Expand All @@ -75,8 +74,7 @@ public TextPlagiarismResult checkTextExercise(TextExercise exercise) throws Exit
* @param exercise exercise to check plagiarism
* @return result of plagiarism checks
*/
public TextPlagiarismResult checkProgrammingExercise(ProgrammingExercise exercise)
throws ExitException, IOException, ProgrammingLanguageNotSupportedForPlagiarismDetectionException {
public TextPlagiarismResult checkProgrammingExercise(ProgrammingExercise exercise) throws IOException, ProgrammingLanguageNotSupportedForPlagiarismDetectionException {
checkProgrammingLanguageSupport(exercise);

var plagiarismResult = programmingPlagiarismDetectionService.checkPlagiarism(exercise.getId(), exercise.getPlagiarismDetectionConfig().getSimilarityThreshold(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import de.jplag.clustering.ClusteringOptions;
import de.jplag.cpp.CPPLanguage;
import de.jplag.csharp.CSharpLanguage;
import de.jplag.exceptions.ExitException;
import de.jplag.golang.GoLanguage;
import de.jplag.java.JavaLanguage;
import de.jplag.javascript.JavaScriptLanguage;
Expand Down Expand Up @@ -117,8 +116,7 @@ public ProgrammingPlagiarismDetectionService(FileService fileService, Programmin
* @param minimumScore consider only submissions whose score is greater or equal to this value
* @param minimumSize consider only submissions whose number of lines in diff to template is greater or equal to this value
* @return the text plagiarism result container with up to 500 comparisons with the highest similarity values
* @throws ExitException is thrown if JPlag exits unexpectedly
* @throws IOException is thrown for file handling errors
* @throws IOException is thrown for file handling errors
*/
public TextPlagiarismResult checkPlagiarism(long programmingExerciseId, float similarityThreshold, int minimumScore, int minimumSize) throws IOException {
long start = System.nanoTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import de.jplag.JPlagResult;
import de.jplag.Language;
import de.jplag.clustering.ClusteringOptions;
import de.jplag.exceptions.ExitException;
import de.jplag.options.JPlagOptions;
import de.jplag.text.NaturalLanguage;
import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException;
Expand Down Expand Up @@ -84,7 +83,6 @@ public List<TextSubmission> textSubmissionsForComparison(TextExercise exerciseWi
* @param minimumScore consider only submissions whose score is greater or equal to this value
* @param minimumSize consider only submissions whose size is greater or equal to this value
* @return a zip file that can be returned to the client
* @throws ExitException is thrown if JPlag exits unexpectedly
*/
public TextPlagiarismResult checkPlagiarism(TextExercise textExercise, float similarityThreshold, int minimumScore, int minimumSize) {
// Only one plagiarism check per course allowed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,6 @@ private ProgrammingExercise retrieveProgrammingExerciseByBuildJobId(String build
* In case of an error during file deletion, it logs the error and continues processing.
* </p>
*
* @throws IOException if an I/O error occurs while accessing the build log files directory or
* deleting files.
*/
@Scheduled(cron = "${artemis.continuous-integration.build-log.cleanup-schedule:0 0 3 * * ?}")
public void deleteOldBuildLogsFiles() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public ResponseEntity<PlagiarismResultDTO<TextPlagiarismResult>> checkPlagiarism
log.info("Started manual plagiarism checks for programming exercise: exerciseId={}.", exerciseId);
PlagiarismDetectionConfigHelper.updateWithTemporaryParameters(programmingExercise, similarityThreshold, minimumScore, minimumSize);
try {
var plagiarismResult = (TextPlagiarismResult) plagiarismDetectionService.checkProgrammingExercise(programmingExercise);
var plagiarismResult = plagiarismDetectionService.checkProgrammingExercise(programmingExercise);
return buildPlagiarismResultResponse(plagiarismResult);
}
catch (ProgrammingLanguageNotSupportedForPlagiarismDetectionException e) {
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/config/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ logging:
org.springframework.web.socket.config: INFO
liquibase: INFO
de.jplag.Submission: ERROR
de.jplag.logging: WARN
de.jplag.reporting: WARN
logback:
rollingpolicy:
max-history: 90
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ void testJWTAuthentication() throws Exception {

var responseBody = new ObjectMapper().readValue(response.getContentAsString(), new TypeReference<Map<String, Object>>() {
});
assertThat(tokenProvider.validateTokenForAuthority(responseBody.get("access_token").toString())).isTrue();
assertThat(tokenProvider.validateTokenForAuthority(responseBody.get("access_token").toString(), null)).isTrue();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void testValidTokenShouldNotCountAnything() {

String validToken = createValidToken();

tokenProvider.validateTokenForAuthority(validToken);
tokenProvider.validateTokenForAuthority(validToken, null);

assertThat(aggregate(counters)).isZero();
}
Expand All @@ -73,7 +73,7 @@ void testTokenExpiredCount() {

String expiredToken = createExpiredToken();

tokenProvider.validateTokenForAuthority(expiredToken);
tokenProvider.validateTokenForAuthority(expiredToken, null);

assertThat(meterRegistry.get(INVALID_TOKENS_METER_EXPECTED_NAME).tag("cause", "expired").counter().count()).isEqualTo(1);
}
Expand All @@ -84,7 +84,7 @@ void testTokenUnsupportedCount() {

String unsupportedToken = createUnsupportedToken();

tokenProvider.validateTokenForAuthority(unsupportedToken);
tokenProvider.validateTokenForAuthority(unsupportedToken, null);

assertThat(meterRegistry.get(INVALID_TOKENS_METER_EXPECTED_NAME).tag("cause", "unsupported").counter().count()).isEqualTo(1);
}
Expand All @@ -95,7 +95,7 @@ void testTokenSignatureInvalidCount() {

String tokenWithDifferentSignature = createTokenWithDifferentSignature();

tokenProvider.validateTokenForAuthority(tokenWithDifferentSignature);
tokenProvider.validateTokenForAuthority(tokenWithDifferentSignature, null);

assertThat(meterRegistry.get(INVALID_TOKENS_METER_EXPECTED_NAME).tag("cause", "invalid-signature").counter().count()).isEqualTo(1);
}
Expand All @@ -106,7 +106,7 @@ void testTokenMalformedCount() {

String malformedToken = createMalformedToken();

tokenProvider.validateTokenForAuthority(malformedToken);
tokenProvider.validateTokenForAuthority(malformedToken, null);

assertThat(meterRegistry.get(INVALID_TOKENS_METER_EXPECTED_NAME).tag("cause", "malformed").counter().count()).isEqualTo(1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void setup() {

@Test
void testReturnFalseWhenJWThasInvalidSignature() {
boolean isTokenValid = tokenProvider.validateTokenForAuthority(createTokenWithDifferentSignature());
boolean isTokenValid = tokenProvider.validateTokenForAuthority(createTokenWithDifferentSignature(), null);

assertThat(isTokenValid).isFalse();
}
Expand All @@ -65,7 +65,7 @@ void testReturnFalseWhenJWTisMalformed() {
Authentication authentication = createAuthentication();
String token = tokenProvider.createToken(authentication, false);
String invalidToken = token.substring(1);
boolean isTokenValid = tokenProvider.validateTokenForAuthority(invalidToken);
boolean isTokenValid = tokenProvider.validateTokenForAuthority(invalidToken, null);

assertThat(isTokenValid).isFalse();
}
Expand All @@ -77,7 +77,7 @@ void testReturnFalseWhenJWTisExpired() {
Authentication authentication = createAuthentication();
String token = tokenProvider.createToken(authentication, false);

boolean isTokenValid = tokenProvider.validateTokenForAuthority(token);
boolean isTokenValid = tokenProvider.validateTokenForAuthority(token, null);

assertThat(isTokenValid).isFalse();
}
Expand All @@ -86,14 +86,14 @@ void testReturnFalseWhenJWTisExpired() {
void testReturnFalseWhenJWTisUnsupported() {
String unsupportedToken = createUnsupportedToken();

boolean isTokenValid = tokenProvider.validateTokenForAuthority(unsupportedToken);
boolean isTokenValid = tokenProvider.validateTokenForAuthority(unsupportedToken, null);

assertThat(isTokenValid).isFalse();
}

@Test
void testReturnFalseWhenJWTisInvalid() {
boolean isTokenValid = tokenProvider.validateTokenForAuthority("");
boolean isTokenValid = tokenProvider.validateTokenForAuthority("", null);

assertThat(isTokenValid).isFalse();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import org.junit.jupiter.api.Test;

import de.jplag.exceptions.BasecodeException;
import de.jplag.exceptions.ExitException;
import de.tum.cit.aet.artemis.core.domain.Course;
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.exercise.domain.Exercise;
Expand Down Expand Up @@ -69,7 +67,7 @@ class ContinuousPlagiarismControlServiceTest {
plagiarismCaseService, plagiarismCaseRepository, plagiarismPostService, plagiarismResultRepository);

@Test
void shouldExecuteChecks() throws ExitException, IOException, ProgrammingLanguageNotSupportedForPlagiarismDetectionException {
void shouldExecuteChecks() throws IOException, ProgrammingLanguageNotSupportedForPlagiarismDetectionException {
// given: text exercise with cpc enabled
var textExercise = new TextExercise();
textExercise.setId(101L);
Expand Down Expand Up @@ -114,7 +112,7 @@ void shouldExecuteChecks() throws ExitException, IOException, ProgrammingLanguag
}

@Test
void shouldAddSubmissionsToPlagiarismCase() throws ExitException {
void shouldAddSubmissionsToPlagiarismCase() {
// given: text exercise with cpc enabled
var exercise = new TextExercise();
exercise.setId(99L);
Expand Down Expand Up @@ -152,7 +150,7 @@ void shouldAddSubmissionsToPlagiarismCase() throws ExitException {
}

@Test
void shouldRemoveStalePlagiarismCase() throws ExitException {
void shouldRemoveStalePlagiarismCase() {
// given: text exercise with cpc enabled
var exercise = new TextExercise();
exercise.setId(99L);
Expand Down Expand Up @@ -211,20 +209,20 @@ void shouldDoNothingForFileUploadAndQuizExercises() {
}

@Test
void shouldSilentAnyJPlagExceptionsThrown() throws Exception {
void shouldSilentAnyJPlagExceptionsThrown() {
// given
var textExercise = new TextExercise();
textExercise.setId(123L);
when(exerciseRepository.findAllExercisesWithDueDateOnOrAfterYesterdayAndContinuousPlagiarismControlEnabledIsTrue()).thenReturn(singleton(textExercise));
when(plagiarismChecksService.checkTextExercise(textExercise)).thenThrow(new BasecodeException("msg"));
when(plagiarismChecksService.checkTextExercise(textExercise)).thenThrow(new NullPointerException("null"));

// then
assertThatNoException().isThrownBy(service::executeChecks);
verify(plagiarismResultRepository).deletePlagiarismResultsByExerciseId(123L);
}

@Test
void shouldSilentAnyUnknownExceptionsThrown() throws Exception {
void shouldSilentAnyUnknownExceptionsThrown() {
// given
var textExercise = new TextExercise();
textExercise.setId(101L);
Expand Down

0 comments on commit 4e56238

Please sign in to comment.