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

#2406 Add lastVisit field and set it when user logs in #2527

Merged
merged 13 commits into from
Oct 27, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,16 @@ class FirebaseTokenAuthenticationFilter(
filterChain: FilterChain
) {
verifyToken(request)
markVisit()
filterChain.doFilter(request, response)
}

private fun markVisit() {
if (SecurityContextHolder.getContext().authentication != null) {
userAccountService.markVisitForCurrentUser()
}
ElenaSpb marked this conversation as resolved.
Show resolved Hide resolved
}

private fun verifyToken(request: HttpServletRequest) {
val token: String? = tokenHelperUtils.getBearerToken(request)
try {
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/com/epam/brn/dto/UserAccountDto.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ data class UserAccountDto(
var active: Boolean = true,
val created: LocalDateTime = LocalDateTime.now(ZoneOffset.UTC),
val changed: LocalDateTime = LocalDateTime.now(ZoneOffset.UTC),
val lastVisit: LocalDateTime? = null,
var avatar: String? = null,
val photo: String? = null,
val description: String? = null,
Expand Down
3 changes: 3 additions & 0 deletions src/main/kotlin/com/epam/brn/model/UserAccount.kt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ data class UserAccount(
@LastModifiedBy
@Column(name = "changed_by")
var changedBy: String = "",
@Column(name = "last_visit")
var lastVisit: LocalDateTime? = null,
var avatar: String? = null,
var photo: String? = null,
var description: String? = null,
Expand Down Expand Up @@ -81,6 +83,7 @@ data class UserAccount(
gender = gender?.let { BrnGender.valueOf(it) },
created = created,
changed = changed,
lastVisit = lastVisit,
avatar = avatar,
photo = photo,
description = description,
Expand Down
11 changes: 11 additions & 0 deletions src/main/kotlin/com/epam/brn/repo/UserAccountRepository.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import com.epam.brn.model.UserAccount
import org.springframework.data.domain.Page
import org.springframework.data.domain.Pageable
import org.springframework.data.jpa.repository.JpaRepository
import org.springframework.data.jpa.repository.Modifying
import org.springframework.data.jpa.repository.Query
import org.springframework.stereotype.Repository
import org.springframework.transaction.annotation.Transactional
import java.time.LocalDateTime
import java.util.Optional

@Repository
Expand Down Expand Up @@ -50,4 +53,12 @@ interface UserAccountRepository : JpaRepository<UserAccount, Long> {
left JOIN FETCH u.headphones where roles.name = :roleName"""
)
fun findUsersAccountsByRole(roleName: String): List<UserAccount>

@Transactional
@Modifying
@Query(
"""update UserAccount u SET u.lastVisit = :lastVisit
where u.email = :email"""
)
fun updateLastVisitByEmail(email: String, lastVisit: LocalDateTime)
}
2 changes: 2 additions & 0 deletions src/main/kotlin/com/epam/brn/service/UserAccountService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ interface UserAccountService {
fun updateDoctorForPatient(userId: Long, doctorId: Long): UserAccount
fun removeDoctorFromPatient(userId: Long): UserAccount
fun getPatientsForDoctor(doctorId: Long): List<UserAccountDto>

fun markVisitForCurrentUser()
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.epam.brn.model.UserAccount
import com.epam.brn.repo.UserAccountRepository
import com.epam.brn.service.HeadphonesService
import com.epam.brn.service.RoleService
import com.epam.brn.service.TimeService
import com.epam.brn.service.UserAccountService
import com.google.firebase.auth.UserRecord
import org.springframework.data.domain.Pageable
Expand All @@ -24,7 +25,8 @@ import java.security.Principal
class UserAccountServiceImpl(
private val userAccountRepository: UserAccountRepository,
private val roleService: RoleService,
private val headphonesService: HeadphonesService
private val headphonesService: HeadphonesService,
private val timeService: TimeService
) : UserAccountService {

override fun findUserByEmail(email: String): UserAccountDto =
Expand Down Expand Up @@ -67,12 +69,17 @@ class UserAccountServiceImpl(
.toSet()

override fun getCurrentUser(): UserAccount {
val authentication = SecurityContextHolder.getContext().authentication
val email = authentication.name ?: getNameFromPrincipals(authentication)
val email = getCurrentUserEmail()
return userAccountRepository.findUserAccountByEmail(email)
.orElseThrow { EntityNotFoundException("No user was found for email=$email") }
}

override fun markVisitForCurrentUser() {
val email = getCurrentUserEmail()
val lastVisit = timeService.now()
return userAccountRepository.updateLastVisitByEmail(email, lastVisit)
}

override fun getCurrentUserDto(): UserAccountDto =
getCurrentUser().toDto()

Expand Down Expand Up @@ -149,6 +156,11 @@ class UserAccountServiceImpl(
return this
}

private fun getCurrentUserEmail(): String {
val authentication = SecurityContextHolder.getContext().authentication
return authentication.name ?: getNameFromPrincipals(authentication)
}

private fun getNameFromPrincipals(authentication: Authentication): String {
val principal = authentication.principal
if (principal is UserDetails)
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/db/migration/V220230119_2406.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
alter table user_account add column if not exists last_visit timestamp;
ElenaSpb marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.mockk.every
import io.mockk.impl.annotations.InjectMockKs
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.justRun
import io.mockk.mockk
import io.mockk.verify
import org.amshove.kluent.shouldBeInstanceOf
Expand Down Expand Up @@ -85,6 +86,8 @@ internal class FirebaseTokenAuthenticationFilterTest {
every { firebaseAuth.verifyIdToken(token, true) } returns firebaseTokenMock
every { firebaseTokenMock.email } returns email
every { brainUpUserDetailsService.loadUserByUsername(email) } returns customUserDetailsMock
justRun { userAccountService.markVisitForCurrentUser() }

// WHEN
firebaseTokenAuthenticationFilter.doFilter(request, response, filterChain)
// THEN
Expand All @@ -97,6 +100,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
verify(exactly = 1) { tokenHelperUtils.getBearerToken(request) }
verify(exactly = 1) { firebaseAuth.verifyIdToken(token, true) }
verify(exactly = 1) { brainUpUserDetailsService.loadUserByUsername(email) }
verify(exactly = 1) { userAccountService.markVisitForCurrentUser() }
verify(exactly = 0) { firebaseUserService.getUserByUuid(any()) }
verify(exactly = 0) { userAccountService.createUser(any()) }
}
Expand Down Expand Up @@ -124,6 +128,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
gender = null,
name = fullName
)
justRun { userAccountService.markVisitForCurrentUser() }

// WHEN
firebaseTokenAuthenticationFilter.doFilter(requestMock, responseMock, filterChain)
Expand All @@ -139,6 +144,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
verify(exactly = 2) { brainUpUserDetailsService.loadUserByUsername(email) }
verify(exactly = 1) { firebaseUserService.getUserByUuid(uuid) }
verify(exactly = 1) { userAccountService.createUser(any()) }
verify(exactly = 1) { userAccountService.markVisitForCurrentUser() }
}

@Test
Expand All @@ -163,6 +169,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
verify(exactly = 0) { brainUpUserDetailsService.loadUserByUsername(any()) }
verify(exactly = 0) { firebaseUserService.getUserByUuid(any()) }
verify(exactly = 0) { userAccountService.createUser(any()) }
verify(exactly = 0) { userAccountService.markVisitForCurrentUser() }
}

@Test
Expand All @@ -187,6 +194,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
verify(exactly = 0) { brainUpUserDetailsService.loadUserByUsername(any()) }
verify(exactly = 0) { firebaseUserService.getUserByUuid(any()) }
verify(exactly = 0) { userAccountService.createUser(any()) }
verify(exactly = 0) { userAccountService.markVisitForCurrentUser() }
}

@Test
Expand Down Expand Up @@ -216,6 +224,7 @@ internal class FirebaseTokenAuthenticationFilterTest {
verify(exactly = 1) { firebaseAuth.verifyIdToken(tokenMock, true) }
verify(exactly = 1) { brainUpUserDetailsService.loadUserByUsername(email) }
verify(exactly = 1) { firebaseUserService.getUserByUuid(uuid) }
verify(exactly = 0) { userAccountService.markVisitForCurrentUser() }
verify(exactly = 0) { userAccountService.createUser(any()) }
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.epam.brn.integration.repo

import com.epam.brn.model.UserAccount
import com.epam.brn.repo.UserAccountRepository
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Assertions.assertDoesNotThrow
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest
import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager
import java.time.LocalDateTime
import java.time.temporal.ChronoUnit

@DataJpaTest
@Tag("integration-test")
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@DisplayName("UserAccountRepository tests")
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class UserAccountRepositoryTest {

@Autowired
private lateinit var repository: UserAccountRepository

@Autowired
private lateinit var testEntityManager: TestEntityManager

@Test
fun `should update lastVisit date of user`() {
// GIVEN
val yesterday = LocalDateTime.now().minusDays(1).truncatedTo(ChronoUnit.MILLIS)
val today = LocalDateTime.now().truncatedTo(ChronoUnit.MILLIS)

val email = "[email protected]"
val user = UserAccount(
email = email,
fullName = "John Doe",
lastVisit = yesterday
)
val savedUser = testEntityManager.persistAndFlush(user)

// WHEN
repository.updateLastVisitByEmail(email, today)

testEntityManager.flush()
testEntityManager.clear()

// THEN
val retrievedUser = testEntityManager.find(UserAccount::class.java, savedUser.id)
assertThat(retrievedUser).isNotNull
val actualLastVisit = retrievedUser.lastVisit?.truncatedTo(ChronoUnit.MILLIS)
assertThat(actualLastVisit).isEqualTo(today)
}

@Test
fun `should not throw error when user doesn't exist`() {
// GIVEN
val today = LocalDateTime.now().truncatedTo(ChronoUnit.MILLIS)
val email = "[email protected]"

// WHEN & THEN
assertDoesNotThrow {
repository.updateLastVisitByEmail(email, today)
}
}
}
25 changes: 25 additions & 0 deletions src/test/kotlin/com/epam/brn/service/UserAccountServiceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import com.epam.brn.service.impl.UserAccountServiceImpl
import com.google.firebase.auth.UserRecord
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.matchers.shouldBe
import io.mockk.Runs
import io.mockk.every
import io.mockk.impl.annotations.InjectMockKs
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.mockkClass
import io.mockk.just
import io.mockk.slot
import io.mockk.verify
import org.apache.commons.lang3.math.NumberUtils
Expand All @@ -33,6 +35,7 @@ import org.springframework.security.core.Authentication
import org.springframework.security.core.context.SecurityContext
import org.springframework.security.core.context.SecurityContextHolder
import java.time.LocalDateTime
import java.time.ZoneOffset
import java.util.Optional
import kotlin.test.assertFailsWith
import kotlin.test.assertNotNull
Expand Down Expand Up @@ -78,6 +81,9 @@ internal class UserAccountServiceTest {
@MockK
lateinit var pageable: Pageable

@MockK
lateinit var timeService: TimeService

@Nested
@DisplayName("Tests for getting users")
inner class GetUserAccounts {
Expand Down Expand Up @@ -295,6 +301,25 @@ internal class UserAccountServiceTest {
assertThat(userForSave.fullName).isEqualTo("newName")
assertThat(userForSave.id).isEqualTo(userAccount.id)
}

@Test
fun `should mark visit for current session user`() {
// GIVEN
val email = "[email protected]"
val now = LocalDateTime.now(ZoneOffset.UTC)

SecurityContextHolder.setContext(securityContext)
every { securityContext.authentication } returns authentication
every { authentication.name } returns email
every { timeService.now() } returns now
every { userAccountRepository.updateLastVisitByEmail(email, now) } just Runs

// WHEN
userAccountService.markVisitForCurrentUser()

// THEN
verify { userAccountRepository.updateLastVisitByEmail(email, now) }
}
}

@Nested
Expand Down
Loading