From 95b0b02cd7c6560b8042d11a298ec925d6453e93 Mon Sep 17 00:00:00 2001 From: Topias Heinonen Date: Thu, 31 Aug 2023 16:48:14 +0300 Subject: [PATCH] HAI-1873 Remove explicit produces tags Remove explicit produces and consumes tags where they were application/json. Add response content type checking for all get-requests. --- .../fi/hel/haitaton/hanke/SpringDocITest.kt | 3 ++- .../application/ApplicationControllerITest.kt | 12 ++++++++---- .../hanke/attachment/AttachmentTestUtil.kt | 7 ++----- .../ApplicationAttachmentControllerITest.kt | 9 ++++++--- .../hanke/HankeAttachmentControllerITests.kt | 8 ++++++-- .../fi/hel/haitaton/hanke/HankeController.kt | 15 +++++---------- .../hel/haitaton/hanke/PublicHankeController.kt | 3 +-- .../hanke/permissions/HankeKayttajaController.kt | 3 +-- .../fi/hel/haitaton/hanke/security/AccessRules.kt | 2 ++ .../fi/hel/haitaton/hanke/ControllerTest.kt | 8 ++++++-- 10 files changed, 39 insertions(+), 31 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/SpringDocITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/SpringDocITest.kt index d95562daf..adbbe8084 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/SpringDocITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/SpringDocITest.kt @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc import org.springframework.boot.test.context.SpringBootTest +import org.springframework.http.MediaType import org.springframework.test.context.ActiveProfiles import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content @@ -19,7 +20,7 @@ class SpringdocITest(@Autowired override val mockMvc: MockMvc) : ControllerTest, @Test fun `Should display Swagger UI page`() { - get("/swagger-ui/index.html") + get("/swagger-ui/index.html", resultType = MediaType.TEXT_HTML) .andExpect(status().isOk()) .andExpect(content().string(containsString("Swagger UI"))) } diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationControllerITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationControllerITest.kt index 6d87e95c3..1b525591c 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationControllerITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/application/ApplicationControllerITest.kt @@ -695,7 +695,7 @@ class ApplicationControllerITest(@Autowired override val mockMvc: MockMvc) : Con val id = 11L every { applicationService.getApplicationById(id) } throws ApplicationNotFoundException(id) - get("$BASE_URL/$id/paatos", APPLICATION_PDF, APPLICATION_JSON) + get("$BASE_URL/$id/paatos", accept = arrayOf(APPLICATION_PDF, APPLICATION_JSON)) .andExpect(status().isNotFound) .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(jsonPath("errorCode").value("HAI2001")) @@ -715,7 +715,7 @@ class ApplicationControllerITest(@Autowired override val mockMvc: MockMvc) : Con every { applicationService.downloadDecision(id, USERNAME) } throws ApplicationDecisionNotFoundException("Decision not found in Allu. alluid=23") - get("$BASE_URL/11/paatos", APPLICATION_PDF, APPLICATION_JSON) + get("$BASE_URL/11/paatos", accept = arrayOf(APPLICATION_PDF, APPLICATION_JSON)) .andExpect(status().isNotFound) .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(jsonPath("errorCode").value("HAI2006")) @@ -737,7 +737,11 @@ class ApplicationControllerITest(@Autowired override val mockMvc: MockMvc) : Con every { applicationService.downloadDecision(11, USERNAME) } returns Pair("JS230001", pdfBytes) - get("$BASE_URL/11/paatos", APPLICATION_PDF, APPLICATION_JSON) + get( + "$BASE_URL/11/paatos", + resultType = APPLICATION_PDF, + accept = arrayOf(APPLICATION_PDF, APPLICATION_JSON) + ) .andExpect(status().isOk) .andExpect(header().string("Content-Disposition", "inline; filename=JS230001.pdf")) .andExpect(content().contentType(APPLICATION_PDF)) @@ -757,7 +761,7 @@ class ApplicationControllerITest(@Autowired override val mockMvc: MockMvc) : Con AlluDataFactory.createApplication(id = id, hankeTunnus = HANKE_TUNNUS) every { permissionService.hasPermission(42, USERNAME, VIEW) } returns false - get("$BASE_URL/$id/paatos", APPLICATION_PDF, APPLICATION_JSON) + get("$BASE_URL/$id/paatos", accept = arrayOf(APPLICATION_PDF, APPLICATION_JSON)) .andExpect(status().isNotFound) verify { applicationService.getApplicationById(id) } diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/AttachmentTestUtil.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/AttachmentTestUtil.kt index 5a6775a6a..6a13c4095 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/AttachmentTestUtil.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/AttachmentTestUtil.kt @@ -5,6 +5,7 @@ import fi.hel.haitaton.hanke.attachment.common.FileResult import fi.hel.haitaton.hanke.attachment.common.FileScanData import fi.hel.haitaton.hanke.attachment.common.FileScanResponse import fi.hel.haitaton.hanke.getResourceAsBytes +import fi.hel.haitaton.hanke.hankeError import fi.hel.haitaton.hanke.toJsonString import okhttp3.mockwebserver.MockResponse import org.springframework.http.MediaType.APPLICATION_JSON_VALUE @@ -12,7 +13,6 @@ import org.springframework.http.MediaType.APPLICATION_PDF_VALUE import org.springframework.mock.web.MockMultipartFile import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated import org.springframework.test.web.servlet.ResultActions -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status const val USERNAME = "username" @@ -34,10 +34,7 @@ fun testFile( ) = MockMultipartFile(fileParam, fileName, contentType, data) fun ResultActions.andExpectError(error: HankeError): ResultActions = - andExpect(unauthenticated()) - .andExpect(status().isUnauthorized) - .andExpect(jsonPath("$.errorCode").value(error.errorCode)) - .andExpect(jsonPath("$.errorMessage").value(error.errorMessage)) + andExpect(unauthenticated()).andExpect(status().isUnauthorized).andExpect(hankeError(error)) fun response(data: String): MockResponse = MockResponse() diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/application/ApplicationAttachmentControllerITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/application/ApplicationAttachmentControllerITest.kt index bd5264e5b..4a781efa7 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/application/ApplicationAttachmentControllerITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/application/ApplicationAttachmentControllerITest.kt @@ -46,6 +46,8 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest import org.springframework.context.annotation.Import import org.springframework.http.HttpHeaders.CONTENT_DISPOSITION +import org.springframework.http.MediaType +import org.springframework.http.MediaType.APPLICATION_JSON import org.springframework.http.MediaType.APPLICATION_PDF import org.springframework.http.MediaType.APPLICATION_PDF_VALUE import org.springframework.mock.web.MockMultipartFile @@ -130,7 +132,6 @@ class ApplicationAttachmentControllerITest(@Autowired override val mockMvc: Mock getAttachmentContent(attachmentId = attachmentId) .andExpect(status().isOk) .andExpect(header().string(CONTENT_DISPOSITION, "attachment; filename=$FILE_NAME_PDF")) - .andExpect(content().contentType(APPLICATION_PDF)) .andExpect(content().bytes(dummyData)) verifyOrder { @@ -256,7 +257,7 @@ class ApplicationAttachmentControllerITest(@Autowired override val mockMvc: Mock @WithAnonymousUser fun `unauthorized without authenticated user`() { getMetadataList().andExpectError(HAI0001) - getAttachmentContent().andExpectError(HAI0001) + getAttachmentContent(resultType = APPLICATION_JSON).andExpectError(HAI0001) postAttachment().andExpectError(HAI0001) deleteAttachment().andExpectError(HAI0001) } @@ -267,7 +268,9 @@ class ApplicationAttachmentControllerITest(@Autowired override val mockMvc: Mock private fun getAttachmentContent( applicationId: Long = APPLICATION_ID, attachmentId: UUID = randomUUID(), - ): ResultActions = get("/hakemukset/$applicationId/liitteet/$attachmentId/content") + resultType: MediaType = APPLICATION_PDF, + ): ResultActions = + get("/hakemukset/$applicationId/liitteet/$attachmentId/content", resultType = resultType) private fun postAttachment( applicationId: Long = APPLICATION_ID, diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/hanke/HankeAttachmentControllerITests.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/hanke/HankeAttachmentControllerITests.kt index 61d170121..74c8ec2d3 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/hanke/HankeAttachmentControllerITests.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/hanke/HankeAttachmentControllerITests.kt @@ -35,6 +35,8 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest import org.springframework.context.annotation.Import import org.springframework.http.HttpHeaders.CONTENT_DISPOSITION +import org.springframework.http.MediaType +import org.springframework.http.MediaType.APPLICATION_JSON import org.springframework.http.MediaType.APPLICATION_PDF import org.springframework.http.MediaType.APPLICATION_PDF_VALUE import org.springframework.mock.web.MockMultipartFile @@ -167,7 +169,7 @@ class HankeAttachmentControllerITests(@Autowired override val mockMvc: MockMvc) @WithAnonymousUser fun `unauthorized without authenticated user`() { getMetadataList().andExpectError(HAI0001) - getAttachmentContent().andExpectError(HAI0001) + getAttachmentContent(resultType = APPLICATION_JSON).andExpectError(HAI0001) postAttachment().andExpectError(HAI0001) deleteAttachment().andExpectError(HAI0001) } @@ -178,7 +180,9 @@ class HankeAttachmentControllerITests(@Autowired override val mockMvc: MockMvc) private fun getAttachmentContent( hankeTunnus: String = HANKE_TUNNUS, attachmentId: UUID = randomUUID(), - ): ResultActions = get("/hankkeet/$hankeTunnus/liitteet/$attachmentId/content") + resultType: MediaType = APPLICATION_PDF, + ): ResultActions = + get("/hankkeet/$hankeTunnus/liitteet/$attachmentId/content", resultType = resultType) private fun postAttachment( hankeTunnus: String = HANKE_TUNNUS, diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt index 3a0ca9b36..e93e622f0 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeController.kt @@ -21,7 +21,6 @@ import jakarta.validation.ConstraintViolationException import mu.KotlinLogging import org.springframework.beans.factory.annotation.Autowired import org.springframework.http.HttpStatus -import org.springframework.http.MediaType.APPLICATION_JSON_VALUE import org.springframework.validation.annotation.Validated import org.springframework.web.bind.annotation.DeleteMapping import org.springframework.web.bind.annotation.ExceptionHandler @@ -48,7 +47,7 @@ class HankeController( @Autowired private val featureFlags: FeatureFlags, ) { - @GetMapping("/{hankeTunnus}", produces = [APPLICATION_JSON_VALUE]) + @GetMapping("/{hankeTunnus}") @Operation(summary = "Get hanke", description = "Get specific hanke by hankeTunnus.") @ApiResponses( value = @@ -77,7 +76,7 @@ class HankeController( return hanke } - @GetMapping(produces = [APPLICATION_JSON_VALUE]) + @GetMapping @Operation( summary = "Get hanke list", description = @@ -124,7 +123,7 @@ class HankeController( return hankeList } - @GetMapping("/{hankeTunnus}/hakemukset", produces = [APPLICATION_JSON_VALUE]) + @GetMapping("/{hankeTunnus}/hakemukset") @Operation( summary = "Get hanke applications", description = "Returns list of applications belonging to a given hanke." @@ -148,7 +147,7 @@ class HankeController( } } - @PostMapping(consumes = [APPLICATION_JSON_VALUE], produces = [APPLICATION_JSON_VALUE]) + @PostMapping @Operation( summary = "Create new hanke", description = @@ -194,11 +193,7 @@ When Hanke is created: return createdHanke } - @PutMapping( - "/{hankeTunnus}", - consumes = [APPLICATION_JSON_VALUE], - produces = [APPLICATION_JSON_VALUE] - ) + @PutMapping("/{hankeTunnus}") @Operation( summary = "Update hanke", description = diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PublicHankeController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PublicHankeController.kt index b11dc7354..037fda04e 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PublicHankeController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/PublicHankeController.kt @@ -8,7 +8,6 @@ import io.swagger.v3.oas.annotations.media.Content import io.swagger.v3.oas.annotations.media.Schema import io.swagger.v3.oas.annotations.responses.ApiResponse import io.swagger.v3.oas.annotations.responses.ApiResponses -import org.springframework.http.MediaType.APPLICATION_JSON_VALUE import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController @@ -17,7 +16,7 @@ import org.springframework.web.bind.annotation.RestController @RequestMapping("/public-hankkeet") class PublicHankeController(private val hankeService: HankeService) { - @GetMapping(produces = [APPLICATION_JSON_VALUE]) + @GetMapping @Operation( summary = "Get list of public hanke", description = diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaController.kt index 9a6fd5095..efbab06dd 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaController.kt @@ -16,7 +16,6 @@ import io.swagger.v3.oas.annotations.responses.ApiResponses import io.swagger.v3.oas.annotations.security.SecurityRequirement import mu.KotlinLogging import org.springframework.http.HttpStatus -import org.springframework.http.MediaType.APPLICATION_JSON_VALUE import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable @@ -38,7 +37,7 @@ class HankeKayttajaController( private val featureFlags: FeatureFlags, ) { - @GetMapping("/hankkeet/{hankeTunnus}/kayttajat", produces = [APPLICATION_JSON_VALUE]) + @GetMapping("/hankkeet/{hankeTunnus}/kayttajat") @Operation( summary = "Get Hanke users", description = "Returns a list of users and their Hanke related information." diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/security/AccessRules.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/security/AccessRules.kt index c174f531e..8ae259aad 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/security/AccessRules.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/security/AccessRules.kt @@ -6,6 +6,7 @@ import io.sentry.Sentry import jakarta.servlet.http.HttpServletResponse import mu.KotlinLogging import org.springframework.http.HttpMethod +import org.springframework.http.MediaType.APPLICATION_JSON_VALUE import org.springframework.security.config.annotation.web.builders.HttpSecurity private val logger = KotlinLogging.logger {} @@ -45,6 +46,7 @@ object AccessRules { Sentry.captureException(authenticationException) response.writer.print(OBJECT_MAPPER.writeValueAsString(HankeError.HAI0001)) response.status = HttpServletResponse.SC_UNAUTHORIZED + response.contentType = APPLICATION_JSON_VALUE } } } diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/ControllerTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/ControllerTest.kt index 76ca09ed9..cf04b1541 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/ControllerTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/ControllerTest.kt @@ -6,6 +6,7 @@ import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.ResultActions import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder import org.springframework.test.web.servlet.request.MockMvcRequestBuilders +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content interface ControllerTest { val mockMvc: MockMvc @@ -13,9 +14,12 @@ interface ControllerTest { /** Send a GET request to the given URL. */ fun get( url: String, - vararg accept: MediaType = arrayOf(MediaType.APPLICATION_JSON) + resultType: MediaType = MediaType.APPLICATION_JSON, + accept: Array = arrayOf(MediaType.APPLICATION_JSON) ): ResultActions { - return mockMvc.perform(MockMvcRequestBuilders.get(url).accept(*accept)) + return mockMvc + .perform(MockMvcRequestBuilders.get(url).accept(*accept)) + .andExpect(content().contentType(resultType)) } /**