Add Async Operation Status Tracking OSGi Component #6641
Conversation
components/async-status-mgt/org.wso2.carbon.identity.framework.async.status.mgt/pom.xml
Outdated
Show resolved
Hide resolved
|
|
||
| public String getCode() { | ||
|
|
||
| return ERROR_PREFIX + code; |
There was a problem hiding this comment.
Normally the error prefix is added in the API layer
| /** | ||
| * Enum for Error Message | ||
| */ | ||
| public static enum ErrorMessages { |
There was a problem hiding this comment.
Lets move this to a separate class in the constant package
| * Enum for Error Message | ||
| */ | ||
| public static enum ErrorMessages { | ||
| ERROR_CODE_INVALID_REQUEST_BODY("xx001", "Invalid request.", |
There was a problem hiding this comment.
Use, 65xxx for server errors and 60xxx for client errors for error codes
...main/java/org/wso2/carbon/identity/framework/async/status/mgt/dao/AsyncStatusMgtDAOImpl.java
Outdated
Show resolved
Hide resolved
| } catch (SQLException e) { | ||
| String errorMessage = | ||
| "Error while registering the asynchronous operation"; | ||
| throw new RuntimeException(errorMessage, e); |
There was a problem hiding this comment.
Rather than throwing the base RuntimeException, let's create a specific runtime exception class specific for this service and use it here.
e.g.,
| int rowsAffected = statement.executeUpdate(); | ||
| LOGGER.info("Async Operation Registering Success. Rows affected: " + rowsAffected); | ||
|
|
||
| ResultSet generatedKeys = statement.getGeneratedKeys(); |
There was a problem hiding this comment.
Let's use try with resource for the ResultSet, or else make sure to close it
| SQLConstants.OperationStatusTableColumns.IDN_OPERATION_POLICY, record.getOperationPolicy()); | ||
|
|
||
| int rowsAffected = statement.executeUpdate(); | ||
| LOGGER.info("Async Operation Registering Success. Rows affected: " + rowsAffected); |
There was a problem hiding this comment.
Do we need to make these logs as info ?
| public List<ResponseOperationRecord> getOperationStatusByOperationTypeAndOperationSubjectId( | ||
| String operationType, String operationSubjectId) { | ||
|
|
||
| String sql = |
There was a problem hiding this comment.
Let's move this to the SQLConstants class.
Check other places
| } | ||
|
|
||
| @Override | ||
| public List<ResponseOperationRecord> getOperationStatusByOperationSubjectTypeAndOperationSubjectIdAndOperationType( |
There was a problem hiding this comment.
Let's make the method names simple, don't need to specify the arguments in the method name.
Check other places
| } | ||
| } catch (SQLException e) { | ||
| String errorMessage = "Error fetching async operation status for subject: " + operationSubjectId; | ||
| LOGGER.info(errorMessage + e); |
There was a problem hiding this comment.
| LOGGER.info(errorMessage + e); | |
| LOGGER.error(errorMessage + e); |
Shouldn't this be an error ?
| List<ResponseOperationRecord> operationRecords; | ||
| NamedJdbcTemplate namedJdbcTemplate = Utils.getNewTemplate(); | ||
| try { | ||
| operationRecords = namedJdbcTemplate.executeQuery(sqlStmt, |
There was a problem hiding this comment.
I suggest to use this NamedJdbcTemplate in other methods as well.
| private FilterQueryBuilder buildFilterQuery(List<ExpressionNode> expressionNodes, String attributeUsedForCursor) | ||
| throws AsyncStatusMgtServerException { | ||
|
|
||
| FilterQueryBuilder filterQueryBuilder = new FilterQueryBuilder(); | ||
| appendFilterQuery(expressionNodes, filterQueryBuilder, attributeUsedForCursor); | ||
| return filterQueryBuilder; | ||
| } | ||
|
|
||
| private void appendFilterQuery(List<ExpressionNode> expressionNodes, FilterQueryBuilder filterQueryBuilder, | ||
| String attributeUsedForCursor) | ||
| throws AsyncStatusMgtServerException { | ||
|
|
||
| int count = 1; | ||
| StringBuilder filter = new StringBuilder(); |
There was a problem hiding this comment.
We should not do this in DAO layer. This should be handled in service layer.
| ResponseOperationRecord record = new ResponseOperationRecord(); | ||
| record.setOperationId(resultSet.getString(1)); | ||
| record.setCorrelationId(resultSet.getString(2)); | ||
| record.setOperationType(resultSet.getString(3)); | ||
| record.setOperationSubjectType(resultSet.getString(4)); | ||
| record.setOperationSubjectId(resultSet.getString(5)); | ||
| record.setResidentOrgId(resultSet.getString(6)); | ||
| record.setInitiatorId(resultSet.getString(7)); | ||
| record.setOperationStatus(resultSet.getString(8)); | ||
| record.setOperationPolicy(resultSet.getString(9)); | ||
| record.setCreatedTime(Timestamp.valueOf(resultSet.getString(10))); | ||
| record.setModifiedTime(Timestamp.valueOf(resultSet.getString(11))); | ||
| return record; |
There was a problem hiding this comment.
If we use object builder pattern for this ResponseOperationRecord class, this part can be further improved.
| private boolean hasExistingRecords(Connection connection, String operationType, String operationSubjectId) | ||
| throws SQLException { | ||
|
|
||
| String checkSql = "SELECT COUNT(*) FROM IDN_ASYNC_OPERATION_STATUS " + |
There was a problem hiding this comment.
| String checkSql = "SELECT COUNT(*) FROM IDN_ASYNC_OPERATION_STATUS " + | |
| String checkSql = "SELECT 1 FROM IDN_ASYNC_OPERATION_STATUS " + |
If we are checking only the existence, this can be more efficient rather than using count
|
|
||
| import javax.sql.DataSource; | ||
|
|
||
| public class Utils { |
There was a problem hiding this comment.
Let's do not have common util class.
Split into separate util classes based on the functionality.
|
Current Limitation
WSO2 Identity Server (IS) performs various asynchronous operations, such as sharing applications and users across organizations, bulk user imports, and outbound provisioning. While error logs exist, they are not convenient for administrators to filter and analyze, making it difficult to identify failures and take corrective actions efficiently. Without such notifications, administrators may be unaware of failures or issues, leading to inconsistencies and delays in corrective actions.
Purpose
This pull request introduces a new OSGi component designed to enhance the WSO2 Carbon Identity framework's ability to manage and track the status of asynchronous operations. This component provides a comprehensive solution for monitoring the progress and outcomes of long-running tasks.
This component takes care of providing an overview of the most recent sharing status related with an asynchronous operation at the feature level.
Key Features and Changes:
Asynchronous Operation Status Tracking: Implements a robust and persistent mechanism for recording and monitoring the status of asynchronous operations within the WSO2 Carbon Identity framework.
Database Persistence:
Introduces two new database tables to persist operation statuses, ensuring data durability across system restarts:
IDN_ASYNC_OPERATION_STATUSandIDN_ASYNC_OPERATION_STATUS_UNIT.OSGi Service Component:
Develops an OSGi service component, comprising the
AsyncStatusMgtServiceinterface and its implementation classAsyncStatusMgtServiceImpl, to manage the lifecycle and data access for asynchronous operation statuses.The
AsyncStatusMgtServiceinterface defines the core methods for registering, updating, retrieving, and querying operation statuses.The
AsyncStatusMgtServiceImplclass provides the concrete implementation, handling database interactions and business logic.Related Issues
wso2/product-is#22806 (comment)