From 857f6466780b98e5af8dfe9b66253683c5f1629e Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 3 Dec 2024 10:55:28 +0100 Subject: [PATCH 1/6] Upgrade Spring Boot version to address many security vulnerabilities (#52) Fixes #28 ([CVE-2024-38809](https://spring.io/security/cve-2024-38809)), #29 ([CVE-2024-38816](https://spring.io/security/cve-2024-38816)), and #46 ([CVE-2024-38820](https://spring.io/security/cve-2024-38820)) Chose to stay on the 3.2 Spring Boot train despite 3.4 being out. Waiting for a more conscious to do the upgrade in case there are other changes required. Luckily none of the preconditions of the vulnerabilities were true for SciPro so they could not be exploited. Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/52 Reviewed-by: Tom Zhao <tom.zhao@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index e69b87c989..8448e670cf 100755 --- a/pom.xml +++ b/pom.xml @@ -101,7 +101,7 @@ <dependency> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-dependencies</artifactId> - <version>3.2.5</version> + <version>3.2.12</version> <scope>import</scope> <type>pom</type> </dependency> From c6bd17d9ad5124a9cb334701a181bc4a06a83653 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 16 Dec 2024 11:24:33 +0100 Subject: [PATCH 2/6] Fix grade calculator being serialized (#59) The new calculator that's based on templates has a reference to the @Entity for the template which should not be serialized. Fixes #40 ## How to test/replicate 1. Log in as a supervisor 1. Open a project that's new enough to use a grading report template with grade limits 1. Go to the "Finishing up" tab 1. Go to the sub-tab for an individual author Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/59 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../su/dsv/scipro/report/GradeCalculator.java | 4 +--- .../grading/GradingReportPointsPanel.java | 20 ++++++------------- .../IndividualAuthorAssessmentPanel.java | 4 ++-- .../grading/GradingReportPointsPanelTest.java | 2 +- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java b/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java index 55cd0c3343..67abd527b3 100644 --- a/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java +++ b/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java @@ -1,8 +1,6 @@ package se.su.dsv.scipro.report; -import java.io.Serializable; - -public interface GradeCalculator extends Serializable { +public interface GradeCalculator { GradingReport.Grade getGrade(GradingReport gradingReport); long getPoints(GradingReport gradingReport); diff --git a/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java index 593ee532c5..4f95d80881 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java @@ -4,7 +4,6 @@ import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.model.IModel; -import org.apache.wicket.model.LoadableDetachableModel; import se.su.dsv.scipro.components.OppositeVisibility; import se.su.dsv.scipro.report.GradeCalculator; import se.su.dsv.scipro.report.GradingReport; @@ -18,15 +17,13 @@ public class GradingReportPointsPanel extends Panel { public GradingReportPointsPanel( String id, final IModel<? extends GradingReport> gradingReportIModel, - final GradeCalculator gradeCalculator + final IModel<GradeCalculator> gradeCalculator ) { super(id, gradingReportIModel); - final IModel<GradingReport.Grade> gradeModel = new LoadableDetachableModel<>() { - @Override - protected GradingReport.Grade load() { - return gradingReportIModel.getObject().getGrade(gradeCalculator); - } - }; + final IModel<GradingReport.Grade> gradeModel = gradingReportIModel.combineWith( + gradeCalculator, + GradingReport::getGrade + ); final Label grade = new Label(GRADE, gradeModel.map(GradingReport.Grade::name)) { @Override protected void onConfigure() { @@ -36,12 +33,7 @@ public class GradingReportPointsPanel extends Panel { }; add(grade); - final IModel<Long> points = new LoadableDetachableModel<>() { - @Override - protected Long load() { - return gradingReportIModel.getObject().getPoints(gradeCalculator); - } - }; + final IModel<Long> points = gradingReportIModel.combineWith(gradeCalculator, GradingReport::getPoints); add(new Label(POINTS_LABEL, points)); add(new WebMarkupContainer(NO_GRADE_EXPLANATION).add(new OppositeVisibility(grade))); diff --git a/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java index 6d4ba8e0d6..c9c0938a58 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java @@ -271,8 +271,8 @@ public class IndividualAuthorAssessmentPanel extends GenericPanel<User> { new TemplatePanel("points_to_grade_conversion", gradingReport.map(SupervisorGradingReport::getProject)) ); - GradeCalculator supervisorCalculator = gradeCalculatorService.getSupervisorCalculator( - gradingReport.getObject().getProject() + IModel<GradeCalculator> supervisorCalculator = LoadableDetachableModel.of(() -> + gradeCalculatorService.getSupervisorCalculator(gradingReport.getObject().getProject()) ); add(new GradingReportPointsPanel("points", gradingReport, supervisorCalculator)); diff --git a/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java b/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java index caa5835e3f..f358d419e6 100644 --- a/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java @@ -62,7 +62,7 @@ public class GradingReportPointsPanelTest extends SciProTest { private void startPanel() { panel = tester.startComponentInPage( - new GradingReportPointsPanel("id", Model.of(gradingReport), gradeCalculator) + new GradingReportPointsPanel("id", Model.of(gradingReport), () -> gradeCalculator) ); } From f67f37ecdd6bf4f45b36e9fb30039f6ad4b7f142 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 16 Dec 2024 13:23:37 +0100 Subject: [PATCH 3/6] Keep and validate project type selection when creating/editing application periods (#47) If you have FormComponents in a ListView you need to call setReuseItems(true) on the ListView. Otherwise the ListItems will be recreated before rendering which results in them losing their "converted input" (what Wicket calls the submitted value). Instead of simply calling setReuseItems(true) on the ListView, which would've solved the problem, it was instead replaced by a proper FormComponent for dealing with this exact case (a CheckboxMultipleChoice component). This reduces the amount of code required and more clearly communicates intent. The change required some minor test refactoring. Fixes #33 --- Now requires at least one project type to be selected before saving. Fixes #34 --- ## How to test 1. Go to "Admin" / "Match" / "Application periods" 2. Click create new 3. Submit without selecting any types 4. See that there's proper feedback 5. Leave name blank and select some types 6. Submit 7. See that the project type selection sticks around Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/47 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../AdminEditApplicationPeriodPage.html | 6 +-- .../AdminEditApplicationPeriodPage.java | 50 ++++--------------- .../AdminEditApplicationPeriodPage.properties | 3 +- .../AdminEditApplicationPeriodPageTest.java | 25 +++++++++- 4 files changed, 36 insertions(+), 48 deletions(-) diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html index 04de12622c..8c5597de96 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html @@ -16,10 +16,8 @@ <fieldset class="mb-3"> <legend><wicket:message key="projectTypes"/></legend> - <div class="form-check" wicket:id="projectTypes"> - <input type="checkbox" wicket:id="checkbox" class="form-check-input"/> - <label class="form-check-label" wicket:for="checkbox"><span wicket:id="name"></span></label> - </div> + <div wicket:id="projectTypes"></div> + <div wicket:id="projectTypesFeedback"></div> </fieldset> <div class="form-row"> diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java index 2409d13d83..8781578a73 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java @@ -4,17 +4,14 @@ import jakarta.inject.Inject; import java.time.LocalDate; import java.time.LocalTime; import java.util.List; -import org.apache.wicket.extensions.model.AbstractCheckBoxModel; import org.apache.wicket.feedback.FencedFeedbackPanel; -import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.form.*; -import org.apache.wicket.markup.html.list.ListItem; -import org.apache.wicket.markup.html.list.ListView; import org.apache.wicket.markup.html.panel.ComponentFeedbackPanel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; import org.apache.wicket.model.LoadableDetachableModel; import org.apache.wicket.request.mapper.parameter.PageParameters; +import se.su.dsv.scipro.components.BootstrapCheckBoxMultipleChoice; import se.su.dsv.scipro.components.BootstrapDatePicker; import se.su.dsv.scipro.components.BootstrapTimePicker; import se.su.dsv.scipro.components.DatesValidator; @@ -39,7 +36,6 @@ public class AdminEditApplicationPeriodPage public static final String TITLE = "title"; public static final String FEEDBACK = "Feedback"; public static final String TITLE_FEEDBACK = "titleFeedback"; - public static final String CHECKBOX = "checkbox"; @Inject private ProjectTypeService projectTypeService; @@ -62,19 +58,15 @@ public class AdminEditApplicationPeriodPage ); add(new ComponentFeedbackPanel(TITLE_FEEDBACK, title)); add(title); - add( - new ListView<>(PROJECT_TYPES, availableProjectTypes()) { - @Override - protected void populateItem(ListItem<ProjectType> item) { - item.add( - new CheckBox(CHECKBOX, new ProjectTypeSelectionModel(item.getModel())).setOutputMarkupId( - true - ) - ); - item.add(new Label("name", item.getModel().map(ProjectType::getName))); - } - } + BootstrapCheckBoxMultipleChoice<ProjectType> projectTypeChoice = new BootstrapCheckBoxMultipleChoice<>( + PROJECT_TYPES, + LambdaModel.of(getModel(), ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes), + availableProjectTypes(), + new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId) ); + projectTypeChoice.setRequired(true); + add(projectTypeChoice); + add(new FencedFeedbackPanel("projectTypesFeedback", projectTypeChoice)); final FormComponent<LocalDate> startDate = addDateField( START_DATE, LambdaModel.of(getModel(), ApplicationPeriod::getStartDate, ApplicationPeriod::setStartDate) @@ -139,30 +131,6 @@ public class AdminEditApplicationPeriodPage getRootForm().error(getString("overlapping")); } } - - private class ProjectTypeSelectionModel extends AbstractCheckBoxModel { - - private final IModel<ProjectType> model; - - public ProjectTypeSelectionModel(IModel<ProjectType> model) { - this.model = model; - } - - @Override - public boolean isSelected() { - return getModelObject().getProjectTypes().contains(model.getObject()); - } - - @Override - public void select() { - getModelObject().addProjectType(model.getObject()); - } - - @Override - public void unselect() { - getModelObject().removeProjectType(model.getObject()); - } - } } private LoadableDetachableModel<ApplicationPeriod> getLoaded(final PageParameters pp) { diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties index f5095052d4..dc945a5788 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties @@ -14,4 +14,5 @@ success= Application period saved. overlapping= Overlapping application period already exists. date.Required= You need to specify a valid date. hours.Required= Hours field is required. -minutes.Required= Minutes field is required. \ No newline at end of file +minutes.Required= Minutes field is required. +projectTypes.Required=You must select at least one project type. diff --git a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java index 7b68730909..a6bc599bd9 100644 --- a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java +++ b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java @@ -7,8 +7,10 @@ import java.io.Serializable; import java.time.LocalDate; import java.util.Collections; import java.util.List; +import org.apache.wicket.Component; import org.apache.wicket.Page; import org.apache.wicket.feedback.FeedbackMessage; +import org.apache.wicket.markup.html.form.CheckBoxMultipleChoice; import org.apache.wicket.markup.html.form.RequiredTextField; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.util.tester.FormTester; @@ -36,6 +38,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { @BeforeEach public void setUp() throws Exception { bachelor = new ProjectType(DegreeType.BACHELOR, "Bachelor", "Bachelor"); + bachelor.setId(8L); bachelor.addModule(ProjectModule.MATCH); when(projectTypeService.findWithModule(ProjectModule.MATCH)).thenReturn(Collections.singletonList(bachelor)); startPage(); @@ -47,8 +50,12 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { } @Test + @SuppressWarnings("unchecked") public void contains_project_type_selection() { - tester.assertModelValue(path(FORM, PROJECT_TYPES), projectTypeService.findWithModule(ProjectModule.MATCH)); + tester.assertComponent(path(FORM, PROJECT_TYPES), CheckBoxMultipleChoice.class); + Component component = tester.getComponentFromLastRenderedPage(path(FORM, PROJECT_TYPES)); + CheckBoxMultipleChoice<ProjectType> choice = (CheckBoxMultipleChoice<ProjectType>) component; + Assertions.assertEquals(projectTypeService.findWithModule(ProjectModule.MATCH), choice.getChoices()); } @Test @@ -105,7 +112,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { ); FormTester formTester = tester.newFormTester(FORM); fillInForm("Title", 0, 1, 2, formTester); - formTester.setValue(path(PROJECT_TYPES, 0, CHECKBOX), true); + formTester.setValue(path(PROJECT_TYPES), String.valueOf(bachelor.getId())); formTester.submit(); ArgumentCaptor<ApplicationPeriod> captor = ArgumentCaptor.forClass(ApplicationPeriod.class); @@ -113,12 +120,25 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { MatcherAssert.assertThat(captor.getValue().getProjectTypes(), Matchers.hasItem(bachelor)); } + @Test + public void requires_at_least_one_project_type_to_be_selected() { + FormTester formTester = tester.newFormTester(FORM); + fillInFormWithValidValues(formTester); + formTester.setValue(path(PROJECT_TYPES), ""); + formTester.submit(); + tester.assertErrorMessages(tester.getLastRenderedPage().getString("projectTypes.Required")); + } + private void submitForm(String title, int startDate, int endDate, int courseStartDate) { FormTester formTester = tester.newFormTester(FORM); fillInForm(title, startDate, endDate, courseStartDate, formTester); formTester.submit(); } + private void fillInFormWithValidValues(FormTester formTester) { + fillInForm("Title", 0, 1, 2, formTester); + } + private void fillInForm(String title, int startDate, int endDate, int courseStartDate, FormTester formTester) { formTester.setValue(TITLE, title); final LocalDate now = LocalDate.now(); @@ -126,6 +146,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { formTester.setValue(END_DATE, now.plusDays(endDate).toString()); formTester.setValue(COURSE_START_DATE, now.plusDays(courseStartDate).toString()); formTester.setValue("courseStartTime", "08:00"); + formTester.setValue(PROJECT_TYPES, String.valueOf(bachelor.getId())); } private void startPage() { From 89c8a4f8a25c43376e5f1eea6f80be7c8505fa33 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 16 Dec 2024 13:26:19 +0100 Subject: [PATCH 4/6] Update instructions for how to get Prettier to format on save (#55) IntelliJ requires Node.js to be installed for it to be able to run Prettier and format the code. Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/55 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- README.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index dc2b0cb977..f5b81e7a88 100644 --- a/README.md +++ b/README.md @@ -19,9 +19,14 @@ to format all Java code. To reformat the code run Yes it's a mouthful but unfortunately the [prettier-maven-plugin](https://github.com/HubSpot/prettier-maven-plugin) does not work due to an [outstanding issue](https://github.com/HubSpot/prettier-maven-plugin/issues/79). -An easier way to reformat code is to set IntelliJ to do it on save. Go to -`Settings -> Language & Frameworks -> JavaScript -> Prettier` and then check +The formatting is validated by CI, but you should do it beforehand with a simple `./mvnw verify -pl .`. + +### Making IntelliJ format for you +For this to work you also need to have [Node.js](https://nodejs.org) +installed and configured under `Settings -> Language & Frameworks -> Node.js` +and the file you're saving *must* be able to compile otherwise no formatting +can be performed. + +Go to `Settings -> Language & Frameworks -> JavaScript -> Prettier` and then check `Automatic Prettier Configuration`, set `Run for files` to `**/*.{java}`, and finally check `Run on save`. - -The formatting is validated by CI, but you should do it beforehand with a simple `./mvnw verify -pl .`. From 5fbf4ec0c0591dd42a7ce7101096b5eba1c36599 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 16 Dec 2024 13:55:33 +0100 Subject: [PATCH 5/6] Switch from an in-memory HSQLDB to MariaDB during integration tests (#57) Currently our integration tests run against an in-memory HSQLDB whose schema is created by Hibernate based on our JPA annotations. This has differences from the MariaDB schema created by our Flyway migrations. It is also a completely different database engine so who knows what other differences there are. This proposal changes this so that it will instead use [Testcontainers](https://testcontainers.com/) to spin up a MariaDB Docker container that then has the Flyway migrations ran before being used in tests. Pros: * Same database engine in tests as production * Flyway migrations are tested * Database schema is the same in tests as production (`NOT NULL` constraints, foreign keys, and so on) Cons: * *Much* slower test executions and they will get slower over time as more migrations are added Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/57 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> --- core/pom.xml | 19 +++++++++++++-- .../main/resources/META-INF/persistence.xml | 12 ---------- .../FinalSeminarSchedulingTest.java | 1 + .../peer/CommentThreadServiceImplTest.java | 2 ++ .../peer/PeerRequestServiceImplTest.java | 2 ++ .../PeerReviewServiceImplIntegrationTest.java | 2 ++ .../peer/PeerReviewServiceImplTest.java | 2 ++ .../se/su/dsv/scipro/peer/TestPeerReview.java | 2 ++ .../UrkundSubmissionRepositoryTest.java | 4 ++++ .../se/su/dsv/scipro/test/SpringTest.java | 23 +++++++++++++++++-- pom.xml | 9 -------- 11 files changed, 53 insertions(+), 25 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index a3c2a1a1e9..e34cb44596 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -86,8 +86,23 @@ <scope>test</scope> </dependency> <dependency> - <groupId>org.hsqldb</groupId> - <artifactId>hsqldb</artifactId> + <groupId>org.testcontainers</groupId> + <artifactId>junit-jupiter</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.testcontainers</groupId> + <artifactId>mariadb</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.mariadb.jdbc</groupId> + <artifactId>mariadb-java-client</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.flywaydb</groupId> + <artifactId>flyway-mysql</artifactId> <scope>test</scope> </dependency> diff --git a/core/src/main/resources/META-INF/persistence.xml b/core/src/main/resources/META-INF/persistence.xml index fc5f0a5977..902cc7aeae 100755 --- a/core/src/main/resources/META-INF/persistence.xml +++ b/core/src/main/resources/META-INF/persistence.xml @@ -4,9 +4,6 @@ xsi:schemaLocation="https://jakarta.ee/xml/ns/persistence https://jakarta.ee/xml/ns/persistence/persistence_3_0.xsd" version="3.0"> - <!-- NOTE THAT THERE ARE TWO PERSISTENCE UNITS, one default and one test - used for either running or unit-tests --> - <!-- A JPA Persistence Unit --> <persistence-unit name="defaultPersistenceUnit" transaction-type="RESOURCE_LOCAL"> @@ -17,13 +14,4 @@ </properties> </persistence-unit> - <!-- A JPA Persistence Unit used for tests --> - <persistence-unit name="testPersistenceUnit" - transaction-type="RESOURCE_LOCAL"> - <properties> - <property name="jakarta.persistence.jdbc.driver" value="org.hsqldb.jdbc.JDBCDriver"/> - <property name="jakarta.persistence.jdbc.url" value="jdbc:hsqldb:mem:test"/> - <property name="hibernate.hbm2ddl.auto" value="create"/> - </properties> - </persistence-unit> </persistence> diff --git a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java index 23ebd8113c..cca047aed5 100644 --- a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java +++ b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java @@ -197,6 +197,7 @@ public class FinalSeminarSchedulingTest extends IntegrationTest { NonWorkDayPeriod nonWorkDayPeriod = new NonWorkDayPeriod(); nonWorkDayPeriod.setStartDate(date); nonWorkDayPeriod.setEndDate(date); + nonWorkDayPeriod.setComment("test non work day"); save(nonWorkDayPeriod); } diff --git a/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java index cd037f3ee5..625162f167 100644 --- a/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java +++ b/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java @@ -12,6 +12,7 @@ import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.DegreeType; +import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.test.Dates; @@ -66,6 +67,7 @@ public class CommentThreadServiceImplTest extends IntegrationTest { PeerRequest peerRequest = new PeerRequest(); peerRequest.setProject(project); peerRequest.setRequester(createUser()); + peerRequest.setLanguage(Language.ENGLISH); final FileDescription fileDescription = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(fileDescription); diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java index 215e6c5f45..fc98ba6f7b 100644 --- a/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java +++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java @@ -13,6 +13,7 @@ import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.DegreeType; +import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.PageRequest; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; @@ -158,6 +159,7 @@ public class PeerRequestServiceImplTest extends IntegrationTest { peerRequest.setProject(project); peerRequest.setRequester(requester); peerRequest.setStatus(status); + peerRequest.setLanguage(Language.ENGLISH); final FileDescription fileDescription = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(fileDescription); diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java index 15a004fd89..a954f455be 100644 --- a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java +++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java @@ -14,6 +14,7 @@ import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.DegreeType; +import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.PageRequest; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; @@ -102,6 +103,7 @@ public class PeerReviewServiceImplIntegrationTest extends IntegrationTest { PeerRequest peerRequest = new PeerRequest(); peerRequest.setProject(createProject()); peerRequest.setRequester(createUser()); + peerRequest.setLanguage(Language.ENGLISH); final FileDescription fileDescription = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(fileDescription); diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java index 9c6a5dd3da..6495faaf7a 100644 --- a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java +++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java @@ -14,6 +14,7 @@ import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.DegreeType; +import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.PageRequest; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; @@ -95,6 +96,7 @@ public class PeerReviewServiceImplTest extends IntegrationTest { peerRequest.setRequester( save(User.builder().firstName("Bob").lastName("Sponge").emailAddress("bob@example.com").build()) ); + peerRequest.setLanguage(Language.ENGLISH); final FileDescription fileDescription = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(fileDescription); diff --git a/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java b/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java index 34e5e5c7e2..fe5e2be992 100755 --- a/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java +++ b/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java @@ -19,6 +19,7 @@ import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.project.ProjectStatus; +import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.PageRequest; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; @@ -196,6 +197,7 @@ public class TestPeerReview extends IntegrationTest { request.setComment(comment); request.setRequester(requester); request.setProject(project); + request.setLanguage(Language.ENGLISH); final FileDescription fileDescription = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(fileDescription); diff --git a/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java b/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java index c09f3ea67e..a3b40ed7ae 100644 --- a/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java +++ b/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java @@ -13,6 +13,7 @@ import org.hamcrest.TypeSafeMatcher; import org.junit.jupiter.api.Test; import se.su.dsv.scipro.file.FileDescription; import se.su.dsv.scipro.file.FileReference; +import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.test.SpringTest; public class UrkundSubmissionRepositoryTest extends SpringTest { @@ -23,11 +24,14 @@ public class UrkundSubmissionRepositoryTest extends SpringTest { @Test public void save() { final Instant submitted = Instant.now(); + User bob = User.builder().firstName("Bob").lastName("Sponge").emailAddress("bob@example.com").build(); + save(bob); final UrkundSubmission submission = new UrkundSubmission(); submission.setState(UrkundSubmission.State.SUBMITTED); submission.setMessage("Hi"); submission.setSubmitted(submitted); submission.setNextPoll(submitted); + submission.setReceiver(bob); final FileDescription file = save(new FileDescription()); final FileReference fileReference = new FileReference(); fileReference.setFileDescription(file); diff --git a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java index 2b68b55008..04d0f70da7 100644 --- a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java +++ b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java @@ -4,28 +4,47 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.EntityTransaction; import jakarta.persistence.Persistence; +import java.sql.SQLException; import java.time.Clock; +import java.util.Map; import java.util.Optional; +import org.flywaydb.core.Flyway; +import org.hibernate.cfg.Environment; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.mariadb.jdbc.MariaDbDataSource; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.testcontainers.containers.MariaDBContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; import se.su.dsv.scipro.CoreConfig; import se.su.dsv.scipro.RepositoryConfiguration; import se.su.dsv.scipro.profiles.CurrentProfile; import se.su.dsv.scipro.sukat.Sukat; import se.su.dsv.scipro.system.CurrentUser; +@Testcontainers public abstract class SpringTest { private EntityManager entityManager; private EntityManagerFactory entityManagerFactory; + @Container + static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11"); + @BeforeEach - public final void prepareSpring() { - entityManagerFactory = Persistence.createEntityManagerFactory("testPersistenceUnit"); + public final void prepareSpring() throws SQLException { + MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl()); + dataSource.setUser(mariaDBContainer.getUsername()); + dataSource.setPassword(mariaDBContainer.getPassword()); + + Flyway.configure().dataSource(dataSource).load().migrate(); + + Map<String, Object> jpaProperties = Map.of(Environment.JAKARTA_JTA_DATASOURCE, dataSource); + entityManagerFactory = Persistence.createEntityManagerFactory("defaultPersistenceUnit", jpaProperties); this.entityManager = entityManagerFactory.createEntityManager(); EntityTransaction transaction = entityManager.getTransaction(); transaction.begin(); diff --git a/pom.xml b/pom.xml index 8448e670cf..a4b4f57961 100755 --- a/pom.xml +++ b/pom.xml @@ -34,7 +34,6 @@ <querydsl.version>5.0.0</querydsl.version> <jakarta.servlet.version>5.0.0</jakarta.servlet.version> <junit.version>5.9.3</junit.version> - <hsqldb.version>2.7.1</hsqldb.version> <mockito.version>5.3.1</mockito.version> <flyway.version>9.19.1</flyway.version> <jersey.version>3.1.6</jersey.version> @@ -133,14 +132,6 @@ <scope>import</scope> </dependency> - <!-- Database stuff --> - <dependency> - <groupId>org.hsqldb</groupId> - <artifactId>hsqldb</artifactId> - <version>${hsqldb.version}</version> - <scope>test</scope> - </dependency> - <dependency> <groupId>org.mariadb.jdbc</groupId> <artifactId>mariadb-java-client</artifactId> From a1d3d0be8d813593b8b9a255c0bbc9746b707169 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 16 Dec 2024 14:20:41 +0100 Subject: [PATCH 6/6] Fix certain milestones not getting activated (#54) There was a missing bean definition that was responsible for marking certain milestones based on system events. Fixes #53 ## How to test 1. Log in as an author 2. Go to "Peer" tab within a project 3. Request a peer review 4. [If needed] Switch to another author and request another review 5. Perform a peer review 6. See that the peer reviewer completed milestone (first or second depending) is marked as completed Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/54 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../main/java/se/su/dsv/scipro/CoreConfig.java | 18 ++++++++++++++++++ .../dsv/scipro/test/BeanDefinitionsTest.java | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java diff --git a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java index 2dc8f54929..43c4d7ed10 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -96,6 +96,7 @@ import se.su.dsv.scipro.match.TargetRepository; import se.su.dsv.scipro.match.TargetServiceImpl; import se.su.dsv.scipro.milestones.MilestoneActivityTemplateRepository; import se.su.dsv.scipro.milestones.service.ActivateCompletedMilestonesOnNewProjects; +import se.su.dsv.scipro.milestones.service.MilestoneActivator; import se.su.dsv.scipro.milestones.service.MilestoneActivityTemplateService; import se.su.dsv.scipro.milestones.service.impl.MilestoneActivityTemplateServiceImpl; import se.su.dsv.scipro.milestones.service.impl.MilestonePhaseTemplateServiceImpl; @@ -1129,4 +1130,21 @@ public class CoreConfig { public GroupFacadeImpl groupFacade() { return new GroupFacadeImpl(); } + + @Bean + public MilestoneActivator milestoneActivator( + EventBus eventBus, + MilestoneServiceImpl milestoneService, + MilestoneActivityTemplateService milestoneActivityTemplateService, + FinalSeminarService finalSeminarService, + NotificationController notificationController + ) { + return new MilestoneActivator( + milestoneActivityTemplateService, + milestoneService, + eventBus, + finalSeminarService, + notificationController + ); + } } diff --git a/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java b/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java new file mode 100644 index 0000000000..8dd6ff6130 --- /dev/null +++ b/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java @@ -0,0 +1,18 @@ +package se.su.dsv.scipro.test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import jakarta.inject.Inject; +import org.junit.jupiter.api.Test; +import se.su.dsv.scipro.milestones.service.MilestoneActivator; + +public class BeanDefinitionsTest extends IntegrationTest { + + @Inject + MilestoneActivator milestoneActivator; + + @Test + public void milestone_activator() { + assertNotNull(milestoneActivator); + } +}