Refactor ActionManagementDAO to handle action type specific properties #6119

ashanthamara · 2024-11-10T15:10:04Z

Proposed changes in this pull request

With this PR, ActionManagementDAO will be able to handle properties specific to each actionTypes. (Currently for PRE_ISSUE_ACCESS_TOKEN and PRE_UPDATE_PASSWORD)
action-mgt component will utilize certificate-mgt for the operations related to the certificates used in PRE_UPDATE_PASSWORD action type.
Also with this PR, the updateActionEndpointAuthentication will be removed as it has no use case in the product
The IDN_ACTION_ENDPOINT table will be renamed to IDN_ACTION_PROPERTIES and now it will be utilize to store action type related properties as well.

Related Issue

Improve Action Management APIs to support for pre password update action type specific properties. product-is#21661

codecov · 2024-11-10T15:35:21Z

Codecov Report

Attention: Patch coverage is 93.77163% with 18 lines in your changes missing coverage. Please review.

Project coverage is 40.83%. Comparing base (6ffda9f) to head (374b341).

Files with missing lines	Patch %	Lines
...management/internal/ActionMgtServiceComponent.java	0.00%	10 Missing ⚠️
...n/management/dao/impl/ActionManagementDAOImpl.java	96.93%	4 Missing and 2 partials ⚠️
...entity/secret/mgt/core/dao/impl/SecretDAOImpl.java	87.50%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #6119      +/-   ##
============================================
+ Coverage     40.78%   40.83%   +0.05%     
- Complexity    14546    14556      +10     
============================================
  Files          1772     1773       +1     
  Lines        118320   118382      +62     
  Branches      19266    19245      -21     
============================================
+ Hits          48251    48342      +91     
+ Misses        62773    62749      -24     
+ Partials       7296     7291       -5

Flag	Coverage Δ
unit	`25.16% <93.77%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…queries

jenkins-is-staging · 2024-11-10T21:27:50Z

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/11768640854

sonarcloud · 2024-11-10T21:29:50Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jenkins-is-staging · 2024-11-10T22:14:20Z

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11768640854
Status: failure

malithie · 2024-11-11T08:22:05Z

components/action-mgt/org.wso2.carbon.identity.action.management/pom.xml

                                        </limit>
                                        <limit implementation="org.jacoco.report.check.Limit">
                                            <counter>COMPLEXITY</counter>
                                            <value>COVEREDRATIO</value>
                                                <!-- Temp decreasing this. There is an ongoing effort to improve
                                                the code complexity coverage.  -->
-                                            <minimum>0.60</minimum>
+                                            <minimum>0.68</minimum>


Let's keep this to 0.6

malithie · 2024-11-11T08:22:31Z

components/action-mgt/org.wso2.carbon.identity.action.management/pom.xml

@@ -178,14 +183,14 @@
                                            <!-- Temporarily decreasing this.
                                            Will be improved upon in a follow-up effort.
                                            Related issue: https://github.com/wso2/product-is/issues/21368 -->
-                                            <minimum>0.70</minimum>
+                                            <minimum>0.77</minimum>


Target here should be 0.8

malithie · 2024-11-11T08:25:31Z

...nt/src/main/java/org/wso2/carbon/identity/action/management/constant/ActionMgtConstants.java

-    public static final String AUTHN_TYPE_ATTRIBUTE = "authnType";
+    public static final String URI_PROPERTY = "uri";
+    public static final String AUTHN_TYPE_PROPERTY = "authnType";
+    public static final String PASSWORD_SHARING_FORMAT_PROPERTY = "passwordSharingFormat";


This constant should not come to the actions core. It's specific to a particular extension

malithie · 2024-11-11T08:25:39Z

...nt/src/main/java/org/wso2/carbon/identity/action/management/constant/ActionMgtConstants.java

+    public static final String URI_PROPERTY = "uri";
+    public static final String AUTHN_TYPE_PROPERTY = "authnType";
+    public static final String PASSWORD_SHARING_FORMAT_PROPERTY = "passwordSharingFormat";
+    public static final String CERTIFICATE_ID_PROPERTY = "certificateId";


malithie · 2024-11-11T08:27:06Z

...nt/src/main/java/org/wso2/carbon/identity/action/management/constant/ActionMgtConstants.java

@@ -53,19 +55,30 @@ public enum ErrorMessages {
                "%s is empty."),
        ERROR_INVALID_ACTION_REQUEST_FIELD("60005", "Invalid request.",
                "%s is invalid."),
+        ERROR_INVALID_ACTION_CERTIFICATE("60006", "Invalid request.", "Provided certificate is invalid."),


This is specific to a particular extension, we should be able to take these validations out from the core

ashanthamara added 6 commits November 7, 2024 11:39

Add PreUpdatePasswordAction class

0756224

Merge remote-tracking branch 'upstream/master' into action-properties

c207614

Improve action-mgt to handle action related properties

a3a6111

Improve unit tests for DAO

891c883

Merge remote-tracking branch 'upstream/master' into action-properties

9245f16

lower coverage levels

1f987bb

ashanthamara added 5 commits November 11, 2024 02:08

Refactor unit tests

4dca5c1

Rename IDN_ACTION_ENDPOINT table name to IDN_ACTION_PROPERTIES

1296c3a

Add jdbcTemplate.withTransaction for delete-secret and update-secret …

ab1bb18

…queries

Fix sonarcloud suggestions

f4b2f28

Move errors defined to rest-api layer to service layer

374b341

malithie reviewed Nov 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ActionManagementDAO to handle action type specific properties #6119

Refactor ActionManagementDAO to handle action type specific properties #6119

ashanthamara commented Nov 10, 2024 •

edited

Loading

codecov bot commented Nov 10, 2024 •

edited

Loading

jenkins-is-staging commented Nov 10, 2024

sonarcloud bot commented Nov 10, 2024

jenkins-is-staging commented Nov 10, 2024

malithie Nov 11, 2024 •

edited

Loading

malithie Nov 11, 2024

malithie Nov 11, 2024

malithie Nov 11, 2024

malithie Nov 11, 2024

Refactor ActionManagementDAO to handle action type specific properties #6119

Are you sure you want to change the base?

Refactor ActionManagementDAO to handle action type specific properties #6119

Conversation

ashanthamara commented Nov 10, 2024 • edited Loading

Proposed changes in this pull request

Related Issue

codecov bot commented Nov 10, 2024 • edited Loading

Codecov Report

jenkins-is-staging commented Nov 10, 2024

sonarcloud bot commented Nov 10, 2024

Quality Gate passed

jenkins-is-staging commented Nov 10, 2024

malithie Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

malithie Nov 11, 2024

Choose a reason for hiding this comment

malithie Nov 11, 2024

Choose a reason for hiding this comment

malithie Nov 11, 2024

Choose a reason for hiding this comment

malithie Nov 11, 2024

Choose a reason for hiding this comment

ashanthamara commented Nov 10, 2024 •

edited

Loading

codecov bot commented Nov 10, 2024 •

edited

Loading

malithie Nov 11, 2024 •

edited

Loading