From cb6c15146e856f341aad94ed61812b583ade9478 Mon Sep 17 00:00:00 2001 From: Nathan Cannon Date: Mon, 4 Mar 2019 10:59:32 +0000 Subject: [PATCH] Some routes now return actual HTTP error codes --- .../controller/GroupSessionController.kt | 18 ++++++++++----- .../controller/ObservationsController.kt | 23 +++++++++++-------- .../ObservationsControllerTest.kt | 3 ++- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/GroupSessionController.kt b/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/GroupSessionController.kt index 156caa7..6ecdf09 100644 --- a/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/GroupSessionController.kt +++ b/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/GroupSessionController.kt @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.core.env.Environment import org.springframework.core.env.get +import org.springframework.http.HttpStatus import org.springframework.messaging.simp.SimpMessagingTemplate import org.springframework.web.bind.annotation.CrossOrigin import org.springframework.web.bind.annotation.GetMapping @@ -14,6 +15,7 @@ import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController +import org.springframework.web.server.ResponseStatusException import uk.co.neviyn.observationdatabase.GroupObservation import uk.co.neviyn.observationdatabase.GroupObservationInit import uk.co.neviyn.observationdatabase.GroupSessionManager @@ -56,11 +58,11 @@ class GroupSessionController { val tutors = tutorRepository.findAllById(initData.tutors).toSet() if (!site.isPresent) { logger.info("Attempted to add Observation without a site.") - return mapOf("error" to "Site required") + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Site required") } if (tutors.isEmpty() || tutors.size != initData.tutors.size) { logger.info("Attempted to add Observation without a tutor") - return mapOf("error" to "Tutor(s) required") + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Tutor required") } val sessionId = GroupSessionManager.startNewSession(site.get(), tutors, initData.type, initData.scenarioTitles) return getConnectionDetails().plus("id" to sessionId.toString()) @@ -76,7 +78,7 @@ class GroupSessionController { return getConnectionDetails().plus(mapOf("id" to GroupSessionManager.sessionId.toString(), "scenarios" to GroupSessionManager.asScenarioView())) } logger.info("Tried to recover a session but no session is active") - return mapOf("error" to "No session currently active") + throw ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "No group session is currently running") } /** @@ -87,8 +89,12 @@ class GroupSessionController { if (GroupSessionManager.isValid(id)) { return mapOf("titles" to GroupSessionManager.scenarioTitles!!) } + if (GroupSessionManager.isValid()) { + logger.warn("Group observation requested with id $id but id is currently ${GroupSessionManager.sessionId})") + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Session ID incorrect") + } logger.warn("Group observation requested with id $id but there is no valid session") - return mapOf("error" to "no valid session") + throw ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "No group session is currently running") } /** @@ -151,9 +157,9 @@ class GroupSessionController { websocketMessenger.convertAndSend("/ws/status", mapOf("status" to "complete")) return mapOf("success" to "The submission was successfully completed.") } else if (!GroupSessionManager.dataComplete()) { - return mapOf("error" to "Data is incomplete") + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Data is incomplete") } - return mapOf("error" to "Session was not valid") + throw ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "No valid session") } /** diff --git a/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/ObservationsController.kt b/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/ObservationsController.kt index e237f80..f9ebf7b 100644 --- a/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/ObservationsController.kt +++ b/backend/src/main/kotlin/uk/co/neviyn/observationdatabase/controller/ObservationsController.kt @@ -5,6 +5,7 @@ import org.slf4j.Logger import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.cache.annotation.Cacheable +import org.springframework.http.HttpStatus import org.springframework.web.bind.annotation.CrossOrigin import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable @@ -12,6 +13,7 @@ import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController +import org.springframework.web.server.ResponseStatusException import uk.co.neviyn.observationdatabase.AfiPieChart import uk.co.neviyn.observationdatabase.AfiPieChartDataset import uk.co.neviyn.observationdatabase.AverageData @@ -73,7 +75,7 @@ class ObservationsController { val site = siteRepository.findById(id) if (site.isPresent) return site.map { site1 -> site1.tutors.map { NameValue(it.name, it.id) } }.get() - return null + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "No Site with given ID") } /** @@ -89,14 +91,15 @@ class ObservationsController { @PostMapping("/tutor") fun addTutor(@Valid @RequestBody newTutor: NewTutor): NameValue? { logger.debug("Attempting to add tutor\n$newTutor") - var nameValue: NameValue? = null - siteRepository.findById(newTutor.siteId).ifPresent { - val tutor = tutorRepository.save(Tutor(name = newTutor.name, site = it)) - it.tutors.add(tutor) - siteRepository.save(it) - nameValue = NameValue(tutor.name, tutor.id) + val site = siteRepository.findById(newTutor.siteId) + if (site.isPresent) { + val data = site.get() + val tutor = tutorRepository.save(Tutor(name = newTutor.name, site = data)) + data.tutors.add(tutor) + siteRepository.save(data) + return NameValue(tutor.name, tutor.id) } - return nameValue + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Site invalid") } /** @@ -123,11 +126,11 @@ class ObservationsController { val tutors = tutorRepository.findAllById(newObservation.tutors).toSet() if (!site.isPresent) { logger.info("Attempted to add Observation without a site.") - return null + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Observation requires a Site") } if (tutors.isEmpty() || tutors.size != newObservation.tutors.size) { logger.info("Attempted to add Observation without a tutor") - return null + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Observation requires at least one Tutor") } var observation = Observation( site = site.get(), diff --git a/backend/src/test/kotlin/uk/co/neviyn/observationdatabase/ObservationsControllerTest.kt b/backend/src/test/kotlin/uk/co/neviyn/observationdatabase/ObservationsControllerTest.kt index cba18de..503d816 100644 --- a/backend/src/test/kotlin/uk/co/neviyn/observationdatabase/ObservationsControllerTest.kt +++ b/backend/src/test/kotlin/uk/co/neviyn/observationdatabase/ObservationsControllerTest.kt @@ -7,6 +7,7 @@ import org.junit.runner.RunWith import org.mockito.* import org.mockito.Mockito.* import org.mockito.junit.MockitoJUnitRunner +import org.springframework.web.server.ResponseStatusException import uk.co.neviyn.observationdatabase.controller.ObservationsController import java.util.* @@ -58,7 +59,7 @@ class ObservationsControllerTest { assertTrue(result.contains(NameValue("Bar", 2))) } - @Test + @Test(expected = ResponseStatusException::class) fun testGetTutorsForNullSite() { doReturn(Optional.ofNullable(null)).`when`(siteRepository).findById(1) val result: List? = controller.getTutorsForSite(1)