Feature/user created at #498
Conversation
WalkthroughThe changes introduce JPA auditing for automatic timestamp management in the application. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserService
participant UserRepository
participant User (Entity)
participant BaseTimeEntity
Client->>UserService: findUsers()
UserService->>UserRepository: findAll()
UserRepository-->>UserService: List<User>
loop For each User
UserService->>User: decryptData(aesEncryptor)
User->>BaseTimeEntity: Access auditing fields (createdAt, updatedAt, deletedAt)
end
UserService-->>Client: AllUsersInfoResponse (with auditing fields)
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
Adds timestamp tracking for users and integrates JPA auditing.
- Introduce
created_at,updated_at,deleted_atcolumns in theuserstable - Create
BaseTimeEntitysuperclass with auditing fields and soft-delete support - Wire auditing into entities, services, and DTOs to expose time fields in responses
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/db/add_time.sql | Add created_at, updated_at, deleted_at columns |
| src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt | New mapped superclass for audit timestamps and soft-delete |
| src/main/kotlin/upbrella/be/user/service/UserService.kt | Adjust login lookup, decrypt user data in place |
| src/main/kotlin/upbrella/be/user/entity/User.kt | Extend BaseTimeEntity, revise decryptData logic |
| src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt | Include timestamp fields in single-user response |
| src/main/kotlin/upbrella/be/user/dto/response/AllUsersInfoResponse.kt | Update list mapping signature to non-null users |
| src/main/kotlin/upbrella/be/BeApplication.kt | Enable JPA auditing |
Comments suppressed due to low confidence (2)
src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt:23
- There’s no test coverage for the
softDeletemethod or the auditing fields. Consider adding unit tests to verify thatdeletedAtis set and thatcreatedAt/updatedAtbehave as expected.
fun softDelete() {
src/main/kotlin/upbrella/be/user/service/UserService.kt:34
- Using
socialId.hashCode().toLong()alters the original identifier and will cause lookups to fail. Please passsocialIddirectly tofindBySocialId.
val foundUser = userReader.findBySocialId(socialId.hashCode().toLong())
| accountNumber = aesEncryptor.decrypt(accountNumber), | ||
| id = id | ||
| ) | ||
| fun decryptData(aesEncryptor: AesEncryptor) { |
There was a problem hiding this comment.
[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.
| fun decryptData(aesEncryptor: AesEncryptor) { | |
| fun decryptDataInPlace(aesEncryptor: AesEncryptor) { |
| var updatedAt: LocalDateTime? =null | ||
|
|
There was a problem hiding this comment.
The updatedAt and deletedAt fields lack @Column annotations. Adding @Column (with appropriate properties) will make the schema mapping explicit and improve readability.
| var updatedAt: LocalDateTime? =null | |
| @Column(nullable = false) | |
| var updatedAt: LocalDateTime? = null | |
| @Column |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/main/resources/db/add_time.sql (1)
1-4: Database schema correctly implements auditing columns.The SQL implementation properly adds the necessary columns for JPA auditing:
created_atwith appropriate default valueupdated_atwith automatic update triggerdeleted_atfor soft deletion supportConsider using
TIMESTAMPinstead ofDATETIMEfor better timezone handling:- 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 '삭제일'; + ADD COLUMN created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '생성일', + ADD COLUMN updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT '수정일', + ADD COLUMN deleted_at TIMESTAMP NULL COMMENT '삭제일';src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt (1)
11-26: Excellent JPA auditing implementation with minor formatting fix needed.The BaseTimeEntity class is well-designed with proper JPA auditing annotations and soft deletion support. The implementation follows best practices for auditing timestamps.
Fix the spacing issue on line 19:
- var updatedAt: LocalDateTime? =null + var updatedAt: LocalDateTime? = null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/kotlin/upbrella/be/BeApplication.kt(1 hunks)src/main/kotlin/upbrella/be/user/dto/response/AllUsersInfoResponse.kt(1 hunks)src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt(3 hunks)src/main/kotlin/upbrella/be/user/entity/User.kt(3 hunks)src/main/kotlin/upbrella/be/user/service/UserService.kt(3 hunks)src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt(1 hunks)src/main/resources/db/add_time.sql(1 hunks)
🔇 Additional comments (6)
src/main/kotlin/upbrella/be/BeApplication.kt (1)
5-7: LGTM! JPA auditing correctly enabled.The addition of
@EnableJpaAuditingand corresponding import properly enables JPA auditing features for automatic timestamp management.src/main/kotlin/upbrella/be/user/dto/response/AllUsersInfoResponse.kt (1)
7-9: Excellent improvement in type safety!Changing from
List<User?>toList<User>and removing the non-null assertion operator (!!) significantly improves type safety and makes the code more robust.src/main/kotlin/upbrella/be/user/service/UserService.kt (2)
59-59: LGTM - In-place decryption aligns with updated User entity.The forEach approach with in-place decryption is efficient and aligns well with the updated User entity's decryptData method that now mutates the entity directly.
103-109: LGTM - Simplified user retrieval with in-place decryption.The refactored approach is cleaner and more efficient, leveraging the updated decryptData method that mutates the entity in-place rather than creating new instances.
src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt (1)
14-16: Well-structured timestamp fields for auditing support.The addition of auditing timestamp fields properly supports the new BaseTimeEntity functionality. The nullable deletedAt field correctly represents optional soft deletion timestamps.
src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt (1)
23-25: Well-implemented soft deletion method.The softDelete() method provides a clean API for marking entities as deleted without physical removal from the database, which is a good practice for data retention and audit trails.
| import upbrella.be.user.dto.request.JoinRequest | ||
| import upbrella.be.user.dto.response.KakaoLoginResponse | ||
| import upbrella.be.util.AesEncryptor | ||
| import upbrella.be.util.BaseTimeEntity |
There was a problem hiding this comment.
💡 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
decryptDatato ensure callers expect the method to modify the current instance. - Update any tests or documentation that assume
decryptDatareturns a freshUserrather 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.
| fun decryptData(aesEncryptor: AesEncryptor) { | ||
| this.bank = aesEncryptor.decrypt(bank) | ||
| this.accountNumber = aesEncryptor.decrypt(accountNumber) | ||
| } |
There was a problem hiding this comment.
💡 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 mappingUserobjects, notUnit. -
src/test/kotlin/upbrella/be/user/entity/UserTest.kt
Changeval 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 login(socialId: Long): SessionUser { | ||
| val foundUser = userReader.findBySocialId(socialId) | ||
| val foundUser = userReader.findBySocialId(socialId.hashCode().toLong()) |
There was a problem hiding this comment.
💡 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 kotlinLength 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.
| createdAt = user.createdAt!!, | ||
| updatedAt = user.updatedAt!!, | ||
| deletedAt = user.deletedAt |
There was a problem hiding this comment.
💡 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 kotlinLength 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.ktLength 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.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt(3 hunks)src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt(1 hunks)src/main/resources/db/add_time.sql(1 hunks)src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt(3 hunks)src/test/kotlin/upbrella/be/user/entity/UserTest.kt(1 hunks)src/test/kotlin/upbrella/be/user/service/UserServiceTest.kt(0 hunks)
💤 Files with no reviewable changes (1)
- src/test/kotlin/upbrella/be/user/service/UserServiceTest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/kotlin/upbrella/be/user/dto/response/SingleUserInfoResponse.kt
- src/main/kotlin/upbrella/be/util/BaseTimeEntity.kt
- src/main/resources/db/add_time.sql
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/test/kotlin/upbrella/be/user/entity/UserTest.kt (2)
113-113: LGTM: Correct adaptation to in-place mutation.The test correctly reflects the change in
decryptDatamethod behavior from returning a new User instance to mutating the existing instance in place.
117-118: LGTM: Test assertions properly updated.The assertions correctly validate the decrypted fields directly on the original
userobject, which aligns with the new in-place mutation behavior ofdecryptData.src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt (3)
81-81: LGTM: Correct adaptation to in-place mutation.The test correctly calls
decryptDatawithout capturing a return value, reflecting the new in-place mutation behavior.
84-84: LGTM: Mock setup properly updated.The mock now returns the original
userobject instead of a separately assigned decrypted user, which is consistent with the in-place mutation approach.
464-466: LGTM: Test setup correctly handles auditing fields.The test properly sets the
createdAtfield and applies in-place decryption to each user before constructing the response. This aligns with the new auditing functionality.
| .description("관리자 여부"), | ||
| fieldWithPath("users[].createdAt").type(JsonFieldType.ARRAY) | ||
| .description("사용자 생성일") |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the field type in API documentation.
The createdAt field is documented as JsonFieldType.ARRAY, but based on the context of timestamp auditing, this should likely be JsonFieldType.STRING or a date/time type.
🏁 Script executed:
#!/bin/bash
# Description: Check how createdAt is defined in the User entity and DTOs to verify correct field type
# Expected: createdAt should be a timestamp/datetime field, not an array
# Check User entity definition
echo "=== User entity definition ==="
rg -A 5 -B 5 "createdAt" --type kotlin
# Check DTO definitions
echo "=== DTO definitions ==="
ast-grep --pattern 'class $_ {
$$$
createdAt: $TYPE
$$$
}'
# Check if there are any LocalDateTime imports or usages
echo "=== LocalDateTime usage ==="
rg "LocalDateTime" --type kotlin -A 2 -B 2Length of output: 70329
Change users[].createdAt to use JsonFieldType.STRING
The createdAt field is a LocalDateTime and will be serialized as a JSON string, not an array.
• File: src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt
• Around line 506, update the field descriptor:
- fieldWithPath("users[].createdAt").type(JsonFieldType.ARRAY)
+ fieldWithPath("users[].createdAt").type(JsonFieldType.STRING)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .description("관리자 여부"), | |
| fieldWithPath("users[].createdAt").type(JsonFieldType.ARRAY) | |
| .description("사용자 생성일") | |
| .description("관리자 여부"), | |
| fieldWithPath("users[].createdAt").type(JsonFieldType.STRING) | |
| .description("사용자 생성일") |
🤖 Prompt for AI Agents
In src/test/kotlin/upbrella/be/user/controller/UserControllerTest.kt around
lines 505 to 507, the field descriptor for "users[].createdAt" is incorrectly
set as JsonFieldType.ARRAY. Since createdAt is a LocalDateTime and serialized as
a JSON string, change the field type from JsonFieldType.ARRAY to
JsonFieldType.STRING to accurately reflect the data type in the API
documentation.
🟢 구현내용
🧩 고민과 해결과정
Summary by CodeRabbit
New Features
Bug Fixes