Skip to content

Commit

Permalink
PI-2712 Stream users response to reduce memory overhead (#4509)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcus-bcl authored Dec 23, 2024
1 parent a357d88 commit 767729b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders
import org.springframework.test.web.servlet.result.MockMvcResultHandlers.print
import org.springframework.test.web.servlet.result.MockMvcResultMatchers
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import uk.gov.justice.digital.hmpps.api.model.sentence.Address
Expand Down Expand Up @@ -36,15 +35,13 @@ class UserLocationIntegrationTest {
@Test
fun `user not found`() {
mockMvc.perform(MockMvcRequestBuilders.get("/user/user/locations").withToken())
.andDo(print())
.andExpect(MockMvcResultMatchers.status().isNotFound)
.andExpect(jsonPath("$.message", equalTo("User with username of user not found")))
}

@Test
fun `get user locations`() {
val response = mockMvc.perform(MockMvcRequestBuilders.get("/user/peter-parker/locations").withToken())
.andDo(print())
.andExpect(MockMvcResultMatchers.status().isOk)
.andReturn().response.contentAsJson<UserOfficeLocation>()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@ package uk.gov.justice.digital.hmpps

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.equalTo
import org.hamcrest.Matchers.equalToIgnoringCase
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.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.MvcResult
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.*
import uk.gov.justice.digital.hmpps.api.model.CaseAccess
import uk.gov.justice.digital.hmpps.api.model.CaseAccessList
import uk.gov.justice.digital.hmpps.api.model.User
Expand All @@ -34,8 +33,11 @@ class UserIntegrationTest {
@Test
fun `get all users`() {
mockMvc.perform(get("/users").withToken())
.andExpect(request().asyncStarted())
.andDo(MvcResult::getAsyncResult)
.andExpect(status().is2xxSuccessful)
.andExpect(jsonPath("$[0].username", equalToIgnoringCase("JoeBloggs")))
.andExpect(content().contentTypeCompatibleWith("application/json"))
.andExpect(content().json("""[{"username":"JoeBloggs","staffCode":"N02ABS1"}]"""))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package uk.gov.justice.digital.hmpps.api.resource

import io.swagger.v3.oas.annotations.Operation
import jakarta.validation.constraints.Size
import org.springframework.http.MediaType.APPLICATION_JSON
import org.springframework.http.ResponseEntity
import org.springframework.security.access.prepost.PreAuthorize
import org.springframework.web.bind.annotation.*
import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody
import uk.gov.justice.digital.hmpps.service.UserAccessService
import uk.gov.justice.digital.hmpps.service.UserService

Expand Down Expand Up @@ -34,7 +37,9 @@ class UserResource(

@GetMapping("/users")
@Operation(summary = "Returns all users with the Delius `MAABT001` role")
fun allUsers() = userService.findAllUsersWithRole()
fun allUsers() = ResponseEntity.ok()
.contentType(APPLICATION_JSON)
.body(StreamingResponseBody { userService.writeAllUsersWithRole(it) })

@GetMapping("/person/{crn}/limited-access/all")
@Operation(summary = "Returns all limited access information (restrictions and exclusions) for a Delius CRN")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.springframework.data.jpa.repository.JpaRepository
import org.springframework.data.jpa.repository.Query
import uk.gov.justice.digital.hmpps.exception.NotFoundException
import java.time.LocalDate
import java.util.stream.Stream

interface StaffRepository : JpaRepository<StaffRecord, Long> {
@EntityGraph(attributePaths = ["grade.dataset", "user"])
Expand All @@ -23,7 +24,7 @@ interface StaffRepository : JpaRepository<StaffRecord, Long> {
fun findStaffForUsernamesIn(
usernames: List<String>,
usernamesUppercase: List<String> = usernames.map { it.uppercase() }
): List<StaffWithUser>
): Stream<StaffWithUser>

@EntityGraph(attributePaths = ["grade.dataset"])
fun findByCode(code: String): Staff?
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package uk.gov.justice.digital.hmpps.service

import com.fasterxml.jackson.databind.ObjectMapper
import jakarta.transaction.Transactional
import org.springframework.stereotype.Service
import uk.gov.justice.digital.hmpps.api.model.CaseAccessList
import uk.gov.justice.digital.hmpps.api.model.User
Expand All @@ -8,20 +10,25 @@ import uk.gov.justice.digital.hmpps.integrations.delius.person.PersonRepository
import uk.gov.justice.digital.hmpps.integrations.delius.provider.StaffRepository
import uk.gov.justice.digital.hmpps.integrations.delius.user.ExclusionRepository
import uk.gov.justice.digital.hmpps.integrations.delius.user.RestrictionRepository
import java.io.OutputStream
import java.util.stream.Stream
import kotlin.streams.asSequence

@Service
@Transactional
class UserService(
private val personRepository: PersonRepository,
private val exclusionRepository: ExclusionRepository,
private val restrictionRepository: RestrictionRepository,
private val staffRepository: StaffRepository,
private val ldapService: LdapService,
private val objectMapper: ObjectMapper,
) {
fun getAllAccessLimitations(crn: String, staffCodesFilter: List<String>? = null): CaseAccessList {
val person = personRepository.findByCrnAndSoftDeletedFalse(crn) ?: throw NotFoundException("Person", "crn", crn)
val exclusions = exclusionRepository.findByPersonId(person.id).map { it.user.username }
val restrictions = restrictionRepository.findByPersonId(person.id).map { it.user.username }
val staffCodes = staffRepository.findStaffForUsernamesIn(exclusions + restrictions)
val staffCodes = staffRepository.findStaffForUsernamesIn(exclusions + restrictions).asSequence()
.associate { it.user?.username to it.code }
return CaseAccessList(
crn = crn,
Expand All @@ -34,10 +41,20 @@ class UserService(
)
}

fun findAllUsersWithRole(role: String = "MAABT001"): List<User> {
fun findAllUsersWithRole(role: String = "MAABT001"): Stream<User> {
val usernames = ldapService.findAllUsersWithRole(role)
val staff = staffRepository.findStaffForUsernamesIn(usernames)
return staff.map { User(it.user!!.username, it.code) }
}

fun writeAllUsersWithRole(outputStream: OutputStream) {
objectMapper.factory.createGenerator(outputStream).use { json ->
json.writeStartArray()
findAllUsersWithRole().use { users ->
users.forEach { user -> json.writeObject(user) }
}
json.writeEndArray()
}
}
}

0 comments on commit 767729b

Please sign in to comment.