Hcmpre 2655 single instance multi tenant #768
Conversation
…5-Single-instance-multi-tenant
…55-Single-instance-multi-tenant
…55-Single-instance-multi-tenant
…PRE-2655-Single-instance-multi-tenant
WalkthroughThis update introduces tenant-specific database schema support across the OTP, User, and MDMS core services. It adds dynamic schema resolution utilities, refactors repositories and services to use schema placeholders, and updates migration scripts to support multi-schema environments. Method signatures and constructors are revised to include tenant context, and related tests are adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant UserService
participant DatabaseSchemaUtils
participant DB
UserService->>DatabaseSchemaUtils: replaceSchemaPlaceholder(query, tenantId)
DatabaseSchemaUtils-->>UserService: query with schema replaced
UserService->>DB: Execute query on tenant-specific schema
sequenceDiagram
participant Producer
participant MultiStateInstanceUtil
participant Kafka
Producer->>MultiStateInstanceUtil: getStateSpecificTopicName(tenantId, topic)
MultiStateInstanceUtil-->>Producer: tenant-specific topic
Producer->>Kafka: push(message to tenant-specific topic)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
core-services/egov-otp/src/main/resources/db/migrate.sh (1)
1-12: Add error handling and validation to the migration script.The script lacks error handling for missing environment variables and schema name validation, which could cause issues in production deployments.
#!/bin/bash +set -e # Exit on any error + +# Validate required environment variables +if [ -z "$DB_URL" ] || [ -z "$SCHEMA_NAME" ] || [ -z "$FLYWAY_USER" ] || [ -z "$FLYWAY_PASSWORD" ]; then + echo "Error: Required environment variables are missing" + echo "Required: DB_URL, SCHEMA_NAME, FLYWAY_USER, FLYWAY_PASSWORD" + exit 1 +fi + baseurl=$DB_URL echo "the baseurl : $DB_URL" schemasetter="?currentSchema=" schemas=$SCHEMA_NAME echo "the schemas : $schemas" + +# Validate that schemas is not empty +if [ -z "$schemas" ]; then + echo "Error: SCHEMA_NAME cannot be empty" + exit 1 +fi + for schemaname in ${schemas//,/ } do + # Trim whitespace from schema name + schemaname=$(echo "$schemaname" | xargs) + if [ -z "$schemaname" ]; then + echo "Warning: Skipping empty schema name" + continue + fi echo "the schema name : ${baseurl}${schemasetter}${schemaname}" flyway -url=${baseurl}${schemasetter}${schemaname} -table=$SCHEMA_TABLE -user=$FLYWAY_USER -password=$FLYWAY_PASSWORD -locations=$FLYWAY_LOCATIONS -baselineOnMigrate=true -outOfOrder=true migrate + if [ $? -ne 0 ]; then + echo "Error: Migration failed for schema: $schemaname" + exit 1 + fi done
♻️ Duplicate comments (1)
core-services/egov-user/src/main/resources/db/migrate.sh (1)
1-12: Add error handling and validation (same as egov-otp script).This script has the same implementation as the egov-otp migrate.sh script and therefore has the same issues with missing error handling and validation. Apply the same improvements to ensure robust migration execution.
#!/bin/bash +set -e # Exit on any error + +# Validate required environment variables +if [ -z "$DB_URL" ] || [ -z "$SCHEMA_NAME" ] || [ -z "$FLYWAY_USER" ] || [ -z "$FLYWAY_PASSWORD" ]; then + echo "Error: Required environment variables are missing" + echo "Required: DB_URL, SCHEMA_NAME, FLYWAY_USER, FLYWAY_PASSWORD" + exit 1 +fi + baseurl=$DB_URL echo "the baseurl : $DB_URL" schemasetter="?currentSchema=" schemas=$SCHEMA_NAME echo "the schemas : $schemas" + +# Validate that schemas is not empty +if [ -z "$schemas" ]; then + echo "Error: SCHEMA_NAME cannot be empty" + exit 1 +fi + for schemaname in ${schemas//,/ } do + # Trim whitespace from schema name + schemaname=$(echo "$schemaname" | xargs) + if [ -z "$schemaname" ]; then + echo "Warning: Skipping empty schema name" + continue + fi echo "the schema name : ${baseurl}${schemasetter}${schemaname}" flyway -url=${baseurl}${schemasetter}${schemaname} -table=$SCHEMA_TABLE -user=$FLYWAY_USER -password=$FLYWAY_PASSWORD -locations=$FLYWAY_LOCATIONS -baselineOnMigrate=true -outOfOrder=true migrate + if [ $? -ne 0 ]; then + echo "Error: Migration failed for schema: $schemaname" + exit 1 + fi doneConsider creating a shared migration script to avoid duplication across services.
🧹 Nitpick comments (10)
core-services/egov-enc-service/README.md (2)
77-77: Typo: Use 'i.e.' instead of 'ie.'
Apply:- ie. `egov_enc_service_schema` + i.e. `egov_enc_service_schema`
69-92: Clarify schema context in manual migration steps.
The current instructions referencepublic.egov_enc_service_schema, which may confuse users on non-public schemas. Consider parameterizing or explicitly documenting use of${SCHEMA_NAME}for the schema:SELECT * FROM ${SCHEMA_NAME}.${SCHEMA_TABLE}; DELETE FROM ${SCHEMA_NAME}.${SCHEMA_TABLE};This will prevent confusion in multi-tenant setups.
core-services/egov-otp/pom.xml (1)
119-123: Consider version consistency and production stability.The added
services-commondependency uses version2.9.0-SNAPSHOTwhile the existingtracerdependency uses2.9.1-SNAPSHOT. Consider:
- Version Alignment: Ensure consistent versioning across related eGov dependencies to avoid compatibility issues.
- Snapshot Dependencies: Using SNAPSHOT versions in production can introduce instability. Consider using stable release versions when available.
Consider aligning the version with the tracer dependency:
- <version>2.9.0-SNAPSHOT</version> + <version>2.9.1-SNAPSHOT</version>core-services/egov-user/src/main/java/org/egov/user/config/UserServiceConstants.java (1)
73-73: Consider centralizing duplicate error codes.The same constant
INVALID_TENANT_ID_ERR_CODE = "INVALID_TENANT_ID"is defined in both this class andcore-services/mdms-v2/src/main/java/org/egov/infra/mdms/errors/ErrorCodes.java. This duplication could lead to maintenance issues.Consider:
- Centralization: Move common error codes to the
services-commonmodule- Shared Constants: Create a shared constants class for cross-module error codes
- Documentation: If duplication is intentional for module independence, document the rationale
Verify if there are other duplicate constants across modules:
#!/bin/bash # Search for duplicate constant definitions across the codebase rg "INVALID_TENANT_ID_ERR_CODE" --type java -A 1 -B 1core-services/egov-user/src/test/java/org/egov/user/web/controller/UserRequestControllerTest.java (1)
47-47: Track re-enabling of disabled tests.The entire test class has been disabled with
@Ignore, which reduces test coverage during the multi-tenant refactoring. Please ensure this is temporary and create a follow-up task to re-enable these tests once the multi-tenant changes are stable.#!/bin/bash # Description: Check for other @Ignore annotations in test files to assess scope of disabled tests # Find all test files with @Ignore annotations rg -A 2 -B 2 "@Ignore" --type java --glob "*Test*.java"Would you like me to help create a tracking issue for re-enabling the disabled tests?
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
5-5: Clean up unused imports.The imports for
when,MaskingService, andMdmsFetcherappear to be unused in the current test implementation. Consider removing them to keep the imports clean.-import static org.mockito.Mockito.when; -import org.egov.encryption.masking.MaskingService; -import org.egov.encryption.util.MdmsFetcher;Also applies to: 12-13, 19-19
core-services/mdms-v2/src/main/resources/db/migrate.sh (1)
1-11: Enhance script robustness with error handling.The multi-schema migration script is well-implemented with good debugging output. However, consider adding error handling for missing environment variables and proper exit codes for failed migrations.
#!/bin/bash +set -e # Exit on any error + +# Validate required environment variables +if [[ -z "$DB_URL" || -z "$SCHEMA_NAME" || -z "$FLYWAY_USER" || -z "$FLYWAY_PASSWORD" ]]; then + echo "Error: Required environment variables are missing" + echo "Required: DB_URL, SCHEMA_NAME, FLYWAY_USER, FLYWAY_PASSWORD" + exit 1 +fi + baseurl=$DB_URL echo "the baseurl : $DB_URL" schemasetter="?currentSchema=" schemas=$SCHEMA_NAME echo "the schemas : $schemas" + +failed_schemas=() for schemaname in ${schemas//,/ } do echo "the schema name : ${baseurl}${schemasetter}${schemaname}" - flyway -url=${baseurl}${schemasetter}${schemaname} -table=$SCHEMA_TABLE -user=$FLYWAY_USER -password=$FLYWAY_PASSWORD -locations=$FLYWAY_LOCATIONS -baselineOnMigrate=true -outOfOrder=true migrate + if ! flyway -url="${baseurl}${schemasetter}${schemaname}" -table="$SCHEMA_TABLE" -user="$FLYWAY_USER" -password="$FLYWAY_PASSWORD" -locations="$FLYWAY_LOCATIONS" -baselineOnMigrate=true -outOfOrder=true migrate; then + failed_schemas+=("$schemaname") + echo "Migration failed for schema: $schemaname" + fi done + +if [[ ${#failed_schemas[@]} -gt 0 ]]; then + echo "Migration failed for schemas: ${failed_schemas[*]}" + exit 1 +fi + +echo "All schema migrations completed successfully"core-services/egov-user/src/main/java/org/egov/user/security/oauth2/custom/authproviders/CustomAuthenticationProvider.java (1)
56-56: Consider renaming the field to reflect its new purpose.The field name
centraInstanceUtil(with typo "centra" instead of "central") doesn't align with the newDatabaseSchemaUtilstype. Consider renaming it to something more descriptive likedatabaseSchemaUtilsorschemaUtils.- private DatabaseSchemaUtils centraInstanceUtil; + private DatabaseSchemaUtils databaseSchemaUtils;Don't forget to update the usage at line 93 accordingly.
core-services/egov-user/src/test/java/org/egov/user/persistence/repository/AddressRepositoryTest.java (1)
36-37: Consider using mock objects for better test isolation.While the dependency injection is correctly updated to match the new constructor signature, for unit testing it's generally better practice to use
@Mockand@MockBeanannotations instead of@Autowiredfor utility dependencies likeDatabaseSchemaUtils. This provides better test isolation and faster execution.- @Autowired - private DatabaseSchemaUtils databaseSchemaUtils; + @Mock + private DatabaseSchemaUtils databaseSchemaUtils;And in the constructor call:
- addressRepository = new AddressRepository(namedParameterJdbcTemplate, jdbcTemplate, databaseSchemaUtils); + addressRepository = new AddressRepository(namedParameterJdbcTemplate, jdbcTemplate, databaseSchemaUtils);Also applies to: 44-44
core-services/egov-user/src/test/java/org/egov/user/persistence/repository/UserRepositoryTest.java (1)
71-72: Consider using mock objects for better test isolation.Similar to other test classes in this PR, consider using
@Mockinstead of@Autowiredfor theDatabaseSchemaUtilsdependency to improve test isolation and performance.- @Autowired - private DatabaseSchemaUtils databaseSchemaUtils; + @Mock + private DatabaseSchemaUtils databaseSchemaUtils;Also applies to: 91-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
core-services/egov-enc-service/CHANGELOG.md(1 hunks)core-services/egov-enc-service/README.md(1 hunks)core-services/egov-enc-service/pom.xml(1 hunks)core-services/egov-enc-service/src/main/resources/db/migration/main/V20180607185601__eg_enc.sql(0 hunks)core-services/egov-enc-service/src/main/resources/db/migration/main/V20250519185601__eg_enc.sql(1 hunks)core-services/egov-otp/pom.xml(1 hunks)core-services/egov-otp/src/main/java/org/egov/OtpApplication.java(2 hunks)core-services/egov-otp/src/main/java/org/egov/domain/service/TokenService.java(0 hunks)core-services/egov-otp/src/main/java/org/egov/persistence/repository/TokenRepository.java(4 hunks)core-services/egov-otp/src/main/resources/db/migrate.sh(1 hunks)core-services/egov-otp/src/test/java/org/egov/persistence/repository/TokenRepositoryTest.java(2 hunks)core-services/egov-user/pom.xml(1 hunks)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/model/UpdateRequest.java(2 hunks)core-services/egov-user/src/main/java/org/egov/user/domain/service/UserService.java(7 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(15 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)core-services/egov-user/src/main/java/org/egov/user/web/contract/UserRequest.java(4 hunks)core-services/egov-user/src/main/java/org/egov/user/web/contract/UserSearchRequest.java(2 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)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)core-services/mdms-v2/CHANGELOG.md(1 hunks)core-services/mdms-v2/pom.xml(2 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/errors/ErrorCodes.java(1 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/producer/Producer.java(1 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/MdmsDataRepositoryImpl.java(5 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/SchemaDefinitionDbRepositoryImpl.java(4 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java(1 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilderV2.java(1 hunks)core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/SchemaDefinitionQueryBuilder.java(2 hunks)core-services/mdms-v2/src/main/resources/db/migrate.sh(1 hunks)
💤 Files with no reviewable changes (2)
- core-services/egov-otp/src/main/java/org/egov/domain/service/TokenService.java
- core-services/egov-enc-service/src/main/resources/db/migration/main/V20180607185601__eg_enc.sql
🧰 Additional context used
🪛 LanguageTool
core-services/egov-enc-service/README.md
[uncategorized] ~77-~77: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...ariable SCHEMA_TABLE for the service. ie. egov_enc_service_schema 2. Check the ...
(I_E)
🔇 Additional comments (67)
core-services/egov-user/pom.xml (2)
49-49: Bump services-common dependency version
Aligns egov-user with tenant-aware schema support introduced in services-common 2.0.0-SNAPSHOT.
54-54: Upgrade enc-client dependency version
Ensures compatibility with the updated encryption client (2.0.4-SNAPSHOT) that supports multi-tenant schema handling.core-services/egov-enc-service/pom.xml (1)
12-12: Bump project version. Updated to 2.10.0-SNAPSHOT to reflect the new release with multi-schema migration support and Flyway fix.core-services/egov-enc-service/CHANGELOG.md (1)
6-8: Changelog entry for 2.10.0 looks good. Documents the Flyway migration fix and manual deletion step clearly.core-services/egov-enc-service/README.md (1)
67-68: Add 'NA' for Kafka Producers section. Matches the Kafka Consumers section indicating no producers.core-services/egov-enc-service/src/main/resources/db/migration/main/V20250519185601__eg_enc.sql (2)
1-14: Defineeg_enc_symmetric_keyswith unique constraints.
The table and indexes are properly declared withIF NOT EXISTS. Ensure Flyway is pointed at the correct tenant schema so these tables are created in the intended context.
15-28: Defineeg_enc_asymmetric_keyswith unique constraints.
As above, this DDL correctly sets up the table structure and constraints. UsingIF NOT EXISTSprotects re-runs.core-services/mdms-v2/src/main/java/org/egov/infra/mdms/errors/ErrorCodes.java (1)
28-28: LGTM! Good centralized error handling.The addition of
INVALID_TENANT_ID_ERR_CODEfollows the established pattern and supports proper error handling for tenant validation in the multi-tenant architecture.core-services/egov-otp/src/main/java/org/egov/OtpApplication.java (2)
8-8: LGTM! Proper import for multi-tenant support.The import of
MultiStateInstanceUtilis correctly added to support tenant-aware schema resolution.
27-27: LGTM! Correct Spring configuration for multi-tenant support.Adding
MultiStateInstanceUtil.classto the@Importannotation properly integrates the utility into the Spring context for tenant-aware operations.core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilderV2.java (2)
13-13: LGTM: Proper import for schema placeholder constant.The import for
SCHEMA_REPLACE_STRINGfromMultiStateInstanceUtilis correctly added to support dynamic schema resolution in multi-tenant environments.
21-22: LGTM: Correct implementation of schema placeholder in SQL query.The SQL query has been properly updated to use the schema placeholder pattern (
SCHEMA_REPLACE_STRING + ".eg_mdms_data") instead of the hardcoded table name. This enables tenant-specific schema resolution at runtime, which is essential for multi-tenant support.core-services/egov-user/src/main/java/org/egov/user/domain/model/UpdateRequest.java (2)
187-195: LGTM: Proper tenant ID propagation to permanent address.The addition of
.tenantId(tenantId)ensures that the permanent address domain object carries the tenant context, which is essential for proper multi-tenant schema resolution in downstream operations.
197-205: LGTM: Consistent tenant ID propagation to correspondence address.The tenant ID is correctly propagated to the correspondence address, maintaining consistency with the permanent address implementation and ensuring proper multi-tenant support.
core-services/egov-user/src/test/java/org/egov/user/web/controller/PasswordControllerTest.java (1)
33-33: Track re-enabling of disabled test class.Similar to other test classes in this PR, this entire test class has been disabled with
@Ignore. Please ensure there's a plan to re-enable these tests after the multi-tenant refactoring is complete to maintain test coverage.core-services/mdms-v2/pom.xml (2)
14-14: Verify the major version bump is intentional.The project version has been bumped from
1.3.3-SNAPSHOTto2.10.0-SNAPSHOT, which is a significant jump. Please confirm this major version increase is intentional and aligns with the multi-tenant feature introduction.
108-108: Verify services-common dependency version compatibility.The
services-commondependency has been updated from2.0.0-SNAPSHOTto2.9.0-SNAPSHOT. This update likely includes theMultiStateInstanceUtilclass used for schema resolution. Please ensure this version is available and compatible with the multi-tenant features being implemented.#!/bin/bash # Description: Verify the services-common dependency version exists in the repository # Check if the version exists in the Maven repository curl -s "https://nexus-repo.digit.org/nexus/content/repositories/snapshots/org/egov/services/services-common/2.9.0-SNAPSHOT/" | grep -q "maven-metadata.xml" if [ $? -eq 0 ]; then echo "✓ services-common 2.9.0-SNAPSHOT found in repository" else echo "✗ services-common 2.9.0-SNAPSHOT not found in repository" ficore-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java (2)
9-9: LGTM: Schema placeholder import added correctly.The import of
SCHEMA_REPLACE_STRINGfromMultiStateInstanceUtilis consistent with the multi-tenant schema support implementation.
15-15: Verify schema placeholder replacement mechanism.The SQL query correctly uses the schema placeholder instead of a hardcoded schema name. However, ensure that the repository layer properly replaces this placeholder with the actual schema name at runtime.
#!/bin/bash # Description: Verify that MdmsDataRepositoryImpl or related classes handle schema placeholder replacement # Search for classes that use this query and implement schema replacement ast-grep --pattern 'class $_ { $$$ SEARCH_MDMS_DATA_QUERY $$$ }' # Also search for schema replacement logic in repository implementations rg -A 10 "SCHEMA_REPLACE_STRING" --type javacore-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/SchemaDefinitionQueryBuilder.java (2)
12-12: LGTM: Consistent schema placeholder import.The import is consistent with the multi-tenant schema support pattern implemented across MDMS query builders.
22-22: Good: Schema placeholder usage is consistent.The schema placeholder replacement in the SQL query follows the same pattern as other query builders, ensuring consistent multi-tenant support.
core-services/egov-user/src/main/java/org/egov/user/web/contract/UserSearchRequest.java (2)
12-12: LGTM: NotNull validation import added.The import for
@NotNullannotation is correctly added to support tenant ID validation.
63-63: Good: Tenant ID validation enforced with clear message.The
@NotNullannotation with a descriptive error message ensures that user search requests must include a valid tenant ID, which is essential for multi-tenant support.core-services/egov-user/src/main/java/org/egov/user/web/contract/UserRequest.java (3)
19-19: LGTM: NotNull validation import added.The import for validation constraint is correctly added to support tenant ID validation.
135-135: Good: Tenant ID validation enforced.The
@NotNullannotation with a clear error message ensures tenant ID presence in user requests, which is essential for multi-tenant operations.
281-281: Excellent: Tenant context propagated to Address objects.Setting the
tenantIdwhen building Address domain objects ensures tenant context is maintained throughout the data model, which is crucial for multi-tenant schema resolution.Also applies to: 291-291
core-services/egov-user/src/main/java/org/egov/user/security/oauth2/custom/authproviders/CustomAuthenticationProvider.java (2)
27-27: Update import statement looks good.The import has been correctly updated from
MultiStateInstanceUtiltoDatabaseSchemaUtilsas part of the refactoring effort.
93-93: ```shell
#!/bin/bashLocate declaration of isEnvironmentCentralInstance field and its type
rg -n "isEnvironmentCentralInstance" -n core-services/libraries/services-common/src/main/java/org/egov/common/utils/MultiStateInstanceUtil.java -C2
Locate constructor signature for MultiStateInstanceUtil
rg -n "public MultiStateInstanceUtil" -n core-services/libraries/services-common/src/main/java/org/egov/common/utils/MultiStateInstanceUtil.java -C2
</details> <details> <summary>core-services/egov-otp/src/test/java/org/egov/persistence/repository/TokenRepositoryTest.java (3)</summary> `12-12`: **Import statements updated correctly.** The new imports for `MultiStateInstanceUtil` and `OtpConfiguration` are properly added to support the updated test setup. Also applies to: 18-18 --- `36-41`: **Test dependencies correctly added.** The new dependencies are properly injected using `@Autowired` annotation, which is appropriate for test classes. --- `47-47`: ```bash #!/bin/bash # Locate the TokenRepository source file file=$(find core-services/egov-otp/src -name TokenRepository.java | head -n1) echo "Inspecting: $file" # Show class declaration and all constructors grep -nE "class TokenRepository|TokenRepository\(" "$file"core-services/egov-user/src/main/java/org/egov/user/repository/builder/RoleQueryBuilder.java (3)
4-4: Schema placeholder import added correctly.The import statement properly brings in the
SCHEMA_REPLACE_STRINGconstant needed for dynamic schema resolution.
10-17: Schema placeholders implemented consistently across all queries.All SQL queries now properly use the schema placeholder, enabling dynamic schema resolution at runtime. The string concatenation approach is efficient and maintains query readability.
4-4: ```shell
#!/bin/bashSearch for SCHEMA_REPLACE_STRING in the codebase and show its definition with context
rg -n "SCHEMA_REPLACE_STRING" -C2
</details> <details> <summary>core-services/egov-user/src/main/java/org/egov/user/repository/builder/UserTypeQueryBuilder.java (1)</summary> `53-53`: **LGTM! Consistent schema placeholder implementation.** The systematic replacement of hardcoded schema names with `SCHEMA_REPLACE_STRING` placeholders is well-implemented and consistent across all SQL queries. This enables proper tenant-specific schema resolution at runtime. Also applies to: 71-72, 80-80, 83-83, 86-86, 89-89, 92-92, 289-289, 296-296, 305-305 </details> <details> <summary>core-services/mdms-v2/CHANGELOG.md (1)</summary> `7-12`: **Verify version dates and ordering.** There's a chronological inconsistency in the changelog: - Version 2.10.0 is dated 2025-05-09 - Version 2.9.1 is dated 2025-05-21 (later date but lower version) Please verify that the dates are correct and ensure proper semantic versioning order. Also applies to: 13-15 </details> <details> <summary>core-services/egov-user/src/main/java/org/egov/user/persistence/repository/RoleRepository.java (1)</summary> `25-25`: **Excellent implementation of tenant-aware schema resolution.** The integration of `DatabaseSchemaUtils` is well-implemented with: - Clear constructor documentation explaining the new dependency - Consistent application of schema placeholder replacement across all query methods - Proper tenant ID propagation to the schema resolution utility This follows the established pattern for multi-tenant support and maintains consistency with other repository classes in the service. Also applies to: 48-48, 65-79, 93-96, 111-113, 132-135 </details> <details> <summary>core-services/mdms-v2/src/main/java/org/egov/infra/mdms/producer/Producer.java (2)</summary> `3-4`: **Excellent implementation of constructor-based dependency injection and multi-tenant support.** The refactoring to use constructor injection with final fields and the addition of `MultiStateInstanceUtil` for tenant-aware operations follows Spring best practices and properly supports multi-tenant architecture. Also applies to: 9-9, 13-27 --- `37-41`: **Well-implemented tenant-aware topic resolution.** The method correctly computes tenant-specific topic names and includes informative logging. The parameter ordering with `tenantId` first makes the tenant context explicit and mandatory. </details> <details> <summary>core-services/egov-user/src/main/java/org/egov/user/repository/builder/AddressQueryBuilder.java (1)</summary> `5-5`: **Correct implementation of schema placeholder for multi-tenant support.** The replacement of hardcoded schema names with `SCHEMA_REPLACE_STRING` placeholder enables dynamic tenant-specific schema resolution at runtime. This approach is consistent with the broader multi-tenant architecture. Also applies to: 10-10, 12-12 </details> <details> <summary>core-services/egov-user/src/main/java/org/egov/user/domain/service/UserService.java (2)</summary> `361-361`: **Consistent tenant ID propagation throughout service methods.** The updates to pass `tenantId` to repository methods ensure tenant-aware operations across user management functions. This maintains proper tenant isolation. Also applies to: 375-375, 421-421, 426-426, 505-505, 544-544, 562-562 --- `165-165`: **Proper addition of tenant context to user lookup.** The `tenantId` parameter addition to `getUserByUuid` method correctly enables tenant-scoped user operations. The tenant ID is properly passed to the search criteria. Verify that all callers of `getUserByUuid` have been updated to pass the tenant ID parameter: ```shell #!/bin/bash # Description: Find all calls to getUserByUuid method to ensure they pass tenantId # Expected: All calls should include two parameters (uuid, tenantId) ast-grep --pattern 'getUserByUuid($_, $_)'Also applies to: 169-169
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java (1)
4-4: Good implementation of constructor-based dependency injection.The addition of
DatabaseSchemaUtilsdependency and updated constructor with proper documentation follows Spring best practices for dependency management.Also applies to: 19-19, 32-47
core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/SchemaDefinitionDbRepositoryImpl.java (3)
27-57: Excellent use of final fields and comprehensive constructor documentation.The use of final fields and detailed constructor documentation demonstrates good coding practices. The dependency injection is properly implemented for multi-tenant support.
67-67: Correct tenant-aware Kafka message publishing.The updated
producer.pushcall with tenant ID as the first parameter ensures messages are published to tenant-specific topics, maintaining proper multi-tenant isolation.
81-87: Excellent error handling and logging practices.The try-catch block properly handles
InvalidTenantIdExceptionand converts it to aCustomExceptionwith appropriate error code. The use of parameterized logging (log.info("Schema definition search query: {}", query)) is a best practice for performance and security.core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/MdmsDataRepositoryImpl.java (3)
40-40: LGTM: Well-structured dependency injection and constructor documentation.The constructor is properly documented with clear parameter descriptions and the MultiStateInstanceUtil dependency is correctly injected for tenant-aware schema resolution.
Also applies to: 54-68
75-75: LGTM: Correct tenant-aware message publishing.The producer.push calls now correctly include the tenant ID as the first parameter, enabling tenant-specific topic resolution for Kafka messages.
Also applies to: 83-83
94-101: Excellent implementation of tenant-aware schema replacement.The schema replacement logic is consistently implemented with proper error handling. The try-catch blocks appropriately convert InvalidTenantIdException to CustomException with standardized error codes, and the logging provides good debugging visibility.
Also applies to: 112-119
core-services/egov-otp/src/main/java/org/egov/persistence/repository/TokenRepository.java (4)
30-34: LGTM: Consistent schema placeholder usage in SQL constants.All SQL query constants correctly use the SCHEMA_REPLACE_STRING placeholder, enabling dynamic schema resolution at runtime.
49-53: Well-documented constructor with clear dependency injection.The constructor properly documents the purpose of each dependency, making the multi-tenant schema handling clear to maintainers.
58-62: Correct tenant ID handling for state-level operations.The code appropriately distinguishes between original tenant ID (for schema resolution) and state-level tenant ID (for data storage), ensuring proper tenant isolation.
Also applies to: 100-101
139-146: Excellent design: Centralized schema replacement with proper error handling.The private replaceSchemaPlaceholder method centralizes the schema replacement logic and provides consistent error handling by converting InvalidTenantIdException to CustomException. This reduces code duplication and ensures uniform error responses.
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AddressRepository.java (4)
24-30: LGTM: Consistent schema placeholder usage in SQL constants.All SQL query constants are properly updated to use SCHEMA_REPLACE_STRING, enabling dynamic schema resolution for multi-tenant support.
36-47: Well-documented constructor following dependency injection best practices.The constructor documentation clearly explains each dependency's purpose, and the final field declarations improve immutability.
72-74: Consistent schema replacement implementation across all database operations.The schema replacement pattern is uniformly applied across all methods, ensuring tenant-aware database access. The inline comments provide good clarity about the operation being performed.
Also applies to: 87-88, 105-106, 132-134, 177-178, 199-201, 210-212
196-196: Good code quality improvement using CollectionUtils.Replacing manual null/empty checks with
CollectionUtils.isEmptyimproves code readability and consistency with Spring conventions.core-services/egov-user/src/main/java/org/egov/user/utils/DatabaseSchemaUtils.java (5)
10-27: Excellent utility class design with comprehensive documentation.The class documentation clearly explains the purpose and scope, and the configuration properties are well-defined with sensible defaults. The @configuration annotation ensures proper Spring integration.
37-59: Robust schema replacement implementation with proper error handling.The method correctly handles both central instance and non-central instance scenarios. The regex pattern with case-insensitive matching is appropriate, and the tenant ID validation prevents runtime errors.
71-78: Correct tenant level determination logic.The method properly distinguishes between state-level and non-state-level tenants based on the environment configuration and tenant ID structure.
92-106: Well-implemented state-level tenant extraction.The method correctly handles tenant ID parsing for both central and non-central instances, with proper array bounds checking and string concatenation logic.
118-130: Comprehensive topic name generation for multi-tenant messaging.The method supports tenant-specific Kafka topic naming, which is essential for proper message routing in multi-tenant environments.
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/UserRepository.java (5)
53-53: LGTM: Proper dependency injection for multi-tenant support.The DatabaseSchemaUtils dependency is correctly injected and the constructor signature is updated appropriately to support tenant-aware database operations.
Also applies to: 64-68
89-89: Consistent schema replacement implementation across query operations.The schema replacement pattern is uniformly applied across all database queries, ensuring tenant-aware data access throughout the user repository.
Also applies to: 111-112, 131-132, 156-157
342-343: Excellent enhancement: Failed login attempt methods now tenant-aware.The failed login attempt tracking methods now properly accept tenantId parameters, enabling tenant-specific security monitoring and schema-aware database operations.
Also applies to: 346-346, 365-365, 378-378
586-596: Well-documented tenant-aware sequence generation.The getNextSequence method is properly updated to accept tenantId and includes clear documentation about the tenant-specific schema replacement functionality.
459-459: Correct usage of DatabaseSchemaUtils for state-level tenant extraction.The code properly uses the utility method to extract state-level tenant ID for role validation, maintaining consistency with the multi-tenant architecture.
core-services/egov-user/src/test/java/org/egov/user/web/controller/UserControllerTest.java
Show resolved
Hide resolved
core-services/egov-user/src/main/java/org/egov/user/persistence/repository/AuditRepository.java
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core-services/egov-otp/CHANGELOG.md (2)
5-9: Consistent bullet punctuation in CHANGELOGThe new bullets mix ending periods and lack thereof. Previously, bullets in this file did not use trailing periods. For consistency, remove periods from lines 6, 7, and 9, and trim the extra space at the end of line 8:
## 2.10.0 - 2025-05-27 - Introduced tenant-specific database schema support and dynamic schema resolution - Refactored repositories and services to use dynamic schema placeholders and centralized schema utilities - Updated method signatures and constructors for tenant context and new dependencies - Updated test classes for new dependencies - Implemented multi-schema migration scripts and utilities for schema handling
4-4: Add PR reference for traceabilityConsider appending “(PR #768)” to the version header or one of the bullets so readers can quickly trace these changes back to the originating pull request.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core-services/egov-otp/CHANGELOG.md(1 hunks)core-services/egov-otp/pom.xml(2 hunks)core-services/egov-user/CHANGELOG.md(1 hunks)core-services/egov-user/pom.xml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- core-services/egov-user/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- core-services/egov-otp/pom.xml
- core-services/egov-user/pom.xml
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Tests
Chores