Skip to content

Commit

Permalink
Merge branch 'develop' into bugfix/communication/manage-button-alignment
Browse files Browse the repository at this point in the history
  • Loading branch information
krusche authored Nov 3, 2024
2 parents 2328f29 + 1644dc3 commit efe49d7
Show file tree
Hide file tree
Showing 392 changed files with 5,386 additions and 7,252 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ jacocoTestCoverageVerification {
counter = "INSTRUCTION"
value = "COVEREDRATIO"
// TODO: in the future the following value should become higher than 0.92
minimum = 0.894
minimum = 0.892
}
limit {
counter = "CLASS"
Expand Down
4 changes: 4 additions & 0 deletions docs/admin/setup.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This section describes some additional steps that are of interest for production
For information on how to set up extension services to activate additional functionality in your Artemis instance, see
:ref:`their respective documentation <extensions_setup>`.

We recommend using the `Artemis Ansible Collection <https://github.com/ls1intum/artemis-ansible-collection>`_ for
setting up Artemis in production. The collection provides a set of Ansible roles that automate the setup of Artemis,
including the required external system with sane configuration defaults.

.. toctree::
:includehidden:
:maxdepth: 2
Expand Down
3 changes: 2 additions & 1 deletion docs/admin/setup/distributed.rst
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ These credentials are used to clone repositories via HTTPS. You must also add th
specify-concurrent-builds: true # Set to false, if the number of concurrent build jobs should be chosen automatically based on system resources
concurrent-build-size: 1 # If previous value is true: Set to desired value but keep available system resources in mind
asynchronous: true
timeout-seconds: 240 # Time limit of a build before it will be cancelled
build-container-prefix: local-ci-
image-cleanup:
enabled: true # If set to true (recommended), old Docker images will be deleted on a schedule.
Expand All @@ -620,6 +619,8 @@ These credentials are used to clone repositories via HTTPS. You must also add th
build-agent:
short-name: "artemis-build-agent-X" # Short name of the build agent. This should be unique for each build agent. Only lowercase letters, numbers and hyphens are allowed.
display-name: "Artemis Build Agent X" # This value is optional. If omitted, the short name will be used as display name. Display name of the build agent. This is shown in the Artemis UI.
build-timeout-seconds:
max: 240 # (Optional, default 240) Maximum time in seconds a build job is allowed to run. If a build job exceeds this time, it will be cancelled.
Please note that ``artemis.continuous-integration.build-agent.short-name`` must be provided. Otherwise, the build agent will not start.
Expand Down
21 changes: 21 additions & 0 deletions docs/admin/setup/programming-exercises.rst
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,24 @@ Adjust ``dockerFlags`` and ``mavenFlags`` only for student submissions, like thi
dockerFlags += ' --network none'
mavenFlags += ' --offline'
}
Timeout Options
^^^^^^^^^^^^^^^

You can adjust possible :ref:`timeout options<edit_build_duration>` for the build process in :ref:`Integrated Code Lifecycle Setup <Integrated Code Lifecycle Setup>`.
These values will determine what is the minimum, maximum, and default value for the build timeout in seconds that can be set in the Artemis UI.
The max value is the upper limit for the timeout, if the value is set higher than the max value, the max value will be used.

If you want to change these values, you need to change them in ``localci`` and ``buildagent`` nodes.
The corresponding configuration files are ``application-localci.yml`` and ``application-buildagent.yml``.


.. code-block:: yaml
artemis:
continuous-integration:
build-timeout-seconds:
min: <value>
max: <value>
default: <value>
52 changes: 24 additions & 28 deletions docs/admin/setup/security.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,45 +126,41 @@ For Artemis to find the key set `artemis.version-control.ssh-host-key-path` to t
Adapting Nginx to Enable SSH Routing
""""""""""""""""""""""""""""""""""""

To enable SSH routing through Nginx, you can set up an SSH proxy. However, Nginx by itself does
not support SSH, but you can use Nginx to reverse proxy an SSH service (e.g., using sslh to multiplex SSH and HTTPS).
To enable SSH routing through Nginx, you can set up an SSH proxy.

Configure sslh to listen on port 443 (to handle both HTTPS and SSH), by editing the sslh configuration
file (e.g., /etc/default/sslh):

.. code-block:: text
RUN=yes
DAEMON=/usr/sbin/sslh
DAEMON_OPTS="--user sslh --listen 0.0.0.0:443 --ssh 127.0.0.1:22 --ssl 127.0.0.1:8443"
Configure Nginx to proxy HTTPS traffic, by adapting the configuration file to listen on port 8443 for HTTPS:
Configure Nginx to proxy HTTPS traffic on port 443 and SSH traffic on port 7921.

.. code-block:: nginx
server {
listen 8443 ssl;
server_name yourdomain.com;
ssl_certificate /etc/nginx/ssl/nginx.crt;
ssl_certificate_key /etc/nginx/ssl/nginx.key;
http {
server {
listen 443 ssl;
server_name yourdomain.com;
ssl_certificate /etc/nginx/ssl/nginx.crt;
ssl_certificate_key /etc/nginx/ssl/nginx.key;
location / {
proxy_pass http://127.0.0.1:8080;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
}
}
}
location / {
proxy_pass http://127.0.0.1:8080;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
stream {
server {
listen 7921;
proxy_pass 127.0.0.1:7921;
}
}
Restart sslh and Nginx:
Restart Nginx:

.. code-block:: bash
sudo systemctl restart sslh
sudo systemctl restart nginx
By following these steps, you ensure that your key pairs are properly generated and distributed across all
Expand Down
2 changes: 1 addition & 1 deletion docs/dev/setup.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ following dependencies/tools on your machine:
2. `MySQL Database Server 9 <https://dev.mysql.com/downloads/mysql>`__, or `PostgreSQL 17 <https://www.postgresql.org/>`_:
Artemis uses Hibernate to store entities in an SQL database and Liquibase to
automatically apply schema transformations when updating Artemis.
3. `Node.js <https://nodejs.org/en/download>`__: We use Node LTS (>=20.16.0 < 21) to compile
3. `Node.js <https://nodejs.org/en/download>`__: We use Node LTS (>=22.10.0 < 23) to compile
and run the client Angular application. Depending on your system, you
can install Node either from source or as a pre-packaged bundle.
4. `Npm <https://nodejs.org/en/download>`__: We use Npm (>=10.8.0) to
Expand Down
4 changes: 3 additions & 1 deletion docs/user/exercises/programming-exercise-setup.inc
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ You must then change the paths in the build script if necessary. Please refer to
- Changing the checkout paths can lead to build errors if the build script is not adapted accordingly.
- For C programming exercises, if used with the default docker image, changing the checkout paths will lead to build errors. The default docker image is configured to work with the default checkout paths.

.. _configure_static_code_analysis_tools:
.. _edit_build_duration:

Edit Maximum Build Duration
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -412,6 +412,8 @@ You can change the maximum build duration by using the slider.
.. figure:: programming/timeout-slider.png
:align: center

.. _configure_static_code_analysis_tools:

Configure static code analysis
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ rootProject.name=Artemis
profile=dev

# Build properties
node_version=20.16.0
node_version=22.10.0
npm_version=10.8.0

# Dependency versions
Expand Down
8 changes: 4 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ module.exports = {
coverageThreshold: {
global: {
// TODO: in the future, the following values should increase to at least 90%
statements: 87.52,
branches: 73.62,
functions: 82.12,
lines: 87.57,
statements: 87.46,
branches: 73.56,
functions: 82.04,
lines: 87.52,
},
},
coverageReporters: ['clover', 'json', 'lcov', 'text-summary'],
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
"weak-napi": "2.0.2"
},
"engines": {
"node": ">=20.16.0"
"node": ">=22.10.0"
},
"scripts": {
"build": "npm run webapp:prod --",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
import java.util.Optional;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;
import org.springframework.transaction.annotation.Transactional;

import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText;
import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository;
Expand Down Expand Up @@ -42,6 +44,14 @@ public interface LongFeedbackTextRepository extends ArtemisJpaRepository<LongFee
""")
Optional<LongFeedbackText> findWithFeedbackAndResultAndParticipationByFeedbackId(@Param("feedbackId") final Long feedbackId);

@Modifying
@Transactional
@Query("""
DELETE FROM LongFeedbackText longFeedback
WHERE longFeedback.feedback.id IN :feedbackIds
""")
void deleteByFeedbackIds(@Param("feedbackIds") List<Long> feedbackIds);

default LongFeedbackText findByFeedbackIdWithFeedbackAndResultAndParticipationElseThrow(final Long feedbackId) {
return getValueElseThrow(findWithFeedbackAndResultAndParticipationByFeedbackId(feedbackId), feedbackId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO;
import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository;
import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository;
import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository;
import de.tum.cit.aet.artemis.assessment.repository.ResultRepository;
import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService;
import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService;
Expand Down Expand Up @@ -67,10 +68,12 @@ public class AssessmentService {

protected final ResultWebsocketService resultWebsocketService;

private final LongFeedbackTextRepository longFeedbackTextRepository;

public AssessmentService(ComplaintResponseService complaintResponseService, ComplaintRepository complaintRepository, FeedbackRepository feedbackRepository,
ResultRepository resultRepository, StudentParticipationRepository studentParticipationRepository, ResultService resultService, SubmissionService submissionService,
SubmissionRepository submissionRepository, ExamDateService examDateService, UserRepository userRepository, Optional<LtiNewResultService> ltiNewResultService,
SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService) {
SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService, LongFeedbackTextRepository longFeedbackTextRepository) {
this.complaintResponseService = complaintResponseService;
this.complaintRepository = complaintRepository;
this.feedbackRepository = feedbackRepository;
Expand All @@ -84,6 +87,7 @@ public AssessmentService(ComplaintResponseService complaintResponseService, Comp
this.ltiNewResultService = ltiNewResultService;
this.singleUserNotificationService = singleUserNotificationService;
this.resultWebsocketService = resultWebsocketService;
this.longFeedbackTextRepository = longFeedbackTextRepository;
}

/**
Expand Down Expand Up @@ -283,6 +287,9 @@ public Result saveManualAssessment(final Submission submission, final List<Feedb
User user = userRepository.getUser();
result.setAssessor(user);

// long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save}
resultService.deleteLongFeedback(feedbackList, result);

result.updateAllFeedbackItems(feedbackList, false);
result.determineAssessmentType();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

import jakarta.annotation.Nullable;
Expand Down Expand Up @@ -494,45 +495,49 @@ public Map<Long, String> getLogsAvailabilityForResults(List<Result> results, Par

@NotNull
private List<Feedback> saveFeedbackWithHibernateWorkaround(@NotNull Result result, List<Feedback> feedbackList) {
// Avoid hibernate exception
List<Feedback> savedFeedbacks = new ArrayList<>();
// Collect ids of feedbacks that have long feedback.
List<Long> feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId)
.toList();
// Get long feedback list from the database.
List<LongFeedbackText> longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback);

// Convert list to map for accessing later.
Map<Long, LongFeedbackText> longLongFeedbackTextMap = longFeedbackTextList.stream()
.collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), longFeedbackText -> longFeedbackText));
feedbackList.forEach(feedback -> {
// cut association to parent object
feedback.setResult(null);
LongFeedbackText longFeedback = null;
// look for long feedback that parent feedback has and cut the association between them.
if (feedback.getId() != null && feedback.getHasLongFeedbackText()) {
longFeedback = longLongFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.clearLongFeedback();
}
}
// persist the child object without an association to the parent object.
feedback = feedbackRepository.saveAndFlush(feedback);
// restore the association to the parent object
feedback.setResult(result);
// Fetch long feedback texts associated with the provided feedback list
Map<Long, LongFeedbackText> longFeedbackTextMap = longFeedbackTextRepository
.findByFeedbackIds(feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId).toList()).stream()
.collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), Function.identity()));

// restore the association of the long feedback to the parent feedback
if (longFeedback != null) {
feedback.setDetailText(longFeedback.getText());
}
feedbackList.forEach(feedback -> {
handleFeedbackPersistence(feedback, result, longFeedbackTextMap);
savedFeedbacks.add(feedback);
});

return savedFeedbacks;
}

private void handleFeedbackPersistence(Feedback feedback, Result result, Map<Long, LongFeedbackText> longFeedbackTextMap) {
// Temporarily detach feedback from the parent result to avoid Hibernate issues
feedback.setResult(null);

// Clear the long feedback if it exists in the map
if (feedback.getId() != null && feedback.getHasLongFeedbackText()) {
LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.clearLongFeedback();
}
}

// Persist the feedback entity without the parent association
feedback = feedbackRepository.saveAndFlush(feedback);

// Restore associations to the result and long feedback after persistence
feedback.setResult(result);
LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId());
if (longFeedback != null) {
feedback.setDetailText(longFeedback.getText());
}
}

@NotNull
private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) {
if (shouldSave) {
// long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save}
deleteLongFeedback(result.getFeedbacks(), result);
// Note: This also saves the feedback objects in the database because of the 'cascade = CascadeType.ALL' option.
return resultRepository.save(result);
}
Expand Down Expand Up @@ -623,4 +628,33 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee
public long getMaxCountForExercise(long exerciseId) {
return studentParticipationRepository.findMaxCountForExercise(exerciseId);
}

/**
* Deletes long feedback texts for the provided list of feedback items to prevent duplicate entries in the {@link LongFeedbackTextRepository}.
* <br>
* This method processes the provided list of feedback items, identifies those with associated long feedback texts, and removes them in bulk
* from the repository to avoid potential duplicate entry errors when saving new feedback entries.
* <p>
* Primarily used to ensure data consistency in the {@link LongFeedbackTextRepository}, especially during operations where feedback entries are
* overridden or updated. The deletion is performed only for feedback items with a non-null ID and an associated long feedback text.
* <p>
* This approach reduces the need for individual deletion calls and performs batch deletion in a single database operation.
* <p>
* **Note:** This method should only be used for manually assessed submissions, not for fully automatic assessments, due to its dependency on the
* {@link Result#updateAllFeedbackItems} method, which is designed for manual feedback management. Using this method with automatic assessments could
* lead to unintended behavior or data inconsistencies.
*
* @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. Only feedback items that have long feedback texts and a
* non-null ID will be processed.
* @param result The {@link Result} object associated with the feedback items, used to update feedback list before processing.
*/
public void deleteLongFeedback(List<Feedback> feedbackList, Result result) {
if (feedbackList == null) {
return;
}
List<Long> feedbackIdsWithLongText = feedbackList.stream().filter(feedback -> feedback.getHasLongFeedbackText() && feedback.getId() != null).map(Feedback::getId).toList();
longFeedbackTextRepository.deleteByFeedbackIds(feedbackIdsWithLongText);
List<Feedback> feedbacks = new ArrayList<>(feedbackList);
result.updateAllFeedbackItems(feedbacks, true);
}
}
Loading

0 comments on commit efe49d7

Please sign in to comment.