feat: introduce UserContextData and Auditable #39
Conversation
feat: CommonConstant feat: CommonMapper feat: introduce AbstractAuditableEntity, AbstractBaseEntity interface in common
WalkthroughThis pull request introduces several significant changes, including the addition of new interfaces and classes for auditing and entity management, the introduction of validation dependencies, and modifications to existing data transfer objects (DTOs). Key updates include the creation of Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (12)
sep490-idp/src/main/java/sep490/idp/entity/AbstractBaseEntity.java (1)
47-54: Consider Using Hibernate's Utility Method for Proxy Class RetrievalThe custom method
getPersistenceClassWithoutInitializingProxycan be replaced with the Hibernate utilityHibernate.getClass(entity), which serves the same purpose and reduces custom code.Apply this refactor to utilize Hibernate's built-in method:
-import org.hibernate.proxy.HibernateProxy; -import org.hibernate.proxy.LazyInitializer; ... -private static Class<?> getPersistenceClassWithoutInitializingProxy(Object entity) { - if (entity instanceof HibernateProxy proxy) { - LazyInitializer li = proxy.getHibernateLazyInitializer(); - return li.getPersistentClass(); - } else { - return entity.getClass(); - } -} + +import org.hibernate.Hibernate; ... if (Hibernate.getClass(this) != Hibernate.getClass(obj)) { return false; }sep490-common/src/main/java/sep490/common/api/dto/PageDTO.java (1)
12-20: LGTM! Consider updating the factory method documentation.The record definition and validation constraints are well-structured. The renaming to PageDTO aligns well with the broader DTO naming standardization effort.
Consider adding Javadoc for the static factory method:
/** * Creates a new PageDTO instance. * * @param pageSize The number of items per page (1-100) * @param pageNumber The zero-based page number * @return A new PageDTO instance */ public static PageDTO of(int pageSize, int pageNumber) {sep490-common/src/main/java/sep490/common/api/dto/SortDTO.java (1)
13-20: LGTM! Consider updating the factory method documentation.The record definition and validation constraints are well-structured. The renaming to SortDTO maintains consistency with the project's DTO naming convention.
Consider adding Javadoc for the static factory method:
/** * Creates a new SortDTO instance. * * @param field The name of the field to sort by * @param direction The sort direction (ascending or descending) * @return A new SortDTO instance */ public static SortDTO of(String field, SortDirection direction) {sep490-idp/src/main/java/sep490/idp/configs/AuditorAwareImpl.java (1)
16-20: Consider enhancing error handling and logging.While the implementation is functional, consider these improvements for robustness:
- Add error handling for cases where security context might be unavailable
- Include logging for audit trail visibility
Here's a suggested implementation:
+import lombok.extern.slf4j.Slf4j; + @Configuration @RequiredArgsConstructor +@Slf4j public class AuditorAwareImpl implements AuditorAware<String> { @Nonnull @Override public Optional<String> getCurrentAuditor() { - return SecurityUtils.getUserContextData().map(UserContextData::getUsername); + try { + Optional<String> username = SecurityUtils.getUserContextData() + .map(UserContextData::getUsername); + username.ifPresent(name -> + log.debug("Current auditor username: {}", name)); + return username; + } catch (Exception e) { + log.warn("Failed to retrieve current auditor", e); + return Optional.empty(); + } } }sep490-common/src/main/java/sep490/common/api/dto/DateRangeDTO.java (1)
23-26: Consider enhancing the validation error message.The validation message could be more descriptive by including the actual dates.
- throw new IllegalArgumentException("Start date must be before or equal to end date."); + throw new IllegalArgumentException(String.format("Start date '%s' must be before or equal to end date '%s'.", from, to));sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java (1)
Line range hint
22-37: Consider documenting the immutability design choice.The removal of setters for audit fields enforces immutability, which is good practice. Consider adding a class-level Javadoc explaining this design decision.
+/** + * Base class for auditable entities with immutable audit fields. + * Audit information is automatically managed by JPA auditing. + */ @Getter @FieldNameConstantssep490-idp/src/main/java/sep490/idp/entity/UserEntity.java (2)
13-17: Consider adding database indexes for performance.Add indexes for frequently queried fields like username and email.
@Entity @Getter @Setter @NoArgsConstructor @Table(name = "users") +@Table(name = "users", indexes = { + @Index(name = "idx_users_username", columnList = "username"), + @Index(name = "idx_users_email", columnList = "email") +})
1-46: Consider implementing additional security measures.Recommendations for enhancing security:
- Add account status flags (enabled, locked, etc.)
- Add password expiry tracking
- Consider adding failed login attempt tracking
Would you like me to provide a detailed implementation for these security enhancements?
sep490-idp/src/main/java/sep490/idp/mapper/CommonMapper.java (2)
15-16: Add interface documentationPlease add Javadoc to describe the interface's purpose and its method's contract.
Add documentation like this:
@Mapper(componentModel = MappingConstants.ComponentModel.SPRING) +/** + * Common mapper interface for converting DTOs to Spring domain objects. + */ public interface CommonMapper {
24-31: Consider extracting sort logic to a separate methodThe sort conversion logic could be cleaner and more reusable if extracted to a separate method.
Consider refactoring like this:
+ default Sort createSort(SortDTO sortDto) { + if (Objects.nonNull(sortDto) + && StringUtils.isNotBlank(sortDto.field()) + && Objects.nonNull(sortDto.direction())) { + var direction = Objects.equals(sortDto.direction(), SortDirection.ASC) + ? Sort.Direction.ASC + : Sort.Direction.DESC; + return Sort.by(direction, sortDto.field()); + } + return Sort.unsorted(); + } + default Pageable toPageable(PageDTO searchPageDto, SortDTO sortDto) { // ... pagination logic ... - if (Objects.nonNull(sortDto) - && StringUtils.isNotBlank(sortDto.field()) - && Objects.nonNull(sortDto.direction())) { - var direction = Objects.equals(sortDto.direction(), SortDirection.ASC) - ? Sort.Direction.ASC - : Sort.Direction.DESC; - return PageRequest.of(pageNumber, pageSize, Sort.by(direction, sortDto.field())); - } - return PageRequest.of(pageNumber, pageSize); + return PageRequest.of(pageNumber, pageSize, createSort(sortDto)); }sep490-common/build.gradle (1)
38-38: Remove duplicate commons-validator dependencyThe commons-validator dependency is declared in both
implementationandapiconfigurations. Since it's already exposed viaapi, theimplementationdeclaration is redundant.Remove the redundant implementation:
- implementation group: 'commons-validator', name: 'commons-validator', version: '1.9.0' api group: 'commons-validator', name: 'commons-validator', version: '1.9.0'Also applies to: 46-46
sep490-idp/build.gradle (1)
47-47: Move validation dependency to appropriate sectionThe
spring-boot-starter-validationdependency is incorrectly placed under the Database section. It should be grouped with other Spring Boot starters or in a new Validation section.Move the dependency to a more appropriate section:
// Database - implementation 'org.springframework.boot:spring-boot-starter-validation' implementation 'org.springframework.boot:spring-boot-starter-data-jpa' implementation 'org.flywaydb:flyway-core' implementation 'org.flywaydb:flyway-database-postgresql' runtimeOnly 'org.postgresql:postgresql' + // Validation + implementation 'org.springframework.boot:spring-boot-starter-validation' + // Testing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (22)
sep490-common/build.gradle(1 hunks)sep490-common/src/main/java/sep490/common/api/AuditableEntity.java(1 hunks)sep490-common/src/main/java/sep490/common/api/BaseEntity.java(1 hunks)sep490-common/src/main/java/sep490/common/api/dto/DateRangeDTO.java(2 hunks)sep490-common/src/main/java/sep490/common/api/dto/PageDTO.java(1 hunks)sep490-common/src/main/java/sep490/common/api/dto/SearchCriteria.java(0 hunks)sep490-common/src/main/java/sep490/common/api/dto/SearchCriteriaDTO.java(1 hunks)sep490-common/src/main/java/sep490/common/api/dto/SearchResultDTO.java(1 hunks)sep490-common/src/main/java/sep490/common/api/dto/SortDTO.java(1 hunks)sep490-common/src/main/java/sep490/common/api/utils/CommonConstant.java(1 hunks)sep490-idp/build.gradle(2 hunks)sep490-idp/src/main/java/sep490/idp/IdentityProvider.java(1 hunks)sep490-idp/src/main/java/sep490/idp/configs/AuditorAwareImpl.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/AbstractBaseEntity.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/JpaUtils.java(0 hunks)sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java(1 hunks)sep490-idp/src/main/java/sep490/idp/mapper/CommonMapper.java(1 hunks)sep490-idp/src/main/java/sep490/idp/security/UserContextData.java(1 hunks)sep490-idp/src/main/java/sep490/idp/utils/SecurityUtils.java(1 hunks)sep490-idp/src/main/resources/db/migration/V0.0.1.0__Users.sql(1 hunks)sep490-infrastructure/postgres/init-db.sql(1 hunks)
💤 Files with no reviewable changes (2)
- sep490-common/src/main/java/sep490/common/api/dto/SearchCriteria.java
- sep490-idp/src/main/java/sep490/idp/entity/JpaUtils.java
✅ Files skipped from review due to trivial changes (4)
- sep490-idp/src/main/java/sep490/idp/IdentityProvider.java
- sep490-common/src/main/java/sep490/common/api/utils/CommonConstant.java
- sep490-common/src/main/java/sep490/common/api/BaseEntity.java
- sep490-infrastructure/postgres/init-db.sql
🔇 Additional comments (8)
sep490-idp/src/main/java/sep490/idp/entity/AbstractBaseEntity.java (1)
24-24: Implementation of BaseEntity Interface Enhances Consistency
Implementing the BaseEntity interface provides a standardized way to access the id and version fields across all entities, improving consistency and maintainability in the codebase.
sep490-common/src/main/java/sep490/common/api/AuditableEntity.java (1)
1-10: Introduction of AuditableEntity Interface Enhances Auditing Capabilities
Defining the AuditableEntity interface standardizes auditing methods across entities, facilitating consistent tracking of creation and modification metadata.
sep490-common/src/main/java/sep490/common/api/dto/SearchCriteriaDTO.java (1)
5-13: Effective Use of Java Records for Data Transfer Objects
The SearchCriteriaDTO record provides an immutable and concise way to encapsulate search criteria, enhancing code readability and maintainability. The inclusion of the static of method improves the usability of the record.
sep490-common/src/main/java/sep490/common/api/dto/SearchResultDTO.java (1)
8-13: Validation Annotations Ensure Data Integrity
Applying @NotNull and @Min(0) annotations to the results and totalElements fields enforces data integrity by validating input data, which is a good practice for robust application design.
sep490-idp/src/main/java/sep490/idp/configs/AuditorAwareImpl.java (2)
12-14: LGTM! Configuration setup is correct.
The class is properly configured as a Spring bean with appropriate annotations.
1-21: Verify integration with JPA auditing configuration.
Ensure that JPA auditing is properly enabled in your configuration.
✅ Verification successful
Let me check the content of IdentityProvider.java and look for any auditable entities in the codebase using a different pattern.
Let me check the content of these audit-related files to verify the complete integration.
JPA auditing is properly configured and integrated
The verification confirms that:
@EnableJpaAuditingis correctly configured in the main application classAbstractAuditableEntityis properly set up with@EntityListeners(AuditingEntityListener.class)and all required audit annotations- The
AuditorAwareImplimplementation will correctly provide the username for audit fields
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for @EnableJpaAuditing annotation in configuration files
rg -l "EnableJpaAuditing" --type java
# Check for auditable entity implementations
ast-grep --pattern 'class $_ implements AuditableEntity {
$$$
}'
Length of output: 164
Script:
#!/bin/bash
# Check the content of IdentityProvider.java
cat sep490-idp/src/main/java/sep490/idp/IdentityProvider.java
# Look for classes with audit-related annotations
ast-grep --pattern '@EntityListeners(AuditingEntityListener.class)
class $_ {
$$$
}'
# Look for audit-related fields
rg -l "CreatedDate|LastModifiedDate|CreatedBy|LastModifiedBy" --type java
Length of output: 772
Script:
#!/bin/bash
# Check the content of AbstractAuditableEntity
cat sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java
# Check the content of AuditableEntity interface
cat sep490-common/src/main/java/sep490/common/api/AuditableEntity.java
# Look for entities extending AbstractAuditableEntity
ast-grep --pattern 'class $_ extends AbstractAuditableEntity {
$$$
}'
Length of output: 1752
sep490-common/src/main/java/sep490/common/api/dto/DateRangeDTO.java (1)
Line range hint 8-16: LGTM! Clean DTO structure with proper validation.
The record declaration with NotNull constraints is well-structured and follows best practices.
sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java (1)
21-21: Verify JPA auditing configuration in the application.
The class is properly annotated for auditing, but ensure that @EnableJpaAuditing is configured and an AuditorAware implementation is provided.
✅ Verification successful
Let me check the actual implementation of these files to verify the JPA auditing configuration.
JPA auditing is properly configured
The application has correct JPA auditing setup with:
@EnableJpaAuditingin the main application class- A properly implemented
AuditorAwareImplthat retrieves the current user from security context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for JPA auditing configuration
rg -l "EnableJpaAuditing|AuditorAware"
Length of output: 164
Script:
#!/bin/bash
# Check the content of the configuration files
echo "=== IdentityProvider.java ==="
cat sep490-idp/src/main/java/sep490/idp/IdentityProvider.java
echo -e "\n=== AuditorAwareImpl.java ==="
cat sep490-idp/src/main/java/sep490/idp/configs/AuditorAwareImpl.java
Length of output: 1292
feat: CommonConstant
feat: CommonMapper
feat: introduce AbstractAuditableEntity, AbstractBaseEntity interface in common
Summary by CodeRabbit
New Features
commons-validatorlibrary.AuditableEntityandBaseEntityfor enhanced entity management.SearchCriteriaDTO,SearchResultDTO, andSortDTOfor improved data handling.UserEntityclass for user management with validation constraints.SecurityUtilsfor user context data retrieval.Bug Fixes
Chores