-
Notifications
You must be signed in to change notification settings - Fork 544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add rule management component. #6204
base: master
Are you sure you want to change the base?
Conversation
Applicable suggestions are addressed |
Quality Gate passedIssues Measures |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6204 +/- ##
============================================
- Coverage 45.40% 41.20% -4.20%
- Complexity 13941 15555 +1614
============================================
Files 1620 1816 +196
Lines 100555 123703 +23148
Branches 16846 21751 +4905
============================================
+ Hits 45658 50978 +5320
- Misses 48146 65207 +17061
- Partials 6751 7518 +767
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
template.executeInsert(RuleSQLConstants.Query.ADD_RULE, | ||
statement -> { | ||
statement.setString(RuleSQLConstants.Column.UUID, rule.getId()); | ||
statement.setBlob(RuleSQLConstants.Column.RULE_INDEX, ruleJson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use setBinaryStream(String name, InputStream inputStream, long length) method with the column name reference.
https://github.com/wso2/carbon-utils/blob/90b5d4300fafc5d4abaaadb7e41a9a340429f390/components/org.wso2.carbon.database.utils/src/main/java/org/wso2/carbon/database/utils/jdbc/NamedPreparedStatement.java#L226
|
||
/** | ||
* Add a new Rule. | ||
* @param rule Rule object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check formatting.
Add a new line between the description and params.
Check other places as well
* | ||
* @param ruleId Rule ID | ||
* @param tenantId Tenant ID | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing @throws line.
Check other places
public void addRule(Rule rule, int tenantId) throws RuleManagementException { | ||
|
||
try { | ||
addRuleToDB(rule, tenantId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to do following tasks in one transaction, then we need to initiate the jdbcTemplate.withTransaction() from this level
public void updateRule(Rule rule, int tenantId) throws RuleManagementException { | ||
|
||
try { | ||
updateRuleInDB(rule, tenantId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above comment
); | ||
|
||
CREATE TABLE IF NOT EXISTS IDN_RULE_REFERENCES ( | ||
ID INTEGER NOT NULL AUTO_INCREMENT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting
Proposed changes in this pull request
Resolves wso2/product-is#22026
This PR adds the rule management service component.