Development: Use object mapper and streams to improve and simplify the code

[krusche]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• This is a small issue that I tested locally and was confirmed by another developer on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
• I strictly followed the server coding and design guidelines.
• I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
• I documented the Java code using JavaDoc style.

Motivation and Context

We aim to replace the Google gson library with the Jackson ObjectMapper for parsing JSON and XML. This change will help us maintain consistency and simplify the server code. Additionally, we are gradually transitioning from using classes to records for our DTOs.

Description

This PR reduces the reliance on gson and replaces it with the ObjectMapper. We focus on the structure oracle generator classes used for generating structural tests in programming exercises. Additionally we replace classes related to Legal Documents, which were used as DTOs, with records.

Steps for Testing

Prerequisites:

• 1 Admin
• 1 Student
• 1 Programming Exercise with structural tests

Best tested locally, or on a test server where you are allowed to change the Imprint / Privacy Statement as admin

1. Log in to Artemis as admin
2. Navigate to Server Administration
3. Under Imprint set a new text and save, same for Privacy Statement
4. Click on Imprint / Privacy in the footer to see if the new text appears
5. Create a programming exercise with structural tests (there should be some per default)
6. Check that they pass for solution and fail for template
7. Participate in the programing exercise as a student
8. See that the tests are passing as expected

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked

Review Progress

Performance Review

• I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance
• I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Test Coverage

Client

Class/File	Line Coverage	Confirmation (assert/expect)
imprint.component.ts	88.23%	✅
privacy.component.ts	95%	✅
legal-document.model.ts	100%	✅

Server

Class/File	Line Coverage	Confirmation (assert/expect)
LegalDocumentService.java	100%	✅
DataExportExerciseCreationService.java	93%	✅
JavaClassDiff.java	87%	✅
JavaClassDiffSerializer.java	81%	✅
OracleGenerator.java	83%	✅
SerializerUtil.java	69%	✅
AdminImprintResource.java	100%	✅
AdminPrivacyStatementResource.java	100%	✅
ImprintDTO.java	100%	✅
PrivacyStatementDTO.java	100%	✅
PublicImprintResource.java	100%	✅
PublicPrivacyStatementResource.java	100%	✅

[github-actions]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[coderabbitai]

Walkthrough

The recent updates focus on transitioning from direct entity manipulation to utilizing Data Transfer Objects (DTOs) for LegalDocument, PrivacyStatement, and `Imprint. This shift aims to enhance data encapsulation and separation between internal models and external representations. Additionally, adjustments were made to Java class comparison logic, serialization methods, and test cases to align with these new DTOs. Front-end components and REST resources were also modified to accommodate these changes.

Changes

File Path	Change Summary
src/main/java/de/tum/in/www1/artemis/service/LegalDocumentService.java	Replaced entity references with corresponding DTOs in methods and imports.
src/main/java/de/tum/in/www1/artemis/service/export/DataExportExerciseCreationService.java	Updated a comment to reflect method name change for clarity.
src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/JavaClassDiff.java	Improved list comparison logic by using HashSet.
src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/JavaClassDiffSerializer.java	Switched to Jackson for JSON serialization, adjusting method return types.
src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/OracleGenerator.java	Introduced Optional return type, added new methods, and updated JSON handling to Jackson.
src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/SerializerUtil.java	Replaced Gson with Jackson for JSON serialization methods.
src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminImprintResource.java	Updated method signatures to use ImprintDTO instead of Imprint.
src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminPrivacyStatementResource.java	Adjusted methods to return ResponseEntity<PrivacyStatementDTO> and updated imports.
src/main/java/de/tum/in/www1/artemis/web/rest/dto/ImprintDTO.java	Introduced ImprintDTO class implementing LegalDocument interface.
src/main/java/de/tum/in/www1/artemis/web/rest/dto/LegalDocument.java	Defined LegalDocument interface with methods for type, text, and language.
src/main/java/de/tum/in/www1/artemis/web/rest/dto/PrivacyStatementDTO.java	Added PrivacyStatementDTO class implementing LegalDocument interface.
src/main/java/de/tum/in/www1/artemis/web/rest/open/PublicImprintResource.java	Modified getImprint method to return ImprintDTO.
src/main/java/de/tum/in/www1/artemis/web/rest/open/PublicPrivacyStatementResource.java	Changed getPrivacyStatement method to return PrivacyStatementDTO.
src/main/webapp/app/core/legal/imprint.component.ts	Made imprint property optional.
src/main/webapp/app/core/legal/privacy.component.ts	Made privacyStatement property optional.
src/main/webapp/app/entities/legal-document.model.ts	Changed text property to optional.
src/test/java/de/tum/in/www1/artemis/ImprintResourceIntegrationTest.java	Updated test cases to use ImprintDTO instead of Imprint.
src/test/java/de/tum/in/www1/artemis/PrivacyStatementResourceIntegrationTest.java	Refactored tests to use PrivacyStatementDTO instead of PrivacyStatement.

---

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c93848e and c82393b.

Files selected for processing (2)

• src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/JavaClassDiffSerializer.java (4 hunks)
• src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/OracleGenerator.java (6 hunks)

Additional Context Used

Path-based Instructions (2)

src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/JavaClassDiffSerializer.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/OracleGenerator.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

Additional comments not posted (11)

src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/JavaClassDiffSerializer.java (6)

5-7: Switched from Gson to Jackson for JSON processing.

---

23-23: Introduced a static ObjectMapper instance for reuse across methods, aligning with best practices for performance.

---

70-80: Optimized serialization of super interfaces and annotations using streams and Jackson's ArrayNode.

---

42-66: Refactored serializeClassProperties to use Jackson's ObjectNode. Ensure that all calling methods are updated to handle the new return type.

Verification successful

---

The serializeClassProperties method is called in the generateDiffJSON method of the OracleGenerator class. The return value, which is an ObjectNode, is correctly handled by setting it to the diffJSON object using the set method.

• diffJSON.set("class", serializer.serializeClassProperties());

This indicates that the new return type ObjectNode is being handled appropriately.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify all function calls to `serializeClassProperties` handle the new return type correctly.

# Test: Search for the function usage. Expect: Proper handling of ObjectNode.
rg --type java $'serializeClassProperties'

Length of output: 355

---

---

Script:

#!/bin/bash
# Extract the relevant code from OracleGenerator.java to verify the handling of the ObjectNode return type.
rg -A 10 -B 10 'serializeClassProperties' src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/OracleGenerator.java

Length of output: 1105

---

Line range hint 108-164: Methods for serializing enums, constructors, and methods have been updated to use Jackson and stream operations. Review the use of HashSet in serialization to ensure it aligns with the intended behavior.

Verification successful

---

The search results confirm the use of HashSet in the JavaClassDiffSerializer class, specifically in the serializeConstructor and createMethodJson methods. These methods wrap the modifiers in a HashSet before passing them to SerializerUtil methods. This usage appears to be intentional to ensure the modifiers are treated as a set, which is appropriate for serialization purposes.

• serializeConstructor method:

  constructorJSON.set("modifiers", SerializerUtil.serializeModifiers(new HashSet<>(constructor.getModifiers()), constructor));

• createMethodJson method:

  ObjectNode methodJSON = SerializerUtil.createJsonObject(method.getName(), new HashSet<>(method.getModifiers()), method, method.getAnnotations());

These usages align with the intended behavior of ensuring unique modifiers are serialized correctly.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify the use of HashSet in serialization methods.

# Test: Search for the use of HashSet in serialization methods. Expect: Correct usage.
rg --type java $'HashSet'

Length of output: 128068

---

88-101: Refactored serializeAttributes to filter and serialize attributes using Jackson. Ensure the filtering logic (isElementToIgnore) is correctly implemented.

Verification successful

---

The isElementToIgnore method implementation has been successfully retrieved. It checks if the element has a tag named "oracleIgnore" and returns true if the tag is present, indicating that the element should be ignored.

• Implementation of isElementToIgnore:

  public static boolean isElementToIgnore(JavaAnnotatedElement element) {
      return element.getTagByName("oracleIgnore") != null;
  }

This implementation appears to be straightforward and correct, as it filters out elements based on the presence of a specific tag.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify the filtering logic in `serializeAttributes`.

# Test: Search for the implementation of `isElementToIgnore`. Expect: Correct implementation.
rg --type java $'isElementToIgnore'

Length of output: 1356

---

---

Script:

#!/bin/bash
# Extract the implementation of the `isElementToIgnore` method from `JavaClassDiffSerializer.java`
ast-grep --lang java --pattern $'public static boolean isElementToIgnore(JavaAnnotatedElement element) {
  $$$
}' --json

Length of output: 875

src/main/java/de/tum/in/www1/artemis/service/util/structureoraclegenerator/OracleGenerator.java (5)

11-21: Switched from Gson to Jackson for JSON processing and introduced stream operations for better functional programming practices.

---

67-67: Introduced a static ObjectMapper instance for reuse, which is a good practice for performance optimization.

---

121-129: Implemented error handling in prettyPrint to manage JSON processing exceptions. This is crucial for robustness, especially in server-side code.

---

80-86: Refactored generateStructureOracleJSON to use Jackson's ArrayNode and stream operations for mapping and filtering. Ensure all dependent functionalities are updated to handle these changes.

Verification successful

---

The refactored generateStructureOracleJSON method correctly uses Jackson's ArrayNode and stream operations. The dependent functionality in ProgrammingExerciseService.java handles the resulting JSON string appropriately.

• ProgrammingExerciseService.java correctly processes the structureOracleJSON string generated by generateStructureOracleJSON.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify all dependent functionalities of `generateStructureOracleJSON`.

# Test: Search for the function usage. Expect: Proper handling of ArrayNode.
rg --type java $'generateStructureOracleJSON'

Length of output: 466

---

---

Script:

#!/bin/bash
# Extract the relevant code from ProgrammingExerciseService.java to review the handling of ArrayNode.
rg --type java -A 10 -B 10 'generateStructureOracleJSON' src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseService.java

Length of output: 1563

---

---

Script:

#!/bin/bash
# Extract the relevant code from ProgrammingExerciseService.java to review the handling of structureOracleJSON in saveAndPushStructuralOracle.
rg --type java -A 20 -B 0 'private boolean saveAndPushStructuralOracle' src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseService.java

Length of output: 1497

---

88-112: Refactored generateDiffJSON to use Optional and Jackson for JSON creation. This method now properly handles cases where classes are equal or should be ignored. Ensure that the logic for determining class equality and ignore conditions is robust.

Verification successful

---

The implementations of the classesAreEqual and isElementToIgnore methods have been successfully retrieved.

• classesAreEqual method in JavaClassDiff.java:

  boolean classesAreEqual() {
      return !this.isInterfaceDifferent && !this.isEnumDifferent && !this.isAbstractDifferent && this.superClassNameDiff.isEmpty() && this.superInterfacesDiff.isEmpty()
              && this.attributesDiff.isEmpty() && this.constructorsDiff.isEmpty() && this.methodsDiff.isEmpty() && this.annotationsDiff.isEmpty();
  }

• isElementToIgnore method in JavaClassDiffSerializer.java:

  public static boolean isElementToIgnore(JavaAnnotatedElement element) {
      return element.getTagByName("oracleIgnore") != null;
  }

These methods appear to be robust in determining class equality and ignore conditions. The classesAreEqual method checks various differences between classes, and the isElementToIgnore method checks for a specific tag.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Description: Verify the logic for class equality and ignore conditions in `generateDiffJSON`.

# Test: Search for the implementation of class equality and ignore conditions. Expect: Robust implementation.
rg --type java $'classesAreEqual|isElementToIgnore'

Length of output: 1500

---

---

Script:

#!/bin/bash
# Extract the implementation of the `classesAreEqual` method from `JavaClassDiff.java`.
ast-grep --lang java --pattern $'class JavaClassDiff {
  $$$
  boolean classesAreEqual() {
    $$$
  }
  $$$
}'

Length of output: 202

---

Script:

#!/bin/bash
# Extract the implementation of the `isElementToIgnore` method from `JavaClassDiffSerializer.java`.
ast-grep --lang java --pattern $'class JavaClassDiffSerializer {
  $$$
  public static boolean isElementToIgnore(JavaAnnotatedElement element) {
    $$$
  }
  $$$
}'

Length of output: 256

---

---

Script:

#!/bin/bash
# Search for the `classesAreEqual` method in `JavaClassDiff.java`.
rg --type java 'boolean classesAreEqual' -A 10

Length of output: 878

---

Script:

#!/bin/bash
# Search for the `isElementToIgnore` method in `JavaClassDiffSerializer.java`.
rg --type java 'public static boolean isElementToIgnore' -A 10

Length of output: 625

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 6

Out of diff range and nitpick comments (1)

src/test/java/de/tum/in/www1/artemis/exercise/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java (1)

218-218: Ensure consistent handling of logs across different test scenarios.

Consider using a centralized method for log generation to ensure consistency and reduce duplication across test methods.

Also applies to: 245-245

[pzdr7]

Tested on TS3.

• Changed and viewed the imprint; worked for both English and German ✅
• Created a programming exercise with the default template
  • Template: 0% ✅
  • Solution: 100% ✅
  • Participation: Tests pass and fail as expected ✅

[Jan-Thurner]

Tested on TS3. Works as expected.

[DominikRemo]

Code LGTM

[pzdr7]

Tested again on TS3. Imprint, privacy statement, and programming exercises still work as expected.