Improve db connection usages. #7670
Conversation
WalkthroughThis pull request refactors database type detection across multiple components to use runtime database product name retrieval from connection metadata instead of static configuration checks. The changes introduce string-based database type verification methods and systematically update DAOs to select database-specific SQL queries at runtime. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.4)components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.javaThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
....mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java
Show resolved
Hide resolved
....mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java
Show resolved
Hide resolved
...main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java
Show resolved
Hide resolved
...main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java
Show resolved
Hide resolved
...rg.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java
Show resolved
Hide resolved
...rg.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java
Show resolved
Hide resolved
...-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
Show resolved
Hide resolved
...-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
Show resolved
Hide resolved
...tity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java
Outdated
Show resolved
Hide resolved
...tity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java
Outdated
Show resolved
Hide resolved
...ity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.java
Show resolved
Hide resolved
...entity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.java
Show resolved
Hide resolved
...entity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.java
Show resolved
Hide resolved
...t/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.java
Show resolved
Hide resolved
...t/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.java
Show resolved
Hide resolved
...on.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.java
Show resolved
Hide resolved
...on.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.java
Show resolved
Hide resolved
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.java (1)
118-122: Inconsistent ResultSet cleanup.The
rsvariable is declared and used (lines 94, 103) butnullis passed tocloseAllConnectionsinstead ofrs. This is inconsistent with the cleanup improvements made in other methods likelistWorkflows(line 326) andgetWorkflowParams(line 488).🔎 Proposed fix
} catch (SQLException e) { throw new InternalWorkflowException(errorMessage, e); } finally { - IdentityDatabaseUtil.closeAllConnections(connection, null, prepStmt); + IdentityDatabaseUtil.closeAllConnections(connection, rs, prepStmt); }components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java (1)
415-422: Connection and ResultSet leak ingetTrustedTokenIssuerSearch.
Connection dbConnectionand theResultSetreturned fromgetTrustedTokenIssuerQueryResultSetare never closed, unlike the non-trusted path which uses try‑with‑resources. This will leak DB resources under load.Consider mirroring the pattern used in
getIdPsSearch(...):Proposed fix using try‑with‑resources
- try { - Connection dbConnection = IdentityDatabaseUtil.getDBConnection(false); - ResultSet resultSet = getTrustedTokenIssuerQueryResultSet(dbConnection, sortedOrder, tenantId, offset, limit, - filterQueryBuilder, requiredAttributes); - return populateIdentityProviderList(resultSet, dbConnection, requiredAttributes, tenantId); - } catch (SQLException e) { + try (Connection dbConnection = IdentityDatabaseUtil.getDBConnection(false); + ResultSet resultSet = getTrustedTokenIssuerQueryResultSet(dbConnection, sortedOrder, tenantId, offset, + limit, filterQueryBuilder, requiredAttributes)) { + return populateIdentityProviderList(resultSet, dbConnection, requiredAttributes, tenantId); + } catch (SQLException e) { String message = "Error occurred while retrieving Identity Provider for tenant: " + IdentityTenantUtil.getTenantDomain(tenantId); throw IdPManagementUtil.handleServerException(IdPManagementConstants.ErrorMessage .ERROR_CODE_CONNECTING_DATABASE, message, e); }components/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java (2)
1061-1113: Same nested transaction pattern asupdateMetadataForMYSQL.This method also creates a new
JdbcTemplateand starts a transaction (lines 1065-1067, 1088) while being called from withinreplaceResource's transaction. The same atomicity concerns apply here. Consider refactoring to accept the outerTemplateto maintain transactional consistency.
567-601: Nested transactions with separate connections risk data inconsistency.The
replaceResourcemethod starts a transaction at line 568, then callsupdateMetadataForMYSQLandupdateMetadataForH2, which each create newJdbcTemplateinstances (lines 1015 and 1065 respectively) and start their own transactions (lines 1017 and 1067). This creates nested transactions with separate connections.If an inner transaction commits but the outer transaction fails, the metadata changes from the inner transaction will persist without being rolled back, causing data inconsistency.
Refactor
updateMetadataForMYSQLandupdateMetadataForH2to accept the existing transaction template from the outer scope instead of creating new JdbcTemplate instances.components/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java (1)
1003-1026: Premature commit breaks transaction atomicity.Same issue as
updateSCIMRoleName: the method commits at line 1019 before the caller (deleteRole) completes its transaction. IfclearRoleAuthorization(line 982) fails afterdeleteSCIMRolecommits, the SCIM deletion persists while the role deletion is rolled back.Remove the commit/rollback calls to let the caller manage transaction boundaries:
🔎 Proposed fix
protected void deleteSCIMRole(String roleName, String tenantDomain, Connection connection) throws IdentityRoleManagementException { int tenantId = IdentityTenantUtil.getTenantId(tenantDomain); // Append internal domain in order to maintain the backward compatibility. roleName = appendInternalDomain(roleName); if (LOG.isDebugEnabled()) { LOG.debug("Deleting the role: " + roleName + " for the role: " + roleName + " in the tenantDomain: " + tenantDomain); } try (NamedPreparedStatement statement = new NamedPreparedStatement(connection, DELETE_SCIM_ROLE_SQL)) { statement.setInt(RoleTableColumns.TENANT_ID, tenantId); statement.setString(RoleTableColumns.ROLE_NAME, roleName); statement.executeUpdate(); - - IdentityDatabaseUtil.commitTransaction(connection); } catch (SQLException e) { - IdentityDatabaseUtil.rollbackTransaction(connection); String errorMessage = "Error while deleting the the role: %s for the role: %s in the tenantDomain: %s"; throw new IdentityRoleManagementServerException(UNEXPECTED_SERVER_ERROR.getCode(), String.format(errorMessage, roleName, roleName, tenantDomain), e); } }
🤖 Fix all issues with AI Agents
In
@components/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java:
- Around line 1015-1058: The code creates a new JdbcTemplate (jdbcTemplate =
JdbcUtils.getNewTemplate()) and calls withTransaction inside replaceResource,
which opens a separate DB connection and breaks atomicity and database-product
checks; instead, remove the new JdbcTemplate and invoke executeInsert on the
existing transaction/template provided by the caller (use the outer
Template/transaction variable in ConfigurationDAOImpl.replaceResource rather
than jdbcTemplate.withTransaction), eliminating the nested withTransaction call
so the metadata insert runs on the same connection as the outer transaction and
any earlier getDatabaseProductName() call.
In
@components/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java:
- Around line 916-937: The updateSCIMRoleName method is incorrectly
committing/rolling back the connection it did not open, breaking atomicity in
the caller updateRoleName; remove the
IdentityDatabaseUtil.commitTransaction(connection) and
IdentityDatabaseUtil.rollbackTransaction(connection) calls from
updateSCIMRoleName and leave exception handling intact (throw
IdentityRoleManagementServerException on SQLException) so the outer
updateRoleName caller (which calls resetPermissionOnUpdateRole and commits/rolls
back) manages the transaction lifecycle.
🧹 Nitpick comments (11)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java (2)
278-305: DB-product-aware H2 query selection looks correct; consider minor error-handling tweak.Using
dbConnection.getMetaData().getDatabaseProductName()plusisH2DB(databaseProductName)to switch betweenGET_SP_METADATA_BY_SP_ID_H2and the generic query is a good alignment with connection-specific DB type and keeps non-H2 databases on the standard path (DB2, MySQL, Oracle, MSSQL, PostgreSQL, etc.). Based on learnings, this preserves multi-DB support.Minor optional refinements:
- You might move the
getMetaData().getDatabaseProductName()call inside thetryso that failures there are wrapped by the same contextual message as the rest of the JDBC work.- The catch rethrows a new
SQLException; if you care about SQLState/vendor code, you could either rethrow the originalSQLExceptionor log the detailed info before wrapping.
315-347: Consistent H2 handling in inserts; consider centralizing DB product lookup and aligning otherisH2DBusages.Selecting between
ADD_SP_METADATA_H2andADD_SP_METADATAusingisH2DB(databaseProductName)mirrors the read path and keeps H2-specific quoting isolated to the H2 query while other DBs continue using the generic SQL, which is good for cross-DB compatibility. Based on learnings, this continues to support the required DB set.Optional follow-ups:
- You’re calling
dbConnection.getMetaData().getDatabaseProductName()in multiple nearby methods; consider a small helper (or reusing a cached per-connection value) to avoid duplication and keep the error-handling policy in one place.- There are still no-arg
JdbcUtils.isH2DB()usages elsewhere in this class (e.g., SP property lookups). For consistency with this PR’s approach of basing decisions on the activeConnection, you may want to migrate those call sites to the newdatabaseProductName-driven overload as well.components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.java (1)
1-2: Update copyright year.The copyright year should reflect the current year (2026) or a range ending in the current year (e.g., 2025-2026). As per coding guidelines, ensure proper license header format.
🔎 Proposed fix
/* - * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2025-2026, WSO2 LLC. (http://www.wso2.com).components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java (1)
137-146: Consider null-safety for the databaseProductName parameter.If
databaseProductNameisnull, these methods will returnfalse(sinceequalsIgnoreCasehandles null gracefully on the constant side), which is acceptable behavior. However, you may want to add explicit null handling or document this behavior for clarity.components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.java (2)
1-2: Update copyright year.The copyright year should reflect the current year (2026) or a range ending in the current year (e.g., 2025-2026). As per coding guidelines, ensure proper license header format.
🔎 Proposed fix
/* - * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2025-2026, WSO2 LLC. (http://www.wso2.com).
394-423: Consider removing unusedDataAccessExceptionfrom method signature.The refactored
getSqlQuerymethod no longer calls any methods that throwDataAccessException, yet it's still declared in the throws clause. This can be cleaned up for accuracy.The method correctly supports all required database types per coding guidelines: H2, MySQL, MariaDB, Oracle, MSSQL, PostgreSQL, and DB2. Based on learnings, all required database types are properly covered.
🔎 Proposed fix
private String getSqlQuery(String filterField, Connection connection) - throws InternalWorkflowException, DataAccessException, WorkflowClientException, SQLException { + throws InternalWorkflowException, WorkflowClientException, SQLException {components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java (2)
1151-1154: Oracle/H2 handling inaddIdentityProviderPropertiesis sound; consider renaming local flag.The new
databaseProductNamebranching and H2‑specificADD_IDP_METADATA_H2keep all required DBs supported while handling Oracle’s empty‑string semantics viaisOracleDB. Only minor nit is the local variable name shadowing the imported method, which is a bit confusing.A small cleanup would improve readability:
Proposed rename for clarity
- String databaseProductName = dbConnection.getMetaData().getDatabaseProductName(); - boolean isOracleDB = isOracleDB(databaseProductName); - String sqlStmt = isH2DB(databaseProductName) ? IdPManagementConstants.SQLQueries.ADD_IDP_METADATA_H2 : + String databaseProductName = dbConnection.getMetaData().getDatabaseProductName(); + boolean isOracleDatabase = isOracleDB(databaseProductName); + String sqlStmt = isH2DB(databaseProductName) ? IdPManagementConstants.SQLQueries.ADD_IDP_METADATA_H2 : IdPManagementConstants.SQLQueries.ADD_IDP_METADATA; ... - if (isOracleDB ? StringUtils.isNotEmpty(property.getValue()) : property.getValue() != null) { + if (isOracleDatabase ? StringUtils.isNotEmpty(property.getValue()) : property.getValue() != null) {Based on learnings, this keeps the DAO’s multi‑DB behavior aligned across DB2, H2, MSSQL, MySQL, Oracle, and PostgreSQL.
Also applies to: 1175-1177
6426-6429: Good H2 query selection ingetIdPNameByMetadataProperty, but avoid logging raw metadata values.The H2‑aware switch between
GET_IDP_NAME_BY_METADATA_H2and the generic query is correct. However, the error message still concatenates the rawvalue, which might contain identifiers or other sensitive data and end up in logs.Consider omitting or masking the value in the exception message:
Proposed safer error message
- } catch (SQLException e) { - throw new IdentityProviderManagementException("Error occurred while retrieving Identity Provider " + - "information for IDP metadata property name: " + property + " value: " + value, e); + } catch (SQLException e) { + throw new IdentityProviderManagementException( + "Error occurred while retrieving Identity Provider information for IDP metadata property: " + + property, e); }This keeps logs useful without risking sensitive data exposure.
Also applies to: 6441-6443
components/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java (2)
1-2: Update the copyright year to include 2026.The copyright header shows
2018, but per coding guidelines, it should be the current year or a range ending in the current year (e.g.,2018-2026).-/* - * Copyright (c) 2018, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. +/* + * Copyright (c) 2018-2026, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
943-971: Inconsistent DB type checking pattern.This method uses static
isH2DB(),isPostgreSQLDB(), etc. without passingdatabaseProductName, whilereplaceResourceandupdateMetadataForMYSQLuse the new overloaded methods with the product name parameter. For consistency with the PR's objective of improving DB connection usages, consider applying the same pattern here.🔎 Suggested refactor to use databaseProductName
public void replaceAttribute(String attributeId, String resourceId, Attribute attribute) throws ConfigurationManagementException { JdbcTemplate jdbcTemplate = JdbcUtils.getNewTemplate(); try { + String databaseProductName = jdbcTemplate.getDatabaseProductName(); String query; - if (isH2DB()) { + if (isH2DB(databaseProductName)) { query = INSERT_OR_UPDATE_ATTRIBUTE_H2; - } else if (isPostgreSQLDB()) { + } else if (isPostgreSQLDB(databaseProductName)) { query = INSERT_OR_UPDATE_ATTRIBUTE_POSTGRESQL; - } else if (isMSSqlDB() || isDB2DB()) { + } else if (isMSSqlDB(databaseProductName) || isDB2DB(databaseProductName)) { query = INSERT_OR_UPDATE_ATTRIBUTE_MSSQL_OR_DB2; - } else if (isOracleDB()) { + } else if (isOracleDB(databaseProductName)) { query = INSERT_OR_UPDATE_ATTRIBUTE_ORACLE; } else { query = INSERT_OR_UPDATE_ATTRIBUTE_MYSQL; }components/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java (1)
1-2: Update the copyright year to include 2026.The copyright header shows
2020, but per coding guidelines, it should be the current year or a range ending in the current year (e.g.,2020-2026).-/* - * Copyright (c) 2020, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. +/* + * Copyright (c) 2020-2026, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.javacomponents/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.javacomponents/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.javacomponents/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.javacomponents/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026). Use the Apache License 2.0 header format specified.
All public methods should have a docstring.
Comments should start with a space and first letter capitalized.
Comments should always end with a period.
If there is string concatenation in a debug log statement, then havingif (LOG.isDebugEnabled())is mandatory to avoid unnecessary computation.
For simple log messages (e.g., static strings or simple variable interpolation), you can useLOG.debugdirectly without the debug check.
Scrutinize all user-controlled input for potential SQL Injection, Cross-Site Scripting (XSS), or Command Injection vulnerabilities.
Search for and eliminate any hardcoded secrets such as API keys, passwords, or private tokens.
Verify that any new endpoints or data access functions have proper authorization and permission checks to prevent broken access control.
Ensure that no sensitive user data (e.g., PII, credentials) is being logged or sent in error messages.
Files:
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.javacomponents/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.javacomponents/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.javacomponents/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.javacomponents/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
**/*DAO.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All database queries in the DAO layer should support the following database types: DB2, H2, MS SQL Server, MySQL, Oracle, and PostgreSQL. If a query is not supported by one of these databases, it should be fixed or documented.
Files:
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.javacomponents/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: wso2/carbon-identity-framework PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-05T05:39:39.932Z
Learning: Applies to **/*DAO.java : All database queries in the DAO layer should support the following database types: DB2, H2, MS SQL Server, MySQL, Oracle, and PostgreSQL. If a query is not supported by one of these databases, it should be fixed or documented.
📚 Learning: 2026-01-05T05:39:39.932Z
Learnt from: CR
Repo: wso2/carbon-identity-framework PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-05T05:39:39.932Z
Learning: Applies to **/*DAO.java : All database queries in the DAO layer should support the following database types: DB2, H2, MS SQL Server, MySQL, Oracle, and PostgreSQL. If a query is not supported by one of these databases, it should be fixed or documented.
Applied to files:
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.javacomponents/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.javacomponents/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.javacomponents/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
🧬 Code graph analysis (4)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.java (1)
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java (1)
JdbcUtils(40-388)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java (1)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationMgtDBQueries.java (1)
ApplicationMgtDBQueries(32-707)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.java (2)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/SQLConstants.java (1)
SQLConstants(28-538)components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java (1)
JdbcUtils(40-388)
components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java (1)
components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/util/IdPManagementConstants.java (2)
IdPManagementConstants(28-753)SQLQueries(189-668)
🔇 Additional comments (13)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestAssociationDAO.java (1)
270-270: LGTM! Improved resource cleanup.The explicit initialization of the
ResultSetand passing it toIdentityDatabaseUtil.closeAllConnectionsensures proper resource cleanup. This prevents potential resource leaks that could occur when theResultSetwas not being closed (previously passed asnull).Also applies to: 297-297
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowRequestDAO.java (1)
136-136: LGTM! Improved resource cleanup.The updated call to
IdentityDatabaseUtil.closeAllConnectionsnow correctly passes theResultSet(rs) instead ofnull, ensuring theResultSetis properly closed and preventing potential resource leaks.components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/util/Utils.java (1)
71-72: The null-safety check is already handled correctly.The
JdbcUtils.isPostgreSQLDB(String databaseProductName)method usesPOSTGRE_SQL.equalsIgnoreCase(databaseProductName), which safely handles null input by returningfalse. This is standard Java String behavior—equalsIgnoreCase()does not throw an exception when comparing with null; it simply returnsfalse.components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/WorkflowDAO.java (4)
259-286: LGTM!The
getSqlQuery(connection)call correctly passes the connection to derive the database product name. The try-with-resources pattern ensures proper cleanup of PreparedStatement and ResultSet.
325-327: LGTM!Good improvement to include
rsincloseAllConnectionsto ensure proper ResultSet cleanup.
487-489: LGTM!Proper ResultSet cleanup by including
rsincloseAllConnections.
500-522: LGTM!The refactored
getSqlQuerymethod correctly:
- Accepts a
Connectionparameter to derive the database product name from connection metadata- Supports all required database types per coding guidelines: H2, MySQL, MariaDB, Oracle, MSSQL, PostgreSQL, and DB2
- Throws an appropriate exception for unsupported databases
This is a cleaner approach than static database type checks. Based on learnings, all required database types (DB2, H2, MS SQL Server, MySQL, Oracle, and PostgreSQL) are supported.
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java (1)
160-169: LGTM!The new overloaded methods for database type checking using
databaseProductNamestrings are well-implemented:
- Consistent pattern across all methods
- Proper Javadoc documentation
- Case-insensitive comparison using
equalsIgnoreCaseisMSSqlDB(String)correctly checks bothMICROSOFTandS_MICROSOFTconstants, consistent with the existingDatabaseenum overloadAlso applies to: 182-191, 216-225, 250-259, 284-293, 318-327, 352-361
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/dao/AssociationDAO.java (3)
156-189: LGTM!The
getSqlQuery(filterField, connection)call correctly passes the connection to derive the database product name. The try-with-resources pattern ensures proper cleanup.
202-225: LGTM!Good improvement to properly declare and close the
ResultSetby including it incloseAllConnections.
284-312: LGTM!Proper ResultSet cleanup by declaring
rsand including it incloseAllConnections.components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java (1)
1060-1063: H2-specific metadata query selection ingetIdentityPropertiesByIdpIdlooks good.Using
databaseProductNamewithisH2DB(databaseProductName)to switch betweenGET_IDP_METADATA_BY_IDP_ID_H2and the generic query cleanly fixes the H2VALUE-column issue without affecting other supported DBs. Resource handling and exception propagation remain consistent with the rest of the DAO.Also applies to: 1129-1130
components/role-mgt/org.wso2.carbon.identity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java (1)
866-914: Verify transaction isolation with the updated helper methods.The refactoring to pass the connection to
updateSCIMRoleNameis a good improvement for connection reuse. However, ensure that after fixing the premature commit issue inupdateSCIMRoleName, the transaction boundary is correctly maintained by this outer method'scommitUserDBTransactionat line 899.
| JdbcTemplate jdbcTemplate = JdbcUtils.getNewTemplate(); | ||
| final String finalQuery = query; | ||
| jdbcTemplate.withTransaction(template -> | ||
| template.executeInsert( | ||
| useCreatedTime ? finalQuery : | ||
| INSERT_OR_UPDATE_RESOURCE_MYSQL_WITHOUT_CREATED_TIME, | ||
| preparedStatement -> { | ||
| int initialParameterIndex = 1; | ||
| preparedStatement.setString(initialParameterIndex, resource.getResourceId()); | ||
| if (isOracle) { | ||
| preparedStatement.setInt(++initialParameterIndex, | ||
| PrivilegedCarbonContext.getThreadLocalCarbonContext() | ||
| .getTenantId()); | ||
| preparedStatement.setString(++initialParameterIndex, resource.getResourceName()); | ||
| if (useCreatedTime) { | ||
| preparedStatement.setTimestamp(++initialParameterIndex, currentTime, calendar); | ||
| } | ||
| preparedStatement.setTimestamp(++initialParameterIndex, currentTime, calendar); | ||
| /* | ||
| Resource files are uploaded using a separate endpoint. Therefore resource creation does | ||
| not create files. It is allowed to create a resource without files or attributes in order | ||
| to allow file upload after resource creation. | ||
| */ | ||
|
|
||
| if (isOracleOrMssql) { | ||
| preparedStatement.setInt(++initialParameterIndex, 0); | ||
| preparedStatement.setInt(++initialParameterIndex, isAttributeExists ? 1 : 0); | ||
| } else { | ||
| preparedStatement.setBoolean(++initialParameterIndex, false); | ||
| preparedStatement.setBoolean(++initialParameterIndex, isAttributeExists); | ||
| } | ||
| preparedStatement.setInt(++initialParameterIndex, 0); | ||
| preparedStatement.setInt(++initialParameterIndex, isAttributeExists ? 1 : 0); | ||
| preparedStatement.setString(++initialParameterIndex, resourceTypeId); | ||
| }, resource, false) | ||
| ); | ||
| } catch (DataAccessException e) { | ||
| throw handleServerException(ERROR_CODE_CHECK_DB_METADATA, e.getMessage(), e); | ||
| } | ||
| preparedStatement.setString(++initialParameterIndex, resource.getResourceId()); | ||
| } | ||
| preparedStatement.setInt(++initialParameterIndex, | ||
| PrivilegedCarbonContext.getThreadLocalCarbonContext() | ||
| .getTenantId()); | ||
| preparedStatement.setString(++initialParameterIndex, resource.getResourceName()); | ||
| if (useCreatedTime) { | ||
| preparedStatement.setTimestamp(++initialParameterIndex, currentTime, calendar); | ||
| } | ||
| preparedStatement.setTimestamp(++initialParameterIndex, currentTime, calendar); | ||
| /* | ||
| Resource files are uploaded using a separate endpoint. Therefore resource creation does | ||
| not create files. It is allowed to create a resource without files or attributes in order | ||
| to allow file upload after resource creation. | ||
| */ | ||
|
|
||
| if (isOracleOrMssql) { | ||
| preparedStatement.setInt(++initialParameterIndex, 0); | ||
| preparedStatement.setInt(++initialParameterIndex, isAttributeExists ? 1 : 0); | ||
| } else { | ||
| preparedStatement.setBoolean(++initialParameterIndex, false); | ||
| preparedStatement.setBoolean(++initialParameterIndex, isAttributeExists); | ||
| } | ||
| preparedStatement.setString(++initialParameterIndex, resourceTypeId); | ||
| }, resource, false) | ||
| ); |
There was a problem hiding this comment.
Nested transaction creates separate connection.
Lines 1015-1017 create a new JdbcTemplate and call withTransaction, but this method is invoked from within replaceResource's existing transaction block. The new JdbcTemplate will obtain a separate connection, so:
- The metadata update commits independently of the outer transaction
- If the outer transaction rolls back, the metadata change persists
- If
getDatabaseProductName()was called earlier on a different connection, the product name might not match
Consider passing the Template from the calling transaction to maintain atomicity:
🔎 Suggested approach
- private void updateMetadataForMYSQL(Resource resource, String resourceTypeId, boolean isAttributeExists,
- Timestamp currentTime, boolean useCreatedTime, String databaseProductName)
- throws TransactionException {
+ private void updateMetadataForMYSQL(Template<?> template, Resource resource, String resourceTypeId,
+ boolean isAttributeExists, Timestamp currentTime, boolean useCreatedTime,
+ String databaseProductName) throws DataAccessException {
String query = INSERT_OR_UPDATE_RESOURCE_MYSQL;
// ... query selection logic ...
boolean isOracleOrMssql = isOracle || isMSSqlDB(databaseProductName);
- JdbcTemplate jdbcTemplate = JdbcUtils.getNewTemplate();
- final String finalQuery = query;
- jdbcTemplate.withTransaction(template ->
- template.executeInsert(
- useCreatedTime ? finalQuery : INSERT_OR_UPDATE_RESOURCE_MYSQL_WITHOUT_CREATED_TIME,
+ template.executeInsert(
+ useCreatedTime ? query : INSERT_OR_UPDATE_RESOURCE_MYSQL_WITHOUT_CREATED_TIME,
// ... rest of prepared statement setup ...
- }, resource, false)
- );
+ }, resource, false);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@components/configuration-mgt/org.wso2.carbon.identity.configuration.mgt.core/src/main/java/org/wso2/carbon/identity/configuration/mgt/core/dao/impl/ConfigurationDAOImpl.java
around lines 1015-1058, The code creates a new JdbcTemplate (jdbcTemplate =
JdbcUtils.getNewTemplate()) and calls withTransaction inside replaceResource,
which opens a separate DB connection and breaks atomicity and database-product
checks; instead, remove the new JdbcTemplate and invoke executeInsert on the
existing transaction/template provided by the caller (use the outer
Template/transaction variable in ConfigurationDAOImpl.replaceResource rather
than jdbcTemplate.withTransaction), eliminating the nested withTransaction call
so the metadata insert runs on the same connection as the outer transaction and
any earlier getDatabaseProductName() call.
...tity.role.mgt.core/src/main/java/org/wso2/carbon/identity/role/mgt/core/dao/RoleDAOImpl.java
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Pull request overview
This PR improves database connection usage by obtaining the database product name directly from the connection instead of relying on thread-local context. The changes also fix resource leaks by ensuring ResultSet objects are properly closed in finally blocks.
- Pass database product name from connection to database type checking methods
- Fix resource leaks by properly closing ResultSet objects
- Update exception handling from DataAccessException to SQLException where appropriate
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| JdbcUtils.java | Adds new overloaded methods that accept databaseProductName as a parameter for database type checking (H2, DB2, MySQL, MSSql, MariaDB, PostgreSQL, Informix, Oracle) |
| Utils.java | Updates generatePrepStmt to get database product name from connection and pass it to JdbcUtils.isPostgreSQLDB() |
| WorkflowRequestDAO.java | Fixes resource leak by properly closing ResultSet in retrieveWorkflow method |
| WorkflowRequestAssociationDAO.java | Initializes ResultSet to null and ensures it's properly closed in getWorkflowAssociationsForRequest method |
| WorkflowDAO.java | Modifies getSqlQuery to accept Connection parameter, retrieves database product name, and passes it to all JdbcUtils methods; also updates method documentation |
| AssociationDAO.java | Similar changes to WorkflowDAO - getSqlQuery now accepts Connection parameter and passes database product name to JdbcUtils methods; fixes resource leaks |
| IdPManagementDAO.java | Updates three methods to retrieve database product name from connection and pass it to JdbcUtils methods; changes exception handling from DataAccessException to SQLException |
| ApplicationDAOImpl.java | Updates getServicePropertiesBySpId and addServiceProviderProperties to retrieve database product name from connection and pass it to isH2DB; changes exception handling from DataAccessException to SQLException |
| ConfigurationDAOImpl.java | Updates replaceResource and updateMetadataForMYSQL to retrieve and use database product name; updates exception handling to include DataAccessException; removes unnecessary try-catch blocks in updateMetadataForH2 |
| if (isH2DB()) { | ||
| query = INSERT_OR_UPDATE_ATTRIBUTE_H2; | ||
| } else if (isPostgreSQLDB()) { | ||
| query = INSERT_OR_UPDATE_ATTRIBUTE_POSTGRESQL; | ||
| } else if (isMSSqlDB() || isDB2DB()) { | ||
| query = INSERT_OR_UPDATE_ATTRIBUTE_MSSQL_OR_DB2; | ||
| } else if (isOracleDB()) { | ||
| query = INSERT_OR_UPDATE_ATTRIBUTE_ORACLE; | ||
| } else { | ||
| query = INSERT_OR_UPDATE_ATTRIBUTE_MYSQL; |
There was a problem hiding this comment.
The database type checking methods are being called without passing the database product name. Since this method uses JdbcTemplate which has database connection context, you should obtain the database product name and pass it to these methods to be consistent with the PR's objective of improving database connection usage. Consider getting the database product name before the database type checks similar to how it's done in the replaceResource method.
| /** | ||
| * Get SQL query to retrieve workflows of a tenant with pagination. | ||
| * | ||
| * @param connection Database connection. | ||
| * @throws InternalWorkflowException | ||
| * @throws DataAccessException | ||
| * @throws SQLException | ||
| */ |
There was a problem hiding this comment.
| private String getSqlQuery(String filterField, Connection connection) | ||
| throws InternalWorkflowException, DataAccessException, WorkflowClientException, SQLException { |
There was a problem hiding this comment.
The DataAccessException in the method signature is no longer needed. Since the method now receives a Connection parameter and calls connection.getMetaData().getDatabaseProductName() which only throws SQLException, the DataAccessException can be removed from the throws clause.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java (1)
1-17: Update copyright year to 2026.The copyright year is currently 2025 but should be updated to 2026 or a range ending in the current year (e.g., 2018-2026).
As per coding guidelines, all Java files must contain the appropriate license header with the copyright year as the current year or a range ending in the current year.
📝 Proposed fix for the copyright year
/* - * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com). * * WSO2 LLC. licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except
🧹 Nitpick comments (1)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java (1)
72-79: Consider making database name constants static final.The database name constants
POSTGRESQL_DB_NAMEandMSSQL_DB_NAMEshould be declared asstatic finalsince they are compile-time constants that don't change between test instances.♻️ Proposed refactor for constants
@Mock private DatabaseMetaData mockDatabaseMetaData; - private final String POSTGRESQL_DB_NAME = "PostgreSQL"; - private final String MSSQL_DB_NAME = "MSSQL"; + private static final String POSTGRESQL_DB_NAME = "PostgreSQL"; + private static final String MSSQL_DB_NAME = "MSSQL";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.javacomponents/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026). Use the Apache License 2.0 header format specified.
All public methods should have a docstring.
Comments should start with a space and first letter capitalized.
Comments should always end with a period.
If there is string concatenation in a debug log statement, then havingif (LOG.isDebugEnabled())is mandatory to avoid unnecessary computation.
For simple log messages (e.g., static strings or simple variable interpolation), you can useLOG.debugdirectly without the debug check.
Scrutinize all user-controlled input for potential SQL Injection, Cross-Site Scripting (XSS), or Command Injection vulnerabilities.
Search for and eliminate any hardcoded secrets such as API keys, passwords, or private tokens.
Verify that any new endpoints or data access functions have proper authorization and permission checks to prevent broken access control.
Ensure that no sensitive user data (e.g., PII, credentials) is being logged or sent in error messages.
Files:
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: wso2/carbon-identity-framework PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-05T05:39:39.932Z
Learning: Applies to **/*DAO.java : All database queries in the DAO layer should support the following database types: DB2, H2, MS SQL Server, MySQL, Oracle, and PostgreSQL. If a query is not supported by one of these databases, it should be fixed or documented.
🧬 Code graph analysis (1)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java (1)
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/JdbcUtils.java (1)
JdbcUtils(40-388)
🔇 Additional comments (2)
components/workflow-mgt/org.wso2.carbon.identity.workflow.mgt/src/test/java/org/wso2/carbon/identity/workflow/mgt/util/UtilsTest.java (2)
110-131: LGTM! PostgreSQL test correctly validates database-specific parameter ordering.The test properly mocks DatabaseMetaData to return the PostgreSQL product name and verifies that parameters are bound in the correct order for PostgreSQL databases (tenant, filter, limit, offset). This aligns with the PR's goal of using runtime database product name retrieval.
133-154: LGTM! Non-PostgreSQL test correctly validates standard parameter ordering.The test properly mocks DatabaseMetaData to return the MSSQL product name and verifies that parameters are bound in the standard order for non-PostgreSQL databases (tenant, filter, offset, limit). This ensures the code handles different database types correctly.
|
2 participants
Purpose
Related issues
Integration tests run]
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.