From bf487a8a274e702cfac97d5365faa4a6819b6246 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Fri, 27 Oct 2023 15:53:00 +0200 Subject: [PATCH 1/9] unify JsonObject --- .../artemis/domain/lti/Lti13AgsClaim.java | 13 +++++---- .../domain/lti/Lti13LaunchRequest.java | 6 ++-- .../security/lti/Lti13LaunchFilter.java | 7 +++-- .../service/connectors/lti/Lti13Service.java | 21 +++++++------- .../tum/in/www1/artemis/ArchitectureTest.java | 12 ++++++++ .../ImprintResourceIntegrationTest.java | 11 ++++---- ...ivacyStatementResourceIntegrationTest.java | 11 ++++---- .../artemis/connectors/Lti13ServiceTest.java | 28 +++++++++---------- ...dResultBitbucketBambooIntegrationTest.java | 15 +++++----- ...AndResultGitlabJenkinsIntegrationTest.java | 9 +++--- .../security/Lti13LaunchFilterTest.java | 13 +++++---- ...ngExerciseFeedbackCreationServiceTest.java | 12 ++++---- 12 files changed, 87 insertions(+), 71 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java index a744a7da69c9..d6999f460047 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java @@ -4,8 +4,9 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; -import com.nimbusds.jose.shaded.json.JSONArray; -import com.nimbusds.jose.shaded.json.JSONObject; +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; /** * A wrapper class for an LTI 1.3 Assignment and Grading Services Claim. We support the Score Publishing Service in order to transmit scores. @@ -23,23 +24,23 @@ public class Lti13AgsClaim { * @return an Ags-Claim if one was present in idToken. */ public static Optional from(OidcIdToken idToken) { - JSONObject agsClaimJson = idToken.getClaim(Claims.AGS_CLAIM); + JsonObject agsClaimJson = idToken.getClaim(Claims.AGS_CLAIM); if (agsClaimJson == null) { return Optional.empty(); } Lti13AgsClaim agsClaim = new Lti13AgsClaim(); - JSONArray scopes = (JSONArray) agsClaimJson.get("scope"); + JsonArray scopes = agsClaimJson.get("scope").getAsJsonArray(); if (scopes == null) { return Optional.empty(); } - if (scopes.contains(Scopes.AGS_SCORE)) { + if (scopes.contains(new JsonPrimitive(Scopes.AGS_SCORE))) { agsClaim.setScope(Collections.singletonList(Scopes.AGS_SCORE)); } - agsClaim.setLineItem((String) agsClaimJson.get("lineitem")); + agsClaim.setLineItem(agsClaimJson.get("lineitem").getAsString()); return Optional.of(agsClaim); } diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java index 45029132ef76..faeee4175d0e 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java @@ -3,7 +3,7 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.util.Assert; -import com.nimbusds.jose.shaded.json.JSONObject; +import com.google.gson.JsonObject; public class Lti13LaunchRequest { @@ -24,8 +24,8 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) { this.sub = ltiIdToken.getClaim("sub"); this.deploymentId = ltiIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID); - JSONObject resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK); - this.resourceLinkId = resourceLinkClaim != null ? (String) resourceLinkClaim.get("id") : null; + JsonObject resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK); + this.resourceLinkId = resourceLinkClaim != null ? resourceLinkClaim.get("id").getAsString() : null; this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI); this.agsClaim = Lti13AgsClaim.from(ltiIdToken).orElse(null); diff --git a/src/main/java/de/tum/in/www1/artemis/security/lti/Lti13LaunchFilter.java b/src/main/java/de/tum/in/www1/artemis/security/lti/Lti13LaunchFilter.java index c29b4584e56a..359b8306c961 100644 --- a/src/main/java/de/tum/in/www1/artemis/security/lti/Lti13LaunchFilter.java +++ b/src/main/java/de/tum/in/www1/artemis/security/lti/Lti13LaunchFilter.java @@ -19,9 +19,10 @@ import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.util.UriComponentsBuilder; +import com.google.gson.JsonObject; + import de.tum.in.www1.artemis.domain.lti.Claims; import de.tum.in.www1.artemis.service.connectors.lti.Lti13Service; -import net.minidev.json.JSONObject; import uk.ac.ox.ctl.lti13.security.oauth2.client.lti.authentication.OidcAuthenticationToken; import uk.ac.ox.ctl.lti13.security.oauth2.client.lti.web.OAuth2LoginAuthenticationFilter; @@ -90,8 +91,8 @@ private void writeResponse(String targetLinkUri, HttpServletResponse response) t UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(targetLinkUri); lti13Service.buildLtiResponse(uriBuilder, response); - JSONObject json = new JSONObject(); - json.put("targetLinkUri", uriBuilder.build().toUriString()); + JsonObject json = new JsonObject(); + json.addProperty("targetLinkUri", uriBuilder.build().toUriString()); response.setContentType("application/json"); response.setCharacterEncoding("UTF-8"); diff --git a/src/main/java/de/tum/in/www1/artemis/service/connectors/lti/Lti13Service.java b/src/main/java/de/tum/in/www1/artemis/service/connectors/lti/Lti13Service.java index 7697abf862d6..c4b747ffe608 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/connectors/lti/Lti13Service.java +++ b/src/main/java/de/tum/in/www1/artemis/service/connectors/lti/Lti13Service.java @@ -26,6 +26,8 @@ import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; +import com.google.gson.JsonObject; + import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.lti.*; import de.tum.in.www1.artemis.domain.participation.StudentParticipation; @@ -33,7 +35,6 @@ import de.tum.in.www1.artemis.security.lti.Lti13TokenRetriever; import de.tum.in.www1.artemis.service.OnlineCourseConfigurationService; import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException; -import net.minidev.json.JSONObject; @Service @Profile("lti") @@ -222,15 +223,15 @@ private String getScoresUrl(String lineItemUrl) { } private String getScoreBody(String userId, String comment, Double score) { - JSONObject requestBody = new JSONObject(); - requestBody.put("userId", userId); - requestBody.put("timestamp", (new DateTime()).toString()); - requestBody.put("activityProgress", "Submitted"); - requestBody.put("gradingProgress", "FullyGraded"); - requestBody.put("comment", comment); - requestBody.put("scoreGiven", score); - requestBody.put("scoreMaximum", 100D); - return requestBody.toJSONString(); + JsonObject requestBody = new JsonObject(); + requestBody.addProperty("userId", userId); + requestBody.addProperty("timestamp", (new DateTime()).toString()); + requestBody.addProperty("activityProgress", "Submitted"); + requestBody.addProperty("gradingProgress", "FullyGraded"); + requestBody.addProperty("comment", comment); + requestBody.addProperty("scoreGiven", score); + requestBody.addProperty("scoreMaximum", 100D); + return requestBody.toString(); } /** diff --git a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java index 67157ec29ac6..8b821ba54c66 100644 --- a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java +++ b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java @@ -107,6 +107,18 @@ void testLogging() { GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS.check(classes); } + @Test + void testJSONImplementations() { + ArchRule jsonObject = noClasses().should() + .dependOnClassesThat(have(simpleName("JsonObject")).or(have(simpleName("JSONObject"))).and(not(resideInAPackage("com.google.gson")))); + + ArchRule jsonArray = noClasses().should() + .dependOnClassesThat(have(simpleName("JsonArray")).or(have(simpleName("JSONArray"))).and(not(resideInAPackage("com.google.gson")))); + + jsonObject.check(allClasses); + jsonArray.check(allClasses); + } + // Custom Predicates for JavaAnnotations since ArchUnit only defines them for classes private DescribedPredicate> simpleNameAnnotation(String name) { diff --git a/src/test/java/de/tum/in/www1/artemis/ImprintResourceIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/ImprintResourceIntegrationTest.java index bca382ba465c..f7495bc349ef 100644 --- a/src/test/java/de/tum/in/www1/artemis/ImprintResourceIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/ImprintResourceIntegrationTest.java @@ -17,9 +17,10 @@ import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.google.gson.JsonObject; + import de.tum.in.www1.artemis.domain.Imprint; import de.tum.in.www1.artemis.domain.enumeration.Language; -import net.minidev.json.JSONObject; class ImprintResourceIntegrationTest extends AbstractSpringIntegrationIndependentTest { @@ -212,10 +213,10 @@ void testUpdateImprint_writesFile_ReturnsUpdatedFileContent() throws Exception { @Test @WithMockUser(username = TEST_PREFIX + "admin", roles = "ADMIN") void testUpdateImprint_unsupportedLanguageBadRequest() throws Exception { - JSONObject body = new JSONObject(); - body.put("text", "test"); - body.put("language", "FRENCH"); - request.put("/api/admin/imprint", body, HttpStatus.BAD_REQUEST); + JsonObject body = new JsonObject(); + body.addProperty("text", "test"); + body.addProperty("language", "FRENCH"); + request.put("/api/admin/imprint", body.toString(), HttpStatus.BAD_REQUEST); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/PrivacyStatementResourceIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/PrivacyStatementResourceIntegrationTest.java index 8bb40f27e639..2823b8b23e65 100644 --- a/src/test/java/de/tum/in/www1/artemis/PrivacyStatementResourceIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/PrivacyStatementResourceIntegrationTest.java @@ -17,9 +17,10 @@ import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.google.gson.JsonObject; + import de.tum.in.www1.artemis.domain.PrivacyStatement; import de.tum.in.www1.artemis.domain.enumeration.Language; -import net.minidev.json.JSONObject; class PrivacyStatementResourceIntegrationTest extends AbstractSpringIntegrationIndependentTest { @@ -220,10 +221,10 @@ void testUpdatePrivacyStatement_writesFile_ReturnsUpdatedFileContent() throws Ex @Test @WithMockUser(username = TEST_PREFIX + "admin", roles = "ADMIN") void testUpdatePrivacyStatement_unsupportedLanguageBadRequest() throws Exception { - JSONObject body = new JSONObject(); - body.put("text", "test"); - body.put("language", "FRENCH"); - request.put("/api/admin/privacy-statement", body, HttpStatus.BAD_REQUEST); + JsonObject body = new JsonObject(); + body.addProperty("text", "test"); + body.addProperty("language", "FRENCH"); + request.put("/api/admin/privacy-statement", body.toString(), HttpStatus.BAD_REQUEST); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/connectors/Lti13ServiceTest.java b/src/test/java/de/tum/in/www1/artemis/connectors/Lti13ServiceTest.java index 4a678b367840..12a9b12f98ff 100644 --- a/src/test/java/de/tum/in/www1/artemis/connectors/Lti13ServiceTest.java +++ b/src/test/java/de/tum/in/www1/artemis/connectors/Lti13ServiceTest.java @@ -25,9 +25,8 @@ import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; -import com.nimbusds.jose.shaded.json.JSONObject; -import com.nimbusds.jose.shaded.json.parser.JSONParser; -import com.nimbusds.jose.shaded.json.parser.ParseException; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.lti.LtiResourceLaunch; @@ -118,8 +117,8 @@ void performLaunch_exerciseFound() { when(oidcIdToken.getClaim("sub")).thenReturn("1"); when(oidcIdToken.getClaim("iss")).thenReturn("http://otherDomain.com"); when(oidcIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID)).thenReturn("1"); - JSONObject jsonObject = new JSONObject(); - jsonObject.put("id", "resourceLinkUrl"); + JsonObject jsonObject = new JsonObject(); + jsonObject.addProperty("id", "resourceLinkUrl"); when(oidcIdToken.getClaim(Claims.RESOURCE_LINK)).thenReturn(jsonObject); when(oidcIdToken.getClaim(Claims.TARGET_LINK_URI)).thenReturn("https://some-artemis-domain.org/courses/" + courseId + "/exercises/" + exerciseId); @@ -420,7 +419,7 @@ void onNewResultTokenFails() { } @Test - void onNewResult() throws ParseException { + void onNewResult() { Result result = new Result(); double scoreGiven = 60D; result.setScore(scoreGiven); @@ -464,16 +463,15 @@ void onNewResult() throws ParseException { assertThat(authHeaders).as("Score publish request must contain an Authorization header").isNotNull(); assertThat(authHeaders).as("Score publish request must contain the corresponding Authorization Bearer token").contains("Bearer " + accessToken); - JSONParser jsonParser = new JSONParser(JSONParser.MODE_JSON_SIMPLE); - JSONObject body = (JSONObject) jsonParser.parse(httpEntity.getBody()); - assertThat(body.get("userId")).as("Invalid parameter in score publish request: userId").isEqualTo(launch.getSub()); - assertThat(body.get("timestamp")).as("Parameter missing in score publish request: timestamp").isNotNull(); - assertThat(body.get("activityProgress")).as("Parameter missing in score publish request: activityProgress").isNotNull(); - assertThat(body.get("gradingProgress")).as("Parameter missing in score publish request: gradingProgress").isNotNull(); + JsonObject body = JsonParser.parseString(httpEntity.getBody()).getAsJsonObject(); + assertThat(body.get("userId").getAsString()).as("Invalid parameter in score publish request: userId").isEqualTo(launch.getSub()); + assertThat(body.get("timestamp").getAsString()).as("Parameter missing in score publish request: timestamp").isNotNull(); + assertThat(body.get("activityProgress").getAsString()).as("Parameter missing in score publish request: activityProgress").isNotNull(); + assertThat(body.get("gradingProgress").getAsString()).as("Parameter missing in score publish request: gradingProgress").isNotNull(); - assertThat(body.get("comment")).as("Invalid parameter in score publish request: comment").isEqualTo("Good job. Not so good"); - assertThat(body.get("scoreGiven")).as("Invalid parameter in score publish request: scoreGiven").isEqualTo(scoreGiven); - assertThat(body.get("scoreMaximum")).as("Invalid parameter in score publish request: scoreMaximum").isEqualTo(100d); + assertThat(body.get("comment").getAsString()).as("Invalid parameter in score publish request: comment").isEqualTo("Good job. Not so good"); + assertThat(body.get("scoreGiven").getAsDouble()).as("Invalid parameter in score publish request: scoreGiven").isEqualTo(scoreGiven); + assertThat(body.get("scoreMaximum").getAsDouble()).as("Invalid parameter in score publish request: scoreMaximum").isEqualTo(100d); assertThat(launch.getScoreLineItemUrl() + "/scores").as("Score publish request was sent to a wrong URI").isEqualTo(urlCapture.getValue()); } diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java index a6e42f002c5e..259a4be57106 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java @@ -17,8 +17,6 @@ import javax.validation.constraints.NotNull; import org.eclipse.jgit.lib.ObjectId; -import org.json.simple.JSONArray; -import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -35,6 +33,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.google.gson.JsonParser; import de.tum.in.www1.artemis.AbstractSpringIntegrationBambooBitbucketJiraTest; import de.tum.in.www1.artemis.course.CourseUtilService; @@ -648,12 +647,12 @@ private static Stream shouldSavebuildLogsOnStudentParticipationArgume @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void shouldSetSubmissionDateForBuildCorrectlyIfOnlyOnePushIsReceived() throws Exception { testService.setUp_shouldSetSubmissionDateForBuildCorrectlyIfOnlyOnePushIsReceived(TEST_PREFIX); - var pushJSON = (JSONObject) new JSONParser().parse(BITBUCKET_PUSH_EVENT_REQUEST); - var changes = (JSONArray) pushJSON.get("changes"); - var firstChange = (JSONObject) changes.get(0); - var firstCommitHash = (String) firstChange.get("fromHash"); - var secondCommitHash = (String) firstChange.get("toHash"); - var secondCommitDate = ZonedDateTime.parse(pushJSON.get("date").toString(), DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")); + var pushJSON = JsonParser.parseString(BITBUCKET_PUSH_EVENT_REQUEST).getAsJsonObject(); + var changes = pushJSON.get("changes").getAsJsonArray(); + var firstChange = changes.get(0).getAsJsonObject(); + var firstCommitHash = firstChange.get("fromHash").getAsString(); + var secondCommitHash = firstChange.get("toHash").getAsString(); + var secondCommitDate = ZonedDateTime.parse(pushJSON.get("date").getAsString(), DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")); var firstCommitDate = secondCommitDate.minusSeconds(30); doReturn(defaultBranch).when(versionControlService).getOrRetrieveBranchOfExercise(testService.programmingExercise); diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java index 2b4ad74291a5..a192ad30428c 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java @@ -11,8 +11,6 @@ import java.util.Map; import java.util.stream.Stream; -import org.json.simple.JSONObject; -import org.json.simple.parser.JSONParser; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.google.gson.JsonParser; import de.tum.in.www1.artemis.AbstractSpringIntegrationJenkinsGitlabTest; import de.tum.in.www1.artemis.domain.*; @@ -287,9 +286,9 @@ void shouldReturnBadRequestWhenPlanKeyDoesntExist(ProgrammingLanguage programmin void shouldSetSubmissionDateForBuildCorrectlyIfOnlyOnePushIsReceived() throws Exception { testService.setUp_shouldSetSubmissionDateForBuildCorrectlyIfOnlyOnePushIsReceived(TEST_PREFIX); String userLogin = TEST_PREFIX + "student1"; - var pushJSON = (JSONObject) new JSONParser().parse(GITLAB_PUSH_EVENT_REQUEST); - var firstCommitHash = (String) pushJSON.get("before"); - var secondCommitHash = (String) pushJSON.get("after"); + var pushJSON = new JsonParser().parse(GITLAB_PUSH_EVENT_REQUEST).getAsJsonObject(); + var firstCommitHash = pushJSON.get("before").getAsString(); + var secondCommitHash = pushJSON.get("after").getAsString(); var firstCommitDate = ZonedDateTime.now().minusSeconds(60); var secondCommitDate = ZonedDateTime.now().minusSeconds(30); diff --git a/src/test/java/de/tum/in/www1/artemis/security/Lti13LaunchFilterTest.java b/src/test/java/de/tum/in/www1/artemis/security/Lti13LaunchFilterTest.java index 51fd2c6e30ca..7dd90e9538d3 100644 --- a/src/test/java/de/tum/in/www1/artemis/security/Lti13LaunchFilterTest.java +++ b/src/test/java/de/tum/in/www1/artemis/security/Lti13LaunchFilterTest.java @@ -28,10 +28,11 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import com.google.gson.JsonObject; + import de.tum.in.www1.artemis.config.lti.CustomLti13Configurer; import de.tum.in.www1.artemis.security.lti.Lti13LaunchFilter; import de.tum.in.www1.artemis.service.connectors.lti.Lti13Service; -import net.minidev.json.JSONObject; import uk.ac.ox.ctl.lti13.lti.Claims; import uk.ac.ox.ctl.lti13.security.oauth2.client.lti.authentication.OidcAuthenticationToken; import uk.ac.ox.ctl.lti13.security.oauth2.client.lti.web.OAuth2LoginAuthenticationFilter; @@ -108,8 +109,8 @@ private void initValidIdToken() { idTokenClaims.put("iss", "https://some.lms.org"); idTokenClaims.put("sub", "23423435"); idTokenClaims.put(Claims.LTI_DEPLOYMENT_ID, "some-deployment-id"); - JSONObject resourceLinkClaim = new JSONObject(); - resourceLinkClaim.put("id", "some-resource-id"); + JsonObject resourceLinkClaim = new JsonObject(); + resourceLinkClaim.addProperty("id", "some-resource-id"); idTokenClaims.put(Claims.RESOURCE_LINK, resourceLinkClaim); idTokenClaims.put(Claims.TARGET_LINK_URI, targetLinkUri); } @@ -129,11 +130,11 @@ void authenticatedLogin() throws Exception { verify(httpResponse).setCharacterEncoding("UTF-8"); verify(lti13Service).performLaunch(any(), any()); - ArgumentCaptor argument = ArgumentCaptor.forClass(JSONObject.class); + ArgumentCaptor argument = ArgumentCaptor.forClass(JsonObject.class); verify(responseWriter).print(argument.capture()); - JSONObject responseJsonBody = argument.getValue(); + JsonObject responseJsonBody = argument.getValue(); verify(lti13Service).buildLtiResponse(any(), any()); - assertThat(((String) responseJsonBody.get("targetLinkUri"))).as("Response body contains the expected targetLinkUri").contains(this.targetLinkUri); + assertThat((responseJsonBody.get("targetLinkUri").getAsString())).as("Response body contains the expected targetLinkUri").contains(this.targetLinkUri); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseFeedbackCreationServiceTest.java b/src/test/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseFeedbackCreationServiceTest.java index 460fd2662a6f..593e8b3f6806 100644 --- a/src/test/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseFeedbackCreationServiceTest.java +++ b/src/test/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseFeedbackCreationServiceTest.java @@ -4,11 +4,13 @@ import java.util.List; -import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.test.context.support.WithMockUser; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + import de.tum.in.www1.artemis.AbstractSpringIntegrationIndependentTest; import de.tum.in.www1.artemis.config.Constants; import de.tum.in.www1.artemis.domain.Feedback; @@ -224,8 +226,8 @@ void testRemoveCIDirectoriesFromPath() { .createFeedbackFromStaticCodeAnalysisReports(resultNotification1.getBuild().jobs().get(0).staticCodeAnalysisReports()); for (var feedback : staticCodeAnalysisFeedback1) { - JSONObject issueJSON = new JSONObject(feedback.getDetailText()); - assertThat(pathWithoutWorkingDir).isEqualTo(issueJSON.get("filePath")); + JsonObject issueJSON = JsonParser.parseString(feedback.getDetailText()).getAsJsonObject(); + assertThat(pathWithoutWorkingDir).isEqualTo(issueJSON.get("filePath").getAsString()); } // 2. Test that null or empty paths default to FeedbackRepository.DEFAULT_FILEPATH @@ -250,8 +252,8 @@ void testRemoveCIDirectoriesFromPath() { .createFeedbackFromStaticCodeAnalysisReports(resultNotification2.getBuild().jobs().get(0).staticCodeAnalysisReports()); for (var feedback : staticCodeAnalysisFeedback2) { - JSONObject issueJSON = new JSONObject(feedback.getDetailText()); - assertThat(issueJSON.get("filePath")).isEqualTo("notAvailable"); + JsonObject issueJSON = JsonParser.parseString(feedback.getDetailText()).getAsJsonObject(); + assertThat(issueJSON.get("filePath").getAsString()).isEqualTo("notAvailable"); } } } From fae24d41182d67c8fdb4b06fa313061f4737f753 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Fri, 27 Oct 2023 21:05:18 +0200 Subject: [PATCH 2/9] unify ObjectMapper --- .../GitlabServiceTest.java | 15 +++++++------- ...dResultBitbucketBambooIntegrationTest.java | 20 +++++++++---------- ...issionAndResultIntegrationTestService.java | 7 ++++--- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/GitlabServiceTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/GitlabServiceTest.java index 2a8fd2b7f8ce..9e3bc0fe5356 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/GitlabServiceTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/GitlabServiceTest.java @@ -9,8 +9,6 @@ import java.net.URL; import org.gitlab4j.api.GitLabApiException; -import org.json.simple.parser.JSONParser; -import org.json.simple.parser.ParseException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -22,6 +20,7 @@ import org.springframework.security.test.context.support.WithMockUser; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import de.tum.in.www1.artemis.AbstractSpringIntegrationJenkinsGitlabTest; import de.tum.in.www1.artemis.domain.Commit; @@ -118,9 +117,9 @@ void testGetOrRetrieveDefaultBranch() throws GitLabApiException { } @Test - void testGetLastCommitDetails() throws ParseException { + void testGetLastCommitDetails() throws JsonProcessingException { String latestCommitHash = "11028e4104243d8cbae9175f2bc938cb8c2d7924"; - Object body = new JSONParser().parse(GITLAB_PUSH_EVENT_REQUEST); + Object body = new ObjectMapper().readValue(GITLAB_PUSH_EVENT_REQUEST, Object.class); Commit commit = versionControlService.getLastCommitDetails(body); assertThat(commit.getCommitHash()).isEqualTo(latestCommitHash); assertThat(commit.getBranch()).isNotNull(); @@ -130,9 +129,9 @@ void testGetLastCommitDetails() throws ParseException { } @Test - void testGetLastCommitDetailsWrongCommitOrder() throws ParseException { + void testGetLastCommitDetailsWrongCommitOrder() throws JsonProcessingException { String latestCommitHash = "11028e4104243d8cbae9175f2bc938cb8c2d7924"; - Object body = new JSONParser().parse(GITLAB_PUSH_EVENT_REQUEST_WRONG_COMMIT_ORDER); + Object body = new ObjectMapper().readValue(GITLAB_PUSH_EVENT_REQUEST_WRONG_COMMIT_ORDER, Object.class); Commit commit = versionControlService.getLastCommitDetails(body); assertThat(commit.getCommitHash()).isEqualTo(latestCommitHash); assertThat(commit.getBranch()).isNotNull(); @@ -142,9 +141,9 @@ void testGetLastCommitDetailsWrongCommitOrder() throws ParseException { } @Test - void testGetLastCommitDetailsWithoutCommits() throws ParseException { + void testGetLastCommitDetailsWithoutCommits() throws JsonProcessingException { String latestCommitHash = "11028e4104243d8cbae9175f2bc938cb8c2d7924"; - Object body = new JSONParser().parse(GITLAB_PUSH_EVENT_REQUEST_WITHOUT_COMMIT); + Object body = new ObjectMapper().readValue(GITLAB_PUSH_EVENT_REQUEST_WITHOUT_COMMIT, Object.class); Commit commit = versionControlService.getLastCommitDetails(body); assertThat(commit.getCommitHash()).isEqualTo(latestCommitHash); assertThat(commit.getBranch()).isNull(); diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java index 259a4be57106..837092124525 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultBitbucketBambooIntegrationTest.java @@ -17,7 +17,6 @@ import javax.validation.constraints.NotNull; import org.eclipse.jgit.lib.ObjectId; -import org.json.simple.parser.JSONParser; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -31,6 +30,7 @@ import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.google.gson.JsonParser; @@ -174,8 +174,7 @@ void tearDown() { @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void shouldNotCreateSubmissionOnNotifyPushForInvalidParticipationId() throws Exception { long fakeParticipationId = 9999L; - JSONParser jsonParser = new JSONParser(); - Object obj = jsonParser.parse(BITBUCKET_PUSH_EVENT_REQUEST); + Object obj = new ObjectMapper().readValue(BITBUCKET_PUSH_EVENT_REQUEST, Object.class); // Api should return not found. request.postWithoutLocation("/api/public/programming-submissions/" + fakeParticipationId, obj, HttpStatus.NOT_FOUND, new HttpHeaders()); // No submission should be created for the fake participation. @@ -1050,8 +1049,8 @@ private ProgrammingSubmission postSubmission(Long participationId, HttpStatus ex */ @SuppressWarnings("unchecked") private void postTestRepositorySubmission() throws Exception { - JSONParser jsonParser = new JSONParser(); - Object obj = jsonParser.parse(BITBUCKET_PUSH_EVENT_REQUEST); + ObjectMapper mapper = new ObjectMapper(); + Object obj = mapper.readValue(BITBUCKET_PUSH_EVENT_REQUEST, Object.class); Map requestBodyMap = (Map) obj; List> changes = (List>) requestBodyMap.get("changes"); @@ -1065,8 +1064,8 @@ private void postTestRepositorySubmission() throws Exception { * Simulate a commit to the test repository, this executes a http request from the VCS to Artemis. */ private void postTestRepositorySubmissionWithoutCommit(HttpStatus status) throws Exception { - JSONParser jsonParser = new JSONParser(); - Object obj = jsonParser.parse(BITBUCKET_PUSH_EVENT_REQUEST_WITHOUT_COMMIT); + ObjectMapper mapper = new ObjectMapper(); + Object obj = mapper.readValue(BITBUCKET_PUSH_EVENT_REQUEST_WITHOUT_COMMIT, Object.class); request.postWithoutLocation("/api/public/programming-exercises/test-cases-changed/" + exerciseId, obj, status, new HttpHeaders()); } @@ -1128,14 +1127,13 @@ private void postResult(BambooBuildResultNotificationDTO buildResultNotification request.postWithoutLocation("/api/public/programming-exercises/new-result", alteredObj, expectedStatus, httpHeaders); } - private BambooBuildResultNotificationDTO createBambooBuildResultNotificationDTO(String buildPlanKey) throws Exception { - JSONParser jsonParser = new JSONParser(); + private BambooBuildResultNotificationDTO createBambooBuildResultNotificationDTO(String buildPlanKey) throws JsonProcessingException { + ObjectMapper mapper = new ObjectMapper(); // replace plan.key in BAMBOO_BUILD_RESULT_REQUEST with buildPlanKey as well as the var buildResult = BAMBOO_BUILD_RESULT_REQUEST.replace("TEST201904BPROGRAMMINGEXERCISE6-STUDENT1", buildPlanKey).replace("2019-07-27T17:07:46.642Z[Zulu]", ZonedDateTime.now().toString()); - Object obj = jsonParser.parse(buildResult); + Object obj = mapper.readValue(buildResult, Object.class); - ObjectMapper mapper = new ObjectMapper(); mapper.registerModule(new JavaTimeModule()); return mapper.convertValue(obj, BambooBuildResultNotificationDTO.class); } diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultIntegrationTestService.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultIntegrationTestService.java index 0b9924617553..cb6f3a6807be 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultIntegrationTestService.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingSubmissionAndResultIntegrationTestService.java @@ -7,12 +7,13 @@ import java.time.temporal.ChronoUnit; import java.util.List; -import org.json.simple.parser.JSONParser; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; +import com.fasterxml.jackson.databind.ObjectMapper; + import de.tum.in.www1.artemis.domain.Course; import de.tum.in.www1.artemis.domain.ProgrammingExercise; import de.tum.in.www1.artemis.domain.ProgrammingSubmission; @@ -95,8 +96,8 @@ public void shouldSetSubmissionDateForBuildCorrectlyIfOnlyOnePushIsReceived(Stri * @return The submission that was created */ public ProgrammingSubmission postSubmission(Long participationId, HttpStatus expectedStatus, String jsonRequest) throws Exception { - JSONParser jsonParser = new JSONParser(); - Object obj = jsonParser.parse(jsonRequest); + ObjectMapper mapper = new ObjectMapper(); + Object obj = mapper.readValue(jsonRequest, Object.class); // Api should return ok. request.postWithoutLocation("/api/public/programming-submissions/" + participationId, obj, expectedStatus, new HttpHeaders()); From e45b539ff75c09e2eeec47c2f659d7e4e3324fdb Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Fri, 27 Oct 2023 22:19:58 +0200 Subject: [PATCH 3/9] unify remaining ObjectMapper --- src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java | 4 ++++ .../ProgrammingExerciseBitbucketBambooIntegrationTest.java | 5 +++-- .../ProgrammingExerciseGitlabJenkinsIntegrationTest.java | 5 +++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java index 8b821ba54c66..2b1446c9c0e1 100644 --- a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java +++ b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java @@ -115,8 +115,12 @@ void testJSONImplementations() { ArchRule jsonArray = noClasses().should() .dependOnClassesThat(have(simpleName("JsonArray")).or(have(simpleName("JSONArray"))).and(not(resideInAPackage("com.google.gson")))); + ArchRule jsonParser = noClasses().should() + .dependOnClassesThat(have(simpleName("JsonParser")).or(have(simpleName("JSONParser"))).and(not(resideInAPackage("com.google.gson")))); + jsonObject.check(allClasses); jsonArray.check(allClasses); + jsonParser.check(allClasses); } // Custom Predicates for JavaAnnotations since ArchUnit only defines them for classes diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java index 6fb4f6d83217..c8f955d45bef 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java @@ -13,7 +13,6 @@ import java.util.Arrays; import java.util.stream.Stream; -import org.json.simple.parser.JSONParser; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -24,6 +23,8 @@ import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.fasterxml.jackson.databind.ObjectMapper; + import de.tum.in.www1.artemis.AbstractSpringIntegrationBambooBitbucketJiraTest; import de.tum.in.www1.artemis.domain.enumeration.ExerciseMode; import de.tum.in.www1.artemis.domain.enumeration.ProgrammingLanguage; @@ -218,7 +219,7 @@ void resumeProgrammingExercise_doesNotExist(ExerciseMode exerciseMode) throws Ex void resumeProgrammingExerciseByPushingIntoRepo_correctInitializationState(ExerciseMode exerciseMode) throws Exception { final String requestAsArtemisUser = BITBUCKET_PUSH_EVENT_REQUEST.replace("\"name\": \"admin\"", "\"name\": \"Artemis\"").replace("\"displayName\": \"Admin\"", "\"displayName\": \"Artemis\""); - Object body = new JSONParser().parse(requestAsArtemisUser); + Object body = new ObjectMapper().readValue(requestAsArtemisUser, Object.class); programmingExerciseTestService.resumeProgrammingExerciseByPushingIntoRepo_correctInitializationState(exerciseMode, body); } diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java index 473f79767efd..85e61e914823 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java @@ -13,7 +13,6 @@ import java.util.Arrays; import java.util.stream.Stream; -import org.json.simple.parser.JSONParser; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -24,6 +23,8 @@ import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.fasterxml.jackson.databind.ObjectMapper; + import de.tum.in.www1.artemis.AbstractSpringIntegrationJenkinsGitlabTest; import de.tum.in.www1.artemis.domain.enumeration.ExerciseMode; import de.tum.in.www1.artemis.domain.enumeration.ProgrammingLanguage; @@ -278,7 +279,7 @@ void resumeProgrammingExercise_doesNotExist(ExerciseMode exerciseMode) throws Ex @EnumSource(ExerciseMode.class) @WithMockUser(username = TEST_PREFIX + studentLogin, roles = "USER") void resumeProgrammingExerciseByPushingIntoRepo_correctInitializationState(ExerciseMode exerciseMode) throws Exception { - Object body = new JSONParser().parse(GITLAB_PUSH_EVENT_REQUEST); + Object body = new ObjectMapper().readValue(GITLAB_PUSH_EVENT_REQUEST, Object.class); programmingExerciseTestService.resumeProgrammingExerciseByPushingIntoRepo_correctInitializationState(exerciseMode, body); } From dd619832c0c1da719a8d90b1f0651eaacc20d31f Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 10:45:50 +0200 Subject: [PATCH 4/9] add Lucas feedback --- .../java/de/tum/in/www1/artemis/ArchitectureTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java index 2b1446c9c0e1..0b37e4772ef1 100644 --- a/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java +++ b/src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java @@ -109,14 +109,11 @@ void testLogging() { @Test void testJSONImplementations() { - ArchRule jsonObject = noClasses().should() - .dependOnClassesThat(have(simpleName("JsonObject")).or(have(simpleName("JSONObject"))).and(not(resideInAPackage("com.google.gson")))); + ArchRule jsonObject = noClasses().should().dependOnClassesThat(have(simpleName("JsonObject").or(simpleName("JSONObject"))).and(not(resideInAPackage("com.google.gson")))); - ArchRule jsonArray = noClasses().should() - .dependOnClassesThat(have(simpleName("JsonArray")).or(have(simpleName("JSONArray"))).and(not(resideInAPackage("com.google.gson")))); + ArchRule jsonArray = noClasses().should().dependOnClassesThat(have(simpleName("JsonArray").or(simpleName("JSONArray"))).and(not(resideInAPackage("com.google.gson")))); - ArchRule jsonParser = noClasses().should() - .dependOnClassesThat(have(simpleName("JsonParser")).or(have(simpleName("JSONParser"))).and(not(resideInAPackage("com.google.gson")))); + ArchRule jsonParser = noClasses().should().dependOnClassesThat(have(simpleName("JsonParser").or(simpleName("JSONParser"))).and(not(resideInAPackage("com.google.gson")))); jsonObject.check(allClasses); jsonArray.check(allClasses); From 709353836ea05196457c3caf4cc637279d2fe4c5 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 15:30:36 +0200 Subject: [PATCH 5/9] fix for ClassCastException --- .../de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java | 6 ++---- .../tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java index d6999f460047..5a84cc43c798 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java @@ -4,9 +4,7 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; -import com.google.gson.JsonArray; -import com.google.gson.JsonObject; -import com.google.gson.JsonPrimitive; +import com.google.gson.*; /** * A wrapper class for an LTI 1.3 Assignment and Grading Services Claim. We support the Score Publishing Service in order to transmit scores. @@ -24,7 +22,7 @@ public class Lti13AgsClaim { * @return an Ags-Claim if one was present in idToken. */ public static Optional from(OidcIdToken idToken) { - JsonObject agsClaimJson = idToken.getClaim(Claims.AGS_CLAIM); + JsonObject agsClaimJson = JsonParser.parseString(idToken.getClaim(Claims.AGS_CLAIM).toString()).getAsJsonObject(); if (agsClaimJson == null) { return Optional.empty(); } diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java index faeee4175d0e..d8eb66845b49 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java @@ -4,6 +4,7 @@ import org.springframework.util.Assert; import com.google.gson.JsonObject; +import com.google.gson.JsonParser; public class Lti13LaunchRequest { @@ -24,7 +25,7 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) { this.sub = ltiIdToken.getClaim("sub"); this.deploymentId = ltiIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID); - JsonObject resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK); + JsonObject resourceLinkClaim = JsonParser.parseString(ltiIdToken.getClaim(Claims.RESOURCE_LINK).toString()).getAsJsonObject(); this.resourceLinkId = resourceLinkClaim != null ? resourceLinkClaim.get("id").getAsString() : null; this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI); From 6a3d06fba045213e13df40c0b10662f84707b9fe Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 16:10:49 +0200 Subject: [PATCH 6/9] add note for avoiding arbitrary libraries for json parsing to dev guideline --- docs/dev/guidelines/server.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/dev/guidelines/server.rst b/docs/dev/guidelines/server.rst index 5bde2091026d..7ac33bd1d919 100644 --- a/docs/dev/guidelines/server.rst +++ b/docs/dev/guidelines/server.rst @@ -648,3 +648,10 @@ Now, instead of mocking the whole Service, we can just mock the static method, l You should notice here that we can avoid the use of a Bean and also test deeper. Instead of mocking the uppermost method we only throw the exception at the place where it could actually happen. Very important to mention is that you need to close the mock at the end of the test again. For a real example where a SpyBean was replaced with a static mock look at the SubmissionExportIntegrationTest.java in `here `__. + +27. Avoid Arbitrary JSON Parsing Libraries +========================================== + +* Utilize well-established JSON parsing libraries such as Gson or Jackson. +* Whenever possible, use ObjectMapper for JSON parsing. If ObjectMapper is not suitable for the specific task, use to Gson's JsonParser. +* By adhering to these points, we ensure to maintain a consistent and reliable approach to JSON parsing. This helps reduce potential maintenance challenges and compatibility issues. From 1e22456e87cdaf8e8981f94086662ece192f29f8 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 21:47:15 +0200 Subject: [PATCH 7/9] fix server tests --- .../java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java | 3 ++- .../de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java index 5a84cc43c798..4bdcf0463176 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java @@ -22,7 +22,8 @@ public class Lti13AgsClaim { * @return an Ags-Claim if one was present in idToken. */ public static Optional from(OidcIdToken idToken) { - JsonObject agsClaimJson = JsonParser.parseString(idToken.getClaim(Claims.AGS_CLAIM).toString()).getAsJsonObject(); + JsonObject agsClaimJson = idToken.getClaimAsString(Claims.AGS_CLAIM) != null ? JsonParser.parseString(idToken.getClaimAsString(Claims.AGS_CLAIM)).getAsJsonObject() : null; + if (agsClaimJson == null) { return Optional.empty(); } diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java index d8eb66845b49..ab4ec0ac8b38 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java @@ -25,7 +25,9 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) { this.sub = ltiIdToken.getClaim("sub"); this.deploymentId = ltiIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID); - JsonObject resourceLinkClaim = JsonParser.parseString(ltiIdToken.getClaim(Claims.RESOURCE_LINK).toString()).getAsJsonObject(); + JsonObject resourceLinkClaim = ltiIdToken.getClaimAsString(Claims.RESOURCE_LINK) != null + ? JsonParser.parseString(ltiIdToken.getClaimAsString(Claims.RESOURCE_LINK)).getAsJsonObject() + : null; this.resourceLinkId = resourceLinkClaim != null ? resourceLinkClaim.get("id").getAsString() : null; this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI); From c5aeab32d3d8f5a43937b775083e0704955bc8d1 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 22:12:19 +0200 Subject: [PATCH 8/9] fix remaining server test --- .../java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java | 2 +- .../de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java index 4bdcf0463176..b9c809d51153 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java @@ -22,7 +22,7 @@ public class Lti13AgsClaim { * @return an Ags-Claim if one was present in idToken. */ public static Optional from(OidcIdToken idToken) { - JsonObject agsClaimJson = idToken.getClaimAsString(Claims.AGS_CLAIM) != null ? JsonParser.parseString(idToken.getClaimAsString(Claims.AGS_CLAIM)).getAsJsonObject() : null; + JsonObject agsClaimJson = idToken.getClaim(Claims.AGS_CLAIM) != null ? JsonParser.parseString(idToken.getClaim(Claims.AGS_CLAIM).toString()).getAsJsonObject() : null; if (agsClaimJson == null) { return Optional.empty(); diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java index ab4ec0ac8b38..1f9b11e85ef1 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java @@ -25,8 +25,8 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) { this.sub = ltiIdToken.getClaim("sub"); this.deploymentId = ltiIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID); - JsonObject resourceLinkClaim = ltiIdToken.getClaimAsString(Claims.RESOURCE_LINK) != null - ? JsonParser.parseString(ltiIdToken.getClaimAsString(Claims.RESOURCE_LINK)).getAsJsonObject() + JsonObject resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK) != null + ? JsonParser.parseString(ltiIdToken.getClaim(Claims.RESOURCE_LINK).toString()).getAsJsonObject() : null; this.resourceLinkId = resourceLinkClaim != null ? resourceLinkClaim.get("id").getAsString() : null; this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI); From 0822e4ae70cfc3ba189af6ed8947f94781c9b9a4 Mon Sep 17 00:00:00 2001 From: Basak Akan Date: Sat, 28 Oct 2023 23:34:48 +0200 Subject: [PATCH 9/9] add feedback suggestions --- .../in/www1/artemis/domain/lti/Lti13AgsClaim.java | 6 +++--- .../www1/artemis/domain/lti/Lti13LaunchRequest.java | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java index b9c809d51153..8217bc519332 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13AgsClaim.java @@ -22,12 +22,12 @@ public class Lti13AgsClaim { * @return an Ags-Claim if one was present in idToken. */ public static Optional from(OidcIdToken idToken) { - JsonObject agsClaimJson = idToken.getClaim(Claims.AGS_CLAIM) != null ? JsonParser.parseString(idToken.getClaim(Claims.AGS_CLAIM).toString()).getAsJsonObject() : null; - - if (agsClaimJson == null) { + if (idToken.getClaim(Claims.AGS_CLAIM) == null) { return Optional.empty(); } + JsonObject agsClaimJson = JsonParser.parseString(idToken.getClaim(Claims.AGS_CLAIM).toString()).getAsJsonObject(); + Lti13AgsClaim agsClaim = new Lti13AgsClaim(); JsonArray scopes = agsClaimJson.get("scope").getAsJsonArray(); diff --git a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java index 1f9b11e85ef1..738804afe4bc 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/lti/Lti13LaunchRequest.java @@ -3,7 +3,6 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.util.Assert; -import com.google.gson.JsonObject; import com.google.gson.JsonParser; public class Lti13LaunchRequest { @@ -25,10 +24,13 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) { this.sub = ltiIdToken.getClaim("sub"); this.deploymentId = ltiIdToken.getClaim(Claims.LTI_DEPLOYMENT_ID); - JsonObject resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK) != null - ? JsonParser.parseString(ltiIdToken.getClaim(Claims.RESOURCE_LINK).toString()).getAsJsonObject() - : null; - this.resourceLinkId = resourceLinkClaim != null ? resourceLinkClaim.get("id").getAsString() : null; + var resourceLinkClaim = ltiIdToken.getClaim(Claims.RESOURCE_LINK); + if (resourceLinkClaim != null) { + this.resourceLinkId = JsonParser.parseString(resourceLinkClaim.toString()).getAsJsonObject().get("id").getAsString(); + } + else { + this.resourceLinkId = null; + } this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI); this.agsClaim = Lti13AgsClaim.from(ltiIdToken).orElse(null);