Skip to content

Feature/user created at #498

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.

Sign up for GitHub

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

Merged
birdieHyun merged 3 commits into from
Jun 1, 2025
Merged

Feature/user created at #498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f08582
feat: user time entity 추가
birdieHyun Jun 1, 2025
c06787d
fix: user login 버그 수정
birdieHyun Jun 1, 2025
376d307
fix: 테스트 수정
birdieHyun Jun 1, 2025
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
2 changes: 2 additions & 0 deletions src/main/kotlin/upbrella/be/BeApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package upbrella.be

import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.runApplication
import org.springframework.data.jpa.repository.config.EnableJpaAuditing

@EnableJpaAuditing
@SpringBootApplication
class BeApplication

Expand Down
4 changes: 2 additions & 2 deletions src/main/kotlin/upbrella/be/user/dto/response/AllUsersInfoResponse.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import upbrella.be.user.entity.User

data class AllUsersInfoResponse(val users: List<SingleUserInfoResponse>) {
companion object {
fun fromUsers(users: List<User?>): AllUsersInfoResponse {
fun fromUsers(users: List<User>): AllUsersInfoResponse {
return AllUsersInfoResponse(
users = users.map { SingleUserInfoResponse.fromUser(it!!) }
users = users.map { SingleUserInfoResponse.fromUser(it) }
)
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package upbrella.be.user.dto.response

import upbrella.be.user.entity.User
import java.time.LocalDateTime

data class SingleUserInfoResponse(
val id: Long,
Expand All @@ -9,7 +10,10 @@ data class SingleUserInfoResponse(
val email: String,
val bank: String?,
val accountNumber: String?,
val adminStatus: Boolean
val adminStatus: Boolean,
val createdAt: LocalDateTime,
val updatedAt: LocalDateTime,
val deletedAt: LocalDateTime? = null
) {
companion object {
fun fromUser(user: User): SingleUserInfoResponse {
Expand All @@ -20,7 +24,10 @@ data class SingleUserInfoResponse(
email = user.email,
bank = user.bank,
accountNumber = user.accountNumber,
adminStatus = user.adminStatus
adminStatus = user.adminStatus,
createdAt = user.createdAt!!,
updatedAt = user.updatedAt!!,
deletedAt = user.deletedAt
Copy link

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Potential null pointer risk with non-null assertions.

The non-null assertions (!!) on createdAt and updatedAt could cause runtime exceptions if JPA auditing fails to populate these fields. Consider using safe calls or ensuring these fields are properly initialized.

🏁 Script executed:

#!/bin/bash
# Description: Verify BaseTimeEntity fields are properly initialized by JPA auditing
# Expected: Confirm that @CreatedDate and @LastModifiedDate annotations ensure non-null values

# Check for any existing null checks or initialization in User entity
rg -A 5 -B 5 "createdAt|updatedAt" src/main/kotlin/upbrella/be/user/entity/User.kt

# Look for JPA auditing configuration
rg -A 3 -B 3 "@EnableJpaAuditing|AuditingEntityListener" --type kotlin

Length of output: 1855

🏁 Script executed:

#!/bin/bash
# Display the BaseTimeEntity class to inspect field definitions
sed -n '1,200p' src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt

Length of output: 792

Handle potential null values for createdAt and updatedAt
The BaseTimeEntity declares both createdAt and updatedAt as LocalDateTime? and relies on JPA auditing to populate them. While @EnableJpaAuditing with @CreatedDate/@LastModifiedDate generally sets these on persist, they can still be null (e.g., before first save, in tests, or if auditing isn’t triggered). Using !! here will lead to an NPE.

• File: src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt
Replace these lines:

- createdAt = user.createdAt!!,
- updatedAt = user.updatedAt!!,

With a null-safe mapping, for example:

createdAt = user.createdAt 
    ?: throw IllegalStateException("createdAt must not be null"),
updatedAt = user.updatedAt 
    ?: user.createdAt 
    ?: throw IllegalStateException("updatedAt must not be null")

• Alternatively, enforce non-nullability at the entity level by changing the fields in BaseTimeEntity to non-null (lateinit var createdAt: LocalDateTime and lateinit var updatedAt: LocalDateTime with @Column(nullable = false)), so that JPA auditing populates them and the compiler guarantees their presence.

🤖 Prompt for AI Agents 
In src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt
around lines 28 to 30, the use of non-null assertions (!!) on user.createdAt and
user.updatedAt risks runtime exceptions if these fields are null. Replace the !!
with safe calls and provide fallback logic that throws an IllegalStateException
with a clear message if createdAt or updatedAt are null, for example, assign
createdAt as user.createdAt ?: throw IllegalStateException("createdAt must not
be null") and updatedAt as user.updatedAt ?: user.createdAt ?: throw
IllegalStateException("updatedAt must not be null"). This ensures null safety
without risking NPEs.

)
}
}
Expand Down
17 changes: 5 additions & 12 deletions src/main/kotlin/upbrella/be/user/entity/User.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package upbrella.be.user.entity
import upbrella.be.user.dto.request.JoinRequest
import upbrella.be.user.dto.response.KakaoLoginResponse
import upbrella.be.util.AesEncryptor
import upbrella.be.util.BaseTimeEntity
Copy link

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify BaseTimeEntity implementation and ensure auditing fields are properly configured.

The inheritance from BaseTimeEntity is a good approach for adding auditing capabilities. However, I need to verify the implementation details.

Also applies to: 24-24

🏁 Script executed:

#!/bin/bash
# Description: Verify BaseTimeEntity implementation and its JPA auditing configuration

# Find and examine BaseTimeEntity class
fd -e kt BaseTimeEntity --exec cat {} \;

# Search for any other entities extending BaseTimeEntity to ensure consistency
ast-grep --pattern 'class $_ : BaseTimeEntity'

Length of output: 819

Audit fields verified – BaseTimeEntity is properly implemented

The BaseTimeEntity superclass is correctly defined with @MappedSuperclass, @EntityListeners(AuditingEntityListener::class), and JPA auditing annotations (@CreatedDate, @LastModifiedDate). No further changes needed for auditing.

Breaking change: decryptData now mutates the instance

The switch from returning a new User to in-place mutation is a breaking API change. Please:

  • src/main/kotlin/upbrella/be/user/entity/User.kt: Review all call sites of decryptData to ensure callers expect the method to modify the current instance.
  • Update any tests or documentation that assume decryptData returns a fresh User rather than mutating it.
🤖 Prompt for AI Agents 
In src/main/kotlin/upbrella/be/user/entity/User.kt at lines 6 and 24, the
BaseTimeEntity inheritance is correctly implemented with proper auditing
annotations, so no changes are needed there. However, the decryptData method now
mutates the current User instance instead of returning a new one, which is a
breaking change. Review all usages of decryptData throughout the codebase to
confirm they expect in-place mutation, and update any tests or documentation
that assume it returns a new User object accordingly.

import javax.persistence.Entity
import javax.persistence.GeneratedValue
import javax.persistence.GenerationType
Expand All @@ -20,7 +21,7 @@ class User(
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
val id: Long? = null,
) {
) : BaseTimeEntity() {

companion object {
fun createNewUser(
Expand Down Expand Up @@ -64,17 +65,9 @@ class User(
this.accountNumber = null
}

fun decryptData(aesEncryptor: AesEncryptor): User {
return User(
socialId = socialId,
name = name,
phoneNumber = phoneNumber,
email = email,
adminStatus = adminStatus,
bank = aesEncryptor.decrypt(bank),
accountNumber = aesEncryptor.decrypt(accountNumber),
id = id
)
fun decryptData(aesEncryptor: AesEncryptor) {
Copy link

Copilot AI Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] decryptData now mutates the existing User rather than returning a new instance. Consider renaming it to indicate in-place mutation (e.g., decryptDataInPlace) or documenting its side effects.

Suggested change
fun decryptData(aesEncryptor: AesEncryptor) {
fun decryptDataInPlace(aesEncryptor: AesEncryptor) {

Copilot uses AI. Check for mistakes.
this.bank = aesEncryptor.decrypt(bank)
this.accountNumber = aesEncryptor.decrypt(accountNumber)
}
Comment on lines +68 to 71
Copy link

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Breaking change: decryptData method now mutates instance in-place.

The decryptData method has been changed from returning a new User instance to mutating the current instance in-place. This is a breaking change that affects the method's API contract.

Verify that all usages of decryptData in the codebase have been updated to handle the new in-place mutation behavior:

🏁 Script executed:

#!/bin/bash
# Description: Find all usages of decryptData to ensure they handle in-place mutation

# Search for all calls to decryptData method
rg -A 5 -B 2 'decryptData'

# Look for any assignments that might expect a return value
rg -A 3 -B 1 '\.decryptData\s*\('

Length of output: 8585

Ensure tests accommodate decryptData’s in-place mutation

The decryptData method now returns Unit and mutates the User instance directly. Existing tests still treat it as a factory (expecting a returned User), causing compile/runtime errors. Update the following:

  • src/test/kotlin/upbrella/be/user/service/UserServiceTest.kt
    Replace

    .map { user -> user.decryptData(aesEncryptor) }
.map { decrypted -> SingleUserInfoResponse.fromUser(decrypted) }

    With either:

    // Option A: apply mutation, then map
.onEach { it.decryptData(aesEncryptor) }
.map { SingleUserInfoResponse.fromUser(it) }

    Or:

    .map { user -> user.apply { decryptData(aesEncryptor) } }
      .map { SingleUserInfoResponse.fromUser(it) }

  • src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt
    Update its .map { user -> user.decryptData(aesEncryptor) } in the same way as above so that you’re mapping User objects, not Unit.

  • src/test/kotlin/upbrella/be/user/entity/UserTest.kt
    Change

    val decryptedUser = user.decryptData(aesEncryptor)

    To:

    user.decryptData(aesEncryptor)
val decryptedUser = user

After these adjustments, tests will correctly reflect the new in-place behavior of decryptData.

🤖 Prompt for AI Agents 
In src/main/kotlin/upbrella/be/user/entity/User.kt around lines 68 to 71, the
decryptData method was changed to mutate the User instance in-place and return
Unit instead of a new User. To fix related issues, update all usages of
decryptData to no longer expect a returned User. In test files like
UserServiceTest.kt and UserControllerTest.kt, replace map calls that invoke
decryptData with onEach or map using apply to perform mutation and then map the
User objects. In UserTest.kt, call decryptData on the user instance without
assignment, then use the original user variable as the decrypted user. This
ensures all code and tests handle the new in-place mutation correctly.


fun deleteBankAccount() {
Expand Down
12 changes: 8 additions & 4 deletions src/main/kotlin/upbrella/be/user/service/UserService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class UserService(
) {

fun login(socialId: Long): SessionUser {
val foundUser = userReader.findBySocialId(socialId)
val foundUser = userReader.findBySocialId(socialId.hashCode().toLong())
Copy link

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify consistent hashing implementation across the codebase.

The socialId is being hashed before querying. Ensure this hashing approach is consistent with how socialId is stored and used throughout the application, particularly in the join method and other query operations.

🏁 Script executed:

#!/bin/bash
# Description: Verify consistent usage of socialId hashing across the codebase
# Expected: All socialId operations should use consistent hashing

# Search for socialId usage patterns
rg -A 3 -B 3 "socialId.*hashCode|findBySocialId|existsBySocialId" --type kotlin

Length of output: 12146

Align socialId hashing consistently across the codebase

The login method is hashing its incoming socialId again (socialId.hashCode().toLong()), which leads to a different value than the one stored by join/User.ofEncryptJoin (which only hashes the original provider ID once). This double-hashing will break your tests and any lookups.

• In src/main/kotlin/upbrella/be/user/service/UserService.kt (line 34), change:

val foundUser = userReader.findBySocialId(socialId.hashCode().toLong())

to:

val foundUser = userReader.findBySocialId(socialId)

• In your tests (src/test/kotlin/upbrella/be/user/service/UserServiceTest.kt), make sure you’re stubbing and verifying findBySocialId with the same single-hashed value (or pass the raw Kakao ID into login and stub with rawId.hashCode().toLong()).

• Confirm that FixtureBuilderFactory (src/main/kotlin/upbrella/be/config/FixtureBuilderFactory.kt) generates socialId exactly as your production code expects (i.e., one .hashCode().toLong()).

This will ensure all findBySocialId/existsBySocialId calls use the same hashed key.

🤖 Prompt for AI Agents 
In src/main/kotlin/upbrella/be/user/service/UserService.kt at line 34, the
socialId is being hashed again before querying, causing inconsistency with how
socialId is stored and used elsewhere. To fix this, remove the extra hashing and
pass the socialId directly to userReader.findBySocialId. Also, update related
tests and FixtureBuilderFactory to ensure they use the same single-hashed or raw
socialId consistently across the codebase.


return SessionUser.fromUser(foundUser)
}
Expand All @@ -56,7 +56,7 @@ class UserService(

fun findUsers(): AllUsersInfoResponse {
val users = userReader.findAll()
.map { it.decryptData(aesEncryptor) }
users.forEach { it.decryptData(aesEncryptor) }

return AllUsersInfoResponse.fromUsers(users)
}
Expand Down Expand Up @@ -100,16 +100,20 @@ class UserService(
fun findDecryptedUserById(sessionUser: SessionUser): User {
val id = sessionUser.id

return userRepository.findById(id)
val user = userRepository.findById(id)
.orElseThrow { NonExistingMemberException("[ERROR] 존재하지 않는 회원입니다.") }
.decryptData(aesEncryptor)

user.decryptData(aesEncryptor)

return user
}

@Transactional
fun deleteUserBankAccount(id: Long) {
val foundUser = userReader.findUserById(id)
foundUser.deleteBankAccount()
}

@Transactional
fun updateAdminStatus(id: Long) {
val foundUser = userReader.findUserById(id)
Expand Down
26 changes: 26 additions & 0 deletions src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package upbrella.be.util

import org.springframework.data.annotation.CreatedDate
import org.springframework.data.annotation.LastModifiedDate
import org.springframework.data.jpa.domain.support.AuditingEntityListener
import java.time.LocalDateTime
import javax.persistence.Column
import javax.persistence.EntityListeners
import javax.persistence.MappedSuperclass

@MappedSuperclass
@EntityListeners(AuditingEntityListener::class)
abstract class BaseTimeEntity {
@CreatedDate
@Column(updatable = false)
var createdAt: LocalDateTime? = null

@LastModifiedDate
var updatedAt: LocalDateTime? =null

Copy link

Copilot AI Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updatedAt and deletedAt fields lack @Column annotations. Adding @Column (with appropriate properties) will make the schema mapping explicit and improve readability.

Suggested change
var updatedAt: LocalDateTime? =null
@Column(nullable = false)
var updatedAt: LocalDateTime? = null
@Column

Copilot uses AI. Check for mistakes.
var deletedAt: LocalDateTime? = null

fun softDelete() {
deletedAt = LocalDateTime.now()
}
}
4 changes: 4 additions & 0 deletions src/main/resources/db/add_time.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE users
ADD COLUMN created_at DATETIME NOT NULL DEFAULT NOW() COMMENT '생성일',
ADD COLUMN updated_at DATETIME NOT NULL DEFAULT NOW() ON UPDATE NOW() COMMENT '수정일',
ADD COLUMN deleted_at DATETIME NULL COMMENT '삭제일';
Loading