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 +- .../controller/TestResultsControllerTest.java | 249 ++++++++++++++++++--- .../markr/service/TestResultsServiceTest.java | 12 +- 5 files changed, 267 insertions(+), 57 deletions(-) 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); diff --git a/src/test/java/com/stileeducation/markr/controller/TestResultsControllerTest.java b/src/test/java/com/stileeducation/markr/controller/TestResultsControllerTest.java index efb20d6..cd1f3cb 100644 --- a/src/test/java/com/stileeducation/markr/controller/TestResultsControllerTest.java +++ b/src/test/java/com/stileeducation/markr/controller/TestResultsControllerTest.java @@ -4,6 +4,8 @@ import com.stileeducation.markr.MarkrApplication; import com.stileeducation.markr.converter.XmlMarkrMessageConverter; import com.stileeducation.markr.dto.ImportResponseDTO; import com.stileeducation.markr.dto.MCQTestResultsDTO; +import com.stileeducation.markr.repository.TestResultRepository; +import com.stileeducation.markr.service.TestService; import jakarta.xml.bind.JAXBContext; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.Marshaller; @@ -33,6 +35,11 @@ public class TestResultsControllerTest { @Autowired private TestRestTemplate restTemplate; + @Autowired + private TestResultRepository testResultRepository; + @Autowired + private TestService testService; + @BeforeAll static void setup() throws IOException { validPayload = new String(new ClassPathResource("sample-results.xml").getInputStream().readAllBytes(), UTF_8); @@ -95,11 +102,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - + - Alysander - 002299 - 9863 + Szukalski + 001122 + 2024001 @@ -123,11 +130,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ + + Szymon - 002299 - 9863 + 001122 + 2024001 @@ -151,11 +158,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander + + Szymon + Szukalski - 9863 + 2024001 @@ -179,10 +186,10 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander - 002299 + + Szymon + Szukalski + 2024001 @@ -207,11 +214,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander - 002299 - 9863 + + Szymon + Szukalski + 001122 + 2024001 """; @@ -234,11 +241,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander - 002299 - 9863 + + Szymon + Szukalski + 001122 + 2024001 @@ -262,11 +269,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander - 002299 - 9863 + + Szymon + Szukalski + 001122 + 2024001 @@ -290,11 +297,11 @@ public class TestResultsControllerTest { // Given String invalidPayload = """ - - KJ - Alysander - 002299 - 9863 + + Szymon + Szukalski + 001122 + 2024001 + + John + Doe + 002233 + 202402 + + + + """; + + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(XmlMarkrMessageConverter.MEDIA_TYPE); + HttpEntity entity = new HttpEntity<>(validPayload, headers); + + // When + ResponseEntity response = restTemplate.postForEntity(IMPORT_ENDPOINT, entity, ImportResponseDTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().getStatus()).isEqualTo("success"); + assertThat(response.getBody().getMessage()).isEqualTo("Import completed successfully"); + assertThat(response.getBody().getData().getStudentsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getStudentsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestResultsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestResultsUpdated()).isEqualTo(0); + } + + + @Test + void testSubsequentSubmitReportsNoCreatedOrUpdatedEntities() throws Exception { + // Given + String validPayload = """ + + + John + Doe + 003344 + 202403 + + + + John + Doe + 003344 + 202403 + + + + """; + + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(XmlMarkrMessageConverter.MEDIA_TYPE); + HttpEntity entity = new HttpEntity<>(validPayload, headers); + + // Initial Submit + restTemplate.postForEntity(IMPORT_ENDPOINT, entity, ImportResponseDTO.class); + + // When + ResponseEntity response = restTemplate.postForEntity(IMPORT_ENDPOINT, entity, ImportResponseDTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().getStatus()).isEqualTo("success"); + assertThat(response.getBody().getMessage()).isEqualTo("Import completed successfully"); + assertThat(response.getBody().getData().getStudentsCreated()).isEqualTo(0); + assertThat(response.getBody().getData().getStudentsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestsCreated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestResultsCreated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestResultsUpdated()).isEqualTo(0); + } + + + @Test + void testMarksAvailableAndObtainedAreUpdated() throws Exception { + // Given + String payload = """ + + + John + Doe + 004455 + 202404 + + + + John + Doe + 004455 + 202404 + + + + """; + + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(XmlMarkrMessageConverter.MEDIA_TYPE); + HttpEntity entity = new HttpEntity<>(payload, headers); + + // When + ResponseEntity response = restTemplate.postForEntity(IMPORT_ENDPOINT, entity, ImportResponseDTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().getStatus()).isEqualTo("success"); + assertThat(response.getBody().getMessage()).isEqualTo("Import completed successfully"); + + assertThat(response.getBody().getData().getStudentsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getStudentsUpdated()).isEqualTo(0); + + assertThat(response.getBody().getData().getTestsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestsUpdated()).isEqualTo(1); + + assertThat(response.getBody().getData().getTestResultsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestResultsUpdated()).isEqualTo(1); + } + + @Test + void testMarksAvailableAndObtainedAreNotUpdatedWhenFirstSubmissionIsHigher() throws Exception { + // Given + String payload = """ + + + John + Doe + 005566 + 202405 + + + + John + Doe + 005566 + 202405 + + + + """; + + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(XmlMarkrMessageConverter.MEDIA_TYPE); + HttpEntity entity = new HttpEntity<>(payload, headers); + + // When + ResponseEntity response = restTemplate.postForEntity(IMPORT_ENDPOINT, entity, ImportResponseDTO.class); + + // Then + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().getStatus()).isEqualTo("success"); + assertThat(response.getBody().getMessage()).isEqualTo("Import completed successfully"); + assertThat(response.getBody().getData().getStudentsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getStudentsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestsUpdated()).isEqualTo(0); + assertThat(response.getBody().getData().getTestResultsCreated()).isEqualTo(1); + assertThat(response.getBody().getData().getTestResultsUpdated()).isEqualTo(0); + } + } \ No newline at end of file diff --git a/src/test/java/com/stileeducation/markr/service/TestResultsServiceTest.java b/src/test/java/com/stileeducation/markr/service/TestResultsServiceTest.java index ad4e476..f9a8871 100644 --- a/src/test/java/com/stileeducation/markr/service/TestResultsServiceTest.java +++ b/src/test/java/com/stileeducation/markr/service/TestResultsServiceTest.java @@ -89,37 +89,37 @@ class TestResultsServiceTest { testResult1 = new TestResultBuilder() .withStudent(student1) .withTest(test1) - .withMarksAwarded(10) + .withMarksObtained(10) .build(); testResult2 = new TestResultBuilder() .withStudent(student2) .withTest(test1) - .withMarksAwarded(20) + .withMarksObtained(20) .build(); testResult3 = new TestResultBuilder() .withStudent(student3) .withTest(test1) - .withMarksAwarded(40) + .withMarksObtained(40) .build(); testResult4 = new TestResultBuilder() .withStudent(student4) .withTest(test1) - .withMarksAwarded(50) + .withMarksObtained(50) .build(); testResult5 = new TestResultBuilder() .withStudent(student3) .withTest(test1) - .withMarksAwarded(70) + .withMarksObtained(70) .build(); testResult6 = new TestResultBuilder() .withStudent(student4) .withTest(test1) - .withMarksAwarded(80) + .withMarksObtained(80) .build(); test1Results = List.of(testResult1, testResult2, testResult3, testResult4, testResult5, testResult6); -- cgit v1.2.3