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: Unify json usage #7457

Merged
merged 13 commits into from
Oct 29, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -23,23 +24,23 @@ public class Lti13AgsClaim {
* @return an Ags-Claim if one was present in idToken.
*/
public static Optional<Lti13AgsClaim> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@
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;
import de.tum.in.www1.artemis.repository.*;
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")
Expand Down Expand Up @@ -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();
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,19 @@ void testLogging() {
GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS.check(classes);
}

@Test
void testJSONImplementations() {
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(simpleName("JSONArray"))).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);
jsonParser.check(allClasses);
}

// Custom Predicates for JavaAnnotations since ArchUnit only defines them for classes

private DescribedPredicate<? super JavaAnnotation<?>> simpleNameAnnotation(String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -420,7 +419,7 @@ void onNewResultTokenFails() {
}

@Test
void onNewResult() throws ParseException {
void onNewResult() {
Result result = new Result();
double scoreGiven = 60D;
result.setScore(scoreGiven);
Expand Down Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Loading
Loading