Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces foundational JPA entities and Spring Data repositories for user authentication management. New entities include UserEntity with profile/authorization fields and RefreshTokenEntity for token storage. Supporting enums define user roles and token statuses. Repository interfaces provide standard CRUD and custom query operations for users and refresh tokens. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Around line 31-32: The status field in RefreshTokenEntity is currently stored
as an ordinal; update the mapping by annotating the field with
`@Enumerated`(EnumType.STRING) to persist the enum name instead of its ordinal.
Modify the RefreshTokenEntity class to add the `@Enumerated`(EnumType.STRING)
annotation above the private StatusRefreshToken status; ensure you import
javax.persistence.Enumerated and javax.persistence.EnumType (or the project's
JPA equivalents) so the StatusRefreshToken values are stored as strings.
- Around line 14-15: Replace Lombok's `@Data` on RefreshTokenEntity with explicit
`@Getter` and `@Setter` (and keep `@NoArgsConstructor`) to avoid auto-generating
equals/hashCode/toString that touch lazy associations; remove `@Data` and add
`@Getter/`@Setter on the class, and implement equals() and hashCode() manually to
use only the identifier field (id) so comparisons don't trigger lazy load or
fail for detached entities; also avoid including association fields in any
toString() implementation or omit toString entirely to prevent accidental
initialization of `@ManyToOne`(fetch = FetchType.LAZY) relationships.
- Around line 12-33: The Spotless formatting violation is in the
RefreshTokenEntity class; run the Spotless formatter to fix code style issues
(e.g., run ./gradlew spotlessApply), reformat the file containing class
RefreshTokenEntity
(backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java),
review the resulting changes to ensure annotations and imports remain intact
(fields id, tokenHash, user, expiresAt, status and their JPA annotations
unchanged), then add and commit the formatted file.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`:
- Around line 16-60: The file UserEntity has a Spotless formatting violation;
run the formatter and apply changes (e.g., execute ./gradlew spotlessApply) to
reformat the class UserEntity and related source (preserving annotations like
`@Entity`, `@Table`, `@Id`, `@GeneratedValue`, fields such as id, nickname, email,
passwordHash, role, createdAt, updatedAt) so the file conforms to the project's
Spotless rules, then review and commit the resulting whitespace/formatting-only
changes.
- Around line 61-70: Remove the dead commented-out UserDetails implementation
inside the UserEntity class: delete the commented overrides for getAuthorities()
and getPassword() (the lines referencing SimpleGrantedAuthority, role, and
passwordHash) so the class no longer contains commented code; if the UserDetails
behavior is later required it can be restored from VCS.
In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.java`:
- Around line 3-6: The Role enum (Role, STUDENT, TEACHER) fails Spotless
formatting; run the project's formatter (./gradlew spotlessApply) or reformat
the file to match Spotless rules, then stage and commit the updated Role enum
file so the formatting violation is resolved and CI passes.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/StatusRefreshToken.java`:
- Around line 3-7: The Spotless formatting violation is in the
StatusRefreshToken enum; run the formatter and commit the change by executing
./gradlew spotlessApply (or apply your IDE's Java/Spotless formatting) to
reformat
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/StatusRefreshToken.java
so it meets project Spotless rules, then stage and push the updated file (ensure
enum StatusRefreshToken remains unchanged functionally).
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Around line 31-34: The method name setStatusUsedRefreshToken is misleading
because it implies setting tokens to USED but the method accepts currentStatus
and newStatus; rename it to a clearer, generic name such as
updateStatusByUserAndCurrentStatus (or updateRefreshTokenStatusForUser) and
update all usages and repository/interface references accordingly; ensure the
method signature remains the same (params: UserEntity userEntity,
StatusRefreshToken currentStatus, StatusRefreshToken newStatus) and update any
JPQL/SQL query or mapping annotations that reference the old method name.
- Around line 15-35: Run code formatter to fix the Spotless Java violation in
RefreshTokenRepository by applying the project's Spotless rules (run ./gradlew
spotlessApply), then commit the formatted changes; specifically ensure the
interface RefreshTokenRepository and its methods findByTokenHash,
setStatusByRefreshToken, and setStatusUsedRefreshToken adhere to the project's
formatting style (spacing, indentation, and multiline `@Query` formatting) so CI
no longer fails.
- Around line 23-34: The `@Modifying` annotation on setStatusUsedRefreshToken is
missing the same options used by setStatusByRefreshToken; update the `@Modifying`
on setStatusUsedRefreshToken to include clearAutomatically = true and
flushAutomatically = true so it matches setStatusByRefreshToken and avoids
inconsistent persistence context behavior when updating RefreshTokenEntity.
- Around line 18-21: The JPQL query in RefreshTokenRepository's
setStatusByRefreshToken method contains an extra space before WHERE; edit the
`@Query` string in setStatusByRefreshToken to remove the double space so it reads
"UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash =
:tokenHash" (update the query literal only, leaving the method signature,
`@Transactional`, `@Modifying` and `@Param` annotations unchanged).
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java`:
- Line 11: Rename the parameter in UserRepository's findByEmail method from
"login" to "email" so the signature reads findByEmail(String email), matching
the naming used by existsByEmail and improving clarity and consistency; update
any call sites or usages if necessary to use the new parameter name.
- Around line 9-14: The Spotless formatting violation is caused by Java source
formatting in UserRepository; run the formatter (./gradlew spotlessApply) and
commit the changes so the interface UserRepository (and its methods findByEmail
and existsByEmail) adhere to the project's Spotless rules; ensure any imports,
spacing, and line endings in UserRepository are corrected and then re-run the
Gradle build to confirm the violation is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 241c1830-f7b7-49ba-b174-31e03f91ec38
📒 Files selected for processing (6)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/StatusRefreshToken.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.java
Outdated
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (5)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java (1)
23-24: 🧹 Nitpick | 🔵 TrivialUse consistent
@Modifyingcontext synchronization options.At Line 23,
setStatusUsedRefreshTokenshould match Line 19 (clearAutomatically+flushAutomatically) to avoid stale persistence context behavior.♻️ Suggested fix
- `@Modifying` + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Transactional`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java` around lines 23 - 24, The `@Modifying` annotation on setStatusUsedRefreshToken must include the same context sync options as the other modifying method to avoid stale persistence state; update the `@Modifying` on setStatusUsedRefreshToken in RefreshTokenRepository to include clearAutomatically = true and flushAutomatically = true so it matches the other method (and ensure the `@Transactional` remains present).backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java (2)
31-32:⚠️ Potential issue | 🟠 MajorPersist
statusas STRING to prevent enum-order corruption.At Line 31, without
@Enumerated(EnumType.STRING), JPA stores ordinal values by default.🛠️ Proposed fix
+ `@Enumerated`(EnumType.STRING) `@Column`(nullable = false) private StatusRefreshToken status;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java` around lines 31 - 32, The status field in RefreshTokenEntity is currently a StatusRefreshToken enum stored by ordinal which can break if enum order changes; annotate the field with `@Enumerated`(EnumType.STRING) so JPA persists the enum name instead of its ordinal (update the private StatusRefreshToken status declaration in RefreshTokenEntity to include `@Enumerated`(EnumType.STRING) and import javax.persistence.Enumerated and javax.persistence.EnumType accordingly).
14-26: 🛠️ Refactor suggestion | 🟠 MajorAvoid Lombok
@Dataon JPA entity with lazy association.At Line 14,
@Datacan generateequals/hashCode/toStringtouchinguser(LAZY), causing unintended loads and fragile behavior.♻️ Proposed refactor
-import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.Setter; @@ -@Data +@Getter +@Setter +@EqualsAndHashCode(onlyExplicitlyIncluded = true) `@NoArgsConstructor` public class RefreshTokenEntity { @@ + `@EqualsAndHashCode.Include` `@Id` `@GeneratedValue`(strategy = GenerationType.UUID) private UUID id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java` around lines 14 - 26, The entity RefreshTokenEntity uses Lombok's `@Data` which generates equals/hashCode/toString that may access the LAZY user field and trigger unintended loads; replace `@Data` with explicit Lombok annotations (e.g., `@Getter` and `@Setter`) and remove automatic generation of equals/hashCode/toString, then implement or annotate safe versions that exclude the user association (or use `@EqualsAndHashCode`(onlyExplicitlyIncluded = true) and `@ToString`(onlyExplicitlyIncluded = true) with only id/tokenHash included) so that equals, hashCode and toString do not reference the user field.backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java (1)
61-69: 🧹 Nitpick | 🔵 TrivialRemove commented-out dead security code from the entity.
At Line 61, keeping commented implementations in
UserEntityadds noise and obscures the security boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java` around lines 61 - 69, Remove the commented-out security methods from UserEntity: delete the commented blocks for getAuthorities() and getPassword() (which reference SimpleGrantedAuthority, role, and passwordHash) so the entity no longer contains dead/security-related commented code; if those behaviors are required, implement them in a dedicated SecurityUser or a UserDetails adapter instead of leaving commented methods in UserEntity.backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java (1)
11-11: 🧹 Nitpick | 🔵 TrivialUse
At Line 11,
findByEmail(String login)is semantically correct but inconsistent withexistsByEmail(String email)at Line 13.♻️ Suggested rename
- Optional<UserEntity> findByEmail(String login); + Optional<UserEntity> findByEmail(String email);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java` at line 11, In UserRepository, rename the parameter in the method signature findByEmail(String login) to use the same name as existsByEmail — change it to findByEmail(String email) in the UserRepository interface (affecting the UserEntity lookup) and update any call sites or references to the old parameter name to use "email" so the API is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Around line 31-32: The status field in RefreshTokenEntity is currently a
StatusRefreshToken enum stored by ordinal which can break if enum order changes;
annotate the field with `@Enumerated`(EnumType.STRING) so JPA persists the enum
name instead of its ordinal (update the private StatusRefreshToken status
declaration in RefreshTokenEntity to include `@Enumerated`(EnumType.STRING) and
import javax.persistence.Enumerated and javax.persistence.EnumType accordingly).
- Around line 14-26: The entity RefreshTokenEntity uses Lombok's `@Data` which
generates equals/hashCode/toString that may access the LAZY user field and
trigger unintended loads; replace `@Data` with explicit Lombok annotations (e.g.,
`@Getter` and `@Setter`) and remove automatic generation of
equals/hashCode/toString, then implement or annotate safe versions that exclude
the user association (or use `@EqualsAndHashCode`(onlyExplicitlyIncluded = true)
and `@ToString`(onlyExplicitlyIncluded = true) with only id/tokenHash included) so
that equals, hashCode and toString do not reference the user field.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`:
- Around line 61-69: Remove the commented-out security methods from UserEntity:
delete the commented blocks for getAuthorities() and getPassword() (which
reference SimpleGrantedAuthority, role, and passwordHash) so the entity no
longer contains dead/security-related commented code; if those behaviors are
required, implement them in a dedicated SecurityUser or a UserDetails adapter
instead of leaving commented methods in UserEntity.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Around line 23-24: The `@Modifying` annotation on setStatusUsedRefreshToken must
include the same context sync options as the other modifying method to avoid
stale persistence state; update the `@Modifying` on setStatusUsedRefreshToken in
RefreshTokenRepository to include clearAutomatically = true and
flushAutomatically = true so it matches the other method (and ensure the
`@Transactional` remains present).
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java`:
- Line 11: In UserRepository, rename the parameter in the method signature
findByEmail(String login) to use the same name as existsByEmail — change it to
findByEmail(String email) in the UserRepository interface (affecting the
UserEntity lookup) and update any call sites or references to the old parameter
name to use "email" so the API is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 83307d2f-5a60-4f0a-81a6-fe5e1e2481e5
📒 Files selected for processing (6)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/StatusRefreshToken.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
There was a problem hiding this comment.
♻️ Duplicate comments (2)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java (2)
20-20:⚠️ Potential issue | 🟡 MinorRemove the extra space in JPQL string.
At Line 20, there is a double space before
WHERE; it’s harmless but noisy and was already flagged previously.✏️ Proposed fix
- `@Query`("UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash") + `@Query`("UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java` at line 20, The JPQL string in the `@Query` annotation on RefreshTokenRepository contains an extra space before WHERE; update the Query value in RefreshTokenRepository (the `@Query`("UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash")) to use a single space so it reads "...:status WHERE r.tokenHash = :tokenHash" to remove the double space.
19-20:⚠️ Potential issue | 🟠 MajorFlush before clearing persistence context in bulk updates.
At Line 19 and Line 24,
@Modifying(clearAutomatically = true)can clear the persistence context without first flushing pending dirty entities, which risks silent data loss around bulk updates. AddflushAutomatically = trueto both methods.♻️ Proposed fix
- `@Modifying`(clearAutomatically = true) + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash") void setStatusByRefreshToken(`@Param`("tokenHash") String tokenHash, `@Param`("status") StatusRefreshToken status); `@Transactional` - `@Modifying`(clearAutomatically = true) + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`(""" UPDATE RefreshTokenEntity r#!/bin/bash # Verify all `@Modifying` usages that clear but do not flush. rg -nP --type=java '@Modifying\(\s*clearAutomatically\s*=\s*true(?![^)]*flushAutomatically)' -C2Also applies to: 24-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java` around lines 19 - 20, Two repository methods in RefreshTokenRepository use `@Modifying`(clearAutomatically = true) for bulk updates; add flushAutomatically = true to those `@Modifying` annotations so the persistence context is flushed before it’s cleared. Locate the `@Modifying` annotations that annotate the UPDATE queries (the one for "UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash" and the other bulk-update method further down) and change them to include flushAutomatically = true alongside clearAutomatically = true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Line 20: The JPQL string in the `@Query` annotation on RefreshTokenRepository
contains an extra space before WHERE; update the Query value in
RefreshTokenRepository (the `@Query`("UPDATE RefreshTokenEntity r SET r.status =
:status WHERE r.tokenHash = :tokenHash")) to use a single space so it reads
"...:status WHERE r.tokenHash = :tokenHash" to remove the double space.
- Around line 19-20: Two repository methods in RefreshTokenRepository use
`@Modifying`(clearAutomatically = true) for bulk updates; add flushAutomatically =
true to those `@Modifying` annotations so the persistence context is flushed
before it’s cleared. Locate the `@Modifying` annotations that annotate the UPDATE
queries (the one for "UPDATE RefreshTokenEntity r SET r.status = :status WHERE
r.tokenHash = :tokenHash" and the other bulk-update method further down) and
change them to include flushAutomatically = true alongside clearAutomatically =
true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 134d1173-5efb-41fe-8a80-3e57103f1bea
📒 Files selected for processing (4)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
Owner
|
@coderabbitai, can you add description to this PR? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll compose and update the PR description now using the available context. 🏁 Script executed: Length of output: 604
SummaryThis PR introduces the core database layer for user management and refresh token tracking in the SmartJam API. ChangesEntities
|
Satlykovs
requested changes
Mar 21, 2026
Owner
Satlykovs
left a comment
Satlykovs
left a comment
There was a problem hiding this comment.
Добавь документацию для классов, а так, вроде, почти все ок, есть мелкие поправки, посмотри
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.java
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/RefreshTokenStatus.java
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java
Outdated
Show resolved
Hide resolved
Owner
|
@coderabbitai, and now? Can you add description? And pls check PR's code, if it's actually OK or not |
🧩 Analysis chainLet me try updating the PR description and read the code files for review. 🏁 Script executed: Length of output: 614 🏁 Script executed: Length of output: 4380
PR Description (copy & paste)## Summary
This PR introduces the core database layer for user management and refresh token tracking in the SmartJam API.
---
## Changes
### Entities
#### `UserEntity`
- Mapped to the `users` table.
- UUID primary key generated via `GenerationType.UUID`.
- Fields: `nickname`, `email` (unique, non-null), `passwordHash`, optional `firstName`, `lastName`, `avatarUrl`, `fcmToken`
- `role` — persisted as a string enum, defaults to `Role.STUDENT`
- Audit timestamps: `createdAt` (non-null, non-updatable), `updatedAt` (non-null) via Hibernate `@CreationTimestamp` / `@UpdateTimestamp`
#### `RefreshTokenEntity`
- Mapped to the `refresh_tokens` table.
- UUID primary key generated via `GenerationType.UUID`.
- `tokenHash` — unique, non-null hash of the opaque refresh token (security: prevents replay attacks from a compromised DB)
- `user` — lazy `@ManyToOne` → `UserEntity` via `user_id`
- `expiresAt` — non-null `Instant`
- `status` — non-null `StatusRefreshToken` enum persisted as a string
### Enums
- `Role`: `STUDENT`, `TEACHER`
- `StatusRefreshToken`: `ACTIVE`, `USED`, `REVOKED`
### Repositories
- `UserRepository` — `findByEmail`, `existsByEmail`
- `RefreshTokenRepository` — `findByTokenHash`, `setStatusByRefreshToken` (JPQL bulk update), `updateStatusByUserAndCurrentStatus` (JPQL bulk update)Code ReviewOverall the code is solid and the previous issues (wrong 🔴 Medium —
|
| Check | Status |
|---|---|
UUID primary key (not Long) in both entities |
✅ Fixed |
UserRepository / RefreshTokenRepository extend JpaRepository<..., UUID> |
✅ Fixed |
Hash-based refresh token storage (tokenHash) |
✅ Correct |
email unique, nickname non-unique |
✅ Correct |
@Modifying(clearAutomatically = true, flushAutomatically = true) |
✅ Correct |
JPQL @Query with named @Param bindings |
✅ Correct |
🧠 Learnings used
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), both UserRepository and RefreshTokenRepository incorrectly extend JpaRepository with Long as the ID type, but both UserEntity and RefreshTokenEntity use UUID as the primary key (GenerationType.UUID). The correct declarations are JpaRepository<UserEntity, UUID> and JpaRepository<RefreshTokenEntity, UUID>.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java:14-20
Timestamp: 2026-03-12T21:20:40.707Z
Learning: In the SmartJam project, the JTI-based refresh token approach has been confirmed: store only the `jti` (UUID) from the refresh JWT in RefreshTokenEntity.jti (not the full token string). JwtService.generateRefreshToken() should build a signed JWT with a UUID jti claim. RefreshTokenRepository.findByJti(String jti) is used for lookups. AuthService extracts the jti after verifying the JWT signature and stores/validates against it. This applies to all relevant files under backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/.
Learnt from: men229
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java:25-26
Timestamp: 2026-03-16T23:31:36.243Z
Learning: In SmartJam (Satlykovs/SmartJam), UserEntity uses `email` as the unique login identifier (unique = true in Column). The `username`/`nickname` field is a non-unique display name and does not require a uniqueness constraint. Repository methods like `existsByUsername()` / `findUserEntitiesByUsername()` may have been renamed to reflect `nickname`. Do not flag missing uniqueness on the nickname/display-name field. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java and related repository/service files.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java:14-20
Timestamp: 2026-03-11T22:19:39.121Z
Learning: In the SmartJam project (backend/smartjam-api), the team intends to use signed JWT tokens as refresh tokens (instead of opaque random strings). The RefreshTokenEntity in com.smartjam.smartjamapi.entity stores refresh token references in the DB for revocation tracking. The recommended pattern is to embed a `jti` (JWT ID) claim in the refresh JWT and store only the `jti` (or its hash) in the DB, rather than storing the full token string, to avoid exposing replayable tokens if the DB is compromised.
Learnt from: men229
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java:50-54
Timestamp: 2026-03-16T23:18:13.079Z
Learning: In the SmartJam project (backend/smartjam-api), the team decided to store a **hash** of the opaque refresh token string in RefreshTokenEntity (not the full token, and not a JTI from a signed JWT). This is an intentional security design: hashing prevents token replay from a compromised DB. The previous JTI-based recommendation was declined. Do not flag hash-based refresh token storage as an issue in this project.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), old refresh tokens are never deleted when a new token is issued in AuthService.getNewToken(). The fix is to call refreshTokenRepository.delete(refreshToken) before saving the new one. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), the refresh token expiry in AuthService is incorrectly set to jwtService.getJwtExpiration() (the access token TTL). Refresh tokens need a separate, much longer expiration (e.g. 7 days), configured separately as security.jwt.refresh-expiration-time. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java.
✅ Actions performed
Full review triggered.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java (1)
18-21: 🧹 Nitpick | 🔵 TrivialConsider returning
intto report affected row count.Returning the number of updated rows can be useful for logging, debugging, or detecting when no tokens matched the criteria.
♻️ Suggested change
`@Transactional` `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("UPDATE RefreshTokenEntity r SET r.status = :status WHERE r.tokenHash = :tokenHash") - void setStatusByRefreshToken(`@Param`("tokenHash") String tokenHash, `@Param`("status") StatusRefreshToken status); + int setStatusByRefreshToken(`@Param`("tokenHash") String tokenHash, `@Param`("status") StatusRefreshToken status);Apply similar change to
updateStatusByUserAndCurrentStatusif desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java` around lines 18 - 21, Change the repository update method to return the number of affected rows so callers can detect/ log whether any rows were updated; specifically modify the signature of setStatusByRefreshToken (and optionally updateStatusByUserAndCurrentStatus) to return int instead of void, keep the `@Transactional/`@Modifying/@Query annotations and parameter names tokenHash and status unchanged, and ensure service/test callers handle the returned int to check for zero updates where appropriate.backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java (2)
43-44:⚠️ Potential issue | 🟡 MinorIncrease
avatarUrlcolumn length to accommodate long URLs.URLs can exceed the default
VARCHAR(255)limit (e.g., CDN URLs with query parameters, signed URLs). Consider usingTEXTor specifying a larger length.🛠️ Proposed fix
- `@Column`(name = "avatar_url") + `@Column`(name = "avatar_url", length = 2048) private String avatarUrl;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java` around lines 43 - 44, In UserEntity, the avatarUrl field currently maps to the default VARCHAR(255); update its `@Column` mapping for avatarUrl to support longer URLs (either set a larger length attribute or use columnDefinition = "TEXT") so persisted URLs (e.g., signed/CDN URLs) won't be truncated; modify the avatarUrl field's `@Column` annotation in class UserEntity accordingly and run migrations/DDL to apply the schema change.
16-22: 🧹 Nitpick | 🔵 TrivialConsider replacing
@AllArgsConstructorwith@Builderfor better ergonomics.
@AllArgsConstructorgenerates a constructor that includes auto-managed fields (id,createdAt,updatedAt), which callers shouldn't set. Using@Builderprovides a cleaner API where callers only set relevant fields.♻️ Suggested change
+import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import lombok.AllArgsConstructor; `@Setter` `@Getter` `@NoArgsConstructor` -@AllArgsConstructor +@Builder `@Table`(name = "users") `@Entity` public class UserEntity {Note: If
@Builderis added, you may need@AllArgsConstructor(access = AccessLevel.PRIVATE)for it to work with@NoArgsConstructor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java` around lines 16 - 22, Replace the public all-args constructor pattern with a builder to avoid callers setting auto-managed fields: remove or change the existing `@AllArgsConstructor` on UserEntity and add `@Builder` on the class (retain `@NoArgsConstructor`), and add an `@AllArgsConstructor`(access = AccessLevel.PRIVATE) so Lombok can back the builder while preventing external construction that sets id/createdAt/updatedAt; keep `@Getter/`@Setter and `@Entity/`@Table as-is and ensure any usages constructing UserEntity are migrated to UserEntity.builder()...build().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Around line 22-23: The tokenHash column in RefreshTokenEntity currently uses
the default VARCHAR length; update the `@Column` on the tokenHash field to specify
an explicit length that matches the hash format (for example, add length = 64
for SHA-256 hex) so the schema documents and enforces the expected size (modify
the `@Column` annotation on the tokenHash field in RefreshTokenEntity to include
the length attribute, e.g., length = 64).
- Around line 32-34: The status field in RefreshTokenEntity is non-nullable but
has no default; initialize it to StatusRefreshToken.ACTIVE to avoid violating
the DB constraint when persisting new entities (set the field default inline:
private StatusRefreshToken status = StatusRefreshToken.ACTIVE) and ensure any
constructors/builders (e.g., the entity's constructors or factory methods) do
not overwrite it with null—update them to preserve or explicitly set ACTIVE when
creating new RefreshTokenEntity instances.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Around line 15-16: Rename the method parameter in
RefreshTokenRepository::findByTokenHash from token to tokenHash for naming
consistency with the entity field and other usages; update the method signature
to use tokenHash and adjust any call sites if necessary to match the new
parameter name.
---
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`:
- Around line 43-44: In UserEntity, the avatarUrl field currently maps to the
default VARCHAR(255); update its `@Column` mapping for avatarUrl to support longer
URLs (either set a larger length attribute or use columnDefinition = "TEXT") so
persisted URLs (e.g., signed/CDN URLs) won't be truncated; modify the avatarUrl
field's `@Column` annotation in class UserEntity accordingly and run
migrations/DDL to apply the schema change.
- Around line 16-22: Replace the public all-args constructor pattern with a
builder to avoid callers setting auto-managed fields: remove or change the
existing `@AllArgsConstructor` on UserEntity and add `@Builder` on the class (retain
`@NoArgsConstructor`), and add an `@AllArgsConstructor`(access =
AccessLevel.PRIVATE) so Lombok can back the builder while preventing external
construction that sets id/createdAt/updatedAt; keep `@Getter/`@Setter and
`@Entity/`@Table as-is and ensure any usages constructing UserEntity are migrated
to UserEntity.builder()...build().
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Around line 18-21: Change the repository update method to return the number of
affected rows so callers can detect/ log whether any rows were updated;
specifically modify the signature of setStatusByRefreshToken (and optionally
updateStatusByUserAndCurrentStatus) to return int instead of void, keep the
`@Transactional/`@Modifying/@Query annotations and parameter names tokenHash and
status unchanged, and ensure service/test callers handle the returned int to
check for zero updates where appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa37b6de-fd84-428a-be2f-4786520d9a0f
📒 Files selected for processing (6)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/StatusRefreshToken.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Outdated
Show resolved
Hide resolved
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java
Outdated
Show resolved
Hide resolved
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java (2)
24-25: 🧹 Nitpick | 🔵 TrivialSpecify explicit
tokenHashcolumn length for schema clarity.Please set an explicit length matching your hash format (e.g., 64 for SHA-256 hex) instead of relying on default
VARCHAR(255).Suggested diff
- `@Column`(nullable = false, unique = true) + `@Column`(nullable = false, unique = true, length = 64) private String tokenHash;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java` around lines 24 - 25, The tokenHash field in RefreshTokenEntity currently uses the default VARCHAR length; update the `@Column` annotation on the tokenHash field to specify an explicit length matching your hash format (e.g., length = 64 for SHA-256 hex) so the DB schema is clear and constrained; locate the tokenHash field and change its `@Column`(...) to include length = 64 (or the appropriate size) while preserving nullable and unique attributes.
17-18:⚠️ Potential issue | 🟠 Major
@AllArgsConstructorweakens entity invariants (status, audit fields, id).The generated all-args constructor allows passing
nullforstatusand manually setting generated/audit fields. That bypasses your intended defaults and can still violate non-null constraints at persist time. Prefer removing@AllArgsConstructorand exposing a constrained constructor/factory for required business fields only.#!/bin/bash # Verify whether the all-args constructor is used in a way that can pass nulls # or manually set generated/audit fields. rg -nP --type=java 'new\s+RefreshTokenEntity\s*\(' -C2Also applies to: 34-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java` around lines 17 - 18, Remove the unsafe generated all-args constructor: delete the `@AllArgsConstructor` on RefreshTokenEntity and replace it with a single explicit constructor or static factory that only accepts the required business fields (e.g., the token value, owner/user identifier, and any fields you expect callers to set) while leaving id, audit fields (createdAt/updatedAt) and status to their defaults or to be set by the persistence/audit layer; ensure the class still has `@NoArgsConstructor` for JPA and that the new constructor/factory enforces non-null status/required fields so callers cannot pass null for status or manually set generated audit/id fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Around line 18-21: The `@Modifying` update method setStatusByTokenHash currently
returns boolean which Spring Data JPA doesn't support; change its return type to
int (i.e., int setStatusByTokenHash(...)) so the query returns the number of
affected rows, and do the same for any other `@Modifying` methods in
RefreshTokenRepository that currently return boolean; then update the service
layer to convert the returned int to a boolean (e.g., affectedRows > 0) where a
success flag is needed.
---
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Around line 24-25: The tokenHash field in RefreshTokenEntity currently uses
the default VARCHAR length; update the `@Column` annotation on the tokenHash field
to specify an explicit length matching your hash format (e.g., length = 64 for
SHA-256 hex) so the DB schema is clear and constrained; locate the tokenHash
field and change its `@Column`(...) to include length = 64 (or the appropriate
size) while preserving nullable and unique attributes.
- Around line 17-18: Remove the unsafe generated all-args constructor: delete
the `@AllArgsConstructor` on RefreshTokenEntity and replace it with a single
explicit constructor or static factory that only accepts the required business
fields (e.g., the token value, owner/user identifier, and any fields you expect
callers to set) while leaving id, audit fields (createdAt/updatedAt) and status
to their defaults or to be set by the persistence/audit layer; ensure the class
still has `@NoArgsConstructor` for JPA and that the new constructor/factory
enforces non-null status/required fields so callers cannot pass null for status
or manually set generated audit/id fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b730106-bc13-44c0-8538-435fa327fc6a
📒 Files selected for processing (4)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/RefreshTokenStatus.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
...d/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java
Outdated
Show resolved
Hide resolved
Collaborator
Author
|
@coderabbitai, you can do javadoc for all files in this pr |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4830
|
Collaborator
Author
|
@coderabbitai, write the javadoc again, just add to them what exceptions can be thrown, if at all |
|
Tip For best results, initiate chat on the files or code changes.
|
| Method | Exceptions documented |
|---|---|
findByEmail |
IllegalArgumentException (null arg), DataAccessException (DB error) |
existsByEmail |
IllegalArgumentException (null arg), DataAccessException (DB error) |
findByTokenHash |
IllegalArgumentException (null arg), DataAccessException (DB error) |
setStatusByTokenHash |
IllegalArgumentException (null args), DataAccessException (DB error), TransactionRequiredException (no active tx) |
updateStatusByUserAndCurrentStatus |
IllegalArgumentException (null args), DataAccessException (DB error), TransactionRequiredException (no active tx) |
| Enum constants & entity fields | None — exceptions surface at the persistence layer (documented at class level) |
🧠 Learnings used
Learnt from: men229
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java:25-26
Timestamp: 2026-03-16T23:31:36.243Z
Learning: In SmartJam (Satlykovs/SmartJam), UserEntity uses `email` as the unique login identifier (unique = true in Column). The `username`/`nickname` field is a non-unique display name and does not require a uniqueness constraint. Repository methods like `existsByUsername()` / `findUserEntitiesByUsername()` may have been renamed to reflect `nickname`. Do not flag missing uniqueness on the nickname/display-name field. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java and related repository/service files.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), both UserRepository and RefreshTokenRepository incorrectly extend JpaRepository with Long as the ID type, but both UserEntity and RefreshTokenEntity use UUID as the primary key (GenerationType.UUID). The correct declarations are JpaRepository<UserEntity, UUID> and JpaRepository<RefreshTokenEntity, UUID>.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java:14-20
Timestamp: 2026-03-11T22:19:39.121Z
Learning: In the SmartJam project (backend/smartjam-api), the team intends to use signed JWT tokens as refresh tokens (instead of opaque random strings). The RefreshTokenEntity in com.smartjam.smartjamapi.entity stores refresh token references in the DB for revocation tracking. The recommended pattern is to embed a `jti` (JWT ID) claim in the refresh JWT and store only the `jti` (or its hash) in the DB, rather than storing the full token string, to avoid exposing replayable tokens if the DB is compromised.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java:14-20
Timestamp: 2026-03-12T21:20:40.707Z
Learning: In the SmartJam project, the JTI-based refresh token approach has been confirmed: store only the `jti` (UUID) from the refresh JWT in RefreshTokenEntity.jti (not the full token string). JwtService.generateRefreshToken() should build a signed JWT with a UUID jti claim. RefreshTokenRepository.findByJti(String jti) is used for lookups. AuthService extracts the jti after verifying the JWT signature and stores/validates against it. This applies to all relevant files under backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), old refresh tokens are never deleted when a new token is issued in AuthService.getNewToken(). The fix is to call refreshTokenRepository.delete(refreshToken) before saving the new one. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java.
Learnt from: Satlykovs
Repo: Satlykovs/SmartJam PR: 0
File: :0-0
Timestamp: 2026-03-16T23:56:57.162Z
Learning: In SmartJam (Satlykovs/SmartJam), the refresh token expiry in AuthService is incorrectly set to jwtService.getJwtExpiration() (the access token TTL). Refresh tokens need a separate, much longer expiration (e.g. 7 days), configured separately as security.jwt.refresh-expiration-time. Applies to backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java.
Learnt from: men229
Repo: Satlykovs/SmartJam PR: 2
File: backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/service/AuthService.java:50-54
Timestamp: 2026-03-16T23:18:13.079Z
Learning: In the SmartJam project (backend/smartjam-api), the team decided to store a **hash** of the opaque refresh token string in RefreshTokenEntity (not the full token, and not a JTI from a signed JWT). This is an intentional security design: hashing prevents token replay from a compromised DB. The previous JTI-based recommendation was declined. Do not flag hash-based refresh token storage as an issue in this project.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java (1)
31-33:⚠️ Potential issue | 🟡 Minor
@AllArgsConstructorweakens entity invariants for generated/defaulted fields.This constructor allows callers to pass
nullforroleand set persistence-managed fields (id,createdAt,updatedAt) directly. Prefer a builder or explicit constructor over required business fields only.♻️ Suggested direction
-@AllArgsConstructor +@Builder// Or add an explicit constructor with only required inputs: // public UserEntity(String nickname, String email, String passwordHash) { ... }Also applies to: 78-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java` around lines 31 - 33, Remove the blanket `@AllArgsConstructor` from UserEntity because it allows callers to set persistence-managed fields (id, createdAt, updatedAt) and nullable business fields like role; instead provide an explicit constructor or builder that only accepts required business inputs (e.g., nickname, email, passwordHash) and leave persistence fields and defaults managed by JPA, or add a static builder/factory to enforce non-null role and defaulted fields—update the class by removing `@AllArgsConstructor` and adding the named explicit constructor or builder method to preserve entity invariants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Line 29: Add database indexes to the RefreshTokenEntity JPA mapping to support
queries like updateStatusByUserAndCurrentStatus(...) and cleanup by expiration:
update the `@Table`(...) annotation on class RefreshTokenEntity to include `@Index`
entries for the user+status lookup and for expires_at (e.g. index on columnList
"user_id, status" and index on "expires_at"). Ensure index names are unique
(e.g. idx_refresh_token_user_status, idx_refresh_token_expires_at) and that the
column names match the entity mapping for the user relationship and the
expiresAt field.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`:
- Line 36: Update the Javadoc in RefreshTokenRepository for the method that
accepts tokenHash: change the `@throws` clause to reference the actual parameter
name tokenHash (i.e., "@throws IllegalArgumentException if {`@code` tokenHash} is
{`@code` null}") so the documentation matches the method signature.
---
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`:
- Around line 31-33: Remove the blanket `@AllArgsConstructor` from UserEntity
because it allows callers to set persistence-managed fields (id, createdAt,
updatedAt) and nullable business fields like role; instead provide an explicit
constructor or builder that only accepts required business inputs (e.g.,
nickname, email, passwordHash) and leave persistence fields and defaults managed
by JPA, or add a static builder/factory to enforce non-null role and defaulted
fields—update the class by removing `@AllArgsConstructor` and adding the named
explicit constructor or builder method to preserve entity invariants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d9d3f34-0995-40d5-8103-853998d316aa
📒 Files selected for processing (6)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/RefreshTokenStatus.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
| * {@link org.springframework.dao.DataIntegrityViolationException}). | ||
| */ | ||
| @Entity | ||
| @Table(name = "refresh_tokens") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add table indexes for token lifecycle query paths.
updateStatusByUserAndCurrentStatus(...) filters by user + status; adding an index avoids full scans as token volume grows. An expires_at index also helps cleanup jobs.
⚙️ Proposed schema tuning
-@Table(name = "refresh_tokens")
+@Table(
+ name = "refresh_tokens",
+ indexes = {
+ `@Index`(name = "idx_refresh_tokens_user_status", columnList = "user_id,status"),
+ `@Index`(name = "idx_refresh_tokens_expires_at", columnList = "expires_at")
+ })📝 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.
Suggested change
| @Table(name = "refresh_tokens") | |
| `@Entity` | |
| `@Table`( | |
| name = "refresh_tokens", | |
| indexes = { | |
| `@Index`(name = "idx_refresh_tokens_user_status", columnList = "user_id,status"), | |
| `@Index`(name = "idx_refresh_tokens_expires_at", columnList = "expires_at") | |
| }) | |
| `@Getter` | |
| `@Setter` | |
| `@EqualsAndHashCode`(onlyExplicitlyIncluded = true) | |
| `@NoArgsConstructor` | |
| public class RefreshTokenEntity { | |
| `@Id` | |
| `@GeneratedValue`(strategy = GenerationType.UUID) | |
| `@EqualsAndHashCode.Include` | |
| private UUID id; | |
| `@Column`(nullable = false, unique = true, length = 64) | |
| private String tokenHash; | |
| `@ManyToOne`(fetch = FetchType.LAZY) | |
| `@JoinColumn`(name = "user_id", nullable = false) | |
| private UserEntity user; | |
| `@Column`(nullable = false) | |
| private LocalDateTime expiresAt; | |
| `@Column`(nullable = false) | |
| `@Enumerated`(EnumType.STRING) | |
| private StatusRefreshToken status = StatusRefreshToken.ACTIVE; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`
at line 29, Add database indexes to the RefreshTokenEntity JPA mapping to
support queries like updateStatusByUserAndCurrentStatus(...) and cleanup by
expiration: update the `@Table`(...) annotation on class RefreshTokenEntity to
include `@Index` entries for the user+status lookup and for expires_at (e.g. index
on columnList "user_id, status" and index on "expires_at"). Ensure index names
are unique (e.g. idx_refresh_token_user_status, idx_refresh_token_expires_at)
and that the column names match the entity mapping for the user relationship and
the expiresAt field.
| * @param tokenHash the SHA-256 hex digest (or equivalent hash) of the refresh token; must not be {@code null} | ||
| * @return an {@link Optional} containing the matching {@link RefreshTokenEntity}, or {@link Optional#empty()} if no | ||
| * record with the given hash exists | ||
| * @throws IllegalArgumentException if {@code token} is {@code null} |
There was a problem hiding this comment.
Fix Javadoc parameter name in @throws clause.
The method parameter is tokenHash, but Javadoc says token.
✏️ Proposed fix
- * `@throws` IllegalArgumentException if {`@code` token} is {`@code` null}
+ * `@throws` IllegalArgumentException if {`@code` tokenHash} is {`@code` null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.java`
at line 36, Update the Javadoc in RefreshTokenRepository for the method that
accepts tokenHash: change the `@throws` clause to reference the actual parameter
name tokenHash (i.e., "@throws IllegalArgumentException if {`@code` tokenHash} is
{`@code` null}") so the documentation matches the method signature.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java (1)
28-38: 🛠️ Refactor suggestion | 🟠 MajorAdd
@EqualsAndHashCodebased onidfor consistent JPA behavior.Unlike
UserEntity, this entity lacks@EqualsAndHashCode(onlyExplicitlyIncluded = true)with@EqualsAndHashCode.Includeon theidfield. Without explicit equals/hashCode, Lombok's default (from Object) uses identity, which can cause inconsistent behavior in JPA-managed collections during detachment/reattachment cycles.♻️ Proposed fix
`@Entity` `@Table`(name = "refresh_tokens") `@Getter` `@Setter` `@NoArgsConstructor` +@EqualsAndHashCode(onlyExplicitlyIncluded = true) public class RefreshTokenEntity { /** Unique identifier for the refresh token record, generated automatically as a UUID. */ `@Id` `@GeneratedValue`(strategy = GenerationType.UUID) + `@EqualsAndHashCode.Include` private UUID id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java` around lines 28 - 38, The RefreshTokenEntity class is missing explicit equals/hashCode leading to identity-based comparisons; add Lombok's `@EqualsAndHashCode`(onlyExplicitlyIncluded = true) on the RefreshTokenEntity class and mark the id field with `@EqualsAndHashCode.Include` so equals/hashCode are based solely on the UUID id; ensure imports and annotation placement match existing Lombok annotations (e.g., alongside `@Getter/`@Setter/@NoArgsConstructor) to provide consistent JPA behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`:
- Around line 29-36: The entity UserEntity currently uses Lombok's
`@AllArgsConstructor` which exposes auto-managed fields (id, createdAt,
updatedAt); replace `@AllArgsConstructor` with a safer pattern (e.g., Lombok's
`@Builder` or a custom constructor) so callers can only set user-controlled
fields: remove `@AllArgsConstructor` from the class declaration, add `@Builder` (or
add a constructor that accepts only username/email/password/etc.), and ensure
id, createdAt, and updatedAt remain managed by JPA and are not present in the
public constructor or builder API.
---
Duplicate comments:
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.java`:
- Around line 28-38: The RefreshTokenEntity class is missing explicit
equals/hashCode leading to identity-based comparisons; add Lombok's
`@EqualsAndHashCode`(onlyExplicitlyIncluded = true) on the RefreshTokenEntity
class and mark the id field with `@EqualsAndHashCode.Include` so equals/hashCode
are based solely on the UUID id; ensure imports and annotation placement match
existing Lombok annotations (e.g., alongside `@Getter/`@Setter/@NoArgsConstructor)
to provide consistent JPA behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3807f10-39b0-417d-a6d8-ec5a1af911ce
📒 Files selected for processing (6)
backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/RefreshTokenEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/RefreshTokenStatus.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/enums/Role.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/RefreshTokenRepository.javabackend/smartjam-api/src/main/java/com/smartjam/smartjamapi/repository/UserRepository.java
Comment on lines
+29
to
+36
| @Setter | ||
| @Getter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Table(name = "users") | ||
| @Entity | ||
| @EqualsAndHashCode(onlyExplicitlyIncluded = true) | ||
| public class UserEntity { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider replacing @AllArgsConstructor with @Builder or a custom constructor.
The @AllArgsConstructor annotation generates a constructor that accepts id, createdAt, and updatedAt, which are auto-generated fields. Callers could inadvertently set these, potentially causing conflicts with JPA's lifecycle management. A @Builder pattern or a custom constructor with only user-settable fields would be safer for entity creation.
♻️ Proposed alternative using `@Builder`
`@Setter`
`@Getter`
`@NoArgsConstructor`
-@AllArgsConstructor
+@Builder
`@Table`(name = "users")
`@Entity`
`@EqualsAndHashCode`(onlyExplicitlyIncluded = true)
public class UserEntity {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/smartjam-api/src/main/java/com/smartjam/smartjamapi/entity/UserEntity.java`
around lines 29 - 36, The entity UserEntity currently uses Lombok's
`@AllArgsConstructor` which exposes auto-managed fields (id, createdAt,
updatedAt); replace `@AllArgsConstructor` with a safer pattern (e.g., Lombok's
`@Builder` or a custom constructor) so callers can only set user-controlled
fields: remove `@AllArgsConstructor` from the class declaration, add `@Builder` (or
add a constructor that accepts only username/email/password/etc.), and ensure
id, createdAt, and updatedAt remain managed by JPA and are not present in the
public constructor or builder API.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.