From 491832c004d249e069843c7ec6672b1907f7e568 Mon Sep 17 00:00:00 2001 From: Szymon Szukalski Date: Fri, 26 Jul 2024 14:45:27 +1000 Subject: Simplify entity model --- .../com/stileeducation/markr/entity/Student.java | 21 ++----------------- .../java/com/stileeducation/markr/entity/Test.java | 21 ++----------------- .../stileeducation/markr/entity/TestResult.java | 24 ++++++++++------------ 3 files changed, 15 insertions(+), 51 deletions(-) (limited to 'src/main/java/com/stileeducation') diff --git a/src/main/java/com/stileeducation/markr/entity/Student.java b/src/main/java/com/stileeducation/markr/entity/Student.java index 30a4c2b..2c45448 100644 --- a/src/main/java/com/stileeducation/markr/entity/Student.java +++ b/src/main/java/com/stileeducation/markr/entity/Student.java @@ -2,9 +2,7 @@ package com.stileeducation.markr.entity; import jakarta.persistence.*; -import java.util.HashSet; import java.util.Objects; -import java.util.Set; @Entity @Table(name = "students") @@ -23,9 +21,6 @@ public class Student { @Column(name = "student_number", nullable = false, unique = true) private String studentNumber; - @OneToMany(mappedBy = "student", cascade = CascadeType.ALL, orphanRemoval = true) - private Set testResults = new HashSet<>(); - @Transient private boolean created = false; @@ -64,14 +59,6 @@ public class Student { this.studentNumber = studentNumber; } - public Set getTestResults() { - return testResults; - } - - public void setTestResults(Set testResults) { - this.testResults = testResults; - } - public boolean isCreated() { return created; } @@ -96,15 +83,12 @@ public class Student { return Objects.equals(id, student.id) && Objects.equals(firstName, student.firstName) && Objects.equals(lastName, student.lastName) && - Objects.equals(studentNumber, student.studentNumber) && - Objects.equals(testResults, student.testResults) && - Objects.equals(created, student.created) && - Objects.equals(updated, student.updated); + Objects.equals(studentNumber, student.studentNumber); } @Override public int hashCode() { - return Objects.hash(id, firstName, lastName, studentNumber, testResults); + return Objects.hash(id, firstName, lastName, studentNumber); } @Override @@ -114,7 +98,6 @@ public class Student { ", firstName='" + firstName + '\'' + ", lastName='" + lastName + '\'' + ", studentNumber='" + studentNumber + '\'' + - ", testResults=" + testResults + ", created=" + created + ", updated=" + updated + '}'; diff --git a/src/main/java/com/stileeducation/markr/entity/Test.java b/src/main/java/com/stileeducation/markr/entity/Test.java index ff9088e..e258e9a 100644 --- a/src/main/java/com/stileeducation/markr/entity/Test.java +++ b/src/main/java/com/stileeducation/markr/entity/Test.java @@ -2,9 +2,7 @@ package com.stileeducation.markr.entity; import jakarta.persistence.*; -import java.util.HashSet; import java.util.Objects; -import java.util.Set; @Entity @Table(name = "tests") @@ -20,9 +18,6 @@ public class Test { @Column(name = "marks_available", nullable = false) private Integer marksAvailable; - @OneToMany(mappedBy = "test", cascade = CascadeType.ALL, orphanRemoval = true) - private Set testResults = new HashSet<>(); - @Transient private boolean created = false; @@ -53,14 +48,6 @@ public class Test { this.marksAvailable = marksAvailable; } - public Set getTestResults() { - return testResults; - } - - public void setTestResults(Set testResults) { - this.testResults = testResults; - } - public boolean isCreated() { return created; } @@ -84,15 +71,12 @@ public class Test { Test test = (Test) o; return Objects.equals(id, test.id) && Objects.equals(testId, test.testId) && - Objects.equals(marksAvailable, test.marksAvailable) && - Objects.equals(testResults, test.testResults) && - Objects.equals(created, test.created) && - Objects.equals(updated, test.updated); + Objects.equals(marksAvailable, test.marksAvailable); } @Override public int hashCode() { - return Objects.hash(id, testId, marksAvailable, testResults); + return Objects.hash(id, testId, marksAvailable); } @Override @@ -101,7 +85,6 @@ public class Test { "id=" + id + ", testId='" + testId + '\'' + ", marksAvailable=" + marksAvailable + - ", testResults=" + testResults + ", created=" + created + ", updated=" + updated + '}'; diff --git a/src/main/java/com/stileeducation/markr/entity/TestResult.java b/src/main/java/com/stileeducation/markr/entity/TestResult.java index bdbade4..cfee1c7 100644 --- a/src/main/java/com/stileeducation/markr/entity/TestResult.java +++ b/src/main/java/com/stileeducation/markr/entity/TestResult.java @@ -20,8 +20,8 @@ public class TestResult { @JoinColumn(name = "test_id", nullable = false) private Test test; - @Column(name = "marks_awarded", nullable = false) - private Integer marksAwarded; + @Column(name = "marks_obtained", nullable = false) + private Integer marksObtained; @Transient private boolean created = false; @@ -32,11 +32,11 @@ public class TestResult { public TestResult() { } - public TestResult(Long id, Student student, Test test, Integer marksAwarded) { + public TestResult(Long id, Student student, Test test, Integer marksObtained) { this.id = id; this.student = student; this.test = test; - this.marksAwarded = marksAwarded; + this.marksObtained = marksObtained; } public Long getId() { @@ -63,12 +63,12 @@ public class TestResult { this.test = test; } - public Integer getMarksAwarded() { - return marksAwarded; + public Integer getMarksObtained() { + return marksObtained; } - public void setMarksAwarded(Integer marksAwarded) { - this.marksAwarded = marksAwarded; + public void setMarksObtained(Integer marksObtained) { + this.marksObtained = marksObtained; } public boolean isCreated() { @@ -95,14 +95,12 @@ public class TestResult { return Objects.equals(id, that.id) && Objects.equals(student, that.student) && Objects.equals(test, that.test) && - Objects.equals(marksAwarded, that.marksAwarded) && - Objects.equals(created, that.created) && - Objects.equals(updated, that.updated); + Objects.equals(marksObtained, that.marksObtained); } @Override public int hashCode() { - return Objects.hash(id, student, test, marksAwarded); + return Objects.hash(id, student, test, marksObtained); } @Override @@ -111,7 +109,7 @@ public class TestResult { "id=" + id + ", student=" + student + ", test=" + test + - ", marksAwarded=" + marksAwarded + + ", marksObtained=" + marksObtained + ", created=" + created + ", updated=" + updated + '}'; -- cgit v1.2.3 From 7d4d645eb24a30888825f6cdf50cb3eec6ef59f7 Mon Sep 17 00:00:00 2001 From: Szymon Szukalski Date: Fri, 26 Jul 2024 15:07:54 +1000 Subject: Add test cases for multiple submissions Add test cases which exercise the logic for updating the marks available and marks obtained. Update the service method so that they are transactional and reset the transient 'updated' and 'created' flags after reloading entites to ensure accurate update/create reporting. --- .../markr/service/StudentService.java | 8 +++- .../markr/service/TestResultsService.java | 43 ++++++++++++++++------ .../stileeducation/markr/service/TestService.java | 12 ++++-- 3 files changed, 48 insertions(+), 15 deletions(-) (limited to 'src/main/java/com/stileeducation') diff --git a/src/main/java/com/stileeducation/markr/service/StudentService.java b/src/main/java/com/stileeducation/markr/service/StudentService.java index 95ce182..e49f06a 100644 --- a/src/main/java/com/stileeducation/markr/service/StudentService.java +++ b/src/main/java/com/stileeducation/markr/service/StudentService.java @@ -2,6 +2,7 @@ package com.stileeducation.markr.service; import com.stileeducation.markr.entity.Student; import com.stileeducation.markr.repository.StudentRepository; +import jakarta.transaction.Transactional; import org.springframework.stereotype.Service; import java.util.Optional; @@ -15,10 +16,15 @@ public class StudentService { this.studentRepository = studentRepository; } + @Transactional public Student findOrCreateStudent(String firstName, String lastName, String studentNumber) { Optional optionalStudent = studentRepository.findByStudentNumber(studentNumber); if (optionalStudent.isPresent()) { - return optionalStudent.get(); + Student student = optionalStudent.get(); + // Reset transients + student.setCreated(false); + student.setUpdated(false); + return student; } else { Student student = new Student(); student.setFirstName(firstName); diff --git a/src/main/java/com/stileeducation/markr/service/TestResultsService.java b/src/main/java/com/stileeducation/markr/service/TestResultsService.java index a06400e..7823feb 100644 --- a/src/main/java/com/stileeducation/markr/service/TestResultsService.java +++ b/src/main/java/com/stileeducation/markr/service/TestResultsService.java @@ -8,7 +8,10 @@ import com.stileeducation.markr.entity.Student; import com.stileeducation.markr.entity.Test; import com.stileeducation.markr.entity.TestResult; import com.stileeducation.markr.exception.TestNotFoundException; +import com.stileeducation.markr.repository.StudentRepository; +import com.stileeducation.markr.repository.TestRepository; import com.stileeducation.markr.repository.TestResultRepository; +import jakarta.transaction.Transactional; import org.apache.commons.math3.stat.descriptive.moment.StandardDeviation; import org.apache.commons.math3.stat.descriptive.rank.Percentile; import org.springframework.stereotype.Service; @@ -25,19 +28,29 @@ public class TestResultsService { private final TestResultRepository testResultRepository; + private final StudentRepository studentRepository; + + private final TestRepository testRepository; + private final StudentService studentService; private final TestService testService; - public TestResultsService(TestResultRepository testResultRepository, StudentService studentService, TestService testService) { + public TestResultsService(TestResultRepository testResultRepository, + StudentRepository studentRepository, + TestRepository testRepository, + StudentService studentService, + TestService testService) { this.testResultRepository = testResultRepository; + this.studentRepository = studentRepository; + this.testRepository = testRepository; this.studentService = studentService; this.testService = testService; } private static double[] calculatePercentages(List results, double totalMarksAvailable) { return results.stream() - .mapToDouble(result -> (double) result.getMarksAwarded() / totalMarksAvailable * 100) + .mapToDouble(result -> (double) result.getMarksObtained() / totalMarksAvailable * 100) .toArray(); } @@ -55,6 +68,7 @@ public class TestResultsService { return aggregateResponseDTO; } + @Transactional public ImportResponseDTO processTestResults(MCQTestResultsDTO testResults) { ImportResponseDTO.ImportData importData = new ImportResponseDTO.ImportData(); boolean isValid = true; @@ -70,23 +84,29 @@ public class TestResultsService { return createImportResponse(importData, isValid); } - public TestResult findOrCreateTestResult(Student student, Test test, Integer marksAwarded) { + @Transactional + public TestResult findOrCreateTestResult(Student student, Test test, Integer marksObtained) { Optional optionalTestResult = testResultRepository.findByStudentAndTest(student, test); + if (optionalTestResult.isPresent()) { TestResult testResult = optionalTestResult.get(); - if (marksAwarded > testResult.getMarksAwarded()) { - testResult.setMarksAwarded(marksAwarded); + + // Update marks if new marks are higher + if (marksObtained > testResult.getMarksObtained()) { + testResult.setMarksObtained(marksObtained); + testResult.setCreated(false); testResult.setUpdated(true); - testResultRepository.save(testResult); + return testResultRepository.save(testResult); } else { + testResult.setCreated(false); testResult.setUpdated(false); + return testResult; } - return testResult; } else { TestResult testResult = new TestResult(); testResult.setStudent(student); testResult.setTest(test); - testResult.setMarksAwarded(marksAwarded); + testResult.setMarksObtained(marksObtained); testResult.setCreated(true); return testResultRepository.save(testResult); } @@ -186,7 +206,8 @@ public class TestResultsService { dto.setCount(results.size()); } - private void processTestResult(MCQTestResultDTO mcqTestResult, ImportResponseDTO.ImportData importData) { + @Transactional + protected void processTestResult(MCQTestResultDTO mcqTestResult, ImportResponseDTO.ImportData importData) { Student student = studentService .findOrCreateStudent( @@ -228,10 +249,10 @@ public class TestResultsService { if (isValid) { response.setStatus("success"); - response.setMessage("Import operation completed successfully."); + response.setMessage("Import completed successfully"); } else { response.setStatus("failure"); - response.setMessage("Data was invalid or processing failed."); + response.setMessage("Data was invalid or processing failed"); } return response; diff --git a/src/main/java/com/stileeducation/markr/service/TestService.java b/src/main/java/com/stileeducation/markr/service/TestService.java index 0e109ac..f42da1a 100644 --- a/src/main/java/com/stileeducation/markr/service/TestService.java +++ b/src/main/java/com/stileeducation/markr/service/TestService.java @@ -2,6 +2,7 @@ package com.stileeducation.markr.service; import com.stileeducation.markr.entity.Test; import com.stileeducation.markr.repository.TestRepository; +import jakarta.transaction.Transactional; import org.springframework.stereotype.Service; import java.util.Optional; @@ -16,22 +17,27 @@ public class TestService { this.testRepository = testRepository; } + @Transactional public Optional findTest(String testId) { return testRepository.findByTestId(testId); } + @Transactional public Test findOrCreateTest(String testId, Integer marksAvailable) { Optional optionalTest = testRepository.findByTestId(testId); + if (optionalTest.isPresent()) { Test test = optionalTest.get(); - if (test.getMarksAvailable() < marksAvailable) { + if (marksAvailable > test.getMarksAvailable()) { test.setMarksAvailable(marksAvailable); + test.setCreated(false); test.setUpdated(true); - testRepository.save(test); + return testRepository.save(test); } else { + test.setCreated(false); test.setUpdated(false); + return test; } - return test; } else { Test test = new Test(); test.setTestId(testId); -- cgit v1.2.3 From b59d2f0c722d2e21194ed8d66a17be8128ba7b39 Mon Sep 17 00:00:00 2001 From: Szymon Szukalski Date: Fri, 26 Jul 2024 15:15:06 +1000 Subject: Remove unused variables and tidy comments --- .../java/com/stileeducation/markr/dto/AggregateResponseDTO.java | 1 - .../java/com/stileeducation/markr/service/TestResultsService.java | 6 ------ 2 files changed, 7 deletions(-) (limited to 'src/main/java/com/stileeducation') diff --git a/src/main/java/com/stileeducation/markr/dto/AggregateResponseDTO.java b/src/main/java/com/stileeducation/markr/dto/AggregateResponseDTO.java index 6947764..b16a4b8 100644 --- a/src/main/java/com/stileeducation/markr/dto/AggregateResponseDTO.java +++ b/src/main/java/com/stileeducation/markr/dto/AggregateResponseDTO.java @@ -16,7 +16,6 @@ public class AggregateResponseDTO { private double p75 = 0.0; private int count = 0; - // Getters and Setters public double getMean() { return mean; } diff --git a/src/main/java/com/stileeducation/markr/service/TestResultsService.java b/src/main/java/com/stileeducation/markr/service/TestResultsService.java index 7823feb..3c941dd 100644 --- a/src/main/java/com/stileeducation/markr/service/TestResultsService.java +++ b/src/main/java/com/stileeducation/markr/service/TestResultsService.java @@ -28,10 +28,6 @@ public class TestResultsService { private final TestResultRepository testResultRepository; - private final StudentRepository studentRepository; - - private final TestRepository testRepository; - private final StudentService studentService; private final TestService testService; @@ -42,8 +38,6 @@ public class TestResultsService { StudentService studentService, TestService testService) { this.testResultRepository = testResultRepository; - this.studentRepository = studentRepository; - this.testRepository = testRepository; this.studentService = studentService; this.testService = testService; } -- cgit v1.2.3