feature: Use hibernate filter to add condition to query #121
Conversation
WalkthroughThis pull request introduces enterprise-level filtering and an adapter-based data retrieval mechanism. A new AOP aspect applies a Hibernate filter based on the current user's enterprise ID for repository methods. The Changes
Sequence Diagram(s)sequenceDiagram
participant Repo as UserRepository
participant Aspect as UserRepositoryAdvice
participant Session as Hibernate Session
participant Security as SecurityUtils
participant Caller as Application Code
Caller->>Aspect: Call UserRepository method
Aspect->>Session: Retrieve current Hibernate Session
Session->>Aspect: Return Session
Aspect->>Security: Get current user's enterprise ID
Security-->>Aspect: Return enterprise ID
Aspect->>Session: Enable enterprise filter with ID
Aspect->>Repo: Proceed with repository method call
Repo-->>Aspect: Return result
Aspect->>Session: Disable filter (in finally)
Aspect-->>Caller: Return filtered result
sequenceDiagram
participant Service as UserServiceImpl
participant Adapter as UserRepositoryAdapter
participant Repo as UserRepository
Service->>Adapter: findByIDs(userIds)
Adapter->>Repo: Delegate retrieval fetching UserEntity objects
Repo-->>Adapter: Return user list
Adapter-->>Service: Return user list for deletion
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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: 1
🧹 Nitpick comments (3)
sep490-idp/src/main/java/sep490/idp/repository/UserRepositoryAdapter.java (1)
16-18: Minor naming convention adjustment.Method name uses “IDs”, but common Java naming prefers
findByIds. Consider renaming to match typical camelCase style.Example fix:
- public List<UserEntity> findByIDs(Set<UUID> ids) { + public List<UserEntity> findByIds(Set<UUID> ids) {sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java (1)
37-38: Filter usage looks good!The defined “belongEnterpriseFilter” matches your aspect logic, ensuring users are scoped by enterprise. Continue verifying that the subselect-based condition meets your performance requirements for large datasets. If performance becomes an issue, consider a join-based filter or caching mechanism.
sep490-idp/src/main/java/sep490/idp/service/impl/UserServiceImpl.java (1)
121-121: Verify enterprise-specific filtering in the adapter.The change to use
userRepositoryAdapterlooks good, but we should ensure that the adapter properly applies the enterprise-specific filtering to prevent unauthorized access across enterprises.Let's verify the implementation of the adapter:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if UserRepositoryAdapter properly implements enterprise filtering # Look for filter definition in UserEntity echo "Checking UserEntity for filter definition:" rg -A 5 "@FilterDef.*belongEnterpriseFilter" --type java # Look for filter enablement in UserRepositoryAdapter echo "\nChecking UserRepositoryAdapter implementation:" rg -A 10 "class UserRepositoryAdapter" --type java # Look for filter usage in AOP advice echo "\nChecking AOP advice for filter enablement:" rg -A 10 "UserRepositoryAdvice" --type javaLength of output: 3025
Action: Confirm enterprise-specific filtering is correctly implemented via AOP.
- The
UserEntityproperly defines the enterprise filter with@FilterDefand@Filter.- The
UserRepositoryAdapterdelegates touserRepo.findByIDswithout bypassing any filtering logic.- The AOP advice (
UserRepositoryAdvice) targets all methods within theUserRepositoryAdapter, ensuring the filter is applied automatically before repository calls execute.Based on the evidence, the adapter’s usage is sound, and enterprise-specific filtering will be enforced.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java(2 hunks)sep490-idp/src/main/java/sep490/idp/repository/UserRepositoryAdapter.java(1 hunks)sep490-idp/src/main/java/sep490/idp/service/impl/UserServiceImpl.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Java Modules (sep490-enterprise)
🔇 Additional comments (2)
sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java (1)
23-24: Consider removingfinalfrom theEntityManagerfield or dropping@RequiredArgsConstructor.
@PersistenceContextis typically used with field or setter injection, whereas@RequiredArgsConstructorinjects final fields via the constructor. Mixing both can cause unexpected issues or conflicts in managed bean lifecycles.sep490-idp/src/main/java/sep490/idp/service/impl/UserServiceImpl.java (1)
45-45: LGTM!The field declaration follows proper encapsulation principles and dependency injection patterns.
sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java
Outdated
Show resolved
Hide resolved
GiaBaorr
force-pushed
the
authorize-funny
branch
from
9b5a94b to
74386c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java (1)
37-51: Well-designed multi-tenancy implementation using Hibernate filters.The implementation provides a robust and reusable mechanism for enterprise-level data isolation. Consider adding documentation to explain:
- When and how the filter should be enabled/disabled
- Performance implications and monitoring strategies
- Integration points with security context
Would you like me to help generate the documentation for this feature?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java(3 hunks)sep490-idp/src/main/java/sep490/idp/repository/UserRepositoryAdapter.java(1 hunks)sep490-idp/src/main/java/sep490/idp/service/impl/UserServiceImpl.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- sep490-idp/src/main/java/sep490/idp/service/impl/UserServiceImpl.java
- sep490-idp/src/main/java/sep490/idp/repository/UserRepositoryAdapter.java
- sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java
🔇 Additional comments (2)
sep490-idp/src/main/java/sep490/idp/entity/UserEntity.java (2)
18-20: LGTM!The imports are correctly organized and specific to the new filter functionality.
Also applies to: 28-28
50-51: LGTM!The filter constants follow proper naming conventions and are used consistently throughout the class.
sep490-idp/src/main/java/sep490/idp/aop/UserRepositoryAdvice.java
Outdated
Show resolved
Hide resolved
GiaBaorr
force-pushed
the
authorize-funny
branch
from
74386c3 to
cd7c364
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
sep490-idp/src/main/java/green_buildings/idp/aop/UserRepositoryAdvice.java (1)
33-50: Ensure robust handling of absent enterprise ID.
Currently,SecurityUtils.getCurrentUserEnterpriseId().orElseThrow()will throw if no enterprise ID is available. This is a legitimate approach, but consider adding a more descriptive exception or logging to simplify troubleshooting in scenarios where an enterprise ID is unexpectedly absent.sep490-idp/src/main/java/green_buildings/idp/repository/UserRepositoryAdapter.java (1)
16-18: Add handling or logging for empty ID sets.
Ifidsis empty,findByIDswill likely return an empty list. While this is generally fine, consider logging or short-circuiting to avoid unnecessary repository calls and to improve traceability.public List<UserEntity> findByIDs(Set<UUID> ids) { + if (ids.isEmpty()) { + return Collections.emptyList(); + } return userRepo.findByIDs(ids); }sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java (1)
192-192: Consider using adapter for saving users.For consistency, consider delegating the save operation to the adapter as well, similar to how retrieval is handled.
- userRepo.saveAll(users); + userRepositoryAdapter.saveAll(users);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sep490-idp/src/main/java/green_buildings/idp/aop/UserRepositoryAdvice.java(1 hunks)sep490-idp/src/main/java/green_buildings/idp/entity/UserEntity.java(3 hunks)sep490-idp/src/main/java/green_buildings/idp/repository/UserRepositoryAdapter.java(1 hunks)sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Java Modules (sep490-enterprise)
🔇 Additional comments (5)
sep490-idp/src/main/java/green_buildings/idp/aop/UserRepositoryAdvice.java (1)
32-48: Consider verifying filter applicability for non-session contexts.
The code correctly checks for a non-null session and only then applies/disables the filter. Verify that all necessary repository operations will have a valid session; otherwise, logic may vary in different application flows.sep490-idp/src/main/java/green_buildings/idp/entity/UserEntity.java (2)
36-38: Validate existence of the 'enterprise_users' table and columns.
The filter condition references the subselect onenterprise_userswith fieldsuser_idandenterprise_id. Ensure that the naming and schema match exactly at runtime to avoid runtime exceptions or empty results.
47-49: Great use of descriptive constants.
The constants clearly communicate the filter and parameter names. This enhances readability and maintainability of the filtering logic.sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java (2)
21-21: LGTM! Clean dependency injection setup.The import and field addition for
UserRepositoryAdapterare properly implemented with constructor injection via@RequiredArgsConstructor.Also applies to: 58-58
179-179: LGTM! Good use of adapter for enterprise filtering.The change correctly delegates user retrieval to
UserRepositoryAdapter, which applies enterprise-level filtering through AOP.
Summary by CodeRabbit
New Features
Refactor