-
Notifications
You must be signed in to change notification settings - Fork 82
[HCMPRE-2741] central instance changes for egov user #740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces dynamic schema resolution for multi-tenant support across user-related repositories. SQL queries in repositories and query builders now use a schema placeholder, replaced at runtime using a new Changes
Sequence Diagram(s)sequenceDiagram
participant Service as UserService
participant Repo as UserRepository
participant Utils as DatabaseSchemaUtils
participant DB as Database
Service->>Repo: resetFailedLoginAttempts(tenantId, userUuid)
Repo->>Utils: replaceSchemaPlaceholder(query, tenantId)
Utils-->>Repo: query with schema replaced
Repo->>DB: Execute schema-specific query
Service->>Repo: insertFailedLoginAttempt(tenantId, attempt)
Repo->>Utils: replaceSchemaPlaceholder(query, tenantId)
Utils-->>Repo: query with schema replaced
Repo->>DB: Execute schema-specific query
sequenceDiagram
participant Repo as AddressRepository/RoleRepository/AuditRepository
participant Utils as DatabaseSchemaUtils
participant DB as Database
Repo->>Utils: replaceSchemaPlaceholder(query, tenantId)
Utils-->>Repo: query with schema replaced
Repo->>DB: Execute schema-specific query
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java (1)
210-213
:⚠️ Potential issueIncorrect regex – tenant extraction will never work
tenantId.split("//.")[0]
tries to split on the literal string"//."
, not a dot.
The intention is clearly to obtain the state-level part before'.'
, but the current code leavestenantId
unchanged and can break authorisation or schema resolution logic.- if(UserType.CITIZEN.equals(oldUser.getType()) && tenantId.contains(".")) - tenantId = tenantId.split("//.")[0]; + if (UserType.CITIZEN.equals(oldUser.getType()) && tenantId.contains(".")) { + /* split on a dot; escape it because dot is a regex meta-character */ + tenantId = tenantId.split("\\.")[0]; + }Please add a test covering both a two-level tenant (
pb.amritsar
) and a single-level tenant (in
) to ensure the logic behaves as expected.
🧹 Nitpick comments (12)
core-services/egov-user/src/main/resources/db/migrate.sh (2)
3-3
: Avoid echoing sensitive data to logs.
Printing the full DB URL, schema names, or credentials may leak secrets in CI/CD logs. Limit or mask these echos, or gate them behind a debug flag.Also applies to: 6-6, 9-9
7-11
: Improve schema iteration with IFS and proper quoting.
Instead of${schemas//,/ }
, useIFS
to split on commas and trim whitespace. Quote all expansions in theflyway
command for safety.# Replace the loop with: IFS=',' read -ra schema_array <<< "${SCHEMA_NAME}" for schemaname in "${schema_array[@]}"; do schemaname="${schemaname//[[:space:]]/}" flyway \ -url="${baseurl}?currentSchema=${schemaname}" \ -table="${SCHEMA_TABLE}" \ -user="${FLYWAY_USER}" \ -password="${FLYWAY_PASSWORD}" \ -locations="${FLYWAY_LOCATIONS}" \ -baselineOnMigrate=true \ -outOfOrder=true \ migrate doneThis approach handles edge cases (spaces, empty entries) and ensures each argument is safely quoted.
core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java (1)
5-17
: Consider co-locating the schema constant with the newDatabaseSchemaUtils
utilityAll new repositories now rely on
DatabaseSchemaUtils
for placeholder substitution, yet the constantSCHEMA_REPLACE_STRING
is still sourced fromMultiStateInstanceUtil
. Keeping the constant in a different utility couples the builders to an unrelated class and may be confusing onceMultiStateInstanceUtil
becomes obsolete.You can gradually move the constant to
DatabaseSchemaUtils
(or create a lightweight interface that re-exports it) and update the static imports in the builders.
This is a non-blocking clean-up but will make the intent of the code clearer and reduce accidental coupling.core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java (2)
91-97
: Variable name shadowsjava.util.Map
final Map<String, Object> Map = new HashMap<>();
re-uses the type name as a variable identifier, reducing readability and possibly confusing IDE tooling. Prefer a lower-camel-case name such asparams
orqueryParams
.-final Map<String, Object> Map = new HashMap<String, Object>(); +final Map<String, Object> params = new HashMap<>();
184-189
: Redundant import versus fully-qualified usage
isEmpty
is statically imported at the top (import static org.springframework.util.CollectionUtils.isEmpty;
) but inside this block you still callCollectionUtils.isEmpty(ids)
. Stick to one style to avoid confusion.No functional impact, but a quick clean-up promotes consistency.
core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java (7)
15-17
: Clean up commented-out annotationsThe class has commented-out annotations (@component and @AllArgsConstructor) that should either be removed completely or uncommented if needed. Commented code reduces readability and can confuse other developers about the intended configuration.
@Configuration -//@Component -//@AllArgsConstructor public class DatabaseSchemaUtils {
27-29
: Remove unnecessary null initializationThe
finalQuery
variable is initialized to null but is always assigned a value before being returned. This initialization is unnecessary.public String replaceSchemaPlaceholder(String query, String tenantId) { - String finalQuery = null; + String finalQuery; if (this.getIsEnvironmentCentralInstance()) {
41-45
: Consider consistent schema placeholder replacement logicThe current implementation uses different approaches for replacing the schema placeholder between central and non-central instances:
- For central instances, it replaces the placeholder with a schema name
- For non-central instances, it removes the placeholder AND the dot after it
This difference could lead to SQL syntax issues if queries aren't carefully constructed with this behavior in mind.
Consider making the replacement logic more consistent:
if (this.getIsEnvironmentCentralInstance()) { finalQuery = query.replaceAll("(?i)" + Pattern.quote(SCHEMA_REPLACE_STRING), schemaName); } else { - finalQuery = query.replaceAll("(?i)" + Pattern.quote(SCHEMA_REPLACE_STRING.concat(".")), ""); + // Just remove the schema placeholder itself, consistent with central instance approach + finalQuery = query.replaceAll("(?i)" + Pattern.quote(SCHEMA_REPLACE_STRING), ""); }Alternatively, if SQL queries are designed with the expectation that the dot will be removed in non-central instances, document this behavior clearly to prevent misunderstandings.
58-72
: Use StringBuilder for efficient string concatenationThe current implementation uses string concatenation in a loop which is inefficient. Consider using StringBuilder for better performance, especially if handling large numbers of tenants.
public String getStateLevelTenant(String tenantId) { String[] tenantArray = tenantId.split("\\."); - String stateTenant = tenantArray[0]; + StringBuilder stateTenant = new StringBuilder(tenantArray[0]); if (this.getIsEnvironmentCentralInstance()) { if (this.getStateLevelTenantIdLength() < tenantArray.length) { for(int i = 1; i < this.getStateLevelTenantIdLength(); ++i) { - stateTenant = stateTenant.concat(".").concat(tenantArray[i]); + stateTenant.append(".").append(tenantArray[i]); } } else { - stateTenant = tenantId; + return tenantId; } } - return stateTenant; + return stateTenant.toString(); }
30-32
: Improve error message for better troubleshootingThe current error message when a tenant ID is too short could be more helpful by including the actual values in the message.
if (this.stateSchemaIndexPositionInTenantId >= tenantId.length()) { - throw new CustomException(INVALID_TENANT_ID_ERR_CODE, "The tenantId length is smaller than the defined schema index in tenantId for central instance"); + throw new CustomException(INVALID_TENANT_ID_ERR_CODE, + String.format("The tenantId '%s' length (%d) is smaller than the defined schema index (%d) for central instance", + tenantId, tenantId.length(), this.stateSchemaIndexPositionInTenantId)); }
1-109
: Consider adding JavaDoc documentationThis utility class contains complex logic for schema management and tenant processing, but lacks comprehensive documentation. Adding JavaDoc comments would improve maintainability and help other developers understand how to use this class correctly.
Add class-level and method-level JavaDoc documentation explaining:
- The purpose of the class
- What each method does
- Parameter descriptions
- Return value descriptions
- Exception conditions
- Examples of usage for complex methods
Example for the class:
/** * Utility class for handling database schema operations in multi-tenant environments. * This class provides methods to dynamically resolve schemas based on tenant IDs, * replacing schema placeholders in SQL queries and determining tenant hierarchy levels. * <p> * The behavior varies between central and non-central instances: * - In central instances, tenant IDs contain schema information at configurable positions * - In non-central instances, schema prefixes are removed from queries */ @Configuration public class DatabaseSchemaUtils { // ... }
19-19
: Make SCHEMA_REPLACE_STRING final for immutabilityThe
SCHEMA_REPLACE_STRING
constant should be declared asfinal
since it's not intended to be modified.-public static String SCHEMA_REPLACE_STRING = "{schema}"; +public static final String SCHEMA_REPLACE_STRING = "{schema}";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
core-services/egov-user/src/main/java/org/egov/user/config/UserServiceConstants.java
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/domain/service/UserService.java
(3 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java
(8 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java
(3 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/RoleRepository.java
(6 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java
(16 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java
(4 hunks)core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java
(1 hunks)core-services/egov-user/src/main/resources/db/migrate.sh
(1 hunks)core-services/egov-user/src/test/java/org/egov/user/persistence/repository/AddressRepositoryTest.java
(2 hunks)core-services/egov-user/src/test/java/org/egov/user/persistence/repository/UserRepositoryTest.java
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java (2)
core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java (1)
Component
(7-14)core-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java (1)
Component
(56-308)
🔇 Additional comments (22)
core-services/egov-user/src/main/java/org/egov/user/config/UserServiceConstants.java (1)
73-73
: Addition of INVALID_TENANT_ID_ERR_CODE constant looks good.This constant provides a standardized error code for tenant validation scenarios, which aligns well with the multi-tenant schema support being implemented across the codebase.
core-services/egov-user/src/test/java/org/egov/user/persistence/repository/UserRepositoryTest.java (1)
16-16
: DatabaseSchemaUtils dependency integration looks good.The test has been appropriately updated to inject the new
DatabaseSchemaUtils
dependency and pass it to theUserRepository
constructor, reflecting the changes made to support dynamic schema resolution based on tenant IDs.Also applies to: 71-73, 91-93
core-services/egov-user/src/test/java/org/egov/user/persistence/repository/AddressRepositoryTest.java (1)
5-5
: DatabaseSchemaUtils dependency integration looks good.The
AddressRepositoryTest
has been correctly updated to autowire theDatabaseSchemaUtils
dependency and pass it to the repository constructor, maintaining consistency with other repository tests.Also applies to: 36-38, 44-44
core-services/egov-user/src/main/java/org/egov/user/domain/service/UserService.java (3)
503-504
: Tenant ID now correctly passed to resetFailedLoginAttemptsForUser methodThe change correctly passes the tenant ID to the repository method, enabling proper schema resolution in a multi-tenant environment.
542-544
: Enhanced fetchFailedAttemptsByUserAndTime call with tenant contextThe repository method now receives tenant ID as its first parameter, allowing for proper schema selection in the multi-tenant database environment. This ensures user login attempts are fetched from the correct tenant schema.
561-562
: Added tenant ID to insertFailedLoginAttempt method callThe repository method now correctly receives tenant ID as the first parameter, ensuring failed login attempts are recorded in the appropriate tenant schema.
core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java (3)
5-5
: Added import for schema replacement utilityCorrectly imports the SCHEMA_REPLACE_STRING constant needed for dynamic schema resolution.
10-10
: Applied schema placeholder to GET_ADDRESSBY_IDAND_TENANT queryThe query now includes the schema placeholder prefix that will be dynamically replaced at runtime based on tenant ID.
12-12
: Applied schema placeholder to DELETE_ADDRESS queryThe query now includes the schema placeholder prefix that will be dynamically replaced at runtime based on tenant ID.
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java (6)
4-4
: Added import for schema replacement utilityCorrectly imports the SCHEMA_REPLACE_STRING constant needed for dynamic schema resolution.
19-19
: Added import for DatabaseSchemaUtilsThe import for the new utility class that will handle schema resolution logic.
27-28
: Applied schema placeholder to INSERT_AUDIT_DETAILS queryThe query now includes the schema placeholder prefix that will be dynamically replaced at runtime based on tenant ID.
32-33
: Added DatabaseSchemaUtils fieldAdded field to store the schema utilities instance, which will be used for schema resolution.
35-38
: Updated constructor to include DatabaseSchemaUtilsConstructor now accepts and initializes the DatabaseSchemaUtils dependency for schema resolution.
152-154
: Implemented dynamic schema resolutionThe query is now processed through DatabaseSchemaUtils to replace the schema placeholder with the actual schema based on the tenant ID before execution.
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/RoleRepository.java (6)
25-25
: Added import for DatabaseSchemaUtilsThe import for the new utility class that will handle schema resolution logic.
48-48
: Added DatabaseSchemaUtils fieldAdded field to store the schema utilities instance, which will be used for schema resolution.
66-71
: Updated constructor to include DatabaseSchemaUtilsConstructor now accepts and initializes the DatabaseSchemaUtils dependency for schema resolution.
85-88
: Applied schema resolution to GET_ROLES_BY_ID_TENANTID queryThe query is now processed through DatabaseSchemaUtils to replace the schema placeholder with the actual schema before execution.
102-104
: Applied schema resolution to GET_ROLES_BY_ROLEIDS queryThe query is now processed through DatabaseSchemaUtils to replace the schema placeholder with the actual schema before execution.
122-125
: Applied schema resolution to GET_ROLE_BYTENANT_ANDCODE queryThe query is now processed through DatabaseSchemaUtils to replace the schema placeholder with the actual schema before execution.
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java (1)
451-453
: Double-checkgetStateLevelTenant
implementation
databaseSchemaUtils.getStateLevelTenant()
is invoked here, but its contract is not visible in this diff.
Verify that it:
- Handles
null
/empty input gracefully.- Correctly extracts the state-level code for tenants both with and without a dot.
If the method is unavailable or implemented differently, this call will fail or return an unexpected value, leading to MDMS role look-ups against the wrong schema.
...-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java
Outdated
Show resolved
Hide resolved
...services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java
Outdated
Show resolved
Hide resolved
core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
13-14
: Remove unused imports.The imports for
MaskingService
andMdmsFetcher
appear to be unused in this file. They should be removed to avoid confusion.-import org.egov.encryption.masking.MaskingService; -import org.egov.encryption.util.MdmsFetcher;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java
(2 hunks)core-services/egov-user/src/test/java/org/egov/user/web/controller/UserControllerTest.java
(1 hunks)core-services/egov-user/src/test/java/org/egov/user/web/controller/UserRequestControllerTest.java
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
core-services/egov-user/src/test/java/org/egov/user/web/controller/UserControllerTest.java (2)
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
Ignore
(34-100)core-services/egov-user/src/test/java/org/egov/user/web/controller/UserRequestControllerTest.java (1)
Ignore
(47-210)
core-services/egov-user/src/test/java/org/egov/user/web/controller/UserRequestControllerTest.java (2)
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
Ignore
(34-100)core-services/egov-user/src/test/java/org/egov/user/web/controller/UserControllerTest.java (1)
Ignore
(65-485)
🔇 Additional comments (3)
core-services/egov-user/src/test/java/org/egov/user/web/controller/UserControllerTest.java (1)
65-65
: Ensure this test class doesn't remain ignored long-term.The addition of the
@Ignore
annotation at the class level disables all tests in this class. While this may be necessary during the implementation of multi-tenant schema support, consider documenting why these tests are ignored and create a follow-up task to re-enable them once the schema changes are fully implemented.core-services/egov-user/src/test/java/org/egov/user/web/controller/UserRequestControllerTest.java (1)
47-47
: Consider adding a comment explaining why the test class is ignored.Adding
@Ignore
at the class level disables all tests in this controller. This appears to be part of a coordinated effort to temporarily disable controller tests during the implementation of multi-tenant schema support. To improve maintainability, add a comment explaining the rationale and link to any relevant issue tracking the re-enabling of these tests.core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
34-34
: Document the reason for disabling tests.Similar to the other controller test classes, this class has been annotated with
@Ignore
to skip all tests. Add a comment explaining why these tests are temporarily disabled and how/when they will be re-enabled once the multi-tenant schema implementation is complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
core-services/egov-user/src/main/java/org/egov/user/web/contract/UserRequest.java
(2 hunks)core-services/egov-user/src/main/java/org/egov/user/web/contract/UserSearchRequest.java
(2 hunks)
🔇 Additional comments (2)
core-services/egov-user/src/main/java/org/egov/user/web/contract/UserRequest.java (1)
19-19
: Clean import addition.Good addition of the
@NotBlank
validation constraint import that will be used for tenant ID validation.core-services/egov-user/src/main/java/org/egov/user/web/contract/UserSearchRequest.java (1)
12-12
: Clean import addition.Good addition of the
@NotBlank
validation constraint import that will be used for tenant ID validation.
core-services/egov-user/src/main/java/org/egov/user/web/contract/UserRequest.java
Outdated
Show resolved
Hide resolved
core-services/egov-user/src/main/java/org/egov/user/web/contract/UserSearchRequest.java
Outdated
Show resolved
Hide resolved
core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java
Show resolved
Hide resolved
...services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
core-services/egov-user/pom.xml
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java
(8 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java
(3 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/RoleRepository.java
(6 hunks)core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java
(16 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java
(1 hunks)core-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java
(4 hunks)core-services/egov-user/src/main/java/org/egov/user/security/oauth2/custom/authproviders/CustomAuthenticationProvider.java
(2 hunks)core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- core-services/egov-user/src/main/java/org/egov/user/security/oauth2/custom/authproviders/CustomAuthenticationProvider.java
- core-services/egov-user/pom.xml
🚧 Files skipped from review as they are similar to previous changes (7)
- core-services/egov-user/src/main/java/org/egov/user/persistence/repository/RoleRepository.java
- core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java
- core-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java
- core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java
- core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java
- core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java
- core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java
🔇 Additional comments (11)
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java (11)
36-36
: LGTM: Clean dependency injection pattern for DatabaseSchemaUtils.The introduction of
DatabaseSchemaUtils
as a dependency is well-implemented with proper field declaration and constructor injection.Also applies to: 53-53, 64-68
89-89
: LGTM: Consistent schema replacement pattern for user search.The extraction of tenantId and subsequent schema placeholder replacement follows the expected pattern for multi-tenant support.
Also applies to: 111-112
131-132
: LGTM: Schema replacement applied to role search query.The pattern is consistently applied to the user role search functionality.
156-157
: LGTM: Schema replacement for user presence check.The tenant-specific schema resolution is properly applied to the user existence validation query.
330-332
: LGTM: Schema replacement for user updates.The update query properly uses schema replacement with the extracted tenantId.
342-343
: LGTM: Method signatures updated for tenant-aware failed login operations.The failed login attempt methods now properly accept tenantId parameters, enabling tenant-specific schema resolution for security-related operations.
Also applies to: 346-346, 365-365, 378-378
358-361
: LGTM: Schema replacement applied to failed login attempt queries.All failed login attempt database operations now use proper schema replacement, maintaining consistency with the multi-tenant architecture.
Also applies to: 371-373, 379-382
459-459
: LGTM: Updated to use new schema utility for state-level tenant.The replacement of the old multi-state utility with
databaseSchemaUtils.getStateLevelTenant()
aligns with the new schema management approach.
487-489
: LGTM: Schema replacement applied to batch operations.User role insertions, user creation, and role updates all properly use schema replacement, ensuring tenant isolation for these critical operations.
Also applies to: 579-581, 624-626
586-596
: LGTM: Enhanced documentation and schema support for sequence generation.The
getNextSequence
method is well-documented and properly implements tenant-specific schema resolution for sequence generation.
171-171
:✅ Verification successful
Verify that all callers pass the required tenantId parameter.
The
getNextSequence
method signature change requires tenantId. Ensure all calling code has been updated to provide this parameter.
🏁 Script executed:
#!/bin/bash # Search for calls to getNextSequence to verify they pass tenantId ast-grep --pattern 'getNextSequence($_)'Length of output: 366
All calls to getNextSequence now pass tenantId
Ran an AST search for
getNextSequence(...)
across the codebase and found only two usages—all supplying a tenantId argument:
UserRepository.java:171
→getNextSequence(user.getTenantId())
AddressRepository.java:60
→getNextSequence(tenantId)
No callers remain without the required parameter.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests