Skip to content

Commit

Permalink
HAI-1873 Remove explicit produces tags
Browse files Browse the repository at this point in the history
Remove explicit produces and consumes tags where they were
application/json.

Add response content type checking for all get-requests.
  • Loading branch information
corvidian committed Aug 31, 2023
1 parent bff8d50 commit 95b0b02
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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"))
Expand All @@ -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))
Expand All @@ -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) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ 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
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"
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -77,7 +76,7 @@ class HankeController(
return hanke
}

@GetMapping(produces = [APPLICATION_JSON_VALUE])
@GetMapping
@Operation(
summary = "Get hanke list",
description =
Expand Down Expand Up @@ -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."
Expand All @@ -148,7 +147,7 @@ class HankeController(
}
}

@PostMapping(consumes = [APPLICATION_JSON_VALUE], produces = [APPLICATION_JSON_VALUE])
@PostMapping
@Operation(
summary = "Create new hanke",
description =
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down Expand Up @@ -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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@ 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

/** 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<MediaType> = 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))
}

/**
Expand Down

0 comments on commit 95b0b02

Please sign in to comment.