Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PI-2712 Stream users response to reduce memory overhead #4509

Merged
merged 1 commit into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
}
}
}

Loading